On 07/18/2014 12:55 AM, Hans Verkuil wrote:
> On 07/18/2014 12:45 AM, Mauro Carvalho Chehab wrote:
>> Em Fri, 27 Jun 2014 16:15:41 +0200
>> Hans Verkuil <hverkuil@xxxxxxxxx> escreveu:
>>
>>> From: Hans Verkuil <hans.verkuil@xxxxxxxxx>
>>>
>>> Sometimes dev->tuner_type is compared to UNSET, sometimes to TUNER_ABSENT,
>>> but these defines have different values.
>>>
>>> Standardize to TUNER_ABSENT.
>>
>> That patch looks wrong. UNSET has value -1, while TUNER_ABSENT has value 4.
>
> Well, yes. That's the whole problem. Both values were used to indicate an
> absent tuner, and the 'if's to test whether a tuner was there also checked
> against different values. I standardized here to TUNER_ABSENT, which is what
> tveeprom uses as well (not that this driver looks uses that information from
> tveeprom).
>
> Without this change you cannot correctly model a board without a tuner like
> the one that I'm adding since the logic is all over the place in this driver.
>
> tuner_type should either be a proper tuner or TUNER_ABSENT, but never UNSET.
>
> That's what this patch changes.
Note that in cx23885-cards.c all boards without a tuner set tuner_type to
TUNER_ABSENT. So it makes no sense for the code elsewhere to check against
UNSET. UNSET is never set for tuner_type.
As an aside: some of the board definition leave tuner_type at 0 when I
think it should be TUNER_ABSENT as well (or an actual proper tuner). It's
unclear what happens then, but clearly this patch won't change those boards
(e.g. CX23885_BOARD_MPX885 is one of them).
Regards,
Hans
>
> Regards,
>
> Hans
>
>> The only way that this patch won't be causing regressions is if none
>> was used, with is not the case, IMHO.
>>
>> A patch removing either one would be a way more complex, and should likely
>> touch on other cx23885 files:
>>
>> $ git grep -e UNSET --or -e TUNER_ABSENT -l drivers/media/pci/cx23885/
>> drivers/media/pci/cx23885/cx23885-417.c
>> drivers/media/pci/cx23885/cx23885-cards.c
>> drivers/media/pci/cx23885/cx23885-core.c
>> drivers/media/pci/cx23885/cx23885-video.c
>> drivers/media/pci/cx23885/cx23885.h
>>
>> and also on tveeprom.
>>
>> However, touching at tveeprom would require touching also on all
>> other drivers that support Hauppauge devices.
>>
>> Regards,
>> Mauro
>>
>>>
>>> Signed-off-by: Hans Verkuil <hans.verkuil@xxxxxxxxx>
>>> ---
>>> drivers/media/pci/cx23885/cx23885-417.c | 8 ++++----
>>> drivers/media/pci/cx23885/cx23885-video.c | 10 +++++-----
>>> 2 files changed, 9 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/drivers/media/pci/cx23885/cx23885-417.c b/drivers/media/pci/cx23885/cx23885-417.c
>>> index 95666ee..bf89fc8 100644
>>> --- a/drivers/media/pci/cx23885/cx23885-417.c
>>> +++ b/drivers/media/pci/cx23885/cx23885-417.c
>>> @@ -1266,7 +1266,7 @@ static int vidioc_g_tuner(struct file *file, void *priv,
>>> struct cx23885_fh *fh = file->private_data;
>>> struct cx23885_dev *dev = fh->dev;
>>>
>>> - if (UNSET == dev->tuner_type)
>>> + if (dev->tuner_type == TUNER_ABSENT)
>>> return -EINVAL;
>>> if (0 != t->index)
>>> return -EINVAL;
>>> @@ -1284,7 +1284,7 @@ static int vidioc_s_tuner(struct file *file, void *priv,
>>> struct cx23885_fh *fh = file->private_data;
>>> struct cx23885_dev *dev = fh->dev;
>>>
>>> - if (UNSET == dev->tuner_type)
>>> + if (dev->tuner_type == TUNER_ABSENT)
>>> return -EINVAL;
>>>
>>> /* Update the A/V core */
>>> @@ -1299,7 +1299,7 @@ static int vidioc_g_frequency(struct file *file, void *priv,
>>> struct cx23885_fh *fh = file->private_data;
>>> struct cx23885_dev *dev = fh->dev;
>>>
>>> - if (UNSET == dev->tuner_type)
>>> + if (dev->tuner_type == TUNER_ABSENT)
>>> return -EINVAL;
>>> f->type = V4L2_TUNER_ANALOG_TV;
>>> f->frequency = dev->freq;
>>> @@ -1347,7 +1347,7 @@ static int vidioc_querycap(struct file *file, void *priv,
>>> V4L2_CAP_READWRITE |
>>> V4L2_CAP_STREAMING |
>>> 0;
>>> - if (UNSET != dev->tuner_type)
>>> + if (dev->tuner_type != TUNER_ABSENT)
>>> cap->capabilities |= V4L2_CAP_TUNER;
>>>
>>> return 0;
>>> diff --git a/drivers/media/pci/cx23885/cx23885-video.c b/drivers/media/pci/cx23885/cx23885-video.c
>>> index e0a5952..2a890e9 100644
>>> --- a/drivers/media/pci/cx23885/cx23885-video.c
>>> +++ b/drivers/media/pci/cx23885/cx23885-video.c
>>> @@ -1156,7 +1156,7 @@ static int vidioc_querycap(struct file *file, void *priv,
>>> V4L2_CAP_READWRITE |
>>> V4L2_CAP_STREAMING |
>>> V4L2_CAP_VBI_CAPTURE;
>>> - if (UNSET != dev->tuner_type)
>>> + if (dev->tuner_type != TUNER_ABSENT)
>>> cap->capabilities |= V4L2_CAP_TUNER;
>>> return 0;
>>> }
>>> @@ -1474,7 +1474,7 @@ static int vidioc_g_tuner(struct file *file, void *priv,
>>> {
>>> struct cx23885_dev *dev = ((struct cx23885_fh *)priv)->dev;
>>>
>>> - if (unlikely(UNSET == dev->tuner_type))
>>> + if (dev->tuner_type == TUNER_ABSENT)
>>> return -EINVAL;
>>> if (0 != t->index)
>>> return -EINVAL;
>>> @@ -1490,7 +1490,7 @@ static int vidioc_s_tuner(struct file *file, void *priv,
>>> {
>>> struct cx23885_dev *dev = ((struct cx23885_fh *)priv)->dev;
>>>
>>> - if (UNSET == dev->tuner_type)
>>> + if (dev->tuner_type == TUNER_ABSENT)
>>> return -EINVAL;
>>> if (0 != t->index)
>>> return -EINVAL;
>>> @@ -1506,7 +1506,7 @@ static int vidioc_g_frequency(struct file *file, void *priv,
>>> struct cx23885_fh *fh = priv;
>>> struct cx23885_dev *dev = fh->dev;
>>>
>>> - if (unlikely(UNSET == dev->tuner_type))
>>> + if (dev->tuner_type == TUNER_ABSENT)
>>> return -EINVAL;
>>>
>>> /* f->type = fh->radio ? V4L2_TUNER_RADIO : V4L2_TUNER_ANALOG_TV; */
>>> @@ -1522,7 +1522,7 @@ static int cx23885_set_freq(struct cx23885_dev *dev, const struct v4l2_frequency
>>> {
>>> struct v4l2_control ctrl;
>>>
>>> - if (unlikely(UNSET == dev->tuner_type))
>>> + if (dev->tuner_type == TUNER_ABSENT)
>>> return -EINVAL;
>>> if (unlikely(f->tuner != 0))
>>> return -EINVAL;
>
> --
> 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
>
--
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
