Am 23.01.2013 19:33, schrieb Hans Verkuil: > On Wed January 23 2013 18:03:33 Frank Schäfer wrote: >> Am 23.01.2013 17:13, schrieb Hans Verkuil: >>> On Wed January 23 2013 16:57:40 Frank Schäfer wrote: >>>> Am 23.01.2013 08:35, schrieb Hans Verkuil: >>>>> On Tue January 22 2013 20:46:21 Frank Schäfer wrote: >>>>>> tuner_g_tuner() is supposed to fill struct v4l2_tuner passed by ioctl >>>>>> VIDIOC_G_TUNER, but misses setting the name field. >>>>>> >>>>>> Signed-off-by: Frank Schäfer <fschaefer.oss@xxxxxxxxxxxxxx> >>>>>> Cc: stable@xxxxxxxxxx >>>>>> --- >>>>>> drivers/media/v4l2-core/tuner-core.c | 1 + >>>>>> 1 Datei geändert, 1 Zeile hinzugefügt(+) >>>>>> >>>>>> diff --git a/drivers/media/v4l2-core/tuner-core.c b/drivers/media/v4l2-core/tuner-core.c >>>>>> index b5a819a..95a47cf 100644 >>>>>> --- a/drivers/media/v4l2-core/tuner-core.c >>>>>> +++ b/drivers/media/v4l2-core/tuner-core.c >>>>>> @@ -1187,6 +1187,7 @@ static int tuner_g_tuner(struct v4l2_subdev *sd, struct v4l2_tuner *vt) >>>>>> >>>>>> if (check_mode(t, vt->type) == -EINVAL) >>>>>> return 0; >>>>>> + strcpy(vt->name, t->name); >>>>>> if (vt->type == t->mode && analog_ops->get_afc) >>>>>> vt->afc = analog_ops->get_afc(&t->fe); >>>>>> if (analog_ops->has_signal) >>>>>> >>>>> Nacked-by: Hans Verkuil <hans.verkuil@xxxxxxxxx> >>>>> >>>>> And the reason is that the tuner field should be filled in by the bridge >>>>> driver. That's because you may have multiple tuners and it's only the >>>>> bridge driver that will know which tuner is which and what name to give >>>>> it. >>>> Hmmm... I don't understand. >>>> Isn't his a per-tuner (subdev) operation ? If a device has multiple >>>> tuners (subdevs) it is called for each of them. >>>> So how can the returned tuner name be wrong and why should the bridge >>>> driver know better than the subdevice itself which name is correct ? >>> The name that's filled in is exposed to userspace, so it should be something >>> meaningful and not some internal name. In the case of multiple tuners that >>> means that the name should be something like 'TV 1' or 'TV 2', where the name >>> matches a name (label) of a tuner input of the product. There is no way a >>> subdev driver can know that, only the bridge driver knows what product it is >>> and thus what the labels on the inputs are. >> Hmm... I assume there is a misunderstanding about what the tuner "name" >> means and the corresponding field in struct v42l_tuner should contain... >> With the patch, the returned tuner names are e.g. "Temic PAL (4002 >> FH5)", "LG PAL_BG+FM (TPI8PSB01D)" etc, which is what I expect / think >> is a correct tuner name. > That's the name of the tuner device. That's like reporting to the user > that the video input is named "saa7115 4-0021". That's not useful. It > *is* useful for us as developers, which is why such names are seen in the > kernel log. Not sure if the saa7115 driver could provide better input names, but I agree that letting the bridge driver set it's own names for inputs makes sense. But I would say for the tuner names, the situation is different. Looking at tuner-types.c, I can say that these names are exaclty what I expect to get with the VIDIOC_G_TUNER ioctrl. Btw, bridge drivers can still overwrite the tuner name they get from the subdevice. ;) And getting the full device name from the subdevice is definitely better than getting no device name at all. (I would also say it's somehow inconsistent). ... >> >>> ... >>> >>> The golden rule is that sub-devices should not assume anything about how they >>> are hooked up in the actual product. >> Yeah, sure, but I can't see that we make such an assumption here !? > In this case it is more in the opposite direction: a subdev internal name is > exposed to userspace. It doesn't matter where the name comes from, the important thing is that it matches what the API spec says / userspace expects. So if we can pass the name from the subdevice to userspace directly because it matches these requirements, why not ? I think overwriting names etc. retrieved via a subdevice call should be the exception, not the rule. I actually don't care too much about the content of the tuner name field. I can't remember having seen any applications displaying it. I'm just trying to understand the API and I have a pending patch (as part of an ioctl fixes series) for the em28xx drivers that is supposed to unify the tuner name handing for video/vbi and radio devices (in whatever direction). Regards, Frank > > Regards, > > Hans -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
