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@xxxxxxxxxxx/ > > 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
