On 1/4/23 11:11, Eugen.Hristev@microchip.com wrote:
On 1/4/23 11:38, Marek Vasut wrote:
On 1/4/23 08:30, Eugen.Hristev@microchip.com wrote:
On 1/3/23 01:12, Marek Vasut wrote:
On 12/23/22 13:33, Sergiu Moga wrote:
Add the OHCI and EHCI DT nodes for the sam9x60 boards.
Signed-off-by: Sergiu Moga sergiu.moga@microchip.com
arch/arm/dts/at91-sam9x60_curiosity.dts | 21 +++++++++++++++++++++ arch/arm/dts/sam9x60.dtsi | 18 ++++++++++++++++++
Board and SoC DT changes should be in separate patches.
arch/arm/dts/sam9x60ek.dts | 21 +++++++++++++++++++++ 3 files changed, 60 insertions(+)
[...]
diff --git a/arch/arm/dts/sam9x60.dtsi b/arch/arm/dts/sam9x60.dtsi index 17224ef771..e36a540f78 100644 --- a/arch/arm/dts/sam9x60.dtsi +++ b/arch/arm/dts/sam9x60.dtsi @@ -69,6 +69,24 @@ #size-cells = <1>; ranges;
+ usb1: ohci@600000 {
This should be usb@ instead of ohci@ , if you run "make dtbs_check" on this DT in Linux (please do), you would likely get a warning , see Linux Documentation/devicetree/bindings/usb/usb.yaml .
If in Linux we have ohci@ , then we need to have the same in U-boot.
You should update the Linux DT to usb@ too to avoid dtbs_check warnings.
We can accept the change to usb@ , if there is a pending patch in Linux.
I can make the same argument about Linux, since DTs are OS agnostic. My comment is OS agnostic too and does not apply specifically to U-Boot or Linux, it applies to DT.
Definitely you can comment in Linux , for sure.
U-boot is not the place to review the devicetree.
I strongly disagree with this statement.
There are a few decisions behind this statement. First, the fact that we no longer take bindings into Uboot, and rely solely on bindings in Linux. This means the bindings were reviewed and validated in Linux.
Linux 1e5f532c27371 ("ARM: dts: at91: sam9x60: add device tree for soc and board") which added these ohci@ nodes has been accepted after Linux 14ec072a19ad7 ("dt-bindings: usb: Convert USB HCD generic binding to YAML") was already in place.
Therefore, nobody ran the dtbs_check at that point, which is somewhat understandable, since the dtbs_check at that point was in very early stages.
This however shows the review process is not flawless and subsequent updates may be necessary.
One such update example is Linux: 979813d2ab705 ("ARM: dts: at91: use generic name for reset controller") which does: - reset_controller: rstc@fffffe00 { + reset_controller: reset-controller@fffffe00 { or another one: 6a743ea387e63 ("ARM: dts: at91: Use the generic "rtc" node name for the rtt IPs") - rtt: rtt@fffffe20 { + rtt: rtc@fffffe20 { or another one: 4b6140b96dfe4 ("ARM: dts: at91: Use the generic "crypto" node name for the crypto IPs") - sha: sha@f002c000 { + sha: crypto@f002c000 {
Sure there are a few exceptions but not the usual rule. Second, we rely on devicetree kernel.org mailing list to review all the DTs and binding compliance in Linux. This means that the DT has been validated and checked as ABI in Linux.
I think the ABI argument does not work, your own engineers would've caused multiple ABI breaks by now, see above.
Hence there is no place to question that in Uboot or any other project.
As you can see above, Linux kernel review process is not perfect and the code is constantly evolving rather than begin set in stone. I believe other projects and their reviewers are no worse than Linux kernel ones, and can provide valid review feedback too.
The DT must be the same and ABI across all software that uses the DT. If this was validated and agreed upon in Linux, we can't question that here. So if you disagree with this statement, it's your choice of course.
I strongly disagree.
What does it matter where the review feedback came from ? What does matter is that you can improve the DT based on that feedback, if the feedback is valid.
I agree with that, but if we want to improve things, we need to patch things either in Linux first, or patch projects together. The fact that you are against some patches while they have the same identical node in Linux, is not a reason to not allow devicetree nodes to be added to Uboot. DT syncing with Linux is always a must, and that is part of what Sergiu is doing with these patches.
See below.
The devicetree must be in sync with Linux.
I agree with this statement.
Please update the Linux DT too.
That is one approach but as I said, not a reason to reject a node that is identical with Linux.
Ultimately, I am not the at91 maintainer, so I have no say in this.
I provided review feedback, that the DT checker would warn on the node name, and explained how to deal with it.
Whether the feedback is accepted and the DT is improved , or , the feedback is discarded and the DT checker keeps producing warnings , is not my decision to make.