Re: [PATCH 4/5] ASoC: codecs: Add AB8500 codec-driver

[Mark Brown <broonie@opensource.wolfsonmicro.com>]

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

On Wed, May 30, 2012 at 03:49:51PM +0200, Ola Lilja wrote:
> On 05/30/2012 03:14 PM, Mark Brown wrote:

> > What problems are you finding when you try to use the debug
> > infrastructure in both the regulator API and DAPM to discover the state
> > of the regulators?

> My vision here is that in a simple way, in one place, activate all
> debug-information we need in our driver, prefixed with our dev_xxx. This is very
> valuable for us when debugging, especially when a customer is told to activate
> debug-information that we can use to debug.
> I removed the menuconfig flag on your request, and then we lost the information
> for regulators and clocks when I implemented the clock/regulator-widgets. I'm
> just trying to keep some aspects of what we want to have but still conforming
> what you want to see on mainline.

All the existing diagnostics use dev_name() too...  just turning on the
standard logging ought to give you exactly the same stuff, indeed if
debugfs is on there should be no need to even manually enable equivalent
debugging.

> >> +int ab8500_audio_init_audioblock(struct snd_soc_codec *codec)

> > static.  Lots of other functions in the rest of the driver have the same
> > issue.

> This one should be static, yes. Cannot find any other non-static functions in
> the codec-driver that is missing static.

IIRC the function immediately below was one.

> >> +#define PRE_PMU_POST_PMD			(SND_SOC_DAPM_PRE_PMU | \
> >> +						SND_SOC_DAPM_POST_PMD)

> > You shouldn't define stuff like this in your driver!

> This was mainly to avoid impossible situations trying to comply with the
> 80-char-width.

Adding things like this if they're useful are fine but they shouldn't be
driver specific things.  There is a SND_SOC_DAPM_PRE_POST_PMD already
though.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

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