All of lore.kernel.org
 help / color / mirror / Atom feed
From: Hans Verkuil <hverkuil@xs4all.nl>
To: Andrey Utkin <andrey.utkin@corp.bluecherry.net>,
	Linux Media <linux-media@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"kernel-mentors@selenic.com" <kernel-mentors@selenic.com>,
	devel@driverdev.osuosl.org,
	kernel-janitors <kernel-janitors@vger.kernel.org>,
	Mauro Carvalho Chehab <mchehab@osg.samsung.com>,
	Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Andrey Utkin <andrey.od.utkin@gmail.com>
Subject: Re: [RFC PATCH v0] Add tw5864 driver
Date: Mon, 11 Jan 2016 10:52:25 +0000	[thread overview]
Message-ID: <56938969.30104@xs4all.nl> (raw)
In-Reply-To: <1451785302-3173-1-git-send-email-andrey.utkin@corp.bluecherry.net>

Hi Andrey,

On 01/03/2016 02:41 AM, Andrey Utkin wrote:
> (Disclaimer up to scissors mark)
> 
> Please be so kind to take a look at a new driver.
> We aim to follow high standards of kernel development and possibly to get this driver in mainline kernel.
> The device is multichannel video and audio capture and compression chip TW5864 of Intersil/Techwell.
> 
> The code is in repo https://github.com/bluecherrydvr/linux , branch "tw5864", subdir drivers/staging/media/tw5864 .
> This branch is regularly rebased during development, so that there's a single commit adding this driver on top of current linux-next.
> Direct link to subdir: https://github.com/bluecherrydvr/linux/tree/tw5864/drivers/staging/media/tw5864
> 
> 
> Implementation status
> 
> * H.264 streaming work stable, including concurrent work of multiple channels on same chip;
> * Audio streaming functionality is not implemented, but is considered for future;
> * The chip has motion detection capability, but of same sensitivity level for whole frame; this was considered quite limiting for our needs and we have implemented per-grid-cell sensitivity with a bit of heuristic processing of motion vector data exposed by hardware. Datasheet-suggested mechanism is not used currently.
> 
> Testing status
> 
> * Runtime tests on my test machine show that video streaming works stable. Multichannel streaming was working, but I haven't test this with latest revision lately yet.
> * Runtime performance will be tested later also on few early-adopters' CCTV machines.
> * checkpatch.pl -f on files reports no warnings, errors or style issues;
> * checkpatch.pl on patch reports no warnings, errors or style issues;
> * sparse reports nothing;
> * compilation shows no warnings (gcc 4.9.3);
> * compilation with EXTRA_CFLAGS=-W shows a lot of warnings from included headers (over 9000 lines of output). Seems this technique from Documentation/SubmitChecklist is not practical in this case
> * Other Documentation/SubmitChecklist advises weren't thoroughly worked out.

Did you also test with v4l2-compliance? Before I accept the driver I want to see the
output of 'v4l2-compliance' and 'v4l2-compliance -s'. Basically there shouldn't be
any failures.

I did a quick scan over the source and I saw nothing big. When you post the new
version I will go over it more carefully and do a proper review.

Regards,

	Hans

WARNING: multiple messages have this Message-ID (diff)
From: Hans Verkuil <hverkuil@xs4all.nl>
To: Andrey Utkin <andrey.utkin@corp.bluecherry.net>,
	Linux Media <linux-media@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"kernel-mentors@selenic.com" <kernel-mentors@selenic.com>,
	devel@driverdev.osuosl.org,
	kernel-janitors <kernel-janitors@vger.kernel.org>,
	Mauro Carvalho Chehab <mchehab@osg.samsung.com>,
	Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Andrey Utkin <andrey.od.utkin@gmail.com>
Subject: Re: [RFC PATCH v0] Add tw5864 driver
Date: Mon, 11 Jan 2016 11:52:25 +0100	[thread overview]
Message-ID: <56938969.30104@xs4all.nl> (raw)
In-Reply-To: <1451785302-3173-1-git-send-email-andrey.utkin@corp.bluecherry.net>

Hi Andrey,

On 01/03/2016 02:41 AM, Andrey Utkin wrote:
> (Disclaimer up to scissors mark)
> 
> Please be so kind to take a look at a new driver.
> We aim to follow high standards of kernel development and possibly to get this driver in mainline kernel.
> The device is multichannel video and audio capture and compression chip TW5864 of Intersil/Techwell.
> 
> The code is in repo https://github.com/bluecherrydvr/linux , branch "tw5864", subdir drivers/staging/media/tw5864 .
> This branch is regularly rebased during development, so that there's a single commit adding this driver on top of current linux-next.
> Direct link to subdir: https://github.com/bluecherrydvr/linux/tree/tw5864/drivers/staging/media/tw5864
> 
> 
> Implementation status
> 
> * H.264 streaming work stable, including concurrent work of multiple channels on same chip;
> * Audio streaming functionality is not implemented, but is considered for future;
> * The chip has motion detection capability, but of same sensitivity level for whole frame; this was considered quite limiting for our needs and we have implemented per-grid-cell sensitivity with a bit of heuristic processing of motion vector data exposed by hardware. Datasheet-suggested mechanism is not used currently.
> 
> Testing status
> 
> * Runtime tests on my test machine show that video streaming works stable. Multichannel streaming was working, but I haven't test this with latest revision lately yet.
> * Runtime performance will be tested later also on few early-adopters' CCTV machines.
> * checkpatch.pl -f on files reports no warnings, errors or style issues;
> * checkpatch.pl on patch reports no warnings, errors or style issues;
> * sparse reports nothing;
> * compilation shows no warnings (gcc 4.9.3);
> * compilation with EXTRA_CFLAGS=-W shows a lot of warnings from included headers (over 9000 lines of output). Seems this technique from Documentation/SubmitChecklist is not practical in this case
> * Other Documentation/SubmitChecklist advises weren't thoroughly worked out.

Did you also test with v4l2-compliance? Before I accept the driver I want to see the
output of 'v4l2-compliance' and 'v4l2-compliance -s'. Basically there shouldn't be
any failures.

I did a quick scan over the source and I saw nothing big. When you post the new
version I will go over it more carefully and do a proper review.

Regards,

	Hans

  parent reply	other threads:[~2016-01-11 10:52 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-01-03  1:41 [RFC PATCH v0] Add tw5864 driver Andrey Utkin
2016-01-03  3:47 ` Joe Perches
2016-01-03  3:47   ` Joe Perches
2016-01-03 13:34   ` Andrey Utkin
2016-01-03 13:34     ` Andrey Utkin
2016-01-03  5:38 ` Leon Romanovsky
2016-01-03  5:38   ` Leon Romanovsky
2016-01-11 10:52 ` Hans Verkuil [this message]
2016-01-11 10:52   ` Hans Verkuil
2016-01-15  2:13   ` Andrey Utkin
2016-01-15  2:13     ` Andrey Utkin
2016-02-08  9:58     ` Hans Verkuil
2016-02-08  9:58       ` Hans Verkuil
2016-02-08 10:23       ` Andrey Utkin
2016-02-08 10:23         ` Andrey Utkin
2016-02-08 10:29         ` Hans Verkuil
2016-02-08 10:29           ` Hans Verkuil
2016-03-09 14:29       ` Andrey Utkin
2016-03-09 14:29         ` Andrey Utkin
2016-03-11  8:00         ` Hans Verkuil
2016-03-11  8:00           ` Hans Verkuil
2016-03-11  8:40           ` Andrey Utkin
2016-03-11  8:40             ` Andrey Utkin
2016-03-11  9:59             ` Hans Verkuil
2016-03-11  9:59               ` Hans Verkuil
2016-03-11 13:23               ` Andrey Utkin
2016-03-11 13:23                 ` Andrey Utkin

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=56938969.30104@xs4all.nl \
    --to=hverkuil@xs4all.nl \
    --cc=andrey.od.utkin@gmail.com \
    --cc=andrey.utkin@corp.bluecherry.net \
    --cc=devel@driverdev.osuosl.org \
    --cc=ezequiel@vanguardiasur.com.ar \
    --cc=gregkh@linuxfoundation.org \
    --cc=kernel-janitors@vger.kernel.org \
    --cc=kernel-mentors@selenic.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=mchehab@osg.samsung.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.