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

[Jacopo Mondi <jacopo.mondi@ideasonboard.com>]

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@163.com. 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@163.com>
> > ---
> >   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).

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