* [PATCH] Revert "soundwire: qcom: Add set_channel_map api support"
@ 2025-07-09 17:49 Amit Pundir
2025-07-10 10:35 ` Srinivas Kandagatla
2025-07-15 14:54 ` Vinod Koul
0 siblings, 2 replies; 3+ messages in thread
From: Amit Pundir @ 2025-07-09 17:49 UTC (permalink / raw)
To: Vinod Koul, Mark Brown, Mohammad Rafi Shaik, Srinivas Kandagatla,
Bard Liao, Pierre-Louis Bossart, Sanyog Kale, Dan Carpenter,
Yongqin Liu, Jie Gan
Cc: linux-arm-msm, linux-sound, lkml, stable
This reverts commit 7796c97df6b1b2206681a07f3c80f6023a6593d5.
This patch broke Dragonboard 845c (sdm845). I see:
Unexpected kernel BRK exception at EL1
Internal error: BRK handler: 00000000f20003e8 [#1] SMP
pc : qcom_swrm_set_channel_map+0x7c/0x80 [soundwire_qcom]
lr : snd_soc_dai_set_channel_map+0x34/0x78
Call trace:
qcom_swrm_set_channel_map+0x7c/0x80 [soundwire_qcom] (P)
sdm845_dai_init+0x18c/0x2e0 [snd_soc_sdm845]
snd_soc_link_init+0x28/0x6c
snd_soc_bind_card+0x5f4/0xb0c
snd_soc_register_card+0x148/0x1a4
devm_snd_soc_register_card+0x50/0xb0
sdm845_snd_platform_probe+0x124/0x148 [snd_soc_sdm845]
platform_probe+0x6c/0xd0
really_probe+0xc0/0x2a4
__driver_probe_device+0x7c/0x130
driver_probe_device+0x40/0x118
__device_attach_driver+0xc4/0x108
bus_for_each_drv+0x8c/0xf0
__device_attach+0xa4/0x198
device_initial_probe+0x18/0x28
bus_probe_device+0xb8/0xbc
deferred_probe_work_func+0xac/0xfc
process_one_work+0x244/0x658
worker_thread+0x1b4/0x360
kthread+0x148/0x228
ret_from_fork+0x10/0x20
Kernel panic - not syncing: BRK handler: Fatal exception
Dan has also reported following issues with the original patch
https://lore.kernel.org/all/33fe8fe7-719a-405a-9ed2-d9f816ce1d57@sabinyo.mountain/
Bug #1:
The zeroeth element of ctrl->pconfig[] is supposed to be unused. We
start counting at 1. However this code sets ctrl->pconfig[0].ch_mask = 128.
Bug #2:
There are SLIM_MAX_TX_PORTS (16) elements in tx_ch[] array but only
QCOM_SDW_MAX_PORTS + 1 (15) in the ctrl->pconfig[] array so it corrupts
memory like Yongqin Liu pointed out.
Bug 3:
Like Jie Gan pointed out, it erases all the tx information with the rx
information.
Cc: <stable@vger.kernel.org> # v6.15+
Signed-off-by: Amit Pundir <amit.pundir@linaro.org>
---
drivers/soundwire/qcom.c | 26 --------------------------
1 file changed, 26 deletions(-)
diff --git a/drivers/soundwire/qcom.c b/drivers/soundwire/qcom.c
index 295a46dc2be7..0f45e3404756 100644
--- a/drivers/soundwire/qcom.c
+++ b/drivers/soundwire/qcom.c
@@ -156,7 +156,6 @@ struct qcom_swrm_port_config {
u8 word_length;
u8 blk_group_count;
u8 lane_control;
- u8 ch_mask;
};
/*
@@ -1049,13 +1048,9 @@ static int qcom_swrm_port_enable(struct sdw_bus *bus,
{
u32 reg = SWRM_DP_PORT_CTRL_BANK(enable_ch->port_num, bank);
struct qcom_swrm_ctrl *ctrl = to_qcom_sdw(bus);
- struct qcom_swrm_port_config *pcfg;
u32 val;
- pcfg = &ctrl->pconfig[enable_ch->port_num];
ctrl->reg_read(ctrl, reg, &val);
- if (pcfg->ch_mask != SWR_INVALID_PARAM && pcfg->ch_mask != 0)
- enable_ch->ch_mask = pcfg->ch_mask;
if (enable_ch->enable)
val |= (enable_ch->ch_mask << SWRM_DP_PORT_CTRL_EN_CHAN_SHFT);
@@ -1275,26 +1270,6 @@ static void *qcom_swrm_get_sdw_stream(struct snd_soc_dai *dai, int direction)
return ctrl->sruntime[dai->id];
}
-static int qcom_swrm_set_channel_map(struct snd_soc_dai *dai,
- unsigned int tx_num, const unsigned int *tx_slot,
- unsigned int rx_num, const unsigned int *rx_slot)
-{
- struct qcom_swrm_ctrl *ctrl = dev_get_drvdata(dai->dev);
- int i;
-
- if (tx_slot) {
- for (i = 0; i < tx_num; i++)
- ctrl->pconfig[i].ch_mask = tx_slot[i];
- }
-
- if (rx_slot) {
- for (i = 0; i < rx_num; i++)
- ctrl->pconfig[i].ch_mask = rx_slot[i];
- }
-
- return 0;
-}
-
static int qcom_swrm_startup(struct snd_pcm_substream *substream,
struct snd_soc_dai *dai)
{
@@ -1331,7 +1306,6 @@ static const struct snd_soc_dai_ops qcom_swrm_pdm_dai_ops = {
.shutdown = qcom_swrm_shutdown,
.set_stream = qcom_swrm_set_sdw_stream,
.get_stream = qcom_swrm_get_sdw_stream,
- .set_channel_map = qcom_swrm_set_channel_map,
};
static const struct snd_soc_component_driver qcom_swrm_dai_component = {
--
2.43.0
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] Revert "soundwire: qcom: Add set_channel_map api support"
2025-07-09 17:49 [PATCH] Revert "soundwire: qcom: Add set_channel_map api support" Amit Pundir
@ 2025-07-10 10:35 ` Srinivas Kandagatla
2025-07-15 14:54 ` Vinod Koul
1 sibling, 0 replies; 3+ messages in thread
From: Srinivas Kandagatla @ 2025-07-10 10:35 UTC (permalink / raw)
To: Amit Pundir, Vinod Koul, Mark Brown, Mohammad Rafi Shaik,
Srinivas Kandagatla, Bard Liao, Pierre-Louis Bossart,
Sanyog Kale, Dan Carpenter, Yongqin Liu, Jie Gan
Cc: linux-arm-msm, linux-sound, lkml, stable
On 7/9/25 6:49 PM, Amit Pundir wrote:
> This reverts commit 7796c97df6b1b2206681a07f3c80f6023a6593d5.
>
> This patch broke Dragonboard 845c (sdm845). I see:
>
thanks Amit for sending this out,
> Unexpected kernel BRK exception at EL1
> Internal error: BRK handler: 00000000f20003e8 [#1] SMP
> pc : qcom_swrm_set_channel_map+0x7c/0x80 [soundwire_qcom]
> lr : snd_soc_dai_set_channel_map+0x34/0x78
> Call trace:
> qcom_swrm_set_channel_map+0x7c/0x80 [soundwire_qcom] (P)
> sdm845_dai_init+0x18c/0x2e0 [snd_soc_sdm845]
> snd_soc_link_init+0x28/0x6c
> snd_soc_bind_card+0x5f4/0xb0c
> snd_soc_register_card+0x148/0x1a4
> devm_snd_soc_register_card+0x50/0xb0
> sdm845_snd_platform_probe+0x124/0x148 [snd_soc_sdm845]
> platform_probe+0x6c/0xd0
> really_probe+0xc0/0x2a4
> __driver_probe_device+0x7c/0x130
> driver_probe_device+0x40/0x118
> __device_attach_driver+0xc4/0x108
> bus_for_each_drv+0x8c/0xf0
> __device_attach+0xa4/0x198
> device_initial_probe+0x18/0x28
> bus_probe_device+0xb8/0xbc
> deferred_probe_work_func+0xac/0xfc
> process_one_work+0x244/0x658
> worker_thread+0x1b4/0x360
> kthread+0x148/0x228
> ret_from_fork+0x10/0x20
> Kernel panic - not syncing: BRK handler: Fatal exception
>
> Dan has also reported following issues with the original patch
> https://lore.kernel.org/all/33fe8fe7-719a-405a-9ed2-d9f816ce1d57@sabinyo.mountain/
>
> Bug #1:
> The zeroeth element of ctrl->pconfig[] is supposed to be unused. We
> start counting at 1. However this code sets ctrl->pconfig[0].ch_mask = 128.
>
> Bug #2:
> There are SLIM_MAX_TX_PORTS (16) elements in tx_ch[] array but only
> QCOM_SDW_MAX_PORTS + 1 (15) in the ctrl->pconfig[] array so it corrupts
> memory like Yongqin Liu pointed out.
>
> Bug 3:
> Like Jie Gan pointed out, it erases all the tx information with the rx
> information.
>
> Cc: <stable@vger.kernel.org> # v6.15+
> Signed-off-by: Amit Pundir <amit.pundir@linaro.org>
> ---
Acked-by: Srinivas Kandagatla <srini@kernel.org>
--srini
> drivers/soundwire/qcom.c | 26 --------------------------
> 1 file changed, 26 deletions(-)
>
> diff --git a/drivers/soundwire/qcom.c b/drivers/soundwire/qcom.c
> index 295a46dc2be7..0f45e3404756 100644
> --- a/drivers/soundwire/qcom.c
> +++ b/drivers/soundwire/qcom.c
> @@ -156,7 +156,6 @@ struct qcom_swrm_port_config {
> u8 word_length;
> u8 blk_group_count;
> u8 lane_control;
> - u8 ch_mask;
> };
>
> /*
> @@ -1049,13 +1048,9 @@ static int qcom_swrm_port_enable(struct sdw_bus *bus,
> {
> u32 reg = SWRM_DP_PORT_CTRL_BANK(enable_ch->port_num, bank);
> struct qcom_swrm_ctrl *ctrl = to_qcom_sdw(bus);
> - struct qcom_swrm_port_config *pcfg;
> u32 val;
>
> - pcfg = &ctrl->pconfig[enable_ch->port_num];
> ctrl->reg_read(ctrl, reg, &val);
> - if (pcfg->ch_mask != SWR_INVALID_PARAM && pcfg->ch_mask != 0)
> - enable_ch->ch_mask = pcfg->ch_mask;
>
> if (enable_ch->enable)
> val |= (enable_ch->ch_mask << SWRM_DP_PORT_CTRL_EN_CHAN_SHFT);
> @@ -1275,26 +1270,6 @@ static void *qcom_swrm_get_sdw_stream(struct snd_soc_dai *dai, int direction)
> return ctrl->sruntime[dai->id];
> }
>
> -static int qcom_swrm_set_channel_map(struct snd_soc_dai *dai,
> - unsigned int tx_num, const unsigned int *tx_slot,
> - unsigned int rx_num, const unsigned int *rx_slot)
> -{
> - struct qcom_swrm_ctrl *ctrl = dev_get_drvdata(dai->dev);
> - int i;
> -
> - if (tx_slot) {
> - for (i = 0; i < tx_num; i++)
> - ctrl->pconfig[i].ch_mask = tx_slot[i];
> - }
> -
> - if (rx_slot) {
> - for (i = 0; i < rx_num; i++)
> - ctrl->pconfig[i].ch_mask = rx_slot[i];
> - }
> -
> - return 0;
> -}
> -
> static int qcom_swrm_startup(struct snd_pcm_substream *substream,
> struct snd_soc_dai *dai)
> {
> @@ -1331,7 +1306,6 @@ static const struct snd_soc_dai_ops qcom_swrm_pdm_dai_ops = {
> .shutdown = qcom_swrm_shutdown,
> .set_stream = qcom_swrm_set_sdw_stream,
> .get_stream = qcom_swrm_get_sdw_stream,
> - .set_channel_map = qcom_swrm_set_channel_map,
> };
>
> static const struct snd_soc_component_driver qcom_swrm_dai_component = {
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] Revert "soundwire: qcom: Add set_channel_map api support"
2025-07-09 17:49 [PATCH] Revert "soundwire: qcom: Add set_channel_map api support" Amit Pundir
2025-07-10 10:35 ` Srinivas Kandagatla
@ 2025-07-15 14:54 ` Vinod Koul
1 sibling, 0 replies; 3+ messages in thread
From: Vinod Koul @ 2025-07-15 14:54 UTC (permalink / raw)
To: Mark Brown, Mohammad Rafi Shaik, Srinivas Kandagatla, Bard Liao,
Pierre-Louis Bossart, Sanyog Kale, Dan Carpenter, Yongqin Liu,
Jie Gan, Amit Pundir
Cc: linux-arm-msm, linux-sound, lkml, stable
On Wed, 09 Jul 2025 23:19:49 +0530, Amit Pundir wrote:
> This reverts commit 7796c97df6b1b2206681a07f3c80f6023a6593d5.
>
> This patch broke Dragonboard 845c (sdm845). I see:
>
> Unexpected kernel BRK exception at EL1
> Internal error: BRK handler: 00000000f20003e8 [#1] SMP
> pc : qcom_swrm_set_channel_map+0x7c/0x80 [soundwire_qcom]
> lr : snd_soc_dai_set_channel_map+0x34/0x78
> Call trace:
> qcom_swrm_set_channel_map+0x7c/0x80 [soundwire_qcom] (P)
> sdm845_dai_init+0x18c/0x2e0 [snd_soc_sdm845]
> snd_soc_link_init+0x28/0x6c
> snd_soc_bind_card+0x5f4/0xb0c
> snd_soc_register_card+0x148/0x1a4
> devm_snd_soc_register_card+0x50/0xb0
> sdm845_snd_platform_probe+0x124/0x148 [snd_soc_sdm845]
> platform_probe+0x6c/0xd0
> really_probe+0xc0/0x2a4
> __driver_probe_device+0x7c/0x130
> driver_probe_device+0x40/0x118
> __device_attach_driver+0xc4/0x108
> bus_for_each_drv+0x8c/0xf0
> __device_attach+0xa4/0x198
> device_initial_probe+0x18/0x28
> bus_probe_device+0xb8/0xbc
> deferred_probe_work_func+0xac/0xfc
> process_one_work+0x244/0x658
> worker_thread+0x1b4/0x360
> kthread+0x148/0x228
> ret_from_fork+0x10/0x20
> Kernel panic - not syncing: BRK handler: Fatal exception
>
> [...]
Applied, thanks!
[1/1] Revert "soundwire: qcom: Add set_channel_map api support"
commit: 834bce6a715ae9a9c4dce7892454a19adf22b013
Best regards,
--
~Vinod
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2025-07-15 14:54 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-07-09 17:49 [PATCH] Revert "soundwire: qcom: Add set_channel_map api support" Amit Pundir
2025-07-10 10:35 ` Srinivas Kandagatla
2025-07-15 14:54 ` Vinod Koul
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®