[U-Boot] [PATCH v2 00/11] pmic: sandbox: Add support for MC34709 PMIC
Adding this device required some changes into the PMIC uclass. Most notable one was the support for 3 bytes r/w operations. Moreover, emulation and tests for this device has been added to sandbox.
Lukasz Majewski (11): pmic: fsl: Provide some more definitions for MC34708 PMIC pmic: fsl: Define number of bytes sent at once by MC34708 PMIC pmic: Add support for setting transmission length in uclass private data pmic: dm: Rewrite pmic_reg_{read|write|clrsetbits} to support 3 bytes transmissions pmic: dm: Add support for MC34708 for PMIC DM pmic: Rewrite the pmic command to not only work with single byte transmission sandbox: Rewrite i2c_pmic_emul.c to support PMIC with 3 bytes transmission sandbox: Enable support for MC34708 PMIC in DTS sandbox: Enable MC34708 PMIC support sandbox: tests: Exclude common test code (pmic_get) in test/dm/pmic.c sandbox: tests: Add tests for mc34708 PMIC device
arch/sandbox/dts/sandbox.dts | 4 ++ arch/sandbox/dts/sandbox64.dts | 4 ++ arch/sandbox/dts/sandbox_pmic.dtsi | 33 ++++++++++++ arch/sandbox/dts/test.dts | 4 ++ cmd/pmic.c | 31 +++++++----- configs/sandbox_defconfig | 1 + drivers/power/pmic/Kconfig | 7 +++ drivers/power/pmic/Makefile | 1 + drivers/power/pmic/i2c_pmic_emul.c | 45 ++++++++++++----- drivers/power/pmic/mc34708.c | 101 +++++++++++++++++++++++++++++++++++++ drivers/power/pmic/pmic-uclass.c | 54 +++++++++++++++----- include/fsl_pmic.h | 41 +++++++++++++++ include/power/pmic.h | 4 ++ test/dm/pmic.c | 68 ++++++++++++++++++++++++- 14 files changed, 358 insertions(+), 40 deletions(-) create mode 100644 drivers/power/pmic/mc34708.c
This commit adds some more defines for MC34708 PMIC.
Signed-off-by: Lukasz Majewski lukma@denx.de
---
Changes in v2: - None
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
This patch adds definition of the number of bytes sent at once by the MC34708 PMIC.
Signed-off-by: Lukasz Majewski lukma@denx.de
---
Changes in v2: - None
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
On 7 May 2018 at 06:25, Lukasz Majewski lukma@denx.de wrote:
This patch adds definition of the number of bytes sent at once by the MC34708 PMIC.
Signed-off-by: Lukasz Majewski lukma@denx.de
Changes in v2:
- None
include/fsl_pmic.h | 1 + 1 file changed, 1 insertion(+)
Reviewed-by: Simon Glass sjg@chromium.org
The struct dm_pmic_info's trans_len field stores the number of types to be transmitted per PMIC transfer.
Signed-off-by: Lukasz Majewski lukma@denx.de
---
Changes in v2: - New patch
drivers/power/pmic/pmic-uclass.c | 10 ++++++++++ include/power/pmic.h | 4 ++++ 2 files changed, 14 insertions(+)
diff --git a/drivers/power/pmic/pmic-uclass.c b/drivers/power/pmic/pmic-uclass.c index 5e8f6d6190..88669533bd 100644 --- a/drivers/power/pmic/pmic-uclass.c +++ b/drivers/power/pmic/pmic-uclass.c @@ -166,7 +166,17 @@ int pmic_clrsetbits(struct udevice *dev, uint reg, uint clr, uint set) return pmic_reg_write(dev, reg, byte); }
+static int pmic_pre_probe(struct udevice *dev) +{ + struct dm_pmic_info *pmic_info = dev_get_uclass_priv(dev); + + pmic_info->trans_len = 1; + return 0; +} + UCLASS_DRIVER(pmic) = { .id = UCLASS_PMIC, .name = "pmic", + .pre_probe = pmic_pre_probe, + .per_device_auto_alloc_size = sizeof(struct dm_pmic_info), }; diff --git a/include/power/pmic.h b/include/power/pmic.h index f2fe537fb7..0791c6aa2c 100644 --- a/include/power/pmic.h +++ b/include/power/pmic.h @@ -298,6 +298,10 @@ int pmic_reg_write(struct udevice *dev, uint reg, uint value); */ int pmic_clrsetbits(struct udevice *dev, uint reg, uint clr, uint set);
+struct dm_pmic_info { + uint trans_len; +}; + #endif /* CONFIG_DM_PMIC */
#ifdef CONFIG_POWER
On 7 May 2018 at 06:26, Lukasz Majewski lukma@denx.de wrote:
The struct dm_pmic_info's trans_len field stores the number of types to be transmitted per PMIC transfer.
Signed-off-by: Lukasz Majewski lukma@denx.de
Changes in v2:
- New patch
drivers/power/pmic/pmic-uclass.c | 10 ++++++++++ include/power/pmic.h | 4 ++++ 2 files changed, 14 insertions(+)
Reviewed-by: Simon Glass sjg@chromium.org
Please see below.
diff --git a/drivers/power/pmic/pmic-uclass.c b/drivers/power/pmic/pmic-uclass.c index 5e8f6d6190..88669533bd 100644 --- a/drivers/power/pmic/pmic-uclass.c +++ b/drivers/power/pmic/pmic-uclass.c @@ -166,7 +166,17 @@ int pmic_clrsetbits(struct udevice *dev, uint reg, uint clr, uint set) return pmic_reg_write(dev, reg, byte); }
+static int pmic_pre_probe(struct udevice *dev) +{
struct dm_pmic_info *pmic_info = dev_get_uclass_priv(dev);pmic_info->trans_len = 1;return 0;+}
UCLASS_DRIVER(pmic) = { .id = UCLASS_PMIC, .name = "pmic",
.pre_probe = pmic_pre_probe,.per_device_auto_alloc_size = sizeof(struct dm_pmic_info),}; diff --git a/include/power/pmic.h b/include/power/pmic.h index f2fe537fb7..0791c6aa2c 100644 --- a/include/power/pmic.h +++ b/include/power/pmic.h @@ -298,6 +298,10 @@ int pmic_reg_write(struct udevice *dev, uint reg, uint value); */ int pmic_clrsetbits(struct udevice *dev, uint reg, uint clr, uint set);
+struct dm_pmic_info {
uint trans_len;
Please add a comment for this. Also, how about uc_pmic_priv since this is device-specific private information owned by the uclass.
+};
#endif /* CONFIG_DM_PMIC */
#ifdef CONFIG_POWER
2.11.0
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
---
Changes in v2: - pmic_reg_* fixes to use uclass private structure
drivers/power/pmic/pmic-uclass.c | 44 +++++++++++++++++++++++++++++----------- 1 file changed, 32 insertions(+), 12 deletions(-)
diff --git a/drivers/power/pmic/pmic-uclass.c b/drivers/power/pmic/pmic-uclass.c index 88669533bd..c149b84a68 100644 --- a/drivers/power/pmic/pmic-uclass.c +++ b/drivers/power/pmic/pmic-uclass.c @@ -131,23 +131,36 @@ 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; + struct dm_pmic_info *pmic_info = dev_get_uclass_priv(dev); + int tx_num = pmic_info->trans_len; + 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); + if (tx_num < 1 || tx_num > sizeof(val)) { + printf("Wrong transmission size [%d]\n", tx_num); + return -EINVAL; + } + + 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 : byte; + return ret ? ret : val; }
int pmic_reg_write(struct udevice *dev, uint reg, uint value) { - u8 byte = value; + struct dm_pmic_info *pmic_info = dev_get_uclass_priv(dev); + int tx_num = pmic_info->trans_len; int ret;
- debug("%s: reg=%x, value=%x", __func__, reg, value); - ret = pmic_write(dev, reg, &byte, 1); + if (tx_num < 1 || tx_num > sizeof(value)) { + printf("Wrong transmission size [%d]\n", tx_num); + return -EINVAL; + } + + 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; @@ -155,15 +168,22 @@ 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; + struct dm_pmic_info *pmic_info = dev_get_uclass_priv(dev); + int tx_num = pmic_info->trans_len; + u32 val = 0; int ret;
- ret = pmic_reg_read(dev, reg); + if (tx_num < 1 || tx_num > sizeof(val)) { + printf("Wrong transmission size [%d]\n", tx_num); + return -EINVAL; + } + + 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); }
static int pmic_pre_probe(struct udevice *dev)
Hi Lukasz,
On 7 May 2018 at 06:26, 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
Changes in v2:
- pmic_reg_* fixes to use uclass private structure
drivers/power/pmic/pmic-uclass.c | 44 +++++++++++++++++++++++++++++----------- 1 file changed, 32 insertions(+), 12 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
with comments below
diff --git a/drivers/power/pmic/pmic-uclass.c b/drivers/power/pmic/pmic-uclass.c index 88669533bd..c149b84a68 100644 --- a/drivers/power/pmic/pmic-uclass.c +++ b/drivers/power/pmic/pmic-uclass.c @@ -131,23 +131,36 @@ 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;
struct dm_pmic_info *pmic_info = dev_get_uclass_priv(dev);int tx_num = pmic_info->trans_len;
Can you use trans_len instead of tx_num, which is confusing?
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);
if (tx_num < 1 || tx_num > sizeof(val)) {printf("Wrong transmission size [%d]\n", tx_num);
debug()
return -EINVAL;}debug("%s: reg=%x tx_num:%d", __func__, reg, tx_num);ret = pmic_read(dev, reg, (uint8_t *)&val, tx_num);
This is assuming little-endian, isn't it? I think you need to build up the number in a loop 8 bits at a time, or use some sort of unaligned function.
debug(", value=%x, ret=%d\n", val, ret);
return ret ? ret : byte;
return ret ? ret : val;}
int pmic_reg_write(struct udevice *dev, uint reg, uint value) {
u8 byte = value;
struct dm_pmic_info *pmic_info = dev_get_uclass_priv(dev);int tx_num = pmic_info->trans_len; int ret;
debug("%s: reg=%x, value=%x", __func__, reg, value);ret = pmic_write(dev, reg, &byte, 1);
if (tx_num < 1 || tx_num > sizeof(value)) {printf("Wrong transmission size [%d]\n", tx_num);return -EINVAL;}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;@@ -155,15 +168,22 @@ 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;
struct dm_pmic_info *pmic_info = dev_get_uclass_priv(dev);int tx_num = pmic_info->trans_len;u32 val = 0; int ret;
ret = pmic_reg_read(dev, reg);
if (tx_num < 1 || tx_num > sizeof(val)) {printf("Wrong transmission size [%d]\n", tx_num);return -EINVAL;}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);}
static int pmic_pre_probe(struct udevice *dev)
2.11.0
Regards, Simon
Hi Simon,
Hi Lukasz,
On 7 May 2018 at 06:26, 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
Changes in v2:
- pmic_reg_* fixes to use uclass private structure
drivers/power/pmic/pmic-uclass.c | 44 +++++++++++++++++++++++++++++----------- 1 file changed, 32 insertions(+), 12 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
with comments below
diff --git a/drivers/power/pmic/pmic-uclass.c b/drivers/power/pmic/pmic-uclass.c index 88669533bd..c149b84a68 100644 --- a/drivers/power/pmic/pmic-uclass.c +++ b/drivers/power/pmic/pmic-uclass.c @@ -131,23 +131,36 @@ 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;
struct dm_pmic_info *pmic_info = dev_get_uclass_priv(dev);int tx_num = pmic_info->trans_len;Can you use trans_len instead of tx_num, which is confusing?
Ok.
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);
if (tx_num < 1 || tx_num > sizeof(val)) {printf("Wrong transmission size [%d]\n", tx_num);debug()
return -EINVAL;}debug("%s: reg=%x tx_num:%d", __func__, reg, tx_num);ret = pmic_read(dev, reg, (uint8_t *)&val, tx_num);This is assuming little-endian, isn't it? I think you need to build up the number in a loop 8 bits at a time, or use some sort of unaligned function.
This is fixed in the latter patch (mc34708 PMIC support). [PATCH v2 05/11] pmic: dm: Add support for MC34708 for PMIC DM
The part:
buf[0] = buff[2]; buf[1] = buff[1]; buf[2] = buff[0];
Is responsible for byte swap in this particular device.
This code performs similar operation when compared with old PMIC driver: pmic_reg_write() in the drivers/power/power_i2c.c
However, the new DM model looks a bit better, as we do need to implement only the conversion relevant to the specific PMIC IC.
debug(", value=%x, ret=%d\n", val, ret);
return ret ? ret : byte;
return ret ? ret : val;}
int pmic_reg_write(struct udevice *dev, uint reg, uint value) {
u8 byte = value;
struct dm_pmic_info *pmic_info = dev_get_uclass_priv(dev);int tx_num = pmic_info->trans_len; int ret;
debug("%s: reg=%x, value=%x", __func__, reg, value);ret = pmic_write(dev, reg, &byte, 1);
if (tx_num < 1 || tx_num > sizeof(value)) {printf("Wrong transmission size [%d]\n", tx_num);return -EINVAL;}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;@@ -155,15 +168,22 @@ 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;
struct dm_pmic_info *pmic_info = dev_get_uclass_priv(dev);int tx_num = pmic_info->trans_len;u32 val = 0; int ret;
ret = pmic_reg_read(dev, reg);
if (tx_num < 1 || tx_num > sizeof(val)) {printf("Wrong transmission size [%d]\n", tx_num);return -EINVAL;}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);}
static int pmic_pre_probe(struct udevice *dev)
2.11.0
Regards, Simon
Best regards,
Lukasz Majewski
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de
This patch adds support for MC34708 PMIC, to be used with driver model (DM).
Signed-off-by: Lukasz Majewski lukma@denx.de
---
Changes in v2: - Support for uclass private data with trasfer length
drivers/power/pmic/Kconfig | 7 +++ drivers/power/pmic/Makefile | 1 + drivers/power/pmic/mc34708.c | 101 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 109 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..d9d1a41802 --- /dev/null +++ b/drivers/power/pmic/mc34708.c @@ -0,0 +1,101 @@ +/* + * Copyright (C) 2018 + * Lukasz Majewski, DENX Software Engineering, lukma@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_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) +{ + struct dm_pmic_info *pmic_info = dev_get_uclass_priv(dev); + + pmic_info->trans_len = MC34708_TRANSFER_SIZE; + + /* + * 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, +}; + +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, +};
Hi Lukasz,
On 7 May 2018 at 06:26, Lukasz Majewski lukma@denx.de wrote:
This patch adds support for MC34708 PMIC, to be used with driver model (DM).
Signed-off-by: Lukasz Majewski lukma@denx.de
Changes in v2:
- Support for uclass private data with trasfer length
drivers/power/pmic/Kconfig | 7 +++ drivers/power/pmic/Makefile | 1 + drivers/power/pmic/mc34708.c | 101 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 109 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_PMIChelpThis config enables implementation of driver-model pmic uclass featuresfor 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..d9d1a41802 --- /dev/null +++ b/drivers/power/pmic/mc34708.c @@ -0,0 +1,101 @@ +/*
- Copyright (C) 2018
- Lukasz Majewski, DENX Software Engineering, lukma@denx.de
- SPDX-License-Identifier: GPL-2.0+
- */
+#include <common.h> +#include <dm.h> +#include <errno.h> +#include <i2c.h> +#include <power/pmic.h>
should be at end
+#include <fsl_pmic.h>
should be above i2c.h
+DECLARE_GLOBAL_DATA_PTR;
+static int mc34708_reg_count(struct udevice *dev) +{
return PMIC_NUM_OF_REGS;+}
+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];
What is going on here? It deserves a comment at least.
ret = dm_i2c_write(dev, reg, buf, len);if (ret)printf("write error to device: %p register: %#x!", dev, reg);return ret;+}
Regards, Simon
On Mon, 14 May 2018 08:01:45 +1000 Simon Glass sjg@chromium.org wrote:
Hi Lukasz,
On 7 May 2018 at 06:26, Lukasz Majewski lukma@denx.de wrote:
This patch adds support for MC34708 PMIC, to be used with driver model (DM).
Signed-off-by: Lukasz Majewski lukma@denx.de
Changes in v2:
- Support for uclass private data with trasfer length
drivers/power/pmic/Kconfig | 7 +++ drivers/power/pmic/Makefile | 1 + drivers/power/pmic/mc34708.c | 101 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 109 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_PMIChelpThis config enables implementation of driver-model pmicuclass features
for PMIC MC34708. The driver implements read/writeoperations. + 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..d9d1a41802 --- /dev/null +++ b/drivers/power/pmic/mc34708.c @@ -0,0 +1,101 @@ +/*
- Copyright (C) 2018
- Lukasz Majewski, DENX Software Engineering, lukma@denx.de
- SPDX-License-Identifier: GPL-2.0+
- */
+#include <common.h> +#include <dm.h> +#include <errno.h> +#include <i2c.h> +#include <power/pmic.h>
should be at end
+#include <fsl_pmic.h>
should be above i2c.h
+DECLARE_GLOBAL_DATA_PTR;
+static int mc34708_reg_count(struct udevice *dev) +{
return PMIC_NUM_OF_REGS;+}
+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];What is going on here? It deserves a comment at least.
This is the data endianess conversion for this chip (as described in earlier reply). The upper layer (pmic-uclass) will receive data formatted in little endian.
This unification can be leveraged with pmic_read/write generic functions (as it is done latter).
ret = dm_i2c_write(dev, reg, buf, len);if (ret)printf("write error to device: %p register: %#x!",dev, reg); +
return ret;+}
Regards, Simon
Best regards,
Lukasz Majewski
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de
Up till now it was only possible to use 'pmic' command with a single byte transmission. The pmic_read|write functions has been replaced with ones, which don't need the transmission length as a parameter.
Due to that it is possible now to read data from PMICs transmitting more data than 1 byte at once (e.g. mc34708)
Signed-off-by: Lukasz Majewski lukma@denx.de
---
Changes in v2: - New patch
cmd/pmic.c | 31 +++++++++++++++++++------------ 1 file changed, 19 insertions(+), 12 deletions(-)
diff --git a/cmd/pmic.c b/cmd/pmic.c index 7bf23fb2a9..b3ab2ddd5d 100644 --- a/cmd/pmic.c +++ b/cmd/pmic.c @@ -76,8 +76,9 @@ static int do_list(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
static int do_dump(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) { + struct dm_pmic_info *pmic_info; struct udevice *dev; - uint8_t value; + char fmt[16]; uint reg; int ret;
@@ -87,12 +88,15 @@ static int do_dump(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) }
dev = currdev; - + pmic_info = dev_get_uclass_priv(dev); printf("Dump pmic: %s registers\n", dev->name);
+ sprintf(fmt, "%%%d.%dx ", pmic_info->trans_len * 2, + pmic_info->trans_len * 2); + for (reg = 0; reg < pmic_reg_count(dev); reg++) { - ret = pmic_read(dev, reg, &value, 1); - if (ret) { + ret = pmic_reg_read(dev, reg); + if (ret < 0) { printf("Can't read register: %d\n", reg); return failure(ret); } @@ -100,7 +104,7 @@ static int do_dump(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) if (!(reg % 16)) printf("\n0x%02x: ", reg);
- printf("%2.2x ", value); + printf(fmt, ret); } printf("\n");
@@ -109,9 +113,10 @@ static int do_dump(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
static int do_read(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) { + struct dm_pmic_info *pmic_info; struct udevice *dev; int regs, ret; - uint8_t value; + char fmt[24]; uint reg;
if (!currdev) { @@ -120,6 +125,7 @@ static int do_read(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) }
dev = currdev; + pmic_info = dev_get_uclass_priv(dev);
if (argc != 2) return CMD_RET_USAGE; @@ -131,13 +137,15 @@ static int do_read(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) return failure(-EFAULT); }
- ret = pmic_read(dev, reg, &value, 1); - if (ret) { + ret = pmic_reg_read(dev, reg); + if (ret < 0) { printf("Can't read PMIC register: %d!\n", reg); return failure(ret); }
- printf("0x%02x: 0x%2.2x\n", reg, value); + sprintf(fmt, "0x%%02x: 0x%%%d.%dx\n", pmic_info->trans_len * 2, + pmic_info->trans_len * 2); + printf(fmt, reg, ret);
return CMD_RET_SUCCESS; } @@ -145,9 +153,8 @@ static int do_read(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) static int do_write(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) { struct udevice *dev; + uint reg, value; int regs, ret; - uint8_t value; - uint reg;
if (!currdev) { printf("First, set the PMIC device!\n"); @@ -168,7 +175,7 @@ static int do_write(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
value = simple_strtoul(argv[2], NULL, 0);
- ret = pmic_write(dev, reg, &value, 1); + ret = pmic_reg_write(dev, reg, value); if (ret) { printf("Can't write PMIC register: %d!\n", reg); return failure(ret);
On 7 May 2018 at 06:26, Lukasz Majewski lukma@denx.de wrote:
Up till now it was only possible to use 'pmic' command with a single byte transmission. The pmic_read|write functions has been replaced with ones, which don't need the transmission length as a parameter.
Due to that it is possible now to read data from PMICs transmitting more data than 1 byte at once (e.g. mc34708)
Signed-off-by: Lukasz Majewski lukma@denx.de
Changes in v2:
- New patch
cmd/pmic.c | 31 +++++++++++++++++++------------ 1 file changed, 19 insertions(+), 12 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
This change enables support for MC34708 PMIC in sandbox. Now we can emulate the I2C transfers larger than 1 byte.
Notable changes for this driver:
- From now on the register number is not equal to index in the buffer, which emulates the PMIC registers
- The PMIC register's pool is now dynamically allocated up till 64 regs * 3 bytes each = 192 B
Signed-off-by: Lukasz Majewski lukma@denx.de
---
Changes in v2: - New patch
drivers/power/pmic/i2c_pmic_emul.c | 45 ++++++++++++++++++++++++++------------ 1 file changed, 31 insertions(+), 14 deletions(-)
diff --git a/drivers/power/pmic/i2c_pmic_emul.c b/drivers/power/pmic/i2c_pmic_emul.c index c58ebb8825..6f227a92f3 100644 --- a/drivers/power/pmic/i2c_pmic_emul.c +++ b/drivers/power/pmic/i2c_pmic_emul.c @@ -19,8 +19,11 @@ * @reg: PMICs registers array */ struct sandbox_i2c_pmic_plat_data { - u8 rw_reg; - u8 reg[SANDBOX_PMIC_REG_COUNT]; + u8 rw_reg, rw_idx; + u8 reg_count; + u8 trans_len; + u8 buf_size; + u8 *reg; };
static int sandbox_i2c_pmic_read_data(struct udevice *emul, uchar chip, @@ -28,16 +31,16 @@ static int sandbox_i2c_pmic_read_data(struct udevice *emul, uchar chip, { struct sandbox_i2c_pmic_plat_data *plat = dev_get_platdata(emul);
- if (plat->rw_reg + len > SANDBOX_PMIC_REG_COUNT) { + if (plat->rw_idx + len > plat->buf_size) { pr_err("Request exceeds PMIC register range! Max register: %#x", - SANDBOX_PMIC_REG_COUNT); + plat->reg_count); return -EFAULT; }
- debug("Read PMIC: %#x at register: %#x count: %d\n", - (unsigned)chip & 0xff, plat->rw_reg, len); + debug("Read PMIC: %#x at register: %#x idx: %#x count: %d\n", + (unsigned int)chip & 0xff, plat->rw_reg, plat->rw_idx, len);
- memcpy(buffer, &plat->reg[plat->rw_reg], len); + memcpy(buffer, plat->reg + plat->rw_idx, len);
return 0; } @@ -54,9 +57,10 @@ static int sandbox_i2c_pmic_write_data(struct udevice *emul, uchar chip,
/* Set PMIC register for I/O */ plat->rw_reg = *buffer; + plat->rw_idx = plat->rw_reg * plat->trans_len;
- debug("Write PMIC: %#x at register: %#x count: %d\n", - (unsigned)chip & 0xff, plat->rw_reg, len); + debug("Write PMIC: %#x at register: %#x idx: %#x count: %d\n", + (unsigned int)chip & 0xff, plat->rw_reg, plat->rw_idx, len);
/* For read operation, set (write) only chip reg */ if (next_is_read) @@ -65,12 +69,12 @@ static int sandbox_i2c_pmic_write_data(struct udevice *emul, uchar chip, buffer++; len--;
- if (plat->rw_reg + len > SANDBOX_PMIC_REG_COUNT) { + if (plat->rw_idx + len > plat->buf_size) { pr_err("Request exceeds PMIC register range! Max register: %#x", - SANDBOX_PMIC_REG_COUNT); + plat->reg_count); }
- memcpy(&plat->reg[plat->rw_reg], buffer, len); + memcpy(plat->reg + plat->rw_idx, buffer, len);
return 0; } @@ -101,20 +105,33 @@ static int sandbox_i2c_pmic_xfer(struct udevice *emul, struct i2c_msg *msg, static int sandbox_i2c_pmic_ofdata_to_platdata(struct udevice *emul) { struct sandbox_i2c_pmic_plat_data *plat = dev_get_platdata(emul); + struct udevice *pmic_dev = dev_get_parent(emul); + struct dm_pmic_info *pmic_info = dev_get_uclass_priv(pmic_dev); const u8 *reg_defaults;
debug("%s:%d Setting PMIC default registers\n", __func__, __LINE__); + plat->reg_count = pmic_reg_count(pmic_dev); + plat->trans_len = pmic_info->trans_len; + plat->buf_size = plat->reg_count * plat->trans_len; + + plat->reg = calloc(1, plat->buf_size); + if (!plat->reg) { + pr_err("Canot allocate memory (%d B) for PMIC I2C emulation!\n", + plat->buf_size); + return -ENOMEM; + }
reg_defaults = dev_read_u8_array_ptr(emul, "reg-defaults", - SANDBOX_PMIC_REG_COUNT); + plat->buf_size);
if (!reg_defaults) { pr_err("Property "reg-defaults" not found for device: %s!", emul->name); + free(plat->reg); return -EINVAL; }
- memcpy(&plat->reg, reg_defaults, SANDBOX_PMIC_REG_COUNT); + memcpy(plat->reg, reg_defaults, plat->buf_size);
return 0; }
Hi Lukasz,
On 7 May 2018 at 06:26, Lukasz Majewski lukma@denx.de wrote:
This change enables support for MC34708 PMIC in sandbox. Now we can emulate the I2C transfers larger than 1 byte.
Notable changes for this driver:
From now on the register number is not equal to index in the buffer, which emulates the PMIC registers
The PMIC register's pool is now dynamically allocated up till 64 regs * 3 bytes each = 192 B
Signed-off-by: Lukasz Majewski lukma@denx.de
Changes in v2:
- New patch
drivers/power/pmic/i2c_pmic_emul.c | 45 ++++++++++++++++++++++++++------------ 1 file changed, 31 insertions(+), 14 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
Comments below
diff --git a/drivers/power/pmic/i2c_pmic_emul.c b/drivers/power/pmic/i2c_pmic_emul.c index c58ebb8825..6f227a92f3 100644 --- a/drivers/power/pmic/i2c_pmic_emul.c +++ b/drivers/power/pmic/i2c_pmic_emul.c @@ -19,8 +19,11 @@
- @reg: PMICs registers array
*/ struct sandbox_i2c_pmic_plat_data {
u8 rw_reg;u8 reg[SANDBOX_PMIC_REG_COUNT];
u8 rw_reg, rw_idx;u8 reg_count;u8 trans_len;u8 buf_size;u8 *reg;};
static int sandbox_i2c_pmic_read_data(struct udevice *emul, uchar chip, @@ -28,16 +31,16 @@ static int sandbox_i2c_pmic_read_data(struct udevice *emul, uchar chip, { struct sandbox_i2c_pmic_plat_data *plat = dev_get_platdata(emul);
if (plat->rw_reg + len > SANDBOX_PMIC_REG_COUNT) {
if (plat->rw_idx + len > plat->buf_size) { pr_err("Request exceeds PMIC register range! Max register: %#x",
SANDBOX_PMIC_REG_COUNT);
plat->reg_count); return -EFAULT; }
debug("Read PMIC: %#x at register: %#x count: %d\n",(unsigned)chip & 0xff, plat->rw_reg, len);
debug("Read PMIC: %#x at register: %#x idx: %#x count: %d\n",(unsigned int)chip & 0xff, plat->rw_reg, plat->rw_idx, len);
memcpy(buffer, &plat->reg[plat->rw_reg], len);
memcpy(buffer, plat->reg + plat->rw_idx, len);
Why change this?
return 0;} @@ -54,9 +57,10 @@ static int sandbox_i2c_pmic_write_data(struct udevice *emul, uchar chip,
/* Set PMIC register for I/O */ plat->rw_reg = *buffer;
plat->rw_idx = plat->rw_reg * plat->trans_len;
debug("Write PMIC: %#x at register: %#x count: %d\n",(unsigned)chip & 0xff, plat->rw_reg, len);
debug("Write PMIC: %#x at register: %#x idx: %#x count: %d\n",(unsigned int)chip & 0xff, plat->rw_reg, plat->rw_idx, len); /* For read operation, set (write) only chip reg */ if (next_is_read)@@ -65,12 +69,12 @@ static int sandbox_i2c_pmic_write_data(struct udevice *emul, uchar chip, buffer++; len--;
if (plat->rw_reg + len > SANDBOX_PMIC_REG_COUNT) {
if (plat->rw_idx + len > plat->buf_size) { pr_err("Request exceeds PMIC register range! Max register: %#x",
SANDBOX_PMIC_REG_COUNT);
plat->reg_count); }
memcpy(&plat->reg[plat->rw_reg], buffer, len);
memcpy(plat->reg + plat->rw_idx, buffer, len);
and this?
return 0;} @@ -101,20 +105,33 @@ static int sandbox_i2c_pmic_xfer(struct udevice *emul, struct i2c_msg *msg, static int sandbox_i2c_pmic_ofdata_to_platdata(struct udevice *emul) { struct sandbox_i2c_pmic_plat_data *plat = dev_get_platdata(emul);
struct udevice *pmic_dev = dev_get_parent(emul);struct dm_pmic_info *pmic_info = dev_get_uclass_priv(pmic_dev); const u8 *reg_defaults; debug("%s:%d Setting PMIC default registers\n", __func__, __LINE__);plat->reg_count = pmic_reg_count(pmic_dev);plat->trans_len = pmic_info->trans_len;plat->buf_size = plat->reg_count * plat->trans_len;plat->reg = calloc(1, plat->buf_size);if (!plat->reg) {pr_err("Canot allocate memory (%d B) for PMIC I2C emulation!\n",plat->buf_size);
debug()
return -ENOMEM;} reg_defaults = dev_read_u8_array_ptr(emul, "reg-defaults",
SANDBOX_PMIC_REG_COUNT);
plat->buf_size); if (!reg_defaults) { pr_err("Property \"reg-defaults\" not found for device: %s!", emul->name);free(plat->reg); return -EINVAL; }
memcpy(&plat->reg, reg_defaults, SANDBOX_PMIC_REG_COUNT);
memcpy(plat->reg, reg_defaults, plat->buf_size); return 0;}
2.11.0
Regards, Simon
Hi Simon,
Hi Lukasz,
On 7 May 2018 at 06:26, Lukasz Majewski lukma@denx.de wrote:
This change enables support for MC34708 PMIC in sandbox. Now we can emulate the I2C transfers larger than 1 byte.
Notable changes for this driver:
- From now on the register number is not equal to index in the
buffer, which emulates the PMIC registers
- The PMIC register's pool is now dynamically allocated up till 64 regs * 3 bytes each = 192 B
Signed-off-by: Lukasz Majewski lukma@denx.de
Changes in v2:
- New patch
drivers/power/pmic/i2c_pmic_emul.c | 45 ++++++++++++++++++++++++++------------ 1 file changed, 31 insertions(+), 14 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
Comments below
diff --git a/drivers/power/pmic/i2c_pmic_emul.c b/drivers/power/pmic/i2c_pmic_emul.c index c58ebb8825..6f227a92f3 100644 --- a/drivers/power/pmic/i2c_pmic_emul.c +++ b/drivers/power/pmic/i2c_pmic_emul.c @@ -19,8 +19,11 @@
- @reg: PMICs registers array
*/ struct sandbox_i2c_pmic_plat_data {
u8 rw_reg;u8 reg[SANDBOX_PMIC_REG_COUNT];
u8 rw_reg, rw_idx;u8 reg_count;u8 trans_len;u8 buf_size;u8 *reg;};
static int sandbox_i2c_pmic_read_data(struct udevice *emul, uchar chip, @@ -28,16 +31,16 @@ static int sandbox_i2c_pmic_read_data(struct udevice *emul, uchar chip, { struct sandbox_i2c_pmic_plat_data *plat = dev_get_platdata(emul);
if (plat->rw_reg + len > SANDBOX_PMIC_REG_COUNT) {
if (plat->rw_idx + len > plat->buf_size) { pr_err("Request exceeds PMIC register range! Maxregister: %#x",
SANDBOX_PMIC_REG_COUNT);
plat->reg_count); return -EFAULT; }
debug("Read PMIC: %#x at register: %#x count: %d\n",(unsigned)chip & 0xff, plat->rw_reg, len);
debug("Read PMIC: %#x at register: %#x idx: %#x count:%d\n",
(unsigned int)chip & 0xff, plat->rw_reg,plat->rw_idx, len);
memcpy(buffer, &plat->reg[plat->rw_reg], len);
memcpy(buffer, plat->reg + plat->rw_idx, len);Why change this?
For the sandbox_pmic(), which emulates single byte transmission, the number of registers (rw_reg) is also the index in the pool emulating the device.
With support for devices having 3 bytes sent at once, the register number (rw_req) cannot be used as index anymore.
return 0;} @@ -54,9 +57,10 @@ static int sandbox_i2c_pmic_write_data(struct udevice *emul, uchar chip,
/* Set PMIC register for I/O */ plat->rw_reg = *buffer;
plat->rw_idx = plat->rw_reg * plat->trans_len;
debug("Write PMIC: %#x at register: %#x count: %d\n",(unsigned)chip & 0xff, plat->rw_reg, len);
debug("Write PMIC: %#x at register: %#x idx: %#x count:%d\n",
(unsigned int)chip & 0xff, plat->rw_reg,plat->rw_idx, len);
/* For read operation, set (write) only chip reg */ if (next_is_read)@@ -65,12 +69,12 @@ static int sandbox_i2c_pmic_write_data(struct udevice *emul, uchar chip, buffer++; len--;
if (plat->rw_reg + len > SANDBOX_PMIC_REG_COUNT) {
if (plat->rw_idx + len > plat->buf_size) { pr_err("Request exceeds PMIC register range! Maxregister: %#x",
SANDBOX_PMIC_REG_COUNT);
plat->reg_count); }
memcpy(&plat->reg[plat->rw_reg], buffer, len);
memcpy(plat->reg + plat->rw_idx, buffer, len);and this?
return 0;} @@ -101,20 +105,33 @@ static int sandbox_i2c_pmic_xfer(struct udevice *emul, struct i2c_msg *msg, static int sandbox_i2c_pmic_ofdata_to_platdata(struct udevice *emul) { struct sandbox_i2c_pmic_plat_data *plat = dev_get_platdata(emul);
struct udevice *pmic_dev = dev_get_parent(emul);struct dm_pmic_info *pmic_info =dev_get_uclass_priv(pmic_dev); const u8 *reg_defaults;
debug("%s:%d Setting PMIC default registers\n", __func__,__LINE__);
plat->reg_count = pmic_reg_count(pmic_dev);plat->trans_len = pmic_info->trans_len;plat->buf_size = plat->reg_count * plat->trans_len;plat->reg = calloc(1, plat->buf_size);if (!plat->reg) {pr_err("Canot allocate memory (%d B) for PMIC I2Cemulation!\n",
plat->buf_size);debug()
Ok
return -ENOMEM;} reg_defaults = dev_read_u8_array_ptr(emul, "reg-defaults",
SANDBOX_PMIC_REG_COUNT);
plat->buf_size); if (!reg_defaults) { pr_err("Property \"reg-defaults\" not found fordevice: %s!", emul->name);
free(plat->reg); return -EINVAL; }
memcpy(&plat->reg, reg_defaults, SANDBOX_PMIC_REG_COUNT);
memcpy(plat->reg, reg_defaults, plat->buf_size); return 0;}
2.11.0
Regards, Simon
Best regards,
Lukasz Majewski
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de
This commit also provides the default values of the emulated MC34708 PMIC internal registers content.
Signed-off-by: Lukasz Majewski lukma@denx.de
---
Changes in v2: - New patch
arch/sandbox/dts/sandbox.dts | 4 ++++ arch/sandbox/dts/sandbox64.dts | 4 ++++ arch/sandbox/dts/sandbox_pmic.dtsi | 33 +++++++++++++++++++++++++++++++++ arch/sandbox/dts/test.dts | 4 ++++ 4 files changed, 45 insertions(+)
diff --git a/arch/sandbox/dts/sandbox.dts b/arch/sandbox/dts/sandbox.dts index 1fb8225fbb..b187b6fac1 100644 --- a/arch/sandbox/dts/sandbox.dts +++ b/arch/sandbox/dts/sandbox.dts @@ -115,6 +115,10 @@ sandbox_pmic: sandbox_pmic { reg = <0x40>; }; + + mc34708_pmic: mc34708_pmic { + reg = <0x41>; + }; };
lcd { diff --git a/arch/sandbox/dts/sandbox64.dts b/arch/sandbox/dts/sandbox64.dts index d6efc011de..a8f0efc57e 100644 --- a/arch/sandbox/dts/sandbox64.dts +++ b/arch/sandbox/dts/sandbox64.dts @@ -115,6 +115,10 @@ sandbox_pmic: sandbox_pmic { reg = <0x40>; }; + + mc34708_pmic: mc34708_pmic { + reg = <0x41>; + }; };
lcd { diff --git a/arch/sandbox/dts/sandbox_pmic.dtsi b/arch/sandbox/dts/sandbox_pmic.dtsi index acb4799396..37a171c65e 100644 --- a/arch/sandbox/dts/sandbox_pmic.dtsi +++ b/arch/sandbox/dts/sandbox_pmic.dtsi @@ -82,3 +82,36 @@ regulator-max-microvolt = <1500000>; }; }; + +&mc34708_pmic { + compatible = "fsl,mc34708"; + + pmic_emul { + compatible = "sandbox,i2c-pmic"; + + reg-defaults = /bits/ 8 < + 0x00 0x80 0x08 0xff 0xff 0xff 0x2e 0x01 0x08 + 0x40 0x80 0x81 0x5f 0xff 0xfb 0x1e 0x80 0x18 + 0x00 0x00 0x0e 0x00 0x00 0x14 0x00 0x00 0x00 + 0x00 0x00 0x20 0x00 0x01 0x3a 0x00 0x00 0x00 + 0x00 0x00 0x00 0x00 0x00 0x40 0x00 0x00 0x00 + 0x42 0x21 0x00 0x00 0x00 0x00 0x00 0x00 0x00 + 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x30 0x5f + 0x01 0xff 0xff 0x00 0x00 0x00 0x00 0x7f 0xff + 0x92 0x49 0x24 0x59 0x6d 0x34 0x18 0xc1 0x8c + 0x00 0x60 0x18 0x51 0x48 0x45 0x14 0x51 0x45 + 0x00 0x06 0x32 0x00 0x00 0x00 0x06 0x9c 0x99 + 0x00 0x38 0x0a 0x00 0x38 0x0a 0x00 0x38 0x0a + 0x00 0x38 0x0a 0x84 0x00 0x00 0x00 0x00 0x00 + 0x80 0x90 0x8f 0xf8 0x00 0x04 0x00 0x00 0x00 + 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 + 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 + 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 + 0x01 0x31 0x7e 0x2b 0x03 0xfd 0xc0 0x36 0x1b + 0x60 0x06 0x00 0x00 0x00 0x00 0x00 0x00 0x00 + 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 + 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x00 + 0x00 0x00 0x00 + >; + }; +}; diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts index 06d0e8ce85..6e45ec8fb8 100644 --- a/arch/sandbox/dts/test.dts +++ b/arch/sandbox/dts/test.dts @@ -227,6 +227,10 @@ sandbox_pmic: sandbox_pmic { reg = <0x40>; }; + + mc34708_pmic: mc34708_pmic { + reg = <0x41>; + }; };
adc@0 {
Hi Lukasz,
On Sun, May 6, 2018 at 5:26 PM, Lukasz Majewski lukma@denx.de wrote:
diff --git a/arch/sandbox/dts/sandbox.dts b/arch/sandbox/dts/sandbox.dts index 1fb8225fbb..b187b6fac1 100644 --- a/arch/sandbox/dts/sandbox.dts +++ b/arch/sandbox/dts/sandbox.dts @@ -115,6 +115,10 @@ sandbox_pmic: sandbox_pmic { reg = <0x40>; };
mc34708_pmic: mc34708_pmic {reg = <0x41>;};
I know you are following the current style of this file, but this looks incorrect.
According to Devicetree Specification v0.2 document:
"The name of a node should be somewhat generic, reflecting the function of the device and not its precise programming model."
Also, the reg property needs to have a corresponding unit address.
It would better to rewrite this as:
mc34708: pmic@41 { reg = <0x41> };
Hi Fabio,
Hi Lukasz,
On Sun, May 6, 2018 at 5:26 PM, Lukasz Majewski lukma@denx.de wrote:
diff --git a/arch/sandbox/dts/sandbox.dts b/arch/sandbox/dts/sandbox.dts index 1fb8225fbb..b187b6fac1 100644 --- a/arch/sandbox/dts/sandbox.dts +++ b/arch/sandbox/dts/sandbox.dts @@ -115,6 +115,10 @@ sandbox_pmic: sandbox_pmic { reg = <0x40>; };
mc34708_pmic: mc34708_pmic {reg = <0x41>;};I know you are following the current style of this file, but this looks incorrect.
According to Devicetree Specification v0.2 document:
"The name of a node should be somewhat generic, reflecting the function of the device and not its precise programming model."
Also, the reg property needs to have a corresponding unit address.
It would better to rewrite this as:
mc34708: pmic@41 { reg = <0x41> };
Yes, you are right. I've blindly followed the current (wrong) code.
I'm now waiting for comments from Simon and will fix this in v3.
Thanks for review.
Best regards,
Lukasz Majewski
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de
On 7 May 2018 at 06:26, Lukasz Majewski lukma@denx.de wrote:
This commit also provides the default values of the emulated MC34708 PMIC internal registers content.
Signed-off-by: Lukasz Majewski lukma@denx.de
Changes in v2:
- New patch
arch/sandbox/dts/sandbox.dts | 4 ++++ arch/sandbox/dts/sandbox64.dts | 4 ++++ arch/sandbox/dts/sandbox_pmic.dtsi | 33 +++++++++++++++++++++++++++++++++ arch/sandbox/dts/test.dts | 4 ++++ 4 files changed, 45 insertions(+)
Reviewed-by: Simon Glass sjg@chromium.org
This MC34708 PMIC is somewhat special - it used single transfers (R/W) with 3 bytes size - up till now u-boot's PMICs only used 1 byte.
Signed-off-by: Lukasz Majewski lukma@denx.de
---
Changes in v2: - New patch
configs/sandbox_defconfig | 1 + 1 file changed, 1 insertion(+)
diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig index c1cdd59c54..2fc84a16c9 100644 --- a/configs/sandbox_defconfig +++ b/configs/sandbox_defconfig @@ -142,6 +142,7 @@ CONFIG_DM_PMIC=y CONFIG_PMIC_ACT8846=y CONFIG_DM_PMIC_PFUZE100=y CONFIG_DM_PMIC_MAX77686=y +CONFIG_DM_PMIC_MC34708=y CONFIG_PMIC_PM8916=y CONFIG_PMIC_RK8XX=y CONFIG_PMIC_S2MPS11=y
On 7 May 2018 at 06:26, Lukasz Majewski lukma@denx.de wrote:
This MC34708 PMIC is somewhat special - it used single transfers (R/W) with 3 bytes size - up till now u-boot's PMICs only used 1 byte.
U-Boot's
Signed-off-by: Lukasz Majewski lukma@denx.de
Changes in v2:
- New patch
configs/sandbox_defconfig | 1 + 1 file changed, 1 insertion(+)
Reviewed-by: Simon Glass sjg@chromium.org
The common code can be excluded to be reused by tests for other PMIC.
Signed-off-by: Lukasz Majewski lukma@denx.de
---
Changes in v2: - New patch
test/dm/pmic.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/test/dm/pmic.c b/test/dm/pmic.c index 0e5d671924..83994f5f9f 100644 --- a/test/dm/pmic.c +++ b/test/dm/pmic.c @@ -22,9 +22,9 @@ #include <test/ut.h>
/* Test PMIC get method */ -static int dm_test_power_pmic_get(struct unit_test_state *uts) + +static inline int power_pmic_get(struct unit_test_state *uts, char *name) { - const char *name = "sandbox_pmic"; struct udevice *dev;
ut_assertok(pmic_get(name, &dev)); @@ -35,6 +35,14 @@ static int dm_test_power_pmic_get(struct unit_test_state *uts)
return 0; } + +/* Test PMIC get method */ +static int dm_test_power_pmic_get(struct unit_test_state *uts) +{ + power_pmic_get(uts, "sandbox_pmic"); + + return 0; +} DM_TEST(dm_test_power_pmic_get, DM_TESTF_SCAN_FDT);
/* Test PMIC I/O */
On 7 May 2018 at 06:26, Lukasz Majewski lukma@denx.de wrote:
The common code can be excluded to be reused by tests for other PMIC.
Signed-off-by: Lukasz Majewski lukma@denx.de
Changes in v2:
- New patch
test/dm/pmic.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
Following tests has been added for mc34708 device:
- get_test for mc34708 PMIC - Check if proper number of registers is read - Check if default (emulated via i2c device) value is properly read - Check if value write/read operation is correct - Perform tests to check if pmic_clrsetbits() is working correctly
Signed-off-by: Lukasz Majewski lukma@denx.de
---
Changes in v2: - New patch
test/dm/pmic.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+)
diff --git a/test/dm/pmic.c b/test/dm/pmic.c index 83994f5f9f..30513a9e41 100644 --- a/test/dm/pmic.c +++ b/test/dm/pmic.c @@ -20,6 +20,7 @@ #include <power/pmic.h> #include <power/sandbox_pmic.h> #include <test/ut.h> +#include <fsl_pmic.h>
/* Test PMIC get method */
@@ -45,6 +46,16 @@ static int dm_test_power_pmic_get(struct unit_test_state *uts) } DM_TEST(dm_test_power_pmic_get, DM_TESTF_SCAN_FDT);
+/* PMIC get method - MC34708 - for 3 bytes transmission */ +static int dm_test_power_pmic_mc34708_get(struct unit_test_state *uts) +{ + power_pmic_get(uts, "mc34708_pmic"); + + return 0; +} + +DM_TEST(dm_test_power_pmic_mc34708_get, DM_TESTF_SCAN_FDT); + /* Test PMIC I/O */ static int dm_test_power_pmic_io(struct unit_test_state *uts) { @@ -73,3 +84,48 @@ static int dm_test_power_pmic_io(struct unit_test_state *uts) return 0; } DM_TEST(dm_test_power_pmic_io, DM_TESTF_SCAN_FDT); + +#define MC34708_PMIC_REG_COUNT 64 +#define MC34708_PMIC_TEST_VAL 0x125534 +static int dm_test_power_pmic_mc34708_regs_check(struct unit_test_state *uts) +{ + struct udevice *dev; + int reg_count; + + ut_assertok(pmic_get("mc34708_pmic", &dev)); + + /* Check number of PMIC registers */ + reg_count = pmic_reg_count(dev); + ut_asserteq(reg_count, MC34708_PMIC_REG_COUNT); + + return 0; +} + +DM_TEST(dm_test_power_pmic_mc34708_regs_check, DM_TESTF_SCAN_FDT); + +static int dm_test_power_pmic_mc34708_rw_val(struct unit_test_state *uts) +{ + struct udevice *dev; + int val; + + ut_assertok(pmic_get("mc34708_pmic", &dev)); + + /* Check if single 3 byte read is successful */ + val = pmic_reg_read(dev, REG_POWER_CTL2); + ut_asserteq(val, 0x422100); + + /* Check if RW works */ + val = 0; + ut_assertok(pmic_reg_write(dev, REG_RTC_TIME, val)); + ut_assertok(pmic_reg_write(dev, REG_RTC_TIME, MC34708_PMIC_TEST_VAL)); + val = pmic_reg_read(dev, REG_RTC_TIME); + ut_asserteq(val, MC34708_PMIC_TEST_VAL); + + pmic_clrsetbits(dev, REG_POWER_CTL2, 0x3 << 8, 1 << 9); + val = pmic_reg_read(dev, REG_POWER_CTL2); + ut_asserteq(val, (0x422100 & ~(0x3 << 8)) | (1 << 9)); + + return 0; +} + +DM_TEST(dm_test_power_pmic_mc34708_rw_val, DM_TESTF_SCAN_FDT);
On 7 May 2018 at 06:26, Lukasz Majewski lukma@denx.de wrote:
Following tests has been added for mc34708 device:
- get_test for mc34708 PMIC
- Check if proper number of registers is read
- Check if default (emulated via i2c device) value is properly read
- Check if value write/read operation is correct
- Perform tests to check if pmic_clrsetbits() is working correctly
Signed-off-by: Lukasz Majewski lukma@denx.de
Changes in v2:
- New patch
test/dm/pmic.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+)
Reviewed-by: Simon Glass sjg@chromium.org
participants (3)
-
Fabio Estevam -
Lukasz Majewski -
Simon Glass