* Felipe Balbi | 2012-11-20 14:14:47 [+0200]:
>On Wed, Nov 14, 2012 at 06:14:56PM +0100, Sebastian Andrzej Siewior wrote:
>> diff --git a/drivers/usb/gadget/f_loopback.c b/drivers/usb/gadget/f_loopback.c
>> index 3d103a2..2d5aade 100644
>> --- a/drivers/usb/gadget/f_loopback.c
>> +++ b/drivers/usb/gadget/f_loopback.c
>> @@ -44,9 +45,17 @@ static inline struct f_loopback *func_to_loop(struct usb_function *f)
>> return container_of(f, struct f_loopback, function);
>> }
>>
>> -static unsigned qlen = 32;
>> -module_param(qlen, uint, 0);
>> -MODULE_PARM_DESC(qlenn, "depth of loopback queue");
>> +static unsigned qlen;
>> +static unsigned buflen;
>> +
>> +void lb_set_options(struct usb_function *f, struct usb_zero_options *lb_opt)
>> +{
>> + buflen = lb_opt->bulk_buflen;
>> + qlen = lb_opt->qlen;
>> + if (!qlen)
>> + qlen = 32;
>> +}
>> +EXPORT_SYMBOL_GPL(lb_set_options);
>
>now this looks awfully wrong. Why don't you just expose the modules to
>userland (in fact they already are) and if user wants to change
>something it needs to do it via sysfs before enabling the function ?
This isn't that easy. buflen & qlen here are used for memory allocation.
A change here would require to purge all usb_requests and re-allocate
them. This could be done but the device shouldn't be active while doing
so I think. However you could argue here and say "the user knows what he
does" and hold a lock while the queue is purged and re-created.
Now while this looks "simple" we have the little more complicated case
in sourcesink where changing options requires changing USB descriptors.
So from that angle we should have a "ready/active" sysfile which enables
before it can be used by the host.
No matter how far you go, you remove the qlen und buflen module
parameter from the module which changes currennt behavior and the
question will people be fine with this. Maybe nobody cares about g_zero
but we have other gadgetsâ?Š
>would that work ?
Assume you get through with removal of module paramters. Do we end up
with two interfaces then? One for configfs and one for sysfs?
Sebastian
--
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
