RE: [PATCH] phylib: add driver for aquantia phy
> > for this Aquantia PHY, SUPPORTED_1baseT_Full is a valid define,
> should I set it as below:
> > .features = PHY_GBIT_FEATURES | SUPPORTED_1baseT_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
N�r��yb�X��ǧv�^�){.n�+���z�^�)w* jg��� �ݢj/���z�ޖ��2�ޙ&�)ߡ�a���� �G���h��j:+v���w��٥
Re: [PATCH] phylib: add driver for aquantia phy
On 27/07/15 01:30, Shaohui Xie wrote:
>> -Original Message-
>> From: Florian Fainelli [mailto:f.faine...@gmail.com]
>> Sent: Friday, July 24, 2015 12:39 PM
>> To: shh@gmail.com; netdev@vger.kernel.org; da...@davemloft.net
>> Cc: Xie Shaohui-B21989
>> Subject: Re: [PATCH] phylib: add driver for aquantia phy
>>
>> Le 07/23/15 20:46, shh@gmail.com a écrit :
>>> From: Shaohui Xie
>>>
>>> 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
>>> ---
>>
>> [snip]
>>
>>> +static int aquantia_read_status(struct phy_device *phydev) {
>>> + int reg;
>>> +
>>> + phydev->speed = SPEED_1;
>>> + 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= 0xfff0,
>>> + .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_1baseT_Full is a valid define, should I
> set it as below:
> .features = PHY_GBIT_FEATURES | SUPPORTED_1baseT_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
--
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] phylib: add driver for aquantia phy
> -Original Message-
> From: Florian Fainelli [mailto:f.faine...@gmail.com]
> Sent: Friday, July 24, 2015 12:39 PM
> To: shh@gmail.com; netdev@vger.kernel.org; da...@davemloft.net
> Cc: Xie Shaohui-B21989
> Subject: Re: [PATCH] phylib: add driver for aquantia phy
>
> Le 07/23/15 20:46, shh@gmail.com a écrit :
> > From: Shaohui Xie
> >
> > 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
> > ---
>
> [snip]
>
> > +static int aquantia_read_status(struct phy_device *phydev) {
> > + int reg;
> > +
> > + phydev->speed = SPEED_1;
> > + 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= 0xfff0,
> > + .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_1baseT_Full is a valid define, should I
set it as below:
.features = PHY_GBIT_FEATURES | SUPPORTED_1baseT_Full,
Or handle the SUPPORTED_1baseT_Full separately?
Thanks for reviewing!
Shaohui
Re: [PATCH] phylib: add driver for aquantia phy
Le 07/23/15 20:46, shh@gmail.com a écrit :
> From: Shaohui Xie
>
> 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
> ---
[snip]
> +static int aquantia_read_status(struct phy_device *phydev)
> +{
> + int reg;
> +
> + phydev->speed = SPEED_1;
> + 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= 0xfff0,
> + .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= 0xfff0,
> + .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= 0xfff0,
> + .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= 0xfff0,
> + .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, 0xfff0 },
> + { PHY_ID_AQ2104, 0xfff0 },
> + { PHY_ID_AQR105, 0xfff0 },
> + { PHY_ID_AQR405, 0xfff0 },
> + { }
> +};
> +
> +MODULE_DEVICE_TABLE(mdio, aquantia_tbl);
> +
> +MODULE_DESCRIPTION("Aquantia PHY driver");
> +MODULE_AUTHOR("Shaohui Xie ");
> +MODULE_LICENSE("GPL v2");
>
--
Florian
--
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] phylib: add driver for aquantia phy
From: Shaohui Xie
This patch added driver to support Aquantia PHYs AQ1202, AQ2104, AQR105,
AQR405, which accessed through clause 45.
Signed-off-by: Shaohui Xie
---
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 000..cb848f95
--- /dev/null
+++ b/drivers/net/phy/aquantia.c
@@ -0,0 +1,157 @@
+/*
+ * Driver for Aquantia PHY
+ *
+ * Author: Shaohui Xie
+ *
+ * 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
+#include
+#include
+#include
+#include
+#include
+#include
+
+#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_1baseT_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_1;
+ 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= 0xfff0,
+ .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= 0xfff0,
+ .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= 0xfff0,
+ .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= 0xfff0,
+ .name = "Aquantia AQR405",
+
