[U-Boot] [PATCH v1 3/3] rtc: m41t62: Convert the RTC driver to support the driver model (DM)

[Stefan Roese]

On 21.11.18 23:43, Lukasz Majewski wrote:
> After this change the m41t62.c can be used with RTC subsystem (i.e. date
> command) which uses device model (DM).
> 
> Signed-off-by: Lukasz Majewski <lukma at denx.de>
> 
> ---
> 
> Changes in v1:
> - New
> 
>   drivers/rtc/m41t62.c | 75 +++++++++++++++++++++++++++++++++++++++++++++++++---
>   1 file changed, 72 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/rtc/m41t62.c b/drivers/rtc/m41t62.c
> index cc230e2b78..2dcc1f2a63 100644
> --- a/drivers/rtc/m41t62.c
> +++ b/drivers/rtc/m41t62.c
> @@ -1,5 +1,8 @@
>   // SPDX-License-Identifier: GPL-2.0+
>   /*
> + * (C) Copyright 2018
> + * Lukasz Majewski, DENX Software Engineering, lukma at denx.de.
> + *
>    * (C) Copyright 2008
>    * Stefan Roese, DENX Software Engineering, sr at denx.de.
>    *
> @@ -15,6 +18,7 @@
>   
>   #include <common.h>
>   #include <command.h>
> +#include <dm.h>
>   #include <rtc.h>
>   #include <i2c.h>
>   
> @@ -49,11 +53,23 @@
>   
>   #define M41T80_ALHOUR_HT	(1 << 6)	/* HT: Halt Update Bit */
>   
> +#ifdef CONFIG_DM_RTC
> +static int m41t62_rtc_get(struct udevice *dev, struct rtc_time *tm)
> +{
> +#else
>   int rtc_get(struct rtc_time *tm)
>   {
> +#endif /* CONFIG_DM_RTC */
>   	u8 buf[M41T62_DATETIME_REG_SIZE];
>   
> +#ifdef CONFIG_DM_RTC
> +	int ret = dm_i2c_read(dev, 0, buf, sizeof(buf));
> +
> +	if (ret)
> +		return ret;
> +#else
>   	i2c_read(CONFIG_SYS_I2C_RTC_ADDR, 0, 1, buf, M41T62_DATETIME_REG_SIZE);
> +#endif /* CONFIG_DM_RTC */
>   
>   	debug("%s: raw read data - sec=%02x, min=%02x, hr=%02x, "
>   	      "mday=%02x, mon=%02x, year=%02x, wday=%02x, y2k=%02x\n",
> @@ -81,15 +97,28 @@ int rtc_get(struct rtc_time *tm)
>   	return 0;
>   }

These #ifdef's in the function headers are a bit ugly. I personally
prefer to create a common function and call this from the "old"
(non-DM) function and the newly created DM function instead.

Perhaps you can rework this for all these functions (below as well)
and re-submit?

Thanks,
Stefan

>   
> +#ifdef CONFIG_DM_RTC
> +static int m41t62_rtc_set(struct udevice *dev, const struct rtc_time *tm)
> +{
> +#else
>   int rtc_set(struct rtc_time *tm)
>   {
> +#endif /* CONFIG_DM_RTC */
>   	u8 buf[M41T62_DATETIME_REG_SIZE];
> +	int ret;
>   
>   	debug("Set DATE: %4d-%02d-%02d (wday=%d)  TIME: %2d:%02d:%02d\n",
>   	      tm->tm_year, tm->tm_mon, tm->tm_mday, tm->tm_wday,
>   	      tm->tm_hour, tm->tm_min, tm->tm_sec);
>   
> -	i2c_read(CONFIG_SYS_I2C_RTC_ADDR, 0, 1, buf, M41T62_DATETIME_REG_SIZE);
> +#ifdef CONFIG_DM_RTC
> +	ret = dm_i2c_read(dev, 0, buf, sizeof(buf));
> +#else
> +	ret = i2c_read(CONFIG_SYS_I2C_RTC_ADDR, 0, 1, buf,
> +		       M41T62_DATETIME_REG_SIZE);
> +#endif /* CONFIG_DM_RTC */
> +	if (ret)
> +		return ret;
>   
>   	/* Merge time-data and register flags into buf[0..7] */
>   	buf[M41T62_REG_SSEC] = 0;
> @@ -108,8 +137,13 @@ int rtc_set(struct rtc_time *tm)
>   	/* assume 20YY not 19YY */
>   	buf[M41T62_REG_YEAR] = bin2bcd(tm->tm_year % 100);
>   
> -	if (i2c_write(CONFIG_SYS_I2C_RTC_ADDR, 0, 1, buf,
> -		      M41T62_DATETIME_REG_SIZE)) {
> +#ifdef CONFIG_DM_RTC
> +	ret = dm_i2c_write(dev, 0, buf, sizeof(buf));
> +#else
> +	ret = i2c_write(CONFIG_SYS_I2C_RTC_ADDR, 0, 1, buf,
> +			M41T62_DATETIME_REG_SIZE);
> +#endif /* CONFIG_DM_RTC */
> +	if (ret) {
>   		printf("I2C write failed in %s()\n", __func__);
>   		return -1;
>   	}
> @@ -117,15 +151,50 @@ int rtc_set(struct rtc_time *tm)
>   	return 0;
>   }
>   
> +#ifdef CONFIG_DM_RTC
> +static int m41t62_rtc_reset(struct udevice *dev)
> +{
> +#else
>   void rtc_reset(void)
>   {
> +#endif /* CONFIG_DM_RTC */
>   	u8 val;
>   
>   	/*
>   	 * M41T82: Make sure HT (Halt Update) bit is cleared.
>   	 * This bit is 0 in M41T62 so its save to clear it always.
>   	 */
> +#ifdef CONFIG_DM_RTC
> +	int ret = dm_i2c_read(dev, M41T62_REG_ALARM_HOUR, &val, sizeof(val));
> +
> +	val &= ~M41T80_ALHOUR_HT;
> +	ret |= dm_i2c_write(dev, M41T62_REG_ALARM_HOUR, &val, sizeof(val));
> +
> +	return ret;
> +#else
>   	i2c_read(CONFIG_SYS_I2C_RTC_ADDR, M41T62_REG_ALARM_HOUR, 1, &val, 1);
>   	val &= ~M41T80_ALHOUR_HT;
>   	i2c_write(CONFIG_SYS_I2C_RTC_ADDR, M41T62_REG_ALARM_HOUR, 1, &val, 1);
> +#endif /* CONFIG_DM_RTC */
>   }
> +
> +#ifdef CONFIG_DM_RTC
> +static const struct rtc_ops m41t62_rtc_ops = {
> +	.get = m41t62_rtc_get,
> +	.set = m41t62_rtc_set,
> +	.reset = m41t62_rtc_reset,
> +};
> +
> +static const struct udevice_id m41t62_rtc_ids[] = {
> +	{ .compatible = "st,m41t62" },
> +	{ .compatible = "microcrystal,rv4162" },
> +	{ }
> +};
> +
> +U_BOOT_DRIVER(rtc_m41t62) = {
> +	.name	= "rtc-m41t62",
> +	.id	= UCLASS_RTC,
> +	.of_match = m41t62_rtc_ids,
> +	.ops	= &m41t62_rtc_ops,
> +};
> +#endif /* CONFIG_DM_RTC */
> 

Viele Grüße,
Stefan

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr at denx.de