[PATCH 0/9] n64: small cleanups

STALE2033d

13 messages, 4 authors, 2021-02-21 · open the first message on its own page

[PATCH 0/9] n64: small cleanups

From: Chaitanya Kulkarni <hidden>
Date: 2021-01-25 23:34:39

Hi Lauri,

This cleanup series is based on the top of v11 that adds kernel coding
style fixes in the various places without changing the functionality.

Instead of sending a one big patch I've made small patches which are
easier to review, integrate, blame and revert in case of any error.

I've got the reviewed-by tag from the driver owner offline which
is added to the series.

-ck

Chaitanya Kulkarni (9):
  n64: use pr_fmt to avoid duplicate string
  n64: move module info at the end
  n64: move module param at the top
  n64: use enums for reg
  n64: use sector SECTOR_SHIFT instead 512
  n64: remove curly brackets
  n64: cosmetics changes
  n64: cleanup n64cart_probe()
  n64: store dev instance into disk private data

 drivers/block/n64cart.c | 87 ++++++++++++++++++-----------------------
 1 file changed, 38 insertions(+), 49 deletions(-)

-- 
2.22.1

[PATCH 3/9] n64: move module param at the top

From: Chaitanya Kulkarni <hidden>
Date: 2021-01-25 23:35:18

Move module parameters at the top of the file after macro definition &
global variables below macro definitions just like we have for other
modules.

Signed-off-by: Chaitanya Kulkarni <redacted>
Reviewed-by: Lauri Kasanen <redacted>
---
 drivers/block/n64cart.c | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)
diff --git a/drivers/block/n64cart.c b/drivers/block/n64cart.c
index 63090030ed2b..b18f034ee1ad 100644
--- a/drivers/block/n64cart.c
+++ b/drivers/block/n64cart.c
@@ -13,10 +13,6 @@
 #include <linux/module.h>
 #include <linux/platform_device.h>
 
-static unsigned int start, size;
-static u32 __iomem *reg_base;
-static struct device *dev;
-
 #define PI_DRAM_REG		0
 #define PI_CART_REG		1
 #define PI_READ_REG		2
@@ -31,6 +27,17 @@ static struct device *dev;
 
 #define MIN_ALIGNMENT		8
 
+static u32 __iomem *reg_base;
+static struct device *dev;
+
+static unsigned int start;
+module_param(start, uint, 0);
+MODULE_PARM_DESC(start, "Start address of the cart block data");
+
+static unsigned int size;
+module_param(size, uint, 0);
+MODULE_PARM_DESC(size, "Size of the cart block data, in bytes");
+
 static void n64cart_write_reg(const u8 reg, const u32 value)
 {
 	writel(value, reg_base + reg);
@@ -177,12 +184,6 @@ static int __init n64cart_init(void)
 	return platform_driver_probe(&n64cart_driver, n64cart_probe);
 }
 
-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);
 
 MODULE_AUTHOR("Lauri Kasanen <cand@gmx.com>");
-- 
2.22.1

[PATCH 7/9] n64: cosmetics changes

From: Chaitanya Kulkarni <hidden>
Date: 2021-01-25 23:35:18

Make the variable declaration ascending order and initialize the
variables at the time of declaration when possible.

Signed-off-by: Chaitanya Kulkarni <redacted>
Reviewed-by: Lauri Kasanen <redacted>
---
 drivers/block/n64cart.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/block/n64cart.c b/drivers/block/n64cart.c
index 7906b5b2f12e..3bfb010402e3 100644
--- a/drivers/block/n64cart.c
+++ b/drivers/block/n64cart.c
@@ -88,10 +88,8 @@ static bool n64cart_do_bvec(struct device *dev, struct bio_vec *bv, u32 pos)
 static blk_qc_t n64cart_submit_bio(struct bio *bio)
 {
 	struct bio_vec bvec;
-	u32 pos;
 	struct bvec_iter iter;
-
-	pos = bio->bi_iter.bi_sector << SECTOR_SHIFT;
+	u32 pos = bio->bi_iter.bi_sector << SECTOR_SHIFT;
 
 	bio_for_each_segment(bvec, bio, iter) {
 		if (!n64cart_do_bvec(dev, &bvec, pos))
@@ -119,8 +117,8 @@ static const struct block_device_operations n64cart_fops = {
 static int __init n64cart_probe(struct platform_device *pdev)
 {
 	int err;
-	struct request_queue *queue;
 	struct gendisk *disk;
+	struct request_queue *queue;
 
 	if (!start || !size) {
 		pr_err("start or size not specified\n");
-- 
2.22.1

[PATCH 6/9] n64: remove curly brackets

From: Chaitanya Kulkarni <hidden>
Date: 2021-01-25 23:35:18

Remove extra braces for the if which has only single statement.

Signed-off-by: Chaitanya Kulkarni <redacted>
Reviewed-by: Lauri Kasanen <redacted>
---
 drivers/block/n64cart.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/block/n64cart.c b/drivers/block/n64cart.c
index c83a6af5a718..7906b5b2f12e 100644
--- a/drivers/block/n64cart.c
+++ b/drivers/block/n64cart.c
@@ -133,9 +133,8 @@ static int __init n64cart_probe(struct platform_device *pdev)
 	}
 
 	queue = blk_alloc_queue(NUMA_NO_NODE);
-	if (!queue) {
+	if (!queue)
 		return -ENOMEM;
-	}
 
 	reg_base = devm_platform_ioremap_resource(pdev, 0);
 	if (!reg_base) {
-- 
2.22.1

[PATCH 1/9] n64: use pr_fmt to avoid duplicate string

From: Chaitanya Kulkarni <hidden>
Date: 2021-01-25 23:35:24

Instead of repeating the n64cart string all over the module use pr_fmt
macro and remove the duplicate string. Also, replace and with or in the
one of the error message.

Signed-off-by: Chaitanya Kulkarni <redacted>
Reviewed-by: Lauri Kasanen <redacted>
---
 drivers/block/n64cart.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/block/n64cart.c b/drivers/block/n64cart.c
index e76722acba46..8c7c9249071b 100644
--- a/drivers/block/n64cart.c
+++ b/drivers/block/n64cart.c
@@ -5,6 +5,7 @@
  * Copyright (c) 2021 Lauri Kasanen
  */
 
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 #include <linux/bitops.h>
 #include <linux/blkdev.h>
 #include <linux/dma-mapping.h>
@@ -117,12 +118,12 @@ static int __init n64cart_probe(struct platform_device *pdev)
 	struct gendisk *disk;
 
 	if (!start || !size) {
-		pr_err("n64cart: start and size not specified\n");
+		pr_err("start or size not specified\n");
 		return -ENODEV;
 	}
 
 	if (size & 4095) {
-		pr_err("n64cart: size must be a multiple of 4K\n");
+		pr_err("size must be a multiple of 4K\n");
 		return -ENODEV;
 	}
 
-- 
2.22.1

[PATCH 2/9] n64: move module info at the end

From: Chaitanya Kulkarni <hidden>
Date: 2021-01-25 23:36:28

Move the module auth, description, and license at the end of the file
just like what we have for the other modules.

Signed-off-by: Chaitanya Kulkarni <redacted>
Reviewed-by: Lauri Kasanen <redacted>
---
 drivers/block/n64cart.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/block/n64cart.c b/drivers/block/n64cart.c
index 8c7c9249071b..63090030ed2b 100644
--- a/drivers/block/n64cart.c
+++ b/drivers/block/n64cart.c
@@ -13,10 +13,6 @@
 #include <linux/module.h>
 #include <linux/platform_device.h>
 
-MODULE_AUTHOR("Lauri Kasanen <cand@gmx.com>");
-MODULE_DESCRIPTION("Driver for the N64 cart");
-MODULE_LICENSE("GPL");
-
 static unsigned int start, size;
 static u32 __iomem *reg_base;
 static struct device *dev;
@@ -188,3 +184,7 @@ module_param(size, uint, 0);
 MODULE_PARM_DESC(size, "Size of the cart block data, in bytes");
 
 module_init(n64cart_init);
+
+MODULE_AUTHOR("Lauri Kasanen <cand@gmx.com>");
+MODULE_DESCRIPTION("Driver for the N64 cart");
+MODULE_LICENSE("GPL");
-- 
2.22.1

[PATCH 8/9] n64: cleanup n64cart_probe()

From: Chaitanya Kulkarni <hidden>
Date: 2021-01-25 23:36:29

The goto label fail_queue is needed to cleanup the queue allocation
when devm_platform_ioremap_resource() or alloc_disk() fails, either of
these two functions are not dependent on the queue variable which is
allocated prior to these calls.

Allocate the queue variable after successful alloc_disk(). Return
error directly when devm_platform_ioremap_resource() or alloc_disk()
fail. Remove fail_queue label and a call to the blk_cleanup_queue().

Direct return from these two functions allows us to remove the local
variable err and allocating queue after alloc_disk() allows us to
remove the local variable queue so we use disk->queue directly.

Signed-off-by: Chaitanya Kulkarni <redacted>
Reviewed-by: Lauri Kasanen <redacted>
---
 drivers/block/n64cart.c | 33 +++++++++++----------------------
 1 file changed, 11 insertions(+), 22 deletions(-)
diff --git a/drivers/block/n64cart.c b/drivers/block/n64cart.c
index 3bfb010402e3..43482d158640 100644
--- a/drivers/block/n64cart.c
+++ b/drivers/block/n64cart.c
@@ -116,9 +116,7 @@ static const struct block_device_operations n64cart_fops = {
  */
 static int __init n64cart_probe(struct platform_device *pdev)
 {
-	int err;
 	struct gendisk *disk;
-	struct request_queue *queue;
 
 	if (!start || !size) {
 		pr_err("start or size not specified\n");
@@ -130,26 +128,21 @@ static int __init n64cart_probe(struct platform_device *pdev)
 		return -ENODEV;
 	}
 
-	queue = blk_alloc_queue(NUMA_NO_NODE);
-	if (!queue)
-		return -ENOMEM;
-
 	reg_base = devm_platform_ioremap_resource(pdev, 0);
-	if (!reg_base) {
-		err = -EINVAL;
-		goto fail_queue;
-	}
+	if (!reg_base)
+		return -EINVAL;
 
 	disk = alloc_disk(0);
-	if (!disk) {
-		err = -ENOMEM;
-		goto fail_queue;
-	}
+	if (!disk)
+		return -ENOMEM;
+
+	disk->queue = blk_alloc_queue(NUMA_NO_NODE);
+	if (!disk->queue)
+		return -ENOMEM;
 
 	dev = &pdev->dev;
 
 	disk->first_minor = 0;
-	disk->queue = queue;
 	disk->flags = GENHD_FL_NO_PART_SCAN | GENHD_FL_EXT_DEVT;
 	disk->fops = &n64cart_fops;
 	strcpy(disk->disk_name, "n64cart");
@@ -157,19 +150,15 @@ static int __init n64cart_probe(struct platform_device *pdev)
 	set_capacity(disk, size >> SECTOR_SHIFT);
 	set_disk_ro(disk, 1);
 
-	blk_queue_flag_set(QUEUE_FLAG_NONROT, queue);
-	blk_queue_physical_block_size(queue, 4096);
-	blk_queue_logical_block_size(queue, 4096);
+	blk_queue_flag_set(QUEUE_FLAG_NONROT, disk->queue);
+	blk_queue_physical_block_size(disk->queue, 4096);
+	blk_queue_logical_block_size(disk->queue, 4096);
 
 	add_disk(disk);
 
 	pr_info("n64cart: %u kb disk\n", size / 1024);
 
 	return 0;
-fail_queue:
-	blk_cleanup_queue(queue);
-
-	return err;
 }
 
 static struct platform_driver n64cart_driver = {
-- 
2.22.1

[PATCH 9/9] n64: store dev instance into disk private data

From: Chaitanya Kulkarni <hidden>
Date: 2021-01-25 23:37:00

The device instance is declared globally. Remove global variable & use
the disk->private_data to store the device instance in the
n64cart_probe() and get the same instance from bio->bi_disk->private
data in n64cart_submit_bio.

Signed-off-by: Chaitanya Kulkarni <redacted>
Reviewed-by: Lauri Kasanen <redacted>
---
 drivers/block/n64cart.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/block/n64cart.c b/drivers/block/n64cart.c
index 43482d158640..47bdf324e962 100644
--- a/drivers/block/n64cart.c
+++ b/drivers/block/n64cart.c
@@ -30,7 +30,6 @@ enum {
 #define MIN_ALIGNMENT		8
 
 static u32 __iomem *reg_base;
-static struct device *dev;
 
 static unsigned int start;
 module_param(start, uint, 0);
@@ -89,6 +88,7 @@ static blk_qc_t n64cart_submit_bio(struct bio *bio)
 {
 	struct bio_vec bvec;
 	struct bvec_iter iter;
+	struct device *dev = bio->bi_disk->private_data;
 	u32 pos = bio->bi_iter.bi_sector << SECTOR_SHIFT;
 
 	bio_for_each_segment(bvec, bio, iter) {
@@ -140,11 +140,10 @@ static int __init n64cart_probe(struct platform_device *pdev)
 	if (!disk->queue)
 		return -ENOMEM;
 
-	dev = &pdev->dev;
-
 	disk->first_minor = 0;
 	disk->flags = GENHD_FL_NO_PART_SCAN | GENHD_FL_EXT_DEVT;
 	disk->fops = &n64cart_fops;
+	disk->private_data = &pdev->dev;
 	strcpy(disk->disk_name, "n64cart");
 
 	set_capacity(disk, size >> SECTOR_SHIFT);
-- 
2.22.1

[PATCH 4/9] n64: use enums for reg

From: Chaitanya Kulkarni <hidden>
Date: 2021-01-25 23:37:36

Macros tend to be not type-safe. Use enum for register definitions.

Signed-off-by: Chaitanya Kulkarni <redacted>
Reviewed-by: Lauri Kasanen <redacted>
---
 drivers/block/n64cart.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/drivers/block/n64cart.c b/drivers/block/n64cart.c
index b18f034ee1ad..620f9e080d5d 100644
--- a/drivers/block/n64cart.c
+++ b/drivers/block/n64cart.c
@@ -13,11 +13,13 @@
 #include <linux/module.h>
 #include <linux/platform_device.h>
 
-#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
+enum {
+	PI_DRAM_REG = 0,
+	PI_CART_REG,
+	PI_READ_REG,
+	PI_WRITE_REG,
+	PI_STATUS_REG,
+};
 
 #define PI_STATUS_DMA_BUSY	(1 << 0)
 #define PI_STATUS_IO_BUSY	(1 << 1)
-- 
2.22.1

[PATCH 5/9] n64: use sector SECTOR_SHIFT instead 512

From: Chaitanya Kulkarni <hidden>
Date: 2021-01-25 23:37:36

Instead of using magic numbers use SECTOR_SHIFT to get the number of
sectors from the size.

Signed-off-by: Chaitanya Kulkarni <redacted>
Reviewed-by: Lauri Kasanen <redacted>
---
 drivers/block/n64cart.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/block/n64cart.c b/drivers/block/n64cart.c
index 620f9e080d5d..c83a6af5a718 100644
--- a/drivers/block/n64cart.c
+++ b/drivers/block/n64cart.c
@@ -157,7 +157,7 @@ static int __init n64cart_probe(struct platform_device *pdev)
 	disk->fops = &n64cart_fops;
 	strcpy(disk->disk_name, "n64cart");
 
-	set_capacity(disk, size / 512);
+	set_capacity(disk, size >> SECTOR_SHIFT);
 	set_disk_ro(disk, 1);
 
 	blk_queue_flag_set(QUEUE_FLAG_NONROT, queue);
-- 
2.22.1

Re: [PATCH 0/9] n64: small cleanups

From: Lauri Kasanen <hidden>
Date: 2021-01-26 17:14:43

On Mon, 25 Jan 2021 15:32:34 -0800
Chaitanya Kulkarni [off-list ref] wrote:
I've got the reviewed-by tag from the driver owner offline which
is added to the series.
I confirm this, he sent it privately first. I approve the series.

- Lauri

Re: [PATCH 7/9] n64: cosmetics changes

From: Sergei Shtylyov <hidden>
Date: 2021-01-26 18:52:08

Hello!

On 26.01.2021 2:32, Chaitanya Kulkarni wrote:
Make the variable declaration ascending order and initialize the
    Usually the vars are declared in descending order, no?
variables at the time of declaration when possible.

Signed-off-by: Chaitanya Kulkarni <redacted>
Reviewed-by: Lauri Kasanen <redacted>
[...]

MBR, Sergei

Re: [PATCH 0/9] n64: small cleanups

From: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Date: 2021-02-21 22:59:19

On Mon, Jan 25, 2021 at 03:32:34PM -0800, Chaitanya Kulkarni wrote:
Hi Lauri,

This cleanup series is based on the top of v11 that adds kernel coding
style fixes in the various places without changing the functionality.

Instead of sending a one big patch I've made small patches which are
easier to review, integrate, blame and revert in case of any error.

I've got the reviewed-by tag from the driver owner offline which
is added to the series.

-ck

Chaitanya Kulkarni (9):
  n64: use pr_fmt to avoid duplicate string
  n64: move module info at the end
  n64: move module param at the top
  n64: use enums for reg
  n64: use sector SECTOR_SHIFT instead 512
  n64: remove curly brackets
  n64: cosmetics changes
  n64: cleanup n64cart_probe()
  n64: store dev instance into disk private data

 drivers/block/n64cart.c | 87 ++++++++++++++++++-----------------------
 1 file changed, 38 insertions(+), 49 deletions(-)
series applied to mips-next.

Thomas.

-- 
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea.                                                [ RFC1925, 2.3 ]
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help