Re: [U-Boot] [PATCH 36/37] dm: pmic: add da9063 PMIC driver and regulators
Hi Robert, On 16/10/2019 13:11, Robert Beckett wrote: Huh, I had not seen that. Martin's versions looks more complete than mine, so I would say go with that one. Martin: any objections to including your patches in here? I dont mind pushing it through and handling any review comments etc. I am keen to get get this upstreamed. No that's fine by me. It would be good to get them upstream. Regards, Martin ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH 36/37] dm: pmic: add da9063 PMIC driver and regulators
On Wed, 2019-10-16 at 07:24 +, Schrempf Frieder wrote:
> Hi Robert,
>
> On 15.10.19 17:53, Robert Beckett wrote:
> > Add DM driver to support Dialog DA9063.
> > Currently it support binding regulator children.
> >
> > Signed-off-by: Robert Beckett
>
> I also have a board with DA9063 and was looking for support in U-
> Boot. I
> found patches from Martin Fuzzey [1] and pulled them in almost as is
> and
> found them to work just fine.
>
> Unfortunately I didn't have time to resend them yet, but you can
> find
> the rebased patches here: [2].
>
> On a first glance your implementation looks different, so it seems
> you
> didn't use Martin's patches. Though the resulting features probably
> will
> be similar.
>
> I only had a very quick look and one difference seems to be that
> your
> regulator implementation supports the AUTO and SYNC mode, while
> Martin's
> version supports AUTO, SYNC and SLEEP. There might be other
> differences.
>
> What do you think? Which version would be better?
(+cc Martin Fuzzey)
Huh, I had not seen that.
Martin's versions looks more complete than mine, so I would say go with
that one.
Martin: any objections to including your patches in here? I dont mind
pushing it through and handling any review comments etc. I am keen to
get get this upstreamed.
>
> Also find a few comments below, though I didn't do a full review,
> yet.
>
> Thanks,
> Frieder
>
> [1] https://patchwork.ozlabs.org/cover/979346/
> [2] https://github.com/fschrempf/u-boot/commits/da9063
>
> > ---
> > drivers/power/pmic/Kconfig | 8 +
> > drivers/power/pmic/Makefile | 1 +
> > drivers/power/pmic/da9063.c | 270 ++
> > drivers/power/regulator/Kconfig | 7 +
> > drivers/power/regulator/Makefile | 1 +
> > drivers/power/regulator/da9063.c | 320
> > +++
> > include/power/da9063_pmic.h | 303
> > +
> > 7 files changed, 910 insertions(+)
> > create mode 100644 drivers/power/pmic/da9063.c
> > create mode 100644 drivers/power/regulator/da9063.c
> > create mode 100644 include/power/da9063_pmic.h
> >
> > diff --git a/drivers/power/pmic/Kconfig
> > b/drivers/power/pmic/Kconfig
> > index 586772fdec..6dd7b1bf76 100644
> > --- a/drivers/power/pmic/Kconfig
> > +++ b/drivers/power/pmic/Kconfig
> > @@ -267,3 +267,11 @@ config SPL_PMIC_LP87565
> > help
> > The LP87565 is a PMIC containing a bunch of SMPS.
> > This driver binds the pmic children in SPL.
> > +
> > +config DM_PMIC_DA9063
> > + bool "Enable support for Dialog DA9063 PMIC"
> > + depends on DM_PMIC && (DM_I2C || DM_SPI)
> > + help
> > + The DA9063 is a PMIC providing 6 BUCK converters and 11 LDO
> > regulators.
> > + It can be accessed via I2C or SPI.
> > + This driver binds the pmic children.
> > diff --git a/drivers/power/pmic/Makefile
> > b/drivers/power/pmic/Makefile
> > index 888dbb2857..9be9d5d9a0 100644
> > --- a/drivers/power/pmic/Makefile
> > +++ b/drivers/power/pmic/Makefile
> > @@ -25,6 +25,7 @@ obj-$(CONFIG_$(SPL_)PMIC_PALMAS) += palmas.o
> > obj-$(CONFIG_$(SPL_)PMIC_LP873X) += lp873x.o
> > obj-$(CONFIG_$(SPL_)PMIC_LP87565) += lp87565.o
> > obj-$(CONFIG_PMIC_STPMIC1) += stpmic1.o
> > +obj-$(CONFIG_DM_PMIC_DA9063) += da9063.o
>
> It would be good to be able to enable the driver for U-Boot proper
> and
> SPL separately. So this should be CONFIG_$(SPL_)DM_PMIC_DA9063.
>
> >
> > obj-$(CONFIG_POWER_LTC3676) += pmic_ltc3676.o
> > obj-$(CONFIG_POWER_MAX77696) += pmic_max77696.o
> > diff --git a/drivers/power/pmic/da9063.c
> > b/drivers/power/pmic/da9063.c
> > new file mode 100644
> > index 00..81a7803b09
> > --- /dev/null
> > +++ b/drivers/power/pmic/da9063.c
> > @@ -0,0 +1,270 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * (C) Copyright 2019 Collabora
> > + * (C) Copyright 2019 GE
> > + */
> > +
> > +#define DEBUG 1
>
> This should not be here.
>
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +
> > +static const struct pmic_child_info pmic_children_info[] = {
> > + { .prefix = "bcore", .driver = DA9063_BUCK_DRIVER },
> > + { .prefix = "bpro", .driver = DA9063_BUCK_DRIVER },
> > + { .prefix = "bmem", .driver = DA9063_BUCK_DRIVER },
> > + { .prefix = "bio", .driver = DA9063_BUCK_DRIVER },
> > + { .prefix = "bperi", .driver = DA9063_BUCK_DRIVER },
> > + { .prefix = "ldo", .driver = DA9063_LDO_DRIVER },
> > + { },
> > +};
> > +
> > +static int da9063_reg_count(struct udevice *dev)
> > +{
> > + return DA9063_NUM_OF_REGS;
> > +}
> > +
> > +#if defined(CONFIG_DM_I2C)
> > +static int da9063_i2c_read(struct udevice *dev, uint reg, uint8_t
> > *buff,
> > + int len)
> > +{
> > + int ret;
> > +
> > + /* only support single reg accesses */
> > + if (len != 1)
> > + return -EINVAL;
> > +
> > + ret = dm_i2c_read(d
Re: [U-Boot] [PATCH 36/37] dm: pmic: add da9063 PMIC driver and regulators
Hi Robert,
On 15.10.19 17:53, Robert Beckett wrote:
> Add DM driver to support Dialog DA9063.
> Currently it support binding regulator children.
>
> Signed-off-by: Robert Beckett
I also have a board with DA9063 and was looking for support in U-Boot. I
found patches from Martin Fuzzey [1] and pulled them in almost as is and
found them to work just fine.
Unfortunately I didn't have time to resend them yet, but you can find
the rebased patches here: [2].
On a first glance your implementation looks different, so it seems you
didn't use Martin's patches. Though the resulting features probably will
be similar.
I only had a very quick look and one difference seems to be that your
regulator implementation supports the AUTO and SYNC mode, while Martin's
version supports AUTO, SYNC and SLEEP. There might be other differences.
What do you think? Which version would be better?
Also find a few comments below, though I didn't do a full review, yet.
Thanks,
Frieder
[1] https://patchwork.ozlabs.org/cover/979346/
[2] https://github.com/fschrempf/u-boot/commits/da9063
> ---
> drivers/power/pmic/Kconfig | 8 +
> drivers/power/pmic/Makefile | 1 +
> drivers/power/pmic/da9063.c | 270 ++
> drivers/power/regulator/Kconfig | 7 +
> drivers/power/regulator/Makefile | 1 +
> drivers/power/regulator/da9063.c | 320 +++
> include/power/da9063_pmic.h | 303 +
> 7 files changed, 910 insertions(+)
> create mode 100644 drivers/power/pmic/da9063.c
> create mode 100644 drivers/power/regulator/da9063.c
> create mode 100644 include/power/da9063_pmic.h
>
> diff --git a/drivers/power/pmic/Kconfig b/drivers/power/pmic/Kconfig
> index 586772fdec..6dd7b1bf76 100644
> --- a/drivers/power/pmic/Kconfig
> +++ b/drivers/power/pmic/Kconfig
> @@ -267,3 +267,11 @@ config SPL_PMIC_LP87565
> help
> The LP87565 is a PMIC containing a bunch of SMPS.
> This driver binds the pmic children in SPL.
> +
> +config DM_PMIC_DA9063
> + bool "Enable support for Dialog DA9063 PMIC"
> + depends on DM_PMIC && (DM_I2C || DM_SPI)
> + help
> + The DA9063 is a PMIC providing 6 BUCK converters and 11 LDO regulators.
> + It can be accessed via I2C or SPI.
> + This driver binds the pmic children.
> diff --git a/drivers/power/pmic/Makefile b/drivers/power/pmic/Makefile
> index 888dbb2857..9be9d5d9a0 100644
> --- a/drivers/power/pmic/Makefile
> +++ b/drivers/power/pmic/Makefile
> @@ -25,6 +25,7 @@ obj-$(CONFIG_$(SPL_)PMIC_PALMAS) += palmas.o
> obj-$(CONFIG_$(SPL_)PMIC_LP873X) += lp873x.o
> obj-$(CONFIG_$(SPL_)PMIC_LP87565) += lp87565.o
> obj-$(CONFIG_PMIC_STPMIC1) += stpmic1.o
> +obj-$(CONFIG_DM_PMIC_DA9063) += da9063.o
It would be good to be able to enable the driver for U-Boot proper and
SPL separately. So this should be CONFIG_$(SPL_)DM_PMIC_DA9063.
>
> obj-$(CONFIG_POWER_LTC3676) += pmic_ltc3676.o
> obj-$(CONFIG_POWER_MAX77696) += pmic_max77696.o
> diff --git a/drivers/power/pmic/da9063.c b/drivers/power/pmic/da9063.c
> new file mode 100644
> index 00..81a7803b09
> --- /dev/null
> +++ b/drivers/power/pmic/da9063.c
> @@ -0,0 +1,270 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * (C) Copyright 2019 Collabora
> + * (C) Copyright 2019 GE
> + */
> +
> +#define DEBUG 1
This should not be here.
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +static const struct pmic_child_info pmic_children_info[] = {
> + { .prefix = "bcore", .driver = DA9063_BUCK_DRIVER },
> + { .prefix = "bpro", .driver = DA9063_BUCK_DRIVER },
> + { .prefix = "bmem", .driver = DA9063_BUCK_DRIVER },
> + { .prefix = "bio", .driver = DA9063_BUCK_DRIVER },
> + { .prefix = "bperi", .driver = DA9063_BUCK_DRIVER },
> + { .prefix = "ldo", .driver = DA9063_LDO_DRIVER },
> + { },
> +};
> +
> +static int da9063_reg_count(struct udevice *dev)
> +{
> + return DA9063_NUM_OF_REGS;
> +}
> +
> +#if defined(CONFIG_DM_I2C)
> +static int da9063_i2c_read(struct udevice *dev, uint reg, uint8_t *buff,
> +int len)
> +{
> + int ret;
> +
> + /* only support single reg accesses */
> + if (len != 1)
> + return -EINVAL;
> +
> + ret = dm_i2c_read(dev, reg, buff, len);
> + if (ret) {
> + pr_err("%s: unable to read reg %#x: %d\n", __func__, reg, ret);
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static int da9063_i2c_write(struct udevice *dev, uint reg, const uint8_t
> *buff,
> + int len)
> +{
> + int ret;
> +
> + /* only support single reg accesses */
> + if (len != 1)
> + return -EINVAL;
> +
> + ret = dm_i2c_write(dev, reg, buff, len);
> + if (ret) {
> + pr_err("%s: unable to write reg %#x: %d\n", __func__, reg, ret);
> + return ret;
>
[U-Boot] [PATCH 36/37] dm: pmic: add da9063 PMIC driver and regulators
Add DM driver to support Dialog DA9063.
Currently it support binding regulator children.
Signed-off-by: Robert Beckett
---
drivers/power/pmic/Kconfig | 8 +
drivers/power/pmic/Makefile | 1 +
drivers/power/pmic/da9063.c | 270 ++
drivers/power/regulator/Kconfig | 7 +
drivers/power/regulator/Makefile | 1 +
drivers/power/regulator/da9063.c | 320 +++
include/power/da9063_pmic.h | 303 +
7 files changed, 910 insertions(+)
create mode 100644 drivers/power/pmic/da9063.c
create mode 100644 drivers/power/regulator/da9063.c
create mode 100644 include/power/da9063_pmic.h
diff --git a/drivers/power/pmic/Kconfig b/drivers/power/pmic/Kconfig
index 586772fdec..6dd7b1bf76 100644
--- a/drivers/power/pmic/Kconfig
+++ b/drivers/power/pmic/Kconfig
@@ -267,3 +267,11 @@ config SPL_PMIC_LP87565
help
The LP87565 is a PMIC containing a bunch of SMPS.
This driver binds the pmic children in SPL.
+
+config DM_PMIC_DA9063
+ bool "Enable support for Dialog DA9063 PMIC"
+ depends on DM_PMIC && (DM_I2C || DM_SPI)
+ help
+ The DA9063 is a PMIC providing 6 BUCK converters and 11 LDO regulators.
+ It can be accessed via I2C or SPI.
+ This driver binds the pmic children.
diff --git a/drivers/power/pmic/Makefile b/drivers/power/pmic/Makefile
index 888dbb2857..9be9d5d9a0 100644
--- a/drivers/power/pmic/Makefile
+++ b/drivers/power/pmic/Makefile
@@ -25,6 +25,7 @@ obj-$(CONFIG_$(SPL_)PMIC_PALMAS) += palmas.o
obj-$(CONFIG_$(SPL_)PMIC_LP873X) += lp873x.o
obj-$(CONFIG_$(SPL_)PMIC_LP87565) += lp87565.o
obj-$(CONFIG_PMIC_STPMIC1) += stpmic1.o
+obj-$(CONFIG_DM_PMIC_DA9063) += da9063.o
obj-$(CONFIG_POWER_LTC3676) += pmic_ltc3676.o
obj-$(CONFIG_POWER_MAX77696) += pmic_max77696.o
diff --git a/drivers/power/pmic/da9063.c b/drivers/power/pmic/da9063.c
new file mode 100644
index 00..81a7803b09
--- /dev/null
+++ b/drivers/power/pmic/da9063.c
@@ -0,0 +1,270 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * (C) Copyright 2019 Collabora
+ * (C) Copyright 2019 GE
+ */
+
+#define DEBUG 1
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+static const struct pmic_child_info pmic_children_info[] = {
+ { .prefix = "bcore", .driver = DA9063_BUCK_DRIVER },
+ { .prefix = "bpro", .driver = DA9063_BUCK_DRIVER },
+ { .prefix = "bmem", .driver = DA9063_BUCK_DRIVER },
+ { .prefix = "bio", .driver = DA9063_BUCK_DRIVER },
+ { .prefix = "bperi", .driver = DA9063_BUCK_DRIVER },
+ { .prefix = "ldo", .driver = DA9063_LDO_DRIVER },
+ { },
+};
+
+static int da9063_reg_count(struct udevice *dev)
+{
+ return DA9063_NUM_OF_REGS;
+}
+
+#if defined(CONFIG_DM_I2C)
+static int da9063_i2c_read(struct udevice *dev, uint reg, uint8_t *buff,
+ int len)
+{
+ int ret;
+
+ /* only support single reg accesses */
+ if (len != 1)
+ return -EINVAL;
+
+ ret = dm_i2c_read(dev, reg, buff, len);
+ if (ret) {
+ pr_err("%s: unable to read reg %#x: %d\n", __func__, reg, ret);
+ return ret;
+ }
+
+ return 0;
+}
+
+static int da9063_i2c_write(struct udevice *dev, uint reg, const uint8_t *buff,
+ int len)
+{
+ int ret;
+
+ /* only support single reg accesses */
+ if (len != 1)
+ return -EINVAL;
+
+ ret = dm_i2c_write(dev, reg, buff, len);
+ if (ret) {
+ pr_err("%s: unable to write reg %#x: %d\n", __func__, reg, ret);
+ return ret;
+ }
+
+ return 0;
+}
+#endif
+
+#if defined(CONFIG_DM_SPI)
+static int da9063_spi_read(struct udevice *dev, uint reg, uint8_t *buff,
+ int len)
+{
+ u8 page;
+ u8 data[2];
+ int ret;
+
+ /* only support single reg accesses */
+ if (len != 1)
+ return -EINVAL;
+
+ page = FIELD_GET(DA9063_REG_PAGE_MASK, reg);
+ reg = FIELD_GET(DA9063_REG_ADDR_MASK, reg);
+
+ ret = dm_spi_claim_bus(dev);
+ if (ret)
+ return ret;
+ /* set page */
+ data[0] = FIELD_PREP(DA9063_PROTO_ADDR_MASK, DA9063_PAGE_CON) |
+ FIELD_PREP(DA9063_PROTO_RW_MASK, DA9063_PROTO_WRITE);
+ data[1] = FIELD_PREP(DA9063_PAGE_CON_PAGE, page);
+ ret = dm_spi_xfer(dev, DA9063_PROTO_LEN, data, NULL, SPI_XFER_ONCE);
+ if (ret) {
+ pr_err("%s: unable to set page: %d\n", __func__, ret);
+ goto err_page;
+ }
+
+ /* set target reg */
+ data[0] = FIELD_PREP(DA9063_PROTO_ADDR_MASK, reg) |
+ FIELD_PREP(DA9063_PROTO_RW_MASK, DA9063_PROTO_READ);
+ data[1] = 0;
+ ret = dm_spi_xfer(dev, DA9063_PROTO_LEN, data, data, SPI_XFER_ONCE);
+ if (ret) {
+ pr_err("%s: unable to read reg %#x: %d\n"
