[U-Boot] [PATCH] dm: rtc: Add 16-bit read/write support

[Stefan Roese]

Hi Bin,

On 17.02.2017 06:24, Bin Meng wrote:
> At present there are only 8-bit and 32-bit read/write routines in
> the rtc uclass driver. This adds the 16-bit support.
>
> Signed-off-by: Bin Meng <bmeng.cn at gmail.com>
> ---
>
>  drivers/rtc/rtc-uclass.c | 30 ++++++++++++++++++++++++++++++
>  include/rtc.h            | 20 ++++++++++++++++++++
>  2 files changed, 50 insertions(+)
>
> diff --git a/drivers/rtc/rtc-uclass.c b/drivers/rtc/rtc-uclass.c
> index 300e9b3..89312c5 100644
> --- a/drivers/rtc/rtc-uclass.c
> +++ b/drivers/rtc/rtc-uclass.c
> @@ -60,6 +60,36 @@ int rtc_write8(struct udevice *dev, unsigned int reg, int val)
>  	return ops->write8(dev, reg, val);
>  }
>
> +int rtc_read16(struct udevice *dev, unsigned int reg, u16 *valuep)
> +{
> +	u16 value = 0;
> +	int ret;
> +	int i;
> +
> +	for (i = 0; i < sizeof(value); i++) {
> +		ret = rtc_read8(dev, reg + i);
> +		if (ret < 0)
> +			return ret;
> +		value |= ret << (i << 3);
> +	}
> +
> +	*valuep = value;
> +	return 0;
> +}
> +
> +int rtc_write16(struct udevice *dev, unsigned int reg, u16 value)
> +{
> +	int i, ret;
> +
> +	for (i = 0; i < sizeof(value); i++) {
> +		ret = rtc_write8(dev, reg + i, (value >> (i << 3)) & 0xff);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	return 0;
> +}

These functions look a bit over-complex for handling 2 byte values
to me. Looking at the other functions, you seem to have cloned the
32bit version function. Why not introduce a function taking the
length (2 byte, 4 bytes, etc) as a parameter? This way, both the
16bit and 32bit functions can call this new function.

But its no big deal to me. You can keep this version as well, as its
probably not worth all this consolidating effort.

Thanks,
Stefan