[PATCH v3 00/11] Support Pensando Elba SoC

[PATCH v3 00/11] Support Pensando Elba SoC

[Brad Larson]

This series enables support for Pensando Elba SoC based platforms.
The Elba SoC has the following features:

- Sixteen ARM64 A72 cores
- Dual DDR 4/5 memory controllers
- 32 lanes of PCIe Gen3/4 to the Host
- Network interfaces: Dual 200GE, Quad 100GE, 50GE, 25GE, 10GE and
  also a single 1GE management port.
- Storage/crypto offloads and 144 programmable P4 cores.
- QSPI and EMMC for SoC storage
- Two SPI interfaces for peripheral management
- I2C bus for platform management

This is a respin based on review input.  Summary of the changes are:

0001-gpio-Add-Elba-SoC-gpio-driver-for-spi-cs-control.patch
- This patch is deleted.  Elba SOC specific gpio spics control is
  integrated into spi-dw-mmio.c.

0002-spi-cadence-quadspi-Add-QSPI-support-for-Pensando-El.patch
- Changed compatible to "pensando,elba-qspi" to be more descriptive
  in spi-cadence-quadspi.c.

- Arnd wondered if moving to DT properties for quirks may be the
  way to go.  Feedback I've received on other patches was don't
  mix two efforts in one patch so I'm currently just adding the
  Elba support to the current design.

0003-spi-dw-Add-support-for-Pensando-Elba-SoC-SPI.patch
- Changed the implementation to use existing dw_spi_set_cs() and
  integrated Elba specific CS control into spi-dw-mmio.c.  The
  native designware support is for two chip-selects while Elba
  provides 4 chip-selects.  Instead of adding a new file for
  this support in gpio-elba-spics.c the support is in one
  file (spi-dw-mmio.c).

0004-spidev-Add-Pensando-CPLD-compatible.patch
- This patch is deleted.  The addition of compatible "pensando,cpld"
  to spidev.c is not added and an existing compatible is used 
  in the device tree to enable.

0005-mmc-sdhci-cadence-Add-Pensando-Elba-SoC-support.patch
- Ulf and Yamada-san agreed the amount of code for this support
  is not enough to need a new file.  The support is added into
  sdhci-cadence.c and new files sdhci-cadence-elba.c and
  sdhci-cadence.h are deleted.
- Redundant defines are removed (e.g. use SDHCI_CDNS_HRS04 and
  remove SDIO_REG_HRS4).
- Removed phy init function sd4_set_dlyvr() and used existing
  sdhci_cdns_phy_init(). Init values are from DT properties.
- Replace  devm_ioremap_resource(&pdev->dev, iomem)
     with  devm_platform_ioremap_resource(pdev, 1)
- Refactored the elba priv_writ_l() and elba_write_l() to
  remove a little redundant code.
- The config option CONFIG_MMC_SDHCI_CADENCE_ELBA goes away.
- Only C syntax and Elba functions are prefixed with elba_

0006-arm64-Add-config-for-Pensando-SoC-platforms.patch
- Added a little more info to the platform help text to assist
  users to decide on including platform support or not.

0007-arm64-dts-Add-Pensando-Elba-SoC-support.patch
- Node names changed to DT generic names
- Changed from using 'spi@' which is reserved
- The elba-flash-parts.dtsi is kept separate as
  it is included in multiple dts files.
- SPDX license tags at the top of each file
- The compatible = "pensando,elba" and 'model' are
  now together in the board file.
- UIO nodes removed
- Ordered nodes by increasing unit address
- Removed an unreferenced container node.
- Dropped deprecated 'device_type' for uart0 node.

0010-dt-bindings-spi-cadence-qspi-Add-support-for-Pensand.patch
- Updated since the latest documentation has been converted to yaml

0011-dt-bindings-gpio-Add-Pensando-Elba-SoC-support.patch
- This patch is deleted since the Elba gpio spics is added to
  the spi dw driver and documented there.

Because of the deletion of patches and merging of code
the new patchset is not similar.  A changelog is added into
the patches for merged code to be helpful on the history.


Brad Larson (11):
  dt-bindings: arm: pensando: add Pensando boards
  dt-bindings: Add vendor prefix for Pensando Systems
  dt-bindings: mmc: Add Pensando Elba SoC binding
  dt-bindings: spi: Add compatible for Pensando Elba SoC
  spi: dw: Add Pensando Elba SoC SPI Controller bindings
  MAINTAINERS: Add entry for PENSANDO
  arm64: Add config for Pensando SoC platforms
  spi: cadence-quadspi: Add compatible for Pensando Elba SoC
  mmc: sdhci-cadence: Add Pensando Elba SoC support
  spi: dw: Add support for Pensando Elba SoC
  arm64: dts: Add Pensando Elba SoC support

 .../bindings/arm/pensando,elba.yaml           |  20 ++
 .../devicetree/bindings/mmc/cdns,sdhci.yaml   |  13 +-
 .../bindings/spi/cdns,qspi-nor.yaml           |   3 +-
 .../bindings/spi/snps,dw-apb-ssi.yaml         |   2 +
 .../devicetree/bindings/vendor-prefixes.yaml  |   2 +
 MAINTAINERS                                   |   7 +
 arch/arm64/Kconfig.platforms                  |  12 ++
 arch/arm64/boot/dts/Makefile                  |   1 +
 arch/arm64/boot/dts/pensando/Makefile         |   6 +
 arch/arm64/boot/dts/pensando/elba-16core.dtsi | 192 ++++++++++++++++++
 .../boot/dts/pensando/elba-asic-common.dtsi   |  96 +++++++++
 arch/arm64/boot/dts/pensando/elba-asic.dts    |  23 +++
 .../boot/dts/pensando/elba-flash-parts.dtsi   | 103 ++++++++++
 arch/arm64/boot/dts/pensando/elba.dtsi        | 181 +++++++++++++++++
 drivers/mmc/host/Kconfig                      |   1 +
 drivers/mmc/host/sdhci-cadence.c              | 148 ++++++++++++--
 drivers/spi/spi-cadence-quadspi.c             |  19 ++
 drivers/spi/spi-dw-mmio.c                     |  85 ++++++++
 18 files changed, 894 insertions(+), 20 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/arm/pensando,elba.yaml
 create mode 100644 arch/arm64/boot/dts/pensando/Makefile
 create mode 100644 arch/arm64/boot/dts/pensando/elba-16core.dtsi
 create mode 100644 arch/arm64/boot/dts/pensando/elba-asic-common.dtsi
 create mode 100644 arch/arm64/boot/dts/pensando/elba-asic.dts
 create mode 100644 arch/arm64/boot/dts/pensando/elba-flash-parts.dtsi
 create mode 100644 arch/arm64/boot/dts/pensando/elba.dtsi

-- 
2.17.1

[PATCH v3 01/11] dt-bindings: arm: pensando: add Pensando boards

[Brad Larson]

Document the compatible for Pensando Elba SoC boards.

Signed-off-by: Brad Larson <brad@pensando.io>
---
 .../bindings/arm/pensando,elba.yaml           | 20 +++++++++++++++++++
 1 file changed, 20 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/arm/pensando,elba.yaml

diff --git a/Documentation/devicetree/bindings/arm/pensando,elba.yaml b/Documentation/devicetree/bindings/arm/pensando,elba.yaml
new file mode 100644
index 000000000000..84bd9e7e98e9
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/pensando,elba.yaml
@@ -0,0 +1,20 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/arm/pensando,elba.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Pensando Elba SoC Platforms Device Tree Bindings
+
+maintainers:
+  - Brad Larson  <brad@pensando.io>
+
+properties:
+  $nodename:
+    const: "/"
+  compatible:
+    const: pensando,elba
+
+additionalProperties: true
+
+...
-- 
2.17.1

[PATCH v3 02/11] dt-bindings: Add vendor prefix for Pensando Systems

[Brad Larson]

Add vendor prefix for Pensando Systems: https://pensando.io

Signed-off-by: Brad Larson <brad@pensando.io>
---
 Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml
index a867f7102c35..4d3d29490a12 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
+++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
@@ -880,6 +880,8 @@ patternProperties:
     description: Parallax Inc.
   "^pda,.*":
     description: Precision Design Associates, Inc.
+  "^pensando,.*":
+    description: Pensando Systems Inc.
   "^pericom,.*":
     description: Pericom Technology Inc.
   "^pervasive,.*":
-- 
2.17.1

[PATCH v3 03/11] dt-bindings: mmc: Add Pensando Elba SoC binding

[Brad Larson]

Pensando Elba ARM 64-bit SoC is integrated with this IP and
explicitly controls byte-lane enables resulting in an additional
reg property resource.

Signed-off-by: Brad Larson <brad@pensando.io>
---
 .../devicetree/bindings/mmc/cdns,sdhci.yaml         | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/Documentation/devicetree/bindings/mmc/cdns,sdhci.yaml b/Documentation/devicetree/bindings/mmc/cdns,sdhci.yaml
index af7442f73881..6c68b7b5abec 100644
--- a/Documentation/devicetree/bindings/mmc/cdns,sdhci.yaml
+++ b/Documentation/devicetree/bindings/mmc/cdns,sdhci.yaml
@@ -15,13 +15,16 @@ allOf:
 
 properties:
   compatible:
-    items:
-      - enum:
-          - socionext,uniphier-sd4hc
-      - const: cdns,sd4hc
+    oneOf:
+      - items:
+        - enum:
+            - socionext,uniphier-sd4hc
+            - pensando,elba-emmc
+        - const: cdns,sd4hc
 
   reg:
-    maxItems: 1
+    minItems: 1
+    maxItems: 2
 
   interrupts:
     maxItems: 1
-- 
2.17.1

[PATCH v3 04/11] dt-bindings: spi: Add compatible for Pensando Elba SoC

[Brad Larson]

Document the cadence qspi controller compatible for Pensando Elba SoC
boards.  The Elba qspi fifo size is 1024.

Signed-off-by: Brad Larson <brad@pensando.io>
---
 Documentation/devicetree/bindings/spi/cdns,qspi-nor.yaml | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/spi/cdns,qspi-nor.yaml b/Documentation/devicetree/bindings/spi/cdns,qspi-nor.yaml
index 0e7087cc8bf9..d4413eced17a 100644
--- a/Documentation/devicetree/bindings/spi/cdns,qspi-nor.yaml
+++ b/Documentation/devicetree/bindings/spi/cdns,qspi-nor.yaml
@@ -20,6 +20,7 @@ properties:
               - ti,k2g-qspi
               - ti,am654-ospi
               - intel,lgm-qspi
+              - pensando,elba-qspi
           - const: cdns,qspi-nor
       - const: cdns,qspi-nor
 
@@ -38,7 +39,7 @@ properties:
     description:
       Size of the data FIFO in words.
     $ref: "/schemas/types.yaml#/definitions/uint32"
-    enum: [ 128, 256 ]
+    enum: [ 128, 256, 1024 ]
     default: 128
 
   cdns,fifo-width:
-- 
2.17.1

[PATCH v3 05/11] spi: dw: Add Pensando Elba SoC SPI Controller bindings

[Brad Larson]

The Pensando Elba SoC has integrated the DW APB SPI Controller

Signed-off-by: Brad Larson <brad@pensando.io>
---
 Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
index d7e08b03e204..0b5ebb2ae6e7 100644
--- a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
+++ b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
@@ -73,6 +73,8 @@ properties:
               - renesas,r9a06g032-spi # RZ/N1D
               - renesas,r9a06g033-spi # RZ/N1S
           - const: renesas,rzn1-spi   # RZ/N1
+      - description: Pensando Elba SoC SPI Controller
+        const: pensando,elba-spi
 
   reg:
     minItems: 1
-- 
2.17.1

[PATCH v3 06/11] MAINTAINERS: Add entry for PENSANDO

[Brad Larson]

Add entry for PENSANDO maintainer and files

Signed-off-by: Brad Larson <brad@pensando.io>
---
 MAINTAINERS | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 8d118d7957d2..465771d697b1 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2364,6 +2364,13 @@ S:	Maintained
 W:	http://hackndev.com
 F:	arch/arm/mach-pxa/palmz72.*
 
+ARM/PENSANDO ARM64 ARCHITECTURE
+M:	Brad Larson <brad@pensando.io>
+L:	linux-arm-kernel@lists.infradead.org (moderated for non-subscribers)
+S:	Maintained
+F:	Documentation/devicetree/bindings/*/pensando*
+F:	arch/arm64/boot/dts/pensando/
+
 ARM/PLEB SUPPORT
 M:	Peter Chubb <pleb@gelato.unsw.edu.au>
 S:	Maintained
-- 
2.17.1

[PATCH v3 07/11] arm64: Add config for Pensando SoC platforms

[Brad Larson]

Add ARCH_PENSANDO configuration option for Pensando SoC
based platforms.

Signed-off-by: Brad Larson <brad@pensando.io>
---
 arch/arm64/Kconfig.platforms | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/arch/arm64/Kconfig.platforms b/arch/arm64/Kconfig.platforms
index b0ce18d4cc98..456404c6e898 100644
--- a/arch/arm64/Kconfig.platforms
+++ b/arch/arm64/Kconfig.platforms
@@ -209,6 +209,18 @@ config ARCH_MXC
 	  This enables support for the ARMv8 based SoCs in the
 	  NXP i.MX family.
 
+config ARCH_PENSANDO
+	bool "Pensando Platforms"
+	help
+	  This enables support for the ARMv8 based Pensando SoC
+	  family to include the Elba SoC.
+
+	  Pensando SoCs support a range of Distributed Services
+	  Cards in PCIe format installed into servers.  The Elba
+	  SoC includes 16 A-72 CPU cores, 144 programmable P4
+	  cores for a minimal latency/jitter datapath, and network
+	  interfaces up to 100 Gb/s.
+
 config ARCH_QCOM
 	bool "Qualcomm Platforms"
 	select GPIOLIB
-- 
2.17.1

[PATCH v3 08/11] spi: cadence-quadspi: Add compatible for Pensando Elba SoC

[Brad Larson]

The Pensando Elba SoC has the Cadence QSPI controller integrated.

The quirk CQSPI_NEEDS_APB_AHB_HAZARD_WAR is added and if enabled
a dummy readback from the controller is performed to ensure
synchronization.

Signed-off-by: Brad Larson <brad@pensando.io>
---
 drivers/spi/spi-cadence-quadspi.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/drivers/spi/spi-cadence-quadspi.c b/drivers/spi/spi-cadence-quadspi.c
index 101cc71bffa7..af36514250d2 100644
--- a/drivers/spi/spi-cadence-quadspi.c
+++ b/drivers/spi/spi-cadence-quadspi.c
@@ -35,6 +35,7 @@
 /* Quirks */
 #define CQSPI_NEEDS_WR_DELAY		BIT(0)
 #define CQSPI_DISABLE_DAC_MODE		BIT(1)
+#define CQSPI_NEEDS_APB_AHB_HAZARD_WAR	BIT(2)
 
 /* Capabilities */
 #define CQSPI_SUPPORTS_OCTAL		BIT(0)
@@ -74,6 +75,7 @@ struct cqspi_st {
 	int			current_cs;
 	unsigned long		master_ref_clk_hz;
 	bool			is_decoded_cs;
+	bool			apb_ahb_hazard;
 	u32			fifo_depth;
 	u32			fifo_width;
 	u32			num_chipselect;
@@ -862,6 +864,13 @@ static int cqspi_indirect_write_execute(struct cqspi_flash_pdata *f_pdata,
 	if (cqspi->wr_delay)
 		ndelay(cqspi->wr_delay);
 
+	/*
+	 * If a hazard exists between the APB and AHB interfaces, perform a
+	 * dummy readback from the controller to ensure synchronization.
+	 */
+	if (cqspi->apb_ahb_hazard)
+		(void)readl(reg_base + CQSPI_REG_INDIRECTWR);
+
 	while (remaining > 0) {
 		size_t write_words, mod_bytes;
 
@@ -1548,6 +1557,8 @@ static int cqspi_probe(struct platform_device *pdev)
 			master->mode_bits |= SPI_RX_OCTAL | SPI_TX_OCTAL;
 		if (!(ddata->quirks & CQSPI_DISABLE_DAC_MODE))
 			cqspi->use_direct_mode = true;
+		if (ddata->quirks & CQSPI_NEEDS_APB_AHB_HAZARD_WAR)
+			cqspi->apb_ahb_hazard = true;
 	}
 
 	ret = devm_request_irq(dev, irq, cqspi_irq_handler, 0,
@@ -1656,6 +1667,10 @@ static const struct cqspi_driver_platdata intel_lgm_qspi = {
 	.quirks = CQSPI_DISABLE_DAC_MODE,
 };
 
+static const struct cqspi_driver_platdata pen_cdns_qspi = {
+	.quirks = CQSPI_NEEDS_APB_AHB_HAZARD_WAR | CQSPI_DISABLE_DAC_MODE,
+};
+
 static const struct of_device_id cqspi_dt_ids[] = {
 	{
 		.compatible = "cdns,qspi-nor",
@@ -1673,6 +1688,10 @@ static const struct of_device_id cqspi_dt_ids[] = {
 		.compatible = "intel,lgm-qspi",
 		.data = &intel_lgm_qspi,
 	},
+	{
+		.compatible = "pensando,elba-qspi",
+		.data = &pen_cdns_qspi,
+	},
 	{ /* end of table */ }
 };
 
-- 
2.17.1

[PATCH v3 09/11] mmc: sdhci-cadence: Add Pensando Elba SoC support

[Brad Larson]

Add support for Pensando Elba SoC which explicitly controls
byte-lane enables on writes.  Add priv_write_l() which is
used on Elba platforms for byte-lane control.

Select MMC_SDHCI_IO_ACCESSORS for MMC_SDHCI_CADENCE which
allows Elba SoC sdhci_elba_ops to overwrite the SDHCI
IO memory accessors.

Signed-off-by: Brad Larson <brad@pensando.io>
---
Changelog:
- Ulf and Yamada-san agreed the amount of code for this support
  is not enough to need a new file.  The support is added into
  sdhci-cadence.c and new files sdhci-cadence-elba.c and
  sdhci-cadence.h are deleted.
- Redundant defines are removed (e.g. use SDHCI_CDNS_HRS04 and
  remove SDIO_REG_HRS4).
- Removed phy init function sd4_set_dlyvr() and used existing
  sdhci_cdns_phy_init(). Init values are from DT properties.
- Replace  devm_ioremap_resource(&pdev->dev, iomem)
  with     devm_platform_ioremap_resource(pdev, 1)
- Refactored the elba priv_writ_l() and elba_write_l() to
  remove a little redundant code.
- The config option CONFIG_MMC_SDHCI_CADENCE_ELBA goes away.
- Only C syntax and Elba functions are prefixed with elba_

 drivers/mmc/host/Kconfig         |   1 +
 drivers/mmc/host/sdhci-cadence.c | 148 ++++++++++++++++++++++++++++---
 2 files changed, 135 insertions(+), 14 deletions(-)

diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
index 95b3511b0560..7aa8adf8069a 100644
--- a/drivers/mmc/host/Kconfig
+++ b/drivers/mmc/host/Kconfig
@@ -243,6 +243,7 @@ config MMC_SDHCI_CADENCE
 	tristate "SDHCI support for the Cadence SD/SDIO/eMMC controller"
 	depends on MMC_SDHCI_PLTFM
 	depends on OF
+	select MMC_SDHCI_IO_ACCESSORS
 	help
 	  This selects the Cadence SD/SDIO/eMMC driver.
 
diff --git a/drivers/mmc/host/sdhci-cadence.c b/drivers/mmc/host/sdhci-cadence.c
index 6f2de54a5987..de553926dcfa 100644
--- a/drivers/mmc/host/sdhci-cadence.c
+++ b/drivers/mmc/host/sdhci-cadence.c
@@ -66,7 +66,11 @@ struct sdhci_cdns_phy_param {
 
 struct sdhci_cdns_priv {
 	void __iomem *hrs_addr;
+	void __iomem *ctl_addr;	/* write control */
+	spinlock_t wrlock;	/* write lock */
 	bool enhanced_strobe;
+	void (*priv_write_l)(struct sdhci_cdns_priv *priv, u32 val,
+			     void __iomem *reg);
 	unsigned int nr_phy_params;
 	struct sdhci_cdns_phy_param phy_params[];
 };
@@ -76,6 +80,11 @@ struct sdhci_cdns_phy_cfg {
 	u8 addr;
 };
 
+struct sdhci_cdns_drv_data {
+	int (*init)(struct platform_device *pdev);
+	const struct sdhci_pltfm_data pltfm_data;
+};
+
 static const struct sdhci_cdns_phy_cfg sdhci_cdns_phy_cfgs[] = {
 	{ "cdns,phy-input-delay-sd-highspeed", SDHCI_CDNS_PHY_DLY_SD_HS, },
 	{ "cdns,phy-input-delay-legacy", SDHCI_CDNS_PHY_DLY_SD_DEFAULT, },
@@ -90,6 +99,15 @@ static const struct sdhci_cdns_phy_cfg sdhci_cdns_phy_cfgs[] = {
 	{ "cdns,phy-dll-delay-strobe", SDHCI_CDNS_PHY_DLY_STROBE, },
 };
 
+static inline void sdhci_cdns_priv_writel(struct sdhci_cdns_priv *priv,
+					  u32 val, void __iomem *reg)
+{
+	if (unlikely(priv->priv_write_l))
+		priv->priv_write_l(priv, val, reg);
+	else
+		writel(val, reg);
+}
+
 static int sdhci_cdns_write_phy_reg(struct sdhci_cdns_priv *priv,
 				    u8 addr, u8 data)
 {
@@ -104,17 +122,17 @@ static int sdhci_cdns_write_phy_reg(struct sdhci_cdns_priv *priv,
 
 	tmp = FIELD_PREP(SDHCI_CDNS_HRS04_WDATA, data) |
 	      FIELD_PREP(SDHCI_CDNS_HRS04_ADDR, addr);
-	writel(tmp, reg);
+	sdhci_cdns_priv_writel(priv, tmp, reg);
 
 	tmp |= SDHCI_CDNS_HRS04_WR;
-	writel(tmp, reg);
+	sdhci_cdns_priv_writel(priv, tmp, reg);
 
 	ret = readl_poll_timeout(reg, tmp, tmp & SDHCI_CDNS_HRS04_ACK, 0, 10);
 	if (ret)
 		return ret;
 
 	tmp &= ~SDHCI_CDNS_HRS04_WR;
-	writel(tmp, reg);
+	sdhci_cdns_priv_writel(priv, tmp, reg);
 
 	ret = readl_poll_timeout(reg, tmp, !(tmp & SDHCI_CDNS_HRS04_ACK),
 				 0, 10);
@@ -191,7 +209,7 @@ static void sdhci_cdns_set_emmc_mode(struct sdhci_cdns_priv *priv, u32 mode)
 	tmp = readl(priv->hrs_addr + SDHCI_CDNS_HRS06);
 	tmp &= ~SDHCI_CDNS_HRS06_MODE;
 	tmp |= FIELD_PREP(SDHCI_CDNS_HRS06_MODE, mode);
-	writel(tmp, priv->hrs_addr + SDHCI_CDNS_HRS06);
+	sdhci_cdns_priv_writel(priv, tmp, priv->hrs_addr + SDHCI_CDNS_HRS06);
 }
 
 static u32 sdhci_cdns_get_emmc_mode(struct sdhci_cdns_priv *priv)
@@ -223,7 +241,7 @@ static int sdhci_cdns_set_tune_val(struct sdhci_host *host, unsigned int val)
 	 */
 	for (i = 0; i < 2; i++) {
 		tmp |= SDHCI_CDNS_HRS06_TUNE_UP;
-		writel(tmp, reg);
+		sdhci_cdns_priv_writel(priv, tmp, reg);
 
 		ret = readl_poll_timeout(reg, tmp,
 					 !(tmp & SDHCI_CDNS_HRS06_TUNE_UP),
@@ -309,6 +327,88 @@ static void sdhci_cdns_set_uhs_signaling(struct sdhci_host *host,
 		sdhci_set_uhs_signaling(host, timing);
 }
 
+/*
+ * The Pensando Elba SoC explicitly controls byte-lane enables on writes
+ * which includes writes to the HRS registers.
+ */
+static void elba_priv_write_l(struct sdhci_cdns_priv *priv, u32 val,
+			      void __iomem *reg)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&priv->wrlock, flags);
+	writel(0x78, priv->ctl_addr);
+	writel(val, reg);
+	spin_unlock_irqrestore(&priv->wrlock, flags);
+}
+
+static void elba_write_l(struct sdhci_host *host, u32 val, int reg)
+{
+	elba_priv_write_l(sdhci_cdns_priv(host), val, host->ioaddr + reg);
+}
+
+static void elba_write_w(struct sdhci_host *host, u16 val, int reg)
+{
+	struct sdhci_cdns_priv *priv = sdhci_cdns_priv(host);
+	unsigned long flags;
+	u32 m = (reg & 0x3);
+	u32 msk = (0x3 << (m));
+
+	spin_lock_irqsave(&priv->wrlock, flags);
+	writel(msk << 3, priv->ctl_addr);
+	writew(val, host->ioaddr + reg);
+	spin_unlock_irqrestore(&priv->wrlock, flags);
+}
+
+static void elba_write_b(struct sdhci_host *host, u8 val, int reg)
+{
+	struct sdhci_cdns_priv *priv = sdhci_cdns_priv(host);
+	unsigned long flags;
+	u32 m = (reg & 0x3);
+	u32 msk = (0x1 << (m));
+
+	spin_lock_irqsave(&priv->wrlock, flags);
+	writel(msk << 3, priv->ctl_addr);
+	writeb(val, host->ioaddr + reg);
+	spin_unlock_irqrestore(&priv->wrlock, flags);
+}
+
+static const struct sdhci_ops sdhci_elba_ops = {
+	.write_l = elba_write_l,
+	.write_w = elba_write_w,
+	.write_b = elba_write_b,
+	.set_clock = sdhci_set_clock,
+	.get_timeout_clock = sdhci_cdns_get_timeout_clock,
+	.set_bus_width = sdhci_set_bus_width,
+	.reset = sdhci_reset,
+	.set_uhs_signaling = sdhci_cdns_set_uhs_signaling,
+};
+
+static int elba_drv_init(struct platform_device *pdev)
+{
+	struct sdhci_host *host = platform_get_drvdata(pdev);
+	struct sdhci_cdns_priv *priv = sdhci_cdns_priv(host);
+	struct resource *iomem;
+	void __iomem *ioaddr;
+
+	host->mmc->caps |= (MMC_CAP_1_8V_DDR | MMC_CAP_8_BIT_DATA);
+
+	iomem = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+	if (!iomem)
+		return -ENOMEM;
+
+	ioaddr = devm_platform_ioremap_resource(pdev, 1);
+	if (IS_ERR(ioaddr))
+		return PTR_ERR(ioaddr);
+
+	priv->ctl_addr = ioaddr;
+	priv->priv_write_l = elba_priv_write_l;
+	spin_lock_init(&priv->wrlock);
+	writel(0x78, priv->ctl_addr);
+
+	return 0;
+}
+
 static const struct sdhci_ops sdhci_cdns_ops = {
 	.set_clock = sdhci_set_clock,
 	.get_timeout_clock = sdhci_cdns_get_timeout_clock,
@@ -318,13 +418,24 @@ static const struct sdhci_ops sdhci_cdns_ops = {
 	.set_uhs_signaling = sdhci_cdns_set_uhs_signaling,
 };
 
-static const struct sdhci_pltfm_data sdhci_cdns_uniphier_pltfm_data = {
-	.ops = &sdhci_cdns_ops,
-	.quirks2 = SDHCI_QUIRK2_PRESET_VALUE_BROKEN,
+static const struct sdhci_cdns_drv_data sdhci_cdns_uniphier_drv_data = {
+	.pltfm_data = {
+		.ops = &sdhci_cdns_ops,
+		.quirks2 = SDHCI_QUIRK2_PRESET_VALUE_BROKEN,
+	},
+};
+
+static const struct sdhci_cdns_drv_data sdhci_elba_drv_data = {
+	.init = elba_drv_init,
+	.pltfm_data = {
+		.ops = &sdhci_elba_ops,
+	},
 };
 
-static const struct sdhci_pltfm_data sdhci_cdns_pltfm_data = {
-	.ops = &sdhci_cdns_ops,
+static const struct sdhci_cdns_drv_data sdhci_cdns_drv_data = {
+	.pltfm_data = {
+		.ops = &sdhci_cdns_ops,
+	},
 };
 
 static void sdhci_cdns_hs400_enhanced_strobe(struct mmc_host *mmc,
@@ -350,7 +461,7 @@ static void sdhci_cdns_hs400_enhanced_strobe(struct mmc_host *mmc,
 static int sdhci_cdns_probe(struct platform_device *pdev)
 {
 	struct sdhci_host *host;
-	const struct sdhci_pltfm_data *data;
+	const struct sdhci_cdns_drv_data *data;
 	struct sdhci_pltfm_host *pltfm_host;
 	struct sdhci_cdns_priv *priv;
 	struct clk *clk;
@@ -369,10 +480,10 @@ static int sdhci_cdns_probe(struct platform_device *pdev)
 
 	data = of_device_get_match_data(dev);
 	if (!data)
-		data = &sdhci_cdns_pltfm_data;
+		data = &sdhci_cdns_drv_data;
 
 	nr_phy_params = sdhci_cdns_phy_param_count(dev->of_node);
-	host = sdhci_pltfm_init(pdev, data,
+	host = sdhci_pltfm_init(pdev, &data->pltfm_data,
 				struct_size(priv, phy_params, nr_phy_params));
 	if (IS_ERR(host)) {
 		ret = PTR_ERR(host);
@@ -389,6 +500,11 @@ static int sdhci_cdns_probe(struct platform_device *pdev)
 	host->ioaddr += SDHCI_CDNS_SRS_BASE;
 	host->mmc_host_ops.hs400_enhanced_strobe =
 				sdhci_cdns_hs400_enhanced_strobe;
+	if (data->init) {
+		ret = data->init(pdev);
+		if (ret)
+			goto free;
+	}
 	sdhci_enable_v4_mode(host);
 	__sdhci_read_caps(host, &version, NULL, NULL);
 
@@ -453,7 +569,11 @@ static const struct dev_pm_ops sdhci_cdns_pm_ops = {
 static const struct of_device_id sdhci_cdns_match[] = {
 	{
 		.compatible = "socionext,uniphier-sd4hc",
-		.data = &sdhci_cdns_uniphier_pltfm_data,
+		.data = &sdhci_cdns_uniphier_drv_data,
+	},
+	{
+		.compatible = "pensando,elba-emmc",
+		.data = &sdhci_elba_drv_data
 	},
 	{ .compatible = "cdns,sd4hc" },
 	{ /* sentinel */ }
-- 
2.17.1

[PATCH v3 10/11] spi: dw: Add support for Pensando Elba SoC

[Brad Larson]

The Pensando Elba SoC includes a DW apb_ssi v4 controller
with device specific chip-select control.  The Elba SoC
provides four chip-selects where the native DW IP supports
two chip-selects.

Signed-off-by: Brad Larson <brad@pensando.io>
---
Changelog:
- Changed the implementation to use existing dw_spi_set_cs() and
  integrated Elba specific CS control into spi-dw-mmio.c.  The
  native designware support is for two chip-selects while Elba
  provides 4 chip-selects.  Instead of adding a new file for
  this support in gpio-elba-spics.c the support is in one
  file (spi-dw-mmio.c).

 drivers/spi/spi-dw-mmio.c | 85 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 85 insertions(+)

diff --git a/drivers/spi/spi-dw-mmio.c b/drivers/spi/spi-dw-mmio.c
index 3379720cfcb8..fe7b595fe33d 100644
--- a/drivers/spi/spi-dw-mmio.c
+++ b/drivers/spi/spi-dw-mmio.c
@@ -53,6 +53,24 @@ struct dw_spi_mscc {
 	void __iomem        *spi_mst; /* Not sparx5 */
 };
 
+struct dw_spi_elba {
+	struct regmap *regmap;
+	unsigned int reg;
+};
+
+/*
+ * Elba SoC does not use ssi, pin override is used for cs 0,1 and
+ * gpios for cs 2,3 as defined in the device tree.
+ *
+ * cs:  |       1               0
+ * bit: |---3-------2-------1-------0
+ *      |  cs1   cs1_ovr   cs0   cs0_ovr
+ */
+#define ELBA_SPICS_SHIFT(cs)		(2 * (cs))
+#define ELBA_SPICS_MASK(cs)		(0x3 << ELBA_SPICS_SHIFT(cs))
+#define ELBA_SPICS_SET(cs, val)	\
+			((((val) << 1) | 0x1) << ELBA_SPICS_SHIFT(cs))
+
 /*
  * The Designware SPI controller (referred to as master in the documentation)
  * automatically deasserts chip select when the tx fifo is empty. The chip
@@ -237,6 +255,72 @@ static int dw_spi_canaan_k210_init(struct platform_device *pdev,
 	return 0;
 }
 
+static void elba_spics_set_cs(struct dw_spi_elba *dwselba, int cs, int enable)
+{
+	regmap_update_bits(dwselba->regmap, dwselba->reg, ELBA_SPICS_MASK(cs),
+			   ELBA_SPICS_SET(cs, enable));
+}
+
+static void dw_spi_elba_set_cs(struct spi_device *spi, bool enable)
+{
+	struct dw_spi *dws = spi_master_get_devdata(spi->master);
+	struct dw_spi_mmio *dwsmmio = container_of(dws, struct dw_spi_mmio, dws);
+	struct dw_spi_elba *dwselba = dwsmmio->priv;
+	u8 cs = spi->chip_select;
+
+	if (cs < 2) {
+		/* overridden native chip-select */
+		elba_spics_set_cs(dwselba, spi->chip_select, enable);
+	}
+
+	/*
+	 * The DW SPI controller needs a native CS bit selected to start
+	 * the serial engine, and we have fewer native CSs than we need, so
+	 * use CS0 always.
+	 */
+	spi->chip_select = 0;
+	dw_spi_set_cs(spi, enable);
+	spi->chip_select = cs;
+}
+
+static int dw_spi_elba_init(struct platform_device *pdev,
+			    struct dw_spi_mmio *dwsmmio)
+{
+	struct of_phandle_args args;
+	struct dw_spi_elba *dwselba;
+	struct regmap *regmap;
+	int rc;
+
+	rc = of_parse_phandle_with_fixed_args(pdev->dev.of_node,
+			"pensando,spics", 1, 0, &args);
+	if (rc) {
+		dev_err(&pdev->dev, "could not find pensando,spics\n");
+		return rc;
+	}
+
+	regmap = syscon_node_to_regmap(args.np);
+	if (IS_ERR(regmap)) {
+		dev_err(&pdev->dev, "could not map pensando,spics\n");
+		return PTR_ERR(regmap);
+	}
+
+	dwselba = devm_kzalloc(&pdev->dev, sizeof(*dwselba), GFP_KERNEL);
+	if (!dwselba)
+		return -ENOMEM;
+
+	dwselba->regmap = regmap;
+	dwselba->reg = args.args[0];
+
+	/* deassert cs */
+	elba_spics_set_cs(dwselba, 0, 1);
+	elba_spics_set_cs(dwselba, 1, 1);
+
+	dwsmmio->priv = dwselba;
+	dwsmmio->dws.set_cs = dw_spi_elba_set_cs;
+
+	return 0;
+}
+
 static int dw_spi_mmio_probe(struct platform_device *pdev)
 {
 	int (*init_func)(struct platform_device *pdev,
@@ -351,6 +435,7 @@ static const struct of_device_id dw_spi_mmio_of_match[] = {
 	{ .compatible = "intel,keembay-ssi", .data = dw_spi_keembay_init},
 	{ .compatible = "microchip,sparx5-spi", dw_spi_mscc_sparx5_init},
 	{ .compatible = "canaan,k210-spi", dw_spi_canaan_k210_init},
+	{ .compatible = "pensando,elba-spi", .data = dw_spi_elba_init},
 	{ /* end of table */}
 };
 MODULE_DEVICE_TABLE(of, dw_spi_mmio_of_match);
-- 
2.17.1

[PATCH v3 11/11] arm64: dts: Add Pensando Elba SoC support

[Brad Larson]

Add Pensando common and Elba SoC specific device nodes

Signed-off-by: Brad Larson <brad@pensando.io>
---
Changelog:
- Node names changed to DT generic names
- Changed from using 'spi@' which is reserved
- The elba-flash-parts.dtsi is kept separate as
  it is included in multiple dts files.
- SPDX license tags at the top of each file
- The compatible = "pensando,elba" and 'model' are
  now together in the board file.
- UIO nodes removed
- Ordered nodes by increasing unit address
- Removed an unreferenced container node.
- Dropped deprecated 'device_type' for uart0 node.

 arch/arm64/boot/dts/Makefile                  |   1 +
 arch/arm64/boot/dts/pensando/Makefile         |   6 +
 arch/arm64/boot/dts/pensando/elba-16core.dtsi | 192 ++++++++++++++++++
 .../boot/dts/pensando/elba-asic-common.dtsi   |  96 +++++++++
 arch/arm64/boot/dts/pensando/elba-asic.dts    |  23 +++
 .../boot/dts/pensando/elba-flash-parts.dtsi   | 103 ++++++++++
 arch/arm64/boot/dts/pensando/elba.dtsi        | 181 +++++++++++++++++
 7 files changed, 602 insertions(+)
 create mode 100644 arch/arm64/boot/dts/pensando/Makefile
 create mode 100644 arch/arm64/boot/dts/pensando/elba-16core.dtsi
 create mode 100644 arch/arm64/boot/dts/pensando/elba-asic-common.dtsi
 create mode 100644 arch/arm64/boot/dts/pensando/elba-asic.dts
 create mode 100644 arch/arm64/boot/dts/pensando/elba-flash-parts.dtsi
 create mode 100644 arch/arm64/boot/dts/pensando/elba.dtsi

diff --git a/arch/arm64/boot/dts/Makefile b/arch/arm64/boot/dts/Makefile
index 639e01a4d855..34f99a99c488 100644
--- a/arch/arm64/boot/dts/Makefile
+++ b/arch/arm64/boot/dts/Makefile
@@ -20,6 +20,7 @@ subdir-y += marvell
 subdir-y += mediatek
 subdir-y += microchip
 subdir-y += nvidia
+subdir-y += pensando
 subdir-y += qcom
 subdir-y += realtek
 subdir-y += renesas
diff --git a/arch/arm64/boot/dts/pensando/Makefile b/arch/arm64/boot/dts/pensando/Makefile
new file mode 100644
index 000000000000..61031ec11838
--- /dev/null
+++ b/arch/arm64/boot/dts/pensando/Makefile
@@ -0,0 +1,6 @@
+# SPDX-License-Identifier: GPL-2.0
+dtb-$(CONFIG_ARCH_PENSANDO) += elba-asic.dtb
+
+always-y	:= $(dtb-y)
+subdir-y	:= $(dts-dirs)
+clean-files	:= *.dtb
diff --git a/arch/arm64/boot/dts/pensando/elba-16core.dtsi b/arch/arm64/boot/dts/pensando/elba-16core.dtsi
new file mode 100644
index 000000000000..acf5941afbc1
--- /dev/null
+++ b/arch/arm64/boot/dts/pensando/elba-16core.dtsi
@@ -0,0 +1,192 @@
+// SPDX-License-Identifier: GPL-2.0
+
+/ {
+	cpus {
+		#address-cells = <2>;
+		#size-cells = <0>;
+
+		cpu-map {
+			cluster0 {
+				core0 { cpu = <&cpu0>; };
+				core1 { cpu = <&cpu1>; };
+				core2 { cpu = <&cpu2>; };
+				core3 { cpu = <&cpu3>; };
+			};
+
+			cluster1 {
+				core0 { cpu = <&cpu4>; };
+				core1 { cpu = <&cpu5>; };
+				core2 { cpu = <&cpu6>; };
+				core3 { cpu = <&cpu7>; };
+			};
+
+			cluster2 {
+				core0 { cpu = <&cpu8>; };
+				core1 { cpu = <&cpu9>; };
+				core2 { cpu = <&cpu10>; };
+				core3 { cpu = <&cpu11>; };
+			};
+
+			cluster3 {
+				core0 { cpu = <&cpu12>; };
+				core1 { cpu = <&cpu13>; };
+				core2 { cpu = <&cpu14>; };
+				core3 { cpu = <&cpu15>; };
+			};
+		};
+
+		/* CLUSTER 0 */
+		cpu0: cpu@0 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a72";
+			reg = <0 0x0>;
+			next-level-cache = <&l2_0>;
+			enable-method = "psci";
+		};
+
+		cpu1: cpu@1 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a72";
+			reg = <0 0x1>;
+			next-level-cache = <&l2_0>;
+			enable-method = "psci";
+		};
+
+		cpu2: cpu@2 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a72";
+			reg = <0 0x2>;
+			next-level-cache = <&l2_0>;
+			enable-method = "psci";
+		};
+
+		cpu3: cpu@3 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a72";
+			reg = <0 0x3>;
+			next-level-cache = <&l2_0>;
+			enable-method = "psci";
+		};
+
+		l2_0: l2-cache0 {
+			compatible = "cache";
+		};
+
+		/* CLUSTER 1 */
+		cpu4: cpu@100 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a72";
+			reg = <0 0x100>;
+			next-level-cache = <&l2_1>;
+			enable-method = "psci";
+		};
+
+		cpu5: cpu@101 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a72";
+			reg = <0 0x101>;
+			next-level-cache = <&l2_1>;
+			enable-method = "psci";
+		};
+
+		cpu6: cpu@102 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a72";
+			reg = <0 0x102>;
+			next-level-cache = <&l2_1>;
+			enable-method = "psci";
+		};
+
+		cpu7: cpu@103 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a72";
+			reg = <0 0x103>;
+			next-level-cache = <&l2_1>;
+			enable-method = "psci";
+		};
+
+		l2_1: l2-cache1 {
+			compatible = "cache";
+		};
+
+		/* CLUSTER 2 */
+		cpu8: cpu@200 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a72";
+			reg = <0 0x200>;
+			next-level-cache = <&l2_2>;
+			enable-method = "psci";
+		};
+
+		cpu9: cpu@201 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a72";
+			reg = <0 0x201>;
+			next-level-cache = <&l2_2>;
+			enable-method = "psci";
+		};
+
+		cpu10: cpu@202 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a72";
+			reg = <0 0x202>;
+			next-level-cache = <&l2_2>;
+			enable-method = "psci";
+		};
+
+		cpu11: cpu@203 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a72";
+			reg = <0 0x203>;
+			next-level-cache = <&l2_2>;
+			enable-method = "psci";
+		};
+
+		l2_2: l2-cache2 {
+			compatible = "cache";
+		};
+
+		/* CLUSTER 3 */
+		cpu12: cpu@300 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a72";
+			reg = <0 0x300>;
+			next-level-cache = <&l2_3>;
+			enable-method = "psci";
+		};
+
+		cpu13: cpu@301 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a72";
+			reg = <0 0x301>;
+			next-level-cache = <&l2_3>;
+			enable-method = "psci";
+		};
+
+		cpu14: cpu@302 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a72";
+			reg = <0 0x302>;
+			next-level-cache = <&l2_3>;
+			enable-method = "psci";
+		};
+
+		cpu15: cpu@303 {
+			device_type = "cpu";
+			compatible = "arm,cortex-a72";
+			reg = <0 0x303>;
+			next-level-cache = <&l2_3>;
+			enable-method = "psci";
+		};
+
+		l2_3: l2-cache3 {
+			compatible = "cache";
+		};
+
+		psci {
+			compatible = "arm,psci-0.2";
+			method = "smc";
+		};
+
+	};
+};
diff --git a/arch/arm64/boot/dts/pensando/elba-asic-common.dtsi b/arch/arm64/boot/dts/pensando/elba-asic-common.dtsi
new file mode 100644
index 000000000000..ba584c0fe0d5
--- /dev/null
+++ b/arch/arm64/boot/dts/pensando/elba-asic-common.dtsi
@@ -0,0 +1,96 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2019-2021, Pensando Systems Inc. */
+
+&ahb_clk {
+	clock-frequency = <400000000>;
+};
+
+&emmc_clk {
+	clock-frequency = <200000000>;
+};
+
+&flash_clk {
+	clock-frequency = <400000000>;
+};
+
+&ref_clk {
+	clock-frequency = <156250000>;
+};
+
+&qspi {
+	status = "okay";
+	flash0: flash@0 {
+		compatible = "jedec,spi-nor";
+		reg = <0>;
+		spi-max-frequency = <40000000>;
+		spi-rx-bus-width = <2>;
+		m25p,fast-read;
+		cdns,read-delay = <0>;
+		cdns,tshsl-ns = <0>;
+		cdns,tsd2d-ns = <0>;
+		cdns,tchsh-ns = <0>;
+		cdns,tslch-ns = <0>;
+	};
+};
+
+&gpio0 {
+	status = "okay";
+};
+
+&emmc {
+	bus-width = <8>;
+	status = "okay";
+};
+
+&wdt0 {
+	status = "okay";
+};
+
+&i2c0 {
+	clock-frequency = <100000>;
+	status = "okay";
+	rtc@51 {
+		compatible = "nxp,pcf85263";
+		reg = <0x51>;
+	};
+};
+
+&spi0 {
+	num-cs = <4>;
+	cs-gpios = <0>, <0>, <&porta 1 GPIO_ACTIVE_LOW>,
+		   <&porta 7 GPIO_ACTIVE_LOW>;
+	status = "okay";
+	spi0_cs0@0 {
+		compatible = "semtech,sx1301";	/* Enable spidev */
+		#address-cells = <1>;
+		#size-cells = <1>;
+		spi-max-frequency = <12000000>;
+		reg = <0>;
+	};
+
+	spi0_cs1@1 {
+		compatible = "semtech,sx1301";
+		#address-cells = <1>;
+		#size-cells = <1>;
+		spi-max-frequency = <12000000>;
+		reg = <1>;
+	};
+
+	spi0_cs2@2 {
+		compatible = "semtech,sx1301";
+		#address-cells = <1>;
+		#size-cells = <1>;
+		spi-max-frequency = <12000000>;
+		reg = <2>;
+		interrupt-parent = <&porta>;
+		interrupts = <0 IRQ_TYPE_LEVEL_LOW>;
+	};
+
+	spi0_cs3@3 {
+		compatible = "semtech,sx1301";
+		#address-cells = <1>;
+		#size-cells = <1>;
+		spi-max-frequency = <12000000>;
+		reg = <3>;
+	};
+};
diff --git a/arch/arm64/boot/dts/pensando/elba-asic.dts b/arch/arm64/boot/dts/pensando/elba-asic.dts
new file mode 100644
index 000000000000..131931dc643f
--- /dev/null
+++ b/arch/arm64/boot/dts/pensando/elba-asic.dts
@@ -0,0 +1,23 @@
+// SPDX-License-Identifier: GPL-2.0
+
+/dts-v1/;
+
+/ {
+	model = "Elba ASIC Board";
+	compatible = "pensando,elba";
+
+	aliases {
+		serial0 = &uart0;
+		spi0 = &spi0;
+		spi1 = &qspi;
+	};
+
+	chosen {
+		stdout-path = "serial0:115200n8";
+	};
+};
+
+#include "elba.dtsi"
+#include "elba-16core.dtsi"
+#include "elba-asic-common.dtsi"
+#include "elba-flash-parts.dtsi"
diff --git a/arch/arm64/boot/dts/pensando/elba-flash-parts.dtsi b/arch/arm64/boot/dts/pensando/elba-flash-parts.dtsi
new file mode 100644
index 000000000000..e69734c2c267
--- /dev/null
+++ b/arch/arm64/boot/dts/pensando/elba-flash-parts.dtsi
@@ -0,0 +1,103 @@
+// SPDX-License-Identifier: GPL-2.0
+
+&flash0 {
+	partitions {
+		compatible = "fixed-partitions";
+		#address-cells = <1>;
+		#size-cells = <1>;
+		partition@0 {
+			label = "flash";
+			reg = <0x10000 0xfff0000>;
+		};
+
+		partition@f0000 {
+			label = "golduenv";
+			reg = <0xf0000 0x10000>;
+		};
+
+		partition@100000 {
+			label = "boot0";
+			reg = <0x100000 0x80000>;
+		};
+
+		partition@180000 {
+			label = "golduboot";
+			reg = <0x180000 0x200000>;
+		};
+
+		partition@380000 {
+			label = "brdcfg0";
+			reg = <0x380000 0x10000>;
+		};
+
+		partition@390000 {
+			label = "brdcfg1";
+			reg = <0x390000 0x10000>;
+		};
+
+		partition@400000 {
+			label = "goldfw";
+			reg = <0x400000 0x3c00000>;
+		};
+
+		partition@4010000 {
+			label = "fwmap";
+			reg = <0x4010000 0x20000>;
+		};
+
+		partition@4030000 {
+			label = "fwsel";
+			reg = <0x4030000 0x20000>;
+		};
+
+		partition@4090000 {
+			label = "bootlog";
+			reg = <0x4090000 0x20000>;
+		};
+
+		partition@40b0000 {
+			label = "panicbuf";
+			reg = <0x40b0000 0x20000>;
+		};
+
+		partition@40d0000 {
+			label = "uservars";
+			reg = <0x40d0000 0x20000>;
+		};
+
+		partition@4200000 {
+			label = "uboota";
+			reg = <0x4200000 0x400000>;
+		};
+
+		partition@4600000 {
+			label = "ubootb";
+			reg = <0x4600000 0x400000>;
+		};
+
+		partition@4a00000 {
+			label = "mainfwa";
+			reg = <0x4a00000 0x1000000>;
+		};
+
+		partition@5a00000 {
+			label = "mainfwb";
+			reg = <0x5a00000 0x1000000>;
+		};
+
+		partition@6a00000 {
+			label = "diaguboot";
+			reg = <0x6a00000 0x400000>;
+		};
+
+		partition@8000000 {
+			label = "diagfw";
+			reg = <0x8000000 0x7fe0000>;
+		};
+
+		partition@ffe0000 {
+			label = "ubootenv";
+			reg = <0xffe0000 0x10000>;
+		};
+	};
+};
diff --git a/arch/arm64/boot/dts/pensando/elba.dtsi b/arch/arm64/boot/dts/pensando/elba.dtsi
new file mode 100644
index 000000000000..b28f69e0bd91
--- /dev/null
+++ b/arch/arm64/boot/dts/pensando/elba.dtsi
@@ -0,0 +1,181 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2019-2021, Pensando Systems Inc. */
+
+#include <dt-bindings/gpio/gpio.h>
+#include "dt-bindings/interrupt-controller/arm-gic.h"
+
+/ {
+	interrupt-parent = <&gic>;
+	#address-cells = <2>;
+	#size-cells = <2>;
+
+	dma-coherent;
+
+	ahb_clk: oscillator0 {
+		compatible = "fixed-clock";
+		#clock-cells = <0>;
+	};
+
+	emmc_clk: oscillator2 {
+		compatible = "fixed-clock";
+		#clock-cells = <0>;
+	};
+
+	flash_clk: oscillator3 {
+		compatible = "fixed-clock";
+		#clock-cells = <0>;
+	};
+
+	ref_clk: oscillator4 {
+		compatible = "fixed-clock";
+		#clock-cells = <0>;
+	};
+
+	timer {
+		compatible = "arm,armv8-timer";
+		interrupts = <GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(1) |
+					IRQ_TYPE_LEVEL_LOW)>,
+			     <GIC_PPI 14 (GIC_CPU_MASK_SIMPLE(1) |
+					IRQ_TYPE_LEVEL_LOW)>,
+			     <GIC_PPI 11 (GIC_CPU_MASK_SIMPLE(1) |
+					IRQ_TYPE_LEVEL_LOW)>,
+			     <GIC_PPI 10 (GIC_CPU_MASK_SIMPLE(1) |
+					IRQ_TYPE_LEVEL_LOW)>;
+	};
+
+	pmu {
+		compatible = "arm,cortex-a72-pmu";
+		interrupts = <GIC_PPI 7 (GIC_CPU_MASK_SIMPLE(1) |
+				IRQ_TYPE_LEVEL_LOW)>;
+	};
+
+	soc: soc {
+		compatible = "simple-bus";
+		#address-cells = <2>;
+		#size-cells = <2>;
+		ranges;
+
+		i2c0: i2c@400 {
+			compatible = "snps,designware-i2c";
+			reg = <0x0 0x400 0x0 0x100>;
+			clocks = <&ahb_clk>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+			i2c-sda-hold-time-ns = <480>;
+			snps,sda-timeout-ms = <750>;
+			interrupts = <GIC_SPI 4 IRQ_TYPE_LEVEL_HIGH>;
+			status = "disabled";
+		};
+
+		wdt0: watchdog@1400 {
+			compatible = "snps,dw-wdt";
+			reg = <0x0 0x1400 0x0 0x100>;
+			clocks = <&ahb_clk>;
+			interrupts = <GIC_SPI 10 IRQ_TYPE_LEVEL_HIGH>;
+			status = "disabled";
+		};
+
+		qspi: spi@2400 {
+			compatible = "pensando,elba-qspi", "cdns,qspi-nor";
+			#address-cells = <1>;
+			#size-cells = <0>;
+			reg = <0x0 0x2400 0x0 0x400>,
+			      <0x0 0x7fff0000 0x0 0x1000>;
+			interrupts = <GIC_SPI 0 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&flash_clk>;
+			cdns,fifo-depth = <1024>;
+			cdns,fifo-width = <4>;
+			cdns,trigger-address = <0x7fff0000>;
+			status = "disabled";
+		};
+
+		spi0: spi@2800 {
+			compatible = "pensando,elba-spi";
+			reg = <0x0 0x2800 0x0 0x100>;
+			pensando,spics = <&mssoc 0x2468>;
+			clocks = <&ahb_clk>;
+			interrupts = <GIC_SPI 9 IRQ_TYPE_LEVEL_HIGH>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+			num-cs = <2>;
+			status = "disabled";
+		};
+
+		gpio0: gpio@4000 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			compatible = "snps,dw-apb-gpio";
+			reg = <0x0 0x4000 0x0 0x78>;
+			status = "disabled";
+
+			porta: gpio-port@0 {
+				compatible = "snps,dw-apb-gpio-port";
+				reg = <0>;
+				gpio-controller;
+				#gpio-cells = <2>;
+				ngpios = <8>;
+				interrupts = <GIC_SPI 1 IRQ_TYPE_LEVEL_HIGH>;
+				interrupt-controller;
+				interrupt-parent = <&gic>;
+				#interrupt-cells = <2>;
+			};
+
+			portb: gpio-port@1 {
+				compatible = "snps,dw-apb-gpio-port";
+				reg = <1>;
+				gpio-controller;
+				#gpio-cells = <2>;
+				ngpios = <8>;
+			};
+		};
+
+		uart0: serial@4800 {
+			compatible = "ns16550a";
+			reg = <0x0 0x4800 0x0 0x100>;
+			clocks = <&ref_clk>;
+			interrupts = <GIC_SPI 2 IRQ_TYPE_LEVEL_HIGH>;
+			reg-shift = <2>;
+			reg-io-width = <4>;
+		};
+
+		gic: interrupt-controller@800000 {
+			compatible = "arm,gic-v3";
+			#interrupt-cells = <3>;
+			#address-cells = <2>;
+			#size-cells = <2>;
+			ranges;
+			interrupt-controller;
+			reg = <0x0 0x800000 0x0 0x200000>,	/* GICD */
+			      <0x0 0xa00000 0x0 0x200000>;	/* GICR */
+			interrupts = <GIC_PPI 9 IRQ_TYPE_LEVEL_HIGH>;
+
+			gic_its: msi-controller@820000 {
+				compatible = "arm,gic-v3-its";
+				msi-controller;
+				#msi-cells = <1>;
+				reg = <0x0 0x820000 0x0 0x10000>;
+				socionext,synquacer-pre-its =
+							<0xc00000 0x1000000>;
+			};
+		};
+
+		emmc: mmc@30440000 {
+			compatible = "pensando,elba-emmc", "cdns,sd4hc";
+			clocks = <&emmc_clk>;
+			interrupts = <GIC_SPI 26 IRQ_TYPE_LEVEL_HIGH>;
+			reg = <0x0 0x30440000 0x0 0x10000>,
+			      <0x0 0x30480044 0x0 0x4>;    /* byte-lane ctrl */
+			cdns,phy-input-delay-sd-highspeed = <0x4>;
+			cdns,phy-input-delay-legacy = <0x4>;
+			cdns,phy-input-delay-sd-uhs-sdr50 = <0x6>;
+			cdns,phy-input-delay-sd-uhs-ddr50 = <0x16>;
+			mmc-ddr-1_8v;
+			status = "disabled";
+		};
+
+		mssoc: mssoc@307c0000 {
+			compatible = "syscon", "simple-mfd";
+			reg = <0x0 0x307c0000 0x0 0x3000>;
+		};
+	};
+};
-- 
2.17.1

Re: [PATCH v3 11/11] arm64: dts: Add Pensando Elba SoC support

[Mark Rutland]

Hi,

On Sun, Oct 24, 2021 at 06:51:56PM -0700, Brad Larson wrote:
> Add Pensando common and Elba SoC specific device nodes
> 
> Signed-off-by: Brad Larson <brad@pensando.io>

[...]

> +	timer {
> +		compatible = "arm,armv8-timer";
> +		interrupts = <GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(1) |
> +					IRQ_TYPE_LEVEL_LOW)>,
> +			     <GIC_PPI 14 (GIC_CPU_MASK_SIMPLE(1) |
> +					IRQ_TYPE_LEVEL_LOW)>,
> +			     <GIC_PPI 11 (GIC_CPU_MASK_SIMPLE(1) |
> +					IRQ_TYPE_LEVEL_LOW)>,
> +			     <GIC_PPI 10 (GIC_CPU_MASK_SIMPLE(1) |
> +					IRQ_TYPE_LEVEL_LOW)>;
> +	};

The GIC_CPU_MASK_SIMPLE() stuff is meant for GICv2, but as below you
have GICv3, where this is not valid, so this should go.

Also, beware that GIC_CPU_MASK_SIMPLE(1) means a single CPU, which
doesn't mak sense for the 16 CPUs you have.

> +		gic: interrupt-controller@800000 {
> +			compatible = "arm,gic-v3";
> +			#interrupt-cells = <3>;
> +			#address-cells = <2>;
> +			#size-cells = <2>;
> +			ranges;
> +			interrupt-controller;
> +			reg = <0x0 0x800000 0x0 0x200000>,	/* GICD */
> +			      <0x0 0xa00000 0x0 0x200000>;	/* GICR */
> +			interrupts = <GIC_PPI 9 IRQ_TYPE_LEVEL_HIGH>;
> +
> +			gic_its: msi-controller@820000 {
> +				compatible = "arm,gic-v3-its";
> +				msi-controller;
> +				#msi-cells = <1>;
> +				reg = <0x0 0x820000 0x0 0x10000>;
> +				socionext,synquacer-pre-its =
> +							<0xc00000 0x1000000>;
> +			};
> +		};

Is there any shared lineage with Synquacer? The commit message didn't
describe this quirk.

Thanks,
Mark.

Re: [PATCH v3 11/11] arm64: dts: Add Pensando Elba SoC support

[Marc Zyngier]

On 2021-10-25 10:17, Mark Rutland wrote:
> Hi,
> 
> On Sun, Oct 24, 2021 at 06:51:56PM -0700, Brad Larson wrote:
>> Add Pensando common and Elba SoC specific device nodes
>> 
>> Signed-off-by: Brad Larson <brad@pensando.io>
> 
> [...]
> 
>> +	timer {
>> +		compatible = "arm,armv8-timer";
>> +		interrupts = <GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(1) |
>> +					IRQ_TYPE_LEVEL_LOW)>,
>> +			     <GIC_PPI 14 (GIC_CPU_MASK_SIMPLE(1) |
>> +					IRQ_TYPE_LEVEL_LOW)>,
>> +			     <GIC_PPI 11 (GIC_CPU_MASK_SIMPLE(1) |
>> +					IRQ_TYPE_LEVEL_LOW)>,
>> +			     <GIC_PPI 10 (GIC_CPU_MASK_SIMPLE(1) |
>> +					IRQ_TYPE_LEVEL_LOW)>;
>> +	};
> 
> The GIC_CPU_MASK_SIMPLE() stuff is meant for GICv2, but as below you
> have GICv3, where this is not valid, so this should go.
> 
> Also, beware that GIC_CPU_MASK_SIMPLE(1) means a single CPU, which
> doesn't mak sense for the 16 CPUs you have.
> 
>> +		gic: interrupt-controller@800000 {
>> +			compatible = "arm,gic-v3";
>> +			#interrupt-cells = <3>;
>> +			#address-cells = <2>;
>> +			#size-cells = <2>;
>> +			ranges;
>> +			interrupt-controller;
>> +			reg = <0x0 0x800000 0x0 0x200000>,	/* GICD */
>> +			      <0x0 0xa00000 0x0 0x200000>;	/* GICR */

This is missing the GICv2 compat regions that the CPUs implement.

>> +			interrupts = <GIC_PPI 9 IRQ_TYPE_LEVEL_HIGH>;
>> +
>> +			gic_its: msi-controller@820000 {
>> +				compatible = "arm,gic-v3-its";
>> +				msi-controller;
>> +				#msi-cells = <1>;
>> +				reg = <0x0 0x820000 0x0 0x10000>;
>> +				socionext,synquacer-pre-its =
>> +							<0xc00000 0x1000000>;
>> +			};
>> +		};
> 
> Is there any shared lineage with Synquacer? The commit message didn't
> describe this quirk.

Funny, it looks like there is a sudden outburst of stupid copy/paste
among HW designers. TI did the exact same thing recently.

This totally negates all the advantages of having an ITS and makes
sure that you have all the overhead. Facepalm...

         M.
-- 
Jazz is not dead. It just smells funny...

Re: [PATCH v3 03/11] dt-bindings: mmc: Add Pensando Elba SoC binding

[Rob Herring]

On Sun, 24 Oct 2021 18:51:48 -0700, Brad Larson wrote:
> Pensando Elba ARM 64-bit SoC is integrated with this IP and
> explicitly controls byte-lane enables resulting in an additional
> reg property resource.
> 
> Signed-off-by: Brad Larson <brad@pensando.io>
> ---
>  .../devicetree/bindings/mmc/cdns,sdhci.yaml         | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:
./Documentation/devicetree/bindings/mmc/cdns,sdhci.yaml:20:9: [warning] wrong indentation: expected 10 but found 8 (indentation)

dtschema/dtc warnings/errors:

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/patch/1545481

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.

Re: [PATCH v3 03/11] dt-bindings: mmc: Add Pensando Elba SoC binding

[Rob Herring]

On Sun, Oct 24, 2021 at 06:51:48PM -0700, Brad Larson wrote:
> Pensando Elba ARM 64-bit SoC is integrated with this IP and
> explicitly controls byte-lane enables resulting in an additional
> reg property resource.
> 
> Signed-off-by: Brad Larson <brad@pensando.io>
> ---
>  .../devicetree/bindings/mmc/cdns,sdhci.yaml         | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/mmc/cdns,sdhci.yaml b/Documentation/devicetree/bindings/mmc/cdns,sdhci.yaml
> index af7442f73881..6c68b7b5abec 100644
> --- a/Documentation/devicetree/bindings/mmc/cdns,sdhci.yaml
> +++ b/Documentation/devicetree/bindings/mmc/cdns,sdhci.yaml
> @@ -15,13 +15,16 @@ allOf:
>  
>  properties:
>    compatible:
> -    items:
> -      - enum:
> -          - socionext,uniphier-sd4hc
> -      - const: cdns,sd4hc
> +    oneOf:
> +      - items:
> +        - enum:
> +            - socionext,uniphier-sd4hc
> +            - pensando,elba-emmc
> +        - const: cdns,sd4hc
>  
>    reg:
> -    maxItems: 1
> +    minItems: 1
> +    maxItems: 2

If there is more than 1, then you need to describe what each entry is.

>  
>    interrupts:
>      maxItems: 1
> -- 
> 2.17.1
> 
> 

Re: [PATCH v3 11/11] arm64: dts: Add Pensando Elba SoC support

[Rob Herring]

On Sun, Oct 24, 2021 at 06:51:56PM -0700, Brad Larson wrote:
> Add Pensando common and Elba SoC specific device nodes
> 
> Signed-off-by: Brad Larson <brad@pensando.io>
> ---
> Changelog:
> - Node names changed to DT generic names
> - Changed from using 'spi@' which is reserved
> - The elba-flash-parts.dtsi is kept separate as
>   it is included in multiple dts files.
> - SPDX license tags at the top of each file
> - The compatible = "pensando,elba" and 'model' are
>   now together in the board file.
> - UIO nodes removed
> - Ordered nodes by increasing unit address
> - Removed an unreferenced container node.
> - Dropped deprecated 'device_type' for uart0 node.
> 
>  arch/arm64/boot/dts/Makefile                  |   1 +
>  arch/arm64/boot/dts/pensando/Makefile         |   6 +
>  arch/arm64/boot/dts/pensando/elba-16core.dtsi | 192 ++++++++++++++++++
>  .../boot/dts/pensando/elba-asic-common.dtsi   |  96 +++++++++
>  arch/arm64/boot/dts/pensando/elba-asic.dts    |  23 +++
>  .../boot/dts/pensando/elba-flash-parts.dtsi   | 103 ++++++++++
>  arch/arm64/boot/dts/pensando/elba.dtsi        | 181 +++++++++++++++++
>  7 files changed, 602 insertions(+)
>  create mode 100644 arch/arm64/boot/dts/pensando/Makefile
>  create mode 100644 arch/arm64/boot/dts/pensando/elba-16core.dtsi
>  create mode 100644 arch/arm64/boot/dts/pensando/elba-asic-common.dtsi
>  create mode 100644 arch/arm64/boot/dts/pensando/elba-asic.dts
>  create mode 100644 arch/arm64/boot/dts/pensando/elba-flash-parts.dtsi
>  create mode 100644 arch/arm64/boot/dts/pensando/elba.dtsi
> 
> diff --git a/arch/arm64/boot/dts/Makefile b/arch/arm64/boot/dts/Makefile
> index 639e01a4d855..34f99a99c488 100644
> --- a/arch/arm64/boot/dts/Makefile
> +++ b/arch/arm64/boot/dts/Makefile
> @@ -20,6 +20,7 @@ subdir-y += marvell
>  subdir-y += mediatek
>  subdir-y += microchip
>  subdir-y += nvidia
> +subdir-y += pensando
>  subdir-y += qcom
>  subdir-y += realtek
>  subdir-y += renesas
> diff --git a/arch/arm64/boot/dts/pensando/Makefile b/arch/arm64/boot/dts/pensando/Makefile
> new file mode 100644
> index 000000000000..61031ec11838
> --- /dev/null
> +++ b/arch/arm64/boot/dts/pensando/Makefile
> @@ -0,0 +1,6 @@
> +# SPDX-License-Identifier: GPL-2.0
> +dtb-$(CONFIG_ARCH_PENSANDO) += elba-asic.dtb
> +

> +always-y	:= $(dtb-y)
> +subdir-y	:= $(dts-dirs)
> +clean-files	:= *.dtb

None of these lines should be needed.

> diff --git a/arch/arm64/boot/dts/pensando/elba-16core.dtsi b/arch/arm64/boot/dts/pensando/elba-16core.dtsi
> new file mode 100644
> index 000000000000..acf5941afbc1
> --- /dev/null
> +++ b/arch/arm64/boot/dts/pensando/elba-16core.dtsi
> @@ -0,0 +1,192 @@
> +// SPDX-License-Identifier: GPL-2.0

Do you care about using with non-GPL OS? Dual license is preferred.

> +
> +/ {
> +	cpus {
> +		#address-cells = <2>;
> +		#size-cells = <0>;
> +
> +		cpu-map {
> +			cluster0 {
> +				core0 { cpu = <&cpu0>; };
> +				core1 { cpu = <&cpu1>; };
> +				core2 { cpu = <&cpu2>; };
> +				core3 { cpu = <&cpu3>; };
> +			};
> +
> +			cluster1 {
> +				core0 { cpu = <&cpu4>; };
> +				core1 { cpu = <&cpu5>; };
> +				core2 { cpu = <&cpu6>; };
> +				core3 { cpu = <&cpu7>; };
> +			};
> +
> +			cluster2 {
> +				core0 { cpu = <&cpu8>; };
> +				core1 { cpu = <&cpu9>; };
> +				core2 { cpu = <&cpu10>; };
> +				core3 { cpu = <&cpu11>; };
> +			};
> +
> +			cluster3 {
> +				core0 { cpu = <&cpu12>; };
> +				core1 { cpu = <&cpu13>; };
> +				core2 { cpu = <&cpu14>; };
> +				core3 { cpu = <&cpu15>; };
> +			};
> +		};
> +
> +		/* CLUSTER 0 */
> +		cpu0: cpu@0 {
> +			device_type = "cpu";
> +			compatible = "arm,cortex-a72";
> +			reg = <0 0x0>;
> +			next-level-cache = <&l2_0>;
> +			enable-method = "psci";
> +		};
> +
> +		cpu1: cpu@1 {
> +			device_type = "cpu";
> +			compatible = "arm,cortex-a72";
> +			reg = <0 0x1>;
> +			next-level-cache = <&l2_0>;
> +			enable-method = "psci";
> +		};
> +
> +		cpu2: cpu@2 {
> +			device_type = "cpu";
> +			compatible = "arm,cortex-a72";
> +			reg = <0 0x2>;
> +			next-level-cache = <&l2_0>;
> +			enable-method = "psci";
> +		};
> +
> +		cpu3: cpu@3 {
> +			device_type = "cpu";
> +			compatible = "arm,cortex-a72";
> +			reg = <0 0x3>;
> +			next-level-cache = <&l2_0>;
> +			enable-method = "psci";
> +		};
> +
> +		l2_0: l2-cache0 {
> +			compatible = "cache";
> +		};
> +
> +		/* CLUSTER 1 */
> +		cpu4: cpu@100 {
> +			device_type = "cpu";
> +			compatible = "arm,cortex-a72";
> +			reg = <0 0x100>;
> +			next-level-cache = <&l2_1>;
> +			enable-method = "psci";
> +		};
> +
> +		cpu5: cpu@101 {
> +			device_type = "cpu";
> +			compatible = "arm,cortex-a72";
> +			reg = <0 0x101>;
> +			next-level-cache = <&l2_1>;
> +			enable-method = "psci";
> +		};
> +
> +		cpu6: cpu@102 {
> +			device_type = "cpu";
> +			compatible = "arm,cortex-a72";
> +			reg = <0 0x102>;
> +			next-level-cache = <&l2_1>;
> +			enable-method = "psci";
> +		};
> +
> +		cpu7: cpu@103 {
> +			device_type = "cpu";
> +			compatible = "arm,cortex-a72";
> +			reg = <0 0x103>;
> +			next-level-cache = <&l2_1>;
> +			enable-method = "psci";
> +		};
> +
> +		l2_1: l2-cache1 {
> +			compatible = "cache";
> +		};
> +
> +		/* CLUSTER 2 */
> +		cpu8: cpu@200 {
> +			device_type = "cpu";
> +			compatible = "arm,cortex-a72";
> +			reg = <0 0x200>;
> +			next-level-cache = <&l2_2>;
> +			enable-method = "psci";
> +		};
> +
> +		cpu9: cpu@201 {
> +			device_type = "cpu";
> +			compatible = "arm,cortex-a72";
> +			reg = <0 0x201>;
> +			next-level-cache = <&l2_2>;
> +			enable-method = "psci";
> +		};
> +
> +		cpu10: cpu@202 {
> +			device_type = "cpu";
> +			compatible = "arm,cortex-a72";
> +			reg = <0 0x202>;
> +			next-level-cache = <&l2_2>;
> +			enable-method = "psci";
> +		};
> +
> +		cpu11: cpu@203 {
> +			device_type = "cpu";
> +			compatible = "arm,cortex-a72";
> +			reg = <0 0x203>;
> +			next-level-cache = <&l2_2>;
> +			enable-method = "psci";
> +		};
> +
> +		l2_2: l2-cache2 {
> +			compatible = "cache";
> +		};
> +
> +		/* CLUSTER 3 */
> +		cpu12: cpu@300 {
> +			device_type = "cpu";
> +			compatible = "arm,cortex-a72";
> +			reg = <0 0x300>;
> +			next-level-cache = <&l2_3>;
> +			enable-method = "psci";
> +		};
> +
> +		cpu13: cpu@301 {
> +			device_type = "cpu";
> +			compatible = "arm,cortex-a72";
> +			reg = <0 0x301>;
> +			next-level-cache = <&l2_3>;
> +			enable-method = "psci";
> +		};
> +
> +		cpu14: cpu@302 {
> +			device_type = "cpu";
> +			compatible = "arm,cortex-a72";
> +			reg = <0 0x302>;
> +			next-level-cache = <&l2_3>;
> +			enable-method = "psci";
> +		};
> +
> +		cpu15: cpu@303 {
> +			device_type = "cpu";
> +			compatible = "arm,cortex-a72";
> +			reg = <0 0x303>;
> +			next-level-cache = <&l2_3>;
> +			enable-method = "psci";
> +		};
> +
> +		l2_3: l2-cache3 {
> +			compatible = "cache";
> +		};
> +
> +		psci {

This goes at the root level.

> +			compatible = "arm,psci-0.2";
> +			method = "smc";
> +		};
> +
> +	};
> +};
> diff --git a/arch/arm64/boot/dts/pensando/elba-asic-common.dtsi b/arch/arm64/boot/dts/pensando/elba-asic-common.dtsi
> new file mode 100644
> index 000000000000..ba584c0fe0d5
> --- /dev/null
> +++ b/arch/arm64/boot/dts/pensando/elba-asic-common.dtsi
> @@ -0,0 +1,96 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright (c) 2019-2021, Pensando Systems Inc. */
> +
> +&ahb_clk {
> +	clock-frequency = <400000000>;
> +};
> +
> +&emmc_clk {
> +	clock-frequency = <200000000>;
> +};
> +
> +&flash_clk {
> +	clock-frequency = <400000000>;
> +};
> +
> +&ref_clk {
> +	clock-frequency = <156250000>;
> +};
> +
> +&qspi {
> +	status = "okay";
> +	flash0: flash@0 {
> +		compatible = "jedec,spi-nor";
> +		reg = <0>;
> +		spi-max-frequency = <40000000>;
> +		spi-rx-bus-width = <2>;
> +		m25p,fast-read;
> +		cdns,read-delay = <0>;
> +		cdns,tshsl-ns = <0>;
> +		cdns,tsd2d-ns = <0>;
> +		cdns,tchsh-ns = <0>;
> +		cdns,tslch-ns = <0>;
> +	};
> +};
> +
> +&gpio0 {
> +	status = "okay";
> +};
> +
> +&emmc {
> +	bus-width = <8>;
> +	status = "okay";
> +};
> +
> +&wdt0 {
> +	status = "okay";
> +};
> +
> +&i2c0 {
> +	clock-frequency = <100000>;
> +	status = "okay";
> +	rtc@51 {
> +		compatible = "nxp,pcf85263";
> +		reg = <0x51>;
> +	};
> +};
> +
> +&spi0 {
> +	num-cs = <4>;
> +	cs-gpios = <0>, <0>, <&porta 1 GPIO_ACTIVE_LOW>,
> +		   <&porta 7 GPIO_ACTIVE_LOW>;
> +	status = "okay";
> +	spi0_cs0@0 {

Node names should reflect the class of device and use standard name 
defined in the DT spec. This probably doesn't have one. 'lora' perhaps?


> +		compatible = "semtech,sx1301";	/* Enable spidev */

What's spidev?

> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +		spi-max-frequency = <12000000>;
> +		reg = <0>;
> +	};
> +
> +	spi0_cs1@1 {
> +		compatible = "semtech,sx1301";
> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +		spi-max-frequency = <12000000>;
> +		reg = <1>;
> +	};
> +
> +	spi0_cs2@2 {
> +		compatible = "semtech,sx1301";
> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +		spi-max-frequency = <12000000>;
> +		reg = <2>;
> +		interrupt-parent = <&porta>;
> +		interrupts = <0 IRQ_TYPE_LEVEL_LOW>;
> +	};
> +
> +	spi0_cs3@3 {
> +		compatible = "semtech,sx1301";
> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +		spi-max-frequency = <12000000>;
> +		reg = <3>;
> +	};
> +};
> diff --git a/arch/arm64/boot/dts/pensando/elba-asic.dts b/arch/arm64/boot/dts/pensando/elba-asic.dts
> new file mode 100644
> index 000000000000..131931dc643f
> --- /dev/null
> +++ b/arch/arm64/boot/dts/pensando/elba-asic.dts
> @@ -0,0 +1,23 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +/dts-v1/;
> +
> +/ {
> +	model = "Elba ASIC Board";
> +	compatible = "pensando,elba";

Normally we have a compatible for the board plus the soc compatible.

> +
> +	aliases {
> +		serial0 = &uart0;
> +		spi0 = &spi0;
> +		spi1 = &qspi;
> +	};
> +
> +	chosen {
> +		stdout-path = "serial0:115200n8";
> +	};
> +};
> +
> +#include "elba.dtsi"
> +#include "elba-16core.dtsi"
> +#include "elba-asic-common.dtsi"
> +#include "elba-flash-parts.dtsi"
> diff --git a/arch/arm64/boot/dts/pensando/elba-flash-parts.dtsi b/arch/arm64/boot/dts/pensando/elba-flash-parts.dtsi
> new file mode 100644
> index 000000000000..e69734c2c267
> --- /dev/null
> +++ b/arch/arm64/boot/dts/pensando/elba-flash-parts.dtsi
> @@ -0,0 +1,103 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +&flash0 {
> +	partitions {
> +		compatible = "fixed-partitions";
> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +		partition@0 {
> +			label = "flash";
> +			reg = <0x10000 0xfff0000>;
> +		};
> +
> +		partition@f0000 {
> +			label = "golduenv";
> +			reg = <0xf0000 0x10000>;
> +		};
> +
> +		partition@100000 {
> +			label = "boot0";
> +			reg = <0x100000 0x80000>;
> +		};
> +
> +		partition@180000 {
> +			label = "golduboot";
> +			reg = <0x180000 0x200000>;
> +		};
> +
> +		partition@380000 {
> +			label = "brdcfg0";
> +			reg = <0x380000 0x10000>;
> +		};
> +
> +		partition@390000 {
> +			label = "brdcfg1";
> +			reg = <0x390000 0x10000>;
> +		};
> +
> +		partition@400000 {
> +			label = "goldfw";
> +			reg = <0x400000 0x3c00000>;
> +		};
> +
> +		partition@4010000 {
> +			label = "fwmap";
> +			reg = <0x4010000 0x20000>;
> +		};
> +
> +		partition@4030000 {
> +			label = "fwsel";
> +			reg = <0x4030000 0x20000>;
> +		};
> +
> +		partition@4090000 {
> +			label = "bootlog";
> +			reg = <0x4090000 0x20000>;
> +		};
> +
> +		partition@40b0000 {
> +			label = "panicbuf";
> +			reg = <0x40b0000 0x20000>;
> +		};
> +
> +		partition@40d0000 {
> +			label = "uservars";
> +			reg = <0x40d0000 0x20000>;
> +		};
> +
> +		partition@4200000 {
> +			label = "uboota";
> +			reg = <0x4200000 0x400000>;
> +		};
> +
> +		partition@4600000 {
> +			label = "ubootb";
> +			reg = <0x4600000 0x400000>;
> +		};
> +
> +		partition@4a00000 {
> +			label = "mainfwa";
> +			reg = <0x4a00000 0x1000000>;
> +		};
> +
> +		partition@5a00000 {
> +			label = "mainfwb";
> +			reg = <0x5a00000 0x1000000>;
> +		};
> +
> +		partition@6a00000 {
> +			label = "diaguboot";
> +			reg = <0x6a00000 0x400000>;
> +		};
> +
> +		partition@8000000 {
> +			label = "diagfw";
> +			reg = <0x8000000 0x7fe0000>;
> +		};
> +
> +		partition@ffe0000 {
> +			label = "ubootenv";
> +			reg = <0xffe0000 0x10000>;
> +		};
> +	};
> +};
> diff --git a/arch/arm64/boot/dts/pensando/elba.dtsi b/arch/arm64/boot/dts/pensando/elba.dtsi
> new file mode 100644
> index 000000000000..b28f69e0bd91
> --- /dev/null
> +++ b/arch/arm64/boot/dts/pensando/elba.dtsi
> @@ -0,0 +1,181 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright (c) 2019-2021, Pensando Systems Inc. */
> +
> +#include <dt-bindings/gpio/gpio.h>
> +#include "dt-bindings/interrupt-controller/arm-gic.h"
> +
> +/ {
> +	interrupt-parent = <&gic>;
> +	#address-cells = <2>;
> +	#size-cells = <2>;
> +
> +	dma-coherent;
> +
> +	ahb_clk: oscillator0 {
> +		compatible = "fixed-clock";
> +		#clock-cells = <0>;
> +	};
> +
> +	emmc_clk: oscillator2 {
> +		compatible = "fixed-clock";
> +		#clock-cells = <0>;
> +	};
> +
> +	flash_clk: oscillator3 {
> +		compatible = "fixed-clock";
> +		#clock-cells = <0>;
> +	};
> +
> +	ref_clk: oscillator4 {
> +		compatible = "fixed-clock";
> +		#clock-cells = <0>;
> +	};
> +
> +	timer {
> +		compatible = "arm,armv8-timer";
> +		interrupts = <GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(1) |
> +					IRQ_TYPE_LEVEL_LOW)>,
> +			     <GIC_PPI 14 (GIC_CPU_MASK_SIMPLE(1) |
> +					IRQ_TYPE_LEVEL_LOW)>,
> +			     <GIC_PPI 11 (GIC_CPU_MASK_SIMPLE(1) |
> +					IRQ_TYPE_LEVEL_LOW)>,
> +			     <GIC_PPI 10 (GIC_CPU_MASK_SIMPLE(1) |
> +					IRQ_TYPE_LEVEL_LOW)>;
> +	};
> +
> +	pmu {
> +		compatible = "arm,cortex-a72-pmu";
> +		interrupts = <GIC_PPI 7 (GIC_CPU_MASK_SIMPLE(1) |
> +				IRQ_TYPE_LEVEL_LOW)>;
> +	};
> +
> +	soc: soc {
> +		compatible = "simple-bus";
> +		#address-cells = <2>;
> +		#size-cells = <2>;
> +		ranges;
> +
> +		i2c0: i2c@400 {
> +			compatible = "snps,designware-i2c";
> +			reg = <0x0 0x400 0x0 0x100>;
> +			clocks = <&ahb_clk>;
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			i2c-sda-hold-time-ns = <480>;
> +			snps,sda-timeout-ms = <750>;
> +			interrupts = <GIC_SPI 4 IRQ_TYPE_LEVEL_HIGH>;
> +			status = "disabled";
> +		};
> +
> +		wdt0: watchdog@1400 {
> +			compatible = "snps,dw-wdt";
> +			reg = <0x0 0x1400 0x0 0x100>;
> +			clocks = <&ahb_clk>;
> +			interrupts = <GIC_SPI 10 IRQ_TYPE_LEVEL_HIGH>;
> +			status = "disabled";
> +		};
> +
> +		qspi: spi@2400 {
> +			compatible = "pensando,elba-qspi", "cdns,qspi-nor";
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			reg = <0x0 0x2400 0x0 0x400>,
> +			      <0x0 0x7fff0000 0x0 0x1000>;
> +			interrupts = <GIC_SPI 0 IRQ_TYPE_LEVEL_HIGH>;
> +			clocks = <&flash_clk>;
> +			cdns,fifo-depth = <1024>;
> +			cdns,fifo-width = <4>;
> +			cdns,trigger-address = <0x7fff0000>;
> +			status = "disabled";
> +		};
> +
> +		spi0: spi@2800 {
> +			compatible = "pensando,elba-spi";
> +			reg = <0x0 0x2800 0x0 0x100>;
> +			pensando,spics = <&mssoc 0x2468>;
> +			clocks = <&ahb_clk>;
> +			interrupts = <GIC_SPI 9 IRQ_TYPE_LEVEL_HIGH>;
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			num-cs = <2>;
> +			status = "disabled";
> +		};
> +
> +		gpio0: gpio@4000 {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			compatible = "snps,dw-apb-gpio";
> +			reg = <0x0 0x4000 0x0 0x78>;
> +			status = "disabled";
> +
> +			porta: gpio-port@0 {
> +				compatible = "snps,dw-apb-gpio-port";
> +				reg = <0>;
> +				gpio-controller;
> +				#gpio-cells = <2>;
> +				ngpios = <8>;
> +				interrupts = <GIC_SPI 1 IRQ_TYPE_LEVEL_HIGH>;
> +				interrupt-controller;
> +				interrupt-parent = <&gic>;
> +				#interrupt-cells = <2>;
> +			};
> +
> +			portb: gpio-port@1 {
> +				compatible = "snps,dw-apb-gpio-port";
> +				reg = <1>;
> +				gpio-controller;
> +				#gpio-cells = <2>;
> +				ngpios = <8>;
> +			};
> +		};
> +
> +		uart0: serial@4800 {
> +			compatible = "ns16550a";
> +			reg = <0x0 0x4800 0x0 0x100>;
> +			clocks = <&ref_clk>;
> +			interrupts = <GIC_SPI 2 IRQ_TYPE_LEVEL_HIGH>;
> +			reg-shift = <2>;
> +			reg-io-width = <4>;
> +		};
> +
> +		gic: interrupt-controller@800000 {
> +			compatible = "arm,gic-v3";
> +			#interrupt-cells = <3>;
> +			#address-cells = <2>;
> +			#size-cells = <2>;
> +			ranges;
> +			interrupt-controller;
> +			reg = <0x0 0x800000 0x0 0x200000>,	/* GICD */
> +			      <0x0 0xa00000 0x0 0x200000>;	/* GICR */
> +			interrupts = <GIC_PPI 9 IRQ_TYPE_LEVEL_HIGH>;
> +
> +			gic_its: msi-controller@820000 {
> +				compatible = "arm,gic-v3-its";
> +				msi-controller;
> +				#msi-cells = <1>;
> +				reg = <0x0 0x820000 0x0 0x10000>;
> +				socionext,synquacer-pre-its =
> +							<0xc00000 0x1000000>;
> +			};
> +		};
> +
> +		emmc: mmc@30440000 {
> +			compatible = "pensando,elba-emmc", "cdns,sd4hc";
> +			clocks = <&emmc_clk>;
> +			interrupts = <GIC_SPI 26 IRQ_TYPE_LEVEL_HIGH>;
> +			reg = <0x0 0x30440000 0x0 0x10000>,
> +			      <0x0 0x30480044 0x0 0x4>;    /* byte-lane ctrl */
> +			cdns,phy-input-delay-sd-highspeed = <0x4>;
> +			cdns,phy-input-delay-legacy = <0x4>;
> +			cdns,phy-input-delay-sd-uhs-sdr50 = <0x6>;
> +			cdns,phy-input-delay-sd-uhs-ddr50 = <0x16>;
> +			mmc-ddr-1_8v;
> +			status = "disabled";
> +		};
> +
> +		mssoc: mssoc@307c0000 {
> +			compatible = "syscon", "simple-mfd";
> +			reg = <0x0 0x307c0000 0x0 0x3000>;
> +		};
> +	};
> +};
> -- 
> 2.17.1
> 
> 

Re: [PATCH v3 01/11] dt-bindings: arm: pensando: add Pensando boards

[Rob Herring]

On Sun, Oct 24, 2021 at 06:51:46PM -0700, Brad Larson wrote:
> Document the compatible for Pensando Elba SoC boards.
> 
> Signed-off-by: Brad Larson <brad@pensando.io>
> ---
>  .../bindings/arm/pensando,elba.yaml           | 20 +++++++++++++++++++
>  1 file changed, 20 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/arm/pensando,elba.yaml
> 
> diff --git a/Documentation/devicetree/bindings/arm/pensando,elba.yaml b/Documentation/devicetree/bindings/arm/pensando,elba.yaml
> new file mode 100644
> index 000000000000..84bd9e7e98e9
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm/pensando,elba.yaml
> @@ -0,0 +1,20 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/arm/pensando,elba.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Pensando Elba SoC Platforms Device Tree Bindings
> +
> +maintainers:
> +  - Brad Larson  <brad@pensando.io>
> +
> +properties:
> +  $nodename:
> +    const: "/"
> +  compatible:
> +    const: pensando,elba

Going to need board compatibles.

> +
> +additionalProperties: true
> +
> +...
> -- 
> 2.17.1
> 
> 

Re: [PATCH v3 02/11] dt-bindings: Add vendor prefix for Pensando Systems

[Rob Herring]

On Sun, Oct 24, 2021 at 06:51:47PM -0700, Brad Larson wrote:
> Add vendor prefix for Pensando Systems: https://pensando.io

This should be before patch 1.

> 
> Signed-off-by: Brad Larson <brad@pensando.io>
> ---
>  Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++
>  1 file changed, 2 insertions(+)

Acked-by: Rob Herring <robh@kernel.org>

Re: [PATCH v3 04/11] dt-bindings: spi: Add compatible for Pensando Elba SoC

[Rob Herring]

On Sun, 24 Oct 2021 18:51:49 -0700, Brad Larson wrote:
> Document the cadence qspi controller compatible for Pensando Elba SoC
> boards.  The Elba qspi fifo size is 1024.
> 
> Signed-off-by: Brad Larson <brad@pensando.io>
> ---
>  Documentation/devicetree/bindings/spi/cdns,qspi-nor.yaml | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 

Acked-by: Rob Herring <robh@kernel.org>

Re: [PATCH v3 05/11] spi: dw: Add Pensando Elba SoC SPI Controller bindings

[Rob Herring]

On Sun, 24 Oct 2021 18:51:50 -0700, Brad Larson wrote:
> The Pensando Elba SoC has integrated the DW APB SPI Controller
> 
> Signed-off-by: Brad Larson <brad@pensando.io>
> ---
>  Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml | 2 ++
>  1 file changed, 2 insertions(+)
> 

Acked-by: Rob Herring <robh@kernel.org>

Re: [PATCH v3 04/11] dt-bindings: spi: Add compatible for Pensando Elba SoC

[Serge Semin]

Hello Brad

The patch name "dt-bindings: spi: Add compatible for Pensando Elba
SoC" doesn't mention to what bindings it is referring to. For the sake
of having a more representative git log I'd suggest at least to add
"cdns" vendor name in the title, like: "dt-bindings: spi: cdns: Add ..."
Otherwise it's impossible to understand to what bindings you're adding
a new compatibility especially seeing you are doing the similar thing
for the DW SPI in the next patch.

-Sergey

On Sun, Oct 24, 2021 at 06:51:49PM -0700, Brad Larson wrote:
> Document the cadence qspi controller compatible for Pensando Elba SoC
> boards.  The Elba qspi fifo size is 1024.
> 
> Signed-off-by: Brad Larson <brad@pensando.io>
> ---
>  Documentation/devicetree/bindings/spi/cdns,qspi-nor.yaml | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/spi/cdns,qspi-nor.yaml b/Documentation/devicetree/bindings/spi/cdns,qspi-nor.yaml
> index 0e7087cc8bf9..d4413eced17a 100644
> --- a/Documentation/devicetree/bindings/spi/cdns,qspi-nor.yaml
> +++ b/Documentation/devicetree/bindings/spi/cdns,qspi-nor.yaml
> @@ -20,6 +20,7 @@ properties:
>                - ti,k2g-qspi
>                - ti,am654-ospi
>                - intel,lgm-qspi
> +              - pensando,elba-qspi
>            - const: cdns,qspi-nor
>        - const: cdns,qspi-nor
>  
> @@ -38,7 +39,7 @@ properties:
>      description:
>        Size of the data FIFO in words.
>      $ref: "/schemas/types.yaml#/definitions/uint32"
> -    enum: [ 128, 256 ]
> +    enum: [ 128, 256, 1024 ]
>      default: 128
>  
>    cdns,fifo-width:
> -- 
> 2.17.1
> 

Re: [PATCH v3 05/11] spi: dw: Add Pensando Elba SoC SPI Controller bindings

[Serge Semin]

On Sun, Oct 24, 2021 at 06:51:50PM -0700, Brad Larson wrote:
> The Pensando Elba SoC has integrated the DW APB SPI Controller

Please add the "dt-bindings: " prefix to the patch name and discard
the word "bindings" from the title as the submitting DT-patches
requires:
Documentation/devicetree/bindings/submitting-patches.rst

> 
> Signed-off-by: Brad Larson <brad@pensando.io>
> ---
>  Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
> index d7e08b03e204..0b5ebb2ae6e7 100644
> --- a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
> +++ b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
> @@ -73,6 +73,8 @@ properties:
>                - renesas,r9a06g032-spi # RZ/N1D
>                - renesas,r9a06g033-spi # RZ/N1S
>            - const: renesas,rzn1-spi   # RZ/N1

> +      - description: Pensando Elba SoC SPI Controller
> +        const: pensando,elba-spi

AFAICS from the driver-part of the patchset it's not enough. You've
also got the syscon phandle, which needs to be reflected in the
bindings. That also makes me thinking that you didn't perform the
"dtbs_check" on the dts-files you were going to submit, but for some
reason discarded from this series (btw why?). If you did you would
have got an error of an unevaluated property detection.

-Sergey

>  
>    reg:
>      minItems: 1
> -- 
> 2.17.1
> 

Re: [PATCH v3 05/11] spi: dw: Add Pensando Elba SoC SPI Controller bindings

[Serge Semin]

On Thu, Oct 28, 2021 at 10:49:48AM +0300, Serge Semin wrote:
> On Sun, Oct 24, 2021 at 06:51:50PM -0700, Brad Larson wrote:
> > The Pensando Elba SoC has integrated the DW APB SPI Controller
> 
> Please add the "dt-bindings: " prefix to the patch name and discard
> the word "bindings" from the title as the submitting DT-patches
> requires:
> Documentation/devicetree/bindings/submitting-patches.rst
> 
> > 
> > Signed-off-by: Brad Larson <brad@pensando.io>
> > ---
> >  Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
> > index d7e08b03e204..0b5ebb2ae6e7 100644
> > --- a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
> > +++ b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
> > @@ -73,6 +73,8 @@ properties:
> >                - renesas,r9a06g032-spi # RZ/N1D
> >                - renesas,r9a06g033-spi # RZ/N1S
> >            - const: renesas,rzn1-spi   # RZ/N1
> 
> > +      - description: Pensando Elba SoC SPI Controller
> > +        const: pensando,elba-spi
> 
> AFAICS from the driver-part of the patchset it's not enough. You've
> also got the syscon phandle, which needs to be reflected in the
> bindings. That also makes me thinking that you didn't perform the
> "dtbs_check" on the dts-files you were going to submit,

> but for some reason discarded from this series (btw why?).

Oops. Found it. The question regarding "dtbs_check" is still actual.

-Sergey

> If you did you would
> have got an error of an unevaluated property detection.
> 
> -Sergey
> 
> >  
> >    reg:
> >      minItems: 1
> > -- 
> > 2.17.1
> > 

Re: [PATCH v3 10/11] spi: dw: Add support for Pensando Elba SoC

[Serge Semin]

On Sun, Oct 24, 2021 at 06:51:55PM -0700, Brad Larson wrote:
> The Pensando Elba SoC includes a DW apb_ssi v4 controller
> with device specific chip-select control.  The Elba SoC
> provides four chip-selects where the native DW IP supports
> two chip-selects.
> 
> Signed-off-by: Brad Larson <brad@pensando.io>
> ---
> Changelog:
> - Changed the implementation to use existing dw_spi_set_cs() and
>   integrated Elba specific CS control into spi-dw-mmio.c.  The
>   native designware support is for two chip-selects while Elba
>   provides 4 chip-selects.  Instead of adding a new file for
>   this support in gpio-elba-spics.c the support is in one
>   file (spi-dw-mmio.c).
> 
>  drivers/spi/spi-dw-mmio.c | 85 +++++++++++++++++++++++++++++++++++++++
>  1 file changed, 85 insertions(+)
> 
> diff --git a/drivers/spi/spi-dw-mmio.c b/drivers/spi/spi-dw-mmio.c
> index 3379720cfcb8..fe7b595fe33d 100644
> --- a/drivers/spi/spi-dw-mmio.c
> +++ b/drivers/spi/spi-dw-mmio.c
> @@ -53,6 +53,24 @@ struct dw_spi_mscc {
>  	void __iomem        *spi_mst; /* Not sparx5 */
>  };
>  
> +struct dw_spi_elba {
> +	struct regmap *regmap;
> +	unsigned int reg;
> +};
> +
> +/*

> + * Elba SoC does not use ssi, pin override is used for cs 0,1 and
> + * gpios for cs 2,3 as defined in the device tree.

I believe GPIO-based CS is the platform-property rather than the SoC
one. It's up to the board designers which GPIOs to use as a custom
chip-select signal. Thus it would be better to discard the comment
regarding the GPIOs here.

> + *
> + * cs:  |       1               0
> + * bit: |---3-------2-------1-------0
> + *      |  cs1   cs1_ovr   cs0   cs0_ovr
> + */
> +#define ELBA_SPICS_SHIFT(cs)		(2 * (cs))
> +#define ELBA_SPICS_MASK(cs)		(0x3 << ELBA_SPICS_SHIFT(cs))
> +#define ELBA_SPICS_SET(cs, val)	\
> +			((((val) << 1) | 0x1) << ELBA_SPICS_SHIFT(cs))
> +
>  /*
>   * The Designware SPI controller (referred to as master in the documentation)
>   * automatically deasserts chip select when the tx fifo is empty. The chip
> @@ -237,6 +255,72 @@ static int dw_spi_canaan_k210_init(struct platform_device *pdev,
>  	return 0;
>  }
>  
> +static void elba_spics_set_cs(struct dw_spi_elba *dwselba, int cs, int enable)
> +{
> +	regmap_update_bits(dwselba->regmap, dwselba->reg, ELBA_SPICS_MASK(cs),
> +			   ELBA_SPICS_SET(cs, enable));
> +}
> +
> +static void dw_spi_elba_set_cs(struct spi_device *spi, bool enable)
> +{
> +	struct dw_spi *dws = spi_master_get_devdata(spi->master);
> +	struct dw_spi_mmio *dwsmmio = container_of(dws, struct dw_spi_mmio, dws);
> +	struct dw_spi_elba *dwselba = dwsmmio->priv;
> +	u8 cs = spi->chip_select;
> +
> +	if (cs < 2) {
> +		/* overridden native chip-select */
> +		elba_spics_set_cs(dwselba, spi->chip_select, enable);
> +	}
> +
> +	/*
> +	 * The DW SPI controller needs a native CS bit selected to start

> +	 * the serial engine, and we have fewer native CSs than we need, so
                                  ^
                                  + "the platform may have ..."

> +	 * use CS0 always.
> +	 */
> +	spi->chip_select = 0;
> +	dw_spi_set_cs(spi, enable);
> +	spi->chip_select = cs;

Is it correct to think that the DW SSI output CS signals are
multiplexed between the native DW SSI CS logic and the logic
implemented in the ELBA SPICS syscon? Thus by setting "csX_ovr" in the
ELBA_SPICS CSR do you get to switch between the DW SSI SER logic and
the signal level selected by the "csX" field of that register?

* Most likely I already asked this question in v2 but it was long time
ago, so it's better to clarify things over.

> +}
> +
> +static int dw_spi_elba_init(struct platform_device *pdev,
> +			    struct dw_spi_mmio *dwsmmio)
> +{
> +	struct of_phandle_args args;
> +	struct dw_spi_elba *dwselba;
> +	struct regmap *regmap;
> +	int rc;
> +

> +	rc = of_parse_phandle_with_fixed_args(pdev->dev.of_node,
> +			"pensando,spics", 1, 0, &args);
> +	if (rc) {
> +		dev_err(&pdev->dev, "could not find pensando,spics\n");
> +		return rc;
> +	}
> +
> +	regmap = syscon_node_to_regmap(args.np);
> +	if (IS_ERR(regmap)) {
> +		dev_err(&pdev->dev, "could not map pensando,spics\n");
> +		return PTR_ERR(regmap);
> +	}

There is a good wrapper for this: syscon_regmap_lookup_by_phandle_args() .

The property name isn't well descriptive in the syscon-related
part. Could you add something like:
"pensando,elba-syscon-spics"/"pensando,syscon-spics"?

-Sergey

> +
> +	dwselba = devm_kzalloc(&pdev->dev, sizeof(*dwselba), GFP_KERNEL);
> +	if (!dwselba)
> +		return -ENOMEM;
> +
> +	dwselba->regmap = regmap;
> +	dwselba->reg = args.args[0];
> +
> +	/* deassert cs */
> +	elba_spics_set_cs(dwselba, 0, 1);
> +	elba_spics_set_cs(dwselba, 1, 1);
> +
> +	dwsmmio->priv = dwselba;
> +	dwsmmio->dws.set_cs = dw_spi_elba_set_cs;
> +
> +	return 0;
> +}
> +
>  static int dw_spi_mmio_probe(struct platform_device *pdev)
>  {
>  	int (*init_func)(struct platform_device *pdev,
> @@ -351,6 +435,7 @@ static const struct of_device_id dw_spi_mmio_of_match[] = {
>  	{ .compatible = "intel,keembay-ssi", .data = dw_spi_keembay_init},
>  	{ .compatible = "microchip,sparx5-spi", dw_spi_mscc_sparx5_init},
>  	{ .compatible = "canaan,k210-spi", dw_spi_canaan_k210_init},
> +	{ .compatible = "pensando,elba-spi", .data = dw_spi_elba_init},
>  	{ /* end of table */}
>  };
>  MODULE_DEVICE_TABLE(of, dw_spi_mmio_of_match);
> -- 
> 2.17.1
> 

Re: [PATCH v3 10/11] spi: dw: Add support for Pensando Elba SoC

[Andy Shevchenko]

On Mon, Oct 25, 2021 at 4:54 AM Brad Larson <brad@pensando.io> wrote:
>
> The Pensando Elba SoC includes a DW apb_ssi v4 controller
> with device specific chip-select control.  The Elba SoC
> provides four chip-selects where the native DW IP supports
> two chip-selects.

...

> +static void dw_spi_elba_set_cs(struct spi_device *spi, bool enable)
> +{
> +       struct dw_spi *dws = spi_master_get_devdata(spi->master);
> +       struct dw_spi_mmio *dwsmmio = container_of(dws, struct dw_spi_mmio, dws);
> +       struct dw_spi_elba *dwselba = dwsmmio->priv;

> +       u8 cs = spi->chip_select;

Much easier to maintain and follow the code (in the future) if this
assignment is broken to two parts, i.e...

> +

...like this

       cs = spi->chip_select;
       if (cs < 2) {
         ...

> +       if (cs < 2) {
> +               /* overridden native chip-select */
> +               elba_spics_set_cs(dwselba, spi->chip_select, enable);
> +       }

...

> +       regmap = syscon_node_to_regmap(args.np);
> +       if (IS_ERR(regmap)) {
> +               dev_err(&pdev->dev, "could not map pensando,spics\n");
> +               return PTR_ERR(regmap);
> +       }

Why not return dev_err_probe()?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 11/11] arm64: dts: Add Pensando Elba SoC support

[Brad Larson]

Hi Mark,

On Mon, Oct 25, 2021 at 2:17 AM Mark Rutland <mark.rutland@arm.com> wrote:
>
> Hi,
>
> On Sun, Oct 24, 2021 at 06:51:56PM -0700, Brad Larson wrote:
> > Add Pensando common and Elba SoC specific device nodes
> >
> > Signed-off-by: Brad Larson <brad@pensando.io>
>
> [...]
>
> > +     timer {
> > +             compatible = "arm,armv8-timer";
> > +             interrupts = <GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(1) |
> > +                                     IRQ_TYPE_LEVEL_LOW)>,
> > +                          <GIC_PPI 14 (GIC_CPU_MASK_SIMPLE(1) |
> > +                                     IRQ_TYPE_LEVEL_LOW)>,
> > +                          <GIC_PPI 11 (GIC_CPU_MASK_SIMPLE(1) |
> > +                                     IRQ_TYPE_LEVEL_LOW)>,
> > +                          <GIC_PPI 10 (GIC_CPU_MASK_SIMPLE(1) |
> > +                                     IRQ_TYPE_LEVEL_LOW)>;
> > +     };
>
> The GIC_CPU_MASK_SIMPLE() stuff is meant for GICv2, but as below you
> have GICv3, where this is not valid, so this should go.
>
> Also, beware that GIC_CPU_MASK_SIMPLE(1) means a single CPU, which
> doesn't mak sense for the 16 CPUs you have.
>

Thanks for pointing this out.  Elba SoC is a GICv3 implementation and looking
at other device tree files we should be using this:

        timer {
                compatible = "arm,armv8-timer";
                interrupts = <GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(16) |
                                        IRQ_TYPE_LEVEL_LOW)>,
                             <GIC_PPI 14 (GIC_CPU_MASK_SIMPLE(16) |
                                        IRQ_TYPE_LEVEL_LOW)>,
                             <GIC_PPI 11 (GIC_CPU_MASK_SIMPLE(16) |
                                        IRQ_TYPE_LEVEL_LOW)>,
                             <GIC_PPI 10 (GIC_CPU_MASK_SIMPLE(16) |
                                        IRQ_TYPE_LEVEL_LOW)>;
        };

> > +             gic: interrupt-controller@800000 {
> > +                     compatible = "arm,gic-v3";
> > +                     #interrupt-cells = <3>;
> > +                     #address-cells = <2>;
> > +                     #size-cells = <2>;
> > +                     ranges;
> > +                     interrupt-controller;
> > +                     reg = <0x0 0x800000 0x0 0x200000>,      /* GICD */
> > +                           <0x0 0xa00000 0x0 0x200000>;      /* GICR */
> > +                     interrupts = <GIC_PPI 9 IRQ_TYPE_LEVEL_HIGH>;
> > +
> > +                     gic_its: msi-controller@820000 {
> > +                             compatible = "arm,gic-v3-its";
> > +                             msi-controller;
> > +                             #msi-cells = <1>;
> > +                             reg = <0x0 0x820000 0x0 0x10000>;
> > +                             socionext,synquacer-pre-its =
> > +                                                     <0xc00000 0x1000000>;
> > +                     };
> > +             };
>
> Is there any shared lineage with Synquacer? The commit message didn't
> describe this quirk.

There is no shared lineage with Synqacer.  We are solving the same issue
with the same mechanism.  I'll add a comment to this DTS node.

Thanks,
Brad

Re: [PATCH v3 11/11] arm64: dts: Add Pensando Elba SoC support

[Brad Larson]

Hi Marc,

On Mon, Oct 25, 2021 at 4:15 AM Marc Zyngier <maz@kernel.org> wrote:
>
> On 2021-10-25 10:17, Mark Rutland wrote:
> > Hi,
> >
> > On Sun, Oct 24, 2021 at 06:51:56PM -0700, Brad Larson wrote:
> >> Add Pensando common and Elba SoC specific device nodes
> >>
> >> Signed-off-by: Brad Larson <brad@pensando.io>
> >
> > [...]
> >> +            gic: interrupt-controller@800000 {
> >> +                    compatible = "arm,gic-v3";
> >> +                    #interrupt-cells = <3>;
> >> +                    #address-cells = <2>;
> >> +                    #size-cells = <2>;
> >> +                    ranges;
> >> +                    interrupt-controller;
> >> +                    reg = <0x0 0x800000 0x0 0x200000>,      /* GICD */
> >> +                          <0x0 0xa00000 0x0 0x200000>;      /* GICR */
>
> This is missing the GICv2 compat regions that the CPUs implement.

Is this what is described as optional in the GIC architecture specification
where a GICv3 system can run restricted GICv2 code?  Can you point
me in the right direction in the spec and example dts node if needed.

> >> +                    interrupts = <GIC_PPI 9 IRQ_TYPE_LEVEL_HIGH>;
> >> +
> >> +                    gic_its: msi-controller@820000 {
> >> +                            compatible = "arm,gic-v3-its";
> >> +                            msi-controller;
> >> +                            #msi-cells = <1>;
> >> +                            reg = <0x0 0x820000 0x0 0x10000>;
> >> +                            socionext,synquacer-pre-its =
> >> +                                                    <0xc00000 0x1000000>;
> >> +                    };
> >> +            };
> >
> > Is there any shared lineage with Synquacer? The commit message didn't
> > describe this quirk.
>
> Funny, it looks like there is a sudden outburst of stupid copy/paste
> among HW designers. TI did the exact same thing recently.
>
> This totally negates all the advantages of having an ITS and makes
> sure that you have all the overhead. Facepalm...

Some background may help explain.  To generate an LPI a peripheral must
write to the GITS_TRANSLATER (a specific address). For the ITS to know
which translations apply to the generated interrupts, it must know which
peripheral performed the write. The ID of the peripheral is known as its
DeviceID, which is often carried along with the write as an AXI sideband
signal.

The Elba SoC doesn't carry the DeviceID, so we have to conjure one up
between the peripheral and the ITS.  Instead of telling a peripheral to target
the GITS_TRANSLATER directly, we instead direct it to a specific offset
within a pre-ITS address range (our own IP block).  For writes that land in
that memory range, we derive the DeviceID from (offset >> 2).  The pre-ITS
block then sends (DeviceID, data) to the GITS_TRANSLATER.

The hardware designer came up with the Pre-ITS mechanism in Feb 2018.
When we looked at the upstream kernel later (we developed on 4.14)
we found that not only did it support something similar, it supported the
exact scheme we are using.

Thanks,
Brad

Re: [PATCH v3 03/11] dt-bindings: mmc: Add Pensando Elba SoC binding

[Brad Larson]

Hi Rob,

On Mon, Oct 25, 2021 at 5:54 AM Rob Herring <robh@kernel.org> wrote:
>
> On Sun, 24 Oct 2021 18:51:48 -0700, Brad Larson wrote:
> > Pensando Elba ARM 64-bit SoC is integrated with this IP and
> > explicitly controls byte-lane enables resulting in an additional
> > reg property resource.
> >
> > Signed-off-by: Brad Larson <brad@pensando.io>
> > ---
> >  .../devicetree/bindings/mmc/cdns,sdhci.yaml         | 13 ++++++++-----
> >  1 file changed, 8 insertions(+), 5 deletions(-)
> >
>
> My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
> on your patch (DT_CHECKER_FLAGS is new in v5.13):
>
> yamllint warnings/errors:
> ./Documentation/devicetree/bindings/mmc/cdns,sdhci.yaml:20:9: [warning] wrong indentation: expected 10 but found 8 (indentation)
>
> dtschema/dtc warnings/errors:
>
> doc reference errors (make refcheckdocs):
>
> See https://patchwork.ozlabs.org/patch/1545481
>
> This check can fail if there are any dependencies. The base for a patch
> series is generally the most recent rc1.
>
> If you already ran 'make dt_binding_check' and didn't see the above
> error(s), then make sure 'yamllint' is installed and dt-schema is up to
> date:
>
> pip3 install dtschema --upgrade
>
> Please check and re-submit.
>

yamllint was not installed, it is now and dtschema is updated to run again
before re-submit.

Thanks,
Brad

Re: [PATCH v3 02/11] dt-bindings: Add vendor prefix for Pensando Systems

[Brad Larson]

On Wed, Oct 27, 2021 at 2:38 PM Rob Herring <robh@kernel.org> wrote:
>
> On Sun, Oct 24, 2021 at 06:51:47PM -0700, Brad Larson wrote:
> > Add vendor prefix for Pensando Systems: https://pensando.io
>
> This should be before patch 1.

ack, will order this way for re-spin of the patch set.

Thanks
Brad

Re: [PATCH v3 11/11] arm64: dts: Add Pensando Elba SoC support

[Marc Zyngier]

Brad,

On Fri, 05 Nov 2021 00:02:04 +0000,
Brad Larson <brad@pensando.io> wrote:
> 
> Hi Marc,
> 
> On Mon, Oct 25, 2021 at 4:15 AM Marc Zyngier <maz@kernel.org> wrote:
> >
> > On 2021-10-25 10:17, Mark Rutland wrote:
> > > Hi,
> > >
> > > On Sun, Oct 24, 2021 at 06:51:56PM -0700, Brad Larson wrote:
> > >> Add Pensando common and Elba SoC specific device nodes
> > >>
> > >> Signed-off-by: Brad Larson <brad@pensando.io>
> > >
> > > [...]
> > >> +            gic: interrupt-controller@800000 {
> > >> +                    compatible = "arm,gic-v3";
> > >> +                    #interrupt-cells = <3>;
> > >> +                    #address-cells = <2>;
> > >> +                    #size-cells = <2>;
> > >> +                    ranges;
> > >> +                    interrupt-controller;
> > >> +                    reg = <0x0 0x800000 0x0 0x200000>,      /* GICD */
> > >> +                          <0x0 0xa00000 0x0 0x200000>;      /* GICR */
> >
> > This is missing the GICv2 compat regions that the CPUs implement.
> 
> Is this what is described as optional in the GIC architecture specification
> where a GICv3 system can run restricted GICv2 code?

Yup, that. It is actually implemented by the CPU.

> Can you point me in the right direction in the spec and example dts
> node if needed.

The Cortex-A72 TRM has everything you need [1]. And since you used the
Synquacer as the model for this, you will see that it has the missing
regions. Alternatively, rk3399.dtsi is a good example.


> > >> +                    interrupts = <GIC_PPI 9 IRQ_TYPE_LEVEL_HIGH>;
> > >> +
> > >> +                    gic_its: msi-controller@820000 {
> > >> +                            compatible = "arm,gic-v3-its";
> > >> +                            msi-controller;
> > >> +                            #msi-cells = <1>;
> > >> +                            reg = <0x0 0x820000 0x0 0x10000>;
> > >> +                            socionext,synquacer-pre-its =
> > >> +                                                    <0xc00000 0x1000000>;
> > >> +                    };
> > >> +            };
> > >
> > > Is there any shared lineage with Synquacer? The commit message didn't
> > > describe this quirk.
> >
> > Funny, it looks like there is a sudden outburst of stupid copy/paste
> > among HW designers. TI did the exact same thing recently.
> >
> > This totally negates all the advantages of having an ITS and makes
> > sure that you have all the overhead. Facepalm...
> 
> Some background may help explain.  To generate an LPI a peripheral must
> write to the GITS_TRANSLATER (a specific address). For the ITS to know
> which translations apply to the generated interrupts, it must know which
> peripheral performed the write. The ID of the peripheral is known as its
> DeviceID, which is often carried along with the write as an AXI sideband
> signal.

Yes, I happen to be vaguely familiar with the GIC architecture.

> The Elba SoC doesn't carry the DeviceID, so we have to conjure one up
> between the peripheral and the ITS.  Instead of telling a peripheral to target
> the GITS_TRANSLATER directly, we instead direct it to a specific offset
> within a pre-ITS address range (our own IP block).  For writes that land in
> that memory range, we derive the DeviceID from (offset >> 2).  The pre-ITS
> block then sends (DeviceID, data) to the GITS_TRANSLATER.
> 
> The hardware designer came up with the Pre-ITS mechanism in Feb 2018.
> When we looked at the upstream kernel later (we developed on 4.14)
> we found that not only did it support something similar, it supported the
> exact scheme we are using.

And this scheme is totally wrong. It breaks interrupt isolation.

Instead of having a single doorbell and getting the ITS to segregate
between devices itself, you end-up with multiple ones, allowing a
rogue device to impersonate another one by targeting another doorbell.
You can't even use an SMMU to preserve some isolation, because all the
doorbells are in the *same page*. Unmitigated disaster.

At this stage, why did you bother having an ITS at all? You get none
of the security features. Only the excess area, memory allocation,
additional latency and complexity. All you get is a larger INTID
space.

This only shows that the hardware designer didn't understand the ITS
at all. Which seems a common pattern, unfortunately.

	M.

[1] https://developer.arm.com/documentation/100095/0003/Generic-Interrupt-Controller-CPU-Interface/GIC-functional-description/GIC-memory-map?lang=en#way1382452674438__CHDEBJAJ

-- 
Without deviation from the norm, progress is not possible.

Re: [PATCH v3 11/11] arm64: dts: Add Pensando Elba SoC support

[Mark Rutland]

On Thu, Nov 04, 2021 at 03:53:13PM -0700, Brad Larson wrote:
> On Mon, Oct 25, 2021 at 2:17 AM Mark Rutland <mark.rutland@arm.com> wrote:
> > On Sun, Oct 24, 2021 at 06:51:56PM -0700, Brad Larson wrote:
> > > +     timer {
> > > +             compatible = "arm,armv8-timer";
> > > +             interrupts = <GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(1) |
> > > +                                     IRQ_TYPE_LEVEL_LOW)>,
> > > +                          <GIC_PPI 14 (GIC_CPU_MASK_SIMPLE(1) |
> > > +                                     IRQ_TYPE_LEVEL_LOW)>,
> > > +                          <GIC_PPI 11 (GIC_CPU_MASK_SIMPLE(1) |
> > > +                                     IRQ_TYPE_LEVEL_LOW)>,
> > > +                          <GIC_PPI 10 (GIC_CPU_MASK_SIMPLE(1) |
> > > +                                     IRQ_TYPE_LEVEL_LOW)>;
> > > +     };
> >
> > The GIC_CPU_MASK_SIMPLE() stuff is meant for GICv2, but as below you
> > have GICv3, where this is not valid, so this should go.
> >
> > Also, beware that GIC_CPU_MASK_SIMPLE(1) means a single CPU, which
> > doesn't mak sense for the 16 CPUs you have.
> >
> 
> Thanks for pointing this out.  Elba SoC is a GICv3 implementation and looking
> at other device tree files we should be using this:
> 
>         timer {
>                 compatible = "arm,armv8-timer";
>                 interrupts = <GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(16) |
>                                         IRQ_TYPE_LEVEL_LOW)>,
>                              <GIC_PPI 14 (GIC_CPU_MASK_SIMPLE(16) |
>                                         IRQ_TYPE_LEVEL_LOW)>,
>                              <GIC_PPI 11 (GIC_CPU_MASK_SIMPLE(16) |
>                                         IRQ_TYPE_LEVEL_LOW)>,
>                              <GIC_PPI 10 (GIC_CPU_MASK_SIMPLE(16) |
>                                         IRQ_TYPE_LEVEL_LOW)>;
>         };

No; as above, you should *not* use GIC_CPU_MASK_SIMPLE() at all for GICv3. i.e.

>         timer {
>                 compatible = "arm,armv8-timer";
>                 interrupts = <GIC_PPI 13 IRQ_TYPE_LEVEL_LOW>,
>                              <GIC_PPI 14 IRQ_TYPE_LEVEL_LOW>,
>                              <GIC_PPI 11 IRQ_TYPE_LEVEL_LOW>,
>                              <GIC_PPI 10 IRQ_TYPE_LEVEL_LOW>;
>         };

Please see the GICv3 binding documentation:

  Documentation/devicetree/bindings/interrupt-controller/arm,gic-v3.yaml

... and note that it does not have the cpumask field as use by the binding for
prior generations of GIC:

  Documentation/devicetree/bindings/interrupt-controller/arm,gic.yaml


If you've seen other dts files using GIC_CPU_MASK_SIMPLE() with GICv3, those
are incorrect, and need to be fixed.

Thanks,
Mark.

Re: [PATCH v3 11/11] arm64: dts: Add Pensando Elba SoC support

[Brad Larson]

Hi Mark,

On Mon, Nov 8, 2021 at 2:26 AM Mark Rutland <mark.rutland@arm.com> wrote:
>
> No; as above, you should *not* use GIC_CPU_MASK_SIMPLE() at all for GICv3. i.e.
>
> >         timer {
> >                 compatible = "arm,armv8-timer";
> >                 interrupts = <GIC_PPI 13 IRQ_TYPE_LEVEL_LOW>,
> >                              <GIC_PPI 14 IRQ_TYPE_LEVEL_LOW>,
> >                              <GIC_PPI 11 IRQ_TYPE_LEVEL_LOW>,
> >                              <GIC_PPI 10 IRQ_TYPE_LEVEL_LOW>;
> >         };
>
> Please see the GICv3 binding documentation:
>
>   Documentation/devicetree/bindings/interrupt-controller/arm,gic-v3.yaml
>
> ... and note that it does not have the cpumask field as use by the binding for
> prior generations of GIC:
>
>   Documentation/devicetree/bindings/interrupt-controller/arm,gic.yaml
>
>
> If you've seen other dts files using GIC_CPU_MASK_SIMPLE() with GICv3, those
> are incorrect, and need to be fixed.
>
> Thanks,
> Mark.

I'll use the bindings documentation as the primary reference.  The use of
GIC_CPU_MASK_SIMPLE() is removed and tests ok.  These arm64 dts files in
linux-next are gic-v3 and use GIC_CPU_MASK_SIMPLE(1, 2, 4, 8)

./nvidia/tegra234.dtsi
./renesas/r9a07g044.dtsi
./renesas/r8a779a0.dtsi
./qcom/sm8350.dtsi
./qcom/sm8250.dtsi
./freescale/fsl-ls1028a.dtsi
./freescale/imx8mp.dtsi
./freescale/imx8mn.dtsi
./freescale/imx8mm.dtsi

Thanks,
Brad

Re: [PATCH v3 11/11] arm64: dts: Add Pensando Elba SoC support

[Brad Larson]

Hi Mark,

On Fri, Nov 5, 2021 at 4:35 AM Marc Zyngier <maz@kernel.org> wrote:
>
> > > >> +                    interrupts = <GIC_PPI 9 IRQ_TYPE_LEVEL_HIGH>;
> > > >> +
> > > >> +                    gic_its: msi-controller@820000 {
> > > >> +                            compatible = "arm,gic-v3-its";
> > > >> +                            msi-controller;
> > > >> +                            #msi-cells = <1>;
> > > >> +                            reg = <0x0 0x820000 0x0 0x10000>;
> > > >> +                            socionext,synquacer-pre-its =
> > > >> +                                                    <0xc00000 0x1000000>;
> > > >> +                    };
> > > >> +            };
> > > >
> > > > Is there any shared lineage with Synquacer? The commit message didn't
> > > > describe this quirk.
> > >
> > > Funny, it looks like there is a sudden outburst of stupid copy/paste
> > > among HW designers. TI did the exact same thing recently.
> > >
> > > This totally negates all the advantages of having an ITS and makes
> > > sure that you have all the overhead. Facepalm...
> >
> > Some background may help explain.  To generate an LPI a peripheral must
> > write to the GITS_TRANSLATER (a specific address). For the ITS to know
> > which translations apply to the generated interrupts, it must know which
> > peripheral performed the write. The ID of the peripheral is known as its
> > DeviceID, which is often carried along with the write as an AXI sideband
> > signal.
>
> Yes, I happen to be vaguely familiar with the GIC architecture.
>
> > The Elba SoC doesn't carry the DeviceID, so we have to conjure one up
> > between the peripheral and the ITS.  Instead of telling a peripheral to target
> > the GITS_TRANSLATER directly, we instead direct it to a specific offset
> > within a pre-ITS address range (our own IP block).  For writes that land in
> > that memory range, we derive the DeviceID from (offset >> 2).  The pre-ITS
> > block then sends (DeviceID, data) to the GITS_TRANSLATER.
> >
> > The hardware designer came up with the Pre-ITS mechanism in Feb 2018.
> > When we looked at the upstream kernel later (we developed on 4.14)
> > we found that not only did it support something similar, it supported the
> > exact scheme we are using.
>
> And this scheme is totally wrong. It breaks interrupt isolation.
>
> Instead of having a single doorbell and getting the ITS to segregate
> between devices itself, you end-up with multiple ones, allowing a
> rogue device to impersonate another one by targeting another doorbell.
> You can't even use an SMMU to preserve some isolation, because all the
> doorbells are in the *same page*. Unmitigated disaster.
>
> At this stage, why did you bother having an ITS at all? You get none
> of the security features. Only the excess area, memory allocation,
> additional latency and complexity. All you get is a larger INTID
> space.
>
> This only shows that the hardware designer didn't understand the ITS
> at all. Which seems a common pattern, unfortunately.

The Elba SoC is an embedded chip and not intended as a SBSA-compliant
general platform.  In this implementation the ITS is used to provide
message-based interrupts for our (potentially large set) of hardware
based platform device instances.  Virtualization is not a consideration.
We don't have a SMMU.  Interrupt isolation isn't a practical consideration
for this product.  Propose adding a comment to the dts.

+                       /*
+                        * Elba SoC implemented a pre-ITS that happened to
+                        * be the same implementation as synquacer.
+                        */
                        its: interrupt-controller@820000 {
                                compatible = "arm,gic-v3-its";
                                msi-controller;

Thanks
Brad

Re: [PATCH v3 11/11] arm64: dts: Add Pensando Elba SoC support

[Marc Zyngier]

On Mon, 08 Nov 2021 19:35:14 +0000,
Brad Larson <brad@pensando.io> wrote:
> 
> Hi Mark,

s/k/c/

> 
> On Fri, Nov 5, 2021 at 4:35 AM Marc Zyngier <maz@kernel.org> wrote:
> >
> > > > >> +                    interrupts = <GIC_PPI 9 IRQ_TYPE_LEVEL_HIGH>;
> > > > >> +
> > > > >> +                    gic_its: msi-controller@820000 {
> > > > >> +                            compatible = "arm,gic-v3-its";
> > > > >> +                            msi-controller;
> > > > >> +                            #msi-cells = <1>;
> > > > >> +                            reg = <0x0 0x820000 0x0 0x10000>;
> > > > >> +                            socionext,synquacer-pre-its =
> > > > >> +                                                    <0xc00000 0x1000000>;
> > > > >> +                    };
> > > > >> +            };
> > > > >
> > > > > Is there any shared lineage with Synquacer? The commit message didn't
> > > > > describe this quirk.
> > > >
> > > > Funny, it looks like there is a sudden outburst of stupid copy/paste
> > > > among HW designers. TI did the exact same thing recently.
> > > >
> > > > This totally negates all the advantages of having an ITS and makes
> > > > sure that you have all the overhead. Facepalm...
> > >
> > > Some background may help explain.  To generate an LPI a peripheral must
> > > write to the GITS_TRANSLATER (a specific address). For the ITS to know
> > > which translations apply to the generated interrupts, it must know which
> > > peripheral performed the write. The ID of the peripheral is known as its
> > > DeviceID, which is often carried along with the write as an AXI sideband
> > > signal.
> >
> > Yes, I happen to be vaguely familiar with the GIC architecture.
> >
> > > The Elba SoC doesn't carry the DeviceID, so we have to conjure one up
> > > between the peripheral and the ITS.  Instead of telling a peripheral to target
> > > the GITS_TRANSLATER directly, we instead direct it to a specific offset
> > > within a pre-ITS address range (our own IP block).  For writes that land in
> > > that memory range, we derive the DeviceID from (offset >> 2).  The pre-ITS
> > > block then sends (DeviceID, data) to the GITS_TRANSLATER.
> > >
> > > The hardware designer came up with the Pre-ITS mechanism in Feb 2018.
> > > When we looked at the upstream kernel later (we developed on 4.14)
> > > we found that not only did it support something similar, it supported the
> > > exact scheme we are using.
> >
> > And this scheme is totally wrong. It breaks interrupt isolation.
> >
> > Instead of having a single doorbell and getting the ITS to segregate
> > between devices itself, you end-up with multiple ones, allowing a
> > rogue device to impersonate another one by targeting another doorbell.
> > You can't even use an SMMU to preserve some isolation, because all the
> > doorbells are in the *same page*. Unmitigated disaster.
> >
> > At this stage, why did you bother having an ITS at all? You get none
> > of the security features. Only the excess area, memory allocation,
> > additional latency and complexity. All you get is a larger INTID
> > space.
> >
> > This only shows that the hardware designer didn't understand the ITS
> > at all. Which seems a common pattern, unfortunately.
> 
> The Elba SoC is an embedded chip and not intended as a SBSA-compliant
> general platform.

This has nothing to do with following a standard. It has to do with
following the intended use of the architecture. What you have here is
the system architecture equivalent of trusting userspace to build the
kernel page tables. It can work in limited cases. But would you want
to deploy such construct at scale? Probably not.

> In this implementation the ITS is used to provide message-based
> interrupts for our (potentially large set) of hardware based
> platform device instances.  Virtualization is not a consideration.
> We don't have a SMMU.  Interrupt isolation isn't a practical
> consideration for this product.

Because you have foreseen all use cases for this HW ahead of time, and
can already tell how SW is going to make use of it? Oh well...

> Propose adding a comment to the dts.
> 
> +                       /*
> +                        * Elba SoC implemented a pre-ITS that happened to
> +                        * be the same implementation as synquacer.
> +                        */

Which contains zero information. What you really want is: "We have
decided to ignore the system architecture, good luck".

	M.

-- 
Without deviation from the norm, progress is not possible.

Re: [PATCH v3 11/11] arm64: dts: Add Pensando Elba SoC support

[Brad Larson]

On Mon, Nov 8, 2021 at 11:54 AM Marc Zyngier <maz@kernel.org> wrote:
> >
> > The Elba SoC is an embedded chip and not intended as a SBSA-compliant
> > general platform.
>
> This has nothing to do with following a standard. It has to do with
> following the intended use of the architecture. What you have here is
> the system architecture equivalent of trusting userspace to build the
> kernel page tables. It can work in limited cases. But would you want
> to deploy such construct at scale? Probably not.
>
> > In this implementation the ITS is used to provide message-based
> > interrupts for our (potentially large set) of hardware based
> > platform device instances.  Virtualization is not a consideration.
> > We don't have a SMMU.  Interrupt isolation isn't a practical
> > consideration for this product.
>
> Because you have foreseen all use cases for this HW ahead of time, and
> can already tell how SW is going to make use of it? Oh well...
>
> > Propose adding a comment to the dts.
> >
> > +                       /*
> > +                        * Elba SoC implemented a pre-ITS that happened to
> > +                        * be the same implementation as synquacer.
> > +                        */
>
> Which contains zero information. What you really want is: "We have
> decided to ignore the system architecture, good luck".
>
>         M.
>
> --
> Without deviation from the norm, progress is not possible.

On the contrary, the confusion of using the existing driver match
"socionext,synquacer-pre-its" is answered, why add new code.
Looks like we are deviating from the norm ;-).  I'm not seeing how
this conversation is a productive use of time for a platform in
production.

Thanks,
Brad

Re: [PATCH v3 11/11] arm64: dts: Add Pensando Elba SoC support

[Brad Larson]

Hi Rob,

On Wed, Oct 27, 2021 at 2:37 PM Rob Herring <robh@kernel.org> wrote:
>
> > +always-y     := $(dtb-y)
> > +subdir-y     := $(dts-dirs)
> > +clean-files  := *.dtb
>
> None of these lines should be needed.

Yes, these will be removed.

> > diff --git a/arch/arm64/boot/dts/pensando/elba-16core.dtsi b/arch/arm64/boot/dts/pensando/elba-16core.dtsi
> > new file mode 100644
> > index 000000000000..acf5941afbc1
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/pensando/elba-16core.dtsi
> > @@ -0,0 +1,192 @@
> > +// SPDX-License-Identifier: GPL-2.0
>
> Do you care about using with non-GPL OS? Dual license is preferred.
>
Dual is fine, changing to this:
SPDX-License-Identifier: (GPL-2.0+ OR MIT)

> > +             psci {
>
> This goes at the root level.
>
Moving to the root level

> > +                     compatible = "arm,psci-0.2";
> > +                     method = "smc";
> > +             };
> > +
> > +     };
> > +};
> > diff --git a/arch/arm64/boot/dts/pensando/elba-asic-common.dtsi b/arch/arm64/boot/dts/pensando/elba-asic-common.dtsi
> > +
> > +&spi0 {
> > +     num-cs = <4>;
> > +     cs-gpios = <0>, <0>, <&porta 1 GPIO_ACTIVE_LOW>,
> > +                <&porta 7 GPIO_ACTIVE_LOW>;
> > +     status = "okay";
> > +     spi0_cs0@0 {
>
> Node names should reflect the class of device and use standard name
> defined in the DT spec. This probably doesn't have one. 'lora' perhaps?
>
Right, I didn't see a standard name and found many approaches in other files
so I likely based off of one of these below.   I searched the dts
files for 'lora' and
didn't find it.  Is that an acronym?  I can change it to what the preference is.

./microchip/sparx5_pcb134_board.dtsi:
&spi0 {
   status = "okay";
   spi@0 {
           compatible = "spi-mux";
           ...
};

./rockchip/rk3399.dtsi:
spi5 {
        spi5_clk: spi5-clk {
                rockchip,pins =
                        <2 RK_PC6 2 &pcfg_pull_up>;
        };
        spi5_cs0: spi5-cs0 {
                rockchip,pins =
                        <2 RK_PC7 2 &pcfg_pull_up>;
        };
        spi5_rx: spi5-rx {
                rockchip,pins =
                        <2 RK_PC4 2 &pcfg_pull_up>;
        };
        spi5_tx: spi5-tx {
                rockchip,pins =
                        <2 RK_PC5 2 &pcfg_pull_up>;
        };
};

>
> > +             compatible = "semtech,sx1301";  /* Enable spidev */
>
> What's spidev?
>
It's module drivers/spi/spidev.c which won't populate /dev/spidevB.C unless
there is a match which we need for the system to boot.  An earlier patch added
to the compatible list below and the feedback on that was to remove it.  Later I
noticed the compatible list expanded...

static const struct of_device_id spidev_dt_ids[] = {
        { .compatible = "rohm,dh2228fv" },
        { .compatible = "lineartechnology,ltc2488" },
        { .compatible = "semtech,sx1301" },
        { .compatible = "lwn,bk4" },
        { .compatible = "dh,dhcom-board" },
        { .compatible = "menlo,m53cpld" },
        { .compatible = "cisco,spi-petra" },
        { .compatible = "micron,spi-authenta" },
        {},
};

> > +             #address-cells = <1>;
> > +             #size-cells = <1>;
> > +             spi-max-frequency = <12000000>;
> > +             reg = <0>;
> > +     };
> > +
> > +     spi0_cs1@1 {
> > +             compatible = "semtech,sx1301";
> > +             #address-cells = <1>;
> > +             #size-cells = <1>;
> > +             spi-max-frequency = <12000000>;
> > +             reg = <1>;
> > +     };
> > +
> > +     spi0_cs2@2 {
> > +             compatible = "semtech,sx1301";
> > +             #address-cells = <1>;
> > +             #size-cells = <1>;
> > +             spi-max-frequency = <12000000>;
> > +             reg = <2>;
> > +             interrupt-parent = <&porta>;
> > +             interrupts = <0 IRQ_TYPE_LEVEL_LOW>;
> > +     };
> > +
> > +     spi0_cs3@3 {
> > +             compatible = "semtech,sx1301";
> > +             #address-cells = <1>;
> > +             #size-cells = <1>;
> > +             spi-max-frequency = <12000000>;
> > +             reg = <3>;
> > +     };
> > +};
> > diff --git a/arch/arm64/boot/dts/pensando/elba-asic.dts b/arch/arm64/boot/dts/pensando/elba-asic.dts
> > new file mode 100644
> > index 000000000000..131931dc643f
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/pensando/elba-asic.dts
> > @@ -0,0 +1,23 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +
> > +/dts-v1/;
> > +
> > +/ {
> > +     model = "Elba ASIC Board";
> > +     compatible = "pensando,elba";
>
> Normally we have a compatible for the board plus the soc compatible.

In this case there are currently five different boards/products that have no
variations needing a board level description.

Thanks,
Brad

Re: [PATCH v3 04/11] dt-bindings: spi: Add compatible for Pensando Elba SoC

[Brad Larson]

Hi Sergey,

On Thu, Oct 28, 2021 at 12:26 AM Serge Semin <fancer.lancer@gmail.com> wrote:
>
> Hello Brad
>
> The patch name "dt-bindings: spi: Add compatible for Pensando Elba
> SoC" doesn't mention to what bindings it is referring to. For the sake
> of having a more representative git log I'd suggest at least to add
> "cdns" vendor name in the title, like: "dt-bindings: spi: cdns: Add ..."
> Otherwise it's impossible to understand to what bindings you're adding
> a new compatibility especially seeing you are doing the similar thing
> for the DW SPI in the next patch.

I'll add "cdns" to the git log for the re-spin.

Thanks
Brad

Re: [PATCH v3 05/11] spi: dw: Add Pensando Elba SoC SPI Controller bindings

[Brad Larson]

Hi Sergey,

On Thu, Oct 28, 2021 at 12:49 AM Serge Semin <fancer.lancer@gmail.com> wrote:
>
> On Sun, Oct 24, 2021 at 06:51:50PM -0700, Brad Larson wrote:
> > The Pensando Elba SoC has integrated the DW APB SPI Controller
>
> Please add the "dt-bindings: " prefix to the patch name and discard
> the word "bindings" from the title as the submitting DT-patches
> requires:
> Documentation/devicetree/bindings/submitting-patches.rst

I'll add that.  I recall looking at the recent git log for similar
changes to the file as the current recommended approach.

> >
> > Signed-off-by: Brad Larson <brad@pensando.io>
> > ---
> >  Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
> > index d7e08b03e204..0b5ebb2ae6e7 100644
> > --- a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
> > +++ b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
> > @@ -73,6 +73,8 @@ properties:
> >                - renesas,r9a06g032-spi # RZ/N1D
> >                - renesas,r9a06g033-spi # RZ/N1S
> >            - const: renesas,rzn1-spi   # RZ/N1
>
> > +      - description: Pensando Elba SoC SPI Controller
> > +        const: pensando,elba-spi
>
> AFAICS from the driver-part of the patchset it's not enough. You've
> also got the syscon phandle, which needs to be reflected in the
> bindings. That also makes me thinking that you didn't perform the
> "dtbs_check" on the dts-files you were going to submit, but for some
> reason discarded from this series (btw why?). If you did you would
> have got an error of an unevaluated property detection.

I ran the checks below and didn't get errors.  Rob provided some info
and I found the server did not have yamllint installed (not flagged by
tool).  Also dt-schema was not the latest.  I'm re-doing this and
including "DT_CHECKER_FLAGS=-m" as that is new with v5.13.

make ARCH=arm64 dtbs_check
DT_SCHEMA_FILES=Documentation/devicetree/bindings/mmc/cdns,sdhci.yaml
make ARCH=arm64 dtbs_check
DT_SCHEMA_FILES=Documentation/devicetree/bindings/spi/cdns,qspi-nor.yaml
make ARCH=arm64 dtbs_check
DT_SCHEMA_FILES=Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
make ARCH=arm64 dtbs_check
DT_SCHEMA_FILES=Documentation/devicetree/bindings/vendor-prefixes.yaml
make ARCH=arm64 dtbs_check
DT_SCHEMA_FILES=Documentation/devicetree/bindings/arm/pensando,elba.yaml

make dt_binding_check
DT_SCHEMA_FILES=Documentation/devicetree/bindings/mmc/cdns,sdhci.yaml
make dt_binding_check
DT_SCHEMA_FILES=Documentation/devicetree/bindings/spi/cdns,qspi-nor.yaml
make dt_binding_check
DT_SCHEMA_FILES=Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
make dt_binding_check
DT_SCHEMA_FILES=Documentation/devicetree/bindings/vendor-prefixes.yaml
make dt_binding_check
DT_SCHEMA_FILES=Documentation/devicetree/bindings/arm/pensando,elba.yaml

Thanks
Brad

Re: [PATCH v3 05/11] spi: dw: Add Pensando Elba SoC SPI Controller bindings

[Serge Semin]

On Mon, Nov 15, 2021 at 02:24:40PM -0800, Brad Larson wrote:
> Hi Sergey,
> 
> On Thu, Oct 28, 2021 at 12:49 AM Serge Semin <fancer.lancer@gmail.com> wrote:
> >
> > On Sun, Oct 24, 2021 at 06:51:50PM -0700, Brad Larson wrote:
> > > The Pensando Elba SoC has integrated the DW APB SPI Controller
> >
> > Please add the "dt-bindings: " prefix to the patch name and discard
> > the word "bindings" from the title as the submitting DT-patches
> > requires:
> > Documentation/devicetree/bindings/submitting-patches.rst
> 
> I'll add that.  I recall looking at the recent git log for similar
> changes to the file as the current recommended approach.
> 
> > >
> > > Signed-off-by: Brad Larson <brad@pensando.io>
> > > ---
> > >  Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml | 2 ++
> > >  1 file changed, 2 insertions(+)
> > >
> > > diff --git a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
> > > index d7e08b03e204..0b5ebb2ae6e7 100644
> > > --- a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
> > > +++ b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
> > > @@ -73,6 +73,8 @@ properties:
> > >                - renesas,r9a06g032-spi # RZ/N1D
> > >                - renesas,r9a06g033-spi # RZ/N1S
> > >            - const: renesas,rzn1-spi   # RZ/N1
> >
> > > +      - description: Pensando Elba SoC SPI Controller
> > > +        const: pensando,elba-spi
> >
> > AFAICS from the driver-part of the patchset it's not enough. You've
> > also got the syscon phandle, which needs to be reflected in the
> > bindings. That also makes me thinking that you didn't perform the
> > "dtbs_check" on the dts-files you were going to submit, but for some
> > reason discarded from this series (btw why?). If you did you would
> > have got an error of an unevaluated property detection.
> 
> I ran the checks below and didn't get errors.  Rob provided some info
> and I found the server did not have yamllint installed (not flagged by
> tool).  Also dt-schema was not the latest.  I'm re-doing this and
> including "DT_CHECKER_FLAGS=-m" as that is new with v5.13.
> 

> make ARCH=arm64 dtbs_check
> DT_SCHEMA_FILES=Documentation/devicetree/bindings/mmc/cdns,sdhci.yaml
> make ARCH=arm64 dtbs_check
> DT_SCHEMA_FILES=Documentation/devicetree/bindings/spi/cdns,qspi-nor.yaml
> make ARCH=arm64 dtbs_check
> DT_SCHEMA_FILES=Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
> make ARCH=arm64 dtbs_check
> DT_SCHEMA_FILES=Documentation/devicetree/bindings/vendor-prefixes.yaml
> make ARCH=arm64 dtbs_check
> DT_SCHEMA_FILES=Documentation/devicetree/bindings/arm/pensando,elba.yaml

Hmm, that's weird. Rob, does dtschema tool have the
"unevaluatedProperties" property support?

Brad, anyway you still need to add the syscon-property (pensando,*spics)
requirement in the snps,dw-apb-ssi.yaml schema. See the way it's done there
for instance for "baikal,bt1-sys-ssi" when it comes to the
vendor-specific properties definition in the allOf composition block.
You'll need to define a custom phandle property there in case if a
DT-node is compatible with you SPI controller.

-Sergey

> 
> make dt_binding_check
> DT_SCHEMA_FILES=Documentation/devicetree/bindings/mmc/cdns,sdhci.yaml
> make dt_binding_check
> DT_SCHEMA_FILES=Documentation/devicetree/bindings/spi/cdns,qspi-nor.yaml
> make dt_binding_check
> DT_SCHEMA_FILES=Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
> make dt_binding_check
> DT_SCHEMA_FILES=Documentation/devicetree/bindings/vendor-prefixes.yaml
> make dt_binding_check
> DT_SCHEMA_FILES=Documentation/devicetree/bindings/arm/pensando,elba.yaml
> 
> Thanks
> Brad

Re: [PATCH v3 05/11] spi: dw: Add Pensando Elba SoC SPI Controller bindings

[Brad Larson]

Hi Sergey,

On Tue, Nov 16, 2021 at 3:29 AM Serge Semin <fancer.lancer@gmail.com> wrote:
>
> > > AFAICS from the driver-part of the patchset it's not enough. You've
> > > also got the syscon phandle, which needs to be reflected in the
> > > bindings. That also makes me thinking that you didn't perform the
> > > "dtbs_check" on the dts-files you were going to submit, but for some
> > > reason discarded from this series (btw why?). If you did you would
> > > have got an error of an unevaluated property detection.
> >
> > I ran the checks below and didn't get errors.  Rob provided some info
> > and I found the server did not have yamllint installed (not flagged by
> > tool).  Also dt-schema was not the latest.  I'm re-doing this and
> > including "DT_CHECKER_FLAGS=-m" as that is new with v5.13.
> >
>
> > make ARCH=arm64 dtbs_check
> > DT_SCHEMA_FILES=Documentation/devicetree/bindings/mmc/cdns,sdhci.yaml
> > make ARCH=arm64 dtbs_check
> > DT_SCHEMA_FILES=Documentation/devicetree/bindings/spi/cdns,qspi-nor.yaml
> > make ARCH=arm64 dtbs_check
> > DT_SCHEMA_FILES=Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
> > make ARCH=arm64 dtbs_check
> > DT_SCHEMA_FILES=Documentation/devicetree/bindings/vendor-prefixes.yaml
> > make ARCH=arm64 dtbs_check
> > DT_SCHEMA_FILES=Documentation/devicetree/bindings/arm/pensando,elba.yaml
>
> Hmm, that's weird. Rob, does dtschema tool have the
> "unevaluatedProperties" property support?
>
> Brad, anyway you still need to add the syscon-property (pensando,*spics)
> requirement in the snps,dw-apb-ssi.yaml schema. See the way it's done there
> for instance for "baikal,bt1-sys-ssi" when it comes to the
> vendor-specific properties definition in the allOf composition block.
> You'll need to define a custom phandle property there in case if a
> DT-node is compatible with you SPI controller.

Updating and adding only this bindings update to file
snps,dw-apb-ssi.yaml in 5.16.0-rc1 (next-20211116):

diff --git a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
index d7e08b03e204..99deb587a47b 100644
--- a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
+++ b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
@@ -37,6 +37,21 @@ allOf:
     else:
       required:
         - interrupts
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - pensando,elba-spics
+    then:
+      properties:
+        pensando,spics:
+          $ref: /schemas/types.yaml#/definitions/phandle
+          description:
+            Phandle to the system control device node which provides access to
+            the spics control register
+      required:
+        - pensando,spics

 properties:
   compatible:
@@ -73,6 +88,8 @@ properties:
               - renesas,r9a06g032-spi # RZ/N1D
               - renesas,r9a06g033-spi # RZ/N1S
           - const: renesas,rzn1-spi   # RZ/N1
+      - description: Pensando Elba SoC SPI Controller
+        const: pensando,elba-spics

   reg:
     minItems: 1

$ make ARCH=arm64 defconfig
...

$ make DT_CHECKER_FLAGS=-m dt_binding_check
DT_SCHEMA_FILES=Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
...
  DTEX    Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.example.dts
  DTC     Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.example.dt.yaml
  CHECK   Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.example.dt.yaml
Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.example.dt.yaml:0:0:
/example-0/spi@fff00000/flash@1: failed to match any schema with
compatible: ['spi-nand']

The spi-nand schema match failure happens before I make any change.
The tool also throws errors for these files which are unrelated

Documentation/devicetree/bindings/net/qcom,ipa.yaml: ignoring, error
in schema: properties: qcom,smem-state-names
Documentation/devicetree/bindings/iio/adc/st,stm32-dfsdm-adc.yaml:
ignoring, error in schema: patternProperties: ^filter@[0-9]+$:
properties: st,adc-channel-names
Documentation/devicetree/bindings/interconnect/qcom,rpmh.yaml:
ignoring, error in schema: properties: qcom,bcm-voter-names

Thanks,
Brad

Re: [PATCH v3 03/11] dt-bindings: mmc: Add Pensando Elba SoC binding

[Brad Larson]

Hi Rob,

On Tue, Oct 26, 2021 at 11:10 AM Rob Herring <robh@kernel.org> wrote:
>
> On Sun, Oct 24, 2021 at 06:51:48PM -0700, Brad Larson wrote:
> > Pensando Elba ARM 64-bit SoC is integrated with this IP and
> > explicitly controls byte-lane enables resulting in an additional
> > reg property resource.
> >
> > Signed-off-by: Brad Larson <brad@pensando.io>
> > ---
> >  .../devicetree/bindings/mmc/cdns,sdhci.yaml         | 13 ++++++++-----
> >  1 file changed, 8 insertions(+), 5 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/mmc/cdns,sdhci.yaml b/Documentation/devicetree/bindings/mmc/cdns,sdhci.yaml
> > index af7442f73881..6c68b7b5abec 100644
> > --- a/Documentation/devicetree/bindings/mmc/cdns,sdhci.yaml
> > +++ b/Documentation/devicetree/bindings/mmc/cdns,sdhci.yaml
> > @@ -15,13 +15,16 @@ allOf:
> >
> >  properties:
> >    compatible:
> > -    items:
> > -      - enum:
> > -          - socionext,uniphier-sd4hc
> > -      - const: cdns,sd4hc
> > +    oneOf:
> > +      - items:
> > +        - enum:
> > +            - socionext,uniphier-sd4hc
> > +            - pensando,elba-emmc
> > +        - const: cdns,sd4hc
> >
> >    reg:
> > -    maxItems: 1
> > +    minItems: 1
> > +    maxItems: 2
>
> If there is more than 1, then you need to describe what each entry is.

The dtschema update and yamllint install shows the errors your bot
reported.  With the updated cdns,sdhci.yaml to add the description for
Elba's two reg items this is what I'm getting with in 5.16.0-rc1
(next-20211116):

diff --git a/Documentation/devicetree/bindings/mmc/cdns,sdhci.yaml
b/Documentation/devicetree/bindings/mmc/cdns,sdhci.yaml
index 4207fed62dfe..c01a7283c468 100644
--- a/Documentation/devicetree/bindings/mmc/cdns,sdhci.yaml
+++ b/Documentation/devicetree/bindings/mmc/cdns,sdhci.yaml
@@ -12,17 +12,44 @@ maintainers:

 allOf:
   - $ref: mmc-controller.yaml
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - socionext,uniphier-sd4hc
+    then:
+      properties:
+        reg:
+          maxItems: 1
+          items:
+            - description: SDHCI CDNS HRS registers
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - pensando,elba-emmc
+    then:
+      properties:
+        reg:
+          maxItems: 2
+          items:
+            - description: SDHCI CDNS HRS registers
+            - description: Byte lane control register

 properties:
   compatible:
-    items:
-      - enum:
-          - microchip,mpfs-sd4hc
-          - socionext,uniphier-sd4hc
-      - const: cdns,sd4hc
+    oneOf:
+      - items:
+          - enum:
+              - socionext,uniphier-sd4hc
+              - pensando,elba-emmc
+          - const: cdns,sd4hc

   reg:
-    maxItems: 1
+    minItems: 1
+    maxItems: 2

   interrupts:
     maxItems: 1

$ make DT_CHECKER_FLAGS=-m dt_binding_check
DT_SCHEMA_FILES=Documentation/devicetree/bindings/mmc/cdns,sdhci.yaml
  LINT    Documentation/devicetree/bindings
  CHKDT   Documentation/devicetree/bindings/processed-schema-examples.json
  SCHEMA  Documentation/devicetree/bindings/processed-schema-examples.json
...
  DTEX    Documentation/devicetree/bindings/mmc/cdns,sdhci.example.dts
  DTC     Documentation/devicetree/bindings/mmc/cdns,sdhci.example.dt.yaml
  CHECK   Documentation/devicetree/bindings/mmc/cdns,sdhci.example.dt.yaml

These errors are reported for unrelated files not on DT_SCHEMA_FILES

bindings/net/qcom,ipa.yaml: ignoring, error in schema: properties:
qcom,smem-state-names
bindings/iio/adc/st,stm32-dfsdm-adc.yaml: ignoring, error in schema:
patternProperties: ^filter@[0-9]+$: properties: st,adc-channel-names
bindings/interconnect/qcom,rpmh.yaml: ignoring, error in schema:
properties: qcom,bcm-voter-names

Thanks,
Brad

Re: [PATCH v3 03/11] dt-bindings: mmc: Add Pensando Elba SoC binding

[Brad Larson]

Hi Rob,

On Tue, Nov 16, 2021 at 5:21 PM Brad Larson <brad@pensando.io> wrote:
>
>  properties:
>    compatible:
> -    items:
> -      - enum:
> -          - microchip,mpfs-sd4hc

"microchip,mpfs-sd4hc" was inadvertently removed in moving to
5.16.0-rc1 and won't be in the re-spin of the patchset, its recently
added.

Also, as you mentioned the patchset should be sent against rc1

Thanks
Brad

Re: [PATCH v3 05/11] spi: dw: Add Pensando Elba SoC SPI Controller bindings

[Serge Semin]

On Tue, Nov 16, 2021 at 03:11:53PM -0800, Brad Larson wrote:
> Hi Sergey,
> 
> On Tue, Nov 16, 2021 at 3:29 AM Serge Semin <fancer.lancer@gmail.com> wrote:
> >
> > > > AFAICS from the driver-part of the patchset it's not enough. You've
> > > > also got the syscon phandle, which needs to be reflected in the
> > > > bindings. That also makes me thinking that you didn't perform the
> > > > "dtbs_check" on the dts-files you were going to submit, but for some
> > > > reason discarded from this series (btw why?). If you did you would
> > > > have got an error of an unevaluated property detection.
> > >
> > > I ran the checks below and didn't get errors.  Rob provided some info
> > > and I found the server did not have yamllint installed (not flagged by
> > > tool).  Also dt-schema was not the latest.  I'm re-doing this and
> > > including "DT_CHECKER_FLAGS=-m" as that is new with v5.13.
> > >
> >
> > > make ARCH=arm64 dtbs_check
> > > DT_SCHEMA_FILES=Documentation/devicetree/bindings/mmc/cdns,sdhci.yaml
> > > make ARCH=arm64 dtbs_check
> > > DT_SCHEMA_FILES=Documentation/devicetree/bindings/spi/cdns,qspi-nor.yaml
> > > make ARCH=arm64 dtbs_check
> > > DT_SCHEMA_FILES=Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
> > > make ARCH=arm64 dtbs_check
> > > DT_SCHEMA_FILES=Documentation/devicetree/bindings/vendor-prefixes.yaml
> > > make ARCH=arm64 dtbs_check
> > > DT_SCHEMA_FILES=Documentation/devicetree/bindings/arm/pensando,elba.yaml
> >
> > Hmm, that's weird. Rob, does dtschema tool have the
> > "unevaluatedProperties" property support?
> >
> > Brad, anyway you still need to add the syscon-property (pensando,*spics)
> > requirement in the snps,dw-apb-ssi.yaml schema. See the way it's done there
> > for instance for "baikal,bt1-sys-ssi" when it comes to the
> > vendor-specific properties definition in the allOf composition block.
> > You'll need to define a custom phandle property there in case if a
> > DT-node is compatible with you SPI controller.
> 
> Updating and adding only this bindings update to file
> snps,dw-apb-ssi.yaml in 5.16.0-rc1 (next-20211116):
> 
> diff --git a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
> b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
> index d7e08b03e204..99deb587a47b 100644
> --- a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
> +++ b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
> @@ -37,6 +37,21 @@ allOf:
>      else:
>        required:
>          - interrupts
> +  - if:
> +      properties:

> +        compatible:
> +          contains:
> +            enum:
> +              - pensando,elba-spics

I was wrong using that construction here (fixup patch would be very
welcome) seeing the "snps,dw-apb-ssi" doesn't permit having a generic
"snps,dw*" compatible string. So just const-compatible property should
be enough:

+        compatible:
+          const: pensando,elba-spics


> +    then:
> +      properties:
> +        pensando,spics:
> +          $ref: /schemas/types.yaml#/definitions/phandle
> +          description:
> +            Phandle to the system control device node which provides access to
> +            the spics control register
> +      required:

> +        - pensando,spics

Please note, I've asked to be more specific in this property naming.
Something like this should be fine
"pensando,elba-syscon-spics"/"pensando,syscon-spics".

> 
>  properties:
>    compatible:
> @@ -73,6 +88,8 @@ properties:
>                - renesas,r9a06g032-spi # RZ/N1D
>                - renesas,r9a06g033-spi # RZ/N1S
>            - const: renesas,rzn1-spi   # RZ/N1
> +      - description: Pensando Elba SoC SPI Controller
> +        const: pensando,elba-spics
> 
>    reg:
>      minItems: 1
> 
> $ make ARCH=arm64 defconfig
> ...
> 

> $ make DT_CHECKER_FLAGS=-m dt_binding_check
> DT_SCHEMA_FILES=Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml

I am kind of surprised that this command is still evaluating all
the schemas. Compiling only the depended DT-schemas would much
better... Anyway that's why you are getting unrelated to the
snps,dw-apb-ssi.yaml errors.

> ...
>   DTEX    Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.example.dts
>   DTC     Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.example.dt.yaml
>   CHECK   Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.example.dt.yaml
> Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.example.dt.yaml:0:0:
> /example-0/spi@fff00000/flash@1: failed to match any schema with
> compatible: ['spi-nand']

That means there is no bindings defined for the "spi-nand"-compatible
node. AFAICS it doesn't make the tool to fail though. Anyway fixing this
part would be a subject of additional patches (which would be very
appreciated). That would concern converting the
Documentation/devicetree/bindings/mtd/spi-nand.txt legacy bindings to
the DT-schema. It's not enough though. Seeing that file lacks of
the NAND Flash specific DT-property description, you'd need to detach
ones (described by the "^nand@[a-f0-9]$"-pattern property) from the
Documentation/devicetree/bindings/mtd/nand-controller.yaml schema and
place them into a separate DT-schema file for generic nand-flashes
Documentation/devicetree/bindings/mtd/nand-flash.yaml in a framework
of a pre-requisite patch. Than in the legacy bindings conversion patch
you'd need to use it to correctly evaluate a generic NAND flash node.
Adding some example properties to the DT-schema would be also
required.

-Sergey

> 
> The spi-nand schema match failure happens before I make any change.
> The tool also throws errors for these files which are unrelated
> 
> Documentation/devicetree/bindings/net/qcom,ipa.yaml: ignoring, error
> in schema: properties: qcom,smem-state-names
> Documentation/devicetree/bindings/iio/adc/st,stm32-dfsdm-adc.yaml:
> ignoring, error in schema: patternProperties: ^filter@[0-9]+$:
> properties: st,adc-channel-names
> Documentation/devicetree/bindings/interconnect/qcom,rpmh.yaml:
> ignoring, error in schema: properties: qcom,bcm-voter-names
> 
> Thanks,
> Brad

Re: [PATCH v3 05/11] spi: dw: Add Pensando Elba SoC SPI Controller bindings

[Brad Larson]

Hi Sergey,

On Wed, Nov 17, 2021 at 12:19 AM Serge Semin <fancer.lancer@gmail.com> wrote:
>
> I was wrong using that construction here (fixup patch would be very
> welcome) seeing the "snps,dw-apb-ssi" doesn't permit having a generic
> "snps,dw*" compatible string. So just const-compatible property should
> be enough:
>
> +        compatible:
> +          const: pensando,elba-spics
>
>
> > +    then:
> > +      properties:
> > +        pensando,spics:
> > +          $ref: /schemas/types.yaml#/definitions/phandle
> > +          description:
> > +            Phandle to the system control device node which provides access to
> > +            the spics control register
> > +      required:
>
> > +        - pensando,spics
>
> Please note, I've asked to be more specific in this property naming.
> Something like this should be fine
> "pensando,elba-syscon-spics"/"pensando,syscon-spics".

I would have avoided a typo in the last reply if the spics property
was more specific.  Based on needed construction like this?

DT:
                spi0: spi@2800 {
                        compatible = "pensando,elba-spi";
                        reg = <0x0 0x2800 0x0 0x100>;
                        pensando,elba-syscon-spics = <&mssoc 0x2468>;
                        clocks = <&ahb_clk>;
                        interrupts = <GIC_SPI 9 IRQ_TYPE_LEVEL_HIGH>;
                        #address-cells = <1>;
                        #size-cells = <0>;
                        num-cs = <2>;
                        status = "disabled";
                };

Binding:
--- a/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
+++ b/Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
@@ -37,6 +37,15 @@ allOf:
     else:
       required:
         - interrupts
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - pensando,elba-spi
+    then:
+      required:
+        - pensando,elba-syscon-spics

 properties:
   compatible:
@@ -73,6 +82,8 @@ properties:
               - renesas,r9a06g032-spi # RZ/N1D
               - renesas,r9a06g033-spi # RZ/N1S
           - const: renesas,rzn1-spi   # RZ/N1
+      - description: Pensando Elba SoC SPI Controller
+        const: pensando,elba-spi

   reg:
     minItems: 1

Thanks,
Brad