linux-kernel - Re: [PATCH v1] Input: ads7846: do not overwrite spi->mode flags set by spi framework

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 26 Oct 2020 20:53:09 -0700
From:   Dmitry Torokhov <dmitry.torokhov@...il.com>
To:     Oleksij Rempel <o.rempel@...gutronix.de>
Cc:     "Ardelean, Alexandru" <alexandru.Ardelean@...log.com>,
        David Jander <david@...tonic.nl>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "kernel@...gutronix.de" <kernel@...gutronix.de>,
        "linux-input@...r.kernel.org" <linux-input@...r.kernel.org>
Subject: Re: [PATCH v1] Input: ads7846: do not overwrite spi->mode flags set
 by spi framework

On Thu, Oct 22, 2020 at 08:54:02AM +0200, Oleksij Rempel wrote:
> On Wed, Oct 21, 2020 at 11:27:57AM -0700, Dmitry Torokhov wrote:
> > On Wed, Oct 21, 2020 at 12:56:14PM +0200, Oleksij Rempel wrote:
> > > 
> > > 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.

Thanks.

-- 
Dmitry