From 562378e1f982f3a817c81594e61d40ffa05fac7d Mon Sep 17 00:00:00 2001 From: Trent Piepho Date: Wed, 28 Jan 2009 16:32:58 -0800 Subject: bttv: norm value should be unsigned From: Trent Piepho The norm value in the driver is an index into an array and the the driver doesn't allow it to be negative or otherwise invalid. It should be unsigned but wasn't in all places. Fix some structs and functions to have the norm be unsigned. Get rid of useless checks for "< 0". Most of the driver code can't handle a norm value that's out of range, so change some ">= BTTV_TVNORMS" checks to BUG_ON(). There's no point in silently ignoring invalid driver state just to crash because of it later. Priority: normal Reported-by: Roel Kluin Signed-off-by: Trent Piepho --- linux/drivers/media/video/bt8xx/bttv-driver.c | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) (limited to 'linux/drivers/media/video/bt8xx/bttv-driver.c') diff --git a/linux/drivers/media/video/bt8xx/bttv-driver.c b/linux/drivers/media/video/bt8xx/bttv-driver.c index c02c0ec80..41cf151d8 100644 --- a/linux/drivers/media/video/bt8xx/bttv-driver.c +++ b/linux/drivers/media/video/bt8xx/bttv-driver.c @@ -1300,7 +1300,7 @@ bttv_crop_calc_limits(struct bttv_crop *c) } static void -bttv_crop_reset(struct bttv_crop *c, int norm) +bttv_crop_reset(struct bttv_crop *c, unsigned int norm) { c->rect = bttv_tvnorms[norm].cropcap.defrect; bttv_crop_calc_limits(c); @@ -1313,16 +1313,13 @@ set_tvnorm(struct bttv *btv, unsigned int norm) const struct bttv_tvnorm *tvnorm; v4l2_std_id id; - if (norm < 0 || norm >= BTTV_TVNORMS) - return -EINVAL; + BUG_ON(norm >= BTTV_TVNORMS); + BUG_ON(btv->tvnorm >= BTTV_TVNORMS); tvnorm = &bttv_tvnorms[norm]; - if (btv->tvnorm < 0 || - btv->tvnorm >= BTTV_TVNORMS || - 0 != memcmp(&bttv_tvnorms[btv->tvnorm].cropcap, - &tvnorm->cropcap, - sizeof (tvnorm->cropcap))) { + if (!memcmp(&bttv_tvnorms[btv->tvnorm].cropcap, &tvnorm->cropcap, + sizeof (tvnorm->cropcap))) { bttv_crop_reset(&btv->crop[0], norm); btv->crop[1] = btv->crop[0]; /* current = default */ -- cgit v1.2.3 From 4a49903c6320c8ba9dfe2c139103166f75594f49 Mon Sep 17 00:00:00 2001 From: Trent Piepho Date: Wed, 28 Jan 2009 16:32:59 -0800 Subject: bttv: Fix TDA9880 norm setting code From: Trent Piepho The code to set the norm for the TDA9880 analog demod was comparing btv->norm, an index into the bttv driver's norm array, to V4L2_STD_NTSC, which is a bit flag that's part of the V4L2 API. This doesn't work of course and results in the PAL path always being taken. What's more, it modified the bttv_tvcards[] entries for cards using the TDA9880. This is wrong because changing the norm on one card will also affect other cards of the same type. Writing to bttv_tvcards is also bad because it should be read-only or even devinitdata. Changing the norm would also cause the audio to become unmuted. Have the code get called for both norm setting and audio input setting (which where the gpios are set) to avoid needed to modify bttv_tvcards. Priority: normal Signed-off-by: Trent Piepho --- linux/drivers/media/video/bt8xx/bttv-driver.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) (limited to 'linux/drivers/media/video/bt8xx/bttv-driver.c') diff --git a/linux/drivers/media/video/bt8xx/bttv-driver.c b/linux/drivers/media/video/bt8xx/bttv-driver.c index 41cf151d8..f72c6ffea 100644 --- a/linux/drivers/media/video/bt8xx/bttv-driver.c +++ b/linux/drivers/media/video/bt8xx/bttv-driver.c @@ -1197,13 +1197,22 @@ audio_mux(struct bttv *btv, int input, int mute) gpio_val = bttv_tvcards[btv->c.type].gpiomute; else gpio_val = bttv_tvcards[btv->c.type].gpiomux[input]; + + switch (btv->c.type) { + case BTTV_BOARD_VOODOOTV_FM: + case BTTV_BOARD_VOODOOTV_200: + gpio_val = bttv_tda9880_setnorm(btv, gpio_val); + break; + + default: + gpio_bits(bttv_tvcards[btv->c.type].gpiomask, gpio_val); + } #if 0 printk("bttv%d: amux: input=%d mute=%d signal=%s gpio_mux=%d irq=%s\n", btv->c.nr, input, mute, signal ? "yes" : "no", gpio_val, in_interrupt() ? "yes" : "no"); #endif - gpio_bits(bttv_tvcards[btv->c.type].gpiomask, gpio_val); if (bttv_gpio) bttv_gpio_tracking(btv, audio_modes[mute ? 4 : input]); if (in_interrupt()) @@ -1342,7 +1351,7 @@ set_tvnorm(struct bttv *btv, unsigned int norm) switch (btv->c.type) { case BTTV_BOARD_VOODOOTV_FM: case BTTV_BOARD_VOODOOTV_200: - bttv_tda9880_setnorm(btv,norm); + bttv_tda9880_setnorm(btv, gpio_read()); break; #if 0 case BTTV_BOARD_OSPREY540: -- cgit v1.2.3 From f030fca37c310181b3fce820ab359bb8eceed0e2 Mon Sep 17 00:00:00 2001 From: Trent Piepho Date: Wed, 28 Jan 2009 16:32:59 -0800 Subject: bttv: make tuner card info more consistent From: Trent Piepho The bttv card database structure had a "tuner" field that was the input number of the tuner input or UNSET for no tuner. However, the only values it could ever be are 0 and UNSET. Having a tuner on an input other than 0 didn't work and was never used. There is also a "tuner_type" field that can be set to TUNER_ABSENT to indicate no tuner, which makes "tuner = UNSET" redundant. In many cases, tuner_type was set to UNSET when there was no tuner, which isn't quite correct. tuner_type == UNSET is supposed to mean the tuner type isn't yet known. So, I changed cards where "tuner == UNSET" to always have tuner_type of TUNER_ABSENT. At this point the tuner field is redundant, so I deleted it. I have the card setup code set the card's tuner_type (not the card type's tuner_type!) to TUNER_ABSENT if it hasn't yet been set at the end of the setup code. Various places that check if the card has a tuner will now look for this instead of checking the card type's "tuner" field. Also autoload the tuner module before issuing the TUNER_SET_TYPE_ADDR I2C client call instead of after issuing it. Overall, on ia32 this decreases compiled code size by about 24 bytes and reduces the data size by 640 bytes. Priority: normal Signed-off-by: Trent Piepho --- linux/drivers/media/video/bt8xx/bttv-driver.c | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) (limited to 'linux/drivers/media/video/bt8xx/bttv-driver.c') diff --git a/linux/drivers/media/video/bt8xx/bttv-driver.c b/linux/drivers/media/video/bt8xx/bttv-driver.c index f72c6ffea..53be29350 100644 --- a/linux/drivers/media/video/bt8xx/bttv-driver.c +++ b/linux/drivers/media/video/bt8xx/bttv-driver.c @@ -1384,8 +1384,8 @@ set_input(struct bttv *btv, unsigned int input, unsigned int norm) } else { video_mux(btv,input); } - audio_input(btv,(input == bttv_tvcards[btv->c.type].tuner ? - TVAUDIO_INPUT_TUNER : TVAUDIO_INPUT_EXTERN)); + audio_input(btv, (btv->tuner_type != TUNER_ABSENT && input == 0) ? + TVAUDIO_INPUT_TUNER : TVAUDIO_INPUT_EXTERN); set_tvnorm(btv, norm); } @@ -1935,7 +1935,7 @@ static int bttv_enum_input(struct file *file, void *priv, i->type = V4L2_INPUT_TYPE_CAMERA; i->audioset = 1; - if (i->index == bttv_tvcards[btv->c.type].tuner) { + if (btv->tuner_type != TUNER_ABSENT && i->index == 0) { sprintf(i->name, "Television"); i->type = V4L2_INPUT_TYPE_TUNER; i->tuner = 0; @@ -1999,7 +1999,7 @@ static int bttv_s_tuner(struct file *file, void *priv, if (0 != err) return err; - if (UNSET == bttv_tvcards[btv->c.type].tuner) + if (btv->tuner_type == TUNER_ABSENT) return -EINVAL; if (0 != t->index) @@ -2693,8 +2693,7 @@ static int bttv_querycap(struct file *file, void *priv, if (no_overlay <= 0) cap->capabilities |= V4L2_CAP_VIDEO_OVERLAY; - if (bttv_tvcards[btv->c.type].tuner != UNSET && - bttv_tvcards[btv->c.type].tuner != TUNER_ABSENT) + if (btv->tuner_type != TUNER_ABSENT) cap->capabilities |= V4L2_CAP_TUNER; return 0; } @@ -2977,7 +2976,7 @@ static int bttv_g_tuner(struct file *file, void *priv, struct bttv_fh *fh = priv; struct bttv *btv = fh->btv; - if (UNSET == bttv_tvcards[btv->c.type].tuner) + if (btv->tuner_type == TUNER_ABSENT) return -EINVAL; if (0 != t->index) return -EINVAL; @@ -3537,7 +3536,7 @@ static int radio_g_tuner(struct file *file, void *priv, struct v4l2_tuner *t) struct bttv_fh *fh = priv; struct bttv *btv = fh->btv; - if (UNSET == bttv_tvcards[btv->c.type].tuner) + if (btv->tuner_type == TUNER_ABSENT) return -EINVAL; if (0 != t->index) return -EINVAL; -- cgit v1.2.3 From e418d7277c31fd429d55fb0b1686c5b61bda06cb Mon Sep 17 00:00:00 2001 From: Trent Piepho Date: Wed, 28 Jan 2009 16:32:59 -0800 Subject: bttv: rework the way digital inputs are indicated From: Trent Piepho The code was using a muxsel value of -1U to indicate a digital input. A couple places in were checking of muxsel < 0 to detect this, which doesn't work of course because muxsel is unsigned and can't be negative. Only a couple cards had digital inputs and it was always the last one, so for the card database create a one bit field that indicates the last input is digital. On init, this is used to set a new field in the bttv struct to the digital input's number or UNSET for none. This makes it easier to check if the current input is digital. Priority: normal Signed-off-by: Trent Piepho --- linux/drivers/media/video/bt8xx/bttv-driver.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'linux/drivers/media/video/bt8xx/bttv-driver.c') diff --git a/linux/drivers/media/video/bt8xx/bttv-driver.c b/linux/drivers/media/video/bt8xx/bttv-driver.c index 53be29350..fb57f00bd 100644 --- a/linux/drivers/media/video/bt8xx/bttv-driver.c +++ b/linux/drivers/media/video/bt8xx/bttv-driver.c @@ -1058,7 +1058,7 @@ static void bt848A_set_timing(struct bttv *btv) int table_idx = bttv_tvnorms[btv->tvnorm].sram; int fsc = bttv_tvnorms[btv->tvnorm].Fsc; - if (UNSET == bttv_tvcards[btv->c.type].muxsel[btv->input]) { + if (btv->input == btv->dig) { dprintk("bttv%d: load digital timing table (table_idx=%d)\n", btv->c.nr,table_idx); -- cgit v1.2.3 From c945cec9c8d31064b500e0dff54cd4b3b7ec5136 Mon Sep 17 00:00:00 2001 From: Trent Piepho Date: Wed, 28 Jan 2009 16:32:59 -0800 Subject: bttv: shrink muxsel data in card database From: Trent Piepho Over half of the card database was used to store muxsel data. 64 bytes were used to store one 32 bit word for each of up to 16 inputs. The Bt8x8 only has two bits to control its mux, so muxsel data for 16 inputs will fit into a single 32 bit word. There were a couple cards that had special muxsel data that didn't fit in two bits, but I cleaned them up in earlier patches. Unfortunately, C doesn't allow us to have an array of bit fields. This makes initializing the structure more of a pain. But with some cpp magic, we can do it by changing: .muxsel = { 2, 3, 0, 1 }, .muxsel = { 2, 2, 2, 2, 3, 3, 3, 3, 1, 1 }, Into: .muxsel = MUXSEL(2, 3, 0, 1), .muxsel = MUXSEL(2, 2, 2, 2, 3, 3, 3, 3, 1, 1), That's not so bad. MUXSEL is a fancy macro that packs the arguments (of which there can be one to sixteen!) into a single word two bits at a time. It's a compile time constant (a variadic function wouldn't be) so we can use it to initialize the structure. It's important the the arguments to the macro only be plain decimal integers. Stuff like "0x01", "(2)", or "MUX3" won't work properly. I also created an accessor function, bttv_muxsel(btv, input), that gets the mux bits for the selected input. It makes it cleaner to change the way the muxsel data is stored. This patch doesn't change the code size and decreases the datasegment by 9440 bytes. Priority: normal Signed-off-by: Trent Piepho --- linux/drivers/media/video/bt8xx/bttv-driver.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'linux/drivers/media/video/bt8xx/bttv-driver.c') diff --git a/linux/drivers/media/video/bt8xx/bttv-driver.c b/linux/drivers/media/video/bt8xx/bttv-driver.c index fb57f00bd..bdbd9b97a 100644 --- a/linux/drivers/media/video/bt8xx/bttv-driver.c +++ b/linux/drivers/media/video/bt8xx/bttv-driver.c @@ -1160,7 +1160,7 @@ video_mux(struct bttv *btv, unsigned int input) btand(~BT848_CONTROL_COMP, BT848_E_CONTROL); btand(~BT848_CONTROL_COMP, BT848_O_CONTROL); } - mux = bttv_tvcards[btv->c.type].muxsel[input] & 3; + mux = bttv_muxsel(btv, input); btaor(mux<<5, ~(3<<5), BT848_IFORM); dprintk(KERN_DEBUG "bttv%d: video mux: input=%d mux=%d\n", btv->c.nr,input,mux); -- cgit v1.2.3 From e2b4212172d81f17f5e4741766f032d06e83ce4f Mon Sep 17 00:00:00 2001 From: Trent Piepho Date: Wed, 28 Jan 2009 16:32:59 -0800 Subject: bttv: dynamically allocate device data From: Trent Piepho The bttv driver had static array of structures for up to 16 possible bttv devices, even though few users have more than one or two. The structures were quite large and this resulted in a huge BSS segment. Change the driver to allocate the bttv device data dynamically, which changes "struct bttv bttvs[BTTV_MAX]" to "struct bttv *bttvs[BTTV_MAX]". It would be nice to get ride of "bttvs" entirely but there are some complications with gpio access from the audio & mpeg drivers. To help bttvs removal along anyway, I changed the open() methods use the video device's drvdata to get the driver data instead of looking it up in the bttvs array. This is also more efficient. Some WARN_ON()s are added in cases the device node exists by the bttv device doesn't, which I don't think should be possible. The gpio access functions need to check if bttvs[card] is NULL now. Though calling them on a non-existent card in the first place is wrong, but hard to solve given the fundamental problems in how the gpio access code works. This patch reduces the bss size by 66560 bytes on ia32. Overall change is a reduction of 66398 bytes, as the WARN_ON()s add some 198 bytes. Priority: normal Signed-off-by: Trent Piepho --- linux/drivers/media/video/bt8xx/bttv-driver.c | 45 ++++++++++----------------- 1 file changed, 16 insertions(+), 29 deletions(-) (limited to 'linux/drivers/media/video/bt8xx/bttv-driver.c') diff --git a/linux/drivers/media/video/bt8xx/bttv-driver.c b/linux/drivers/media/video/bt8xx/bttv-driver.c index bdbd9b97a..ab202761b 100644 --- a/linux/drivers/media/video/bt8xx/bttv-driver.c +++ b/linux/drivers/media/video/bt8xx/bttv-driver.c @@ -58,7 +58,7 @@ unsigned int bttv_num; /* number of Bt848s in use */ -struct bttv bttvs[BTTV_MAX]; +struct bttv *bttvs[BTTV_MAX]; unsigned int bttv_debug; unsigned int bttv_verbose = 1; @@ -3245,29 +3245,19 @@ err: static int bttv_open(struct file *file) { int minor = video_devdata(file)->minor; - struct bttv *btv = NULL; + struct bttv *btv = video_drvdata(file); struct bttv_fh *fh; enum v4l2_buf_type type = 0; - unsigned int i; dprintk(KERN_DEBUG "bttv: open minor=%d\n",minor); lock_kernel(); - for (i = 0; i < bttv_num; i++) { - if (bttvs[i].video_dev && - bttvs[i].video_dev->minor == minor) { - btv = &bttvs[i]; - type = V4L2_BUF_TYPE_VIDEO_CAPTURE; - break; - } - if (bttvs[i].vbi_dev && - bttvs[i].vbi_dev->minor == minor) { - btv = &bttvs[i]; - type = V4L2_BUF_TYPE_VBI_CAPTURE; - break; - } - } - if (NULL == btv) { + if (btv->video_dev->minor == minor) { + type = V4L2_BUF_TYPE_VIDEO_CAPTURE; + } else if (btv->vbi_dev->minor == minor) { + type = V4L2_BUF_TYPE_VBI_CAPTURE; + } else { + WARN_ON(1); unlock_kernel(); return -ENODEV; } @@ -3457,20 +3447,14 @@ static struct video_device bttv_video_template = { static int radio_open(struct file *file) { int minor = video_devdata(file)->minor; - struct bttv *btv = NULL; + struct bttv *btv = video_drvdata(file); struct bttv_fh *fh; - unsigned int i; dprintk("bttv: open minor=%d\n",minor); lock_kernel(); - for (i = 0; i < bttv_num; i++) { - if (bttvs[i].radio_dev && bttvs[i].radio_dev->minor == minor) { - btv = &bttvs[i]; - break; - } - } - if (NULL == btv) { + WARN_ON(btv->radio_dev && btv->radio_dev->minor != minor); + if (!btv->radio_dev || btv->radio_dev->minor != minor) { unlock_kernel(); return -ENODEV; } @@ -4235,6 +4219,7 @@ static struct video_device *vdev_init(struct bttv *btv, vfd->parent = &btv->c.pci->dev; vfd->release = video_device_release; vfd->debug = bttv_debug; + video_set_drvdata(vfd, btv); snprintf(vfd->name, sizeof(vfd->name), "BT%d%s %s (%s)", btv->id, (btv->id==848 && btv->revision==0x12) ? "A" : "", type_name, bttv_tvcards[btv->c.type].name); @@ -4349,8 +4334,7 @@ static int __devinit bttv_probe(struct pci_dev *dev, if (bttv_num == BTTV_MAX) return -ENOMEM; printk(KERN_INFO "bttv: Bt8xx card found (%d).\n", bttv_num); - btv=&bttvs[bttv_num]; - memset(btv,0,sizeof(*btv)); + bttvs[bttv_num] = btv = kzalloc(sizeof(*btv), GFP_KERNEL); btv->c.nr = bttv_num; sprintf(btv->c.name,"bttv%d",btv->c.nr); @@ -4554,6 +4538,9 @@ static void __devexit bttv_remove(struct pci_dev *pci_dev) pci_resource_len(btv->c.pci,0)); pci_set_drvdata(pci_dev, NULL); + bttvs[btv->c.nr] = NULL; + kfree(btv); + return; } -- cgit v1.2.3