Hi Nicolas,
Regardless of the fact that LED mc framework in current shape
will probably not materialize in mainline, I have single
remark regarding LED initialization. Please take a look below.
On 2/26/20 3:33 PM, Nicolas Belin wrote:
> Initilial commit in order to support the apa102c RGB leds. This
> is based on the Multicolor Framework.
>
> Reviewed-by: Corentin Labbe <clabbe@xxxxxxxxxxxx>
> Signed-off-by: Nicolas Belin <nbelin@xxxxxxxxxxxx>
> ---
> drivers/leds/Kconfig | 7 ++
> drivers/leds/Makefile | 1 +
> drivers/leds/leds-apa102c.c | 291 ++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 299 insertions(+)
> create mode 100644 drivers/leds/leds-apa102c.c
>
> diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
> index 5dc6535a88ef..71e29727c6ec 100644
> --- a/drivers/leds/Kconfig
> +++ b/drivers/leds/Kconfig
> @@ -79,6 +79,13 @@ config LEDS_AN30259A
> To compile this driver as a module, choose M here: the module
> will be called leds-an30259a.
>
> +config LEDS_APA102C
> + tristate "LED Support for Shiji APA102C"
> + depends on SPI
> + depends on LEDS_CLASS_MULTI_COLOR
> + help
> + This option enables support for APA102C LEDs.
> +
> config LEDS_APU
> tristate "Front panel LED support for PC Engines APU/APU2/APU3 boards"
> depends on LEDS_CLASS
> diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
> index b5305b7d43fb..8334cb6dc7e8 100644
[...]
> +
> + led->priv = priv;
> + led->ldev.max_brightness = MAX_BRIGHTNESS;
> + fwnode_property_read_string(child, "linux,default-trigger",
> + &led->ldev.default_trigger);
> +
> + init_data.fwnode = child;
> + init_data.devicename = APA_DEV_NAME;
> + init_data.default_label = ":";
devicename property should be filled in new drivers only in case
devname_mandatory is set to true.
default_label property is for legacy drivers, for backward compatibility
with old LED naming convention.
For more information please refer to:
- Documentation/leds/leds-class.rst, "LED Device Naming" section
- struct led_init_data documention in linux/leds.h
In effect you need only fwnode here,
> +
> + num_colors = 0;
> + fwnode_for_each_child_node(child, grandchild) {
> + ret = fwnode_property_read_u32(grandchild, "color",
> + &color_id);
> + if (ret) {
> + dev_err(priv->dev, "Cannot read color\n");
> + goto child_out;
> + }
> +
> + set_bit(color_id, &led->mc_cdev.available_colors);
> + num_colors++;
> + }
> +
> + if (num_colors != 3) {
> + ret = -EINVAL;
> + dev_err(priv->dev, "There should be 3 colors\n");
> + goto child_out;
> + }
> +
> + if (led->mc_cdev.available_colors != IS_RGB) {
> + ret = -EINVAL;
> + dev_err(priv->dev, "The led is expected to be RGB\n");
> + goto child_out;
> + }
> +
> + led->mc_cdev.num_leds = num_colors;
> + led->mc_cdev.led_cdev = &led->ldev;
> + led->ldev.brightness_set_blocking = apa102c_brightness_set;
> + ret = devm_led_classdev_multicolor_register_ext(priv->dev,
--
Best regards,
Jacek Anaszewski
