Patchwork dma/imx-sdma+imx-dma: explicitly #include <linux/module.h>

login
register
mail settings
Submitter Uwe Kleine-König
Date Sept. 26, 2011, 6:26 a.m.
Message ID <1317018379-8897-1-git-send-email-u.kleine-koenig@pengutronix.de>
Download mbox | patch
Permalink /patch/116361/
State New
Headers show

Comments

Uwe Kleine-König - Sept. 26, 2011, 6:26 a.m.
This is needed after commit

	include: replace linux/module.h with "struct module" wherever possible

(currently 25215aa in next).

Cc: Paul Gortmaker <paul.gortmaker@windriver.com>
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
Hello,

maybe it's sensible for Paul to take that before his commit?

Best regards
Uwe

 drivers/dma/imx-dma.c  |    1 +
 drivers/dma/imx-sdma.c |    1 +
 2 files changed, 2 insertions(+), 0 deletions(-)
Paul Gortmaker - Sept. 27, 2011, 10:03 p.m.
On 11-09-26 02:26 AM, Uwe Kleine-König wrote:
> This is needed after commit
> 
> 	include: replace linux/module.h with "struct module" wherever possible
> 
> (currently 25215aa in next).
> 
> Cc: Paul Gortmaker <paul.gortmaker@windriver.com>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
> Hello,
> 
> maybe it's sensible for Paul to take that before his commit?

Thanks, I'll blend it into the commit adding module.h to the other
drivers/dma files.  It seems the imx boards don't have a defconfig
in the arch/arm/configs that enables this (CONFIG_IMX_[S]DMA).

What is really interesting (at least to me anyway) is that I
didn't get one iMX build in some 6000+ ARM randconfig builds....

I would have thought it would have showed up at least a couple
times, given that number of builds.  Maybe randconfig doesn't
deal so well with "choice" Kconfig items?

~/git/module.h/linux-2.6.git$ ls -1 ../build-10*arm |wc -l
6399
~/git/module.h/linux-2.6.git$ grep CONFIG_ARCH_[A-Z0-9]*=y ../build-10*arm*/.config | sed 's/.*CON/CON/'|sort |uniq
CONFIG_ARCH_EBSA110=y
CONFIG_ARCH_H7201=y
CONFIG_ARCH_H7202=y
CONFIG_ARCH_H720X=y
CONFIG_ARCH_VERSATILE=y
~/git/module.h/linux-2.6.git$ 

No ARCH_MXC, no ARCH_MXS, PXA, or any of the other 20+ variants.
Definitely not a random spread there, with all 6000 builds falling
in just 5 buckets.

Paul.


> 
> Best regards
> Uwe
> 
>  drivers/dma/imx-dma.c  |    1 +
>  drivers/dma/imx-sdma.c |    1 +
>  2 files changed, 2 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/dma/imx-dma.c b/drivers/dma/imx-dma.c
> index d99f71c..3732de8 100644
> --- a/drivers/dma/imx-dma.c
> +++ b/drivers/dma/imx-dma.c
> @@ -23,6 +23,7 @@
>  #include <linux/slab.h>
>  #include <linux/platform_device.h>
>  #include <linux/dmaengine.h>
> +#include <linux/module.h>
>  
>  #include <asm/irq.h>
>  #include <mach/dma-v1.h>
> diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
> index b5cc27d..86fb6e5 100644
> --- a/drivers/dma/imx-sdma.c
> +++ b/drivers/dma/imx-sdma.c
> @@ -34,6 +34,7 @@
>  #include <linux/dmaengine.h>
>  #include <linux/of.h>
>  #include <linux/of_device.h>
> +#include <linux/module.h>
>  
>  #include <asm/irq.h>
>  #include <mach/sdma.h>
Koul, Vinod - Sept. 28, 2011, 3:49 a.m.
On Tue, 2011-09-27 at 18:03 -0400, Paul Gortmaker wrote:
> On 11-09-26 02:26 AM, Uwe Kleine-König wrote:
> > This is needed after commit
> > 
> > 	include: replace linux/module.h with "struct module" wherever possible
> > 
> > (currently 25215aa in next).
> > 
> > Cc: Paul Gortmaker <paul.gortmaker@windriver.com>
> > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Acked-by Vinod Koul <vinod.koul@intel.com>

> > ---
> > Hello,
> > 
> > maybe it's sensible for Paul to take that before his commit?
> 
> Thanks, I'll blend it into the commit adding module.h to the other
> drivers/dma files.  It seems the imx boards don't have a defconfig
> in the arch/arm/configs that enables this (CONFIG_IMX_[S]DMA).
You can find imx_v4_v5_defconfig in linux-next tree.

> 
> What is really interesting (at least to me anyway) is that I
> didn't get one iMX build in some 6000+ ARM randconfig builds....
> 
> I would have thought it would have showed up at least a couple
> times, given that number of builds.  Maybe randconfig doesn't
> deal so well with "choice" Kconfig items?
> 
> ~/git/module.h/linux-2.6.git$ ls -1 ../build-10*arm |wc -l
> 6399
> ~/git/module.h/linux-2.6.git$ grep CONFIG_ARCH_[A-Z0-9]*=y ../build-10*arm*/.config | sed 's/.*CON/CON/'|sort |uniq
> CONFIG_ARCH_EBSA110=y
> CONFIG_ARCH_H7201=y
> CONFIG_ARCH_H7202=y
> CONFIG_ARCH_H720X=y
> CONFIG_ARCH_VERSATILE=y
> ~/git/module.h/linux-2.6.git$ 
> 
> No ARCH_MXC, no ARCH_MXS, PXA, or any of the other 20+ variants.
> Definitely not a random spread there, with all 6000 builds falling
> in just 5 buckets.
> 
> Paul.
> 
> 
> > 
> > Best regards
> > Uwe
> > 
> >  drivers/dma/imx-dma.c  |    1 +
> >  drivers/dma/imx-sdma.c |    1 +
> >  2 files changed, 2 insertions(+), 0 deletions(-)
> > 
> > diff --git a/drivers/dma/imx-dma.c b/drivers/dma/imx-dma.c
> > index d99f71c..3732de8 100644
> > --- a/drivers/dma/imx-dma.c
> > +++ b/drivers/dma/imx-dma.c
> > @@ -23,6 +23,7 @@
> >  #include <linux/slab.h>
> >  #include <linux/platform_device.h>
> >  #include <linux/dmaengine.h>
> > +#include <linux/module.h>
> >  
> >  #include <asm/irq.h>
> >  #include <mach/dma-v1.h>
> > diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
> > index b5cc27d..86fb6e5 100644
> > --- a/drivers/dma/imx-sdma.c
> > +++ b/drivers/dma/imx-sdma.c
> > @@ -34,6 +34,7 @@
> >  #include <linux/dmaengine.h>
> >  #include <linux/of.h>
> >  #include <linux/of_device.h>
> > +#include <linux/module.h>
> >  
> >  #include <asm/irq.h>
> >  #include <mach/sdma.h>
Uwe Kleine-König - Sept. 28, 2011, 6:46 a.m.
[Cc: += linux-kbuild@v.k.o + Michal Marek]

Hello,

On Tue, Sep 27, 2011 at 06:03:13PM -0400, Paul Gortmaker wrote:
> On 11-09-26 02:26 AM, Uwe Kleine-König wrote:
> > This is needed after commit
> > 
> > 	include: replace linux/module.h with "struct module" wherever possible
> > 
> > (currently 25215aa in next).
> > 
> > Cc: Paul Gortmaker <paul.gortmaker@windriver.com>
> > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> > ---
> > Hello,
> > 
> > maybe it's sensible for Paul to take that before his commit?
> 
> Thanks, I'll blend it into the commit adding module.h to the other
> drivers/dma files.  It seems the imx boards don't have a defconfig
> in the arch/arm/configs that enables this (CONFIG_IMX_[S]DMA).
> 
> What is really interesting (at least to me anyway) is that I
> didn't get one iMX build in some 6000+ ARM randconfig builds....
> 
> I would have thought it would have showed up at least a couple
> times, given that number of builds.  Maybe randconfig doesn't
> deal so well with "choice" Kconfig items?
> 
> ~/git/module.h/linux-2.6.git$ ls -1 ../build-10*arm |wc -l
> 6399
> ~/git/module.h/linux-2.6.git$ grep CONFIG_ARCH_[A-Z0-9]*=y ../build-10*arm*/.config | sed 's/.*CON/CON/'|sort |uniq
> CONFIG_ARCH_EBSA110=y
> CONFIG_ARCH_H7201=y
> CONFIG_ARCH_H7202=y
> CONFIG_ARCH_H720X=y
> CONFIG_ARCH_VERSATILE=y

I don't understand why you did that sed, but AFAICT it doesn't hurt,
too. Maybe adding -c to uniq in your command line is interesting for the
kbuild people?!

> ~/git/module.h/linux-2.6.git$ 
> 
> No ARCH_MXC, no ARCH_MXS, PXA, or any of the other 20+ variants.
> Definitely not a random spread there, with all 6000 builds falling
> in just 5 buckets.
How did you test? Generate a .config, test without your change and if
that succeeds test with it? If so, maybe the problem isn't that
randconfig doesn't generate more random configs but that you got many
failures in the first run?!

Best regards
Uwe
Arnaud Lacombe - Sept. 28, 2011, 7:34 a.m.
Hi,

2011/9/28 Uwe Kleine-König <u.kleine-koenig@pengutronix.de>:
> [Cc: += linux-kbuild@v.k.o + Michal Marek]
>
> Hello,
>
> On Tue, Sep 27, 2011 at 06:03:13PM -0400, Paul Gortmaker wrote:
>> On 11-09-26 02:26 AM, Uwe Kleine-König wrote:
>> > This is needed after commit
>> >
>> >     include: replace linux/module.h with "struct module" wherever possible
>> >
>> > (currently 25215aa in next).
>> >
>> > Cc: Paul Gortmaker <paul.gortmaker@windriver.com>
>> > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
>> > ---
>> > Hello,
>> >
>> > maybe it's sensible for Paul to take that before his commit?
>>
>> Thanks, I'll blend it into the commit adding module.h to the other
>> drivers/dma files.  It seems the imx boards don't have a defconfig
>> in the arch/arm/configs that enables this (CONFIG_IMX_[S]DMA).
>>
>> What is really interesting (at least to me anyway) is that I
>> didn't get one iMX build in some 6000+ ARM randconfig builds....
>>
>> I would have thought it would have showed up at least a couple
>> times, given that number of builds.  Maybe randconfig doesn't
>> deal so well with "choice" Kconfig items?
>>
>> ~/git/module.h/linux-2.6.git$ ls -1 ../build-10*arm |wc -l
>> 6399
>> ~/git/module.h/linux-2.6.git$ grep CONFIG_ARCH_[A-Z0-9]*=y ../build-10*arm*/.config | sed 's/.*CON/CON/'|sort |uniq
>> CONFIG_ARCH_EBSA110=y
>> CONFIG_ARCH_H7201=y
>> CONFIG_ARCH_H7202=y
>> CONFIG_ARCH_H720X=y
>> CONFIG_ARCH_VERSATILE=y
>
> I don't understand why you did that sed, but AFAICT it doesn't hurt,
> too. Maybe adding -c to uniq in your command line is interesting for the
> kbuild people?!
>
this can be reproduced trivially:

for i in $(seq 1 100); do
    make ARCH=arm randconfig
    sed -e '/CONFIG_ARCH_INTEGRATOR/,/CONFIG_ARCH_ZYNQ/!d;' \
        -e '/=y/!d' .config >> arch_selected
done
sort arch_selected | uniq -c

I may have a trivial explanation for part of the symbol, which depends
on MMU, so if these symbols are selected, but MMU is not, it will
fall-back on the default, VERSATILE, which should happen 50% of the
time. However, there is plenty of symbols which have no specific
dependency, and are still not selected.

On the other side, all the result of a randconfig, on the choice alone
(ie. without any 'select' and any other symbols) is spread
"correctly".

Taken as a whole:

% cat arch_selected | sort | uniq -c | wc -l
     3

% cat arch_selected | sort | uniq -c
      2 CONFIG_ARCH_EBSA110=y
      4 CONFIG_ARCH_H720X=y
    100 CONFIG_ARCH_VERSATILE=y

However, when the choice is alone:

% cat arch_selected | sort | uniq -c | wc -l
     39

>> ~/git/module.h/linux-2.6.git$
>>
>> No ARCH_MXC, no ARCH_MXS, PXA, or any of the other 20+ variants.
>> Definitely not a random spread there, with all 6000 builds falling
>> in just 5 buckets.
> How did you test? Generate a .config, test without your change and if
> that succeeds test with it? If so, maybe the problem isn't that
> randconfig doesn't generate more random configs but that you got many
> failures in the first run?!
>
it would seem that taken globally, the result of this particular
choice is not random, but I'm not sure of the dynamic involved
precisely.

I'm not a huge fan of the way randconfig works presently, that it by
just iterating over all symbols and setting a value . I'd prefer to
keep traversing all the menu, picking a random value for
user-changeable setting, over and over until all symbol have been
chosen. However, this is a whole lots of change...

 - Arnaud

> Best regards
> Uwe
>
> --
> Pengutronix e.K.                           | Uwe Kleine-König            |
> Industrial Linux Solutions                 | http://www.pengutronix.de/  |
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>
Paul Gortmaker - Sept. 28, 2011, 1:36 p.m.
On 11-09-28 02:46 AM, Uwe Kleine-König wrote:
> [Cc: += linux-kbuild@v.k.o + Michal Marek]
> 
> Hello,
> 
> On Tue, Sep 27, 2011 at 06:03:13PM -0400, Paul Gortmaker wrote:
>> On 11-09-26 02:26 AM, Uwe Kleine-König wrote:
>>> This is needed after commit
>>>
>>> 	include: replace linux/module.h with "struct module" wherever possible
>>>
>>> (currently 25215aa in next).
>>>
>>> Cc: Paul Gortmaker <paul.gortmaker@windriver.com>
>>> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
>>> ---
>>> Hello,
>>>
>>> maybe it's sensible for Paul to take that before his commit?
>>
>> Thanks, I'll blend it into the commit adding module.h to the other
>> drivers/dma files.  It seems the imx boards don't have a defconfig
>> in the arch/arm/configs that enables this (CONFIG_IMX_[S]DMA).
>>
>> What is really interesting (at least to me anyway) is that I
>> didn't get one iMX build in some 6000+ ARM randconfig builds....
>>
>> I would have thought it would have showed up at least a couple
>> times, given that number of builds.  Maybe randconfig doesn't
>> deal so well with "choice" Kconfig items?
>>
>> ~/git/module.h/linux-2.6.git$ ls -1 ../build-10*arm |wc -l
>> 6399
>> ~/git/module.h/linux-2.6.git$ grep CONFIG_ARCH_[A-Z0-9]*=y ../build-10*arm*/.config | sed 's/.*CON/CON/'|sort |uniq
>> CONFIG_ARCH_EBSA110=y
>> CONFIG_ARCH_H7201=y
>> CONFIG_ARCH_H7202=y
>> CONFIG_ARCH_H720X=y
>> CONFIG_ARCH_VERSATILE=y
> 
> I don't understand why you did that sed, but AFAICT it doesn't hurt,
> too. Maybe adding -c to uniq in your command line is interesting for the
> kbuild people?!

The sed was to strip the build paths from the grep output.  I could
have used "grep -h" instead.

> 
>> ~/git/module.h/linux-2.6.git$ 
>>
>> No ARCH_MXC, no ARCH_MXS, PXA, or any of the other 20+ variants.
>> Definitely not a random spread there, with all 6000 builds falling
>> in just 5 buckets.
> How did you test? Generate a .config, test without your change and if
> that succeeds test with it? If so, maybe the problem isn't that
> randconfig doesn't generate more random configs but that you got many
> failures in the first run?!

Testing was with the following chunk out of my $0.02 script:

 i=10000 ; while [ 1 ] ; do D=../build-$i-$MYARCH ; mkdir $D ; echo xxxxxxx $D xxxxxx  ; make ARCH=$MYARCH O=$D randconfig > $D/cfg.log 2>&1 ; if [ $? != 0 ] ; then echo  cfg failed in $D ; break ; fi ; time make ARCH=$MYARCH O=$D -j16 > $D/build.log 2>&1 ; if [ $? != 0 ]; then echo  bld fail in $D ; fi ; i=$[$i+1] ; done

Then I'd comb all the build logs looking for errors that were caused
by changes that I made.  As you can see, it just calls randconfig, but
the coverage never really "randomly" covered all arm platforms.

Paul.



> 
> Best regards
> Uwe
>

Patch

diff --git a/drivers/dma/imx-dma.c b/drivers/dma/imx-dma.c
index d99f71c..3732de8 100644
--- a/drivers/dma/imx-dma.c
+++ b/drivers/dma/imx-dma.c
@@ -23,6 +23,7 @@ 
 #include <linux/slab.h>
 #include <linux/platform_device.h>
 #include <linux/dmaengine.h>
+#include <linux/module.h>
 
 #include <asm/irq.h>
 #include <mach/dma-v1.h>
diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
index b5cc27d..86fb6e5 100644
--- a/drivers/dma/imx-sdma.c
+++ b/drivers/dma/imx-sdma.c
@@ -34,6 +34,7 @@ 
 #include <linux/dmaengine.h>
 #include <linux/of.h>
 #include <linux/of_device.h>
+#include <linux/module.h>
 
 #include <asm/irq.h>
 #include <mach/sdma.h>