Re: [PATCH 03/13] RTC: ds1307: Add DS1341 specific power-saving options

From: Andrey Smirnov <andrew.smirnov@gmail.com>
To: Alexandre Belloni <alexandre.belloni@free-electrons.com>
Cc: Rob Herring <robh@kernel.org>,
	rtc-linux@googlegroups.com,
	Alessandro Zummo <a.zummo@towertech.it>,
	Pawel Moll <pawel.moll@arm.com>,
	Mark Rutland <mark.rutland@arm.com>,
	Ian Campbell <ijc+devicetree@hellion.org.uk>,
	Kumar Gala <galak@codeaurora.org>,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 03/13] RTC: ds1307: Add DS1341 specific power-saving options
Date: Wed, 20 Jul 2016 07:36:55 -0700	[thread overview]
Message-ID: <CAHQ1cqEoN7PEQApHBo-FOqumtLJWoURHOPZfrzL=6h2p4S0Fdg@mail.gmail.com> (raw)
In-Reply-To: <20160720090220.GQ7132@piout.net>

On Wed, Jul 20, 2016 at 2:02 AM, Alexandre Belloni
<alexandre.belloni@free-electrons.com> wrote:
> On 19/07/2016 at 16:56:56 -0700, Andrey Smirnov wrote :
>> >> I don't see any value in doing that, could you give me a realistic
>> >> example of a scenario in which a user would want to spend some of
>> >> uptime with RTC oscillator fault detection/glitch filtering disabled
>> >> and then enable it?
>> >>
>> >
>> > Well, the issue is not being dynamic, it is differentiating between
>> > hardware description and user configuration. Configuration must not be in
>> > DT.
>>
>> Why? And I don't mean in a generic sense, but in this particular case.
>> What is gained by not having this bit of configuration, whose only
>> consumer is the driver, in the device tree file?
>>
>
> Because configuration doesn't belong to DT. DT is about hardware
> description, not configuration.

That doesn't really answer my question. You just re-iterating some
maxim without explaining what is the point behind applying it.

>
>
>> > And this choice is definitively not hardware related (as opposed to
>> > the trickle charging that depends on the battery that is used on the
>> > board).
>>
>> There's most certainly plenty of precedents of non hardware-related in
>> device tree, first two that come to mind are "chosen" node and
>> "local-mac-address" property and, granted, those might be
>> exceptions/legacy bindings that are just there to stay, but even if
>> you look at RTC subsystem rtc-cmos.txt, atmel,at91sam-rtc.txt and
>> possibly rtc-st-lpc.txt are providing bindings that are not exactly
>> hardware related.
>>
>> Rtc-cmos.txt is especially noticeable example since it literally does
>> what I am trying to do -- allows the user to specify initial values to
>> certain registers that would be initialized by the driver.
>>
>
> Well, the RTC subsystem has been left unmaintained for a while and it is
> not because we made mistakes in the past that we have to make them
> again.
>
> rtc-st-lpc is only hardware related. The mode it is used in is board
> dependant. And I have a plan to change how the gpbr register is
> allocated for the at91 RTT. I agree that rtc-cmos is an example of what
> not to do.
>
>> > Well, it doesn't leak abstraction as long as all the RTC that are able
>> > to disable the oscillator failure detection use the same ABI.
>>
>> Correct me if I am wrong, but no such, at least semi-standardized, ABI
>> exist as of today, correct? If that is so, then what you are saying is
>> that the abstraction doesn't leak as long as you put it inside of a
>> new abstraction that doesn't leak. I am not arguing that it is
>> impossible to create a new one that would allow to hide hardware
>> differences, I am positive it is, what I am arguing is that to do so
>> is a lot of work for as far as I can see for no gain.
>>
>
> You are correct, that ABI doesn't exist and I'm asking to create it.
> That is how kernel development happens.

OK, I don't think this is going anywhere. I am sure by now you know
very well, that I am not going to agree to that. I'll just drop this
part, gut the patchset to it's bare minimum and re-submit it.

Andrey