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

[PATCH v2 RESEND 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(-)

This version is just a resend rebased against 3.14-rc6, with the minor merge
conflicts resolved.

I'm not sure who is supposed to pick this patch up?

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 v2a RESEND 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 | 30 +++++++++++++++++++++++++++++-
 1 file changed, 29 insertions(+), 1 deletion(-)

There was some reworking of the original code in v3.14-rc6, this resolves the
merge conflicts and I retested the resolved patch on Kirkwood.

Florian, do you know what path to mainline a this patch should take?

Thanks,
Jason

diff --git a/drivers/of/of_mdio.c b/drivers/of/of_mdio.c
index 5b3c24f..6cdcbda 100644
--- a/drivers/of/of_mdio.c
+++ b/drivers/of/of_mdio.c
@@ -43,6 +43,30 @@ 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)
+{
+	const char *cp;
+	int cplen, l;
+	unsigned int upper, lower;
+
+	cp = of_get_property(device, "compatible", &cplen);
+	if (cp == NULL)
+		return -EINVAL;
+	while (cplen > 0) {
+		if (sscanf(cp, "ethernet-phy-id%4x.%4x", &upper, &lower) == 2) {
+			*phy_id = ((upper & 0xFFFF) << 16) | (lower & 0xFFFF);
+			return 0;
+		}
+
+		l = strlen(cp) + 1;
+		cp += l;
+		cplen -= l;
+	}
+	return -EINVAL;
+}
+
 static int of_mdiobus_register_phy(struct mii_bus *mdio, struct device_node *child,
 				   u32 addr)
 {
@@ -50,11 +74,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 RESEND 2/2] of_mdio: Allow the DT to specify the phy ID and avoid autoprobing

[Rob Herring]

On Wed, Mar 12, 2014 at 4:05 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>
> ---
>  drivers/of/of_mdio.c | 30 +++++++++++++++++++++++++++++-
>  1 file changed, 29 insertions(+), 1 deletion(-)
>
> There was some reworking of the original code in v3.14-rc6, this resolves the
> merge conflicts and I retested the resolved patch on Kirkwood.
>
> Florian, do you know what path to mainline a this patch should take?

Via David's net tree.

>
> Thanks,
> Jason
>
> diff --git a/drivers/of/of_mdio.c b/drivers/of/of_mdio.c
> index 5b3c24f..6cdcbda 100644
> --- a/drivers/of/of_mdio.c
> +++ b/drivers/of/of_mdio.c
> @@ -43,6 +43,30 @@ 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)
> +{
> +       const char *cp;
> +       int cplen, l;
> +       unsigned int upper, lower;
> +
> +       cp = of_get_property(device, "compatible", &cplen);
> +       if (cp == NULL)
> +               return -EINVAL;
> +       while (cplen > 0) {

This can all be:

  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;
> +               }
> +
> +               l = strlen(cp) + 1;
> +               cp += l;
> +               cplen -= l;
> +       }
> +       return -EINVAL;
> +}
> +
>  static int of_mdiobus_register_phy(struct mii_bus *mdio, struct device_node *child,
>                                    u32 addr)
>  {
> @@ -50,11 +74,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 RESEND 2/2] of_mdio: Allow the DT to specify the phy ID and avoid autoprobing

[Florian Fainelli]

2014-03-12 14:35 GMT-07:00 Rob Herring <robherring2@gmail.com>:
> On Wed, Mar 12, 2014 at 4:05 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>
>> ---
>>  drivers/of/of_mdio.c | 30 +++++++++++++++++++++++++++++-
>>  1 file changed, 29 insertions(+), 1 deletion(-)
>>
>> There was some reworking of the original code in v3.14-rc6, this resolves the
>> merge conflicts and I retested the resolved patch on Kirkwood.
>>
>> Florian, do you know what path to mainline a this patch should take?
>
> Via David's net tree.

In that case, this is probably more relevant for the 'net-next' tree
since it is not a bug fix and we are fairly late in the 3.14 cycle?
-- 
Florian

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

[Sergei Shtylyov]

Hello.

On 03/13/2014 12:05 AM, Jason Gunthorpe 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>
> ---
>   drivers/of/of_mdio.c | 30 +++++++++++++++++++++++++++++-
>   1 file changed, 29 insertions(+), 1 deletion(-)

> There was some reworking of the original code in v3.14-rc6, this resolves the
> merge conflicts and I retested the resolved patch on Kirkwood.

> Florian, do you know what path to mainline a this patch should take?

> Thanks,
> Jason

> diff --git a/drivers/of/of_mdio.c b/drivers/of/of_mdio.c
> index 5b3c24f..6cdcbda 100644
> --- a/drivers/of/of_mdio.c
> +++ b/drivers/of/of_mdio.c
> @@ -43,6 +43,30 @@ 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 */

    The preferred multi-line comment style is:

/*
  * bla
  * bla
  */

WBR, Sergei

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

[Jason Gunthorpe]

On Wed, Mar 12, 2014 at 04:35:54PM -0500, Rob Herring wrote:

> > Florian, do you know what path to mainline a this patch should take?
> 
> Via David's net tree.

Thanks

> > +       cp = of_get_property(device, "compatible", &cplen);
> > +       if (cp == NULL)
> > +               return -EINVAL;
> > +       while (cplen > 0) {
> 
> This can all be:
> 
>   of_property_for_each_string(device, "compatible", prop, cp)

Done

On Thu, Mar 13, 2014 at 01:46:18AM +0300, Sergei Shtylyov wrote:
>    The preferred multi-line comment style is:
> 
> /*
>  * bla
>  * bla
>  */
> 

Sergei, the entire file use the 'net' multi-line commenting style, and
I have maintined that consistency with the new code.

v3 on its way.

Regards,
Jason

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

[Jason Gunthorpe]

On Thu, Mar 20, 2014 at 02:20:11AM +0300, Sergei Shtylyov wrote:
> Hello.
> 
> On 03/20/2014 01:14 AM, Jason Gunthorpe wrote:
> 
> >>    The preferred multi-line comment style is:
> 
> >>/*
> >>  * bla
> >>  * bla
> >>  */
> 
> >Sergei, the entire file use the 'net' multi-line commenting style, and
> >I have maintined that consistency with the new code.
> 
>    No, you haven't followed the networking style. It is:
> 
> /* bla
>  * bla
>  */
> 
> while your comment used:
> 
> /* bla
>  * bla */

Oh Ok - but the latter is the used by the majority of pre-existing
comments..

I thought the convention on stuff like this was to stick with the
existing style?

Do you think it is worth a v4? If so, which style is appropriate?
There is one other pre-existing comment that uses net.

Regards,
Jason

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

[Sergei Shtylyov]

Hello.

On 03/20/2014 01:14 AM, Jason Gunthorpe wrote:

>>     The preferred multi-line comment style is:

>> /*
>>   * bla
>>   * bla
>>   */

> Sergei, the entire file use the 'net' multi-line commenting style, and
> I have maintined that consistency with the new code.

    No, you haven't followed the networking style. It is:

/* bla
  * bla
  */

while your comment used:

/* bla
  * bla */

> v3 on its way.

> Regards,
> Jason

WBR, Sergei

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

[Sergei Shtylyov]

On 03/20/2014 01:36 AM, Jason Gunthorpe wrote:

>>>>     The preferred multi-line comment style is:

>>>> /*
>>>>   * bla
>>>>   * bla
>>>>   */

>>> Sergei, the entire file use the 'net' multi-line commenting style, and
>>> I have maintined that consistency with the new code.

>>     No, you haven't followed the networking style. It is:

>> /* bla
>>   * bla
>>   */

>> while your comment used:

>> /* bla
>>   * bla */

> Oh Ok - but the latter is the used by the majority of pre-existing
> comments..

> I thought the convention on stuff like this was to stick with the
> existing style?

    Hardly so when there's preferred one, described in CodingStyle.

> Do you think it is worth a v4? If so, which style is appropriate?
> There is one other pre-existing comment that uses net.

    Well, use the net style then. I don't see significant difference between 
it and the "default" preferred style.

> Regards,
> Jason

WBR, Sergei