| Message ID | 20230913114711.2937844-2-n-yadav@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add support for GPMC NAND | expand |
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
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 >
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
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 >
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>,
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 --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>,
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(+)