Re: [PATCH v2 2/3] power: sequencing: Add Renesas RZ/G3L Power Ready driver

On Mi, 2026-07-29 at 10:31 +0100, Biju wrote:
> From: Biju Das <[email protected]>
> 
> Add a power sequencing driver for the Renesas RZ/G3L PWRRDY module,
> which signals power readiness for various IPs (USB, DSI, CSI etc.) on the
> SoC. The driver binds as an auxiliary device to the parent SYSC driver,
> using its regmap to toggle the SYS_PWRRDY_N register bits, and exposes
> {usb,dsi,csi}-pwrrdy pwrseq targets.
> 
> Signed-off-by: Biju Das <[email protected]>
> ---
> v1->v2:
>  * Added a comment in pwrseq_rzg3l_pwrrdy_match().
>  * Dropped dev_get_regmap() from probe as regmap is now part of platform
>    data.
>  * Added a blank line before devm_pwrseq_device_register() in probe.
>  * Dropped the error message devm_pwrseq_device_register() as probe prints
>    failure message.
>  * Dropped local variables dev and regmap from probe().
> ---
>  drivers/power/sequencing/Kconfig              |   8 +
>  drivers/power/sequencing/Makefile             |   1 +
>  .../power/sequencing/pwrseq-renesas-pwrrdy.c  | 139 ++++++++++++++++++
>  3 files changed, 148 insertions(+)
>  create mode 100644 drivers/power/sequencing/pwrseq-renesas-pwrrdy.c
> 
[...]
> diff --git a/drivers/power/sequencing/pwrseq-renesas-pwrrdy.c b/drivers/power/sequencing/pwrseq-renesas-pwrrdy.c
> new file mode 100644
> index 000000000000..723d574c3f10
> --- /dev/null
> +++ b/drivers/power/sequencing/pwrseq-renesas-pwrrdy.c
> @@ -0,0 +1,139 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Renesas RZ/G3L Power Ready driver
> + *
> + */
> +
> +#include <linux/auxiliary_bus.h>
> +#include <linux/module.h>
> +#include <linux/pwrseq/provider.h>
> +#include <linux/regmap.h>
> +
> +#define SYS_PWRRDY_N		0xd70
> +#define SYS_PWRRDY_N_USB_MASK	BIT(0)
> +#define SYS_PWRRDY_N_DSI_MASK	BIT(1)
> +#define SYS_PWRRDY_N_CSI_MASK	BIT(2)
> +
> +static int pwrseq_rzg3l_set_pwrrdy(struct pwrseq_device *pwrseq, u32 mask, u32 val)
> +{
> +	struct regmap *regmap = pwrseq_device_get_drvdata(pwrseq);
> +
> +	return regmap_update_bits(regmap, SYS_PWRRDY_N, mask, val);
> +}

Did you mean to use regmap_assign_bits() with a boolean val?
See below.

> +static int pwrseq_rzg3l_usb_pwrrdy_enable(struct pwrseq_device *pwrseq)
> +{
> +	return pwrseq_rzg3l_set_pwrrdy(pwrseq, SYS_PWRRDY_N_USB_MASK, 0);
> +}
> +
> +static int pwrseq_rzg3l_usb_pwrrdy_disable(struct pwrseq_device *pwrseq)
> +{
> +	return pwrseq_rzg3l_set_pwrrdy(pwrseq, SYS_PWRRDY_N_USB_MASK, 1);

This happens to work correctly since SYS_PWRRDY_N_USB_MASK is 1.

> +}
> +
> +static const struct pwrseq_unit_data pwrseq_rzg3l_usb_pwrrdy_unit = {
> +	.name = "usb-pwrrdy-power-sequence",
> +	.enable = pwrseq_rzg3l_usb_pwrrdy_enable,
> +	.disable = pwrseq_rzg3l_usb_pwrrdy_disable,
> +};
> +
> +static int pwrseq_rzg3l_dsi_pwrrdy_enable(struct pwrseq_device *pwrseq)
> +{
> +	return pwrseq_rzg3l_set_pwrrdy(pwrseq, SYS_PWRRDY_N_DSI_MASK, 0);
> +}
> +
> +static int pwrseq_rzg3l_dsi_pwrrdy_disable(struct pwrseq_device *pwrseq)
> +{
> +	return pwrseq_rzg3l_set_pwrrdy(pwrseq, SYS_PWRRDY_N_DSI_MASK, 1);

1 is not in SYS_PWRRDY_N_DSI_MASK, so this will clear the bit. Either
set val = mask or use regmap_assign_bits().

> +}
> +
> +static const struct pwrseq_unit_data pwrseq_rzg3l_dsi_pwrrdy_unit = {
> +	.name = "dsi-pwrrdy-sequence",
> +	.enable = pwrseq_rzg3l_dsi_pwrrdy_enable,
> +	.disable = pwrseq_rzg3l_dsi_pwrrdy_disable,
> +};
> +
> +static int pwrseq_rzg3l_csi_pwrrdy_enable(struct pwrseq_device *pwrseq)
> +{
> +	return pwrseq_rzg3l_set_pwrrdy(pwrseq, SYS_PWRRDY_N_CSI_MASK, 0);
> +}
> +
> +static int pwrseq_rzg3l_csi_pwrrdy_disable(struct pwrseq_device *pwrseq)
> +{
> +	return pwrseq_rzg3l_set_pwrrdy(pwrseq, SYS_PWRRDY_N_CSI_MASK, 1);

Same issue as above.

[...]
> +static int pwrseq_rzg3l_pwrrdy_match(struct pwrseq_device *pwrseq,
> +				     struct device *dev)
> +{
> +	/*
> +	 * The pwrrdy provider does not rely on any device-tree phandle/property
> +	 * matching, so it unconditionally matches every consumer device that
> +	 * requests it.
> +	 */
> +	return PWRSEQ_MATCH_OK;
> +}
> +
> +static int pwrseq_rzg3l_pwrrdy_probe(struct auxiliary_device *adev,
> +				     const struct auxiliary_device_id *id)
> +{
> +	struct pwrseq_device *pwrseq;
> +	struct pwrseq_config config = {
> +		.parent = &adev->dev,
> +		.owner = THIS_MODULE,
> +		.drvdata = adev->dev.platform_data,
> +		.match = pwrseq_rzg3l_pwrrdy_match,
> +		.targets = pwrseq_rzg3l_pwrrdy_targets,
> +	};
> +
> +	pwrseq = devm_pwrseq_device_register(&adev->dev, &config);
> +
> +	return IS_ERR(pwrseq) ? PTR_ERR(pwrseq) : 0;

	return PTR_ERR_OR_ZERO(pwrseq);


regards
Philipp