| Message ID | 20211123102607.13002-2-rogerq@kernel.org |
|---|---|
| State | New |
| Headers | show |
| Series | memory: omap-gpmc: Add AM64 SoC support | expand |
On 23/11/2021 11:26, Roger Quadros wrote: > AM64 SoC contains the GPMC module. Add compatible for it. > > Newer SoCs don't necessarily map GPMC data region at the same place > as legacy SoCs. Add reg-names "data", to provide this information to > the device driver. > > Cc: Rob Herring <robh+dt@kernel.org> > Signed-off-by: Roger Quadros <rogerq@kernel.org> > --- > .../bindings/memory-controllers/ti,gpmc.yaml | 12 +++++++++++- > 1 file changed, 11 insertions(+), 1 deletion(-) > > diff --git a/Documentation/devicetree/bindings/memory-controllers/ti,gpmc.yaml b/Documentation/devicetree/bindings/memory-controllers/ti,gpmc.yaml > index 25b42d68f9b3..1869cc6f949b 100644 > --- a/Documentation/devicetree/bindings/memory-controllers/ti,gpmc.yaml > +++ b/Documentation/devicetree/bindings/memory-controllers/ti,gpmc.yaml > @@ -23,13 +23,20 @@ properties: > items: > - enum: > - ti,am3352-gpmc > + - ti,am64-gpmc > - ti,omap2420-gpmc > - ti,omap2430-gpmc > - ti,omap3430-gpmc > - ti,omap4430-gpmc > > reg: > - maxItems: 1 > + minItems: 1 > + maxItems: 2 > + > + reg-names: > + items: > + - const: cfg > + - const: data I see your driver handles cases with only one reg item, but I have other question - is it correct to have older (ARMv7) platform with two reg items? Or can am64-gpmc come with only one reg? IOW, I am surprised there is no if-else case precising this minItems requirement for different SocS. > > interrupts: > maxItems: 1 > @@ -44,6 +51,9 @@ properties: > items: > - const: fck > > + power-domains: > + maxItems: 1 Similar, but looks like a weaker requirement - could an older SoC define power-domain? > + > dmas: > items: > - description: DMA channel for GPMC NAND prefetch > Best regards, Krzysztof
On 23/11/2021 21:47, Krzysztof Kozlowski wrote: > On 23/11/2021 11:26, Roger Quadros wrote: >> AM64 SoC contains the GPMC module. Add compatible for it. >> >> Newer SoCs don't necessarily map GPMC data region at the same place >> as legacy SoCs. Add reg-names "data", to provide this information to >> the device driver. >> >> Cc: Rob Herring <robh+dt@kernel.org> >> Signed-off-by: Roger Quadros <rogerq@kernel.org> >> --- >> .../bindings/memory-controllers/ti,gpmc.yaml | 12 +++++++++++- >> 1 file changed, 11 insertions(+), 1 deletion(-) >> >> diff --git a/Documentation/devicetree/bindings/memory-controllers/ti,gpmc.yaml b/Documentation/devicetree/bindings/memory-controllers/ti,gpmc.yaml >> index 25b42d68f9b3..1869cc6f949b 100644 >> --- a/Documentation/devicetree/bindings/memory-controllers/ti,gpmc.yaml >> +++ b/Documentation/devicetree/bindings/memory-controllers/ti,gpmc.yaml >> @@ -23,13 +23,20 @@ properties: >> items: >> - enum: >> - ti,am3352-gpmc >> + - ti,am64-gpmc >> - ti,omap2420-gpmc >> - ti,omap2430-gpmc >> - ti,omap3430-gpmc >> - ti,omap4430-gpmc >> >> reg: >> - maxItems: 1 >> + minItems: 1 >> + maxItems: 2 >> + >> + reg-names: >> + items: >> + - const: cfg >> + - const: data > > I see your driver handles cases with only one reg item, but I have other The support for these two items is added in patch 3 of this series "memory: omap-gpmc: Add support for GPMC on AM64 SoC" > question - is it correct to have older (ARMv7) platform with two reg > items? Or can am64-gpmc come with only one reg? Older platforms currently have only one reg, but they can be updated to come with two without breaking functionality. am64-gpmc cannot come with one reg as the defaults for data window are not suitable for AM64. All legacy platforms were using a fixed Data IO window (first 1 GB) but from AM64 this was moved elsewhere, so the need for this change. > IOW, I am surprised there is no if-else case precising this minItems > requirement for different SocS. > OK. I will add this. >> >> interrupts: >> maxItems: 1 >> @@ -44,6 +51,9 @@ properties: >> items: >> - const: fck >> >> + power-domains: >> + maxItems: 1 > > Similar, but looks like a weaker requirement - could an older SoC define > power-domain? No. Will add SoC specific constraint for this as well. > >> + >> dmas: >> items: >> - description: DMA channel for GPMC NAND prefetch >> > > > Best regards, > Krzysztof > cheers, -roger
On Tue, Nov 23, 2021 at 08:47:57PM +0100, Krzysztof Kozlowski wrote: > On 23/11/2021 11:26, Roger Quadros wrote: > > AM64 SoC contains the GPMC module. Add compatible for it. > > > > Newer SoCs don't necessarily map GPMC data region at the same place > > as legacy SoCs. Add reg-names "data", to provide this information to > > the device driver. > > > > Cc: Rob Herring <robh+dt@kernel.org> > > Signed-off-by: Roger Quadros <rogerq@kernel.org> > > --- > > .../bindings/memory-controllers/ti,gpmc.yaml | 12 +++++++++++- > > 1 file changed, 11 insertions(+), 1 deletion(-) > > > > diff --git a/Documentation/devicetree/bindings/memory-controllers/ti,gpmc.yaml b/Documentation/devicetree/bindings/memory-controllers/ti,gpmc.yaml > > index 25b42d68f9b3..1869cc6f949b 100644 > > --- a/Documentation/devicetree/bindings/memory-controllers/ti,gpmc.yaml > > +++ b/Documentation/devicetree/bindings/memory-controllers/ti,gpmc.yaml > > @@ -23,13 +23,20 @@ properties: > > items: > > - enum: > > - ti,am3352-gpmc > > + - ti,am64-gpmc > > - ti,omap2420-gpmc > > - ti,omap2430-gpmc > > - ti,omap3430-gpmc > > - ti,omap4430-gpmc > > > > reg: > > - maxItems: 1 > > + minItems: 1 > > + maxItems: 2 > > + > > + reg-names: > > + items: > > + - const: cfg > > + - const: data > > I see your driver handles cases with only one reg item, but I have other > question - is it correct to have older (ARMv7) platform with two reg > items? Or can am64-gpmc come with only one reg? > IOW, I am surprised there is no if-else case precising this minItems > requirement for different SocS. I don't think that is needed here. If the assumption is 'reg-names' is only present when there are 2 entries, then it is fine. Maybe 'reg-names' should be required for ti,am64-gpmc though. Rob
On 01/12/2021 00:02, Rob Herring wrote: > On Tue, Nov 23, 2021 at 08:47:57PM +0100, Krzysztof Kozlowski wrote: >> On 23/11/2021 11:26, Roger Quadros wrote: >>> AM64 SoC contains the GPMC module. Add compatible for it. >>> >>> Newer SoCs don't necessarily map GPMC data region at the same place >>> as legacy SoCs. Add reg-names "data", to provide this information to >>> the device driver. >>> >>> Cc: Rob Herring <robh+dt@kernel.org> >>> Signed-off-by: Roger Quadros <rogerq@kernel.org> >>> --- >>> .../bindings/memory-controllers/ti,gpmc.yaml | 12 +++++++++++- >>> 1 file changed, 11 insertions(+), 1 deletion(-) >>> >>> diff --git a/Documentation/devicetree/bindings/memory-controllers/ti,gpmc.yaml b/Documentation/devicetree/bindings/memory-controllers/ti,gpmc.yaml >>> index 25b42d68f9b3..1869cc6f949b 100644 >>> --- a/Documentation/devicetree/bindings/memory-controllers/ti,gpmc.yaml >>> +++ b/Documentation/devicetree/bindings/memory-controllers/ti,gpmc.yaml >>> @@ -23,13 +23,20 @@ properties: >>> items: >>> - enum: >>> - ti,am3352-gpmc >>> + - ti,am64-gpmc >>> - ti,omap2420-gpmc >>> - ti,omap2430-gpmc >>> - ti,omap3430-gpmc >>> - ti,omap4430-gpmc >>> >>> reg: >>> - maxItems: 1 >>> + minItems: 1 >>> + maxItems: 2 >>> + >>> + reg-names: >>> + items: >>> + - const: cfg >>> + - const: data >> >> I see your driver handles cases with only one reg item, but I have other >> question - is it correct to have older (ARMv7) platform with two reg >> items? Or can am64-gpmc come with only one reg? >> IOW, I am surprised there is no if-else case precising this minItems >> requirement for different SocS. > > I don't think that is needed here. If the assumption is 'reg-names' is > only present when there are 2 entries, then it is fine. Maybe > 'reg-names' should be required for ti,am64-gpmc though. Yes, I'll make 'reg-names' property required for ti,am64-gpmc. cheers, -roger
diff --git a/Documentation/devicetree/bindings/memory-controllers/ti,gpmc.yaml b/Documentation/devicetree/bindings/memory-controllers/ti,gpmc.yaml index 25b42d68f9b3..1869cc6f949b 100644 --- a/Documentation/devicetree/bindings/memory-controllers/ti,gpmc.yaml +++ b/Documentation/devicetree/bindings/memory-controllers/ti,gpmc.yaml @@ -23,13 +23,20 @@ properties: items: - enum: - ti,am3352-gpmc + - ti,am64-gpmc - ti,omap2420-gpmc - ti,omap2430-gpmc - ti,omap3430-gpmc - ti,omap4430-gpmc reg: - maxItems: 1 + minItems: 1 + maxItems: 2 + + reg-names: + items: + - const: cfg + - const: data interrupts: maxItems: 1 @@ -44,6 +51,9 @@ properties: items: - const: fck + power-domains: + maxItems: 1 + dmas: items: - description: DMA channel for GPMC NAND prefetch
AM64 SoC contains the GPMC module. Add compatible for it. Newer SoCs don't necessarily map GPMC data region at the same place as legacy SoCs. Add reg-names "data", to provide this information to the device driver. Cc: Rob Herring <robh+dt@kernel.org> Signed-off-by: Roger Quadros <rogerq@kernel.org> --- .../bindings/memory-controllers/ti,gpmc.yaml | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-)