Re: [PATCH 1/8] gpio: Add Elba SoC gpio driver for spi cs control
On Thu, Mar 4, 2021 at 12:29 AM Linus Walleij wrote:
>
> Hi Brad,
>
> thanks for your patch!
>
> On Thu, Mar 4, 2021 at 4:42 AM 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
> (...)
>
> > +#include
>
> Use this in new drivers:
> #include
>
> > + * pin: 32| 10
> > + * 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
On Mon, Mar 29, 2021 at 4:19 AM Brad Larson wrote:
> On Sun, Mar 7, 2021 at 11:21 AM Andy Shevchenko
> wrote:
> > On Thu, Mar 4, 2021 at 4:40 PM Brad Larson 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
> >
> > It's not used here, but you missed mod_devicetable.h.
>
> Removed . 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(>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(>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
On Sun, Mar 7, 2021 at 11:21 AM Andy Shevchenko
wrote:
>
> On Thu, Mar 4, 2021 at 4:40 PM Brad Larson 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
>
> It's not used here, but you missed mod_devicetable.h.
Removed . There is no dependency on mod_devicetable.h.
> ...
>
> > +/*
> > + * pin: 32| 10
> > + * 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(>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(>dev, "failed to remap I/O memory\n");
>
> Duplicate noisy message.
>
> > + return PTR_ERR(p->base);
> > + }
>
> > + ret = devm_gpiochip_add_data(>dev, >chip, p);
> > + if (ret) {
> > + dev_err(>dev, "unable to add gpio chip\n");
>
> > + return ret;
> > + }
> > +
> > + dev_info(>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
On Thu, Mar 4, 2021 at 4:40 PM Brad Larson 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
It's not used here, but you missed mod_devicetable.h.
...
> +/*
> + * pin: 32| 10
> + * 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(>dev, res);
p->base = devm_platform_ioremap_resource(pdev, 0);
> + if (IS_ERR(p->base)) {
> + dev_err(>dev, "failed to remap I/O memory\n");
Duplicate noisy message.
> + return PTR_ERR(p->base);
> + }
> + ret = devm_gpiochip_add_data(>dev, >chip, p);
> + if (ret) {
> + dev_err(>dev, "unable to add gpio chip\n");
> + return ret;
> + }
> +
> + dev_info(>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
Hi Brad,
On Thu, Mar 4, 2021 at 4:59 AM 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
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 -- ge...@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
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
> ---
> 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(>dev, sizeof(*p), GFP_KERNEL);
> + if (!p)
> + return -ENOMEM;
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + p->base = devm_ioremap_resource(>dev, res);
> + if (IS_ERR(p->base)) {
> + dev_err(>dev, "failed to remap I/O memory\n");
> + return PTR_ERR(p->base);
> + }
> + spin_lock_init(>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(>dev);
> + p->chip.parent = >dev;
> + p->chip.owner = THIS_MODULE;
> +
> + ret = devm_gpiochip_add_data(>dev, >chip, p);
> + if (ret) {
> + dev_err(>dev, "unable to add gpio chip\n");
> + return ret;
> + }
> +
> + dev_info(>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
> -Original Message-
> From: Brad Larson
> 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
On Thu, Mar 4, 2021 at 10:10 AM Serge Semin wrote: > On Thu, Mar 04, 2021 at 09:29:33AM +0100, Linus Walleij wrote: > > > + * pin: 32| 10 > > > + * 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
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 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
> (...)
>
> > +#include
>
> Use this in new drivers:
> #include
>
> > + * pin: 32| 10
> > + * 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
Hi Brad,
thanks for your patch!
On Thu, Mar 4, 2021 at 4:42 AM 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
(...)
> +#include
Use this in new drivers:
#include
> + * pin: 32| 10
> + * 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
[PATCH 1/8] gpio: Add Elba SoC gpio driver for spi cs control
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
---
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 ..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
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+/*
+ * pin: 32| 10
+ * 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(>lock, flags);
+ tmp = readl_relaxed(p->base);
+ tmp = (tmp & ~SPICS_MASK(pin)) | SPICS_SET(pin, value);
+ writel_relaxed(tmp, p->base);
+ spin_unlock_irqrestore(>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(>dev, sizeof(*p), GFP_KERNEL);
+ if (!p)
+ return -ENOMEM;
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ p->base = devm_ioremap_resource(>dev, res);
+ if (IS_ERR(p->base)) {
+ dev_err(>dev, "failed to remap I/O memory\n");
+ return PTR_ERR(p->base);
+ }
+ spin_lock_init(>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(>dev);
+ p->chip.parent = >dev;
+ p->chip.owner = THIS_MODULE;
+
+ ret = devm_gpiochip_add_data(>dev, >chip, p);
+ if (ret) {
+ dev_err(>dev, "unable to add gpio chip\n");
+ return ret;
+ }
+
+ dev_info(>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,
+
