linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v1 0/4] arm64: mvebu: Support for Marvell 98DX2530 (and variants)
@ 2022-03-10  3:00 Chris Packham
  2022-03-10  3:00 ` [PATCH v1 1/4] dt-bindings: pinctrl: mvebu: Document bindings for AC5 Chris Packham
                   ` (3 more replies)
  0 siblings, 4 replies; 14+ messages in thread
From: Chris Packham @ 2022-03-10  3:00 UTC (permalink / raw)
  To: linus.walleij, robh+dt, catalin.marinas, will, andrew,
	gregory.clement, sebastian.hesselbarth, kostap, robert.marko
  Cc: linux-gpio, devicetree, linux-kernel, linux-arm-kernel, Chris Packham

This series adds support for the Marvell 98DX2530 SoC which is the Control and
Management CPU integrated into the AlleyCat5/AlleyCat5X series of Marvell
switches.

The CPU core is an ARM Cortex-A55 with neon, simd and crypto extensions.

This is fairly similar to the Armada-3700 SoC so most of the required
peripherals are already supported. This series adds a devicetree and pinctrl
driver for the SoC and the RD-AC5X-32G16HVG6HLG reference board.

Chris Packham (4):
  dt-bindings: pinctrl: mvebu: Document bindings for AC5
  pinctrl: mvebu: pinctrl driver for 98DX2530 SoC
  arm64: dts: marvell: Add Armada 98DX2530 SoC and RD-AC5X board
  arm64: marvell: enable the 98DX2530 pinctrl driver

 .../bindings/pinctrl/marvell,ac5-pinctrl.yaml |  73 +++
 arch/arm64/Kconfig.platforms                  |   2 +
 arch/arm64/boot/dts/marvell/Makefile          |   1 +
 .../boot/dts/marvell/armada-98dx2530.dtsi     | 459 ++++++++++++++++++
 arch/arm64/boot/dts/marvell/rd-ac5x.dts       |  27 ++
 drivers/pinctrl/mvebu/Kconfig                 |   4 +
 drivers/pinctrl/mvebu/Makefile                |   1 +
 drivers/pinctrl/mvebu/pinctrl-ac5.c           | 226 +++++++++
 8 files changed, 793 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pinctrl/marvell,ac5-pinctrl.yaml
 create mode 100644 arch/arm64/boot/dts/marvell/armada-98dx2530.dtsi
 create mode 100644 arch/arm64/boot/dts/marvell/rd-ac5x.dts
 create mode 100644 drivers/pinctrl/mvebu/pinctrl-ac5.c

-- 
2.35.1


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v1 1/4] dt-bindings: pinctrl: mvebu: Document bindings for AC5
  2022-03-10  3:00 [PATCH v1 0/4] arm64: mvebu: Support for Marvell 98DX2530 (and variants) Chris Packham
@ 2022-03-10  3:00 ` Chris Packham
  2022-03-10 23:25   ` Rob Herring
  2022-03-10  3:00 ` [PATCH v1 2/4] pinctrl: mvebu: pinctrl driver for 98DX2530 SoC Chris Packham
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 14+ messages in thread
From: Chris Packham @ 2022-03-10  3:00 UTC (permalink / raw)
  To: linus.walleij, robh+dt, catalin.marinas, will, andrew,
	gregory.clement, sebastian.hesselbarth, kostap, robert.marko
  Cc: linux-gpio, devicetree, linux-kernel, linux-arm-kernel, Chris Packham

Add JSON schema for marvell,ac5-pinctrl present on the Marvell 98DX2530
SoC.

Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
---
 .../bindings/pinctrl/marvell,ac5-pinctrl.yaml | 73 +++++++++++++++++++
 1 file changed, 73 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pinctrl/marvell,ac5-pinctrl.yaml

diff --git a/Documentation/devicetree/bindings/pinctrl/marvell,ac5-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/marvell,ac5-pinctrl.yaml
new file mode 100644
index 000000000000..c7ab3d0e8420
--- /dev/null
+++ b/Documentation/devicetree/bindings/pinctrl/marvell,ac5-pinctrl.yaml
@@ -0,0 +1,73 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pinctrl/marvell,ac5-pinctrl.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Marvell AC5 pin controller
+
+maintainers:
+  - Chris Packham <chris.packham@alliedtelesis.co.nz>
+
+description:
+  Bindings for Marvell's AC5 memory-mapped pin controller.
+
+properties:
+  compatible:
+    items:
+      - const: marvell,ac5-pinctrl
+      - const: syscon
+      - const: simple-mfd
+  reg:
+    maxItems: 1
+
+patternProperties:
+  '-pins$':
+    type: object
+    $ref: pinmux-node.yaml#
+
+    properties:
+      marvell,function:
+        $ref: "/schemas/types.yaml#/definitions/string"
+        description:
+          Indicates the function to select.
+        enum: [ gpio, i2c0, i2c1, nand, sdio, spi0, spi1, uart0, uart1, uart2, uart3 ]
+
+      marvell,pins:
+        $ref: /schemas/types.yaml#/definitions/string-array
+        description:
+          Array of MPP pins to be used for the given function.
+        minItems: 1
+        items:
+          enum: [ mpp0, mpp1, mpp2, mpp3, mpp4, mpp5, mpp6, mpp7, mpp8, mpp9,
+                  mpp10, mpp11, mpp12, mpp13, mpp14, mpp15, mpp16, mpp17, mpp18, mpp19,
+                  mpp20, mpp21, mpp22, mpp23, mpp24, mpp25, mpp26, mpp27, mpp28, mpp29,
+                  mpp30, mpp31, mpp32, mpp33, mpp34, mpp35, mpp36, mpp37, mpp38, mpp39,
+                  mpp40, mpp41, mpp42, mpp43, mpp44, mpp45 ]
+
+allOf:
+  - $ref: "pinctrl.yaml#"
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+    pinctrl@80020100 {
+      compatible = "marvell,ac5-pinctrl",
+      "syscon", "simple-mfd";
+      reg = <0x80020100 0x20>;
+
+      i2c0_pins: i2c0-pins {
+        marvell,pins = "mpp26", "mpp27";
+        marvell,function = "i2c0";
+      };
+
+      i2c0_gpio: i2c0-gpio-pins {
+        marvell,pins = "mpp26", "mpp27";
+        marvell,function = "gpio";
+      };
+    };
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v1 2/4] pinctrl: mvebu: pinctrl driver for 98DX2530 SoC
  2022-03-10  3:00 [PATCH v1 0/4] arm64: mvebu: Support for Marvell 98DX2530 (and variants) Chris Packham
  2022-03-10  3:00 ` [PATCH v1 1/4] dt-bindings: pinctrl: mvebu: Document bindings for AC5 Chris Packham
@ 2022-03-10  3:00 ` Chris Packham
  2022-03-10 13:59   ` Andrew Lunn
  2022-03-10  3:00 ` [PATCH v1 3/4] arm64: dts: marvell: Add Armada 98DX2530 SoC and RD-AC5X board Chris Packham
  2022-03-10  3:00 ` [PATCH v1 4/4] arm64: marvell: enable the 98DX2530 pinctrl driver Chris Packham
  3 siblings, 1 reply; 14+ messages in thread
From: Chris Packham @ 2022-03-10  3:00 UTC (permalink / raw)
  To: linus.walleij, robh+dt, catalin.marinas, will, andrew,
	gregory.clement, sebastian.hesselbarth, kostap, robert.marko
  Cc: linux-gpio, devicetree, linux-kernel, linux-arm-kernel, Chris Packham

This pinctrl driver supports the 98DX25xx and 98DX35xx family of chips
from Marvell. It is based on the Marvell SDK with additions for various
(non-gpio) pin configurations based on the datasheet.

Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
---
 drivers/pinctrl/mvebu/Kconfig       |   4 +
 drivers/pinctrl/mvebu/Makefile      |   1 +
 drivers/pinctrl/mvebu/pinctrl-ac5.c | 226 ++++++++++++++++++++++++++++
 3 files changed, 231 insertions(+)
 create mode 100644 drivers/pinctrl/mvebu/pinctrl-ac5.c

diff --git a/drivers/pinctrl/mvebu/Kconfig b/drivers/pinctrl/mvebu/Kconfig
index 0d12894d3ee1..aa5883f09d7b 100644
--- a/drivers/pinctrl/mvebu/Kconfig
+++ b/drivers/pinctrl/mvebu/Kconfig
@@ -45,6 +45,10 @@ config PINCTRL_ORION
 	bool
 	select PINCTRL_MVEBU
 
+config PINCTRL_AC5
+	bool
+	select PINCTRL_MVEBU
+
 config PINCTRL_ARMADA_37XX
 	bool
 	select GENERIC_PINCONF
diff --git a/drivers/pinctrl/mvebu/Makefile b/drivers/pinctrl/mvebu/Makefile
index cd082dca4482..23458ab17c53 100644
--- a/drivers/pinctrl/mvebu/Makefile
+++ b/drivers/pinctrl/mvebu/Makefile
@@ -11,3 +11,4 @@ obj-$(CONFIG_PINCTRL_ARMADA_CP110) += pinctrl-armada-cp110.o
 obj-$(CONFIG_PINCTRL_ARMADA_XP)  += pinctrl-armada-xp.o
 obj-$(CONFIG_PINCTRL_ARMADA_37XX)  += pinctrl-armada-37xx.o
 obj-$(CONFIG_PINCTRL_ORION)  += pinctrl-orion.o
+obj-$(CONFIG_PINCTRL_AC5) += pinctrl-ac5.o
diff --git a/drivers/pinctrl/mvebu/pinctrl-ac5.c b/drivers/pinctrl/mvebu/pinctrl-ac5.c
new file mode 100644
index 000000000000..452c8a6c2a30
--- /dev/null
+++ b/drivers/pinctrl/mvebu/pinctrl-ac5.c
@@ -0,0 +1,226 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Marvell ac5 pinctrl driver based on mvebu pinctrl core
+ *
+ * Copyright (C) 2021 Marvell
+ *
+ * Noam Liron <lnoam@marvell.com>
+ */
+
+#include <linux/err.h>
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/platform_device.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/pinctrl/pinctrl.h>
+
+#include "pinctrl-mvebu.h"
+
+static struct mvebu_mpp_mode ac5_mpp_modes[] = {
+	MPP_MODE(0,
+		 MPP_FUNCTION(0, "gpio",  NULL),
+		 MPP_FUNCTION(1, "sdio",  "d0"),
+		 MPP_FUNCTION(2, "nand",  "io4")),
+	MPP_MODE(1,
+		 MPP_FUNCTION(0, "gpio",  NULL),
+		 MPP_FUNCTION(1, "sdio",  "d1"),
+		 MPP_FUNCTION(2, "nand",  "io3")),
+	MPP_MODE(2,
+		 MPP_FUNCTION(0, "gpio",  NULL),
+		 MPP_FUNCTION(1, "sdio",  "d2"),
+		 MPP_FUNCTION(2, "nand",  "io2")),
+	MPP_MODE(3,
+		 MPP_FUNCTION(0, "gpio",  NULL),
+		 MPP_FUNCTION(1, "sdio",  "d3"),
+		 MPP_FUNCTION(2, "nand",  "io7")),
+	MPP_MODE(4,
+		 MPP_FUNCTION(0, "gpio",  NULL),
+		 MPP_FUNCTION(1, "sdio",  "d4"),
+		 MPP_FUNCTION(2, "nand",  "io6"),
+		 MPP_FUNCTION(3, "uart3", "txd"),
+		 MPP_FUNCTION(4, "uart2", "txd")),
+	MPP_MODE(5,
+		 MPP_FUNCTION(0, "gpio",  NULL),
+		 MPP_FUNCTION(1, "sdio",  "d5"),
+		 MPP_FUNCTION(2, "nand",  "io5"),
+		 MPP_FUNCTION(3, "uart3", "rxd"),
+		 MPP_FUNCTION(4, "uart2", "rxd")),
+	MPP_MODE(6,
+		 MPP_FUNCTION(0, "gpio",  NULL),
+		 MPP_FUNCTION(1, "sdio",  "d6"),
+		 MPP_FUNCTION(2, "nand",  "io0"),
+		 MPP_FUNCTION(3, "i2c1",  "sck")),
+	MPP_MODE(7,
+		 MPP_FUNCTION(0, "gpio",  NULL),
+		 MPP_FUNCTION(1, "sdio",  "d7"),
+		 MPP_FUNCTION(2, "nand",  "io1"),
+		 MPP_FUNCTION(3, "i2c1",  "sda")),
+	MPP_MODE(8,
+		 MPP_FUNCTION(0, "gpio",  NULL),
+		 MPP_FUNCTION(1, "sdio",  "clk"),
+		 MPP_FUNCTION(2, "nand",  "wen")),
+	MPP_MODE(9,
+		 MPP_FUNCTION(0, "gpio",  NULL),
+		 MPP_FUNCTION(1, "sdio",  "cmd"),
+		 MPP_FUNCTION(2, "nand",  "ale")),
+	MPP_MODE(10,
+		 MPP_FUNCTION(0, "gpio",  NULL),
+		 MPP_FUNCTION(1, "sdio",  "ds"),
+		 MPP_FUNCTION(2, "nand",  "cle")),
+	MPP_MODE(11,
+		 MPP_FUNCTION(0, "gpio",  NULL),
+		 MPP_FUNCTION(1, "sdio",  "rst"),
+		 MPP_FUNCTION(2, "nand",  "cen")),
+	MPP_MODE(12,
+		 MPP_FUNCTION(0, "gpio",  NULL),
+		 MPP_FUNCTION(1, "spi0",  "clk")),
+	MPP_MODE(13,
+		 MPP_FUNCTION(0, "gpio",  NULL),
+		 MPP_FUNCTION(1, "spi0",  "csn")),
+	MPP_MODE(14,
+		 MPP_FUNCTION(0, "gpio",  NULL),
+		 MPP_FUNCTION(1, "spi0",  "mosi")),
+	MPP_MODE(15,
+		 MPP_FUNCTION(0, "gpio",  NULL),
+		 MPP_FUNCTION(1, "spi0",  "miso")),
+	MPP_MODE(16,
+		 MPP_FUNCTION(0, "gpio",  NULL),
+		 MPP_FUNCTION(1, "spi0",  "wpn"),
+		 MPP_FUNCTION(2, "nand",  "ren"),
+		 MPP_FUNCTION(3, "uart1", "txd")),
+	MPP_MODE(17,
+		 MPP_FUNCTION(0, "gpio",  NULL),
+		 MPP_FUNCTION(1, "spi0",  "hold"),
+		 MPP_FUNCTION(2, "nand",  "rb"),
+		 MPP_FUNCTION(3, "uart1", "rxd")),
+	MPP_MODE(18,
+		 MPP_FUNCTION(0, "gpio",  NULL),
+		 MPP_FUNCTION(2, "uart2", "rxd")),
+	MPP_MODE(19,
+		 MPP_FUNCTION(0, "gpio",  NULL),
+		 MPP_FUNCTION(2, "uart2", "txd")),
+	MPP_MODE(20,
+		 MPP_FUNCTION(0, "gpio",  NULL),
+		 MPP_FUNCTION(2, "i2c1",  "sck"),
+		 MPP_FUNCTION(3, "spi1",  "clk"),
+		 MPP_FUNCTION(4, "uart3", "txd")),
+	MPP_MODE(21,
+		 MPP_FUNCTION(0, "gpio",  NULL),
+		 MPP_FUNCTION(2, "i2c1",  "sda"),
+		 MPP_FUNCTION(3, "spi1",  "csn"),
+		 MPP_FUNCTION(4, "uart3", "rxd")),
+	MPP_MODE(22,
+		 MPP_FUNCTION(0, "gpio",  NULL),
+		 MPP_FUNCTION(3, "spi1",  "mosi")),
+	MPP_MODE(23,
+		 MPP_FUNCTION(0, "gpio",  NULL),
+		 MPP_FUNCTION(3, "spi1",  "miso")),
+	MPP_MODE(24,
+		 MPP_FUNCTION(0, "gpio",  NULL),
+		 MPP_FUNCTION(2, "uart2", "txd")),
+	MPP_MODE(25,
+		 MPP_FUNCTION(0, "gpio",  NULL),
+		 MPP_FUNCTION(2, "uart2", "rxd")),
+	MPP_MODE(26,
+		 MPP_FUNCTION(0, "gpio",  NULL),
+		 MPP_FUNCTION(1, "i2c0",  "sck"),
+		 MPP_FUNCTION(3, "uart3", "txd")),
+	MPP_MODE(27,
+		 MPP_FUNCTION(0, "gpio",  NULL),
+		 MPP_FUNCTION(1, "i2c0",  "sda"),
+		 MPP_FUNCTION(3, "uart3", "rxd")),
+	MPP_MODE(28,
+		 MPP_FUNCTION(0, "gpio",  NULL),
+		 MPP_FUNCTION(3, "uart3", "txd")),
+	MPP_MODE(29,
+		 MPP_FUNCTION(0, "gpio",  NULL),
+		 MPP_FUNCTION(3, "uart3", "rxd")),
+	MPP_MODE(30,
+		 MPP_FUNCTION(0, "gpio",  NULL)),
+	MPP_MODE(31,
+		 MPP_FUNCTION(0, "gpio",  NULL)),
+	MPP_MODE(32,
+		 MPP_FUNCTION(0, "gpio",  NULL),
+		 MPP_FUNCTION(1, "uart0", "txd")),
+	MPP_MODE(33,
+		 MPP_FUNCTION(0, "gpio",  NULL),
+		 MPP_FUNCTION(1, "uart0", "rxd")),
+	MPP_MODE(34,
+		 MPP_FUNCTION(0, "gpio",  NULL),
+		 MPP_FUNCTION(2, "uart3", "rxd")),
+	MPP_MODE(35,
+		 MPP_FUNCTION(0, "gpio",  NULL),
+		 MPP_FUNCTION(2, "uart3", "txd")),
+	MPP_MODE(36,
+		 MPP_FUNCTION(0, "gpio",  NULL)),
+	MPP_MODE(37,
+		 MPP_FUNCTION(0, "gpio",  NULL)),
+	MPP_MODE(38,
+		 MPP_FUNCTION(0, "gpio",  NULL)),
+	MPP_MODE(39,
+		 MPP_FUNCTION(0, "gpio",  NULL)),
+	MPP_MODE(40,
+		 MPP_FUNCTION(0, "gpio",  NULL)),
+	MPP_MODE(41,
+		 MPP_FUNCTION(0, "gpio",  NULL),
+		 MPP_FUNCTION(4, "uart2", "txd"),
+		 MPP_FUNCTION(5, "i2c1",  "sck")),
+	MPP_MODE(42,
+		 MPP_FUNCTION(0, "gpio",  NULL),
+		 MPP_FUNCTION(4, "uart2", "rxd"),
+		 MPP_FUNCTION(5, "i2c1",  "sda")),
+	MPP_MODE(43,
+		 MPP_FUNCTION(0, "gpio",  NULL)),
+	MPP_MODE(44,
+		 MPP_FUNCTION(0, "gpio",  NULL)),
+	MPP_MODE(45,
+		 MPP_FUNCTION(0, "gpio",  NULL)),
+};
+
+static struct mvebu_pinctrl_soc_info ac5_pinctrl_info;
+
+static const struct of_device_id ac5_pinctrl_of_match[] = {
+	{
+		.compatible = "marvell,ac5-pinctrl",
+	},
+	{ },
+};
+
+static const struct mvebu_mpp_ctrl ac5_mpp_controls[] = {
+	MPP_FUNC_CTRL(0, 45, NULL, mvebu_regmap_mpp_ctrl), };
+
+static struct pinctrl_gpio_range ac5_mpp_gpio_ranges[] = {
+	MPP_GPIO_RANGE(0,   0,  0, 46), };
+
+static int ac5_pinctrl_probe(struct platform_device *pdev)
+{
+	struct mvebu_pinctrl_soc_info *soc = &ac5_pinctrl_info;
+	const struct of_device_id *match =
+		of_match_device(ac5_pinctrl_of_match, &pdev->dev);
+
+	if (!match || !pdev->dev.parent)
+		return -ENODEV;
+
+	soc->variant = 0; /* no variants for ac5 */
+	soc->controls = ac5_mpp_controls;
+	soc->ncontrols = ARRAY_SIZE(ac5_mpp_controls);
+	soc->gpioranges = ac5_mpp_gpio_ranges;
+	soc->ngpioranges = ARRAY_SIZE(ac5_mpp_gpio_ranges);
+	soc->modes = ac5_mpp_modes;
+	soc->nmodes = ac5_mpp_controls[0].npins;
+
+	pdev->dev.platform_data = soc;
+
+	return mvebu_pinctrl_simple_regmap_probe(pdev, &pdev->dev, 0);
+}
+
+static struct platform_driver ac5_pinctrl_driver = {
+	.driver = {
+		.name = "ac5-pinctrl",
+		.of_match_table = of_match_ptr(ac5_pinctrl_of_match),
+	},
+	.probe = ac5_pinctrl_probe,
+};
+
+builtin_platform_driver(ac5_pinctrl_driver);
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v1 3/4] arm64: dts: marvell: Add Armada 98DX2530 SoC and RD-AC5X board
  2022-03-10  3:00 [PATCH v1 0/4] arm64: mvebu: Support for Marvell 98DX2530 (and variants) Chris Packham
  2022-03-10  3:00 ` [PATCH v1 1/4] dt-bindings: pinctrl: mvebu: Document bindings for AC5 Chris Packham
  2022-03-10  3:00 ` [PATCH v1 2/4] pinctrl: mvebu: pinctrl driver for 98DX2530 SoC Chris Packham
@ 2022-03-10  3:00 ` Chris Packham
  2022-03-10 14:26   ` Andrew Lunn
  2022-03-10  3:00 ` [PATCH v1 4/4] arm64: marvell: enable the 98DX2530 pinctrl driver Chris Packham
  3 siblings, 1 reply; 14+ messages in thread
From: Chris Packham @ 2022-03-10  3:00 UTC (permalink / raw)
  To: linus.walleij, robh+dt, catalin.marinas, will, andrew,
	gregory.clement, sebastian.hesselbarth, kostap, robert.marko
  Cc: linux-gpio, devicetree, linux-kernel, linux-arm-kernel, Chris Packham

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>
---

Notes:
    This has a number of undocumented compatible strings. I've got the SDK
    source so I'll either bring through whatever drivers are needed or look
    at for an in-tree alternative (e.g. there is SDK code for a ac5-gpio but
    the existing marvell,orion-gpio seems to cover what is needed if you use
    an appropriate binding).

 arch/arm64/boot/dts/marvell/Makefile          |   1 +
 .../boot/dts/marvell/armada-98dx2530.dtsi     | 459 ++++++++++++++++++
 arch/arm64/boot/dts/marvell/rd-ac5x.dts       |  27 ++
 3 files changed, 487 insertions(+)
 create mode 100644 arch/arm64/boot/dts/marvell/armada-98dx2530.dtsi
 create mode 100644 arch/arm64/boot/dts/marvell/rd-ac5x.dts

diff --git a/arch/arm64/boot/dts/marvell/Makefile b/arch/arm64/boot/dts/marvell/Makefile
index 1c794cdcb8e6..3905dee558b4 100644
--- a/arch/arm64/boot/dts/marvell/Makefile
+++ b/arch/arm64/boot/dts/marvell/Makefile
@@ -24,3 +24,4 @@ dtb-$(CONFIG_ARCH_MVEBU) += cn9132-db.dtb
 dtb-$(CONFIG_ARCH_MVEBU) += cn9132-db-B.dtb
 dtb-$(CONFIG_ARCH_MVEBU) += cn9130-crb-A.dtb
 dtb-$(CONFIG_ARCH_MVEBU) += cn9130-crb-B.dtb
+dtb-$(CONFIG_ARCH_MVEBU) += rd-ac5x.dtb
diff --git a/arch/arm64/boot/dts/marvell/armada-98dx2530.dtsi b/arch/arm64/boot/dts/marvell/armada-98dx2530.dtsi
new file mode 100644
index 000000000000..fac82474c239
--- /dev/null
+++ b/arch/arm64/boot/dts/marvell/armada-98dx2530.dtsi
@@ -0,0 +1,459 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+/*
+ * Device Tree For AC5.
+ *
+ * Copyright (C) 2021 Marvell
+ *
+ */
+
+/dts-v1/;
+
+#include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/interrupt-controller/arm-gic.h>
+
+/ {
+	model = "Marvell AC5 SoC";
+	compatible = "marvell,ac5", "marvell,armada3700";
+	interrupt-parent = <&gic>;
+	#address-cells = <2>;
+	#size-cells = <2>;
+
+	reserved-memory {
+		#address-cells = <2>;
+		#size-cells = <2>;
+		ranges;
+
+		prestera_rsvd: buffer@200000000 {
+			/*
+			 * to be used as a shared pool of DMA buffers for a set
+			 * of devices
+			 */
+			compatible = "shared-dma-pool";
+			/*
+			 * No one other than devices registered for that mem,
+			 * may use this area
+			 */
+			no-map;
+			/*
+			 * addr (first 2 cells) need to be aligned with actual
+			 * DMA that will be allocated, therefore we choose such
+			 * addr, that will be aligned with many DMA sizes
+			 */
+			reg = <0x2 0x00000000 0x0 0x400000>;
+		};
+	};
+
+	mvDma {
+		compatible = "marvell,mv_dma";
+		memory-region = <&prestera_rsvd>;
+		status = "okay";
+	};
+
+	aliases {
+		serial0 = &uart0;
+		spiflash0 = &spiflash0;
+		gpio0 = &gpio0;
+		gpio1 = &gpio1;
+		ethernet0 = &eth0;
+		ethernet1 = &eth1;
+	};
+
+	psci {
+		compatible = "arm,psci-0.2";
+		method = "smc";
+	};
+
+	timer {
+		compatible = "arm,armv8-timer";
+		interrupts = <GIC_PPI 9 IRQ_TYPE_LEVEL_HIGH>,
+				 <GIC_PPI 8 IRQ_TYPE_LEVEL_HIGH>,
+				 <GIC_PPI 10 IRQ_TYPE_LEVEL_HIGH>,
+				 <GIC_PPI 7 IRQ_TYPE_LEVEL_HIGH>;
+		clock-frequency = <25000000>;
+	};
+
+	pmu {
+		compatible = "arm,armv8-pmuv3";
+		interrupts = <GIC_PPI 12 IRQ_TYPE_LEVEL_HIGH>;
+	};
+
+	soc {
+		compatible = "simple-bus";
+		#address-cells = <2>;
+		#size-cells = <2>;
+		ranges;
+		dma-ranges;
+
+		internal-regs@7f000000 {
+			#address-cells = <1>;
+			#size-cells = <1>;
+			compatible = "simple-bus";
+			/* 16M internal register @ 0x7f00_0000 */
+			ranges = <0x0 0x0 0x7f000000 0x1000000>;
+			dma-coherent;
+
+			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>;
+				clock-frequency = <328000000>;
+				status = "okay";
+			};
+
+			mdio: mdio@20000 {
+				#address-cells = <1>;
+				#size-cells = <0>;
+				compatible = "marvell,orion-mdio";
+				reg = <0x22004 0x4>;
+				clocks = <&core_clock>;
+				phy0: ethernet-phy@0 {
+					reg = < 0 0 >;
+				};
+			};
+
+			i2c0: i2c@11000{
+				compatible = "marvell,mv78230-i2c";
+				reg = <0x11000 0x20>;
+
+				clocks = <&core_clock>;
+				clock-names = "core";
+				interrupts = <GIC_SPI 87 IRQ_TYPE_LEVEL_HIGH>;
+				clock-frequency=<100000>;
+				status="okay";
+
+				pinctrl-names = "default", "gpio";
+				pinctrl-0 = <&i2c0_pins>;
+				pinctrl-1 = <&i2c0_gpio>;
+				scl_gpio = <&gpio0 26 GPIO_ACTIVE_HIGH>;
+				sda_gpio = <&gpio0 27 GPIO_ACTIVE_HIGH>;
+			};
+
+			i2c1: i2c@11100{
+				compatible = "marvell,mv78230-i2c";
+				reg = <0x11100 0x20>;
+
+				clocks = <&core_clock>;
+				clock-names = "core";
+				interrupts = <GIC_SPI 88 IRQ_TYPE_LEVEL_HIGH>;
+				clock-frequency=<100000>;
+				status="okay";
+
+				pinctrl-names = "default", "gpio";
+				pinctrl-0 = <&i2c1_pins>;
+				pinctrl-1 = <&i2c1_gpio>;
+				scl_gpio = <&gpio0 20 GPIO_ACTIVE_HIGH>;
+				sda_gpio = <&gpio0 21 GPIO_ACTIVE_HIGH>;
+			};
+
+			gpio0: gpio@18100 {
+				compatible = "marvell,orion-gpio";
+				reg = <0x18100 0x40>;
+				ngpios = <32>;
+				gpio-controller;
+				#gpio-cells = <2>;
+				gpio-ranges = <&pinctrl0 0 0 32>;
+				marvell,pwm-offset = <0x1f0>;
+			};
+
+			gpio1: gpio@18140 {
+				reg = <0x18140 0x40>;
+				compatible = "marvell,orion-gpio";
+				ngpios = <14>;
+				gpio-controller;
+				#gpio-cells = <2>;
+				gpio-ranges = <&pinctrl0 0 32 14>;
+				marvell,pwm-offset = <0x1f0>;
+			};
+		};
+
+		mmc_dma: mmc-dma-peripherals@80500000 {
+				compatible = "simple-bus";
+				#address-cells = <0x2>;
+				#size-cells = <0x2>;
+				ranges;
+				dma-coherent;
+
+				sdhci0: sdhci@805c0000 {
+					compatible = "marvell,ac5-sdhci", "marvell,armada-ap806-sdhci";
+					reg = <0x0 0x805c0000 0x0 0x300>;
+					reg-names = "ctrl", "decoder";
+					interrupts = <GIC_SPI 92 IRQ_TYPE_LEVEL_HIGH>;
+					clocks = <&core_clock>;
+					clock-names = "core";
+					status = "okay";
+					bus-width = <8>;
+					/*marvell,xenon-phy-slow-mode;*/
+					non-removable;
+					mmc-ddr-1_8v;
+					mmc-hs200-1_8v;
+					mmc-hs400-1_8v;
+				};
+		};
+
+		/*
+		 * Dedicated section for devices behind 32bit controllers so we
+		 * can configure specific DMA mapping for them
+		 */
+		behind-32bit-controller@7f000000 {
+			compatible = "simple-bus";
+			#address-cells = <0x2>;
+			#size-cells = <0x2>;
+			ranges = <0x0 0x0 0x0 0x7f000000 0x0 0x1000000>;
+			/* Host phy ram starts at 0x200M */
+			dma-ranges = <0x0 0x0 0x2 0x0 0x1 0x0>;
+			dma-coherent;
+
+			eth0: ethernet@20000 {
+				compatible = "marvell,armada-ac5-neta";
+				reg = <0x0 0x20000 0x0 0x4000>;
+				interrupts = <GIC_SPI 45 IRQ_TYPE_LEVEL_HIGH>;
+				clocks = <&core_clock>;
+				status = "disabled";
+				phy-mode = "sgmii";
+			};
+
+			eth1: ethernet@24000 {
+				compatible = "marvell,armada-ac5-neta";
+				reg = <0x0 0x24000 0x0 0x4000>;
+				interrupts = <GIC_SPI 55 IRQ_TYPE_LEVEL_HIGH>;
+				clocks = <&core_clock>;
+				status = "disabled";
+				phy-mode = "sgmii";
+				fixed-link {
+					speed = <100>;
+					full-duplex;
+				};
+			};
+
+			/* A dummy entry used for chipidea phy init */
+			usb1phy: usbphy {
+				compatible = "usb-nop-xceiv";
+				#phy-cells = <0>;
+			};
+
+			/* USB0 is a host USB */
+			usb0: usb@80000 {
+				compatible = "marvell,ac5-ehci", "marvell,orion-ehci";
+				reg = <0x0 0x80000 0x0 0x500>;
+				interrupts = <GIC_SPI 67 IRQ_TYPE_LEVEL_HIGH>;
+				status = "okay";
+			};
+
+			/* USB1 is a peripheral USB */
+			usb1: usb@a0000 {
+				reg = <0x0 0xa0000 0x0 0x500>;
+				interrupts = <GIC_SPI 69 IRQ_TYPE_LEVEL_HIGH>;
+				status = "okay";
+			};
+		};
+
+		pinctrl0: pinctrl@80020100 {
+			compatible = "marvell,ac5-pinctrl",
+				     "syscon", "simple-mfd";
+			reg = <0 0x80020100 0 0x20>;
+
+			i2c0_pins: i2c0-pins {
+				marvell,pins = "mpp26", "mpp27";
+				marvell,function = "i2c0";
+			};
+
+			i2c0_gpio: i2c0-gpio-pins {
+				marvell,pins = "mpp26", "mpp27";
+				marvell,function = "gpio";
+			};
+
+			i2c1_pins: i2c1-pins {
+				marvell,pins = "mpp20", "mpp21";
+				marvell,function = "i2c1";
+			};
+
+			i2c1_gpio: i2c1-gpio-pins {
+				marvell,pins = "mpp20", "mpp21";
+				marvell,function = "i2c1";
+			};
+		};
+
+		pcie0: pcie@800a0000 {
+			compatible = "marvell,ac5-pcie", "snps,dw-pcie";
+			reg = <0 0x800a0000 0 0x20000>, <0 0x3fff0000 0 0x10000>;
+			reg-names = "ctrl", "config";
+			#address-cells = <3>;
+			#size-cells = <2>;
+			#interrupt-cells = <1>;
+			device_type = "pci";
+			dma-coherent;
+			bus-range = <0 0xff>;
+			/* ranges for the PCI memory and I/O regions */
+			ranges = <0x82000000 0 0x30000000 0 0x30000000 0 0xfff0000>;
+
+			interrupt-map-mask = <0 0 0 1>;
+			interrupt-map = <0 0 0 1 &gic GIC_SPI 99 IRQ_TYPE_LEVEL_HIGH>;
+
+			interrupts = <GIC_SPI 99 IRQ_TYPE_LEVEL_HIGH>;
+
+			num-lanes = <1>;
+			status = "disabled";
+
+			clocks = <&core_clock>;
+		};
+
+		core_clock: core_clock@0 {
+			compatible = "fixed-clock";
+			#clock-cells = <0>;
+			clock-frequency = <400000000>;
+		};
+
+		axi_clock: axi_clock@0 {
+			compatible = "fixed-clock";
+			#clock-cells = <0>;
+			clock-frequency = <325000000>;
+		};
+
+		spi_clock: spi_clock@0 {
+			compatible = "fixed-clock";
+			#clock-cells = <0>;
+			clock-frequency = <200000000>;
+		};
+
+		spi@805a0000 {
+			compatible = "marvell,armada-3700-spi";
+			reg = <0x0 0x805a0000 0x0 0x50>;
+			#address-cells = <0x1>;
+			#size-cells = <0x0>;
+			clocks = <&spi_clock>;
+			interrupts = <GIC_SPI 90 IRQ_TYPE_LEVEL_HIGH>;
+			num-cs = <1>;
+			status = "okay";
+
+			spiflash0: spi-flash@0 {
+				compatible = "spi-nor";
+				spi-max-frequency = <50000000>;
+				spi-tx-bus-width = <1>; /* 1-single, 2-dual, 4-quad */
+				spi-rx-bus-width = <1>; /* 1-single, 2-dual, 4-quad */
+				reg = <0>;
+
+				#address-cells = <1>;
+				#size-cells = <1>;
+
+				partition@0 {
+					label = "spi_flash_part0";
+					reg = <0x0 0x800000>;
+				};
+
+				parition@1 {
+					label = "spi_flash_part1";
+					reg = <0x800000 0x700000>;
+				};
+
+				parition@2 {
+					label = "spi_flash_part2";
+					reg = <0xF00000 0x100000>;
+				};
+			};
+		};
+
+		spi@805a8000 {
+			compatible = "marvell,armada-3700-spi";
+			reg = <0x0 0x805a8000 0x0 0x50>;
+			#address-cells = <0x1>;
+			#size-cells = <0x0>;
+			clocks = <&spi_clock>;
+			interrupts = <GIC_SPI 91 IRQ_TYPE_LEVEL_HIGH>;
+			num-cs = <1>;
+			status = "disabled";
+		};
+
+		nand: nand@805b0000 {
+			compatible = "marvell,ac5-nand-controller";
+			reg =  <0x0 0x805b0000 0x0 0x00000054
+				0x0 0x840F8204 0x0 0x00000004
+				0x0 0x80013010 0x0 0x00000020>;
+			#address-cells = <0x1>;
+			#size-cells = <0x0>;
+			interrupts = <GIC_SPI 89 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&core_clock>;
+			/*marvell,system-controller = <0x15>*/
+			status = "okay";
+
+			nand@0 {
+				reg = <0x0>;
+				label = "main-storage";
+				nand-rb = <0>;
+				nand-ecc-mode = "hw";
+				nand-ecc-strength = <12>;
+				nand-ecc-step-size = <512>;
+			};
+		};
+
+		prestera {
+			compatible = "marvell,armada-ac5-switch";
+			interrupts = <GIC_SPI 0x23 IRQ_TYPE_LEVEL_HIGH>;
+			status = "okay";
+		};
+
+		watchdog@80216000 {
+			compatible = "marvell,ac5-wd";
+			reg = <0x0 0x80216000 0 0x1000>,
+				  <0x0 0x80215000 0 0x1000>;
+			interrupts = <GIC_SPI 124 IRQ_TYPE_LEVEL_HIGH>;
+			timeout-sec = <30>;
+		};
+
+	};
+
+	gic: interrupt-controller@80600000 {
+		compatible = "arm,gic-v3";
+		#interrupt-cells = <3>;
+		interrupt-controller;
+		/*#redistributor-regions = <1>;*/
+		redistributor-stride = <0x0 0x20000>;	// 128kB stride
+		reg = <0x0 0x80600000 0x0 0x10000>, /* GICD */
+			  <0x0 0x80660000 0x0 0x40000>; /* GICR */
+		interrupts = <GIC_PPI 6 IRQ_TYPE_LEVEL_HIGH>;
+	};
+
+	cpus {
+		#address-cells = <2>;
+		#size-cells = <0>;
+
+		cpu-map {
+			cluster0 {
+				core0 {
+					cpu = <&CPU0>;
+				};
+				core1 {
+					cpu = <&CPU1>;
+				};
+			};
+		};
+
+		CPU0:cpu@0 {
+			device_type = "cpu";
+			compatible = "arm,armv8";
+			reg = <0x0 0x0>;
+			enable-method = "psci";
+			next-level-cache = <&L2_0>;
+		};
+
+		CPU1:cpu@1 {
+			device_type = "cpu";
+			compatible = "arm,armv8";
+			reg = <0x0 0x100>;
+			enable-method = "psci";
+			next-level-cache = <&L2_0>;
+		};
+
+		L2_0: l2-cache0 {
+			compatible = "cache";
+		};
+	};
+
+	memory@0 {
+		device_type = "memory";
+		reg = <0x2 0x00000000 0x0 0x40000000>;
+		// linux,usable-memory = <0x2 0x00000000 0x0 0x80000000>;
+	};
+
+};
diff --git a/arch/arm64/boot/dts/marvell/rd-ac5x.dts b/arch/arm64/boot/dts/marvell/rd-ac5x.dts
new file mode 100644
index 000000000000..5e3c1a05acaa
--- /dev/null
+++ b/arch/arm64/boot/dts/marvell/rd-ac5x.dts
@@ -0,0 +1,27 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+/*
+ * Device Tree For AC5X.
+ *
+ * Copyright (C) 2021 Marvell
+ *
+ */
+/*
+ * Device Tree file for Marvell Alleycat 5X development board
+ * This board file supports the B configuration of the board
+ */
+
+#include "armada-98dx2530.dtsi"
+
+
+&eth0 {
+	status = "okay";
+	phy = <&phy0>;
+};
+
+&usb1 {
+	compatible = "chipidea,usb2";
+	phys = <&usb1phy>;
+	phy-names = "usb-phy";
+	dr_mode = "peripheral";
+};
+
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v1 4/4] arm64: marvell: enable the 98DX2530 pinctrl driver
  2022-03-10  3:00 [PATCH v1 0/4] arm64: mvebu: Support for Marvell 98DX2530 (and variants) Chris Packham
                   ` (2 preceding siblings ...)
  2022-03-10  3:00 ` [PATCH v1 3/4] arm64: dts: marvell: Add Armada 98DX2530 SoC and RD-AC5X board Chris Packham
@ 2022-03-10  3:00 ` Chris Packham
  3 siblings, 0 replies; 14+ messages in thread
From: Chris Packham @ 2022-03-10  3:00 UTC (permalink / raw)
  To: linus.walleij, robh+dt, catalin.marinas, will, andrew,
	gregory.clement, sebastian.hesselbarth, kostap, robert.marko
  Cc: linux-gpio, devicetree, linux-kernel, linux-arm-kernel, Chris Packham

This commit makes sure the drivers for the 98DX2530 pin controller is
enabled.

Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
---
 arch/arm64/Kconfig.platforms | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm64/Kconfig.platforms b/arch/arm64/Kconfig.platforms
index 21697449d762..6bbb56901794 100644
--- a/arch/arm64/Kconfig.platforms
+++ b/arch/arm64/Kconfig.platforms
@@ -183,11 +183,13 @@ config ARCH_MVEBU
 	select PINCTRL_ARMADA_37XX
 	select PINCTRL_ARMADA_AP806
 	select PINCTRL_ARMADA_CP110
+	select PINCTRL_AC5
 	help
 	  This enables support for Marvell EBU familly, including:
 	   - Armada 3700 SoC Family
 	   - Armada 7K SoC Family
 	   - Armada 8K SoC Family
+	   - 98DX2530 SoC Family
 
 config ARCH_MXC
 	bool "ARMv8 based NXP i.MX SoC family"
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH v1 2/4] pinctrl: mvebu: pinctrl driver for 98DX2530 SoC
  2022-03-10  3:00 ` [PATCH v1 2/4] pinctrl: mvebu: pinctrl driver for 98DX2530 SoC Chris Packham
@ 2022-03-10 13:59   ` Andrew Lunn
  2022-03-10 21:51     ` Chris Packham
  0 siblings, 1 reply; 14+ messages in thread
From: Andrew Lunn @ 2022-03-10 13:59 UTC (permalink / raw)
  To: Chris Packham
  Cc: linus.walleij, robh+dt, catalin.marinas, will, gregory.clement,
	sebastian.hesselbarth, kostap, robert.marko, linux-gpio,
	devicetree, linux-kernel, linux-arm-kernel

On Thu, Mar 10, 2022 at 04:00:37PM +1300, Chris Packham wrote:
> This pinctrl driver supports the 98DX25xx and 98DX35xx family of chips
> from Marvell. It is based on the Marvell SDK with additions for various
> (non-gpio) pin configurations based on the datasheet.
> 
> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>

Hi Chris

Past experience with pinctrl and gpio for mvebu is that developers get
GPI and GPO pins wrong. Are there any pins which are not GPIO but only
a subset, so only GPI or GPO?

  Andrew

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v1 3/4] arm64: dts: marvell: Add Armada 98DX2530 SoC and RD-AC5X board
  2022-03-10  3:00 ` [PATCH v1 3/4] arm64: dts: marvell: Add Armada 98DX2530 SoC and RD-AC5X board Chris Packham
@ 2022-03-10 14:26   ` Andrew Lunn
  2022-03-10 22:32     ` Chris Packham
  0 siblings, 1 reply; 14+ messages in thread
From: Andrew Lunn @ 2022-03-10 14:26 UTC (permalink / raw)
  To: Chris Packham
  Cc: linus.walleij, robh+dt, catalin.marinas, will, gregory.clement,
	sebastian.hesselbarth, kostap, robert.marko, linux-gpio,
	devicetree, linux-kernel, linux-arm-kernel

On Thu, Mar 10, 2022 at 04:00:38PM +1300, Chris Packham wrote:
> 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>
> ---
> 
> Notes:
>     This has a number of undocumented compatible strings. I've got the SDK
>     source so I'll either bring through whatever drivers are needed or look
>     at for an in-tree alternative (e.g. there is SDK code for a ac5-gpio but
>     the existing marvell,orion-gpio seems to cover what is needed if you use
>     an appropriate binding).

Hi Chris

My understand is, you don't add nodes for which there is no
driver. The driver and its binding needs to be reviewed and accepted
before users of it are added.

> +	mvDma {
> +		compatible = "marvell,mv_dma";
> +		memory-region = <&prestera_rsvd>;
> +		status = "okay";
> +	};

Is this different to the existing Marvell XOR engine?

> +			mdio: mdio@20000 {
> +				#address-cells = <1>;
> +				#size-cells = <0>;
> +				compatible = "marvell,orion-mdio";
> +				reg = <0x22004 0x4>;
> +				clocks = <&core_clock>;
> +				phy0: ethernet-phy@0 {
> +					reg = < 0 0 >;
> +				};

This embedded PHY looks wrong. reg should be a single value.

Is the PHY internal? Generally, the PHY is put in the .dts file, but
if it is internal, that the .dtsi would be correct.

> +				sdhci0: sdhci@805c0000 {
> +					compatible = "marvell,ac5-sdhci", "marvell,armada-ap806-sdhci";

This additional compatible should be added to the existing binding
document.

> +			eth0: ethernet@20000 {
> +				compatible = "marvell,armada-ac5-neta";

So it is not compatible with plain nata?

> +				reg = <0x0 0x20000 0x0 0x4000>;
> +				interrupts = <GIC_SPI 45 IRQ_TYPE_LEVEL_HIGH>;
> +				clocks = <&core_clock>;
> +				status = "disabled";
> +				phy-mode = "sgmii";
> +			};
> +
> +			eth1: ethernet@24000 {
> +				compatible = "marvell,armada-ac5-neta";
> +				reg = <0x0 0x24000 0x0 0x4000>;
> +				interrupts = <GIC_SPI 55 IRQ_TYPE_LEVEL_HIGH>;
> +				clocks = <&core_clock>;
> +				status = "disabled";
> +				phy-mode = "sgmii";
> +				fixed-link {
> +					speed = <100>;
> +					full-duplex;
> +				};

Fast Ethernet? Yet SGMII?

> +			/* USB0 is a host USB */
> +			usb0: usb@80000 {
> +				compatible = "marvell,ac5-ehci", "marvell,orion-ehci";

Please add this compatible to the binding.

> +		pcie0: pcie@800a0000 {
> +			compatible = "marvell,ac5-pcie", "snps,dw-pcie";

Please add this ...

> +			spiflash0: spi-flash@0 {
> +				compatible = "spi-nor";
> +				spi-max-frequency = <50000000>;
> +				spi-tx-bus-width = <1>; /* 1-single, 2-dual, 4-quad */
> +				spi-rx-bus-width = <1>; /* 1-single, 2-dual, 4-quad */
> +				reg = <0>;
> +
> +				#address-cells = <1>;
> +				#size-cells = <1>;
> +
> +				partition@0 {
> +					label = "spi_flash_part0";
> +					reg = <0x0 0x800000>;
> +				};
> +
> +				parition@1 {
> +					label = "spi_flash_part1";
> +					reg = <0x800000 0x700000>;
> +				};
> +
> +				parition@2 {
> +					label = "spi_flash_part2";
> +					reg = <0xF00000 0x100000>;
> +				};

The partitioning of the flash i would expect to be board specific, so
belongs on the .dts file.

> +		nand: nand@805b0000 {
> +			compatible = "marvell,ac5-nand-controller";

The current NAND driver does not work?

> +		prestera {
> +			compatible = "marvell,armada-ac5-switch";
> +			interrupts = <GIC_SPI 0x23 IRQ_TYPE_LEVEL_HIGH>;
> +			status = "okay";
> +		};

Here we have to be careful with naming. I assume Marvell in kernel
Pretera driver does not actually work on the prestera hardware? That
driver assumes you are running Marvell firmware on this SoC, and have
a host running that driver which talks to the SoC firmware?

The name perstera seems O.K, and the compatible
marvell,armada-ac5-switch makes it clear the prestera driver cannot be
used. However, until we do actually have a driver, i don't think this
node should be added.

> +		watchdog@80216000 {
> +			compatible = "marvell,ac5-wd";

Not compatible with the existing watchdog driver?

    Andrew

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v1 2/4] pinctrl: mvebu: pinctrl driver for 98DX2530 SoC
  2022-03-10 13:59   ` Andrew Lunn
@ 2022-03-10 21:51     ` Chris Packham
  0 siblings, 0 replies; 14+ messages in thread
From: Chris Packham @ 2022-03-10 21:51 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: linus.walleij, robh+dt, catalin.marinas, will, gregory.clement,
	sebastian.hesselbarth, kostap, robert.marko, linux-gpio,
	devicetree, linux-kernel, linux-arm-kernel


On 11/03/22 02:59, Andrew Lunn wrote:
> On Thu, Mar 10, 2022 at 04:00:37PM +1300, Chris Packham wrote:
>> This pinctrl driver supports the 98DX25xx and 98DX35xx family of chips
>> from Marvell. It is based on the Marvell SDK with additions for various
>> (non-gpio) pin configurations based on the datasheet.
>>
>> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
> Hi Chris
>
> Past experience with pinctrl and gpio for mvebu is that developers get
> GPI and GPO pins wrong. Are there any pins which are not GPIO but only
> a subset, so only GPI or GPO?
According to the datasheet they are all GPIO. Some are sampled at reset 
so have notes about being careful not to drive them differently during 
reset.
>    Andrew

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v1 3/4] arm64: dts: marvell: Add Armada 98DX2530 SoC and RD-AC5X board
  2022-03-10 14:26   ` Andrew Lunn
@ 2022-03-10 22:32     ` Chris Packham
  2022-03-10 23:25       ` Andrew Lunn
  0 siblings, 1 reply; 14+ messages in thread
From: Chris Packham @ 2022-03-10 22:32 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: linus.walleij, robh+dt, catalin.marinas, will, gregory.clement,
	sebastian.hesselbarth, kostap, robert.marko, linux-gpio,
	devicetree, linux-kernel, linux-arm-kernel


On 11/03/22 03:26, Andrew Lunn wrote:
> On Thu, Mar 10, 2022 at 04:00:38PM +1300, Chris Packham wrote:
>> 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>
>> ---
>>
>> Notes:
>>      This has a number of undocumented compatible strings. I've got the SDK
>>      source so I'll either bring through whatever drivers are needed or look
>>      at for an in-tree alternative (e.g. there is SDK code for a ac5-gpio but
>>      the existing marvell,orion-gpio seems to cover what is needed if you use
>>      an appropriate binding).
> Hi Chris
>
> My understand is, you don't add nodes for which there is no
> driver. The driver and its binding needs to be reviewed and accepted
> before users of it are added.

I thought that might be the case. I'll be limited in what I can test on 
the reference board I have. I'll work to bring in whatever bindings and 
drivers I can test and probably remove anything that I can't.

I might be able to do another round of patches when we get our boards.

>
>> +	mvDma {
>> +		compatible = "marvell,mv_dma";
>> +		memory-region = <&prestera_rsvd>;
>> +		status = "okay";
>> +	};
> Is this different to the existing Marvell XOR engine?

Yes it has something to do with the DMA memory for the integrated L3 
switch. Because that driver doesn't exist I'll probably remove this node 
(and the other prestera node below) in v2.


>> +			mdio: mdio@20000 {
>> +				#address-cells = <1>;
>> +				#size-cells = <0>;
>> +				compatible = "marvell,orion-mdio";
>> +				reg = <0x22004 0x4>;
>> +				clocks = <&core_clock>;
>> +				phy0: ethernet-phy@0 {
>> +					reg = < 0 0 >;
>> +				};
> This embedded PHY looks wrong. reg should be a single value.
>
> Is the PHY internal? Generally, the PHY is put in the .dts file, but
> if it is internal, that the .dtsi would be correct.

Looks like an external 88E1512 PHY so I'll move that to the board dts.

>
>> +				sdhci0: sdhci@805c0000 {
>> +					compatible = "marvell,ac5-sdhci", "marvell,armada-ap806-sdhci";
> This additional compatible should be added to the existing binding
> document.
I'll see what differences there are with the ap806-sdhci. I might be 
able to remove it.
>
>> +			eth0: ethernet@20000 {
>> +				compatible = "marvell,armada-ac5-neta";
> So it is not compatible with plain nata?

There is some odd muxing setup where the serdes are either SGMII or PCIe 
or can even be connected to the internal switch. Whether the Ethernet 
driver needs to care about it I'm not sure.

>
>> +				reg = <0x0 0x20000 0x0 0x4000>;
>> +				interrupts = <GIC_SPI 45 IRQ_TYPE_LEVEL_HIGH>;
>> +				clocks = <&core_clock>;
>> +				status = "disabled";
>> +				phy-mode = "sgmii";
>> +			};
>> +
>> +			eth1: ethernet@24000 {
>> +				compatible = "marvell,armada-ac5-neta";
>> +				reg = <0x0 0x24000 0x0 0x4000>;
>> +				interrupts = <GIC_SPI 55 IRQ_TYPE_LEVEL_HIGH>;
>> +				clocks = <&core_clock>;
>> +				status = "disabled";
>> +				phy-mode = "sgmii";
>> +				fixed-link {
>> +					speed = <100>;
>> +					full-duplex;
>> +				};
> Fast Ethernet? Yet SGMII?

I think the reference code might be trying to connect this to the 
internal switch. I'll remove the fixed-link portion.

>> +			/* USB0 is a host USB */
>> +			usb0: usb@80000 {
>> +				compatible = "marvell,ac5-ehci", "marvell,orion-ehci";
> Please add this compatible to the binding.
Will do (or delete).
>
>> +		pcie0: pcie@800a0000 {
>> +			compatible = "marvell,ac5-pcie", "snps,dw-pcie";
> Please add this ...
Will do (or delete).
>
>> +			spiflash0: spi-flash@0 {
>> +				compatible = "spi-nor";
>> +				spi-max-frequency = <50000000>;
>> +				spi-tx-bus-width = <1>; /* 1-single, 2-dual, 4-quad */
>> +				spi-rx-bus-width = <1>; /* 1-single, 2-dual, 4-quad */
>> +				reg = <0>;
>> +
>> +				#address-cells = <1>;
>> +				#size-cells = <1>;
>> +
>> +				partition@0 {
>> +					label = "spi_flash_part0";
>> +					reg = <0x0 0x800000>;
>> +				};
>> +
>> +				parition@1 {
>> +					label = "spi_flash_part1";
>> +					reg = <0x800000 0x700000>;
>> +				};
>> +
>> +				parition@2 {
>> +					label = "spi_flash_part2";
>> +					reg = <0xF00000 0x100000>;
>> +				};
> The partitioning of the flash i would expect to be board specific, so
> belongs on the .dts file.
Will move.
>> +		nand: nand@805b0000 {
>> +			compatible = "marvell,ac5-nand-controller";
> The current NAND driver does not work?

This is one of the things I can't test on the board I have (uses eMMC 
instead of NAND). Should I put "marvell,armada-8k-nand-controller" in as 
a placeholder or leave the node out entirely?

>
>> +		prestera {
>> +			compatible = "marvell,armada-ac5-switch";
>> +			interrupts = <GIC_SPI 0x23 IRQ_TYPE_LEVEL_HIGH>;
>> +			status = "okay";
>> +		};
> Here we have to be careful with naming. I assume Marvell in kernel
> Pretera driver does not actually work on the prestera hardware? That
> driver assumes you are running Marvell firmware on this SoC, and have
> a host running that driver which talks to the SoC firmware?
>
> The name perstera seems O.K, and the compatible
> marvell,armada-ac5-switch makes it clear the prestera driver cannot be
> used. However, until we do actually have a driver, i don't think this
> node should be added.

On other SoCs I did put in specific prestera compatibles with 
documentation. I've even got out of tree code that uses those 
compatibles although Marvell's SDK hasn't caught up and is still using 
of_find_note_by_path("/soc/prestera").

>> +		watchdog@80216000 {
>> +			compatible = "marvell,ac5-wd";
> Not compatible with the existing watchdog driver?
Will check and either add binding or use a different compatible.
>
>      Andrew

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v1 1/4] dt-bindings: pinctrl: mvebu: Document bindings for AC5
  2022-03-10  3:00 ` [PATCH v1 1/4] dt-bindings: pinctrl: mvebu: Document bindings for AC5 Chris Packham
@ 2022-03-10 23:25   ` Rob Herring
  2022-03-10 23:53     ` Chris Packham
  0 siblings, 1 reply; 14+ messages in thread
From: Rob Herring @ 2022-03-10 23:25 UTC (permalink / raw)
  To: Chris Packham
  Cc: linus.walleij, catalin.marinas, will, andrew, gregory.clement,
	sebastian.hesselbarth, kostap, robert.marko, linux-gpio,
	devicetree, linux-kernel, linux-arm-kernel

On Thu, Mar 10, 2022 at 04:00:36PM +1300, Chris Packham wrote:
> Add JSON schema for marvell,ac5-pinctrl present on the Marvell 98DX2530
> SoC.
> 
> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
> ---
>  .../bindings/pinctrl/marvell,ac5-pinctrl.yaml | 73 +++++++++++++++++++
>  1 file changed, 73 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/pinctrl/marvell,ac5-pinctrl.yaml
> 
> diff --git a/Documentation/devicetree/bindings/pinctrl/marvell,ac5-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/marvell,ac5-pinctrl.yaml
> new file mode 100644
> index 000000000000..c7ab3d0e8420
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pinctrl/marvell,ac5-pinctrl.yaml
> @@ -0,0 +1,73 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/pinctrl/marvell,ac5-pinctrl.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Marvell AC5 pin controller
> +
> +maintainers:
> +  - Chris Packham <chris.packham@alliedtelesis.co.nz>
> +
> +description:
> +  Bindings for Marvell's AC5 memory-mapped pin controller.
> +
> +properties:
> +  compatible:
> +    items:
> +      - const: marvell,ac5-pinctrl
> +      - const: syscon
> +      - const: simple-mfd

How is this a 'syscon' or 'simple-mfd' For syscon, what other 
functions/registers does it have? For simple-mfd, what other functions? 
You haven't defined them in the schema.

blank line needed here.

> +  reg:
> +    maxItems: 1
> +
> +patternProperties:
> +  '-pins$':
> +    type: object
> +    $ref: pinmux-node.yaml#
> +
> +    properties:
> +      marvell,function:
> +        $ref: "/schemas/types.yaml#/definitions/string"
> +        description:
> +          Indicates the function to select.
> +        enum: [ gpio, i2c0, i2c1, nand, sdio, spi0, spi1, uart0, uart1, uart2, uart3 ]
> +
> +      marvell,pins:
> +        $ref: /schemas/types.yaml#/definitions/string-array
> +        description:
> +          Array of MPP pins to be used for the given function.
> +        minItems: 1
> +        items:
> +          enum: [ mpp0, mpp1, mpp2, mpp3, mpp4, mpp5, mpp6, mpp7, mpp8, mpp9,
> +                  mpp10, mpp11, mpp12, mpp13, mpp14, mpp15, mpp16, mpp17, mpp18, mpp19,
> +                  mpp20, mpp21, mpp22, mpp23, mpp24, mpp25, mpp26, mpp27, mpp28, mpp29,
> +                  mpp30, mpp31, mpp32, mpp33, mpp34, mpp35, mpp36, mpp37, mpp38, mpp39,
> +                  mpp40, mpp41, mpp42, mpp43, mpp44, mpp45 ]
> +
> +allOf:
> +  - $ref: "pinctrl.yaml#"
> +
> +required:
> +  - compatible
> +  - reg
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    pinctrl@80020100 {
> +      compatible = "marvell,ac5-pinctrl",
> +      "syscon", "simple-mfd";
> +      reg = <0x80020100 0x20>;
> +
> +      i2c0_pins: i2c0-pins {
> +        marvell,pins = "mpp26", "mpp27";
> +        marvell,function = "i2c0";
> +      };
> +
> +      i2c0_gpio: i2c0-gpio-pins {
> +        marvell,pins = "mpp26", "mpp27";
> +        marvell,function = "gpio";
> +      };
> +    };
> -- 
> 2.35.1
> 
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v1 3/4] arm64: dts: marvell: Add Armada 98DX2530 SoC and RD-AC5X board
  2022-03-10 22:32     ` Chris Packham
@ 2022-03-10 23:25       ` Andrew Lunn
  0 siblings, 0 replies; 14+ messages in thread
From: Andrew Lunn @ 2022-03-10 23:25 UTC (permalink / raw)
  To: Chris Packham
  Cc: linus.walleij, robh+dt, catalin.marinas, will, gregory.clement,
	sebastian.hesselbarth, kostap, robert.marko, linux-gpio,
	devicetree, linux-kernel, linux-arm-kernel

> >> +	mvDma {
> >> +		compatible = "marvell,mv_dma";
> >> +		memory-region = <&prestera_rsvd>;
> >> +		status = "okay";
> >> +	};
> > Is this different to the existing Marvell XOR engine?
> 
> Yes it has something to do with the DMA memory for the integrated L3 
> switch. Because that driver doesn't exist I'll probably remove this node 
> (and the other prestera node below) in v2.

The compatible itself is not so great, since it made me think of the
DMA engine in the SoC. So maybe when the driver is added, it could be
something like prestera_dma?

> >> +				sdhci0: sdhci@805c0000 {
> >> +					compatible = "marvell,ac5-sdhci", "marvell,armada-ap806-sdhci";
> > This additional compatible should be added to the existing binding
> > document.
> I'll see what differences there are with the ap806-sdhci. I might be 
> able to remove it.

It is actually good to have the additional compatible. You might not
spot a difference now, but sometime in the future you do find a
difference which you need to work around in the driver. Having the
compatible in place means you can just change the driver and existing
DT blobs, burnt into flash etc, don't need to change.

> >
> >> +			eth0: ethernet@20000 {
> >> +				compatible = "marvell,armada-ac5-neta";
> > So it is not compatible with plain nata?
> 
> There is some odd muxing setup where the serdes are either SGMII or PCIe 
> or can even be connected to the internal switch. Whether the Ethernet 
> driver needs to care about it I'm not sure.

Russell King probably knows more about this, but it sounds more like a
comphy issues, not neta. The comphy connects the MAC to a SERDES, and
that SERDES can be SGMII, PCIe or USB.

> >> +		nand: nand@805b0000 {
> >> +			compatible = "marvell,ac5-nand-controller";
> > The current NAND driver does not work?
> 
> This is one of the things I can't test on the board I have (uses eMMC 
> instead of NAND). Should I put "marvell,armada-8k-nand-controller" in as 
> a placeholder or leave the node out entirely?

I would leave it out if you cannot test it.

  Andrew

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v1 1/4] dt-bindings: pinctrl: mvebu: Document bindings for AC5
  2022-03-10 23:25   ` Rob Herring
@ 2022-03-10 23:53     ` Chris Packham
  2022-03-11  3:25       ` Chris Packham
  0 siblings, 1 reply; 14+ messages in thread
From: Chris Packham @ 2022-03-10 23:53 UTC (permalink / raw)
  To: Rob Herring
  Cc: linus.walleij, catalin.marinas, will, andrew, gregory.clement,
	sebastian.hesselbarth, kostap, robert.marko, linux-gpio,
	devicetree, linux-kernel, linux-arm-kernel

Hi Rob,

On 11/03/22 12:25, Rob Herring wrote:
> On Thu, Mar 10, 2022 at 04:00:36PM +1300, Chris Packham wrote:
>> Add JSON schema for marvell,ac5-pinctrl present on the Marvell 98DX2530
>> SoC.
>>
>> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
>> ---
>>   .../bindings/pinctrl/marvell,ac5-pinctrl.yaml | 73 +++++++++++++++++++
>>   1 file changed, 73 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/pinctrl/marvell,ac5-pinctrl.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/pinctrl/marvell,ac5-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/marvell,ac5-pinctrl.yaml
>> new file mode 100644
>> index 000000000000..c7ab3d0e8420
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/pinctrl/marvell,ac5-pinctrl.yaml
>> @@ -0,0 +1,73 @@
>> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
>> +%YAML 1.2
>> +---
>> +$id: http://scanmail.trustwave.com/?c=20988&d=5Iiq4o6Dmr7xiwkJNrr25TNxYxK78lIUEWeJ-yq0UA&u=http%3a%2f%2fdevicetree%2eorg%2fschemas%2fpinctrl%2fmarvell%2cac5-pinctrl%2eyaml%23
>> +$schema: http://scanmail.trustwave.com/?c=20988&d=5Iiq4o6Dmr7xiwkJNrr25TNxYxK78lIUEWPe_X2yXg&u=http%3a%2f%2fdevicetree%2eorg%2fmeta-schemas%2fcore%2eyaml%23
>> +
>> +title: Marvell AC5 pin controller
>> +
>> +maintainers:
>> +  - Chris Packham <chris.packham@alliedtelesis.co.nz>
>> +
>> +description:
>> +  Bindings for Marvell's AC5 memory-mapped pin controller.
>> +
>> +properties:
>> +  compatible:
>> +    items:
>> +      - const: marvell,ac5-pinctrl
>> +      - const: syscon
>> +      - const: simple-mfd
> How is this a 'syscon' or 'simple-mfd' For syscon, what other
> functions/registers does it have? For simple-mfd, what other functions?
> You haven't defined them in the schema.

It's in the vendor dts I have and it's likely there because 
marvell,armada-37xx-pinctrl.txt has the same syscon and simple-mfd 
compatibles. Looking at how this is used I think it's more like the 
older mvebu pinctrl devices. I'll remove these two compatibles and 
update the dtsi file.

>
> blank line needed here.
>
>> +  reg:
>> +    maxItems: 1
>> +
>> +patternProperties:
>> +  '-pins$':
>> +    type: object
>> +    $ref: pinmux-node.yaml#
>> +
>> +    properties:
>> +      marvell,function:
>> +        $ref: "/schemas/types.yaml#/definitions/string"
>> +        description:
>> +          Indicates the function to select.
>> +        enum: [ gpio, i2c0, i2c1, nand, sdio, spi0, spi1, uart0, uart1, uart2, uart3 ]
>> +
>> +      marvell,pins:
>> +        $ref: /schemas/types.yaml#/definitions/string-array
>> +        description:
>> +          Array of MPP pins to be used for the given function.
>> +        minItems: 1
>> +        items:
>> +          enum: [ mpp0, mpp1, mpp2, mpp3, mpp4, mpp5, mpp6, mpp7, mpp8, mpp9,
>> +                  mpp10, mpp11, mpp12, mpp13, mpp14, mpp15, mpp16, mpp17, mpp18, mpp19,
>> +                  mpp20, mpp21, mpp22, mpp23, mpp24, mpp25, mpp26, mpp27, mpp28, mpp29,
>> +                  mpp30, mpp31, mpp32, mpp33, mpp34, mpp35, mpp36, mpp37, mpp38, mpp39,
>> +                  mpp40, mpp41, mpp42, mpp43, mpp44, mpp45 ]
>> +
>> +allOf:
>> +  - $ref: "pinctrl.yaml#"
>> +
>> +required:
>> +  - compatible
>> +  - reg
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> +  - |
>> +    pinctrl@80020100 {
>> +      compatible = "marvell,ac5-pinctrl",
>> +      "syscon", "simple-mfd";
>> +      reg = <0x80020100 0x20>;
>> +
>> +      i2c0_pins: i2c0-pins {
>> +        marvell,pins = "mpp26", "mpp27";
>> +        marvell,function = "i2c0";
>> +      };
>> +
>> +      i2c0_gpio: i2c0-gpio-pins {
>> +        marvell,pins = "mpp26", "mpp27";
>> +        marvell,function = "gpio";
>> +      };
>> +    };
>> -- 
>> 2.35.1
>>
>>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v1 1/4] dt-bindings: pinctrl: mvebu: Document bindings for AC5
  2022-03-10 23:53     ` Chris Packham
@ 2022-03-11  3:25       ` Chris Packham
  2022-03-11  3:59         ` Chris Packham
  0 siblings, 1 reply; 14+ messages in thread
From: Chris Packham @ 2022-03-11  3:25 UTC (permalink / raw)
  To: Rob Herring
  Cc: linus.walleij, catalin.marinas, will, andrew, gregory.clement,
	sebastian.hesselbarth, kostap, robert.marko, linux-gpio,
	devicetree, linux-kernel, linux-arm-kernel


On 11/03/22 12:53, Chris Packham wrote:
> Hi Rob,
>
> On 11/03/22 12:25, Rob Herring wrote:
>> On Thu, Mar 10, 2022 at 04:00:36PM +1300, Chris Packham wrote:
>>> Add JSON schema for marvell,ac5-pinctrl present on the Marvell 98DX2530
>>> SoC.
>>>
>>> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
>>> ---
>>>   .../bindings/pinctrl/marvell,ac5-pinctrl.yaml | 73 
>>> +++++++++++++++++++
>>>   1 file changed, 73 insertions(+)
>>>   create mode 100644 
>>> Documentation/devicetree/bindings/pinctrl/marvell,ac5-pinctrl.yaml
>>>
>>> diff --git 
>>> a/Documentation/devicetree/bindings/pinctrl/marvell,ac5-pinctrl.yaml 
>>> b/Documentation/devicetree/bindings/pinctrl/marvell,ac5-pinctrl.yaml
>>> new file mode 100644
>>> index 000000000000..c7ab3d0e8420
>>> --- /dev/null
>>> +++ 
>>> b/Documentation/devicetree/bindings/pinctrl/marvell,ac5-pinctrl.yaml
>>> @@ -0,0 +1,73 @@
>>> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
>>> +%YAML 1.2
>>> +---
>>> +$id: 
>>> http://scanmail.trustwave.com/?c=20988&d=5Iiq4o6Dmr7xiwkJNrr25TNxYxK78lIUEWeJ-yq0UA&u=http%3a%2f%2fdevicetree%2eorg%2fschemas%2fpinctrl%2fmarvell%2cac5-pinctrl%2eyaml%23
>>> +$schema: 
>>> http://scanmail.trustwave.com/?c=20988&d=5Iiq4o6Dmr7xiwkJNrr25TNxYxK78lIUEWPe_X2yXg&u=http%3a%2f%2fdevicetree%2eorg%2fmeta-schemas%2fcore%2eyaml%23
>>> +
>>> +title: Marvell AC5 pin controller
>>> +
>>> +maintainers:
>>> +  - Chris Packham <chris.packham@alliedtelesis.co.nz>
>>> +
>>> +description:
>>> +  Bindings for Marvell's AC5 memory-mapped pin controller.
>>> +
>>> +properties:
>>> +  compatible:
>>> +    items:
>>> +      - const: marvell,ac5-pinctrl
>>> +      - const: syscon
>>> +      - const: simple-mfd
>> How is this a 'syscon' or 'simple-mfd' For syscon, what other
>> functions/registers does it have? For simple-mfd, what other functions?
>> You haven't defined them in the schema.
>
> It's in the vendor dts I have and it's likely there because 
> marvell,armada-37xx-pinctrl.txt has the same syscon and simple-mfd 
> compatibles. Looking at how this is used I think it's more like the 
> older mvebu pinctrl devices. I'll remove these two compatibles and 
> update the dtsi file.
Actually turns out I do need the syscon compatible because the driver 
calls mvebu_pinctrl_simple_regmap_probe() which relies on the syscon 
compatible. I'm still not sure what to put in the binding doc other than 
"syscon" is needed because it is needed.
>
>>
>> blank line needed here.
>>
>>> +  reg:
>>> +    maxItems: 1
>>> +
>>> +patternProperties:
>>> +  '-pins$':
>>> +    type: object
>>> +    $ref: pinmux-node.yaml#
>>> +
>>> +    properties:
>>> +      marvell,function:
>>> +        $ref: "/schemas/types.yaml#/definitions/string"
>>> +        description:
>>> +          Indicates the function to select.
>>> +        enum: [ gpio, i2c0, i2c1, nand, sdio, spi0, spi1, uart0, 
>>> uart1, uart2, uart3 ]
>>> +
>>> +      marvell,pins:
>>> +        $ref: /schemas/types.yaml#/definitions/string-array
>>> +        description:
>>> +          Array of MPP pins to be used for the given function.
>>> +        minItems: 1
>>> +        items:
>>> +          enum: [ mpp0, mpp1, mpp2, mpp3, mpp4, mpp5, mpp6, mpp7, 
>>> mpp8, mpp9,
>>> +                  mpp10, mpp11, mpp12, mpp13, mpp14, mpp15, mpp16, 
>>> mpp17, mpp18, mpp19,
>>> +                  mpp20, mpp21, mpp22, mpp23, mpp24, mpp25, mpp26, 
>>> mpp27, mpp28, mpp29,
>>> +                  mpp30, mpp31, mpp32, mpp33, mpp34, mpp35, mpp36, 
>>> mpp37, mpp38, mpp39,
>>> +                  mpp40, mpp41, mpp42, mpp43, mpp44, mpp45 ]
>>> +
>>> +allOf:
>>> +  - $ref: "pinctrl.yaml#"
>>> +
>>> +required:
>>> +  - compatible
>>> +  - reg
>>> +
>>> +additionalProperties: false
>>> +
>>> +examples:
>>> +  - |
>>> +    pinctrl@80020100 {
>>> +      compatible = "marvell,ac5-pinctrl",
>>> +      "syscon", "simple-mfd";
>>> +      reg = <0x80020100 0x20>;
>>> +
>>> +      i2c0_pins: i2c0-pins {
>>> +        marvell,pins = "mpp26", "mpp27";
>>> +        marvell,function = "i2c0";
>>> +      };
>>> +
>>> +      i2c0_gpio: i2c0-gpio-pins {
>>> +        marvell,pins = "mpp26", "mpp27";
>>> +        marvell,function = "gpio";
>>> +      };
>>> +    };
>>> -- 
>>> 2.35.1
>>>
>>>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v1 1/4] dt-bindings: pinctrl: mvebu: Document bindings for AC5
  2022-03-11  3:25       ` Chris Packham
@ 2022-03-11  3:59         ` Chris Packham
  0 siblings, 0 replies; 14+ messages in thread
From: Chris Packham @ 2022-03-11  3:59 UTC (permalink / raw)
  To: Rob Herring
  Cc: linus.walleij, catalin.marinas, will, andrew, gregory.clement,
	sebastian.hesselbarth, kostap, robert.marko, linux-gpio,
	devicetree, linux-kernel, linux-arm-kernel


On 11/03/22 16:25, Chris Packham wrote:
>
> On 11/03/22 12:53, Chris Packham wrote:
>> Hi Rob,
>>
>> On 11/03/22 12:25, Rob Herring wrote:
>>> On Thu, Mar 10, 2022 at 04:00:36PM +1300, Chris Packham wrote:
>>>> Add JSON schema for marvell,ac5-pinctrl present on the Marvell 
>>>> 98DX2530
>>>> SoC.
>>>>
>>>> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
>>>> ---
>>>>   .../bindings/pinctrl/marvell,ac5-pinctrl.yaml | 73 
>>>> +++++++++++++++++++
>>>>   1 file changed, 73 insertions(+)
>>>>   create mode 100644 
>>>> Documentation/devicetree/bindings/pinctrl/marvell,ac5-pinctrl.yaml
>>>>
>>>> diff --git 
>>>> a/Documentation/devicetree/bindings/pinctrl/marvell,ac5-pinctrl.yaml 
>>>> b/Documentation/devicetree/bindings/pinctrl/marvell,ac5-pinctrl.yaml
>>>> new file mode 100644
>>>> index 000000000000..c7ab3d0e8420
>>>> --- /dev/null
>>>> +++ 
>>>> b/Documentation/devicetree/bindings/pinctrl/marvell,ac5-pinctrl.yaml
>>>> @@ -0,0 +1,73 @@
>>>> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
>>>> +%YAML 1.2
>>>> +---
>>>> +$id: 
>>>> http://scanmail.trustwave.com/?c=20988&d=5Iiq4o6Dmr7xiwkJNrr25TNxYxK78lIUEWeJ-yq0UA&u=http%3a%2f%2fdevicetree%2eorg%2fschemas%2fpinctrl%2fmarvell%2cac5-pinctrl%2eyaml%23
>>>> +$schema: 
>>>> http://scanmail.trustwave.com/?c=20988&d=5Iiq4o6Dmr7xiwkJNrr25TNxYxK78lIUEWPe_X2yXg&u=http%3a%2f%2fdevicetree%2eorg%2fmeta-schemas%2fcore%2eyaml%23
>>>> +
>>>> +title: Marvell AC5 pin controller
>>>> +
>>>> +maintainers:
>>>> +  - Chris Packham <chris.packham@alliedtelesis.co.nz>
>>>> +
>>>> +description:
>>>> +  Bindings for Marvell's AC5 memory-mapped pin controller.
>>>> +
>>>> +properties:
>>>> +  compatible:
>>>> +    items:
>>>> +      - const: marvell,ac5-pinctrl
>>>> +      - const: syscon
>>>> +      - const: simple-mfd
>>> How is this a 'syscon' or 'simple-mfd' For syscon, what other
>>> functions/registers does it have? For simple-mfd, what other functions?
>>> You haven't defined them in the schema.
>>
>> It's in the vendor dts I have and it's likely there because 
>> marvell,armada-37xx-pinctrl.txt has the same syscon and simple-mfd 
>> compatibles. Looking at how this is used I think it's more like the 
>> older mvebu pinctrl devices. I'll remove these two compatibles and 
>> update the dtsi file.
> Actually turns out I do need the syscon compatible because the driver 
> calls mvebu_pinctrl_simple_regmap_probe() which relies on the syscon 
> compatible. I'm still not sure what to put in the binding doc other 
> than "syscon" is needed because it is needed.

I think I get it now. I need a syscon node to be the parent of my 
pinctrl node (and potentially others). That would also explain some 
things I was confused about with the gpio controllers. It's just that 
the vendor dts I've started with is woefully out of date w.r.t current 
best practice.

>>
>>>
>>> blank line needed here.
>>>
>>>> +  reg:
>>>> +    maxItems: 1
>>>> +
>>>> +patternProperties:
>>>> +  '-pins$':
>>>> +    type: object
>>>> +    $ref: pinmux-node.yaml#
>>>> +
>>>> +    properties:
>>>> +      marvell,function:
>>>> +        $ref: "/schemas/types.yaml#/definitions/string"
>>>> +        description:
>>>> +          Indicates the function to select.
>>>> +        enum: [ gpio, i2c0, i2c1, nand, sdio, spi0, spi1, uart0, 
>>>> uart1, uart2, uart3 ]
>>>> +
>>>> +      marvell,pins:
>>>> +        $ref: /schemas/types.yaml#/definitions/string-array
>>>> +        description:
>>>> +          Array of MPP pins to be used for the given function.
>>>> +        minItems: 1
>>>> +        items:
>>>> +          enum: [ mpp0, mpp1, mpp2, mpp3, mpp4, mpp5, mpp6, mpp7, 
>>>> mpp8, mpp9,
>>>> +                  mpp10, mpp11, mpp12, mpp13, mpp14, mpp15, mpp16, 
>>>> mpp17, mpp18, mpp19,
>>>> +                  mpp20, mpp21, mpp22, mpp23, mpp24, mpp25, mpp26, 
>>>> mpp27, mpp28, mpp29,
>>>> +                  mpp30, mpp31, mpp32, mpp33, mpp34, mpp35, mpp36, 
>>>> mpp37, mpp38, mpp39,
>>>> +                  mpp40, mpp41, mpp42, mpp43, mpp44, mpp45 ]
>>>> +
>>>> +allOf:
>>>> +  - $ref: "pinctrl.yaml#"
>>>> +
>>>> +required:
>>>> +  - compatible
>>>> +  - reg
>>>> +
>>>> +additionalProperties: false
>>>> +
>>>> +examples:
>>>> +  - |
>>>> +    pinctrl@80020100 {
>>>> +      compatible = "marvell,ac5-pinctrl",
>>>> +      "syscon", "simple-mfd";
>>>> +      reg = <0x80020100 0x20>;
>>>> +
>>>> +      i2c0_pins: i2c0-pins {
>>>> +        marvell,pins = "mpp26", "mpp27";
>>>> +        marvell,function = "i2c0";
>>>> +      };
>>>> +
>>>> +      i2c0_gpio: i2c0-gpio-pins {
>>>> +        marvell,pins = "mpp26", "mpp27";
>>>> +        marvell,function = "gpio";
>>>> +      };
>>>> +    };
>>>> -- 
>>>> 2.35.1
>>>>
>>>>

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2022-03-11  4:00 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-10  3:00 [PATCH v1 0/4] arm64: mvebu: Support for Marvell 98DX2530 (and variants) Chris Packham
2022-03-10  3:00 ` [PATCH v1 1/4] dt-bindings: pinctrl: mvebu: Document bindings for AC5 Chris Packham
2022-03-10 23:25   ` Rob Herring
2022-03-10 23:53     ` Chris Packham
2022-03-11  3:25       ` Chris Packham
2022-03-11  3:59         ` Chris Packham
2022-03-10  3:00 ` [PATCH v1 2/4] pinctrl: mvebu: pinctrl driver for 98DX2530 SoC Chris Packham
2022-03-10 13:59   ` Andrew Lunn
2022-03-10 21:51     ` Chris Packham
2022-03-10  3:00 ` [PATCH v1 3/4] arm64: dts: marvell: Add Armada 98DX2530 SoC and RD-AC5X board Chris Packham
2022-03-10 14:26   ` Andrew Lunn
2022-03-10 22:32     ` Chris Packham
2022-03-10 23:25       ` Andrew Lunn
2022-03-10  3:00 ` [PATCH v1 4/4] arm64: marvell: enable the 98DX2530 pinctrl driver Chris Packham

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).