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

[Keke Li <keke.li@amlogic.com>]

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

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
>>>