diff mbox

Enable module aliases for vio on sparc/sparc64

Message ID 570F5E89.2080102@physik.fu-berlin.de
State Superseded
Delegated to: David Miller
Headers show

Commit Message

John Paul Adrian Glaubitz April 14, 2016, 9:10 a.m. UTC 
On 04/14/2016 03:53 AM, David Miller wrote:
> You're not hooking this function up at all into the operations.  Looks like
> you are mixing this patch up with other local changes.

You are right. I was switching over to your tree and the __ATTR_RO
somehow fell off while doing that.

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

I have cleaned up the formatting in both patches and edited
the commit messages, attaching updated patches.

Adrian

Comments

Sam Ravnborg April 14, 2016, 10:27 a.m. UTC | #1 
Hi Adrian.

Great work - good to see all the progress on sparc64 in debian!

> >>From b7e3362d71c9d1cb972e5c16e4b9054f8f3de63f Mon Sep 17 00:00:00 2001
> From: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>
> Date: Thu, 14 Apr 2016 01:16:14 +0200
> Subject: [PATCH 1/2] sparc: Implement and wire up modalias_show for vio.

It would be nice if you could add here the effect this patch has - that
it fixes the debian installer.

> 
> Signed-off-by: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>
> ---
>  arch/sparc/kernel/vio.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/arch/sparc/kernel/vio.c b/arch/sparc/kernel/vio.c
> index cb5789c..e7fd1e5 100644
> --- a/arch/sparc/kernel/vio.c
> +++ b/arch/sparc/kernel/vio.c
> @@ -105,9 +105,18 @@ 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)
Please use tabs and the appropriate number of spaces to indent.

> +{
> +	const struct vio_dev *vdev = to_vio_dev(dev);
> +
> +	return sprintf(buf, "vio:T%sS%s\n", vdev->type, vdev->compat);
> +}
> +

At sparclinux we usually submit one patch per mail.
And patches which are specific for either sparc32 or sparc64 are prefixed so.

Like this
[PATCH 1/2] sparc64: ...
[PATCH 2/2] sparc64: ...

	Sam
--
To unsubscribe from this list: send the line "unsubscribe sparclinux" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
John Paul Adrian Glaubitz April 14, 2016, 10:52 a.m. UTC | #2 
Hi Sam!

On 04/14/2016 12:27 PM, Sam Ravnborg wrote:
> Great work - good to see all the progress on sparc64 in debian!

Thanks!

>>> >From b7e3362d71c9d1cb972e5c16e4b9054f8f3de63f Mon Sep 17 00:00:00 2001
>> From: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>
>> Date: Thu, 14 Apr 2016 01:16:14 +0200
>> Subject: [PATCH 1/2] sparc: Implement and wire up modalias_show for vio.
> 
> It would be nice if you could add here the effect this patch has - that
> it fixes the debian installer.

Hmm, I'm not sure whether this belongs here. I thought it was obvious
what these two changes are for due to the canonical function names.

>> Signed-off-by: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>
>> ---
>>  arch/sparc/kernel/vio.c | 9 +++++++++
>>  1 file changed, 9 insertions(+)
>>
>> diff --git a/arch/sparc/kernel/vio.c b/arch/sparc/kernel/vio.c
>> index cb5789c..e7fd1e5 100644
>> --- a/arch/sparc/kernel/vio.c
>> +++ b/arch/sparc/kernel/vio.c
>> @@ -105,9 +105,18 @@ 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)
> Please use tabs and the appropriate number of spaces to indent.

I used the same indentation that is used for modalias_show in
arch/powerpc/kernel/vio.c but I wasn't sure what the proper
indentation would be in this case.

Can you suggest the right indentation?

>> +{
>> +	const struct vio_dev *vdev = to_vio_dev(dev);
>> +
>> +	return sprintf(buf, "vio:T%sS%s\n", vdev->type, vdev->compat);
>> +}
>> +
> 
> At sparclinux we usually submit one patch per mail.
> And patches which are specific for either sparc32 or sparc64 are prefixed so.
> 
> Like this
> [PATCH 1/2] sparc64: ...
> [PATCH 2/2] sparc64: ...

Yeah, I know. I can re-send them with git send-email if that's desired,
I just wanted to avoid cluttering the discussion too much.

Adrian
Sam Ravnborg April 14, 2016, 1:30 p.m. UTC | #3 
Hi Adrian.

> >> Date: Thu, 14 Apr 2016 01:16:14 +0200
> >> Subject: [PATCH 1/2] sparc: Implement and wire up modalias_show for vio.
> > 
> > It would be nice if you could add here the effect this patch has - that
> > it fixes the debian installer.
> 
> Hmm, I'm not sure whether this belongs here. I thought it was obvious
> what these two changes are for due to the canonical function names.
Just a suggestion - skip it as you prefer.

> >> +static ssize_t modalias_show(struct device *dev, struct device_attribute *attr,
> >> +                             char *buf)
> > Please use tabs and the appropriate number of spaces to indent.
> 
> I used the same indentation that is used for modalias_show in
> arch/powerpc/kernel/vio.c but I wasn't sure what the proper
> indentation would be in this case.
> 
> Can you suggest the right indentation?
You shall uses tabs and the necessary number of spaces so the
arguments are aligned with the first argument.
So the alignment is fine - just replace preceeding spaces with tabs.

Like this:
static ssize_t modalias_show(struct device *dev, struct device_attribute *attr,
			     char *buf)

Three leading tabs, then 5 spaces.

> > 
> > At sparclinux we usually submit one patch per mail.
> > And patches which are specific for either sparc32 or sparc64 are prefixed so.
> > 
> > Like this
> > [PATCH 1/2] sparc64: ...
> > [PATCH 2/2] sparc64: ...
> 
> Yeah, I know. I can re-send them with git send-email if that's desired,
> I just wanted to avoid cluttering the discussion too much.
It is my understanding that this is simpler for davem when applying patches.
But maybe patchwork makes things easier with respect to attached patches.
Anyway - if you follow the normal pattern then everything should just works.

Most important, document what was changed from v1 to v2 -
so when davem and other looks at this they know what was changed.

	Sam
--
To unsubscribe from this list: send the line "unsubscribe sparclinux" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
John Paul Adrian Glaubitz April 14, 2016, 4 p.m. UTC | #4 
On 04/14/2016 03:30 PM, Sam Ravnborg wrote:
>> Hmm, I'm not sure whether this belongs here. I thought it was obvious
>> what these two changes are for due to the canonical function names.
> Just a suggestion - skip it as you prefer.

Ok, thanks!

>> I used the same indentation that is used for modalias_show in
>> arch/powerpc/kernel/vio.c but I wasn't sure what the proper
>> indentation would be in this case.
>>
>> Can you suggest the right indentation?
> You shall uses tabs and the necessary number of spaces so the
> arguments are aligned with the first argument.
> So the alignment is fine - just replace preceeding spaces with tabs.
> 
> Like this:
> static ssize_t modalias_show(struct device *dev, struct device_attribute *attr,
> 			     char *buf)
> 
> Three leading tabs, then 5 spaces.

Alright, will do.

>> Yeah, I know. I can re-send them with git send-email if that's desired,
>> I just wanted to avoid cluttering the discussion too much.
> It is my understanding that this is simpler for davem when applying patches.
> But maybe patchwork makes things easier with respect to attached patches.
> Anyway - if you follow the normal pattern then everything should just works.
> 
> Most important, document what was changed from v1 to v2 -
> so when davem and other looks at this they know what was changed.

I'll wait for davem to chime in and then post the patches the
way he prefers.

Thanks for the suggestions!

Adrian
diff mbox

Patch

From 8b42bee34ffadaa1f10d2af6662214fc979fc45d Mon Sep 17 00:00:00 2001
From: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>
Date: Thu, 14 Apr 2016 01:19:40 +0200
Subject: [PATCH 2/2] sparc: Implement and wire up vio_hotplug for vio.

Signed-off-by: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>
---
 arch/sparc/kernel/vio.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/arch/sparc/kernel/vio.c b/arch/sparc/kernel/vio.c
index e7fd1e5..73b33b1 100644
--- a/arch/sparc/kernel/vio.c
+++ b/arch/sparc/kernel/vio.c
@@ -45,6 +45,14 @@  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);
@@ -123,6 +131,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