Re: [PATCH 2/2] memstick: add support for legacy memorysticks

Re: [PATCH 2/2] memstick: add support for legacy memorysticks

[Geert Uytterhoeven]

On Wed, Sep 26, 2012 at 11:49 AM, Maxim Levitsky
<maximlevitsky@gmail.com> wrote:
> --- /dev/null
> +++ b/drivers/memstick/core/ms_block.c

> +#define pr_fmt(fmt) DRIVER_NAME ": " fmt
> +
> +#include <linux/module.h>
> +#include <linux/blkdev.h>
> +#include <linux/memstick.h>
> +#include <linux/idr.h>
> +#include <linux/hdreg.h>
> +#include <linux/delay.h>
> +#include <linux/slab.h>
> +#include <linux/random.h>
> +#include <linux/bitmap.h>
> +#include <linux/scatterlist.h>
> +#include <linux/jiffies.h>
> +#include <linux/workqueue.h>
> +#include <linux/mutex.h>
> +#include "ms_block.h"

http://kisskb.ellerman.id.au/kisskb/buildresult/7280352/
arch/m68k/include/asm/hardirq.h:23:20: error: expected ')' before 'DRIVER_NAME'
make[4]: *** [drivers/memstick/core/ms_block.o] Error 1

The reason for this is that pr_fmt() references DRIVER_NAME and is defined
before the first include, while DRIVER_NAME is only defined in ms_block.h,
which is the last included file. If any subsequent include file uses pr_fmt()
(e.g. the call to pr_crit() in arch/m68k/include/asm/hardirq.h), this
causes a build
failure.

I suggest moving the DRIVER_NAME define to ms_block.c.
Cfr. memstick.c and mspro_block.c, who already have their own definition.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 2/2] memstick: add support for legacy memorysticks

[Maxim Levitsky]

On Sat, 2012-09-29 at 19:20 +0200, Geert Uytterhoeven wrote: 
> On Wed, Sep 26, 2012 at 11:49 AM, Maxim Levitsky
> <maximlevitsky@gmail.com> wrote:
> > --- /dev/null
> > +++ b/drivers/memstick/core/ms_block.c
> 
> > +#define pr_fmt(fmt) DRIVER_NAME ": " fmt
> > +
> > +#include <linux/module.h>
> > +#include <linux/blkdev.h>
> > +#include <linux/memstick.h>
> > +#include <linux/idr.h>
> > +#include <linux/hdreg.h>
> > +#include <linux/delay.h>
> > +#include <linux/slab.h>
> > +#include <linux/random.h>
> > +#include <linux/bitmap.h>
> > +#include <linux/scatterlist.h>
> > +#include <linux/jiffies.h>
> > +#include <linux/workqueue.h>
> > +#include <linux/mutex.h>
> > +#include "ms_block.h"
> 
> http://kisskb.ellerman.id.au/kisskb/buildresult/7280352/
> arch/m68k/include/asm/hardirq.h:23:20: error: expected ')' before 'DRIVER_NAME'
> make[4]: *** [drivers/memstick/core/ms_block.o] Error 1
> 
> The reason for this is that pr_fmt() references DRIVER_NAME and is defined
> before the first include, while DRIVER_NAME is only defined in ms_block.h,
> which is the last included file. If any subsequent include file uses pr_fmt()
> (e.g. the call to pr_crit() in arch/m68k/include/asm/hardirq.h), this
> causes a build
> failure.
> 
> I suggest moving the DRIVER_NAME define to ms_block.c.
> Cfr. memstick.c and mspro_block.c, who already have their own definition.
> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds
Completely agree!
I was somewhat busy lately, so I didn't address this yet.
I'll send a patch tomorrow.

Best regards,
Maxim Levitsky

Fixes for ms_block

[Maxim Levitsky]

Hi,

I finally found time to send the fix for the compile issue on m68k
Among with small cosmetic fix to remove unused field that slipped thorough.

Do you know if my driver will be merged in 3.7 or later?
I hope that the compile issue which I should have handled earlier is not to blame.

[PATCH 1/2] memstick: remove unused field from state struct.

[Maxim Levitsky]

Oops, I forgot that I have thet field there already.
Just save memory by not allocating it.

Signed-off-by: Maxim Levitsky <maximlevitsky@gmail.com>
---
 drivers/memstick/core/ms_block.h |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/memstick/core/ms_block.h b/drivers/memstick/core/ms_block.h
index 6b5b83e..a2b2765 100644
--- a/drivers/memstick/core/ms_block.h
+++ b/drivers/memstick/core/ms_block.h
@@ -162,7 +162,6 @@ struct msb_data {
 	struct workqueue_struct		*io_queue;
 	bool				io_queue_stopped;
 	struct work_struct		io_work;
-	struct scatterlist		prealloc_sg[MS_BLOCK_MAX_SEGS+1];
 	bool				card_dead;
 
 	/* Media properties */
@@ -194,7 +193,7 @@ struct msb_data {
 
 	/* Preallocated buffers */
 	unsigned char			*block_buffer;
-	struct scatterlist		sg[MS_BLOCK_MAX_SEGS+1];
+	struct scatterlist		prealloc_sg[MS_BLOCK_MAX_SEGS+1];
 
 
 	/* handler's local data */
-- 
1.7.9.5

[PATCH 2/2] memstick: ms_block: fix complile issue.

[Maxim Levitsky]

As suggested by Geert Uytterhoeven

http://kisskb.ellerman.id.au/kisskb/buildresult/7280352/
arch/m68k/include/asm/hardirq.h:23:20: error: expected ')' before 'DRIVER_NAME'
make[4]: *** [drivers/memstick/core/ms_block.o] Error 1

The reason for this is that pr_fmt() references DRIVER_NAME and is defined
before the first include, while DRIVER_NAME is only defined in ms_block.h,
which is the last included file. If any subsequent include file uses pr_fmt()
(e.g. the call to pr_crit() in arch/m68k/include/asm/hardirq.h), this
causes a build
failure.

I suggest moving the DRIVER_NAME define to ms_block.c.
Cfr. memstick.c and mspro_block.c, who already have their own definition.


CC: Geert Uytterhoeven <geert@linux-m68k.org>

Signed-off-by: Maxim Levitsky <maximlevitsky@gmail.com>
---
 drivers/memstick/core/ms_block.c |    2 +-
 drivers/memstick/core/ms_block.h |    2 --
 2 files changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/memstick/core/ms_block.c b/drivers/memstick/core/ms_block.c
index c815fe5..71e5937 100644
--- a/drivers/memstick/core/ms_block.c
+++ b/drivers/memstick/core/ms_block.c
@@ -11,7 +11,7 @@
  * Copyright (C) 2007 Alex Dubov <oakad@yahoo.com>
  *
  */
-
+#define DRIVER_NAME "ms_block"
 #define pr_fmt(fmt) DRIVER_NAME ": " fmt
 
 #include <linux/module.h>
diff --git a/drivers/memstick/core/ms_block.h b/drivers/memstick/core/ms_block.h
index a2b2765..546b55e 100644
--- a/drivers/memstick/core/ms_block.h
+++ b/drivers/memstick/core/ms_block.h
@@ -277,8 +277,6 @@ static int msb_reset(struct msb_data *msb, bool full);
 static int h_msb_default_bad(struct memstick_dev *card,
 						struct memstick_request **mrq);
 
-#define DRIVER_NAME "ms_block"
-
 #define __dbg(level, format, ...) \
 	do { \
 		if (debug >= level) \
-- 
1.7.9.5