linux-media.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Hans Verkuil <hansverk@cisco.com>
To: Mauro Carvalho Chehab <mchehab@redhat.com>
Cc: Hans Verkuil <hans.verkuil@cisco.com>, linux-media@vger.kernel.org
Subject: Re: [RFCv1 API PATCH 4/4] tuner-core: map audmode to STEREO for radio devices.
Date: Tue, 25 Sep 2012 15:45:00 +0200	[thread overview]
Message-ID: <201209251545.00414.hansverk@cisco.com> (raw)
In-Reply-To: <20120925103340.76a5db3c@redhat.com>

On Tue 25 September 2012 15:33:40 Mauro Carvalho Chehab wrote:
> Em Fri, 14 Sep 2012 13:15:36 +0200
> Hans Verkuil <hans.verkuil@cisco.com> escreveu:
> 
> > Fixes a v4l2-compliance error: setting audmode to a value other than mono
> > or stereo for a radio device should map to MODE_STEREO.
> > 
> > Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> > ---
> >  drivers/media/v4l2-core/tuner-core.c |    5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/media/v4l2-core/tuner-core.c b/drivers/media/v4l2-core/tuner-core.c
> > index b5a819a..ea71371 100644
> > --- a/drivers/media/v4l2-core/tuner-core.c
> > +++ b/drivers/media/v4l2-core/tuner-core.c
> > @@ -1235,8 +1235,11 @@ static int tuner_s_tuner(struct v4l2_subdev *sd, struct v4l2_tuner *vt)
> >  	if (set_mode(t, vt->type))
> >  		return 0;
> >  
> > -	if (t->mode == V4L2_TUNER_RADIO)
> > +	if (t->mode == V4L2_TUNER_RADIO) {
> >  		t->audmode = vt->audmode;
> > +		if (t->audmode > V4L2_TUNER_MODE_STEREO)
> > +			t->audmode = V4L2_TUNER_MODE_STEREO;
> 
> NACK. It is not a core's task to fix driver's bugs. It would be ok to have here a
> WARN_ON(), but, if a driver is reporting a wrong radio audmode, the fix should be
> there at the drivers, and not here at the core.

tuner-core *is* the driver. A bridge driver just passes v4l2_tuner on to the
subdev driver(s), and it is the subdev driver such as tuner-core that needs to
process the audmode as specified by the user. Which in this case means mapping
audmodes that are invalid when in radio mode to something that is valid as per
the spec.

So this is a real tuner-core bug, not a bridge driver bug since they don't
generally touch the audmode field, they just pass it along. And the vt->audmode
value comes straight from userspace, not from the bridge driver.

Regards,

	Hans

  reply	other threads:[~2012-09-25 13:45 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-09-14 11:15 [RFCv1 API PATCH 1/4] Two fixes and two v4l2-ctrl enhancements Hans Verkuil
2012-09-14 11:15 ` [RFCv1 API PATCH 1/4] vb2: fix wrong owner check Hans Verkuil
2012-09-14 11:15   ` [RFCv1 API PATCH 2/4] v4l2-ctrls: add a notify callback Hans Verkuil
2012-09-26 10:50     ` Laurent Pinchart
2012-09-27  6:44       ` Hans Verkuil
2012-10-01 20:01         ` Mauro Carvalho Chehab
2012-10-02  6:36           ` Hans Verkuil
2012-09-14 11:15   ` [RFCv1 API PATCH 3/4] v4l2-ctrls: add a filter function to v4l2_ctrl_add_handler Hans Verkuil
2012-09-14 11:15   ` [RFCv1 API PATCH 4/4] tuner-core: map audmode to STEREO for radio devices Hans Verkuil
2012-09-25 13:33     ` Mauro Carvalho Chehab
2012-09-25 13:45       ` Hans Verkuil [this message]
2012-09-26  2:29         ` Mauro Carvalho Chehab
2012-09-26  7:03           ` Hans Verkuil
2012-09-26  9:35             ` Mauro Carvalho Chehab
2012-09-26  9:57               ` 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=201209251545.00414.hansverk@cisco.com \
    --to=hansverk@cisco.com \
    --cc=hans.verkuil@cisco.com \
    --cc=linux-media@vger.kernel.org \
    --cc=mchehab@redhat.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).