Linux-Watchdog Archive on lore.kernel.org
 help / color / Atom feed
From: Lee Jones <lee.jones@linaro.org>
To: Rob Herring <robh@kernel.org>
Cc: "Michael Walle" <michael@walle.cc>,
	"Mark Brown" <broonie@kernel.org>,
	linux-gpio@vger.kernel.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-hwmon@vger.kernel.org,
	linux-pwm@vger.kernel.org, linux-watchdog@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	"Linus Walleij" <linus.walleij@linaro.org>,
	"Bartosz Golaszewski" <bgolaszewski@baylibre.com>,
	"Jean Delvare" <jdelvare@suse.com>,
	"Guenter Roeck" <linux@roeck-us.net>,
	"Thierry Reding" <thierry.reding@gmail.com>,
	"Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>,
	"Wim Van Sebroeck" <wim@linux-watchdog.org>,
	"Shawn Guo" <shawnguo@kernel.org>, "Li Yang" <leoyang.li@nxp.com>,
	"Thomas Gleixner" <tglx@linutronix.de>,
	"Jason Cooper" <jason@lakedaemon.net>,
	"Marc Zyngier" <maz@kernel.org>,
	"Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
	"Andy Shevchenko" <andriy.shevchenko@linux.intel.com>
Subject: Re: [PATCH v4 02/11] mfd: Add support for Kontron sl28cpld management controller
Date: Tue, 9 Jun 2020 19:52:31 +0100
Message-ID: <20200609185231.GO4106@dell> (raw)
In-Reply-To: <20200609165401.GB1019634@bogus>

On Tue, 09 Jun 2020, Rob Herring wrote:

> On Mon, Jun 08, 2020 at 09:28:27AM +0100, Lee Jones wrote:
> > Rob, something for you below.
> > 
> > On Sat, 06 Jun 2020, Michael Walle wrote:
> > > Am 2020-06-06 13:46, schrieb Mark Brown:
> > > > On Fri, Jun 05, 2020 at 10:07:36PM +0200, Michael Walle wrote:
> > > > > Am 2020-06-05 12:50, schrieb Mark Brown:
> > > > 
> > > > > > I have no idea what you are thinking of when you say "simple-regmap" so
> > > > > > it is difficult to comment.
> > > > 
> > > > > I guess, Lee is suggesting to be able to create a regmap instance via
> > > > > device tree (and populate its child nodes?). Like
> > > > >   compatible = "syscon", "simple-mfd";
> > > > > but for any regmap, not just MMIO.
> > 
> > Bingo!
> > 
> > > > I don't understand why this would be anything separate to
> > > > simple-mfd.
> > > 
> > > Don't just simple-mfd tells the of core, to probe the children this
> > > node? Where does the regmap then come from?
> > 
> > Right.  I'm suggesting a means to extrapolate complex shared and
> > sometimes intertwined batches of register sets to be consumed by
> > multiple (sub-)devices spanning different subsystems.
> > 
> > Actually scrap that.  The most common case I see is a single Regmap
> > covering all child-devices.  It would be great if there was a way in
> > which we could make an assumption that the entire register address
> > space for a 'tagged' (MFD) device is to be shared (via Regmap) between
> > each of the devices described by its child-nodes.  Probably by picking
> > up on the 'simple-mfd' compatible string in the first instance.
> > 
> > Rob, is the above something you would contemplate?
> 
> No. I'd like to just kill off syscon and simple-mfd really. Those are 
> just hints meaning a specific compatible is still needed, but I see them 
> all the time alone (or combined like above). 'syscon' just serves to 
> create a regmap. This could be accomplished just with a list of 
> compatibles to register a regmap for. That might be a longish list, but 
> wanting a regmap is really a kernel implementation detail and decision.

Exactly.  Syscon is a real tangible thing and Regmap is a Linux
subsystem.  So swapping out the former for the latter sounds like the
opposite of what you'd want to do.

> > > MFD core can
> > > match a device tree node today; but only one per unique compatible
> > > string. So what should I use to differentiate the different
> > > subdevices?
> > 
> > Right.  I have been aware of this issue.  The only suitable solution
> > to this would be to match on 'reg'.
> > 
> > FYI: I plan to fix this.
> > 
> > If your register map needs to change, then I suggest that this is
> > either a new device or at least a different version of the device and
> > would also have to be represented as different (sub-)mfd_cell.
> 
> The same register set at a different offset is the same (sub)device.

See below.

> > > Rob suggested the internal offset, which I did here.
> > 
> > FWIW, I don't like this idea.  DTs should not have to be modified
> > (either in the first instance or subsequently) or specifically
> > designed to patch inadequacies in any given OS.
> 
> My understanding is there can be differing combinations or number of 
> instances of sub devices for this device. That's when having DT sub 
> devices makes sense. If the h/w changes, then the DT should change.

This is the same point I was making above.

> Multiple instances of devices require an address to identify them and we 
> don't make up numbering if we can avoid it. The earlier revisions just 
> had made up indices for addresses.

Right.  Which I'm against.

Placing "internal offsets" into the 'reg' property is a hack.

The issue is, if we need to configure the devices differently, then
how do we identify them in order to ensure the correct OF node pointer
is allocated to the correct platform device?

-- 
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog

  reply index

Thread overview: 74+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-04 21:10 [PATCH v4 00/11] Add support for Kontron sl28cpld Michael Walle
2020-06-04 21:10 ` [PATCH v4 01/11] dt-bindings: mfd: Add bindings for sl28cpld Michael Walle
2020-06-09 16:28   ` Rob Herring
2020-06-04 21:10 ` [PATCH v4 02/11] mfd: Add support for Kontron sl28cpld management controller Michael Walle
2020-06-05  6:57   ` Lee Jones
2020-06-05  9:51     ` Michael Walle
2020-06-05 10:50     ` Mark Brown
2020-06-05 20:07       ` Michael Walle
2020-06-06 11:46         ` Mark Brown
2020-06-06 12:45           ` Michael Walle
2020-06-08  8:28             ` Lee Jones
2020-06-08 10:02               ` Andy Shevchenko
2020-06-08 15:41                 ` Michael Walle
2020-06-08 18:56                   ` Lee Jones
2020-06-08 21:09                     ` Michael Walle
2020-06-09  6:47                       ` Lee Jones
2020-06-09 14:38                         ` Michael Walle
2020-06-09 14:42                           ` Mark Brown
2020-06-09 15:01                             ` Michael Walle
2020-06-09 17:15                               ` Rob Herring
2020-06-09 17:29                                 ` Mark Brown
2020-06-09 18:41                                 ` Lee Jones
2020-06-09 15:19                           ` Lee Jones
2020-06-09 15:30                             ` Michael Walle
2020-06-09 19:45                               ` Lee Jones
2020-06-10  7:10                                 ` Michael Walle
2020-06-10  7:19                                   ` Lee Jones
2020-06-10  7:49                                     ` Michael Walle
2020-06-10  7:56                                       ` Lee Jones
2020-06-10  9:27                                         ` Michael Walle
2020-06-10 18:30                                           ` Lee Jones
2020-06-10 17:16                                     ` Rob Herring
2020-06-10 18:02                                       ` Lee Jones
2020-06-08 18:20                 ` Lee Jones
2020-06-09 16:54               ` Rob Herring
2020-06-09 18:52                 ` Lee Jones [this message]
2020-06-05  8:01   ` Andy Shevchenko
2020-06-05  8:02     ` Andy Shevchenko
2020-06-05 10:09     ` Michael Walle
2020-06-05 10:48       ` Andy Shevchenko
2020-06-05 11:51         ` Michael Walle
2020-06-04 21:10 ` [PATCH v4 03/11] irqchip: add sl28cpld interrupt controller support Michael Walle
2020-06-05  1:26   ` kernel test robot
2020-06-05  8:07   ` Andy Shevchenko
2020-06-08 15:12   ` kernel test robot
2020-06-04 21:10 ` [PATCH v4 04/11] watchdog: add support for sl28cpld watchdog Michael Walle
2020-06-05  8:14   ` Andy Shevchenko
2020-06-05 10:24     ` Michael Walle
2020-06-05 10:50       ` Andy Shevchenko
2020-06-05 13:52         ` Guenter Roeck
2020-06-05 14:09           ` Andy Shevchenko
2020-06-05 15:05             ` Guenter Roeck
2020-06-05 16:04               ` Andy Shevchenko
2020-06-05 16:34                 ` Guenter Roeck
2020-06-04 21:10 ` [PATCH v4 05/11] pwm: add support for sl28cpld PWM controller Michael Walle
2020-06-05  8:15   ` Andy Shevchenko
2020-06-05  8:49   ` Lee Jones
2020-06-05  9:33     ` Andy Shevchenko
2020-06-05 11:39     ` Michael Walle
2020-06-05 18:17     ` Michael Walle
2020-06-08  7:46       ` Lee Jones
2020-06-04 21:10 ` [PATCH v4 06/11] gpio: add support for the sl28cpld GPIO controller Michael Walle
2020-06-05 12:00   ` Andy Shevchenko
2020-06-05 12:42     ` Michael Walle
2020-06-05 13:15       ` Andy Shevchenko
2020-06-05 18:44         ` Michael Walle
2020-06-05 21:19           ` Andy Shevchenko
2020-06-04 21:10 ` [PATCH v4 07/11] hwmon: add support for the sl28cpld hardware monitoring controller Michael Walle
2020-06-05 12:06   ` Andy Shevchenko
2020-06-04 21:10 ` [PATCH v4 08/11] arm64: dts: freescale: sl28: enable sl28cpld Michael Walle
2020-06-04 21:10 ` [PATCH v4 09/11] arm64: dts: freescale: sl28: map GPIOs to input events Michael Walle
2020-06-04 21:10 ` [PATCH v4 10/11] arm64: dts: freescale: sl28: enable LED support Michael Walle
2020-06-04 21:10 ` [PATCH v4 11/11] arm64: dts: freescale: sl28: enable fan support Michael Walle
2020-06-05  6:13 ` [PATCH v4 00/11] Add support for Kontron sl28cpld Lee Jones

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=20200609185231.GO4106@dell \
    --to=lee.jones@linaro.org \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=bgolaszewski@baylibre.com \
    --cc=broonie@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=jason@lakedaemon.net \
    --cc=jdelvare@suse.com \
    --cc=leoyang.li@nxp.com \
    --cc=linus.walleij@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-gpio@vger.kernel.org \
    --cc=linux-hwmon@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pwm@vger.kernel.org \
    --cc=linux-watchdog@vger.kernel.org \
    --cc=linux@roeck-us.net \
    --cc=maz@kernel.org \
    --cc=michael@walle.cc \
    --cc=robh@kernel.org \
    --cc=shawnguo@kernel.org \
    --cc=tglx@linutronix.de \
    --cc=thierry.reding@gmail.com \
    --cc=u.kleine-koenig@pengutronix.de \
    --cc=wim@linux-watchdog.org \
    /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

Linux-Watchdog Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-watchdog/0 linux-watchdog/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-watchdog linux-watchdog/ https://lore.kernel.org/linux-watchdog \
		linux-watchdog@vger.kernel.org
	public-inbox-index linux-watchdog

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-watchdog


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git