2010-09-17 00:08:12

by Vipin Mehta

[permalink] [raw]
Subject: [PATCH] staging: ath6kl: Including a header file to fix a compilation error

The commit fixes a compilation error that was encountered while using a specific kernel configuration file. The problem was the use of some fuunctions defined in <linux/semaphore.h> without including the header file explicitly. It was probably working before because of the dependency getting implicitly satisfied via some other header file. Also, eliminating the inclusion of the same header file more than once. The code needs additional cleanup and may be addressed by a subsequent commit.

Signed-off-by: Vipin Mehta <[email protected]>
---
.../staging/ath6kl/miscdrv/ar3kps/ar3kpsconfig.h | 4 ----
drivers/staging/ath6kl/os/linux/cfg80211.c | 2 --
.../staging/ath6kl/os/linux/include/ar6000_drv.h | 8 --------
.../staging/ath6kl/os/linux/include/osapi_linux.h | 1 +
drivers/staging/ath6kl/os/linux/netbuf.c | 2 --
5 files changed, 1 insertions(+), 16 deletions(-)

diff --git a/drivers/staging/ath6kl/miscdrv/ar3kps/ar3kpsconfig.h b/drivers/staging/ath6kl/miscdrv/ar3kps/ar3kpsconfig.h
index 4358834..4e5b7bf 100644
--- a/drivers/staging/ath6kl/miscdrv/ar3kps/ar3kpsconfig.h
+++ b/drivers/staging/ath6kl/miscdrv/ar3kps/ar3kpsconfig.h
@@ -36,15 +36,11 @@

#include <linux/fs.h>
#include <linux/errno.h>
-#include <linux/string.h>
#include <linux/signal.h>
-#include <linux/timer.h>


#include <linux/ioctl.h>
-#include <linux/skbuff.h>
#include <linux/firmware.h>
-#include <linux/wait.h>


#include <net/bluetooth/bluetooth.h>
diff --git a/drivers/staging/ath6kl/os/linux/cfg80211.c b/drivers/staging/ath6kl/os/linux/cfg80211.c
index 7a3784d..f51c5e1 100644
--- a/drivers/staging/ath6kl/os/linux/cfg80211.c
+++ b/drivers/staging/ath6kl/os/linux/cfg80211.c
@@ -21,8 +21,6 @@
// Author(s): ="Atheros"
//------------------------------------------------------------------------------

-#include <linux/kernel.h>
-#include <linux/netdevice.h>
#include <linux/wireless.h>
#include <linux/ieee80211.h>
#include <net/cfg80211.h>
diff --git a/drivers/staging/ath6kl/os/linux/include/ar6000_drv.h b/drivers/staging/ath6kl/os/linux/include/ar6000_drv.h
index 8be4f55..e624883 100644
--- a/drivers/staging/ath6kl/os/linux/include/ar6000_drv.h
+++ b/drivers/staging/ath6kl/os/linux/include/ar6000_drv.h
@@ -24,22 +24,14 @@
#ifndef _AR6000_H_
#define _AR6000_H_

-#include <linux/version.h>
-
-
-#include <generated/autoconf.h>
#include <linux/init.h>
-#include <linux/kernel.h>
#include <linux/sched.h>
#include <linux/spinlock.h>
-#include <linux/skbuff.h>
#include <linux/if_ether.h>
-#include <linux/netdevice.h>
#include <linux/etherdevice.h>
#include <net/iw_handler.h>
#include <linux/if_arp.h>
#include <linux/ip.h>
-#include <linux/semaphore.h>
#include <linux/wireless.h>
#ifdef ATH6K_CONFIG_CFG80211
#include <net/cfg80211.h>
diff --git a/drivers/staging/ath6kl/os/linux/include/osapi_linux.h b/drivers/staging/ath6kl/os/linux/include/osapi_linux.h
index ef7cc82..9892dfc 100644
--- a/drivers/staging/ath6kl/os/linux/include/osapi_linux.h
+++ b/drivers/staging/ath6kl/os/linux/include/osapi_linux.h
@@ -39,6 +39,7 @@
#include <linux/timer.h>
#include <linux/delay.h>
#include <linux/wait.h>
+#include <linux/semaphore.h>

#include <linux/cache.h>

diff --git a/drivers/staging/ath6kl/os/linux/netbuf.c b/drivers/staging/ath6kl/os/linux/netbuf.c
index 63fa49c..15e5d04 100644
--- a/drivers/staging/ath6kl/os/linux/netbuf.c
+++ b/drivers/staging/ath6kl/os/linux/netbuf.c
@@ -20,8 +20,6 @@
//
// Author(s): ="Atheros"
//------------------------------------------------------------------------------
-#include <linux/kernel.h>
-#include <linux/skbuff.h>
#include <a_config.h>
#include "athdefs.h"
#include "a_types.h"
--
1.6.3.3



2010-09-18 02:00:58

by Vipin Mehta

[permalink] [raw]
Subject: RE: [PATCH] staging: ath6kl: Including a header file to fix a compilation error

>
> Please, fix your email client to wrap lines at a sane place...
Do you have any recommendation for the email client?
I am using outlook currently.

> If the problem is due to another change in a different tree, send me the
> patch and I'll hold on to it until the mmc tree is merged with Linus
> (during the .37 merge window) and then I'll apply it and push it out.
>
> Until then, we can ask Stephen to hold the patch in linux-next to fix
> the build issue.
>
I have sent out a patch for the linux-next tree that
fixes the problem. BTW, is there a way I can include
comments in the patch. How do you establish a context
between what is discussed in these emails with the
patches sent to you?

> But I'd still like the .h file fixup patch to be resolved and in my tree
> first, right?
I have sent you the patch.
>
> thanks,
>
> greg k-h

2010-09-17 23:26:43

by Vipin Mehta

[permalink] [raw]
Subject: RE: [PATCH] staging: ath6kl: Including a header file to fix a compilation error

>
> Ok, I'll not apply this until it is all resolved.
The error popped up due a recent commit which happened after I generated the patch below. I can regenerate the patch that takes care of these new errors as well but the following patch does fix the problem originally reported.

>
> > > diff --git a/drivers/staging/ath6kl/miscdrv/ar3kps/ar3kpsconfig.h
> b/drivers/staging/ath6kl/miscdrv/ar3kps/ar3kpsconfig.h
> > > index 4358834..4e5b7bf 100644
> > > --- a/drivers/staging/ath6kl/miscdrv/ar3kps/ar3kpsconfig.h
> > > +++ b/drivers/staging/ath6kl/miscdrv/ar3kps/ar3kpsconfig.h
> > > @@ -36,15 +36,11 @@
> > >
> > > #include <linux/fs.h>
> > > #include <linux/errno.h>
> > > -#include <linux/string.h>
> > > #include <linux/signal.h>
> > > -#include <linux/timer.h>
> > >
> > >
> > > #include <linux/ioctl.h>
> > > -#include <linux/skbuff.h>
> > > #include <linux/firmware.h>
> > > -#include <linux/wait.h>
>
> Why are you removing these?
The header files being removed are already included by a common header file osapi_linux.h. The file osapi_linux.h is included by all the source files. I saw little point in including these header files again.


> > > -#include <linux/semaphore.h>
> > > #include <linux/wireless.h>
> > > #ifdef ATH6K_CONFIG_CFG80211
> > > #include <net/cfg80211.h>
>
> And these?
>
> All to fix the one build error?
No. The header files being removed are just a minor clean up that I thought can be included along with the fix as they were kind of related. I guess I could use a separate patch.

> > > #include <linux/timer.h>
> > > #include <linux/delay.h>
> > > #include <linux/wait.h>
> > > +#include <linux/semaphore.h>
>
> Don't put #include in a .h file if at all possible please.
I guess the header files can be included explicitly within the source files but it requires a bigger change to the driver. There are some header files which are included by all the source files and contain some commonly used header files. These commonly used header files will otherwise have to be included separately in each of the source files.

>
> > >
> > > #include <linux/cache.h>
> > >
> > > diff --git a/drivers/staging/ath6kl/os/linux/netbuf.c
> b/drivers/staging/ath6kl/os/linux/netbuf.c
> > > index 63fa49c..15e5d04 100644
> > > --- a/drivers/staging/ath6kl/os/linux/netbuf.c
> > > +++ b/drivers/staging/ath6kl/os/linux/netbuf.c
> > > @@ -20,8 +20,6 @@
> > > //
> > > // Author(s): ="Atheros"
> > > //-------------------------------------------------------------------
> -----------
> > > -#include <linux/kernel.h>
> > > -#include <linux/skbuff.h>
>
> Why remove these?
>
> Totally confused,
>
> greg k-h

2010-09-17 00:10:56

by Luis R. Rodriguez

[permalink] [raw]
Subject: Re: [PATCH] staging: ath6kl: Including a header file to fix a compilation error

On Thu, Sep 16, 2010 at 5:08 PM, Vipin Mehta <[email protected]> wrote:
> The commit fixes a compilation error that was encountered while using a specific kernel configuration file. The problem was the use of some fuunctions defined in <linux/semaphore.h> without including the header file explicitly. It was probably working before because of the dependency getting implicitly satisfied via some other header file. Also, eliminating the inclusion of the same header file more than once. The code needs additional cleanup and may be addressed by a subsequent commit.

Keep the commit log lines to less than about 70 lines or so.

Luis

2010-09-17 18:01:14

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH] staging: ath6kl: Including a header file to fix a compilation error

On Fri, Sep 17, 2010 at 10:23:30AM -0700, Randy Dunlap wrote:
> On 09/17/10 09:54, Vipin Mehta wrote:
> > The commit fixes a compilation error that was encountered while using
> > a specific kernel configuration file. The problem was the use of some
> > functions defined in <linux/semaphore.h> without including the header
> > file explicitly. It was probably working before because of the
> > dependency getting implicitly satisfied via some other header file.
> > Also, eliminating the inclusion of the same header file more than
> > once. The code needs additional cleanup and may be addressed by a
> > subsequent commit.
> >
> > Reported-by: Randy Dunlap <[email protected]>
> > Signed-off-by: Vipin Mehta <[email protected]>
> > ---
>
> Hi,
> Thanks for the patch, but I still get these errors & warnings:

<snip>

Ok, I'll not apply this until it is all resolved.

> > diff --git a/drivers/staging/ath6kl/miscdrv/ar3kps/ar3kpsconfig.h b/drivers/staging/ath6kl/miscdrv/ar3kps/ar3kpsconfig.h
> > index 4358834..4e5b7bf 100644
> > --- a/drivers/staging/ath6kl/miscdrv/ar3kps/ar3kpsconfig.h
> > +++ b/drivers/staging/ath6kl/miscdrv/ar3kps/ar3kpsconfig.h
> > @@ -36,15 +36,11 @@
> >
> > #include <linux/fs.h>
> > #include <linux/errno.h>
> > -#include <linux/string.h>
> > #include <linux/signal.h>
> > -#include <linux/timer.h>
> >
> >
> > #include <linux/ioctl.h>
> > -#include <linux/skbuff.h>
> > #include <linux/firmware.h>
> > -#include <linux/wait.h>

Why are you removing these?

> >
> >
> > #include <net/bluetooth/bluetooth.h>
> > diff --git a/drivers/staging/ath6kl/os/linux/cfg80211.c b/drivers/staging/ath6kl/os/linux/cfg80211.c
> > index 7a3784d..f51c5e1 100644
> > --- a/drivers/staging/ath6kl/os/linux/cfg80211.c
> > +++ b/drivers/staging/ath6kl/os/linux/cfg80211.c
> > @@ -21,8 +21,6 @@
> > // Author(s): ="Atheros"
> > //------------------------------------------------------------------------------
> >
> > -#include <linux/kernel.h>
> > -#include <linux/netdevice.h>

And this?

> > #include <linux/wireless.h>
> > #include <linux/ieee80211.h>
> > #include <net/cfg80211.h>
> > diff --git a/drivers/staging/ath6kl/os/linux/include/ar6000_drv.h b/drivers/staging/ath6kl/os/linux/include/ar6000_drv.h
> > index 8be4f55..e624883 100644
> > --- a/drivers/staging/ath6kl/os/linux/include/ar6000_drv.h
> > +++ b/drivers/staging/ath6kl/os/linux/include/ar6000_drv.h
> > @@ -24,22 +24,14 @@
> > #ifndef _AR6000_H_
> > #define _AR6000_H_
> >
> > -#include <linux/version.h>
> > -
> > -
> > -#include <generated/autoconf.h>
> > #include <linux/init.h>
> > -#include <linux/kernel.h>
> > #include <linux/sched.h>
> > #include <linux/spinlock.h>
> > -#include <linux/skbuff.h>
> > #include <linux/if_ether.h>
> > -#include <linux/netdevice.h>
> > #include <linux/etherdevice.h>
> > #include <net/iw_handler.h>
> > #include <linux/if_arp.h>
> > #include <linux/ip.h>
> > -#include <linux/semaphore.h>
> > #include <linux/wireless.h>
> > #ifdef ATH6K_CONFIG_CFG80211
> > #include <net/cfg80211.h>

And these?

All to fix the one build error?

> > diff --git a/drivers/staging/ath6kl/os/linux/include/osapi_linux.h b/drivers/staging/ath6kl/os/linux/include/osapi_linux.h
> > index ef7cc82..9892dfc 100644
> > --- a/drivers/staging/ath6kl/os/linux/include/osapi_linux.h
> > +++ b/drivers/staging/ath6kl/os/linux/include/osapi_linux.h
> > @@ -39,6 +39,7 @@
> > #include <linux/timer.h>
> > #include <linux/delay.h>
> > #include <linux/wait.h>
> > +#include <linux/semaphore.h>

Don't put #include in a .h file if at all possible please.

> >
> > #include <linux/cache.h>
> >
> > diff --git a/drivers/staging/ath6kl/os/linux/netbuf.c b/drivers/staging/ath6kl/os/linux/netbuf.c
> > index 63fa49c..15e5d04 100644
> > --- a/drivers/staging/ath6kl/os/linux/netbuf.c
> > +++ b/drivers/staging/ath6kl/os/linux/netbuf.c
> > @@ -20,8 +20,6 @@
> > //
> > // Author(s): ="Atheros"
> > //------------------------------------------------------------------------------
> > -#include <linux/kernel.h>
> > -#include <linux/skbuff.h>

Why remove these?

Totally confused,

greg k-h

2010-09-18 05:35:01

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH] staging: ath6kl: Including a header file to fix a compilation error

On Fri, Sep 17, 2010 at 07:06:42PM -0700, Vipin Mehta wrote:
> >
> > {sigh} Line length please...
> Sorry about that. I have now configured outlook to wrap the line at 70 characters. It was 76 by default. Hopefully, it shows up better now.

No, it's still wrong, you need to fix outlook to properly add line ends
for your lines.

Or better yet, don't use outlook. Seriously. There are lots of other
email clients out there that are way better, and free, and some even
will work with your exchange server.

good luck,

greg k-h

2010-09-17 17:24:46

by Randy Dunlap

[permalink] [raw]
Subject: Re: [PATCH] staging: ath6kl: Including a header file to fix a compilation error

On 09/17/10 09:54, Vipin Mehta wrote:
> The commit fixes a compilation error that was encountered while using
> a specific kernel configuration file. The problem was the use of some
> functions defined in <linux/semaphore.h> without including the header
> file explicitly. It was probably working before because of the
> dependency getting implicitly satisfied via some other header file.
> Also, eliminating the inclusion of the same header file more than
> once. The code needs additional cleanup and may be addressed by a
> subsequent commit.
>
> Reported-by: Randy Dunlap <[email protected]>
> Signed-off-by: Vipin Mehta <[email protected]>
> ---

Hi,
Thanks for the patch, but I still get these errors & warnings:

on linux-next 2010-0915:

drivers/staging/ath6kl/hif/sdio/linux_sdio/src/hif_scatter.c: In function 'SetupHIFScatterSupport':
drivers/staging/ath6kl/hif/sdio/linux_sdio/src/hif_scatter.c:288: error: 'struct mmc_host' has no member named 'max_hw_segs'
drivers/staging/ath6kl/hif/sdio/linux_sdio/src/hif_scatter.c:289: error: 'struct mmc_host' has no member named 'max_hw_segs'

and on linux-next 2010-0917:

drivers/staging/ath6kl/os/linux/ar6000_drv.c: In function 'ar6000_transfer_bin_file':
drivers/staging/ath6kl/os/linux/ar6000_drv.c:1129: warning: cast from pointer to integer of different size
drivers/staging/ath6kl/os/linux/ar6000_drv.c:1129: warning: cast to pointer from integer of different size
drivers/staging/ath6kl/wmi/wmi.c: In function 'wmi_bssInfo_event_rx':
drivers/staging/ath6kl/wmi/wmi.c:1459: error: 'i' undeclared (first use in this function)


> .../staging/ath6kl/miscdrv/ar3kps/ar3kpsconfig.h | 4 ----
> drivers/staging/ath6kl/os/linux/cfg80211.c | 2 --
> .../staging/ath6kl/os/linux/include/ar6000_drv.h | 8 --------
> .../staging/ath6kl/os/linux/include/osapi_linux.h | 1 +
> drivers/staging/ath6kl/os/linux/netbuf.c | 2 --
> 5 files changed, 1 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/staging/ath6kl/miscdrv/ar3kps/ar3kpsconfig.h b/drivers/staging/ath6kl/miscdrv/ar3kps/ar3kpsconfig.h
> index 4358834..4e5b7bf 100644
> --- a/drivers/staging/ath6kl/miscdrv/ar3kps/ar3kpsconfig.h
> +++ b/drivers/staging/ath6kl/miscdrv/ar3kps/ar3kpsconfig.h
> @@ -36,15 +36,11 @@
>
> #include <linux/fs.h>
> #include <linux/errno.h>
> -#include <linux/string.h>
> #include <linux/signal.h>
> -#include <linux/timer.h>
>
>
> #include <linux/ioctl.h>
> -#include <linux/skbuff.h>
> #include <linux/firmware.h>
> -#include <linux/wait.h>
>
>
> #include <net/bluetooth/bluetooth.h>
> diff --git a/drivers/staging/ath6kl/os/linux/cfg80211.c b/drivers/staging/ath6kl/os/linux/cfg80211.c
> index 7a3784d..f51c5e1 100644
> --- a/drivers/staging/ath6kl/os/linux/cfg80211.c
> +++ b/drivers/staging/ath6kl/os/linux/cfg80211.c
> @@ -21,8 +21,6 @@
> // Author(s): ="Atheros"
> //------------------------------------------------------------------------------
>
> -#include <linux/kernel.h>
> -#include <linux/netdevice.h>
> #include <linux/wireless.h>
> #include <linux/ieee80211.h>
> #include <net/cfg80211.h>
> diff --git a/drivers/staging/ath6kl/os/linux/include/ar6000_drv.h b/drivers/staging/ath6kl/os/linux/include/ar6000_drv.h
> index 8be4f55..e624883 100644
> --- a/drivers/staging/ath6kl/os/linux/include/ar6000_drv.h
> +++ b/drivers/staging/ath6kl/os/linux/include/ar6000_drv.h
> @@ -24,22 +24,14 @@
> #ifndef _AR6000_H_
> #define _AR6000_H_
>
> -#include <linux/version.h>
> -
> -
> -#include <generated/autoconf.h>
> #include <linux/init.h>
> -#include <linux/kernel.h>
> #include <linux/sched.h>
> #include <linux/spinlock.h>
> -#include <linux/skbuff.h>
> #include <linux/if_ether.h>
> -#include <linux/netdevice.h>
> #include <linux/etherdevice.h>
> #include <net/iw_handler.h>
> #include <linux/if_arp.h>
> #include <linux/ip.h>
> -#include <linux/semaphore.h>
> #include <linux/wireless.h>
> #ifdef ATH6K_CONFIG_CFG80211
> #include <net/cfg80211.h>
> diff --git a/drivers/staging/ath6kl/os/linux/include/osapi_linux.h b/drivers/staging/ath6kl/os/linux/include/osapi_linux.h
> index ef7cc82..9892dfc 100644
> --- a/drivers/staging/ath6kl/os/linux/include/osapi_linux.h
> +++ b/drivers/staging/ath6kl/os/linux/include/osapi_linux.h
> @@ -39,6 +39,7 @@
> #include <linux/timer.h>
> #include <linux/delay.h>
> #include <linux/wait.h>
> +#include <linux/semaphore.h>
>
> #include <linux/cache.h>
>
> diff --git a/drivers/staging/ath6kl/os/linux/netbuf.c b/drivers/staging/ath6kl/os/linux/netbuf.c
> index 63fa49c..15e5d04 100644
> --- a/drivers/staging/ath6kl/os/linux/netbuf.c
> +++ b/drivers/staging/ath6kl/os/linux/netbuf.c
> @@ -20,8 +20,6 @@
> //
> // Author(s): ="Atheros"
> //------------------------------------------------------------------------------
> -#include <linux/kernel.h>
> -#include <linux/skbuff.h>
> #include <a_config.h>
> #include "athdefs.h"
> #include "a_types.h"


--
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***

2010-09-17 23:49:54

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH] staging: ath6kl: Including a header file to fix a compilation error

On Fri, Sep 17, 2010 at 04:26:41PM -0700, Vipin Mehta wrote:
> >
> > Ok, I'll not apply this until it is all resolved.
> The error popped up due a recent commit which happened after I generated the patch below. I can regenerate the patch that takes care of these new errors as well but the following patch does fix the problem originally reported.

{sigh} Line length please...

> > > > diff --git a/drivers/staging/ath6kl/miscdrv/ar3kps/ar3kpsconfig.h
> > b/drivers/staging/ath6kl/miscdrv/ar3kps/ar3kpsconfig.h
> > > > index 4358834..4e5b7bf 100644
> > > > --- a/drivers/staging/ath6kl/miscdrv/ar3kps/ar3kpsconfig.h
> > > > +++ b/drivers/staging/ath6kl/miscdrv/ar3kps/ar3kpsconfig.h
> > > > @@ -36,15 +36,11 @@
> > > >
> > > > #include <linux/fs.h>
> > > > #include <linux/errno.h>
> > > > -#include <linux/string.h>
> > > > #include <linux/signal.h>
> > > > -#include <linux/timer.h>
> > > >
> > > >
> > > > #include <linux/ioctl.h>
> > > > -#include <linux/skbuff.h>
> > > > #include <linux/firmware.h>
> > > > -#include <linux/wait.h>
> >
> > Why are you removing these?
> The header files being removed are already included by a common header file osapi_linux.h. The file osapi_linux.h is included by all the source files. I saw little point in including these header files again.

But that has nothing to do with this fix, right? Don't mix things in a
single patch. Cleanup is good and nice, but don't do that in a "fix a
problem" patch at the same time.

Remember:
One patch per "thing" you do.

> > > > -#include <linux/semaphore.h>
> > > > #include <linux/wireless.h>
> > > > #ifdef ATH6K_CONFIG_CFG80211
> > > > #include <net/cfg80211.h>
> >
> > And these?
> >
> > All to fix the one build error?
> No. The header files being removed are just a minor clean up that I thought can be included along with the fix as they were kind of related. I guess I could use a separate patch.

Yes, that is required.

> > > > #include <linux/timer.h>
> > > > #include <linux/delay.h>
> > > > #include <linux/wait.h>
> > > > +#include <linux/semaphore.h>
> >
> > Don't put #include in a .h file if at all possible please.
> I guess the header files can be included explicitly within the source files but it requires a bigger change to the driver. There are some header files which are included by all the source files and contain some commonly used header files. These commonly used header files will otherwise have to be included separately in each of the source files.

That's work that can be done later, let's just fix this build error
first please.

thanks,

greg k-h

2010-09-18 05:35:01

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH] staging: ath6kl: Including a header file to fix a compilation error

On Fri, Sep 17, 2010 at 07:00:56PM -0700, Vipin Mehta wrote:
> >
> > Please, fix your email client to wrap lines at a sane place...
> Do you have any recommendation for the email client?
> I am using outlook currently.

Anything except outlook (thunderbird, evolution, mutt, etc.)

> > If the problem is due to another change in a different tree, send me the
> > patch and I'll hold on to it until the mmc tree is merged with Linus
> > (during the .37 merge window) and then I'll apply it and push it out.
> >
> > Until then, we can ask Stephen to hold the patch in linux-next to fix
> > the build issue.
> >
> I have sent out a patch for the linux-next tree that
> fixes the problem. BTW, is there a way I can include
> comments in the patch. How do you establish a context
> between what is discussed in these emails with the
> patches sent to you?

Read Documentation/SubmittingPatches, section 15, for how to do this.

> > But I'd still like the .h file fixup patch to be resolved and in my tree
> > first, right?
> I have sent you the patch.

I got them, I'll process them next week.

thanks,

greg k-h

2010-09-18 02:06:45

by Vipin Mehta

[permalink] [raw]
Subject: RE: [PATCH] staging: ath6kl: Including a header file to fix a compilation error

>
> {sigh} Line length please...
Sorry about that. I have now configured outlook to wrap the line at 70 characters. It was 76 by default. Hopefully, it shows up better now.

> But that has nothing to do with this fix, right? Don't mix things
> in a
> single patch. Cleanup is good and nice, but don't do that in a "fix
> a
> problem" patch at the same time.
>
> Remember:
> One patch per "thing" you do.
I have split the patch into two and sent you the patches.

2010-09-17 23:13:49

by Vipin Mehta

[permalink] [raw]
Subject: RE: [PATCH] staging: ath6kl: Including a header file to fix a compilation error

> Hi,
> Thanks for the patch, but I still get these errors & warnings:
>
> on linux-next 2010-0915:
>
> drivers/staging/ath6kl/hif/sdio/linux_sdio/src/hif_scatter.c: In function
> 'SetupHIFScatterSupport':
> drivers/staging/ath6kl/hif/sdio/linux_sdio/src/hif_scatter.c:288: error:
> 'struct mmc_host' has no member named 'max_hw_segs'
> drivers/staging/ath6kl/hif/sdio/linux_sdio/src/hif_scatter.c:289: error:
> 'struct mmc_host' has no member named 'max_hw_segs'
>
This is because of a recent change in the mmc_host structure that removes the distinction between hw and phys segments. The structure is different in the staging-next-2.6 tree which is where I usually build and test the changes.

Greg,
The two trees linux-next and staging-next are not in sync so what is the process that is usually followed under these circumstances? I see little point in using the staging-next-2.6 tree to generate and test my patches since the tree eventually drains into linux-next. Should I stop worrying about if the patch works for the staging-next tree and just focus on that things should work with linux-next?

> and on linux-next 2010-0917:
>
> drivers/staging/ath6kl/os/linux/ar6000_drv.c: In function
> 'ar6000_transfer_bin_file':
> drivers/staging/ath6kl/os/linux/ar6000_drv.c:1129: warning: cast from
> pointer to integer of different size
> drivers/staging/ath6kl/os/linux/ar6000_drv.c:1129: warning: cast to
> pointer from integer of different size
> drivers/staging/ath6kl/wmi/wmi.c: In function 'wmi_bssInfo_event_rx':
> drivers/staging/ath6kl/wmi/wmi.c:1459: error: 'i' undeclared (first use in
> this function)
>
>
This is due to a recent patch that went it to use '%pM' format specifier to print MAC addresses. I'll fix it.

2010-09-17 00:13:20

by Vipin Mehta

[permalink] [raw]
Subject: RE: [PATCH] staging: ath6kl: Including a header file to fix a compilation error

You meant 70 characters. Right ?

> -----Original Message-----
> From: Luis R. Rodriguez [mailto:[email protected]]
> Sent: Thursday, September 16, 2010 5:11 PM
> To: Vipin Mehta
> Cc: [email protected]; [email protected];
> [email protected]
> Subject: Re: [PATCH] staging: ath6kl: Including a header file to fix a
> compilation error
>
> On Thu, Sep 16, 2010 at 5:08 PM, Vipin Mehta <[email protected]> wrote:
> > The commit fixes a compilation error that was encountered while using a
> specific kernel configuration file. The problem was the use of some
> fuunctions defined in <linux/semaphore.h> without including the header
> file explicitly. It was probably working before because of the dependency
> getting implicitly satisfied via some other header file. Also, eliminating
> the inclusion of the same header file more than once. The code needs
> additional cleanup and may be addressed by a subsequent commit.
>
> Keep the commit log lines to less than about 70 lines or so.
>
> Luis

2010-09-17 00:14:43

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH] staging: ath6kl: Including a header file to fix a compilation error

On Thu, Sep 16, 2010 at 05:08:07PM -0700, Vipin Mehta wrote:
> The commit fixes a compilation error that was encountered while using a specific kernel configuration file. The problem was the use of some fuunctions defined in <linux/semaphore.h> without including the header file explicitly. It was probably working before because of the dependency getting implicitly satisfied via some other header file. Also, eliminating the inclusion of the same header file more than once. The code needs additional cleanup and may be addressed by a subsequent commit.

Um, please wrap your lines at 72 columns.

>
> Signed-off-by: Vipin Mehta <[email protected]>

Did you forget to report who pointed out the problem to you? That needs
to go in a:
Reported-by: Foo <[email protected]>
line.

Care to redo this?

thanks,

greg k-h

2010-09-17 23:27:59

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH] staging: ath6kl: Including a header file to fix a compilation error

On Fri, Sep 17, 2010 at 04:11:57PM -0700, Vipin Mehta wrote:
> > Hi,
> > Thanks for the patch, but I still get these errors & warnings:
> >
> > on linux-next 2010-0915:
> >
> > drivers/staging/ath6kl/hif/sdio/linux_sdio/src/hif_scatter.c: In function
> > 'SetupHIFScatterSupport':
> > drivers/staging/ath6kl/hif/sdio/linux_sdio/src/hif_scatter.c:288: error:
> > 'struct mmc_host' has no member named 'max_hw_segs'
> > drivers/staging/ath6kl/hif/sdio/linux_sdio/src/hif_scatter.c:289: error:
> > 'struct mmc_host' has no member named 'max_hw_segs'
> >
> This is because of a recent change in the mmc_host structure that removes the distinction between hw and phys segments. The structure is different in the staging-next-2.6 tree which is where I usually build and test the changes.

Please, fix your email client to wrap lines at a sane place...

> Greg,
> The two trees linux-next and staging-next are not in sync so what is
> the process that is usually followed under these circumstances? I see
> little point in using the staging-next-2.6 tree to generate and test
> my patches since the tree eventually drains into linux-next. Should I
> stop worrying about if the patch works for the staging-next tree and
> just focus on that things should work with linux-next?

If the problem is due to another change in a different tree, send me the
patch and I'll hold on to it until the mmc tree is merged with Linus
(during the .37 merge window) and then I'll apply it and push it out.

Until then, we can ask Stephen to hold the patch in linux-next to fix
the build issue.

But I'd still like the .h file fixup patch to be resolved and in my tree
first, right?

thanks,

greg k-h