linux-kernel - Re: [PATCH v1 42/43] ARM: dts: ep93xx: Add EDB9302 DT

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]

Message-ID: <147fba8a-8c3b-6ed5-22ef-599d9f52ca1d@linaro.org>
Date:   Thu, 1 Jun 2023 10:33:25 +0200
From:   Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To:     Nikita Shubin <nikita.shubin@...uefel.me>,
        Alexander Sverdlin <alexander.sverdlin@...il.com>,
        Arnd Bergmann <arnd@...db.de>,
        Linus Walleij <linus.walleij@...aro.org>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>
Cc:     Michael Peters <mpeters@...eddedTS.com>,
        Kris Bahnsen <kris@...eddedTS.com>, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v1 42/43] ARM: dts: ep93xx: Add EDB9302 DT

On 01/06/2023 07:45, Nikita Shubin wrote:
> From: Alexander Sverdlin <alexander.sverdlin@...il.com>
> 
> Add device tree for Cirrus EDB9302.
> 
> Signed-off-by: Alexander Sverdlin <alexander.sverdlin@...il.com>
> Signed-off-by: Nikita Shubin <nikita.shubin@...uefel.me>
> ---
> 
> Notes:
>     v0 -> v1:
>     
>     - added USB
>     - dropped 'Missing USB' in commit message
>     - add mdio + eth phy
> 
>  arch/arm/boot/dts/ep93xx-edb9302.dts | 160 +++++++++++++++++++++++++++
>  1 file changed, 160 insertions(+)
>  create mode 100644 arch/arm/boot/dts/ep93xx-edb9302.dts
> 
> diff --git a/arch/arm/boot/dts/ep93xx-edb9302.dts b/arch/arm/boot/dts/ep93xx-edb9302.dts
> new file mode 100644
> index 000000000000..3ec89f7587db
> --- /dev/null
> +++ b/arch/arm/boot/dts/ep93xx-edb9302.dts
> @@ -0,0 +1,160 @@
> +// SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +/*
> + * Device Tree file for Cirrus Logic EDB9302 board based on EP9302 SoC
> + */
> +/dts-v1/;
> +#include "ep93xx.dtsi"
> +
> +/ {
> +	#address-cells = <1>;
> +	#size-cells = <1>;
> +	compatible = "cirrus,edb9302", "cirrus,ep9301";
> +	model = "cirrus,edb9302";
> +
> +	chosen {
> +	};
> +
> +	memory {
> +		device_type = "memory";
> +	};
> +
> +	soc {
> +		flash@...00000 {

Same comments - override by label/phandle.

> +			compatible = "cfi-flash";
> +			reg = <0x60000000 0x1000000>;

Are you sure that your board adds things to the SoC? The code suggests
that, but I would like to see such circuit.

> +			bank-width = <2>;
> +		};
> +	};
> +
> +	sound {
> +		compatible = "simple-audio-card";
> +		simple-audio-card,name = "EDB93XX";
> +		simple-audio-card,format = "i2s";
> +		simple-audio-card,mclk-fs = <256>;
> +		simple-audio-card,convert-channels = <2>;
> +		simple-audio-card,convert-sample-format = "s32_le";
> +
> +		simple-audio-card,cpu {
> +			sound-dai = <&i2s>;
> +			system-clock-direction-out;
> +			frame-master;
> +			bitclock-master;
> +			dai-sample-format = "s32_le";
> +			dai-channels = <2>;
> +		};
> +
> +		simple-audio-card,codec {
> +			sound-dai = <&codec>;
> +		};
> +	};
> +};
> +
> +&pinctrl {
> +	compatible = "cirrus,ep9301-pinctrl";
> +};
> +
> +&gpio0 {
> +	gpio-ranges = <&pinctrl 0 153 1>,
> +		      <&pinctrl 1 152 1>,
> +		      <&pinctrl 2 151 1>,
> +		      <&pinctrl 3 148 1>,
> +		      <&pinctrl 4 147 1>,
> +		      <&pinctrl 5 146 1>,
> +		      <&pinctrl 6 145 1>,
> +		      <&pinctrl 7 144 1>;
> +};
> +
> +&gpio1 {
> +	gpio-ranges = <&pinctrl 0 143 1>,
> +		      <&pinctrl 1 142 1>,
> +		      <&pinctrl 2 141 1>,
> +		      <&pinctrl 3 140 1>,
> +		      <&pinctrl 4 165 1>,
> +		      <&pinctrl 5 164 1>,
> +		      <&pinctrl 6 163 1>,
> +		      <&pinctrl 7 160 1>;
> +};
> +
> +&gpio2 {
> +	gpio-ranges = <&pinctrl 0 115 1>;
> +	/delete-property/ status;

???

> +	/delete-property/ pinctrl-0;
> +	/delete-property/ pinctrl-names;

???

I have no clue what you are trying to achieve here but this is a proof
your DTSI is bogus.

You should never remove statuses. You should never need to remove
pinctrl as these are board dependent, not SoC. If you remove them, it
means they are not part of SoC in the first place!
Best regards,
Krzysztof