[PATCH 1/3] mfd: always assign of_node in mfd_add_device()

[PATCH 1/3] mfd: always assign of_node in mfd_add_device()

[Stephen Warren]

From: Stephen Warren <swarren@nvidia.com>

mfd_add_device() assigns .of_node in the device objects it creates only
if the mfd_cell for the device has the .of_compatible field set and the
DT node for the top-level MFD device contains a child whose compatible
property matches the cell's .of_compatible field.

This leaves .of_node unset in many cases. When this happens, entries in
the DT /aliases property which refer to the top-level MFD DT node will
never match the MFD child devices, hence causing the requested alias not
to be honored.

Solve this by setting each MFD child device's .of_node equal to the top-
level MFD device's .of_node field in the cases where it would otherwise
remain unset.

The first use-case for this will be aliases for the TPS6586x's RTC
device.

Signed-off-by: Stephen Warren <swarren@nvidia.com>
---
The 3 patches in this series are all independent; they can be applied
to their respective subsystems in any order. I'm simply posting them as
a series to make the use-case more obvious.
---
 drivers/mfd/mfd-core.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/mfd/mfd-core.c b/drivers/mfd/mfd-core.c
index 267649244737..32e8d47d9002 100644
--- a/drivers/mfd/mfd-core.c
+++ b/drivers/mfd/mfd-core.c
@@ -117,6 +117,8 @@ static int mfd_add_device(struct device *parent, int id,
 			}
 		}
 	}
+	if (!pdev->dev.of_node)
+		pdev->dev.of_node = parent->of_node;
 
 	if (cell->pdata_size) {
 		ret = platform_device_add_data(pdev,
-- 
1.8.1.5

[PATCH 2/3] rtc: honor device tree /alias entries when assigning IDs

[Stephen Warren]

From: Stephen Warren <swarren@nvidia.com>

Assign RTC device IDs based on device tree /aliases entries if present,
falling back to the existing numbering scheme if there is no /aliases
entry (which includes when the system isn't booted using DT), or there
is a numbering conflict.

This is useful in systems with multiple RTC devices, to ensure that the
best RTC device is selected as /dev/rtc0, which provides the overall
system time.

For example, Tegra has an on-SoC RTC that is not battery backed,
typically coupled with an off-SoC RTC that is battery backed. Only the
latter is useful for populating the system time, yet the former is
useful e.g. for wakeup timing, since the time is not lost when the
system is sleeps.

Signed-off-by: Stephen Warren <swarren@nvidia.com>
---
The 3 patches in this series are all independent; they can be applied
to their respective subsystems in any order. I'm simply posting them as
a series to make the use-case more obvious.
---
 drivers/rtc/class.c | 22 ++++++++++++++++++----
 1 file changed, 18 insertions(+), 4 deletions(-)

diff --git a/drivers/rtc/class.c b/drivers/rtc/class.c
index 02426812bebc..7989b5d030cb 100644
--- a/drivers/rtc/class.c
+++ b/drivers/rtc/class.c
@@ -14,6 +14,7 @@
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
 #include <linux/module.h>
+#include <linux/of.h>
 #include <linux/rtc.h>
 #include <linux/kdev_t.h>
 #include <linux/idr.h>
@@ -157,12 +158,25 @@ struct rtc_device *rtc_device_register(const char *name, struct device *dev,
 {
 	struct rtc_device *rtc;
 	struct rtc_wkalrm alrm;
-	int id, err;
+	int of_id, id = -1, err;
+
+	if (dev->of_node) {
+		of_id = of_alias_get_id(dev->of_node, "rtc");
+		if (of_id >= 0) {
+			id = ida_simple_get(&rtc_ida, of_id, of_id + 1,
+					    GFP_KERNEL);
+			if (id < 0)
+				dev_warn(dev, "/aliases ID %d not available\n",
+					 of_id);
+		}
+	}
 
-	id = ida_simple_get(&rtc_ida, 0, 0, GFP_KERNEL);
 	if (id < 0) {
-		err = id;
-		goto exit;
+		id = ida_simple_get(&rtc_ida, 0, 0, GFP_KERNEL);
+		if (id < 0) {
+			err = id;
+			goto exit;
+		}
 	}
 
 	rtc = kzalloc(sizeof(struct rtc_device), GFP_KERNEL);
-- 
1.8.1.5

[PATCH 3/3] ARM: tegra: set up /aliases entries for RTCs

[Stephen Warren]

From: Stephen Warren <swarren@nvidia.com>

This ensures that the PMIC RTC provides the system time, rather than
the on-SoC RTC, which is not battery-backed.

Signed-off-by: Stephen Warren <swarren@nvidia.com>
---
This patch is just an example. If the previous two patches are accepted,
I'll flesh this patch out to cover all boards, and repost.
---
 arch/arm/boot/dts/tegra20-seaboard.dts | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/arm/boot/dts/tegra20-seaboard.dts b/arch/arm/boot/dts/tegra20-seaboard.dts
index ff63da7ff00d..1e62b7c5611f 100644
--- a/arch/arm/boot/dts/tegra20-seaboard.dts
+++ b/arch/arm/boot/dts/tegra20-seaboard.dts
@@ -6,6 +6,11 @@
 	model = "NVIDIA Seaboard";
 	compatible = "nvidia,seaboard", "nvidia,tegra20";
 
+	aliases {
+		rtc0 = "/i2c at 7000d000/tps6586x at 34";
+		rtc1 = "/rtc at 7000e000";
+	};
+
 	memory {
 		reg = <0x00000000 0x40000000>;
 	};
-- 
1.8.1.5

[PATCH 1/3] mfd: always assign of_node in mfd_add_device()

[Lee Jones]

> From: Stephen Warren <swarren@nvidia.com>
> 
> mfd_add_device() assigns .of_node in the device objects it creates only
> if the mfd_cell for the device has the .of_compatible field set and the
> DT node for the top-level MFD device contains a child whose compatible
> property matches the cell's .of_compatible field.
> 
> This leaves .of_node unset in many cases. When this happens, entries in
> the DT /aliases property which refer to the top-level MFD DT node will
> never match the MFD child devices, hence causing the requested alias not
> to be honored.
> 
> Solve this by setting each MFD child device's .of_node equal to the top-
> level MFD device's .of_node field in the cases where it would otherwise
> remain unset.

How sure are you that this will be void of repercussions?

> The first use-case for this will be aliases for the TPS6586x's RTC
> device.

Isn't it viable to supply the of_compatible strings for these nodes
and search the parent for its alias property instead?

> Signed-off-by: Stephen Warren <swarren@nvidia.com>
> ---
> The 3 patches in this series are all independent; they can be applied
> to their respective subsystems in any order. I'm simply posting them as
> a series to make the use-case more obvious.
> ---
>  drivers/mfd/mfd-core.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/mfd/mfd-core.c b/drivers/mfd/mfd-core.c
> index 267649244737..32e8d47d9002 100644
> --- a/drivers/mfd/mfd-core.c
> +++ b/drivers/mfd/mfd-core.c
> @@ -117,6 +117,8 @@ static int mfd_add_device(struct device *parent, int id,
>  			}
>  		}
>  	}
> +	if (!pdev->dev.of_node)
> +		pdev->dev.of_node = parent->of_node;
>  
>  	if (cell->pdata_size) {
>  		ret = platform_device_add_data(pdev,

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

[PATCH 1/3] mfd: always assign of_node in mfd_add_device()

[Stephen Warren]

On 12/10/2013 01:40 AM, Lee Jones wrote:
>> From: Stephen Warren <swarren@nvidia.com>
>>
>> mfd_add_device() assigns .of_node in the device objects it creates only
>> if the mfd_cell for the device has the .of_compatible field set and the
>> DT node for the top-level MFD device contains a child whose compatible
>> property matches the cell's .of_compatible field.
>>
>> This leaves .of_node unset in many cases. When this happens, entries in
>> the DT /aliases property which refer to the top-level MFD DT node will
>> never match the MFD child devices, hence causing the requested alias not
>> to be honored.
>>
>> Solve this by setting each MFD child device's .of_node equal to the top-
>> level MFD device's .of_node field in the cases where it would otherwise
>> remain unset.
> 
> How sure are you that this will be void of repercussions?

I'm simply hopeful:-) It doesn't seem likely that this would cause any
issues, since presumably any devices that are being instantiated from DT
either already work correctly, or wouldn't be affected by this change
since they're manually searching whatever the appropriate DT node is.
Are there any particular points you think I should look into?

>> The first use-case for this will be aliases for the TPS6586x's RTC
>> device.
> 
> Isn't it viable to supply the of_compatible strings for these nodes
> and search the parent for its alias property instead?

I did think of that, but there are two reasons I chose not to do that
initially:

a) It requires that every single top-level MFD driver be edited to set
the .of_compatible field in their mfd_cells array, whereas this patch
automatically works for all drivers.

b) The mfd_cells .of_compatible field is a single entry, whereas a
top-level MFD driver could support an arbitrary number of DT compatible
values. I suppose one could work around this by setting the mfd_cells
.of_compatible field at run-time based on the actual compatible value of
the top-level DT node, but that feels icky.

[PATCH 1/3] mfd: always assign of_node in mfd_add_device()

[Lee Jones]

> From: Stephen Warren <swarren@nvidia.com>
> 
> mfd_add_device() assigns .of_node in the device objects it creates only
> if the mfd_cell for the device has the .of_compatible field set and the
> DT node for the top-level MFD device contains a child whose compatible
> property matches the cell's .of_compatible field.
> 
> This leaves .of_node unset in many cases. When this happens, entries in
> the DT /aliases property which refer to the top-level MFD DT node will
> never match the MFD child devices, hence causing the requested alias not
> to be honored.
> 
> Solve this by setting each MFD child device's .of_node equal to the top-
> level MFD device's .of_node field in the cases where it would otherwise
> remain unset.
> 
> The first use-case for this will be aliases for the TPS6586x's RTC
> device.
> 
> Signed-off-by: Stephen Warren <swarren@nvidia.com>
> ---
>  drivers/mfd/mfd-core.c | 2 ++
>  1 file changed, 2 insertions(+)

I've tentatively applied this patch, but if it starts to cause more
problems than it solves we'll have to endeavour to find a different
solution.

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

[PATCH 1/3] mfd: always assign of_node in mfd_add_device()

[Stephen Warren]

On 12/11/2013 02:24 AM, Lee Jones wrote:
>> From: Stephen Warren <swarren@nvidia.com>
>>
>> mfd_add_device() assigns .of_node in the device objects it creates only
>> if the mfd_cell for the device has the .of_compatible field set and the
>> DT node for the top-level MFD device contains a child whose compatible
>> property matches the cell's .of_compatible field.
>>
>> This leaves .of_node unset in many cases. When this happens, entries in
>> the DT /aliases property which refer to the top-level MFD DT node will
>> never match the MFD child devices, hence causing the requested alias not
>> to be honored.
>>
>> Solve this by setting each MFD child device's .of_node equal to the top-
>> level MFD device's .of_node field in the cases where it would otherwise
>> remain unset.
>>
>> The first use-case for this will be aliases for the TPS6586x's RTC
>> device.
>>
>> Signed-off-by: Stephen Warren <swarren@nvidia.com>
>> ---
>>  drivers/mfd/mfd-core.c | 2 ++
>>  1 file changed, 2 insertions(+)
> 
> I've tentatively applied this patch, but if it starts to cause more
> problems than it solves we'll have to endeavour to find a different
> solution.

Thanks. I don't see this in linux-next yet though, so it's not getting
much testing. Did you forget to push your branch?

[PATCH 1/3] mfd: always assign of_node in mfd_add_device()

[Lee Jones]

On Fri, 13 Dec 2013, Stephen Warren wrote:

> On 12/11/2013 02:24 AM, Lee Jones wrote:
> >> From: Stephen Warren <swarren@nvidia.com>
> >>
> >> mfd_add_device() assigns .of_node in the device objects it creates only
> >> if the mfd_cell for the device has the .of_compatible field set and the
> >> DT node for the top-level MFD device contains a child whose compatible
> >> property matches the cell's .of_compatible field.
> >>
> >> This leaves .of_node unset in many cases. When this happens, entries in
> >> the DT /aliases property which refer to the top-level MFD DT node will
> >> never match the MFD child devices, hence causing the requested alias not
> >> to be honored.
> >>
> >> Solve this by setting each MFD child device's .of_node equal to the top-
> >> level MFD device's .of_node field in the cases where it would otherwise
> >> remain unset.
> >>
> >> The first use-case for this will be aliases for the TPS6586x's RTC
> >> device.
> >>
> >> Signed-off-by: Stephen Warren <swarren@nvidia.com>
> >> ---
> >>  drivers/mfd/mfd-core.c | 2 ++
> >>  1 file changed, 2 insertions(+)
> > 
> > I've tentatively applied this patch, but if it starts to cause more
> > problems than it solves we'll have to endeavour to find a different
> > solution.
> 
> Thanks. I don't see this in linux-next yet though, so it's not getting
> much testing. Did you forget to push your branch?

Pushed now, sorry for the delay.

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

[PATCH 1/3] mfd: always assign of_node in mfd_add_device()

[Stephen Warren]

On 12/11/2013 02:24 AM, Lee Jones wrote:
>> From: Stephen Warren <swarren@nvidia.com>
>>
>> mfd_add_device() assigns .of_node in the device objects it creates only
>> if the mfd_cell for the device has the .of_compatible field set and the
>> DT node for the top-level MFD device contains a child whose compatible
>> property matches the cell's .of_compatible field.
>>
>> This leaves .of_node unset in many cases. When this happens, entries in
>> the DT /aliases property which refer to the top-level MFD DT node will
>> never match the MFD child devices, hence causing the requested alias not
>> to be honored.
>>
>> Solve this by setting each MFD child device's .of_node equal to the top-
>> level MFD device's .of_node field in the cases where it would otherwise
>> remain unset.
>>
>> The first use-case for this will be aliases for the TPS6586x's RTC
>> device.
>>
>> Signed-off-by: Stephen Warren <swarren@nvidia.com>
>> ---
>>  drivers/mfd/mfd-core.c | 2 ++
>>  1 file changed, 2 insertions(+)
> 
> I've tentatively applied this patch, but if it starts to cause more
> problems than it solves we'll have to endeavour to find a different
> solution.

OK, we've found a problem already! I guess we should drop or revert this
patch (do you need me to send a patch to do this?) and I'll send a
revised patch to the RTC core to look up aliases in a different way.

For the problem, see:
http://www.spinics.net/lists/arm-kernel/msg295627.html

[PATCH 1/3] mfd: always assign of_node in mfd_add_device()

[Lee Jones]

On Thu, 19 Dec 2013, Stephen Warren wrote:

> On 12/11/2013 02:24 AM, Lee Jones wrote:
> >> From: Stephen Warren <swarren@nvidia.com>
> >>
> >> mfd_add_device() assigns .of_node in the device objects it creates only
> >> if the mfd_cell for the device has the .of_compatible field set and the
> >> DT node for the top-level MFD device contains a child whose compatible
> >> property matches the cell's .of_compatible field.
> >>
> >> This leaves .of_node unset in many cases. When this happens, entries in
> >> the DT /aliases property which refer to the top-level MFD DT node will
> >> never match the MFD child devices, hence causing the requested alias not
> >> to be honored.
> >>
> >> Solve this by setting each MFD child device's .of_node equal to the top-
> >> level MFD device's .of_node field in the cases where it would otherwise
> >> remain unset.
> >>
> >> The first use-case for this will be aliases for the TPS6586x's RTC
> >> device.
> >>
> >> Signed-off-by: Stephen Warren <swarren@nvidia.com>
> >> ---
> >>  drivers/mfd/mfd-core.c | 2 ++
> >>  1 file changed, 2 insertions(+)
> > 
> > I've tentatively applied this patch, but if it starts to cause more
> > problems than it solves we'll have to endeavour to find a different
> > solution.
> 
> OK, we've found a problem already! I guess we should drop or revert this
> patch (do you need me to send a patch to do this?) and I'll send a
> revised patch to the RTC core to look up aliases in a different way.
> 
> For the problem, see:
> http://www.spinics.net/lists/arm-kernel/msg295627.html

Ouch!

No, it's okay, I'll revert it no problem.

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

[PATCH 1/3] mfd: always assign of_node in mfd_add_device()

[Lee Jones]

Hi Stephen,

Please let me know if this is suitable and I can keep your Ack.

----

mfd: Revert "mfd: Always assign of_node in mfd_add_device()"

This reverts commit 68044bee13770918e0b28dd44aa98c889ec7558f.

We've had confirmed reports of this patch causing unforeseen issues
with existing MFD users. It has been agreed by the original author
and myself that reversion is the best solution.

Acked-by: Stephen Warren <swarren@nvidia.com>
Signed-off-by: Lee Jones <lee.jones@linaro.org>

diff --git a/drivers/mfd/mfd-core.c b/drivers/mfd/mfd-core.c
index 32e8d47..2676492 100644
--- a/drivers/mfd/mfd-core.c
+++ b/drivers/mfd/mfd-core.c
@@ -117,8 +117,6 @@ static int mfd_add_device(struct device *parent, int id,
 			}
 		}
 	}
-	if (!pdev->dev.of_node)
-		pdev->dev.of_node = parent->of_node;
 
 	if (cell->pdata_size) {
 		ret = platform_device_add_data(pdev,

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

[PATCH 1/3] mfd: always assign of_node in mfd_add_device()

[Stephen Warren]

On 12/20/2013 07:25 AM, Lee Jones wrote:
> Hi Stephen,
> 
> Please let me know if this is suitable and I can keep your Ack.

Yes, that patch is good. Thanks.

[PATCH 1/3] mfd: always assign of_node in mfd_add_device()

[Stephen Warren]

On 12/20/2013 07:25 AM, Lee Jones wrote:
> Hi Stephen,
> 
> Please let me know if this is suitable and I can keep your Ack.

This patch doesn't seem to have made it into next-20140106. I assume
that's just because of the holidays, not because it was forgotten?

> ----
> 
> mfd: Revert "mfd: Always assign of_node in mfd_add_device()"
> 
> This reverts commit 68044bee13770918e0b28dd44aa98c889ec7558f.
> 
> We've had confirmed reports of this patch causing unforeseen issues
> with existing MFD users. It has been agreed by the original author
> and myself that reversion is the best solution.
> 
> Acked-by: Stephen Warren <swarren@nvidia.com>
> Signed-off-by: Lee Jones <lee.jones@linaro.org>
> 
> diff --git a/drivers/mfd/mfd-core.c b/drivers/mfd/mfd-core.c
> index 32e8d47..2676492 100644
> --- a/drivers/mfd/mfd-core.c
> +++ b/drivers/mfd/mfd-core.c
> @@ -117,8 +117,6 @@ static int mfd_add_device(struct device *parent, int id,
>  			}
>  		}
>  	}
> -	if (!pdev->dev.of_node)
> -		pdev->dev.of_node = parent->of_node;
>  
>  	if (cell->pdata_size) {
>  		ret = platform_device_add_data(pdev,
> 

[PATCH 1/3] mfd: always assign of_node in mfd_add_device()

[Lee Jones]

> > Please let me know if this is suitable and I can keep your Ack.
> 
> This patch doesn't seem to have made it into next-20140106. I assume
> that's just because of the holidays, not because it was forgotten?

Right, it was pushed yesterday morning.

> > ----
> > 
> > mfd: Revert "mfd: Always assign of_node in mfd_add_device()"
> > 
> > This reverts commit 68044bee13770918e0b28dd44aa98c889ec7558f.
> > 
> > We've had confirmed reports of this patch causing unforeseen issues
> > with existing MFD users. It has been agreed by the original author
> > and myself that reversion is the best solution.
> > 
> > Acked-by: Stephen Warren <swarren@nvidia.com>
> > Signed-off-by: Lee Jones <lee.jones@linaro.org>
> > 
> > diff --git a/drivers/mfd/mfd-core.c b/drivers/mfd/mfd-core.c
> > index 32e8d47..2676492 100644
> > --- a/drivers/mfd/mfd-core.c
> > +++ b/drivers/mfd/mfd-core.c
> > @@ -117,8 +117,6 @@ static int mfd_add_device(struct device *parent, int id,
> >  			}
> >  		}
> >  	}
> > -	if (!pdev->dev.of_node)
> > -		pdev->dev.of_node = parent->of_node;
> >  
> >  	if (cell->pdata_size) {
> >  		ret = platform_device_add_data(pdev,
> > 
> 

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