2016-11-21 08:40:27

by Ralph Sennhauser

[permalink] [raw]
Subject: [BUG 4.9] New led trigger usbport gets the kernel to panic

Hi Rafał,

I tried your new usbport trigger in Linux 4.9 with little luck as can be seen in
the following output of the serial console.

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
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
[ 1461.761528] Unable to handle kernel NULL pointer dereference at virtual address 00000000
[ 1461.769734] pgd = de33c000
[ 1461.772454] [00000000] *pgd=1cb2a831, *pte=00000000, *ppte=00000000
[ 1461.778791] Internal error: Oops: 80000007 [#1] SMP ARM
[ 1461.784036] Modules linked in: iptable_nat nft_chain_nat_ipv4 nf_tables_inet
nf_nat_ipv4 nf_conntrack_ipv6 nf_conntrack_ipv4 ipt_REJECT ipt_MASQUERADE
xt_time xt_tcpudp xt_state xt_nat xt_multiport xt_mark xt_mac xt_limit
xt_conntrack xt_comment xt_TCP MSS xt_REDIRECT xt_LOG xt_CT nft_set_rbtree
nft_set_hash nft_reject_inet nft_reject nft_redir_ipv4 nft_redir nft_nat
nft_meta nft_masq_ipv4 nft_masq nft_log nft_limit nft_hash nft_exthdr nft_ct
nft_counter nft_chain_route_ipv6 nft_chain_route_ipv4 nf_tabl es_ipv6
nf_tables_ipv4 nf_tables nf_reject_ipv4 nf_nat_redirect nf_nat_masquerade_ipv4
nf_nat nf_log_ipv4 nf_defrag_ipv6 nf_defrag_ipv4 nf_conntrack_netlink
nf_conntrack macvlan libcrc32c iptable_raw iptable_mangle iptable_filter
ip_tables act_skbedit act _mirred em_u32 cls_u32 cls_tcindex cls_flow cls_route
cls_fw sch_hfsc sch_ingress mwlwifi(O) mac80211 cfg80211 ledtrig_netdev(O)
ip_set_list_set ip_set_hash_netiface ip_set_hash_netport ip_set_hash_netnet
ip_set_hash_net ip_set_hash_netportnet ip_set_hash _mac ip_set_hash_ipportnet
ip_set_hash_ipportip ip_set_hash_ipport ip_set_hash_ipmark ip_set_hash_ip
ip_set_bitmap_port ip_set_bitmap_ipmac ip_set_bitmap_ip ip_set nfnetlink
ip6t_REJECT nf_reject_ipv6 nf_log_ipv6 nf_log_common ip6table_raw
ip6table_mangle ip6table_filter ip6_tables x_tables ifb sit tunnel4 ip_tunnel
vfat fat nls_utf8 nls_iso8859_1 nls_cp850 nls_cp437 rfkill input_core
sha256_generic seqiv jitterentropy_rng drbg hmac ghash_generic gf128mul gcm ctr
ccm ledtrig_usbport ext4 jbd2 mbcache btrf s xor raid6_pq crc32c_generic ubifs
ubi
[ 1461.922401] CPU: 0 PID: 759 Comm: ash Tainted: G O 4.9.0-rc6 #2
[ 1461.929476] Hardware name: Marvell Armada 380/385 (Device Tree)
[ 1461.935418] task: df621080 task.stack: dee0e000
[ 1461.939966] PC is at 0x0
[ 1461.942510] LR is at usbport_trig_port_store+0x8c/0xd8 [ledtrig_usbport]
[ 1461.949238] pc : [<00000000>] lr : [<bf2281f0>] psr: 60000013
[ 1461.949238] sp : dee0fe50 ip : dee0fe10 fp : dee0fe6c
[ 1461.960761] r10: 00000000 r9 : 00000000 r8 : 00000000
[ 1461.966006] r7 : dcabc7c0 r6 : df4164ec r5 : 00000002 r4 : dcabc040
[ 1461.972558] r3 : 00000000 r2 : 0000001a r1 : 00000000 r0 : df4164ec
[ 1461.979112] Flags: nZCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none
[ 1461.986275] Control: 10c5387d Table: 1e33c04a DAC: 00000051
[ 1461.992043] Process ash (pid: 759, stack limit = 0xdee0e210)
[ 1461.997724] Stack: (0xdee0fe50 to 0xdee10000)
[ 1462.002097] fe40: bf228164 00000002 dee0ff80 dcabc7c0
[ 1462.010309] fe60: dee0fe84 dee0fe70 c023f758 bf228170 c023f738 00000002 dee0fe9c dee0fe88
[ 1462.018520] fe80: c0169310 c023f744 df4a1000 00000002 dee0fedc dee0fea0 c0168ab8 c01692d4
[ 1462.026731] fea0: 00000000 00000000 df747c10 df4a100c d4aca1c9 df727a80 c01689c0 dee0ff80
[ 1462.034943] fec0: 00000002 c000fc24 dee0e000 00000000 dee0ff4c dee0fee0 c0100c78 c01689cc
[ 1462.043153] fee0: 00000000 dee0fee8 dee0ff14 dee0fef8 dee0ff14 dee0ff00 c0060668 c04640a0
[ 1462.051365] ff00: dee0ff50 df621350 dee0ff4c dee0ff18 c00298f8 c0060630 fffffff6 dee0ff68
[ 1462.059576] ff20: c00ff23c 00000007 00000002 df727a80 b6f092d0 dee0ff80 c000fc24 dee0e000
[ 1462.067786] ff40: dee0ff7c dee0ff50 c0101ae4 c0100c50 00000003 00000007 df727a80 df727a80
[ 1462.075997] ff60: b6f092d0 00000002 c000fc24 dee0e000 dee0ffa4 dee0ff80 c01028dc c0101a44
[ 1462.084208] ff80: 00000000 00000000 00000000 00000000 ffffffff 00000004 00000000 dee0ffa8
[ 1462.092419] ffa0: c000fa60 c010289c 00000000 00000000 00000001 b6f092d0 00000002 00000000
[ 1462.100630] ffc0: 00000000 00000000 ffffffff 00000004 b6f092d0 00000020 00059ec0 00059ea0
[ 1462.108841] ffe0: beb744f8 beb744e4 b6ee1904 b6ee0dc8 60000010 00000001 00000000 00000000
[ 1462.117049] Backtrace:
[ 1462.119521] [<bf228164>] (usbport_trig_port_store [ledtrig_usbport]) from [<c023f758>] (dev_attr_store+0x20/0x2c)
[ 1462.129826] r7:dcabc7c0 r6:dee0ff80 r5:00000002 r4:bf228164
[ 1462.135511] [<c023f738>] (dev_attr_store) from [<c0169310>] (sysfs_kf_write+0x48/0x4c)
[ 1462.143459] r5:00000002 r4:c023f738
[ 1462.147049] [<c01692c8>] (sysfs_kf_write) from [<c0168ab8>] (kernfs_fop_write+0xf8/0x1f8)
[ 1462.155258] r5:00000002 r4:df4a1000
[ 1462.158850] [<c01689c0>] (kernfs_fop_write) from [<c0100c78>] (__vfs_write+0x34/0x120)
[ 1462.166800] r10:00000000 r9:dee0e000 r8:c000fc24 r7:00000002 r6:dee0ff80 r5:c01689c0
[ 1462.174660] r4:df727a80
[ 1462.177204] [<c0100c44>] (__vfs_write) from [<c0101ae4>] (vfs_write+0xac/0x170)
[ 1462.184543] r9:dee0e000 r8:c000fc24 r7:dee0ff80 r6:b6f092d0 r5:df727a80 r4:00000002
[ 1462.192319] [<c0101a38>] (vfs_write) from [<c01028dc>] (SyS_write+0x4c/0xa8)
[ 1462.199396] r9:dee0e000 r8:c000fc24 r7:00000002 r6:b6f092d0 r5:df727a80 r4:df727a80
[ 1462.207174] [<c0102890>] (SyS_write) from [<c000fa60>] (ret_fast_syscall+0x0/0x3c)
[ 1462.214774] r7:00000004 r6:ffffffff r5:00000000 r4:00000000
[ 1462.220456] Code: bad PC value
[ 1462.223560] ---[ end trace 676638a3a12c7a56 ]---
[ 1462.228197] Kernel panic - not syncing: Fatal exception
[ 1462.233445] CPU1: stopping
[ 1462.236165] CPU: 1 PID: 0 Comm: swapper/1 Tainted: G D O 4.9.0-rc6 #2
[ 1462.243590] Hardware name: Marvell Armada 380/385 (Device Tree)
[ 1462.249531] Backtrace:
[ 1462.252001] [<c00138a8>] (dump_backtrace) from [<c0013b7c>] (show_stack+0x18/0x1c)
[ 1462.259601] r7:df47ff48 r6:60000193 r5:c07dea44 r4:00000000
[ 1462.265285] [<c0013b64>] (show_stack) from [<c01b4fb8>] (dump_stack+0x94/0xa8)
[ 1462.272538] [<c01b4f24>] (dump_stack) from [<c0016bdc>] (handle_IPI+0x178/0x198)
[ 1462.279963] r7:df47ff48 r6:00000000 r5:00000001 r4:c07f9230
[ 1462.285645] [<c0016a64>] (handle_IPI) from [<c00094e4>] (gic_handle_irq+0x90/0x94)
[ 1462.293245] r7:df47ff48 r6:e080210c r5:c07cfbb8 r4:c07ded00
[ 1462.298927] [<c0009454>] (gic_handle_irq) from [<c00146cc>] (__irq_svc+0x6c/0x90)
[ 1462.306439] Exception stack(0xdf47ff48 to 0xdf47ff90)
[ 1462.311510] ff40: 00000001 00000000 00000000 c0020580 df47e000 c07cf02c
[ 1462.319721] ff60: c07cf07c 00000002 00000000 00000000 c07ca378 df47ffa4 df47ffa8 df47ff98
[ 1462.327932] ff80: c00105ac c00105b0 60000013 ffffffff
[ 1462.333004] r9:df47e000 r8:00000000 r7:df47ff7c r6:ffffffff r5:60000013 r4:c00105b0
[ 1462.340781] [<c0010570>] (arch_cpu_idle) from [<c0463f4c>] (default_idle_call+0x28/0x34)
[ 1462.348908] [<c0463f24>] (default_idle_call) from [<c0061494>] (cpu_startup_entry+0x118/0x1f0)
[ 1462.357557] [<c006137c>] (cpu_startup_entry) from [<c0016808>] (secondary_start_kernel+0x150/0x15c)
[ 1462.366638] r7:c07f9240
[ 1462.369181] [<c00166b8>] (secondary_start_kernel) from [<0000962c>] (0x962c)
[ 1462.376256] r5:00000051 r4:1f46406a
[ 1462.379845] Rebooting in 3 seconds..


Would you happen to have an idea what is going wrong here?

Thanks
Ralph


2016-11-29 00:50:23

by Rafał Miłecki

[permalink] [raw]
Subject: Re: [BUG 4.9] New led trigger usbport gets the kernel to panic

Hi Ralph,

On 11/21/2016 09:40 AM, Ralph Sennhauser wrote:
> I tried your new usbport trigger in Linux 4.9 with little luck as can be seen in
> the following output of the serial console.

I'm really happy my trigger got some attention. I'm sorry it crashes for you,
I never experienced any crash so far. I also got few ppl using it as part of
LEDE project but no reports from them neither.

Also sorry for the late reply, my private life took all my time previous week.


> 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
> 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
> [ 1461.761528] Unable to handle kernel NULL pointer dereference at virtual address 00000000
> [ 1461.769734] pgd = de33c000
> [ 1461.772454] [00000000] *pgd=1cb2a831, *pte=00000000, *ppte=00000000
> [ 1461.778791] Internal error: Oops: 80000007 [#1] SMP ARM
> [ 1461.784036] Modules linked in: iptable_nat nft_chain_nat_ipv4 nf_tables_inet
> nf_nat_ipv4 nf_conntrack_ipv6 nf_conntrack_ipv4 ipt_REJECT ipt_MASQUERADE
> xt_time xt_tcpudp xt_state xt_nat xt_multiport xt_mark xt_mac xt_limit
> xt_conntrack xt_comment xt_TCP MSS xt_REDIRECT xt_LOG xt_CT nft_set_rbtree
> nft_set_hash nft_reject_inet nft_reject nft_redir_ipv4 nft_redir nft_nat
> nft_meta nft_masq_ipv4 nft_masq nft_log nft_limit nft_hash nft_exthdr nft_ct
> nft_counter nft_chain_route_ipv6 nft_chain_route_ipv4 nf_tabl es_ipv6
> nf_tables_ipv4 nf_tables nf_reject_ipv4 nf_nat_redirect nf_nat_masquerade_ipv4
> nf_nat nf_log_ipv4 nf_defrag_ipv6 nf_defrag_ipv4 nf_conntrack_netlink
> nf_conntrack macvlan libcrc32c iptable_raw iptable_mangle iptable_filter
> ip_tables act_skbedit act _mirred em_u32 cls_u32 cls_tcindex cls_flow cls_route
> cls_fw sch_hfsc sch_ingress mwlwifi(O) mac80211 cfg80211 ledtrig_netdev(O)
> ip_set_list_set ip_set_hash_netiface ip_set_hash_netport ip_set_hash_netnet
> ip_set_hash_net ip_set_hash_netportnet ip_set_hash _mac ip_set_hash_ipportnet
> ip_set_hash_ipportip ip_set_hash_ipport ip_set_hash_ipmark ip_set_hash_ip
> ip_set_bitmap_port ip_set_bitmap_ipmac ip_set_bitmap_ip ip_set nfnetlink
> ip6t_REJECT nf_reject_ipv6 nf_log_ipv6 nf_log_common ip6table_raw
> ip6table_mangle ip6table_filter ip6_tables x_tables ifb sit tunnel4 ip_tunnel
> vfat fat nls_utf8 nls_iso8859_1 nls_cp850 nls_cp437 rfkill input_core
> sha256_generic seqiv jitterentropy_rng drbg hmac ghash_generic gf128mul gcm ctr
> ccm ledtrig_usbport ext4 jbd2 mbcache btrf s xor raid6_pq crc32c_generic ubifs
> ubi
> [ 1461.922401] CPU: 0 PID: 759 Comm: ash Tainted: G O 4.9.0-rc6 #2
> [ 1461.929476] Hardware name: Marvell Armada 380/385 (Device Tree)
> [ 1461.935418] task: df621080 task.stack: dee0e000
> [ 1461.939966] PC is at 0x0
> [ 1461.942510] LR is at usbport_trig_port_store+0x8c/0xd8 [ledtrig_usbport]
> [ 1461.949238] pc : [<00000000>] lr : [<bf2281f0>] psr: 60000013
> [ 1461.949238] sp : dee0fe50 ip : dee0fe10 fp : dee0fe6c
> [ 1461.960761] r10: 00000000 r9 : 00000000 r8 : 00000000
> [ 1461.966006] r7 : dcabc7c0 r6 : df4164ec r5 : 00000002 r4 : dcabc040
> [ 1461.972558] r3 : 00000000 r2 : 0000001a r1 : 00000000 r0 : df4164ec
> [ 1461.979112] Flags: nZCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none
> [ 1461.986275] Control: 10c5387d Table: 1e33c04a DAC: 00000051
> [ 1461.992043] Process ash (pid: 759, stack limit = 0xdee0e210)
> [ 1461.997724] Stack: (0xdee0fe50 to 0xdee10000)
> [ 1462.002097] fe40: bf228164 00000002 dee0ff80 dcabc7c0
> [ 1462.010309] fe60: dee0fe84 dee0fe70 c023f758 bf228170 c023f738 00000002 dee0fe9c dee0fe88
> [ 1462.018520] fe80: c0169310 c023f744 df4a1000 00000002 dee0fedc dee0fea0 c0168ab8 c01692d4
> [ 1462.026731] fea0: 00000000 00000000 df747c10 df4a100c d4aca1c9 df727a80 c01689c0 dee0ff80
> [ 1462.034943] fec0: 00000002 c000fc24 dee0e000 00000000 dee0ff4c dee0fee0 c0100c78 c01689cc
> [ 1462.043153] fee0: 00000000 dee0fee8 dee0ff14 dee0fef8 dee0ff14 dee0ff00 c0060668 c04640a0
> [ 1462.051365] ff00: dee0ff50 df621350 dee0ff4c dee0ff18 c00298f8 c0060630 fffffff6 dee0ff68
> [ 1462.059576] ff20: c00ff23c 00000007 00000002 df727a80 b6f092d0 dee0ff80 c000fc24 dee0e000
> [ 1462.067786] ff40: dee0ff7c dee0ff50 c0101ae4 c0100c50 00000003 00000007 df727a80 df727a80
> [ 1462.075997] ff60: b6f092d0 00000002 c000fc24 dee0e000 dee0ffa4 dee0ff80 c01028dc c0101a44
> [ 1462.084208] ff80: 00000000 00000000 00000000 00000000 ffffffff 00000004 00000000 dee0ffa8
> [ 1462.092419] ffa0: c000fa60 c010289c 00000000 00000000 00000001 b6f092d0 00000002 00000000
> [ 1462.100630] ffc0: 00000000 00000000 ffffffff 00000004 b6f092d0 00000020 00059ec0 00059ea0
> [ 1462.108841] ffe0: beb744f8 beb744e4 b6ee1904 b6ee0dc8 60000010 00000001 00000000 00000000
> [ 1462.117049] Backtrace:
> [ 1462.119521] [<bf228164>] (usbport_trig_port_store [ledtrig_usbport]) from [<c023f758>] (dev_attr_store+0x20/0x2c)
> [ 1462.129826] r7:dcabc7c0 r6:dee0ff80 r5:00000002 r4:bf228164
> [ 1462.135511] [<c023f738>] (dev_attr_store) from [<c0169310>] (sysfs_kf_write+0x48/0x4c)
> [ 1462.143459] r5:00000002 r4:c023f738
> [ 1462.147049] [<c01692c8>] (sysfs_kf_write) from [<c0168ab8>] (kernfs_fop_write+0xf8/0x1f8)
> [ 1462.155258] r5:00000002 r4:df4a1000
> [ 1462.158850] [<c01689c0>] (kernfs_fop_write) from [<c0100c78>] (__vfs_write+0x34/0x120)
> [ 1462.166800] r10:00000000 r9:dee0e000 r8:c000fc24 r7:00000002 r6:dee0ff80 r5:c01689c0
> [ 1462.174660] r4:df727a80
> [ 1462.177204] [<c0100c44>] (__vfs_write) from [<c0101ae4>] (vfs_write+0xac/0x170)
> [ 1462.184543] r9:dee0e000 r8:c000fc24 r7:dee0ff80 r6:b6f092d0 r5:df727a80 r4:00000002
> [ 1462.192319] [<c0101a38>] (vfs_write) from [<c01028dc>] (SyS_write+0x4c/0xa8)
> [ 1462.199396] r9:dee0e000 r8:c000fc24 r7:00000002 r6:b6f092d0 r5:df727a80 r4:df727a80
> [ 1462.207174] [<c0102890>] (SyS_write) from [<c000fa60>] (ret_fast_syscall+0x0/0x3c)
> [ 1462.214774] r7:00000004 r6:ffffffff r5:00000000 r4:00000000
> [ 1462.220456] Code: bad PC value
> [ 1462.223560] ---[ end trace 676638a3a12c7a56 ]---
> [ 1462.228197] Kernel panic - not syncing: Fatal exception
> [ 1462.233445] CPU1: stopping
> [ 1462.236165] CPU: 1 PID: 0 Comm: swapper/1 Tainted: G D O 4.9.0-rc6 #2
> [ 1462.243590] Hardware name: Marvell Armada 380/385 (Device Tree)
> [ 1462.249531] Backtrace:
> [ 1462.252001] [<c00138a8>] (dump_backtrace) from [<c0013b7c>] (show_stack+0x18/0x1c)
> [ 1462.259601] r7:df47ff48 r6:60000193 r5:c07dea44 r4:00000000
> [ 1462.265285] [<c0013b64>] (show_stack) from [<c01b4fb8>] (dump_stack+0x94/0xa8)
> [ 1462.272538] [<c01b4f24>] (dump_stack) from [<c0016bdc>] (handle_IPI+0x178/0x198)
> [ 1462.279963] r7:df47ff48 r6:00000000 r5:00000001 r4:c07f9230
> [ 1462.285645] [<c0016a64>] (handle_IPI) from [<c00094e4>] (gic_handle_irq+0x90/0x94)
> [ 1462.293245] r7:df47ff48 r6:e080210c r5:c07cfbb8 r4:c07ded00
> [ 1462.298927] [<c0009454>] (gic_handle_irq) from [<c00146cc>] (__irq_svc+0x6c/0x90)
> [ 1462.306439] Exception stack(0xdf47ff48 to 0xdf47ff90)
> [ 1462.311510] ff40: 00000001 00000000 00000000 c0020580 df47e000 c07cf02c
> [ 1462.319721] ff60: c07cf07c 00000002 00000000 00000000 c07ca378 df47ffa4 df47ffa8 df47ff98
> [ 1462.327932] ff80: c00105ac c00105b0 60000013 ffffffff
> [ 1462.333004] r9:df47e000 r8:00000000 r7:df47ff7c r6:ffffffff r5:60000013 r4:c00105b0
> [ 1462.340781] [<c0010570>] (arch_cpu_idle) from [<c0463f4c>] (default_idle_call+0x28/0x34)
> [ 1462.348908] [<c0463f24>] (default_idle_call) from [<c0061494>] (cpu_startup_entry+0x118/0x1f0)
> [ 1462.357557] [<c006137c>] (cpu_startup_entry) from [<c0016808>] (secondary_start_kernel+0x150/0x15c)
> [ 1462.366638] r7:c07f9240
> [ 1462.369181] [<c00166b8>] (secondary_start_kernel) from [<0000962c>] (0x962c)
> [ 1462.376256] r5:00000051 r4:1f46406a
> [ 1462.379845] Rebooting in 3 seconds..
>
>
> Would you happen to have an idea what is going wrong here?

I can't see any obvious bug in the trigger code. Could you apply attached
debugging patch and provide another log, please?


diff --git a/drivers/usb/core/ledtrig-usbport.c b/drivers/usb/core/ledtrig-usbport.c
index 3ed5162..1e30922 100644
--- a/drivers/usb/core/ledtrig-usbport.c
+++ b/drivers/usb/core/ledtrig-usbport.c
@@ -73,7 +73,9 @@ static void usbport_trig_update_count(struct usbport_trig_data *usbport_data)
struct led_classdev *led_cdev = usbport_data->led_cdev;

usbport_data->count = 0;
+ pr_info("[%s] usbport_data->count:%d\n", __func__, usbport_data->count);
usb_for_each_dev(usbport_data, usbport_trig_usb_dev_check);
+ pr_info("[%s] usbport_data->count:%d\n", __func__, usbport_data->count);
led_cdev->brightness_set(led_cdev,
usbport_data->count ? LED_FULL : LED_OFF);
}
@@ -96,9 +98,15 @@ static ssize_t usbport_trig_port_store(struct device *dev,
struct device_attribute *attr,
const char *buf, size_t size)
{
- struct usbport_trig_port *port = container_of(attr,
+ struct usbport_trig_port *port;
+
+ dev_info(dev, "[%s] buf:%s size:%zu\n", __func__, buf, size);
+
+ port = container_of(attr,
struct usbport_trig_port,
attr);
+ dev_info(dev, "[%s] port:%p\n", __func__, port);
+ dev_info(dev, "[%s] port->port_name:%s port->data:%p\n", __func__, port->port_name, port->data);

if (!strcmp(buf, "0") || !strcmp(buf, "0\n"))
port->observed = 0;
@@ -109,6 +117,7 @@ static ssize_t usbport_trig_port_store(struct device *dev,

usbport_trig_update_count(port->data);

+ dev_info(dev, "[%s] size:%zu\n", __func__, size);
return size;
}

@@ -163,6 +172,8 @@ static int usbport_trig_add_port(struct usbport_trig_data *usbport_data,

list_add_tail(&port->list, &usbport_data->ports);

+ dev_info(led_cdev->dev, "[%s] port:%p\n", __func__, port);
+ dev_info(led_cdev->dev, "[%s] port->port_name:%s port->data:%p\n", __func__, port->port_name, port->data);
return 0;

err_free_port_name:

2016-12-01 14:29:28

by Ralph Sennhauser

[permalink] [raw]
Subject: Re: [BUG 4.9] New led trigger usbport gets the kernel to panic

On Mon, 28 Nov 2016 23:20:13 +0100
Rafal Milecki <[email protected]> wrote:

> Hi Ralph,
>
> On 11/21/2016 09:40 AM, Ralph Sennhauser wrote:
> > I tried your new usbport trigger in Linux 4.9 with little luck as
> > can be seen in the following output of the serial console.
>
> I'm really happy my trigger got some attention. I'm sorry it crashes
> for you, I never experienced any crash so far. I also got few ppl
> using it as part of LEDE project but no reports from them neither.
>
> Also sorry for the late reply, my private life took all my time
> previous week.
>
>

Hi Rafal

no worries, I know well how it is not to have enough time.

Below the oops with your debug patch applied.

Thanks
Ralph



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
[ 171.698382] pgd = de71c000
[ 171.701102] [00000000] *pgd=1e362831, *pte=00000000, *ppte=00000000
[ 171.707427] Internal error: Oops: 80000007 [#1] SMP ARM
[ 171.712671] Modules linked in: iptable_nat nft_chain_nat_ipv4 nf_tables_inet
nf_nat_ipv4 nf_conntrack_ipv6 nf_conntrack_ipv4 ipt_REJECT ipt_MASQUERADE
xt_time xt_tcpudp xt_state xt_nat xt_multiport xt_mark xt_mac xt_limit
xt_conntrack xt_comment xt_TCP MSS xt_REDIRECT xt_LOG xt_CT nft_set_rbtree
nft_set_hash nft_reject_inet nft_reject nft_redir_ipv4 nft_redir nft_nat
nft_meta nft_masq_ipv4 nft_masq nft_log nft_limit nft_hash nft_exthdr nft_ct
nft_counter nft_chain_route_ipv6 nft_chain_route_ipv4 nf_tabl es_ipv6
nf_tables_ipv4 nf_tables nf_reject_ipv4 nf_nat_redirect nf_nat_masquerade_ipv4
nf_nat nf_log_ipv4 nf_defrag_ipv6 nf_defrag_ipv4 nf_conntrack_netlink
nf_conntrack macvlan libcrc32c iptable_raw iptable_mangle iptable_filter
ip_tables act_skbedit act _mirred em_u32 cls_u32 cls_tcindex cls_flow cls_route
cls_fw sch_hfsc sch_ingress mwlwifi(O) mac80211 cfg80211 ledtrig_netdev(O)
ip_set_list_set ip_set_hash_netiface ip_set_hash_netport ip_set_hash_netnet
ip_set_hash_net ip_set_hash_netportnet ip_set_hash _mac ip_set_hash_ipportnet
ip_set_hash_ipportip ip_set_hash_ipport ip_set_hash_ipmark ip_set_hash_ip
ip_set_bitmap_port ip_set_bitmap_ipmac ip_set_bitmap_ip ip_set nfnetlink
ip6t_REJECT nf_reject_ipv6 nf_log_ipv6 nf_log_common ip6table_raw
ip6table_mangle ip6table_filter ip6_tables x_tables ifb sit tunnel4 ip_tunnel
vfat fat nls_utf8 nls_iso8859_1 nls_cp850 nls_cp437 rfkill input_core
sha256_generic seqiv jitterentropy_rng drbg hmac ghash_generic gf128mul gcm ctr
ccm ledtrig_usbport ext4 jbd2 mbcache btrf
s xor raid6_pq crc32c_generic ubifs ubi
[ 171.851028] CPU: 1 PID: 724 Comm: ash Tainted: G O 4.9.0-rc7 #2
[ 171.858103] Hardware name: Marvell Armada 380/385 (Device Tree)
[ 171.864046] task: df71bc80 task.stack: df7d0000
[ 171.868594] PC is at 0x0
[ 171.871139] LR is at usbport_trig_port_store+0x10c/0x17c [ledtrig_usbport]
[ 171.878042] pc : [<00000000>] lr : [<bf228270>] psr: 60000013
[ 171.878042] sp : df7d1e48 ip : 00000007 fp : df7d1e6c
[ 171.889566] r10: 00000000 r9 : 00000000 r8 : 00000000
[ 171.894811] r7 : df63c4ec r6 : deeec700 r5 : dd708140 r4 : 00000002
[ 171.901363] r3 : 00000000 r2 : 00000000 r1 : 00000000 r0 : df63c4ec
[ 171.907916] Flags: nZCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none
[ 171.915079] Control: 10c5387d Table: 1e71c04a DAC: 00000051
[ 171.920847] Process ash (pid: 724, stack limit = 0xdf7d0210)
[ 171.926528] Stack: (0xdf7d1e48 to 0xdf7d2000)
[ 171.930902] 1e40: dd708140 ddc7a840 bf228164 00000002 df7d1f80 dd708500
[ 171.939114] 1e60: df7d1e84 df7d1e70 c0237e00 bf228170 c0237de0 00000002 df7d1e9c df7d1e88
[ 171.947325] 1e80: c01663f0 c0237dec decc3d80 00000002 df7d1edc df7d1ea0 c0165b98 c01663b4
[ 171.955537] 1ea0: 00000000 00000000 df5976d0 decc3d8c 32859b28 ddc7a840 c0165aa0 df7d1f80
[ 171.963748] 1ec0: 00000002 c000fc24 df7d0000 00000000 df7d1f4c df7d1ee0 c01006a8 c0165aac
[ 171.971959] 1ee0: 00000000 df7d1ee8 df7d1f14 df7d1ef8 df7d1f14 df7d1f00 c0060668 c0440838
[ 171.980171] 1f00: df7d1f50 df71bf50 df7d1f4c df7d1f18 c00298f8 c0060630 fffffff6 df7d1f68
[ 171.988382] 1f20: c00fec6c 00000007 00000002 ddc7a840 b6f7ff10 df7d1f80 c000fc24 df7d0000
[ 171.996592] 1f40: df7d1f7c df7d1f50 c0101514 c0100680 00000003 00000007 ddc7a840 ddc7a840
[ 172.004803] 1f60: b6f7ff10 00000002 c000fc24 df7d0000 df7d1fa4 df7d1f80 c010230c c0101474
[ 172.013014] 1f80: 00000000 00000000 00000000 00000000 00000020 00000004 00000000 df7d1fa8
[ 172.021226] 1fa0: c000fa60 c01022cc 00000000 00000000 00000001 b6f7ff10 00000002 00000000
[ 172.029437] 1fc0: 00000000 00000000 00000020 00000004 b6f7ff10 00000020 00059ec0 00059ea0
[ 172.037648] 1fe0: bef454f8 bef454e4 b6fcd904 b6fccdc8 60000010 00000001 e58d4004 e599c000
[ 172.045857] Backtrace:
[ 172.048327] [<bf228164>] (usbport_trig_port_store [ledtrig_usbport]) from [<c0237e00>] (dev_attr_store+0x20/0x2c)
[ 172.058631] r7:dd708500 r6:df7d1f80 r5:00000002 r4:bf228164
[ 172.064317] [<c0237de0>] (dev_attr_store) from [<c01663f0>] (sysfs_kf_write+0x48/0x4c)
[ 172.072265] r5:00000002 r4:c0237de0
[ 172.075856] [<c01663a8>] (sysfs_kf_write) from [<c0165b98>] (kernfs_fop_write+0xf8/0x1f8)
[ 172.084065] r5:00000002 r4:decc3d80
[ 172.087657] [<c0165aa0>] (kernfs_fop_write) from [<c01006a8>] (__vfs_write+0x34/0x120)
[ 172.095607] r10:00000000 r9:df7d0000 r8:c000fc24 r7:00000002 r6:df7d1f80 r5:c0165aa0
[ 172.103467] r4:ddc7a840
[ 172.106010] [<c0100674>] (__vfs_write) from [<c0101514>] (vfs_write+0xac/0x170)
[ 172.113350] r9:df7d0000 r8:c000fc24 r7:df7d1f80 r6:b6f7ff10 r5:ddc7a840 r4:00000002
[ 172.121125] [<c0101468>] (vfs_write) from [<c010230c>] (SyS_write+0x4c/0xa8)
[ 172.128202] r9:df7d0000 r8:c000fc24 r7:00000002 r6:b6f7ff10 r5:ddc7a840 r4:ddc7a840
[ 172.135979] [<c01022c0>] (SyS_write) from [<c000fa60>] (ret_fast_syscall+0x0/0x3c)
[ 172.143578] r7:00000004 r6:00000020 r5:00000000 r4:00000000
[ 172.149260] Code: bad PC value
[ 172.152357] ---[ end trace 0df651941c29a0cc ]---
[ 172.156994] Kernel panic - not syncing: Fatal exception
[ 172.162240] CPU0: stopping
[ 172.164959] CPU: 0 PID: 0 Comm: swapper/0 Tainted: G D O 4.9.0-rc7 #2
[ 172.172384] Hardware name: Marvell Armada 380/385 (Device Tree)
[ 172.178326] Backtrace:
[ 172.180796] [<c00138a8>] (dump_backtrace) from [<c0013b7c>] (show_stack+0x18/0x1c)
[ 172.188398] r7:c07a1f08 r6:60000193 r5:c07b2744 r4:00000000
[ 172.194082] [<c0013b64>] (show_stack) from [<c01b2ab8>] (dump_stack+0x94/0xa8)
[ 172.201335] [<c01b2a24>] (dump_stack) from [<c0016bdc>] (handle_IPI+0x178/0x198)
[ 172.208760] r7:c07a1f08 r6:00000000 r5:00000000 r4:c07cb4f0
[ 172.214442] [<c0016a64>] (handle_IPI) from [<c00094e4>] (gic_handle_irq+0x90/0x94)
[ 172.222042] r7:c07a1f08 r6:e080210c r5:c07a3bb8 r4:c07b2a00
[ 172.227724] [<c0009454>] (gic_handle_irq) from [<c00146cc>] (__irq_svc+0x6c/0x90)
[ 172.235235] Exception stack(0xc07a1f08 to 0xc07a1f50)
[ 172.240308] 1f00: 00000001 00000000 00000000 c0020580 c07a0000 c07a302c
[ 172.248520] 1f20: c07a307c 00000001 00000000 00000000 c079d378 c07a1f64 c07a1f68 c07a1f58
[ 172.256729] 1f40: c00105ac c00105b0 60000013 ffffffff
[ 172.261801] r9:c07a0000 r8:00000000 r7:c07a1f3c r6:ffffffff r5:60000013 r4:c00105b0
[ 172.269579] [<c0010570>] (arch_cpu_idle) from [<c04406e4>] (default_idle_call+0x28/0x34)
[ 172.277707] [<c04406bc>] (default_idle_call) from [<c0061494>] (cpu_startup_entry+0x118/0x1f0)
[ 172.286354] [<c006137c>] (cpu_startup_entry) from [<c043ac2c>] (rest_init+0x7c/0x80)
[ 172.294128] r7:ffffffff
[ 172.296670] [<c043abb0>] (rest_init) from [<c052ad38>] (start_kernel+0x3bc/0x3c8)
[ 172.304184] [<c052a97c>] (start_kernel) from [<0000807c>] (0x807c)
[ 172.310391] Rebooting in 3 seconds..

2016-12-01 17:02:09

by Rafał Miłecki

[permalink] [raw]
Subject: Re: [BUG 4.9] New led trigger usbport gets the kernel to panic

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.

2016-12-02 08:48:26

by Ralph Sennhauser

[permalink] [raw]
Subject: Re: [BUG 4.9] New led trigger usbport gets the kernel to panic

On Thu, 1 Dec 2016 17:56:07 +0100
Rafał Miłecki <[email protected]> wrote:

> 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.

Hi Rafał

I just gave 2) led_set_brightness_nosleep a try. The kernel no longer
crashes and the led does work as expected.

Thanks
Ralph

2016-12-06 17:26:29

by Pavel Machek

[permalink] [raw]
Subject: Re: [BUG 4.9] New led trigger usbport gets the kernel to panic

On Fri 2016-12-02 09:48:18, Ralph Sennhauser wrote:
> On Thu, 1 Dec 2016 17:56:07 +0100
> Rafał Miłecki <[email protected]> wrote:
>
> > 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.
>
> Hi Rafał
>
> I just gave 2) led_set_brightness_nosleep a try. The kernel no longer
> crashes and the led does work as expected.

Do you have any patch that needs to be applied?

--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


Attachments:
(No filename) (3.71 kB)
signature.asc (181.00 B)
Digital signature
Download all attachments

2016-12-06 17:29:19

by Rafał Miłecki

[permalink] [raw]
Subject: Re: [BUG 4.9] New led trigger usbport gets the kernel to panic

On 6 December 2016 at 18:26, Pavel Machek <[email protected]> wrote:
> On Fri 2016-12-02 09:48:18, Ralph Sennhauser wrote:
>> On Thu, 1 Dec 2016 17:56:07 +0100
>> Rafał Miłecki <[email protected]> wrote:
>>
>> > 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.
>>
>> Hi Rafał
>>
>> I just gave 2) led_set_brightness_nosleep a try. The kernel no longer
>> crashes and the led does work as expected.
>
> Do you have any patch that needs to be applied?

Yes, it has been pushed:
https://git.kernel.org/cgit/linux/kernel/git/gregkh/usb.git/commit/?h=usb-next&id=89778ba335e302a450932ce5b703c1ee6216e949

When sending it I didn't Cc linux-leds as get_maintainer.pl didn't
pick it. Should we add MAINTAINERS entry for ledtrig-usbport.c maybe?

--
Rafał

2016-12-08 08:40:57

by Jacek Anaszewski

[permalink] [raw]
Subject: Re: [BUG 4.9] New led trigger usbport gets the kernel to panic

Hi Rafał,

On 12/06/2016 06:29 PM, Rafał Miłecki wrote:
> On 6 December 2016 at 18:26, Pavel Machek <[email protected]> wrote:
>> On Fri 2016-12-02 09:48:18, Ralph Sennhauser wrote:
>>> On Thu, 1 Dec 2016 17:56:07 +0100
>>> Rafał Miłecki <[email protected]> wrote:
>>>
>>>> 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.
>>>
>>> Hi Rafał
>>>
>>> I just gave 2) led_set_brightness_nosleep a try. The kernel no longer
>>> crashes and the led does work as expected.
>>
>> Do you have any patch that needs to be applied?
>
> Yes, it has been pushed:
> https://git.kernel.org/cgit/linux/kernel/git/gregkh/usb.git/commit/?h=usb-next&id=89778ba335e302a450932ce5b703c1ee6216e949
>
> When sending it I didn't Cc linux-leds as get_maintainer.pl didn't
> pick it. Should we add MAINTAINERS entry for ledtrig-usbport.c maybe?
>

I haven't tested this trigger earlier, probably due to the fact that
it was targeted for usb subsystem, sorry about that. Nonetheless,
I've just tried it on exynos4412-trats2 board and it seems not to
work properly.

I have the board connected through USB (I have an opened ssh session),
I'm doing "echo usbport > trigger", then I can see the "ports"
directory, but it is empty.

Any ideas on the possible reasons? Exynos4412-trats2 uses
Ethernet Gadget (with CDC Ethernet support) USB Gadget Driver
for USB networking.

--
Best regards,
Jacek Anaszewski

2016-12-08 10:43:36

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [BUG 4.9] New led trigger usbport gets the kernel to panic

On Thu, Dec 08, 2016 at 09:40:43AM +0100, Jacek Anaszewski wrote:
> Hi Rafał,
>
> On 12/06/2016 06:29 PM, Rafał Miłecki wrote:
> > On 6 December 2016 at 18:26, Pavel Machek <[email protected]> wrote:
> > > On Fri 2016-12-02 09:48:18, Ralph Sennhauser wrote:
> > > > On Thu, 1 Dec 2016 17:56:07 +0100
> > > > Rafał Miłecki <[email protected]> wrote:
> > > >
> > > > > 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.
> > > >
> > > > Hi Rafał
> > > >
> > > > I just gave 2) led_set_brightness_nosleep a try. The kernel no longer
> > > > crashes and the led does work as expected.
> > >
> > > Do you have any patch that needs to be applied?
> >
> > Yes, it has been pushed:
> > https://git.kernel.org/cgit/linux/kernel/git/gregkh/usb.git/commit/?h=usb-next&id=89778ba335e302a450932ce5b703c1ee6216e949
> >
> > When sending it I didn't Cc linux-leds as get_maintainer.pl didn't
> > pick it. Should we add MAINTAINERS entry for ledtrig-usbport.c maybe?
> >
>
> I haven't tested this trigger earlier, probably due to the fact that
> it was targeted for usb subsystem, sorry about that. Nonetheless,
> I've just tried it on exynos4412-trats2 board and it seems not to
> work properly.
>
> I have the board connected through USB (I have an opened ssh session),
> I'm doing "echo usbport > trigger", then I can see the "ports"
> directory, but it is empty.
>
> Any ideas on the possible reasons? Exynos4412-trats2 uses
> Ethernet Gadget (with CDC Ethernet support) USB Gadget Driver
> for USB networking.

This trigger is for when your device is a USB host, not a gadget, so I
don't think it works on your hardware :)

thanks,

greg k-h

2016-12-08 11:38:19

by Rafał Miłecki

[permalink] [raw]
Subject: Re: [BUG 4.9] New led trigger usbport gets the kernel to panic

On 8 December 2016 at 09:40, Jacek Anaszewski <[email protected]> wrote:
> On 12/06/2016 06:29 PM, Rafał Miłecki wrote:
>>
>> On 6 December 2016 at 18:26, Pavel Machek <[email protected]> wrote:
>>>
>>> On Fri 2016-12-02 09:48:18, Ralph Sennhauser wrote:
>>>>
>>>> On Thu, 1 Dec 2016 17:56:07 +0100
>>>> Rafał Miłecki <[email protected]> wrote:
>>>>
>>>>> 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.
>>>>
>>>>
>>>> Hi Rafał
>>>>
>>>> I just gave 2) led_set_brightness_nosleep a try. The kernel no longer
>>>> crashes and the led does work as expected.
>>>
>>>
>>> Do you have any patch that needs to be applied?
>>
>>
>> Yes, it has been pushed:
>>
>> https://git.kernel.org/cgit/linux/kernel/git/gregkh/usb.git/commit/?h=usb-next&id=89778ba335e302a450932ce5b703c1ee6216e949
>>
>> When sending it I didn't Cc linux-leds as get_maintainer.pl didn't
>> pick it. Should we add MAINTAINERS entry for ledtrig-usbport.c maybe?
>>
>
> I haven't tested this trigger earlier, probably due to the fact that
> it was targeted for usb subsystem, sorry about that. Nonetheless,
> I've just tried it on exynos4412-trats2 board and it seems not to
> work properly.
>
> I have the board connected through USB (I have an opened ssh session),
> I'm doing "echo usbport > trigger", then I can see the "ports"
> directory, but it is empty.
>
> Any ideas on the possible reasons? Exynos4412-trats2 uses
> Ethernet Gadget (with CDC Ethernet support) USB Gadget Driver
> for USB networking.

If board has any USB ports you shouldn't have empty directory. What about:
ls /sys/bus/usb/devices/*/*-port* | grep ":"

--
Rafał

2016-12-08 13:47:06

by Jacek Anaszewski

[permalink] [raw]
Subject: Re: [BUG 4.9] New led trigger usbport gets the kernel to panic

On 12/08/2016 12:38 PM, Rafał Miłecki wrote:
> On 8 December 2016 at 09:40, Jacek Anaszewski <[email protected]> wrote:
>> On 12/06/2016 06:29 PM, Rafał Miłecki wrote:
>>>
>>> On 6 December 2016 at 18:26, Pavel Machek <[email protected]> wrote:
>>>>
>>>> On Fri 2016-12-02 09:48:18, Ralph Sennhauser wrote:
>>>>>
>>>>> On Thu, 1 Dec 2016 17:56:07 +0100
>>>>> Rafał Miłecki <[email protected]> wrote:
>>>>>
>>>>>> 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.
>>>>>
>>>>>
>>>>> Hi Rafał
>>>>>
>>>>> I just gave 2) led_set_brightness_nosleep a try. The kernel no longer
>>>>> crashes and the led does work as expected.
>>>>
>>>>
>>>> Do you have any patch that needs to be applied?
>>>
>>>
>>> Yes, it has been pushed:
>>>
>>> https://git.kernel.org/cgit/linux/kernel/git/gregkh/usb.git/commit/?h=usb-next&id=89778ba335e302a450932ce5b703c1ee6216e949
>>>
>>> When sending it I didn't Cc linux-leds as get_maintainer.pl didn't
>>> pick it. Should we add MAINTAINERS entry for ledtrig-usbport.c maybe?
>>>
>>
>> I haven't tested this trigger earlier, probably due to the fact that
>> it was targeted for usb subsystem, sorry about that. Nonetheless,
>> I've just tried it on exynos4412-trats2 board and it seems not to
>> work properly.
>>
>> I have the board connected through USB (I have an opened ssh session),
>> I'm doing "echo usbport > trigger", then I can see the "ports"
>> directory, but it is empty.
>>
>> Any ideas on the possible reasons? Exynos4412-trats2 uses
>> Ethernet Gadget (with CDC Ethernet support) USB Gadget Driver
>> for USB networking.
>
> If board has any USB ports you shouldn't have empty directory. What about:
> ls /sys/bus/usb/devices/*/*-port* | grep ":"
>

As Greg noticed my device in this configuration is not a USB host, but
gadget, and its /sys/bus/usb/devices/ dir is empty, so it was false
alarm.

--
Best regards,
Jacek Anaszewski