Re: [PATCH] interconnect: qcom: use icc_sync_state

[Georgi Djakov <djakov@kernel.org>]

On 28.04.22 13:23, Krzysztof Kozlowski wrote:
> On 27/04/2022 17:34, Georgi Djakov wrote:
>> On 27.04.22 17:56, Krzysztof Kozlowski wrote:
>>> Use icc_sync_state for interconnect providers, so that the bandwidth
>>> request doesn't need to stay on maximum value.
>>
>> Did you test this? In general, we should not enable this on boards that
>> do not have full interconnect scaling support in consumer drivers yet.
>> Some of the interconnects could be enabled by default by the bootloader
>> and usually later during boot the consumer drivers request the bandwidth
>> that they need. But if the requests are missing, the interconnects
>> without bandwidth users will be disabled when we reach sync state. So
>> this may (or not) cause issues...
> 
> I understand, thanks for bringing this up. It does not look like an
> issue of interconnect provider but instead consumers and DTS. It's not
> the job of provider driver to know all possible uses and DTS files. The
> driver should expose itself and if platform is not ready, should not use
> it by not enabling the interconnect. It's a job for DTS, not for the
> interconnect provider.

Agree, but we still need to make sure this is tested and does not
introduce any regressions at least with the DT that is upstream.

> Imagine some out of tree DTS which cannot use interconnects because we
> assume that all users of that provider are missing bandwidth requests.
> No, instead provider should allow anyone to use it.

I have an idea to introduce a kernel parameter like clk_ignore_unused,
but for interconnects.

> I understand my change might cause unexpected issues, but it is still
> technically correct, just maybe should be followed with disabling in DTS
> the providers without proper consumers?

Not sure that enabling/disabling stuff in DT is a good option. DT should
be just a description of the hardware and it might not be updated as often
as the kernel.

Thanks,
Georgi