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

[Mark Brown]

On Fri, Jun 03, 2016 at 03:09:05PM -0500, Paul.Handrigan at cirrus.com wrote:

> +		regmap_update_bits(cs35l33->regmap, CS35L33_CLK_CTL,
> +			CS35L33_MCLKDIV2, CS35L33_MCLKDIV2);
> +			cs35l33->mclk_int = freq/2;

Coding style, spaces around /.

> +       if (CS35L33_VBSTMON_OVFL & sticky_val2)
> +               dev_err(codec->dev,
> +                       "ERROR: VPMON Overflow Interrupt\n");

Cut'n'paste.

> +	/* Make sure that the bits are cleared */
> +	for (i = 0; i < CS35L33_INT_ATTEMPTS; i++) {
> +		regmap_read(cs35l33->regmap, CS35L33_INT_STATUS_1,
> +			&sticky_val1);
> +		regmap_read(cs35l33->regmap, CS35L33_INT_STATUS_2,
> +			&sticky_val2);
> +
> +		if (!(sticky_val1 & ~mask1) && !(sticky_val2 & ~mask2))
> +			break;
> +	}

I still don't understand this.  If these interrupts are clear on read
(which seems to be the case since we don't ack anything?) then these
repeated reads are obviously just going to ignore interrupts.  I really
hope that the masking also applies to the clear on read behaviour but
that's a separate thing.

I'm guessing the hardware is just broken here but it's not clear to me
that just letting the interrupt fire a couple of extra times isn't the
best solution and we need a clear explanation of what this is trying to
accomplish and why so that the code is maintainable.

> +       ret = request_threaded_irq(i2c_client->irq, NULL, cs35l33_irq_thread,
> +                       IRQF_ONESHOT | IRQF_TRIGGER_LOW, "cs35l33", cs35l33);
> +       if (ret != 0)
> +               dev_err(&i2c_client->dev, "Failed to request IRQ: %d\n", ret);

But otherwise ignore the error?
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: not available
URL: <http://mailman.alsa-project.org/pipermail/alsa-devel/attachments/20160606/f6e71297/attachment.sig>