On Tue, Oct 03, 2023 at 09:43:15AM +0000, Flavio Suligoi wrote: > Hi Daniel, > > ... > > > > +required: > > > + - compatible > > > + - reg > > > + - max-brightness > > > > Why is this mandatory? > > > > There's no point in setting max-brightness when running in I2C mode (max- > > brightness should default to 31 in that case). > > > > > > > + - default-brightness > > > > Again. I'm not clear why this needs to be mandatory. > > > > > > Ok, you are right, I'll remove max-brightness and default-brightness > from required properties list. I think to change these properties, > for the pwm dimming, into a clearer: > > - brightness-levels (uint32) > - default-brightness-levels (uint32). > > For example: > > brightness-levels: > description: > Number of brightness levels. The actual brightness > level (PWM duty cycle) will be interpolated from 0 to this value. > 0 means a 0% duty cycle (darkest/off), while the brightness-levels represents > a 100% duty cycle (brightest). > $ref: /schemas/types.yaml#/definitions/uint32 > > default-brightness-level: > description: > The default brightness level (from 0 to brightness-levels) > $ref: /schemas/types.yaml#/definitions/uint32 > > Example: > brightness-levels = <10>; > default-brightness-level = <6>; > > What do you think about this solution? If you want to introduce a brightness-levels property then I would expect it to be defined with the same meaning as pwm-backlight (it's not relevant to the bindings but ideally it would be implemented by refactoring and reusing the code from pwm_bl.c). Same with default-brightness-level although I'm not sure why one wouldn't just use default-brightness for new bindings (doesn't default-brightness-level simply do exactly the same thing as default-brightness). Daniel.
