linux-kernel - Re: [PATCH v4 2/2] spi: spi-pic32: Add PIC32 SPI master 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: <56FBAF2C.9090401@microchip.com>
Date:	Wed, 30 Mar 2016 16:19:16 +0530
From:	Purna Chandra Mandal <purna.mandal@...rochip.com>
To:	Mark Brown <broonie@...nel.org>
CC:	<linux-kernel@...r.kernel.org>, <linux-spi@...r.kernel.org>,
	Sergei Shtylyov <sergei.shtylyov@...entembedded.com>,
	Joshua Henderson <digitalpeer@...italpeer.com>,
	Ulf Hansson <ulf.hansson@...aro.org>
Subject: Re: [PATCH v4 2/2] spi: spi-pic32: Add PIC32 SPI master driver

On 03/29/2016 09:55 PM, Mark Brown wrote:

> On Tue, Mar 29, 2016 at 05:32:41PM +0530, Purna Chandra Mandal wrote:
>
>> It is required for MMC-over-SPI support. Linux MMC_SPI driver sometimes
>> (depending on some logic) want chip-select to be kept enabled (using
>> transfer.cs_change) even at the end of SPI message so that following
>> message(s) can also be made part of the running MMC transaction.
>> In this case if there is any error (and in some other cases as well)
> If your hardware doesn't support per-/CS setup then your driver needs to
> record whatever is configured via setup() and apply it later.

PIC32 SPI controller supports one /CS per instance. So per-/CS specific
logic might not be required.

>> MMC_SPI will have to terminate on-going MMC transactions and it is
>> done by calling setup(). It is assumed that setup() will always leave
>> the chip-select deactivated.
> No, this is just completely broken - that's quite simply not what
> setup() does.  See spi-summary.  There is *no* code in the core that
> terminates ongoing transfers as a result of calling setup(), if that's
> happening it's a result of triggering error handling.

Description of spi_setup() in spi.c clearly mentions that "When this
function returns, the spi device is deselected" and this is only
possible if (at least) chip-select is deactivated.

So based on spi-summary and spi_setup() description, except chip-select,
all other settings has to be recorded and applied later when device is
selected and data is transferred to or from the device.

Keeping this in mind I'll just retain cs-deselect logic in setup() and
apply the recorded setting in prepare_message() callback, if and when required.

>> Best thing I can add is wait for unprepare_message() before calling
>> cleanup().
> The cleanup() function is for deallocating dynamically allocated data,
> it will be called by the core.

ack.

>> If you could suggest better alternative that will be great.
> If you want to change the chip select you need to use the normal
> interfaces for changing the chip select via message processing.

ack.