Re: [RFCv1] DVB-USB improvements [alternative 2]

[Antti Palosaari <crope@iki.fi>]

On 21.05.2012 06:50, Mauro Carvalho Chehab wrote:
> Em 20-05-2012 17:55, Antti Palosaari escreveu:
>> I did some more planning and made alternative RFC.
>> As the earlier alternative was more like changing old functionality that new one goes much more deeper.
>>
>> As a basic rule I designed it to reduce stuff from the current struct dvb_usb_device_properties. Currently there is many nested structs introducing same callbacks. For that one I dropped all frontend and adapter level callbacks to device level. Currently struct contains 2 adapters and 3 frontends - which means we have 2 * 3 = 6 "similar" callbacks and only 1 is used. It wastes some space since devices having more than one adapter or frontend are rather rare. Making callback selection inside individual driver is very trivial even without a designated callback. Here is common example from the use of .frontend_attach() callback in case of only one callback used:
>> static int frontend_attach(struct dvb_usb_adapter *adap)
>> {
>>    if (adap->id == 0)
>>      return frontend_attach_1();
>>    else
>>      return frontend_attach_2();
>> }
>>
>> Functionality enhancement mentioned earlier RFC are valid too:
>> http://www.mail-archive.com/linux-media@vger.kernel.org/msg46352.html
>>
>> As I was a little bit lazy I wrote only quick skeleton code to represent new simplified "struct dvb_usb_device_properties":
>>
>> struct dvb_usb_device_properties = {
>>    /* download_firmware() success return values to signal what happens next */
>>    #define RECONNECTS_USB                  (1<<  0)
>>    #define RECONNECTS_USB_USING_NEW_ID     (1<<  1)
>>
>>   .size_of_priv = sizeof(struct 'state'),
>>
>>    /* firmware download */
>>    .identify_state(struct dvb_usb_device *d, int *cold),
>>    .get_firmware_name(struct dvb_usb_device *d, char *firmware_name),
>>    .download_firmware(struct dvb_usb_device *d, const struct firmware *fw),
>>    .allow_dynamic_id = true,
>>
>>    .power_ctrl(struct dvb_usb_device *d, int onoff),
>>    .read_config(struct dvb_usb_device *d, u8 mac[6]),
>>    .get_adapter_count(struct dvb_usb_device *d, int *count),
>>
>>    .frontend_attach(struct dvb_usb_adapter *adap),
>>    .tuner_attach(struct dvb_usb_adapter *adap),
>>
>>    .init(struct dvb_usb_device *d),
>>
>>    .get_rc(struct dvb_rc *),
>>    .i2c_algo = (struct i2c_algorithm),
>>
>>    .frontend_ctrl(struct dvb_frontend *fe, int onoff),
>>    .get_stream_props(struct usb_data_stream_properties *),
>>    .streaming_ctrl(struct dvb_usb_adapter *adap, int onoff),
>>
>>    .generic_bulk_ctrl_endpoint = (int),
>>    .generic_bulk_ctrl_endpoint_response = (int),
>>
>>    .devices = (struct dvb_usb_device)[],
>> };
>>
>> struct dvb_usb_device dvb_usb_devices {
>>    char *name = "name",
>>    .rc_map = RC_MAP_EMPTY,
>>    .device_id = (struct usb_device_id),
>> }
>
> It looks OK to me. It may make sense to add an optional
> per-device field, to allow drivers to add more board-specific
> information, if they need, in order to avoid duplicating
> things there.
>
> Another option would be for the drivers to do:
>
> struct dvb_usb_drive_foo dvb_usb_driver_foo {
> 	struct dvb_usb_device dvb_usb_devices dvb_usb_dev;
> 	int foo;
> 	long bar;
> 	...
> }
>
> And, inside the core, use the container_of() macro to go from
> the device-specific table to struct dvb_usb_device.
>
> This way, simple drivers can do just:
>
> struct dvb_usb_drive_foo dvb_usb_driver_foo {
> 	struct dvb_usb_device dvb_usb_devices dvb_usb_dev;
> }
>
> And complex drivers can add more stuff there.

I have now implemented some basic stuff. Most interesting is new way of 
map device id and properties for it. I found that I can use .driver_info 
field from the (struct usb_device_id) to carry pointer. I used it to 
carry all the other data to the DVB USB core. Thus that one big issue is 
now resolved. It reduces something like 8-9 kB of binary size which is 
huge improvement. Same will happen for every driver using multiple 
(struct dvb_usb_device_properties) - for more you are used more you save.

Here is 3 example drivers I have converted to that new style:
http://palosaari.fi/linux/v4l-dvb/dvb-usb-2012-05-25/

regards
Antti
-- 
http://palosaari.fi/