diff mbox series

[6/6] block: Add n64 cart driver

Message ID 20210104155031.9b4e39ff48a6d7accc93461d@gmx.com (mailing list archive)
State Not Applicable
Headers show
Series [1/6] Revert "MIPS: Remove unused R4300 CPU support" | expand

Commit Message

Lauri Kasanen Jan. 4, 2021, 1:50 p.m. UTC
Signed-off-by: Lauri Kasanen <cand@gmx.com>
---
 drivers/block/Kconfig   |   6 ++
 drivers/block/Makefile  |   1 +
 drivers/block/n64cart.c | 203 ++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 210 insertions(+)
 create mode 100644 drivers/block/n64cart.c

block folks: rest of the series is on linux-mips. Being a mips-specific driver,
not sure which tree it should go to.

--
2.6.2

Comments

Jens Axboe Jan. 4, 2021, 3:40 p.m. UTC | #1
On 1/4/21 6:50 AM, Lauri Kasanen wrote:
> Signed-off-by: Lauri Kasanen <cand@gmx.com>
> ---
>  drivers/block/Kconfig   |   6 ++
>  drivers/block/Makefile  |   1 +
>  drivers/block/n64cart.c | 203 ++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 210 insertions(+)
>  create mode 100644 drivers/block/n64cart.c
> 
> block folks: rest of the series is on linux-mips. Being a
> mips-specific driver, not sure which tree it should go to.

It should definitely get reviewed first. One easy question - there's no
commit message in this one. It's a new driver, it should include some
documentation on what it is, etc.
Lauri Kasanen Jan. 4, 2021, 3:43 p.m. UTC | #2
On Mon, 4 Jan 2021 08:40:10 -0700
Jens Axboe <axboe@kernel.dk> wrote:

> On 1/4/21 6:50 AM, Lauri Kasanen wrote:
> > Signed-off-by: Lauri Kasanen <cand@gmx.com>
> > ---
> >  drivers/block/Kconfig   |   6 ++
> >  drivers/block/Makefile  |   1 +
> >  drivers/block/n64cart.c | 203 ++++++++++++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 210 insertions(+)
> >  create mode 100644 drivers/block/n64cart.c
> >
> > block folks: rest of the series is on linux-mips. Being a
> > mips-specific driver, not sure which tree it should go to.
>
> It should definitely get reviewed first. One easy question - there's no
> commit message in this one. It's a new driver, it should include some
> documentation on what it is, etc.

It's already had one (rfc) round on linux-mips. Or do you mean by
others (who?)

- Lauri
Keith Busch Jan. 4, 2021, 3:49 p.m. UTC | #3
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.
Jens Axboe Jan. 4, 2021, 3:49 p.m. UTC | #4
On 1/4/21 8:43 AM, Lauri Kasanen wrote:
> On Mon, 4 Jan 2021 08:40:10 -0700
> Jens Axboe <axboe@kernel.dk> wrote:
> 
>> On 1/4/21 6:50 AM, Lauri Kasanen wrote:
>>> Signed-off-by: Lauri Kasanen <cand@gmx.com>
>>> ---
>>>  drivers/block/Kconfig   |   6 ++
>>>  drivers/block/Makefile  |   1 +
>>>  drivers/block/n64cart.c | 203 ++++++++++++++++++++++++++++++++++++++++++++++++
>>>  3 files changed, 210 insertions(+)
>>>  create mode 100644 drivers/block/n64cart.c
>>>
>>> block folks: rest of the series is on linux-mips. Being a
>>> mips-specific driver, not sure which tree it should go to.
>>
>> It should definitely get reviewed first. One easy question - there's no
>> commit message in this one. It's a new driver, it should include some
>> documentation on what it is, etc.
> 
> It's already had one (rfc) round on linux-mips. Or do you mean by
> others (who?)

I mean the actual block driver from the block point of view. From
a quick look, looks like there's no irq or anything, so it's
strictly polled mode? Probably would make sense to push this out
of the app path then, by using BLK_MQ_F_BLOCKING.
Lauri Kasanen Jan. 4, 2021, 3:56 p.m. UTC | #5
On Mon, 4 Jan 2021 08:49:55 -0700
Jens Axboe <axboe@kernel.dk> wrote:

> On 1/4/21 8:43 AM, Lauri Kasanen wrote:
> > On Mon, 4 Jan 2021 08:40:10 -0700
> > Jens Axboe <axboe@kernel.dk> wrote:
> >> It should definitely get reviewed first. One easy question - there's no
> >> commit message in this one. It's a new driver, it should include some
> >> documentation on what it is, etc.
> >
> > It's already had one (rfc) round on linux-mips. Or do you mean by
> > others (who?)
>
> I mean the actual block driver from the block point of view. From
> a quick look, looks like there's no irq or anything, so it's
> strictly polled mode? Probably would make sense to push this out
> of the app path then, by using BLK_MQ_F_BLOCKING.

It's very unclear to me what that flag does. I read all the docs
available, but nothing made it clear (same applies for almost all
BLK_MQ_*).

- Lauri
Lauri Kasanen Jan. 4, 2021, 4:01 p.m. UTC | #6
On Mon, 4 Jan 2021 07:49:02 -0800
Keith Busch <kbusch@kernel.org> wrote:

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

Thanks.

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

The media is read-only (but not runtime removable).

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

No, it's not possible. Even the cart's size is not discoverable, and
where the disk image starts and how long it is could only be scanned if
you knew in advance what FS what used, etc. There is no FS on the cart
itself, it's just raw data.

The bootloader embeds these values into itself when it's built.

- Lauri
Jens Axboe Jan. 4, 2021, 4:07 p.m. UTC | #7
On 1/4/21 8:56 AM, Lauri Kasanen wrote:
> On Mon, 4 Jan 2021 08:49:55 -0700
> Jens Axboe <axboe@kernel.dk> wrote:
> 
>> On 1/4/21 8:43 AM, Lauri Kasanen wrote:
>>> On Mon, 4 Jan 2021 08:40:10 -0700
>>> Jens Axboe <axboe@kernel.dk> wrote:
>>>> It should definitely get reviewed first. One easy question - there's no
>>>> commit message in this one. It's a new driver, it should include some
>>>> documentation on what it is, etc.
>>>
>>> It's already had one (rfc) round on linux-mips. Or do you mean by
>>> others (who?)
>>
>> I mean the actual block driver from the block point of view. From
>> a quick look, looks like there's no irq or anything, so it's
>> strictly polled mode? Probably would make sense to push this out
>> of the app path then, by using BLK_MQ_F_BLOCKING.
> 
> It's very unclear to me what that flag does. I read all the docs
> available, but nothing made it clear (same applies for almost all
> BLK_MQ_*).

It pushes the actual processing to a helper thread instead of doing
it inline on submit. Generally used if the driver needs to block
to submit IO, as per the name, but also useful for really slow
hardware like this one.
Keith Busch Jan. 4, 2021, 4:13 p.m. UTC | #8
On Mon, Jan 04, 2021 at 06:01:15PM +0200, Lauri Kasanen wrote:
> On Mon, 4 Jan 2021 07:49:02 -0800
> Keith Busch <kbusch@kernel.org> wrote:
> 
> > 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.
> 
> Thanks.

To be more clear, the initial commit probably makes since to go through
mips once the series is ready, but new block drivers do need to be sent
to linux-block for the appropriate acks and reviews first.
 
> > > +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).
> 
> The media is read-only (but not runtime removable).

It's been a while, but I could swear we can save state on these carts.
If so, it sounds like that must be separate from the media this driver
is accessing, so is that capability provided through a different driver?
Lauri Kasanen Jan. 4, 2021, 4:27 p.m. UTC | #9
On Mon, 4 Jan 2021 08:13:32 -0800
Keith Busch <kbusch@kernel.org> wrote:

> > The media is read-only (but not runtime removable).
>
> It's been a while, but I could swear we can save state on these carts.
> If so, it sounds like that must be separate from the media this driver
> is accessing, so is that capability provided through a different driver?

Saving uses a separate mechanism, and there are several depending on
what type of cart it is (no saves, flash, eeprom, or sram). If the cart
has no save possibility, there is an optional memory card attached to
the joypad controller.

Currently there is no driver for any of these.

- Lauri
Jens Axboe Jan. 4, 2021, 4:28 p.m. UTC | #10
On 1/4/21 9:27 AM, Lauri Kasanen wrote:
> On Mon, 4 Jan 2021 08:13:32 -0800
> Keith Busch <kbusch@kernel.org> wrote:
> 
>>> The media is read-only (but not runtime removable).
>>
>> It's been a while, but I could swear we can save state on these carts.
>> If so, it sounds like that must be separate from the media this driver
>> is accessing, so is that capability provided through a different driver?
> 
> Saving uses a separate mechanism, and there are several depending on
> what type of cart it is (no saves, flash, eeprom, or sram). If the cart
> has no save possibility, there is an optional memory card attached to
> the joypad controller.
> 
> Currently there is no driver for any of these.

Maybe it'd make sense to make this runtime configurable through configfs
or similar, requiring boot parameters (and hence a reboot to change
them) seems pretty iffy. Similarly, why isn't it available as a module?
Lauri Kasanen Jan. 4, 2021, 4:42 p.m. UTC | #11
On Mon, 4 Jan 2021 09:28:44 -0700
Jens Axboe <axboe@kernel.dk> wrote:

> On 1/4/21 9:27 AM, Lauri Kasanen wrote:
> > On Mon, 4 Jan 2021 08:13:32 -0800
> > Keith Busch <kbusch@kernel.org> wrote:
> >
> >>> The media is read-only (but not runtime removable).
> >>
> >> It's been a while, but I could swear we can save state on these carts.
> >> If so, it sounds like that must be separate from the media this driver
> >> is accessing, so is that capability provided through a different driver?
> >
> > Saving uses a separate mechanism, and there are several depending on
> > what type of cart it is (no saves, flash, eeprom, or sram). If the cart
> > has no save possibility, there is an optional memory card attached to
> > the joypad controller.
> >
> > Currently there is no driver for any of these.
>
> Maybe it'd make sense to make this runtime configurable through configfs
> or similar, requiring boot parameters (and hence a reboot to change
> them) seems pretty iffy. Similarly, why isn't it available as a module?

You can't change the media without a reboot. What would be the use?
(it's not possible to have multiple carts, if you're wondering)

The lack of a module option is because the target system has 8mb ram.
The kernel is already very bloated, at one time during testing just
enabling the smallest IO governor (mq deadline) caused an OOM during
boot.

Same for configfs, debugfs, etc. No such options to save RAM.

- Lauri
diff mbox series

Patch

diff --git a/drivers/block/Kconfig b/drivers/block/Kconfig
index ecceaaa..924d768 100644
--- a/drivers/block/Kconfig
+++ b/drivers/block/Kconfig
@@ -72,6 +72,12 @@  config AMIGA_Z2RAM
 	  To compile this driver as a module, choose M here: the
 	  module will be called z2ram.

+config N64CART
+	bool "N64 cart support"
+	depends on MACH_NINTENDO64
+	help
+	  Support for the N64 cart.
+
 config CDROM
 	tristate
 	select BLK_SCSI_REQUEST
diff --git a/drivers/block/Makefile b/drivers/block/Makefile
index e1f6311..b9642cf 100644
--- a/drivers/block/Makefile
+++ b/drivers/block/Makefile
@@ -17,6 +17,7 @@  obj-$(CONFIG_PS3_DISK)		+= ps3disk.o
 obj-$(CONFIG_PS3_VRAM)		+= ps3vram.o
 obj-$(CONFIG_ATARI_FLOPPY)	+= ataflop.o
 obj-$(CONFIG_AMIGA_Z2RAM)	+= z2ram.o
+obj-$(CONFIG_N64CART)		+= n64cart.o
 obj-$(CONFIG_BLK_DEV_RAM)	+= brd.o
 obj-$(CONFIG_BLK_DEV_LOOP)	+= loop.o
 obj-$(CONFIG_XILINX_SYSACE)	+= xsysace.o
diff --git a/drivers/block/n64cart.c b/drivers/block/n64cart.c
new file mode 100644
index 0000000..ae2ad4f
--- /dev/null
+++ b/drivers/block/n64cart.c
@@ -0,0 +1,203 @@ 
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Support for the N64 cart.
+ *
+ * Copyright (c) 2020 Lauri Kasanen
+ */
+
+#include <linux/bitops.h>
+#include <linux/blk-mq.h>
+#include <linux/dma-mapping.h>
+#include <linux/init.h>
+#include <linux/major.h>
+#include <linux/module.h>
+#include <linux/spinlock.h>
+
+#include <asm/addrspace.h>
+#include <asm/io.h>
+
+MODULE_AUTHOR("Lauri Kasanen <cand@gmx.com>");
+MODULE_DESCRIPTION("Driver for the N64 cart");
+MODULE_LICENSE("GPL");
+
+#define BUFSIZE (64 * 1024)
+
+static unsigned int start, size;
+static int major;
+static struct request_queue *queue;
+static struct blk_mq_tag_set tag_set;
+static struct gendisk *disk;
+
+static void *buf;
+static dma_addr_t dma_addr;
+
+static DEFINE_SPINLOCK(n64cart_lock);
+
+#define REG_BASE ((u32 *) TO_UNCAC(0x4600000))
+
+#define PI_DRAM_REG 0
+#define PI_CART_REG 1
+#define PI_READ_REG 2
+#define PI_WRITE_REG 3
+#define PI_STATUS_REG 4
+
+#define PI_STATUS_DMA_BUSY (1 << 0)
+#define PI_STATUS_IO_BUSY (1 << 1)
+
+static void n64cart_write_reg(const u8 reg, const u32 value)
+{
+	__raw_writel(value, REG_BASE + reg);
+}
+
+static u32 n64cart_read_reg(const u8 reg)
+{
+	return __raw_readl(REG_BASE + reg);
+}
+
+static void n64cart_wait_dma(void)
+{
+	while (n64cart_read_reg(PI_STATUS_REG) &
+		(PI_STATUS_DMA_BUSY | PI_STATUS_IO_BUSY))
+		;
+}
+
+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;
+
+	bstart += start;
+
+	while (len) {
+		const u32 curlen = len < BUFSIZE ? len : BUFSIZE;
+
+		dma_cache_inv((unsigned long) buf, curlen);
+
+		n64cart_wait_dma();
+
+		barrier();
+		n64cart_write_reg(PI_DRAM_REG, dma_addr);
+		barrier();
+		n64cart_write_reg(PI_CART_REG, (bstart | 0x10000000) & 0x1FFFFFFF);
+		barrier();
+		n64cart_write_reg(PI_WRITE_REG, curlen - 1);
+		barrier();
+
+		n64cart_wait_dma();
+
+		memcpy(dst, buf, curlen);
+
+		len -= curlen;
+		dst += curlen;
+		bstart += curlen;
+	}
+
+	return BLK_STS_OK;
+}
+
+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);
+
+	do {
+		err = get_seg(req);
+	} while (blk_update_request(req, err, blk_rq_cur_bytes(req)));
+
+	spin_unlock_irqrestore(&n64cart_lock, flags);
+	blk_mq_end_request(req, BLK_STS_OK);
+	return BLK_STS_OK;
+}
+
+static const struct blk_mq_ops n64cart_mq_ops = {
+	.queue_rq = n64cart_queue_rq,
+};
+
+static const struct block_device_operations n64cart_fops = {
+	.owner		= THIS_MODULE,
+};
+
+static int __init n64cart_init(void)
+{
+	int err;
+
+	if (!start || !size) {
+		pr_err("n64cart: start and size not specified\n");
+		return -ENODEV;
+	}
+
+	if (size & 4095) {
+		pr_err("n64cart: size must be a multiple of 4K\n");
+		return -ENODEV;
+	}
+
+	major = register_blkdev(0, "n64cart");
+	if (major <= 0) {
+		pr_err("n64cart: unable to get major number\n");
+		return -EBUSY;
+	}
+
+	queue = blk_mq_init_sq_queue(&tag_set, &n64cart_mq_ops, 16,
+				     BLK_MQ_F_SHOULD_MERGE);
+	if (IS_ERR(queue)) {
+		err = PTR_ERR(queue);
+		goto fail_reg;
+	}
+
+	buf = kmalloc(BUFSIZE, GFP_DMA | GFP_KERNEL);
+	if (!buf) {
+		err = -ENOMEM;
+		goto fail_queue;
+	}
+	dma_addr = virt_to_phys(buf);
+
+	disk = alloc_disk(1);
+	if (!disk) {
+		err = -ENOMEM;
+		goto fail_dma;
+	}
+
+	disk->major = major;
+	disk->first_minor = 0;
+	disk->queue = queue;
+	disk->flags = GENHD_FL_NO_PART_SCAN;
+	disk->fops = &n64cart_fops;
+	strcpy(disk->disk_name, "n64cart");
+
+	set_capacity(disk, size / 512);
+
+	blk_queue_flag_set(QUEUE_FLAG_NONROT, queue);
+	blk_queue_physical_block_size(queue, 4096);
+	blk_queue_logical_block_size(queue, 4096);
+
+	add_disk(disk);
+
+	pr_info("n64cart: %u kb disk\n", size / 1024);
+
+	return 0;
+fail_dma:
+	kfree(buf);
+fail_queue:
+	blk_cleanup_queue(queue);
+fail_reg:
+	unregister_blkdev(major, "n64cart");
+	return err;
+}
+
+module_param(start, uint, 0);
+MODULE_PARM_DESC(start, "Start address of the cart block data");
+
+module_param(size, uint, 0);
+MODULE_PARM_DESC(size, "Size of the cart block data, in bytes");
+
+module_init(n64cart_init);