Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Mon, 13 Jan 2020 15:53:32 +0000
From:   Mark Brown <broonie@kernel.org>
To:     Jeff Chang <richtek.jeff.chang@gmail.com>
Cc:     lgirdwood@gmail.com, perex@perex.cz, tiwai@suse.com,
        matthias.bgg@gmail.com, alsa-devel@alsa-project.org,
        linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org,
        jeff_chang@richtek.com
Subject: Re: [PATCH v5] ASoC: Add MediaTek MT6660 Speaker Amp Driver
Message-ID: <20200113155332.GO3897@sirena.org.uk>
References: <1578644496-16575-1-git-send-email-richtek.jeff.chang@gmail.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
        protocol="application/pgp-signature"; boundary="/TUrtqMIkCP4YtJm"
Content-Disposition: inline
In-Reply-To: <1578644496-16575-1-git-send-email-richtek.jeff.chang@gmail.com>
User-Agent: Mutt/1.10.1 (2018-07-13)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk


--/TUrtqMIkCP4YtJm
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

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.

--/TUrtqMIkCP4YtJm
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQEzBAABCgAdFiEEreZoqmdXGLWf4p/qJNaLcl1Uh9AFAl4cknsACgkQJNaLcl1U
h9BTeAf8C55cmcfxY+HHqvC1iVVcYfUzf1wTdh3zed7cpd624wm6yrjvr2UzzcR4
t/LEvnTIeZTPFvorQNMaZLDM5KWU8YWrll6SM6deiXo2dv5uAFzs2XGY+eRvTnN3
HjcX+dnQlvd7ZCekd/b7M03IhNGIx5VY3QDDhbDTDrHA26wpil5GT3seJhV0yXsM
w1+ITl4WshJ1kDwE8+jIBG5j7FfR/4ig3ZDb2iP2frIKn7r12AJDlQXfM/pf3vpm
IXFs+JX8tyuHEU35AyG1vMOoReF0/UDGvgKI0m0hiV7E9W37FHfB+bJS64sSmnTv
4EUo2bTKPCiJv7BxPJVngMSK1D+qeA==
=xYed
-----END PGP SIGNATURE-----

--/TUrtqMIkCP4YtJm--