Re: [PATCH v2 2/3] soc: qcom: ice: Enable PM runtime for ICE driver

[Krzysztof Kozlowski]

On 18/08/2026 09:35, Linlin Zhang wrote:
> 
> 
> On 8/18/2026 3:30 PM, Linlin Zhang wrote:
>>
>>
>> On 5/15/2026 10:48 PM, Krzysztof Kozlowski wrote:
>>> On 15/05/2026 16:22, Linlin Zhang wrote:
>>>>
>>>> Hi Krzysztof,
>>>>
>>>> Thanks for the review.
>>>>
>>>> For the SCMI-based platforms (e.g. sa8255p), the ICE resources such as
>>>> clocks are not controlled directly by the ICE driver. Instead, they are
>>>> managed by remote firmware and exposed to Linux via power domains. As a
>>>> result, the ICE driver cannot use clk_prepare_enable() directly to
>>>> control the hardware clock.
>>>>
>>>> The intention of moving the clock handling into runtime PM callbacks is
>>>> to align the ICE driver with the power domain framework used on these
>>>> platforms. When the ICE device is attached to a power domain, invoking
>>>> pm_runtime_resume_and_get() will trigger the provider (remote firmware
>>>> via SCMI) to power up the device, which in turn enables the underlying
>>>> clock and other resources.
>>>>
>>>> This design follows the guidance where the runtime PM framework is
>>>> used as the common mechanism to abstract both:
>>>>   - direct clock control on non-SCMI platforms, and
>>>>   - firmware-controlled resources via power domains on SCMI platforms.
>>>>
>>>> In both cases, the runtime PM callbacks are responsible for performing
>>>> the actual resource enable/disable:
>>>>   - for legacy platforms: clk_prepare_enable()/disable_unprepare()
>>>>   - for SCMI platforms: power domain on/off handled by firmware
>>>>
>>>> So while it may look like an additional layer on legacy platforms, this
>>>> approach provides a unified mechanism without requiring separate driver
>>>> entry points or special handling in the upper layers (e.g. UFS driver).
>>>>
>>>> That said, I understand your concern that introducing runtime PM solely
>>>> for clock gating can be seen as unnecessary overhead on existing
>>>> platforms. I will revisit the implementation to ensure that:
>>>>   - the runtime PM integration does not introduce regressions for legacy
>>>>     platforms, and
>>>>   - the design clearly justifies the common abstraction for both SCMI
>>>>     and non-SCMI cases.
>>>>
>>>> In addition, I rewrite the commit message as the following to make the
>>>> intention more clear.
>>>>
>>>>   On some platforms the ICE device is placed in a firmware-managed power
>>>>   domain. In those cases the ICE core resources (including the clock) are
>>>>   not directly controllable by Linux and are instead toggled by the power
>>>>   domain provider (e.g. remote firmware via SCMI).
>>>>
>>>>   Wire the ICE device into runtime PM so that a single pm_runtime
>>>>   transition is used to bring the ICE device up/down. When the device is
>>>>   attached to a PM domain, pm_runtime_resume_and_get()/pm_runtime_put_sync()
>>>>   will invoke the PM domain callbacks and let the provider manage the
>>>>   resources. On platforms without a PM domain the runtime PM callbacks
>>>>   continue to perform the existing clock enable/disable locally.
>>>>
>>>>   No functional change is intended for non-firmware-managed platforms; the
>>>>   change provides a common control point that allows ICE to operate when
>>>>   resources are owned by a PM domain provider.
>>>>
>>>
>>>
>>> Nothing here resolves the comments. Also, it's top posted. Honestly, I
>>> won't be talking through you with LLM, so consider patch NAKed.
>>>
>>> Best regards,
>>> Krzysztof
>>
>> Sorry for the late due to personal matters. 
>>
>> At that time, I reviewed your comments one by one and provided answers, then
>> let AI polish the answer.
>>
>> I'll add new replies after your comments point by point.
>>
>> Thanks for the review.
>>
> 
> Alternatively, if you would prefer to review a revised patch series that
> incorporates the changes we have already agreed upon for patches 0/3 and 1/3,
> please let me know and I would be happy to prepare it.

You received review which you did not implement. Reviewing such code
again is waste of time. Pasting excessively long, unparseable LLM
outputs is just another way to waste time of reviewers. Pasting here
result, intention or instructions for LLM is another thing not to do,
recently there is plague of this. We are not your LLM. We are not going
to talk with your LLM through you. We are not going to receive your prompts.

NAK

Best regards,
Krzysztof