linux-kernel - Re: [PATCH 2/2] ASoC: Adds support for TAS575x to the pcm512x driver

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 for Android: free password hash cracker in your pocket

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

Date:   Mon, 11 Sep 2023 13:59:46 +0200
From:   Joerg Schambacher <joerg.hifiberry@...il.com>
To:     Mark Brown <broonie@...nel.org>
Cc:     a-krasser@...com, joerg@...iberry.com,
        Liam Girdwood <lgirdwood@...il.com>,
        Jaroslav Kysela <perex@...ex.cz>,
        Takashi Iwai <tiwai@...e.com>,
        Uwe Kleine-König 
        <u.kleine-koenig@...gutronix.de>,
        Zhang Qilong <zhangqilong3@...wei.com>,
        alsa-devel@...a-project.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/2] ASoC: Adds support for TAS575x to the pcm512x driver

Am 07.09.2023 um 17:28 hat Mark Brown geschrieben:
> On Thu, Sep 07, 2023 at 06:21:50PM +0200, Joerg Schambacher wrote:
> 
> > And yes, we cannot be sure that future devices may require different
> > settings, but I can hardly imagine anything completely different than
> > this approach with the usual audio MCLK frequencies.
> 
> They may for example be restricted and just not the the incoming MCLK
> divider, or require a higher frequency for some fancy processing.  In
> any case tas_device is just an obviously bad name here - there should be
> a flag per quirk, not a flag per entire class of devices.
> 
Ok, I see your point and completely agree. I tend for now to leave that
part out of the patch. This leaves the PLL divider programmings
completely 'untouched'. Nevertheless, I'll continue with testing here
on the hardware.
> > > This is probably a separate quirk?  I'm a bit concerned about what's
> > > turning the PLL off here...
> 
> > The PLL should not be used in this specific case where all divisions are
> > just divide-by-2's. Your original code does reflect that and turns the
> > PLL off. It improves jitter and finally audio performance.
> 
> > But in the case of the TAS-devices we even then need the PLL to
> > drive the AMP clocks.
> 
> That's definitely a separate quirk, and does sound like it should be
> driven from the bias management or DAPM more than hw_params.

Then it makes sense to use a DT-param 'force_pll_on' and even
remove the compatible string fixes from the patch series.
Still, I think, this is the best part of the code to act on the PLL.
Simply instead of checking 'do we need it or not' just let it run.
What do you think?


--