Re: [PATCH 4/5] power: sequencing: renesas-pwrrdy: Add RZ/V2H(P) and RZ/V2N A/D converter support

[Lad, Prabhakar]

Hi Bartosz,

Thank you for the review.

On Thu, Oct 8, 2026 at 11:10 AM Bartosz Golaszewski <brgl@xxxxxxxxxx> wrote:
>
> On Tue, 29 Sep 2026 14:24:06 +0200, Prabhakar <prabhakar.csengg@xxxxxxxxx> said:
> > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx>
> >
> > On RZ/V2H(P) and RZ/V2N, the SYS_ADC_CFG.SY_MSTP_ADA bit controls
> > A/D converter power-down. RZ/V2H(P) has a single A/D converter unit,
> > while RZ/V2N has three units sharing the same bit.
> >
> > Add an "adc" power sequencing target for these SoCs and match it to the
> > RZ/V2H(P) ADC compatible, which is also used as the fallback compatible
> > by RZ/V2N.
> >
> > Clear SY_MSTP_ADA when the first A/D converter unit is enabled and set it
> > again when the last unit is disabled. Wait 20 us after clearing the bit
> > before allowing conversion to start, ensuring ADCLK has been running for
> > the required time.
> >
> > Bind the power sequencer to the RZ/V2H(P) and RZ/V2N auxiliary devices
> > created by the SYS driver.
> >
> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx>
> > ---
> >  .../power/sequencing/pwrseq-renesas-pwrrdy.c  | 84 +++++++++++++++++++
> >  1 file changed, 84 insertions(+)
> >
> > diff --git a/drivers/power/sequencing/pwrseq-renesas-pwrrdy.c b/drivers/power/sequencing/pwrseq-renesas-pwrrdy.c
> > index 0190e53913bf..ab20b7ba9b83 100644
> > --- a/drivers/power/sequencing/pwrseq-renesas-pwrrdy.c
> > +++ b/drivers/power/sequencing/pwrseq-renesas-pwrrdy.c
> > @@ -5,6 +5,7 @@
> >   */
> >
> >  #include <linux/auxiliary_bus.h>
> > +#include <linux/delay.h>
> >  #include <linux/module.h>
> >  #include <linux/of.h>
> >  #include <linux/pwrseq/provider.h>
> > @@ -15,11 +16,68 @@
> >  #define SYS_PWRRDY_N_DSI_MASK        BIT(1)
> >  #define SYS_PWRRDY_N_CSI_MASK        BIT(2)
> >
> > +#define RZV2H_SYS_ADC_CFG            0x1600
> > +#define RZV2H_SYS_ADC_CFG_SY_MSTP_ADA        BIT(0)
> > +/* Minimum time with ADCLK running after leaving power-down mode */
> > +#define RZV2H_ADC_POWER_UP_US                20
> > +
> >  struct pwrseq_rz_info {
> >       int (*probe)(struct auxiliary_device *adev,
> >                    const struct auxiliary_device_id *id);
> >  };
> >
> > +static int pwrseq_rzv2h_adc_enable(struct pwrseq_device *pwrseq)
> > +{
> > +     struct regmap *regmap = pwrseq_device_get_drvdata(pwrseq);
> > +     int ret;
> > +
> > +     ret = regmap_clear_bits(regmap, RZV2H_SYS_ADC_CFG,
> > +                             RZV2H_SYS_ADC_CFG_SY_MSTP_ADA);
> > +     if (ret)
> > +             return ret;
> > +
> > +     fsleep(RZV2H_ADC_POWER_UP_US);
> > +
> > +     return 0;
> > +}
> > +
> > +static int pwrseq_rzv2h_adc_disable(struct pwrseq_device *pwrseq)
> > +{
> > +     struct regmap *regmap = pwrseq_device_get_drvdata(pwrseq);
> > +
> > +     return regmap_set_bits(regmap, RZV2H_SYS_ADC_CFG,
> > +                            RZV2H_SYS_ADC_CFG_SY_MSTP_ADA);
> > +}
> > +
> > +static const struct pwrseq_unit_data pwrseq_rzv2h_adc_unit = {
> > +     .name = "adc-power-sequence",
> > +     .enable = pwrseq_rzv2h_adc_enable,
> > +     .disable = pwrseq_rzv2h_adc_disable,
> > +};
> > +
> > +static const struct pwrseq_target_data pwrseq_rzv2h_adc_target = {
> > +     .name = "adc-power",
> > +     .unit = &pwrseq_rzv2h_adc_unit,
> > +};
> > +
> > +static const struct pwrseq_target_data *pwrseq_rzv2h_targets[] = {
> > +     &pwrseq_rzv2h_adc_target,
> > +     NULL
> > +};
> > +
> > +static const char * const pwrseq_rzv2h_compats[] = {
> > +     "renesas,r9a09g057-adc",
> > +     NULL
> > +};
> > +
> > +static int pwrseq_rzv2h_match(struct pwrseq_device *pwrseq, struct device *dev)
> > +{
> > +     if (of_device_compatible_match(dev->of_node, pwrseq_rzv2h_compats))
> > +             return PWRSEQ_MATCH_OK;
> > +
> > +     return PWRSEQ_NO_MATCH;
> > +}
> > +
> >  static int pwrseq_rzg3l_set_pwrrdy(struct pwrseq_device *pwrseq, u32 mask,
> >                                  bool val)
> >  {
> > @@ -127,6 +185,20 @@ static int pwrseq_rzg3l_pwrrdy_probe(struct auxiliary_device *adev,
> >       return PTR_ERR_OR_ZERO(devm_pwrseq_device_register(&adev->dev, &config));
> >  }
> >
> > +static int pwrseq_rzv2h_probe(struct auxiliary_device *adev,
> > +                           const struct auxiliary_device_id *id)
> > +{
> > +     struct pwrseq_config config = {
> > +             .parent = &adev->dev,
> > +             .owner = THIS_MODULE,
> > +             .drvdata = adev->dev.platform_data,
> > +             .match = pwrseq_rzv2h_match,
> > +             .targets = pwrseq_rzv2h_targets,
> > +     };
> > +
> > +     return PTR_ERR_OR_ZERO(devm_pwrseq_device_register(&adev->dev, &config));
> > +}
>
> This probe variant looks almost exactly the same as pwrseq_rzg3l_pwrrdy_probe().
> Can't you not just store the match and targets pointers in OF match
> data and keep
> the same code in top-level probe()?
>
Ok I will switch to the proposed approach and send a v2.

Cheers,
Prabhakar