On Fri, 18 Jan 2013 10:03:28 +0100, Thomas De Schampheleire <[email protected]> wrote: > 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?
Yes > > - 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? It is common to use dev_dbg(). They are turned off by removing #define DEBUG from the top of the file. As long as they don't impair readability of the code I would keep them in. > - 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? Correct. The master driver needs to work around limitations of the hardware, or reject them as unsupported. > 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? Probably > Or force SPI_LSB_FIRST in these cases, leaving the bit swapping to the > protocol driver? No. it should be abstracted > Or disable support for sub-8 and sub-16 bpw transactions? Only if the first option isn't feasable. g. ------------------------------------------------------------------------------ Free Next-Gen Firewall Hardware Offer Buy your Sophos next-gen firewall before the end March 2013 and get the hardware for free! Learn more. http://p.sf.net/sfu/sophos-d2d-feb _______________________________________________ spi-devel-general mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/spi-devel-general
