Re: [PATCH] ASoC: Add support for NAU8824 codec to ASoC
From: Mark Brown
Date: Fri Mar 06 2015 - 16:07:40 EST
On Fri, Mar 06, 2015 at 03:28:33PM +0800, Chih-Chiang Chang wrote:
Please fix your mailer to word wrap within paragraphs, it makes things a
lot easier to read.
> On 2015/3/4 äå 08:55, Mark Brown wrote:
> > On Wed, Mar 04, 2015 at 08:35:52PM +0800, Chih-Chiang Chang wrote:
> >> On 2015/2/24 äå 10:13, Mark Brown wrote:
> > Add relevant control types if you need them, it's important to have
> > proper stereo controls available to userspace.
> We cannot find suitable macro in file "include\sound\soc.h", so we want to add below two macro for our chip.
> SOC_DOUBLE_L_R_VALUE
> SOC_DOUBLE_L_R_TLV
Sounds good.
> >>> This looks like you're reimplementing regmap's register sequence
> >>> stuff... It's also a *very* large sequence you have, are you sure it's
> >>> all required? It seems like this may be doing a bunch of machine
> >>> specific configuration but since it's all magic numbers it's hard to
> >>> tell.
> >> Initial settings are arranged in order
> > This doesn't answer or address my concern.
> These large number of register setting is used to initial our codec,
> and some of other codec have the same behavior. We will remove few
> unnecessary register default setting and add some remark for
> registers.
I'd really like to have a better understanding of what this is doing -
it can be valid to do this but there are some warning signs here such as
the volume of writes being large in comparison with the set of controls
the driver exposes which mean I'd like to be sure the use matches
expectations. Normally this sort of thing is a small number of fixes
for undocumented registers or updates to register defaults changed in
later revisions of the chip.
> > Don't include noise like this in upstream communication, if your company
> > won't fix this then please use an external mail account for upstream
> > communication.
> Our MIS report they have disabled to append message in mail. Hope you do not see it in this mail.
It's gone, thanks.
Attachment:
signature.asc
Description: Digital signature