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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20170824211940.3hxktw4slodvxcul@piout.net>
Date:   Thu, 24 Aug 2017 23:19:40 +0200
From:   Alexandre Belloni <alexandre.belloni@...e-electrons.com>
To:     Nikita Yushchenko <nikita.yoush@...entembedded.com>
Cc:     Alessandro Zummo <a.zummo@...ertech.it>,
        Heiner Kallweit <hkallweit1@...il.com>,
        Linus Walleij <linus.walleij@...aro.org>,
        Arnaud Ebalard <arno@...isbad.org>,
        David Lowe <dave-lowe@...world.com>,
        Javier Martinez Canillas <javier@....samsung.com>,
        Marek Vasut <marex@...x.de>, Tin Huynh <tnhuynh@....com>,
        linux-rtc@...r.kernel.org, linux-kernel@...r.kernel.org,
        Andrey Smirnov <andrew.smirnov@...il.com>,
        Aleksander Morgado <aleksander@...ksander.es>,
        Chris Healy <cphealy@...il.com>
Subject: Re: [PATCH v2] rtc: ds1307: add basic support for ds1341 chip

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@...entembedded.com>
> Reviewed-by: Linus Walleij <linus.walleij@...aro.org>
> Tested-by: Aleksander Morgado <aleksander@...ksander.es>
> ---
>  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