Re: [PATCH] spi: rb4xx: Fix set_cs logic.

[Bert Vermeulen <bert@biot.com>]

On 04/21/2015 11:46 AM, Geert Uytterhoeven wrote:
> On Mon, Apr 20, 2015 at 10:37 PM, Mark Brown <broonie@kernel.org> wrote:
>> On Mon, Apr 20, 2015 at 03:53:25PM +0200, Bert Vermeulen wrote:
>>> As it turns out, the set_cs() enable parameter refers to the logic level
>>> on the CS pin, not the state of chip selection.
>>
>>> This broke functionality of the LEDs behind the CPLD, or at least delayed
>>> the commands until another one came in to toggle CS.
>>
>> No, the enable parameter *should* refer to chip select assertion (see
>> how we handle GPIO chip selects).  However it's possible that this
>> device has an inverted chip select and should be registered with the
>> SPI_CS_HIGH flag?
> 
> It's logic level:
> 
>  * @set_cs: set the logic level of the chip select line.  May be called
>  *          from interrupt context.

Right It's the implementation which doesn't really make sense IMHO: it
always inverts the "enable" parameter (unless SPI_CS_HIGH is set), in
keeping with the default active-low.

So the docs are right, but "enable" doesn't match what it does. Chip select
assertion would be a better API here. Is it worth fixing?


-- 
Bert Vermeulen        bert@biot.com          email/xmpp