| Message ID | 20230613130011.305589-1-linux@rasmusvillemoes.dk |
|---|---|
| Headers | show |
| Series | rtc: isl12022: battery backup voltage and clock support | expand |
On Wed, Jun 14, 2023 at 03:50:36PM +0200, Alexandre Belloni wrote: > On 14/06/2023 15:16:14+0300, Andy Shevchenko wrote: > > On Tue, Jun 13, 2023 at 11:26:51PM +0200, Alexandre Belloni wrote: > > > On 13/06/2023 18:20:56+0300, Andy Shevchenko wrote: > > > > On Tue, Jun 13, 2023 at 03:00:07PM +0200, Rasmus Villemoes wrote: ... > > > > > + ret = regmap_read(regmap, ISL12022_REG_SR, &val); > > > > > + if (ret < 0) > > > > > > > > I always feel uneasy with ' < 0' — Does positive error makes sense? > > > > Is it even possible? OTOH if the entire driver uses this idiom... > > > > oh well, let's make it consistent. > > > > > > /** > > > * regmap_read() - Read a value from a single register > > > * > > > * @map: Register map to read from > > > * @reg: Register to be read from > > > * @val: Pointer to store read value > > > * > > > * A value of zero will be returned on success, a negative errno will > > > * be returned in error cases. > > > */ > > > > I'm not sure what you meant by this. Yes, I know that there is no > > possibility that regmap API returns positive value. Do you mean that > > regmap API documentation is unclear? > > No, I mean that you'd have to be clearer as to why you are uneasy with a > test for a negative value when the function returns 0 for success and a > negative value for an error. Else, this is pure bullying.
The current handling of the low-battery bits in the status register is wrong. The first six patches fix that and implement proper support for RTC_VL_READ. The last two patches allow describing the isl12022 as a clock provider, for now just as a fixed 32kHz clock. They are also tangentially related to the backup battery, in that when the isl12022 is not used as a clock source, one can save some power consumption in battery mode by setting the FOx bits to 0. v2 changes: Patch 2: add Alexandre as maintainer [Rob's bot]. Patch 4: On arm64, <linux/bitfield.h> apparently ends up being included implicitly, but not so on arm [kernel test robot]. Use the more common post-increment in for loops [Andy]. Patch 5: Drop RTC_VL_CLR, just do RTC_VL_READ [Alexandre]. Patch 6: Set the TSE bit to trigger a manual detection, but drop the part reading the SR register and issuing a dev_warn() in case of low battery [Alexandre]. Patch 7: (Hopefully) properly describe the "at most one of interrupts and #clock-cells" [thanks Krzysztof]. Patch 8: Drop a useless dev_warn() in case clearing the FOx bits fails. Rasmus Villemoes (8): rtc: isl12022: remove wrong warning for low battery level dt-bindings: rtc: Move isil,isl12022 from trivial-rtc.yaml into own schema file dt-bindings: rtc: isl12022: add bindings for battery alarm trip levels rtc: isl12022: add support for trip level DT bindings rtc: isl12022: implement RTC_VL_READ ioctl rtc: isl12022: trigger battery level detection during probe dt-bindings: rtc: isl12022: add #clock-cells property rtc: isl12022: implement support for the #clock-cells DT property .../bindings/rtc/intersil,isl12022.yaml | 69 ++++++++++ .../devicetree/bindings/rtc/trivial-rtc.yaml | 2 - drivers/rtc/rtc-isl12022.c | 118 +++++++++++++++++- 3 files changed, 181 insertions(+), 8 deletions(-) create mode 100644 Documentation/devicetree/bindings/rtc/intersil,isl12022.yaml