LED Trigger Pattern issue

Technical Support
, , ,
1

Hi,

We have a custom carrier board for Verdin iMX8M-Plus SoM and use linux-toradex_5.4.193.

And the carrier board has a LED on PWM_1 pin of the SOM (SODIMM 15). What we want to get is that during booting, the led to be faded in and out periodically.

For that, first I enabled CONFIG_LEDS_TRIGGER_PATTERN=y in toradex_defconfig file.
And second I defined the LED in the device tree:

/ {
    pwmleds {
        compatible = "pwm-leds";

        led@0 {
            label = "power";
            pwms = <&pwm1 0 2000000 0>;
            max_brightness = <255>;
            linux,default-trigger = "pattern";
            led-pattern = <0 500 255 500>;
        };
    };
};

/* Verdin PWM_1 */
&pwm1 {
    status = "okay";
};

But I never get the LED turned on or any special effect with above device tree. It simply stays turned off during boot time.

On the other side I get expected effect at runtime if I type following line in the terminal:
echo "0 500 255 500" > /sys/class/leds/power/pattern

Also, if I change following line in device tree with timer instead of pattern :
linux,default-trigger = "timer"; then I start seeing the led blinking every half seconds during boot time. So something seems wrong with pattern setting. Have anyone tested this feature before?

Thank you.

2

Hi @Fide !

I could reproduce the behavior that you are facing.

Also, interestingly, the led just don’t appear at /sys/class/leds if this pwm-led node is applied via device tree overlay (instead of recompiling the whole device tree - the dtb file)

I am trying to understand why it isn’t working during boot.

I will let you know if I find something.

Best reagrds

1 Like
3

Hi @henrique.tx ,

Thank you for the confirmation. Since this requirement is pretty standard among our products, I can say that same pattern configuration works fine under Colibri VF50 with Linux Kernel 4.4. And we don’t use device tree overlay mechanism, we have one complete device tree per products.

Looking forward to hearing from you for a solution.

Regards,
Fide.

4

Hi @Fide !

I tried to find out how the LED pattern works on kernel 4.4 (specifically on linux-toradex, branch toradex_vf_4.4), but seems like there is no support for it (or maybe it had another name…).

Did you/your team implement it on kernel 4.4 by yourselves?

Best regards,

5

Actually yes we backported led pattern feature to Toradex Linux 4.4.

I went though the code in our toradex_vf_4.4 repo and linux-toradex_5.4.193 and realized that one of the patch is missing in the upstream linux code.

linux-toradex_5.4.193\drivers\leds\leds-pwm.c needs to be modified as:

static int led_pwm_add(struct device *dev, struct led_pwm_priv *priv,
		       struct led_pwm *led, struct fwnode_handle *fwnode)
{
	struct led_pwm_data *led_data = &priv->leds[priv->num_leds];
	struct pwm_args pargs;
	int ret;
    struct led_init_data init_data = {};

	led_data->active_low = led->active_low;
	led_data->cdev.name = led->name;
	led_data->cdev.default_trigger = led->default_trigger;
	led_data->cdev.brightness = LED_OFF;
	led_data->cdev.max_brightness = led->max_brightness;
	led_data->cdev.flags = LED_CORE_SUSPENDRESUME;

	if (fwnode)
		led_data->pwm = devm_fwnode_pwm_get(dev, fwnode, NULL);
	else
		led_data->pwm = devm_pwm_get(dev, led->name);
	if (IS_ERR(led_data->pwm)) {
		ret = PTR_ERR(led_data->pwm);
		if (ret != -EPROBE_DEFER)
			dev_err(dev, "unable to request PWM for %s: %d\n",
				led->name, ret);
		return ret;
	}

	led_data->cdev.brightness_set_blocking = led_pwm_set;

	/*
	 * FIXME: pwm_apply_args() should be removed when switching to the
	 * atomic PWM API.
	 */
	pwm_apply_args(led_data->pwm);

	pwm_get_args(led_data->pwm, &pargs);

	led_data->period = pargs.period;
	if (!led_data->period && (led->pwm_period_ns > 0))
		led_data->period = led->pwm_period_ns;

	init_data.fwnode = fwnode;
	ret = devm_led_classdev_register_ext(dev, &led_data->cdev, &init_data);
	if (ret == 0) {
		priv->num_leds++;
		led_pwm_set(&led_data->cdev, led_data->cdev.brightness);
	} else {
		dev_err(dev, "failed to register PWM led for %s: %d\n",
			led->name, ret);
	}

	return ret;
}

Now pattern works as expected.
Thank you.

6

Hi @Fide !

That is great!

Thanks for the feedback!

It would be great if you could submit it to upstream :smiley:

Would you do it?

Best regards,

7

Hi @henrique.tx

I went through linux kernel history and saw that our patch was already in the upstream code (cb14e6d6d8f411b7a05f36d1f877450c036d8c56 → 07/12/2018)

But later on 09/06/2019 with commit b2b998c0f944993c9ef435569651e407d607af41, they introduced the same bug by overriding the our changes.

And finally they fixed the bug again on 19/09/2020 with commit de73f275a059a01c5b514c7762bc80821f8df6d9

You can cherry pick the last one.

Regards,
Fide.

1 Like
8

Hi @Fide !

Thanks for the feedback :slight_smile:

Best regards,

1 Like