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

[Krzysztof Kozlowski]

On 01/06/2023 07:45, Nikita Shubin wrote:
> From: Alexander Sverdlin <alexander.sverdlin@xxxxxxxxx>
> 
> Add device tree for Cirrus EDB9302.
> 
> Signed-off-by: Alexander Sverdlin <alexander.sverdlin@xxxxxxxxx>
> Signed-off-by: Nikita Shubin <nikita.shubin@xxxxxxxxxxx>
> ---
> 
> 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@60000000 {

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