On Mon, Mar 09, 2020 at 08:45:50PM +0100, Heiner Kallweit wrote: > On 09.03.2020 08:40, Oleksij Rempel wrote: > > TJA1102 is an dual T1 PHY chip. Both PHYs are separately addressable. > > PHY 0 can be identified by PHY ID. PHY 1 has no PHY ID and can be > > configured in device tree by setting compatible = "ethernet-phy-id0180.dc81". > > > > PHY 1 has less supported registers and functionality. For current driver > > it will affect only the HWMON support. > > > > Signed-off-by: Oleksij Rempel > > --- > > drivers/net/phy/nxp-tja11xx.c | 102 ++++++++++++++++++++++++++++++++++ > > 1 file changed, 102 insertions(+) > > > > diff --git a/drivers/net/phy/nxp-tja11xx.c b/drivers/net/phy/nxp-tja11xx.c > > index b705d0bd798b..f79c9aa051ed 100644 > > --- a/drivers/net/phy/nxp-tja11xx.c > > +++ b/drivers/net/phy/nxp-tja11xx.c > > @@ -15,6 +15,7 @@ > > #define PHY_ID_MASK 0xfffffff0 > > #define PHY_ID_TJA1100 0x0180dc40 > > #define PHY_ID_TJA1101 0x0180dd00 > > +#define PHY_ID_TJA1102 0x0180dc80 > > > > #define MII_ECTRL 17 > > #define MII_ECTRL_LINK_CONTROL BIT(15) > > @@ -40,6 +41,10 @@ > > #define MII_INTSRC_TEMP_ERR BIT(1) > > #define MII_INTSRC_UV_ERR BIT(3) > > > > +#define MII_INTEN 22 > > +#define MII_INTEN_LINK_FAIL BIT(10) > > +#define MII_INTEN_LINK_UP BIT(9) > > + > > #define MII_COMMSTAT 23 > > #define MII_COMMSTAT_LINK_UP BIT(15) > > > > @@ -190,6 +195,7 @@ static int tja11xx_config_init(struct phy_device *phydev) > > return ret; > > break; > > case PHY_ID_TJA1101: > > + case PHY_ID_TJA1102: > > ret = phy_set_bits(phydev, MII_COMMCFG, MII_COMMCFG_AUTO_OP); > > if (ret) > > return ret; > > @@ -354,6 +360,66 @@ static int tja11xx_probe(struct phy_device *phydev) > > return PTR_ERR_OR_ZERO(priv->hwmon_dev); > > } > > > > +static int tja1102_match_phy_device(struct phy_device *phydev, bool port0) > > +{ > > + int ret; > > + > > + if ((phydev->phy_id & PHY_ID_MASK) != PHY_ID_TJA1102) > > For port 1 you rely on DT forcing the appropriate phy_id > (else it would be 0 and port 1 wouldn't be matched). > This is worth a describing comment. There is a second patch which will do it automatically, no need to force the PHY ID in the devicetree. > > + return 0; > > + > > + ret = phy_read(phydev, MII_PHYSID2); > > + if (ret < 0) > > + return ret; > > + > > + /* TJA1102 Port 1 has phyid 0 and doesn't support temperature > > + * and undervoltage alarms. > > + */ > > + if (port0) > > + return ret ? 1 : 0; > > + > > + return !ret; > > +} > > + > > +static int tja1102_p0_match_phy_device(struct phy_device *phydev) > > +{ > > + return tja1102_match_phy_device(phydev, true); > > +} > > + > > +static int tja1102_p1_match_phy_device(struct phy_device *phydev) > > +{ > > + return tja1102_match_phy_device(phydev, false); > > +} > > + > > +static int tja11xx_ack_interrupt(struct phy_device *phydev) > > +{ > > + int ret; > > + > > + ret = phy_read(phydev, MII_INTSRC); > > + > > + return (ret < 0) ? ret : 0; > > +} > > + > > +static int tja11xx_config_intr(struct phy_device *phydev) > > +{ > > + int value; > > + int ret; > > + > > + value = phy_read(phydev, MII_INTEN); > > + if (value < 0) > > + return value; > > + > > + if (phydev->interrupts == PHY_INTERRUPT_ENABLED) { > > + value |= MII_INTEN_LINK_FAIL; > > + value |= MII_INTEN_LINK_UP; > > + > > This may leave unwanted interrupt sources active. Why not > simply setting a fixed value like in the else clause? done > > + ret = phy_write(phydev, MII_INTEN, value); > > + } > > + else > > Kernel style: > Closing brace and else belong to one line. And the else clause > needs braces too. checkpatch.pl should complain here. done > > + ret = phy_write(phydev, MII_INTEN, 0); > > + > > + return ret; > > +} > > + > > static struct phy_driver tja11xx_driver[] = { > > { > > PHY_ID_MATCH_MODEL(PHY_ID_TJA1100), > > @@ -385,6 +451,41 @@ static struct phy_driver tja11xx_driver[] = { > > .get_sset_count = tja11xx_get_sset_count, > > .get_strings = tja11xx_get_strings, > > .get_stats = tja11xx_get_stats, > > + }, { > > + .name = "NXP TJA1102 Port 0", > > + .features = PHY_BASIC_T1_FEATURES, > > + .probe = tja11xx_probe, > > + .soft_reset = tja11xx_soft_reset, > > + .config_init = tja11xx_config_init, > > + .read_status = tja11xx_read_status, > > + .match_phy_device = tja1102_p0_match_phy_device, > > + .suspend = genphy_suspend, > > + .resume = genphy_resume, > > + .set_loopback = genphy_loopback, > > + /* Statistics */ > > + .get_sset_count = tja11xx_get_sset_count, > > + .get_strings = tja11xx_get_strings, > > + .get_stats = tja11xx_get_stats, > > + .ack_interrupt = tja11xx_ack_interrupt, > > + .config_intr = tja11xx_config_intr, > > + > > + }, { > > + .name = "NXP TJA1102 Port 1", > > + .features = PHY_BASIC_T1_FEATURES, > > + /* currently no probe for Port 1 is need */ > > + .soft_reset = tja11xx_soft_reset, > > + .config_init = tja11xx_config_init, > > + .read_status = tja11xx_read_status, > > + .match_phy_device = tja1102_p1_match_phy_device, > > + .suspend = genphy_suspend, > > + .resume = genphy_resume, > > + .set_loopback = genphy_loopback, > > + /* Statistics */ > > + .get_sset_count = tja11xx_get_sset_count, > > + .get_strings = tja11xx_get_strings, > > + .get_stats = tja11xx_get_stats, > > + .ack_interrupt = tja11xx_ack_interrupt, > > + .config_intr = tja11xx_config_intr, > > } > > }; > > > > @@ -393,6 +494,7 @@ module_phy_driver(tja11xx_driver); > > static struct mdio_device_id __maybe_unused tja11xx_tbl[] = { > > { PHY_ID_MATCH_MODEL(PHY_ID_TJA1100) }, > > { PHY_ID_MATCH_MODEL(PHY_ID_TJA1101) }, > > + { PHY_ID_MATCH_MODEL(PHY_ID_TJA1102) }, > > { } > > }; > > > > > > > -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |