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