[U-Boot] [PATCH v2 1/2] rtc: pl031: convert the driver to driver model

[Heinrich Schuchardt]

On 07/04/2018 09:36 AM, AKASHI Takahiro wrote:
> Signed-off-by: AKASHI Takahiro <takahiro.akashi at linaro.org>
> ---
>  drivers/rtc/Kconfig                  |   6 ++
>  drivers/rtc/pl031.c                  | 109 +++++++++++++++++----------
>  include/dm/platform_data/rtc_pl031.h |  12 +++
>  3 files changed, 87 insertions(+), 40 deletions(-)
>  create mode 100644 include/dm/platform_data/rtc_pl031.h
> 
> diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
> index a3f8c8aecc..96c4cce410 100644
> --- a/drivers/rtc/Kconfig
> +++ b/drivers/rtc/Kconfig
> @@ -55,6 +55,12 @@ config RTC_MV
>  	  Enable Marvell RTC driver. This driver supports the rtc that is present
>  	  on some Marvell SoCs.
>  
> +config RTC_PL031
> +	bool "Enable ARM PL031 driver"
> +	depends on DM_RTC
> +	help
> +	  Enable ARM PL031 driver.
> +
>  config RTC_S35392A
>  	bool "Enable S35392A driver"
>  	select BITREVERSE
> diff --git a/drivers/rtc/pl031.c b/drivers/rtc/pl031.c
> index 8955805e3b..eecade8374 100644
> --- a/drivers/rtc/pl031.c
> +++ b/drivers/rtc/pl031.c
> @@ -8,14 +8,10 @@
>  
>  #include <common.h>
>  #include <command.h>
> +#include <dm.h>
> +#include <dm/platform_data/rtc_pl031.h>
>  #include <rtc.h>
>  
> -#if defined(CONFIG_CMD_DATE)
> -
> -#ifndef CONFIG_SYS_RTC_PL031_BASE
> -#error CONFIG_SYS_RTC_PL031_BASE is not defined!
> -#endif
> -
>  /*
>   * Register definitions
>   */
> @@ -30,78 +26,111 @@
>  
>  #define RTC_CR_START	(1 << 0)
>  
> -#define	RTC_WRITE_REG(addr, val) \
> -			(*(volatile unsigned int *)(CONFIG_SYS_RTC_PL031_BASE + (addr)) = (val))
> -#define	RTC_READ_REG(addr)	\
> -			(*(volatile unsigned int *)(CONFIG_SYS_RTC_PL031_BASE + (addr)))
> +#define	RTC_WRITE_REG(base, addr, val) \
> +			(*(volatile unsigned int *)((base) + (addr)) = (val))
> +#define	RTC_READ_REG(base, addr)	\
> +			(*(volatile unsigned int *)((base) + (addr)))
>  
>  static int pl031_initted = 0;
>  
>  /* Enable RTC Start in Control register*/
> -void rtc_init(void)
> +void pl031_rtc_init(struct pl031_rtc_platdata *pdata)
>  {
> -	RTC_WRITE_REG(RTC_CR, RTC_CR_START);
> +	RTC_WRITE_REG(pdata->base, RTC_CR, RTC_CR_START);
>  
>  	pl031_initted = 1;
>  }
>  
>  /*
> - * Reset the RTC. We set the date back to 1970-01-01.
> + * Get the current time from the RTC
>   */
> -void rtc_reset(void)
> +static int pl031_rtc_get(struct udevice *dev, struct rtc_time *tm)
>  {
> -	RTC_WRITE_REG(RTC_LR, 0x00);
> -	if(!pl031_initted)
> -		rtc_init();
> +	struct pl031_rtc_platdata *pdata = dev_get_platdata(dev);
> +	ulong tim;
> +
> +	if (!tm) {
> +		puts("Error getting the date/time\n");
> +		return -1;
> +	}
> +
> +	if (!pl031_initted)
> +		pl031_rtc_init(pdata);
> +
> +	tim = RTC_READ_REG(pdata->base, RTC_DR);
> +
> +	rtc_to_tm(tim, tm);

Please, check the return code. The RTC may contain invalid data.

> +
> +	debug("Get 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);
> +
> +	return 0;
>  }
>  
>  /*
>   * Set the RTC
> -*/
> -int rtc_set(struct rtc_time *tmp)
> + */
> +static int pl031_rtc_set(struct udevice *dev, const struct rtc_time *tm)
>  {
> +	struct pl031_rtc_platdata *pdata = dev_get_platdata(dev);
>  	unsigned long tim;

Please, add a debug statement here, too.

>  
> -	if(!pl031_initted)
> -		rtc_init();
> -
> -	if (tmp == NULL) {
> +	if (!tm) {
>  		puts("Error setting the date/time\n");
>  		return -1;
>  	}
>  
> +	if (!pl031_initted)
> +		pl031_rtc_init(pdata);
> +
>  	/* Calculate number of seconds this incoming time represents */
> -	tim = rtc_mktime(tmp);
> +	tim = rtc_mktime(tm);
>  
> -	RTC_WRITE_REG(RTC_LR, tim);
> +	RTC_WRITE_REG(pdata->base, RTC_LR, tim);
>  
>  	return -1;

No error has occurred. Please, return 0.

>  }
>  
>  /*
> - * Get the current time from the RTC
> + * Reset the RTC. We set the date back to 1970-01-01.
>   */
> -int rtc_get(struct rtc_time *tmp)
> +static int pl031_rtc_reset(struct udevice *dev)
>  {
> -	ulong tim;
> +	struct pl031_rtc_platdata *pdata = dev_get_platdata(dev);
>  
> -	if(!pl031_initted)
> -		rtc_init();
> +	RTC_WRITE_REG(pdata->base, RTC_LR, 0x00);
>  
> -	if (tmp == NULL) {
> -		puts("Error getting the date/time\n");
> -		return -1;
> -	}
> +	if (!pl031_initted)

nits:

There is no English word initted. I would prefer pl031_initialized.

Best regards

Heinrich

> +		pl031_rtc_init(pdata);
>  
> -	tim = RTC_READ_REG(RTC_DR);
> +	return 0;
> +}
> +
> +static const struct rtc_ops pl031_rtc_ops = {
> +	.get = pl031_rtc_get,
> +	.set = pl031_rtc_set,
> +	.reset = pl031_rtc_reset,
> +};
>  
> -	rtc_to_tm(tim, tmp);
> +static const struct udevice_id pl031_rtc_ids[] = {
> +	{ .compatible = "arm,pl031" },
> +	{ }
> +};
>  
> -	debug ( "Get DATE: %4d-%02d-%02d (wday=%d)  TIME: %2d:%02d:%02d\n",
> -		tmp->tm_year, tmp->tm_mon, tmp->tm_mday, tmp->tm_wday,
> -		tmp->tm_hour, tmp->tm_min, tmp->tm_sec);
> +static int pl031_rtc_ofdata_to_platdata(struct udevice *dev)
> +{
> +	struct pl031_rtc_platdata *pdata = dev_get_platdata(dev);
>  
> +	pdata->base = devfdt_get_addr(dev);
>  	return 0;
>  }
>  
> -#endif
> +U_BOOT_DRIVER(rtc_pl031) = {
> +	.name	= "rtc-pl031",
> +	.id	= UCLASS_RTC,
> +	.of_match = pl031_rtc_ids,
> +	.ofdata_to_platdata = pl031_rtc_ofdata_to_platdata,
> +	.platdata_auto_alloc_size = sizeof(struct pl031_rtc_platdata),
> +	.ops	= &pl031_rtc_ops,
> +};
> diff --git a/include/dm/platform_data/rtc_pl031.h b/include/dm/platform_data/rtc_pl031.h
> new file mode 100644
> index 0000000000..8e4ba1ce69
> --- /dev/null
> +++ b/include/dm/platform_data/rtc_pl031.h
> @@ -0,0 +1,12 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +
> +#ifndef __rtc_pl031_h
> +#define __rtc_pl031_h
> +
> +#include <asm/types.h>
> +
> +struct pl031_rtc_platdata {
> +	phys_addr_t base;
> +};
> +
> +#endif
>