[1/2] ASoC: codecs: wsa883x: do not set can_multi_write flag

  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(-)
  

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