[2.6.24-rc6-mm1]Build failure in drivers/net/ehea/ehea_main.c

[2.6.24-rc6-mm1]Build failure in drivers/net/ehea/ehea_main.c

[Sudhir Kumar]

Hi Andrew,
Kernel build fails on my machine with error :


  LD      drivers/net/ehea/built-in.o
  CC [M]  drivers/net/ehea/ehea_main.o
drivers/net/ehea/ehea_main.c: In function ‘ehea_driver_sysfs_add’:
drivers/net/ehea/ehea_main.c:2812: error: ‘struct device_driver’ has no
member named ‘kobj’
drivers/net/ehea/ehea_main.c:2815: error: ‘struct device_driver’ has no
member named ‘kobj’
drivers/net/ehea/ehea_main.c:2818: error: ‘struct device_driver’ has no
member named ‘kobj’
drivers/net/ehea/ehea_main.c: In function ‘ehea_driver_sysfs_remove’:
drivers/net/ehea/ehea_main.c:2830: error: ‘struct device_driver’ has no
member named ‘kobj’
make[3]: *** [drivers/net/ehea/ehea_main.o] Error 1
make[2]: *** [drivers/net/ehea] Error 2
make[1]: *** [drivers/net] Error 2
make: *** [drivers] Error 2

The structure device_driver(in device.h) has a member struct driver_private which
contains the member kobj (according to drivers/base/base.h).
But in device.h struct driver_private has been declared localy and 
neither defined nor included from base.h.
So my effort to use driver->driver_private->obj also does not work.
(I am surprised from where do you access the struct device_driver)

Thanks and Regards
Sudhir Kumar

Re: [2.6.24-rc6-mm1]Build failure in drivers/net/ehea/ehea_main.c

[Greg KH]

On Tue, Jan 08, 2008 at 10:03:05PM +0530, Sudhir Kumar wrote:
> Hi Andrew,
> Kernel build fails on my machine with error :
> 
> 
>   LD      drivers/net/ehea/built-in.o
>   CC [M]  drivers/net/ehea/ehea_main.o
> drivers/net/ehea/ehea_main.c: In function ???ehea_driver_sysfs_add???:
> drivers/net/ehea/ehea_main.c:2812: error: ???struct device_driver??? has no
> member named ???kobj???
> drivers/net/ehea/ehea_main.c:2815: error: ???struct device_driver??? has no
> member named ???kobj???
> drivers/net/ehea/ehea_main.c:2818: error: ???struct device_driver??? has no
> member named ???kobj???
> drivers/net/ehea/ehea_main.c: In function ???ehea_driver_sysfs_remove???:
> drivers/net/ehea/ehea_main.c:2830: error: ???struct device_driver??? has no
> member named ???kobj???
> make[3]: *** [drivers/net/ehea/ehea_main.o] Error 1
> make[2]: *** [drivers/net/ehea] Error 2
> make[1]: *** [drivers/net] Error 2
> make: *** [drivers] Error 2
> 
> The structure device_driver(in device.h) has a member struct driver_private which
> contains the member kobj (according to drivers/base/base.h).
> But in device.h struct driver_private has been declared localy and 
> neither defined nor included from base.h.
> So my effort to use driver->driver_private->obj also does not work.
> (I am surprised from where do you access the struct device_driver)

That is because a driver should not be accessing such a field.

And especially not in this manner, why would this driver be creating a
symlink that has already been created by the driver core?  This whole
thing can just be removed with no problems.  Can you try just removing
the ehea_driver_sysfs_add and ehea_driver_sysfs_remove functions to
verify this as I don't have the hardware present to test it out.

thanks,

greg k-h

Re: [2.6.24-rc6-mm1]Build failure in drivers/net/ehea/ehea_main.c

[Jan-Bernd Themann]

Hi,

sorry for answering so late, I'm only tracking netdev and ppc mailing list.

On Thursday 10 January 2008 18:34, Greg KH wrote:
> > The structure device_driver(in device.h) has a member struct driver_private which
> > contains the member kobj (according to drivers/base/base.h).
> > But in device.h struct driver_private has been declared localy and 
> > neither defined nor included from base.h.
> > So my effort to use driver->driver_private->obj also does not work.
> > (I am surprised from where do you access the struct device_driver)
> 
> That is because a driver should not be accessing such a field.
> 
> And especially not in this manner, why would this driver be creating a
> symlink that has already been created by the driver core?  This whole
> thing can just be removed with no problems.  Can you try just removing
> the ehea_driver_sysfs_add and ehea_driver_sysfs_remove functions to
> verify this as I don't have the hardware present to test it out.

The eHEA driver tries to orginize its sys-entries as close as possible to
other ethernet drivers. Each eHEA NIC has multiple ports which is not that
common in PCI. This means that each port is represented by a subdirectory
which has not the "driver" sys-link, only the root directory has.
Some tools expect to have this driver link in each port directory.
That is the reason why this link is created manually.

Are there any other ways to create this link?

Regards,
Jan-Bernd Themann + Christoph Raisch

Re: [2.6.24-rc6-mm1]Build failure in drivers/net/ehea/ehea_main.c

[Nathan Lynch]

Jan-Bernd Themann wrote:
> Hi,
> 
> sorry for answering so late, I'm only tracking netdev and ppc mailing list.
> 
> On Thursday 10 January 2008 18:34, Greg KH wrote:
> > > The structure device_driver(in device.h) has a member struct driver_private which
> > > contains the member kobj (according to drivers/base/base.h).
> > > But in device.h struct driver_private has been declared localy and 
> > > neither defined nor included from base.h.
> > > So my effort to use driver->driver_private->obj also does not work.
> > > (I am surprised from where do you access the struct device_driver)
> > 
> > That is because a driver should not be accessing such a field.
> > 
> > And especially not in this manner, why would this driver be creating a
> > symlink that has already been created by the driver core?  This whole
> > thing can just be removed with no problems.  Can you try just removing
> > the ehea_driver_sysfs_add and ehea_driver_sysfs_remove functions to
> > verify this as I don't have the hardware present to test it out.
> 
> The eHEA driver tries to orginize its sys-entries as close as possible to
> other ethernet drivers. Each eHEA NIC has multiple ports which is not that
> common in PCI. This means that each port is represented by a subdirectory
> which has not the "driver" sys-link, only the root directory has.
> Some tools expect to have this driver link in each port directory.
> That is the reason why this link is created manually.
> 
> Are there any other ways to create this link?


This is now broken in mainline...

drivers/net/ehea/ehea_main.c: In function 'ehea_driver_sysfs_add':
drivers/net/ehea/ehea_main.c:2812: error: 'struct device_driver' has
no member named 'kobj'
drivers/net/ehea/ehea_main.c:2815: error: 'struct device_driver' has
no member named 'kobj'
drivers/net/ehea/ehea_main.c:2818: error: 'struct device_driver' has
no member named 'kobj'
drivers/net/ehea/ehea_main.c: In function 'ehea_driver_sysfs_remove':
drivers/net/ehea/ehea_main.c:2830: error: 'struct device_driver' has
no member named 'kobj'

Re: [2.6.24-rc6-mm1]Build failure in drivers/net/ehea/ehea_main.c

[Greg KH]

On Fri, Jan 25, 2008 at 01:10:48PM -0600, Nathan Lynch wrote:
> Jan-Bernd Themann wrote:
> > Hi,
> > 
> > sorry for answering so late, I'm only tracking netdev and ppc mailing list.
> > 
> > On Thursday 10 January 2008 18:34, Greg KH wrote:
> > > > The structure device_driver(in device.h) has a member struct driver_private which
> > > > contains the member kobj (according to drivers/base/base.h).
> > > > But in device.h struct driver_private has been declared localy and 
> > > > neither defined nor included from base.h.
> > > > So my effort to use driver->driver_private->obj also does not work.
> > > > (I am surprised from where do you access the struct device_driver)
> > > 
> > > That is because a driver should not be accessing such a field.
> > > 
> > > And especially not in this manner, why would this driver be creating a
> > > symlink that has already been created by the driver core?  This whole
> > > thing can just be removed with no problems.  Can you try just removing
> > > the ehea_driver_sysfs_add and ehea_driver_sysfs_remove functions to
> > > verify this as I don't have the hardware present to test it out.
> > 
> > The eHEA driver tries to orginize its sys-entries as close as possible to
> > other ethernet drivers. Each eHEA NIC has multiple ports which is not that
> > common in PCI. This means that each port is represented by a subdirectory
> > which has not the "driver" sys-link, only the root directory has.
> > Some tools expect to have this driver link in each port directory.
> > That is the reason why this link is created manually.
> > 
> > Are there any other ways to create this link?
> 
> 
> This is now broken in mainline...
> 
> drivers/net/ehea/ehea_main.c: In function 'ehea_driver_sysfs_add':
> drivers/net/ehea/ehea_main.c:2812: error: 'struct device_driver' has
> no member named 'kobj'
> drivers/net/ehea/ehea_main.c:2815: error: 'struct device_driver' has
> no member named 'kobj'
> drivers/net/ehea/ehea_main.c:2818: error: 'struct device_driver' has
> no member named 'kobj'
> drivers/net/ehea/ehea_main.c: In function 'ehea_driver_sysfs_remove':
> drivers/net/ehea/ehea_main.c:2830: error: 'struct device_driver' has
> no member named 'kobj'

Crap, I missed that one.  I'll go fix it up right now...

sorry about this.

greg k-h

Re: [2.6.24-rc6-mm1]Build failure in drivers/net/ehea/ehea_main.c

[Greg KH]

On Fri, Jan 25, 2008 at 01:10:48PM -0600, Nathan Lynch wrote:
> Jan-Bernd Themann wrote:
> > Hi,
> > 
> > sorry for answering so late, I'm only tracking netdev and ppc mailing list.
> > 
> > On Thursday 10 January 2008 18:34, Greg KH wrote:
> > > > The structure device_driver(in device.h) has a member struct driver_private which
> > > > contains the member kobj (according to drivers/base/base.h).
> > > > But in device.h struct driver_private has been declared localy and 
> > > > neither defined nor included from base.h.
> > > > So my effort to use driver->driver_private->obj also does not work.
> > > > (I am surprised from where do you access the struct device_driver)
> > > 
> > > That is because a driver should not be accessing such a field.
> > > 
> > > And especially not in this manner, why would this driver be creating a
> > > symlink that has already been created by the driver core?  This whole
> > > thing can just be removed with no problems.  Can you try just removing
> > > the ehea_driver_sysfs_add and ehea_driver_sysfs_remove functions to
> > > verify this as I don't have the hardware present to test it out.
> > 
> > The eHEA driver tries to orginize its sys-entries as close as possible to
> > other ethernet drivers. Each eHEA NIC has multiple ports which is not that
> > common in PCI. This means that each port is represented by a subdirectory
> > which has not the "driver" sys-link, only the root directory has.
> > Some tools expect to have this driver link in each port directory.
> > That is the reason why this link is created manually.
> > 
> > Are there any other ways to create this link?
> 
> 
> This is now broken in mainline...
> 
> drivers/net/ehea/ehea_main.c: In function 'ehea_driver_sysfs_add':
> drivers/net/ehea/ehea_main.c:2812: error: 'struct device_driver' has
> no member named 'kobj'
> drivers/net/ehea/ehea_main.c:2815: error: 'struct device_driver' has
> no member named 'kobj'
> drivers/net/ehea/ehea_main.c:2818: error: 'struct device_driver' has
> no member named 'kobj'
> drivers/net/ehea/ehea_main.c: In function 'ehea_driver_sysfs_remove':
> drivers/net/ehea/ehea_main.c:2830: error: 'struct device_driver' has
> no member named 'kobj'

Does the patch below fix this?  That driver should not have been trying
to create symlinks that the driver core has already created for it.
Dumb, dumb, dumb...


thanks,

greg k-h

------------------

---
 drivers/net/ehea/ehea_main.c |   37 -------------------------------------
 1 file changed, 37 deletions(-)

--- a/drivers/net/ehea/ehea_main.c
+++ b/drivers/net/ehea/ehea_main.c
@@ -2804,34 +2804,6 @@ static void __devinit logical_port_relea
 	of_node_put(port->ofdev.node);
 }
 
-static int ehea_driver_sysfs_add(struct device *dev,
-				 struct device_driver *driver)
-{
-	int ret;
-
-	ret = sysfs_create_link(&driver->kobj, &dev->kobj,
-				kobject_name(&dev->kobj));
-	if (ret == 0) {
-		ret = sysfs_create_link(&dev->kobj, &driver->kobj,
-					"driver");
-		if (ret)
-			sysfs_remove_link(&driver->kobj,
-					  kobject_name(&dev->kobj));
-	}
-	return ret;
-}
-
-static void ehea_driver_sysfs_remove(struct device *dev,
-				     struct device_driver *driver)
-{
-	struct device_driver *drv = driver;
-
-	if (drv) {
-		sysfs_remove_link(&drv->kobj, kobject_name(&dev->kobj));
-		sysfs_remove_link(&dev->kobj, "driver");
-	}
-}
-
 static struct device *ehea_register_port(struct ehea_port *port,
 					 struct device_node *dn)
 {
@@ -2856,16 +2828,8 @@ static struct device *ehea_register_port
 		goto out_unreg_of_dev;
 	}
 
-	ret = ehea_driver_sysfs_add(&port->ofdev.dev, &ehea_driver.driver);
-	if (ret) {
-		ehea_error("failed to register sysfs driver link");
-		goto out_rem_dev_file;
-	}
-
 	return &port->ofdev.dev;
 
-out_rem_dev_file:
-	device_remove_file(&port->ofdev.dev, &dev_attr_log_port_id);
 out_unreg_of_dev:
 	of_device_unregister(&port->ofdev);
 out:
@@ -2874,7 +2838,6 @@ out:
 
 static void ehea_unregister_port(struct ehea_port *port)
 {
-	ehea_driver_sysfs_remove(&port->ofdev.dev, &ehea_driver.driver);
 	device_remove_file(&port->ofdev.dev, &dev_attr_log_port_id);
 	of_device_unregister(&port->ofdev);
 }

Re: [2.6.24-rc6-mm1]Build failure in drivers/net/ehea/ehea_main.c

[Greg KH]

On Fri, Jan 18, 2008 at 10:16:48AM +0100, Jan-Bernd Themann wrote:
> Hi,
> 
> sorry for answering so late, I'm only tracking netdev and ppc mailing list.
> 
> On Thursday 10 January 2008 18:34, Greg KH wrote:
> > > The structure device_driver(in device.h) has a member struct driver_private which
> > > contains the member kobj (according to drivers/base/base.h).
> > > But in device.h struct driver_private has been declared localy and 
> > > neither defined nor included from base.h.
> > > So my effort to use driver->driver_private->obj also does not work.
> > > (I am surprised from where do you access the struct device_driver)
> > 
> > That is because a driver should not be accessing such a field.
> > 
> > And especially not in this manner, why would this driver be creating a
> > symlink that has already been created by the driver core?  This whole
> > thing can just be removed with no problems.  Can you try just removing
> > the ehea_driver_sysfs_add and ehea_driver_sysfs_remove functions to
> > verify this as I don't have the hardware present to test it out.
> 
> The eHEA driver tries to orginize its sys-entries as close as possible to
> other ethernet drivers. Each eHEA NIC has multiple ports which is not that
> common in PCI. This means that each port is represented by a subdirectory
> which has not the "driver" sys-link, only the root directory has.
> Some tools expect to have this driver link in each port directory.
> That is the reason why this link is created manually.
> 
> Are there any other ways to create this link?

The driver core already creates this link for you.  Don't try to
duplicate what is already there for you.

It should just be a matter of deleting the code, and everything should
be fine.  See the patch that I just sent.

thanks,

greg k-h

Re: [2.6.24-rc6-mm1]Build failure in drivers/net/ehea/ehea_main.c

[Nathan Lynch]

Greg KH wrote:
> On Fri, Jan 25, 2008 at 01:10:48PM -0600, Nathan Lynch wrote:
> > Jan-Bernd Themann wrote:
> > > 
> > > On Thursday 10 January 2008 18:34, Greg KH wrote:
> > > > > The structure device_driver(in device.h) has a member struct driver_private which
> > > > > contains the member kobj (according to drivers/base/base.h).
> > > > > But in device.h struct driver_private has been declared localy and 
> > > > > neither defined nor included from base.h.
> > > > > So my effort to use driver->driver_private->obj also does not work.
> > > > > (I am surprised from where do you access the struct device_driver)
> > > > 
> > > > That is because a driver should not be accessing such a field.
> > > > 
> > > > And especially not in this manner, why would this driver be creating a
> > > > symlink that has already been created by the driver core?  This whole
> > > > thing can just be removed with no problems.  Can you try just removing
> > > > the ehea_driver_sysfs_add and ehea_driver_sysfs_remove functions to
> > > > verify this as I don't have the hardware present to test it out.
> > > 
> > > The eHEA driver tries to orginize its sys-entries as close as possible to
> > > other ethernet drivers. Each eHEA NIC has multiple ports which is not that
> > > common in PCI. This means that each port is represented by a subdirectory
> > > which has not the "driver" sys-link, only the root directory has.
> > > Some tools expect to have this driver link in each port directory.
> > > That is the reason why this link is created manually.
> > > 
> > > Are there any other ways to create this link?
> > 
> > 
> > This is now broken in mainline...
> > 
> > drivers/net/ehea/ehea_main.c: In function 'ehea_driver_sysfs_add':
> > drivers/net/ehea/ehea_main.c:2812: error: 'struct device_driver' has
> > no member named 'kobj'
> > drivers/net/ehea/ehea_main.c:2815: error: 'struct device_driver' has
> > no member named 'kobj'
> > drivers/net/ehea/ehea_main.c:2818: error: 'struct device_driver' has
> > no member named 'kobj'
> > drivers/net/ehea/ehea_main.c: In function 'ehea_driver_sysfs_remove':
> > drivers/net/ehea/ehea_main.c:2830: error: 'struct device_driver' has
> > no member named 'kobj'
> 
> Does the patch below fix this?  That driver should not have been trying
> to create symlinks that the driver core has already created for it.

Yes, it fixes the build error, by just removing the code that got
broken.  Jan-Bernd gave a rationale for creating the symlink that
didn't really seem to be answered.

Re: [2.6.24-rc6-mm1]Build failure in drivers/net/ehea/ehea_main.c

[Greg KH]

On Mon, Jan 28, 2008 at 01:22:04PM -0600, Nathan Lynch wrote:
> Greg KH wrote:
> > On Fri, Jan 25, 2008 at 01:10:48PM -0600, Nathan Lynch wrote:
> > > Jan-Bernd Themann wrote:
> > > > 
> > > > On Thursday 10 January 2008 18:34, Greg KH wrote:
> > > > > > The structure device_driver(in device.h) has a member struct driver_private which
> > > > > > contains the member kobj (according to drivers/base/base.h).
> > > > > > But in device.h struct driver_private has been declared localy and 
> > > > > > neither defined nor included from base.h.
> > > > > > So my effort to use driver->driver_private->obj also does not work.
> > > > > > (I am surprised from where do you access the struct device_driver)
> > > > > 
> > > > > That is because a driver should not be accessing such a field.
> > > > > 
> > > > > And especially not in this manner, why would this driver be creating a
> > > > > symlink that has already been created by the driver core?  This whole
> > > > > thing can just be removed with no problems.  Can you try just removing
> > > > > the ehea_driver_sysfs_add and ehea_driver_sysfs_remove functions to
> > > > > verify this as I don't have the hardware present to test it out.
> > > > 
> > > > The eHEA driver tries to orginize its sys-entries as close as possible to
> > > > other ethernet drivers. Each eHEA NIC has multiple ports which is not that
> > > > common in PCI. This means that each port is represented by a subdirectory
> > > > which has not the "driver" sys-link, only the root directory has.
> > > > Some tools expect to have this driver link in each port directory.
> > > > That is the reason why this link is created manually.
> > > > 
> > > > Are there any other ways to create this link?
> > > 
> > > 
> > > This is now broken in mainline...
> > > 
> > > drivers/net/ehea/ehea_main.c: In function 'ehea_driver_sysfs_add':
> > > drivers/net/ehea/ehea_main.c:2812: error: 'struct device_driver' has
> > > no member named 'kobj'
> > > drivers/net/ehea/ehea_main.c:2815: error: 'struct device_driver' has
> > > no member named 'kobj'
> > > drivers/net/ehea/ehea_main.c:2818: error: 'struct device_driver' has
> > > no member named 'kobj'
> > > drivers/net/ehea/ehea_main.c: In function 'ehea_driver_sysfs_remove':
> > > drivers/net/ehea/ehea_main.c:2830: error: 'struct device_driver' has
> > > no member named 'kobj'
> > 
> > Does the patch below fix this?  That driver should not have been trying
> > to create symlinks that the driver core has already created for it.
> 
> Yes, it fixes the build error, by just removing the code that got
> broken.  Jan-Bernd gave a rationale for creating the symlink that
> didn't really seem to be answered.

See my other post to him.  I do not see why this is not just duplicating
the same exact code in the driver core.  If you are trying to "fake out"
userspace by saying a device is bound to a driver, well, that's just
wrong.

thanks,

greg k-h

Re: [2.6.24-rc6-mm1]Build failure in drivers/net/ehea/ehea_main.c

[Jan-Bernd Themann]

On Monday 28 January 2008 20:22, you wrote:
> Greg KH wrote:
> > On Fri, Jan 25, 2008 at 01:10:48PM -0600, Nathan Lynch wrote:
> > > Jan-Bernd Themann wrote:
> > > 
> > > This is now broken in mainline...
> > > 
> > > drivers/net/ehea/ehea_main.c: In function 'ehea_driver_sysfs_add':
> > > drivers/net/ehea/ehea_main.c:2812: error: 'struct device_driver' has
> > > no member named 'kobj'
> > > drivers/net/ehea/ehea_main.c:2815: error: 'struct device_driver' has
> > > no member named 'kobj'
> > > drivers/net/ehea/ehea_main.c:2818: error: 'struct device_driver' has
> > > no member named 'kobj'
> > > drivers/net/ehea/ehea_main.c: In function 'ehea_driver_sysfs_remove':
> > > drivers/net/ehea/ehea_main.c:2830: error: 'struct device_driver' has
> > > no member named 'kobj'
> > 
> > Does the patch below fix this?  That driver should not have been trying
> > to create symlinks that the driver core has already created for it.
> 
> Yes, it fixes the build error, by just removing the code that got
> broken.  Jan-Bernd gave a rationale for creating the symlink that
> didn't really seem to be answered.
> 

> > > > The eHEA driver tries to orginize its sys-entries as close as possible to
> > > > other ethernet drivers. Each eHEA NIC has multiple ports which is not that
> > > > common in PCI. This means that each port is represented by a subdirectory
> > > > which has not the "driver" sys-link, only the root directory has.
> > > > Some tools expect to have this driver link in each port directory.
> > > > That is the reason why this link is created manually.

In addition to the explaination above:

Just to be sure that we talk about the same thing:
The eHEA driver controlls multiple eHEAs which have multiple
ethernet ports each. So the logical structure looks like this:
eHEA1 ---> port1 (ethX)
        -> port2 (ethX+1)
        -> port3 (ethX+2)
   ...
eHEA2 ---> port1 (eth?)
        -> port2
        -> port3 
   ...
This structure is represented in /sys/bus/ibmebus/devices in the same way
described above.

If you want to determine the driver belonging to an ethX, you would go
the following path:
/sys/class/net/ethX/device/driver


ls /sys/class/net/ethX/device:
drwxr-xr-x 2 root root    0 2008-01-29 10:00 .
drwxr-xr-x 4 root root    0 2008-01-29 10:00 ..
lrwxrwxrwx 1 root root    0 2008-01-29 10:00 bus -> ../../../../bus/ibmebus
-r--r--r-- 1 root root 4096 2008-01-29 10:00 devspec
lrwxrwxrwx 1 root root    0 2008-01-29 10:00 driver -> ../../../../bus/ibmebus/drivers/ehea
                                             ^^^^^
	In our case this one is created by the code that does not work any longer.

The sym-link is not gereated automatically as the device for portX is added
to the eHEA device (as subnode) where the eHEA device is not a bus.

If this sym-link is of interest (which I guess is the case as most devices
have it) we have to create it somehow.

Would the following proposal work?

There is an exported kernel function called 
"device_bind_driver" which creates the same links. However, it does some
additional stuff as well and there is currently no 
"device_unbind_driver" yet. Either "device_unbind_driver" needs to
be added or the following two already existing functions could be exported:

driver_sysfs_add
driver_sysfs_remove


Regards,
Jan-Bernd & Christoph Raisch

Re: [2.6.24-rc6-mm1]Build failure in drivers/net/ehea/ehea_main.c

[Greg KH]

On Tue, Jan 29, 2008 at 11:12:40AM +0100, Jan-Bernd Themann wrote:
> On Monday 28 January 2008 20:22, you wrote:
> > Greg KH wrote:
> > > On Fri, Jan 25, 2008 at 01:10:48PM -0600, Nathan Lynch wrote:
> > > > Jan-Bernd Themann wrote:
> > > > 
> > > > This is now broken in mainline...
> > > > 
> > > > drivers/net/ehea/ehea_main.c: In function 'ehea_driver_sysfs_add':
> > > > drivers/net/ehea/ehea_main.c:2812: error: 'struct device_driver' has
> > > > no member named 'kobj'
> > > > drivers/net/ehea/ehea_main.c:2815: error: 'struct device_driver' has
> > > > no member named 'kobj'
> > > > drivers/net/ehea/ehea_main.c:2818: error: 'struct device_driver' has
> > > > no member named 'kobj'
> > > > drivers/net/ehea/ehea_main.c: In function 'ehea_driver_sysfs_remove':
> > > > drivers/net/ehea/ehea_main.c:2830: error: 'struct device_driver' has
> > > > no member named 'kobj'
> > > 
> > > Does the patch below fix this?  That driver should not have been trying
> > > to create symlinks that the driver core has already created for it.
> > 
> > Yes, it fixes the build error, by just removing the code that got
> > broken.  Jan-Bernd gave a rationale for creating the symlink that
> > didn't really seem to be answered.
> > 
> 
> > > > > The eHEA driver tries to orginize its sys-entries as close as possible to
> > > > > other ethernet drivers. Each eHEA NIC has multiple ports which is not that
> > > > > common in PCI. This means that each port is represented by a subdirectory
> > > > > which has not the "driver" sys-link, only the root directory has.
> > > > > Some tools expect to have this driver link in each port directory.
> > > > > That is the reason why this link is created manually.
> 
> In addition to the explaination above:
> 
> Just to be sure that we talk about the same thing:
> The eHEA driver controlls multiple eHEAs which have multiple
> ethernet ports each. So the logical structure looks like this:
> eHEA1 ---> port1 (ethX)
>         -> port2 (ethX+1)
>         -> port3 (ethX+2)
>    ...
> eHEA2 ---> port1 (eth?)
>         -> port2
>         -> port3 
>    ...
> This structure is represented in /sys/bus/ibmebus/devices in the same way
> described above.
> 
> If you want to determine the driver belonging to an ethX, you would go
> the following path:
> /sys/class/net/ethX/device/driver
> 
> 
> ls /sys/class/net/ethX/device:
> drwxr-xr-x 2 root root    0 2008-01-29 10:00 .
> drwxr-xr-x 4 root root    0 2008-01-29 10:00 ..
> lrwxrwxrwx 1 root root    0 2008-01-29 10:00 bus -> ../../../../bus/ibmebus
> -r--r--r-- 1 root root 4096 2008-01-29 10:00 devspec
> lrwxrwxrwx 1 root root    0 2008-01-29 10:00 driver -> ../../../../bus/ibmebus/drivers/ehea
>                                              ^^^^^
> 	In our case this one is created by the code that does not work any longer.
> 
> The sym-link is not gereated automatically as the device for portX is added
> to the eHEA device (as subnode) where the eHEA device is not a bus.

Then please fix that, no other driver has this kind of problem, right?
Are you just passing the wrong "device" to the networking subsystem?

> If this sym-link is of interest (which I guess is the case as most devices
> have it) we have to create it somehow.

Why would you have to do this by hand?  What makes this driver so unique
in the kernel that it would have to do this?  We have lots of other
multi-port ethernet drivers today without this issue, right?

confused,

greg k-h

Re: [2.6.24-rc6-mm1]Build failure in drivers/net/ehea/ehea_main.c

[Christoph Raisch]

Greg KH <greg@kroah.com> wrote on 29.01.2008 14:23:09:

> On Tue, Jan 29, 2008 at 11:12:40AM +0100, Jan-Bernd Themann wrote:
...
> > The sym-link is not gereated automatically as the device for portX is
added
> > to the eHEA device (as subnode) where the eHEA device is not a bus.
>
> Then please fix that, no other driver has this kind of problem, right?
> Are you just passing the wrong "device" to the networking subsystem?
>
> > If this sym-link is of interest (which I guess is the case as most
devices
> > have it) we have to create it somehow.
>
> Why would you have to do this by hand?  What makes this driver so unique
> in the kernel that it would have to do this?  We have lots of other
> multi-port ethernet drivers today without this issue, right?
>
> confused,
>
> greg k-h

well, the major difference is hea is not PCI.
All PCI cards we checked have a 1:1 relationship between PCI function (PCI
config space)
and a single ethernet port.
Even if the same Ethernet chip has two ports, it shows up as two separate
adapters from the PCI perspective (two PCI entries in /sys/bus/pci/devices

host:/ # ls -l /sys/bus/pci/devices/0000\:c8\:01.0/
total 0
lrwxrwxrwx 1 root root      0 2008-01-28 14:59 bus -> ../../../../bus/pci
-r--r--r-- 1 root root   4096 2008-01-28 14:59 class
-rw-r--r-- 1 root root    256 2008-01-28 14:59 config
-r--r--r-- 1 root root   4096 2008-01-28 14:59 device
-r--r--r-- 1 root root   4096 2008-01-29 14:26 devspec
lrwxrwxrwx 1 root root      0 2008-01-28 14:59 driver ->
../../../../bus/pci/drivers/e1000
-r--r--r-- 1 root root   4096 2008-01-28 14:59 irq
-r--r--r-- 1 root root   4096 2008-01-29 14:26 local_cpus
-r--r--r-- 1 root root   4096 2008-01-28 14:59 modalias
lrwxrwxrwx 1 root root      0 2008-01-29 14:26 net:eth1 ->
../../../../class/net/eth1
-r--r--r-- 1 root root   4096 2008-01-28 14:59 resource
....
host:/ # ls -l /sys/bus/pci/devices/0000\:c8\:01.1/
total 0
lrwxrwxrwx 1 root root      0 2008-01-28 14:59 bus -> ../../../../bus/pci
-r--r--r-- 1 root root   4096 2008-01-28 14:59 class
-rw-r--r-- 1 root root    256 2008-01-28 14:59 config
-r--r--r-- 1 root root   4096 2008-01-28 14:59 device
-r--r--r-- 1 root root   4096 2008-01-29 14:29 devspec
lrwxrwxrwx 1 root root      0 2008-01-28 14:59 driver ->
../../../../bus/pci/drivers/e1000
-r--r--r-- 1 root root   4096 2008-01-28 14:59 irq
-r--r--r-- 1 root root   4096 2008-01-29 14:29 local_cpus
-r--r--r-- 1 root root   4096 2008-01-28 14:59 modalias
lrwxrwxrwx 1 root root      0 2008-01-29 14:29 net:eth2 ->
../../../../class/net/eth2
...

These pci functions corresponds to a
/sys/bus/ibmebus/devices/789D.001.XXXXXX-P1/port0
and
/sys/bus/ibmebus/devices/789D.001.XXXXXX-P1/port1

The busdriver currently does not find out, how many ports are in a
/sys/bus/ibmebus/devices/789D.001.XXXXXX-P1.
This is up to the hardware specific driver responsible for ehea or ehca.
Think of a PCI card where the PCI busdriver
can not determine how many ports are implemented on the card.

How should this be mapped to /sys ?

Should we try to "flatten" the ports to something like
/sys/bus/ibmebus/devices/789D.001.XXXXXX-P1
/sys/bus/ibmebus/devices/789D.001.XXXXXX-P1_port0
/sys/bus/ibmebus/devices/789D.001.XXXXXX-P1_port1
...which means physical hierarchy information would look a bit strange,
but could be the simpler one.

The way which corresponds to the hardware would be to
improve the kernel in such a way that hierarchical ports also wortk for
netdev_register.
Then we could keep this structure
/sys/bus/ibmebus/devices/789D.001.XXXXXX-P1/port0
/sys/bus/ibmebus/devices/789D.001.XXXXXX-P1/port1


So, which way should we try?

Gruss / Regards
Christoph Raisch

Re: [2.6.24-rc6-mm1]Build failure in drivers/net/ehea/ehea_main.c

[Jan-Bernd Themann]

On Tuesday 29 January 2008 15:20, Christoph Raisch wrote:

> These pci functions corresponds to a
> /sys/bus/ibmebus/devices/789D.001.XXXXXX-P1/port0
> and
> /sys/bus/ibmebus/devices/789D.001.XXXXXX-P1/port1
> 
> The busdriver currently does not find out, how many ports are in a
> /sys/bus/ibmebus/devices/789D.001.XXXXXX-P1.
> This is up to the hardware specific driver responsible for ehea or ehca.
> Think of a PCI card where the PCI busdriver
> can not determine how many ports are implemented on the card.
> 
> How should this be mapped to /sys ?
> 
> Should we try to "flatten" the ports to something like
> /sys/bus/ibmebus/devices/789D.001.XXXXXX-P1
> /sys/bus/ibmebus/devices/789D.001.XXXXXX-P1_port0
> /sys/bus/ibmebus/devices/789D.001.XXXXXX-P1_port1
> ...which means physical hierarchy information would look a bit strange,
> but could be the simpler one.
> 
> The way which corresponds to the hardware would be to
> improve the kernel in such a way that hierarchical ports also wortk for
> netdev_register.
> Then we could keep this structure
> /sys/bus/ibmebus/devices/789D.001.XXXXXX-P1/port0
> /sys/bus/ibmebus/devices/789D.001.XXXXXX-P1/port1
> 
> 
> So, which way should we try?
> 
> Gruss / Regards
> Christoph Raisch
> 
> 
> 

Hi,

so far we fould 2 options. The ibmebus manages (add / remove)
ONLY eHEA adapters (not ehea ethernet ports).
All ethernet ports have to be added / removed by the eHEA device driver when
an eHEA adapter has been added by the ibmebus as a new device.
This means the sysfs-links "driver" for our eHEA ethernet devices (ports) 
are not automatically generated.

Option 1)
- We can create then ourselves. Therefore we would 
  need two functions exported and included in the proper header file:
    - static int driver_sysfs_add(struct device *dev)
    - static void driver_sysfs_remove(struct device *dev)

Option 2)
- we just leave it out which would be inconsistent with
  other pci based ethernet adapters.

So far we prefer option 1. 
Do you see any other ways we should investigate? 

To resolve the build issue I've posted a first patch to remove the
old sysfs links. 


Regards, 
Jan-Bernd & Christoph

Re: [2.6.24-rc6-mm1]Build failure in drivers/net/ehea/ehea_main.c

[Greg KH]

On Tue, Jan 29, 2008 at 03:20:20PM +0100, Christoph Raisch wrote:
> Greg KH <greg@kroah.com> wrote on 29.01.2008 14:23:09:
> 
> > On Tue, Jan 29, 2008 at 11:12:40AM +0100, Jan-Bernd Themann wrote:
> ...
> > > The sym-link is not gereated automatically as the device for portX is
> added
> > > to the eHEA device (as subnode) where the eHEA device is not a bus.
> >
> > Then please fix that, no other driver has this kind of problem, right?
> > Are you just passing the wrong "device" to the networking subsystem?
> >
> > > If this sym-link is of interest (which I guess is the case as most
> devices
> > > have it) we have to create it somehow.
> >
> > Why would you have to do this by hand?  What makes this driver so unique
> > in the kernel that it would have to do this?  We have lots of other
> > multi-port ethernet drivers today without this issue, right?
> >
> > confused,
> >
> > greg k-h
> 
> well, the major difference is hea is not PCI.

What is it?  It has to live on some kind of bus, right?

> All PCI cards we checked have a 1:1 relationship between PCI function (PCI
> config space) and a single ethernet port.
> Even if the same Ethernet chip has two ports, it shows up as two separate
> adapters from the PCI perspective (two PCI entries in /sys/bus/pci/devices
> 
> host:/ # ls -l /sys/bus/pci/devices/0000\:c8\:01.0/
> total 0
> lrwxrwxrwx 1 root root      0 2008-01-28 14:59 bus -> ../../../../bus/pci
> -r--r--r-- 1 root root   4096 2008-01-28 14:59 class
> -rw-r--r-- 1 root root    256 2008-01-28 14:59 config
> -r--r--r-- 1 root root   4096 2008-01-28 14:59 device
> -r--r--r-- 1 root root   4096 2008-01-29 14:26 devspec
> lrwxrwxrwx 1 root root      0 2008-01-28 14:59 driver ->
> ../../../../bus/pci/drivers/e1000
> -r--r--r-- 1 root root   4096 2008-01-28 14:59 irq
> -r--r--r-- 1 root root   4096 2008-01-29 14:26 local_cpus
> -r--r--r-- 1 root root   4096 2008-01-28 14:59 modalias
> lrwxrwxrwx 1 root root      0 2008-01-29 14:26 net:eth1 ->
> ../../../../class/net/eth1
> -r--r--r-- 1 root root   4096 2008-01-28 14:59 resource
> ....
> host:/ # ls -l /sys/bus/pci/devices/0000\:c8\:01.1/
> total 0
> lrwxrwxrwx 1 root root      0 2008-01-28 14:59 bus -> ../../../../bus/pci
> -r--r--r-- 1 root root   4096 2008-01-28 14:59 class
> -rw-r--r-- 1 root root    256 2008-01-28 14:59 config
> -r--r--r-- 1 root root   4096 2008-01-28 14:59 device
> -r--r--r-- 1 root root   4096 2008-01-29 14:29 devspec
> lrwxrwxrwx 1 root root      0 2008-01-28 14:59 driver ->
> ../../../../bus/pci/drivers/e1000
> -r--r--r-- 1 root root   4096 2008-01-28 14:59 irq
> -r--r--r-- 1 root root   4096 2008-01-29 14:29 local_cpus
> -r--r--r-- 1 root root   4096 2008-01-28 14:59 modalias
> lrwxrwxrwx 1 root root      0 2008-01-29 14:29 net:eth2 ->
> ../../../../class/net/eth2
> ...
> 
> These pci functions corresponds to a
> /sys/bus/ibmebus/devices/789D.001.XXXXXX-P1/port0
> and
> /sys/bus/ibmebus/devices/789D.001.XXXXXX-P1/port1
> 
> The busdriver currently does not find out, how many ports are in a
> /sys/bus/ibmebus/devices/789D.001.XXXXXX-P1.
> This is up to the hardware specific driver responsible for ehea or ehca.
> Think of a PCI card where the PCI busdriver
> can not determine how many ports are implemented on the card.
> 
> How should this be mapped to /sys ?
> 
> Should we try to "flatten" the ports to something like
> /sys/bus/ibmebus/devices/789D.001.XXXXXX-P1
> /sys/bus/ibmebus/devices/789D.001.XXXXXX-P1_port0
> /sys/bus/ibmebus/devices/789D.001.XXXXXX-P1_port1
> ...which means physical hierarchy information would look a bit strange,
> but could be the simpler one.

No.  Why have a separate "port" device for every ethernet port?  What
keeps you from just creating the different network devices for your
device, and pointing the parent to the same 789D.001.XXXXXX-P1 device?

Lots of PCI devices hang "class devices" off of them all the time, why
would this be any different from that?

I think you all are trying to make this more complex than it really is
:)

thanks,

greg k-h

Re: [2.6.24-rc6-mm1]Build failure in drivers/net/ehea/ehea_main.c

[Christoph Raisch]

Greg KH <greg@kroah.com> wrote on 07.02.2008 23:17:12:

> On Tue, Jan 29, 2008 at 03:20:20PM +0100, Christoph Raisch wrote:
> What is it?  It has to live on some kind of bus, right?

It is a piece of hardware with a firmware/hypervisor abstraction layer on
top.
The hypervisor provides virtualization interfaces to
add and remove ethernet adapters and ports.
Each port is represented in the open firmware device tree (OFDT) as a
subnode of the
ehea adapter node.

System P has a userspace DLPAR application communicating with firmware, the
kernel, and
the hardware management console to change all that on the flight.
This tool needs a capability to identify which open firmware device tree
entry
belongs to which ethernet device.
Each node created by the ibmebus driver has a devspec entry associated to
the
device node in OFDT, used by the DLPAR application.
Each port created by the ibmebus driver has a devspec entry associated to
the
port node in OFDT, used by the DLPAR application.

> >
> > host:/ # ls -l /sys/bus/pci/devices/0000\:c8\:01.0/
> > total 0
...
> > -r--r--r-- 1 root root   4096 2008-01-29 14:26 devspec
...

> >
> > These pci functions corresponds to a
> > /sys/bus/ibmebus/devices/789D.001.XXXXXX-P1/port0
> > and
> > /sys/bus/ibmebus/devices/789D.001.XXXXXX-P1/port1
> >
> > The busdriver currently does not find out, how many ports are in a
> > /sys/bus/ibmebus/devices/789D.001.XXXXXX-P1.
> > This is up to the hardware specific driver responsible for ehea or
ehca.
> > Think of a PCI card where the PCI busdriver
> > can not determine how many ports are implemented on the card.
> >
> > How should this be mapped to /sys ?
> >
> > Should we try to "flatten" the ports to something like
> > /sys/bus/ibmebus/devices/789D.001.XXXXXX-P1
> > /sys/bus/ibmebus/devices/789D.001.XXXXXX-P1_port0
> > /sys/bus/ibmebus/devices/789D.001.XXXXXX-P1_port1
> > ...which means physical hierarchy information would look a bit strange,
> > but could be the simpler one.
>
> No.  Why have a separate "port" device for every ethernet port?  What
> keeps you from just creating the different network devices for your
> device, and pointing the parent to the same 789D.001.XXXXXX-P1 device?
>
>
> I think you all are trying to make this more complex than it really is
> :)

If you register multiple netdevs in a ehea adapter node,
how is it possible to find out which ethXXX belongs to which devspec port
entry in the OFDT?

Is there a simpler way than the one we chose to get this in addition?

>
> thanks,
>
> greg k-h
Gruss / Regards
Christoph Raisch