Re: [PATCH] rtc: add ht1380 driver

Hi Sergei,

On Tue, 25 Oct 2022 at 08:24, Sergei Antonov saproj@gmail.com wrote:

Support Holtek HT1380/HT1381 Serial Timekeeper Chip. Datasheet: https://www.holtek.com.tw/documents/10179/11842/ht1380_1v130.pdf

Signed-off-by: Sergei Antonov saproj@gmail.com

drivers/rtc/Kconfig | 7 + drivers/rtc/Makefile | 1 + drivers/rtc/ht1380.c | 303 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 311 insertions(+) create mode 100644 drivers/rtc/ht1380.c

Looks good, just some minor things.

diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig index 23963271928a..f62ee43b0f89 100644 --- a/drivers/rtc/Kconfig +++ b/drivers/rtc/Kconfig @@ -220,4 +220,11 @@ config RTC_ZYNQMP Say "yes" here to support the on chip real time clock present on Xilinx ZynqMP SoC.

+config RTC_HT1380

•   bool "Enable Holtek HT1380/HT1381 RTC driver"
  

•   depends on DM_RTC && DM_GPIO
  

•   help
  

•     Say "yes" here to get support for Holtek HT1380/HT1381
  

•     Serial Timekeeper Chip.
  

What features does it have? Needs a bit more detail.

endmenu diff --git a/drivers/rtc/Makefile b/drivers/rtc/Makefile index d621be622848..d04ac8fb0938 100644 --- a/drivers/rtc/Makefile +++ b/drivers/rtc/Makefile @@ -24,6 +24,7 @@ obj-$(CONFIG_RTC_DS3231) += ds3231.o obj-$(CONFIG_RTC_DS3232) += ds3232.o obj-$(CONFIG_RTC_EMULATION) += emul_rtc.o obj-$(CONFIG_RTC_FTRTC010) += ftrtc010.o +obj-$(CONFIG_RTC_HT1380) += ht1380.o obj-$(CONFIG_SANDBOX) += i2c_rtc_emul.o obj-$(CONFIG_RTC_IMXDI) += imxdi.o obj-$(CONFIG_RTC_ISL1208) += isl1208.o diff --git a/drivers/rtc/ht1380.c b/drivers/rtc/ht1380.c new file mode 100644 index 000000000000..af28b5f25fcd --- /dev/null +++ b/drivers/rtc/ht1380.c @@ -0,0 +1,303 @@ +// SPDX-License-Identifier: GPL-2.0+ +/*

• • Holtek HT1380/HT1381 Serial Timekeeper Chip
• */

+#include <common.h> +#include <dm.h> +#include <rtc.h> +#include <bcd.h> +#include <asm/gpio.h> +#include <linux/delay.h>

+struct ht1380_priv {

•   struct gpio_desc rst_desc;
  

•   struct gpio_desc clk_desc;
  

•   struct gpio_desc dat_desc;
  

+};

+enum registers {

•   SEC,
  

•   MIN,
  

•   HOUR,
  

•   MDAY,
  

•   MONTH,
  

•   WDAY,
  

•   YEAR,
  

•   WP,
  

•   N_REGS
  

+};

+static const int BURST = 0xBE; +static const int READ = 1;

+static void ht1380_half_period_delay(void) +{

•   /* Delay for half a period. 1 us complies with the 500 KHz maximum
  

•      input serial clock limit given by the datasheet. */
  

/* * Delay for half... * second line */

Please fix globally.

•   udelay(1);
  

+}

+static int ht1380_send_byte(struct ht1380_priv *priv, int byte) +{

•   int ret;
  

•   for (int bit = 0; bit < 8; bit++) {
  

•           ret = dm_gpio_set_value(&priv->dat_desc, byte >> bit & 1);
  

•           if (ret)
  

•                   break;
  

•           ht1380_half_period_delay();
  

•           ret = dm_gpio_set_value(&priv->clk_desc, 1);
  

•           if (ret)
  

•                   break;
  

•           ht1380_half_period_delay();
  

•           ret = dm_gpio_set_value(&priv->clk_desc, 0);
  

•           if (ret)
  

•                   break;
  

•   }
  

•   return ret;
  

+}

+static int ht1380_reset_off(struct ht1380_priv *priv) +{

•   const unsigned int T_CC = 4; /* us, Reset to Clock Setup */
  

•   int ret;
  

•   /* Go out of RESET state.
  

•      Make sure we make the minimal delay required by the datasheet. */
  

•   ret = dm_gpio_set_value(&priv->rst_desc, 1);
  

•   udelay(T_CC);
  

blank line before return

•   return ret;
  

+}

+static int ht1380_reset_on(struct ht1380_priv *priv)

function comment - does it stay in reset state or automatically exit?

+{

•   const unsigned int T_CWH = 4; /* us, Reset Inactive Time */
  

•   int ret;
  

•   /* Enter RESET state.
  

•      Make sure we make the minimal delay required by the datasheet. */
  

•   ret = dm_gpio_set_value(&priv->rst_desc, 0);
  

•   udelay(T_CWH);
  

•   return ret;
  

+}

+static int ht1380_rtc_get(struct udevice *dev, struct rtc_time *tm) +{

•   struct ht1380_priv *priv = dev_get_priv(dev);
  

•   int ret, i, bit, reg[N_REGS];
  

•   ret = dm_gpio_set_value(&priv->clk_desc, 0);
  

•   if (ret)
  

•           return ret;
  

•   ret = dm_gpio_set_dir_flags(&priv->dat_desc, GPIOD_IS_OUT);
  

•   if (ret)
  

•           return ret;
  

•   ret = ht1380_reset_off(priv);
  

•   if (ret)
  

•           goto exit;
  

•   ret = ht1380_send_byte(priv, BURST + READ);
  

•   if (ret)
  

•           goto exit;
  

•   ret = dm_gpio_set_dir_flags(&priv->dat_desc, GPIOD_IS_IN);
  

•   if (ret)
  

•           goto exit;
  

•   for (i = 0; i < N_REGS; i++) {
  

•           reg[i] = 0;
  

•           for (bit = 0; bit < 8; bit++) {
  

•                   ht1380_half_period_delay();
  

•                   ret = dm_gpio_set_value(&priv->clk_desc, 1);
  

•                   if (ret)
  

•                           goto exit;
  

•                   ht1380_half_period_delay();
  

•                   reg[i] |= dm_gpio_get_value(&priv->dat_desc) << bit;
  

•                   ret = dm_gpio_set_value(&priv->clk_desc, 0);
  

•                   if (ret)
  

•                           goto exit;
  

•           }
  

•   }
  

•   ret = -EINVAL;
  

•   /* Correctness check: some bits are always zero */
  

•   if ((reg[MIN] & 0x80) || (reg[HOUR] & 0x40) || (reg[MDAY] & 0xC0) ||
  

•       (reg[MONTH] & 0xE0) || (reg[WDAY] & 0xF8) || (reg[WP] & 0x7F))
  

•           goto exit;
  

Lower-case hex.

•   /* Correctness check: some registers are always > 0 */
  

•   if (!reg[MDAY] || !reg[MONTH] || !reg[WDAY])
  

•           goto exit;
  

•   tm->tm_sec = bcd2bin(reg[SEC]);
  

•   tm->tm_min = bcd2bin(reg[MIN]);
  

•   if (reg[HOUR] & 0x80) {
  

I suggest having an enum or #define for 0x80 and the 0x20 below

•           /* AM-PM Mode, range is 01-12 */
  

•           tm->tm_hour = bcd2bin(reg[HOUR] & 0x1F) % 12;
  

•           if (reg[HOUR] & 0x20) {
  

•                   /* it is PM (otherwise AM) */
  

•                   tm->tm_hour += 12;
  

•           }
  

•   } else {
  

•           /* 24-hour Mode, range is 0-23 */
  

•           tm->tm_hour = bcd2bin(reg[HOUR]);
  

•   }
  

•   tm->tm_mday = bcd2bin(reg[MDAY]);
  

•   tm->tm_mon = bcd2bin(reg[MONTH]);
  

•   tm->tm_year = 2000 + bcd2bin(reg[YEAR]);
  

•   tm->tm_wday = bcd2bin(reg[WDAY]) - 1;
  

•   tm->tm_yday = 0;
  

•   tm->tm_isdst = 0;
  

•   ret = 0;
  

+exit:

•   ht1380_reset_on(priv);
  

•   return ret;
  

+}

+static int ht1380_write_protection_off(struct ht1380_priv *priv) +{

•   int ret;
  

•   const int PROTECT = 0x8E;
  

Define at top of file, e.g. in an enum

•   ret = ht1380_reset_off(priv);
  

•   if (ret)
  

•           return ret;
  

•   ret = ht1380_send_byte(priv, PROTECT);
  

•   if (ret)
  

•           return ret;
  

•   ret = ht1380_send_byte(priv, 0); /* WP bit is 0 */
  

•   if (ret)
  

•           return ret;
  

•   return ht1380_reset_on(priv);
  

+}

+static int ht1380_rtc_set(struct udevice *dev, const struct rtc_time *tm) +{

•   struct ht1380_priv *priv = dev_get_priv(dev);
  

•   int ret, i, reg[N_REGS];
  

•   ret = dm_gpio_set_value(&priv->clk_desc, 0);
  

•   if (ret)
  

•           return ret;
  

•   ret = dm_gpio_set_dir_flags(&priv->dat_desc, GPIOD_IS_OUT);
  

•   if (ret)
  

•           goto exit;
  

•   ret = ht1380_write_protection_off(priv);
  

•   if (ret)
  

•           goto exit;
  

•   reg[SEC] = bin2bcd(tm->tm_sec);
  

•   reg[MIN] = bin2bcd(tm->tm_min);
  

•   reg[HOUR] = bin2bcd(tm->tm_hour);
  

•   reg[MDAY] = bin2bcd(tm->tm_mday);
  

•   reg[MONTH] = bin2bcd(tm->tm_mon);
  

•   reg[WDAY] = bin2bcd(tm->tm_wday) + 1;
  

•   reg[YEAR] = bin2bcd(tm->tm_year - 2000);
  

•   reg[WP] = 0x80; /* WP bit is 1 */
  

•   ret = ht1380_reset_off(priv);
  

•   if (ret)
  

•           goto exit;
  

•   ret = ht1380_send_byte(priv, BURST);
  

•   for (i = 0; i < N_REGS && ret; i++)
  

•           ret = ht1380_send_byte(priv, reg[i]);
  

+exit:

•   ht1380_reset_on(priv);
  

•   return ret;
  

+}

+static int ht1380_probe(struct udevice *dev) +{

•   int ret;
  

•   struct ht1380_priv *priv;
  

•   priv = dev_get_priv(dev);
  

•   if (!priv)
  

•           return -EINVAL;
  

•   ret = gpio_request_by_name(dev, "rst-gpio", 0,
  

•                              &priv->rst_desc, 0);
  

•   if (ret)
  

•           goto fail_rst;
  

•   ret = gpio_request_by_name(dev, "clk-gpio", 0,
  

•                              &priv->clk_desc, 0);
  

•   if (ret)
  

•           goto fail_clk;
  

•   ret = gpio_request_by_name(dev, "dat-gpio", 0,
  

•                              &priv->dat_desc, 0);
  

•   if (ret)
  

•           goto fail_dat;
  

•   ret = dm_gpio_set_dir_flags(&priv->clk_desc, GPIOD_IS_OUT);
  

•   if (ret)
  

•           goto fail;
  

•   ret = dm_gpio_set_dir_flags(&priv->rst_desc, GPIOD_IS_OUT);
  

•   if (ret)
  

•           goto fail;
  

•   ret = ht1380_reset_on(priv);
  

•   if (ret)
  

•           goto fail;
  

•   return 0;
  

+fail:

•   dm_gpio_free(dev, &priv->dat_desc);
  

+fail_dat:

•   dm_gpio_free(dev, &priv->clk_desc);
  

+fail_clk:

•   dm_gpio_free(dev, &priv->rst_desc);
  

+fail_rst:

•   return ret;
  

+}

+static int ht1380_remove(struct udevice *dev) +{

•   struct ht1380_priv *priv = dev_get_priv(dev);
  

•   dm_gpio_free(dev, &priv->rst_desc);
  

•   dm_gpio_free(dev, &priv->clk_desc);
  

•   dm_gpio_free(dev, &priv->dat_desc);
  

•   return 0;
  

+}

+static const struct rtc_ops ht1380_rtc_ops = {

•   .get = ht1380_rtc_get,
  

•   .set = ht1380_rtc_set,
  

+};

+static const struct udevice_id ht1380_rtc_ids[] = {

•   { .compatible = "holtek,ht1380" },
  

•   { }
  

+};

+U_BOOT_DRIVER(rtc_ht1380) = {

•   .name = "rtc-ht1380",
  

•   .id = UCLASS_RTC,
  

•   .probe = ht1380_probe,
  

•   .remove = ht1380_remove,
  

•   .of_match = ht1380_rtc_ids,
  

•   .ops = &ht1380_rtc_ops,
  

•   .priv_auto = sizeof(struct ht1380_priv),
  

+};

2.34.1

Regards, Simon