On 6/6/16, 1:31 PM, "Mark Brown" broonie@kernel.org wrote:
On Fri, Jun 03, 2016 at 03:09:05PM -0500, Paul.Handrigan@cirrus.com wrote:
regmap_update_bits(cs35l33->regmap, CS35L33_CLK_CTL,
CS35L33_MCLKDIV2, CS35L33_MCLKDIV2);
cs35l33->mclk_int = freq/2;
Coding style, spaces around /.
Thanks!
if (CS35L33_VBSTMON_OVFL & sticky_val2)
dev_err(codec->dev,
"ERROR: VPMON Overflow Interrupt\n");
Cut'n'paste.
Thanks!
- /* 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.
It is just to ensure that the unmasked bits are cleared. We can remove this code all together.
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?
This really should be a warning. The device can be used without an interrupt pin. This should be devm_ as well.