On Wed, Apr 14, 2010 at 07:16:35PM +0200, Micha?? Nazarewicz wrote: >> On Wed, Apr 14, 2010 at 06:41:00PM +0200, Micha?? Nazarewicz wrote: >>> Hello everyone, >>> >>> I've noticed that composite.c declares several module parameters >>> to let one change various device IDs, however, those have >>> permissions set to zero. Is there any reason why not to set them >>> to 0644? > > On Wed, 14 Apr 2010 18:59:49 +0200, Greg KH <greg@xxxxxxxxx> wrote: >> I think it is because you can't change these after the module is loaded, >> it needs to be there before the connection happens, right? > > Oh... So zero as perm means that one can set the value as a module > parameter but then it won't be seen in sysfs? Now I get it, thanks! > > Still, since with FunctionFS the composite device may be registered > and deregistered several times while the module is running would it > make sense to create a macro for changing the perm? I'm thinking of > > #v+ > #ifndef USB_COMPOSITE_PARAM_PERM > # define USB_COMPOSITE_PARAM_PERM 0 > #endif > > static ushort idVendor; > module_param(idVendor, ushort, USB_COMPOSITE_PARAM_PERM); > MODULE_PARM_DESC(idVendor, "USB Vendor ID"); > > /* ... */ > #v- > > #v+ > #define USB_COMPOSITE_PARAM_PERM 0644 > #include "composite.c" > #v- > > Or is it too ugly? Ick, that's ugly :) And it keeps anyone from building a kernel that works on multiple types of systems, which is a strong goal right now for a large number of people, even for ARM systems. > For me, the other solution would be duplicating similar functionality > in the g_ffs (see my patches last week) which I would like to avoid. Well, you can get a notification in your code when these options are written to from userspace, so perhaps if you just properly handle things it should be fine. Maybe you need to fail the write if the device is connected and you can't change the values at that time. thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
