| Message ID | 20180419082816.109338-1-mans.andersson@nibe.se |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
| Series | net: phy: TLK10X initial driver submission | expand |
On Thu, Apr 19, 2018 at 10:28:16AM +0200, Måns Andersson wrote: > From: Mans Andersson <mans.andersson@nibe.se> > > Add suport for the TI TLK105 and TLK106 10/100Mbit ethernet phys. > > In addition the TLK10X needs to be removed from DP83848 driver as the > power back off support is added here for this device. > > Datasheet: > http://www.ti.com/lit/gpn/tlk106 > --- > .../devicetree/bindings/net/ti,tlk10x.txt | 27 +++ > drivers/net/phy/Kconfig | 5 + > drivers/net/phy/Makefile | 1 + > drivers/net/phy/dp83848.c | 3 - > drivers/net/phy/tlk10x.c | 209 +++++++++++++++++++++ > 5 files changed, 242 insertions(+), 3 deletions(-) > create mode 100644 Documentation/devicetree/bindings/net/ti,tlk10x.txt > create mode 100644 drivers/net/phy/tlk10x.c > > diff --git a/Documentation/devicetree/bindings/net/ti,tlk10x.txt b/Documentation/devicetree/bindings/net/ti,tlk10x.txt > new file mode 100644 > index 0000000..371d0d7 > --- /dev/null > +++ b/Documentation/devicetree/bindings/net/ti,tlk10x.txt > @@ -0,0 +1,27 @@ > +* Texas Instruments - TLK105 / TLK106 ethernet PHYs > + > +Required properties: > + - reg - The ID number for the phy, usually a small integer > + > +Optional properties: > + - ti,power-back-off - Power Back Off Level > + Please refer to data sheet chapter 8.6 and TI Application > + Note SLLA3228 > + 0 - Normal Operation > + 1 - Level 1 (up to 140m cable between TLK link partners) > + 2 - Level 2 (up to 100m cable between TLK link partners) > + 3 - Level 3 (up to 80m cable between TLK link partners) Hi Måns Device tree is all about board properties. In most cases, power back off is not a board properties, since it depends on the cable length and the peer board. If however, your board has two PHYs back to back, say to connect to an Ethernet switch, that would be a valid board property. How are you using this? I know of others who would like such a configuration. Marvell PHYs can do something similar. I've always suggested adding a PHY tunable. Pass the cable length in meters and let the PHY driver pick the nearest it can do, rounding up. The Marvell PHYs also support measuring the cable length as part of the cable diagnostics. So it would be good to reserve a configuration value to mean 'auto' - measure the cable and then pick the best power back off. Quickly scanning the data sheet, i see that this PHY also has the ability to measure the cable length. > +static int tlk10x_read(struct phy_device *phydev, int reg) > +{ > + if (reg & ~0x1f) { > + /* Extended register */ > + phy_write(phydev, TLK10X_REGCR, 0x001F); > + phy_write(phydev, TLK10X_ADDAR, reg); > + phy_write(phydev, TLK10X_REGCR, 0x401F); > + reg = TLK10X_ADDAR; > + } > + > + return phy_read(phydev, reg); > +} > + > +static int tlk10x_write(struct phy_device *phydev, int reg, int val) > +{ > + if (reg & ~0x1f) { > + /* Extended register */ > + phy_write(phydev, TLK10X_REGCR, 0x001F); > + phy_write(phydev, TLK10X_ADDAR, reg); > + phy_write(phydev, TLK10X_REGCR, 0x401F); > + reg = TLK10X_ADDAR; > + } > + > + return phy_write(phydev, reg, val); > +} This looks to be phy_read_mmd() and phy_write_mmd(). If so, please use them, they get the locking correct. > +#ifdef CONFIG_OF_MDIO > +static int tlk10x_of_init(struct phy_device *phydev) > +{ > + struct tlk10x_private *tlk10x = phydev->priv; > + struct device *dev = &phydev->mdio.dev; > + struct device_node *of_node = dev->of_node; > + int ret; > + > + if (!of_node) > + return 0; > + > + ret = of_property_read_u32(of_node, "ti,power-back-off", > + &tlk10x->pwrbo_level); > + if (ret) { > + dev_err(dev, "missing ti,power-back-off property"); > + tlk10x->pwrbo_level = 0; > + } If we decide to accept this, you should do range checking, and return -EINVAL if the value is out of range. > +static int tlk10x_config_init(struct phy_device *phydev) > +{ > + int ret, reg; > + struct tlk10x_private *tlk10x; > + > + ret = genphy_config_init(phydev); > + if (ret < 0) > + return ret; > + > + if (!phydev->priv) { > + tlk10x = devm_kzalloc(&phydev->mdio.dev, sizeof(*tlk10x), > + GFP_KERNEL); > + if (!tlk10x) > + return -ENOMEM; > + > + phydev->priv = tlk10x; > + ret = tlk10x_of_init(phydev); > + if (ret) > + return ret; > + } else { > + tlk10x = (struct tlk10x_private *)phydev->priv; > + } This allocation should be done in .probe > + > + // Power back off > + if (tlk10x->pwrbo_level < 0 || tlk10x->pwrbo_level > 3) > + tlk10x->pwrbo_level = 0; > + reg = tlk10x_read(phydev, TLK10X_PWRBOCR); > + reg = ((reg & ~TLK10X_PWRBOCR_MASK) > + | (tlk10x->pwrbo_level << 6)); > + ret = tlk10x_write(phydev, TLK10X_PWRBOCR, reg); > + if (ret < 0) { > + dev_err(&phydev->mdio.dev, > + "unable to set power back-off (err=%d)\n", ret); > + return ret; > + } > + dev_info(&phydev->mdio.dev, "power back-off set to level %d\n", > + tlk10x->pwrbo_level); > + > + return 0; > +} Andrew
On Thu, Apr 19, 2018 at 10:28 AM, Måns Andersson <mans.andersson@nibe.se> wrote: > From: Mans Andersson <mans.andersson@nibe.se> > > Add suport for the TI TLK105 and TLK106 10/100Mbit ethernet phys. > Hi Mans, Some quick notes. > In addition the TLK10X needs to be removed from DP83848 driver as the > power back off support is added here for this device. > > Datasheet: > http://www.ti.com/lit/gpn/tlk106 Missing signature. > --- > .../devicetree/bindings/net/ti,tlk10x.txt | 27 +++ > drivers/net/phy/Kconfig | 5 + > drivers/net/phy/Makefile | 1 + > drivers/net/phy/dp83848.c | 3 - > drivers/net/phy/tlk10x.c | 209 +++++++++++++++++++++ > 5 files changed, 242 insertions(+), 3 deletions(-) > create mode 100644 Documentation/devicetree/bindings/net/ti,tlk10x.txt > create mode 100644 drivers/net/phy/tlk10x.c > > diff --git a/Documentation/devicetree/bindings/net/ti,tlk10x.txt b/Documentation/devicetree/bindings/net/ti,tlk10x.txt > new file mode 100644 > index 0000000..371d0d7 > --- /dev/null > +++ b/Documentation/devicetree/bindings/net/ti,tlk10x.txt > @@ -0,0 +1,27 @@ > +* Texas Instruments - TLK105 / TLK106 ethernet PHYs > + > +Required properties: > + - reg - The ID number for the phy, usually a small integer > + > +Optional properties: > + - ti,power-back-off - Power Back Off Level > + Please refer to data sheet chapter 8.6 and TI Application > + Note SLLA3228 > + 0 - Normal Operation > + 1 - Level 1 (up to 140m cable between TLK link partners) > + 2 - Level 2 (up to 100m cable between TLK link partners) > + 3 - Level 3 (up to 80m cable between TLK link partners) > + > +Default child nodes are standard Ethernet PHY device > +nodes as described in Documentation/devicetree/bindings/net/phy.txt > + > +Example: > + > + ethernet-phy@0 { > + reg = <0>; > + ti,power-back-off = <2>; > + }; > + > +Datasheets and documentation can be found at: > +http://www.ti.com/lit/gpn/tlk106 > +http://www.ti.com/lit/an/slla328/slla328.pdf > diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig > index bdfbabb..c980240 100644 > --- a/drivers/net/phy/Kconfig > +++ b/drivers/net/phy/Kconfig > @@ -295,6 +295,11 @@ config DP83867_PHY > ---help--- > Currently supports the DP83867 PHY. > > +config TLK10X_PHY > + tristate "Texas Instruments TLK10x PHY" > + ---help--- > + Supports the TLK105 and TLK106 PHYs. > + > config FIXED_PHY > tristate "MDIO Bus/PHY emulation with fixed speed/link PHYs" > depends on PHYLIB > diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile > index 01acbcb..37e4e02 100644 > --- a/drivers/net/phy/Makefile > +++ b/drivers/net/phy/Makefile > @@ -79,5 +79,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_TLK10X_PHY) += tlk10x.o > obj-$(CONFIG_VITESSE_PHY) += vitesse.o > obj-$(CONFIG_XILINX_GMII2RGMII) += xilinx_gmii2rgmii.o > diff --git a/drivers/net/phy/dp83848.c b/drivers/net/phy/dp83848.c > index cd09c3a..435f401 100644 > --- a/drivers/net/phy/dp83848.c > +++ b/drivers/net/phy/dp83848.c > @@ -19,7 +19,6 @@ > #define TI_DP83848C_PHY_ID 0x20005ca0 > #define TI_DP83620_PHY_ID 0x20005ce0 > #define NS_DP83848C_PHY_ID 0x20005c90 > -#define TLK10X_PHY_ID 0x2000a210 > > /* Registers */ > #define DP83848_MICR 0x11 /* MII Interrupt Control Register */ > @@ -78,7 +77,6 @@ static struct mdio_device_id __maybe_unused dp83848_tbl[] = { > { TI_DP83848C_PHY_ID, 0xfffffff0 }, > { NS_DP83848C_PHY_ID, 0xfffffff0 }, > { TI_DP83620_PHY_ID, 0xfffffff0 }, > - { TLK10X_PHY_ID, 0xfffffff0 }, > { } > }; > MODULE_DEVICE_TABLE(mdio, dp83848_tbl); > @@ -105,7 +103,6 @@ static struct phy_driver dp83848_driver[] = { > DP83848_PHY_DRIVER(TI_DP83848C_PHY_ID, "TI DP83848C 10/100 Mbps PHY"), > DP83848_PHY_DRIVER(NS_DP83848C_PHY_ID, "NS DP83848C 10/100 Mbps PHY"), > DP83848_PHY_DRIVER(TI_DP83620_PHY_ID, "TI DP83620 10/100 Mbps PHY"), > - DP83848_PHY_DRIVER(TLK10X_PHY_ID, "TI TLK10X 10/100 Mbps PHY"), > }; > module_phy_driver(dp83848_driver); > > diff --git a/drivers/net/phy/tlk10x.c b/drivers/net/phy/tlk10x.c > new file mode 100644 > index 0000000..1efc81e > --- /dev/null > +++ b/drivers/net/phy/tlk10x.c > @@ -0,0 +1,209 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/** > + * Driver for the Texas Instruments TLK105 / TLK106 > + * > + * Copyright (C) 2018 NIBE Industrier AB - http://www.nibe.com > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation; either version 2 of the License. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. Since you are using the SPDX id, please remove the license text (which is actually wrong: it seems you have cut the v2+ version and then removed the last sentence of the first paragraph? :-). > + */ > + > +#include <linux/module.h> > +#include <linux/phy.h> > +#include <linux/of.h> > + > +#define TLK10X_PHY_ID 0x2000a210 > + > +/* Registers */ > +#define TLK10X_MICR 0x11 /* MII Interrupt Control Reg */ > +#define TLK10X_MISR 0x12 /* MII Interrupt Status Reg */ > +#define TLK10X_REGCR 0x0d /* Register Control Register */ > +#define TLK10X_ADDAR 0x0e /* Data Register */ > +#define TLK10X_PWRBOCR 0xae /* Power Backoff Register */ > + > +/* MICR Register Fields */ > +#define TLK10X_MICR_INT_OE BIT(0) /* Interrupt Output Enable */ > +#define TLK10X_MICR_INTEN BIT(1) /* Interrupt Enable */ > + > +/* MISR Register Fields */ > +#define TLK10X_MISR_RHF_INT_EN BIT(0) /* Receive Error Counter */ > +#define TLK10X_MISR_FHF_INT_EN BIT(1) /* False Carrier Counter */ > +#define TLK10X_MISR_ANC_INT_EN BIT(2) /* Auto-negotiation complete */ > +#define TLK10X_MISR_DUP_INT_EN BIT(3) /* Duplex Status */ > +#define TLK10X_MISR_SPD_INT_EN BIT(4) /* Speed status */ > +#define TLK10X_MISR_LINK_INT_EN BIT(5) /* Link status */ > +#define TLK10X_MISR_ED_INT_EN BIT(6) /* Energy detect */ > +#define TLK10X_MISR_LQM_INT_EN BIT(7) /* Link Quality Monitor */ > + > +/* PWRBOCR Register Fields */ > +#define TLK10X_PWRBOCR_MASK 0xe0 /* Power Backoff Mask */ > + > +#define TLK10X_INT_EN_MASK \ > + (TLK10X_MISR_ANC_INT_EN | \ > + TLK10X_MISR_DUP_INT_EN | \ > + TLK10X_MISR_SPD_INT_EN | \ > + TLK10X_MISR_LINK_INT_EN) > + > +struct tlk10x_private { > + int pwrbo_level; > +}; > + > +static int tlk10x_read(struct phy_device *phydev, int reg) > +{ > + if (reg & ~0x1f) { 0x1f or ~0x1f should probably have a #define name. > + /* Extended register */ > + phy_write(phydev, TLK10X_REGCR, 0x001F); > + phy_write(phydev, TLK10X_ADDAR, reg); > + phy_write(phydev, TLK10X_REGCR, 0x401F); > + reg = TLK10X_ADDAR; > + } > + > + return phy_read(phydev, reg); > +} > + > +static int tlk10x_write(struct phy_device *phydev, int reg, int val) > +{ > + if (reg & ~0x1f) { Ditto. > + /* Extended register */ > + phy_write(phydev, TLK10X_REGCR, 0x001F); > + phy_write(phydev, TLK10X_ADDAR, reg); > + phy_write(phydev, TLK10X_REGCR, 0x401F); > + reg = TLK10X_ADDAR; > + } > + > + return phy_write(phydev, reg, val); > +} > + > +#ifdef CONFIG_OF_MDIO Maybe you want the #ifdef inside. > +static int tlk10x_of_init(struct phy_device *phydev) > +{ > + struct tlk10x_private *tlk10x = phydev->priv; > + struct device *dev = &phydev->mdio.dev; > + struct device_node *of_node = dev->of_node; > + int ret; > + > + if (!of_node) > + return 0; > + > + ret = of_property_read_u32(of_node, "ti,power-back-off", > + &tlk10x->pwrbo_level); > + if (ret) { > + dev_err(dev, "missing ti,power-back-off property"); > + tlk10x->pwrbo_level = 0; > + } > + > + return 0; > +} > +#else > +static int tlk10x_of_init(struct phy_device *phydev) > +{ > + return 0; > +} > +#endif /* CONFIG_OF_MDIO */ > + > +static int tlk10x_config_init(struct phy_device *phydev) > +{ > + int ret, reg; > + struct tlk10x_private *tlk10x; > + > + ret = genphy_config_init(phydev); > + if (ret < 0) > + return ret; > + > + if (!phydev->priv) { > + tlk10x = devm_kzalloc(&phydev->mdio.dev, sizeof(*tlk10x), > + GFP_KERNEL); > + if (!tlk10x) > + return -ENOMEM; > + > + phydev->priv = tlk10x; > + ret = tlk10x_of_init(phydev); > + if (ret) > + return ret; > + } else { > + tlk10x = (struct tlk10x_private *)phydev->priv; > + } > + > + // Power back off > + if (tlk10x->pwrbo_level < 0 || tlk10x->pwrbo_level > 3) > + tlk10x->pwrbo_level = 0; > + reg = tlk10x_read(phydev, TLK10X_PWRBOCR); > + reg = ((reg & ~TLK10X_PWRBOCR_MASK) > + | (tlk10x->pwrbo_level << 6)); Maybe the 6 should have a name, or maybe a bigger macro for this would clarify. > + ret = tlk10x_write(phydev, TLK10X_PWRBOCR, reg); > + if (ret < 0) { > + dev_err(&phydev->mdio.dev, > + "unable to set power back-off (err=%d)\n", ret); > + return ret; > + } > + dev_info(&phydev->mdio.dev, "power back-off set to level %d\n", > + tlk10x->pwrbo_level); > + > + return 0; > +} > + > +static int tlk10x_ack_interrupt(struct phy_device *phydev) > +{ > + int err = tlk10x_read(phydev, TLK10X_MISR); Following the style of the rest of the file, shouldn't this be: if (err < 0) return err; return 0; ? > + > + return err < 0 ? err : 0; > +} > + > +static int tlk10x_config_intr(struct phy_device *phydev) > +{ > + int control, ret; > + > + control = tlk10x_read(phydev, TLK10X_MICR); > + if (control < 0) > + return control; > + > + if (phydev->interrupts == PHY_INTERRUPT_ENABLED) { > + control |= TLK10X_MICR_INT_OE; > + control |= TLK10X_MICR_INTEN; > + > + ret = tlk10x_write(phydev, TLK10X_MISR, TLK10X_INT_EN_MASK); > + if (ret < 0) > + return ret; > + } else { > + control &= ~TLK10X_MICR_INTEN; > + } > + > + return tlk10x_write(phydev, TLK10X_MICR, control); > +} > + > +static struct phy_driver tlk10x_driver[] = { > + { > + .phy_id = TLK10X_PHY_ID, > + .phy_id_mask = 0xfffffff0, > + .name = "TI TLK10X 10/100 Mbps PHY", > + .features = PHY_BASIC_FEATURES, > + .flags = PHY_HAS_INTERRUPT, > + > + .config_init = tlk10x_config_init, > + .soft_reset = genphy_soft_reset, > + > + /* IRQ related */ > + .ack_interrupt = tlk10x_ack_interrupt, > + .config_intr = tlk10x_config_intr, > + > + .suspend = genphy_suspend, > + .resume = genphy_resume, > + }, > +}; > +module_phy_driver(tlk10x_driver); > + > +static struct mdio_device_id __maybe_unused tlk10x_tbl[] = { > + { TLK10X_PHY_ID, 0xfffffff0 }, > + { } > +}; > +MODULE_DEVICE_TABLE(mdio, tlk10x_tbl); > + > +MODULE_DESCRIPTION("Texas Instruments TLK105 / TLK106 PHY driver"); > +MODULE_AUTHOR("Mans Andersson <mans.andersson@nibe.se>"); > +MODULE_LICENSE("GPL"); Should be "GPL v2". Cheers, Miguel
On Thu, Apr 19, 2018 at 02:09:02PM +0200, Andrew Lunn wrote: > On Thu, Apr 19, 2018 at 10:28:16AM +0200, Måns Andersson wrote: > > From: Mans Andersson <mans.andersson@nibe.se> > > > > Add suport for the TI TLK105 and TLK106 10/100Mbit ethernet phys. > > > > In addition the TLK10X needs to be removed from DP83848 driver as the > > power back off support is added here for this device. > > > > Datasheet: > > http://www.ti.com/lit/gpn/tlk106 > > --- > > .../devicetree/bindings/net/ti,tlk10x.txt | 27 +++ > > drivers/net/phy/Kconfig | 5 + > > drivers/net/phy/Makefile | 1 + > > drivers/net/phy/dp83848.c | 3 - > > drivers/net/phy/tlk10x.c | 209 +++++++++++++++++++++ > > 5 files changed, 242 insertions(+), 3 deletions(-) > > create mode 100644 Documentation/devicetree/bindings/net/ti,tlk10x.txt > > create mode 100644 drivers/net/phy/tlk10x.c > > > > diff --git a/Documentation/devicetree/bindings/net/ti,tlk10x.txt b/Documentation/devicetree/bindings/net/ti,tlk10x.txt > > new file mode 100644 > > index 0000000..371d0d7 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/net/ti,tlk10x.txt > > @@ -0,0 +1,27 @@ > > +* Texas Instruments - TLK105 / TLK106 ethernet PHYs > > + > > +Required properties: > > + - reg - The ID number for the phy, usually a small integer > > + > > +Optional properties: > > + - ti,power-back-off - Power Back Off Level > > + Please refer to data sheet chapter 8.6 and TI Application > > + Note SLLA3228 > > + 0 - Normal Operation > > + 1 - Level 1 (up to 140m cable between TLK link partners) > > + 2 - Level 2 (up to 100m cable between TLK link partners) > > + 3 - Level 3 (up to 80m cable between TLK link partners) > > Hi Måns > > Device tree is all about board properties. In most cases, power back > off is not a board properties, since it depends on the cable length > and the peer board. If however, your board has two PHYs back to back, > say to connect to an Ethernet switch, that would be a valid board > property. > > How are you using this? > > I know of others who would like such a configuration. Marvell PHYs can > do something similar. I've always suggested adding a PHY tunable. Pass > the cable length in meters and let the PHY driver pick the nearest it > can do, rounding up. The Marvell PHYs also support measuring the cable > length as part of the cable diagnostics. So it would be good to > reserve a configuration value to mean 'auto' - measure the cable and > then pick the best power back off. Quickly scanning the data sheet, i > see that this PHY also has the ability to measure the cable length. > Hi Andrew, Thanks for your comments, highly appreciated! I'm using this to lock down the PHY to the IEEE 802.3 100m standard cable length, as opposed to the extended 150m which the PHY supports in its default operation (see pg. 2 of the data sheet). The reason why I need this is that the board has too high EMC emissions when running with the default operation. For me the setting is therefore used as a board property, i.e. it's not something that will be changed during operation. > > +static int tlk10x_read(struct phy_device *phydev, int reg) > > +{ > > + if (reg & ~0x1f) { > > + /* Extended register */ > > + phy_write(phydev, TLK10X_REGCR, 0x001F); > > + phy_write(phydev, TLK10X_ADDAR, reg); > > + phy_write(phydev, TLK10X_REGCR, 0x401F); > > + reg = TLK10X_ADDAR; > > + } > > + > > + return phy_read(phydev, reg); > > +} > > + > > +static int tlk10x_write(struct phy_device *phydev, int reg, int val) > > +{ > > + if (reg & ~0x1f) { > > + /* Extended register */ > > + phy_write(phydev, TLK10X_REGCR, 0x001F); > > + phy_write(phydev, TLK10X_ADDAR, reg); > > + phy_write(phydev, TLK10X_REGCR, 0x401F); > > + reg = TLK10X_ADDAR; > > + } > > + > > + return phy_write(phydev, reg, val); > > +} > > This looks to be phy_read_mmd() and phy_write_mmd(). If so, please use > them, they get the locking correct. > Yes, that's correct, will fix that. > > > +#ifdef CONFIG_OF_MDIO > > +static int tlk10x_of_init(struct phy_device *phydev) > > +{ > > + struct tlk10x_private *tlk10x = phydev->priv; > > + struct device *dev = &phydev->mdio.dev; > > + struct device_node *of_node = dev->of_node; > > + int ret; > > + > > + if (!of_node) > > + return 0; > > + > > + ret = of_property_read_u32(of_node, "ti,power-back-off", > > + &tlk10x->pwrbo_level); > > + if (ret) { > > + dev_err(dev, "missing ti,power-back-off property"); > > + tlk10x->pwrbo_level = 0; > > + } > > If we decide to accept this, you should do range checking, and return > -EINVAL if the value is out of range. Ok, will fix that. > > > +static int tlk10x_config_init(struct phy_device *phydev) > > +{ > > + int ret, reg; > > + struct tlk10x_private *tlk10x; > > + > > + ret = genphy_config_init(phydev); > > + if (ret < 0) > > + return ret; > > + > > + if (!phydev->priv) { > > + tlk10x = devm_kzalloc(&phydev->mdio.dev, sizeof(*tlk10x), > > + GFP_KERNEL); > > + if (!tlk10x) > > + return -ENOMEM; > > + > > + phydev->priv = tlk10x; > > + ret = tlk10x_of_init(phydev); > > + if (ret) > > + return ret; > > + } else { > > + tlk10x = (struct tlk10x_private *)phydev->priv; > > + } > > This allocation should be done in .probe Ok, will fix that. > > > + > > + // Power back off > > + if (tlk10x->pwrbo_level < 0 || tlk10x->pwrbo_level > 3) > > + tlk10x->pwrbo_level = 0; > > + reg = tlk10x_read(phydev, TLK10X_PWRBOCR); > > + reg = ((reg & ~TLK10X_PWRBOCR_MASK) > > + | (tlk10x->pwrbo_level << 6)); > > + ret = tlk10x_write(phydev, TLK10X_PWRBOCR, reg); > > + if (ret < 0) { > > + dev_err(&phydev->mdio.dev, > > + "unable to set power back-off (err=%d)\n", ret); > > + return ret; > > + } > > + dev_info(&phydev->mdio.dev, "power back-off set to level %d\n", > > + tlk10x->pwrbo_level); > > + > > + return 0; > > +} > > Andrew // Måns
On Thu, Apr 19, 2018 at 02:24:27PM +0200, Miguel Ojeda wrote: > On Thu, Apr 19, 2018 at 10:28 AM, Måns Andersson <mans.andersson@nibe.se> wrote: > > From: Mans Andersson <mans.andersson@nibe.se> > > > > Add suport for the TI TLK105 and TLK106 10/100Mbit ethernet phys. > > > > Hi Mans, > > Some quick notes. > > > In addition the TLK10X needs to be removed from DP83848 driver as the > > power back off support is added here for this device. > > > > Datasheet: > > http://www.ti.com/lit/gpn/tlk106 > > Missing signature. Hi Miguel, My bad, will fix. > > > --- > > .../devicetree/bindings/net/ti,tlk10x.txt | 27 +++ > > drivers/net/phy/Kconfig | 5 + > > drivers/net/phy/Makefile | 1 + > > drivers/net/phy/dp83848.c | 3 - > > drivers/net/phy/tlk10x.c | 209 +++++++++++++++++++++ > > 5 files changed, 242 insertions(+), 3 deletions(-) > > create mode 100644 Documentation/devicetree/bindings/net/ti,tlk10x.txt > > create mode 100644 drivers/net/phy/tlk10x.c > > > > diff --git a/Documentation/devicetree/bindings/net/ti,tlk10x.txt b/Documentation/devicetree/bindings/net/ti,tlk10x.txt > > new file mode 100644 > > index 0000000..371d0d7 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/net/ti,tlk10x.txt > > @@ -0,0 +1,27 @@ > > +* Texas Instruments - TLK105 / TLK106 ethernet PHYs > > + > > +Required properties: > > + - reg - The ID number for the phy, usually a small integer > > + > > +Optional properties: > > + - ti,power-back-off - Power Back Off Level > > + Please refer to data sheet chapter 8.6 and TI Application > > + Note SLLA3228 > > + 0 - Normal Operation > > + 1 - Level 1 (up to 140m cable between TLK link partners) > > + 2 - Level 2 (up to 100m cable between TLK link partners) > > + 3 - Level 3 (up to 80m cable between TLK link partners) > > + > > +Default child nodes are standard Ethernet PHY device > > +nodes as described in Documentation/devicetree/bindings/net/phy.txt > > + > > +Example: > > + > > + ethernet-phy@0 { > > + reg = <0>; > > + ti,power-back-off = <2>; > > + }; > > + > > +Datasheets and documentation can be found at: > > +http://www.ti.com/lit/gpn/tlk106 > > +http://www.ti.com/lit/an/slla328/slla328.pdf > > diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig > > index bdfbabb..c980240 100644 > > --- a/drivers/net/phy/Kconfig > > +++ b/drivers/net/phy/Kconfig > > @@ -295,6 +295,11 @@ config DP83867_PHY > > ---help--- > > Currently supports the DP83867 PHY. > > > > +config TLK10X_PHY > > + tristate "Texas Instruments TLK10x PHY" > > + ---help--- > > + Supports the TLK105 and TLK106 PHYs. > > + > > config FIXED_PHY > > tristate "MDIO Bus/PHY emulation with fixed speed/link PHYs" > > depends on PHYLIB > > diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile > > index 01acbcb..37e4e02 100644 > > --- a/drivers/net/phy/Makefile > > +++ b/drivers/net/phy/Makefile > > @@ -79,5 +79,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_TLK10X_PHY) += tlk10x.o > > obj-$(CONFIG_VITESSE_PHY) += vitesse.o > > obj-$(CONFIG_XILINX_GMII2RGMII) += xilinx_gmii2rgmii.o > > diff --git a/drivers/net/phy/dp83848.c b/drivers/net/phy/dp83848.c > > index cd09c3a..435f401 100644 > > --- a/drivers/net/phy/dp83848.c > > +++ b/drivers/net/phy/dp83848.c > > @@ -19,7 +19,6 @@ > > #define TI_DP83848C_PHY_ID 0x20005ca0 > > #define TI_DP83620_PHY_ID 0x20005ce0 > > #define NS_DP83848C_PHY_ID 0x20005c90 > > -#define TLK10X_PHY_ID 0x2000a210 > > > > /* Registers */ > > #define DP83848_MICR 0x11 /* MII Interrupt Control Register */ > > @@ -78,7 +77,6 @@ static struct mdio_device_id __maybe_unused dp83848_tbl[] = { > > { TI_DP83848C_PHY_ID, 0xfffffff0 }, > > { NS_DP83848C_PHY_ID, 0xfffffff0 }, > > { TI_DP83620_PHY_ID, 0xfffffff0 }, > > - { TLK10X_PHY_ID, 0xfffffff0 }, > > { } > > }; > > MODULE_DEVICE_TABLE(mdio, dp83848_tbl); > > @@ -105,7 +103,6 @@ static struct phy_driver dp83848_driver[] = { > > DP83848_PHY_DRIVER(TI_DP83848C_PHY_ID, "TI DP83848C 10/100 Mbps PHY"), > > DP83848_PHY_DRIVER(NS_DP83848C_PHY_ID, "NS DP83848C 10/100 Mbps PHY"), > > DP83848_PHY_DRIVER(TI_DP83620_PHY_ID, "TI DP83620 10/100 Mbps PHY"), > > - DP83848_PHY_DRIVER(TLK10X_PHY_ID, "TI TLK10X 10/100 Mbps PHY"), > > }; > > module_phy_driver(dp83848_driver); > > > > diff --git a/drivers/net/phy/tlk10x.c b/drivers/net/phy/tlk10x.c > > new file mode 100644 > > index 0000000..1efc81e > > --- /dev/null > > +++ b/drivers/net/phy/tlk10x.c > > @@ -0,0 +1,209 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/** > > + * Driver for the Texas Instruments TLK105 / TLK106 > > + * > > + * Copyright (C) 2018 NIBE Industrier AB - http://www.nibe.com > > + * > > + * This program is free software; you can redistribute it and/or modify > > + * it under the terms of the GNU General Public License as published by > > + * the Free Software Foundation; either version 2 of the License. > > + * > > + * This program is distributed in the hope that it will be useful, > > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > > + * GNU General Public License for more details. > > Since you are using the SPDX id, please remove the license text (which > is actually wrong: it seems you have cut the v2+ version and then > removed the last sentence of the first paragraph? :-). > Ok. > > + */ > > + > > +#include <linux/module.h> > > +#include <linux/phy.h> > > +#include <linux/of.h> > > + > > +#define TLK10X_PHY_ID 0x2000a210 > > + > > +/* Registers */ > > +#define TLK10X_MICR 0x11 /* MII Interrupt Control Reg */ > > +#define TLK10X_MISR 0x12 /* MII Interrupt Status Reg */ > > +#define TLK10X_REGCR 0x0d /* Register Control Register */ > > +#define TLK10X_ADDAR 0x0e /* Data Register */ > > +#define TLK10X_PWRBOCR 0xae /* Power Backoff Register */ > > + > > +/* MICR Register Fields */ > > +#define TLK10X_MICR_INT_OE BIT(0) /* Interrupt Output Enable */ > > +#define TLK10X_MICR_INTEN BIT(1) /* Interrupt Enable */ > > + > > +/* MISR Register Fields */ > > +#define TLK10X_MISR_RHF_INT_EN BIT(0) /* Receive Error Counter */ > > +#define TLK10X_MISR_FHF_INT_EN BIT(1) /* False Carrier Counter */ > > +#define TLK10X_MISR_ANC_INT_EN BIT(2) /* Auto-negotiation complete */ > > +#define TLK10X_MISR_DUP_INT_EN BIT(3) /* Duplex Status */ > > +#define TLK10X_MISR_SPD_INT_EN BIT(4) /* Speed status */ > > +#define TLK10X_MISR_LINK_INT_EN BIT(5) /* Link status */ > > +#define TLK10X_MISR_ED_INT_EN BIT(6) /* Energy detect */ > > +#define TLK10X_MISR_LQM_INT_EN BIT(7) /* Link Quality Monitor */ > > + > > +/* PWRBOCR Register Fields */ > > +#define TLK10X_PWRBOCR_MASK 0xe0 /* Power Backoff Mask */ > > + > > +#define TLK10X_INT_EN_MASK \ > > + (TLK10X_MISR_ANC_INT_EN | \ > > + TLK10X_MISR_DUP_INT_EN | \ > > + TLK10X_MISR_SPD_INT_EN | \ > > + TLK10X_MISR_LINK_INT_EN) > > + > > +struct tlk10x_private { > > + int pwrbo_level; > > +}; > > + > > +static int tlk10x_read(struct phy_device *phydev, int reg) > > +{ > > + if (reg & ~0x1f) { > > 0x1f or ~0x1f should probably have a #define name. > That's true, will fix that. > > + /* Extended register */ > > + phy_write(phydev, TLK10X_REGCR, 0x001F); > > + phy_write(phydev, TLK10X_ADDAR, reg); > > + phy_write(phydev, TLK10X_REGCR, 0x401F); > > + reg = TLK10X_ADDAR; > > + } > > + > > + return phy_read(phydev, reg); > > +} > > + > > +static int tlk10x_write(struct phy_device *phydev, int reg, int val) > > +{ > > + if (reg & ~0x1f) { > > Ditto. > Ok. > > + /* Extended register */ > > + phy_write(phydev, TLK10X_REGCR, 0x001F); > > + phy_write(phydev, TLK10X_ADDAR, reg); > > + phy_write(phydev, TLK10X_REGCR, 0x401F); > > + reg = TLK10X_ADDAR; > > + } > > + > > + return phy_write(phydev, reg, val); > > +} > > + > > +#ifdef CONFIG_OF_MDIO > > Maybe you want the #ifdef inside. > What's the preferred way by the kernel maintainers? I've seen both solutions used but figured this was the most common. > > +static int tlk10x_of_init(struct phy_device *phydev) > > +{ > > + struct tlk10x_private *tlk10x = phydev->priv; > > + struct device *dev = &phydev->mdio.dev; > > + struct device_node *of_node = dev->of_node; > > + int ret; > > + > > + if (!of_node) > > + return 0; > > + > > + ret = of_property_read_u32(of_node, "ti,power-back-off", > > + &tlk10x->pwrbo_level); > > + if (ret) { > > + dev_err(dev, "missing ti,power-back-off property"); > > + tlk10x->pwrbo_level = 0; > > + } > > + > > + return 0; > > +} > > +#else > > +static int tlk10x_of_init(struct phy_device *phydev) > > +{ > > + return 0; > > +} > > +#endif /* CONFIG_OF_MDIO */ > > + > > +static int tlk10x_config_init(struct phy_device *phydev) > > +{ > > + int ret, reg; > > + struct tlk10x_private *tlk10x; > > + > > + ret = genphy_config_init(phydev); > > + if (ret < 0) > > + return ret; > > + > > + if (!phydev->priv) { > > + tlk10x = devm_kzalloc(&phydev->mdio.dev, sizeof(*tlk10x), > > + GFP_KERNEL); > > + if (!tlk10x) > > + return -ENOMEM; > > + > > + phydev->priv = tlk10x; > > + ret = tlk10x_of_init(phydev); > > + if (ret) > > + return ret; > > + } else { > > + tlk10x = (struct tlk10x_private *)phydev->priv; > > + } > > + > > + // Power back off > > + if (tlk10x->pwrbo_level < 0 || tlk10x->pwrbo_level > 3) > > + tlk10x->pwrbo_level = 0; > > + reg = tlk10x_read(phydev, TLK10X_PWRBOCR); > > + reg = ((reg & ~TLK10X_PWRBOCR_MASK) > > + | (tlk10x->pwrbo_level << 6)); > > Maybe the 6 should have a name, or maybe a bigger macro for this would clarify. > Will look into this. > > + ret = tlk10x_write(phydev, TLK10X_PWRBOCR, reg); > > + if (ret < 0) { > > + dev_err(&phydev->mdio.dev, > > + "unable to set power back-off (err=%d)\n", ret); > > + return ret; > > + } > > + dev_info(&phydev->mdio.dev, "power back-off set to level %d\n", > > + tlk10x->pwrbo_level); > > + > > + return 0; > > +} > > + > > +static int tlk10x_ack_interrupt(struct phy_device *phydev) > > +{ > > + int err = tlk10x_read(phydev, TLK10X_MISR); > > Following the style of the rest of the file, shouldn't this be: > > if (err < 0) > return err; > > return 0; > > ? > True, will fix that. > > + > > + return err < 0 ? err : 0; > > +} > > + > > +static int tlk10x_config_intr(struct phy_device *phydev) > > +{ > > + int control, ret; > > + > > + control = tlk10x_read(phydev, TLK10X_MICR); > > + if (control < 0) > > + return control; > > + > > + if (phydev->interrupts == PHY_INTERRUPT_ENABLED) { > > + control |= TLK10X_MICR_INT_OE; > > + control |= TLK10X_MICR_INTEN; > > + > > + ret = tlk10x_write(phydev, TLK10X_MISR, TLK10X_INT_EN_MASK); > > + if (ret < 0) > > + return ret; > > + } else { > > + control &= ~TLK10X_MICR_INTEN; > > + } > > + > > + return tlk10x_write(phydev, TLK10X_MICR, control); > > +} > > + > > +static struct phy_driver tlk10x_driver[] = { > > + { > > + .phy_id = TLK10X_PHY_ID, > > + .phy_id_mask = 0xfffffff0, > > + .name = "TI TLK10X 10/100 Mbps PHY", > > + .features = PHY_BASIC_FEATURES, > > + .flags = PHY_HAS_INTERRUPT, > > + > > + .config_init = tlk10x_config_init, > > + .soft_reset = genphy_soft_reset, > > + > > + /* IRQ related */ > > + .ack_interrupt = tlk10x_ack_interrupt, > > + .config_intr = tlk10x_config_intr, > > + > > + .suspend = genphy_suspend, > > + .resume = genphy_resume, > > + }, > > +}; > > +module_phy_driver(tlk10x_driver); > > + > > +static struct mdio_device_id __maybe_unused tlk10x_tbl[] = { > > + { TLK10X_PHY_ID, 0xfffffff0 }, > > + { } > > +}; > > +MODULE_DEVICE_TABLE(mdio, tlk10x_tbl); > > + > > +MODULE_DESCRIPTION("Texas Instruments TLK105 / TLK106 PHY driver"); > > +MODULE_AUTHOR("Mans Andersson <mans.andersson@nibe.se>"); > > +MODULE_LICENSE("GPL"); > > Should be "GPL v2". > > Cheers, > Miguel Good catch, will update that as well. Thanks for all the input! I will get back in a couple of days with an updated patch. // Måns
On Thu, Apr 19, 2018 at 10:28:16AM +0200, Måns Andersson wrote: > From: Mans Andersson <mans.andersson@nibe.se> > > Add suport for the TI TLK105 and TLK106 10/100Mbit ethernet phys. > > In addition the TLK10X needs to be removed from DP83848 driver as the > power back off support is added here for this device. > > Datasheet: > http://www.ti.com/lit/gpn/tlk106 > --- > .../devicetree/bindings/net/ti,tlk10x.txt | 27 +++ Please split bindings to a separate patch. > drivers/net/phy/Kconfig | 5 + > drivers/net/phy/Makefile | 1 + > drivers/net/phy/dp83848.c | 3 - > drivers/net/phy/tlk10x.c | 209 +++++++++++++++++++++ > 5 files changed, 242 insertions(+), 3 deletions(-) > create mode 100644 Documentation/devicetree/bindings/net/ti,tlk10x.txt > create mode 100644 drivers/net/phy/tlk10x.c > > diff --git a/Documentation/devicetree/bindings/net/ti,tlk10x.txt b/Documentation/devicetree/bindings/net/ti,tlk10x.txt > new file mode 100644 > index 0000000..371d0d7 > --- /dev/null > +++ b/Documentation/devicetree/bindings/net/ti,tlk10x.txt > @@ -0,0 +1,27 @@ > +* Texas Instruments - TLK105 / TLK106 ethernet PHYs > + > +Required properties: > + - reg - The ID number for the phy, usually a small integer Isn't this the MDIO bus address? This should have a compatible string too. > + > +Optional properties: > + - ti,power-back-off - Power Back Off Level > + Please refer to data sheet chapter 8.6 and TI Application > + Note SLLA3228 > + 0 - Normal Operation > + 1 - Level 1 (up to 140m cable between TLK link partners) > + 2 - Level 2 (up to 100m cable between TLK link partners) > + 3 - Level 3 (up to 80m cable between TLK link partners) > + > +Default child nodes are standard Ethernet PHY device > +nodes as described in Documentation/devicetree/bindings/net/phy.txt > + > +Example: > + > + ethernet-phy@0 { > + reg = <0>; > + ti,power-back-off = <2>; > + }; > + > +Datasheets and documentation can be found at: > +http://www.ti.com/lit/gpn/tlk106 > +http://www.ti.com/lit/an/slla328/slla328.pdf Move this to before the properties. Rob
On 04/19/2018 01:28 AM, Måns Andersson wrote: > From: Mans Andersson <mans.andersson@nibe.se> > > Add suport for the TI TLK105 and TLK106 10/100Mbit ethernet phys. > > In addition the TLK10X needs to be removed from DP83848 driver as the > power back off support is added here for this device. I would not think this is a compelling enough reason, you could very well just adjust the dp83848.c driver just to account for these properties that you are introducing. More comments below. [snip] > +#define TLK10X_INT_EN_MASK \ > + (TLK10X_MISR_ANC_INT_EN | \ > + TLK10X_MISR_DUP_INT_EN | \ > + TLK10X_MISR_SPD_INT_EN | \ > + TLK10X_MISR_LINK_INT_EN) > + > +struct tlk10x_private { > + int pwrbo_level; unsigned int > +}; > + > +static int tlk10x_read(struct phy_device *phydev, int reg) > +{ > + if (reg & ~0x1f) { > + /* Extended register */ > + phy_write(phydev, TLK10X_REGCR, 0x001F); > + phy_write(phydev, TLK10X_ADDAR, reg); > + phy_write(phydev, TLK10X_REGCR, 0x401F); > + reg = TLK10X_ADDAR; > + } Humm, this looks a bit fragile, you would likely want to create separate helper functions for these extended registers and make sure you handle write failures as well. Also consider making use of the page helpers from include/linux/phy.h. > + > + return phy_read(phydev, reg); > +} > + > +static int tlk10x_write(struct phy_device *phydev, int reg, int val) > +{ > + if (reg & ~0x1f) { > + /* Extended register */ > + phy_write(phydev, TLK10X_REGCR, 0x001F); > + phy_write(phydev, TLK10X_ADDAR, reg); > + phy_write(phydev, TLK10X_REGCR, 0x401F); > + reg = TLK10X_ADDAR; > + } Same here. > + > + return phy_write(phydev, reg, val); > +} > + > +#ifdef CONFIG_OF_MDIO > +static int tlk10x_of_init(struct phy_device *phydev) > +{ > + struct tlk10x_private *tlk10x = phydev->priv; > + struct device *dev = &phydev->mdio.dev; > + struct device_node *of_node = dev->of_node; > + int ret; > + > + if (!of_node) > + return 0; > + > + ret = of_property_read_u32(of_node, "ti,power-back-off", > + &tlk10x->pwrbo_level); > + if (ret) { > + dev_err(dev, "missing ti,power-back-off property"); > + tlk10x->pwrbo_level = 0; This should not be necessary, that should be the default with a zero initialized private data structure. > + } > + > + return 0; > +} > +#else > +static int tlk10x_of_init(struct phy_device *phydev) > +{ > + return 0; > +} > +#endif /* CONFIG_OF_MDIO */ > + > +static int tlk10x_config_init(struct phy_device *phydev) > +{ > + int ret, reg; > + struct tlk10x_private *tlk10x; > + > + ret = genphy_config_init(phydev); > + if (ret < 0) > + return ret; > + > + if (!phydev->priv) { > + tlk10x = devm_kzalloc(&phydev->mdio.dev, sizeof(*tlk10x), > + GFP_KERNEL); > + if (!tlk10x) > + return -ENOMEM; > + > + phydev->priv = tlk10x; > + ret = tlk10x_of_init(phydev); > + if (ret) > + return ret; > + } else { > + tlk10x = (struct tlk10x_private *)phydev->priv; > + } You need to implement a probe() function that is responsible for allocation private memory instead of doing this check. > + > + // Power back off > + if (tlk10x->pwrbo_level < 0 || tlk10x->pwrbo_level > 3) > + tlk10x->pwrbo_level = 0; How can you have pwrb_level < 0 when you use of_read_property_u32()? > + reg = tlk10x_read(phydev, TLK10X_PWRBOCR); > + reg = ((reg & ~TLK10X_PWRBOCR_MASK) > + | (tlk10x->pwrbo_level << 6)); One too many levels of parenthesis, the outer ones should not be necessary. > + ret = tlk10x_write(phydev, TLK10X_PWRBOCR, reg); > + if (ret < 0) { > + dev_err(&phydev->mdio.dev, > + "unable to set power back-off (err=%d)\n", ret); > + return ret; > + } > + dev_info(&phydev->mdio.dev, "power back-off set to level %d\n", > + tlk10x->pwrbo_level); config_init() is called often, consider making this a debugging statement.
> > +Required properties: > > + - reg - The ID number for the phy, usually a small integer > > Isn't this the MDIO bus address? Hi Rob Yes. This text has been take direct from the generic PHY binding. > This should have a compatible string too. Please see Documentation/devicetree/bindings/net/phy.txt compatible strings are optional. We know what device it is from its ID registers, which are in a well known location. Andrew
diff --git a/Documentation/devicetree/bindings/net/ti,tlk10x.txt b/Documentation/devicetree/bindings/net/ti,tlk10x.txt new file mode 100644 index 0000000..371d0d7 --- /dev/null +++ b/Documentation/devicetree/bindings/net/ti,tlk10x.txt @@ -0,0 +1,27 @@ +* Texas Instruments - TLK105 / TLK106 ethernet PHYs + +Required properties: + - reg - The ID number for the phy, usually a small integer + +Optional properties: + - ti,power-back-off - Power Back Off Level + Please refer to data sheet chapter 8.6 and TI Application + Note SLLA3228 + 0 - Normal Operation + 1 - Level 1 (up to 140m cable between TLK link partners) + 2 - Level 2 (up to 100m cable between TLK link partners) + 3 - Level 3 (up to 80m cable between TLK link partners) + +Default child nodes are standard Ethernet PHY device +nodes as described in Documentation/devicetree/bindings/net/phy.txt + +Example: + + ethernet-phy@0 { + reg = <0>; + ti,power-back-off = <2>; + }; + +Datasheets and documentation can be found at: +http://www.ti.com/lit/gpn/tlk106 +http://www.ti.com/lit/an/slla328/slla328.pdf diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig index bdfbabb..c980240 100644 --- a/drivers/net/phy/Kconfig +++ b/drivers/net/phy/Kconfig @@ -295,6 +295,11 @@ config DP83867_PHY ---help--- Currently supports the DP83867 PHY. +config TLK10X_PHY + tristate "Texas Instruments TLK10x PHY" + ---help--- + Supports the TLK105 and TLK106 PHYs. + config FIXED_PHY tristate "MDIO Bus/PHY emulation with fixed speed/link PHYs" depends on PHYLIB diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile index 01acbcb..37e4e02 100644 --- a/drivers/net/phy/Makefile +++ b/drivers/net/phy/Makefile @@ -79,5 +79,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_TLK10X_PHY) += tlk10x.o obj-$(CONFIG_VITESSE_PHY) += vitesse.o obj-$(CONFIG_XILINX_GMII2RGMII) += xilinx_gmii2rgmii.o diff --git a/drivers/net/phy/dp83848.c b/drivers/net/phy/dp83848.c index cd09c3a..435f401 100644 --- a/drivers/net/phy/dp83848.c +++ b/drivers/net/phy/dp83848.c @@ -19,7 +19,6 @@ #define TI_DP83848C_PHY_ID 0x20005ca0 #define TI_DP83620_PHY_ID 0x20005ce0 #define NS_DP83848C_PHY_ID 0x20005c90 -#define TLK10X_PHY_ID 0x2000a210 /* Registers */ #define DP83848_MICR 0x11 /* MII Interrupt Control Register */ @@ -78,7 +77,6 @@ static struct mdio_device_id __maybe_unused dp83848_tbl[] = { { TI_DP83848C_PHY_ID, 0xfffffff0 }, { NS_DP83848C_PHY_ID, 0xfffffff0 }, { TI_DP83620_PHY_ID, 0xfffffff0 }, - { TLK10X_PHY_ID, 0xfffffff0 }, { } }; MODULE_DEVICE_TABLE(mdio, dp83848_tbl); @@ -105,7 +103,6 @@ static struct phy_driver dp83848_driver[] = { DP83848_PHY_DRIVER(TI_DP83848C_PHY_ID, "TI DP83848C 10/100 Mbps PHY"), DP83848_PHY_DRIVER(NS_DP83848C_PHY_ID, "NS DP83848C 10/100 Mbps PHY"), DP83848_PHY_DRIVER(TI_DP83620_PHY_ID, "TI DP83620 10/100 Mbps PHY"), - DP83848_PHY_DRIVER(TLK10X_PHY_ID, "TI TLK10X 10/100 Mbps PHY"), }; module_phy_driver(dp83848_driver); diff --git a/drivers/net/phy/tlk10x.c b/drivers/net/phy/tlk10x.c new file mode 100644 index 0000000..1efc81e --- /dev/null +++ b/drivers/net/phy/tlk10x.c @@ -0,0 +1,209 @@ +// SPDX-License-Identifier: GPL-2.0 +/** + * Driver for the Texas Instruments TLK105 / TLK106 + * + * Copyright (C) 2018 NIBE Industrier AB - http://www.nibe.com + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#include <linux/module.h> +#include <linux/phy.h> +#include <linux/of.h> + +#define TLK10X_PHY_ID 0x2000a210 + +/* Registers */ +#define TLK10X_MICR 0x11 /* MII Interrupt Control Reg */ +#define TLK10X_MISR 0x12 /* MII Interrupt Status Reg */ +#define TLK10X_REGCR 0x0d /* Register Control Register */ +#define TLK10X_ADDAR 0x0e /* Data Register */ +#define TLK10X_PWRBOCR 0xae /* Power Backoff Register */ + +/* MICR Register Fields */ +#define TLK10X_MICR_INT_OE BIT(0) /* Interrupt Output Enable */ +#define TLK10X_MICR_INTEN BIT(1) /* Interrupt Enable */ + +/* MISR Register Fields */ +#define TLK10X_MISR_RHF_INT_EN BIT(0) /* Receive Error Counter */ +#define TLK10X_MISR_FHF_INT_EN BIT(1) /* False Carrier Counter */ +#define TLK10X_MISR_ANC_INT_EN BIT(2) /* Auto-negotiation complete */ +#define TLK10X_MISR_DUP_INT_EN BIT(3) /* Duplex Status */ +#define TLK10X_MISR_SPD_INT_EN BIT(4) /* Speed status */ +#define TLK10X_MISR_LINK_INT_EN BIT(5) /* Link status */ +#define TLK10X_MISR_ED_INT_EN BIT(6) /* Energy detect */ +#define TLK10X_MISR_LQM_INT_EN BIT(7) /* Link Quality Monitor */ + +/* PWRBOCR Register Fields */ +#define TLK10X_PWRBOCR_MASK 0xe0 /* Power Backoff Mask */ + +#define TLK10X_INT_EN_MASK \ + (TLK10X_MISR_ANC_INT_EN | \ + TLK10X_MISR_DUP_INT_EN | \ + TLK10X_MISR_SPD_INT_EN | \ + TLK10X_MISR_LINK_INT_EN) + +struct tlk10x_private { + int pwrbo_level; +}; + +static int tlk10x_read(struct phy_device *phydev, int reg) +{ + if (reg & ~0x1f) { + /* Extended register */ + phy_write(phydev, TLK10X_REGCR, 0x001F); + phy_write(phydev, TLK10X_ADDAR, reg); + phy_write(phydev, TLK10X_REGCR, 0x401F); + reg = TLK10X_ADDAR; + } + + return phy_read(phydev, reg); +} + +static int tlk10x_write(struct phy_device *phydev, int reg, int val) +{ + if (reg & ~0x1f) { + /* Extended register */ + phy_write(phydev, TLK10X_REGCR, 0x001F); + phy_write(phydev, TLK10X_ADDAR, reg); + phy_write(phydev, TLK10X_REGCR, 0x401F); + reg = TLK10X_ADDAR; + } + + return phy_write(phydev, reg, val); +} + +#ifdef CONFIG_OF_MDIO +static int tlk10x_of_init(struct phy_device *phydev) +{ + struct tlk10x_private *tlk10x = phydev->priv; + struct device *dev = &phydev->mdio.dev; + struct device_node *of_node = dev->of_node; + int ret; + + if (!of_node) + return 0; + + ret = of_property_read_u32(of_node, "ti,power-back-off", + &tlk10x->pwrbo_level); + if (ret) { + dev_err(dev, "missing ti,power-back-off property"); + tlk10x->pwrbo_level = 0; + } + + return 0; +} +#else +static int tlk10x_of_init(struct phy_device *phydev) +{ + return 0; +} +#endif /* CONFIG_OF_MDIO */ + +static int tlk10x_config_init(struct phy_device *phydev) +{ + int ret, reg; + struct tlk10x_private *tlk10x; + + ret = genphy_config_init(phydev); + if (ret < 0) + return ret; + + if (!phydev->priv) { + tlk10x = devm_kzalloc(&phydev->mdio.dev, sizeof(*tlk10x), + GFP_KERNEL); + if (!tlk10x) + return -ENOMEM; + + phydev->priv = tlk10x; + ret = tlk10x_of_init(phydev); + if (ret) + return ret; + } else { + tlk10x = (struct tlk10x_private *)phydev->priv; + } + + // Power back off + if (tlk10x->pwrbo_level < 0 || tlk10x->pwrbo_level > 3) + tlk10x->pwrbo_level = 0; + reg = tlk10x_read(phydev, TLK10X_PWRBOCR); + reg = ((reg & ~TLK10X_PWRBOCR_MASK) + | (tlk10x->pwrbo_level << 6)); + ret = tlk10x_write(phydev, TLK10X_PWRBOCR, reg); + if (ret < 0) { + dev_err(&phydev->mdio.dev, + "unable to set power back-off (err=%d)\n", ret); + return ret; + } + dev_info(&phydev->mdio.dev, "power back-off set to level %d\n", + tlk10x->pwrbo_level); + + return 0; +} + +static int tlk10x_ack_interrupt(struct phy_device *phydev) +{ + int err = tlk10x_read(phydev, TLK10X_MISR); + + return err < 0 ? err : 0; +} + +static int tlk10x_config_intr(struct phy_device *phydev) +{ + int control, ret; + + control = tlk10x_read(phydev, TLK10X_MICR); + if (control < 0) + return control; + + if (phydev->interrupts == PHY_INTERRUPT_ENABLED) { + control |= TLK10X_MICR_INT_OE; + control |= TLK10X_MICR_INTEN; + + ret = tlk10x_write(phydev, TLK10X_MISR, TLK10X_INT_EN_MASK); + if (ret < 0) + return ret; + } else { + control &= ~TLK10X_MICR_INTEN; + } + + return tlk10x_write(phydev, TLK10X_MICR, control); +} + +static struct phy_driver tlk10x_driver[] = { + { + .phy_id = TLK10X_PHY_ID, + .phy_id_mask = 0xfffffff0, + .name = "TI TLK10X 10/100 Mbps PHY", + .features = PHY_BASIC_FEATURES, + .flags = PHY_HAS_INTERRUPT, + + .config_init = tlk10x_config_init, + .soft_reset = genphy_soft_reset, + + /* IRQ related */ + .ack_interrupt = tlk10x_ack_interrupt, + .config_intr = tlk10x_config_intr, + + .suspend = genphy_suspend, + .resume = genphy_resume, + }, +}; +module_phy_driver(tlk10x_driver); + +static struct mdio_device_id __maybe_unused tlk10x_tbl[] = { + { TLK10X_PHY_ID, 0xfffffff0 }, + { } +}; +MODULE_DEVICE_TABLE(mdio, tlk10x_tbl); + +MODULE_DESCRIPTION("Texas Instruments TLK105 / TLK106 PHY driver"); +MODULE_AUTHOR("Mans Andersson <mans.andersson@nibe.se>"); +MODULE_LICENSE("GPL");
From: Mans Andersson <mans.andersson@nibe.se> Add suport for the TI TLK105 and TLK106 10/100Mbit ethernet phys. In addition the TLK10X needs to be removed from DP83848 driver as the power back off support is added here for this device. Datasheet: http://www.ti.com/lit/gpn/tlk106 --- .../devicetree/bindings/net/ti,tlk10x.txt | 27 +++ drivers/net/phy/Kconfig | 5 + drivers/net/phy/Makefile | 1 + drivers/net/phy/dp83848.c | 3 - drivers/net/phy/tlk10x.c | 209 +++++++++++++++++++++ 5 files changed, 242 insertions(+), 3 deletions(-) create mode 100644 Documentation/devicetree/bindings/net/ti,tlk10x.txt create mode 100644 drivers/net/phy/tlk10x.c