Re: [PATCH v2 01/11] net: phy: Add rockchip phy driver support

[David.Wu]

Hi Florian,

在 2017/7/28 0:51, Florian Fainelli 写道:

+}
+
+static int rockchip_internal_phy_config_init(struct phy_device *phydev)
+{
+   int val;
+
+   /*
+* The auto MIDX has linked problem on some board,
+* workround to disable auto MDIX.
+*/

If this a board-specific problem you may consider register a PHY fixup
for the affected boards, or introduce a specific property to illustrate
that MDI-X is broken.


You mean is that if I need disable Auto-mdix, then I'm going to add a 
property to DTS, from here to decide whether or not to close Auto-mdix?

Re: [PATCH v2 01/11] net: phy: Add rockchip phy driver support

[David.Wu]

Hi Andrew,

在 2017/7/27 21:38, Andrew Lunn 写道:

+   /*
+* If mode switch happens from 10BT to 100BT, all DSP/AFE
+* registers are set to default values. So any AFE/DSP
+* registers have to be re-initialized in this case.
+*/


Hi David

Are they also lost on suspend and resume?


Yes, i will add the re-initialized on suspend and resume，and thank you！



 Andrew

Re: [PATCH v2 01/11] net: phy: Add rockchip phy driver support

[Florian Fainelli]

On 07/27/2017 05:55 AM, David Wu wrote:
> Support internal ephy currently.
> 
> Signed-off-by: David Wu 
> ---
> changes in v2:
>  - Alphabetic order for Kconfig and Makefile.
>  - Add analog register init.
>  - Disable auto-mdix for workround.
>  - Rename config
> 
>  drivers/net/phy/Kconfig|   5 ++
>  drivers/net/phy/Makefile   |   1 +
>  drivers/net/phy/rockchip.c | 128 
> +
>  3 files changed, 134 insertions(+)
>  create mode 100644 drivers/net/phy/rockchip.c
> 
> diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
> index 2dda720..8dc6cd7 100644
> --- a/drivers/net/phy/Kconfig
> +++ b/drivers/net/phy/Kconfig
> @@ -334,6 +334,11 @@ config REALTEK_PHY
>   ---help---
> Supports the Realtek 821x PHY.
>  
> +config ROCKCHIP_PHY
> +tristate "Drivers for ROCKCHIP PHYs"

"Driver for Rockchip Ethernet PHYs" would seem more appropriate, this is
just one driver for now.

> +---help---
> +  Currently supports the internal ephy.

EPHY is usually how an Ethernet PHY is abbreviated.

> +
>  config SMSC_PHY
>   tristate "SMSC PHYs"
>   ---help---
> diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
> index 8e9b9f3..350520e 100644
> --- a/drivers/net/phy/Makefile
> +++ b/drivers/net/phy/Makefile
> @@ -66,6 +66,7 @@ obj-$(CONFIG_MICROSEMI_PHY) += mscc.o
>  obj-$(CONFIG_NATIONAL_PHY)   += national.o
>  obj-$(CONFIG_QSEMI_PHY)  += qsemi.o
>  obj-$(CONFIG_REALTEK_PHY)+= realtek.o
> +obj-$(CONFIG_ROCKCHIP_PHY)   += rockchip.o
>  obj-$(CONFIG_SMSC_PHY)   += smsc.o
>  obj-$(CONFIG_STE10XP)+= ste10Xp.o
>  obj-$(CONFIG_TERANETICS_PHY) += teranetics.o
> diff --git a/drivers/net/phy/rockchip.c b/drivers/net/phy/rockchip.c
> new file mode 100644
> index 000..3f74658
> --- /dev/null
> +++ b/drivers/net/phy/rockchip.c
> @@ -0,0 +1,128 @@
> +/**
> + * drivers/net/phy/rockchip.c
> + *
> + * Driver for ROCKCHIP PHY
> + *
> + * Copyright (c) 2017, Fuzhou Rockchip Electronics Co., Ltd
> + *
> + * David Wu

Missing space between your last name and your email address, there is
another typo like this in the MODULE_AUTHOR() macro.

> + *
> + * 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, or
> + * (at your option) any later version.
> + *
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#define MII_INTERNAL_CTRL_STATUS 17
> +#define SMI_ADDR_TSTCNTL 20
> +#define SMI_ADDR_TSTREAD121
> +#define SMI_ADDR_TSTREAD222
> +#define SMI_ADDR_TSTWRITE23
> +
> +#define AUTOMDIX_EN  BIT(7)
> +#define TSTCNTL_RD   (BIT(15) | BIT(10))
> +#define TSTCNTL_WR   (BIT(14) | BIT(10))
> +
> +#define WR_ADDR_A7CFG0x18
> +
> +static void rockchip_init_tstmode(struct phy_device *phydev)
> +{
> + /* Enable access to Analog and DSP register banks */
> + phy_write(phydev, SMI_ADDR_TSTCNTL, 0x0400);
> + phy_write(phydev, SMI_ADDR_TSTCNTL, 0x);
> + phy_write(phydev, SMI_ADDR_TSTCNTL, 0x0400);
> +}
> +
> +static void  rockchip_close_tstmode(struct phy_device *phydev)
> +{
> + /* Back to basic register bank */
> + phy_write(phydev, SMI_ADDR_TSTCNTL, 0x);
> +}
> +
> +static void rockchip_internal_phy_analog_init(struct phy_device *phydev)
> +{
> + rockchip_init_tstmode(phydev);

Technically MDIO writes can fail, but you are not propagating the return
value, so you could be stuck on a bad page/bank.

> +
> + /*
> +  * Adjust tx amplitude to make sginal better,
> +  * the default value is 0x8.
> +  */
> + phy_write(phydev, SMI_ADDR_TSTWRITE, 0xB);
> + phy_write(phydev, SMI_ADDR_TSTCNTL, TSTCNTL_WR | WR_ADDR_A7CFG);

Likewise.

> +
> + rockchip_close_tstmode(phydev);

Same here.

> +}
> +
> +static int rockchip_internal_phy_config_init(struct phy_device *phydev)
> +{
> + int val;
> +
> + /*
> +  * The auto MIDX has linked problem on some board,
> +  * workround to disable auto MDIX.
> +  */

If this a board-specific problem you may consider register a PHY fixup
for the affected boards, or introduce a specific property to illustrate
that MDI-X is broken.

> + val = phy_read(phydev, MII_INTERNAL_CTRL_STATUS);
> + val &= ~AUTOMDIX_EN;
> + phy_write(phydev, MII_INTERNAL_CTRL_STATUS, val);

You also need to reject MDI configuration requests coming via
phy_ethtool_ksettings_set() which you should see in the
phyddrv::config_ane callback.

> +
> + rockchip_internal_phy_analog_init(phydev);
> +
> + return 0;
> +}
> +
> +static int rockchip_internal_phy_read_status(struct phy_device *phydev)
> +{
> + int ret, old_speed;
> +
> + 

Re: [PATCH v2 01/11] net: phy: Add rockchip phy driver support

[Andrew Lunn]

> + /*
> +  * If mode switch happens from 10BT to 100BT, all DSP/AFE
> +  * registers are set to default values. So any AFE/DSP
> +  * registers have to be re-initialized in this case.
> +  */

Hi David

Are they also lost on suspend and resume?

Andrew

[PATCH v2 01/11] net: phy: Add rockchip phy driver support

[David Wu]

Support internal ephy currently.

Signed-off-by: David Wu 
---
changes in v2:
 - Alphabetic order for Kconfig and Makefile.
 - Add analog register init.
 - Disable auto-mdix for workround.
 - Rename config

 drivers/net/phy/Kconfig|   5 ++
 drivers/net/phy/Makefile   |   1 +
 drivers/net/phy/rockchip.c | 128 +
 3 files changed, 134 insertions(+)
 create mode 100644 drivers/net/phy/rockchip.c

diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index 2dda720..8dc6cd7 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -334,6 +334,11 @@ config REALTEK_PHY
---help---
  Supports the Realtek 821x PHY.
 
+config ROCKCHIP_PHY
+tristate "Drivers for ROCKCHIP PHYs"
+---help---
+  Currently supports the internal ephy.
+
 config SMSC_PHY
tristate "SMSC PHYs"
---help---
diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
index 8e9b9f3..350520e 100644
--- a/drivers/net/phy/Makefile
+++ b/drivers/net/phy/Makefile
@@ -66,6 +66,7 @@ obj-$(CONFIG_MICROSEMI_PHY)   += mscc.o
 obj-$(CONFIG_NATIONAL_PHY) += national.o
 obj-$(CONFIG_QSEMI_PHY)+= qsemi.o
 obj-$(CONFIG_REALTEK_PHY)  += realtek.o
+obj-$(CONFIG_ROCKCHIP_PHY) += rockchip.o
 obj-$(CONFIG_SMSC_PHY) += smsc.o
 obj-$(CONFIG_STE10XP)  += ste10Xp.o
 obj-$(CONFIG_TERANETICS_PHY)   += teranetics.o
diff --git a/drivers/net/phy/rockchip.c b/drivers/net/phy/rockchip.c
new file mode 100644
index 000..3f74658
--- /dev/null
+++ b/drivers/net/phy/rockchip.c
@@ -0,0 +1,128 @@
+/**
+ * drivers/net/phy/rockchip.c
+ *
+ * Driver for ROCKCHIP PHY
+ *
+ * Copyright (c) 2017, Fuzhou Rockchip Electronics Co., Ltd
+ *
+ * David Wu
+ *
+ * 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, or
+ * (at your option) any later version.
+ *
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define MII_INTERNAL_CTRL_STATUS   17
+#define SMI_ADDR_TSTCNTL   20
+#define SMI_ADDR_TSTREAD1  21
+#define SMI_ADDR_TSTREAD2  22
+#define SMI_ADDR_TSTWRITE  23
+
+#define AUTOMDIX_ENBIT(7)
+#define TSTCNTL_RD (BIT(15) | BIT(10))
+#define TSTCNTL_WR (BIT(14) | BIT(10))
+
+#define WR_ADDR_A7CFG  0x18
+
+static void rockchip_init_tstmode(struct phy_device *phydev)
+{
+   /* Enable access to Analog and DSP register banks */
+   phy_write(phydev, SMI_ADDR_TSTCNTL, 0x0400);
+   phy_write(phydev, SMI_ADDR_TSTCNTL, 0x);
+   phy_write(phydev, SMI_ADDR_TSTCNTL, 0x0400);
+}
+
+static void  rockchip_close_tstmode(struct phy_device *phydev)
+{
+   /* Back to basic register bank */
+   phy_write(phydev, SMI_ADDR_TSTCNTL, 0x);
+}
+
+static void rockchip_internal_phy_analog_init(struct phy_device *phydev)
+{
+   rockchip_init_tstmode(phydev);
+
+   /*
+* Adjust tx amplitude to make sginal better,
+* the default value is 0x8.
+*/
+   phy_write(phydev, SMI_ADDR_TSTWRITE, 0xB);
+   phy_write(phydev, SMI_ADDR_TSTCNTL, TSTCNTL_WR | WR_ADDR_A7CFG);
+
+   rockchip_close_tstmode(phydev);
+}
+
+static int rockchip_internal_phy_config_init(struct phy_device *phydev)
+{
+   int val;
+
+   /*
+* The auto MIDX has linked problem on some board,
+* workround to disable auto MDIX.
+*/
+   val = phy_read(phydev, MII_INTERNAL_CTRL_STATUS);
+   val &= ~AUTOMDIX_EN;
+   phy_write(phydev, MII_INTERNAL_CTRL_STATUS, val);
+
+   rockchip_internal_phy_analog_init(phydev);
+
+   return 0;
+}
+
+static int rockchip_internal_phy_read_status(struct phy_device *phydev)
+{
+   int ret, old_speed;
+
+   old_speed = phydev->speed;
+   ret = genphy_read_status(phydev);
+   if (ret)
+   return ret;
+
+   /*
+* If mode switch happens from 10BT to 100BT, all DSP/AFE
+* registers are set to default values. So any AFE/DSP
+* registers have to be re-initialized in this case.
+*/
+   if ((old_speed == SPEED_10) && (phydev->speed == SPEED_100))
+   rockchip_internal_phy_analog_init(phydev);
+
+   return ret;
+}
+
+static struct phy_driver rockchip_phy_driver[] = {
+{
+   .phy_id = 0x1234d400,
+   .phy_id_mask= 0x,
+   .name   = "rockchip internal ephy",
+   .features   = (PHY_BASIC_FEATURES | SUPPORTED_Pause
+ | SUPPORTED_Asym_Pause),
+   .soft_reset = genphy_soft_reset,
+   .config_init= rockchip_internal_phy_config_init,
+   .config_aneg= genphy_config_aneg,
+   .read_status=