| Message ID | 1410428289-18229-7-git-send-email-javier.martinez@collabora.co.uk (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
On Thu, 11 Sep 2014, Javier Martinez Canillas wrote: > From: Todd Broch <tbroch@chromium.org> > > If the EC device tree node has sub-nodes, try to instantiate them as > MFD sub-devices. We can configure the EC features provided by the board. > > Signed-off-by: Todd Broch <tbroch@chromium.org> > Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk> > --- > > Changes since v2: > - Drop if guards since of_node is unconditionally built. Suggested by Lee Jones > - Drop unneeded comment about of_platform_populate(). Suggested by Lee Jones. > - Fix error message if of_platform_populate() fails. Suggested by Lee Jones. > > Changes since v1: > - Don't leave an empty struct mfd_cell array. Suggested by Lee Jones. > - Just use of_platform_populate() instead of manually iterating through > sub-devices and calling mfd_add_devices. Suggested by Lee Jones. > --- > drivers/mfd/cros_ec.c | 27 ++++++++------------------- > 1 file changed, 8 insertions(+), 19 deletions(-) > > diff --git a/drivers/mfd/cros_ec.c b/drivers/mfd/cros_ec.c > index 751af0b..7c533d2 100644 > --- a/drivers/mfd/cros_ec.c > +++ b/drivers/mfd/cros_ec.c > @@ -21,6 +21,7 @@ > #include <linux/slab.h> > #include <linux/module.h> > #include <linux/mfd/core.h> > +#include <linux/of_platform.h> > #include <linux/mfd/cros_ec.h> > #include <linux/mfd/cros_ec_commands.h> > #include <linux/delay.h> > @@ -109,22 +110,10 @@ int cros_ec_cmd_xfer(struct cros_ec_device *ec_dev, > } > EXPORT_SYMBOL(cros_ec_cmd_xfer); > > -static const struct mfd_cell cros_devs[] = { > - { > - .name = "cros-ec-keyb", > - .id = 1, > - .of_compatible = "google,cros-ec-keyb", > - }, > - { > - .name = "cros-ec-i2c-tunnel", > - .id = 2, > - .of_compatible = "google,cros-ec-i2c-tunnel", > - }, > -}; > - > int cros_ec_register(struct cros_ec_device *ec_dev) > { > struct device *dev = ec_dev->dev; > + struct device_node *node = dev->of_node; > int err = 0; > > if (ec_dev->din_size) { > @@ -140,12 +129,12 @@ int cros_ec_register(struct cros_ec_device *ec_dev) > > mutex_init(&ec_dev->lock); > > - err = mfd_add_devices(dev, 0, cros_devs, > - ARRAY_SIZE(cros_devs), > - NULL, ec_dev->irq, NULL); > - if (err) { > - dev_err(dev, "failed to add mfd devices\n"); > - return err; So these devices will only ever probe with DT now ... > + if (node) { So it would be wrong for dev->of_node not to be populated. > + err = of_platform_populate(node, NULL, NULL, dev); > + if (err) { > + dev_err(dev, "Failed to register subordinate devices"); > + return err; > + } > } > > dev_info(dev, "Chrome EC device registered\n");
Hello Lee, On 09/17/2014 06:31 PM, Lee Jones wrote: >> >> -static const struct mfd_cell cros_devs[] = { >> - { >> - .name = "cros-ec-keyb", >> - .id = 1, >> - .of_compatible = "google,cros-ec-keyb", >> - }, >> - { >> - .name = "cros-ec-i2c-tunnel", >> - .id = 2, >> - .of_compatible = "google,cros-ec-i2c-tunnel", >> - }, >> -}; >> - >> int cros_ec_register(struct cros_ec_device *ec_dev) >> { >> struct device *dev = ec_dev->dev; >> + struct device_node *node = dev->of_node; >> int err = 0; >> >> if (ec_dev->din_size) { >> @@ -140,12 +129,12 @@ int cros_ec_register(struct cros_ec_device *ec_dev) >> >> mutex_init(&ec_dev->lock); >> >> - err = mfd_add_devices(dev, 0, cros_devs, >> - ARRAY_SIZE(cros_devs), >> - NULL, ec_dev->irq, NULL); >> - if (err) { >> - dev_err(dev, "failed to add mfd devices\n"); >> - return err; > > So these devices will only ever probe with DT now ... > Well, these are preparatory patches to reduce the delta between upstream and the downstream so the missing functionality could be added. One of the missing drivers is the cros_ec_dev.c [0] which allows user-space to access the ChromeOS Embedded Controller using a virtual character device (/dev/cros_ec). Since that is a virtual device, it does not fit on the DT which only describes hw and also is used on x86 machines so that subdevice is still probed using mfd_add_devices() and the mfd_cells array is not empty in the downstream cros_ec driver [1]. That's why I didn't just made the cros_ec MFD to depend on OF, since I didn't want to diverge too much from the downstream driver because the idea of the series was to reduce the difference in order to add the missing bits on top. >> + if (node) { > > So it would be wrong for dev->of_node not to be populated. > As explained above, DT, non-DT and x86 platforms instantiate the cros-ec-dev cells but DT platforms can define other child nodes. But I can remove the conditional if you want and reintroduce it once cros-ec-dev support is added. >> + err = of_platform_populate(node, NULL, NULL, dev); >> + if (err) { >> + dev_err(dev, "Failed to register subordinate devices"); >> + return err; >> + } >> } >> >> dev_info(dev, "Chrome EC device registered\n"); > Best regards, Javier [0]: https://chromium.googlesource.com/chromiumos/third_party/kernel/+/chromeos-3.8/drivers/mfd/cros_ec_dev.c [1]: https://chromium.googlesource.com/chromiumos/third_party/kernel/+/chromeos-3.8/drivers/mfd/cros_ec.c#93 -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hello Lee, On 09/18/2014 09:49 AM, Javier Martinez Canillas wrote: >>> int cros_ec_register(struct cros_ec_device *ec_dev) >>> { >>> struct device *dev = ec_dev->dev; >>> + struct device_node *node = dev->of_node; >>> int err = 0; >>> >>> if (ec_dev->din_size) { >>> @@ -140,12 +129,12 @@ int cros_ec_register(struct cros_ec_device *ec_dev) >>> >>> mutex_init(&ec_dev->lock); >>> >>> - err = mfd_add_devices(dev, 0, cros_devs, >>> - ARRAY_SIZE(cros_devs), >>> - NULL, ec_dev->irq, NULL); >>> - if (err) { >>> - dev_err(dev, "failed to add mfd devices\n"); >>> - return err; >> >> So these devices will only ever probe with DT now ... >> > > Well, these are preparatory patches to reduce the delta between upstream and > the downstream so the missing functionality could be added. One of the missing > drivers is the cros_ec_dev.c [0] which allows user-space to access the > ChromeOS Embedded Controller using a virtual character device (/dev/cros_ec). > > Since that is a virtual device, it does not fit on the DT which only describes > hw and also is used on x86 machines so that subdevice is still probed using > mfd_add_devices() and the mfd_cells array is not empty in the downstream > cros_ec driver [1]. > I posted a new revision (v4) of the series but dropped this patch since this change probably makes more sense once the cros_ec_dev driver lands. Since then the mfd cells array will not be empty and not all subdevices will be probed with of_platform_populate(). Best regards, Javier -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, 18 Sep 2014, Javier Martinez Canillas wrote: > Hello Lee, > > On 09/17/2014 06:31 PM, Lee Jones wrote: > >> > >> -static const struct mfd_cell cros_devs[] = { > >> - { > >> - .name = "cros-ec-keyb", > >> - .id = 1, > >> - .of_compatible = "google,cros-ec-keyb", > >> - }, > >> - { > >> - .name = "cros-ec-i2c-tunnel", > >> - .id = 2, > >> - .of_compatible = "google,cros-ec-i2c-tunnel", > >> - }, > >> -}; > >> - > >> int cros_ec_register(struct cros_ec_device *ec_dev) > >> { > >> struct device *dev = ec_dev->dev; > >> + struct device_node *node = dev->of_node; > >> int err = 0; > >> > >> if (ec_dev->din_size) { > >> @@ -140,12 +129,12 @@ int cros_ec_register(struct cros_ec_device *ec_dev) > >> > >> mutex_init(&ec_dev->lock); > >> > >> - err = mfd_add_devices(dev, 0, cros_devs, > >> - ARRAY_SIZE(cros_devs), > >> - NULL, ec_dev->irq, NULL); > >> - if (err) { > >> - dev_err(dev, "failed to add mfd devices\n"); > >> - return err; > > > > So these devices will only ever probe with DT now ... > > > > Well, these are preparatory patches to reduce the delta between upstream and > the downstream so the missing functionality could be added. One of the missing > drivers is the cros_ec_dev.c [0] which allows user-space to access the > ChromeOS Embedded Controller using a virtual character device (/dev/cros_ec). > > Since that is a virtual device, it does not fit on the DT which only describes > hw and also is used on x86 machines so that subdevice is still probed using > mfd_add_devices() and the mfd_cells array is not empty in the downstream > cros_ec driver [1]. > > That's why I didn't just made the cros_ec MFD to depend on OF, since I didn't > want to diverge too much from the downstream driver because the idea of the > series was to reduce the difference in order to add the missing bits on top. > > >> + if (node) { > > > > So it would be wrong for dev->of_node not to be populated. > > > > As explained above, DT, non-DT and x86 platforms instantiate the cros-ec-dev > cells but DT platforms can define other child nodes. But I can remove the > conditional if you want and reintroduce it once cros-ec-dev support is added. Yes, that makes sense. I only care about doing what's right for Mainline, so if it doesn't make sense here, then we shouldn't be doing it. > >> + err = of_platform_populate(node, NULL, NULL, dev); > >> + if (err) { > >> + dev_err(dev, "Failed to register subordinate devices"); > >> + return err; > >> + } > >> } > >> > >> dev_info(dev, "Chrome EC device registered\n"); > > > > Best regards, > Javier > > [0]: > https://chromium.googlesource.com/chromiumos/third_party/kernel/+/chromeos-3.8/drivers/mfd/cros_ec_dev.c > [1]: > https://chromium.googlesource.com/chromiumos/third_party/kernel/+/chromeos-3.8/drivers/mfd/cros_ec.c#93
Hello Lee, On 09/19/2014 02:15 AM, Lee Jones wrote: >> >> As explained above, DT, non-DT and x86 platforms instantiate the cros-ec-dev >> cells but DT platforms can define other child nodes. But I can remove the >> conditional if you want and reintroduce it once cros-ec-dev support is added. > > Yes, that makes sense. I only care about doing what's right for > Mainline, so if it doesn't make sense here, then we shouldn't be doing > it. > Understood, that's why I dropped this since as explained earlier, it makes more sense to include this change once the cros_ec_dev driver is pushed to upstream. The last version of the series (v4) I posted a week ago [0], does not include this patch so I hope the series can be picked for 3.18 since most of the patches were already acked by you. Thanks a lot and best regards, Javier [0]: https://www.mail-archive.com/linux-samsung-soc@vger.kernel.org/msg36770.html -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, 25 Sep 2014, Javier Martinez Canillas wrote: > Hello Lee, > > On 09/19/2014 02:15 AM, Lee Jones wrote: > >> > >> As explained above, DT, non-DT and x86 platforms instantiate the cros-ec-dev > >> cells but DT platforms can define other child nodes. But I can remove the > >> conditional if you want and reintroduce it once cros-ec-dev support is added. > > > > Yes, that makes sense. I only care about doing what's right for > > Mainline, so if it doesn't make sense here, then we shouldn't be doing > > it. > > > > Understood, that's why I dropped this since as explained earlier, it makes more > sense to include this change once the cros_ec_dev driver is pushed to upstream. > > The last version of the series (v4) I posted a week ago [0], does not include > this patch so I hope the series can be picked for 3.18 since most of the patches > were already acked by you. Unfortunately this is going to have to wait until v3.19 now, as Linus is closing the -rcs early. I have subsequently applied my last patch to the MFD tree for v3.18. It will be good for these patches to get some soak testing in -next anyway, rather than rush them in.
On 09/25/2014 04:48 PM, Lee Jones wrote: > On Thu, 25 Sep 2014, Javier Martinez Canillas wrote: >> >> The last version of the series (v4) I posted a week ago [0], does not include >> this patch so I hope the series can be picked for 3.18 since most of the patches >> were already acked by you. > > Unfortunately this is going to have to wait until v3.19 now, as Linus > is closing the -rcs early. I have subsequently applied my last > patch to the MFD tree for v3.18. It will be good for these patches to > get some soak testing in -next anyway, rather than rush them in. > No worries. Does that mean that you are going to pick this series after the 3.18 merge window or should I resend this once 3.18-rc1 is released? Best regards, Javier -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, 25 Sep 2014, Javier Martinez Canillas wrote: > On 09/25/2014 04:48 PM, Lee Jones wrote: > > On Thu, 25 Sep 2014, Javier Martinez Canillas wrote: > >> > >> The last version of the series (v4) I posted a week ago [0], does not include > >> this patch so I hope the series can be picked for 3.18 since most of the patches > >> were already acked by you. > > > > Unfortunately this is going to have to wait until v3.19 now, as Linus > > is closing the -rcs early. I have subsequently applied my last > > patch to the MFD tree for v3.18. It will be good for these patches to > > get some soak testing in -next anyway, rather than rush them in. > > > > No worries. Does that mean that you are going to pick this series after > the 3.18 merge window or should I resend this once 3.18-rc1 is released? If you can re-send with any Acks you've been given (no new ones) after -rc1, that would be helpful.
On 09/26/2014 09:21 AM, Lee Jones wrote: > On Thu, 25 Sep 2014, Javier Martinez Canillas wrote: >> > Unfortunately this is going to have to wait until v3.19 now, as Linus >> > is closing the -rcs early. I have subsequently applied my last >> > patch to the MFD tree for v3.18. It will be good for these patches to >> > get some soak testing in -next anyway, rather than rush them in. >> > >> >> No worries. Does that mean that you are going to pick this series after >> the 3.18 merge window or should I resend this once 3.18-rc1 is released? > > If you can re-send with any Acks you've been given (no new ones) after > -rc1, that would be helpful. > Will do it, thanks a lot for your help. -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hello Lee, On 09/25/2014 04:48 PM, Lee Jones wrote: > On Thu, 25 Sep 2014, Javier Martinez Canillas wrote: >> >> Understood, that's why I dropped this since as explained earlier, it makes more >> sense to include this change once the cros_ec_dev driver is pushed to upstream. >> >> The last version of the series (v4) I posted a week ago [0], does not include >> this patch so I hope the series can be picked for 3.18 since most of the patches >> were already acked by you. > > Unfortunately this is going to have to wait until v3.19 now, as Linus > is closing the -rcs early. I have subsequently applied my last > patch to the MFD tree for v3.18. It will be good for these patches to > get some soak testing in -next anyway, rather than rush them in. > I don't want to be annoying but I was just wondering if now that Linus has delayed the 3.18 merge window for two weeks [1], you will consider picking this series. Most of the patches have been acked by you a couple of weeks ago anyways and I dropped $subject from the last version of the series [2], which was the patch you had issues with. It would be awesome if this can make it to 3.18 so we can start adding the missing functionality and target it to 3.19. Thanks a lot and best regards, Javier [1]: http://thread.gmane.org/gmane.linux.kernel/1797057 [2]: https://www.mail-archive.com/linux-samsung-soc@vger.kernel.org/msg36770.html -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/mfd/cros_ec.c b/drivers/mfd/cros_ec.c index 751af0b..7c533d2 100644 --- a/drivers/mfd/cros_ec.c +++ b/drivers/mfd/cros_ec.c @@ -21,6 +21,7 @@ #include <linux/slab.h> #include <linux/module.h> #include <linux/mfd/core.h> +#include <linux/of_platform.h> #include <linux/mfd/cros_ec.h> #include <linux/mfd/cros_ec_commands.h> #include <linux/delay.h> @@ -109,22 +110,10 @@ int cros_ec_cmd_xfer(struct cros_ec_device *ec_dev, } EXPORT_SYMBOL(cros_ec_cmd_xfer); -static const struct mfd_cell cros_devs[] = { - { - .name = "cros-ec-keyb", - .id = 1, - .of_compatible = "google,cros-ec-keyb", - }, - { - .name = "cros-ec-i2c-tunnel", - .id = 2, - .of_compatible = "google,cros-ec-i2c-tunnel", - }, -}; - int cros_ec_register(struct cros_ec_device *ec_dev) { struct device *dev = ec_dev->dev; + struct device_node *node = dev->of_node; int err = 0; if (ec_dev->din_size) { @@ -140,12 +129,12 @@ int cros_ec_register(struct cros_ec_device *ec_dev) mutex_init(&ec_dev->lock); - err = mfd_add_devices(dev, 0, cros_devs, - ARRAY_SIZE(cros_devs), - NULL, ec_dev->irq, NULL); - if (err) { - dev_err(dev, "failed to add mfd devices\n"); - return err; + if (node) { + err = of_platform_populate(node, NULL, NULL, dev); + if (err) { + dev_err(dev, "Failed to register subordinate devices"); + return err; + } } dev_info(dev, "Chrome EC device registered\n");