Re: [PATCH 2/2] regulator: max20339: add Maxim MAX20339 regulator driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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


[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux