| Message ID | 20230523154605.4284-1-srinivas.kandagatla@linaro.org (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 40ba0411074485e2cf1bf8ee0f3db27bdff88394 |
| Headers | show |
| Series | [1/2] ASoC: codecs: wsa883x: do not set can_multi_write flag | expand |
On Tue, May 23, 2023 at 04:46:04PM +0100, Srinivas Kandagatla wrote: > regmap-sdw does not support multi register writes, so there is > no point in setting this flag. This also leads to incorrect > programming of WSA codecs with regmap_multi_reg_write() call. > This invalid configuration should have been rejected by regmap-sdw. Does the CODEC support mulitple writes? If so it seems better to leave the flag set and just do the appropriate fix in the regmap code until such time as it's updated to be able to exercise it.
On 23/05/2023 17:55, Mark Brown wrote: > On Tue, May 23, 2023 at 04:46:04PM +0100, Srinivas Kandagatla wrote: >> regmap-sdw does not support multi register writes, so there is >> no point in setting this flag. This also leads to incorrect >> programming of WSA codecs with regmap_multi_reg_write() call. > >> This invalid configuration should have been rejected by regmap-sdw. > > Does the CODEC support mulitple writes? If so it seems better to leave No, the codec itself does not support multi-write. All the codec register level interface is via SoundWire Bus. which also does not support with the existing code. --srini > the flag set and just do the appropriate fix in the regmap code until > such time as it's updated to be able to exercise it.
On Wed, May 24, 2023 at 09:51:00AM +0100, Srinivas Kandagatla wrote: > On 23/05/2023 17:55, Mark Brown wrote: > > Does the CODEC support mulitple writes? If so it seems better to leave > No, the codec itself does not support multi-write. All the codec register > level interface is via SoundWire Bus. which also does not support with the > existing code. I'm unclear, is this a limitation of the hardware or of the current Soundwire code?
On 24/05/2023 09:57, Mark Brown wrote: > On Wed, May 24, 2023 at 09:51:00AM +0100, Srinivas Kandagatla wrote: >> On 23/05/2023 17:55, Mark Brown wrote: > >>> Does the CODEC support mulitple writes? If so it seems better to leave > >> No, the codec itself does not support multi-write. All the codec register >> level interface is via SoundWire Bus. which also does not support with the >> existing code. > > I'm unclear, is this a limitation of the hardware or of the current > Soundwire code? Its both. Codec itself does not have any private write callback to support this and AFAIU Qualcomm Soundwire controller does not have any such hw facility to program multi-registers for device at one shot. Also to note, the current state of code soundwire regmap write callback assumes that the request to write will always have base address at the start followed by register values. This is not true for multi-register writes which comes in address-value-padding pairs. Am confused on the multi_write feature and looking at regmap bus level write callback. Is write callback used for both Bulk writes and multi-writes? Is multi-write feature of regmap bus or device? --srini
On Wed, May 24, 2023 at 10:42:21AM +0100, Srinivas Kandagatla wrote: > On 24/05/2023 09:57, Mark Brown wrote: > > I'm unclear, is this a limitation of the hardware or of the current > > Soundwire code? > Its both. > Codec itself does not have any private write callback to support this and > AFAIU Qualcomm Soundwire controller does not have any such hw facility to > program multi-registers for device at one shot. How about the *CODEC* hardware? > Is write callback used for both Bulk writes and multi-writes? Only multi-write at this point but we probably should consider redoing bulk writes to use it as well. > Is multi-write feature of regmap bus or device? Well, I don't think any buses that understand registers have implemented it yet but there's nothing fundamental that means that a bus couldn't usefully do something with multi-write. The current users all use raw buses that don't know anything about registers or values.
On 24/05/2023 10:48, Mark Brown wrote: > On Wed, May 24, 2023 at 10:42:21AM +0100, Srinivas Kandagatla wrote: >> On 24/05/2023 09:57, Mark Brown wrote: > >>> I'm unclear, is this a limitation of the hardware or of the current >>> Soundwire code? > >> Its both. > >> Codec itself does not have any private write callback to support this and >> AFAIU Qualcomm Soundwire controller does not have any such hw facility to >> program multi-registers for device at one shot. > > How about the *CODEC* hardware? > No, Codec does not have any such interface to write to device registers directly without going via Soundwire. >> Is write callback used for both Bulk writes and multi-writes? > > Only multi-write at this point but we probably should consider redoing > bulk writes to use it as well. By the looks of the code, its other way around. > >> Is multi-write feature of regmap bus or device? > > Well, I don't think any buses that understand registers have implemented > it yet but there's nothing fundamental that means that a bus couldn't > usefully do something with multi-write. The current users all use raw > buses that don't know anything about registers or values.
On Wed, May 24, 2023 at 11:09:32AM +0100, Srinivas Kandagatla wrote: > On 24/05/2023 10:48, Mark Brown wrote: > > > Is write callback used for both Bulk writes and multi-writes? > > Only multi-write at this point but we probably should consider redoing > > bulk writes to use it as well. > By the looks of the code, its other way around. No, that's not possible. A bulk write requires a contiguous block of registers so can be expressed in terms of a multi-write but a write with discontiguous registers can't be done as a bulk write.
On Tue, 23 May 2023 16:46:04 +0100, Srinivas Kandagatla wrote: > regmap-sdw does not support multi register writes, so there is > no point in setting this flag. This also leads to incorrect > programming of WSA codecs with regmap_multi_reg_write() call. > > This invalid configuration should have been rejected by regmap-sdw. > > > [...] Applied to https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next Thanks! [1/2] ASoC: codecs: wsa883x: do not set can_multi_write flag commit: 40ba0411074485e2cf1bf8ee0f3db27bdff88394 [2/2] ASoC: codecs: wsa881x: do not set can_multi_write flag commit: 6e7a6d4797ef521c0762914610ed682e102b9d36 All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted. You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed. If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced. Please add any relevant lists and maintainers to the CCs when replying to this mail. Thanks, Mark
On Wed, May 24, 2023 at 12:28:10PM +0100, Mark Brown wrote: > On Tue, 23 May 2023 16:46:04 +0100, Srinivas Kandagatla wrote: > > regmap-sdw does not support multi register writes, so there is > > no point in setting this flag. This also leads to incorrect > > programming of WSA codecs with regmap_multi_reg_write() call. > > > > This invalid configuration should have been rejected by regmap-sdw. > > > > > > [...] > > Applied to > > https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next > > Thanks! > > [1/2] ASoC: codecs: wsa883x: do not set can_multi_write flag > commit: 40ba0411074485e2cf1bf8ee0f3db27bdff88394 > [2/2] ASoC: codecs: wsa881x: do not set can_multi_write flag > commit: 6e7a6d4797ef521c0762914610ed682e102b9d36 These were merged for 6.5 but the corresponding sanity check for regmap has now been included in 6.4-rc5 which consequently breaks these codecs (similar for wcd938x-sdw): [ 11.443485] wsa883x-codec sdw:0:0217:0202:00:1: error -ENOTSUPP: regmap_init failed [ 11.443525] wsa883x-codec sdw:0:0217:0202:00:1: Probe of wsa883x-codec failed: -524 [ 11.443554] wsa883x-codec: probe of sdw:0:0217:0202:00:1 failed with error -52 Is it possible to get also these fixes into 6.4 final? Johan
On Mon, Jun 05, 2023 at 10:08:34AM +0200, Johan Hovold wrote:
> Is it possible to get also these fixes into 6.4 final?
They're already queued for 6.4.
On Mon, Jun 05, 2023 at 01:24:04PM +0100, Mark Brown wrote: > On Mon, Jun 05, 2023 at 10:08:34AM +0200, Johan Hovold wrote: > > > Is it possible to get also these fixes into 6.4 final? > > They're already queued for 6.4. Indeed they are. Your patch-applied message said "Applied to ... for-next" and I didn't check your repo before reporting. Sorry about the noise. Johan
diff --git a/sound/soc/codecs/wsa883x.c b/sound/soc/codecs/wsa883x.c index b83b5b0d4bab..2faf66c60703 100644 --- a/sound/soc/codecs/wsa883x.c +++ b/sound/soc/codecs/wsa883x.c @@ -946,7 +946,6 @@ static struct regmap_config wsa883x_regmap_config = { .writeable_reg = wsa883x_writeable_register, .reg_format_endian = REGMAP_ENDIAN_NATIVE, .val_format_endian = REGMAP_ENDIAN_NATIVE, - .can_multi_write = true, .use_single_read = true, };
regmap-sdw does not support multi register writes, so there is no point in setting this flag. This also leads to incorrect programming of WSA codecs with regmap_multi_reg_write() call. This invalid configuration should have been rejected by regmap-sdw. Fixes: 43b8c7dc85a1 ("ASoC: codecs: add wsa883x amplifier support") Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> --- sound/soc/codecs/wsa883x.c | 1 - 1 file changed, 1 deletion(-)