From: Alexandre Belloni <alexandre.belloni@free-electrons.com> To: Andrey Smirnov <andrew.smirnov@gmail.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 11:02:20 +0200 [thread overview] Message-ID: <20160720090220.GQ7132@piout.net> (raw) In-Reply-To: <CAHQ1cqF+Z5sKhtxgFiTGJdaOtkQRtQmXU=DzG0qxnDDwfAMp_g@mail.gmail.com> 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. > > 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. > > > >> > - on subsequent reboots, the mode is kept alongside the time and date > >> > > >> > >> This assumes that your bootloader leaves those mode bits alone. > >> > > > > Well, if that is not the case, the bootloader as to be fixed anyway and > > silently changing the configuration back using DT is probably worse. > > > > How so? Consider the following two scenarios with assumption that the > bootloader is broken: > > - Bits set wrong by bootloader, then corrected by kernel, device is > powered off RTC consumes expected amount of current > > - Bits set wrong by bootloader, kernel does nothing, device is > powered off RTC consumes more than anticipated and we drain the power > storage device and loose time. > > What do you you think former is worse than latter? > Whether is is done in the kernel or in userspace doesn't change much to that use case. -- Alexandre Belloni, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com
WARNING: multiple messages have this Message-ID (diff)
From: Alexandre Belloni <alexandre.belloni@free-electrons.com> To: Andrey Smirnov <andrew.smirnov@gmail.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: [rtc-linux] Re: [PATCH 03/13] RTC: ds1307: Add DS1341 specific power-saving options Date: Wed, 20 Jul 2016 11:02:20 +0200 [thread overview] Message-ID: <20160720090220.GQ7132@piout.net> (raw) In-Reply-To: <CAHQ1cqF+Z5sKhtxgFiTGJdaOtkQRtQmXU=DzG0qxnDDwfAMp_g@mail.gmail.com> 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. > > 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. > > > >> > - on subsequent reboots, the mode is kept alongside the time and date > >> > > >> > >> This assumes that your bootloader leaves those mode bits alone. > >> > > > > Well, if that is not the case, the bootloader as to be fixed anyway and > > silently changing the configuration back using DT is probably worse. > > > > How so? Consider the following two scenarios with assumption that the > bootloader is broken: > > - Bits set wrong by bootloader, then corrected by kernel, device is > powered off RTC consumes expected amount of current > > - Bits set wrong by bootloader, kernel does nothing, device is > powered off RTC consumes more than anticipated and we drain the power > storage device and loose time. > > What do you you think former is worse than latter? > Whether is is done in the kernel or in userspace doesn't change much to that use case. -- Alexandre Belloni, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com -- You received this message because you are subscribed to "rtc-linux". Membership options at http://groups.google.com/group/rtc-linux . Please read http://groups.google.com/group/rtc-linux/web/checklist before submitting a driver. --- You received this message because you are subscribed to the Google Groups "rtc-linux" group. To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
next prev parent reply other threads:[~2016-07-20 9:02 UTC|newest] Thread overview: 66+ messages / expand[flat|nested] mbox.gz Atom feed top 2016-06-15 5:59 [PATCH 00/13] DS1341 support and code cleanup Andrey Smirnov 2016-06-15 5:59 ` [rtc-linux] " Andrey Smirnov 2016-06-15 5:59 ` [PATCH 01/13] RTC: ds1307: Add DS1341 variant Andrey Smirnov 2016-06-15 5:59 ` [rtc-linux] " Andrey Smirnov 2016-06-15 5:59 ` [PATCH 02/13] RTC: ds1307: Disable square wave and timers as default Andrey Smirnov 2016-06-15 5:59 ` [rtc-linux] " Andrey Smirnov 2016-06-15 5:59 ` [PATCH 03/13] RTC: ds1307: Add DS1341 specific power-saving options Andrey Smirnov 2016-06-15 5:59 ` Andrey Smirnov 2016-06-15 5:59 ` [rtc-linux] " Andrey Smirnov 2016-06-19 14:29 ` Rob Herring 2016-06-19 14:29 ` Rob Herring 2016-06-19 14:29 ` [rtc-linux] " Rob Herring 2016-06-19 18:12 ` Andrey Smirnov 2016-06-19 18:12 ` Andrey Smirnov 2016-06-19 18:12 ` [rtc-linux] " Andrey Smirnov 2016-06-21 20:49 ` Rob Herring 2016-06-21 20:49 ` Rob Herring 2016-06-21 20:49 ` [rtc-linux] " Rob Herring 2016-06-21 21:07 ` Alexandre Belloni 2016-06-21 21:07 ` Alexandre Belloni 2016-06-21 21:07 ` [rtc-linux] " Alexandre Belloni 2016-06-22 2:34 ` Andrey Smirnov 2016-06-22 2:34 ` Andrey Smirnov 2016-06-22 2:34 ` [rtc-linux] " Andrey Smirnov 2016-07-12 16:21 ` Andrey Smirnov 2016-07-12 16:21 ` Andrey Smirnov 2016-07-12 16:21 ` [rtc-linux] " Andrey Smirnov 2016-07-19 22:47 ` Alexandre Belloni 2016-07-19 22:47 ` Alexandre Belloni 2016-07-19 22:47 ` [rtc-linux] " Alexandre Belloni 2016-07-19 23:56 ` Andrey Smirnov 2016-07-19 23:56 ` Andrey Smirnov 2016-07-19 23:56 ` [rtc-linux] " Andrey Smirnov 2016-07-20 9:02 ` Alexandre Belloni [this message] 2016-07-20 9:02 ` Alexandre Belloni 2016-07-20 14:36 ` Andrey Smirnov 2016-07-20 14:36 ` Andrey Smirnov 2016-07-20 14:36 ` [rtc-linux] " Andrey Smirnov 2016-07-20 15:38 ` Alexandre Belloni 2016-07-20 15:38 ` Alexandre Belloni 2016-07-20 15:38 ` [rtc-linux] " Alexandre Belloni 2016-07-20 16:11 ` Andrey Smirnov 2016-07-20 16:11 ` Andrey Smirnov 2016-07-20 16:11 ` [rtc-linux] " Andrey Smirnov 2016-06-21 23:23 ` Andrey Smirnov 2016-06-21 23:23 ` [rtc-linux] " Andrey Smirnov 2016-06-15 5:59 ` [PATCH 04/13] RTC: ds1307: Convert ds1307_can_wakeup_device into a predicate Andrey Smirnov 2016-06-15 5:59 ` [rtc-linux] " Andrey Smirnov 2016-06-15 5:59 ` [PATCH 05/13] RTC: ds1307: Convert want_irq " Andrey Smirnov 2016-06-15 5:59 ` [rtc-linux] " Andrey Smirnov 2016-06-15 5:59 ` [PATCH 06/13] RTC: ds1307: Move chip configuration into a separate routine Andrey Smirnov 2016-06-15 5:59 ` [rtc-linux] " Andrey Smirnov 2016-06-15 5:59 ` [PATCH 07/13] RTC: ds1307: Move chip sanity checking " Andrey Smirnov 2016-06-15 5:59 ` [rtc-linux] " Andrey Smirnov 2016-06-15 5:59 ` [PATCH 08/13] RTC: ds1307: Remove register "cache" Andrey Smirnov 2016-06-15 5:59 ` [rtc-linux] " Andrey Smirnov 2016-06-15 5:59 ` [PATCH 09/13] RTC: ds1307: Constify struct ds1307 where possible Andrey Smirnov 2016-06-15 5:59 ` [rtc-linux] " Andrey Smirnov 2016-06-15 5:59 ` [PATCH 10/13] RTC: ds1307: Convert goto to a loop Andrey Smirnov 2016-06-15 5:59 ` [rtc-linux] " Andrey Smirnov 2016-06-15 5:59 ` [PATCH 11/13] RTC: ds1307: Redefine RX8025_REG_* to minimize extra code Andrey Smirnov 2016-06-15 5:59 ` [rtc-linux] " Andrey Smirnov 2016-06-15 5:59 ` [PATCH 12/13] RTC: ds1307: Report oscillator problems more intelligently Andrey Smirnov 2016-06-15 5:59 ` [rtc-linux] " Andrey Smirnov 2016-06-15 5:59 ` [PATCH 13/13] RTC: ds1307: Move last bits of sanity checking out of chip_configure Andrey Smirnov 2016-06-15 5:59 ` [rtc-linux] " Andrey Smirnov
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20160720090220.GQ7132@piout.net \ --to=alexandre.belloni@free-electrons.com \ --cc=a.zummo@towertech.it \ --cc=andrew.smirnov@gmail.com \ --cc=devicetree@vger.kernel.org \ --cc=galak@codeaurora.org \ --cc=ijc+devicetree@hellion.org.uk \ --cc=linux-kernel@vger.kernel.org \ --cc=mark.rutland@arm.com \ --cc=pawel.moll@arm.com \ --cc=robh@kernel.org \ --cc=rtc-linux@googlegroups.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.