Re: [PATCH] ASoC: mediatek: mt8188-mt6359: Remove hardcoded dmic codec

[Chen-Yu Tsai <wenst@xxxxxxxxxxxx>]

On Tue, Jan 7, 2025 at 1:33 AM Nícolas F. R. A. Prado
<nfraprado@xxxxxxxxxxxxx> wrote:
>
> On Thu, Dec 26, 2024 at 04:30:17PM +0800, Chen-Yu Tsai wrote:
> > Hi,
> >
> > On Wed, Dec 4, 2024 at 3:22 AM Nícolas F. R. A. Prado
> > <nfraprado@xxxxxxxxxxxxx> wrote:
> > >
> > > Remove hardcoded dmic codec from the UL_SRC dai link to avoid requiring
> > > a dmic codec to be present for the driver to probe, as not every
> > > MT8188-based platform might need a dmic codec. The codec can be assigned
> > > to the dai link through the dai-link property in Devicetree on the
> > > platforms where it is needed.
> >
> > A followup question about this. The DMICs on the Chromebooks are attached
> > to the PMIC codec's input side, which then converts the signals to standard
> > I2S and passes them out to the SoC through its AIF1. So the original code
> > was somewhat incorrect, though it works.
> >
> > How should we describe such a connection, given that the MediaTek sound
> > bindings aren't a full graph?
>
> What you're describing is that the hardware topology looks like this:
>
> --------------------      --------------------
> |    SoC           |      |   MT6359 PMIC    |
> |        UL_SRC BE | <--- | AIF1   AIN0_DMIC | <-- DMic
> --------------------      --------------------

Correct.

> But that the dailink definition in the machine driver had the DMic codec
> connected directly to the UL_SRC BE instead, alongside the connection to the
> PMIC, unlike the topology above.
>
> My understanding is that the dmic codec was added simply to allow the usage of
> the wakeup-delays. From [1] it appears that DAI connections between two codecs
> are possible, though rare. So the PMIC -> DMic connection description might be
> possible in that way, although I'm not sure it brings any benefits besides
> closer resembling the hardware topology.

I suspect we would want to keep the wakeup delays though. AFAICT they aren't
the same number across the board (no pun intended), but actually differ
between devices, perhaps due to differences in the actual DMIC used.

If we don't want the full description, maybe we add the wakeup delay to
the PMIC codec then?

AFAICT [1] is basically hardcoding in the dmic-codec in a different way,
so basically reverting your original patch.

ChenYu

> [1] https://www.kernel.org/doc/html/latest/sound/soc/codec-to-codec.html
>
> >
> > > No Devicetree currently relies on it so it is safe to remove without
> > > worrying about backward compatibility.
> >
> > Removing it didn't seem to cause any issues for the Chromebooks that
> > do actually have DMICs. I suspect the only difference would be that
> > the wakeup-delays no longer apply correctly.
>
> That's my guess too.
>
> Thanks,
> Nícolas