Re: [PULL] http://hg.jannau.net/v4l-dvb/

[Alan Nisota <alannisota@xxxxxxxxx>]

Mauro Carvalho Chehab wrote:
> On Fri, 19 Dec 2008 03:23:52 +0100
> Janne Grunau <janne-dvb@xxxxxxxxx> wrote:
>
>   
>> Hi,
>>
>> On Friday 19 December 2008 00:27:33 Mauro Carvalho Chehab wrote:
>>     
>>> Patrick/Alan,
>>>
>>> Could you please review this patch?
>>>
>>> From my side, I see a trouble on this code (also present at the original
>>> code):
>>>
>>> +	cmd[0] =  c->symbol_rate        & 0xff;
>>> +	cmd[1] = (c->symbol_rate >>  8) & 0xff;
>>> +	cmd[2] = (c->symbol_rate >> 16) & 0xff;
>>> +	cmd[3] = (c->symbol_rate >> 24) & 0xff;
>>>
>>> This code assumes Intel endiannes.
>>>       
>> Huh, why? It's stored in as big endian in cmd but the arithmetic operations 
>> (shifts and bitwise and) are not endianess dependent. Btw there is similar 
>> code for the frequency.
>>     
>
> Hmm... true. Yet, it seems to be better documented to use something like:
>
> cmd = cpu_to_be32(c->symbol_rate);
>
>   
I don't see the purpose in that as it ends up converting the character 
array into something that looks like a structure, and the API I got from 
the manufacturer didn't portray it that way.

>> There is a problem with the existing code though. cmd is allocated on the 
>> stack. Some arches can't use aarays from stack with USB transfers.
>>     
>
> Very true.
>
>   
Please tell me what you'd like me to do.
The patch I've provided is no worse that what we've been using for the 
past few years without issue.

I am unsure what the rules are with regards to USB transfers, and what 
the proper way to solve that issue is...do I need a vmalloc or a kmalloc 
or is there a better way?  It seems wasteful for me to need to solve a 
problem I don't understand when it is clear that there are folks here 
who do understand it, and can solve it more effectively than I can.

Regardless, I will do whatever you like to resolve these issues.  My 
primary purpose here is to get the code converted to the S2 API before 
the S2API is merged to MythTV so that my users are not left out in the 
cold, and we can get rid of the 'extended API' stuff in Myth as soon as 
possible.


_______________________________________________
v4l-dvb-maintainer mailing list
v4l-dvb-maintainer@xxxxxxxxxxx
http://www.linuxtv.org/cgi-bin/mailman/listinfo/v4l-dvb-maintainer