linux-kernel - Re: [PATCH v2 2/2] ASoC: codecs: wsa884x: Add WSA884x family of speakers

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-prev] [day] [month] [year] [list]

Message-ID: <bf81773c-8d9d-46bc-b9ba-db7b86600e3d@sirena.org.uk>
Date:   Mon, 12 Jun 2023 14:36:33 +0100
From:   Mark Brown <broonie@...nel.org>
To:     Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
Cc:     Srinivas Kandagatla <srinivas.kandagatla@...aro.org>,
        Banajit Goswami <bgoswami@...cinc.com>,
        Andy Gross <agross@...nel.org>,
        Bjorn Andersson <andersson@...nel.org>,
        Konrad Dybcio <konrad.dybcio@...aro.org>,
        Liam Girdwood <lgirdwood@...il.com>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Conor Dooley <conor+dt@...nel.org>,
        Jaroslav Kysela <perex@...ex.cz>,
        Takashi Iwai <tiwai@...e.com>, alsa-devel@...a-project.org,
        linux-arm-msm@...r.kernel.org, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org, Patrick Lai <quic_plai@...cinc.com>
Subject: Re: [PATCH v2 2/2] ASoC: codecs: wsa884x: Add WSA884x family of
 speakers

On Mon, Jun 12, 2023 at 09:25:44AM +0200, Krzysztof Kozlowski wrote:
> On 11/06/2023 13:57, Mark Brown wrote:
> > On Sun, Jun 11, 2023 at 12:26:57PM +0200, Krzysztof Kozlowski wrote:

> >> +	{ WSA884X_OTP_REG_40,			0x00 },
> >> +	{ WSA884X_OTP_REG_41,			0x00 },
> >> +	{ WSA884X_OTP_REG_63,			0x40 },

> > These appear to be OTP data which suggests that they shouldn't have
> > defaults either since they can be programmed.

> Just to be clear - I don't have access to datasheet so I don't know what
> are these. The downstream driver actually initializes (writes to) two
> OTP registers - 38 and 40.

That's...  counterintuitive given the naming.

> >> +static bool wsa884x_readonly_register(struct device *dev, unsigned int reg)
> >> +{
> >> +	switch (reg) {

> > In general the read only registers probably shouldn't have defaults...

> For the case when regmap is being read before device enumerates (thus
> synced)?

If you're reading read only registers from the cache defaults like that
you may as well cut out the middle man and not bother parsing the
register at all - the value is just hard coded.  Either power up the
device to find out what the values are or use the values directly.

> >> +	/* Speaker mode by default */
> >> +	{ WSA884X_DRE_CTL_0, 0x7 << WSA884X_DRE_CTL_0_PROG_DELAY_SHIFT },
> >> +	{ WSA884X_CLSH_CTL_0, (0x37 & ~WSA884X_CLSH_CTL_0_DLY_CODE_MASK) |
> >> +			      (0x6 << WSA884X_CLSH_CTL_0_DLY_CODE_SHIFT) },
> >> +	{ WSA884X_CLSH_SOFT_MAX, 0xff },

> > Why not just leave as the chip default?

> Sincerely, I don't know. Without any documentation, I am relying
> entirely on downstream driver which has some code like this. I don't
> know whether some parts here make sense only for downstream case or
> shall be applicable also for upstream.

This sounds like someone hard coding their use case TBH.

Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)