On Wed, 30 Dec 2009 02:43:48 +0800 Erwin Authried <eauth@xxxxxxxxxxxxx> wrote: Hi Erwin, Thanks for the comments. > > Hi, > > looking at your driver, there are several points that make me doubt > that it is working at all: You can trust me on this point:) it has worked reliably on our HW platform for long time. I don't know if Alan has played with the platform, but David Woodhouse has, max3110's console is one of the main debug methods for developers. Actually it can co-work well with other SPI devices connecting to the same SPI controller(our platform use a Designware core based controller) > > * The MAX3110 requires CS going low at the start of each 16-bit > transfer, if you look at the datasheet. Our SPI controller can handle this (drivers/spi/dw_spi.c), it has a dedicated chip select register for handling chip select. And this part is transparent for all SPI slave devices connecting to the controller > > * in max3110_read_multi, it looks to me that this will work for > big-endian architectures only. We are running on x86 architecture, with little endian. But endian issue is a good point > > * I can't see how send_circ_buf() should work at all. You are just > sending a burst of characters to the MAX3110 without checking the > transmit buffer status at all. The MAX3110 has a single TX buffer > only, so that will cause transmit characters to be lost. Yes, I let the SPI controller driver handle this, which has a rather big FIFO. And structure spi_transfer has a member "speed_hz", this driver set it according to current UART baud rate, then SPI controller driver will adjust the bus frequency between controller and 3110 accordingly to prevent its overflow, which will avoid explicit delay in driver > > I think there's no need for a MAX3100 **and** a MAX3110 driver, this > is just confusing. The MAX3110 driver is identical to the MAX3100 > from the software view, it is simply a MAX3100 with transceivers > added to the chip. If there's any improvement, that should be merged > into the existing MAX3100 driver. I agree there should not be 2 drivers cover 1 family of HW, so this is a RFC. I've thinked about merge with current 3100 code, but it depends on one char per spi_transfer, while my driver relys on batch data transfer for efficiency. Another key point is the console, SPI UART can't be directly accessed by CPU, so every spi_transfer will go through tasklet/workqueue, which makes supporting printk a big part of my driver. Thanks, Feng > > -Erwin > > -- To unsubscribe from this list: send the line "unsubscribe linux-serial" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
