linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Savoy, Pavan" <pavan_savoy@ti.com>
To: Ohad Ben-Cohen <ohad@wizery.com>
Cc: "mchehab@infradead.org" <mchehab@infradead.org>,
	"hverkuil@xs4all.nl" <hverkuil@xs4all.nl>,
	"Halli, Manjunatha" <manjunatha_halli@ti.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-media@vger.kernel.org" <linux-media@vger.kernel.org>,
	Greg KH <greg@kroah.com>
Subject: RE: [PATCH v4 0/6] FM V4L2 drivers for WL128x
Date: Thu, 18 Nov 2010 15:56:25 +0530	[thread overview]
Message-ID: <19F8576C6E063C45BE387C64729E739404BC76239C@dbde02.ent.ti.com> (raw)
In-Reply-To: <AANLkTimE13JNS2gPhCpdZZk0ANyTgfxeO75pABrKLQDR@mail.gmail.com>

Ohad,
 
> -----Original Message-----
> From: Ohad Ben-Cohen [mailto:ohad@wizery.com]
> Sent: Thursday, November 18, 2010 2:36 AM
> To: Savoy, Pavan
> Cc: mchehab@infradead.org; hverkuil@xs4all.nl; Halli, Manjunatha; linux-
> kernel@vger.kernel.org; linux-media@vger.kernel.org; Greg KH
> Subject: Re: [PATCH v4 0/6] FM V4L2 drivers for WL128x
> 
> Hi Pavan,
> 
> > On Wed, Nov 17, 2010 at 6:32 PM, Ohad Ben-Cohen <ohad@wizery.com> wrote:
> >>>  drivers/staging/ti-st/Kconfig        |   10 +
> >>>  drivers/staging/ti-st/Makefile       |    2 +
> >>>  drivers/staging/ti-st/fmdrv.h        |  259 ++++
> >>>  drivers/staging/ti-st/fmdrv_common.c | 2141
> ++++++++++++++++++++++++++++++++++
> >>>  drivers/staging/ti-st/fmdrv_common.h |  458 ++++++++
> >>>  drivers/staging/ti-st/fmdrv_rx.c     |  979 ++++++++++++++++
> >>>  drivers/staging/ti-st/fmdrv_rx.h     |   59 +
> >>>  drivers/staging/ti-st/fmdrv_tx.c     |  461 ++++++++
> >>>  drivers/staging/ti-st/fmdrv_tx.h     |   37 +
> >>>  drivers/staging/ti-st/fmdrv_v4l2.c   |  757 ++++++++++++
> >>>  drivers/staging/ti-st/fmdrv_v4l2.h   |   32 +
> >>>  11 files changed, 5195 insertions(+), 0 deletions(-)
> >>
> >> Usually when a driver is added to staging, it should also have a TODO
> >> file specifying what needs to be done before the driver can be taken
> >> out of staging (at least as far as the author knows of today).
> >>
> >> It helps keeping track of the open issues in the driver, which is good
> >> for everyone - the author, the random contributor/observer, and
> >> probably even the staging maintainer.
> >>
> >> Can you please add such a TODO file ?
> ...
> > Thanks Ohad, for the comments, We do have an internal TODO.
> > In terms of functionality we have stuff like TX RDS which already has
> > few CIDs in the extended controls.
> > extend V4L2 for complete-scan, add in stop search during hw_seek .. etc...
> 
> You need to understand and list the reasons why this driver cannot go
> directly to mainline; missing functionality is usually not the
> culprit.

I don't.
I always doubt I would get all the answers from the maintainers if I keep posting 10 files with 200 lines of code in each of them every time.

> > But I just wanted to ask whether this is good enough to be staged.
> > Because as we begin to implement and add in the items in TODO - the
> > patch set will keep continuing to grow.
> >
> > So Hans, Mauro, What do you think ?
> > It would be real helpful - if this can be staged, it is becoming
> > difficult to maintain for us.
> 
> Greg has mentioned that staging acceptance rules are:
> 
> 1. Code compiles
> 2. Is self sustained (does not touch code out of staging)
> 3. Has a clear roadmap out of staging (that TODO file)
> 4. Is maintained
> 
> But I really think you should always prefer to upstream your code
> directly to mainline. Submit the code, have it reviewed by the
> relevant maintainers and upstream developers, and fix it appropriately
> until it is accepted.

Yes, I would love to do that too, however this is the problem we have with
submission of functionally completed code.

> Only if you feel (/know) it would take substantial cleanup/redesign
> efforts until it is accepted upstream, then staging is indeed the way
> to go. But then you should know what gates it from upstream merger,
> and focus on that (rather than on adding functionality) until it is
> taken out of staging. IMHO adding functionality will just make it
> harder for you to take it out of staging eventually. Usually the
> opposite road is taken: first get a minimal driver accepted upstream,
> and then gradually add the missing functionality.

Yes, hence inputs from Hans, Mauro and you too are welcome in suggesting as
To whether it is good enough for staging, and once staged, you are also
Welcome to add on to list of TODOs which we will take care :)

Thanks,
Pavan

> Good luck,
> Ohad.

      reply	other threads:[~2010-11-18 10:26 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-11-16 13:18 [PATCH v4 0/6] FM V4L2 drivers for WL128x manjunatha_halli
2010-11-16 13:18 ` [PATCH v4 1/6] drivers:staging: ti-st: fmdrv common header file manjunatha_halli
2010-11-16 13:18   ` [PATCH v4 2/6] drivers:staging: ti-st: fmdrv_v4l2 sources manjunatha_halli
2010-11-16 13:18     ` [PATCH v4 3/6] drivers:staging: ti-st: fmdrv_common sources manjunatha_halli
2010-11-16 13:18       ` [PATCH v4 4/6] drivers:staging: ti-st: fmdrv_rx sources manjunatha_halli
2010-11-16 13:18         ` [PATCH v4 5/6] drivers:staging: ti-st: fmdrv_tx sources manjunatha_halli
2010-11-16 13:18           ` [PATCH v4 6/6] drivers:staging: ti-st: Kconfig & Makefile change manjunatha_halli
2010-11-18  8:34         ` [PATCH v4 4/6] drivers:staging: ti-st: fmdrv_rx sources Hans Verkuil
2010-11-18  8:19       ` [PATCH v4 3/6] drivers:staging: ti-st: fmdrv_common sources Hans Verkuil
2010-12-10  6:18         ` halli manjunatha
2010-11-17  1:24     ` [PATCH v4 2/6] drivers:staging: ti-st: fmdrv_v4l2 sources Figo.zhang
2010-11-18  7:49     ` Hans Verkuil
2010-11-18  7:44   ` [PATCH v4 1/6] drivers:staging: ti-st: fmdrv common header file Hans Verkuil
2010-11-17 13:02 ` [PATCH v4 0/6] FM V4L2 drivers for WL128x Ohad Ben-Cohen
2010-11-18  5:54   ` Pavan Savoy
2010-11-18  7:17     ` Hans Verkuil
2010-11-18  8:35     ` Ohad Ben-Cohen
2010-11-18 10:26       ` Savoy, Pavan [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=19F8576C6E063C45BE387C64729E739404BC76239C@dbde02.ent.ti.com \
    --to=pavan_savoy@ti.com \
    --cc=greg@kroah.com \
    --cc=hverkuil@xs4all.nl \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=manjunatha_halli@ti.com \
    --cc=mchehab@infradead.org \
    --cc=ohad@wizery.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).