linux-kernel - Re: [PATCH v5] ASoC: Add MediaTek MT6660 Speaker Amp 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] [day] [month] [year] [list]

Message-ID: <20200113155332.GO3897@sirena.org.uk>
Date:   Mon, 13 Jan 2020 15:53:32 +0000
From:   Mark Brown <broonie@...nel.org>
To:     Jeff Chang <richtek.jeff.chang@...il.com>
Cc:     lgirdwood@...il.com, perex@...ex.cz, tiwai@...e.com,
        matthias.bgg@...il.com, alsa-devel@...a-project.org,
        linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
        jeff_chang@...htek.com
Subject: Re: [PATCH v5] ASoC: Add MediaTek MT6660 Speaker Amp Driver

On Fri, Jan 10, 2020 at 04:21:36PM +0800, Jeff Chang wrote:

> +	- rt,init_setting_num : The initial register setting element number.
> +
> +	- rt,init_setting_addr : the addreses array for INIT Setting table.
> +
> +	- rt,init_setting_mask : the mask array for INIT Setting table.
> +
> +	- rt,init_setting_val : the value array for INIT Setting table.

I'm not 100% clear what this is indended to be used for but given
that it's just raw register writes it's hard to see how it can be
used well in a general driver - it basically means we have no
idea how the chip might be configured in any given system which
*might* be fine but it could break some assumption the driver
makes about the state of the system on some particular OS or
version.  If there are some system specific tuning parameters
that need to be set as raw values it would be safer to do that by
just including the data in DT (eg, "X bytes of data to be written
starting at register Y to tune component Z" type stuff) or by
exposing individual parameters for things that are documentable.
Allowing a completely unconstrained sequence of register writes
doesn't seem like a good idea.

If possible it might be best to send a version of the driver that
doesn't let this stuff be configured at all then a patch on top
of that which configures all this stuff, the rest of the driver
looks good now.

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