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/ root@wrt1900acs:/sys/devices/platform/soc/soc:internal-regs/f1011000.i2c/i2c-0/0-0068/leds/pca963x:shelby:white:usb2# echo usbport > trigger [ 124.727665] leds pca963x:shelby:white:usb2: [usbport_trig_add_port] port:dd708240 [ 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] port:dd7083c0 [ 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] port:dd708300 [ 124.771114] leds pca963x:shelby:white:usb2: [usbport_trig_add_port] port->port_name:usb3-port1 port->data:dd708140 root@wrt1900acs:/sys/devices/platform/soc/soc:internal-regs/f1011000.i2c/i2c-0/0-0068/leds/pca963x:shelby:white:usb2# 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] port:dd7083c0 [ 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 problem. 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 led_cdev->brightness_set with led_set_brightness_nosleep and give it a try. This should fix crashes for you. I'll look at this again during next days. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
