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
Date: Mon, 13 Jan 2020 15:53:32 +0000 [thread overview]
Message-ID: <20200113155332.GO3897@sirena.org.uk> (raw)
In-Reply-To: <1578644496-16575-1-git-send-email-richtek.jeff.chang@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1363 bytes --]
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.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
prev parent reply other threads:[~2020-01-13 15:53 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-01-10 8:21 [PATCH v5] ASoC: Add MediaTek MT6660 Speaker Amp Driver Jeff Chang
2020-01-13 15:53 ` Mark Brown [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200113155332.GO3897@sirena.org.uk \
--to=broonie@kernel.org \
--cc=alsa-devel@alsa-project.org \
--cc=jeff_chang@richtek.com \
--cc=lgirdwood@gmail.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=matthias.bgg@gmail.com \
--cc=perex@perex.cz \
--cc=richtek.jeff.chang@gmail.com \
--cc=tiwai@suse.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).