Skip to content
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 14 additions & 6 deletions drivers/soundwire/bus.c
Original file line number Diff line number Diff line change
Expand Up @@ -1664,8 +1664,12 @@ static int sdw_handle_slave_alerts(struct sdw_slave *slave)
/* Read Intstat 1, Intstat 2 and Intstat 3 registers */
ret = sdw_read_no_pm(slave, SDW_SCP_INT1);
if (ret < 0) {
dev_err(&slave->dev,
"SDW_SCP_INT1 read failed:%d\n", ret);
if (ret == -ENODATA)
dev_warn(&slave->dev,
"SDW_SCP_INT1 read command was ignored\n");
else
dev_err(&slave->dev,
"SDW_SCP_INT1 read failed:%d\n", ret);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the problem is that there are three cases with Command_Ignored
a) an attached device loses sync and read/writes fail. That's an error.
b) the code tries to access a device which never attached. that's harmless but why was this sequence run in the first place?
c) there are cases with SDCA where Command_Ignored is used for flow-control/device-busy.

You would need to handle the different cases instead of treating Ignored as a blanket 'does not matter' result.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@plbossart Thanks for the feedback. It happened when running the suspend resume test. Looks like an interrupt happened after the codec is unattached and before reattached. It looks like an electrical issue to me. But to be honesty, I don't know how to filter out the electrical issue. Or we should not try to filter it out. Just let the driver report the issue.

[ 3668.349727] kernel: soundwire_intel:intel_shim_vs_set_clock_source: soundwire_intel soundwire_intel.link.0: clock source 0 LVSCTL 0x0
[ 3668.349733] kernel: soundwire_intel:intel_link_power_up: soundwire_intel soundwire_intel.link.0: first link up, programming SYNCPRD
[ 3668.349766] kernel: soundwire_bus:sdw_modify_slave_status: rt711-sdca sdw:0:0:025d:0711:01: initializing enumeration and init completion for Slave 6
[ 3668.349887] kernel: soundwire_cadence:cdns_init_clock_ctrl: soundwire_intel soundwire_intel.link.0: mclk 19200000 max 9600000 row 50 col 4
[ 3668.350195] kernel: soundwire_cadence:cdns_update_slave_status_work: soundwire_intel soundwire_intel.link.0: Slave status change: 0x4000000
[ 3668.350590] kernel: soundwire_cadence:cdns_fill_msg_resp: soundwire_intel soundwire_intel.link.0: Msg ignored for Slave 6
[ 3668.350594] kernel: rt711-sdca sdw:0:0:025d:0711:01: SDW_SCP_INT1 read failed:-61
[ 3668.350596] kernel: rt711-sdca sdw:0:0:025d:0711:01: Slave 6 alert handling failed: -61
[ 3668.350633] kernel: soundwire_cadence:cdns_update_slave_status_work: soundwire_intel soundwire_intel.link.0: Slave status change: 0x1000000
[ 3668.350648] kernel: rt711-sdca sdw:0:0:025d:0711:01: Slave 6 state check1: UNATTACHED, status was 2
[ 3668.350649] kernel: soundwire_bus:sdw_modify_slave_status: rt711-sdca sdw:0:0:025d:0711:01: initializing enumeration and init completion for Slave 6
[ 3668.351273] kernel: soundwire_cadence:cdns_update_slave_status_work: soundwire_intel soundwire_intel.link.0: Slave status change: 0x2
[ 3668.351293] kernel: soundwire_bus:sdw_handle_slave_status: soundwire sdw-master-0-0: Slave attached, programming device number
[ 3668.351463] kernel: soundwire_bus:sdw_extract_slave_id: soundwire sdw-master-0-0: SDW Slave Addr: 30025d071101
[ 3668.351464] kernel: soundwire_bus:sdw_extract_slave_id: soundwire sdw-master-0-0: SDW Slave class_id 0x01, mfg_id 0x025d, part_id 0x0711, unique_id 0x0, version 0x3
[ 3668.351764] kernel: soundwire_cadence:cdns_fill_msg_resp: soundwire_intel soundwire_intel.link.0: Msg ignored for Slave 0
[ 3668.351767] kernel: soundwire_bus:sdw_program_device_num: soundwire sdw-master-0-0: No more devices to enumerate

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well in that case you have information on the device status. If you get a command ignored but the device is already unattached then you could indeed ignore the read/write error.

goto io_err;
}
buf = ret;
Expand Down Expand Up @@ -1959,10 +1963,14 @@ int sdw_handle_slave_status(struct sdw_bus *bus,

case SDW_SLAVE_ALERT:
ret = sdw_handle_slave_alerts(slave);
if (ret < 0)
dev_err(&slave->dev,
"Slave %d alert handling failed: %d\n",
i, ret);
if (ret < 0) {
if (ret == -ENODATA)
dev_warn(&slave->dev,
"Slave %d alert is ignored\n", i);
else
dev_err(&slave->dev,
"Slave %d alert handling failed: %d\n", i, ret);
}
break;

case SDW_SLAVE_ATTACHED:
Expand Down
Loading