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  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]
Date:   Sun, 24 Mar 2019 07:56:17 +0100
From:   "H. Nikolaus Schaller" <>
To:     Linus Walleij <>
Cc:     Jan Kotas <>, LKML <>,
        Discussions about the Letux Kernel 
        "open list:GPIO SUBSYSTEM" <>,
        linux-spi <>,
        devicetree <>,
        Rob Herring <>,
        Mark Brown <>
Subject: Re: [BUG] gpiolib: spi chip select legacy support breaks modern chip select and whitens the GTA04 LCD panel

Hi Linus,

> Am 24.03.2019 um 05:15 schrieb Linus Walleij <>:
> On Sat, Mar 23, 2019 at 3:40 PM H. Nikolaus Schaller <> wrote:
>> (1) c1c04cea13dc gpio: of: Fix logic inversion
>> together with the basic patch
>> (2) 6953c57ab172 gpio: of: Handle SPI chipselect legacy bindings
>> leads to a severe regression for our GTA04 board.
> Sorry! :(
> I found the same problem on my Nomadik board.
> But I fixed it in that case by introducing a spi-cs-high into the DTS file:

Yes, that of course works and is our temporary solution.

And I see that you also have it on the controller node and not the slave node.

>> I learned that it tries to handle a legacy "spi-cs-high" property of SPI slaves, but was stopped
>> from doing so by a bug (1). So only with both patches, the legacy handler becomes active which
>> explains why it was not noticed earlier.
>> Now, our GTA04 device tree from 2014 (v3.16-rc1) was already written without any legacy spi properties
>> in mind
> I'm sorry about that, however if you look at the DT binding document:
> Documentation/devicetree/bindings/spi/spi-bus.txt

Shouldn't it be a property of the slave node and not the controller node?

> You will see that spi-cs-high is mandatory. So these DTS files are
> incorrect.

How do you read that it is mandatory from

"All slave nodes can contain the following optional properties:
- spi-cs-high     - Empty property indicating device requires chip select
		    active high."

I read it as optional and indicative. Equal priority to cs-gpios.

>> Therefore I would suggest:
>> * revert both patches as soon as possible (v5.1-rc series) to remove the unexpected spi legacy
>>  code handler from the gpio subsystem.
>> * replace all uses of spi-cs-high by correct cs-gpios flags - unless they already are there.
>>  fgrep spi-cs-high arch/*/boot/dts/*.dts* shows only a handful of DTS candidates.
>> * fix spi-bus.txt documentation to describe this potential pitfall.
> This does not work because there are devices that requires spi-cs-high to be
> respected and the DTS second cell GPIO flag to be ignored.

Then, those should be fixed...

> Jan Kotas reported this problem.

Thanks for adding him to the discussion.

> They might have deployed DTB binaries that need to keep working,

Well, that is a weak argument. What if the GTA04 would have the DTB in FLASH
and would need it working (fortunately we always reflash kernel + dtbs)?

> so we
> cannot change it to ignore spi-cs-high like this. (I might give in if it can be
> proven that all of them just recompile the DTS all the time and no
> DTBs are in flash.)

BTW, on which node do these invariable DTBs have the spi-cs-high flag?
In the controller node (IMHO wrong) or the slave node (according to bindings doc)?

I have checked with randomly picked imx51-babbage.dts and it has it in the
slave node (pmic: mc13892@0). It also seems to be an example where different
CS lines want different settings.

If the all these DTBs have spi-cs-high in the slave node, none of them will
be fixed by the current code.

> I think in this case the oldest binding wins.

Ok, it is the question when such deprecated things are really removed.
There is no clear answer...

> The spi-cs-high was there before
> we came up with the scheme to use the flags cell with GPIO phandles.

> I think you simply have to patch these GTA04 DTS files to use
> spi-cs-high.

Ugly... Well, if DTS maintainers accept that?

> But I'm open to other ideas, let's discuss this.

What about a CONFIG to explicitly enable/disable this legacy support?

IMHO it will need to be enabled for les than 1% of the kernel builds and
therefore also keeps the zImage smaller for all others. And avoids DTB
changes where the gpio flags are already correct.

BR and thanks,

Powered by blists - more mailing lists