* [PATCH] drivers: pmic: Add sysreset driver to da9063 pmic device @ 2021-09-20 15:48 ` Alexandre Ghiti 2021-09-21 7:23 ` Heinrich Schuchardt 2021-09-23 12:23 ` Jaehoon Chung 0 siblings, 2 replies; 5+ messages in thread From: Alexandre Ghiti @ 2021-09-20 15:48 UTC (permalink / raw) To: Paul Walmsley, Pragnesh Patel, Green Wan, Jaehoon Chung, Heinrich Schuchardt, u-boot Cc: Alexandre Ghiti This pmic device is present on the SiFive Unmatched board and this new driver adds the possibility to reset it. Signed-off-by: Alexandre Ghiti <alexandre.ghiti@canonical.com> --- configs/sifive_unmatched_defconfig | 2 ++ drivers/power/pmic/da9063.c | 49 ++++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+) diff --git a/configs/sifive_unmatched_defconfig b/configs/sifive_unmatched_defconfig index 978818b688..9ab058be39 100644 --- a/configs/sifive_unmatched_defconfig +++ b/configs/sifive_unmatched_defconfig @@ -43,3 +43,5 @@ CONFIG_DM_USB=y CONFIG_USB_XHCI_HCD=y CONFIG_USB_XHCI_PCI=y CONFIG_BOARD_EARLY_INIT_F=y +CONFIG_DM_PMIC=y +CONFIG_DM_PMIC_DA9063=y diff --git a/drivers/power/pmic/da9063.c b/drivers/power/pmic/da9063.c index 25101d18f7..b04879d9c5 100644 --- a/drivers/power/pmic/da9063.c +++ b/drivers/power/pmic/da9063.c @@ -10,6 +10,7 @@ #include <dm.h> #include <i2c.h> #include <log.h> +#include <dm/lists.h> #include <power/pmic.h> #include <power/regulator.h> #include <power/da9063_pmic.h> @@ -87,6 +88,7 @@ static int da9063_bind(struct udevice *dev) { ofnode regulators_node; int children; + int ret; regulators_node = dev_read_subnode(dev, "regulators"); if (!ofnode_valid(regulators_node)) { @@ -101,6 +103,14 @@ static int da9063_bind(struct udevice *dev) if (!children) debug("%s: %s - no child found\n", __func__, dev->name); + if (CONFIG_IS_ENABLED(SYSRESET)) { + ret = device_bind_driver(dev, "da9063-sysreset", + "da9063-sysreset", NULL); + if (ret) + debug("%s: %s - failed to bind sysreset driver\n", + __func__, dev->name); + } + /* Always return success for this device */ return 0; } @@ -129,3 +139,42 @@ U_BOOT_DRIVER(pmic_da9063) = { .probe = da9063_probe, .ops = &da9063_ops, }; + +#ifdef CONFIG_SYSRESET +#include <sysreset.h> + +static int da9063_sysreset_request(struct udevice *dev, enum sysreset_t type) +{ + struct udevice *pmic_dev = dev->parent; + uint ret; + + if (type != SYSRESET_WARM && type != SYSRESET_COLD) + return -EPROTONOSUPPORT; + + ret = pmic_reg_write(pmic_dev, DA9063_REG_PAGE_CON, 0x00); + if (ret < 0) + return ret; + + /* Sets the WAKE_UP bit */ + ret = pmic_reg_write(pmic_dev, DA9063_REG_CONTROL_F, 0x04); + if (ret < 0) + return ret; + + /* Powerdown! */ + ret = pmic_reg_write(pmic_dev, DA9063_REG_CONTROL_A, 0x68); + if (ret < 0) + return ret; + + return -EINPROGRESS; +} + +static struct sysreset_ops da9063_sysreset_ops = { + .request = da9063_sysreset_request, +}; + +U_BOOT_DRIVER(da9063_sysreset) = { + .name = "da9063-sysreset", + .id = UCLASS_SYSRESET, + .ops = &da9063_sysreset_ops, +}; +#endif -- 2.30.2 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] drivers: pmic: Add sysreset driver to da9063 pmic device 2021-09-20 15:48 ` [PATCH] drivers: pmic: Add sysreset driver to da9063 pmic device Alexandre Ghiti @ 2021-09-21 7:23 ` Heinrich Schuchardt 2021-09-22 7:13 ` Alexandre Ghiti 2021-09-23 12:23 ` Jaehoon Chung 1 sibling, 1 reply; 5+ messages in thread From: Heinrich Schuchardt @ 2021-09-21 7:23 UTC (permalink / raw) To: Alexandre Ghiti, Paul Walmsley, Pragnesh Patel, Green Wan, Jaehoon Chung, u-boot On 9/20/21 5:48 PM, Alexandre Ghiti wrote: > This pmic device is present on the SiFive Unmatched board and this > new driver adds the possibility to reset it. > > Signed-off-by: Alexandre Ghiti <alexandre.ghiti@canonical.com> > --- > configs/sifive_unmatched_defconfig | 2 ++ > drivers/power/pmic/da9063.c | 49 ++++++++++++++++++++++++++++++ > 2 files changed, 51 insertions(+) > > diff --git a/configs/sifive_unmatched_defconfig b/configs/sifive_unmatched_defconfig > index 978818b688..9ab058be39 100644 > --- a/configs/sifive_unmatched_defconfig > +++ b/configs/sifive_unmatched_defconfig > @@ -43,3 +43,5 @@ CONFIG_DM_USB=y > CONFIG_USB_XHCI_HCD=y > CONFIG_USB_XHCI_PCI=y > CONFIG_BOARD_EARLY_INIT_F=y > +CONFIG_DM_PMIC=y > +CONFIG_DM_PMIC_DA9063=y > diff --git a/drivers/power/pmic/da9063.c b/drivers/power/pmic/da9063.c > index 25101d18f7..b04879d9c5 100644 > --- a/drivers/power/pmic/da9063.c > +++ b/drivers/power/pmic/da9063.c > @@ -10,6 +10,7 @@ > #include <dm.h> > #include <i2c.h> > #include <log.h> > +#include <dm/lists.h> > #include <power/pmic.h> > #include <power/regulator.h> > #include <power/da9063_pmic.h> > @@ -87,6 +88,7 @@ static int da9063_bind(struct udevice *dev) > { > ofnode regulators_node; > int children; > + int ret; > > regulators_node = dev_read_subnode(dev, "regulators"); > if (!ofnode_valid(regulators_node)) { > @@ -101,6 +103,14 @@ static int da9063_bind(struct udevice *dev) > if (!children) > debug("%s: %s - no child found\n", __func__, dev->name); > > + if (CONFIG_IS_ENABLED(SYSRESET)) { Thank you for addressing the missing reset driver for the SiFive Unmatched board. I imagine some existing or future boards using the DA9063 will have a GPIO for system reset. Should all boards having a DA9063 PMIC implement system reset via the power management IC? We could instead use the devicetree to identify if a board shall use the DA9063 for system reset. > + ret = device_bind_driver(dev, "da9063-sysreset", > + "da9063-sysreset", NULL); > + if (ret) > + debug("%s: %s - failed to bind sysreset driver\n", > + __func__, dev->name); > + } > + > /* Always return success for this device */ > return 0; > } > @@ -129,3 +139,42 @@ U_BOOT_DRIVER(pmic_da9063) = { > .probe = da9063_probe, > .ops = &da9063_ops, > }; > + > +#ifdef CONFIG_SYSRESET The linker will remove functions that are not used automatically. We have tended to not enclose functions in #ifdef but instead allow the compiler to check the code. > +#include <sysreset.h> Please, keep includes at the top of the code. > + Even though this is a static function I would add a Sphinx style comment here. Cf. https://www.kernel.org/doc/html/latest/doc-guide/kernel-doc.html#function-documentation Best regards Heinrich > +static int da9063_sysreset_request(struct udevice *dev, enum sysreset_t type) > +{ > + struct udevice *pmic_dev = dev->parent; > + uint ret; > + > + if (type != SYSRESET_WARM && type != SYSRESET_COLD) > + return -EPROTONOSUPPORT; > + > + ret = pmic_reg_write(pmic_dev, DA9063_REG_PAGE_CON, 0x00); > + if (ret < 0) > + return ret; > + > + /* Sets the WAKE_UP bit */ > + ret = pmic_reg_write(pmic_dev, DA9063_REG_CONTROL_F, 0x04); > + if (ret < 0) > + return ret; > + > + /* Powerdown! */ > + ret = pmic_reg_write(pmic_dev, DA9063_REG_CONTROL_A, 0x68); > + if (ret < 0) > + return ret; > + > + return -EINPROGRESS; > +} > + > +static struct sysreset_ops da9063_sysreset_ops = { > + .request = da9063_sysreset_request, > +}; > + > +U_BOOT_DRIVER(da9063_sysreset) = { > + .name = "da9063-sysreset", > + .id = UCLASS_SYSRESET, > + .ops = &da9063_sysreset_ops, > +}; > +#endif > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] drivers: pmic: Add sysreset driver to da9063 pmic device 2021-09-21 7:23 ` Heinrich Schuchardt @ 2021-09-22 7:13 ` Alexandre Ghiti 0 siblings, 0 replies; 5+ messages in thread From: Alexandre Ghiti @ 2021-09-22 7:13 UTC (permalink / raw) To: Heinrich Schuchardt Cc: Paul Walmsley, Pragnesh Patel, Green Wan, Jaehoon Chung, u-boot On Tue, Sep 21, 2021 at 9:23 AM Heinrich Schuchardt <heinrich.schuchardt@canonical.com> wrote: > > > > On 9/20/21 5:48 PM, Alexandre Ghiti wrote: > > This pmic device is present on the SiFive Unmatched board and this > > new driver adds the possibility to reset it. > > > > Signed-off-by: Alexandre Ghiti <alexandre.ghiti@canonical.com> > > --- > > configs/sifive_unmatched_defconfig | 2 ++ > > drivers/power/pmic/da9063.c | 49 ++++++++++++++++++++++++++++++ > > 2 files changed, 51 insertions(+) > > > > diff --git a/configs/sifive_unmatched_defconfig b/configs/sifive_unmatched_defconfig > > index 978818b688..9ab058be39 100644 > > --- a/configs/sifive_unmatched_defconfig > > +++ b/configs/sifive_unmatched_defconfig > > @@ -43,3 +43,5 @@ CONFIG_DM_USB=y > > CONFIG_USB_XHCI_HCD=y > > CONFIG_USB_XHCI_PCI=y > > CONFIG_BOARD_EARLY_INIT_F=y > > +CONFIG_DM_PMIC=y > > +CONFIG_DM_PMIC_DA9063=y > > diff --git a/drivers/power/pmic/da9063.c b/drivers/power/pmic/da9063.c > > index 25101d18f7..b04879d9c5 100644 > > --- a/drivers/power/pmic/da9063.c > > +++ b/drivers/power/pmic/da9063.c > > @@ -10,6 +10,7 @@ > > #include <dm.h> > > #include <i2c.h> > > #include <log.h> > > +#include <dm/lists.h> > > #include <power/pmic.h> > > #include <power/regulator.h> > > #include <power/da9063_pmic.h> > > @@ -87,6 +88,7 @@ static int da9063_bind(struct udevice *dev) > > { > > ofnode regulators_node; > > int children; > > + int ret; > > > > regulators_node = dev_read_subnode(dev, "regulators"); > > if (!ofnode_valid(regulators_node)) { > > @@ -101,6 +103,14 @@ static int da9063_bind(struct udevice *dev) > > if (!children) > > debug("%s: %s - no child found\n", __func__, dev->name); > > > > + if (CONFIG_IS_ENABLED(SYSRESET)) { > > Thank you for addressing the missing reset driver for the SiFive > Unmatched board. > > I imagine some existing or future boards using the DA9063 will have a > GPIO for system reset. Should all boards having a DA9063 PMIC implement > system reset via the power management IC? > > We could instead use the devicetree to identify if a board shall use the > DA9063 for system reset. Indeed, with this patch, any board with this chip may use this reset handler, I'm not sure in which order the sysreset drivers are called though. We could add a new device like the rtc/watchdog ones, I'll give it a try, that sounds way cleaner. > > > + ret = device_bind_driver(dev, "da9063-sysreset", > > + "da9063-sysreset", NULL); > > + if (ret) > > + debug("%s: %s - failed to bind sysreset driver\n", > > + __func__, dev->name); > > + } > > + > > /* Always return success for this device */ > > return 0; > > } > > @@ -129,3 +139,42 @@ U_BOOT_DRIVER(pmic_da9063) = { > > .probe = da9063_probe, > > .ops = &da9063_ops, > > }; > > + > > +#ifdef CONFIG_SYSRESET > > The linker will remove functions that are not used automatically. We > have tended to not enclose functions in #ifdef but instead allow the > compiler to check the code. > Ok thanks! > > +#include <sysreset.h> > > Please, keep includes at the top of the code. > > > + > > Even though this is a static function I would add a Sphinx style comment > here. Cf. > https://www.kernel.org/doc/html/latest/doc-guide/kernel-doc.html#function-documentation > > Best regards > > Heinrich > > > +static int da9063_sysreset_request(struct udevice *dev, enum sysreset_t type) > > +{ > > + struct udevice *pmic_dev = dev->parent; > > + uint ret; > > + > > + if (type != SYSRESET_WARM && type != SYSRESET_COLD) > > + return -EPROTONOSUPPORT; > > + > > + ret = pmic_reg_write(pmic_dev, DA9063_REG_PAGE_CON, 0x00); > > + if (ret < 0) > > + return ret; > > + > > + /* Sets the WAKE_UP bit */ > > + ret = pmic_reg_write(pmic_dev, DA9063_REG_CONTROL_F, 0x04); > > + if (ret < 0) > > + return ret; > > + > > + /* Powerdown! */ > > + ret = pmic_reg_write(pmic_dev, DA9063_REG_CONTROL_A, 0x68); > > + if (ret < 0) > > + return ret; > > + > > + return -EINPROGRESS; > > +} > > + > > +static struct sysreset_ops da9063_sysreset_ops = { > > + .request = da9063_sysreset_request, > > +}; > > + > > +U_BOOT_DRIVER(da9063_sysreset) = { > > + .name = "da9063-sysreset", > > + .id = UCLASS_SYSRESET, > > + .ops = &da9063_sysreset_ops, > > +}; > > +#endif > > Alex ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] drivers: pmic: Add sysreset driver to da9063 pmic device 2021-09-20 15:48 ` [PATCH] drivers: pmic: Add sysreset driver to da9063 pmic device Alexandre Ghiti 2021-09-21 7:23 ` Heinrich Schuchardt @ 2021-09-23 12:23 ` Jaehoon Chung 2021-09-24 8:45 ` Alexandre Ghiti 1 sibling, 1 reply; 5+ messages in thread From: Jaehoon Chung @ 2021-09-23 12:23 UTC (permalink / raw) To: Alexandre Ghiti, Paul Walmsley, Pragnesh Patel, Green Wan, Heinrich Schuchardt, u-boot Hi Alexandre, On 9/21/21 12:48 AM, Alexandre Ghiti wrote: > This pmic device is present on the SiFive Unmatched board and this > new driver adds the possibility to reset it. Is there any patch before applying this? I cant' apply this from patchwork for checking. If i missed something, let me know, plz. Best Regards, Jaehoon Chung > > Signed-off-by: Alexandre Ghiti <alexandre.ghiti@canonical.com> > --- > configs/sifive_unmatched_defconfig | 2 ++ > drivers/power/pmic/da9063.c | 49 ++++++++++++++++++++++++++++++ > 2 files changed, 51 insertions(+) > > diff --git a/configs/sifive_unmatched_defconfig b/configs/sifive_unmatched_defconfig > index 978818b688..9ab058be39 100644 > --- a/configs/sifive_unmatched_defconfig > +++ b/configs/sifive_unmatched_defconfig > @@ -43,3 +43,5 @@ CONFIG_DM_USB=y > CONFIG_USB_XHCI_HCD=y > CONFIG_USB_XHCI_PCI=y > CONFIG_BOARD_EARLY_INIT_F=y > +CONFIG_DM_PMIC=y > +CONFIG_DM_PMIC_DA9063=y > diff --git a/drivers/power/pmic/da9063.c b/drivers/power/pmic/da9063.c > index 25101d18f7..b04879d9c5 100644 > --- a/drivers/power/pmic/da9063.c > +++ b/drivers/power/pmic/da9063.c > @@ -10,6 +10,7 @@ > #include <dm.h> > #include <i2c.h> > #include <log.h> > +#include <dm/lists.h> > #include <power/pmic.h> > #include <power/regulator.h> > #include <power/da9063_pmic.h> > @@ -87,6 +88,7 @@ static int da9063_bind(struct udevice *dev) > { > ofnode regulators_node; > int children; > + int ret; > > regulators_node = dev_read_subnode(dev, "regulators"); > if (!ofnode_valid(regulators_node)) { > @@ -101,6 +103,14 @@ static int da9063_bind(struct udevice *dev) > if (!children) > debug("%s: %s - no child found\n", __func__, dev->name); > > + if (CONFIG_IS_ENABLED(SYSRESET)) { > + ret = device_bind_driver(dev, "da9063-sysreset", > + "da9063-sysreset", NULL); > + if (ret) > + debug("%s: %s - failed to bind sysreset driver\n", > + __func__, dev->name); > + } > + > /* Always return success for this device */ > return 0; > } > @@ -129,3 +139,42 @@ U_BOOT_DRIVER(pmic_da9063) = { > .probe = da9063_probe, > .ops = &da9063_ops, > }; > + > +#ifdef CONFIG_SYSRESET > +#include <sysreset.h> > + > +static int da9063_sysreset_request(struct udevice *dev, enum sysreset_t type) > +{ > + struct udevice *pmic_dev = dev->parent; > + uint ret; > + > + if (type != SYSRESET_WARM && type != SYSRESET_COLD) > + return -EPROTONOSUPPORT; > + > + ret = pmic_reg_write(pmic_dev, DA9063_REG_PAGE_CON, 0x00); > + if (ret < 0) > + return ret; > + > + /* Sets the WAKE_UP bit */ > + ret = pmic_reg_write(pmic_dev, DA9063_REG_CONTROL_F, 0x04); > + if (ret < 0) > + return ret; > + > + /* Powerdown! */ > + ret = pmic_reg_write(pmic_dev, DA9063_REG_CONTROL_A, 0x68); > + if (ret < 0) > + return ret; > + > + return -EINPROGRESS; > +} > + > +static struct sysreset_ops da9063_sysreset_ops = { > + .request = da9063_sysreset_request, > +}; > + > +U_BOOT_DRIVER(da9063_sysreset) = { > + .name = "da9063-sysreset", > + .id = UCLASS_SYSRESET, > + .ops = &da9063_sysreset_ops, > +}; > +#endif > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] drivers: pmic: Add sysreset driver to da9063 pmic device 2021-09-23 12:23 ` Jaehoon Chung @ 2021-09-24 8:45 ` Alexandre Ghiti 0 siblings, 0 replies; 5+ messages in thread From: Alexandre Ghiti @ 2021-09-24 8:45 UTC (permalink / raw) To: Jaehoon Chung Cc: Paul Walmsley, Pragnesh Patel, Green Wan, Heinrich Schuchardt, u-boot Hi Jaehoon, On Thu, Sep 23, 2021 at 2:23 PM Jaehoon Chung <jh80.chung@samsung.com> wrote: > > Hi Alexandre, > > On 9/21/21 12:48 AM, Alexandre Ghiti wrote: > > This pmic device is present on the SiFive Unmatched board and this > > new driver adds the possibility to reset it. > > Is there any patch before applying this? > I cant' apply this from patchwork for checking. > If i missed something, let me know, plz. Sorry about this, I was working on another tree than master. Please see the v2 of this patchset which fixes that and takes into account Heinrich suggestion. Thanks, Alex > > Best Regards, > Jaehoon Chung > > > > > Signed-off-by: Alexandre Ghiti <alexandre.ghiti@canonical.com> > > --- > > configs/sifive_unmatched_defconfig | 2 ++ > > drivers/power/pmic/da9063.c | 49 ++++++++++++++++++++++++++++++ > > 2 files changed, 51 insertions(+) > > > > diff --git a/configs/sifive_unmatched_defconfig b/configs/sifive_unmatched_defconfig > > index 978818b688..9ab058be39 100644 > > --- a/configs/sifive_unmatched_defconfig > > +++ b/configs/sifive_unmatched_defconfig > > @@ -43,3 +43,5 @@ CONFIG_DM_USB=y > > CONFIG_USB_XHCI_HCD=y > > CONFIG_USB_XHCI_PCI=y > > CONFIG_BOARD_EARLY_INIT_F=y > > +CONFIG_DM_PMIC=y > > +CONFIG_DM_PMIC_DA9063=y > > diff --git a/drivers/power/pmic/da9063.c b/drivers/power/pmic/da9063.c > > index 25101d18f7..b04879d9c5 100644 > > --- a/drivers/power/pmic/da9063.c > > +++ b/drivers/power/pmic/da9063.c > > @@ -10,6 +10,7 @@ > > #include <dm.h> > > #include <i2c.h> > > #include <log.h> > > +#include <dm/lists.h> > > #include <power/pmic.h> > > #include <power/regulator.h> > > #include <power/da9063_pmic.h> > > @@ -87,6 +88,7 @@ static int da9063_bind(struct udevice *dev) > > { > > ofnode regulators_node; > > int children; > > + int ret; > > > > regulators_node = dev_read_subnode(dev, "regulators"); > > if (!ofnode_valid(regulators_node)) { > > @@ -101,6 +103,14 @@ static int da9063_bind(struct udevice *dev) > > if (!children) > > debug("%s: %s - no child found\n", __func__, dev->name); > > > > + if (CONFIG_IS_ENABLED(SYSRESET)) { > > + ret = device_bind_driver(dev, "da9063-sysreset", > > + "da9063-sysreset", NULL); > > + if (ret) > > + debug("%s: %s - failed to bind sysreset driver\n", > > + __func__, dev->name); > > + } > > + > > /* Always return success for this device */ > > return 0; > > } > > @@ -129,3 +139,42 @@ U_BOOT_DRIVER(pmic_da9063) = { > > .probe = da9063_probe, > > .ops = &da9063_ops, > > }; > > + > > +#ifdef CONFIG_SYSRESET > > +#include <sysreset.h> > > + > > +static int da9063_sysreset_request(struct udevice *dev, enum sysreset_t type) > > +{ > > + struct udevice *pmic_dev = dev->parent; > > + uint ret; > > + > > + if (type != SYSRESET_WARM && type != SYSRESET_COLD) > > + return -EPROTONOSUPPORT; > > + > > + ret = pmic_reg_write(pmic_dev, DA9063_REG_PAGE_CON, 0x00); > > + if (ret < 0) > > + return ret; > > + > > + /* Sets the WAKE_UP bit */ > > + ret = pmic_reg_write(pmic_dev, DA9063_REG_CONTROL_F, 0x04); > > + if (ret < 0) > > + return ret; > > + > > + /* Powerdown! */ > > + ret = pmic_reg_write(pmic_dev, DA9063_REG_CONTROL_A, 0x68); > > + if (ret < 0) > > + return ret; > > + > > + return -EINPROGRESS; > > +} > > + > > +static struct sysreset_ops da9063_sysreset_ops = { > > + .request = da9063_sysreset_request, > > +}; > > + > > +U_BOOT_DRIVER(da9063_sysreset) = { > > + .name = "da9063-sysreset", > > + .id = UCLASS_SYSRESET, > > + .ops = &da9063_sysreset_ops, > > +}; > > +#endif > > > ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-09-24 8:45 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <CGME20210920154820epcas1p3088b204ad0e5fb1d203affd867be2835@epcas1p3.samsung.com>
2021-09-20 15:48 ` [PATCH] drivers: pmic: Add sysreset driver to da9063 pmic device Alexandre Ghiti
2021-09-21 7:23 ` Heinrich Schuchardt
2021-09-22 7:13 ` Alexandre Ghiti
2021-09-23 12:23 ` Jaehoon Chung
2021-09-24 8:45 ` Alexandre Ghiti
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.