diff mbox

[1/5] MFD/OF: document MFD devices and handle simple-mfd

Message ID 1425375148-4369-2-git-send-email-linus.walleij@linaro.org (mailing list archive)
State New, archived
Headers show

Commit Message

Linus Walleij March 3, 2015, 9:32 a.m. UTC
This defines a new compatible option for MFD devices "simple-mfd" that will
make the OF core spawn child devices for all subnodes of that MFD device.
It is optional but handy for things like syscon and possibly other
simpler MFD devices.

Since there was no file to put the documentation in, I took this opportunity
to make a small writeup on MFD devices and add the compatible definition
there.

Suggested-by: Lee Jones <lee.jones@linaro.org>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Devicetree <devicetree@vger.kernel.org>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Grant Likely <grant.likely@linaro.org>
Cc: Pawel Moll <pawel.moll@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
Cc: Kumar Gala <galak@codeaurora.org>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
I make the patch to the OF core in this one, it makes much more sense since
it's a oneliner

Lee: this is a (tested!) implementation of your suggestion for simple-mfd.
If you can eventually ACK this from the MFD side, I think it should be
funneled through the ARM SoC tree.

Grant/Rob: if either of you can ACK the change to the OF core likewise it can
be taken through ARM SoC.

DT binings maintainers: there is some background discussion on this here:
http://marc.info/?l=linux-arm-kernel&m=142486676603889&w=2
http://marc.info/?l=devicetree&m=142166313621469&w=2
---
 Documentation/devicetree/bindings/mfd/mfd.txt | 40 +++++++++++++++++++++++++++
 drivers/of/platform.c                         |  1 +
 2 files changed, 41 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mfd/mfd.txt

Comments

Lee Jones March 3, 2015, 10:25 a.m. UTC | #1
On Tue, 03 Mar 2015, Linus Walleij wrote:

> This defines a new compatible option for MFD devices "simple-mfd" that will
> make the OF core spawn child devices for all subnodes of that MFD device.
> It is optional but handy for things like syscon and possibly other
> simpler MFD devices.
> 
> Since there was no file to put the documentation in, I took this opportunity
> to make a small writeup on MFD devices and add the compatible definition
> there.
> 
> Suggested-by: Lee Jones <lee.jones@linaro.org>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Devicetree <devicetree@vger.kernel.org>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Grant Likely <grant.likely@linaro.org>
> Cc: Pawel Moll <pawel.moll@arm.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
> Cc: Kumar Gala <galak@codeaurora.org>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> I make the patch to the OF core in this one, it makes much more sense since
> it's a oneliner
> 
> Lee: this is a (tested!) implementation of your suggestion for simple-mfd.
> If you can eventually ACK this from the MFD side, I think it should be
> funneled through the ARM SoC tree.
> 
> Grant/Rob: if either of you can ACK the change to the OF core likewise it can
> be taken through ARM SoC.
> 
> DT binings maintainers: there is some background discussion on this here:
> http://marc.info/?l=linux-arm-kernel&m=142486676603889&w=2
> http://marc.info/?l=devicetree&m=142166313621469&w=2
> ---
>  Documentation/devicetree/bindings/mfd/mfd.txt | 40 +++++++++++++++++++++++++++
>  drivers/of/platform.c                         |  1 +
>  2 files changed, 41 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mfd/mfd.txt
> 
> diff --git a/Documentation/devicetree/bindings/mfd/mfd.txt b/Documentation/devicetree/bindings/mfd/mfd.txt
> new file mode 100644
> index 000000000000..cc057438abe8
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/mfd.txt
> @@ -0,0 +1,40 @@
> +Multi-Function Devices (MFD)
> +
> +These devices comprise a nexus for heterogeneous hardware blocks 

I don't think you meant 'comprise' here.  Perhaps 'represent' might
fit better?

> +spawning multiple child devices.

Spawning devices is something software will do.  Think less about what
the actions the OS will take order to handle these IPs and more about
what they are physically.  I would prefer the description mentions
that these chips are single pieces of silicon which contain >1 devices
with non-unique/varying functionality.

> +A typical MFD can be:
> +
> +- A mixed signal ASIC on an external bus, sometimes a PMIC (power management
> +  integrated circuit) that is manufactured in a lower technology node (rough

Please capitalise the expanded PMIC abbreviation.

> +  silicon) that handles analog drivers for things like audio amplifiers, LED
> +  drivers, level shifters, PHY (physical interfaces to things like USB or
> +  ethernet), regulators etc.
> +
> +- A range of memory registers containing "miscellaneous system registers" also
> +  known as a system controller "syscon" or any other memory range containing a
> +  mix of unrelated registers.

s/registers/{hardware|devices}/

> +Optional properties:
> +
> +- compatible : "simple-mfd" - this signifies that the operating system should
> +  spawn child devices for all the subnodes of the MFD device akin to how

This is pretty Linux specific IMHO.  Other OSes might not spawn
seperate devices to handle MFD type h/w.  The idea is fine, but please
generify a little.

> +  "simple-bus" inidicates when to spawn children for a simple memory-mapped
> +  bus. For more complex devices, when the nexus driver has to probe registers
> +  to figure out what children exist etc, this should not be used. In the latter
> +  case the child devices will be instantiated by the operating system.
> +
> +Example:
> +
> +foo@1000 {

Should be 10000.
> +		compatible = "syscon", "simple-mfd";

Please address your tabbing issues.

> +		reg = <0x010000 0x1000>;
> +
> +		led@08.0 {
> +			compatible = "register-bit-led";
> +			offset = <0x08>;
> +			mask = <0x01>;
> +			label = "myled";
> +			default-state = "on";
> +		};
> +};
> diff --git a/drivers/of/platform.c b/drivers/of/platform.c
> index b189733a1539..969ba4e1460c 100644
> --- a/drivers/of/platform.c
> +++ b/drivers/of/platform.c
> @@ -26,6 +26,7 @@
>  
>  const struct of_device_id of_default_bus_match_table[] = {
>  	{ .compatible = "simple-bus", },
> +	{ .compatible = "simple-mfd", },
>  #ifdef CONFIG_ARM_AMBA
>  	{ .compatible = "arm,amba-bus", },
>  #endif /* CONFIG_ARM_AMBA */
Rob Herring March 3, 2015, 5:47 p.m. UTC | #2
On Tue, Mar 3, 2015 at 3:32 AM, Linus Walleij <linus.walleij@linaro.org> wrote:
> This defines a new compatible option for MFD devices "simple-mfd" that will
> make the OF core spawn child devices for all subnodes of that MFD device.
> It is optional but handy for things like syscon and possibly other
> simpler MFD devices.
>
> Since there was no file to put the documentation in, I took this opportunity
> to make a small writeup on MFD devices and add the compatible definition
> there.
>
> Suggested-by: Lee Jones <lee.jones@linaro.org>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Devicetree <devicetree@vger.kernel.org>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Grant Likely <grant.likely@linaro.org>
> Cc: Pawel Moll <pawel.moll@arm.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
> Cc: Kumar Gala <galak@codeaurora.org>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> I make the patch to the OF core in this one, it makes much more sense since
> it's a oneliner
>
> Lee: this is a (tested!) implementation of your suggestion for simple-mfd.
> If you can eventually ACK this from the MFD side, I think it should be
> funneled through the ARM SoC tree.
>
> Grant/Rob: if either of you can ACK the change to the OF core likewise it can
> be taken through ARM SoC.
>
> DT binings maintainers: there is some background discussion on this here:
> http://marc.info/?l=linux-arm-kernel&m=142486676603889&w=2
> http://marc.info/?l=devicetree&m=142166313621469&w=2
> ---
>  Documentation/devicetree/bindings/mfd/mfd.txt | 40 +++++++++++++++++++++++++++
>  drivers/of/platform.c                         |  1 +
>  2 files changed, 41 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mfd/mfd.txt
>
> diff --git a/Documentation/devicetree/bindings/mfd/mfd.txt b/Documentation/devicetree/bindings/mfd/mfd.txt
> new file mode 100644
> index 000000000000..cc057438abe8
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/mfd.txt
> @@ -0,0 +1,40 @@
> +Multi-Function Devices (MFD)
> +
> +These devices comprise a nexus for heterogeneous hardware blocks spawning
> +multiple child devices.
> +
> +A typical MFD can be:
> +
> +- A mixed signal ASIC on an external bus, sometimes a PMIC (power management
> +  integrated circuit) that is manufactured in a lower technology node (rough
> +  silicon) that handles analog drivers for things like audio amplifiers, LED
> +  drivers, level shifters, PHY (physical interfaces to things like USB or
> +  ethernet), regulators etc.
> +
> +- A range of memory registers containing "miscellaneous system registers" also
> +  known as a system controller "syscon" or any other memory range containing a
> +  mix of unrelated registers.
> +
> +Optional properties:
> +
> +- compatible : "simple-mfd" - this signifies that the operating system should
> +  spawn child devices for all the subnodes of the MFD device akin to how
> +  "simple-bus" inidicates when to spawn children for a simple memory-mapped
> +  bus. For more complex devices, when the nexus driver has to probe registers
> +  to figure out what children exist etc, this should not be used. In the latter
> +  case the child devices will be instantiated by the operating system.

If you point of_platform_populate to the parent node, it should
instantiate all children regardless of "simple-bus" or any other
match. The match is to probe the grandchildren. I could be mistaken,
but that's how it works at the root level. You don't have a driver
probe for the parent, so you want the top level of_platform_populate
call to instantiate these devices?

Why does simple-bus not work for you? After all, it is not
"simple-memory-map-bus."

Rob
Lee Jones March 3, 2015, 5:56 p.m. UTC | #3
On Tue, 03 Mar 2015, Rob Herring wrote:

> On Tue, Mar 3, 2015 at 3:32 AM, Linus Walleij <linus.walleij@linaro.org> wrote:
> > This defines a new compatible option for MFD devices "simple-mfd" that will
> > make the OF core spawn child devices for all subnodes of that MFD device.
> > It is optional but handy for things like syscon and possibly other
> > simpler MFD devices.
> >
> > Since there was no file to put the documentation in, I took this opportunity
> > to make a small writeup on MFD devices and add the compatible definition
> > there.
> >
> > Suggested-by: Lee Jones <lee.jones@linaro.org>
> > Cc: Arnd Bergmann <arnd@arndb.de>
> > Cc: Devicetree <devicetree@vger.kernel.org>
> > Cc: Rob Herring <robh+dt@kernel.org>
> > Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> > Cc: Grant Likely <grant.likely@linaro.org>
> > Cc: Pawel Moll <pawel.moll@arm.com>
> > Cc: Mark Rutland <mark.rutland@arm.com>
> > Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
> > Cc: Kumar Gala <galak@codeaurora.org>
> > Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> > ---
> > I make the patch to the OF core in this one, it makes much more sense since
> > it's a oneliner
> >
> > Lee: this is a (tested!) implementation of your suggestion for simple-mfd.
> > If you can eventually ACK this from the MFD side, I think it should be
> > funneled through the ARM SoC tree.
> >
> > Grant/Rob: if either of you can ACK the change to the OF core likewise it can
> > be taken through ARM SoC.
> >
> > DT binings maintainers: there is some background discussion on this here:
> > http://marc.info/?l=linux-arm-kernel&m=142486676603889&w=2
> > http://marc.info/?l=devicetree&m=142166313621469&w=2
> > ---
> >  Documentation/devicetree/bindings/mfd/mfd.txt | 40 +++++++++++++++++++++++++++
> >  drivers/of/platform.c                         |  1 +
> >  2 files changed, 41 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/mfd/mfd.txt
> >
> > diff --git a/Documentation/devicetree/bindings/mfd/mfd.txt b/Documentation/devicetree/bindings/mfd/mfd.txt
> > new file mode 100644
> > index 000000000000..cc057438abe8
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/mfd/mfd.txt
> > @@ -0,0 +1,40 @@
> > +Multi-Function Devices (MFD)
> > +
> > +These devices comprise a nexus for heterogeneous hardware blocks spawning
> > +multiple child devices.
> > +
> > +A typical MFD can be:
> > +
> > +- A mixed signal ASIC on an external bus, sometimes a PMIC (power management
> > +  integrated circuit) that is manufactured in a lower technology node (rough
> > +  silicon) that handles analog drivers for things like audio amplifiers, LED
> > +  drivers, level shifters, PHY (physical interfaces to things like USB or
> > +  ethernet), regulators etc.
> > +
> > +- A range of memory registers containing "miscellaneous system registers" also
> > +  known as a system controller "syscon" or any other memory range containing a
> > +  mix of unrelated registers.
> > +
> > +Optional properties:
> > +
> > +- compatible : "simple-mfd" - this signifies that the operating system should
> > +  spawn child devices for all the subnodes of the MFD device akin to how
> > +  "simple-bus" inidicates when to spawn children for a simple memory-mapped
> > +  bus. For more complex devices, when the nexus driver has to probe registers
> > +  to figure out what children exist etc, this should not be used. In the latter
> > +  case the child devices will be instantiated by the operating system.
> 
> If you point of_platform_populate to the parent node, it should
> instantiate all children regardless of "simple-bus" or any other
> match. The match is to probe the grandchildren. I could be mistaken,
> but that's how it works at the root level. You don't have a driver
> probe for the parent, so you want the top level of_platform_populate
> call to instantiate these devices?
> 
> Why does simple-bus not work for you? After all, it is not
> "simple-memory-map-bus."

simple-bus does 'work'.  There are even some (3 I think) platforms
actively making use of it.  However, there was some controversy over
using it, as syscon isn't really a bus.
Linus Walleij March 9, 2015, 2:29 p.m. UTC | #4
On Tue, Mar 3, 2015 at 11:25 AM, Lee Jones <lee.jones@linaro.org> wrote:
> On Tue, 03 Mar 2015, Linus Walleij wrote:

>> +Multi-Function Devices (MFD)
>> +
>> +These devices comprise a nexus for heterogeneous hardware blocks
>
> I don't think you meant 'comprise' here.  Perhaps 'represent' might
> fit better?

I meant "comprise", even looked up the same word in a Swedish-English
dictionary. I mean those devices really are such heterogeneous things,
not just modeled like such in Linux.

>> +spawning multiple child devices.
>
> Spawning devices is something software will do.  Think less about what
> the actions the OS will take order to handle these IPs and more about
> what they are physically.  I would prefer the description mentions
> that these chips are single pieces of silicon which contain >1 devices
> with non-unique/varying functionality.

OK.

>> +A typical MFD can be:
>> +
>> +- A mixed signal ASIC on an external bus, sometimes a PMIC (power management
>> +  integrated circuit) that is manufactured in a lower technology node (rough
>
> Please capitalise the expanded PMIC abbreviation.

OK.

>> +  silicon) that handles analog drivers for things like audio amplifiers, LED
>> +  drivers, level shifters, PHY (physical interfaces to things like USB or
>> +  ethernet), regulators etc.
>> +
>> +- A range of memory registers containing "miscellaneous system registers" also
>> +  known as a system controller "syscon" or any other memory range containing a
>> +  mix of unrelated registers.
>
> s/registers/{hardware|devices}/

OK.

>> +Optional properties:
>> +
>> +- compatible : "simple-mfd" - this signifies that the operating system should
>> +  spawn child devices for all the subnodes of the MFD device akin to how
>
> This is pretty Linux specific IMHO.  Other OSes might not spawn
> seperate devices to handle MFD type h/w.  The idea is fine, but please
> generify a little.

OK.

>> +foo@1000 {
>
> Should be 10000.

OK.

>> +             compatible = "syscon", "simple-mfd";
>
> Please address your tabbing issues.

OK.

Yours,
Linus Walleij
Linus Walleij March 9, 2015, 2:35 p.m. UTC | #5
On Tue, Mar 3, 2015 at 6:47 PM, Rob Herring <robherring2@gmail.com> wrote:
> On Tue, Mar 3, 2015 at 3:32 AM, Linus Walleij <linus.walleij@linaro.org> wrote:

>> +Optional properties:
>> +
>> +- compatible : "simple-mfd" - this signifies that the operating system should
>> +  spawn child devices for all the subnodes of the MFD device akin to how
>> +  "simple-bus" inidicates when to spawn children for a simple memory-mapped
>> +  bus. For more complex devices, when the nexus driver has to probe registers
>> +  to figure out what children exist etc, this should not be used. In the latter
>> +  case the child devices will be instantiated by the operating system.
>
> If you point of_platform_populate to the parent node, it should
> instantiate all children regardless of "simple-bus" or any other
> match. The match is to probe the grandchildren.

Yep, if children are only children of the top node, yes.

> I could be mistaken,
> but that's how it works at the root level.

Yes but only at the root level.

> You don't have a driver
> probe for the parent, so you want the top level of_platform_populate
> call to instantiate these devices?

of_platform_populate() only populates one level unless the node has
something like "simple-bus" or this new "simple-mfd" compatible on
it.

> Why does simple-bus not work for you? After all, it is not
> "simple-memory-map-bus."

As Lee says it works (obviously since I'm just adding a oneliner
doing exactly the same thing as "simple-bus"), but the
terminology is just wrong. It's not a bus in any hardware sense,
as MFD is a logical subdevice split, nothing else.

Yours,
Linus Walleij
Alexandre Belloni March 12, 2015, 9:14 a.m. UTC | #6
Hi,

As part of the system timer cleanup, I'll actually be needing that
patch. Is it scheduled for inclusion or should I use "simple-bus" in the
meantime?

On 03/03/2015 at 10:32:24 +0100, Linus Walleij wrote :
> This defines a new compatible option for MFD devices "simple-mfd" that will
> make the OF core spawn child devices for all subnodes of that MFD device.
> It is optional but handy for things like syscon and possibly other
> simpler MFD devices.
> 
> Since there was no file to put the documentation in, I took this opportunity
> to make a small writeup on MFD devices and add the compatible definition
> there.
> 
> Suggested-by: Lee Jones <lee.jones@linaro.org>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Devicetree <devicetree@vger.kernel.org>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Grant Likely <grant.likely@linaro.org>
> Cc: Pawel Moll <pawel.moll@arm.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
> Cc: Kumar Gala <galak@codeaurora.org>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> I make the patch to the OF core in this one, it makes much more sense since
> it's a oneliner
> 
> Lee: this is a (tested!) implementation of your suggestion for simple-mfd.
> If you can eventually ACK this from the MFD side, I think it should be
> funneled through the ARM SoC tree.
> 
> Grant/Rob: if either of you can ACK the change to the OF core likewise it can
> be taken through ARM SoC.
> 
> DT binings maintainers: there is some background discussion on this here:
> http://marc.info/?l=linux-arm-kernel&m=142486676603889&w=2
> http://marc.info/?l=devicetree&m=142166313621469&w=2
> ---
>  Documentation/devicetree/bindings/mfd/mfd.txt | 40 +++++++++++++++++++++++++++
>  drivers/of/platform.c                         |  1 +
>  2 files changed, 41 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mfd/mfd.txt
> 
> diff --git a/Documentation/devicetree/bindings/mfd/mfd.txt b/Documentation/devicetree/bindings/mfd/mfd.txt
> new file mode 100644
> index 000000000000..cc057438abe8
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/mfd.txt
> @@ -0,0 +1,40 @@
> +Multi-Function Devices (MFD)
> +
> +These devices comprise a nexus for heterogeneous hardware blocks spawning
> +multiple child devices.
> +
> +A typical MFD can be:
> +
> +- A mixed signal ASIC on an external bus, sometimes a PMIC (power management
> +  integrated circuit) that is manufactured in a lower technology node (rough
> +  silicon) that handles analog drivers for things like audio amplifiers, LED
> +  drivers, level shifters, PHY (physical interfaces to things like USB or
> +  ethernet), regulators etc.
> +
> +- A range of memory registers containing "miscellaneous system registers" also
> +  known as a system controller "syscon" or any other memory range containing a
> +  mix of unrelated registers.
> +
> +Optional properties:
> +
> +- compatible : "simple-mfd" - this signifies that the operating system should
> +  spawn child devices for all the subnodes of the MFD device akin to how
> +  "simple-bus" inidicates when to spawn children for a simple memory-mapped
> +  bus. For more complex devices, when the nexus driver has to probe registers
> +  to figure out what children exist etc, this should not be used. In the latter
> +  case the child devices will be instantiated by the operating system.
> +
> +Example:
> +
> +foo@1000 {
> +		compatible = "syscon", "simple-mfd";
> +		reg = <0x010000 0x1000>;
> +
> +		led@08.0 {
> +			compatible = "register-bit-led";
> +			offset = <0x08>;
> +			mask = <0x01>;
> +			label = "myled";
> +			default-state = "on";
> +		};
> +};
> diff --git a/drivers/of/platform.c b/drivers/of/platform.c
> index b189733a1539..969ba4e1460c 100644
> --- a/drivers/of/platform.c
> +++ b/drivers/of/platform.c
> @@ -26,6 +26,7 @@
>  
>  const struct of_device_id of_default_bus_match_table[] = {
>  	{ .compatible = "simple-bus", },
> +	{ .compatible = "simple-mfd", },
>  #ifdef CONFIG_ARM_AMBA
>  	{ .compatible = "arm,amba-bus", },
>  #endif /* CONFIG_ARM_AMBA */
> -- 
> 1.9.3
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Linus Walleij March 17, 2015, 10:28 a.m. UTC | #7
On Thu, Mar 12, 2015 at 10:14 AM, Alexandre Belloni
<alexandre.belloni@free-electrons.com> wrote:

> As part of the system timer cleanup, I'll actually be needing that
> patch. Is it scheduled for inclusion or should I use "simple-bus" in the
> meantime?

No replies to the thread so I will send a pull request to ARM SoC cleanup
branch.

Yours,
Linus Walleij
Linus Walleij March 17, 2015, 10:38 a.m. UTC | #8
On Tue, Mar 17, 2015 at 11:28 AM, Linus Walleij
<linus.walleij@linaro.org> wrote:
> On Thu, Mar 12, 2015 at 10:14 AM, Alexandre Belloni
> <alexandre.belloni@free-electrons.com> wrote:
>
>> As part of the system timer cleanup, I'll actually be needing that
>> patch. Is it scheduled for inclusion or should I use "simple-bus" in the
>> meantime?
>
> No replies to the thread so I will send a pull request to ARM SoC cleanup
> branch.

Ah well I had some pending changes requested by Lee. Once Lee
ACK it for MFD and (hopefully) Bryan pitches in for LEDs, I'll post
a pull request.

BTW: Alexandre: your ACK or Reviewed-by would be appreciated ;)

Antione: if you're also using this, pls provide your ACK on the series.

Yours,
Linus Walleij
Alexandre Belloni March 18, 2015, 4:06 p.m. UTC | #9
On 03/03/2015 at 10:32:24 +0100, Linus Walleij wrote :
> This defines a new compatible option for MFD devices "simple-mfd" that will
> make the OF core spawn child devices for all subnodes of that MFD device.
> It is optional but handy for things like syscon and possibly other
> simpler MFD devices.
> 
> Since there was no file to put the documentation in, I took this opportunity
> to make a small writeup on MFD devices and add the compatible definition
> there.
> 
> Suggested-by: Lee Jones <lee.jones@linaro.org>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Devicetree <devicetree@vger.kernel.org>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Grant Likely <grant.likely@linaro.org>
> Cc: Pawel Moll <pawel.moll@arm.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
> Cc: Kumar Gala <galak@codeaurora.org>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>

Acked-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
Antoine Tenart March 20, 2015, 1:26 p.m. UTC | #10
On Tue, Mar 03, 2015 at 10:32:24AM +0100, Linus Walleij wrote:
> This defines a new compatible option for MFD devices "simple-mfd" that will
> make the OF core spawn child devices for all subnodes of that MFD device.
> It is optional but handy for things like syscon and possibly other
> simpler MFD devices.
> 
> Since there was no file to put the documentation in, I took this opportunity
> to make a small writeup on MFD devices and add the compatible definition
> there.
> 
> Suggested-by: Lee Jones <lee.jones@linaro.org>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Devicetree <devicetree@vger.kernel.org>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Grant Likely <grant.likely@linaro.org>
> Cc: Pawel Moll <pawel.moll@arm.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
> Cc: Kumar Gala <galak@codeaurora.org>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>

Since I'm also using this,

Acked-by: Antoine Tenart <antoine.tenart@free-electrons.com>

> ---
> I make the patch to the OF core in this one, it makes much more sense since
> it's a oneliner
> 
> Lee: this is a (tested!) implementation of your suggestion for simple-mfd.
> If you can eventually ACK this from the MFD side, I think it should be
> funneled through the ARM SoC tree.
> 
> Grant/Rob: if either of you can ACK the change to the OF core likewise it can
> be taken through ARM SoC.
> 
> DT binings maintainers: there is some background discussion on this here:
> http://marc.info/?l=linux-arm-kernel&m=142486676603889&w=2
> http://marc.info/?l=devicetree&m=142166313621469&w=2
> ---
>  Documentation/devicetree/bindings/mfd/mfd.txt | 40 +++++++++++++++++++++++++++
>  drivers/of/platform.c                         |  1 +
>  2 files changed, 41 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mfd/mfd.txt
> 
> diff --git a/Documentation/devicetree/bindings/mfd/mfd.txt b/Documentation/devicetree/bindings/mfd/mfd.txt
> new file mode 100644
> index 000000000000..cc057438abe8
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/mfd.txt
> @@ -0,0 +1,40 @@
> +Multi-Function Devices (MFD)
> +
> +These devices comprise a nexus for heterogeneous hardware blocks spawning
> +multiple child devices.
> +
> +A typical MFD can be:
> +
> +- A mixed signal ASIC on an external bus, sometimes a PMIC (power management
> +  integrated circuit) that is manufactured in a lower technology node (rough
> +  silicon) that handles analog drivers for things like audio amplifiers, LED
> +  drivers, level shifters, PHY (physical interfaces to things like USB or
> +  ethernet), regulators etc.
> +
> +- A range of memory registers containing "miscellaneous system registers" also
> +  known as a system controller "syscon" or any other memory range containing a
> +  mix of unrelated registers.
> +
> +Optional properties:
> +
> +- compatible : "simple-mfd" - this signifies that the operating system should
> +  spawn child devices for all the subnodes of the MFD device akin to how
> +  "simple-bus" inidicates when to spawn children for a simple memory-mapped
> +  bus. For more complex devices, when the nexus driver has to probe registers
> +  to figure out what children exist etc, this should not be used. In the latter
> +  case the child devices will be instantiated by the operating system.
> +
> +Example:
> +
> +foo@1000 {
> +		compatible = "syscon", "simple-mfd";
> +		reg = <0x010000 0x1000>;
> +
> +		led@08.0 {
> +			compatible = "register-bit-led";
> +			offset = <0x08>;
> +			mask = <0x01>;
> +			label = "myled";
> +			default-state = "on";
> +		};
> +};
> diff --git a/drivers/of/platform.c b/drivers/of/platform.c
> index b189733a1539..969ba4e1460c 100644
> --- a/drivers/of/platform.c
> +++ b/drivers/of/platform.c
> @@ -26,6 +26,7 @@
>  
>  const struct of_device_id of_default_bus_match_table[] = {
>  	{ .compatible = "simple-bus", },
> +	{ .compatible = "simple-mfd", },
>  #ifdef CONFIG_ARM_AMBA
>  	{ .compatible = "arm,amba-bus", },
>  #endif /* CONFIG_ARM_AMBA */
> -- 
> 1.9.3
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Grant Likely March 28, 2015, 1:41 p.m. UTC | #11
On Tue,  3 Mar 2015 10:32:24 +0100
, Linus Walleij <linus.walleij@linaro.org>
 wrote:
> This defines a new compatible option for MFD devices "simple-mfd" that will
> make the OF core spawn child devices for all subnodes of that MFD device.
> It is optional but handy for things like syscon and possibly other
> simpler MFD devices.
> 
> Since there was no file to put the documentation in, I took this opportunity
> to make a small writeup on MFD devices and add the compatible definition
> there.
> 
> Suggested-by: Lee Jones <lee.jones@linaro.org>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Devicetree <devicetree@vger.kernel.org>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Grant Likely <grant.likely@linaro.org>
> Cc: Pawel Moll <pawel.moll@arm.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
> Cc: Kumar Gala <galak@codeaurora.org>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> I make the patch to the OF core in this one, it makes much more sense since
> it's a oneliner
> 
> Lee: this is a (tested!) implementation of your suggestion for simple-mfd.
> If you can eventually ACK this from the MFD side, I think it should be
> funneled through the ARM SoC tree.
> 
> Grant/Rob: if either of you can ACK the change to the OF core likewise it can
> be taken through ARM SoC.
> 
> DT binings maintainers: there is some background discussion on this here:
> http://marc.info/?l=linux-arm-kernel&m=142486676603889&w=2
> http://marc.info/?l=devicetree&m=142166313621469&w=2
> ---
>  Documentation/devicetree/bindings/mfd/mfd.txt | 40 +++++++++++++++++++++++++++
>  drivers/of/platform.c                         |  1 +
>  2 files changed, 41 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mfd/mfd.txt
> 
> diff --git a/Documentation/devicetree/bindings/mfd/mfd.txt b/Documentation/devicetree/bindings/mfd/mfd.txt
> new file mode 100644
> index 000000000000..cc057438abe8
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/mfd.txt
> @@ -0,0 +1,40 @@
> +Multi-Function Devices (MFD)
> +
> +These devices comprise a nexus for heterogeneous hardware blocks spawning
> +multiple child devices.
> +
> +A typical MFD can be:
> +
> +- A mixed signal ASIC on an external bus, sometimes a PMIC (power management
> +  integrated circuit) that is manufactured in a lower technology node (rough
> +  silicon) that handles analog drivers for things like audio amplifiers, LED
> +  drivers, level shifters, PHY (physical interfaces to things like USB or
> +  ethernet), regulators etc.
> +
> +- A range of memory registers containing "miscellaneous system registers" also
> +  known as a system controller "syscon" or any other memory range containing a
> +  mix of unrelated registers.
> +
> +Optional properties:
> +
> +- compatible : "simple-mfd" - this signifies that the operating system should
> +  spawn child devices for all the subnodes of the MFD device akin to how
> +  "simple-bus" inidicates when to spawn children for a simple memory-mapped
> +  bus. For more complex devices, when the nexus driver has to probe registers
> +  to figure out what children exist etc, this should not be used. In the latter
> +  case the child devices will be instantiated by the operating system.
> +
> +Example:
> +
> +foo@1000 {
> +		compatible = "syscon", "simple-mfd";
> +		reg = <0x010000 0x1000>;
> +
> +		led@08.0 {
> +			compatible = "register-bit-led";
> +			offset = <0x08>;
> +			mask = <0x01>;
> +			label = "myled";
> +			default-state = "on";
> +		};
> +};
> diff --git a/drivers/of/platform.c b/drivers/of/platform.c
> index b189733a1539..969ba4e1460c 100644
> --- a/drivers/of/platform.c
> +++ b/drivers/of/platform.c
> @@ -26,6 +26,7 @@
>  
>  const struct of_device_id of_default_bus_match_table[] = {
>  	{ .compatible = "simple-bus", },
> +	{ .compatible = "simple-mfd", },
>  #ifdef CONFIG_ARM_AMBA
>  	{ .compatible = "arm,amba-bus", },
>  #endif /* CONFIG_ARM_AMBA */

Alright, so:

Acked-by: Grant Likely <grant.likely@linaro.org>

However, I think this is going to lead to a future problem. It solves
the immediate case of getting an MFD device to be auto populated when
the kernel doesn't need to immediately do something. However, I can
forsee a situation where the device can be supported by simple-mfd, but
at some point it is discovered that better features can be enabled if
the mfd device has a proper driver.

If/when we get into that situation, the above still works, but we're
going to need to add a blacklist match table so that simple-mfd can be
overridden by a more-specific compatible value. I don't have any
problems with doing that, but someone needs to implement it.

g.
Olof Johansson April 3, 2015, 6:34 p.m. UTC | #12
Hi,

I understand this is bikeshedding to some extent, but I'd also like to
avoid needless variation in binding formats:

On Tue, Mar 3, 2015 at 1:32 AM, Linus Walleij <linus.walleij@linaro.org> wrote:

> +
> +foo@1000 {
> +               compatible = "syscon", "simple-mfd";
> +               reg = <0x010000 0x1000>;
> +
> +               led@08.0 {

This doesn't seem to be a typo, you're using a period in the unit
address. I've never seen that done before, usually commas are used
instead.

Was there a reason for going with period?


-Olof
Lee Jones April 7, 2015, 7:16 a.m. UTC | #13
On Fri, 03 Apr 2015, Olof Johansson wrote:

> Hi,
> 
> I understand this is bikeshedding to some extent, but I'd also like to
> avoid needless variation in binding formats:
> 
> On Tue, Mar 3, 2015 at 1:32 AM, Linus Walleij <linus.walleij@linaro.org> wrote:
> 
> > +
> > +foo@1000 {
> > +               compatible = "syscon", "simple-mfd";
> > +               reg = <0x010000 0x1000>;
> > +
> > +               led@08.0 {
> 
> This doesn't seem to be a typo, you're using a period in the unit
> address. I've never seen that done before, usually commas are used
> instead.
> 
> Was there a reason for going with period?

LEDs have always done it [1].  I guess Bryan would be the best person
to answer the 'why'.

git grep "led.*@[0-9]*[.,][0-9]*" -- Documentation/devicetree/ arch/arm/boot/dts/
Linus Walleij April 7, 2015, 9 a.m. UTC | #14
On Fri, Apr 3, 2015 at 8:34 PM, Olof Johansson <olof@lixom.net> wrote:
> On Tue, Mar 3, 2015 at 1:32 AM, Linus Walleij <linus.walleij@linaro.org> wrote:
>
>> +
>> +foo@1000 {
>> +               compatible = "syscon", "simple-mfd";
>> +               reg = <0x010000 0x1000>;
>> +
>> +               led@08.0 {
>
> This doesn't seem to be a typo, you're using a period in the unit
> address. I've never seen that done before, usually commas are used
> instead.
>
> Was there a reason for going with period?

It was the suggested syntax from DT binding maintainer Rob Herring in
July last year:
http://marc.info/?l=linux-arm-kernel&m=140629736615939&w=2

Yours,
Linus Walleij
Linus Walleij April 7, 2015, 9:03 a.m. UTC | #15
On Sat, Mar 28, 2015 at 2:41 PM, Grant Likely <grant.likely@linaro.org> wrote:

> However, I think this is going to lead to a future problem. It solves
> the immediate case of getting an MFD device to be auto populated when
> the kernel doesn't need to immediately do something. However, I can
> forsee a situation where the device can be supported by simple-mfd, but
> at some point it is discovered that better features can be enabled if
> the mfd device has a proper driver.

For the general case where access to the MFD needs some mediation
or marshalling yes, I think this is theoretically possible.

For the most common MMIO case where the MFD devide is a syscon,
it is not so likely. Or rather extremelt unlikely.

We could restrict the use cases to strictly syscon/MMIO MFD devices
but it would just be a documentation fix.

Yours,
Linus Walleij
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/mfd/mfd.txt b/Documentation/devicetree/bindings/mfd/mfd.txt
new file mode 100644
index 000000000000..cc057438abe8
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/mfd.txt
@@ -0,0 +1,40 @@ 
+Multi-Function Devices (MFD)
+
+These devices comprise a nexus for heterogeneous hardware blocks spawning
+multiple child devices.
+
+A typical MFD can be:
+
+- A mixed signal ASIC on an external bus, sometimes a PMIC (power management
+  integrated circuit) that is manufactured in a lower technology node (rough
+  silicon) that handles analog drivers for things like audio amplifiers, LED
+  drivers, level shifters, PHY (physical interfaces to things like USB or
+  ethernet), regulators etc.
+
+- A range of memory registers containing "miscellaneous system registers" also
+  known as a system controller "syscon" or any other memory range containing a
+  mix of unrelated registers.
+
+Optional properties:
+
+- compatible : "simple-mfd" - this signifies that the operating system should
+  spawn child devices for all the subnodes of the MFD device akin to how
+  "simple-bus" inidicates when to spawn children for a simple memory-mapped
+  bus. For more complex devices, when the nexus driver has to probe registers
+  to figure out what children exist etc, this should not be used. In the latter
+  case the child devices will be instantiated by the operating system.
+
+Example:
+
+foo@1000 {
+		compatible = "syscon", "simple-mfd";
+		reg = <0x010000 0x1000>;
+
+		led@08.0 {
+			compatible = "register-bit-led";
+			offset = <0x08>;
+			mask = <0x01>;
+			label = "myled";
+			default-state = "on";
+		};
+};
diff --git a/drivers/of/platform.c b/drivers/of/platform.c
index b189733a1539..969ba4e1460c 100644
--- a/drivers/of/platform.c
+++ b/drivers/of/platform.c
@@ -26,6 +26,7 @@ 
 
 const struct of_device_id of_default_bus_match_table[] = {
 	{ .compatible = "simple-bus", },
+	{ .compatible = "simple-mfd", },
 #ifdef CONFIG_ARM_AMBA
 	{ .compatible = "arm,amba-bus", },
 #endif /* CONFIG_ARM_AMBA */