[U-Boot] [PATCH v2] rtc: ds3232/ds3231: Add support to generate 32KHz output for driver module

[U-Boot] [PATCH v2] rtc: ds3232/ds3231: Add support to generate 32KHz output for driver module

[Chuanhua Han]

This patch add an implementation of the rtc_enable_32khz_output() that
uses the driver model i2c APIs.

Signed-off-by: Chuanhua Han <chuanhua.han@nxp.com>
---
Change in v2:
	- Add RTC_ENABLE_32KHZ_OUTPUT option so this code compiles only 
in that cases where it is really useful.

 drivers/rtc/Kconfig  | 10 ++++++++++
 drivers/rtc/ds3231.c | 21 +++++++++++++++++++++
 include/rtc.h        |  6 ++++++
 3 files changed, 37 insertions(+)

diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
index fd0009b..040d241 100644
--- a/drivers/rtc/Kconfig
+++ b/drivers/rtc/Kconfig
@@ -31,6 +31,12 @@ config TPL_DM_RTC
 	  drivers to perform the actual functions. See rtc.h for a
 	  description of the API.
 
+config RTC_ENABLE_32KHZ_OUTPUT
+	bool "Enable RTC 32Khz output"
+	help
+	   Some real-time clocks support the output of 32kHz square waves (such as ds3231),
+	   the config symbol choose Real Time Clock device 32Khz output feature.
+
 config RTC_PCF2127
 	bool "Enable PCF2127 driver"
 	depends on DM_RTC
@@ -41,6 +47,10 @@ config RTC_PCF2127
 	  has a selectable I2C-bus or SPI-bus, a backup battery switch-over circuit, a
 	  programmable watchdog function, a timestamp function, and many other features.
 
+config DS3231_BUS_NUM
+	hex "I2C bus of the DS3231 device"
+	default 0
+
 config RTC_DS1307
 	bool "Enable DS1307 driver"
 	depends on DM_RTC
diff --git a/drivers/rtc/ds3231.c b/drivers/rtc/ds3231.c
index 79b026a..dbd77a6 100644
--- a/drivers/rtc/ds3231.c
+++ b/drivers/rtc/ds3231.c
@@ -148,11 +148,13 @@ void rtc_reset (void)
 /*
  * Enable 32KHz output
  */
+#ifdef CONFIG_RTC_ENABLE_32KHZ_OUTPUT
 void rtc_enable_32khz_output(void)
 {
 	rtc_write(RTC_STAT_REG_ADDR,
 		  RTC_STAT_BIT_BB32KHZ | RTC_STAT_BIT_EN32KHZ);
 }
+#endif
 
 /*
  * Helper functions
@@ -251,6 +253,25 @@ static int ds3231_probe(struct udevice *dev)
 	return 0;
 }
 
+#ifdef CONFIG_RTC_ENABLE_32KHZ_OUTPUT
+void rtc_enable_32khz_output(void)
+{
+	int ret;
+	struct udevice *dev;
+
+#ifdef CONFIG_DS3231_BUS_NUM
+	ret = i2c_get_chip_for_busnum(CONFIG_DS3231_BUS_NUM,
+				      CONFIG_SYS_I2C_RTC_ADDR, 1, &dev);
+#else
+	ret = i2c_get_chip_for_busnum(0, CONFIG_SYS_I2C_RTC_ADDR, 1, &dev);
+#endif
+	if (!ret)
+		dm_i2c_reg_write(dev, RTC_STAT_REG_ADDR,
+				 RTC_STAT_BIT_BB32KHZ |
+				 RTC_STAT_BIT_EN32KHZ);
+}
+#endif
+
 static const struct rtc_ops ds3231_rtc_ops = {
 	.get = ds3231_rtc_get,
 	.set = ds3231_rtc_set,
diff --git a/include/rtc.h b/include/rtc.h
index b255bdc..df7de09 100644
--- a/include/rtc.h
+++ b/include/rtc.h
@@ -166,11 +166,17 @@ int rtc_read32(struct udevice *dev, unsigned int reg, u32 *valuep);
  */
 int rtc_write32(struct udevice *dev, unsigned int reg, u32 value);
 
+#ifdef CONFIG_RTC_ENABLE_32KHZ_OUTPUT
+void rtc_enable_32khz_output(void);
+#endif
+
 #else
 int rtc_get (struct rtc_time *);
 int rtc_set (struct rtc_time *);
 void rtc_reset (void);
+#ifdef CONFIG_RTC_ENABLE_32KHZ_OUTPUT
 void rtc_enable_32khz_output(void);
+#endif
 
 /**
  * rtc_read8() - Read an 8-bit register
-- 
2.9.5

[U-Boot] [PATCH v2] rtc: ds3232/ds3231: Add support to generate 32KHz output for driver module

[Wolfgang Denk]

Dear Chuanhua Han,

In message <20190723095745.37138-1-chuanhua.han@nxp.com> you wrote:
> This patch add an implementation of the rtc_enable_32khz_output() that
> uses the driver model i2c APIs.
>
> Signed-off-by: Chuanhua Han <chuanhua.han@nxp.com>
> ---
> Change in v2:
> 	- Add RTC_ENABLE_32KHZ_OUTPUT option so this code compiles only 
> in that cases where it is really useful.

So when exactly is it useful?

If I understand correctly, there are no users of this code in
mainline.  Should the patch then not be part of a patch series that
adds such users?

Adding potentially "useful" code just on speculation is not nice
maintenance-wise.

I recommend to withdraw this patch and submit it together with some
real consumer of this feature.

Thanks.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"The question of whether a computer can think is no more  interesting
than the question of whether a submarine can swim"
                                                - Edsgar W.  Dijkstra

[U-Boot] [EXT] Re: [PATCH v2] rtc: ds3232/ds3231: Add support to generate 32KHz output for driver module

[Chuanhua Han]

Dear Wolfgang Denk

> -----Original Message-----
> From: Wolfgang Denk <wd@denx.de>
> Sent: 2019年7月23日 23:05
> To: Chuanhua Han <chuanhua.han@nxp.com>
> Cc: lukma at denx.de; trini at konsulko.com; Prabhakar Kushwaha
> <prabhakar.kushwaha@nxp.com>; u-boot at lists.denx.de
> Subject: [EXT] Re: [PATCH v2] rtc: ds3232/ds3231: Add support to generate
> 32KHz output for driver module
> 
> Caution: EXT Email
> 
> Dear Chuanhua Han,
> 
> In message <20190723095745.37138-1-chuanhua.han@nxp.com> you wrote:
> > This patch add an implementation of the rtc_enable_32khz_output() that
> > uses the driver model i2c APIs.
> >
> > Signed-off-by: Chuanhua Han <chuanhua.han@nxp.com>
> > ---
> > Change in v2:
> >       - Add RTC_ENABLE_32KHZ_OUTPUT option so this code compiles
> only
> > in that cases where it is really useful.
> 
> So when exactly is it useful?
> 
> If I understand correctly, there are no users of this code in mainline.  Should
> the patch then not be part of a patch series that adds such users?
> 
> Adding potentially "useful" code just on speculation is not nice
> maintenance-wise.
> 
> I recommend to withdraw this patch and submit it together with some real
> consumer of this feature.
Ok, I will send the current patch together with the one that actually uses this function. Thanks for your advice!
> 
> Thanks.
> 
> Best regards,
> 
> Wolfgang Denk
> 
> --
> DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
> "The question of whether a computer can think is no more  interesting than
> the question of whether a submarine can swim"
>                                                 - Edsgar W.  Dijkstra