[U-Boot] [PATCH 1/5] pmic: fsl: Provide some more definitions for MC34708 PMIC

[U-Boot] [PATCH 1/5] pmic: fsl: Provide some more definitions for MC34708 PMIC

[Lukasz Majewski]

This commit adds some more defines for MC34708 PMIC.

Signed-off-by: Lukasz Majewski <lukma@denx.de>
---

 include/fsl_pmic.h | 40 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 40 insertions(+)

diff --git a/include/fsl_pmic.h b/include/fsl_pmic.h
index e8a67d552a..f2fe187769 100644
--- a/include/fsl_pmic.h
+++ b/include/fsl_pmic.h
@@ -108,6 +108,7 @@ enum {
 
 /* MC34708 Definitions */
 #define SWx_VOLT_MASK_MC34708	0x3F
+#define SWx_1_110V_MC34708	0x24
 #define SWx_1_250V_MC34708	0x30
 #define SWx_1_300V_MC34708	0x34
 #define TIMER_MASK_MC34708	0x300
@@ -117,4 +118,43 @@ enum {
 #define SWBST_CTRL		31
 #define SWBST_AUTO		0x8
 
+#define MC34708_REG_SW12_OPMODE	28
+
+#define MC34708_SW1AMODE_MASK	0x00000f
+#define MC34708_SW1AMHMODE	0x000010
+#define MC34708_SW1AUOMODE	0x000020
+#define MC34708_SW1DVSSPEED	0x0000c0
+#define MC34708_SW2MODE_MASK	0x03c000
+#define MC34708_SW2MHMODE	0x040000
+#define MC34708_SW2UOMODE	0x080000
+#define MC34708_SW2DVSSPEED	0x300000
+#define MC34708_PLLEN		0x400000
+#define MC34708_PLLX		0x800000
+
+#define MC34708_REG_SW345_OPMODE	29
+
+#define MC34708_SW3MODE_MASK	0x00000f
+#define MC34708_SW3MHMODE	0x000010
+#define MC34708_SW3UOMODE	0x000020
+#define MC34708_SW4AMODE_MASK	0x0003c0
+#define MC34708_SW4AMHMODE	0x000400
+#define MC34708_SW4AUOMODE	0x000800
+#define MC34708_SW4BMODE_MASK	0x00f000
+#define MC34708_SW4BMHMODE	0x010000
+#define MC34708_SW4BUOMODE	0x020000
+#define MC34708_SW5MODE_MASK	0x3c0000
+#define MC34708_SW5MHMODE	0x400000
+#define MC34708_SW5UOMODE	0x800000
+
+#define SW_MODE_OFFOFF		0x00
+#define SW_MODE_PWMOFF		0x01
+#define SW_MODE_PFMOFF		0x03
+#define SW_MODE_APSOFF		0x04
+#define SW_MODE_PWMPWM		0x05
+#define SW_MODE_PWMAPS		0x06
+#define SW_MODE_APSAPS		0x08
+#define SW_MODE_APSPFM		0x0c
+#define SW_MODE_PWMPFM		0x0d
+#define SW_MODE_PFMPFM		0x0f
+
 #endif
-- 
2.11.0

[U-Boot] [PATCH 2/5] pmic: fsl: Define number of bytes sent at once by MC34708 PMIC

[Lukasz Majewski]

This patch adds definition of the number of bytes sent at once by the
MC34708 PMIC.

Signed-off-by: Lukasz Majewski <lukma@denx.de>

---

 include/fsl_pmic.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/fsl_pmic.h b/include/fsl_pmic.h
index f2fe187769..ff598e7e16 100644
--- a/include/fsl_pmic.h
+++ b/include/fsl_pmic.h
@@ -157,4 +157,5 @@ enum {
 #define SW_MODE_PWMPFM		0x0d
 #define SW_MODE_PFMPFM		0x0f
 
+#define MC34708_TRANSFER_SIZE 3
 #endif
-- 
2.11.0

[U-Boot] [PATCH 3/5] pmic: dm: Provide *trans_len() callback for pmic-uclass

[Lukasz Majewski]

This callback provides information of number of bytes to be sent/read
during single transfer, as some PMICs transfer more bytes at once (e.g.
MC34708 from NXP/Freescale).

At present the PMIC DM supports only transfers with one byte. The old PMIC
framework was able to transfer up till 3 bytes at once.

Let's provide the same capability to new PMIC framework.

Signed-off-by: Lukasz Majewski <lukma@denx.de>
---

 drivers/power/pmic/pmic-uclass.c | 10 ++++++++++
 include/power/pmic.h             |  2 ++
 2 files changed, 12 insertions(+)

diff --git a/drivers/power/pmic/pmic-uclass.c b/drivers/power/pmic/pmic-uclass.c
index 9347b40688..0148e5c883 100644
--- a/drivers/power/pmic/pmic-uclass.c
+++ b/drivers/power/pmic/pmic-uclass.c
@@ -105,6 +105,16 @@ int pmic_reg_count(struct udevice *dev)
 	return ops->reg_count(dev);
 }
 
+int pmic_trans_len(struct udevice *dev)
+{
+	const struct dm_pmic_ops *ops = dev_get_driver_ops(dev);
+
+	if (!ops || !ops->trans_len)
+		return -ENOSYS;
+
+	return ops->trans_len(dev);
+}
+
 int pmic_read(struct udevice *dev, uint reg, uint8_t *buffer, int len)
 {
 	const struct dm_pmic_ops *ops = dev_get_driver_ops(dev);
diff --git a/include/power/pmic.h b/include/power/pmic.h
index f2fe537fb7..d89f6be666 100644
--- a/include/power/pmic.h
+++ b/include/power/pmic.h
@@ -159,12 +159,14 @@ struct pmic {
  * @reg_count: device's register count
  * @read:      read 'len' bytes at "reg" and store it into the 'buffer'
  * @write:     write 'len' bytes from the 'buffer' to the register at 'reg' address
+ * @trans_len: device's number of data bytes for transfer
  */
 struct dm_pmic_ops {
 	int (*reg_count)(struct udevice *dev);
 	int (*read)(struct udevice *dev, uint reg, uint8_t *buffer, int len);
 	int (*write)(struct udevice *dev, uint reg, const uint8_t *buffer,
 		     int len);
+	int (*trans_len)(struct udevice *dev);
 };
 
 /**
-- 
2.11.0

[U-Boot] [PATCH 4/5] pmic: dm: Rewrite pmic_reg_{read|write} and pmic_clrsetbits to support transmissions larger than 1 byte

[Lukasz Majewski]

This commit provides support for transmissions larger than 1 byte for
PMIC devices used with DM (e.g. MC34708 from NXP).

Signed-off-by: Lukasz Majewski <lukma@denx.de>
---

 drivers/power/pmic/pmic-uclass.c | 38 ++++++++++++++++++++++++++------------
 1 file changed, 26 insertions(+), 12 deletions(-)

diff --git a/drivers/power/pmic/pmic-uclass.c b/drivers/power/pmic/pmic-uclass.c
index 0148e5c883..9004911daa 100644
--- a/drivers/power/pmic/pmic-uclass.c
+++ b/drivers/power/pmic/pmic-uclass.c
@@ -143,23 +143,32 @@ int pmic_write(struct udevice *dev, uint reg, const uint8_t *buffer, int len)
 
 int pmic_reg_read(struct udevice *dev, uint reg)
 {
-	u8 byte;
+	int tx_num = 1;
+	u32 val = 0;
 	int ret;
 
-	debug("%s: reg=%x", __func__, reg);
-	ret = pmic_read(dev, reg, &byte, 1);
-	debug(", value=%x, ret=%d\n", byte, ret);
+	ret = pmic_trans_len(dev);
+	if (ret > 1 && ret <= sizeof(val))
+		tx_num = ret;
 
-	return ret ? ret : byte;
+	debug("%s: reg=%x tx_num:%d", __func__, reg, tx_num);
+	ret = pmic_read(dev, reg, (uint8_t *)&val, tx_num);
+	debug(", value=%x, ret=%d\n", val, ret);
+
+	return ret ? ret : val;
 }
 
 int pmic_reg_write(struct udevice *dev, uint reg, uint value)
 {
-	u8 byte = value;
+	int tx_num = 1;
 	int ret;
 
-	debug("%s: reg=%x, value=%x", __func__, reg, value);
-	ret = pmic_write(dev, reg, &byte, 1);
+	ret = pmic_trans_len(dev);
+	if (ret > 1 && ret <= sizeof(value))
+		tx_num = ret;
+
+	debug("%s: reg=%x, value=%x tx_num:%d", __func__, reg, value, tx_num);
+	ret = pmic_write(dev, reg, (uint8_t *)&value, tx_num);
 	debug(", ret=%d\n", ret);
 
 	return ret;
@@ -167,15 +176,20 @@ int pmic_reg_write(struct udevice *dev, uint reg, uint value)
 
 int pmic_clrsetbits(struct udevice *dev, uint reg, uint clr, uint set)
 {
-	u8 byte;
+	int tx_num = 1;
+	u32 val = 0;
 	int ret;
 
-	ret = pmic_reg_read(dev, reg);
+	ret = pmic_trans_len(dev);
+	if (ret > 1 && ret <= sizeof(val))
+		tx_num = ret;
+
+	ret = pmic_read(dev, reg, (uint8_t *)&val, tx_num);
 	if (ret < 0)
 		return ret;
-	byte = (ret & ~clr) | set;
 
-	return pmic_reg_write(dev, reg, byte);
+	val = (val & ~clr) | set;
+	return pmic_write(dev, reg, (uint8_t *)&val, tx_num);
 }
 
 UCLASS_DRIVER(pmic) = {
-- 
2.11.0

[U-Boot] [PATCH 5/5] pmic: dm: Add support for MC34708 for PMIC DM

[Lukasz Majewski]

This patch adds support for MC34708 PMIC, to be used with driver model
(DM).

Signed-off-by: Lukasz Majewski <lukma@denx.de>
---

 drivers/power/pmic/Kconfig   |   7 +++
 drivers/power/pmic/Makefile  |   1 +
 drivers/power/pmic/mc34708.c | 103 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 111 insertions(+)
 create mode 100644 drivers/power/pmic/mc34708.c

diff --git a/drivers/power/pmic/Kconfig b/drivers/power/pmic/Kconfig
index 40ab9f7fa5..d504c28b77 100644
--- a/drivers/power/pmic/Kconfig
+++ b/drivers/power/pmic/Kconfig
@@ -69,6 +69,13 @@ config DM_PMIC_MAX8998
 	This config enables implementation of driver-model pmic uclass features
 	for PMIC MAX8998. The driver implements read/write operations.
 
+config DM_PMIC_MC34708
+	bool "Enable Driver Model for PMIC MC34708"
+	depends on DM_PMIC
+	help
+	 This config enables implementation of driver-model pmic uclass features
+	 for PMIC MC34708. The driver implements read/write operations.
+
 config PMIC_MAX8997
 	bool "Enable Driver Model for PMIC MAX8997"
 	depends on DM_PMIC
diff --git a/drivers/power/pmic/Makefile b/drivers/power/pmic/Makefile
index ad32068b3a..418b5e7aee 100644
--- a/drivers/power/pmic/Makefile
+++ b/drivers/power/pmic/Makefile
@@ -8,6 +8,7 @@
 obj-$(CONFIG_DM_PMIC) += pmic-uclass.o
 obj-$(CONFIG_DM_PMIC_MAX77686) += max77686.o
 obj-$(CONFIG_DM_PMIC_MAX8998) += max8998.o
+obj-$(CONFIG_DM_PMIC_MC34708) += mc34708.o
 obj-$(CONFIG_$(SPL_)DM_PMIC_PFUZE100) += pfuze100.o
 obj-$(CONFIG_PMIC_S2MPS11) += s2mps11.o
 obj-$(CONFIG_DM_PMIC_SANDBOX) += sandbox.o i2c_pmic_emul.o
diff --git a/drivers/power/pmic/mc34708.c b/drivers/power/pmic/mc34708.c
new file mode 100644
index 0000000000..b40314e8ea
--- /dev/null
+++ b/drivers/power/pmic/mc34708.c
@@ -0,0 +1,103 @@
+/*
+ * Copyright (C) 2018
+ * Lukasz Majewski, DENX Software Engineering, lukma at denx.de
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#include <common.h>
+#include <dm.h>
+#include <errno.h>
+#include <i2c.h>
+#include <power/pmic.h>
+#include <fsl_pmic.h>
+
+DECLARE_GLOBAL_DATA_PTR;
+
+static int mc34708_reg_count(struct udevice *dev)
+{
+	return PMIC_NUM_OF_REGS;
+}
+
+static int mc34708_trans_len(struct udevice *dev)
+{
+	return MC34708_TRANSFER_SIZE;
+}
+
+static int mc34708_write(struct udevice *dev, uint reg, const u8 *buff,
+			 int len)
+{
+	u8 buf[3] = { 0 };
+	int ret;
+
+	if (len != MC34708_TRANSFER_SIZE)
+		return -EINVAL;
+
+	buf[0] = buff[2];
+	buf[1] = buff[1];
+	buf[2] = buff[0];
+
+	ret = dm_i2c_write(dev, reg, buf, len);
+	if (ret)
+		printf("write error to device: %p register: %#x!", dev, reg);
+
+	return ret;
+}
+
+static int mc34708_read(struct udevice *dev, uint reg, u8 *buff, int len)
+{
+	u8 buf[3] = { 0 };
+	int ret;
+
+	if (len != MC34708_TRANSFER_SIZE)
+		return -EINVAL;
+
+	ret = dm_i2c_read(dev, reg, buf, len);
+	if (ret)
+		printf("read error from device: %p register: %#x!", dev, reg);
+
+	buff[0] = buf[2];
+	buff[1] = buf[1];
+	buff[2] = buf[0];
+
+	return ret;
+}
+
+static int mc34708_probe(struct udevice *dev)
+{
+	/*
+	 * Handle PMIC Errata 37: APS mode not fully functional,
+	 * use explicit PWM or PFM instead
+	 */
+	pmic_clrsetbits(dev, MC34708_REG_SW12_OPMODE,
+			MC34708_SW1AMODE_MASK | MC34708_SW2MODE_MASK,
+			SW_MODE_PWMPWM | (SW_MODE_PWMPWM << 14u));
+
+	pmic_clrsetbits(dev, MC34708_REG_SW345_OPMODE,
+			MC34708_SW3MODE_MASK | MC34708_SW4AMODE_MASK |
+			MC34708_SW4BMODE_MASK | MC34708_SW5MODE_MASK,
+			SW_MODE_PWMPWM | (SW_MODE_PWMPWM << 6u) |
+			(SW_MODE_PWMPWM << 12u) | (SW_MODE_PWMPWM << 18u));
+
+	return 0;
+}
+
+static struct dm_pmic_ops mc34708_ops = {
+	.reg_count = mc34708_reg_count,
+	.read	= mc34708_read,
+	.write	= mc34708_write,
+	.trans_len = mc34708_trans_len,
+};
+
+static const struct udevice_id mc34708_ids[] = {
+	{ .compatible = "fsl,mc34708" },
+	{ }
+};
+
+U_BOOT_DRIVER(pmic_mc34708) = {
+	.name		= "mc34708_pmic",
+	.id		= UCLASS_PMIC,
+	.of_match	= mc34708_ids,
+	.probe          = mc34708_probe,
+	.ops		= &mc34708_ops,
+};
-- 
2.11.0

[U-Boot] [PATCH 3/5] pmic: dm: Provide *trans_len() callback for pmic-uclass

[Simon Glass]

Hi Lucasz,

On 26 April 2018 at 06:19, Lukasz Majewski <lukma@denx.de> wrote:
> This callback provides information of number of bytes to be sent/read
> during single transfer, as some PMICs transfer more bytes at once (e.g.
> MC34708 from NXP/Freescale).
>
> At present the PMIC DM supports only transfers with one byte. The old PMIC
> framework was able to transfer up till 3 bytes at once.
>
> Let's provide the same capability to new PMIC framework.

Can we instead make this a property of uclass-private data for each device?

I don't see a lot of point in adding a new method, since the value is
presumably set once and does not change through the life of the
device.

Regards,
Simon

[U-Boot] [PATCH 4/5] pmic: dm: Rewrite pmic_reg_{read|write} and pmic_clrsetbits to support transmissions larger than 1 byte

[Simon Glass]

Hi Lukasz,

On 26 April 2018 at 06:19, Lukasz Majewski <lukma@denx.de> wrote:
> This commit provides support for transmissions larger than 1 byte for
> PMIC devices used with DM (e.g. MC34708 from NXP).
>
> Signed-off-by: Lukasz Majewski <lukma@denx.de>
> ---
>
>  drivers/power/pmic/pmic-uclass.c | 38 ++++++++++++++++++++++++++------------
>  1 file changed, 26 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/power/pmic/pmic-uclass.c b/drivers/power/pmic/pmic-uclass.c
> index 0148e5c883..9004911daa 100644
> --- a/drivers/power/pmic/pmic-uclass.c
> +++ b/drivers/power/pmic/pmic-uclass.c
> @@ -143,23 +143,32 @@ int pmic_write(struct udevice *dev, uint reg, const uint8_t *buffer, int len)
>
>  int pmic_reg_read(struct udevice *dev, uint reg)
>  {
> -       u8 byte;
> +       int tx_num = 1;
> +       u32 val = 0;
>         int ret;
>
> -       debug("%s: reg=%x", __func__, reg);
> -       ret = pmic_read(dev, reg, &byte, 1);
> -       debug(", value=%x, ret=%d\n", byte, ret);
> +       ret = pmic_trans_len(dev);

As with the previous patch, I think it is better to put this in the
uclass's device-private data. It can be set to 1 by default, and the
device can updated it to 2, etc, as needed.

Then we avoid making this extra call for every operation.

Also please don't forget to add a sandbox test for it.

> +       if (ret > 1 && ret <= sizeof(val))
> +               tx_num = ret;
>
> -       return ret ? ret : byte;
> +       debug("%s: reg=%x tx_num:%d", __func__, reg, tx_num);
> +       ret = pmic_read(dev, reg, (uint8_t *)&val, tx_num);
> +       debug(", value=%x, ret=%d\n", val, ret);
> +
> +       return ret ? ret : val;
>  }
>
>  int pmic_reg_write(struct udevice *dev, uint reg, uint value)
>  {
> -       u8 byte = value;
> +       int tx_num = 1;
>         int ret;
>
> -       debug("%s: reg=%x, value=%x", __func__, reg, value);
> -       ret = pmic_write(dev, reg, &byte, 1);
> +       ret = pmic_trans_len(dev);
> +       if (ret > 1 && ret <= sizeof(value))
> +               tx_num = ret;
> +
> +       debug("%s: reg=%x, value=%x tx_num:%d", __func__, reg, value, tx_num);
> +       ret = pmic_write(dev, reg, (uint8_t *)&value, tx_num);
>         debug(", ret=%d\n", ret);
>
>         return ret;
> @@ -167,15 +176,20 @@ int pmic_reg_write(struct udevice *dev, uint reg, uint value)
>
>  int pmic_clrsetbits(struct udevice *dev, uint reg, uint clr, uint set)
>  {
> -       u8 byte;
> +       int tx_num = 1;
> +       u32 val = 0;
>         int ret;
>
> -       ret = pmic_reg_read(dev, reg);
> +       ret = pmic_trans_len(dev);
> +       if (ret > 1 && ret <= sizeof(val))
> +               tx_num = ret;
> +
> +       ret = pmic_read(dev, reg, (uint8_t *)&val, tx_num);
>         if (ret < 0)
>                 return ret;
> -       byte = (ret & ~clr) | set;
>
> -       return pmic_reg_write(dev, reg, byte);
> +       val = (val & ~clr) | set;
> +       return pmic_write(dev, reg, (uint8_t *)&val, tx_num);
>  }
>
>  UCLASS_DRIVER(pmic) = {
> --
> 2.11.0
>

Regards,
Simon