Valve's Steam Deck uses CS35L41 in shared boost mode, where both speakers
share the boost circuit.
Add this support in the shared lib, but for now, shared boost is not
supported in HDA systems as would require BIOS changes.

Based on David Rhodes shared boost patches.

Also, fix boost config overwriting in IRQ found in the review and do a
small refactor of the code.

Changes from V4:
 - Fix Document subject

Changes from V3:
 - Fix wrong code sent
 - Fix ISO C90 mixed declarations and code 

Changes from V2:
 - Drop External boost without VSPK Documentation
 - Move Shared boost to use values 2 and 3
 - Revert back to reg_sequence but reading the value first and only update
the necessary bits
 - Fix bug found by Intel kernel Test Robot

Changes from V1:
 - Fix Documentation patch subject
 - New patch for External boost without VSPK Documentation
 - New patch to fix boost IRQ overwriting issue
 - New patch to refactor IRQ release error code
 - reinit_completion on pcm_startup
 - fix DRE switch overwriting
 - return IRQ_HANDLED in PLL_LOCK case

Lucas Tanure (4):
  ASoC: cs35l41: Only disable internal boost
  ASoC: cs35l41: Refactor error release code
  ALSA: cs35l41: Add shared boost feature
  ASoC: dt-bindings: cirrus,cs35l41: Document CS35l41 shared boost

 .../bindings/sound/cirrus,cs35l41.yaml        |  10 +-
 include/sound/cs35l41.h                       |  13 +-
 sound/pci/hda/cs35l41_hda.c                   |   6 +-
 sound/soc/codecs/cs35l41-lib.c                |  73 +++++++++-
 sound/soc/codecs/cs35l41.c                    | 125 +++++++++---------
 sound/soc/codecs/cs35l41.h                    |   1 +
 6 files changed, 157 insertions(+), 71 deletions(-)

-- 
2.39.1

Apologies for the formatting mistake. Resending the previous reply.

On 2/12/23 03:28, Lucas Tanure wrote:
 > On 11-02-2023 17:06, Charles Keepax wrote:
 >> On Fri, Feb 10, 2023 at 02:39:56PM +0000, Lucas Tanure wrote:
 >>> On 10-02-2023 13:43, Charles Keepax wrote:
 >>>> On Fri, Feb 10, 2023 at 09:19:41AM +0000, Lucas Tanure wrote:
 >>>>> +    {CS35L41_MDSYNC_EN,        0x00001000},
 >>>> David's internal patch appears to set 0x3000 on the active side,
 >>>> not sure where that difference snuck in, or which is the correct
 >>>> value. Your settings appear to make logical sense to me though, TX
 >>>> on the active side, RX on the passive side.
 >>> And as the patch sets TX and RX in the same chip I changed to follow
 >>> the documentation.
 >>
 >> Yeah I mean I suspect this is sensible, unless there is some
 >> reason the controller side also needs to have RX enabled. Perhaps
 >> for feedback or something from the passive side, but I imagine
 >> this is just a typo in the original patch.
 >
 > Ok, but the other side doesn't have both RX and TX enabled.
 > If the active side needed RX to receive information for the other 
side, the passive one would need TX enabled too.
 > So if a feedback is necessary, both channels on both sides would be 
enabled, not one channel in one side and both on the other.
 >
Both amps need to transmit their boost targets to the MDSYNC bus. The 
active amp needs to receive the combined boost target from the MDSYNC 
bus. That is why the active amp should enable both RX and TX, and the 
passive amp only needs to enable TX. It is not simply a unidirectional 
flow of data from one amp to the other.

Sorry if the documentation has been mismatched or confusing at times. 
It's taken me a while to gather the right understanding of how this all 
works.


On 2/10/23 03:19, Lucas Tanure wrote:
 > +      Shared boost allows two amplifiers to share a single boost 
circuit by
 > +      communicating on the MDSYNC bus. The passive amplifier does 
not control
 > +      the boost and receives data from the active amplifier. GPIO1 
should be

Not quite correct. I would suggest: "Shared boost allows two amplifiers 
to share a single boost circuit by communicating on the MDSYNC bus. The 
active amplifier controls the boost circuit using combined data from 
both amplifiers."


On 2/10/23 08:39, Lucas Tanure wrote:
 >
 > This write here is to select the boost control source, which for the 
active should be "Class H tracking value".

Active should use the MDSYNC value. Otherwise it will not provide boost 
to the passive amp when there is only audio on the passive amp's channel.


I believe there is another change needed for the Deck, to handle the 
'legacy' property names instead of bst-type?

Other than the changes needed to the reg_sequences this looks good.


Thanks,

David

On Fri, 10 Feb 2023 10:19:38 +0100,
Lucas Tanure wrote:
> 
> Valve's Steam Deck uses CS35L41 in shared boost mode, where both speakers
> share the boost circuit.
> Add this support in the shared lib, but for now, shared boost is not
> supported in HDA systems as would require BIOS changes.
> 
> Based on David Rhodes shared boost patches.
> 
> Also, fix boost config overwriting in IRQ found in the review and do a
> small refactor of the code.
> 
> Changes from V4:
>  - Fix Document subject
> 
> Changes from V3:
>  - Fix wrong code sent
>  - Fix ISO C90 mixed declarations and code 
> 
> Changes from V2:
>  - Drop External boost without VSPK Documentation
>  - Move Shared boost to use values 2 and 3
>  - Revert back to reg_sequence but reading the value first and only update
> the necessary bits
>  - Fix bug found by Intel kernel Test Robot
> 
> Changes from V1:
>  - Fix Documentation patch subject
>  - New patch for External boost without VSPK Documentation
>  - New patch to fix boost IRQ overwriting issue
>  - New patch to refactor IRQ release error code
>  - reinit_completion on pcm_startup
>  - fix DRE switch overwriting
>  - return IRQ_HANDLED in PLL_LOCK case
> 
> Lucas Tanure (4):
>   ASoC: cs35l41: Only disable internal boost
>   ASoC: cs35l41: Refactor error release code
>   ALSA: cs35l41: Add shared boost feature
>   ASoC: dt-bindings: cirrus,cs35l41: Document CS35l41 shared boost

In case the series going through Mark's tree:

Reviewed-by: Takashi Iwai <tiwai@suse.de>


thanks,

Takashi