From: Hans Verkuil <hverkuil@xs4all.nl>
To: Dean Anderson <linux-dev@sensoray.com>
Cc: linux-media@vger.kernel.org
Subject: Re: [PATCH] s2255drv: port to videobuf2
Date: Tue, 04 Feb 2014 11:04:46 +0100 [thread overview]
Message-ID: <52F0BB3E.2060601@xs4all.nl> (raw)
In-Reply-To: <4019df5ff7ddbc7945122a7a571ed57b@sensoray.com>
Hi Dean,
On 02/03/14 18:06, Dean Anderson wrote:
> On 2014-02-03 03:51, Hans Verkuil wrote:
>> Hi Dean,
>>
>> Some specific comments below, but first two general comments:
>>
>> It is easier to review if at least the removal of the old s2255_fh struct
>> was done as a separate patch. It's always good to try and keep the changes
>> in patches as small as possible. The actual vb2 conversion is always a
>> 'big bang' patch, that's unavoidable, but it's easier if it isn't mixed in
>> with other changes that are not directly related to the vb2 conversion.
>
>
> I figured removal of s2255_fh was a natural part of the videobuf2 conversion process, but I can break it up.
It's more like the first phase of a vb2 conversion. It really is wrong
for videobuf as well, so it makes sense to do that first.
> I also did change some formatting and naming changes (s2255_channel to s2255_vc) that can be postponed.
Just put it in a separate patch either before or after the patch that does
the vb2 conversion.
>
>>
>> And did you also run the v4l2-compliance utility for this driver? That's
>> useful to check that everything it still correct.
>
> Thanks for the comments. I'll do a v2 soon with v4l2-compliance fully tested too.
Rather than the standard v4l2-compliance from v4l-utils, can you use this
from my own tree:
http://git.linuxtv.org/hverkuil/v4l-utils.git/shortlog/refs/heads/streaming
I've started work to add tests for streaming to v4l2-compliance. While not
complete it should cover what the s2255 driver needs. I'm very interested
in what it finds (or, as the case might be, what it doesn't find).
In order to do the streaming tests you have to run it with option -s.
Regards,
Hans
next prev parent reply other threads:[~2014-02-04 10:08 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-01-31 17:35 [PATCH] s2255drv: port to videobuf2 Dean Anderson
2014-02-03 9:51 ` Hans Verkuil
2014-02-03 17:06 ` Dean Anderson
2014-02-04 10:04 ` Hans Verkuil [this message]
2014-02-04 19:09 ` Dean Anderson
2014-02-05 21:44 ` Dean Anderson
2014-02-05 22:10 ` Hans Verkuil
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=52F0BB3E.2060601@xs4all.nl \
--to=hverkuil@xs4all.nl \
--cc=linux-dev@sensoray.com \
--cc=linux-media@vger.kernel.org \
/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