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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@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.
> 

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



[Index of Archives]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux