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@xxxxxxxxxx>
>>
>> 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@xxxxxxxxxx>
>> ---
>> 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@xxxxxxxxxx>
---
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
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
