[PATCH v2 0/2] ASoC: tas2783-sdw: fix the read-only Control handling
Andrey Golovko <[email protected]>
| Newsgroups | gmane.linux.kernel,gmane.linux.sound |
|---|---|
| Message-ID | <[email protected]> |
The two read-only Control patches from 13 August, resent together as a series because the second one depends on the first, and with the change Mark asked for. He pointed out that the XU22 Control 0x14 in the read-only list is SDCA_CTL_XU_FDL_STATUS, which sdca_fdl_process() writes. He is right, and the description I gave it - "firmware download status" - reads like a status the device owns, which it is not. It is a handshake: the device sets the bits it owns, and the host writes its response back into the same Control, keeping the device bits, which is exactly what sdca_fdl_process() does with response | (status & ~SDCA_CTL_XU_FDLH_MASK). So it is out of the list in 1/2. Whether the write is reachable on this device today: the driver registers no SDCA interrupts and never calls into sdca_fdl.c. It downloads its firmware itself, with request_firmware_nowait() and its own container format in tas2783_fw_ready(), and the machines I have describe no Smart Amp SDCA function in the BIOS at all, so nothing populates the interrupt that would drive the state machine. But the Function does implement the FDL Controls - the driver lists 0x10 and 0x12 through 0x16 in its defaults - so the state machine is a plausible thing for this driver to grow, and _regmap_write() answers -EIO for a register the callback refuses. That would break the handshake at the first response. Not a trap worth planting for one entry in a list. 2/2 still keeps the Control out of the cache, and now says why on its own: sdca_fdl_process() reads it to see what the device asked for, and a cached answer would hide the device half of the handshake. Writable and volatile, which is the normal shape for a handshake register. The two are otherwise unchanged. The behaviour I measured on an ASUS ProArt PX13 for v1 stands: the calibration data written at probe is byte for byte what it was before the series, and a read of the PDE23 actual power state answers 0x0 with the Function powered, where before it answered the 0x3 placeholder from the defaults. The only difference in v2 is that a write to the File Download Status is no longer rejected, and nothing in the driver issues one. The questions in 1/2 stand as well: the list is static because sdca_regmap_writeable() is not available to a driver running its fallback tables, so it would be good to have it confirmed against the documentation, and to know whether the read-only Controls were meant to be in tas2783_reg_default[] at all. Andrey Golovko (2): ASoC: tas2783-sdw: do not treat read-only Controls as writable ASoC: tas2783-sdw: do not cache read-only Controls sound/soc/codecs/tas2783-sdw.c | 99 +++++++++++++++++++++++----------- 1 file changed, 68 insertions(+), 31 deletions(-)