[PATCH 0/6] staging: unisys: module autoload support

[PATCH 0/6] staging: unisys: module autoload support

[Benjamin Romer]

This patch series adds support for automatically loading the unisys
drivers from modules.

Prarit Bhargava (6):
  drivers, staging, unisys, cleanup channel_guid.h
  staging: unisys: add UUID strings to channel_guid.h
  drivers, staging, unisys Add modalias files to visorbus devices
  drivers, staging, unisys Add visorbus module autoloading code
  drivers, staging, unisys Add visornic module autoloading code
  drivers, staging, unisys Add visorhid module autoloading code

 drivers/staging/unisys/include/channel_guid.h     | 24 +++++-------
 drivers/staging/unisys/visorbus/visorbus_main.c   | 45 +++++++++++++++++++++--
 drivers/staging/unisys/visorhba/visorhba_main.c   |  2 +
 drivers/staging/unisys/visorhid/keyboardchannel.h |  3 +-
 drivers/staging/unisys/visorhid/mousechannel.h    |  4 +-
 drivers/staging/unisys/visorhid/visorhid.c        | 18 +++------
 drivers/staging/unisys/visornic/visornic_main.c   |  2 +
 7 files changed, 66 insertions(+), 32 deletions(-)

-- 
2.1.4

[PATCH 1/6] drivers, staging, unisys, cleanup channel_guid.h

[Benjamin Romer]

From: Prarit Bhargava <prarit@redhat.com>

This file has a lot of dead comments and needs to be cleaned up.

Signed-off-by: Prarit Bhargava <prarit@redhat.com>
---
 drivers/staging/unisys/include/channel_guid.h | 20 +++++---------------
 1 file changed, 5 insertions(+), 15 deletions(-)

diff --git a/drivers/staging/unisys/include/channel_guid.h b/drivers/staging/unisys/include/channel_guid.h
index 706363fc..2cc4b0b 100644
--- a/drivers/staging/unisys/include/channel_guid.h
+++ b/drivers/staging/unisys/include/channel_guid.h
@@ -17,35 +17,27 @@
  * CHANNEL Guids
  */
 
-/* Used in IOChannel
- * {414815ed-c58c-11da-95a9-00e08161165f}
- */
+/* {414815ed-c58c-11da-95a9-00e08161165f} */
 #define SPAR_VHBA_CHANNEL_PROTOCOL_UUID \
 		UUID_LE(0x414815ed, 0xc58c, 0x11da, \
 				0x95, 0xa9, 0x0, 0xe0, 0x81, 0x61, 0x16, 0x5f)
 static const uuid_le spar_vhba_channel_protocol_uuid =
 	SPAR_VHBA_CHANNEL_PROTOCOL_UUID;
 
-/* Used in IOChannel
- * {8cd5994d-c58e-11da-95a9-00e08161165f}
- */
+/* {8cd5994d-c58e-11da-95a9-00e08161165f} */
 #define SPAR_VNIC_CHANNEL_PROTOCOL_UUID \
 		UUID_LE(0x8cd5994d, 0xc58e, 0x11da, \
 				0x95, 0xa9, 0x0, 0xe0, 0x81, 0x61, 0x16, 0x5f)
 static const uuid_le spar_vnic_channel_protocol_uuid =
 	SPAR_VNIC_CHANNEL_PROTOCOL_UUID;
 
-/* Used in IOChannel
- * {72120008-4AAB-11DC-8530-444553544200}
- */
+/* {72120008-4AAB-11DC-8530-444553544200} */
 #define SPAR_SIOVM_UUID \
 		UUID_LE(0x72120008, 0x4AAB, 0x11DC, \
 				0x85, 0x30, 0x44, 0x45, 0x53, 0x54, 0x42, 0x00)
 static const uuid_le spar_siovm_uuid = SPAR_SIOVM_UUID;
 
-/* Used in visornoop/visornoop_main.c
- * {5b52c5ac-e5f5-4d42-8dff-429eaecd221f}
- */
+/* {5b52c5ac-e5f5-4d42-8dff-429eaecd221f} */
 #define SPAR_CONTROLDIRECTOR_CHANNEL_PROTOCOL_UUID  \
 		UUID_LE(0x5b52c5ac, 0xe5f5, 0x4d42, \
 				0x8d, 0xff, 0x42, 0x9e, 0xae, 0xcd, 0x22, 0x1f)
@@ -53,9 +45,7 @@ static const uuid_le spar_siovm_uuid = SPAR_SIOVM_UUID;
 static const uuid_le spar_controldirector_channel_protocol_uuid =
 	SPAR_CONTROLDIRECTOR_CHANNEL_PROTOCOL_UUID;
 
-/* Used in visorchipset/visorchipset_main.c
- * {B4E79625-AEDE-4EAA-9E11-D3EDDCD4504C}
- */
+/* {b4e79625-aede-4eAA-9e11-D3eddcd4504c} */
 #define SPAR_DIAG_POOL_CHANNEL_PROTOCOL_UUID				\
 		UUID_LE(0xb4e79625, 0xaede, 0x4eaa, \
 				0x9e, 0x11, 0xd3, 0xed, 0xdc, 0xd4, 0x50, 0x4c)
-- 
2.1.4

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 2/6] staging: unisys: add UUID strings to channel_guid.h

[Benjamin Romer]

From: Prarit Bhargava <prarit@redhat.com>

Define additional strings for module loading code ease of use.

Signed-off-by: Prarit Bhargava <prarit@redhat.com>
---
 drivers/staging/unisys/include/channel_guid.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/staging/unisys/include/channel_guid.h b/drivers/staging/unisys/include/channel_guid.h
index 2cc4b0b..17cb499 100644
--- a/drivers/staging/unisys/include/channel_guid.h
+++ b/drivers/staging/unisys/include/channel_guid.h
@@ -23,6 +23,8 @@
 				0x95, 0xa9, 0x0, 0xe0, 0x81, 0x61, 0x16, 0x5f)
 static const uuid_le spar_vhba_channel_protocol_uuid =
 	SPAR_VHBA_CHANNEL_PROTOCOL_UUID;
+#define SPAR_VHBA_CHANNEL_PROTOCOL_UUID_STR \
+	"414815ed-c58c-11da-95a9-00e08161165f"
 
 /* {8cd5994d-c58e-11da-95a9-00e08161165f} */
 #define SPAR_VNIC_CHANNEL_PROTOCOL_UUID \
@@ -30,6 +32,8 @@ static const uuid_le spar_vhba_channel_protocol_uuid =
 				0x95, 0xa9, 0x0, 0xe0, 0x81, 0x61, 0x16, 0x5f)
 static const uuid_le spar_vnic_channel_protocol_uuid =
 	SPAR_VNIC_CHANNEL_PROTOCOL_UUID;
+#define SPAR_VNIC_CHANNEL_PROTOCOL_UUID_STR \
+	"8cd5994d-c58e-11da-95a9-00e08161165f"
 
 /* {72120008-4AAB-11DC-8530-444553544200} */
 #define SPAR_SIOVM_UUID \
-- 
2.1.4

[PATCH 3/6] drivers, staging, unisys Add modalias files to visorbus devices

[Benjamin Romer]

From: Prarit Bhargava <prarit@redhat.com>

This patch adds modalias files that export the device UUID type to sysfs
so that udev can autoload the appropriate device driver on demand.  Note
that is required a minor name change to the channel device sysfs files
which are currently named visorbus_dev_groups, and are now named
visorbus_channel_groups.

Signed-off-by: Prarit Bhargava <prarit@redhat.com>
---
 drivers/staging/unisys/visorbus/visorbus_main.c | 45 +++++++++++++++++++++++--
 1 file changed, 42 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/unisys/visorbus/visorbus_main.c b/drivers/staging/unisys/visorbus/visorbus_main.c
index 403c13b..cc4a029 100644
--- a/drivers/staging/unisys/visorbus/visorbus_main.c
+++ b/drivers/staging/unisys/visorbus/visorbus_main.c
@@ -69,6 +69,38 @@ static const struct attribute_group *visorbus_bus_groups[] = {
 	NULL,
 };
 
+/*
+ * DEVICE type attributes
+ *
+ * The modalias file will contain the guid of the device.
+ */
+static ssize_t modalias_show(struct device *dev, struct device_attribute *attr,
+			     char *buf)
+{
+	struct visor_device *vdev;
+	uuid_le guid;
+
+	vdev = to_visor_device(dev);
+	guid = visorchannel_get_uuid(vdev->visorchannel);
+	return snprintf(buf, PAGE_SIZE, "visorbus:%pUl\n", &guid);
+}
+static DEVICE_ATTR_RO(modalias);
+
+static struct attribute *visorbus_dev_attrs[] = {
+	&dev_attr_modalias.attr,
+	NULL,
+};
+
+/* sysfs example for bridge-only sysfs files using device_type's */
+static const struct attribute_group visorbus_dev_group = {
+	.attrs = visorbus_dev_attrs,
+};
+
+const struct attribute_group *visorbus_dev_groups[] = {
+	&visorbus_dev_group,
+	NULL,
+};
+
 /** This describes the TYPE of bus.
  *  (Don't confuse this with an INSTANCE of the bus.)
  */
@@ -76,6 +108,7 @@ struct bus_type visorbus_type = {
 	.name = "visorbus",
 	.match = visorbus_match,
 	.uevent = visorbus_uevent,
+	.dev_groups = visorbus_dev_groups,
 	.bus_groups = visorbus_bus_groups,
 };
 
@@ -128,7 +161,13 @@ static LIST_HEAD(list_all_device_instances);
 static int
 visorbus_uevent(struct device *xdev, struct kobj_uevent_env *env)
 {
-	if (add_uevent_var(env, "VERSION=%s", VERSION))
+	struct visor_device *dev;
+	uuid_le guid;
+
+	dev = to_visor_device(xdev);
+	guid = visorchannel_get_uuid(dev->visorchannel);
+
+	if (add_uevent_var(env, "MODALIAS=visorbus:%pUl", &guid))
 		return -ENOMEM;
 	return 0;
 }
@@ -477,7 +516,7 @@ static struct attribute_group channel_attr_grp = {
 		.attrs = channel_attrs,
 };
 
-static const struct attribute_group *visorbus_dev_groups[] = {
+static const struct attribute_group *visorbus_channel_groups[] = {
 		&channel_attr_grp,
 		NULL
 };
@@ -936,7 +975,7 @@ create_visor_device(struct visor_device *dev)
 
 	sema_init(&dev->visordriver_callback_lock, 1);	/* unlocked */
 	dev->device.bus = &visorbus_type;
-	dev->device.groups = visorbus_dev_groups;
+	dev->device.groups = visorbus_channel_groups;
 	device_initialize(&dev->device);
 	dev->device.release = visorbus_release_device;
 	/* keep a reference just for us (now 2) */
-- 
2.1.4

[PATCH 4/6] drivers, staging, unisys Add visorbus module autoloading code

[Benjamin Romer]

From: Prarit Bhargava <prarit@redhat.com>

This patch adds an module alias and a MODULE_DEVICE_TABLE to autoload the
visorhba driver when an appropriate device is created by the visorbus.

Signed-off-by: Prarit Bhargava <prarit@redhat.com>
---
 drivers/staging/unisys/visorhba/visorhba_main.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/staging/unisys/visorhba/visorhba_main.c b/drivers/staging/unisys/visorhba/visorhba_main.c
index 031c6fa..50ecdf2 100644
--- a/drivers/staging/unisys/visorhba/visorhba_main.c
+++ b/drivers/staging/unisys/visorhba/visorhba_main.c
@@ -80,6 +80,8 @@ static struct visor_driver visorhba_driver = {
 	.resume = visorhba_resume,
 	.channel_interrupt = NULL,
 };
+MODULE_DEVICE_TABLE(visorbus, visorhba_channel_types);
+MODULE_ALIAS("visorbus:" SPAR_VHBA_CHANNEL_PROTOCOL_UUID_STR);
 
 struct visor_thread_info {
 	struct task_struct *task;
-- 
2.1.4

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 5/6] drivers, staging, unisys Add visornic module autoloading code

[Benjamin Romer]

From: Prarit Bhargava <prarit@redhat.com>

This patch adds an module alias and a MODULE_DEVICE_TABLE to autoload the
visornic driver when an appropriate device is created by the visorbus.

Signed-off-by: Prarit Bhargava <prarit@redhat.com>
---
 drivers/staging/unisys/visornic/visornic_main.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/staging/unisys/visornic/visornic_main.c b/drivers/staging/unisys/visornic/visornic_main.c
index 4b5fc07..bfc9727 100644
--- a/drivers/staging/unisys/visornic/visornic_main.c
+++ b/drivers/staging/unisys/visornic/visornic_main.c
@@ -71,6 +71,8 @@ static struct visor_channeltype_descriptor visornic_channel_types[] = {
 	{ SPAR_VNIC_CHANNEL_PROTOCOL_UUID, "ultravnic" },
 	{ NULL_UUID_LE, NULL }
 };
+MODULE_DEVICE_TABLE(visorbus, visornic_channel_types);
+MODULE_ALIAS("visorbus:" SPAR_VNIC_CHANNEL_PROTOCOL_UUID_STR);
 
 /* This is used to tell the visor bus driver which types of visor devices
  * we support, and what functions to call when a visor device that we support
-- 
2.1.4

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 6/6] drivers, staging, unisys Add visorhid module autoloading code

[Benjamin Romer]

From: Prarit Bhargava <prarit@redhat.com>

This patch adds an module alias and a MODULE_DEVICE_TABLE to autoload
the
visorhid driver when an appropriate device is created by the visorbus.
Additional code cleanups to match the other visor* drivers.

Signed-off-by: Prarit Bhargava <prarit@redhat.com>
---
 drivers/staging/unisys/visorhid/keyboardchannel.h |  3 ++-
 drivers/staging/unisys/visorhid/mousechannel.h    |  4 +++-
 drivers/staging/unisys/visorhid/visorhid.c        | 18 ++++++------------
 3 files changed, 11 insertions(+), 14 deletions(-)

diff --git a/drivers/staging/unisys/visorhid/keyboardchannel.h b/drivers/staging/unisys/visorhid/keyboardchannel.h
index 1d11966..2ed2602 100644
--- a/drivers/staging/unisys/visorhid/keyboardchannel.h
+++ b/drivers/staging/unisys/visorhid/keyboardchannel.h
@@ -21,10 +21,11 @@
 #include "channel.h"
 #include "ultrainputreport.h"
 
-/* {C73416D0-B0B8-44af-B304-9D2AE99F1B3D} */
+/* {c73416d0-b0b8-44af-b304-9d2ae99f1b3d} */
 #define SPAR_KEYBOARD_CHANNEL_PROTOCOL_UUID				\
 	UUID_LE(0xc73416d0, 0xb0b8, 0x44af,				\
 		0xb3, 0x4, 0x9d, 0x2a, 0xe9, 0x9f, 0x1b, 0x3d)
+#define SPAR_KEYBOARD_CHANNEL_PROTOCOL_UUID_STR "c73416d0-b0b8-44af-b304-9d2ae99f1b3d"
 #define SPAR_KEYBOARD_CHANNEL_PROTOCOL_VERSIONID 1
 #define KEYBOARD_MAXINPUTREPORTS 50
 
diff --git a/drivers/staging/unisys/visorhid/mousechannel.h b/drivers/staging/unisys/visorhid/mousechannel.h
index 2633078..256477a 100644
--- a/drivers/staging/unisys/visorhid/mousechannel.h
+++ b/drivers/staging/unisys/visorhid/mousechannel.h
@@ -21,10 +21,12 @@
 #include "channel.h"
 #include "ultrainputreport.h"
 
-/* {ADDF07D4-94A9-46e2-81C3-61ABCDBDBD87} */
+/* {addf07d4-94a9-46e2-81c3-61abcdbdbd87} */
 #define SPAR_MOUSE_CHANNEL_PROTOCOL_UUID  \
 	UUID_LE(0xaddf07d4, 0x94a9, 0x46e2, \
 		0x81, 0xc3, 0x61, 0xab, 0xcd, 0xbd, 0xbd, 0x87)
+#define SPAR_MOUSE_CHANNEL_PROTOCOL_UUID_STR \
+	"addf07d4-94a9-46e2-81c3-61abcdbdbd87"
 #define SPAR_MOUSE_CHANNEL_PROTOCOL_VERSIONID 1
 #define MOUSE_MAXINPUTREPORTS 50
 
diff --git a/drivers/staging/unisys/visorhid/visorhid.c b/drivers/staging/unisys/visorhid/visorhid.c
index 1eaade0..7785088 100644
--- a/drivers/staging/unisys/visorhid/visorhid.c
+++ b/drivers/staging/unisys/visorhid/visorhid.c
@@ -53,19 +53,13 @@ static void unregister_client_input(struct input_dev *visorinput_dev);
 
 /* GUIDS for all channel types supported by this driver. */
 static struct visor_channeltype_descriptor visorhid_channel_types[] = {
-	{
-		.guid = SPAR_KEYBOARD_CHANNEL_PROTOCOL_UUID,
-		.name = "keyboard"
-	},
-	{
-		.guid = SPAR_MOUSE_CHANNEL_PROTOCOL_UUID,
-		.name = "mouse"
-	},
-	{
-		.guid = NULL_UUID_LE,
-		.name = NULL
-	}
+	{ SPAR_KEYBOARD_CHANNEL_PROTOCOL_UUID, "keyboard"},
+	{ SPAR_MOUSE_CHANNEL_PROTOCOL_UUID, "mouse"},
+	{ NULL_UUID_LE, NULL }
 };
+MODULE_DEVICE_TABLE(visorbus, visorhid_channel_types);
+MODULE_ALIAS("visorbus:" SPAR_MOUSE_CHANNEL_PROTOCOL_UUID_STR);
+MODULE_ALIAS("visorbus:" SPAR_KEYBOARD_CHANNEL_PROTOCOL_UUID_STR);
 
 /** This is used to tell the visor bus driver which types of visor devices
  *  we support, and what functions to call when a visor device that we support
-- 
2.1.4

Re: [PATCH 4/6] drivers, staging, unisys Add visorbus module autoloading code

[Greg KH]

On Fri, Jul 24, 2015 at 12:06:54PM -0400, Benjamin Romer wrote:
> From: Prarit Bhargava <prarit@redhat.com>
> 
> This patch adds an module alias and a MODULE_DEVICE_TABLE to autoload the
> visorhba driver when an appropriate device is created by the visorbus.
> 
> Signed-off-by: Prarit Bhargava <prarit@redhat.com>
> ---
>  drivers/staging/unisys/visorhba/visorhba_main.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/staging/unisys/visorhba/visorhba_main.c b/drivers/staging/unisys/visorhba/visorhba_main.c
> index 031c6fa..50ecdf2 100644
> --- a/drivers/staging/unisys/visorhba/visorhba_main.c
> +++ b/drivers/staging/unisys/visorhba/visorhba_main.c
> @@ -80,6 +80,8 @@ static struct visor_driver visorhba_driver = {
>  	.resume = visorhba_resume,
>  	.channel_interrupt = NULL,
>  };
> +MODULE_DEVICE_TABLE(visorbus, visorhba_channel_types);
> +MODULE_ALIAS("visorbus:" SPAR_VHBA_CHANNEL_PROTOCOL_UUID_STR);

You shouldn't have to write MODULE_ALIAS lines "by hand", they should be
generated for you automagically from the MODULE_DEVICE_TABLE() define,
if you do it right.

You all didn't do it right :)

Please fix up.

thanks,

greg k-h
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 0/6] staging: unisys: module autoload support

[Greg KH]

On Fri, Jul 24, 2015 at 12:06:50PM -0400, Benjamin Romer wrote:
> This patch series adds support for automatically loading the unisys
> drivers from modules.

first 3 patches applied only.
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 4/6] drivers, staging, unisys Add visorbus module autoloading code

[Prarit Bhargava]

On 07/31/2015 07:07 PM, Greg KH wrote:
> On Fri, Jul 24, 2015 at 12:06:54PM -0400, Benjamin Romer wrote:
>> From: Prarit Bhargava <prarit@redhat.com>
>>
>> This patch adds an module alias and a MODULE_DEVICE_TABLE to autoload the
>> visorhba driver when an appropriate device is created by the visorbus.
>>
>> Signed-off-by: Prarit Bhargava <prarit@redhat.com>
>> ---
>>  drivers/staging/unisys/visorhba/visorhba_main.c | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/staging/unisys/visorhba/visorhba_main.c b/drivers/staging/unisys/visorhba/visorhba_main.c
>> index 031c6fa..50ecdf2 100644
>> --- a/drivers/staging/unisys/visorhba/visorhba_main.c
>> +++ b/drivers/staging/unisys/visorhba/visorhba_main.c
>> @@ -80,6 +80,8 @@ static struct visor_driver visorhba_driver = {
>>  	.resume = visorhba_resume,
>>  	.channel_interrupt = NULL,
>>  };
>> +MODULE_DEVICE_TABLE(visorbus, visorhba_channel_types);
>> +MODULE_ALIAS("visorbus:" SPAR_VHBA_CHANNEL_PROTOCOL_UUID_STR);
> 
> You shouldn't have to write MODULE_ALIAS lines "by hand", they should be
> generated for you automagically from the MODULE_DEVICE_TABLE() define,
> if you do it right.
> 
> You all didn't do it right :)
> 
> Please fix up.
> 

Whups.  Will fix up.

P.

> thanks,
> 
> greg k-h
> 
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 4/6] drivers, staging, unisys Add visorbus module autoloading code

[Prarit Bhargava]

On 07/31/2015 07:07 PM, Greg KH wrote:> On Fri, Jul 24, 2015 at 12:06:54PM -0400, Benjamin Romer wrote:
>> From: Prarit Bhargava <prarit@redhat.com>
>>
>> This patch adds an module alias and a MODULE_DEVICE_TABLE to autoload the
>> visorhba driver when an appropriate device is created by the visorbus.
>>
>> Signed-off-by: Prarit Bhargava <prarit@redhat.com>
>> ---
>>  drivers/staging/unisys/visorhba/visorhba_main.c | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/staging/unisys/visorhba/visorhba_main.c b/drivers/staging/unisys/visorhba/visorhba_main.c
>> index 031c6fa..50ecdf2 100644
>> --- a/drivers/staging/unisys/visorhba/visorhba_main.c
>> +++ b/drivers/staging/unisys/visorhba/visorhba_main.c
>> @@ -80,6 +80,8 @@ static struct visor_driver visorhba_driver = {
>>  	.resume = visorhba_resume,
>>  	.channel_interrupt = NULL,
>>  };
>> +MODULE_DEVICE_TABLE(visorbus, visorhba_channel_types);
>> +MODULE_ALIAS("visorbus:" SPAR_VHBA_CHANNEL_PROTOCOL_UUID_STR);
> 
> You shouldn't have to write MODULE_ALIAS lines "by hand", they should be
> generated for you automagically from the MODULE_DEVICE_TABLE() define,
> if you do it right.
> 
> You all didn't do it right :)
> 

Greg,

I need further clarification on the right approach for the autoload code.

In order to "do it right" I would have to do something like the following
which would correctly add module alias entries for the visorbus bus.
The issue that I have with this patch is that it touches code outside
of drivers/staging for a driver that is solely within drivers/staging.

Is modifying non drivers/staging okay to do?  Or is simply doing

MODULE_ALIAS("visorbus:" SPAR_VHBA_CHANNEL_PROTOCOL_UUID_STR);

with a loud /* FIXME before submitting ... */ warning a better approach?

Thanks,

P.

---8<---

drivers/staging: visorbus, add module autoload functionality

This patch adds module autoload functionality to the visorbus bus drivers
(currently only visornic is in tree).  This patch adds visorbus alias
creation entries to modpost, and moves the appropriate structure define
into include/linux/mod_devicetable.h.

Signed-off-by: Prarit Bhargava <prarit@redhat.com>
---
 drivers/staging/unisys/include/visorbus.h       |  8 --------
 drivers/staging/unisys/visorbus/visorbus_main.c |  1 +
 include/linux/mod_devicetable.h                 | 11 +++++++++++
 scripts/mod/devicetable-offsets.c               |  4 ++++
 scripts/mod/file2alias.c                        | 11 +++++++++++
 5 files changed, 27 insertions(+), 8 deletions(-)

diff --git a/drivers/staging/unisys/include/visorbus.h b/drivers/staging/unisys/include/visorbus.h
index a0144c6..35da159 100644
--- a/drivers/staging/unisys/include/visorbus.h
+++ b/drivers/staging/unisys/include/visorbus.h
@@ -53,14 +53,6 @@ struct visorchipset_state {
 	/* Remaining bits in this 32-bit word are unused. */
 };
 
-/** This struct describes a specific Supervisor channel, by providing its
- *  GUID, name, and sizes.
- */
-struct visor_channeltype_descriptor {
-	const uuid_le guid;
-	const char *name;
-};
-
 /** Information provided by each visor driver when it registers with the
  *  visorbus driver.
  */
diff --git a/drivers/staging/unisys/visorbus/visorbus_main.c b/drivers/staging/unisys/visorbus/visorbus_main.c
index cc4a029..fcb491a 100644
--- a/drivers/staging/unisys/visorbus/visorbus_main.c
+++ b/drivers/staging/unisys/visorbus/visorbus_main.c
@@ -15,6 +15,7 @@
  */
 
 #include <linux/uuid.h>
+#include <linux/mod_devicetable.h>
 
 #include "visorbus.h"
 #include "visorbus_private.h"
diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_devicetable.h
index 34f25b7..acb70ed 100644
--- a/include/linux/mod_devicetable.h
+++ b/include/linux/mod_devicetable.h
@@ -650,4 +650,15 @@ struct ulpi_device_id {
 	kernel_ulong_t driver_data;
 };
 
+/**
+ * struct visor_channeltype_descriptor - VISORBUS Channel Type Descriptor
+ * @guid: UUID for the channel
+ * @name: name of the channel
+ *
+ * describes a specific Supervisor channel, by providing its * GUID and name.
+ */
+struct visor_channeltype_descriptor {
+	uuid_le guid;
+	char *name;
+};
 #endif /* LINUX_MOD_DEVICETABLE_H */
diff --git a/scripts/mod/devicetable-offsets.c b/scripts/mod/devicetable-offsets.c
index e70fcd1..4b8797b 100644
--- a/scripts/mod/devicetable-offsets.c
+++ b/scripts/mod/devicetable-offsets.c
@@ -196,5 +196,9 @@ int main(void)
 	DEVID_FIELD(ulpi_device_id, vendor);
 	DEVID_FIELD(ulpi_device_id, product);
 
+	DEVID(visor_channeltype_descriptor);
+	DEVID_FIELD(visor_channeltype_descriptor, guid);
+	DEVID_FIELD(visor_channeltype_descriptor, name);
+
 	return 0;
 }
diff --git a/scripts/mod/file2alias.c b/scripts/mod/file2alias.c
index 5f20882..e90374ba 100644
--- a/scripts/mod/file2alias.c
+++ b/scripts/mod/file2alias.c
@@ -1250,6 +1250,17 @@ static int do_ulpi_entry(const char *filename, void *symval,
 }
 ADD_TO_DEVTABLE("ulpi", ulpi_device_id, do_ulpi_entry);
 
+static int do_visorbus_entry(const char *filename, void *symval, char *alias)
+{
+	DEF_FIELD_ADDR(symval, visor_channeltype_descriptor, guid);
+
+	sprintf(alias, "visorbus:");
+	add_uuid(alias, *guid);
+
+	return 1;
+}
+ADD_TO_DEVTABLE("visorbus", visor_channeltype_descriptor, do_visorbus_entry);
+
 /* Does namelen bytes of name exactly match the symbol? */
 static bool sym_is(const char *name, unsigned namelen, const char *symbol)
 {
-- 
2.4.3

Re: [PATCH 4/6] drivers, staging, unisys Add visorbus module autoloading code

[Prarit Bhargava]

Top posting ;)  Greg -- I know you're busy but I just wanted to make sure you
didn't miss this question re: drivers/staging and patches that touch other areas
of the kernel.

Thanks!

P.

On 08/03/2015 01:21 PM, Prarit Bhargava wrote:
> On 07/31/2015 07:07 PM, Greg KH wrote:> On Fri, Jul 24, 2015 at 12:06:54PM -0400, Benjamin Romer wrote:
>>> From: Prarit Bhargava <prarit@redhat.com>
>>>
>>> This patch adds an module alias and a MODULE_DEVICE_TABLE to autoload the
>>> visorhba driver when an appropriate device is created by the visorbus.
>>>
>>> Signed-off-by: Prarit Bhargava <prarit@redhat.com>
>>> ---
>>>  drivers/staging/unisys/visorhba/visorhba_main.c | 2 ++
>>>  1 file changed, 2 insertions(+)
>>>
>>> diff --git a/drivers/staging/unisys/visorhba/visorhba_main.c b/drivers/staging/unisys/visorhba/visorhba_main.c
>>> index 031c6fa..50ecdf2 100644
>>> --- a/drivers/staging/unisys/visorhba/visorhba_main.c
>>> +++ b/drivers/staging/unisys/visorhba/visorhba_main.c
>>> @@ -80,6 +80,8 @@ static struct visor_driver visorhba_driver = {
>>>  	.resume = visorhba_resume,
>>>  	.channel_interrupt = NULL,
>>>  };
>>> +MODULE_DEVICE_TABLE(visorbus, visorhba_channel_types);
>>> +MODULE_ALIAS("visorbus:" SPAR_VHBA_CHANNEL_PROTOCOL_UUID_STR);
>>
>> You shouldn't have to write MODULE_ALIAS lines "by hand", they should be
>> generated for you automagically from the MODULE_DEVICE_TABLE() define,
>> if you do it right.
>>
>> You all didn't do it right :)
>>
> 
> Greg,
> 
> I need further clarification on the right approach for the autoload code.
> 
> In order to "do it right" I would have to do something like the following
> which would correctly add module alias entries for the visorbus bus.
> The issue that I have with this patch is that it touches code outside
> of drivers/staging for a driver that is solely within drivers/staging.
> 
> Is modifying non drivers/staging okay to do?  Or is simply doing
> 
> MODULE_ALIAS("visorbus:" SPAR_VHBA_CHANNEL_PROTOCOL_UUID_STR);
> 
> with a loud /* FIXME before submitting ... */ warning a better approach?
> 
> Thanks,
> 
> P.
> 
> ---8<---
> 
> drivers/staging: visorbus, add module autoload functionality
> 
> This patch adds module autoload functionality to the visorbus bus drivers
> (currently only visornic is in tree).  This patch adds visorbus alias
> creation entries to modpost, and moves the appropriate structure define
> into include/linux/mod_devicetable.h.
> 
> Signed-off-by: Prarit Bhargava <prarit@redhat.com>
> ---
>  drivers/staging/unisys/include/visorbus.h       |  8 --------
>  drivers/staging/unisys/visorbus/visorbus_main.c |  1 +
>  include/linux/mod_devicetable.h                 | 11 +++++++++++
>  scripts/mod/devicetable-offsets.c               |  4 ++++
>  scripts/mod/file2alias.c                        | 11 +++++++++++
>  5 files changed, 27 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/staging/unisys/include/visorbus.h b/drivers/staging/unisys/include/visorbus.h
> index a0144c6..35da159 100644
> --- a/drivers/staging/unisys/include/visorbus.h
> +++ b/drivers/staging/unisys/include/visorbus.h
> @@ -53,14 +53,6 @@ struct visorchipset_state {
>  	/* Remaining bits in this 32-bit word are unused. */
>  };
>  
> -/** This struct describes a specific Supervisor channel, by providing its
> - *  GUID, name, and sizes.
> - */
> -struct visor_channeltype_descriptor {
> -	const uuid_le guid;
> -	const char *name;
> -};
> -
>  /** Information provided by each visor driver when it registers with the
>   *  visorbus driver.
>   */
> diff --git a/drivers/staging/unisys/visorbus/visorbus_main.c b/drivers/staging/unisys/visorbus/visorbus_main.c
> index cc4a029..fcb491a 100644
> --- a/drivers/staging/unisys/visorbus/visorbus_main.c
> +++ b/drivers/staging/unisys/visorbus/visorbus_main.c
> @@ -15,6 +15,7 @@
>   */
>  
>  #include <linux/uuid.h>
> +#include <linux/mod_devicetable.h>
>  
>  #include "visorbus.h"
>  #include "visorbus_private.h"
> diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_devicetable.h
> index 34f25b7..acb70ed 100644
> --- a/include/linux/mod_devicetable.h
> +++ b/include/linux/mod_devicetable.h
> @@ -650,4 +650,15 @@ struct ulpi_device_id {
>  	kernel_ulong_t driver_data;
>  };
>  
> +/**
> + * struct visor_channeltype_descriptor - VISORBUS Channel Type Descriptor
> + * @guid: UUID for the channel
> + * @name: name of the channel
> + *
> + * describes a specific Supervisor channel, by providing its * GUID and name.
> + */
> +struct visor_channeltype_descriptor {
> +	uuid_le guid;
> +	char *name;
> +};
>  #endif /* LINUX_MOD_DEVICETABLE_H */
> diff --git a/scripts/mod/devicetable-offsets.c b/scripts/mod/devicetable-offsets.c
> index e70fcd1..4b8797b 100644
> --- a/scripts/mod/devicetable-offsets.c
> +++ b/scripts/mod/devicetable-offsets.c
> @@ -196,5 +196,9 @@ int main(void)
>  	DEVID_FIELD(ulpi_device_id, vendor);
>  	DEVID_FIELD(ulpi_device_id, product);
>  
> +	DEVID(visor_channeltype_descriptor);
> +	DEVID_FIELD(visor_channeltype_descriptor, guid);
> +	DEVID_FIELD(visor_channeltype_descriptor, name);
> +
>  	return 0;
>  }
> diff --git a/scripts/mod/file2alias.c b/scripts/mod/file2alias.c
> index 5f20882..e90374ba 100644
> --- a/scripts/mod/file2alias.c
> +++ b/scripts/mod/file2alias.c
> @@ -1250,6 +1250,17 @@ static int do_ulpi_entry(const char *filename, void *symval,
>  }
>  ADD_TO_DEVTABLE("ulpi", ulpi_device_id, do_ulpi_entry);
>  
> +static int do_visorbus_entry(const char *filename, void *symval, char *alias)
> +{
> +	DEF_FIELD_ADDR(symval, visor_channeltype_descriptor, guid);
> +
> +	sprintf(alias, "visorbus:");
> +	add_uuid(alias, *guid);
> +
> +	return 1;
> +}
> +ADD_TO_DEVTABLE("visorbus", visor_channeltype_descriptor, do_visorbus_entry);
> +
>  /* Does namelen bytes of name exactly match the symbol? */
>  static bool sym_is(const char *name, unsigned namelen, const char *symbol)
>  {
> 
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 4/6] drivers, staging, unisys Add visorbus module autoloading code

[Dan Carpenter]

On Mon, Aug 03, 2015 at 01:21:35PM -0400, Prarit Bhargava wrote:
> 
> On 07/31/2015 07:07 PM, Greg KH wrote:> On Fri, Jul 24, 2015 at 12:06:54PM -0400, Benjamin Romer wrote:
> >> From: Prarit Bhargava <prarit@redhat.com>
> >>
> >> This patch adds an module alias and a MODULE_DEVICE_TABLE to autoload the
> >> visorhba driver when an appropriate device is created by the visorbus.
> >>
> >> Signed-off-by: Prarit Bhargava <prarit@redhat.com>
> >> ---
> >>  drivers/staging/unisys/visorhba/visorhba_main.c | 2 ++
> >>  1 file changed, 2 insertions(+)
> >>
> >> diff --git a/drivers/staging/unisys/visorhba/visorhba_main.c b/drivers/staging/unisys/visorhba/visorhba_main.c
> >> index 031c6fa..50ecdf2 100644
> >> --- a/drivers/staging/unisys/visorhba/visorhba_main.c
> >> +++ b/drivers/staging/unisys/visorhba/visorhba_main.c
> >> @@ -80,6 +80,8 @@ static struct visor_driver visorhba_driver = {
> >>  	.resume = visorhba_resume,
> >>  	.channel_interrupt = NULL,
> >>  };
> >> +MODULE_DEVICE_TABLE(visorbus, visorhba_channel_types);
> >> +MODULE_ALIAS("visorbus:" SPAR_VHBA_CHANNEL_PROTOCOL_UUID_STR);
> > 
> > You shouldn't have to write MODULE_ALIAS lines "by hand", they should be
> > generated for you automagically from the MODULE_DEVICE_TABLE() define,
> > if you do it right.
> > 
> > You all didn't do it right :)
> > 
> 
> Greg,
> 
> I need further clarification on the right approach for the autoload code.
> 
> In order to "do it right" I would have to do something like the following
> which would correctly add module alias entries for the visorbus bus.
> The issue that I have with this patch is that it touches code outside
> of drivers/staging for a driver that is solely within drivers/staging.
> 

Yeah.  Don't do that.

> Is modifying non drivers/staging okay to do?  Or is simply doing
> 
> MODULE_ALIAS("visorbus:" SPAR_VHBA_CHANNEL_PROTOCOL_UUID_STR);
> 
> with a loud /* FIXME before submitting ... */ warning a better approach?
> 

I'm not Greg, but I would go with this approach.  Put the file2alias.c
change in the FIXME comment so we can just copy it over after the driver
is merged.

regards,
dan carpenter

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel