Hi Krzysztof,
On Mon, 2024-09-16 at 22:06 +0200, Krzysztof Kozlowski wrote:
> On 16/09/2024 18:48, André Draszik wrote:
> > The MAX20339 is an overvoltage protection (OVP) device which also
> > integrates two load switches with resistor programmable current
> > limiting and includes ESD protection for the USB Type-C signal pins.
> >
> > This driver exposes the main path and the two the load switches via the
>
> ...
>
> > +
> > +
> > +static irqreturn_t max20339_irq(int irqno, void *data)
> > +{
> > + struct max20339_irq_data *max20339 = data;
> > + struct device *dev = max20339->dev;
> > + struct regmap *regmap = max20339->regmap;
> > + u8 status[6];
> > + int ret;
> > +
> > + ret = regmap_bulk_read(regmap, MAX20339_STATUS1, status,
> > + ARRAY_SIZE(status));
> > + if (ret) {
> > + dev_err(dev, "Failed to read IRQ status: %d\n", ret);
> > + return IRQ_NONE;
> > + }
> > +
> > + dev_dbg(dev,
> > + "INT1 2 3: %#.2x %#.2x %#.2x STATUS1 2 3: %#.2x %#.2x %#.2x\n",
> > + status[3], status[4], status[5], status[0], status[1],
> > + status[2]);
>
> You should not have prints, even debugs, in interrupt handlers. This can
> flood the dmesg.
>
> > +
> > + if (!status[3] && !status[4] && !status[5])
> > + return IRQ_NONE;
> > +
> > + /* overall status */
> > + if (status[3] & status[0] & MAX20339_THMFAULT) {
> > + dev_warn(dev, "Thermal fault\n");
> > + for (int i = 0; i < ARRAY_SIZE(max20339->rdevs); ++i)
> > + regulator_notifier_call_chain(max20339->rdevs[i],
> > + REGULATOR_EVENT_OVER_TEMP,
> > + NULL);
> > + }
> > +
> > + /* 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?
You unplug the USB cable :-), but yes, I'll come up with something better.
> [...]
> > +}
> > +
> > +static int max20339_lsw_dt_parse(struct device_node *np,
> > + const struct regulator_desc *desc,
> > + struct regulator_config *cfg)
> > +{
> > + struct max20339_regulator *data = cfg->driver_data;
> > +
> > + /* we turn missing properties into a fatal issue during probe() */
>
> Your binding does not look in sync with above.
Do you mean it doesn't enforce existence of this property? (It does and
binding check appropriately complains if it's missing). Otherwise, can
you please point me to the problem you're seeing?
