Re: [PATCH RFC] media: i2c: mt9p031: add OF support
Hi, One more point for your devicetree conversions, On Mon, Apr 29, 2013 at 01:30:01PM +0530, Prabhakar Lad wrote: From: Lad, Prabhakar prabhakar.cse...@gmail.com add OF support for the mt9p031 sensor driver. +static struct mt9p031_platform_data + *mt9p031_get_pdata(struct i2c_client *client) + +{ + if (!client-dev.platform_data client-dev.of_node) { + struct device_node *np; + struct mt9p031_platform_data *pdata; + int ret; + + np = v4l2_of_get_next_endpoint(client-dev.of_node, NULL); + if (!np) + return NULL; + + pdata = devm_kzalloc(client-dev, + sizeof(struct mt9p031_platform_data), + GFP_KERNEL); + if (!pdata) { + pr_warn(mt9p031 failed allocate memeory\n); + return NULL; + } + ret = of_property_read_u32(np, reset, pdata-reset); + if (ret == -EINVAL) + pdata-reset = -1; + else if (ret == -ENODATA) + return NULL; + + if (of_property_read_u32(np, ext_freq, pdata-ext_freq)) + return NULL; + + if (of_property_read_u32(np, target_freq, + pdata-target_freq)) + return NULL; + + return pdata; + } I don't know how the others see this, but IMO it would be cleaner to first add a duplicate of the members of pdata in struct mt9p031 and then initialize them either from pdata or from devicetree data. The (artificial) creation of platform_data for the devicetree case adds a new level of indirection. This may not be a problem here, but there are cases where there is no 1:1 transcription between pdata and devicetree possible. Sascha -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- | -- 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 1/4] [media] s3c-camif: Remove redundant NULL check
clk_unprepare checks for NULL pointer. Hence convert IS_ERR_OR_NULL to IS_ERR only. Signed-off-by: Sachin Kamat sachin.ka...@linaro.org --- drivers/media/platform/s3c-camif/camif-core.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/platform/s3c-camif/camif-core.c b/drivers/media/platform/s3c-camif/camif-core.c index 0d0fab1..2449f13 100644 --- a/drivers/media/platform/s3c-camif/camif-core.c +++ b/drivers/media/platform/s3c-camif/camif-core.c @@ -341,7 +341,7 @@ static void camif_clk_put(struct camif_dev *camif) int i; for (i = 0; i CLK_MAX_NUM; i++) { - if (IS_ERR_OR_NULL(camif-clock[i])) + if (IS_ERR(camif-clock[i])) continue; clk_unprepare(camif-clock[i]); clk_put(camif-clock[i]); -- 1.7.9.5 -- 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/4] [media] s3c-camif: Fix incorrect variable type
'rotation' was an 8 bit variable and hence could not have values greater than 255. Since we need higher values, chnage it to 16 bit type. Signed-off-by: Sachin Kamat sachin.ka...@linaro.org --- drivers/media/platform/s3c-camif/camif-core.h |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/platform/s3c-camif/camif-core.h b/drivers/media/platform/s3c-camif/camif-core.h index 261134b..35d2fcd 100644 --- a/drivers/media/platform/s3c-camif/camif-core.h +++ b/drivers/media/platform/s3c-camif/camif-core.h @@ -229,7 +229,7 @@ struct camif_vp { unsigned intstate; u16 fmt_flags; u8 id; - u8 rotation; + u16 rotation; u8 hflip; u8 vflip; unsigned intoffset; -- 1.7.9.5 -- 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 3/4] [media] s3c-camif: Staticize local symbols
These symbols are local to the file and should be static. Signed-off-by: Sachin Kamat sachin.ka...@linaro.org --- drivers/media/platform/s3c-camif/camif-regs.c |4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/media/platform/s3c-camif/camif-regs.c b/drivers/media/platform/s3c-camif/camif-regs.c index 1a3b4fc..7b22d6c 100644 --- a/drivers/media/platform/s3c-camif/camif-regs.c +++ b/drivers/media/platform/s3c-camif/camif-regs.c @@ -379,7 +379,7 @@ static void camif_hw_set_prescaler(struct camif_vp *vp) camif_write(camif, S3C_CAMIF_REG_CISCPREDST(vp-id, vp-offset), cfg); } -void camif_s3c244x_hw_set_scaler(struct camif_vp *vp) +static void camif_s3c244x_hw_set_scaler(struct camif_vp *vp) { struct camif_dev *camif = vp-camif; struct camif_scaler *scaler = vp-scaler; @@ -426,7 +426,7 @@ void camif_s3c244x_hw_set_scaler(struct camif_vp *vp) scaler-main_h_ratio, scaler-main_v_ratio); } -void camif_s3c64xx_hw_set_scaler(struct camif_vp *vp) +static void camif_s3c64xx_hw_set_scaler(struct camif_vp *vp) { struct camif_dev *camif = vp-camif; struct camif_scaler *scaler = vp-scaler; -- 1.7.9.5 -- 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/4] [media] s3c-camif: Use dev_info instead of printk
dev_info is preferred to printk. Silences the related checkpatch warning. WARNING: Prefer netdev_info(netdev, ... then dev_info(dev, ... then pr_info(... to printk(KERN_INFO ... Signed-off-by: Sachin Kamat sachin.ka...@linaro.org --- drivers/media/platform/s3c-camif/camif-regs.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/platform/s3c-camif/camif-regs.c b/drivers/media/platform/s3c-camif/camif-regs.c index 7b22d6c..a9e3b16 100644 --- a/drivers/media/platform/s3c-camif/camif-regs.c +++ b/drivers/media/platform/s3c-camif/camif-regs.c @@ -601,6 +601,6 @@ void camif_hw_dump_regs(struct camif_dev *camif, const char *label) pr_info(--- %s ---\n, label); for (i = 0; i ARRAY_SIZE(registers); i++) { u32 cfg = readl(camif-io_base + registers[i].offset); - printk(KERN_INFO %s:\t0x%08x\n, registers[i].name, cfg); + dev_info(camif-dev, %s:\t0x%08x\n, registers[i].name, cfg); } } -- 1.7.9.5 -- 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] [media] s5c73m3: Fix off-by-one valid range checking for fie-index
Hi Axel, Thanks for the fix. On 30.04.2013 06:42, Axel Lin wrote: Current code uses fie-index as array subscript, thus the valid value range is 0 ... ARRAY_SIZE(s5c73m3_intervals) - 1. Signed-off-by: Axel Lin axel@ingics.com Acked-by: Andrzej Hajda a.ha...@samsung.com --- drivers/media/i2c/s5c73m3/s5c73m3-core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/i2c/s5c73m3/s5c73m3-core.c b/drivers/media/i2c/s5c73m3/s5c73m3-core.c index b353c50..cd365bb 100644 --- a/drivers/media/i2c/s5c73m3/s5c73m3-core.c +++ b/drivers/media/i2c/s5c73m3/s5c73m3-core.c @@ -956,7 +956,7 @@ static int s5c73m3_oif_enum_frame_interval(struct v4l2_subdev *sd, if (fie-pad != OIF_SOURCE_PAD) return -EINVAL; - if (fie-index ARRAY_SIZE(s5c73m3_intervals)) + if (fie-index = ARRAY_SIZE(s5c73m3_intervals)) return -EINVAL; mutex_lock(state-lock); -- 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 1/1] [media] s5c73m3: Fix whitespace related warnings
Silences the following type of warning: WARNING: space prohibited before semicolon Signed-off-by: Sachin Kamat sachin.ka...@linaro.org Cc: Andrzej Hajda a.ha...@samsung.com --- drivers/media/i2c/s5c73m3/s5c73m3-spi.c |4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/media/i2c/s5c73m3/s5c73m3-spi.c b/drivers/media/i2c/s5c73m3/s5c73m3-spi.c index 6f3a9c0..8079e26 100644 --- a/drivers/media/i2c/s5c73m3/s5c73m3-spi.c +++ b/drivers/media/i2c/s5c73m3/s5c73m3-spi.c @@ -73,7 +73,7 @@ int s5c73m3_spi_write(struct s5c73m3 *state, const void *addr, memset(padding, 0, sizeof(padding)); - for (i = 0; i count ; i++) { + for (i = 0; i count; i++) { r = spi_xmit(spi_dev, (void *)addr + j, tx_size, SPI_DIR_TX); if (r 0) return r; @@ -98,7 +98,7 @@ int s5c73m3_spi_read(struct s5c73m3 *state, void *addr, unsigned int i, j = 0; int r = 0; - for (i = 0; i count ; i++) { + for (i = 0; i count; i++) { r = spi_xmit(spi_dev, addr + j, tx_size, SPI_DIR_RX); if (r 0) return r; -- 1.7.9.5 -- 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 1/2] [media] soc_camera: Constify dev_pm_ops in mt9t031.c
Silences the following warning: WARNING: struct dev_pm_ops should normally be const Signed-off-by: Sachin Kamat sachin.ka...@linaro.org --- drivers/media/i2c/soc_camera/mt9t031.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/i2c/soc_camera/mt9t031.c b/drivers/media/i2c/soc_camera/mt9t031.c index 26a15b8..191e3f1 100644 --- a/drivers/media/i2c/soc_camera/mt9t031.c +++ b/drivers/media/i2c/soc_camera/mt9t031.c @@ -595,7 +595,7 @@ static int mt9t031_runtime_resume(struct device *dev) return 0; } -static struct dev_pm_ops mt9t031_dev_pm_ops = { +static const struct dev_pm_ops mt9t031_dev_pm_ops = { .runtime_suspend= mt9t031_runtime_suspend, .runtime_resume = mt9t031_runtime_resume, }; -- 1.7.9.5 -- 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/2] [media] soc_camera: Fix checkpatch warning in ov9640.c
Silences the following warning: WARNING: please, no space before tabs Signed-off-by: Sachin Kamat sachin.ka...@linaro.org --- drivers/media/i2c/soc_camera/ov9640.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/i2c/soc_camera/ov9640.c b/drivers/media/i2c/soc_camera/ov9640.c index 20ca62d..f4eba1b 100644 --- a/drivers/media/i2c/soc_camera/ov9640.c +++ b/drivers/media/i2c/soc_camera/ov9640.c @@ -61,7 +61,7 @@ static const struct ov9640_reg ov9640_regs_dflt[] = { /* Configurations * NOTE: for YUV, alter the following registers: - * COM12 |= OV9640_COM12_YUV_AVG + * COM12 |= OV9640_COM12_YUV_AVG * * for RGB, alter the following registers: * COM7 |= OV9640_COM7_RGB -- 1.7.9.5 -- 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 RFC] media: i2c: mt9p031: add OF support
Hi Sascha, On Tuesday 30 April 2013 08:16:25 Sascha Hauer wrote: On Mon, Apr 29, 2013 at 01:30:01PM +0530, Prabhakar Lad wrote: From: Lad, Prabhakar prabhakar.cse...@gmail.com add OF support for the mt9p031 sensor driver. +static struct mt9p031_platform_data + *mt9p031_get_pdata(struct i2c_client *client) + +{ + if (!client-dev.platform_data client-dev.of_node) { + struct device_node *np; + struct mt9p031_platform_data *pdata; + int ret; + + np = v4l2_of_get_next_endpoint(client-dev.of_node, NULL); + if (!np) + return NULL; + + pdata = devm_kzalloc(client-dev, +sizeof(struct mt9p031_platform_data), +GFP_KERNEL); + if (!pdata) { + pr_warn(mt9p031 failed allocate memeory\n); + return NULL; + } + ret = of_property_read_u32(np, reset, pdata-reset); + if (ret == -EINVAL) + pdata-reset = -1; + else if (ret == -ENODATA) + return NULL; + + if (of_property_read_u32(np, ext_freq, pdata-ext_freq)) + return NULL; + + if (of_property_read_u32(np, target_freq, +pdata-target_freq)) + return NULL; + + return pdata; + } I don't know how the others see this, but IMO it would be cleaner to first add a duplicate of the members of pdata in struct mt9p031 and then initialize them either from pdata or from devicetree data. The (artificial) creation of platform_data for the devicetree case adds a new level of indirection. This may not be a problem here, but there are cases where there is no 1:1 transcription between pdata and devicetree possible. I have no strong opinion on this. In the mt9p031 case it won't matter much, but it's probably a good idea in general. -- Regards, Laurent Pinchart -- 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 v9 02/20] V4L2: support asynchronous subdevice registration
Hi Guennadi, On Fri, Apr 12, 2013 at 05:40:22PM +0200, Guennadi Liakhovetski wrote: Currently bridge device drivers register devices for all subdevices synchronously, tupically, during their probing. E.g. if an I2C CMOS sensor is attached to a video bridge device, the bridge driver will create an I2C device and wait for the respective I2C driver to probe. This makes linking of devices straight forward, but this approach cannot be used with intrinsically asynchronous and unordered device registration systems like the Flattened Device Tree. To support such systems this patch adds an asynchronous subdevice registration framework to V4L2. To use it respective (e.g. I2C) subdevice drivers must register themselves with the framework. A bridge driver on the other hand must register notification callbacks, that will be called upon various related events. Signed-off-by: Guennadi Liakhovetski g.liakhovet...@gmx.de --- + +static struct v4l2_async_subdev *v4l2_async_belongs(struct v4l2_async_notifier *notifier, + struct v4l2_async_subdev_list *asdl) +{ + struct v4l2_subdev *sd = v4l2_async_to_subdev(asdl); + struct v4l2_async_subdev *asd = NULL; + bool (*match)(struct device *, + struct v4l2_async_hw_info *); + + list_for_each_entry (asd, notifier-waiting, list) { + struct v4l2_async_hw_info *hw = asd-hw; + + /* bus_type has been verified valid before */ + switch (hw-bus_type) { + case V4L2_ASYNC_BUS_CUSTOM: + match = hw-match.custom.match; + if (!match) + /* Match always */ + return asd; + break; + case V4L2_ASYNC_BUS_PLATFORM: + match = match_platform; + break; + case V4L2_ASYNC_BUS_I2C: + match = match_i2c; + break; + default: + /* Cannot happen, unless someone breaks us */ + WARN_ON(true); + return NULL; + } + + if (match match(sd-dev, hw)) + break; + } + + return asd; 'asd' can never be NULL here. You have to explicitly return NULL when leaving the loop without match. Sascha -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- | -- 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 v9 02/20] V4L2: support asynchronous subdevice registration
Hi Sascha On Tue, 30 Apr 2013, Sascha Hauer wrote: Hi Guennadi, On Fri, Apr 12, 2013 at 05:40:22PM +0200, Guennadi Liakhovetski wrote: Currently bridge device drivers register devices for all subdevices synchronously, tupically, during their probing. E.g. if an I2C CMOS sensor is attached to a video bridge device, the bridge driver will create an I2C device and wait for the respective I2C driver to probe. This makes linking of devices straight forward, but this approach cannot be used with intrinsically asynchronous and unordered device registration systems like the Flattened Device Tree. To support such systems this patch adds an asynchronous subdevice registration framework to V4L2. To use it respective (e.g. I2C) subdevice drivers must register themselves with the framework. A bridge driver on the other hand must register notification callbacks, that will be called upon various related events. Signed-off-by: Guennadi Liakhovetski g.liakhovet...@gmx.de --- + +static struct v4l2_async_subdev *v4l2_async_belongs(struct v4l2_async_notifier *notifier, + struct v4l2_async_subdev_list *asdl) +{ + struct v4l2_subdev *sd = v4l2_async_to_subdev(asdl); + struct v4l2_async_subdev *asd = NULL; + bool (*match)(struct device *, + struct v4l2_async_hw_info *); + + list_for_each_entry (asd, notifier-waiting, list) { + struct v4l2_async_hw_info *hw = asd-hw; + + /* bus_type has been verified valid before */ + switch (hw-bus_type) { + case V4L2_ASYNC_BUS_CUSTOM: + match = hw-match.custom.match; + if (!match) + /* Match always */ + return asd; + break; + case V4L2_ASYNC_BUS_PLATFORM: + match = match_platform; + break; + case V4L2_ASYNC_BUS_I2C: + match = match_i2c; + break; + default: + /* Cannot happen, unless someone breaks us */ + WARN_ON(true); + return NULL; + } + + if (match match(sd-dev, hw)) + break; + } + + return asd; 'asd' can never be NULL here. You have to explicitly return NULL when leaving the loop without match. I've already proposed a fix for this and Laurent has proposed a simplified version. Thanks Guennadi Sascha -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- | --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- 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] [media] exynos4-is: Fix off-by-one valid range checking for is-config_index
Current code uses is-config_index as array subscript, thus the valid value range is 0 ... ARRAY_SIZE(cmd) - 1. Signed-off-by: Axel Lin axel@ingics.com --- drivers/media/platform/exynos4-is/fimc-is-regs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/platform/exynos4-is/fimc-is-regs.c b/drivers/media/platform/exynos4-is/fimc-is-regs.c index b0ff67b..d05eaa2 100644 --- a/drivers/media/platform/exynos4-is/fimc-is-regs.c +++ b/drivers/media/platform/exynos4-is/fimc-is-regs.c @@ -174,7 +174,7 @@ int fimc_is_hw_change_mode(struct fimc_is *is) HIC_CAPTURE_STILL, HIC_CAPTURE_VIDEO, }; - if (WARN_ON(is-config_index ARRAY_SIZE(cmd))) + if (WARN_ON(is-config_index = ARRAY_SIZE(cmd))) return -EINVAL; mcuctl_write(cmd[is-config_index], is, MCUCTL_REG_ISSR(0)); -- 1.8.1.2 -- 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 v2 1/4] V4L2: soc_camera: Renesas R-Car VIN driver
Hi, Guennadi, Thank you for the review! Sergei Shtylyov wrote: I also strongly suspent some #include media/v4l2-*.h headers are missing above. Hm, I wonder which. I'm certainly not V4L2 expert... added following: #include media/v4l2-common.h #include media/v4l2-dev.h #include media/v4l2-device.h #include media/v4l2-mediabus.h #include media/v4l2-subdev.h +alloc_ctxs[0] = priv-alloc_ctx; + +if (!vq-num_buffers) +priv-sequence = 0; + +if (!*count) +*count = 2; +priv-vb_count = *count; + +*num_planes = 1; + +/* Number of hardware slots */ +if (priv-vb_count MAX_BUFFER_NUM) +priv-nr_hw_slots = MAX_BUFFER_NUM; +else +priv-nr_hw_slots = 1; Is this really correct: with up to 3 buffers only one HW slot is used? Probably not. After replacing (priv-vb_count MAX_BUFFER_NUM) with is_continuous_transfer(priv) the logic becomes clear. +break; +default: +vnmc = VNMC_IM_ODD; +break; +} + +/* input interface */ +switch (icd-current_fmt-code) { +case V4L2_MBUS_FMT_YUYV8_1X16: +/* BT.601/BT.1358 16bit YCbCr422 */ +vnmc |= VNMC_INF_YUV16; +input_is_yuv = 1; +break; +case V4L2_MBUS_FMT_YUYV8_2X8: +input_is_yuv = 1; +/* BT.656 8bit YCbCr422 or BT.601 8bit YCbCr422 */ +vnmc |= priv-pdata-flags RCAR_VIN_BT656 ? +VNMC_INF_YUV8_BT656 : VNMC_INF_YUV8_BT601; Let's clarify this. By BT.656 you mean embedded synchronisation patterns, right? In that case HSYNC and VSYNC signals aren't used. Probably so, at least I know for sure HSYNC/VSYNC aren't used. Yes, it is. BT.656 uses embedded synchronization and HSYNC/VSYNC are not used for this type of interface. The BT.601 is a so-called by soc-camera layer MBUS_PARALLEL. But in your .set_bus_param() method you only support V4L2_MBUS_PARALLEL and not V4L2_MBUS_BT656. And what do you call BT601? A bus with sync signals used? Yeah, judging from the manual, HSYNC, VSYNC, FIELD are used in BT.601. The name ITUR BT601 YCbCr that comes from specification is a MBUS_PARALLEL name per soc-camera layer naming. The BT601 is expected to use h/w sync signals. I've removed the pre initialization of v4l2_mbus_config .type filed in favour of getting it from camera/decoder subdevice. Thx for pointing to this. +} +} +spin_unlock_irq(priv-lock); + +pm_runtime_put_sync(ici-v4l2_dev.dev); Do you really need the _sync version above? I'm not runtime PM expert, to be honest... replaced with pm_runtime_put(). thx for pointing to this. +static u16 calc_scale(unsigned int src, unsigned int *dst) +{ +u16 scale; + +if (src == *dst) +return 0; + +scale = (src * 4096 / *dst) ~7; + +while (scale 4096 size_dst(src, scale) *dst) +scale -= 8; + +*dst = size_dst(src, scale); + +return scale; return value of this function is unused by the caller. Generally, your use of these two functions is different than on CEU, you might want to get rid of them completely. I'd prefer to leave this function and provide fixes after scaling is fully tested. +/* Set Start/End Pixel/Line Pre-Clip */ +iowrite32(left_offset dsize, priv-base + VNSPPRC_REG); +iowrite32((left_offset + cam-width - 1) dsize, + priv-base + VNEPPRC_REG); Do you have to shift for all 32-bit formats, not only for RGB32? I understand this is related to the fact, that you don't support pass-through... At least the manual says to program an even number to VnSPPrC... The driver explicitly says that V4L2_PIX_FMT_RGB32 is the only 32bit format supported. +static void capture_stop_preserve(struct rcar_vin_priv *priv, u32 *vnmc) +{ +*vnmc = ioread32(priv-base + VNMC_REG); +/* module disable */ +iowrite32(*vnmc ~VNMC_ME, priv-base + VNMC_REG); +} + +static void capture_restore(struct rcar_vin_priv *priv, u32 vnmc) +{ +unsigned long timeout = jiffies + 10 * HZ; + +if (!(vnmc ~VNMC_ME)) +/* Nothing to restore */ +return; And you don't have to wait for a frame end? If the module wasn't active, there's probably no point... however, let's defer it to Vladimir. Right. Thx for catching this. +}, +{ +.fourcc= V4L2_PIX_FMT_YUYV, +.name= YUYV, +.bits_per_sample= 16, +.packing= SOC_MBUS_PACKING_NONE, +.order= SOC_MBUS_ORDER_LE, This conversion block is identical to the respective one in soc_mediabus.c, which suggests to me, that no conversion is taking place here. Then this mode should be usable for generic 8- or 16-bit pass-through? Let's defer this question to Vladimir. Will add pass-through. Thank you. +switch (code) { +case V4L2_MBUS_FMT_YUYV8_1X16: +case V4L2_MBUS_FMT_YUYV8_2X8: +if (cam-extra_fmt) +break; + +/* Add all our formats that can be
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: Tue Apr 30 19:00:27 CEST 2013 git branch: test git hash: 02615ed5e1b2283db2495af3cf8f4ee172c77d80 gcc version:i686-linux-gcc (GCC) 4.7.2 host hardware: x86_64 host os:3.8-3.slh.2-amd64 linux-git-arm-davinci: OK linux-git-arm-exynos: WARNINGS linux-git-arm-omap: WARNINGS linux-git-blackfin: WARNINGS 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.31.14-i686: WARNINGS linux-2.6.32.27-i686: WARNINGS linux-2.6.33.7-i686: WARNINGS linux-2.6.34.7-i686: WARNINGS linux-2.6.35.9-i686: WARNINGS linux-2.6.36.4-i686: WARNINGS linux-2.6.37.6-i686: WARNINGS linux-2.6.38.8-i686: WARNINGS linux-2.6.39.4-i686: WARNINGS linux-3.0.60-i686: WARNINGS linux-3.1.10-i686: WARNINGS linux-3.2.37-i686: WARNINGS linux-3.3.8-i686: WARNINGS linux-3.4.27-i686: WARNINGS linux-3.5.7-i686: WARNINGS linux-3.6.11-i686: WARNINGS linux-3.7.4-i686: WARNINGS linux-3.8-i686: OK linux-3.9-rc1-i686: OK linux-2.6.31.14-x86_64: WARNINGS linux-2.6.32.27-x86_64: WARNINGS linux-2.6.33.7-x86_64: WARNINGS linux-2.6.34.7-x86_64: WARNINGS linux-2.6.35.9-x86_64: WARNINGS linux-2.6.36.4-x86_64: WARNINGS linux-2.6.37.6-x86_64: WARNINGS linux-2.6.38.8-x86_64: WARNINGS linux-2.6.39.4-x86_64: WARNINGS linux-3.0.60-x86_64: WARNINGS linux-3.1.10-x86_64: WARNINGS linux-3.2.37-x86_64: WARNINGS linux-3.3.8-x86_64: WARNINGS linux-3.4.27-x86_64: WARNINGS linux-3.5.7-x86_64: WARNINGS linux-3.6.11-x86_64: WARNINGS linux-3.7.4-x86_64: WARNINGS linux-3.8-x86_64: OK linux-3.9-rc1-x86_64: OK apps: WARNINGS spec-git: OK sparse: ERRORS Detailed results are available here: http://www.xs4all.nl/~hverkuil/logs/Tuesday.log Full logs are available here: http://www.xs4all.nl/~hverkuil/logs/Tuesday.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
Re: [alsa-devel] [PATCH] MAINTAINERS: Add linux-samsung-soc list to all related entries
On Mon, Apr 22, 2013 at 03:23:29PM +0900, Kyungmin Park wrote: I don't think it's not required, each tree has each own mailing list. don't need to post all patches to samsung-soc list. It can be useful to get system level input on some stuff, I guess it mostly depends if the people on the generic list mind the extra traffic or if they find it useful. signature.asc Description: Digital signature
[PATCH] [RFC] mutex: w/w mutex slowpath debugging
Injects EDEADLK conditions at pseudo-random interval, with exponential backoff up to UINT_MAX (to ensure that every lock operation still completes in a reasonable time). This way we can test the wound slowpath even for ww mutex users where contention is never expected, and the ww deadlock avoidance algorithm is only needed for correctness against malicious userspace. An example would be protecting kernel modesetting properties, which thanks to single-threaded X isn't really expected to contend, ever. I've looked into using the CONFIG_FAULT_INJECTION infrastructure, but decided against it for two reasons: - EDEADLK handling is mandatory for ww mutex users and should never affect the outcome of a syscall. This is in contrast to -ENOMEM injection. So fine configurability isn't required. - The fault injection framework only allows to set a simple probability for failure. Now the probability that a ww mutex acquire stage with N locks will never complete (due to too many injected EDEADLK backoffs) is zero. But the expected number of ww_mutex_lock operations for the completely uncontended case would be O(exp(N)). The per-acuiqire ctx exponential backoff solution choosen here only results in O(log N) overhead due to injection and so O(log N * N) lock operations. This way we can fail with high probability (and so have good test coverage even for fancy backoff and lock acquisition paths) without running into patalogical cases. Note that EDEADLK will only ever be injected when we managed to acquire the lock. This prevents any behaviour changes for users which rely on the EALREADY semantics. Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch --- include/linux/mutex.h |8 kernel/mutex.c| 31 +++ lib/Kconfig.debug | 10 ++ 3 files changed, 49 insertions(+) diff --git a/include/linux/mutex.h b/include/linux/mutex.h index 004f863..82d56ec 100644 --- a/include/linux/mutex.h +++ b/include/linux/mutex.h @@ -93,6 +93,10 @@ struct ww_acquire_ctx { #ifdef CONFIG_DEBUG_LOCK_ALLOC struct lockdep_map dep_map; #endif +#ifdef CONFIG_DEBUG_WW_MUTEX_SLOWPATH + unsigned deadlock_inject_interval; + unsigned deadlock_inject_countdown; +#endif }; struct ww_mutex { @@ -278,6 +282,10 @@ static inline void ww_acquire_init(struct ww_acquire_ctx *ctx, ww_class-acquire_key, 0); mutex_acquire(ctx-dep_map, 0, 0, _RET_IP_); #endif +#ifdef CONFIG_DEBUG_WW_MUTEX_SLOWPATH + ctx-deadlock_inject_interval = ctx-stamp 0xf; + ctx-deadlock_inject_countdown = ctx-deadlock_inject_interval; +#endif } /** diff --git a/kernel/mutex.c b/kernel/mutex.c index 66807c7..1cc3487 100644 --- a/kernel/mutex.c +++ b/kernel/mutex.c @@ -827,6 +827,35 @@ int __sched mutex_trylock(struct mutex *lock) EXPORT_SYMBOL(mutex_trylock); #ifndef CONFIG_DEBUG_LOCK_ALLOC + +#ifdef CONFIG_DEBUG_WW_MUTEX_SLOWPATH +static int __sched +ww_mutex_deadlock_injection(struct ww_mutex *lock, struct ww_acquire_ctx *ctx) +{ + if (ctx-deadlock_inject_countdown-- == 0) { + tmp = ctx-deadlock_inject_interval; + if (tmp UINT_MAX/4) + tmp = UINT_MAX; + else + tmp = tmp*2 + tmp + tmp/2; + + ctx-deadlock_inject_interval = tmp; + ctx-deadlock_inject_countdown = tmp; + + ww_mutex_unlock(lock); + + return -EDEADLK; + } + + return 0; +} +#else +static int __sched +ww_mutex_deadlock_injection(struct ww_mutex *lock, struct ww_acquire_ctx *ctx) +{ + return 0; +} +#endif int __sched ww_mutex_lock(struct ww_mutex *lock, struct ww_acquire_ctx *ctx) { @@ -839,6 +868,7 @@ ww_mutex_lock(struct ww_mutex *lock, struct ww_acquire_ctx *ctx) if (likely(!ret)) { ww_mutex_set_context_fastpath(lock, ctx); mutex_set_owner(lock-base); + return ww_mutex_deadlock_injection(lock, ctx); } else ret = __ww_mutex_lock_slowpath(lock, ctx); return ret; @@ -857,6 +887,7 @@ ww_mutex_lock_interruptible(struct ww_mutex *lock, struct ww_acquire_ctx *ctx) if (likely(!ret)) { ww_mutex_set_context_fastpath(lock, ctx); mutex_set_owner(lock-base); + return ww_mutex_deadlock_injection(lock, ctx); } else ret = __ww_mutex_lock_interruptible_slowpath(lock, ctx); return ret; diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug index 28be08c..8c41f73 100644 --- a/lib/Kconfig.debug +++ b/lib/Kconfig.debug @@ -547,6 +547,16 @@ config DEBUG_MUTEXES This feature allows mutex semantics violations to be detected and reported. +config DEBUG_WW_MUTEX_SLOWPATH + bool Wait/wound mutex debugging: Slowpath testing + depends on DEBUG_KERNEL + help +This feature enables slowpath testing for w/w mutex users
Re: [PATCH v3 2/3] mutex: add support for wound/wait style locks, v3
On Sun, Apr 28, 2013 at 07:04:07PM +0200, Maarten Lankhorst wrote: Changes since RFC patch v1: - Updated to use atomic_long instead of atomic, since the reservation_id was a long. - added mutex_reserve_lock_slow and mutex_reserve_lock_intr_slow - removed mutex_locked_set_reservation_id (or w/e it was called) Changes since RFC patch v2: - remove use of __mutex_lock_retval_arg, add warnings when using wrong combination of mutex_(,reserve_)lock/unlock. Changes since v1: - Add __always_inline to __mutex_lock_common, otherwise reservation paths can be triggered from normal locks, because __builtin_constant_p might evaluate to false for the constant 0 in that case. Tests for this have been added in the next patch. - Updated documentation slightly. Changes since v2: - Renamed everything to ww_mutex. (mlankhorst) - Added ww_acquire_ctx and ww_class. (mlankhorst) - Added a lot of checks for wrong api usage. (mlankhorst) - Documentation updates. (danvet) While writing the kerneldoc I've carefully check that all restrictions are enforced through debug checks somehow. I think that with full mutex debug (including lockdep) enabled, plus the slowpath injector patch I've just posted, _all_ interface abuse will be catched at runtime as long as all the single-threaded/uncontended cases are exercises sufficiently. So I think we've fully achieved level 5 on the Rusty API safety scale here. Higher levels seem pretty hard given that the concepts are rather fancy, but I think with the new (and much more consitent) naming, plus the explicit introduction as (more abstruct) structures for ww_class and ww_acquire_context the interface is about as intuitive as it gets. So all together I'm pretty happy with what the interface looks like. And one quick bikeshed below on the implementation. -Daniel Signed-off-by: Maarten Lankhorst maarten.lankho...@canonical.com Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch --- Documentation/ww-mutex-design.txt | 322 +++ include/linux/mutex-debug.h |1 include/linux/mutex.h | 257 + kernel/mutex.c| 445 - lib/debug_locks.c |2 5 files changed, 1010 insertions(+), 17 deletions(-) create mode 100644 Documentation/ww-mutex-design.txt [snip] +/* + * after acquiring lock with fastpath or when we lost out in contested + * slowpath, set ctx and wake up any waiters so they can recheck. + * + * This function is never called when CONFIG_DEBUG_LOCK_ALLOC is set, + * as the fastpath and opportunistic spinning are disabled in that case. + */ +static __always_inline void +ww_mutex_set_context_fastpath(struct ww_mutex *lock, +struct ww_acquire_ctx *ctx) +{ + unsigned long flags; + struct mutex_waiter *cur; + + ww_mutex_lock_acquired(lock, ctx, false); + + lock-ctx = ctx; + smp_mb__after_atomic_dec(); I think this should be + smp_mb__after_atomic_dec(); + lock-ctx = ctx; + smp_mb(); Also I wonder a bit how much this hurts the fastpath, and whether we should just shovel the ctx into the atomic field with a cmpxcht, like the rt mutex code does with the current pointer. + + /* + * Check if lock is contended, if not there is nobody to wake up + */ + if (likely(atomic_read(lock-base.count) == 0)) + return; + + /* + * Uh oh, we raced in fastpath, wake up everyone in this case, + * so they can see the new ctx + */ + spin_lock_mutex(lock-base.wait_lock, flags); + list_for_each_entry(cur, lock-base.wait_list, list) { + debug_mutex_wake_waiter(lock-base, cur); + wake_up_process(cur-task); + } + spin_unlock_mutex(lock-base.wait_lock, flags); +} + -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- 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] [RFC] mutex: w/w mutex slowpath debugging
On Tue, 2013-04-30 at 20:45 +0200, Daniel Vetter wrote: /** diff --git a/kernel/mutex.c b/kernel/mutex.c index 66807c7..1cc3487 100644 --- a/kernel/mutex.c +++ b/kernel/mutex.c @@ -827,6 +827,35 @@ int __sched mutex_trylock(struct mutex *lock) EXPORT_SYMBOL(mutex_trylock); #ifndef CONFIG_DEBUG_LOCK_ALLOC + +#ifdef CONFIG_DEBUG_WW_MUTEX_SLOWPATH +static int __sched +ww_mutex_deadlock_injection(struct ww_mutex *lock, struct ww_acquire_ctx *ctx) +{ + if (ctx-deadlock_inject_countdown-- == 0) { + tmp = ctx-deadlock_inject_interval; + if (tmp UINT_MAX/4) + tmp = UINT_MAX; + else + tmp = tmp*2 + tmp + tmp/2; + + ctx-deadlock_inject_interval = tmp; + ctx-deadlock_inject_countdown = tmp; + + ww_mutex_unlock(lock); + + return -EDEADLK; + } + + return 0; +} +#else +static int __sched +ww_mutex_deadlock_injection(struct ww_mutex *lock, struct ww_acquire_ctx *ctx) +{ + return 0; +} This should be a static inline, and remove the __sched. There's no reason to make this anything but a nop when disabled. -- Steve -- 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] [RFC] mutex: w/w mutex slowpath debugging
Injects EDEADLK conditions at pseudo-random interval, with exponential backoff up to UINT_MAX (to ensure that every lock operation still completes in a reasonable time). This way we can test the wound slowpath even for ww mutex users where contention is never expected, and the ww deadlock avoidance algorithm is only needed for correctness against malicious userspace. An example would be protecting kernel modesetting properties, which thanks to single-threaded X isn't really expected to contend, ever. I've looked into using the CONFIG_FAULT_INJECTION infrastructure, but decided against it for two reasons: - EDEADLK handling is mandatory for ww mutex users and should never affect the outcome of a syscall. This is in contrast to -ENOMEM injection. So fine configurability isn't required. - The fault injection framework only allows to set a simple probability for failure. Now the probability that a ww mutex acquire stage with N locks will never complete (due to too many injected EDEADLK backoffs) is zero. But the expected number of ww_mutex_lock operations for the completely uncontended case would be O(exp(N)). The per-acuiqire ctx exponential backoff solution choosen here only results in O(log N) overhead due to injection and so O(log N * N) lock operations. This way we can fail with high probability (and so have good test coverage even for fancy backoff and lock acquisition paths) without running into patalogical cases. Note that EDEADLK will only ever be injected when we managed to acquire the lock. This prevents any behaviour changes for users which rely on the EALREADY semantics. v2: Drop the cargo-culted __sched (I should read docs next time around) and annotate the non-debug case with inline to prevent gcc from doing something horrible. Cc: Steven Rostedt rost...@goodmis.org Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch --- include/linux/mutex.h |8 kernel/mutex.c| 32 lib/Kconfig.debug | 10 ++ 3 files changed, 50 insertions(+) diff --git a/include/linux/mutex.h b/include/linux/mutex.h index 004f863..82d56ec 100644 --- a/include/linux/mutex.h +++ b/include/linux/mutex.h @@ -93,6 +93,10 @@ struct ww_acquire_ctx { #ifdef CONFIG_DEBUG_LOCK_ALLOC struct lockdep_map dep_map; #endif +#ifdef CONFIG_DEBUG_WW_MUTEX_SLOWPATH + unsigned deadlock_inject_interval; + unsigned deadlock_inject_countdown; +#endif }; struct ww_mutex { @@ -278,6 +282,10 @@ static inline void ww_acquire_init(struct ww_acquire_ctx *ctx, ww_class-acquire_key, 0); mutex_acquire(ctx-dep_map, 0, 0, _RET_IP_); #endif +#ifdef CONFIG_DEBUG_WW_MUTEX_SLOWPATH + ctx-deadlock_inject_interval = ctx-stamp 0xf; + ctx-deadlock_inject_countdown = ctx-deadlock_inject_interval; +#endif } /** diff --git a/kernel/mutex.c b/kernel/mutex.c index 66807c7..816fdfc 100644 --- a/kernel/mutex.c +++ b/kernel/mutex.c @@ -827,6 +827,36 @@ int __sched mutex_trylock(struct mutex *lock) EXPORT_SYMBOL(mutex_trylock); #ifndef CONFIG_DEBUG_LOCK_ALLOC + +#ifdef CONFIG_DEBUG_WW_MUTEX_SLOWPATH +static int +ww_mutex_deadlock_injection(struct ww_mutex *lock, struct ww_acquire_ctx *ctx) +{ + if (ctx-deadlock_inject_countdown-- == 0) { + tmp = ctx-deadlock_inject_interval; + if (tmp UINT_MAX/4) + tmp = UINT_MAX; + else + tmp = tmp*2 + tmp + tmp/2; + + ctx-deadlock_inject_interval = tmp; + ctx-deadlock_inject_countdown = tmp; + + ww_mutex_unlock(lock); + + return -EDEADLK; + } + + return 0; +} +#else +static inline int +ww_mutex_deadlock_injection(struct ww_mutex *lock, struct ww_acquire_ctx *ctx) +{ + return 0; +} +#endif + int __sched ww_mutex_lock(struct ww_mutex *lock, struct ww_acquire_ctx *ctx) { @@ -839,6 +869,7 @@ ww_mutex_lock(struct ww_mutex *lock, struct ww_acquire_ctx *ctx) if (likely(!ret)) { ww_mutex_set_context_fastpath(lock, ctx); mutex_set_owner(lock-base); + return ww_mutex_deadlock_injection(lock, ctx); } else ret = __ww_mutex_lock_slowpath(lock, ctx); return ret; @@ -857,6 +888,7 @@ ww_mutex_lock_interruptible(struct ww_mutex *lock, struct ww_acquire_ctx *ctx) if (likely(!ret)) { ww_mutex_set_context_fastpath(lock, ctx); mutex_set_owner(lock-base); + return ww_mutex_deadlock_injection(lock, ctx); } else ret = __ww_mutex_lock_interruptible_slowpath(lock, ctx); return ret; diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug index 28be08c..8c41f73 100644 --- a/lib/Kconfig.debug +++ b/lib/Kconfig.debug @@ -547,6 +547,16 @@ config DEBUG_MUTEXES This feature allows mutex semantics violations to be detected and reported. +config
Re: [PATCH v2 1/4] V4L2: soc_camera: Renesas R-Car VIN driver
On Tue, 30 Apr 2013, Vladimir Barinov wrote: [snip] +static int rcar_vin_init_videobuf2(struct vb2_queue *vq, + struct soc_camera_device *icd) +{ +vq-type = V4L2_BUF_TYPE_VIDEO_CAPTURE; +vq-io_modes = VB2_MMAP | VB2_USERPTR; +vq-drv_priv = icd; +vq-ops = rcar_vin_vb2_ops; +vq-mem_ops = vb2_dma_contig_memops; +vq-buf_struct_size = sizeof(struct rcar_vin_buffer); Please, add vq-timestamp_type = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC; There is not such field in struct vb2_queue. Please, look at the current kernel, e.g. the next tree, within days this will be in Linus' tree too. Mainline submissions should always be developed against the newest kernel version. Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- 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