* [PATCH] soundwire: qcom: Use guard to avoid mixing cleanup and goto
@ 2025-12-01 10:26 Krzysztof Kozlowski
2025-12-01 10:59 ` Konrad Dybcio
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Krzysztof Kozlowski @ 2025-12-01 10:26 UTC (permalink / raw)
To: Srinivas Kandagatla, Vinod Koul, Bard Liao, Pierre-Louis Bossart,
linux-sound, linux-arm-msm, linux-kernel
Cc: Krzysztof Kozlowski
qcom_swrm_stream_alloc_ports() already uses cleanup.h but also has goto.
Such combination is error-prone and discouraged:
"... and that the "goto" statement can jump between scopes, the
expectation is that usage of "goto" and cleanup helpers is never mixed
in the same function."
Actually simplify the code with a guard which allows to fix the
discouraged style by removing the goto.
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
---
drivers/soundwire/qcom.c | 12 +++++-------
1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/drivers/soundwire/qcom.c b/drivers/soundwire/qcom.c
index 17afc5aa8b44..8102a1b0d516 100644
--- a/drivers/soundwire/qcom.c
+++ b/drivers/soundwire/qcom.c
@@ -1228,7 +1228,7 @@ static int qcom_swrm_stream_alloc_ports(struct qcom_swrm_ctrl *ctrl,
struct sdw_port_runtime *p_rt;
struct sdw_slave *slave;
unsigned long *port_mask;
- int maxport, pn, nports = 0, ret = 0;
+ int maxport, pn, nports = 0;
unsigned int m_port;
struct sdw_port_config *pconfig __free(kfree) = kcalloc(ctrl->nports,
sizeof(*pconfig), GFP_KERNEL);
@@ -1246,7 +1246,8 @@ static int qcom_swrm_stream_alloc_ports(struct qcom_swrm_ctrl *ctrl,
sconfig.type = stream->type;
sconfig.bps = 1;
- mutex_lock(&ctrl->port_lock);
+ guard(mutex)(&ctrl->port_lock);
+
list_for_each_entry(m_rt, &stream->master_list, stream_node) {
/*
* For streams with multiple masters:
@@ -1272,8 +1273,7 @@ static int qcom_swrm_stream_alloc_ports(struct qcom_swrm_ctrl *ctrl,
if (pn > maxport) {
dev_err(ctrl->dev, "All ports busy\n");
- ret = -EBUSY;
- goto out;
+ return -EBUSY;
}
set_bit(pn, port_mask);
pconfig[nports].num = pn;
@@ -1285,10 +1285,8 @@ static int qcom_swrm_stream_alloc_ports(struct qcom_swrm_ctrl *ctrl,
sdw_stream_add_master(&ctrl->bus, &sconfig, pconfig,
nports, stream);
-out:
- mutex_unlock(&ctrl->port_lock);
- return ret;
+ return 0;
}
static int qcom_swrm_hw_params(struct snd_pcm_substream *substream,
--
2.48.1
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] soundwire: qcom: Use guard to avoid mixing cleanup and goto
2025-12-01 10:26 [PATCH] soundwire: qcom: Use guard to avoid mixing cleanup and goto Krzysztof Kozlowski
@ 2025-12-01 10:59 ` Konrad Dybcio
2025-12-01 11:04 ` Srinivas Kandagatla
2025-12-16 9:19 ` Vinod Koul
2 siblings, 0 replies; 4+ messages in thread
From: Konrad Dybcio @ 2025-12-01 10:59 UTC (permalink / raw)
To: Krzysztof Kozlowski, Srinivas Kandagatla, Vinod Koul, Bard Liao,
Pierre-Louis Bossart, linux-sound, linux-arm-msm, linux-kernel
On 12/1/25 11:26 AM, Krzysztof Kozlowski wrote:
> qcom_swrm_stream_alloc_ports() already uses cleanup.h but also has goto.
> Such combination is error-prone and discouraged:
>
> "... and that the "goto" statement can jump between scopes, the
> expectation is that usage of "goto" and cleanup helpers is never mixed
> in the same function."
>
> Actually simplify the code with a guard which allows to fix the
> discouraged style by removing the goto.
>
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
> ---
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Konrad
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] soundwire: qcom: Use guard to avoid mixing cleanup and goto
2025-12-01 10:26 [PATCH] soundwire: qcom: Use guard to avoid mixing cleanup and goto Krzysztof Kozlowski
2025-12-01 10:59 ` Konrad Dybcio
@ 2025-12-01 11:04 ` Srinivas Kandagatla
2025-12-16 9:19 ` Vinod Koul
2 siblings, 0 replies; 4+ messages in thread
From: Srinivas Kandagatla @ 2025-12-01 11:04 UTC (permalink / raw)
To: Krzysztof Kozlowski, Srinivas Kandagatla, Vinod Koul, Bard Liao,
Pierre-Louis Bossart, linux-sound, linux-arm-msm, linux-kernel
On 12/1/25 10:26 AM, Krzysztof Kozlowski wrote:
> qcom_swrm_stream_alloc_ports() already uses cleanup.h but also has goto.
> Such combination is error-prone and discouraged:
>
> "... and that the "goto" statement can jump between scopes, the
> expectation is that usage of "goto" and cleanup helpers is never mixed
> in the same function."
>
> Actually simplify the code with a guard which allows to fix the
> discouraged style by removing the goto.
>
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
> ---
Thanks for the patch,
LGTM,
Reviewed-by: Srinivas Kandagatla <srinivas.kandagatla@oss.qualcomm.com>
--srini> drivers/soundwire/qcom.c | 12 +++++-------
> 1 file changed, 5 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/soundwire/qcom.c b/drivers/soundwire/qcom.c
> index 17afc5aa8b44..8102a1b0d516 100644
> --- a/drivers/soundwire/qcom.c
> +++ b/drivers/soundwire/qcom.c
> @@ -1228,7 +1228,7 @@ static int qcom_swrm_stream_alloc_ports(struct qcom_swrm_ctrl *ctrl,
> struct sdw_port_runtime *p_rt;
> struct sdw_slave *slave;
> unsigned long *port_mask;
> - int maxport, pn, nports = 0, ret = 0;
> + int maxport, pn, nports = 0;
> unsigned int m_port;
> struct sdw_port_config *pconfig __free(kfree) = kcalloc(ctrl->nports,
> sizeof(*pconfig), GFP_KERNEL);
> @@ -1246,7 +1246,8 @@ static int qcom_swrm_stream_alloc_ports(struct qcom_swrm_ctrl *ctrl,
> sconfig.type = stream->type;
> sconfig.bps = 1;
>
> - mutex_lock(&ctrl->port_lock);
> + guard(mutex)(&ctrl->port_lock);
> +
> list_for_each_entry(m_rt, &stream->master_list, stream_node) {
> /*
> * For streams with multiple masters:
> @@ -1272,8 +1273,7 @@ static int qcom_swrm_stream_alloc_ports(struct qcom_swrm_ctrl *ctrl,
>
> if (pn > maxport) {
> dev_err(ctrl->dev, "All ports busy\n");
> - ret = -EBUSY;
> - goto out;
> + return -EBUSY;
> }
> set_bit(pn, port_mask);
> pconfig[nports].num = pn;
> @@ -1285,10 +1285,8 @@ static int qcom_swrm_stream_alloc_ports(struct qcom_swrm_ctrl *ctrl,
>
> sdw_stream_add_master(&ctrl->bus, &sconfig, pconfig,
> nports, stream);
> -out:
> - mutex_unlock(&ctrl->port_lock);
>
> - return ret;
> + return 0;
> }
>
> static int qcom_swrm_hw_params(struct snd_pcm_substream *substream,
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] soundwire: qcom: Use guard to avoid mixing cleanup and goto
2025-12-01 10:26 [PATCH] soundwire: qcom: Use guard to avoid mixing cleanup and goto Krzysztof Kozlowski
2025-12-01 10:59 ` Konrad Dybcio
2025-12-01 11:04 ` Srinivas Kandagatla
@ 2025-12-16 9:19 ` Vinod Koul
2 siblings, 0 replies; 4+ messages in thread
From: Vinod Koul @ 2025-12-16 9:19 UTC (permalink / raw)
To: Srinivas Kandagatla, Bard Liao, Pierre-Louis Bossart, linux-sound,
linux-arm-msm, linux-kernel, Krzysztof Kozlowski
On Mon, 01 Dec 2025 11:26:28 +0100, Krzysztof Kozlowski wrote:
> qcom_swrm_stream_alloc_ports() already uses cleanup.h but also has goto.
> Such combination is error-prone and discouraged:
>
> "... and that the "goto" statement can jump between scopes, the
> expectation is that usage of "goto" and cleanup helpers is never mixed
> in the same function."
>
> [...]
Applied, thanks!
[1/1] soundwire: qcom: Use guard to avoid mixing cleanup and goto
commit: 82ab754d102273f4c974a285aa8025bed7521b15
Best regards,
--
~Vinod
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2025-12-16 9:19 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-12-01 10:26 [PATCH] soundwire: qcom: Use guard to avoid mixing cleanup and goto Krzysztof Kozlowski
2025-12-01 10:59 ` Konrad Dybcio
2025-12-01 11:04 ` Srinivas Kandagatla
2025-12-16 9:19 ` Vinod Koul
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox