[RFC/PATCH] Block device for the ISS simulator - Kernel

This is a discussion on [RFC/PATCH] Block device for the ISS simulator - Kernel ; The ISS simulator is a simple powerpc simulator used among other things for hardware bringup. It implements a simple memory mapped block device interface. This is a simple block driver that attaches to it. Note that the choice of a ...

+ Reply to Thread
Results 1 to 6 of 6

Thread: [RFC/PATCH] Block device for the ISS simulator

  1. [RFC/PATCH] Block device for the ISS simulator

    The ISS simulator is a simple powerpc simulator used among other things
    for hardware bringup. It implements a simple memory mapped block device
    interface.

    This is a simple block driver that attaches to it. Note that the choice
    of a major device number is fishy, though because it's a simulator and
    not real hardware, it's not necessarily a big deal.

    Comments welcome,

    Signed-off-by: Benjamin Herrenschmidt
    ---

    (And yes, I will try to get ISS to implement an IDE emulation instead
    but that's not what's there at this stage)

    arch/powerpc/boot/dts/iss4xx.dts | 5
    drivers/block/Kconfig | 4
    drivers/block/Makefile | 1
    drivers/block/iss_blk.c | 365 +++++++++++++++++++++++++++++++++++++++
    4 files changed, 375 insertions(+)

    --- /dev/null 1970-01-01 00:00:00.000000000 +0000
    +++ linux-work/drivers/block/iss_blk.c 2008-09-23 11:12:03.000000000 +1000
    @@ -0,0 +1,365 @@
    +/*
    + * Simple block device for the ISS simulator
    + */
    +
    +#undef DEBUG
    +
    +#include
    +#include
    +#include
    +#include
    +#include
    +#include
    +#include
    +#include
    +#include
    +#include
    +#include
    +
    +#include
    +
    +#define MAJOR_NR 63 /* FIXME */
    +#define NUM_ISS_BLK_MINOR 4
    +
    +/* Command codes */
    +enum {
    + ISS_BD_CMD_NOP = 0,
    + ISS_BD_CMD_OPEN = 1,
    + ISS_BD_CMD_CLOSE = 2,
    + ISS_BD_CMD_READ = 3,
    + ISS_BD_CMD_WRITE = 4,
    + ISS_BD_CMD_STATUS = 5,
    + ISS_BD_CMD_CHKCHANGE = 6,
    + ISS_BD_CMD_SYNC = 7,
    + ISS_BD_CMD_GET_BLKSIZE = 8,
    + ISS_BD_CMD_GET_DEVSIZE = 9,
    +};
    +
    +/* Status codes */
    +enum {
    + ISS_BD_STATUS_OK = 0,
    + ISS_BD_STATUS_OP_ER = 1, /* Open error */
    + ISS_BD_ALREADY_OPEN = 2, /* Block file already open */
    + ISS_BD_NOT_OPEN = 3, /* Block file not open */
    + ISS_BD_BAD_DEV_NUM = 4, /* Bad device number */
    + ISS_BD_BAD_SEC_CNT = 5, /* Bad sector number */
    + ISS_BD_SEEK_ERROR = 6, /* Bad sector count */
    + ISS_BD_RW_ERROR = 7, /* Read/Write error */
    + ISS_BD_SIZE_ERROR = 8, /* Unable to determine file size */
    +};
    +
    +struct iss_blk_regs {
    + u8 cmd;
    + u8 pad0[3];
    + u32 stat;
    + u32 sector;
    + u32 count;
    + u32 devno;
    + u32 size;
    + u8 pad1[0x1e8];
    + u8 data[0x800];
    +};
    +
    +struct iss_blk {
    + struct gendisk *disk;
    + unsigned int devno;
    + unsigned int sectsize;
    + unsigned int capacity;
    + unsigned int present;
    + unsigned int changed;
    +} iss_blks[NUM_ISS_BLK_MINOR];
    +
    +static spinlock_t iss_blk_qlock;
    +static spinlock_t iss_blk_reglock;
    +static struct iss_blk_regs __iomem *iss_blk_regs;
    +
    +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);
    + if (stat != ISS_BD_STATUS_OK) {
    + pr_debug(" -> no file\n");
    + goto failed;
    + }
    + out_8(&iss_blk_regs->cmd, ISS_BD_CMD_GET_BLKSIZE);
    + ib->sectsize = in_be32(&iss_blk_regs->size);
    + if (ib->sectsize != 512) {
    + pr_err("issblk: unsupported sector size %d\n", ib->sectsize);
    + goto failed;
    + }
    + out_8(&iss_blk_regs->cmd, ISS_BD_CMD_GET_DEVSIZE);
    + ib->capacity = in_be32(&iss_blk_regs->size);
    + ib->present = 1;
    + ib->changed = 0;
    + spin_unlock_irqrestore(&iss_blk_reglock, flags);
    +
    + pr_debug(" -> 0x%x sectors 0f %d bytes\n",
    + ib->capacity, ib->sectsize);
    +
    + blk_queue_bounce_limit(ib->disk->queue, BLK_BOUNCE_HIGH);
    + blk_queue_hardsect_size(ib->disk->queue, ib->sectsize);
    + set_capacity(ib->disk, ib->capacity);
    + return;
    +
    + failed:
    + spin_unlock_irqrestore(&iss_blk_reglock, flags);
    +}
    +
    +static int __iss_blk_read(struct iss_blk *ib, void *buffer,
    + unsigned long sector, unsigned long count)
    +{
    + unsigned long lcount, flags;
    + u32 stat;
    +
    + pr_debug("__iss_blk_read 0x%ld sectors @ 0x%lx\n", count, sector);
    +
    + while(count) {
    + lcount = min(count, 4ul);
    + spin_lock_irqsave(&iss_blk_reglock, flags);
    + out_be32(&iss_blk_regs->devno, ib->devno);
    + out_be32(&iss_blk_regs->sector, sector);
    + out_be32(&iss_blk_regs->count, lcount);
    + out_8(&iss_blk_regs->cmd, ISS_BD_CMD_READ);
    + stat = in_be32(&iss_blk_regs->stat);
    + if (stat != ISS_BD_STATUS_OK) {
    + spin_unlock_irqrestore(&iss_blk_reglock, flags);
    + return -EIO;
    + }
    + memcpy_fromio(buffer, &iss_blk_regs->data, lcount * ib->sectsize);
    + spin_unlock_irqrestore(&iss_blk_reglock, flags);
    + count -= lcount;
    + sector += lcount;
    + buffer += lcount * ib->sectsize;
    + }
    + return 0;
    +}
    +
    +static int __iss_blk_write(struct iss_blk *ib, void *buffer,
    + unsigned long sector, unsigned long count)
    +{
    + unsigned long lcount, flags;
    + u32 stat;
    +
    + pr_debug("__iss_blk_write 0x%ld sectors @ 0x%lx\n", count, sector);
    +
    + while(count) {
    + lcount = min(count, 4ul);
    + spin_lock_irqsave(&iss_blk_reglock, flags);
    + out_be32(&iss_blk_regs->devno, ib->devno);
    + out_be32(&iss_blk_regs->sector, sector);
    + out_be32(&iss_blk_regs->count, lcount);
    + memcpy_toio(&iss_blk_regs->data, buffer, lcount * ib->sectsize);
    + out_8(&iss_blk_regs->cmd, ISS_BD_CMD_WRITE);
    + stat = in_be32(&iss_blk_regs->stat);
    + spin_unlock_irqrestore(&iss_blk_reglock, flags);
    + if (stat != ISS_BD_STATUS_OK)
    + return -EIO;
    + count -= lcount;
    + sector += lcount;
    + buffer += lcount * ib->sectsize;
    + }
    + return 0;
    +}
    +
    +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 */
    + }
    +}
    +
    +static int iss_blk_release(struct inode *inode, struct file *file)
    +{
    + struct gendisk *disk = inode->i_bdev->bd_disk;
    + struct iss_blk *ib = disk->private_data;
    + unsigned long flags;
    +
    + pr_debug("issblk%d: release !\n", disk->first_minor);
    +
    + spin_lock_irqsave(&iss_blk_reglock, flags);
    + out_be32(&iss_blk_regs->devno, ib->devno);
    + out_8(&iss_blk_regs->cmd, ISS_BD_CMD_SYNC);
    + spin_unlock_irqrestore(&iss_blk_reglock, flags);
    +
    + return 0;
    +}
    +
    +static int iss_blk_revalidate(struct gendisk *disk)
    +{
    + struct iss_blk *ib = disk->private_data;
    + unsigned long flags;
    +
    + pr_debug("issblk%d: revalidate !\n", disk->first_minor);
    +
    + if (ib->present && ib->changed) {
    + spin_lock_irqsave(&iss_blk_reglock, flags);
    + out_be32(&iss_blk_regs->devno, ib->devno);
    + out_8(&iss_blk_regs->cmd, ISS_BD_CMD_CLOSE);
    + ib->present = ib->changed = 0;
    + spin_unlock_irqrestore(&iss_blk_reglock, flags);
    + }
    + iss_blk_setup(ib);
    + return 0;
    +}
    +
    +static int iss_blk_media_changed(struct gendisk *disk)
    +{
    + struct iss_blk *ib = disk->private_data;
    +
    + pr_debug("issblk%d: media_changed !\n", disk->first_minor);
    +
    + /* Not implemented -> query change status from ISS */
    +
    + return ib->changed;
    +}
    +
    +static int iss_blk_open(struct inode *inode, struct file *file)
    +{
    + struct gendisk *disk = inode->i_bdev->bd_disk;
    + struct iss_blk *ib = disk->private_data;
    +
    + pr_debug("issblk%d: open !\n", disk->first_minor);
    +
    + check_disk_change(inode->i_bdev);
    + if (ib->changed)
    + iss_blk_setup(ib);
    + if (!ib->present)
    + return -ENOMEDIUM;
    + return 0;
    +}
    +
    +static struct block_device_operations iss_blk_fops = {
    + .owner = THIS_MODULE,
    + .open = iss_blk_open,
    + .release = iss_blk_release,
    + .media_changed = iss_blk_media_changed,
    + .revalidate_disk = iss_blk_revalidate,
    +};
    +
    +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);
    + 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);
    + }
    + }
    +}
    +
    +module_init(iss_blk_init);
    +module_exit(iss_blk_exit);
    +
    +MODULE_DESCRIPTION("ISS Simulator Block Device");
    +MODULE_LICENSE("GPL");
    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
    +
    config CDROM_PKTCDVD
    tristate "Packet writing on CD/DVD media"
    depends on !UML
    Index: linux-work/drivers/block/Makefile
    ================================================== =================
    --- linux-work.orig/drivers/block/Makefile 2008-07-17 14:43:58.000000000 +1000
    +++ linux-work/drivers/block/Makefile 2008-09-23 11:12:03.000000000 +1000
    @@ -30,5 +30,6 @@ obj-$(CONFIG_VIODASD) += viodasd.o
    obj-$(CONFIG_BLK_DEV_SX8) += sx8.o
    obj-$(CONFIG_BLK_DEV_UB) += ub.o
    obj-$(CONFIG_BLK_DEV_HD) += hd.o
    +obj-$(CONFIG_BLK_DEV_ISS) += iss_blk.o

    obj-$(CONFIG_XEN_BLKDEV_FRONTEND) += xen-blkfront.o
    Index: linux-work/arch/powerpc/boot/dts/iss4xx.dts
    ================================================== =================
    --- linux-work.orig/arch/powerpc/boot/dts/iss4xx.dts 2008-09-23 11:12:02.000000000 +1000
    +++ linux-work/arch/powerpc/boot/dts/iss4xx.dts 2008-09-23 11:12:03.000000000 +1000
    @@ -101,6 +101,11 @@
    };
    };

    + iss-block {
    + compatible = "ibm,iss-sim-block-device";
    + reg = <0 0xEF701000 0x1000>;
    + };
    +
    chosen {
    linux,stdout-path = "/plb/opb/serial@40000200";
    };
    --
    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/

  2. Re: [RFC/PATCH] Block device for the ISS simulator

    Benjamin Herrenschmidt writes:

    > The ISS simulator is a simple powerpc simulator used among other things
    > for hardware bringup. It implements a simple memory mapped block device
    > interface.


    ....
    >
    > --- /dev/null 1970-01-01 00:00:00.000000000 +0000
    > +++ linux-work/drivers/block/iss_blk.c 2008-09-23 11:12:03.000000000 +1000
    > @@ -0,0 +1,365 @@
    > +/*
    > + * Simple block device for the ISS simulator
    > + */


    The first paragraph in your description above should be in this comment.

    -Andi
    --
    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/

  3. Re: [RFC/PATCH] Block device for the ISS simulator

    On Fri, Oct 03, 2008 at 10:08:42AM +1000, Benjamin Herrenschmidt wrote:
    > The ISS simulator is a simple powerpc simulator used among other things
    > for hardware bringup. It implements a simple memory mapped block device
    > interface.
    >
    > This is a simple block driver that attaches to it. Note that the choice
    > of a major device number is fishy, though because it's a simulator and
    > not real hardware, it's not necessarily a big deal.


    Please don't put in more block devices for every bloody simulator in the
    world. Just emulated some simpler enough existing hardware.

    --
    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/

  4. Re: [RFC/PATCH] Block device for the ISS simulator

    On Fri, 2008-10-03 at 04:35 -0400, Christoph Hellwig wrote:
    > On Fri, Oct 03, 2008 at 10:08:42AM +1000, Benjamin Herrenschmidt wrote:
    > > The ISS simulator is a simple powerpc simulator used among other things
    > > for hardware bringup. It implements a simple memory mapped block device
    > > interface.
    > >
    > > This is a simple block driver that attaches to it. Note that the choice
    > > of a major device number is fishy, though because it's a simulator and
    > > not real hardware, it's not necessarily a big deal.

    >
    > Please don't put in more block devices for every bloody simulator in the
    > world. Just emulated some simpler enough existing hardware.


    I'm not sure I want it merged, but having it on the list "for the
    record" is useful and it might be useful to get comments in case it's
    terminally busted :-)

    I'm trying to get ISS to implement an IDE interface in fact.

    Cheers,
    Ben.

    --
    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/

  5. Re: [RFC/PATCH] Block device for the ISS simulator

    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.



    >+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));



    >+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?



    >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
    --
    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/

  6. Re: [RFC/PATCH] Block device for the ISS simulator

    On Fri, 2008-10-03 at 08:03 -0400, Josh Boyer wrote:
    > 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.


    Yeah well, old habits :-)

    > >+ 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));


    I knew copy/pasting from a crap driver was going to come back and bite
    me

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


    Yup, suppose so.

    > Shouldn't you unmap iss_blk_regs at this point?


    Might be a good idea yeah.

    Ben.


    --
    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/

+ Reply to Thread