On Thu, Mar 4, 2021 at 12:29 AM Linus Walleij [off-list ref] wrote:

Hi Brad,

thanks for your patch!

On Thu, Mar 4, 2021 at 4:42 AM Brad Larson [off-list ref] wrote:

quoted

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

(...)

quoted

+#include <linux/gpio.h>

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

quoted

+ * 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!)

quoted

+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).

quoted

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