[U-Boot] [PATCH v2] rtc: Add RTC chip pcf2127 support

[Simon Glass]

Hi Meng,

On 30 November 2016 at 00:47, Meng Yi <meng.yi at nxp.com> wrote:
> This driver compatible with pcf2127 and pcf2129
>
> Signed-off-by: Meng Yi <meng.yi at nxp.com>
> ---
> change in V2:
> -convert to using u-boot driver module
> ---
>  drivers/rtc/Kconfig   |   6 +++
>  drivers/rtc/Makefile  |   1 +
>  drivers/rtc/pcf2127.c | 107 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 114 insertions(+)
>  create mode 100644 drivers/rtc/pcf2127.c
>

Reviewed-by: Simon Glass <sjg at chromium.org>

Nits below

> diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
> index b5d9048..57af1b5 100644
> --- a/drivers/rtc/Kconfig
> +++ b/drivers/rtc/Kconfig
> @@ -13,4 +13,10 @@ config DM_RTC
>           drivers to perform the actual functions. See rtc.h for a
>           description of the API.
>
> +config RTC_PCF2127
> +       bool "Enable PCF2127 driver"
> +       depends on DM_RTC
> +       help
> +         Enable pcf2127 driver which provides rtc get and set function

Please list the manufacturer and explain briefly what features it has
(rtc, non-volatile ram, battery backup...?)

> +
>  endmenu
> diff --git a/drivers/rtc/Makefile b/drivers/rtc/Makefile
> index fc38a3f..c919427 100644
> --- a/drivers/rtc/Makefile
> +++ b/drivers/rtc/Makefile
> @@ -45,6 +45,7 @@ obj-$(CONFIG_RTC_MV) += mvrtc.o
>  obj-$(CONFIG_RTC_MX27) += mx27rtc.o
>  obj-$(CONFIG_RTC_MXS) += mxsrtc.o
>  obj-$(CONFIG_RTC_PCF8563) += pcf8563.o
> +obj-$(CONFIG_RTC_PCF2127) += pcf2127.o
>  obj-$(CONFIG_RTC_PL031) += pl031.o
>  obj-$(CONFIG_RTC_PT7C4338) += pt7c4338.o
>  obj-$(CONFIG_RTC_RS5C372A) += rs5c372.o
> diff --git a/drivers/rtc/pcf2127.c b/drivers/rtc/pcf2127.c
> new file mode 100644
> index 0000000..bc59c6c
> --- /dev/null
> +++ b/drivers/rtc/pcf2127.c
> @@ -0,0 +1,107 @@
> +/*
> + * Copyright (C) 2016 by NXP Semiconductors Inc.
> + * Date & Time support for PCF2127 RTC
> + */
> +
> +/*     #define DEBUG   */
> +
> +#include <common.h>
> +#include <command.h>
> +#include <dm.h>
> +#include <i2c.h>
> +#include <rtc.h>
> +
> +#define PCF2127_REG_CTRL1      (0x00)

Please drop the () around these

> +#define PCF2127_REG_CTRL2      (0x01)
> +#define PCF2127_REG_CTRL3      (0x02)
> +#define PCF2127_REG_SC         (0x03)  /* datetime */
> +#define PCF2127_REG_MN         (0x04)
> +#define PCF2127_REG_HR         (0x05)
> +#define PCF2127_REG_DM         (0x06)
> +#define PCF2127_REG_DW         (0x07)
> +#define PCF2127_REG_MO         (0x08)
> +#define PCF2127_REG_YR         (0x09)
> +
> +static int pcf2127_rtc_set(struct udevice *dev, const struct rtc_time *tm)
> +{
> +       uchar buf[8];
> +       int i = 0;
> +
> +       /* start register address */
> +       buf[i++] = PCF2127_REG_SC;
> +
> +       /* hours, minutes and seconds */
> +       buf[i++] = bin2bcd(tm->tm_sec);
> +       buf[i++] = bin2bcd(tm->tm_min);
> +       buf[i++] = bin2bcd(tm->tm_hour);
> +       buf[i++] = bin2bcd(tm->tm_mday);
> +       buf[i++] = tm->tm_wday & 0x07;
> +
> +       /* month, 1 - 12 */
> +       buf[i++] = bin2bcd(tm->tm_mon + 1);
> +
> +       /* year */
> +       buf[i++] = bin2bcd(tm->tm_year % 100);
> +
> +       /* write register's data */
> +       if (dm_i2c_write(dev, PCF2127_REG_CTRL1, buf, sizeof(buf)) < 0)
> +               return -1;

You should return the value you get from dm_i2c_write()

> +
> +       return 0;
> +}
> +
> +static int pcf2127_rtc_get(struct udevice *dev, struct rtc_time *tm)
> +{
> +       int rel = 0;
> +       uchar buf[10] = { PCF2127_REG_CTRL1 };
> +
> +       if (dm_i2c_write(dev, PCF2127_REG_CTRL1, buf, 1) < 0)
> +               return -1;

Same for these

> +       if (dm_i2c_read(dev, PCF2127_REG_CTRL1, buf, sizeof(buf)) < 0)
> +               return -1;
> +
> +       if (buf[PCF2127_REG_CTRL3] & 0x04)
> +               puts("### Warning: RTC Low Voltage - date/time not reliable\n");
> +
> +       tm->tm_sec  = bcd2bin(buf[PCF2127_REG_SC] & 0x7F);
> +       tm->tm_min  = bcd2bin(buf[PCF2127_REG_MN] & 0x7F);
> +       tm->tm_hour = bcd2bin(buf[PCF2127_REG_HR] & 0x3F);
> +       tm->tm_mday = bcd2bin(buf[PCF2127_REG_DM] & 0x3F);
> +       tm->tm_mon  = bcd2bin(buf[PCF2127_REG_MO] & 0x1F) - 1;
> +       tm->tm_year = bcd2bin(buf[PCF2127_REG_YR]) + 1900;
> +       if (tm->tm_year < 1970)
> +               tm->tm_year += 100;     /* assume we are in 1970...2069 */
> +       tm->tm_wday = buf[PCF2127_REG_DW] & 0x07;
> +       tm->tm_yday = 0;
> +       tm->tm_isdst = 0;
> +
> +       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 rel;
> +}
> +
> +static int pcf2127_rtc_reset(struct udevice *dev)
> +{
> +       /*Doing nothing here*/

/* Doing nothing here */

blank line here (before 'return')

> +       return 0;
> +}
> +
> +static const struct rtc_ops pcf2127_rtc_ops = {
> +       .get = pcf2127_rtc_get,
> +       .set = pcf2127_rtc_set,
> +       .reset = pcf2127_rtc_reset,
> +};
> +
> +static const struct udevice_id pcf2127_rtc_ids[] = {
> +       { .compatible = "pcf2127-rtc" },
> +       { }
> +};
> +
> +U_BOOT_DRIVER(rtc_pcf2127) = {
> +       .name   = "rtc-pcf2127",
> +       .id     = UCLASS_RTC,
> +       .of_match = pcf2127_rtc_ids,
> +       .ops    = &pcf2127_rtc_ops,
> +};
> --
> 2.1.0.27.g96db324
>

Regards,
Simon