[alsa-devel] [PATCH v3 1/2] ASoC: cs35l33: Initial commit of the cs35l33 CODEC driver.

[Paul Handrigan]

On Fri, 13 May 2016, Mark Brown wrote:

> On Fri, May 13, 2016 at 03:50:28PM -0500, Paul.Handrigan at cirrus.com wrote:
> 
> > +static struct regmap *cs35l33_get_regmap(struct device *dev)
> > +{
> > +	struct cs35l33_private *cs35l33 = dev_get_drvdata(dev);
> > +
> > +	return cs35l33->regmap;
> > +}
> 
> Doesn't the default work for you?
> 
I can remove this function.


> > +	for (i = 0; i < CS35L33_INT_ATTEMPTS; i++) {
> > +		/* ack the irq by reading both status registers */
> 
> Why do we need this...?

The we need to keep handling the unmasked interrupts until they are
all cleared.  We could get another interrupt while handling the first
triggered interrupt, but the interrupt line could be de-asserted due to
the read-to-clear nature of the interrupt status registers.
> 
> > +		if (sticky_val1 & CS35L33_AMP_SHORT) {
> > +			dev_err(codec->dev, "Amp short error\n");
> > +
>			if (!(current_val & CS35L33_AMP_SHORT)) {
> 
> dev_crit()

OK.

> 
> > +		if (sticky_val1 & CS35L33_OTE) {
> > +			dev_err(codec->dev, "Over temperature error\n");
> 
> 
> Same here - these are extremely serious and probably indicate very
> serious physical problems.

OK.

> 
> > +	} else if (IS_ERR(cs35l33->reset_gpio)) {
> > +	dev_err(&i2c_client->dev, "%s ERROR: Can't get reset GPIO\n",
> > +			__func__);
> > +		return PTR_ERR(cs35l33->reset_gpio);
> 
> Indentation is messed up here.

OK.  Thanks!
>