On Mon, 02 Aug 2004 00:09:35 +0200
Andreas Oberritter <obi@linuxtv.org> wrote:
> Am So, den 01.08.2004 schrieb Roberto Ragusa um 17:59:
> +
> + if ((ret = mt312_read(i2c, LNB_FREQ_H, &buf, sizeof(buf))) < 0)
> + return ret;
>
> this should probably be buf, not &buf, although the address of the
> pointer and the value of the pointer are the same for local arrays IIRC.
Right. But the same error occurs two times in similar function.
I copy&pasted errors too. :-)
I'll fix it.
> + else{ //Auto symbol rate
> + if ((ret = set_tv_freq(i2c, p->frequency, 20000000)) <
> 0) //20000000 as placeholder
> + return ret;
> +
>
> I don't understand how you specify the symbol rate range, but could you
> replace the 20000000 by the average value of the range?
That number is only used in the sl1935 code path, which I can't test.
The function should always set the frequency, but uses the symbol rate
to control something in the hardware in an optimal way.
I don't know what it is, don't have the datasheet, can't test it.
I suppose it is something not critical; in any case it would only affect
the auto sr search, so no regression.
Read my other reply to hear something about the ranges.
> I am glad that you corrected mt312_get_symbol_rate(). Can you please
> split the patch so I can commit this part first?
Done.
Attached two patch: the second one has trivial printk improvements.
> Then, because we surely want to submit all changes to the official
> kernel tree, it would be good to follow the kernel coding style, i.e.
> replace c++ comments by c comments etc. Please have a look at
> linux/Documentation/CodingStyle.
"//" is part of c and used in great quantity in the kernel (try a grep
in the dvb subdir); it is preferred for short comments because you
can then comment a big block with "/*"..."*/ without nesting comments.
I'll change my "if (a){" with "if (a) {", and split a couple of long lines.
Anything else?
Regards.
--
Roberto Ragusa mail at robertoragusa.it
Attachment:
mt312.c_get_sr.diff
Description: Binary data
Attachment:
mt312.c_trivial.diff
Description: Binary data