[PATCH v3 1/3] Documentation: devicetree: Add PHY no lane swap binding
Add the documentation to avoid PHY lane swapping. This is a boolean entry to notify the phy device drivers that the TX/RX lanes NO need to be swapped. The use case for this binding mostly happens after wrong HW configuration of PHY IC during bootstrap. Signed-off-by: Lukasz Majewski <lu...@denx.de> --- Changes for v3: - Change binding to property --- Documentation/devicetree/bindings/net/phy.txt | 4 1 file changed, 4 insertions(+) diff --git a/Documentation/devicetree/bindings/net/phy.txt b/Documentation/devicetree/bindings/net/phy.txt index fb5056b..b558576 100644 --- a/Documentation/devicetree/bindings/net/phy.txt +++ b/Documentation/devicetree/bindings/net/phy.txt @@ -39,6 +39,10 @@ Optional Properties: - enet-phy-lane-swap: If set, indicates the PHY will swap the TX/RX lanes to compensate for the board being designed with the lanes swapped. +- enet-phy-lane-no-swap: If set, indicates that PHY will disable swap of the + TX/RX lanes. This property allows the PHY to work correcly after e.g. wrong + bootstrap configuration caused by issues in PCB layout design. + - eee-broken-100tx: - eee-broken-1000t: - eee-broken-10gt: -- 2.1.4
[PATCH v3 2/3] net: phy: dp83867: Add lane swapping support in the DP83867 TI's PHY driver
This patch adds support for enabling or disabling the lane swapping (called
"port mirroring" in PHY's CFG4 register) feature of the DP83867 TI's PHY
device.
One use case is when bootstrap configuration enables this feature (because
of e.g. LED_0 wrong wiring) so then one needs to disable it in software
(at u-boot/Linux).
Signed-off-by: Lukasz Majewski <lu...@denx.de>
---
Changes for v3:
- Add "line swapping" to the patch description
- Add DP83867_PORT_MIRROING_KEEP enum for better code readability
Changes for v2:
- use "net-phy-lane-swap" and "net-phy-lane-no-swap" generic PHY properties.
instead of TI specific one
---
drivers/net/phy/dp83867.c | 38 ++
1 file changed, 38 insertions(+)
diff --git a/drivers/net/phy/dp83867.c b/drivers/net/phy/dp83867.c
index ca1b462..be6fa24 100644
--- a/drivers/net/phy/dp83867.c
+++ b/drivers/net/phy/dp83867.c
@@ -32,6 +32,7 @@
#define DP83867_CFG3 0x1e
/* Extended Registers */
+#define DP83867_CFG40x0031
#define DP83867_RGMIICTL 0x0032
#define DP83867_RGMIIDCTL 0x0086
#define DP83867_IO_MUX_CFG 0x0170
@@ -70,11 +71,21 @@
#define DP83867_IO_MUX_CFG_IO_IMPEDANCE_MAX0x0
#define DP83867_IO_MUX_CFG_IO_IMPEDANCE_MIN0x1f
+/* CFG4 bits */
+#define DP83867_CFG4_PORT_MIRROR_EN BIT(0)
+
+enum {
+ DP83867_PORT_MIRROING_KEEP,
+ DP83867_PORT_MIRROING_EN,
+ DP83867_PORT_MIRROING_DIS,
+};
+
struct dp83867_private {
int rx_id_delay;
int tx_id_delay;
int fifo_depth;
int io_impedance;
+ int port_mirroring;
};
static int dp83867_ack_interrupt(struct phy_device *phydev)
@@ -111,6 +122,24 @@ static int dp83867_config_intr(struct phy_device *phydev)
return phy_write(phydev, MII_DP83867_MICR, micr_status);
}
+static int dp83867_config_port_mirroring(struct phy_device *phydev)
+{
+ struct dp83867_private *dp83867 =
+ (struct dp83867_private *)phydev->priv;
+ u16 val;
+
+ val = phy_read_mmd_indirect(phydev, DP83867_CFG4, DP83867_DEVADDR);
+
+ if (dp83867->port_mirroring == DP83867_PORT_MIRROING_EN)
+ val |= DP83867_CFG4_PORT_MIRROR_EN;
+ else
+ val &= ~DP83867_CFG4_PORT_MIRROR_EN;
+
+ phy_write_mmd_indirect(phydev, DP83867_CFG4, DP83867_DEVADDR, val);
+
+ return 0;
+}
+
#ifdef CONFIG_OF_MDIO
static int dp83867_of_init(struct phy_device *phydev)
{
@@ -144,6 +173,12 @@ static int dp83867_of_init(struct phy_device *phydev)
phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID))
return ret;
+ if (of_property_read_bool(of_node, "enet-phy-lane-swap"))
+ dp83867->port_mirroring = DP83867_PORT_MIRROING_EN;
+
+ if (of_property_read_bool(of_node, "enet-phy-lane-no-swap"))
+ dp83867->port_mirroring = DP83867_PORT_MIRROING_DIS;
+
return of_property_read_u32(of_node, "ti,fifo-depth",
>fifo_depth);
}
@@ -228,6 +263,9 @@ static int dp83867_config_init(struct phy_device *phydev)
phy_write(phydev, DP83867_CFG3, val);
}
+ if (dp83867->port_mirroring != DP83867_PORT_MIRROING_KEEP)
+ dp83867_config_port_mirroring(phydev);
+
return 0;
}
--
2.1.4
[PATCH v3 3/3] net: phy: dp83867: Recover from "port mirroring" N/A MODE4
The DP83867 when not properly bootstrapped - especially with LED_0 pin -
can enter N/A MODE4 for "port mirroring" feature.
To provide normal operation of the PHY, one needs not only to explicitly
disable the port mirroring feature, but as well stop some IC internal
testing (which disables RGMII communication).
To do that the STRAP_STS1 (0x006E) register must be read and RESERVED bit
11 examined. When it is set, the another RESERVED bit (11) at PHYCR
(0x0010) register must be clear to disable testing mode and enable RGMII
communication.
Thorough explanation of the problem can be found at following e2e thread:
"DP83867IR: Problem with RESERVED bits in PHY Control Register (PHYCR) -
Linux driver"
https://e2e.ti.com/support/interface/ethernet/f/903/p/571313/2096954#2096954
Signed-off-by: Lukasz Majewski <lu...@denx.de>
---
Changes for v3:
- None
---
drivers/net/phy/dp83867.c | 23 ++-
1 file changed, 22 insertions(+), 1 deletion(-)
diff --git a/drivers/net/phy/dp83867.c b/drivers/net/phy/dp83867.c
index be6fa24..1986553 100644
--- a/drivers/net/phy/dp83867.c
+++ b/drivers/net/phy/dp83867.c
@@ -34,6 +34,7 @@
/* Extended Registers */
#define DP83867_CFG40x0031
#define DP83867_RGMIICTL 0x0032
+#define DP83867_STRAP_STS1 0x006E
#define DP83867_RGMIIDCTL 0x0086
#define DP83867_IO_MUX_CFG 0x0170
@@ -58,9 +59,13 @@
#define DP83867_RGMII_TX_CLK_DELAY_EN BIT(1)
#define DP83867_RGMII_RX_CLK_DELAY_EN BIT(0)
+/* STRAP_STS1 bits */
+#define DP83867_STRAP_STS1_RESERVEDBIT(11)
+
/* PHY CTRL bits */
#define DP83867_PHYCR_FIFO_DEPTH_SHIFT 14
#define DP83867_PHYCR_FIFO_DEPTH_MASK (3 << 14)
+#define DP83867_PHYCR_RESERVED_MASKBIT(11)
/* RGMIIDCTL bits */
#define DP83867_RGMII_TX_CLK_DELAY_SHIFT 4
@@ -192,7 +197,7 @@ static int dp83867_of_init(struct phy_device *phydev)
static int dp83867_config_init(struct phy_device *phydev)
{
struct dp83867_private *dp83867;
- int ret, val;
+ int ret, val, bs;
u16 delay;
if (!phydev->priv) {
@@ -215,6 +220,22 @@ static int dp83867_config_init(struct phy_device *phydev)
return val;
val &= ~DP83867_PHYCR_FIFO_DEPTH_MASK;
val |= (dp83867->fifo_depth << DP83867_PHYCR_FIFO_DEPTH_SHIFT);
+
+ /* The code below checks if "port mirroring" N/A MODE4 has been
+* enabled during power on bootstrap.
+*
+* Such N/A mode enabled by mistake can put PHY IC in some
+* internal testing mode and disable RGMII transmission.
+*
+* In this particular case one needs to check STRAP_STS1
+* register's bit 11 (marked as RESERVED).
+*/
+
+ bs = phy_read_mmd_indirect(phydev, DP83867_STRAP_STS1,
+ DP83867_DEVADDR);
+ if (bs & DP83867_STRAP_STS1_RESERVED)
+ val &= ~DP83867_PHYCR_RESERVED_MASK;
+
ret = phy_write(phydev, MII_DP83867_PHYCTRL, val);
if (ret)
return ret;
--
2.1.4
Re: [PATCH] Documentation: devicetree: Add PHY no lane swap binding
Hi Florian, Andrew, > Le 02/04/17 à 09:23, Andrew Lunn a écrit : > > On Sat, Feb 04, 2017 at 04:47:47PM +0100, Lukasz Majewski wrote: > >> Add the documentation to avoid PHY lane swapping. This is a boolean > >> entry to notify the phy device drivers that the TX/RX lanes NO need > > > > that the TX/RX lanes should not be swapped. > > > >> to be swapped. > >> The use case for this binding mostly happens after wrong HW > >> configuration of PHY IC during bootstrap. > >> > >> Signed-off-by: Lukasz Majewski <lu...@denx.de> > >> --- > >> Documentation/devicetree/bindings/net/phy.txt | 4 > >> 1 file changed, 4 insertions(+) > >> > >> diff --git a/Documentation/devicetree/bindings/net/phy.txt > >> b/Documentation/devicetree/bindings/net/phy.txt index > >> fb5056b..5e25bc9 100644 --- > >> a/Documentation/devicetree/bindings/net/phy.txt +++ > >> b/Documentation/devicetree/bindings/net/phy.txt @@ -39,6 +39,10 @@ > >> Optional Properties: > >> - enet-phy-lane-swap: If set, indicates the PHY will swap the > >> TX/RX lanes to compensate for the board being designed with the > >> lanes swapped. > >> +- enet-phy-lane-no-swap: If set, indicates that PHY will disable > >> swap of the > >> + TX/RX lanes. This binding allows the PHY to work correcly after > >> e.g. wrong > >> + bootstrap configuration caused by issues in PCB layout design. > > s/binding/property/ > > >> + > > > > We are leaving it undefined what it means if neither > > enet-phy-lane-no-swap nor enet-phy-lane-swap properties are present. > > Do we want to define this? That the swap should be left untouched by > > the driver? > > Since this is a description of the hardware, absence of a properties > should mean that the driver is at freedom to either keep the hardware > defaults, or come up with its own settings that are sensible for that > particular PHY device. > > What would you see clarified here? Any more comments to this patch? Is the explanation informative enough? Best regards, Lukasz Majewski -- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de
Re: [PATCH v2 1/2] net: phy: dp83867: Port mirroring support in the DP83867 TI's PHY driver
Hi Andrew,
> 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.
That was my goal - to use naming from datasheet.
> 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.
Ok. No problem with that.
>
> > +enum {
>
> Maybe give the 0 value a name. DP83867_PORT_MIRROING_KEEP?
I can add this - no problem.
>
> > + 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.
Ok. I see your point.
>
> Thanks
> Andrew
Thanks for review :-)
Best regards,
Lukasz Majewski
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de
[PATCH v2 2/2] net: phy: dp83867: Recover from "port mirroring" N/A MODE4
The DP83867 when not properly bootstrapped - especially with LED_0 pin -
can enter N/A MODE4 for "port mirroring" feature.
To provide normal operation of the PHY, one needs not only to explicitly
disable the port mirroring feature, but as well stop some IC internal
testing (which disables RGMII communication).
To do that the STRAP_STS1 (0x006E) register must be read and RESERVED bit
11 examined. When it is set, the another RESERVED bit (11) at PHYCR
(0x0010) register must be clear to disable testing mode and enable RGMII
communication.
Thorough explanation of the problem can be found at following e2e thread:
"DP83867IR: Problem with RESERVED bits in PHY Control Register (PHYCR) -
Linux driver"
https://e2e.ti.com/support/interface/ethernet/f/903/p/571313/2096954#2096954
Signed-off-by: Lukasz Majewski <lu...@denx.de>
---
drivers/net/phy/dp83867.c | 23 ++-
1 file changed, 22 insertions(+), 1 deletion(-)
diff --git a/drivers/net/phy/dp83867.c b/drivers/net/phy/dp83867.c
index b5f0c2d..7a4208e 100644
--- a/drivers/net/phy/dp83867.c
+++ b/drivers/net/phy/dp83867.c
@@ -34,6 +34,7 @@
/* Extended Registers */
#define DP83867_CFG40x0031
#define DP83867_RGMIICTL 0x0032
+#define DP83867_STRAP_STS1 0x006E
#define DP83867_RGMIIDCTL 0x0086
#define DP83867_IO_MUX_CFG 0x0170
@@ -58,9 +59,13 @@
#define DP83867_RGMII_TX_CLK_DELAY_EN BIT(1)
#define DP83867_RGMII_RX_CLK_DELAY_EN BIT(0)
+/* STRAP_STS1 bits */
+#define DP83867_STRAP_STS1_RESERVEDBIT(11)
+
/* PHY CTRL bits */
#define DP83867_PHYCR_FIFO_DEPTH_SHIFT 14
#define DP83867_PHYCR_FIFO_DEPTH_MASK (3 << 14)
+#define DP83867_PHYCR_RESERVED_MASKBIT(11)
/* RGMIIDCTL bits */
#define DP83867_RGMII_TX_CLK_DELAY_SHIFT 4
@@ -191,7 +196,7 @@ static int dp83867_of_init(struct phy_device *phydev)
static int dp83867_config_init(struct phy_device *phydev)
{
struct dp83867_private *dp83867;
- int ret, val;
+ int ret, val, bs;
u16 delay;
if (!phydev->priv) {
@@ -214,6 +219,22 @@ static int dp83867_config_init(struct phy_device *phydev)
return val;
val &= ~DP83867_PHYCR_FIFO_DEPTH_MASK;
val |= (dp83867->fifo_depth << DP83867_PHYCR_FIFO_DEPTH_SHIFT);
+
+ /* The code below checks if "port mirroring" N/A MODE4 has been
+* enabled during power on bootstrap.
+*
+* Such N/A mode enabled by mistake can put PHY IC in some
+* internal testing mode and disable RGMII transmission.
+*
+* In this particular case one needs to check STRAP_STS1
+* register's bit 11 (marked as RESERVED).
+*/
+
+ bs = phy_read_mmd_indirect(phydev, DP83867_STRAP_STS1,
+ DP83867_DEVADDR);
+ if (bs & DP83867_STRAP_STS1_RESERVED)
+ val &= ~DP83867_PHYCR_RESERVED_MASK;
+
ret = phy_write(phydev, MII_DP83867_PHYCTRL, val);
if (ret)
return ret;
--
2.1.4
[PATCH v2 1/2] net: phy: dp83867: Port mirroring support in the DP83867 TI's PHY driver
This patch adds support for enabling or disabling the port mirroring
(at CFG4 register) feature of the DP83867 TI's PHY device.
One use case is when bootstrap configuration enables this feature (because
of e.g. LED wiring) so then one needs to disable it in software
(u-boot/Linux).
Signed-off-by: Lukasz Majewski <lu...@denx.de>
---
Changes for v2:
- use "net-phy-lane-swap" and "net-phy-lane-no-swap" generic PHY properties.
instead of TI specific one
---
drivers/net/phy/dp83867.c | 37 +
1 file changed, 37 insertions(+)
diff --git a/drivers/net/phy/dp83867.c b/drivers/net/phy/dp83867.c
index ca1b462..b5f0c2d 100644
--- a/drivers/net/phy/dp83867.c
+++ b/drivers/net/phy/dp83867.c
@@ -32,6 +32,7 @@
#define DP83867_CFG3 0x1e
/* Extended Registers */
+#define DP83867_CFG40x0031
#define DP83867_RGMIICTL 0x0032
#define DP83867_RGMIIDCTL 0x0086
#define DP83867_IO_MUX_CFG 0x0170
@@ -70,11 +71,20 @@
#define DP83867_IO_MUX_CFG_IO_IMPEDANCE_MAX0x0
#define DP83867_IO_MUX_CFG_IO_IMPEDANCE_MIN0x1f
+/* CFG4 bits */
+#define DP83867_CFG4_PORT_MIRROR_EN BIT(0)
+
+enum {
+ DP83867_PORT_MIRROING_EN = 1,
+ DP83867_PORT_MIRROING_DIS,
+};
+
struct dp83867_private {
int rx_id_delay;
int tx_id_delay;
int fifo_depth;
int io_impedance;
+ int port_mirroring;
};
static int dp83867_ack_interrupt(struct phy_device *phydev)
@@ -111,6 +121,24 @@ static int dp83867_config_intr(struct phy_device *phydev)
return phy_write(phydev, MII_DP83867_MICR, micr_status);
}
+static int dp83867_config_port_mirroring(struct phy_device *phydev)
+{
+ struct dp83867_private *dp83867 =
+ (struct dp83867_private *)phydev->priv;
+ u16 val;
+
+ val = phy_read_mmd_indirect(phydev, DP83867_CFG4, DP83867_DEVADDR);
+
+ if (dp83867->port_mirroring == DP83867_PORT_MIRROING_EN)
+ val |= DP83867_CFG4_PORT_MIRROR_EN;
+ else
+ val &= ~DP83867_CFG4_PORT_MIRROR_EN;
+
+ phy_write_mmd_indirect(phydev, DP83867_CFG4, DP83867_DEVADDR, val);
+
+ return 0;
+}
+
#ifdef CONFIG_OF_MDIO
static int dp83867_of_init(struct phy_device *phydev)
{
@@ -144,6 +172,12 @@ static int dp83867_of_init(struct phy_device *phydev)
phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID))
return ret;
+ if (of_property_read_bool(of_node, "enet-phy-lane-swap"))
+ dp83867->port_mirroring = DP83867_PORT_MIRROING_EN;
+
+ if (of_property_read_bool(of_node, "enet-phy-lane-no-swap"))
+ dp83867->port_mirroring = DP83867_PORT_MIRROING_DIS;
+
return of_property_read_u32(of_node, "ti,fifo-depth",
>fifo_depth);
}
@@ -228,6 +262,9 @@ static int dp83867_config_init(struct phy_device *phydev)
phy_write(phydev, DP83867_CFG3, val);
}
+ if (dp83867->port_mirroring)
+ dp83867_config_port_mirroring(phydev);
+
return 0;
}
--
2.1.4
[PATCH] Documentation: devicetree: Add PHY no lane swap binding
Add the documentation to avoid PHY lane swapping. This is a boolean entry to notify the phy device drivers that the TX/RX lanes NO need to be swapped. The use case for this binding mostly happens after wrong HW configuration of PHY IC during bootstrap. Signed-off-by: Lukasz Majewski <lu...@denx.de> --- Documentation/devicetree/bindings/net/phy.txt | 4 1 file changed, 4 insertions(+) diff --git a/Documentation/devicetree/bindings/net/phy.txt b/Documentation/devicetree/bindings/net/phy.txt index fb5056b..5e25bc9 100644 --- a/Documentation/devicetree/bindings/net/phy.txt +++ b/Documentation/devicetree/bindings/net/phy.txt @@ -39,6 +39,10 @@ Optional Properties: - enet-phy-lane-swap: If set, indicates the PHY will swap the TX/RX lanes to compensate for the board being designed with the lanes swapped. +- enet-phy-lane-no-swap: If set, indicates that PHY will disable swap of the + TX/RX lanes. This binding allows the PHY to work correcly after e.g. wrong + bootstrap configuration caused by issues in PCB layout design. + - eee-broken-100tx: - eee-broken-1000t: - eee-broken-10gt: -- 2.1.4
Re: [PATCH] net: phy: dp83867: Port mirroring support in the DP83867 TI's PHY driver
On Thu, 2 Feb 2017 14:11:12 +0100 Andrew Lunn <and...@lunn.ch> wrote: > > The bootstrapping process in the PHY sets this bit. This is wrong > > since the board lane layout is not "swapped" > > Ah, you mean a strapping pin? Resistor to ground/VCC? Yes, exactly. > That is a different matter. It makes it a lot less likely to break > some existing board with such a change. > > > I have thought a bit about that and I think that we should define > > complementary "net-phy-lane-no-swap" as suggested by Florian. Then > > affected boards could define it and use. > > This is the most flexible solution. Yes, that is O.K. for me. Ok. thanks. > > Andrew Best regards, Lukasz Majewski -- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de
Re: [PATCH] net: phy: dp83867: Port mirroring support in the DP83867 TI's PHY driver
Hi Andrew, > On Wed, Feb 01, 2017 at 11:13:23PM +0100, Lukasz Majewski wrote: > > Hi Andrew, > > > > > > We would need a tri-state device tree properly: > > > > > > > > 1. Not defined - do nothing > > > > 2. Defined as 0 -> explicitly disable port mirroring > > > > 3. Defined as 1 -> explicitly enable port mirriring > > > > > > > > The "net-phy-lane-swap" only fulfills points 1 and 3 above. > > > > > > > > In my use case I do need point 2. > > > > > > Looking at the datasheet, PORT_MIRROR_EN defaults to 0. So it > > > seems reasonable to unconditionally set it to 0 when the PHY > > > driver loads. Any device which needs a value of 1 can set > > > "net-phy-lane-swap" > > > > Unconditionally setting this field to 0 (as we expect the reset > > default setting) seems to me like a good solution. > > > > However, I was not sure if such approach is acceptable by the > > community. > > So the issue here is, are there boards with bootloaders which set this > "lane swap" bit, The bootstrapping process in the PHY sets this bit. This is wrong since the board lane layout is not "swapped" The bootloader (u-boot) fixes this, since we need to support networking (tftp, ping). > and rely on Linux not changing it, When we boot Linux everything is OK, until the dp83867 driver comes into play and performs reset (including register reset). Then the "bootstrap", initial line swap is setup again (wrongly). So we need a way in Linux to make things correct again. > because the > hardware design has such a swap? > > It seems the bootloader you are using does this. The bootloader fixes things, but then in Linux the PHY driver (dp83867.c) performs "RESET" which breaks networking again. > But in your case, it > does it wrongly. The bootloader does its job correctly. > I'm guessing you have a vendor bootloader? And no > easy access to the sources? Mainline u-boot (all vendor code will be upstreamed). > Otherwise you would of fixed the > bootloader. So can we assume there are vendor designed boards which > require the swap? Do they run a mainline kernel? Or only the vendor > kernel? All stuff is going to run mainline kernel (LTS - v4.9 ?). > > If we know of mainline boards which are going to break, we should not > do this. > > If however we do decide to reset it to default value, i think it would > be good to implement net-phy-lane-swap as well, so giving people an > easier path towards mainline. I have thought a bit about that and I think that we should define complementary "net-phy-lane-no-swap" as suggested by Florian. Then affected boards could define it and use. > >Andrew Best regards, Lukasz Majewski -- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de
Re: [PATCH] net: phy: dp83867: Port mirroring support in the DP83867 TI's PHY driver
Hi Andrew, > > We would need a tri-state device tree properly: > > > > 1. Not defined - do nothing > > 2. Defined as 0 -> explicitly disable port mirroring > > 3. Defined as 1 -> explicitly enable port mirriring > > > > The "net-phy-lane-swap" only fulfills points 1 and 3 above. > > > > In my use case I do need point 2. > > Looking at the datasheet, PORT_MIRROR_EN defaults to 0. So it seems > reasonable to unconditionally set it to 0 when the PHY driver > loads. Any device which needs a value of 1 can set "net-phy-lane-swap" Unconditionally setting this field to 0 (as we expect the reset default setting) seems to me like a good solution. However, I was not sure if such approach is acceptable by the community. > >Andrew Best regards, Lukasz Majewski -- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de
Re: [PATCH] net: phy: dp83867: Port mirroring support in the DP83867 TI's PHY driver
Dear All, Thanks for prompt reply. > On 02/01/2017 09:16 AM, Andrew Lunn wrote: > > On Wed, Feb 01, 2017 at 03:43:35PM +0100, Lukasz Majewski wrote: > >> This patch adds support for enabling or disabling the port > >> mirroring feature of the DP83867 TI's PHY device. > >> > >> One use case is when bootstrap configuration enables this feature > >> (because of e.g. LED wiring) so then one needs to disable it in > >> software (u-boot/Linux). > > > > Hi Lukasz > > > > How does this differ from "enet-phy-lane-swap"? > > Same here, I am confused about what port mirroring could be meaning > here The "net-phy-lane-swap" when defined indicates that the "lane swap" needs to be enabled. This is a simple bool variable. In my case it would mean: "please enable port mirroring -> write 1 to CFG4 register's PORT_MIRROR_EN field" My use case is unfortunately different: - Due to HW design - during the bootstrap PHY phase - the PHY enables "port mirroring", which is incorrect. Then, in SW I do need to explicitly disable port mirroring (write 0 to PORT_MIRROR_EN field in CFG4 register). > and if we can find a better way to describe what is being added. > Thanks! We would need a tri-state device tree properly: 1. Not defined - do nothing 2. Defined as 0 -> explicitly disable port mirroring 3. Defined as 1 -> explicitly enable port mirriring The "net-phy-lane-swap" only fulfills points 1 and 3 above. In my use case I do need point 2. Best regards, Lukasz Majewski -- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de
[PATCH] net: phy: dp83867: Port mirroring support in the DP83867 TI's PHY driver
This patch adds support for enabling or disabling the port mirroring
feature of the DP83867 TI's PHY device.
One use case is when bootstrap configuration enables this feature (because
of e.g. LED wiring) so then one needs to disable it in software
(u-boot/Linux).
Signed-off-by: Lukasz Majewski <lu...@denx.de>
---
.../devicetree/bindings/net/ti,dp83867.txt | 4 +++
drivers/net/phy/dp83867.c | 40 ++
2 files changed, 44 insertions(+)
diff --git a/Documentation/devicetree/bindings/net/ti,dp83867.txt
b/Documentation/devicetree/bindings/net/ti,dp83867.txt
index afe9630..d1be241 100644
--- a/Documentation/devicetree/bindings/net/ti,dp83867.txt
+++ b/Documentation/devicetree/bindings/net/ti,dp83867.txt
@@ -18,6 +18,10 @@ Optional property:
- ti,max-output-impedance - MAC Interface Impedance control to set
the programmable output impedance to
maximum value (70 ohms).
+ - ti,port-mirroring - Set port mirroring. It is possible to overwrite -
+ enable (by setting <1>) or disable
+ (by setting <0>) the port mirroring feature set
+ by bootstrap initial reading.
Note: ti,min-output-impedance and ti,max-output-impedance are mutually
exclusive. When both properties are present ti,max-output-impedance
diff --git a/drivers/net/phy/dp83867.c b/drivers/net/phy/dp83867.c
index ca1b462..98948bd 100644
--- a/drivers/net/phy/dp83867.c
+++ b/drivers/net/phy/dp83867.c
@@ -32,6 +32,7 @@
#define DP83867_CFG3 0x1e
/* Extended Registers */
+#define DP83867_CFG40x0031
#define DP83867_RGMIICTL 0x0032
#define DP83867_RGMIIDCTL 0x0086
#define DP83867_IO_MUX_CFG 0x0170
@@ -70,11 +71,20 @@
#define DP83867_IO_MUX_CFG_IO_IMPEDANCE_MAX0x0
#define DP83867_IO_MUX_CFG_IO_IMPEDANCE_MIN0x1f
+/* CFG4 bits */
+#define DP83867_CFG4_PORT_MIRROR_EN BIT(0)
+
+enum {
+ DP83867_PORT_MIRROING_EN = 1,
+ DP83867_PORT_MIRROING_DIS,
+};
+
struct dp83867_private {
int rx_id_delay;
int tx_id_delay;
int fifo_depth;
int io_impedance;
+ int port_mirroring;
};
static int dp83867_ack_interrupt(struct phy_device *phydev)
@@ -111,6 +121,24 @@ static int dp83867_config_intr(struct phy_device *phydev)
return phy_write(phydev, MII_DP83867_MICR, micr_status);
}
+static int dp83867_config_port_mirroring(struct phy_device *phydev)
+{
+ struct dp83867_private *dp83867 =
+ (struct dp83867_private *)phydev->priv;
+ u16 val;
+
+ val = phy_read_mmd_indirect(phydev, DP83867_CFG4, DP83867_DEVADDR);
+
+ if (dp83867->port_mirroring == DP83867_PORT_MIRROING_EN)
+ val |= DP83867_CFG4_PORT_MIRROR_EN;
+ else
+ val &= ~DP83867_CFG4_PORT_MIRROR_EN;
+
+ phy_write_mmd_indirect(phydev, DP83867_CFG4, DP83867_DEVADDR, val);
+
+ return 0;
+}
+
#ifdef CONFIG_OF_MDIO
static int dp83867_of_init(struct phy_device *phydev)
{
@@ -118,6 +146,7 @@ static int dp83867_of_init(struct phy_device *phydev)
struct device *dev = >mdio.dev;
struct device_node *of_node = dev->of_node;
int ret;
+ u32 v;
if (!of_node)
return -ENODEV;
@@ -144,6 +173,14 @@ static int dp83867_of_init(struct phy_device *phydev)
phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID))
return ret;
+ ret = of_property_read_u32(of_node, "ti,port-mirroring", );
+ if (!ret) {
+ if (v)
+ dp83867->port_mirroring = DP83867_PORT_MIRROING_EN;
+ else
+ dp83867->port_mirroring = DP83867_PORT_MIRROING_DIS;
+ }
+
return of_property_read_u32(of_node, "ti,fifo-depth",
>fifo_depth);
}
@@ -228,6 +265,9 @@ static int dp83867_config_init(struct phy_device *phydev)
phy_write(phydev, DP83867_CFG3, val);
}
+ if (dp83867->port_mirroring)
+ dp83867_config_port_mirroring(phydev);
+
return 0;
}
--
2.1.4
