beagleboard / Latest-Images

Please use; https://git.beagleboard.org/beagleboard/Latest-Images
https://git.beagleboard.org/beagleboard/Latest-Images
MIT License
10 stars 11 forks source link

pwm export broken in bb-customizations 20210225 #82

Open mvduin opened 3 years ago

mvduin commented 3 years ago

In /etc/udev/rules.d/81-pwm-noroot.rules in bb-customizations 20210225 the two lines below "automatically export pwm channels" are inexplicably commented out. Doing so prevents pwm channels from being automatically exported and makes the entire udev rule essentially pointless.

To make the pwm channels usable for programmers they should be exported to userspace.

This is a regression from 20200522 where it worked correctly.

RobertCNelson commented 3 years ago

Hey @mvduin yeah that udev rule is awesome... But, it breaks any overlay with a backlight..

https://github.com/beagleboard/bb.org-overlays/blob/master/src/arm/BB-BONE-NH7C-01-A0.dts#L188-L205

Some history:

https://github.com/beagleboard/Latest-Images/issues/45

https://github.com/beagleboard/Latest-Images/issues/54

Is there a conditional statement we can add to the pwm rule to "not" run when a backlight node is present in the device tree?

Regards,

RobertCNelson commented 3 years ago

Sorry, was in a meeting, here's more details..

With the udev rule active, the pwm channel will get exported way before the lcd's backlight can grab it, so users end up with an lcd screen that doesn't light up.. (silly race condition..) could we have a delay, or check for an backlight node?

Regards,

mvduin commented 3 years ago

Ah. We could check for a backlight node, but it would be difficult to impossible to check which pwm output it uses. A better solution would be to have overlays that want to use a pwm output leave an explicit tag in DT to prevent it from being exported, something like

&ehrpwm1 {
    pwm-0-no-export;
};

and test for that tag in the udev rule

mvduin commented 3 years ago

I think the udev rule could check for that like this but I'd need to test it to be sure:

SUBSYSTEM=="pwm", KERNEL=="pwmchip*", ACTION=="add", TEST!="device/of_node/pwm-0-no-export",  ATTR{export}="0"
SUBSYSTEM=="pwm", KERNEL=="pwmchip*", ACTION=="add", ATTR{npwm}!="1", TEST!="device/of_node/pwm-1-no-export",  ATTR{export}="1"
RobertCNelson commented 3 years ago

@mvduin cool, i'll test that idea tomorrow, with an LCD/BBB combo. ;)

mvduin commented 3 years ago

If TEST!= doesn't work like I hope it does, this would be the alternative:

SUBSYSTEM=="pwm", KERNEL=="pwmchip*", TEST=="device/of_node/pwm-0-no-export",  GOTO="skip_pwm_0_export"
SUBSYSTEM=="pwm", KERNEL=="pwmchip*", ACTION=="add",  ATTR{export}="0"
LABEL="skip_pwm_0_export"
SUBSYSTEM=="pwm", KERNEL=="pwmchip*", TEST=="device/of_node/pwm-1-no-export",  GOTO="skip_pwm_1_export"
SUBSYSTEM=="pwm", KERNEL=="pwmchip*", ACTION=="add", ATTR{npwm}!="1",  ATTR{export}="1"
LABEL="skip_pwm_1_export"
mvduin commented 3 years ago

Or just patch out the in-use check like you did with gpios ;-)

(please don't)

mvduin commented 3 years ago

any update on this?