Date:   Mon, 13 Feb 2023 10:22:21 +0000
From:   Charles Keepax <ckeepax@opensource.cirrus.com>
To:     Lucas Tanure <lucas.tanure@collabora.com>
CC:     David Rhodes <david.rhodes@cirrus.com>,
        Liam Girdwood <lgirdwood@gmail.com>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        Mark Brown <broonie@kernel.org>,
        "Rob Herring" <robh+dt@kernel.org>,
        Jaroslav Kysela <perex@perex.cz>,
        Takashi Iwai <tiwai@suse.com>, <alsa-devel@alsa-project.org>,
        <devicetree@vger.kernel.org>, <patches@opensource.cirrus.com>,
        <linux-kernel@vger.kernel.org>, <kernel@collabora.com>
Subject: Re: [PATCH v5 3/4] ALSA: cs35l41: Add shared boost feature
Message-ID: <20230213102221.GH68926@ediswmail.ad.cirrus.com>
References: <20230210091942.10866-1-lucas.tanure@collabora.com>
 <20230210091942.10866-4-lucas.tanure@collabora.com>
 <20230210134341.GF68926@ediswmail.ad.cirrus.com>
 <cfacc3d6-2daa-6aa3-ba19-281b7e48bb47@collabora.com>
 <20230211170638.GG68926@ediswmail.ad.cirrus.com>
 <1e3ef067-9b39-dc19-5fbc-75436c67f206@collabora.com>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <1e3ef067-9b39-dc19-5fbc-75436c67f206@collabora.com>
User-Agent: Mutt/1.5.21 (2010-09-15)
Precedence: bulk

On Sun, Feb 12, 2023 at 09:28:39AM +0000, 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:
> 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.

A very good point :-)

> >"When powering up a second (and each subsequent) CS35L41B onto a
> >shared MDSYNC bus, the following protocol must
> >be followed"
> >
> >But very unclear if that sequence should be followed on only the
> >new device, the master device, or on all devices. I will try to
> >find some time to chase some apps guys next week see if anyone
> >has any ideas.
> I had long talks with apps guys on this when I was at Cirrus.
> And my understanding is:
> A new CS35L41 can misunderstand the information on MDSYNC bus if a
> communication is already in place, between another pair of CS35L41,
> so to avoid that, any CS35L41 being turn on in a already active
> MDSYNC bus, must execute those steps.

Ok so that implies we are ok, since that suggests we are
saying that only the new amp to the bus needs to execute the
sequence, not the amps already on the bus.

> We could move the active amp up in DAPM graph so its enabled before
> all passive ones, but we would still need to execute that for all
> passive amps. So there is not much point in that.

Agree, fine as is.

> 
> Here I can see that if I enable SYNC_EN during probe without clocks
> the device becomes unresponsive, at least with the current code.
> So following the datasheet and enabling SYNC_EN only after clocks
> seems to resolve Steam decks issue.
> 
> Questions I never got an answer from APPS guys:
> 
> - Can we enable SYNC_EN during probe if we know there is no playback
> happening, no clocks and Global enable off? Steam decks seem to
> answer no here. If yes, why having pm_runtime features makes the
> device become unresponsive?
> 
> - Can we leave SYNC_EN enabled during Global enable off and no clocks?

These two I think are not too much of a concern, turning sync on as
part of powering up the amps doesn't seem to be a big concern,
its not a lot of writes.

> - If SYNC_EN is enabled and we only set Global enable on after the
> PLL lock happened, do we still need to execute those steps? I mean,
> if the driver only deals with Global enable and leaves shared boost
> configured since boost, will MDSYNC bus work?

Yeah I think here it is also probably safer to just do it anyway.

I would still like David to do a quick review, unfortunately he
is off at the moment but should be back Monday next week. But
from my side I think this is probably all good:

Acked-by: Charles Keepax <ckeepax@opensource.cirrus.com>

Thanks,
Charles