On Mon, Oct 26, 2020 at 08:53:09PM -0700, Dmitry Torokhov wrote:

On Thu, Oct 22, 2020 at 08:54:02AM +0200, Oleksij Rempel wrote:

quoted

On Wed, Oct 21, 2020 at 11:27:57AM -0700, Dmitry Torokhov wrote:

quoted

On Wed, Oct 21, 2020 at 12:56:14PM +0200, Oleksij Rempel wrote:

quoted

As you can see, I would need to configure my dts with spi-cs-high flag,
even if the hardware is actually ACTIVE_LOW. If I will go this way, I
would risk a regression as soon as this issue is fixed.

Since the spi framework is already parsing devicetree and set all needed
flags, I assume it is wrong to blindly drop all this flags in the
driver.

Yes, but I wonder if the devices can only work in mode 0 we should be
doing:

	spi->mode &= ~SPI_MODE_MASK; // to be defined as 0x03 in spi.h
	spi->mode |= SPI_MODE_0;

as we can't simply "or" mode value as is

Why not? This values are taken from device tree. If some developer
decided to add them, then driver should take it over. Even if this
values will break the functionality.

Other properties of this driver will break the functionality too of this
driver too, so why should we silently filter only set of this bits?

What I was trying to say is that if driver wants to set mode to
particular value it should not "or" the value, as it will not reset the
relevant bits. I.e. if there some undesirable data in spi->mode mode
bits it will not get set properly by essentially doing "spi->mode |= 0".
That is why I said the driver needs to clear mode bits and set them to
the desired mode.

Ok, i'll update this patch as you suggested.

Regards,
Oleksij
-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |