Am Mittwoch, den 30.12.2009, 00:05 +0800 schrieb Tang, Feng: > >-----Original Message----- > >From: Baruch Siach [mailto:baruch@xxxxxxxxxx] > >Sent: 2009年12月29日 23:00 > >To: Tang, Feng > >Cc: Greg Kroah-Hartman; David Brownell; Grant Likely; spi-devel-list; > >linux-serial@xxxxxxxxxxxxxxx; alan@xxxxxxxxxxxxxxxxxxx; Andrew Morton > >Subject: Re: [spi-devel-general] [RFC][PATCH] serial: spi: add spi-uart driver for > >Maxim 3110 > > > >Hi Feng, > > > >On Tue, Dec 29, 2009 at 10:20:06PM +0800, Feng Tang wrote: > >> Here is a driver for Maxim 3110 SPI-UART device, please help to review. > > > >Is this 3110 device so significantly different from the MAX3100 driver that's > >already in the mainline kernel (drivers/serial/max3100.c), to require a whole > >new driver? > > Yes, I know this question will be asked :) I developed the max3110 before max3100 > was posted in public, so the 2 designs differs a lot from the start. I think this driver > has 2 good points: > 1. It provides a console, which is the main reason that our platform use max3110 > 2. It utilizes the RX buffer of max3110 that it can reads up to 8 characters in one > spi_transfer, and its Tx function can transmit up to 128 chars in one spi_transfer > which will save much system load comparing to 1 char per spi transfer > > Current max3100.c also has its advantage, like good support in CTS/RTS control, > and I think these 2 can merge in the future. > > Thanks, > Feng > > > >> It has been validated with Designware SPI controller (drivers/spi: dw_spi.c > >> & dw_spi_pci.c). It supports polling and IRQ mode, supports batch read, and > >> provides a console. > > > >baruch > > > >-- Hi, looking at your driver, there are several points that make me doubt that it is working at all: * The MAX3110 requires CS going low at the start of each 16-bit transfer, if you look at the datasheet. * in max3110_read_multi, it looks to me that this will work for big-endian architectures only. * 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. 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. -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
