Re: [PATCH] media: davinci: vpif: fix array out of bound warnings
Hi Prabhakar, On Fri, Jul 18, 2014 at 05:31:51PM +0100, Lad, Prabhakar wrote: This patch fixes following array out of bound warnings, drivers/media/platform/davinci/vpif_display.c: In function 'vpif_remove': drivers/media/platform/davinci/vpif_display.c:1389:36: warning: iteration 1u invokes undefined behavior [-Waggressive-loop-optimizations] vb2_dma_contig_cleanup_ctx(common-alloc_ctx); ^ drivers/media/platform/davinci/vpif_display.c:1385:2: note: containing loop for (i = 0; i VPIF_DISPLAY_MAX_DEVICES; i++) { ^ drivers/media/platform/davinci/vpif_capture.c: In function 'vpif_remove': drivers/media/platform/davinci/vpif_capture.c:1581:36: warning: iteration 1u invokes undefined behavior [-Waggressive-loop-optimizations] vb2_dma_contig_cleanup_ctx(common-alloc_ctx); ^ drivers/media/platform/davinci/vpif_capture.c:1577:2: note: containing loop for (i = 0; i VPIF_CAPTURE_MAX_DEVICES; i++) { ^ drivers/media/platform/davinci/vpif_capture.c:1580:23: warning: array subscript is above array bounds [-Warray-bounds] common = ch-common[i]; Reported-by: Hans Verkuil hans.verk...@cisco.com Signed-off-by: Lad, Prabhakar prabhakar.cse...@gmail.com --- drivers/media/platform/davinci/vpif_capture.c | 2 +- drivers/media/platform/davinci/vpif_display.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/media/platform/davinci/vpif_capture.c b/drivers/media/platform/davinci/vpif_capture.c index 2f90f0d..3a85238 100644 --- a/drivers/media/platform/davinci/vpif_capture.c +++ b/drivers/media/platform/davinci/vpif_capture.c @@ -1577,7 +1577,7 @@ static int vpif_remove(struct platform_device *device) for (i = 0; i VPIF_CAPTURE_MAX_DEVICES; i++) { /* Get the pointer to the channel object */ ch = vpif_obj.dev[i]; - common = ch-common[i]; + common = ch-common[VPIF_VIDEO_INDEX]; You could refer to the alloc_ctz directly w/o extra local variables. Also local variables that are only used inside the loop could be declared there as well. Acked-by: Sakari Ailus sakari.ai...@iki.fi vb2_dma_contig_cleanup_ctx(common-alloc_ctx); /* Unregister video device */ video_unregister_device(ch-video_dev); diff --git a/drivers/media/platform/davinci/vpif_display.c b/drivers/media/platform/davinci/vpif_display.c index 0bd6dcb..6c6bd6b 100644 --- a/drivers/media/platform/davinci/vpif_display.c +++ b/drivers/media/platform/davinci/vpif_display.c @@ -1385,7 +1385,7 @@ static int vpif_remove(struct platform_device *device) for (i = 0; i VPIF_DISPLAY_MAX_DEVICES; i++) { /* Get the pointer to the channel object */ ch = vpif_obj.dev[i]; - common = ch-common[i]; + common = ch-common[VPIF_VIDEO_INDEX]; vb2_dma_contig_cleanup_ctx(common-alloc_ctx); /* Unregister video device */ video_unregister_device(ch-video_dev); -- Kind regards, Sakari Ailus e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk -- 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
[PATCH] staging: media: as102: replace custom dprintk() with dev_dbg()
remove dprintk() and replace it with dev_dbg() in order to use the common kernel coding style. Signed-off-by: Martin Kepplinger mart...@posteo.de --- I don't have the device but this builds. I think this is ok when it gets reviewed. applies to -next20140801 drivers/staging/media/as102/as102_drv.c | 15 + drivers/staging/media/as102/as102_drv.h |7 - drivers/staging/media/as102/as102_fe.c | 44 +++ drivers/staging/media/as102/as102_usb_drv.c | 41 ++--- 4 files changed, 62 insertions(+), 45 deletions(-) diff --git a/drivers/staging/media/as102/as102_drv.c b/drivers/staging/media/as102/as102_drv.c index 09d64cd..e0ee618 100644 --- a/drivers/staging/media/as102/as102_drv.c +++ b/drivers/staging/media/as102/as102_drv.c @@ -31,10 +31,6 @@ #include as102_fw.h #include dvbdev.h -int as102_debug; -module_param_named(debug, as102_debug, int, 0644); -MODULE_PARM_DESC(debug, Turn on/off debugging (default: off)); - int dual_tuner; module_param_named(dual_tuner, dual_tuner, int, 0644); MODULE_PARM_DESC(dual_tuner, Activate Dual-Tuner config (default: off)); @@ -74,7 +70,8 @@ static void as102_stop_stream(struct as102_dev_t *dev) return; if (as10x_cmd_stop_streaming(bus_adap) 0) - dprintk(debug, as10x_cmd_stop_streaming failed\n); + dev_dbg(dev-bus_adap.usb_dev-dev, + as10x_cmd_stop_streaming failed\n); mutex_unlock(dev-bus_adap.lock); } @@ -112,14 +109,16 @@ static int as10x_pid_filter(struct as102_dev_t *dev, int ret = -EFAULT; if (mutex_lock_interruptible(dev-bus_adap.lock)) { - dprintk(debug, mutex_lock_interruptible(lock) failed !\n); + dev_dbg(dev-bus_adap.usb_dev-dev, + amutex_lock_interruptible(lock) failed !\n); return -EBUSY; } switch (onoff) { case 0: ret = as10x_cmd_del_PID_filter(bus_adap, (uint16_t) pid); - dprintk(debug, DEL_PID_FILTER([%02d] 0x%04x) ret = %d\n, + dev_dbg(dev-bus_adap.usb_dev-dev, + DEL_PID_FILTER([%02d] 0x%04x) ret = %d\n, index, pid, ret); break; case 1: @@ -131,7 +130,7 @@ static int as10x_pid_filter(struct as102_dev_t *dev, filter.pid = pid; ret = as10x_cmd_add_PID_filter(bus_adap, filter); - dprintk(debug, + dev_dbg(dev-bus_adap.usb_dev-dev, ADD_PID_FILTER([%02d - %02d], 0x%04x) ret = %d\n, index, filter.idx, filter.pid, ret); break; diff --git a/drivers/staging/media/as102/as102_drv.h b/drivers/staging/media/as102/as102_drv.h index a06837d..49d0c42 100644 --- a/drivers/staging/media/as102/as102_drv.h +++ b/drivers/staging/media/as102/as102_drv.h @@ -27,17 +27,10 @@ #define DRIVER_FULL_NAME Abilis Systems as10x usb driver #define DRIVER_NAME as10x_usb -extern int as102_debug; #define debug as102_debug extern struct usb_driver as102_usb_driver; extern int elna_enable; -#define dprintk(debug, args...) \ - do { if (debug) { \ - pr_debug(%s: , __func__); \ - printk(args); \ - } } while (0) - #define AS102_DEVICE_MAJOR 192 #define AS102_USB_BUF_SIZE 512 diff --git a/drivers/staging/media/as102/as102_fe.c b/drivers/staging/media/as102/as102_fe.c index b686b76..69ffd51 100644 --- a/drivers/staging/media/as102/as102_fe.c +++ b/drivers/staging/media/as102/as102_fe.c @@ -46,7 +46,8 @@ static int as102_fe_set_frontend(struct dvb_frontend *fe) /* send abilis command: SET_TUNE */ ret = as10x_cmd_set_tune(dev-bus_adap, tune_args); if (ret != 0) - dprintk(debug, as10x_cmd_set_tune failed. (err = %d)\n, ret); + dev_dbg(dev-bus_adap.usb_dev-dev, + as10x_cmd_set_tune failed. (err = %d)\n, ret); mutex_unlock(dev-bus_adap.lock); @@ -82,10 +83,17 @@ static int as102_fe_get_tune_settings(struct dvb_frontend *fe, struct dvb_frontend_tune_settings *settings) { #if 0 - dprintk(debug, step_size= %d\n, settings-step_size); - dprintk(debug, max_drift= %d\n, settings-max_drift); - dprintk(debug, min_delay_ms = %d - %d\n, settings-min_delay_ms, - 1000); + struct as102_dev_t *dev; + + dev = (struct as102_dev_t *) fe-tuner_priv; + if (dev == NULL) + return -EINVAL; + dev_dbg(dev-bus_adap.usb_dev-dev, + step_size= %d\n, settings-step_size); + dev_dbg(dev-bus_adap.usb_dev-dev, + max_drift= %d\n, settings-max_drift); + dev_dbg(dev-bus_adap.usb_dev-dev, + min_delay_ms = %d - %d\n, settings-min_delay_ms, 1000); #endif
cron job: media_tree daily build: WARNINGS
This message is generated daily by a cron job that builds media_tree for the kernels and architectures in the list below. Results of the daily build of media_tree: date: Mon Aug 4 04:00:14 CEST 2014 git branch: test git hash: 0f3bf3dc1ca394a8385079a5653088672b65c5c4 gcc version:i686-linux-gcc (GCC) 4.9.1 sparse version: v0.5.0-16-g1db35d0 host hardware: x86_64 host os:3.15-7.slh.3-amd64 linux-git-arm-at91: OK linux-git-arm-davinci: OK linux-git-arm-exynos: OK linux-git-arm-mx: OK linux-git-arm-omap: OK linux-git-arm-omap1: OK linux-git-arm-pxa: OK linux-git-blackfin: OK linux-git-i686: OK linux-git-m32r: OK linux-git-mips: OK linux-git-powerpc64: OK linux-git-sh: OK linux-git-x86_64: OK linux-2.6.32.27-i686: OK linux-2.6.33.7-i686: OK linux-2.6.34.7-i686: OK linux-2.6.35.9-i686: OK linux-2.6.36.4-i686: OK linux-2.6.37.6-i686: OK linux-2.6.38.8-i686: OK linux-2.6.39.4-i686: OK linux-3.0.60-i686: OK linux-3.1.10-i686: OK linux-3.2.37-i686: OK linux-3.3.8-i686: OK linux-3.4.27-i686: OK linux-3.5.7-i686: OK linux-3.6.11-i686: OK linux-3.7.4-i686: OK linux-3.8-i686: OK linux-3.9.2-i686: OK linux-3.10.1-i686: OK linux-3.11.1-i686: OK linux-3.12.23-i686: OK linux-3.13.11-i686: OK linux-3.14.9-i686: OK linux-3.15.2-i686: OK linux-3.16-rc1-i686: OK linux-2.6.32.27-x86_64: OK linux-2.6.33.7-x86_64: OK linux-2.6.34.7-x86_64: OK linux-2.6.35.9-x86_64: OK linux-2.6.36.4-x86_64: OK linux-2.6.37.6-x86_64: OK linux-2.6.38.8-x86_64: OK linux-2.6.39.4-x86_64: OK linux-3.0.60-x86_64: OK linux-3.1.10-x86_64: OK linux-3.2.37-x86_64: OK linux-3.3.8-x86_64: OK linux-3.4.27-x86_64: OK linux-3.5.7-x86_64: OK linux-3.6.11-x86_64: OK linux-3.7.4-x86_64: OK linux-3.8-x86_64: OK linux-3.9.2-x86_64: OK linux-3.10.1-x86_64: OK linux-3.11.1-x86_64: OK linux-3.12.23-x86_64: OK linux-3.13.11-x86_64: OK linux-3.14.9-x86_64: OK linux-3.15.2-x86_64: OK linux-3.16-rc1-x86_64: OK apps: WARNINGS spec-git: OK sparse: WARNINGS Detailed results are available here: http://www.xs4all.nl/~hverkuil/logs/Monday.log Full logs are available here: http://www.xs4all.nl/~hverkuil/logs/Monday.tar.bz2 The Media Infrastructure API from this daily build is here: http://www.xs4all.nl/~hverkuil/spec/media.html -- 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
[PATCH] v4l2: Change call of function in videobuf2-core.c
This patch changes the call of vb2_buffer_core to use VB2_BUFFER_STATE_ACTIVE inside the for instead of not setting in correctly to VB2_BUFFER_STATE_ERROR. Signed-off-by: Nicholas Krause xerofo...@gmail.com --- drivers/media/v4l2-core/videobuf2-core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c index 7c4489c..08e478b 100644 --- a/drivers/media/v4l2-core/videobuf2-core.c +++ b/drivers/media/v4l2-core/videobuf2-core.c @@ -2115,7 +2115,7 @@ static void __vb2_queue_cancel(struct vb2_queue *q) if (WARN_ON(atomic_read(q-owned_by_drv_count))) { for (i = 0; i q-num_buffers; ++i) if (q-bufs[i]-state == VB2_BUF_STATE_ACTIVE) - vb2_buffer_done(q-bufs[i], VB2_BUF_STATE_ERROR); + vb2_buffer_done(q-bufs[i], VB2_BUF_STATE_ACTIVE); /* Must be zero now */ WARN_ON(atomic_read(q-owned_by_drv_count)); } -- 1.9.1 -- 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: [PATCH] v4l2: Change call of function in videobuf2-core.c
On 4 August 2014 13:25, Nicholas Krause xerofo...@gmail.com wrote: This patch changes the call of vb2_buffer_core to use VB2_BUFFER_STATE_ACTIVE inside the for instead of not setting in correctly to VB2_BUFFER_STATE_ERROR. Please go back and read every mail sent to you in the last few weeks. then read them again, go nuts read them again. Still wondering where I'm going with this? read them again. then understand that I mean this in the nicest way possible. please fuck off. you are wasting developers time. Dave. -- 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
[PATCH 2/9] ddbridge: I2C client for tda18212
Used tda18212 tuner is implemented as a I2C driver. Implement I2C client to ddbridge and use it for tda18212. Signed-off-by: Antti Palosaari cr...@iki.fi --- drivers/media/pci/ddbridge/ddbridge-core.c | 70 +- drivers/media/pci/ddbridge/ddbridge.h | 1 + 2 files changed, 40 insertions(+), 31 deletions(-) diff --git a/drivers/media/pci/ddbridge/ddbridge-core.c b/drivers/media/pci/ddbridge/ddbridge-core.c index c66b1b3..ccd6a57 100644 --- a/drivers/media/pci/ddbridge/ddbridge-core.c +++ b/drivers/media/pci/ddbridge/ddbridge-core.c @@ -834,45 +834,47 @@ static int tuner_attach_tda18271(struct ddb_input *input) return 0; } -static struct tda18212_config tda18212_config_60 = { - .i2c_address = 0x60, - .if_dvbt_6 = 3550, - .if_dvbt_7 = 3700, - .if_dvbt_8 = 4150, - .if_dvbt2_6 = 3250, - .if_dvbt2_7 = 4000, - .if_dvbt2_8 = 4000, - .if_dvbc = 5000, -}; - -static struct tda18212_config tda18212_config_63 = { - .i2c_address = 0x63, - .if_dvbt_6 = 3550, - .if_dvbt_7 = 3700, - .if_dvbt_8 = 4150, - .if_dvbt2_6 = 3250, - .if_dvbt2_7 = 4000, - .if_dvbt2_8 = 4000, - .if_dvbc = 5000, -}; - static int tuner_attach_tda18212(struct ddb_input *input) { - struct i2c_adapter *i2c = input-port-i2c-adap; + struct i2c_adapter *adapter = input-port-i2c-adap; struct ddb_dvb *dvb = input-port-dvb[input-nr 1]; - struct dvb_frontend *fe; - struct tda18212_config *config; + struct i2c_client *client; + struct tda18212_config config = { + .fe = dvb-fe, + .if_dvbt_6 = 3550, + .if_dvbt_7 = 3700, + .if_dvbt_8 = 4150, + .if_dvbt2_6 = 3250, + .if_dvbt2_7 = 4000, + .if_dvbt2_8 = 4000, + .if_dvbc = 5000, + }; + struct i2c_board_info board_info = { + .type = tda18212, + .platform_data = config, + }; if (input-nr 1) - config = tda18212_config_63; + board_info.addr = 0x63; else - config = tda18212_config_60; + board_info.addr = 0x60; - fe = dvb_attach(tda18212_attach, dvb-fe, i2c, config); - if (!fe) { - pr_err(No TDA18212 found!\n); - return -ENODEV; + request_module(board_info.type); + + client = i2c_new_device(adapter, board_info); + if (client == NULL || client-dev.driver == NULL) + goto err; + + if (!try_module_get(client-dev.driver-owner)) { + i2c_unregister_device(client); + goto err; } + + dvb-i2c_client[0] = client; + + return 0; +err: + dev_notice(input-port-dev-dev, TDA18212 tuner not found. Device is not fully operational.\n); return 0; } @@ -1059,9 +1061,15 @@ static void dvb_input_detach(struct ddb_input *input) { struct ddb_dvb *dvb = input-port-dvb[input-nr 1]; struct dvb_demux *dvbdemux = dvb-demux; + struct i2c_client *client; switch (dvb-attached) { case 0x31: + client = dvb-i2c_client[0]; + if (client) { + module_put(client-dev.driver-owner); + i2c_unregister_device(client); + } if (dvb-fe2) dvb_unregister_frontend(dvb-fe2); if (dvb-fe) diff --git a/drivers/media/pci/ddbridge/ddbridge.h b/drivers/media/pci/ddbridge/ddbridge.h index 601cf24..2f98ac6 100644 --- a/drivers/media/pci/ddbridge/ddbridge.h +++ b/drivers/media/pci/ddbridge/ddbridge.h @@ -181,6 +181,7 @@ struct ddb_dvb { struct dvb_adapter*adap; intadap_registered; struct dvb_device *dev; + struct i2c_client *i2c_client[1]; struct dvb_frontend *fe; struct dvb_frontend *fe2; struct dmxdev dmxdev; -- http://palosaari.fi/ -- 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
[PATCH 4/9] em28xx: convert tda18212 tuner to I2C client
Used tda18212 tuner is implemented as a I2C driver. Use em28xx tuner I2C client for tda18212 driver. Signed-off-by: Antti Palosaari cr...@iki.fi --- drivers/media/usb/em28xx/em28xx-dvb.c | 32 ++-- 1 file changed, 26 insertions(+), 6 deletions(-) diff --git a/drivers/media/usb/em28xx/em28xx-dvb.c b/drivers/media/usb/em28xx/em28xx-dvb.c index 3a3e243..7d62ff5 100644 --- a/drivers/media/usb/em28xx/em28xx-dvb.c +++ b/drivers/media/usb/em28xx/em28xx-dvb.c @@ -373,7 +373,6 @@ static struct tda18271_config kworld_ub435q_v2_config = { }; static struct tda18212_config kworld_ub435q_v3_config = { - .i2c_address= 0x60, .if_atsc_vsb= 3600, .if_atsc_qam= 3600, }; @@ -1435,6 +1434,15 @@ static int em28xx_dvb_init(struct em28xx *dev) } break; case EM2874_BOARD_KWORLD_UB435Q_V3: + { + struct i2c_client *client; + struct i2c_adapter *adapter = dev-i2c_adap[dev-def_i2c_bus]; + struct i2c_board_info board_info = { + .type = tda18212, + .addr = 0x60, + .platform_data = kworld_ub435q_v3_config, + }; + dvb-fe[0] = dvb_attach(lgdt3305_attach, em2874_lgdt3305_nogate_dev, dev-i2c_adap[dev-def_i2c_bus]); @@ -1443,14 +1451,26 @@ static int em28xx_dvb_init(struct em28xx *dev) goto out_free; } - /* Attach the demodulator. */ - if (!dvb_attach(tda18212_attach, dvb-fe[0], - dev-i2c_adap[dev-def_i2c_bus], - kworld_ub435q_v3_config)) { - result = -EINVAL; + /* attach tuner */ + kworld_ub435q_v3_config.fe = dvb-fe[0]; + request_module(tda18212); + client = i2c_new_device(adapter, board_info); + if (client == NULL || client-dev.driver == NULL) { + dvb_frontend_detach(dvb-fe[0]); + result = -ENODEV; goto out_free; } + + if (!try_module_get(client-dev.driver-owner)) { + i2c_unregister_device(client); + dvb_frontend_detach(dvb-fe[0]); + result = -ENODEV; + goto out_free; + } + + dvb-i2c_client_tuner = client; break; + } case EM2874_BOARD_PCTV_HD_MINI_80E: dvb-fe[0] = dvb_attach(drx39xxj_attach, dev-i2c_adap[dev-def_i2c_bus]); if (dvb-fe[0] != NULL) { -- http://palosaari.fi/ -- 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
[PATCH 8/9] tda18212: convert to RegMap API
Use RegMap API to handle all the boring I2C register access boilerplate stuff. Signed-off-by: Antti Palosaari cr...@iki.fi --- drivers/media/tuners/Kconfig| 1 + drivers/media/tuners/tda18212.c | 131 ++-- 2 files changed, 18 insertions(+), 114 deletions(-) diff --git a/drivers/media/tuners/Kconfig b/drivers/media/tuners/Kconfig index d79fd1c..483963d 100644 --- a/drivers/media/tuners/Kconfig +++ b/drivers/media/tuners/Kconfig @@ -204,6 +204,7 @@ config MEDIA_TUNER_FC0013 config MEDIA_TUNER_TDA18212 tristate NXP TDA18212 silicon tuner depends on MEDIA_SUPPORT I2C + select REGMAP_I2C default m if !MEDIA_SUBDRV_AUTOSELECT help NXP TDA18212 silicon tuner driver. diff --git a/drivers/media/tuners/tda18212.c b/drivers/media/tuners/tda18212.c index 95a5ebf..c7c476e 100644 --- a/drivers/media/tuners/tda18212.c +++ b/drivers/media/tuners/tda18212.c @@ -19,122 +19,16 @@ */ #include tda18212.h - -/* Max transfer size done by I2C transfer functions */ -#define MAX_XFER_SIZE 64 +#include linux/regmap.h struct tda18212 { struct tda18212_config cfg; struct i2c_client *client; + struct regmap *regmap; u32 if_frequency; }; -/* write multiple registers */ -static int tda18212_wr_regs(struct tda18212 *s, u8 reg, u8 *val, int len) -{ - int ret; - u8 buf[MAX_XFER_SIZE]; - struct i2c_msg msg[1] = { - { - .addr = s-client-addr, - .flags = 0, - .len = 1 + len, - .buf = buf, - } - }; - - if (1 + len sizeof(buf)) { - dev_warn(s-client-dev, - i2c wr reg=%04x: len=%d is too big!\n, - reg, len); - return -EINVAL; - } - - buf[0] = reg; - memcpy(buf[1], val, len); - - ret = i2c_transfer(s-client-adapter, msg, 1); - if (ret == 1) { - ret = 0; - } else { - dev_warn(s-client-dev, - i2c wr failed=%d reg=%02x len=%d\n, - ret, reg, len); - ret = -EREMOTEIO; - } - return ret; -} - -/* read multiple registers */ -static int tda18212_rd_regs(struct tda18212 *s, u8 reg, u8 *val, int len) -{ - int ret; - u8 buf[MAX_XFER_SIZE]; - struct i2c_msg msg[2] = { - { - .addr = s-client-addr, - .flags = 0, - .len = 1, - .buf = reg, - }, { - .addr = s-client-addr, - .flags = I2C_M_RD, - .len = len, - .buf = buf, - } - }; - - if (len sizeof(buf)) { - dev_warn(s-client-dev, - i2c rd reg=%04x: len=%d is too big!\n, - reg, len); - return -EINVAL; - } - - ret = i2c_transfer(s-client-adapter, msg, 2); - if (ret == 2) { - memcpy(val, buf, len); - ret = 0; - } else { - dev_warn(s-client-dev, - i2c rd failed=%d reg=%02x len=%d\n, - ret, reg, len); - ret = -EREMOTEIO; - } - - return ret; -} - -/* write single register */ -static int tda18212_wr_reg(struct tda18212 *s, u8 reg, u8 val) -{ - return tda18212_wr_regs(s, reg, val, 1); -} - -/* read single register */ -static int tda18212_rd_reg(struct tda18212 *s, u8 reg, u8 *val) -{ - return tda18212_rd_regs(s, reg, val, 1); -} - -#if 0 /* keep, useful when developing driver */ -static void tda18212_dump_regs(struct tda18212 *s) -{ - int i; - u8 buf[256]; - - #define TDA18212_RD_LEN 32 - for (i = 0; i sizeof(buf); i += TDA18212_RD_LEN) - tda18212_rd_regs(s, i, buf[i], TDA18212_RD_LEN); - - print_hex_dump(KERN_INFO, , DUMP_PREFIX_OFFSET, 32, 1, buf, - sizeof(buf), true); - - return; -} -#endif - static int tda18212_set_params(struct dvb_frontend *fe) { struct tda18212 *s = fe-tuner_priv; @@ -231,15 +125,15 @@ static int tda18212_set_params(struct dvb_frontend *fe) goto error; } - ret = tda18212_wr_reg(s, 0x23, bw_params[i][2]); + ret = regmap_write(s-regmap, 0x23, bw_params[i][2]); if (ret) goto error; - ret = tda18212_wr_reg(s, 0x06, 0x00); + ret = regmap_write(s-regmap, 0x06, 0x00); if (ret) goto error; - ret = tda18212_wr_reg(s, 0x0f, bw_params[i][0]); + ret = regmap_write(s-regmap, 0x0f, bw_params[i][0]); if (ret) goto error; @@ -252,7 +146,7 @@ static int tda18212_set_params(struct dvb_frontend *fe)
[PATCH 1/9] tda18212: prepare for I2C client conversion
We need carry pointer to frontend via config struct (I2C platform_data ptr) when I2C model is used. Add that pointer first in order to keep build unbreakable during conversion. Signed-off-by: Antti Palosaari cr...@iki.fi --- drivers/media/tuners/tda18212.h | 5 + 1 file changed, 5 insertions(+) diff --git a/drivers/media/tuners/tda18212.h b/drivers/media/tuners/tda18212.h index c36b49e..265559a 100644 --- a/drivers/media/tuners/tda18212.h +++ b/drivers/media/tuners/tda18212.h @@ -37,6 +37,11 @@ struct tda18212_config { u16 if_dvbc; u16 if_atsc_vsb; u16 if_atsc_qam; + + /* +* pointer to DVB frontend +*/ + struct dvb_frontend *fe; }; #if IS_ENABLED(CONFIG_MEDIA_TUNER_TDA18212) -- http://palosaari.fi/ -- 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
[PATCH 9/9] ddbridge: fix I2C adapter capabilities
It is I2C adapter, not SMBUS, which could do some simple SMBUS operations. Report is as a I2C capable too. Wrong reported type causes RegMap to fail write multiple registers using I2C register address auto-increment. Signed-off-by: Antti Palosaari cr...@iki.fi --- drivers/media/pci/ddbridge/ddbridge-i2c.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/pci/ddbridge/ddbridge-i2c.c b/drivers/media/pci/ddbridge/ddbridge-i2c.c index 6845d2a..121a12b 100644 --- a/drivers/media/pci/ddbridge/ddbridge-i2c.c +++ b/drivers/media/pci/ddbridge/ddbridge-i2c.c @@ -163,7 +163,7 @@ static int ddb_i2c_master_xfer(struct i2c_adapter *adapter, static u32 ddb_i2c_functionality(struct i2c_adapter *adap) { - return I2C_FUNC_SMBUS_EMUL; + return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL; } struct i2c_algorithm ddb_i2c_algo = { -- http://palosaari.fi/ -- 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
[PATCH 7/9] tda18212: rename state from 'priv' to 's'
Rename driver state pointer to 's' through whole driver, just because I like use shortest possible name for such common struct used everywhere in the driver. Signed-off-by: Antti Palosaari cr...@iki.fi --- drivers/media/tuners/tda18212.c | 104 1 file changed, 51 insertions(+), 53 deletions(-) diff --git a/drivers/media/tuners/tda18212.c b/drivers/media/tuners/tda18212.c index 5d1d785..95a5ebf 100644 --- a/drivers/media/tuners/tda18212.c +++ b/drivers/media/tuners/tda18212.c @@ -23,7 +23,7 @@ /* Max transfer size done by I2C transfer functions */ #define MAX_XFER_SIZE 64 -struct tda18212_priv { +struct tda18212 { struct tda18212_config cfg; struct i2c_client *client; @@ -31,14 +31,13 @@ struct tda18212_priv { }; /* write multiple registers */ -static int tda18212_wr_regs(struct tda18212_priv *priv, u8 reg, u8 *val, - int len) +static int tda18212_wr_regs(struct tda18212 *s, u8 reg, u8 *val, int len) { int ret; u8 buf[MAX_XFER_SIZE]; struct i2c_msg msg[1] = { { - .addr = priv-client-addr, + .addr = s-client-addr, .flags = 0, .len = 1 + len, .buf = buf, @@ -46,7 +45,7 @@ static int tda18212_wr_regs(struct tda18212_priv *priv, u8 reg, u8 *val, }; if (1 + len sizeof(buf)) { - dev_warn(priv-client-dev, + dev_warn(s-client-dev, i2c wr reg=%04x: len=%d is too big!\n, reg, len); return -EINVAL; @@ -55,11 +54,11 @@ static int tda18212_wr_regs(struct tda18212_priv *priv, u8 reg, u8 *val, buf[0] = reg; memcpy(buf[1], val, len); - ret = i2c_transfer(priv-client-adapter, msg, 1); + ret = i2c_transfer(s-client-adapter, msg, 1); if (ret == 1) { ret = 0; } else { - dev_warn(priv-client-dev, + dev_warn(s-client-dev, i2c wr failed=%d reg=%02x len=%d\n, ret, reg, len); ret = -EREMOTEIO; @@ -68,19 +67,18 @@ static int tda18212_wr_regs(struct tda18212_priv *priv, u8 reg, u8 *val, } /* read multiple registers */ -static int tda18212_rd_regs(struct tda18212_priv *priv, u8 reg, u8 *val, - int len) +static int tda18212_rd_regs(struct tda18212 *s, u8 reg, u8 *val, int len) { int ret; u8 buf[MAX_XFER_SIZE]; struct i2c_msg msg[2] = { { - .addr = priv-client-addr, + .addr = s-client-addr, .flags = 0, .len = 1, .buf = reg, }, { - .addr = priv-client-addr, + .addr = s-client-addr, .flags = I2C_M_RD, .len = len, .buf = buf, @@ -88,18 +86,18 @@ static int tda18212_rd_regs(struct tda18212_priv *priv, u8 reg, u8 *val, }; if (len sizeof(buf)) { - dev_warn(priv-client-dev, + dev_warn(s-client-dev, i2c rd reg=%04x: len=%d is too big!\n, reg, len); return -EINVAL; } - ret = i2c_transfer(priv-client-adapter, msg, 2); + ret = i2c_transfer(s-client-adapter, msg, 2); if (ret == 2) { memcpy(val, buf, len); ret = 0; } else { - dev_warn(priv-client-dev, + dev_warn(s-client-dev, i2c rd failed=%d reg=%02x len=%d\n, ret, reg, len); ret = -EREMOTEIO; @@ -109,26 +107,26 @@ static int tda18212_rd_regs(struct tda18212_priv *priv, u8 reg, u8 *val, } /* write single register */ -static int tda18212_wr_reg(struct tda18212_priv *priv, u8 reg, u8 val) +static int tda18212_wr_reg(struct tda18212 *s, u8 reg, u8 val) { - return tda18212_wr_regs(priv, reg, val, 1); + return tda18212_wr_regs(s, reg, val, 1); } /* read single register */ -static int tda18212_rd_reg(struct tda18212_priv *priv, u8 reg, u8 *val) +static int tda18212_rd_reg(struct tda18212 *s, u8 reg, u8 *val) { - return tda18212_rd_regs(priv, reg, val, 1); + return tda18212_rd_regs(s, reg, val, 1); } #if 0 /* keep, useful when developing driver */ -static void tda18212_dump_regs(struct tda18212_priv *priv) +static void tda18212_dump_regs(struct tda18212 *s) { int i; u8 buf[256]; #define TDA18212_RD_LEN 32 for (i = 0; i sizeof(buf); i += TDA18212_RD_LEN) - tda18212_rd_regs(priv, i, buf[i], TDA18212_RD_LEN); + tda18212_rd_regs(s, i, buf[i], TDA18212_RD_LEN);
[PATCH 5/9] tda18212: convert driver to I2C binding
Convert driver from DVB proprietary model to common I2C model. Signed-off-by: Antti Palosaari cr...@iki.fi --- drivers/media/tuners/tda18212.c | 129 drivers/media/tuners/tda18212.h | 14 - 2 files changed, 79 insertions(+), 64 deletions(-) diff --git a/drivers/media/tuners/tda18212.c b/drivers/media/tuners/tda18212.c index 15b09f8..659787b 100644 --- a/drivers/media/tuners/tda18212.c +++ b/drivers/media/tuners/tda18212.c @@ -24,8 +24,8 @@ #define MAX_XFER_SIZE 64 struct tda18212_priv { - struct tda18212_config *cfg; - struct i2c_adapter *i2c; + struct tda18212_config cfg; + struct i2c_client *client; u32 if_frequency; }; @@ -38,7 +38,7 @@ static int tda18212_wr_regs(struct tda18212_priv *priv, u8 reg, u8 *val, u8 buf[MAX_XFER_SIZE]; struct i2c_msg msg[1] = { { - .addr = priv-cfg-i2c_address, + .addr = priv-client-addr, .flags = 0, .len = 1 + len, .buf = buf, @@ -46,7 +46,7 @@ static int tda18212_wr_regs(struct tda18212_priv *priv, u8 reg, u8 *val, }; if (1 + len sizeof(buf)) { - dev_warn(priv-i2c-dev, + dev_warn(priv-client-dev, %s: i2c wr reg=%04x: len=%d is too big!\n, KBUILD_MODNAME, reg, len); return -EINVAL; @@ -55,12 +55,12 @@ static int tda18212_wr_regs(struct tda18212_priv *priv, u8 reg, u8 *val, buf[0] = reg; memcpy(buf[1], val, len); - ret = i2c_transfer(priv-i2c, msg, 1); + ret = i2c_transfer(priv-client-adapter, msg, 1); if (ret == 1) { ret = 0; } else { - dev_warn(priv-i2c-dev, %s: i2c wr failed=%d reg=%02x \ - len=%d\n, KBUILD_MODNAME, ret, reg, len); + dev_warn(priv-client-dev, %s: i2c wr failed=%d reg=%02x len=%d\n, + KBUILD_MODNAME, ret, reg, len); ret = -EREMOTEIO; } return ret; @@ -74,12 +74,12 @@ static int tda18212_rd_regs(struct tda18212_priv *priv, u8 reg, u8 *val, u8 buf[MAX_XFER_SIZE]; struct i2c_msg msg[2] = { { - .addr = priv-cfg-i2c_address, + .addr = priv-client-addr, .flags = 0, .len = 1, .buf = reg, }, { - .addr = priv-cfg-i2c_address, + .addr = priv-client-addr, .flags = I2C_M_RD, .len = len, .buf = buf, @@ -87,19 +87,19 @@ static int tda18212_rd_regs(struct tda18212_priv *priv, u8 reg, u8 *val, }; if (len sizeof(buf)) { - dev_warn(priv-i2c-dev, + dev_warn(priv-client-dev, %s: i2c rd reg=%04x: len=%d is too big!\n, KBUILD_MODNAME, reg, len); return -EINVAL; } - ret = i2c_transfer(priv-i2c, msg, 2); + ret = i2c_transfer(priv-client-adapter, msg, 2); if (ret == 2) { memcpy(val, buf, len); ret = 0; } else { - dev_warn(priv-i2c-dev, %s: i2c rd failed=%d reg=%02x \ - len=%d\n, KBUILD_MODNAME, ret, reg, len); + dev_warn(priv-client-dev, %s: i2c rd failed=%d reg=%02x len=%d\n, + KBUILD_MODNAME, ret, reg, len); ret = -EREMOTEIO; } @@ -166,7 +166,7 @@ static int tda18212_set_params(struct dvb_frontend *fe) [ATSC_QAM] = { 0x7d, 0x20, 0x63 }, }; - dev_dbg(priv-i2c-dev, + dev_dbg(priv-client-dev, %s: delivery_system=%d frequency=%d bandwidth_hz=%d\n, __func__, c-delivery_system, c-frequency, c-bandwidth_hz); @@ -176,25 +176,25 @@ static int tda18212_set_params(struct dvb_frontend *fe) switch (c-delivery_system) { case SYS_ATSC: - if_khz = priv-cfg-if_atsc_vsb; + if_khz = priv-cfg.if_atsc_vsb; i = ATSC_VSB; break; case SYS_DVBC_ANNEX_B: - if_khz = priv-cfg-if_atsc_qam; + if_khz = priv-cfg.if_atsc_qam; i = ATSC_QAM; break; case SYS_DVBT: switch (c-bandwidth_hz) { case 600: - if_khz = priv-cfg-if_dvbt_6; + if_khz = priv-cfg.if_dvbt_6; i = DVBT_6; break; case 700: - if_khz = priv-cfg-if_dvbt_7; + if_khz = priv-cfg.if_dvbt_7;
[PATCH 6/9] tda18212: clean logging
There is no need to print module name nor function name as those are done by kernel logging system when dev_xxx logging is used and driver is proper I2C driver. Signed-off-by: Antti Palosaari cr...@iki.fi --- drivers/media/tuners/tda18212.c | 35 ++- 1 file changed, 18 insertions(+), 17 deletions(-) diff --git a/drivers/media/tuners/tda18212.c b/drivers/media/tuners/tda18212.c index 659787b..5d1d785 100644 --- a/drivers/media/tuners/tda18212.c +++ b/drivers/media/tuners/tda18212.c @@ -47,8 +47,8 @@ static int tda18212_wr_regs(struct tda18212_priv *priv, u8 reg, u8 *val, if (1 + len sizeof(buf)) { dev_warn(priv-client-dev, -%s: i2c wr reg=%04x: len=%d is too big!\n, -KBUILD_MODNAME, reg, len); + i2c wr reg=%04x: len=%d is too big!\n, + reg, len); return -EINVAL; } @@ -59,8 +59,9 @@ static int tda18212_wr_regs(struct tda18212_priv *priv, u8 reg, u8 *val, if (ret == 1) { ret = 0; } else { - dev_warn(priv-client-dev, %s: i2c wr failed=%d reg=%02x len=%d\n, - KBUILD_MODNAME, ret, reg, len); + dev_warn(priv-client-dev, + i2c wr failed=%d reg=%02x len=%d\n, + ret, reg, len); ret = -EREMOTEIO; } return ret; @@ -88,8 +89,8 @@ static int tda18212_rd_regs(struct tda18212_priv *priv, u8 reg, u8 *val, if (len sizeof(buf)) { dev_warn(priv-client-dev, -%s: i2c rd reg=%04x: len=%d is too big!\n, -KBUILD_MODNAME, reg, len); + i2c rd reg=%04x: len=%d is too big!\n, + reg, len); return -EINVAL; } @@ -98,8 +99,9 @@ static int tda18212_rd_regs(struct tda18212_priv *priv, u8 reg, u8 *val, memcpy(val, buf, len); ret = 0; } else { - dev_warn(priv-client-dev, %s: i2c rd failed=%d reg=%02x len=%d\n, - KBUILD_MODNAME, ret, reg, len); + dev_warn(priv-client-dev, + i2c rd failed=%d reg=%02x len=%d\n, + ret, reg, len); ret = -EREMOTEIO; } @@ -167,8 +169,8 @@ static int tda18212_set_params(struct dvb_frontend *fe) }; dev_dbg(priv-client-dev, - %s: delivery_system=%d frequency=%d bandwidth_hz=%d\n, - __func__, c-delivery_system, c-frequency, + delivery_system=%d frequency=%d bandwidth_hz=%d\n, + c-delivery_system, c-frequency, c-bandwidth_hz); if (fe-ops.i2c_gate_ctrl) @@ -266,7 +268,7 @@ exit: return ret; error: - dev_dbg(priv-client-dev, %s: failed=%d\n, __func__, ret); + dev_dbg(priv-client-dev, failed=%d\n, ret); goto exit; } @@ -305,7 +307,7 @@ static int tda18212_probe(struct i2c_client *client, priv = kzalloc(sizeof(*priv), GFP_KERNEL); if (!priv) { ret = -ENOMEM; - dev_err(client-dev, %s: kzalloc() failed\n, KBUILD_MODNAME); + dev_err(client-dev, kzalloc() failed\n); goto err; } @@ -317,7 +319,7 @@ static int tda18212_probe(struct i2c_client *client, fe-ops.i2c_gate_ctrl(fe, 1); /* open I2C-gate */ ret = tda18212_rd_reg(priv, 0x00, chip_id); - dev_dbg(priv-client-dev, %s: chip_id=%02x\n, __func__, chip_id); + dev_dbg(priv-client-dev, chip_id=%02x\n, chip_id); if (fe-ops.i2c_gate_ctrl) fe-ops.i2c_gate_ctrl(fe, 0); /* close I2C-gate */ @@ -338,8 +340,7 @@ static int tda18212_probe(struct i2c_client *client, } dev_info(priv-client-dev, - %s: NXP TDA18212HN/%s successfully identified\n, - KBUILD_MODNAME, version); + NXP TDA18212HN/%s successfully identified\n, version); fe-tuner_priv = priv; memcpy(fe-ops.tuner_ops, tda18212_tuner_ops, @@ -348,7 +349,7 @@ static int tda18212_probe(struct i2c_client *client, return 0; err: - dev_dbg(client-dev, %s: failed=%d\n, __func__, ret); + dev_dbg(client-dev, failed=%d\n, ret); kfree(priv); return ret; } @@ -358,7 +359,7 @@ static int tda18212_remove(struct i2c_client *client) struct tda18212_priv *priv = i2c_get_clientdata(client); struct dvb_frontend *fe = priv-cfg.fe; - dev_dbg(client-dev, %s:\n, __func__); + dev_dbg(client-dev, \n); memset(fe-ops.tuner_ops, 0, sizeof(struct dvb_tuner_ops)); fe-tuner_priv = NULL; -- http://palosaari.fi/ -- To unsubscribe from this list:
Re: [PATCH] v4l2: Change call of function in videobuf2-core.c
On 08/04/2014 05:25 AM, Nicholas Krause wrote: This patch changes the call of vb2_buffer_core to use VB2_BUFFER_STATE_ACTIVE inside the for instead of not setting in correctly to VB2_BUFFER_STATE_ERROR. Signed-off-by: Nicholas Krause xerofo...@gmail.com Dunno what's going on here after reading Dave Airlie's reply, but: Nacked-by: Hans Verkuil hans.verk...@cisco.com It's clearly wrong and if you get here at all you have a driver bug anyway. That WARN_ON is there for a reason. Your driver isn't returning buffers correctly in stop_streaming or in start_streaming if start_streaming fails with an error. Regards, Hans --- drivers/media/v4l2-core/videobuf2-core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c index 7c4489c..08e478b 100644 --- a/drivers/media/v4l2-core/videobuf2-core.c +++ b/drivers/media/v4l2-core/videobuf2-core.c @@ -2115,7 +2115,7 @@ static void __vb2_queue_cancel(struct vb2_queue *q) if (WARN_ON(atomic_read(q-owned_by_drv_count))) { for (i = 0; i q-num_buffers; ++i) if (q-bufs[i]-state == VB2_BUF_STATE_ACTIVE) - vb2_buffer_done(q-bufs[i], VB2_BUF_STATE_ERROR); + vb2_buffer_done(q-bufs[i], VB2_BUF_STATE_ACTIVE); /* Must be zero now */ WARN_ON(atomic_read(q-owned_by_drv_count)); } -- 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: [PATCH] v4l2: Change call of function in videobuf2-core.c
On Mon, Aug 4, 2014 at 1:03 AM, Hans Verkuil hverk...@xs4all.nl wrote: On 08/04/2014 05:25 AM, Nicholas Krause wrote: This patch changes the call of vb2_buffer_core to use VB2_BUFFER_STATE_ACTIVE inside the for instead of not setting in correctly to VB2_BUFFER_STATE_ERROR. Signed-off-by: Nicholas Krause xerofo...@gmail.com Dunno what's going on here after reading Dave Airlie's reply, but: Nacked-by: Hans Verkuil hans.verk...@cisco.com It's clearly wrong and if you get here at all you have a driver bug anyway. That WARN_ON is there for a reason. Your driver isn't returning buffers correctly in stop_streaming or in start_streaming if start_streaming fails with an error. Regards, Hans --- drivers/media/v4l2-core/videobuf2-core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c index 7c4489c..08e478b 100644 --- a/drivers/media/v4l2-core/videobuf2-core.c +++ b/drivers/media/v4l2-core/videobuf2-core.c @@ -2115,7 +2115,7 @@ static void __vb2_queue_cancel(struct vb2_queue *q) if (WARN_ON(atomic_read(q-owned_by_drv_count))) { for (i = 0; i q-num_buffers; ++i) if (q-bufs[i]-state == VB2_BUF_STATE_ACTIVE) - vb2_buffer_done(q-bufs[i], VB2_BUF_STATE_ERROR); + vb2_buffer_done(q-bufs[i], VB2_BUF_STATE_ACTIVE); /* Must be zero now */ WARN_ON(atomic_read(q-owned_by_drv_count)); } Yes , That was an idiot mistake. So sorry about that would someone mind as a big help a list of common debugging macros or a link to somewhere I can read them. I want to apologize sincerely for my bad mistakes. I do want to help out and by helping me sand out my mistakes and learn from them I can help much better. I do want to help and if people are willing to get me grow like this I will continue to try and help. Regards Nick -- 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
[PATCH for v3.17] videobuf2-core: add comments before the WARN_ON
Recently WARN_ON() calls have been added to warn if the driver is not properly returning buffers to vb2 in start_streaming (if it fails) or stop_streaming(). Add comments before those WARN_ON calls that refer to the videobuf2-core.h header that explains what drivers are supposed to do in these situations. That should help point developers in the right direction if they see these warnings. Signed-off-by: Hans Verkuil hans.verk...@cisco.com --- drivers/media/v4l2-core/videobuf2-core.c | 12 1 file changed, 12 insertions(+) diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c index c359006..d3f2a22 100644 --- a/drivers/media/v4l2-core/videobuf2-core.c +++ b/drivers/media/v4l2-core/videobuf2-core.c @@ -1762,6 +1762,12 @@ static int vb2_start_streaming(struct vb2_queue *q) q-start_streaming_called = 0; dprintk(1, driver refused to start streaming\n); + /* +* If you see this warning, then the driver isn't cleaning up properly +* after a failed start_streaming(). See the start_streaming() +* documentation in videobuf2-core.h for more information how buffers +* should be returned to vb2 in start_streaming(). +*/ if (WARN_ON(atomic_read(q-owned_by_drv_count))) { unsigned i; @@ -2123,6 +2129,12 @@ static void __vb2_queue_cancel(struct vb2_queue *q) if (q-start_streaming_called) call_void_qop(q, stop_streaming, q); + /* +* If you see this warning, then the driver isn't cleaning up properly +* in stop_streaming(). See the stop_streaming() documentation in +* videobuf2-core.h for more information how buffers should be returned +* to vb2 in stop_streaming(). +*/ if (WARN_ON(atomic_read(q-owned_by_drv_count))) { for (i = 0; i q-num_buffers; ++i) if (q-bufs[i]-state == VB2_BUF_STATE_ACTIVE) -- 2.0.1 -- 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: [PATCH] v4l2: Change call of function in videobuf2-core.c
On 4 August 2014 15:03, Hans Verkuil hverk...@xs4all.nl wrote: On 08/04/2014 05:25 AM, Nicholas Krause wrote: This patch changes the call of vb2_buffer_core to use VB2_BUFFER_STATE_ACTIVE inside the for instead of not setting in correctly to VB2_BUFFER_STATE_ERROR. Signed-off-by: Nicholas Krause xerofo...@gmail.com Dunno what's going on here after reading Dave Airlie's reply, but: Nick has decided he wants to be a kernel developer, a laudable goal. He however has decided not to take any advice given to me by a number of other kernel developers on how to work on the kernel. So instead he sends random broken patches to random subsystems in the hope that one will slip past a sleepy maintainer and end up in the kernel. He isn't willing to spend his own time learning anything, he is expecting that kernel developers want to spoon feed someone who sends them broken patches. We've asked him to stop, he keeps doing it, then when caught out apologizes with something along the lines, of I'm trying to learn, idiot mistake, despite having been told to take a step back and try and learn how the kernel works. Now we have to waste more maintainer time making sure nobody accidentally merges anything he sends. Dave. -- 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
[GIT PULL FOR v3.17] vb2: one fix and adding comments
I'd like to get this in for 3.17: the first patch fixes a BUG_ON due to a wrong gfp_flags value, the second adds comments before two WARN_ONs that are triggered fairly often due to buggy drivers and that point the driver developer into the right direction on how to solve it. Regards, Hans The following changes since commit 0f3bf3dc1ca394a8385079a5653088672b65c5c4: [media] cx23885: fix UNSET/TUNER_ABSENT confusion (2014-08-01 15:30:59 -0300) are available in the git repository at: git://linuxtv.org/hverkuil/media_tree.git for-v3.17i for you to fetch changes up to 4778bc67ba7a97e35a5f3c7159444fe5e44154f9: videobuf2-core: add comments before the WARN_ON (2014-08-04 07:33:53 +0200) Hans Verkuil (2): videobuf2-dma-sg: fix for wrong GFP mask to sg_alloc_table_from_pages videobuf2-core: add comments before the WARN_ON drivers/media/v4l2-core/videobuf2-core.c | 12 drivers/media/v4l2-core/videobuf2-dma-sg.c | 2 +- 2 files changed, 13 insertions(+), 1 deletion(-) -- 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: [PATCH] v4l2: Change call of function in videobuf2-core.c
On Mon, Aug 4, 2014 at 1:38 AM, Dave Airlie airl...@gmail.com wrote: On 4 August 2014 15:03, Hans Verkuil hverk...@xs4all.nl wrote: On 08/04/2014 05:25 AM, Nicholas Krause wrote: This patch changes the call of vb2_buffer_core to use VB2_BUFFER_STATE_ACTIVE inside the for instead of not setting in correctly to VB2_BUFFER_STATE_ERROR. Signed-off-by: Nicholas Krause xerofo...@gmail.com Dunno what's going on here after reading Dave Airlie's reply, but: Nick has decided he wants to be a kernel developer, a laudable goal. He however has decided not to take any advice given to me by a number of other kernel developers on how to work on the kernel. So instead he sends random broken patches to random subsystems in the hope that one will slip past a sleepy maintainer and end up in the kernel. He isn't willing to spend his own time learning anything, he is expecting that kernel developers want to spoon feed someone who sends them broken patches. We've asked him to stop, he keeps doing it, then when caught out apologizes with something along the lines, of I'm trying to learn, idiot mistake, despite having been told to take a step back and try and learn how the kernel works. Now we have to waste more maintainer time making sure nobody accidentally merges anything he sends. Dave. All of my merges are not in the main kernel and have been revoked. Cheers Nick -- 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: [PATCH] v4l2: Change call of function in videobuf2-core.c
On Mon, Aug 4, 2014 at 1:43 AM, Nick Krause xerofo...@gmail.com wrote: On Mon, Aug 4, 2014 at 1:38 AM, Dave Airlie airl...@gmail.com wrote: On 4 August 2014 15:03, Hans Verkuil hverk...@xs4all.nl wrote: On 08/04/2014 05:25 AM, Nicholas Krause wrote: This patch changes the call of vb2_buffer_core to use VB2_BUFFER_STATE_ACTIVE inside the for instead of not setting in correctly to VB2_BUFFER_STATE_ERROR. Signed-off-by: Nicholas Krause xerofo...@gmail.com Dunno what's going on here after reading Dave Airlie's reply, but: Nick has decided he wants to be a kernel developer, a laudable goal. He however has decided not to take any advice given to me by a number of other kernel developers on how to work on the kernel. So instead he sends random broken patches to random subsystems in the hope that one will slip past a sleepy maintainer and end up in the kernel. He isn't willing to spend his own time learning anything, he is expecting that kernel developers want to spoon feed someone who sends them broken patches. We've asked him to stop, he keeps doing it, then when caught out apologizes with something along the lines, of I'm trying to learn, idiot mistake, despite having been told to take a step back and try and learn how the kernel works. Now we have to waste more maintainer time making sure nobody accidentally merges anything he sends. Dave. All of my merges are not in the main kernel and have been revoked. Cheers Nick Dave, I understand your issues with my programming. I need to try and understand the kernel first before programming for it. Regards Nick -- 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: [PATCH] v4l2: Change call of function in videobuf2-core.c
Dave, I understand your issues with my programming. I need to try and understand the kernel first before programming for it. Why do you insist on sending more patches then, every day you try and send another one or two, despite been told multiple times to a) understand what you are writing, b) build test, c) actual test on hw or in a VM if applicable. Frankly I think you are taking the piss, probably writing some stupid Uni thesis on how to subvert the kernel development model by trolling it with broken patches. Dave. -- 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: [PATCH for v3.17] videobuf2-core: add comments before the WARN_ON
On Mon, Aug 4, 2014 at 2:36 PM, Hans Verkuil hverk...@xs4all.nl wrote: Recently WARN_ON() calls have been added to warn if the driver is not properly returning buffers to vb2 in start_streaming (if it fails) or stop_streaming(). Add comments before those WARN_ON calls that refer to the videobuf2-core.h header that explains what drivers are supposed to do in these situations. That should help point developers in the right direction if they see these warnings. Signed-off-by: Hans Verkuil hans.verk...@cisco.com Acked-by: Pawel Osciak pa...@osciak.com --- drivers/media/v4l2-core/videobuf2-core.c | 12 1 file changed, 12 insertions(+) diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c index c359006..d3f2a22 100644 --- a/drivers/media/v4l2-core/videobuf2-core.c +++ b/drivers/media/v4l2-core/videobuf2-core.c @@ -1762,6 +1762,12 @@ static int vb2_start_streaming(struct vb2_queue *q) q-start_streaming_called = 0; dprintk(1, driver refused to start streaming\n); + /* +* If you see this warning, then the driver isn't cleaning up properly +* after a failed start_streaming(). See the start_streaming() +* documentation in videobuf2-core.h for more information how buffers +* should be returned to vb2 in start_streaming(). +*/ if (WARN_ON(atomic_read(q-owned_by_drv_count))) { unsigned i; @@ -2123,6 +2129,12 @@ static void __vb2_queue_cancel(struct vb2_queue *q) if (q-start_streaming_called) call_void_qop(q, stop_streaming, q); + /* +* If you see this warning, then the driver isn't cleaning up properly +* in stop_streaming(). See the stop_streaming() documentation in +* videobuf2-core.h for more information how buffers should be returned +* to vb2 in stop_streaming(). +*/ if (WARN_ON(atomic_read(q-owned_by_drv_count))) { for (i = 0; i q-num_buffers; ++i) if (q-bufs[i]-state == VB2_BUF_STATE_ACTIVE) -- 2.0.1 -- Best regards, Pawel Osciak -- 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