Message-ID: <20220614081633.GA14160@plvision.eu>
Date:   Tue, 14 Jun 2022 11:16:33 +0300
From:   Vadym Kochan <vadym.kochan@...ision.eu>
To:     Chris Packham <Chris.Packham@...iedtelesis.co.nz>
Cc:     Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Catalin Marinas <catalin.marinas@....com>,
        Will Deacon <will@...nel.org>,
        Gregory CLEMENT <gregory.clement@...tlin.com>,
        Konstantin Porotchkin <kostap@...vell.com>,
        Robert Marko <robert.marko@...tura.hr>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH v9 2/3] arm64: dts: marvell: Add Armada 98DX2530 SoC and
 RD-AC5X board

Hi Chris,

On Tue, Jun 14, 2022 at 05:26:39AM +0000, Chris Packham wrote:
> 
> On 14/06/22 17:11, Chris Packham wrote:
> >
> > On 14/06/22 10:53, Vadym Kochan wrote:
> >> From: Chris Packham <chris.packham@...iedtelesis.co.nz>
> >>
> >> The 98DX2530 SoC is the Control and Management CPU integrated into
> >> the Marvell 98DX25xx and 98DX35xx series of switch chip (internally
> >> referred to as AlleyCat5 and AlleyCat5X).
> >>
> >> These files have been taken from the Marvell SDK and lightly cleaned
> >> up with the License and copyright retained.
> >>
> >> Signed-off-by: Chris Packham <chris.packham@...iedtelesis.co.nz>
> >> Signed-off-by: Vadym Kochan <vadym.kochan@...ision.eu>
> >> ---
> >>
> >> Notes:
> >>      The Marvell SDK has a number of new compatible strings. I've 
> >> brought
> >>      through some of the drivers or where possible used an in-tree
> >>      alternative (e.g. there is SDK code for a ac5-gpio but two 
> >> instances of
> >>      the existing marvell,orion-gpio seems to cover what is needed if 
> >> you use
> >>      an appropriate binding). I expect that there will a new series of
> >>      patches when I get some different hardware (or additions to this 
> >> series
> >>      depending on if/when it lands).
> >>           Changes in v9 (proposed by Marvell):
> >>        It was discussed with Chris that Marvell will add some changes:
> >>
> >>           1) Rename "armada-" prefix in dts(i) file names to ac5, 
> >> because
> >>              Armada has not much common with AC5 SoC.
> >>
> >>           2) Add clock fixes:
> >>              - rename core_clock to cnm_clock
> >>              - remove axi_clock
> >>              - change cnm_clock to 325MHZ
> >>              - use cnm_clock for the UART
> >>
> >>      Changes in v8:
> >>      - Remove unnecessary clock-frequency property on armv8-timer
> >>      - Remove unnecessary redistributor-stride property on gic
> >>      - Add GIC_SPI interrupts for gpios
> >>      Changes in v7:
> >>      - Add missing compatible on usb1
> >>      - Add "rd-ac5x" compatible for board
> >>      - Move aliases to board dts
> >>      - Move board specific usb info to board dts
> >>      - Consolidate usb1 board settings and remove unnecessary compatible
> >>      - Add Allied Telesis copyright
> >>      - Rename files after mailng-list discussion
> >>      Changes in v6:
> >>      - Move CPU nodes above the SoC (Krzysztof)
> >>      - Minor formatting clean ups (Krzysztof)
> >>      - Run through `make dtbs_check`
> >>      - Move gic nodes inside SoC
> >>      - Group clocks under a clock node
> >>      Changes in v5:
> >>      - add #{address,size}-cells property to i2c nodes
> >>      - make i2c nodes disabled in the SoC and enable them in the board
> >>      - add interrupt controller attributes to gpio nodes
> >>      - Move fixed-clock nodes up a level and remove unnecessary @0
> >>      Changes in v4:
> >>      - use 'phy-handle' instead of 'phy'
> >>      - move status="okay" on usb nodes to board dts
> >>      - Add review from Andrew
> >>      Changes in v3:
> >>      - Move memory node to board
> >>      - Use single digit reg value for phy address
> >>      - Remove MMC node (driver needs work)
> >>      - Remove syscon & simple-mfd for pinctrl
> >>      Changes in v2:
> >>      - Make pinctrl a child node of a syscon node
> >>      - Use marvell,armada-8k-gpio instead of orion-gpio
> >>      - Remove nand peripheral. The Marvell SDK does have some changes 
> >> for the
> >>        ac5-nand-controller but I currently lack hardware with NAND 
> >> fitted so
> >>        I can't test it right now. I've therefore chosen to omit the 
> >> node and
> >>        not attempted to bring in the driver or binding.
> >>      - Remove pcie peripheral. Again there are changes in the SDK and 
> >> I have
> >>        no way of testing them.
> >>      - Remove prestera node.
> >>      - Remove "marvell,ac5-ehci" compatible from USB node as
> >>        "marvell,orion-ehci" is sufficient
> >>      - Remove watchdog node. There is a buggy driver for the ac5 
> >> watchdog in
> >>        the SDK but it needs some work so I've dropped the node for now.
> >>
> >>   arch/arm64/boot/dts/marvell/Makefile          |   1 +
> >>   arch/arm64/boot/dts/marvell/ac5-98dx25xx.dtsi | 291 ++++++++++++++++++
> >>   .../boot/dts/marvell/ac5-98dx35xx-rd.dts      | 101 ++++++
> >>   arch/arm64/boot/dts/marvell/ac5-98dx35xx.dtsi |  13 +
> >>   4 files changed, 406 insertions(+)
> >>   create mode 100644 arch/arm64/boot/dts/marvell/ac5-98dx25xx.dtsi
> >>   create mode 100644 arch/arm64/boot/dts/marvell/ac5-98dx35xx-rd.dts
> >>   create mode 100644 arch/arm64/boot/dts/marvell/ac5-98dx35xx.dtsi
> >>

[snip]

> >> +
> >> +            uart0: serial@...00 {
> >> +                compatible = "snps,dw-apb-uart";
> >> +                reg = <0x12000 0x100>;
> >> +                reg-shift = <2>;
> >> +                interrupts = <GIC_SPI 83 IRQ_TYPE_LEVEL_HIGH>;
> >> +                reg-io-width = <1>;
> >> +                clocks = <&cnm_clock>;
> >
> > With this change I see some garbled output when the "Serial: AMBA 
> > PL011 UART" driver starts.
> >
> > It could be that my bootloader has the same wrong clock value as the 
> > earlier iteration of this device tree.
> Fixing u-boot doesn't help but there are also references to 328000000 in 
> mv-ddr and ATF so I'll look to see if updating them fixes the issue 
> tomorrow.
> >

Interesting, because Marvell suggested to use cnm_clock with 328MHz for AC5, and 325MHz
for AC5X.

[snip]

> >> +
> >> +    clocks {
> >> +        cnm_clock: core-clock {
> >> +            compatible = "fixed-clock";
> >> +            #clock-cells = <0>;
> >> +            clock-frequency = <325000000>;
> >> +        };
> >> +
> >> +        spi_clock: spi-clock {
> >> +            compatible = "fixed-clock";
> >> +            #clock-cells = <0>;
> >> +            clock-frequency = <200000000>;
> >> +        };
> >> +    };
> >> +};

[snip]

Regards,

Powered by blists - more mailing lists