From mboxrd@z Thu Jan 1 00:00:00 1970 From: AKASHI Takahiro Date: Thu, 5 Jul 2018 18:37:40 +0900 Subject: [U-Boot] [PATCH v2 1/2] rtc: pl031: convert the driver to driver model In-Reply-To: <2311e25a-3ea1-0584-4c54-e59c259bc1da@gmx.de> References: <20180704073628.23596-1-takahiro.akashi@linaro.org> <20180704073628.23596-2-takahiro.akashi@linaro.org> <2311e25a-3ea1-0584-4c54-e59c259bc1da@gmx.de> Message-ID: <20180705093739.GK28220@linaro.org> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On Wed, Jul 04, 2018 at 12:50:52PM +0200, Heinrich Schuchardt wrote: > On 07/04/2018 09:36 AM, AKASHI Takahiro wrote: > > Signed-off-by: AKASHI Takahiro > > --- > > 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 > > #include > > +#include > > +#include > > #include > > > > -#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. But how do we know if the value is bogus or not? > > + > > + 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. OK. > > > > - 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. Ouch. > > } > > > > /* > > - * 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. This variable will go away. Thanks, -Takahiro AKASHI > 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 > > + > > +struct pl031_rtc_platdata { > > + phys_addr_t base; > > +}; > > + > > +#endif > > >