On 02/07/2024 17:00, Conor Dooley wrote: > Rob/Krzysztof, Haylen, >> + >> +maintainers: >> + - Haylen Chu <heylenay@xxxxxxxxxxx> >> + >> +description: Binding for Sophgo CV1800 on-SoC thermal sensor Drop "Binding" >> + >> +properties: >> + compatible: >> + enum: >> + - sophgo,cv1800-thermal >> + >> + reg: >> + maxItems: 1 >> + >> + clocks: >> + description: The thermal sensor clock >> + >> + interrupts: >> + maxItems: 1 >> + >> + accumulation-period: >> + $ref: /schemas/types.yaml#/definitions/uint32 >> + description: Accumulation period for a sample >> + enum: >> + - 512 >> + - 1024 >> + - 2048 >> + - 4096 >> + default: 2048 >> + >> + chop-period: period in what sort of units? Sounds like time to me, so this would require proper unit suffix. >> + $ref: /schemas/types.yaml#/definitions/uint32 >> + description: ADC chop period >> + enum: >> + - 128 >> + - 256 >> + - 512 >> + - 1024 >> + default: 1024 >> + >> + sample-cycle-us: > > the more common term btw would be "sample-rate" rather than > "sample-cycle". yeah, sample-rate-hz > >> + description: Period between samples. Should be greater than 524us. > > The constraint here should be "minimum: 524". What's the upper limit? > >> + default: 1000000 > > Rob/Krzysztof, could you comment on the suitability of the three custom > properties here? I know if this was an IIO device, these kinds of things > would be controllable from userspace, and not in the binding. I > mentioned this on the previous version, but I'm not really sure if > thermal devices are somehow different: > https://lore.kernel.org/all/SEYPR01MB4221A739D0645EF0255336EBD7CE2@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx/ > Why would different boards have different values here? Does it affect accuracy? If so, how much? I doubt there are any boards with different values, thus it sounds like unnecessary tuning parameter. Best regards, Krzysztof
