[alsa-devel] [PATCH] ASoC: da9055: Partially fix device

Opensource [Adam Thomson] Adam.Thomson.Opensource at diasemi.com
Tue Jan 14 12:04:22 CET 2014


On Mon, 13 Jan 2014 21:19:06 +0000, Mark Brown wrote:

> We also get people complaining about other stylistic things that they
> get asked to fix and people going round fixing style issues - the whole
> reason this broke is that people noticed and fixed the fact that the
> drivers looked wrong to inspection without realising that there were
> multiple drivers for the device because it's not done in the manner we
> usually do this (that's definitely why the CODEC driver was changed, I
> picked up on it during review on submission).  It's really useful to
> have things follow common patterns, it makes maintianing the system
> easier and it makes factoring out shareable code easier.  Things like
> interfaces to the rest of the system tend to get particular focus.
> 
> Please, this isn't helping...

I don't disagree with you on common patterns, and I have been an advocate of
following these when working in the kernel as it generally makes things simpler
and easier. Here I'm not trying to be awkward but am trying to get a simple,
flexible, solution which fits best with our devices, whilst still using common
kernel frameworks.
 
> > Well, the truth is that there are default I2C addresses for both PMIC and Codec
> > but as they are standalone devices, albeit in one package, both addresses can
> > be changed. This is one more reason why I don't believe we should be taking your
> > method of implementation here. If you just make sure PMIC and Codec drivers
> 
> ...but on the other hand this sounds like an actual technical issue
> which could be a good reason to do something different - it's a bit
> surprising that it didn't come up before.  What exactly is the
> flexibility the devices have here?  It's a pretty unusual hardware
> design.

The PMIC is configurable via a register in its register map, and the Codec is
configurable via an OTP register, which a customer is able to set themselves
through a series of register writes, in the Codec register map. When you
mentioned the i2c_add_dummy() approach again in your last mail it caused me to
go over your patch once more, and in doing so I then spotted this. I do wish I
had picked this out earlier though as it may have saved some time in e-mail
discussions.


More information about the Alsa-devel mailing list