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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20191219170409.GH35479@atomide.com>
Date:   Thu, 19 Dec 2019 09:04:09 -0800
From:   Tony Lindgren <tony@...mide.com>
To:     Linus Walleij <linus.walleij@...aro.org>
Cc:     lee.jones@...aro.org, linux-kernel@...r.kernel.org,
        Mark Brown <broonie@...nel.org>,
        Sebastian Reichel <sre@...nel.org>
Subject: Re: [PATCH] mfd: motorola-cpcap: Do not hardcode SPI mode flags

Hi Linus,

* Linus Walleij <linus.walleij@...aro.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@...nel.org>
> Signed-off-by: Linus Walleij <linus.walleij@...aro.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
>