On 7/5/21 2:20 PM, Mark Brown wrote:
Thanks for the review. I believe I can fix all of these in the next
revision.
On Fri, Jul 02, 2021 at 03:51:26PM -0500, David Rhodes wrote:
+ struct regulator_bulk_data supplies[2];
+ int num_supplies;
Why might the number of supplies vary?
Good point. This is set by the size of a const array so I will replace
this with a #define.
+ /* Check to see if unmasked bits are active */
+ if (!(status[0] & ~masks[0]) && !(status[1] & ~masks[1]) &&
+ !(status[2] & ~masks[2]) && !(status[3] & ~masks[3]))
+ return IRQ_NONE;
+ }
+
+ return IRQ_HANDLED;
+}
This seems to handle any asserted interrupt, is it clear on read?
Theoretically every unmasked interrupt source should have a handler
here, so the function should have exited early if there was nothing handled.
I think it's easier to read and more explicitly enforced if I change it
so that the return code is only set to IRQ_HANDLED inside the conditions
where the bits are actually cleared.
+ case SND_SOC_DAPM_POST_PMD:
+ regmap_read(cs35l41->regmap, CS35L41_PWR_CTRL1, &val);
+ if (val & CS35L41_GLOBAL_EN_MASK) {
+ regmap_update_bits(cs35l41->regmap, CS35L41_PWR_CTRL1,
+ CS35L41_GLOBAL_EN_MASK, 0);
I can't see any references to GLOBAL_EN outside this function, why might
it not be set?
This check prevents an incorrect 'PDN Failed' message if DAPM
initializes and turns the widget off at boot (with no prior power-up).
+ ret = devm_request_threaded_irq(cs35l41->dev, cs35l41->irq, NULL,
+ cs35l41_irq, IRQF_ONESHOT | IRQF_SHARED | irq_pol,
+ "cs35l41", cs35l41);
+
+ /* CS35L41 needs INT for PDN_DONE */
+ if (ret != 0) {
+ dev_err(cs35l41->dev, "Failed to request IRQ: %d\n", ret);
+ ret = -ENODEV;
+ goto err;
+ }
+
+ /* Set interrupt masks for critical errors */
+ regmap_write(cs35l41->regmap, CS35L41_IRQ1_MASK1,
+ CS35L41_INT1_MASK_DEFAULT);
Shouldn't this be configured prior to requesting interrupts or might
there be a race?
Should not be any problems with unmasking before the request as long as
the pin configuration is also done beforehand.
Thanks,
David
