Re: [PATCH v2 1/2] driver: power: add support for TPS65224
Hello Jaehoon,
> > Hi Bhargav,
> >
> > On Mon, 6 Nov 2023 at 10:11, Bhargav Raviprakash wrote:
> > >
> > > Added support for PMIC TPS65224. Includes driver for pmic,
> > > and disabling Watchdog.
> > >
> > > Signed-off-by: Bhargav Raviprakash
> > > ---
> > > drivers/power/pmic/Kconfig| 6 ++
> > > drivers/power/pmic/Makefile | 1 +
> > > drivers/power/pmic/tps65224.c | 141 ++
> > > include/power/tps65224.h | 57 ++
> > > 4 files changed, 205 insertions(+)
> > > create mode 100644 drivers/power/pmic/tps65224.c
> > > create mode 100644 include/power/tps65224.h
> > >
> > > diff --git a/drivers/power/pmic/Kconfig b/drivers/power/pmic/Kconfig
> > > index 4a6f0ce093..b06bd31823 100644
> > > --- a/drivers/power/pmic/Kconfig
> > > +++ b/drivers/power/pmic/Kconfig
> > > @@ -378,6 +378,12 @@ config PMIC_TPS65941
> > > The TPS65941 is a PMIC containing a bunch of SMPS & LDOs.
> > > This driver binds the pmic children.
> > >
> > > +config PMIC_TPS65224
> > > + bool "Enable driver for Texas Instruments TPS65224 PMIC"
> > > + help
> > > + The TPS65224 is a PMIC containing a bunch of SMPS & LDOs.
> > > + This driver binds the pmic children.
> > > +
> > > config PMIC_TPS65219
> > > bool "Enable driver for Texas Instruments TPS65219 PMIC"
> > > depends on DM_PMIC
> > > diff --git a/drivers/power/pmic/Makefile b/drivers/power/pmic/Makefile
> > > index 0b3b3d62d0..cec16e57d3 100644
> > > --- a/drivers/power/pmic/Makefile
> > > +++ b/drivers/power/pmic/Makefile
> > > @@ -33,6 +33,7 @@ obj-$(CONFIG_PMIC_STPMIC1) += stpmic1.o
> > > obj-$(CONFIG_PMIC_TPS65217) += pmic_tps65217.o
> > > obj-$(CONFIG_PMIC_TPS65219) += tps65219.o
> > > obj-$(CONFIG_PMIC_TPS65941) += tps65941.o
> > > +obj-$(CONFIG_PMIC_TPS65224) += tps65224.o
> > > obj-$(CONFIG_POWER_TPS65218) += pmic_tps65218.o
> > >
> > > ifeq ($(CONFIG_$(SPL_)POWER_LEGACY),y)
> > > diff --git a/drivers/power/pmic/tps65224.c b/drivers/power/pmic/tps65224.c
> > > new file mode 100644
> > > index 00..33395f6edf
> > > --- /dev/null
> > > +++ b/drivers/power/pmic/tps65224.c
> > > @@ -0,0 +1,141 @@
> > > +// SPDX-License-Identifier: GPL-2.0+
> > > +/*
> > > + * (C) Copyright 2023 Texas Instruments Incorporated,
> > > + */
> > > +#include
> > > +#include
> > > +#include
> > > +#include
> > > +#include
> > > +#include
> > > +#include
> > > +#include
> > > +#include
> > > +
> > > +static const struct pmic_child_info pmic_children_info[] = {
> > > + { .prefix = "ldo", .driver = TPS65224_LDO_DRIVER },
> > > + { .prefix = "buck", .driver = TPS65224_BUCK_DRIVER },
> > > + { },
> > > +};
> > > +
> > > +static int tps65224_write(struct udevice *dev, uint reg, const uint8_t
> > > *buff,
> > > + int len)
> > > +{
> > > + if (dm_i2c_write(dev, reg, buff, len)) {
> > > + pr_err("write error to device: %p register: %#x!\n", dev,
> > > reg);
> > > + return -EIO;
> > > + }
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static int tps65224_read(struct udevice *dev, uint reg, uint8_t *buff,
> > > int len)
> > > +{
> > > + if (dm_i2c_read(dev, reg, buff, len)) {
> > > + pr_err("read error from device: %p register: %#x!\n",
> > > dev, reg);
> > > + return -EIO;
> > > + }
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static int tps65224_bind(struct udevice *dev)
> > > +{
> > > + ofnode regulators_node;
> > > + int children;
> > > +
> > > + if (dev->driver_data == TPS65224_WD)
> > > + return 0;
> > > +
> > > + regulators_node = dev_read_subnode(dev, "regulators");
> > > + if (!ofnode_valid(regulators_node)) {
> > > + debug("%s: %s regulators subnode not found!\n", __func__,
> > > + dev->name);
> > > + return -ENXIO;
> > > + }
> > > +
> > > + debug("%s: '%s' - found regulators subnode\n", __func__,
> > > dev->name);
> > > +
> > > + children = pmic_bind_children(dev, regulators_node,
> > > pmic_children_info);
> > > + if (!children)
> > > + printf("%s: %s - no child found\n", __func__, dev->name);
> > > +
> > > + /* Probe all the child devices */
> >
> > bind, not probe
> >
> > > + return dm_scan_fdt_dev(dev);
> > > +}
> > > +
> > > +static int stop_watchdog(struct udevice *wd_i2c_dev)
> > > +{
> > > + int ret;
> > > +
> > > + /* Maintain WD long window */
> > > + ret = dm_i2c_reg_read(wd_i2c_dev, TPS65224_WD_MODE_REG);
> > > + if (ret < 0) {
> > > + debug("failed to read i2c reg (%d)\n", ret);
> > > + return ret;
> > > + }
> > > +
> > > + ret &= ~TPS65224_WD_PWRHOLD_MASK;
> > > + ret |= TPS65224_WD_PWRHOLD_MASK;
> > > + ret = dm_i2c_reg_write(wd_i2c_dev, TPS65224_WD_MODE_REG, ret);
> > > + if (ret)
Re: [PATCH v2 1/2] driver: power: add support for TPS65224
> Hi Bhargav,
>
> On Mon, 6 Nov 2023 at 10:11, Bhargav Raviprakash wrote:
> >
> > Added support for PMIC TPS65224. Includes driver for pmic,
> > and disabling Watchdog.
> >
> > Signed-off-by: Bhargav Raviprakash
> > ---
> > drivers/power/pmic/Kconfig| 6 ++
> > drivers/power/pmic/Makefile | 1 +
> > drivers/power/pmic/tps65224.c | 141 ++
> > include/power/tps65224.h | 57 ++
> > 4 files changed, 205 insertions(+)
> > create mode 100644 drivers/power/pmic/tps65224.c
> > create mode 100644 include/power/tps65224.h
> >
> > diff --git a/drivers/power/pmic/Kconfig b/drivers/power/pmic/Kconfig
> > index 4a6f0ce093..b06bd31823 100644
> > --- a/drivers/power/pmic/Kconfig
> > +++ b/drivers/power/pmic/Kconfig
> > @@ -378,6 +378,12 @@ config PMIC_TPS65941
> > The TPS65941 is a PMIC containing a bunch of SMPS & LDOs.
> > This driver binds the pmic children.
> >
> > +config PMIC_TPS65224
> > + bool "Enable driver for Texas Instruments TPS65224 PMIC"
> > + help
> > + The TPS65224 is a PMIC containing a bunch of SMPS & LDOs.
> > + This driver binds the pmic children.
> > +
> > config PMIC_TPS65219
> > bool "Enable driver for Texas Instruments TPS65219 PMIC"
> > depends on DM_PMIC
> > diff --git a/drivers/power/pmic/Makefile b/drivers/power/pmic/Makefile
> > index 0b3b3d62d0..cec16e57d3 100644
> > --- a/drivers/power/pmic/Makefile
> > +++ b/drivers/power/pmic/Makefile
> > @@ -33,6 +33,7 @@ obj-$(CONFIG_PMIC_STPMIC1) += stpmic1.o
> > obj-$(CONFIG_PMIC_TPS65217) += pmic_tps65217.o
> > obj-$(CONFIG_PMIC_TPS65219) += tps65219.o
> > obj-$(CONFIG_PMIC_TPS65941) += tps65941.o
> > +obj-$(CONFIG_PMIC_TPS65224) += tps65224.o
> > obj-$(CONFIG_POWER_TPS65218) += pmic_tps65218.o
> >
> > ifeq ($(CONFIG_$(SPL_)POWER_LEGACY),y)
> > diff --git a/drivers/power/pmic/tps65224.c b/drivers/power/pmic/tps65224.c
> > new file mode 100644
> > index 00..33395f6edf
> > --- /dev/null
> > +++ b/drivers/power/pmic/tps65224.c
> > @@ -0,0 +1,141 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * (C) Copyright 2023 Texas Instruments Incorporated,
> > + */
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +
> > +static const struct pmic_child_info pmic_children_info[] = {
> > + { .prefix = "ldo", .driver = TPS65224_LDO_DRIVER },
> > + { .prefix = "buck", .driver = TPS65224_BUCK_DRIVER },
> > + { },
> > +};
> > +
> > +static int tps65224_write(struct udevice *dev, uint reg, const uint8_t
> > *buff,
> > + int len)
> > +{
> > + if (dm_i2c_write(dev, reg, buff, len)) {
> > + pr_err("write error to device: %p register: %#x!\n", dev,
> > reg);
> > + return -EIO;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static int tps65224_read(struct udevice *dev, uint reg, uint8_t *buff, int
> > len)
> > +{
> > + if (dm_i2c_read(dev, reg, buff, len)) {
> > + pr_err("read error from device: %p register: %#x!\n", dev,
> > reg);
> > + return -EIO;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static int tps65224_bind(struct udevice *dev)
> > +{
> > + ofnode regulators_node;
> > + int children;
> > +
> > + if (dev->driver_data == TPS65224_WD)
> > + return 0;
> > +
> > + regulators_node = dev_read_subnode(dev, "regulators");
> > + if (!ofnode_valid(regulators_node)) {
> > + debug("%s: %s regulators subnode not found!\n", __func__,
> > + dev->name);
> > + return -ENXIO;
> > + }
> > +
> > + debug("%s: '%s' - found regulators subnode\n", __func__, dev->name);
> > +
> > + children = pmic_bind_children(dev, regulators_node,
> > pmic_children_info);
> > + if (!children)
> > + printf("%s: %s - no child found\n", __func__, dev->name);
> > +
> > + /* Probe all the child devices */
>
> bind, not probe
>
> > + return dm_scan_fdt_dev(dev);
> > +}
> > +
> > +static int stop_watchdog(struct udevice *wd_i2c_dev)
> > +{
> > + int ret;
> > +
> > + /* Maintain WD long window */
> > + ret = dm_i2c_reg_read(wd_i2c_dev, TPS65224_WD_MODE_REG);
> > + if (ret < 0) {
> > + debug("failed to read i2c reg (%d)\n", ret);
> > + return ret;
> > + }
> > +
> > + ret &= ~TPS65224_WD_PWRHOLD_MASK;
> > + ret |= TPS65224_WD_PWRHOLD_MASK;
> > + ret = dm_i2c_reg_write(wd_i2c_dev, TPS65224_WD_MODE_REG, ret);
> > + if (ret) {
> > + debug("%s: %s write WD_PWRHOLD fail!\n", __func__,
> > wd_i2c_dev->name);
> > + return ret;
> > + }
> > +
> > + ret = dm_i2c_reg_read(wd_i2c_dev, TPS65224_WD_MODE_REG);
> > + if (ret < 0) {
> > + debug("failed to read back i2c reg (%d)
Re: [PATCH v2 1/2] driver: power: add support for TPS65224
> Hi Bhargav,
>
> > -Original Message-
> > From: Bhargav Raviprakash
> > Sent: Monday, November 6, 2023 11:07 PM
> > To: u-boot@lists.denx.de
> > Cc: jh80.ch...@samsung.com; Bhargav Raviprakash
> > Subject: [PATCH v2 1/2] driver: power: add support for TPS65224
> >
> > Added support for PMIC TPS65224. Includes driver for pmic,
> > and disabling Watchdog.
> >
> > Signed-off-by: Bhargav Raviprakash
> > ---
> > drivers/power/pmic/Kconfig| 6 ++
> > drivers/power/pmic/Makefile | 1 +
> > drivers/power/pmic/tps65224.c | 141 ++
> > include/power/tps65224.h | 57 ++
> > 4 files changed, 205 insertions(+)
> > create mode 100644 drivers/power/pmic/tps65224.c
> > create mode 100644 include/power/tps65224.h
> >
> > diff --git a/drivers/power/pmic/Kconfig b/drivers/power/pmic/Kconfig
> > index 4a6f0ce093..b06bd31823 100644
> > --- a/drivers/power/pmic/Kconfig
> > +++ b/drivers/power/pmic/Kconfig
> > @@ -378,6 +378,12 @@ config PMIC_TPS65941
> > The TPS65941 is a PMIC containing a bunch of SMPS & LDOs.
> > This driver binds the pmic children.
> >
> > +config PMIC_TPS65224
> > + bool "Enable driver for Texas Instruments TPS65224 PMIC"
> > + help
> > + The TPS65224 is a PMIC containing a bunch of SMPS & LDOs.
> > + This driver binds the pmic children.
> > +
> > config PMIC_TPS65219
> > bool "Enable driver for Texas Instruments TPS65219 PMIC"
> > depends on DM_PMIC
> > diff --git a/drivers/power/pmic/Makefile b/drivers/power/pmic/Makefile
> > index 0b3b3d62d0..cec16e57d3 100644
> > --- a/drivers/power/pmic/Makefile
> > +++ b/drivers/power/pmic/Makefile
> > @@ -33,6 +33,7 @@ obj-$(CONFIG_PMIC_STPMIC1) += stpmic1.o
> > obj-$(CONFIG_PMIC_TPS65217) += pmic_tps65217.o
> > obj-$(CONFIG_PMIC_TPS65219) += tps65219.o
> > obj-$(CONFIG_PMIC_TPS65941) += tps65941.o
> > +obj-$(CONFIG_PMIC_TPS65224) += tps65224.o
>
> Ordering this. Maybe it can be located at tps65941.o.
>
Sure, will do!
> > obj-$(CONFIG_POWER_TPS65218) += pmic_tps65218.o
> >
> > ifeq ($(CONFIG_$(SPL_)POWER_LEGACY),y)
> > diff --git a/drivers/power/pmic/tps65224.c b/drivers/power/pmic/tps65224.c
> > new file mode 100644
> > index 00..33395f6edf
> > --- /dev/null
> > +++ b/drivers/power/pmic/tps65224.c
> > @@ -0,0 +1,141 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * (C) Copyright 2023 Texas Instruments Incorporated,
> > + */
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +
> > +static const struct pmic_child_info pmic_children_info[] = {
> > + { .prefix = "ldo", .driver = TPS65224_LDO_DRIVER },
> > + { .prefix = "buck", .driver = TPS65224_BUCK_DRIVER },
> > + { },
> > +};
> > +
> > +static int tps65224_write(struct udevice *dev, uint reg, const uint8_t
> > *buff,
> > + int len)
> > +{
> > + if (dm_i2c_write(dev, reg, buff, len)) {
> > + pr_err("write error to device: %p register: %#x!\n", dev, reg);
> > + return -EIO;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static int tps65224_read(struct udevice *dev, uint reg, uint8_t *buff, int
> > len)
> > +{
> > + if (dm_i2c_read(dev, reg, buff, len)) {
> > + pr_err("read error from device: %p register: %#x!\n", dev, reg);
> > + return -EIO;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static int tps65224_bind(struct udevice *dev)
> > +{
> > + ofnode regulators_node;
> > + int children;
> > +
> > + if (dev->driver_data == TPS65224_WD)
> > + return 0;
> > +
> > + regulators_node = dev_read_subnode(dev, "regulators");
> > + if (!ofnode_valid(regulators_node)) {
> > + debug("%s: %s regulators subnode not found!\n", __func__,
> > + dev->name);
> > + return -ENXIO;
> > + }
> > +
> > + debug("%s: '%s' - found regulators subnode\n", __func__, dev->name);
> > +
> > + children = pmic_bind_children(dev, regulators_node, pmic_children_info);
> > + if (!children)
> > + printf("%s: %s - no child found\n", __func__, dev->name);
> > +
> > + /* Probe all the child devices */
> > + return dm_scan_fdt_dev(dev);
> > +}
> > +
> > +static int stop_watchdog(struct udevice *wd_i2c_dev)
> > +{
> > + int ret;
> > +
> > + /* Maintain WD long window */
> > + ret = dm_i2c_reg_read(wd_i2c_dev, TPS65224_WD_MODE_REG);
> > + if (ret < 0) {
> > + debug("failed to read i2c reg (%d)\n", ret);
> > + return ret;
> > + }
> > +
> > + ret &= ~TPS65224_WD_PWRHOLD_MASK;
> > + ret |= TPS65224_WD_PWRHOLD_MASK;
>
> Is it a right behavior? After cleared its bit, set again?
> Any reason to do this?
>
> How about using val and ret as variable to clarify?
>
Using two variables val and ret will help in readability. Will work on it and
repsin.
Thanks for the suggestion!
>
> > + ret = dm_i2c_reg_write(wd_i2c_dev, TPS65224_WD_MODE_REG, ret);
> > +
Re: [PATCH v2 1/2] driver: power: add support for TPS65224
> Hi Bhargav,
>
> On Mon, 6 Nov 2023 at 10:11, Bhargav Raviprakash wrote:
> >
> > Added support for PMIC TPS65224. Includes driver for pmic,
> > and disabling Watchdog.
> >
> > Signed-off-by: Bhargav Raviprakash
> > ---
> > drivers/power/pmic/Kconfig| 6 ++
> > drivers/power/pmic/Makefile | 1 +
> > drivers/power/pmic/tps65224.c | 141 ++
> > include/power/tps65224.h | 57 ++
> > 4 files changed, 205 insertions(+)
> > create mode 100644 drivers/power/pmic/tps65224.c
> > create mode 100644 include/power/tps65224.h
> >
> > diff --git a/drivers/power/pmic/Kconfig b/drivers/power/pmic/Kconfig
> > index 4a6f0ce093..b06bd31823 100644
> > --- a/drivers/power/pmic/Kconfig
> > +++ b/drivers/power/pmic/Kconfig
> > @@ -378,6 +378,12 @@ config PMIC_TPS65941
> > The TPS65941 is a PMIC containing a bunch of SMPS & LDOs.
> > This driver binds the pmic children.
> >
> > +config PMIC_TPS65224
> > + bool "Enable driver for Texas Instruments TPS65224 PMIC"
> > + help
> > + The TPS65224 is a PMIC containing a bunch of SMPS & LDOs.
> > + This driver binds the pmic children.
> > +
> > config PMIC_TPS65219
> > bool "Enable driver for Texas Instruments TPS65219 PMIC"
> > depends on DM_PMIC
> > diff --git a/drivers/power/pmic/Makefile b/drivers/power/pmic/Makefile
> > index 0b3b3d62d0..cec16e57d3 100644
> > --- a/drivers/power/pmic/Makefile
> > +++ b/drivers/power/pmic/Makefile
> > @@ -33,6 +33,7 @@ obj-$(CONFIG_PMIC_STPMIC1) += stpmic1.o
> > obj-$(CONFIG_PMIC_TPS65217) += pmic_tps65217.o
> > obj-$(CONFIG_PMIC_TPS65219) += tps65219.o
> > obj-$(CONFIG_PMIC_TPS65941) += tps65941.o
> > +obj-$(CONFIG_PMIC_TPS65224) += tps65224.o
> > obj-$(CONFIG_POWER_TPS65218) += pmic_tps65218.o
> >
> > ifeq ($(CONFIG_$(SPL_)POWER_LEGACY),y)
> > diff --git a/drivers/power/pmic/tps65224.c b/drivers/power/pmic/tps65224.c
> > new file mode 100644
> > index 00..33395f6edf
> > --- /dev/null
> > +++ b/drivers/power/pmic/tps65224.c
> > @@ -0,0 +1,141 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * (C) Copyright 2023 Texas Instruments Incorporated,
> > + */
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +
> > +static const struct pmic_child_info pmic_children_info[] = {
> > + { .prefix = "ldo", .driver = TPS65224_LDO_DRIVER },
> > + { .prefix = "buck", .driver = TPS65224_BUCK_DRIVER },
> > + { },
> > +};
> > +
> > +static int tps65224_write(struct udevice *dev, uint reg, const uint8_t
> > *buff,
> > + int len)
> > +{
> > + if (dm_i2c_write(dev, reg, buff, len)) {
> > + pr_err("write error to device: %p register: %#x!\n", dev,
> > reg);
> > + return -EIO;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static int tps65224_read(struct udevice *dev, uint reg, uint8_t *buff, int
> > len)
> > +{
> > + if (dm_i2c_read(dev, reg, buff, len)) {
> > + pr_err("read error from device: %p register: %#x!\n", dev,
> > reg);
> > + return -EIO;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static int tps65224_bind(struct udevice *dev)
> > +{
> > + ofnode regulators_node;
> > + int children;
> > +
> > + if (dev->driver_data == TPS65224_WD)
> > + return 0;
> > +
> > + regulators_node = dev_read_subnode(dev, "regulators");
> > + if (!ofnode_valid(regulators_node)) {
> > + debug("%s: %s regulators subnode not found!\n", __func__,
> > + dev->name);
> > + return -ENXIO;
> > + }
> > +
> > + debug("%s: '%s' - found regulators subnode\n", __func__, dev->name);
> > +
> > + children = pmic_bind_children(dev, regulators_node,
> > pmic_children_info);
> > + if (!children)
> > + printf("%s: %s - no child found\n", __func__, dev->name);
> > +
> > + /* Probe all the child devices */
>
> bind, not probe
>
> > + return dm_scan_fdt_dev(dev);
> > +}
> > +
> > +static int stop_watchdog(struct udevice *wd_i2c_dev)
> > +{
> > + int ret;
> > +
> > + /* Maintain WD long window */
> > + ret = dm_i2c_reg_read(wd_i2c_dev, TPS65224_WD_MODE_REG);
> > + if (ret < 0) {
> > + debug("failed to read i2c reg (%d)\n", ret);
> > + return ret;
> > + }
> > +
> > + ret &= ~TPS65224_WD_PWRHOLD_MASK;
> > + ret |= TPS65224_WD_PWRHOLD_MASK;
> > + ret = dm_i2c_reg_write(wd_i2c_dev, TPS65224_WD_MODE_REG, ret);
> > + if (ret) {
> > + debug("%s: %s write WD_PWRHOLD fail!\n", __func__,
> > wd_i2c_dev->name);
> > + return ret;
> > + }
> > +
> > + ret = dm_i2c_reg_read(wd_i2c_dev, TPS65224_WD_MODE_REG);
> > + if (ret < 0) {
> > + debug("failed to read back i2c reg (%d)
RE: [PATCH v2 1/2] driver: power: add support for TPS65224
> -Original Message-
> From: U-Boot On Behalf Of Jaehoon Chung
> Sent: Tuesday, November 7, 2023 3:42 PM
> To: 'Bhargav Raviprakash' ; u-boot@lists.denx.de
> Subject: RE: [PATCH v2 1/2] driver: power: add support for TPS65224
>
> Hi Bhargav,
>
> > -Original Message-
> > From: Bhargav Raviprakash
> > Sent: Monday, November 6, 2023 11:07 PM
> > To: u-boot@lists.denx.de
> > Cc: jh80.ch...@samsung.com; Bhargav Raviprakash
> > Subject: [PATCH v2 1/2] driver: power: add support for TPS65224
> >
> > Added support for PMIC TPS65224. Includes driver for pmic,
> > and disabling Watchdog.
> >
> > Signed-off-by: Bhargav Raviprakash
> > ---
> > drivers/power/pmic/Kconfig| 6 ++
> > drivers/power/pmic/Makefile | 1 +
> > drivers/power/pmic/tps65224.c | 141 ++
> > include/power/tps65224.h | 57 ++
> > 4 files changed, 205 insertions(+)
> > create mode 100644 drivers/power/pmic/tps65224.c
> > create mode 100644 include/power/tps65224.h
> >
> > diff --git a/drivers/power/pmic/Kconfig b/drivers/power/pmic/Kconfig
> > index 4a6f0ce093..b06bd31823 100644
> > --- a/drivers/power/pmic/Kconfig
> > +++ b/drivers/power/pmic/Kconfig
> > @@ -378,6 +378,12 @@ config PMIC_TPS65941
> > The TPS65941 is a PMIC containing a bunch of SMPS & LDOs.
> > This driver binds the pmic children.
> >
> > +config PMIC_TPS65224
> > + bool "Enable driver for Texas Instruments TPS65224 PMIC"
> > + help
> > + The TPS65224 is a PMIC containing a bunch of SMPS & LDOs.
> > + This driver binds the pmic children.
> > +
> > config PMIC_TPS65219
> > bool "Enable driver for Texas Instruments TPS65219 PMIC"
> > depends on DM_PMIC
> > diff --git a/drivers/power/pmic/Makefile b/drivers/power/pmic/Makefile
> > index 0b3b3d62d0..cec16e57d3 100644
> > --- a/drivers/power/pmic/Makefile
> > +++ b/drivers/power/pmic/Makefile
> > @@ -33,6 +33,7 @@ obj-$(CONFIG_PMIC_STPMIC1) += stpmic1.o
> > obj-$(CONFIG_PMIC_TPS65217) += pmic_tps65217.o
> > obj-$(CONFIG_PMIC_TPS65219) += tps65219.o
> > obj-$(CONFIG_PMIC_TPS65941) += tps65941.o
> > +obj-$(CONFIG_PMIC_TPS65224) += tps65224.o
>
> Ordering this. Maybe it can be located at tps65941.o.
>
> > obj-$(CONFIG_POWER_TPS65218) += pmic_tps65218.o
> >
> > ifeq ($(CONFIG_$(SPL_)POWER_LEGACY),y)
> > diff --git a/drivers/power/pmic/tps65224.c b/drivers/power/pmic/tps65224.c
> > new file mode 100644
> > index 00..33395f6edf
> > --- /dev/null
> > +++ b/drivers/power/pmic/tps65224.c
> > @@ -0,0 +1,141 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * (C) Copyright 2023 Texas Instruments Incorporated,
> > + */
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +
> > +static const struct pmic_child_info pmic_children_info[] = {
> > + { .prefix = "ldo", .driver = TPS65224_LDO_DRIVER },
> > + { .prefix = "buck", .driver = TPS65224_BUCK_DRIVER },
> > + { },
> > +};
> > +
> > +static int tps65224_write(struct udevice *dev, uint reg, const uint8_t
> > *buff,
> > + int len)
> > +{
> > + if (dm_i2c_write(dev, reg, buff, len)) {
> > + pr_err("write error to device: %p register: %#x!\n", dev, reg);
> > + return -EIO;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static int tps65224_read(struct udevice *dev, uint reg, uint8_t *buff, int
> > len)
> > +{
> > + if (dm_i2c_read(dev, reg, buff, len)) {
> > + pr_err("read error from device: %p register: %#x!\n", dev, reg);
> > + return -EIO;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static int tps65224_bind(struct udevice *dev)
> > +{
> > + ofnode regulators_node;
> > + int children;
> > +
> > + if (dev->driver_data == TPS65224_WD)
> > + return 0;
> > +
> > + regulators_node = dev_read_subnode(dev, "regulators");
> > + if (!ofnode_valid(regulators_node)) {
> > + debug("%s: %s regulators subnode not found!\n", __func__,
> > + dev->name);
> > + return -ENXIO;
> > + }
> > +
> > + debug("%s: '%s' - found regulators su
RE: [PATCH v2 1/2] driver: power: add support for TPS65224
Hi Bhargav,
> -Original Message-
> From: Bhargav Raviprakash
> Sent: Monday, November 6, 2023 11:07 PM
> To: u-boot@lists.denx.de
> Cc: jh80.ch...@samsung.com; Bhargav Raviprakash
> Subject: [PATCH v2 1/2] driver: power: add support for TPS65224
>
> Added support for PMIC TPS65224. Includes driver for pmic,
> and disabling Watchdog.
>
> Signed-off-by: Bhargav Raviprakash
> ---
> drivers/power/pmic/Kconfig| 6 ++
> drivers/power/pmic/Makefile | 1 +
> drivers/power/pmic/tps65224.c | 141 ++
> include/power/tps65224.h | 57 ++
> 4 files changed, 205 insertions(+)
> create mode 100644 drivers/power/pmic/tps65224.c
> create mode 100644 include/power/tps65224.h
>
> diff --git a/drivers/power/pmic/Kconfig b/drivers/power/pmic/Kconfig
> index 4a6f0ce093..b06bd31823 100644
> --- a/drivers/power/pmic/Kconfig
> +++ b/drivers/power/pmic/Kconfig
> @@ -378,6 +378,12 @@ config PMIC_TPS65941
> The TPS65941 is a PMIC containing a bunch of SMPS & LDOs.
> This driver binds the pmic children.
>
> +config PMIC_TPS65224
> + bool "Enable driver for Texas Instruments TPS65224 PMIC"
> + help
> + The TPS65224 is a PMIC containing a bunch of SMPS & LDOs.
> + This driver binds the pmic children.
> +
> config PMIC_TPS65219
> bool "Enable driver for Texas Instruments TPS65219 PMIC"
> depends on DM_PMIC
> diff --git a/drivers/power/pmic/Makefile b/drivers/power/pmic/Makefile
> index 0b3b3d62d0..cec16e57d3 100644
> --- a/drivers/power/pmic/Makefile
> +++ b/drivers/power/pmic/Makefile
> @@ -33,6 +33,7 @@ obj-$(CONFIG_PMIC_STPMIC1) += stpmic1.o
> obj-$(CONFIG_PMIC_TPS65217) += pmic_tps65217.o
> obj-$(CONFIG_PMIC_TPS65219) += tps65219.o
> obj-$(CONFIG_PMIC_TPS65941) += tps65941.o
> +obj-$(CONFIG_PMIC_TPS65224) += tps65224.o
Ordering this. Maybe it can be located at tps65941.o.
> obj-$(CONFIG_POWER_TPS65218) += pmic_tps65218.o
>
> ifeq ($(CONFIG_$(SPL_)POWER_LEGACY),y)
> diff --git a/drivers/power/pmic/tps65224.c b/drivers/power/pmic/tps65224.c
> new file mode 100644
> index 00..33395f6edf
> --- /dev/null
> +++ b/drivers/power/pmic/tps65224.c
> @@ -0,0 +1,141 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * (C) Copyright 2023 Texas Instruments Incorporated,
> + */
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +static const struct pmic_child_info pmic_children_info[] = {
> + { .prefix = "ldo", .driver = TPS65224_LDO_DRIVER },
> + { .prefix = "buck", .driver = TPS65224_BUCK_DRIVER },
> + { },
> +};
> +
> +static int tps65224_write(struct udevice *dev, uint reg, const uint8_t *buff,
> + int len)
> +{
> + if (dm_i2c_write(dev, reg, buff, len)) {
> + pr_err("write error to device: %p register: %#x!\n", dev, reg);
> + return -EIO;
> + }
> +
> + return 0;
> +}
> +
> +static int tps65224_read(struct udevice *dev, uint reg, uint8_t *buff, int
> len)
> +{
> + if (dm_i2c_read(dev, reg, buff, len)) {
> + pr_err("read error from device: %p register: %#x!\n", dev, reg);
> + return -EIO;
> + }
> +
> + return 0;
> +}
> +
> +static int tps65224_bind(struct udevice *dev)
> +{
> + ofnode regulators_node;
> + int children;
> +
> + if (dev->driver_data == TPS65224_WD)
> + return 0;
> +
> + regulators_node = dev_read_subnode(dev, "regulators");
> + if (!ofnode_valid(regulators_node)) {
> + debug("%s: %s regulators subnode not found!\n", __func__,
> + dev->name);
> + return -ENXIO;
> + }
> +
> + debug("%s: '%s' - found regulators subnode\n", __func__, dev->name);
> +
> + children = pmic_bind_children(dev, regulators_node, pmic_children_info);
> + if (!children)
> + printf("%s: %s - no child found\n", __func__, dev->name);
> +
> + /* Probe all the child devices */
> + return dm_scan_fdt_dev(dev);
> +}
> +
> +static int stop_watchdog(struct udevice *wd_i2c_dev)
> +{
> + int ret;
> +
> + /* Maintain WD long window */
> + ret = dm_i2c_reg_read(wd_i2c_dev, TPS65224_WD_MODE_REG);
> + if (ret < 0) {
> + debug("failed to read i2c reg (%d)\n", ret);
> + return ret;
> + }
> +
> + ret &= ~TPS65224_WD_PWRHOLD_MASK;
> + ret |= TPS65224_WD_PWRHOLD_MASK;
Is it a right behavior? After cleared its bit, set again?
Any reason to do this?
How about using val and ret as variable to clarify?
> + ret = dm_i2c_reg_write(wd_i2c_dev, TPS65224_WD_MODE_REG, ret);
> + if (ret) {
> + debug("%s: %s write WD_PWRHOLD fail!\n", __func__,
> wd_i2c_dev->name);
> + return ret;
> + }
> +
> + ret = dm_i2c_reg_read(wd_i2c_dev, TPS65224_WD_MODE_REG);
> + if (ret < 0) {
> + debug("failed to read back i2c reg (%d)\n", ret);
> + return ret;
> + }
> +
Re: [PATCH v2 1/2] driver: power: add support for TPS65224
Hi Bhargav,
On Mon, 6 Nov 2023 at 10:11, Bhargav Raviprakash wrote:
>
> Added support for PMIC TPS65224. Includes driver for pmic,
> and disabling Watchdog.
>
> Signed-off-by: Bhargav Raviprakash
> ---
> drivers/power/pmic/Kconfig| 6 ++
> drivers/power/pmic/Makefile | 1 +
> drivers/power/pmic/tps65224.c | 141 ++
> include/power/tps65224.h | 57 ++
> 4 files changed, 205 insertions(+)
> create mode 100644 drivers/power/pmic/tps65224.c
> create mode 100644 include/power/tps65224.h
>
> diff --git a/drivers/power/pmic/Kconfig b/drivers/power/pmic/Kconfig
> index 4a6f0ce093..b06bd31823 100644
> --- a/drivers/power/pmic/Kconfig
> +++ b/drivers/power/pmic/Kconfig
> @@ -378,6 +378,12 @@ config PMIC_TPS65941
> The TPS65941 is a PMIC containing a bunch of SMPS & LDOs.
> This driver binds the pmic children.
>
> +config PMIC_TPS65224
> + bool "Enable driver for Texas Instruments TPS65224 PMIC"
> + help
> + The TPS65224 is a PMIC containing a bunch of SMPS & LDOs.
> + This driver binds the pmic children.
> +
> config PMIC_TPS65219
> bool "Enable driver for Texas Instruments TPS65219 PMIC"
> depends on DM_PMIC
> diff --git a/drivers/power/pmic/Makefile b/drivers/power/pmic/Makefile
> index 0b3b3d62d0..cec16e57d3 100644
> --- a/drivers/power/pmic/Makefile
> +++ b/drivers/power/pmic/Makefile
> @@ -33,6 +33,7 @@ obj-$(CONFIG_PMIC_STPMIC1) += stpmic1.o
> obj-$(CONFIG_PMIC_TPS65217) += pmic_tps65217.o
> obj-$(CONFIG_PMIC_TPS65219) += tps65219.o
> obj-$(CONFIG_PMIC_TPS65941) += tps65941.o
> +obj-$(CONFIG_PMIC_TPS65224) += tps65224.o
> obj-$(CONFIG_POWER_TPS65218) += pmic_tps65218.o
>
> ifeq ($(CONFIG_$(SPL_)POWER_LEGACY),y)
> diff --git a/drivers/power/pmic/tps65224.c b/drivers/power/pmic/tps65224.c
> new file mode 100644
> index 00..33395f6edf
> --- /dev/null
> +++ b/drivers/power/pmic/tps65224.c
> @@ -0,0 +1,141 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * (C) Copyright 2023 Texas Instruments Incorporated,
> + */
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +static const struct pmic_child_info pmic_children_info[] = {
> + { .prefix = "ldo", .driver = TPS65224_LDO_DRIVER },
> + { .prefix = "buck", .driver = TPS65224_BUCK_DRIVER },
> + { },
> +};
> +
> +static int tps65224_write(struct udevice *dev, uint reg, const uint8_t *buff,
> + int len)
> +{
> + if (dm_i2c_write(dev, reg, buff, len)) {
> + pr_err("write error to device: %p register: %#x!\n", dev,
> reg);
> + return -EIO;
> + }
> +
> + return 0;
> +}
> +
> +static int tps65224_read(struct udevice *dev, uint reg, uint8_t *buff, int
> len)
> +{
> + if (dm_i2c_read(dev, reg, buff, len)) {
> + pr_err("read error from device: %p register: %#x!\n", dev,
> reg);
> + return -EIO;
> + }
> +
> + return 0;
> +}
> +
> +static int tps65224_bind(struct udevice *dev)
> +{
> + ofnode regulators_node;
> + int children;
> +
> + if (dev->driver_data == TPS65224_WD)
> + return 0;
> +
> + regulators_node = dev_read_subnode(dev, "regulators");
> + if (!ofnode_valid(regulators_node)) {
> + debug("%s: %s regulators subnode not found!\n", __func__,
> + dev->name);
> + return -ENXIO;
> + }
> +
> + debug("%s: '%s' - found regulators subnode\n", __func__, dev->name);
> +
> + children = pmic_bind_children(dev, regulators_node,
> pmic_children_info);
> + if (!children)
> + printf("%s: %s - no child found\n", __func__, dev->name);
> +
> + /* Probe all the child devices */
bind, not probe
> + return dm_scan_fdt_dev(dev);
> +}
> +
> +static int stop_watchdog(struct udevice *wd_i2c_dev)
> +{
> + int ret;
> +
> + /* Maintain WD long window */
> + ret = dm_i2c_reg_read(wd_i2c_dev, TPS65224_WD_MODE_REG);
> + if (ret < 0) {
> + debug("failed to read i2c reg (%d)\n", ret);
> + return ret;
> + }
> +
> + ret &= ~TPS65224_WD_PWRHOLD_MASK;
> + ret |= TPS65224_WD_PWRHOLD_MASK;
> + ret = dm_i2c_reg_write(wd_i2c_dev, TPS65224_WD_MODE_REG, ret);
> + if (ret) {
> + debug("%s: %s write WD_PWRHOLD fail!\n", __func__,
> wd_i2c_dev->name);
> + return ret;
> + }
> +
> + ret = dm_i2c_reg_read(wd_i2c_dev, TPS65224_WD_MODE_REG);
> + if (ret < 0) {
> + debug("failed to read back i2c reg (%d)\n", ret);
> + return ret;
> + }
> +
> + /* Disable WD */
> + ret = dm_i2c_reg_read(wd_i2c_dev, TPS65224_WD_THR_CFG);
> + if (ret < 0) {
> + debug("failed to read i2c reg (%d)\n", ret);
> + return ret;
> + }
> +
> + ret &=
