Re: [PATCH v4 2/7] media: s5p-mfc: use generic reserved memory bindings
Hello, On 2016-05-25 19:36, Rob Herring wrote: On Wed, May 25, 2016 at 11:18:59AM -0400, Javier Martinez Canillas wrote: Hello Marek, On 05/24/2016 09:31 AM, Marek Szyprowski wrote: Use generic reserved memory bindings and mark old, custom properties as obsoleted. Signed-off-by: Marek Szyprowski --- .../devicetree/bindings/media/s5p-mfc.txt | 39 +- 1 file changed, 31 insertions(+), 8 deletions(-) diff --git a/Documentation/devicetree/bindings/media/s5p-mfc.txt b/Documentation/devicetree/bindings/media/s5p-mfc.txt index 2d5787e..92c94f5 100644 --- a/Documentation/devicetree/bindings/media/s5p-mfc.txt +++ b/Documentation/devicetree/bindings/media/s5p-mfc.txt @@ -21,15 +21,18 @@ Required properties: - clock-names : from common clock binding: must contain "mfc", corresponding to entry in the clocks property. - - samsung,mfc-r : Base address of the first memory bank used by MFC - for DMA contiguous memory allocation and its size. - - - samsung,mfc-l : Base address of the second memory bank used by MFC - for DMA contiguous memory allocation and its size. - Optional properties: - power-domains : power-domain property defined with a phandle to respective power domain. + - memory-region : from reserved memory binding: phandles to two reserved + memory regions, first is for "left" mfc memory bus interfaces, + second if for the "right" mfc memory bus, used when no SYSMMU + support is available + +Obsolete properties: + - samsung,mfc-r, samsung,mfc-l : support removed, please use memory-region + property instead + I wonder if we should maintain backward compatibility for this driver since s5p-mfc memory allocation won't work with an old FDT if support for the old properties are removed. Well, minimally the commit log should indicate that compatibility is being broken. Compatibility is only partially broken. I add this to the commit message. Old bindings will still work with the new driver when IOMMU is enabled - in such case reserved memory regions are ignored so this should not be a big issue. Using IOMMU also increases total memory space for the video buffers without wasting it as 'reserved'. Hope that once those patches are merged, the IOMMU can be finally enabled in the exynos_defconfig. Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland -- 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 v4 2/7] media: s5p-mfc: use generic reserved memory bindings
On 05/24/2016 03:31 PM, Marek Szyprowski wrote: > Use generic reserved memory bindings and mark old, custom properties > as obsoleted. > > Signed-off-by: Marek Szyprowski > --- > .../devicetree/bindings/media/s5p-mfc.txt | 39 > +- > 1 file changed, 31 insertions(+), 8 deletions(-) > Acked-by: Krzysztof Kozlowski Best regards, Krzysztof -- 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
cron job: media_tree daily build: OK
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: Fri May 27 04:00:28 CEST 2016 git branch: test git hash: bc2b80ee3490651904f121eac1c8fb7652d48253 gcc version:i686-linux-gcc (GCC) 5.3.0 sparse version: v0.5.0-56-g7647c77 smatch version: v0.5.0-3428-gdfe27cf host hardware: x86_64 host os:4.5.0-264 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-bf561: 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.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.7-i686: OK linux-3.17.8-i686: OK linux-3.18.7-i686: OK linux-3.19-i686: OK linux-4.0-i686: OK linux-4.1.1-i686: OK linux-4.2-i686: OK linux-4.3-i686: OK linux-4.4-i686: OK linux-4.5-i686: OK linux-4.6-i686: 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.7-x86_64: OK linux-3.17.8-x86_64: OK linux-3.18.7-x86_64: OK linux-3.19-x86_64: OK linux-4.0-x86_64: OK linux-4.1.1-x86_64: OK linux-4.2-x86_64: OK linux-4.3-x86_64: OK linux-4.4-x86_64: OK linux-4.5-x86_64: OK linux-4.6-x86_64: OK apps: OK spec-git: OK sparse: WARNINGS smatch: WARNINGS Detailed results are available here: http://www.xs4all.nl/~hverkuil/logs/Friday.log Full logs are available here: http://www.xs4all.nl/~hverkuil/logs/Friday.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: How should I use kernel-defined i2c structs in this driver
On 05/26/2016 04:59 PM, Andrey Utkin wrote: Could anybody please give a hint - which kernel-defined i2c objects, and how many of them, I need to define and use to substitute these driver-defined functions i2c_read(), i2c_write() ? https://github.com/bluecherrydvr/linux/blob/release/tw5864/1.16/drivers/media/pci/tw5864/tw5864-config.c In a word, there's 4 chips with different addresses, to which this code communicates via main chip's dedicated registers. Do i need a single i2c_adapter or several? Do i need i2c_client entities? where should I put what is named "devid" here? Thanks in advance. It depends how those are connected at hardware level. Quickly looking I think "devid" is here to select proper I2C adapter. So I think there is 4 I2C adapters and each of those adapter has 1 slave device. Is that correct? If yes, then register 4 I2C adapters and register single client for each of those adapters. regards Antti -- 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
Re: [PATCH v4 1/7] of: reserved_mem: add support for using more than one region for given device
On Tue, May 24, 2016 at 8:31 AM, Marek Szyprowski wrote: > This patch allows device drivers to initialize more than one reserved > memory region assigned to given device. When driver needs to use more > than one reserved memory region, it should allocate child devices and > initialize regions by index for each of its child devices. > > Signed-off-by: Marek Szyprowski > --- > drivers/of/of_reserved_mem.c| 85 > +++-- > include/linux/of_reserved_mem.h | 25 ++-- > 2 files changed, 86 insertions(+), 24 deletions(-) Acked-by: Rob Herring -- 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
How should I use kernel-defined i2c structs in this driver
Could anybody please give a hint - which kernel-defined i2c objects, and how many of them, I need to define and use to substitute these driver-defined functions i2c_read(), i2c_write() ? https://github.com/bluecherrydvr/linux/blob/release/tw5864/1.16/drivers/media/pci/tw5864/tw5864-config.c In a word, there's 4 chips with different addresses, to which this code communicates via main chip's dedicated registers. Do i need a single i2c_adapter or several? Do i need i2c_client entities? where should I put what is named "devid" here? Thanks in advance. -- 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: [v4l-utils PATCH 1/2] libmediactl: Drop length argument from media_get_entity_by_name()
On Thu, May 26, 2016 at 03:07:41PM +0300, Laurent Pinchart wrote: > Hi Sakari, > > On Tuesday 24 May 2016 23:50:44 Sakari Ailus wrote: > > On Tue, May 24, 2016 at 08:09:37PM +0300, Laurent Pinchart wrote: > > ... > > > > > > + if (strcmp(entity->info.name, name) == 0) > > > > > > While the kernel API guarantees that entity->info.name will be NULL- > > > terminated, wouldn't it be safer to add a safety check here ? > > > > The kernel implementation in media-device.c does use strlcpy() so this is > > even not about drivers doing this right. If you insist, I'll change it. :-) > > I suppose we'll have other problems if the kernel doesn't behave. > > Acked-by: Laurent Pinchart Thanks! I'll then proceed to push the two patches to v4l-utils. -- 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
Re: [v4l-utils PATCH 1/2] libmediactl: Drop length argument from media_get_entity_by_name()
Hi Sakari, On Tuesday 24 May 2016 23:50:44 Sakari Ailus wrote: > On Tue, May 24, 2016 at 08:09:37PM +0300, Laurent Pinchart wrote: > ... > > > > + if (strcmp(entity->info.name, name) == 0) > > > > While the kernel API guarantees that entity->info.name will be NULL- > > terminated, wouldn't it be safer to add a safety check here ? > > The kernel implementation in media-device.c does use strlcpy() so this is > even not about drivers doing this right. If you insist, I'll change it. :-) I suppose we'll have other problems if the kernel doesn't behave. Acked-by: Laurent Pinchart -- 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 v2 2/2] media: Add a driver for the ov5645 camera sensor.
Hi Hans,
Thanks for the review.
On 05/23/2016 01:36 PM, Hans Verkuil wrote:
> Hi Todor,
>
> Thanks for the patch series! I got a few comments:
>
> On 05/18/2016 01:50 PM, Todor Tomov wrote:
>> The ov5645 sensor from Omnivision supports up to 2592x1944
>> and CSI2 interface.
>>
>> The driver adds support for the following modes:
>> - 1280x960
>> - 1920x1080
>> - 2592x1944
>>
>> Output format is packed 8bit UYVY.
>>
>> Signed-off-by: Todor Tomov
>> ---
>> drivers/media/i2c/Kconfig | 11 +
>> drivers/media/i2c/Makefile |1 +
>> drivers/media/i2c/ov5645.c | 1425
>>
>> 3 files changed, 1437 insertions(+)
>> create mode 100644 drivers/media/i2c/ov5645.c
>
>
>
>>
>> +static int ov5645_change_mode(struct ov5645 *ov5645, enum ov5645_mode mode)
>> +{
>> +struct reg_value *settings;
>> +u32 num_settings;
>> +int ret = 0;
>> +
>> +settings = ov5645_mode_info_data[mode].data;
>> +num_settings = ov5645_mode_info_data[mode].data_size;
>> +ret = ov5645_set_register_array(ov5645, settings, num_settings);
>
> Just do 'return ov5645_set_register_array(ov5645, settings, num_settings);'
> No need for the 'ret' variable.
Ok.
>
>> +
>> +return ret;
>> +}
>> +
>> +static int ov5645_set_power_on(struct ov5645 *ov5645)
>> +{
>> +int ret = 0;
>> +
>> +dev_dbg(ov5645->dev, "%s: Enter\n", __func__);
>> +
>> +ret = clk_prepare_enable(ov5645->xclk);
>> +if (ret < 0) {
>> +dev_err(ov5645->dev, "clk prepare enable failed\n");
>> +return ret;
>> +}
>> +
>> +ret = ov5645_regulators_enable(ov5645);
>> +if (ret < 0) {
>> +clk_disable_unprepare(ov5645->xclk);
>> +return ret;
>> +}
>> +
>> +usleep_range(5000, 15000);
>> +if (ov5645->pwdn_gpio)
>> +gpiod_set_value_cansleep(ov5645->pwdn_gpio, 1);
>> +
>> +usleep_range(1000, 2000);
>> +if (ov5645->rst_gpio)
>> +gpiod_set_value_cansleep(ov5645->rst_gpio, 1);
>> +msleep(20);
>> +
>> +return ret;
>> +}
>> +
>> +static void ov5645_set_power_off(struct ov5645 *ov5645)
>> +{
>> +dev_dbg(ov5645->dev, "%s: Enter\n", __func__);
>> +
>> +if (ov5645->rst_gpio)
>> +gpiod_set_value_cansleep(ov5645->rst_gpio, 0);
>> +if (ov5645->pwdn_gpio)
>> +gpiod_set_value_cansleep(ov5645->pwdn_gpio, 0);
>> +ov5645_regulators_disable(ov5645);
>> +clk_disable_unprepare(ov5645->xclk);
>> +}
>> +
>> +static int ov5645_s_power(struct v4l2_subdev *sd, int on)
>> +{
>> +struct ov5645 *ov5645 = to_ov5645(sd);
>> +int ret = 0;
>> +
>> +dev_dbg(ov5645->dev, "%s: on = %d\n", __func__, on);
>> +
>> +mutex_lock(&ov5645->power_lock);
>> +
>> +/* If the power count is modified from 0 to != 0 or from != 0 to 0,
>> + * update the power state.
>> + */
>> +if (ov5645->power == !on) {
>> +if (on) {
>> +ret = ov5645_set_power_on(ov5645);
>> +if (ret < 0) {
>> +dev_err(ov5645->dev, "could not set power %s\n",
>> +on ? "on" : "off");
>> +goto exit;
>> +}
>> +
>> +ret = ov5645_init(ov5645);
>> +if (ret < 0) {
>> +dev_err(ov5645->dev,
>> +"could not set init registers\n");
>> +goto exit;
>> +}
>> +
>> +ov5645_write_reg(ov5645, OV5645_SYSTEM_CTRL0,
>> + OV5645_SYSTEM_CTRL0_STOP);
>> +} else {
>> +ov5645_set_power_off(ov5645);
>> +}
>> +
>> +/* Update the power count. */
>> +ov5645->power += on ? 1 : -1;
>
> Huh? Is ov5645->power a bool or a counter? If it is a counter then this line
> should be outside this 'if'. If it is a bool, then the comments (and the
> power field itself!) should be updated.
>
> As far as I can tell, the 'power' field should be a bool and everything should
> be updated accordingly.
Yes, I'll change it to bool.
>
>
>> +WARN_ON(ov5645->power < 0);
>> +}
>> +
>> +exit:
>> +mutex_unlock(&ov5645->power_lock);
>> +
>> +return ret;
>> +}
>> +
>> +
>> +static int ov5645_set_saturation(struct ov5645 *ov5645, s32 value)
>> +{
>> +u32 reg_value = (value * 0x10) + 0x40;
>> +int ret = 0;
>> +
>> +ret |= ov5645_write_reg(ov5645, OV5645_SDE_SAT_U, reg_value);
>> +ret |= ov5645_write_reg(ov5645, OV5645_SDE_SAT_V, reg_value);
>> +
>> +dev_dbg(ov5645->dev, "%s: value = %d\n", __func__, value);
>> +
>> +return ret;
>> +}
>> +
>> +static int ov5645_set_hflip(struct ov5645 *ov5645, s32 value)
>> +{
>> +u8 val;
>> +
>> +ov5645_read_reg(ov5645, OV5645_TIMING_TC_REG21, &val);
>> +if (value == 0)
>> +val &= ~(OV5645_SENSOR_MIRROR);
>> +else
>> +
