> +
> +MODULE_AUTHOR("Lauri Kasanen <cand@xxxxxxx>");
> +MODULE_DESCRIPTION("Driver for the N64 cart");
> +MODULE_LICENSE("GPL");
> +
> +#define BUFSIZE (64 * 1024)
Maybe use SZ_64K here?
> + void *dst = bio_data(req->bio);
> +
> + if (bstart + len > size)
> + return BLK_STS_IOERR;
> +
> + bstart += start;
> +
> + while (len) {
> + const u32 curlen = len < BUFSIZE ? len : BUFSIZE;
> +
> + dma_cache_inv((unsigned long) buf, curlen);
dma_cache_inv is not a public API, but only a helper for the DMA API.
> + n64cart_wait_dma();
> +
> + barrier();
> + n64cart_write_reg(PI_DRAM_REG, dma_addr);
> + barrier();
barrier is just a compiler barrier. But the mmio APIs already include
actual cpu barriers where applicable, so I don't think you need these at
al.
> + n64cart_write_reg(PI_CART_REG, (bstart | 0x10000000) & 0x1FFFFFFF);
Overly long line. Also this looks a little too magic, can you explain
what is going on here with symbolic constants and&or a helper?
> +
> + do {
> + err = get_seg(req);
> + } while (blk_update_request(req, err, blk_rq_cur_bytes(req)));
> +
> + blk_mq_end_request(req, BLK_STS_OK);
This should be __blk_mq_end_request given that you call
blk_update_request manually.
> + major = register_blkdev(0, "n64cart");
No need to call register_blkdev in a new driver, just set the
GENHD_FL_EXT_DEVT flag in struct gendisk.
> + buf = kmalloc(BUFSIZE, GFP_DMA | GFP_KERNEL);
> + if (!buf) {
> + err = -ENOMEM;
> + goto fail_queue;
> + }
> + dma_addr = virt_to_phys(buf);
Please use dma_aloc_noncoherent to allocate dma memory. And then
use the dma_sync_single_for_{device,cpu} to transfer ownership to
and from the device.
Just curious: wouldn't it make more sense to implement this as a bio
based driver with a helper thread? It doesn't look like blk-mq provides
you a lot of benefits here. And then we could make the blk-mq code
optional and you could save a fair amount of a kernel memory on your
rather constrained device.
