On Wed, Nov 11, 2020 at 02:05:19AM +0100, Linus Walleij wrote: > On Mon, Nov 9, 2020 at 3:41 PM Sven Van Asbroeck wrote: > > I don't disagree. Fact is that after the imx cspi bus driver was converted > > to gpio descriptors, most spi client drivers broke. It would be great if this > > could be fixed. Any method that the community can find a consensus on, > > would be great :) > I think your patch is the quick fix. > I would say that anything that has: > spi->mode = ... > is essentially broken. This is not clear to me, most of these settings are things that are constant for the device so it's not clear that they should be being set by the device tree in the first place. The idea that the chip select might be being inverted like it is by this whole gpiolib/DT/new binding thing is breaking expectations too. > The core sets up vital things in .mode from e.g. device tree in > of_spi_parse_dt(): > /* Mode (clock phase/polarity/etc.) */ > if (of_property_read_bool(nc, "spi-cpha")) > spi->mode |= SPI_CPHA; > if (of_property_read_bool(nc, "spi-cpol")) > spi->mode |= SPI_CPOL; > if (of_property_read_bool(nc, "spi-3wire")) > spi->mode |= SPI_3WIRE; > if (of_property_read_bool(nc, "spi-lsb-first")) > spi->mode |= SPI_LSB_FIRST; > if (of_property_read_bool(nc, "spi-cs-high")) > spi->mode |= SPI_CS_HIGH; > All this gets overwritten and ignored when a client just assigns mode > like that. Not just SPI_CS_HIGH. I doubt things are different > with ACPI. OTOH most of these are things the device driver should just get right without needing any input from DT, there's a few where there's plausible options (eg, you can imagine pin strap configuration for 3 wire mode) so generally it's not clear how many of these make sense for anything other than spidev. This binding all predates my involvement so I don't know the thought process here.