staging: mt7621-eth: Fix sparse warning in ethtool.c
Subsystems (2)
PATCH 1/1 staging: mt7621-eth: Fix sparse warning in ethtool.c
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?
--- 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(-)
drivers/staging/mt7621-eth/mtk_eth_soc.c +0 -1
@@ -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)
drivers/staging/mt7621-eth/mtk_eth_soc.h +1 -0
@@ -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
Signature · 1 line
2.11.0
Greg Kroah-Hartman
maintainerQuoted 19 lines · Chris Coffey
Include the local
ethtool.hheader file inmtk_eth_soc.hso 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 <[email protected]>--- 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(-)drivers/staging/mt7621-eth/mtk_eth_soc.c +0
-0
drivers/staging/mt7621-eth/mtk_eth_soc.c line 38
Quoted 4 lines
drivers/staging/mt7621-eth/mtk_eth_soc.c +0
-1@@ -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
PATCH v2 1/1 staging: mt7621-eth: Fix sparse warning in ethtool.c
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?
---
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(+)
drivers/staging/mt7621-eth/ethtool.c +1 -0
@@ -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,
Signature · 1 line
2.11.0
Quoted 6 lines · 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 <[email protected]>
Thanks,
NeilBrown
Quoted 24 lines
---
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 inethtool.c(which is that it didn't includeethtool.hanywhere --mtk_set_ethtool_opsis not static).drivers/staging/mt7621-eth/ethtool.c | 1 + 1 file changed, 1 insertion(+)drivers/staging/mt7621-eth/ethtool.c +1
-0@@ -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,Signature · 1 line
2.11.0
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 832 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180402/92da8c83/attachment.sig>
Quoted 7 lines · NeilBrown
Quoted 5 lines · 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?
Quoted 7 lines
Quoted 1 line
Signed-off-by Chris Coffey <[email protected]>Reviewed-by NeilBrown <[email protected]>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
Quoted 29 lines
Quoted 25 lines
---
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 inethtool.c(which is that it didn't includeethtool.hanywhere --mtk_set_ethtool_opsis not static).drivers/staging/mt7621-eth/ethtool.c | 1 + 1 file changed, 1 insertion(+)drivers/staging/mt7621-eth/ethtool.c +1
-0@@ -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,Signature · 1 line
2.11.0_______________________________________________
Linux-mediatek mailing list
Linux-mediatek at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek
Quoted 15 lines · Sean Wang
Quoted 7 lines · NeilBrown
Quoted 5 lines · 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?Quoted 7 lines
Quoted 1 line
Signed-off-by Chris Coffey <[email protected]>Reviewed-by NeilBrown <[email protected]>Thanks,
NeilBrown
Quoted 8 lines
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.
Quoted 5 lines
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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 832 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180402/75ddc73d/attachment.sig>
Quoted 12 lines · NeilBrown
Quoted 10 lines · Sean Wang
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.
Quoted 8 lines
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
Quoted 31 lines · Christian L?tke-Stetzkamp
Quoted 12 lines · NeilBrown
Quoted 10 lines · Sean Wang
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.
Quoted 8 lines
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,
NeilBrownHi,
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.
Quoted 4 lines
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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 832 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20180403/4a26d57f/attachment.sig>