Re: [PATCH 1/9] media: rzg2l-cru: Add device_link from CRU to CSI-2

Hi Jacopo,
Thanks for your review.

On Mon, Jul 27, 2026 at 11:45:50AM +0200, Jacopo Mondi wrote:
> Hi Tommaso
> 
> On Tue, Jun 16, 2026 at 07:05:31PM +0200, Tommaso Merciai wrote:
> > The CRU device depends on CSI-2 to operate. Without an explicit
> > device_link the PM core has no knowledge of this dependency and may
> > suspend CSI-2 while CRU is still active, or resume CRU before CSI-2
> > is ready.
> >
> > Add a DL_FLAG_STATELESS device_link from the CRU to CSI-2 when the
> > CSI-2 subdev binds. This instructs the PM core to suspend CRU before
> > CSI-2 and to resume CSI-2 before CRU. The link is deleted on unbind.
> >
> > Signed-off-by: Tommaso Merciai <[email protected]>
> > ---
> >  drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c | 7 +++++++
> >  1 file changed, 7 insertions(+)
> >
> > diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
> > index 3c5fbd857371..1b12d91eaec9 100644
> > --- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
> > +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-core.c
> > @@ -101,6 +101,7 @@ static void rzg2l_cru_group_notify_unbind(struct v4l2_async_notifier *notifier,
> >  	mutex_lock(&cru->mdev_lock);
> >
> >  	if (cru->csi.asd == asd) {
> > +		device_link_remove(cru->dev, subdev->dev);
> >  		cru->csi.subdev = NULL;
> >  		dev_dbg(cru->dev, "Unbind CSI-2 %s\n", subdev->name);
> >  	}
> > @@ -118,6 +119,12 @@ static int rzg2l_cru_group_notify_bound(struct v4l2_async_notifier *notifier,
> >
> >  	if (cru->csi.asd == asd) {
> >  		cru->csi.subdev = subdev;
> > +		if (!device_link_add(cru->dev, subdev->dev, DL_FLAG_STATELESS)) {
> 
> So, cru-core and cru-csi2 are two different drivers, guarded by two
> different config symbols, both live in drivers/media/platforms/renesas/rzg2l-cru/
> 
> I guess it doesn't make much sense to have one without the other, but
> the build system allows that. That might be the reason why you used
> STATELESS here, but since this call happens at _bound() time, doesn't
> it mean we're sure the supplier driver is there and has probed
> correctly already ?

Exactly at this point we are sure that the supplier driver is there and
has probed correctly.

DL_FLAG_STATELESS here is to get the right suspend/resume ordering
side effect.

> 
> I guess you don't want DL_FLAG_PM_RUNTIME as this series is for system
> suspend/resume, right ?

You are correct. I'm testing continuos stream over suspend/resume using
RZ/G3E SMARC EVK with OV5645 camera connected to CSI and CRU enabled.

echo deep > /sys/power/mem_sleep
echo mem > /sys/power/state

Kind Regards,
Tommaso

> 
> > +			dev_err(cru->dev, "Failed to create device link to CSI-2 %s\n",
> > +				subdev->name);
> > +			mutex_unlock(&cru->mdev_lock);
> > +			return -EINVAL;
> > +		}
> >  		dev_dbg(cru->dev, "Bound CSI-2 %s\n", subdev->name);
> >  	}
> >
> > --
> > 2.54.0
> >