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

Hi Bartosz Golaszewski,

Thanks for the feedback.

> -----Original Message-----
> From: Bartosz Golaszewski <[email protected]>
> Sent: 28 July 2026 15:22
> Subject: Re: [PATCH 2/3] power: sequencing: Add Renesas RZ/G3L Power Ready driver
> 
> On Tue, 28 Jul 2026 13:55:22 +0200, Biju Das <[email protected]> said:
> > Hi Bartosz Golaszewski,
> >
> >> -----Original Message-----
> >> From: Bartosz Golaszewski <[email protected]>
> >> Sent: 28 July 2026 12:35
> >> Subject: Re: [PATCH 2/3] power: sequencing: Add Renesas RZ/G3L Power
> >> Ready driver
> >>
> >> On Tue, 28 Jul 2026 11:17:58 +0200, Biju Das <[email protected]> said:
> >> > Hi Bartosz Golaszewski,
> >> >
> >> > Thanks for the feedback.
> >> >
> >>
> >> ...
> >>
> >> >> > +
> >> >> > +static int pwrseq_rzg3l_pwrrdy_match(struct pwrseq_device *pwrseq,
> >> >> > +				     struct device *dev)
> >> >> > +{
> >> >> > +	return PWRSEQ_MATCH_OK;
> >> >>
> >> >> When I see an always-tru match() callback, it always raises an alarm bell.
> >> >> Typically, I'd expect there to be some validation of the consumer happening.
> >> >>
> >> >> Please at least provide an explanation of why it's ok.
> >> >
> >> > Without the match function the driver is not probing.
> >> >
> >>
> >> Yes, that's expected.
> >>
> >> > sysc(driver) ->powerseq(driver) ->{DSI,CSI,USB}(driver) there is no
> >> > property that I can match here as the resource is a register on the
> >> > SYSC IP and powerseq driver is instantiated by the SYSC driver.
> >> >
> >> > Maybe I can provide a comment here saying that??
> >> > "No DT property to match here, so it is OK always"
> >> >
> >>
> >> So the sysc is the parent of the pwrseq provider and the CSI, DSI and
> >> USB drivers are the consumers of the pwrseq descriptor? The latter
> >> are children of the platform bus, am I getting this right? Can you point me to the device tree that
> describes it?
> >
> > Yes,
> >
> > Please see [1], [2], [3]
> >
> > [1] sysc node
> >
> > [2] phyrst node (I planned to remove renesas,sysc-pwrrdy in the next
> > version as it is going to use pwrseq)
> >
> > [3] dsi node (I planned to remove renesas,sysc-pwrrdy in the next
> > version as it is going to use pwrseq)
> >
> > [1]
> > https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tr
> > ee/arch/arm64/boot/dts/renesas/r9a08g046.dtsi?h=next-20260727#n609
> >
> > [2]
> > https://lore.kernel.org/all/TY3PR01MB11346FA21EFADC8A3D53D81BE86CF2@TY
> > 3PR01MB11346.jpnprd01.prod.outlook.com/T/#mf24ae1363925a97c840b275c108
> > a9b72942c40bc
> >
> > [3]
> > https://lore.kernel.org/all/[email protected]
> > esas.com/T/#m31c6711b5403c5a387b22467bb88371df73ae2b1
> >
> >
> > Cheers,
> > Biju
> >
> 
> Then it seems you have a phandle from the DSI node to the sysc node:
> 
> renesas,sysc-pwrrdy = <&sysc 0xd70 0x2>;

But this is no more required after modelling the pwrrdy signal as powerseq driver.

> 
> In match() you can check if that phandle leads to the sysc device who's the parent of the power sequencer
> device and return MATCH_OK if it is.

As per the DT maintainer Krzysztof Kozlowski, we should not use random phandles for pwrrdy signal.

Also, I confirm that this property is unnecessary if we model it as Power sequencer.

From ABI, point now only matters is the pwrseq_target_data.name in the power sequence driver and
consumers.

dsi->pwrseq = devm_pwrseq_get(dsi->dev, "dsi-pwrrdy");
priv->pwrseq = devm_pwrseq_get(dev, "usb-pwrrdy");

From the experience, I don't think, DT maintainers will allow
to define a vendor property just for the driver match.

Cheers,
Biju