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
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Fri, 12 Jan 2024 08:12:42 +0100
From: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To: Delphine CC Chiu <Delphine_CC_Chiu@...ynn.com>, patrick@...cx.xyz,
 Avi Fishman <avifishman70@...il.com>, Tomer Maimon <tmaimon77@...il.com>,
 Tali Perry <tali.perry1@...il.com>, Patrick Venture <venture@...gle.com>,
 Nancy Yuen <yuenn@...gle.com>, Benjamin Fair <benjaminfair@...gle.com>,
 Rob Herring <robh+dt@...nel.org>,
 Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
 Conor Dooley <conor+dt@...nel.org>
Cc: Jonathan Neuschäfer <j.neuschaefer@....net>,
 openbmc@...ts.ozlabs.org, devicetree@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH v1 2/2] ARM64: dts: nuvoton: Add initial yosemitev4 device
 tree

On 12/01/2024 02:36, Delphine CC Chiu wrote:
> Add linux device tree entry related to
> Yosemite 4 specific devices connected to BMC SoC.
> 

Prefix is arm64, not ARM64.

> Signed-off-by: Delphine CC Chiu <Delphine_CC_Chiu@...ynn.com>
> ---
>  arch/arm64/boot/dts/nuvoton/Makefile          |    1 +
>  .../dts/nuvoton/nuvoton-npcm845-yosemite4.dts | 1493 +++++++++++++++++
>  2 files changed, 1494 insertions(+)
>  create mode 100644 arch/arm64/boot/dts/nuvoton/nuvoton-npcm845-yosemite4.dts
> 
> diff --git a/arch/arm64/boot/dts/nuvoton/Makefile b/arch/arm64/boot/dts/nuvoton/Makefile
> index 3bc9787801a5..2b3c03083dc0 100644
> --- a/arch/arm64/boot/dts/nuvoton/Makefile
> +++ b/arch/arm64/boot/dts/nuvoton/Makefile
> @@ -2,3 +2,4 @@
>  dtb-$(CONFIG_ARCH_MA35) += ma35d1-iot-512m.dtb
>  dtb-$(CONFIG_ARCH_MA35) += ma35d1-som-256m.dtb
>  dtb-$(CONFIG_ARCH_NPCM) += nuvoton-npcm845-evb.dtb
> +dtb-$(CONFIG_ARCH_NPCM) += nuvoton-npcm845-yosemite4.dtb
> diff --git a/arch/arm64/boot/dts/nuvoton/nuvoton-npcm845-yosemite4.dts b/arch/arm64/boot/dts/nuvoton/nuvoton-npcm845-yosemite4.dts
> new file mode 100644
> index 000000000000..f6a6a47b1397
> --- /dev/null
> +++ b/arch/arm64/boot/dts/nuvoton/nuvoton-npcm845-yosemite4.dts
> @@ -0,0 +1,1493 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +// Copyright 2023 Facebook Inc.
> +
> +/dts-v1/;
> +#include "nuvoton-npcm845.dtsi"
> +#include "nuvoton-npcm845-pincfg-evb.dtsi"
> +#include <dt-bindings/i2c/i2c.h>
> +
> +/ {
> +	model = "Facebook Yosemite 4 BMC";
> +	compatible = "facebook,yosemite4-n-bmc", "nuvoton,npcm845";
> +
> +	aliases {
> +		serial4 = &serial0;
> +		serial0 = &serial1;
> +		serial1 = &serial3;
> +		serial2 = &serial4;
> +		serial3 = &serial5;
> +		serial5 = &cpld_serial0;
> +		serial6 = &cpld_serial1;
> +		serial7 = &cpld_serial2;
> +		serial8 = &cpld_serial3;
> +		fiu0 = &fiu0;
> +
> +		i2c16 = &imux16;
> +		i2c17 = &imux17;
> +		i2c18 = &imux18;
> +		i2c19 = &imux19;
> +		i2c20 = &imux20;
> +		i2c21 = &imux21;
> +		i2c22 = &imux22;
> +		i2c23 = &imux23;
> +		i2c24 = &imux24;
> +		i2c25 = &imux25;
> +		i2c26 = &imux26;
> +		i2c27 = &imux27;
> +		i2c28 = &imux28;
> +		i2c29 = &imux29;
> +		i2c30 = &imux30;
> +		i2c31 = &imux31;
> +		i2c32 = &imux32;
> +		i2c33 = &imux33;
> +		i2c34 = &imux34;
> +		i2c35 = &imux35;
> +		i2c36 = &imux36;
> +		i2c37 = &imux37;
> +	};
> +
> +	chosen {
> +		stdout-path = &serial0;
> +	};
> +
> +	memory {
> +		device_type = "memory";
> +		reg = <0x0 0x0 0x0 0x40000000>;
> +	};
> +
> +	iio-hwmon {
> +		compatible = "iio-hwmon";
> +		io-channels = <&adc 0>, <&adc 1>, <&adc 2>, <&adc 3>,
> +			<&adc 4>, <&adc 5>, <&adc 6>, <&adc 7>;
> +	};
> +
> +	firmware {
> +		optee {
> +			compatible = "linaro,optee-tz";
> +			method = "smc";
> +		};
> +	};
> +
> +	reserved-memory {
> +		#address-cells = <2>;
> +		#size-cells = <2>;
> +		ranges;
> +
> +		tip_reserved: tip@0x0 {
> +			reg = <0x0 0x0 0x0 0x6200000>;
> +		};
> +	};
> +
> +	spi_gpio: spi-gpio {
> +		compatible = "spi-gpio";
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +		gpio-sck = <&gpio0 19 GPIO_ACTIVE_HIGH>; // GPIO19
> +		gpio-mosi = <&gpio0 18 GPIO_ACTIVE_HIGH>; // GPIO18
> +		gpio-miso = <&gpio0 17 GPIO_ACTIVE_HIGH>; // GPIO17
> +		num-chipselects = <1>;
> +		cs-gpios = <&gpio6 11 GPIO_ACTIVE_LOW>; // GPIO203
> +
> +		tpmdev@0 {

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 = "tcg,tpm_tis-spi";

Please base on ongoing work adding specific compatibles.

> +			spi-max-frequency = <33000000>;
> +			reg = <0>;

reg is always after compatible.

> +		};
> +	};
> +
> +	cpld_serial0: cpld_uart@...00800 {

Eh... so again you make the same mistakes and send the same downstream
poor code with the same bad patterns we asked to fix.

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


> +		device_type = "serial";

compatible is always first, reg is second. Why do you need this property
anyway?

> +		compatible = "ns16450";
> +		reg = <0x0 0xf8000800 0x0 0x200>;
> +		reg-shift = <0>;
> +		clocks = <&clk NPCM8XX_CLK_UART>;
> +		interrupt-parent = <&gpio1>;
> +		interrupts = <15 IRQ_TYPE_EDGE_FALLING>;
> +	};
> +
> +	cpld_serial1: cpld_uart@...00a00 {
> +		device_type = "serial";
> +		compatible = "ns16450";
> +		reg = <0x0 0xf8000a00 0x0 0x200>;
> +		reg-shift = <0>;
> +		clocks = <&clk NPCM8XX_CLK_UART>;
> +		interrupt-parent = <&gpio1>;
> +		interrupts = <15 IRQ_TYPE_EDGE_FALLING>;
> +	};
> +
> +	cpld_serial2: cpld_uart@...00c00 {
> +		device_type = "serial";
> +		compatible = "ns16450";
> +		reg = <0x0 0xf8000c00 0x0 0x200>;
> +		reg-shift = <0>;
> +		clocks = <&clk NPCM8XX_CLK_UART>;
> +		interrupt-parent = <&gpio1>;
> +		interrupts = <15 IRQ_TYPE_EDGE_FALLING>;
> +	};
> +
> +	cpld_serial3: cpld_uart@...00e00 {
> +		device_type = "serial";
> +		compatible = "ns16450";
> +		reg = <0x0 0xf8000e00 0x0 0x200>;
> +		reg-shift = <0>;
> +		clocks = <&clk NPCM8XX_CLK_UART>;
> +		interrupt-parent = <&gpio1>;
> +		interrupts = <15 IRQ_TYPE_EDGE_FALLING>;
> +	};
> +};
> +
> +&serial0 {
> +	status = "okay";
> +};
> +
> +&serial1 {
> +	status = "okay";
> +};
> +
> +&serial3 {
> +	status = "okay";
> +};
> +
> +&serial4 {
> +	status = "okay";
> +};
> +
> +&serial5 {
> +	status = "okay";
> +};
> +
> +&watchdog1 {
> +	status = "okay";
> +};
> +
> +&watchdog2 {
> +	status = "okay";
> +};
> +
> +&gmac2 {
> +	status = "okay";
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&r1_pins
> +			&r1oen_pins>;
> +	use-ncsi;
> +};
> +
> +&gmac3 {
> +	status = "okay";
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&r2_pins
> +			&r2oen_pins>;
> +	use-ncsi;
> +};
> +
> +&fiu0 {
> +	status = "okay";
> +	spi-nor@0 {

NAK

I am not going review further. You keep repeating the same mistakes.

Best regards,
Krzysztof


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ