pmic: sandbox: Add support for MC34709 PMIC
v2 cover 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
Signature · 1 line
2.11.0
PATCH v2 1/11 pmic: fsl: Provide some more definitions for MC34708 PMIC
This commit adds some more defines for MC34708 PMIC.
---
Changes in v2:
- None
include/fsl_pmic.h | 40 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+)
include/fsl_pmic.h +40 -0
@@ -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
Signature · 1 line
2.11.0
PATCH v2 2/11 pmic: fsl: Define number of bytes sent at once by MC34708 PMIC
In reply to Lukasz MajewskiThis patch adds definition of the number of bytes sent at once by the MC34708 PMIC.
---
Changes in v2:
- None
include/fsl_pmic.h | 1 + 1 file changed, 1 insertion(+)
include/fsl_pmic.h +1 -0
@@ -157,4 +157,5 @@ enum {#define SW_MODE_PWMPFM 0x0d#define SW_MODE_PFMPFM 0x0f +#define MC34708_TRANSFER_SIZE 3#endif
Signature · 1 line
2.11.0
PATCH v2 2/11 pmic: fsl: Define number of bytes sent at once by MC34708 PMIC
Quoted 5 lines · Lukasz Majewski
This patch adds definition of the number of bytes sent at once by the MC34708 PMIC.
Signed-off-by Lukasz Majewski <[email protected]>
Quoted 7 lines
---
Changes in v2:
- Noneinclude/fsl_pmic.h | 1 + 1 file changed, 1 insertion(+)
PATCH v2 3/11 pmic: Add support for setting transmission length in uclass private data
In reply to Lukasz MajewskiThe struct dm_pmic_info's trans_len field stores the number of types to be transmitted per PMIC transfer.
---
Changes in v2:
- New patch
drivers/power/pmic/pmic-uclass.c | 10 ++++++++++ include/power/pmic.h | 4 ++++ 2 files changed, 14 insertions(+)
drivers/power/pmic/pmic-uclass.c +10 -0
@@ -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 ), };
include/power/pmic.h +4 -0
@@ -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
Signature · 1 line
2.11.0
PATCH v2 3/11 pmic: Add support for setting transmission length in uclass private data
Quoted 5 lines · Lukasz Majewski
The struct dm_pmic_info's
trans_lenfield stores the number of types to be transmitted per PMIC transfer.Signed-off-by Lukasz Majewski <[email protected]>
Quoted 9 lines
---
Changes in v2:
- New patchdrivers/power/pmic/pmic-uclass.c | 10 ++++++++++ include/power/pmic.h | 4 ++++ 2 files changed, 14 insertions(+)
Please see below.
Quoted 27 lines
drivers/power/pmic/pmic-uclass.c +10
-0@@ -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 ), };include/power/pmic.h +0
-0
include/power/pmic.h int pmic_reg_write(struct udevice *dev, uint reg, uint value); · line 302
Quoted 5 lines
include/power/pmic.h +2
-0@@ -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.
Quoted 8 lines
include/power/pmic.h +2
-1+}; +#endif /* CONFIG_DM_PMIC */ #ifdef CONFIG_POWER --2.11.0
PATCH v2 4/11 pmic: dm: Rewrite pmic_reg_{read|write|clrsetbits} to support 3 bytes transmissions
In reply to Lukasz MajewskiThis commit provides support for transmissions larger than 1 byte for PMIC devices used with DM (e.g. MC34708 from NXP).
---
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(-)
drivers/power/pmic/pmic-uclass.c +32 -12
@@ -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)
Signature · 1 line
2.11.0
PATCH v2 4/11 pmic: dm: Rewrite pmic_reg_{read|write|clrsetbits} to support 3 bytes transmissions
Hi Lukasz,
Quoted 5 lines · 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 <[email protected]>
Quoted 8 lines
---
Changes in v2:
- pmic_reg_* fixes to use uclass private structuredrivers/power/pmic/pmic-uclass.c | 44 +++++++++++++++++++++++++++++----------- 1 file changed, 32 insertions(+), 12 deletions(-)
with comments below
drivers/power/pmic/pmic-uclass.c int pmic_write(struct udevice *dev, uint reg, const uint8_t *buffer, int len) · line 135
Quoted 11 lines
drivers/power/pmic/pmic-uclass.c +2
-1@@ -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?
drivers/power/pmic/pmic-uclass.c int pmic_write(struct udevice *dev, uint reg, const uint8_t *buffer, int len) · line 140
Quoted 8 lines
drivers/power/pmic/pmic-uclass.c +3
-3+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()
drivers/power/pmic/pmic-uclass.c int pmic_write(struct udevice *dev, uint reg, const uint8_t *buffer, int len) · line 145
Quoted 5 lines
drivers/power/pmic/pmic-uclass.c +5
-0+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.
Quoted 53 lines
drivers/power/pmic/pmic-uclass.c +22
-9+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) --
Quoted 2 lines
2.11.0
Regards,
Simon
PATCH v2 4/11 pmic: dm: Rewrite pmic_reg_{read|write|clrsetbits} to support 3 bytes transmissions
Hi Simon,
Quoted 28 lines
Hi Lukasz,
Quoted 5 lines · 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 <[email protected]>Quoted 8 lines
---
Changes in v2:
- pmic_reg_* fixes to use uclass private structuredrivers/power/pmic/pmic-uclass.c | 44+++++++++++++++++++++++++++++----------- 1 file changed, 32 insertions(+), 12 deletions(-)
Reviewed-by Simon Glass <[email protected]>with comments below
Quoted 7 lines
(quoted diff) +0
-0
b/drivers/power/pmic/pmic-uclass.cindex88669533bd..c149b84a68100644 ---a/drivers/power/pmic/pmic-uclass.c
+++b/drivers/power/pmic/pmic-uclass.c(quoted diff) +0
-0@@ -131,23 +131,36 @@ int pmic_write(struct udevice *dev, uint reg, const uint8_t *buffer, int len)
Quoted 7 lines
drivers/power/pmic/pmic-uclass.c int pmic_write(struct udevice *dev, uint reg, const uint8_t *buffer, int len) · line 135
Quoted 5 lines
drivers/power/pmic/pmic-uclass.c +2
-1int 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_leninstead oftx_num, which is confusing?
Ok.
Quoted 18 lines
drivers/power/pmic/pmic-uclass.c int pmic_write(struct udevice *dev, uint reg, const uint8_t *buffer, int len) · line 140
Quoted 9 lines
drivers/power/pmic/pmic-uclass.c +3
-3+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()drivers/power/pmic/pmic-uclass.c int pmic_write(struct udevice *dev, uint reg, const uint8_t *buffer, int len) · line 145
Quoted 5 lines
drivers/power/pmic/pmic-uclass.c +5
-0+return -EINVAL ; + } + +debug ("%s: reg=%x tx_num:%d" , __func__, reg, tx_num); + ret =pmic_read (dev, reg, (uint8_t *)&val, tx_num);
Quoted 3 lines
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.
Quoted 54 lines
Quoted 54 lines
drivers/power/pmic/pmic-uclass.c +11
-4+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;drivers/power/pmic/pmic-uclass.c +11
-4@@ -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); }
Quoted 7 lines
Quoted 4 lines
static int
pmic_pre_probe(struct udevice *dev)
--
2.11.0Regards,
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 at denx.de -------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180514/60a86dc2/attachment.sig>
PATCH v2 5/11 pmic: dm: Add support for MC34708 for PMIC DM
In reply to Lukasz MajewskiThis patch adds support for MC34708 PMIC, to be used with driver model (DM).
---
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
drivers/power/pmic/Kconfig +7 -0
@@ -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
drivers/power/pmic/Makefile +1 -0
@@ -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
drivers/power/pmic/mc34708.c new file +101 -0
101 lines not on this page · raw message
Signature · 1 line
2.11.0
PATCH v2 5/11 pmic: dm: Add support for MC34708 for PMIC DM
Hi Lukasz,
Quoted 54 lines · Lukasz Majewski
This patch adds support for MC34708 PMIC, to be used with driver model (DM).
Signed-off-by Lukasz Majewski <[email protected]>---
Changes in v2:
- Support for uclass private data with trasfer lengthdrivers/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.cdrivers/power/pmic/Kconfig +7
-0@@ -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_PMICdrivers/power/pmic/Makefile +1
-0@@ -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.odrivers/power/pmic/mc34708.c new file +2
-0@@ -0,0 +1,101 @@ +/* +* Copyright (C) 2018
drivers/power/pmic/mc34708.c line 12
Quoted 10 lines
drivers/power/pmic/mc34708.c new file +10
-0+* 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>
should be at end
drivers/power/pmic/mc34708.c line 13
Quoted 1 line
drivers/power/pmic/mc34708.c new file +1
-0+#include <fsl_pmic.h>
should be above i2c.h
Quoted 10 lines
drivers/power/pmic/mc34708.c new file +10
-0+ +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)
drivers/power/pmic/mc34708.c line 33
Quoted 10 lines
drivers/power/pmic/mc34708.c new file +10
-0+{ +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.
drivers/power/pmic/mc34708.c line 41
Quoted 8 lines
drivers/power/pmic/mc34708.c new file +8
-0+ + ret =dm_i2c_write (dev, reg, buf, len); +if (ret) +printf ("write error to device: %p register: %#x!" , dev, reg); + +return ret; +} +
Regards,
Simon
PATCH v2 5/11 pmic: dm: Add support for MC34708 for PMIC DM
Quoted 91 lines · On Mon, 14 May 2018 08:01:45 +1000 Simon Glass <[email protected]> wrote:
Hi Lukasz,
Quoted 56 lines · Lukasz Majewski
This patch adds support for MC34708 PMIC, to be used with driver model (DM).
Signed-off-by Lukasz Majewski <[email protected]>---
Changes in v2:
- Support for uclass private data with trasfer lengthdrivers/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.cdrivers/power/pmic/Kconfig +0
-0@@ -69,6 +69,13 @@ config DM_PMIC_MAX8998 This config enables implementation of driver-model pmicuclass features for PMIC MAX8998. The driver implements read/write operations.
drivers/power/pmic/Kconfig +5
-0+config DM_PMIC_MC34708 + bool "Enable Driver Model for PMIC MC34708" + depends on DM_PMIC + help + This config enables implementation of driver-model pmicuclass features
+ for PMIC MC34708. The driver implements read/write
operations. +
configPMIC_MAX8997
bool "Enable Driver Model for PMIC MAX8997" depends onDM_PMIC(quoted diff) +0
-0b/drivers/power/pmic/Makefile index
ad32068b3a..418b5e7aee100644drivers/power/pmic/Makefile +1
-0@@ -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(quoted diff) +0
-0
b/drivers/power/pmic/mc34708.cnew file mode 100644 index 0000000000..d9d1a41802drivers/power/pmic/mc34708.c new file +2
-0@@ -0,0 +1,101 @@ +/* +* Copyright (C) 2018 drivers/power/pmic/mc34708.c line 12
Quoted 10 lines
drivers/power/pmic/mc34708.c new file +10
-0+* 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> should be at end
drivers/power/pmic/mc34708.c line 13
Quoted 1 line
drivers/power/pmic/mc34708.c new file +1
-0+#include <fsl_pmic.h> should be above
i2c.hQuoted 15 lines
drivers/power/pmic/mc34708.c new file +9
-0+ +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,
drivers/power/pmic/mc34708.c new file +5
-0+ int len) +{ +u8 buf[3 ] = {0 }; +int ret; +
Quoted 8 lines
drivers/power/pmic/mc34708.c line 33
Quoted 6 lines
drivers/power/pmic/mc34708.c new file +6
-0+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).
Quoted 12 lines
drivers/power/pmic/mc34708.c line 41
Quoted 9 lines
drivers/power/pmic/mc34708.c new file +4
-0+ + ret =dm_i2c_write (dev, reg, buf, len); +if (ret) + printf("write error to device: %p register: %#x!",dev, reg); +
drivers/power/pmic/mc34708.c new file +3
-0+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 at denx.de -------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180514/84d8b060/attachment.sig>
PATCH v2 6/11 pmic: Rewrite the pmic command to not only work with single byte transmission
In reply to Lukasz MajewskiUp 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)
---
Changes in v2:
- New patch
cmd/pmic.c | 31 +++++++++++++++++++------------ 1 file changed, 19 insertions(+), 12 deletions(-)
cmd/pmic.c +19 -12
@@ -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);
Signature · 1 line
2.11.0
PATCH v2 6/11 pmic: Rewrite the pmic command to not only work with single byte transmission
Quoted 10 lines · Lukasz Majewski
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 <[email protected]>
Quoted 7 lines
---
Changes in v2:
- New patchcmd/pmic.c | 31 +++++++++++++++++++------------ 1 file changed, 19 insertions(+), 12 deletions(-)
PATCH v2 7/11 sandbox: Rewrite i2c_pmic_emul.c to support PMIC with 3 bytes transmission
In reply to Lukasz MajewskiThis 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
---
Changes in v2:
- New patch
drivers/power/pmic/i2c_pmic_emul.c | 45 ++++++++++++++++++++++++++------------ 1 file changed, 31 insertions(+), 14 deletions(-)
drivers/power/pmic/i2c_pmic_emul.c +31 -14
@@ -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 ; }
Signature · 1 line
2.11.0
PATCH v2 7/11 sandbox: Rewrite i2c_pmic_emul.c to support PMIC with 3 bytes transmission
Hi Lukasz,
Quoted 13 lines · Lukasz Majewski
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 <[email protected]>
Quoted 7 lines
---
Changes in v2:
- New patchdrivers/power/pmic/i2c_pmic_emul.c | 45 ++++++++++++++++++++++++++------------ 1 file changed, 31 insertions(+), 14 deletions(-)
Comments below
Quoted 28 lines
drivers/power/pmic/i2c_pmic_emul.c +7
-4@@ -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);
drivers/power/pmic/i2c_pmic_emul.c static int sandbox_i2c_pmic_read_data(struct udevice *emul, uchar chip, · line 43
Quoted 10 lines
drivers/power/pmic/i2c_pmic_emul.c +3
-3return -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?
Quoted 19 lines
drivers/power/pmic/i2c_pmic_emul.c +3
-2return 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--;
drivers/power/pmic/i2c_pmic_emul.c static int sandbox_i2c_pmic_write_data(struct udevice *emul, uchar chip, · line 77
Quoted 10 lines
drivers/power/pmic/i2c_pmic_emul.c +3
-3-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?
Quoted 10 lines
drivers/power/pmic/i2c_pmic_emul.c +2
-0return 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;
drivers/power/pmic/i2c_pmic_emul.c static int sandbox_i2c_pmic_xfer(struct udevice *emul, struct i2c_msg *msg, · line 120
Quoted 10 lines
drivers/power/pmic/i2c_pmic_emul.c +8
-0debug ("%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()
Quoted 20 lines
drivers/power/pmic/i2c_pmic_emul.c +5
-3+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 ; } --
Quoted 2 lines
2.11.0
Regards,
Simon
PATCH v2 7/11 sandbox: Rewrite i2c_pmic_emul.c to support PMIC with 3 bytes transmission
Hi Simon,
Quoted 69 lines
Hi Lukasz,
Quoted 13 lines · Lukasz Majewski
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 <[email protected]>Quoted 8 lines
---
Changes in v2:
- New patchdrivers/power/pmic/i2c_pmic_emul.c | 45++++++++++++++++++++++++++------------ 1 file changed, 31 insertions(+), 14 deletions(-)
Reviewed-by Simon Glass <[email protected]>Comments below
Quoted 40 lines
(quoted diff) +0
-0
b/drivers/power/pmic/i2c_pmic_emul.cindexc58ebb8825..6f227a92f3100644 ---a/drivers/power/pmic/i2c_pmic_emul.c
+++b/drivers/power/pmic/i2c_pmic_emul.cdrivers/power/pmic/i2c_pmic_emul.c +5
-2@@ -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, ucharchip, @@ -28,16 +31,16 @@ static int sandbox_i2c_pmic_read_data(struct udevice *emul, uchar chip, {
structsandbox_i2c_pmic_plat_data*plat =dev_get_platdata(emul);drivers/power/pmic/i2c_pmic_emul.c +1
-1-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",
drivers/power/pmic/i2c_pmic_emul.c +2
-3- 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);
Quoted 4 lines
drivers/power/pmic/i2c_pmic_emul.c static int sandbox_i2c_pmic_read_data(struct udevice *emul, uchar chip, · line 43
Quoted 2 lines
drivers/power/pmic/i2c_pmic_emul.c +1
-1-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.
Quoted 49 lines
Quoted 24 lines
return 0;}
drivers/power/pmic/i2c_pmic_emul.c +3
-2@@ -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)drivers/power/pmic/i2c_pmic_emul.c +0
-0@@ -65,12 +69,12 @@ static int sandbox_i2c_pmic_write_data(struct udevice *emul, uchar chip, buffer++; len--;drivers/power/pmic/i2c_pmic_emul.c static int sandbox_i2c_pmic_write_data(struct · line 78
Quoted 10 lines
drivers/power/pmic/i2c_pmic_emul.c +3
-3-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?
Quoted 12 lines
(quoted diff) +0
-0return 0; }@@ -101,20 +105,33 @@ static int sandbox_i2c_pmic_xfer(structudevice *emul, struct
i2c_msg*msg, static intsandbox_i2c_pmic_ofdata_to_platdata(struct udevice *emul){
structsandbox_i2c_pmic_plat_data*plat =dev_get_platdata(emul);(quoted diff) +2
-0+ struct udevice *pmic_dev = dev_get_parent(emul); + struct dm_pmic_info *pmic_info =
dev_get_uclass_priv(pmic_dev); const u8*reg_defaults;
Quoted 13 lines
Quoted 11 lines
debug("%s:%d Setting PMIC default registers\n",
__func__,__LINE__);drivers/power/pmic/i2c_pmic_emul.c +7
-0+ 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
Quoted 19 lines
Quoted 19 lines
drivers/power/pmic/i2c_pmic_emul.c +2
-1+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);drivers/power/pmic/i2c_pmic_emul.c +2
-1+free (plat->reg);return -EINVAL ; } -memcpy (&plat->reg, reg_defaults,SANDBOX_PMIC_REG_COUNT ); +memcpy (plat->reg, reg_defaults, plat->buf_size);
Quoted 8 lines
Quoted 5 lines
return 0;}
--
2.11.0Regards,
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 at denx.de -------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180514/503a5db6/attachment.sig>
PATCH v2 8/11 sandbox: Enable support for MC34708 PMIC in DTS
In reply to Lukasz MajewskiThis commit also provides the default values of the emulated MC34708 PMIC internal registers content.
---
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(+)
arch/sandbox/dts/sandbox.dts +4 -0
@@ -115,6 +115,10 @@ sandbox_pmic: sandbox_pmic { reg = <0x40>; }; + + mc34708_pmic: mc34708_pmic { + reg = <0x41>; + }; }; lcd {
arch/sandbox/dts/sandbox64.dts +4 -0
@@ -115,6 +115,10 @@ sandbox_pmic: sandbox_pmic { reg = <0x40>; }; + + mc34708_pmic: mc34708_pmic { + reg = <0x41>; + }; }; lcd {
arch/sandbox/dts/sandbox_pmic.dtsi +33 -0
@@ -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 + >; + }; +};
arch/sandbox/dts/test.dts +4 -0
@@ -227,6 +227,10 @@ sandbox_pmic: sandbox_pmic { reg = <0x40>; }; + + mc34708_pmic: mc34708_pmic { + reg = <0x41>; + }; }; adc at 0 {
Signature · 1 line
2.11.0
PATCH v2 8/11 sandbox: Enable support for MC34708 PMIC in DTS
Hi Lukasz,
arch/sandbox/dts/sandbox.dts line 121
Quoted 12 lines · Lukasz Majewski
arch/sandbox/dts/sandbox.dts +4
-0@@ -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 at 41 {
reg = <0x41>
};PATCH v2 8/11 sandbox: Enable support for MC34708 PMIC in DTS
Hi Fabio,
Quoted 25 lines
Hi Lukasz,
arch/sandbox/dts/sandbox.dts line 121
Quoted 12 lines · Lukasz Majewski
(quoted diff) +0
-0
b/arch/sandbox/dts/sandbox.dtsindex1fb8225fbb..b187b6fac1100644arch/sandbox/dts/sandbox.dts +4
-0@@ -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."
Quoted 7 lines
Also, the reg property needs to have a corresponding unit address.
It would better to rewrite this as:
mc34708: pmic at 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 at denx.de -------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180507/b4ea880f/attachment.sig>
PATCH v2 8/11 sandbox: Enable support for MC34708 PMIC in DTS
In reply to Lukasz MajewskiQuoted 7 lines · Lukasz Majewski
This commit also provides the default values of the emulated MC34708 PMIC internal registers content.
Signed-off-by Lukasz Majewski <[email protected]>---
Quoted 8 lines
Changes in v2:
- New patcharch/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(+)
PATCH v2 9/11 sandbox: Enable MC34708 PMIC support
In reply to Lukasz MajewskiThis 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.
---
Changes in v2:
- New patch
configs/sandbox_defconfig | 1 + 1 file changed, 1 insertion(+)
configs/sandbox_defconfig +1 -0
@@ -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
Signature · 1 line
2.11.0
PATCH v2 9/11 sandbox: Enable MC34708 PMIC support
Quoted 2 lines · Lukasz Majewski
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
Quoted 10 lines
Signed-off-by Lukasz Majewski <[email protected]>---
Changes in v2:
- New patchconfigs/sandbox_defconfig | 1 + 1 file changed, 1 insertion(+)
PATCH v2 10/11 sandbox: tests: Exclude common test code (pmic_get) in test/dm/pmic.c
In reply to Lukasz MajewskiThe common code can be excluded to be reused by tests for other PMIC.
---
Changes in v2:
- New patch
test/dm/pmic.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-)
test/dm/pmic.c +10 -2
@@ -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 */
Signature · 1 line
2.11.0
PATCH v2 10/11 sandbox: tests: Exclude common test code (pmic_get) in test/dm/pmic.c
Quoted 11 lines · Lukasz Majewski
The common code can be excluded to be reused by tests for other PMIC.
Signed-off-by Lukasz Majewski <[email protected]>---
Changes in v2:
- New patchtest/dm/pmic.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-)
PATCH v2 11/11 sandbox: tests: Add tests for mc34708 PMIC device
In reply to Lukasz MajewskiFollowing 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
---
Changes in v2:
- New patch
test/dm/pmic.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+)
test/dm/pmic.c +56 -0
@@ -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 );
Signature · 1 line
2.11.0
PATCH v2 11/11 sandbox: tests: Add tests for mc34708 PMIC device
Quoted 10 lines · Lukasz Majewski
Following tests has been added for mc34708 device:
-
get_testfor 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 ifpmic_clrsetbits()is working correctlySigned-off-by Lukasz Majewski <[email protected]>
Quoted 7 lines
---
Changes in v2:
- New patchtest/dm/pmic.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+)