* [PATCH 0/2] cx23885: add support for Hauppauge ImpactVCB-e
@ 2014-06-27 14:15 Hans Verkuil
2014-06-27 14:15 ` [PATCH 1/2] cx23885: fix UNSET/TUNER_ABSENT confusion Hans Verkuil
2014-06-27 14:15 ` [PATCH 2/2] cx23885: add support for Hauppauge ImpactVCB-e Hans Verkuil
0 siblings, 2 replies; 7+ messages in thread
From: Hans Verkuil @ 2014-06-27 14:15 UTC (permalink / raw)
To: linux-media; +Cc: it, =stoth
This has been sitting in my queue for some time now. I've cleaned it
up and it should be ready for 3.17.
The main problem was related to inconsistent use of UNSET and
TUNER_ABSENT to denote an absent tuner (as is the case of this new
Hauppauge device). So the first patch cleans that up first.
Regards,
Hans
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/2] cx23885: fix UNSET/TUNER_ABSENT confusion.
2014-06-27 14:15 [PATCH 0/2] cx23885: add support for Hauppauge ImpactVCB-e Hans Verkuil
@ 2014-06-27 14:15 ` Hans Verkuil
2014-07-17 22:45 ` Mauro Carvalho Chehab
2014-06-27 14:15 ` [PATCH 2/2] cx23885: add support for Hauppauge ImpactVCB-e Hans Verkuil
1 sibling, 1 reply; 7+ messages in thread
From: Hans Verkuil @ 2014-06-27 14:15 UTC (permalink / raw)
To: linux-media; +Cc: it, =stoth, Hans Verkuil
From: Hans Verkuil <hans.verkuil@cisco.com>
Sometimes dev->tuner_type is compared to UNSET, sometimes to TUNER_ABSENT,
but these defines have different values.
Standardize to TUNER_ABSENT.
Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
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;
--
2.0.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/2] cx23885: add support for Hauppauge ImpactVCB-e
2014-06-27 14:15 [PATCH 0/2] cx23885: add support for Hauppauge ImpactVCB-e Hans Verkuil
2014-06-27 14:15 ` [PATCH 1/2] cx23885: fix UNSET/TUNER_ABSENT confusion Hans Verkuil
@ 2014-06-27 14:15 ` Hans Verkuil
1 sibling, 0 replies; 7+ messages in thread
From: Hans Verkuil @ 2014-06-27 14:15 UTC (permalink / raw)
To: linux-media; +Cc: it, =stoth, Hans Verkuil
From: Hans Verkuil <hans.verkuil@cisco.com>
Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
drivers/media/pci/cx23885/cx23885-cards.c | 31 ++++++++++++++++++++++++++++++-
drivers/media/pci/cx23885/cx23885-video.c | 1 +
drivers/media/pci/cx23885/cx23885.h | 1 +
3 files changed, 32 insertions(+), 1 deletion(-)
diff --git a/drivers/media/pci/cx23885/cx23885-cards.c b/drivers/media/pci/cx23885/cx23885-cards.c
index 79f20c8..9723067 100644
--- a/drivers/media/pci/cx23885/cx23885-cards.c
+++ b/drivers/media/pci/cx23885/cx23885-cards.c
@@ -649,7 +649,26 @@ struct cx23885_board cx23885_boards[] = {
CX25840_NONE1_CH3,
.amux = CX25840_AUDIO6,
} },
- }
+ },
+ [CX23885_BOARD_HAUPPAUGE_IMPACTVCBE] = {
+ .name = "Hauppauge ImpactVCB-e",
+ .tuner_type = TUNER_ABSENT,
+ .porta = CX23885_ANALOG_VIDEO,
+ .input = {{
+ .type = CX23885_VMUX_COMPOSITE1,
+ .vmux = CX25840_VIN7_CH3 |
+ CX25840_VIN4_CH2 |
+ CX25840_VIN6_CH1,
+ .amux = CX25840_AUDIO7,
+ }, {
+ .type = CX23885_VMUX_SVIDEO,
+ .vmux = CX25840_VIN7_CH3 |
+ CX25840_VIN4_CH2 |
+ CX25840_VIN8_CH1 |
+ CX25840_SVIDEO_ON,
+ .amux = CX25840_AUDIO7,
+ } },
+ },
};
const unsigned int cx23885_bcount = ARRAY_SIZE(cx23885_boards);
@@ -897,6 +916,10 @@ struct cx23885_subid cx23885_subids[] = {
.subvendor = 0x1461,
.subdevice = 0xd939,
.card = CX23885_BOARD_AVERMEDIA_HC81R,
+ }, {
+ .subvendor = 0x0070,
+ .subdevice = 0x7133,
+ .card = CX23885_BOARD_HAUPPAUGE_IMPACTVCBE,
},
};
const unsigned int cx23885_idcount = ARRAY_SIZE(cx23885_subids);
@@ -977,6 +1000,9 @@ static void hauppauge_eeprom(struct cx23885_dev *dev, u8 *eeprom_data)
case 71009:
/* WinTV-HVR1200 (PCIe, Retail, full height)
* DVB-T and basic analog */
+ case 71100:
+ /* WinTV-ImpactVCB-e (PCIe, Retail, half height)
+ * Basic analog */
case 71359:
/* WinTV-HVR1200 (PCIe, OEM, half height)
* DVB-T and basic analog */
@@ -1701,6 +1727,7 @@ void cx23885_card_setup(struct cx23885_dev *dev)
case CX23885_BOARD_HAUPPAUGE_HVR1850:
case CX23885_BOARD_HAUPPAUGE_HVR1290:
case CX23885_BOARD_HAUPPAUGE_HVR4400:
+ case CX23885_BOARD_HAUPPAUGE_IMPACTVCBE:
if (dev->i2c_bus[0].i2c_rc == 0)
hauppauge_eeprom(dev, eeprom+0xc0);
break;
@@ -1807,6 +1834,7 @@ void cx23885_card_setup(struct cx23885_dev *dev)
case CX23885_BOARD_HAUPPAUGE_HVR1200:
case CX23885_BOARD_HAUPPAUGE_HVR1700:
case CX23885_BOARD_HAUPPAUGE_HVR1400:
+ case CX23885_BOARD_HAUPPAUGE_IMPACTVCBE:
case CX23885_BOARD_LEADTEK_WINFAST_PXDVR3200_H:
case CX23885_BOARD_LEADTEK_WINFAST_PXPVR2200:
case CX23885_BOARD_LEADTEK_WINFAST_PXDVR3200_H_XC4000:
@@ -1835,6 +1863,7 @@ void cx23885_card_setup(struct cx23885_dev *dev)
break;
case CX23885_BOARD_HAUPPAUGE_HVR1250:
case CX23885_BOARD_HAUPPAUGE_HVR1800:
+ case CX23885_BOARD_HAUPPAUGE_IMPACTVCBE:
case CX23885_BOARD_HAUPPAUGE_HVR1800lp:
case CX23885_BOARD_HAUPPAUGE_HVR1700:
case CX23885_BOARD_LEADTEK_WINFAST_PXDVR3200_H:
diff --git a/drivers/media/pci/cx23885/cx23885-video.c b/drivers/media/pci/cx23885/cx23885-video.c
index 2a890e9..91e4cb4 100644
--- a/drivers/media/pci/cx23885/cx23885-video.c
+++ b/drivers/media/pci/cx23885/cx23885-video.c
@@ -507,6 +507,7 @@ static int cx23885_video_mux(struct cx23885_dev *dev, unsigned int input)
if ((dev->board == CX23885_BOARD_HAUPPAUGE_HVR1800) ||
(dev->board == CX23885_BOARD_MPX885) ||
(dev->board == CX23885_BOARD_HAUPPAUGE_HVR1250) ||
+ (dev->board == CX23885_BOARD_HAUPPAUGE_IMPACTVCBE) ||
(dev->board == CX23885_BOARD_HAUPPAUGE_HVR1255) ||
(dev->board == CX23885_BOARD_HAUPPAUGE_HVR1255_22111) ||
(dev->board == CX23885_BOARD_HAUPPAUGE_HVR1850) ||
diff --git a/drivers/media/pci/cx23885/cx23885.h b/drivers/media/pci/cx23885/cx23885.h
index 0fa4048..6a4b20e 100644
--- a/drivers/media/pci/cx23885/cx23885.h
+++ b/drivers/media/pci/cx23885/cx23885.h
@@ -96,6 +96,7 @@
#define CX23885_BOARD_TBS_6981 40
#define CX23885_BOARD_TBS_6980 41
#define CX23885_BOARD_LEADTEK_WINFAST_PXPVR2200 42
+#define CX23885_BOARD_HAUPPAUGE_IMPACTVCBE 43
#define GPIO_0 0x00000001
#define GPIO_1 0x00000002
--
2.0.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] cx23885: fix UNSET/TUNER_ABSENT confusion.
2014-06-27 14:15 ` [PATCH 1/2] cx23885: fix UNSET/TUNER_ABSENT confusion Hans Verkuil
@ 2014-07-17 22:45 ` Mauro Carvalho Chehab
2014-07-17 22:55 ` Hans Verkuil
0 siblings, 1 reply; 7+ messages in thread
From: Mauro Carvalho Chehab @ 2014-07-17 22:45 UTC (permalink / raw)
To: Hans Verkuil; +Cc: linux-media, it, =stoth, Hans Verkuil
Em Fri, 27 Jun 2014 16:15:41 +0200
Hans Verkuil <hverkuil@xs4all.nl> escreveu:
> From: Hans Verkuil <hans.verkuil@cisco.com>
>
> 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.
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@cisco.com>
> ---
> 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;
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] cx23885: fix UNSET/TUNER_ABSENT confusion.
2014-07-17 22:45 ` Mauro Carvalho Chehab
@ 2014-07-17 22:55 ` Hans Verkuil
2014-07-18 5:21 ` Hans Verkuil
0 siblings, 1 reply; 7+ messages in thread
From: Hans Verkuil @ 2014-07-17 22:55 UTC (permalink / raw)
To: Mauro Carvalho Chehab; +Cc: linux-media, it, =stoth, Hans Verkuil
On 07/18/2014 12:45 AM, Mauro Carvalho Chehab wrote:
> Em Fri, 27 Jun 2014 16:15:41 +0200
> Hans Verkuil <hverkuil@xs4all.nl> escreveu:
>
>> From: Hans Verkuil <hans.verkuil@cisco.com>
>>
>> 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.
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@cisco.com>
>> ---
>> 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;
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] cx23885: fix UNSET/TUNER_ABSENT confusion.
2014-07-17 22:55 ` Hans Verkuil
@ 2014-07-18 5:21 ` Hans Verkuil
2014-08-01 6:46 ` Hans Verkuil
0 siblings, 1 reply; 7+ messages in thread
From: Hans Verkuil @ 2014-07-18 5:21 UTC (permalink / raw)
To: Mauro Carvalho Chehab; +Cc: linux-media, it, =stoth, Hans Verkuil
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@xs4all.nl> escreveu:
>>
>>> From: Hans Verkuil <hans.verkuil@cisco.com>
>>>
>>> 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@cisco.com>
>>> ---
>>> 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@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] cx23885: fix UNSET/TUNER_ABSENT confusion.
2014-07-18 5:21 ` Hans Verkuil
@ 2014-08-01 6:46 ` Hans Verkuil
0 siblings, 0 replies; 7+ messages in thread
From: Hans Verkuil @ 2014-08-01 6:46 UTC (permalink / raw)
To: Mauro Carvalho Chehab; +Cc: linux-media, it, =stoth, Hans Verkuil
Mauro,
What is the status of this patch?
https://patchwork.linuxtv.org/patch/24553/
I actually thought it was merged, but I just found out it isn't.
You did merge https://patchwork.linuxtv.org/patch/24552/, but that's pointless
without fixing the UNSET/TUNER_ABSENT mess in this driver. The new board won't
work correctly without it.
Regards,
Hans
On 07/18/2014 07:21 AM, Hans Verkuil wrote:
> 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@xs4all.nl> escreveu:
>>>
>>>> From: Hans Verkuil <hans.verkuil@cisco.com>
>>>>
>>>> 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@cisco.com>
>>>> ---
>>>> 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@vger.kernel.org
>> 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@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2014-08-01 6:47 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-06-27 14:15 [PATCH 0/2] cx23885: add support for Hauppauge ImpactVCB-e Hans Verkuil
2014-06-27 14:15 ` [PATCH 1/2] cx23885: fix UNSET/TUNER_ABSENT confusion Hans Verkuil
2014-07-17 22:45 ` Mauro Carvalho Chehab
2014-07-17 22:55 ` Hans Verkuil
2014-07-18 5:21 ` Hans Verkuil
2014-08-01 6:46 ` Hans Verkuil
2014-06-27 14:15 ` [PATCH 2/2] cx23885: add support for Hauppauge ImpactVCB-e Hans Verkuil
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).