2016-12-05 08:16:15

by Nikita Yushchenko

[permalink] [raw]
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 <[email protected]>
---
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(-)

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


2016-12-05 09:19:00

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [patch net v2] net: fec: fix compile with CONFIG_M5272

On Mon, Dec 5, 2016 at 9:16 AM, Nikita Yushchenko
<[email protected]> 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 <[email protected]>

Reviewed-by: Geert Uytterhoeven <[email protected]>

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

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

2016-12-05 10:02:29

by Fabio Estevam

[permalink] [raw]
Subject: Re: [patch net v2] net: fec: fix compile with CONFIG_M5272

On Mon, Dec 5, 2016 at 6:16 AM, Nikita Yushchenko
<[email protected]> 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 <[email protected]>

Looks better now:

Reviewed-by: Fabio Estevam <[email protected]>

2016-12-05 10:59:01

by Andy Duan

[permalink] [raw]
Subject: RE: [patch net v2] net: fec: fix compile with CONFIG_M5272

From: Nikita Yushchenko <[email protected]> Sent: Monday, December 05, 2016 4:16 PM
>To: David S. Miller <[email protected]>; Andy Duan
><[email protected]>; Troy Kisky <[email protected]>;
>Andrew Lunn <[email protected]>; Eric Nelson <[email protected]>; Philippe
>Reynes <[email protected]>; Johannes Berg <[email protected]>;
>[email protected]
>Cc: Chris Healy <[email protected]>; Fabio Estevam
><[email protected]>; [email protected]; Nikita
>Yushchenko <[email protected]>
>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 <[email protected]>
>---
>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 <[email protected]>

>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

2016-12-05 13:19:01

by kernel test robot

[permalink] [raw]
Subject: Re: [patch net v2] net: fec: fix compile with CONFIG_M5272

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


Attachments:
(No filename) (1.48 kB)
.config.gz (27.23 kB)
Download all attachments

2016-12-05 14:03:50

by Nikita Yushchenko

[permalink] [raw]
Subject: Re: [patch net v2] net: fec: fix compile with CONFIG_M5272

diff --git a/drivers/net/ethernet/freescale/fec_main.c
b/drivers/net/ethernet/freescale/fec_main.c
index 741cf4a57bfc..12aef1b15356 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -3301,7 +3301,7 @@ fec_probe(struct platform_device *pdev)

/* Init network device */
ndev = alloc_etherdev_mqs(sizeof(struct fec_enet_private) +
- FEC_STAT_SIZE, num_tx_qs, num_rx_qs);
+ FEC_STATS_SIZE, num_tx_qs, num_rx_qs);
if (!ndev)
return -ENOMEM;


> 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
>>

2016-12-05 14:05:03

by Nikita Yushchenko

[permalink] [raw]
Subject: Re: [patch net v2] net: fec: fix compile with CONFIG_M5272

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
>

2016-12-05 17:14:13

by David Miller

[permalink] [raw]
Subject: Re: [patch net v2] net: fec: fix compile with CONFIG_M5272

From: Nikita Yushchenko <[email protected]>
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.

2016-12-05 17:27:14

by Nikita Yushchenko

[permalink] [raw]
Subject: Re: [patch net v2] net: fec: fix compile with CONFIG_M5272

> From: Nikita Yushchenko <[email protected]>
> 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

2016-12-05 17:28:25

by David Miller

[permalink] [raw]
Subject: Re: [patch net v2] net: fec: fix compile with CONFIG_M5272

From: Nikita Yushchenko <[email protected]>
Date: Mon, 5 Dec 2016 20:26:52 +0300

>> From: Nikita Yushchenko <[email protected]>
>> 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.