2023-11-20 19:08:10

by Krzysztof Kozlowski

[permalink] [raw]
Subject: [PATCH 1/2] soundwire: qcom: drop unneeded qcom_swrm_stream_alloc_ports() cleanup

The cleanup in "err" goto label clears bits from pconfig array which is
a local variable. This does not have any effect outside of this
function, so drop this useless code.

Signed-off-by: Krzysztof Kozlowski <[email protected]>
---
drivers/soundwire/qcom.c | 11 +++--------
1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/drivers/soundwire/qcom.c b/drivers/soundwire/qcom.c
index a1e2d6c98186..754870a4a047 100644
--- a/drivers/soundwire/qcom.c
+++ b/drivers/soundwire/qcom.c
@@ -1157,7 +1157,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 i, maxport, pn, nports = 0, ret = 0;
+ int maxport, pn, nports = 0, ret = 0;
unsigned int m_port;

mutex_lock(&ctrl->port_lock);
@@ -1183,7 +1183,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 err;
+ goto out;
}
set_bit(pn, port_mask);
pconfig[nports].num = pn;
@@ -1205,12 +1205,7 @@ static int qcom_swrm_stream_alloc_ports(struct qcom_swrm_ctrl *ctrl,
sconfig.bps = 1;
sdw_stream_add_master(&ctrl->bus, &sconfig, pconfig,
nports, stream);
-err:
- if (ret) {
- for (i = 0; i < nports; i++)
- clear_bit(pconfig[i].num, port_mask);
- }
-
+out:
mutex_unlock(&ctrl->port_lock);

return ret;
--
2.34.1


2023-11-20 19:09:31

by Krzysztof Kozlowski

[permalink] [raw]
Subject: [PATCH 2/2] soundwire: qcom: move sconfig in qcom_swrm_stream_alloc_ports() out of critical section

Setting members of local variable "sconfig" in
qcom_swrm_stream_alloc_ports() does not depend on any earlier code in
this function, so can be moved up before the critical section. This
makes the code a bit easier to follow because critical section is
smaller.

Signed-off-by: Krzysztof Kozlowski <[email protected]>
---
drivers/soundwire/qcom.c | 21 +++++++++++----------
1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/drivers/soundwire/qcom.c b/drivers/soundwire/qcom.c
index 754870a4a047..e9a52c1bd359 100644
--- a/drivers/soundwire/qcom.c
+++ b/drivers/soundwire/qcom.c
@@ -1160,6 +1160,17 @@ static int qcom_swrm_stream_alloc_ports(struct qcom_swrm_ctrl *ctrl,
int maxport, pn, nports = 0, ret = 0;
unsigned int m_port;

+ if (direction == SNDRV_PCM_STREAM_CAPTURE)
+ sconfig.direction = SDW_DATA_DIR_TX;
+ else
+ sconfig.direction = SDW_DATA_DIR_RX;
+
+ /* hw parameters wil be ignored as we only support PDM */
+ sconfig.ch_count = 1;
+ sconfig.frame_rate = params_rate(params);
+ sconfig.type = stream->type;
+ sconfig.bps = 1;
+
mutex_lock(&ctrl->port_lock);
list_for_each_entry(m_rt, &stream->master_list, stream_node) {
if (m_rt->direction == SDW_DATA_DIR_RX) {
@@ -1193,16 +1204,6 @@ static int qcom_swrm_stream_alloc_ports(struct qcom_swrm_ctrl *ctrl,
}
}

- if (direction == SNDRV_PCM_STREAM_CAPTURE)
- sconfig.direction = SDW_DATA_DIR_TX;
- else
- sconfig.direction = SDW_DATA_DIR_RX;
-
- /* hw parameters wil be ignored as we only support PDM */
- sconfig.ch_count = 1;
- sconfig.frame_rate = params_rate(params);
- sconfig.type = stream->type;
- sconfig.bps = 1;
sdw_stream_add_master(&ctrl->bus, &sconfig, pconfig,
nports, stream);
out:
--
2.34.1

2023-11-22 19:57:32

by Konrad Dybcio

[permalink] [raw]
Subject: Re: [PATCH 1/2] soundwire: qcom: drop unneeded qcom_swrm_stream_alloc_ports() cleanup



On 11/20/23 20:07, Krzysztof Kozlowski wrote:
> The cleanup in "err" goto label clears bits from pconfig array which is
> a local variable. This does not have any effect outside of this
> function, so drop this useless code.
>
> Signed-off-by: Krzysztof Kozlowski <[email protected]>
> ---
Reviewed-by: Konrad Dybcio <[email protected]>

Konrad

2023-11-22 19:59:08

by Konrad Dybcio

[permalink] [raw]
Subject: Re: [PATCH 2/2] soundwire: qcom: move sconfig in qcom_swrm_stream_alloc_ports() out of critical section



On 11/20/23 20:07, Krzysztof Kozlowski wrote:
> Setting members of local variable "sconfig" in
> qcom_swrm_stream_alloc_ports() does not depend on any earlier code in
> this function, so can be moved up before the critical section. This
> makes the code a bit easier to follow because critical section is
> smaller.
>
> Signed-off-by: Krzysztof Kozlowski <[email protected]>
> ---
Reviewed-by: Konrad Dybcio <[email protected]>

Konrad

2023-11-23 07:24:09

by Vinod Koul

[permalink] [raw]
Subject: Re: [PATCH 1/2] soundwire: qcom: drop unneeded qcom_swrm_stream_alloc_ports() cleanup


On Mon, 20 Nov 2023 20:07:39 +0100, Krzysztof Kozlowski wrote:
> The cleanup in "err" goto label clears bits from pconfig array which is
> a local variable. This does not have any effect outside of this
> function, so drop this useless code.
>
>

Applied, thanks!

[1/2] soundwire: qcom: drop unneeded qcom_swrm_stream_alloc_ports() cleanup
commit: 5c68b66d4d7eff8cdb6f508f8537faa30c5faa6d
[2/2] soundwire: qcom: move sconfig in qcom_swrm_stream_alloc_ports() out of critical section
commit: 5bdc61ef45007908df9d8587111c7a5a552bdd46

Best regards,
--
~Vinod