Hi Claudiu, On 03/03/24 5:49 pm, claudiu beznea wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > s/ARM: dts: at91/ARM: dts: microchip > > in title. > > On 23.02.2024 19:31, Varshini Rajendran wrote: >> Add device tree file for sam9x75 curiosity board. >> >> Signed-off-by: Varshini Rajendran <varshini.rajendran@xxxxxxxxxxxxx> >> --- >> Changes in v4: >> - Removed full node paths >> - Renamed Leds with color names >> - Corrected regulator node names >> - Added support for classd and i2s nodes and their corresponding >> pinctrl nodes >> - Dropped USB nodes owing to the discussion here >> https://lore.kernel.org/linux-devicetree/CAL_JsqJ9PrX6fj-EbffeJce09MXs=B7t+KS_kOinxaRx38=WxA@xxxxxxxxxxxxxx/ >> (Explained elaborately in the cover letter) >> - Updated the linux,code property with the necessary value >> --- >> arch/arm/boot/dts/microchip/Makefile | 3 + >> .../dts/microchip/at91-sam9x75_curiosity.dts | 309 ++++++++++++++++++ >> 2 files changed, 312 insertions(+) >> create mode 100644 arch/arm/boot/dts/microchip/at91-sam9x75_curiosity.dts >> >> diff --git a/arch/arm/boot/dts/microchip/Makefile b/arch/arm/boot/dts/microchip/Makefile >> index efde9546c8f4..5b3d518da319 100644 >> --- a/arch/arm/boot/dts/microchip/Makefile >> +++ b/arch/arm/boot/dts/microchip/Makefile >> @@ -12,6 +12,7 @@ DTC_FLAGS_at91-sama5d3_eds := -@ >> DTC_FLAGS_at91-sama5d3_xplained := -@ >> DTC_FLAGS_at91-sama5d4_xplained := -@ >> DTC_FLAGS_at91-sama7g5ek := -@ >> +DTC_FLAGS_at91-sam9x75_curiosity := -@ > > Keep it alphanumerically sorted, thus after sam9x60 entry. > >> dtb-$(CONFIG_SOC_AT91RM9200) += \ >> at91rm9200ek.dtb \ >> mpa1600.dtb >> @@ -59,6 +60,8 @@ dtb-$(CONFIG_SOC_AT91SAM9) += \ >> dtb-$(CONFIG_SOC_SAM9X60) += \ >> at91-sam9x60_curiosity.dtb \ >> at91-sam9x60ek.dtb >> +dtb-$(CONFIG_SOC_SAM9X7) += \ >> + at91-sam9x75_curiosity.dtb >> dtb-$(CONFIG_SOC_SAM_V7) += \ >> at91-kizbox2-2.dtb \ >> at91-kizbox3-hs.dtb \ >> diff --git a/arch/arm/boot/dts/microchip/at91-sam9x75_curiosity.dts b/arch/arm/boot/dts/microchip/at91-sam9x75_curiosity.dts >> new file mode 100644 >> index 000000000000..be37022d3d05 >> --- /dev/null >> +++ b/arch/arm/boot/dts/microchip/at91-sam9x75_curiosity.dts >> @@ -0,0 +1,309 @@ >> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT) >> +/* >> + * at91-sam9x75_curiosity.dts - Device Tree file for Microchip SAM9X75 Curiosity board >> + * >> + * Copyright (C) 2023 Microchip Technology Inc. and its subsidiaries >> + * >> + * Author: Varshini Rajendran <varshini.rajendran@xxxxxxxxxxxxx> >> + */ >> +/dts-v1/; >> +#include "sam9x7.dtsi" >> +#include <dt-bindings/input/input.h> >> + >> +/ { >> + model = "Microchip SAM9X75 Curiosity"; >> + compatible = "microchip,sam9x75-curiosity", "microchip,sam9x7", "atmel,at91sam9"; >> + >> + aliases { >> + i2c0 = &i2c6; >> + }; >> + >> + chosen { >> + stdout-path = "serial0:115200n8"; >> + }; >> + >> + gpio-keys { >> + compatible = "gpio-keys"; >> + pinctrl-names = "default"; >> + pinctrl-0 = <&pinctrl_key_gpio_default>; >> + >> + button-user { >> + label = "USER"; >> + gpios = <&pioC 9 GPIO_ACTIVE_LOW>; >> + linux,code = <KEY_0>; >> + wakeup-source; >> + }; >> + }; >> + >> + leds { >> + compatible = "gpio-leds"; >> + pinctrl-names = "default"; >> + pinctrl-0 = <&pinctrl_led_gpio_default>; >> + >> + led-red { >> + label = "red"; >> + gpios = <&pioC 19 GPIO_ACTIVE_HIGH>; >> + }; >> + >> + led-green { >> + label = "green"; >> + gpios = <&pioC 21 GPIO_ACTIVE_HIGH>; >> + }; >> + >> + led-blue { >> + label = "blue"; >> + gpios = <&pioC 20 GPIO_ACTIVE_HIGH>; >> + linux,default-trigger = "heartbeat"; >> + }; >> + }; >> + >> + memory@20000000 { >> + device_type = "memory"; >> + reg = <0x20000000 0x10000000>; >> + }; >> +}; >> + >> +&classd { >> + pinctrl-names = "default"; >> + pinctrl-0 = <&pinctrl_classd>; >> + atmel,pwm-type = "diff"; >> + atmel,non-overlap-time = <10>; >> + status = "okay"; >> +}; >> + >> +&dbgu { >> + pinctrl-names = "default"; >> + pinctrl-0 = <&pinctrl_dbgu>; >> + status = "okay"; >> +}; >> + >> +&dma0 { >> + status = "okay"; >> +}; >> + >> +&flx6 { >> + atmel,flexcom-mode = <ATMEL_FLEXCOM_MODE_TWI>; >> + status = "okay"; >> +}; >> + >> +&i2c6 { > > I don't know if you got any review comments w/ regards to this in the > previous email but having flexcoms and inner node grouped together is > easier to follow (at least to me). e.g.: > Yes, I received a comment not to duplicate nodes. https://patchwork.kernel.org/project/linux-mmc/patch/20230623203056.689705-46-varshini.rajendran@xxxxxxxxxxxxx/ >> + #address-cells = <1>; >> + #size-cells = <0>; >> + pinctrl-names = "default"; >> + pinctrl-0 = <&pinctrl_flx6_default>; >> + i2c-analog-filter; >> + i2c-digital-filter; >> + i2c-digital-filter-width-ns = <35>; >> + status = "okay"; >> + >> + pmic@5b { >> + compatible = "microchip,mcp16502"; >> + reg = <0x5b>; >> + >> + regulators { >> + vdd_3v3: VDD_IO { >> + regulator-name = "VDD_IO"; >> + regulator-min-microvolt = <3000000>; >> + regulator-max-microvolt = <3600000>; > > I can't find the schematics for this but these values here should reflect > the voltage that the board support not the ones that the PMIC itself > supports. Valid for all the other regulators. > Okay. I will double check this with hardware experts as well. > >> + regulator-initial-mode = <2>; >> + regulator-allowed-modes = <2>, <4>; >> + regulator-always-on; >> + >> + regulator-state-standby { >> + regulator-on-in-suspend; >> + regulator-mode = <4>; >> + }; >> + >> + regulator-state-mem { >> + regulator-mode = <4>; >> + }; >> + }; >> + >> + vddioddr: VDD_DDR { >> + regulator-name = "VDD_DDR"; >> + regulator-min-microvolt = <1283000>; >> + regulator-max-microvolt = <1450000>; >> + regulator-initial-mode = <2>; >> + regulator-allowed-modes = <2>, <4>; >> + regulator-always-on; >> + >> + regulator-state-standby { >> + regulator-on-in-suspend; >> + regulator-mode = <4>; >> + }; >> + >> + regulator-state-mem { >> + regulator-on-in-suspend; >> + regulator-mode = <4>; >> + }; >> + }; >> + >> + vddcore: VDD_CORE { >> + regulator-name = "VDD_CORE"; >> + regulator-min-microvolt = <500000>; >> + regulator-max-microvolt = <1210000>; >> + regulator-initial-mode = <2>; >> + regulator-allowed-modes = <2>, <4>; >> + regulator-always-on; >> + >> + regulator-state-standby { >> + regulator-on-in-suspend; >> + regulator-mode = <4>; >> + }; >> + >> + regulator-state-mem { >> + regulator-mode = <4>; >> + }; >> + }; >> + >> + vddcpu: VDD_OTHER { >> + regulator-name = "VDD_OTHER"; >> + regulator-min-microvolt = <1700000>; >> + regulator-max-microvolt = <3600000>; >> + regulator-initial-mode = <2>; >> + regulator-allowed-modes = <2>, <4>; >> + regulator-ramp-delay = <3125>; >> + regulator-always-on; >> + >> + regulator-state-standby { >> + regulator-on-in-suspend; >> + regulator-mode = <4>; >> + }; >> + >> + regulator-state-mem { >> + regulator-mode = <4>; >> + }; >> + }; >> + >> + vldo1: LDO1 { >> + regulator-name = "LDO1"; >> + regulator-min-microvolt = <1200000>; >> + regulator-max-microvolt = <3700000>; >> + regulator-always-on; >> + >> + regulator-state-standby { >> + regulator-on-in-suspend; >> + }; >> + }; >> + >> + vldo2: LDO2 { >> + regulator-name = "LDO2"; >> + regulator-min-microvolt = <1200000>; >> + regulator-max-microvolt = <3700000>; >> + >> + regulator-state-standby { >> + regulator-on-in-suspend; >> + }; >> + }; >> + }; >> + }; >> +}; >> + >> +&i2s { >> + pinctrl-names = "default"; >> + pinctrl-0 = <&pinctrl_i2s_default>; >> + #sound-dai-cells = <0>; >> + status = "disabled"; > > Any reason this is disabled? There is a conflict. My bad, I missed mentioning it since the other node support is not added yet. I will add it in the next version. > >> +}; >> + >> +&main_xtal { >> + clock-frequency = <24000000>; >> +}; >> + >> +&pinctrl { >> + > > This line could be removed. > >> + classd { >> + pinctrl_classd: classd { >> + atmel,pins = >> + <AT91_PIOA 18 AT91_PERIPH_C AT91_PINCTRL_PULL_UP > > Try to be compliant with coding style from here (valid everywhere): > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/dts-coding-style.rst#n167 > >> + AT91_PIOA 19 AT91_PERIPH_C AT91_PINCTRL_PULL_DOWN>; >> + }; >> + }; >> + >> + dbgu { >> + pinctrl_dbgu: dbgu-0 { > > usually pinctrl label is something like the following in Microchip AT91 DTSes: > pinctrl_<ip-name>_default and node name is <ip-name>-default. > > Please use the same rule everywhere. > >> + atmel,pins = <AT91_PIOA 26 AT91_PERIPH_A AT91_PINCTRL_PULL_UP >> + AT91_PIOA 27 AT91_PERIPH_A AT91_PINCTRL_NONE>; >> + }; >> + }; >> + >> + flexcom { >> + pinctrl_flx6_default: flx6-twi { >> + atmel,pins = >> + <AT91_PIOA 24 AT91_PERIPH_A AT91_PINCTRL_PULL_UP >> + AT91_PIOA 25 AT91_PERIPH_A AT91_PINCTRL_PULL_UP>; >> + }; >> + }; >> + >> + gpio-keys { >> + pinctrl_key_gpio_default: key-gpio-default { >> + atmel,pins = <AT91_PIOC 9 AT91_PERIPH_GPIO AT91_PINCTRL_NONE>; >> + }; >> + }; >> + >> + i2s { >> + pinctrl_i2s_default: i2s { >> + atmel,pins = >> + <AT91_PIOB 26 AT91_PERIPH_D AT91_PINCTRL_NONE /* I2SCK */ >> + AT91_PIOB 15 AT91_PERIPH_D AT91_PINCTRL_NONE /* I2SWS */ >> + AT91_PIOB 16 AT91_PERIPH_D AT91_PINCTRL_NONE /* I2SDIN */ >> + AT91_PIOB 17 AT91_PERIPH_D AT91_PINCTRL_NONE /* I2SDOUT */ >> + AT91_PIOB 25 AT91_PERIPH_D AT91_PINCTRL_NONE>; /* I2SMCK */ >> + }; >> + }; >> + >> + leds { >> + pinctrl_led_gpio_default: led-gpio-default { >> + atmel,pins = <AT91_PIOC 19 AT91_PERIPH_GPIO AT91_PINCTRL_NONE >> + AT91_PIOC 21 AT91_PERIPH_GPIO AT91_PINCTRL_NONE >> + AT91_PIOC 20 AT91_PERIPH_GPIO AT91_PINCTRL_NONE>; >> + }; >> + }; >> + >> + sdmmc0 { >> + pinctrl_sdmmc0_default: sdmmc0 { >> + atmel,pins = >> + <AT91_PIOA 2 AT91_PERIPH_A (AT91_PINCTRL_DRIVE_STRENGTH_HI | AT91_PINCTRL_SLEWRATE_DIS) /* PA2 CK periph A with pullup */ >> + AT91_PIOA 1 AT91_PERIPH_A (AT91_PINCTRL_PULL_UP | AT91_PINCTRL_DRIVE_STRENGTH_HI | AT91_PINCTRL_SLEWRATE_DIS) /* PA1 CMD periph A with pullup */ >> + AT91_PIOA 0 AT91_PERIPH_A (AT91_PINCTRL_PULL_UP | AT91_PINCTRL_DRIVE_STRENGTH_HI | AT91_PINCTRL_SLEWRATE_DIS) /* PA0 DAT0 periph A */ >> + AT91_PIOA 3 AT91_PERIPH_A (AT91_PINCTRL_PULL_UP | AT91_PINCTRL_DRIVE_STRENGTH_HI | AT91_PINCTRL_SLEWRATE_DIS) /* PA3 DAT1 periph A with pullup */ >> + AT91_PIOA 4 AT91_PERIPH_A (AT91_PINCTRL_PULL_UP | AT91_PINCTRL_DRIVE_STRENGTH_HI | AT91_PINCTRL_SLEWRATE_DIS) /* PA4 DAT2 periph A with pullup */ >> + AT91_PIOA 5 AT91_PERIPH_A (AT91_PINCTRL_PULL_UP | AT91_PINCTRL_DRIVE_STRENGTH_HI | AT91_PINCTRL_SLEWRATE_DIS)>; /* PA5 DAT3 periph A with pullup */ >> + }; >> + }; >> + > > You can remove this line > >> +}; /* pinctrl */ >> + >> +&rtt { >> + atmel,rtt-rtc-time-reg = <&gpbr 0x0>; >> +}; >> + >> +&sdmmc0 { >> + bus-width = <4>; >> + pinctrl-names = "default"; >> + pinctrl-0 = <&pinctrl_sdmmc0_default>; >> + cd-gpios = <&pioA 23 GPIO_ACTIVE_LOW>; >> + disable-wp; >> + status = "okay"; >> +}; >> + >> +&slow_xtal { >> + clock-frequency = <32768>; >> +}; >> + >> +&power_management { >> + debounce-delay-us = <976>; >> + status = "okay"; >> + >> + input@0 { >> + reg = <0>; >> + }; >> +}; >> + >> +&trng { >> + status = "okay"; >> +}; >> + >> +&watchdog { >> + status = "okay"; >> +}; -- Thanks and Regards, Varshini Rajendran.