Re: [PATCH] arm64: dts: qcom: sm8450: delete incorrect ufs interconnect fields

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 4/11/22 10:16 PM, Bjorn Andersson wrote:
On Thu 07 Apr 17:38 CDT 2022, Jonathan Marek wrote:

On 4/7/22 5:16 PM, Krzysztof Kozlowski wrote:
On 07/04/2022 21:40, Vladimir Zapolskiy wrote:
On 4/7/22 20:21, Jonathan Marek wrote:
Upstream sm8450.dtsi has #interconnect-cells = <2>; so these are wrong.
Ignored and undocumented with upstream UFS driver so delete for now.

This is the upstream and they are documented here, although as pointed
by Vladimir this was rather a reverse-documentation. The documentation
might be incorrect, but then the bindings should be corrected instead of
only modifying the DTS.


Basically the description was added by a commit 462c5c0aa798 ("dt-bindings: ufs:
qcom,ufs: convert to dtschema").

It's questionable, if an example in the new yaml file is totally correct
in connection to the discussed issue.

To be honest - the example probably is not correct, because it was based
on existing DTS without your patch. :)

Another question is whether the interconnect properties are here correct
at all. I assumed that DTS is correct because it should describe the
hardware, even if driver does not use it. However maybe that was a false
assumption...


writing-bindings.rst says it is OK to document even if it isn't used by the
driver (seems wrong to me, at least for interconnects which are a firmware
abstraction and not hardware).


The devicetree, and hence the binding, should describe the hardware, so
that an implementation can make use of the hardware. So there's no
problem expressing the interconnect in the binding/dts even though the
driver isn't using it.

I'm not sure if I'm misunderstanding you, the interconnect paths
described here are a description of the hardware requirements for this
device. (I.e. it need the buses between ufs and ddr, and cpu and ufs to
operate).


This is pedantic but what if my kernel lives in imem and not ddr. Or it runs on adsp and not cpu? "ufs-ddr" and "cpu-ufs" are not necessarily hardware requirements.

(I was thinking of something else when I wrote that comment, but it doesn't actually matter if its firmware/hardware if a driver can implement the same functionality either way)

462c5c0aa798 wasn't in my 5.17+ tree pulled after dts changes were merged (I
guess doc changes come later), so my commit message is incorrect, but I
think it makes more sense to have the documentation reflect the driver. Its
also not an important issue, so I'll let others sort it out.


I believe that the correctness of the interconnect property will ensure
that the interconnect provider doesn't hit sync_state until the ufs
driver has probed - regardless of the driver actually implementing the
interconnect voting. But perhaps I've misunderstood the magic involved?


AFAICT, if its not used by the driver it will be ignored completely, unless you use OPP (which has some interconnect magic).

Regards,
Bjorn




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux