Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Tue, 14 Jun 2022 11:16:33 +0300
From:   Vadym Kochan <vadym.kochan@plvision.eu>
To:     Chris Packham <Chris.Packham@alliedtelesis.co.nz>
Cc:     Rob Herring <robh+dt@kernel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        Catalin Marinas <catalin.marinas@arm.com>,
        Will Deacon <will@kernel.org>,
        Gregory CLEMENT <gregory.clement@bootlin.com>,
        Konstantin Porotchkin <kostap@marvell.com>,
        Robert Marko <robert.marko@sartura.hr>,
        "devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "linux-arm-kernel@lists.infradead.org" 
        <linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH v9 2/3] arm64: dts: marvell: Add Armada 98DX2530 SoC and
 RD-AC5X board
Message-ID: <20220614081633.GA14160@plvision.eu>
References: <20220613225338.393-1-vadym.kochan@plvision.eu>
 <20220613225338.393-3-vadym.kochan@plvision.eu>
 <4670aab9-b7fa-f2c7-567a-12ca24535b0c@alliedtelesis.co.nz>
 <37d7b4f1-fd39-dc05-374a-764070c56cc6@alliedtelesis.co.nz>
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <37d7b4f1-fd39-dc05-374a-764070c56cc6@alliedtelesis.co.nz>
User-Agent: Mutt/1.9.4 (2018-02-28)
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?iso-8859-1?Q?zywKrFYZ3hrPwjc8KBYCxPs4/0WVTaG+m7ocJuTcHbyOgwngcncCzfx25q?=
 =?iso-8859-1?Q?IuY2Jm2WBFWKMkybN0y9hR7aQUrgJrYtuK0KMIom7T+qSTgm5doe5q1k0t?=
 =?iso-8859-1?Q?HGHGZxhLby31+wKDvz6qoV/nmo3WFSTrT4U1LK3WxgOZr1+UVtUx2oJwjk?=
 =?iso-8859-1?Q?YW/oszrSZvGXiWbNtr4AIMOsbIA/W1YSlXvTHpWFox2WKU0hQ5kIcjCBhj?=
 =?iso-8859-1?Q?/Vzlfm3WkxBZ9csUJQ188VRrqxCkqMl6jyhX+8B8mNSQtb6XF8YHMsPrLk?=
 =?iso-8859-1?Q?CGy4jnqOdtOJMifdNMo41vahfwFxB5i02+sBObmWYRPdMNQRc1eL+u0qIs?=
 =?iso-8859-1?Q?DTzoa3LVGGlrOei7uFd3f4lTp1NP1nek9yiPRRPeCmCBJSjcW+3vymtn6G?=
 =?iso-8859-1?Q?MXGoAxw7//FZ06WDDOsNKQEzhjZ5FsmXiuQQ/UMkDHBsBRX8590TZJ1J4L?=
 =?iso-8859-1?Q?hSyeaUU32e4TIy3yISmLOLX7foa4cXhQt4fv7VIAhMX+lRUnCVjSWhlPji?=
 =?iso-8859-1?Q?WvfSY4L+fduXyPGRJmWemhVTj13K+ReopyM2+cwYXk/caPOcyn3ZC947wH?=
 =?iso-8859-1?Q?NmBIxczWs7Rblsnikyh4/jPVI1eErQ3ceYZENOq2isxjcFNarSDFAFRRdS?=
 =?iso-8859-1?Q?91oafnXRb/MVM5EK+rTwlmBcIw+Eqrem9ppsO6vkNMELB5SHNboWNGbW+W?=
 =?iso-8859-1?Q?WsQQCpeEKt4iA7e0u1dHfcaLVs4eDGS9rBpgLxFjFT6HIykmLCtk/yka52?=
 =?iso-8859-1?Q?OF2G9+w+tbX/nBWd/TI/SNgV1FxJ3GC9sA+Dx4+OAs8NYGBdCRov9GlHLX?=
 =?iso-8859-1?Q?tmjeIzFsnVNGu5xaMcNiq900LMR4wFopN8HtiOCHSxsHS48f3sg4eQuHEh?=
 =?iso-8859-1?Q?VslOCdLKaFcZc8U/+OnbW4zGftclCtZccOi07GAkf3WxGwK7BkjWRmtMjf?=
 =?iso-8859-1?Q?AnNNTk5I51m70T9JqSCRovV98OLhQLxBlJgt3NCE5yOxp3GJCZTf/4/XfM?=
 =?iso-8859-1?Q?Tnee2MKR+bW/pZ2u4CuoFaWdEcl0WeKO1C+AN6Swas4ngeoEoSDYwusV6K?=
 =?iso-8859-1?Q?pXB//VQNmY+8EFywI8jtuaq0pd/mWtNCAJwta3CN/MhRaDicljNJT72uJE?=
 =?iso-8859-1?Q?Bz2sQqtASOi11Rn3bC1npS6DMWbMqJ3CJIOZmmpbVYMB6cmxPMaEGUN9qa?=
 =?iso-8859-1?Q?7iGji5BFvSGDrvwLwLm2Pz55ccujmqRXHXEsq1w1MLOFc5IaF7vZHSfPd/?=
 =?iso-8859-1?Q?Pb1F4UQ9cGq4nx5T9k6tp/j4vclbw9BXljj5beJDqTx4a9kduOWI8YLJAA?=
 =?iso-8859-1?Q?QynNblb0+MJ1rCuSxqPwPUDMAWH1NYu3a7MQAElVVjv/POElFklYEoxl7u?=
 =?iso-8859-1?Q?It1uP0PWVwK/1zEJI9cLIntJEk92xrvdA8Yjaai1tHfJaKPiIrzP/Bh8VQ?=
 =?iso-8859-1?Q?Va6XXuis1hhB8desvMofep2QZJLqw6HzcylHM7k7qg5hBJCMe5Gkl7dXtp?=
 =?iso-8859-1?Q?Nf3DZa630o50HU0CH9n83qnlDphoeMYJBaE/JBCdUJ4pTbqjzWOTF8y16s?=
 =?iso-8859-1?Q?Q01iTatJQD0GTqf/nQcuoQ2vpNx99AAul0b50X9o7hz36nF+Gxd7PyjY7v?=
 =?iso-8859-1?Q?lCpOaZdWMMtA3QmEAe4u/M0napdTlt9Qrz74MbN8zqhsY2dMJj30MmrVg2?=
 =?iso-8859-1?Q?u+1cbnEz5LykD1Tr9zzprke+nkUuf557g0eCuAHAARQyiH3076b9jCGlif?=
 =?iso-8859-1?Q?/z1P7hE8B8e3E0ThIE/2ebn35kHL4b47hNcQr+2cVnP5IN+5k9t5ZW+ccr?=
 =?iso-8859-1?Q?YdBiub/QKXA8UqMEeFyWJ78cCchDnR8=3D?=
X-OriginatorOrg: plvision.eu
X-MS-Exchange-CrossTenant-Network-Message-Id: 99954d9e-3764-4e9a-ad61-08da4dde32c0
X-MS-Exchange-CrossTenant-AuthSource: VI1P190MB0317.EURP190.PROD.OUTLOOK.COM
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 14 Jun 2022 08:16:35.7006
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 03707b74-30f3-46b6-a0e0-ff0a7438c9c4
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: gnLfff3T2ZJwC14lAWmAkOxw0yFMP1F0ef/104/KR0JZtnKLhVZ7I3mICwY4osxqPRGcvV98qkDzUxdninlOXLj6ucRI8EYFHodJvdeG7u4=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PAXP190MB1693
X-Spam-Status: No, score=-1.7 required=5.0 tests=BAYES_00,DKIM_INVALID,
        DKIM_SIGNED,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,
        SPF_PASS,T_SCC_BODY_TEXT_LINE autolearn=no autolearn_force=no
        version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

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@alliedtelesis.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@alliedtelesis.co.nz>
> >> Signed-off-by: Vadym Kochan <vadym.kochan@plvision.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@12000 {
> >> +??????????????? 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,