[PATCH] of_mdio: use of_property_read_u32_array()

[PATCH] of_mdio: use of_property_read_u32_array()

[Sergei Shtylyov]

[-- Attachment #1: of_mdio-use-of_property_read_u32_array.patch --]
[-- Type: text/plain, Size: 2069 bytes --]

The "fixed-link" prop support predated of_property_read_u32_array(), so
basically had to open-code it. Using the modern API saves 24 bytes of the
object code (ARM gcc 4.8.5); the only behavior change would be that the
prop length check is now less strict (however the strict pre-check done
in of_phy_is_fixed_link() is left intact anyway)...

Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

---
The patch is against the 'dt/next' branch of Rob Herring's 'linux-git' repo
plus the previously posted patch killing the useless local variable in
of_phy_register_fixed_link().

 drivers/of/of_mdio.c |   16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

Index: linux/drivers/of/of_mdio.c
===================================================================
--- linux.orig/drivers/of/of_mdio.c
+++ linux/drivers/of/of_mdio.c
@@ -421,10 +421,10 @@ int of_phy_register_fixed_link(struct de
 {
 	struct fixed_phy_status status = {};
 	struct device_node *fixed_link_node;
-	const __be32 *fixed_link_prop;
+	u32 fixed_link_prop[5];
 	struct phy_device *phy;
 	const char *managed;
-	int link_gpio, len;
+	int link_gpio;
 
 	if (of_property_read_string(np, "managed", &managed) == 0) {
 		if (strcmp(managed, "in-band-status") == 0) {
@@ -459,13 +459,13 @@ int of_phy_register_fixed_link(struct de
 	}
 
 	/* Old binding */
-	fixed_link_prop = of_get_property(np, "fixed-link", &len);
-	if (fixed_link_prop && len == (5 * sizeof(__be32))) {
+	if (of_property_read_u32_array(np, "fixed-link", fixed_link_prop,
+				       ARRAY_SIZE(fixed_link_prop)) == 0) {
 		status.link = 1;
-		status.duplex = be32_to_cpu(fixed_link_prop[1]);
-		status.speed = be32_to_cpu(fixed_link_prop[2]);
-		status.pause = be32_to_cpu(fixed_link_prop[3]);
-		status.asym_pause = be32_to_cpu(fixed_link_prop[4]);
+		status.duplex = fixed_link_prop[1];
+		status.speed  = fixed_link_prop[2];
+		status.pause  = fixed_link_prop[3];
+		status.asym_pause = fixed_link_prop[4];
 		phy = fixed_phy_register(PHY_POLL, &status, -1, np);
 		return PTR_ERR_OR_ZERO(phy);
 	}

Re: [PATCH] of_mdio: use of_property_read_u32_array()

[Andrew Lunn]

On Sat, Aug 05, 2017 at 12:43:43AM +0300, Sergei Shtylyov wrote:
> The "fixed-link" prop support predated of_property_read_u32_array(), so
> basically had to open-code it. Using the modern API saves 24 bytes of the
> object code (ARM gcc 4.8.5); the only behavior change would be that the
> prop length check is now less strict (however the strict pre-check done
> in of_phy_is_fixed_link() is left intact anyway)...
> 
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov-M4DtvfQ/ZS1MRgGoP+s0PdBPR1lH4CV8@public.gmane.org>

Reviewed-by: Andrew Lunn <andrew-g2DYL2Zd6BY@public.gmane.org>

    Andrew
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] of_mdio: use of_property_read_u32_array()

[Rob Herring]

On Fri, Aug 4, 2017 at 4:43 PM, Sergei Shtylyov
<sergei.shtylyov-M4DtvfQ/ZS1MRgGoP+s0PdBPR1lH4CV8@public.gmane.org> wrote:
> The "fixed-link" prop support predated of_property_read_u32_array(), so
> basically had to open-code it. Using the modern API saves 24 bytes of the
> object code (ARM gcc 4.8.5); the only behavior change would be that the
> prop length check is now less strict (however the strict pre-check done
> in of_phy_is_fixed_link() is left intact anyway)...
>
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov-M4DtvfQ/ZS1MRgGoP+s0PdBPR1lH4CV8@public.gmane.org>
>
> ---
> The patch is against the 'dt/next' branch of Rob Herring's 'linux-git' repo
> plus the previously posted patch killing the useless local variable in
> of_phy_register_fixed_link().

It shouldn't depend on anything in my tree and David normally takes
of_mdio.c changes.

Reviewed-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

>
>  drivers/of/of_mdio.c |   16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] of_mdio: use of_property_read_u32_array()

[Sergei Shtylyov]

Hello!

On 08/07/2017 05:18 PM, Rob Herring wrote:

>> The "fixed-link" prop support predated of_property_read_u32_array(), so
>> basically had to open-code it. Using the modern API saves 24 bytes of the
>> object code (ARM gcc 4.8.5); the only behavior change would be that the
>> prop length check is now less strict (however the strict pre-check done
>> in of_phy_is_fixed_link() is left intact anyway)...
>>
>> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
>>
>> ---
>> The patch is against the 'dt/next' branch of Rob Herring's 'linux-git' repo
>> plus the previously posted patch killing the useless local variable in
>> of_phy_register_fixed_link().
> 
> It shouldn't depend on anything in my tree and David normally takes
> of_mdio.c changes.

    MAINTAINERS still only point at the DT repo, perhaps it should be updated?

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

    Thank you.

>>   drivers/of/of_mdio.c |   16 ++++++++--------
>>   1 file changed, 8 insertions(+), 8 deletions(-)

MBR, Sergei

Re: [PATCH] of_mdio: use of_property_read_u32_array()

[Florian Fainelli]

On 08/07/2017 09:18 AM, Sergei Shtylyov wrote:
> Hello!
> 
> On 08/07/2017 05:18 PM, Rob Herring wrote:
> 
>>> The "fixed-link" prop support predated of_property_read_u32_array(), so
>>> basically had to open-code it. Using the modern API saves 24 bytes of
>>> the
>>> object code (ARM gcc 4.8.5); the only behavior change would be that the
>>> prop length check is now less strict (however the strict pre-check done
>>> in of_phy_is_fixed_link() is left intact anyway)...
>>>
>>> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
>>>
>>> ---
>>> The patch is against the 'dt/next' branch of Rob Herring's
>>> 'linux-git' repo
>>> plus the previously posted patch killing the useless local variable in
>>> of_phy_register_fixed_link().
>>
>> It shouldn't depend on anything in my tree and David normally takes
>> of_mdio.c changes.
> 
>    MAINTAINERS still only point at the DT repo, perhaps it should be
> updated?

More or less done with this (minus the repo part):

http://patchwork.ozlabs.org/patch/795887/
-- 
Florian

Re: [PATCH] of_mdio: use of_property_read_u32_array()

[Rob Herring]

On Mon, Aug 7, 2017 at 1:01 PM, Florian Fainelli <f.fainelli@gmail.com> wrote:
> On 08/07/2017 09:18 AM, Sergei Shtylyov wrote:
>> Hello!
>>
>> On 08/07/2017 05:18 PM, Rob Herring wrote:
>>
>>>> The "fixed-link" prop support predated of_property_read_u32_array(), so
>>>> basically had to open-code it. Using the modern API saves 24 bytes of
>>>> the
>>>> object code (ARM gcc 4.8.5); the only behavior change would be that the
>>>> prop length check is now less strict (however the strict pre-check done
>>>> in of_phy_is_fixed_link() is left intact anyway)...
>>>>
>>>> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
>>>>
>>>> ---
>>>> The patch is against the 'dt/next' branch of Rob Herring's
>>>> 'linux-git' repo
>>>> plus the previously posted patch killing the useless local variable in
>>>> of_phy_register_fixed_link().
>>>
>>> It shouldn't depend on anything in my tree and David normally takes
>>> of_mdio.c changes.
>>
>>    MAINTAINERS still only point at the DT repo, perhaps it should be
>> updated?
>
> More or less done with this (minus the repo part):
>
> http://patchwork.ozlabs.org/patch/795887/

Really I'd like to see this fixed by moving of_mdio.c and of_net.c to
drivers/net/ as we've done for all other subsystems.

Rob

Re: [PATCH] of_mdio: use of_property_read_u32_array()

[David Miller]

From: Sergei Shtylyov <sergei.shtylyov-M4DtvfQ/ZS1MRgGoP+s0PdBPR1lH4CV8@public.gmane.org>
Date: Sat, 05 Aug 2017 00:43:43 +0300

> The "fixed-link" prop support predated of_property_read_u32_array(), so
> basically had to open-code it. Using the modern API saves 24 bytes of the
> object code (ARM gcc 4.8.5); the only behavior change would be that the
> prop length check is now less strict (however the strict pre-check done
> in of_phy_is_fixed_link() is left intact anyway)...
> 
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov-M4DtvfQ/ZS1MRgGoP+s0PdBPR1lH4CV8@public.gmane.org>

Applied to net-next.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html