changes v4:
- remove unused phy_id variable
changes v3:
- export part of of_mdiobus_register_phy() and reuse it in tja11xx
driver
- coding style fixes
changes v2:
- use .match_phy_device
- add irq support
- add add delayed registration for PHY1
Oleksij Rempel (4):
dt-bindings: net: phy: Add support for NXP TJA11xx
net: phy: tja11xx: add initial TJA1102 support
net: mdio: of: export part of of_mdiobus_register_phy()
net: phy: tja11xx: add delayed registration of TJA1102 PHY1
.../devicetree/bindings/net/nxp,tja11xx.yaml | 61 ++++++
drivers/net/phy/nxp-tja11xx.c | 201 +++++++++++++++++-
drivers/of/of_mdio.c | 73 ++++---
include/linux/of_mdio.h | 11 +-
4 files changed, 308 insertions(+), 38 deletions(-)
create mode 100644 Documentation/devicetree/bindings/net/nxp,tja11xx.yaml
--
2.25.1
TJA1102 is a dual PHY package with PHY0 having proper PHYID and PHY1
having no ID. On one hand it is possible to for PHY detection by
compatible, on other hand we should be able to reset complete chip
before PHY1 configured it, and we need to define dependencies for proper
power management.
We can solve it by defining PHY1 as child of PHY0:
tja1102_phy0: ethernet-phy@4 {
reg = <0x4>;
interrupts-extended = <&gpio5 8 IRQ_TYPE_LEVEL_LOW>;
reset-gpios = <&gpio5 9 GPIO_ACTIVE_LOW>;
reset-assert-us = <20>;
reset-deassert-us = <2000>;
tja1102_phy1: ethernet-phy@5 {
reg = <0x5>;
interrupts-extended = <&gpio5 8 IRQ_TYPE_LEVEL_LOW>;
};
};
The PHY1 should be a subnode of PHY0 and registered only after PHY0 was
completely reset and initialized.
Signed-off-by: Oleksij Rempel <[email protected]>
---
drivers/net/phy/nxp-tja11xx.c | 112 +++++++++++++++++++++++++++++++---
1 file changed, 105 insertions(+), 7 deletions(-)
diff --git a/drivers/net/phy/nxp-tja11xx.c b/drivers/net/phy/nxp-tja11xx.c
index a064e4ab3616..2bde9386baf1 100644
--- a/drivers/net/phy/nxp-tja11xx.c
+++ b/drivers/net/phy/nxp-tja11xx.c
@@ -6,11 +6,14 @@
#include <linux/delay.h>
#include <linux/ethtool.h>
#include <linux/kernel.h>
+#include <linux/mdio.h>
#include <linux/mii.h>
#include <linux/module.h>
#include <linux/phy.h>
#include <linux/hwmon.h>
#include <linux/bitfield.h>
+#include <linux/of_mdio.h>
+#include <linux/of_irq.h>
#define PHY_ID_MASK 0xfffffff0
#define PHY_ID_TJA1100 0x0180dc40
@@ -57,6 +60,8 @@
struct tja11xx_priv {
char *hwmon_name;
struct device *hwmon_dev;
+ struct phy_device *phydev;
+ struct work_struct phy_register_work;
};
struct tja11xx_phy_stats {
@@ -333,16 +338,12 @@ static const struct hwmon_chip_info tja11xx_hwmon_chip_info = {
.info = tja11xx_hwmon_info,
};
-static int tja11xx_probe(struct phy_device *phydev)
+static int tja11xx_hwmon_register(struct phy_device *phydev,
+ struct tja11xx_priv *priv)
{
struct device *dev = &phydev->mdio.dev;
- struct tja11xx_priv *priv;
int i;
- priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
- if (!priv)
- return -ENOMEM;
-
priv->hwmon_name = devm_kstrdup(dev, dev_name(dev), GFP_KERNEL);
if (!priv->hwmon_name)
return -ENOMEM;
@@ -360,6 +361,103 @@ static int tja11xx_probe(struct phy_device *phydev)
return PTR_ERR_OR_ZERO(priv->hwmon_dev);
}
+static int tja11xx_probe(struct phy_device *phydev)
+{
+ struct device *dev = &phydev->mdio.dev;
+ struct tja11xx_priv *priv;
+
+ priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
+
+ priv->phydev = phydev;
+
+ return tja11xx_hwmon_register(phydev, priv);
+}
+
+static void tja1102_p1_register(struct work_struct *work)
+{
+ struct tja11xx_priv *priv = container_of(work, struct tja11xx_priv,
+ phy_register_work);
+ struct phy_device *phydev_phy0 = priv->phydev;
+ struct mii_bus *bus = phydev_phy0->mdio.bus;
+ struct device *dev = &phydev_phy0->mdio.dev;
+ struct device_node *np = dev->of_node;
+ struct device_node *child;
+ int ret;
+
+ for_each_available_child_of_node(np, child) {
+ struct phy_device *phy;
+ int addr;
+
+ addr = of_mdio_parse_addr(dev, child);
+ if (addr < 0) {
+ dev_err(dev, "Can't parse addr\n");
+ continue;
+ } else if (addr != phydev_phy0->mdio.addr + 1) {
+ /* Currently we care only about double PHY chip TJA1102.
+ * If some day NXP will decide to bring chips with more
+ * PHYs, this logic should be reworked.
+ */
+ dev_err(dev, "Unexpected address. Should be: %i\n",
+ phydev_phy0->mdio.addr + 1);
+ continue;
+ }
+
+ if (mdiobus_is_registered_device(bus, addr)) {
+ dev_err(dev, "device is already registered\n");
+ continue;
+ }
+
+ /* Real PHY ID of Port 1 is 0 */
+ phy = phy_device_create(bus, addr, PHY_ID_TJA1102, false, NULL);
+ if (IS_ERR(phy)) {
+ dev_err(dev, "Can't create PHY device for Port 1: %i\n",
+ addr);
+ continue;
+ }
+
+ /* Overwrite parent device. phy_device_create() set parent to
+ * the mii_bus->dev, which is not correct in case.
+ */
+ phy->mdio.dev.parent = dev;
+
+ ret = __of_mdiobus_register_phy(bus, phy, child, addr);
+ if (ret) {
+ /* All resources needed for Port 1 should be already
+ * available for Port 0. Both ports use the same
+ * interrupt line, so -EPROBE_DEFER would make no sense
+ * here.
+ */
+ dev_err(dev, "Can't register Port 1. Unexpected error: %i\n",
+ ret);
+ phy_device_free(phy);
+ }
+ }
+}
+
+static int tja1102_p0_probe(struct phy_device *phydev)
+{
+ struct device *dev = &phydev->mdio.dev;
+ struct tja11xx_priv *priv;
+ int ret;
+
+ priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
+
+ priv->phydev = phydev;
+ INIT_WORK(&priv->phy_register_work, tja1102_p1_register);
+
+ ret = tja11xx_hwmon_register(phydev, priv);
+ if (ret)
+ return ret;
+
+ schedule_work(&priv->phy_register_work);
+
+ return 0;
+}
+
static int tja1102_match_phy_device(struct phy_device *phydev, bool port0)
{
int ret;
@@ -443,7 +541,7 @@ static struct phy_driver tja11xx_driver[] = {
}, {
.name = "NXP TJA1102 Port 0",
.features = PHY_BASIC_T1_FEATURES,
- .probe = tja11xx_probe,
+ .probe = tja1102_p0_probe,
.soft_reset = tja11xx_soft_reset,
.config_init = tja11xx_config_init,
.read_status = tja11xx_read_status,
--
2.25.1
This function will be needed in tja11xx driver for secondary PHY
support.
Signed-off-by: Oleksij Rempel <[email protected]>
---
drivers/of/of_mdio.c | 73 ++++++++++++++++++++++++-----------------
include/linux/of_mdio.h | 11 ++++++-
2 files changed, 52 insertions(+), 32 deletions(-)
diff --git a/drivers/of/of_mdio.c b/drivers/of/of_mdio.c
index 8270bbf505fb..d9e637b624ce 100644
--- a/drivers/of/of_mdio.c
+++ b/drivers/of/of_mdio.c
@@ -60,39 +60,15 @@ static struct mii_timestamper *of_find_mii_timestamper(struct device_node *node)
return register_mii_timestamper(arg.np, arg.args[0]);
}
-static int of_mdiobus_register_phy(struct mii_bus *mdio,
- struct device_node *child, u32 addr)
+int __of_mdiobus_register_phy(struct mii_bus *mdio, struct phy_device *phy,
+ struct device_node *child, u32 addr)
{
- struct mii_timestamper *mii_ts;
- struct phy_device *phy;
- bool is_c45;
int rc;
- u32 phy_id;
-
- mii_ts = of_find_mii_timestamper(child);
- if (IS_ERR(mii_ts))
- return PTR_ERR(mii_ts);
-
- is_c45 = of_device_is_compatible(child,
- "ethernet-phy-ieee802.3-c45");
-
- if (!is_c45 && !of_get_phy_id(child, &phy_id))
- phy = phy_device_create(mdio, addr, phy_id, 0, NULL);
- else
- phy = get_phy_device(mdio, addr, is_c45);
- if (IS_ERR(phy)) {
- if (mii_ts)
- unregister_mii_timestamper(mii_ts);
- return PTR_ERR(phy);
- }
rc = of_irq_get(child, 0);
- if (rc == -EPROBE_DEFER) {
- if (mii_ts)
- unregister_mii_timestamper(mii_ts);
- phy_device_free(phy);
+ if (rc == -EPROBE_DEFER)
return rc;
- }
+
if (rc > 0) {
phy->irq = rc;
mdio->irq[addr] = rc;
@@ -117,11 +93,48 @@ static int of_mdiobus_register_phy(struct mii_bus *mdio,
/* All data is now stored in the phy struct;
* register it */
rc = phy_device_register(phy);
+ if (rc) {
+ of_node_put(child);
+ return rc;
+ }
+
+ dev_dbg(&mdio->dev, "registered phy %pOFn at address %i\n",
+ child, addr);
+ return 0;
+}
+EXPORT_SYMBOL(__of_mdiobus_register_phy);
+
+static int of_mdiobus_register_phy(struct mii_bus *mdio,
+ struct device_node *child, u32 addr)
+{
+ struct mii_timestamper *mii_ts;
+ struct phy_device *phy;
+ bool is_c45;
+ int rc;
+ u32 phy_id;
+
+ mii_ts = of_find_mii_timestamper(child);
+ if (IS_ERR(mii_ts))
+ return PTR_ERR(mii_ts);
+
+ is_c45 = of_device_is_compatible(child,
+ "ethernet-phy-ieee802.3-c45");
+
+ if (!is_c45 && !of_get_phy_id(child, &phy_id))
+ phy = phy_device_create(mdio, addr, phy_id, 0, NULL);
+ else
+ phy = get_phy_device(mdio, addr, is_c45);
+ if (IS_ERR(phy)) {
+ if (mii_ts)
+ unregister_mii_timestamper(mii_ts);
+ return PTR_ERR(phy);
+ }
+
+ rc = __of_mdiobus_register_phy(mdio, phy, child, addr);
if (rc) {
if (mii_ts)
unregister_mii_timestamper(mii_ts);
phy_device_free(phy);
- of_node_put(child);
return rc;
}
@@ -132,8 +145,6 @@ static int of_mdiobus_register_phy(struct mii_bus *mdio,
if (mii_ts)
phy->mii_ts = mii_ts;
- dev_dbg(&mdio->dev, "registered phy %pOFn at address %i\n",
- child, addr);
return 0;
}
diff --git a/include/linux/of_mdio.h b/include/linux/of_mdio.h
index 491a2b7e77c1..b99e9f932002 100644
--- a/include/linux/of_mdio.h
+++ b/include/linux/of_mdio.h
@@ -30,7 +30,9 @@ extern struct mii_bus *of_mdio_find_bus(struct device_node *mdio_np);
extern int of_phy_register_fixed_link(struct device_node *np);
extern void of_phy_deregister_fixed_link(struct device_node *np);
extern bool of_phy_is_fixed_link(struct device_node *np);
-
+extern int __of_mdiobus_register_phy(struct mii_bus *mdio,
+ struct phy_device *phy,
+ struct device_node *child, u32 addr);
static inline int of_mdio_parse_addr(struct device *dev,
const struct device_node *np)
@@ -118,6 +120,13 @@ static inline bool of_phy_is_fixed_link(struct device_node *np)
{
return false;
}
+
+static inline int __of_mdiobus_register_phy(struct mii_bus *mdio,
+ struct phy_device *phy,
+ struct device_node *child, u32 addr)
+{
+ return -ENOSYS;
+}
#endif
--
2.25.1
On Tue, Apr 21, 2020 at 02:52:18PM +0200, Oleksij Rempel wrote:
> This function will be needed in tja11xx driver for secondary PHY
> support.
>
> Signed-off-by: Oleksij Rempel <[email protected]>
> ---
> drivers/of/of_mdio.c | 73 ++++++++++++++++++++++++-----------------
> include/linux/of_mdio.h | 11 ++++++-
> 2 files changed, 52 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/of/of_mdio.c b/drivers/of/of_mdio.c
> index 8270bbf505fb..d9e637b624ce 100644
> --- a/drivers/of/of_mdio.c
> +++ b/drivers/of/of_mdio.c
> @@ -60,39 +60,15 @@ static struct mii_timestamper *of_find_mii_timestamper(struct device_node *node)
> return register_mii_timestamper(arg.np, arg.args[0]);
> }
>
> -static int of_mdiobus_register_phy(struct mii_bus *mdio,
> - struct device_node *child, u32 addr)
> +int __of_mdiobus_register_phy(struct mii_bus *mdio, struct phy_device *phy,
> + struct device_node *child, u32 addr)
> {
Hi Oleksij
I would prefer a different name. __foo functions often indicate
locking is needed, or an internal API which should not be used except
internally. This is going to be an official API. Maybe call it
of_mdiobus_phy_device_register() ?
Andrew
On Tue, Apr 21, 2020 at 03:51:47PM +0200, Andrew Lunn wrote:
> On Tue, Apr 21, 2020 at 02:52:18PM +0200, Oleksij Rempel wrote:
> > This function will be needed in tja11xx driver for secondary PHY
> > support.
> >
> > Signed-off-by: Oleksij Rempel <[email protected]>
> > ---
> > drivers/of/of_mdio.c | 73 ++++++++++++++++++++++++-----------------
> > include/linux/of_mdio.h | 11 ++++++-
> > 2 files changed, 52 insertions(+), 32 deletions(-)
> >
> > diff --git a/drivers/of/of_mdio.c b/drivers/of/of_mdio.c
> > index 8270bbf505fb..d9e637b624ce 100644
> > --- a/drivers/of/of_mdio.c
> > +++ b/drivers/of/of_mdio.c
> > @@ -60,39 +60,15 @@ static struct mii_timestamper *of_find_mii_timestamper(struct device_node *node)
> > return register_mii_timestamper(arg.np, arg.args[0]);
> > }
> >
> > -static int of_mdiobus_register_phy(struct mii_bus *mdio,
> > - struct device_node *child, u32 addr)
> > +int __of_mdiobus_register_phy(struct mii_bus *mdio, struct phy_device *phy,
> > + struct device_node *child, u32 addr)
> > {
>
> Hi Oleksij
>
> I would prefer a different name. __foo functions often indicate
> locking is needed, or an internal API which should not be used except
> internally. This is going to be an official API. Maybe call it
> of_mdiobus_phy_device_register() ?
ok, sounds good. I'll send new version
Regards,
Oleksij
--
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 |