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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <99a2f5bd-5b73-4f4e-95c6-8bc03ffed90c@sirena.org.uk>
Date: Wed, 28 May 2025 11:58:58 +0100
From: Mark Brown <broonie@...nel.org>
To: samuel.kayode@...oirfairelinux.com
Cc: Lee Jones <lee@...nel.org>, Rob Herring <robh@...nel.org>,
	Krzysztof Kozlowski <krzk+dt@...nel.org>,
	Conor Dooley <conor+dt@...nel.org>,
	Liam Girdwood <lgirdwood@...il.com>,
	Dmitry Torokhov <dmitry.torokhov@...il.com>,
	Sebastian Reichel <sre@...nel.org>, devicetree@...r.kernel.org,
	linux-kernel@...r.kernel.org, linux-input@...r.kernel.org,
	linux-pm@...r.kernel.org, eballetbo@...il.com, abelvesa@...ux.com,
	b38343@...escale.com, yibin.gong@....com,
	Abel Vesa <abelvesa@...nel.org>
Subject: Re: [PATCH v3 3/6] regulator: pf1550: add support for regulator

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@...escale.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.

Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)