linux-kernel - Re: [PATCH v5 3/4] ALSA: cs35l41: Add shared boost feature

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <09bf8e07-6275-654f-4a70-d46b54e9b853@collabora.com>
Date:   Tue, 21 Feb 2023 08:28:42 +0000
From:   Lucas Tanure <lucas.tanure@...labora.com>
To:     David Rhodes <drhodes@...nsource.cirrus.com>,
        Charles Keepax <ckeepax@...nsource.cirrus.com>
Cc:     David Rhodes <david.rhodes@...rus.com>,
        Liam Girdwood <lgirdwood@...il.com>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Mark Brown <broonie@...nel.org>,
        Rob Herring <robh+dt@...nel.org>,
        Takashi Iwai <tiwai@...e.com>, alsa-devel@...a-project.org,
        devicetree@...r.kernel.org, patches@...nsource.cirrus.com,
        linux-kernel@...r.kernel.org, kernel@...labora.com
Subject: Re: [PATCH v5 3/4] ALSA: cs35l41: Add shared boost feature

On 20-02-2023 19:25, David Rhodes wrote:
> 
> 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."
Ok

> 
> 
> 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.
David can you confirm that both sides should use MDSYNC for boost 
control source?


> 
> 
> I believe there is another change needed for the Deck, to handle the 
> 'legacy' property names instead of bst-type?
I am working with valve to update their bios.

> 
> Other than the changes needed to the reg_sequences this looks good.
> 
> 
> Thanks,
> 
> David
>