On 28/02/24 13:03, Andy Shevchenko wrote: > On Tue, Feb 27, 2024 at 11:22 PM Chris Packham > <chris.packham@xxxxxxxxxxxxxxxxxxx> wrote: > > ... > >> + segment-gpios: >> + description: >> + An array of GPIOs one per segment. > This is a vague description. Please explain the order (e.g., LSB = > 'a', MSB = 'g'), use of DP (optional?), etc. > >> + minItems: 7 > maxItems? > > ... I plan on saying maxItems: 7 (more discussion below) > >> + led-7seg { > Probably it should be more human readable. DT people might suggest > something better. > >> + compatible = "generic-gpio-7seg"; >> + segment-gpios = <&gpio 0 GPIO_ACTIVE_LOW >> + &gpio 1 GPIO_ACTIVE_LOW >> + &gpio 2 GPIO_ACTIVE_LOW >> + &gpio 3 GPIO_ACTIVE_LOW >> + &gpio 4 GPIO_ACTIVE_LOW >> + &gpio 5 GPIO_ACTIVE_LOW >> + &gpio 6 GPIO_ACTIVE_LOW>; > Dunno how to handle DP. Either we always expect it to be here (as > placeholder) or with additional property. My current plan was to ignore it. As you see it my later patch I'm (ab)using DP as a discrete gpio-led with a different function. We could either a separate dp-gpios property or set maxItems to 8. Right now the driver won't do anything with either option.To actually do something in the linedisp driver we'd need to have a new character map that includes the extra LED. >> + };