From mboxrd@z Thu Jan 1 00:00:00 1970 From: Lukasz Majewski Date: Mon, 14 May 2018 11:47:14 +0200 Subject: [U-Boot] [PATCH v2 04/11] pmic: dm: Rewrite pmic_reg_{read|write|clrsetbits} to support 3 bytes transmissions In-Reply-To: References: <20180506202608.5899-1-lukma@denx.de> <20180506202608.5899-5-lukma@denx.de> Message-ID: <20180514114714.5bcf6140@jawa> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hi Simon, > Hi Lukasz, > > On 7 May 2018 at 06:26, Lukasz Majewski 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 > > > > --- > > > > 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 > > 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 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: