Re: [PATCH v3 7/9] mfd: intel-m10-bmc: Add PMCI driver

[Mark Brown <broonie@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 1402 bytes --]

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.

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.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]