diff mbox

ARM: Fix rd_size declaration

Message ID 20170417231003.7178-1-bart.vanassche@sandisk.com
State New, archived
Headers show

Commit Message

Bart Van Assche April 17, 2017, 11:10 p.m. UTC
The global variable 'rd_size' is declared as 'int' in source file
arch/arm/kernel/atags_parse.c and as 'unsigned long' in
drivers/block/brd.c. Fix this inconsistency.

Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
Cc: Russell King <linux@armlinux.org.uk>
Cc: Jens Axboe <axboe@kernel.dk>
Cc: Jan Kara <jack@suse.cz>
Cc: <yanaijie@huawei.com>
Cc: <zhaohongjiang@huawei.com>
Cc: <miaoxie@huawei.com>
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-block@vger.kernel.org
---
 arch/arm/kernel/atags_parse.c | 4 ++--
 drivers/block/brd.c           | 1 +
 include/linux/brd.h           | 1 +
 3 files changed, 4 insertions(+), 2 deletions(-)
 create mode 100644 include/linux/brd.h

Comments

Johannes Thumshirn April 18, 2017, 7:35 a.m. UTC | #1
On Mon, Apr 17, 2017 at 04:10:03PM -0700, Bart Van Assche wrote:
> The global variable 'rd_size' is declared as 'int' in source file
> arch/arm/kernel/atags_parse.c and as 'unsigned long' in
> drivers/block/brd.c. Fix this inconsistency.
> 
> Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
> Cc: Russell King <linux@armlinux.org.uk>
> Cc: Jens Axboe <axboe@kernel.dk>
> Cc: Jan Kara <jack@suse.cz>
> Cc: <yanaijie@huawei.com>
> Cc: <zhaohongjiang@huawei.com>
> Cc: <miaoxie@huawei.com>
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-block@vger.kernel.org
> ---
> diff --git a/include/linux/brd.h b/include/linux/brd.h
> new file mode 100644
> index 000000000000..dbb0f92fefc8
> --- /dev/null
> +++ b/include/linux/brd.h
> @@ -0,0 +1 @@
> +extern unsigned long rd_size;

Small nit, can you add an include guard here as well?

Thanks,
	Johannes
Bart Van Assche April 18, 2017, 2:07 p.m. UTC | #2
On Tue, 2017-04-18 at 09:35 +0200, Johannes Thumshirn wrote:
> On Mon, Apr 17, 2017 at 04:10:03PM -0700, Bart Van Assche wrote:

> > The global variable 'rd_size' is declared as 'int' in source file

> > arch/arm/kernel/atags_parse.c and as 'unsigned long' in

> > drivers/block/brd.c. Fix this inconsistency.

> > 

> > Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>

> > Cc: Russell King <linux@armlinux.org.uk>

> > Cc: Jens Axboe <axboe@kernel.dk>

> > Cc: Jan Kara <jack@suse.cz>

> > Cc: <yanaijie@huawei.com>

> > Cc: <zhaohongjiang@huawei.com>

> > Cc: <miaoxie@huawei.com>

> > Cc: linux-arm-kernel@lists.infradead.org

> > Cc: linux-block@vger.kernel.org

> > ---

> > diff --git a/include/linux/brd.h b/include/linux/brd.h

> > new file mode 100644

> > index 000000000000..dbb0f92fefc8

> > --- /dev/null

> > +++ b/include/linux/brd.h

> > @@ -0,0 +1 @@

> > +extern unsigned long rd_size;

> 

> Small nit, can you add an include guard here as well?


Hello Johannes,

Thanks for the review. But are you aware that with the current content an
include guard is overkill because it is safe to evaluate the "extern unsigned
long rd_size" declaration multiple times?

Bart.
Johannes Thumshirn April 18, 2017, 2:10 p.m. UTC | #3
On Tue, Apr 18, 2017 at 02:07:53PM +0000, Bart Van Assche wrote:
> Hello Johannes,
> 
> Thanks for the review. But are you aware that with the current content an
> include guard is overkill because it is safe to evaluate the "extern unsigned
> long rd_size" declaration multiple times?

Yes I am. But once someone does changes to this header and forgets the include
guard as well it may cause errors.

I am aware that this is rather cosmetic than needed, that's why I declared it
as a nit.

Byte,
	Johannes
Bart Van Assche April 26, 2017, 8:51 p.m. UTC | #4
On Mon, 2017-04-17 at 16:10 -0700, Bart Van Assche wrote:
> The global variable 'rd_size' is declared as 'int' in source file
> arch/arm/kernel/atags_parse.c and as 'unsigned long' in
> drivers/block/brd.c. Fix this inconsistency.
> [ ... ]

Hello Russell,

Have I sent this patch to the right maintainer?

Thanks,

Bart.
Russell King - ARM Linux admin May 3, 2017, 7:25 p.m. UTC | #5
On Wed, Apr 26, 2017 at 08:51:35PM +0000, Bart Van Assche wrote:
> On Mon, 2017-04-17 at 16:10 -0700, Bart Van Assche wrote:
> > The global variable 'rd_size' is declared as 'int' in source file
> > arch/arm/kernel/atags_parse.c and as 'unsigned long' in
> > drivers/block/brd.c. Fix this inconsistency.
> > [ ... ]
> 
> Hello Russell,
> 
> Have I sent this patch to the right maintainer?

There were comments on the patch which seemed to be unresolved.

I, too, don't like the idea of a single-line header file.  I'm also
wondering what the right solution here is - we're the only architecture
in the modern kernel that writes to rd_size, no one else does that.
I haven't been able to dig into the history to find out whether other
architectures used to, or what.

However, it would be nice if rd_size could go with some other related
declarations somewhere (if there are any.)
Bart Van Assche May 3, 2017, 7:38 p.m. UTC | #6
On Wed, 2017-05-03 at 20:25 +0100, Russell King - ARM Linux wrote:
> There were comments on the patch which seemed to be unresolved.

Do you mean the header guard? That's easy to resolve.

> However, it would be nice if rd_size could go with some other related
> declarations somewhere (if there are any.)

Sorry but I don't know about any other declarations that should be moved
into a brd header file.

Bart.
diff mbox

Patch

diff --git a/arch/arm/kernel/atags_parse.c b/arch/arm/kernel/atags_parse.c
index 68c6ae0b9e4c..85cb659e622a 100644
--- a/arch/arm/kernel/atags_parse.c
+++ b/arch/arm/kernel/atags_parse.c
@@ -23,6 +23,8 @@ 
 #include <linux/root_dev.h>
 #include <linux/screen_info.h>
 #include <linux/memblock.h>
+#include <linux/brd.h>		/* rd_size */
+#include <linux/initrd.h>	/* rd_image_start, rd_prompt, rd_doload */
 
 #include <asm/setup.h>
 #include <asm/system_info.h>
@@ -91,8 +93,6 @@  __tagtable(ATAG_VIDEOTEXT, parse_tag_videotext);
 #ifdef CONFIG_BLK_DEV_RAM
 static int __init parse_tag_ramdisk(const struct tag *tag)
 {
-	extern int rd_size, rd_image_start, rd_prompt, rd_doload;
-
 	rd_image_start = tag->u.ramdisk.start;
 	rd_doload = (tag->u.ramdisk.flags & 1) == 0;
 	rd_prompt = (tag->u.ramdisk.flags & 2) == 0;
diff --git a/drivers/block/brd.c b/drivers/block/brd.c
index 3adc32a3153b..6d4bd38a9b7c 100644
--- a/drivers/block/brd.c
+++ b/drivers/block/brd.c
@@ -24,6 +24,7 @@ 
 #endif
 
 #include <linux/uaccess.h>
+#include <linux/brd.h>
 
 #define SECTOR_SHIFT		9
 #define PAGE_SECTORS_SHIFT	(PAGE_SHIFT - SECTOR_SHIFT)
diff --git a/include/linux/brd.h b/include/linux/brd.h
new file mode 100644
index 000000000000..dbb0f92fefc8
--- /dev/null
+++ b/include/linux/brd.h
@@ -0,0 +1 @@ 
+extern unsigned long rd_size;