| Message ID | 20220510231002.1160798-2-chris.packham@alliedtelesis.co.nz (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | arm64: mvebu: Support for Marvell 98DX2530 (and variants) | expand |
On 11/05/2022 01:10, Chris Packham wrote: > Describe the compatible properties for the Marvell Alleycat5/5X switches > with integrated CPUs. > > Alleycat5: > * 98DX2538: 24x1G + 2x10G + 2x10G Stack > * 98DX2535: 24x1G + 4x1G Stack > * 98DX2532: 8x1G + 2x10G + 2x1G Stack > * 98DX2531: 8x1G + 4x1G Stack > * 98DX2528: 24x1G + 2x10G + 2x10G Stack > * 98DX2525: 24x1G + 4x1G Stack > * 98DX2522: 8x1G + 2x10G + 2x1G Stack > * 98DX2521: 8x1G + 4x1G Stack > * 98DX2518: 24x1G + 2x10G + 2x10G Stack > * 98DX2515: 24x1G + 4x1G Stack > * 98DX2512: 8x1G + 2x10G + 2x1G Stack > * 98DX2511: 8x1G + 4x1G Stack > > Alleycat5X: > * 98DX3500: 24x1G + 6x25G > * 98DX3501: 16x1G + 6x10G > * 98DX3510: 48x1G + 6x25G > * 98DX3520: 24x2.5G + 6x25G > * 98DX3530: 48x2.5G + 6x25G > * 98DX3540: 12x5G/6x10G + 6x25G > * 98DX3550: 24x5G/12x10G + 6x25G > > Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz> > --- > > Notes: > Changes in v6: > - New > > .../bindings/arm/marvell/armada-98dx2530.yaml | 27 +++++++++++++++++++ > 1 file changed, 27 insertions(+) > create mode 100644 Documentation/devicetree/bindings/arm/marvell/armada-98dx2530.yaml > > diff --git a/Documentation/devicetree/bindings/arm/marvell/armada-98dx2530.yaml b/Documentation/devicetree/bindings/arm/marvell/armada-98dx2530.yaml > new file mode 100644 > index 000000000000..6d9185baf0c5 > --- /dev/null > +++ b/Documentation/devicetree/bindings/arm/marvell/armada-98dx2530.yaml > @@ -0,0 +1,27 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/arm/marvell/armada-98dx2530.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Marvell Alleycat5/5X Platforms > + > +maintainers: > + - Chris Packham <chris.packham@alliedtelesis.co.nz> > + > +properties: > + $nodename: > + const: '/' > + compatible: > + oneOf: > + > + - description: Alleycat5 (98DX25xx) > + items: > + - const: marvell,ac5 This is confusing and does not look correct. The DTS calls AC5 a SoC and you cannot have SoC alone. It's unusable without a SoM or board. > + > + - description: Alleycat5X (98DX35xx) > + items: > + - const: marvell,ac5x > + - const: marvell,ac5 This entry looks correct except ac5x once is called a SoC and once a RD-AC5X board. It cannot be both. Probably you need third compatible, assuming AC5x is a flavor of AC5. items: - enum: - marvell,rd-ac5x - const: marvell,ac5x - const: marvell,ac5 > + > +additionalProperties: true Best regards, Krzysztof
On Wed, May 11, 2022 at 11:10:00AM +1200, Chris Packham wrote: > Describe the compatible properties for the Marvell Alleycat5/5X switches > with integrated CPUs. > > Alleycat5: > * 98DX2538: 24x1G + 2x10G + 2x10G Stack > * 98DX2535: 24x1G + 4x1G Stack > * 98DX2532: 8x1G + 2x10G + 2x1G Stack > * 98DX2531: 8x1G + 4x1G Stack > * 98DX2528: 24x1G + 2x10G + 2x10G Stack > * 98DX2525: 24x1G + 4x1G Stack > * 98DX2522: 8x1G + 2x10G + 2x1G Stack > * 98DX2521: 8x1G + 4x1G Stack > * 98DX2518: 24x1G + 2x10G + 2x10G Stack > * 98DX2515: 24x1G + 4x1G Stack > * 98DX2512: 8x1G + 2x10G + 2x1G Stack > * 98DX2511: 8x1G + 4x1G Stack > > Alleycat5X: > * 98DX3500: 24x1G + 6x25G > * 98DX3501: 16x1G + 6x10G > * 98DX3510: 48x1G + 6x25G > * 98DX3520: 24x2.5G + 6x25G > * 98DX3530: 48x2.5G + 6x25G > * 98DX3540: 12x5G/6x10G + 6x25G > * 98DX3550: 24x5G/12x10G + 6x25G Hi Chris When looking at this list, is it just the switch which changes, and everything else in the package stays the same? I'm thinking back to plain Kirkwood. There were 3 Kirkwood SoCs. We had kirkwood.dtsi which described everything common to all three SoCs. And then kirkwood-6192.dtsi, kirkwood-6281.dtsi, kirkwood-6282.dtsi which extended that base with whatever additional things each SoC had. I'm wondering if something similar is needed here? armada-98DX25xx.dtsi which describes everything common to Alleycat5. armada-98DX35xx.dtsi which describes everything common to Alleycat5X, maybe making use of armada-98DX25xx.dtsi?. armada-98DX2538.dtsi which extends armada-98DX25xx.dtsi And then a board file which includes armada-98DX2538.dtsi and add the board specific bits? I've no idea how these different devices differ, so i don't know what the correct hierarchy should be. Andrew
On 12/05/22 05:02, Andrew Lunn wrote: > On Wed, May 11, 2022 at 11:10:00AM +1200, Chris Packham wrote: >> Describe the compatible properties for the Marvell Alleycat5/5X switches >> with integrated CPUs. >> >> Alleycat5: >> * 98DX2538: 24x1G + 2x10G + 2x10G Stack >> * 98DX2535: 24x1G + 4x1G Stack >> * 98DX2532: 8x1G + 2x10G + 2x1G Stack >> * 98DX2531: 8x1G + 4x1G Stack >> * 98DX2528: 24x1G + 2x10G + 2x10G Stack >> * 98DX2525: 24x1G + 4x1G Stack >> * 98DX2522: 8x1G + 2x10G + 2x1G Stack >> * 98DX2521: 8x1G + 4x1G Stack >> * 98DX2518: 24x1G + 2x10G + 2x10G Stack >> * 98DX2515: 24x1G + 4x1G Stack >> * 98DX2512: 8x1G + 2x10G + 2x1G Stack >> * 98DX2511: 8x1G + 4x1G Stack >> >> Alleycat5X: >> * 98DX3500: 24x1G + 6x25G >> * 98DX3501: 16x1G + 6x10G >> * 98DX3510: 48x1G + 6x25G >> * 98DX3520: 24x2.5G + 6x25G >> * 98DX3530: 48x2.5G + 6x25G >> * 98DX3540: 12x5G/6x10G + 6x25G >> * 98DX3550: 24x5G/12x10G + 6x25G > Hi Chris > > When looking at this list, is it just the switch which changes, and > everything else in the package stays the same? CPU wise I've been told everything is identical. The differences are all in the switch side. > I'm thinking back to plain Kirkwood. There were 3 Kirkwood SoCs. We > had kirkwood.dtsi which described everything common to all three > SoCs. And then kirkwood-6192.dtsi, kirkwood-6281.dtsi, > kirkwood-6282.dtsi which extended that base with whatever additional > things each SoC had. > > I'm wondering if something similar is needed here? > > armada-98DX25xx.dtsi which describes everything common to Alleycat5. > > armada-98DX35xx.dtsi which describes everything common to Alleycat5X, > maybe making use of armada-98DX25xx.dtsi?. Right now there would be no difference between 25xx and 35xx but perhaps having armada-98DX35xx.dtsi just #include armada-98DX25xx.dtsi would make the boards able to pull in something that more naturally fits the actual chip that is used. > armada-98DX2538.dtsi which extends armada-98DX25xx.dtsi There wouldn't be anything to add in 98DX2538 (at least not until we have a proper switchdev driver). > And then a board file which includes armada-98DX2538.dtsi and add the > board specific bits? > > I've no idea how these different devices differ, so i don't know what > the correct hierarchy should be. If you put aside the switch stuff they don't differ at all. Which is a bit different to the 98dx3236/98dx3336/98dx4251 support I added a few years ago where there were differences w.r.t number of CPU cores and the odd peripheral. My main goal has been to get the CPU side stuff landed first. In what I've submitted so far I haven't tried to incorporate the switch register space, that's where you might see some difference like 'compatible = "marvell,prestera-98dx2538", "marvell,prestera";'.
On Wed, May 11, 2022 at 11:14:25PM +0000, Chris Packham wrote: > > On 12/05/22 05:02, Andrew Lunn wrote: > > On Wed, May 11, 2022 at 11:10:00AM +1200, Chris Packham wrote: > >> Describe the compatible properties for the Marvell Alleycat5/5X switches > >> with integrated CPUs. > >> > >> Alleycat5: > >> * 98DX2538: 24x1G + 2x10G + 2x10G Stack > >> * 98DX2535: 24x1G + 4x1G Stack > >> * 98DX2532: 8x1G + 2x10G + 2x1G Stack > >> * 98DX2531: 8x1G + 4x1G Stack > >> * 98DX2528: 24x1G + 2x10G + 2x10G Stack > >> * 98DX2525: 24x1G + 4x1G Stack > >> * 98DX2522: 8x1G + 2x10G + 2x1G Stack > >> * 98DX2521: 8x1G + 4x1G Stack > >> * 98DX2518: 24x1G + 2x10G + 2x10G Stack > >> * 98DX2515: 24x1G + 4x1G Stack > >> * 98DX2512: 8x1G + 2x10G + 2x1G Stack > >> * 98DX2511: 8x1G + 4x1G Stack > >> > >> Alleycat5X: > >> * 98DX3500: 24x1G + 6x25G > >> * 98DX3501: 16x1G + 6x10G > >> * 98DX3510: 48x1G + 6x25G > >> * 98DX3520: 24x2.5G + 6x25G > >> * 98DX3530: 48x2.5G + 6x25G > >> * 98DX3540: 12x5G/6x10G + 6x25G > >> * 98DX3550: 24x5G/12x10G + 6x25G > > Hi Chris > > > > When looking at this list, is it just the switch which changes, and > > everything else in the package stays the same? > > CPU wise I've been told everything is identical. The differences are all > in the switch side. O.K. That helps a lot with this description. > > armada-98DX2538.dtsi which extends armada-98DX25xx.dtsi > > There wouldn't be anything to add in 98DX2538 (at least not until we > have a proper switchdev driver). Does the switch/SoC have ID registers? For mv88e6xxx, the switch is identified by its ID registers, so we don't have switch specific compatible value in DT. Hopefully it is the same here. All we need to say is that there is a switch in the main .dtsi file, and the .dts file would then indicate which ports are actually used. Andrew
On 12/05/22 12:27, Andrew Lunn wrote: > On Wed, May 11, 2022 at 11:14:25PM +0000, Chris Packham wrote: >> On 12/05/22 05:02, Andrew Lunn wrote: >>> On Wed, May 11, 2022 at 11:10:00AM +1200, Chris Packham wrote: >>>> Describe the compatible properties for the Marvell Alleycat5/5X switches >>>> with integrated CPUs. >>>> >>>> Alleycat5: >>>> * 98DX2538: 24x1G + 2x10G + 2x10G Stack >>>> * 98DX2535: 24x1G + 4x1G Stack >>>> * 98DX2532: 8x1G + 2x10G + 2x1G Stack >>>> * 98DX2531: 8x1G + 4x1G Stack >>>> * 98DX2528: 24x1G + 2x10G + 2x10G Stack >>>> * 98DX2525: 24x1G + 4x1G Stack >>>> * 98DX2522: 8x1G + 2x10G + 2x1G Stack >>>> * 98DX2521: 8x1G + 4x1G Stack >>>> * 98DX2518: 24x1G + 2x10G + 2x10G Stack >>>> * 98DX2515: 24x1G + 4x1G Stack >>>> * 98DX2512: 8x1G + 2x10G + 2x1G Stack >>>> * 98DX2511: 8x1G + 4x1G Stack >>>> >>>> Alleycat5X: >>>> * 98DX3500: 24x1G + 6x25G >>>> * 98DX3501: 16x1G + 6x10G >>>> * 98DX3510: 48x1G + 6x25G >>>> * 98DX3520: 24x2.5G + 6x25G >>>> * 98DX3530: 48x2.5G + 6x25G >>>> * 98DX3540: 12x5G/6x10G + 6x25G >>>> * 98DX3550: 24x5G/12x10G + 6x25G >>> Hi Chris >>> >>> When looking at this list, is it just the switch which changes, and >>> everything else in the package stays the same? >> CPU wise I've been told everything is identical. The differences are all >> in the switch side. > O.K. That helps a lot with this description. > >>> armada-98DX2538.dtsi which extends armada-98DX25xx.dtsi >> There wouldn't be anything to add in 98DX2538 (at least not until we >> have a proper switchdev driver). > Does the switch/SoC have ID registers? For mv88e6xxx, the switch is > identified by its ID registers, so we don't have switch specific > compatible value in DT. Hopefully it is the same here. All we need to > say is that there is a switch in the main .dtsi file, and the .dts > file would then indicate which ports are actually used. Yes there are registers that you can read to identify the specific chip. It still might be useful to have a expected vs actual check as those ID values are determined by pin strapping resistors. It could also be used to validate the dts (e.g. port 20 would be invalid on a 98DX3501). But those are considerations for further down the track. > Andrew
> Yes there are registers that you can read to identify the specific chip. > > It still might be useful to have a expected vs actual check as those ID > values are determined by pin strapping resistors. That i don't get? Can i turn a * 98DX2538: 24x1G + 2x10G + 2x10G Stack into a * 98DX2535: 24x1G + 4x1G Stack by strapping its pin differently? > It could also be used > to validate the dts (e.g. port 20 would be invalid on a 98DX3501). But > those are considerations for further down the track. Yes, that would be up to the switchdev driver to validate the DT based on the ID register. Andrew
On 12/05/22 12:45, Andrew Lunn wrote: >> Yes there are registers that you can read to identify the specific chip. >> >> It still might be useful to have a expected vs actual check as those ID >> values are determined by pin strapping resistors. > That i don't get? Can i turn a > > * 98DX2538: 24x1G + 2x10G + 2x10G Stack > > into a > > * 98DX2535: 24x1G + 4x1G Stack > > by strapping its pin differently? I'm not sure it'd actually work properly but yes there are external PU/PD resistors that if you fitted differently would at least make the ID say that a 98DX2538 is a 98DX2535. The HW docs have these as "reserved" pins that must be pulled up/down depending on the specific part. In reality I suspect that the different serdes arrangements are based on what level of screening the silicon passed (similar to how some SoC speed grades are distinguished). So you might be able to go down (i.e. 2538 -> 2535) but probably not up (i.e 2535 -> 2538). > >> It could also be used >> to validate the dts (e.g. port 20 would be invalid on a 98DX3501). But >> those are considerations for further down the track. > Yes, that would be up to the switchdev driver to validate the DT based > on the ID register. > > Andrew
On 12/05/22 04:34, Krzysztof Kozlowski wrote: > On 11/05/2022 01:10, Chris Packham wrote: >> Describe the compatible properties for the Marvell Alleycat5/5X switches >> with integrated CPUs. >> >> Alleycat5: >> * 98DX2538: 24x1G + 2x10G + 2x10G Stack >> * 98DX2535: 24x1G + 4x1G Stack >> * 98DX2532: 8x1G + 2x10G + 2x1G Stack >> * 98DX2531: 8x1G + 4x1G Stack >> * 98DX2528: 24x1G + 2x10G + 2x10G Stack >> * 98DX2525: 24x1G + 4x1G Stack >> * 98DX2522: 8x1G + 2x10G + 2x1G Stack >> * 98DX2521: 8x1G + 4x1G Stack >> * 98DX2518: 24x1G + 2x10G + 2x10G Stack >> * 98DX2515: 24x1G + 4x1G Stack >> * 98DX2512: 8x1G + 2x10G + 2x1G Stack >> * 98DX2511: 8x1G + 4x1G Stack >> >> Alleycat5X: >> * 98DX3500: 24x1G + 6x25G >> * 98DX3501: 16x1G + 6x10G >> * 98DX3510: 48x1G + 6x25G >> * 98DX3520: 24x2.5G + 6x25G >> * 98DX3530: 48x2.5G + 6x25G >> * 98DX3540: 12x5G/6x10G + 6x25G >> * 98DX3550: 24x5G/12x10G + 6x25G >> >> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz> >> --- >> >> Notes: >> Changes in v6: >> - New >> >> .../bindings/arm/marvell/armada-98dx2530.yaml | 27 +++++++++++++++++++ >> 1 file changed, 27 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/arm/marvell/armada-98dx2530.yaml >> >> diff --git a/Documentation/devicetree/bindings/arm/marvell/armada-98dx2530.yaml b/Documentation/devicetree/bindings/arm/marvell/armada-98dx2530.yaml >> new file mode 100644 >> index 000000000000..6d9185baf0c5 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/arm/marvell/armada-98dx2530.yaml >> @@ -0,0 +1,27 @@ >> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >> +%YAML 1.2 >> +--- >> +$id: http://scanmail.trustwave.com/?c=20988&d=heX74s-dh8HSCAJmafRigZHOoyY0XQDl80QSCXWitw&u=http%3a%2f%2fdevicetree%2eorg%2fschemas%2farm%2fmarvell%2farmada-98dx2530%2eyaml%23 >> +$schema: http://scanmail.trustwave.com/?c=20988&d=heX74s-dh8HSCAJmafRigZHOoyY0XQDl80oVWnOltA&u=http%3a%2f%2fdevicetree%2eorg%2fmeta-schemas%2fcore%2eyaml%23 >> + >> +title: Marvell Alleycat5/5X Platforms >> + >> +maintainers: >> + - Chris Packham <chris.packham@alliedtelesis.co.nz> >> + >> +properties: >> + $nodename: >> + const: '/' >> + compatible: >> + oneOf: >> + >> + - description: Alleycat5 (98DX25xx) >> + items: >> + - const: marvell,ac5 > This is confusing and does not look correct. The DTS calls AC5 a SoC and > you cannot have SoC alone. It's unusable without a SoM or board. > >> + >> + - description: Alleycat5X (98DX35xx) >> + items: >> + - const: marvell,ac5x >> + - const: marvell,ac5 > This entry looks correct except ac5x once is called a SoC and once a > RD-AC5X board. > > It cannot be both. Probably you need third compatible, assuming AC5x is > a flavor of AC5. Yeah it's a bit confusing RD-AC5X-(bunch of extra numbers and letters) is the board I have. AC5X is a L3 switch chip with integrated CPU. AC5 is a L3 switch chip with integrated CPU. Switch wise the AC5X and AC5 are quite different but the CPU block is the same between the two. > > items: > - enum: > - marvell,rd-ac5x > - const: marvell,ac5x > - const: marvell,ac5 I can go with that but I'm a little vague on what the requirements are. I was trying to follow the armada-7k-8k.yaml as an example. If I look at the cn9130-crb-A board it ends up with: compatible = "marvell,cn9130", "marvell,armada-ap807-quad", "marvell,armada-ap807"; I know the ap807 has something to do with the vagaries of the cn9130 SoC but isn't the "marvell,cn9130" still referring to the SoC. From what you've said shouldn't there be a "marvell,cn9130-crb" somewhere in the mix? Perhaps I've picked a bad example but the other dtbs I've poked at don't have any board binding. >> + >> +additionalProperties: true > > Best regards, > Krzysztof
On 12/05/22 13:20, Chris Packham wrote: > > On 12/05/22 04:34, Krzysztof Kozlowski wrote: >> On 11/05/2022 01:10, Chris Packham wrote: >>> Describe the compatible properties for the Marvell Alleycat5/5X >>> switches >>> with integrated CPUs. >>> >>> Alleycat5: >>> * 98DX2538: 24x1G + 2x10G + 2x10G Stack >>> * 98DX2535: 24x1G + 4x1G Stack >>> * 98DX2532: 8x1G + 2x10G + 2x1G Stack >>> * 98DX2531: 8x1G + 4x1G Stack >>> * 98DX2528: 24x1G + 2x10G + 2x10G Stack >>> * 98DX2525: 24x1G + 4x1G Stack >>> * 98DX2522: 8x1G + 2x10G + 2x1G Stack >>> * 98DX2521: 8x1G + 4x1G Stack >>> * 98DX2518: 24x1G + 2x10G + 2x10G Stack >>> * 98DX2515: 24x1G + 4x1G Stack >>> * 98DX2512: 8x1G + 2x10G + 2x1G Stack >>> * 98DX2511: 8x1G + 4x1G Stack >>> >>> Alleycat5X: >>> * 98DX3500: 24x1G + 6x25G >>> * 98DX3501: 16x1G + 6x10G >>> * 98DX3510: 48x1G + 6x25G >>> * 98DX3520: 24x2.5G + 6x25G >>> * 98DX3530: 48x2.5G + 6x25G >>> * 98DX3540: 12x5G/6x10G + 6x25G >>> * 98DX3550: 24x5G/12x10G + 6x25G >>> >>> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz> >>> --- >>> >>> Notes: >>> Changes in v6: >>> - New >>> >>> .../bindings/arm/marvell/armada-98dx2530.yaml | 27 >>> +++++++++++++++++++ >>> 1 file changed, 27 insertions(+) >>> create mode 100644 >>> Documentation/devicetree/bindings/arm/marvell/armada-98dx2530.yaml >>> >>> diff --git >>> a/Documentation/devicetree/bindings/arm/marvell/armada-98dx2530.yaml >>> b/Documentation/devicetree/bindings/arm/marvell/armada-98dx2530.yaml >>> new file mode 100644 >>> index 000000000000..6d9185baf0c5 >>> --- /dev/null >>> +++ >>> b/Documentation/devicetree/bindings/arm/marvell/armada-98dx2530.yaml >>> @@ -0,0 +1,27 @@ >>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >>> +%YAML 1.2 >>> +--- >>> +$id: >>> http://scanmail.trustwave.com/?c=20988&d=heX74s-dh8HSCAJmafRigZHOoyY0XQDl80QSCXWitw&u=http%3a%2f%2fdevicetree%2eorg%2fschemas%2farm%2fmarvell%2farmada-98dx2530%2eyaml%23 >>> +$schema: >>> http://scanmail.trustwave.com/?c=20988&d=heX74s-dh8HSCAJmafRigZHOoyY0XQDl80oVWnOltA&u=http%3a%2f%2fdevicetree%2eorg%2fmeta-schemas%2fcore%2eyaml%23 >>> + >>> +title: Marvell Alleycat5/5X Platforms >>> + >>> +maintainers: >>> + - Chris Packham <chris.packham@alliedtelesis.co.nz> >>> + >>> +properties: >>> + $nodename: >>> + const: '/' >>> + compatible: >>> + oneOf: >>> + >>> + - description: Alleycat5 (98DX25xx) >>> + items: >>> + - const: marvell,ac5 >> This is confusing and does not look correct. The DTS calls AC5 a SoC and >> you cannot have SoC alone. It's unusable without a SoM or board. >> >>> + >>> + - description: Alleycat5X (98DX35xx) >>> + items: >>> + - const: marvell,ac5x >>> + - const: marvell,ac5 >> This entry looks correct except ac5x once is called a SoC and once a >> RD-AC5X board. >> >> It cannot be both. Probably you need third compatible, assuming AC5x is >> a flavor of AC5. > > Yeah it's a bit confusing > > RD-AC5X-(bunch of extra numbers and letters) is the board I have. > AC5X is a L3 switch chip with integrated CPU. > AC5 is a L3 switch chip with integrated CPU. > > Switch wise the AC5X and AC5 are quite different but the CPU block is > the same between the two. > >> >> items: >> - enum: >> - marvell,rd-ac5x >> - const: marvell,ac5x >> - const: marvell,ac5 > > I can go with that but I'm a little vague on what the requirements > are. I was trying to follow the armada-7k-8k.yaml as an example. > > If I look at the cn9130-crb-A board it ends up with: > > compatible = "marvell,cn9130", "marvell,armada-ap807-quad", > "marvell,armada-ap807"; > > I know the ap807 has something to do with the vagaries of the cn9130 > SoC but isn't the "marvell,cn9130" still referring to the SoC. From > what you've said shouldn't there be a "marvell,cn9130-crb" somewhere > in the mix? > > Perhaps I've picked a bad example but the other dtbs I've poked at > don't have any board binding. > The fsl,imx boards seem like a better example to follow Documentation/devicetree/bindings/arm/fsl.yaml >>> + >>> +additionalProperties: true >> >> Best regards, >> Krzysztof
On 12/05/2022 03:20, Chris Packham wrote: > > On 12/05/22 04:34, Krzysztof Kozlowski wrote: >> On 11/05/2022 01:10, Chris Packham wrote: >>> Describe the compatible properties for the Marvell Alleycat5/5X switches >>> with integrated CPUs. >>> >>> Alleycat5: >>> * 98DX2538: 24x1G + 2x10G + 2x10G Stack >>> * 98DX2535: 24x1G + 4x1G Stack >>> * 98DX2532: 8x1G + 2x10G + 2x1G Stack >>> * 98DX2531: 8x1G + 4x1G Stack >>> * 98DX2528: 24x1G + 2x10G + 2x10G Stack >>> * 98DX2525: 24x1G + 4x1G Stack >>> * 98DX2522: 8x1G + 2x10G + 2x1G Stack >>> * 98DX2521: 8x1G + 4x1G Stack >>> * 98DX2518: 24x1G + 2x10G + 2x10G Stack >>> * 98DX2515: 24x1G + 4x1G Stack >>> * 98DX2512: 8x1G + 2x10G + 2x1G Stack >>> * 98DX2511: 8x1G + 4x1G Stack >>> >>> Alleycat5X: >>> * 98DX3500: 24x1G + 6x25G >>> * 98DX3501: 16x1G + 6x10G >>> * 98DX3510: 48x1G + 6x25G >>> * 98DX3520: 24x2.5G + 6x25G >>> * 98DX3530: 48x2.5G + 6x25G >>> * 98DX3540: 12x5G/6x10G + 6x25G >>> * 98DX3550: 24x5G/12x10G + 6x25G >>> >>> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz> >>> --- >>> >>> Notes: >>> Changes in v6: >>> - New >>> >>> .../bindings/arm/marvell/armada-98dx2530.yaml | 27 +++++++++++++++++++ >>> 1 file changed, 27 insertions(+) >>> create mode 100644 Documentation/devicetree/bindings/arm/marvell/armada-98dx2530.yaml >>> >>> diff --git a/Documentation/devicetree/bindings/arm/marvell/armada-98dx2530.yaml b/Documentation/devicetree/bindings/arm/marvell/armada-98dx2530.yaml >>> new file mode 100644 >>> index 000000000000..6d9185baf0c5 >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/arm/marvell/armada-98dx2530.yaml >>> @@ -0,0 +1,27 @@ >>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >>> +%YAML 1.2 >>> +--- >>> +$id: http://scanmail.trustwave.com/?c=20988&d=heX74s-dh8HSCAJmafRigZHOoyY0XQDl80QSCXWitw&u=http%3a%2f%2fdevicetree%2eorg%2fschemas%2farm%2fmarvell%2farmada-98dx2530%2eyaml%23 >>> +$schema: http://scanmail.trustwave.com/?c=20988&d=heX74s-dh8HSCAJmafRigZHOoyY0XQDl80oVWnOltA&u=http%3a%2f%2fdevicetree%2eorg%2fmeta-schemas%2fcore%2eyaml%23 >>> + >>> +title: Marvell Alleycat5/5X Platforms >>> + >>> +maintainers: >>> + - Chris Packham <chris.packham@alliedtelesis.co.nz> >>> + >>> +properties: >>> + $nodename: >>> + const: '/' >>> + compatible: >>> + oneOf: >>> + >>> + - description: Alleycat5 (98DX25xx) >>> + items: >>> + - const: marvell,ac5 >> This is confusing and does not look correct. The DTS calls AC5 a SoC and >> you cannot have SoC alone. It's unusable without a SoM or board. >> >>> + >>> + - description: Alleycat5X (98DX35xx) >>> + items: >>> + - const: marvell,ac5x >>> + - const: marvell,ac5 >> This entry looks correct except ac5x once is called a SoC and once a >> RD-AC5X board. >> >> It cannot be both. Probably you need third compatible, assuming AC5x is >> a flavor of AC5. > > Yeah it's a bit confusing > > RD-AC5X-(bunch of extra numbers and letters) is the board I have. > AC5X is a L3 switch chip with integrated CPU. > AC5 is a L3 switch chip with integrated CPU. > > Switch wise the AC5X and AC5 are quite different but the CPU block is > the same between the two. > >> >> items: >> - enum: >> - marvell,rd-ac5x >> - const: marvell,ac5x >> - const: marvell,ac5 > > I can go with that but I'm a little vague on what the requirements are. > I was trying to follow the armada-7k-8k.yaml as an example. > > If I look at the cn9130-crb-A board it ends up with: > > compatible = "marvell,cn9130", "marvell,armada-ap807-quad", > "marvell,armada-ap807"; > > I know the ap807 has something to do with the vagaries of the cn9130 SoC > but isn't the "marvell,cn9130" still referring to the SoC. From what > you've said shouldn't there be a "marvell,cn9130-crb" somewhere in the mix? > > Perhaps I've picked a bad example but the other dtbs I've poked at don't > have any board binding. The CN9130 looks wrong the same way. They have cn9130.dtsi with "Marvell Armada CN9130 SoC", so it is clearly a SoC. It has its own compatibles. Then this DTSI is included in board DTSes. Till now everything is correct. However the board DTS does not define its own compatible and re-uses SoC compatible, so this is wrong. It seems it was done like this inf commit 6a380172f171 ("dt-bindings: marvell: Declare the CN913x SoC compatibles") . That commit even explains "There are three development boards based on these SoCs:" but then fails to define these boards and instead later everything uses SoC compatibles as board ones! Anyone knowing Marvell HW/architecture could fix it? Best regards, Krzysztof
On 12/05/2022 03:51, Chris Packham wrote: >> >> I know the ap807 has something to do with the vagaries of the cn9130 >> SoC but isn't the "marvell,cn9130" still referring to the SoC. From >> what you've said shouldn't there be a "marvell,cn9130-crb" somewhere >> in the mix? >> >> Perhaps I've picked a bad example but the other dtbs I've poked at >> don't have any board binding. >> > The fsl,imx boards seem like a better example to follow > Documentation/devicetree/bindings/arm/fsl.yaml Yes. Best regards, Krzysztof
diff --git a/Documentation/devicetree/bindings/arm/marvell/armada-98dx2530.yaml b/Documentation/devicetree/bindings/arm/marvell/armada-98dx2530.yaml new file mode 100644 index 000000000000..6d9185baf0c5 --- /dev/null +++ b/Documentation/devicetree/bindings/arm/marvell/armada-98dx2530.yaml @@ -0,0 +1,27 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/arm/marvell/armada-98dx2530.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Marvell Alleycat5/5X Platforms + +maintainers: + - Chris Packham <chris.packham@alliedtelesis.co.nz> + +properties: + $nodename: + const: '/' + compatible: + oneOf: + + - description: Alleycat5 (98DX25xx) + items: + - const: marvell,ac5 + + - description: Alleycat5X (98DX35xx) + items: + - const: marvell,ac5x + - const: marvell,ac5 + +additionalProperties: true
Describe the compatible properties for the Marvell Alleycat5/5X switches with integrated CPUs. Alleycat5: * 98DX2538: 24x1G + 2x10G + 2x10G Stack * 98DX2535: 24x1G + 4x1G Stack * 98DX2532: 8x1G + 2x10G + 2x1G Stack * 98DX2531: 8x1G + 4x1G Stack * 98DX2528: 24x1G + 2x10G + 2x10G Stack * 98DX2525: 24x1G + 4x1G Stack * 98DX2522: 8x1G + 2x10G + 2x1G Stack * 98DX2521: 8x1G + 4x1G Stack * 98DX2518: 24x1G + 2x10G + 2x10G Stack * 98DX2515: 24x1G + 4x1G Stack * 98DX2512: 8x1G + 2x10G + 2x1G Stack * 98DX2511: 8x1G + 4x1G Stack Alleycat5X: * 98DX3500: 24x1G + 6x25G * 98DX3501: 16x1G + 6x10G * 98DX3510: 48x1G + 6x25G * 98DX3520: 24x2.5G + 6x25G * 98DX3530: 48x2.5G + 6x25G * 98DX3540: 12x5G/6x10G + 6x25G * 98DX3550: 24x5G/12x10G + 6x25G Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz> --- Notes: Changes in v6: - New .../bindings/arm/marvell/armada-98dx2530.yaml | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) create mode 100644 Documentation/devicetree/bindings/arm/marvell/armada-98dx2530.yaml