Re: [PATCH] mfd: motorola-cpcap: Do not hardcode SPI mode flags

Tony Lindgren
2019-12-19T17:04:15Z

Hi Linus,

* Linus Walleij <linus.walleij@linaro.org> [700101 00:00]:
> The current use of mode flags to us SPI_MODE_0 and
> SPI_CS_HIGH is fragile: it overwrites anything already
> assigned by the SPI core. Change it thusly:
>
> - Just |= the SPI_MODE_0 so we keep other flags
> - Assign ^= SPI_CS_HIGH since we might be active high
> already, and that is usually the case with GPIOs used
> for chip select, even if they are in practice active low.
>
> Add a comment clarifying why ^= SPI_CS_HIGH is the right
> choice here.

Looks like this breaks booting for droid4 with a cpcap
PMIC, probably as regulators won't work. There's no GPIO
controller involved in this case for the chip select, the
pins are directly controlled by the spi-omap2-mcspi.c
driver.

From the pin muxing setup we see there's a pull-down on
mcspi1_cs0 pin meaning it's active high:

/* 0x4a100138 mcspi1_cs0.mcspi1_cs0 ae23 */
OMAP4_IOPAD(0x138, PIN_INPUT_PULLDOWN | MUX_MODE0)

My guess a similar issue is with similar patches for
all non-gpio spi controllers?

Let me know if you want me to test some other changes,
or if this patch depends on some other changes.

Regards,

Tony

> Reported-by: Mark Brown <broonie@kernel.org>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> drivers/mfd/motorola-cpcap.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mfd/motorola-cpcap.c b/drivers/mfd/motorola-cpcap.c
> index 52f38e57cdc1..a3bc61b8008c 100644
> --- a/drivers/mfd/motorola-cpcap.c
> +++ b/drivers/mfd/motorola-cpcap.c
> @@ -279,7 +279,13 @@ static int cpcap_probe(struct spi_device *spi)
> spi_set_drvdata(spi, cpcap);
>
> spi->bits_per_word = 16;
> - spi->mode = SPI_MODE_0 | SPI_CS_HIGH;
> + spi->mode |= SPI_MODE_0;
> + /*
> + * Active high should be defined as "inverse polarity" as GPIO-based
> + * chip selects can be logically active high but inverted by the GPIO
> + * library.
> + */
> + spi->mode ^= SPI_CS_HIGH;
>
> ret = spi_setup(spi);
> if (ret)
> --
> 2.23.0
>