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

[Brad Larson <brad@xxxxxxxxxxx>]

Hi Linus,

On Thu, Mar 4, 2021 at 12:29 AM Linus Walleij <linus.walleij@xxxxxxxxxx> wrote:
> On Thu, Mar 4, 2021 at 4:42 AM Brad Larson <brad@xxxxxxxxxxx> 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