On Sun, Dec 16 2007, Adrian McMenamin wrote:
> +static int gdrom_readdisk_dma(int block, int block_cnt, char *buffer)
> +{
> +     int err;
> +     struct packet_command *read_command;
> +     /* release the spin lock but check later
> +      * we're not in the middle of some dma */
> +     spin_unlock(&gdrom_lock);
> +     ctrl_outl(0x8843407F, GDROM_DMA_ACCESS_CTRL_REG); /* memory setting */
> +     ctrl_outl(9, GDROM_DMA_WAIT_REG); /* DMA word setting */
> +     ctrl_outl(PHYSADDR(buffer), GDROM_DMA_STARTADDR_REG);
> +     ctrl_outl(block_cnt * GDROM_HARD_SECTOR, GDROM_DMA_LENGTH_REG);
> +     ctrl_outl(1, GDROM_DMA_DIRECTION_REG);
> +     ctrl_outl(1, GDROM_DMA_ENABLE_REG);
> +     /* send command */
> +     read_command = kzalloc(sizeof(struct packet_command), GFP_KERNEL);
> +     if (!read_command)
> +             return -ENOMEM;
> +     read_command->cmd[0] = 0x30;
> +     read_command->cmd[1] = 0x20;
> +     read_command->cmd[2] = (block >> 16) & 0xFF;
> +     read_command->cmd[3] = (block >> 8) & 0xFF;
> +     read_command->cmd[4] = block & 0xFF;
> +     read_command->cmd[8] = (block_cnt >> 16) & 0xFF;
> +     read_command->cmd[9] = (block_cnt >> 8) & 0xFF;
> +     read_command->cmd[10] = block_cnt & 0xFF;
> +     /* set for DMA */
> +     ctrl_outb(1, GDROM_ERROR_REG);
> +     /* other parameters */
> +     ctrl_outb(0, GDROM_SECNUM_REG);
> +     ctrl_outb(0, GDROM_BCL_REG);
> +     ctrl_outb(0, GDROM_BCH_REG);
> +     ctrl_outb(0, GDROM_DSEL_REG);
> +     ctrl_outb(0, GDROM_INTSEC_REG);
> +     /* In multiple DMA transfers need to wait */
> +     while (ctrl_inb(GDROM_ALTSTATUS_REG) & 0x80)
> +             cpu_relax();
> +     ctrl_outb(GDROM_COM_PACKET, GDROM_STATUSCOMMAND_REG);
> +     while ((ctrl_inb(GDROM_ALTSTATUS_REG) & 0x88) != 8)
> +             cpu_relax(); /* wait for DRQ to be set to 1 */
> +     gd.pending = 1;
> +     gd.transfer = 1;
> +     outsw(PHYSADDR(GDROM_DATA_REG), &read_command->cmd, 6);
> +     while (ctrl_inb(GDROM_DMA_STATUS_REG))
> +             cpu_relax();
> +     ctrl_outb(1, GDROM_DMA_STATUS_REG);
> +     /* 5 second error margin here seems more reasonable */
> +     wait_event_interruptible_timeout(request_queue, gd.transfer == 0, HZ * 
> 5);
> +     err = ctrl_inb(GDROM_DMA_STATUS_REG);
> +     err = gd.transfer;
> +     gd.transfer = 0;
> +     gd.pending = 0;
> +     kfree(read_command);
> +     spin_lock(&gdrom_lock);
> +     return err;
> +}    
> +
> +static void gdrom_request_handler_dma(struct request *req)
> +{
> +     int err, block, block_cnt;
> +     err = 0;
> +     block = req->sector/GD_TO_BLK + GD_SESSION_OFFSET;
> +     block_cnt = req->nr_sectors/GD_TO_BLK;
> +     err = gdrom_readdisk_dma(block, block_cnt, req->buffer);        
> +     if (unlikely(err)) {
> +             end_request(req, 0);
> +             spin_unlock(&gdrom_lock);
> +             gdrom_getsense(NULL);
> +             spin_lock(&gdrom_lock);
> +     }
> +     else
> +             end_request(req, 1);
> +}
> +
> +static void gdrom_request(struct request_queue *rq)
> +{
> +     struct request *req;
> +     unsigned long pages;
> +     pages = rq->backing_dev_info.ra_pages;
> +     while ((req = elv_next_request(rq)) != NULL) {
> +             if (! blk_fs_request(req)) {
> +                     printk(KERN_DEBUG "GDROM: Non-fs request ignored\n");
> +                     end_request(req, 0);
> +             }
> +             if (rq_data_dir(req)) {
> +                     printk(KERN_NOTICE "GDROM: Read only device - write 
> request ignored\n");
> +                     end_request(req, 0);
> +             }
> +             if (req->nr_sectors) {
> +                     gdrom_request_handler_dma(req);
> +             }
> +     }
> +}
> +

Few notes:

- Compare rq_data_dir() with WRITE, don't just assume that any non-zero
  will be a write.

- You need to offload this request handling to a workqueue of some sort,
  your current request handling is very broken for two reasons: One is
  that interrupts are still disabled when you drop your queue lock, so
  you cannot use sleeping functions like GFP_KERNEL allocations or
  wait_event(). The other is that it's illegal to sleep from your
  request_fn context in the first place, since you could be stalling
  others.

- You also seem to be busy waiting for other transactions to finish. Any
  idea how long those might take? Perhaps put an upper bound on this
  waiting, and/or do blocking waits?

- I'm assuming this hardware can't do sg transfers?

-- 
Jens Axboe

-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to