linux-kernel - Re: [PATCH v3 1/2] regulator: rt5133: Add RT5133 PMIC regulator Support

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: <b4b3dde0-18a5-40d3-8cd2-53e4cdbd3e69@sirena.org.uk>
Date: Mon, 21 Jul 2025 13:18:58 +0100
From: Mark Brown <broonie@...nel.org>
To: jeff_chang@...htek.com
Cc: lgirdwood@...il.com, robh@...nel.org, krzk+dt@...nel.org,
	conor+dt@...nel.org, linux-kernel@...r.kernel.org,
	devicetree@...r.kernel.org
Subject: Re: [PATCH v3 1/2] regulator: rt5133: Add RT5133 PMIC regulator
 Support

On Mon, Jul 21, 2025 at 02:01:34PM +0800, jeff_chang@...htek.com wrote:

> RT5133 is an intefrated chip. It includes 8 LDOs and 3 GPOs that
> can be used to drive output high/low purpose. The dependency of the
> GPO block is internally LDO1 Voltage. If LDO1 voltage output disabled,
> GPO cannot be used to drive output high. It need to pay more attention
> on the usage.

This looks mostly fine, a couple of very small nits below:

> +static irqreturn_t rt5133_intr_handler(int irq_number, void *data)
> +{
> +	struct rt5133_priv *priv = data;
> +	u32 intr_evts = 0, handle_evts;
> +	int i, ret;
> +
> +	ret = regmap_bulk_read(priv->regmap, RT5133_REG_BASE_EVT, &intr_evts,
> +			       RT5133_INTR_BYTE_NR);
> +	if (ret) {
> +		dev_err(priv->dev, "%s, read event failed\n", __func__);
> +		return IRQ_HANDLED;
> +	}
> +
> +	handle_evts = intr_evts & RT5133_BASE_EVT_MASK;

> +
> +	ret = regmap_bulk_write(priv->regmap, RT5133_REG_BASE_EVT, &intr_evts,
> +				RT5133_INTR_BYTE_NR);
> +	if (ret)
> +		dev_err(priv->dev, "%s, clear event failed\n", __func__);
> +
> +	return IRQ_HANDLED;

If there weren't any events flagged that we handled we should return
IRQ_NONE here, this supports shared interrupts and lets the error
handling in the interrupt core handle errors where the interrupt line
gets stuck.

> +// Due to crc, cache is not necessary
> +static const struct regmap_config rt5133_regmap_config = {
> +	.reg_bits = 8,
> +	.val_bits = 8,
> +	.max_register = RT5133_REG_LDO8_CTRL4,
> +};

Might be worth enabling a cache?

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