netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] phylib: add driver for aquantia phy
@ 2015-07-24  3:46 shh.xie
  2015-07-24  4:39 ` Florian Fainelli
  0 siblings, 1 reply; 5+ messages in thread
From: shh.xie @ 2015-07-24  3:46 UTC (permalink / raw)
  To: netdev, davem; +Cc: Shaohui Xie

From: Shaohui Xie <Shaohui.Xie@freescale.com>

This patch added driver to support Aquantia PHYs AQ1202, AQ2104, AQR105,
AQR405, which accessed through clause 45.

Signed-off-by: Shaohui Xie <Shaohui.Xie@freescale.com>
---
 drivers/net/phy/Kconfig    |   5 ++
 drivers/net/phy/Makefile   |   1 +
 drivers/net/phy/aquantia.c | 157 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 163 insertions(+)
 create mode 100644 drivers/net/phy/aquantia.c

diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index d6aff87..839b84c 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -14,6 +14,11 @@ if PHYLIB
 
 comment "MII PHY device drivers"
 
+config AQUANTIA_PHY
+        tristate "Drivers for the Aquantia PHYs"
+        ---help---
+          Currently supports the Aquantia AQ1202, AQ2104, AQR105, AQR405
+
 config AT803X_PHY
 	tristate "Drivers for Atheros AT803X PHYs"
 	---help---
diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
index 16aac1c..9bb1033 100644
--- a/drivers/net/phy/Makefile
+++ b/drivers/net/phy/Makefile
@@ -3,6 +3,7 @@
 libphy-objs			:= phy.o phy_device.o mdio_bus.o
 
 obj-$(CONFIG_PHYLIB)		+= libphy.o
+obj-$(CONFIG_AQUANTIA_PHY)	+= aquantia.o
 obj-$(CONFIG_MARVELL_PHY)	+= marvell.o
 obj-$(CONFIG_DAVICOM_PHY)	+= davicom.o
 obj-$(CONFIG_CICADA_PHY)	+= cicada.o
diff --git a/drivers/net/phy/aquantia.c b/drivers/net/phy/aquantia.c
new file mode 100644
index 0000000..cb848f95
--- /dev/null
+++ b/drivers/net/phy/aquantia.c
@@ -0,0 +1,157 @@
+/*
+ * Driver for Aquantia PHY
+ *
+ * Author: Shaohui Xie <Shaohui.Xie@freescale.com>
+ *
+ * Copyright 2015 Freescale Semiconductor, Inc.
+ *
+ * This file is licensed under the terms of the GNU General Public License
+ * version 2.  This program is licensed "as is" without any warranty of any
+ * kind, whether express or implied.
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/delay.h>
+#include <linux/mii.h>
+#include <linux/ethtool.h>
+#include <linux/phy.h>
+#include <linux/mdio.h>
+
+#define PHY_ID_AQ1202	0x03a1b445
+#define PHY_ID_AQ2104	0x03a1b460
+#define PHY_ID_AQR105	0x03a1b4a2
+#define PHY_ID_AQR405	0x03a1b4b0
+
+static int aquantia_soft_reset(struct phy_device *phydev)
+{
+	return 0;
+}
+
+static int aquantia_config_init(struct phy_device *phydev)
+{
+	return 0;
+}
+
+static int aquantia_config_aneg(struct phy_device *phydev)
+{
+	phydev->supported = SUPPORTED_10000baseT_Full | PHY_GBIT_FEATURES;
+	phydev->advertising = phydev->supported;
+
+	return 0;
+}
+
+static int aquantia_aneg_done(struct phy_device *phydev)
+{
+	int reg;
+
+	reg = phy_read_mmd(phydev, MDIO_MMD_AN, MDIO_STAT1);
+	return (reg < 0) ? reg : (reg & BMSR_ANEGCOMPLETE);
+}
+
+static int aquantia_read_status(struct phy_device *phydev)
+{
+	int reg;
+
+	phydev->speed = SPEED_10000;
+	phydev->duplex = DUPLEX_FULL;
+
+	reg = phy_read_mmd(phydev, MDIO_MMD_AN, MDIO_STAT1);
+	reg = phy_read_mmd(phydev, MDIO_MMD_AN, MDIO_STAT1);
+	if (reg & MDIO_STAT1_LSTATUS)
+		phydev->link = 1;
+	else
+		phydev->link = 0;
+
+	reg = phy_read_mmd(phydev, MDIO_MMD_AN, 0xc800);
+	mdelay(10);
+	reg = phy_read_mmd(phydev, MDIO_MMD_AN, 0xc800);
+	if (reg == 0x9)
+		phydev->speed = SPEED_2500;
+	else if (reg == 0x5)
+		phydev->speed = SPEED_1000;
+	else if (reg == 0x3)
+		phydev->speed = SPEED_100;
+
+	return 0;
+}
+
+static struct phy_driver aquantia_driver[] = {
+{
+	.phy_id		= PHY_ID_AQ1202,
+	.phy_id_mask	= 0xfffffff0,
+	.name		= "Aquantia AQ1202",
+	.features	= PHY_GBIT_FEATURES,
+	.soft_reset	= aquantia_soft_reset,
+	.aneg_done	= aquantia_aneg_done,
+	.config_init    = aquantia_config_init,
+	.config_aneg    = aquantia_config_aneg,
+	.read_status	= aquantia_read_status,
+	.driver		= { .owner = THIS_MODULE,},
+},
+{
+	.phy_id		= PHY_ID_AQ2104,
+	.phy_id_mask	= 0xfffffff0,
+	.name		= "Aquantia AQ2104",
+	.features	= PHY_GBIT_FEATURES,
+	.soft_reset	= aquantia_soft_reset,
+	.aneg_done	= aquantia_aneg_done,
+	.config_init    = aquantia_config_init,
+	.config_aneg    = aquantia_config_aneg,
+	.read_status	= aquantia_read_status,
+	.driver		= { .owner = THIS_MODULE,},
+},
+{
+	.phy_id		= PHY_ID_AQR105,
+	.phy_id_mask	= 0xfffffff0,
+	.name		= "Aquantia AQR105",
+	.features	= PHY_GBIT_FEATURES,
+	.soft_reset	= aquantia_soft_reset,
+	.aneg_done	= aquantia_aneg_done,
+	.config_init    = aquantia_config_init,
+	.config_aneg    = aquantia_config_aneg,
+	.read_status	= aquantia_read_status,
+	.driver		= { .owner = THIS_MODULE,},
+},
+{
+	.phy_id		= PHY_ID_AQR405,
+	.phy_id_mask	= 0xfffffff0,
+	.name		= "Aquantia AQR405",
+	.features	= PHY_GBIT_FEATURES,
+	.soft_reset	= aquantia_soft_reset,
+	.aneg_done	= aquantia_aneg_done,
+	.config_init    = aquantia_config_init,
+	.config_aneg    = aquantia_config_aneg,
+	.read_status	= aquantia_read_status,
+	.driver		= { .owner = THIS_MODULE,},
+},
+};
+
+static int __init aquantia_init(void)
+{
+	return phy_drivers_register(aquantia_driver,
+				    ARRAY_SIZE(aquantia_driver));
+}
+
+static void __exit aquantia_exit(void)
+{
+	return phy_drivers_unregister(aquantia_driver,
+				      ARRAY_SIZE(aquantia_driver));
+}
+
+module_init(aquantia_init);
+module_exit(aquantia_exit);
+
+static struct mdio_device_id __maybe_unused aquantia_tbl[] = {
+	{ PHY_ID_AQ1202, 0xfffffff0 },
+	{ PHY_ID_AQ2104, 0xfffffff0 },
+	{ PHY_ID_AQR105, 0xfffffff0 },
+	{ PHY_ID_AQR405, 0xfffffff0 },
+	{ }
+};
+
+MODULE_DEVICE_TABLE(mdio, aquantia_tbl);
+
+MODULE_DESCRIPTION("Aquantia PHY driver");
+MODULE_AUTHOR("Shaohui Xie <Shaohui.Xie@freescale.com>");
+MODULE_LICENSE("GPL v2");
-- 
2.1.0.27.g96db324

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] phylib: add driver for aquantia phy
  2015-07-24  3:46 [PATCH] phylib: add driver for aquantia phy shh.xie
@ 2015-07-24  4:39 ` Florian Fainelli
  2015-07-27  8:30   ` Shaohui Xie
  0 siblings, 1 reply; 5+ messages in thread
From: Florian Fainelli @ 2015-07-24  4:39 UTC (permalink / raw)
  To: shh.xie, netdev, davem; +Cc: Shaohui Xie

Le 07/23/15 20:46, shh.xie@gmail.com a écrit :
> From: Shaohui Xie <Shaohui.Xie@freescale.com>
> 
> This patch added driver to support Aquantia PHYs AQ1202, AQ2104, AQR105,
> AQR405, which accessed through clause 45.

Could you prefix your patches with "net: phy: " in the future to be
consistent with what is typically used?

See comments below

> 
> Signed-off-by: Shaohui Xie <Shaohui.Xie@freescale.com>
> ---

[snip]

> +static int aquantia_read_status(struct phy_device *phydev)
> +{
> +	int reg;
> +
> +	phydev->speed = SPEED_10000;
> +	phydev->duplex = DUPLEX_FULL;
> +
> +	reg = phy_read_mmd(phydev, MDIO_MMD_AN, MDIO_STAT1);
> +	reg = phy_read_mmd(phydev, MDIO_MMD_AN, MDIO_STAT1);
> +	if (reg & MDIO_STAT1_LSTATUS)
> +		phydev->link = 1;
> +	else
> +		phydev->link = 0;
> +
> +	reg = phy_read_mmd(phydev, MDIO_MMD_AN, 0xc800);
> +	mdelay(10);
> +	reg = phy_read_mmd(phydev, MDIO_MMD_AN, 0xc800);
> +	if (reg == 0x9)
> +		phydev->speed = SPEED_2500;
> +	else if (reg == 0x5)
> +		phydev->speed = SPEED_1000;
> +	else if (reg == 0x3)
> +		phydev->speed = SPEED_100;

Could we use a switch/case here? How about 10Mbits/sec and duplex are we
guaranteed to be full-duplex at e.g: 100 or 10Mbits/sec?

> +
> +	return 0;
> +}
> +
> +static struct phy_driver aquantia_driver[] = {
> +{
> +	.phy_id		= PHY_ID_AQ1202,
> +	.phy_id_mask	= 0xfffffff0,
> +	.name		= "Aquantia AQ1202",
> +	.features	= PHY_GBIT_FEATURES,

If these are 10GbE PHYs, should not we start defining a new features
bitmask here to reflect that accordingly? That way MAC

> +	.soft_reset	= aquantia_soft_reset,
> +	.aneg_done	= aquantia_aneg_done,
> +	.config_init    = aquantia_config_init,
> +	.config_aneg    = aquantia_config_aneg,
> +	.read_status	= aquantia_read_status,
> +	.driver		= { .owner = THIS_MODULE,},
> +},
> +{
> +	.phy_id		= PHY_ID_AQ2104,
> +	.phy_id_mask	= 0xfffffff0,
> +	.name		= "Aquantia AQ2104",
> +	.features	= PHY_GBIT_FEATURES,
> +	.soft_reset	= aquantia_soft_reset,
> +	.aneg_done	= aquantia_aneg_done,
> +	.config_init    = aquantia_config_init,
> +	.config_aneg    = aquantia_config_aneg,
> +	.read_status	= aquantia_read_status,
> +	.driver		= { .owner = THIS_MODULE,},
> +},
> +{
> +	.phy_id		= PHY_ID_AQR105,
> +	.phy_id_mask	= 0xfffffff0,
> +	.name		= "Aquantia AQR105",
> +	.features	= PHY_GBIT_FEATURES,
> +	.soft_reset	= aquantia_soft_reset,
> +	.aneg_done	= aquantia_aneg_done,
> +	.config_init    = aquantia_config_init,
> +	.config_aneg    = aquantia_config_aneg,
> +	.read_status	= aquantia_read_status,
> +	.driver		= { .owner = THIS_MODULE,},
> +},
> +{
> +	.phy_id		= PHY_ID_AQR405,
> +	.phy_id_mask	= 0xfffffff0,
> +	.name		= "Aquantia AQR405",
> +	.features	= PHY_GBIT_FEATURES,
> +	.soft_reset	= aquantia_soft_reset,
> +	.aneg_done	= aquantia_aneg_done,
> +	.config_init    = aquantia_config_init,
> +	.config_aneg    = aquantia_config_aneg,
> +	.read_status	= aquantia_read_status,
> +	.driver		= { .owner = THIS_MODULE,},
> +},
> +};
> +
> +static int __init aquantia_init(void)
> +{
> +	return phy_drivers_register(aquantia_driver,
> +				    ARRAY_SIZE(aquantia_driver));
> +}
> +
> +static void __exit aquantia_exit(void)
> +{
> +	return phy_drivers_unregister(aquantia_driver,
> +				      ARRAY_SIZE(aquantia_driver));
> +}
> +
> +module_init(aquantia_init);
> +module_exit(aquantia_exit);
> +
> +static struct mdio_device_id __maybe_unused aquantia_tbl[] = {
> +	{ PHY_ID_AQ1202, 0xfffffff0 },
> +	{ PHY_ID_AQ2104, 0xfffffff0 },
> +	{ PHY_ID_AQR105, 0xfffffff0 },
> +	{ PHY_ID_AQR405, 0xfffffff0 },
> +	{ }
> +};
> +
> +MODULE_DEVICE_TABLE(mdio, aquantia_tbl);
> +
> +MODULE_DESCRIPTION("Aquantia PHY driver");
> +MODULE_AUTHOR("Shaohui Xie <Shaohui.Xie@freescale.com>");
> +MODULE_LICENSE("GPL v2");
> 


-- 
Florian

^ permalink raw reply	[flat|nested] 5+ messages in thread

* RE: [PATCH] phylib: add driver for aquantia phy
  2015-07-24  4:39 ` Florian Fainelli
@ 2015-07-27  8:30   ` Shaohui Xie
  2015-07-27 21:49     ` Florian Fainelli
  0 siblings, 1 reply; 5+ messages in thread
From: Shaohui Xie @ 2015-07-27  8:30 UTC (permalink / raw)
  To: Florian Fainelli, netdev, davem

> -----Original Message-----
> From: Florian Fainelli [mailto:f.fainelli@gmail.com]
> Sent: Friday, July 24, 2015 12:39 PM
> To: shh.xie@gmail.com; netdev@vger.kernel.org; davem@davemloft.net
> Cc: Xie Shaohui-B21989
> Subject: Re: [PATCH] phylib: add driver for aquantia phy
> 
> Le 07/23/15 20:46, shh.xie@gmail.com a écrit :
> > From: Shaohui Xie <Shaohui.Xie@freescale.com>
> >
> > This patch added driver to support Aquantia PHYs AQ1202, AQ2104,
> > AQR105, AQR405, which accessed through clause 45.
> 
> Could you prefix your patches with "net: phy: " in the future to be
> consistent with what is typically used?
[S.H] OK.

> 
> See comments below
> 
> >
> > Signed-off-by: Shaohui Xie <Shaohui.Xie@freescale.com>
> > ---
> 
> [snip]
> 
> > +static int aquantia_read_status(struct phy_device *phydev) {
> > +	int reg;
> > +
> > +	phydev->speed = SPEED_10000;
> > +	phydev->duplex = DUPLEX_FULL;
> > +
> > +	reg = phy_read_mmd(phydev, MDIO_MMD_AN, MDIO_STAT1);
> > +	reg = phy_read_mmd(phydev, MDIO_MMD_AN, MDIO_STAT1);
> > +	if (reg & MDIO_STAT1_LSTATUS)
> > +		phydev->link = 1;
> > +	else
> > +		phydev->link = 0;
> > +
> > +	reg = phy_read_mmd(phydev, MDIO_MMD_AN, 0xc800);
> > +	mdelay(10);
> > +	reg = phy_read_mmd(phydev, MDIO_MMD_AN, 0xc800);
> > +	if (reg == 0x9)
> > +		phydev->speed = SPEED_2500;
> > +	else if (reg == 0x5)
> > +		phydev->speed = SPEED_1000;
> > +	else if (reg == 0x3)
> > +		phydev->speed = SPEED_100;
> 
> Could we use a switch/case here? 
[S.H] OK.

How about 10Mbits/sec and duplex are we
> guaranteed to be full-duplex at e.g: 100 or 10Mbits/sec?
[S.H] The PHY does not support 10M bits/sec. 
When link to 100M. the phy is full-duplex.

> 
> > +
> > +	return 0;
> > +}
> > +
> > +static struct phy_driver aquantia_driver[] = { {
> > +	.phy_id		= PHY_ID_AQ1202,
> > +	.phy_id_mask	= 0xfffffff0,
> > +	.name		= "Aquantia AQ1202",
> > +	.features	= PHY_GBIT_FEATURES,
> 
> If these are 10GbE PHYs, should not we start defining a new features
> bitmask here to reflect that accordingly? That way MAC
[S.H] there are several defines for 10G PHYs, should be used by a given 10G PHY. 

for this Aquantia PHY, SUPPORTED_10000baseT_Full is a valid define, should I set it as below:
.features	= PHY_GBIT_FEATURES | SUPPORTED_10000baseT_Full,

Or handle the SUPPORTED_10000baseT_Full separately?

Thanks for reviewing!
Shaohui

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] phylib: add driver for aquantia phy
  2015-07-27  8:30   ` Shaohui Xie
@ 2015-07-27 21:49     ` Florian Fainelli
  2015-07-28  2:23       ` Shaohui Xie
  0 siblings, 1 reply; 5+ messages in thread
From: Florian Fainelli @ 2015-07-27 21:49 UTC (permalink / raw)
  To: Shaohui Xie, netdev, davem

On 27/07/15 01:30, Shaohui Xie wrote:
>> -----Original Message-----
>> From: Florian Fainelli [mailto:f.fainelli@gmail.com]
>> Sent: Friday, July 24, 2015 12:39 PM
>> To: shh.xie@gmail.com; netdev@vger.kernel.org; davem@davemloft.net
>> Cc: Xie Shaohui-B21989
>> Subject: Re: [PATCH] phylib: add driver for aquantia phy
>>
>> Le 07/23/15 20:46, shh.xie@gmail.com a écrit :
>>> From: Shaohui Xie <Shaohui.Xie@freescale.com>
>>>
>>> This patch added driver to support Aquantia PHYs AQ1202, AQ2104,
>>> AQR105, AQR405, which accessed through clause 45.
>>
>> Could you prefix your patches with "net: phy: " in the future to be
>> consistent with what is typically used?
> [S.H] OK.
> 
>>
>> See comments below
>>
>>>
>>> Signed-off-by: Shaohui Xie <Shaohui.Xie@freescale.com>
>>> ---
>>
>> [snip]
>>
>>> +static int aquantia_read_status(struct phy_device *phydev) {
>>> +	int reg;
>>> +
>>> +	phydev->speed = SPEED_10000;
>>> +	phydev->duplex = DUPLEX_FULL;
>>> +
>>> +	reg = phy_read_mmd(phydev, MDIO_MMD_AN, MDIO_STAT1);
>>> +	reg = phy_read_mmd(phydev, MDIO_MMD_AN, MDIO_STAT1);
>>> +	if (reg & MDIO_STAT1_LSTATUS)
>>> +		phydev->link = 1;
>>> +	else
>>> +		phydev->link = 0;
>>> +
>>> +	reg = phy_read_mmd(phydev, MDIO_MMD_AN, 0xc800);
>>> +	mdelay(10);
>>> +	reg = phy_read_mmd(phydev, MDIO_MMD_AN, 0xc800);
>>> +	if (reg == 0x9)
>>> +		phydev->speed = SPEED_2500;
>>> +	else if (reg == 0x5)
>>> +		phydev->speed = SPEED_1000;
>>> +	else if (reg == 0x3)
>>> +		phydev->speed = SPEED_100;
>>
>> Could we use a switch/case here? 
> [S.H] OK.
> 
> How about 10Mbits/sec and duplex are we
>> guaranteed to be full-duplex at e.g: 100 or 10Mbits/sec?
> [S.H] The PHY does not support 10M bits/sec. 
> When link to 100M. the phy is full-duplex.

Ok, that means you need to restrict the supported flags accordingly not
to advertise these modes as being supported in the first place, see below:

> 
>>
>>> +
>>> +	return 0;
>>> +}
>>> +
>>> +static struct phy_driver aquantia_driver[] = { {
>>> +	.phy_id		= PHY_ID_AQ1202,
>>> +	.phy_id_mask	= 0xfffffff0,
>>> +	.name		= "Aquantia AQ1202",
>>> +	.features	= PHY_GBIT_FEATURES,
>>
>> If these are 10GbE PHYs, should not we start defining a new features
>> bitmask here to reflect that accordingly? That way MAC
> [S.H] there are several defines for 10G PHYs, should be used by a given 10G PHY. 
> 
> for this Aquantia PHY, SUPPORTED_10000baseT_Full is a valid define, should I set it as below:
> .features	= PHY_GBIT_FEATURES | SUPPORTED_10000baseT_Full,

PHY_GBIT_FEATURES means 10/100/1000 half and full-duplex are supported,
which are not supported as you indicated above, I would go with adding
only the supported modes here, this is really important since this is
the contract between the PHY driver and the Ethernet MAC using it
through the PHY library.

Thanks!
-- 
Florian

^ permalink raw reply	[flat|nested] 5+ messages in thread

* RE: [PATCH] phylib: add driver for aquantia phy
  2015-07-27 21:49     ` Florian Fainelli
@ 2015-07-28  2:23       ` Shaohui Xie
  0 siblings, 0 replies; 5+ messages in thread
From: Shaohui Xie @ 2015-07-28  2:23 UTC (permalink / raw)
  To: Florian Fainelli, netdev, davem

> > for this Aquantia PHY, SUPPORTED_10000baseT_Full is a valid define,
> should I set it as below:
> > .features	= PHY_GBIT_FEATURES | SUPPORTED_10000baseT_Full,
> 
> PHY_GBIT_FEATURES means 10/100/1000 half and full-duplex are supported,
> which are not supported as you indicated above, I would go with adding
> only the supported modes here, this is really important since this is the
> contract between the PHY driver and the Ethernet MAC using it through the
> PHY library.
[S.H] OK. I'll revise the features accordingly.
Thanks.

Shaohui

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2015-07-28  2:23 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-07-24  3:46 [PATCH] phylib: add driver for aquantia phy shh.xie
2015-07-24  4:39 ` Florian Fainelli
2015-07-27  8:30   ` Shaohui Xie
2015-07-27 21:49     ` Florian Fainelli
2015-07-28  2:23       ` Shaohui Xie

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).