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.kir...@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.kra...@gmail.com>
> + * Author: Kirill Kranke <kirill.kra...@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
