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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <249f8109-31b1-4cb8-a5a4-b30c27b2e987@oss.qualcomm.com>
Date: Thu, 28 Aug 2025 23:46:21 +0530
From: Odelu Kukatla <odelu.kukatla@....qualcomm.com>
To: Krzysztof Kozlowski <krzk@...nel.org>, Georgi Djakov <djakov@...nel.org>,
        Rob Herring <robh@...nel.org>,
        Krzysztof Kozlowski <krzk+dt@...nel.org>,
        Conor Dooley <conor+dt@...nel.org>,
        Bjorn Andersson <andersson@...nel.org>,
        Konrad Dybcio <konradybcio@...nel.org>
Cc: Raviteja Laggyshetty <quic_rlaggysh@...cinc.com>,
        Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com>,
        Bartosz Golaszewski <bartosz.golaszewski@...aro.org>,
        linux-arm-msm@...r.kernel.org, linux-pm@...r.kernel.org,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
        Mike Tipton <mike.tipton@....qualcomm.com>
Subject: Re: [PATCH 1/3] dt-bindings: interconnect: add clocks property to
 enable QoS on sa8775p



On 8/24/2025 2:38 PM, Krzysztof Kozlowski wrote:
> On 20/08/2025 10:51, Odelu Kukatla wrote:
>>
>>
>> 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.
> 
> 
> That's irrelevant reason. Writing bindings since long time ask pretty
> clearly to describe hardware completely, regardless whether Linux
> supports this or not.
> 
> It does not matter if you enable QoS or not.
> 
I agree with you. Ideally, the bindings should have described the
hardware fully from the beginning. However, this was not done at the
time of initial upstreaming, and the driver was contributed by someone
from the community. I’m working now to improve the binding by adding the
missing pieces to support QoS configuration.

>>
>> The motivation for this change is that certain interconnect paths on
>> sa8775p require specific clocks to be enabled to access QoS registers.
> 
> This does not look at all like existing device is completely broken.
> 
> You just add new feature, so no ABI break.
> 
Yes, you are right. This is a feature aimed at performance enhancement
to improve system performance under concurrent workloads.

>> 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
> 
> So how was it working for the last 2 years?
> 
The system may function normally without this feature. However, enabling
QoS helps optimize latency and bandwidth across subsystems like CPU,
GPU, and multimedia engines, which becomes important in high-throughput
scenarios.

Best regards,
Odelu
> 
>> concurrent workloads. Enabling QoS improves system responsiveness and
>> ensures more predictable behavior in high-throughput scenarios.
> 
> 
> 
> Best regards,
> Krzysztof