On 6/29/21 10:07 AM, Mark Brown wrote: > On Tue, Jun 29, 2021 at 05:01:57PM +0000, Dan.Sneddon@xxxxxxxxxxxxx wrote: >> On 6/29/21 9:47 AM, Mark Brown wrote: > >> >In what way does it do that? I can't tell what the patch is supposed >to >> >do. > >> The SPI_MASTER_GPIO_SS flag has to be set so that the set_cs function >> gets called even when using gpio cs pins. > > This all needs to be clear in the changelog. I'll update the commit message. > >> >> - enable =3D (!!(spi->mode & SPI_CS_HIGH) =3D=3D enable); >> >> =20 >> >> - if (enable) { >> >> + if ((enable && (spi->mode & SPI_CS_HIGH)) >> >> + || (!enable && !(spi->mode & SPI_CS_HIGH))) { > >> >This looks especially suspicious. > >> It's due to the fact that the spi core tells set_cs if the cs should be >> high or low, not active or disabled. This logic is to convert from >> high/low to active/disabled. > > spi_set_cs() handles SPI_CS_HIGH... this looks like a separate existing > driver bug, it should just be ignoring SPI_CS_HIGH if it's providing a > set_cs() operation and letting the core implement SPI_CS_HIGH for it. I > only checked breifly but it looks like spi-atmel is trying to use the > core support for chipselect handling here. >