linux-mediatek.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [bug report] net: dsa: mt7530: Add the support of MT7531 switch
@ 2020-09-16 14:35 Dan Carpenter
  2020-09-17  7:15 ` Landen Chao
  0 siblings, 1 reply; 2+ messages in thread
From: Dan Carpenter @ 2020-09-16 14:35 UTC (permalink / raw)
  To: landen.chao; +Cc: linux-mediatek

Hello Landen Chao,

The patch c288575f7810: "net: dsa: mt7530: Add the support of MT7531
switch" from Sep 11, 2020, leads to the following static checker
warning:

	drivers/net/dsa/mt7530.c:2298 mt7531_cpu_port_config()
	error: uninitialized symbol 'interface'.

drivers/net/dsa/mt7530.c
  2273  static int
  2274  mt7531_cpu_port_config(struct dsa_switch *ds, int port)
  2275  {
  2276          struct mt7530_priv *priv = ds->priv;
  2277          phy_interface_t interface;
  2278          int speed;
  2279  
  2280          switch (port) {
  2281          case 5:
  2282                  if (mt7531_is_rgmii_port(priv, port))
  2283                          interface = PHY_INTERFACE_MODE_RGMII;
  2284                  else
  2285                          interface = PHY_INTERFACE_MODE_2500BASEX;
  2286  
  2287                  priv->p5_interface = interface;
  2288                  break;
  2289          case 6:
  2290                  interface = PHY_INTERFACE_MODE_2500BASEX;
  2291  
  2292                  mt7531_pad_setup(ds, interface);
  2293  
  2294                  priv->p6_interface = interface;
  2295                  break;

Does this need a default case?  Smatch thinks "port" can be 0-6.

  2296          }
  2297  
  2298          if (interface == PHY_INTERFACE_MODE_2500BASEX)
  2299                  speed = SPEED_2500;
  2300          else
  2301                  speed = SPEED_1000;
  2302  
  2303          mt7531_mac_config(ds, port, MLO_AN_FIXED, interface);
  2304          mt7530_write(priv, MT7530_PMCR_P(port),
  2305                       PMCR_CPU_PORT_SETTING(priv->id));
  2306          mt753x_phylink_mac_link_up(ds, port, MLO_AN_FIXED, interface, NULL,
  2307                                     speed, DUPLEX_FULL, true, true);
  2308  
  2309          return 0;
  2310  }

regards,
dan carpenter

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

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

* Re: [bug report] net: dsa: mt7530: Add the support of MT7531 switch
  2020-09-16 14:35 [bug report] net: dsa: mt7530: Add the support of MT7531 switch Dan Carpenter
@ 2020-09-17  7:15 ` Landen Chao
  0 siblings, 0 replies; 2+ messages in thread
From: Landen Chao @ 2020-09-17  7:15 UTC (permalink / raw)
  To: Dan Carpenter; +Cc: linux-mediatek@lists.infradead.org

Hello Dan,

Thanks for your review.
On Wed, 2020-09-16 at 22:35 +0800, Dan Carpenter wrote:
> Hello Landen Chao,
> 
> The patch c288575f7810: "net: dsa: mt7530: Add the support of MT7531
> switch" from Sep 11, 2020, leads to the following static checker
> warning:
> 
> 	drivers/net/dsa/mt7530.c:2298 mt7531_cpu_port_config()
> 	error: uninitialized symbol 'interface'.
> 
> drivers/net/dsa/mt7530.c
>   2273  static int
>   2274  mt7531_cpu_port_config(struct dsa_switch *ds, int port)
>   2275  {
>   2276          struct mt7530_priv *priv = ds->priv;
>   2277          phy_interface_t interface;
>   2278          int speed;
>   2279  
>   2280          switch (port) {
>   2281          case 5:
>   2282                  if (mt7531_is_rgmii_port(priv, port))
>   2283                          interface = PHY_INTERFACE_MODE_RGMII;
>   2284                  else
>   2285                          interface = PHY_INTERFACE_MODE_2500BASEX;
>   2286  
>   2287                  priv->p5_interface = interface;
>   2288                  break;
>   2289          case 6:
>   2290                  interface = PHY_INTERFACE_MODE_2500BASEX;
>   2291  
>   2292                  mt7531_pad_setup(ds, interface);
>   2293  
>   2294                  priv->p6_interface = interface;
>   2295                  break;
> 
> Does this need a default case?  Smatch thinks "port" can be 0-6.
Yes, it needs a default case to catch unexpected dts setting. "port" can
be 0-6 generally, but only 5 and 6 are valid cpu port.

Although, "port" 0-4 can also be labeled as "cpu" in dts, but no real
hardware can support this kind of configuration currently.
> 
>   2296          }
>   2297  
>   2298          if (interface == PHY_INTERFACE_MODE_2500BASEX)
>   2299                  speed = SPEED_2500;
>   2300          else
>   2301                  speed = SPEED_1000;
>   2302  
>   2303          mt7531_mac_config(ds, port, MLO_AN_FIXED, interface);
>   2304          mt7530_write(priv, MT7530_PMCR_P(port),
>   2305                       PMCR_CPU_PORT_SETTING(priv->id));
>   2306          mt753x_phylink_mac_link_up(ds, port, MLO_AN_FIXED, interface, NULL,
>   2307                                     speed, DUPLEX_FULL, true, true);
>   2308  
>   2309          return 0;
>   2310  }
> 
> regards,
> dan carpenter
regards,
landen

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

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

end of thread, other threads:[~2020-09-17  7:23 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-09-16 14:35 [bug report] net: dsa: mt7530: Add the support of MT7531 switch Dan Carpenter
2020-09-17  7:15 ` Landen Chao

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