| Message ID | 20170804214354.351406407@cogentembedded.com |
|---|---|
| State | Accepted, archived |
| Delegated to: | David Miller |
| Headers | show |
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@cogentembedded.com> Reviewed-by: Andrew Lunn <andrew@lunn.ch> Andrew
On Fri, Aug 4, 2017 at 4:43 PM, Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> 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. Reviewed-by: Rob Herring <robh@kernel.org> > > drivers/of/of_mdio.c | 16 ++++++++-------- > 1 file changed, 8 insertions(+), 8 deletions(-)
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
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/
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
From: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> 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@cogentembedded.com> Applied to net-next.
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); }
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(-)