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

[Tony Lindgren <tony@atomide.com>]

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
>