Message-ID: <20250905230209.GA1423697-robh@kernel.org>
Date: Fri, 5 Sep 2025 18:02:09 -0500
From: Rob Herring <robh@...nel.org>
To: Aleksandrs Vinarskis <alex@...arskis.com>
Cc: Hans de Goede <hansg@...nel.org>, Lee Jones <lee@...nel.org>,
	Pavel Machek <pavel@...nel.org>,
	Krzysztof Kozlowski <krzk+dt@...nel.org>,
	Conor Dooley <conor+dt@...nel.org>,
	Bryan O'Donoghue <bryan.odonoghue@...aro.org>,
	Daniel Thompson <danielt@...nel.org>,
	Jingoo Han <jingoohan1@...il.com>,
	Mauro Carvalho Chehab <mchehab@...nel.org>,
	Jean-Jacques Hiblot <jjhiblot@...phandler.com>,
	Jacopo Mondi <jacopo@...ndi.org>,
	Sakari Ailus <sakari.ailus@...ux.intel.com>,
	Bjorn Andersson <andersson@...nel.org>,
	Konrad Dybcio <konradybcio@...nel.org>, linux-leds@...r.kernel.org,
	devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
	Daniel Thompson <daniel.thompson@...aro.org>,
	dri-devel@...ts.freedesktop.org, linux-media@...r.kernel.org,
	linux-arm-msm@...r.kernel.org
Subject: Re: [PATCH v2 2/4] dt-bindings: leds: commonize leds property

On Fri, Sep 05, 2025 at 04:48:52PM +0000, Aleksandrs Vinarskis wrote:
> On Friday, September 5th, 2025 at 17:24, Rob Herring <robh@...nel.org> wrote:
> 
> > 
> > 
> > On Fri, Sep 05, 2025 at 09:59:30AM +0200, Aleksandrs Vinarskis wrote:
> > 
> > > A number of existing schemas use 'leds' property to provide
> > > phandle-array of LED(s) to the consumer. Additionally, with the
> > > upcoming privacy-led support in device-tree, v4l2 subnode could be a
> > > LED consumer, meaning that all camera sensors should support 'leds'
> > > and 'led-names' property via common 'video-interface-devices.yaml'.
> > > 
> > > To avoid dublication, commonize 'leds' property from existing schemas
> > > to newly introduced 'led-consumer.yaml'.
> > > 
> > > Signed-off-by: Aleksandrs Vinarskis alex@...arskis.com
> > > ---
> > > .../devicetree/bindings/leds/backlight/led-backlight.yaml | 7 +------
> > > Documentation/devicetree/bindings/leds/leds-group-multicolor.yaml | 6 +-----
> > > .../devicetree/bindings/media/video-interface-devices.yaml | 3 +++
> > > 3 files changed, 5 insertions(+), 11 deletions(-)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/leds/backlight/led-backlight.yaml b/Documentation/devicetree/bindings/leds/backlight/led-backlight.yaml
> > > index f5554da6bc6c73e94c4a2c32b150b28351b25f16..5e19b4376715eeb05cb789255db209ed27f8822f 100644
> > > --- a/Documentation/devicetree/bindings/leds/backlight/led-backlight.yaml
> > > +++ b/Documentation/devicetree/bindings/leds/backlight/led-backlight.yaml
> > > @@ -18,17 +18,12 @@ description:
> > > 
> > > allOf:
> > > - $ref: common.yaml#
> > > + - $ref: /schemas/leds/leds-consumer.yaml#
> > 
> > 
> > Drop.
> > 
> > > properties:
> > > compatible:
> > > const: led-backlight
> > > 
> > > - leds:
> > > - description: A list of LED nodes
> > > - $ref: /schemas/types.yaml#/definitions/phandle-array
> > > - items:
> > > - maxItems: 1
> > 
> > 
> > You need to keep the property here:
> > 
> > leds: true
> > 
> > > -
> > > required:
> > > - compatible
> > > - leds
> > > diff --git a/Documentation/devicetree/bindings/leds/leds-group-multicolor.yaml b/Documentation/devicetree/bindings/leds/leds-group-multicolor.yaml
> > > index 8ed059a5a724f68389a1d0c4396c85b9ccb2d9af..b4f326e8822a3bf452b22f5b9fa7189696f760a4 100644
> > > --- a/Documentation/devicetree/bindings/leds/leds-group-multicolor.yaml
> > > +++ b/Documentation/devicetree/bindings/leds/leds-group-multicolor.yaml
> > > @@ -17,16 +17,12 @@ properties:
> > > compatible:
> > > const: leds-group-multicolor
> > > 
> > > - leds:
> > > - description:
> > > - An aray of monochromatic leds
> > > - $ref: /schemas/types.yaml#/definitions/phandle-array
> > > -
> > > required:
> > > - leds
> > > 
> > > allOf:
> > > - $ref: leds-class-multicolor.yaml#
> > > + - $ref: /schemas/leds/leds-consumer.yaml#
> > 
> > 
> > 
> > Same comments in this one.
> > 
> > > unevaluatedProperties: false
> > > 
> > > diff --git a/Documentation/devicetree/bindings/media/video-interface-devices.yaml b/Documentation/devicetree/bindings/media/video-interface-devices.yaml
> > > index cf7712ad297c01c946fa4dfdaf9a21646e125099..1e25cea0ff71da2cfd1c7c4642713199f3542c0a 100644
> > > --- a/Documentation/devicetree/bindings/media/video-interface-devices.yaml
> > > +++ b/Documentation/devicetree/bindings/media/video-interface-devices.yaml
> > > @@ -10,6 +10,9 @@ maintainers:
> > > - Jacopo Mondi jacopo@...ndi.org
> > > - Sakari Ailus sakari.ailus@...ux.intel.com
> > > 
> > > +allOf:
> > > + - $ref: /schemas/leds/leds-consumer.yaml#
> > 
> > 
> > This can be dropped. The user still has to define how many entries and
> > what the values of led-names are.
> 
> Hmm, but where should it be added then? If I just drop it, MIPI 
> camera schemas which are based on 'video-interface-devices.yaml' and 
> have 'unevaluatedProperties: false' throw warnings because 'leds' was 
> not expected. Including the example in 'led-consumer.yaml' as found 
> by your bot (because of patch order your bot only run on 1/4, adding 
> this very change fixes it).

> In this case, v4l2 subnode is the LED user, which is some camera. It 
> seems most/all of these cameras are based on this binding, so instead 
> of adding new led related properties to all of them, I thought this 
> is a good common place for it... Shall I add #entries and available 
> options for 'led-names' here to make it complete?

Every camera doesn't have the same LEDs, so you have to define exactly 
what's there for each one. If you want to do it in 
video-interface-devices.yanl, then you are standardizing it for 
everyone. Maybe that's fine? If so, you need something like:

leds:
  minItems: 1
  maxItems: 2

led-names:
  items:
    enum:
      - flash
      - privacy

Rob

Powered by blists - more mailing lists