On 12/01/2016 03:28 PM, Ralph Sennhauser wrote:
Below the oops with your debug patch applied.


root@wrt1900acs:/# cd sys/class/leds/pca963x\:shelby\:white\:usb2/
 echo usbport > trigger
[  124.727665] leds pca963x:shelby:white:usb2: [usbport_trig_add_port] 
[  124.735266] leds pca963x:shelby:white:usb2: [usbport_trig_add_port] 
port->port_name:usb1-port1 port->data:dd708140
[  124.745671] leds pca963x:shelby:white:usb2: [usbport_trig_add_port] 
[  124.753194] leds pca963x:shelby:white:usb2: [usbport_trig_add_port] 
port->port_name:usb2-port1 port->data:dd708140
[  124.763594] leds pca963x:shelby:white:usb2: [usbport_trig_add_port] 
[  124.771114] leds pca963x:shelby:white:usb2: [usbport_trig_add_port] 
port->port_name:usb3-port1 port->data:dd708140
 echo 1 > ports/usb2-port1[  171.649751] leds pca963x:shelby:white:usb2: 
[usbport_trig_port_store] buf:1
[  171.649751]  size:2

[  171.660160] leds pca963x:shelby:white:usb2: [usbport_trig_port_store] 
[  171.668103] leds pca963x:shelby:white:usb2: [usbport_trig_port_store] 
port->port_name:usb2-port1 port->data:dd708140
[  171.678678] [usbport_trig_update_count] usbport_data->count:0
[  171.684457] [usbport_trig_update_count] usbport_data->count:0
[  171.690253] Unable to handle kernel NULL pointer dereference at virtual 
address 00000000

Oh, so this happens a bit later than I expected or I could read from the
backtrace. Anyway this debugging was still helpful, I think I can see a possible

So most likely the crash happens at the:
led_cdev->brightness_set(led_cdev, ...);
call. I'm not sure what I was thinking when writing that code. It looks wrong.

The thing is some LEDs (drivers) don't provide brightness_set op. It's fine, we
should just use brightness_set_blocking op when that happens. Of course kernel
has proper helpers for that, we don't have to worry about the choice of op or
scheduling the work. I have no idea why I didn't use a proper helper in the
first place.

So we should simply replace above call with one of following ones:
1) led_set_brightness(led_cdev, ...);
2) led_set_brightness_nosleep(led_cdev, ...);
3) led_set_brightness_sync(led_cdev, ...);

I still have to check which one is correct. In theory we don't deal blinking at
this point so we shouldn't need to use led_set_brightness.

led_set_brightness_nosleep looks like the most likely correct choice.

led_set_brightness_sync requires brightness_set_blocking which is not always
present so most likely we don't want this one.

If you have some free time and you want to play with this, please replace
and give it a try. This should fix crashes for you.

I'll look at this again during next days.

Reply via email to