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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 5 Dec 2022 12:05:14 +0000
From:   Mark Brown <broonie@...nel.org>
To:     Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
Cc:     Xu Yilun <yilun.xu@...el.com>, linux-fpga@...r.kernel.org,
        Wu Hao <hao.wu@...el.com>, Tom Rix <trix@...hat.com>,
        Moritz Fischer <mdf@...nel.org>, Lee Jones <lee@...nel.org>,
        Matthew Gerlach <matthew.gerlach@...ux.intel.com>,
        Russ Weight <russell.h.weight@...el.com>,
        Tianfei zhang <tianfei.zhang@...el.com>,
        Greg KH <gregkh@...uxfoundation.org>,
        Marco Pagani <marpagan@...hat.com>,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v3 7/9] mfd: intel-m10-bmc: Add PMCI driver

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.

Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)