[PATCH 0/4] Cleanups for the mini2440 board

[PATCH 0/4] Cleanups for the mini2440 board

[Wolfram Sang]

When compiling 3.0-rc6 for a mini2440, I noticed a few build warnings (some
seem to have been there for years!). Give the platform a little love...

Regards,

   Wolfram

Wolfram Sang (4):
  arm: plat-s3c24xx: dma: don't use uninitialized variable
  arm: plat-s3c24xx: dma: drop return codes in void function
  arm: mach-s3c2440: dma: fix section mismatch
  arm: mach-s3c2440: mini2440: fix section mismatch

 arch/arm/mach-s3c2440/dma.c           |    2 +-
 arch/arm/mach-s3c2440/mach-mini2440.c |    2 +-
 arch/arm/plat-s3c24xx/dma.c           |   12 +++---------
 3 files changed, 5 insertions(+), 11 deletions(-)

-- 
1.7.5.4

[PATCH 1/4] arm: plat-s3c24xx: dma: don't use uninitialized variable

[Wolfram Sang]

Commit 8970ef47 (S3C24XX: Remove hardware specific registers from DMA
calls) removed the parameter dcon in s3c2410_dma_config() and calculates
it on its own. So the debug-output for the old parameter can go, too.
Fixes:

arch/arm/plat-s3c24xx/dma.c: In function 's3c2410_dma_config':
arch/arm/plat-s3c24xx/dma.c:1030:2: warning: 'dcon' is used uninitialized in this function

Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>
Cc: Ben Dooks <ben-linux@fluff.org>
Cc: Kukjin Kim <kgene.kim@samsung.com>
---
 arch/arm/plat-s3c24xx/dma.c |    8 ++------
 1 files changed, 2 insertions(+), 6 deletions(-)

diff --git a/arch/arm/plat-s3c24xx/dma.c b/arch/arm/plat-s3c24xx/dma.c
index 2abf966..a5b3684 100644
--- a/arch/arm/plat-s3c24xx/dma.c
+++ b/arch/arm/plat-s3c24xx/dma.c
@@ -1027,17 +1027,13 @@ int s3c2410_dma_config(unsigned int channel,
 	struct s3c2410_dma_chan *chan = s3c_dma_lookup_channel(channel);
 	unsigned int dcon;
 
-	pr_debug("%s: chan=%d, xfer_unit=%d, dcon=%08x\n",
-		 __func__, channel, xferunit, dcon);
+	pr_debug("%s: chan=%d, xfer_unit=%d\n", __func__, channel, xferunit);
 
 	if (chan == NULL)
 		return -EINVAL;
 
-	pr_debug("%s: Initial dcon is %08x\n", __func__, dcon);
-
 	dcon = chan->dcon & dma_sel.dcon_mask;
-
-	pr_debug("%s: New dcon is %08x\n", __func__, dcon);
+	pr_debug("%s: dcon is %08x\n", __func__, dcon);
 
 	switch (chan->req_ch) {
 	case DMACH_I2S_IN:
-- 
1.7.5.4

[PATCH 1/4] arm: plat-s3c24xx: dma: don't use uninitialized variable

[Kukjin Kim]

Wolfram Sang wrote:
> 
> Commit 8970ef47 (S3C24XX: Remove hardware specific registers from DMA
> calls) removed the parameter dcon in s3c2410_dma_config() and calculates
> it on its own. So the debug-output for the old parameter can go, too.
> Fixes:
> 
> arch/arm/plat-s3c24xx/dma.c: In function 's3c2410_dma_config':
> arch/arm/plat-s3c24xx/dma.c:1030:2: warning: 'dcon' is used uninitialized
in this
> function
> 
> Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>
> Cc: Ben Dooks <ben-linux@fluff.org>
> Cc: Kukjin Kim <kgene.kim@samsung.com>
> ---
>  arch/arm/plat-s3c24xx/dma.c |    8 ++------
>  1 files changed, 2 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/arm/plat-s3c24xx/dma.c b/arch/arm/plat-s3c24xx/dma.c
> index 2abf966..a5b3684 100644
> --- a/arch/arm/plat-s3c24xx/dma.c
> +++ b/arch/arm/plat-s3c24xx/dma.c
> @@ -1027,17 +1027,13 @@ int s3c2410_dma_config(unsigned int channel,
>  	struct s3c2410_dma_chan *chan = s3c_dma_lookup_channel(channel);
>  	unsigned int dcon;
> 
> -	pr_debug("%s: chan=%d, xfer_unit=%d, dcon=%08x\n",
> -		 __func__, channel, xferunit, dcon);
> +	pr_debug("%s: chan=%d, xfer_unit=%d\n", __func__, channel,
xferunit);
> 
>  	if (chan == NULL)
>  		return -EINVAL;
> 
> -	pr_debug("%s: Initial dcon is %08x\n", __func__, dcon);
> -
>  	dcon = chan->dcon & dma_sel.dcon_mask;
> -
> -	pr_debug("%s: New dcon is %08x\n", __func__, dcon);
> +	pr_debug("%s: dcon is %08x\n", __func__, dcon);
> 
>  	switch (chan->req_ch) {
>  	case DMACH_I2S_IN:
> --
> 1.7.5.4

OK, will apply.
Thanks.

Best regards,
Kgene.
--
Kukjin Kim <kgene.kim@samsung.com>, Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.

[PATCH 2/4] arm: plat-s3c24xx: dma: drop return codes in void function

[Wolfram Sang]

Commit bb072c3c (ARM / Samsung: Use struct syscore_ops for "core" power
management) turned s3c2410_dma_resume_chan() from int to void. So, drop
the actual return values, too. Fixes:

arch/arm/plat-s3c24xx/dma.c: In function 's3c2410_dma_resume_chan':
arch/arm/plat-s3c24xx/dma.c:1238:3: warning: 'return' with a value, in function returning void
arch/arm/plat-s3c24xx/dma.c:1250:2: warning: 'return' with a value, in function returning void

Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>
Cc: Rafael J. Wysocki <rjw@sisk.pl>
Cc: Kukjin Kim <kgene.kim@samsung.com>
---
 arch/arm/plat-s3c24xx/dma.c |    4 +---
 1 files changed, 1 insertions(+), 3 deletions(-)

diff --git a/arch/arm/plat-s3c24xx/dma.c b/arch/arm/plat-s3c24xx/dma.c
index a5b3684..a79a8cc 100644
--- a/arch/arm/plat-s3c24xx/dma.c
+++ b/arch/arm/plat-s3c24xx/dma.c
@@ -1231,7 +1231,7 @@ static void s3c2410_dma_resume_chan(struct s3c2410_dma_chan *cp)
 	/* restore channel's hardware configuration */
 
 	if (!cp->in_use)
-		return 0;
+		return;
 
 	printk(KERN_INFO "dma%d: restoring configuration\n", cp->number);
 
@@ -1242,8 +1242,6 @@ static void s3c2410_dma_resume_chan(struct s3c2410_dma_chan *cp)
 
 	if (cp->map != NULL)
 		dma_sel.select(cp, cp->map);
-
-	return 0;
 }
 
 static void s3c2410_dma_resume(void)
-- 
1.7.5.4

[PATCH 2/4] arm: plat-s3c24xx: dma: drop return codes in void function

[Rafael J. Wysocki]

On Thursday, July 07, 2011, Wolfram Sang wrote:
> Commit bb072c3c (ARM / Samsung: Use struct syscore_ops for "core" power
> management) turned s3c2410_dma_resume_chan() from int to void. So, drop
> the actual return values, too. Fixes:
> 
> arch/arm/plat-s3c24xx/dma.c: In function 's3c2410_dma_resume_chan':
> arch/arm/plat-s3c24xx/dma.c:1238:3: warning: 'return' with a value, in function returning void
> arch/arm/plat-s3c24xx/dma.c:1250:2: warning: 'return' with a value, in function returning void
> 
> Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>

Acked-by: Rafael J. Wysocki <rjw@sisk.pl>

> Cc: Kukjin Kim <kgene.kim@samsung.com>
> ---
>  arch/arm/plat-s3c24xx/dma.c |    4 +---
>  1 files changed, 1 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm/plat-s3c24xx/dma.c b/arch/arm/plat-s3c24xx/dma.c
> index a5b3684..a79a8cc 100644
> --- a/arch/arm/plat-s3c24xx/dma.c
> +++ b/arch/arm/plat-s3c24xx/dma.c
> @@ -1231,7 +1231,7 @@ static void s3c2410_dma_resume_chan(struct s3c2410_dma_chan *cp)
>  	/* restore channel's hardware configuration */
>  
>  	if (!cp->in_use)
> -		return 0;
> +		return;
>  
>  	printk(KERN_INFO "dma%d: restoring configuration\n", cp->number);
>  
> @@ -1242,8 +1242,6 @@ static void s3c2410_dma_resume_chan(struct s3c2410_dma_chan *cp)
>  
>  	if (cp->map != NULL)
>  		dma_sel.select(cp, cp->map);
> -
> -	return 0;
>  }
>  
>  static void s3c2410_dma_resume(void)
> 

[PATCH 2/4] arm: plat-s3c24xx: dma: drop return codes in void function

[Kukjin Kim]

Rafael J. Wysocki wrote:
> 
> On Thursday, July 07, 2011, Wolfram Sang wrote:
> > Commit bb072c3c (ARM / Samsung: Use struct syscore_ops for "core" power
> > management) turned s3c2410_dma_resume_chan() from int to void. So, drop
> > the actual return values, too. Fixes:
> >
> > arch/arm/plat-s3c24xx/dma.c: In function 's3c2410_dma_resume_chan':
> > arch/arm/plat-s3c24xx/dma.c:1238:3: warning: 'return' with a value, in
function
> returning void
> > arch/arm/plat-s3c24xx/dma.c:1250:2: warning: 'return' with a value, in
function
> returning void
> >
> > Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>
> 
> Acked-by: Rafael J. Wysocki <rjw@sisk.pl>
> 

Will apply this into -fix tree for 3.0.
Thanks.

Best regards,
Kgene.
--
Kukjin Kim <kgene.kim@samsung.com>, Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.


> > Cc: Kukjin Kim <kgene.kim@samsung.com>
> > ---
> >  arch/arm/plat-s3c24xx/dma.c |    4 +---
> >  1 files changed, 1 insertions(+), 3 deletions(-)
> >
> > diff --git a/arch/arm/plat-s3c24xx/dma.c b/arch/arm/plat-s3c24xx/dma.c
> > index a5b3684..a79a8cc 100644
> > --- a/arch/arm/plat-s3c24xx/dma.c
> > +++ b/arch/arm/plat-s3c24xx/dma.c
> > @@ -1231,7 +1231,7 @@ static void s3c2410_dma_resume_chan(struct
> s3c2410_dma_chan *cp)
> >  	/* restore channel's hardware configuration */
> >
> >  	if (!cp->in_use)
> > -		return 0;
> > +		return;
> >
> >  	printk(KERN_INFO "dma%d: restoring configuration\n", cp->number);
> >
> > @@ -1242,8 +1242,6 @@ static void s3c2410_dma_resume_chan(struct
> s3c2410_dma_chan *cp)
> >
> >  	if (cp->map != NULL)
> >  		dma_sel.select(cp, cp->map);
> > -
> > -	return 0;
> >  }
> >
> >  static void s3c2410_dma_resume(void)
> >

[PATCH 3/4] arm: mach-s3c2440: dma: fix section mismatch

[Wolfram Sang]

If s3c2440_dma_add() and s3c2440_dma_init() are in __init, then the
struct s3c2440_dma_driver they are using should be __initdata, too.

Fixes:

WARNING: vmlinux.o(.data+0x14ac): Section mismatch in reference from the variable s3c2440_dma_driver to the function .init.text:s3c2440_dma_add()
The variable s3c2440_dma_driver references the function __init s3c2440_dma_add()

Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>
Cc: Ben Dooks <ben-linux@fluff.org>
Cc: Kukjin Kim <kgene.kim@samsung.com>
---
 arch/arm/mach-s3c2440/dma.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-s3c2440/dma.c b/arch/arm/mach-s3c2440/dma.c
index 3b0529f..f417959 100644
--- a/arch/arm/mach-s3c2440/dma.c
+++ b/arch/arm/mach-s3c2440/dma.c
@@ -198,7 +198,7 @@ static int __init s3c2440_dma_add(struct sys_device *sysdev)
 	return s3c24xx_dma_init_map(&s3c2440_dma_sel);
 }
 
-static struct sysdev_driver s3c2440_dma_driver = {
+static struct sysdev_driver __initdata s3c2440_dma_driver = {
 	.add	= s3c2440_dma_add,
 };
 
-- 
1.7.5.4

[PATCH 3/4] arm: mach-s3c2440: dma: fix section mismatch

[Russell King - ARM Linux]

On Thu, Jul 07, 2011 at 12:18:38PM +0200, Wolfram Sang wrote:
> If s3c2440_dma_add() and s3c2440_dma_init() are in __init, then the
> struct s3c2440_dma_driver they are using should be __initdata, too.
> 
> Fixes:
> 
> WARNING: vmlinux.o(.data+0x14ac): Section mismatch in reference from the variable s3c2440_dma_driver to the function .init.text:s3c2440_dma_add()
> The variable s3c2440_dma_driver references the function __init s3c2440_dma_add()

No, this is soo wrong (and illustrates why its important to check
before adding __init stuff.)

> diff --git a/arch/arm/mach-s3c2440/dma.c b/arch/arm/mach-s3c2440/dma.c
> index 3b0529f..f417959 100644
> --- a/arch/arm/mach-s3c2440/dma.c
> +++ b/arch/arm/mach-s3c2440/dma.c
> @@ -198,7 +198,7 @@ static int __init s3c2440_dma_add(struct sys_device *sysdev)
>  	return s3c24xx_dma_init_map(&s3c2440_dma_sel);
>  }
>  
> -static struct sysdev_driver s3c2440_dma_driver = {
> +static struct sysdev_driver __initdata s3c2440_dma_driver = {
>  	.add	= s3c2440_dma_add,
>  };

The code does this:

static int __init s3c2440_dma_init(void)
{
        return sysdev_driver_register(&s3c2440_sysclass, &s3c2440_dma_driver);
}

So, this registers the s3c2440_dma_driver structure.  Internally,
sysdev_driver_register() does this:

                list_add_tail(&drv->entry, &cls->drivers);

which adds s3c2440_dma_driver to a list of sysdev drivers.  Whenever
a sysdev is added, the code does this:

                /* Notify class auxiliary drivers */
                list_for_each_entry(drv, &cls->drivers, entry) {
                        if (drv->add)
                                drv->add(sysdev);
                }

What do you think will happen when this list is walked, but your
s3c2440_dma_driver structure has been discarded with the init section
and replaced with random data?

[PATCH 3/4] arm: mach-s3c2440: dma: fix section mismatch

[Wolfram Sang]

> What do you think will happen when this list is walked, but your
> s3c2440_dma_driver structure has been discarded with the init section
> and replaced with random data?

OOPS. That was not enough love, mea culpa.

Thanks!

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20110707/e15a53cb/attachment.sig>

[PATCH 4/4] arm: mach-s3c2440: mini2440: fix section mismatch

[Wolfram Sang]

If mini2440_init() is in __init, mini2440_parse_features() should also
be in __init. Fixes:

(.text+0x9adc): Section mismatch in reference from the function mini2440_parse_features.clone.0() to the (unknown reference) .init.data:(unknown)
The function mini2440_parse_features.clone.0() references the (unknown reference) __initdata (unknown).

Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>
Cc: Michel Pollet <buserror@gmail.com>
Cc: Kukjin Kim <kgene.kim@samsung.com>
---
 arch/arm/mach-s3c2440/mach-mini2440.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-s3c2440/mach-mini2440.c b/arch/arm/mach-s3c2440/mach-mini2440.c
index dd3120d..fc2dc0b 100644
--- a/arch/arm/mach-s3c2440/mach-mini2440.c
+++ b/arch/arm/mach-s3c2440/mach-mini2440.c
@@ -552,7 +552,7 @@ struct mini2440_features_t {
 	struct platform_device *optional[8];
 };
 
-static void mini2440_parse_features(
+static void __init mini2440_parse_features(
 		struct mini2440_features_t * features,
 		const char * features_str )
 {
-- 
1.7.5.4

[PATCH 4/4] arm: mach-s3c2440: mini2440: fix section mismatch

[Kukjin Kim]

Wolfram Sang wrote:
> 
> If mini2440_init() is in __init, mini2440_parse_features() should also
> be in __init. Fixes:
> 
> (.text+0x9adc): Section mismatch in reference from the function
> mini2440_parse_features.clone.0() to the (unknown reference)
.init.data:(unknown)
> The function mini2440_parse_features.clone.0() references the (unknown
reference)
> __initdata (unknown).

But I couldn't find above "Section mismatch" in my building with
CONFIG_DEBUG_SECTION_MISMATCH=y.

Hmm...

> 
> Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>
> Cc: Michel Pollet <buserror@gmail.com>
> Cc: Kukjin Kim <kgene.kim@samsung.com>
> ---
>  arch/arm/mach-s3c2440/mach-mini2440.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/arm/mach-s3c2440/mach-mini2440.c b/arch/arm/mach-
> s3c2440/mach-mini2440.c
> index dd3120d..fc2dc0b 100644
> --- a/arch/arm/mach-s3c2440/mach-mini2440.c
> +++ b/arch/arm/mach-s3c2440/mach-mini2440.c
> @@ -552,7 +552,7 @@ struct mini2440_features_t {
>  	struct platform_device *optional[8];
>  };
> 
> -static void mini2440_parse_features(
> +static void __init mini2440_parse_features(
>  		struct mini2440_features_t * features,
>  		const char * features_str )
>  {
> --
> 1.7.5.4

Thanks.

Best regards,
Kgene.
--
Kukjin Kim <kgene.kim@samsung.com>, Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.

[PATCH 4/4] arm: mach-s3c2440: mini2440: fix section mismatch

[Russell King - ARM Linux]

On Fri, Jul 08, 2011 at 03:06:17PM +0900, Kukjin Kim wrote:
> Wolfram Sang wrote:
> > 
> > If mini2440_init() is in __init, mini2440_parse_features() should also
> > be in __init. Fixes:
> > 
> > (.text+0x9adc): Section mismatch in reference from the function
> > mini2440_parse_features.clone.0() to the (unknown reference)
> .init.data:(unknown)
> > The function mini2440_parse_features.clone.0() references the (unknown
> reference)
> > __initdata (unknown).
> 
> But I couldn't find above "Section mismatch" in my building with
> CONFIG_DEBUG_SECTION_MISMATCH=y.

Your compiler may have inlined the function, so defeating the check.

static struct s3c2410fb_display mini2440_lcd_cfg[] __initdata = {

and mini2440_parse_features() references this variable.  Therefore
either mini2440_parse_features() needs to be marked __init (it's
only caller is from __init, so that seems sensible) or the
__initdata markers on the referenced data need to be removed.

Adding __init looks like the right thing.

[PATCH 4/4] arm: mach-s3c2440: mini2440: fix section mismatch

[Kukjin Kim]

Russell King - ARM Linux wrote:
> 
> On Fri, Jul 08, 2011 at 03:06:17PM +0900, Kukjin Kim wrote:
> > Wolfram Sang wrote:
> > >
> > > If mini2440_init() is in __init, mini2440_parse_features() should also
> > > be in __init. Fixes:
> > >
> > > (.text+0x9adc): Section mismatch in reference from the function
> > > mini2440_parse_features.clone.0() to the (unknown reference)
> > .init.data:(unknown)
> > > The function mini2440_parse_features.clone.0() references the (unknown
> > reference)
> > > __initdata (unknown).
> >
> > But I couldn't find above "Section mismatch" in my building with
> > CONFIG_DEBUG_SECTION_MISMATCH=y.
> 
> Your compiler may have inlined the function, so defeating the check.
> 
> static struct s3c2410fb_display mini2440_lcd_cfg[] __initdata = {
> 
> and mini2440_parse_features() references this variable.  Therefore
> either mini2440_parse_features() needs to be marked __init (it's
> only caller is from __init, so that seems sensible) or the
> __initdata markers on the referenced data need to be removed.
> 
> Adding __init looks like the right thing.

OK, applied this into -fix.
Thanks.

Best regards,
Kgene.
--
Kukjin Kim <kgene.kim@samsung.com>, Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.