Re: Nested function in drivers/of/of_mdio.c

2009-10-08 Thread Jérôme Pouiller 
I did some grep on codebase. I have not found any other instances of 
nested functions, but my regexps are not enough to be 100% sure.

On Wednesday 07 October 2009 18:23:04 v...@vsbe.com wrote:
 Guys, are there other instances of nested C functions in the codebase
 or was this the first attempt?
 
 On Wed, Oct 7, 2009 at 9:11 AM, Grant Likely 
grant.lik...@secretlab.ca wrote:
  On Wed, Oct 7, 2009 at 9:15 AM, Jérôme Pouiller j...@sysmic.org 
wrote:
  Dear,
 
  I have a problem with commit
  8bc487d150b939e69830c39322df4ee486efe381 in file
  drivers/of/of_mdio.c in function of_phy_find_device.
 
  As you see, this function define match() as a nested function. My
  compiler (powerpc-e500-linux-gnu-gcc-3.4.1) raise an error during
  link due to this nested definition:
   drivers/built-in.o(.text+0x5e2a4): In function
  `of_phy_find_device':
  /home/jezz/linux-next/drivers/of/of_mdio.c:107: undefined
  reference to `__trampoline_setup'
 
  I am sure I could solve problem by rebuilding my toolchain.
  Nevertheless, I think nested function definition is not perfectly
  supported by all compilers. Also, I suggest to place function
  match() outside of scope of of_phy_find_device as in following
  patch.
 
  I'm okay with that, but if you're moving code out of the file
  scope, then please rename the function to of_phy_match() to avoid
  global namespace conflicts.
 
  g.
 
  diff --git a/drivers/of/of_mdio.c b/drivers/of/of_mdio.c
  index bacaa53..c7b2e26 100644
  --- a/drivers/of/of_mdio.c
  +++ b/drivers/of/of_mdio.c
  @@ -97,6 +97,10 @@ int of_mdiobus_register(struct mii_bus *mdio,
  struct device_node *np) }
   EXPORT_SYMBOL(of_mdiobus_register);
 
  +static int match(struct device *dev, void *phy_np)
  +{
  +   return dev_archdata_get_node(dev-archdata) == phy_np;
  +}
   /**
   * of_phy_find_device - Give a PHY node, find the phy_device
   * @phy_np: Pointer to the phy's device tree node
  @@ -106,11 +110,6 @@ EXPORT_SYMBOL(of_mdiobus_register);
   struct phy_device *of_phy_find_device(struct device_node *phy_np)
   {
 struct device *d;
  -   int match(struct device *dev, void *phy_np)
  -   {
  -   return dev_archdata_get_node(dev-archdata) ==
  phy_np; -   }
  -
 if (!phy_np)
 return NULL;
 
 
  What do you think about it?
 
  Best regards,
 
  --
  Jérôme Pouiller (jezz AT sysmic DOT org)
 
  --
  Grant Likely, B.Sc., P.Eng.
  Secret Lab Technologies Ltd.
  ___
  Linuxppc-dev mailing list
  Linuxppc-dev@lists.ozlabs.org
  https://lists.ozlabs.org/listinfo/linuxppc-dev
 
 ___
 Linuxppc-dev mailing list
 Linuxppc-dev@lists.ozlabs.org
 https://lists.ozlabs.org/listinfo/linuxppc-dev
 

-- 
Jérôme Pouiller (jerome AT sysmic DOT org)
Expert Linux Embarqué
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Nested function in drivers/of/of_mdio.c

2009-10-07 Thread Jérôme Pouiller 
Dear,

I have a problem with commit 8bc487d150b939e69830c39322df4ee486efe381 
in file drivers/of/of_mdio.c in function of_phy_find_device.

As you see, this function define match() as a nested function. My 
compiler (powerpc-e500-linux-gnu-gcc-3.4.1) raise an error during link 
due to this nested definition:
  drivers/built-in.o(.text+0x5e2a4): In function `of_phy_find_device': 
/home/jezz/linux-next/drivers/of/of_mdio.c:107: 
undefined reference to `__trampoline_setup'

I am sure I could solve problem by rebuilding my toolchain. 
Nevertheless, I think nested function definition is not perfectly 
supported by all compilers. Also, I suggest to place function match() 
outside of scope of of_phy_find_device as in following patch.

diff --git a/drivers/of/of_mdio.c b/drivers/of/of_mdio.c
index bacaa53..c7b2e26 100644
--- a/drivers/of/of_mdio.c
+++ b/drivers/of/of_mdio.c
@@ -97,6 +97,10 @@ int of_mdiobus_register(struct mii_bus *mdio, struct 
device_node *np)
 }
 EXPORT_SYMBOL(of_mdiobus_register);

+static int match(struct device *dev, void *phy_np)
+{
+   return dev_archdata_get_node(dev-archdata) == phy_np;
+}
 /**
  * of_phy_find_device - Give a PHY node, find the phy_device
  * @phy_np: Pointer to the phy's device tree node
@@ -106,11 +110,6 @@ EXPORT_SYMBOL(of_mdiobus_register);
 struct phy_device *of_phy_find_device(struct device_node *phy_np)
 {
struct device *d;
-   int match(struct device *dev, void *phy_np)
-   {
-   return dev_archdata_get_node(dev-archdata) == phy_np;
-   }
-
if (!phy_np)
return NULL;


What do you think about it?

Best regards,

-- 
Jérôme Pouiller (jezz AT sysmic DOT org)
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: Nested function in drivers/of/of_mdio.c

2009-10-07 Thread Grant Likely 
On Wed, Oct 7, 2009 at 9:15 AM, Jérôme Pouiller j...@sysmic.org wrote:
 Dear,

 I have a problem with commit 8bc487d150b939e69830c39322df4ee486efe381
 in file drivers/of/of_mdio.c in function of_phy_find_device.

 As you see, this function define match() as a nested function. My
 compiler (powerpc-e500-linux-gnu-gcc-3.4.1) raise an error during link
 due to this nested definition:
  drivers/built-in.o(.text+0x5e2a4): In function `of_phy_find_device': 
 /home/jezz/linux-next/drivers/of/of_mdio.c:107:
 undefined reference to `__trampoline_setup'

 I am sure I could solve problem by rebuilding my toolchain.
 Nevertheless, I think nested function definition is not perfectly
 supported by all compilers. Also, I suggest to place function match()
 outside of scope of of_phy_find_device as in following patch.

I'm okay with that, but if you're moving code out of the file scope,
then please rename the function to of_phy_match() to avoid global
namespace conflicts.

g.


 diff --git a/drivers/of/of_mdio.c b/drivers/of/of_mdio.c
 index bacaa53..c7b2e26 100644
 --- a/drivers/of/of_mdio.c
 +++ b/drivers/of/of_mdio.c
 @@ -97,6 +97,10 @@ int of_mdiobus_register(struct mii_bus *mdio, struct 
 device_node *np)
  }
  EXPORT_SYMBOL(of_mdiobus_register);

 +static int match(struct device *dev, void *phy_np)
 +{
 +       return dev_archdata_get_node(dev-archdata) == phy_np;
 +}
  /**
  * of_phy_find_device - Give a PHY node, find the phy_device
  * @phy_np: Pointer to the phy's device tree node
 @@ -106,11 +110,6 @@ EXPORT_SYMBOL(of_mdiobus_register);
  struct phy_device *of_phy_find_device(struct device_node *phy_np)
  {
        struct device *d;
 -       int match(struct device *dev, void *phy_np)
 -       {
 -               return dev_archdata_get_node(dev-archdata) == phy_np;
 -       }
 -
        if (!phy_np)
                return NULL;


 What do you think about it?

 Best regards,

 --
 Jérôme Pouiller (jezz AT sysmic DOT org)




-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: Nested function in drivers/of/of_mdio.c

2009-10-07 Thread vb 
Guys, are there other instances of nested C functions in the codebase
or was this the first attempt?


On Wed, Oct 7, 2009 at 9:11 AM, Grant Likely grant.lik...@secretlab.ca wrote:
 On Wed, Oct 7, 2009 at 9:15 AM, Jérôme Pouiller j...@sysmic.org wrote:
 Dear,

 I have a problem with commit 8bc487d150b939e69830c39322df4ee486efe381
 in file drivers/of/of_mdio.c in function of_phy_find_device.

 As you see, this function define match() as a nested function. My
 compiler (powerpc-e500-linux-gnu-gcc-3.4.1) raise an error during link
 due to this nested definition:
  drivers/built-in.o(.text+0x5e2a4): In function `of_phy_find_device': 
 /home/jezz/linux-next/drivers/of/of_mdio.c:107:
 undefined reference to `__trampoline_setup'

 I am sure I could solve problem by rebuilding my toolchain.
 Nevertheless, I think nested function definition is not perfectly
 supported by all compilers. Also, I suggest to place function match()
 outside of scope of of_phy_find_device as in following patch.

 I'm okay with that, but if you're moving code out of the file scope,
 then please rename the function to of_phy_match() to avoid global
 namespace conflicts.

 g.


 diff --git a/drivers/of/of_mdio.c b/drivers/of/of_mdio.c
 index bacaa53..c7b2e26 100644
 --- a/drivers/of/of_mdio.c
 +++ b/drivers/of/of_mdio.c
 @@ -97,6 +97,10 @@ int of_mdiobus_register(struct mii_bus *mdio, struct 
 device_node *np)
  }
  EXPORT_SYMBOL(of_mdiobus_register);

 +static int match(struct device *dev, void *phy_np)
 +{
 +       return dev_archdata_get_node(dev-archdata) == phy_np;
 +}
  /**
  * of_phy_find_device - Give a PHY node, find the phy_device
  * @phy_np: Pointer to the phy's device tree node
 @@ -106,11 +110,6 @@ EXPORT_SYMBOL(of_mdiobus_register);
  struct phy_device *of_phy_find_device(struct device_node *phy_np)
  {
        struct device *d;
 -       int match(struct device *dev, void *phy_np)
 -       {
 -               return dev_archdata_get_node(dev-archdata) == phy_np;
 -       }
 -
        if (!phy_np)
                return NULL;


 What do you think about it?

 Best regards,

 --
 Jérôme Pouiller (jezz AT sysmic DOT org)




 --
 Grant Likely, B.Sc., P.Eng.
 Secret Lab Technologies Ltd.
 ___
 Linuxppc-dev mailing list
 Linuxppc-dev@lists.ozlabs.org
 https://lists.ozlabs.org/listinfo/linuxppc-dev

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev