| Message ID | 1480925763-20254-1-git-send-email-nikita.yoush@cogentembedded.com |
|---|---|
| State | Superseded, archived |
| Delegated to: | David Miller |
| Headers | show |
From: Nikita Yushchenko <nikita.yoush@cogentembedded.com> Sent: Monday, December 05, 2016 4:16 PM >To: David S. Miller <davem@davemloft.net>; Andy Duan ><fugang.duan@nxp.com>; Troy Kisky <troy.kisky@boundarydevices.com>; >Andrew Lunn <andrew@lunn.ch>; Eric Nelson <eric@nelint.com>; Philippe >Reynes <tremyfr@gmail.com>; Johannes Berg <johannes@sipsolutions.net>; >netdev@vger.kernel.org >Cc: Chris Healy <cphealy@gmail.com>; Fabio Estevam ><fabio.estevam@nxp.com>; linux-kernel@vger.kernel.org; Nikita >Yushchenko <nikita.yoush@cogentembedded.com> >Subject: [patch net v2] net: fec: fix compile with CONFIG_M5272 > >Commit 4dfb80d18d05 ("net: fec: cache statistics while device is down") >introduced unconditional statistics-related actions. > >However, when driver is compiled with CONFIG_M5272, staticsics-related >definitions do not exist, which results into build errors. > >Fix that by adding explicit handling of !defined(CONFIG_M5272) case. > >Fixes: 4dfb80d18d05 ("net: fec: cache statistics while device is down") >Signed-off-by: Nikita Yushchenko <nikita.yoush@cogentembedded.com> >--- >Changes since v1: >- instead of #ifdef'ing calls to fec_enet_update_ethtool_stats(), add > definition of empty fec_enet_update_ethtool_stats() for CONFIG_M5272 > case, >- add FEC_STATS_SIZE macro to avoid #ifdef in the middle of > alloc_etherdev_mqs() args. > > drivers/net/ethernet/freescale/fec_main.c | 13 ++++++++++--- > 1 file changed, 10 insertions(+), 3 deletions(-) > Acked-by: Fugang Duan <fugang.duan@nxp.com> >diff --git a/drivers/net/ethernet/freescale/fec_main.c >b/drivers/net/ethernet/freescale/fec_main.c >index 5f77caa59534..741cf4a57bfc 100644 >--- a/drivers/net/ethernet/freescale/fec_main.c >+++ b/drivers/net/ethernet/freescale/fec_main.c >@@ -2313,6 +2313,8 @@ static const struct fec_stat { > { "IEEE_rx_octets_ok", IEEE_R_OCTETS_OK }, }; > >+#define FEC_STATS_SIZE (ARRAY_SIZE(fec_stats) * sizeof(u64)) >+ > static void fec_enet_update_ethtool_stats(struct net_device *dev) { > struct fec_enet_private *fep = netdev_priv(dev); @@ -2330,7 >+2332,7 @@ static void fec_enet_get_ethtool_stats(struct net_device *dev, > if (netif_running(dev)) > fec_enet_update_ethtool_stats(dev); > >- memcpy(data, fep->ethtool_stats, ARRAY_SIZE(fec_stats) * >sizeof(u64)); >+ memcpy(data, fep->ethtool_stats, FEC_STATS_SIZE); > } > > static void fec_enet_get_strings(struct net_device *netdev, @@ -2355,6 >+2357,12 @@ static int fec_enet_get_sset_count(struct net_device *dev, int >sset) > return -EOPNOTSUPP; > } > } >+ >+#else /* !defined(CONFIG_M5272) */ >+#define FEC_STATS_SIZE 0 >+static inline void fec_enet_update_ethtool_stats(struct net_device >+*dev) { } > #endif /* !defined(CONFIG_M5272) */ > > static int fec_enet_nway_reset(struct net_device *dev) @@ -3293,8 >+3301,7 @@ fec_probe(struct platform_device *pdev) > > /* Init network device */ > ndev = alloc_etherdev_mqs(sizeof(struct fec_enet_private) + >- ARRAY_SIZE(fec_stats) * sizeof(u64), >- num_tx_qs, num_rx_qs); >+ FEC_STAT_SIZE, num_tx_qs, num_rx_qs); > if (!ndev) > return -ENOMEM; > >-- >2.1.4
On Mon, Dec 5, 2016 at 9:16 AM, Nikita Yushchenko <nikita.yoush@cogentembedded.com> wrote: > Commit 4dfb80d18d05 ("net: fec: cache statistics while device is down") > introduced unconditional statistics-related actions. > > However, when driver is compiled with CONFIG_M5272, staticsics-related > definitions do not exist, which results into build errors. > > Fix that by adding explicit handling of !defined(CONFIG_M5272) case. > > Fixes: 4dfb80d18d05 ("net: fec: cache statistics while device is down") > Signed-off-by: Nikita Yushchenko <nikita.yoush@cogentembedded.com> Reviewed-by: Geert Uytterhoeven <geert@linux-m68k.org> Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
On Mon, Dec 5, 2016 at 6:16 AM, Nikita Yushchenko <nikita.yoush@cogentembedded.com> wrote: > Commit 4dfb80d18d05 ("net: fec: cache statistics while device is down") > introduced unconditional statistics-related actions. > > However, when driver is compiled with CONFIG_M5272, staticsics-related > definitions do not exist, which results into build errors. > > Fix that by adding explicit handling of !defined(CONFIG_M5272) case. > > Fixes: 4dfb80d18d05 ("net: fec: cache statistics while device is down") > Signed-off-by: Nikita Yushchenko <nikita.yoush@cogentembedded.com> Looks better now: Reviewed-by: Fabio Estevam <fabio.estevam@nxp.com>
Hi Nikita,
[auto build test ERROR on net/master]
url: https://github.com/0day-ci/linux/commits/Nikita-Yushchenko/net-fec-fix-compile-with-CONFIG_M5272/20161205-181735
config: arm-multi_v5_defconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=arm
All errors (new ones prefixed by >>):
drivers/net/ethernet/freescale/fec_main.c: In function 'fec_probe':
>> drivers/net/ethernet/freescale/fec_main.c:3304:7: error: 'FEC_STAT_SIZE' undeclared (first use in this function)
FEC_STAT_SIZE, num_tx_qs, num_rx_qs);
^~~~~~~~~~~~~
drivers/net/ethernet/freescale/fec_main.c:3304:7: note: each undeclared identifier is reported only once for each function it appears in
vim +/FEC_STAT_SIZE +3304 drivers/net/ethernet/freescale/fec_main.c
3298 int num_rx_qs;
3299
3300 fec_enet_get_queue_num(pdev, &num_tx_qs, &num_rx_qs);
3301
3302 /* Init network device */
3303 ndev = alloc_etherdev_mqs(sizeof(struct fec_enet_private) +
> 3304 FEC_STAT_SIZE, num_tx_qs, num_rx_qs);
3305 if (!ndev)
3306 return -ENOMEM;
3307
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
Aieee I was typing too fast today, sorry... send separate "fix for the fix", or re-send patch without that silly typo? Nikita > Hi Nikita, > > [auto build test ERROR on net/master] > > url: https://github.com/0day-ci/linux/commits/Nikita-Yushchenko/net-fec-fix-compile-with-CONFIG_M5272/20161205-181735 > config: arm-multi_v5_defconfig (attached as .config) > compiler: arm-linux-gnueabi-gcc (Debian 6.1.1-9) 6.1.1 20160705 > reproduce: > wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross > chmod +x ~/bin/make.cross > # save the attached .config to linux build tree > make.cross ARCH=arm > > All errors (new ones prefixed by >>): > > drivers/net/ethernet/freescale/fec_main.c: In function 'fec_probe': >>> drivers/net/ethernet/freescale/fec_main.c:3304:7: error: 'FEC_STAT_SIZE' undeclared (first use in this function) > FEC_STAT_SIZE, num_tx_qs, num_rx_qs); > ^~~~~~~~~~~~~ > drivers/net/ethernet/freescale/fec_main.c:3304:7: note: each undeclared identifier is reported only once for each function it appears in > > vim +/FEC_STAT_SIZE +3304 drivers/net/ethernet/freescale/fec_main.c > > 3298 int num_rx_qs; > 3299 > 3300 fec_enet_get_queue_num(pdev, &num_tx_qs, &num_rx_qs); > 3301 > 3302 /* Init network device */ > 3303 ndev = alloc_etherdev_mqs(sizeof(struct fec_enet_private) + >> 3304 FEC_STAT_SIZE, num_tx_qs, num_rx_qs); > 3305 if (!ndev) > 3306 return -ENOMEM; > 3307 > > --- > 0-DAY kernel test infrastructure Open Source Technology Center > https://lists.01.org/pipermail/kbuild-all Intel Corporation >
From: Nikita Yushchenko <nikita.yoush@cogentembedded.com> Date: Mon, 5 Dec 2016 16:55:04 +0300 > Aieee I was typing too fast today, sorry... > > send separate "fix for the fix", or re-send patch without that silly typo? If the patch hasn't been applied yet, you resend a fixed version of the patch, always.
> From: Nikita Yushchenko <nikita.yoush@cogentembedded.com> > Date: Mon, 5 Dec 2016 16:55:04 +0300 > >> Aieee I was typing too fast today, sorry... >> >> send separate "fix for the fix", or re-send patch without that silly typo? > > If the patch hasn't been applied yet, you resend a fixed version of the > patch, always. Ok, will repost shortly. What I don't understand is - how could test robot fetch it before it was applied? Nikita
From: Nikita Yushchenko <nikita.yoush@cogentembedded.com> Date: Mon, 5 Dec 2016 20:26:52 +0300 >> From: Nikita Yushchenko <nikita.yoush@cogentembedded.com> >> Date: Mon, 5 Dec 2016 16:55:04 +0300 >> >>> Aieee I was typing too fast today, sorry... >>> >>> send separate "fix for the fix", or re-send patch without that silly typo? >> >> If the patch hasn't been applied yet, you resend a fixed version of the >> patch, always. > > Ok, will repost shortly. > > What I don't understand is - how could test robot fetch it before it was > applied? It takes them from the mailing list, and exactly this is the value of the robot. It can test patches before I add them to my tree.
diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index 5f77caa59534..741cf4a57bfc 100644 --- a/drivers/net/ethernet/freescale/fec_main.c +++ b/drivers/net/ethernet/freescale/fec_main.c @@ -2313,6 +2313,8 @@ static const struct fec_stat { { "IEEE_rx_octets_ok", IEEE_R_OCTETS_OK }, }; +#define FEC_STATS_SIZE (ARRAY_SIZE(fec_stats) * sizeof(u64)) + static void fec_enet_update_ethtool_stats(struct net_device *dev) { struct fec_enet_private *fep = netdev_priv(dev); @@ -2330,7 +2332,7 @@ static void fec_enet_get_ethtool_stats(struct net_device *dev, if (netif_running(dev)) fec_enet_update_ethtool_stats(dev); - memcpy(data, fep->ethtool_stats, ARRAY_SIZE(fec_stats) * sizeof(u64)); + memcpy(data, fep->ethtool_stats, FEC_STATS_SIZE); } static void fec_enet_get_strings(struct net_device *netdev, @@ -2355,6 +2357,12 @@ static int fec_enet_get_sset_count(struct net_device *dev, int sset) return -EOPNOTSUPP; } } + +#else /* !defined(CONFIG_M5272) */ +#define FEC_STATS_SIZE 0 +static inline void fec_enet_update_ethtool_stats(struct net_device *dev) +{ +} #endif /* !defined(CONFIG_M5272) */ static int fec_enet_nway_reset(struct net_device *dev) @@ -3293,8 +3301,7 @@ fec_probe(struct platform_device *pdev) /* Init network device */ ndev = alloc_etherdev_mqs(sizeof(struct fec_enet_private) + - ARRAY_SIZE(fec_stats) * sizeof(u64), - num_tx_qs, num_rx_qs); + FEC_STAT_SIZE, num_tx_qs, num_rx_qs); if (!ndev) return -ENOMEM;
Commit 4dfb80d18d05 ("net: fec: cache statistics while device is down") introduced unconditional statistics-related actions. However, when driver is compiled with CONFIG_M5272, staticsics-related definitions do not exist, which results into build errors. Fix that by adding explicit handling of !defined(CONFIG_M5272) case. Fixes: 4dfb80d18d05 ("net: fec: cache statistics while device is down") Signed-off-by: Nikita Yushchenko <nikita.yoush@cogentembedded.com> --- Changes since v1: - instead of #ifdef'ing calls to fec_enet_update_ethtool_stats(), add definition of empty fec_enet_update_ethtool_stats() for CONFIG_M5272 case, - add FEC_STATS_SIZE macro to avoid #ifdef in the middle of alloc_etherdev_mqs() args. drivers/net/ethernet/freescale/fec_main.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-)