[PATCH v2 0/4] Samsung Galaxy Express SGH-I437 Support

[PATCH v2 0/4] Samsung Galaxy Express SGH-I437 Support

[Rudraksha Gupta]

This patch series adds support for the Samsung Galaxy Express SGH-I437.
Currently the following things work on this phone: UART, eMMC, SD Card, and USB.

version 2:
- Combined patch 1 into patch 4, as the sleep_clk label is specifically needed for the USB node.
- Reformatted the commit messages to align with the style used in other commit messages that modify the same files.
- Included a cover letter to provide an overview of the patch series.
- Slight refactoring of the device tree source (DTS) file.

Rudraksha Gupta (4):
  dt-bindings: arm: qcom: Add Samsung Galaxy Express
  dt-bindings: Add qcom,usb-hs-phy-msm8960
  ARM: dts: qcom: msm8960: Add USB node
  ARM: dts: qcom: Add Samsung Galaxy Express support

 .../devicetree/bindings/arm/qcom.yaml         |   1 +
 .../bindings/phy/qcom,usb-hs-phy.yaml         |   1 +
 arch/arm/boot/dts/Makefile                    |   1 +
 .../dts/qcom-msm8960-samsung-expressatt.dts   | 334 ++++++++++++++++++
 arch/arm/boot/dts/qcom-msm8960.dtsi           |  35 +-
 5 files changed, 371 insertions(+), 1 deletion(-)
 create mode 100644 arch/arm/boot/dts/qcom-msm8960-samsung-expressatt.dts

-- 
2.34.1

Re: [PATCH v2 0/4] Samsung Galaxy Express SGH-I437 Support

[Krzysztof Kozlowski]

On 28/05/2023 02:10, Rudraksha Gupta wrote:
> This patch series adds support for the Samsung Galaxy Express SGH-I437.
> Currently the following things work on this phone: UART, eMMC, SD Card, and USB.
> 
> version 2:
> - Combined patch 1 into patch 4, as the sleep_clk label is specifically needed for the USB node.
> - Reformatted the commit messages to align with the style used in other commit messages that modify the same files.
> - Included a cover letter to provide an overview of the patch series.
> - Slight refactoring of the device tree source (DTS) file.
> 

Do not attach (thread) your patchsets to some other threads (unrelated
or older versions). This buries them deep in the mailbox and might
interfere with applying entire sets.

Best regards,
Krzysztof

[PATCH v2 1/4] dt-bindings: arm: qcom: Add Samsung Galaxy Express

[Rudraksha Gupta]

Add a compatible for Samsung Galaxy Express SGH-I437.

Signed-off-by: Rudraksha Gupta <guptarud@gmail.com>
---
 Documentation/devicetree/bindings/arm/qcom.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/arm/qcom.yaml b/Documentation/devicetree/bindings/arm/qcom.yaml
index d9dd25695c3d..133d69b2b73d 100644
--- a/Documentation/devicetree/bindings/arm/qcom.yaml
+++ b/Documentation/devicetree/bindings/arm/qcom.yaml
@@ -191,6 +191,7 @@ properties:
       - items:
           - enum:
               - qcom,msm8960-cdp
+              - samsung,expressatt
           - const: qcom,msm8960
 
       - items:
-- 
2.34.1

Re: [PATCH v2 1/4] dt-bindings: arm: qcom: Add Samsung Galaxy Express

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 831 bytes --]

On Sat, May 27, 2023 at 08:10:06PM -0400, Rudraksha Gupta wrote:
> Add a compatible for Samsung Galaxy Express SGH-I437.
> 
> Signed-off-by: Rudraksha Gupta <guptarud@gmail.com>
> ---
>  Documentation/devicetree/bindings/arm/qcom.yaml | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/devicetree/bindings/arm/qcom.yaml b/Documentation/devicetree/bindings/arm/qcom.yaml
> index d9dd25695c3d..133d69b2b73d 100644
> --- a/Documentation/devicetree/bindings/arm/qcom.yaml
> +++ b/Documentation/devicetree/bindings/arm/qcom.yaml
> @@ -191,6 +191,7 @@ properties:
>        - items:
>            - enum:
>                - qcom,msm8960-cdp
> +              - samsung,expressatt

Where did the "att" come from in the compatible. Is this some carrier
specific model of the phone?

Cheers,
Conor.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v2 1/4] dt-bindings: arm: qcom: Add Samsung Galaxy Express

[Rudraksha Gupta]

 > Where did the "att" come from in the compatible. Is this some carrier 
specific model of the phone?

This is the code name for the device. Since there are usually multiple 
variants of a device, using the code name differentiates between those 
variants. For example, if I left this as "samsung,express", it would be 
unclear if I am referring to the GT-I8730 (code name: expresslte) or the 
SGH-I437 model. This is typically done in postmarketOS: 
https://wiki.postmarketos.org/wiki/Devices and XDA developers. I believe 
it is a carrier specific model of the Samsung Galaxy Express.

Re: [PATCH v2 1/4] dt-bindings: arm: qcom: Add Samsung Galaxy Express

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 950 bytes --]

On Sun, May 28, 2023 at 09:36:37AM -0400, Rudraksha Gupta wrote:
> > Where did the "att" come from in the compatible. Is this some carrier
> specific model of the phone?
> 
> This is the code name for the device. Since there are usually multiple
> variants of a device, using the code name differentiates between those
> variants. For example, if I left this as "samsung,express", it would be
> unclear if I am referring to the GT-I8730 (code name: expresslte) or the
> SGH-I437 model. This is typically done in postmarketOS:
> https://wiki.postmarketos.org/wiki/Devices and XDA developers. I believe it
> is a carrier specific model of the Samsung Galaxy Express.

Okay, thanks for the information. I'll not dispute the codename, was
wondering whether there were other devices or not. I'll let the qcom
folks decide whether or not that is the right codename..
Acked-by: Conor Dooley <conor.dooley@microchip.com>

Thanks,
Conor.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v2 1/4] dt-bindings: arm: qcom: Add Samsung Galaxy Express

[Konrad Dybcio]

On 28.05.2023 15:36, Rudraksha Gupta wrote:
>> Where did the "att" come from in the compatible. Is this some carrier specific model of the phone?
> 
> This is the code name for the device. Since there are usually multiple variants of a device, using the code name differentiates between those variants. For example, if I left this as "samsung,express", it would be unclear if I am referring to the GT-I8730 (code name: expresslte) or the SGH-I437 model. This is typically done in postmarketOS: https://wiki.postmarketos.org/wiki/Devices and XDA developers. I believe it is a carrier specific model of the Samsung Galaxy Express.
> 

Please fix your email client:

- wrap each line at about 80 characters
- don't trim messages unless they're very long or
  there's some other good reason
- Don't send v(n+1) as a reply to v(n), send it in a
  separate thread.



Konrad

Re: [PATCH v2 1/4] dt-bindings: arm: qcom: Add Samsung Galaxy Express

[Rudraksha Gupta]

On Mon May 29, 2023 at 3:41 AM EDT, Konrad Dybcio wrote:
>
>
> On 28.05.2023 15:36, Rudraksha Gupta wrote:
> >> Where did the "att" come from in the compatible. Is this some carrier specific model of the phone?
> > 
> > This is the code name for the device. Since there are usually multiple variants of a device, using the code name differentiates between those variants. For example, if I left this as "samsung,express", it would be unclear if I am referring to the GT-I8730 (code name: expresslte) or the SGH-I437 model. This is typically done in postmarketOS: https://wiki.postmarketos.org/wiki/Devices and XDA developers. I believe it is a carrier specific model of the Samsung Galaxy Express.
> > 
>
> Please fix your email client:
>
> - wrap each line at about 80 characters
> - don't trim messages unless they're very long or
>   there's some other good reason
> - Don't send v(n+1) as a reply to v(n), send it in a
>   separate thread.
>
Gotcha, I will do that next time. Should I still send v2 in a separate
thread? Not sure if I should as I don't want to spam anyone.

>
>
> Konrad

Re: [PATCH v2 1/4] dt-bindings: arm: qcom: Add Samsung Galaxy Express

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 1173 bytes --]

On Mon, May 29, 2023 at 06:28:05PM -0400, Rudraksha Gupta wrote:
> On Mon May 29, 2023 at 3:41 AM EDT, Konrad Dybcio wrote:

> > Please fix your email client:
> >
> > - wrap each line at about 80 characters
> > - don't trim messages unless they're very long or
> >   there's some other good reason
> > - Don't send v(n+1) as a reply to v(n), send it in a
> >   separate thread.
> >
> Gotcha, I will do that next time. Should I still send v2 in a separate
> thread?

That'd then be either [PATCH v3], or [RESEND PATCH v2]. Ideally v3 since
you will have added the tags you received on v2.
AFAIK Bjorn uses b4, which should be able to deal with the series as-is,
but sending a v3 with the Acks etc would not be a bad idea. You
should probably give it a couple days before doing that to see if it
gets picked up before doing that.

> Not sure if I should as I don't want to spam anyone.

Sending new revisions of patches as a new thread is not spamming, we
signed up to be maintainers, or get to email from the lists, after all.
Sending several revisions of the same patches in a day would be spamming
though, but that is not what we are talking about here ;)

Cheers,
Conor.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v2 1/4] dt-bindings: arm: qcom: Add Samsung Galaxy Express

[Rudraksha Gupta]

On Mon May 29, 2023 at 6:37 PM EDT, Conor Dooley wrote:
> On Mon, May 29, 2023 at 06:28:05PM -0400, Rudraksha Gupta wrote:
>
> > Gotcha, I will do that next time. Should I still send v2 in a separate
> > thread?
>
> That'd then be either [PATCH v3], or [RESEND PATCH v2]. Ideally v3 since
> you will have added the tags you received on v2.
> AFAIK Bjorn uses b4, which should be able to deal with the series as-is,
> but sending a v3 with the Acks etc would not be a bad idea. You
> should probably give it a couple days before doing that to see if it
> gets picked up before doing that.
>
> > Not sure if I should as I don't want to spam anyone.
>
> Sending new revisions of patches as a new thread is not spamming, we
> signed up to be maintainers, or get to email from the lists, after all.
> Sending several revisions of the same patches in a day would be spamming
> though, but that is not what we are talking about here ;)
>
> Cheers,
> Conor.

Ah, I see. Thank you for being patient with me. I'm really excited to see my
patch go in! :)

Re: [PATCH v2 1/4] dt-bindings: arm: qcom: Add Samsung Galaxy Express

[Krzysztof Kozlowski]

On Sat, 27 May 2023 20:10:06 -0400, Rudraksha Gupta wrote:
> Add a compatible for Samsung Galaxy Express SGH-I437.
> 
> Signed-off-by: Rudraksha Gupta <guptarud@gmail.com>
> ---
>  Documentation/devicetree/bindings/arm/qcom.yaml | 1 +
>  1 file changed, 1 insertion(+)
> 

Running 'make dtbs_check' with the schema in this patch gives the
following warnings. Consider if they are expected or the schema is
incorrect. These may not be new warnings.

Note that it is not yet a requirement to have 0 warnings for dtbs_check.
This will change in the future.

Full log is available here: https://patchwork.ozlabs.org/patch/1786779


/: qcom,board-id: False schema does not allow [[8, 0]]
	arch/arm/boot/dts/qcom-msm8974pro-oneplus-bacon.dtb

/: qcom,msm-id: False schema does not allow [[194, 65536]]
	arch/arm/boot/dts/qcom-msm8974pro-oneplus-bacon.dtb

[PATCH v2 2/4] dt-bindings: Add qcom,usb-hs-phy-msm8960

[Rudraksha Gupta]

Adds qcom,usb-hs-phy-msm8960 compatible

Signed-off-by: Rudraksha Gupta <guptarud@gmail.com>
---
 Documentation/devicetree/bindings/phy/qcom,usb-hs-phy.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/phy/qcom,usb-hs-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,usb-hs-phy.yaml
index aa97478dd016..6309c8e9d0b0 100644
--- a/Documentation/devicetree/bindings/phy/qcom,usb-hs-phy.yaml
+++ b/Documentation/devicetree/bindings/phy/qcom,usb-hs-phy.yaml
@@ -40,6 +40,7 @@ properties:
           - qcom,usb-hs-phy-apq8064
           - qcom,usb-hs-phy-msm8226
           - qcom,usb-hs-phy-msm8916
+          - qcom,usb-hs-phy-msm8960
           - qcom,usb-hs-phy-msm8974
       - const: qcom,usb-hs-phy
 
-- 
2.34.1

Re: [PATCH v2 2/4] dt-bindings: Add qcom,usb-hs-phy-msm8960

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 242 bytes --]

On Sat, May 27, 2023 at 08:10:07PM -0400, Rudraksha Gupta wrote:
> Adds qcom,usb-hs-phy-msm8960 compatible
> 
> Signed-off-by: Rudraksha Gupta <guptarud@gmail.com>

Acked-by: Conor Dooley <conor.dooley@microchip.com>

Thanks,
Conor.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v2 2/4] dt-bindings: Add qcom,usb-hs-phy-msm8960

[Krzysztof Kozlowski]

On 28/05/2023 02:10, Rudraksha Gupta wrote:
> Adds qcom,usb-hs-phy-msm8960 compatible
> 

Please use subject prefixes matching the subsystem. You can get them for
example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
your patch is touching.

Best regards,
Krzysztof

Re: [PATCH v2 2/4] dt-bindings: Add qcom,usb-hs-phy-msm8960

[Rudraksha Gupta]

On Tue May 30, 2023 at 9:22 AM EDT, Krzysztof Kozlowski wrote:
> On 28/05/2023 02:10, Rudraksha Gupta wrote:
> > Adds qcom,usb-hs-phy-msm8960 compatible
> > 
>
> Please use subject prefixes matching the subsystem. You can get them for
> example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
> your patch is touching.
>
> Best regards,
> Krzysztof

It seems like "dt-bindings:" is the subject prefix for this file. Would
you like me to use another prefix instead?

Re: [PATCH v2 2/4] dt-bindings: Add qcom,usb-hs-phy-msm8960

[Dmitry Baryshkov]

On Wed, 31 May 2023 at 05:49, Rudraksha Gupta <guptarud@gmail.com> wrote:
>
> On Tue May 30, 2023 at 9:22 AM EDT, Krzysztof Kozlowski wrote:
> > On 28/05/2023 02:10, Rudraksha Gupta wrote:
> > > Adds qcom,usb-hs-phy-msm8960 compatible
> > >
> >
> > Please use subject prefixes matching the subsystem. You can get them for
> > example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
> > your patch is touching.
> >
> > Best regards,
> > Krzysztof
>
> It seems like "dt-bindings:" is the subject prefix for this file. Would
> you like me to use another prefix instead?

At least it should be "dt-bindings: phy: ". However as this change
covers existing file, it probably should be "dt-bindings: phy:
qcom,usb-hs-phy: "

-- 
With best wishes
Dmitry

[PATCH v2 3/4] ARM: dts: qcom: msm8960: Add USB node

[Rudraksha Gupta]

Add the required nodes to support USB on the MSM8960 SoC. As it's very
similar to the APQ8064 SoC, the nodes are almost identical

Add a label to sleep_clk for the USB node to reference

Signed-off-by: Rudraksha Gupta <guptarud@gmail.com>
---
 arch/arm/boot/dts/qcom-msm8960.dtsi | 35 ++++++++++++++++++++++++++++-
 1 file changed, 34 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/qcom-msm8960.dtsi b/arch/arm/boot/dts/qcom-msm8960.dtsi
index 2a668cd535cc..b11e606440cd 100644
--- a/arch/arm/boot/dts/qcom-msm8960.dtsi
+++ b/arch/arm/boot/dts/qcom-msm8960.dtsi
@@ -3,6 +3,7 @@
 
 #include <dt-bindings/interrupt-controller/arm-gic.h>
 #include <dt-bindings/clock/qcom,gcc-msm8960.h>
+#include <dt-bindings/reset/qcom,gcc-msm8960.h>
 #include <dt-bindings/clock/qcom,lcc-msm8960.h>
 #include <dt-bindings/mfd/qcom-rpm.h>
 #include <dt-bindings/soc/qcom,gsbi.h>
@@ -71,7 +72,7 @@ pxo_board: pxo_board {
 			clock-output-names = "pxo_board";
 		};
 
-		sleep_clk {
+		sleep_clk: sleep_clk {
 			compatible = "fixed-clock";
 			#clock-cells = <0>;
 			clock-frequency = <32768>;
@@ -370,5 +371,37 @@ gsbi1_spi: spi@16080000 {
 				status = "disabled";
 			};
 		};
+
+		usb1: usb@12500000 {
+			compatible = "qcom,ci-hdrc";
+			reg = <0x12500000 0x200>,
+			      <0x12500200 0x200>;
+			interrupts = <GIC_SPI 100 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&gcc USB_HS1_XCVR_CLK>, <&gcc USB_HS1_H_CLK>;
+			clock-names = "core", "iface";
+			assigned-clocks = <&gcc USB_HS1_XCVR_CLK>;
+			assigned-clock-rates = <60000000>;
+			resets = <&gcc USB_HS1_RESET>;
+			reset-names = "core";
+			phy_type = "ulpi";
+			ahb-burst-config = <0>;
+			phys = <&usb_hs1_phy>;
+			phy-names = "usb-phy";
+			status = "disabled";
+			#reset-cells = <1>;
+
+			ulpi {
+				usb_hs1_phy: phy {
+					compatible = "qcom,usb-hs-phy-msm8960",
+						     "qcom,usb-hs-phy";
+					clocks = <&sleep_clk>, <&cxo_board>;
+					clock-names = "sleep", "ref";
+					resets = <&usb1 0>;
+					reset-names = "por";
+					#phy-cells = <0>;
+				};
+			};
+		};
+
 	};
 };
-- 
2.34.1

Re: [PATCH v2 3/4] ARM: dts: qcom: msm8960: Add USB node

[Krzysztof Kozlowski]

On 28/05/2023 02:10, Rudraksha Gupta wrote:
> Add the required nodes to support USB on the MSM8960 SoC. As it's very
> similar to the APQ8064 SoC, the nodes are almost identical
> 
> Add a label to sleep_clk for the USB node to reference
> 
> Signed-off-by: Rudraksha Gupta <guptarud@gmail.com>


> +		usb1: usb@12500000 {
> +			compatible = "qcom,ci-hdrc";
> +			reg = <0x12500000 0x200>,
> +			      <0x12500200 0x200>;
> +			interrupts = <GIC_SPI 100 IRQ_TYPE_LEVEL_HIGH>;
> +			clocks = <&gcc USB_HS1_XCVR_CLK>, <&gcc USB_HS1_H_CLK>;
> +			clock-names = "core", "iface";
> +			assigned-clocks = <&gcc USB_HS1_XCVR_CLK>;
> +			assigned-clock-rates = <60000000>;
> +			resets = <&gcc USB_HS1_RESET>;
> +			reset-names = "core";
> +			phy_type = "ulpi";
> +			ahb-burst-config = <0>;
> +			phys = <&usb_hs1_phy>;
> +			phy-names = "usb-phy";
> +			status = "disabled";

status is the last

> +			#reset-cells = <1>;
> +
> +			ulpi {
> +				usb_hs1_phy: phy {
> +					compatible = "qcom,usb-hs-phy-msm8960",
> +						     "qcom,usb-hs-phy";
> +					clocks = <&sleep_clk>, <&cxo_board>;
> +					clock-names = "sleep", "ref";
> +					resets = <&usb1 0>;
> +					reset-names = "por";
> +					#phy-cells = <0>;
> +				};
> +			};
> +		};
> +

Don't add stray blank lines.

Best regards,
Krzysztof

[PATCH v2 4/4] ARM: dts: qcom: Add Samsung Galaxy Express support

[Rudraksha Gupta]

This adds a very basic device tree file for the Samsung Galaxy Express
SGH-I437. Currently, the following things work: UART, eMMC, SD Card, and
USB.

Signed-off-by: Rudraksha Gupta <guptarud@gmail.com>
---
 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 {
+		mux {
+			pins = "gpio6", "gpio7", "gpio9";
+			function = "gsbi1";
+		};
+
+		mosi {
+			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;
+		};
+	};
+};
+
+
+&rpm {
+	regulators {
+		compatible = "qcom,rpm-pm8921-regulators";
+		vin_lvs1_3_6-supply = <&pm8921_s4>;
+		vin_lvs2-supply = <&pm8921_s4>;
+		vin_lvs4_5_7-supply = <&pm8921_s4>;
+		vdd_ncp-supply = <&pm8921_l6>;
+		vdd_l1_l2_l12_l18-supply = <&pm8921_s4>;
+		vdd_l21_l23_l29-supply = <&pm8921_s8>;
+		vdd_l24-supply = <&pm8921_s1>;
+		vdd_l25-supply = <&pm8921_s1>;
+		vdd_l27-supply = <&pm8921_s7>;
+		vdd_l28-supply = <&pm8921_s7>;
+
+		/* Buck SMPS */
+		pm8921_s1: s1 {
+			regulator-always-on;
+			regulator-min-microvolt = <1225000>;
+			regulator-max-microvolt = <1225000>;
+			qcom,switch-mode-frequency = <3200000>;
+			bias-pull-down;
+		};
+
+		pm8921_s2: s2 {
+			regulator-min-microvolt = <1300000>;
+			regulator-max-microvolt = <1300000>;
+			qcom,switch-mode-frequency = <1600000>;
+			bias-pull-down;
+		};
+
+		pm8921_s3: s3 {
+			regulator-min-microvolt = <500000>;
+			regulator-max-microvolt = <1150000>;
+			qcom,switch-mode-frequency = <4800000>;
+			bias-pull-down;
+		};
+
+		pm8921_s4: s4 {
+			regulator-always-on;
+			regulator-min-microvolt = <1800000>;
+			regulator-max-microvolt = <1800000>;
+			qcom,switch-mode-frequency = <1600000>;
+			bias-pull-down;
+			qcom,force-mode = <QCOM_RPM_FORCE_MODE_AUTO>;
+		};
+
+		pm8921_s7: s7 {
+			regulator-min-microvolt = <1150000>;
+			regulator-max-microvolt = <1150000>;
+			qcom,switch-mode-frequency = <3200000>;
+			bias-pull-down;
+		};
+
+		pm8921_s8: s8 {
+			regulator-always-on;
+			regulator-min-microvolt = <2050000>;
+			regulator-max-microvolt = <2050000>;
+			qcom,switch-mode-frequency = <1600000>;
+			bias-pull-down;
+		};
+
+		/* PMOS LDO */
+		pm8921_l1: l1 {
+			regulator-always-on;
+			regulator-min-microvolt = <1050000>;
+			regulator-max-microvolt = <1050000>;
+			bias-pull-down;
+		};
+
+		pm8921_l2: l2 {
+			regulator-min-microvolt = <1200000>;
+			regulator-max-microvolt = <1200000>;
+			bias-pull-down;
+		};
+
+		pm8921_l3: l3 {
+			regulator-min-microvolt = <3075000>;
+			regulator-max-microvolt = <3300000>;
+			bias-pull-down;
+		};
+
+		pm8921_l4: l4 {
+			regulator-always-on;
+			regulator-min-microvolt = <1800000>;
+			regulator-max-microvolt = <1800000>;
+			bias-pull-down;
+		};
+
+		pm8921_l5: l5 {
+			regulator-min-microvolt = <2950000>;
+			regulator-max-microvolt = <2950000>;
+			bias-pull-down;
+		};
+
+		pm8921_l6: l6 {
+			regulator-min-microvolt = <2950000>;
+			regulator-max-microvolt = <2950000>;
+			bias-pull-down;
+		};
+
+		pm8921_l7: l7 {
+			regulator-always-on;
+			regulator-min-microvolt = <1850000>;
+			regulator-max-microvolt = <2950000>;
+			bias-pull-down;
+		};
+
+		pm8921_l8: l8 {
+			regulator-min-microvolt = <3000000>;
+			regulator-max-microvolt = <3100000>;
+			bias-pull-down;
+		};
+
+		pm8921_l9: l9 {
+			regulator-min-microvolt = <2850000>;
+			regulator-max-microvolt = <2850000>;
+			bias-pull-down;
+		};
+
+		pm8921_l10: l10 {
+			regulator-min-microvolt = <3000000>;
+			regulator-max-microvolt = <3000000>;
+			bias-pull-down;
+		};
+
+		pm8921_l11: l11 {
+			regulator-min-microvolt = <2800000>;
+			regulator-max-microvolt = <3300000>;
+			bias-pull-down;
+		};
+
+		pm8921_l12: l12 {
+			regulator-min-microvolt = <1200000>;
+			regulator-max-microvolt = <1200000>;
+			bias-pull-down;
+		};
+
+		pm8921_l14: l14 {
+			regulator-min-microvolt = <1800000>;
+			regulator-max-microvolt = <1800000>;
+			bias-pull-down;
+		};
+
+		pm8921_l15: l15 {
+			regulator-min-microvolt = <1800000>;
+			regulator-max-microvolt = <2950000>;
+			bias-pull-down;
+		};
+
+		pm8921_l16: l16 {
+			regulator-min-microvolt = <2800000>;
+			regulator-max-microvolt = <3000000>;
+			bias-pull-down;
+		};
+
+		pm8921_l17: l17 {
+			regulator-min-microvolt = <1800000>;
+			regulator-max-microvolt = <3300000>;
+			bias-pull-down;
+		};
+
+		pm8921_l18: l18 {
+			regulator-min-microvolt = <1200000>;
+			regulator-max-microvolt = <1500000>;
+			bias-pull-down;
+		};
+
+		pm8921_l21: l21 {
+			regulator-min-microvolt = <1900000>;
+			regulator-max-microvolt = <1900000>;
+			bias-pull-down;
+		};
+
+		pm8921_l22: l22 {
+			regulator-min-microvolt = <2750000>;
+			regulator-max-microvolt = <2750000>;
+			bias-pull-down;
+		};
+
+		pm8921_l23: l23 {
+			regulator-always-on;
+			regulator-min-microvolt = <1800000>;
+			regulator-max-microvolt = <1800000>;
+			bias-pull-down;
+		};
+
+		pm8921_l24: l24 {
+			regulator-min-microvolt = <750000>;
+			regulator-max-microvolt = <1150000>;
+			bias-pull-down;
+		};
+
+		pm8921_l25: l25 {
+			regulator-always-on;
+			regulator-min-microvolt = <1225000>;
+			regulator-max-microvolt = <1225000>;
+			bias-pull-down;
+		};
+
+		/* Low Voltage Switch */
+		pm8921_lvs1: lvs1 {
+			bias-pull-down;
+		};
+
+		pm8921_lvs2: lvs2 {
+			bias-pull-down;
+		};
+
+		pm8921_lvs3: lvs3 {
+			bias-pull-down;
+		};
+
+		pm8921_lvs4: lvs4 {
+			bias-pull-down;
+		};
+
+		pm8921_lvs5: lvs5 {
+			bias-pull-down;
+		};
+
+		pm8921_lvs6: lvs6 {
+			bias-pull-down;
+		};
+
+		pm8921_lvs7: lvs7 {
+			bias-pull-down;
+		};
+
+		pm8921_ncp: ncp {
+			regulator-min-microvolt = <1800000>;
+			regulator-max-microvolt = <1800000>;
+			qcom,switch-mode-frequency = <1600000>;
+		};
+	};
+};
+
+&usb_hs1_phy {
+	v3p3-supply = <&pm8921_l3>;
+	v1p8-supply = <&pm8921_l4>;
+};
+
+&usb1 {
+	dr_mode = "otg";
+	status = "okay";
+};
+
-- 
2.34.1

Re: [PATCH v2 4/4] ARM: dts: qcom: Add Samsung Galaxy Express support

[Krzysztof Kozlowski]

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@gmail.com>
> ---
>  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