Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Mon, 5 Dec 2022 12:05:14 +0000
From:   Mark Brown <broonie@kernel.org>
To:     Ilpo =?iso-8859-1?Q?J=E4rvinen?= <ilpo.jarvinen@linux.intel.com>
Cc:     Xu Yilun <yilun.xu@intel.com>, linux-fpga@vger.kernel.org,
        Wu Hao <hao.wu@intel.com>, Tom Rix <trix@redhat.com>,
        Moritz Fischer <mdf@kernel.org>, Lee Jones <lee@kernel.org>,
        Matthew Gerlach <matthew.gerlach@linux.intel.com>,
        Russ Weight <russell.h.weight@intel.com>,
        Tianfei zhang <tianfei.zhang@intel.com>,
        Greg KH <gregkh@linuxfoundation.org>,
        Marco Pagani <marpagan@redhat.com>,
        LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v3 7/9] mfd: intel-m10-bmc: Add PMCI driver
Message-ID: <Y43eejWSYIBIlUKB@sirena.org.uk>
References: <20221202100841.4741-1-ilpo.jarvinen@linux.intel.com>
 <20221202100841.4741-8-ilpo.jarvinen@linux.intel.com>
 <Y4ox5J0junaUYyT7@yilunxu-OptiPlex-7050>
 <855d463e-fb84-1910-f53-58e6b0a633a4@linux.intel.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
        protocol="application/pgp-signature"; boundary="eKKE6fgqZADZWzdx"
Content-Disposition: inline
In-Reply-To: <855d463e-fb84-1910-f53-58e6b0a633a4@linux.intel.com>
Precedence: bulk


--eKKE6fgqZADZWzdx
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Mon, Dec 05, 2022 at 11:51:15AM +0200, Ilpo J=E4rvinen wrote:
> On Sat, 3 Dec 2022, Xu Yilun wrote:
> > On 2022-12-02 at 12:08:39 +0200, Ilpo J=E4rvinen 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=20
> 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.

--eKKE6fgqZADZWzdx
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQEzBAABCgAdFiEEreZoqmdXGLWf4p/qJNaLcl1Uh9AFAmON3nkACgkQJNaLcl1U
h9Co7Qf/eIz7h6ibggGjkJrx3WaPVwE60Kkxjo2FsG52Nc0CbeV9PYzJZD4Y8cTd
j2GO5Y0k91Sq+K01GdWIUXoBuU6j9cj5PVTh7oSMTLoE9VRgyDKzz8E44heZRdI/
N1mGPiAykoqoiftA7oD//33t4klMzLdp2Rylg+uFRzOi+U+qdSOHMrAheUQ9U0oh
miqElD3SlnJCbEIuhP3l0PYmQ0nTi/+GlXIbQWp6LSZeDCpdlQxnDgi/8ObLp4KT
efZMN6bdv68UcWpNQdbXiwZru1jOs5IGg60MES8gTpQWM4PxVv/uEE1rvAmL9dCp
jcmtWY37whnNfjf5vUGq4IPMMTy5vw==
=CtcO
-----END PGP SIGNATURE-----

--eKKE6fgqZADZWzdx--