Re: Enable module aliases for vio on sparc/sparc64

[David Miller]

From: John Paul Adrian Glaubitz 
Date: Thu, 14 Apr 2016 01:22:37 +0200

> From 8d95bce6a35dc037d1e419896af19e12b3cda910 Mon Sep 17 00:00:00 2001
> From: John Paul Adrian Glaubitz 
> Date: Thu, 14 Apr 2016 01:16:14 +0200
> Subject: [PATCH 1/2] sparc/kernel/vio.c: implement modalias_show()
> 
> Signed-off-by: John Paul Adrian Glaubitz 
> ---
>  arch/sparc/kernel/vio.c | 7 +++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/arch/sparc/kernel/vio.c b/arch/sparc/kernel/vio.c
> index cb5789c..5a9bb9d 100644
> --- a/arch/sparc/kernel/vio.c
> +++ b/arch/sparc/kernel/vio.c
> @@ -105,6 +105,13 @@ static ssize_t type_show(struct device *dev,
>   return sprintf(buf, "%s\n", vdev->type);
>  }
>  
> +static ssize_t modalias_show(struct device *dev, struct device_attribute 
> *attr,
> + char *buf)
> +{
> + const struct vio_dev *vdev = to_vio_dev(dev);
> + return sprintf(buf, "vio:T%sS%s\n", vdev->type, vdev->compat);
> +}
> +

You're not hooking this function up at all into the operations.  Looks like
you are mixing this patch up with other local changes.

And again, please, empty line after local variable declarations.

Thanks.

Re: Enable module aliases for vio on sparc/sparc64

[David Miller]

From: John Paul Adrian Glaubitz 
Date: Thu, 14 Apr 2016 01:22:37 +0200

> From 3515ad550f7b1db467664acc9bf0ed3d2aec9fdd Mon Sep 17 00:00:00 2001
> From: John Paul Adrian Glaubitz 
> Date: Thu, 14 Apr 2016 01:19:40 +0200
> Subject: [PATCH 2/2] sparc/kernel/vio.c: implement vio_hotplug and add it to
>  vio_bus_type
> 
> Signed-off-by: John Paul Adrian Glaubitz 
> ---
>  arch/sparc/kernel/vio.c | 8 
>  1 file changed, 8 insertions(+)
> 
> diff --git a/arch/sparc/kernel/vio.c b/arch/sparc/kernel/vio.c
> index 5a9bb9d..59f4b7c 100644
> --- a/arch/sparc/kernel/vio.c
> +++ b/arch/sparc/kernel/vio.c
> @@ -45,6 +45,13 @@ static const struct vio_device_id *vio_match_device(
>   return NULL;
>  }
>  
> +static int vio_hotplug(struct device *dev, struct kobj_uevent_env *env)
> +{
> + const struct vio_dev *vio_dev = to_vio_dev(dev);
> + add_uevent_var(env, "MODALIAS=vio:T%sS%s", vio_dev->type, 
> vio_dev->compat);
> +  return 0;

Extra space before "return 0" after the TAB.

Need an empty line after the local variable declarations.

Re: Enable module aliases for vio on sparc/sparc64

[David Miller]

From: John Paul Adrian Glaubitz 
Date: Thu, 14 Apr 2016 01:04:17 +0200

> On 04/14/2016 01:01 AM, David Miller wrote:
>> Ok those are the block devices, where are the networking interfaces?
> 
> Whoops, sorry:
> 
> root@deb4g:/sys/devices/channel-devices# cat vdc*/mod*
> vio:Tvdc-portSunknown
> vio:Tvdc-portSunknown
> vio:Tvdc-portSunknown
> vio:Tvdc-portSunknown
> vio:Tvdc-portSunknown
> root@deb4g:/sys/devices/channel-devices#
> 
> Testing a much simpler version now. Second.

Those all say "vdc-port", which again is the block device.

Where are the networking devices?

Re: Enable module aliases for vio on sparc/sparc64

[Patrick Baggett]

You guys are my heroes. Keep up the great work!

On Wednesday, April 13, 2016, John Paul Adrian Glaubitz <
glaub...@physik.fu-berlin.de> wrote:

> On 04/14/2016 01:01 AM, David Miller wrote:
> > Ok those are the block devices, where are the networking interfaces?
>
> Whoops, sorry:
>
> root@deb4g:/sys/devices/channel-devices# cat vdc*/mod*
> vio:Tvdc-portSunknown
> vio:Tvdc-portSunknown
> vio:Tvdc-portSunknown
> vio:Tvdc-portSunknown
> vio:Tvdc-portSunknown
> root@deb4g:/sys/devices/channel-devices#
>
> Testing a much simpler version now. Second.
>
> --
>  .''`.  John Paul Adrian Glaubitz
> : :' :  Debian Developer - glaub...@debian.org 
> `. `'   Freie Universitaet Berlin - glaub...@physik.fu-berlin.de
> 
>   `-GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913
>
>

Re: Bug#815977: closed by Ben Hutchings <b...@decadent.org.uk> (Re: Bug#815977: kernel-image-4.4.0-1-sparc64-di: Please add sunvnet and sunvdc for d-i)

[John Paul Adrian Glaubitz]

Control: tags -1 patch

Hi Ben!

On 04/13/2016 05:40 PM, Ben Hutchings wrote:
> Try deleting the 'if (!cp)' block.

That wasn't enough. I invested some more time and now have a patch
that does the trick. Module aliases are created correctly and
module autoloading is working as expected. This has been tested
with Debian unstable and kernel 4.5.1 on a SPARC-T5 in a Linux
LDOM.

Attaching my patch. I also sent it as two single patches upstream.

Adrian

-- 
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer - glaub...@debian.org
`. `'   Freie Universitaet Berlin - glaub...@physik.fu-berlin.de
  `-GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913
diff --git a/arch/sparc/kernel/vio.c b/arch/sparc/kernel/vio.c
index cb5789c..59f4b7c 100644
--- a/arch/sparc/kernel/vio.c
+++ b/arch/sparc/kernel/vio.c
@@ -45,6 +45,13 @@ static const struct vio_device_id *vio_match_device(
 	return NULL;
 }
 
+static int vio_hotplug(struct device *dev, struct kobj_uevent_env *env)
+{
+	const struct vio_dev *vio_dev = to_vio_dev(dev);
+	add_uevent_var(env, "MODALIAS=vio:T%sS%s", vio_dev->type, vio_dev->compat);
+	 return 0;
+}
+
 static int vio_bus_match(struct device *dev, struct device_driver *drv)
 {
 	struct vio_dev *vio_dev = to_vio_dev(dev);
@@ -105,6 +112,13 @@ static ssize_t type_show(struct device *dev,
 	return sprintf(buf, "%s\n", vdev->type);
 }
 
+static ssize_t modalias_show(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+	const struct vio_dev *vdev = to_vio_dev(dev);
+	return sprintf(buf, "vio:T%sS%s\n", vdev->type, vdev->compat);
+}
+
 static struct device_attribute vio_dev_attrs[] = {
 	__ATTR_RO(devspec),
 	__ATTR_RO(type),
@@ -114,6 +128,7 @@ static struct device_attribute vio_dev_attrs[] = {
 static struct bus_type vio_bus_type = {
 	.name		= "vio",
 	.dev_attrs	= vio_dev_attrs,
+	.uevent = vio_hotplug,
 	.match		= vio_bus_match,
 	.probe		= vio_device_probe,
 	.remove		= vio_device_remove,


signature.asc
Description: OpenPGP digital signature

Re: Enable module aliases for vio on sparc/sparc64

[John Paul Adrian Glaubitz]

On 04/14/2016 01:04 AM, John Paul Adrian Glaubitz wrote:
> Testing a much simpler version now. Second.

Attaching two patches which add modalias_show and vio_hotplug. With
the patches applied, module autoloading works as expected.

Going to bed now, it's past midnight here.

Adrian

-- 
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer - glaub...@debian.org
`. `'   Freie Universitaet Berlin - glaub...@physik.fu-berlin.de
  `-GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913
>From 3515ad550f7b1db467664acc9bf0ed3d2aec9fdd Mon Sep 17 00:00:00 2001
From: John Paul Adrian Glaubitz 
Date: Thu, 14 Apr 2016 01:19:40 +0200
Subject: [PATCH 2/2] sparc/kernel/vio.c: implement vio_hotplug and add it to
 vio_bus_type

Signed-off-by: John Paul Adrian Glaubitz 
---
 arch/sparc/kernel/vio.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/arch/sparc/kernel/vio.c b/arch/sparc/kernel/vio.c
index 5a9bb9d..59f4b7c 100644
--- a/arch/sparc/kernel/vio.c
+++ b/arch/sparc/kernel/vio.c
@@ -45,6 +45,13 @@ static const struct vio_device_id *vio_match_device(
 	return NULL;
 }
 
+static int vio_hotplug(struct device *dev, struct kobj_uevent_env *env)
+{
+	const struct vio_dev *vio_dev = to_vio_dev(dev);
+	add_uevent_var(env, "MODALIAS=vio:T%sS%s", vio_dev->type, vio_dev->compat);
+	 return 0;
+}
+
 static int vio_bus_match(struct device *dev, struct device_driver *drv)
 {
 	struct vio_dev *vio_dev = to_vio_dev(dev);
@@ -121,6 +128,7 @@ static struct device_attribute vio_dev_attrs[] = {
 static struct bus_type vio_bus_type = {
 	.name		= "vio",
 	.dev_attrs	= vio_dev_attrs,
+	.uevent = vio_hotplug,
 	.match		= vio_bus_match,
 	.probe		= vio_device_probe,
 	.remove		= vio_device_remove,
-- 
2.8.0.rc3

>From 8d95bce6a35dc037d1e419896af19e12b3cda910 Mon Sep 17 00:00:00 2001
From: John Paul Adrian Glaubitz 
Date: Thu, 14 Apr 2016 01:16:14 +0200
Subject: [PATCH 1/2] sparc/kernel/vio.c: implement modalias_show()

Signed-off-by: John Paul Adrian Glaubitz 
---
 arch/sparc/kernel/vio.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/arch/sparc/kernel/vio.c b/arch/sparc/kernel/vio.c
index cb5789c..5a9bb9d 100644
--- a/arch/sparc/kernel/vio.c
+++ b/arch/sparc/kernel/vio.c
@@ -105,6 +105,13 @@ static ssize_t type_show(struct device *dev,
 	return sprintf(buf, "%s\n", vdev->type);
 }
 
+static ssize_t modalias_show(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+	const struct vio_dev *vdev = to_vio_dev(dev);
+	return sprintf(buf, "vio:T%sS%s\n", vdev->type, vdev->compat);
+}
+
 static struct device_attribute vio_dev_attrs[] = {
 	__ATTR_RO(devspec),
 	__ATTR_RO(type),
-- 
2.8.0.rc3

Re: Enable module aliases for vio on sparc/sparc64

[David Miller]

From: John Paul Adrian Glaubitz 
Date: Thu, 14 Apr 2016 00:44:19 +0200

> Yeah, I modified Ben's patch and it works:
> 
> root@deb4g:/sys/devices/channel-devices# cat vdc*/mod*
> vio:Tvdc-portSunknown
> vio:Tvdc-portSunknown
> vio:Tvdc-portSunknown
> vio:Tvdc-portSunknown
> vio:Tvdc-portSunknown

Ok those are the block devices, where are the networking interfaces?

Re: Enable module aliases for vio on sparc/sparc64

[John Paul Adrian Glaubitz]

On 04/14/2016 01:01 AM, David Miller wrote:
> Ok those are the block devices, where are the networking interfaces?

Whoops, sorry:

root@deb4g:/sys/devices/channel-devices# cat vdc*/mod*
vio:Tvdc-portSunknown
vio:Tvdc-portSunknown
vio:Tvdc-portSunknown
vio:Tvdc-portSunknown
vio:Tvdc-portSunknown
root@deb4g:/sys/devices/channel-devices#

Testing a much simpler version now. Second.

-- 
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer - glaub...@debian.org
`. `'   Freie Universitaet Berlin - glaub...@physik.fu-berlin.de
  `-GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913

Re: Enable module aliases for vio on sparc/sparc64

[David Miller]

From: John Paul Adrian Glaubitz 
Date: Thu, 14 Apr 2016 00:35:58 +0200

> On 04/14/2016 12:23 AM, David Miller wrote:
>> The missing bit is probably having the individual sparc64 VIO drivers
>> define proper module aliases.
> 
> I'm actually currently working on a patch. The aliases are already
> working, now it's just the actual hotplug handler which is missing.

Are you sure?  See my other email, VIO devices don't come from the
Openfirmware device tree at all.

Re: Enable module aliases for vio on sparc/sparc64

[David Miller]

From: David Miller 
Date: Wed, 13 Apr 2016 18:23:49 -0400 (EDT)

> From: John Paul Adrian Glaubitz 
> Date: Wed, 13 Apr 2016 17:33:25 +0200
> 
>> On 04/11/2016 09:04 PM, David Miller wrote:
>>> Either way, I'll look at this patch, thanks.
>> 
>> Just as a heads-up:
>> 
>> The suggested patch does not work unfortunately. Modules are not
>> loaded automatically and it seems modaliases are not generated
>> properly.
>> 
>> It seems that the vio implementation for sparc which you adapted
>> from powerpc uses a different name to retrieve the device nodes.
> 
> The missing bit is probably having the individual sparc64 VIO drivers
> define proper module aliases.

Actually there is another, more fundamental, problem.

The VIO devices on sparc64 are not instantiated from openfirmware
device nodes.

They are instead instantiated from nodes in the machine description.

Ben's patch is therefore buggy, he's looking at the of_node but that
will never be filled in for a VIO device on sparc64.

That's why I kinda cringed when it was suggested that the PowerPC and
Sparc64 VIO stuff should be merged together.  They simply can't, as
they are instantiating devices from two different sources.

Does Debian's installer have a machine description counterpart to the
opernfirmware device probing and discovery?  I bet it doesn't, and
that seems to be the first piece of infrastructure that needs to be
added before any of this can work.

Re: Enable module aliases for vio on sparc/sparc64

[John Paul Adrian Glaubitz]

On 04/14/2016 12:37 AM, David Miller wrote:
> Are you sure?  See my other email, VIO devices don't come from the
> Openfirmware device tree at all.

Yeah, I modified Ben's patch and it works:

root@deb4g:/sys/devices/channel-devices# cat vdc*/mod*
vio:Tvdc-portSunknown
vio:Tvdc-portSunknown
vio:Tvdc-portSunknown
vio:Tvdc-portSunknown
vio:Tvdc-portSunknown
root@deb4g:/sys/devices/channel-devices#

But I need to do some more clean up. Then I'll send you a patch.

Adrian

-- 
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer - glaub...@debian.org
`. `'   Freie Universitaet Berlin - glaub...@physik.fu-berlin.de
  `-GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913

Re: Enable module aliases for vio on sparc/sparc64

[David Miller]

From: John Paul Adrian Glaubitz 
Date: Wed, 13 Apr 2016 17:33:25 +0200

> On 04/11/2016 09:04 PM, David Miller wrote:
>> Either way, I'll look at this patch, thanks.
> 
> Just as a heads-up:
> 
> The suggested patch does not work unfortunately. Modules are not
> loaded automatically and it seems modaliases are not generated
> properly.
> 
> It seems that the vio implementation for sparc which you adapted
> from powerpc uses a different name to retrieve the device nodes.

The missing bit is probably having the individual sparc64 VIO drivers
define proper module aliases.

Re: Enable module aliases for vio on sparc/sparc64

[John Paul Adrian Glaubitz]

On 04/14/2016 12:23 AM, David Miller wrote:
> The missing bit is probably having the individual sparc64 VIO drivers
> define proper module aliases.

I'm actually currently working on a patch. The aliases are already
working, now it's just the actual hotplug handler which is missing.

Adrian

-- 
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer - glaub...@debian.org
`. `'   Freie Universitaet Berlin - glaub...@physik.fu-berlin.de
  `-GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913

Re: Bug#815977: closed by Ben Hutchings <b...@decadent.org.uk> (Re: Bug#815977: kernel-image-4.4.0-1-sparc64-di: Please add sunvnet and sunvdc for d-i)

[Ben Hutchings]

Control: reopen -1
Control: retitle -1 vio devices on SPARC do not have modaliases

On Wed, 2016-04-13 at 17:27 +0200, John Paul Adrian Glaubitz wrote:
> On 04/13/2016 02:03 PM, John Paul Adrian Glaubitz wrote:
> > 
> > On 04/11/2016 01:32 AM, Ben Hutchings wrote:
> > > 
> > > The attached patch might fix that, though the correct fix would
> > > presumably be to merge the two implementations.
> > Thanks a lot for the explanation and the patch. I will test the
> > patch and then we can maybe decide if we include it as a work
> > around.
> Ok, tested it and it didn't work unfortunately. The modules are
> not loaded automatically and it seems the reason is that the
> modaliases are not generated:
> 
> root@deb4g:~# cat /sys/devices/channel-devices/vdc-port-0-0/modalias
> 
> root@deb4g:~#
> 
> Will do further debugging.

Try deleting the 'if (!cp)' block.

Ben.

-- 
Ben Hutchings
It is easier to change the specification to fit the program than vice versa.


signature.asc
Description: This is a digitally signed message part

Re: Enable module aliases for vio on sparc/sparc64

[John Paul Adrian Glaubitz]

On 04/11/2016 09:04 PM, David Miller wrote:
> Either way, I'll look at this patch, thanks.

Just as a heads-up:

The suggested patch does not work unfortunately. Modules are not
loaded automatically and it seems modaliases are not generated
properly.

It seems that the vio implementation for sparc which you adapted
from powerpc uses a different name to retrieve the device nodes.

Adrian

-- 
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer - glaub...@debian.org
`. `'   Freie Universitaet Berlin - glaub...@physik.fu-berlin.de
  `-GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913

Re: Bug#815977 closed by Ben Hutchings <b...@decadent.org.uk> (Re: Bug#815977: kernel-image-4.4.0-1-sparc64-di: Please add sunvnet and sunvdc for d-i)

[John Paul Adrian Glaubitz]

On 04/13/2016 02:03 PM, John Paul Adrian Glaubitz wrote:
> On 04/11/2016 01:32 AM, Ben Hutchings wrote:
>> The attached patch might fix that, though the correct fix would
>> presumably be to merge the two implementations.
> 
> Thanks a lot for the explanation and the patch. I will test the
> patch and then we can maybe decide if we include it as a work
> around.

Ok, tested it and it didn't work unfortunately. The modules are
not loaded automatically and it seems the reason is that the
modaliases are not generated:

root@deb4g:~# cat /sys/devices/channel-devices/vdc-port-0-0/modalias

root@deb4g:~#

Will do further debugging.

Adrian

-- 
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer - glaub...@debian.org
`. `'   Freie Universitaet Berlin - glaub...@physik.fu-berlin.de
  `-GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913



signature.asc
Description: OpenPGP digital signature

Re: Bug#815977 closed by Ben Hutchings <b...@decadent.org.uk> (Re: Bug#815977: kernel-image-4.4.0-1-sparc64-di: Please add sunvnet and sunvdc for d-i)

[John Paul Adrian Glaubitz]

On 04/11/2016 01:32 AM, Ben Hutchings wrote:
> The attached patch might fix that, though the correct fix would
> presumably be to merge the two implementations.

Thanks a lot for the explanation and the patch. I will test the
patch and then we can maybe decide if we include it as a work
around.

I have also notified upstream and Dave Miller is now looking
at a proper fix.

Adrian

-- 
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer - glaub...@debian.org
`. `'   Freie Universitaet Berlin - glaub...@physik.fu-berlin.de
  `-GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913



signature.asc
Description: OpenPGP digital signature