Re: [alsa-devel] [PATCH] ASoC: pcm512x: Add PCM512x driver

[Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>]

[-- Attachment #1: Type: text/plain, Size: 1651 bytes --]

On Thu, Feb 06, 2014 at 02:24:07PM +0100, Lars-Peter Clausen wrote:
> On 02/06/2014 01:26 PM, Mark Brown wrote:

> >+static const char *pcm512x_dsp_program_texts[] = {

> Nitpick: should be "static const char * const ...", a immutable
> array of pointers pointing to immutable strings. Same for all the
> enum texts in this driver.

Oh, good - someone fixed the core code for that without me noticing.
Last time I looked if you made the strings const there were annoying
warnings since they got passed in to things that weren't expecting const
strings and it seemed to annoying to check that it was actually safe in
all cases.

> Another reason why I think it is better to separate the I2C and SPI
> bits into different modules. If the registration of the SPI driver
> fails you'll return an error and the module will not be loaded. At
> the same time the i2c driver is already registered. I know that this
> is rather theoretical, but if we don't care if our error handling is
> correct, because we assume that the error will never happen, we do
> not need error handling at all.

Like I say I'm perfectly aware of the issues, I just don't have much
enthusiasm myself since they pretty much only affect unrealistic and
apparently quite rare randconfigs - the time I might spend on that is
more likely to get spent on things like the format negotiation stuff.
If someone (you? :P) wants to do the work that's fine but nobody had
shown any sign of that until your driver the other day.  

I do also want to see some explicit conversion happening so it's clearer
that this is a deliberate change that's being rolled out.

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