Hi Simon, > Hi Lukasz, > > On 7 May 2018 at 06:26, Lukasz Majewski <lu...@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 <lu...@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 <s...@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: w...@denx.de
pgpDNFTVKEd84.pgp
Description: OpenPGP digital signature
_______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot