Hi Sandeep I had another look at your code and am wondering about the correctness of more comments, and I wonder about error handling in the ISR:
2009/9/1 <[email protected]>: > From: Sandeep Paulraj <[email protected]> > + * davinci_spi_bufs - functions which will handle transfer data > + * @spi: spi device on which data transfer to be done > + * @t: spi transfer in which transfer info is filled > + * > + * This function will put data to be transferred into data register > + * of SPI controller and then wait untill the completion will be marked > + * by the IRQ Handler. > + */ Is this true? For me the code seems to be polling in the transmit case. Look down: > +static int davinci_spi_bufs_pio(struct spi_device *spi, struct spi_transfer > *t) > +{ [snip] > + /* Determine the command to execute READ or WRITE */ > + if (t->tx_buf) { > + clear_io_bits(davinci_spi->base + SPIINT, SPIINT_MASKALL); > + > + while (1) { > + tx_data = davinci_spi->get_tx(davinci_spi); > + > + data1_reg_val &= ~(0xFFFF); > + data1_reg_val |= (0xFFFF & tx_data); > + > + buf_val = ioread32(davinci_spi->base + SPIBUF); > + if ((buf_val & SPIBUF_TXFULL_MASK) == 0) { > + iowrite32(data1_reg_val, > + davinci_spi->base + SPIDAT1); > + > + count--; > + } > + while (ioread32(davinci_spi->base + SPIBUF) > + & SPIBUF_RXEMPTY_MASK) > + cpu_relax(); > + > + /* getting the returned byte */ > + if (t->rx_buf) { > + buf_val = ioread32(davinci_spi->base + > SPIBUF); > + davinci_spi->get_rx(buf_val, davinci_spi); > + } > + if (count <= 0) > + break; > + } In this case you're polling the SPIBUF_TXFULL_MASK bit before you write and then the SPIBUF_RXEMPTY_MASK bit before you read. cpu_relax() is just an alias to barrier(), so there is no wait for an interrupt, am I right? > + * davinci_spi_irq - probe function for SPI Master Controller > + * @irq: IRQ number for this SPI Master > + * @context_data: structure for SPI Master controller davinci_spi > + * > + * ISR will determine that interrupt arrives either for READ or WRITE > command. > + * According to command it will do the appropriate action. It will check > + * transfer length and if it is not zero then dispatch transfer command > again. > + * If transfer length is zero then it will indicate the COMPLETION so that > + * davinci_spi_bufs function can go ahead. > + */ I can't find anything in the below ISR code with regard to differing between READ and WRITE or checking transfer lengthes. > +static irqreturn_t davinci_spi_irq(s32 irq, void *context_data) > +{ > + struct davinci_spi *davinci_spi = context_data; > + u32 int_status, rx_data = 0; > + irqreturn_t ret = IRQ_NONE; > + > + int_status = ioread32(davinci_spi->base + SPIFLG); > + while ((int_status & SPIFLG_MASK) != 0) { SPIFLG_MASK is many more bits than just the interrupt flag. If you got an OVRNINTFLG set, you will never end this loop, as you do not reset this flag in davinci_spi_check_error()! In addition you would acknowledge the interrupt, even if you just got an error and actually another interrupt handler would have been responsible for the interrupt that just occurred. Because of the last argument I would move down the next line into the following if-clause. > + ret = IRQ_HANDLED; > + > + if (likely(int_status & SPIFLG_RX_INTR_MASK)) { > + rx_data = ioread32(davinci_spi->base + SPIBUF); > + davinci_spi->get_rx(rx_data, davinci_spi); > + > + /* Disable Receive Interrupt */ > + iowrite32(~SPIINT_RX_INTR, > + davinci_spi->base + SPIINT); > + } else > + (void)davinci_spi_check_error(davinci_spi, > int_status); > + > + int_status = ioread32(davinci_spi->base + SPIFLG); > + } > + > + return ret; > +} There are also two small typos, "untill" and "bbased" inside comments. Regards Siegbert ------------------------------------------------------------------------------ Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-Day trial. Simplify your report design, integration and deployment - and focus on what you do best, core application coding. Discover what's new with Crystal Reports now. http://p.sf.net/sfu/bobj-july _______________________________________________ spi-devel-general mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/spi-devel-general
