Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

System freeze when sudo modprobe i2c_bcm2708 #776

Closed
jacqu opened this issue Jan 31, 2015 · 25 comments
Closed

System freeze when sudo modprobe i2c_bcm2708 #776

jacqu opened this issue Jan 31, 2015 · 25 comments

Comments

@jacqu
Copy link

jacqu commented Jan 31, 2015

Bug appeared since last update (January 30th 2015). Not seen before.

I verified that the bug is not in:
pi@raspberrypi ~ $ uname -r
3.12.28+

Reproducible on various RPIs:

pi@raspberrypi ~ $ sudo rmmod i2c_bcm2708
pi@raspberrypi ~ $ sudo modprobe i2c_bcm2708
--> System freeze.

I2c has been configured at 400kHz:
pi@raspberrypi ~ $ cat /etc/modprobe.d/i2c.conf
options i2c_bcm2708 baudrate=400000

pi@raspberrypi ~ $ uname -a
Linux raspberrypi 3.12.35+ #730 PREEMPT Fri Dec 19 18:31:24 GMT 2014 armv6l GNU/Linux

pi@raspberrypi ~ $ dpkg -s libc6 | grep ^Version
Version: 2.13-38+rpi2+deb7u7

@popcornmix
Copy link
Collaborator

I'm not seeing this:

pi@raspberrypi:~ $ uname -a
Linux raspberrypi 3.18.5+ #744 PREEMPT Fri Jan 30 18:19:07 GMT 2015 armv6l GNU/Linux
pi@raspberrypi:~ $ sudo rmmod i2c_bcm2708
Error: Module i2c_bcm2708 is not currently loaded
pi@raspberrypi:~ $ sudo modprobe i2c_bcm2708
pi@raspberrypi:~ $ lsmod
Module                  Size  Used by
i2c_bcm2708             6004  0 
joydev                  9766  0 
evdev                  11000  3 
uio_pdrv_genirq         3666  0 
uio                     9897  1 uio_pdrv_genirq

Any non-standard settings in cmdline.txt or config.txt?

@pelwell
Copy link
Contributor

pelwell commented Jan 31, 2015

You don't show a full load/unload/load cycle - have you tested that?

@popcornmix
Copy link
Collaborator

Works for me

pi@raspberrypi:~ $ sudo modprobe i2c_bcm2708
pi@raspberrypi:~ $ sudo rmmod i2c_bcm2708
pi@raspberrypi:~ $ sudo modprobe i2c_bcm2708
pi@raspberrypi:~ $ sudo rmmod i2c_bcm2708
pi@raspberrypi:~ $ lsmod
Module                  Size  Used by
joydev                  9766  0 
evdev                  11000  3 
uio_pdrv_genirq         3666  0 
uio                     9897  1 uio_pdrv_genirq
pi@raspberrypi:~ $ sudo modprobe i2c_bcm2708
pi@raspberrypi:~ $ lsmod
Module                  Size  Used by
i2c_bcm2708             6004  0 
joydev                  9766  0 
evdev                  11000  3 
uio_pdrv_genirq         3666  0 
uio                     9897  1 uio_pdrv_genirq

@jacqu
Copy link
Author

jacqu commented Jan 31, 2015

Note that I can reproduce this bug on 3 different RPIs. I have a backup SD card with an older version of the kernel, 3.12.28+ instead of 3.12.35+. And with the same RPI, the SD with the older kernel doesn't have this bug.

On my system, the i2c module are loaded at boot time. For that, you have to issue these commands:

echo i2c-bcm2708 >> /etc/modules
echo i2c-dev >> /etc/modules

Also remove the blacklisting of the i2c-bcm2708 module:

pi@raspberrypi /etc/modprobe.d $ cat raspi-blacklist.conf 
blacklist spi-bcm2708
#blacklist i2c-bcm2708
blacklist snd_soc_bcm2708_i2s

Furthermore I run i2c at full speed (400kHz). To do so:

echo options i2c_bcm2708 baudrate=400000 > /etc/modprobe.d/i2c.conf

If I have some time, I will try a diff between the 3.12.28+ and 3.12.35+ source code of the i2c-bcm2708 driver.

@jacqu
Copy link
Author

jacqu commented Jan 31, 2015

FYI, this is the last modification of the drivers/i2c/busses/i2c-bcm2708.c file (tweaking the spinlocks may trigger system freeze):

@@ -265,10 +265,11 @@ static int bcm2708_i2c_master_xfer(struct i2c_adapter *adap,
bi->nmsgs = num;
bi->error = false;
- spin_unlock_irqrestore(&bi->lock, flags);
-
bcm2708_bsc_setup(bi);
+ /* unlockig _after_ the setup to avoid races with the interrupt routine */
+ spin_unlock_irqrestore(&bi->lock, flags);
+
ret = wait_for_completion_timeout(&bi->done,
msecs_to_jiffies(I2C_TIMEOUT_MS));
if (ret == 0) {

@jacqu
Copy link
Author

jacqu commented Feb 2, 2015

Good news: with today's kernel update, the problem disappeared .

root@raspberrypi:/etc/init.d# uname -a
Linux raspberrypi 3.18.5+ #744 PREEMPT Fri Jan 30 18:19:07 GMT 2015 armv6l GNU/Linux

@popcornmix
Copy link
Collaborator

Cool. There was another kernel update today - can you check if that is still okay.

@mw46d
Copy link

mw46d commented Feb 5, 2015

Somewhat related:-( It looks like I lost my i2c bus completely with the upgrade to 3.18.5+:-(
root@issabove:/boot# uname -a
Linux issabove 3.18.5+ #744 PREEMPT Fri Jan 30 18:19:07 GMT 2015 armv6l GNU/Linux
root@issabove:/boot# i2cdetect -l
root@issabove:/boot#
that worked before the upgrade:-(

The i2c modules are loaded:
root@issabove:/boot# lsmod | fgrep i2c
i2c_bcm2708 6004 0
i2c_dev 6709 0

But the spi- module does not want to load:-(
root@issabove:/boot# modprobe -v spi-bcm2708
insmod /lib/modules/3.18.5+/kernel/drivers/spi/spi-bcm2708.ko
ERROR: could not insert 'spi_bcm2708': No such device

My last working kernel was:
Linux version 3.12.29+ #714 PREEMPT Wed Oct 1 23:11:38 BST 2014

@mw46d
Copy link

mw46d commented Feb 5, 2015

same problem still with
pi@issabove ~ $ uname -a
Linux issabove 3.18.5+ #748 PREEMPT Wed Feb 4 21:24:41 GMT 2015 armv6l GNU/Linux

@pelwell
Copy link
Contributor

pelwell commented Feb 5, 2015

You need to add:

dtparam=i2c_arm=on,spi=on

to /boot/config.txt, and reboot. If you've done an apt-get upgrade then your new raspi-config can do that for you.

See: http://www.raspberrypi.org/forums/viewtopic.php?f=28&t=97314&p=676583

@jacqu
Copy link
Author

jacqu commented Feb 5, 2015

I did what pelwell suggests. And below is the result of rmmoding then modprobing of the i2c driver.

Furthermore, I specified in /etc/modprobe.d/i2c.conf that I want i2c to run at 400kHz:

echo options i2c_bcm2708 baudrate=400000 > /etc/modprobe.d/i2c.conf

But if you read the end of dmesg ouptut, you see that this option is not taken into account:

BSC1 Controller at 0x20804000 (irq 79) (baudrate 100000)

This used to work before.

[  250.316413] ------------[ cut here ]------------
[  250.316483] WARNING: CPU: 0 PID: 3178 at drivers/clk/clk.c:850 __clk_disable+0x68/0x74()
[  250.316498] Modules linked in: ipt_MASQUERADE nf_nat_masquerade_ipv4 iptable_nat nf_nat_ipv4 nf_nat nf_conntrack_ipv4 nf_defrag_ipv4 xt_state nf_conntrack iptable_filter ip_tables x_tables rfcomm bnep bluetooth rfkill i2c_dev snd_bcm2835 snd_pcm snd_seq snd_seq_device snd_timer snd ch341 8192cu i2c_bcm2708(-) spi_bcm2708 usbserial uio_pdrv_genirq uio
[  250.316642] CPU: 0 PID: 3178 Comm: rmmod Not tainted 3.18.5+ #744
[  250.316727] [<c00151fc>] (unwind_backtrace) from [<c0012710>] (show_stack+0x20/0x24)
[  250.316770] [<c0012710>] (show_stack) from [<c05553e0>] (dump_stack+0x20/0x28)
[  250.316804] [<c05553e0>] (dump_stack) from [<c0022e7c>] (warn_slowpath_common+0x7c/0x9c)
[  250.316828] [<c0022e7c>] (warn_slowpath_common) from [<c0022f58>] (warn_slowpath_null+0x2c/0x34)
[  250.316855] [<c0022f58>] (warn_slowpath_null) from [<c045dc28>] (__clk_disable+0x68/0x74)
[  250.316881] [<c045dc28>] (__clk_disable) from [<c045dd54>] (clk_disable+0x34/0x40)
[  250.316922] [<c045dd54>] (clk_disable) from [<bf01e064>] (bcm2708_i2c_remove+0x44/0x5c [i2c_bcm2708])
[  250.316977] [<bf01e064>] (bcm2708_i2c_remove [i2c_bcm2708]) from [<c03671a4>] (platform_drv_remove+0x28/0x40)
[  250.317009] [<c03671a4>] (platform_drv_remove) from [<c0364e5c>] (__device_release_driver+0x68/0xbc)
[  250.317032] [<c0364e5c>] (__device_release_driver) from [<c0365690>] (driver_detach+0x120/0x124)
[  250.317054] [<c0365690>] (driver_detach) from [<c0364b78>] (bus_remove_driver+0x5c/0xb0)
[  250.317075] [<c0364b78>] (bus_remove_driver) from [<c0365f58>] (driver_unregister+0x38/0x58)
[  250.317097] [<c0365f58>] (driver_unregister) from [<c036728c>] (platform_driver_unregister+0x1c/0x20)
[  250.317132] [<c036728c>] (platform_driver_unregister) from [<bf01ec8c>] (bcm2708_i2c_exit+0x14/0x1c [i2c_bcm2708])
[  250.317181] [<bf01ec8c>] (bcm2708_i2c_exit [i2c_bcm2708]) from [<c00877ac>] (SyS_delete_module+0x140/0x1c0)
[  250.317212] [<c00877ac>] (SyS_delete_module) from [<c000e8c0>] (ret_fast_syscall+0x0/0x48)
[  250.317225] ---[ end trace 2664190f8ad9f09b ]---
[  257.054305] bcm2708_i2c_init_pinmode(1,2)
[  257.054341] bcm2708_i2c_init_pinmode(1,3)
[  257.059459] bcm2708_i2c 20804000.i2c: BSC1 Controller at 0x20804000 (irq 79) (baudrate 100000)

@pelwell
Copy link
Contributor

pelwell commented Feb 5, 2015

Yesterday's update brought some new parameters. What you want is:

dtparam=i2c_arm=on,i2c_arm_baudrate=400000

It's all in /boot/overlays/README.

@jacqu
Copy link
Author

jacqu commented Feb 5, 2015

I rebooted with the parameters you suggested. But with no luck.

pi@raspberrypi ~ $ dmesg|grep baudrate
[    7.163477] bcm2708_i2c 20804000.i2c: BSC1 Controller at 0x20804000 (irq 79) (baudrate 100000)
pi@raspberrypi ~ $ cat /boot/config.txt
# uncomment if you get no picture on HDMI for a default "safe" mode
#hdmi_safe=1

# uncomment this if your display has a black border of unused pixels visible
# and your display can output without overscan
#disable_overscan=1

# uncomment the following to adjust overscan. Use positive numbers if console
# goes off screen, and negative if there is too much border
#overscan_left=16
#overscan_right=16
#overscan_top=16
#overscan_bottom=16

# uncomment to force a console size. By default it will be display's size minus
# overscan.
#framebuffer_width=1280
#framebuffer_height=720

# uncomment if hdmi display is not detected and composite is being output
#hdmi_force_hotplug=1

# uncomment to force a specific HDMI mode (this will force VGA)
#hdmi_group=1
#hdmi_mode=1

# uncomment to force a HDMI mode rather than DVI. This can make audio work in
# DMT (computer monitor) modes
#hdmi_drive=2

# uncomment to increase signal to HDMI, if you have interference, blanking, or
# no display
#config_hdmi_boost=4

# uncomment for composite PAL
#sdtv_mode=2

#uncomment to overclock the arm. 700 MHz is the default.
arm_freq=900

# for more options see http://elinux.org/RPi_config.txt
start_x=1
gpu_mem=128
core_freq=250
sdram_freq=450
over_voltage=2
dtparam=spi=on
dtparam=i2c_arm=on,i2c_arm_baudrate=400000

@jacqu
Copy link
Author

jacqu commented Feb 5, 2015

BTW, the kernel warning is deterministic on rmmoding the i2c driver.

@pelwell
Copy link
Contributor

pelwell commented Feb 5, 2015

For those options to work, you need the latest firmware. If you haven't updated in the last 24 hours, do so. If you have, try this:

strings /boot/bcm2708-rpi-b-plus.dtb | grep baudrate
strings /boot/start.elf | grep baudrate

If the board type is different, or you are using a different loader (perhaps start_x) then feel free to change the filenames above, but all ought to be kept in sync.

You're right about the warning - I'll check that out.

@jacqu
Copy link
Author

jacqu commented Feb 5, 2015

If have upgraded 1 hour ago (apt-get update;apt-get upgrade).
I still think I'm lagging behind the latest version because:

pi@raspberrypi ~ $ uname -a
Linux raspberrypi 3.18.5+ #744 PREEMPT Fri Jan 30 18:19:07 GMT 2015 armv6l GNU/Linux

And just above, mw64d has:

pi@issabove ~ $ uname -a
Linux issabove 3.18.5+ #748 PREEMPT Wed Feb 4 21:24:41 GMT 2015 armv6l GNU/Linux

The results of your commands, plus the same one with start_x.elf is blank.

@pelwell
Copy link
Contributor

pelwell commented Feb 5, 2015

Ah, yes, you need to rpi-update to get the latest release. The Raspbian (apt-get upgrade) releases change less frequently, but I would anticipate a point release fairly soon.

@jacqu
Copy link
Author

jacqu commented Feb 5, 2015

The 400kHz param is now working. Thanks.

pi@raspberrypi ~ $ uname -a
Linux raspberrypi 3.18.5+ #748 PREEMPT Wed Feb 4 21:24:41 GMT 2015 armv6l GNU/Linux
pi@raspberrypi ~ $ dmesg|grep baudrate
[    6.437895] bcm2708_i2c 20804000.i2c: BSC1 Controller at 0x20804000 (irq 79) (baudrate 400000)
pi@raspberrypi ~ $ sudo rmmod i2c_bcm2708
pi@raspberrypi ~ $ dmesg
[...][  185.840180] ------------[ cut here ]------------
[  185.840251] WARNING: CPU: 0 PID: 3170 at drivers/clk/clk.c:850 __clk_disable+0x68/0x74()
[  185.840266] Modules linked in: ipt_MASQUERADE nf_nat_masquerade_ipv4 iptable_nat nf_nat_ipv4 nf_nat nf_conntrack_ipv4 nf_defrag_ipv4 xt_state nf_conntrack iptable_filter ip_tables x_tables bnep rfcomm bluetooth rfkill i2c_dev snd_bcm2835 snd_pcm snd_seq snd_seq_device snd_timer snd ch341 8192cu i2c_bcm2708(-) spi_bcm2708 usbserial uio_pdrv_genirq uio
[  185.840412] CPU: 0 PID: 3170 Comm: rmmod Not tainted 3.18.5+ #748
[  185.840479] [<c00151fc>] (unwind_backtrace) from [<c0012710>] (show_stack+0x20/0x24)
[  185.840519] [<c0012710>] (show_stack) from [<c05556d8>] (dump_stack+0x20/0x28)
[  185.840559] [<c05556d8>] (dump_stack) from [<c0022e7c>] (warn_slowpath_common+0x7c/0x9c)
[  185.840590] [<c0022e7c>] (warn_slowpath_common) from [<c0022f58>] (warn_slowpath_null+0x2c/0x34)
[  185.840623] [<c0022f58>] (warn_slowpath_null) from [<c045df20>] (__clk_disable+0x68/0x74)
[  185.840652] [<c045df20>] (__clk_disable) from [<c045e04c>] (clk_disable+0x34/0x40)
[  185.840699] [<c045e04c>] (clk_disable) from [<bf01e064>] (bcm2708_i2c_remove+0x44/0x5c [i2c_bcm2708])
[  185.840763] [<bf01e064>] (bcm2708_i2c_remove [i2c_bcm2708]) from [<c03671f0>] (platform_drv_remove+0x28/0x40)
[  185.840797] [<c03671f0>] (platform_drv_remove) from [<c0364ea8>] (__device_release_driver+0x68/0xbc)
[  185.840824] [<c0364ea8>] (__device_release_driver) from [<c03656dc>] (driver_detach+0x120/0x124)
[  185.840853] [<c03656dc>] (driver_detach) from [<c0364bc4>] (bus_remove_driver+0x5c/0xb0)
[  185.840880] [<c0364bc4>] (bus_remove_driver) from [<c0365fa4>] (driver_unregister+0x38/0x58)
[  185.840908] [<c0365fa4>] (driver_unregister) from [<c03672d8>] (platform_driver_unregister+0x1c/0x20)
[  185.840947] [<c03672d8>] (platform_driver_unregister) from [<bf01ec8c>] (bcm2708_i2c_exit+0x14/0x1c [i2c_bcm2708])
[  185.841004] [<bf01ec8c>] (bcm2708_i2c_exit [i2c_bcm2708]) from [<c00877ac>] (SyS_delete_module+0x140/0x1c0)
[  185.841040] [<c00877ac>] (SyS_delete_module) from [<c000e8c0>] (ret_fast_syscall+0x0/0x48)
[  185.841057] ---[ end trace 20ef05c1697a217c ]---

@pelwell
Copy link
Contributor

pelwell commented Feb 5, 2015

bec4f0a should have fixed it.

@mw46d
Copy link

mw46d commented Feb 5, 2015

OK, my PiGlow is working again as well;-)

I tried to search/google for my problem but that forum post did not show up:-( And since I started with the normal `apt-get update && apt-get upgrade', I did not expect the boot parameters to change:-(

Thanks,
mw46d

@pelwell
Copy link
Contributor

pelwell commented Feb 5, 2015

We realised some time ago that we should enable Device Tree by default. If our DT usage was going to be in the true spirit of DT, that meant abandoning the "always on" semantics of the old board support code; this necessarily meant that some configuration changes would be needed after the update. After some debate we decidedit was better to require some user intervention after this update rather than attempt to change things automatically behind the scenes, because it is both safer and educational.

@pelwell
Copy link
Contributor

pelwell commented Mar 9, 2015

Can we close this now?

@mw46d
Copy link

mw46d commented Mar 9, 2015

yes, thanks.

@jacqu
Copy link
Author

jacqu commented Mar 10, 2015

It didn't show up in kernels compiled after the 5th February. Many thanks for your reactivity !

@pelwell
Copy link
Contributor

pelwell commented Mar 10, 2015

Thanks for the clear reports.

@pelwell pelwell closed this as completed Mar 10, 2015
pfpacket pushed a commit to pfpacket/linux-rpi-rust that referenced this issue Apr 7, 2023
Remove panicking stub for `__mulodi4`
popcornmix pushed a commit that referenced this issue Oct 28, 2024
During the migration of Soundwire runtime stream allocation from
the Qualcomm Soundwire controller to SoC's soundcard drivers the sdm845
soundcard was forgotten.

At this point any playback attempt or audio daemon startup, for instance
on sdm845-db845c (Qualcomm RB3 board), will result in stream pointer
NULL dereference:

 Unable to handle kernel NULL pointer dereference at virtual
 address 0000000000000020
 Mem abort info:
   ESR = 0x0000000096000004
   EC = 0x25: DABT (current EL), IL = 32 bits
   SET = 0, FnV = 0
   EA = 0, S1PTW = 0
   FSC = 0x04: level 0 translation fault
 Data abort info:
   ISV = 0, ISS = 0x00000004, ISS2 = 0x00000000
   CM = 0, WnR = 0, TnD = 0, TagAccess = 0
   GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0
 user pgtable: 4k pages, 48-bit VAs, pgdp=0000000101ecf000
 [0000000000000020] pgd=0000000000000000, p4d=0000000000000000
 Internal error: Oops: 0000000096000004 [#1] PREEMPT SMP
 Modules linked in: ...
 CPU: 5 UID: 0 PID: 1198 Comm: aplay
 Not tainted 6.12.0-rc2-qcomlt-arm64-00059-g9d78f315a362-dirty #18
 Hardware name: Thundercomm Dragonboard 845c (DT)
 pstate: 60400005 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
 pc : sdw_stream_add_slave+0x44/0x380 [soundwire_bus]
 lr : sdw_stream_add_slave+0x44/0x380 [soundwire_bus]
 sp : ffff80008a2035c0
 x29: ffff80008a2035c0 x28: ffff80008a203978 x27: 0000000000000000
 x26: 00000000000000c0 x25: 0000000000000000 x24: ffff1676025f4800
 x23: ffff167600ff1cb8 x22: ffff167600ff1c98 x21: 0000000000000003
 x20: ffff167607316000 x19: ffff167604e64e80 x18: 0000000000000000
 x17: 0000000000000000 x16: ffffcec265074160 x15: 0000000000000000
 x14: 0000000000000000 x13: 0000000000000000 x12: 0000000000000000
 x11: 0000000000000000 x10: 0000000000000000 x9 : 0000000000000000
 x8 : 0000000000000000 x7 : 0000000000000000 x6 : ffff167600ff1cec
 x5 : ffffcec22cfa2010 x4 : 0000000000000000 x3 : 0000000000000003
 x2 : ffff167613f836c0 x1 : 0000000000000000 x0 : ffff16761feb60b8
 Call trace:
  sdw_stream_add_slave+0x44/0x380 [soundwire_bus]
  wsa881x_hw_params+0x68/0x80 [snd_soc_wsa881x]
  snd_soc_dai_hw_params+0x3c/0xa4
  __soc_pcm_hw_params+0x230/0x660
  dpcm_be_dai_hw_params+0x1d0/0x3f8
  dpcm_fe_dai_hw_params+0x98/0x268
  snd_pcm_hw_params+0x124/0x460
  snd_pcm_common_ioctl+0x998/0x16e8
  snd_pcm_ioctl+0x34/0x58
  __arm64_sys_ioctl+0xac/0xf8
  invoke_syscall+0x48/0x104
  el0_svc_common.constprop.0+0x40/0xe0
  do_el0_svc+0x1c/0x28
  el0_svc+0x34/0xe0
  el0t_64_sync_handler+0x120/0x12c
  el0t_64_sync+0x190/0x194
 Code: aa0403fb f9418400 9100e000 9400102f (f8420f22)
 ---[ end trace 0000000000000000 ]---

0000000000006108 <sdw_stream_add_slave>:
    6108:       d503233f        paciasp
    610c:       a9b97bfd        stp     x29, x30, [sp, #-112]!
    6110:       910003fd        mov     x29, sp
    6114:       a90153f3        stp     x19, x20, [sp, #16]
    6118:       a9025bf5        stp     x21, x22, [sp, #32]
    611c:       aa0103f6        mov     x22, x1
    6120:       2a0303f5        mov     w21, w3
    6124:       a90363f7        stp     x23, x24, [sp, #48]
    6128:       aa0003f8        mov     x24, x0
    612c:       aa0203f7        mov     x23, x2
    6130:       a9046bf9        stp     x25, x26, [sp, #64]
    6134:       aa0403f9        mov     x25, x4        <-- x4 copied to x25
    6138:       a90573fb        stp     x27, x28, [sp, #80]
    613c:       aa0403fb        mov     x27, x4
    6140:       f9418400        ldr     x0, [x0, #776]
    6144:       9100e000        add     x0, x0, #0x38
    6148:       94000000        bl      0 <mutex_lock>
    614c:       f8420f22        ldr     x2, [x25, #32]!  <-- offset 0x44
    ^^^
This is 0x6108 + offset 0x44 from the beginning of sdw_stream_add_slave()
where data abort happens.
wsa881x_hw_params() is called with stream = NULL and passes it further
in register x4 (5th argument) to sdw_stream_add_slave() without any checks.
Value from x4 is copied to x25 and finally it aborts on trying to load
a value from address in x25 plus offset 32 (in dec) which corresponds
to master_list member in struct sdw_stream_runtime:

struct sdw_stream_runtime {
        const char  *              name;	/*     0     8 */
        struct sdw_stream_params   params;	/*     8    12 */
        enum sdw_stream_state      state;	/*    20     4 */
        enum sdw_stream_type       type;	/*    24     4 */
        /* XXX 4 bytes hole, try to pack */
 here-> struct list_head           master_list;	/*    32    16 */
        int                        m_rt_count;	/*    48     4 */
        /* size: 56, cachelines: 1, members: 6 */
        /* sum members: 48, holes: 1, sum holes: 4 */
        /* padding: 4 */
        /* last cacheline: 56 bytes */

Fix this by adding required calls to qcom_snd_sdw_startup() and
sdw_release_stream() to startup and shutdown routines which restores
the previous correct behaviour when ->set_stream() method is called to
set a valid stream runtime pointer on playback startup.

Reproduced and then fix was tested on db845c RB3 board.

Reported-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Cc: stable@vger.kernel.org
Fixes: 15c7fab ("ASoC: qcom: Move Soundwire runtime stream alloc to soundcards")
Cc: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
Cc: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Cc: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Cc: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Signed-off-by: Alexey Klimov <alexey.klimov@linaro.org>
Tested-by: Steev Klimaszewski <steev@kali.org> # Lenovo Yoga C630
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Reviewed-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
Link: https://patch.msgid.link/20241009213922.999355-1-alexey.klimov@linaro.org
Signed-off-by: Mark Brown <broonie@kernel.org>
popcornmix pushed a commit that referenced this issue Nov 1, 2024
commit d0e806b upstream.

During the migration of Soundwire runtime stream allocation from
the Qualcomm Soundwire controller to SoC's soundcard drivers the sdm845
soundcard was forgotten.

At this point any playback attempt or audio daemon startup, for instance
on sdm845-db845c (Qualcomm RB3 board), will result in stream pointer
NULL dereference:

 Unable to handle kernel NULL pointer dereference at virtual
 address 0000000000000020
 Mem abort info:
   ESR = 0x0000000096000004
   EC = 0x25: DABT (current EL), IL = 32 bits
   SET = 0, FnV = 0
   EA = 0, S1PTW = 0
   FSC = 0x04: level 0 translation fault
 Data abort info:
   ISV = 0, ISS = 0x00000004, ISS2 = 0x00000000
   CM = 0, WnR = 0, TnD = 0, TagAccess = 0
   GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0
 user pgtable: 4k pages, 48-bit VAs, pgdp=0000000101ecf000
 [0000000000000020] pgd=0000000000000000, p4d=0000000000000000
 Internal error: Oops: 0000000096000004 [#1] PREEMPT SMP
 Modules linked in: ...
 CPU: 5 UID: 0 PID: 1198 Comm: aplay
 Not tainted 6.12.0-rc2-qcomlt-arm64-00059-g9d78f315a362-dirty #18
 Hardware name: Thundercomm Dragonboard 845c (DT)
 pstate: 60400005 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
 pc : sdw_stream_add_slave+0x44/0x380 [soundwire_bus]
 lr : sdw_stream_add_slave+0x44/0x380 [soundwire_bus]
 sp : ffff80008a2035c0
 x29: ffff80008a2035c0 x28: ffff80008a203978 x27: 0000000000000000
 x26: 00000000000000c0 x25: 0000000000000000 x24: ffff1676025f4800
 x23: ffff167600ff1cb8 x22: ffff167600ff1c98 x21: 0000000000000003
 x20: ffff167607316000 x19: ffff167604e64e80 x18: 0000000000000000
 x17: 0000000000000000 x16: ffffcec265074160 x15: 0000000000000000
 x14: 0000000000000000 x13: 0000000000000000 x12: 0000000000000000
 x11: 0000000000000000 x10: 0000000000000000 x9 : 0000000000000000
 x8 : 0000000000000000 x7 : 0000000000000000 x6 : ffff167600ff1cec
 x5 : ffffcec22cfa2010 x4 : 0000000000000000 x3 : 0000000000000003
 x2 : ffff167613f836c0 x1 : 0000000000000000 x0 : ffff16761feb60b8
 Call trace:
  sdw_stream_add_slave+0x44/0x380 [soundwire_bus]
  wsa881x_hw_params+0x68/0x80 [snd_soc_wsa881x]
  snd_soc_dai_hw_params+0x3c/0xa4
  __soc_pcm_hw_params+0x230/0x660
  dpcm_be_dai_hw_params+0x1d0/0x3f8
  dpcm_fe_dai_hw_params+0x98/0x268
  snd_pcm_hw_params+0x124/0x460
  snd_pcm_common_ioctl+0x998/0x16e8
  snd_pcm_ioctl+0x34/0x58
  __arm64_sys_ioctl+0xac/0xf8
  invoke_syscall+0x48/0x104
  el0_svc_common.constprop.0+0x40/0xe0
  do_el0_svc+0x1c/0x28
  el0_svc+0x34/0xe0
  el0t_64_sync_handler+0x120/0x12c
  el0t_64_sync+0x190/0x194
 Code: aa0403fb f9418400 9100e000 9400102f (f8420f22)
 ---[ end trace 0000000000000000 ]---

0000000000006108 <sdw_stream_add_slave>:
    6108:       d503233f        paciasp
    610c:       a9b97bfd        stp     x29, x30, [sp, #-112]!
    6110:       910003fd        mov     x29, sp
    6114:       a90153f3        stp     x19, x20, [sp, #16]
    6118:       a9025bf5        stp     x21, x22, [sp, #32]
    611c:       aa0103f6        mov     x22, x1
    6120:       2a0303f5        mov     w21, w3
    6124:       a90363f7        stp     x23, x24, [sp, #48]
    6128:       aa0003f8        mov     x24, x0
    612c:       aa0203f7        mov     x23, x2
    6130:       a9046bf9        stp     x25, x26, [sp, #64]
    6134:       aa0403f9        mov     x25, x4        <-- x4 copied to x25
    6138:       a90573fb        stp     x27, x28, [sp, #80]
    613c:       aa0403fb        mov     x27, x4
    6140:       f9418400        ldr     x0, [x0, #776]
    6144:       9100e000        add     x0, x0, #0x38
    6148:       94000000        bl      0 <mutex_lock>
    614c:       f8420f22        ldr     x2, [x25, #32]!  <-- offset 0x44
    ^^^
This is 0x6108 + offset 0x44 from the beginning of sdw_stream_add_slave()
where data abort happens.
wsa881x_hw_params() is called with stream = NULL and passes it further
in register x4 (5th argument) to sdw_stream_add_slave() without any checks.
Value from x4 is copied to x25 and finally it aborts on trying to load
a value from address in x25 plus offset 32 (in dec) which corresponds
to master_list member in struct sdw_stream_runtime:

struct sdw_stream_runtime {
        const char  *              name;	/*     0     8 */
        struct sdw_stream_params   params;	/*     8    12 */
        enum sdw_stream_state      state;	/*    20     4 */
        enum sdw_stream_type       type;	/*    24     4 */
        /* XXX 4 bytes hole, try to pack */
 here-> struct list_head           master_list;	/*    32    16 */
        int                        m_rt_count;	/*    48     4 */
        /* size: 56, cachelines: 1, members: 6 */
        /* sum members: 48, holes: 1, sum holes: 4 */
        /* padding: 4 */
        /* last cacheline: 56 bytes */

Fix this by adding required calls to qcom_snd_sdw_startup() and
sdw_release_stream() to startup and shutdown routines which restores
the previous correct behaviour when ->set_stream() method is called to
set a valid stream runtime pointer on playback startup.

Reproduced and then fix was tested on db845c RB3 board.

Reported-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Cc: stable@vger.kernel.org
Fixes: 15c7fab ("ASoC: qcom: Move Soundwire runtime stream alloc to soundcards")
Cc: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
Cc: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Cc: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Cc: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Signed-off-by: Alexey Klimov <alexey.klimov@linaro.org>
Tested-by: Steev Klimaszewski <steev@kali.org> # Lenovo Yoga C630
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Reviewed-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
Link: https://patch.msgid.link/20241009213922.999355-1-alexey.klimov@linaro.org
Signed-off-by: Mark Brown <broonie@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants