diff mbox series

[1/3] arm64: dts: ti: Add GPMC NAND support

Message ID	20230913114711.2937844-2-n-yadav@ti.com
State	New
Headers	Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:2 as permitted sender) client-ip=2620:137:e000::3:2; From: Nitin Yadav <n-yadav@ti.com> To: <nm@ti.com>, <vigneshr@ti.com>, <rogerq@ti.com> CC: <kristo@kernel.org>, <robh+dt@kernel.org>, <krzysztof.kozlowski+dt@linaro.org>, <conor+dt@kernel.org>, <catalin.marinas@arm.com>, <will@kernel.org>, <linux-arm-kernel@lists.infradead.org>, <devicetree@vger.kernel.org>, <linux-kernel@vger.kernel.org> Subject: [PATCH 1/3] arm64: dts: ti: Add GPMC NAND support Date: Wed, 13 Sep 2023 17:17:09 +0530 Message-ID: <20230913114711.2937844-2-n-yadav@ti.com> In-Reply-To: <20230913114711.2937844-1-n-yadav@ti.com> References: <20230913114711.2937844-1-n-yadav@ti.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Content-Type: text/plain Precedence: bulk
Series	Add support for GPMC NAND \| [0/3] Add support for GPMC NAND [1/3] arm64: dts: ti: Add GPMC NAND support [2/3] arm64: dts: ti: Add GPMC NAND overlay [3/3] arm64: defconfig: Enable GPMC NAND support

Commit Message

Nitin Yadav Sept. 13, 2023, 11:47 a.m. UTC

  Add support for AM62Q NAND card: X8 NAND EXPANSION
BOARD card (PROC143E1) for AM62x LP SK board.

Signed-off-by: Nitin Yadav <n-yadav@ti.com>
---
 arch/arm64/boot/dts/ti/k3-am62-main.dtsi | 29 ++++++++++++++++++++++++
 arch/arm64/boot/dts/ti/k3-am62.dtsi      |  2 ++
 2 files changed, 31 insertions(+)

Comments

Krzysztof Kozlowski Sept. 14, 2023, 6:27 a.m. UTC | #1

On 13/09/2023 13:47, Nitin Yadav wrote:
> Add support for AM62Q NAND card: X8 NAND EXPANSION
> BOARD card (PROC143E1) for AM62x LP SK board.
> 
> Signed-off-by: Nitin Yadav <n-yadav@ti.com>
> ---
>  arch/arm64/boot/dts/ti/k3-am62-main.dtsi | 29 ++++++++++++++++++++++++
>  arch/arm64/boot/dts/ti/k3-am62.dtsi      |  2 ++
>  2 files changed, 31 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/ti/k3-am62-main.dtsi b/arch/arm64/boot/dts/ti/k3-am62-main.dtsi
> index 284b90c94da8..e93e79d8083f 100644
> --- a/arch/arm64/boot/dts/ti/k3-am62-main.dtsi
> +++ b/arch/arm64/boot/dts/ti/k3-am62-main.dtsi
> @@ -955,4 +955,33 @@ mcasp2: audio-controller@2b20000 {
>  		power-domains = <&k3_pds 192 TI_SCI_PD_EXCLUSIVE>;
>  		status = "disabled";
>  	};
> +	gpmc0: memory-controller@3b000000 {
> +		status = "disabled";

status is never first in DTSI. Really, where did you see such code?

> +		compatible = "ti,am64-gpmc";
> +		power-domains = <&k3_pds 80 TI_SCI_PD_EXCLUSIVE>;

First is compatible, second is reg/reg-names/ranges.


Best regards,
Krzysztof

Nitin Yadav Sept. 14, 2023, 9:26 a.m. UTC | #2

Hi Krzysztof,

On 14/09/23 11:57, Krzysztof Kozlowski wrote:
> On 13/09/2023 13:47, Nitin Yadav wrote:
>> Add support for AM62Q NAND card: X8 NAND EXPANSION
>> BOARD card (PROC143E1) for AM62x LP SK board.
>>
>> Signed-off-by: Nitin Yadav <n-yadav@ti.com>
>> ---
>>  arch/arm64/boot/dts/ti/k3-am62-main.dtsi | 29 ++++++++++++++++++++++++
>>  arch/arm64/boot/dts/ti/k3-am62.dtsi      |  2 ++
>>  2 files changed, 31 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/ti/k3-am62-main.dtsi b/arch/arm64/boot/dts/ti/k3-am62-main.dtsi
>> index 284b90c94da8..e93e79d8083f 100644
>> --- a/arch/arm64/boot/dts/ti/k3-am62-main.dtsi
>> +++ b/arch/arm64/boot/dts/ti/k3-am62-main.dtsi
>> @@ -955,4 +955,33 @@ mcasp2: audio-controller@2b20000 {
>>  		power-domains = <&k3_pds 192 TI_SCI_PD_EXCLUSIVE>;
>>  		status = "disabled";
>>  	};
>> +	gpmc0: memory-controller@3b000000 {
>> +		status = "disabled";
> 
> status is never first in DTSI. Really, where did you see such code?
Thank for pointing out, Will send a revised version.
> 
>> +		compatible = "ti,am64-gpmc";
>> +		power-domains = <&k3_pds 80 TI_SCI_PD_EXCLUSIVE>;
> 
> First is compatible, second is reg/reg-names/ranges.
> 
> 
> Best regards,
> Krzysztof
>

Nishanth Menon Sept. 14, 2023, 4:04 p.m. UTC | #3

On 14:56-20230914, Nitin Yadav wrote:
> Hi Krzysztof,
> 
> On 14/09/23 11:57, Krzysztof Kozlowski wrote:
> > On 13/09/2023 13:47, Nitin Yadav wrote:
> >> Add support for AM62Q NAND card: X8 NAND EXPANSION
> >> BOARD card (PROC143E1) for AM62x LP SK board.

Commit message is all too wrong as well. Sigh.

> >>
> >> Signed-off-by: Nitin Yadav <n-yadav@ti.com>
> >> ---
> >>  arch/arm64/boot/dts/ti/k3-am62-main.dtsi | 29 ++++++++++++++++++++++++
> >>  arch/arm64/boot/dts/ti/k3-am62.dtsi      |  2 ++
> >>  2 files changed, 31 insertions(+)
> >>
> >> diff --git a/arch/arm64/boot/dts/ti/k3-am62-main.dtsi b/arch/arm64/boot/dts/ti/k3-am62-main.dtsi
> >> index 284b90c94da8..e93e79d8083f 100644
> >> --- a/arch/arm64/boot/dts/ti/k3-am62-main.dtsi
> >> +++ b/arch/arm64/boot/dts/ti/k3-am62-main.dtsi
> >> @@ -955,4 +955,33 @@ mcasp2: audio-controller@2b20000 {
> >>  		power-domains = <&k3_pds 192 TI_SCI_PD_EXCLUSIVE>;
> >>  		status = "disabled";
> >>  	};
> >> +	gpmc0: memory-controller@3b000000 {
> >> +		status = "disabled";
> > 
> > status is never first in DTSI. Really, where did you see such code?
> Thank for pointing out, Will send a revised version.

GPMC is not functional without board specific interface configuration
such as pinmux. this approach, in fact is all over the place now and
discussed in the mailing list multiple times now.

What is missing here is the documentation of the constraints as to why
it is set as disabled by default.


> > 
> >> +		compatible = "ti,am64-gpmc";
> >> +		power-domains = <&k3_pds 80 TI_SCI_PD_EXCLUSIVE>;
> > 
> > First is compatible, second is reg/reg-names/ranges.
> > 
> > 
> > Best regards,
> > Krzysztof
> > 
> 
> -- 
> Regards,
> Nitin

Nitin Yadav Sept. 15, 2023, 9:23 a.m. UTC | #4

On 14/09/23 21:34, Nishanth Menon wrote:
> On 14:56-20230914, Nitin Yadav wrote:
>> Hi Krzysztof,
>>
>> On 14/09/23 11:57, Krzysztof Kozlowski wrote:
>>> On 13/09/2023 13:47, Nitin Yadav wrote:
>>>> Add support for AM62Q NAND card: X8 NAND EXPANSION
>>>> BOARD card (PROC143E1) for AM62x LP SK board.
> 
> Commit message is all too wrong as well. Sigh.
> 
>>>>
>>>> Signed-off-by: Nitin Yadav <n-yadav@ti.com>
>>>> ---
>>>>  arch/arm64/boot/dts/ti/k3-am62-main.dtsi | 29 ++++++++++++++++++++++++
>>>>  arch/arm64/boot/dts/ti/k3-am62.dtsi      |  2 ++
>>>>  2 files changed, 31 insertions(+)
>>>>
>>>> diff --git a/arch/arm64/boot/dts/ti/k3-am62-main.dtsi b/arch/arm64/boot/dts/ti/k3-am62-main.dtsi
>>>> index 284b90c94da8..e93e79d8083f 100644
>>>> --- a/arch/arm64/boot/dts/ti/k3-am62-main.dtsi
>>>> +++ b/arch/arm64/boot/dts/ti/k3-am62-main.dtsi
>>>> @@ -955,4 +955,33 @@ mcasp2: audio-controller@2b20000 {
>>>>  		power-domains = <&k3_pds 192 TI_SCI_PD_EXCLUSIVE>;
>>>>  		status = "disabled";
>>>>  	};
>>>> +	gpmc0: memory-controller@3b000000 {
>>>> +		status = "disabled";
>>>
>>> status is never first in DTSI. Really, where did you see such code?
>> Thank for pointing out, Will send a revised version.
> 
> GPMC is not functional without board specific interface configuration
> such as pinmux. this approach, in fact is all over the place now and
> discussed in the mailing list multiple times now.
> 
> What is missing here is the documentation of the constraints as to why
> it is set as disabled by default.
gpmc nand is only am62x lp sk in am62x series. it has pinmux conflict
with macsp1, so disabling gpmc & elm by default for other am62 series.
For am62x lpsk in overlay macsp1 is disabled.
> 
> 
>>>
>>>> +		compatible = "ti,am64-gpmc";
>>>> +		power-domains = <&k3_pds 80 TI_SCI_PD_EXCLUSIVE>;
>>>
>>> First is compatible, second is reg/reg-names/ranges.
>>>
>>>
>>> Best regards,
>>> Krzysztof
>>>
>>
>> -- 
>> Regards,
>> Nitin
>

Roger Quadros Sept. 15, 2023, 11:16 a.m. UTC | #5

Hi Nitin,

On 13.9.2023 14.47, Nitin Yadav wrote:
> Add support for AM62Q NAND card: X8 NAND EXPANSION
> BOARD card (PROC143E1) for AM62x LP SK board.

This patch is not adding NAND support but GPMC and ELM nodes.

> 
> Signed-off-by: Nitin Yadav <n-yadav@ti.com>
> ---
>   arch/arm64/boot/dts/ti/k3-am62-main.dtsi | 29 ++++++++++++++++++++++++
>   arch/arm64/boot/dts/ti/k3-am62.dtsi      |  2 ++
>   2 files changed, 31 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/ti/k3-am62-main.dtsi b/arch/arm64/boot/dts/ti/k3-am62-main.dtsi
> index 284b90c94da8..e93e79d8083f 100644
> --- a/arch/arm64/boot/dts/ti/k3-am62-main.dtsi
> +++ b/arch/arm64/boot/dts/ti/k3-am62-main.dtsi
> @@ -955,4 +955,33 @@ mcasp2: audio-controller@2b20000 {
>   		power-domains = <&k3_pds 192 TI_SCI_PD_EXCLUSIVE>;
>   		status = "disabled";
>   	};
> +	gpmc0: memory-controller@3b000000 {
> +		status = "disabled";
> +		compatible = "ti,am64-gpmc";
> +		power-domains = <&k3_pds 80 TI_SCI_PD_EXCLUSIVE>;
> +		clocks = <&k3_clks 80 0>;
> +		clock-names = "fck";
> +		reg = <0x00 0x03b000000 0x00 0x400>,
> +		      <0x00 0x050000000 0x00 0x8000000>;
> +		reg-names = "cfg", "data";
> +		interrupts = <GIC_SPI 106 IRQ_TYPE_LEVEL_HIGH>;
> +		gpmc,num-cs = <3>;
> +		gpmc,num-waitpins = <2>;
> +		#address-cells = <2>;
> +		#size-cells = <1>;
> +		interrupt-controller;
> +		#interrupt-cells = <2>;
> +		gpio-controller;
> +		#gpio-cells = <2>;
> +	};
> +
> +	elm0: ecc@25010000 {
> +		status = "disabled";
> +		compatible = "ti,am3352-elm";
> +		reg = <0x00 0x25010000 0x00 0x2000>;
> +		interrupts = <GIC_SPI 132 IRQ_TYPE_LEVEL_HIGH>;
> +		power-domains = <&k3_pds 54 TI_SCI_PD_EXCLUSIVE>;
> +		clocks = <&k3_clks 54 0>;
> +		clock-names = "fck";
> +	};
>   };
> diff --git a/arch/arm64/boot/dts/ti/k3-am62.dtsi b/arch/arm64/boot/dts/ti/k3-am62.dtsi
> index 11f14eef2d44..f7d8aad0a016 100644
> --- a/arch/arm64/boot/dts/ti/k3-am62.dtsi
> +++ b/arch/arm64/boot/dts/ti/k3-am62.dtsi
> @@ -76,6 +76,8 @@ cbass_main: bus@f0000 {
>   			 <0x00 0x70000000 0x00 0x70000000 0x00 0x00010000>, /* OCSRAM */
>   			 <0x01 0x00000000 0x01 0x00000000 0x00 0x00310000>, /* A53 PERIPHBASE */
>   			 <0x05 0x00000000 0x05 0x00000000 0x01 0x00000000>, /* FSS0 DAT3 */
> +			 <0x00 0x3b000000 0x00 0x3b000000 0x00 0x00000400>, /* GPMC0_CFG */
> +			 <0x00 0x50000000 0x00 0x50000000 0x00 0x08000000>, /* GPMC0 DATA */
>   
>   			 /* MCU Domain Range */
>   			 <0x00 0x04000000 0x00 0x04000000 0x00 0x01ff1400>,

Nishanth Menon Sept. 15, 2023, 12:22 p.m. UTC | #6

On 14:53-20230915, Nitin Yadav wrote:
> 
> 
> On 14/09/23 21:34, Nishanth Menon wrote:
> > On 14:56-20230914, Nitin Yadav wrote:
> >> Hi Krzysztof,
> >>
> >> On 14/09/23 11:57, Krzysztof Kozlowski wrote:
> >>> On 13/09/2023 13:47, Nitin Yadav wrote:
> >>>> Add support for AM62Q NAND card: X8 NAND EXPANSION
> >>>> BOARD card (PROC143E1) for AM62x LP SK board.
> > 
> > Commit message is all too wrong as well. Sigh.
> > 
> >>>>
> >>>> Signed-off-by: Nitin Yadav <n-yadav@ti.com>
> >>>> ---
> >>>>  arch/arm64/boot/dts/ti/k3-am62-main.dtsi | 29 ++++++++++++++++++++++++
> >>>>  arch/arm64/boot/dts/ti/k3-am62.dtsi      |  2 ++
> >>>>  2 files changed, 31 insertions(+)
> >>>>
> >>>> diff --git a/arch/arm64/boot/dts/ti/k3-am62-main.dtsi b/arch/arm64/boot/dts/ti/k3-am62-main.dtsi
> >>>> index 284b90c94da8..e93e79d8083f 100644
> >>>> --- a/arch/arm64/boot/dts/ti/k3-am62-main.dtsi
> >>>> +++ b/arch/arm64/boot/dts/ti/k3-am62-main.dtsi
> >>>> @@ -955,4 +955,33 @@ mcasp2: audio-controller@2b20000 {
> >>>>  		power-domains = <&k3_pds 192 TI_SCI_PD_EXCLUSIVE>;
> >>>>  		status = "disabled";
> >>>>  	};
> >>>> +	gpmc0: memory-controller@3b000000 {
> >>>> +		status = "disabled";
> >>>
> >>> status is never first in DTSI. Really, where did you see such code?
> >> Thank for pointing out, Will send a revised version.
> > 
> > GPMC is not functional without board specific interface configuration
> > such as pinmux. this approach, in fact is all over the place now and
> > discussed in the mailing list multiple times now.
> > 
> > What is missing here is the documentation of the constraints as to why
> > it is set as disabled by default.

> gpmc nand is only am62x lp sk in am62x series. it has pinmux conflict
> with macsp1, so disabling gpmc & elm by default for other am62 series.
> For am62x lpsk in overlay macsp1 is disabled.


When introducing a patch for SoC dtsi - explain in commit message and
code comments from the SoC's perspective, not the specific board
perspective.

diff mbox series

Patch

diff --git a/arch/arm64/boot/dts/ti/k3-am62-main.dtsi b/arch/arm64/boot/dts/ti/k3-am62-main.dtsi
index 284b90c94da8..e93e79d8083f 100644
--- a/arch/arm64/boot/dts/ti/k3-am62-main.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-am62-main.dtsi
@@ -955,4 +955,33 @@  mcasp2: audio-controller@2b20000 {
 		power-domains = <&k3_pds 192 TI_SCI_PD_EXCLUSIVE>;
 		status = "disabled";
 	};
+	gpmc0: memory-controller@3b000000 {
+		status = "disabled";
+		compatible = "ti,am64-gpmc";
+		power-domains = <&k3_pds 80 TI_SCI_PD_EXCLUSIVE>;
+		clocks = <&k3_clks 80 0>;
+		clock-names = "fck";
+		reg = <0x00 0x03b000000 0x00 0x400>,
+		      <0x00 0x050000000 0x00 0x8000000>;
+		reg-names = "cfg", "data";
+		interrupts = <GIC_SPI 106 IRQ_TYPE_LEVEL_HIGH>;
+		gpmc,num-cs = <3>;
+		gpmc,num-waitpins = <2>;
+		#address-cells = <2>;
+		#size-cells = <1>;
+		interrupt-controller;
+		#interrupt-cells = <2>;
+		gpio-controller;
+		#gpio-cells = <2>;
+	};
+
+	elm0: ecc@25010000 {
+		status = "disabled";
+		compatible = "ti,am3352-elm";
+		reg = <0x00 0x25010000 0x00 0x2000>;
+		interrupts = <GIC_SPI 132 IRQ_TYPE_LEVEL_HIGH>;
+		power-domains = <&k3_pds 54 TI_SCI_PD_EXCLUSIVE>;
+		clocks = <&k3_clks 54 0>;
+		clock-names = "fck";
+	};
 };
diff --git a/arch/arm64/boot/dts/ti/k3-am62.dtsi b/arch/arm64/boot/dts/ti/k3-am62.dtsi
index 11f14eef2d44..f7d8aad0a016 100644
--- a/arch/arm64/boot/dts/ti/k3-am62.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-am62.dtsi
@@ -76,6 +76,8 @@  cbass_main: bus@f0000 {
 			 <0x00 0x70000000 0x00 0x70000000 0x00 0x00010000>, /* OCSRAM */
 			 <0x01 0x00000000 0x01 0x00000000 0x00 0x00310000>, /* A53 PERIPHBASE */
 			 <0x05 0x00000000 0x05 0x00000000 0x01 0x00000000>, /* FSS0 DAT3 */
+			 <0x00 0x3b000000 0x00 0x3b000000 0x00 0x00000400>, /* GPMC0_CFG */
+			 <0x00 0x50000000 0x00 0x50000000 0x00 0x08000000>, /* GPMC0 DATA */
 
 			 /* MCU Domain Range */
 			 <0x00 0x04000000 0x00 0x04000000 0x00 0x01ff1400>,