diff mbox series

[v2,2/5] soundwire: qcom: update port map allocation bit mask

Message ID 20210309141514.24744-3-srinivas.kandagatla@linaro.org
State Superseded
Headers show
Series soundwire: add static port map support | expand

Commit Message

Srinivas Kandagatla March 9, 2021, 2:15 p.m. UTC 
currently the internal bitmask used for allocating ports starts with offset 0.
This is bit confusing as data port numbers on Qualcomm controller are valid
from 1 to 14. So adjust this bit mask accordingly, this will also help while
adding static port map support.

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>

---
 drivers/soundwire/qcom.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

-- 
2.21.0

Comments

Pierre-Louis Bossart March 9, 2021, 3:55 p.m. UTC | #1 
On 3/9/21 8:15 AM, Srinivas Kandagatla wrote:
> currently the internal bitmask used for allocating ports starts with offset 0.

> This is bit confusing as data port numbers on Qualcomm controller are valid

> from 1 to 14. So adjust this bit mask accordingly, this will also help while

> adding static port map support.

> 

> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>

> ---

>   drivers/soundwire/qcom.c | 11 +++++++----

>   1 file changed, 7 insertions(+), 4 deletions(-)

> 

> diff --git a/drivers/soundwire/qcom.c b/drivers/soundwire/qcom.c

> index 6d22df01f354..f4f1c5f2af0b 100644

> --- a/drivers/soundwire/qcom.c

> +++ b/drivers/soundwire/qcom.c

> @@ -519,7 +519,7 @@ static void qcom_swrm_stream_free_ports(struct qcom_swrm_ctrl *ctrl,

>   			port_mask = &ctrl->din_port_mask;

>   

>   		list_for_each_entry(p_rt, &m_rt->port_list, port_node)

> -			clear_bit(p_rt->num - 1, port_mask);

> +			clear_bit(p_rt->num, port_mask);

>   	}

>   

>   	mutex_unlock(&ctrl->port_lock);

> @@ -552,13 +552,13 @@ static int qcom_swrm_stream_alloc_ports(struct qcom_swrm_ctrl *ctrl,

>   			list_for_each_entry(p_rt, &s_rt->port_list, port_node) {

>   				/* Port numbers start from 1 - 14*/

>   				pn = find_first_zero_bit(port_mask, maxport);

> -				if (pn > (maxport - 1)) {

> +				if (pn > (maxport)) {


nit-pick: useless parentheses

>   					dev_err(ctrl->dev, "All ports busy\n");

>   					ret = -EBUSY;

>   					goto err;

>   				}

>   				set_bit(pn, port_mask);

> -				pconfig[nports].num = pn + 1;

> +				pconfig[nports].num = pn;

>   				pconfig[nports].ch_mask = p_rt->ch_mask;

>   				nports++;

>   			}

> @@ -580,7 +580,7 @@ static int qcom_swrm_stream_alloc_ports(struct qcom_swrm_ctrl *ctrl,

>   err:

>   	if (ret) {

>   		for (i = 0; i < nports; i++)

> -			clear_bit(pconfig[i].num - 1, port_mask);

> +			clear_bit(pconfig[i].num, port_mask);

>   	}

>   

>   	mutex_unlock(&ctrl->port_lock);

> @@ -754,6 +754,9 @@ static int qcom_swrm_get_port_config(struct qcom_swrm_ctrl *ctrl)

>   	ctrl->num_dout_ports = val;

>   

>   	nports = ctrl->num_dout_ports + ctrl->num_din_ports;

> +	/* port numbers are non zero, so mark port 0 */


mask?

> +	set_bit(0, &ctrl->dout_port_mask);

> +	set_bit(0, &ctrl->din_port_mask);

>   

>   	ret = of_property_read_u8_array(np, "qcom,ports-offset1",

>   					off1, nports);

>
Srinivas Kandagatla March 11, 2021, 2:29 p.m. UTC | #2 
On 09/03/2021 15:55, Pierre-Louis Bossart wrote:
> 

> 

> On 3/9/21 8:15 AM, Srinivas Kandagatla wrote:

>> currently the internal bitmask used for allocating ports starts with 

>> offset 0.

>> This is bit confusing as data port numbers on Qualcomm controller are 

>> valid

>> from 1 to 14. So adjust this bit mask accordingly, this will also help 

>> while

>> adding static port map support.

>>

>> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>

>> ---

>>   drivers/soundwire/qcom.c | 11 +++++++----

>>   1 file changed, 7 insertions(+), 4 deletions(-)

>>

>> diff --git a/drivers/soundwire/qcom.c b/drivers/soundwire/qcom.c

>> index 6d22df01f354..f4f1c5f2af0b 100644

>> --- a/drivers/soundwire/qcom.c

>> +++ b/drivers/soundwire/qcom.c

>> @@ -519,7 +519,7 @@ static void qcom_swrm_stream_free_ports(struct 

>> qcom_swrm_ctrl *ctrl,

>>               port_mask = &ctrl->din_port_mask;

>>           list_for_each_entry(p_rt, &m_rt->port_list, port_node)

>> -            clear_bit(p_rt->num - 1, port_mask);

>> +            clear_bit(p_rt->num, port_mask);

>>       }

>>       mutex_unlock(&ctrl->port_lock);

>> @@ -552,13 +552,13 @@ static int qcom_swrm_stream_alloc_ports(struct 

>> qcom_swrm_ctrl *ctrl,

>>               list_for_each_entry(p_rt, &s_rt->port_list, port_node) {

>>                   /* Port numbers start from 1 - 14*/

>>                   pn = find_first_zero_bit(port_mask, maxport);

>> -                if (pn > (maxport - 1)) {

>> +                if (pn > (maxport)) {

> 

> nit-pick: useless parentheses


I agree, will remove this and address the other comment too in next spin!

--srini
> 

>>                       dev_err(ctrl->dev, "All ports busy\n");

>>                       ret = -EBUSY;

>>                       goto err;

>>                   }

>>                   set_bit(pn, port_mask);

>> -                pconfig[nports].num = pn + 1;

>> +                pconfig[nports].num = pn;

>>                   pconfig[nports].ch_mask = p_rt->ch_mask;

>>                   nports++;

>>               }

>> @@ -580,7 +580,7 @@ static int qcom_swrm_stream_alloc_ports(struct 

>> qcom_swrm_ctrl *ctrl,

>>   err:

>>       if (ret) {

>>           for (i = 0; i < nports; i++)

>> -            clear_bit(pconfig[i].num - 1, port_mask);

>> +            clear_bit(pconfig[i].num, port_mask);

>>       }

>>       mutex_unlock(&ctrl->port_lock);

>> @@ -754,6 +754,9 @@ static int qcom_swrm_get_port_config(struct 

>> qcom_swrm_ctrl *ctrl)

>>       ctrl->num_dout_ports = val;

>>       nports = ctrl->num_dout_ports + ctrl->num_din_ports;

>> +    /* port numbers are non zero, so mark port 0 */

> 

> mask?

> 

>> +    set_bit(0, &ctrl->dout_port_mask);

>> +    set_bit(0, &ctrl->din_port_mask);

>>       ret = of_property_read_u8_array(np, "qcom,ports-offset1",

>>                       off1, nports);

>>
diff mbox series

Patch

diff --git a/drivers/soundwire/qcom.c b/drivers/soundwire/qcom.c
index 6d22df01f354..f4f1c5f2af0b 100644
--- a/drivers/soundwire/qcom.c
+++ b/drivers/soundwire/qcom.c
@@ -519,7 +519,7 @@  static void qcom_swrm_stream_free_ports(struct qcom_swrm_ctrl *ctrl,
 			port_mask = &ctrl->din_port_mask;
 
 		list_for_each_entry(p_rt, &m_rt->port_list, port_node)
-			clear_bit(p_rt->num - 1, port_mask);
+			clear_bit(p_rt->num, port_mask);
 	}
 
 	mutex_unlock(&ctrl->port_lock);
@@ -552,13 +552,13 @@  static int qcom_swrm_stream_alloc_ports(struct qcom_swrm_ctrl *ctrl,
 			list_for_each_entry(p_rt, &s_rt->port_list, port_node) {
 				/* Port numbers start from 1 - 14*/
 				pn = find_first_zero_bit(port_mask, maxport);
-				if (pn > (maxport - 1)) {
+				if (pn > (maxport)) {
 					dev_err(ctrl->dev, "All ports busy\n");
 					ret = -EBUSY;
 					goto err;
 				}
 				set_bit(pn, port_mask);
-				pconfig[nports].num = pn + 1;
+				pconfig[nports].num = pn;
 				pconfig[nports].ch_mask = p_rt->ch_mask;
 				nports++;
 			}
@@ -580,7 +580,7 @@  static int qcom_swrm_stream_alloc_ports(struct qcom_swrm_ctrl *ctrl,
 err:
 	if (ret) {
 		for (i = 0; i < nports; i++)
-			clear_bit(pconfig[i].num - 1, port_mask);
+			clear_bit(pconfig[i].num, port_mask);
 	}
 
 	mutex_unlock(&ctrl->port_lock);
@@ -754,6 +754,9 @@  static int qcom_swrm_get_port_config(struct qcom_swrm_ctrl *ctrl)
 	ctrl->num_dout_ports = val;
 
 	nports = ctrl->num_dout_ports + ctrl->num_din_ports;
+	/* port numbers are non zero, so mark port 0 */
+	set_bit(0, &ctrl->dout_port_mask);
+	set_bit(0, &ctrl->din_port_mask);
 
 	ret = of_property_read_u8_array(np, "qcom,ports-offset1",
 					off1, nports);