* [PATCH] staging: mt7621-eth: Fix sparse warning in ethtool.c
@ 2018-03-28 21:18 Chris Coffey
2018-03-29 9:56 ` Greg Kroah-Hartman
0 siblings, 1 reply; 8+ messages in thread
From: Chris Coffey @ 2018-03-28 21:18 UTC (permalink / raw)
To: Greg Kroah-Hartman, NeilBrown, Matthias Brugger
Cc: devel, linux-arm-kernel, linux-mediatek, linux-kernel, Chris Coffey
Include the local ethtool.h header file in mtk_eth_soc.h so
implementation files have centralized access to it.
This fixes the following sparse warning:
drivers/staging/mt7621-eth/ethtool.c:213:6: warning: symbol
'mtk_set_ethtool_ops' was not declared. Should it be static?
Signed-off-by: Chris Coffey <cmc@babblebit.net>
---
drivers/staging/mt7621-eth/mtk_eth_soc.c | 1 -
drivers/staging/mt7621-eth/mtk_eth_soc.h | 1 +
2 files changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/mt7621-eth/mtk_eth_soc.c b/drivers/staging/mt7621-eth/mtk_eth_soc.c
index cbc7339843..0574e71573 100644
--- a/drivers/staging/mt7621-eth/mtk_eth_soc.c
+++ b/drivers/staging/mt7621-eth/mtk_eth_soc.c
@@ -35,7 +35,6 @@
#include "mtk_eth_soc.h"
#include "mdio.h"
-#include "ethtool.h"
#define MAX_RX_LENGTH 1536
#define MTK_RX_ETH_HLEN (VLAN_ETH_HLEN + VLAN_HLEN + ETH_FCS_LEN)
diff --git a/drivers/staging/mt7621-eth/mtk_eth_soc.h b/drivers/staging/mt7621-eth/mtk_eth_soc.h
index 443f88d8af..01deeddfdd 100644
--- a/drivers/staging/mt7621-eth/mtk_eth_soc.h
+++ b/drivers/staging/mt7621-eth/mtk_eth_soc.h
@@ -23,6 +23,7 @@
#include <linux/ethtool.h>
#include <linux/version.h>
#include <linux/atomic.h>
+#include "ethtool.h"
/* these registers have different offsets depending on the SoC. we use a lookup
* table for these
--
2.11.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] staging: mt7621-eth: Fix sparse warning in ethtool.c
2018-03-28 21:18 [PATCH] staging: mt7621-eth: Fix sparse warning in ethtool.c Chris Coffey
@ 2018-03-29 9:56 ` Greg Kroah-Hartman
2018-03-29 12:47 ` [PATCH v2] " Chris Coffey
0 siblings, 1 reply; 8+ messages in thread
From: Greg Kroah-Hartman @ 2018-03-29 9:56 UTC (permalink / raw)
To: Chris Coffey
Cc: NeilBrown, Matthias Brugger, devel, linux-arm-kernel,
linux-mediatek, linux-kernel
On Wed, Mar 28, 2018 at 10:18:48PM +0100, Chris Coffey wrote:
> Include the local ethtool.h header file in mtk_eth_soc.h so
> implementation files have centralized access to it.
>
> This fixes the following sparse warning:
>
> drivers/staging/mt7621-eth/ethtool.c:213:6: warning: symbol
> 'mtk_set_ethtool_ops' was not declared. Should it be static?
>
> Signed-off-by: Chris Coffey <cmc@babblebit.net>
> ---
> drivers/staging/mt7621-eth/mtk_eth_soc.c | 1 -
> drivers/staging/mt7621-eth/mtk_eth_soc.h | 1 +
> 2 files changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/staging/mt7621-eth/mtk_eth_soc.c b/drivers/staging/mt7621-eth/mtk_eth_soc.c
> index cbc7339843..0574e71573 100644
> --- a/drivers/staging/mt7621-eth/mtk_eth_soc.c
> +++ b/drivers/staging/mt7621-eth/mtk_eth_soc.c
> @@ -35,7 +35,6 @@
>
> #include "mtk_eth_soc.h"
> #include "mdio.h"
> -#include "ethtool.h"
How about just moving ethtool.h above the mtk_eth_soc.h include?
Putting .h file dependancies in other .h files is generally not a good
idea if at all possible. Keeping them "clean" is better, and this
driver has a bunch of work to go there, so let's not make it worse :)
thanks,
greg k-h
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2] staging: mt7621-eth: Fix sparse warning in ethtool.c
2018-03-29 9:56 ` Greg Kroah-Hartman
@ 2018-03-29 12:47 ` Chris Coffey
2018-04-01 23:34 ` NeilBrown
0 siblings, 1 reply; 8+ messages in thread
From: Chris Coffey @ 2018-03-29 12:47 UTC (permalink / raw)
To: Greg Kroah-Hartman, NeilBrown, Matthias Brugger
Cc: devel, linux-arm-kernel, linux-mediatek, linux-kernel, Chris Coffey
This fixes the following sparse warning:
drivers/staging/mt7621-eth/ethtool.c:213:6: warning: symbol
'mtk_set_ethtool_ops' was not declared. Should it be static?
Signed-off-by: Chris Coffey <cmc@babblebit.net>
Reviewed-by: NeilBrown <neil@brown.name>
---
Changes in v2:
- Per GregKH's feedback (thanks!), don't add unnecessary new .h file
dependencies. This patch version reverts those changes and fixes the
problem directly in ethtool.c (which is that it didn't include
ethtool.h anywhere -- mtk_set_ethtool_ops is not static).
drivers/staging/mt7621-eth/ethtool.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/staging/mt7621-eth/ethtool.c b/drivers/staging/mt7621-eth/ethtool.c
index 38ba0c040a..5268c5ca09 100644
--- a/drivers/staging/mt7621-eth/ethtool.c
+++ b/drivers/staging/mt7621-eth/ethtool.c
@@ -13,6 +13,7 @@
*/
#include "mtk_eth_soc.h"
+#include "ethtool.h"
static const char mtk_gdma_str[][ETH_GSTRING_LEN] = {
#define _FE(x...) # x,
--
2.11.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2] staging: mt7621-eth: Fix sparse warning in ethtool.c
2018-03-29 12:47 ` [PATCH v2] " Chris Coffey
@ 2018-04-01 23:34 ` NeilBrown
2018-04-02 3:04 ` Sean Wang
0 siblings, 1 reply; 8+ messages in thread
From: NeilBrown @ 2018-04-01 23:34 UTC (permalink / raw)
To: Chris Coffey, Greg Kroah-Hartman, Matthias Brugger
Cc: devel, Chris Coffey, linux-mediatek, linux-kernel, linux-arm-kernel
[-- Attachment #1.1: Type: text/plain, Size: 1148 bytes --]
On Thu, Mar 29 2018, Chris Coffey wrote:
> This fixes the following sparse warning:
>
> drivers/staging/mt7621-eth/ethtool.c:213:6: warning: symbol
> 'mtk_set_ethtool_ops' was not declared. Should it be static?
>
> Signed-off-by: Chris Coffey <cmc@babblebit.net>
Reviewed-by: NeilBrown <neil@brown.name>
Thanks,
NeilBrown
> ---
> Changes in v2:
> - Per GregKH's feedback (thanks!), don't add unnecessary new .h file
> dependencies. This patch version reverts those changes and fixes the
> problem directly in ethtool.c (which is that it didn't include
> ethtool.h anywhere -- mtk_set_ethtool_ops is not static).
>
> drivers/staging/mt7621-eth/ethtool.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/staging/mt7621-eth/ethtool.c b/drivers/staging/mt7621-eth/ethtool.c
> index 38ba0c040a..5268c5ca09 100644
> --- a/drivers/staging/mt7621-eth/ethtool.c
> +++ b/drivers/staging/mt7621-eth/ethtool.c
> @@ -13,6 +13,7 @@
> */
>
> #include "mtk_eth_soc.h"
> +#include "ethtool.h"
>
> static const char mtk_gdma_str[][ETH_GSTRING_LEN] = {
> #define _FE(x...) # x,
> --
> 2.11.0
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]
[-- Attachment #2: Type: text/plain, Size: 169 bytes --]
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] staging: mt7621-eth: Fix sparse warning in ethtool.c
2018-04-01 23:34 ` NeilBrown
@ 2018-04-02 3:04 ` Sean Wang
2018-04-02 3:41 ` NeilBrown
0 siblings, 1 reply; 8+ messages in thread
From: Sean Wang @ 2018-04-02 3:04 UTC (permalink / raw)
To: NeilBrown
Cc: Chris Coffey, Greg Kroah-Hartman, Matthias Brugger, devel,
linux-mediatek, linux-kernel, linux-arm-kernel
On Mon, 2018-04-02 at 09:34 +1000, NeilBrown wrote:
> On Thu, Mar 29 2018, Chris Coffey wrote:
>
> > This fixes the following sparse warning:
> >
> > drivers/staging/mt7621-eth/ethtool.c:213:6: warning: symbol
> > 'mtk_set_ethtool_ops' was not declared. Should it be static?
> >
> > Signed-off-by: Chris Coffey <cmc@babblebit.net>
>
> Reviewed-by: NeilBrown <neil@brown.name>
>
> Thanks,
> NeilBrown
>
Hi, Neil
Forgive me I cannot find the cover letter in the original series in my
mailbox to make a reply, so I rudely made here just letting you know
something good to the growth of mt7621 support in upstream.
do you have maintained an out-of-tree branch to boot the mt7621 machine
with those staging patches?
If so, it would become a bit easier for me that maybe I could give a
hand for migrating these staging driver for mt7621 to mainline. I
thought mmc, pci, ethernet, gsw and hsdma all could probably reuse the
current mainline code.
Sean
>
> > ---
> > Changes in v2:
> > - Per GregKH's feedback (thanks!), don't add unnecessary new .h file
> > dependencies. This patch version reverts those changes and fixes the
> > problem directly in ethtool.c (which is that it didn't include
> > ethtool.h anywhere -- mtk_set_ethtool_ops is not static).
> >
> > drivers/staging/mt7621-eth/ethtool.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/staging/mt7621-eth/ethtool.c b/drivers/staging/mt7621-eth/ethtool.c
> > index 38ba0c040a..5268c5ca09 100644
> > --- a/drivers/staging/mt7621-eth/ethtool.c
> > +++ b/drivers/staging/mt7621-eth/ethtool.c
> > @@ -13,6 +13,7 @@
> > */
> >
> > #include "mtk_eth_soc.h"
> > +#include "ethtool.h"
> >
> > static const char mtk_gdma_str[][ETH_GSTRING_LEN] = {
> > #define _FE(x...) # x,
> > --
> > 2.11.0
> _______________________________________________
> Linux-mediatek mailing list
> Linux-mediatek@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-mediatek
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] staging: mt7621-eth: Fix sparse warning in ethtool.c
2018-04-02 3:04 ` Sean Wang
@ 2018-04-02 3:41 ` NeilBrown
2018-04-02 11:41 ` Christian Lütke-Stetzkamp
0 siblings, 1 reply; 8+ messages in thread
From: NeilBrown @ 2018-04-02 3:41 UTC (permalink / raw)
To: Sean Wang
Cc: devel, Greg Kroah-Hartman, linux-kernel, linux-mediatek,
Matthias Brugger, Chris Coffey, linux-arm-kernel
[-- Attachment #1.1: Type: text/plain, Size: 1731 bytes --]
On Mon, Apr 02 2018, Sean Wang wrote:
> On Mon, 2018-04-02 at 09:34 +1000, NeilBrown wrote:
>> On Thu, Mar 29 2018, Chris Coffey wrote:
>>
>> > This fixes the following sparse warning:
>> >
>> > drivers/staging/mt7621-eth/ethtool.c:213:6: warning: symbol
>> > 'mtk_set_ethtool_ops' was not declared. Should it be static?
>> >
>> > Signed-off-by: Chris Coffey <cmc@babblebit.net>
>>
>> Reviewed-by: NeilBrown <neil@brown.name>
>>
>> Thanks,
>> NeilBrown
>>
> Hi, Neil
>
> Forgive me I cannot find the cover letter in the original series in my
> mailbox to make a reply, so I rudely made here just letting you know
> something good to the growth of mt7621 support in upstream.
>
> do you have maintained an out-of-tree branch to boot the mt7621 machine
> with those staging patches?
I can boot v4.16 plus staging-next on my mt7621 with no other patches.
An important fix (a63d706ea71919) landed in 4.16-rc7.
It do have
https://github.com/neilbrown/linux/commits/gnubee/v4.15
which was part of preparation for this, but now that very nearly all I
need is in mainline or on its way, I won't be doing much more in that
tree.
>
> If so, it would become a bit easier for me that maybe I could give a
> hand for migrating these staging driver for mt7621 to mainline. I
> thought mmc, pci, ethernet, gsw and hsdma all could probably reuse the
> current mainline code.
I agree that it is quite likely that several of these drivers could and
should reuse current mainline code. I would love to have some help
sorting this out. I won't have much time myself to dig into it for
several weeks, but I'll make time to review and test any code that is
contributed.
Thanks,
NeilBrown
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]
[-- Attachment #2: Type: text/plain, Size: 169 bytes --]
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] staging: mt7621-eth: Fix sparse warning in ethtool.c
2018-04-02 3:41 ` NeilBrown
@ 2018-04-02 11:41 ` Christian Lütke-Stetzkamp
2018-04-02 22:39 ` NeilBrown
0 siblings, 1 reply; 8+ messages in thread
From: Christian Lütke-Stetzkamp @ 2018-04-02 11:41 UTC (permalink / raw)
To: NeilBrown
Cc: devel, Greg Kroah-Hartman, Sean Wang, linux-kernel,
linux-mediatek, Matthias Brugger, Chris Coffey, linux-arm-kernel
On Mon, Apr 02, 2018 at 01:41:33PM +1000, NeilBrown wrote:
> On Mon, Apr 02 2018, Sean Wang wrote:
> > Hi, Neil
> >
> > Forgive me I cannot find the cover letter in the original series in my
> > mailbox to make a reply, so I rudely made here just letting you know
> > something good to the growth of mt7621 support in upstream.
> >
> > If so, it would become a bit easier for me that maybe I could give a
> > hand for migrating these staging driver for mt7621 to mainline. I
> > thought mmc, pci, ethernet, gsw and hsdma all could probably reuse the
> > current mainline code.
>
> I agree that it is quite likely that several of these drivers could and
> should reuse current mainline code. I would love to have some help
> sorting this out. I won't have much time myself to dig into it for
> several weeks, but I'll make time to review and test any code that is
> contributed.
>
> Thanks,
> NeilBrown
Hi,
I have started a patch series for the mmc driver that is currently in
staging, that makes some of the first cleanups (whitespace, indent,..)
and also starts to clean up the code, to make the similarities and
differences to the current mainline code clearer. The series is far
from being complete, but I think I'm going to send it out today, so
you can look and comment.
I would suggest that we continue cleaning the code that is currently
in staging to remove the dead parts and make the diff to the mainline
code obvious. Do you agree with that proposal or do you suggest a
different plan?
Christian
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] staging: mt7621-eth: Fix sparse warning in ethtool.c
2018-04-02 11:41 ` Christian Lütke-Stetzkamp
@ 2018-04-02 22:39 ` NeilBrown
0 siblings, 0 replies; 8+ messages in thread
From: NeilBrown @ 2018-04-02 22:39 UTC (permalink / raw)
To: Christian Lütke-Stetzkamp
Cc: devel, Greg Kroah-Hartman, Sean Wang, linux-kernel,
linux-mediatek, Matthias Brugger, Chris Coffey, linux-arm-kernel
[-- Attachment #1.1: Type: text/plain, Size: 1794 bytes --]
On Mon, Apr 02 2018, Christian Lütke-Stetzkamp wrote:
> On Mon, Apr 02, 2018 at 01:41:33PM +1000, NeilBrown wrote:
>> On Mon, Apr 02 2018, Sean Wang wrote:
>> > Hi, Neil
>> >
>> > Forgive me I cannot find the cover letter in the original series in my
>> > mailbox to make a reply, so I rudely made here just letting you know
>> > something good to the growth of mt7621 support in upstream.
>> >
>> > If so, it would become a bit easier for me that maybe I could give a
>> > hand for migrating these staging driver for mt7621 to mainline. I
>> > thought mmc, pci, ethernet, gsw and hsdma all could probably reuse the
>> > current mainline code.
>>
>> I agree that it is quite likely that several of these drivers could and
>> should reuse current mainline code. I would love to have some help
>> sorting this out. I won't have much time myself to dig into it for
>> several weeks, but I'll make time to review and test any code that is
>> contributed.
>>
>> Thanks,
>> NeilBrown
>
> Hi,
>
> I have started a patch series for the mmc driver that is currently in
> staging, that makes some of the first cleanups (whitespace, indent,..)
> and also starts to clean up the code, to make the similarities and
> differences to the current mainline code clearer. The series is far
> from being complete, but I think I'm going to send it out today, so
> you can look and comment.
>
> I would suggest that we continue cleaning the code that is currently
> in staging to remove the dead parts and make the diff to the mainline
> code obvious. Do you agree with that proposal or do you suggest a
> different plan?
I think this is an excellent plan - thanks for doing it.
I'll hopefully look over your patches today and send any comments.
Thanks,
NeilBrown
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]
[-- Attachment #2: Type: text/plain, Size: 169 bytes --]
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2018-04-02 22:39 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-28 21:18 [PATCH] staging: mt7621-eth: Fix sparse warning in ethtool.c Chris Coffey
2018-03-29 9:56 ` Greg Kroah-Hartman
2018-03-29 12:47 ` [PATCH v2] " Chris Coffey
2018-04-01 23:34 ` NeilBrown
2018-04-02 3:04 ` Sean Wang
2018-04-02 3:41 ` NeilBrown
2018-04-02 11:41 ` Christian Lütke-Stetzkamp
2018-04-02 22:39 ` NeilBrown
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).