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 <takahiro.aka...@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.

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 <asm/types.h>
> > +
> > +struct pl031_rtc_platdata {
> > +   phys_addr_t base;
> > +};
> > +
> > +#endif
> > 
> 
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Reply via email to