Mailing List Archive

Re: [spi-devel-general] [PATCH][2.16.17-rc1-mm2] spi: Added spi master driver for Freescale MPC83xx SPI controller
Hi,

Kumar Gala wrote:

>+irqreturn_t mpc83xx_spi_irq(s32 irq, void *context_data,
>+ struct pt_regs * ptregs)
>+{
>+ struct mpc83xx_spi *mpc83xx_spi = context_data;
>+ u32 event;
>+ irqreturn_t ret = IRQ_NONE;
>+
>+ /* Get interrupt events(tx/rx) */
>+ event = mpc83xx_spi_read_reg(&mpc83xx_spi->base->event);
>+
>+ /* We need handle RX first */
>+ if (event & SPIE_NE) {
>+ u32 rx_data = mpc83xx_spi_read_reg(&mpc83xx_spi->base->receive);
>+
>+ if (mpc83xx_spi->rx)
>+ mpc83xx_spi->get_rx(rx_data, mpc83xx_spi);
>+
>+ ret = IRQ_HANDLED;
>+ }
>+
>+ if ((event & SPIE_NF) == 0)
>+ /* spin until TX is done */
>+ while (((event =
>+ mpc83xx_spi_read_reg(&mpc83xx_spi->base->event)) &
>+ SPIE_NF) == 0)
>+ ;
>
>
This is a potentially endless loop so two questions here.
First, did you do any measurements on how long it can loop here?
The, what if a bus error occurs and this bit is never set?

>+
>+ mpc83xx_spi->count -= 1;
>+ if (mpc83xx_spi->count) {
>+ if (mpc83xx_spi->tx) {
>+ u32 word = mpc83xx_spi->get_tx(mpc83xx_spi);
>+ mpc83xx_spi_write_reg(&mpc83xx_spi->base->transmit,
>+ word);
>+ }
>+ } else {
>+ complete(&mpc83xx_spi->done);
>+ }
>
>
So, if it's not SPIE_NF, it's not marked as HANDLED?

Vitaly
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [spi-devel-general] [PATCH][2.16.17-rc1-mm2] spi: Added spi master driver for Freescale MPC83xx SPI controller [ In reply to ]
On Apr 10, 2006, at 3:03 PM, Vitaly Wool wrote:

> Hi,
>
> Kumar Gala wrote:
>
>> +irqreturn_t mpc83xx_spi_irq(s32 irq, void *context_data,
>> + struct pt_regs * ptregs)
>> +{
>> + struct mpc83xx_spi *mpc83xx_spi = context_data;
>> + u32 event;
>> + irqreturn_t ret = IRQ_NONE;
>> +
>> + /* Get interrupt events(tx/rx) */
>> + event = mpc83xx_spi_read_reg(&mpc83xx_spi->base->event);
>> +
>> + /* We need handle RX first */
>> + if (event & SPIE_NE) {
>> + u32 rx_data = mpc83xx_spi_read_reg(&mpc83xx_spi->base->receive);
>> +
>> + if (mpc83xx_spi->rx)
>> + mpc83xx_spi->get_rx(rx_data, mpc83xx_spi);
>> +
>> + ret = IRQ_HANDLED;
>> + }
>> +
>> + if ((event & SPIE_NF) == 0)
>> + /* spin until TX is done */
>> + while (((event =
>> + mpc83xx_spi_read_reg(&mpc83xx_spi->base->event)) &
>> + SPIE_NF) == 0)
>> + ;
>>
> This is a potentially endless loop so two questions here.
> First, did you do any measurements on how long it can loop here?
> The, what if a bus error occurs and this bit is never set?

In my measurements the if is never true, so we never spin.

I wasn't particular happy about the spinning for ever, wasn't sure
what would be better. I need to ensure we've gotten both a TX & RX
event before transmitting the next character. I'm open to
suggestions on how to do this better.

>> +
>> + mpc83xx_spi->count -= 1;
>> + if (mpc83xx_spi->count) {
>> + if (mpc83xx_spi->tx) {
>> + u32 word = mpc83xx_spi->get_tx(mpc83xx_spi);
>> + mpc83xx_spi_write_reg(&mpc83xx_spi->base->transmit,
>> + word);
>> + }
>> + } else {
>> + complete(&mpc83xx_spi->done);
>> + }
>>
> So, if it's not SPIE_NF, it's not marked as HANDLED?

I wasn't sure what the best thing here was. In truth we will only
get an interrupt if SPIE_NF is set so it should always be handled.


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [spi-devel-general] [PATCH][2.16.17-rc1-mm2] spi: Added spi master driver for Freescale MPC83xx SPI controller [ In reply to ]
On Monday 10 April 2006 1:22 pm, Kumar Gala wrote:
>

> I wasn't particular happy about the spinning for ever, wasn't sure
> what would be better. I need to ensure we've gotten both a TX & RX
> event before transmitting the next character. I'm open to
> suggestions on how to do this better.

I figured that it's impossible to get an RX event without the TX
having completed. :)

- Dave

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [spi-devel-general] [PATCH][2.16.17-rc1-mm2] spi: Added spi master driver for Freescale MPC83xx SPI controller [ In reply to ]
On Apr 10, 2006, at 4:06 PM, David Brownell wrote:

> On Monday 10 April 2006 1:22 pm, Kumar Gala wrote:
>>
>
>> I wasn't particular happy about the spinning for ever, wasn't sure
>> what would be better. I need to ensure we've gotten both a TX & RX
>> event before transmitting the next character. I'm open to
>> suggestions on how to do this better.
>
> I figured that it's impossible to get an RX event without the TX
> having completed. :)

Should I just get ride of the spin loop then?

- k
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/