Message ID | 1528469132-9161-1-git-send-email-kranke.kirill@gmail.com |
---|---|
State | Deferred, archived |
Delegated to: | David Miller |
Headers | show |
Series | net: phy: Add TJA1100 BroadR-Reach PHY driver. | expand |
On Fri, Jun 08, 2018 at 05:45:32PM +0300, Kirill Kranke wrote: > Current generic PHY driver does not work with TJA1100 BroadR-REACH PHY > properly. TJA1100 does not have any standard ability enabled at MII_BMSR > register. Instead it has BroadR-REACH ability at MII_ESTATUS enabled, which > is not handled by generic driver yet. Therefore generic driver is unable to > guess required link speed, duplex etc. Device is started up with 10Mbps > halfduplex which is incorrect. > > BroadR-REACH able flag is not specified in IEEE802.3-2015. Which is why I > did not add BroadR-REACH able flag support at generic driver. Once > BroadR-REACH able flag gets into IEEE802.3 it should be reasonable to > support it in the generic PHY driver. Hi Kirill Thank for making the changes. It is normal to put 'v2' after PATCH in the subject line. Also, make a brief list of changes since the previous version, after a line with ---. They will get removed when the patch is committed, but help reviewers see what has changed. For network patches, you should also include which tree these patches are for. net-next in this case. See the networking FAQ. > > Signed-off-by: Kirill Kranke <kranke.kirill@gmail.com> > > diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig > index 343989f..7014eb7 100644 > --- a/drivers/net/phy/Kconfig > +++ b/drivers/net/phy/Kconfig > @@ -422,6 +422,14 @@ config TERANETICS_PHY > ---help--- > Currently supports the Teranetics TN2020 > > +config TJA1100_PHY > + tristate "NXP TJA1100 PHY" Please call this NXP_TJA1100_PHY. Putting the vendor first is the general pattern. Are are a few TI drivers which ignore this, but other follow this. This also means moving it up so it comes after NATIONAL_PHY. > + help > + Support of NXP TJA1100 BroadR-REACH ethernet PHY. > + Generic driver is not suitable for TJA1100 PHY while the PHY does not > + advertise any standard IEEE capabilities. It uses BroadR-REACH able > + flag instead. This driver configures capabilities of the PHY properly. > Does 100Base-T1/cause 96 define a way to identify a PHY which implements this? I'm just wondering if we can do this in the generic code, for devices which correctly implement the standard? + > config VITESSE_PHY > tristate "Vitesse PHYs" > ---help--- > diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile > index 5805c0b..4d2a69d 100644 > --- a/drivers/net/phy/Makefile > +++ b/drivers/net/phy/Makefile > @@ -83,5 +83,6 @@ obj-$(CONFIG_ROCKCHIP_PHY) += rockchip.o > obj-$(CONFIG_SMSC_PHY) += smsc.o > obj-$(CONFIG_STE10XP) += ste10Xp.o > obj-$(CONFIG_TERANETICS_PHY) += teranetics.o > +obj-$(CONFIG_TJA1100_PHY) += tja1100.o > obj-$(CONFIG_VITESSE_PHY) += vitesse.o > obj-$(CONFIG_XILINX_GMII2RGMII) += xilinx_gmii2rgmii.o > diff --git a/drivers/net/phy/tja1100.c b/drivers/net/phy/tja1100.c > new file mode 100644 > index 0000000..cddf4d7 > --- /dev/null > +++ b/drivers/net/phy/tja1100.c > @@ -0,0 +1,68 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* tja1100.c: TJA1100 BoardR-REACH PHY driver. > + * > + * Copyright (c) 2017 Kirill Kranke <kirill.kranke@gmail.com> > + * Author: Kirill Kranke <kirill.kranke@gmail.com> > + */ > + > +#include <linux/kernel.h> > +#include <linux/module.h> > +#include <linux/phy.h> > + > +static int tja1100_phy_config_init(struct phy_device *phydev) > +{ > + phydev->autoneg = AUTONEG_DISABLE; > + phydev->speed = SPEED_100; > + phydev->duplex = DUPLEX_FULL; > + > + return 0; > +} > + > +static int tja1100_phy_config_aneg(struct phy_device *phydev) > +{ > + if (phydev->autoneg == AUTONEG_ENABLE) { > + phydev_err(phydev, "autonegotiation is not supported\n"); > + return -EINVAL; > + } > + > + if (phydev->speed != SPEED_100 || phydev->duplex != DUPLEX_FULL) { > + phydev_err(phydev, "only 100MBps Full Duplex allowed\n"); > + return -EINVAL; > + } > + > + return 0; > +} > + > +static struct phy_driver tja1100_phy_driver[] = { > + { > + .phy_id = 0x0180dc48, > + .phy_id_mask = 0xfffffff0, > + .name = "NXP TJA1100", > + > + /* TJA1100 has only 100BASE-BroadR-REACH ability specified > + * at MII_ESTATUS register. Standard modes are not > + * supported. Therefore BroadR-REACH allow only 100Mbps > + * full duplex without autoneg. > + */ > + .features = SUPPORTED_100baseT_Full | SUPPORTED_MII, This is the second T1 driver we have had recently. It might make sense to add a PHY_T1_FEATURES macro the include/linux/phy.h Don't you also want SUPPORTED_TP? Andrew
Hi Andrew. Thanks for your comments. I will update the patch a bit later. > > Does 100Base-T1/cause 96 define a way to identify a PHY which > implements this? I'm just wondering if we can do this in the generic > code, for devices which correctly implement the standard? > Well, I did research IEEE 802.3 standards before implementing the Patch. Initially I wanted to update generic phy driver. I did not find a way to identify 100Base-T1 PHY using Clause 22 MDIO. This section is completely missing at IEEE 802.3bw, which describe 100Base-T1. There are some updates to Clause 45 registers at IEEE 802.3bw. They add "BASE-T1 PMA/PMD extended ability" to PMA/PMD registers. At Clause 96 they state following: "The MDIO capability described in Clause 45 defines several variables that provide control and status information for and about the PMA and PCS." In the same time I have played with a two different 100Base-T1 PHYs. Both use different Clause 22 registers to advertise their abilities, both are incompatible. None use Clause 45 for this purpose. It seems that this is going to be 100Base-T1 mess while IEEE 802.3bw miss Clause 22 updates. Clause 45 is rarely used from my experience. Probably IEEE expected 100Base-T1 PHYs to go for Clause 45 MDIO and this did not work so far. > > This is the second T1 driver we have had recently. It might make sense to add a > PHY_T1_FEATURES macro the include/linux/phy.h > This seems reasonable, indeed. > > Don't you also want SUPPORTED_TP? > True, I will add SUPPORTED_TP in next revision of the Patch. Kirill
> It seems that this is going to be 100Base-T1 mess while IEEE 802.3bw > miss Clause 22 updates. Clause 45 is rarely used from my experience. Probably > IEEE expected 100Base-T1 PHYs to go for Clause 45 MDIO and this did not work > so far. Hi Kirill Thanks for this information. So lets forget generic support for the moment. Andrew
diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig index 343989f..7014eb7 100644 --- a/drivers/net/phy/Kconfig +++ b/drivers/net/phy/Kconfig @@ -422,6 +422,14 @@ config TERANETICS_PHY ---help--- Currently supports the Teranetics TN2020 +config TJA1100_PHY + tristate "NXP TJA1100 PHY" + help + Support of NXP TJA1100 BroadR-REACH ethernet PHY. + Generic driver is not suitable for TJA1100 PHY while the PHY does not + advertise any standard IEEE capabilities. It uses BroadR-REACH able + flag instead. This driver configures capabilities of the PHY properly. + config VITESSE_PHY tristate "Vitesse PHYs" ---help--- diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile index 5805c0b..4d2a69d 100644 --- a/drivers/net/phy/Makefile +++ b/drivers/net/phy/Makefile @@ -83,5 +83,6 @@ obj-$(CONFIG_ROCKCHIP_PHY) += rockchip.o obj-$(CONFIG_SMSC_PHY) += smsc.o obj-$(CONFIG_STE10XP) += ste10Xp.o obj-$(CONFIG_TERANETICS_PHY) += teranetics.o +obj-$(CONFIG_TJA1100_PHY) += tja1100.o obj-$(CONFIG_VITESSE_PHY) += vitesse.o obj-$(CONFIG_XILINX_GMII2RGMII) += xilinx_gmii2rgmii.o diff --git a/drivers/net/phy/tja1100.c b/drivers/net/phy/tja1100.c new file mode 100644 index 0000000..cddf4d7 --- /dev/null +++ b/drivers/net/phy/tja1100.c @@ -0,0 +1,68 @@ +// SPDX-License-Identifier: GPL-2.0 +/* tja1100.c: TJA1100 BoardR-REACH PHY driver. + * + * Copyright (c) 2017 Kirill Kranke <kirill.kranke@gmail.com> + * Author: Kirill Kranke <kirill.kranke@gmail.com> + */ + +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/phy.h> + +static int tja1100_phy_config_init(struct phy_device *phydev) +{ + phydev->autoneg = AUTONEG_DISABLE; + phydev->speed = SPEED_100; + phydev->duplex = DUPLEX_FULL; + + return 0; +} + +static int tja1100_phy_config_aneg(struct phy_device *phydev) +{ + if (phydev->autoneg == AUTONEG_ENABLE) { + phydev_err(phydev, "autonegotiation is not supported\n"); + return -EINVAL; + } + + if (phydev->speed != SPEED_100 || phydev->duplex != DUPLEX_FULL) { + phydev_err(phydev, "only 100MBps Full Duplex allowed\n"); + return -EINVAL; + } + + return 0; +} + +static struct phy_driver tja1100_phy_driver[] = { + { + .phy_id = 0x0180dc48, + .phy_id_mask = 0xfffffff0, + .name = "NXP TJA1100", + + /* TJA1100 has only 100BASE-BroadR-REACH ability specified + * at MII_ESTATUS register. Standard modes are not + * supported. Therefore BroadR-REACH allow only 100Mbps + * full duplex without autoneg. + */ + .features = SUPPORTED_100baseT_Full | SUPPORTED_MII, + + .config_aneg = tja1100_phy_config_aneg, + .config_init = tja1100_phy_config_init, + + .suspend = genphy_suspend, + .resume = genphy_resume, + } +}; + +module_phy_driver(tja1100_phy_driver); + +MODULE_DESCRIPTION("NXP TJA1100 driver"); +MODULE_AUTHOR("Kirill Kranke <kkranke@topcon.com>"); +MODULE_LICENSE("GPL"); + +static struct mdio_device_id __maybe_unused nxp_tbl[] = { + { 0x0180dc48, 0xfffffff0 }, + {} +}; + +MODULE_DEVICE_TABLE(mdio, nxp_tbl);
Current generic PHY driver does not work with TJA1100 BroadR-REACH PHY properly. TJA1100 does not have any standard ability enabled at MII_BMSR register. Instead it has BroadR-REACH ability at MII_ESTATUS enabled, which is not handled by generic driver yet. Therefore generic driver is unable to guess required link speed, duplex etc. Device is started up with 10Mbps halfduplex which is incorrect. BroadR-REACH able flag is not specified in IEEE802.3-2015. Which is why I did not add BroadR-REACH able flag support at generic driver. Once BroadR-REACH able flag gets into IEEE802.3 it should be reasonable to support it in the generic PHY driver. Signed-off-by: Kirill Kranke <kranke.kirill@gmail.com>