Re: [PATCH -next] net: dsa: mv88e6xxx: Fix build warning when CONFIG_NET_DSA_LEGACY is n
Hi Andrew,
On 2019/3/4 22:57, Andrew Lunn wrote:
> On Mon, Mar 04, 2019 at 10:16:08PM +0800, Zhangshaokun wrote:
>> Hi Andrew,
>>
>> On 2019/3/4 21:26, Andrew Lunn wrote:
>>> On Mon, Mar 04, 2019 at 08:43:01PM +0800, Shaokun Zhang wrote:
When CONFIG_NET_DSA_LEGACY is n, there is a GCC bulid warning:
drivers/net/dsa/mv88e6xxx/chip.c:4623:13: warning:
‘mv88e6xxx_ports_cmode_init’ defined but not used [-Wunused-function]
static void mv88e6xxx_ports_cmode_init(struct mv88e6xxx_chip *chip)
Let's fix it.
>>>
>>> Hi Shaokun, Heiner
>>>
>>> Although this fixes the warning, i suspect there i something wrong
>>> with the original patch adding mv88e6390x_port_set_cmode(). It should
>>> also be used without CONFIG_NET_DSA_LEGACY.
>>
>> I checked the commit-id 2a93c1a3651f ("net: dsa: Allow compiling out legacy
>> support") by Florian.
>> Do you mean that CONFIG_NET_DSA_LEGACY shall be removed completely? :-)
>
> No, i suspect mv88e6390x_ports_cmode_init() is being called from the
> wrong place, or needs to be called from a second location.
>
Ok, I didn't check it carefully.
> [Goes and looks at the code]
>
> Yes, it should also be called in mv88e6xxx_probe(). I would call it
> just after the call to mv88e6xxx_detect(), so that it is the same as
> in mv88e6xxx_drv_probe().
>
> There are two ways DSA drivers can be probed. The legacy way, which is
> optional, and is slowly getting removed, and the current way. Heiner
> is new to DSA and probably missed that, and only handled the legacy
> probe method. I also missed checking when i reviewed to patch :-(
>
Hmm, I am not familiar wit it, please feel free to fix it.
Thanks,
Shaokun
>Andrew
>
>
>
>
> .
>
Re: [PATCH -next] net: dsa: mv88e6xxx: Fix build warning when CONFIG_NET_DSA_LEGACY is n
On 3/4/19 10:31 AM, Heiner Kallweit wrote:
> On 04.03.2019 19:24, Florian Fainelli wrote:
>> On 3/4/19 10:18 AM, Heiner Kallweit wrote:
>>> On 04.03.2019 15:57, Andrew Lunn wrote:
On Mon, Mar 04, 2019 at 10:16:08PM +0800, Zhangshaokun wrote:
> Hi Andrew,
>
> On 2019/3/4 21:26, Andrew Lunn wrote:
>> On Mon, Mar 04, 2019 at 08:43:01PM +0800, Shaokun Zhang wrote:
>>> When CONFIG_NET_DSA_LEGACY is n, there is a GCC bulid warning:
>>> drivers/net/dsa/mv88e6xxx/chip.c:4623:13: warning:
>>> ‘mv88e6xxx_ports_cmode_init’ defined but not used [-Wunused-function]
>>> static void mv88e6xxx_ports_cmode_init(struct mv88e6xxx_chip *chip)
>>> Let's fix it.
>>
>> Hi Shaokun, Heiner
>>
>> Although this fixes the warning, i suspect there i something wrong
>> with the original patch adding mv88e6390x_port_set_cmode(). It should
>> also be used without CONFIG_NET_DSA_LEGACY.
>
> I checked the commit-id 2a93c1a3651f ("net: dsa: Allow compiling out
> legacy support") by Florian.
> Do you mean that CONFIG_NET_DSA_LEGACY shall be removed completely? :-)
No, i suspect mv88e6390x_ports_cmode_init() is being called from the
wrong place, or needs to be called from a second location.
[Goes and looks at the code]
Yes, it should also be called in mv88e6xxx_probe(). I would call it
just after the call to mv88e6xxx_detect(), so that it is the same as
in mv88e6xxx_drv_probe().
There are two ways DSA drivers can be probed. The legacy way, which is
optional, and is slowly getting removed, and the current way. Heiner
is new to DSA and probably missed that, and only handled the legacy
probe method. I also missed checking when i reviewed to patch :-(
>>> Right, I missed that, will submit a fix.
>>>
>>> I just saw that the Kconfig entry comment for NET_DSA_LEGACY says:
>>> "This feature is scheduled for removal in 4.17."
>>>
>>> Was forgotten to remove it or did somebody scream loud enough
>>> "But I depend on it" ?
>>
>> The intent was to remove it by that kernel version but the 88e6060
>> driver still depends on it, and there appears to be some active users
>> that Andrew worked with.
>>
> I see, thanks. And migrating this driver to the new DSA framework
> version isn't possible or not worth the effort?
Andrew is working on it actually. We could have 88E6060 select
NET_DSA_LEGACY and drop legacy probing from mv88e6xxx as an in between
solution.
--
Florian
Re: [PATCH -next] net: dsa: mv88e6xxx: Fix build warning when CONFIG_NET_DSA_LEGACY is n
On 04.03.2019 19:24, Florian Fainelli wrote:
> On 3/4/19 10:18 AM, Heiner Kallweit wrote:
>> On 04.03.2019 15:57, Andrew Lunn wrote:
>>> On Mon, Mar 04, 2019 at 10:16:08PM +0800, Zhangshaokun wrote:
Hi Andrew,
On 2019/3/4 21:26, Andrew Lunn wrote:
> On Mon, Mar 04, 2019 at 08:43:01PM +0800, Shaokun Zhang wrote:
>> When CONFIG_NET_DSA_LEGACY is n, there is a GCC bulid warning:
>> drivers/net/dsa/mv88e6xxx/chip.c:4623:13: warning:
>> ‘mv88e6xxx_ports_cmode_init’ defined but not used [-Wunused-function]
>> static void mv88e6xxx_ports_cmode_init(struct mv88e6xxx_chip *chip)
>> Let's fix it.
>
> Hi Shaokun, Heiner
>
> Although this fixes the warning, i suspect there i something wrong
> with the original patch adding mv88e6390x_port_set_cmode(). It should
> also be used without CONFIG_NET_DSA_LEGACY.
I checked the commit-id 2a93c1a3651f ("net: dsa: Allow compiling out
legacy support") by Florian.
Do you mean that CONFIG_NET_DSA_LEGACY shall be removed completely? :-)
>>>
>>> No, i suspect mv88e6390x_ports_cmode_init() is being called from the
>>> wrong place, or needs to be called from a second location.
>>>
>>> [Goes and looks at the code]
>>>
>>> Yes, it should also be called in mv88e6xxx_probe(). I would call it
>>> just after the call to mv88e6xxx_detect(), so that it is the same as
>>> in mv88e6xxx_drv_probe().
>>>
>>> There are two ways DSA drivers can be probed. The legacy way, which is
>>> optional, and is slowly getting removed, and the current way. Heiner
>>> is new to DSA and probably missed that, and only handled the legacy
>>> probe method. I also missed checking when i reviewed to patch :-(
>>>
>> Right, I missed that, will submit a fix.
>>
>> I just saw that the Kconfig entry comment for NET_DSA_LEGACY says:
>> "This feature is scheduled for removal in 4.17."
>>
>> Was forgotten to remove it or did somebody scream loud enough
>> "But I depend on it" ?
>
> The intent was to remove it by that kernel version but the 88e6060
> driver still depends on it, and there appears to be some active users
> that Andrew worked with.
>
I see, thanks. And migrating this driver to the new DSA framework
version isn't possible or not worth the effort?
Re: [PATCH -next] net: dsa: mv88e6xxx: Fix build warning when CONFIG_NET_DSA_LEGACY is n
On 3/4/19 10:18 AM, Heiner Kallweit wrote:
> On 04.03.2019 15:57, Andrew Lunn wrote:
>> On Mon, Mar 04, 2019 at 10:16:08PM +0800, Zhangshaokun wrote:
>>> Hi Andrew,
>>>
>>> On 2019/3/4 21:26, Andrew Lunn wrote:
On Mon, Mar 04, 2019 at 08:43:01PM +0800, Shaokun Zhang wrote:
> When CONFIG_NET_DSA_LEGACY is n, there is a GCC bulid warning:
> drivers/net/dsa/mv88e6xxx/chip.c:4623:13: warning:
> ‘mv88e6xxx_ports_cmode_init’ defined but not used [-Wunused-function]
> static void mv88e6xxx_ports_cmode_init(struct mv88e6xxx_chip *chip)
> Let's fix it.
Hi Shaokun, Heiner
Although this fixes the warning, i suspect there i something wrong
with the original patch adding mv88e6390x_port_set_cmode(). It should
also be used without CONFIG_NET_DSA_LEGACY.
>>>
>>> I checked the commit-id 2a93c1a3651f ("net: dsa: Allow compiling out legacy
>>> support") by Florian.
>>> Do you mean that CONFIG_NET_DSA_LEGACY shall be removed completely? :-)
>>
>> No, i suspect mv88e6390x_ports_cmode_init() is being called from the
>> wrong place, or needs to be called from a second location.
>>
>> [Goes and looks at the code]
>>
>> Yes, it should also be called in mv88e6xxx_probe(). I would call it
>> just after the call to mv88e6xxx_detect(), so that it is the same as
>> in mv88e6xxx_drv_probe().
>>
>> There are two ways DSA drivers can be probed. The legacy way, which is
>> optional, and is slowly getting removed, and the current way. Heiner
>> is new to DSA and probably missed that, and only handled the legacy
>> probe method. I also missed checking when i reviewed to patch :-(
>>
> Right, I missed that, will submit a fix.
>
> I just saw that the Kconfig entry comment for NET_DSA_LEGACY says:
> "This feature is scheduled for removal in 4.17."
>
> Was forgotten to remove it or did somebody scream loud enough
> "But I depend on it" ?
The intent was to remove it by that kernel version but the 88e6060
driver still depends on it, and there appears to be some active users
that Andrew worked with.
--
Florian
Re: [PATCH -next] net: dsa: mv88e6xxx: Fix build warning when CONFIG_NET_DSA_LEGACY is n
On 04.03.2019 15:57, Andrew Lunn wrote:
> On Mon, Mar 04, 2019 at 10:16:08PM +0800, Zhangshaokun wrote:
>> Hi Andrew,
>>
>> On 2019/3/4 21:26, Andrew Lunn wrote:
>>> On Mon, Mar 04, 2019 at 08:43:01PM +0800, Shaokun Zhang wrote:
When CONFIG_NET_DSA_LEGACY is n, there is a GCC bulid warning:
drivers/net/dsa/mv88e6xxx/chip.c:4623:13: warning:
‘mv88e6xxx_ports_cmode_init’ defined but not used [-Wunused-function]
static void mv88e6xxx_ports_cmode_init(struct mv88e6xxx_chip *chip)
Let's fix it.
>>>
>>> Hi Shaokun, Heiner
>>>
>>> Although this fixes the warning, i suspect there i something wrong
>>> with the original patch adding mv88e6390x_port_set_cmode(). It should
>>> also be used without CONFIG_NET_DSA_LEGACY.
>>
>> I checked the commit-id 2a93c1a3651f ("net: dsa: Allow compiling out legacy
>> support") by Florian.
>> Do you mean that CONFIG_NET_DSA_LEGACY shall be removed completely? :-)
>
> No, i suspect mv88e6390x_ports_cmode_init() is being called from the
> wrong place, or needs to be called from a second location.
>
> [Goes and looks at the code]
>
> Yes, it should also be called in mv88e6xxx_probe(). I would call it
> just after the call to mv88e6xxx_detect(), so that it is the same as
> in mv88e6xxx_drv_probe().
>
> There are two ways DSA drivers can be probed. The legacy way, which is
> optional, and is slowly getting removed, and the current way. Heiner
> is new to DSA and probably missed that, and only handled the legacy
> probe method. I also missed checking when i reviewed to patch :-(
>
Right, I missed that, will submit a fix.
I just saw that the Kconfig entry comment for NET_DSA_LEGACY says:
"This feature is scheduled for removal in 4.17."
Was forgotten to remove it or did somebody scream loud enough
"But I depend on it" ?
>Andrew
>
Heiner
>
>
>
Re: [PATCH -next] net: dsa: mv88e6xxx: Fix build warning when CONFIG_NET_DSA_LEGACY is n
On Mon, Mar 04, 2019 at 10:16:08PM +0800, Zhangshaokun wrote:
> Hi Andrew,
>
> On 2019/3/4 21:26, Andrew Lunn wrote:
> > On Mon, Mar 04, 2019 at 08:43:01PM +0800, Shaokun Zhang wrote:
> >> When CONFIG_NET_DSA_LEGACY is n, there is a GCC bulid warning:
> >> drivers/net/dsa/mv88e6xxx/chip.c:4623:13: warning:
> >> ‘mv88e6xxx_ports_cmode_init’ defined but not used [-Wunused-function]
> >> static void mv88e6xxx_ports_cmode_init(struct mv88e6xxx_chip *chip)
> >> Let's fix it.
> >
> > Hi Shaokun, Heiner
> >
> > Although this fixes the warning, i suspect there i something wrong
> > with the original patch adding mv88e6390x_port_set_cmode(). It should
> > also be used without CONFIG_NET_DSA_LEGACY.
>
> I checked the commit-id 2a93c1a3651f ("net: dsa: Allow compiling out legacy
> support") by Florian.
> Do you mean that CONFIG_NET_DSA_LEGACY shall be removed completely? :-)
No, i suspect mv88e6390x_ports_cmode_init() is being called from the
wrong place, or needs to be called from a second location.
[Goes and looks at the code]
Yes, it should also be called in mv88e6xxx_probe(). I would call it
just after the call to mv88e6xxx_detect(), so that it is the same as
in mv88e6xxx_drv_probe().
There are two ways DSA drivers can be probed. The legacy way, which is
optional, and is slowly getting removed, and the current way. Heiner
is new to DSA and probably missed that, and only handled the legacy
probe method. I also missed checking when i reviewed to patch :-(
Andrew
Re: [PATCH -next] net: dsa: mv88e6xxx: Fix build warning when CONFIG_NET_DSA_LEGACY is n
Hi Andrew,
On 2019/3/4 21:26, Andrew Lunn wrote:
> On Mon, Mar 04, 2019 at 08:43:01PM +0800, Shaokun Zhang wrote:
>> When CONFIG_NET_DSA_LEGACY is n, there is a GCC bulid warning:
>> drivers/net/dsa/mv88e6xxx/chip.c:4623:13: warning:
>> ‘mv88e6xxx_ports_cmode_init’ defined but not used [-Wunused-function]
>> static void mv88e6xxx_ports_cmode_init(struct mv88e6xxx_chip *chip)
>> Let's fix it.
>
> Hi Shaokun, Heiner
>
> Although this fixes the warning, i suspect there i something wrong
> with the original patch adding mv88e6390x_port_set_cmode(). It should
> also be used without CONFIG_NET_DSA_LEGACY.
I checked the commit-id 2a93c1a3651f ("net: dsa: Allow compiling out legacy
support") by Florian.
Do you mean that CONFIG_NET_DSA_LEGACY shall be removed completely? :-)
Thanks,
Shaokun
>
> Andrew
>
>
>>
>> Fixes: ed8fe20205ac ("net: dsa: mv88e6xxx: prevent interrupt storm caused by
>> mv88e6390x_port_set_cmode")
>> Cc: Heiner Kallweit
>> Cc: Andrew Lunn
>> Cc: Vivien Didelot
>> Cc: Florian Fainelli
>> Cc: "David S. Miller"
>> Signed-off-by: Shaokun Zhang
>> ---
>> drivers/net/dsa/mv88e6xxx/chip.c | 16
>> 1 file changed, 8 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/net/dsa/mv88e6xxx/chip.c
>> b/drivers/net/dsa/mv88e6xxx/chip.c
>> index e4ad16b2dc38..168d4898c36f 100644
>> --- a/drivers/net/dsa/mv88e6xxx/chip.c
>> +++ b/drivers/net/dsa/mv88e6xxx/chip.c
>> @@ -4620,14 +4620,6 @@ static int mv88e6xxx_smi_init(struct mv88e6xxx_chip
>> *chip,
>> return 0;
>> }
>>
>> -static void mv88e6xxx_ports_cmode_init(struct mv88e6xxx_chip *chip)
>> -{
>> -int i;
>> -
>> -for (i = 0; i < mv88e6xxx_num_ports(chip); i++)
>> -chip->ports[i].cmode = MV88E6XXX_PORT_STS_CMODE_INVALID;
>> -}
>> -
>> static enum dsa_tag_protocol mv88e6xxx_get_tag_protocol(struct dsa_switch
>> *ds,
>> int port)
>> {
>> @@ -4637,6 +4629,14 @@ static enum dsa_tag_protocol
>> mv88e6xxx_get_tag_protocol(struct dsa_switch *ds,
>> }
>>
>> #if IS_ENABLED(CONFIG_NET_DSA_LEGACY)
>> +static void mv88e6xxx_ports_cmode_init(struct mv88e6xxx_chip *chip)
>> +{
>> +int i;
>> +
>> +for (i = 0; i < mv88e6xxx_num_ports(chip); i++)
>> +chip->ports[i].cmode = MV88E6XXX_PORT_STS_CMODE_INVALID;
>> +}
>> +
>> static const char *mv88e6xxx_drv_probe(struct device *dsa_dev,
>> struct device *host_dev, int sw_addr,
>> void **priv)
>> --
>> 2.7.4
>>
>
> .
>
Re: [PATCH -next] net: dsa: mv88e6xxx: Fix build warning when CONFIG_NET_DSA_LEGACY is n
On Mon, Mar 04, 2019 at 08:43:01PM +0800, Shaokun Zhang wrote:
> When CONFIG_NET_DSA_LEGACY is n, there is a GCC bulid warning:
> drivers/net/dsa/mv88e6xxx/chip.c:4623:13: warning:
> ‘mv88e6xxx_ports_cmode_init’ defined but not used [-Wunused-function]
> static void mv88e6xxx_ports_cmode_init(struct mv88e6xxx_chip *chip)
> Let's fix it.
Hi Shaokun, Heiner
Although this fixes the warning, i suspect there i something wrong
with the original patch adding mv88e6390x_port_set_cmode(). It should
also be used without CONFIG_NET_DSA_LEGACY.
Andrew
>
> Fixes: ed8fe20205ac ("net: dsa: mv88e6xxx: prevent interrupt storm caused by
> mv88e6390x_port_set_cmode")
> Cc: Heiner Kallweit
> Cc: Andrew Lunn
> Cc: Vivien Didelot
> Cc: Florian Fainelli
> Cc: "David S. Miller"
> Signed-off-by: Shaokun Zhang
> ---
> drivers/net/dsa/mv88e6xxx/chip.c | 16
> 1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/net/dsa/mv88e6xxx/chip.c
> b/drivers/net/dsa/mv88e6xxx/chip.c
> index e4ad16b2dc38..168d4898c36f 100644
> --- a/drivers/net/dsa/mv88e6xxx/chip.c
> +++ b/drivers/net/dsa/mv88e6xxx/chip.c
> @@ -4620,14 +4620,6 @@ static int mv88e6xxx_smi_init(struct mv88e6xxx_chip
> *chip,
> return 0;
> }
>
> -static void mv88e6xxx_ports_cmode_init(struct mv88e6xxx_chip *chip)
> -{
> - int i;
> -
> - for (i = 0; i < mv88e6xxx_num_ports(chip); i++)
> - chip->ports[i].cmode = MV88E6XXX_PORT_STS_CMODE_INVALID;
> -}
> -
> static enum dsa_tag_protocol mv88e6xxx_get_tag_protocol(struct dsa_switch
> *ds,
> int port)
> {
> @@ -4637,6 +4629,14 @@ static enum dsa_tag_protocol
> mv88e6xxx_get_tag_protocol(struct dsa_switch *ds,
> }
>
> #if IS_ENABLED(CONFIG_NET_DSA_LEGACY)
> +static void mv88e6xxx_ports_cmode_init(struct mv88e6xxx_chip *chip)
> +{
> + int i;
> +
> + for (i = 0; i < mv88e6xxx_num_ports(chip); i++)
> + chip->ports[i].cmode = MV88E6XXX_PORT_STS_CMODE_INVALID;
> +}
> +
> static const char *mv88e6xxx_drv_probe(struct device *dsa_dev,
> struct device *host_dev, int sw_addr,
> void **priv)
> --
> 2.7.4
>
[PATCH -next] net: dsa: mv88e6xxx: Fix build warning when CONFIG_NET_DSA_LEGACY is n
When CONFIG_NET_DSA_LEGACY is n, there is a GCC bulid warning:
drivers/net/dsa/mv88e6xxx/chip.c:4623:13: warning:
???mv88e6xxx_ports_cmode_init??? defined but not used [-Wunused-function]
static void mv88e6xxx_ports_cmode_init(struct mv88e6xxx_chip *chip)
Let's fix it.
Fixes: ed8fe20205ac ("net: dsa: mv88e6xxx: prevent interrupt storm caused by
mv88e6390x_port_set_cmode")
Cc: Heiner Kallweit
Cc: Andrew Lunn
Cc: Vivien Didelot
Cc: Florian Fainelli
Cc: "David S. Miller"
Signed-off-by: Shaokun Zhang
---
drivers/net/dsa/mv88e6xxx/chip.c | 16
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index e4ad16b2dc38..168d4898c36f 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -4620,14 +4620,6 @@ static int mv88e6xxx_smi_init(struct mv88e6xxx_chip
*chip,
return 0;
}
-static void mv88e6xxx_ports_cmode_init(struct mv88e6xxx_chip *chip)
-{
- int i;
-
- for (i = 0; i < mv88e6xxx_num_ports(chip); i++)
- chip->ports[i].cmode = MV88E6XXX_PORT_STS_CMODE_INVALID;
-}
-
static enum dsa_tag_protocol mv88e6xxx_get_tag_protocol(struct dsa_switch *ds,
int port)
{
@@ -4637,6 +4629,14 @@ static enum dsa_tag_protocol
mv88e6xxx_get_tag_protocol(struct dsa_switch *ds,
}
#if IS_ENABLED(CONFIG_NET_DSA_LEGACY)
+static void mv88e6xxx_ports_cmode_init(struct mv88e6xxx_chip *chip)
+{
+ int i;
+
+ for (i = 0; i < mv88e6xxx_num_ports(chip); i++)
+ chip->ports[i].cmode = MV88E6XXX_PORT_STS_CMODE_INVALID;
+}
+
static const char *mv88e6xxx_drv_probe(struct device *dsa_dev,
struct device *host_dev, int sw_addr,
void **priv)
--
2.7.4
