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