[PATCH v5 0/2] Multicolor PWM LED support

[PATCH v5 0/2] Multicolor PWM LED support

[sven]

From: Sven Schwermer <sven.schwermer@disruptive-technologies.com>

Hi,

This patch series is getting mature. I have removed the RFC tag for this
version. The initial discussion happened here [1].

I would appreciate if anyone would test this code. It runs on my
i.MX6ULL-based hardware.

Best regards,
Sven

[1]:https://lore.kernel.org/linux-leds/37540afd-f2f1-52dd-f4f1-6e7b436e9595@svenschwermer.de/

Sven Schwermer (2):
  dt-bindings: leds: Add multicolor PWM LED bindings
  leds: Add PWM multicolor driver

 .../bindings/leds/leds-pwm-multicolor.yaml    |  75 +++++++
 drivers/leds/Kconfig                          |  11 ++
 drivers/leds/Makefile                         |   1 +
 drivers/leds/leds-pwm-multicolor.c            | 186 ++++++++++++++++++
 4 files changed, 273 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/leds/leds-pwm-multicolor.yaml
 create mode 100644 drivers/leds/leds-pwm-multicolor.c

Interdiff against v4:
diff --git a/drivers/leds/leds-pwm-multicolor.c b/drivers/leds/leds-pwm-multicolor.c
index 96712b8ca98e..45e38708ecb1 100644
--- a/drivers/leds/leds-pwm-multicolor.c
+++ b/drivers/leds/leds-pwm-multicolor.c
@@ -58,6 +58,43 @@ static int led_pwm_mc_set(struct led_classdev *cdev,
 	return ret;
 }
 
+static int iterate_subleds(struct device *dev, struct pwm_mc_led *priv,
+			   struct fwnode_handle *mcnode)
+{
+	struct mc_subled *subled = priv->mc_cdev.subled_info;
+	struct fwnode_handle *fwnode;
+	struct pwm_led *pwmled;
+	u32 color;
+	int ret;
+
+	/* iterate over the nodes inside the multi-led node */
+	fwnode_for_each_child_node(mcnode, fwnode) {
+		pwmled = &priv->leds[priv->mc_cdev.num_colors];
+		pwmled->pwm = devm_fwnode_pwm_get(dev, fwnode, NULL);
+		if (IS_ERR(pwmled->pwm)) {
+			ret = PTR_ERR(pwmled->pwm);
+			dev_err(dev, "unable to request PWM: %d\n", ret);
+			goto release_fwnode;
+		}
+		pwm_init_state(pwmled->pwm, &pwmled->state);
+
+		ret = fwnode_property_read_u32(fwnode, "color", &color);
+		if (ret) {
+			dev_err(dev, "cannot read color: %d\n", ret);
+			goto release_fwnode;
+		}
+
+		subled[priv->mc_cdev.num_colors].color_index = color;
+		priv->mc_cdev.num_colors++;
+	}
+
+	return 0;
+
+release_fwnode:
+	fwnode_handle_put(fwnode);
+	return ret;
+}
+
 static int led_pwm_mc_probe(struct platform_device *pdev)
 {
 	struct fwnode_handle *mcnode, *fwnode;
@@ -65,10 +102,8 @@ static int led_pwm_mc_probe(struct platform_device *pdev)
 	struct led_classdev *cdev;
 	struct mc_subled *subled;
 	struct pwm_mc_led *priv;
-	struct pwm_led *pwmled;
 	int count = 0;
 	int ret = 0;
-	u32 color;
 
 	mcnode = device_get_named_child_node(&pdev->dev, "multi-led");
 	if (!mcnode)
@@ -101,28 +136,9 @@ static int led_pwm_mc_probe(struct platform_device *pdev)
 	cdev->flags = LED_CORE_SUSPENDRESUME;
 	cdev->brightness_set_blocking = led_pwm_mc_set;
 
-	/* iterate over the nodes inside the multi-led node */
-	fwnode_for_each_child_node(mcnode, fwnode) {
-		pwmled = &priv->leds[priv->mc_cdev.num_colors];
-		pwmled->pwm = devm_fwnode_pwm_get(&pdev->dev, fwnode, NULL);
-		if (IS_ERR(pwmled->pwm)) {
-			ret = PTR_ERR(pwmled->pwm);
-			dev_err(&pdev->dev, "unable to request PWM: %d\n", ret);
-			fwnode_handle_put(fwnode);
-			goto release_mcnode;
-		}
-		pwm_init_state(pwmled->pwm, &pwmled->state);
-
-		ret = fwnode_property_read_u32(fwnode, "color", &color);
-		if (ret) {
-			dev_err(&pdev->dev, "cannot read color: %d\n", ret);
-			fwnode_handle_put(fwnode);
-			goto release_mcnode;
-		}
-
-		subled[priv->mc_cdev.num_colors].color_index = color;
-		priv->mc_cdev.num_colors++;
-	}
+	ret = iterate_subleds(&pdev->dev, priv, mcnode);
+	if (ret)
+		goto release_mcnode;
 
 	init_data.fwnode = mcnode;
 	ret = devm_led_classdev_multicolor_register_ext(&pdev->dev,
-- 
2.35.1

[PATCH v5 1/2] dt-bindings: leds: Add multicolor PWM LED bindings

[sven]

From: Sven Schwermer <sven.schwermer@disruptive-technologies.com>

This allows to group multiple PWM-connected monochrome LEDs into
multicolor LEDs, e.g. RGB LEDs.

Signed-off-by: Sven Schwermer <sven.schwermer@disruptive-technologies.com>
---

Notes:
    Changes in v5:
    * (no changes)
    
    Changes in v4:
    * (no changes)
    
    Changes in v3:
    * Remove multi-led unit name

 .../bindings/leds/leds-pwm-multicolor.yaml    | 75 +++++++++++++++++++
 1 file changed, 75 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/leds/leds-pwm-multicolor.yaml

diff --git a/Documentation/devicetree/bindings/leds/leds-pwm-multicolor.yaml b/Documentation/devicetree/bindings/leds/leds-pwm-multicolor.yaml
new file mode 100644
index 000000000000..5a7ed5e1bb9f
--- /dev/null
+++ b/Documentation/devicetree/bindings/leds/leds-pwm-multicolor.yaml
@@ -0,0 +1,75 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/leds/leds-pwm-multicolor.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Multi-color LEDs connected to PWM
+
+maintainers:
+  - Sven Schwermer <sven.schwermer@disruptive-technologies.com>
+
+description: |
+  This driver combines several monochrome PWM LEDs into one multi-color
+  LED using the multicolor LED class.
+
+properties:
+  compatible:
+    const: pwm-leds-multicolor
+
+  multi-led:
+    type: object
+    allOf:
+      - $ref: leds-class-multicolor.yaml#
+
+    patternProperties:
+      "^led-[0-9a-z]+$":
+        type: object
+        properties:
+          pwms:
+            maxItems: 1
+
+          pwm-names: true
+
+          color:
+            $ref: common.yaml#/properties/color
+
+        required:
+          - pwms
+          - color
+
+required:
+  - compatible
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/leds/common.h>
+
+    rgb-led {
+        compatible = "pwm-leds-multicolor";
+
+        multi-led {
+          color = <LED_COLOR_ID_RGB>;
+          function = LED_FUNCTION_INDICATOR;
+          max-brightness = <65535>;
+
+          led-red {
+              pwms = <&pwm1 0 1000000>;
+              color = <LED_COLOR_ID_RED>;
+          };
+
+          led-green {
+              pwms = <&pwm2 0 1000000>;
+              color = <LED_COLOR_ID_GREEN>;
+          };
+
+          led-blue {
+              pwms = <&pwm3 0 1000000>;
+              color = <LED_COLOR_ID_BLUE>;
+          };
+        };
+    };
+
+...
-- 
2.35.1

[PATCH v5 2/2] leds: Add PWM multicolor driver

[sven]

From: Sven Schwermer <sven.schwermer@disruptive-technologies.com>

By allowing to group multiple monochrome PWM LEDs into multicolor LEDs,
all involved LEDs can be controlled in-sync. This enables using effects
using triggers, etc.

Signed-off-by: Sven Schwermer <sven.schwermer@disruptive-technologies.com>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
---

Notes:
    Changes in v5:
    * Factor iteration over subleds out into function
    
    Changes in v4:
    * Remove mutex destruction and remove function
    * Include missing headers
    * Use post-increment instead of pre-increment
    * Variable declarations in reverse xmas tree order
    * Use dev_err_probe where possible
    * Return immediately where possible
    * Cosmetic changes
    * Document LKM name
    
    Changes in v3:
    * Release fwnode handles
    * Sort header includes
    * Remove deprecated device tree properties
    * Remove deprecated LED_OFF
    * Remove subled channel assignment
    * s/pwmstate/state/

 drivers/leds/Kconfig               |  11 ++
 drivers/leds/Makefile              |   1 +
 drivers/leds/leds-pwm-multicolor.c | 186 +++++++++++++++++++++++++++++
 3 files changed, 198 insertions(+)
 create mode 100644 drivers/leds/leds-pwm-multicolor.c

diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index 6090e647daee..e70a46704076 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -552,6 +552,17 @@ config LEDS_PWM
 	help
 	  This option enables support for pwm driven LEDs
 
+config LEDS_PWM_MULTICOLOR
+	tristate "PWM driven multi-color LED Support"
+	depends on LEDS_CLASS_MULTICOLOR
+	depends on PWM
+	help
+	  This option enables support for PWM driven monochrome LEDs that are
+	  grouped into multicolor LEDs.
+
+	  To compile this driver as a module, choose M here: the module
+	  will be called leds-pwm-multicolor.
+
 config LEDS_REGULATOR
 	tristate "REGULATOR driven LED support"
 	depends on LEDS_CLASS
diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
index e58ecb36360f..ba2c2c1edf12 100644
--- a/drivers/leds/Makefile
+++ b/drivers/leds/Makefile
@@ -73,6 +73,7 @@ obj-$(CONFIG_LEDS_PCA963X)		+= leds-pca963x.o
 obj-$(CONFIG_LEDS_PM8058)		+= leds-pm8058.o
 obj-$(CONFIG_LEDS_POWERNV)		+= leds-powernv.o
 obj-$(CONFIG_LEDS_PWM)			+= leds-pwm.o
+obj-$(CONFIG_LEDS_PWM_MULTICOLOR)	+= leds-pwm-multicolor.o
 obj-$(CONFIG_LEDS_REGULATOR)		+= leds-regulator.o
 obj-$(CONFIG_LEDS_S3C24XX)		+= leds-s3c24xx.o
 obj-$(CONFIG_LEDS_SC27XX_BLTC)		+= leds-sc27xx-bltc.o
diff --git a/drivers/leds/leds-pwm-multicolor.c b/drivers/leds/leds-pwm-multicolor.c
new file mode 100644
index 000000000000..45e38708ecb1
--- /dev/null
+++ b/drivers/leds/leds-pwm-multicolor.c
@@ -0,0 +1,186 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * PWM-based multi-color LED control
+ *
+ * Copyright 2022 Sven Schwermer <sven.schwermer@disruptive-technologies.com>
+ */
+
+#include <linux/err.h>
+#include <linux/kernel.h>
+#include <linux/led-class-multicolor.h>
+#include <linux/leds.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/platform_device.h>
+#include <linux/property.h>
+#include <linux/pwm.h>
+
+struct pwm_led {
+	struct pwm_device *pwm;
+	struct pwm_state state;
+};
+
+struct pwm_mc_led {
+	struct led_classdev_mc mc_cdev;
+	struct mutex lock;
+	struct pwm_led leds[];
+};
+
+static int led_pwm_mc_set(struct led_classdev *cdev,
+			  enum led_brightness brightness)
+{
+	struct led_classdev_mc *mc_cdev = lcdev_to_mccdev(cdev);
+	struct pwm_mc_led *priv = container_of(mc_cdev, struct pwm_mc_led, mc_cdev);
+	unsigned long long duty;
+	int ret = 0;
+	int i;
+
+	led_mc_calc_color_components(mc_cdev, brightness);
+
+	mutex_lock(&priv->lock);
+
+	for (i = 0; i < mc_cdev->num_colors; i++) {
+		duty = priv->leds[i].state.period;
+		duty *= mc_cdev->subled_info[i].brightness;
+		do_div(duty, cdev->max_brightness);
+
+		priv->leds[i].state.duty_cycle = duty;
+		priv->leds[i].state.enabled = duty > 0;
+		ret = pwm_apply_state(priv->leds[i].pwm,
+				      &priv->leds[i].state);
+		if (ret)
+			break;
+	}
+
+	mutex_unlock(&priv->lock);
+
+	return ret;
+}
+
+static int iterate_subleds(struct device *dev, struct pwm_mc_led *priv,
+			   struct fwnode_handle *mcnode)
+{
+	struct mc_subled *subled = priv->mc_cdev.subled_info;
+	struct fwnode_handle *fwnode;
+	struct pwm_led *pwmled;
+	u32 color;
+	int ret;
+
+	/* iterate over the nodes inside the multi-led node */
+	fwnode_for_each_child_node(mcnode, fwnode) {
+		pwmled = &priv->leds[priv->mc_cdev.num_colors];
+		pwmled->pwm = devm_fwnode_pwm_get(dev, fwnode, NULL);
+		if (IS_ERR(pwmled->pwm)) {
+			ret = PTR_ERR(pwmled->pwm);
+			dev_err(dev, "unable to request PWM: %d\n", ret);
+			goto release_fwnode;
+		}
+		pwm_init_state(pwmled->pwm, &pwmled->state);
+
+		ret = fwnode_property_read_u32(fwnode, "color", &color);
+		if (ret) {
+			dev_err(dev, "cannot read color: %d\n", ret);
+			goto release_fwnode;
+		}
+
+		subled[priv->mc_cdev.num_colors].color_index = color;
+		priv->mc_cdev.num_colors++;
+	}
+
+	return 0;
+
+release_fwnode:
+	fwnode_handle_put(fwnode);
+	return ret;
+}
+
+static int led_pwm_mc_probe(struct platform_device *pdev)
+{
+	struct fwnode_handle *mcnode, *fwnode;
+	struct led_init_data init_data = {};
+	struct led_classdev *cdev;
+	struct mc_subled *subled;
+	struct pwm_mc_led *priv;
+	int count = 0;
+	int ret = 0;
+
+	mcnode = device_get_named_child_node(&pdev->dev, "multi-led");
+	if (!mcnode)
+		return dev_err_probe(&pdev->dev, -ENODEV,
+				     "expected multi-led node\n");
+
+	/* count the nodes inside the multi-led node */
+	fwnode_for_each_child_node(mcnode, fwnode)
+		count++;
+
+	priv = devm_kzalloc(&pdev->dev, struct_size(priv, leds, count),
+			    GFP_KERNEL);
+	if (!priv) {
+		ret = -ENOMEM;
+		goto release_mcnode;
+	}
+	mutex_init(&priv->lock);
+
+	subled = devm_kcalloc(&pdev->dev, count, sizeof(*subled), GFP_KERNEL);
+	if (!subled) {
+		ret = -ENOMEM;
+		goto release_mcnode;
+	}
+	priv->mc_cdev.subled_info = subled;
+
+	/* init the multicolor's LED class device */
+	cdev = &priv->mc_cdev.led_cdev;
+	fwnode_property_read_u32(mcnode, "max-brightness",
+				 &cdev->max_brightness);
+	cdev->flags = LED_CORE_SUSPENDRESUME;
+	cdev->brightness_set_blocking = led_pwm_mc_set;
+
+	ret = iterate_subleds(&pdev->dev, priv, mcnode);
+	if (ret)
+		goto release_mcnode;
+
+	init_data.fwnode = mcnode;
+	ret = devm_led_classdev_multicolor_register_ext(&pdev->dev,
+							&priv->mc_cdev,
+							&init_data);
+	if (ret) {
+		dev_err(&pdev->dev,
+			"failed to register multicolor PWM led for %s: %d\n",
+			cdev->name, ret);
+		goto release_mcnode;
+	}
+
+	ret = led_pwm_mc_set(cdev, cdev->brightness);
+	if (ret)
+		return dev_err_probe(&pdev->dev, ret,
+				     "failed to set led PWM value for %s: %d",
+				     cdev->name, ret);
+
+	platform_set_drvdata(pdev, priv);
+	return 0;
+
+release_mcnode:
+	fwnode_handle_put(mcnode);
+	return ret;
+}
+
+static const struct of_device_id of_pwm_leds_mc_match[] = {
+	{ .compatible = "pwm-leds-multicolor", },
+	{}
+};
+MODULE_DEVICE_TABLE(of, of_pwm_leds_mc_match);
+
+static struct platform_driver led_pwm_mc_driver = {
+	.probe		= led_pwm_mc_probe,
+	.driver		= {
+		.name	= "leds_pwm_multicolor",
+		.of_match_table = of_pwm_leds_mc_match,
+	},
+};
+module_platform_driver(led_pwm_mc_driver);
+
+MODULE_AUTHOR("Sven Schwermer <sven.schwermer@disruptive-technologies.com>");
+MODULE_DESCRIPTION("multi-color PWM LED driver");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:leds-pwm-multicolor");
-- 
2.35.1

Re: [PATCH v5 1/2] dt-bindings: leds: Add multicolor PWM LED bindings

[Rob Herring]

On Mon, Feb 07, 2022 at 11:03:25AM +0100, sven@svenschwermer.de wrote:
> From: Sven Schwermer <sven.schwermer@disruptive-technologies.com>
> 
> This allows to group multiple PWM-connected monochrome LEDs into
> multicolor LEDs, e.g. RGB LEDs.
> 
> Signed-off-by: Sven Schwermer <sven.schwermer@disruptive-technologies.com>
> ---
> 
> Notes:
>     Changes in v5:
>     * (no changes)
>     
>     Changes in v4:
>     * (no changes)
>     
>     Changes in v3:
>     * Remove multi-led unit name
> 
>  .../bindings/leds/leds-pwm-multicolor.yaml    | 75 +++++++++++++++++++
>  1 file changed, 75 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/leds/leds-pwm-multicolor.yaml
> 
> diff --git a/Documentation/devicetree/bindings/leds/leds-pwm-multicolor.yaml b/Documentation/devicetree/bindings/leds/leds-pwm-multicolor.yaml
> new file mode 100644
> index 000000000000..5a7ed5e1bb9f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/leds/leds-pwm-multicolor.yaml
> @@ -0,0 +1,75 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/leds/leds-pwm-multicolor.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Multi-color LEDs connected to PWM
> +
> +maintainers:
> +  - Sven Schwermer <sven.schwermer@disruptive-technologies.com>
> +
> +description: |
> +  This driver combines several monochrome PWM LEDs into one multi-color
> +  LED using the multicolor LED class.
> +
> +properties:
> +  compatible:
> +    const: pwm-leds-multicolor
> +
> +  multi-led:
> +    type: object
> +    allOf:
> +      - $ref: leds-class-multicolor.yaml#

This schema says 'multi-led' here should have a child called 
"^multi-led@([0-9a-f])$". You are off a level.
> +
> +    patternProperties:
> +      "^led-[0-9a-z]+$":
> +        type: object

           $ref: common.yaml#
           additionalProperties: false


> +        properties:
> +          pwms:
> +            maxItems: 1
> +
> +          pwm-names: true
> +
> +          color:
> +            $ref: common.yaml#/properties/color

And then drop this ref.

> +
> +        required:
> +          - pwms
> +          - color
> +
> +required:
> +  - compatible
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/leds/common.h>
> +
> +    rgb-led {
> +        compatible = "pwm-leds-multicolor";
> +
> +        multi-led {

Can't this be collapsed into 1 level? I don't see "pwm-leds-multicolor" 
having other child nodes.

> +          color = <LED_COLOR_ID_RGB>;
> +          function = LED_FUNCTION_INDICATOR;
> +          max-brightness = <65535>;
> +
> +          led-red {
> +              pwms = <&pwm1 0 1000000>;
> +              color = <LED_COLOR_ID_RED>;
> +          };
> +
> +          led-green {
> +              pwms = <&pwm2 0 1000000>;
> +              color = <LED_COLOR_ID_GREEN>;
> +          };
> +
> +          led-blue {
> +              pwms = <&pwm3 0 1000000>;
> +              color = <LED_COLOR_ID_BLUE>;
> +          };
> +        };
> +    };
> +
> +...
> -- 
> 2.35.1
> 
> 

Re: [PATCH v5 1/2] dt-bindings: leds: Add multicolor PWM LED bindings

[Sven Schwermer]

Hi Rob,

Thanks for your comments.

On 2/7/22 21:21, Rob Herring wrote:
>> +properties:
>> +  compatible:
>> +    const: pwm-leds-multicolor
>> +
>> +  multi-led:
>> +    type: object
>> +    allOf:
>> +      - $ref: leds-class-multicolor.yaml#
> 
> This schema says 'multi-led' here should have a child called
> "^multi-led@([0-9a-f])$". You are off a level.

So it should have been?

properties:
   compatible:
     const: pwm-leds-multicolor
   allOf:
     - $ref: leds-class-multicolor.yaml#

This would imply that the multi-led node requires a unit address (reg 
property). That doesn't make sense in this case. How should we resolve this?

>> +
>> +    patternProperties:
>> +      "^led-[0-9a-z]+$":
>> +        type: object
> 
>             $ref: common.yaml#
>             additionalProperties: false

Sounds good.

>> +        properties:
>> +          pwms:
>> +            maxItems: 1
>> +
>> +          pwm-names: true
>> +
>> +          color:
>> +            $ref: common.yaml#/properties/color
> 
> And then drop this ref.

Curiosity question: why? Should I refer to an unsigned integer type instead?

>> +    rgb-led {
>> +        compatible = "pwm-leds-multicolor";
>> +
>> +        multi-led {
> 
> Can't this be collapsed into 1 level? I don't see "pwm-leds-multicolor"
> having other child nodes.

It could. The reason I added the multi-led level is because the 
leds-class-multicolor.yaml schema calls for it. Perhaps I missed the 
intention of that schema but isn't it there to create a uniform binding 
schema structure across drivers?

Best regards,
Sven

Re: [PATCH v5 1/2] dt-bindings: leds: Add multicolor PWM LED bindings

[Rob Herring]

On Mon, Feb 7, 2022 at 2:44 PM Sven Schwermer <sven@svenschwermer.de> wrote:
>
> Hi Rob,
>
> Thanks for your comments.
>
> On 2/7/22 21:21, Rob Herring wrote:
> >> +properties:
> >> +  compatible:
> >> +    const: pwm-leds-multicolor
> >> +
> >> +  multi-led:
> >> +    type: object
> >> +    allOf:
> >> +      - $ref: leds-class-multicolor.yaml#
> >
> > This schema says 'multi-led' here should have a child called
> > "^multi-led@([0-9a-f])$". You are off a level.
>
> So it should have been?
>
> properties:
>    compatible:
>      const: pwm-leds-multicolor
>    allOf:
>      - $ref: leds-class-multicolor.yaml#

Not quite. DT property names and json-schema vocab names should never
be at the same level. So allOf should be at the root level.

> This would imply that the multi-led node requires a unit address (reg
> property). That doesn't make sense in this case. How should we resolve this?

I meant to mention that. Update the regex pattern to allow just
'multi-led': "^multi-led(@[0-9a-f])?$"


> >> +    patternProperties:
> >> +      "^led-[0-9a-z]+$":
> >> +        type: object
> >
> >             $ref: common.yaml#
> >             additionalProperties: false
>
> Sounds good.
>
> >> +        properties:
> >> +          pwms:
> >> +            maxItems: 1
> >> +
> >> +          pwm-names: true
> >> +
> >> +          color:
> >> +            $ref: common.yaml#/properties/color
> >
> > And then drop this ref.
>
> Curiosity question: why? Should I refer to an unsigned integer type instead?

Generally we want schemas to apply to nodes rather than individual
properties. Think of each node as a class and nodes can be 1 or more
subclasses. It's more important when using 'unevaluatedProperties' in
combination with refs.

'color: true' is all you need here. So it's less duplication. Not so
much here since it is just 1 property, but in general.

>
> >> +    rgb-led {
> >> +        compatible = "pwm-leds-multicolor";
> >> +
> >> +        multi-led {
> >
> > Can't this be collapsed into 1 level? I don't see "pwm-leds-multicolor"
> > having other child nodes.
>
> It could. The reason I added the multi-led level is because the
> leds-class-multicolor.yaml schema calls for it. Perhaps I missed the
> intention of that schema but isn't it there to create a uniform binding
> schema structure across drivers?

Yeah, I guess that's a good enough reason.

Rob