--- On Wed, 3/11/09, Uri Shkolnik <uri...@yahoo.com> wrote:
> From: Uri Shkolnik <uri...@yahoo.com>
> Subject: Re: [PULL] http://linuxtv.org/hg/~mkrufky/sms1xxx-gpio
> To: "Devin Heitmueller" <devin.heitmuel...@gmail.com>
> Cc: "Michael Krufky" <mkru...@linuxtv.org>, "Mauro Carvalho Chehab"
> <mche...@infradead.org>, linux-media@vger.kernel.org
> Date: Wednesday, March 11, 2009, 6:41 PM
>
>
>
> --- On Wed, 3/11/09, Devin Heitmueller <devin.heitmuel...@gmail.com>
> wrote:
>
> > From: Devin Heitmueller <devin.heitmuel...@gmail.com>
> > Subject: Re: [PULL] http://linuxtv.org/hg/~mkrufky/sms1xxx-gpio
> > To: "Uri Shkolnik" <uri...@yahoo.com>
> > Cc: "Michael Krufky" <mkru...@linuxtv.org>,
> "Mauro Carvalho Chehab" <mche...@infradead.org>,
> linux-media@vger.kernel.org
> > Date: Wednesday, March 11, 2009, 4:59 PM
> > Hello Uri,
> >
> > On Wed, Mar 11, 2009 at 10:19 AM, Uri Shkolnik <uri...@yahoo.com>
> > wrote:
> > > Answered above
> > >
> > >> #5 - Your patches break bisection tests. I
> broke
> > them
> > >> down to avoid this issue, but you prevented
> the
> > merge.
> > >>
> > >
> > > I don't know what are "bisection tests", please
> > elaborate.
> >
> > I think this point above may be where the
> communication
> > breakdown is.
> > The Linux kernel development environment is a little
> > different than
> > traditional source control environments. Here is
> how
> > it works:
> >
> > No *individual* patch is permitted to cause
> regressions
> > with devices
> > that are currently supported in the kernel tree.
> This
> > means that if
> > you have a patch series of ten patches and during
> > development you
> > found out patch # 3 breaks some Hauppauge device, you
> > cannot just
> > provide the fix in patch # 8. You actually have to
> go
> > back and
> > provide a new version of patch # 3 which does not have
> the
> > regression.
> >
> > Within a patch series, *every* patch has to compile,
> work,
> > and not
> > introduce regressions. This is because of what is
> > called bisecting -
> > which is a debugging technique where kernel developers
> use
> > an
> > automated binary search to look for the source of
> > bugs. For example,
> > if you submit a patch that doesn't compile, and you
> provide
> > the fix in
> > the next patch, then bisection doesn't work because a
> > developer
> > (possibly that is doing development completely
> unrelated to
> > the v4l
> > project) cannot bisect with the first version since
> his
> > kernel tree
> > will not compile.
> >
> > I took a look at the patches, and some of them are
> *huge*
> > by kernel
> > standards. I see single patches that fix five or
> six
> > issues. These
> > need to be broken down into individual atomic patches,
> each
> > describing
> > in detail what the change does. This is what
> Michael
> > has been doing.
> >
> > It's a change in mindset in that you need to not just
> think
> > about the
> > end result (which I'm confident passes all your
> internal
> > tests), but
> > also each intermediate step along the way.
> >
> > Devin
> >
> > --
> > Devin J. Heitmueller
> > http://www.devinheitmueller.com
> > AIM: devinheitmueller
> > --
> > To unsubscribe from this list: send the line
> "unsubscribe
> > linux-media" in
> > the body of a message to majord...@vger.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> >
>
> Hi Devin,
>
> Thanks for the detailed explanation.
>
> After reviewing the entire chain of patches (10221...10239)
> I find that the entire chain, and each patch separately,
> suppose to pass bisection tests, since every such patch
> passed at least full sanity test at the time it had been
> submitted.
>
> If necessary, I can re-check it again (even I'm quite sure
> that I'll get the same results), or simpler, if there is a
> test that runs automatically over
> http://patchwork.kernel.org/project/linux-media/list ,
> it should submit some kind of report, no? how do I get that
> report?
>
> Uri
>
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe
> linux-media" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
Hi Mike,
I'll not embedded my answers this time, since the post got quite long and hard
to navigate in it.
1) Regarding GPIO - True you took Siano's code, but you modified it, and broke
it. Some examples - since various devices use different DMA mechanisms, which
require specific, aligned allocation of DMA buffers, we use such dynamic
allocations, which you converted to stack variables, and that fails many
non-USB devices.
You also implement send-and-forget option (with if/"wait") which is illegal by
Siano host-device protocol. It will probably work well with sporadic GPIO
operations (such as done with LEDs indications in HPG case), but it will fail
with more frequent usage. You can said that this is "HPG" only code, but there
is no such thing in reality, many of our clients take this code and patch it to
match their target hardware, and the favorite method of copy-and-paste always
prevails, and soon our FAE will start to get support request on that issue.
(!) Note the HPG devices work with the generic set-and-wait-for-reply, as coded
in the original patch, without any problem.
2) Regarding breaking the tree - the patches has been tested one by one. Which
patch break the build? I'll fix it ASAP.
3) Regarding diff'ing against Siano's repository, I have never asked for it, on
the contrary, I wrote in a previous posts chain, that after we'll finish to
merge everything and we'll have a full updated hg tree, I'll back-merge the
results to Siano's repository (by this I gave the hg repository priority over
Siano's, which I doubt has ever been given by many other commercial companies).
As I wrote before, since your patches break non-HPG SMS-based devices support,
I can not approve them.
Best Regards,
Uri
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
