All of lore.kernel.org
 help / color / mirror / Atom feed
From: arnd@arndb.de (Arnd Bergmann)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 2/2] ARM: dts: meson: Adding hwrev syscon node
Date: Fri, 26 Feb 2016 17:00:57 +0100	[thread overview]
Message-ID: <2692251.6GoexVJEYt@wuerfel> (raw)
In-Reply-To: <CAOQ7t2Za5t_3nAm0wyf0rmFa+RTRSGUDAg5b64VvXp-okLMoNQ@mail.gmail.com>

On Friday 26 February 2016 16:34:59 Carlo Caione wrote:
> On Fri, Feb 19, 2016 at 1:54 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> 
> <cut>
> 
> > This still really sounds like a mixed bag to me, which should better get represented
> > as a syscon node, except that there are also some more structured areas in
> > CBUS.
> >
> > Having just the registers between METAL_REVISION and HW_REV in a syscon
> > is clearly wrong, as that would include the pinctrl area that already has
> > a driver, but would not include some other parts that want the syscon.
> >
> > Maybe the best way is to make it compatible with both syscon and
> > simple-bus and put the other nodes underneath. That is still rather
> > ugly, but at least it works and can be extended.
> 
> More on this topic.
> 
> On the meson platforms (at least on the meson8 / meson8b) we have two
> buses: cbus and aobus. Since in cbus we have a bunch of scattered
> registers, Arnd suggested to make it compatible with both syscon and
> simple-bus. So my idea was actually to update the meson8b DTSI file
> adding the two buses to make it closer to the actual hardware.
> 
> In the most simple cases moving the devices under the correct bus is a
> trivial operation since (of course) the same driver can be used when
> the device is attached to a different bus, like in the uart_AO case
> (http://lxr.free-electrons.com/source/arch/arm/boot/dts/meson8b.dtsi#L114).
> 
> Unfortunately pinctrl is a different beast since it requires
> (http://lxr.free-electrons.com/source/drivers/pinctrl/meson/pinctrl-meson.c#L659)
> at least two subnodes: one accessing registers from aobus, and the
> other one from cbus.
> 
> I know this is quite a peculiar case but I'm wondering what is the
> best way to approach this issue.
> 
> 1) We could move only the pinctrl device outside both aobus and cbus
> but IMO this is ugly (at this point is probably better not having the
> two buses at all described in the DTS).
> 2) The second option is just to fix the driver so that the two
> subnodes are not strictly required. The problem with this second
> solution is that in the driver we still need to access some data
> (http://lxr.free-electrons.com/source/drivers/pinctrl/meson/pinctrl-meson8b.c#L873)
> that is specific to each bus. So we will end up having two different
> compatibles for the two buses (meson8b-pinctrl-aobus using data from
> 'ao-bank', and meson8b-pinctrl-cbus using data from 'banks').
> 3) Another option is just to have the driver with a unique compatible
> poking the parent node (or just to another property) to determine on
> which bus it is so that it can use the correct bus-specific data.
> 
> Any idea / feedback?

Would it be possible to split the pin controller driver into two drivers
that each only access registers on one of the buses? Is that a split
that makes sense from the point of view of that driver?

	Arnd

WARNING: multiple messages have this Message-ID (diff)
From: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
To: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
Cc: Carlo Caione <carlo-KA+7E9HrN00dnm+yROfE0A@public.gmane.org>,
	devicetree <devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	linux-meson-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org,
	Daniel Drake <drake-6IF/jdPJHihWk0Htik3J/w@public.gmane.org>,
	Carlo Caione <carlo-6IF/jdPJHihWk0Htik3J/w@public.gmane.org>,
	Romain Perier
	<romain.perier-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Subject: Re: [PATCH 2/2] ARM: dts: meson: Adding hwrev syscon node
Date: Fri, 26 Feb 2016 17:00:57 +0100	[thread overview]
Message-ID: <2692251.6GoexVJEYt@wuerfel> (raw)
In-Reply-To: <CAOQ7t2Za5t_3nAm0wyf0rmFa+RTRSGUDAg5b64VvXp-okLMoNQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

On Friday 26 February 2016 16:34:59 Carlo Caione wrote:
> On Fri, Feb 19, 2016 at 1:54 PM, Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org> wrote:
> 
> <cut>
> 
> > This still really sounds like a mixed bag to me, which should better get represented
> > as a syscon node, except that there are also some more structured areas in
> > CBUS.
> >
> > Having just the registers between METAL_REVISION and HW_REV in a syscon
> > is clearly wrong, as that would include the pinctrl area that already has
> > a driver, but would not include some other parts that want the syscon.
> >
> > Maybe the best way is to make it compatible with both syscon and
> > simple-bus and put the other nodes underneath. That is still rather
> > ugly, but at least it works and can be extended.
> 
> More on this topic.
> 
> On the meson platforms (at least on the meson8 / meson8b) we have two
> buses: cbus and aobus. Since in cbus we have a bunch of scattered
> registers, Arnd suggested to make it compatible with both syscon and
> simple-bus. So my idea was actually to update the meson8b DTSI file
> adding the two buses to make it closer to the actual hardware.
> 
> In the most simple cases moving the devices under the correct bus is a
> trivial operation since (of course) the same driver can be used when
> the device is attached to a different bus, like in the uart_AO case
> (http://lxr.free-electrons.com/source/arch/arm/boot/dts/meson8b.dtsi#L114).
> 
> Unfortunately pinctrl is a different beast since it requires
> (http://lxr.free-electrons.com/source/drivers/pinctrl/meson/pinctrl-meson.c#L659)
> at least two subnodes: one accessing registers from aobus, and the
> other one from cbus.
> 
> I know this is quite a peculiar case but I'm wondering what is the
> best way to approach this issue.
> 
> 1) We could move only the pinctrl device outside both aobus and cbus
> but IMO this is ugly (at this point is probably better not having the
> two buses at all described in the DTS).
> 2) The second option is just to fix the driver so that the two
> subnodes are not strictly required. The problem with this second
> solution is that in the driver we still need to access some data
> (http://lxr.free-electrons.com/source/drivers/pinctrl/meson/pinctrl-meson8b.c#L873)
> that is specific to each bus. So we will end up having two different
> compatibles for the two buses (meson8b-pinctrl-aobus using data from
> 'ao-bank', and meson8b-pinctrl-cbus using data from 'banks').
> 3) Another option is just to have the driver with a unique compatible
> poking the parent node (or just to another property) to determine on
> which bus it is so that it can use the correct bus-specific data.
> 
> Any idea / feedback?

Would it be possible to split the pin controller driver into two drivers
that each only access registers on one of the buses? Is that a split
that makes sense from the point of view of that driver?

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

  reply	other threads:[~2016-02-26 16:00 UTC|newest]

Thread overview: 48+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-02-17 17:28 [PATCH 0/2] Adding support to show SoC revision in /proc/cpuinfo Romain Perier
2016-02-17 17:28 ` Romain Perier
2016-02-17 17:28 ` [PATCH 1/2] ARM: meson: Adding support to retrieve serial and SoC revision Romain Perier
2016-02-17 17:28   ` Romain Perier
2016-02-17 20:34   ` Carlo Caione
2016-02-17 20:34     ` Carlo Caione
2016-02-18 12:20     ` Romain Perier
2016-02-18 12:20       ` Romain Perier
2016-02-18 12:24       ` Carlo Caione
2016-02-18 12:24         ` Carlo Caione
2016-02-17 17:28 ` [PATCH 2/2] ARM: dts: meson: Adding hwrev syscon node Romain Perier
2016-02-17 17:28   ` Romain Perier
2016-02-17 20:36   ` Carlo Caione
2016-02-17 20:36     ` Carlo Caione
2016-02-18 12:33     ` Romain Perier
2016-02-18 12:33       ` Romain Perier
2016-02-18 12:43       ` Arnd Bergmann
2016-02-18 12:43         ` Arnd Bergmann
2016-02-18 14:14         ` Carlo Caione
2016-02-18 14:14           ` Carlo Caione
2016-02-18 14:22           ` Arnd Bergmann
2016-02-18 14:22             ` Arnd Bergmann
2016-02-18 21:04             ` Carlo Caione
2016-02-18 21:04               ` Carlo Caione
2016-02-18 21:24               ` Carlo Caione
2016-02-18 21:24                 ` Carlo Caione
2016-02-18 21:27               ` Daniel Drake
2016-02-18 21:27                 ` Daniel Drake
2016-02-19 11:53                 ` Arnd Bergmann
2016-02-19 11:53                   ` Arnd Bergmann
2016-02-19 12:54                 ` Arnd Bergmann
2016-02-19 12:54                   ` Arnd Bergmann
2016-02-19 13:25                   ` Carlo Caione
2016-02-19 13:25                     ` Carlo Caione
2016-02-24 20:42                     ` Carlo Caione
2016-02-24 20:42                       ` Carlo Caione
2016-02-26 15:34                   ` Carlo Caione
2016-02-26 15:34                     ` Carlo Caione
2016-02-26 16:00                     ` Arnd Bergmann [this message]
2016-02-26 16:00                       ` Arnd Bergmann
2016-02-26 16:43                       ` Carlo Caione
2016-02-26 16:43                         ` Carlo Caione
2016-02-26 17:00                         ` Arnd Bergmann
2016-02-26 17:00                           ` Arnd Bergmann
2016-02-26 17:40                           ` Carlo Caione
2016-02-26 17:40                             ` Carlo Caione
2016-02-17 20:50   ` Arnd Bergmann
2016-02-17 20:50     ` Arnd Bergmann

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=2692251.6GoexVJEYt@wuerfel \
    --to=arnd@arndb.de \
    --cc=linux-arm-kernel@lists.infradead.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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.