linux-kernel - Re: [PATCH v2 8/8] rtc: isl12022: implement support for the #clock-cells DT property

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <c826b8a9-ed12-a6cc-d089-47adbc200f37@rasmusvillemoes.dk>
Date:   Wed, 14 Jun 2023 12:51:47 +0200
From:   Rasmus Villemoes <linux@...musvillemoes.dk>
To:     Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
Cc:     Alessandro Zummo <a.zummo@...ertech.it>,
        Alexandre Belloni <alexandre.belloni@...tlin.com>,
        devicetree@...r.kernel.org, Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Conor Dooley <conor+dt@...nel.org>, linux-rtc@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 8/8] rtc: isl12022: implement support for the
 #clock-cells DT property

On 13/06/2023 17.25, Andy Shevchenko wrote:
> On Tue, Jun 13, 2023 at 03:00:10PM +0200, Rasmus Villemoes wrote:
>> If device tree implies that the chip's IRQ/F_OUT pin is used as a
>> clock, expose that in the driver. For now, pretend it is a
>> fixed-rate (32kHz) clock; if other use cases appear the driver can be
>> updated to provide its own clk_ops etc.
>>
>> When the clock output is not used on a given board, one can prolong
>> the battery life by ensuring that the FOx bits are 0. For the hardware
>> I'm currently working on, the RTC draws 1.2uA with the FOx bits at
>> their default 0001 value, dropping to 0.88uA when those bits are
>> cleared.
> 
> ...
> 
>> +#define ISL12022_INT_FO_MASK	GENMASK(3, 0)
>> +#define ISL12022_INT_FO_OFF	0x0
>> +#define ISL12022_INT_FO_32K	0x1
> 
> A nit-pick. Are they decimal or bit fields? 

-ENOPARSE. A number is a number. Its representation in C code may be
decimal or hexadecimal (or...). And sure, 0 and 0x0 are different
spellings of the same thing. The data sheet lists the possible values in
terms of individual bits, so I suppose I could even do 0b0000 and
0b0001, but that's too unusual (even if perfectly acceptable by gcc).

> To me seems like the 0x can be dropped.

Can, but won't, a single hex digit is more natural way to represent a
four-bit field.

>> +	ret = regmap_update_bits(regmap, ISL12022_REG_INT, ISL12022_INT_FO_MASK, ISL12022_INT_FO_32K);
> 
> Seems too long even for 100 limit.
> Maybe:
> 
> 	ret = regmap_update_bits(regmap, ISL12022_REG_INT,
> 				 ISL12022_INT_FO_MASK, ISL12022_INT_FO_32K);

Sure.

Rasmus