Re: [PATCH 09/13] m68k: coldfire: add devicetree for mcf5475-evb platform

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

 



On 19/02/2025 09:23, Greg Ungerer wrote:
Hi Krzysztof,

On 19/2/25 17:06, Krzysztof Kozlowski wrote:
On 18/02/2025 13:46, Greg Ungerer wrote:
Add a simple devicetree to support the Freescale MCF5475-EVB platform.

Signed-off-by: Greg Ungerer <gerg@xxxxxxxxxxxxxx>
---
  arch/m68k/boot/dts/Makefile       |  1 +
  arch/m68k/boot/dts/mcf5475evb.dts | 55 +++++++++++++++++++++++++++++++
  2 files changed, 56 insertions(+)
  create mode 100644 arch/m68k/boot/dts/mcf5475evb.dts

diff --git a/arch/m68k/boot/dts/Makefile b/arch/m68k/boot/dts/Makefile
index d69e37b48558..f0e9643e6b71 100644
--- a/arch/m68k/boot/dts/Makefile
+++ b/arch/m68k/boot/dts/Makefile
@@ -1,4 +1,5 @@
  # SPDX-License-Identifier: GPL-2.0
  
  dtb-y += mcf5208evb.dtb
+dtb-y += mcf5475evb.dtb
  
diff --git a/arch/m68k/boot/dts/mcf5475evb.dts b/arch/m68k/boot/dts/mcf5475evb.dts
new file mode 100644
index 000000000000..247a938bfafd
--- /dev/null
+++ b/arch/m68k/boot/dts/mcf5475evb.dts
@@ -0,0 +1,55 @@
+// SPDX-License-Identifier: GPL-2.0
+/dts-v1/;
+
+/ {
+	model = "Freescale MCF5475EVB";
+	compatible = "mcf5475evb";

Incorrect format.

Please run scripts/checkpatch.pl and fix reported warnings. After that,
run also `scripts/checkpatch.pl --strict` and (probably) fix more
warnings. Some warnings can be ignored, especially from --strict run,
but the code here looks like it needs a fix. Feel free to get in touch
if the warning is not clear.

If that is an incorrect format then checkpatch is not reporting it.

There are two issues here.
1. Incorrect format: see DT spec or any other example.
2. Below checkpatch warning.


Checkpatch here (even --strict) only reports that it is undocumented:

     WARNING: DT compatible string "mcf5475evb" appears un-documented -- check ./Documentation/devicetree/bindings/
     #36: FILE: arch/m68k/boot/dts/mcf5475evb.dts:6:
     +	compatible = "mcf5475evb";

Which I totally expect at this point. As pointed out in the cover letter this
work is very much an incomplete proof-of-concept still. Much work still to do here.


Patch should be marked as RFC with that explanation in cover letter, so
you won't get such review comments.

+	#address-cells = <1>;
+	#size-cells = <1>;
+	interrupt-parent = <&intc>;
+
+	coreclk: clock-266000000 {
+		#clock-cells = <0>;
+		compatible = "fixed-clock";
+		clock-frequency = <266000000>;
+	};
+
+	intc: interrupt-controller@ff000700 {
+		compatible = "fsl,intc-2";
+		interrupt-controller;
+		#interrupt-cells = <1>;
+		reg = <0xff000700 0x80>;
+	};
+
+	uart0: serial@ff008600 {
+		compatible = "fsl,mcfuart";
+		reg = <0xff008600 0x80>;
+		interrupts = <99>;
+		clocks = <&coreclk>;
+		status = "okay";

Did you disable it anywhere?

No.


So no need for that status.

Best regards,
Krzysztof




[Index of Archives]     [Video for Linux]     [Yosemite News]     [Linux S/390]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux