Stephen Street <[email protected]> wrote:
>
> ...
> > > + unmap_dma_buffers(drv_data);
> > > +
> > > + /* Calculate number of trailing bytes, read them */
> > > + trailing_sssr = SSP_REG(sssr);
> > > + if ((trailing_sssr & 0xf008) != 0xf000) {
> > > + drv_data->rx = drv_data->rx_end -
> > > + (((trailing_sssr >> 12) & 0x0f) + 1);
> > > + drv_data->read(drv_data);
> > > + }
> > > + msg->actual_length += drv_data->len;
> > > +
> > > + /* Release chip select if requested, transfer delays are
> > > + * handled in pump_transfers */
> > > + if (drv_data->cs_change)
> > > + drv_data->cs_control(PXA2XX_CS_DEASSERT);
> > > +
> > > + /* Move to next transfer */
> > > + msg->state = next_transfer(drv_data);
> > > +
> > > + /* Schedule transfer tasklet */
> > > + tasklet_schedule(&drv_data->pump_transfers);
> > > +
> > > + return IRQ_HANDLED;
> > > + }
> > > +
> > > + /* Never Fail */
> >
> > WARN_ON(1)?
> >
> > Why not return IRQ_NONE here? That way, the IRQ system will save the
> > machine if the IRQ gets stuck.
> >
> In my generally confused state I decided that if the IRQ handler ran
> then by definition I handled the interrupt. But thats probably not
> right. Will change.
Yes, IRQ_NONE means "I don't have a clue why this IRQ handler was called -
none of my device registers indicate that anything needs servicing".
The core kernel IRQ handling will see that as a signal that perhaps the
hardware is busted and ultimately it will disable the entire IRQ line so
the machine can continue to struggle along.
> > This all looks very non-64-bit-capable.
> Just the null_dma_buf issue or something more?
Well, yes, that expression. Generally if you get all the types right and
avoid typecasting, the compiler will shout at you about 64-bit-brokenness.
> > > +#ifdef CONFIG_PM
> > > +static int stall_queue(struct driver_data *drv_data)
> > > +{
> > > + unsigned long flags;
> > > + unsigned limit = 500;
> > > +
> > > + spin_lock_irqsave(&drv_data->lock, flags);
> > > +
> > > + drv_data->run = QUEUE_STALLED;
> > > +
> > > + while (drv_data->busy && limit--) {
> > > + spin_unlock_irqrestore(&drv_data->lock, flags);
> > > + msleep(10);
> > > + spin_lock_irqsave(&drv_data->lock, flags);
> > > + }
> >
> > That looks a bit lame. What's happening here?
> Sort of dumb, I agree. I interpreted PM_EVENT_FREEZE to mean that I
> should stop processing the internal message queue but leave the queue
> intact so that it can be restarted. "stall_queue" does this by setting
> the run flag to QUEUE_STALLED (checked in pump_messages) and waiting for
> the busy to indicate the idle state. I considered using an wait_queue
> but this seemed to much for to little. Would you prefer a wait_queue?
>
I guess a waitqueue would be nicer. I don't recall seeing drivers doing
anything really fancy like this in response to a suspend request though.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
