Re: [PATCH] rtc: add ht1380 driver
Hi Sergei, On Fri, 9 Dec 2022 at 00:50, Sergei Antonov wrote: > > On Thu, 8 Dec 2022 at 00:52, Simon Glass wrote: > > > > Is this some sort of I2C protocol? > > > > > > Like I2C it uses a pin for clock and a pin for data in/out. Unlike I2C > > > it does not use addressing. I am not sure whether this driver can > > > utilize some of the existing I2C code in U-Boot. Wrote my own bit > > > banging routines. > > > > Yes you can use i2c, by setting the offset_len to 0, e.g. with: > > > >u-boot,i2c-offset-len = <0>; > > I2C transmits/receives bytes most significant bit first (msb), but > this device needs lsb. Also it seems that it is not actually using the i2c protocol, even with that change. Regards, Simon
Re: [PATCH] rtc: add ht1380 driver
On Thu, 8 Dec 2022 at 00:52, Simon Glass wrote: > > > Is this some sort of I2C protocol? > > > > Like I2C it uses a pin for clock and a pin for data in/out. Unlike I2C > > it does not use addressing. I am not sure whether this driver can > > utilize some of the existing I2C code in U-Boot. Wrote my own bit > > banging routines. > > Yes you can use i2c, by setting the offset_len to 0, e.g. with: > >u-boot,i2c-offset-len = <0>; I2C transmits/receives bytes most significant bit first (msb), but this device needs lsb.
Re: [PATCH] rtc: add ht1380 driver
Hi Sergei,
On Thu, 8 Dec 2022 at 06:09, Sergei Antonov wrote:
>
> On Wed, 7 Dec 2022 at 04:08, Simon Glass wrote:
> >
> > Hi Sergei,
> >
> > On Tue, 6 Dec 2022 at 23:07, Sergei Antonov wrote:
> > >
> > > Support Holtek HT1380/HT1381 Serial Timekeeper Chip. It provides seconds
> > > , minutes, hours, day of the week, date, month and year information.
> > >
> > > Datasheet:
> > > https://www.holtek.com.tw/documents/10179/11842/ht1380_1v130.pdf
> > >
> > > Signed-off-by: Sergei Antonov
> > > ---
> > >
> > > v2:
> > > * The RESET pin is now to be described as ACTIVE_LOW in dts.
> > >
> > > Changes suggested by Simon Glass:
> > > * a more detailed driver description in Kconfig
> > > * multi-line comments' style
> > > * enum for 0x80 and the 0x20 at top of file
> > > * lower-case hex constants
> > > * function comments for ht1380_reset_on/off
> > > * blank line before returns
> > >
> > > PROTECT remains in a function scope for the sake of locality of
> > > definitions.
> > >
> > > drivers/rtc/Kconfig | 8 +
> > > drivers/rtc/Makefile | 1 +
> > > drivers/rtc/ht1380.c | 337 +++
> > > 3 files changed, 346 insertions(+)
> > > create mode 100644 drivers/rtc/ht1380.c
> > >
[..]
> > > +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(>clk_desc, 0);
> > > + if (ret)
> > > + return ret;
> > > +
> > > + ret = dm_gpio_set_dir_flags(>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(>dat_desc, GPIOD_IS_IN);
> > > + if (ret)
> > > + goto exit;
> > > +
> >
> > Is this some sort of I2C protocol?
>
> Like I2C it uses a pin for clock and a pin for data in/out. Unlike I2C
> it does not use addressing. I am not sure whether this driver can
> utilize some of the existing I2C code in U-Boot. Wrote my own bit
> banging routines.
Yes you can use i2c, by setting the offset_len to 0, e.g. with:
u-boot,i2c-offset-len = <0>;
>
> > > + 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(>clk_desc, 1);
> > > + if (ret)
> > > + goto exit;
> > > + ht1380_half_period_delay();
> > > +
> > > + reg[i] |= dm_gpio_get_value(>dat_desc) <<
> > > bit;
> > > + ret = dm_gpio_set_value(>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;
> >
> > Drop extra brackets ?
>
> OK. Will be done in v3. I put extra brackets expecting reviewers to
> criticize code readability :).
:)
Regards,
Simon
Re: [PATCH] rtc: add ht1380 driver
On Wed, 7 Dec 2022 at 04:08, Simon Glass wrote:
>
> Hi Sergei,
>
> On Tue, 6 Dec 2022 at 23:07, Sergei Antonov wrote:
> >
> > Support Holtek HT1380/HT1381 Serial Timekeeper Chip. It provides seconds
> > , minutes, hours, day of the week, date, month and year information.
> >
> > Datasheet:
> > https://www.holtek.com.tw/documents/10179/11842/ht1380_1v130.pdf
> >
> > Signed-off-by: Sergei Antonov
> > ---
> >
> > v2:
> > * The RESET pin is now to be described as ACTIVE_LOW in dts.
> >
> > Changes suggested by Simon Glass:
> > * a more detailed driver description in Kconfig
> > * multi-line comments' style
> > * enum for 0x80 and the 0x20 at top of file
> > * lower-case hex constants
> > * function comments for ht1380_reset_on/off
> > * blank line before returns
> >
> > PROTECT remains in a function scope for the sake of locality of definitions.
> >
> > drivers/rtc/Kconfig | 8 +
> > drivers/rtc/Makefile | 1 +
> > drivers/rtc/ht1380.c | 337 +++
> > 3 files changed, 346 insertions(+)
> > create mode 100644 drivers/rtc/ht1380.c
> >
> > diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
> > index 23963271928a..eed48e35a578 100644
> > --- a/drivers/rtc/Kconfig
> > +++ b/drivers/rtc/Kconfig
> > @@ -220,4 +220,12 @@ 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 IC which provides seconds, minutes, hours,
> > + day of the week, date, month and year information.
>
> Perhaps mention how it is connected, i.e. three GPIOs.
>
> > +
> > endmenu
> > diff --git a/drivers/rtc/Makefile b/drivers/rtc/Makefile
> > index 009dd9d28c95..f3164782b605 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 ..25335227d893
> > --- /dev/null
> > +++ b/drivers/rtc/ht1380.c
> > @@ -0,0 +1,337 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Holtek HT1380/HT1381 Serial Timekeeper Chip
> > + *
> > + * Communication with the chip is vendor-specific.
> > + * It is done via 3 GPIO pins: reset, clock, and data.
> > + * Describe in .dts this way:
> > + *
> > + * rtc {
> > + * compatible = "holtek,ht1380";
> > + * rst-gpio = < 19 GPIO_ACTIVE_LOW>;
> > + * clk-gpio = < 20 GPIO_ACTIVE_HIGH>;
> > + * dat-gpio = < 21 GPIO_ACTIVE_HIGH>;
> > + * };
>
> Is there a binding file for this?
Not in the U-Boot repo yet. I am going to submit a .dts file binding
to this driver. But I need to submit 2 more device drivers first. I
guess, submitting .dts should be the final step.
> I believe the standard name should be rst-gpios (i.e. plural)?
I grepped files in arch/arm/dts and it was not clear. There were more
plural names however. So I can change it to plural in v3.
> > + *
> > + */
> > +
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +
> > +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
> > +};
> > +
> > +enum hour_mode {
> > + AMPM_MODE = 0x80, /* RTC is in AM/PM mode */
> > + PM_NOW = 0x20,/* set if PM, clear if AM */
> > +};
> > +
> > +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.
> > +*/
> > + 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(>dat_desc, byte >> bit & 1);
> > + if (ret)
> > + break;
> > + ht1380_half_period_delay();
> > +
> > + ret = dm_gpio_set_value(>clk_desc, 1);
> > + if (ret)
> > + break;
> > + ht1380_half_period_delay();
> > +
> > + ret =
Re: [PATCH] rtc: add ht1380 driver
On Tue, Dec 06, 2022 at 01:06:59PM +0300, Sergei Antonov wrote: > Support Holtek HT1380/HT1381 Serial Timekeeper Chip. It provides seconds > , minutes, hours, day of the week, date, month and year information. > > Datasheet: > https://www.holtek.com.tw/documents/10179/11842/ht1380_1v130.pdf > > Signed-off-by: Sergei Antonov Please enable this on sandbox so that it gets build tested and not removed later as unused / dead code, thanks. -- Tom signature.asc Description: PGP signature
Re: [PATCH] rtc: add ht1380 driver
Hi Sergei,
On Tue, 6 Dec 2022 at 23:07, Sergei Antonov wrote:
>
> Support Holtek HT1380/HT1381 Serial Timekeeper Chip. It provides seconds
> , minutes, hours, day of the week, date, month and year information.
>
> Datasheet:
> https://www.holtek.com.tw/documents/10179/11842/ht1380_1v130.pdf
>
> Signed-off-by: Sergei Antonov
> ---
>
> v2:
> * The RESET pin is now to be described as ACTIVE_LOW in dts.
>
> Changes suggested by Simon Glass:
> * a more detailed driver description in Kconfig
> * multi-line comments' style
> * enum for 0x80 and the 0x20 at top of file
> * lower-case hex constants
> * function comments for ht1380_reset_on/off
> * blank line before returns
>
> PROTECT remains in a function scope for the sake of locality of definitions.
>
> drivers/rtc/Kconfig | 8 +
> drivers/rtc/Makefile | 1 +
> drivers/rtc/ht1380.c | 337 +++
> 3 files changed, 346 insertions(+)
> create mode 100644 drivers/rtc/ht1380.c
>
> diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
> index 23963271928a..eed48e35a578 100644
> --- a/drivers/rtc/Kconfig
> +++ b/drivers/rtc/Kconfig
> @@ -220,4 +220,12 @@ 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 IC which provides seconds, minutes, hours,
> + day of the week, date, month and year information.
Perhaps mention how it is connected, i.e. three GPIOs.
> +
> endmenu
> diff --git a/drivers/rtc/Makefile b/drivers/rtc/Makefile
> index 009dd9d28c95..f3164782b605 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 ..25335227d893
> --- /dev/null
> +++ b/drivers/rtc/ht1380.c
> @@ -0,0 +1,337 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Holtek HT1380/HT1381 Serial Timekeeper Chip
> + *
> + * Communication with the chip is vendor-specific.
> + * It is done via 3 GPIO pins: reset, clock, and data.
> + * Describe in .dts this way:
> + *
> + * rtc {
> + * compatible = "holtek,ht1380";
> + * rst-gpio = < 19 GPIO_ACTIVE_LOW>;
> + * clk-gpio = < 20 GPIO_ACTIVE_HIGH>;
> + * dat-gpio = < 21 GPIO_ACTIVE_HIGH>;
> + * };
Is there a binding file for this?
I believe the standard name should be rst-gpios (i.e. plural)?
> + *
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +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
> +};
> +
> +enum hour_mode {
> + AMPM_MODE = 0x80, /* RTC is in AM/PM mode */
> + PM_NOW = 0x20,/* set if PM, clear if AM */
> +};
> +
> +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.
> +*/
> + 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(>dat_desc, byte >> bit & 1);
> + if (ret)
> + break;
> + ht1380_half_period_delay();
> +
> + ret = dm_gpio_set_value(>clk_desc, 1);
> + if (ret)
> + break;
> + ht1380_half_period_delay();
> +
> + ret = dm_gpio_set_value(>clk_desc, 0);
> + if (ret)
> + break;
> + }
> +
> + return ret;
> +}
> +
> +/*
> + * Leave reset state. The transfer operation can then be started.
> + */
> +static int ht1380_reset_off(struct ht1380_priv *priv)
> +{
> + const unsigned int T_CC = 4; /* us, Reset to Clock Setup */
> + int ret;
> +
> + /*
> +* Leave RESET state.
> +* Make sure we make the minimal delay required by the datasheet.
> +*/
> + ret = dm_gpio_set_value(>rst_desc, 0);
> + udelay(T_CC);
> +
> + return ret;
> +}
> +
> +/*
> + * Enter reset state. Completes the
[PATCH] rtc: add ht1380 driver
Support Holtek HT1380/HT1381 Serial Timekeeper Chip. It provides seconds
, minutes, hours, day of the week, date, month and year information.
Datasheet:
https://www.holtek.com.tw/documents/10179/11842/ht1380_1v130.pdf
Signed-off-by: Sergei Antonov
---
v2:
* The RESET pin is now to be described as ACTIVE_LOW in dts.
Changes suggested by Simon Glass:
* a more detailed driver description in Kconfig
* multi-line comments' style
* enum for 0x80 and the 0x20 at top of file
* lower-case hex constants
* function comments for ht1380_reset_on/off
* blank line before returns
PROTECT remains in a function scope for the sake of locality of definitions.
drivers/rtc/Kconfig | 8 +
drivers/rtc/Makefile | 1 +
drivers/rtc/ht1380.c | 337 +++
3 files changed, 346 insertions(+)
create mode 100644 drivers/rtc/ht1380.c
diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
index 23963271928a..eed48e35a578 100644
--- a/drivers/rtc/Kconfig
+++ b/drivers/rtc/Kconfig
@@ -220,4 +220,12 @@ 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 IC which provides seconds, minutes, hours,
+ day of the week, date, month and year information.
+
endmenu
diff --git a/drivers/rtc/Makefile b/drivers/rtc/Makefile
index 009dd9d28c95..f3164782b605 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 ..25335227d893
--- /dev/null
+++ b/drivers/rtc/ht1380.c
@@ -0,0 +1,337 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Holtek HT1380/HT1381 Serial Timekeeper Chip
+ *
+ * Communication with the chip is vendor-specific.
+ * It is done via 3 GPIO pins: reset, clock, and data.
+ * Describe in .dts this way:
+ *
+ * rtc {
+ * compatible = "holtek,ht1380";
+ * rst-gpio = < 19 GPIO_ACTIVE_LOW>;
+ * clk-gpio = < 20 GPIO_ACTIVE_HIGH>;
+ * dat-gpio = < 21 GPIO_ACTIVE_HIGH>;
+ * };
+ *
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+
+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
+};
+
+enum hour_mode {
+ AMPM_MODE = 0x80, /* RTC is in AM/PM mode */
+ PM_NOW = 0x20,/* set if PM, clear if AM */
+};
+
+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.
+*/
+ 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(>dat_desc, byte >> bit & 1);
+ if (ret)
+ break;
+ ht1380_half_period_delay();
+
+ ret = dm_gpio_set_value(>clk_desc, 1);
+ if (ret)
+ break;
+ ht1380_half_period_delay();
+
+ ret = dm_gpio_set_value(>clk_desc, 0);
+ if (ret)
+ break;
+ }
+
+ return ret;
+}
+
+/*
+ * Leave reset state. The transfer operation can then be started.
+ */
+static int ht1380_reset_off(struct ht1380_priv *priv)
+{
+ const unsigned int T_CC = 4; /* us, Reset to Clock Setup */
+ int ret;
+
+ /*
+* Leave RESET state.
+* Make sure we make the minimal delay required by the datasheet.
+*/
+ ret = dm_gpio_set_value(>rst_desc, 0);
+ udelay(T_CC);
+
+ return ret;
+}
+
+/*
+ * Enter reset state. Completes the transfer operation.
+ */
+static int ht1380_reset_on(struct ht1380_priv *priv)
+{
+ 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(>rst_desc, 1);
+ 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,
Re: [PATCH] rtc: add ht1380 driver
On Mon, Nov 21, 2022 at 05:47:16PM +0300, Sergei Antonov wrote:
> On Wed, 26 Oct 2022 at 14:34, Sergei Antonov wrote:
> >
> > On Wed, 26 Oct 2022 at 02:35, Simon Glass wrote:
> >
> > > > +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.
> >
> > OK. I was confused by doc/develop/codingstyle.rst:
> > * The exception for net files to the `multi-line comment
> > `_
> > applies only to Linux, not to U-Boot.
> > What is the U-Boot style of multi-line comments remains a mystery to me.
>
> Tom,
> could you please clarify what is U-Boot style for multi-line comments?
> I can not draw it from doc/develop/codingstyle.rst
I'll make an attempt at re-wording the line there, but, per:
https://www.kernel.org/doc/html/latest/process/coding-style.html#commenting
/*
* It should be
* like this.
*/
--
Tom
signature.asc
Description: PGP signature
Re: [PATCH] rtc: add ht1380 driver
On Wed, 26 Oct 2022 at 14:34, Sergei Antonov wrote:
>
> On Wed, 26 Oct 2022 at 02:35, Simon Glass wrote:
>
> > > +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.
>
> OK. I was confused by doc/develop/codingstyle.rst:
> * The exception for net files to the `multi-line comment
> `_
> applies only to Linux, not to U-Boot.
> What is the U-Boot style of multi-line comments remains a mystery to me.
Tom,
could you please clarify what is U-Boot style for multi-line comments?
I can not draw it from doc/develop/codingstyle.rst
Re: [PATCH] rtc: add ht1380 driver
On Wed, 26 Oct 2022 at 02:35, Simon Glass wrote:
> > +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.
OK. I was confused by doc/develop/codingstyle.rst:
* The exception for net files to the `multi-line comment
`_
applies only to Linux, not to U-Boot.
What is the U-Boot style of multi-line comments remains a mystery to me.
> > +static int ht1380_reset_on(struct ht1380_priv *priv)
>
> function comment - does it stay in reset state or automatically exit?
It stays in reset state. Until the counterpart function
ht1380_reset_off is called.
> > + /* 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
Oh. I hoped so much for /* */ comments put after lines with 0x80 and 0x20.
> > + /* 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
What about locality of definitions?
Thanks for your quick comments.
Re: [PATCH] rtc: add ht1380 driver
Hi Sergei,
On Tue, 25 Oct 2022 at 08:24, Sergei Antonov 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
> ---
> 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 ..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
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +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(>dat_desc, byte >> bit & 1);
> + if (ret)
> + break;
> + ht1380_half_period_delay();
> +
> + ret = dm_gpio_set_value(>clk_desc, 1);
> + if (ret)
> + break;
> + ht1380_half_period_delay();
> +
> + ret = dm_gpio_set_value(>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(>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(>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(>clk_desc, 0);
> + if (ret)
> + return ret;
> +
> + ret = dm_gpio_set_dir_flags(>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(>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++) {
> +
[PATCH] rtc: add ht1380 driver
Support Holtek HT1380/HT1381 Serial Timekeeper Chip.
Datasheet:
https://www.holtek.com.tw/documents/10179/11842/ht1380_1v130.pdf
Signed-off-by: Sergei Antonov
---
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
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.
+
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 ..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
+#include
+#include
+#include
+#include
+#include
+
+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. */
+ 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(>dat_desc, byte >> bit & 1);
+ if (ret)
+ break;
+ ht1380_half_period_delay();
+
+ ret = dm_gpio_set_value(>clk_desc, 1);
+ if (ret)
+ break;
+ ht1380_half_period_delay();
+
+ ret = dm_gpio_set_value(>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(>rst_desc, 1);
+ udelay(T_CC);
+ return ret;
+}
+
+static int ht1380_reset_on(struct ht1380_priv *priv)
+{
+ 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(>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(>clk_desc, 0);
+ if (ret)
+ return ret;
+
+ ret = dm_gpio_set_dir_flags(>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(>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(>clk_desc, 1);
+ if (ret)
+ goto exit;
+ ht1380_half_period_delay();
+
+ reg[i] |= dm_gpio_get_value(>dat_desc) << bit;
+ ret = dm_gpio_set_value(>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) ||
+
