Hi, On 09/06/2020 18:23:48-0400, Kevin P. Fleming wrote: > On Tue, Jun 9, 2020 at 6:14 PM Rob Herring <robh@xxxxxxxxxx> wrote: > > > --- > > > .../bindings/rtc/abracon,abx80x.txt | 6 ++++ > > > drivers/rtc/rtc-abx80x.c | 34 +++++++++++++++++++ > > > 2 files changed, 40 insertions(+) > > > > Binding should be a separate patch? > > Indeed, it was re-sent with the patches separated. > > > > +All of the devices can have a 47pf capacitor attached to increase the > > > +autocalibration accuracy of their RC oscillators. To enable usage of the > > > +capacitor the following property has to be defined: > > > + > > > + - "abracon,autocal-filter" > > > > Can't the standard 'quartz-load-femtofarads' property be used here? You > > might not need to know the value, but presence of the property can > > enable the feature. > > On these devices the capacitor is connected to the RC oscillator, not > the crystal oscillator, so this property is controlling a different > function. I'm certainly open to suggestions for different names for > the property if that is desired. I agree that this is something different, quartz-load-femtofarads is about asking the RTC to put the correct load depending on the crystal populated on the board. Here, you want to indicate the presence or absence of a filter capacitor. When working with RTCs, there is one issue though: boolean properties are not working well because there is no way to express the 3 different conditions: 1/ the capacitor is present, set the register 2/ the capacitor is absent, clear the register 3/ the device tree didn't have this property until not and the register may have been set or cleared using another mean, don't touch it. As your patch is written, it only handles 1 and 3 which is probably the safest option but then we will never have a way to clear it from the driver. I'd say that this is not an issue but it is also something we will never be able to change without breaking some setups. -- Alexandre Belloni, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
