On Sun, Jan 26, 2014 at 10:14:34AM +0200, Baruch Siach wrote:

>  
> -     if (!last_transfer->cs_change && dws->cs_control)
> -             dws->cs_control(MRST_SPI_DEASSERT);
> +     if (!last_transfer->cs_change) {
> +             if (dws->cs_control)
> +                     dws->cs_control(MRST_SPI_DEASSERT);
> +             if (gpio_is_valid(cs_gpio))
> +                     gpio_set_value(cs_gpio, !(mode & SPI_CS_HIGH));
> +     }

Why is this not using spi_chip_sel()?  I know the old code wasn't either
but since both sites need to be changed...

> +             spi_chip_sel(dws, spi->chip_select, spi->cs_gpio,
> +                             spi->mode & SPI_CS_HIGH);

I would expect the /CS polarity handling to be abstracted inside the
function?

> +     if (gpio_is_valid(spi->cs_gpio)) {
> +             ret = devm_gpio_request(&spi->dev, spi->cs_gpio,
> +                             dev_name(&spi->dev));
> +             if (ret)
> +                     return ret;
> +             ret = gpio_direction_output(spi->cs_gpio,
> +                             !(spi->mode & SPI_CS_HIGH));
> +             if (ret)
> +                     return ret;
> +     }

Should be devm_gpio_request_one().  This won't work with probe deferral,
it's outside the probe() function so nothing will retry.

Attachment: signature.asc
Description: Digital signature

Reply via email to