On Fri, Oct 03, 2008 at 10:08:42AM +1000, Benjamin Herrenschmidt wrote:
>+static void iss_blk_setup(struct iss_blk *ib)
>+{
>+      unsigned long flags;
>+      u32 stat;
>+
>+      pr_debug("iss_blk_setup %d\n", ib->devno);
>+
>+      spin_lock_irqsave(&iss_blk_reglock, flags);
>+      out_8(iss_blk_regs->data, 0);
>+      out_be32(&iss_blk_regs->devno, ib->devno);
>+      out_8(&iss_blk_regs->cmd, ISS_BD_CMD_OPEN);
>+      stat = in_be32(&iss_blk_regs->stat);

Should probably use the ioread/iowrite functions instead of raw out/in.
Same comment throughout.

<snip>

>+static void iss_blk_do_request(struct request_queue * q)
>+{
>+      struct iss_blk *ib = q->queuedata;
>+      struct request *req;
>+      int rc = 0;
>+
>+      pr_debug("iss_do_request dev %d\n", ib->devno);
>+
>+      while ((req = elv_next_request(q)) != NULL) {
>+              pr_debug(" -> req @ %p, changed: %d\n", req, ib->changed);
>+              if (ib->changed) {
>+                      end_request(req, 0);    /* failure */
>+                      continue;
>+              }
>+              switch (rq_data_dir(req)) {
>+              case READ:
>+                      rc = __iss_blk_read(ib, req->buffer, req->sector,
>+                                          req->current_nr_sectors);
>+                      break;
>+              case WRITE:
>+                      rc = __iss_blk_write(ib, req->buffer, req->sector,
>+                                           req->current_nr_sectors);
>+              };
>+
>+              pr_debug(" -> ending request, rc = %d\n", rc);
>+              if (rc)
>+                      end_request(req, 0);    /* failure */
>+              else
>+                      end_request(req, 1);    /* success */

Could possibly just do:

end_request(req, (!rc));

<snip>

>+static int __init iss_blk_init(void)
>+{
>+      struct device_node *np;
>+      int i;
>+
>+      pr_debug("iss_regs offsets:\n");
>+      pr_debug("  cmd    : 0x%x\n", offsetof(struct iss_blk_regs, cmd));
>+      pr_debug("  stat   : 0x%x\n", offsetof(struct iss_blk_regs, stat));
>+      pr_debug("  sector : 0x%x\n", offsetof(struct iss_blk_regs, sector));
>+      pr_debug("  count  : 0x%x\n", offsetof(struct iss_blk_regs, count));
>+      pr_debug("  devno  : 0x%x\n", offsetof(struct iss_blk_regs, devno));
>+      pr_debug("  size   : 0x%x\n", offsetof(struct iss_blk_regs, size));
>+      pr_debug("  data   : 0x%x\n", offsetof(struct iss_blk_regs, data));
>+
>+      np = of_find_node_by_path("/iss-block");
>+      if (np == NULL)
>+              return -ENODEV;
>+      iss_blk_regs = of_iomap(np, 0);

of_find_node_by_path increments the refcount for that node.  Need to
do an of_node_put when you are done.

>+      if (iss_blk_regs == NULL) {
>+              pr_err("issblk: Failed to map registers\n");
>+              return -ENOMEM;
>+      }
>+
>+      if (register_blkdev(MAJOR_NR, "iss_blk"))
>+              return -EIO;
>+
>+      spin_lock_init(&iss_blk_qlock);
>+      spin_lock_init(&iss_blk_reglock);
>+
>+      printk(KERN_INFO "ISS Block driver initializing for %d minors\n",
>+             NUM_ISS_BLK_MINOR);
>+
>+      for (i = 0; i < NUM_ISS_BLK_MINOR; i++) {
>+              struct gendisk *disk = alloc_disk(1);
>+              struct request_queue *q;
>+              struct iss_blk *ib = &iss_blks[i];
>+
>+              if (!disk) {
>+                      pr_err("issblk%d: Failed to allocate disk\n", i);
>+                      break;
>+              }
>+
>+              q = blk_init_queue(iss_blk_do_request, &iss_blk_qlock);
>+              if (q == NULL) {
>+                      pr_err("issblk%d: Failed to init queue\n", i);
>+                      put_disk(disk);
>+                      break;
>+              }
>+              q->queuedata = ib;
>+
>+              ib->disk = disk;
>+              ib->devno = i;
>+              ib->present = 0;
>+              ib->changed = 0;
>+              ib->capacity = 0;
>+              ib->sectsize = 512;
>+
>+              disk->major = MAJOR_NR;
>+              disk->first_minor = i;
>+              disk->fops = &iss_blk_fops;
>+              disk->private_data = &iss_blks[i];
>+              disk->flags = GENHD_FL_REMOVABLE;
>+              disk->queue = q;
>+              sprintf(disk->disk_name, "issblk%d", i);
>+
>+              iss_blk_setup(ib);
>+
>+              add_disk(disk);
>+      }
>+
>+      return 0;
>+}
>+
>+static void __exit iss_blk_exit(void)
>+{
>+      int i;
>+
>+      unregister_blkdev(MAJOR_NR, "iss_blk");
>+
>+      for (i = 0; i < NUM_ISS_BLK_MINOR; i++) {
>+              struct iss_blk *ib = &iss_blks[i];
>+
>+              if (ib->present) {
>+                      out_be32(&iss_blk_regs->devno, ib->devno);
>+                      out_8(&iss_blk_regs->cmd, ISS_BD_CMD_CLOSE);
>+              }
>+      }

Shouldn't you unmap iss_blk_regs at this point?

<snip>

>Index: linux-work/drivers/block/Kconfig
>===================================================================
>--- linux-work.orig/drivers/block/Kconfig      2008-07-17 14:43:58.000000000 
>+1000
>+++ linux-work/drivers/block/Kconfig   2008-09-23 11:12:03.000000000 +1000
>@@ -357,6 +357,10 @@ config BLK_DEV_XIP
>         will prevent RAM block device backing store memory from being
>         allocated from highmem (only a problem for highmem systems).
> 
>+config BLK_DEV_ISS
>+       bool "Support ISS Simulator Block Device"
>+       default n
>+

Should probably have:

        depends on PPC



josh
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

Reply via email to