Re: [PATCH 1/3] dt-bindings: interconnect: add clocks property to enable QoS on sa8775p

[Odelu Kukatla <odelu.kukatla@oss.qualcomm.com>]

On 8/13/2025 11:32 AM, Krzysztof Kozlowski wrote:
> On 13/08/2025 07:55, Odelu Kukatla wrote:
>>
>>
>> On 8/12/2025 3:47 PM, Krzysztof Kozlowski wrote:
>>> On 08/08/2025 16:02, Odelu Kukatla wrote:
>>>> Add reg and clocks properties to enable the clocks required
>>>> for accessing QoS configuration.
>>>
>>>
>>> Nothing here explains why EXISTING hardware is being changed. I also
>>> remember big discussions and big confusing patches regarding sa8775p
>>> (its rename, dropping/changing all providers), and this patch feels like
>>> pieces of it without proper justification.
>>>
>> Thanks for the review.
>> I have added description in cover letter, i will add here as well in next revision.> And this is hidden ABI break, no justification, no mentioning either.
>>> Again we are discussing basics of ABI breaking patches?
>>>
>> If you are talking ABI break if we load old DT which may lead to crash, we have .qos_requires_clocks flag which takes care of skipping QoS if required clocks are not enabled.we have addressed this issue through https://lore.kernel.org/all/20240704125515.22194-1-quic_okukatla@quicinc.com/ 
> 
> Format your emails correctly, it's difficult to read.
> 
> Your binding did not require reg and clocks. Now it requires reg and
> clocks. This is called ABI break.
> 
> Please follow Qualcomm extensive upstreaming guide, it explains this,
> doesn't it? Or follow writing bindings...
> 

Thanks for your review and guidance.

I agree that adding reg and clocks properties to existing bindings is an
ABI break. The sa8775p is a relatively older platform, and when the
interconnect provider driver was initially upstreamed, QoS configuration
support was not available in the framework. As a result, QoS was not
enabled at that time.

The motivation for this change is that certain interconnect paths on
sa8775p require specific clocks to be enabled to access QoS registers.
QoS configuration is essential for managing latency and bandwidth across
subsystems such as CPU, GPU, and multimedia engines. Without it, the
system may experience performance degradation, especially under
concurrent workloads. Enabling QoS improves system responsiveness and
ensures more predictable behavior in high-throughput scenarios.

We previously discussed ABI concerns when introducing QoS support on the
SC7280 platform. To address this, we added a .qos_requires_clocks flag
in the driver, which ensures that QoS configuration is skipped if the
required clocks are not defined. This mechanism prevents crashes when
older DTs are used, thereby preserving compatibility.

I will update the commit message to include this justification. We also
plan to follow a similar approach for other platforms like SA8775P,
where the provider driver is already upstreamed and QoS enablement will
be submitted as a separate patch series.

Thanks again for your feedback.

Best regards,
Odelu


> Best regards,
> Krzysztof