On Mon, Jan 04, 2021 at 03:50:31PM +0200, Lauri Kasanen wrote:
> block folks: rest of the series is on linux-mips. Being a mips-specific driver,
> not sure which tree it should go to.
Probably through mips.
> +static void n64cart_wait_dma(void)
> +{
> + while (n64cart_read_reg(PI_STATUS_REG) &
> + (PI_STATUS_DMA_BUSY | PI_STATUS_IO_BUSY))
> + ;
> +}
These sorts of loops generally call cpu_relax().
> +static blk_status_t get_seg(struct request *req)
> +{
> + u32 bstart = blk_rq_pos(req) * 512;
> + u32 len = blk_rq_cur_bytes(req);
> + void *dst = bio_data(req->bio);
> +
> + if (bstart + len > size || rq_data_dir(req) == WRITE)
> + return BLK_STS_IOERR;
If you don't support writes (is that limitation temporary?), then you
can prevent such operations from reaching the driver by setting the
"disk" to read-only during initialization with set_disk_ro(disk, true).
> +static blk_status_t n64cart_queue_rq(struct blk_mq_hw_ctx *hctx,
> + const struct blk_mq_queue_data *bd)
> +{
> + unsigned long flags;
> + struct request *req = bd->rq;
> + blk_status_t err;
> +
> + blk_mq_start_request(req);
> +
> + spin_lock_irqsave(&n64cart_lock, flags);
The .queue_rq() isn't called from an interrupts disabled context, so
there should be no need to save the flags. And since you're not taking
this lock from an interrupt context anywhere else, I don't think you
need to use the spin_lock_irq() variant either.
But since you just want to single-thread all IO, you could get that by
setting your tagset queue_depth to 1 and remove the spinlock entirely.
> +static int __init n64cart_init(void)
> +{
> + int err;
> +
> + if (!start || !size) {
> + pr_err("n64cart: start and size not specified\n");
> + return -ENODEV;
> + }
Just curious, is it not possible to discover these values from the
installed cart? Requiring module parameters seems a bit fragile.
