Fixing eSPI controller driver: some queries

[Thomas De Schampheleire <patrickdepinguin+spidevel-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]

Hi,

The Freescale eSPI controller driver is broken in several ways. I already
attempted to fix this with a patch many months back. The patch works for
me, but never got feedback from the original author.
(see https://patchwork.kernel.org/patch/988802/)

The same problems are still present on the current 3.x version of the
driver. I will now re-investigate the problem, and submit a revised patch
later.

I have some queries though:
- When reading from memory devices, the first bytes received may not yet be
the real contents of memory, but rather to-be-ignored bytes caused by the
full-duplex nature of the transaction while sending the command and address
bytes.
Am I correct in understanding that these to-be-ignored bytes are to be
transparently passed through to the requester of the transaction (i.e. a
protocol driver or a userspace application through spidev) and that it's
the requestor's responsability to know that these bytes are to be ignored?

- While investigating the various problems of the driver, I am adding print
statements throughout the code, displaying certain variables and buffer
contents. What should I do with these when the problems are fixed? Remove
all such statements, or rather, use a mechanism like dev_dbg (or similar)
and thus keep them in the code? What is your preference?

- One of the aspects that seems to be broken in the driver is accesses that
do not have either 8-bits-per-word or 16-bits-per-word, e.g. 7 bpw or 12
bpw. The hardware defaults to sending least-significant-bits first, and the
setting to change this is only allowed for 8 or 16 bpw:

-------- (datasheet)
Reverse data mode. Determines the receive and transmit character bit order.
0 lsb of the character sent and received first
1 msb of the character sent and received first-for 8/16 bits data character
only
-------

However, the driver sets this bit (CSMODE_REV) by default, except when the
mode SPI_LSB_FIRST is explicitly set. This means that a protocol driver
that requests a 12bpw transaction, will inadvertently cause an illegal mode
setting in the hardware. It doesn't and shouldn't know of this hardware
restriction. Is this analysis correct?

What is the correct way of handling this? Clear the bit in case of a sub-8
or sub-16 bpw transaction, and do the bit swapping in software if
SPI_LSB_FIRST is not explicitly set?
Or force SPI_LSB_FIRST in these cases, leaving the bit swapping to the
protocol driver?
Or disable support for sub-8 and sub-16 bpw transactions?


Thanks,
Thomas
------------------------------------------------------------------------------
Master HTML5, CSS3, ASP.NET, MVC, AJAX, Knockout.js, Web API and
much more. Get web development skills now with LearnDevNow -
350+ hours of step-by-step video tutorials by Microsoft MVPs and experts.
SALE $99.99 this month only -- learn more at:
http://p.sf.net/sfu/learnmore_122812