Re: dvb_usb_v2: use pointers to properties[REGRESSION]

[Antti Palosaari <crope@iki.fi>]

I did example for you, here you are :)

On 06/16/2012 03:55 AM, Antti Palosaari wrote:
> On 06/16/2012 03:35 AM, Malcolm Priestley wrote:
>> On Sat, 2012-06-16 at 01:54 +0300, Antti Palosaari wrote:
>>> Hello Malcolm,
>>>
>>> On 06/16/2012 01:11 AM, Malcolm Priestley wrote:
>>>> Hi Antti
>>>>
>>>> You can't have dvb_usb_device_properties as constant structure pointer.
>>>>
>>>> At run time it needs to be copied to a private area.
>>>
>>> Having constant structure for properties was one of main idea of whole
>>> change. Earlier it causes some problems when driver changes those values
>>> - for example remote configuration based info from the eeprom.
>>>
>>>> Two or more devices of the same type on the system will be pointing to
>>>> the same structure.
>>>
>>> Yes and no. You can define struct dvb_usb_device_properties for each
>>> USB ID.
>>>
>>>> Any changes they make to the structure will be common to all.
>>>
>>> For those devices having same USB ID only.
>>> Changing dvb_usb_device_properties is *not* allowed. It is constant and
>>> should be. That was how I designed it. Due to that I introduced those
>>> new callbacks to resolve needed values dynamically.
>> Yes, but it does make run-time tweaks difficult.
>>
>>> If there is still something that is needed to resolve at runtime I am
>>> happy to add new callback. For example PID filter configuration is
>>> static currently as per adapter and if it is needed to to reconfigure at
>>> runtime new callback is needed.
>> I will look at the PID filter later, it defaulted to off.
>>
>> However, in my builds for ARM devices it is defaulted on. I will be
>> testing this later. I can't see any problems.
>>
>>>
>>> Could you say what is your problem I can likely say how to resolve it.
>>>
>>
>> Well, the problem is, I now need two separate structures for LME2510 and
>> LME2510C as in the existing driver, the hope was to merge them as one.
>> The only difference being the stream endpoint number.
>
> Then you should use .get_usb_stream_config() instead of static stream
> configuration. That is now supported.
>
> I have found one logical error in my current implementation.
> get_usb_stream_config gets frontend as a parameter, but it is called
> frontend == NULL when attaching adapters and after that frontend is real
> value when called (just before streaming is started). Now what happens
> if we has multiple adapters? We cannot know which adapter is requested
> during attach since FE is NULL :)
> But that is problem case only if you have multiple adapters. I will find
> out better solution next few days. It is ugly now.
> You can just skip fe checking or something. See AF9015 for example.
>
>> Currently, it is implemented in identify_state on dvb_usb_v2.
>>
>> The get_usb_stream_config has no access to device to to allow a run-time
>> change there.
>
> Hmmm, what you try to say? As FE is given as a pointer, you have also
> adapter and device. Those are nested, device is root, then adapter is
> under that and finally frontend are under the adapter.
>
>
> .
> └── device
>      ├── adapter0
>      │   ├── frontend0
>      │   ├── frontend1
>      │   └── frontend2
>      └── adapter1
>          ├── frontend0
>          ├── frontend1
>          └── frontend2
>
>
> regards
> Antti

static int lme2510_get_usb_stream_config(struct dvb_frontend *fe,
		struct usb_data_stream_properties *stream)
{
	struct dvb_usb_adapter *adap;
	struct lme2510_state *state;

	stream->type = USB_BULK;
	stream->count = 10;
	stream->u.bulk.buffersize = 4096;

// ugly part begins
	if (fe == NULL)
		return 0;
	adap = fe->dvb->priv;
	state = adap->dev->priv;
// ugly part ends

	if (state->chip_id == lme2510c)
		stream->endpoint = 8;
	else
		stream->endpoint = 6;

	return 0;
}

Ugly part between comments is what I am going to change, but currently 
it works as is.


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