From: Andrew Lunn <andrew@lunn.ch>
To: Lukasz Majewski <lukma@denx.de>
Cc: Florian Fainelli <f.fainelli@gmail.com>,
"David S. Miller" <davem@davemloft.net>,
Karicheri Muralidharan <m-karicheri2@ti.com>,
linux-kernel@vger.kernel.org, Eric Engestrom <eric@engestrom.ch>,
netdev@vger.kernel.org, Kishon Vijay Abraham I <kishon@ti.com>,
Grygorii Strashko <grygorii.strashko@ti.com>
Subject: Re: [PATCH v2 1/2] net: phy: dp83867: Port mirroring support in the DP83867 TI's PHY driver
Date: Sat, 4 Feb 2017 18:32:56 +0100 [thread overview]
Message-ID: <20170204173256.GB8364@lunn.ch> (raw)
In-Reply-To: <1486224132-15420-1-git-send-email-lukma@denx.de>
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
next prev parent reply other threads:[~2017-02-04 17:33 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-02-04 16:02 [PATCH v2 1/2] net: phy: dp83867: Port mirroring support in the DP83867 TI's PHY driver Lukasz Majewski
2017-02-04 16:02 ` [PATCH v2 2/2] net: phy: dp83867: Recover from "port mirroring" N/A MODE4 Lukasz Majewski
2017-02-04 17:32 ` Andrew Lunn [this message]
2017-02-05 14:32 ` [PATCH v2 1/2] net: phy: dp83867: Port mirroring support in the DP83867 TI's PHY driver Lukasz Majewski
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20170204173256.GB8364@lunn.ch \
--to=andrew@lunn.ch \
--cc=davem@davemloft.net \
--cc=eric@engestrom.ch \
--cc=f.fainelli@gmail.com \
--cc=grygorii.strashko@ti.com \
--cc=kishon@ti.com \
--cc=linux-kernel@vger.kernel.org \
--cc=lukma@denx.de \
--cc=m-karicheri2@ti.com \
--cc=netdev@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox