[v1] soundwire: stream: Revert "soundwire: stream: fix programming slave ports for non-continous port maps"

[PATCH RESEND] soundwire: stream: Revert "soundwire: stream: fix programming slave ports for non-continous port maps"

Posted by Krzysztof Kozlowski 2 months, 3 weeks ago

This reverts commit ab8d66d132bc8f1992d3eb6cab8d32dda6733c84 because it
breaks codecs using non-continuous masks in source and sink ports.  The
commit missed the point that port numbers are not used as indices for
iterating over prop.sink_ports or prop.source_ports.

Soundwire core and existing codecs expect that the array passed as
prop.sink_ports and prop.source_ports is continuous.  The port mask still
might be non-continuous, but that's unrelated.

Reported-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Closes: https://lore.kernel.org/all/b6c75eee-761d-44c8-8413-2a5b34ee2f98@linux.intel.com/
Fixes: ab8d66d132bc ("soundwire: stream: fix programming slave ports for non-continous port maps")
Acked-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Reviewed-by: Charles Keepax <ckeepax@opensource.cirrus.com>
Cc: <stable@vger.kernel.org>
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

---

Resending with Ack/Rb tags and missing Cc-stable.
---
 drivers/soundwire/stream.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c
index f275143d7b18..7aa4900dcf31 100644
--- a/drivers/soundwire/stream.c
+++ b/drivers/soundwire/stream.c
@@ -1291,18 +1291,18 @@ struct sdw_dpn_prop *sdw_get_slave_dpn_prop(struct sdw_slave *slave,
 					    unsigned int port_num)
 {
 	struct sdw_dpn_prop *dpn_prop;
-	unsigned long mask;
+	u8 num_ports;
 	int i;
 
 	if (direction == SDW_DATA_DIR_TX) {
-		mask = slave->prop.source_ports;
+		num_ports = hweight32(slave->prop.source_ports);
 		dpn_prop = slave->prop.src_dpn_prop;
 	} else {
-		mask = slave->prop.sink_ports;
+		num_ports = hweight32(slave->prop.sink_ports);
 		dpn_prop = slave->prop.sink_dpn_prop;
 	}
 
-	for_each_set_bit(i, &mask, 32) {
+	for (i = 0; i < num_ports; i++) {
 		if (dpn_prop[i].num == port_num)
 			return &dpn_prop[i];
 	}
-- 
2.43.0

Re: [PATCH RESEND] soundwire: stream: Revert "soundwire: stream: fix programming slave ports for non-continous port maps"

Posted by Vinod Koul 2 months, 2 weeks ago

On Mon, 09 Sep 2024 18:47:46 +0200, Krzysztof Kozlowski wrote:
> This reverts commit ab8d66d132bc8f1992d3eb6cab8d32dda6733c84 because it
> breaks codecs using non-continuous masks in source and sink ports.  The
> commit missed the point that port numbers are not used as indices for
> iterating over prop.sink_ports or prop.source_ports.
> 
> Soundwire core and existing codecs expect that the array passed as
> prop.sink_ports and prop.source_ports is continuous.  The port mask still
> might be non-continuous, but that's unrelated.
> 
> [...]

Applied, thanks!

[1/1] soundwire: stream: Revert "soundwire: stream: fix programming slave ports for non-continous port maps"
      commit: 233a95fd574fde1c375c486540a90304a2d2d49f

Best regards,
-- 
~Vinod

Re: [PATCH RESEND] soundwire: stream: Revert "soundwire: stream: fix programming slave ports for non-continous port maps"

Posted by Péter Ujfalusi 2 months, 2 weeks ago


On 09/09/2024 19:47, Krzysztof Kozlowski wrote:
> This reverts commit ab8d66d132bc8f1992d3eb6cab8d32dda6733c84 because it
> breaks codecs using non-continuous masks in source and sink ports.  The
> commit missed the point that port numbers are not used as indices for
> iterating over prop.sink_ports or prop.source_ports.
> 
> Soundwire core and existing codecs expect that the array passed as
> prop.sink_ports and prop.source_ports is continuous.  The port mask still
> might be non-continuous, but that's unrelated.
> 
> Reported-by: Bard Liao <yung-chuan.liao@linux.intel.com>
> Closes: https://lore.kernel.org/all/b6c75eee-761d-44c8-8413-2a5b34ee2f98@linux.intel.com/
> Fixes: ab8d66d132bc ("soundwire: stream: fix programming slave ports for non-continous port maps")
> Acked-by: Bard Liao <yung-chuan.liao@linux.intel.com>
> Reviewed-by: Charles Keepax <ckeepax@opensource.cirrus.com>
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Tested-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>

> 
> ---
> 
> Resending with Ack/Rb tags and missing Cc-stable.
> ---
>  drivers/soundwire/stream.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c
> index f275143d7b18..7aa4900dcf31 100644
> --- a/drivers/soundwire/stream.c
> +++ b/drivers/soundwire/stream.c
> @@ -1291,18 +1291,18 @@ struct sdw_dpn_prop *sdw_get_slave_dpn_prop(struct sdw_slave *slave,
>  					    unsigned int port_num)
>  {
>  	struct sdw_dpn_prop *dpn_prop;
> -	unsigned long mask;
> +	u8 num_ports;
>  	int i;
>  
>  	if (direction == SDW_DATA_DIR_TX) {
> -		mask = slave->prop.source_ports;
> +		num_ports = hweight32(slave->prop.source_ports);
>  		dpn_prop = slave->prop.src_dpn_prop;
>  	} else {
> -		mask = slave->prop.sink_ports;
> +		num_ports = hweight32(slave->prop.sink_ports);
>  		dpn_prop = slave->prop.sink_dpn_prop;
>  	}
>  
> -	for_each_set_bit(i, &mask, 32) {
> +	for (i = 0; i < num_ports; i++) {
>  		if (dpn_prop[i].num == port_num)
>  			return &dpn_prop[i];
>  	}

-- 
Péter

Re: [PATCH RESEND] soundwire: stream: Revert "soundwire: stream: fix programming slave ports for non-continous port maps"

Posted by Péter Ujfalusi 2 months, 2 weeks ago


On 10/09/2024 16:05, Péter Ujfalusi wrote:
> 
> 
> On 09/09/2024 19:47, Krzysztof Kozlowski wrote:
>> This reverts commit ab8d66d132bc8f1992d3eb6cab8d32dda6733c84 because it
>> breaks codecs using non-continuous masks in source and sink ports.  The
>> commit missed the point that port numbers are not used as indices for
>> iterating over prop.sink_ports or prop.source_ports.
>>
>> Soundwire core and existing codecs expect that the array passed as
>> prop.sink_ports and prop.source_ports is continuous.  The port mask still
>> might be non-continuous, but that's unrelated.
>>
>> Reported-by: Bard Liao <yung-chuan.liao@linux.intel.com>
>> Closes: https://lore.kernel.org/all/b6c75eee-761d-44c8-8413-2a5b34ee2f98@linux.intel.com/
>> Fixes: ab8d66d132bc ("soundwire: stream: fix programming slave ports for non-continous port maps")
>> Acked-by: Bard Liao <yung-chuan.liao@linux.intel.com>
>> Reviewed-by: Charles Keepax <ckeepax@opensource.cirrus.com>
>> Cc: <stable@vger.kernel.org>
>> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> 
> Tested-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>

Vinod: can you pick this patch for 6.11 if there is still time since
upstream is also broken since 6.11-rc6


> 
>>
>> ---
>>
>> Resending with Ack/Rb tags and missing Cc-stable.
>> ---
>>  drivers/soundwire/stream.c | 8 ++++----
>>  1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/soundwire/stream.c b/drivers/soundwire/stream.c
>> index f275143d7b18..7aa4900dcf31 100644
>> --- a/drivers/soundwire/stream.c
>> +++ b/drivers/soundwire/stream.c
>> @@ -1291,18 +1291,18 @@ struct sdw_dpn_prop *sdw_get_slave_dpn_prop(struct sdw_slave *slave,
>>  					    unsigned int port_num)
>>  {
>>  	struct sdw_dpn_prop *dpn_prop;
>> -	unsigned long mask;
>> +	u8 num_ports;
>>  	int i;
>>  
>>  	if (direction == SDW_DATA_DIR_TX) {
>> -		mask = slave->prop.source_ports;
>> +		num_ports = hweight32(slave->prop.source_ports);
>>  		dpn_prop = slave->prop.src_dpn_prop;
>>  	} else {
>> -		mask = slave->prop.sink_ports;
>> +		num_ports = hweight32(slave->prop.sink_ports);
>>  		dpn_prop = slave->prop.sink_dpn_prop;
>>  	}
>>  
>> -	for_each_set_bit(i, &mask, 32) {
>> +	for (i = 0; i < num_ports; i++) {
>>  		if (dpn_prop[i].num == port_num)
>>  			return &dpn_prop[i];
>>  	}
> 

-- 
Péter

Re: [PATCH RESEND] soundwire: stream: Revert "soundwire: stream: fix programming slave ports for non-continous port maps"

Posted by Vinod Koul 2 months, 2 weeks ago

On 10-09-24, 16:49, Péter Ujfalusi wrote:
> 
> 
> On 10/09/2024 16:05, Péter Ujfalusi wrote:
> > 
> > 
> > On 09/09/2024 19:47, Krzysztof Kozlowski wrote:
> >> This reverts commit ab8d66d132bc8f1992d3eb6cab8d32dda6733c84 because it
> >> breaks codecs using non-continuous masks in source and sink ports.  The
> >> commit missed the point that port numbers are not used as indices for
> >> iterating over prop.sink_ports or prop.source_ports.
> >>
> >> Soundwire core and existing codecs expect that the array passed as
> >> prop.sink_ports and prop.source_ports is continuous.  The port mask still
> >> might be non-continuous, but that's unrelated.
> >>
> >> Reported-by: Bard Liao <yung-chuan.liao@linux.intel.com>
> >> Closes: https://lore.kernel.org/all/b6c75eee-761d-44c8-8413-2a5b34ee2f98@linux.intel.com/
> >> Fixes: ab8d66d132bc ("soundwire: stream: fix programming slave ports for non-continous port maps")
> >> Acked-by: Bard Liao <yung-chuan.liao@linux.intel.com>
> >> Reviewed-by: Charles Keepax <ckeepax@opensource.cirrus.com>
> >> Cc: <stable@vger.kernel.org>
> >> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> > 
> > Tested-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
> 
> Vinod: can you pick this patch for 6.11 if there is still time since
> upstream is also broken since 6.11-rc6

Done, should be sent to Linus tomorrow...

-- 
~Vinod