Re: [PATCH] input/spi: add ads7843 support to ads7846 touchscreen driver

On Thursday 21 December 2006 6:40 am, Nicolas Ferre wrote:
Nicolas Ferre a écrit :
As the SPI underlying code behaves quite differently from a
controller driver to another whan not having a tx_buf filled,
I have add a zerroed buffer to give to the spi layer while
receiving data.

You must be working with a buggy controller driver then. That part of
this patch should never be needed. It's expected that rx-only transfers
will omit a tx buf; all controller drivers must handle that case.

I said that because it is true that most of spi controller drivers
manage rx only transactions filling the tx buffer with zerros but the
spi_s3c24xx.c driver seems to fill with ones (line 177 hw_txbyte())

Anyway, I will check in our controller driver to sort this out.

I dug a bit into this.

I always like to see followup on such issues. :)


Well, in the atmel_spi driver code, we use previous rx buffer if we do
not provide a tx_buf (as it is said that in struct spi_transfer
comments, "If the transmit buffer is null, undefined data will be
shifted out while filling rx_buf").

That seems like a reasonable approach. If it's undefined, the only
reasons I can think of to not use the rx buffer are that:

(a) the cachelines might not be managed correctly during DMA ...
i.e. to defend against a bug in the controller driver; and
(b) that writing such _defined_ data would be a "covert channel"
in the security sense.

Now, (a) is just a bug to fix, and in most cases (b) won't be an
issue since even if the system with that driver is being evaluated
at a level where such channels matter, the hardware hooked up via
SPI will probably already be trusted. (Unless the system allows
thing like MMC or SD cards...) However, see below.


So, the touchscreen controller sees sometimes a "start" condition (bit 7
of a control byte). It then takes the control byte and sets trash bits
as a configuration.

Actually, now that I look at it I see that the docs for both the
ads7846 and the ads7843 show that DIN/MOSI should be zero/low
after the command is given, while reading DOUT/MISO.

Which means that the real issue here is that the driver was wrong
in the first place to not explicitly write zeroes while it's reading
back that data.

The testing I've done with it involved two controller drivers which
do happen to interpret "undefined" as "write zeros": omap_uwire,
which I'm guessing does so in hardware (MicroWire is half duplex),
and pxa2xx_spi, which explicitly writes zeros (null_writer).


I ran into those troubles and add a zerroed buffer as tx.

Do you think that shifting zerros out when a tx_buf is not specified is
the desired behavior ?

I'm leaning towards a "yes" there -- changing the spec for spi_transfer.

The alternative would seem to mean teaching the SPI interface about
two different kinds of "rx only" transfers, one with "must tx zero"
semantics and the other with "tx data doesn't matter" (and security
audits would define it anyway, to avoid covert channels). And I can't
easily justify that.

For now, I'd suggest you update the atmel_spi driver so that it shifts
zeroes in that case; can't hurt (too much). And I'll forward the issue
to the SPI list. If nobody there objects, I'll send patches to update
the spec for spi_transfer, and the s3c driver.

- Dave

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Relevant Pages

  • Re: [PATCH] input/spi: add ads7843 support to ads7846 touchscreen driver
    ... to the spi layer while receiving data. ... I said that because it is true that most of spi controller drivers manage rx only transactions filling the tx buffer with zerros but the spi_s3c24xx.c driver seems to fill with ones ) ... I will check in our controller driver to sort this out. ... I ran into those troubles and add a zerroed buffer as tx. ...
    (Linux-Kernel)
  • Re: where is eboots ethernet download buffer?
    ... oh but i am using my own controller driver, and the big data buffer I am ... >> and I found out that someone is using this space during image download. ... >> seems it is the eboot downloading buffer. ...
    (microsoft.public.windowsce.platbuilder)
  • MC251x CAN controller driver example
    ... And here's an example of a CAN controller driver. ... MCP251x which uses an SPI interface to the ... +} + +static void mcp251x_hw_sleep+{ ... +} + +static int mcp251x_open+{ ...
    (Linux-Kernel)
  • Re: [PATCH] SPI
    ... >>It'd be fine if for example your PNX controller driver worked that way ... Why couldn't for example SPI sit on a PCI bus? ... >>Doing it the way you are prevents you from declaring all the SPI devices in ... cluttering headers which are addressed to _every_ driver. ...
    (Linux-Kernel)