Thank you for your patch. There is something to discuss/improve. On 28/05/2023 02:10, Rudraksha Gupta wrote: > This adds a very basic device tree file for the Samsung Galaxy Express Please do not use "This commit/patch", but imperative mood. See longer explanation here: https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst#L95 > SGH-I437. Currently, the following things work: UART, eMMC, SD Card, and > USB. > > Signed-off-by: Rudraksha Gupta <guptarud@xxxxxxxxx> > --- > arch/arm/boot/dts/Makefile | 1 + > .../dts/qcom-msm8960-samsung-expressatt.dts | 334 ++++++++++++++++++ > 2 files changed, 335 insertions(+) > create mode 100644 arch/arm/boot/dts/qcom-msm8960-samsung-expressatt.dts > > diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile > index 59829fc90315..12c90f263142 100644 > --- a/arch/arm/boot/dts/Makefile > +++ b/arch/arm/boot/dts/Makefile > @@ -1081,6 +1081,7 @@ dtb-$(CONFIG_ARCH_QCOM) += \ > qcom-msm8916-samsung-grandmax.dtb \ > qcom-msm8916-samsung-serranove.dtb \ > qcom-msm8960-cdp.dtb \ > + qcom-msm8960-samsung-expressatt.dtb \ > qcom-msm8974-lge-nexus5-hammerhead.dtb \ > qcom-msm8974-sony-xperia-rhine-amami.dtb \ > qcom-msm8974-sony-xperia-rhine-honami.dtb \ > diff --git a/arch/arm/boot/dts/qcom-msm8960-samsung-expressatt.dts b/arch/arm/boot/dts/qcom-msm8960-samsung-expressatt.dts > new file mode 100644 > index 000000000000..a1ee9c272558 > --- /dev/null > +++ b/arch/arm/boot/dts/qcom-msm8960-samsung-expressatt.dts > @@ -0,0 +1,334 @@ > +// SPDX-License-Identifier: GPL-2.0 > +#include <dt-bindings/input/input.h> > + > +#include "qcom-msm8960.dtsi" > +#include <dt-bindings/reset/qcom,gcc-msm8960.h> > + > +/ { > + model = "Samsung Galaxy Express SGH-I437"; > + compatible = "samsung,expressatt", "qcom,msm8960"; > + chassis-type = "handset"; > + > + aliases { > + serial0 = &gsbi5_serial; > + mmc0 = &sdcc1; /* SDCC1 eMMC slot */ > + mmc1 = &sdcc3; /* SDCC3 SD card slot */ > + }; > + > + chosen { > + stdout-path = "serial0:115200n8"; > + }; > +}; > + > +&gsbi5 { > + qcom,mode = <GSBI_PROT_I2C_UART>; > + status = "okay"; > +}; > + > +&gsbi5_serial { > + status = "okay"; > +}; > + > +&sdcc1 { > + vmmc-supply = <&pm8921_l5>; > + status = "okay"; > +}; > + > +&sdcc3 { > + vmmc-supply = <&pm8921_l6>; > + vqmmc-supply = <&pm8921_l7>; > + status = "okay"; > +}; > + > +&gsbi1 { > + qcom,mode = <GSBI_PROT_SPI>; > + pinctrl-0 = <&spi1_default>; > + pinctrl-names = "default"; > + status = "okay"; > +}; > + > +&gsbi1_spi { > + status = "okay"; > +}; > + > +&msmgpio { > + spi1_default: spi1_default { No underscores in node names, missing proper suffix. It does not look like you tested the DTS against bindings. Please run `make dtbs_check` (see Documentation/devicetree/bindings/writing-schema.rst for instructions). > + mux { It does not look like you tested the DTS against bindings. Please run `make dtbs_check` (see Documentation/devicetree/bindings/writing-schema.rst for instructions). > + pins = "gpio6", "gpio7", "gpio9"; > + function = "gsbi1"; > + }; > + > + mosi { It does not look like you tested the DTS against bindings. Please run `make dtbs_check` (see Documentation/devicetree/bindings/writing-schema.rst for instructions). > + pins = "gpio6"; > + drive-strength = <12>; > + bias-disable; > + }; > + > + miso { > + pins = "gpio7"; > + drive-strength = <12>; > + bias-disable; > + }; > + > + cs { > + pins = "gpio8"; > + drive-strength = <12>; > + bias-disable; > + output-low; > + }; > + > + clk { > + pins = "gpio9"; > + drive-strength = <12>; > + bias-disable; > + }; > + }; > +}; > + > + One blank line, not two. ... > +&usb1 { > + dr_mode = "otg"; > + status = "okay"; > +}; > + No improvements here. Best regards, Krzysztof
