| Age | Commit message (Collapse) | Author |
|---|
|
Now that sdw_slave_sysfs_init() only calls sdw_slave_sysfs_dpn_init(),
just do that instead and remove sdw_slave_sysfs_init() to get it out of
the way to save a bit of logic and code size.
Cc: Vinod Koul <vkoul@kernel.org>
Cc: Bard Liao <yung-chuan.liao@linux.intel.com>
Cc: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Cc: Sanyog Kale <sanyog.r.kale@intel.com>
Cc: alsa-devel@alsa-project.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Reviewed-by: Dan Williams <dan.j.williams@intel.com>
Tested-By: Vijendar Mukunda <Vijendar.Mukunda@amd.com>
Link: https://lore.kernel.org/r/2024013030-denatured-swaddling-b047@gregkh
Signed-off-by: Vinod Koul <vkoul@kernel.org>
|
|
The driver core supports the ability to handle the creation and removal
of device-specific sysfs files in a race-free manner. Take advantage of
that by converting this driver to use this by moving the sysfs
attributes into a group and assigning the dev_groups pointer to it.
Cc: Vinod Koul <vkoul@kernel.org>
Cc: Bard Liao <yung-chuan.liao@linux.intel.com>
Cc: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Cc: Sanyog Kale <sanyog.r.kale@intel.com>
Cc: alsa-devel@alsa-project.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Reviewed-by: Dan Williams <dan.j.williams@intel.com>
Tested-By: Vijendar Mukunda <Vijendar.Mukunda@amd.com>
Link: https://lore.kernel.org/r/2024013030-worsening-rocket-a3cb@gregkh
Signed-off-by: Vinod Koul <vkoul@kernel.org>
|
|
Since commit d492cc2573a0 ("driver core: device.h: make struct
bus_type a const *"), the driver core can properly handle constant
struct bus_type, move the sdw_bus_type variable to be a constant
structure as well, placing it into read-only memory which can not be
modified at runtime.
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Suggested-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: "Ricardo B. Marliere" <ricardo@marliere.net>
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Link: https://lore.kernel.org/r/20240213-bus_cleanup-soundwire-v1-1-3878b00f6f57@marliere.net
Signed-off-by: Vinod Koul <vkoul@kernel.org>
|
|
SoundWire has provisions for a simple callback for the IRQ handling so
has no hard dependency on IRQ_DOMAIN, but the recent addition of IRQ
handling was causing builds without IRQ_DOMAIN to fail. Resolve this by
moving the IRQ handling into its own file and only add it to the build
when IRQ_DOMAIN is included in the kernel.
Fixes: 12a95123bfe1 ("soundwire: bus: Allow SoundWire peripherals to register IRQ handlers")
Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202309150522.MoKeF4jx-lkp@intel.com/
Acked-by: Randy Dunlap <rdunlap@infradead.org>
Tested-by: Randy Dunlap <rdunlap@infradead.org>
Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com>
Link: https://lore.kernel.org/r/20230920160401.854052-1-ckeepax@opensource.cirrus.com
Signed-off-by: Vinod Koul <vkoul@kernel.org>
|
|
Currently the in-band alerts for SoundWire peripherals can only
be communicated to the driver through the interrupt_callback
function. This however is slightly inconvenient for devices that wish
to share IRQ handling code between SoundWire and I2C/SPI, the later
would normally register an IRQ handler with the IRQ subsystem. However
there is no reason the SoundWire in-band IRQs can not also be
communicated as an actual IRQ to the driver.
Add support for SoundWire peripherals to register a normal IRQ
handler to receive SoundWire in-band alerts, allowing code to be
shared across control buses. Note that we allow users to use both the
interrupt_callback and the IRQ handler, this is useful for devices
which must clear additional chip specific SoundWire registers that are
not a part of the normal IRQ flow, or the SoundWire specification.
Signed-off-by: Lucas Tanure <tanureal@opensource.cirrus.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Acked-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Acked-by: Vinod Koul <vkoul@kernel.org>
Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com>
Link: https://lore.kernel.org/r/20230804104602.395892-2-ckeepax@opensource.cirrus.com
Signed-off-by: Lee Jones <lee@kernel.org>
|
|
git://git.kernel.org/pub/scm/linux/kernel/git/vkoul/soundwire
Pull soundwire updates from Vinod Koul:
"This is a small update which features a bit of core changes and driver
updates in Intel and cadence driver.
Core:
- sdw_transfer_defer() API change to drop an argument
- Reset page address rework
- Export sdw_nwrite_no_pm and sdw_nread_no_pm APIs
Drivers:
- Cadence and related intel driver updates for FIFO handling and low
level msg transfers"
* tag 'soundwire-6.3-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/vkoul/soundwire:
soundwire: cadence: further simplify low-level xfer_msg_defer() callback
soundwire: cadence: use directly bus sdw_defer structure
soundwire: bus: remove sdw_defer argument in sdw_transfer_defer()
soundwire: stream: use consistent pattern for freeing buffers
soundwire: bus: Remove unused reset_page_addr() callback
soundwire: bus: Don't zero page registers after every transaction
soundwire: bus_type: Avoid lockdep assert in sdw_drv_probe()
soundwire: stream: Move remaining register accesses over to no_pm
soundwire: debugfs: Switch to sdw_read_no_pm
soundwire: Provide build stubs for common functions
soundwire: bus: export sdw_nwrite_no_pm and sdw_nread_no_pm functions
soundwire: cadence: remove unused sdw_cdns_master_ops declaration
soundwire: enable optional clock registers for SoundWire 1.2 devices
ASoC/soundwire: remove is_sdca boolean property
soundwire: cadence: Drain the RX FIFO after an IO timeout
soundwire: cadence: Remove wasted space in response_buf
soundwire: cadence: Don't overflow the command FIFOs
soundwire: intel: remove DAI startup/shutdown
|
|
Don't hold sdw_dev_lock while calling the peripheral driver
probe() and remove() callbacks.
Holding sdw_dev_lock around the probe() and remove() calls causes
a theoretical mutex inversion which lockdep will assert on.
During probe() the sdw_dev_lock mutex is taken first and then
ASoC/ALSA locks are taken by the probe() implementation.
During normal operation ASoC can take its locks and then trigger
a runtime resume of the component. The SoundWire resume will then
take sdw_dev_lock. This is the reverse order compared to probe().
It's not necessary to hold sdw_dev_lock when calling the probe()
and remove(), it is only used to prevent the bus core calling the
driver callbacks if there isn't a driver or the driver is removing.
All calls to the driver callbacks are guarded by the 'probed' flag.
So if sdw_dev_lock is held while setting and clearing the 'probed'
flag this is sufficient to guarantee the safety of callback
functions.
Removing the mutex from around the call to probe() means that it
is now possible for a bus event (PING response) to be handled in
parallel with the probe(). But sdw_bus_probe() already has
handling for this by calling the device update_status() after
the probe() has completed.
Example lockdep assert:
[ 46.098514] ======================================================
[ 46.104736] WARNING: possible circular locking dependency detected
[ 46.110961] 6.1.0-rc4-jamerson #1 Tainted: G E
[ 46.116842] ------------------------------------------------------
[ 46.123063] mpg123/1130 is trying to acquire lock:
[ 46.127883] ffff8b445031fb80 (&slave->sdw_dev_lock){+.+.}-{3:3}, at: sdw_update_slave_status+0x26/0x70
[ 46.137225]
but task is already holding lock:
[ 46.143074] ffffffffc1455310 (&card->pcm_mutex){+.+.}-{3:3}, at: dpcm_fe_dai_open+0x49/0x830
[ 46.151536]
which lock already depends on the new lock.[ 46.159732]
the existing dependency chain (in reverse order) is:
[ 46.167231]
-> #4 (&card->pcm_mutex){+.+.}-{3:3}:
[ 46.173428] __mutex_lock+0x94/0x920
[ 46.177542] snd_soc_dpcm_runtime_update+0x2e/0x100
[ 46.182958] snd_soc_dapm_put_enum_double+0x1c2/0x200
[ 46.188548] snd_ctl_elem_write+0x10c/0x1d0
[ 46.193268] snd_ctl_ioctl+0x126/0x850
[ 46.197556] __x64_sys_ioctl+0x87/0xc0
[ 46.201845] do_syscall_64+0x38/0x90
[ 46.205959] entry_SYSCALL_64_after_hwframe+0x63/0xcd
[ 46.211553]
-> #3 (&card->controls_rwsem){++++}-{3:3}:
[ 46.218188] down_write+0x2b/0xd0
[ 46.222038] snd_ctl_add_replace+0x39/0xb0
[ 46.226672] snd_soc_add_controls+0x53/0x80
[ 46.231393] soc_probe_component+0x1e4/0x2a0
[ 46.236202] snd_soc_bind_card+0x51a/0xc80
[ 46.240836] devm_snd_soc_register_card+0x43/0x90
[ 46.246079] mc_probe+0x982/0xfe0 [snd_soc_sof_sdw]
[ 46.251500] platform_probe+0x3c/0xa0
[ 46.255700] really_probe+0xde/0x390
[ 46.259814] __driver_probe_device+0x78/0x180
[ 46.264710] driver_probe_device+0x1e/0x90
[ 46.269347] __driver_attach+0x9f/0x1f0
[ 46.273721] bus_for_each_dev+0x78/0xc0
[ 46.278098] bus_add_driver+0x1ac/0x200
[ 46.282473] driver_register+0x8f/0xf0
[ 46.286759] do_one_initcall+0x58/0x310
[ 46.291136] do_init_module+0x4c/0x1f0
[ 46.295422] __do_sys_finit_module+0xb4/0x130
[ 46.300321] do_syscall_64+0x38/0x90
[ 46.304434] entry_SYSCALL_64_after_hwframe+0x63/0xcd
[ 46.310027]
-> #2 (&card->mutex){+.+.}-{3:3}:
[ 46.315883] __mutex_lock+0x94/0x920
[ 46.320000] snd_soc_bind_card+0x3e/0xc80
[ 46.324551] devm_snd_soc_register_card+0x43/0x90
[ 46.329798] mc_probe+0x982/0xfe0 [snd_soc_sof_sdw]
[ 46.335219] platform_probe+0x3c/0xa0
[ 46.339420] really_probe+0xde/0x390
[ 46.343532] __driver_probe_device+0x78/0x180
[ 46.348430] driver_probe_device+0x1e/0x90
[ 46.353065] __driver_attach+0x9f/0x1f0
[ 46.357437] bus_for_each_dev+0x78/0xc0
[ 46.361812] bus_add_driver+0x1ac/0x200
[ 46.366716] driver_register+0x8f/0xf0
[ 46.371528] do_one_initcall+0x58/0x310
[ 46.376424] do_init_module+0x4c/0x1f0
[ 46.381239] __do_sys_finit_module+0xb4/0x130
[ 46.386665] do_syscall_64+0x38/0x90
[ 46.391299] entry_SYSCALL_64_after_hwframe+0x63/0xcd
[ 46.397416]
-> #1 (client_mutex){+.+.}-{3:3}:
[ 46.404307] __mutex_lock+0x94/0x920
[ 46.408941] snd_soc_add_component+0x24/0x2c0
[ 46.414345] devm_snd_soc_register_component+0x54/0xa0
[ 46.420522] cs35l56_common_probe+0x280/0x370 [snd_soc_cs35l56]
[ 46.427487] cs35l56_sdw_probe+0xf4/0x170 [snd_soc_cs35l56_sdw]
[ 46.434442] sdw_drv_probe+0x80/0x1a0
[ 46.439136] really_probe+0xde/0x390
[ 46.443738] __driver_probe_device+0x78/0x180
[ 46.449120] driver_probe_device+0x1e/0x90
[ 46.454247] __driver_attach+0x9f/0x1f0
[ 46.459106] bus_for_each_dev+0x78/0xc0
[ 46.463971] bus_add_driver+0x1ac/0x200
[ 46.468825] driver_register+0x8f/0xf0
[ 46.473592] do_one_initcall+0x58/0x310
[ 46.478441] do_init_module+0x4c/0x1f0
[ 46.483202] __do_sys_finit_module+0xb4/0x130
[ 46.488572] do_syscall_64+0x38/0x90
[ 46.493158] entry_SYSCALL_64_after_hwframe+0x63/0xcd
[ 46.499229]
-> #0 (&slave->sdw_dev_lock){+.+.}-{3:3}:
[ 46.506737] __lock_acquire+0x1121/0x1df0
[ 46.511765] lock_acquire+0xd5/0x300
[ 46.516360] __mutex_lock+0x94/0x920
[ 46.520949] sdw_update_slave_status+0x26/0x70
[ 46.526409] sdw_clear_slave_status+0xd8/0xe0
[ 46.531783] intel_resume_runtime+0x139/0x2a0
[ 46.537155] __rpm_callback+0x41/0x120
[ 46.541919] rpm_callback+0x5d/0x70
[ 46.546422] rpm_resume+0x531/0x7e0
[ 46.550920] __pm_runtime_resume+0x4a/0x80
[ 46.556024] snd_soc_pcm_component_pm_runtime_get+0x2f/0xc0
[ 46.562611] __soc_pcm_open+0x62/0x520
[ 46.567375] dpcm_be_dai_startup+0x116/0x210
[ 46.572661] dpcm_fe_dai_open+0xf7/0x830
[ 46.577597] snd_pcm_open_substream+0x54a/0x8b0
[ 46.583145] snd_pcm_open.part.0+0xdc/0x200
[ 46.588341] snd_pcm_playback_open+0x51/0x80
[ 46.593625] chrdev_open+0xc0/0x250
[ 46.598129] do_dentry_open+0x15f/0x430
[ 46.602981] path_openat+0x75e/0xa80
[ 46.607575] do_filp_open+0xb2/0x160
[ 46.612162] do_sys_openat2+0x9a/0x160
[ 46.616922] __x64_sys_openat+0x53/0xa0
[ 46.621767] do_syscall_64+0x38/0x90
[ 46.626352] entry_SYSCALL_64_after_hwframe+0x63/0xcd
[ 46.632414]
other info that might help us debug this:[ 46.641862] Chain exists of:
&slave->sdw_dev_lock --> &card->controls_rwsem --> &card->pcm_mutex[ 46.655145] Possible unsafe locking scenario:[ 46.662048] CPU0 CPU1
[ 46.667080] ---- ----
[ 46.672108] lock(&card->pcm_mutex);
[ 46.676267] lock(&card->controls_rwsem);
[ 46.683382] lock(&card->pcm_mutex);
[ 46.690063] lock(&slave->sdw_dev_lock);
[ 46.694574]
*** DEADLOCK ***[ 46.701942] 2 locks held by mpg123/1130:
[ 46.706356] #0: ffff8b4457b22b90 (&pcm->open_mutex){+.+.}-{3:3}, at: snd_pcm_open.part.0+0xc9/0x200
[ 46.715999] #1: ffffffffc1455310 (&card->pcm_mutex){+.+.}-{3:3}, at: dpcm_fe_dai_open+0x49/0x830
[ 46.725390]
stack backtrace:
[ 46.730752] CPU: 0 PID: 1130 Comm: mpg123 Tainted: G E 6.1.0-rc4-jamerson #1
[ 46.739703] Hardware name: AAEON UP-WHL01/UP-WHL01, BIOS UPW1AM19 11/10/2020
[ 46.747270] Call Trace:
[ 46.750239] <TASK>
[ 46.752857] dump_stack_lvl+0x56/0x73
[ 46.757045] check_noncircular+0x102/0x120
[ 46.761664] __lock_acquire+0x1121/0x1df0
[ 46.766197] lock_acquire+0xd5/0x300
[ 46.770292] ? sdw_update_slave_status+0x26/0x70
[ 46.775432] ? lock_is_held_type+0xe2/0x140
[ 46.780143] __mutex_lock+0x94/0x920
[ 46.784241] ? sdw_update_slave_status+0x26/0x70
[ 46.789387] ? find_held_lock+0x2b/0x80
[ 46.793750] ? sdw_update_slave_status+0x26/0x70
[ 46.798894] ? lock_release+0x147/0x2f0
[ 46.803262] ? lockdep_init_map_type+0x47/0x250
[ 46.808315] ? sdw_update_slave_status+0x26/0x70
[ 46.813456] sdw_update_slave_status+0x26/0x70
[ 46.818422] sdw_clear_slave_status+0xd8/0xe0
[ 46.823302] ? pm_generic_runtime_suspend+0x30/0x30
[ 46.828706] intel_resume_runtime+0x139/0x2a0
[ 46.833583] ? _raw_spin_unlock_irq+0x24/0x50
[ 46.838462] ? pm_generic_runtime_suspend+0x30/0x30
[ 46.843866] __rpm_callback+0x41/0x120
[ 46.848142] ? pm_generic_runtime_suspend+0x30/0x30
[ 46.853550] rpm_callback+0x5d/0x70
[ 46.857568] rpm_resume+0x531/0x7e0
[ 46.861578] ? _raw_spin_lock_irqsave+0x62/0x70
[ 46.866634] __pm_runtime_resume+0x4a/0x80
[ 46.871258] snd_soc_pcm_component_pm_runtime_get+0x2f/0xc0
[ 46.877358] __soc_pcm_open+0x62/0x520
[ 46.881634] ? dpcm_add_paths.isra.0+0x35d/0x4c0
[ 46.886784] dpcm_be_dai_startup+0x116/0x210
[ 46.891592] dpcm_fe_dai_open+0xf7/0x830
[ 46.896046] ? debug_mutex_init+0x33/0x50
[ 46.900591] snd_pcm_open_substream+0x54a/0x8b0
[ 46.905658] snd_pcm_open.part.0+0xdc/0x200
[ 46.910376] ? wake_up_q+0x90/0x90
[ 46.914312] snd_pcm_playback_open+0x51/0x80
[ 46.919118] chrdev_open+0xc0/0x250
[ 46.923147] ? cdev_device_add+0x90/0x90
[ 46.927608] do_dentry_open+0x15f/0x430
[ 46.931976] path_openat+0x75e/0xa80
[ 46.936086] do_filp_open+0xb2/0x160
[ 46.940194] ? lock_release+0x147/0x2f0
[ 46.944563] ? _raw_spin_unlock+0x29/0x50
[ 46.949101] do_sys_openat2+0x9a/0x160
[ 46.953377] __x64_sys_openat+0x53/0xa0
[ 46.957733] do_syscall_64+0x38/0x90
[ 46.961829] entry_SYSCALL_64_after_hwframe+0x63/0xcd
[ 46.967402] RIP: 0033:0x7fa6397ccd3b
[ 46.971506] Code: 25 00 00 41 00 3d 00 00 41 00 74 4b 64 8b 04 25 18 00 00 00 85 c0 75 67 44 89 e2 48 89 ee bf 9c ff ff ff b8 01 01 00 00 0f 05 <48> 3d 00 f0 ff ff 0f 87 91 00 00 00 48 8b 4c 24 28 64 48 33 0c 25
[ 46.991413] RSP: 002b:00007fff838e8990 EFLAGS: 00000246 ORIG_RAX: 0000000000000101
[ 46.999580] RAX: ffffffffffffffda RBX: 0000000000080802 RCX: 00007fa6397ccd3b
[ 47.007311] RDX: 0000000000080802 RSI: 00007fff838e8b50 RDI: 00000000ffffff9c
[ 47.015047] RBP: 00007fff838e8b50 R08: 0000000000000000 R09: 0000000000000011
[ 47.022787] R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000080802
[ 47.030539] R13: 0000000000000004 R14: 0000000000000000 R15: 00007fff838e8b50
[ 47.038289] </TASK>
Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Link: https://lore.kernel.org/r/20230123172520.339367-1-rf@opensource.cirrus.com
Signed-off-by: Vinod Koul <vkoul@kernel.org>
|
|
The uevent() callback in struct device_type should not be modifying the
device that is passed into it, so mark it as a const * and propagate the
function signature changes out into all relevant subsystems that use
this callback.
Cc: Alan Stern <stern@rowland.harvard.edu>
Cc: Andreas Noever <andreas.noever@gmail.com>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Bard Liao <yung-chuan.liao@linux.intel.com>
Cc: Chaitanya Kulkarni <kch@nvidia.com>
Cc: Dan Williams <dan.j.williams@intel.com>
Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: Frank Rowand <frowand.list@gmail.com>
Cc: Ira Weiny <ira.weiny@intel.com>
Cc: Jason Gunthorpe <jgg@ziepe.ca>
Cc: Jens Axboe <axboe@kernel.dk>
Cc: Jilin Yuan <yuanjilin@cdjrlc.com>
Cc: Jiri Slaby <jirislaby@kernel.org>
Cc: Len Brown <lenb@kernel.org>
Cc: Mark Gross <markgross@kernel.org>
Cc: "Martin K. Petersen" <martin.petersen@oracle.com>
Cc: "Matthew Wilcox (Oracle)" <willy@infradead.org>
Cc: Maximilian Luz <luzmaximilian@gmail.com>
Cc: Michael Jamet <michael.jamet@intel.com>
Cc: Ming Lei <ming.lei@redhat.com>
Cc: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
Cc: Sanyog Kale <sanyog.r.kale@intel.com>
Cc: Sean Young <sean@mess.org>
Cc: Stefan Richter <stefanr@s5r6.in-berlin.de>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Won Chung <wonchung@google.com>
Cc: Yehezkel Bernat <YehezkelShB@gmail.com>
Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Acked-by: Mika Westerberg <mika.westerberg@linux.intel.com> # for Thunderbolt
Acked-by: Mauro Carvalho Chehab <mchehab@kernel.org>
Acked-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
Acked-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Acked-by: Wolfram Sang <wsa@kernel.org>
Acked-by: Vinod Koul <vkoul@kernel.org>
Acked-by: Hans de Goede <hdegoede@redhat.com>
Link: https://lore.kernel.org/r/20230111113018.459199-6-gregkh@linuxfoundation.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
|
In the SoundWire probe, we store a pointer from the driver ops into
the 'slave' structure. This can lead to kernel oopses when unbinding
codec drivers, e.g. with the following sequence to remove machine
driver and codec driver.
/sbin/modprobe -r snd_soc_sof_sdw
/sbin/modprobe -r snd_soc_rt711
The full details can be found in the BugLink below, for reference the
two following examples show different cases of driver ops/callbacks
being invoked after the driver .remove().
kernel: BUG: kernel NULL pointer dereference, address: 0000000000000150
kernel: Workqueue: events cdns_update_slave_status_work [soundwire_cadence]
kernel: RIP: 0010:mutex_lock+0x19/0x30
kernel: Call Trace:
kernel: ? sdw_handle_slave_status+0x426/0xe00 [soundwire_bus 94ff184bf398570c3f8ff7efe9e32529f532e4ae]
kernel: ? newidle_balance+0x26a/0x400
kernel: ? cdns_update_slave_status_work+0x1e9/0x200 [soundwire_cadence 1bcf98eebe5ba9833cd433323769ac923c9c6f82]
kernel: BUG: unable to handle page fault for address: ffffffffc07654c8
kernel: Workqueue: pm pm_runtime_work
kernel: RIP: 0010:sdw_bus_prep_clk_stop+0x6f/0x160 [soundwire_bus]
kernel: Call Trace:
kernel: <TASK>
kernel: sdw_cdns_clock_stop+0xb5/0x1b0 [soundwire_cadence 1bcf98eebe5ba9833cd433323769ac923c9c6f82]
kernel: intel_suspend_runtime+0x5f/0x120 [soundwire_intel aca858f7c87048d3152a4a41bb68abb9b663a1dd]
kernel: ? dpm_sysfs_remove+0x60/0x60
This was not detected earlier in Intel tests since the tests first
remove the parent PCI device and shut down the bus. The sequence
above is a corner case which keeps the bus operational but without a
driver bound.
While trying to solve this kernel oopses, it became clear that the
existing SoundWire bus does not deal well with the unbind case.
Commit 528be501b7d4a ("soundwire: sdw_slave: add probe_complete structure and new fields")
added a 'probed' status variable and a 'probe_complete'
struct completion. This status is however not reset on remove and
likewise the 'probe complete' is not re-initialized, so the
bind/unbind/bind test cases would fail. The timeout used before the
'update_status' callback was also a bad idea in hindsight, there
should really be no timing assumption as to if and when a driver is
bound to a device.
An initial draft was based on device_lock() and device_unlock() was
tested. This proved too complicated, with deadlocks created during the
suspend-resume sequences, which also use the same device_lock/unlock()
as the bind/unbind sequences. On a CometLake device, a bad DSDT/BIOS
caused spurious resumes and the use of device_lock() caused hangs
during suspend. After multiple weeks or testing and painful
reverse-engineering of deadlocks on different devices, we looked for
alternatives that did not interfere with the device core.
A bus notifier was used successfully to keep track of DRIVER_BOUND and
DRIVER_UNBIND events. This solved the bind-unbind-bind case in tests,
but it can still be defeated with a theoretical corner case where the
memory is freed by a .remove while the callback is in use. The
notifier only helps make sure the driver callbacks are valid, but not
that the memory allocated in probe remains valid while the callbacks
are invoked.
This patch suggests the introduction of a new 'sdw_dev_lock' mutex
protecting probe/remove and all driver callbacks. Since this mutex is
'local' to SoundWire only, it does not interfere with existing locks
and does not create deadlocks. In addition, this patch removes the
'probe_complete' completion, instead we directly invoke the
'update_status' from the probe routine. That removes any sort of
timing dependency and a much better support for the device/driver
model, the driver could be bound before the bus started, or eons after
the bus started and the hardware would be properly initialized in all
cases.
BugLink: https://github.com/thesofproject/linux/issues/3531
Fixes: 56d4fe31af77 ("soundwire: Add MIPI DisCo property helpers")
Fixes: 528be501b7d4a ("soundwire: sdw_slave: add probe_complete structure and new fields")
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Reviewed-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Reviewed-by: Péter Ujfalusi <peter.ujfalusi@linux.intel.com>
Link: https://lore.kernel.org/r/20220621225641.221170-2-pierre-louis.bossart@linux.intel.com
Signed-off-by: Vinod Koul <vkoul@kernel.org>
|
|
The bus sdw_drv_remove() and sdw_drv_shutdown() helpers are used
conditionally, if the driver provides these routines.
These helpers already test if the driver provides a .remove or
.shutdown callback, so there's no harm in invoking the
sdw_drv_remove() and sdw_drv_shutdown() unconditionally.
In addition, the current code is imbalanced with
dev_pm_domain_attach() called from sdw_drv_probe(), but
dev_pm_domain_detach() called from sdw_drv_remove() only if the driver
provides a .remove callback.
Fixes: 9251345dca24b ("soundwire: Add SoundWire bus type")
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@intel.com>
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Link: https://lore.kernel.org/r/20220610015105.25987-1-yung-chuan.liao@linux.intel.com
Signed-off-by: Vinod Koul <vkoul@kernel.org>
|
|
None of the existing codec drivers set the sdw_driver.name, but
instead set sdw_driver.driver.name.
This leads to error messages such as
[ 23.935355] rt700 sdw:2:25d:700:0: Probe of (null) failed: -19
We could remove this sdw_driver.name if it doesn't have any
purpose. This patch only suggests using the proper indirection.
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Guennadi Liakhovetski <guennadi.liakhovetski@intel.com>
Reviewed-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Link: https://lore.kernel.org/r/20210302091122.13952-2-yung-chuan.liao@linux.intel.com
Signed-off-by: Vinod Koul <vkoul@kernel.org>
|
|
Currently Slave devices are only added on startup, either from Device
Tree or ACPI entries. However Slave devices that are physically
present on the bus, but not described in platform firmware, will never
be added to the device list. The user/integrator can only know the
list of devices by looking a dynamic debug logs.
This patch suggests adding a Slave device even if there is no matching
DT or ACPI entry, so that we can see this in sysfs entry.
Initial code from Srinivas. Comments, fixes for ACPI probe and edits
of commit message by Pierre.
Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
Co-developed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Link: https://lore.kernel.org/r/20200924194430.121058-2-pierre-louis.bossart@linux.intel.com
Signed-off-by: Vinod Koul <vkoul@kernel.org>
|
|
The SoundWire 1.2 specification adds new capabilities that were not
present in previous version, such as the class ID.
To enable support for class drivers, and well as drivers that address
a specific version, all fields of the sdw_device_id structure need to
be exposed. For SoundWire 1.0 and 1.1 devices, a wildcard is used so
class and version information are ignored.
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@linux.intel.com>
Reviewed-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Link: https://lore.kernel.org/r/20200608205436.2402-4-yung-chuan.liao@linux.intel.com
Signed-off-by: Vinod Koul <vkoul@kernel.org>
|
|
No functionality change, just more structured code.
Suggested-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@linux.intel.com>
Reviewed-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Link: https://lore.kernel.org/r/20200608205436.2402-3-yung-chuan.liao@linux.intel.com
Signed-off-by: Vinod Koul <vkoul@kernel.org>
|
|
Expose MIPI DisCo Slave properties in sysfs.
For Slave properties and Data Port 0, the attributes are managed with
simple devm_ support.
A Slave Device may have more than one Data Port (DPN), and each Data
Port can be sink or source. The attributes are created dynamically
using pre-canned macros, but still use devm_ with a name attribute
group to avoid creating kobjects - as requested by GregKH. In the
_show function, we use container_of() to retrieve port number and
direction required to extract the information.
Audio modes are not supported for now. Depending on the discussions
the SoundWire Device Class, we may add it later as is or follow the
new specification.
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Link: https://lore.kernel.org/r/20200518203551.2053-4-yung-chuan.liao@linux.intel.com
Signed-off-by: Vinod Koul <vkoul@kernel.org>
|
|
this is a preparatory patch before the introduction of the
sdw_master_type. The SoundWire slave support is slightly modified with
the use of a sdw_slave_type, and the uevent handling move to
slave.c (since it's not necessary for the master).
No functionality change other than moving code around.
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Acked-by: Jaroslav Kysela <perex@perex.cz>
Link: https://lore.kernel.org/r/20200518174322.31561-3-yung-chuan.liao@linux.intel.com
Signed-off-by: Vinod Koul <vkoul@kernel.org>
|
|
The driver probe takes care of basic initialization and is invoked
when a Slave becomes attached, after a match between the Slave DevID
registers and ACPI/DT entries.
The update_status callback is invoked when a Slave state changes,
e.g. when it is assigned a non-zero Device Number and it reports with
an ATTACHED/ALERT state.
The state change detection is usually hardware-based and based on the
SoundWire frame rate (e.g. double-digit microseconds) while the probe
is a pure software operation, which may involve a kernel module
load. In corner cases, it's possible that the state changes before the
probe completes.
This patch suggests the use of wait_for_completion to avoid races on
startup, so that the update_status callback does not rely on invalid
pointers/data structures.
Signed-off-by: Rander Wang <rander.wang@intel.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Link: https://lore.kernel.org/r/20200115000844.14695-2-pierre-louis.bossart@linux.intel.com
Signed-off-by: Vinod Koul <vkoul@kernel.org>
|
|
Add base debugfs mechanism for SoundWire bus by creating soundwire
root and master-N and slave-x hierarchy.
Also add SDW Slave SCP, DP0 and DP-N register debug file.
Registers not implemented will print as "XX"
Credits: this patch is based on an earlier internal contribution by
Vinod Koul, Sanyog Kale, Shreyas Nc and Hardik Shah.
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Acked-by: Sanyog Kale <sanyog.r.kale@intel.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Link: https://lore.kernel.org/r/20190821185821.12690-2-pierre-louis.bossart@linux.intel.com
Signed-off-by: Vinod Koul <vkoul@kernel.org>
|
|
Use Linux style
Reviewed-by: Takashi Iwai <tiwai@suse.de>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
|
The limitation of being able to check only for -EPROBE_DEFER from
dev_pm_domain_attach() has been removed. Hence let's respect all error
codes and bail out accordingly.
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
Acked-by: Vinod Koul <vkoul@kernel.org>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
|
|
MIPI Discovery And Configuration (DisCo) Specification for SoundWire
specifies properties to be implemented for SoundWire Masters and
Slaves. The DisCo spec doesn't mandate these properties. However,
SDW bus cannot work without knowing these values.
The helper functions read the Master and Slave properties.
Implementers of Master or Slave drivers can use any of the below
three mechanisms:
a) Use these APIs here as .read_prop() callback for Master
and Slave
b) Implement own methods and set those as .read_prop(), but invoke
APIs in this file for generic read and override the values with
platform specific data
c) Implement ones own methods which do not use anything provided
here
Signed-off-by: Sanyog Kale <sanyog.r.kale@intel.com>
Reviewed-by: Philippe Ombredanne <pombredanne@nexb.com>
Acked-By: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Takashi Iwai <tiwai@suse.de>
Signed-off-by: Vinod Koul <vinod.koul@intel.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
|
This adds the base SoundWire bus type, bus and driver registration.
along with changes to module device table for new SoundWire
device type.
Signed-off-by: Sanyog Kale <sanyog.r.kale@intel.com>
Reviewed-by: Philippe Ombredanne <pombredanne@nexb.com>
Acked-By: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Takashi Iwai <tiwai@suse.de>
Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Vinod Koul <vinod.koul@intel.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
|
