Hi Tommaso, Thanks for the feedback.
> -----Original Message----- > From: Tommaso Merciai <[email protected]> > Sent: 27 August 2026 11:29 > Subject: Re: [PATCH v3 04/13] drm: renesas: rzg2l_mipi_dsi: Add support for > DSI PWRRDY > > Hi Biju, > Thanks for your patch. > > On Wed, Aug 26, 2026 at 06:48:38PM +0100, Biju wrote: > > From: Biju Das <[email protected]> > > > > The register for controlling power to the DSI region is in the SYSC > > (System Controller) block. Add support for controlling the DSI PWRRDY > > signal so the driver can efficiently manage power to the DSI region. > > > > Signed-off-by: Biju Das <[email protected]> > > --- > > v2->v3: > > * Updated powerseq API names. > > v1->v2: > > * Switched to power sequence for handling pwrrdy signal. > > * Dropped header file regmap.h and mfd/syscon.h > > * Added header file pwrseq/consumer.h > > --- > > .../gpu/drm/renesas/rz-du/rzg2l_mipi_dsi.c | 38 +++++++++++++++++++ > > 1 file changed, 38 insertions(+) > > > > diff --git a/drivers/gpu/drm/renesas/rz-du/rzg2l_mipi_dsi.c > > b/drivers/gpu/drm/renesas/rz-du/rzg2l_mipi_dsi.c > > index d67e1230b70c..7df5f4a4161b 100644 > > --- a/drivers/gpu/drm/renesas/rz-du/rzg2l_mipi_dsi.c > > +++ b/drivers/gpu/drm/renesas/rz-du/rzg2l_mipi_dsi.c > > @@ -18,6 +18,7 @@ > > #include <linux/of_graph.h> > > #include <linux/platform_device.h> > > #include <linux/pm_runtime.h> > > +#include <linux/pwrseq/consumer.h> > > #include <linux/reset.h> > > #include <linux/slab.h> > > #include <linux/units.h> > > @@ -63,6 +64,7 @@ struct rzg2l_mipi_dsi_hw_info { > > unsigned long max_dclk; > > u16 activation_dly; > > u8 features; > > + bool pwrrdy; > > }; > > > > struct rzv2h_dsi_mode_calc { > > @@ -87,6 +89,8 @@ struct rzg2l_mipi_dsi { > > struct clk *vclk; > > struct clk *lpclk; > > > > + struct pwrseq_desc *pwrseq; > > + > > enum mipi_dsi_pixel_format format; > > unsigned int num_data_lanes; > > unsigned int lanes; > > @@ -1396,6 +1400,36 @@ static const struct dev_pm_ops rzg2l_mipi_pm_ops = { > > * Probe & Remove > > */ > > > > +static void rzg2l_mipi_dsi_pwrrdy_off(void *data) { > > + pwrseq_disable(data); > > +} > > + > > +static int rzg2l_mipi_dsi_pwrrdy_init(struct rzg2l_mipi_dsi *dsi) { > > + int ret; > > + > > + if (!dsi->info->pwrrdy) > > Maybe better to introduce a new feature flag like: > > RZ_MIPI_DSI_FEATURE_PWRRDY Ok. > > And drop bool pwrrdy from rzg2l_mipi_dsi_hw_info? > > > + return 0; > > + > > + dsi->pwrseq = devm_pwrseq_get(dsi->dev, "dsi-pwrrdy"); > > + if (IS_ERR(dsi->pwrseq)) { > > + /* > > + * This platform requires a sequencer. If we can't get it, we > > + * must return the error (including -EPROBE_DEFER to wait for > > + * the provider to appear) > > + */ > > + return dev_err_probe(dsi->dev, PTR_ERR(dsi->pwrseq), > > + "Failed to get required power > > sequencer\n"); > > + } > > + > > + ret = pwrseq_enable(dsi->pwrseq); > > + if (ret) > > + return ret; > > + > > + return devm_add_action_or_reset(dsi->dev, rzg2l_mipi_dsi_pwrrdy_off, > > +dsi->pwrseq); > > I think we can drop devm_add_action_or_reset() as devm_pwrseq_get() already > register devm_pwrseq_put() -> > pwrseq_put() which already call pwrseq_disable() if desc->powered_on. > > Then we can drop rzg2l_mipi_dsi_pwrrdy_off(). OK will fix this in next version. Cheers, Biju
