Currently when none of CONFIG_NET_DSA_TAG_DSA, CONFIG_NET_DSA_TAG_EDSA and
CONFIG_NET_DSA_TAG_TRAILER is defined, we get following compilation warnings:
net/dsa/slave.c:51:12: warning: 'dsa_slave_init' defined but not used [-Wunused-function]
net/dsa/slave.c:60:12: warning: 'dsa_slave_open' defined but not used [-Wunused-function]
net/dsa/slave.c:98:12: warning: 'dsa_slave_close' defined but not used [-Wunused-function]
net/dsa/slave.c:116:13: warning: 'dsa_slave_change_rx_flags' defined but not used [-Wunused-function]
net/dsa/slave.c:127:13: warning: 'dsa_slave_set_rx_mode' defined but not used [-Wunused-function]
net/dsa/slave.c:136:12: warning: 'dsa_slave_set_mac_address' defined but not used [-Wunused-function]
net/dsa/slave.c:164:12: warning: 'dsa_slave_ioctl' defined but not used [-Wunused-function]
Fix them by enclosing these routines under #ifdef,endif.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
net/dsa/slave.c | 4 ++++
1 file changed, 4 insertions(+)
From: Ben Hutchings <hidden> Date: 2012-10-29 19:59:46
On Mon, 2012-10-29 at 22:27 +0530, Viresh Kumar wrote:
Currently when none of CONFIG_NET_DSA_TAG_DSA, CONFIG_NET_DSA_TAG_EDSA and
CONFIG_NET_DSA_TAG_TRAILER is defined, we get following compilation warnings:
net/dsa/slave.c:51:12: warning: 'dsa_slave_init' defined but not used [-Wunused-function]
net/dsa/slave.c:60:12: warning: 'dsa_slave_open' defined but not used [-Wunused-function]
net/dsa/slave.c:98:12: warning: 'dsa_slave_close' defined but not used [-Wunused-function]
net/dsa/slave.c:116:13: warning: 'dsa_slave_change_rx_flags' defined but not used [-Wunused-function]
net/dsa/slave.c:127:13: warning: 'dsa_slave_set_rx_mode' defined but not used [-Wunused-function]
net/dsa/slave.c:136:12: warning: 'dsa_slave_set_mac_address' defined but not used [-Wunused-function]
net/dsa/slave.c:164:12: warning: 'dsa_slave_ioctl' defined but not used [-Wunused-function]
Fix them by enclosing these routines under #ifdef,endif.
[...]
This is not a useful configuration. It might make more sense to make
NET_DSA a hidden option and have the DSA drivers (in drivers/net/dsa)
select it rather than depending on it.
Ben.
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
On 30 October 2012 01:29, Ben Hutchings [off-list ref] wrote:
On Mon, 2012-10-29 at 22:27 +0530, Viresh Kumar wrote:
quoted
Currently when none of CONFIG_NET_DSA_TAG_DSA, CONFIG_NET_DSA_TAG_EDSA and
CONFIG_NET_DSA_TAG_TRAILER is defined, we get following compilation warnings:
net/dsa/slave.c:51:12: warning: 'dsa_slave_init' defined but not used [-Wunused-function]
net/dsa/slave.c:60:12: warning: 'dsa_slave_open' defined but not used [-Wunused-function]
net/dsa/slave.c:98:12: warning: 'dsa_slave_close' defined but not used [-Wunused-function]
net/dsa/slave.c:116:13: warning: 'dsa_slave_change_rx_flags' defined but not used [-Wunused-function]
net/dsa/slave.c:127:13: warning: 'dsa_slave_set_rx_mode' defined but not used [-Wunused-function]
net/dsa/slave.c:136:12: warning: 'dsa_slave_set_mac_address' defined but not used [-Wunused-function]
net/dsa/slave.c:164:12: warning: 'dsa_slave_ioctl' defined but not used [-Wunused-function]
Fix them by enclosing these routines under #ifdef,endif.
[...]
This is not a useful configuration. It might make more sense to make
NET_DSA a hidden option and have the DSA drivers (in drivers/net/dsa)
select it rather than depending on it.
I don't have any idea about net/dsa/***. I just wanted to fix this
awkward looking warning :)
What i understood from your comment is: Atleast one of the tagging
formats must be
always enabled if we want to use net/dsa/ stuff ??
And so the functions i have enclosed under ifdefs will always be used
if net/dsa/ is
used.
And so, if we select NET_DSA from these tagging drivers, then only slave.c will
get compiled. Otherwise slave.c dsa.c dsa_core.c wouldn't be compiled and so
no warnings.
Correct??
--
viresh
On 30 October 2012 12:15, Viresh Kumar [off-list ref] wrote:
And so, if we select NET_DSA from these tagging drivers, then only slave.c will
get compiled. Otherwise slave.c dsa.c dsa_core.c wouldn't be compiled and so
no warnings.
If my above explanation/assumption is correct, then please review following
patch:
--------------------------x-----------------------x----------------
From: Viresh Kumar <viresh.kumar@linaro.org>
Date: Mon, 29 Oct 2012 22:19:14 +0530
Subject: [PATCH] net: dsa/slave: Fix compilation warnings
Currently when none of CONFIG_NET_DSA_TAG_DSA, CONFIG_NET_DSA_TAG_EDSA and
CONFIG_NET_DSA_TAG_TRAILER is defined, we get following compilation warnings:
net/dsa/slave.c:51:12: warning: 'dsa_slave_init' defined but not used
[-Wunused-function]
net/dsa/slave.c:60:12: warning: 'dsa_slave_open' defined but not used
[-Wunused-function]
net/dsa/slave.c:98:12: warning: 'dsa_slave_close' defined but not used
[-Wunused-function]
net/dsa/slave.c:116:13: warning: 'dsa_slave_change_rx_flags' defined
but not used [-Wunused-function]
net/dsa/slave.c:127:13: warning: 'dsa_slave_set_rx_mode' defined but
not used [-Wunused-function]
net/dsa/slave.c:136:12: warning: 'dsa_slave_set_mac_address' defined
but not used [-Wunused-function]
net/dsa/slave.c:164:12: warning: 'dsa_slave_ioctl' defined but not
used [-Wunused-function]
Earlier approach to fix this was discussed here:
lkml.org/lkml/2012/10/29/549
This is another approach to fix it. This is done by some changes in config
options, which make more sense than the earlier approach. As, atleast one
tagging option must always be selected for using net/dsa/ infrastructure, this
patch selects NET_DSA from tagging configs instead of having it as an selectable
config.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
drivers/net/dsa/Kconfig | 1 -
net/dsa/Kconfig | 16 +++++++++-------
2 files changed, 9 insertions(+), 8 deletions(-)
From: Daniel Lezcano <hidden> Date: 2012-10-30 07:53:37
On 10/30/2012 08:31 AM, Viresh Kumar wrote:
quoted hunk
On 30 October 2012 12:15, Viresh Kumar [off-list ref] wrote:
quoted
And so, if we select NET_DSA from these tagging drivers, then only slave.c will
get compiled. Otherwise slave.c dsa.c dsa_core.c wouldn't be compiled and so
no warnings.
If my above explanation/assumption is correct, then please review following
patch:
--------------------------x-----------------------x----------------
From: Viresh Kumar <viresh.kumar@linaro.org>
Date: Mon, 29 Oct 2012 22:19:14 +0530
Subject: [PATCH] net: dsa/slave: Fix compilation warnings
Currently when none of CONFIG_NET_DSA_TAG_DSA, CONFIG_NET_DSA_TAG_EDSA and
CONFIG_NET_DSA_TAG_TRAILER is defined, we get following compilation warnings:
net/dsa/slave.c:51:12: warning: 'dsa_slave_init' defined but not used
[-Wunused-function]
net/dsa/slave.c:60:12: warning: 'dsa_slave_open' defined but not used
[-Wunused-function]
net/dsa/slave.c:98:12: warning: 'dsa_slave_close' defined but not used
[-Wunused-function]
net/dsa/slave.c:116:13: warning: 'dsa_slave_change_rx_flags' defined
but not used [-Wunused-function]
net/dsa/slave.c:127:13: warning: 'dsa_slave_set_rx_mode' defined but
not used [-Wunused-function]
net/dsa/slave.c:136:12: warning: 'dsa_slave_set_mac_address' defined
but not used [-Wunused-function]
net/dsa/slave.c:164:12: warning: 'dsa_slave_ioctl' defined but not
used [-Wunused-function]
Earlier approach to fix this was discussed here:
lkml.org/lkml/2012/10/29/549
This is another approach to fix it. This is done by some changes in config
options, which make more sense than the earlier approach. As, atleast one
tagging option must always be selected for using net/dsa/ infrastructure, this
patch selects NET_DSA from tagging configs instead of having it as an selectable
config.
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
drivers/net/dsa/Kconfig | 1 -
net/dsa/Kconfig | 16 +++++++++-------
2 files changed, 9 insertions(+), 8 deletions(-)
Unbelievable mistake :(
Will fix it after some reviews now :)
Yeah, has to be ! ;)
It is very curious if we disable all the configs option, a slave
creation raise a BUG (cf. dsa_slave_create). IIUC, booting with NET_DSA
enabled and none of the NET_DSA_TAG* enabled will raise a BUG in the
probe function, right ?
Maybe we should force at least one config when none are set ?
!NET_DSA_TAG_TRAILER)
---help---
This allows you to use hardware switch chips that use
the Distributed Switch Architecture.
@@ -12,15 +13,15 @@ if NET_DSA # tagging formats config NET_DSA_TAG_DSA- bool+ bool "tag dsa" default n config NET_DSA_TAG_EDSA- bool+ bool "tag edsa" default n config NET_DSA_TAG_TRAILER- bool+ bool "tag trailer" default n endif
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
On 30 October 2012 14:30, Daniel Lezcano [off-list ref] wrote:
It is very curious if we disable all the configs option, a slave
creation raise a BUG (cf. dsa_slave_create). IIUC, booting with NET_DSA
enabled and none of the NET_DSA_TAG* enabled will raise a BUG in the
probe function, right ?
Maybe we should force at least one config when none are set ?
I thought of it earlier. But found the one which i posted better. As we will not
compile DSA stuff now without these tags.
--
viresh
From: Daniel Lezcano <hidden> Date: 2012-10-30 09:27:01
On 10/30/2012 10:03 AM, Viresh Kumar wrote:
On 30 October 2012 14:30, Daniel Lezcano [off-list ref] wrote:
quoted
It is very curious if we disable all the configs option, a slave
creation raise a BUG (cf. dsa_slave_create). IIUC, booting with NET_DSA
enabled and none of the NET_DSA_TAG* enabled will raise a BUG in the
probe function, right ?
Maybe we should force at least one config when none are set ?
I thought of it earlier. But found the one which i posted better. As we will not
compile DSA stuff now without these tags.
Well, it is the same here, no ? Except, it is up to the user to disable
the option.
On 30 October 2012 14:56, Daniel Lezcano [off-list ref] wrote:
Well, it is the same here, no ? Except, it is up to the user to disable
the option.
I didn't wanted to add code like this:
quoted
+ select NET_DSA_TAG_DSA if (!NET_DSA_TAG_EDSA &&
!NET_DSA_TAG_TRAILER)
Why should we select NET_DSA_TAG_DSA by default? What made us
select that?
For this reason, i went to the solution i offered.
Anyway, i just wanted to get rid of the warning. Whatever maintainer
feels is better must be selected.
--
viresh