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

[AKASHI Takahiro <takahiro.akashi@linaro.org>]

On Tue, Sep 04, 2018 at 07:18:18PM +0200, Heinrich Schuchardt wrote:
> On 07/23/2018 09:17 AM, AKASHI Takahiro wrote:
> > On Sat, Jul 21, 2018 at 06:53:16AM +0200, Heinrich Schuchardt wrote:
> >> On 07/11/2018 11:06 AM, AKASHI Takahiro wrote:
> >>> With this patch, PL031 driver is converted to driver-model-compliant
> >>> driver. In addition, CONFIG_SYS_RTC_PL031_BASE is no longer valid.
> >>>
> >>> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> >>> ---
> >>>  drivers/rtc/Kconfig          |   6 ++
> >>>  drivers/rtc/pl031.c          | 126 ++++++++++++++++++++++-------------
> >>>  scripts/config_whitelist.txt |   1 -
> >>>  3 files changed, 86 insertions(+), 47 deletions(-)
> >>>
> >>> 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.
> >>> +
> >>
> >> Tom merged
> >> http://git.denx.de/?p=u-boot.git;a=commit;h=b19886b9469174213877ef37670ce35c55acb456
> >> https://patchwork.ozlabs.org/patch/936533/
> >> ARM: qemu-arm: enable RTC
> >> which is superseeded by your patch series.
> >>
> >> We should avoid duplicate entries CONFIG_RTC_PL031.
> >>
> >> Symbol CONFIG_SYS_RTC_PL031_BASE can be removed in
> >> include/configs/qemu-arm.h with this patch.
> >>
> >> Could you, please, respin your patch series.

Thank you for this reminder.
While it's nothing much for me to respin my patch, I wonder how maintainers
handle coming patches in ML. I submitted my first counterpart patch
just a few days after your post in a form of a *reply* to your e-mail.
Then your patch was merged and mine was just ignored.

Thanks,
-Takahiro AKASHI

> > I think that your patch be reverted first and my patch be
> > merged (in this merge window or next -rc1) as u-boot master
> > is still open.
> > 
> > -Takahiro AKASHI
> > 
> > 
> 
> Hello Takahiro,
> 
> I think the changes you planned make perfectly sense.
> 
> Will you rework this patch series, so that it will be included into
> v2018.11?
> 
> Best regards
> 
> Heinrich
> 
> 
> >> Best regards
> >>
> >> Heinrich
> >>
> >>>  config RTC_S35392A
> >>>  	bool "Enable S35392A driver"
> >>>  	select BITREVERSE
> >>> diff --git a/drivers/rtc/pl031.c b/drivers/rtc/pl031.c
> >>> index 8955805e3b..b8fd944e44 100644
> >>> --- a/drivers/rtc/pl031.c
> >>> +++ b/drivers/rtc/pl031.c
> >>> @@ -8,13 +8,11 @@
> >>>  
> >>>  #include <common.h>
> >>>  #include <command.h>
> >>> +#include <dm.h>
> >>> +#include <errno.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
> >>> +#include <asm/io.h>
> >>> +#include <asm/types.h>
> >>>  
> >>>  /*
> >>>   * Register definitions
> >>> @@ -30,78 +28,114 @@
> >>>  
> >>>  #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)))
> >>> +struct pl031_platdata {
> >>> +	phys_addr_t base;
> >>> +};
> >>>  
> >>> -static int pl031_initted = 0;
> >>> +static inline u32 pl031_read_reg(struct udevice *dev, int reg)
> >>> +{
> >>> +	struct pl031_platdata *pdata = dev_get_platdata(dev);
> >>>  
> >>> -/* Enable RTC Start in Control register*/
> >>> -void rtc_init(void)
> >>> +	return readl(pdata->base + reg);
> >>> +}
> >>> +
> >>> +static inline u32 pl031_write_reg(struct udevice *dev, int reg, u32 value)
> >>>  {
> >>> -	RTC_WRITE_REG(RTC_CR, RTC_CR_START);
> >>> +	struct pl031_platdata *pdata = dev_get_platdata(dev);
> >>>  
> >>> -	pl031_initted = 1;
> >>> +	return writel(value, pdata->base + reg);
> >>>  }
> >>>  
> >>>  /*
> >>> - * Reset the RTC. We set the date back to 1970-01-01.
> >>> + * Probe RTC device
> >>> + */
> >>> +static int pl031_probe(struct udevice *dev)
> >>> +{
> >>> +	/* Enable RTC Start in Control register*/
> >>> +	pl031_write_reg(dev, RTC_CR, RTC_CR_START);
> >>> +
> >>> +	return 0;
> >>> +}
> >>> +
> >>> +/*
> >>> + * Get the current time from the RTC
> >>>   */
> >>> -void rtc_reset(void)
> >>> +static int pl031_get(struct udevice *dev, struct rtc_time *tm)
> >>>  {
> >>> -	RTC_WRITE_REG(RTC_LR, 0x00);
> >>> -	if(!pl031_initted)
> >>> -		rtc_init();
> >>> +	unsigned long tim;
> >>> +
> >>> +	if (!tm)
> >>> +		return -EINVAL;
> >>> +
> >>> +	tim = pl031_read_reg(dev, RTC_DR);
> >>> +
> >>> +	rtc_to_tm(tim, tm);
> >>> +
> >>> +	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_set(struct udevice *dev, const struct rtc_time *tm)
> >>>  {
> >>>  	unsigned long tim;
> >>>  
> >>> -	if(!pl031_initted)
> >>> -		rtc_init();
> >>> +	if (!tm)
> >>> +		return -EINVAL;
> >>>  
> >>> -	if (tmp == NULL) {
> >>> -		puts("Error setting the date/time\n");
> >>> -		return -1;
> >>> -	}
> >>> +	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);
> >>>  
> >>>  	/* Calculate number of seconds this incoming time represents */
> >>> -	tim = rtc_mktime(tmp);
> >>> +	tim = rtc_mktime(tm);
> >>>  
> >>> -	RTC_WRITE_REG(RTC_LR, tim);
> >>> +	pl031_write_reg(dev, RTC_LR, tim);
> >>>  
> >>> -	return -1;
> >>> +	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_reset(struct udevice *dev)
> >>>  {
> >>> -	ulong tim;
> >>> +	pl031_write_reg(dev, RTC_LR, 0);
> >>>  
> >>> -	if(!pl031_initted)
> >>> -		rtc_init();
> >>> +	return 0;
> >>> +}
> >>>  
> >>> -	if (tmp == NULL) {
> >>> -		puts("Error getting the date/time\n");
> >>> -		return -1;
> >>> -	}
> >>> +static const struct rtc_ops pl031_ops = {
> >>> +	.get = pl031_get,
> >>> +	.set = pl031_set,
> >>> +	.reset = pl031_reset,
> >>> +};
> >>>  
> >>> -	tim = RTC_READ_REG(RTC_DR);
> >>> +static const struct udevice_id pl031_ids[] = {
> >>> +	{ .compatible = "arm,pl031" },
> >>> +	{ }
> >>> +};
> >>>  
> >>> -	rtc_to_tm(tim, tmp);
> >>> +static int pl031_ofdata_to_platdata(struct udevice *dev)
> >>> +{
> >>> +	struct pl031_platdata *pdata = dev_get_platdata(dev);
> >>>  
> >>> -	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);
> >>> +	pdata->base = dev_read_addr(dev);
> >>>  
> >>>  	return 0;
> >>>  }
> >>>  
> >>> -#endif
> >>> +U_BOOT_DRIVER(rtc_pl031) = {
> >>> +	.name	= "rtc-pl031",
> >>> +	.id	= UCLASS_RTC,
> >>> +	.of_match = pl031_ids,
> >>> +	.probe	= pl031_probe,
> >>> +	.ofdata_to_platdata = pl031_ofdata_to_platdata,
> >>> +	.platdata_auto_alloc_size = sizeof(struct pl031_platdata),
> >>> +	.ops	= &pl031_ops,
> >>> +};
> >>> diff --git a/scripts/config_whitelist.txt b/scripts/config_whitelist.txt
> >>> index 1219dcc3be..17bd2d681c 100644
> >>> --- a/scripts/config_whitelist.txt
> >>> +++ b/scripts/config_whitelist.txt
> >>> @@ -4115,7 +4115,6 @@ CONFIG_SYS_RSTC_RMR_VAL
> >>>  CONFIG_SYS_RTC_BUS_NUM
> >>>  CONFIG_SYS_RTC_CNT
> >>>  CONFIG_SYS_RTC_OSCILLATOR
> >>> -CONFIG_SYS_RTC_PL031_BASE
> >>>  CONFIG_SYS_RTC_REG_BASE_ADDR
> >>>  CONFIG_SYS_RTC_SETUP
> >>>  CONFIG_SYS_RV3029_TCR
> >>>
> >>
> > 
>