soundwire: lower the trace level to warning when the alert is ignored #5619
Conversation
bardliao
requested review from
charleskeepax,
Copilot,
shumingfan and
ujfalusi
There was a problem hiding this comment.
Pull request overview
This PR adjusts logging behavior for SoundWire Command Ignored responses by downgrading error messages to warnings. The Command Ignored response (-ENODATA) is a valid status that can occur when accessing unattached devices during unexpected interrupts, and treating it as an error unnecessarily alarms users.
Key Changes:
- Added conditional logging based on error type, using
dev_warn()for -ENODATA instead ofdev_err() - Applied this pattern to both direct register read failures and slave alert handling failures
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
drivers/soundwire/bus.c
Outdated
| "SDW_SCP_INT1 read failed:%d\n", ret); | ||
| if (ret == -ENODATA) | ||
| dev_warn(&slave->dev, | ||
| "SDW_SCP_INT1 read command was ignored:%d\n", ret); |
There was a problem hiding this comment.
Missing space after colon in warning message. Should be 'was ignored: %d' for consistency with the error message on line 1672.
| "SDW_SCP_INT1 read command was ignored:%d\n", ret); | |
| "SDW_SCP_INT1 read command was ignored: %d\n", ret); |
There was a problem hiding this comment.
There is no space on line 1672, too.
drivers/soundwire/bus.c
Outdated
| "SDW_SCP_INT1 read failed:%d\n", ret); | ||
| if (ret == -ENODATA) | ||
| dev_warn(&slave->dev, | ||
| "SDW_SCP_INT1 read command was ignored:%d\n", ret); |
There was a problem hiding this comment.
[nitpick] Including the error code (%d) in the warning message is redundant since -ENODATA is already confirmed by the condition. The error code adds no additional value for this specific case.
| "SDW_SCP_INT1 read command was ignored:%d\n", ret); | |
| "SDW_SCP_INT1 read command was ignored\n"); |
Command Ignored is a valid response of SoundWire. It could happen when an invalid interrupt occurs when the device is not attached. The driver will try to access the device and will get an Ignored response. It is harmless to ignore the unexpected interrupt. Lower the trace level to not scare users. Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
| "SDW_SCP_INT1 read command was ignored\n"); | ||
| else | ||
| dev_err(&slave->dev, | ||
| "SDW_SCP_INT1 read failed:%d\n", ret); |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
@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
There was a problem hiding this comment.
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.
2 participants
Command Ignored is a valid response of SoundWire. It could happen when an invalid interrupt occurs when the device is not attached. The driver will try to access the device and will get an Ignored response. It is harmless to ignore the unexpected interrupt. Lower the trace level to not scare users.