mbox series

[v2,00/13] Support Pensando Elba SoC

Message ID 20210329015938.20316-1-brad@pensando.io
Headers show
Series Support Pensando Elba SoC | expand

Message

Brad Larson March 29, 2021, 1:59 a.m. UTC
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

See below for an overview of changes since v1.

== Patch overview ==

- 01    Fix typo, return code value and log message.
- 03    Remove else clause, intrinsic DW chip-select is never used.
- 08-11 Split out dts and bindings to sub-patches
- 10    Converted existing cadence-quadspi.txt to YAML schema
- 13    New driver should use <linux/gpio/driver.h>

Brad Larson (13):
  gpio: Add Elba SoC gpio driver for spi cs control
  spi: cadence-quadspi: Add QSPI support for Pensando Elba SoC
  spi: dw: Add support for Pensando Elba SoC SPI
  spidev: Add Pensando CPLD compatible
  mmc: sdhci-cadence: Add Pensando Elba SoC support
  arm64: Add config for Pensando SoC platforms
  arm64: dts: Add Pensando Elba SoC support
  dt-bindings: Add pensando vendor prefix
  dt-bindings: mmc: Add Pensando Elba SoC binding
  dt-bindings: spi: cadence-qspi: Add support for Pensando Elba SoC
  dt-bindings: gpio: Add Pensando Elba SoC support
  MAINTAINERS: Add entry for PENSANDO
  gpio: Use linux/gpio/driver.h

 .../bindings/gpio/pensando,elba-spics.yaml    |  50 +++
 .../devicetree/bindings/mmc/cdns,sdhci.yaml   |   1 +
 .../bindings/spi/cadence-quadspi.txt          |  68 ----
 .../bindings/spi/cadence-quadspi.yaml         | 153 +++++++++
 .../devicetree/bindings/vendor-prefixes.yaml  |   2 +
 MAINTAINERS                                   |   9 +
 arch/arm64/Kconfig.platforms                  |   5 +
 arch/arm64/boot/dts/Makefile                  |   1 +
 arch/arm64/boot/dts/pensando/Makefile         |   6 +
 arch/arm64/boot/dts/pensando/elba-16core.dtsi | 170 ++++++++++
 .../boot/dts/pensando/elba-asic-common.dtsi   | 112 +++++++
 arch/arm64/boot/dts/pensando/elba-asic.dts    |   7 +
 .../boot/dts/pensando/elba-flash-parts.dtsi   |  78 +++++
 arch/arm64/boot/dts/pensando/elba.dtsi        | 310 ++++++++++++++++++
 drivers/gpio/Kconfig                          |   6 +
 drivers/gpio/Makefile                         |   1 +
 drivers/gpio/gpio-elba-spics.c                | 113 +++++++
 drivers/mmc/host/Kconfig                      |  15 +
 drivers/mmc/host/Makefile                     |   1 +
 drivers/mmc/host/sdhci-cadence-elba.c         | 137 ++++++++
 drivers/mmc/host/sdhci-cadence.c              |  81 +++--
 drivers/mmc/host/sdhci-cadence.h              |  68 ++++
 drivers/spi/spi-cadence-quadspi.c             |   9 +
 drivers/spi/spi-dw-mmio.c                     |  28 +-
 drivers/spi/spidev.c                          |   1 +
 25 files changed, 1321 insertions(+), 111 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/gpio/pensando,elba-spics.yaml
 delete mode 100644 Documentation/devicetree/bindings/spi/cadence-quadspi.txt
 create mode 100644 Documentation/devicetree/bindings/spi/cadence-quadspi.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
 create mode 100644 drivers/gpio/gpio-elba-spics.c
 create mode 100644 drivers/mmc/host/sdhci-cadence-elba.c
 create mode 100644 drivers/mmc/host/sdhci-cadence.h

Comments

Brad Larson March 30, 2021, 3:27 a.m. UTC | #1
On Mon, Mar 29, 2021 at 3:45 AM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
>
> On Mon, Mar 29, 2021 at 5:01 AM Brad Larson <brad@pensando.io> wrote:
> >
> > Pensando Elba SoC platforms have a SPI connected CPLD
> > for platform management.
>
> And? It's not a good justification to spread the (debugging only)
> spidev interface.
>
> What tool is going to use it? Why can't you have a driver for that in
> the kernel?

The driver is in userspace and we need to instantiate /dev/spi0.N
in the /dev directory.  The CPLD includes a device id and version
id that userspace applications use to differentiate functionality on
different boards.  It wouldn't really be appropriate to use one of
the existing entries.

For example even with high pin count SoCs we are offloading
low speed functionality into the CPLD connected over SPI.  The
elba-asic-common.dtsi file shows a compatible string of
"pensando,cpld-rd1173" which does have a kernel driver we
intend to contribute later if there is interest.  This IP in the CPLD
is readily available from Lattice which provides two I2C Masters
which in our case we use for access to the network port transceivers.
What was missing in the kernel was a bridge driver that exposes
what looks like a standard I2C device to userspace where the
drivers/i2c/busses/i2c-rd1173.c handles the spi transfers to the
Lattice IP in the CPLD.

>
> --
> With Best Regards,
> Andy Shevchenko
Pratyush Yadav March 30, 2021, 11:13 a.m. UTC | #2
On 28/03/21 06:59PM, Brad Larson wrote:
> Add QSPI controller support for Pensando Elba SoC
> 
> Signed-off-by: Brad Larson <brad@pensando.io>
> ---
>  drivers/spi/spi-cadence-quadspi.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/spi/spi-cadence-quadspi.c b/drivers/spi/spi-cadence-quadspi.c
> index 52ddb3255d88..4aacb0b2dbc7 100644
> --- a/drivers/spi/spi-cadence-quadspi.c
> +++ b/drivers/spi/spi-cadence-quadspi.c
> @@ -1353,6 +1353,7 @@ static int cqspi_request_mmap_dma(struct cqspi_st *cqspi)
>  	cqspi->rx_chan = dma_request_chan_by_mask(&mask);
>  	if (IS_ERR(cqspi->rx_chan)) {
>  		int ret = PTR_ERR(cqspi->rx_chan);
> +

Unrelated whitespace change. Please move it into a separate cleanup 
patch.

>  		cqspi->rx_chan = NULL;
>  		return dev_err_probe(&cqspi->pdev->dev, ret, "No Rx DMA available\n");
>  	}
> @@ -1633,6 +1634,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_WR_DELAY | CQSPI_DISABLE_DAC_MODE,
> +};
> +
>  static const struct of_device_id cqspi_dt_ids[] = {
>  	{
>  		.compatible = "cdns,qspi-nor",
> @@ -1650,6 +1655,10 @@ static const struct of_device_id cqspi_dt_ids[] = {
>  		.compatible = "intel,lgm-qspi",
>  		.data = &intel_lgm_qspi,
>  	},
> +	{
> +		.compatible = "pensando,cdns-qspi",
> +		.data = &pen_cdns_qspi,
> +	},
>  	{ /* end of table */ }
>  };
>  
> -- 
> 2.17.1
> 

Rest of the patch looks good to me.
Rob Herring March 30, 2021, 9:55 p.m. UTC | #3
On Sun, Mar 28, 2021 at 06:59:32PM -0700, Brad Larson wrote:
> Add Pensando common and Elba SoC specific device nodes

Build your dtb with W=1 and 'make dtbs_check' and fix warnings.

> 
> Signed-off-by: Brad Larson <brad@pensando.io>
> ---
>  arch/arm64/boot/dts/Makefile                  |   1 +
>  arch/arm64/boot/dts/pensando/Makefile         |   6 +
>  arch/arm64/boot/dts/pensando/elba-16core.dtsi | 170 ++++++++++
>  .../boot/dts/pensando/elba-asic-common.dtsi   | 112 +++++++
>  arch/arm64/boot/dts/pensando/elba-asic.dts    |   7 +
>  .../boot/dts/pensando/elba-flash-parts.dtsi   |  78 +++++
>  arch/arm64/boot/dts/pensando/elba.dtsi        | 310 ++++++++++++++++++
>  7 files changed, 684 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 f1173cd93594..c85db0a097fe 100644
> --- a/arch/arm64/boot/dts/Makefile
> +++ b/arch/arm64/boot/dts/Makefile
> @@ -19,6 +19,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..0c2c0961e64a
> --- /dev/null
> +++ b/arch/arm64/boot/dts/pensando/Makefile
> @@ -0,0 +1,6 @@
> +# SPDX-License-Identifier: GPL-2.0
> +dtb-$(CONFIG_ARCH_PENSANDO_ELBA_SOC) += 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..a6c47899b69a
> --- /dev/null
> +++ b/arch/arm64/boot/dts/pensando/elba-16core.dtsi
> @@ -0,0 +1,170 @@
> +
> +/ {
> +	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", "arm,armv8";

This should give you a warning.

> +			reg = <0 0x0>;
> +			enable-method = "spin-table";
> +			next-level-cache = <&l2_0>;
> +		};
> +		cpu1: cpu@1 {
> +			device_type = "cpu";
> +			compatible = "arm,cortex-a72", "arm,armv8";
> +			reg = <0 0x1>;
> +			enable-method = "spin-table";
> +			next-level-cache = <&l2_0>;
> +		};
> +		cpu2: cpu@2 {
> +			device_type = "cpu";
> +			compatible = "arm,cortex-a72", "arm,armv8";
> +			reg = <0 0x2>;
> +			enable-method = "spin-table";
> +			next-level-cache = <&l2_0>;
> +		};
> +		cpu3: cpu@3 {
> +			device_type = "cpu";
> +			compatible = "arm,cortex-a72", "arm,armv8";
> +			reg = <0 0x3>;
> +			enable-method = "spin-table";
> +			next-level-cache = <&l2_0>;
> +		};
> +
> +		l2_0: l2-cache0 {
> +			compatible = "cache";
> +		};
> +
> +		// CLUSTER 1
> +		cpu4: cpu@100 {
> +			device_type = "cpu";
> +			compatible = "arm,cortex-a72", "arm,armv8";
> +			reg = <0 0x100>;
> +			enable-method = "spin-table";
> +			next-level-cache = <&l2_1>;
> +		};
> +		cpu5: cpu@101 {
> +			device_type = "cpu";
> +			compatible = "arm,cortex-a72", "arm,armv8";
> +			reg = <0 0x101>;
> +			enable-method = "spin-table";
> +			next-level-cache = <&l2_1>;
> +		};
> +		cpu6: cpu@102 {
> +			device_type = "cpu";
> +			compatible = "arm,cortex-a72", "arm,armv8";
> +			reg = <0 0x102>;
> +			enable-method = "spin-table";
> +			next-level-cache = <&l2_1>;
> +		};
> +		cpu7: cpu@103 {
> +			device_type = "cpu";
> +			compatible = "arm,cortex-a72", "arm,armv8";
> +			reg = <0 0x103>;
> +			enable-method = "spin-table";
> +			next-level-cache = <&l2_1>;
> +		};
> +
> +		l2_1: l2-cache1 {
> +			compatible = "cache";
> +		};
> +
> +		// CLUSTER 2
> +		cpu8: cpu@200 {
> +			device_type = "cpu";
> +			compatible = "arm,cortex-a72", "arm,armv8";
> +			reg = <0 0x200>;
> +			enable-method = "spin-table";
> +			next-level-cache = <&l2_2>;
> +		};
> +		cpu9: cpu@201 {
> +			device_type = "cpu";
> +			compatible = "arm,cortex-a72", "arm,armv8";
> +			reg = <0 0x201>;
> +			enable-method = "spin-table";
> +			next-level-cache = <&l2_2>;
> +		};
> +		cpu10: cpu@202 {
> +			device_type = "cpu";
> +			compatible = "arm,cortex-a72", "arm,armv8";
> +			reg = <0 0x202>;
> +			enable-method = "spin-table";
> +			next-level-cache = <&l2_2>;
> +		};
> +		cpu11: cpu@203 {
> +			device_type = "cpu";
> +			compatible = "arm,cortex-a72", "arm,armv8";
> +			reg = <0 0x203>;
> +			enable-method = "spin-table";
> +			next-level-cache = <&l2_2>;
> +		};
> +
> +		l2_2: l2-cache2 {
> +			compatible = "cache";
> +		};
> +
> +		// CLUSTER 3
> +		cpu12: cpu@300 {
> +			device_type = "cpu";
> +			compatible = "arm,cortex-a72", "arm,armv8";
> +			reg = <0 0x300>;
> +			enable-method = "spin-table";
> +			next-level-cache = <&l2_3>;
> +		};
> +		cpu13: cpu@301 {
> +			device_type = "cpu";
> +			compatible = "arm,cortex-a72", "arm,armv8";
> +			reg = <0 0x301>;
> +			enable-method = "spin-table";
> +			next-level-cache = <&l2_3>;
> +		};
> +		cpu14: cpu@302 {
> +			device_type = "cpu";
> +			compatible = "arm,cortex-a72", "arm,armv8";
> +			reg = <0 0x302>;
> +			enable-method = "spin-table";
> +			next-level-cache = <&l2_3>;
> +		};
> +		cpu15: cpu@303 {
> +			device_type = "cpu";
> +			compatible = "arm,cortex-a72", "arm,armv8";
> +			reg = <0 0x303>;
> +			enable-method = "spin-table";
> +			next-level-cache = <&l2_3>;
> +		};
> +
> +		l2_3: l2-cache3 {
> +			compatible = "cache";
> +		};
> +	};
> +};
> 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..7de2c35e7fcc
> --- /dev/null
> +++ b/arch/arm64/boot/dts/pensando/elba-asic-common.dtsi
> @@ -0,0 +1,112 @@
> +
> +/ {
> +	model = "Elba ASIC Board";
> +
> +	aliases {
> +		serial0 = &uart0;
> +                spi0 = &spi0;
> +                spi1 = &qspi;
> +	};
> +
> +	chosen {
> +		stdout-path = "serial0:19200n8";
> +	};
> +};
> +
> +&ahb_clk {
> +	clock-frequency = <400000000>;
> +};
> +
> +&emmc_clk {
> +	clock-frequency = <200000000>;
> +};
> +
> +&flash_clk {
> +	clock-frequency = <400000000>;
> +};
> +
> +&ref_clk {
> +	clock-frequency = <156250000>;
> +};
> +
> +&qspi {
> +	status = "okay";
> +	flash0: mt25q@0 {

flash@0

> +		compatible = "jdec,spi-nor";

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 = "ok";
> +};
> +
> +&emmc {
> +	bus-width = <8>;
> +	status = "ok";
> +};
> +
> +&wdt0 {
> +	status = "okay";
> +};
> +
> +&i2c0 {
> +	clock-frequency = <100000>;
> +	status = "okay";
> +	tmp451@4c {
> +		compatible = "ti,tmp451";
> +		reg = <0x4c>;
> +	};
> +	tps53659@62 {
> +		compatible = "ti,tps53659";
> +		reg = <0x62>;
> +	};
> +	pcf85263@51 {
> +		compatible = "nxp,pcf85263";
> +		reg = <0x51>;
> +	};
> +};
> +
> +&spi0 {
> +	num-cs = <4>;
> +	cs-gpios = <&spics 0 0>, <&spics 1 0>, <&porta 1 0>, <&porta 7 0>;
> +	status = "okay";
> +	spi@0 {

'spi@' is reserved for SPI controllers.

> +		compatible = "pensando,cpld";

Any new compatibles need to be documented with schema.

> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +		spi-max-frequency = <12000000>;
> +		reg = <0>;
> +	};
> +	spi@1 {
> +		compatible = "pensando,cpld";
> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +		spi-max-frequency = <12000000>;
> +		reg = <1>;
> +	};
> +	spi@2 {
> +		compatible = "pensando,cpld-rd1173";
> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +		spi-max-frequency = <12000000>;
> +		reg = <2>;
> +		interrupt-parent = <&porta>;
> +		interrupts = <0 IRQ_TYPE_LEVEL_LOW>;
> +	};
> +	spi@3 {
> +		compatible = "pensando,cpld";
> +		#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..d074b1f1574a
> --- /dev/null
> +++ b/arch/arm64/boot/dts/pensando/elba-asic.dts
> @@ -0,0 +1,7 @@
> +
> +/dts-v1/;
> +
> +#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..7fff1d653592
> --- /dev/null
> +++ b/arch/arm64/boot/dts/pensando/elba-flash-parts.dtsi
> @@ -0,0 +1,78 @@
> +&flash0 {
> +	partitions {
> +		compatible = "fixed-partitions";

This should just be moved into elba-asic-common.dtsi IMO unless you have 
good reasons to keep it separate.

> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +		partition@0 {
> +			label = "flash";
> +			reg = <0x00010000 0x0fff0000>;
> +		};
> +		partition@f0000 {
> +			label = "golduenv";
> +			reg = <0x000f0000 0x00010000>;
> +		};
> +		partition@100000 {
> +			label = "boot0";
> +			reg = <0x00100000 0x00080000>;
> +		};
> +		partition@180000 {
> +			label = "golduboot";
> +			reg = <0x00180000 0x00200000>;
> +		};
> +		partition@400000 {
> +			label = "goldfw";
> +			reg = <0x00400000 0x03c00000>;
> +		};
> +		partition@4010000 {
> +			label = "fwmap";
> +			reg = <0x04010000 0x00020000>;
> +		};
> +		partition@4030000 {
> +			label = "fwsel";
> +			reg = <0x04030000 0x00020000>;
> +		};
> +		partition@4090000 {
> +			label = "bootlog";
> +			reg = <0x04090000 0x00020000>;
> +		};
> +		partition@40b0000 {
> +			label = "panicbuf";
> +			reg = <0x040b0000 0x00020000>;
> +		};
> +		partition@40d0000 {
> +			label = "uservars";
> +			reg = <0x040d0000 0x00020000>;
> +		};
> +		partition@4200000 {
> +			label = "uboota";
> +			reg = <0x04200000 0x00400000>;
> +		};
> +		partition@4600000 {
> +			label = "ubootb";
> +			reg = <0x04600000 0x00400000>;
> +		};
> +		partition@4a00000 {
> +			label = "mainfwa";
> +			reg = <0x04a00000 0x01000000>;
> +		};
> +		partition@5a00000 {
> +			label = "mainfwb";
> +			reg = <0x05a00000 0x01000000>;
> +		};
> +		partition@8000000 {
> +			label = "diagfw";
> +			reg = <0x08000000 0x07fe0000>;
> +		};
> +		partition@ffe0000 {
> +			label = "ubootenv";
> +			reg = <0x0ffe0000 0x00010000>;
> +		};
> +	};
> +};
> +
> +&soc {
> +	panicdump@740b0000 {
> +		compatible = "pensando,capri-crash";
> +		reg = <0x0 0x740b0000 0x0 0x00020000>;
> +	};
> +};
> diff --git a/arch/arm64/boot/dts/pensando/elba.dtsi b/arch/arm64/boot/dts/pensando/elba.dtsi
> new file mode 100644
> index 000000000000..6f6cfab2b502
> --- /dev/null
> +++ b/arch/arm64/boot/dts/pensando/elba.dtsi
> @@ -0,0 +1,310 @@
> +

You need SPDX license tags at the top of each file. checkpatch.pl should 
tell you this.

> +/*
> + * Copyright (c) 2019, Pensando Systems Inc.
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include "dt-bindings/interrupt-controller/arm-gic.h"
> +
> +/ {
> +	compatible = "pensando,elba";

This needs an SoC family schema.

Generally this and 'model' should be in the same file (the board file).

> +
> +	interrupt-parent = <&gic>;
> +	#address-cells = <2>;
> +	#size-cells = <2>;
> +
> +	clocks {

Drop this container node.

> +		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)>;
> +	};
> +
> +	/* Common UIO device for MSI drivers */
> +	uio_penmsi {
> +		compatible = "pensando,uio_penmsi";
> +		name = "uio_penmsi";

No. What's UIO?

> +	};
> +
> +
> +	soc: soc {
> +		compatible = "simple-bus";
> +		#address-cells = <2>;
> +		#size-cells = <2>;
> +		ranges;
> +
> +		gic: interrupt-controller@800000 {

Order nodes by increasing unit address.

> +			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>;
> +
> +			its: interrupt-controller@820000 {
> +				compatible = "arm,gic-v3-its";
> +				msi-controller;
> +				#msi-cells = <1>;
> +				reg = <0x0 0x820000 0x0 0x10000>;
> +				socionext,synquacer-pre-its =
> +							<0xc00000 0x1000000>;
> +			};
> +		};
> +
> +		/*
> +		 * Until we  know the interrupt domain following this, we
> +		 * are forced to use this is the place where interrupts from
> +		 * PCI converge. In the ideal case, we use one domain higher,
> +		 * where the PCI-ness has been shed.
> +		 */
> +		pxc0_intr: intc@20102200 {

interrupt-controller@...

> +			compatible = "pensando,soc-ictlr-csrintr";
> +			interrupt-controller;
> +			reg = <0x0 0x20102200 0x0 0x4>;
> +			#interrupt-cells = <3>;
> +			interrupt-parent = <&gic>;
> +			interrupts = <GIC_SPI 17 IRQ_TYPE_LEVEL_HIGH>;
> +			interrupt-names = "pxc0_intr";
> +		};
> +
> +		uart0: serial@4800 {
> +			device_type = "serial";

Drop 'device_type'. It's deprecated for all but memory and pci.

> +			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>;
> +		};
> +
> +		qspi: spi@2400 {
> +			compatible = "pensando,cdns-qspi";
> +			#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";
> +		};
> +
> +		gpio0: gpio@4000 {
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			compatible = "snps,dw-apb-gpio";
> +			reg = <0x0 0x4000 0x0 0x78>;
> +			status = "disabled";
> +
> +			porta: gpio-controller@0 {

gpio@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-controller@1 {
> +				compatible = "snps,dw-apb-gpio-port";
> +				reg = <1>;
> +				gpio-controller;
> +				#gpio-cells = <2>;
> +				ngpios = <8>;
> +			};
> +		};
> +
> +		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";
> +		};
> +
> +		/* UIO device using interrupt line PCIEMAC */

UIO is a kernel thing. It doesn't belong in DT.

> +		pciemac@20102200 {
> +			#address-cells = <2>;
> +			#size-cells = <2>;
> +			#interrupt-cells = <3>;
> +
> +			compatible = "pensando,uio_pciemac";
> +			register-type = "csr-interrupt";
> +			interrupt-parent = <&pxc0_intr>;
> +			interrupts = <GIC_SPI 0 IRQ_TYPE_LEVEL_HIGH>;
> +			reg = <0x0 0x20102200 0x0 0x10>; /* pxc0_intr */
> +
> +			enable_csr_paddr = <0x0 0x20102200 0x0 0x10>;
> +			enable_mask = <(1 << 0)>;
> +		};

This needs some work, but no idea what the h/w is to give any guidance.

> +
> +		/* MSI UIO device 1 */
> +		uio_penmsi1@520000 {
> +			compatible = "pensando,uio_penmsi1";
> +			reg = <0x0 0x520000 0x0 0x10000>;
> +			msi-parent = <&its 0xa>;
> +			num-interrupts = <16>;  /* # MSI interrupts to get */
> +		};
> +
> +		spics: spics@307c2468 {
> +			compatible = "pensando,elba-spics";
> +			reg = <0x0 0x307c2468 0x0 0x4>;
> +			gpio-controller;
> +			#gpio-cells = <2>;
> +		};
> +
> +		spi0: spi@2800 {
> +			compatible = "pensando,elba-spi";
> +			reg = <0x0 0x2800 0x0 0x100>;
> +			clocks = <&ahb_clk>;
> +			interrupts = <GIC_SPI 9 IRQ_TYPE_LEVEL_HIGH>;
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +			num-cs = <2>;
> +			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";
> +		};
> +		wdt1: watchdog@1800 {
> +			compatible = "snps,dw-wdt";
> +			reg = <0x0 0x1800 0x0 0x100>;
> +			clocks = <&ahb_clk>;
> +			interrupts = <GIC_SPI 11 IRQ_TYPE_LEVEL_HIGH>;
> +			status = "disabled";
> +		};
> +		wdt2: watchdog@1c00 {
> +			compatible = "snps,dw-wdt";
> +			reg = <0x0 0x1c00 0x0 0x100>;
> +			clocks = <&ahb_clk>;
> +			interrupts = <GIC_SPI 12 IRQ_TYPE_LEVEL_HIGH>;
> +			status = "disabled";
> +		};
> +		wdt3: watchdog@2000 {
> +			compatible = "snps,dw-wdt";
> +			reg = <0x0 0x2000 0x0 0x100>;
> +			clocks = <&ahb_clk>;
> +			interrupts = <GIC_SPI 13 IRQ_TYPE_LEVEL_HIGH>;
> +			status = "disabled";
> +		};

4 watchdogs? Hope that's enough... ;)

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

Use <> around each entry.

> +			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>;
> +			cdns,mmc-ddr-1_8v;
> +			status = "disabled";
> +		} ;
> +
> +		pcie@307c2480 {
> +			compatible = "pensando,pcie";
> +			reg = <0x0 0x307c2480 0x0 0x4   /* MS CFG_WDT */
> +			       0x0 0x00001400 0x0 0x10  /* WDT0 */
> +			       0x0 0x20000000 0x0 0x00380000>; /* PXB Base */
> +		};
> +
> +		panic: panicdump@0 {
> +			compatible = "pensando,pen-crash";
> +			status = "disabled";
> +		};
> +
> +		bsm@307c2080 {
> +			compatible = "pensando,bsm";
> +			reg = <0x0 0x307c2080 0x0 0x4>;
> +		};
> +	};
> +	mnet0: mnet0 {
> +		compatible = "pensando,mnet";
> +		msi-parent = <&its 0x0>;
> +	};
> +	mnet1: mnet1 {
> +		compatible = "pensando,mnet";
> +		msi-parent = <&its 0x1>;
> +	};
> +	mnet2: mnet2 {
> +		compatible = "pensando,mnet";
> +		msi-parent = <&its 0x2>;
> +	};
> +	mnet3: mnet3 {
> +		compatible = "pensando,mnet";
> +		msi-parent = <&its 0x3>;
> +	};
> +	mnet4: mnet4 {
> +		compatible = "pensando,mnet";
> +		msi-parent = <&its 0x4>;
> +	};
> +	mnet5: mnet5 {
> +		compatible = "pensando,mnet";
> +		msi-parent = <&its 0x5>;
> +	};
> +	mnet6: mnet6 {
> +		compatible = "pensando,mnet";
> +		msi-parent = <&its 0x6>;
> +	};
> +	mnet7: mnet7 {
> +		compatible = "pensando,mnet";
> +		msi-parent = <&its 0x7>;
> +	};
> +	mnet8: mnet8 {
> +		compatible = "pensando,mnet";
> +		msi-parent = <&its 0x8>;
> +	};
> +	mnet9: mnet9 {
> +		compatible = "pensando,mnet";
> +		msi-parent = <&its 0x9>;
> +	};
> +};
> -- 
> 2.17.1
>
Rob Herring March 30, 2021, 9:56 p.m. UTC | #4
On Sun, 28 Mar 2021 18:59:34 -0700, Brad Larson wrote:
> Pensando Elba ARM 64-bit SoC is integrated with this IP

> 

> Signed-off-by: Brad Larson <brad@pensando.io>

> ---

>  Documentation/devicetree/bindings/mmc/cdns,sdhci.yaml | 1 +

>  1 file changed, 1 insertion(+)

> 


Acked-by: Rob Herring <robh@kernel.org>
Brad Larson Aug. 23, 2021, 1:18 a.m. UTC | #5
Hi Sergey,

On Wed, Mar 31, 2021 at 9:17 AM Serge Semin <fancer.lancer@gmail.com> wrote:
>

> Hi Brad

>

> On Sun, Mar 28, 2021 at 06:59:25PM -0700, Brad Larson wrote:

> > This series enables support for Pensando Elba SoC based platforms.

> > The Elba SoC has the following features:

[...]
> > See below for an overview of changes since v1.

> >

> > == Patch overview ==

> >

> > - 01    Fix typo, return code value and log message.

> > - 03    Remove else clause, intrinsic DW chip-select is never used.

> > - 08-11 Split out dts and bindings to sub-patches

> > - 10    Converted existing cadence-quadspi.txt to YAML schema

> > - 13    New driver should use <linux/gpio/driver.h>

>

> That would be super-useful if each changelog entry was also added to the

> individual patches below "---" splitter.


I'll do that for the v3 patchset once the discussions are resolved.

Regards,
Brad
Brad Larson Aug. 23, 2021, 1:36 a.m. UTC | #6
Hi Rob,

On Tue, Mar 30, 2021 at 2:55 PM Rob Herring <robh@kernel.org> wrote:
>
> On Sun, Mar 28, 2021 at 06:59:32PM -0700, Brad Larson wrote:
> > Add Pensando common and Elba SoC specific device nodes
>
> Build your dtb with W=1 and 'make dtbs_check' and fix warnings.

I'll do that thanks.

> > +
> > +             // CLUSTER 0
> > +             cpu0: cpu@0 {
> > +                     device_type = "cpu";
> > +                     compatible = "arm,cortex-a72", "arm,armv8";
>
> This should give you a warning.

I'll be sure to W=1 and make dtbs_check.  All the C++ comments are C syntax now.

> > +&qspi {
> > +     status = "okay";
> > +     flash0: mt25q@0 {
>
> flash@0

Yes, generic node name should be used.  Changed mt25q to flash.
-       flash0: mt25q@0 {
+       flash0: flash@0 {

> > +             compatible = "jdec,spi-nor";
>
> jedec,spi-nor

Changed jdec to jedec
-               compatible = "jdec,spi-nor";
+               compatible = "jedec,spi-nor";

> > +     spi@0 {
>
> 'spi@' is reserved for SPI controllers.

Changed the node name to indicate the spi bus number and chip select

&spi0 {
        num-cs = <4>;
        cs-gpios = <&spics 0 GPIO_ACTIVE_LOW>, <&spics 1 GPIO_ACTIVE_LOW>,
                   <&porta 1 GPIO_ACTIVE_LOW>, <&porta 7 GPIO_ACTIVE_LOW>;
        status = "okay";
-       spi@0 {
+       spi0_cs0@0 {
                compatible = "pensando,cpld";
                #address-cells = <1>;
                #size-cells = <1>;
                spi-max-frequency = <12000000>;
                reg = <0>;
        };
-       spi@1 {
+       spi0_cs1@1 {
        ...

> > +             compatible = "pensando,cpld";
>
> Any new compatibles need to be documented with schema.

I'll create the bindings file for the v3 patchset dependent on the
discussion revolving around adding pensando,cpld to this list in
spi/spidev.c.

static const struct of_device_id spidev_dt_ids[] = {
        { .compatible = "rohm,dh2228fv" },
        { .compatible = "lineartechnology,ltc2488" },
        { .compatible = "ge,achc" },
        { .compatible = "semtech,sx1301" },
        { .compatible = "lwn,bk4" },
        { .compatible = "dh,dhcom-board" },
        { .compatible = "menlo,m53cpld" },
        { .compatible = "pensando,cpld" },    <===
        {},
};
MODULE_DEVICE_TABLE(of, spidev_dt_ids);

> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/pensando/elba-flash-parts.dtsi
> > @@ -0,0 +1,78 @@
> > +&flash0 {
> > +     partitions {
> > +             compatible = "fixed-partitions";
>
> This should just be moved into elba-asic-common.dtsi IMO unless you have
> good reasons to keep it separate.

This file is shared across platforms which have additional flash
devices and partition schemes.

> You need SPDX license tags at the top of each file. checkpatch.pl should
> tell you this.

I don't recall checkpatch.pl reporting that for device tree files.
I've added to each file now.

> > +/*
> > + * Copyright (c) 2019, Pensando Systems Inc.
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License version 2 as
> > + * published by the Free Software Foundation.
> > + */
> > +
> > +#include "dt-bindings/interrupt-controller/arm-gic.h"
> > +
> > +/ {
> > +     compatible = "pensando,elba";
>
> This needs an SoC family schema.
>
> Generally this and 'model' should be in the same file (the board file).

Will add schema for pensando,elba in the v3 patchset.  The compatible
and model are now in the board file elba-asic.dts as shown below

+/ {
+       model = "Elba ASIC Board";
+       compatible = "pensando,elba";
+
+       aliases {
+               serial0 = &uart0;
+               spi0 = &spi0;
+               spi1 = &qspi;
+       };
+
+       chosen {
+               stdout-path = "serial0:115200n8";
+       };
+};

> > +
> > +     interrupt-parent = <&gic>;
> > +     #address-cells = <2>;
> > +     #size-cells = <2>;
> > +
> > +     clocks {
>
> Drop this container node.

Removed the container node.

> > +     /* Common UIO device for MSI drivers */
> > +     uio_penmsi {
> > +             compatible = "pensando,uio_penmsi";
> > +             name = "uio_penmsi";
>
> No. What's UIO?

UIO nodes are removed, only core SoC support will be included

> > +     soc: soc {
> > +             compatible = "simple-bus";
> > +             #address-cells = <2>;
> > +             #size-cells = <2>;
> > +             ranges;
> > +
> > +             gic: interrupt-controller@800000 {
>
> Order nodes by increasing unit address.

Yes, I've done this and will be part of the v3 patchset

> > +             /*
> > +              * Until we  know the interrupt domain following this, we
> > +              * are forced to use this is the place where interrupts from
> > +              * PCI converge. In the ideal case, we use one domain higher,
> > +              * where the PCI-ness has been shed.
> > +              */
> > +             pxc0_intr: intc@20102200 {
>
> interrupt-controller@...

Changed intc to interrupt-controller

> > +                     compatible = "pensando,soc-ictlr-csrintr";
> > +                     interrupt-controller;
> > +                     reg = <0x0 0x20102200 0x0 0x4>;
> > +                     #interrupt-cells = <3>;
> > +                     interrupt-parent = <&gic>;
> > +                     interrupts = <GIC_SPI 17 IRQ_TYPE_LEVEL_HIGH>;
> > +                     interrupt-names = "pxc0_intr";
> > +             };
> > +
> > +             uart0: serial@4800 {
> > +                     device_type = "serial";
>
> Drop 'device_type'. It's deprecated for all but memory and pci.
>

Removed  device_type = "serial";

> > +             gpio0: gpio@4000 {
> > +                     #address-cells = <1>;
> > +                     #size-cells = <0>;
> > +                     compatible = "snps,dw-apb-gpio";
> > +                     reg = <0x0 0x4000 0x0 0x78>;
> > +                     status = "disabled";
> > +
> > +                     porta: gpio-controller@0 {
>
> gpio@0

Changed from gpio@0 to gpio-port@0 based on additional reviewer input

-                       porta: gpio-controller@0 {
+                       porta: gpio-port@0 {

> > +
> > +             /* UIO device using interrupt line PCIEMAC */
>
> UIO is a kernel thing. It doesn't belong in DT.

UIO nodes are removed for core SoC support patchset

> > +
> > +             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>;
>
> Use <> around each entry.

The system fails to boot if I make that change, mmc probe fails and
rootfs not found

-               reg = <0x0 0x30440000 0x0 0x10000
-                      0x0 0x30480044 0x0 0x4>;
+               reg = <0x0 0x30440000 0x0 0x10000>,
+                     <0x0 0x30480044 0x0 0x4>;

Regards,
Brad