Message-ID: <CAHFNz9JPbE_m1Xk5CeVdDp3yQmwCvTjpmstAfWHh5XtUmQRAvQ@mail.gmail.com>
Date:	Mon, 1 Jul 2013 23:44:12 +0530
From:	Manu Abraham <abraham.manu@...il.com>
To:	Mauro Carvalho Chehab <mchehab@...hat.com>
Cc:	Linus Torvalds <torvalds@...ux-foundation.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Linux Media Mailing List <linux-media@...r.kernel.org>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Zoran Turalija <zoran.turalija@...il.com>,
	Srinivas KANDAGATLA <srinivas.kandagatla@...com>,
	Nicolas THERY <nicolas.thery@...com>,
	Divneil Rai WADHAWAN <divneil.wadhawan@...com>,
	Vincent ABRIOU <vincent.abriou@...com>,
	Alain VOLMAT <alain.volmat@...com>
Subject: Re: [GIT PULL for v3.11] media patches for v3.11

On Mon, Jul 1, 2013 at 11:12 PM, Mauro Carvalho Chehab
<mchehab@...hat.com> wrote:
> Em Mon, 1 Jul 2013 21:17:58 +0530
> Manu Abraham <abraham.manu@...il.com> escreveu:
>
>> On Mon, Jul 1, 2013 at 9:05 PM, Mauro Carvalho Chehab
>> <mchehab@...hat.com> wrote:
>> > Em Mon, 1 Jul 2013 16:37:58 +0530
>> > Manu Abraham <abraham.manu@...il.com> escreveu:
>> >
>> >> Mauro,
>> >>
>> >> On Mon, Jul 1, 2013 at 4:28 PM, Mauro Carvalho Chehab
>> >> <mchehab@...hat.com> wrote:
>> >> > Hi Linus,
>> >> >
>> >> > Please pull from:
>> >> >   git://git.kernel.org/pub/scm/linux/kernel/git/mchehab/linux-media v4l_for_linus
>> >> >
>> >> > For the media patches for Kernel v3.11.
>> >> >
>> >>
>> >> >
>> >> > Zoran Turalija (2):
>> >> >       [media] stb0899: allow minimum symbol rate of 1000000
>> >> >       [media] stb0899: allow minimum symbol rate of 2000000
>> >>
>> >>
>> >> Somehow, I missed these patches; These are incorrect. Please revert
>> >> these changes.
>> >> Simply changing the advertized minima values don't change the search algorithm
>> >> behaviour, it simply leads to broken behaviour.
>> >>
>> >> NACK for these changes.
>> >
>> > While this patch came from a sub-maintainer's tree, looking at its
>> > history, the patch was proposed here:
>> >         https://linuxtv.org/patch/18341/
>> >
>>
>>
>> Wherever it came from, the patch is incorrect. Anyone can throw in
>> any patch as they want.
>>
>>
>> > From what it is said there, with this patch, 6 additional channels
>> > were discovered when using with Eutelsat 16A, that uses a symbol
>> > rate between 2MS/s to 5 MS/s. Without this patch, those channels won't
>> > be discovered, as the core won't try to use a symbol rate outside
>> > the range.
>>
>> What you are stating is a hit and miss scenario, sometimes it might lock
>> and sometimes it wouldn't.
>
> Well, before this change, it was a full miss scenario, as it will
> never lock on low symbol rate channels.
>
>> The scanning algorithm that I implemented for the demodulator works
>> with a symbol rate as low as 5 MSPS alone. Anything lower than that
>> is hit and miss.
>
> Ok, so latter patches need to improve the algorithm to improve it
> for lower symbol rates. By getting feedback about this patch, we'll
> know more about how bad is the current algorithm, as people may now
> report and work on improve it.
>
>> > Of course, transponders with a symbol rate equal or upper than 5MS/s
>> > won't be affected by this patch.
>> >
>>
>>
>> How can you be sure ? I myself am not very sure. While we worked on
>> the demodulator in the early days, we had different situations where a
>> previous failed state could cause lockup of the demodulator, eventually
>> resulting tuning failures.
>
> If that happens, that would be a firmware or hardware issue.
> As I said before, ST can provide us an answer if the hardware has
> such bug.
>
>> > Even if this is not a perfect patch and some changes would be
>> > needed to improve tuning for those low symbol rate transponders,
>> > it seems better than before, as at least now some channels are tuned.
>> >
>> > The only reason I can see to reverse this patch is that if setting
>> > the frontend to low bit ranges could damage the frontend or could
>> > hit some bug on the hardware (or internal firmware).
>> >
>> > Yet, from the datasheet pointed by the patch author, it seems that
>> > this frontend allows such low symbol rates:
>> >         http://comtech.sg1002.myweb.hinet.net/pdf/dvbs2-6899.pdf
>>
>>
>> The frontend allows a different lower symbol rate with a different
>> scanning algorithm, not with this existing current one.
>>
>> I am pretty sure, that author saw some specifications written some
>> place and simply copied those numbers in here. Also sure that he
>> has no idea about the algorithm in use.
>>
>> According to ST itself, a 2MSPS algorithm was created for a very
>> specific customer requirement, which is not applicable to the existing
>> algorithm in use with the Linux STB0899 demodulator driver.
>
> Ok, so let's wait for ST to provide us some feedback on this public
> thread, in order to be sure that we need to reverse it because of
> some hardware bug, or to get an improved algorithm that will better
> work with low symbol rates.

I don't think anyone is going to spend enough time to answer your
stupid comments.

As being the author of this driver, and the person who added support
for cards based on the chipset: NACK

Linus,

I NACK the patches for the said reasons.


Regards,

Manu



Regards,

Manu
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists