Re: [PATCH 04/21] ARM: dts: at91: sam9x7: add device tree for soc

[Conor Dooley]

Hey Varshini,

On Sun, Jun 04, 2023 at 01:32:26AM +0530, Varshini Rajendran wrote:
> Add device tree file for SAM9X7 SoC family
> 
> Signed-off-by: Varshini Rajendran <varshini.rajendran@xxxxxxxxxxxxx>
> [nicolas.ferre@xxxxxxxxxxxxx: add support for gmac to sam9x7]

Please just replace these [] things with a Co-developed-by.

> Signed-off-by: Nicolas Ferre <nicolas.ferre@xxxxxxxxxxxxx>
> [balamanikandan.gunasundar@xxxxxxxxxxxxx: Add device node csi2host and isc]
> Signed-off-by: Balamanikandan Gunasundar <balamanikandan.gunasundar@xxxxxxxxxxxxx>
> ---
>  arch/arm/boot/dts/sam9x7.dtsi | 1333 +++++++++++++++++++++++++++++++++
>  1 file changed, 1333 insertions(+)
>  create mode 100644 arch/arm/boot/dts/sam9x7.dtsi
> 
> diff --git a/arch/arm/boot/dts/sam9x7.dtsi b/arch/arm/boot/dts/sam9x7.dtsi
> new file mode 100644
> index 000000000000..f98160182fe6
> --- /dev/null
> +++ b/arch/arm/boot/dts/sam9x7.dtsi
> @@ -0,0 +1,1333 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +/*
> + * sam9x7.dtsi - Device Tree Include file for Microchip SAM9X7 SoC family
> + *
> + * Copyright (C) 2022 Microchip Technology Inc. and its subsidiaries
> + *
> + * Author: Varshini Rajendran <varshini.rajendran@xxxxxxxxxxxxx>
> + */
> +
> +#include <dt-bindings/clock/at91.h>
> +#include <dt-bindings/dma/at91.h>
> +#include <dt-bindings/gpio/gpio.h>
> +#include <dt-bindings/interrupt-controller/arm-gic.h>
> +#include <dt-bindings/interrupt-controller/irq.h>
> +#include <dt-bindings/mfd/atmel-flexcom.h>
> +#include <dt-bindings/pinctrl/at91.h>
> +
> +/ {
> +	#address-cells = <1>;
> +	#size-cells = <1>;
> +	model = "Microchip SAM9X7 SoC";
> +	compatible = "microchip,sam9x7";

Unless I am mistaken, sam9x7 is a family, not an soc. I'll have to
defer to Nicolas or someone that actually properly understands the
naming scheme here though! It's certainly odd to use sam9x7 here, when
the file is filled with references to sam9x60, which is a soc-specific
compatible.

Either way, the compatible is undocumented as far as I can tell and I
assume that this was not actually tested, since there doesn't appear
to be any dts including this file and therefore no way to build it?

> +	interrupt-parent = <&aic>;
> +
> +	aliases {
> +		serial0 = &dbgu;
> +		gpio0 = &pioA;
> +		gpio1 = &pioB;
> +		gpio2 = &pioC;
> +		gpio3 = &pioD;
> +	};
> +
> +	cpus {
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		cpu@0 {
> +			compatible = "arm,arm926ej-s";
> +			device_type = "cpu";
> +			reg = <0>;
> +		};
> +	};
> +
> +	clocks {
> +		slow_xtal: slow_xtal {
> +			compatible = "fixed-clock";
> +			#clock-cells = <0>;
> +		};
> +
> +		main_xtal: main_xtal {
> +			compatible = "fixed-clock";
> +			#clock-cells = <0>;
> +		};
> +	};
> +
> +	sram: sram@300000 {
> +		compatible = "mmio-sram";
> +		reg = <0x300000 0x10000>;
> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +		ranges = <0 0x300000 0x10000>;
> +	};
> +
> +	ahb {
> +		compatible = "simple-bus";
> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +		ranges;
> +
> +		usb0: gadget@500000 {
> +			compatible = "microchip,sam9x60-udc";

This is not a sam9x60, so it should not only have that SoC's compatible
here. Ideally, "microchip,sam9x7{0,2,5}" with the sam9x60 one as a
fallback.

> +			reg = <0x500000 0x100000>,
> +			      <0xf803c000 0x400>;
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			interrupts = <23 IRQ_TYPE_LEVEL_HIGH 2>;
> +			clocks = <&pmc PMC_TYPE_PERIPHERAL 23>, <&pmc PMC_TYPE_CORE PMC_UTMI>;
> +			clock-names = "pclk", "hclk";
> +			assigned-clocks = <&pmc PMC_TYPE_CORE PMC_UTMI>;
> +			assigned-clock-rates = <480000000>;
> +			status = "disabled";
> +		};
> +
> +		ohci0: usb@600000 {
> +			compatible = "atmel,at91rm9200-ohci", "usb-ohci";

Ditto here.

> +			reg = <0x600000 0x100000>;
> +			interrupts = <22 IRQ_TYPE_LEVEL_HIGH 2>;
> +			clocks = <&pmc PMC_TYPE_PERIPHERAL 22>, <&pmc PMC_TYPE_PERIPHERAL 22>, <&pmc PMC_TYPE_SYSTEM 6>;
> +			clock-names = "ohci_clk", "hclk", "uhpck";
> +			status = "disabled";
> +		};
> +
> +		ehci0: usb@700000 {
> +			compatible = "atmel,at91sam9g45-ehci", "usb-ehci";

And here.

> +			reg = <0x700000 0x100000>;
> +			interrupts = <22 IRQ_TYPE_LEVEL_HIGH 2>;
> +			clocks = <&pmc PMC_TYPE_CORE PMC_UTMI>, <&pmc PMC_TYPE_PERIPHERAL 22>;
> +			clock-names = "usb_clk", "ehci_clk";
> +			assigned-clocks = <&pmc PMC_TYPE_CORE PMC_UTMI>;
> +			assigned-clock-rates = <480000000>;
> +			status = "disabled";
> +		};
> +
> +		sdmmc0: sdio-host@80000000 {
> +			compatible = "microchip,sam9x60-sdhci";

And here.

> +			reg = <0x80000000 0x300>;
> +			interrupts = <12 IRQ_TYPE_LEVEL_HIGH 0>;
> +			clocks = <&pmc PMC_TYPE_PERIPHERAL 12>, <&pmc PMC_TYPE_GCK 12>;
> +			clock-names = "hclock", "multclk";
> +			assigned-clocks = <&pmc PMC_TYPE_GCK 12>;
> +			assigned-clock-rates = <100000000>;
> +			status = "disabled";
> +		};
> +
> +		sdmmc1: sdio-host@90000000 {
> +			compatible = "microchip,sam9x60-sdhci";

There's no point me typing it every time, but ditto the whole way
through this file ;)

Cheers,
Conor.

Attachment: signature.asc
Description: PGP signature