> You're right about that. I was not questioning your code. That seems to
> be o.k. for me (without having tested it). I was questioning your
> comment on top of the function, as it describes something, that is not
> implemented in your function. Only one out of three possible code paths
> use interrupts, so the comment should reflect the other two polling
> paths, too.
> 
> >>> + * 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.
> 
> > [Sandeep] as I have mentioned above we do not have a transmit interrupt
> 
> Again, my remark was only about the comment describing the ISR. That one
> is not in accordance with your actual implementation.
> 
> >>> +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.
> 
> > [Sandeep] Yes that's correct
> 
> So here, it's the code that must be changed.
[Sandeep] I have to get in touch with David Brownell regarding this.
Earlier what I did was when I submitted the patch I took care of some comments 
pointed out to me but that got a NAK because it was not the original that Dave 
had sent.

I think these fixes are to be sent as incremental patches.
> 
> Best 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

Reply via email to