[PATCH v2a 1/2] dt: Document a compatible entry for MDIO ethernet Phys

[PATCH v2a 1/2] dt: Document a compatible entry for MDIO ethernet Phys

[Jason Gunthorpe]

This describes a compatible entry of the form:
  ethernet-phy-idAAAA,BBBB
Which is modelled after the PCI structured compatible entry
(pciVVVV,DDDD.SSSS.ssss.RR)

If present the OF core will be able to use this information to
directly create the correct phy without auto probing the bus.

Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
Acked-by: Rob Herring <robh@kernel.org>
Acked-by: Florian Fainelli <f.fainelli@gmail.com>
---
 Documentation/devicetree/bindings/net/phy.txt | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

- Resend for net-next

diff --git a/Documentation/devicetree/bindings/net/phy.txt b/Documentation/devicetree/bindings/net/phy.txt
index 58307d0..5b8c589 100644
--- a/Documentation/devicetree/bindings/net/phy.txt
+++ b/Documentation/devicetree/bindings/net/phy.txt
@@ -21,10 +21,18 @@ Optional Properties:
   elements.
 - max-speed: Maximum PHY supported speed (10, 100, 1000...)
 
+  If the phy's identifier is known then the list may contain an entry
+  of the form: "ethernet-phy-idAAAA.BBBB" where
+     AAAA - The value of the 16 bit Phy Identifier 1 register as
+            4 hex digits. This is the chip vendor OUI bits 3:18
+     BBBB - The value of the 16 bit Phy Identifier 2 register as
+            4 hex digits. This is the chip vendor OUI bits 19:24,
+            followed by 10 bits of a vendor specific ID.
+
 Example:
 
 ethernet-phy@0 {
-	compatible = "ethernet-phy-ieee802.3-c22";
+	compatible = "ethernet-phy-id0141.0e90", "ethernet-phy-ieee802.3-c22";
 	interrupt-parent = <40000>;
 	interrupts = <35 1>;
 	reg = <0>;
-- 
1.8.1.2

[PATCH v3 2/2] of_mdio: Allow the DT to specify the phy ID and avoid autoprobing

[Jason Gunthorpe]

This makes the generic of_mdiobus_register parse the DT compatible string for
the pattern ethernet-phy-idAAAA.BBBB. If present it should be a value that
matches the phy-id register normally readable through MDIO.

When the ID is given the phy autoprobing is defeated and the phy is
created directly.

This is necessary to support phy's that cannot be autoprobed when
of_mdiobus_register is called. Specifically, my case has the phy in reset at
of_mdiobus_register, the reset is only released once the ethernet driver
starts, before it attaches to the phy.

Tested on ARM Kirkwood with phy id 0x01410e90 (Marvell 88E1318)

Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
Acked-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/of/of_mdio.c | 23 ++++++++++++++++++++++-
 1 file changed, 22 insertions(+), 1 deletion(-)

For net-next

v3:
 - Use of_property_for_each_string as per Rob Herring
 - Rebase+retest against 3.14-rc7

diff --git a/drivers/of/of_mdio.c b/drivers/of/of_mdio.c
index 5b3c24f..9a95831 100644
--- a/drivers/of/of_mdio.c
+++ b/drivers/of/of_mdio.c
@@ -43,6 +43,23 @@ static void of_set_phy_supported(struct phy_device *phydev, u32 max_speed)
 	}
 }
 
+/* Extract the clause 22 phy ID from the compatible string of the form
+ * ethernet-phy-idAAAA.BBBB */
+static int of_get_phy_id(struct device_node *device, u32 *phy_id)
+{
+	struct property *prop;
+	const char *cp;
+	unsigned int upper, lower;
+
+	of_property_for_each_string(device, "compatible", prop, cp) {
+		if (sscanf(cp, "ethernet-phy-id%4x.%4x", &upper, &lower) == 2) {
+			*phy_id = ((upper & 0xFFFF) << 16) | (lower & 0xFFFF);
+			return 0;
+		}
+	}
+	return -EINVAL;
+}
+
 static int of_mdiobus_register_phy(struct mii_bus *mdio, struct device_node *child,
 				   u32 addr)
 {
@@ -50,11 +67,15 @@ static int of_mdiobus_register_phy(struct mii_bus *mdio, struct device_node *chi
 	bool is_c45;
 	int rc;
 	u32 max_speed = 0;
+	u32 phy_id;
 
 	is_c45 = of_device_is_compatible(child,
 					 "ethernet-phy-ieee802.3-c45");
 
-	phy = get_phy_device(mdio, addr, is_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 (!phy || IS_ERR(phy))
 		return 1;
 
-- 
1.8.1.2

Re: [PATCH v3 2/2] of_mdio: Allow the DT to specify the phy ID and avoid autoprobing

[Rob Herring]

On Wed, Mar 19, 2014 at 5:15 PM, Jason Gunthorpe
<jgunthorpe@obsidianresearch.com> wrote:
> This makes the generic of_mdiobus_register parse the DT compatible string for
> the pattern ethernet-phy-idAAAA.BBBB. If present it should be a value that
> matches the phy-id register normally readable through MDIO.
>
> When the ID is given the phy autoprobing is defeated and the phy is
> created directly.
>
> This is necessary to support phy's that cannot be autoprobed when
> of_mdiobus_register is called. Specifically, my case has the phy in reset at
> of_mdiobus_register, the reset is only released once the ethernet driver
> starts, before it attaches to the phy.
>
> Tested on ARM Kirkwood with phy id 0x01410e90 (Marvell 88E1318)
>
> Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
> Acked-by: Florian Fainelli <f.fainelli@gmail.com>

Despite the outrageous commenting style issues:

Acked-by: Rob Herring <robh@kernel.org>


> ---
>  drivers/of/of_mdio.c | 23 ++++++++++++++++++++++-
>  1 file changed, 22 insertions(+), 1 deletion(-)
>
> For net-next
>
> v3:
>  - Use of_property_for_each_string as per Rob Herring
>  - Rebase+retest against 3.14-rc7
>
> diff --git a/drivers/of/of_mdio.c b/drivers/of/of_mdio.c
> index 5b3c24f..9a95831 100644
> --- a/drivers/of/of_mdio.c
> +++ b/drivers/of/of_mdio.c
> @@ -43,6 +43,23 @@ static void of_set_phy_supported(struct phy_device *phydev, u32 max_speed)
>         }
>  }
>
> +/* Extract the clause 22 phy ID from the compatible string of the form
> + * ethernet-phy-idAAAA.BBBB */
> +static int of_get_phy_id(struct device_node *device, u32 *phy_id)
> +{
> +       struct property *prop;
> +       const char *cp;
> +       unsigned int upper, lower;
> +
> +       of_property_for_each_string(device, "compatible", prop, cp) {
> +               if (sscanf(cp, "ethernet-phy-id%4x.%4x", &upper, &lower) == 2) {
> +                       *phy_id = ((upper & 0xFFFF) << 16) | (lower & 0xFFFF);
> +                       return 0;
> +               }
> +       }
> +       return -EINVAL;
> +}
> +
>  static int of_mdiobus_register_phy(struct mii_bus *mdio, struct device_node *child,
>                                    u32 addr)
>  {
> @@ -50,11 +67,15 @@ static int of_mdiobus_register_phy(struct mii_bus *mdio, struct device_node *chi
>         bool is_c45;
>         int rc;
>         u32 max_speed = 0;
> +       u32 phy_id;
>
>         is_c45 = of_device_is_compatible(child,
>                                          "ethernet-phy-ieee802.3-c45");
>
> -       phy = get_phy_device(mdio, addr, is_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 (!phy || IS_ERR(phy))
>                 return 1;
>
> --
> 1.8.1.2
>

Re: [PATCH v2a 1/2] dt: Document a compatible entry for MDIO ethernet Phys

[Ben Dooks]

On 19/03/14 23:15, Jason Gunthorpe wrote:
> This describes a compatible entry of the form:
>    ethernet-phy-idAAAA,BBBB
> Which is modelled after the PCI structured compatible entry
> (pciVVVV,DDDD.SSSS.ssss.RR)
>
> If present the OF core will be able to use this information to
> directly create the correct phy without auto probing the bus.
>
> Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
> Acked-by: Rob Herring <robh@kernel.org>
> Acked-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
>   Documentation/devicetree/bindings/net/phy.txt | 10 +++++++++-
>   1 file changed, 9 insertions(+), 1 deletion(-)
>
> - Resend for net-next
>
> diff --git a/Documentation/devicetree/bindings/net/phy.txt b/Documentation/devicetree/bindings/net/phy.txt
> index 58307d0..5b8c589 100644
> --- a/Documentation/devicetree/bindings/net/phy.txt
> +++ b/Documentation/devicetree/bindings/net/phy.txt
> @@ -21,10 +21,18 @@ Optional Properties:
>     elements.
>   - max-speed: Maximum PHY supported speed (10, 100, 1000...)
>
> +  If the phy's identifier is known then the list may contain an entry
> +  of the form: "ethernet-phy-idAAAA.BBBB" where
> +     AAAA - The value of the 16 bit Phy Identifier 1 register as
> +            4 hex digits. This is the chip vendor OUI bits 3:18
> +     BBBB - The value of the 16 bit Phy Identifier 2 register as
> +            4 hex digits. This is the chip vendor OUI bits 19:24,
> +            followed by 10 bits of a vendor specific ID.
> +

How about having "ethernet-phy" and adding a single property in the
PHY node saying phy-id = <AAAABBBB>, which would be much easier to
parse.

>   Example:
>
>   ethernet-phy@0 {
> -	compatible = "ethernet-phy-ieee802.3-c22";
> +	compatible = "ethernet-phy-id0141.0e90", "ethernet-phy-ieee802.3-c22";
>   	interrupt-parent = <40000>;
>   	interrupts = <35 1>;
>   	reg = <0>;
>


-- 
Ben Dooks				http://www.codethink.co.uk/
Senior Engineer				Codethink - Providing Genius

Re: [PATCH v2a 1/2] dt: Document a compatible entry for MDIO ethernet Phys

[Jason Gunthorpe]

On Thu, Mar 20, 2014 at 09:26:28AM +0100, Ben Dooks wrote:

> >+  If the phy's identifier is known then the list may contain an entry
> >+  of the form: "ethernet-phy-idAAAA.BBBB" where
> >+     AAAA - The value of the 16 bit Phy Identifier 1 register as
> >+            4 hex digits. This is the chip vendor OUI bits 3:18
> >+     BBBB - The value of the 16 bit Phy Identifier 2 register as
> >+            4 hex digits. This is the chip vendor OUI bits 19:24,
> >+            followed by 10 bits of a vendor specific ID.
> >+
> 
> How about having "ethernet-phy" and adding a single property in the
> PHY node saying phy-id = <AAAABBBB>, which would be much easier to
> parse.

That option was discussed (and my first patch was this way)..

However, using the compatible string is consistent with other OF
schemes, such as PCI which uses compatible strings like pciVVVV,DDDD
pciclass,CCSS and so on.

It does make a fair amount sense because the purpose of compatible is
to identify the device.

Jason

Re: [PATCH v2a 1/2] dt: Document a compatible entry for MDIO ethernet Phys

[David Miller]

From: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
Date: Wed, 19 Mar 2014 16:15:23 -0600

> This describes a compatible entry of the form:
>   ethernet-phy-idAAAA,BBBB
> Which is modelled after the PCI structured compatible entry
> (pciVVVV,DDDD.SSSS.ssss.RR)
> 
> If present the OF core will be able to use this information to
> directly create the correct phy without auto probing the bus.
> 
> Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
> Acked-by: Rob Herring <robh@kernel.org>
> Acked-by: Florian Fainelli <f.fainelli@gmail.com>

Applied to net-next.

Re: [PATCH v3 2/2] of_mdio: Allow the DT to specify the phy ID and avoid autoprobing

[David Miller]

From: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
Date: Wed, 19 Mar 2014 16:15:24 -0600

> This makes the generic of_mdiobus_register parse the DT compatible string for
> the pattern ethernet-phy-idAAAA.BBBB. If present it should be a value that
> matches the phy-id register normally readable through MDIO.
> 
> When the ID is given the phy autoprobing is defeated and the phy is
> created directly.
> 
> This is necessary to support phy's that cannot be autoprobed when
> of_mdiobus_register is called. Specifically, my case has the phy in reset at
> of_mdiobus_register, the reset is only released once the ethernet driver
> starts, before it attaches to the phy.
> 
> Tested on ARM Kirkwood with phy id 0x01410e90 (Marvell 88E1318)
> 
> Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
> Acked-by: Florian Fainelli <f.fainelli@gmail.com>

Also applied to net-next.