[U-Boot] [PATCH 4/5] pmic: dm: Rewrite pmic_reg_{read|write} and pmic_clrsetbits to support transmissions larger than 1 byte

[Simon Glass]

Hi Lukasz,

On 26 April 2018 at 06:19, Lukasz Majewski <lukma at 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 at denx.de>
> ---
>
>  drivers/power/pmic/pmic-uclass.c | 38 ++++++++++++++++++++++++++------------
>  1 file changed, 26 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/power/pmic/pmic-uclass.c b/drivers/power/pmic/pmic-uclass.c
> index 0148e5c883..9004911daa 100644
> --- a/drivers/power/pmic/pmic-uclass.c
> +++ b/drivers/power/pmic/pmic-uclass.c
> @@ -143,23 +143,32 @@ 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;
> +       int tx_num = 1;
> +       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);
> +       ret = pmic_trans_len(dev);

As with the previous patch, I think it is better to put this in the
uclass's device-private data. It can be set to 1 by default, and the
device can updated it to 2, etc, as needed.

Then we avoid making this extra call for every operation.

Also please don't forget to add a sandbox test for it.

> +       if (ret > 1 && ret <= sizeof(val))
> +               tx_num = ret;
>
> -       return ret ? ret : byte;
> +       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 : val;
>  }
>
>  int pmic_reg_write(struct udevice *dev, uint reg, uint value)
>  {
> -       u8 byte = value;
> +       int tx_num = 1;
>         int ret;
>
> -       debug("%s: reg=%x, value=%x", __func__, reg, value);
> -       ret = pmic_write(dev, reg, &byte, 1);
> +       ret = pmic_trans_len(dev);
> +       if (ret > 1 && ret <= sizeof(value))
> +               tx_num = ret;
> +
> +       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;
> @@ -167,15 +176,20 @@ 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;
> +       int tx_num = 1;
> +       u32 val = 0;
>         int ret;
>
> -       ret = pmic_reg_read(dev, reg);
> +       ret = pmic_trans_len(dev);
> +       if (ret > 1 && ret <= sizeof(val))
> +               tx_num = ret;
> +
> +       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);
>  }
>
>  UCLASS_DRIVER(pmic) = {
> --
> 2.11.0
>

Regards,
Simon