On Mon, Sep 16, 2024 at 10:06:37PM +0200, Krzysztof Kozlowski wrote: > On 16/09/2024 18:48, André Draszik wrote: > > + /* INSW status */ > > + if ((status[3] & MAX20339_VINVALID) > > + && !(status[0] & MAX20339_VINVALID)) { > > + dev_warn(dev, "Vin over- or undervoltage\n"); > Same with all these. What happens if interrupt is triggered constantly? Logs on physical error conditions are a lot more appropriate than debug logs, they should basically never be triggered in normal operation and often it's a priorty to get information out about a failure in case someone might actually see something going wrong - especially with regulators, the system might be about to fall over if we're failing to regulate except in cases like SD cards. However in the case of the regulator API where you're telling the core about the error it's good to defer this to the core. We should probably be doing a better job here and logging something in the core. > > + if (val & MAX20339_LSWxSHORTFAULT) > > + *flags |= REGULATOR_ERROR_OVER_CURRENT; > > + > > + if (val & MAX20339_LSWxOVFAULT) > > + *flags |= REGULATOR_ERROR_OVER_VOLTAGE_WARN; > > + > > + if (val & MAX20339_LSWxOCFAULT) > > + *flags |= REGULATOR_ERROR_OVER_CURRENT; These should be notified to the core too, especially over voltage. > > + irq_flags = IRQF_ONESHOT | IRQF_SHARED; > Why shared? Why not? In general if a driver can support a shared interrupt it's polite for it to do so.
Attachment:
signature.asc
Description: PGP signature