Re: [PATCH 2/2] media: c3-mipi-csi2: handle runtime resume failure

[Keke Li]

Hi Jacopo,

Thanks for your reply.

On 10/19/26 15:31, Jacopo Mondi wrote:
> [ EXTERNAL EMAIL ]
>
> Hi Keke
>
> On Mon, Oct 19, 2026 at 02:24:34PM +0800, Keke Li wrote:
> > Hi Jiale,
> >
> > On 10/3/26 16:11, Jiale Yao wrote:
> > > [You don't often get email from yaojiale02@xxxxxxx. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
> > >
> > > [ EXTERNAL EMAIL ]
> > >
> > > c3_mipi_csi_enable_streams() ignores runtime resume failures and starts
> > > the receiver before reporting that streaming is enabled. If the power
> > > domain could not be resumed, this accesses unavailable registers and
> > > the disable path later drops a reference that was not acquired.
> > >
> > > Return the runtime PM error before starting the receiver.
> > >
> > > Fixes: b63ef604a28d ("media: platform: Add C3 MIPI CSI-2 driver")
> > > Signed-off-by: Jiale Yao <yaojiale02@xxxxxxx>
> > > ---
> > >    drivers/media/platform/amlogic/c3/mipi-csi2/c3-mipi-csi2.c | 4 +++-
> > >    1 file changed, 3 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/media/platform/amlogic/c3/mipi-csi2/c3-mipi-csi2.c b/drivers/media/platform/amlogic/c3/mipi-csi2/c3-mipi-csi2.c
> > > index b9e4ef3fc308..4731d58bf2dd 100644
> > > --- a/drivers/media/platform/amlogic/c3/mipi-csi2/c3-mipi-csi2.c
> > > +++ b/drivers/media/platform/amlogic/c3/mipi-csi2/c3-mipi-csi2.c
> > > @@ -431,7 +431,9 @@ static int c3_mipi_csi_enable_streams(struct v4l2_subdev *sd,
> > >
> > >           src_sd = media_entity_to_v4l2_subdev(csi->src_pad->entity);
> > >
> > > -       pm_runtime_resume_and_get(csi->dev);
> > > +       ret = pm_runtime_resume_and_get(csi->dev);
> > > +       if (ret < 0)
> > Is   ' if (ret)'  better?
> > Does your driver depend on select PM ? It doesn't seem the case to me.
> >
> > If PM is disabled pm_runtime_resume_and_get() returns 1
> > if my reading of
> >
> >          static inline int __pm_runtime_resume(struct device *dev, int rpmflags)
> >          {
> >                  return 1;
> >          }
> >
> > is correct.
> >
> > If you
> >          if (ret)
> >
> > also this case will be caught as an error, while we instead want to
> > continue as the peripheral is meant to stay powered on from probe() if
> > CONFIG_PM is not selected.
> >
> > (I quickly had a look at the csi2 driver probe() routine
> > c3_mipi_csi_probe() and it doesn't seem to power up the interface in
> > probe() to support !CONFIG_PM. Personally, I would just select PM
> > unconditionally almost everywhere these days and be done with it).

Yes, you are correct.🙂

a. Here should keep 'if (ret < 0)'.

b. Add 'select PM' in each Kconfig file.

Thanks very much!

> > > +               return ret;
> > >
> > >           c3_mipi_csi_start_stream(csi);
> > >
> > > --
> > > 2.34.1