Re: [PATCH] interconnect: qcom: use icc_sync_state

[Georgi Djakov]

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