Re: av7110 and budget_av are broken! (was: Re: changeset 14351:2eda2bcc8d6f)
Hans Verkuil wrote:
> On Saturday 20 March 2010 15:07:08 Oliver Endriss wrote:
> > e9hack wrote:
> > > Am 13.3.2010 17:27, schrieb Hans Verkuil:
> > > > If there are no further comments, then I'll post a pull request in a
> > > > few days.
> > > >
> > > > Tested with the mxb board. It would be nice if you can verify this with
> > > > the
> > > > av7110.
> > >
> > > Hi hans,
> > >
> > > it works with my TT-C2300 perfectly. The main problem of your changes
> > > was: It wasn't
> > > possible to unload the module for the TT-C2300.
> >
> > Guys, when will you finally apply this fix?
>
> Thanks for reminding me, I frankly forgot about this.
>
> Hartmut, is the problem with unloading the module something that my patch
> caused? Or was that there as well before changeset 14351:2eda2bcc8d6f?
> Are there any kernel messages indicating why it won't unload?
The patch caused the problem.
You moved v4l2_device_register() from saa7146_vv_init() to
saa7146_vv_devinit(), but you did not modify av7110_v4l.c and
budget-av.c accordingly.
$ grep saa7146_vv_init v4l/*c
v4l/av7110_v4l.c: ret = saa7146_vv_init(dev, vv_data);
v4l/budget-av.c:if (0 != saa7146_vv_init(dev, &vv_data)) {
v4l/hexium_gemini.c:saa7146_vv_init(dev, &vv_data);
v4l/hexium_orion.c: saa7146_vv_init(dev, &vv_data);
v4l/mxb.c: saa7146_vv_init(dev, &vv_data);
v4l/saa7146_fops.c:int saa7146_vv_init(struct saa7146_dev* dev, struct
saa7146_ext_vv *ext_vv)
v4l/saa7146_fops.c:EXPORT_SYMBOL_GPL(saa7146_vv_init);
v4l/saa7146_fops.c:static int __init saa7146_vv_init_module(void)
v4l/saa7146_fops.c:module_init(saa7146_vv_init_module);
$ grep saa7146_vv_devinit v4l/*c
v4l/hexium_gemini.c:ret = saa7146_vv_devinit(dev);
v4l/hexium_orion.c: err = saa7146_vv_devinit(dev);
v4l/mxb.c: err = saa7146_vv_devinit(dev);
v4l/saa7146_fops.c:int saa7146_vv_devinit(struct saa7146_dev *dev)
v4l/saa7146_fops.c:EXPORT_SYMBOL_GPL(saa7146_vv_devinit);
CU
Oliver
--
VDR Remote Plugin 0.4.0: http://www.escape-edv.de/endriss/vdr/
4 MByte Mod: http://www.escape-edv.de/endriss/dvb-mem-mod/
Full-TS Mod: http://www.escape-edv.de/endriss/dvb-full-ts-mod/
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: av7110 and budget_av are broken! (was: Re: changeset 14351:2eda2bcc8d6f)
On Saturday 20 March 2010 15:07:08 Oliver Endriss wrote: > e9hack wrote: > > Am 13.3.2010 17:27, schrieb Hans Verkuil: > > > If there are no further comments, then I'll post a pull request in a few > > > days. > > > > > > Tested with the mxb board. It would be nice if you can verify this with > > > the > > > av7110. > > > > Hi hans, > > > > it works with my TT-C2300 perfectly. The main problem of your changes was: > > It wasn't > > possible to unload the module for the TT-C2300. > > Guys, when will you finally apply this fix? Thanks for reminding me, I frankly forgot about this. Hartmut, is the problem with unloading the module something that my patch caused? Or was that there as well before changeset 14351:2eda2bcc8d6f? Are there any kernel messages indicating why it won't unload? Regards, Hans > As Hartmut pointed out, changeset 14351:2eda2bcc8d6f broke the av7110 > driver (and also budget-av). > > It is time to fix it. This bug must not go into the kernel! > > CU > Oliver > > -- Hans Verkuil - video4linux developer - sponsored by TANDBERG -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
av7110 and budget_av are broken! (was: Re: changeset 14351:2eda2bcc8d6f)
e9hack wrote: > Am 13.3.2010 17:27, schrieb Hans Verkuil: > > If there are no further comments, then I'll post a pull request in a few > > days. > > > > Tested with the mxb board. It would be nice if you can verify this with the > > av7110. > > Hi hans, > > it works with my TT-C2300 perfectly. The main problem of your changes was: It > wasn't > possible to unload the module for the TT-C2300. Guys, when will you finally apply this fix? As Hartmut pointed out, changeset 14351:2eda2bcc8d6f broke the av7110 driver (and also budget-av). It is time to fix it. This bug must not go into the kernel! CU Oliver -- VDR Remote Plugin 0.4.0: http://www.escape-edv.de/endriss/vdr/ 4 MByte Mod: http://www.escape-edv.de/endriss/dvb-mem-mod/ Full-TS Mod: http://www.escape-edv.de/endriss/dvb-full-ts-mod/ -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: changeset 14351:2eda2bcc8d6f
Am 13.3.2010 17:27, schrieb Hans Verkuil: > If there are no further comments, then I'll post a pull request in a few days. > > Tested with the mxb board. It would be nice if you can verify this with the > av7110. Hi hans, it works with my TT-C2300 perfectly. The main problem of your changes was: It wasn't possible to unload the module for the TT-C2300. Regards, Hartmut -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: changeset 14351:2eda2bcc8d6f
ia/saa7146_vv.h b/include/media/saa7146_vv.h
index b9da1f5..4aeff96 100644
--- a/include/media/saa7146_vv.h
+++ b/include/media/saa7146_vv.h
@@ -188,7 +188,6 @@ void saa7146_buffer_timeout(unsigned long data);
void saa7146_dma_free(struct saa7146_dev* dev,struct videobuf_queue *q,
struct saa7146_buf *buf);
-int saa7146_vv_devinit(struct saa7146_dev *dev);
int saa7146_vv_init(struct saa7146_dev* dev, struct saa7146_ext_vv *ext_vv);
int saa7146_vv_release(struct saa7146_dev* dev);
On Wednesday 03 March 2010 12:39:27 e9hack wrote:
> Hi,
>
> changeset 14351:2eda2bcc8d6f is incomplete. If the init function is split in
> two function,
> the deinit function shall consider this. The changes shall be apply also to
> av7110_init_v4l().
>
> diff -r 58ae12f18e80 linux/drivers/media/common/saa7146_fops.c
> --- a/linux/drivers/media/common/saa7146_fops.c Tue Mar 02 23:52:36 2010 -0300
> +++ b/linux/drivers/media/common/saa7146_fops.c Wed Mar 03 12:15:23 2010 +0100
> @@ -481,8 +481,10 @@ int saa7146_vv_release(struct saa7146_de
> DEB_EE(("dev:%p\n",dev));
>
> v4l2_device_unregister(&dev->v4l2_dev);
> - pci_free_consistent(dev->pci, SAA7146_CLIPPING_MEM,
> vv->d_clipping.cpu_addr,
> vv->d_clipping.dma_handle);
> - kfree(vv);
> + if (vv) {
> + pci_free_consistent(dev->pci, SAA7146_CLIPPING_MEM,
> vv->d_clipping.cpu_addr, vv->d_clipping.dma_handle);
> + kfree(vv);
> + }
> dev->vv_data = NULL;
> dev->vv_callback = NULL;
>
> diff -r 58ae12f18e80 linux/drivers/media/dvb/ttpci/av7110_v4l.c
> --- a/linux/drivers/media/dvb/ttpci/av7110_v4l.cTue Mar 02 23:52:36
> 2010 -0300
> +++ b/linux/drivers/media/dvb/ttpci/av7110_v4l.cWed Mar 03 12:15:23
> 2010 +0100
> @@ -790,12 +790,20 @@ int av7110_init_v4l(struct av7110 *av711
> vv_data = &av7110_vv_data_c;
> else
> vv_data = &av7110_vv_data_st;
> + ret = saa7146_vv_devinit(dev);
> +
> + if (ret < 0) {
> + ERR(("cannot init device. skipping.\n"));
> + return ret;
> + }
> +
> ret = saa7146_vv_init(dev, vv_data);
> -
> - if (ret) {
> + if (ret < 0) {
> ERR(("cannot init capture device. skipping.\n"));
> + saa7146_vv_release(dev);
> return ret;
> }
> +
> vv_data->ops.vidioc_enum_input = vidioc_enum_input;
> vv_data->ops.vidioc_g_input = vidioc_g_input;
> vv_data->ops.vidioc_s_input = vidioc_s_input;
>
> Regards,
> Hartmut
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
--
Hans Verkuil - video4linux developer - sponsored by TANDBERG
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
changeset 14351:2eda2bcc8d6f
Hi,
changeset 14351:2eda2bcc8d6f is incomplete. If the init function is split in
two function,
the deinit function shall consider this. The changes shall be apply also to
av7110_init_v4l().
diff -r 58ae12f18e80 linux/drivers/media/common/saa7146_fops.c
--- a/linux/drivers/media/common/saa7146_fops.c Tue Mar 02 23:52:36 2010 -0300
+++ b/linux/drivers/media/common/saa7146_fops.c Wed Mar 03 12:15:23 2010 +0100
@@ -481,8 +481,10 @@ int saa7146_vv_release(struct saa7146_de
DEB_EE(("dev:%p\n",dev));
v4l2_device_unregister(&dev->v4l2_dev);
- pci_free_consistent(dev->pci, SAA7146_CLIPPING_MEM,
vv->d_clipping.cpu_addr,
vv->d_clipping.dma_handle);
- kfree(vv);
+ if (vv) {
+ pci_free_consistent(dev->pci, SAA7146_CLIPPING_MEM,
vv->d_clipping.cpu_addr, vv->d_clipping.dma_handle);
+ kfree(vv);
+ }
dev->vv_data = NULL;
dev->vv_callback = NULL;
diff -r 58ae12f18e80 linux/drivers/media/dvb/ttpci/av7110_v4l.c
--- a/linux/drivers/media/dvb/ttpci/av7110_v4l.cTue Mar 02 23:52:36
2010 -0300
+++ b/linux/drivers/media/dvb/ttpci/av7110_v4l.cWed Mar 03 12:15:23
2010 +0100
@@ -790,12 +790,20 @@ int av7110_init_v4l(struct av7110 *av711
vv_data = &av7110_vv_data_c;
else
vv_data = &av7110_vv_data_st;
+ ret = saa7146_vv_devinit(dev);
+
+ if (ret < 0) {
+ ERR(("cannot init device. skipping.\n"));
+ return ret;
+ }
+
ret = saa7146_vv_init(dev, vv_data);
-
- if (ret) {
+ if (ret < 0) {
ERR(("cannot init capture device. skipping.\n"));
+ saa7146_vv_release(dev);
return ret;
}
+
vv_data->ops.vidioc_enum_input = vidioc_enum_input;
vv_data->ops.vidioc_g_input = vidioc_g_input;
vv_data->ops.vidioc_s_input = vidioc_s_input;
Regards,
Hartmut
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
