linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH -next] net: dsa: mv88e6xxx: Fix build warning when CONFIG_NET_DSA_LEGACY is n
@ 2019-03-04 12:43 Shaokun Zhang
  2019-03-04 13:26 ` Andrew Lunn
  0 siblings, 1 reply; 9+ messages in thread
From: Shaokun Zhang @ 2019-03-04 12:43 UTC (permalink / raw)
  To: netdev, linux-kernel
  Cc: Shaokun Zhang, Heiner Kallweit, Andrew Lunn, Vivien Didelot,
	Florian Fainelli, David S. Miller

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="y", Size: 1847 bytes --]

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 <hkallweit1@gmail.com>
Cc: Andrew Lunn <andrew@lunn.ch>
Cc: Vivien Didelot <vivien.didelot@gmail.com> 
Cc: Florian Fainelli <f.fainelli@gmail.com> 
Cc: "David S. Miller" <davem@davemloft.net>
Signed-off-by: Shaokun Zhang <zhangshaokun@hisilicon.com>
---
 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


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH -next] net: dsa: mv88e6xxx: Fix build warning when CONFIG_NET_DSA_LEGACY is n
  2019-03-04 12:43 [PATCH -next] net: dsa: mv88e6xxx: Fix build warning when CONFIG_NET_DSA_LEGACY is n Shaokun Zhang
@ 2019-03-04 13:26 ` Andrew Lunn
  2019-03-04 14:16   ` Zhangshaokun
  0 siblings, 1 reply; 9+ messages in thread
From: Andrew Lunn @ 2019-03-04 13:26 UTC (permalink / raw)
  To: Shaokun Zhang
  Cc: netdev, linux-kernel, Heiner Kallweit, Vivien Didelot,
	Florian Fainelli, David S. Miller

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 <hkallweit1@gmail.com>
> Cc: Andrew Lunn <andrew@lunn.ch>
> Cc: Vivien Didelot <vivien.didelot@gmail.com> 
> Cc: Florian Fainelli <f.fainelli@gmail.com> 
> Cc: "David S. Miller" <davem@davemloft.net>
> Signed-off-by: Shaokun Zhang <zhangshaokun@hisilicon.com>
> ---
>  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
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH -next] net: dsa: mv88e6xxx: Fix build warning when CONFIG_NET_DSA_LEGACY is n
  2019-03-04 13:26 ` Andrew Lunn
@ 2019-03-04 14:16   ` Zhangshaokun
  2019-03-04 14:57     ` Andrew Lunn
  0 siblings, 1 reply; 9+ messages in thread
From: Zhangshaokun @ 2019-03-04 14:16 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: netdev, linux-kernel, Heiner Kallweit, Vivien Didelot,
	Florian Fainelli, David S. Miller

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 <hkallweit1@gmail.com>
>> Cc: Andrew Lunn <andrew@lunn.ch>
>> Cc: Vivien Didelot <vivien.didelot@gmail.com> 
>> Cc: Florian Fainelli <f.fainelli@gmail.com> 
>> Cc: "David S. Miller" <davem@davemloft.net>
>> Signed-off-by: Shaokun Zhang <zhangshaokun@hisilicon.com>
>> ---
>>  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
>>
> 
> .
> 


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH -next] net: dsa: mv88e6xxx: Fix build warning when CONFIG_NET_DSA_LEGACY is n
  2019-03-04 14:16   ` Zhangshaokun
@ 2019-03-04 14:57     ` Andrew Lunn
  2019-03-04 18:18       ` Heiner Kallweit
  2019-03-05  0:57       ` Zhangshaokun
  0 siblings, 2 replies; 9+ messages in thread
From: Andrew Lunn @ 2019-03-04 14:57 UTC (permalink / raw)
  To: Zhangshaokun
  Cc: netdev, linux-kernel, Heiner Kallweit, Vivien Didelot,
	Florian Fainelli, David S. Miller

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




^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH -next] net: dsa: mv88e6xxx: Fix build warning when CONFIG_NET_DSA_LEGACY is n
  2019-03-04 14:57     ` Andrew Lunn
@ 2019-03-04 18:18       ` Heiner Kallweit
  2019-03-04 18:24         ` Florian Fainelli
  2019-03-05  0:57       ` Zhangshaokun
  1 sibling, 1 reply; 9+ messages in thread
From: Heiner Kallweit @ 2019-03-04 18:18 UTC (permalink / raw)
  To: Andrew Lunn, Zhangshaokun
  Cc: netdev, linux-kernel, Vivien Didelot, Florian Fainelli, David S. Miller

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


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH -next] net: dsa: mv88e6xxx: Fix build warning when CONFIG_NET_DSA_LEGACY is n
  2019-03-04 18:18       ` Heiner Kallweit
@ 2019-03-04 18:24         ` Florian Fainelli
  2019-03-04 18:31           ` Heiner Kallweit
  0 siblings, 1 reply; 9+ messages in thread
From: Florian Fainelli @ 2019-03-04 18:24 UTC (permalink / raw)
  To: Heiner Kallweit, Andrew Lunn, Zhangshaokun
  Cc: netdev, linux-kernel, Vivien Didelot, David S. Miller

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH -next] net: dsa: mv88e6xxx: Fix build warning when CONFIG_NET_DSA_LEGACY is n
  2019-03-04 18:24         ` Florian Fainelli
@ 2019-03-04 18:31           ` Heiner Kallweit
  2019-03-04 18:36             ` Florian Fainelli
  0 siblings, 1 reply; 9+ messages in thread
From: Heiner Kallweit @ 2019-03-04 18:31 UTC (permalink / raw)
  To: Florian Fainelli, Andrew Lunn, Zhangshaokun
  Cc: netdev, linux-kernel, Vivien Didelot, David S. Miller

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?

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH -next] net: dsa: mv88e6xxx: Fix build warning when CONFIG_NET_DSA_LEGACY is n
  2019-03-04 18:31           ` Heiner Kallweit
@ 2019-03-04 18:36             ` Florian Fainelli
  0 siblings, 0 replies; 9+ messages in thread
From: Florian Fainelli @ 2019-03-04 18:36 UTC (permalink / raw)
  To: Heiner Kallweit, Andrew Lunn, Zhangshaokun
  Cc: netdev, linux-kernel, Vivien Didelot, David S. Miller

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH -next] net: dsa: mv88e6xxx: Fix build warning when CONFIG_NET_DSA_LEGACY is n
  2019-03-04 14:57     ` Andrew Lunn
  2019-03-04 18:18       ` Heiner Kallweit
@ 2019-03-05  0:57       ` Zhangshaokun
  1 sibling, 0 replies; 9+ messages in thread
From: Zhangshaokun @ 2019-03-05  0:57 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: netdev, linux-kernel, Heiner Kallweit, Vivien Didelot,
	Florian Fainelli, David S. Miller

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


^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2019-03-05  0:58 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-04 12:43 [PATCH -next] net: dsa: mv88e6xxx: Fix build warning when CONFIG_NET_DSA_LEGACY is n Shaokun Zhang
2019-03-04 13:26 ` Andrew Lunn
2019-03-04 14:16   ` Zhangshaokun
2019-03-04 14:57     ` Andrew Lunn
2019-03-04 18:18       ` Heiner Kallweit
2019-03-04 18:24         ` Florian Fainelli
2019-03-04 18:31           ` Heiner Kallweit
2019-03-04 18:36             ` Florian Fainelli
2019-03-05  0:57       ` Zhangshaokun

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).