| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <lqw5blalkc2e4owzds5qs6bntsk6oh5jq5o6si3ugpa4sm7wu7@ssg4l3da6mww>
Date: Fri, 13 Dec 2024 10:41:57 +0100
From: Krzysztof Kozlowski <krzk@...nel.org>
To: Marc Kleine-Budde <mkl@...gutronix.de>
Cc: Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>,
Maxime Coquelin <mcoquelin.stm32@...il.com>, Alexandre Torgue <alexandre.torgue@...s.st.com>,
kernel@...gutronix.de, devicetree@...r.kernel.org,
linux-stm32@...md-mailman.stormreply.com, linux-arm-kernel@...ts.infradead.org,
linux-kernel@...r.kernel.org, Leonard Göhrs <l.goehrs@...gutronix.de>
Subject: Re: [PATCH stm32-next 2/2] ARM: dts: stm32: lxa-fairytux2: add Linux
Automation GmbH FairyTux 2
On Tue, Dec 10, 2024 at 10:32:28AM +0100, Marc Kleine-Budde wrote:
> +&gpiof {
> + gpio-line-names = "GPIO1", "GPIO2", "", "", "", /* 0 */
> + "", "", "", "", "", /* 5 */
That's not really readable. Fix the alignment with opening ", see DTS
coding style.
> + "", "", "", "", "", /* 10 */
> + ""; /* 15 */
> +};
> +
> +&gpioh {
> + gpio-line-names = "", "", "", "", "LCD_RESET", /* 0 */
> + "", "", "", "", "", /* 5 */
> + "", "", "", "GPIO3", "", /* 10 */
> + ""; /* 15 */
> +};
> +
> +&gpioi {
> + gpio-line-names = "", "", "", "", "", /* 0 */
> + "", "", "", "ETH_", "", /* 5 */
> + "", "USER_BTN1"; /* 10 */
> +};
> +
> +&i2c1 {
> + pinctrl-names = "default", "sleep";
> + pinctrl-0 = <&i2c1_pins_b>;
> + pinctrl-1 = <&i2c1_sleep_pins_b>;
> + status = "okay";
> +
> + io_board_gpio: tca6408@20 {
Node names should be generic. See also an explanation and list of
examples (not exhaustive) in DT specification:
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation
> + compatible = "ti,tca6408";
> + reg = <0x20>;
> + gpio-controller;
> + #gpio-cells = <2>;
> + vcc-supply = <&v3v3_hdmi>;
> + gpio-line-names = "LED1_GA_YK", "LED2_GA_YK", "LED1_GK_YA", "LED2_GK_YA",
> + "RS485_EN", "RS485_120R", "", "CAN_120R";
> + };
> +};
> +
> +&led_controller_io {
> + /*
> + * led-2 and led-3 are internally connected antiparallel to one
> + * another inside the ethernet jack like this:
> + * GPIO1 ---+---|led-2|>--+--- GPIO3
> + * +--<|led-3|---+
> + * E.g. only one of the LEDs can be illuminated at a time while
> + * the other output must be driven low.
> + * This should likely be implemented using a multi color LED
> + * driver for antiparallel LEDs.
> + */
> + led-2 {
> + label = "fairytux2:green:act";
Drop "fairytux2" and rather use color and function properties.
> + gpios = <&io_board_gpio 1 GPIO_ACTIVE_HIGH>;
> + };
> +
> + led-3 {
> + label = "fairytux2:orange:act";
> + gpios = <&io_board_gpio 3 GPIO_ACTIVE_HIGH>;
> + };
> +};
> +
> +&usart3 {
> + /*
> + * On Gen 1 FairyTux 2 only RTS can be used and not CTS as well,
> + * Because pins PD11 (CTS) and PI11 (USER_BTN1) share the same
> + * interrupt and only one of them can be used at a time.
> + */
> + rts-gpios = <&gpiod 12 GPIO_ACTIVE_LOW>;
> +};
> +
> +&usbotg_hs {
> + dr_mode = "peripheral";
> +};
> diff --git a/arch/arm/boot/dts/st/stm32mp153c-lxa-fairytux2-gen2.dts b/arch/arm/boot/dts/st/stm32mp153c-lxa-fairytux2-gen2.dts
> new file mode 100644
> index 0000000000000000000000000000000000000000..c84f83a75731f70165c240812f7f4230bf9bfbb0
> --- /dev/null
> +++ b/arch/arm/boot/dts/st/stm32mp153c-lxa-fairytux2-gen2.dts
> @@ -0,0 +1,146 @@
> +// SPDX-License-Identifier: (GPL-2.0-or-later OR BSD-3-Clause)
> +/*
> + * Copyright (C) 2024 Leonard Göhrs, Pengutronix
> + */
> +
> +/dts-v1/;
> +
> +#include "stm32mp153c-lxa-fairytux2.dtsi"
> +
> +/ {
> + model = "Linux Automation GmbH FairyTux 2 Gen 2";
> + compatible = "lxa,stm32mp153c-fairytux2-gen2", "oct,stm32mp153x-osd32", "st,stm32mp153";
> +
> + gpio-keys {
> + compatible = "gpio-keys";
> +
> + button-left {
> + label = "USER_BTN1";
> + linux,code = <KEY_ESC>;
> + gpios = <&gpioi 10 (GPIO_ACTIVE_LOW | GPIO_PULL_UP)>;
> + };
> +
> + button-right {
> + label = "USER_BTN2";
> + linux,code = <KEY_HOME>;
> + gpios = <&gpioe 9 (GPIO_ACTIVE_LOW | GPIO_PULL_UP)>;
> + };
> + };
> +};
> +
> +&gpiof {
> + gpio-line-names = "", "", "", "", "", /* 0 */
> + "", "", "", "", "", /* 5 */
> + "", "", "", "", "", /* 10 */
> + ""; /* 15 */
> +};
> +
> +&gpioh {
> + gpio-line-names = "", "", "", "", "LCD_RESET", /* 0 */
> + "", "", "", "", "", /* 5 */
> + "", "", "GPIO1", "GPIO_INT", "", /* 10 */
> + ""; /* 15 */
> +};
> +
> +&gpioi {
> + gpio-line-names = "GPIO2", "", "", "", "", /* 0 */
> + "", "", "", "ETH_", "", /* 5 */
> + "", "USER_BTN1"; /* 10 */
> +};
> +
> +&i2c1 {
> + pinctrl-names = "default", "sleep";
> + pinctrl-0 = <&i2c1_pins_b>;
> + pinctrl-1 = <&i2c1_sleep_pins_b>;
> + status = "okay";
> +
> + io_board_gpio: tca6408@20 {
Node names should be generic. See also an explanation and list of
examples (not exhaustive) in DT specification:
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation
> + compatible = "ti,tca6408";
> + reg = <0x20>;
> + gpio-controller;
> + #gpio-cells = <2>;
> + interrupt-parent = <&gpioh>;
> + interrupts = <13 IRQ_TYPE_LEVEL_LOW>;
> + interrupt-controller;
> + pinctrl-names = "default";
> + pinctrl-0 = <&board_tca6408_pins>;
> + #interrupt-cells = <2>;
> + vcc-supply = <&v3v3_hdmi>;
> + gpio-line-names = "LED1_GA_YK", "LED2_GA_YK", "LED1_GK_YA", "USB_CC_ALERT",
> + "RS485_EN", "RS485_120R", "USB_CC_RESET", "CAN_120R";
Messed alignment.
> + };
> +
> + usb_c: stusb1600@28 {
Node names should be generic. See also an explanation and list of
examples (not exhaustive) in DT specification:
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation
> + compatible = "st,stusb1600";
> + reg = <0x28>;
> + interrupt-parent = <&io_board_gpio>;
> + interrupts = <3 IRQ_TYPE_EDGE_FALLING>;
> + vdd-supply = <®_5v>;
> + vsys-supply = <&v3v3_hdmi>;
> +
> + connector {
> + compatible = "usb-c-connector";
> + label = "USB-C";
> + power-role = "dual";
> + typec-power-opmode = "default";
> +
> + port {
> + con_usbotg_hs_ep: endpoint {
> + remote-endpoint = <&usbotg_hs_ep>;
> + };
> + };
> + };
> + };
> +
> + io_board_eeprom: eeprom@56 {
> + compatible = "atmel,24c04";
> + reg = <0x56>;
> + vcc-supply = <&v3v3_hdmi>;
> + };
> +
> + temperature-sensor@48 {
> + compatible = "national,lm75a";
> + reg = <0x48>;
> + /*
> + * The sensor itself is powered by a voltage divider from the
> + * always-on 5V supply.
> + * The required pull-up resistors however are on v3v3_hdmi.
> + */
> + vs-supply = <&v3v3_hdmi>;
> + };
> +};
> +
> +&rtc {
> + status = "okay";
> +};
> +
> +&led_controller_io {
> + led-2 {
> + label = "fairytux2:orange:act";
> + gpios = <&io_board_gpio 1 GPIO_ACTIVE_LOW>;
> + };
> +};
> +
> +&usart3 {
> + rts-gpios = <&gpiod 12 GPIO_ACTIVE_LOW>;
> + cts-gpios = <&gpiod 11 GPIO_ACTIVE_LOW>;
> +};
> +
> +&usbotg_hs {
> + usb-role-switch;
> +
> + port {
> + usbotg_hs_ep: endpoint {
> + remote-endpoint = <&con_usbotg_hs_ep>;
> + };
> + };
> +};
> +
> +&pinctrl {
> + board_tca6408_pins: stusb1600-0 {
> + pins {
> + pinmux = <STM32_PINMUX('H', 13, GPIO)>;
> + bias-pull-up;
> + };
> + };
> +};
> diff --git a/arch/arm/boot/dts/st/stm32mp153c-lxa-fairytux2.dtsi b/arch/arm/boot/dts/st/stm32mp153c-lxa-fairytux2.dtsi
> new file mode 100644
> index 0000000000000000000000000000000000000000..0e0e1b36cb087dfbfb47e383094c6044786a2c6a
> --- /dev/null
> +++ b/arch/arm/boot/dts/st/stm32mp153c-lxa-fairytux2.dtsi
> @@ -0,0 +1,394 @@
> +// SPDX-License-Identifier: (GPL-2.0-or-later OR BSD-3-Clause)
> +/*
> + * Copyright (C) 2020 STMicroelectronics - All Rights Reserved
> + * Copyright (C) 2021 Rouven Czerwinski, Pengutronix
> + * Copyright (C) 2023, 2024 Leonard Göhrs, Pengutronix
> + */
> +
> +#include "stm32mp153.dtsi"
> +#include "stm32mp15xc.dtsi"
> +#include "stm32mp15xx-osd32.dtsi"
> +#include "stm32mp15xxac-pinctrl.dtsi"
> +
> +#include <dt-bindings/gpio/gpio.h>
> +#include <dt-bindings/input/input.h>
> +#include <dt-bindings/leds/common.h>
> +#include <dt-bindings/pwm/pwm.h>
> +
> +/ {
> + aliases {
> + can0 = &m_can1;
> + ethernet0 = ðernet0;
> + i2c0 = &i2c1;
> + i2c1 = &i2c4;
> + mmc1 = &sdmmc2;
> + serial0 = &uart4;
> + serial1 = &usart3;
> + spi0 = &spi4;
> + };
> +
> + chosen {
> + stdout-path = &uart4;
> + };
> +
> + backlight: backlight {
> + compatible = "pwm-backlight";
> + power-supply = <&v3v3>;
> +
> + brightness-levels = <0 31 63 95 127 159 191 223 255>;
> + default-brightness-level = <7>;
> + pwms = <&led_pwm 3 1000000 0>;
> + };
> +
> + led-controller-cpu {
> + compatible = "gpio-leds";
> +
> + led-0 {
> + label = "fairytux2:green:status";
Same comment.
> + gpios = <&gpioa 13 GPIO_ACTIVE_HIGH>;
> + linux,default-trigger = "heartbeat";
> + };
> + };
> +
> + led_controller_io: led-controller-io {
> + compatible = "gpio-leds";
> +
> + /*
> + * led-0 and led-1 are internally connected antiparallel to one
> + * another inside the ethernet jack like this:
> + * GPIO0 ---+---|led-0|>--+--- GPIO2
> + * +--<|led-1|---+
> + * E.g. only one of the LEDs can be illuminated at a time while
> + * the other output must be driven low.
> + * This should likely be implemented using a multi color LED
> + * driver for antiparallel LEDs.
> + */
> + led-0 {
> + label = "fairytux2:green:link";
> + gpios = <&io_board_gpio 0 GPIO_ACTIVE_HIGH>;
> + };
> +
> + led-1 {
> + label = "fairytux2:orange:link";
> + gpios = <&io_board_gpio 2 GPIO_ACTIVE_HIGH>;
> + };
> + };
> +
> + reg_5v: regulator-5v {
> + compatible = "regulator-fixed";
> + regulator-name = "5V";
> + regulator-min-microvolt = <5000000>;
> + regulator-max-microvolt = <5000000>;
> + regulator-always-on;
> + };
> +
> + reg_1v2: regulator-1v2 {
> + compatible = "regulator-fixed";
> + regulator-name = "1V2";
> + regulator-min-microvolt = <1200000>;
> + regulator-max-microvolt = <1200000>;
> + regulator-always-on;
> + vin-supply = <®_5v>;
> + };
> +};
> +
> +baseboard_eeprom: &sip_eeprom {
> +};
> +
> +&crc1 {
> + status = "okay";
> +};
> +
> +&cryp1 {
> + status = "okay";
> +};
> +
> +&dts {
> + status = "okay";
> +};
> +
> +ðernet0 {
> + assigned-clocks = <&rcc ETHCK_K>, <&rcc PLL4_P>;
> + assigned-clock-parents = <&rcc PLL4_P>;
> + assigned-clock-rates = <125000000>; /* Clock PLL4 to 750Mhz in ATF */
> +
> + pinctrl-names = "default", "sleep";
> + pinctrl-0 = <ðernet0_rgmii_pins_b>;
> + pinctrl-1 = <ðernet0_rgmii_sleep_pins_b>;
> +
> + st,eth-clk-sel;
> + phy-mode = "rgmii-id";
> + phy-handle = <ðphy>;
> + status = "okay";
> +
> + mdio {
> + compatible = "snps,dwmac-mdio";
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + ethphy: ethernet-phy@3 { /* KSZ9031RN */
> + reg = <3>;
> + reset-gpios = <&gpioe 11 GPIO_ACTIVE_LOW>; /* ETH_RST# */
> + interrupt-parent = <&gpioa>;
> + interrupts = <6 IRQ_TYPE_EDGE_FALLING>; /* ETH_MDINT# */
> + reset-assert-us = <10000>;
> + reset-deassert-us = <300>;
> + micrel,force-master;
> + };
> + };
> +};
> +
> +&gpioa {
> + gpio-line-names = "", "", "", "", "", /* 0 */
> + "", "ETH_INT", "", "", "", /* 5 */
Really unreadable indentation. See DTS coding style.
Best regards,
Krzysztof
Powered by blists - more mailing lists