Re: [PATCH 2/3] net: renesas: rswitch: add offloading for L2 switching

[Andrew Lunn <andrew@lunn.ch>]

>  #include <linux/platform_device.h>
> +#include <linux/phy.h>
> +

It seems odd that a patch adding L2 support needs to touch PHYs?

> @@ -994,10 +1018,18 @@ struct rswitch_device {
>  	DECLARE_BITMAP(ts_skb_used, TS_TAGS_PER_PORT);
>  	bool disabled;
>  
> +	struct list_head list;
> +
>  	int port;
>  	struct rswitch_etha *etha;
>  	struct device_node *np_port;
>  	struct phy *serdes;
> +
> +	struct net_device *brdev;	/* master bridge device */

How many ports does this device have? If it is just two, this might
work. But for a multi-port device, you need to keep this in the port
structure.

> +bool is_rdev(const struct net_device *ndev);
> +void rswitch_modify(void __iomem *addr, enum rswitch_reg reg, u32 clear, u32 set);

Are these actually needed? It seems like they could be local
functions.

> +	if (offload_brdev && !priv->offload_brdev)
> +		dev_info(&priv->pdev->dev, "starting l2 offload for %s\n",
> +			 netdev_name(offload_brdev));
> +	else if (!offload_brdev && priv->offload_brdev)
> +		dev_info(&priv->pdev->dev, "stopping l2 offload for %s\n",
> +			 netdev_name(priv->offload_brdev));

Please don't spam the log like this dev_dbg() maybe.

> @@ -128,6 +134,14 @@ static void rswitch_fwd_init(struct rswitch_private *priv)
>  		iowrite32(0, priv->addr + FWPBFC(i));
>  	}
>  
> +	/* Configure MAC table aging */
> +	rswitch_modify(priv->addr, FWMACAGUSPC, FWMACAGUSPC_MACAGUSP,
> +		       FIELD_PREP(FWMACAGUSPC_MACAGUSP, 0x140));
> +
> +	reg_val = FIELD_PREP(FWMACAGC_MACAGT, RSW_AGEING_TIME);
> +	reg_val |= FWMACAGC_MACAGE | FWMACAGC_MACAGSL;
> +	iowrite32(reg_val, priv->addr + FWMACAGC);
> +

Please pull ageing out into a patch of its own.

    Andrew

---
pw-bot: cr