On 03/07/2023 18:14, Conor Dooley wrote: >>>>> @@ -88,6 +88,17 @@ patternProperties: >>>>> 4: 1600 (default) >>>>> 5: 2400 >>>>> 6: 3300 >>>>> + 7: 3300 >>>>> + >>>>> + Data acquisition rate in samples per second for ADS1115 >>>>> + 0: 8 >>>>> + 1: 16 >>>>> + 2: 32 >>>>> + 3: 64 >>>>> + 4: 128 (default) >>>>> + 5: 250 >>>>> + 6: 475 >>>>> + 7: 860 >>>> >>>> I'll leave this one to Rob or Krzysztof to ack/review, but this does >>>> seem like as good an opportunity as any to migrate to a property that >>>> allows you to put the actual data acquisition rate in & not have to add >>>> new key-value mappings to the binding to support devices with differing >>>> schemes. >>> >>> I agree a value would have been better, but now we are where we are, >>> I'm not sure it's worth the churn of changing it - particularly as the >>> driver will need to support the old binding for every anyway. >> >> Yep, this would be an API change :/ > > Of course, but so what you have in these patches anyway. Change being > the operative word, not break ;) > > Either way, I passed the buck to Rob and Krzysztof on this one anyway. It's fine. Device-specific, so not common, properties can be expressing programming model (register value). https://lore.kernel.org/linux-devicetree/20230412133921.GA2017891-robh@xxxxxxxxxx/ Such properties are usually less readable in DTS, so they have clear disadvantage. However using here common units would require mapping in the driver which is additional churn. >From every rule there are also exceptions. For example consider common regulator values or number of samples where hwmon core is ready to parse it properly: https://lore.kernel.org/linux-devicetree/82d76824-ef5b-23f9-149e-2c5d9f88e94a@xxxxxxxxxx/T/#mb2808172369a9960890a2de538464ca68dc86455 That's not the case here, so it's fine. Best regards, Krzysztof
