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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <20201022065402.x7hlp2zncmnjyum7@pengutronix.de>
Date:   Thu, 22 Oct 2020 08:54:02 +0200
From:   Oleksij Rempel <o.rempel@...gutronix.de>
To:     Dmitry Torokhov <dmitry.torokhov@...il.com>
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 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?

> (well, mode 0 is kind of working, but just on accident).

Good question, will be probably a good reason to measure it.

> Thanks.
> 
> -- 
> Dmitry
> 
> 

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 |