Add support for configurable impedance control for TI dp83867
phy via devicetree. More documentation in [1].
CPSW second ethernet is not working, fix it by enabling
impedance configuration on the phy.
Verified the patch on DRA72 Rev C evm, logs at [2]. Also pushed
a branch [3] for others to test.
Changes from initial version:
* As per Sekhar's comment, instead of passing impedance values,
change to max and min impedance from DT
* Adopted phy_read_mmd_indirect() to cunnrent implementation.
* Corrected the phy delay timings to the optimal value.
[1] - http://www.ti.com/lit/ds/symlink/dp83867ir.pdf
[2] - http://pastebin.ubuntu.com/23274616/
[3] - git://git.ti.com/~mugunthanvnm/ti-linux-kernel/linux.git dp83867-v2
Mugunthan V N (4):
net: phy: dp83867: Add documentation for optional impedance control
net: phy: dp83867: add support for MAC impedance configuration
ARM: dts: dra72-evm-revc: fix correct phy delay and impedance settings
ARM: dts: dra72-evm-revc: fix correct phy delay
.../devicetree/bindings/net/ti,dp83867.txt | 12 ++++++++++
arch/arm/boot/dts/dra72-evm-revc.dts | 10 ++++----
drivers/net/phy/dp83867.c | 28 ++++++++++++++++++++++
3 files changed, 46 insertions(+), 4 deletions(-)
--
2.10.0.372.g6fe1b14
The default impedance settings of the phy is not the optimal
value, due to this the second ethernet is not working. Fix it
with correct values which makes the second ethernet port to work.
Signed-off-by: Mugunthan V N <[email protected]>
---
arch/arm/boot/dts/dra72-evm-revc.dts | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/arm/boot/dts/dra72-evm-revc.dts b/arch/arm/boot/dts/dra72-evm-revc.dts
index f9cfd3b..d626cd7 100644
--- a/arch/arm/boot/dts/dra72-evm-revc.dts
+++ b/arch/arm/boot/dts/dra72-evm-revc.dts
@@ -62,6 +62,7 @@
ti,rx-internal-delay = <DP83867_RGMIIDCTL_2_00_NS>;
ti,tx-internal-delay = <DP83867_RGMIIDCTL_1_NS>;
ti,fifo-depth = <DP83867_PHYCR_FIFO_DEPTH_8_B_NIB>;
+ ti,min-output-imepdance;
};
dp83867_1: ethernet-phy@3 {
@@ -69,5 +70,6 @@
ti,rx-internal-delay = <DP83867_RGMIIDCTL_2_00_NS>;
ti,tx-internal-delay = <DP83867_RGMIIDCTL_1_NS>;
ti,fifo-depth = <DP83867_PHYCR_FIFO_DEPTH_8_B_NIB>;
+ ti,min-output-imepdance;
};
};
--
2.10.0.372.g6fe1b14
The current delay settings of the phy are not the optimal value,
fix it with correct values.
Signed-off-by: Mugunthan V N <[email protected]>
---
arch/arm/boot/dts/dra72-evm-revc.dts | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch/arm/boot/dts/dra72-evm-revc.dts b/arch/arm/boot/dts/dra72-evm-revc.dts
index d626cd7..8472a8c 100644
--- a/arch/arm/boot/dts/dra72-evm-revc.dts
+++ b/arch/arm/boot/dts/dra72-evm-revc.dts
@@ -59,16 +59,16 @@
&davinci_mdio {
dp83867_0: ethernet-phy@2 {
reg = <2>;
- ti,rx-internal-delay = <DP83867_RGMIIDCTL_2_00_NS>;
- ti,tx-internal-delay = <DP83867_RGMIIDCTL_1_NS>;
+ ti,rx-internal-delay = <DP83867_RGMIIDCTL_2_25_NS>;
+ ti,tx-internal-delay = <DP83867_RGMIIDCTL_250_PS>;
ti,fifo-depth = <DP83867_PHYCR_FIFO_DEPTH_8_B_NIB>;
ti,min-output-imepdance;
};
dp83867_1: ethernet-phy@3 {
reg = <3>;
- ti,rx-internal-delay = <DP83867_RGMIIDCTL_2_00_NS>;
- ti,tx-internal-delay = <DP83867_RGMIIDCTL_1_NS>;
+ ti,rx-internal-delay = <DP83867_RGMIIDCTL_2_25_NS>;
+ ti,tx-internal-delay = <DP83867_RGMIIDCTL_250_PS>;
ti,fifo-depth = <DP83867_PHYCR_FIFO_DEPTH_8_B_NIB>;
ti,min-output-imepdance;
};
--
2.10.0.372.g6fe1b14
Add documention of ti,impedance-control which can be used to
correct MAC impedance mismatch using phy extended registers.
Signed-off-by: Mugunthan V N <[email protected]>
---
Documentation/devicetree/bindings/net/ti,dp83867.txt | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/Documentation/devicetree/bindings/net/ti,dp83867.txt b/Documentation/devicetree/bindings/net/ti,dp83867.txt
index 5d21141..85bf945 100644
--- a/Documentation/devicetree/bindings/net/ti,dp83867.txt
+++ b/Documentation/devicetree/bindings/net/ti,dp83867.txt
@@ -9,6 +9,18 @@ Required properties:
- ti,fifo-depth - Transmitt FIFO depth- see dt-bindings/net/ti-dp83867.h
for applicable values
+Optional property:
+ - ti,min-output-impedance - MAC Interface Impedance control to set
+ the programmable output impedance to
+ minimum value (35 ohms).
+ - ti,max-output-impedance - MAC Interface Impedance control to set
+ the programmable output impedance to
+ maximum value (70 ohms).
+
+Note: ti,min-output-impedance and ti,max-output-impedance are mutually
+ exclusive. When both properties are present ti,max-output-impedance
+ takes precedence.
+
Default child nodes are standard Ethernet PHY device
nodes as described in Documentation/devicetree/bindings/net/phy.txt
--
2.10.0.372.g6fe1b14
Add support for programmable MAC impedance configuration
Signed-off-by: Mugunthan V N <[email protected]>
---
drivers/net/phy/dp83867.c | 28 ++++++++++++++++++++++++++++
1 file changed, 28 insertions(+)
diff --git a/drivers/net/phy/dp83867.c b/drivers/net/phy/dp83867.c
index 91177a4..795ae17 100644
--- a/drivers/net/phy/dp83867.c
+++ b/drivers/net/phy/dp83867.c
@@ -33,6 +33,7 @@
/* Extended Registers */
#define DP83867_RGMIICTL 0x0032
#define DP83867_RGMIIDCTL 0x0086
+#define DP83867_IO_MUX_CFG 0x0170
#define DP83867_SW_RESET BIT(15)
#define DP83867_SW_RESTART BIT(14)
@@ -62,10 +63,17 @@
/* RGMIIDCTL bits */
#define DP83867_RGMII_TX_CLK_DELAY_SHIFT 4
+/* IO_MUX_CFG bits */
+#define DP83867_IO_MUX_CFG_IO_IMPEDANCE_CTRL 0x1f
+
+#define DP83867_IO_MUX_CFG_IO_IMPEDANCE_MAX 0x0
+#define DP83867_IO_MUX_CFG_IO_IMPEDANCE_MIN 0x1f
+
struct dp83867_private {
int rx_id_delay;
int tx_id_delay;
int fifo_depth;
+ int io_impedance;
};
static int dp83867_ack_interrupt(struct phy_device *phydev)
@@ -111,6 +119,14 @@ static int dp83867_of_init(struct phy_device *phydev)
if (!of_node)
return -ENODEV;
+ dp83867->io_impedance = -EINVAL;
+
+ /* Optional configuration */
+ if (of_property_read_bool(of_node, "ti,max-output-imepdance"))
+ dp83867->io_impedance = DP83867_IO_MUX_CFG_IO_IMPEDANCE_MAX;
+ else if (of_property_read_bool(of_node, "ti,min-output-imepdance"))
+ dp83867->io_impedance = DP83867_IO_MUX_CFG_IO_IMPEDANCE_MIN;
+
ret = of_property_read_u32(of_node, "ti,rx-internal-delay",
&dp83867->rx_id_delay);
if (ret)
@@ -184,6 +200,18 @@ static int dp83867_config_init(struct phy_device *phydev)
phy_write_mmd_indirect(phydev, DP83867_RGMIIDCTL,
DP83867_DEVADDR, delay);
+
+ if (dp83867->io_impedance >= 0) {
+ val = phy_read_mmd_indirect(phydev, DP83867_IO_MUX_CFG,
+ DP83867_DEVADDR);
+
+ val &= ~DP83867_IO_MUX_CFG_IO_IMPEDANCE_CTRL;
+ val |= dp83867->io_impedance &
+ DP83867_IO_MUX_CFG_IO_IMPEDANCE_CTRL;
+
+ phy_write_mmd_indirect(phydev, DP83867_IO_MUX_CFG,
+ DP83867_DEVADDR, val);
+ }
}
return 0;
--
2.10.0.372.g6fe1b14
> + if (of_property_read_bool(of_node, "ti,max-output-imepdance"))
> + dp83867->io_impedance = DP83867_IO_MUX_CFG_IO_IMPEDANCE_MAX;
> + else if (of_property_read_bool(of_node, "ti,min-output-imepdance"))
Did you really test this? Or did you make the same typos in your device
tree file?
Andrew
On Tue, Oct 04, 2016 at 06:26:06PM +0530, Mugunthan V N wrote:
> The default impedance settings of the phy is not the optimal
> value, due to this the second ethernet is not working. Fix it
> with correct values which makes the second ethernet port to work.
>
> Signed-off-by: Mugunthan V N <[email protected]>
> ---
> arch/arm/boot/dts/dra72-evm-revc.dts | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/arch/arm/boot/dts/dra72-evm-revc.dts b/arch/arm/boot/dts/dra72-evm-revc.dts
> index f9cfd3b..d626cd7 100644
> --- a/arch/arm/boot/dts/dra72-evm-revc.dts
> +++ b/arch/arm/boot/dts/dra72-evm-revc.dts
> @@ -62,6 +62,7 @@
> ti,rx-internal-delay = <DP83867_RGMIIDCTL_2_00_NS>;
> ti,tx-internal-delay = <DP83867_RGMIIDCTL_1_NS>;
> ti,fifo-depth = <DP83867_PHYCR_FIFO_DEPTH_8_B_NIB>;
> + ti,min-output-imepdance;
And there is my answer :-(
Andrew
On Tuesday 04 October 2016 06:26 PM, Mugunthan V N wrote:
> Add support for programmable MAC impedance configuration
>
> Signed-off-by: Mugunthan V N <[email protected]>
> ---
> drivers/net/phy/dp83867.c | 28 ++++++++++++++++++++++++++++
> 1 file changed, 28 insertions(+)
>
> diff --git a/drivers/net/phy/dp83867.c b/drivers/net/phy/dp83867.c
> index 91177a4..795ae17 100644
> --- a/drivers/net/phy/dp83867.c
> +++ b/drivers/net/phy/dp83867.c
> @@ -33,6 +33,7 @@
> /* Extended Registers */
> #define DP83867_RGMIICTL 0x0032
> #define DP83867_RGMIIDCTL 0x0086
> +#define DP83867_IO_MUX_CFG 0x0170
>
> #define DP83867_SW_RESET BIT(15)
> #define DP83867_SW_RESTART BIT(14)
> @@ -62,10 +63,17 @@
> /* RGMIIDCTL bits */
> #define DP83867_RGMII_TX_CLK_DELAY_SHIFT 4
>
> +/* IO_MUX_CFG bits */
> +#define DP83867_IO_MUX_CFG_IO_IMPEDANCE_CTRL 0x1f
> +
> +#define DP83867_IO_MUX_CFG_IO_IMPEDANCE_MAX 0x0
> +#define DP83867_IO_MUX_CFG_IO_IMPEDANCE_MIN 0x1f
> +
> struct dp83867_private {
> int rx_id_delay;
> int tx_id_delay;
> int fifo_depth;
> + int io_impedance;
> };
>
> static int dp83867_ack_interrupt(struct phy_device *phydev)
> @@ -111,6 +119,14 @@ static int dp83867_of_init(struct phy_device *phydev)
> if (!of_node)
> return -ENODEV;
>
> + dp83867->io_impedance = -EINVAL;
> +
> + /* Optional configuration */
> + if (of_property_read_bool(of_node, "ti,max-output-imepdance"))
s/imepdance/impedance
> + dp83867->io_impedance = DP83867_IO_MUX_CFG_IO_IMPEDANCE_MAX;
> + else if (of_property_read_bool(of_node, "ti,min-output-imepdance"))
s/imepdance/impedance
Thanks and regards,
Lokesh
> + dp83867->io_impedance = DP83867_IO_MUX_CFG_IO_IMPEDANCE_MIN;
> +
> ret = of_property_read_u32(of_node, "ti,rx-internal-delay",
> &dp83867->rx_id_delay);
> if (ret)
> @@ -184,6 +200,18 @@ static int dp83867_config_init(struct phy_device *phydev)
>
> phy_write_mmd_indirect(phydev, DP83867_RGMIIDCTL,
> DP83867_DEVADDR, delay);
> +
> + if (dp83867->io_impedance >= 0) {
> + val = phy_read_mmd_indirect(phydev, DP83867_IO_MUX_CFG,
> + DP83867_DEVADDR);
> +
> + val &= ~DP83867_IO_MUX_CFG_IO_IMPEDANCE_CTRL;
> + val |= dp83867->io_impedance &
> + DP83867_IO_MUX_CFG_IO_IMPEDANCE_CTRL;
> +
> + phy_write_mmd_indirect(phydev, DP83867_IO_MUX_CFG,
> + DP83867_DEVADDR, val);
> + }
> }
>
> return 0;
>
On Tuesday 04 October 2016 06:26 PM, Mugunthan V N wrote:
> The default impedance settings of the phy is not the optimal
> value, due to this the second ethernet is not working. Fix it
> with correct values which makes the second ethernet port to work.
>
> Signed-off-by: Mugunthan V N <[email protected]>
> ---
> arch/arm/boot/dts/dra72-evm-revc.dts | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/arch/arm/boot/dts/dra72-evm-revc.dts b/arch/arm/boot/dts/dra72-evm-revc.dts
> index f9cfd3b..d626cd7 100644
> --- a/arch/arm/boot/dts/dra72-evm-revc.dts
> +++ b/arch/arm/boot/dts/dra72-evm-revc.dts
> @@ -62,6 +62,7 @@
> ti,rx-internal-delay = <DP83867_RGMIIDCTL_2_00_NS>;
> ti,tx-internal-delay = <DP83867_RGMIIDCTL_1_NS>;
> ti,fifo-depth = <DP83867_PHYCR_FIFO_DEPTH_8_B_NIB>;
> + ti,min-output-imepdance;
s/imepdance/impedance
> };
>
> dp83867_1: ethernet-phy@3 {
> @@ -69,5 +70,6 @@
> ti,rx-internal-delay = <DP83867_RGMIIDCTL_2_00_NS>;
> ti,tx-internal-delay = <DP83867_RGMIIDCTL_1_NS>;
> ti,fifo-depth = <DP83867_PHYCR_FIFO_DEPTH_8_B_NIB>;
> + ti,min-output-imepdance;
same here.
Thanks and regards,
Lokesh
> };
> };
>
On Tue, Oct 04, 2016 at 06:26:07PM +0530, Mugunthan V N wrote:
> The current delay settings of the phy are not the optimal value,
> fix it with correct values.
This should be a separate patch, since it has nothing to do with impedance.
Andrew
>
> Signed-off-by: Mugunthan V N <[email protected]>
> ---
> arch/arm/boot/dts/dra72-evm-revc.dts | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm/boot/dts/dra72-evm-revc.dts b/arch/arm/boot/dts/dra72-evm-revc.dts
> index d626cd7..8472a8c 100644
> --- a/arch/arm/boot/dts/dra72-evm-revc.dts
> +++ b/arch/arm/boot/dts/dra72-evm-revc.dts
> @@ -59,16 +59,16 @@
> &davinci_mdio {
> dp83867_0: ethernet-phy@2 {
> reg = <2>;
> - ti,rx-internal-delay = <DP83867_RGMIIDCTL_2_00_NS>;
> - ti,tx-internal-delay = <DP83867_RGMIIDCTL_1_NS>;
> + ti,rx-internal-delay = <DP83867_RGMIIDCTL_2_25_NS>;
> + ti,tx-internal-delay = <DP83867_RGMIIDCTL_250_PS>;
> ti,fifo-depth = <DP83867_PHYCR_FIFO_DEPTH_8_B_NIB>;
> ti,min-output-imepdance;
> };
>
> dp83867_1: ethernet-phy@3 {
> reg = <3>;
> - ti,rx-internal-delay = <DP83867_RGMIIDCTL_2_00_NS>;
> - ti,tx-internal-delay = <DP83867_RGMIIDCTL_1_NS>;
> + ti,rx-internal-delay = <DP83867_RGMIIDCTL_2_25_NS>;
> + ti,tx-internal-delay = <DP83867_RGMIIDCTL_250_PS>;
> ti,fifo-depth = <DP83867_PHYCR_FIFO_DEPTH_8_B_NIB>;
> ti,min-output-imepdance;
> };
> --
> 2.10.0.372.g6fe1b14
>
On Tuesday 04 October 2016 06:40 PM, Andrew Lunn wrote:
>> + if (of_property_read_bool(of_node, "ti,max-output-imepdance"))
>> + dp83867->io_impedance = DP83867_IO_MUX_CFG_IO_IMPEDANCE_MAX;
>> + else if (of_property_read_bool(of_node, "ti,min-output-imepdance"))
>
> Did you really test this? Or did you make the same typos in your device
> tree file?
>
I have tested this and attached the log in cover letter. Since there is
a typo error on both dts and driver it worked as expected. Will send a
v3 ASAP.
Regards
Mugunthan V N
On Tuesday 04 October 2016 06:41 PM, Lokesh Vutla wrote:
>
> On Tuesday 04 October 2016 06:26 PM, Mugunthan V N wrote:
>> > The default impedance settings of the phy is not the optimal
>> > value, due to this the second ethernet is not working. Fix it
>> > with correct values which makes the second ethernet port to work.
>> >
>> > Signed-off-by: Mugunthan V N <[email protected]>
>> > ---
>> > arch/arm/boot/dts/dra72-evm-revc.dts | 2 ++
>> > 1 file changed, 2 insertions(+)
>> >
>> > diff --git a/arch/arm/boot/dts/dra72-evm-revc.dts b/arch/arm/boot/dts/dra72-evm-revc.dts
>> > index f9cfd3b..d626cd7 100644
>> > --- a/arch/arm/boot/dts/dra72-evm-revc.dts
>> > +++ b/arch/arm/boot/dts/dra72-evm-revc.dts
>> > @@ -62,6 +62,7 @@
>> > ti,rx-internal-delay = <DP83867_RGMIIDCTL_2_00_NS>;
>> > ti,tx-internal-delay = <DP83867_RGMIIDCTL_1_NS>;
>> > ti,fifo-depth = <DP83867_PHYCR_FIFO_DEPTH_8_B_NIB>;
>> > + ti,min-output-imepdance;
> s/imepdance/impedance
>
Thanks for quick catch. Will fix this in v3.
Regards
Mugunthan V N