Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1573719

Re: [PATCH v2 1/2] net: phy: dp83867: Port mirroring support in the DP83867 TI's PHY driver

Show all headers | View raw

On Sat, Feb 04, 2017 at 05:02:11PM +0100, Lukasz Majewski wrote:
> This patch adds support for enabling or disabling the port mirroring 
> (at CFG4 register) feature of the DP83867 TI's PHY device.

As we discussed before, "port mirroring" is bad naming. Yes, we should
use it, because that is what the datasheet calls this feature. But the
commit message should also contain a description of what this means,
and reference that the linux name for this concept is lane swapping.

> +enum {

Maybe give the 0 value a name. DP83867_PORT_MIRROING_KEEP?

> +	DP83867_PORT_MIRROING_EN = 1,
> +	DP83867_PORT_MIRROING_DIS,
> +};
> +

That extra enum value can then make this more obvious:
  
        if (dp83867->port_mirroring != DP83867_PORT_MIRROING_KEEP)
		dp83867_config_port_mirroring(phydev);

On the first reading of the patch, i though you were setting mirroring
on/off under all conditions, but in fact you don't. This makes it
clearer.

	Thanks
		Andrew

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[PATCH v2 1/2] net: phy: dp83867: Port mirroring support in the DP83867 TI's PHY driver Lukasz Majewski <lukma@denx.de> - 2017-02-04 17:10 +0100
  Re: [PATCH v2 1/2] net: phy: dp83867: Port mirroring support in the  DP83867 TI's PHY driver Andrew Lunn <andrew@lunn.ch> - 2017-02-04 18:40 +0100
    Re: [PATCH v2 1/2] net: phy: dp83867: Port mirroring support in the  DP83867 TI's PHY driver Lukasz Majewski <lukma@denx.de> - 2017-02-05 15:40 +0100

csiph-web