2023-09-13 13:30:12

by Nitin Yadav

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

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

Signed-off-by: Nitin Yadav <[email protected]>
---
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>,
--
2.25.1

2023-09-14 08:25:48

by Krzysztof Kozlowski

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

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 <[email protected]>
> ---
> 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

2023-09-14 10:59:30

by Nitin Yadav

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

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 <[email protected]>
>> ---
>> 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
>

--
Regards,
Nitin

2023-09-14 22:01:02

by Nishanth Menon

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

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 <[email protected]>
> >> ---
> >> 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

--
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3 1A34 DDB5 849D 1736 249D

2023-09-15 14:06:44

by Nitin Yadav

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

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 <[email protected]>
>>>> ---
>>>> 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
>

--
Regards,
Nitin

2023-09-15 15:25:44

by Nishanth Menon

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

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 <[email protected]>
> >>>> ---
> >>>> 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.

--
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3 1A34 DDB5 849D 1736 249D

2023-09-15 16:00:25

by Roger Quadros

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

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 <[email protected]>
> ---
> 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>,

--
cheers,
-roger