| Message ID | 20190723095745.37138-1-chuanhua.han@nxp.com |
|---|---|
| State | Superseded |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | [U-Boot,v2] rtc: ds3232/ds3231: Add support to generate 32KHz output for driver module | expand |
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
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@denx.de; trini@konsulko.com; Prabhakar Kushwaha > <prabhakar.kushwaha@nxp.com>; u-boot@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@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
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
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(+)