[PATCH 0/8] Support Pensando Elba SoC

[PATCH 0/8] 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

Brad Larson (8):
  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
  MAINTAINERS: Add entry for PENSANDO

 .../bindings/gpio/pensando,elba-spics.txt     |  24 ++
 .../devicetree/bindings/mmc/cdns,sdhci.yaml   |   2 +-
 .../bindings/spi/cadence-quadspi.txt          |   1 +
 .../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 | 171 ++++++++++
 .../boot/dts/pensando/elba-asic-common.dtsi   | 113 +++++++
 arch/arm64/boot/dts/pensando/elba-asic.dts    |   8 +
 .../boot/dts/pensando/elba-flash-parts.dtsi   |  80 +++++
 arch/arm64/boot/dts/pensando/elba.dtsi        | 310 ++++++++++++++++++
 drivers/gpio/Kconfig                          |   6 +
 drivers/gpio/Makefile                         |   1 +
 drivers/gpio/gpio-elba-spics.c                | 120 +++++++
 drivers/mmc/host/Kconfig                      |  15 +
 drivers/mmc/host/Makefile                     |   1 +
 drivers/mmc/host/sdhci-cadence-elba.c         | 137 ++++++++
 drivers/mmc/host/sdhci-cadence.c              |  78 ++---
 drivers/mmc/host/sdhci-cadence.h              |  68 ++++
 drivers/spi/spi-cadence-quadspi.c             |   9 +
 drivers/spi/spi-dw-mmio.c                     |  35 +-
 drivers/spi/spidev.c                          |   1 +
 24 files changed, 1159 insertions(+), 44 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/gpio/pensando,elba-spics.txt
 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

-- 
2.17.1

[PATCH 1/8] gpio: Add Elba SoC gpio driver for spi cs control

[Brad Larson]

This GPIO driver is for the Pensando Elba SoC which
provides control of four chip selects on two SPI busses.

Signed-off-by: Brad Larson <brad@pensando.io>
---
 drivers/gpio/Kconfig           |   6 ++
 drivers/gpio/Makefile          |   1 +
 drivers/gpio/gpio-elba-spics.c | 120 +++++++++++++++++++++++++++++++++
 3 files changed, 127 insertions(+)
 create mode 100644 drivers/gpio/gpio-elba-spics.c

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index e3607ec4c2e8..d99bc82aa8fa 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -241,6 +241,12 @@ config GPIO_EIC_SPRD
 	help
 	  Say yes here to support Spreadtrum EIC device.
 
+config GPIO_ELBA_SPICS
+	bool "Pensando Elba SPI chip-select"
+	depends on ARCH_PENSANDO_ELBA_SOC
+	help
+	  Say yes here to support the Pensndo Elba SoC SPI chip-select driver
+
 config GPIO_EM
 	tristate "Emma Mobile GPIO"
 	depends on (ARCH_EMEV2 || COMPILE_TEST) && OF_GPIO
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index c58a90a3c3b1..c5c7acad371b 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -54,6 +54,7 @@ obj-$(CONFIG_GPIO_DAVINCI)		+= gpio-davinci.o
 obj-$(CONFIG_GPIO_DLN2)			+= gpio-dln2.o
 obj-$(CONFIG_GPIO_DWAPB)		+= gpio-dwapb.o
 obj-$(CONFIG_GPIO_EIC_SPRD)		+= gpio-eic-sprd.o
+obj-$(CONFIG_GPIO_ELBA_SPICS)		+= gpio-elba-spics.o
 obj-$(CONFIG_GPIO_EM)			+= gpio-em.o
 obj-$(CONFIG_GPIO_EP93XX)		+= gpio-ep93xx.o
 obj-$(CONFIG_GPIO_EXAR)			+= gpio-exar.o
diff --git a/drivers/gpio/gpio-elba-spics.c b/drivers/gpio/gpio-elba-spics.c
new file mode 100644
index 000000000000..a845525cf2a3
--- /dev/null
+++ b/drivers/gpio/gpio-elba-spics.c
@@ -0,0 +1,120 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Pensando Elba ASIC SPI chip select driver
+ *
+ * Copyright (c) 2020-2021, Pensando Systems Inc.
+ */
+
+#include <linux/err.h>
+#include <linux/gpio.h>
+#include <linux/module.h>
+#include <linux/io.h>
+#include <linux/init.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/spinlock.h>
+#include <linux/types.h>
+
+/*
+ * pin:	     3		  2	   |	   1		0
+ * bit:	 7------6------5------4----|---3------2------1------0
+ *	cs1  cs1_ovr  cs0  cs0_ovr |  cs1  cs1_ovr  cs0	 cs0_ovr
+ *		   ssi1		   |		 ssi0
+ */
+#define SPICS_PIN_SHIFT(pin)	(2 * (pin))
+#define SPICS_MASK(pin)		(0x3 << SPICS_PIN_SHIFT(pin))
+#define SPICS_SET(pin, val)	((((val) << 1) | 0x1) << SPICS_PIN_SHIFT(pin))
+
+struct elba_spics_priv {
+	void __iomem *base;
+	spinlock_t lock;
+	struct gpio_chip chip;
+};
+
+static int elba_spics_get_value(struct gpio_chip *chip, unsigned int pin)
+{
+	return -ENXIO;
+}
+
+static void elba_spics_set_value(struct gpio_chip *chip,
+		unsigned int pin, int value)
+{
+	struct elba_spics_priv *p = gpiochip_get_data(chip);
+	unsigned long flags;
+	u32 tmp;
+
+	/* select chip select from register */
+	spin_lock_irqsave(&p->lock, flags);
+	tmp = readl_relaxed(p->base);
+	tmp = (tmp & ~SPICS_MASK(pin)) | SPICS_SET(pin, value);
+	writel_relaxed(tmp, p->base);
+	spin_unlock_irqrestore(&p->lock, flags);
+}
+
+static int elba_spics_direction_input(struct gpio_chip *chip, unsigned int pin)
+{
+	return -ENXIO;
+}
+
+static int elba_spics_direction_output(struct gpio_chip *chip,
+		unsigned int pin, int value)
+{
+	elba_spics_set_value(chip, pin, value);
+	return 0;
+}
+
+static int elba_spics_probe(struct platform_device *pdev)
+{
+	struct elba_spics_priv *p;
+	struct resource *res;
+	int ret;
+
+	p = devm_kzalloc(&pdev->dev, sizeof(*p), GFP_KERNEL);
+	if (!p)
+		return -ENOMEM;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	p->base = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(p->base)) {
+		dev_err(&pdev->dev, "failed to remap I/O memory\n");
+		return PTR_ERR(p->base);
+	}
+	spin_lock_init(&p->lock);
+	platform_set_drvdata(pdev, p);
+
+	p->chip.ngpio = 4;	/* 2 cs pins for spi0, and 2 for spi1 */
+	p->chip.base = -1;
+	p->chip.direction_input = elba_spics_direction_input;
+	p->chip.direction_output = elba_spics_direction_output;
+	p->chip.get = elba_spics_get_value;
+	p->chip.set = elba_spics_set_value;
+	p->chip.label = dev_name(&pdev->dev);
+	p->chip.parent = &pdev->dev;
+	p->chip.owner = THIS_MODULE;
+
+	ret = devm_gpiochip_add_data(&pdev->dev, &p->chip, p);
+	if (ret) {
+		dev_err(&pdev->dev, "unable to add gpio chip\n");
+		return ret;
+	}
+
+	dev_info(&pdev->dev, "elba spics registered\n");
+	return 0;
+}
+
+static const struct of_device_id ebla_spics_of_match[] = {
+	{ .compatible = "pensando,elba-spics" },
+	{}
+};
+
+static struct platform_driver elba_spics_driver = {
+	.probe = elba_spics_probe,
+	.driver = {
+		.name = "pensando-elba-spics",
+		.of_match_table = ebla_spics_of_match,
+	},
+};
+module_platform_driver(elba_spics_driver);
+
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("Elba SPI chip-select driver");
-- 
2.17.1

Re: [PATCH 1/8] gpio: Add Elba SoC gpio driver for spi cs control

[Linus Walleij]

Hi Brad,

thanks for your patch!

On Thu, Mar 4, 2021 at 4:42 AM Brad Larson <brad@pensando.io> wrote:

> This GPIO driver is for the Pensando Elba SoC which
> provides control of four chip selects on two SPI busses.
>
> Signed-off-by: Brad Larson <brad@pensando.io>
(...)

> +#include <linux/gpio.h>

Use this in new drivers:
#include <linux/gpio/driver.h>

> + * pin:             3            2        |       1            0
> + * bit:         7------6------5------4----|---3------2------1------0
> + *     cs1  cs1_ovr  cs0  cs0_ovr |  cs1  cs1_ovr  cs0  cs0_ovr
> + *                ssi1            |             ssi0
> + */
> +#define SPICS_PIN_SHIFT(pin)   (2 * (pin))
> +#define SPICS_MASK(pin)                (0x3 << SPICS_PIN_SHIFT(pin))
> +#define SPICS_SET(pin, val)    ((((val) << 1) | 0x1) << SPICS_PIN_SHIFT(pin))

So 2 bits per GPIO line in one register? (Nice doc!)

> +struct elba_spics_priv {
> +       void __iomem *base;
> +       spinlock_t lock;
> +       struct gpio_chip chip;
> +};
> +
> +static int elba_spics_get_value(struct gpio_chip *chip, unsigned int pin)
> +{
> +       return -ENXIO;
> +}

Write a comment that the chip only supports output mode,
because it repurposes SPI CS pins as generic GPIO out,
maybe at the top of the file?

I suppose these systems also actually (ab)use the SPI cs
for things that are not really SPI CS? Because otherwise
this could just be part of the SPI driver (native chip select).

> +static const struct of_device_id ebla_spics_of_match[] = {
> +       { .compatible = "pensando,elba-spics" },

Have you documented this?

Other than that this is a nice and complete driver.

Yours,
Linus Walleij

Re: [PATCH 1/8] gpio: Add Elba SoC gpio driver for spi cs control

[Serge Semin]

Hello Linus,

I started reviewing from the DW APB SPI driver part of this series,
that's why I suggested to remove the CS callback from there seeing it
doesn't really differ much from the generic one. But after looking at
the dts file and in this driver I think that the alterations layout
needs to be a bit different.

This module looks more like being a part of a SoC System Controller
seeing it's just a single register. Corresponding pins seem like
being multiplexed between SPI controller and GPO (being directly driven
by setting a bit in the corresponding register). See the next comment.

On Thu, Mar 04, 2021 at 09:29:33AM +0100, Linus Walleij wrote:
> Hi Brad,
> 
> thanks for your patch!
> 
> On Thu, Mar 4, 2021 at 4:42 AM Brad Larson <brad@pensando.io> wrote:
> 
> > This GPIO driver is for the Pensando Elba SoC which
> > provides control of four chip selects on two SPI busses.
> >
> > Signed-off-by: Brad Larson <brad@pensando.io>
> (...)
> 
> > +#include <linux/gpio.h>
> 
> Use this in new drivers:
> #include <linux/gpio/driver.h>
> 
> > + * pin:             3            2        |       1            0
> > + * bit:         7------6------5------4----|---3------2------1------0
> > + *             cs1  cs1_ovr  cs0  cs0_ovr |  cs1  cs1_ovr  cs0  cs0_ovr
> > + *                        ssi1            |             ssi0
> > + */
> > +#define SPICS_PIN_SHIFT(pin)   (2 * (pin))
> > +#define SPICS_MASK(pin)                (0x3 << SPICS_PIN_SHIFT(pin))
> > +#define SPICS_SET(pin, val)    ((((val) << 1) | 0x1) << SPICS_PIN_SHIFT(pin))
> 

> So 2 bits per GPIO line in one register? (Nice doc!)

I suppose the first bit is the CS-pin-override flag. So when it's set
the output is directly driven by the second bit, otherwise the
corresponding DW APB SPI controller drives it. That's how the
multiplexing is implemented here.

> 
> > +struct elba_spics_priv {
> > +       void __iomem *base;
> > +       spinlock_t lock;
> > +       struct gpio_chip chip;
> > +};
> > +
> > +static int elba_spics_get_value(struct gpio_chip *chip, unsigned int pin)
> > +{
> > +       return -ENXIO;
> > +}
> 
> Write a comment that the chip only supports output mode,
> because it repurposes SPI CS pins as generic GPIO out,
> maybe at the top of the file?
> 

> I suppose these systems also actually (ab)use the SPI cs
> for things that are not really SPI CS?

I haven't noticed that in the dts file submitted by Brad. So most
likely these are just CS pins, which can be either automatically
driven by the DW APB SPI controller (yeah, DW APB SPI controller
doesn't provide a way to directly set he native CS value, it
sets the CS value low automatically when starts SPI xfers) or can be
manually set low/high by means of that SPI-CS register.

> Because otherwise
> this could just be part of the SPI driver (native chip select).

That's what I suggested in my comment to the patch
[PATCH 7/8] arm64: dts: Add Pensando Elba SoC support
in this series. Although imho it's better to be done by means
of a System Controller.

-Sergey

> 
> > +static const struct of_device_id ebla_spics_of_match[] = {
> > +       { .compatible = "pensando,elba-spics" },
> 
> Have you documented this?
> 
> Other than that this is a nice and complete driver.
> 
> Yours,
> Linus Walleij

Re: [PATCH 1/8] gpio: Add Elba SoC gpio driver for spi cs control

[Linus Walleij]

On Thu, Mar 4, 2021 at 10:10 AM Serge Semin <fancer.lancer@gmail.com> wrote:
> On Thu, Mar 04, 2021 at 09:29:33AM +0100, Linus Walleij wrote:

> > > + * pin:             3            2        |       1            0
> > > + * bit:         7------6------5------4----|---3------2------1------0
> > > + *             cs1  cs1_ovr  cs0  cs0_ovr |  cs1  cs1_ovr  cs0  cs0_ovr
> > > + *                        ssi1            |             ssi0
> > > + */
> > > +#define SPICS_PIN_SHIFT(pin)   (2 * (pin))
> > > +#define SPICS_MASK(pin)                (0x3 << SPICS_PIN_SHIFT(pin))
> > > +#define SPICS_SET(pin, val)    ((((val) << 1) | 0x1) << SPICS_PIN_SHIFT(pin))
> >
>
> > So 2 bits per GPIO line in one register? (Nice doc!)
>
> I suppose the first bit is the CS-pin-override flag. So when it's set
> the output is directly driven by the second bit, otherwise the
> corresponding DW APB SPI controller drives it. That's how the
> multiplexing is implemented here.

If these output lines are so tightly coupled to the SPI block
and will not be used for any other GPO (general purpose output)
I think it makes more sense to bundle the handling into the
DW SPI driver, and activate it based on the Elba compatible
string (if of_is_compatible(...)).

I am a bit cautious because it has happened in the past that
people repurpose CS lines who were originally for SPI CS
to all kind of other purposes, such as a power-on LED and
in that case it needs to be a separate GPIO driver. So the
author needs to have a good idea about what is a realistic
use case here.

Yours,
Linus Walleij

Re: [PATCH 1/8] gpio: Add Elba SoC gpio driver for spi cs control

[Brad Larson]

Hi Linus,

On Thu, Mar 4, 2021 at 5:38 AM Linus Walleij <linus.walleij@linaro.org> wrote:
>
> On Thu, Mar 4, 2021 at 10:10 AM Serge Semin <fancer.lancer@gmail.com> wrote:
> > On Thu, Mar 04, 2021 at 09:29:33AM +0100, Linus Walleij wrote:
>
> > > > + * pin:             3            2        |       1            0
> > > > + * bit:         7------6------5------4----|---3------2------1------0
> > > > + *             cs1  cs1_ovr  cs0  cs0_ovr |  cs1  cs1_ovr  cs0  cs0_ovr
> > > > + *                        ssi1            |             ssi0
> > > > + */
> > > > +#define SPICS_PIN_SHIFT(pin)   (2 * (pin))
> > > > +#define SPICS_MASK(pin)                (0x3 << SPICS_PIN_SHIFT(pin))
> > > > +#define SPICS_SET(pin, val)    ((((val) << 1) | 0x1) << SPICS_PIN_SHIFT(pin))
> > >
> >
> > > So 2 bits per GPIO line in one register? (Nice doc!)
> >
> > I suppose the first bit is the CS-pin-override flag. So when it's set
> > the output is directly driven by the second bit, otherwise the
> > corresponding DW APB SPI controller drives it. That's how the
> > multiplexing is implemented here.
>
> If these output lines are so tightly coupled to the SPI block
> and will not be used for any other GPO (general purpose output)
> I think it makes more sense to bundle the handling into the
> DW SPI driver, and activate it based on the Elba compatible
> string (if of_is_compatible(...)).
>
> I am a bit cautious because it has happened in the past that
> people repurpose CS lines who were originally for SPI CS
> to all kind of other purposes, such as a power-on LED and
> in that case it needs to be a separate GPIO driver. So the
> author needs to have a good idea about what is a realistic
> use case here.

The gpio pins being used for the Elba SoC SPI CS are dedicated to this
function.  Are you recommending that the code in
drivers/gpio/gpio-elba-spics.c be integrated into
drivers/spi/spi-dw-mmio.c?

Regards,
Brad

Re: [PATCH 1/8] gpio: Add Elba SoC gpio driver for spi cs control

[Linus Walleij]

On Mon, Aug 23, 2021 at 3:06 AM Brad Larson <brad@pensando.io> wrote:

> The gpio pins being used for the Elba SoC SPI CS are dedicated to this
> function.  Are you recommending that the code in
> drivers/gpio/gpio-elba-spics.c be integrated into
> drivers/spi/spi-dw-mmio.c?

That makes most sense does it not?

Special purpose pins should be managed by that special purpose
hardware driver, DW SPI in this case.

The compatible string etc should be enough to determine that we
need some extra GPIO control here, possibly specify extra registers
for the SPI host etc.

The struct spi_master has a special callback .set_cs() and you
should make this behave special for your special hardware.
In the case of the DW driver it appears that even subdrivers can
pass a custom version of this call in struct dw_spi.

Yours,
Linus Walleij

Re: [PATCH 1/8] gpio: Add Elba SoC gpio driver for spi cs control

[Brad Larson]

On Sun, Aug 29, 2021 at 2:09 PM Linus Walleij <linus.walleij@linaro.org> wrote:
>
> On Mon, Aug 23, 2021 at 3:06 AM Brad Larson <brad@pensando.io> wrote:
>
> > The gpio pins being used for the Elba SoC SPI CS are dedicated to this
> > function.  Are you recommending that the code in
> > drivers/gpio/gpio-elba-spics.c be integrated into
> > drivers/spi/spi-dw-mmio.c?
>
> That makes most sense does it not?
>
> Special purpose pins should be managed by that special purpose
> hardware driver, DW SPI in this case.
>
> The compatible string etc should be enough to determine that we
> need some extra GPIO control here, possibly specify extra registers
> for the SPI host etc.
>
> The struct spi_master has a special callback .set_cs() and you
> should make this behave special for your special hardware.
> In the case of the DW driver it appears that even subdrivers can
> pass a custom version of this call in struct dw_spi.
>
> Yours,
> Linus Walleij

Yes that works, please see the diff below where the file
gpio-elba-spics.c goes away.  The original implementation was
motivated by gpio-spear-spics.c.

Best,
Brad

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index d3b509e175e47..14751c7ccd1f4 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -240,11 +240,6 @@ config GPIO_EIC_SPRD
  help
   Say yes here to support Spreadtrum EIC device.

-config GPIO_ELBA_SPICS
- bool "Pensando Elba SoC SPI Chip Select as GPIO support"
- depends on ARCH_PENSANDO_ELBA_SOC
- def_bool y
-
 config GPIO_EM
  tristate "Emma Mobile GPIO"
  depends on (ARCH_EMEV2 || COMPILE_TEST) && OF_GPIO


diff --git a/arch/arm64/boot/dts/pensando/elba-asic-common.dtsi
b/arch/arm64/boot/dts/pensando/elba-asic-common.dtsi
index a59405dc5676d..17dfb0c91f84c 100644
--- a/arch/arm64/boot/dts/pensando/elba-asic-common.dtsi
+++ b/arch/arm64/boot/dts/pensando/elba-asic-common.dtsi
@@ -66,8 +66,8 @@

 &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>;
+ cs-gpios = <0>, <0>,
+ <&porta 1 GPIO_ACTIVE_LOW>, <&porta 7 GPIO_ACTIVE_LOW>;
  status = "okay";
  spi0_cs0@0 {
  compatible = "pensando,cpld";


diff --git a/arch/arm64/boot/dts/pensando/elba.dtsi
b/arch/arm64/boot/dts/pensando/elba.dtsi
index 029dd5f0045f3..3ff4c39815639 100644
--- a/arch/arm64/boot/dts/pensando/elba.dtsi
+++ b/arch/arm64/boot/dts/pensando/elba.dtsi
@@ -127,6 +127,7 @@
  spi0: spi@2800 {
  compatible = "pensando,elba-spi";
  reg = <0x0 0x2800 0x0 0x100>;
+ pensando,spics = <&mssoc 0x2468 0>;
  clocks = <&ahb_clk>;
  interrupts = <GIC_SPI 9 IRQ_TYPE_LEVEL_HIGH>;
  #address-cells = <1>;
@@ -257,13 +258,6 @@
  reg = <0x0 0x307c2080 0x0 0x4>;
  };

- spics: spics@307c2468 {
- compatible = "pensando,elba-spics";
- reg = <0x0 0x307c2468 0x0 0x4>;
- gpio-controller;
- #gpio-cells = <2>;
- };
-
  pcie@307c2480 {
  compatible = "pensando,pcie";
  reg = <0x0 0x307c2480 0x0 0x4 /* MS CFG_WDT */


diff --git a/drivers/spi/spi-dw-mmio.c b/drivers/spi/spi-dw-mmio.c
index 3379720cfcb8..64644bae8923 100644
--- a/drivers/spi/spi-dw-mmio.c
+++ b/drivers/spi/spi-dw-mmio.c
@@ -16,6 +16,7 @@
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/of_platform.h>
+#include <linux/gpio.h>
 #include <linux/acpi.h>
 #include <linux/property.h>
 #include <linux/regmap.h>
@@ -53,6 +54,24 @@ struct dw_spi_mscc {
        void __iomem        *spi_mst; /* Not sparx5 */
 };

+struct dw_spi_elba {
+       struct regmap *regmap;
+       unsigned int reg;
+       unsigned int ctl;
+};
+
+/*
+ * ctl:              1               |               0
+ * cs:       1               0       |       1               0
+ * bit:  7-------6-------5-------4---|---3-------2-------1-------0
+ *      cs1   cs1_ovr   cs0   cs0_ovr|  cs1   cs1_ovr   cs0   cs0_ovr
+ *                  ssi1             |            ssi0
+ */
+#define ELBA_SPICS_SHIFT(ctl, cs)      (4 * (ctl) + 2 * (cs))
+#define ELBA_SPICS_MASK(ctl, cs)       (0x3 << ELBA_SPICS_SHIFT(ctl, cs))
+#define ELBA_SPICS_SET(ctl, cs, val)   \
+                       ((((val) << 1) | 0x1) << ELBA_SPICS_SHIFT(ctl, 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 +256,74 @@ 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(dwselba->ctl, cs),
+                          ELBA_SPICS_SET(dwselba->ctl, 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", 2, 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];
+       dwselba->ctl = args.args[1];
+
+       /* 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 +438,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);

Re: [PATCH 1/8] gpio: Add Elba SoC gpio driver for spi cs control

[Linus Walleij]

On Mon, Oct 4, 2021 at 6:46 PM Brad Larson <brad@pensando.io> wrote:

> Yes that works, please see the diff below where the file
> gpio-elba-spics.c goes away.  The original implementation was
> motivated by gpio-spear-spics.c.

This looks good to me :)

Yours,
Linus Walleij

Re: [PATCH 1/8] gpio: Add Elba SoC gpio driver for spi cs control

[Brad Larson]

On Tue, Oct 12, 2021 at 4:52 PM Linus Walleij <linus.walleij@linaro.org> wrote:
>
> On Mon, Oct 4, 2021 at 6:46 PM Brad Larson <brad@pensando.io> wrote:
>
> > Yes that works, please see the diff below where the file
> > gpio-elba-spics.c goes away.  The original implementation was
> > motivated by gpio-spear-spics.c.
>
> This looks good to me :)
>
> Yours,
> Linus Walleij

Hi Linus,

:-)  It's better to not have to look at a related gpio driver file to
the spi-dw-mmio.c
driver and think it could possibly be used as general purpose gpio.

Here is a response summary per patch.  Should I start respinning the
patchset against
the latest linux-next tag?  The changes are merged to our production
5.10.28 kernel
and the next step is to re-spin the set against the latest linux-next
which has a newer dtc,
run checkpatch, etc.  For reference as this has been cooking for
awhile here is the
overview from V2 patchset cover letter.

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

Summary of response to V1/V2 patchset
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 removed.

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.
- Added syscon usage

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.

Best,
Brad

Re: [PATCH 1/8] gpio: Add Elba SoC gpio driver for spi cs control

[Brad Larson]

On Thu, Mar 4, 2021 at 12:29 AM Linus Walleij <linus.walleij@linaro.org> wrote:
>
> Hi Brad,
>
> thanks for your patch!
>
> On Thu, Mar 4, 2021 at 4:42 AM Brad Larson <brad@pensando.io> wrote:
>
> > This GPIO driver is for the Pensando Elba SoC which
> > provides control of four chip selects on two SPI busses.
> >
> > Signed-off-by: Brad Larson <brad@pensando.io>
> (...)
>
> > +#include <linux/gpio.h>
>
> Use this in new drivers:
> #include <linux/gpio/driver.h>
>
> > + * pin:             3            2        |       1            0
> > + * bit:         7------6------5------4----|---3------2------1------0
> > + *     cs1  cs1_ovr  cs0  cs0_ovr |  cs1  cs1_ovr  cs0  cs0_ovr
> > + *                ssi1            |             ssi0
> > + */
> > +#define SPICS_PIN_SHIFT(pin)   (2 * (pin))
> > +#define SPICS_MASK(pin)                (0x3 << SPICS_PIN_SHIFT(pin))
> > +#define SPICS_SET(pin, val)    ((((val) << 1) | 0x1) << SPICS_PIN_SHIFT(pin))
>
> So 2 bits per GPIO line in one register? (Nice doc!)
>
> > +struct elba_spics_priv {
> > +       void __iomem *base;
> > +       spinlock_t lock;
> > +       struct gpio_chip chip;
> > +};
> > +
> > +static int elba_spics_get_value(struct gpio_chip *chip, unsigned int pin)
> > +{
> > +       return -ENXIO;
> > +}
>
> Write a comment that the chip only supports output mode,
> because it repurposes SPI CS pins as generic GPIO out,
> maybe at the top of the file?
>

I'll add a comment regarding gpio pin mode.  Yes output
only mode as SPI chip-selects.

> I suppose these systems also actually (ab)use the SPI cs
> for things that are not really SPI CS? Because otherwise
> this could just be part of the SPI driver (native chip select).
>
> > +static const struct of_device_id ebla_spics_of_match[] = {
> > +       { .compatible = "pensando,elba-spics" },
>
> Have you documented this?

Yes in Documentation/devicetree/bindings, I'll double check
the content for completeness.  The SPI CS isn't used for
something else, the integrated DesignWare IP doesn't
support 4 chip-selects on two spi busses.

>
> Other than that this is a nice and complete driver.
>
> Yours,
> Linus Walleij

Thanks for the review!

Re: [PATCH 1/8] gpio: Add Elba SoC gpio driver for spi cs control

[Brad Larson]

Hi Linus,

On Thu, Mar 4, 2021 at 12:29 AM Linus Walleij <linus.walleij@linaro.org> wrote:
> On Thu, Mar 4, 2021 at 4:42 AM Brad Larson <brad@pensando.io> wrote:
>
> > This GPIO driver is for the Pensando Elba SoC which
> > provides control of four chip selects on two SPI busses.
[...]
> > +#include <linux/gpio.h>
>
> Use this in new drivers:
> #include <linux/gpio/driver.h>

The updated patchset will use linux/gpio/driver.h

> > + * pin:             3            2        |       1            0
> > + * bit:         7------6------5------4----|---3------2------1------0
> > + *     cs1  cs1_ovr  cs0  cs0_ovr |  cs1  cs1_ovr  cs0  cs0_ovr
> > + *                ssi1            |             ssi0
> > + */
> > +#define SPICS_PIN_SHIFT(pin)   (2 * (pin))
> > +#define SPICS_MASK(pin)                (0x3 << SPICS_PIN_SHIFT(pin))
> > +#define SPICS_SET(pin, val)    ((((val) << 1) | 0x1) << SPICS_PIN_SHIFT(pin))
>
> So 2 bits per GPIO line in one register? (Nice doc!)
>
> > +struct elba_spics_priv {
> > +       void __iomem *base;
> > +       spinlock_t lock;
> > +       struct gpio_chip chip;
> > +};
> > +
> > +static int elba_spics_get_value(struct gpio_chip *chip, unsigned int pin)
> > +{
> > +       return -ENXIO;
> > +}
>
> Write a comment that the chip only supports output mode,
> because it repurposes SPI CS pins as generic GPIO out,
> maybe at the top of the file?

The top of the file will look like this in the updated patchset.

 * Pensando Elba ASIC SPI chip select driver.  The SoC supports output
 * direction only as it uses a generic GPIO pin for SPI CS.

> I suppose these systems also actually (ab)use the SPI cs
> for things that are not really SPI CS? Because otherwise
> this could just be part of the SPI driver (native chip select).

The SPI cs are not used for any other purpose, we needed four chip
selects and native DW supports two.

> > +static const struct of_device_id ebla_spics_of_match[] = {
> > +       { .compatible = "pensando,elba-spics" },
>
> Have you documented this?

Yes as part of patchset v2: [PATCH v2 11/13] dt-bindings: gpio: Add
Pensando Elba SoC support
which documents "pensando,elba-spics" in new file
bindings/gpio/pensando,elba-spics.yaml.

Regards,
Brad

RE: [PATCH 1/8] gpio: Add Elba SoC gpio driver for spi cs control

[Elliott, Robert (Servers)]

> -----Original Message-----
> From: Brad Larson <brad@pensando.io>
> Sent: Wednesday, March 3, 2021 9:42 PM
> Subject: [PATCH 1/8] gpio: Add Elba SoC gpio driver for spi cs control
.
> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
...
> +config GPIO_ELBA_SPICS
> +	bool "Pensando Elba SPI chip-select"
> +	depends on ARCH_PENSANDO_ELBA_SOC
> +	help
> +	  Say yes here to support the Pensndo Elba SoC SPI chip-select
> driver

Pensndo should be Pensando

> diff --git a/drivers/gpio/gpio-elba-spics.c b/drivers/gpio/gpio-elba-spics.c
> + * Pensando Elba ASIC SPI chip select driver
...
> +MODULE_LICENSE("GPL v2");
> +MODULE_DESCRIPTION("Elba SPI chip-select driver");

I think it's conventional to include the company name there, so
start that with "Pensando Elba"

Also, "SoC" and "ASIC" are sometimes included after Elba, but sometimes
are not. Consistency might be helpful.

> +static const struct of_device_id ebla_spics_of_match[] = {
...
> +		.of_match_table = ebla_spics_of_match,

ebla should be elba

Re: [PATCH 1/8] gpio: Add Elba SoC gpio driver for spi cs control

[Brad Larson]

Hi Elliott,

On Thu, Mar 4, 2021 at 12:44 PM Elliott, Robert (Servers)
<elliott@hpe.com> wrote:
[...]
> > +config GPIO_ELBA_SPICS
> > +     bool "Pensando Elba SPI chip-select"
> > +     depends on ARCH_PENSANDO_ELBA_SOC
> > +     help
> > +       Say yes here to support the Pensndo Elba SoC SPI chip-select
> > driver
>
> Pensndo should be Pensando

Fixed the typo, thanks.

> > diff --git a/drivers/gpio/gpio-elba-spics.c b/drivers/gpio/gpio-elba-spics.c
> > + * Pensando Elba ASIC SPI chip select driver
> ...
> > +MODULE_LICENSE("GPL v2");
> > +MODULE_DESCRIPTION("Elba SPI chip-select driver");
>
> I think it's conventional to include the company name there, so
> start that with "Pensando Elba"

Ok, thanks.  BTW I deleted these lines as I should have used
builtin_platform_driver() and not a loadable module.

> Also, "SoC" and "ASIC" are sometimes included after Elba, but sometimes
> are not. Consistency might be helpful.
>
> > +static const struct of_device_id ebla_spics_of_match[] = {
> ...
> > +             .of_match_table = ebla_spics_of_match,
>
> ebla should be elba

Fixed the typo and using SoC which is more accurate.

Regards,
Brad

Re: [PATCH 1/8] gpio: Add Elba SoC gpio driver for spi cs control

[Krzysztof Kozlowski]

On 04/03/2021 04:41, Brad Larson wrote:
> This GPIO driver is for the Pensando Elba SoC which
> provides control of four chip selects on two SPI busses.
> 
> Signed-off-by: Brad Larson <brad@pensando.io>
> ---
>  drivers/gpio/Kconfig           |   6 ++
>  drivers/gpio/Makefile          |   1 +
>  drivers/gpio/gpio-elba-spics.c | 120 +++++++++++++++++++++++++++++++++
>  3 files changed, 127 insertions(+)
>  create mode 100644 drivers/gpio/gpio-elba-spics.c

(...)

> +static int elba_spics_probe(struct platform_device *pdev)
> +{
> +	struct elba_spics_priv *p;
> +	struct resource *res;
> +	int ret;
> +
> +	p = devm_kzalloc(&pdev->dev, sizeof(*p), GFP_KERNEL);
> +	if (!p)
> +		return -ENOMEM;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	p->base = devm_ioremap_resource(&pdev->dev, res);
> +	if (IS_ERR(p->base)) {
> +		dev_err(&pdev->dev, "failed to remap I/O memory\n");
> +		return PTR_ERR(p->base);
> +	}
> +	spin_lock_init(&p->lock);
> +	platform_set_drvdata(pdev, p);
> +
> +	p->chip.ngpio = 4;	/* 2 cs pins for spi0, and 2 for spi1 */
> +	p->chip.base = -1;
> +	p->chip.direction_input = elba_spics_direction_input;
> +	p->chip.direction_output = elba_spics_direction_output;
> +	p->chip.get = elba_spics_get_value;
> +	p->chip.set = elba_spics_set_value;
> +	p->chip.label = dev_name(&pdev->dev);
> +	p->chip.parent = &pdev->dev;
> +	p->chip.owner = THIS_MODULE;
> +
> +	ret = devm_gpiochip_add_data(&pdev->dev, &p->chip, p);
> +	if (ret) {
> +		dev_err(&pdev->dev, "unable to add gpio chip\n");
> +		return ret;
> +	}
> +
> +	dev_info(&pdev->dev, "elba spics registered\n");

Don't print trivial probe results, unless you print here something
useful. If you need it for debugging, keep it dev_dbg.

Best regards,
Krzysztof

Re: [PATCH 1/8] gpio: Add Elba SoC gpio driver for spi cs control

[Brad Larson]

Hi Krzysztof,

On Fri, Mar 5, 2021 at 3:25 AM Krzysztof Kozlowski
<krzysztof.kozlowski@canonical.com> wrote:
>
> On 04/03/2021 04:41, Brad Larson wrote:
> > This GPIO driver is for the Pensando Elba SoC which
> > provides control of four chip selects on two SPI busses.
[...]
> > +     ret = devm_gpiochip_add_data(&pdev->dev, &p->chip, p);
> > +     if (ret) {
> > +             dev_err(&pdev->dev, "unable to add gpio chip\n");
> > +             return ret;
> > +     }
> > +
> > +     dev_info(&pdev->dev, "elba spics registered\n");
>
> Don't print trivial probe results, unless you print here something
> useful. If you need it for debugging, keep it dev_dbg.

Removed that extraneous logging

Regards,
Brad

Re: [PATCH 1/8] gpio: Add Elba SoC gpio driver for spi cs control

[Geert Uytterhoeven]

Hi Brad,

On Thu, Mar 4, 2021 at 4:59 AM Brad Larson <brad@pensando.io> wrote:
> This GPIO driver is for the Pensando Elba SoC which
> provides control of four chip selects on two SPI busses.
>
> Signed-off-by: Brad Larson <brad@pensando.io>

Thanks for your patch!

> --- a/drivers/gpio/Kconfig
> +++ b/drivers/gpio/Kconfig
> @@ -241,6 +241,12 @@ config GPIO_EIC_SPRD
>         help
>           Say yes here to support Spreadtrum EIC device.
>
> +config GPIO_ELBA_SPICS
> +       bool "Pensando Elba SPI chip-select"
> +       depends on ARCH_PENSANDO_ELBA_SOC

Any specific reason this can't be "... || COMPILE_TEST"?

> +       help
> +         Say yes here to support the Pensndo Elba SoC SPI chip-select driver
> +
>  config GPIO_EM
>         tristate "Emma Mobile GPIO"
>         depends on (ARCH_EMEV2 || COMPILE_TEST) && OF_GPIO

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 1/8] gpio: Add Elba SoC gpio driver for spi cs control

[Brad Larson]

Hi Geert,

On Fri, Mar 5, 2021 at 5:57 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> Hi Brad,
>
> On Thu, Mar 4, 2021 at 4:59 AM Brad Larson <brad@pensando.io> wrote:
> > This GPIO driver is for the Pensando Elba SoC which
> > provides control of four chip selects on two SPI busses.
[...]
> > +config GPIO_ELBA_SPICS
> > +       bool "Pensando Elba SPI chip-select"
> > +       depends on ARCH_PENSANDO_ELBA_SOC
>
> Any specific reason this can't be "... || COMPILE_TEST"?

Added COMPILE_TEST

Regards,
Brad

Re: [PATCH 1/8] gpio: Add Elba SoC gpio driver for spi cs control

[Andy Shevchenko]

On Thu, Mar 4, 2021 at 4:40 PM Brad Larson <brad@pensando.io> wrote:
>
> This GPIO driver is for the Pensando Elba SoC which
> provides control of four chip selects on two SPI busses.

I will try to avoid repeating otheris in their reviews, but my comments below.

...

> +config GPIO_ELBA_SPICS
> +       bool "Pensando Elba SPI chip-select"

Can't it be a module? Why?

> +       depends on ARCH_PENSANDO_ELBA_SOC
> +       help
> +         Say yes here to support the Pensndo Elba SoC SPI chip-select driver

Please give more explanation what it is and why users might need it,
and also tell users how the module will be named (if there is no
strong argument why it can't be a  module).

...

> +#include <linux/of.h>

It's not used here, but you missed mod_devicetable.h.

...

> +/*
> + * pin:             3            2        |       1            0
> + * bit:         7------6------5------4----|---3------2------1------0
> + *     cs1  cs1_ovr  cs0  cs0_ovr |  cs1  cs1_ovr  cs0  cs0_ovr
> + *                ssi1            |             ssi0
> + */
> +#define SPICS_PIN_SHIFT(pin)   (2 * (pin))
> +#define SPICS_MASK(pin)                (0x3 << SPICS_PIN_SHIFT(pin))

> +#define SPICS_SET(pin, val)    ((((val) << 1) | 0x1) << SPICS_PIN_SHIFT(pin))

Isn't it easier to define as ((value) << (2 * (pin) + 1) | BIT(2 * (pin)))

...

> +struct elba_spics_priv {
> +       void __iomem *base;
> +       spinlock_t lock;

> +       struct gpio_chip chip;

If you put it as a first member a container_of() becomes a no-op. OTOH
dunno if there is any such container_of() use in the code.

> +};

...

> +static int elba_spics_get_value(struct gpio_chip *chip, unsigned int pin)
> +{
> +       return -ENXIO;

Hmm... Is it really acceptable error code here?

> +}
...

> +static int elba_spics_direction_input(struct gpio_chip *chip, unsigned int pin)
> +{
> +       return -ENXIO;

Ditto.

> +}

...

> +       res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +       p->base = devm_ioremap_resource(&pdev->dev, res);

p->base = devm_platform_ioremap_resource(pdev, 0);

> +       if (IS_ERR(p->base)) {

> +               dev_err(&pdev->dev, "failed to remap I/O memory\n");

Duplicate noisy message.

> +               return PTR_ERR(p->base);
> +       }

> +       ret = devm_gpiochip_add_data(&pdev->dev, &p->chip, p);
> +       if (ret) {
> +               dev_err(&pdev->dev, "unable to add gpio chip\n");

> +               return ret;
> +       }
> +
> +       dev_info(&pdev->dev, "elba spics registered\n");
> +       return 0;

if (ret)
  dev_err(...);
return ret;

> +}

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 1/8] gpio: Add Elba SoC gpio driver for spi cs control

[Brad Larson]

On Sun, Mar 7, 2021 at 11:21 AM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
>
> On Thu, Mar 4, 2021 at 4:40 PM Brad Larson <brad@pensando.io> wrote:
> >
> > This GPIO driver is for the Pensando Elba SoC which
> > provides control of four chip selects on two SPI busses.
>
> > +config GPIO_ELBA_SPICS
> > +       bool "Pensando Elba SPI chip-select"
>
> Can't it be a module? Why?

All Elba SoC based platforms require this driver to be built-in to boot and
removing the module would result in a variety of exceptions/errors.

> > +       depends on ARCH_PENSANDO_ELBA_SOC
> > +       help
> > +         Say yes here to support the Pensndo Elba SoC SPI chip-select driver
>
> Please give more explanation what it is and why users might need it,
> and also tell users how the module will be named (if there is no
> strong argument why it can't be a  module).
>
Fixed the typo.

> > +#include <linux/of.h>
>
> It's not used here, but you missed mod_devicetable.h.

Removed <linux/of.h>.  There is no dependency on mod_devicetable.h.

> ...
>
> > +/*
> > + * pin:             3            2        |       1            0
> > + * bit:         7------6------5------4----|---3------2------1------0
> > + *     cs1  cs1_ovr  cs0  cs0_ovr |  cs1  cs1_ovr  cs0  cs0_ovr
> > + *                ssi1            |             ssi0
> > + */
> > +#define SPICS_PIN_SHIFT(pin)   (2 * (pin))
> > +#define SPICS_MASK(pin)                (0x3 << SPICS_PIN_SHIFT(pin))
>
> > +#define SPICS_SET(pin, val)    ((((val) << 1) | 0x1) << SPICS_PIN_SHIFT(pin))
>
> Isn't it easier to define as ((value) << (2 * (pin) + 1) | BIT(2 * (pin)))
>
> ...
>
> > +struct elba_spics_priv {
> > +       void __iomem *base;
> > +       spinlock_t lock;
>
> > +       struct gpio_chip chip;
>
> If you put it as a first member a container_of() becomes a no-op. OTOH
> dunno if there is any such container_of() use in the code.
>

There is no use of container_of()

> > +static int elba_spics_get_value(struct gpio_chip *chip, unsigned int pin)
> > +{
> > +       return -ENXIO;
>
> Hmm... Is it really acceptable error code here?
>
> > +static int elba_spics_direction_input(struct gpio_chip *chip, unsigned int pin)
> > +{
> > +       return -ENXIO;
>
> Ditto.
>
Changed both to -ENOTSUPP.

> > +       res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > +       p->base = devm_ioremap_resource(&pdev->dev, res);
>
> p->base = devm_platform_ioremap_resource(pdev, 0);

Implementation follows devm_ioremap_resource() example in lib/devres.c.

> > +       if (IS_ERR(p->base)) {
>
> > +               dev_err(&pdev->dev, "failed to remap I/O memory\n");
>
> Duplicate noisy message.
>
> > +               return PTR_ERR(p->base);
> > +       }
>
> > +       ret = devm_gpiochip_add_data(&pdev->dev, &p->chip, p);
> > +       if (ret) {
> > +               dev_err(&pdev->dev, "unable to add gpio chip\n");
>
> > +               return ret;
> > +       }
> > +
> > +       dev_info(&pdev->dev, "elba spics registered\n");
> > +       return 0;
>
> if (ret)
>   dev_err(...);
> return ret;

Cleaned this up in patchset v2.

Re: [PATCH 1/8] gpio: Add Elba SoC gpio driver for spi cs control

[Andy Shevchenko]

On Mon, Mar 29, 2021 at 4:19 AM Brad Larson <brad@pensando.io> wrote:
> On Sun, Mar 7, 2021 at 11:21 AM Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:
> > On Thu, Mar 4, 2021 at 4:40 PM Brad Larson <brad@pensando.io> wrote:

...

> > > +config GPIO_ELBA_SPICS
> > > +       bool "Pensando Elba SPI chip-select"
> >
> > Can't it be a module? Why?
>
> All Elba SoC based platforms require this driver to be built-in to boot and
> removing the module would result in a variety of exceptions/errors.

Needs to be at least in the commit message.

> > > +       depends on ARCH_PENSANDO_ELBA_SOC
> > > +       help
> > > +         Say yes here to support the Pensndo Elba SoC SPI chip-select driver
> >
> > Please give more explanation what it is and why users might need it,
> > and also tell users how the module will be named (if there is no
> > strong argument why it can't be a  module).
> >
> Fixed the typo.

Yeah, according to the above, you better elaborate what this module is
and why people would need it.
Also can be a good hint to add
default ARCH_MY_COOL_PLATFORM

...

> > > +#include <linux/of.h>
> >
> > It's not used here, but you missed mod_devicetable.h.
>
> Removed <linux/of.h>.  There is no dependency on mod_devicetable.h.

What do you mean? You don't use data structures from that?
of_device_id or other ID structures are defined there. Your module
works without them?

...

> > > +       res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > > +       p->base = devm_ioremap_resource(&pdev->dev, res);
> >
> > p->base = devm_platform_ioremap_resource(pdev, 0);
>
> Implementation follows devm_ioremap_resource() example in lib/devres.c.

So? How does this make it impossible to address my comment?

> > > +       if (IS_ERR(p->base)) {
> >
> > > +               dev_err(&pdev->dev, "failed to remap I/O memory\n");
> >
> > Duplicate noisy message.
> >
> > > +               return PTR_ERR(p->base);
> > > +       }

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 1/8] gpio: Add Elba SoC gpio driver for spi cs control

[Brad Larson]

Hi Andy,

On Mon, Mar 29, 2021 at 3:40 AM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
>
> On Mon, Mar 29, 2021 at 4:19 AM Brad Larson <brad@pensando.io> wrote:
> > On Sun, Mar 7, 2021 at 11:21 AM Andy Shevchenko
> > <andy.shevchenko@gmail.com> wrote:
> > > On Thu, Mar 4, 2021 at 4:40 PM Brad Larson <brad@pensando.io> wrote:
>
> ...
>
> > > > +config GPIO_ELBA_SPICS
> > > > +       bool "Pensando Elba SPI chip-select"
> > >
> > > Can't it be a module? Why?
> >
> > All Elba SoC based platforms require this driver to be built-in to boot and
> > removing the module would result in a variety of exceptions/errors.
>
> Needs to be at least in the commit message.
>
>
>
> > > > +       depends on ARCH_PENSANDO_ELBA_SOC
> > > > +       help
> > > > +         Say yes here to support the Pensndo Elba SoC SPI chip-select driver
> > >
> > > Please give more explanation what it is and why users might need it,
> > > and also tell users how the module will be named (if there is no
> > > strong argument why it can't be a  module).
> > >
> > Fixed the typo.
>
> Yeah, according to the above, you better elaborate what this module is
> and why people would need it.
> Also can be a good hint to add
> default ARCH_MY_COOL_PLATFORM

Regarding the above module question and Kconfig definition, since I
first looked at this and reviewed the comments I realized I should be
using builtin.  The file gpio/Kconfig is currently this

config GPIO_ELBA_SPICS
        def_bool y
        depends on ARCH_PENSANDO_ELBA_SOC || COMPILE_TEST

> ...
>
> > > > +#include <linux/of.h>
> > >
> > > It's not used here, but you missed mod_devicetable.h.
> >
> > Removed <linux/of.h>.  There is no dependency on mod_devicetable.h.
>
> What do you mean? You don't use data structures from that?
> of_device_id or other ID structures are defined there. Your module
> works without them?
>
I typed the wrong filename.  I do still have <linux/of.h>

> > > > +       res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > > > +       p->base = devm_ioremap_resource(&pdev->dev, res);
> > >
> > > p->base = devm_platform_ioremap_resource(pdev, 0);
> >
> > Implementation follows devm_ioremap_resource() example in lib/devres.c.
>
> So? How does this make it impossible to address my comment?

I was simply stating that I followed the recommended API per the
source code although I don't recall if I was looking at 4.14, 5.10 or
linux-next at the time.  Changed to using
devm_platform_ioremap_resource().

> > > > +       if (IS_ERR(p->base)) {
> > >
> > > > +               dev_err(&pdev->dev, "failed to remap I/O memory\n");
> > >
> > > Duplicate noisy message.
> > >
> > > > +               return PTR_ERR(p->base);
> > > > +       }

Yep, I've removed the extraneous log message.

Regards,
Brad

Re: [PATCH 1/8] gpio: Add Elba SoC gpio driver for spi cs control

[Geert Uytterhoeven]

Hi Brad,

On Mon, Aug 23, 2021 at 3:14 AM Brad Larson <brad@pensando.io> wrote:
> On Mon, Mar 29, 2021 at 3:40 AM Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:
> >
> > On Mon, Mar 29, 2021 at 4:19 AM Brad Larson <brad@pensando.io> wrote:
> > > On Sun, Mar 7, 2021 at 11:21 AM Andy Shevchenko
> > > <andy.shevchenko@gmail.com> wrote:
> > > > On Thu, Mar 4, 2021 at 4:40 PM Brad Larson <brad@pensando.io> wrote:
> >
> > ...
> >
> > > > > +config GPIO_ELBA_SPICS
> > > > > +       bool "Pensando Elba SPI chip-select"
> > > >
> > > > Can't it be a module? Why?
> > >
> > > All Elba SoC based platforms require this driver to be built-in to boot and
> > > removing the module would result in a variety of exceptions/errors.
> >
> > Needs to be at least in the commit message.
> >
> > > > > +       depends on ARCH_PENSANDO_ELBA_SOC
> > > > > +       help
> > > > > +         Say yes here to support the Pensndo Elba SoC SPI chip-select driver

Pensando

> > > >
> > > > Please give more explanation what it is and why users might need it,
> > > > and also tell users how the module will be named (if there is no
> > > > strong argument why it can't be a  module).
> > > >
> > > Fixed the typo.
> >
> > Yeah, according to the above, you better elaborate what this module is
> > and why people would need it.
> > Also can be a good hint to add
> > default ARCH_MY_COOL_PLATFORM
>
> Regarding the above module question and Kconfig definition, since I
> first looked at this and reviewed the comments I realized I should be
> using builtin.  The file gpio/Kconfig is currently this
>
> config GPIO_ELBA_SPICS
>         def_bool y
>         depends on ARCH_PENSANDO_ELBA_SOC || COMPILE_TEST

That means the driver will default to yes by merely enabling
COMPILE_TEST, which is a no-go.

    config GPIO_ELBA_SPICS
            bool "one-line summary"
            depends on ARCH_PENSANDO_ELBA_SOC || COMPILE_TEST
            default y if ARCH_PENSANDO_ELBA_SOC

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 1/8] gpio: Add Elba SoC gpio driver for spi cs control

[Brad Larson]

Hi Geert,

On Mon, Aug 23, 2021 at 12:50 AM Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
>
> Hi Brad,
>
> On Mon, Aug 23, 2021 at 3:14 AM Brad Larson <brad@pensando.io> wrote:
> > On Mon, Mar 29, 2021 at 3:40 AM Andy Shevchenko
[...]
> > Regarding the above module question and Kconfig definition, since I
> > first looked at this and reviewed the comments I realized I should be
> > using builtin.  The file gpio/Kconfig is currently this
> >
> > config GPIO_ELBA_SPICS
> >         def_bool y
> >         depends on ARCH_PENSANDO_ELBA_SOC || COMPILE_TEST
>
> That means the driver will default to yes by merely enabling
> COMPILE_TEST, which is a no-go.
>
>     config GPIO_ELBA_SPICS
>             bool "one-line summary"
>             depends on ARCH_PENSANDO_ELBA_SOC || COMPILE_TEST
>             default y if ARCH_PENSANDO_ELBA_SOC

Thanks Geert, changed to this

--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -241,8 +241,9 @@ config GPIO_EIC_SPRD
          Say yes here to support Spreadtrum EIC device.

 config GPIO_ELBA_SPICS
+       bool "Pensando Elba SoC SPI Chip Select as GPIO support"
+       depends on ARCH_PENSANDO_ELBA_SOC
        def_bool y
-       depends on ARCH_PENSANDO_ELBA_SOC || COMPILE_TEST

Regards,
Brad

Re: [PATCH 1/8] gpio: Add Elba SoC gpio driver for spi cs control

[Geert Uytterhoeven]

Hi Brad,

On Mon, Aug 23, 2021 at 6:31 PM Brad Larson <brad@pensando.io> wrote:
> On Mon, Aug 23, 2021 at 12:50 AM Geert Uytterhoeven
> <geert@linux-m68k.org> wrote:
> > On Mon, Aug 23, 2021 at 3:14 AM Brad Larson <brad@pensando.io> wrote:
> > > On Mon, Mar 29, 2021 at 3:40 AM Andy Shevchenko
> [...]
> > > Regarding the above module question and Kconfig definition, since I
> > > first looked at this and reviewed the comments I realized I should be
> > > using builtin.  The file gpio/Kconfig is currently this
> > >
> > > config GPIO_ELBA_SPICS
> > >         def_bool y
> > >         depends on ARCH_PENSANDO_ELBA_SOC || COMPILE_TEST
> >
> > That means the driver will default to yes by merely enabling
> > COMPILE_TEST, which is a no-go.
> >
> >     config GPIO_ELBA_SPICS
> >             bool "one-line summary"
> >             depends on ARCH_PENSANDO_ELBA_SOC || COMPILE_TEST
> >             default y if ARCH_PENSANDO_ELBA_SOC
>
> Thanks Geert, changed to this
>
> --- a/drivers/gpio/Kconfig
> +++ b/drivers/gpio/Kconfig
> @@ -241,8 +241,9 @@ config GPIO_EIC_SPRD
>           Say yes here to support Spreadtrum EIC device.
>
>  config GPIO_ELBA_SPICS
> +       bool "Pensando Elba SoC SPI Chip Select as GPIO support"
> +       depends on ARCH_PENSANDO_ELBA_SOC
>         def_bool y
> -       depends on ARCH_PENSANDO_ELBA_SOC || COMPILE_TEST

So we're losing the COMPILE_TEST ability again?

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 1/8] gpio: Add Elba SoC gpio driver for spi cs control

[Brad Larson]

On Mon, Aug 23, 2021 at 1:11 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> Hi Brad,
>
> On Mon, Aug 23, 2021 at 6:31 PM Brad Larson <brad@pensando.io> wrote:
> > On Mon, Aug 23, 2021 at 12:50 AM Geert Uytterhoeven
> > <geert@linux-m68k.org> wrote:
> > > On Mon, Aug 23, 2021 at 3:14 AM Brad Larson <brad@pensando.io> wrote:
> > > > On Mon, Mar 29, 2021 at 3:40 AM Andy Shevchenko
> > [...]
> > > > Regarding the above module question and Kconfig definition, since I
> > > > first looked at this and reviewed the comments I realized I should be
> > > > using builtin.  The file gpio/Kconfig is currently this
> > > >
> > > > config GPIO_ELBA_SPICS
> > > >         def_bool y
> > > >         depends on ARCH_PENSANDO_ELBA_SOC || COMPILE_TEST
> > >
> > > That means the driver will default to yes by merely enabling
> > > COMPILE_TEST, which is a no-go.
> > >
> > >     config GPIO_ELBA_SPICS
> > >             bool "one-line summary"
> > >             depends on ARCH_PENSANDO_ELBA_SOC || COMPILE_TEST
> > >             default y if ARCH_PENSANDO_ELBA_SOC
> >
> > Thanks Geert, changed to this
> >
> > --- a/drivers/gpio/Kconfig
> > +++ b/drivers/gpio/Kconfig
> > @@ -241,8 +241,9 @@ config GPIO_EIC_SPRD
> >           Say yes here to support Spreadtrum EIC device.
> >
> >  config GPIO_ELBA_SPICS
> > +       bool "Pensando Elba SoC SPI Chip Select as GPIO support"
> > +       depends on ARCH_PENSANDO_ELBA_SOC
> >         def_bool y
> > -       depends on ARCH_PENSANDO_ELBA_SOC || COMPILE_TEST
>
> So we're losing the COMPILE_TEST ability again?
>

Hi Geert,

The gpio-elba-spics.c driver is being deleted with the spi chip-select
control integrated into spi-dw-mmio.c.  The GPIO_ELBA_SPICS config
option goes away and fixes my breakage of COMPILE_TEST.

Best,
Brad

Re: [PATCH 1/8] gpio: Add Elba SoC gpio driver for spi cs control

[Geert Uytterhoeven]

Hi Brad,

On Mon, Oct 4, 2021 at 7:14 PM Brad Larson <brad@pensando.io> wrote:
> On Mon, Aug 23, 2021 at 1:11 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > On Mon, Aug 23, 2021 at 6:31 PM Brad Larson <brad@pensando.io> wrote:
> > > On Mon, Aug 23, 2021 at 12:50 AM Geert Uytterhoeven
> > > <geert@linux-m68k.org> wrote:
> > > > On Mon, Aug 23, 2021 at 3:14 AM Brad Larson <brad@pensando.io> wrote:
> > > > > On Mon, Mar 29, 2021 at 3:40 AM Andy Shevchenko
> > > [...]
> > > > > Regarding the above module question and Kconfig definition, since I
> > > > > first looked at this and reviewed the comments I realized I should be
> > > > > using builtin.  The file gpio/Kconfig is currently this
> > > > >
> > > > > config GPIO_ELBA_SPICS
> > > > >         def_bool y
> > > > >         depends on ARCH_PENSANDO_ELBA_SOC || COMPILE_TEST
> > > >
> > > > That means the driver will default to yes by merely enabling
> > > > COMPILE_TEST, which is a no-go.
> > > >
> > > >     config GPIO_ELBA_SPICS
> > > >             bool "one-line summary"
> > > >             depends on ARCH_PENSANDO_ELBA_SOC || COMPILE_TEST
> > > >             default y if ARCH_PENSANDO_ELBA_SOC
> > >
> > > Thanks Geert, changed to this
> > >
> > > --- a/drivers/gpio/Kconfig
> > > +++ b/drivers/gpio/Kconfig
> > > @@ -241,8 +241,9 @@ config GPIO_EIC_SPRD
> > >           Say yes here to support Spreadtrum EIC device.
> > >
> > >  config GPIO_ELBA_SPICS
> > > +       bool "Pensando Elba SoC SPI Chip Select as GPIO support"
> > > +       depends on ARCH_PENSANDO_ELBA_SOC
> > >         def_bool y
> > > -       depends on ARCH_PENSANDO_ELBA_SOC || COMPILE_TEST
> >
> > So we're losing the COMPILE_TEST ability again?
> >
>
> Hi Geert,
>
> The gpio-elba-spics.c driver is being deleted with the spi chip-select
> control integrated into spi-dw-mmio.c.  The GPIO_ELBA_SPICS config
> option goes away and fixes my breakage of COMPILE_TEST.

OK. Thanks for the follow-up.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 1/8] gpio: Add Elba SoC gpio driver for spi cs control

[Brad Larson]

Hi Andy,

On Sun, Mar 7, 2021 at 11:21 AM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
>
> On Thu, Mar 4, 2021 at 4:40 PM Brad Larson <brad@pensando.io> wrote:
> >
> > This GPIO driver is for the Pensando Elba SoC which
> > provides control of four chip selects on two SPI busses.
>
> I will try to avoid repeating otheris in their reviews, but my comments below.
>
> ...
>
> > +config GPIO_ELBA_SPICS
> > +       bool "Pensando Elba SPI chip-select"
>
> Can't it be a module? Why?
>
> > +       depends on ARCH_PENSANDO_ELBA_SOC
> > +       help
> > +         Say yes here to support the Pensndo Elba SoC SPI chip-select driver
>
> Please give more explanation what it is and why users might need it,
> and also tell users how the module will be named (if there is no
> strong argument why it can't be a  module).
>
> ...
>
> > +#include <linux/of.h>
>
> It's not used here, but you missed mod_devicetable.h.

Based on the feedback I realized this should not be a loadable module.
I should be using builtin_platform_driver(elba_spics_driver).
Currently I have this for gpio/Kconfig

config GPIO_ELBA_SPICS
        def_bool y
        depends on ARCH_PENSANDO_ELBA_SOC || COMPILE_TEST

> > +/*
> > + * pin:             3            2        |       1            0
> > + * bit:         7------6------5------4----|---3------2------1------0
> > + *     cs1  cs1_ovr  cs0  cs0_ovr |  cs1  cs1_ovr  cs0  cs0_ovr
> > + *                ssi1            |             ssi0
> > + */
> > +#define SPICS_PIN_SHIFT(pin)   (2 * (pin))
> > +#define SPICS_MASK(pin)                (0x3 << SPICS_PIN_SHIFT(pin))
>
> > +#define SPICS_SET(pin, val)    ((((val) << 1) | 0x1) << SPICS_PIN_SHIFT(pin))
>
> Isn't it easier to define as ((value) << (2 * (pin) + 1) | BIT(2 * (pin)))

Both are functionally correct.  I don't have a preference, do you want
this change?

> > +struct elba_spics_priv {
> > +       void __iomem *base;
> > +       spinlock_t lock;
>
> > +       struct gpio_chip chip;
>
> If you put it as a first member a container_of() becomes a no-op. OTOH
> dunno if there is any such container_of() use in the code.

There is no use of container_of() for this structure

> > +static int elba_spics_get_value(struct gpio_chip *chip, unsigned int pin)
> > +{
> > +       return -ENXIO;
>
> Hmm... Is it really acceptable error code here?

No it's not, thanks.  Changed to -ENOTSUPP as gpio output direction
only is supported.

> > +static int elba_spics_direction_input(struct gpio_chip *chip, unsigned int pin)
> > +{
> > +       return -ENXIO;
>
> Ditto.

Changed to ENOTSUPP

> > +       res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > +       p->base = devm_ioremap_resource(&pdev->dev, res);
>
> p->base = devm_platform_ioremap_resource(pdev, 0);

Changed to single call to devm_platform_ioremap_resource(pdev, 0)

> > +       if (IS_ERR(p->base)) {
>
> > +               dev_err(&pdev->dev, "failed to remap I/O memory\n");
>
> Duplicate noisy message.

Removed extra log message

> > +               return PTR_ERR(p->base);
> > +       }
>
> > +       ret = devm_gpiochip_add_data(&pdev->dev, &p->chip, p);
> > +       if (ret) {
> > +               dev_err(&pdev->dev, "unable to add gpio chip\n");
>
> > +               return ret;
> > +       }
> > +
> > +       dev_info(&pdev->dev, "elba spics registered\n");
> > +       return 0;
>
> if (ret)
>   dev_err(...);
> return ret;

Yes, made this change and will include in v3 patchset

--- a/drivers/gpio/gpio-elba-spics.c
+++ b/drivers/gpio/gpio-elba-spics.c
@@ -91,13 +91,9 @@ static int elba_spics_probe(struct platform_device *pdev)
        ret = devm_gpiochip_add_data(&pdev->dev, &p->chip, p);
-       if (ret) {
+       if (ret)
                dev_err(&pdev->dev, "unable to add gpio chip\n");
-               return ret;
-       }
-
-       dev_info(&pdev->dev, "elba spics registered\n");
-       return 0;
+       return ret;

Regards,
Brad

[PATCH 2/8] spi: cadence-quadspi: Add QSPI support for Pensando Elba SoC

[Brad Larson]

Add QSPI controller support fo 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 442cc7c53a47..fb0d9b0bd596 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);
+
 		cqspi->rx_chan = NULL;
 		return dev_err_probe(&cqspi->pdev->dev, ret, "No Rx DMA available\n");
 	}
@@ -1632,6 +1633,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",
@@ -1649,6 +1654,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

Re: [PATCH 2/8] spi: cadence-quadspi: Add QSPI support for Pensando Elba SoC

[Arnd Bergmann]

On Thu, Mar 4, 2021 at 4:41 AM Brad Larson <brad@pensando.io> wrote:
>
> Add QSPI controller support fo 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 442cc7c53a47..fb0d9b0bd596 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);
> +
>                 cqspi->rx_chan = NULL;
>                 return dev_err_probe(&cqspi->pdev->dev, ret, "No Rx DMA available\n");
>         }

Please don't mix whitespace changes with code changes.

> @@ -1632,6 +1633,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",
> @@ -1649,6 +1654,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 */ }

As mentioned in my reply to the dts file, the compatible string needs to be
somewhat more specific.

I also wonder if it would be better to define separate DT properties for the
quirks at this point, so not every new SoC using this device needs to have
its own quirks definition.

       Arnd

[PATCH 3/8] spi: dw: Add support for Pensando Elba SoC SPI

[Brad Larson]

The Pensando Elba SoC uses a GPIO based chip select
for two DW SPI busses with each bus having two
chip selects.

Signed-off-by: Brad Larson <brad@pensando.io>
---
 drivers/spi/spi-dw-mmio.c | 35 ++++++++++++++++++++++++++++++++++-
 1 file changed, 34 insertions(+), 1 deletion(-)

diff --git a/drivers/spi/spi-dw-mmio.c b/drivers/spi/spi-dw-mmio.c
index 17c06039a74d..417bd2125c07 100644
--- a/drivers/spi/spi-dw-mmio.c
+++ b/drivers/spi/spi-dw-mmio.c
@@ -56,7 +56,7 @@ struct dw_spi_mscc {
 /*
  * The Designware SPI controller (referred to as master in the documentation)
  * automatically deasserts chip select when the tx fifo is empty. The chip
- * selects then needs to be either driven as GPIOs or, for the first 4 using the
+ * selects then needs to be either driven as GPIOs or, for the first 4 using
  * the SPI boot controller registers. the final chip select is an OR gate
  * between the Designware SPI controller and the SPI boot controller.
  */
@@ -237,6 +237,38 @@ static int dw_spi_canaan_k210_init(struct platform_device *pdev,
 	return 0;
 }
 
+static void dw_spi_elba_set_cs(struct spi_device *spi, bool enable)
+{
+	struct dw_spi *dws = spi_master_get_devdata(spi->master);
+
+	if (!enable) {
+		if (spi->cs_gpiod) {
+			/*
+			 * Using a GPIO-based chip-select, the DW SPI
+			 * controller still needs its own CS bit selected
+			 * to start the serial engine.  On Elba the specific
+			 * CS doesn't matter, so use CS0.
+			 */
+			dw_writel(dws, DW_SPI_SER, BIT(0));
+		} else {
+			/*
+			 * Using the intrinsic DW chip-select; set the
+			 * appropriate CS.
+			 */
+			dw_writel(dws, DW_SPI_SER, BIT(spi->chip_select));
+		}
+	} else
+		dw_writel(dws, DW_SPI_SER, 0);
+}
+
+static int dw_spi_elba_init(struct platform_device *pdev,
+			    struct dw_spi_mmio *dwsmmio)
+{
+	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 +383,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 3/8] spi: dw: Add support for Pensando Elba SoC SPI

[Serge Semin]

Hello Brad.
Thanks for the patch. See my comments below.

On Wed, Mar 03, 2021 at 07:41:36PM -0800, Brad Larson wrote:
> The Pensando Elba SoC uses a GPIO based chip select
> for two DW SPI busses with each bus having two
> chip selects.

I see a contradiction here. Normally GPIO-based chip-select is a
property of a platform, but not a SoC/CPU/MCU/etc. Most of the time
SoC SPI interfaces still get to have native CS pins, while at some
platform configurations (like in case of DW APB SPI, which doesn't
provide a way to directly toggle its native CSs) it's easier or even
safer to use GPIOs as CS signals. Of course theoretically a SoC could
be synthesized so it doesn't have native CS output pins, but only some
virtual internal CS flags, but I've never seen such. Anyway according
to the custom CS method below it's not your case because you still
provide support for SPI-devices handled by native CS (else branch in
the if (spi->cs_gpiod) {} else {} statement).

> 
> Signed-off-by: Brad Larson <brad@pensando.io>
> ---
>  drivers/spi/spi-dw-mmio.c | 35 ++++++++++++++++++++++++++++++++++-
>  1 file changed, 34 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/spi/spi-dw-mmio.c b/drivers/spi/spi-dw-mmio.c
> index 17c06039a74d..417bd2125c07 100644
> --- a/drivers/spi/spi-dw-mmio.c
> +++ b/drivers/spi/spi-dw-mmio.c
> @@ -56,7 +56,7 @@ struct dw_spi_mscc {
>  /*
>   * The Designware SPI controller (referred to as master in the documentation)
>   * automatically deasserts chip select when the tx fifo is empty. The chip
> - * selects then needs to be either driven as GPIOs or, for the first 4 using the
> + * selects then needs to be either driven as GPIOs or, for the first 4 using
>   * the SPI boot controller registers. the final chip select is an OR gate
>   * between the Designware SPI controller and the SPI boot controller.
>   */
> @@ -237,6 +237,38 @@ static int dw_spi_canaan_k210_init(struct platform_device *pdev,
>  	return 0;
>  }
>
  
> +static void dw_spi_elba_set_cs(struct spi_device *spi, bool enable)
> +{
> +	struct dw_spi *dws = spi_master_get_devdata(spi->master);
> +
> +	if (!enable) {
> +		if (spi->cs_gpiod) {
> +			/*
> +			 * Using a GPIO-based chip-select, the DW SPI
> +			 * controller still needs its own CS bit selected
> +			 * to start the serial engine.  On Elba the specific
> +			 * CS doesn't matter, so use CS0.
> +			 */
> +			dw_writel(dws, DW_SPI_SER, BIT(0));
> +		} else {
> +			/*
> +			 * Using the intrinsic DW chip-select; set the
> +			 * appropriate CS.
> +			 */
> +			dw_writel(dws, DW_SPI_SER, BIT(spi->chip_select));
> +		}
> -	} else
  +	} else {
> +		dw_writel(dws, DW_SPI_SER, 0);
  +	} /* See [1] */
> +}

The custom CS-method above doesn't look much different from the
dw_spi_set_cs() method defined in the spi-dw-core.o driver, except
having at least two problems:
1) It assumes that "enable" argument means the CS-enabling flag, while
in fact it's the CS-level which depending on the SPI_CS_HIGH flag
set/cleared will be 1/0 respectively if CS is supposed to be enabled.
That aspect has already been fixed in the dw_spi_set_cs() method.
2) The method enables CS[0] if GPIO-CS is used for a particular SPI
device. That will cause problems for a GPIO/native CS intermixed case
of having for instance one SPI-device connected to native CS[0] and
another one - to a GPIO. So trying to communicate with the second SPI
device you'll end up having the native CS[0] activated too thus
having an SPI transfer sent to two SPI-device at the same time.
Of course that's not what you'd want.

Anyway I don't really see why you even need a custom CS method here. A
generic method dw_spi_set_cs() shall work for your SPI interface.
If I am wrong, please explain why. Did you try the generic CS method
on your platform?

[1] Placing Braces and Spaces. Chapter 3). Documentation/process/coding-style.rst

> +
> +static int dw_spi_elba_init(struct platform_device *pdev,
> +			    struct dw_spi_mmio *dwsmmio)
> +{
> +	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 +383,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 },

If you agree with me and remove the custom CS-method defined above in
this patch, then all you'll need is just to add "pensando,elba-spi" here
with generic init-callback set - dw_spi_dw_apb_init.

Finally defining new compatible string requires the bindings update.
In the framework of DW APB SPI interface they are defined in:
Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
So you need to have that DT-schema accordingly altered.

The bindings note concerns the rest of the updates in your patchset too.

-Sergey

>  	{ /* end of table */}
>  };
>  MODULE_DEVICE_TABLE(of, dw_spi_mmio_of_match);
> -- 
> 2.17.1
> 

Re: [PATCH 3/8] spi: dw: Add support for Pensando Elba SoC SPI

[Brad Larson]

Hi Sergey,

Thanks again for the reviews.  I've been able to work on this recently
and test the changes using 5.10.28 on a production server.  I'm going
back to the beginning to reply to each comment and work towards
closure of open issues before preparing patchset v3 which will need to
be re-done against the latest linux-next.

On Wed, Mar 3, 2021 at 10:44 PM Serge Semin <fancer.lancer@gmail.com> wrote:
>
> Hello Brad.
> Thanks for the patch. See my comments below.
>
> On Wed, Mar 03, 2021 at 07:41:36PM -0800, Brad Larson wrote:
> > The Pensando Elba SoC uses a GPIO based chip select
> > for two DW SPI busses with each bus having two
> > chip selects.
>
> I see a contradiction here. Normally GPIO-based chip-select is a
> property of a platform, but not a SoC/CPU/MCU/etc. Most of the time
> SoC SPI interfaces still get to have native CS pins, while at some
> platform configurations (like in case of DW APB SPI, which doesn't
> provide a way to directly toggle its native CSs) it's easier or even
> safer to use GPIOs as CS signals. Of course theoretically a SoC could
> be synthesized so it doesn't have native CS output pins, but only some
> virtual internal CS flags, but I've never seen such. Anyway according
> to the custom CS method below it's not your case because you still
> provide support for SPI-devices handled by native CS (else branch in
> the if (spi->cs_gpiod) {} else {} statement).

The native DW CS is not supported, that code is removed which caused
the confusion.  The existing dw_spi_set_cs() works fine with the
updated version of this function being

/*
 * Using a GPIO-based chip-select, the DW SPI controller still needs
 * its own CS bit selected to start the serial engine.  On Elba the
 * specific CS doesn't matter, so use CS0.
 */
static void dw_spi_elba_set_cs(struct spi_device *spi, bool enable)
{
        spi->chip_select = 0;
        dw_spi_set_cs(spi, enable);
}

which is much better than the original version shown below

> > +static void dw_spi_elba_set_cs(struct spi_device *spi, bool enable)
> > +{
> > +     struct dw_spi *dws = spi_master_get_devdata(spi->master);
> > +
> > +     if (!enable) {
> > +             if (spi->cs_gpiod) {
> > +                     /*
> > +                      * Using a GPIO-based chip-select, the DW SPI
> > +                      * controller still needs its own CS bit selected
> > +                      * to start the serial engine.  On Elba the specific
> > +                      * CS doesn't matter, so use CS0.
> > +                      */
> > +                     dw_writel(dws, DW_SPI_SER, BIT(0));
> > +             } else {
> > +                     /*
> > +                      * Using the intrinsic DW chip-select; set the
> > +                      * appropriate CS.
> > +                      */
> > +                     dw_writel(dws, DW_SPI_SER, BIT(spi->chip_select));
> > +             }
> > -     } else
>   +     } else {
> > +             dw_writel(dws, DW_SPI_SER, 0);
>   +     } /* See [1] */
> > +}
>
> The custom CS-method above doesn't look much different from the
> dw_spi_set_cs() method defined in the spi-dw-core.o driver, except
> having at least two problems:
> 1) It assumes that "enable" argument means the CS-enabling flag, while
> in fact it's the CS-level which depending on the SPI_CS_HIGH flag
> set/cleared will be 1/0 respectively if CS is supposed to be enabled.
> That aspect has already been fixed in the dw_spi_set_cs() method.
> 2) The method enables CS[0] if GPIO-CS is used for a particular SPI
> device. That will cause problems for a GPIO/native CS intermixed case
> of having for instance one SPI-device connected to native CS[0] and
> another one - to a GPIO. So trying to communicate with the second SPI
> device you'll end up having the native CS[0] activated too thus
> having an SPI transfer sent to two SPI-device at the same time.
> Of course that's not what you'd want.
>
> Anyway I don't really see why you even need a custom CS method here. A
> generic method dw_spi_set_cs() shall work for your SPI interface.
> If I am wrong, please explain why. Did you try the generic CS method
> on your platform?
>
> [1] Placing Braces and Spaces. Chapter 3). Documentation/process/coding-style.rst

Yes, exactly.  The generic method dw_spi_set_cs() works ok and
correctly handles active high/low.

> > +static int dw_spi_elba_init(struct platform_device *pdev,
> > +                         struct dw_spi_mmio *dwsmmio)
> > +{
> > +     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 +383,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 },
>
> If you agree with me and remove the custom CS-method defined above in
> this patch, then all you'll need is just to add "pensando,elba-spi" here
> with generic init-callback set - dw_spi_dw_apb_init.

The existing dw_spi_set_cs() is now being used.  Using
dw_spi_dw_apb_init results in every spi transfer failing which is why
dw_spi_elba_init() is still proposed which results in set_cs calling
dw_spi_elba_set_cs().

> Finally defining new compatible string requires the bindings update.
> In the framework of DW APB SPI interface they are defined in:
> Documentation/devicetree/bindings/spi/snps,dw-apb-ssi.yaml
> So you need to have that DT-schema accordingly altered.
>
> The bindings note concerns the rest of the updates in your patchset too.
>
> -Sergey

Patchset v2 separated out the bindings updates.  There will be more
bindings needed for v3 of the patchset.  I won't be sending v3 until
all discussions are resolved.

Regards,
Brad

Re: [PATCH 3/8] spi: dw: Add support for Pensando Elba SoC SPI

[Linus Walleij]

On Thu, Mar 4, 2021 at 4:42 AM Brad Larson <brad@pensando.io> wrote:

> The Pensando Elba SoC uses a GPIO based chip select
> for two DW SPI busses with each bus having two
> chip selects.
>
> Signed-off-by: Brad Larson <brad@pensando.io>

I agree with Serge's comments here: the existing cs callback should
work for your SoC, you should only need the new compatible string.

I see why you need the special GPIO driver for this now, as that
is obviously driven by totally different hardware.

Yours,
Linus Walleij

Re: [PATCH 3/8] spi: dw: Add support for Pensando Elba SoC SPI

[Brad Larson]

On Thu, Mar 4, 2021 at 12:48 AM Linus Walleij <linus.walleij@linaro.org> wrote:

> On Thu, Mar 4, 2021 at 4:42 AM Brad Larson <brad@pensando.io> wrote:
>
> > The Pensando Elba SoC uses a GPIO based chip select
> > for two DW SPI busses with each bus having two
> > chip selects.
> >
> > Signed-off-by: Brad Larson <brad@pensando.io>
>
> I agree with Serge's comments here: the existing cs callback should
> work for your SoC, you should only need the new compatible string.
>
> I see why you need the special GPIO driver for this now, as that
> is obviously driven by totally different hardware.
>
> Yours,
> Linus Walleij

Thanks Serge and Linus for the review.

In the SPI driver, the reason we need our own set_cs function is that
our DW SPI controller only supports intrinsic 2 chip-select pins.

This is the standard DW set_cs function:

void dw_spi_set_cs(struct spi_device *spi, bool enable)
{
        struct dw_spi *dws = spi_controller_get_devdata(spi->controller);
        bool cs_high = !!(spi->mode & SPI_CS_HIGH);

        /*
         * DW SPI controller demands any native CS being set in order to
         * proceed with data transfer. So in order to activate the SPI
         * communications we must set a corresponding bit in the Slave
         * Enable register no matter whether the SPI core is configured to
         * support active-high or active-low CS level.
         */
        if (cs_high == enable)
                dw_writel(dws, DW_SPI_SER, BIT(spi->chip_select));
        else
                dw_writel(dws, DW_SPI_SER, 0);
}

The dw_writel function argument DW_SPI_SER, BIT(spi->chip_select)
works for chip-select 0 & 1, but not for 2 & 3, as the IP only
implements bits [1:0] in the DW_SPI_SER register.  In the Elba SoC we
require GPIO-style chip-selects, our own set_cs function, and we
always use bit 0 of DW_SPI_SER to start the serial machine, not as a
chip-select control.  In the dw_spi_set_cs() function the below else
clause is never taken and leads to confusion.

             } else {
                        /*
                         * Using the intrinsic DW chip-select; set the
                         * appropriate CS.
                         */
                        dw_writel(dws, DW_SPI_SER, BIT(spi->chip_select));
                }

This else clause will be removed in patch set V2.  I tried the generic
dw_spi_set_cs() thinking it would just start the serial machine while
the Elba spics drives the gpio chip select, that didn't work.  I will
take another look at it as I work on V2 of the patchset to see exactly
what breaks during spi init.

Best,
Brad

[PATCH 4/8] spidev: Add Pensando CPLD compatible

[Brad Larson]

Pensando Elba SoC platforms have a SPI connected CPLD
for platform management.

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

diff --git a/drivers/spi/spidev.c b/drivers/spi/spidev.c
index 8cb4d923aeaa..8b285852ce82 100644
--- a/drivers/spi/spidev.c
+++ b/drivers/spi/spidev.c
@@ -683,6 +683,7 @@ static const struct of_device_id spidev_dt_ids[] = {
 	{ .compatible = "dh,dhcom-board" },
 	{ .compatible = "menlo,m53cpld" },
 	{ .compatible = "cisco,spi-petra" },
+	{ .compatible = "pensando,cpld" },
 	{},
 };
 MODULE_DEVICE_TABLE(of, spidev_dt_ids);
-- 
2.17.1

Re: [PATCH 4/8] spidev: Add Pensando CPLD compatible

[Arnd Bergmann]

On Thu, Mar 4, 2021 at 4:41 AM Brad Larson <brad@pensando.io> wrote:
>
> Pensando Elba SoC platforms have a SPI connected CPLD
> for platform management.
>
> Signed-off-by: Brad Larson <brad@pensando.io>
> ---
>  drivers/spi/spidev.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/spi/spidev.c b/drivers/spi/spidev.c
> index 8cb4d923aeaa..8b285852ce82 100644
> --- a/drivers/spi/spidev.c
> +++ b/drivers/spi/spidev.c
> @@ -683,6 +683,7 @@ static const struct of_device_id spidev_dt_ids[] = {
>         { .compatible = "dh,dhcom-board" },
>         { .compatible = "menlo,m53cpld" },
>         { .compatible = "cisco,spi-petra" },
> +       { .compatible = "pensando,cpld" },
>         {},
>  };

This does not seem appropriate, I think a platform management driver should
have a proper kernel abstraction instead of a user passthrough.

As mentioned elsewhere, it also needs to be way more specific. If this
is a programmable block, the compatible string might in fact need to
contain both a board identifier and a revision number for the programmable
logic, to ensure that the driver knows how to talk to it.

      Arnd

[PATCH 5/8] mmc: sdhci-cadence: Add Pensando Elba SoC support

[Brad Larson]

Add support for Pensando Elba SoC which explicitly controls
byte-lane enables on writes.  Refactor to allow platform
specific write ops.

Signed-off-by: Brad Larson <brad@pensando.io>
---
 drivers/mmc/host/Kconfig              |  15 +++
 drivers/mmc/host/Makefile             |   1 +
 drivers/mmc/host/sdhci-cadence-elba.c | 137 ++++++++++++++++++++++++++
 drivers/mmc/host/sdhci-cadence.c      |  78 +++++++--------
 drivers/mmc/host/sdhci-cadence.h      |  68 +++++++++++++
 5 files changed, 257 insertions(+), 42 deletions(-)
 create mode 100644 drivers/mmc/host/sdhci-cadence-elba.c
 create mode 100644 drivers/mmc/host/sdhci-cadence.h

diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
index b236dfe2e879..65ea323c06f2 100644
--- a/drivers/mmc/host/Kconfig
+++ b/drivers/mmc/host/Kconfig
@@ -250,6 +250,21 @@ config MMC_SDHCI_CADENCE
 
 	  If unsure, say N.
 
+config MMC_SDHCI_CADENCE_ELBA
+	tristate "SDHCI support for the Pensando/Cadence SD/SDIO/eMMC controller"
+	depends on ARCH_PENSANDO_ELBA_SOC
+	depends on MMC_SDHCI
+	depends on OF
+	depends on MMC_SDHCI_CADENCE
+	depends on MMC_SDHCI_PLTFM
+	select MMC_SDHCI_IO_ACCESSORS
+	help
+	  This selects the Pensando/Cadence SD/SDIO/eMMC controller.
+
+	  If you have a controller with this interface, say Y or M here.
+
+	  If unsure, say N.
+
 config MMC_SDHCI_CNS3XXX
 	tristate "SDHCI support on the Cavium Networks CNS3xxx SoC"
 	depends on ARCH_CNS3XXX || COMPILE_TEST
diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile
index 6df5c4774260..f2a6d50e64de 100644
--- a/drivers/mmc/host/Makefile
+++ b/drivers/mmc/host/Makefile
@@ -80,6 +80,7 @@ obj-$(CONFIG_MMC_REALTEK_USB)	+= rtsx_usb_sdmmc.o
 
 obj-$(CONFIG_MMC_SDHCI_PLTFM)		+= sdhci-pltfm.o
 obj-$(CONFIG_MMC_SDHCI_CADENCE)		+= sdhci-cadence.o
+obj-$(CONFIG_MMC_SDHCI_CADENCE_ELBA)	+= sdhci-cadence-elba.o
 obj-$(CONFIG_MMC_SDHCI_CNS3XXX)		+= sdhci-cns3xxx.o
 obj-$(CONFIG_MMC_SDHCI_ESDHC_MCF)       += sdhci-esdhc-mcf.o
 obj-$(CONFIG_MMC_SDHCI_ESDHC_IMX)	+= sdhci-esdhc-imx.o
diff --git a/drivers/mmc/host/sdhci-cadence-elba.c b/drivers/mmc/host/sdhci-cadence-elba.c
new file mode 100644
index 000000000000..e128e83e9be9
--- /dev/null
+++ b/drivers/mmc/host/sdhci-cadence-elba.c
@@ -0,0 +1,137 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (C) 2020-2021 Pensando Systems, Inc.
+ */
+
+#include <linux/bitfield.h>
+#include <linux/bits.h>
+#include <linux/iopoll.h>
+#include <linux/module.h>
+#include <linux/mmc/host.h>
+#include <linux/mmc/mmc.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+
+#include "sdhci-pltfm.h"
+#include "sdhci-cadence.h"
+
+// delay regs address
+#define SDIO_REG_HRS4		0x10
+#define REG_DELAY_HS		0x00
+#define REG_DELAY_DEFAULT	0x01
+#define REG_DELAY_UHSI_SDR50	0x04
+#define REG_DELAY_UHSI_DDR50	0x05
+
+static void elba_write_l(struct sdhci_host *host, u32 val, int reg)
+{
+	struct sdhci_cdns_priv *priv = sdhci_cdns_priv(host);
+	unsigned long flags;
+
+	spin_lock_irqsave(&priv->wrlock, flags);
+	writel(0x78, priv->ctl_addr);
+	writel(val, host->ioaddr + reg);
+	spin_unlock_irqrestore(&priv->wrlock, flags);
+}
+
+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 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 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 void sd4_set_dlyvr(struct sdhci_host *host,
+			  unsigned char addr, unsigned char data)
+{
+	unsigned long dlyrv_reg;
+
+	dlyrv_reg = ((unsigned long)data << 8);
+	dlyrv_reg |= addr;
+
+	// set data and address
+	writel(dlyrv_reg, host->ioaddr + SDIO_REG_HRS4);
+	dlyrv_reg |= (1uL << 24uL);
+	// send write request
+	writel(dlyrv_reg, host->ioaddr + SDIO_REG_HRS4);
+	dlyrv_reg &= ~(1uL << 24);
+	// clear write request
+	writel(dlyrv_reg, host->ioaddr + SDIO_REG_HRS4);
+}
+
+static void phy_config(struct sdhci_host *host)
+{
+	sd4_set_dlyvr(host, REG_DELAY_DEFAULT, 0x04);
+	sd4_set_dlyvr(host, REG_DELAY_HS, 0x04);
+	sd4_set_dlyvr(host, REG_DELAY_UHSI_SDR50, 0x06);
+	sd4_set_dlyvr(host, REG_DELAY_UHSI_DDR50, 0x16);
+}
+
+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_ioremap_resource(&pdev->dev, iomem);
+	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);
+	phy_config(host);
+	return 0;
+}
+
+const struct sdhci_cdns_drv_data sdhci_elba_drv_data = {
+	.init = elba_drv_init,
+	.pltfm_data = {
+		.ops = &sdhci_elba_ops,
+	},
+};
diff --git a/drivers/mmc/host/sdhci-cadence.c b/drivers/mmc/host/sdhci-cadence.c
index 6f2de54a5987..3c6bc93d3314 100644
--- a/drivers/mmc/host/sdhci-cadence.c
+++ b/drivers/mmc/host/sdhci-cadence.c
@@ -14,6 +14,7 @@
 #include <linux/of_device.h>
 
 #include "sdhci-pltfm.h"
+#include "sdhci-cadence.h"
 
 /* HRS - Host Register Set (specific to Cadence) */
 #define SDHCI_CDNS_HRS04		0x10		/* PHY access port */
@@ -59,23 +60,6 @@
  */
 #define SDHCI_CDNS_MAX_TUNING_LOOP	40
 
-struct sdhci_cdns_phy_param {
-	u8 addr;
-	u8 data;
-};
-
-struct sdhci_cdns_priv {
-	void __iomem *hrs_addr;
-	bool enhanced_strobe;
-	unsigned int nr_phy_params;
-	struct sdhci_cdns_phy_param phy_params[];
-};
-
-struct sdhci_cdns_phy_cfg {
-	const char *property;
-	u8 addr;
-};
-
 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, },
@@ -104,17 +88,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);
@@ -167,14 +151,7 @@ static int sdhci_cdns_phy_init(struct sdhci_cdns_priv *priv)
 	return 0;
 }
 
-static void *sdhci_cdns_priv(struct sdhci_host *host)
-{
-	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
-
-	return sdhci_pltfm_priv(pltfm_host);
-}
-
-static unsigned int sdhci_cdns_get_timeout_clock(struct sdhci_host *host)
+unsigned int sdhci_cdns_get_timeout_clock(struct sdhci_host *host)
 {
 	/*
 	 * Cadence's spec says the Timeout Clock Frequency is the same as the
@@ -191,7 +168,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 +200,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),
@@ -275,7 +252,7 @@ static int sdhci_cdns_execute_tuning(struct sdhci_host *host, u32 opcode)
 	return sdhci_cdns_set_tune_val(host, end_of_streak - max_streak / 2);
 }
 
-static void sdhci_cdns_set_uhs_signaling(struct sdhci_host *host,
+void sdhci_cdns_set_uhs_signaling(struct sdhci_host *host,
 					 unsigned int timing)
 {
 	struct sdhci_cdns_priv *priv = sdhci_cdns_priv(host);
@@ -318,13 +295,17 @@ 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_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 +331,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 +350,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,11 +370,18 @@ 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;
-	sdhci_enable_v4_mode(host);
-	__sdhci_read_caps(host, &version, NULL, NULL);
 
 	sdhci_get_of_property(pdev);
 
+	if (data->init) {
+		ret = data->init(pdev);
+		if (ret)
+			goto free;
+	}
+
+	sdhci_enable_v4_mode(host);
+	__sdhci_read_caps(host, &version, NULL, NULL);
+
 	ret = mmc_of_parse(host->mmc);
 	if (ret)
 		goto free;
@@ -453,8 +441,14 @@ 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,
 	},
+#ifdef CONFIG_MMC_SDHCI_CADENCE_ELBA
+	{
+		.compatible = "pensando,elba-emmc",
+		.data = &sdhci_elba_drv_data
+	},
+#endif
 	{ .compatible = "cdns,sd4hc" },
 	{ /* sentinel */ }
 };
diff --git a/drivers/mmc/host/sdhci-cadence.h b/drivers/mmc/host/sdhci-cadence.h
new file mode 100644
index 000000000000..bf48e8d13430
--- /dev/null
+++ b/drivers/mmc/host/sdhci-cadence.h
@@ -0,0 +1,68 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Copyright (C) 2016 Socionext Inc.
+ *   Author: Masahiro Yamada <yamada.masahiro@socionext.com>
+ */
+
+#ifndef _SDHCI_CADENCE_H_
+#define _SDHCI_CADENCE_H_
+
+struct sdhci_cdns_phy_param {
+	u8 addr;
+	u8 data;
+};
+
+struct sdhci_cdns_priv {
+	void __iomem *hrs_addr;
+#ifdef CONFIG_MMC_SDHCI_CADENCE_ELBA
+	void __iomem *ctl_addr;	/* write control */
+	spinlock_t wrlock;	/* write lock */
+#endif
+	bool enhanced_strobe;
+	void (*priv_write_l)(struct sdhci_cdns_priv *priv, u32 val,
+			     void __iomem *reg); /* for cadence-elba.c */
+	unsigned int nr_phy_params;
+	struct sdhci_cdns_phy_param phy_params[];
+};
+
+struct sdhci_cdns_phy_cfg {
+	const char *property;
+	u8 addr;
+};
+
+struct sdhci_cdns_drv_data {
+	int (*init)(struct platform_device *pdev);
+	const struct sdhci_pltfm_data pltfm_data;
+};
+
+static inline void *sdhci_cdns_priv(struct sdhci_host *host)
+{
+	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+
+	return sdhci_pltfm_priv(pltfm_host);
+}
+
+/*
+ * The Pensando Elba SoC explicitly controls byte-lane enables on writes,
+ * which includes writes to the HRS registers.
+ * sdhci_cdns_priv_writel() is used in the common sdhci-cadence.c code
+ * to write HRS registers, and this function dispatches to the specific
+ * code.
+ */
+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);
+}
+
+#ifdef CONFIG_MMC_SDHCI_CADENCE_ELBA
+extern const struct sdhci_cdns_drv_data sdhci_elba_drv_data;
+#endif
+
+unsigned int sdhci_cdns_get_timeout_clock(struct sdhci_host *host);
+void sdhci_cdns_set_uhs_signaling(struct sdhci_host *host, unsigned int timing);
+
+#endif
-- 
2.17.1

Re: [PATCH 5/8] mmc: sdhci-cadence: Add Pensando Elba SoC support

[Arnd Bergmann]

On Thu, Mar 4, 2021 at 4:41 AM Brad Larson <brad@pensando.io> wrote:

> +
> +static void elba_write_l(struct sdhci_host *host, u32 val, int reg)
> +{
> +       struct sdhci_cdns_priv *priv = sdhci_cdns_priv(host);
> +       unsigned long flags;
> +
> +       spin_lock_irqsave(&priv->wrlock, flags);
> +       writel(0x78, priv->ctl_addr);
> +       writel(val, host->ioaddr + reg);
> +       spin_unlock_irqrestore(&priv->wrlock, flags);
> +}

Please be aware that the spinlock does not actually guarantee serializing
a series of mmio writes unless the MMIO mapping is non-posted, or
you follow it with a readl() from the same device before the spin_unlock().

> @@ -453,8 +441,14 @@ 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,
>         },
> +#ifdef CONFIG_MMC_SDHCI_CADENCE_ELBA
> +       {
> +               .compatible = "pensando,elba-emmc",
> +               .data = &sdhci_elba_drv_data
> +       },
> +#endif
>         { .compatible = "cdns,sd4hc" },
>         { /* sentinel */ }
>  };

This introduces a reverse dependency between the modules, which will cause
problems at link time depending on how you configure it. There are two ways
to avoid this:

a) the simple method is to always link every backend into the driver module,
usually leaving them all enabled at compile time.

b) once this gets out of hand because there are too many variants, or the
differences between them are too big, you refactor the common code into
a library module that just exports a functions but has no module_init()
itself, plus a front-end driver for each variant, which now calls into
the common
code rather than being called by the common code.

      Arnd

[PATCH 6/8] 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 | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/arm64/Kconfig.platforms b/arch/arm64/Kconfig.platforms
index cdfd5fed457f..803e7cf1df55 100644
--- a/arch/arm64/Kconfig.platforms
+++ b/arch/arm64/Kconfig.platforms
@@ -210,6 +210,11 @@ 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 chipsets
+
 config ARCH_QCOM
 	bool "Qualcomm Platforms"
 	select GPIOLIB
-- 
2.17.1

Re: [PATCH 6/8] arm64: Add config for Pensando SoC platforms

[Arnd Bergmann]

On Thu, Mar 4, 2021 at 4:41 AM Brad Larson <brad@pensando.io> wrote:
>
> Add ARCH_PENSANDO configuration option for Pensando SoC
> based platforms.
>
> Signed-off-by: Brad Larson <brad@pensando.io>

The changelog and the platform help text could use a little more information
about what that platform is and where to find more information. This will
help users decide whether they should enable support for the platform or not.

       Arnd

[PATCH 7/8] arm64: dts: Add Pensando Elba SoC support

[Brad Larson]

Add Pensando common and Elba SoC specific device nodes
and corresponding binding documentation.

Signed-off-by: Brad Larson <brad@pensando.io>
---
 .../bindings/gpio/pensando,elba-spics.txt     |  24 ++
 .../devicetree/bindings/mmc/cdns,sdhci.yaml   |   2 +-
 .../bindings/spi/cadence-quadspi.txt          |   1 +
 .../devicetree/bindings/vendor-prefixes.yaml  |   2 +
 arch/arm64/boot/dts/Makefile                  |   1 +
 arch/arm64/boot/dts/pensando/Makefile         |   6 +
 arch/arm64/boot/dts/pensando/elba-16core.dtsi | 171 ++++++++++
 .../boot/dts/pensando/elba-asic-common.dtsi   | 113 +++++++
 arch/arm64/boot/dts/pensando/elba-asic.dts    |   8 +
 .../boot/dts/pensando/elba-flash-parts.dtsi   |  80 +++++
 arch/arm64/boot/dts/pensando/elba.dtsi        | 310 ++++++++++++++++++
 11 files changed, 717 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/gpio/pensando,elba-spics.txt
 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/Documentation/devicetree/bindings/gpio/pensando,elba-spics.txt b/Documentation/devicetree/bindings/gpio/pensando,elba-spics.txt
new file mode 100644
index 000000000000..30f5f3275238
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/pensando,elba-spics.txt
@@ -0,0 +1,24 @@
+Pensando Elba SPI Chip Select Driver
+
+The Pensando Elba ASIC provides four SPI bus chip selects
+
+Required properties:
+- compatible: Should be "pensando,elba-spics"
+- reg: Address range of spics controller
+- gpio-controller: Marks the device node as gpio controller
+- #gpio-cells: Must be 2
+
+Example:
+-------
+spics: spics@307c2468 {
+        compatible = "pensando,elba-spics";
+        reg = <0x0 0x307c2468 0x0 0x4>;
+        gpio-controller;
+        #gpio-cells = <2>;
+};
+
+&spi0 {
+        num-cs = <4>;
+        cs-gpios = <&spics 0 0>, <&spics 1 0>, <&porta 1 0>, <&porta 7 0>;
+	...
+}
diff --git a/Documentation/devicetree/bindings/mmc/cdns,sdhci.yaml b/Documentation/devicetree/bindings/mmc/cdns,sdhci.yaml
index af7442f73881..645ae696ba24 100644
--- a/Documentation/devicetree/bindings/mmc/cdns,sdhci.yaml
+++ b/Documentation/devicetree/bindings/mmc/cdns,sdhci.yaml
@@ -122,7 +122,7 @@ unevaluatedProperties: false
 examples:
   - |
     emmc: mmc@5a000000 {
-        compatible = "socionext,uniphier-sd4hc", "cdns,sd4hc";
+        compatible = "socionext,uniphier-sd4hc", "cdns,sd4hc", "pensando,elba-emmc";
         reg = <0x5a000000 0x400>;
         interrupts = <0 78 4>;
         clocks = <&clk 4>;
diff --git a/Documentation/devicetree/bindings/spi/cadence-quadspi.txt b/Documentation/devicetree/bindings/spi/cadence-quadspi.txt
index 8ace832a2d80..dbb346b2b1d7 100644
--- a/Documentation/devicetree/bindings/spi/cadence-quadspi.txt
+++ b/Documentation/devicetree/bindings/spi/cadence-quadspi.txt
@@ -6,6 +6,7 @@ Required properties:
 	For TI 66AK2G SoC - "ti,k2g-qspi", "cdns,qspi-nor".
 	For TI AM654 SoC  - "ti,am654-ospi", "cdns,qspi-nor".
 	For Intel LGM SoC - "intel,lgm-qspi", "cdns,qspi-nor".
+	For Pensando SoC - "pensando,cdns-qspi".
 - reg : Contains two entries, each of which is a tuple consisting of a
 	physical address and length. The first entry is the address and
 	length of the controller register set. The second entry is the
diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml
index f6064d84a424..9a21d780c5e1 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
+++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
@@ -850,6 +850,8 @@ patternProperties:
     description: Parallax Inc.
   "^pda,.*":
     description: Precision Design Associates, Inc.
+  "^pensando,.*":
+    description: Pensando Systems Inc.
   "^pericom,.*":
     description: Pericom Technology Inc.
   "^pervasive,.*":
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..b0386864cfec
--- /dev/null
+++ b/arch/arm64/boot/dts/pensando/elba-16core.dtsi
@@ -0,0 +1,171 @@
+// 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", "arm,armv8";
+			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..9623df208131
--- /dev/null
+++ b/arch/arm64/boot/dts/pensando/elba-asic-common.dtsi
@@ -0,0 +1,113 @@
+// SPDX-License-Identifier: GPL-2.0
+
+/ {
+	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 {
+		compatible = "jdec,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 {
+		compatible = "pensando,cpld";
+		#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..411c48457006
--- /dev/null
+++ b/arch/arm64/boot/dts/pensando/elba-asic.dts
@@ -0,0 +1,8 @@
+// SPDX-License-Identifier: GPL-2.0
+
+/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..1983de1a8403
--- /dev/null
+++ b/arch/arm64/boot/dts/pensando/elba-flash-parts.dtsi
@@ -0,0 +1,80 @@
+// SPDX-License-Identifier: GPL-2.0
+
+&flash0 {
+	partitions {
+		compatible = "fixed-partitions";
+		#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..72245e279483
--- /dev/null
+++ b/arch/arm64/boot/dts/pensando/elba.dtsi
@@ -0,0 +1,310 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2019-2021, 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";
+
+	interrupt-parent = <&gic>;
+	#address-cells = <2>;
+	#size-cells = <2>;
+
+	clocks {
+		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";
+	};
+
+
+	soc: soc {
+		compatible = "simple-bus";
+		#address-cells = <2>;
+		#size-cells = <2>;
+		ranges;
+
+		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>;
+
+			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 {
+			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";
+			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 {
+				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 */
+		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)>;
+		};
+
+		/* 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";
+		};
+
+		emmc: sdio-host-chip@30440000 {
+			compatible = "pensando,elba-emmc";
+			clocks = <&emmc_clk>;
+			interrupts = <GIC_SPI 26 IRQ_TYPE_LEVEL_HIGH>;
+			reg = <0x0 0x30440000 0x0 0x10000
+			       0x0 0x30480044 0x0 0x4>;
+			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

Re: [PATCH 7/8] arm64: dts: Add Pensando Elba SoC support

[Serge Semin]

On Wed, Mar 03, 2021 at 07:41:40PM -0800, Brad Larson wrote:
> Add Pensando common and Elba SoC specific device nodes
> and corresponding binding documentation.

This also needs to be split up into sub-patches seeing these are
unrelated changes like device bindings update, new platform DT file.

Note text-based bindings are deprecated in favor of the DT schemas.
Also note dts needs to pass dtbs_check validation. So all new HW/DT
nodes need to be reflected in the DT-schemas. See [1] for details.

[1] Documentation/devicetree/writing-schema.rst

> 
> Signed-off-by: Brad Larson <brad@pensando.io>
> ---
>  .../bindings/gpio/pensando,elba-spics.txt     |  24 ++
>  .../devicetree/bindings/mmc/cdns,sdhci.yaml   |   2 +-
>  .../bindings/spi/cadence-quadspi.txt          |   1 +
>  .../devicetree/bindings/vendor-prefixes.yaml  |   2 +
>  arch/arm64/boot/dts/Makefile                  |   1 +
>  arch/arm64/boot/dts/pensando/Makefile         |   6 +
>  arch/arm64/boot/dts/pensando/elba-16core.dtsi | 171 ++++++++++
>  .../boot/dts/pensando/elba-asic-common.dtsi   | 113 +++++++
>  arch/arm64/boot/dts/pensando/elba-asic.dts    |   8 +
>  .../boot/dts/pensando/elba-flash-parts.dtsi   |  80 +++++
>  arch/arm64/boot/dts/pensando/elba.dtsi        | 310 ++++++++++++++++++
>  11 files changed, 717 insertions(+), 1 deletion(-)
>  create mode 100644 Documentation/devicetree/bindings/gpio/pensando,elba-spics.txt
>  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/Documentation/devicetree/bindings/gpio/pensando,elba-spics.txt b/Documentation/devicetree/bindings/gpio/pensando,elba-spics.txt
> new file mode 100644
> index 000000000000..30f5f3275238
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/gpio/pensando,elba-spics.txt
> @@ -0,0 +1,24 @@
> +Pensando Elba SPI Chip Select Driver
> +
> +The Pensando Elba ASIC provides four SPI bus chip selects
> +
> +Required properties:
> +- compatible: Should be "pensando,elba-spics"
> +- reg: Address range of spics controller
> +- gpio-controller: Marks the device node as gpio controller
> +- #gpio-cells: Must be 2
> +
> +Example:
> +-------
> +spics: spics@307c2468 {
> +        compatible = "pensando,elba-spics";
> +        reg = <0x0 0x307c2468 0x0 0x4>;
> +        gpio-controller;
> +        #gpio-cells = <2>;
> +};
> +
> +&spi0 {
> +        num-cs = <4>;
> +        cs-gpios = <&spics 0 0>, <&spics 1 0>, <&porta 1 0>, <&porta 7 0>;
> +	...
> +}
> diff --git a/Documentation/devicetree/bindings/mmc/cdns,sdhci.yaml b/Documentation/devicetree/bindings/mmc/cdns,sdhci.yaml
> index af7442f73881..645ae696ba24 100644
> --- a/Documentation/devicetree/bindings/mmc/cdns,sdhci.yaml
> +++ b/Documentation/devicetree/bindings/mmc/cdns,sdhci.yaml
> @@ -122,7 +122,7 @@ unevaluatedProperties: false
>  examples:
>    - |
>      emmc: mmc@5a000000 {

> -        compatible = "socionext,uniphier-sd4hc", "cdns,sd4hc";
> +        compatible = "socionext,uniphier-sd4hc", "cdns,sd4hc", "pensando,elba-emmc";

Alas it's not enough. New HW compatible strings shall be defined in the
binding schema.

>          reg = <0x5a000000 0x400>;
>          interrupts = <0 78 4>;
>          clocks = <&clk 4>;
> diff --git a/Documentation/devicetree/bindings/spi/cadence-quadspi.txt b/Documentation/devicetree/bindings/spi/cadence-quadspi.txt
> index 8ace832a2d80..dbb346b2b1d7 100644
> --- a/Documentation/devicetree/bindings/spi/cadence-quadspi.txt
> +++ b/Documentation/devicetree/bindings/spi/cadence-quadspi.txt
> @@ -6,6 +6,7 @@ Required properties:
>  	For TI 66AK2G SoC - "ti,k2g-qspi", "cdns,qspi-nor".
>  	For TI AM654 SoC  - "ti,am654-ospi", "cdns,qspi-nor".
>  	For Intel LGM SoC - "intel,lgm-qspi", "cdns,qspi-nor".

> +	For Pensando SoC - "pensando,cdns-qspi".

What about converting this file to DT-schema and adding new HW
bindings in there?

>  - reg : Contains two entries, each of which is a tuple consisting of a
>  	physical address and length. The first entry is the address and
>  	length of the controller register set. The second entry is the
> diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml
> index f6064d84a424..9a21d780c5e1 100644
> --- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
> +++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
> @@ -850,6 +850,8 @@ patternProperties:
>      description: Parallax Inc.
>    "^pda,.*":
>      description: Precision Design Associates, Inc.
> +  "^pensando,.*":
> +    description: Pensando Systems Inc.
>    "^pericom,.*":
>      description: Pericom Technology Inc.
>    "^pervasive,.*":
> 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..b0386864cfec
> --- /dev/null
> +++ b/arch/arm64/boot/dts/pensando/elba-16core.dtsi
> @@ -0,0 +1,171 @@
> +// 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", "arm,armv8";
> +			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..9623df208131
> --- /dev/null
> +++ b/arch/arm64/boot/dts/pensando/elba-asic-common.dtsi
> @@ -0,0 +1,113 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +/ {
> +	model = "Elba ASIC Board";
> +
> +	aliases {
> +		serial0 = &uart0;
> +                spi0 = &spi0;
> +                spi1 = &qspi;
> +	};
> +
> +	chosen {

> +		stdout-path = "serial0:19200n8";

Baudrate of 19200? So sad.(

> +	};
> +};
> +
> +&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 {
> +		compatible = "jdec,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>;

Oh, you've got four peripheral SPI devices connected with only two native CS
available. Hmm, then I don't really know a better way, but just to forget about
the native DW APB CS functionality and activate the direct driving of
all the CS-pins at the moment of the DW APB SPI controller probe
procedure. Then indeed you'll need a custom CS function defined in the DW APB
SPI driver to handle that.

> +	status = "okay";
> +	spi@0 {
> +		compatible = "pensando,cpld";
> +		#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..411c48457006
> --- /dev/null
> +++ b/arch/arm64/boot/dts/pensando/elba-asic.dts
> @@ -0,0 +1,8 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +/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..1983de1a8403
> --- /dev/null
> +++ b/arch/arm64/boot/dts/pensando/elba-flash-parts.dtsi
> @@ -0,0 +1,80 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +&flash0 {
> +	partitions {
> +		compatible = "fixed-partitions";
> +		#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..72245e279483
> --- /dev/null
> +++ b/arch/arm64/boot/dts/pensando/elba.dtsi
> @@ -0,0 +1,310 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2019-2021, 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";
> +
> +	interrupt-parent = <&gic>;
> +	#address-cells = <2>;
> +	#size-cells = <2>;
> +
> +	clocks {
> +		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";
> +	};
> +
> +
> +	soc: soc {
> +		compatible = "simple-bus";
> +		#address-cells = <2>;
> +		#size-cells = <2>;
> +		ranges;
> +
> +		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>;
> +
> +			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 {
> +			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";
> +			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 {
> +				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 */
> +		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)>;
> +		};
> +
> +		/* 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>;
> +		};

So that GPIO-controller is just a single register which provides a way
to toggle the DW APB SPI CS-mode together with their output value.
If so and seeing there are a few more tiny spaces of config
registers added to eMMC, PCI, etc DT node, I suppose all of them
belong to some bigger config space of the SoC. Thus I'd suggest to at
least implement them as part of a System Controller DT node. Then use
that device service to switch on/off corresponding functionality.
See [2] and the rest of added to the kernel DTS files with
syscon-nodes for example.

[2] Documentation/devicetree/bindings/mfd/syscon.yaml

-Sergey

> +
> +		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";
> +		};
> +
> +		emmc: sdio-host-chip@30440000 {
> +			compatible = "pensando,elba-emmc";
> +			clocks = <&emmc_clk>;
> +			interrupts = <GIC_SPI 26 IRQ_TYPE_LEVEL_HIGH>;
> +			reg = <0x0 0x30440000 0x0 0x10000
> +			       0x0 0x30480044 0x0 0x4>;
> +			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
> 

Re: [PATCH 7/8] arm64: dts: Add Pensando Elba SoC support

[Brad Larson]

On Thu, Mar 4, 2021 at 12:03 AM Serge Semin <fancer.lancer@gmail.com> wrote:
>
> On Wed, Mar 03, 2021 at 07:41:40PM -0800, Brad Larson wrote:
> > Add Pensando common and Elba SoC specific device nodes
> > and corresponding binding documentation.
>
> This also needs to be split up into sub-patches seeing these are
> unrelated changes like device bindings update, new platform DT file.

In patchset v2 this is split into sub-patches.

> What about converting this file to DT-schema and adding new HW
> bindings in there?

Converted existing file devicetree/bindings/spi/cadence-quadspi.txt to
YAML schema.

> > +&spi0 {
> > +     num-cs = <4>;
>
> > +     cs-gpios = <&spics 0 0>, <&spics 1 0>, <&porta 1 0>, <&porta 7 0>;
>
> Oh, you've got four peripheral SPI devices connected with only two native CS
> available. Hmm, then I don't really know a better way, but just to forget about
> the native DW APB CS functionality and activate the direct driving of
> all the CS-pins at the moment of the DW APB SPI controller probe
> procedure. Then indeed you'll need a custom CS function defined in the DW APB
> SPI driver to handle that.

Yes, with an Elba SoC specific gpio driver.

> So that GPIO-controller is just a single register which provides a way
> to toggle the DW APB SPI CS-mode together with their output value.
> If so and seeing there are a few more tiny spaces of config
> registers added to eMMC, PCI, etc DT node, I suppose all of them
> belong to some bigger config space of the SoC. Thus I'd suggest to at
> least implement them as part of a System Controller DT node. Then use
> that device service to switch on/off corresponding functionality.
> See [2] and the rest of added to the kernel DTS files with
> syscon-nodes for example.
>
> [2] Documentation/devicetree/bindings/mfd/syscon.yaml

To us it was more understandable to implement a standard gpio driver
for the spi chip-selects.

Re: [PATCH 7/8] arm64: dts: Add Pensando Elba SoC support

[Brad Larson]

Hi Sergey,

On Thu, Mar 4, 2021 at 12:03 AM Serge Semin <fancer.lancer@gmail.com> wrote:
>
> On Wed, Mar 03, 2021 at 07:41:40PM -0800, Brad Larson wrote:
> > Add Pensando common and Elba SoC specific device nodes
> > and corresponding binding documentation.
>
> This also needs to be split up into sub-patches seeing these are
> unrelated changes like device bindings update, new platform DT file.
>
> Note text-based bindings are deprecated in favor of the DT schemas.
> Also note dts needs to pass dtbs_check validation. So all new HW/DT
> nodes need to be reflected in the DT-schemas. See [1] for details.
>
> [1] Documentation/devicetree/writing-schema.rst

Yes, patchset v2 was a first cut at organizing into sub-patches and in
v2 I used DT schemas for new files.  I will need to add additional new
sub-patches per review comments for v3 of the patchset.

> > diff --git a/Documentation/devicetree/bindings/mmc/cdns,sdhci.yaml b/Documentation/devicetree/bindings/mmc/cdns,sdhci.yaml
> > index af7442f73881..645ae696ba24 100644
> > --- a/Documentation/devicetree/bindings/mmc/cdns,sdhci.yaml
> > +++ b/Documentation/devicetree/bindings/mmc/cdns,sdhci.yaml
> > @@ -122,7 +122,7 @@ unevaluatedProperties: false
> >  examples:
> >    - |
> >      emmc: mmc@5a000000 {
>
> > -        compatible = "socionext,uniphier-sd4hc", "cdns,sd4hc";
> > +        compatible = "socionext,uniphier-sd4hc", "cdns,sd4hc", "pensando,elba-emmc";
>
> Alas it's not enough. New HW compatible strings shall be defined in the
> binding schema.

Based upon the next-20210818 version of cdns,sdhci.yaml below is the
proposed change.  In terms of defining new HW compatible strings is an
added example sufficient for pensando,elba-emmc?  There is no
additional definition for socionext,uniphier-sd4hc other than the
example in this file.

--- a/Documentation/devicetree/bindings/mmc/cdns,sdhci.yaml
+++ b/Documentation/devicetree/bindings/mmc/cdns,sdhci.yaml
@@ -15,9 +15,12 @@ allOf:
 properties:
   compatible:
-    items:
-      - enum:
-          - socionext,uniphier-sd4hc
+    oneOf:
+      - items:
+          - enum:
+              - socionext,uniphier-sd4hc
+              - pensando,elba-emmc
+          - const: cdns,sd4hc
       - const: cdns,sd4hc

   reg:
@@ -132,3 +135,17 @@ examples:
         mmc-hs400-1_8v;
         cdns,phy-dll-delay-sdclk = <0>;
     };
+  - |
+    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>;
+        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;
+    };

> > diff --git a/Documentation/devicetree/bindings/spi/cadence-quadspi.txt b/Documentation/devicetree/bindings/spi/cadence-quadspi.txt
> > index 8ace832a2d80..dbb346b2b1d7 100644
> > --- a/Documentation/devicetree/bindings/spi/cadence-quadspi.txt
> > +++ b/Documentation/devicetree/bindings/spi/cadence-quadspi.txt
> > @@ -6,6 +6,7 @@ Required properties:
> >       For TI 66AK2G SoC - "ti,k2g-qspi", "cdns,qspi-nor".
> >       For TI AM654 SoC  - "ti,am654-ospi", "cdns,qspi-nor".
> >       For Intel LGM SoC - "intel,lgm-qspi", "cdns,qspi-nor".
>
> > +     For Pensando SoC - "pensando,cdns-qspi".
>
> What about converting this file to DT-schema and adding new HW
> bindings in there?

The file cadence-quadspi.txt has been converted to cdns,qspi-nor.yaml
in next-20210818.  This would be the updated change where
pensando,cdns-qspi is now pensando,elba-qspi to be more specific.

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

> > +     chosen {
>
> > +             stdout-path = "serial0:19200n8";
>
> Baudrate of 19200? So sad.(

The default baudrate for patchset v3 will be 115200  :-)

> > +&spi0 {
> > +     num-cs = <4>;
>
> > +     cs-gpios = <&spics 0 0>, <&spics 1 0>, <&porta 1 0>, <&porta 7 0>;
>
> Oh, you've got four peripheral SPI devices connected with only two native CS
> available. Hmm, then I don't really know a better way, but just to forget about
> the native DW APB CS functionality and activate the direct driving of
> all the CS-pins at the moment of the DW APB SPI controller probe
> procedure. Then indeed you'll need a custom CS function defined in the DW APB
> SPI driver to handle that.

Right, confusion was created by leaving in code implying that the two
native CS are supported.  CS0 is used just to start the serial engine.
The existing dw_spi_set_cs() function works fine resulting in this
implementation.

static void dw_spi_elba_set_cs(struct spi_device *spi, bool enable)
{
        spi->chip_select = 0;
        dw_spi_set_cs(spi, enable);
}

> > +             spics: spics@307c2468 {
> > +                     compatible = "pensando,elba-spics";
> > +                     reg = <0x0 0x307c2468 0x0 0x4>;
> > +                     gpio-controller;
> > +                     #gpio-cells = <2>;
> > +             };
>
> So that GPIO-controller is just a single register which provides a way
> to toggle the DW APB SPI CS-mode together with their output value.
> If so and seeing there are a few more tiny spaces of config
> registers added to eMMC, PCI, etc DT node, I suppose all of them
> belong to some bigger config space of the SoC. Thus I'd suggest to at
> least implement them as part of a System Controller DT node. Then use
> that device service to switch on/off corresponding functionality.
> See [2] and the rest of added to the kernel DTS files with
> syscon-nodes for example.
>
> [2] Documentation/devicetree/bindings/mfd/syscon.yaml
>
> -Sergey
>

I've looked at the syscon documentation, other drivers that use it and
tried the below proposed example with variations.  The result is Elba
works ok for its four SPI devices but the host has a machine check
which must be due to a pcie access error.  From another thread on this
topic here is the recommended change to using syscon.

> Rob, please see here having a small sized reg-space one more time.
> Having so many small-sized registers scattered around the dts file
> makes me thinking that most of them likely belong to some bigger
> block like "System Controller". If so then there must be a main node
> compatible with "syscon" device, which phandle would be referenced in
> the particular device nodes. Like this:
>
> \ {
>         soc {
>                 syscon: syscon@307c0000 {
>                         compatible = "pensando,elba-sys-con", "syscon", "simple-mfd";
>                         reg = <0x0 0x307c0000 0x0 0x10000>;
>
>                         spics: spics@307c2468 {
>                                 compatible = "pensando,elba-spics";
>                                 gpio-controller;
>                                 #gpio-cells = <2>;
>                         };
>                 };
>                 pcie@307c2480 {
>                         compatible = "pensando,pcie";
>                         reg = <0x0 0x20000000 0x0 0x00380000>; /* PXB Base */
>
>                         syscon = <&syscon>;
>                 };
>
>                 /* etc */
>         };
> };

The current pcie node is

>         pcie@307c2480 {
>                 compatible = "pensando,pcie";
>                 reg = <0x0 0x307c2480 0x0 0x4           /* MS CFG_WDT */
>                        0x0 0x1400 0x0 0x10              /* WDT0 */
>                        0x0 0x20000000 0x0 0x380000>;    /* PXB Base */
>         };

Regards,
Brad

Re: [PATCH 7/8] arm64: dts: Add Pensando Elba SoC support

[Linus Walleij]

On Thu, Mar 4, 2021 at 4:42 AM Brad Larson <brad@pensando.io> wrote:

> Add Pensando common and Elba SoC specific device nodes
> and corresponding binding documentation.
>
> Signed-off-by: Brad Larson <brad@pensando.io>
(...)
>  .../bindings/gpio/pensando,elba-spics.txt     |  24 ++

Please use YAML schema for this.

See Documentation/devicetree/writing-schema.rst
for instructions, you need to install some python pip packages
to test your schema.

Yours,
Linus Walleij

Re: [PATCH 7/8] arm64: dts: Add Pensando Elba SoC support

[Brad Larson]

On Thu, Mar 4, 2021 at 12:52 AM Linus Walleij <linus.walleij@linaro.org> wrote:
>
> On Thu, Mar 4, 2021 at 4:42 AM Brad Larson <brad@pensando.io> wrote:
>
> > Add Pensando common and Elba SoC specific device nodes
> > and corresponding binding documentation.
> >
> > Signed-off-by: Brad Larson <brad@pensando.io>
> (...)
> >  .../bindings/gpio/pensando,elba-spics.txt     |  24 ++
>
> Please use YAML schema for this.

In patchset v2 changed to YAML schema and passed dt_binding_check and
dtbs_check.

Re: [PATCH 7/8] arm64: dts: Add Pensando Elba SoC support

[Arnd Bergmann]

On Thu, Mar 4, 2021 at 4:41 AM Brad Larson <brad@pensando.io> wrote:
>
> Add Pensando common and Elba SoC specific device nodes
> and corresponding binding documentation.
>
> Signed-off-by: Brad Larson <brad@pensando.io>
> ---
>  .../bindings/gpio/pensando,elba-spics.txt     |  24 ++
>  .../devicetree/bindings/mmc/cdns,sdhci.yaml   |   2 +-
>  .../bindings/spi/cadence-quadspi.txt          |   1 +

It would be better to split each of the above out into a separate patch for
easier review, and send them along with the driver changes.

> diff --git a/Documentation/devicetree/bindings/gpio/pensando,elba-spics.txt b/Documentation/devicetree/bindings/gpio/pensando,elba-spics.txt
> new file mode 100644
> index 000000000000..30f5f3275238
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/gpio/pensando,elba-spics.txt
> @@ -0,0 +1,24 @@
> +Pensando Elba SPI Chip Select Driver
> +
> +The Pensando Elba ASIC provides four SPI bus chip selects
> +
> +Required properties:
> +- compatible: Should be "pensando,elba-spics"
> +- reg: Address range of spics controller
> +- gpio-controller: Marks the device node as gpio controller
> +- #gpio-cells: Must be 2

You need to document what each of the cells are for. In your
example, the second cell is always zero, is that intentional?

> diff --git a/Documentation/devicetree/bindings/mmc/cdns,sdhci.yaml b/Documentation/devicetree/bindings/mmc/cdns,sdhci.yaml
> index af7442f73881..645ae696ba24 100644
> --- a/Documentation/devicetree/bindings/mmc/cdns,sdhci.yaml
> +++ b/Documentation/devicetree/bindings/mmc/cdns,sdhci.yaml
> @@ -122,7 +122,7 @@ unevaluatedProperties: false
>  examples:
>    - |
>      emmc: mmc@5a000000 {
> -        compatible = "socionext,uniphier-sd4hc", "cdns,sd4hc";
> +        compatible = "socionext,uniphier-sd4hc", "cdns,sd4hc", "pensando,elba-emmc";
>          reg = <0x5a000000 0x400>;
>          interrupts = <0 78 4>;
>          clocks = <&clk 4>;

These are in the wrong order, the most generic one (cdns,sd4hc) always
comes last.

If you add the string in the example, it also has to be an option in the
actual binding, otherwise neither the example nor your dtb would
be valid.

You also wouldn't find a controller that is compatible with both the uniphier
variant and the elba variant, unless your 'elba' SoC is strictly derived from
Socionext's Uniphier products and inherits all the quirks in its sdhci
implementation that were not already part of Cadence's IP block.

> diff --git a/Documentation/devicetree/bindings/spi/cadence-quadspi.txt b/Documentation/devicetree/bindings/spi/cadence-quadspi.txt
> index 8ace832a2d80..dbb346b2b1d7 100644
> --- a/Documentation/devicetree/bindings/spi/cadence-quadspi.txt
> +++ b/Documentation/devicetree/bindings/spi/cadence-quadspi.txt
> @@ -6,6 +6,7 @@ Required properties:
>         For TI 66AK2G SoC - "ti,k2g-qspi", "cdns,qspi-nor".
>         For TI AM654 SoC  - "ti,am654-ospi", "cdns,qspi-nor".
>         For Intel LGM SoC - "intel,lgm-qspi", "cdns,qspi-nor".
> +       For Pensando SoC - "pensando,cdns-qspi".

This does not look specific enough: There is no guarantee that this
is the only time Pensando uses any Cadenci qspi block. If the company
is not yet out of business, you should be prepared for future products
and have the name of the chip in there as well.

> +               cpu0: cpu@0 {
> +                       device_type = "cpu";
> +                       compatible = "arm,cortex-a72", "arm,armv8";
> +                       reg = <0 0x0>;
> +                       enable-method = "spin-table";
> +                       next-level-cache = <&l2_0>;

spin-table is not really something we want to see for new machines.
Please add proper psci support to your boot loader.

> index 000000000000..9623df208131
> --- /dev/null
> +++ b/arch/arm64/boot/dts/pensando/elba-asic-common.dtsi
> @@ -0,0 +1,113 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +/ {
> +       model = "Elba ASIC Board";
> +
> +       aliases {
> +               serial0 = &uart0;
> +                spi0 = &spi0;
> +                spi1 = &qspi;
> +       };
> +
> +       chosen {
> +               stdout-path = "serial0:19200n8";
> +       };
> +};

These properties are usually board specific, and should be moved into the
.dts file.

> +       spi@0 {
> +               compatible = "pensando,cpld";
> +               #address-cells = <1>;
> +               #size-cells = <1>;
> +               spi-max-frequency = <12000000>;
> +               reg = <0>;
> +       };


> +       spi@2 {
> +               compatible = "pensando,cpld-rd1173";

These don't seem to have a binding document, which needs to be added
first. What is a Pensando "cpld"? Is it possible that there will be multiple
versions of it that need to be uniquely identified?

> +
> +       /* Common UIO device for MSI drivers */
> +       uio_penmsi {
> +               compatible = "pensando,uio_penmsi";
> +               name = "uio_penmsi";
> +       };

Missing binding again. Since you name this a UIO device, I assume this
is actually tied to a particular Linux device driver and exported to user
space. The information in the dts should however not assume a particular
OS implementation but describe the platform.

Is this for PCI MSI? If so, I would recommend just using the GICv3 that you
also have, and leave this device completely unused.

In either case, please leave out the device node until a binding has
been agreed and a matching kernel driver was reviewed (if any)

> +
> +               /*
> +                * 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 {
> +                       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";
> +               };

Leave this one out as well, this has to be reviewed in combination with the
PCI driver.

> +               pcie@307c2480 {
> +                       compatible = "pensando,pcie";
> +                       reg = <0x0 0x307c2480 0x0 0x4   /* MS CFG_WDT */
> +                              0x0 0x00001400 0x0 0x10  /* WDT0 */
> +                              0x0 0x20000000 0x0 0x00380000>; /* PXB Base */
> +               };

This does not follow the PCI host bridge binding. Leave it out for now,
and bring it back once you have a proper PCI driver.

        Arnd

Re: [PATCH 7/8] arm64: dts: Add Pensando Elba SoC support

[Rob Herring]

On Wed, 03 Mar 2021 19:41:40 -0800, Brad Larson wrote:
> Add Pensando common and Elba SoC specific device nodes
> and corresponding binding documentation.
> 
> Signed-off-by: Brad Larson <brad@pensando.io>
> ---
>  .../bindings/gpio/pensando,elba-spics.txt     |  24 ++
>  .../devicetree/bindings/mmc/cdns,sdhci.yaml   |   2 +-
>  .../bindings/spi/cadence-quadspi.txt          |   1 +
>  .../devicetree/bindings/vendor-prefixes.yaml  |   2 +
>  arch/arm64/boot/dts/Makefile                  |   1 +
>  arch/arm64/boot/dts/pensando/Makefile         |   6 +
>  arch/arm64/boot/dts/pensando/elba-16core.dtsi | 171 ++++++++++
>  .../boot/dts/pensando/elba-asic-common.dtsi   | 113 +++++++
>  arch/arm64/boot/dts/pensando/elba-asic.dts    |   8 +
>  .../boot/dts/pensando/elba-flash-parts.dtsi   |  80 +++++
>  arch/arm64/boot/dts/pensando/elba.dtsi        | 310 ++++++++++++++++++
>  11 files changed, 717 insertions(+), 1 deletion(-)
>  create mode 100644 Documentation/devicetree/bindings/gpio/pensando,elba-spics.txt
>  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
> 

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/mmc/cdns,sdhci.example.dt.yaml: mmc@5a000000: compatible: ['socionext,uniphier-sd4hc', 'cdns,sd4hc', 'pensando,elba-emmc'] is too long
	From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/mmc/cdns,sdhci.yaml
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/mmc/cdns,sdhci.example.dt.yaml: mmc@5a000000: compatible: Additional items are not allowed ('pensando,elba-emmc' was unexpected)
	From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/mmc/cdns,sdhci.yaml

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

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 7/8] arm64: dts: Add Pensando Elba SoC support

[Krzysztof Kozlowski]

On 04/03/2021 04:41, Brad Larson wrote:
> Add Pensando common and Elba SoC specific device nodes
> and corresponding binding documentation.
> 
> Signed-off-by: Brad Larson <brad@pensando.io>
> ---
>  .../bindings/gpio/pensando,elba-spics.txt     |  24 ++
>  .../devicetree/bindings/mmc/cdns,sdhci.yaml   |   2 +-
>  .../bindings/spi/cadence-quadspi.txt          |   1 +
>  .../devicetree/bindings/vendor-prefixes.yaml  |   2 +

Hi,

dt-bindings go always to separate patches, at beginning of patchset.

>  arch/arm64/boot/dts/Makefile                  |   1 +
>  arch/arm64/boot/dts/pensando/Makefile         |   6 +
>  arch/arm64/boot/dts/pensando/elba-16core.dtsi | 171 ++++++++++
>  .../boot/dts/pensando/elba-asic-common.dtsi   | 113 +++++++
>  arch/arm64/boot/dts/pensando/elba-asic.dts    |   8 +
>  .../boot/dts/pensando/elba-flash-parts.dtsi   |  80 +++++
>  arch/arm64/boot/dts/pensando/elba.dtsi        | 310 ++++++++++++++++++

You need the board/vendor YAML file. See:
Documentation/devicetree/bindings/arm/

>  11 files changed, 717 insertions(+), 1 deletion(-)
>  create mode 100644 Documentation/devicetree/bindings/gpio/pensando,elba-spics.txt
>  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/Documentation/devicetree/bindings/gpio/pensando,elba-spics.txt b/Documentation/devicetree/bindings/gpio/pensando,elba-spics.txt
> new file mode 100644
> index 000000000000..30f5f3275238
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/gpio/pensando,elba-spics.txt
> @@ -0,0 +1,24 @@
> +Pensando Elba SPI Chip Select Driver
> +
> +The Pensando Elba ASIC provides four SPI bus chip selects
> +
> +Required properties:
> +- compatible: Should be "pensando,elba-spics"
> +- reg: Address range of spics controller
> +- gpio-controller: Marks the device node as gpio controller
> +- #gpio-cells: Must be 2
> +
> +Example:
> +-------
> +spics: spics@307c2468 {
> +        compatible = "pensando,elba-spics";
> +        reg = <0x0 0x307c2468 0x0 0x4>;
> +        gpio-controller;
> +        #gpio-cells = <2>;
> +};
> +
> +&spi0 {
> +        num-cs = <4>;
> +        cs-gpios = <&spics 0 0>, <&spics 1 0>, <&porta 1 0>, <&porta 7 0>;
> +	...
> +}
> diff --git a/Documentation/devicetree/bindings/mmc/cdns,sdhci.yaml b/Documentation/devicetree/bindings/mmc/cdns,sdhci.yaml
> index af7442f73881..645ae696ba24 100644
> --- a/Documentation/devicetree/bindings/mmc/cdns,sdhci.yaml
> +++ b/Documentation/devicetree/bindings/mmc/cdns,sdhci.yaml
> @@ -122,7 +122,7 @@ unevaluatedProperties: false
>  examples:
>    - |
>      emmc: mmc@5a000000 {
> -        compatible = "socionext,uniphier-sd4hc", "cdns,sd4hc";
> +        compatible = "socionext,uniphier-sd4hc", "cdns,sd4hc", "pensando,elba-emmc";

Why are you doing this?

>          reg = <0x5a000000 0x400>;
>          interrupts = <0 78 4>;
>          clocks = <&clk 4>;
> diff --git a/Documentation/devicetree/bindings/spi/cadence-quadspi.txt b/Documentation/devicetree/bindings/spi/cadence-quadspi.txt
> index 8ace832a2d80..dbb346b2b1d7 100644
> --- a/Documentation/devicetree/bindings/spi/cadence-quadspi.txt
> +++ b/Documentation/devicetree/bindings/spi/cadence-quadspi.txt
> @@ -6,6 +6,7 @@ Required properties:
>  	For TI 66AK2G SoC - "ti,k2g-qspi", "cdns,qspi-nor".
>  	For TI AM654 SoC  - "ti,am654-ospi", "cdns,qspi-nor".
>  	For Intel LGM SoC - "intel,lgm-qspi", "cdns,qspi-nor".
> +	For Pensando SoC - "pensando,cdns-qspi".
>  - reg : Contains two entries, each of which is a tuple consisting of a
>  	physical address and length. The first entry is the address and
>  	length of the controller register set. The second entry is the
> diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml
> index f6064d84a424..9a21d780c5e1 100644
> --- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
> +++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
> @@ -850,6 +850,8 @@ patternProperties:
>      description: Parallax Inc.
>    "^pda,.*":
>      description: Precision Design Associates, Inc.
> +  "^pensando,.*":
> +    description: Pensando Systems Inc.
>    "^pericom,.*":
>      description: Pericom Technology Inc.
>    "^pervasive,.*":
> 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..b0386864cfec
> --- /dev/null
> +++ b/arch/arm64/boot/dts/pensando/elba-16core.dtsi
> @@ -0,0 +1,171 @@
> +// 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", "arm,armv8";
> +			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";
> +		};

Run the dtbs_check and build dtbs with W=1 - no warnings are usually
expected.

> +	};
> +};
> 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..9623df208131
> --- /dev/null
> +++ b/arch/arm64/boot/dts/pensando/elba-asic-common.dtsi
> @@ -0,0 +1,113 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +/ {
> +	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 {
> +		compatible = "jdec,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 {

Generic node name needed, like in devicetree spec.

> +		compatible = "ti,tmp451";
> +		reg = <0x4c>;
> +	};

Here and everywhere else - line break between each node.

> +	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 {
> +		compatible = "pensando,cpld";
> +		#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..411c48457006
> --- /dev/null
> +++ b/arch/arm64/boot/dts/pensando/elba-asic.dts
> @@ -0,0 +1,8 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +/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..1983de1a8403
> --- /dev/null
> +++ b/arch/arm64/boot/dts/pensando/elba-flash-parts.dtsi
> @@ -0,0 +1,80 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +&flash0 {
> +	partitions {
> +		compatible = "fixed-partitions";
> +		#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>;

No leading 0 in address and size.

> +		};
> +		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..72245e279483
> --- /dev/null
> +++ b/arch/arm64/boot/dts/pensando/elba.dtsi
> @@ -0,0 +1,310 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2019-2021, 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.

You have SPDX header. No boiler plate.

OK, I stopped...

Best regards,
Krzysztof

[PATCH 8/8] MAINTAINERS: Add entry for PENSANDO

[Brad Larson]

Add entry for PENSANDO maintainer and files.

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

diff --git a/MAINTAINERS b/MAINTAINERS
index 973a937386fa..3f2eebda2396 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2246,6 +2246,15 @@ S:	Maintained
 W:	http://hackndev.com
 F:	arch/arm/mach-pxa/palmz72.*
 
+ARM/PENSANDO SUPPORT
+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/
+F:	drivers/gpio/gpio-elba-spics.c
+F:	drivers/mmc/host/sdhci-cadence-elba.c
+
 ARM/PLEB SUPPORT
 M:	Peter Chubb <pleb@gelato.unsw.edu.au>
 S:	Maintained
-- 
2.17.1