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