Re: [PATCH] ASoC: rt1308: Add RT1308 amplifier driver

From: Mark Brown <broonie@kernel.org>
To: "Derek [方德義]" <derek.fang@realtek.com>
Cc: "Oder Chiou" <oder_chiou@realtek.com>,
	"Jack Yu" <jack.yu@realtek.com>,
	"alsa-devel@alsa-project.org" <alsa-devel@alsa-project.org>,
	"lars@metafoo.de" <lars@metafoo.de>,
	"lgirdwood@gmail.com" <lgirdwood@gmail.com>,
	"Albert Chen" <albertchen@realtek.com>,
	"bard.liao@intel.com" <bard.liao@intel.com>,
	"Shuming [范書銘]" <shumingf@realtek.com>,
	"Flove(HsinFu)" <flove@realtek.com>
Subject: Re: [PATCH] ASoC: rt1308: Add RT1308 amplifier driver
Date: Wed, 17 Apr 2019 16:05:18 +0100	[thread overview]
Message-ID: <20190417150518.GB5703@sirena.org.uk> (raw)
In-Reply-To: <BC6F7617C38F3E4E8CA887E07B35B8280116192E@RTITMBSVM07.realtek.com.tw>

[-- Attachment #1.1: Type: text/plain, Size: 1167 bytes --]

On Wed, Apr 17, 2019 at 11:40:28AM +0000, Derek [方德義] wrote:

Please fix your mail client to word wrap within paragraphs at something
substantially less than 80 columns.  Doing this makes your messages much
easier to read and reply to.

> > The ones here with comments look an awful lot like they're system specific config
> > which should be left up to either platform data (or DT/ACPI configuration) or
> > done at runtime (like gains).  Some of the others like the pads configuration
> > might also fit into that.

> We don't open the DA gain setting for user at runtime, so have to set
> a default level in the init list. 

That's not the way Linux drivers generally work...  is there some
technical reason for that or is it just some kind of policy decision?

> > > +static void rt1308_reset(struct regmap *regmap) {
> > > +	regmap_write(regmap, RT1308_RESET, 0); }
> > 
> > If we do need the init list shouldn't we also do a reninit every time we do a reset?

> The init list is only set at driver probe, not every time after resetting.

This is what I'm pointing out; if you don't restore these values then
why set them at all?

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]