[PATCH] net: phy: DP83TC811: Fix diabling interrupts
Fix a bug where INT_STAT1 was written twice and INT_STAT2 was ignored when disabling interrupts. Signed-off-by: Dan Murphy --- drivers/net/phy/dp83tc811.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/phy/dp83tc811.c b/drivers/net/phy/dp83tc811.c index 081d99aa3985..49ac678eb2dc 100644 --- a/drivers/net/phy/dp83tc811.c +++ b/drivers/net/phy/dp83tc811.c @@ -222,7 +222,7 @@ static int dp83811_config_intr(struct phy_device *phydev) if (err < 0) return err; - err = phy_write(phydev, MII_DP83811_INT_STAT1, 0); + err = phy_write(phydev, MII_DP83811_INT_STAT2, 0); } return err; -- 2.17.0.582.gccdcbd54c
Re: [PATCH 2/2] net: phy: DP83TC811: Fix SGMII enable/disable
Andrew On 06/28/2018 03:20 AM, Andrew Lunn wrote: > On Wed, Jun 27, 2018 at 01:16:18PM -0500, Dan Murphy wrote: >> If SGMII was selected in the DT then the device should >> write the SGMII enable bit. >> >> If SGMII is not selected in the DT then the SGMII bit >> should be disabled. >> >> Signed-off-by: Dan Murphy >> --- >> arch/arm/configs/omap2plus_defconfig | 1 + >> drivers/net/phy/dp83tc811.c | 20 +--- >> 2 files changed, 10 insertions(+), 11 deletions(-) >> >> diff --git a/arch/arm/configs/omap2plus_defconfig >> b/arch/arm/configs/omap2plus_defconfig >> index 06fb948ecfb3..30857d5b7a6c 100644 >> --- a/arch/arm/configs/omap2plus_defconfig >> +++ b/arch/arm/configs/omap2plus_defconfig >> @@ -182,6 +182,7 @@ CONFIG_TI_CPTS=y >> CONFIG_AT803X_PHY=y >> CONFIG_DP83848_PHY=y >> CONFIG_DP83867_PHY=y >> +CONFIG_DP83TC811_PHY=y >> CONFIG_MICREL_PHY=y >> CONFIG_SMSC_PHY=y >> CONFIG_PPP=m > > Hi Dan > > This change does not belong here. > Correct I will remove it. > Andrew > -- -- Dan Murphy
Re: [PATCH 1/2] net: phy: DP83TC811: Add INT_STAT3
Andrew On 06/28/2018 03:16 AM, Andrew Lunn wrote: >> err = phy_write(phydev, MII_DP83811_INT_STAT1, 0); >> if (err < 0) >> return err; >> >> -err = phy_write(phydev, MII_DP83811_INT_STAT1, 0); >> +err = phy_write(phydev, MII_DP83811_INT_STAT2, 0); >> +if (err < 0) >> +return err; >> + >> +err = phy_write(phydev, MII_DP83811_INT_STAT3, 0); >> } >> > > Hi Dan > > This seems like a bug fix, and so should be in a patch of its own, for > net, not net-next. > Yes I was debating whether to include this change in the patch or have it stand alone for stable back port. I will pull it out and cc stable > Andrew > -- -- Dan Murphy
[PATCH 1/2] net: phy: DP83TC811: Add INT_STAT3
Add INT_STAT3 interrupt setting and clearing.
Also fixed writing to INT_STAT2 when disabling
the interrupts as there was a double write to
INT_STAT1.
Signed-off-by: Dan Murphy
---
drivers/net/phy/dp83tc811.c | 28 +++-
1 file changed, 27 insertions(+), 1 deletion(-)
diff --git a/drivers/net/phy/dp83tc811.c b/drivers/net/phy/dp83tc811.c
index 081d99aa3985..f8653f5d8789 100644
--- a/drivers/net/phy/dp83tc811.c
+++ b/drivers/net/phy/dp83tc811.c
@@ -21,6 +21,7 @@
#define MII_DP83811_SGMII_CTRL 0x09
#define MII_DP83811_INT_STAT1 0x12
#define MII_DP83811_INT_STAT2 0x13
+#define MII_DP83811_INT_STAT3 0x18
#define MII_DP83811_RESET_CTRL 0x1f
#define DP83811_HW_RESET BIT(15)
@@ -44,6 +45,11 @@
#define DP83811_OVERVOLTAGE_INT_EN BIT(6)
#define DP83811_UNDERVOLTAGE_INT_ENBIT(7)
+/* INT_STAT3 bits */
+#define DP83811_LPS_INT_EN BIT(0)
+#define DP83811_NO_FRAME_INT_ENBIT(3)
+#define DP83811_POR_DONE_INT_ENBIT(4)
+
#define MII_DP83811_RXSOP1 0x04a5
#define MII_DP83811_RXSOP2 0x04a6
#define MII_DP83811_RXSOP3 0x04a7
@@ -81,6 +87,10 @@ static int dp83811_ack_interrupt(struct phy_device *phydev)
if (err < 0)
return err;
+ err = phy_read(phydev, MII_DP83811_INT_STAT3);
+ if (err < 0)
+ return err;
+
return 0;
}
@@ -216,13 +226,29 @@ static int dp83811_config_intr(struct phy_device *phydev)
DP83811_UNDERVOLTAGE_INT_EN);
err = phy_write(phydev, MII_DP83811_INT_STAT2, misr_status);
+ if (err < 0)
+ return err;
+
+ misr_status = phy_read(phydev, MII_DP83811_INT_STAT3);
+ if (misr_status < 0)
+ return misr_status;
+
+ misr_status |= (DP83811_LPS_INT_EN |
+ DP83811_NO_FRAME_INT_EN |
+ DP83811_POR_DONE_INT_EN);
+
+ err = phy_write(phydev, MII_DP83811_INT_STAT3, misr_status);
} else {
err = phy_write(phydev, MII_DP83811_INT_STAT1, 0);
if (err < 0)
return err;
- err = phy_write(phydev, MII_DP83811_INT_STAT1, 0);
+ err = phy_write(phydev, MII_DP83811_INT_STAT2, 0);
+ if (err < 0)
+ return err;
+
+ err = phy_write(phydev, MII_DP83811_INT_STAT3, 0);
}
return err;
--
2.17.0.582.gccdcbd54c
[PATCH 2/2] net: phy: DP83TC811: Fix SGMII enable/disable
If SGMII was selected in the DT then the device should
write the SGMII enable bit.
If SGMII is not selected in the DT then the SGMII bit
should be disabled.
Signed-off-by: Dan Murphy
---
arch/arm/configs/omap2plus_defconfig | 1 +
drivers/net/phy/dp83tc811.c | 20 +---
2 files changed, 10 insertions(+), 11 deletions(-)
diff --git a/arch/arm/configs/omap2plus_defconfig
b/arch/arm/configs/omap2plus_defconfig
index 06fb948ecfb3..30857d5b7a6c 100644
--- a/arch/arm/configs/omap2plus_defconfig
+++ b/arch/arm/configs/omap2plus_defconfig
@@ -182,6 +182,7 @@ CONFIG_TI_CPTS=y
CONFIG_AT803X_PHY=y
CONFIG_DP83848_PHY=y
CONFIG_DP83867_PHY=y
+CONFIG_DP83TC811_PHY=y
CONFIG_MICREL_PHY=y
CONFIG_SMSC_PHY=y
CONFIG_PPP=m
diff --git a/drivers/net/phy/dp83tc811.c b/drivers/net/phy/dp83tc811.c
index f8653f5d8789..78cad134a79e 100644
--- a/drivers/net/phy/dp83tc811.c
+++ b/drivers/net/phy/dp83tc811.c
@@ -284,21 +284,19 @@ static int dp83811_config_init(struct phy_device *phydev)
if (err < 0)
return err;
+ value = phy_read(phydev, MII_DP83811_SGMII_CTRL);
if (phydev->interface == PHY_INTERFACE_MODE_SGMII) {
- value = phy_read(phydev, MII_DP83811_SGMII_CTRL);
- if (!(value & DP83811_SGMII_EN)) {
- err = phy_write(phydev, MII_DP83811_SGMII_CTRL,
+ err = phy_write(phydev, MII_DP83811_SGMII_CTRL,
(DP83811_SGMII_EN | value));
- if (err < 0)
- return err;
- } else {
- err = phy_write(phydev, MII_DP83811_SGMII_CTRL,
- (~DP83811_SGMII_EN & value));
- if (err < 0)
- return err;
- }
+ } else {
+ err = phy_write(phydev, MII_DP83811_SGMII_CTRL,
+ (~DP83811_SGMII_EN & value));
}
+ if (err < 0)
+
+ return err;
+
value = DP83811_WOL_MAGIC_EN | DP83811_WOL_SECURE_ON | DP83811_WOL_EN;
return phy_write_mmd(phydev, DP83811_DEVADDR, MII_DP83811_WOL_CFG,
--
2.17.0.582.gccdcbd54c
Re: [PATCH v3] net: phy: DP83TC811: Introduce support for the DP83TC811 phy
Andrew On 05/11/2018 01:30 PM, Andrew Lunn wrote: > On Fri, May 11, 2018 at 01:08:19PM -0500, Dan Murphy wrote: >> Add support for the DP83811 phy. >> >> The DP83811 supports both rgmii and sgmii interfaces. >> There are 2 part numbers for this the DP83TC811R does not >> reliably support the SGMII interface but the DP83TC811S will. >> >> There is not a way to differentiate these parts from the >> hardware or register set. So this is controlled via the DT >> to indicate which phy mode is required. Or the part can be >> strapped to a certain interface. >> >> Data sheet can be found here: >> http://www.ti.com/product/DP83TC811S-Q1/description >> http://www.ti.com/product/DP83TC811R-Q1/description >> >> Signed-off-by: Dan Murphy <dmur...@ti.com> > > Hi Dan > > It is normal to add any Reviewed-by, or Tested-by: tags you received, > so long as you don't make major changes. > Thanks for the reminder. I usually add them if I get them explicitly stated in the review. I have not seen any Reviewed-by or Tested-by tags in any of the replies for the patch. But I may have missed it. Dan > Andrew > -- -- Dan Murphy
[PATCH v3] net: phy: DP83TC811: Introduce support for the DP83TC811 phy
Add support for the DP83811 phy.
The DP83811 supports both rgmii and sgmii interfaces.
There are 2 part numbers for this the DP83TC811R does not
reliably support the SGMII interface but the DP83TC811S will.
There is not a way to differentiate these parts from the
hardware or register set. So this is controlled via the DT
to indicate which phy mode is required. Or the part can be
strapped to a certain interface.
Data sheet can be found here:
http://www.ti.com/product/DP83TC811S-Q1/description
http://www.ti.com/product/DP83TC811R-Q1/description
Signed-off-by: Dan Murphy <dmur...@ti.com>
---
v3 - Variable length alignment - https://patchwork.kernel.org/patch/10389657/
v2 - Remove extra config_init in reset, update config_init call back function
fix a checkpatch alignment issue, add SGMII check in autoneg api -
https://patchwork.kernel.org/patch/10389323/
drivers/net/phy/Kconfig | 5 +
drivers/net/phy/Makefile| 1 +
drivers/net/phy/dp83tc811.c | 347
3 files changed, 353 insertions(+)
create mode 100644 drivers/net/phy/dp83tc811.c
diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index bdfbabb86ee0..810140a9e114 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -285,6 +285,11 @@ config DP83822_PHY
---help---
Supports the DP83822 PHY.
+config DP83TC811_PHY
+ tristate "Texas Instruments DP83TC822 PHY"
+ ---help---
+ Supports the DP83TC822 PHY.
+
config DP83848_PHY
tristate "Texas Instruments DP83848 PHY"
---help---
diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
index 01acbcb2c798..00445b61a9a8 100644
--- a/drivers/net/phy/Makefile
+++ b/drivers/net/phy/Makefile
@@ -57,6 +57,7 @@ obj-$(CONFIG_CORTINA_PHY) += cortina.o
obj-$(CONFIG_DAVICOM_PHY) += davicom.o
obj-$(CONFIG_DP83640_PHY) += dp83640.o
obj-$(CONFIG_DP83822_PHY) += dp83822.o
+obj-$(CONFIG_DP83TC811_PHY)+= dp83tc811.o
obj-$(CONFIG_DP83848_PHY) += dp83848.o
obj-$(CONFIG_DP83867_PHY) += dp83867.o
obj-$(CONFIG_FIXED_PHY)+= fixed_phy.o
diff --git a/drivers/net/phy/dp83tc811.c b/drivers/net/phy/dp83tc811.c
new file mode 100644
index ..081d99aa3985
--- /dev/null
+++ b/drivers/net/phy/dp83tc811.c
@@ -0,0 +1,347 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Driver for the Texas Instruments DP83TC811 PHY
+ *
+ * Copyright (C) 2018 Texas Instruments Incorporated - http://www.ti.com/
+ *
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define DP83TC811_PHY_ID 0x2000a253
+#define DP83811_DEVADDR0x1f
+
+#define MII_DP83811_SGMII_CTRL 0x09
+#define MII_DP83811_INT_STAT1 0x12
+#define MII_DP83811_INT_STAT2 0x13
+#define MII_DP83811_RESET_CTRL 0x1f
+
+#define DP83811_HW_RESET BIT(15)
+#define DP83811_SW_RESET BIT(14)
+
+/* INT_STAT1 bits */
+#define DP83811_RX_ERR_HF_INT_EN BIT(0)
+#define DP83811_MS_TRAINING_INT_EN BIT(1)
+#define DP83811_ANEG_COMPLETE_INT_EN BIT(2)
+#define DP83811_ESD_EVENT_INT_EN BIT(3)
+#define DP83811_WOL_INT_EN BIT(4)
+#define DP83811_LINK_STAT_INT_EN BIT(5)
+#define DP83811_ENERGY_DET_INT_EN BIT(6)
+#define DP83811_LINK_QUAL_INT_EN BIT(7)
+
+/* INT_STAT2 bits */
+#define DP83811_JABBER_DET_INT_EN BIT(0)
+#define DP83811_POLARITY_INT_ENBIT(1)
+#define DP83811_SLEEP_MODE_INT_EN BIT(2)
+#define DP83811_OVERTEMP_INT_ENBIT(3)
+#define DP83811_OVERVOLTAGE_INT_EN BIT(6)
+#define DP83811_UNDERVOLTAGE_INT_ENBIT(7)
+
+#define MII_DP83811_RXSOP1 0x04a5
+#define MII_DP83811_RXSOP2 0x04a6
+#define MII_DP83811_RXSOP3 0x04a7
+
+/* WoL Registers */
+#define MII_DP83811_WOL_CFG0x04a0
+#define MII_DP83811_WOL_STAT 0x04a1
+#define MII_DP83811_WOL_DA10x04a2
+#define MII_DP83811_WOL_DA20x04a3
+#define MII_DP83811_WOL_DA30x04a4
+
+/* WoL bits */
+#define DP83811_WOL_MAGIC_EN BIT(0)
+#define DP83811_WOL_SECURE_ON BIT(5)
+#define DP83811_WOL_EN BIT(7)
+#define DP83811_WOL_INDICATION_SEL BIT(8)
+#define DP83811_WOL_CLR_INDICATION BIT(11)
+
+/* SGMII CTRL bits */
+#define DP83811_TDR_AUTO BIT(8)
+#define DP83811_SGMII_EN BIT(12)
+#define DP83811_SGMII_AUTO_NEG_EN BIT(13)
+#define DP83811_SGMII_TX_ERR_DIS BIT(14)
+#define DP83811_SGMII_SOFT_RESET BIT(15)
+
+static int dp83811_ack_interrupt(struct phy_device *phydev)
+{
+ int err;
+
+ err = phy_read(phydev, MII_DP83811_INT_STAT1);
+ if (err < 0)
+ return err;
+
+ err = phy_read(phydev, MII_DP83811_INT_STAT2);
+ if (err < 0)
+ return err;
+
+ return 0;
+}
+
+static int dp83811_set_wol(struct phy_device *phydev,
+ struct ethtool_wolinfo *wol)
+{
+ struct net_device *ndev = phydev->attached_dev;
+
[PATCH v2] net: phy: DP83TC811: Introduce support for the DP83TC811 phy
Add support for the DP83811 phy.
The DP83811 supports both rgmii and sgmii interfaces.
There are 2 part numbers for this the DP83TC811R does not
reliably support the SGMII interface but the DP83TC811S will.
There is not a way to differentiate these parts from the
hardware or register set. So this is controlled via the DT
to indicate which phy mode is required. Or the part can be
strapped to a certain interface.
Data sheet can be found here:
http://www.ti.com/product/DP83TC811S-Q1/description
http://www.ti.com/product/DP83TC811R-Q1/description
Signed-off-by: Dan Murphy <dmur...@ti.com>
---
v2 - Remove extra config_init in reset, update config_init call back function
fix a checkpatch alignment issue, add SGMII check in autoneg api -
https://patchwork.kernel.org/patch/10389323/
drivers/net/phy/Kconfig | 5 +
drivers/net/phy/Makefile| 1 +
drivers/net/phy/dp83tc811.c | 350
3 files changed, 356 insertions(+)
create mode 100644 drivers/net/phy/dp83tc811.c
diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index bdfbabb86ee0..810140a9e114 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -285,6 +285,11 @@ config DP83822_PHY
---help---
Supports the DP83822 PHY.
+config DP83TC811_PHY
+ tristate "Texas Instruments DP83TC822 PHY"
+ ---help---
+ Supports the DP83TC822 PHY.
+
config DP83848_PHY
tristate "Texas Instruments DP83848 PHY"
---help---
diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
index 01acbcb2c798..00445b61a9a8 100644
--- a/drivers/net/phy/Makefile
+++ b/drivers/net/phy/Makefile
@@ -57,6 +57,7 @@ obj-$(CONFIG_CORTINA_PHY) += cortina.o
obj-$(CONFIG_DAVICOM_PHY) += davicom.o
obj-$(CONFIG_DP83640_PHY) += dp83640.o
obj-$(CONFIG_DP83822_PHY) += dp83822.o
+obj-$(CONFIG_DP83TC811_PHY)+= dp83tc811.o
obj-$(CONFIG_DP83848_PHY) += dp83848.o
obj-$(CONFIG_DP83867_PHY) += dp83867.o
obj-$(CONFIG_FIXED_PHY)+= fixed_phy.o
diff --git a/drivers/net/phy/dp83tc811.c b/drivers/net/phy/dp83tc811.c
new file mode 100644
index ..10c20426bcfa
--- /dev/null
+++ b/drivers/net/phy/dp83tc811.c
@@ -0,0 +1,350 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Driver for the Texas Instruments DP83TC811 PHY
+ *
+ * Copyright (C) 2018 Texas Instruments Incorporated - http://www.ti.com/
+ *
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define DP83TC811_PHY_ID 0x2000a253
+#define DP83811_DEVADDR0x1f
+
+#define MII_DP83811_SGMII_CTRL 0x09
+#define MII_DP83811_INT_STAT1 0x12
+#define MII_DP83811_INT_STAT2 0x13
+#define MII_DP83811_RESET_CTRL 0x1f
+
+#define DP83811_HW_RESET BIT(15)
+#define DP83811_SW_RESET BIT(14)
+
+/* INT_STAT1 bits */
+#define DP83811_RX_ERR_HF_INT_EN BIT(0)
+#define DP83811_MS_TRAINING_INT_EN BIT(1)
+#define DP83811_ANEG_COMPLETE_INT_EN BIT(2)
+#define DP83811_ESD_EVENT_INT_EN BIT(3)
+#define DP83811_WOL_INT_EN BIT(4)
+#define DP83811_LINK_STAT_INT_EN BIT(5)
+#define DP83811_ENERGY_DET_INT_EN BIT(6)
+#define DP83811_LINK_QUAL_INT_EN BIT(7)
+
+/* INT_STAT2 bits */
+#define DP83811_JABBER_DET_INT_EN BIT(0)
+#define DP83811_POLARITY_INT_ENBIT(1)
+#define DP83811_SLEEP_MODE_INT_EN BIT(2)
+#define DP83811_OVERTEMP_INT_ENBIT(3)
+#define DP83811_OVERVOLTAGE_INT_EN BIT(6)
+#define DP83811_UNDERVOLTAGE_INT_ENBIT(7)
+
+#define MII_DP83811_RXSOP1 0x04a5
+#define MII_DP83811_RXSOP2 0x04a6
+#define MII_DP83811_RXSOP3 0x04a7
+
+/* WoL Registers */
+#define MII_DP83811_WOL_CFG0x04a0
+#define MII_DP83811_WOL_STAT 0x04a1
+#define MII_DP83811_WOL_DA10x04a2
+#define MII_DP83811_WOL_DA20x04a3
+#define MII_DP83811_WOL_DA30x04a4
+
+/* WoL bits */
+#define DP83811_WOL_MAGIC_EN BIT(0)
+#define DP83811_WOL_SECURE_ON BIT(5)
+#define DP83811_WOL_EN BIT(7)
+#define DP83811_WOL_INDICATION_SEL BIT(8)
+#define DP83811_WOL_CLR_INDICATION BIT(11)
+
+/* SGMII CTRL bits */
+#define DP83811_TDR_AUTO BIT(8)
+#define DP83811_SGMII_EN BIT(12)
+#define DP83811_SGMII_AUTO_NEG_EN BIT(13)
+#define DP83811_SGMII_TX_ERR_DIS BIT(14)
+#define DP83811_SGMII_SOFT_RESET BIT(15)
+
+static int dp83811_ack_interrupt(struct phy_device *phydev)
+{
+ int err;
+
+ err = phy_read(phydev, MII_DP83811_INT_STAT1);
+ if (err < 0)
+ return err;
+
+ err = phy_read(phydev, MII_DP83811_INT_STAT2);
+ if (err < 0)
+ return err;
+
+ return 0;
+}
+
+static int dp83811_set_wol(struct phy_device *phydev,
+ struct ethtool_wolinfo *wol)
+{
+ struct net_device *ndev = phydev->attached_dev;
+ u16 value;
+ const u8 *mac;
+
+ if (wol->wo
Re: [PATCH] net: phy: DP83TC811: Introduce support for the DP83TC811 phy
Andrew
On 05/09/2018 08:58 AM, Andrew Lunn wrote:
> On Wed, May 09, 2018 at 08:50:58AM -0500, Dan Murphy wrote:
>> Andrew
>>
>> Thanks for the review
>>
>> On 05/09/2018 08:43 AM, Andrew Lunn wrote:
>>>> +static int dp83811_config_aneg(struct phy_device *phydev)
>>>> +{
>>>> + int err;
>>>> + int value;
>>>> +
>>>> + value = phy_read(phydev, MII_DP83811_SGMII_CTRL);
>>>> + if (phydev->autoneg == AUTONEG_ENABLE) {
>>>> + err = phy_write(phydev, MII_DP83811_SGMII_CTRL,
>>>> + (DP83811_SGMII_AUTO_NEG_EN | value));
>>>> + if (err < 0)
>>>> + return err;
>>>> + } else {
>>>> + err = phy_write(phydev, MII_DP83811_SGMII_CTRL,
>>>> + (~DP83811_SGMII_AUTO_NEG_EN & value));
>>>> + if (err < 0)
>>>> + return err;
>>>> + }
>>>> +
>>>
>>> Hi Dan
>>>
>>> You say SGMII is unreliable on one of these devices. Should you check
>>> phydev->interface before enabling SGMII autoneg?
>>
>>
>> If SGMII enable bit(12) is not set in the device then setting auto neg has
>> no affect on the device.
>
> Ah, O.K. Maybe add a comment about this.
>
I will add the check it will be clearer if written in code and explicit as
opposed to explaining it.
Dan
>>>> +
>>>> +static int dp83811_config_init(struct phy_device *phydev)
>>>> +{
>>>> + int err;
>>>> + int value;
>>>> +
>>>> + err = genphy_config_init(phydev);
>>>> + if (err < 0)
>>>> + return err;
>>>> +
>>>> + if (phydev->interface == PHY_INTERFACE_MODE_SGMII) {
>>>> + value = phy_read(phydev, MII_DP83811_SGMII_CTRL);
>>>> + if (!(value & DP83811_SGMII_EN)) {
>>>> + err = phy_write(phydev, MII_DP83811_SGMII_CTRL,
>>>> + (DP83811_SGMII_EN | value));
>>>> + if (err < 0)
>>>> + return err;
>>>> + } else {
>>>> + err = phy_write(phydev, MII_DP83811_SGMII_CTRL,
>>>> + (~DP83811_SGMII_EN & value));
>>>> + if (err < 0)
>>>> + return err;
>>>> + }
>>>
>>> This looks to be a duplicate of dp83811_config_aneg()?
>>
>> It is almost the same but this function sets bit 12 and aneg function sets
>> bit 13.
>> We can have SGMII with or without auto neg.
>
> Yep, i missed the difference.
>
> Andrew
>
--
--
Dan Murphy
Re: [PATCH] net: phy: DP83TC811: Introduce support for the DP83TC811 phy
Andrew
Thanks for the review
On 05/09/2018 08:43 AM, Andrew Lunn wrote:
>> +static int dp83811_config_aneg(struct phy_device *phydev)
>> +{
>> +int err;
>> +int value;
>> +
>> +value = phy_read(phydev, MII_DP83811_SGMII_CTRL);
>> +if (phydev->autoneg == AUTONEG_ENABLE) {
>> +err = phy_write(phydev, MII_DP83811_SGMII_CTRL,
>> +(DP83811_SGMII_AUTO_NEG_EN | value));
>> +if (err < 0)
>> +return err;
>> +} else {
>> +err = phy_write(phydev, MII_DP83811_SGMII_CTRL,
>> +(~DP83811_SGMII_AUTO_NEG_EN & value));
>> +if (err < 0)
>> +return err;
>> +}
>> +
>
> Hi Dan
>
> You say SGMII is unreliable on one of these devices. Should you check
> phydev->interface before enabling SGMII autoneg?
If SGMII enable bit(12) is not set in the device then setting auto neg has no
affect on the device.
Only the SGMII has auto negotiation capability so if we set this with other MII
interfaces then it is ignored by the device.
If we want to protect this with the SGMII check I can add it but it may be over
kill.
Let me know what is preferred.
>
>> +return genphy_config_aneg(phydev);
>> +}
>> +
>> +static int dp83811_config_init(struct phy_device *phydev)
>> +{
>> +int err;
>> +int value;
>> +
>> +err = genphy_config_init(phydev);
>> +if (err < 0)
>> +return err;
>> +
>> +if (phydev->interface == PHY_INTERFACE_MODE_SGMII) {
>> +value = phy_read(phydev, MII_DP83811_SGMII_CTRL);
>> +if (!(value & DP83811_SGMII_EN)) {
>> +err = phy_write(phydev, MII_DP83811_SGMII_CTRL,
>> +(DP83811_SGMII_EN | value));
>> +if (err < 0)
>> +return err;
>> +} else {
>> +err = phy_write(phydev, MII_DP83811_SGMII_CTRL,
>> +(~DP83811_SGMII_EN & value));
>> +if (err < 0)
>> +return err;
>> +}
>
> This looks to be a duplicate of dp83811_config_aneg()?
It is almost the same but this function sets bit 12 and aneg function sets bit
13.
We can have SGMII with or without auto neg.
>
>> +}
>> +
>> +value = DP83811_WOL_MAGIC_EN | DP83811_WOL_SECURE_ON | DP83811_WOL_EN;
>> +
>> +return phy_write_mmd(phydev, DP83811_DEVADDR, MII_DP83811_WOL_CFG,
>> + value);
>> +}
>> +
>> +static int dp83811_phy_reset(struct phy_device *phydev)
>> +{
>> +int err;
>> +
>> +err = phy_write(phydev, MII_DP83811_RESET_CTRL, DP83811_HW_RESET);
>> +if (err < 0)
>> +return err;
>> +
>> +dp83811_config_init(phydev);
>
> I don't think you need to initialize it here. phylib should call that
> soon after the reset.
OK I will remove it.
>
>> +
>> +return 0;
>> +}
>> +
>
>> +static struct phy_driver dp83811_driver[] = {
>> +{
>> +.phy_id = DP83TC811_PHY_ID,
>> +.phy_id_mask = 0xfff0,
>> +.name = "TI DP83TC811",
>> +.features = PHY_BASIC_FEATURES,
>> +.flags = PHY_HAS_INTERRUPT,
>> +.config_init = genphy_config_init,
>
>
>
I missed that I must have had the config init being called in my testing
through the reset function.
I will update.
> Andrew
>
--
--
Dan Murphy
[PATCH] net: phy: DP83TC811: Introduce support for the DP83TC811 phy
Add support for the DP83811 phy.
The DP83811 supports both rgmii and sgmii interfaces.
There are 2 part numbers for this the DP83TC811R does not
reliably support the SGMII interface but the DP83TC811S will.
There is not a way to differentiate these parts from the
hardware or register set. So this is controlled via the DT
to indicate which phy mode is required. Or the part can be
strapped to a certain interface.
Data sheet can be found here:
http://www.ti.com/product/DP83TC811S-Q1/description
http://www.ti.com/product/DP83TC811R-Q1/description
Signed-off-by: Dan Murphy <dmur...@ti.com>
---
drivers/net/phy/Kconfig | 5 +
drivers/net/phy/Makefile| 1 +
drivers/net/phy/dp83tc811.c | 350
3 files changed, 356 insertions(+)
create mode 100644 drivers/net/phy/dp83tc811.c
diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index bdfbabb86ee0..810140a9e114 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -285,6 +285,11 @@ config DP83822_PHY
---help---
Supports the DP83822 PHY.
+config DP83TC811_PHY
+ tristate "Texas Instruments DP83TC822 PHY"
+ ---help---
+ Supports the DP83TC822 PHY.
+
config DP83848_PHY
tristate "Texas Instruments DP83848 PHY"
---help---
diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
index 01acbcb2c798..00445b61a9a8 100644
--- a/drivers/net/phy/Makefile
+++ b/drivers/net/phy/Makefile
@@ -57,6 +57,7 @@ obj-$(CONFIG_CORTINA_PHY) += cortina.o
obj-$(CONFIG_DAVICOM_PHY) += davicom.o
obj-$(CONFIG_DP83640_PHY) += dp83640.o
obj-$(CONFIG_DP83822_PHY) += dp83822.o
+obj-$(CONFIG_DP83TC811_PHY)+= dp83tc811.o
obj-$(CONFIG_DP83848_PHY) += dp83848.o
obj-$(CONFIG_DP83867_PHY) += dp83867.o
obj-$(CONFIG_FIXED_PHY)+= fixed_phy.o
diff --git a/drivers/net/phy/dp83tc811.c b/drivers/net/phy/dp83tc811.c
new file mode 100644
index ..01cb0e246449
--- /dev/null
+++ b/drivers/net/phy/dp83tc811.c
@@ -0,0 +1,350 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Driver for the Texas Instruments DP83TC811 PHY
+ *
+ * Copyright (C) 2018 Texas Instruments Incorporated - http://www.ti.com/
+ *
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define DP83TC811_PHY_ID 0x2000a253
+#define DP83811_DEVADDR0x1f
+
+#define MII_DP83811_SGMII_CTRL 0x09
+#define MII_DP83811_INT_STAT1 0x12
+#define MII_DP83811_INT_STAT2 0x13
+#define MII_DP83811_RESET_CTRL 0x1f
+
+#define DP83811_HW_RESET BIT(15)
+#define DP83811_SW_RESET BIT(14)
+
+/* INT_STAT1 bits */
+#define DP83811_RX_ERR_HF_INT_EN BIT(0)
+#define DP83811_MS_TRAINING_INT_EN BIT(1)
+#define DP83811_ANEG_COMPLETE_INT_EN BIT(2)
+#define DP83811_ESD_EVENT_INT_EN BIT(3)
+#define DP83811_WOL_INT_EN BIT(4)
+#define DP83811_LINK_STAT_INT_EN BIT(5)
+#define DP83811_ENERGY_DET_INT_EN BIT(6)
+#define DP83811_LINK_QUAL_INT_EN BIT(7)
+
+/* INT_STAT2 bits */
+#define DP83811_JABBER_DET_INT_EN BIT(0)
+#define DP83811_POLARITY_INT_ENBIT(1)
+#define DP83811_SLEEP_MODE_INT_EN BIT(2)
+#define DP83811_OVERTEMP_INT_ENBIT(3)
+#define DP83811_OVERVOLTAGE_INT_EN BIT(6)
+#define DP83811_UNDERVOLTAGE_INT_ENBIT(7)
+
+#define MII_DP83811_RXSOP1 0x04a5
+#define MII_DP83811_RXSOP2 0x04a6
+#define MII_DP83811_RXSOP3 0x04a7
+
+/* WoL Registers */
+#define MII_DP83811_WOL_CFG0x04a0
+#define MII_DP83811_WOL_STAT 0x04a1
+#define MII_DP83811_WOL_DA10x04a2
+#define MII_DP83811_WOL_DA20x04a3
+#define MII_DP83811_WOL_DA30x04a4
+
+/* WoL bits */
+#define DP83811_WOL_MAGIC_EN BIT(0)
+#define DP83811_WOL_SECURE_ON BIT(5)
+#define DP83811_WOL_EN BIT(7)
+#define DP83811_WOL_INDICATION_SEL BIT(8)
+#define DP83811_WOL_CLR_INDICATION BIT(11)
+
+/* SGMII CTRL bits */
+#define DP83811_TDR_AUTO BIT(8)
+#define DP83811_SGMII_EN BIT(12)
+#define DP83811_SGMII_AUTO_NEG_EN BIT(13)
+#define DP83811_SGMII_TX_ERR_DIS BIT(14)
+#define DP83811_SGMII_SOFT_RESET BIT(15)
+
+static int dp83811_ack_interrupt(struct phy_device *phydev)
+{
+ int err;
+
+ err = phy_read(phydev, MII_DP83811_INT_STAT1);
+ if (err < 0)
+ return err;
+
+ err = phy_read(phydev, MII_DP83811_INT_STAT2);
+ if (err < 0)
+ return err;
+
+ return 0;
+}
+
+static int dp83811_set_wol(struct phy_device *phydev,
+ struct ethtool_wolinfo *wol)
+{
+ struct net_device *ndev = phydev->attached_dev;
+ u16 value;
+ const u8 *mac;
+
+ if (wol->wolopts & (WAKE_MAGIC | WAKE_MAGICSECURE)) {
+ mac = (const u8 *)ndev->dev_addr;
+
+ if (!is_valid_ether_addr(mac))
+ return -EINVAL;
+
+ /
Re: [PATCH] net: phy: DP83811: Add support for the phy
Florian On 05/08/2018 12:14 PM, Florian Fainelli wrote: > On 05/08/2018 10:13 AM, Dan Murphy wrote: >> Andrew >> >> On 05/08/2018 11:49 AM, Andrew Lunn wrote: >>> On Tue, May 08, 2018 at 10:56:55AM -0500, Dan Murphy wrote: >>>> All >>>> >>>> On 05/08/2018 09:11 AM, Dan Murphy wrote: >>>>> Add support for the DP83811 phy by extending >>>>> the DP83822 driver to recognize the PHY IDs. >>>>> >>>>> The DP83811 supports both rgmii and sgmii interfaces. >>>>> There are 2 part numbers for this the DP83811R does not >>>>> reliably support the SGMII interface but the DP83811S will. >>>>> >>>>> There is not a way to differentiate these parts from the >>>>> hardware or register set. So this is controlled via the DT >>>>> to indicate which phy mode is required. Or the part can be >>>>> strapped to a certain interface. >>>>> >>>>> Data sheet can be found here: >>>>> http://www.ti.com/product/DP83TC811S-Q1/description >>>>> http://www.ti.com/product/DP83TC811R-Q1/description >>>>> >>>> >>>> I am withdrawing this patch for comment. >>>> Some of the future features have varying register definitions between the >>>> DP83811 >>>> and DP83822 >>> >>> Hi Dan >>> >>> It might be worth talking to the ASIC engineers and the >>> test/qualification engineers. There are sometime undocumented >>> registers for testing. You might be able to identify the exact device >>> from these registers. >>> >> >> Thanks. I talked to them prior to submitting this patch about determining >> which part is on the board. >> I will ping them again and poke a little harder. >> >> It turns out that we will probably need a new driver for this part anyway as >> there are >> additional features that need to be supported that the 811 just does not >> support. >> >> They want to support Master/Slave configurations. >> The 822 supports fiber and eee while the 811 does not. >> The 811 supports the IEEE802.3bw specific fields in MMD1 and MMD3, which are >> not in the 822. >> The 811 does not support auto-negotiation on the MDI so all the auto-neg >> registers are invalid for the 811 but are valid for the 822. >> >> Our Customer engineers felt that combining these two devices into a single >> driver may confuse the customer. > > Why? Having a single Kconfig option to enable and "automatically" > gaining support for new hardware is nice. > I do agree that it is nice to have. But with additional features or lack of features the driver source will get pretty messy differentiating between devices. What I added was just the basic if 811 & SGMII check. Dan -- -- Dan Murphy
Re: [PATCH] net: phy: DP83811: Add support for the phy
Andrew On 05/08/2018 11:49 AM, Andrew Lunn wrote: > On Tue, May 08, 2018 at 10:56:55AM -0500, Dan Murphy wrote: >> All >> >> On 05/08/2018 09:11 AM, Dan Murphy wrote: >>> Add support for the DP83811 phy by extending >>> the DP83822 driver to recognize the PHY IDs. >>> >>> The DP83811 supports both rgmii and sgmii interfaces. >>> There are 2 part numbers for this the DP83811R does not >>> reliably support the SGMII interface but the DP83811S will. >>> >>> There is not a way to differentiate these parts from the >>> hardware or register set. So this is controlled via the DT >>> to indicate which phy mode is required. Or the part can be >>> strapped to a certain interface. >>> >>> Data sheet can be found here: >>> http://www.ti.com/product/DP83TC811S-Q1/description >>> http://www.ti.com/product/DP83TC811R-Q1/description >>> >> >> I am withdrawing this patch for comment. >> Some of the future features have varying register definitions between the >> DP83811 >> and DP83822 > > Hi Dan > > It might be worth talking to the ASIC engineers and the > test/qualification engineers. There are sometime undocumented > registers for testing. You might be able to identify the exact device > from these registers. > Thanks. I talked to them prior to submitting this patch about determining which part is on the board. I will ping them again and poke a little harder. It turns out that we will probably need a new driver for this part anyway as there are additional features that need to be supported that the 811 just does not support. They want to support Master/Slave configurations. The 822 supports fiber and eee while the 811 does not. The 811 supports the IEEE802.3bw specific fields in MMD1 and MMD3, which are not in the 822. The 811 does not support auto-negotiation on the MDI so all the auto-neg registers are invalid for the 811 but are valid for the 822. Our Customer engineers felt that combining these two devices into a single driver may confuse the customer. Dan > Andrew > -- -- Dan Murphy
Re: [PATCH] net: phy: DP83811: Add support for the phy
All
On 05/08/2018 09:11 AM, Dan Murphy wrote:
> Add support for the DP83811 phy by extending
> the DP83822 driver to recognize the PHY IDs.
>
> The DP83811 supports both rgmii and sgmii interfaces.
> There are 2 part numbers for this the DP83811R does not
> reliably support the SGMII interface but the DP83811S will.
>
> There is not a way to differentiate these parts from the
> hardware or register set. So this is controlled via the DT
> to indicate which phy mode is required. Or the part can be
> strapped to a certain interface.
>
> Data sheet can be found here:
> http://www.ti.com/product/DP83TC811S-Q1/description
> http://www.ti.com/product/DP83TC811R-Q1/description
>
I am withdrawing this patch for comment.
Some of the future features have varying register definitions between the
DP83811
and DP83822
> Signed-off-by: Dan Murphy <dmur...@ti.com>
> ---
> drivers/net/phy/dp83822.c | 42 ---
> 1 file changed, 39 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/phy/dp83822.c b/drivers/net/phy/dp83822.c
> index 6e8a2a4f3a6e..5c379ff25dac 100644
> --- a/drivers/net/phy/dp83822.c
> +++ b/drivers/net/phy/dp83822.c
> @@ -23,8 +23,10 @@
> #include
>
> #define DP83822_PHY_ID 0x2000a240
> +#define DP83811_PHY_ID 0x2000a253
> #define DP83822_DEVADDR 0x1f
>
> +#define MII_DP83811_SGMII_CTRL 0x09
> #define MII_DP83822_PHYSCR 0x11
> #define MII_DP83822_MISR10x12
> #define MII_DP83822_MISR20x13
> @@ -79,6 +81,13 @@
> #define DP83822_WOL_INDICATION_SEL BIT(8)
> #define DP83822_WOL_CLR_INDICATION BIT(11)
>
> +/* DP83811 SGMII CTRL bits */
> +#define DP83811_TDR_AUTO BIT(8)
> +#define DP83811_SGMII_EN BIT(12)
> +#define DP83811_SGMII_AUTO_NEG_ENBIT(13)
> +#define DP83811_SGMII_TX_ERR_DIS BIT(14)
> +#define DP83811_SGMII_SOFT_RESET BIT(15)
> +
> static int dp83822_ack_interrupt(struct phy_device *phydev)
> {
> int err;
> @@ -267,6 +276,17 @@ static int dp83822_config_init(struct phy_device *phydev)
> if (err < 0)
> return err;
>
> + if ((phydev->interface == PHY_INTERFACE_MODE_SGMII &&
> + phydev->phy_id == DP83811_PHY_ID)) {
> + value = phy_read(phydev, MII_DP83811_SGMII_CTRL);
> + if (!(value & DP83811_SGMII_EN)) {
> + err = phy_write(phydev, MII_DP83811_SGMII_CTRL,
> + (DP83811_SGMII_EN | value));
> + if (err < 0)
> + return err;
> + }
> + }
> +
> value = DP83822_WOL_MAGIC_EN | DP83822_WOL_SECURE_ON | DP83822_WOL_EN;
>
> return phy_write_mmd(phydev, DP83822_DEVADDR, MII_DP83822_WOL_CFG,
> @@ -328,15 +348,31 @@ static struct phy_driver dp83822_driver[] = {
> .suspend = dp83822_suspend,
> .resume = dp83822_resume,
>},
> + {
> + .phy_id = DP83811_PHY_ID,
> + .phy_id_mask = 0xfff0,
> + .name = "TI DP83811",
> + .features = PHY_BASIC_FEATURES,
> + .flags = PHY_HAS_INTERRUPT,
> + .config_init = genphy_config_init,
> + .soft_reset = dp83822_phy_reset,
> + .get_wol = dp83822_get_wol,
> + .set_wol = dp83822_set_wol,
> + .ack_interrupt = dp83822_ack_interrupt,
> + .config_intr = dp83822_config_intr,
> + .suspend = dp83822_suspend,
> + .resume = dp83822_resume,
> + },
> };
> module_phy_driver(dp83822_driver);
>
> static struct mdio_device_id __maybe_unused dp83822_tbl[] = {
> - { DP83822_PHY_ID, 0xfff0 },
> - { },
> + {DP83822_PHY_ID, 0xfff0},
> + {DP83811_PHY_ID, 0xfff0},
> + {}
> };
> MODULE_DEVICE_TABLE(mdio, dp83822_tbl);
>
> -MODULE_DESCRIPTION("Texas Instruments DP83822 PHY driver");
> +MODULE_DESCRIPTION("Texas Instruments DP83811/22 PHY driver");
> MODULE_AUTHOR("Dan Murphy <dmur...@ti.com");
> MODULE_LICENSE("GPL");
>
--
--
Dan Murphy
[PATCH] net: phy: DP83811: Add support for the phy
Add support for the DP83811 phy by extending
the DP83822 driver to recognize the PHY IDs.
The DP83811 supports both rgmii and sgmii interfaces.
There are 2 part numbers for this the DP83811R does not
reliably support the SGMII interface but the DP83811S will.
There is not a way to differentiate these parts from the
hardware or register set. So this is controlled via the DT
to indicate which phy mode is required. Or the part can be
strapped to a certain interface.
Data sheet can be found here:
http://www.ti.com/product/DP83TC811S-Q1/description
http://www.ti.com/product/DP83TC811R-Q1/description
Signed-off-by: Dan Murphy <dmur...@ti.com>
---
drivers/net/phy/dp83822.c | 42 ---
1 file changed, 39 insertions(+), 3 deletions(-)
diff --git a/drivers/net/phy/dp83822.c b/drivers/net/phy/dp83822.c
index 6e8a2a4f3a6e..5c379ff25dac 100644
--- a/drivers/net/phy/dp83822.c
+++ b/drivers/net/phy/dp83822.c
@@ -23,8 +23,10 @@
#include
#define DP83822_PHY_ID 0x2000a240
+#define DP83811_PHY_ID 0x2000a253
#define DP83822_DEVADDR0x1f
+#define MII_DP83811_SGMII_CTRL 0x09
#define MII_DP83822_PHYSCR 0x11
#define MII_DP83822_MISR1 0x12
#define MII_DP83822_MISR2 0x13
@@ -79,6 +81,13 @@
#define DP83822_WOL_INDICATION_SEL BIT(8)
#define DP83822_WOL_CLR_INDICATION BIT(11)
+/* DP83811 SGMII CTRL bits */
+#define DP83811_TDR_AUTO BIT(8)
+#define DP83811_SGMII_EN BIT(12)
+#define DP83811_SGMII_AUTO_NEG_EN BIT(13)
+#define DP83811_SGMII_TX_ERR_DIS BIT(14)
+#define DP83811_SGMII_SOFT_RESET BIT(15)
+
static int dp83822_ack_interrupt(struct phy_device *phydev)
{
int err;
@@ -267,6 +276,17 @@ static int dp83822_config_init(struct phy_device *phydev)
if (err < 0)
return err;
+ if ((phydev->interface == PHY_INTERFACE_MODE_SGMII &&
+phydev->phy_id == DP83811_PHY_ID)) {
+ value = phy_read(phydev, MII_DP83811_SGMII_CTRL);
+ if (!(value & DP83811_SGMII_EN)) {
+ err = phy_write(phydev, MII_DP83811_SGMII_CTRL,
+ (DP83811_SGMII_EN | value));
+ if (err < 0)
+ return err;
+ }
+ }
+
value = DP83822_WOL_MAGIC_EN | DP83822_WOL_SECURE_ON | DP83822_WOL_EN;
return phy_write_mmd(phydev, DP83822_DEVADDR, MII_DP83822_WOL_CFG,
@@ -328,15 +348,31 @@ static struct phy_driver dp83822_driver[] = {
.suspend = dp83822_suspend,
.resume = dp83822_resume,
},
+ {
+ .phy_id = DP83811_PHY_ID,
+ .phy_id_mask = 0xfff0,
+ .name = "TI DP83811",
+ .features = PHY_BASIC_FEATURES,
+ .flags = PHY_HAS_INTERRUPT,
+ .config_init = genphy_config_init,
+ .soft_reset = dp83822_phy_reset,
+ .get_wol = dp83822_get_wol,
+ .set_wol = dp83822_set_wol,
+ .ack_interrupt = dp83822_ack_interrupt,
+ .config_intr = dp83822_config_intr,
+ .suspend = dp83822_suspend,
+ .resume = dp83822_resume,
+},
};
module_phy_driver(dp83822_driver);
static struct mdio_device_id __maybe_unused dp83822_tbl[] = {
- { DP83822_PHY_ID, 0xfff0 },
- { },
+ {DP83822_PHY_ID, 0xfff0},
+ {DP83811_PHY_ID, 0xfff0},
+ {}
};
MODULE_DEVICE_TABLE(mdio, dp83822_tbl);
-MODULE_DESCRIPTION("Texas Instruments DP83822 PHY driver");
+MODULE_DESCRIPTION("Texas Instruments DP83811/22 PHY driver");
MODULE_AUTHOR("Dan Murphy <dmur...@ti.com");
MODULE_LICENSE("GPL");
--
2.17.0.252.gfe0a9eaf3
Re: [PATCH v5 1/2] net: phy: DP83822 initial driver submission
Florian
On 10/10/2017 01:56 PM, woojung@microchip.com wrote:
>> +static int dp83822_config_intr(struct phy_device *phydev)
>> +{
>> +int misr_status;
>> +int physcr_status;
>> +int err;
>> +
>> +if (phydev->interrupts == PHY_INTERRUPT_ENABLED) {
>> +misr_status = phy_read(phydev, MII_DP83822_MISR1);
>> +if (misr_status < 0)
>> +return misr_status;
>> +
>> +misr_status |= (DP83822_RX_ERR_HF_INT_EN |
>> +DP83822_FALSE_CARRIER_HF_INT_EN |
>> +DP83822_ANEG_COMPLETE_INT_EN |
>> +DP83822_DUP_MODE_CHANGE_INT_EN |
>> +DP83822_SPEED_CHANGED_INT_EN |
>> +DP83822_LINK_STAT_INT_EN |
>> +DP83822_ENERGY_DET_INT_EN |
>> +DP83822_LINK_QUAL_INT_EN);
>> +
>> +err = phy_write(phydev, MII_DP83822_MISR1, misr_status);
>> +if (err < 0)
>> +return err;
>> +
>> +misr_status = phy_read(phydev, MII_DP83822_MISR2);
>> +if (misr_status < 0)
>> +return misr_status;
>> +
>> +misr_status |= (DP83822_JABBER_DET_INT_EN |
>> +DP83822_WOL_PKT_INT_EN |
>> +DP83822_SLEEP_MODE_INT_EN |
>> +DP83822_MDI_XOVER_INT_EN |
>> +DP83822_LB_FIFO_INT_EN |
>> +DP83822_PAGE_RX_INT_EN |
>> +DP83822_ANEG_ERR_INT_EN |
>> +DP83822_EEE_ERROR_CHANGE_INT_EN);
>> +
>> +err = phy_write(phydev, MII_DP83822_MISR2, misr_status);
>> +if (err < 0)
>> +return err;
>> +
>> +physcr_status = phy_read(phydev, MII_DP83822_PHYSCR);
>> +if (physcr_status < 0)
>> +return physcr_status;
>> +
>> +physcr_status |= DP83822_PHYSCR_INT_OE |
>> DP83822_PHYSCR_INTEN;
>> +
> Don't want to be picky, but seeing extra blank line here.
>
Let me know if you want me to fix this up. Or if you will when you apply it.
Dan
--
--
Dan Murphy
[PATCH v5 1/2] net: phy: DP83822 initial driver submission
Add support for the TI DP83822 10/100Mbit ethernet phy. The DP83822 provides flexibility to connect to a MAC through a standard MII, RMII or RGMII interface. In addition the DP83822 needs to be removed from the DP83848 driver as the WoL support is added here for this device. Datasheet: http://www.ti.com/product/DP83822I/datasheet Signed-off-by: Dan Murphy <dmur...@ti.com> --- v5 - Fixed bit mask, added config_init to set WOL register bits, fixed spacing issues, moved password storage under secure on if statement, and added INT_EN bit setting for interrupt enable. https://www.mail-archive.com/netdev@vger.kernel.org/msg192495.html v4 - Squash DP83822 removal patch into this patch - https://www.mail-archive.com/netdev@vger.kernel.org/msg192424.html v3 - Fixed WoL indication bit and removed mutex for suspend/resume - https://www.mail-archive.com/netdev@vger.kernel.org/msg191891.html and https://www.mail-archive.com/netdev@vger.kernel.org/msg191665.html v2 - Updated per comments. Removed unnessary parantheis, called genphy_suspend/ resume routines and then performing WoL changes, reworked sopass storage and reduced the number of phy reads, and moved WOL_SECURE_ON - https://www.mail-archive.com/netdev@vger.kernel.org/msg191392.html drivers/net/phy/Kconfig | 5 + drivers/net/phy/Makefile | 1 + drivers/net/phy/dp83822.c | 344 ++ drivers/net/phy/dp83848.c | 3 - 4 files changed, 350 insertions(+), 3 deletions(-) create mode 100644 drivers/net/phy/dp83822.c diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig index cd931cf9dcc2..8e78a482e09e 100644 --- a/drivers/net/phy/Kconfig +++ b/drivers/net/phy/Kconfig @@ -277,6 +277,11 @@ config DAVICOM_PHY ---help--- Currently supports dm9161e and dm9131 +config DP83822_PHY + tristate "Texas Instruments DP83822 PHY" + ---help--- + Supports the DP83822 PHY. + config DP83848_PHY tristate "Texas Instruments DP83848 PHY" ---help--- diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile index 416df92fbf4f..df3b82ba8550 100644 --- a/drivers/net/phy/Makefile +++ b/drivers/net/phy/Makefile @@ -55,6 +55,7 @@ obj-$(CONFIG_CICADA_PHY) += cicada.o obj-$(CONFIG_CORTINA_PHY) += cortina.o obj-$(CONFIG_DAVICOM_PHY) += davicom.o obj-$(CONFIG_DP83640_PHY) += dp83640.o +obj-$(CONFIG_DP83822_PHY) += dp83822.o obj-$(CONFIG_DP83848_PHY) += dp83848.o obj-$(CONFIG_DP83867_PHY) += dp83867.o obj-$(CONFIG_FIXED_PHY)+= fixed_phy.o diff --git a/drivers/net/phy/dp83822.c b/drivers/net/phy/dp83822.c new file mode 100644 index ..14335d14e9e4 --- /dev/null +++ b/drivers/net/phy/dp83822.c @@ -0,0 +1,344 @@ +/* + * Driver for the Texas Instruments DP83822 PHY + * + * Copyright (C) 2017 Texas Instruments Inc. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#include +#include +#include +#include +#include +#include +#include +#include + +#define DP83822_PHY_ID 0x2000a240 +#define DP83822_DEVADDR0x1f + +#define MII_DP83822_PHYSCR 0x11 +#define MII_DP83822_MISR1 0x12 +#define MII_DP83822_MISR2 0x13 +#define MII_DP83822_RESET_CTRL 0x1f + +#define DP83822_HW_RESET BIT(15) +#define DP83822_SW_RESET BIT(14) + +/* PHYSCR Register Fields */ +#define DP83822_PHYSCR_INT_OE BIT(0) /* Interrupt Output Enable */ +#define DP83822_PHYSCR_INTEN BIT(1) /* Interrupt Enable */ + +/* MISR1 bits */ +#define DP83822_RX_ERR_HF_INT_EN BIT(0) +#define DP83822_FALSE_CARRIER_HF_INT_ENBIT(1) +#define DP83822_ANEG_COMPLETE_INT_EN BIT(2) +#define DP83822_DUP_MODE_CHANGE_INT_EN BIT(3) +#define DP83822_SPEED_CHANGED_INT_EN BIT(4) +#define DP83822_LINK_STAT_INT_EN BIT(5) +#define DP83822_ENERGY_DET_INT_EN BIT(6) +#define DP83822_LINK_QUAL_INT_EN BIT(7) + +/* MISR2 bits */ +#define DP83822_JABBER_DET_INT_EN BIT(0) +#define DP83822_WOL_PKT_INT_EN BIT(1) +#define DP83822_SLEEP_MODE_INT_EN BIT(2) +#define DP83822_MDI_XOVER_INT_EN BIT(3) +#define DP83822_LB_FIFO_INT_EN BIT(4) +#define DP83822_PAGE_RX_INT_EN BIT(5) +#define DP83822_ANEG_ERR_INT_ENBIT(6) +#define DP83822_EEE_ERROR_CHANGE_INT_ENBIT(7) + +/* INT_STAT1 bits */ +#define DP83822_WOL_INT_EN BIT(4) +#define DP83822_WOL_INT_STAT BIT(12) + +#define MII_DP83822_RXSOP1 0x04a5 +#defineMII_DP83822_RXSOP2 0x04a6 +#defineMII_
[PATCH v5 2/2] net: phy: at803x: Change error to EINVAL for invalid MAC
Change the return error code to EINVAL if the MAC
address is not valid in the set_wol function.
Signed-off-by: Dan Murphy <dmur...@ti.com>
---
v5 - No changes made
v4 - Updated $subject to include the part number -
https://www.mail-archive.com/netdev@vger.kernel.org/msg192424.html
v3 - No changes made
v2 - There was no v1 on this patch this is new.
drivers/net/phy/at803x.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
index c1e52b9dc58d..5f93e6add563 100644
--- a/drivers/net/phy/at803x.c
+++ b/drivers/net/phy/at803x.c
@@ -167,7 +167,7 @@ static int at803x_set_wol(struct phy_device *phydev,
mac = (const u8 *) ndev->dev_addr;
if (!is_valid_ether_addr(mac))
- return -EFAULT;
+ return -EINVAL;
for (i = 0; i < 3; i++) {
phy_write(phydev, AT803X_MMD_ACCESS_CONTROL,
--
2.14.0
Re: [PATCH v4 1/2] net: phy: DP83822 initial driver submission
Andrew Thanks for the review On 10/09/2017 02:12 PM, Andrew F. Davis wrote: > On 10/09/2017 11:59 AM, Dan Murphy wrote: >> Add support for the TI DP83822 10/100Mbit ethernet phy. >> >> The DP83822 provides flexibility to connect to a MAC through a >> standard MII, RMII or RGMII interface. >> >> In addition the DP83822 needs to be removed from the DP83848 driver >> as the WoL support is added here for this device. >> >> Datasheet: >> http://www.ti.com/product/DP83822I/datasheet >> >> Signed-off-by: Dan Murphy <dmur...@ti.com>> --- >> >> v4 - Squash DP83822 removal patch into this patch - >> https://www.mail-archive.com/netdev@vger.kernel.org/msg192424.html >> >> v3 - Fixed WoL indication bit and removed mutex for suspend/resume - >> https://www.mail-archive.com/netdev@vger.kernel.org/msg191891.html and >> https://www.mail-archive.com/netdev@vger.kernel.org/msg191665.html >> >> v2 - Updated per comments. Removed unnessary parantheis, called >> genphy_suspend/ >> resume routines and then performing WoL changes, reworked sopass storage and >> reduced >> the number of phy reads, and moved WOL_SECURE_ON - >> https://www.mail-archive.com/netdev@vger.kernel.org/msg191392.html >> >> drivers/net/phy/Kconfig | 5 + >> drivers/net/phy/Makefile | 1 + >> drivers/net/phy/dp83822.c | 302 >> ++ >> drivers/net/phy/dp83848.c | 3 - >> 4 files changed, 308 insertions(+), 3 deletions(-) >> create mode 100644 drivers/net/phy/dp83822.c >> >> diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig >> index cd931cf9dcc2..8e78a482e09e 100644 >> --- a/drivers/net/phy/Kconfig >> +++ b/drivers/net/phy/Kconfig >> @@ -277,6 +277,11 @@ config DAVICOM_PHY >> ---help--- >>Currently supports dm9161e and dm9131 >> >> +config DP83822_PHY >> +tristate "Texas Instruments DP83822 PHY" >> +---help--- >> + Supports the DP83822 PHY. >> + >> config DP83848_PHY >> tristate "Texas Instruments DP83848 PHY" >> ---help--- >> diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile >> index 416df92fbf4f..df3b82ba8550 100644 >> --- a/drivers/net/phy/Makefile >> +++ b/drivers/net/phy/Makefile >> @@ -55,6 +55,7 @@ obj-$(CONFIG_CICADA_PHY) += cicada.o >> obj-$(CONFIG_CORTINA_PHY) += cortina.o >> obj-$(CONFIG_DAVICOM_PHY) += davicom.o >> obj-$(CONFIG_DP83640_PHY) += dp83640.o >> +obj-$(CONFIG_DP83822_PHY) += dp83822.o >> obj-$(CONFIG_DP83848_PHY) += dp83848.o >> obj-$(CONFIG_DP83867_PHY) += dp83867.o >> obj-$(CONFIG_FIXED_PHY) += fixed_phy.o >> diff --git a/drivers/net/phy/dp83822.c b/drivers/net/phy/dp83822.c >> new file mode 100644 >> index ..de196dbc46cd >> --- /dev/null >> +++ b/drivers/net/phy/dp83822.c >> @@ -0,0 +1,302 @@ >> +/* >> + * Driver for the Texas Instruments DP83822 PHY >> + * >> + * Copyright (C) 2017 Texas Instruments Inc. >> + * >> + * This program is free software; you can redistribute it and/or modify >> + * it under the terms of the GNU General Public License as published by >> + * the Free Software Foundation; either version 2 of the License. >> + * >> + * This program is distributed in the hope that it will be useful, >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >> + * GNU General Public License for more details. >> + */ >> + >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> + >> +#define DP83822_PHY_ID 0x2000a240 >> +#define DP83822_DEVADDR 0x1f >> + >> +#define MII_DP83822_MISR1 0x12 >> +#define MII_DP83822_MISR2 0x13 >> +#define MII_DP83822_RESET_CTRL 0x1f >> + >> +#define DP83822_HW_RESETBIT(15) >> +#define DP83822_SW_RESETBIT(14) >> + >> +/* MISR1 bits */ >> +#define DP83822_RX_ERR_HF_INT_ENBIT(0) >> +#define DP83822_FALSE_CARRIER_HF_INT_EN BIT(1) >> +#define DP83822_ANEG_COMPLETE_INT_ENBIT(2) >> +#define DP83822_DUP_MODE_CHANGE_INT_EN BIT(3) >> +#define DP83822_SPEED_CHANGED_INT_ENBIT(4) >> +#define DP83822_LINK_STAT_INT_ENBIT(5) >> +#define DP83822_ENERGY_DET_INT_EN BIT(6) >> +#define DP83822_LINK_QUAL_INT_ENBIT(7) >> + >>
[PATCH v4 2/2] net: phy: at803x: Change error to EINVAL for invalid MAC
Change the return error code to EINVAL if the MAC
address is not valid in the set_wol function.
Signed-off-by: Dan Murphy <dmur...@ti.com>
---
v4 - Updated $subject to include the part number -
https://www.mail-archive.com/netdev@vger.kernel.org/msg192424.html
v3 - No changes made
v2 - There was no v1 on this patch this is new.
drivers/net/phy/at803x.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
index c1e52b9dc58d..5f93e6add563 100644
--- a/drivers/net/phy/at803x.c
+++ b/drivers/net/phy/at803x.c
@@ -167,7 +167,7 @@ static int at803x_set_wol(struct phy_device *phydev,
mac = (const u8 *) ndev->dev_addr;
if (!is_valid_ether_addr(mac))
- return -EFAULT;
+ return -EINVAL;
for (i = 0; i < 3; i++) {
phy_write(phydev, AT803X_MMD_ACCESS_CONTROL,
--
2.14.0
[PATCH v4 1/2] net: phy: DP83822 initial driver submission
Add support for the TI DP83822 10/100Mbit ethernet phy. The DP83822 provides flexibility to connect to a MAC through a standard MII, RMII or RGMII interface. In addition the DP83822 needs to be removed from the DP83848 driver as the WoL support is added here for this device. Datasheet: http://www.ti.com/product/DP83822I/datasheet Signed-off-by: Dan Murphy <dmur...@ti.com> --- v4 - Squash DP83822 removal patch into this patch - https://www.mail-archive.com/netdev@vger.kernel.org/msg192424.html v3 - Fixed WoL indication bit and removed mutex for suspend/resume - https://www.mail-archive.com/netdev@vger.kernel.org/msg191891.html and https://www.mail-archive.com/netdev@vger.kernel.org/msg191665.html v2 - Updated per comments. Removed unnessary parantheis, called genphy_suspend/ resume routines and then performing WoL changes, reworked sopass storage and reduced the number of phy reads, and moved WOL_SECURE_ON - https://www.mail-archive.com/netdev@vger.kernel.org/msg191392.html drivers/net/phy/Kconfig | 5 + drivers/net/phy/Makefile | 1 + drivers/net/phy/dp83822.c | 302 ++ drivers/net/phy/dp83848.c | 3 - 4 files changed, 308 insertions(+), 3 deletions(-) create mode 100644 drivers/net/phy/dp83822.c diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig index cd931cf9dcc2..8e78a482e09e 100644 --- a/drivers/net/phy/Kconfig +++ b/drivers/net/phy/Kconfig @@ -277,6 +277,11 @@ config DAVICOM_PHY ---help--- Currently supports dm9161e and dm9131 +config DP83822_PHY + tristate "Texas Instruments DP83822 PHY" + ---help--- + Supports the DP83822 PHY. + config DP83848_PHY tristate "Texas Instruments DP83848 PHY" ---help--- diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile index 416df92fbf4f..df3b82ba8550 100644 --- a/drivers/net/phy/Makefile +++ b/drivers/net/phy/Makefile @@ -55,6 +55,7 @@ obj-$(CONFIG_CICADA_PHY) += cicada.o obj-$(CONFIG_CORTINA_PHY) += cortina.o obj-$(CONFIG_DAVICOM_PHY) += davicom.o obj-$(CONFIG_DP83640_PHY) += dp83640.o +obj-$(CONFIG_DP83822_PHY) += dp83822.o obj-$(CONFIG_DP83848_PHY) += dp83848.o obj-$(CONFIG_DP83867_PHY) += dp83867.o obj-$(CONFIG_FIXED_PHY)+= fixed_phy.o diff --git a/drivers/net/phy/dp83822.c b/drivers/net/phy/dp83822.c new file mode 100644 index ..de196dbc46cd --- /dev/null +++ b/drivers/net/phy/dp83822.c @@ -0,0 +1,302 @@ +/* + * Driver for the Texas Instruments DP83822 PHY + * + * Copyright (C) 2017 Texas Instruments Inc. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#include +#include +#include +#include +#include +#include +#include +#include + +#define DP83822_PHY_ID 0x2000a240 +#define DP83822_DEVADDR0x1f + +#define MII_DP83822_MISR1 0x12 +#define MII_DP83822_MISR2 0x13 +#define MII_DP83822_RESET_CTRL 0x1f + +#define DP83822_HW_RESET BIT(15) +#define DP83822_SW_RESET BIT(14) + +/* MISR1 bits */ +#define DP83822_RX_ERR_HF_INT_EN BIT(0) +#define DP83822_FALSE_CARRIER_HF_INT_ENBIT(1) +#define DP83822_ANEG_COMPLETE_INT_EN BIT(2) +#define DP83822_DUP_MODE_CHANGE_INT_EN BIT(3) +#define DP83822_SPEED_CHANGED_INT_EN BIT(4) +#define DP83822_LINK_STAT_INT_EN BIT(5) +#define DP83822_ENERGY_DET_INT_EN BIT(6) +#define DP83822_LINK_QUAL_INT_EN BIT(7) + +/* MISR2 bits */ +#define DP83822_JABBER_DET_INT_EN BIT(0) +#define DP83822_WOL_PKT_INT_EN BIT(1) +#define DP83822_SLEEP_MODE_INT_EN BIT(2) +#define DP83822_MDI_XOVER_INT_EN BIT(3) +#define DP83822_LB_FIFO_INT_EN BIT(4) +#define DP83822_PAGE_RX_INT_EN BIT(5) +#define DP83822_ANEG_ERR_INT_ENBIT(6) +#define DP83822_EEE_ERROR_CHANGE_INT_ENBIT(7) + +/* INT_STAT1 bits */ +#define DP83822_WOL_INT_EN BIT(4) +#define DP83822_WOL_INT_STAT BIT(12) + +#define MII_DP83822_RXSOP1 0x04a5 +#defineMII_DP83822_RXSOP2 0x04a6 +#defineMII_DP83822_RXSOP3 0x04a7 + +/* WoL Registers */ +#defineMII_DP83822_WOL_CFG 0x04a0 +#defineMII_DP83822_WOL_STAT0x04a1 +#defineMII_DP83822_WOL_DA1 0x04a2 +#defineMII_DP83822_WOL_DA2 0x04a3 +#defineMII_DP83822_WOL_DA3 0x04a4 + +/* WoL bits */ +#define DP83822_WOL_MAGIC_EN BIT(1) +#define DP83822_WOL_SECURE_ON BIT(5) +#define DP83822_WOL_EN BIT(7) +#define DP83822_WOL_INDICATION_SEL BIT(8) +#define DP83822
Re: [PATCH v3 1/3] net: phy: Remove TI DP83822 from DP83848 driver
Florian
On 10/09/2017 11:44 AM, Florian Fainelli wrote:
> On 10/09/2017 05:03 AM, Dan Murphy wrote:
>> Removing the DP83822 device from the DP83848 to
>> support the TI DP83822 dedicated driver that will
>> initially support WoL settings.
> Hi Dan,
>
> The ordering of patch 1 and 2 may have to be reversed, otherwise you are
> leaving people with the Generic PHY driver matching the DP83822 PHY
> after applying patch 1, and without proper interrupt management again
> unless they apply patch 2.
>
> It may even be better to combine patch 1 and 2 actually to have a
> cleaner transition.
>
No worries I will squash patch 1 and 2.
Dan
> Sorry for noticing this so late in the series...
>
>>
>> Signed-off-by: Dan Murphy <dmur...@ti.com>
>> ---
>>
>> v3 - No changes made
>> v2 - There was no v1 on this patch this is new.
>>
>> drivers/net/phy/dp83848.c | 3 ---
>> 1 file changed, 3 deletions(-)
>>
>> diff --git a/drivers/net/phy/dp83848.c b/drivers/net/phy/dp83848.c
>> index 3de4fe4dda77..3966d43c5146 100644
>> --- a/drivers/net/phy/dp83848.c
>> +++ b/drivers/net/phy/dp83848.c
>> @@ -20,7 +20,6 @@
>> #define TI_DP83620_PHY_ID 0x20005ce0
>> #define NS_DP83848C_PHY_ID 0x20005c90
>> #define TLK10X_PHY_ID 0x2000a210
>> -#define TI_DP83822_PHY_ID 0x2000a240
>>
>> /* Registers */
>> #define DP83848_MICR0x11 /* MII Interrupt Control
>> Register */
>> @@ -80,7 +79,6 @@ static struct mdio_device_id __maybe_unused dp83848_tbl[]
>> = {
>> { NS_DP83848C_PHY_ID, 0xfff0 },
>> { TI_DP83620_PHY_ID, 0xfff0 },
>> { TLK10X_PHY_ID, 0xfff0 },
>> -{ TI_DP83822_PHY_ID, 0xfff0 },
>> { }
>> };
>> MODULE_DEVICE_TABLE(mdio, dp83848_tbl);
>> @@ -110,7 +108,6 @@ static struct phy_driver dp83848_driver[] = {
>> DP83848_PHY_DRIVER(NS_DP83848C_PHY_ID, "NS DP83848C 10/100 Mbps PHY"),
>> DP83848_PHY_DRIVER(TI_DP83620_PHY_ID, "TI DP83620 10/100 Mbps PHY"),
>> DP83848_PHY_DRIVER(TLK10X_PHY_ID, "TI TLK10X 10/100 Mbps PHY"),
>> -DP83848_PHY_DRIVER(TI_DP83822_PHY_ID, "TI DP83822 10/100 Mbps PHY"),
>> };
>> module_phy_driver(dp83848_driver);
>>
>>
>
>
--
--
Dan Murphy
[PATCH v3 3/3] net: phy: Change error to EINVAL for invalid MAC
Change the return error code to EINVAL if the MAC
address is not valid in the set_wol function.
Signed-off-by: Dan Murphy <dmur...@ti.com>
---
v3 - No changes made
v2 - There was no v1 on this patch this is new.
drivers/net/phy/at803x.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
index c1e52b9dc58d..5f93e6add563 100644
--- a/drivers/net/phy/at803x.c
+++ b/drivers/net/phy/at803x.c
@@ -167,7 +167,7 @@ static int at803x_set_wol(struct phy_device *phydev,
mac = (const u8 *) ndev->dev_addr;
if (!is_valid_ether_addr(mac))
- return -EFAULT;
+ return -EINVAL;
for (i = 0; i < 3; i++) {
phy_write(phydev, AT803X_MMD_ACCESS_CONTROL,
--
2.14.0
[PATCH v3 2/3] net: phy: DP83822 initial driver submission
Add support for the TI DP83822 10/100Mbit ethernet phy.
The DP83822 provides flexibility to connect to a MAC through a
standard MII, RMII or RGMII interface.
Datasheet:
http://www.ti.com/product/DP83822I/datasheet
Signed-off-by: Dan Murphy <dmur...@ti.com>
---
v3 - Fixed WoL indication bit and removed mutex for suspend/resume -
https://www.mail-archive.com/netdev@vger.kernel.org/msg191891.html and
https://www.mail-archive.com/netdev@vger.kernel.org/msg191665.html
v2 - Updated per comments. Removed unnessary parantheis, called genphy_suspend/
resume routines and then performing WoL changes, reworked sopass storage and
reduced
the number of phy reads, and moved WOL_SECURE_ON -
https://www.mail-archive.com/netdev@vger.kernel.org/msg191392.html
drivers/net/phy/Kconfig | 5 +
drivers/net/phy/Makefile | 1 +
drivers/net/phy/dp83822.c | 302 ++
3 files changed, 308 insertions(+)
create mode 100644 drivers/net/phy/dp83822.c
diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index cd931cf9dcc2..8e78a482e09e 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -277,6 +277,11 @@ config DAVICOM_PHY
---help---
Currently supports dm9161e and dm9131
+config DP83822_PHY
+ tristate "Texas Instruments DP83822 PHY"
+ ---help---
+ Supports the DP83822 PHY.
+
config DP83848_PHY
tristate "Texas Instruments DP83848 PHY"
---help---
diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
index 416df92fbf4f..df3b82ba8550 100644
--- a/drivers/net/phy/Makefile
+++ b/drivers/net/phy/Makefile
@@ -55,6 +55,7 @@ obj-$(CONFIG_CICADA_PHY) += cicada.o
obj-$(CONFIG_CORTINA_PHY) += cortina.o
obj-$(CONFIG_DAVICOM_PHY) += davicom.o
obj-$(CONFIG_DP83640_PHY) += dp83640.o
+obj-$(CONFIG_DP83822_PHY) += dp83822.o
obj-$(CONFIG_DP83848_PHY) += dp83848.o
obj-$(CONFIG_DP83867_PHY) += dp83867.o
obj-$(CONFIG_FIXED_PHY)+= fixed_phy.o
diff --git a/drivers/net/phy/dp83822.c b/drivers/net/phy/dp83822.c
new file mode 100644
index ..de196dbc46cd
--- /dev/null
+++ b/drivers/net/phy/dp83822.c
@@ -0,0 +1,302 @@
+/*
+ * Driver for the Texas Instruments DP83822 PHY
+ *
+ * Copyright (C) 2017 Texas Instruments Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define DP83822_PHY_ID 0x2000a240
+#define DP83822_DEVADDR0x1f
+
+#define MII_DP83822_MISR1 0x12
+#define MII_DP83822_MISR2 0x13
+#define MII_DP83822_RESET_CTRL 0x1f
+
+#define DP83822_HW_RESET BIT(15)
+#define DP83822_SW_RESET BIT(14)
+
+/* MISR1 bits */
+#define DP83822_RX_ERR_HF_INT_EN BIT(0)
+#define DP83822_FALSE_CARRIER_HF_INT_ENBIT(1)
+#define DP83822_ANEG_COMPLETE_INT_EN BIT(2)
+#define DP83822_DUP_MODE_CHANGE_INT_EN BIT(3)
+#define DP83822_SPEED_CHANGED_INT_EN BIT(4)
+#define DP83822_LINK_STAT_INT_EN BIT(5)
+#define DP83822_ENERGY_DET_INT_EN BIT(6)
+#define DP83822_LINK_QUAL_INT_EN BIT(7)
+
+/* MISR2 bits */
+#define DP83822_JABBER_DET_INT_EN BIT(0)
+#define DP83822_WOL_PKT_INT_EN BIT(1)
+#define DP83822_SLEEP_MODE_INT_EN BIT(2)
+#define DP83822_MDI_XOVER_INT_EN BIT(3)
+#define DP83822_LB_FIFO_INT_EN BIT(4)
+#define DP83822_PAGE_RX_INT_EN BIT(5)
+#define DP83822_ANEG_ERR_INT_ENBIT(6)
+#define DP83822_EEE_ERROR_CHANGE_INT_ENBIT(7)
+
+/* INT_STAT1 bits */
+#define DP83822_WOL_INT_EN BIT(4)
+#define DP83822_WOL_INT_STAT BIT(12)
+
+#define MII_DP83822_RXSOP1 0x04a5
+#defineMII_DP83822_RXSOP2 0x04a6
+#defineMII_DP83822_RXSOP3 0x04a7
+
+/* WoL Registers */
+#defineMII_DP83822_WOL_CFG 0x04a0
+#defineMII_DP83822_WOL_STAT0x04a1
+#defineMII_DP83822_WOL_DA1 0x04a2
+#defineMII_DP83822_WOL_DA2 0x04a3
+#defineMII_DP83822_WOL_DA3 0x04a4
+
+/* WoL bits */
+#define DP83822_WOL_MAGIC_EN BIT(1)
+#define DP83822_WOL_SECURE_ON BIT(5)
+#define DP83822_WOL_EN BIT(7)
+#define DP83822_WOL_INDICATION_SEL BIT(8)
+#define DP83822_WOL_CLR_INDICATION BIT(11)
+
+static int dp83822_ack_interrupt(struct phy_device *phydev)
+{
+ int err = phy_read(phydev, MII_DP83822_MISR1);
+
+ if (err < 0)
+ return err;
+
+ err = phy_read(phydev, MII_DP83822_MISR2);
+ if (err < 0)
+
[PATCH v3 1/3] net: phy: Remove TI DP83822 from DP83848 driver
Removing the DP83822 device from the DP83848 to
support the TI DP83822 dedicated driver that will
initially support WoL settings.
Signed-off-by: Dan Murphy <dmur...@ti.com>
---
v3 - No changes made
v2 - There was no v1 on this patch this is new.
drivers/net/phy/dp83848.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/net/phy/dp83848.c b/drivers/net/phy/dp83848.c
index 3de4fe4dda77..3966d43c5146 100644
--- a/drivers/net/phy/dp83848.c
+++ b/drivers/net/phy/dp83848.c
@@ -20,7 +20,6 @@
#define TI_DP83620_PHY_ID 0x20005ce0
#define NS_DP83848C_PHY_ID 0x20005c90
#define TLK10X_PHY_ID 0x2000a210
-#define TI_DP83822_PHY_ID 0x2000a240
/* Registers */
#define DP83848_MICR 0x11 /* MII Interrupt Control Register
*/
@@ -80,7 +79,6 @@ static struct mdio_device_id __maybe_unused dp83848_tbl[] = {
{ NS_DP83848C_PHY_ID, 0xfff0 },
{ TI_DP83620_PHY_ID, 0xfff0 },
{ TLK10X_PHY_ID, 0xfff0 },
- { TI_DP83822_PHY_ID, 0xfff0 },
{ }
};
MODULE_DEVICE_TABLE(mdio, dp83848_tbl);
@@ -110,7 +108,6 @@ static struct phy_driver dp83848_driver[] = {
DP83848_PHY_DRIVER(NS_DP83848C_PHY_ID, "NS DP83848C 10/100 Mbps PHY"),
DP83848_PHY_DRIVER(TI_DP83620_PHY_ID, "TI DP83620 10/100 Mbps PHY"),
DP83848_PHY_DRIVER(TLK10X_PHY_ID, "TI TLK10X 10/100 Mbps PHY"),
- DP83848_PHY_DRIVER(TI_DP83822_PHY_ID, "TI DP83822 10/100 Mbps PHY"),
};
module_phy_driver(dp83848_driver);
--
2.14.0
Re: [PATCH 2/3 v2] net: phy: DP83822 initial driver submission
Andrew
On 10/04/2017 06:53 PM, Andrew Lunn wrote:
> On Wed, Oct 04, 2017 at 10:44:36PM +, woojung@microchip.com wrote:
>>> +static int dp83822_suspend(struct phy_device *phydev)
>>> +{
>>> + int value;
>>> +
>>> + mutex_lock(>lock);
>>> + value = phy_read_mmd(phydev, DP83822_DEVADDR,
>>> MII_DP83822_WOL_CFG);
>>> + mutex_unlock(>lock);
>
>> Would we need mutex to access phy_read_mmd()?
>> phy_read_mmd() has mdio_lock for indirect access.
>
> Hi Woojung
>
> The mdio lock is not sufficient. It protects against two mdio
> accesses. But here we need to protect against two phy operations.
> There is a danger something else tries to access the phy during
> suspend.
>
>>> + if (!(value & DP83822_WOL_EN))
>>> + genphy_suspend(phydev);
>
> Releasing the lock before calling genphy_suspend() is not so nice.
> Maybe add a version which assumes the lock has already been taken?
>
The marvell driver does not take a lock and calls genphy_suspend/resume
so I am wondering if this driver needs to take a lock.
The at803x needs to take the lock because it does not call into the genphy
functions.
I will submit a new version with the lock removed.
Dan
> Andrew
>
--
--
Dan Murphy
Re: [PATCH 2/3 v2] net: phy: DP83822 initial driver submission
Andrew
On 10/04/2017 06:53 PM, Andrew Lunn wrote:
> On Wed, Oct 04, 2017 at 10:44:36PM +, woojung@microchip.com wrote:
>>> +static int dp83822_suspend(struct phy_device *phydev)
>>> +{
>>> + int value;
>>> +
>>> + mutex_lock(>lock);
>>> + value = phy_read_mmd(phydev, DP83822_DEVADDR,
>>> MII_DP83822_WOL_CFG);
>>> + mutex_unlock(>lock);
>
>> Would we need mutex to access phy_read_mmd()?
>> phy_read_mmd() has mdio_lock for indirect access.
>
> Hi Woojung
>
> The mdio lock is not sufficient. It protects against two mdio
> accesses. But here we need to protect against two phy operations.
> There is a danger something else tries to access the phy during
> suspend.
>
>>> + if (!(value & DP83822_WOL_EN))
>>> + genphy_suspend(phydev);
>
> Releasing the lock before calling genphy_suspend() is not so nice.
> Maybe add a version which assumes the lock has already been taken?
>
The marvell driver does not take a lock and calls genphy_suspend/resume
so I am wondering if this driver needs to take a lock.
The at803x needs to take the lock because it does not call into the genphy
functions.
Dan
> Andrew
>
--
--
Dan Murphy
Re: [PATCH] net: phy: DP83822 initial driver submission (fwd)
Julia
On 10/05/2017 11:10 AM, Julia Lawall wrote:
> DP83822_WOL_CLR_INDICATION appears twice on line 136. Perhaps this is not
> what is wanted.
>
That line is wrong it should have DP83822_WOL_INDICATION_SEL included as well.
Onto v3.
Dan
> julia
>
> -- Forwarded message --
> Date: Thu, 5 Oct 2017 21:38:28 +0800
> From: kbuild test robot <fengguang...@intel.com>
> To: kbu...@01.org
> Cc: Julia Lawall <julia.law...@lip6.fr>
> Subject: Re: [PATCH] net: phy: DP83822 initial driver submission
>
> CC: kbuild-...@01.org
> In-Reply-To: <20171003155316.12312-1-dmur...@ti.com>
> TO: Dan Murphy <dmur...@ti.com>
> CC: and...@lunn.ch, f.faine...@gmail.com, netdev@vger.kernel.org, Dan Murphy
> <dmur...@ti.com>
> CC: netdev@vger.kernel.org, Dan Murphy <dmur...@ti.com>
>
> Hi Dan,
>
> [auto build test WARNING on net-next/master]
> [also build test WARNING on v4.14-rc3 next-20170929]
> [if your patch is applied to the wrong git tree, please drop us a note to
> help improve the system]
>
> url:
> https://github.com/0day-ci/linux/commits/Dan-Murphy/net-phy-DP83822-initial-driver-submission/20171005-165547
> :: branch date: 5 hours ago
> :: commit date: 5 hours ago
>
>>> drivers/net/phy/dp83822.c:136:29-55: duplicated argument to & or |
>
> #
> https://github.com/0day-ci/linux/commit/49190df6a2304f031dc2a6ac63710447db36bc23
> git remote add linux-review https://github.com/0day-ci/linux
> git remote update linux-review
> git checkout 49190df6a2304f031dc2a6ac63710447db36bc23
> vim +136 drivers/net/phy/dp83822.c
>
> 49190df6 Dan Murphy 2017-10-03 90
> 49190df6 Dan Murphy 2017-10-03 91 static int dp83822_set_wol(struct
> phy_device *phydev,
> 49190df6 Dan Murphy 2017-10-03 92 struct
> ethtool_wolinfo *wol)
> 49190df6 Dan Murphy 2017-10-03 93 {
> 49190df6 Dan Murphy 2017-10-03 94 struct net_device *ndev =
> phydev->attached_dev;
> 49190df6 Dan Murphy 2017-10-03 95 u16 value;
> 49190df6 Dan Murphy 2017-10-03 96 const u8 *mac;
> 49190df6 Dan Murphy 2017-10-03 97
> 49190df6 Dan Murphy 2017-10-03 98 if (wol->wolopts & (WAKE_MAGIC |
> WAKE_MAGICSECURE)) {
> 49190df6 Dan Murphy 2017-10-03 99 mac = (const u8
> *)ndev->dev_addr;
> 49190df6 Dan Murphy 2017-10-03 100
> 49190df6 Dan Murphy 2017-10-03 101 if (!is_valid_ether_addr(mac))
> 49190df6 Dan Murphy 2017-10-03 102 return -EFAULT;
> 49190df6 Dan Murphy 2017-10-03 103
> 49190df6 Dan Murphy 2017-10-03 104 /* MAC addresses start with
> byte 5, but stored in mac[0].
> 49190df6 Dan Murphy 2017-10-03 105* 822 PHYs store bytes 4|5,
> 2|3, 0|1
> 49190df6 Dan Murphy 2017-10-03 106*/
> 49190df6 Dan Murphy 2017-10-03 107 phy_write_mmd(phydev,
> DP83822_DEVADDR,
> 49190df6 Dan Murphy 2017-10-03 108
> MII_DP83822_WOL_DA1, (mac[1] << 8) | mac[0]);
> 49190df6 Dan Murphy 2017-10-03 109 phy_write_mmd(phydev,
> DP83822_DEVADDR,
> 49190df6 Dan Murphy 2017-10-03 110
> MII_DP83822_WOL_DA2, (mac[3] << 8) | mac[2]);
> 49190df6 Dan Murphy 2017-10-03 111 phy_write_mmd(phydev,
> DP83822_DEVADDR, MII_DP83822_WOL_DA3,
> 49190df6 Dan Murphy 2017-10-03 112 (mac[5] << 8) |
> mac[4]);
> 49190df6 Dan Murphy 2017-10-03 113
> 49190df6 Dan Murphy 2017-10-03 114 value = phy_read_mmd(phydev,
> DP83822_DEVADDR,
> 49190df6 Dan Murphy 2017-10-03 115
> MII_DP83822_WOL_CFG);
> 49190df6 Dan Murphy 2017-10-03 116 if (wol->wolopts & WAKE_MAGIC)
> 49190df6 Dan Murphy 2017-10-03 117 value |=
> DP83822_WOL_MAGIC_EN;
> 49190df6 Dan Murphy 2017-10-03 118 else
> 49190df6 Dan Murphy 2017-10-03 119 value &=
> ~DP83822_WOL_MAGIC_EN;
> 49190df6 Dan Murphy 2017-10-03 120
> 49190df6 Dan Murphy 2017-10-03 121 if (wol->wolopts &
> WAKE_MAGICSECURE) {
> 49190df6 Dan Murphy 2017-10-03 122 value |=
> DP83822_WOL_SECURE_ON;
> 49190df6 Dan Murphy 2017-10-03 123 phy_write_mmd(phydev,
> DP83822_DEVADDR,
> 49190df6 Dan Murphy 2017-10-03 124
> MII_DP83822_RXSOP1,
> 49190df6 Dan Murphy 2017-10-03 125
> (wol->sopass[1] << 8) | wol->sopass[0]);
> 49190df6 Dan Murphy 2017-10-03 126 phy_write_mmd(phydev,
> DP83822_DEVADDR,
> 49190df6 Dan Murphy 2017-10-03 127
> MII_DP83822_RXSOP2,
&g
[PATCH 2/3 v2] net: phy: DP83822 initial driver submission
Add support for the TI DP83822 10/100Mbit ethernet phy.
The DP83822 provides flexibility to connect to a MAC through a
standard MII, RMII or RGMII interface.
Datasheet:
http://www.ti.com/product/DP83822I/datasheet
Signed-off-by: Dan Murphy <dmur...@ti.com>
---
v2 - Updated per comments. Removed unnessary parantheis, called genphy_suspend/
resume routines and then performing WoL changes, reworked sopass storage and
reduced
the number of phy reads, and moved WOL_SECURE_ON -
https://www.mail-archive.com/netdev@vger.kernel.org/msg191392.html
drivers/net/phy/Kconfig | 5 +
drivers/net/phy/Makefile | 1 +
drivers/net/phy/dp83822.c | 306 ++
3 files changed, 312 insertions(+)
create mode 100644 drivers/net/phy/dp83822.c
diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index cd931cf9dcc2..8e78a482e09e 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -277,6 +277,11 @@ config DAVICOM_PHY
---help---
Currently supports dm9161e and dm9131
+config DP83822_PHY
+ tristate "Texas Instruments DP83822 PHY"
+ ---help---
+ Supports the DP83822 PHY.
+
config DP83848_PHY
tristate "Texas Instruments DP83848 PHY"
---help---
diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
index 416df92fbf4f..df3b82ba8550 100644
--- a/drivers/net/phy/Makefile
+++ b/drivers/net/phy/Makefile
@@ -55,6 +55,7 @@ obj-$(CONFIG_CICADA_PHY) += cicada.o
obj-$(CONFIG_CORTINA_PHY) += cortina.o
obj-$(CONFIG_DAVICOM_PHY) += davicom.o
obj-$(CONFIG_DP83640_PHY) += dp83640.o
+obj-$(CONFIG_DP83822_PHY) += dp83822.o
obj-$(CONFIG_DP83848_PHY) += dp83848.o
obj-$(CONFIG_DP83867_PHY) += dp83867.o
obj-$(CONFIG_FIXED_PHY)+= fixed_phy.o
diff --git a/drivers/net/phy/dp83822.c b/drivers/net/phy/dp83822.c
new file mode 100644
index ..200a0e39756e
--- /dev/null
+++ b/drivers/net/phy/dp83822.c
@@ -0,0 +1,306 @@
+/*
+ * Driver for the Texas Instruments DP83822 PHY
+ *
+ * Copyright (C) 2017 Texas Instruments Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define DP83822_PHY_ID 0x2000a240
+#define DP83822_DEVADDR0x1f
+
+#define MII_DP83822_MISR1 0x12
+#define MII_DP83822_MISR2 0x13
+#define MII_DP83822_RESET_CTRL 0x1f
+
+#define DP83822_HW_RESET BIT(15)
+#define DP83822_SW_RESET BIT(14)
+
+/* MISR1 bits */
+#define DP83822_RX_ERR_HF_INT_EN BIT(0)
+#define DP83822_FALSE_CARRIER_HF_INT_ENBIT(1)
+#define DP83822_ANEG_COMPLETE_INT_EN BIT(2)
+#define DP83822_DUP_MODE_CHANGE_INT_EN BIT(3)
+#define DP83822_SPEED_CHANGED_INT_EN BIT(4)
+#define DP83822_LINK_STAT_INT_EN BIT(5)
+#define DP83822_ENERGY_DET_INT_EN BIT(6)
+#define DP83822_LINK_QUAL_INT_EN BIT(7)
+
+/* MISR2 bits */
+#define DP83822_JABBER_DET_INT_EN BIT(0)
+#define DP83822_WOL_PKT_INT_EN BIT(1)
+#define DP83822_SLEEP_MODE_INT_EN BIT(2)
+#define DP83822_MDI_XOVER_INT_EN BIT(3)
+#define DP83822_LB_FIFO_INT_EN BIT(4)
+#define DP83822_PAGE_RX_INT_EN BIT(5)
+#define DP83822_ANEG_ERR_INT_ENBIT(6)
+#define DP83822_EEE_ERROR_CHANGE_INT_ENBIT(7)
+
+/* INT_STAT1 bits */
+#define DP83822_WOL_INT_EN BIT(4)
+#define DP83822_WOL_INT_STAT BIT(12)
+
+#define MII_DP83822_RXSOP1 0x04a5
+#defineMII_DP83822_RXSOP2 0x04a6
+#defineMII_DP83822_RXSOP3 0x04a7
+
+/* WoL Registers */
+#defineMII_DP83822_WOL_CFG 0x04a0
+#defineMII_DP83822_WOL_STAT0x04a1
+#defineMII_DP83822_WOL_DA1 0x04a2
+#defineMII_DP83822_WOL_DA2 0x04a3
+#defineMII_DP83822_WOL_DA3 0x04a4
+
+/* WoL bits */
+#define DP83822_WOL_MAGIC_EN BIT(1)
+#define DP83822_WOL_SECURE_ON BIT(5)
+#define DP83822_WOL_EN BIT(7)
+#define DP83822_WOL_INDICATION_SEL BIT(8)
+#define DP83822_WOL_CLR_INDICATION BIT(11)
+
+static int dp83822_ack_interrupt(struct phy_device *phydev)
+{
+ int err = phy_read(phydev, MII_DP83822_MISR1);
+
+ if (err < 0)
+ return err;
+
+ err = phy_read(phydev, MII_DP83822_MISR2);
+ if (err < 0)
+ return err;
+
+ return 0;
+}
+
+static int dp83822_set_wol(struct phy_device *phydev,
+ struct ethtool_wolinfo *wol)
+{
+ struct net_device *ndev = phydev->attached_de
[PATCH 3/3 v2] net: phy: Change error to EINVAL for invalid MAC
Change the return error code to EINVAL if the MAC
address is not valid in the set_wol function.
Signed-off-by: Dan Murphy <dmur...@ti.com>
---
v2 - There was no v1 on this patch this is new.
drivers/net/phy/at803x.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
index c1e52b9dc58d..5f93e6add563 100644
--- a/drivers/net/phy/at803x.c
+++ b/drivers/net/phy/at803x.c
@@ -167,7 +167,7 @@ static int at803x_set_wol(struct phy_device *phydev,
mac = (const u8 *) ndev->dev_addr;
if (!is_valid_ether_addr(mac))
- return -EFAULT;
+ return -EINVAL;
for (i = 0; i < 3; i++) {
phy_write(phydev, AT803X_MMD_ACCESS_CONTROL,
--
2.14.0
[PATCH 1/3 v2] net: phy: Remove TI DP83822 from DP83848 driver
Removing the DP83822 device from the DP83848 to
support the TI DP83822 dedicated driver that will
initially support WoL settings.
Signed-off-by: Dan Murphy <dmur...@ti.com>
---
v2 - There was no v1 on this patch this is new.
drivers/net/phy/dp83848.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/net/phy/dp83848.c b/drivers/net/phy/dp83848.c
index 3de4fe4dda77..3966d43c5146 100644
--- a/drivers/net/phy/dp83848.c
+++ b/drivers/net/phy/dp83848.c
@@ -20,7 +20,6 @@
#define TI_DP83620_PHY_ID 0x20005ce0
#define NS_DP83848C_PHY_ID 0x20005c90
#define TLK10X_PHY_ID 0x2000a210
-#define TI_DP83822_PHY_ID 0x2000a240
/* Registers */
#define DP83848_MICR 0x11 /* MII Interrupt Control Register
*/
@@ -80,7 +79,6 @@ static struct mdio_device_id __maybe_unused dp83848_tbl[] = {
{ NS_DP83848C_PHY_ID, 0xfff0 },
{ TI_DP83620_PHY_ID, 0xfff0 },
{ TLK10X_PHY_ID, 0xfff0 },
- { TI_DP83822_PHY_ID, 0xfff0 },
{ }
};
MODULE_DEVICE_TABLE(mdio, dp83848_tbl);
@@ -110,7 +108,6 @@ static struct phy_driver dp83848_driver[] = {
DP83848_PHY_DRIVER(NS_DP83848C_PHY_ID, "NS DP83848C 10/100 Mbps PHY"),
DP83848_PHY_DRIVER(TI_DP83620_PHY_ID, "TI DP83620 10/100 Mbps PHY"),
DP83848_PHY_DRIVER(TLK10X_PHY_ID, "TI TLK10X 10/100 Mbps PHY"),
- DP83848_PHY_DRIVER(TI_DP83822_PHY_ID, "TI DP83822 10/100 Mbps PHY"),
};
module_phy_driver(dp83848_driver);
--
2.14.0
Re: [PATCH] net: phy: DP83822 initial driver submission
Florian
On 10/04/2017 11:18 AM, Florian Fainelli wrote:
> On 10/04/2017 08:41 AM, Dan Murphy wrote:
>> Florian
>>
>> On 10/03/2017 01:31 PM, Florian Fainelli wrote:
>>> On 10/03/2017 11:03 AM, Dan Murphy wrote:
>>>> Florian
>>>>
>>>> Thanks for the review
>>>>
>>>> On 10/03/2017 12:15 PM, Florian Fainelli wrote:
>>>>>> +} else {
>>>>>> +value &= ~DP83822_WOL_SECURE_ON;
>>>>>> +}
>>>>>> +
>>>>>> +value |= (DP83822_WOL_EN | DP83822_WOL_CLR_INDICATION |
>>>>>> + DP83822_WOL_CLR_INDICATION);
>>>>>
>>>>> The extra parenthesis should not be required here.
>>>>
>>>> I did not code that in. I had to add it after Checkpatch cribbed about it.
>>>> Let me know if you want me to remove it.
>>>
>>> Let's keep those, that does not change much.
>>>
>>>>
>>>>>
>>>>>> +phy_write_mmd(phydev, DP83822_DEVADDR,
>>>>>> MII_DP83822_WOL_CFG,
>>>>>> + value);
>>>>>> +} else {
>>>>>> +value =
>>>>>> +phy_read_mmd(phydev, DP83822_DEVADDR,
>>>>>> MII_DP83822_WOL_CFG);
>>>>>> +value &= (~DP83822_WOL_EN);
>>>>>
>>>>> Same here, parenthesis should not be needed.
>>>>
>>>> There are three lines of code in the else. This code all needs to be
>>>> excuted in the else case.
>>>> I might reformat it to read better. Lindent messed that one up.
>>>
>>> sorry, I meant to write that you don't need the parenthesis around
>>> DP83822_WOL_EN since that is just a single bit here.
>>>
>>> [snip]
>>>
>>>>>> +
>>>>>> +mutex_unlock(>lock);
>>>>>> +
>>>>>> +return 0;
>>>>>> +}
>>>>>> +
>>>>>> +static int dp83822_resume(struct phy_device *phydev)
>>>>>> +{
>>>>>> +int value;
>>>>>> +
>>>>>> +mutex_lock(>lock);
>>>>>> +
>>>>>> +value = phy_read(phydev, MII_BMCR);
>>>>>> +phy_write(phydev, MII_BMCR, value & ~BMCR_PDOWN);
>>>>>
>>>>> And genphy_resume() here as well?
>>>>
>>>> genphy_resume does not have WoL.
>>>
>>> I should have been cleared, I meant using genphy_{suspend,resume} to
>>> avoid open coding the setting of the BMCR_PDOWN bit, conversely clearing
>>> of that bit. Because of the locking, maybe you could introduce unlocked
>>> versions of these two routines, or you acquire and release the lock
>>> outside of genphy_{suspend,resume}?
>>
>> OK I have addressed all of the open comments and will be posting v2 shortly.
>>
>> I do have a question on this request.
>> Are you indicating to exclusively call genphy_(suspend/resume) from within
>> the over ridden
>> routine and then take care of the WoL bits in the phy specific code?
>>
>> Since the genphy code is duplicated here for the BMCR value that would make
>> the most sense
>> to me. The genphy code is exported so I can call it explicitly then do the
>> WoL
>
> I would expect you to wrap your own calls around genphy_suspend and
> genphy_resume, such your functions become:
>
> static int dp83822_suspend(struct phy_device *phydev)
> {
> int value;
>
> mutex_lock(>lock);
> value = phy_read_mmd(phydev, DP83822_DEVADDR, MII_DP83822_WOL_CFG);
> mutex_unlock(>lock);
>
> if (~value & DP83822_WOL_EN)
> genphy_suspend(phydev);
> }
>
> static int dp83822_resume(struct phy_device *phydev)
> {
> int value;
>
> genphy_resume(phydev);
>
> mutex_lock(>lock);
> value = phy_read_mmd(phydev, DP83822_DEVADDR, MII_DP83822_WOL_CFG);
>
> phy_write_mmd(phydev, DP83822_DEVADDR, MII_DP83822_WOL_CFG, value |
> DP83822_WOL_CLR_INDICATION);
>
> mutex_unlock(>lock);
>
> return 0;
> }
>
Great thats exactly what I did.
Dan
>>
>> Dan
>>
>> [snip]--
>> --
>> Dan Murphy
>>
>
>
--
--
Dan Murphy
Re: [PATCH] net: phy: DP83822 initial driver submission
Florian
On 10/03/2017 01:31 PM, Florian Fainelli wrote:
> On 10/03/2017 11:03 AM, Dan Murphy wrote:
>> Florian
>>
>> Thanks for the review
>>
>> On 10/03/2017 12:15 PM, Florian Fainelli wrote:
>>>> + } else {
>>>> + value &= ~DP83822_WOL_SECURE_ON;
>>>> + }
>>>> +
>>>> + value |= (DP83822_WOL_EN | DP83822_WOL_CLR_INDICATION |
>>>> +DP83822_WOL_CLR_INDICATION);
>>>
>>> The extra parenthesis should not be required here.
>>
>> I did not code that in. I had to add it after Checkpatch cribbed about it.
>> Let me know if you want me to remove it.
>
> Let's keep those, that does not change much.
>
>>
>>>
>>>> + phy_write_mmd(phydev, DP83822_DEVADDR, MII_DP83822_WOL_CFG,
>>>> +value);
>>>> + } else {
>>>> + value =
>>>> + phy_read_mmd(phydev, DP83822_DEVADDR, MII_DP83822_WOL_CFG);
>>>> + value &= (~DP83822_WOL_EN);
>>>
>>> Same here, parenthesis should not be needed.
>>
>> There are three lines of code in the else. This code all needs to be
>> excuted in the else case.
>> I might reformat it to read better. Lindent messed that one up.
>
> sorry, I meant to write that you don't need the parenthesis around
> DP83822_WOL_EN since that is just a single bit here.
>
> [snip]
>
>>>> +
>>>> + mutex_unlock(>lock);
>>>> +
>>>> + return 0;
>>>> +}
>>>> +
>>>> +static int dp83822_resume(struct phy_device *phydev)
>>>> +{
>>>> + int value;
>>>> +
>>>> + mutex_lock(>lock);
>>>> +
>>>> + value = phy_read(phydev, MII_BMCR);
>>>> + phy_write(phydev, MII_BMCR, value & ~BMCR_PDOWN);
>>>
>>> And genphy_resume() here as well?
>>
>> genphy_resume does not have WoL.
>
> I should have been cleared, I meant using genphy_{suspend,resume} to
> avoid open coding the setting of the BMCR_PDOWN bit, conversely clearing
> of that bit. Because of the locking, maybe you could introduce unlocked
> versions of these two routines, or you acquire and release the lock
> outside of genphy_{suspend,resume}?
OK I have addressed all of the open comments and will be posting v2 shortly.
I do have a question on this request.
Are you indicating to exclusively call genphy_(suspend/resume) from within the
over ridden
routine and then take care of the WoL bits in the phy specific code?
Since the genphy code is duplicated here for the BMCR value that would make the
most sense
to me. The genphy code is exported so I can call it explicitly then do the WoL
Dan
[snip]--
--
Dan Murphy
Re: [PATCH] net: phy: DP83822 initial driver submission
Florian
Thanks for the review
On 10/03/2017 12:15 PM, Florian Fainelli wrote:
> On 10/03/2017 08:53 AM, Dan Murphy wrote:
>> Add support for the TI DP83822 10/100Mbit ethernet phy.
>>
>> The DP83822 provides flexibility to connect to a MAC through a
>> standard MII, RMII or RGMII interface.
>>
>> Datasheet:
>> http://www.ti.com/product/DP83822I/datasheet
>
> This looks pretty good, just a few nits below.
>
> [snip]
>
>> +static int dp83822_set_wol(struct phy_device *phydev,
>> + struct ethtool_wolinfo *wol)
>> +{
>> +struct net_device *ndev = phydev->attached_dev;
>> +u16 value;
>> +const u8 *mac;
>> +
>> +if (wol->wolopts & (WAKE_MAGIC | WAKE_MAGICSECURE)) {
>> +mac = (const u8 *)ndev->dev_addr;
>> +
>> +if (!is_valid_ether_addr(mac))
>> +return -EFAULT;
>
> -EINVAL maybe?
I referenced the at803x driver for the error code. I can change it if you want
it to be -EINVAL.
I can submit a patch to do the same for the other driver.
-EIVAL does make more sense.
>
>> +
>> +/* MAC addresses start with byte 5, but stored in mac[0].
>> + * 822 PHYs store bytes 4|5, 2|3, 0|1
>> + */
>> +phy_write_mmd(phydev, DP83822_DEVADDR,
>> + MII_DP83822_WOL_DA1, (mac[1] << 8) | mac[0]);
>> +phy_write_mmd(phydev, DP83822_DEVADDR,
>> + MII_DP83822_WOL_DA2, (mac[3] << 8) | mac[2]);
>> +phy_write_mmd(phydev, DP83822_DEVADDR, MII_DP83822_WOL_DA3,
>> + (mac[5] << 8) | mac[4]);
>> +
>> +value = phy_read_mmd(phydev, DP83822_DEVADDR,
>> + MII_DP83822_WOL_CFG);
>> +if (wol->wolopts & WAKE_MAGIC)
>> +value |= DP83822_WOL_MAGIC_EN;
>> +else
>> +value &= ~DP83822_WOL_MAGIC_EN;
>> +
>> +if (wol->wolopts & WAKE_MAGICSECURE) {
>> +value |= DP83822_WOL_SECURE_ON;
>
> Just in case any of the writes below fail, you would probably want to
> set this bit last, thus indicating that the password was successfully set.
Good point
>
>> +phy_write_mmd(phydev, DP83822_DEVADDR,
>> + MII_DP83822_RXSOP1,
>> + (wol->sopass[1] << 8) | wol->sopass[0]);
>> +phy_write_mmd(phydev, DP83822_DEVADDR,
>> + MII_DP83822_RXSOP2,
>> + (wol->sopass[3] << 8) | wol->sopass[2]);
>> +phy_write_mmd(phydev, DP83822_DEVADDR,
>> + MII_DP83822_RXSOP3,
>> + (wol->sopass[5] << 8) | wol->sopass[4]);
>
> In the else clause, you don't appear to be clearing the MagicPacket
> SecureOn password, but your get_wol function does not check for
> DP83822_WOL_SECURE_ON before returning the secure password, so either
> one of these two should be fixed. I would go with fixing the get_wol
> function see below.
OK
>
>> +} else {
>> +value &= ~DP83822_WOL_SECURE_ON;
>> +}
>> +
>> +value |= (DP83822_WOL_EN | DP83822_WOL_CLR_INDICATION |
>> + DP83822_WOL_CLR_INDICATION);
>
> The extra parenthesis should not be required here.
I did not code that in. I had to add it after Checkpatch cribbed about it.
Let me know if you want me to remove it.
>
>> +phy_write_mmd(phydev, DP83822_DEVADDR, MII_DP83822_WOL_CFG,
>> + value);
>> +} else {
>> +value =
>> +phy_read_mmd(phydev, DP83822_DEVADDR, MII_DP83822_WOL_CFG);
>> +value &= (~DP83822_WOL_EN);
>
> Same here, parenthesis should not be needed.
There are three lines of code in the else. This code all needs to be excuted
in the else case.
I might reformat it to read better. Lindent messed that one up.
>
>> +phy_write_mmd(phydev, DP83822_DEVADDR, MII_DP83822_WOL_CFG,
>> + value);
>> +}
>> +
>> +return 0;
>> +}
>> +
>> +static void dp83822_get_wol(struct phy_device *phydev,
>> +struct ethtool_wolinfo *wol)
>> +{
>> +int value;
>> +
>> +wol->supported = (WAKE_MAGIC | WAKE_MAGICSECURE);
Re: [PATCH] net: phy: DP83822 initial driver submission
All On 10/03/2017 10:53 AM, Dan Murphy wrote: > Add support for the TI DP83822 10/100Mbit ethernet phy. > > The DP83822 provides flexibility to connect to a MAC through a > standard MII, RMII or RGMII interface. > I need to submit an additional patch to remove DP83822 from the DP83848. The main difference in the driver is that this driver supports WoL and the DP83848 does not. So please kindly review this code and I can submit v2 with the DP83848 change Dan > Datasheet: > http://www.ti.com/product/DP83822I/datasheet > > Signed-off-by: Dan Murphy <dmur...@ti.com> > --- > drivers/net/phy/Kconfig | 5 + > drivers/net/phy/Makefile | 1 + > drivers/net/phy/dp83822.c | 313 > ++ > 3 files changed, 319 insertions(+) > create mode 100644 drivers/net/phy/dp83822.c > > diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig > index cd931cf..8e78a48 100644 > --- a/drivers/net/phy/Kconfig > +++ b/drivers/net/phy/Kconfig > @@ -277,6 +277,11 @@ config DAVICOM_PHY > ---help--- > Currently supports dm9161e and dm9131 > > +config DP83822_PHY > + tristate "Texas Instruments DP83822 PHY" > + ---help--- > + Supports the DP83822 PHY. > + > config DP83848_PHY > tristate "Texas Instruments DP83848 PHY" > ---help--- > diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile > index 416df92..df3b82b 100644 > --- a/drivers/net/phy/Makefile > +++ b/drivers/net/phy/Makefile > @@ -55,6 +55,7 @@ obj-$(CONFIG_CICADA_PHY)+= cicada.o > obj-$(CONFIG_CORTINA_PHY)+= cortina.o > obj-$(CONFIG_DAVICOM_PHY)+= davicom.o > obj-$(CONFIG_DP83640_PHY)+= dp83640.o > +obj-$(CONFIG_DP83822_PHY)+= dp83822.o > obj-$(CONFIG_DP83848_PHY)+= dp83848.o > obj-$(CONFIG_DP83867_PHY)+= dp83867.o > obj-$(CONFIG_FIXED_PHY) += fixed_phy.o > diff --git a/drivers/net/phy/dp83822.c b/drivers/net/phy/dp83822.c > new file mode 100644 > index 000..1d77515 > --- /dev/null > +++ b/drivers/net/phy/dp83822.c > @@ -0,0 +1,313 @@ > +/* > + * Driver for the Texas Instruments DP83822 PHY > + * > + * Copyright (C) 2017 Texas Instruments Inc. > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation; either version 2 of the License. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#define DP83822_PHY_ID 0x2000a240 > +#define DP83822_DEVADDR 0x1f > + > +#define MII_DP83822_MISR10x12 > +#define MII_DP83822_MISR20x13 > +#define MII_DP83822_RESET_CTRL 0x1f > + > +#define DP83822_HW_RESET BIT(15) > +#define DP83822_SW_RESET BIT(14) > + > +/* MISR1 bits */ > +#define DP83822_RX_ERR_HF_INT_EN BIT(0) > +#define DP83822_FALSE_CARRIER_HF_INT_EN BIT(1) > +#define DP83822_ANEG_COMPLETE_INT_EN BIT(2) > +#define DP83822_DUP_MODE_CHANGE_INT_EN BIT(3) > +#define DP83822_SPEED_CHANGED_INT_EN BIT(4) > +#define DP83822_LINK_STAT_INT_EN BIT(5) > +#define DP83822_ENERGY_DET_INT_ENBIT(6) > +#define DP83822_LINK_QUAL_INT_EN BIT(7) > + > +/* MISR2 bits */ > +#define DP83822_JABBER_DET_INT_ENBIT(0) > +#define DP83822_WOL_PKT_INT_EN BIT(1) > +#define DP83822_SLEEP_MODE_INT_ENBIT(2) > +#define DP83822_MDI_XOVER_INT_EN BIT(3) > +#define DP83822_LB_FIFO_INT_EN BIT(4) > +#define DP83822_PAGE_RX_INT_EN BIT(5) > +#define DP83822_ANEG_ERR_INT_EN BIT(6) > +#define DP83822_EEE_ERROR_CHANGE_INT_EN BIT(7) > + > +/* INT_STAT1 bits */ > +#define DP83822_WOL_INT_EN BIT(4) > +#define DP83822_WOL_INT_STAT BIT(12) > + > +#define MII_DP83822_RXSOP1 0x04A5 > +#define MII_DP83822_RXSOP2 0x04A6 > +#define MII_DP83822_RXSOP3 0x04A7 > + > +/* WoL Registers */ > +#define MII_DP83822_WOL_CFG 0x04A0 > +#define MII_DP83822_WOL_STAT0x04A1 > +#define MII_DP83822_WOL_DA1 0x04A2 > +#define MII_DP83822_WOL_DA2 0x04A3 > +#define MII_DP83822_WOL_DA3 0x04A4 > + > +/* WoL bits */ > +#define DP83822_WOL_MAGIC_EN BIT(1) > +#define DP83822_WOL_SECURE_ONBIT(5) > +#define DP83822_WOL_EN BIT(
[PATCH] net: phy: DP83822 initial driver submission
Add support for the TI DP83822 10/100Mbit ethernet phy.
The DP83822 provides flexibility to connect to a MAC through a
standard MII, RMII or RGMII interface.
Datasheet:
http://www.ti.com/product/DP83822I/datasheet
Signed-off-by: Dan Murphy <dmur...@ti.com>
---
drivers/net/phy/Kconfig | 5 +
drivers/net/phy/Makefile | 1 +
drivers/net/phy/dp83822.c | 313 ++
3 files changed, 319 insertions(+)
create mode 100644 drivers/net/phy/dp83822.c
diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index cd931cf..8e78a48 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -277,6 +277,11 @@ config DAVICOM_PHY
---help---
Currently supports dm9161e and dm9131
+config DP83822_PHY
+ tristate "Texas Instruments DP83822 PHY"
+ ---help---
+ Supports the DP83822 PHY.
+
config DP83848_PHY
tristate "Texas Instruments DP83848 PHY"
---help---
diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
index 416df92..df3b82b 100644
--- a/drivers/net/phy/Makefile
+++ b/drivers/net/phy/Makefile
@@ -55,6 +55,7 @@ obj-$(CONFIG_CICADA_PHY) += cicada.o
obj-$(CONFIG_CORTINA_PHY) += cortina.o
obj-$(CONFIG_DAVICOM_PHY) += davicom.o
obj-$(CONFIG_DP83640_PHY) += dp83640.o
+obj-$(CONFIG_DP83822_PHY) += dp83822.o
obj-$(CONFIG_DP83848_PHY) += dp83848.o
obj-$(CONFIG_DP83867_PHY) += dp83867.o
obj-$(CONFIG_FIXED_PHY)+= fixed_phy.o
diff --git a/drivers/net/phy/dp83822.c b/drivers/net/phy/dp83822.c
new file mode 100644
index 000..1d77515
--- /dev/null
+++ b/drivers/net/phy/dp83822.c
@@ -0,0 +1,313 @@
+/*
+ * Driver for the Texas Instruments DP83822 PHY
+ *
+ * Copyright (C) 2017 Texas Instruments Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define DP83822_PHY_ID 0x2000a240
+#define DP83822_DEVADDR0x1f
+
+#define MII_DP83822_MISR1 0x12
+#define MII_DP83822_MISR2 0x13
+#define MII_DP83822_RESET_CTRL 0x1f
+
+#define DP83822_HW_RESET BIT(15)
+#define DP83822_SW_RESET BIT(14)
+
+/* MISR1 bits */
+#define DP83822_RX_ERR_HF_INT_EN BIT(0)
+#define DP83822_FALSE_CARRIER_HF_INT_ENBIT(1)
+#define DP83822_ANEG_COMPLETE_INT_EN BIT(2)
+#define DP83822_DUP_MODE_CHANGE_INT_EN BIT(3)
+#define DP83822_SPEED_CHANGED_INT_EN BIT(4)
+#define DP83822_LINK_STAT_INT_EN BIT(5)
+#define DP83822_ENERGY_DET_INT_EN BIT(6)
+#define DP83822_LINK_QUAL_INT_EN BIT(7)
+
+/* MISR2 bits */
+#define DP83822_JABBER_DET_INT_EN BIT(0)
+#define DP83822_WOL_PKT_INT_EN BIT(1)
+#define DP83822_SLEEP_MODE_INT_EN BIT(2)
+#define DP83822_MDI_XOVER_INT_EN BIT(3)
+#define DP83822_LB_FIFO_INT_EN BIT(4)
+#define DP83822_PAGE_RX_INT_EN BIT(5)
+#define DP83822_ANEG_ERR_INT_ENBIT(6)
+#define DP83822_EEE_ERROR_CHANGE_INT_ENBIT(7)
+
+/* INT_STAT1 bits */
+#define DP83822_WOL_INT_EN BIT(4)
+#define DP83822_WOL_INT_STAT BIT(12)
+
+#define MII_DP83822_RXSOP1 0x04A5
+#defineMII_DP83822_RXSOP2 0x04A6
+#defineMII_DP83822_RXSOP3 0x04A7
+
+/* WoL Registers */
+#defineMII_DP83822_WOL_CFG 0x04A0
+#defineMII_DP83822_WOL_STAT0x04A1
+#defineMII_DP83822_WOL_DA1 0x04A2
+#defineMII_DP83822_WOL_DA2 0x04A3
+#defineMII_DP83822_WOL_DA3 0x04A4
+
+/* WoL bits */
+#define DP83822_WOL_MAGIC_EN BIT(1)
+#define DP83822_WOL_SECURE_ON BIT(5)
+#define DP83822_WOL_EN BIT(7)
+#define DP83822_WOL_INDICATION_SEL BIT(8)
+#define DP83822_WOL_CLR_INDICATION BIT(11)
+
+static int dp83822_ack_interrupt(struct phy_device *phydev)
+{
+ int err = phy_read(phydev, MII_DP83822_MISR1);
+
+ if (err < 0)
+ return err;
+
+ err = phy_read(phydev, MII_DP83822_MISR2);
+ if (err < 0)
+ return err;
+
+ return 0;
+}
+
+static int dp83822_set_wol(struct phy_device *phydev,
+ struct ethtool_wolinfo *wol)
+{
+ struct net_device *ndev = phydev->attached_dev;
+ u16 value;
+ const u8 *mac;
+
+ if (wol->wolopts & (WAKE_MAGIC | WAKE_MAGICSECURE)) {
+ mac = (const u8 *)ndev->dev_addr;
+
+ if (!is_valid_ether_addr(mac))
+ return -EFAULT;
+
+ /* MAC addresses start with byte 5, but
Re: [PATCH v2 2/2] phy dp83867: Make rgmii parameters optional
On 05/17/2016 03:37 PM, Alexander Graf wrote: > Hi David, > >> Am 17.05.2016 um 20:48 schrieb David Miller <da...@davemloft.net>: >> >> From: Dan Murphy <dmur...@ti.com> >> Date: Tue, 17 May 2016 13:34:34 -0500 >> >>> David >>> >>>> On 05/17/2016 01:22 PM, David Miller wrote: >>>> From: Alexander Graf <ag...@suse.de> >>>> Date: Mon, 16 May 2016 20:52:43 +0200 >>>> >>>>> If you compile without OF_MDIO support in an RGMII configuration, we fail >>>>> to configure the dp83867 phy today by writing garbage into its >>>>> configuration >>>>> registers. >>>>> >>>>> On the other hand if you do compile with OF_MDIO and the phy gets loaded >>>>> via >>>>> device tree, you have to have the properties set in the device tree, >>>>> otherwise >>>>> we fail to load the driver and don't even attach the generic phy driver to >>>>> the interface anymore. >>>>> >>>>> To make things slightly more consistent, make the rgmii configuration >>>>> properties >>>>> optional and allow a user to omit them in their device tree. >>>>> >>>>> Signed-off-by: Alexander Graf <ag...@suse.de> >>>> Applied. >>> This patch should not have been applied. >>> >>> I did not believe the implementation was proper for that driver. >>> >>> It seems my objection to the code was not seen. Nor was Andrew's point >>> about the DT bindings document >>> >>> https://patchwork.kernel.org/patch/9105371/ >> The discussions around the recent phy patches have been a labrynth that I've >> found hard to follow, sorry. >> >> I'll revert these two, sigh > The first patch is an obvious and correct fix. Discussions were only about > the second one (which I'm happy to drop given the rat hole this turned out to > be). So are you going to abandon the second patch all together? If you do, let me know I can submit a patch. Dan > > Alex > > -- -- Dan Murphy
Re: [PATCH v2 2/2] phy dp83867: Make rgmii parameters optional
David On 05/17/2016 01:22 PM, David Miller wrote: > From: Alexander Graf <ag...@suse.de> > Date: Mon, 16 May 2016 20:52:43 +0200 > >> If you compile without OF_MDIO support in an RGMII configuration, we fail >> to configure the dp83867 phy today by writing garbage into its configuration >> registers. >> >> On the other hand if you do compile with OF_MDIO and the phy gets loaded via >> device tree, you have to have the properties set in the device tree, >> otherwise >> we fail to load the driver and don't even attach the generic phy driver to >> the interface anymore. >> >> To make things slightly more consistent, make the rgmii configuration >> properties >> optional and allow a user to omit them in their device tree. >> >> Signed-off-by: Alexander Graf <ag...@suse.de> > Applied. This patch should not have been applied. I did not believe the implementation was proper for that driver. It seems my objection to the code was not seen. Nor was Andrew's point about the DT bindings document https://patchwork.kernel.org/patch/9105371/ Dan -- -- Dan Murphy
Re: [PATCH v2 2/2] phy dp83867: Make rgmii parameters optional
On 05/16/2016 01:52 PM, Alexander Graf wrote:
> If you compile without OF_MDIO support in an RGMII configuration, we fail
> to configure the dp83867 phy today by writing garbage into its configuration
> registers.
>
> On the other hand if you do compile with OF_MDIO and the phy gets loaded via
> device tree, you have to have the properties set in the device tree, otherwise
> we fail to load the driver and don't even attach the generic phy driver to
> the interface anymore.
>
> To make things slightly more consistent, make the rgmii configuration
> properties
> optional and allow a user to omit them in their device tree.
>
> Signed-off-by: Alexander Graf <ag...@suse.de>
> ---
> drivers/net/phy/dp83867.c | 31 ---
> 1 file changed, 28 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/phy/dp83867.c b/drivers/net/phy/dp83867.c
> index 94cc278..1b01680 100644
> --- a/drivers/net/phy/dp83867.c
> +++ b/drivers/net/phy/dp83867.c
> @@ -65,6 +65,7 @@ struct dp83867_private {
> int rx_id_delay;
> int tx_id_delay;
> int fifo_depth;
> + int values_are_sane;
> };
>
> static int dp83867_ack_interrupt(struct phy_device *phydev)
> @@ -113,15 +114,30 @@ static int dp83867_of_init(struct phy_device *phydev)
> ret = of_property_read_u32(of_node, "ti,rx-internal-delay",
> >rx_id_delay);
> if (ret)
> - return ret;
> + goto invalid_dt;
>
> ret = of_property_read_u32(of_node, "ti,tx-internal-delay",
> >tx_id_delay);
> if (ret)
> - return ret;
> + goto invalid_dt;
Optional means you may or may not have the entries
I would prefer to wrap the DT reading with the interface type check.
if (phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID ||
phydev->interface == PHY_INTERFACE_MODE_RGMII_ID )
ret = of_property_read_u32(of_node, "ti,tx-internal-delay",
>tx_id_delay);
if (ret)
goto invalid_dt;
Otherwise this continues to mandate that you need to declare all the DT entries
when in fact you may only have to declare 1.
And if the other interfaces are declared then DT entries are ignored. And
configuring
internal delay is not required per section 8.9 footnote 3 of the data sheet.
Dan
>
> - return of_property_read_u32(of_node, "ti,fifo-depth",
> + ret = of_property_read_u32(of_node, "ti,fifo-depth",
> >fifo_depth);
> + if (ret)
> + goto invalid_dt;
> +
> + dp83867->values_are_sane = 1;
> +
> + return 0;
> +
> +invalid_dt:
> + phydev_err(phydev, "missing properties in device tree");
> +
> + /*
> + * We can still run with a broken dt by not using any of the optional
> + * parameters, so just don't set dp83867->values_are_sane.
> + */
> + return 0;
> }
> #else
> static int dp83867_of_init(struct phy_device *phydev)
> @@ -150,6 +166,15 @@ static int dp83867_config_init(struct phy_device *phydev)
> dp83867 = (struct dp83867_private *)phydev->priv;
> }
>
> + /*
> + * With no or broken device tree, we don't have the values that we would
> + * want to configure the phy with. In that case, cross our fingers and
> + * assume that firmware did everything correctly for us or that we don't
> + * need them.
> + */
> + if (!dp83867->values_are_sane)
> + return 0;
> +
> if (phy_interface_is_rgmii(phydev)) {
> ret = phy_write(phydev, MII_DP83867_PHYCTRL,
> (dp83867->fifo_depth <<
> DP83867_PHYCR_FIFO_DEPTH_SHIFT));
--
--
Dan Murphy
Re: [PATCH] phy dp83867: depend on CONFIG_OF_MDIO
Alex
On 05/16/2016 12:57 PM, Alexander Graf wrote:
> Hi Dan,
>
> On 16.05.16 15:38, Dan Murphy wrote:
>> Alexander
>>
>> On 05/16/2016 06:28 AM, Alexander Graf wrote:
>>> The DP83867 phy driver doesn't actually work when CONFIG_OF_MDIO isn't
>>> enabled.
>>> It simply passes the device tree test, but leaves all internal configuration
>>> initialized at 0. Then it configures the phy with those values and renders a
>>> previously working configuration useless.
>>>
>>> This patch makes sure that we only build the DP83867 phy code when
>>> CONFIG_OF_MDIO is set, to not run into that problem.
>>>
>>> Signed-off-by: Alexander Graf <ag...@suse.de>
>>> ---
>>> drivers/net/phy/Kconfig | 1 +
>>> drivers/net/phy/dp83867.c | 7 ---
>>> 2 files changed, 1 insertion(+), 7 deletions(-)
>>>
>>> diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
>>> index 6dad9a9..4265ad5 100644
>>> --- a/drivers/net/phy/Kconfig
>>> +++ b/drivers/net/phy/Kconfig
>>> @@ -148,6 +148,7 @@ config DP83848_PHY
>>>
>>> config DP83867_PHY
>>> tristate "Drivers for Texas Instruments DP83867 Gigabit PHY"
>>> + depends on OF_MDIO
>>> ---help---
>>> Currently supports the DP83867 PHY.
>>>
>>> diff --git a/drivers/net/phy/dp83867.c b/drivers/net/phy/dp83867.c
>>> index 2afa61b..ff867ba 100644
>>> --- a/drivers/net/phy/dp83867.c
>>> +++ b/drivers/net/phy/dp83867.c
>>> @@ -99,7 +99,6 @@ static int dp83867_config_intr(struct phy_device *phydev)
>>> return phy_write(phydev, MII_DP83867_MICR, micr_status);
>>> }
>>>
>>> -#ifdef CONFIG_OF_MDIO
>>> static int dp83867_of_init(struct phy_device *phydev)
>>> {
>>> struct dp83867_private *dp83867 = phydev->priv;
>>> @@ -123,12 +122,6 @@ static int dp83867_of_init(struct phy_device *phydev)
>>> return of_property_read_u32(of_node, "ti,fifo-depth",
>>>>fifo_depth);
>>> }
>>> -#else
>>> -static int dp83867_of_init(struct phy_device *phydev)
>>> -{
>>> - return 0;
>>> -}
>>> -#endif /* CONFIG_OF_MDIO */
>>>
>>> static int dp83867_config_init(struct phy_device *phydev)
>>> {
>> I don't think we want this to depend solely on OF_MDIO.
>>
>> The #else case should probably be coded to look at platform data, if
>> it exists. I don't have any boards that still used platform data to test
>> this
>> out so I did not feel comfortable adding code I could not test.
> Since there was no code to look at platform data, those boards would be
> broken just as well today, no? So at the end of the day, this change
> should be no regression for them.
As Andrew pointed out if you are not using RGMII you don't need internal delay
or fifo_depth so making the driver dependent on OF_MDIO
does not make sense.
The DP83867 RGMII tx and rx delays and fifo should really be changed to
optional parameters and only programmed if set.
Dan
>
> Alex
--
--
Dan Murphy
Re: [PATCH] phy dp83867: depend on CONFIG_OF_MDIO
Alexander
On 05/16/2016 06:28 AM, Alexander Graf wrote:
> The DP83867 phy driver doesn't actually work when CONFIG_OF_MDIO isn't
> enabled.
> It simply passes the device tree test, but leaves all internal configuration
> initialized at 0. Then it configures the phy with those values and renders a
> previously working configuration useless.
>
> This patch makes sure that we only build the DP83867 phy code when
> CONFIG_OF_MDIO is set, to not run into that problem.
>
> Signed-off-by: Alexander Graf <ag...@suse.de>
> ---
> drivers/net/phy/Kconfig | 1 +
> drivers/net/phy/dp83867.c | 7 ---
> 2 files changed, 1 insertion(+), 7 deletions(-)
>
> diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
> index 6dad9a9..4265ad5 100644
> --- a/drivers/net/phy/Kconfig
> +++ b/drivers/net/phy/Kconfig
> @@ -148,6 +148,7 @@ config DP83848_PHY
>
> config DP83867_PHY
> tristate "Drivers for Texas Instruments DP83867 Gigabit PHY"
> + depends on OF_MDIO
> ---help---
> Currently supports the DP83867 PHY.
>
> diff --git a/drivers/net/phy/dp83867.c b/drivers/net/phy/dp83867.c
> index 2afa61b..ff867ba 100644
> --- a/drivers/net/phy/dp83867.c
> +++ b/drivers/net/phy/dp83867.c
> @@ -99,7 +99,6 @@ static int dp83867_config_intr(struct phy_device *phydev)
> return phy_write(phydev, MII_DP83867_MICR, micr_status);
> }
>
> -#ifdef CONFIG_OF_MDIO
> static int dp83867_of_init(struct phy_device *phydev)
> {
> struct dp83867_private *dp83867 = phydev->priv;
> @@ -123,12 +122,6 @@ static int dp83867_of_init(struct phy_device *phydev)
> return of_property_read_u32(of_node, "ti,fifo-depth",
> >fifo_depth);
> }
> -#else
> -static int dp83867_of_init(struct phy_device *phydev)
> -{
> - return 0;
> -}
> -#endif /* CONFIG_OF_MDIO */
>
> static int dp83867_config_init(struct phy_device *phydev)
> {
I don't think we want this to depend solely on OF_MDIO.
The #else case should probably be coded to look at platform data, if
it exists. I don't have any boards that still used platform data to test this
out so I did not feel comfortable adding code I could not test.
The platform data should contain the RGMII tx/rx delay and FIFO control.
Dan
--
--
Dan Murphy
Re: [PATCH] net: phy: dp83848: Add TI DP83848 Ethernet PHY
On 10/20/2015 04:28 PM, Andrew F. Davis wrote:
> Add support for the TI DP83848 Ethernet PHY device.
>
> The DP83848 is a highly reliable, feature rich, IEEE 802.3 compliant
> single port 10/100 Mb/s Ethernet Physical Layer Transceiver supporting
> the MII and RMII interfaces.
>
> Signed-off-by: Andrew F. Davis <a...@ti.com>
> Signed-off-by: Dan Murphy <dmur...@ti.com>
> ---
> drivers/net/phy/Kconfig | 5 +++
> drivers/net/phy/Makefile | 1 +
> drivers/net/phy/dp83848.c | 99
> +++
> 3 files changed, 105 insertions(+)
> create mode 100644 drivers/net/phy/dp83848.c
>
> diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
> index c5ad98a..12fa29a 100644
> --- a/drivers/net/phy/Kconfig
> +++ b/drivers/net/phy/Kconfig
> @@ -122,6 +122,11 @@ config MICREL_PHY
> ---help---
> Supports the KSZ9021, VSC8201, KS8001 PHYs.
>
> +config DP83848_PHY
> + tristate "Driver for Texas Instruments DP83848 PHY"
> + ---help---
> + Supports the DP83848 PHY.
> +
> config DP83867_PHY
> tristate "Drivers for Texas Instruments DP83867 Gigabit PHY"
> ---help---
> diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
> index 87f079c..b748224 100644
> --- a/drivers/net/phy/Makefile
> +++ b/drivers/net/phy/Makefile
> @@ -24,6 +24,7 @@ obj-$(CONFIG_MDIO_BITBANG) += mdio-bitbang.o
> obj-$(CONFIG_MDIO_GPIO) += mdio-gpio.o
> obj-$(CONFIG_NATIONAL_PHY) += national.o
> obj-$(CONFIG_DP83640_PHY)+= dp83640.o
> +obj-$(CONFIG_DP83848_PHY)+= dp83848.o
> obj-$(CONFIG_DP83867_PHY)+= dp83867.o
> obj-$(CONFIG_STE10XP)+= ste10Xp.o
> obj-$(CONFIG_MICREL_PHY) += micrel.o
> diff --git a/drivers/net/phy/dp83848.c b/drivers/net/phy/dp83848.c
> new file mode 100644
> index 000..5ce9bef
> --- /dev/null
> +++ b/drivers/net/phy/dp83848.c
> @@ -0,0 +1,99 @@
> +/*
> + * Driver for the Texas Instruments DP83848 PHY
> + *
> + * Copyright (C) 2015 Texas Instruments Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + */
> +
> +#include
> +#include
> +
> +#define DP83848_PHY_ID 0x20005c90
> +
> +/* Registers */
> +#define DP83848_MICR 0x11
> +#define DP83848_MISR 0x12
> +
> +/* MICR Register Fields */
> +#define DP83848_MICR_INT_OE BIT(0) /* Interrupt Output Enable */
> +#define DP83848_MICR_INTEN BIT(1) /* Interrupt Enable */
> +
> +/* MISR Register Fields */
> +#define DP83848_MISR_RHF_INT_EN BIT(0) /* Receive Error Counter
> */
> +#define DP83848_MISR_FHF_INT_EN BIT(1) /* False Carrier Counter
> */
> +#define DP83848_MISR_ANC_INT_EN BIT(2) /* Auto-negotiation
> complete */
> +#define DP83848_MISR_DUP_INT_EN BIT(3) /* Duplex Status */
> +#define DP83848_MISR_SPD_INT_EN BIT(4) /* Speed status */
> +#define DP83848_MISR_LINK_INT_EN BIT(5) /* Link status */
> +#define DP83848_MISR_ED_INT_EN BIT(6) /* Energy detect */
> +#define DP83848_MISR_LQM_INT_EN BIT(7) /* Link Quality Monitor
> */
> +
> +static int dp83848_ack_interrupt(struct phy_device *phydev)
> +{
> + int err = phy_read(phydev, DP83848_MISR);
> +
> + return err < 0 ? err : 0;
> +}
> +
> +static int dp83848_config_intr(struct phy_device *phydev)
> +{
> + int err;
> +
> + if (phydev->interrupts == PHY_INTERRUPT_ENABLED) {
> + err = phy_write(phydev, DP83848_MICR,
> + DP83848_MICR_INT_OE |
> + DP83848_MICR_INTEN);
> + if (err < 0)
> + return err;
> +
> + return phy_write(phydev, DP83848_MISR,
> + DP83848_MISR_ANC_INT_EN |
> + DP83848_MISR_DUP_INT_EN |
> + DP83848_MISR_SPD_INT_EN |
> + DP83848_MISR_LINK_INT_EN);
> + }
> +
> + return phy_write(phydev, DP83848_MICR, 0x0);
> +}
> +
> +static struct mdio_device_id __maybe_unused dp83848_tbl[] = {
> + { DP83848_PHY_ID, 0xfff0 },
[PATCH] net: phy: dp83867: Fix warning check for setting the internal delay
Fix warning: logical ‘or’ of collectively exhaustive tests is always true
Change the internal delay check from an 'or' condition to an 'and'
condition.
Reported-by: David Binderman dcb...@hotmail.com
Signed-off-by: Dan Murphy dmur...@ti.com
---
drivers/net/phy/dp83867.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/phy/dp83867.c b/drivers/net/phy/dp83867.c
index c7a12e2..8a3bf54 100644
--- a/drivers/net/phy/dp83867.c
+++ b/drivers/net/phy/dp83867.c
@@ -164,7 +164,7 @@ static int dp83867_config_init(struct phy_device *phydev)
return ret;
}
- if ((phydev-interface = PHY_INTERFACE_MODE_RGMII_ID) ||
+ if ((phydev-interface = PHY_INTERFACE_MODE_RGMII_ID)
(phydev-interface = PHY_INTERFACE_MODE_RGMII_RXID)) {
val = phy_read_mmd_indirect(phydev, DP83867_RGMIICTL,
DP83867_DEVADDR, phydev-addr);
--
1.9.1
--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: drivers/net/phy/dp83867.c:167: possible bad if ?
Florian
On 07/20/2015 12:28 PM, Florian Fainelli wrote:
Adding Dan,
On 20/07/15 05:37, David Binderman wrote:
Hello there,
drivers/net/phy/dp83867.c:167:57: warning: logical ‘or’ of collectively
exhaustive tests is always true [-Wlogical-op]
Source code is
if ((phydev-interface= PHY_INTERFACE_MODE_RGMII_ID) ||
(phydev-interface = PHY_INTERFACE_MODE_RGMII_RXID)) {
Maybe
if ((phydev-interface= PHY_INTERFACE_MODE_RGMII_ID)
(phydev-interface = PHY_INTERFACE_MODE_RGMII_RXID)) {
Sounds like the former is the intended comparison that will make sure
that phydev-interface is between MODE_RGMII_ID and MODE_RGMII_RXID, and
not below or after.
That is correct. The internal delay only needs to be set if this is declared
via the DT. There can be one of 3 interface internal delay (RGMII_ID), RX
internal delay (RGMII_RX_ID) or TX
internal delay (RGMII_TX_ID).
This internal delay is only applicable for RGMII and can be made specific to the
board.
The driver only needs to set the delay per the declaration in the DT.
Dan
--
--
Dan Murphy
--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: drivers/net/phy/dp83867.c:167: possible bad if ?
On 07/20/2015 12:40 PM, Florian Fainelli wrote:
On 20/07/15 10:37, Dan Murphy wrote:
Florian
On 07/20/2015 12:28 PM, Florian Fainelli wrote:
Adding Dan,
On 20/07/15 05:37, David Binderman wrote:
Hello there,
drivers/net/phy/dp83867.c:167:57: warning: logical ‘or’ of collectively
exhaustive tests is always true [-Wlogical-op]
Source code is
if ((phydev-interface= PHY_INTERFACE_MODE_RGMII_ID) ||
(phydev-interface = PHY_INTERFACE_MODE_RGMII_RXID)) {
Maybe
if ((phydev-interface= PHY_INTERFACE_MODE_RGMII_ID)
(phydev-interface = PHY_INTERFACE_MODE_RGMII_RXID)) {
Sounds like the former is the intended comparison that will make sure
that phydev-interface is between MODE_RGMII_ID and MODE_RGMII_RXID, and
not below or after.
That is correct. The internal delay only needs to be set if this is declared
via the DT. There can be one of 3 interface internal delay (RGMII_ID), RX
internal delay (RGMII_RX_ID) or TX
internal delay (RGMII_TX_ID).
Sorry, I meant to write the latter instead of the former here, the
current code seems to be potentially too permissive, is not it? In that
case, it seems to me like David's proposed fix is relevant.
The proposed fix is logically correct. And it matches the code for
phy_interface_is_rgmii with the ''.
Did you want me to patch and test?
Dan
This internal delay is only applicable for RGMII and can be made specific to
the
board.
The driver only needs to set the delay per the declaration in the DT.
Dan
--
--
Dan Murphy
--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] net: phy: dp83867: Fix device tree entries
Fix the device tree entries to modify the '_' to '-'.
Also changes the names of the internal delay properties
from -int- to -internal- as the -int- appeared as a keyword.
Signed-off-by: Dan Murphy dmur...@ti.com
---
v1 - Fixed device tree entries and added data sheet reference per
http://www.spinics.net/lists/netdev/msg331941.html
Documentation/devicetree/bindings/net/ti,dp83867.txt | 18 --
drivers/net/phy/dp83867.c| 6 +++---
2 files changed, 15 insertions(+), 9 deletions(-)
diff --git a/Documentation/devicetree/bindings/net/ti,dp83867.txt
b/Documentation/devicetree/bindings/net/ti,dp83867.txt
index 46bb67a..58d935b 100644
--- a/Documentation/devicetree/bindings/net/ti,dp83867.txt
+++ b/Documentation/devicetree/bindings/net/ti,dp83867.txt
@@ -2,18 +2,24 @@
Required properties:
- reg - The ID number for the phy, usually a small integer
- - ti,rx_int_delay - RGMII Recieve Clock Delay - see
dt-bindings/net/ti-dp83867.h
+ - ti,rx-internal-delay - RGMII Recieve Clock Delay - see
dt-bindings/net/ti-dp83867.h
for applicable values
- - ti,tx_int_delay - RGMII Transmit Clock Delay - see
dt-bindings/net/ti-dp83867.h
+ - ti,tx-internal-delay - RGMII Transmit Clock Delay - see
dt-bindings/net/ti-dp83867.h
for applicable values
- - ti,fifo_depth - Transmitt FIFO depth- see dt-bindings/net/ti-dp83867.h
+ - ti,fifo-depth - Transmitt FIFO depth- see dt-bindings/net/ti-dp83867.h
for applicable values
+Default child nodes are standard Ethernet PHY device
+nodes as described in Documentation/devicetree/bindings/net/phy.txt
+
Example:
ethernet-phy@0 {
reg = 0;
- ti,rx_int_delay = DP83867_RGMIIDCTL_2_25_NS;
- ti,tx_int_delay = DP83867_RGMIIDCTL_2_75_NS;
- ti,fifo_depth = DP83867_PHYCR_FIFO_DEPTH_4_B_NIB;
+ ti,rx-internal-delay = DP83867_RGMIIDCTL_2_25_NS;
+ ti,tx-internal-delay = DP83867_RGMIIDCTL_2_75_NS;
+ ti,fifo-depth = DP83867_PHYCR_FIFO_DEPTH_4_B_NIB;
};
+
+Datasheet can be found:
+http://www.ti.com/product/DP83867IR/datasheet
diff --git a/drivers/net/phy/dp83867.c b/drivers/net/phy/dp83867.c
index ef0b4eb..c7a12e2 100644
--- a/drivers/net/phy/dp83867.c
+++ b/drivers/net/phy/dp83867.c
@@ -113,17 +113,17 @@ static int dp83867_of_init(struct phy_device *phydev)
if (!phydev-dev.of_node)
return -ENODEV;
- ret = of_property_read_u32(of_node, ti,rx_int_delay,
+ ret = of_property_read_u32(of_node, ti,rx-internal-delay,
dp83867-rx_id_delay);
if (ret)
return ret;
- ret = of_property_read_u32(of_node, ti,tx_int_delay,
+ ret = of_property_read_u32(of_node, ti,tx-internal-delay,
dp83867-tx_id_delay);
if (ret)
return ret;
- ret = of_property_read_u32(of_node, ti,fifo_depth,
+ ret = of_property_read_u32(of_node, ti,fifo-depth,
dp83867-fifo_depth);
if (ret)
return ret;
--
1.9.1
--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] net: phy: dp83867: Add TI dp83867 phy
Florian
Thanks for the re-review
On 06/03/2015 09:47 PM, Florian Fainelli wrote:
Le 06/02/15 07:34, Dan Murphy a écrit :
Add support for the TI dp83867 Gigabit ethernet phy
device.
The DP83867 is a robust, low power, fully featured
Physical Layer transceiver with integrated PMD
sublayers to support 10BASE-T, 100BASE-TX and
1000BASE-T Ethernet protocols.
Sorry for the late feedback, since this is a new driver, things outline
below can still be submitted as incremental fixes.
[snip]
+Required properties:
+- reg - The ID number for the phy, usually a small integer
+- ti,rx_int_delay - RGMII Recieve Clock Delay - see
dt-bindings/net/ti-dp83867.h
+for applicable values
+- ti,tx_int_delay - RGMII Transmit Clock Delay - see
dt-bindings/net/ti-dp83867.h
+for applicable values
+- ti,fifo_depth - Transmitt FIFO depth- see dt-bindings/net/ti-dp83867.h
+for applicable values
We typically use - to separate words in DT properties, not _. I am
not sure about the required nature of these 3 proprietary/specific
properties, cannot there be good reset defaults from the HW in any case?
Yes you are correct I will modify as an incremental fix.
The hardware is defaulted to an internal delay of 2 ns. This value needs to be
adjusted
per product based on trace length. I would anticipate the DT properties to
grow a little as the
part gets used more.
You might want to add a reference to net/phy.txt for the remaiming DT
properties.
[snip]
+
+if (phy_interface_is_rgmii(phydev)) {
+ret = phy_write(phydev, MII_DP83867_PHYCTRL,
+(dp83867-fifo_depth
DP83867_PHYCR_FIFO_DEPTH_SHIFT));
+if (ret)
+return ret;
+}
+
+if ((phydev-interface = PHY_INTERFACE_MODE_RGMII_ID) ||
+(phydev-interface = PHY_INTERFACE_MODE_RGMII_RXID)) {
This one has not been converted to use the phy_interface_is_rgmii()
helper, but in fact, once you do that, you could probably just add an
early check for this condition, return, and reduce the indentation for
the normal case/RGMII, and eliminate two redundant condition checks.
This check is different then the generic RGMII check. I am checking to see if
the interface has the internal
delay flag set. The RGMII check is to broad. Maybe I can add a helper for this
if it makes sense thoughts? Maybe a phy_interface_has_rgmii_int_delay API just
to enter this check.
Dan
Thanks!
--
--
Dan Murphy
--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] net: phy: dp83867: Add TI dp83867 phy
Add support for the TI dp83867 Gigabit ethernet phy
device.
The DP83867 is a robust, low power, fully featured
Physical Layer transceiver with integrated PMD
sublayers to support 10BASE-T, 100BASE-TX and
1000BASE-T Ethernet protocols.
Signed-off-by: Dan Murphy dmur...@ti.com
---
v2 - Add device tree support and other minor changes. Added change for rgmii
helper
http://patchwork.ozlabs.org/patch/476446/
.../devicetree/bindings/net/ti,dp83867.txt | 19 ++
drivers/net/phy/Kconfig| 6 +-
drivers/net/phy/Makefile | 1 +
drivers/net/phy/dp83867.c | 239 +
include/dt-bindings/net/ti-dp83867.h | 45
5 files changed, 309 insertions(+), 1 deletion(-)
create mode 100644 Documentation/devicetree/bindings/net/ti,dp83867.txt
create mode 100644 drivers/net/phy/dp83867.c
create mode 100644 include/dt-bindings/net/ti-dp83867.h
diff --git a/Documentation/devicetree/bindings/net/ti,dp83867.txt
b/Documentation/devicetree/bindings/net/ti,dp83867.txt
new file mode 100644
index 000..46bb67a
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/ti,dp83867.txt
@@ -0,0 +1,19 @@
+* Texas Instruments - dp83867 Giga bit ethernet phy
+
+Required properties:
+ - reg - The ID number for the phy, usually a small integer
+ - ti,rx_int_delay - RGMII Recieve Clock Delay - see
dt-bindings/net/ti-dp83867.h
+ for applicable values
+ - ti,tx_int_delay - RGMII Transmit Clock Delay - see
dt-bindings/net/ti-dp83867.h
+ for applicable values
+ - ti,fifo_depth - Transmitt FIFO depth- see dt-bindings/net/ti-dp83867.h
+ for applicable values
+
+Example:
+
+ ethernet-phy@0 {
+ reg = 0;
+ ti,rx_int_delay = DP83867_RGMIIDCTL_2_25_NS;
+ ti,tx_int_delay = DP83867_RGMIIDCTL_2_75_NS;
+ ti,fifo_depth = DP83867_PHYCR_FIFO_DEPTH_4_B_NIB;
+ };
diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index 70641d2..939f06e 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -119,6 +119,11 @@ config MICREL_PHY
---help---
Supports the KSZ9021, VSC8201, KS8001 PHYs.
+config DP83867_PHY
+ tristate Drivers for Texas Instruments DP83867 Gigabit PHY
+ ---help---
+ Currently supports the DP83867 PHY.
+
config FIXED_PHY
tristate Driver for MDIO Bus/PHY emulation with fixed speed/link PHYs
depends on PHYLIB
@@ -212,7 +217,6 @@ config MDIO_BCM_UNIMAC
This hardware can be found in the Broadcom GENET Ethernet MAC
controllers as well as some Broadcom Ethernet switches such as the
Starfighter 2 switches.
-
endif # PHYLIB
config MICREL_KS8995MA
diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
index 501ea76..58e6131 100644
--- a/drivers/net/phy/Makefile
+++ b/drivers/net/phy/Makefile
@@ -22,6 +22,7 @@ obj-$(CONFIG_MDIO_BITBANG)+= mdio-bitbang.o
obj-$(CONFIG_MDIO_GPIO)+= mdio-gpio.o
obj-$(CONFIG_NATIONAL_PHY) += national.o
obj-$(CONFIG_DP83640_PHY) += dp83640.o
+obj-$(CONFIG_DP83867_PHY) += dp83867.o
obj-$(CONFIG_STE10XP) += ste10Xp.o
obj-$(CONFIG_MICREL_PHY) += micrel.o
obj-$(CONFIG_MDIO_OCTEON) += mdio-octeon.o
diff --git a/drivers/net/phy/dp83867.c b/drivers/net/phy/dp83867.c
new file mode 100644
index 000..ef0b4eb
--- /dev/null
+++ b/drivers/net/phy/dp83867.c
@@ -0,0 +1,239 @@
+/*
+ * Driver for the Texas Instruments DP83867 PHY
+ *
+ * Copyright (C) 2015 Texas Instruments Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include linux/ethtool.h
+#include linux/kernel.h
+#include linux/mii.h
+#include linux/module.h
+#include linux/of.h
+#include linux/phy.h
+
+#include dt-bindings/net/ti-dp83867.h
+
+#define DP83867_PHY_ID 0x2000a231
+#define DP83867_DEVADDR0x1f
+
+#define MII_DP83867_PHYCTRL0x10
+#define MII_DP83867_MICR 0x12
+#define MII_DP83867_ISR0x13
+#define DP83867_CTRL 0x1f
+
+/* Extended Registers */
+#define DP83867_RGMIICTL 0x0032
+#define DP83867_RGMIIDCTL 0x0086
+
+#define DP83867_SW_RESET BIT(15)
+#define DP83867_SW_RESTART BIT(14)
+
+/* MICR Interrupt bits */
+#define MII_DP83867_MICR_AN_ERR_INT_EN BIT(15)
+#define MII_DP83867_MICR_SPEED_CHNG_INT_EN BIT(14)
+#define MII_DP83867_MICR_DUP_MODE_CHNG_INT_EN BIT(13)
+#define MII_DP83867_MICR_PAGE_RXD_INT_EN BIT(12
[PATCH] net: phy: dp83867: Add TI dp83867 phy
Add support for the TI dp83867 Gigabit ethernet phy
device.
The DP83867 is a robust, low power, fully featured
Physical Layer transceiver with integrated PMD
sublayers to support 10BASE-T, 100BASE-TX and
1000BASE-T Ethernet protocols.
Signed-off-by: Dan Murphy dmur...@ti.com
---
drivers/net/phy/Kconfig | 6 +-
drivers/net/phy/Makefile | 1 +
drivers/net/phy/dp83867.c | 208 ++
3 files changed, 214 insertions(+), 1 deletion(-)
create mode 100644 drivers/net/phy/dp83867.c
diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index 70641d2..939f06e 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -119,6 +119,11 @@ config MICREL_PHY
---help---
Supports the KSZ9021, VSC8201, KS8001 PHYs.
+config DP83867_PHY
+ tristate Drivers for Texas Instruments DP83867 Gigabit PHY
+ ---help---
+ Currently supports the DP83867 PHY.
+
config FIXED_PHY
tristate Driver for MDIO Bus/PHY emulation with fixed speed/link PHYs
depends on PHYLIB
@@ -212,7 +217,6 @@ config MDIO_BCM_UNIMAC
This hardware can be found in the Broadcom GENET Ethernet MAC
controllers as well as some Broadcom Ethernet switches such as the
Starfighter 2 switches.
-
endif # PHYLIB
config MICREL_KS8995MA
diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
index 501ea76..58e6131 100644
--- a/drivers/net/phy/Makefile
+++ b/drivers/net/phy/Makefile
@@ -22,6 +22,7 @@ obj-$(CONFIG_MDIO_BITBANG)+= mdio-bitbang.o
obj-$(CONFIG_MDIO_GPIO)+= mdio-gpio.o
obj-$(CONFIG_NATIONAL_PHY) += national.o
obj-$(CONFIG_DP83640_PHY) += dp83640.o
+obj-$(CONFIG_DP83867_PHY) += dp83867.o
obj-$(CONFIG_STE10XP) += ste10Xp.o
obj-$(CONFIG_MICREL_PHY) += micrel.o
obj-$(CONFIG_MDIO_OCTEON) += mdio-octeon.o
diff --git a/drivers/net/phy/dp83867.c b/drivers/net/phy/dp83867.c
new file mode 100644
index 000..cc8466b
--- /dev/null
+++ b/drivers/net/phy/dp83867.c
@@ -0,0 +1,208 @@
+/*
+ * Driver for the Texas Instruments DP83867 PHY
+ *
+ * Copyright (C) 2015 Texas Instruments Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include linux/ethtool.h
+#include linux/kernel.h
+#include linux/mii.h
+#include linux/module.h
+#include linux/phy.h
+
+#define DP83867_PHY_ID 0x2000a231
+#define DP83867_DEVADDR0x1f
+
+#define MII_DP83867_PHYCTRL0x10
+#define MII_DP83867_MICR 0x12
+#define MII_DP83867_ISR0x13
+#define DP83867_CTRL 0x1f
+
+/* Extended Registers */
+#define DP83867_RGMIICTL 0x0032
+#define DP83867_RGMIIDCTL 0x0086
+
+#define DP83867_SW_RESET BIT(15)
+#define DP83867_SW_RESTART BIT(14)
+
+/* MICR Interrupt bits */
+#define MII_DP83867_MICR_AN_ERR_INT_EN BIT(15)
+#define MII_DP83867_MICR_SPEED_CHNG_INT_EN BIT(14)
+#define MII_DP83867_MICR_DUP_MODE_CHNG_INT_EN BIT(13)
+#define MII_DP83867_MICR_PAGE_RXD_INT_EN BIT(12)
+#define MII_DP83867_MICR_AUTONEG_COMP_INT_EN BIT(11)
+#define MII_DP83867_MICR_LINK_STS_CHNG_INT_EN BIT(10)
+#define MII_DP83867_MICR_FALSE_CARRIER_INT_EN BIT(8)
+#define MII_DP83867_MICR_SLEEP_MODE_CHNG_INT_ENBIT(4)
+#define MII_DP83867_MICR_WOL_INT_ENBIT(3)
+#define MII_DP83867_MICR_XGMII_ERR_INT_EN BIT(2)
+#define MII_DP83867_MICR_POL_CHNG_INT_EN BIT(1)
+#define MII_DP83867_MICR_JABBER_INT_EN BIT(0)
+
+/* RGMIICTL bits */
+#define DP83867_RGMII_TX_CLK_DELAY_EN BIT(1)
+#define DP83867_RGMII_RX_CLK_DELAY_EN BIT(0)
+
+/* PHY CTRL bits */
+#define DP83867_PHYCR_FIFO_DEPTH_SHIFT 14
+#define DP83867_PHYCR_FIFO_DEPTH_3_B_NIB 0x00
+#define DP83867_PHYCR_FIFO_DEPTH_4_B_NIB 0x01
+#define DP83867_PHYCR_FIFO_DEPTH_6_B_NIB 0x02
+#define DP83867_PHYCR_FIFO_DEPTH_8_B_NIB 0x03
+
+/* RGMIIDCTL bits */
+#define DP83867_RGMII_RX_CLK_DELAY_SHIFT 4
+enum {
+ DP83867_RGMIIDCTL_250_PS,
+ DP83867_RGMIIDCTL_500_PS,
+ DP83867_RGMIIDCTL_750_PS,
+ DP83867_RGMIIDCTL_1_NS,
+ DP83867_RGMIIDCTL_1_25_NS,
+ DP83867_RGMIIDCTL_1_50_NS,
+ DP83867_RGMIIDCTL_1_75_NS,
+ DP83867_RGMIIDCTL_2_00_NS,
+ DP83867_RGMIIDCTL_2_25_NS,
+ DP83867_RGMIIDCTL_2_50_NS,
+ DP83867_RGMIIDCTL_2_75_NS,
+ DP83867_RGMIIDCTL_3_00_NS,
+ DP83867_RGMIIDCTL_3_25_NS,
+ DP83867_RGMIIDCTL_3_50_NS,
+ DP83867_RGMIIDCTL_3_75_NS,
+ DP83867_RGMIIDCTL_4_00_NS,
+};
+
+int rx_tx_delay
Re: [PATCH] net: phy: dp83867: Add TI dp83867 phy
Florian
Thanks for the review!
On 05/26/2015 01:04 PM, Florian Fainelli wrote:
On 26/05/15 06:07, Dan Murphy wrote:
Add support for the TI dp83867 Gigabit ethernet phy
device.
The DP83867 is a robust, low power, fully featured
Physical Layer transceiver with integrated PMD
sublayers to support 10BASE-T, 100BASE-TX and
1000BASE-T Ethernet protocols.
Signed-off-by: Dan Murphy dmur...@ti.com
---
[snip]
+
+int rx_tx_delay = (DP83867_RGMIIDCTL_2_75_NS
DP83867_RGMII_RX_CLK_DELAY_SHIFT) | DP83867_RGMIIDCTL_2_25_NS;
+module_param(rx_tx_delay, int, 0664);
This is not going to work, rx and tx delays are inherent properties of
PCB/board designs, you want to be able to get that value from your
platform configuration, Device Tree would certainly be preferred here.
Asking an user to figure this out through module parameters is going to
be both error prone, and limiting yourself to no more than one instance.
Yeah I agree. I also have platform data for legacy products that I could put
in the
DT as well.
[snip]
+
+static int dp83867_phy_reset(struct phy_device *phydev)
+{
+int err;
+
+err = phy_write(phydev, DP83867_CTRL, DP83867_SW_RESET);
+if (err 0)
+return err;
+
+err = dp83867_config_init(phydev);
+return err;
you could do a tail-call return directly?
Yep. Will change that
[snip]
+
+static int __init dp83867_init(void)
+{
+return phy_driver_register(dp83867_driver);
+}
+
+static void __exit dp83867_exit(void)
+{
+phy_driver_unregister(dp83867_driver);
+}
+
+module_init(dp83867_init);
+module_exit(dp83867_exit);
You could use module_phy_driver to eliminate some boilerplate here.
Nice I will do that.
Dan
--
--
Dan Murphy
--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
