Re: [PATCH 4/7 v2] spi: cadence: Convert to use CS GPIO descriptors

[Janek Kotas <jank@xxxxxxxxxxx>]

> On 7 Jan 2019, at 16:51, Linus Walleij <linus.walleij@xxxxxxxxxx> 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