| Message ID | 1386626809-6251-1-git-send-email-swarren@wwwdotorg.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
> 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,
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.
> 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.
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?
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.
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
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.
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,