* [PATCH 2/4] phylib: Add generic 10G driver
@ 2013-11-11 11:07 shh.xie
2013-11-12 12:31 ` Shaohui Xie
2013-11-12 17:53 ` Florian Fainelli
0 siblings, 2 replies; 4+ messages in thread
From: shh.xie @ 2013-11-11 11:07 UTC (permalink / raw)
To: linuxppc-dev, linux-kernel; +Cc: madalin.bucur, Shruti, Shaohui Xie
From: Andy Fleming
Very incomplete, but will allow for binding an ethernet controller
to it.
Also, Add XGMII interface type
Signed-off-by: Andy Fleming
Signed-off-by: Shaohui Xie <Shaohui.Xie@freescale.com>
---
drivers/net/phy/phy_device.c | 101 ++++++++++++++++++++++++++++++++++++++++++-
include/linux/phy.h | 1 +
2 files changed, 101 insertions(+), 1 deletion(-)
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 74630e9..30bf2d5 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -32,6 +32,7 @@
#include <linux/module.h>
#include <linux/mii.h>
#include <linux/ethtool.h>
+#include <linux/mdio.h>
#include <linux/phy.h>
#include <asm/io.h>
@@ -689,6 +690,13 @@ static int genphy_config_advert(struct phy_device *phydev)
return changed;
}
+int gen10g_config_advert(struct phy_device *dev)
+{
+ return 0;
+}
+EXPORT_SYMBOL(gen10g_config_advert);
+
+
/**
* genphy_setup_forced - configures/forces speed/duplex from @phydev
* @phydev: target phy_device struct
@@ -742,6 +750,12 @@ int genphy_restart_aneg(struct phy_device *phydev)
}
EXPORT_SYMBOL(genphy_restart_aneg);
+int gen10g_restart_aneg(struct phy_device *phydev)
+{
+ return 0;
+}
+EXPORT_SYMBOL(gen10g_restart_aneg);
+
/**
* genphy_config_aneg - restart auto-negotiation or write BMCR
@@ -784,6 +798,13 @@ int genphy_config_aneg(struct phy_device *phydev)
}
EXPORT_SYMBOL(genphy_config_aneg);
+int gen10g_config_aneg(struct phy_device *phydev)
+{
+ return 0;
+}
+EXPORT_SYMBOL(gen10g_config_aneg);
+
+
/**
* genphy_update_link - update link status in @phydev
* @phydev: target phy_device struct
@@ -913,6 +934,35 @@ int genphy_read_status(struct phy_device *phydev)
}
EXPORT_SYMBOL(genphy_read_status);
+int gen10g_read_status(struct phy_device *phydev)
+{
+ int devad, reg;
+ u32 mmd_mask = phydev->c45_ids.devices_in_package;
+
+ phydev->link = 1;
+
+ /* For now just lie and say it's 10G all the time */
+ phydev->speed = 10000;
+ phydev->duplex = DUPLEX_FULL;
+
+ for (devad = 0; mmd_mask; devad++, mmd_mask = mmd_mask >> 1) {
+ if (!(mmd_mask & 1))
+ continue;
+
+ /* Read twice because link state is latched and a
+ * read moves the current state into the register
+ */
+ phy_read_mmd(phydev, devad, MDIO_STAT1);
+ reg = phy_read_mmd(phydev, devad, MDIO_STAT1);
+ if (reg < 0 || !(reg & MDIO_STAT1_LSTATUS))
+ phydev->link = 0;
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL(gen10g_read_status);
+
+
static int genphy_config_init(struct phy_device *phydev)
{
int val;
@@ -959,6 +1009,15 @@ static int genphy_config_init(struct phy_device *phydev)
return 0;
}
+
+static int gen10g_config_init(struct phy_device *phydev)
+{
+ /* Temporarily just say we support everything */
+ phydev->supported = phydev->advertising = SUPPORTED_10000baseT_Full;
+
+ return 0;
+}
+
int genphy_suspend(struct phy_device *phydev)
{
int value;
@@ -974,6 +1033,13 @@ int genphy_suspend(struct phy_device *phydev)
}
EXPORT_SYMBOL(genphy_suspend);
+int gen10g_suspend(struct phy_device *phydev)
+{
+ return 0;
+}
+EXPORT_SYMBOL(gen10g_suspend);
+
+
int genphy_resume(struct phy_device *phydev)
{
int value;
@@ -989,6 +1055,13 @@ int genphy_resume(struct phy_device *phydev)
}
EXPORT_SYMBOL(genphy_resume);
+int gen10g_resume(struct phy_device *phydev)
+{
+ return 0;
+}
+EXPORT_SYMBOL(gen10g_resume);
+
+
/**
* phy_probe - probe and init a PHY device
* @dev: device to probe and init
@@ -1129,6 +1202,20 @@ static struct phy_driver genphy_driver = {
.driver = {.owner= THIS_MODULE, },
};
+static struct phy_driver gen10g_driver = {
+ .phy_id = 0xffffffff,
+ .phy_id_mask = 0xffffffff,
+ .name = "Generic 10G PHY",
+ .config_init = gen10g_config_init,
+ .features = 0,
+ .config_aneg = gen10g_config_aneg,
+ .read_status = gen10g_read_status,
+ .suspend = gen10g_suspend,
+ .resume = gen10g_resume,
+ .driver = {.owner = THIS_MODULE, },
+};
+
+
static int __init phy_init(void)
{
int rc;
@@ -1139,13 +1226,25 @@ static int __init phy_init(void)
rc = phy_driver_register(&genphy_driver);
if (rc)
- mdio_bus_exit();
+ goto genphy_register_failed;
+
+ rc = phy_driver_register(&gen10g_driver);
+ if (rc)
+ goto gen10g_register_failed;
+
+ return rc;
+
+gen10g_register_failed:
+ phy_driver_unregister(&genphy_driver);
+genphy_register_failed:
+ mdio_bus_exit();
return rc;
}
static void __exit phy_exit(void)
{
+ phy_driver_unregister(&gen10g_driver);
phy_driver_unregister(&genphy_driver);
mdio_bus_exit();
}
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 684925a..f864004 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -66,6 +66,7 @@ typedef enum {
PHY_INTERFACE_MODE_RGMII_TXID,
PHY_INTERFACE_MODE_RTBI,
PHY_INTERFACE_MODE_SMII,
+ PHY_INTERFACE_MODE_XGMII,
} phy_interface_t;
--
1.8.4.1
^ permalink raw reply [flat|nested] 4+ messages in thread* RE: [PATCH 2/4] phylib: Add generic 10G driver
2013-11-11 11:07 [PATCH 2/4] phylib: Add generic 10G driver shh.xie
@ 2013-11-12 12:31 ` Shaohui Xie
2013-11-12 17:53 ` Florian Fainelli
1 sibling, 0 replies; 4+ messages in thread
From: Shaohui Xie @ 2013-11-12 12:31 UTC (permalink / raw)
To: shh.xie, linuxppc-dev, linux-kernel
Cc: Madalin-Cristian Bucur, Shruti Kanetkar, davem, jg1.han,
f.fainelli, peppe.cavallaro, michal.simek, netdev
Added more people and list.
Best Regards,
Shaohui Xie
> -----Original Message-----
> From: shh.xie@gmail.com [mailto:shh.xie@gmail.com]
> Sent: Monday, November 11, 2013 7:07 PM
> To: linuxppc-dev@lists.ozlabs.org; linux-kernel@vger.kernel.org
> Cc: Bucur Madalin-Cristian-B32716; Kanetkar Shruti-B44454; Xie Shaohui-B21989
> Subject: [PATCH 2/4] phylib: Add generic 10G driver
>
> From: Andy Fleming
>
> Very incomplete, but will allow for binding an ethernet controller to it.
>
> Also, Add XGMII interface type
>
> Signed-off-by: Andy Fleming
> Signed-off-by: Shaohui Xie <Shaohui.Xie@freescale.com>
> ---
> drivers/net/phy/phy_device.c | 101 ++++++++++++++++++++++++++++++++++++++++++-
> include/linux/phy.h | 1 +
> 2 files changed, 101 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c index
> 74630e9..30bf2d5 100644
> --- a/drivers/net/phy/phy_device.c
> +++ b/drivers/net/phy/phy_device.c
> @@ -32,6 +32,7 @@
> #include <linux/module.h>
> #include <linux/mii.h>
> #include <linux/ethtool.h>
> +#include <linux/mdio.h>
> #include <linux/phy.h>
>
> #include <asm/io.h>
> @@ -689,6 +690,13 @@ static int genphy_config_advert(struct phy_device *phydev)
> return changed;
> }
>
> +int gen10g_config_advert(struct phy_device *dev) {
> + return 0;
> +}
> +EXPORT_SYMBOL(gen10g_config_advert);
> +
> +
> /**
> * genphy_setup_forced - configures/forces speed/duplex from @phydev
> * @phydev: target phy_device struct
> @@ -742,6 +750,12 @@ int genphy_restart_aneg(struct phy_device *phydev) }
> EXPORT_SYMBOL(genphy_restart_aneg);
>
> +int gen10g_restart_aneg(struct phy_device *phydev) {
> + return 0;
> +}
> +EXPORT_SYMBOL(gen10g_restart_aneg);
> +
>
> /**
> * genphy_config_aneg - restart auto-negotiation or write BMCR @@ -784,6
> +798,13 @@ int genphy_config_aneg(struct phy_device *phydev) }
> EXPORT_SYMBOL(genphy_config_aneg);
>
> +int gen10g_config_aneg(struct phy_device *phydev) {
> + return 0;
> +}
> +EXPORT_SYMBOL(gen10g_config_aneg);
> +
> +
> /**
> * genphy_update_link - update link status in @phydev
> * @phydev: target phy_device struct
> @@ -913,6 +934,35 @@ int genphy_read_status(struct phy_device *phydev) }
> EXPORT_SYMBOL(genphy_read_status);
>
> +int gen10g_read_status(struct phy_device *phydev) {
> + int devad, reg;
> + u32 mmd_mask = phydev->c45_ids.devices_in_package;
> +
> + phydev->link = 1;
> +
> + /* For now just lie and say it's 10G all the time */
> + phydev->speed = 10000;
> + phydev->duplex = DUPLEX_FULL;
> +
> + for (devad = 0; mmd_mask; devad++, mmd_mask = mmd_mask >> 1) {
> + if (!(mmd_mask & 1))
> + continue;
> +
> + /* Read twice because link state is latched and a
> + * read moves the current state into the register
> + */
> + phy_read_mmd(phydev, devad, MDIO_STAT1);
> + reg = phy_read_mmd(phydev, devad, MDIO_STAT1);
> + if (reg < 0 || !(reg & MDIO_STAT1_LSTATUS))
> + phydev->link = 0;
> + }
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(gen10g_read_status);
> +
> +
> static int genphy_config_init(struct phy_device *phydev) {
> int val;
> @@ -959,6 +1009,15 @@ static int genphy_config_init(struct phy_device *phydev)
>
> return 0;
> }
> +
> +static int gen10g_config_init(struct phy_device *phydev) {
> + /* Temporarily just say we support everything */
> + phydev->supported = phydev->advertising = SUPPORTED_10000baseT_Full;
> +
> + return 0;
> +}
> +
> int genphy_suspend(struct phy_device *phydev) {
> int value;
> @@ -974,6 +1033,13 @@ int genphy_suspend(struct phy_device *phydev) }
> EXPORT_SYMBOL(genphy_suspend);
>
> +int gen10g_suspend(struct phy_device *phydev) {
> + return 0;
> +}
> +EXPORT_SYMBOL(gen10g_suspend);
> +
> +
> int genphy_resume(struct phy_device *phydev) {
> int value;
> @@ -989,6 +1055,13 @@ int genphy_resume(struct phy_device *phydev) }
> EXPORT_SYMBOL(genphy_resume);
>
> +int gen10g_resume(struct phy_device *phydev) {
> + return 0;
> +}
> +EXPORT_SYMBOL(gen10g_resume);
> +
> +
> /**
> * phy_probe - probe and init a PHY device
> * @dev: device to probe and init
> @@ -1129,6 +1202,20 @@ static struct phy_driver genphy_driver = {
> .driver = {.owner= THIS_MODULE, },
> };
>
> +static struct phy_driver gen10g_driver = {
> + .phy_id = 0xffffffff,
> + .phy_id_mask = 0xffffffff,
> + .name = "Generic 10G PHY",
> + .config_init = gen10g_config_init,
> + .features = 0,
> + .config_aneg = gen10g_config_aneg,
> + .read_status = gen10g_read_status,
> + .suspend = gen10g_suspend,
> + .resume = gen10g_resume,
> + .driver = {.owner = THIS_MODULE, },
> +};
> +
> +
> static int __init phy_init(void)
> {
> int rc;
> @@ -1139,13 +1226,25 @@ static int __init phy_init(void)
>
> rc = phy_driver_register(&genphy_driver);
> if (rc)
> - mdio_bus_exit();
> + goto genphy_register_failed;
> +
> + rc = phy_driver_register(&gen10g_driver);
> + if (rc)
> + goto gen10g_register_failed;
> +
> + return rc;
> +
> +gen10g_register_failed:
> + phy_driver_unregister(&genphy_driver);
> +genphy_register_failed:
> + mdio_bus_exit();
>
> return rc;
> }
>
> static void __exit phy_exit(void)
> {
> + phy_driver_unregister(&gen10g_driver);
> phy_driver_unregister(&genphy_driver);
> mdio_bus_exit();
> }
> diff --git a/include/linux/phy.h b/include/linux/phy.h index 684925a..f864004
> 100644
> --- a/include/linux/phy.h
> +++ b/include/linux/phy.h
> @@ -66,6 +66,7 @@ typedef enum {
> PHY_INTERFACE_MODE_RGMII_TXID,
> PHY_INTERFACE_MODE_RTBI,
> PHY_INTERFACE_MODE_SMII,
> + PHY_INTERFACE_MODE_XGMII,
> } phy_interface_t;
>
>
> --
> 1.8.4.1
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH 2/4] phylib: Add generic 10G driver
2013-11-11 11:07 [PATCH 2/4] phylib: Add generic 10G driver shh.xie
2013-11-12 12:31 ` Shaohui Xie
@ 2013-11-12 17:53 ` Florian Fainelli
2013-11-13 11:56 ` Shaohui Xie
1 sibling, 1 reply; 4+ messages in thread
From: Florian Fainelli @ 2013-11-12 17:53 UTC (permalink / raw)
To: shh.xie; +Cc: linuxppc-dev, linux-kernel, madalin.bucur, Shruti, Shaohui Xie
Hello Shaohui,
2013/11/11 <shh.xie@gmail.com>:
> From: Andy Fleming
>
> Very incomplete, but will allow for binding an ethernet controller
> to it.
>
> Also, Add XGMII interface type
So that should be two separate patches, and
drivers/of/of_net.c::of_get_phy_mode() must be updated to know about
XMGII.
>
> Signed-off-by: Andy Fleming
Missing Andy's Signed-off-by tag.
> Signed-off-by: Shaohui Xie <Shaohui.Xie@freescale.com>
> ---
> drivers/net/phy/phy_device.c | 101 ++++++++++++++++++++++++++++++++++++++++++-
> include/linux/phy.h | 1 +
> 2 files changed, 101 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
> index 74630e9..30bf2d5 100644
> --- a/drivers/net/phy/phy_device.c
> +++ b/drivers/net/phy/phy_device.c
> @@ -32,6 +32,7 @@
> #include <linux/module.h>
> #include <linux/mii.h>
> #include <linux/ethtool.h>
> +#include <linux/mdio.h>
> #include <linux/phy.h>
>
> #include <asm/io.h>
> @@ -689,6 +690,13 @@ static int genphy_config_advert(struct phy_device *phydev)
> return changed;
> }
>
> +int gen10g_config_advert(struct phy_device *dev)
> +{
> + return 0;
> +}
> +EXPORT_SYMBOL(gen10g_config_advert);
> +
> +
> /**
> * genphy_setup_forced - configures/forces speed/duplex from @phydev
> * @phydev: target phy_device struct
> @@ -742,6 +750,12 @@ int genphy_restart_aneg(struct phy_device *phydev)
> }
> EXPORT_SYMBOL(genphy_restart_aneg);
>
> +int gen10g_restart_aneg(struct phy_device *phydev)
> +{
> + return 0;
> +}
> +EXPORT_SYMBOL(gen10g_restart_aneg);
> +
>
> /**
> * genphy_config_aneg - restart auto-negotiation or write BMCR
> @@ -784,6 +798,13 @@ int genphy_config_aneg(struct phy_device *phydev)
> }
> EXPORT_SYMBOL(genphy_config_aneg);
>
> +int gen10g_config_aneg(struct phy_device *phydev)
> +{
> + return 0;
> +}
> +EXPORT_SYMBOL(gen10g_config_aneg);
> +
> +
> /**
> * genphy_update_link - update link status in @phydev
> * @phydev: target phy_device struct
> @@ -913,6 +934,35 @@ int genphy_read_status(struct phy_device *phydev)
> }
> EXPORT_SYMBOL(genphy_read_status);
>
> +int gen10g_read_status(struct phy_device *phydev)
> +{
> + int devad, reg;
> + u32 mmd_mask = phydev->c45_ids.devices_in_package;
> +
> + phydev->link = 1;
> +
> + /* For now just lie and say it's 10G all the time */
> + phydev->speed = 10000;
Can you at least make this a little more proof? Something along:
if (phydev->supported & (SUPPORTED_10000baseT_Full))
phydev->speed = SPEED_10000;
else if (phydev->supported & (SUPPORTED_1000baseT_Full)
phydev->speed = SPEED_1000;
Although ideally we should be reading the relevant registers to figure
out what to do.
> + phydev->duplex = DUPLEX_FULL;
> +
> + for (devad = 0; mmd_mask; devad++, mmd_mask = mmd_mask >> 1) {
> + if (!(mmd_mask & 1))
> + continue;
> +
> + /* Read twice because link state is latched and a
> + * read moves the current state into the register
> + */
> + phy_read_mmd(phydev, devad, MDIO_STAT1);
> + reg = phy_read_mmd(phydev, devad, MDIO_STAT1);
> + if (reg < 0 || !(reg & MDIO_STAT1_LSTATUS))
> + phydev->link = 0;
> + }
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(gen10g_read_status);
> +
> +
> static int genphy_config_init(struct phy_device *phydev)
> {
> int val;
> @@ -959,6 +1009,15 @@ static int genphy_config_init(struct phy_device *phydev)
>
> return 0;
> }
> +
> +static int gen10g_config_init(struct phy_device *phydev)
> +{
> + /* Temporarily just say we support everything */
> + phydev->supported = phydev->advertising = SUPPORTED_10000baseT_Full;
For consistency you should set SUPPORTED_TP, 1000baseT_Full does not
make sense for anything but twisted pairs AFAIR.
> +
> + return 0;
> +}
> +
> int genphy_suspend(struct phy_device *phydev)
> {
> int value;
> @@ -974,6 +1033,13 @@ int genphy_suspend(struct phy_device *phydev)
> }
> EXPORT_SYMBOL(genphy_suspend);
>
> +int gen10g_suspend(struct phy_device *phydev)
> +{
> + return 0;
> +}
> +EXPORT_SYMBOL(gen10g_suspend);
> +
> +
> int genphy_resume(struct phy_device *phydev)
> {
> int value;
> @@ -989,6 +1055,13 @@ int genphy_resume(struct phy_device *phydev)
> }
> EXPORT_SYMBOL(genphy_resume);
>
> +int gen10g_resume(struct phy_device *phydev)
> +{
> + return 0;
> +}
> +EXPORT_SYMBOL(gen10g_resume);
> +
> +
> /**
> * phy_probe - probe and init a PHY device
> * @dev: device to probe and init
> @@ -1129,6 +1202,20 @@ static struct phy_driver genphy_driver = {
> .driver = {.owner= THIS_MODULE, },
> };
>
> +static struct phy_driver gen10g_driver = {
> + .phy_id = 0xffffffff,
> + .phy_id_mask = 0xffffffff,
> + .name = "Generic 10G PHY",
> + .config_init = gen10g_config_init,
> + .features = 0,
This should be updated to be PHY_10GBIT_FEATURES where
PHY_10GBIT_FEATURES is defined to contain at least PHY_GBIT_FEATURES.
> + .config_aneg = gen10g_config_aneg,
> + .read_status = gen10g_read_status,
> + .suspend = gen10g_suspend,
> + .resume = gen10g_resume,
> + .driver = {.owner = THIS_MODULE, },
> +};
> +
> +
> static int __init phy_init(void)
> {
> int rc;
> @@ -1139,13 +1226,25 @@ static int __init phy_init(void)
>
> rc = phy_driver_register(&genphy_driver);
> if (rc)
> - mdio_bus_exit();
> + goto genphy_register_failed;
> +
> + rc = phy_driver_register(&gen10g_driver);
> + if (rc)
> + goto gen10g_register_failed;
> +
> + return rc;
> +
> +gen10g_register_failed:
> + phy_driver_unregister(&genphy_driver);
> +genphy_register_failed:
> + mdio_bus_exit();
As a subsequent patch you could use phy_drivers_register()
>
> return rc;
> }
>
> static void __exit phy_exit(void)
> {
> + phy_driver_unregister(&gen10g_driver);
> phy_driver_unregister(&genphy_driver);
And phy_drivers_unregister() here.
> mdio_bus_exit();
> }
> diff --git a/include/linux/phy.h b/include/linux/phy.h
> index 684925a..f864004 100644
> --- a/include/linux/phy.h
> +++ b/include/linux/phy.h
> @@ -66,6 +66,7 @@ typedef enum {
> PHY_INTERFACE_MODE_RGMII_TXID,
> PHY_INTERFACE_MODE_RTBI,
> PHY_INTERFACE_MODE_SMII,
> + PHY_INTERFACE_MODE_XGMII,
> } phy_interface_t;
>
>
> --
> 1.8.4.1
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
--
Florian
^ permalink raw reply [flat|nested] 4+ messages in thread* RE: [PATCH 2/4] phylib: Add generic 10G driver
2013-11-12 17:53 ` Florian Fainelli
@ 2013-11-13 11:56 ` Shaohui Xie
0 siblings, 0 replies; 4+ messages in thread
From: Shaohui Xie @ 2013-11-13 11:56 UTC (permalink / raw)
To: Florian Fainelli, shh.xie
Cc: linuxppc-dev, linux-kernel, Madalin-Cristian Bucur, Shruti Kanetkar
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 7421 bytes --]
Hello, Florian,
Thank you for reviewing the patches!
Please see my comments inline.
Best Regards,
Shaohui Xie
> -----Original Message-----
> From: Florian Fainelli [mailto:f.fainelli@gmail.com]
> Sent: Wednesday, November 13, 2013 1:54 AM
> To: shh.xie@gmail.com
> Cc: linuxppc-dev; linux-kernel@vger.kernel.org; Bucur Madalin-Cristian-
> B32716; Kanetkar Shruti-B44454; Xie Shaohui-B21989
> Subject: Re: [PATCH 2/4] phylib: Add generic 10G driver
>
> Hello Shaohui,
>
> 2013/11/11 <shh.xie@gmail.com>:
> > From: Andy Fleming
> >
> > Very incomplete, but will allow for binding an ethernet controller to
> > it.
> >
> > Also, Add XGMII interface type
>
> So that should be two separate patches, and
> drivers/of/of_net.c::of_get_phy_mode() must be updated to know about
> XMGII.
>
> >
> > Signed-off-by: Andy Fleming
>
> Missing Andy's Signed-off-by tag.
[S.H] Will add in next version, I removed it to make git work since Andy's e-mail address is not valid.
>
> > Signed-off-by: Shaohui Xie <Shaohui.Xie@freescale.com>
> > ---
> > drivers/net/phy/phy_device.c | 101
> ++++++++++++++++++++++++++++++++++++++++++-
> > include/linux/phy.h | 1 +
> > 2 files changed, 101 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/phy/phy_device.c
> > b/drivers/net/phy/phy_device.c index 74630e9..30bf2d5 100644
> > --- a/drivers/net/phy/phy_device.c
> > +++ b/drivers/net/phy/phy_device.c
> > @@ -32,6 +32,7 @@
> > #include <linux/module.h>
> > #include <linux/mii.h>
> > #include <linux/ethtool.h>
> > +#include <linux/mdio.h>
> > #include <linux/phy.h>
> >
> > #include <asm/io.h>
> > @@ -689,6 +690,13 @@ static int genphy_config_advert(struct phy_device
> *phydev)
> > return changed;
> > }
> >
> > +int gen10g_config_advert(struct phy_device *dev) {
> > + return 0;
> > +}
> > +EXPORT_SYMBOL(gen10g_config_advert);
> > +
> > +
> > /**
> > * genphy_setup_forced - configures/forces speed/duplex from @phydev
> > * @phydev: target phy_device struct
> > @@ -742,6 +750,12 @@ int genphy_restart_aneg(struct phy_device
> > *phydev) } EXPORT_SYMBOL(genphy_restart_aneg);
> >
> > +int gen10g_restart_aneg(struct phy_device *phydev) {
> > + return 0;
> > +}
> > +EXPORT_SYMBOL(gen10g_restart_aneg);
> > +
> >
> > /**
> > * genphy_config_aneg - restart auto-negotiation or write BMCR @@
> > -784,6 +798,13 @@ int genphy_config_aneg(struct phy_device *phydev) }
> > EXPORT_SYMBOL(genphy_config_aneg);
> >
> > +int gen10g_config_aneg(struct phy_device *phydev) {
> > + return 0;
> > +}
> > +EXPORT_SYMBOL(gen10g_config_aneg);
> > +
> > +
> > /**
> > * genphy_update_link - update link status in @phydev
> > * @phydev: target phy_device struct
> > @@ -913,6 +934,35 @@ int genphy_read_status(struct phy_device *phydev)
> > } EXPORT_SYMBOL(genphy_read_status);
> >
> > +int gen10g_read_status(struct phy_device *phydev) {
> > + int devad, reg;
> > + u32 mmd_mask = phydev->c45_ids.devices_in_package;
> > +
> > + phydev->link = 1;
> > +
> > + /* For now just lie and say it's 10G all the time */
> > + phydev->speed = 10000;
>
> Can you at least make this a little more proof? Something along:
>
> if (phydev->supported & (SUPPORTED_10000baseT_Full))
> phydev->speed = SPEED_10000; else if (phydev->supported &
> (SUPPORTED_1000baseT_Full)
> phydev->speed = SPEED_1000;
[S.H] some 10G PHY only support 10G speed.
>
> Although ideally we should be reading the relevant registers to figure
> out what to do.
[S.H] Yes, code below will try to read the mmds to get status.
>
> > + phydev->duplex = DUPLEX_FULL;
> > +
> > + for (devad = 0; mmd_mask; devad++, mmd_mask = mmd_mask >> 1) {
> > + if (!(mmd_mask & 1))
> > + continue;
> > +
> > + /* Read twice because link state is latched and a
> > + * read moves the current state into the register
> > + */
> > + phy_read_mmd(phydev, devad, MDIO_STAT1);
> > + reg = phy_read_mmd(phydev, devad, MDIO_STAT1);
> > + if (reg < 0 || !(reg & MDIO_STAT1_LSTATUS))
> > + phydev->link = 0;
> > + }
> > +
> > + return 0;
> > +}
> > +EXPORT_SYMBOL(gen10g_read_status);
> > +
> > +
> > static int genphy_config_init(struct phy_device *phydev) {
> > int val;
> > @@ -959,6 +1009,15 @@ static int genphy_config_init(struct phy_device
> > *phydev)
> >
> > return 0;
> > }
> > +
> > +static int gen10g_config_init(struct phy_device *phydev) {
> > + /* Temporarily just say we support everything */
> > + phydev->supported = phydev->advertising =
> > +SUPPORTED_10000baseT_Full;
>
> For consistency you should set SUPPORTED_TP, 1000baseT_Full does not make
> sense for anything but twisted pairs AFAIR.
[S.H] OK.
>
> > +
> > + return 0;
> > +}
> > +
> > int genphy_suspend(struct phy_device *phydev) {
> > int value;
> > @@ -974,6 +1033,13 @@ int genphy_suspend(struct phy_device *phydev) }
> > EXPORT_SYMBOL(genphy_suspend);
> >
> > +int gen10g_suspend(struct phy_device *phydev) {
> > + return 0;
> > +}
> > +EXPORT_SYMBOL(gen10g_suspend);
> > +
> > +
> > int genphy_resume(struct phy_device *phydev) {
> > int value;
> > @@ -989,6 +1055,13 @@ int genphy_resume(struct phy_device *phydev) }
> > EXPORT_SYMBOL(genphy_resume);
> >
> > +int gen10g_resume(struct phy_device *phydev) {
> > + return 0;
> > +}
> > +EXPORT_SYMBOL(gen10g_resume);
> > +
> > +
> > /**
> > * phy_probe - probe and init a PHY device
> > * @dev: device to probe and init
> > @@ -1129,6 +1202,20 @@ static struct phy_driver genphy_driver = {
> > .driver = {.owner= THIS_MODULE, },
> > };
> >
> > +static struct phy_driver gen10g_driver = {
> > + .phy_id = 0xffffffff,
> > + .phy_id_mask = 0xffffffff,
> > + .name = "Generic 10G PHY",
> > + .config_init = gen10g_config_init,
> > + .features = 0,
>
> This should be updated to be PHY_10GBIT_FEATURES where
> PHY_10GBIT_FEATURES is defined to contain at least PHY_GBIT_FEATURES.
[S.H] for a generic 10G PHY, what is the feature should be supported?
>
> > + .config_aneg = gen10g_config_aneg,
> > + .read_status = gen10g_read_status,
> > + .suspend = gen10g_suspend,
> > + .resume = gen10g_resume,
> > + .driver = {.owner = THIS_MODULE, },
> > +};
> > +
> > +
> > static int __init phy_init(void)
> > {
> > int rc;
> > @@ -1139,13 +1226,25 @@ static int __init phy_init(void)
> >
> > rc = phy_driver_register(&genphy_driver);
> > if (rc)
> > - mdio_bus_exit();
> > + goto genphy_register_failed;
> > +
> > + rc = phy_driver_register(&gen10g_driver);
> > + if (rc)
> > + goto gen10g_register_failed;
> > +
> > + return rc;
> > +
> > +gen10g_register_failed:
> > + phy_driver_unregister(&genphy_driver);
> > +genphy_register_failed:
> > + mdio_bus_exit();
>
> As a subsequent patch you could use phy_drivers_register()
[S.H] you mean like phy_drivers_register(gen10g_driver, ARRAY_SIZE(gen10g_driver))?
ÿôèº{.nÇ+‰·Ÿ®‰†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a¢f£¢·hšïêÿ‘êçz_è®\x03(階ŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨èÚ&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2013-11-13 11:57 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-11-11 11:07 [PATCH 2/4] phylib: Add generic 10G driver shh.xie
2013-11-12 12:31 ` Shaohui Xie
2013-11-12 17:53 ` Florian Fainelli
2013-11-13 11:56 ` Shaohui Xie
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®