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