mbox series

[0/7,v2] SPI CS using GPIO descriptors

Message ID 20190107155156.3738-1-linus.walleij@linaro.org
Headers show
Series SPI CS using GPIO descriptors | expand

Message

Linus Walleij Jan. 7, 2019, 3:51 p.m. UTC 
This is a rebased (v5.0-rc1) version of trying to pull
GPIO descriptor handling of SPI chip selects into the
SPI core.

The core grows a bit, bit handles descriptors in addition
to the already handled DT and static GPIO lines, and then
I start converting over some drivers to show the utility
of pulling this into the core.

There are *many* drivers to convert.

This should also cover the ACPI usecase by way of the
completely generic GPIO descriptor handling code that
simply picks the GPIO "cs" descriptors associated with
the device.

Linus Walleij (7):
  spi: Optionally use GPIO descriptors for CS GPIOs
  spi: ath79: Convert to use CS GPIO descriptors
  spi: atmel: Convert to use CS GPIO descriptors
  spi: cadence: Convert to use CS GPIO descriptors
  spi: clps711x: Convert to use CS GPIO descriptors
  spi: davinci: Convert to use CS GPIO descriptors
  spi: dw: Convert to use CS GPIO descriptors

 drivers/spi/spi-ath79.c    |  42 +++++----------
 drivers/spi/spi-atmel.c    |  93 ++++++++++-----------------------
 drivers/spi/spi-cadence.c  |  67 +-----------------------
 drivers/spi/spi-clps711x.c |  23 +-------
 drivers/spi/spi-davinci.c  |  53 +++++--------------
 drivers/spi/spi-dw-mmio.c  |  22 --------
 drivers/spi/spi-dw.c       |   9 +---
 drivers/spi/spi.c          | 104 +++++++++++++++++++++++++++++++++----
 include/linux/spi/spi.h    |  23 ++++++--
 9 files changed, 172 insertions(+), 264 deletions(-)

-- 
2.19.2

Comments

Jan Kotas Jan. 10, 2019, 8:56 a.m. UTC | #1 
> On 7 Jan 2019, at 16:51, Linus Walleij <linus.walleij@linaro.org> wrote:

> 

> +	master->use_gpio_descriptors = true;

> 	master->prepare_transfer_hardware = cdns_prepare_transfer_hardware;

> 	master->prepare_message = cdns_prepare_message;

> 	master->transfer_one = cdns_transfer_one;

> 	master->unprepare_transfer_hardware = cdns_unprepare_transfer_hardware;

> 	master->set_cs = cdns_spi_chipselect;

> -	master->setup = cdns_spi_setup;

> -	master->cleanup = cdns_spi_cleanup;

> 	master->auto_runtime_pm = true;

> 	master->mode_bits = SPI_CPOL | SPI_CPHA;

> 


Hi,

It seems this patch breaks the Cadence SPI driver:

spi spi0.0: setup: unsupported mode bits 4
cdns-spi fd0b0000.spi: can't setup spi0.0, status -22


It looks like the reason is that, the driver only sets:
master->mode_bits = SPI_CPOL | SPI_CPHA;

The of_spi_parse_dt function adds an SPI_CS_HIGH:
if (ctlr->use_gpio_descriptors)
	spi->mode |= SPI_CS_HIGH;

However the spi_setup function checks if a driver supports 
selected SPI modes:
bad_bits = spi->mode & ~(spi->controller->mode_bits | SPI_CS_WORD);

Because the of_spi_parse_dt added an SPI_CS_HIGH, 
which is not set in the controller the setup fails:

if (bad_bits) {
	dev_err(&spi->dev, "setup: unsupported mode bits %x\n",
		bad_bits);
	return -EINVAL;
}

Sorry for not picking it up earlier, I was busy with some other stuff.
I noticed it, when our Jenkins job for SPI Kernel CI failed.

I think it’s not an uncommon case, maybe if use_gpio_descriptors
is set, we should ignore an SPI_CS_HIGH in the setup?

Regards,
Jan