| Message ID | 20211013185812.590931-1-clabbe@baylibre.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | staging: media: zoran: fusion in one module | expand |
Hi Corentin, I noticed some code review comments from Dan and a kernel test robot issue. Can you post a v3 fixing those by the end of the week? Next week I will have access again to my zoran board, so then I can test the v3 series. BTW, I agree with Dan, just drop the 'Enable zoran debugfs' config option. It's not worth the additional complexity. Instead, just #ifdef CONFIG_DEBUG_FS where necessary (in most cases you shouldn't even have to do that since the since you have dummy debug_fs_* functions if CONFIG_DEBUG_FS isn't set). Regards, Hans On 13/10/2021 20:58, Corentin Labbe wrote: > Hello > > The main change of this serie is to fusion all zoran related modules in > one. > This fixes the load order problem when everything is built-in. > > Regards > > Changes since v1: > - add missing debugfs cleaning > - clean some remaining module_get/put functions which made impossible to > remove the zoran module > - added the two latest patchs > > Corentin Labbe (10): > staging: media: zoran: move module parameter checks to zoran_probe > staging: media: zoran: use module_pci_driver > staging: media: zoran: rename debug module parameter > staging: media: zoran: add debugfs > staging: media: zoran: videocode: remove procfs > staging: media: zoran: fusion all modules > staging: media: zoran: remove vidmem > staging: media: zoran: move videodev alloc > staging: media: zoran: move config select on primary kconfig > staging: media: zoran: introduce zoran_i2c_init > > drivers/staging/media/zoran/Kconfig | 46 +-- > drivers/staging/media/zoran/Makefile | 8 +- > drivers/staging/media/zoran/videocodec.c | 68 +---- > drivers/staging/media/zoran/videocodec.h | 6 +- > drivers/staging/media/zoran/zoran.h | 6 +- > drivers/staging/media/zoran/zoran_card.c | 328 ++++++++++++++------- > drivers/staging/media/zoran/zoran_driver.c | 5 +- > drivers/staging/media/zoran/zr36016.c | 24 +- > drivers/staging/media/zoran/zr36016.h | 2 + > drivers/staging/media/zoran/zr36050.c | 21 +- > drivers/staging/media/zoran/zr36050.h | 2 + > drivers/staging/media/zoran/zr36060.c | 21 +- > drivers/staging/media/zoran/zr36060.h | 2 + > 13 files changed, 291 insertions(+), 248 deletions(-) >
Hi Corentin, On 13/10/2021 20:58, Corentin Labbe wrote: > Hello > > The main change of this serie is to fusion all zoran related modules in > one. > This fixes the load order problem when everything is built-in. > > Regards > > Changes since v1: > - add missing debugfs cleaning > - clean some remaining module_get/put functions which made impossible to > remove the zoran module > - added the two latest patchs Something weird is wrong with this series. I have a DC30, but loading this with: modprobe zr36067 card=3 results in this error message in the kernel log: [ 58.645557] zr36067: module is from the staging directory, the quality is unknown, you have been warned. [ 58.646658] zr36067 0000:03:00.0: Zoran MJPEG board driver version 0.10.1 [ 58.646793] zr36067 0000:03:00.0: Zoran ZR36057 (rev 1), irq: 18, memory: 0xf4000000 [ 58.648821] zr36067 0000:03:00.0: Initializing i2c bus... [ 58.662420] vpx3220 22-0047: vpx3216b found @ 0x8e (DC30[0]) [ 58.737445] zr36067 0000:03:00.0: Fail to get encoder This works before, so why this is now failing is not clear to me. It does work with 'card=0', but I really have a DC30. If I test with 'card=0' then the rmmod issue is now solved. Regards, Hans > > Corentin Labbe (10): > staging: media: zoran: move module parameter checks to zoran_probe > staging: media: zoran: use module_pci_driver > staging: media: zoran: rename debug module parameter > staging: media: zoran: add debugfs > staging: media: zoran: videocode: remove procfs > staging: media: zoran: fusion all modules > staging: media: zoran: remove vidmem > staging: media: zoran: move videodev alloc > staging: media: zoran: move config select on primary kconfig > staging: media: zoran: introduce zoran_i2c_init > > drivers/staging/media/zoran/Kconfig | 46 +-- > drivers/staging/media/zoran/Makefile | 8 +- > drivers/staging/media/zoran/videocodec.c | 68 +---- > drivers/staging/media/zoran/videocodec.h | 6 +- > drivers/staging/media/zoran/zoran.h | 6 +- > drivers/staging/media/zoran/zoran_card.c | 328 ++++++++++++++------- > drivers/staging/media/zoran/zoran_driver.c | 5 +- > drivers/staging/media/zoran/zr36016.c | 24 +- > drivers/staging/media/zoran/zr36016.h | 2 + > drivers/staging/media/zoran/zr36050.c | 21 +- > drivers/staging/media/zoran/zr36050.h | 2 + > drivers/staging/media/zoran/zr36060.c | 21 +- > drivers/staging/media/zoran/zr36060.h | 2 + > 13 files changed, 291 insertions(+), 248 deletions(-) >
Le Mon, Oct 18, 2021 at 11:55:40AM +0200, Hans Verkuil a écrit : > Hi Corentin, > > I noticed some code review comments from Dan and a kernel test robot issue. > Can you post a v3 fixing those by the end of the week? Next week I will have > access again to my zoran board, so then I can test the v3 series. > > BTW, I agree with Dan, just drop the 'Enable zoran debugfs' config option. It's > not worth the additional complexity. Instead, just #ifdef CONFIG_DEBUG_FS > where necessary (in most cases you shouldn't even have to do that since the > since you have dummy debug_fs_* functions if CONFIG_DEBUG_FS isn't set). > Hello Ok I started fixing issues and will send V3 this week. Regards
Le Mon, Oct 25, 2021 at 02:45:02PM +0200, Hans Verkuil a écrit : > Hi Corentin, > > On 13/10/2021 20:58, Corentin Labbe wrote: > > Hello > > > > The main change of this serie is to fusion all zoran related modules in > > one. > > This fixes the load order problem when everything is built-in. > > > > Regards > > > > Changes since v1: > > - add missing debugfs cleaning > > - clean some remaining module_get/put functions which made impossible to > > remove the zoran module > > - added the two latest patchs > > Something weird is wrong with this series. I have a DC30, but loading this with: > > modprobe zr36067 card=3 > > results in this error message in the kernel log: > > [ 58.645557] zr36067: module is from the staging directory, the quality is unknown, you have been warned. > [ 58.646658] zr36067 0000:03:00.0: Zoran MJPEG board driver version 0.10.1 > [ 58.646793] zr36067 0000:03:00.0: Zoran ZR36057 (rev 1), irq: 18, memory: 0xf4000000 > [ 58.648821] zr36067 0000:03:00.0: Initializing i2c bus... > [ 58.662420] vpx3220 22-0047: vpx3216b found @ 0x8e (DC30[0]) > [ 58.737445] zr36067 0000:03:00.0: Fail to get encoder > > This works before, so why this is now failing is not clear to me. > > It does work with 'card=0', but I really have a DC30. > > If I test with 'card=0' then the rmmod issue is now solved. Everything normal, since card 0 does not have encoder. Could you check that adv7175 is compiled ? I got the same problem with my DC10+ where saa7110 was not compiled. This issue was reproduced randomly and I have no explanation. (kconfig problem ?) Regards
On 25/10/2021 16:21, LABBE Corentin wrote: > Le Mon, Oct 25, 2021 at 02:45:02PM +0200, Hans Verkuil a écrit : >> Hi Corentin, >> >> On 13/10/2021 20:58, Corentin Labbe wrote: >>> Hello >>> >>> The main change of this serie is to fusion all zoran related modules in >>> one. >>> This fixes the load order problem when everything is built-in. >>> >>> Regards >>> >>> Changes since v1: >>> - add missing debugfs cleaning >>> - clean some remaining module_get/put functions which made impossible to >>> remove the zoran module >>> - added the two latest patchs >> >> Something weird is wrong with this series. I have a DC30, but loading this with: >> >> modprobe zr36067 card=3 >> >> results in this error message in the kernel log: >> >> [ 58.645557] zr36067: module is from the staging directory, the quality is unknown, you have been warned. >> [ 58.646658] zr36067 0000:03:00.0: Zoran MJPEG board driver version 0.10.1 >> [ 58.646793] zr36067 0000:03:00.0: Zoran ZR36057 (rev 1), irq: 18, memory: 0xf4000000 >> [ 58.648821] zr36067 0000:03:00.0: Initializing i2c bus... >> [ 58.662420] vpx3220 22-0047: vpx3216b found @ 0x8e (DC30[0]) >> [ 58.737445] zr36067 0000:03:00.0: Fail to get encoder >> >> This works before, so why this is now failing is not clear to me. >> >> It does work with 'card=0', but I really have a DC30. >> >> If I test with 'card=0' then the rmmod issue is now solved. > > Everything normal, since card 0 does not have encoder. > Could you check that adv7175 is compiled ? Yes, and it loaded as well (I see it with lsmod). However, there is no adv7175 on my board, instead it appears to have an ITT MSE3000. There is no driver for this one (and I don't even think it is an i2c device), so I suspect that before the driver just continued without encoder support, whereas now it fails when it can't load the encoder. Could that be the reason? In the absence of an encoder, I think it should just continue, esp. since the driver doesn't use the encoder anyway. Regards, Hans > > I got the same problem with my DC10+ where saa7110 was not compiled. > This issue was reproduced randomly and I have no explanation. (kconfig problem ?) > > Regards >
Le Mon, Oct 25, 2021 at 05:13:04PM +0200, Hans Verkuil a écrit : > On 25/10/2021 16:21, LABBE Corentin wrote: > > Le Mon, Oct 25, 2021 at 02:45:02PM +0200, Hans Verkuil a écrit : > >> Hi Corentin, > >> > >> On 13/10/2021 20:58, Corentin Labbe wrote: > >>> Hello > >>> > >>> The main change of this serie is to fusion all zoran related modules in > >>> one. > >>> This fixes the load order problem when everything is built-in. > >>> > >>> Regards > >>> > >>> Changes since v1: > >>> - add missing debugfs cleaning > >>> - clean some remaining module_get/put functions which made impossible to > >>> remove the zoran module > >>> - added the two latest patchs > >> > >> Something weird is wrong with this series. I have a DC30, but loading this with: > >> > >> modprobe zr36067 card=3 > >> > >> results in this error message in the kernel log: > >> > >> [ 58.645557] zr36067: module is from the staging directory, the quality is unknown, you have been warned. > >> [ 58.646658] zr36067 0000:03:00.0: Zoran MJPEG board driver version 0.10.1 > >> [ 58.646793] zr36067 0000:03:00.0: Zoran ZR36057 (rev 1), irq: 18, memory: 0xf4000000 > >> [ 58.648821] zr36067 0000:03:00.0: Initializing i2c bus... > >> [ 58.662420] vpx3220 22-0047: vpx3216b found @ 0x8e (DC30[0]) > >> [ 58.737445] zr36067 0000:03:00.0: Fail to get encoder > >> > >> This works before, so why this is now failing is not clear to me. > >> > >> It does work with 'card=0', but I really have a DC30. > >> > >> If I test with 'card=0' then the rmmod issue is now solved. > > > > Everything normal, since card 0 does not have encoder. > > Could you check that adv7175 is compiled ? > > Yes, and it loaded as well (I see it with lsmod). > > However, there is no adv7175 on my board, instead it appears to have an ITT MSE3000. > There is no driver for this one (and I don't even think it is an i2c device), so > I suspect that before the driver just continued without encoder support, whereas now > it fails when it can't load the encoder. > > Could that be the reason? In the absence of an encoder, I think it should just > continue, esp. since the driver doesn't use the encoder anyway. > So probably the card list is wrong against DC30. I checked high resolution photo of DC30 on internet, and it confirms the fact that DC30 does not have adv7175. Since DC30 and DC30+ are identical in the card list, perhaps it is a very old copy/paste error. So I will add a patch removing adv7175 from DC30. Thanks for the report Regards