From: "Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>
To: Mark Brown <broonie@kernel.org>
Cc: linux-fpga@vger.kernel.org, Xu Yilun <yilun.xu@intel.com>,
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>,
"Rafael J. Wysocki" <rafael@kernel.org>,
LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v2 07/11] regmap: indirect: Add indirect regmap support
Date: Mon, 21 Nov 2022 15:37:40 +0200 (EET) [thread overview]
Message-ID: <a97ce076-4ca6-5f1b-eba4-4068dcb64b3d@linux.intel.com> (raw)
In-Reply-To: <Y3eOzOmTeTKBoPrd@sirena.org.uk>
[-- Attachment #1: Type: text/plain, Size: 3413 bytes --]
On Fri, 18 Nov 2022, Mark Brown wrote:
> On Fri, Nov 18, 2022 at 02:49:45PM +0200, Ilpo Järvinen wrote:
> > On Thu, 17 Nov 2022, Mark Brown wrote:
>
> > > No, what I'm objecting to there is pretty much the same thing I'm
> > > saying here - this doesn't seem like it's a particularly generic
> > > implementation and I'm really not clear that there'd be anything
> > > meaningful left by the time the implementation assumptions are
> > > removed.
>
> > That's probably because it sounds to me you're trying to extend its
> > genericness beyond the domain where it's generic. That is, you're looking
> > for genericness outside of IPs (that have their own driver each) in Intel
> > FPGA domain.
>
> This just says it's adding "indirect regmap support" - there's
> nothing here saying that it's some Intel specific thing but it's
> quite specific to some IPs.
Yeah, but it's that way mainly because of your earlier comments. :-)
I tried to make it more "generic" to the extent possible because of your
concern related to genericness and I therefore intentionally put the Intel
specific numbers into the other change.
Previously you were against saying it clearly that it's Intel FPGA
specific when Matthew proposed changing the name to not sound something
too generic. If you're ok with that now, I'm happy to make such change.
> Perhaps you have some name for this
> interface? You're only adding one user here which isn't helping
> make the case that this is something generic.
>
> > Please also keep in mind that we're talking about an FPGA device here, a
> > device that is capable of implementing other devices that fall under
> > various drivers/xx/. Obviously each would have a driver of their own so
> > there is no as strong only single device/driver mapping here as you might
> > be thinking.
>
> I can't tell what you're trying to say here. Are you saying that
> this is somehow baked into some FPGA design so that it's memory
> mapped with only a few registers showing to the rest of the
> system rather than just having a substantial memory mapped
> window like is typically used for FPGAs, but someohow this
> register window stuff is implemented in the soft IP so people are
> just throwng vaugely similar interfaces into a random host mapped
> register layout?
What I tried to say the users are not expected to be nicely confined into
drivers/mfd/ (and a single driver in there).
You didn't answer at all my question about where to place the code?
I'm repeating it with the context below since you cut it off:
That's probably because it sounds to me you're trying to extend its
genericness beyond the domain where it's generic. That is, you're looking
for genericness outside of IPs (that have their own driver each) in Intel
FPGA domain.
Whether that is "generic" enough to reside in drivers/base/regmap can
of course be debated but lets say I put it into drivers/mfd/ along with
the code currently using it. By doing that, we'll postpone this discussion
to the point when the first driver using it outside of drivers/mfd/ comes
by. At that point, having the indirect code in drivers/mfd/ is shown to
be a wrong choice.
It's of course nothing that couldn't be fixed by patches moving the code
around to some more preferred location. And that location likely turns out
to be drivers/base/regmap, no? Or do you have a better place for it in
that case?
--
i.
next prev parent reply other threads:[~2022-11-21 13:40 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-11-17 12:05 [PATCH v2 00/11] intel-m10-bmc: Split BMC to core and SPI parts & add PMCI+N6000 support Ilpo Järvinen
2022-11-17 12:05 ` [PATCH v2 01/11] mfd: intel-m10-bmc: Create m10bmc_platform_info for type specific info Ilpo Järvinen
2022-11-17 12:05 ` [PATCH v2 02/11] mfd: intel-m10-bmc: Rename the local variables Ilpo Järvinen
2022-11-17 12:05 ` [PATCH v2 03/11] mfd: intel-m10-bmc: Split into core and spi specific parts Ilpo Järvinen
2022-11-17 12:05 ` [PATCH v2 04/11] mfd: intel-m10-bmc: Support multiple CSR register layouts Ilpo Järvinen
2022-11-17 12:05 ` [PATCH v2 05/11] fpga: intel-m10-bmc: Add flash ops for sec update Ilpo Järvinen
2022-11-17 12:05 ` [PATCH v2 06/11] mfd: intel-m10-bmc: Downscope SPI defines & prefix with M10BMC_SPI Ilpo Järvinen
2022-11-17 12:05 ` [PATCH v2 07/11] regmap: indirect: Add indirect regmap support Ilpo Järvinen
2022-11-17 13:35 ` Mark Brown
2022-11-17 14:35 ` Ilpo Järvinen
2022-11-17 15:29 ` Mark Brown
2022-11-18 12:49 ` Ilpo Järvinen
2022-11-18 13:55 ` Mark Brown
2022-11-21 13:37 ` Ilpo Järvinen [this message]
2022-11-25 18:53 ` Mark Brown
2022-11-17 12:05 ` [PATCH v2 08/11] intel-m10-bmc: Add regmap_indirect_cfg for Intel FPGA IPs Ilpo Järvinen
2022-11-17 12:05 ` [PATCH v2 09/11] mfd: intel-m10-bmc: Add PMCI driver Ilpo Järvinen
2022-11-17 12:05 ` [PATCH v2 10/11] fpga: m10bmc-sec: Add support for N6000 Ilpo Järvinen
2022-11-17 12:05 ` [PATCH v2 11/11] mfd: intel-m10-bmc: Change MODULE_LICENSE() to GPL Ilpo Järvinen
2022-12-04 9:45 ` Greg KH
2022-11-22 2:43 ` [PATCH v2 00/11] intel-m10-bmc: Split BMC to core and SPI parts & add PMCI+N6000 support Xu Yilun
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=a97ce076-4ca6-5f1b-eba4-4068dcb64b3d@linux.intel.com \
--to=ilpo.jarvinen@linux.intel.com \
--cc=broonie@kernel.org \
--cc=gregkh@linuxfoundation.org \
--cc=hao.wu@intel.com \
--cc=lee@kernel.org \
--cc=linux-fpga@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=marpagan@redhat.com \
--cc=matthew.gerlach@linux.intel.com \
--cc=mdf@kernel.org \
--cc=rafael@kernel.org \
--cc=russell.h.weight@intel.com \
--cc=tianfei.zhang@intel.com \
--cc=trix@redhat.com \
--cc=yilun.xu@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).