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

[Mark Brown <broonie@kernel.org>]

[-- 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 --]