[PATCH] mfd: core: introduce of_node_name for mfd sub devices

[PATCH] mfd: core: introduce of_node_name for mfd sub devices

[Laxman Dewangan]

Multi Function Devices (MFDs) have multiple sub module whose driver is
developed in different sub-system like GPIO, regulators, RTC, clock etc.
The device tree of such device contains multiple sub-node which contains
the properties of these sub-modules.

The sub module gets of_node handle either by the dev->of_node or by getting
the child node handle from parent DT handle by finding child name on parent's
of_node.

To provide the of_node of sub-module directly, currently there is only one
approach:
- Add compatible value when defining the sub-module in mfd core and
  add this properties when adding DT.

Introduce the of_node_name of each sub devices which is set when defining
the mfd_cells of the sub devices and get the handle of these child node
when adding the mfd_devices by getting the sub-node handle with matching
the node name getting the sub-node handle with matching the node name.

Signed-off-by: Laxman Dewangan <ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
Creating this patch based on the discussion on patch
[PATCH 1/4] mfd: add support for AMS AS3722 PMIC
The discussion on above patch is not concluded and want to have
further discussion on this patch.

 Documentation/devicetree/bindings/mfd/mfd-core.txt |   57 ++++++++++++++++++++
 drivers/mfd/mfd-core.c                             |   10 +++-
 include/linux/mfd/core.h                           |    6 ++
 3 files changed, 71 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/mfd/mfd-core.txt

diff --git a/Documentation/devicetree/bindings/mfd/mfd-core.txt b/Documentation/devicetree/bindings/mfd/mfd-core.txt
new file mode 100644
index 0000000..d68c893
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/mfd-core.txt
@@ -0,0 +1,57 @@
+MFD DT binding document
+-----------------------
+
+Multi Function Devices (MFDs) have multiple sub module whose driver is developed
+in different sub-system like GPIO, regulators, RTC, clock etc. The device
+tree of such device contains multiple sub-node which contains the properties
+of these sub-modules.
+The sub modules get of_node handle either by the dev->of_node or by getting
+the child node handle from parent DT handle by finding child name on parent's
+of_node.
+To provide the of_node of sub-module directly, there is two approach:
+- Add compatible value when defining the sub-module in mfd core and
+  add this properties when adding DT.
+- Add the of_node_name when defining the sub-module in mfd core and
+  add keep same name of child node when adding DT.
+
+If none of above matches then sub-module driver will not get their of_node
+and they need to derive the method to get their node from parent node.
+
+Examples:
+DT file:
+--------
+	mfd_device_dt {
+		....
+		gpio_child {
+			/* Properties which need by gpio sub module */
+		};
+
+		rtc_child {
+			/* Properties which need by rtc sub module */
+		};
+
+		regulator_child {
+			/* Properties which need by regulator sub module */
+		};
+	};
+
+
+Driver code:
+-----------
+static struct mfd_cell mfd_abc_devs[] = {
+	{
+		.name = "mfd-abc-gpio",
+		.of_node_name = "gpio_child";
+	},
+	{
+		.name = "mfd-abc-regulator",
+		.of_node_name = "regulator_child";
+	},
+	{
+		.name = "mfd-abc-rtc",
+		.of_node_name = "rtc_child";
+	},
+};
+
+Here sub-node names are gpio_child, rtc_child, regulator_child and it is same
+as of_node_name defined in the driver.
diff --git a/drivers/mfd/mfd-core.c b/drivers/mfd/mfd-core.c
index f421586..88836c2 100644
--- a/drivers/mfd/mfd-core.c
+++ b/drivers/mfd/mfd-core.c
@@ -99,9 +99,15 @@ static int mfd_add_device(struct device *parent, int id,
 	pdev->dev.dma_mask = parent->dma_mask;
 	pdev->dev.dma_parms = parent->dma_parms;
 
-	if (parent->of_node && cell->of_compatible) {
+	if (parent->of_node && (cell->of_compatible || cell->of_node_name)) {
 		for_each_child_of_node(parent->of_node, np) {
-			if (of_device_is_compatible(np, cell->of_compatible)) {
+			if (cell->of_compatible &&
+			    of_device_is_compatible(np, cell->of_compatible)) {
+				pdev->dev.of_node = np;
+				break;
+			}
+			if (cell->of_node_name && np->name &&
+			    !strcmp(cell->of_node_name, np->name)) {
 				pdev->dev.of_node = np;
 				break;
 			}
diff --git a/include/linux/mfd/core.h b/include/linux/mfd/core.h
index cebe97e..4cf891f 100644
--- a/include/linux/mfd/core.h
+++ b/include/linux/mfd/core.h
@@ -45,6 +45,12 @@ struct mfd_cell {
 	const char		*of_compatible;
 
 	/*
+	 * Device tree sub-node name.
+	 * See: Documentation/devicetree/bindings/mfd/mfd-core.txt
+	 */
+	const char		*of_node_name;
+
+	/*
 	 * These resources can be specified relative to the parent device.
 	 * For accessing hardware you should use resources from the platform dev
 	 */
-- 
1.7.1.1

--
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

Re: [PATCH] mfd: core: introduce of_node_name for mfd sub devices

[Lee Jones]

On Thu, 19 Sep 2013, Laxman Dewangan wrote:

> Multi Function Devices (MFDs) have multiple sub module whose driver is
> developed in different sub-system like GPIO, regulators, RTC, clock etc.
> The device tree of such device contains multiple sub-node which contains
> the properties of these sub-modules.
> 
> The sub module gets of_node handle either by the dev->of_node or by getting
> the child node handle from parent DT handle by finding child name on parent's
> of_node.
> 
> To provide the of_node of sub-module directly, currently there is only one
> approach:
> - Add compatible value when defining the sub-module in mfd core and
>   add this properties when adding DT.
> 
> Introduce the of_node_name of each sub devices which is set when defining
> the mfd_cells of the sub devices and get the handle of these child node
> when adding the mfd_devices by getting the sub-node handle with matching
> the node name getting the sub-node handle with matching the node name.
> 
> Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
> ---
> Creating this patch based on the discussion on patch
> [PATCH 1/4] mfd: add support for AMS AS3722 PMIC
> The discussion on above patch is not concluded and want to have
> further discussion on this patch.

I'm not entirely sure this is what Mark was saying. I think he was
complaining about the existence of the sub-nodes rather than how the
MFD Core assigns their of_node. My take is that the chip is really a
single device which provides different bits of functionality. To break
that functionality up and disperse the drivers into various subsystems
is a Linuxisum. By providing each functional block with its own node
you're describing how we do things in Linux, rather than specifying a
single node for the AS3722 which would probably be the norm.

Do the sub-nodes have their own properties? If so, it would be worth
breaking them up as other OSes could reuse the specifics. If they do,
then you need so put them in the binding. If they don't, then you do
not require sub-nodes. The MFD core will ensure the sub-devices are
probed and there is no requirement for the of_node to be assigned.

>  Documentation/devicetree/bindings/mfd/mfd-core.txt |   57 ++++++++++++++++++++
>  drivers/mfd/mfd-core.c                             |   10 +++-
>  include/linux/mfd/core.h                           |    6 ++
>  3 files changed, 71 insertions(+), 2 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/mfd/mfd-core.txt
> 
> diff --git a/Documentation/devicetree/bindings/mfd/mfd-core.txt b/Documentation/devicetree/bindings/mfd/mfd-core.txt
> new file mode 100644
> index 0000000..d68c893
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/mfd-core.txt
> @@ -0,0 +1,57 @@
> +MFD DT binding document
> +-----------------------
> +
> +Multi Function Devices (MFDs) have multiple sub module whose driver is developed
> +in different sub-system like GPIO, regulators, RTC, clock etc. The device
> +tree of such device contains multiple sub-node which contains the properties
> +of these sub-modules.
> +The sub modules get of_node handle either by the dev->of_node or by getting
> +the child node handle from parent DT handle by finding child name on parent's
> +of_node.
> +To provide the of_node of sub-module directly, there is two approach:
> +- Add compatible value when defining the sub-module in mfd core and
> +  add this properties when adding DT.
> +- Add the of_node_name when defining the sub-module in mfd core and
> +  add keep same name of child node when adding DT.
> +
> +If none of above matches then sub-module driver will not get their of_node
> +and they need to derive the method to get their node from parent node.
> +
> +Examples:
> +DT file:
> +--------
> +	mfd_device_dt {
> +		....
> +		gpio_child {
> +			/* Properties which need by gpio sub module */
> +		};
> +
> +		rtc_child {
> +			/* Properties which need by rtc sub module */
> +		};
> +
> +		regulator_child {
> +			/* Properties which need by regulator sub module */
> +		};
> +	};
> +
> +
> +Driver code:
> +-----------
> +static struct mfd_cell mfd_abc_devs[] = {
> +	{
> +		.name = "mfd-abc-gpio",
> +		.of_node_name = "gpio_child";
> +	},
> +	{
> +		.name = "mfd-abc-regulator",
> +		.of_node_name = "regulator_child";
> +	},
> +	{
> +		.name = "mfd-abc-rtc",
> +		.of_node_name = "rtc_child";
> +	},
> +};
> +
> +Here sub-node names are gpio_child, rtc_child, regulator_child and it is same
> +as of_node_name defined in the driver.
> diff --git a/drivers/mfd/mfd-core.c b/drivers/mfd/mfd-core.c
> index f421586..88836c2 100644
> --- a/drivers/mfd/mfd-core.c
> +++ b/drivers/mfd/mfd-core.c
> @@ -99,9 +99,15 @@ static int mfd_add_device(struct device *parent, int id,
>  	pdev->dev.dma_mask = parent->dma_mask;
>  	pdev->dev.dma_parms = parent->dma_parms;
>  
> -	if (parent->of_node && cell->of_compatible) {
> +	if (parent->of_node && (cell->of_compatible || cell->of_node_name)) {
>  		for_each_child_of_node(parent->of_node, np) {
> -			if (of_device_is_compatible(np, cell->of_compatible)) {
> +			if (cell->of_compatible &&
> +			    of_device_is_compatible(np, cell->of_compatible)) {
> +				pdev->dev.of_node = np;
> +				break;
> +			}
> +			if (cell->of_node_name && np->name &&
> +			    !strcmp(cell->of_node_name, np->name)) {
>  				pdev->dev.of_node = np;
>  				break;
>  			}
> diff --git a/include/linux/mfd/core.h b/include/linux/mfd/core.h
> index cebe97e..4cf891f 100644
> --- a/include/linux/mfd/core.h
> +++ b/include/linux/mfd/core.h
> @@ -45,6 +45,12 @@ struct mfd_cell {
>  	const char		*of_compatible;
>  
>  	/*
> +	 * Device tree sub-node name.
> +	 * See: Documentation/devicetree/bindings/mfd/mfd-core.txt
> +	 */
> +	const char		*of_node_name;
> +
> +	/*
>  	 * These resources can be specified relative to the parent device.
>  	 * For accessing hardware you should use resources from the platform dev
>  	 */

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH] mfd: core: introduce of_node_name for mfd sub devices

[Laxman Dewangan]

On Thursday 19 September 2013 02:00 PM, Lee Jones wrote:
> On Thu, 19 Sep 2013, Laxman Dewangan wrote:
>
>> Multi Function Devices (MFDs) have multiple sub module whose driver is
>> developed in different sub-system like GPIO, regulators, RTC, clock etc.
>> The device tree of such device contains multiple sub-node which contains
>> the properties of these sub-modules.
>>
>> The sub module gets of_node handle either by the dev->of_node or by getting
>> the child node handle from parent DT handle by finding child name on parent's
>> of_node.
>>
>> To provide the of_node of sub-module directly, currently there is only one
>> approach:
>> - Add compatible value when defining the sub-module in mfd core and
>>    add this properties when adding DT.
>>
>> Introduce the of_node_name of each sub devices which is set when defining
>> the mfd_cells of the sub devices and get the handle of these child node
>> when adding the mfd_devices by getting the sub-node handle with matching
>> the node name getting the sub-node handle with matching the node name.
>>
>> Signed-off-by: Laxman Dewangan <ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
>> ---
>> Creating this patch based on the discussion on patch
>> [PATCH 1/4] mfd: add support for AMS AS3722 PMIC
>> The discussion on above patch is not concluded and want to have
>> further discussion on this patch.
> I'm not entirely sure this is what Mark was saying. I think he was
> complaining about the existence of the sub-nodes rather than how the
> MFD Core assigns their of_node.

Yes, Mark is only saying that he is not comfortable with the compatible 
until it is ip based driver.
I wanted to continue the discussion to find out the acceptable way so 
that I can use this in as3722 driver.

I added the of_node_name because each sub-driver gets its sub node by 
finding child node name from parent and hence duplicated the code on 
each driver.


--
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

Re: [PATCH] mfd: core: introduce of_node_name for mfd sub devices

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1324 bytes --]

On Thu, Sep 19, 2013 at 09:30:50AM +0100, Lee Jones wrote:

> I'm not entirely sure this is what Mark was saying. I think he was
> complaining about the existence of the sub-nodes rather than how the
> MFD Core assigns their of_node. My take is that the chip is really a
> single device which provides different bits of functionality. To break
> that functionality up and disperse the drivers into various subsystems
> is a Linuxisum. By providing each functional block with its own node
> you're describing how we do things in Linux, rather than specifying a
> single node for the AS3722 which would probably be the norm.

Yes, that's exactly what I was thinking of.

> Do the sub-nodes have their own properties? If so, it would be worth
> breaking them up as other OSes could reuse the specifics. If they do,
> then you need so put them in the binding. If they don't, then you do
> not require sub-nodes. The MFD core will ensure the sub-devices are
> probed and there is no requirement for the of_node to be assigned.

You do see some reusable IP blocks (like the regualtors on the wm831x
PMICs for example, they're repeated blocks) which can be reused but
generally they have a register base as part of the binding.  Personally
if it's just a property or two I'd probably just put them on the root
node for the device.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] mfd: core: introduce of_node_name for mfd sub devices

[Lee Jones]

On Thu, 19 Sep 2013, Mark Brown wrote:

> On Thu, Sep 19, 2013 at 09:30:50AM +0100, Lee Jones wrote:
> 
> > I'm not entirely sure this is what Mark was saying. I think he was
> > complaining about the existence of the sub-nodes rather than how the
> > MFD Core assigns their of_node. My take is that the chip is really a
> > single device which provides different bits of functionality. To break
> > that functionality up and disperse the drivers into various subsystems
> > is a Linuxisum. By providing each functional block with its own node
> > you're describing how we do things in Linux, rather than specifying a
> > single node for the AS3722 which would probably be the norm.
> 
> Yes, that's exactly what I was thinking of.
> 
> > Do the sub-nodes have their own properties? If so, it would be worth
> > breaking them up as other OSes could reuse the specifics. If they do,
> > then you need so put them in the binding. If they don't, then you do
> > not require sub-nodes. The MFD core will ensure the sub-devices are
> > probed and there is no requirement for the of_node to be assigned.
> 
> You do see some reusable IP blocks (like the regualtors on the wm831x
> PMICs for example, they're repeated blocks) which can be reused but
> generally they have a register base as part of the binding.  Personally
> if it's just a property or two I'd probably just put them on the root
> node for the device.

Agreed. Besides, there doesn't seem to be *any* sub-device properties
defined in the binding document. So what are you trying to achieve
with the child nodes?

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
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

Re: [PATCH] mfd: core: introduce of_node_name for mfd sub devices

[Lee Jones]

> >>>Do the sub-nodes have their own properties? If so, it would be worth
> >>>breaking them up as other OSes could reuse the specifics. If they do,
> >>>then you need so put them in the binding. If they don't, then you do
> >>>not require sub-nodes. The MFD core will ensure the sub-devices are
> >>>probed and there is no requirement for the of_node to be assigned.
> >>You do see some reusable IP blocks (like the regualtors on the wm831x
> >>PMICs for example, they're repeated blocks) which can be reused but
> >>generally they have a register base as part of the binding.  Personally
> >>if it's just a property or two I'd probably just put them on the root
> >>node for the device.
> >Agreed. Besides, there doesn't seem to be *any* sub-device properties
> >defined in the binding document. So what are you trying to achieve
> >with the child nodes?
> 
> I wanted to have the DT like:
> 
> as3722 {
>                 compatible = "ams,as3722";
>                 reg = <0x40>;
> 
>                 #interrupt-controller;
>                 .....
> 
> 
>                 regulators {
>                             ldo1-in-supply = <..>;
>                             ....
>                             sd0 {
>                                     regulator-name = "vdd-cpu";
>                                     .....
>                             };
>                             sd1 {
>                                     regulator-name = "vdd-ddr";
>                                     .....
>                             };
>                             ....
>             };
> };
> 
> And regulator driver should get the regulator node by their
> pdev->dev.of_node.
> Currently, in most of driver, we are having the code on regulator
> driver to get "regulators" node from parent node which I want to
> avoid.

Ah, I see. Yes, I believe the regulators should have their own node,
complete with a compatible string. To have each regulator listed
separately in the parent node seems a little messy. Just out of
interest, how many regulators are we talking about here?

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
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

Re: [PATCH] mfd: core: introduce of_node_name for mfd sub devices

[Laxman Dewangan]

On Thursday 19 September 2013 05:30 PM, Lee Jones wrote:
> On Thu, 19 Sep 2013, Mark Brown wrote:
>
>> On Thu, Sep 19, 2013 at 09:30:50AM +0100, Lee Jones wrote:
>>
>>> I'm not entirely sure this is what Mark was saying. I think he was
>>> complaining about the existence of the sub-nodes rather than how the
>>> MFD Core assigns their of_node. My take is that the chip is really a
>>> single device which provides different bits of functionality. To break
>>> that functionality up and disperse the drivers into various subsystems
>>> is a Linuxisum. By providing each functional block with its own node
>>> you're describing how we do things in Linux, rather than specifying a
>>> single node for the AS3722 which would probably be the norm.
>> Yes, that's exactly what I was thinking of.
>>
>>> Do the sub-nodes have their own properties? If so, it would be worth
>>> breaking them up as other OSes could reuse the specifics. If they do,
>>> then you need so put them in the binding. If they don't, then you do
>>> not require sub-nodes. The MFD core will ensure the sub-devices are
>>> probed and there is no requirement for the of_node to be assigned.
>> You do see some reusable IP blocks (like the regualtors on the wm831x
>> PMICs for example, they're repeated blocks) which can be reused but
>> generally they have a register base as part of the binding.  Personally
>> if it's just a property or two I'd probably just put them on the root
>> node for the device.
> Agreed. Besides, there doesn't seem to be *any* sub-device properties
> defined in the binding document. So what are you trying to achieve
> with the child nodes?
>

I wanted to have the DT like:

as3722 {
                 compatible = "ams,as3722";
                 reg = <0x40>;

                 #interrupt-controller;
                 .....


                 regulators {
                             ldo1-in-supply = <..>;
                             ....
                             sd0 {
                                     regulator-name = "vdd-cpu";
                                     .....
                             };
                             sd1 {
                                     regulator-name = "vdd-ddr";
                                     .....
                             };
                             ....
             };
};



And regulator driver should get the regulator node by their 
pdev->dev.of_node.
Currently, in most of driver, we are having the code on regulator driver 
to get "regulators" node from parent node which I want to avoid.





--
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

Re: [PATCH] mfd: core: introduce of_node_name for mfd sub devices

[Laxman Dewangan]

On Thursday 19 September 2013 05:52 PM, Lee Jones wrote:
>>>>> Do the sub-nodes have their own properties? If so, it would be worth
>>>>> breaking them up as other OSes could reuse the specifics. If they do,
>>>>> then you need so put them in the binding. If they don't, then you do
>>>>> not require sub-nodes. The MFD core will ensure the sub-devices are
>>>>> probed and there is no requirement for the of_node to be assigned.
>>>> You do see some reusable IP blocks (like the regualtors on the wm831x
>>>> PMICs for example, they're repeated blocks) which can be reused but
>>>> generally they have a register base as part of the binding.  Personally
>>>> if it's just a property or two I'd probably just put them on the root
>>>> node for the device.
>>> Agreed. Besides, there doesn't seem to be *any* sub-device properties
>>> defined in the binding document. So what are you trying to achieve
>>> with the child nodes?
>> I wanted to have the DT like:
>>
>> as3722 {
>>                  compatible = "ams,as3722";
>>                  reg = <0x40>;
>>
>>                  #interrupt-controller;
>>                  .....
>>
>>
>>                  regulators {
>>                              ldo1-in-supply = <..>;
>>                              ....
>>                              sd0 {
>>                                      regulator-name = "vdd-cpu";
>>                                      .....
>>                              };
>>                              sd1 {
>>                                      regulator-name = "vdd-ddr";
>>                                      .....
>>                              };
>>                              ....
>>              };
>> };
>>
>> And regulator driver should get the regulator node by their
>> pdev->dev.of_node.
>> Currently, in most of driver, we are having the code on regulator
>> driver to get "regulators" node from parent node which I want to
>> avoid.
> Ah, I see. Yes, I believe the regulators should have their own node,
> complete with a compatible string. To have each regulator listed
> separately in the parent node seems a little messy. Just out of
> interest, how many regulators are we talking about here?
>
There is 7 DCDC step down and 10 LDOs.
It is more readable if sub-module properties are grouped and defined on 
different sub-node in place of having this in single parent node.

Re: [PATCH] mfd: core: introduce of_node_name for mfd sub devices

[Stephen Warren]

On 09/19/2013 06:22 AM, Lee Jones wrote:
>>>>> Do the sub-nodes have their own properties? If so, it would be worth
>>>>> breaking them up as other OSes could reuse the specifics. If they do,
>>>>> then you need so put them in the binding. If they don't, then you do
>>>>> not require sub-nodes. The MFD core will ensure the sub-devices are
>>>>> probed and there is no requirement for the of_node to be assigned.
>>>> You do see some reusable IP blocks (like the regualtors on the wm831x
>>>> PMICs for example, they're repeated blocks) which can be reused but
>>>> generally they have a register base as part of the binding.  Personally
>>>> if it's just a property or two I'd probably just put them on the root
>>>> node for the device.
>>> Agreed. Besides, there doesn't seem to be *any* sub-device properties
>>> defined in the binding document. So what are you trying to achieve
>>> with the child nodes?
>>
>> I wanted to have the DT like:
>>
>> as3722 {
>>                 compatible = "ams,as3722";
>>                 reg = <0x40>;
>>
>>                 #interrupt-controller;
>>                 .....
>>
>>
>>                 regulators {
>>                             ldo1-in-supply = <..>;
>>                             ....
>>                             sd0 {
>>                                     regulator-name = "vdd-cpu";
>>                                     .....
>>                             };
>>                             sd1 {
>>                                     regulator-name = "vdd-ddr";
>>                                     .....
>>                             };
>>                             ....
>>             };
>> };
>>
>> And regulator driver should get the regulator node by their
>> pdev->dev.of_node.
>> Currently, in most of driver, we are having the code on regulator
>> driver to get "regulators" node from parent node which I want to
>> avoid.
> 
> Ah, I see. Yes, I believe the regulators should have their own node,

The use of a "regulators" node to keep all the regulator configuration
in one place seems fine...

> complete with a compatible string.

... but I see not reason why that node has to have a separate compatible
property, or /has/ to have a separate driver.

I think having a compatible value in this node would only be required if
the HW block that implements those registers is actually expected to be
shared between n different chips, and hence it's likely that you'd get
re-use out of a separate binding, driver, etc.

It's perfectly reasonable for the regulator MFD driver to know that the
binding for the top-level PMIC node has a regulators child node, and go
find it by name, and read whatever properties/nodes it needs directly
out of it. Writing code that way in no ways implies a need for a
compatible value.

> To have each regulator listed
> separately in the parent node seems a little messy. Just out of
> interest, how many regulators are we talking about here?
--
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

Re: [PATCH] mfd: core: introduce of_node_name for mfd sub devices

[Stephen Warren]

On 09/19/2013 02:29 AM, Laxman Dewangan wrote:

> diff --git a/Documentation/devicetree/bindings/mfd/mfd-core.txt b/Documentation/devicetree/bindings/mfd/mfd-core.txt

> +MFD DT binding document
> +-----------------------
> +
> +Multi Function Devices (MFDs) have multiple sub module whose driver is developed
> +in different sub-system like GPIO, regulators, RTC, clock etc. The device
> +tree of such device contains multiple sub-node which contains the properties
> +of these sub-modules.
> +The sub modules get of_node handle either by the dev->of_node or by getting
> +the child node handle from parent DT handle by finding child name on parent's
> +of_node.
> +To provide the of_node of sub-module directly, there is two approach:
> +- Add compatible value when defining the sub-module in mfd core and
> +  add this properties when adding DT.
> +- Add the of_node_name when defining the sub-module in mfd core and
> +  add keep same name of child node when adding DT.
> +
> +If none of above matches then sub-module driver will not get their of_node
> +and they need to derive the method to get their node from parent node.

That's all *far* too specific to Linux's internals for a DT binding
document.

I think it's fine for DT bindings for aggregate devices to be written to
require a separate node (with a specific name) for various
sub-components of the HW, if it makes sense to do so from a purely DT
binding perspective. However, the fact that some DT bindings might be
written that way implies nothing about any other particular DT binding,
nor about all other DT bindings.

And indeed if you find that there are a bunch of DT bindings that share
this structure, by all means create some common code to simplify the
drivers for those bindings. However, that still doesn't mean that every
driver or binding has to be structured like that.
--
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

Re: [PATCH] mfd: core: introduce of_node_name for mfd sub devices

[Lee Jones]

> >> And regulator driver should get the regulator node by their
> >> pdev->dev.of_node.
> >> Currently, in most of driver, we are having the code on regulator
> >> driver to get "regulators" node from parent node which I want to
> >> avoid.
> > 
> > Ah, I see. Yes, I believe the regulators should have their own node,
> 
> The use of a "regulators" node to keep all the regulator configuration
> in one place seems fine...
> 
> > complete with a compatible string.
> 
> ... but I see not reason why that node has to have a separate compatible
> property, or /has/ to have a separate driver.
> 
> I think having a compatible value in this node would only be required if
> the HW block that implements those registers is actually expected to be
> shared between n different chips, and hence it's likely that you'd get
> re-use out of a separate binding, driver, etc.
> 
> It's perfectly reasonable for the regulator MFD driver to know that the
> binding for the top-level PMIC node has a regulators child node, and go
> find it by name, and read whatever properties/nodes it needs directly
> out of it. Writing code that way in no ways implies a need for a
> compatible value.

Sounds fine.

> > To have each regulator listed
> > separately in the parent node seems a little messy. Just out of
> > interest, how many regulators are we talking about here?

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog