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