-- Antoniu Miclăuş > -----Original Message----- > From: Chris Packham <Chris.Packham@xxxxxxxxxxxxxxxxxxx> > Sent: Monday, February 19, 2024 4:52 AM > To: Miclaus, Antoniu <Antoniu.Miclaus@xxxxxxxxxx>; Alessandro Zummo > <a.zummo@xxxxxxxxxxxx>; Alexandre Belloni > <alexandre.belloni@xxxxxxxxxxx>; Rob Herring <robh+dt@xxxxxxxxxx>; > Krzysztof Kozlowski <krzysztof.kozlowski+dt@xxxxxxxxxx>; Conor Dooley > <conor+dt@xxxxxxxxxx>; Jean Delvare <jdelvare@xxxxxxxx>; Guenter Roeck > <linux@xxxxxxxxxxxx>; linux-rtc@xxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx; > linux-kernel@xxxxxxxxxxxxxxx; linux-hwmon@xxxxxxxxxxxxxxx > Subject: Re: [PATCH v9 2/2] rtc: max31335: add driver support > > [External] > > Hi All, > > I'm looking at folding this into the rest of the max313xx support (but > I'll stick with the max31335 name since that's landed) and I noticed a > problem. > > On 21/11/23 01:00, Antoniu Miclaus wrote: > > RTC driver for MAX31335 ±2ppm Automotive Real-Time Clock with > > Integrated MEMS Resonator. > > > > Reviewed-by: Guenter Roeck <linux@xxxxxxxxxxxx> > > Signed-off-by: Antoniu Miclaus <antoniu.miclaus@xxxxxxxxxx> > <snip> > > + > > +static bool max31335_volatile_reg(struct device *dev, unsigned int reg) > > +{ > > + /* time keeping registers */ > > + if (reg >= MAX31335_SECONDS && > > + reg < MAX31335_SECONDS + MAX31335_TIME_SIZE) > > + return true; > > + > > + /* interrupt status register */ > > + if (reg == MAX31335_INT_EN1_A1IE) > > + return true; > Presumably this should be something else as MAX31335_INT_EN1_A1IE is a > bitfield offset not a register. Based on the other chips I'm guessing > this should be `reg == MAX31335_STATUS1`. I'll try to incorporate a fix > into my update but someone might want to fix it up for stable. Indeed, MAX31335_STATUS1 should be used here. I re-checked the latest datasheet that I have. Thanks for pointing that out. I will add a patch soon. > > + > > + /* temperature registers */ > > + if (reg == MAX31335_TEMP_DATA_MSB || > MAX31335_TEMP_DATA_LSB) > > + return true; > > + > > + return false; > > +} > > +