diff mbox series

soundwire: bus: Fix wrong port number in sdw_handle_slave_alerts()

Message ID 20220917140256.689678-1-rf@opensource.cirrus.com (mailing list archive)
State New, archived
Headers show
Series soundwire: bus: Fix wrong port number in sdw_handle_slave_alerts() | expand

Commit Message

Richard Fitzgerald Sept. 17, 2022, 2:02 p.m. UTC
for_each_set_bit() gives the bit-number counting from 0 (LSbit==0).
When processing INTSTAT2, bit 0 is DP4 so the port number is (bit + 4).
Likewise for INTSTAT3 bit 0 is DP11 so port number is (bit + 11).

Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>
---
 drivers/soundwire/bus.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Pierre-Louis Bossart Sept. 19, 2022, 8:44 a.m. UTC | #1
On 9/17/22 16:02, Richard Fitzgerald wrote:
> for_each_set_bit() gives the bit-number counting from 0 (LSbit==0).
> When processing INTSTAT2, bit 0 is DP4 so the port number is (bit + 4).
> Likewise for INTSTAT3 bit 0 is DP11 so port number is (bit + 11).
> 
> Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>

Another thing that never worked, but no one has used port interrupts so
far. I only used it for PRBS tests in early enabling some 3 years ago,
and it was for port 2 IIRC.

Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

> ---
>  drivers/soundwire/bus.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c
> index 8eded1a55227..df0ae869ee51 100644
> --- a/drivers/soundwire/bus.c
> +++ b/drivers/soundwire/bus.c
> @@ -1622,7 +1622,7 @@ static int sdw_handle_slave_alerts(struct sdw_slave *slave)
>  			port = buf2[0] & SDW_SCP_INTSTAT2_PORT4_10;
>  			for_each_set_bit(bit, &port, 8) {
>  				/* scp2 ports start from 4 */
> -				port_num = bit + 3;
> +				port_num = bit + 4;
>  				sdw_handle_port_interrupt(slave,
>  						port_num,
>  						&port_status[port_num]);
> @@ -1634,7 +1634,7 @@ static int sdw_handle_slave_alerts(struct sdw_slave *slave)
>  			port = buf2[1] & SDW_SCP_INTSTAT3_PORT11_14;
>  			for_each_set_bit(bit, &port, 8) {
>  				/* scp3 ports start from 11 */
> -				port_num = bit + 10;
> +				port_num = bit + 11;
>  				sdw_handle_port_interrupt(slave,
>  						port_num,
>  						&port_status[port_num]);
Richard Fitzgerald Sept. 20, 2022, 9:17 a.m. UTC | #2
On 19/09/2022 09:44, Pierre-Louis Bossart wrote:
> 
> 
> On 9/17/22 16:02, Richard Fitzgerald wrote:
>> for_each_set_bit() gives the bit-number counting from 0 (LSbit==0).
>> When processing INTSTAT2, bit 0 is DP4 so the port number is (bit + 4).
>> Likewise for INTSTAT3 bit 0 is DP11 so port number is (bit + 11).
>>
>> Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>
> 
> Another thing that never worked, but no one has used port interrupts so
> far. I only used it for PRBS tests in early enabling some 3 years ago,
> and it was for port 2 IIRC.
> 

Except us because Cirrus Soundwire requires a full DP prepare sequence,
it doesn't support simplified DP_SM.

There's still the problem of the code deadlocking while waiting for the
prepare interrupt, because it holds the bus lock while waiting for the
completion, so the alert handler can't run. That's something I need to
look at fixing. But it's not a blocker, just means that the code has to
wait the whole timeout for each amp. (That's another thing, couldn't we
issue all the DP prepares first so the amps prepare in parallel, then
wait for them to ack?)
diff mbox series

Patch

diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c
index 8eded1a55227..df0ae869ee51 100644
--- a/drivers/soundwire/bus.c
+++ b/drivers/soundwire/bus.c
@@ -1622,7 +1622,7 @@  static int sdw_handle_slave_alerts(struct sdw_slave *slave)
 			port = buf2[0] & SDW_SCP_INTSTAT2_PORT4_10;
 			for_each_set_bit(bit, &port, 8) {
 				/* scp2 ports start from 4 */
-				port_num = bit + 3;
+				port_num = bit + 4;
 				sdw_handle_port_interrupt(slave,
 						port_num,
 						&port_status[port_num]);
@@ -1634,7 +1634,7 @@  static int sdw_handle_slave_alerts(struct sdw_slave *slave)
 			port = buf2[1] & SDW_SCP_INTSTAT3_PORT11_14;
 			for_each_set_bit(bit, &port, 8) {
 				/* scp3 ports start from 11 */
-				port_num = bit + 10;
+				port_num = bit + 11;
 				sdw_handle_port_interrupt(slave,
 						port_num,
 						&port_status[port_num]);