Re: [PATCH v2] rtc: ds1307: add basic support for ds1341 chip

[Alexandre Belloni]

Hi,

On 24/08/2017 at 09:32:11 +0300, Nikita Yushchenko wrote:
> This adds support for reading and writing date/time from/to ds1341 chip.
> 
> ds1341 chip has other features - alarms, input clock (can be used instead
> of intercal oscillator for better accuracy), output clock ("square wave
> generation"). However, not all of that is available at the same time.
> Same chip pins, CLKIN/nINTA and SQW/nINTB, can be used either for
> input/output clocks, or for alarm interrupts. Role of these pins on
> particular board depends on hardware wiring.
> 
> We can add device tree properties that describe if each of pins is wired
> as clock, or as interrupt, or left unconnected, and enable support for
> corresponding functionality based on that. But that is cumbersome, requires
> hardware for testing, and has to deal with bit enabling/disabling output
> clock also affects which pins alarm interrupts are routed to.
> 
> Another factor is that there are hardware setups (i.e. ZII RDU2) that
> power DS1341 from SuperCap, which makes power saving critical. For such
> setups, kernel driver should leave register bits that control mentioned
> pins in the state configured by bootloader.
> 
> Given all that, it was decided to limit support to "only date/time" for
> now. That is enough for common use case. Full (and cumbersome)
> implementation can be added later if ever needed.
> 
> Signed-off-by: Nikita Yushchenko <nikita.yoush@xxxxxxxxxxxxxxxxxx>
> Reviewed-by: Linus Walleij <linus.walleij@xxxxxxxxxx>
> Tested-by: Aleksander Morgado <aleksander@xxxxxxxxxxxxx>
> ---
>  drivers/rtc/Kconfig      | 10 +++++-----
>  drivers/rtc/rtc-ds1307.c | 13 +++++++++++++
>  2 files changed, 18 insertions(+), 5 deletions(-)

I've applied but...

> @@ -1323,6 +1334,7 @@ static int ds1307_probe(struct i2c_client *client,
>  	static const int	bbsqi_bitpos[] = {
>  		[ds_1337] = 0,
>  		[ds_1339] = DS1339_BIT_BBSQI,
> +		[ds_1341] = 0,
>  		[ds_3231] = DS3231_BIT_BBSQW,
>  	};
>  	const struct rtc_class_ops *rtc_ops = &ds13xx_rtc_ops;

I had to remove that change following http://patchwork.ozlabs.org/patch/787009/

-- 
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com