linux-kernel - Re: [PATCH 2/2] spi: Add Baikal-T1 System Boot SPI Controller driver

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: <740f1569-d8d3-606a-73a8-f9feeb20e06f@alliedtelesis.co.nz>
Date:   Sun, 10 May 2020 22:17:16 +0000
From:   Chris Packham <Chris.Packham@...iedtelesis.co.nz>
To:     Serge Semin <Sergey.Semin@...kalelectronics.ru>,
        Mark Brown <broonie@...nel.org>
CC:     Serge Semin <fancer.lancer@...il.com>,
        Ramil Zaripov <Ramil.Zaripov@...kalelectronics.ru>,
        Alexey Malahov <Alexey.Malahov@...kalelectronics.ru>,
        Thomas Bogendoerfer <tsbogend@...ha.franken.de>,
        Paul Burton <paulburton@...nel.org>,
        "Ralf Baechle" <ralf@...ux-mips.org>,
        Lee Jones <lee.jones@...aro.org>,
        "Miquel Raynal" <miquel.raynal@...tlin.com>,
        Arnd Bergmann <arnd@...db.de>,
        "Rob Herring" <robh+dt@...nel.org>,
        "linux-mips@...r.kernel.org" <linux-mips@...r.kernel.org>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        John Garry <john.garry@...wei.com>,
        "Chuanhong Guo" <gch981213@...il.com>,
        Andy Shevchenko <andy.shevchenko@...il.com>,
        "Eddie James" <eajames@...ux.ibm.com>,
        Geert Uytterhoeven <geert@...ux-m68k.org>,
        Tomer Maimon <tmaimon77@...il.com>,
        Masahisa Kojima <masahisa.kojima@...aro.org>,
        Krzysztof Kozlowski <krzk@...nel.org>,
        "Florian Fainelli" <f.fainelli@...il.com>,
        Jassi Brar <jaswinder.singh@...aro.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-spi@...r.kernel.org" <linux-spi@...r.kernel.org>
Subject: Re: [PATCH 2/2] spi: Add Baikal-T1 System Boot SPI Controller driver


On 10/05/20 12:20 pm, Serge Semin wrote:
> On Fri, May 08, 2020 at 12:37:51PM +0100, Mark Brown wrote:
<snip>
>>> +	writel(BIT(req->cs), bs->regs + BC_SPI_SER);
>>> +	if (req->cs_gpiod) {
>>> +		gpiod_set_value_cansleep(req->cs_gpiod,
>>> +					 !!(bs->cfg.mode & SPI_CS_HIGH));
>> If you have a GPIO chip select you should just let the core manage it
>> through cs_gpiod rather than open coding.
> Of course I know this, and normally I would have omitted the GPIO manual
> assertion (hopefully soon my hands get to merging the AX99100 driver I've
> developed some time ago). The thing is that this Baikal-T1 System SSI device
> driver has been initially written before commit 05766050d5bd ("spi: spi-mem:
> fallback to using transfers when CS gpios are used"). So asserting GPIO CS had
> been required to initiate the SPI memory communications seeing the generic
> spi_mem_exec_op() doesn't do this. Manual GPIO manipulation is indeed redundant
> for the current SPI-mem op execution procedure.
>
> Secondly the message of that commit states "Devices with chip selects driven
> via GPIO are not compatible with the spi-mem operations." I find this statement
> questionable, because for instance this device supports memory operations with
> GPIO-driven CS. Though in current implementation the driver fallback to using normal
> push-pull IO mode if GPIO CS is utilized as safer one. But even in this case
> it's better than splitting the memory operations up into the transfers, which is
> developed in the spi_mem_exec_op() method.
On this specific bit. My use-case for 05766050d5bd was a SPI controller 
that supported direct mem accesses but a hardware design that required a 
GPIO CS. So yes I probably should have qualified it as _some_ devices.
> So in this matter my question is: how to modify the SPI-mem interface so the
> SPI-memory operations would also work with GPIO driven CS? Some additional flag
> might work...