Re: [PATCH 1/2] soundwire: bus: Don't filter slave alerts

On 1/19/23 10:51, Charles Keepax wrote:

Currently the SoundWire core will loop handling slave alerts but it will only handle those present when the alert was first raised. This causes some issues with the Cadence SoundWire IP, which only generates an IRQ when alert changes state. This means that if a new alert arrives whilst old alerts are being handled it will not be handled in the currently loop and then no further alerts will be processed since alert never changes state to trigger a new IRQ.

Correct this issue by allowing the core to handle all pending alerts in the IRQ handling loop. The code will still only loop up to SDW_READ_INTR_CLEAR_RETRY times, so it shouldn't be possible for it get completely stuck and if you are generating IRQs faster than you can handle them you likely have bigger problems anyway.

The change makes sense, but it's a bit odd to change the way the interrupts are handled because of a specific design. The bus should be able to deal with various designs, not force a one-size-fits-all policy that may not be quite right in all cases.

Could we have a new flag at the bus level that says that peripheral interrupts are not filtered, and set if for the Intel case?

We could similarly make the SDW_READ_INTR_CLEAR_RETRY constant bus/platform specific. The SoundWire spec mandates that we re-read the status after clearing the interrupt, but it doesn't say how to deal with recurring interrupts.

Signed-off-by: Charles Keepax ckeepax@opensource.cirrus.com

drivers/soundwire/bus.c | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c index 633d411b64f35..daee2cca94a4d 100644 --- a/drivers/soundwire/bus.c +++ b/drivers/soundwire/bus.c @@ -1560,7 +1560,7 @@ static int sdw_handle_slave_alerts(struct sdw_slave *slave) unsigned long port; bool slave_notify; u8 sdca_cascade = 0;

• u8 buf, buf2[2], _buf, _buf2[2];

• u8 buf, buf2[2]; bool parity_check; bool parity_quirk;

@@ -1716,9 +1716,9 @@ static int sdw_handle_slave_alerts(struct sdw_slave *slave) "SDW_SCP_INT1 recheck read failed:%d\n", ret); goto io_err; }

• _buf = ret;
  

• buf = ret;
  
  

• ret = sdw_nread_no_pm(slave, SDW_SCP_INTSTAT2, 2, _buf2);
  

• ret = sdw_nread_no_pm(slave, SDW_SCP_INTSTAT2, 2, buf2);
  

  if (ret < 0) { dev_err(&slave->dev, "SDW_SCP_INT2/3 recheck read failed:%d\n", ret);

@@ -1736,12 +1736,8 @@ static int sdw_handle_slave_alerts(struct sdw_slave *slave) }

/*

•  * Make sure no interrupts are pending, but filter to limit loop
  

•  * to interrupts identified in the first status read
  

•  * Make sure no interrupts are pending
  

  */

• buf &= _buf;
  

• buf2[0] &= _buf2[0];
  

• buf2[1] &= _buf2[1];
  

  stat = buf || buf2[0] || buf2[1] || sdca_cascade;

  /*