On 6/6/16, 1:31 PM, "Mark Brown" <broonie@xxxxxxxxxx> wrote:
>On Fri, Jun 03, 2016 at 03:09:05PM -0500, Paul.Handrigan@xxxxxxxxxx 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.
>
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
