Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1705044 > unrolled thread

[RESEND PATCH 0/4] EDAC: support reduce bus width on 98dx3236

Back to article view | Back to linux.kernel

Contents

  [RESEND PATCH 0/4] EDAC: support reduce bus width on 98dx3236 Chris Packham <chris.packham@alliedtelesis.co.nz> - 2017-08-07 03:50 +0200
    [RESEND PATCH 2/4] dt-bindings: add "reduced-width" property for Armada XP SDRAM controller Chris Packham <chris.packham@alliedtelesis.co.nz> - 2017-08-07 03:50 +0200
      Re: [RESEND PATCH 2/4] dt-bindings: add "reduced-width" property for  Armada XP SDRAM controller Rob Herring <robh@kernel.org> - 2017-08-10 22:40 +0200
        Re: [RESEND PATCH 2/4] dt-bindings: add "reduced-width" property for  Armada XP SDRAM controller Chris Packham <Chris.Packham@alliedtelesis.co.nz> - 2017-08-10 23:20 +0200
          Re: [RESEND PATCH 2/4] dt-bindings: add "reduced-width" property  for Armada XP SDRAM controller Jan Lübbe <jlu@pengutronix.de> - 2017-08-11 11:40 +0200

#1705044 — [RESEND PATCH 0/4] EDAC: support reduce bus width on 98dx3236

(sorry I messed up sending this earlier, there is one additional patch and I'll
actually include the linux-arm and linux-edac mailing lists)

This series applies on top of Jan Lubbe's "EDAC drivers for Armada XP L2 and
DDR" series[1]. 1/4, 2/4 and 3/4 don't strictly depend on Jan's work so they
could go in through the ARM tree if that is preferred.

The 98dx3236 and similar switch chips with integrated CPUs have fewer pins
available for the SDRAM interface so the definition of "full" and "half" is
different to the Armada-XP SoC. In this series I introduce a
"marvell,reduced-width" device tree property and use this to identify such a
system.

I chose to use a new property instead of a new compatible string because the IP
block really is the Armada-XP one (at least according to the Marvell FAE I
spoke to) and because the scenario of requiring a reduced pin-count when going
from an external SoC to an integrated one will be reasonably common as we see
more an more of these switches with integrated ARM cores.


[1] - http://marc.info/?l=linux-edac&m=150167758312924

Chris Packham (4):
  ARM: dts: enable L2 cache parity and ecc on db-xc3-24g4xg board
  dt-bindings: add "reduced-width" property for Armada XP SDRAM
    controller
  ARM: dts: mvebu: set reduced-width property for SDRAM on 98dx3236
  EDAC: add support for reduced-width Armada-XP SDRAM

 .../bindings/memory-controllers/mvebu-sdram-controller.txt          | 6 ++++++
 arch/arm/boot/dts/armada-xp-98dx3236.dtsi                           | 1 +
 arch/arm/boot/dts/armada-xp-db-xc3-24g4xg.dts                       | 5 +++++
 drivers/edac/armada_xp_edac.c                                       | 3 +++
 4 files changed, 15 insertions(+)

-- 
2.13.0

[toc] | [next] | [standalone]

#1705045 — [RESEND PATCH 2/4] dt-bindings: add "reduced-width" property for Armada XP SDRAM controller

Some SoC implementations that use this controller have a reduced pin
count so the meaning of "full" and "half" with change.

Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
---
 .../bindings/memory-controllers/mvebu-sdram-controller.txt          | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/memory-controllers/mvebu-sdram-controller.txt b/Documentation/devicetree/bindings/memory-controllers/mvebu-sdram-controller.txt
index 89657d1d4cd4..3041868321c8 100644
--- a/Documentation/devicetree/bindings/memory-controllers/mvebu-sdram-controller.txt
+++ b/Documentation/devicetree/bindings/memory-controllers/mvebu-sdram-controller.txt
@@ -13,6 +13,12 @@ Required properties:
  - reg: a resource specifier for the register space, which should
    include all SDRAM controller registers as per the datasheet.
 
+Optional properties:
+ - marvell,reduced-width: some SoCs that use this SDRAM controller have
+   a reduced pin count. On such systems "full" width is 32-bits and
+   "half" width is 16-bits. Set this property to indicate that the SoC
+   used is such a system.
+
 Example:
 
 sdramc@1400 {
-- 
2.13.0

[toc] | [prev] | [next] | [standalone]

#1709008 — Re: [RESEND PATCH 2/4] dt-bindings: add "reduced-width" property for Armada XP SDRAM controller

On Mon, Aug 07, 2017 at 01:46:39PM +1200, Chris Packham wrote:
> Some SoC implementations that use this controller have a reduced pin
> count so the meaning of "full" and "half" with change.

s/with/width/ ?

> 
> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
> ---
>  .../bindings/memory-controllers/mvebu-sdram-controller.txt          | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/memory-controllers/mvebu-sdram-controller.txt b/Documentation/devicetree/bindings/memory-controllers/mvebu-sdram-controller.txt
> index 89657d1d4cd4..3041868321c8 100644
> --- a/Documentation/devicetree/bindings/memory-controllers/mvebu-sdram-controller.txt
> +++ b/Documentation/devicetree/bindings/memory-controllers/mvebu-sdram-controller.txt
> @@ -13,6 +13,12 @@ Required properties:
>   - reg: a resource specifier for the register space, which should
>     include all SDRAM controller registers as per the datasheet.
>  
> +Optional properties:
> + - marvell,reduced-width: some SoCs that use this SDRAM controller have
> +   a reduced pin count. On such systems "full" width is 32-bits and
> +   "half" width is 16-bits. Set this property to indicate that the SoC
> +   used is such a system.

Maybe you should just state what the width is.

Or your compatible string should just be specific enough to know the 
width.

Rob

[toc] | [prev] | [next] | [standalone]

#1709028 — Re: [RESEND PATCH 2/4] dt-bindings: add "reduced-width" property for Armada XP SDRAM controller

On 11/08/17 08:38, Rob Herring wrote:
> On Mon, Aug 07, 2017 at 01:46:39PM +1200, Chris Packham wrote:
>> Some SoC implementations that use this controller have a reduced pin
>> count so the meaning of "full" and "half" with change.
> 
> s/with/width/ ?
> 

Yes will include in v2.

>>
>> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
>> ---
>>   .../bindings/memory-controllers/mvebu-sdram-controller.txt          | 6 ++++++
>>   1 file changed, 6 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/memory-controllers/mvebu-sdram-controller.txt b/Documentation/devicetree/bindings/memory-controllers/mvebu-sdram-controller.txt
>> index 89657d1d4cd4..3041868321c8 100644
>> --- a/Documentation/devicetree/bindings/memory-controllers/mvebu-sdram-controller.txt
>> +++ b/Documentation/devicetree/bindings/memory-controllers/mvebu-sdram-controller.txt
>> @@ -13,6 +13,12 @@ Required properties:
>>    - reg: a resource specifier for the register space, which should
>>      include all SDRAM controller registers as per the datasheet.
>>   
>> +Optional properties:
>> + - marvell,reduced-width: some SoCs that use this SDRAM controller have
>> +   a reduced pin count. On such systems "full" width is 32-bits and
>> +   "half" width is 16-bits. Set this property to indicate that the SoC
>> +   used is such a system.
> 
> Maybe you should just state what the width is.


Specifying a number like 64/32/16 is done in for some other properties I 
dismissed that because what this is about how we interpret a 
pin-strapping option. I guess "max-width = <64>;" and "max-width = 
<32>"; would achieve the same.


> Or your compatible string should just be specific enough to know the
> width.

I decided against a new compatible sting that because the IP block 
really is the Armada-XP one and the existing compatible string is used 
in other places (using multiple compatible strings would solve that).

I'm not too fussed which of the 3 options are used. Is there any 
particular preference?

[toc] | [prev] | [next] | [standalone]

#1709411 — Re: [RESEND PATCH 2/4] dt-bindings: add "reduced-width" property for Armada XP SDRAM controller

On Thu, 2017-08-10 at 21:17 +0000, Chris Packham wrote:
> On 11/08/17 08:38, Rob Herring wrote:
> > On Mon, Aug 07, 2017 at 01:46:39PM +1200, Chris Packham wrote:
[...]  
> > > +Optional properties:
> > > + - marvell,reduced-width: some SoCs that use this SDRAM controller have
> > > +   a reduced pin count. On such systems "full" width is 32-bits and
> > > +   "half" width is 16-bits. Set this property to indicate that the SoC
> > > +   used is such a system.
> > 
> > Maybe you should just state what the width is.
> 
> Specifying a number like 64/32/16 is done in for some other properties I 
> dismissed that because what this is about how we interpret a 
> pin-strapping option. I guess "max-width = <64>;" and "max-width = 
> <32>"; would achieve the same.
> 
> > Or your compatible string should just be specific enough to know the
> > width.
> 
> I decided against a new compatible sting that because the IP block 
> really is the Armada-XP one and the existing compatible string is used 
> in other places (using multiple compatible strings would solve that).
> 
> I'm not too fussed which of the 3 options are used. Is there any 
> particular preference?

I'd prefer a specific compatible string, as it would avoid adding even
more properties if further difference turn up.

Rob, I seem to remember that some drivers match the top-level
compatible against a list of SoC variants to detect SoC-dependent
features in a generic IP block. Is that something you'd prefer instead?

Regards,
Jan

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web