diff mbox series

[v2,1/2] net: dsa: lantiq: allow to use all GPHYs on xRX300 and xRX330

Message ID 20201206132713.13452-2-olek2@wp.pl (mailing list archive)
State Changes Requested
Delegated to: Netdev Maintainers
Headers show
Series net: dsa: lantiq: add support for xRX300 and xRX330 | expand

Checks

Context Check Description
netdev/cover_letter success Link
netdev/fixes_present success Link
netdev/patch_count success Link
netdev/tree_selection success Guessed tree name to be net-next
netdev/subject_prefix success Link
netdev/source_inline success Was 0 now: 0
netdev/verify_signedoff success Link
netdev/module_param success Was 0 now: 0
netdev/build_32bit fail Errors and warnings before: 0 this patch: 5
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/verify_fixes success Link
netdev/checkpatch warning WARNING: DT compatible string "lantiq,xrx300-gswip" appears un-documented -- check ./Documentation/devicetree/bindings/ WARNING: DT compatible string "lantiq,xrx330-gswip" appears un-documented -- check ./Documentation/devicetree/bindings/ WARNING: line length of 95 exceeds 80 columns
netdev/build_allmodconfig_warn fail Errors and warnings before: 0 this patch: 5
netdev/header_inline success Link
netdev/stable success Stable not CCed

Commit Message

Aleksander Jan Bajkowski Dec. 6, 2020, 1:27 p.m. UTC
This patch allows to use all PHYs on GRX300 and GRX330. The ARX300 has 3
and the GRX330 has 4 integrated PHYs connected to different ports compared
to VRX200.

Port configurations:

xRX200:
GMAC0: RGMII/MII/REVMII/RMII port
GMAC1: RGMII/MII/REVMII/RMII port
GMAC2: GPHY0 (GMII)
GMAC3: GPHY0 (MII)
GMAC4: GPHY1 (GMII)
GMAC5: GPHY1 (MII) or RGMII port

xRX300:
GMAC0: RGMII port
GMAC1: GPHY2 (GMII)
GMAC2: GPHY0 (GMII)
GMAC3: GPHY0 (MII)
GMAC4: GPHY1 (GMII)
GMAC5: GPHY1 (MII) or RGMII port

xRX330:
GMAC0: RGMII/GMII/RMII port
GMAC1: GPHY2 (GMII)
GMAC2: GPHY0 (GMII)
GMAC3: GPHY0 (MII) or GPHY3 (GMII)
GMAC4: GPHY1 (GMII)
GMAC5: GPHY1 (MII) or RGMII/RMII port

Tested on D-Link DWR966 with OpenWRT.

Signed-off-by: Aleksander Jan Bajkowski <olek2@wp.pl>
---
 drivers/net/dsa/lantiq_gswip.c | 170 +++++++++++++++++++++++++++------
 1 file changed, 141 insertions(+), 29 deletions(-)

Comments

Vladimir Oltean Dec. 6, 2020, 6:54 p.m. UTC | #1
On Sun, Dec 06, 2020 at 02:27:12PM +0100, Aleksander Jan Bajkowski wrote:
> This patch allows to use all PHYs on GRX300 and GRX330. The ARX300 has 3
> and the GRX330 has 4 integrated PHYs connected to different ports compared
> to VRX200.
> 
> Port configurations:
> 
> xRX200:
> GMAC0: RGMII/MII/REVMII/RMII port
> GMAC1: RGMII/MII/REVMII/RMII port
> GMAC2: GPHY0 (GMII)
> GMAC3: GPHY0 (MII)
> GMAC4: GPHY1 (GMII)
> GMAC5: GPHY1 (MII) or RGMII port
> 
> xRX300:
> GMAC0: RGMII port
> GMAC1: GPHY2 (GMII)
> GMAC2: GPHY0 (GMII)
> GMAC3: GPHY0 (MII)
> GMAC4: GPHY1 (GMII)
> GMAC5: GPHY1 (MII) or RGMII port
> 
> xRX330:
> GMAC0: RGMII/GMII/RMII port
> GMAC1: GPHY2 (GMII)
> GMAC2: GPHY0 (GMII)
> GMAC3: GPHY0 (MII) or GPHY3 (GMII)
> GMAC4: GPHY1 (GMII)
> GMAC5: GPHY1 (MII) or RGMII/RMII port

When you say GMII/MII when you are talking to the GPHY ports, what you
are really talking about is 1000Base-T vs 100Base-TX, right?

How about xRX330, does that really expose the full parallel GMAC pinout
on GMAC0?

> 
> Tested on D-Link DWR966 with OpenWRT.
> 
> Signed-off-by: Aleksander Jan Bajkowski <olek2@wp.pl>
> ---
>  drivers/net/dsa/lantiq_gswip.c | 170 +++++++++++++++++++++++++++------
>  1 file changed, 141 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/net/dsa/lantiq_gswip.c b/drivers/net/dsa/lantiq_gswip.c
> index 09701c17f3f6..4c8f611ed397 100644
> --- a/drivers/net/dsa/lantiq_gswip.c
> +++ b/drivers/net/dsa/lantiq_gswip.c
> @@ -94,6 +94,7 @@
>  /* GSWIP MII Registers */
>  #define GSWIP_MII_CFG0			0x00
>  #define GSWIP_MII_CFG1			0x02
> +#define GSWIP_MII_CFG3			0xc3
>  #define GSWIP_MII_CFG5			0x04
>  #define  GSWIP_MII_CFG_EN		BIT(14)
>  #define  GSWIP_MII_CFG_LDCLKDIS		BIT(12)
> @@ -102,6 +103,7 @@
>  #define  GSWIP_MII_CFG_MODE_RMIIP	0x2
>  #define  GSWIP_MII_CFG_MODE_RMIIM	0x3
>  #define  GSWIP_MII_CFG_MODE_RGMII	0x4
> +#define  GSWIP_MII_CFG_MODE_GMII	0x9
>  #define  GSWIP_MII_CFG_MODE_MASK	0xf
>  #define  GSWIP_MII_CFG_RATE_M2P5	0x00
>  #define  GSWIP_MII_CFG_RATE_M25	0x10
> @@ -222,6 +224,7 @@
>  struct gswip_hw_info {
>  	int max_ports;
>  	int cpu_port;
> +	struct dsa_switch_ops *ops;
>  };
>  
>  struct xway_gphy_match_data {
> @@ -392,12 +395,19 @@ static void gswip_mii_mask(struct gswip_priv *priv, u32 clear, u32 set,
>  static void gswip_mii_mask_cfg(struct gswip_priv *priv, u32 clear, u32 set,
>  			       int port)
>  {
> +	struct device_node *np = priv->ds->dev->of_node;
> +
>  	switch (port) {
>  	case 0:
>  		gswip_mii_mask(priv, clear, set, GSWIP_MII_CFG0);
>  		break;
>  	case 1:
> -		gswip_mii_mask(priv, clear, set, GSWIP_MII_CFG1);
> +		if (of_device_is_compatible(np, "lantiq,xrx200-gswip"))
> +			gswip_mii_mask(priv, clear, set, GSWIP_MII_CFG1);
> +		break;
> +	case 3:
> +		if (of_device_is_compatible(np, "lantiq,xrx330-gswip"))
> +			gswip_mii_mask(priv, clear, set, GSWIP_MII_CFG3);
>  		break;
>  	case 5:
>  		gswip_mii_mask(priv, clear, set, GSWIP_MII_CFG5);
> @@ -1409,12 +1419,40 @@ static int gswip_port_fdb_dump(struct dsa_switch *ds, int port,
>  	return 0;
>  }
>  
> -static void gswip_phylink_validate(struct dsa_switch *ds, int port,
> -				   unsigned long *supported,
> -				   struct phylink_link_state *state)
> +static void gswip_phylink_set_capab(unsigned long *supported, struct phylink_link_state *state)
>  {
>  	__ETHTOOL_DECLARE_LINK_MODE_MASK(mask) = { 0, };
>  
> +	/* Allow all the expected bits */
> +	phylink_set(mask, Autoneg);
> +	phylink_set_port_modes(mask);
> +	phylink_set(mask, Pause);
> +	phylink_set(mask, Asym_Pause);
> +
> +	/* With the exclusion of MII and Reverse MII, we support Gigabit,
> +	 * including Half duplex
> +	 */
> +	if (state->interface != PHY_INTERFACE_MODE_MII &&
> +	    state->interface != PHY_INTERFACE_MODE_REVMII) {
> +		phylink_set(mask, 1000baseT_Full);
> +		phylink_set(mask, 1000baseT_Half);
> +	}
> +
> +	phylink_set(mask, 10baseT_Half);
> +	phylink_set(mask, 10baseT_Full);
> +	phylink_set(mask, 100baseT_Half);
> +	phylink_set(mask, 100baseT_Full);
> +
> +	bitmap_and(supported, supported, mask,
> +		   __ETHTOOL_LINK_MODE_MASK_NBITS);
> +	bitmap_and(state->advertising, state->advertising, mask,
> +		   __ETHTOOL_LINK_MODE_MASK_NBITS);
> +}
> +
> +static void gswip_xrx200_phylink_validate(struct dsa_switch *ds, int port,
> +					  unsigned long *supported,
> +					  struct phylink_link_state *state)
> +{
>  	switch (port) {
>  	case 0:
>  	case 1:
> @@ -1441,37 +1479,56 @@ static void gswip_phylink_validate(struct dsa_switch *ds, int port,
>  		return;
>  	}
>  
> -	/* Allow all the expected bits */
> -	phylink_set(mask, Autoneg);
> -	phylink_set_port_modes(mask);
> -	phylink_set(mask, Pause);
> -	phylink_set(mask, Asym_Pause);
> +	gswip_phylink_set_capab(supported, state);
>  
> -	/* With the exclusion of MII and Reverse MII, we support Gigabit,
> -	 * including Half duplex
> -	 */
> -	if (state->interface != PHY_INTERFACE_MODE_MII &&
> -	    state->interface != PHY_INTERFACE_MODE_REVMII) {
> -		phylink_set(mask, 1000baseT_Full);
> -		phylink_set(mask, 1000baseT_Half);
> +	return;
> +
> +unsupported:
> +	bitmap_zero(supported, __ETHTOOL_LINK_MODE_MASK_NBITS);
> +	dev_err(ds->dev, "Unsupported interface '%s' for port %d\n",
> +		phy_modes(state->interface), port);
> +}
> +
> +static void gswip_xrx300_phylink_validate(struct dsa_switch *ds, int port,
> +					  unsigned long *supported,
> +					  struct phylink_link_state *state)
> +{
> +	__ETHTOOL_DECLARE_LINK_MODE_MASK(mask) = { 0, };
> +
> +	switch (port) {
> +	case 0:
> +		if (!phy_interface_mode_is_rgmii(state->interface) &&
> +		    state->interface != PHY_INTERFACE_MODE_GMII &&
> +		    state->interface != PHY_INTERFACE_MODE_RMII)
> +			goto unsupported;
> +		break;
> +	case 1:
> +	case 2:
> +	case 3:
> +	case 4:
> +		if (state->interface != PHY_INTERFACE_MODE_INTERNAL)
> +			goto unsupported;
> +		break;
> +	case 5:
> +		if (!phy_interface_mode_is_rgmii(state->interface) &&
> +		    state->interface != PHY_INTERFACE_MODE_INTERNAL &&
> +		    state->interface != PHY_INTERFACE_MODE_RMII)
> +			goto unsupported;
> +		break;
> +	default:
> +		bitmap_zero(supported, __ETHTOOL_LINK_MODE_MASK_NBITS);
> +		dev_err(ds->dev, "Unsupported port: %i\n", port);
> +		return;
>  	}

I think there is a pre-existing issue in gswip_phylink_validate in that
when state->interface == PHY_INTERFACE_MODE_NA, this triggers the "goto
unsupported" code path, when it should instead report all supported link
modes. phylink calls phylink_validate() with PHY_INTERFACE_MODE_NA when
the MII protocol is not known off-hand but depends on what is attached /
what that is configured to advertise.

In your case I think it is a bit unlikely to have a clause 45 PHY that
changes MII protocol dynamically, or an SFP module attached directly to
your GMAC, given that on the port without an internal PHY you only have
parallel interfaces and not a SERDES (note that there still exist RGMII
PHYs - see RTL8211FS - that have an optical media side and can therefore
be connected to an SFP cage, but I don't really know how phylink deals
with those given the structure of sfp_select_interface). So it is mainly
a question of API compliance. Do we want to allow drivers to ignore
something than the API requires, because the driver writers know that
the code path will not be executed in real life.

>  
> -	phylink_set(mask, 10baseT_Half);
> -	phylink_set(mask, 10baseT_Full);
> -	phylink_set(mask, 100baseT_Half);
> -	phylink_set(mask, 100baseT_Full);
> +	gswip_phylink_set_capab(supported, state);
>  
> -	bitmap_and(supported, supported, mask,
> -		   __ETHTOOL_LINK_MODE_MASK_NBITS);
> -	bitmap_and(state->advertising, state->advertising, mask,
> -		   __ETHTOOL_LINK_MODE_MASK_NBITS);
>  	return;
>  
>  unsupported:
>  	bitmap_zero(supported, __ETHTOOL_LINK_MODE_MASK_NBITS);
>  	dev_err(ds->dev, "Unsupported interface '%s' for port %d\n",
>  		phy_modes(state->interface), port);
> -	return;
>  }
>  
>  static void gswip_phylink_mac_config(struct dsa_switch *ds, int port,
> @@ -1500,6 +1557,9 @@ static void gswip_phylink_mac_config(struct dsa_switch *ds, int port,
>  	case PHY_INTERFACE_MODE_RGMII_TXID:
>  		miicfg |= GSWIP_MII_CFG_MODE_RGMII;
>  		break;
> +	case PHY_INTERFACE_MODE_GMII:
> +		miicfg |= GSWIP_MII_CFG_MODE_GMII;
> +		break;
>  	default:
>  		dev_err(ds->dev,
>  			"Unsupported interface: %d\n", state->interface);
> @@ -1614,7 +1674,7 @@ static int gswip_get_sset_count(struct dsa_switch *ds, int port, int sset)
>  	return ARRAY_SIZE(gswip_rmon_cnt);
>  }
>  
> -static const struct dsa_switch_ops gswip_switch_ops = {
> +static const struct dsa_switch_ops gswip_xrx200_switch_ops = {
>  	.get_tag_protocol	= gswip_get_tag_protocol,
>  	.setup			= gswip_setup,
>  	.port_enable		= gswip_port_enable,
> @@ -1630,7 +1690,32 @@ static const struct dsa_switch_ops gswip_switch_ops = {
>  	.port_fdb_add		= gswip_port_fdb_add,
>  	.port_fdb_del		= gswip_port_fdb_del,
>  	.port_fdb_dump		= gswip_port_fdb_dump,
> -	.phylink_validate	= gswip_phylink_validate,
> +	.phylink_validate	= gswip_xrx200_phylink_validate,
> +	.phylink_mac_config	= gswip_phylink_mac_config,
> +	.phylink_mac_link_down	= gswip_phylink_mac_link_down,
> +	.phylink_mac_link_up	= gswip_phylink_mac_link_up,
> +	.get_strings		= gswip_get_strings,
> +	.get_ethtool_stats	= gswip_get_ethtool_stats,
> +	.get_sset_count		= gswip_get_sset_count,
> +};
> +
> +static const struct dsa_switch_ops gswip_xrx300_switch_ops = {
> +	.get_tag_protocol	= gswip_get_tag_protocol,
> +	.setup			= gswip_setup,
> +	.port_enable		= gswip_port_enable,
> +	.port_disable		= gswip_port_disable,
> +	.port_bridge_join	= gswip_port_bridge_join,
> +	.port_bridge_leave	= gswip_port_bridge_leave,
> +	.port_fast_age		= gswip_port_fast_age,
> +	.port_vlan_filtering	= gswip_port_vlan_filtering,
> +	.port_vlan_prepare	= gswip_port_vlan_prepare,
> +	.port_vlan_add		= gswip_port_vlan_add,
> +	.port_vlan_del		= gswip_port_vlan_del,
> +	.port_stp_state_set	= gswip_port_stp_state_set,
> +	.port_fdb_add		= gswip_port_fdb_add,
> +	.port_fdb_del		= gswip_port_fdb_del,
> +	.port_fdb_dump		= gswip_port_fdb_dump,
> +	.phylink_validate	= gswip_xrx300_phylink_validate,
>  	.phylink_mac_config	= gswip_phylink_mac_config,
>  	.phylink_mac_link_down	= gswip_phylink_mac_link_down,
>  	.phylink_mac_link_up	= gswip_phylink_mac_link_up,
> @@ -1859,7 +1944,7 @@ static int gswip_gphy_fw_list(struct gswip_priv *priv,
>  static int gswip_probe(struct platform_device *pdev)
>  {
>  	struct gswip_priv *priv;
> -	struct device_node *mdio_np, *gphy_fw_np;
> +	struct device_node *np, *mdio_np, *gphy_fw_np;
>  	struct device *dev = &pdev->dev;
>  	int err;
>  	int i;
> @@ -1892,10 +1977,28 @@ static int gswip_probe(struct platform_device *pdev)
>  	priv->ds->dev = dev;
>  	priv->ds->num_ports = priv->hw_info->max_ports;
>  	priv->ds->priv = priv;
> -	priv->ds->ops = &gswip_switch_ops;
> +	priv->ds->ops = priv->hw_info->ops;
>  	priv->dev = dev;
>  	version = gswip_switch_r(priv, GSWIP_VERSION);
>  
> +	np = dev->of_node;
> +	switch (version) {
> +	case GSWIP_VERSION_2_0:
> +	case GSWIP_VERSION_2_1:
> +		if (!of_device_is_compatible(np, "lantiq,xrx200-gswip"))
> +			return -EINVAL;
> +		break;
> +	case GSWIP_VERSION_2_2:
> +	case GSWIP_VERSION_2_2_ETC:
> +		if (!of_device_is_compatible(np, "lantiq,xrx300-gswip") &&
> +		    !of_device_is_compatible(np, "lantiq,xrx330-gswip"))
> +			return -EINVAL;
> +		break;
> +	default:
> +		dev_err(dev, "unknown GSWIP version: 0x%x", version);
> +		return -ENOENT;
> +	}
> +
>  	/* bring up the mdio bus */
>  	gphy_fw_np = of_get_compatible_child(dev->of_node, "lantiq,gphy-fw");
>  	if (gphy_fw_np) {
> @@ -1973,10 +2076,19 @@ static int gswip_remove(struct platform_device *pdev)
>  static const struct gswip_hw_info gswip_xrx200 = {
>  	.max_ports = 7,
>  	.cpu_port = 6,
> +	.ops = &gswip_xrx200_switch_ops,
> +};
> +
> +static const struct gswip_hw_info gswip_xrx300 = {
> +	.max_ports = 7,
> +	.cpu_port = 6,
> +	.ops = &gswip_xrx300_switch_ops,
>  };
>  
>  static const struct of_device_id gswip_of_match[] = {
>  	{ .compatible = "lantiq,xrx200-gswip", .data = &gswip_xrx200 },
> +	{ .compatible = "lantiq,xrx300-gswip", .data = &gswip_xrx300 },
> +	{ .compatible = "lantiq,xrx330-gswip", .data = &gswip_xrx300 },
>  	{},
>  };
>  MODULE_DEVICE_TABLE(of, gswip_of_match);
> -- 
> 2.20.1
>
Jakub Kicinski Dec. 7, 2020, 9:43 p.m. UTC | #2
On Sun,  6 Dec 2020 14:27:12 +0100 Aleksander Jan Bajkowski wrote:
> This patch allows to use all PHYs on GRX300 and GRX330. The ARX300 has 3
> and the GRX330 has 4 integrated PHYs connected to different ports compared
> to VRX200.
> 
> Port configurations:
> 
> xRX200:
> GMAC0: RGMII/MII/REVMII/RMII port
> GMAC1: RGMII/MII/REVMII/RMII port
> GMAC2: GPHY0 (GMII)
> GMAC3: GPHY0 (MII)
> GMAC4: GPHY1 (GMII)
> GMAC5: GPHY1 (MII) or RGMII port
> 
> xRX300:
> GMAC0: RGMII port
> GMAC1: GPHY2 (GMII)
> GMAC2: GPHY0 (GMII)
> GMAC3: GPHY0 (MII)
> GMAC4: GPHY1 (GMII)
> GMAC5: GPHY1 (MII) or RGMII port
> 
> xRX330:
> GMAC0: RGMII/GMII/RMII port
> GMAC1: GPHY2 (GMII)
> GMAC2: GPHY0 (GMII)
> GMAC3: GPHY0 (MII) or GPHY3 (GMII)
> GMAC4: GPHY1 (GMII)
> GMAC5: GPHY1 (MII) or RGMII/RMII port
> 
> Tested on D-Link DWR966 with OpenWRT.
> 
> Signed-off-by: Aleksander Jan Bajkowski <olek2@wp.pl>

Please make sure you don't add W=1 C=1 build warnings:

In file included from ../include/linux/kasan-checks.h:5,
                 from ../include/asm-generic/rwonce.h:26,
                 from ./arch/x86/include/generated/asm/rwonce.h:1,
                 from ../include/linux/compiler.h:246,
                 from ../include/linux/err.h:5,
                 from ../include/linux/clk.h:12,
                 from ../drivers/net/dsa/lantiq_gswip.c:28:
drivers/net/dsa/lantiq_gswip.c: In function ‘gswip_xrx300_phylink_validate’:
drivers/net/dsa/lantiq_gswip.c:1496:35: warning: unused variable ‘mask’ [-Wunused-variable]
 1496 |  __ETHTOOL_DECLARE_LINK_MODE_MASK(mask) = { 0, };
      |                                   ^~~~
include/linux/types.h:11:16: note: in definition of macro ‘DECLARE_BITMAP’
   11 |  unsigned long name[BITS_TO_LONGS(bits)]
      |                ^~~~
drivers/net/dsa/lantiq_gswip.c:1496:2: note: in expansion of macro ‘__ETHTOOL_DECLARE_LINK_MODE_MASK’
 1496 |  __ETHTOOL_DECLARE_LINK_MODE_MASK(mask) = { 0, };
      |  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/net/dsa/lantiq_gswip.c: At top level:
drivers/net/dsa/lantiq_gswip.c:2079:9: warning: initialization discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
 2079 |  .ops = &gswip_xrx200_switch_ops,
      |         ^
drivers/net/dsa/lantiq_gswip.c:2085:9: warning: initialization discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
 2085 |  .ops = &gswip_xrx300_switch_ops,
      |         ^
drivers/net/dsa/lantiq_gswip.c:2079:17: warning: incorrect type in initializer (different modifiers)
drivers/net/dsa/lantiq_gswip.c:2079:17:    expected struct dsa_switch_ops *ops
drivers/net/dsa/lantiq_gswip.c:2079:17:    got struct dsa_switch_ops const *
drivers/net/dsa/lantiq_gswip.c:2085:17: warning: incorrect type in initializer (different modifiers)
drivers/net/dsa/lantiq_gswip.c:2085:17:    expected struct dsa_switch_ops *ops
drivers/net/dsa/lantiq_gswip.c:2085:17:    got struct dsa_switch_ops const *
kernel test robot Dec. 9, 2020, 2:46 p.m. UTC | #3
Hi Aleksander,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on robh/for-next]
[also build test WARNING on net-next/master net/master linus/master sparc-next/master v5.10-rc7 next-20201208]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Aleksander-Jan-Bajkowski/net-dsa-lantiq-add-support-for-xRX300-and-xRX330/20201206-213318
base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
config: x86_64-randconfig-s022-20201209 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
reproduce:
        # apt-get install sparse
        # sparse version: v0.6.3-179-ga00755aa-dirty
        # https://github.com/0day-ci/linux/commit/3a3400f9d3cd41f2b72cd741f208a70fca7fe93f
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Aleksander-Jan-Bajkowski/net-dsa-lantiq-add-support-for-xRX300-and-xRX330/20201206-213318
        git checkout 3a3400f9d3cd41f2b72cd741f208a70fca7fe93f
        # save the attached .config to linux build tree
        make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>


"sparse warnings: (new ones prefixed by >>)"
>> drivers/net/dsa/lantiq_gswip.c:2068:17: sparse: sparse: incorrect type in initializer (different modifiers) @@     expected struct dsa_switch_ops *ops @@     got struct dsa_switch_ops const * @@
   drivers/net/dsa/lantiq_gswip.c:2068:17: sparse:     expected struct dsa_switch_ops *ops
   drivers/net/dsa/lantiq_gswip.c:2068:17: sparse:     got struct dsa_switch_ops const *
   drivers/net/dsa/lantiq_gswip.c:2074:17: sparse: sparse: incorrect type in initializer (different modifiers) @@     expected struct dsa_switch_ops *ops @@     got struct dsa_switch_ops const * @@
   drivers/net/dsa/lantiq_gswip.c:2074:17: sparse:     expected struct dsa_switch_ops *ops
   drivers/net/dsa/lantiq_gswip.c:2074:17: sparse:     got struct dsa_switch_ops const *

vim +2068 drivers/net/dsa/lantiq_gswip.c

  2064	
  2065	static const struct gswip_hw_info gswip_xrx200 = {
  2066		.max_ports = 7,
  2067		.cpu_port = 6,
> 2068		.ops = &gswip_xrx200_switch_ops,
  2069	};
  2070	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
kernel test robot Dec. 10, 2020, 8:20 a.m. UTC | #4
Hi Aleksander,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on robh/for-next]
[also build test WARNING on net-next/master net/master linus/master sparc-next/master v5.10-rc7 next-20201209]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Aleksander-Jan-Bajkowski/net-dsa-lantiq-add-support-for-xRX300-and-xRX330/20201206-213318
base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
config: parisc-randconfig-r033-20201209 (attached as .config)
compiler: hppa-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/3a3400f9d3cd41f2b72cd741f208a70fca7fe93f
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Aleksander-Jan-Bajkowski/net-dsa-lantiq-add-support-for-xRX300-and-xRX330/20201206-213318
        git checkout 3a3400f9d3cd41f2b72cd741f208a70fca7fe93f
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=parisc 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   In file included from include/linux/kasan-checks.h:5,
                    from include/asm-generic/rwonce.h:26,
                    from ./arch/parisc/include/generated/asm/rwonce.h:1,
                    from include/linux/compiler.h:232,
                    from include/linux/err.h:5,
                    from include/linux/clk.h:12,
                    from drivers/net/dsa/lantiq_gswip.c:28:
   drivers/net/dsa/lantiq_gswip.c: In function 'gswip_xrx300_phylink_validate':
   drivers/net/dsa/lantiq_gswip.c:1495:35: warning: unused variable 'mask' [-Wunused-variable]
    1495 |  __ETHTOOL_DECLARE_LINK_MODE_MASK(mask) = { 0, };
         |                                   ^~~~
   include/linux/types.h:11:16: note: in definition of macro 'DECLARE_BITMAP'
      11 |  unsigned long name[BITS_TO_LONGS(bits)]
         |                ^~~~
   drivers/net/dsa/lantiq_gswip.c:1495:2: note: in expansion of macro '__ETHTOOL_DECLARE_LINK_MODE_MASK'
    1495 |  __ETHTOOL_DECLARE_LINK_MODE_MASK(mask) = { 0, };
         |  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/net/dsa/lantiq_gswip.c: At top level:
>> drivers/net/dsa/lantiq_gswip.c:2068:9: warning: initialization discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
    2068 |  .ops = &gswip_xrx200_switch_ops,
         |         ^
   drivers/net/dsa/lantiq_gswip.c:2074:9: warning: initialization discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
    2074 |  .ops = &gswip_xrx300_switch_ops,
         |         ^
   drivers/net/dsa/lantiq_gswip.c:1741:34: warning: 'xway_gphy_match' defined but not used [-Wunused-const-variable=]
    1741 | static const struct of_device_id xway_gphy_match[] = {
         |                                  ^~~~~~~~~~~~~~~

vim +/const +2068 drivers/net/dsa/lantiq_gswip.c

  2064	
  2065	static const struct gswip_hw_info gswip_xrx200 = {
  2066		.max_ports = 7,
  2067		.cpu_port = 6,
> 2068		.ops = &gswip_xrx200_switch_ops,
  2069	};
  2070	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
diff mbox series

Patch

diff --git a/drivers/net/dsa/lantiq_gswip.c b/drivers/net/dsa/lantiq_gswip.c
index 09701c17f3f6..4c8f611ed397 100644
--- a/drivers/net/dsa/lantiq_gswip.c
+++ b/drivers/net/dsa/lantiq_gswip.c
@@ -94,6 +94,7 @@ 
 /* GSWIP MII Registers */
 #define GSWIP_MII_CFG0			0x00
 #define GSWIP_MII_CFG1			0x02
+#define GSWIP_MII_CFG3			0xc3
 #define GSWIP_MII_CFG5			0x04
 #define  GSWIP_MII_CFG_EN		BIT(14)
 #define  GSWIP_MII_CFG_LDCLKDIS		BIT(12)
@@ -102,6 +103,7 @@ 
 #define  GSWIP_MII_CFG_MODE_RMIIP	0x2
 #define  GSWIP_MII_CFG_MODE_RMIIM	0x3
 #define  GSWIP_MII_CFG_MODE_RGMII	0x4
+#define  GSWIP_MII_CFG_MODE_GMII	0x9
 #define  GSWIP_MII_CFG_MODE_MASK	0xf
 #define  GSWIP_MII_CFG_RATE_M2P5	0x00
 #define  GSWIP_MII_CFG_RATE_M25	0x10
@@ -222,6 +224,7 @@ 
 struct gswip_hw_info {
 	int max_ports;
 	int cpu_port;
+	struct dsa_switch_ops *ops;
 };
 
 struct xway_gphy_match_data {
@@ -392,12 +395,19 @@  static void gswip_mii_mask(struct gswip_priv *priv, u32 clear, u32 set,
 static void gswip_mii_mask_cfg(struct gswip_priv *priv, u32 clear, u32 set,
 			       int port)
 {
+	struct device_node *np = priv->ds->dev->of_node;
+
 	switch (port) {
 	case 0:
 		gswip_mii_mask(priv, clear, set, GSWIP_MII_CFG0);
 		break;
 	case 1:
-		gswip_mii_mask(priv, clear, set, GSWIP_MII_CFG1);
+		if (of_device_is_compatible(np, "lantiq,xrx200-gswip"))
+			gswip_mii_mask(priv, clear, set, GSWIP_MII_CFG1);
+		break;
+	case 3:
+		if (of_device_is_compatible(np, "lantiq,xrx330-gswip"))
+			gswip_mii_mask(priv, clear, set, GSWIP_MII_CFG3);
 		break;
 	case 5:
 		gswip_mii_mask(priv, clear, set, GSWIP_MII_CFG5);
@@ -1409,12 +1419,40 @@  static int gswip_port_fdb_dump(struct dsa_switch *ds, int port,
 	return 0;
 }
 
-static void gswip_phylink_validate(struct dsa_switch *ds, int port,
-				   unsigned long *supported,
-				   struct phylink_link_state *state)
+static void gswip_phylink_set_capab(unsigned long *supported, struct phylink_link_state *state)
 {
 	__ETHTOOL_DECLARE_LINK_MODE_MASK(mask) = { 0, };
 
+	/* Allow all the expected bits */
+	phylink_set(mask, Autoneg);
+	phylink_set_port_modes(mask);
+	phylink_set(mask, Pause);
+	phylink_set(mask, Asym_Pause);
+
+	/* With the exclusion of MII and Reverse MII, we support Gigabit,
+	 * including Half duplex
+	 */
+	if (state->interface != PHY_INTERFACE_MODE_MII &&
+	    state->interface != PHY_INTERFACE_MODE_REVMII) {
+		phylink_set(mask, 1000baseT_Full);
+		phylink_set(mask, 1000baseT_Half);
+	}
+
+	phylink_set(mask, 10baseT_Half);
+	phylink_set(mask, 10baseT_Full);
+	phylink_set(mask, 100baseT_Half);
+	phylink_set(mask, 100baseT_Full);
+
+	bitmap_and(supported, supported, mask,
+		   __ETHTOOL_LINK_MODE_MASK_NBITS);
+	bitmap_and(state->advertising, state->advertising, mask,
+		   __ETHTOOL_LINK_MODE_MASK_NBITS);
+}
+
+static void gswip_xrx200_phylink_validate(struct dsa_switch *ds, int port,
+					  unsigned long *supported,
+					  struct phylink_link_state *state)
+{
 	switch (port) {
 	case 0:
 	case 1:
@@ -1441,37 +1479,56 @@  static void gswip_phylink_validate(struct dsa_switch *ds, int port,
 		return;
 	}
 
-	/* Allow all the expected bits */
-	phylink_set(mask, Autoneg);
-	phylink_set_port_modes(mask);
-	phylink_set(mask, Pause);
-	phylink_set(mask, Asym_Pause);
+	gswip_phylink_set_capab(supported, state);
 
-	/* With the exclusion of MII and Reverse MII, we support Gigabit,
-	 * including Half duplex
-	 */
-	if (state->interface != PHY_INTERFACE_MODE_MII &&
-	    state->interface != PHY_INTERFACE_MODE_REVMII) {
-		phylink_set(mask, 1000baseT_Full);
-		phylink_set(mask, 1000baseT_Half);
+	return;
+
+unsupported:
+	bitmap_zero(supported, __ETHTOOL_LINK_MODE_MASK_NBITS);
+	dev_err(ds->dev, "Unsupported interface '%s' for port %d\n",
+		phy_modes(state->interface), port);
+}
+
+static void gswip_xrx300_phylink_validate(struct dsa_switch *ds, int port,
+					  unsigned long *supported,
+					  struct phylink_link_state *state)
+{
+	__ETHTOOL_DECLARE_LINK_MODE_MASK(mask) = { 0, };
+
+	switch (port) {
+	case 0:
+		if (!phy_interface_mode_is_rgmii(state->interface) &&
+		    state->interface != PHY_INTERFACE_MODE_GMII &&
+		    state->interface != PHY_INTERFACE_MODE_RMII)
+			goto unsupported;
+		break;
+	case 1:
+	case 2:
+	case 3:
+	case 4:
+		if (state->interface != PHY_INTERFACE_MODE_INTERNAL)
+			goto unsupported;
+		break;
+	case 5:
+		if (!phy_interface_mode_is_rgmii(state->interface) &&
+		    state->interface != PHY_INTERFACE_MODE_INTERNAL &&
+		    state->interface != PHY_INTERFACE_MODE_RMII)
+			goto unsupported;
+		break;
+	default:
+		bitmap_zero(supported, __ETHTOOL_LINK_MODE_MASK_NBITS);
+		dev_err(ds->dev, "Unsupported port: %i\n", port);
+		return;
 	}
 
-	phylink_set(mask, 10baseT_Half);
-	phylink_set(mask, 10baseT_Full);
-	phylink_set(mask, 100baseT_Half);
-	phylink_set(mask, 100baseT_Full);
+	gswip_phylink_set_capab(supported, state);
 
-	bitmap_and(supported, supported, mask,
-		   __ETHTOOL_LINK_MODE_MASK_NBITS);
-	bitmap_and(state->advertising, state->advertising, mask,
-		   __ETHTOOL_LINK_MODE_MASK_NBITS);
 	return;
 
 unsupported:
 	bitmap_zero(supported, __ETHTOOL_LINK_MODE_MASK_NBITS);
 	dev_err(ds->dev, "Unsupported interface '%s' for port %d\n",
 		phy_modes(state->interface), port);
-	return;
 }
 
 static void gswip_phylink_mac_config(struct dsa_switch *ds, int port,
@@ -1500,6 +1557,9 @@  static void gswip_phylink_mac_config(struct dsa_switch *ds, int port,
 	case PHY_INTERFACE_MODE_RGMII_TXID:
 		miicfg |= GSWIP_MII_CFG_MODE_RGMII;
 		break;
+	case PHY_INTERFACE_MODE_GMII:
+		miicfg |= GSWIP_MII_CFG_MODE_GMII;
+		break;
 	default:
 		dev_err(ds->dev,
 			"Unsupported interface: %d\n", state->interface);
@@ -1614,7 +1674,7 @@  static int gswip_get_sset_count(struct dsa_switch *ds, int port, int sset)
 	return ARRAY_SIZE(gswip_rmon_cnt);
 }
 
-static const struct dsa_switch_ops gswip_switch_ops = {
+static const struct dsa_switch_ops gswip_xrx200_switch_ops = {
 	.get_tag_protocol	= gswip_get_tag_protocol,
 	.setup			= gswip_setup,
 	.port_enable		= gswip_port_enable,
@@ -1630,7 +1690,32 @@  static const struct dsa_switch_ops gswip_switch_ops = {
 	.port_fdb_add		= gswip_port_fdb_add,
 	.port_fdb_del		= gswip_port_fdb_del,
 	.port_fdb_dump		= gswip_port_fdb_dump,
-	.phylink_validate	= gswip_phylink_validate,
+	.phylink_validate	= gswip_xrx200_phylink_validate,
+	.phylink_mac_config	= gswip_phylink_mac_config,
+	.phylink_mac_link_down	= gswip_phylink_mac_link_down,
+	.phylink_mac_link_up	= gswip_phylink_mac_link_up,
+	.get_strings		= gswip_get_strings,
+	.get_ethtool_stats	= gswip_get_ethtool_stats,
+	.get_sset_count		= gswip_get_sset_count,
+};
+
+static const struct dsa_switch_ops gswip_xrx300_switch_ops = {
+	.get_tag_protocol	= gswip_get_tag_protocol,
+	.setup			= gswip_setup,
+	.port_enable		= gswip_port_enable,
+	.port_disable		= gswip_port_disable,
+	.port_bridge_join	= gswip_port_bridge_join,
+	.port_bridge_leave	= gswip_port_bridge_leave,
+	.port_fast_age		= gswip_port_fast_age,
+	.port_vlan_filtering	= gswip_port_vlan_filtering,
+	.port_vlan_prepare	= gswip_port_vlan_prepare,
+	.port_vlan_add		= gswip_port_vlan_add,
+	.port_vlan_del		= gswip_port_vlan_del,
+	.port_stp_state_set	= gswip_port_stp_state_set,
+	.port_fdb_add		= gswip_port_fdb_add,
+	.port_fdb_del		= gswip_port_fdb_del,
+	.port_fdb_dump		= gswip_port_fdb_dump,
+	.phylink_validate	= gswip_xrx300_phylink_validate,
 	.phylink_mac_config	= gswip_phylink_mac_config,
 	.phylink_mac_link_down	= gswip_phylink_mac_link_down,
 	.phylink_mac_link_up	= gswip_phylink_mac_link_up,
@@ -1859,7 +1944,7 @@  static int gswip_gphy_fw_list(struct gswip_priv *priv,
 static int gswip_probe(struct platform_device *pdev)
 {
 	struct gswip_priv *priv;
-	struct device_node *mdio_np, *gphy_fw_np;
+	struct device_node *np, *mdio_np, *gphy_fw_np;
 	struct device *dev = &pdev->dev;
 	int err;
 	int i;
@@ -1892,10 +1977,28 @@  static int gswip_probe(struct platform_device *pdev)
 	priv->ds->dev = dev;
 	priv->ds->num_ports = priv->hw_info->max_ports;
 	priv->ds->priv = priv;
-	priv->ds->ops = &gswip_switch_ops;
+	priv->ds->ops = priv->hw_info->ops;
 	priv->dev = dev;
 	version = gswip_switch_r(priv, GSWIP_VERSION);
 
+	np = dev->of_node;
+	switch (version) {
+	case GSWIP_VERSION_2_0:
+	case GSWIP_VERSION_2_1:
+		if (!of_device_is_compatible(np, "lantiq,xrx200-gswip"))
+			return -EINVAL;
+		break;
+	case GSWIP_VERSION_2_2:
+	case GSWIP_VERSION_2_2_ETC:
+		if (!of_device_is_compatible(np, "lantiq,xrx300-gswip") &&
+		    !of_device_is_compatible(np, "lantiq,xrx330-gswip"))
+			return -EINVAL;
+		break;
+	default:
+		dev_err(dev, "unknown GSWIP version: 0x%x", version);
+		return -ENOENT;
+	}
+
 	/* bring up the mdio bus */
 	gphy_fw_np = of_get_compatible_child(dev->of_node, "lantiq,gphy-fw");
 	if (gphy_fw_np) {
@@ -1973,10 +2076,19 @@  static int gswip_remove(struct platform_device *pdev)
 static const struct gswip_hw_info gswip_xrx200 = {
 	.max_ports = 7,
 	.cpu_port = 6,
+	.ops = &gswip_xrx200_switch_ops,
+};
+
+static const struct gswip_hw_info gswip_xrx300 = {
+	.max_ports = 7,
+	.cpu_port = 6,
+	.ops = &gswip_xrx300_switch_ops,
 };
 
 static const struct of_device_id gswip_of_match[] = {
 	{ .compatible = "lantiq,xrx200-gswip", .data = &gswip_xrx200 },
+	{ .compatible = "lantiq,xrx300-gswip", .data = &gswip_xrx300 },
+	{ .compatible = "lantiq,xrx330-gswip", .data = &gswip_xrx300 },
 	{},
 };
 MODULE_DEVICE_TABLE(of, gswip_of_match);