> 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