Re: [PATCH v3 3/6] regulator: pf1550: add support for regulator

Mark Brown
2025-05-28T10:59:04Z

[-- Attachment #1: Type: text/plain, Size: 1866 bytes --]

On Tue, May 27, 2025 at 06:25:35PM -0400, Samuel Kayode via B4 Relay wrote:

> @@ -0,0 +1,353 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * pf1550.c - regulator driver for the PF1550

Please make the entire comment a C++ one so things look more
intentional.

> + *
> + * Copyright (C) 2016 Freescale Semiconductor, Inc.
> + * Robin Gong <yibin.gong@freescale.com>

Presumably there's been some updates since then?

> +static int pf1550_set_ramp_delay(struct regulator_dev *rdev, int ramp_delay)
> +{
> + int id = rdev_get_id(rdev);
> + unsigned int ramp_bits;
> + int ret;
> +
> + if (id > PF1550_VREFDDR)
> + return -EACCES;
> +
> + ramp_delay = 6250 / ramp_delay;
> + ramp_bits = ramp_delay >> 1;
> + ret = regmap_update_bits(rdev->regmap, rdev->desc->vsel_reg + 4, 0x10,
> + ramp_bits << 4);

Shouldn't we validate the value of ramp_delay?

> +static irqreturn_t pf1550_regulator_irq_handler(int irq, void *data)
> +{

> + switch (irq_type) {
> + case PF1550_PMIC_IRQ_SW1_LS:
> + case PF1550_PMIC_IRQ_SW2_LS:
> + case PF1550_PMIC_IRQ_SW3_LS:
> + dev_info(dev, "lowside interrupt triggered! irq_type=%d\n",
> + irq_type);
> + break;
> + case PF1550_PMIC_IRQ_SW1_HS:
> + case PF1550_PMIC_IRQ_SW2_HS:
> + case PF1550_PMIC_IRQ_SW3_HS:
> + dev_info(dev, "highside interrupt triggered! irq_type=%d\n",
> + irq_type);
> + break;

Are these under and overvoltage events which should be reported as such?

> + case PF1550_PMIC_IRQ_LDO1_FAULT:
> + case PF1550_PMIC_IRQ_LDO2_FAULT:
> + case PF1550_PMIC_IRQ_LDO3_FAULT:
> + dev_info(dev, "ldo fault triggered! irq_type=%d\n", irq_type);
> + break;

Similarly, we can report error events.

> + case PF1550_PMIC_IRQ_TEMP_110:
> + case PF1550_PMIC_IRQ_TEMP_125:
> + dev_info(dev, "thermal exception triggered! irq_type=%d\n",
> + irq_type);
> + break;

We also have an over temperature event type.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]