On 2022-12-05 at 12:05:14 +0000, Mark Brown wrote:
> On Mon, Dec 05, 2022 at 11:51:15AM +0200, Ilpo Järvinen wrote:
> > On Sat, 3 Dec 2022, Xu Yilun wrote:
> > > On 2022-12-02 at 12:08:39 +0200, Ilpo Järvinen wrote:
>
> > > > +struct regmap *__devm_m10_regmap_indirect(struct device *dev,
>
> > > We name the file intel-m10-bmc-pmci-xxx.c, and this function
> > > xx_m10_regmap_xx(). But I can see the implementation is just about the indirect
> > > bus which from your commit message could be used by various DFL features
> > > like HSSI or PMCI. So is it better we put the implementation in
> > > drivers/fpga and name the file dfl-indirect-regmap.c and the
> > > initialization function dfl_indirect_regmap_init()?
>
> > I guess that would be doable unless Mark objects. My understanding was
> > that he preferred to have in the driver that is currently using it.
>
> > Mark, any opinion on this?
>
> The above does not look good. As I have said several times now drivers
> implementing their own regmap operations should use the reg_read() and
> reg_write() operations in regmap_config when allocating their regmap
> unless they're doing something unusual. There are a few cases where it
> makes sense but nothing I've seen here makes it look like this is one of
> them. Most of the current users don't fit.
It is good for now to implement the indirect access interface in
regmap_config, as intel-m10-bmc is the only one who uses it. But I'm not
sure when a second IP block(like HSSI) in intel FPGA uses it, how to
implement? A shared library?
Some background about hardware:
Several IP blocks in intel FPGA integrate the same mmio register layout
(so called indirect access interface here) as the bridge to the IP's real
registers address space. Like:
+---------+ +---------+
| m10 BMC | | HSSI |
+---------+ +---------+
|indirect | |indirect |
| access | | access |
| MMIOs | | MMIOs |
+----+----+ +----+----+
| |
| |
+----+-----+ +---------+
|m10 bmc | | HSSI |
|registers | |registers|
+----------+ +---------+
Thanks,
Yilun
>
> Please, just implement a normal driver using a normal regmap_config as
> I've repeatedly said you should if you don't want to provide something
> generic.
