Re: [PATCH v2] media: ov13858: Fix 4224x3136 video flickering at some vblanks

[Tomasz Figa]

Hi Chiranjeevi,

On Tue, Sep 19, 2017 at 7:47 AM, Chiranjeevi Rapolu
 wrote:
> Previously, with crop (0, 0), (4255, 3167), VTS < 0xC9E was resulting in blank
> frames sometimes. This appeared as video flickering. But we need VTS < 0xC9E 
> to
> get ~30fps.
>
> Omni Vision recommends to use crop (0,8), (4255, 3159) for 4224x3136. With 
> this
> crop, VTS 0xC8E is supported and yields ~30fps.
>
> Signed-off-by: Chiranjeevi Rapolu 
> ---
> Changes in v2:
> - Include Tomasz clarifications in the commit message.

Thanks for explanation. It makes perfect sense now.

Reviewed-by: Tomasz Figa 

Best regards,
Tomasz

cron job: media_tree daily build: WARNINGS

[Hans Verkuil]

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 Sep 19 05:00:19 CEST 2017
media-tree git hash:1efdf1776e2253b77413c997bed862410e4b6aaf
media_build git hash:   19087750b61fc0c5528e798c47ff845f9234
v4l-utils git hash: 8d89a6ad9940f520ae4e816c3973551e52557d71
gcc version:i686-linux-gcc (GCC) 7.1.0
sparse version: v0.5.0
smatch version: v0.5.0-3553-g78b2ea6
host hardware:  x86_64
host os:4.12.0-164

linux-git-arm-at91: OK
linux-git-arm-davinci: OK
linux-git-arm-multi: OK
linux-git-arm-pxa: OK
linux-git-arm-stm32: 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: 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: WARNINGS
linux-3.9.2-i686: WARNINGS
linux-3.10.1-i686: WARNINGS
linux-3.11.1-i686: WARNINGS
linux-3.12.67-i686: WARNINGS
linux-3.13.11-i686: WARNINGS
linux-3.14.9-i686: WARNINGS
linux-3.15.2-i686: WARNINGS
linux-3.16.7-i686: WARNINGS
linux-3.17.8-i686: WARNINGS
linux-3.18.7-i686: WARNINGS
linux-3.19-i686: WARNINGS
linux-4.0.9-i686: WARNINGS
linux-4.1.33-i686: WARNINGS
linux-4.2.8-i686: WARNINGS
linux-4.3.6-i686: WARNINGS
linux-4.4.22-i686: WARNINGS
linux-4.5.7-i686: WARNINGS
linux-4.6.7-i686: WARNINGS
linux-4.7.5-i686: WARNINGS
linux-4.8-i686: OK
linux-4.9.26-i686: OK
linux-4.10.14-i686: OK
linux-4.11-i686: OK
linux-4.12.1-i686: OK
linux-4.13-i686: OK
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: WARNINGS
linux-3.9.2-x86_64: WARNINGS
linux-3.10.1-x86_64: WARNINGS
linux-3.11.1-x86_64: WARNINGS
linux-3.12.67-x86_64: WARNINGS
linux-3.13.11-x86_64: WARNINGS
linux-3.14.9-x86_64: WARNINGS
linux-3.15.2-x86_64: WARNINGS
linux-3.16.7-x86_64: WARNINGS
linux-3.17.8-x86_64: WARNINGS
linux-3.18.7-x86_64: WARNINGS
linux-3.19-x86_64: WARNINGS
linux-4.0.9-x86_64: WARNINGS
linux-4.1.33-x86_64: WARNINGS
linux-4.2.8-x86_64: WARNINGS
linux-4.3.6-x86_64: WARNINGS
linux-4.4.22-x86_64: WARNINGS
linux-4.5.7-x86_64: WARNINGS
linux-4.6.7-x86_64: WARNINGS
linux-4.7.5-x86_64: WARNINGS
linux-4.8-x86_64: WARNINGS
linux-4.9.26-x86_64: WARNINGS
linux-4.10.14-x86_64: WARNINGS
linux-4.11-x86_64: WARNINGS
linux-4.12.1-x86_64: WARNINGS
linux-4.13-x86_64: OK
apps: OK
spec-git: OK

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/index.html

[PATCH v5 3/3] media: ov7670: Add the s_power operation

[Wenyou Yang]

Add the s_power operation which is responsible for manipulating the
power dowm mode through the PWDN pin and the reset operation through
the RESET pin.

Signed-off-by: Wenyou Yang 
---

Changes in v5: None
Changes in v4: None
Changes in v3: None
Changes in v2:
 - Add the patch to support the get_fmt ops.
 - Remove the redundant invoking ov7670_init_gpio().

 drivers/media/i2c/ov7670.c | 32 +++-
 1 file changed, 27 insertions(+), 5 deletions(-)

diff --git a/drivers/media/i2c/ov7670.c b/drivers/media/i2c/ov7670.c
index 73ceec63a8ca..8e86479d8a24 100644
--- a/drivers/media/i2c/ov7670.c
+++ b/drivers/media/i2c/ov7670.c
@@ -1544,6 +1544,22 @@ static int ov7670_s_register(struct v4l2_subdev *sd, 
const struct v4l2_dbg_regis
 }
 #endif
 
+static int ov7670_s_power(struct v4l2_subdev *sd, int on)
+{
+   struct ov7670_info *info = to_state(sd);
+
+   if (info->pwdn_gpio)
+   gpiod_direction_output(info->pwdn_gpio, !on);
+   if (on && info->resetb_gpio) {
+   gpiod_set_value(info->resetb_gpio, 1);
+   usleep_range(500, 1000);
+   gpiod_set_value(info->resetb_gpio, 0);
+   usleep_range(3000, 5000);
+   }
+
+   return 0;
+}
+
 static void ov7670_get_default_format(struct v4l2_subdev *sd,
  struct v4l2_mbus_framefmt *format)
 {
@@ -1577,6 +1593,7 @@ static const struct v4l2_subdev_core_ops ov7670_core_ops 
= {
.g_register = ov7670_g_register,
.s_register = ov7670_s_register,
 #endif
+   .s_power = ov7670_s_power,
 };
 
 static const struct v4l2_subdev_video_ops ov7670_video_ops = {
@@ -1694,23 +1711,25 @@ static int ov7670_probe(struct i2c_client *client,
if (ret)
return ret;
 
-   ret = ov7670_init_gpio(client, info);
-   if (ret)
-   goto clk_disable;
-
info->clock_speed = clk_get_rate(info->clk) / 100;
if (info->clock_speed < 10 || info->clock_speed > 48) {
ret = -EINVAL;
goto clk_disable;
}
 
+   ret = ov7670_init_gpio(client, info);
+   if (ret)
+   goto clk_disable;
+
+   ov7670_s_power(sd, 1);
+
/* Make sure it's an ov7670 */
ret = ov7670_detect(sd);
if (ret) {
v4l_dbg(1, debug, client,
"chip found @ 0x%x (%s) is not an ov7670 chip.\n",
client->addr << 1, client->adapter->name);
-   goto clk_disable;
+   goto power_off;
}
v4l_info(client, "chip found @ 0x%02x (%s)\n",
client->addr << 1, client->adapter->name);
@@ -1789,6 +1808,8 @@ static int ov7670_probe(struct i2c_client *client,
 #endif
 hdl_free:
v4l2_ctrl_handler_free(>hdl);
+power_off:
+   ov7670_s_power(sd, 0);
 clk_disable:
clk_disable_unprepare(info->clk);
return ret;
@@ -1806,6 +1827,7 @@ static int ov7670_remove(struct i2c_client *client)
 #if defined(CONFIG_MEDIA_CONTROLLER)
media_entity_cleanup(>sd.entity);
 #endif
+   ov7670_s_power(sd, 0);
return 0;
 }
 
-- 
2.13.0

[PATCH v5 2/3] media: ov7670: Add the get_fmt callback

[Wenyou Yang]

Add the get_fmt callback, also enable V4L2_SUBDEV_FL_HAS_DEVNODE flag
to make this subdev has device node.

Signed-off-by: Wenyou Yang 
---

Changes in v5:
 - Fix the build warning on the declaration *mbus_fmt(ISO C90 forbids mixed).

Changes in v4:
 - Fix the build error when not enabling V4L2 sub-device userspace API option.

Changes in v3:
 - Keep tried format info in the try_fmt member of
   v4l2_subdev__pad_config struct.
 - Add the internal_ops callback to set default format.

Changes in v2: None

 drivers/media/i2c/ov7670.c | 77 +-
 1 file changed, 76 insertions(+), 1 deletion(-)

diff --git a/drivers/media/i2c/ov7670.c b/drivers/media/i2c/ov7670.c
index 553945d4ca28..73ceec63a8ca 100644
--- a/drivers/media/i2c/ov7670.c
+++ b/drivers/media/i2c/ov7670.c
@@ -232,6 +232,7 @@ struct ov7670_info {
struct v4l2_ctrl *saturation;
struct v4l2_ctrl *hue;
};
+   struct v4l2_mbus_framefmt format;
struct ov7670_format_struct *fmt;  /* Current format */
struct clk *clk;
struct gpio_desc *resetb_gpio;
@@ -975,6 +976,9 @@ static int ov7670_try_fmt_internal(struct v4l2_subdev *sd,
fmt->width = wsize->width;
fmt->height = wsize->height;
fmt->colorspace = ov7670_formats[index].colorspace;
+
+   info->format = *fmt;
+
return 0;
 }
 
@@ -988,6 +992,9 @@ static int ov7670_set_fmt(struct v4l2_subdev *sd,
struct ov7670_format_struct *ovfmt;
struct ov7670_win_size *wsize;
struct ov7670_info *info = to_state(sd);
+#ifdef CONFIG_VIDEO_V4L2_SUBDEV_API
+   struct v4l2_mbus_framefmt *mbus_fmt;
+#endif
unsigned char com7;
int ret;
 
@@ -998,8 +1005,13 @@ static int ov7670_set_fmt(struct v4l2_subdev *sd,
ret = ov7670_try_fmt_internal(sd, >format, NULL, NULL);
if (ret)
return ret;
-   cfg->try_fmt = format->format;
+#ifdef CONFIG_VIDEO_V4L2_SUBDEV_API
+   mbus_fmt = v4l2_subdev_get_try_format(sd, cfg, format->pad);
+   *mbus_fmt = format->format;
return 0;
+#else
+   return -ENOTTY;
+#endif
}
 
ret = ov7670_try_fmt_internal(sd, >format, , );
@@ -1041,6 +1053,30 @@ static int ov7670_set_fmt(struct v4l2_subdev *sd,
return 0;
 }
 
+static int ov7670_get_fmt(struct v4l2_subdev *sd,
+ struct v4l2_subdev_pad_config *cfg,
+ struct v4l2_subdev_format *format)
+{
+   struct ov7670_info *info = to_state(sd);
+#ifdef CONFIG_VIDEO_V4L2_SUBDEV_API
+   struct v4l2_mbus_framefmt *mbus_fmt;
+#endif
+
+   if (format->which == V4L2_SUBDEV_FORMAT_TRY) {
+#ifdef CONFIG_VIDEO_V4L2_SUBDEV_API
+   mbus_fmt = v4l2_subdev_get_try_format(sd, cfg, 0);
+   format->format = *mbus_fmt;
+   return 0;
+#else
+   return -ENOTTY;
+#endif
+   } else {
+   format->format = info->format;
+   }
+
+   return 0;
+}
+
 /*
  * Implement G/S_PARM.  There is a "high quality" mode we could try
  * to do someday; for now, we just do the frame rate tweak.
@@ -1508,6 +1544,30 @@ static int ov7670_s_register(struct v4l2_subdev *sd, 
const struct v4l2_dbg_regis
 }
 #endif
 
+static void ov7670_get_default_format(struct v4l2_subdev *sd,
+ struct v4l2_mbus_framefmt *format)
+{
+   struct ov7670_info *info = to_state(sd);
+
+   format->width = info->devtype->win_sizes[0].width;
+   format->height = info->devtype->win_sizes[0].height;
+   format->colorspace = info->fmt->colorspace;
+   format->code = info->fmt->mbus_code;
+   format->field = V4L2_FIELD_NONE;
+}
+
+#ifdef CONFIG_VIDEO_V4L2_SUBDEV_API
+static int ov7670_open(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh)
+{
+   struct v4l2_mbus_framefmt *format =
+   v4l2_subdev_get_try_format(sd, fh->pad, 0);
+
+   ov7670_get_default_format(sd, format);
+
+   return 0;
+}
+#endif
+
 /* --- */
 
 static const struct v4l2_subdev_core_ops ov7670_core_ops = {
@@ -1528,6 +1588,7 @@ static const struct v4l2_subdev_pad_ops ov7670_pad_ops = {
.enum_frame_interval = ov7670_enum_frame_interval,
.enum_frame_size = ov7670_enum_frame_size,
.enum_mbus_code = ov7670_enum_mbus_code,
+   .get_fmt = ov7670_get_fmt,
.set_fmt = ov7670_set_fmt,
 };
 
@@ -1537,6 +1598,12 @@ static const struct v4l2_subdev_ops ov7670_ops = {
.pad = _pad_ops,
 };
 
+#ifdef CONFIG_VIDEO_V4L2_SUBDEV_API
+static const struct v4l2_subdev_internal_ops ov7670_subdev_internal_ops = {
+   .open = ov7670_open,
+};
+#endif
+
 /* --- */
 
 static const struct ov7670_devtype ov7670_devdata[] = {
@@ -1589,6 +1656,11 @@ 

[PATCH v5 1/3] media: ov7670: Add entity pads initialization

[Wenyou Yang]

Add the media entity pads initialization.

Signed-off-by: Wenyou Yang 
---

Changes in v5: None
Changes in v4:
 - Fix the build error when not enabling Media Controller API option.

Changes in v3: None
Changes in v2: None

 drivers/media/i2c/ov7670.c | 21 -
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/drivers/media/i2c/ov7670.c b/drivers/media/i2c/ov7670.c
index e88549f0e704..553945d4ca28 100644
--- a/drivers/media/i2c/ov7670.c
+++ b/drivers/media/i2c/ov7670.c
@@ -213,6 +213,9 @@ struct ov7670_devtype {
 struct ov7670_format_struct;  /* coming later */
 struct ov7670_info {
struct v4l2_subdev sd;
+#if defined(CONFIG_MEDIA_CONTROLLER)
+   struct media_pad pad;
+#endif
struct v4l2_ctrl_handler hdl;
struct {
/* gain cluster */
@@ -1688,14 +1691,27 @@ static int ov7670_probe(struct i2c_client *client,
v4l2_ctrl_auto_cluster(2, >auto_exposure,
   V4L2_EXPOSURE_MANUAL, false);
v4l2_ctrl_cluster(2, >saturation);
+
+#if defined(CONFIG_MEDIA_CONTROLLER)
+   info->pad.flags = MEDIA_PAD_FL_SOURCE;
+   info->sd.entity.function = MEDIA_ENT_F_CAM_SENSOR;
+   ret = media_entity_pads_init(>sd.entity, 1, >pad);
+   if (ret < 0)
+   goto hdl_free;
+#endif
+
v4l2_ctrl_handler_setup(>hdl);
 
ret = v4l2_async_register_subdev(>sd);
if (ret < 0)
-   goto hdl_free;
+   goto entity_cleanup;
 
return 0;
 
+entity_cleanup:
+#if defined(CONFIG_MEDIA_CONTROLLER)
+   media_entity_cleanup(>sd.entity);
+#endif
 hdl_free:
v4l2_ctrl_handler_free(>hdl);
 clk_disable:
@@ -1712,6 +1728,9 @@ static int ov7670_remove(struct i2c_client *client)
v4l2_device_unregister_subdev(sd);
v4l2_ctrl_handler_free(>hdl);
clk_disable_unprepare(info->clk);
+#if defined(CONFIG_MEDIA_CONTROLLER)
+   media_entity_cleanup(>sd.entity);
+#endif
return 0;
 }
 
-- 
2.13.0

[PATCH v5 0/3] media: ov7670: Add entity init and power operation

[Wenyou Yang]

This patch set is to add the media entity pads initialization,
the s_power operation and get_fmt callback support.

Changes in v5:
 - Fix the build warning on the declaration *mbus_fmt(ISO C90 forbids mixed).

Changes in v4:
 - Fix the build error when not enabling Media Controller API option.
 - Fix the build error when not enabling V4L2 sub-device userspace API option.

Changes in v3:
 - Keep tried format info in the try_fmt member of
   v4l2_subdev__pad_config struct.
 - Add the internal_ops callback to set default format.

Changes in v2:
 - Add the patch to support the get_fmt ops.
 - Remove the redundant invoking ov7670_init_gpio().

Wenyou Yang (3):
  media: ov7670: Add entity pads initialization
  media: ov7670: Add the get_fmt callback
  media: ov7670: Add the s_power operation

 drivers/media/i2c/ov7670.c | 130 ++---
 1 file changed, 123 insertions(+), 7 deletions(-)

-- 
2.13.0

Re: [PATCH] media: imx: Fix VDIC CSI1 selection

[Steve Longerbeam]

Thanks Tim for catching this error.

Acked-by: Steve Longerbeam 


On 09/18/2017 04:08 PM, Tim Harvey wrote:

When using VDIC with CSI1, make sure to select the correct CSI in
IPU_CONF.

Fixes: f0d9c8924e2c3376 ("[media] media: imx: Add IC subdev drivers")
Suggested-by: Marek Vasut 
Signed-off-by: Tim Harvey 
---
  drivers/staging/media/imx/imx-ic-prp.c | 5 +++--
  1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/media/imx/imx-ic-prp.c 
b/drivers/staging/media/imx/imx-ic-prp.c
index c2bb5ef..9e41987 100644
--- a/drivers/staging/media/imx/imx-ic-prp.c
+++ b/drivers/staging/media/imx/imx-ic-prp.c
@@ -320,9 +320,10 @@ static int prp_link_validate(struct v4l2_subdev *sd,
 * the ->PRPENC link cannot be enabled if the source
 * is the VDIC
 */
-   if (priv->sink_sd_prpenc)
+   if (priv->sink_sd_prpenc) {
ret = -EINVAL;
-   goto out;
+   goto out;
+   }
} else {
/* the source is a CSI */
if (!csi) {

[PATCH] media: imx: Fix VDIC CSI1 selection

[Tim Harvey]

When using VDIC with CSI1, make sure to select the correct CSI in
IPU_CONF.

Fixes: f0d9c8924e2c3376 ("[media] media: imx: Add IC subdev drivers")
Suggested-by: Marek Vasut 
Signed-off-by: Tim Harvey 
---
 drivers/staging/media/imx/imx-ic-prp.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/media/imx/imx-ic-prp.c 
b/drivers/staging/media/imx/imx-ic-prp.c
index c2bb5ef..9e41987 100644
--- a/drivers/staging/media/imx/imx-ic-prp.c
+++ b/drivers/staging/media/imx/imx-ic-prp.c
@@ -320,9 +320,10 @@ static int prp_link_validate(struct v4l2_subdev *sd,
 * the ->PRPENC link cannot be enabled if the source
 * is the VDIC
 */
-   if (priv->sink_sd_prpenc)
+   if (priv->sink_sd_prpenc) {
ret = -EINVAL;
-   goto out;
+   goto out;
+   }
} else {
/* the source is a CSI */
if (!csi) {
-- 
2.7.4

[PATCH v2] media: ov13858: Fix 4224x3136 video flickering at some vblanks

[Chiranjeevi Rapolu]

Previously, with crop (0, 0), (4255, 3167), VTS < 0xC9E was resulting in blank
frames sometimes. This appeared as video flickering. But we need VTS < 0xC9E to
get ~30fps.

Omni Vision recommends to use crop (0,8), (4255, 3159) for 4224x3136. With this
crop, VTS 0xC8E is supported and yields ~30fps.

Signed-off-by: Chiranjeevi Rapolu 
---
Changes in v2:
- Include Tomasz clarifications in the commit message.
 drivers/media/i2c/ov13858.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/i2c/ov13858.c b/drivers/media/i2c/ov13858.c
index af7af0d..f7c5771 100644
--- a/drivers/media/i2c/ov13858.c
+++ b/drivers/media/i2c/ov13858.c
@@ -238,11 +238,11 @@ struct ov13858_mode {
{0x3800, 0x00},
{0x3801, 0x00},
{0x3802, 0x00},
-   {0x3803, 0x00},
+   {0x3803, 0x08},
{0x3804, 0x10},
{0x3805, 0x9f},
{0x3806, 0x0c},
-   {0x3807, 0x5f},
+   {0x3807, 0x57},
{0x3808, 0x10},
{0x3809, 0x80},
{0x380a, 0x0c},
-- 
1.9.1

RE: [PATCH v1] media: ov13858: Fix 4224x3136 video flickering at some vblanks

[Rapolu, Chiranjeevi]

Hi Tomasz,

>The change of register values itself doesn't give any information about what 
>is changed. Could you explain the following:
>- What is the "crop" in this case?

The new crop is (0,8), (4255, 3159).

>- What value was it set to before and why was it wrong?

Old crop was (0, 0), (4255, 3167). With this crop, VTS < 0xC9E was resulting in
blank frames sometimes. But we need VTS < 0xC9E to get ~30fps.

>- What is the new value and why is it good?

With new crop as above,  VTS 0xC8E is supported and yields ~30fps.

Chiran.

Re: [PATCH v10 20/24] dt: bindings: smiapp: Document lens-focus and flash properties

[Sakari Ailus]

Hi Rob,

Rob Herring wrote:

On Mon, Sep 11, 2017 at 11:00:04AM +0300, Sakari Ailus wrote:

Document optional lens-focus and flash properties for the smiapp driver.

Signed-off-by: Sakari Ailus 
---
 Documentation/devicetree/bindings/media/i2c/nokia,smia.txt | 2 ++
 1 file changed, 2 insertions(+)


Acked-by: Rob Herring 


Thanks for the ack. There have been since a few iterations of the set, 
and the corresponding patch in v13 has minor changes to this:




Essentially "flash" was renamed to "flash-leds" as the current flash 
devices we have are all LEDs and the referencing assumes LED framework's 
ways to describe LEDs. The same change is present in the patch adding 
the property to video-interfaces.txt:




--
Regards,

Sakari Ailus
sakari.ai...@linux.intel.com

Re: [PATCH v10 20/24] dt: bindings: smiapp: Document lens-focus and flash properties

[Rob Herring]

On Mon, Sep 11, 2017 at 11:00:04AM +0300, Sakari Ailus wrote:
> Document optional lens-focus and flash properties for the smiapp driver.
> 
> Signed-off-by: Sakari Ailus 
> ---
>  Documentation/devicetree/bindings/media/i2c/nokia,smia.txt | 2 ++
>  1 file changed, 2 insertions(+)

Acked-by: Rob Herring 

Re: [RESEND PATCH v2 4/6] dt: bindings: as3645a: Improve label documentation, DT example

[Pavel Machek]

On Mon 2017-09-18 17:49:23, Sakari Ailus wrote:
> Hi Pavel,
> 
> On Mon, Sep 18, 2017 at 12:56:55PM +0200, Pavel Machek wrote:
> > Hi!
> > 
> > > Specify the exact label used if the label property is omitted in DT, as
> > > well as use label in the example that conforms to LED device naming.
> > > 
> > > @@ -69,11 +73,11 @@ Example
> > >   flash-max-microamp = <32>;
> > >   led-max-microamp = <6>;
> > >   ams,input-max-microamp = <175>;
> > > - label = "as3645a:flash";
> > > + label = "as3645a:white:flash";
> > >   };
> > >   indicator@1 {
> > >   reg = <0x1>;
> > >   led-max-microamp = <1>;
> > > - label = "as3645a:indicator";
> > > + label = "as3645a:red:indicator";
> > >   };
> > >   };
> > 
> > Ok, but userspace still has no chance to determine if this is flash
> > from main camera or flash for front camera; todays smartphones have
> > flashes on both cameras.
> > 
> > So.. Can I suggset as3645a:white:main_camera_flash or main_flash or
> > ?
> 
> If there's just a single one in the device, could you use that?
> 
> Even if we name this so for N9 (and N900), the application still would only
> work with the two devices.

Well, I'd plan to name it on other devices, too.

> My suggestion would be to look for a flash LED, and perhaps the maximum
> current as well. That should generally work better than assumptions on the
> label.

If you just look for flash LED, you don't know if it is front one or
back one. Its true that if you have just one flash it is usually on
the back camera, but you can't know if maybe driver is not available
for the main flash.

Lets get this right, please "main_camera_flash" is 12 bytes more than
"flash", and it saves application logic.. more than 12 bytes, I'm sure. 

> For association with a particular camera --- in the long run I'd propose to
> use Media controller / property API for that in the long run. We don't have
> that yet though.

We don't have that yet. Plus simple applications may not want to talk
v4l2 ioctls

Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature

[PATCH v3] media: ov13858: Calculate pixel-rate at runtime, use mode

[Chiranjeevi Rapolu]

Calculate pixel-rate at run time instead of compile time.

Instead of using hardcoded pixels-per-line, extract it from current sensor
mode.

Signed-off-by: Chiranjeevi Rapolu 
---
Changes in v3:
- Use LINK_FREQ_TO_PIXEL_RATE macro.
- Fix incorrect indentation.
 drivers/media/i2c/ov13858.c | 49 +++--
 1 file changed, 29 insertions(+), 20 deletions(-)

diff --git a/drivers/media/i2c/ov13858.c b/drivers/media/i2c/ov13858.c
index af7af0d..4e331b4 100644
--- a/drivers/media/i2c/ov13858.c
+++ b/drivers/media/i2c/ov13858.c
@@ -104,7 +104,6 @@ struct ov13858_reg_list {
 
 /* Link frequency config */
 struct ov13858_link_freq_config {
-   u32 pixel_rate;
u32 pixels_per_line;
 
/* PLL registers for this link frequency */
@@ -948,6 +947,12 @@ struct ov13858_mode {
 #define OV13858_LINK_FREQ_INDEX_0  0
 #define OV13858_LINK_FREQ_INDEX_1  1
 
+/*
+ * pixel_rate = link_freq * data-rate * nr_of_lanes / bits_per_sample
+ * data rate => double data rate; number of lanes => 4; bits per pixel => 10
+ */
+#define LINK_FREQ_TO_PIXEL_RATE(f) (((f) * 2 * 4) / 10)
+
 /* Menu items for LINK_FREQ V4L2 control */
 static const s64 link_freq_menu_items[OV13858_NUM_OF_LINK_FREQS] = {
OV13858_LINK_FREQ_540MHZ,
@@ -958,8 +963,6 @@ struct ov13858_mode {
 static const struct ov13858_link_freq_config
link_freq_configs[OV13858_NUM_OF_LINK_FREQS] = {
{
-   /* pixel_rate = link_freq * 2 * nr_of_lanes / bits_per_sample */
-   .pixel_rate = (OV13858_LINK_FREQ_540MHZ * 2 * 4) / 10,
.pixels_per_line = OV13858_PPL_540MHZ,
.reg_list = {
.num_of_regs = ARRAY_SIZE(mipi_data_rate_1080mbps),
@@ -967,8 +970,6 @@ struct ov13858_mode {
}
},
{
-   /* pixel_rate = link_freq * 2 * nr_of_lanes / bits_per_sample */
-   .pixel_rate = (OV13858_LINK_FREQ_270MHZ * 2 * 4) / 10,
.pixels_per_line = OV13858_PPL_270MHZ,
.reg_list = {
.num_of_regs = ARRAY_SIZE(mipi_data_rate_540mbps),
@@ -1385,6 +1386,8 @@ static int ov13858_get_pad_format(struct v4l2_subdev *sd,
s32 vblank_def;
s32 vblank_min;
s64 h_blank;
+   s64 pixel_rate;
+   s64 link_freq;
 
mutex_lock(>mutex);
 
@@ -1400,9 +1403,10 @@ static int ov13858_get_pad_format(struct v4l2_subdev *sd,
} else {
ov13858->cur_mode = mode;
__v4l2_ctrl_s_ctrl(ov13858->link_freq, mode->link_freq_index);
-   __v4l2_ctrl_s_ctrl_int64(
-   ov13858->pixel_rate,
-   link_freq_configs[mode->link_freq_index].pixel_rate);
+   link_freq = link_freq_menu_items[mode->link_freq_index];
+   pixel_rate = LINK_FREQ_TO_PIXEL_RATE(link_freq);
+   __v4l2_ctrl_s_ctrl_int64(ov13858->pixel_rate, pixel_rate);
+
/* Update limits and set FPS to default */
vblank_def = ov13858->cur_mode->vts_def -
 ov13858->cur_mode->height;
@@ -1617,6 +1621,10 @@ static int ov13858_init_controls(struct ov13858 *ov13858)
s64 exposure_max;
s64 vblank_def;
s64 vblank_min;
+   s64 hblank;
+   s64 pixel_rate_min;
+   s64 pixel_rate_max;
+   const struct ov13858_mode *mode;
int ret;
 
ctrl_hdlr = >ctrl_handler;
@@ -1634,29 +1642,30 @@ static int ov13858_init_controls(struct ov13858 
*ov13858)
link_freq_menu_items);
ov13858->link_freq->flags |= V4L2_CTRL_FLAG_READ_ONLY;
 
+   pixel_rate_max = LINK_FREQ_TO_PIXEL_RATE(link_freq_menu_items[0]);
+   pixel_rate_min = LINK_FREQ_TO_PIXEL_RATE(link_freq_menu_items[1]);
/* By default, PIXEL_RATE is read only */
ov13858->pixel_rate = v4l2_ctrl_new_std(ctrl_hdlr, _ctrl_ops,
-   V4L2_CID_PIXEL_RATE, 0,
-   link_freq_configs[0].pixel_rate, 1,
-   link_freq_configs[0].pixel_rate);
+   V4L2_CID_PIXEL_RATE,
+   pixel_rate_min, pixel_rate_max,
+   1, pixel_rate_max);
 
-   vblank_def = ov13858->cur_mode->vts_def - ov13858->cur_mode->height;
-   vblank_min = ov13858->cur_mode->vts_min - ov13858->cur_mode->height;
+   mode = ov13858->cur_mode;
+   vblank_def = mode->vts_def - mode->height;
+   vblank_min = mode->vts_min - mode->height;
ov13858->vblank = v4l2_ctrl_new_std(
ctrl_hdlr, _ctrl_ops, V4L2_CID_VBLANK,
-   vblank_min,
-   OV13858_VTS_MAX - ov13858->cur_mode->height, 1,
+   

[PATCH 2/2] [media] vicam: Return directly after a failed kmalloc() in vicam_dostream()

[SF Markus Elfring]

From: Markus Elfring 
Date: Mon, 18 Sep 2017 21:56:55 +0200

* Return directly after a call of the function "kmalloc" failed
  at the beginning.

* Delete the jump target "exit" which became unnecessary
  with this refactoring.

Signed-off-by: Markus Elfring 
---
 drivers/media/usb/gspca/vicam.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/usb/gspca/vicam.c b/drivers/media/usb/gspca/vicam.c
index 15b6887a8e97..11508ab283cd 100644
--- a/drivers/media/usb/gspca/vicam.c
+++ b/drivers/media/usb/gspca/vicam.c
@@ -184,7 +184,7 @@ static void vicam_dostream(struct work_struct *work)
   HEADER_SIZE;
buffer = kmalloc(frame_sz, GFP_KERNEL | GFP_DMA);
if (!buffer)
-   goto exit;
+   return;
 
while (gspca_dev->present && gspca_dev->streaming) {
 #ifdef CONFIG_PM
@@ -205,7 +205,7 @@ static void vicam_dostream(struct work_struct *work)
frame_sz - HEADER_SIZE);
gspca_frame_add(gspca_dev, LAST_PACKET, NULL, 0);
}
-exit:
+
kfree(buffer);
 }
 
-- 
2.14.1

[PATCH 1/2] [media] vicam: Delete an error message for a failed memory allocation in vicam_dostream()

[SF Markus Elfring]

From: Markus Elfring 
Date: Mon, 18 Sep 2017 21:48:55 +0200

Omit an extra message for a memory allocation failure in this function.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring 
---
 drivers/media/usb/gspca/vicam.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/media/usb/gspca/vicam.c b/drivers/media/usb/gspca/vicam.c
index 554b90ef2200..15b6887a8e97 100644
--- a/drivers/media/usb/gspca/vicam.c
+++ b/drivers/media/usb/gspca/vicam.c
@@ -183,10 +183,8 @@ static void vicam_dostream(struct work_struct *work)
frame_sz = gspca_dev->cam.cam_mode[gspca_dev->curr_mode].sizeimage +
   HEADER_SIZE;
buffer = kmalloc(frame_sz, GFP_KERNEL | GFP_DMA);
-   if (!buffer) {
-   pr_err("Couldn't allocate USB buffer\n");
+   if (!buffer)
goto exit;
-   }
 
while (gspca_dev->present && gspca_dev->streaming) {
 #ifdef CONFIG_PM
-- 
2.14.1

[PATCH 0/2] [media] ViCam: Adjustments for vicam_dostream()

[SF Markus Elfring]

From: Markus Elfring 
Date: Mon, 18 Sep 2017 22:05:22 +0200

Two update suggestions were taken into account
from static source code analysis.

Markus Elfring (2):
  Delete an error message for a failed memory allocation
  Return directly after a failed kmalloc()

 drivers/media/usb/gspca/vicam.c | 8 +++-
 1 file changed, 3 insertions(+), 5 deletions(-)

-- 
2.14.1

Re: [PATCHv6 3/5] dt-bindings: document the CEC GPIO bindings

[Rob Herring]

On Sun, Sep 17, 2017 at 5:15 AM, Hans Verkuil  wrote:
> From: Hans Verkuil 
>
> Document the bindings for the cec-gpio module for hardware where the
> CEC line and optionally the HPD line are connected to GPIO lines.
>
> Signed-off-by: Hans Verkuil 
> Reviewed-by: Linus Walleij 
> ---
>  .../devicetree/bindings/media/cec-gpio.txt | 32 
> ++
>  1 file changed, 32 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/cec-gpio.txt

Reviewed-by: Rob Herring 

[PATCH] [media] sq905: Delete an error message for a failed memory allocation in sq905_dostream()

[SF Markus Elfring]

From: Markus Elfring 
Date: Mon, 18 Sep 2017 21:30:58 +0200

Omit an extra message for a memory allocation failure in this function.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring 
---
 drivers/media/usb/gspca/sq905.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/media/usb/gspca/sq905.c b/drivers/media/usb/gspca/sq905.c
index f1da34a10ce8..ca8cdd753a2b 100644
--- a/drivers/media/usb/gspca/sq905.c
+++ b/drivers/media/usb/gspca/sq905.c
@@ -218,10 +218,8 @@ static void sq905_dostream(struct work_struct *work)
u8 *buffer;
 
buffer = kmalloc(SQ905_MAX_TRANSFER, GFP_KERNEL | GFP_DMA);
-   if (!buffer) {
-   pr_err("Couldn't allocate USB buffer\n");
+   if (!buffer)
goto quit_stream;
-   }
 
frame_sz = gspca_dev->cam.cam_mode[gspca_dev->curr_mode].sizeimage
+ FRAME_HEADER_LEN;
-- 
2.14.1

Re: [PATCH v5 1/2] media:imx274 device tree binding file

[Rob Herring]

On Fri, Sep 15, 2017 at 03:49:51PM +0800, Leon Luo wrote:
> The binding file for imx274 CMOS sensor V4l2 driver
> 
> Signed-off-by: Leon Luo 
> ---
> v5:
>  - add 'port' and 'endpoint' information
> v4:
>  - no changes
> v3:
>  - remove redundant properties and references
>  - document 'reg' property
> v2:
>  - no changes
> ---
>  .../devicetree/bindings/media/i2c/imx274.txt   | 33 
> ++
>  1 file changed, 33 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/i2c/imx274.txt

Acked-by: Rob Herring 

[PATCH 1/1] staging: media: MAINTAINERS: Add entry for atomisp driver

[Sakari Ailus]

Add the maintainers entry to the atomisp staging media driver.

Signed-off-by: Sakari Ailus 
---
 MAINTAINERS | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index eb930eb..3e25df3 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -12469,6 +12469,13 @@ L: sta...@vger.kernel.org
 S: Supported
 F: Documentation/process/stable-kernel-rules.rst
 
+STAGING - ATOMISP DRIVER
+M: Alan Cox 
+M: Sakari Ailus 
+L: linux-media@vger.kernel.org
+S: Maintained
+F: drivers/staging/media/atomisp/
+
 STAGING - COMEDI
 M: Ian Abbott 
 M: H Hartley Sweeten 
-- 
2.7.4

Re: [PATCH v2 3/4] [media] bcm2835-unicam: Driver for CCP2/CSI2 camera interface

[Eric Anholt]

Dave Stevenson  writes:
> diff --git a/drivers/media/platform/bcm2835/bcm2835-unicam.c 
> b/drivers/media/platform/bcm2835/bcm2835-unicam.c
> new file mode 100644
> index 000..5b1adc3
> --- /dev/null
> +++ b/drivers/media/platform/bcm2835/bcm2835-unicam.c
> @@ -0,0 +1,2192 @@
> +/*
> + * BCM2835 Unicam capture Driver
> + *
> + * Copyright (C) 2017 - Raspberry Pi (Trading) Ltd.
> + *
> + * Dave Stevenson 
> + *
> + * Based on TI am437x driver by Benoit Parrot and Lad, Prabhakar and
> + * TI CAL camera interface driver by Benoit Parrot.
> + *

Possible future improvement: this description of the driver is really
nice and could be turned into kernel-doc.

> + * There are two camera drivers in the kernel for BCM283x - this one
> + * and bcm2835-camera (currently in staging).
> + *
> + * This driver is purely the kernel control the Unicam peripheral - there

Maybe "This driver directly controls..."?

> + * is no involvement with the VideoCore firmware. Unicam receives CSI-2
> + * or CCP2 data and writes it into SDRAM. The only processing options are
> + * to repack Bayer data into an alternate format, and applying windowing
> + * (currently not implemented).
> + * It should be possible to connect it to any sensor with a
> + * suitable output interface and V4L2 subdevice driver.
> + *
> + * bcm2835-camera uses with the VideoCore firmware to control the sensor,

"uses the"

> + * Unicam, ISP, and all tuner control loops. Fully processed frames are
> + * delivered to the driver by the firmware. It only has sensor drivers
> + * for Omnivision OV5647, and Sony IMX219 sensors.
> + *
> + * The two drivers are mutually exclusive for the same Unicam instance.
> + * The VideoCore firmware checks the device tree configuration during boot.
> + * If it finds device tree nodes called csi0 or csi1 it will block the
> + * firmware from accessing the peripheral, and bcm2835-camera will
> + * not be able to stream data.

Thanks for describing this here!

> +/*
> + * The peripheral can unpack and repack between several of
> + * the Bayer raw formats, so any Bayer format can be advertised
> + * as the same Bayer order in each of the supported bit depths.
> + * Use lower case to avoid clashing with V4L2_PIX_FMT_SGBRG8
> + * formats.
> + */
> +#define PIX_FMT_ALL_BGGR  v4l2_fourcc('b', 'g', 'g', 'r')
> +#define PIX_FMT_ALL_RGGB  v4l2_fourcc('r', 'g', 'g', 'b')
> +#define PIX_FMT_ALL_GBRG  v4l2_fourcc('g', 'b', 'r', 'g')
> +#define PIX_FMT_ALL_GRBG  v4l2_fourcc('g', 'r', 'b', 'g')

Should thes fourccs be defined in a common v4l2 header, to reserve it
from clashing with others later?

This is really the only question I have about this driver before seeing
it merged.  As far as me wearing my platform maintainer hat, I'm happy
with the driver, and my other little notes are optional.

> +static int unicam_probe(struct platform_device *pdev)
> +{
> + struct unicam_cfg *unicam_cfg;
> + struct unicam_device *unicam;
> + struct v4l2_ctrl_handler *hdl;
> + struct resource *res;
> + int ret;
> +
> + unicam = devm_kzalloc(>dev, sizeof(*unicam), GFP_KERNEL);
> + if (!unicam)
> + return -ENOMEM;
> +
> + unicam->pdev = pdev;
> + unicam_cfg = >cfg;
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + unicam_cfg->base = devm_ioremap_resource(>dev, res);
> + if (IS_ERR(unicam_cfg->base)) {
> + unicam_err(unicam, "Failed to get main io block\n");
> + return PTR_ERR(unicam_cfg->base);
> + }
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> + unicam_cfg->clk_gate_base = devm_ioremap_resource(>dev, res);
> + if (IS_ERR(unicam_cfg->clk_gate_base)) {
> + unicam_err(unicam, "Failed to get 2nd io block\n");
> + return PTR_ERR(unicam_cfg->clk_gate_base);
> + }
> +
> + unicam->clock = devm_clk_get(>dev, "lp_clock");
> + if (IS_ERR(unicam->clock)) {
> + unicam_err(unicam, "Failed to get clock\n");
> + return PTR_ERR(unicam->clock);
> + }
> +
> + ret = platform_get_irq(pdev, 0);
> + if (ret <= 0) {
> + dev_err(>dev, "No IRQ resource\n");
> + return -ENODEV;
> + }
> + unicam_cfg->irq = ret;
> +
> + ret = devm_request_irq(>dev, unicam_cfg->irq, unicam_isr, 0,
> +"unicam_capture0", unicam);

Looks like there's no need to keep "irq" in the device private struct.

> + if (ret) {
> + dev_err(>dev, "Unable to request interrupt\n");
> + return -EINVAL;
> + }
> +
> + ret = v4l2_device_register(>dev, >v4l2_dev);
> + if (ret) {
> + unicam_err(unicam,
> +"Unable to register v4l2 device.\n");
> + return ret;
> + }
> +
> + /* Reserve space for the controls */
> + hdl = >ctrl_handler;
> + ret = v4l2_ctrl_handler_init(hdl, 16);
> + if (ret < 0)
> 

[PATCH] [media] jl2005bcd: Delete an error message for a failed memory allocation in jl2005c_dostream()

[SF Markus Elfring]

From: Markus Elfring 
Date: Mon, 18 Sep 2017 19:24:24 +0200

Omit an extra message for a memory allocation failure in this function.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring 
---
 drivers/media/usb/gspca/jl2005bcd.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/media/usb/gspca/jl2005bcd.c 
b/drivers/media/usb/gspca/jl2005bcd.c
index 17c7a953564c..23400ead1919 100644
--- a/drivers/media/usb/gspca/jl2005bcd.c
+++ b/drivers/media/usb/gspca/jl2005bcd.c
@@ -320,10 +320,8 @@ static void jl2005c_dostream(struct work_struct *work)
u8 *buffer;
 
buffer = kmalloc(JL2005C_MAX_TRANSFER, GFP_KERNEL | GFP_DMA);
-   if (!buffer) {
-   pr_err("Couldn't allocate USB buffer\n");
+   if (!buffer)
goto quit_stream;
-   }
 
while (gspca_dev->present && gspca_dev->streaming) {
 #ifdef CONFIG_PM
-- 
2.14.1

[PATCH] [media] gspca: Use common error handling code in gspca_init_transfer()

[SF Markus Elfring]

From: Markus Elfring 
Date: Mon, 18 Sep 2017 18:40:05 +0200

Add a jump target so that a bit of exception handling can be better reused
at the end of this function.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring 
---
 drivers/media/usb/gspca/gspca.c | 17 +
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/drivers/media/usb/gspca/gspca.c b/drivers/media/usb/gspca/gspca.c
index 0f141762abf1..22cdefe38e07 100644
--- a/drivers/media/usb/gspca/gspca.c
+++ b/drivers/media/usb/gspca/gspca.c
@@ -904,10 +904,8 @@ static int gspca_init_transfer(struct gspca_dev *gspca_dev)
ret = create_urbs(gspca_dev,
alt_xfer(>altsetting[alt], xfer,
 gspca_dev->xfer_ep));
-   if (ret < 0) {
-   destroy_urbs(gspca_dev);
-   goto out;
-   }
+   if (ret < 0)
+   goto destroy_urbs;
}
 
/* clear the bulk endpoint */
@@ -917,10 +915,9 @@ static int gspca_init_transfer(struct gspca_dev *gspca_dev)
 
/* start the cam */
ret = gspca_dev->sd_desc->start(gspca_dev);
-   if (ret < 0) {
-   destroy_urbs(gspca_dev);
-   goto out;
-   }
+   if (ret < 0)
+   goto destroy_urbs;
+
gspca_dev->streaming = 1;
v4l2_ctrl_handler_setup(gspca_dev->vdev.ctrl_handler);
 
@@ -970,6 +967,10 @@ static int gspca_init_transfer(struct gspca_dev *gspca_dev)
 out:
gspca_input_create_urb(gspca_dev);
return ret;
+
+destroy_urbs:
+   destroy_urbs(gspca_dev);
+   goto out;
 }
 
 static void gspca_set_default_mode(struct gspca_dev *gspca_dev)
-- 
2.14.1

[PATCH] [media] gspca: Delete two error messages for a failed memory allocation in gspca_dev_probe2()

[SF Markus Elfring]

From: Markus Elfring 
Date: Mon, 18 Sep 2017 17:47:58 +0200

Omit extra messages for a memory allocation failure in this function.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring 
---
 drivers/media/usb/gspca/gspca.c | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/media/usb/gspca/gspca.c b/drivers/media/usb/gspca/gspca.c
index 0f141762abf1..8be1c81b7cab 100644
--- a/drivers/media/usb/gspca/gspca.c
+++ b/drivers/media/usb/gspca/gspca.c
@@ -2037,10 +2037,8 @@ int gspca_dev_probe2(struct usb_interface *intf,
-   if (!gspca_dev) {
-   pr_err("couldn't kzalloc gspca struct\n");
+   if (!gspca_dev)
return -ENOMEM;
-   }
+
gspca_dev->usb_buf = kmalloc(USB_BUF_SZ, GFP_KERNEL);
if (!gspca_dev->usb_buf) {
-   pr_err("out of memory\n");
ret = -ENOMEM;
goto out;
}
-- 
2.14.1

Re: [PATCHv4 3/3] drm/i915: add DisplayPort CEC-Tunneling-over-AUX support

[Ville Syrjälä]

On Mon, Sep 18, 2017 at 05:26:43PM +0200, Hans Verkuil wrote:
> On 09/18/2017 04:36 PM, Ville Syrjälä wrote:
> > On Mon, Sep 18, 2017 at 04:07:41PM +0200, Hans Verkuil wrote:
> >> Hi Ville,
> >>
> >> On 09/18/2017 03:05 PM, Ville Syrjälä wrote:
> >>> On Sat, Sep 16, 2017 at 04:17:26PM +0200, Hans Verkuil wrote:
>  From: Hans Verkuil 
> 
>  Implement support for this DisplayPort feature.
> 
>  The cec device is created whenever it detects an adapter that
>  has this feature. It is only removed when a new adapter is connected
>  that does not support this. If a new adapter is connected that has
>  different properties than the previous one, then the old cec device is
>  unregistered and a new one is registered to replace the old one.
> 
>  Signed-off-by: Hans Verkuil 
>  Tested-by: Carlos Santa 
>  ---
>   drivers/gpu/drm/i915/intel_dp.c | 18 ++
>   1 file changed, 14 insertions(+), 4 deletions(-)
> 
>  diff --git a/drivers/gpu/drm/i915/intel_dp.c 
>  b/drivers/gpu/drm/i915/intel_dp.c
>  index 64fa774c855b..fdb853d2c458 100644
>  --- a/drivers/gpu/drm/i915/intel_dp.c
>  +++ b/drivers/gpu/drm/i915/intel_dp.c
>  @@ -32,6 +32,7 @@
>   #include 
>   #include 
>   #include 
>  +#include 
>   #include 
>   #include 
>   #include 
>  @@ -1449,6 +1450,7 @@ static void intel_aux_reg_init(struct intel_dp 
>  *intel_dp)
>   static void
>   intel_dp_aux_fini(struct intel_dp *intel_dp)
>   {
>  +cec_unregister_adapter(intel_dp->aux.cec_adap);
>   kfree(intel_dp->aux.name);
>   }
>   
>  @@ -4587,6 +4589,7 @@ intel_dp_set_edid(struct intel_dp *intel_dp)
>   intel_connector->detect_edid = edid;
>   
>   intel_dp->has_audio = drm_detect_monitor_audio(edid);
>  +cec_s_phys_addr_from_edid(intel_dp->aux.cec_adap, edid);
>   }
>   
>   static void
>  @@ -4596,6 +4599,7 @@ intel_dp_unset_edid(struct intel_dp *intel_dp)
>   
>   kfree(intel_connector->detect_edid);
>   intel_connector->detect_edid = NULL;
>  +cec_phys_addr_invalidate(intel_dp->aux.cec_adap);
>   
>   intel_dp->has_audio = false;
>   }
>  @@ -4616,13 +4620,17 @@ intel_dp_long_pulse(struct intel_connector 
>  *intel_connector)
>   intel_display_power_get(to_i915(dev), 
>  intel_dp->aux_power_domain);
>   
>   /* Can't disconnect eDP, but you can close the lid... */
>  -if (is_edp(intel_dp))
>  +if (is_edp(intel_dp)) {
>   status = edp_detect(intel_dp);
>  -else if (intel_digital_port_connected(to_i915(dev),
>  -  dp_to_dig_port(intel_dp)))
>  +} else if (intel_digital_port_connected(to_i915(dev),
>  +
>  dp_to_dig_port(intel_dp))) {
>   status = intel_dp_detect_dpcd(intel_dp);
>  -else
>  +if (status == connector_status_connected)
>  +drm_dp_cec_configure_adapter(_dp->aux,
>  + intel_dp->aux.name, dev->dev);
> >>>
> >>> This is cluttering up the code a bit. Maybe do this call somewhere
> >>> around the intel_dp_configure_mst() call instead since that seems to be
> >>> the place where we start to do changes to externally visible state.
> >>>
> >>> Actually, do we want to register cec adapters for MST devices?
> >>>
> >>> And shouldn't we call this regardless of the connector state so that
> >>> the cec adapter gets unregistered when the device is disconnected?
> >>
> >> This hasn't (AFAIK) anything to do with MST. This is in a branch device 
> >> (i.e.
> >> a DP to HDMI adapter).
> > 
> > You are now potentiall registering the CEC adapter to the immediately
> > upstream MST device (ie. the one that we talk to over the normal AUX stuff),
> > but kms will consider that paticular connector as disconnected, and
> > instead only sinks downstream of that device may have connected connectors
> > associated with them. Presumably the CEC towards that device goes
> > nowhere, and instead we'd have to talk to the remote branch devices
> > somewhere downstream.
> > 
> > Thus my question whether we want to potentially register the CEC adapter
> > to the immediately upstream MST device or not. I would imagine not, and
> > thus the call should perhaps be moved past the 'is_mst? -> disconnected'
> > checks.
> 
> Ah, now I see what you mean. Sorry, I misunderstood you earlier. I can
> certainly move it down. But an MST device would never set the CEC capability
> in the DPCD, would it?

It might. You don't even have to drive it as an MST device if you don't
want to, or you source device 

Re: [PATCH v1] media: rc: Add driver for tango IR decoder

[Måns Rullgård]

Marc Gonzalez  writes:

> The tango IR decoder supports NEC, RC-5, RC-6 protocols.
> NB: Only the NEC decoder was tested.
>
> Signed-off-by: Marc Gonzalez 
> ---
>  drivers/media/rc/Kconfig|   5 +
>  drivers/media/rc/Makefile   |   1 +
>  drivers/media/rc/tango-ir.c | 263 
> 
>  3 files changed, 269 insertions(+)
>  create mode 100644 drivers/media/rc/tango-ir.c
>
> diff --git a/drivers/media/rc/Kconfig b/drivers/media/rc/Kconfig
> index d9ce8ff55d0c..f84923289964 100644
> --- a/drivers/media/rc/Kconfig
> +++ b/drivers/media/rc/Kconfig
> @@ -469,6 +469,11 @@ config IR_SIR
>  To compile this driver as a module, choose M here: the module will
>  be called sir-ir.
>  +config IR_TANGO
> + tristate "Sigma Designs SMP86xx IR decoder"
> + depends on RC_CORE
> + depends on ARCH_TANGO || COMPILE_TEST
> +
>  config IR_ZX
>   tristate "ZTE ZX IR remote control"
>   depends on RC_CORE

This hunk looks damaged.

What have you changed compared to my original code?

I tested all three protocols with a few random remotes I had lying
around back when I wrote the driver, but that's quite a while ago.

You should also write a devicetree binding.

Finally, when sending patches essentially written by someone else,
please make sure to set a From: line for correct attribution.  It's not
nice to take other people's code and apparently pass it off as your own
even you've made a few small changes.

-- 
Måns Rullgård

Re: [PATCHv4 3/3] drm/i915: add DisplayPort CEC-Tunneling-over-AUX support

[Hans Verkuil]

On 09/18/2017 04:36 PM, Ville Syrjälä wrote:
> On Mon, Sep 18, 2017 at 04:07:41PM +0200, Hans Verkuil wrote:
>> Hi Ville,
>>
>> On 09/18/2017 03:05 PM, Ville Syrjälä wrote:
>>> On Sat, Sep 16, 2017 at 04:17:26PM +0200, Hans Verkuil wrote:
 From: Hans Verkuil 

 Implement support for this DisplayPort feature.

 The cec device is created whenever it detects an adapter that
 has this feature. It is only removed when a new adapter is connected
 that does not support this. If a new adapter is connected that has
 different properties than the previous one, then the old cec device is
 unregistered and a new one is registered to replace the old one.

 Signed-off-by: Hans Verkuil 
 Tested-by: Carlos Santa 
 ---
  drivers/gpu/drm/i915/intel_dp.c | 18 ++
  1 file changed, 14 insertions(+), 4 deletions(-)

 diff --git a/drivers/gpu/drm/i915/intel_dp.c 
 b/drivers/gpu/drm/i915/intel_dp.c
 index 64fa774c855b..fdb853d2c458 100644
 --- a/drivers/gpu/drm/i915/intel_dp.c
 +++ b/drivers/gpu/drm/i915/intel_dp.c
 @@ -32,6 +32,7 @@
  #include 
  #include 
  #include 
 +#include 
  #include 
  #include 
  #include 
 @@ -1449,6 +1450,7 @@ static void intel_aux_reg_init(struct intel_dp 
 *intel_dp)
  static void
  intel_dp_aux_fini(struct intel_dp *intel_dp)
  {
 +  cec_unregister_adapter(intel_dp->aux.cec_adap);
kfree(intel_dp->aux.name);
  }
  
 @@ -4587,6 +4589,7 @@ intel_dp_set_edid(struct intel_dp *intel_dp)
intel_connector->detect_edid = edid;
  
intel_dp->has_audio = drm_detect_monitor_audio(edid);
 +  cec_s_phys_addr_from_edid(intel_dp->aux.cec_adap, edid);
  }
  
  static void
 @@ -4596,6 +4599,7 @@ intel_dp_unset_edid(struct intel_dp *intel_dp)
  
kfree(intel_connector->detect_edid);
intel_connector->detect_edid = NULL;
 +  cec_phys_addr_invalidate(intel_dp->aux.cec_adap);
  
intel_dp->has_audio = false;
  }
 @@ -4616,13 +4620,17 @@ intel_dp_long_pulse(struct intel_connector 
 *intel_connector)
intel_display_power_get(to_i915(dev), intel_dp->aux_power_domain);
  
/* Can't disconnect eDP, but you can close the lid... */
 -  if (is_edp(intel_dp))
 +  if (is_edp(intel_dp)) {
status = edp_detect(intel_dp);
 -  else if (intel_digital_port_connected(to_i915(dev),
 -dp_to_dig_port(intel_dp)))
 +  } else if (intel_digital_port_connected(to_i915(dev),
 +  dp_to_dig_port(intel_dp))) {
status = intel_dp_detect_dpcd(intel_dp);
 -  else
 +  if (status == connector_status_connected)
 +  drm_dp_cec_configure_adapter(_dp->aux,
 +   intel_dp->aux.name, dev->dev);
>>>
>>> This is cluttering up the code a bit. Maybe do this call somewhere
>>> around the intel_dp_configure_mst() call instead since that seems to be
>>> the place where we start to do changes to externally visible state.
>>>
>>> Actually, do we want to register cec adapters for MST devices?
>>>
>>> And shouldn't we call this regardless of the connector state so that
>>> the cec adapter gets unregistered when the device is disconnected?
>>
>> This hasn't (AFAIK) anything to do with MST. This is in a branch device (i.e.
>> a DP to HDMI adapter).
> 
> You are now potentiall registering the CEC adapter to the immediately
> upstream MST device (ie. the one that we talk to over the normal AUX stuff),
> but kms will consider that paticular connector as disconnected, and
> instead only sinks downstream of that device may have connected connectors
> associated with them. Presumably the CEC towards that device goes
> nowhere, and instead we'd have to talk to the remote branch devices
> somewhere downstream.
> 
> Thus my question whether we want to potentially register the CEC adapter
> to the immediately upstream MST device or not. I would imagine not, and
> thus the call should perhaps be moved past the 'is_mst? -> disconnected'
> checks.

Ah, now I see what you mean. Sorry, I misunderstood you earlier. I can
certainly move it down. But an MST device would never set the CEC capability
in the DPCD, would it? That makes no sense. So it would never register a
CEC device in practice. Although I do need to test what happens when you
first connect a USB-C to HDMI adapter that supports CEC, then disconnect it,
then connect an MST hub. The CEC device should be unregistered in that case,
but I'm not sure if that actually happens. I'll have to test that tomorrow.

> 
>>
>> The CEC adapter should ideally be associated with the branch device (since 
>> that
>> is what implements the CEC tunneling): i.e. when you connect the adapter, 
>> then
>> 

Re: [RESEND PATCH v2 4/6] dt: bindings: as3645a: Improve label documentation, DT example

[Sakari Ailus]

Hi Pavel,

On Mon, Sep 18, 2017 at 12:56:55PM +0200, Pavel Machek wrote:
> Hi!
> 
> > Specify the exact label used if the label property is omitted in DT, as
> > well as use label in the example that conforms to LED device naming.
> > 
> > @@ -69,11 +73,11 @@ Example
> > flash-max-microamp = <32>;
> > led-max-microamp = <6>;
> > ams,input-max-microamp = <175>;
> > -   label = "as3645a:flash";
> > +   label = "as3645a:white:flash";
> > };
> > indicator@1 {
> > reg = <0x1>;
> > led-max-microamp = <1>;
> > -   label = "as3645a:indicator";
> > +   label = "as3645a:red:indicator";
> > };
> > };
> 
> Ok, but userspace still has no chance to determine if this is flash
> from main camera or flash for front camera; todays smartphones have
> flashes on both cameras.
> 
> So.. Can I suggset as3645a:white:main_camera_flash or main_flash or
> ?

If there's just a single one in the device, could you use that?

Even if we name this so for N9 (and N900), the application still would only
work with the two devices.

My suggestion would be to look for a flash LED, and perhaps the maximum
current as well. That should generally work better than assumptions on the
label.

For association with a particular camera --- in the long run I'd propose to
use Media controller / property API for that in the long run. We don't have
that yet though.

Cc Jacek, too.

-- 
Sakari Ailus
e-mail: sakari.ai...@iki.fi

Re: [PATCH v4] drm/bridge/sii8620: add remote control support

[Hans Verkuil]

On 09/18/2017 04:15 PM, Maciej Purski wrote:
> Hi Hans,
> some time ago in reply to your email I described what messages does
> the MHL driver receive and at what time intervals.
> Regarding that information, do you think that a similar solution as
> in [1] is required? Would it be OK, if I just set REP_DELAY and REP_PERIOD
> to values, which I presented in my last email?

Based on the timings you measured I would say that there is a 99% chance that 
MHL
uses exactly the same "Press and Hold" mechanism as CEC. Since you already 
specify
RC_PROTO_BIT_CEC in the driver, it will set REP_DELAY and REP_PERIOD to the 
right
values automatically.

You will have to implement the same code as in cec-adap.c for the key press 
handling,
though. It's a bit tricky to get it right.

Since you will have to do the same thing as I do in CEC, I wonder if it would 
make
more sense to move this code to the RC core. Basically it ensures that repeat 
mode
doesn't turn on until you get two identical key presses 550ms or less apart. 
This
is independent of REP_DELAY which can be changed by userspace.

Sean, what do you think?

Regards,

Hans

> 
> 
> Best Regards,
> 
>   Maciej
> 
> [1] 
> https://git.linuxtv.org/media_tree.git/commit/drivers/media/cec/cec-adap.c?id=a9a249a2c997506a64eaee22f1458fda893f62a8
> 
> On 08/27/2017 02:40 PM, Hans Verkuil wrote:
> 
>> Hi Maciej,
>>
>> On 24/08/17 10:58, Maciej Purski wrote:
>>> MHL specification defines Remote Control Protocol(RCP) to
>>> send input events between MHL devices.
>>> The driver now recognizes RCP messages and reacts to them
>>> by reporting key events to input subsystem, allowing
>>> a user to control a device using TV remote control.
>> Before I Ack this I would like to know how this behaves w.r.t. autorepeat.
>>
>> If you keep pressing a remote key, what RCP messages do you receive and
>> at what time intervals? If that's similar to what CEC does, then it is
>> very likely that the same rules apply and I will have to review this patch
>> again with that in mind.
>>
>> See the commit log for the patching fixing the CEC 'Press and Hold' behavior:
>>
>> https://git.linuxtv.org/media_tree.git/commit/drivers/media/cec/cec-adap.c?id=a9a249a2c997506a64eaee22f1458fda893f62a8
>>
>> If you have access to the HDMI 2.0 specification, then that spec describes 
>> the CEC
>> 'Press and Hold' behavior in detail.
>>
>> Regards,
>>
>>  Hans
>>
>>> Signed-off-by: Maciej Purski 
>>> ---
>>>
>>> Changes in v2:
>>> - use RC subsystem (including CEC keymap)
>>> - RC device initialized in attach drm_bridge callback and
>>>removed in detach callback. This is necessary, because RC_CORE,
>>>which is needed during rc_dev init, is loaded after sii8620.
>>>DRM bridge is binded later which solves the problem.
>>> - add RC_CORE dependency
>>>
>>> Changes in v3:
>>> - fix error handling in init_rcp and in attach callback
>>>
>>> Changes in v4:
>>> - usage of rc-core API compatible with upcoming changes
>>> - fix error handling in init_rcp
>>> - fix commit message
>>> ---
>>>   drivers/gpu/drm/bridge/Kconfig   |  2 +-
>>>   drivers/gpu/drm/bridge/sil-sii8620.c | 96 
>>> ++--
>>>   include/drm/bridge/mhl.h |  4 ++
>>>   3 files changed, 96 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
>>> index adf9ae0..6ef901c 100644
>>> --- a/drivers/gpu/drm/bridge/Kconfig
>>> +++ b/drivers/gpu/drm/bridge/Kconfig
>>> @@ -71,7 +71,7 @@ config DRM_PARADE_PS8622
>>>   
>>>   config DRM_SIL_SII8620
>>> tristate "Silicon Image SII8620 HDMI/MHL bridge"
>>> -   depends on OF
>>> +   depends on OF && RC_CORE
>>> select DRM_KMS_HELPER
>>> help
>>>   Silicon Image SII8620 HDMI/MHL bridge chip driver.
>>> diff --git a/drivers/gpu/drm/bridge/sil-sii8620.c 
>>> b/drivers/gpu/drm/bridge/sil-sii8620.c
>>> index 2d51a22..ecb26c4 100644
>>> --- a/drivers/gpu/drm/bridge/sil-sii8620.c
>>> +++ b/drivers/gpu/drm/bridge/sil-sii8620.c
>>> @@ -28,6 +28,8 @@
>>>   #include 
>>>   #include 
>>>   
>>> +#include 
>>> +
>>>   #include "sil-sii8620.h"
>>>   
>>>   #define SII8620_BURST_BUF_LEN 288
>>> @@ -58,6 +60,7 @@ enum sii8620_mt_state {
>>>   struct sii8620 {
>>> struct drm_bridge bridge;
>>> struct device *dev;
>>> +   struct rc_dev *rc_dev;
>>> struct clk *clk_xtal;
>>> struct gpio_desc *gpio_reset;
>>> struct gpio_desc *gpio_int;
>>> @@ -431,6 +434,16 @@ static void sii8620_mt_rap(struct sii8620 *ctx, u8 
>>> code)
>>> sii8620_mt_msc_msg(ctx, MHL_MSC_MSG_RAP, code);
>>>   }
>>>   
>>> +static void sii8620_mt_rcpk(struct sii8620 *ctx, u8 code)
>>> +{
>>> +   sii8620_mt_msc_msg(ctx, MHL_MSC_MSG_RCPK, code);
>>> +}
>>> +
>>> +static void sii8620_mt_rcpe(struct sii8620 *ctx, u8 code)
>>> +{
>>> +   sii8620_mt_msc_msg(ctx, MHL_MSC_MSG_RCPE, code);
>>> +}
>>> +
>>>   static void sii8620_mt_read_devcap_send(struct sii8620 *ctx,
>>>

Re: [PATCHv4 3/3] drm/i915: add DisplayPort CEC-Tunneling-over-AUX support

[Ville Syrjälä]

On Mon, Sep 18, 2017 at 04:07:41PM +0200, Hans Verkuil wrote:
> Hi Ville,
> 
> On 09/18/2017 03:05 PM, Ville Syrjälä wrote:
> > On Sat, Sep 16, 2017 at 04:17:26PM +0200, Hans Verkuil wrote:
> >> From: Hans Verkuil 
> >>
> >> Implement support for this DisplayPort feature.
> >>
> >> The cec device is created whenever it detects an adapter that
> >> has this feature. It is only removed when a new adapter is connected
> >> that does not support this. If a new adapter is connected that has
> >> different properties than the previous one, then the old cec device is
> >> unregistered and a new one is registered to replace the old one.
> >>
> >> Signed-off-by: Hans Verkuil 
> >> Tested-by: Carlos Santa 
> >> ---
> >>  drivers/gpu/drm/i915/intel_dp.c | 18 ++
> >>  1 file changed, 14 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/intel_dp.c 
> >> b/drivers/gpu/drm/i915/intel_dp.c
> >> index 64fa774c855b..fdb853d2c458 100644
> >> --- a/drivers/gpu/drm/i915/intel_dp.c
> >> +++ b/drivers/gpu/drm/i915/intel_dp.c
> >> @@ -32,6 +32,7 @@
> >>  #include 
> >>  #include 
> >>  #include 
> >> +#include 
> >>  #include 
> >>  #include 
> >>  #include 
> >> @@ -1449,6 +1450,7 @@ static void intel_aux_reg_init(struct intel_dp 
> >> *intel_dp)
> >>  static void
> >>  intel_dp_aux_fini(struct intel_dp *intel_dp)
> >>  {
> >> +  cec_unregister_adapter(intel_dp->aux.cec_adap);
> >>kfree(intel_dp->aux.name);
> >>  }
> >>  
> >> @@ -4587,6 +4589,7 @@ intel_dp_set_edid(struct intel_dp *intel_dp)
> >>intel_connector->detect_edid = edid;
> >>  
> >>intel_dp->has_audio = drm_detect_monitor_audio(edid);
> >> +  cec_s_phys_addr_from_edid(intel_dp->aux.cec_adap, edid);
> >>  }
> >>  
> >>  static void
> >> @@ -4596,6 +4599,7 @@ intel_dp_unset_edid(struct intel_dp *intel_dp)
> >>  
> >>kfree(intel_connector->detect_edid);
> >>intel_connector->detect_edid = NULL;
> >> +  cec_phys_addr_invalidate(intel_dp->aux.cec_adap);
> >>  
> >>intel_dp->has_audio = false;
> >>  }
> >> @@ -4616,13 +4620,17 @@ intel_dp_long_pulse(struct intel_connector 
> >> *intel_connector)
> >>intel_display_power_get(to_i915(dev), intel_dp->aux_power_domain);
> >>  
> >>/* Can't disconnect eDP, but you can close the lid... */
> >> -  if (is_edp(intel_dp))
> >> +  if (is_edp(intel_dp)) {
> >>status = edp_detect(intel_dp);
> >> -  else if (intel_digital_port_connected(to_i915(dev),
> >> -dp_to_dig_port(intel_dp)))
> >> +  } else if (intel_digital_port_connected(to_i915(dev),
> >> +  dp_to_dig_port(intel_dp))) {
> >>status = intel_dp_detect_dpcd(intel_dp);
> >> -  else
> >> +  if (status == connector_status_connected)
> >> +  drm_dp_cec_configure_adapter(_dp->aux,
> >> +   intel_dp->aux.name, dev->dev);
> > 
> > This is cluttering up the code a bit. Maybe do this call somewhere
> > around the intel_dp_configure_mst() call instead since that seems to be
> > the place where we start to do changes to externally visible state.
> > 
> > Actually, do we want to register cec adapters for MST devices?
> > 
> > And shouldn't we call this regardless of the connector state so that
> > the cec adapter gets unregistered when the device is disconnected?
> 
> This hasn't (AFAIK) anything to do with MST. This is in a branch device (i.e.
> a DP to HDMI adapter).

You are now potentiall registering the CEC adapter to the immediately
upstream MST device (ie. the one that we talk to over the normal AUX stuff),
but kms will consider that paticular connector as disconnected, and
instead only sinks downstream of that device may have connected connectors
associated with them. Presumably the CEC towards that device goes
nowhere, and instead we'd have to talk to the remote branch devices
somewhere downstream.

Thus my question whether we want to potentially register the CEC adapter
to the immediately upstream MST device or not. I would imagine not, and
thus the call should perhaps be moved past the 'is_mst? -> disconnected'
checks.

> 
> The CEC adapter should ideally be associated with the branch device (since 
> that
> is what implements the CEC tunneling): i.e. when you connect the adapter, then
> the CEC device is created, when you disconnect the adapter, then the CEC 
> device
> should be unregistered. This is not the same as connecting/disconnecting the
> HDMI cable to/from the adapter: that just sets or invalidates the CEC physical
> address (which is read from the EDID).
> 
> However, I have not seen any code that tells me when the adapter is plugged in
> or is unplugged. So all I have to go on is when the HDMI cable is connected.
> 
> Note that the 'late_register' you mentioned in your 1/3 review isn't called 
> when
> connecting the adapter. So that too cannot be used as a trigger to detect 

[PATCH] media: rc: Delete duplicate debug message

[Marc Gonzalez]

ir_setkeytable() and ir_create_table() print the same debug message.
Delete the one in ir_setkeytable()

Signed-off-by: Marc Gonzalez 
---
 drivers/media/rc/rc-main.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/media/rc/rc-main.c b/drivers/media/rc/rc-main.c
index 981cccd6b988..a881d7c161e1 100644
--- a/drivers/media/rc/rc-main.c
+++ b/drivers/media/rc/rc-main.c
@@ -439,9 +439,6 @@ static int ir_setkeytable(struct rc_dev *dev,
if (rc)
return rc;
 
-	IR_dprintk(1, "Allocated space for %u keycode entries (%u bytes)\n",

-  rc_map->size, rc_map->alloc);
-
for (i = 0; i < from->size; i++) {
index = ir_establish_scancode(dev, rc_map,
  from->scan[i].scancode, false);
--
2.11.0

[PATCH] [media] cxusb: pass buf as a const u8 * pointer and make buf static const

[Colin King]

From: Colin Ian King 

Don't populate the read-only u8 array buf on the stack at run time but
instead make it static const; makes object code smaller saving over 480
bytes:

Before:
   textdata bss dec hex filename
  33030   65936 192   99158   18356 drivers/media/usb/dvb-usb/cxusb.o

After:
   textdata bss dec hex filename
  32446   66032 192   98670   1816e drivers/media/usb/dvb-usb/cxusb.o

Signed-off-by: Colin Ian King 
---
 drivers/media/usb/dvb-usb/cxusb.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/media/usb/dvb-usb/cxusb.c 
b/drivers/media/usb/dvb-usb/cxusb.c
index 37dea0adc695..b5cc1b990065 100644
--- a/drivers/media/usb/dvb-usb/cxusb.c
+++ b/drivers/media/usb/dvb-usb/cxusb.c
@@ -56,7 +56,7 @@ DVB_DEFINE_MOD_OPT_ADAPTER_NR(adapter_nr);
 #define deb_i2c(args...)dprintk(dvb_usb_cxusb_debug, 0x02, args)
 
 static int cxusb_ctrl_msg(struct dvb_usb_device *d,
- u8 cmd, u8 *wbuf, int wlen, u8 *rbuf, int rlen)
+ u8 cmd, const u8 *wbuf, int wlen, u8 *rbuf, int rlen)
 {
struct cxusb_state *st = d->priv;
int ret;
@@ -290,7 +290,8 @@ static int cxusb_aver_power_ctrl(struct dvb_usb_device *d, 
int onoff)
/* FIXME: We don't know why, but we need to configure the
 * lgdt3303 with the register settings below on resume */
int i;
-   u8 buf, bufs[] = {
+   u8 buf;
+   static const u8 bufs[] = {
0x0e, 0x2, 0x00, 0x7f,
0x0e, 0x2, 0x02, 0xfe,
0x0e, 0x2, 0x02, 0x01,
-- 
2.14.1

[PATCH v1] media: rc: Add driver for tango IR decoder

[Marc Gonzalez]

The tango IR decoder supports NEC, RC-5, RC-6 protocols.
NB: Only the NEC decoder was tested.

Signed-off-by: Marc Gonzalez 
---
 drivers/media/rc/Kconfig|   5 +
 drivers/media/rc/Makefile   |   1 +
 drivers/media/rc/tango-ir.c | 263 
 3 files changed, 269 insertions(+)
 create mode 100644 drivers/media/rc/tango-ir.c

diff --git a/drivers/media/rc/Kconfig b/drivers/media/rc/Kconfig
index d9ce8ff55d0c..f84923289964 100644
--- a/drivers/media/rc/Kconfig
+++ b/drivers/media/rc/Kconfig
@@ -469,6 +469,11 @@ config IR_SIR
   To compile this driver as a module, choose M here: the module will
   be called sir-ir.
 
+config IR_TANGO

+   tristate "Sigma Designs SMP86xx IR decoder"
+   depends on RC_CORE
+   depends on ARCH_TANGO || COMPILE_TEST
+
 config IR_ZX
tristate "ZTE ZX IR remote control"
depends on RC_CORE
diff --git a/drivers/media/rc/Makefile b/drivers/media/rc/Makefile
index 9bc6a3980ed0..643797dc971b 100644
--- a/drivers/media/rc/Makefile
+++ b/drivers/media/rc/Makefile
@@ -44,3 +44,4 @@ obj-$(CONFIG_IR_SERIAL) += serial_ir.o
 obj-$(CONFIG_IR_SIR) += sir_ir.o
 obj-$(CONFIG_IR_MTK) += mtk-cir.o
 obj-$(CONFIG_IR_ZX) += zx-irdec.o
+obj-$(CONFIG_IR_TANGO) += tango-ir.o
diff --git a/drivers/media/rc/tango-ir.c b/drivers/media/rc/tango-ir.c
new file mode 100644
index ..fedb8d35ac10
--- /dev/null
+++ b/drivers/media/rc/tango-ir.c
@@ -0,0 +1,263 @@
+/*
+ * Copyright (C) 2015 Mans Rullgard 
+ *
+ * This program is free software; you can redistribute  it and/or modify it
+ * under  the terms of  the GNU General  Public License as published by the
+ * Free Software Foundation;  either version 2 of the  License, or (at your
+ * option) any later version.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define IR_NEC_CTRL0x00
+#define IR_NEC_DATA0x04
+#define IR_CTRL0x08
+#define IR_RC5_CLK_DIV 0x0c
+#define IR_RC5_DATA0x10
+#define IR_INT 0x14
+
+#define NEC_TIME_BASE  560
+#define RC5_TIME_BASE  1778
+
+#define RC6_CTRL   0x00
+#define RC6_CLKDIV 0x04
+#define RC6_DATA0  0x08
+#define RC6_DATA1  0x0c
+#define RC6_DATA2  0x10
+#define RC6_DATA3  0x14
+#define RC6_DATA4  0x18
+
+#define RC6_CARRIER36000
+#define RC6_TIME_BASE  16
+
+struct tango_ir {
+   void __iomem *rc5_base;
+   void __iomem *rc6_base;
+   struct rc_dev *rc;
+   struct clk *clk;
+};
+
+static void tango_ir_handle_nec(struct tango_ir *ir)
+{
+   u32 v, code;
+   enum rc_proto proto;
+
+   v = readl_relaxed(ir->rc5_base + IR_NEC_DATA);
+   if (!v) {
+   rc_repeat(ir->rc);
+   return;
+   }
+
+   code = ir_nec_bytes_to_scancode(v, v >> 8, v >> 16, v >> 24, );
+   rc_keydown(ir->rc, proto, code, 0);
+}
+
+static void tango_ir_handle_rc5(struct tango_ir *ir)
+{
+   u32 data, field, toggle, addr, cmd, code;
+
+   data = readl_relaxed(ir->rc5_base + IR_RC5_DATA);
+   if (data & BIT(31))
+   return;
+
+   field = data >> 12 & 1;
+   toggle = data >> 11 & 1;
+   addr = data >> 6 & 0x1f;
+   cmd = (data & 0x3f) | (field ^ 1) << 6;
+
+   code = RC_SCANCODE_RC5(addr, cmd);
+   rc_keydown(ir->rc, RC_PROTO_RC5, code, toggle);
+}
+
+static void tango_ir_handle_rc6(struct tango_ir *ir)
+{
+   u32 data0, data1, toggle, mode, addr, cmd, code;
+
+   data0 = readl_relaxed(ir->rc6_base + RC6_DATA0);
+   data1 = readl_relaxed(ir->rc6_base + RC6_DATA1);
+
+   mode = data0 >> 1 & 7;
+   if (mode != 0)
+   return;
+
+   toggle = data0 & 1;
+   addr = data0 >> 16;
+   cmd = data1;
+
+   code = RC_SCANCODE_RC6_0(addr, cmd);
+   rc_keydown(ir->rc, RC_PROTO_RC6_0, code, toggle);
+}
+
+static irqreturn_t tango_ir_irq(int irq, void *dev_id)
+{
+   struct tango_ir *ir = dev_id;
+   unsigned int rc5_stat;
+   unsigned int rc6_stat;
+
+   rc5_stat = readl_relaxed(ir->rc5_base + IR_INT);
+   writel_relaxed(rc5_stat, ir->rc5_base + IR_INT);
+
+   rc6_stat = readl_relaxed(ir->rc6_base + RC6_CTRL);
+   writel_relaxed(rc6_stat, ir->rc6_base + RC6_CTRL);
+
+   if (!(rc5_stat & 3) && !(rc6_stat & BIT(31)))
+   return IRQ_NONE;
+
+   if (rc5_stat & BIT(0))
+   tango_ir_handle_rc5(ir);
+
+   if (rc5_stat & BIT(1))
+   tango_ir_handle_nec(ir);
+
+   if (rc6_stat & BIT(31))
+   tango_ir_handle_rc6(ir);
+
+   return IRQ_HANDLED;
+}
+
+#define DISABLE_NEC(BIT(4) | BIT(8))
+#define ENABLE_RC5 (BIT(0) | BIT(9))
+#define ENABLE_RC6 (BIT(0) | BIT(7))
+
+static int tango_change_protocol(struct rc_dev *dev, u64 *rc_type)
+{
+   struct tango_ir *ir = dev->priv;
+   u32 rc5_ctrl = DISABLE_NEC;
+   u32 rc6_ctrl = 0;
+
+   if (*rc_type & 

Re: [PATCH v4] drm/bridge/sii8620: add remote control support

[Maciej Purski]

Hi Hans,
some time ago in reply to your email I described what messages does
the MHL driver receive and at what time intervals.
Regarding that information, do you think that a similar solution as
in [1] is required? Would it be OK, if I just set REP_DELAY and REP_PERIOD
to values, which I presented in my last email?


Best Regards,

Maciej

[1] 
https://git.linuxtv.org/media_tree.git/commit/drivers/media/cec/cec-adap.c?id=a9a249a2c997506a64eaee22f1458fda893f62a8

On 08/27/2017 02:40 PM, Hans Verkuil wrote:


Hi Maciej,

On 24/08/17 10:58, Maciej Purski wrote:

MHL specification defines Remote Control Protocol(RCP) to
send input events between MHL devices.
The driver now recognizes RCP messages and reacts to them
by reporting key events to input subsystem, allowing
a user to control a device using TV remote control.

Before I Ack this I would like to know how this behaves w.r.t. autorepeat.

If you keep pressing a remote key, what RCP messages do you receive and
at what time intervals? If that's similar to what CEC does, then it is
very likely that the same rules apply and I will have to review this patch
again with that in mind.

See the commit log for the patching fixing the CEC 'Press and Hold' behavior:

https://git.linuxtv.org/media_tree.git/commit/drivers/media/cec/cec-adap.c?id=a9a249a2c997506a64eaee22f1458fda893f62a8

If you have access to the HDMI 2.0 specification, then that spec describes the 
CEC
'Press and Hold' behavior in detail.

Regards,

Hans


Signed-off-by: Maciej Purski 
---

Changes in v2:
- use RC subsystem (including CEC keymap)
- RC device initialized in attach drm_bridge callback and
   removed in detach callback. This is necessary, because RC_CORE,
   which is needed during rc_dev init, is loaded after sii8620.
   DRM bridge is binded later which solves the problem.
- add RC_CORE dependency

Changes in v3:
- fix error handling in init_rcp and in attach callback

Changes in v4:
- usage of rc-core API compatible with upcoming changes
- fix error handling in init_rcp
- fix commit message
---
  drivers/gpu/drm/bridge/Kconfig   |  2 +-
  drivers/gpu/drm/bridge/sil-sii8620.c | 96 ++--
  include/drm/bridge/mhl.h |  4 ++
  3 files changed, 96 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
index adf9ae0..6ef901c 100644
--- a/drivers/gpu/drm/bridge/Kconfig
+++ b/drivers/gpu/drm/bridge/Kconfig
@@ -71,7 +71,7 @@ config DRM_PARADE_PS8622
  
  config DRM_SIL_SII8620

tristate "Silicon Image SII8620 HDMI/MHL bridge"
-   depends on OF
+   depends on OF && RC_CORE
select DRM_KMS_HELPER
help
  Silicon Image SII8620 HDMI/MHL bridge chip driver.
diff --git a/drivers/gpu/drm/bridge/sil-sii8620.c 
b/drivers/gpu/drm/bridge/sil-sii8620.c
index 2d51a22..ecb26c4 100644
--- a/drivers/gpu/drm/bridge/sil-sii8620.c
+++ b/drivers/gpu/drm/bridge/sil-sii8620.c
@@ -28,6 +28,8 @@
  #include 
  #include 
  
+#include 

+
  #include "sil-sii8620.h"
  
  #define SII8620_BURST_BUF_LEN 288

@@ -58,6 +60,7 @@ enum sii8620_mt_state {
  struct sii8620 {
struct drm_bridge bridge;
struct device *dev;
+   struct rc_dev *rc_dev;
struct clk *clk_xtal;
struct gpio_desc *gpio_reset;
struct gpio_desc *gpio_int;
@@ -431,6 +434,16 @@ static void sii8620_mt_rap(struct sii8620 *ctx, u8 code)
sii8620_mt_msc_msg(ctx, MHL_MSC_MSG_RAP, code);
  }
  
+static void sii8620_mt_rcpk(struct sii8620 *ctx, u8 code)

+{
+   sii8620_mt_msc_msg(ctx, MHL_MSC_MSG_RCPK, code);
+}
+
+static void sii8620_mt_rcpe(struct sii8620 *ctx, u8 code)
+{
+   sii8620_mt_msc_msg(ctx, MHL_MSC_MSG_RCPE, code);
+}
+
  static void sii8620_mt_read_devcap_send(struct sii8620 *ctx,
struct sii8620_mt_msg *msg)
  {
@@ -1753,6 +1766,25 @@ static void sii8620_send_features(struct sii8620 *ctx)
sii8620_write_buf(ctx, REG_MDT_XMIT_WRITE_PORT, buf, ARRAY_SIZE(buf));
  }
  
+static bool sii8620_rcp_consume(struct sii8620 *ctx, u8 scancode)

+{
+   bool pressed = !(scancode & MHL_RCP_KEY_RELEASED_MASK);
+
+   scancode &= MHL_RCP_KEY_ID_MASK;
+
+   if (!ctx->rc_dev) {
+   dev_dbg(ctx->dev, "RCP input device not initialized\n");
+   return false;
+   }
+
+   if (pressed)
+   rc_keydown(ctx->rc_dev, RC_PROTO_CEC, scancode, 0);
+   else
+   rc_keyup(ctx->rc_dev);
+
+   return true;
+}
+
  static void sii8620_msc_mr_set_int(struct sii8620 *ctx)
  {
u8 ints[MHL_INT_SIZE];
@@ -1804,19 +1836,25 @@ static void sii8620_msc_mt_done(struct sii8620 *ctx)
  
  static void sii8620_msc_mr_msc_msg(struct sii8620 *ctx)

  {
-   struct sii8620_mt_msg *msg = sii8620_msc_msg_first(ctx);
+   struct sii8620_mt_msg *msg;
u8 buf[2];
  
-	if (!msg)

-   return;
-
sii8620_read_buf(ctx, 

Re: [PATCHv4 3/3] drm/i915: add DisplayPort CEC-Tunneling-over-AUX support

[Hans Verkuil]

Hi Ville,

On 09/18/2017 03:05 PM, Ville Syrjälä wrote:
> On Sat, Sep 16, 2017 at 04:17:26PM +0200, Hans Verkuil wrote:
>> From: Hans Verkuil 
>>
>> Implement support for this DisplayPort feature.
>>
>> The cec device is created whenever it detects an adapter that
>> has this feature. It is only removed when a new adapter is connected
>> that does not support this. If a new adapter is connected that has
>> different properties than the previous one, then the old cec device is
>> unregistered and a new one is registered to replace the old one.
>>
>> Signed-off-by: Hans Verkuil 
>> Tested-by: Carlos Santa 
>> ---
>>  drivers/gpu/drm/i915/intel_dp.c | 18 ++
>>  1 file changed, 14 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_dp.c 
>> b/drivers/gpu/drm/i915/intel_dp.c
>> index 64fa774c855b..fdb853d2c458 100644
>> --- a/drivers/gpu/drm/i915/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>> @@ -32,6 +32,7 @@
>>  #include 
>>  #include 
>>  #include 
>> +#include 
>>  #include 
>>  #include 
>>  #include 
>> @@ -1449,6 +1450,7 @@ static void intel_aux_reg_init(struct intel_dp 
>> *intel_dp)
>>  static void
>>  intel_dp_aux_fini(struct intel_dp *intel_dp)
>>  {
>> +cec_unregister_adapter(intel_dp->aux.cec_adap);
>>  kfree(intel_dp->aux.name);
>>  }
>>  
>> @@ -4587,6 +4589,7 @@ intel_dp_set_edid(struct intel_dp *intel_dp)
>>  intel_connector->detect_edid = edid;
>>  
>>  intel_dp->has_audio = drm_detect_monitor_audio(edid);
>> +cec_s_phys_addr_from_edid(intel_dp->aux.cec_adap, edid);
>>  }
>>  
>>  static void
>> @@ -4596,6 +4599,7 @@ intel_dp_unset_edid(struct intel_dp *intel_dp)
>>  
>>  kfree(intel_connector->detect_edid);
>>  intel_connector->detect_edid = NULL;
>> +cec_phys_addr_invalidate(intel_dp->aux.cec_adap);
>>  
>>  intel_dp->has_audio = false;
>>  }
>> @@ -4616,13 +4620,17 @@ intel_dp_long_pulse(struct intel_connector 
>> *intel_connector)
>>  intel_display_power_get(to_i915(dev), intel_dp->aux_power_domain);
>>  
>>  /* Can't disconnect eDP, but you can close the lid... */
>> -if (is_edp(intel_dp))
>> +if (is_edp(intel_dp)) {
>>  status = edp_detect(intel_dp);
>> -else if (intel_digital_port_connected(to_i915(dev),
>> -  dp_to_dig_port(intel_dp)))
>> +} else if (intel_digital_port_connected(to_i915(dev),
>> +dp_to_dig_port(intel_dp))) {
>>  status = intel_dp_detect_dpcd(intel_dp);
>> -else
>> +if (status == connector_status_connected)
>> +drm_dp_cec_configure_adapter(_dp->aux,
>> + intel_dp->aux.name, dev->dev);
> 
> This is cluttering up the code a bit. Maybe do this call somewhere
> around the intel_dp_configure_mst() call instead since that seems to be
> the place where we start to do changes to externally visible state.
> 
> Actually, do we want to register cec adapters for MST devices?
> 
> And shouldn't we call this regardless of the connector state so that
> the cec adapter gets unregistered when the device is disconnected?

This hasn't (AFAIK) anything to do with MST. This is in a branch device (i.e.
a DP to HDMI adapter).

The CEC adapter should ideally be associated with the branch device (since that
is what implements the CEC tunneling): i.e. when you connect the adapter, then
the CEC device is created, when you disconnect the adapter, then the CEC device
should be unregistered. This is not the same as connecting/disconnecting the
HDMI cable to/from the adapter: that just sets or invalidates the CEC physical
address (which is read from the EDID).

However, I have not seen any code that tells me when the adapter is plugged in
or is unplugged. So all I have to go on is when the HDMI cable is connected.

Note that the 'late_register' you mentioned in your 1/3 review isn't called when
connecting the adapter. So that too cannot be used as a trigger to detect if
this protocol is supported.

I know doing this here is not ideal, but I have not found another way and I am
not even certain if it is possible at all, it might be intrinsic to how DP 
works.
I do not consider myself a DP expert, though.

So this is how it works now, e.g. on my Intel NUC and a USB-C to HDMI adapter:

1) I connect the adapter (no HDMI connected yet): nothing happens.
2) I connect the HDMI cable to the adapter: drm_dp_cec_configure_adapter is 
called and
   it detects the CEC capability in the DPCD. It now creates the CEC device.
3) I disconnect the HDMI cable: the physical address of the CEC device is 
invalidated,
   but the CEC device is not removed.
4) I disconnect the adapter: nothing happens.
5) I reconnect the adapter: nothing happens.
6) I reconnect the HDMI cable: drm_dp_cec_configure_adapter is called and it 
checks the
   DPCD. If the capabilities are 

[PATCH 6/6] [media] go7007: Delete an unnecessary variable initialisation in go7007_snd_init()

[SF Markus Elfring]

From: Markus Elfring 
Date: Mon, 18 Sep 2017 14:35:43 +0200

The variable "ret" will eventually be set to an appropriate value
a bit later. Thus omit the explicit initialisation at the beginning.

Signed-off-by: Markus Elfring 
---
 drivers/media/usb/go7007/snd-go7007.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/usb/go7007/snd-go7007.c 
b/drivers/media/usb/go7007/snd-go7007.c
index 7ae4d03ed3f7..27c09fd44657 100644
--- a/drivers/media/usb/go7007/snd-go7007.c
+++ b/drivers/media/usb/go7007/snd-go7007.c
@@ -227,7 +227,7 @@ int go7007_snd_init(struct go7007 *go)
 {
static int dev;
struct go7007_snd *gosnd;
-   int ret = 0;
+   int ret;
 
if (dev >= SNDRV_CARDS)
return -ENODEV;
-- 
2.14.1

[PATCH 5/6] [media] go7007: Use common error handling code in go7007_snd_init()

[SF Markus Elfring]

From: Markus Elfring 
Date: Mon, 18 Sep 2017 14:28:59 +0200

Add jump targets so that a bit of exception handling can be better reused
at the end of this function.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring 
---
 drivers/media/usb/go7007/snd-go7007.c | 35 +--
 1 file changed, 17 insertions(+), 18 deletions(-)

diff --git a/drivers/media/usb/go7007/snd-go7007.c 
b/drivers/media/usb/go7007/snd-go7007.c
index 68e421bf38e1..7ae4d03ed3f7 100644
--- a/drivers/media/usb/go7007/snd-go7007.c
+++ b/drivers/media/usb/go7007/snd-go7007.c
@@ -243,22 +243,18 @@ int go7007_snd_init(struct go7007 *go)
gosnd->capturing = 0;
ret = snd_card_new(go->dev, index[dev], id[dev], THIS_MODULE, 0,
   >card);
-   if (ret < 0) {
-   kfree(gosnd);
-   return ret;
-   }
+   if (ret < 0)
+   goto free_snd;
+
ret = snd_device_new(gosnd->card, SNDRV_DEV_LOWLEVEL, go,
_snd_device_ops);
-   if (ret < 0) {
-   kfree(gosnd);
-   return ret;
-   }
+   if (ret < 0)
+   goto free_snd;
+
ret = snd_pcm_new(gosnd->card, "go7007", 0, 0, 1, >pcm);
-   if (ret < 0) {
-   snd_card_free(gosnd->card);
-   kfree(gosnd);
-   return ret;
-   }
+   if (ret < 0)
+   goto free_card;
+
strlcpy(gosnd->card->driver, "go7007", sizeof(gosnd->card->driver));
strlcpy(gosnd->card->shortname, go->name, sizeof(gosnd->card->driver));
strlcpy(gosnd->card->longname, gosnd->card->shortname,
@@ -269,11 +265,8 @@ int go7007_snd_init(struct go7007 *go)
_snd_capture_ops);
 
ret = snd_card_register(gosnd->card);
-   if (ret < 0) {
-   snd_card_free(gosnd->card);
-   kfree(gosnd);
-   return ret;
-   }
+   if (ret < 0)
+   goto free_card;
 
gosnd->substream = NULL;
go->snd_context = gosnd;
@@ -281,6 +274,12 @@ int go7007_snd_init(struct go7007 *go)
++dev;
 
return 0;
+
+free_card:
+   snd_card_free(gosnd->card);
+free_snd:
+   kfree(gosnd);
+   return ret;
 }
 EXPORT_SYMBOL(go7007_snd_init);
 
-- 
2.14.1

[PATCH 4/6] [media] go7007: Use common error handling code in s2250_probe()

[SF Markus Elfring]

From: Markus Elfring 
Date: Mon, 18 Sep 2017 13:50:45 +0200

Adjust jump targets so that a bit of exception handling can be better
reused at the end of this function.

This refactoring might fix also an error situation where the
function "i2c_unregister_device" was not called after a software failure
was noticed by the data member "hdl.error".

Signed-off-by: Markus Elfring 
---
 drivers/media/usb/go7007/s2250-board.c | 37 +-
 1 file changed, 18 insertions(+), 19 deletions(-)

diff --git a/drivers/media/usb/go7007/s2250-board.c 
b/drivers/media/usb/go7007/s2250-board.c
index 1fd4c09dd516..1bd9a7f2e7a3 100644
--- a/drivers/media/usb/go7007/s2250-board.c
+++ b/drivers/media/usb/go7007/s2250-board.c
@@ -510,6 +510,7 @@ static int s2250_probe(struct i2c_client *client,
u8 *data;
struct go7007 *go = i2c_get_adapdata(adapter);
struct go7007_usb *usb = go->hpi_context;
+   int code;
 
audio = i2c_new_dummy(adapter, TLV320_ADDRESS >> 1);
if (!audio)
@@ -517,8 +518,8 @@ static int s2250_probe(struct i2c_client *client,
 
state = kzalloc(sizeof(*state), GFP_KERNEL);
if (!state) {
-   i2c_unregister_device(audio);
-   return -ENOMEM;
+   code = -ENOMEM;
+   goto unregister_device;
}
 
sd = >sd;
@@ -538,11 +539,8 @@ static int s2250_probe(struct i2c_client *client,
V4L2_CID_HUE, -512, 511, 1, 0);
sd->ctrl_handler = >hdl;
if (state->hdl.error) {
-   int err = state->hdl.error;
-
-   v4l2_ctrl_handler_free(>hdl);
-   kfree(state);
-   return err;
+   code = state->hdl.error;
+   goto free_handler;
}
 
state->std = V4L2_STD_NTSC;
@@ -555,17 +553,13 @@ static int s2250_probe(struct i2c_client *client,
/* initialize the audio */
if (write_regs(audio, aud_regs) < 0) {
dev_err(>dev, "error initializing audio\n");
-   goto fail;
+   goto e_io;
}
 
-   if (write_regs(client, vid_regs) < 0) {
-   dev_err(>dev, "error initializing decoder\n");
-   goto fail;
-   }
-   if (write_regs_fp(client, vid_regs_fp) < 0) {
-   dev_err(>dev, "error initializing decoder\n");
-   goto fail;
-   }
+   if (write_regs(client, vid_regs) < 0 ||
+   write_regs_fp(client, vid_regs_fp) < 0)
+   goto report_write_failure;
+
/* set default channel */
/* composite */
write_reg_fp(client, 0x20, 0x020 | 1);
@@ -602,11 +596,16 @@ static int s2250_probe(struct i2c_client *client,
v4l2_info(sd, "initialized successfully\n");
return 0;
 
-fail:
-   i2c_unregister_device(audio);
+report_write_failure:
+   dev_err(>dev, "error initializing decoder\n");
+e_io:
+   code = -EIO;
+free_handler:
v4l2_ctrl_handler_free(>hdl);
kfree(state);
-   return -EIO;
+unregister_device:
+   i2c_unregister_device(audio);
+   return code;
 }
 
 static int s2250_remove(struct i2c_client *client)
-- 
2.14.1

[PATCH 3/6] [media] go7007: Improve a size determination in four functions

[SF Markus Elfring]

From: Markus Elfring 
Date: Mon, 18 Sep 2017 11:27:30 +0200

Replace the specification of data structures by pointer dereferences
as the parameter for the operator "sizeof" to make the corresponding size
determination a bit safer according to the Linux coding style convention.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring 
---
 drivers/media/usb/go7007/go7007-driver.c | 2 +-
 drivers/media/usb/go7007/go7007-usb.c| 2 +-
 drivers/media/usb/go7007/s2250-board.c   | 2 +-
 drivers/media/usb/go7007/snd-go7007.c| 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/media/usb/go7007/go7007-driver.c 
b/drivers/media/usb/go7007/go7007-driver.c
index 390f66ec8fd2..f8c06b2f9d2f 100644
--- a/drivers/media/usb/go7007/go7007-driver.c
+++ b/drivers/media/usb/go7007/go7007-driver.c
@@ -699,5 +699,5 @@ struct go7007 *go7007_alloc(const struct go7007_board_info 
*board,
struct go7007 *go;
int i;
 
-   go = kzalloc(sizeof(struct go7007), GFP_KERNEL);
+   go = kzalloc(sizeof(*go), GFP_KERNEL);
if (!go)
diff --git a/drivers/media/usb/go7007/go7007-usb.c 
b/drivers/media/usb/go7007/go7007-usb.c
index 5ad40b77763d..f0f70a92541a 100644
--- a/drivers/media/usb/go7007/go7007-usb.c
+++ b/drivers/media/usb/go7007/go7007-usb.c
@@ -1122,5 +1122,5 @@ static int go7007_usb_probe(struct usb_interface *intf,
if (!go)
return -ENOMEM;
 
-   usb = kzalloc(sizeof(struct go7007_usb), GFP_KERNEL);
+   usb = kzalloc(sizeof(*usb), GFP_KERNEL);
if (!usb) {
diff --git a/drivers/media/usb/go7007/s2250-board.c 
b/drivers/media/usb/go7007/s2250-board.c
index d987c5f2b45a..1fd4c09dd516 100644
--- a/drivers/media/usb/go7007/s2250-board.c
+++ b/drivers/media/usb/go7007/s2250-board.c
@@ -515,5 +515,5 @@ static int s2250_probe(struct i2c_client *client,
if (!audio)
return -ENOMEM;
 
-   state = kzalloc(sizeof(struct s2250), GFP_KERNEL);
+   state = kzalloc(sizeof(*state), GFP_KERNEL);
if (!state) {
diff --git a/drivers/media/usb/go7007/snd-go7007.c 
b/drivers/media/usb/go7007/snd-go7007.c
index 4e612cf1afd9..68e421bf38e1 100644
--- a/drivers/media/usb/go7007/snd-go7007.c
+++ b/drivers/media/usb/go7007/snd-go7007.c
@@ -235,5 +235,5 @@ int go7007_snd_init(struct go7007 *go)
dev++;
return -ENOENT;
}
-   gosnd = kmalloc(sizeof(struct go7007_snd), GFP_KERNEL);
+   gosnd = kmalloc(sizeof(*gosnd), GFP_KERNEL);
if (!gosnd)
-- 
2.14.1

[PATCH 2/6] [media] go7007: Adjust 35 checks for null pointers

[SF Markus Elfring]

From: Markus Elfring 
Date: Mon, 18 Sep 2017 11:13:27 +0200
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The script “checkpatch.pl” pointed information out like the following.

Comparison to NULL could be written …

Thus fix the affected source code places.

Signed-off-by: Markus Elfring 
---
 drivers/media/usb/go7007/go7007-driver.c | 10 +-
 drivers/media/usb/go7007/go7007-fw.c |  8 
 drivers/media/usb/go7007/go7007-loader.c |  4 ++--
 drivers/media/usb/go7007/go7007-usb.c| 18 +-
 drivers/media/usb/go7007/go7007-v4l2.c   |  6 +++---
 drivers/media/usb/go7007/s2250-board.c   | 20 +---
 drivers/media/usb/go7007/snd-go7007.c|  6 +++---
 7 files changed, 35 insertions(+), 37 deletions(-)

diff --git a/drivers/media/usb/go7007/go7007-driver.c 
b/drivers/media/usb/go7007/go7007-driver.c
index 222332189fa4..390f66ec8fd2 100644
--- a/drivers/media/usb/go7007/go7007-driver.c
+++ b/drivers/media/usb/go7007/go7007-driver.c
@@ -91,6 +91,6 @@ static int go7007_load_encoder(struct go7007 *go)
int fw_len, rv = 0;
u16 intr_val, intr_data;
 
-   if (go->boot_fw == NULL) {
+   if (!go->boot_fw) {
if (request_firmware(_entry, fw_name, go->dev)) {
v4l2_err(go, "unable to load firmware from file 
\"%s\"\n", fw_name);
@@ -103,5 +103,5 @@ static int go7007_load_encoder(struct go7007 *go)
}
fw_len = fw_entry->size - 16;
bounce = kmemdup(fw_entry->data + 16, fw_len, GFP_KERNEL);
-   if (bounce == NULL) {
+   if (!bounce) {
release_firmware(fw_entry);
@@ -448,7 +448,7 @@ static struct go7007_buffer *frame_boundary(struct go7007 
*go, struct go7007_buf
u32 *bytesused;
struct go7007_buffer *vb_tmp = NULL;
 
-   if (vb == NULL) {
+   if (!vb) {
spin_lock(>spinlock);
if (!list_empty(>vidq_active))
vb = go->active_buf =
@@ -598,7 +598,7 @@ void go7007_parse_video_stream(struct go7007 *go, u8 *buf, 
int length)
(buf[i] == seq_start_code ||
 buf[i] == gop_start_code ||
 buf[i] == frame_start_code)) {
-   if (vb == NULL || go->seen_frame)
+   if (!vb || go->seen_frame)
vb = frame_boundary(go, vb);
go->seen_frame = buf[i] == frame_start_code;
if (vb && go->seen_frame)
@@ -703,4 +703,4 @@ struct go7007 *go7007_alloc(const struct go7007_board_info 
*board,
-   if (go == NULL)
+   if (!go)
return NULL;
go->dev = dev;
go->board_info = board;
diff --git a/drivers/media/usb/go7007/go7007-fw.c 
b/drivers/media/usb/go7007/go7007-fw.c
index 60bf5f0644d1..a70a3fba79fb 100644
--- a/drivers/media/usb/go7007/go7007-fw.c
+++ b/drivers/media/usb/go7007/go7007-fw.c
@@ -378,7 +378,7 @@ static int gen_mjpeghdr_to_package(struct go7007 *go, 
__le16 *code, int space)
int size = 0, i, off = 0, chunk;
 
buf = kzalloc(4096, GFP_KERNEL);
-   if (buf == NULL)
+   if (!buf)
return -ENOMEM;
 
for (i = 1; i < 32; ++i) {
@@ -645,7 +645,7 @@ static int gen_mpeg1hdr_to_package(struct go7007 *go,
int i, off = 0, chunk;
 
buf = kzalloc(5120, GFP_KERNEL);
-   if (buf == NULL)
+   if (!buf)
return -ENOMEM;
 
framelen[0] = mpeg1_frame_header(go, buf, 0, 1, PFRAME);
@@ -831,7 +831,7 @@ static int gen_mpeg4hdr_to_package(struct go7007 *go,
int i, off = 0, chunk;
 
buf = kzalloc(5120, GFP_KERNEL);
-   if (buf == NULL)
+   if (!buf)
return -ENOMEM;
 
framelen[0] = mpeg4_frame_header(go, buf, 0, PFRAME);
@@ -1577,7 +1577,7 @@ int go7007_construct_fw_image(struct go7007 *go, u8 **fw, 
int *fwlen)
return -1;
}
code = kzalloc(codespace * 2, GFP_KERNEL);
-   if (code == NULL)
+   if (!code)
goto fw_failed;
 
src = (__le16 *)fw_entry->data;
diff --git a/drivers/media/usb/go7007/go7007-loader.c 
b/drivers/media/usb/go7007/go7007-loader.c
index 042f78a31283..5e94f03c044d 100644
--- a/drivers/media/usb/go7007/go7007-loader.c
+++ b/drivers/media/usb/go7007/go7007-loader.c
@@ -67,7 +67,7 @@ static int go7007_loader_probe(struct usb_interface 
*interface,
break;
 
/* Should never happen */
-   if (fw_configs[i].fw_name1 == NULL)
+   if (!fw_configs[i].fw_name1)
goto failed2;
 
fw1 = fw_configs[i].fw_name1;
@@ -87,7 +87,7 @@ static int go7007_loader_probe(struct usb_interface 
*interface,
goto failed2;
}
 
-   if (fw2 

[PATCH 1/6] [media] go7007: Delete an error message for a failed memory allocation in go7007_load_encoder()

[SF Markus Elfring]

From: Markus Elfring 
Date: Mon, 18 Sep 2017 10:52:42 +0200

Omit an extra message for a memory allocation failure in this function.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring 
---
 drivers/media/usb/go7007/go7007-driver.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/media/usb/go7007/go7007-driver.c 
b/drivers/media/usb/go7007/go7007-driver.c
index 05b1126f263e..222332189fa4 100644
--- a/drivers/media/usb/go7007/go7007-driver.c
+++ b/drivers/media/usb/go7007/go7007-driver.c
@@ -107,4 +107,3 @@ static int go7007_load_encoder(struct go7007 *go)
-   v4l2_err(go, "unable to allocate %d bytes for firmware 
transfer\n", fw_len);
release_firmware(fw_entry);
return -1;
}
-- 
2.14.1

[PATCH 0/6] [media] Go7007: Adjustments for some function implementations

[SF Markus Elfring]

From: Markus Elfring 
Date: Mon, 18 Sep 2017 14:54:32 +0200

Some update suggestions were taken into account
from static source code analysis.

Markus Elfring (6):
  Delete an error message for a failed memory allocation in 
go7007_load_encoder()
  Adjust 35 checks for null pointers
  Improve a size determination in four functions
  Use common error handling code in s2250_probe()
  Use common error handling code in go7007_snd_init()
  Delete an unnecessary variable initialisation in go7007_snd_init()

 drivers/media/usb/go7007/go7007-driver.c | 13 ---
 drivers/media/usb/go7007/go7007-fw.c |  8 ++---
 drivers/media/usb/go7007/go7007-loader.c |  4 +--
 drivers/media/usb/go7007/go7007-usb.c| 20 +--
 drivers/media/usb/go7007/go7007-v4l2.c   |  6 ++--
 drivers/media/usb/go7007/s2250-board.c   | 59 +++-
 drivers/media/usb/go7007/snd-go7007.c| 45 
 7 files changed, 75 insertions(+), 80 deletions(-)

-- 
2.14.1

[PATCH 1/1] staging: atomisp: Add driver prefix to Kconfig option and module names

[Sakari Ailus]

By adding the "atomisp-" prefix to module names (and "ATOMISP_" to Kconfig
options), the staging drivers for e.g. sensors are labelled as being
specific to atomisp, which they effectively are.

Signed-off-by: Sakari Ailus 
---
 drivers/staging/media/atomisp/i2c/Kconfig| 18 +-
 drivers/staging/media/atomisp/i2c/Makefile   | 20 ++--
 .../media/atomisp/i2c/{ap1302.c => atomisp-ap1302.c} |  0
 .../media/atomisp/i2c/{gc0310.c => atomisp-gc0310.c} |  0
 .../media/atomisp/i2c/{gc2235.c => atomisp-gc2235.c} |  0
 ...libmsrlisthelper.c => atomisp-libmsrlisthelper.c} |  0
 .../media/atomisp/i2c/{lm3554.c => atomisp-lm3554.c} |  0
 .../atomisp/i2c/{mt9m114.c => atomisp-mt9m114.c} |  0
 .../media/atomisp/i2c/{ov2680.c => atomisp-ov2680.c} |  0
 .../media/atomisp/i2c/{ov2722.c => atomisp-ov2722.c} |  0
 drivers/staging/media/atomisp/i2c/imx/Kconfig|  4 ++--
 drivers/staging/media/atomisp/i2c/imx/Makefile   |  8 
 drivers/staging/media/atomisp/i2c/ov5693/Kconfig |  2 +-
 drivers/staging/media/atomisp/i2c/ov5693/Makefile|  2 +-
 .../i2c/ov5693/{ov5693.c => atomisp-ov5693.c}|  0
 15 files changed, 27 insertions(+), 27 deletions(-)
 rename drivers/staging/media/atomisp/i2c/{ap1302.c => atomisp-ap1302.c} (100%)
 rename drivers/staging/media/atomisp/i2c/{gc0310.c => atomisp-gc0310.c} (100%)
 rename drivers/staging/media/atomisp/i2c/{gc2235.c => atomisp-gc2235.c} (100%)
 rename drivers/staging/media/atomisp/i2c/{libmsrlisthelper.c => 
atomisp-libmsrlisthelper.c} (100%)
 rename drivers/staging/media/atomisp/i2c/{lm3554.c => atomisp-lm3554.c} (100%)
 rename drivers/staging/media/atomisp/i2c/{mt9m114.c => atomisp-mt9m114.c} 
(100%)
 rename drivers/staging/media/atomisp/i2c/{ov2680.c => atomisp-ov2680.c} (100%)
 rename drivers/staging/media/atomisp/i2c/{ov2722.c => atomisp-ov2722.c} (100%)
 rename drivers/staging/media/atomisp/i2c/ov5693/{ov5693.c => atomisp-ov5693.c} 
(100%)

diff --git a/drivers/staging/media/atomisp/i2c/Kconfig 
b/drivers/staging/media/atomisp/i2c/Kconfig
index 57505b7..09b1a97 100644
--- a/drivers/staging/media/atomisp/i2c/Kconfig
+++ b/drivers/staging/media/atomisp/i2c/Kconfig
@@ -5,7 +5,7 @@
 source "drivers/staging/media/atomisp/i2c/ov5693/Kconfig"
 source "drivers/staging/media/atomisp/i2c/imx/Kconfig"
 
-config VIDEO_OV2722
+config VIDEO_ATOMISP_OV2722
tristate "OVT ov2722 sensor support"
depends on I2C && VIDEO_V4L2
---help---
@@ -16,7 +16,7 @@ config VIDEO_OV2722
 
 It currently only works with the atomisp driver.
 
-config VIDEO_GC2235
+config VIDEO_ATOMISP_GC2235
tristate "Galaxy gc2235 sensor support"
depends on I2C && VIDEO_V4L2
---help---
@@ -27,7 +27,7 @@ config VIDEO_GC2235
 
 It currently only works with the atomisp driver.
 
-config VIDEO_OV8858
+config VIDEO_ATOMISP_OV8858
tristate "Omnivision ov8858 sensor support"
depends on I2C && VIDEO_V4L2 && VIDEO_ATOMISP
---help---
@@ -38,7 +38,7 @@ config VIDEO_OV8858
 
 It currently only works with the atomisp driver.
 
-config VIDEO_MSRLIST_HELPER
+config VIDEO_ATOMISP_MSRLIST_HELPER
tristate "Helper library to load, parse and apply large register lists."
depends on I2C
---help---
@@ -48,7 +48,7 @@ config VIDEO_MSRLIST_HELPER
 To compile this driver as a module, choose M here: the
 module will be called libmsrlisthelper.
 
-config VIDEO_MT9M114
+config VIDEO_ATOMISP_MT9M114
tristate "Aptina mt9m114 sensor support"
depends on I2C && VIDEO_V4L2
---help---
@@ -59,7 +59,7 @@ config VIDEO_MT9M114
 
 It currently only works with the atomisp driver.
 
-config VIDEO_AP1302
+config VIDEO_ATOMISP_AP1302
tristate "AP1302 external ISP support"
depends on I2C && VIDEO_V4L2
select REGMAP_I2C
@@ -71,14 +71,14 @@ config VIDEO_AP1302
 
 It currently only works with the atomisp driver.
 
-config VIDEO_GC0310
+config VIDEO_ATOMISP_GC0310
tristate "GC0310 sensor support"
depends on I2C && VIDEO_V4L2
---help---
  This is a Video4Linux2 sensor-level driver for the Galaxycore
  GC0310 0.3MP sensor.
 
-config VIDEO_OV2680
+config VIDEO_ATOMISP_OV2680
tristate "Omnivision OV2680 sensor support"
depends on I2C && VIDEO_V4L2
---help---
@@ -93,7 +93,7 @@ config VIDEO_OV2680
 # Kconfig for flash drivers
 #
 
-config VIDEO_LM3554
+config VIDEO_ATOMISP_LM3554
tristate "LM3554 flash light driver"
depends on VIDEO_V4L2 && I2C
---help---
diff --git a/drivers/staging/media/atomisp/i2c/Makefile 
b/drivers/staging/media/atomisp/i2c/Makefile
index be13fab..3d27c75 100644
--- a/drivers/staging/media/atomisp/i2c/Makefile
+++ b/drivers/staging/media/atomisp/i2c/Makefile
@@ -2,22 +2,22 @@
 # Makefile for sensor drivers
 #
 
-obj-$(CONFIG_VIDEO_IMX)+= imx/

Re: [PATCH v4 3/3] media: ov7670: Add the s_power operation

[Sakari Ailus]

Hi Wenyou,

On Mon, Sep 18, 2017 at 05:26:16PM +0800, Yang, Wenyou wrote:
> Hi Sakari,
> 
> 
> On 2017/9/18 15:35, Sakari Ailus wrote:
> > Hi Wenyou,
> > 
> > Thanks for the update.
> > 
> > The driver exposes controls that are accessible through the sub-device node
> > even if the device hasn't been powered on.
> > 
> > Many ISP and bridge drivers will also power the sensor down once the last
> > user of the user space device nodes disappears. You could keep the device
> > powered at all times or change the behaviour so that controls can be
> > accessed when the power is off.
> > 
> > The best option would be to convert the driver to use runtime PM.
> Yes, I agree with you.
> I also noticed there are a lot of things needed to improve except for it,
> such as the platform data via device tree.
> I would like do it in another patch set. I will continue to work on it.

Adding runtime PM support later on sound good to me.

> > An example of this can be found in drivers/media/i2c/ov13858.c .
> A good example, thank you for your providing.

-- 
Sakari Ailus
e-mail: sakari.ai...@iki.fi

Re: [PATCH] media: i2c: OV5647: ensure clock lane in LP-11 state before streaming on

[Sakari Ailus]

Hi Jacob,

On Mon, Sep 11, 2017 at 09:53:40AM +0800, Jacob Chen wrote:
> When I was supporting Rpi Camera Module on the ASUS Tinker board,
> I found this driver have some issues with rockchip's mipi-csi driver.
> It didn't place clock lane in LP-11 state before performing
> D-PHY initialisation.
> 
> From our experience, on some OV sensors,
> LP-11 state is not achieved while BIT(5)-0x4800 is cleared.
> 
> So let's set BIT(5) and BIT(0) both while not streaming, in order to
> coax the clock lane into LP-11 state.
> 
> 0x4800 : MIPI CTRL 00
>   BIT(5) : clock lane gate enable
>   0: continuous
>   1: none-continuous
>   BIT(0) : manually set clock lane
>   0: Not used
>   1: used
> 
> Changes in V2:
>   modify commit messages

Change log doesn't belong to the commit message.

> 
> Signed-off-by: Jacob Chen 
> ---
>  drivers/media/i2c/ov5647.c | 13 -
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/i2c/ov5647.c b/drivers/media/i2c/ov5647.c
> index 95ce90f..247302d 100644
> --- a/drivers/media/i2c/ov5647.c
> +++ b/drivers/media/i2c/ov5647.c
> @@ -253,6 +253,10 @@ static int ov5647_stream_on(struct v4l2_subdev *sd)
>  {
>   int ret;
>  
> + ret = ov5647_write(sd, 0x4800, 0x04);

Could you use macros for the register bits instead of using magic number,
please, especially now that it appears that the meaning of these bits is
known?

> + if (ret < 0)
> + return ret;
> +
>   ret = ov5647_write(sd, 0x4202, 0x00);
>   if (ret < 0)
>   return ret;
> @@ -264,6 +268,10 @@ static int ov5647_stream_off(struct v4l2_subdev *sd)
>  {
>   int ret;
>  
> + ret = ov5647_write(sd, 0x4800, 0x25);
> + if (ret < 0)
> + return ret;
> +
>   ret = ov5647_write(sd, 0x4202, 0x0f);
>   if (ret < 0)
>   return ret;
> @@ -320,7 +328,10 @@ static int __sensor_init(struct v4l2_subdev *sd)
>   return ret;
>   }
>  
> - return ov5647_write(sd, 0x4800, 0x04);
> + /*
> +  * stream off to make the clock lane into LP-11 state.
> +  */
> + return ov5647_stream_off(sd);
>  }
>  
>  static int ov5647_sensor_power(struct v4l2_subdev *sd, int on)
> -- 
> 2.7.4
> 

-- 
Kind regards,

Sakari Ailus
e-mail: sakari.ai...@iki.fi

Re: [PATCHv4 3/3] drm/i915: add DisplayPort CEC-Tunneling-over-AUX support

[Ville Syrjälä]

On Sat, Sep 16, 2017 at 04:17:26PM +0200, Hans Verkuil wrote:
> From: Hans Verkuil 
> 
> Implement support for this DisplayPort feature.
> 
> The cec device is created whenever it detects an adapter that
> has this feature. It is only removed when a new adapter is connected
> that does not support this. If a new adapter is connected that has
> different properties than the previous one, then the old cec device is
> unregistered and a new one is registered to replace the old one.
> 
> Signed-off-by: Hans Verkuil 
> Tested-by: Carlos Santa 
> ---
>  drivers/gpu/drm/i915/intel_dp.c | 18 ++
>  1 file changed, 14 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 64fa774c855b..fdb853d2c458 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -32,6 +32,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -1449,6 +1450,7 @@ static void intel_aux_reg_init(struct intel_dp 
> *intel_dp)
>  static void
>  intel_dp_aux_fini(struct intel_dp *intel_dp)
>  {
> + cec_unregister_adapter(intel_dp->aux.cec_adap);
>   kfree(intel_dp->aux.name);
>  }
>  
> @@ -4587,6 +4589,7 @@ intel_dp_set_edid(struct intel_dp *intel_dp)
>   intel_connector->detect_edid = edid;
>  
>   intel_dp->has_audio = drm_detect_monitor_audio(edid);
> + cec_s_phys_addr_from_edid(intel_dp->aux.cec_adap, edid);
>  }
>  
>  static void
> @@ -4596,6 +4599,7 @@ intel_dp_unset_edid(struct intel_dp *intel_dp)
>  
>   kfree(intel_connector->detect_edid);
>   intel_connector->detect_edid = NULL;
> + cec_phys_addr_invalidate(intel_dp->aux.cec_adap);
>  
>   intel_dp->has_audio = false;
>  }
> @@ -4616,13 +4620,17 @@ intel_dp_long_pulse(struct intel_connector 
> *intel_connector)
>   intel_display_power_get(to_i915(dev), intel_dp->aux_power_domain);
>  
>   /* Can't disconnect eDP, but you can close the lid... */
> - if (is_edp(intel_dp))
> + if (is_edp(intel_dp)) {
>   status = edp_detect(intel_dp);
> - else if (intel_digital_port_connected(to_i915(dev),
> -   dp_to_dig_port(intel_dp)))
> + } else if (intel_digital_port_connected(to_i915(dev),
> + dp_to_dig_port(intel_dp))) {
>   status = intel_dp_detect_dpcd(intel_dp);
> - else
> + if (status == connector_status_connected)
> + drm_dp_cec_configure_adapter(_dp->aux,
> +  intel_dp->aux.name, dev->dev);

This is cluttering up the code a bit. Maybe do this call somewhere
around the intel_dp_configure_mst() call instead since that seems to be
the place where we start to do changes to externally visible state.

Actually, do we want to register cec adapters for MST devices?

And shouldn't we call this regardless of the connector state so that
the cec adapter gets unregistered when the device is disconnected?

> + } else {
>   status = connector_status_disconnected;
> + }
>  
>   if (status == connector_status_disconnected) {
>   memset(_dp->compliance, 0, sizeof(intel_dp->compliance));
> @@ -5011,6 +5019,8 @@ intel_dp_hpd_pulse(struct intel_digital_port 
> *intel_dig_port, bool long_hpd)
>  
>   intel_display_power_get(dev_priv, intel_dp->aux_power_domain);
>  
> + drm_dp_cec_irq(_dp->aux);
> +
>   if (intel_dp->is_mst) {
>   if (intel_dp_check_mst_status(intel_dp) == -EINVAL) {
>   /*
> -- 
> 2.14.1

-- 
Ville Syrjälä
Intel OTC

Re: [PATCHv4 1/3] drm: add support for DisplayPort CEC-Tunneling-over-AUX

[Ville Syrjälä]

On Sat, Sep 16, 2017 at 04:17:24PM +0200, Hans Verkuil wrote:
> From: Hans Verkuil 
> 
> This adds support for the DisplayPort CEC-Tunneling-over-AUX
> feature that is part of the DisplayPort 1.3 standard.
> 
> Unfortunately, not all DisplayPort/USB-C to HDMI adapters with a
> chip that has this capability actually hook up the CEC pin, so
> even though a CEC device is created, it may not actually work.
> 

FYI we prefer to put the changelog into the commit message. Makes it
easier to see which version of the patch we're dealing with, and avoids
having to open up some other mail when doing the review.

> Signed-off-by: Hans Verkuil 
> Tested-by: Carlos Santa 
> ---
>  drivers/gpu/drm/Kconfig  |  10 ++
>  drivers/gpu/drm/Makefile |   1 +
>  drivers/gpu/drm/drm_dp_cec.c | 292 
> +++
>  include/drm/drm_dp_helper.h  |  24 
>  4 files changed, 327 insertions(+)
>  create mode 100644 drivers/gpu/drm/drm_dp_cec.c
> 
> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> index 83cb2a88c204..1f2708df5c4e 100644
> --- a/drivers/gpu/drm/Kconfig
> +++ b/drivers/gpu/drm/Kconfig
> @@ -120,6 +120,16 @@ config DRM_LOAD_EDID_FIRMWARE
> default case is N. Details and instructions how to build your own
> EDID data are given in Documentation/EDID/HOWTO.txt.
>  
> +config DRM_DP_CEC
> + bool "Enable DisplayPort CEC-Tunneling-over-AUX HDMI support"
> + select CEC_CORE
> + help
> +   Choose this option if you want to enable HDMI CEC support for
> +   DisplayPort/USB-C to HDMI adapters.
> +
> +   Note: not all adapters support this feature, and even for those
> +   that do support this they often do not hook up the CEC pin.
> +
>  config DRM_TTM
>   tristate
>   depends on DRM && MMU
> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> index 24a066e1841c..c6552c62049e 100644
> --- a/drivers/gpu/drm/Makefile
> +++ b/drivers/gpu/drm/Makefile
> @@ -40,6 +40,7 @@ drm_kms_helper-$(CONFIG_DRM_LOAD_EDID_FIRMWARE) += 
> drm_edid_load.o
>  drm_kms_helper-$(CONFIG_DRM_FBDEV_EMULATION) += drm_fb_helper.o
>  drm_kms_helper-$(CONFIG_DRM_KMS_CMA_HELPER) += drm_fb_cma_helper.o
>  drm_kms_helper-$(CONFIG_DRM_DP_AUX_CHARDEV) += drm_dp_aux_dev.o
> +drm_kms_helper-$(CONFIG_DRM_DP_CEC) += drm_dp_cec.o
>  
>  obj-$(CONFIG_DRM_KMS_HELPER) += drm_kms_helper.o
>  obj-$(CONFIG_DRM_DEBUG_MM_SELFTEST) += selftests/
> diff --git a/drivers/gpu/drm/drm_dp_cec.c b/drivers/gpu/drm/drm_dp_cec.c
> new file mode 100644
> index ..d110cac007de
> --- /dev/null
> +++ b/drivers/gpu/drm/drm_dp_cec.c
> @@ -0,0 +1,292 @@
> +/*
> + * DisplayPort CEC-Tunneling-over-AUX support
> + *
> + * Copyright 2017 Cisco Systems, Inc. and/or its affiliates. All rights 
> reserved.
> + *
> + * This program is free software; you may redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; version 2 of the License.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> + * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
> + * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
> + * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
> + * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
> + * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
> + * SOFTWARE.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +/*
> + * Unfortunately it turns out that we have a chicken-and-egg situation
> + * here. Quite a few active (mini-)DP-to-HDMI or USB-C-to-HDMI adapters
> + * have a converter chip that supports CEC-Tunneling-over-AUX (usually the
> + * Parade PS176), but they do not wire up the CEC pin, thus making CEC
> + * useless.
> + *
> + * Sadly there is no way for this driver to know this. What happens is
> + * that a /dev/cecX device is created that is isolated and unable to see
> + * any of the other CEC devices. Quite literally the CEC wire is cut
> + * (or in this case, never connected in the first place).
> + *
> + * I suspect that the reason so few adapters support this is that this
> + * tunneling protocol was never supported by any OS. So there was no
> + * easy way of testing it, and no incentive to correctly wire up the
> + * CEC pin.
> + *
> + * Hopefully by creating this driver it will be easier for vendors to
> + * finally fix their adapters and test the CEC functionality.
> + *
> + * I keep a list of known working adapters here:
> + *
> + * https://hverkuil.home.xs4all.nl/cec-status.txt
> + *
> + * Please mail me (hverk...@xs4all.nl) if you find an adapter that works
> + * and is not yet listed there.
> + */
> +
> +/**
> + * DOC: dp cec helpers
> + *
> + * These functions take care of 

Re: [PATCH] Support HVR-1200 analog video as a clone of HVR-1500. Tested, composite and s-video inputs.

[Devin Heitmueller]

On Sun, Sep 17, 2017 at 5:42 PM, Nigel Kettlewell
 wrote:
> I propose the following patch to support Hauppauge HVR-1200 analog video,
> nothing more than a clone of HVR-1500. Patch based on Linux 4.9 commit
> 69973b830859bc6529a7a0468ba0d80ee5117826
>
> I have tested composite and S-Video inputs.
>
> With the change, HVR-1200 devices have a /dev/video entry which is
> accessible in the normal way.
>
> Let me know if you need anything more.

I'm not confident the tuner config for this board is correct.  The
HVR-1200 is much closer to the HVR-1250 as opposed to the HVR-1500,
and IIRC it didn't have an xc3028.

I don't dispute that with the patch in question the composite/s-video
are probably working ok, but I wouldn't recommend accepting this patch
as-is until the tuner is verified for DVB-T and analog (ideally both).

Can you provide the output of dmesg on device load?  If it's filled
with a bunch of errors showing xc3028 firmware load failures, that
would be a smoking gun that it doesn't have the xc3028.

Devin

-- 
Devin J. Heitmueller - Kernel Labs
http://www.kernellabs.com

Re: [PATCH v2] media: ov13858: Calculate pixel-rate at runtime, use mode

[Sakari Ailus]

Hi Chiranjeevi,

On Wed, Sep 06, 2017 at 11:26:33AM -0700, Chiranjeevi Rapolu wrote:
> Calculate pixel-rate at run time instead of compile time.
> 
> Instead of using hardcoded pixels-per-line, extract it from current sensor
> mode.
> 
> Signed-off-by: Chiranjeevi Rapolu 
> ---
> Changes in v2:
>   - Removed pixel-rate from struct definition.
>   - Used pixel-rate formula wherever needed.
>   - Changed commit message to reflect above changes.
>  drivers/media/i2c/ov13858.c | 42 ++
>  1 file changed, 22 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/media/i2c/ov13858.c b/drivers/media/i2c/ov13858.c
> index af7af0d..77f256e 100644
> --- a/drivers/media/i2c/ov13858.c
> +++ b/drivers/media/i2c/ov13858.c
> @@ -104,7 +104,6 @@ struct ov13858_reg_list {
>  
>  /* Link frequency config */
>  struct ov13858_link_freq_config {
> - u32 pixel_rate;
>   u32 pixels_per_line;
>  
>   /* PLL registers for this link frequency */
> @@ -958,8 +957,6 @@ struct ov13858_mode {
>  static const struct ov13858_link_freq_config
>   link_freq_configs[OV13858_NUM_OF_LINK_FREQS] = {
>   {
> - /* pixel_rate = link_freq * 2 * nr_of_lanes / bits_per_sample */
> - .pixel_rate = (OV13858_LINK_FREQ_540MHZ * 2 * 4) / 10,
>   .pixels_per_line = OV13858_PPL_540MHZ,
>   .reg_list = {
>   .num_of_regs = ARRAY_SIZE(mipi_data_rate_1080mbps),
> @@ -967,8 +964,6 @@ struct ov13858_mode {
>   }
>   },
>   {
> - /* pixel_rate = link_freq * 2 * nr_of_lanes / bits_per_sample */
> - .pixel_rate = (OV13858_LINK_FREQ_270MHZ * 2 * 4) / 10,
>   .pixels_per_line = OV13858_PPL_270MHZ,
>   .reg_list = {
>   .num_of_regs = ARRAY_SIZE(mipi_data_rate_540mbps),
> @@ -1385,6 +1380,7 @@ static int ov13858_get_pad_format(struct v4l2_subdev 
> *sd,
>   s32 vblank_def;
>   s32 vblank_min;
>   s64 h_blank;
> + s64 pixel_rate;
>  
>   mutex_lock(>mutex);
>  
> @@ -1400,9 +1396,9 @@ static int ov13858_get_pad_format(struct v4l2_subdev 
> *sd,
>   } else {
>   ov13858->cur_mode = mode;
>   __v4l2_ctrl_s_ctrl(ov13858->link_freq, mode->link_freq_index);
> - __v4l2_ctrl_s_ctrl_int64(
> - ov13858->pixel_rate,
> - link_freq_configs[mode->link_freq_index].pixel_rate);
> + pixel_rate =
> + (link_freq_menu_items[mode->link_freq_index] * 2 * 4) / 10;

You should indent what falls on the next line, and add subsequent line
wraps if needed.

> + __v4l2_ctrl_s_ctrl_int64(ov13858->pixel_rate, pixel_rate);
>   /* Update limits and set FPS to default */
>   vblank_def = ov13858->cur_mode->vts_def -
>ov13858->cur_mode->height;
> @@ -1617,6 +1613,10 @@ static int ov13858_init_controls(struct ov13858 
> *ov13858)
>   s64 exposure_max;
>   s64 vblank_def;
>   s64 vblank_min;
> + s64 hblank;
> + s64 pixel_rate_min;
> + s64 pixel_rate_max;
> + const struct ov13858_mode *mode;
>   int ret;
>  
>   ctrl_hdlr = >ctrl_handler;
> @@ -1634,29 +1634,31 @@ static int ov13858_init_controls(struct ov13858 
> *ov13858)
>   link_freq_menu_items);
>   ov13858->link_freq->flags |= V4L2_CTRL_FLAG_READ_ONLY;
>  
> + /* pixel_rate = link_freq * 2 * nr_of_lanes / bits_per_sample */
> + pixel_rate_max = (link_freq_menu_items[0] * 2 * 4) / 10;
> + pixel_rate_min = (link_freq_menu_items[1] * 2 * 4) / 10;

You're using the same, some could say non-trivial, formula in three places.

Could you add a macro for it? E.g.

/* double data rate => 2; four lanes => four; 10 bits per pixel => 10 */
#define link_freq_to_pixel_rate(f) ((f) * 2 * 4 / 10)

>   /* By default, PIXEL_RATE is read only */
>   ov13858->pixel_rate = v4l2_ctrl_new_std(ctrl_hdlr, _ctrl_ops,
> - V4L2_CID_PIXEL_RATE, 0,
> - link_freq_configs[0].pixel_rate, 1,
> - link_freq_configs[0].pixel_rate);
> + V4L2_CID_PIXEL_RATE,
> + pixel_rate_min, pixel_rate_max,
> + 1, pixel_rate_max);
>  
> - vblank_def = ov13858->cur_mode->vts_def - ov13858->cur_mode->height;
> - vblank_min = ov13858->cur_mode->vts_min - ov13858->cur_mode->height;
> + mode = ov13858->cur_mode;
> + vblank_def = mode->vts_def - mode->height;
> + vblank_min = mode->vts_min - mode->height;
>   ov13858->vblank = v4l2_ctrl_new_std(
>   ctrl_hdlr, _ctrl_ops, V4L2_CID_VBLANK,
> - vblank_min,
> - OV13858_VTS_MAX 

Re: [PATCH] Staging: media: atomisp: Merge assignment with return

[Sakari Ailus]

On Tue, Sep 12, 2017 at 07:55:07PM +0530, Srishti Sharma wrote:
> Merge the assignment and the return statements to return the value
> directly. Done using the following semantic patch by coccinelle.
> 
> @@
> local idexpression ret;
> expression e;
> @@
> 
> -ret =
> +return
>  e;
> -return ret;
> 
> Signed-off-by: Srishti Sharma 

Hi Srishti,

I've merged the two patches as they're trivial and the commit message is
the same.

It's entirely reasonable to have a patch per driver but you should mention
that in the commit message (subject line) at least. This case is a bit
special because the other driver is also specific to the atomisp staging
driver.

Thanks.

-- 
Sakari Ailus
e-mail: sakari.ai...@iki.fi

Re: [RESEND PATCH v2 4/6] dt: bindings: as3645a: Improve label documentation, DT example

[Pavel Machek]

Hi!

> Specify the exact label used if the label property is omitted in DT, as
> well as use label in the example that conforms to LED device naming.
> 
> @@ -69,11 +73,11 @@ Example
>   flash-max-microamp = <32>;
>   led-max-microamp = <6>;
>   ams,input-max-microamp = <175>;
> - label = "as3645a:flash";
> + label = "as3645a:white:flash";
>   };
>   indicator@1 {
>   reg = <0x1>;
>   led-max-microamp = <1>;
> - label = "as3645a:indicator";
> + label = "as3645a:red:indicator";
>   };
>   };

Ok, but userspace still has no chance to determine if this is flash
from main camera or flash for front camera; todays smartphones have
flashes on both cameras.

So.. Can I suggset as3645a:white:main_camera_flash or main_flash or
?

Thanks,
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature

[PATCH] cec-ioc-dqevent.rst: fix typo

[Hans Verkuil]

The documentation talked about INITIAL_VALUE when the actual define is
INITIAL_STATE.

Fix this.

Signed-off-by: Hans Verkuil 
Reported-by: Jonas Karlman 
---
diff --git a/Documentation/media/uapi/cec/cec-ioc-dqevent.rst 
b/Documentation/media/uapi/cec/cec-ioc-dqevent.rst
index a5c821809cc6..4fe96e2adf4c 100644
--- a/Documentation/media/uapi/cec/cec-ioc-dqevent.rst
+++ b/Documentation/media/uapi/cec/cec-ioc-dqevent.rst
@@ -172,9 +172,9 @@ it is guaranteed that the state did change in between the 
two events.
 :stub-columns: 0
 :widths:   3 1 8

-* .. _`CEC-EVENT-FL-INITIAL-VALUE`:
+* .. _`CEC-EVENT-FL-INITIAL-STATE`:

-  - ``CEC_EVENT_FL_INITIAL_VALUE``
+  - ``CEC_EVENT_FL_INITIAL_STATE``
   - 1
   - Set for the initial events that are generated when the device is
opened. See the table above for which events do this. This allows

[RESEND PATCH v2 5/6] as3645a: Add colour to LED name

[Sakari Ailus]

Add the colour of the LED to the LED name, as specified in
Documentation/leds/leds-class.txt.

Signed-off-by: Sakari Ailus 
---
 drivers/leds/leds-as3645a.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/leds/leds-as3645a.c b/drivers/leds/leds-as3645a.c
index 605e0c64e974..edeb0a499f6c 100644
--- a/drivers/leds/leds-as3645a.c
+++ b/drivers/leds/leds-as3645a.c
@@ -528,7 +528,7 @@ static int as3645a_parse_node(struct as3645a *flash,
strlcpy(names->flash, name, sizeof(names->flash));
else
snprintf(names->flash, sizeof(names->flash),
-"%s:flash", node->name);
+"%s:white:flash", node->name);
 
rval = of_property_read_u32(flash->flash_node, "flash-timeout-us",
>flash_timeout_us);
@@ -572,7 +572,7 @@ static int as3645a_parse_node(struct as3645a *flash,
strlcpy(names->indicator, name, sizeof(names->indicator));
else
snprintf(names->indicator, sizeof(names->indicator),
-"%s:indicator", node->name);
+"%s:red:indicator", node->name);
 
rval = of_property_read_u32(flash->indicator_node, "led-max-microamp",
>indicator_max_ua);
-- 
2.11.0

[RESEND PATCH v2 6/6] as3645a: Unregister indicator LED on device unbind

[Sakari Ailus]

The indicator LED was registered in probe but was not removed in driver
remove callback. Fix this.

Signed-off-by: Sakari Ailus 
---
 drivers/leds/leds-as3645a.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/leds/leds-as3645a.c b/drivers/leds/leds-as3645a.c
index edeb0a499f6c..9494ba26c64b 100644
--- a/drivers/leds/leds-as3645a.c
+++ b/drivers/leds/leds-as3645a.c
@@ -743,6 +743,7 @@ static int as3645a_remove(struct i2c_client *client)
as3645a_set_control(flash, AS_MODE_EXT_TORCH, false);
 
v4l2_flash_release(flash->vf);
+   v4l2_flash_release(flash->vfind);
 
led_classdev_flash_unregister(>fled);
led_classdev_unregister(>iled_cdev);
-- 
2.11.0

[RESEND PATCH v2 0/6] AS3645A fixes

[Sakari Ailus]

Hi folks,

(Resending, fixed linux-media list address.)

Here are a few fixes for the as3645a DTS as well as changes in bindings.
The driver is not in a release yet. I'd like to get these in as through
the media tree fixes branch.

since v1:

- Add LED colour to the name of the LED, this adds two patches to the set.

- Add a patch to unregister the indicator LED in driver remove function.

- No changes to v1 patches.

Sakari Ailus (6):
  as3645a: Use ams,input-max-microamp as documented in DT bindings
  dt: bindings: as3645a: Use LED number to refer to LEDs
  as3645a: Use integer numbers for parsing LEDs
  dt: bindings: as3645a: Improve label documentation, DT example
  as3645a: Add colour to LED name
  as3645a: Unregister indicator LED on device unbind

 .../devicetree/bindings/leds/ams,as3645a.txt   | 40 ++
 arch/arm/boot/dts/omap3-n950-n9.dtsi   | 10 --
 drivers/leds/leds-as3645a.c| 33 +++---
 3 files changed, 61 insertions(+), 22 deletions(-)

-- 
2.11.0

[RESEND PATCH v2 2/6] dt: bindings: as3645a: Use LED number to refer to LEDs

[Sakari Ailus]

Use integers (reg property) to tell the number of the LED to the driver
instead of the node name. While both of these approaches are currently
used by the LED bindings, using integers will require less driver changes
for ACPI support. Additionally, it will make possible LED naming using
chip and LED node names, effectively making the label property most useful
for human-readable names only.

Signed-off-by: Sakari Ailus 
Acked-by: Jacek Anaszewski 
Acked-by: Rob Herring 
---
 .../devicetree/bindings/leds/ams,as3645a.txt   | 28 ++
 1 file changed, 18 insertions(+), 10 deletions(-)

diff --git a/Documentation/devicetree/bindings/leds/ams,as3645a.txt 
b/Documentation/devicetree/bindings/leds/ams,as3645a.txt
index 12c5ef26ec73..fdc40e354a64 100644
--- a/Documentation/devicetree/bindings/leds/ams,as3645a.txt
+++ b/Documentation/devicetree/bindings/leds/ams,as3645a.txt
@@ -15,11 +15,14 @@ Required properties
 
 compatible : Must be "ams,as3645a".
 reg: The I2C address of the device. Typically 0x30.
+#address-cells : 1
+#size-cells: 0
 
 
-Required properties of the "flash" child node
-=
+Required properties of the flash child node (0)
+===
 
+reg: 0
 flash-timeout-us: Flash timeout in microseconds. The value must be in
  the range [10, 85] and divisible by 5.
 flash-max-microamp: Maximum flash current in microamperes. Has to be
@@ -33,20 +36,21 @@ ams,input-max-microamp: Maximum flash controller input 
current. The
and divisible by 5.
 
 
-Optional properties of the "flash" child node
-=
+Optional properties of the flash child node
+===
 
 label  : The label of the flash LED.
 
 
-Required properties of the "indicator" child node
-=
+Required properties of the indicator child node (1)
+===
 
+reg: 1
 led-max-microamp: Maximum indicator current. The allowed values are
  2500, 5000, 7500 and 1.
 
-Optional properties of the "indicator" child node
-=
+Optional properties of the indicator child node
+===
 
 label  : The label of the indicator LED.
 
@@ -55,16 +59,20 @@ Example
 ===
 
as3645a@30 {
+   #address-cells: 1
+   #size-cells: 0
reg = <0x30>;
compatible = "ams,as3645a";
-   flash {
+   flash@0 {
+   reg = <0x0>;
flash-timeout-us = <15>;
flash-max-microamp = <32>;
led-max-microamp = <6>;
ams,input-max-microamp = <175>;
label = "as3645a:flash";
};
-   indicator {
+   indicator@1 {
+   reg = <0x1>;
led-max-microamp = <1>;
label = "as3645a:indicator";
};
-- 
2.11.0

[RESEND PATCH v2 1/6] as3645a: Use ams,input-max-microamp as documented in DT bindings

[Sakari Ailus]

DT bindings document the property "ams,input-max-microamp" that limits the
chip's maximum input current. The driver and the DTS however used
"peak-current-limit" property. Fix this by using the property documented
in DT binding documentation.

Signed-off-by: Sakari Ailus 
Acked-by: Pavel Machek 
Acked-by: Jacek Anaszewski 
---
 arch/arm/boot/dts/omap3-n950-n9.dtsi | 2 +-
 drivers/leds/leds-as3645a.c  | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/boot/dts/omap3-n950-n9.dtsi 
b/arch/arm/boot/dts/omap3-n950-n9.dtsi
index cb47ae79a5f9..b86fc83a5a65 100644
--- a/arch/arm/boot/dts/omap3-n950-n9.dtsi
+++ b/arch/arm/boot/dts/omap3-n950-n9.dtsi
@@ -273,7 +273,7 @@
flash-timeout-us = <15>;
flash-max-microamp = <32>;
led-max-microamp = <6>;
-   peak-current-limit = <175>;
+   ams,input-max-microamp = <175>;
};
indicator {
led-max-microamp = <1>;
diff --git a/drivers/leds/leds-as3645a.c b/drivers/leds/leds-as3645a.c
index e0898233..e3f89c6130d2 100644
--- a/drivers/leds/leds-as3645a.c
+++ b/drivers/leds/leds-as3645a.c
@@ -534,7 +534,7 @@ static int as3645a_parse_node(struct as3645a *flash,
of_property_read_u32(flash->flash_node, "voltage-reference",
 >voltage_reference);
 
-   of_property_read_u32(flash->flash_node, "peak-current-limit",
+   of_property_read_u32(flash->flash_node, "ams,input-max-microamp",
 >peak);
cfg->peak = AS_PEAK_mA_TO_REG(cfg->peak);
 
-- 
2.11.0

[RESEND PATCH v2 4/6] dt: bindings: as3645a: Improve label documentation, DT example

[Sakari Ailus]

Specify the exact label used if the label property is omitted in DT, as
well as use label in the example that conforms to LED device naming.

Signed-off-by: Sakari Ailus 
---
 Documentation/devicetree/bindings/leds/ams,as3645a.txt | 12 
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/leds/ams,as3645a.txt 
b/Documentation/devicetree/bindings/leds/ams,as3645a.txt
index fdc40e354a64..9adba41e74b3 100644
--- a/Documentation/devicetree/bindings/leds/ams,as3645a.txt
+++ b/Documentation/devicetree/bindings/leds/ams,as3645a.txt
@@ -39,7 +39,9 @@ ams,input-max-microamp: Maximum flash controller input 
current. The
 Optional properties of the flash child node
 ===
 
-label  : The label of the flash LED.
+label  : The label of the flash LED. The label is otherwise assumed to
+ be the device node name concatenated with ":white:" and the
+ name of the flash LED node is assumed if omitted.
 
 
 Required properties of the indicator child node (1)
@@ -52,7 +54,9 @@ led-max-microamp: Maximum indicator current. The allowed 
values are
 Optional properties of the indicator child node
 ===
 
-label  : The label of the indicator LED.
+label  : The label of the indicator LED. The label is otherwise assumed
+ to be the device node name concatenated with ":red:" and the
+ name of the indicator LED node is assumed if omitted.
 
 
 Example
@@ -69,11 +73,11 @@ Example
flash-max-microamp = <32>;
led-max-microamp = <6>;
ams,input-max-microamp = <175>;
-   label = "as3645a:flash";
+   label = "as3645a:white:flash";
};
indicator@1 {
reg = <0x1>;
led-max-microamp = <1>;
-   label = "as3645a:indicator";
+   label = "as3645a:red:indicator";
};
};
-- 
2.11.0

[RESEND PATCH v2 3/6] as3645a: Use integer numbers for parsing LEDs

[Sakari Ailus]

Use integer numbers for LEDs, 0 is the flash and 1 is the indicator.

Signed-off-by: Sakari Ailus 
Acked-by: Pavel Machek 
Acked-by: Jacek Anaszewski 
---
 arch/arm/boot/dts/omap3-n950-n9.dtsi |  8 ++--
 drivers/leds/leds-as3645a.c  | 26 --
 2 files changed, 30 insertions(+), 4 deletions(-)

diff --git a/arch/arm/boot/dts/omap3-n950-n9.dtsi 
b/arch/arm/boot/dts/omap3-n950-n9.dtsi
index b86fc83a5a65..1b0bd72945f2 100644
--- a/arch/arm/boot/dts/omap3-n950-n9.dtsi
+++ b/arch/arm/boot/dts/omap3-n950-n9.dtsi
@@ -267,15 +267,19 @@
clock-frequency = <40>;
 
as3645a@30 {
+   #address-cells = <1>;
+   #size-cells = <0>;
reg = <0x30>;
compatible = "ams,as3645a";
-   flash {
+   flash@0 {
+   reg = <0x0>;
flash-timeout-us = <15>;
flash-max-microamp = <32>;
led-max-microamp = <6>;
ams,input-max-microamp = <175>;
};
-   indicator {
+   indicator@1 {
+   reg = <0x1>;
led-max-microamp = <1>;
};
};
diff --git a/drivers/leds/leds-as3645a.c b/drivers/leds/leds-as3645a.c
index e3f89c6130d2..605e0c64e974 100644
--- a/drivers/leds/leds-as3645a.c
+++ b/drivers/leds/leds-as3645a.c
@@ -112,6 +112,10 @@
 #define AS_PEAK_mA_TO_REG(a) \
((min_t(u32, AS_PEAK_mA_MAX, a) - 1250) / 250)
 
+/* LED numbers for Devicetree */
+#define AS_LED_FLASH   0
+#define AS_LED_INDICATOR   1
+
 enum as_mode {
AS_MODE_EXT_TORCH = 0 << AS_CONTROL_MODE_SETTING_SHIFT,
AS_MODE_INDICATOR = 1 << AS_CONTROL_MODE_SETTING_SHIFT,
@@ -491,10 +495,29 @@ static int as3645a_parse_node(struct as3645a *flash,
  struct device_node *node)
 {
struct as3645a_config *cfg = >cfg;
+   struct device_node *child;
const char *name;
int rval;
 
-   flash->flash_node = of_get_child_by_name(node, "flash");
+   for_each_child_of_node(node, child) {
+   u32 id = 0;
+
+   of_property_read_u32(child, "reg", );
+
+   switch (id) {
+   case AS_LED_FLASH:
+   flash->flash_node = of_node_get(child);
+   break;
+   case AS_LED_INDICATOR:
+   flash->indicator_node = of_node_get(child);
+   break;
+   default:
+   dev_warn(>client->dev,
+"unknown LED %u encountered, ignoring\n", id);
+   break;
+   }
+   }
+
if (!flash->flash_node) {
dev_err(>client->dev, "can't find flash node\n");
return -ENODEV;
@@ -538,7 +561,6 @@ static int as3645a_parse_node(struct as3645a *flash,
 >peak);
cfg->peak = AS_PEAK_mA_TO_REG(cfg->peak);
 
-   flash->indicator_node = of_get_child_by_name(node, "indicator");
if (!flash->indicator_node) {
dev_warn(>client->dev,
 "can't find indicator node\n");
-- 
2.11.0

Re: as3645a flash userland interface

[Sakari Ailus]

Hi Jacek,

On Tue, Sep 12, 2017 at 08:53:33PM +0200, Jacek Anaszewski wrote:
> Hi Pavel,
> 
> On 09/12/2017 12:36 PM, Pavel Machek wrote:
> > Hi!
> > 
> > There were some changes to as3645a flash controller. Before we have
> > stable interface we have to keep forever I want to ask:
> 
> Note that we have already two LED flash class drivers - leds-max77693
> and leds-aat1290. They have been present in mainline for over two years
> now.
> 
> > What directory are the flash controls in?
> > 
> > /sys/class/leds/led-controller:flash ?
> > 
> > Could we arrange for something less generic, like
> > 
> > /sys/class/leds/main-camera:flash ?
> 
> I'd rather avoid overcomplicating this. LED class device name pattern
> is well defined to devicename:colour:function
> (see Documentation/leds/leds-class.txt, "LED Device Naming" section).
> 
> In this case "flash" in place of the "function" segment makes the
> things clear enough I suppose.

Oh. I have to admit I've completely missed this. :-o

I'll address this in v2 of the as3645a fixes, plus submit related V4L2
flash class documentation fixes a little later.

-- 
Regards,

Sakari Ailus
e-mail: sakari.ai...@iki.fi

[PATCH v4] [DVB][FRONTEND] Added a new ioctl for optimizing frontend property set operation

[Satendra Singh Thakur]

Hello  Mr Chehab,
It seems that there is a mismatch among tab spacing
in local patch on my PC, the patch in email
and the patch in lkml site.
This is causing alignment problem. Even if I fix alignment problem
in my PC, alignment is different in lkml and email.
Anyway, I have run checkpatch and got 0 err and warnings
Please review it.
Thanks for the patience.

Signed-off-by: Satendra Singh Thakur 
---
 Documentation/media/uapi/dvb/fe-get-property.rst |  24 ++-
 drivers/media/dvb-core/dvb_frontend.c| 191 ++-
 include/uapi/linux/dvb/frontend.h|  24 +++
 3 files changed, 158 insertions(+), 81 deletions(-)

diff --git a/Documentation/media/uapi/dvb/fe-get-property.rst 
b/Documentation/media/uapi/dvb/fe-get-property.rst
index 015d4db..b63a588 100644
--- a/Documentation/media/uapi/dvb/fe-get-property.rst
+++ b/Documentation/media/uapi/dvb/fe-get-property.rst
@@ -2,14 +2,14 @@
 
 .. _FE_GET_PROPERTY:
 
-**
-ioctl FE_SET_PROPERTY, FE_GET_PROPERTY
-**
+**
+ioctl FE_SET_PROPERTY, FE_GET_PROPERTY, FE_SET_PROPERTY_SHORT
+**
 
 Name
 
 
-FE_SET_PROPERTY - FE_GET_PROPERTY - FE_SET_PROPERTY sets one or more frontend 
properties. - FE_GET_PROPERTY returns one or more frontend properties.
+FE_SET_PROPERTY and FE_SET_PROPERTY_SHORT set one or more frontend properties. 
FE_GET_PROPERTY returns one or more frontend properties.
 
 
 Synopsis
@@ -21,6 +21,8 @@ Synopsis
 .. c:function:: int ioctl( int fd, FE_SET_PROPERTY, struct dtv_properties 
*argp )
 :name: FE_SET_PROPERTY
 
+.. c:function:: int ioctl( int fd, FE_SET_PROPERTY_SHORT, struct 
dtv_properties_short *argp )
+:name: FE_SET_PROPERTY_SHORT
 
 Arguments
 =
@@ -29,15 +31,16 @@ Arguments
 File descriptor returned by :ref:`open() `.
 
 ``argp``
-pointer to struct :c:type:`dtv_properties`
+Pointer to struct :c:type:`dtv_properties` or
+   struct :c:type:`dtv_properties_short`.
 
 
 Description
 ===
 
-All DVB frontend devices support the ``FE_SET_PROPERTY`` and
-``FE_GET_PROPERTY`` ioctls. The supported properties and statistics
-depends on the delivery system and on the device:
+All DVB frontend devices support the ``FE_SET_PROPERTY``, ``FE_GET_PROPERTY``
+and ``FE_SET_PROPERTY_SHORT`` ioctls. The supported  properties and
+statistics depends on the delivery system and on the device:
 
 -  ``FE_SET_PROPERTY:``
 
@@ -60,6 +63,11 @@ depends on the delivery system and on the device:
 
-  This call only requires read-only access to the device.
 
+-  ``FE_SET_PROPERTY_SHORT:``
+
+   -  This ioctl is similar to FE_SET_PROPERTY ioctl mentioned above
+  except that the arguments of the former utilize a struct 
:c:type:`dtv_property_short`
+  which is smaller in size.
 
 Return Value
 
diff --git a/drivers/media/dvb-core/dvb_frontend.c 
b/drivers/media/dvb-core/dvb_frontend.c
index e3fff8f..7c96197 100644
--- a/drivers/media/dvb-core/dvb_frontend.c
+++ b/drivers/media/dvb-core/dvb_frontend.c
@@ -1081,28 +1081,25 @@ static struct dtv_cmds_h dtv_cmds[DTV_MAX_COMMAND + 1] 
= {
_DTV_CMD(DTV_STAT_TOTAL_BLOCK_COUNT, 0, 0),
 };
 
-static void dtv_property_dump(struct dvb_frontend *fe,
- bool is_set,
+static void dtv_get_property_dump(struct dvb_frontend *fe,
  struct dtv_property *tvp)
 {
int i;
 
if (tvp->cmd <= 0 || tvp->cmd > DTV_MAX_COMMAND) {
-   dev_warn(fe->dvb->device, "%s: %s tvp.cmd = 0x%08x undefined\n",
-   __func__,
-   is_set ? "SET" : "GET",
-   tvp->cmd);
+   dev_warn(fe->dvb->device, "%s: GET tvp.cmd = 0x%08x undefined\n"
+, __func__,
+tvp->cmd);
return;
}
 
-   dev_dbg(fe->dvb->device, "%s: %s tvp.cmd= 0x%08x (%s)\n", __func__,
-   is_set ? "SET" : "GET",
-   tvp->cmd,
-   dtv_cmds[tvp->cmd].name);
+   dev_dbg(fe->dvb->device, "%s: GET tvp.cmd= 0x%08x (%s)\n", __func__,
+   tvp->cmd,
+   dtv_cmds[tvp->cmd].name);
 
if (dtv_cmds[tvp->cmd].buffer) {
dev_dbg(fe->dvb->device, "%s: tvp.u.buffer.len = 0x%02x\n",
-   __func__, tvp->u.buffer.len);
+   __func__, tvp->u.buffer.len);
 
for(i = 0; i < tvp->u.buffer.len; i++)
dev_dbg(fe->dvb->device,
@@ -1515,7 +1512,7 @@ static int dtv_property_process_get(struct dvb_frontend 
*fe,
return r;
}
 
-   dtv_property_dump(fe, false, tvp);
+   dtv_get_property_dump(fe, tvp);
 
return 0;
 }
@@ 

Re: [PATCH v4 3/3] media: ov7670: Add the s_power operation

[Yang, Wenyou]

Hi Sakari,


On 2017/9/18 15:35, Sakari Ailus wrote:

Hi Wenyou,

Thanks for the update.

The driver exposes controls that are accessible through the sub-device node
even if the device hasn't been powered on.

Many ISP and bridge drivers will also power the sensor down once the last
user of the user space device nodes disappears. You could keep the device
powered at all times or change the behaviour so that controls can be
accessed when the power is off.

The best option would be to convert the driver to use runtime PM.

Yes, I agree with you.
I also noticed there are a lot of things needed to improve except for 
it, such as the platform data via device tree.

I would like do it in another patch set. I will continue to work on it.

An example of this can be found in drivers/media/i2c/ov13858.c .

A good example, thank you for your providing.


On Mon, Sep 18, 2017 at 02:45:14PM +0800, Wenyou Yang wrote:

Add the s_power operation which is responsible for manipulating the
power dowm mode through the PWDN pin and the reset operation through
the RESET pin.

Signed-off-by: Wenyou Yang 
---

Changes in v4: None
Changes in v3: None
Changes in v2:
  - Add the patch to support the get_fmt ops.
  - Remove the redundant invoking ov7670_init_gpio().

  drivers/media/i2c/ov7670.c | 32 +++-
  1 file changed, 27 insertions(+), 5 deletions(-)

diff --git a/drivers/media/i2c/ov7670.c b/drivers/media/i2c/ov7670.c
index 456f48057605..304abc769a67 100644
--- a/drivers/media/i2c/ov7670.c
+++ b/drivers/media/i2c/ov7670.c
@@ -1542,6 +1542,22 @@ static int ov7670_s_register(struct v4l2_subdev *sd, 
const struct v4l2_dbg_regis
  }
  #endif
  
+static int ov7670_s_power(struct v4l2_subdev *sd, int on)

+{
+   struct ov7670_info *info = to_state(sd);
+
+   if (info->pwdn_gpio)
+   gpiod_direction_output(info->pwdn_gpio, !on);
+   if (on && info->resetb_gpio) {
+   gpiod_set_value(info->resetb_gpio, 1);
+   usleep_range(500, 1000);
+   gpiod_set_value(info->resetb_gpio, 0);
+   usleep_range(3000, 5000);
+   }
+
+   return 0;
+}
+
  static void ov7670_get_default_format(struct v4l2_subdev *sd,
  struct v4l2_mbus_framefmt *format)
  {
@@ -1575,6 +1591,7 @@ static const struct v4l2_subdev_core_ops ov7670_core_ops 
= {
.g_register = ov7670_g_register,
.s_register = ov7670_s_register,
  #endif
+   .s_power = ov7670_s_power,
  };
  
  static const struct v4l2_subdev_video_ops ov7670_video_ops = {

@@ -1692,23 +1709,25 @@ static int ov7670_probe(struct i2c_client *client,
if (ret)
return ret;
  
-	ret = ov7670_init_gpio(client, info);

-   if (ret)
-   goto clk_disable;
-
info->clock_speed = clk_get_rate(info->clk) / 100;
if (info->clock_speed < 10 || info->clock_speed > 48) {
ret = -EINVAL;
goto clk_disable;
}
  
+	ret = ov7670_init_gpio(client, info);

+   if (ret)
+   goto clk_disable;
+
+   ov7670_s_power(sd, 1);
+
/* Make sure it's an ov7670 */
ret = ov7670_detect(sd);
if (ret) {
v4l_dbg(1, debug, client,
"chip found @ 0x%x (%s) is not an ov7670 chip.\n",
client->addr << 1, client->adapter->name);
-   goto clk_disable;
+   goto power_off;
}
v4l_info(client, "chip found @ 0x%02x (%s)\n",
client->addr << 1, client->adapter->name);
@@ -1787,6 +1806,8 @@ static int ov7670_probe(struct i2c_client *client,
  #endif
  hdl_free:
v4l2_ctrl_handler_free(>hdl);
+power_off:
+   ov7670_s_power(sd, 0);
  clk_disable:
clk_disable_unprepare(info->clk);
return ret;
@@ -1804,6 +1825,7 @@ static int ov7670_remove(struct i2c_client *client)
  #if defined(CONFIG_MEDIA_CONTROLLER)
media_entity_cleanup(>sd.entity);
  #endif
+   ov7670_s_power(sd, 0);
return 0;
  }
  
--

2.13.0



Best Regards,
Wenyou Yang

Re: [PATCH v4 2/3] media: ov7670: Add the get_fmt callback

[Yang, Wenyou]

Hi Sakari,


On 2017/9/18 15:36, Sakari Ailus wrote:

Hi Wenyou,

On Mon, Sep 18, 2017 at 02:45:13PM +0800, Wenyou Yang wrote:

@@ -998,8 +1002,15 @@ static int ov7670_set_fmt(struct v4l2_subdev *sd,
ret = ov7670_try_fmt_internal(sd, >format, NULL, NULL);
if (ret)
return ret;
-   cfg->try_fmt = format->format;
+#ifdef CONFIG_VIDEO_V4L2_SUBDEV_API
+   struct v4l2_mbus_framefmt *mbus_fmt;

This will emit a compiler warning at least.

Thank you for your review.
Will be fixed in next version.



+
+   mbus_fmt = v4l2_subdev_get_try_format(sd, cfg, format->pad);
+   *mbus_fmt = format->format;
return 0;
+#else
+   return -ENOTTY;
+#endif
}
  
  	ret = ov7670_try_fmt_internal(sd, >format, , );


Best Regards,
Wenyou Yang

[PATCH] cec-core.rst/cec-ioc-receive.rst: clarify CEC_TX_STATUS_ERROR

[Hans Verkuil]

CEC_TX_STATUS_ERROR can be used if the HW cannot tell LOST_ARB and LOW_DRIVE
apart, or when some other error occurs. It is not a replacement for NACK.

So the hardware must be able to tell the difference between OK, NACK and 
'something
else'.

Clarify the documentation (both public and kernel API) on this point.

Also fix two small typos (this messages -> this message).

Signed-off-by: Hans Verkuil 
---
diff --git a/Documentation/media/kapi/cec-core.rst 
b/Documentation/media/kapi/cec-core.rst
index 28866259998c..d37e107f2fde 100644
--- a/Documentation/media/kapi/cec-core.rst
+++ b/Documentation/media/kapi/cec-core.rst
@@ -227,8 +227,8 @@ CEC_TX_STATUS_LOW_DRIVE:
retransmission.

 CEC_TX_STATUS_ERROR:
-   some unspecified error occurred: this can be one of
-   the previous two if the hardware cannot differentiate or something
+   some unspecified error occurred: this can be one of ARB_LOST
+   or LOW_DRIVE if the hardware cannot differentiate or something
else entirely.

 CEC_TX_STATUS_MAX_RETRIES:
@@ -238,6 +238,9 @@ CEC_TX_STATUS_MAX_RETRIES:
doesn't have to make another attempt to transmit the message
since the hardware did that already.

+The hardware must be able to differentiate between OK, NACK and 'something
+else'.
+
 The \*_cnt arguments are the number of error conditions that were seen.
 This may be 0 if no information is available. Drivers that do not support
 hardware retry can just set the counter corresponding to the transmit error
diff --git a/Documentation/media/uapi/cec/cec-ioc-receive.rst 
b/Documentation/media/uapi/cec/cec-ioc-receive.rst
index 0f397c535a4c..bdad4b197bcd 100644
--- a/Documentation/media/uapi/cec/cec-ioc-receive.rst
+++ b/Documentation/media/uapi/cec/cec-ioc-receive.rst
@@ -131,7 +131,7 @@ View On' messages from initiator 0xf ('Unregistered') to 
destination 0 ('TV').
   - ``tx_status``
   - The status bits of the transmitted message. See
:ref:`cec-tx-status` for the possible status values. It is 0 if
-   this messages was received, not transmitted.
+   this message was received, not transmitted.
 * - __u8
   - ``msg[16]``
   - The message payload. For :ref:`ioctl CEC_TRANSMIT ` this 
is filled in by the
@@ -168,7 +168,7 @@ View On' messages from initiator 0xf ('Unregistered') to 
destination 0 ('TV').
   - ``tx_status``
   - The status bits of the transmitted message. See
:ref:`cec-tx-status` for the possible status values. It is 0 if
-   this messages was received, not transmitted.
+   this message was received, not transmitted.
 * - __u8
   - ``tx_arb_lost_cnt``
   - A counter of the number of transmit attempts that resulted in the
@@ -256,9 +256,9 @@ View On' messages from initiator 0xf ('Unregistered') to 
destination 0 ('TV').
   - ``CEC_TX_STATUS_ERROR``
   - 0x10
   - Some error occurred. This is used for any errors that do not fit
-   the previous two, either because the hardware could not tell which
-   error occurred, or because the hardware tested for other
-   conditions besides those two.
+   ``CEC_TX_STATUS_ARB_LOST`` or ``CEC_TX_STATUS_LOW_DRIVE``, either 
because
+   the hardware could not tell which error occurred, or because the 
hardware
+   tested for other conditions besides those two.
 * .. _`CEC-TX-STATUS-MAX-RETRIES`:

   - ``CEC_TX_STATUS_MAX_RETRIES``

Re: [PATCH v5 2/2] media:imx274 V4l2 driver for Sony imx274 CMOS sensor

[Sakari Ailus]

Hi Leon,

Thanks for the update. Please see my comments below.

On Fri, Sep 15, 2017 at 03:49:52PM +0800, Leon Luo wrote:
> The imx274 is a Sony CMOS image sensor that has 1/2.5 image size.
> It supports up to 3840x2160 (4K) 60fps, 1080p 120fps. The interface
> is 4-lane MIPI CSI-2 running at 1.44Gbps each.
> 
> This driver has been tested on Xilinx ZCU102 platform with a Leopard
> LI-IMX274MIPI-FMC camera board.
> 
> Support for the following features:
> -Resolutions: 3840x2160, 1920x1080, 1280x720
> -Frame rate: 3840x2160 : 5 – 60fps
> 1920x1080 : 5 – 120fps
> 1280x720 : 5 – 120fps
> -Exposure time: 16 – (frame interval) micro-seconds
> -Gain: 1x - 180x
> -VFLIP: enable/disabledrivers/media/i2c/imx274.c
> -Test pattern: 12 test patterns
> 
> Signed-off-by: Leon Luo 
> Tested-by: Sören Brinkmann 
> ---
> v5:
>  - no changes
> v4:
>  - use 32-bit data type to avoid __divdi3 compile error for i386
>  - clean up OR together error codes
> v3:
>  - clean up header files
>  - use struct directly instead of alias #define
>  - use v4l2_ctrl_s_ctrl instead of v4l2_s_ctrl
>  - revise debug output
>  - move static helpers closer to their call site
>  - don't OR toegether error codes
>  - use closest valid gain value instead of erroring out
>  - assigne lock to the control handler and omit explicit locking
>  - acquire mutex lock for imx274_get_fmt
>  - remove format->pad check in imx274_set_fmt since the pad is always 0
>  - pass struct v4l2_ctrl pointer in gain/exposure/vlip/test pattern controls
>  - remove priv->ctrls.vflip->val = val in imx274_set_vflip()
>  - remove priv->ctrls.test_pattern->val = val in imx274_set_test_pattern()
>  - remove empty open/close callbacks
>  - remove empty core ops
>  - add more error labels in probe
>  - use v4l2_async_unregister_subdev instead of v4l2_device_unregister_subdev
>  - use dynamic debug
>  - split start_stream to two steps: imx274_mode_regs() and 
> imx274_start_stream()
>frame rate & exposure can be updated
>between imx274_mode_regs() & imx274_start_stream()
> 
> v2:
>  - Fix Kconfig to not remove existing options
> ---
>  drivers/media/i2c/Kconfig  |7 +
>  drivers/media/i2c/Makefile |1 +
>  drivers/media/i2c/imx274.c | 1808 
> 
>  3 files changed, 1816 insertions(+)
>  create mode 100644 drivers/media/i2c/imx274.c
> 
> diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
> index 94153895fcd4..ad2e70a02363 100644
> --- a/drivers/media/i2c/Kconfig
> +++ b/drivers/media/i2c/Kconfig
> @@ -547,6 +547,13 @@ config VIDEO_APTINA_PLL
>  config VIDEO_SMIAPP_PLL
>   tristate
>  
> +config VIDEO_IMX274
> + tristate "Sony IMX274 sensor support"
> + depends on I2C && VIDEO_V4L2 && VIDEO_V4L2_SUBDEV_API
> + ---help---
> +   This is a V4L2 sensor-level driver for the Sony IMX274
> +   CMOS image sensor.
> +
>  config VIDEO_OV2640
>   tristate "OmniVision OV2640 sensor support"
>   depends on VIDEO_V4L2 && I2C
> diff --git a/drivers/media/i2c/Makefile b/drivers/media/i2c/Makefile
> index c843c181dfb9..f8d57e453936 100644
> --- a/drivers/media/i2c/Makefile
> +++ b/drivers/media/i2c/Makefile
> @@ -92,5 +92,6 @@ obj-$(CONFIG_VIDEO_IR_I2C)  += ir-kbd-i2c.o
>  obj-$(CONFIG_VIDEO_ML86V7667)+= ml86v7667.o
>  obj-$(CONFIG_VIDEO_OV2659)   += ov2659.o
>  obj-$(CONFIG_VIDEO_TC358743) += tc358743.o
> +obj-$(CONFIG_VIDEO_IMX274)   += imx274.o
>  
>  obj-$(CONFIG_SDR_MAX2175) += max2175.o
> diff --git a/drivers/media/i2c/imx274.c b/drivers/media/i2c/imx274.c
> new file mode 100644
> index ..66cbfa81c030
> --- /dev/null
> +++ b/drivers/media/i2c/imx274.c
> @@ -0,0 +1,1808 @@
> +/*
> + * imx274.c - IMX274 CMOS Image Sensor driver
> + *
> + * Copyright (C) 2017, Leopard Imaging, Inc.
> + *
> + * Leon Luo 
> + * Edwin Zou 
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> + * more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program.  If not, see .
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +#include 
> +#include 

Alphabetical order, please.

> +#include 

Do you need this header?

> +
> +/*
> + * See "SHR, SVR Setting" in datasheet
> + */
> +#define IMX274_DEFAULT_FRAME_LENGTH  (4550)
> +#define 

[PATCH 2/2] dt: bindings: media: Document data lane numbering without lane reordering

[Sakari Ailus]

Most devices do not support lane reordering and in many cases the
documentation of the data-lanes property is incomplete for such devices.
Document that in case the lane reordering isn't supported, monotonically
incremented values from 0 or 1 shall be used.

Signed-off-by: Sakari Ailus 
---
 Documentation/devicetree/bindings/media/video-interfaces.txt | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/media/video-interfaces.txt 
b/Documentation/devicetree/bindings/media/video-interfaces.txt
index 4e0527d..fc1964e 100644
--- a/Documentation/devicetree/bindings/media/video-interfaces.txt
+++ b/Documentation/devicetree/bindings/media/video-interfaces.txt
@@ -111,7 +111,10 @@ Optional endpoint properties
   determines the logical lane number, while the value of an entry indicates
   physical lane, e.g. for 2-lane MIPI CSI-2 bus we could have
   "data-lanes = <1 2>;", assuming the clock lane is on hardware lane 0.
-  This property is valid for serial busses only (e.g. MIPI CSI-2).
+  If the hardware does not support lane reordering, monotonically
+  incremented values shall be used from 0 or 1 onwards, depending on
+  whether or not there is also a clock lane. This property is valid for
+  serial busses only (e.g. MIPI CSI-2).
 - clock-lanes: an array of physical clock lane indexes. Position of an entry
   determines the logical lane number, while the value of an entry indicates
   physical lane, e.g. for a MIPI CSI-2 bus we could have "clock-lanes = <0>;",
-- 
2.7.4

[PATCH 1/2] dt: bindings: media: Document port and endpoint numbering

[Sakari Ailus]

A lot of devices do not need and do not document port or endpoint
numbering at all, e.g. in case where there's just a single port and a
single endpoint. Whereas this is just common sense, document it to make it
explicit.

Signed-off-by: Sakari Ailus 
---
 Documentation/devicetree/bindings/media/video-interfaces.txt | 12 
 1 file changed, 12 insertions(+)

diff --git a/Documentation/devicetree/bindings/media/video-interfaces.txt 
b/Documentation/devicetree/bindings/media/video-interfaces.txt
index 852041a..4e0527d 100644
--- a/Documentation/devicetree/bindings/media/video-interfaces.txt
+++ b/Documentation/devicetree/bindings/media/video-interfaces.txt
@@ -55,6 +55,18 @@ divided into two separate ITU-R BT.656 8-bit busses.  In 
such case bus-width
 and data-shift properties can be used to assign physical data lines to each
 endpoint node (logical bus).
 
+Port and endpoint numbering
+---
+
+While the port and endpoint numbers are ultimately specific to a device,
+most devices have more limited scope than what the interface allows. They
+may, for instance, only support a single endpoint on a port. Or there may
+be only a single port on a device.
+
+Therefore, if ports are not explicitly documented for a device, only port
+number zero shall be used. The same applies to endpoints: if endpoint
+numbers are not explicitly documented, only endpoint number zero shall be
+used.
 
 Required properties
 ---
-- 
2.7.4

[PATCH 0/2] Improve generic DT binding documentation for media devices

[Sakari Ailus]

Hi folks,

This set improves the DT binding documentation for media devices where
device specific documentation is lacking:

- Port and endpoint numbering
- lane numbering for the data-lanes property

Sakari Ailus (2):
  dt: bindings: media: Document port and endpoint numbering
  dt: bindings: media: Document data lane numbering without lane
reordering

 .../devicetree/bindings/media/video-interfaces.txt  | 17 -
 1 file changed, 16 insertions(+), 1 deletion(-)

-- 
2.7.4

[PATCH v3] [DVB][FRONTEND] Added a new ioctl for optimizing frontend property set operation

[Satendra Singh Thakur]

Hello  Mr Chehab,
Thanks for the review.
I have modified the code and documentation as per your comments.
Please let me know if this patch is ready for merging.

Signed-off-by: Satendra Singh Thakur 
---
 Documentation/media/uapi/dvb/fe-get-property.rst |  24 ++-
 drivers/media/dvb-core/dvb_frontend.c| 191 ++-
 include/uapi/linux/dvb/frontend.h|  24 +++
 3 files changed, 158 insertions(+), 81 deletions(-)

diff --git a/Documentation/media/uapi/dvb/fe-get-property.rst 
b/Documentation/media/uapi/dvb/fe-get-property.rst
index 015d4db..b63a588 100644
--- a/Documentation/media/uapi/dvb/fe-get-property.rst
+++ b/Documentation/media/uapi/dvb/fe-get-property.rst
@@ -2,14 +2,14 @@
 
 .. _FE_GET_PROPERTY:
 
-**
-ioctl FE_SET_PROPERTY, FE_GET_PROPERTY
-**
+**
+ioctl FE_SET_PROPERTY, FE_GET_PROPERTY, FE_SET_PROPERTY_SHORT
+**
 
 Name
 
 
-FE_SET_PROPERTY - FE_GET_PROPERTY - FE_SET_PROPERTY sets one or more frontend 
properties. - FE_GET_PROPERTY returns one or more frontend properties.
+FE_SET_PROPERTY and FE_SET_PROPERTY_SHORT set one or more frontend properties. 
FE_GET_PROPERTY returns one or more frontend properties.
 
 
 Synopsis
@@ -21,6 +21,8 @@ Synopsis
 .. c:function:: int ioctl( int fd, FE_SET_PROPERTY, struct dtv_properties 
*argp )
 :name: FE_SET_PROPERTY
 
+.. c:function:: int ioctl( int fd, FE_SET_PROPERTY_SHORT, struct 
dtv_properties_short *argp )
+:name: FE_SET_PROPERTY_SHORT
 
 Arguments
 =
@@ -29,15 +31,16 @@ Arguments
 File descriptor returned by :ref:`open() `.
 
 ``argp``
-pointer to struct :c:type:`dtv_properties`
+Pointer to struct :c:type:`dtv_properties` or
+   struct :c:type:`dtv_properties_short`.
 
 
 Description
 ===
 
-All DVB frontend devices support the ``FE_SET_PROPERTY`` and
-``FE_GET_PROPERTY`` ioctls. The supported properties and statistics
-depends on the delivery system and on the device:
+All DVB frontend devices support the ``FE_SET_PROPERTY``, ``FE_GET_PROPERTY``
+and ``FE_SET_PROPERTY_SHORT`` ioctls. The supported  properties and
+statistics depends on the delivery system and on the device:
 
 -  ``FE_SET_PROPERTY:``
 
@@ -60,6 +63,11 @@ depends on the delivery system and on the device:
 
-  This call only requires read-only access to the device.
 
+-  ``FE_SET_PROPERTY_SHORT:``
+
+   -  This ioctl is similar to FE_SET_PROPERTY ioctl mentioned above
+  except that the arguments of the former utilize a struct 
:c:type:`dtv_property_short`
+  which is smaller in size.
 
 Return Value
 
diff --git a/drivers/media/dvb-core/dvb_frontend.c 
b/drivers/media/dvb-core/dvb_frontend.c
index e3fff8f..3e49127 100644
--- a/drivers/media/dvb-core/dvb_frontend.c
+++ b/drivers/media/dvb-core/dvb_frontend.c
@@ -1081,28 +1081,25 @@ static struct dtv_cmds_h dtv_cmds[DTV_MAX_COMMAND + 1] 
= {
_DTV_CMD(DTV_STAT_TOTAL_BLOCK_COUNT, 0, 0),
 };
 
-static void dtv_property_dump(struct dvb_frontend *fe,
- bool is_set,
+static void dtv_get_property_dump(struct dvb_frontend *fe,
  struct dtv_property *tvp)
 {
int i;
 
if (tvp->cmd <= 0 || tvp->cmd > DTV_MAX_COMMAND) {
-   dev_warn(fe->dvb->device, "%s: %s tvp.cmd = 0x%08x undefined\n",
-   __func__,
-   is_set ? "SET" : "GET",
-   tvp->cmd);
+   dev_warn(fe->dvb->device, "%s: GET tvp.cmd = 0x%08x undefined\n"
+, __func__,
+tvp->cmd);
return;
}
 
-   dev_dbg(fe->dvb->device, "%s: %s tvp.cmd= 0x%08x (%s)\n", __func__,
-   is_set ? "SET" : "GET",
-   tvp->cmd,
-   dtv_cmds[tvp->cmd].name);
+   dev_dbg(fe->dvb->device, "%s: GET tvp.cmd= 0x%08x (%s)\n", __func__,
+   tvp->cmd,
+   dtv_cmds[tvp->cmd].name);
 
if (dtv_cmds[tvp->cmd].buffer) {
dev_dbg(fe->dvb->device, "%s: tvp.u.buffer.len = 0x%02x\n",
-   __func__, tvp->u.buffer.len);
+   __func__, tvp->u.buffer.len);
 
for(i = 0; i < tvp->u.buffer.len; i++)
dev_dbg(fe->dvb->device,
@@ -1515,7 +1512,7 @@ static int dtv_property_process_get(struct dvb_frontend 
*fe,
return r;
}
 
-   dtv_property_dump(fe, false, tvp);
+   dtv_get_property_dump(fe, tvp);
 
return 0;
 }
@@ -1738,23 +1735,35 @@ static int dvbv3_set_delivery_system(struct 
dvb_frontend *fe)
return emulate_delivery_system(fe, delsys);
 }
 
+/**
+ * dtv_property_process_set -  Sets a single DTV 

[PATCH 2/2] [media] dvb_usb_core: Improve a size determination in two functions

[SF Markus Elfring]

From: Markus Elfring 
Date: Mon, 18 Sep 2017 09:36:33 +0200

Replace the specification of data structures by variable references
as the parameter for the operator "sizeof" to make the corresponding size
determination a bit safer according to the Linux coding style convention.

Signed-off-by: Markus Elfring 
---
 drivers/media/usb/dvb-usb-v2/dvb_usb_core.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/usb/dvb-usb-v2/dvb_usb_core.c 
b/drivers/media/usb/dvb-usb-v2/dvb_usb_core.c
index d0fbf0b0b1cb..f0b225ee4515 100644
--- a/drivers/media/usb/dvb-usb-v2/dvb_usb_core.c
+++ b/drivers/media/usb/dvb-usb-v2/dvb_usb_core.c
@@ -279,5 +279,5 @@ static int dvb_usb_start_feed(struct dvb_demux_feed 
*dvbdmxfeed)
if (d->props->get_stream_config) {
memcpy(_props, >props->stream,
-   sizeof(struct usb_data_stream_properties));
+  sizeof(stream_props));
ret = d->props->get_stream_config(adap->fe[adap->active_fe],
>ts_type, _props);
@@ -919,5 +919,5 @@ int dvb_usbv2_probe(struct usb_interface *intf,
goto err;
}
 
-   d = kzalloc(sizeof(struct dvb_usb_device), GFP_KERNEL);
+   d = kzalloc(sizeof(*d), GFP_KERNEL);
if (!d) {
-- 
2.14.1

[PATCH 1/2] [media] dvb_usb_core: Delete two error messages for a failed memory allocation in dvb_usbv2_probe()

[SF Markus Elfring]

From: Markus Elfring 
Date: Mon, 18 Sep 2017 09:25:19 +0200

Omit extra messages for a memory allocation failure in this function.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring 
---
 drivers/media/usb/dvb-usb-v2/dvb_usb_core.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/media/usb/dvb-usb-v2/dvb_usb_core.c 
b/drivers/media/usb/dvb-usb-v2/dvb_usb_core.c
index 096bb75a24e5..d0fbf0b0b1cb 100644
--- a/drivers/media/usb/dvb-usb-v2/dvb_usb_core.c
+++ b/drivers/media/usb/dvb-usb-v2/dvb_usb_core.c
@@ -923,5 +923,4 @@ int dvb_usbv2_probe(struct usb_interface *intf,
if (!d) {
-   dev_err(>dev, "%s: kzalloc() failed\n", KBUILD_MODNAME);
ret = -ENOMEM;
goto err;
}
@@ -946,6 +945,4 @@ int dvb_usbv2_probe(struct usb_interface *intf,
if (!d->priv) {
-   dev_err(>udev->dev, "%s: kzalloc() failed\n",
-   KBUILD_MODNAME);
ret = -ENOMEM;
goto err_free_all;
}
-- 
2.14.1

[PATCH 0/2] [media] dvb_usb_core: Adjustments for two function implementations

[SF Markus Elfring]

From: Markus Elfring 
Date: Mon, 18 Sep 2017 09:51:23 +0200

Two update suggestions were taken into account
from static source code analysis.

Markus Elfring (2):
  Delete two error messages for a failed memory allocation in dvb_usbv2_probe()
  Improve a size determination in two functions

 drivers/media/usb/dvb-usb-v2/dvb_usb_core.c | 7 ++-
 1 file changed, 2 insertions(+), 5 deletions(-)

-- 
2.14.1

Re: [PATCH v4 2/3] media: ov7670: Add the get_fmt callback

[Sakari Ailus]

Hi Wenyou,

On Mon, Sep 18, 2017 at 02:45:13PM +0800, Wenyou Yang wrote:
> @@ -998,8 +1002,15 @@ static int ov7670_set_fmt(struct v4l2_subdev *sd,
>   ret = ov7670_try_fmt_internal(sd, >format, NULL, NULL);
>   if (ret)
>   return ret;
> - cfg->try_fmt = format->format;
> +#ifdef CONFIG_VIDEO_V4L2_SUBDEV_API
> + struct v4l2_mbus_framefmt *mbus_fmt;

This will emit a compiler warning at least.

> +
> + mbus_fmt = v4l2_subdev_get_try_format(sd, cfg, format->pad);
> + *mbus_fmt = format->format;
>   return 0;
> +#else
> + return -ENOTTY;
> +#endif
>   }
>  
>   ret = ov7670_try_fmt_internal(sd, >format, , );

-- 
Sakari Ailus
e-mail: sakari.ai...@iki.fi

Re: [PATCH v4 2/3] media: ov7670: Add the get_fmt callback

[kbuild test robot]

Hi Wenyou,

[auto build test WARNING on linuxtv-media/master]
[also build test WARNING on v4.14-rc1 next-20170918]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Wenyou-Yang/media-ov7670-Add-entity-init-and-power-operation/20170918-145527
base:   git://linuxtv.org/media_tree.git master
config: x86_64-randconfig-x003-201738 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64 

All warnings (new ones prefixed by >>):

   drivers/media/i2c/ov7670.c: In function 'ov7670_set_fmt':
>> drivers/media/i2c/ov7670.c:1006:3: warning: ISO C90 forbids mixed 
>> declarations and code [-Wdeclaration-after-statement]
  struct v4l2_mbus_framefmt *mbus_fmt;
  ^~

vim +1006 drivers/media/i2c/ov7670.c

   984  
   985  /*
   986   * Set a format.
   987   */
   988  static int ov7670_set_fmt(struct v4l2_subdev *sd,
   989  struct v4l2_subdev_pad_config *cfg,
   990  struct v4l2_subdev_format *format)
   991  {
   992  struct ov7670_format_struct *ovfmt;
   993  struct ov7670_win_size *wsize;
   994  struct ov7670_info *info = to_state(sd);
   995  unsigned char com7;
   996  int ret;
   997  
   998  if (format->pad)
   999  return -EINVAL;
  1000  
  1001  if (format->which == V4L2_SUBDEV_FORMAT_TRY) {
  1002  ret = ov7670_try_fmt_internal(sd, >format, 
NULL, NULL);
  1003  if (ret)
  1004  return ret;
  1005  #ifdef CONFIG_VIDEO_V4L2_SUBDEV_API
> 1006  struct v4l2_mbus_framefmt *mbus_fmt;
  1007  
  1008  mbus_fmt = v4l2_subdev_get_try_format(sd, cfg, 
format->pad);
  1009  *mbus_fmt = format->format;
  1010  return 0;
  1011  #else
  1012  return -ENOTTY;
  1013  #endif
  1014  }
  1015  
  1016  ret = ov7670_try_fmt_internal(sd, >format, , 
);
  1017  
  1018  if (ret)
  1019  return ret;
  1020  /*
  1021   * COM7 is a pain in the ass, it doesn't like to be read then
  1022   * quickly written afterward.  But we have everything we need
  1023   * to set it absolutely here, as long as the format-specific
  1024   * register sets list it first.
  1025   */
  1026  com7 = ovfmt->regs[0].value;
  1027  com7 |= wsize->com7_bit;
  1028  ov7670_write(sd, REG_COM7, com7);
  1029  /*
  1030   * Now write the rest of the array.  Also store start/stops
  1031   */
  1032  ov7670_write_array(sd, ovfmt->regs + 1);
  1033  ov7670_set_hw(sd, wsize->hstart, wsize->hstop, wsize->vstart,
  1034  wsize->vstop);
  1035  ret = 0;
  1036  if (wsize->regs)
  1037  ret = ov7670_write_array(sd, wsize->regs);
  1038  info->fmt = ovfmt;
  1039  
  1040  /*
  1041   * If we're running RGB565, we must rewrite clkrc after setting
  1042   * the other parameters or the image looks poor.  If we're *not*
  1043   * doing RGB565, we must not rewrite clkrc or the image looks
  1044   * *really* poor.
  1045   *
  1046   * (Update) Now that we retain clkrc state, we should be able
  1047   * to write it unconditionally, and that will make the frame
  1048   * rate persistent too.
  1049   */
  1050  if (ret == 0)
  1051  ret = ov7670_write(sd, REG_CLKRC, info->clkrc);
  1052  return 0;
  1053  }
  1054  

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip

Re: [PATCH v4 3/3] media: ov7670: Add the s_power operation

[Sakari Ailus]

Hi Wenyou,

Thanks for the update.

The driver exposes controls that are accessible through the sub-device node
even if the device hasn't been powered on.

Many ISP and bridge drivers will also power the sensor down once the last
user of the user space device nodes disappears. You could keep the device
powered at all times or change the behaviour so that controls can be
accessed when the power is off.

The best option would be to convert the driver to use runtime PM. An
example of this can be found in drivers/media/i2c/ov13858.c .

On Mon, Sep 18, 2017 at 02:45:14PM +0800, Wenyou Yang wrote:
> Add the s_power operation which is responsible for manipulating the
> power dowm mode through the PWDN pin and the reset operation through
> the RESET pin.
> 
> Signed-off-by: Wenyou Yang 
> ---
> 
> Changes in v4: None
> Changes in v3: None
> Changes in v2:
>  - Add the patch to support the get_fmt ops.
>  - Remove the redundant invoking ov7670_init_gpio().
> 
>  drivers/media/i2c/ov7670.c | 32 +++-
>  1 file changed, 27 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/media/i2c/ov7670.c b/drivers/media/i2c/ov7670.c
> index 456f48057605..304abc769a67 100644
> --- a/drivers/media/i2c/ov7670.c
> +++ b/drivers/media/i2c/ov7670.c
> @@ -1542,6 +1542,22 @@ static int ov7670_s_register(struct v4l2_subdev *sd, 
> const struct v4l2_dbg_regis
>  }
>  #endif
>  
> +static int ov7670_s_power(struct v4l2_subdev *sd, int on)
> +{
> + struct ov7670_info *info = to_state(sd);
> +
> + if (info->pwdn_gpio)
> + gpiod_direction_output(info->pwdn_gpio, !on);
> + if (on && info->resetb_gpio) {
> + gpiod_set_value(info->resetb_gpio, 1);
> + usleep_range(500, 1000);
> + gpiod_set_value(info->resetb_gpio, 0);
> + usleep_range(3000, 5000);
> + }
> +
> + return 0;
> +}
> +
>  static void ov7670_get_default_format(struct v4l2_subdev *sd,
> struct v4l2_mbus_framefmt *format)
>  {
> @@ -1575,6 +1591,7 @@ static const struct v4l2_subdev_core_ops 
> ov7670_core_ops = {
>   .g_register = ov7670_g_register,
>   .s_register = ov7670_s_register,
>  #endif
> + .s_power = ov7670_s_power,
>  };
>  
>  static const struct v4l2_subdev_video_ops ov7670_video_ops = {
> @@ -1692,23 +1709,25 @@ static int ov7670_probe(struct i2c_client *client,
>   if (ret)
>   return ret;
>  
> - ret = ov7670_init_gpio(client, info);
> - if (ret)
> - goto clk_disable;
> -
>   info->clock_speed = clk_get_rate(info->clk) / 100;
>   if (info->clock_speed < 10 || info->clock_speed > 48) {
>   ret = -EINVAL;
>   goto clk_disable;
>   }
>  
> + ret = ov7670_init_gpio(client, info);
> + if (ret)
> + goto clk_disable;
> +
> + ov7670_s_power(sd, 1);
> +
>   /* Make sure it's an ov7670 */
>   ret = ov7670_detect(sd);
>   if (ret) {
>   v4l_dbg(1, debug, client,
>   "chip found @ 0x%x (%s) is not an ov7670 chip.\n",
>   client->addr << 1, client->adapter->name);
> - goto clk_disable;
> + goto power_off;
>   }
>   v4l_info(client, "chip found @ 0x%02x (%s)\n",
>   client->addr << 1, client->adapter->name);
> @@ -1787,6 +1806,8 @@ static int ov7670_probe(struct i2c_client *client,
>  #endif
>  hdl_free:
>   v4l2_ctrl_handler_free(>hdl);
> +power_off:
> + ov7670_s_power(sd, 0);
>  clk_disable:
>   clk_disable_unprepare(info->clk);
>   return ret;
> @@ -1804,6 +1825,7 @@ static int ov7670_remove(struct i2c_client *client)
>  #if defined(CONFIG_MEDIA_CONTROLLER)
>   media_entity_cleanup(>sd.entity);
>  #endif
> + ov7670_s_power(sd, 0);
>   return 0;
>  }
>  
> -- 
> 2.13.0
> 

-- 
Sakari Ailus
e-mail: sakari.ai...@iki.fi

Re: [PATCH] staging: atomisp: add a driver for ov5648 camera sensor

[Andy Shevchenko]

On Mon, 2017-09-11 at 20:03 +0200, Devid Antonio Filoni wrote:
> On Mon, Sep 11, 2017 at 05:55:29PM +0300, Sakari Ailus wrote:
> > Hi Devid,
> > 
> > Please see my comments below.
> > 
> > Andy: please look for "INT5648".
> 
> Hi Sakari,
> I'm replying below to your comments. I'll work on a v2 patch as soon
> as we get
> more comments.
> 
> About "INT5648", I extracted it from the DSDT of my Lenovo Miix 310,
> take a look
> at https://pastebin.com/ExHWYr8g .

First of all, thank you, Sakari, to raise a flag here.

Second, Devid, please answer to the following:
is it an official BIOS which is available in the wild?

If it's so, please, add a paragraph to the commit message explaining how do you 
get this and point to the DSDT excerpt.
Put an answer to above question to the commit message as well.

-- 
Andy Shevchenko 
Intel Finland Oy

[PATCH v4 3/3] media: ov7670: Add the s_power operation

[Wenyou Yang]

Add the s_power operation which is responsible for manipulating the
power dowm mode through the PWDN pin and the reset operation through
the RESET pin.

Signed-off-by: Wenyou Yang 
---

Changes in v4: None
Changes in v3: None
Changes in v2:
 - Add the patch to support the get_fmt ops.
 - Remove the redundant invoking ov7670_init_gpio().

 drivers/media/i2c/ov7670.c | 32 +++-
 1 file changed, 27 insertions(+), 5 deletions(-)

diff --git a/drivers/media/i2c/ov7670.c b/drivers/media/i2c/ov7670.c
index 456f48057605..304abc769a67 100644
--- a/drivers/media/i2c/ov7670.c
+++ b/drivers/media/i2c/ov7670.c
@@ -1542,6 +1542,22 @@ static int ov7670_s_register(struct v4l2_subdev *sd, 
const struct v4l2_dbg_regis
 }
 #endif
 
+static int ov7670_s_power(struct v4l2_subdev *sd, int on)
+{
+   struct ov7670_info *info = to_state(sd);
+
+   if (info->pwdn_gpio)
+   gpiod_direction_output(info->pwdn_gpio, !on);
+   if (on && info->resetb_gpio) {
+   gpiod_set_value(info->resetb_gpio, 1);
+   usleep_range(500, 1000);
+   gpiod_set_value(info->resetb_gpio, 0);
+   usleep_range(3000, 5000);
+   }
+
+   return 0;
+}
+
 static void ov7670_get_default_format(struct v4l2_subdev *sd,
  struct v4l2_mbus_framefmt *format)
 {
@@ -1575,6 +1591,7 @@ static const struct v4l2_subdev_core_ops ov7670_core_ops 
= {
.g_register = ov7670_g_register,
.s_register = ov7670_s_register,
 #endif
+   .s_power = ov7670_s_power,
 };
 
 static const struct v4l2_subdev_video_ops ov7670_video_ops = {
@@ -1692,23 +1709,25 @@ static int ov7670_probe(struct i2c_client *client,
if (ret)
return ret;
 
-   ret = ov7670_init_gpio(client, info);
-   if (ret)
-   goto clk_disable;
-
info->clock_speed = clk_get_rate(info->clk) / 100;
if (info->clock_speed < 10 || info->clock_speed > 48) {
ret = -EINVAL;
goto clk_disable;
}
 
+   ret = ov7670_init_gpio(client, info);
+   if (ret)
+   goto clk_disable;
+
+   ov7670_s_power(sd, 1);
+
/* Make sure it's an ov7670 */
ret = ov7670_detect(sd);
if (ret) {
v4l_dbg(1, debug, client,
"chip found @ 0x%x (%s) is not an ov7670 chip.\n",
client->addr << 1, client->adapter->name);
-   goto clk_disable;
+   goto power_off;
}
v4l_info(client, "chip found @ 0x%02x (%s)\n",
client->addr << 1, client->adapter->name);
@@ -1787,6 +1806,8 @@ static int ov7670_probe(struct i2c_client *client,
 #endif
 hdl_free:
v4l2_ctrl_handler_free(>hdl);
+power_off:
+   ov7670_s_power(sd, 0);
 clk_disable:
clk_disable_unprepare(info->clk);
return ret;
@@ -1804,6 +1825,7 @@ static int ov7670_remove(struct i2c_client *client)
 #if defined(CONFIG_MEDIA_CONTROLLER)
media_entity_cleanup(>sd.entity);
 #endif
+   ov7670_s_power(sd, 0);
return 0;
 }
 
-- 
2.13.0

[PATCH v4 2/3] media: ov7670: Add the get_fmt callback

[Wenyou Yang]

Add the get_fmt callback, also enable V4L2_SUBDEV_FL_HAS_DEVNODE flag
to make this subdev has device node.

Signed-off-by: Wenyou Yang 
---

Changes in v4:
 - Fix the build error when not enabling V4L2 sub-device userspace API option.

Changes in v3:
 - Keep tried format info in the try_fmt member of
   v4l2_subdev__pad_config struct.
 - Add the internal_ops callback to set default format.

Changes in v2: None

 drivers/media/i2c/ov7670.c | 75 +-
 1 file changed, 74 insertions(+), 1 deletion(-)

diff --git a/drivers/media/i2c/ov7670.c b/drivers/media/i2c/ov7670.c
index 553945d4ca28..456f48057605 100644
--- a/drivers/media/i2c/ov7670.c
+++ b/drivers/media/i2c/ov7670.c
@@ -232,6 +232,7 @@ struct ov7670_info {
struct v4l2_ctrl *saturation;
struct v4l2_ctrl *hue;
};
+   struct v4l2_mbus_framefmt format;
struct ov7670_format_struct *fmt;  /* Current format */
struct clk *clk;
struct gpio_desc *resetb_gpio;
@@ -975,6 +976,9 @@ static int ov7670_try_fmt_internal(struct v4l2_subdev *sd,
fmt->width = wsize->width;
fmt->height = wsize->height;
fmt->colorspace = ov7670_formats[index].colorspace;
+
+   info->format = *fmt;
+
return 0;
 }
 
@@ -998,8 +1002,15 @@ static int ov7670_set_fmt(struct v4l2_subdev *sd,
ret = ov7670_try_fmt_internal(sd, >format, NULL, NULL);
if (ret)
return ret;
-   cfg->try_fmt = format->format;
+#ifdef CONFIG_VIDEO_V4L2_SUBDEV_API
+   struct v4l2_mbus_framefmt *mbus_fmt;
+
+   mbus_fmt = v4l2_subdev_get_try_format(sd, cfg, format->pad);
+   *mbus_fmt = format->format;
return 0;
+#else
+   return -ENOTTY;
+#endif
}
 
ret = ov7670_try_fmt_internal(sd, >format, , );
@@ -1041,6 +1052,29 @@ static int ov7670_set_fmt(struct v4l2_subdev *sd,
return 0;
 }
 
+static int ov7670_get_fmt(struct v4l2_subdev *sd,
+ struct v4l2_subdev_pad_config *cfg,
+ struct v4l2_subdev_format *format)
+{
+   struct ov7670_info *info = to_state(sd);
+
+   if (format->which == V4L2_SUBDEV_FORMAT_TRY) {
+#ifdef CONFIG_VIDEO_V4L2_SUBDEV_API
+   struct v4l2_mbus_framefmt *mbus_fmt;
+
+   mbus_fmt = v4l2_subdev_get_try_format(sd, cfg, 0);
+   format->format = *mbus_fmt;
+   return 0;
+#else
+   return -ENOTTY;
+#endif
+   } else {
+   format->format = info->format;
+   }
+
+   return 0;
+}
+
 /*
  * Implement G/S_PARM.  There is a "high quality" mode we could try
  * to do someday; for now, we just do the frame rate tweak.
@@ -1508,6 +1542,30 @@ static int ov7670_s_register(struct v4l2_subdev *sd, 
const struct v4l2_dbg_regis
 }
 #endif
 
+static void ov7670_get_default_format(struct v4l2_subdev *sd,
+ struct v4l2_mbus_framefmt *format)
+{
+   struct ov7670_info *info = to_state(sd);
+
+   format->width = info->devtype->win_sizes[0].width;
+   format->height = info->devtype->win_sizes[0].height;
+   format->colorspace = info->fmt->colorspace;
+   format->code = info->fmt->mbus_code;
+   format->field = V4L2_FIELD_NONE;
+}
+
+#ifdef CONFIG_VIDEO_V4L2_SUBDEV_API
+static int ov7670_open(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh)
+{
+   struct v4l2_mbus_framefmt *format =
+   v4l2_subdev_get_try_format(sd, fh->pad, 0);
+
+   ov7670_get_default_format(sd, format);
+
+   return 0;
+}
+#endif
+
 /* --- */
 
 static const struct v4l2_subdev_core_ops ov7670_core_ops = {
@@ -1528,6 +1586,7 @@ static const struct v4l2_subdev_pad_ops ov7670_pad_ops = {
.enum_frame_interval = ov7670_enum_frame_interval,
.enum_frame_size = ov7670_enum_frame_size,
.enum_mbus_code = ov7670_enum_mbus_code,
+   .get_fmt = ov7670_get_fmt,
.set_fmt = ov7670_set_fmt,
 };
 
@@ -1537,6 +1596,12 @@ static const struct v4l2_subdev_ops ov7670_ops = {
.pad = _pad_ops,
 };
 
+#ifdef CONFIG_VIDEO_V4L2_SUBDEV_API
+static const struct v4l2_subdev_internal_ops ov7670_subdev_internal_ops = {
+   .open = ov7670_open,
+};
+#endif
+
 /* --- */
 
 static const struct ov7670_devtype ov7670_devdata[] = {
@@ -1589,6 +1654,11 @@ static int ov7670_probe(struct i2c_client *client,
sd = >sd;
v4l2_i2c_subdev_init(sd, client, _ops);
 
+#ifdef CONFIG_VIDEO_V4L2_SUBDEV_API
+   sd->internal_ops = _subdev_internal_ops;
+   sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
+#endif
+
info->clock_speed = 30; /* default: a guess */
if (client->dev.platform_data) {
struct ov7670_config *config = 

[PATCH v4 1/3] media: ov7670: Add entity pads initialization

[Wenyou Yang]

Add the media entity pads initialization.

Signed-off-by: Wenyou Yang 
---

Changes in v4:
 - Fix the build error when not enabling Media Controller API option.

Changes in v3: None
Changes in v2: None

 drivers/media/i2c/ov7670.c | 21 -
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/drivers/media/i2c/ov7670.c b/drivers/media/i2c/ov7670.c
index e88549f0e704..553945d4ca28 100644
--- a/drivers/media/i2c/ov7670.c
+++ b/drivers/media/i2c/ov7670.c
@@ -213,6 +213,9 @@ struct ov7670_devtype {
 struct ov7670_format_struct;  /* coming later */
 struct ov7670_info {
struct v4l2_subdev sd;
+#if defined(CONFIG_MEDIA_CONTROLLER)
+   struct media_pad pad;
+#endif
struct v4l2_ctrl_handler hdl;
struct {
/* gain cluster */
@@ -1688,14 +1691,27 @@ static int ov7670_probe(struct i2c_client *client,
v4l2_ctrl_auto_cluster(2, >auto_exposure,
   V4L2_EXPOSURE_MANUAL, false);
v4l2_ctrl_cluster(2, >saturation);
+
+#if defined(CONFIG_MEDIA_CONTROLLER)
+   info->pad.flags = MEDIA_PAD_FL_SOURCE;
+   info->sd.entity.function = MEDIA_ENT_F_CAM_SENSOR;
+   ret = media_entity_pads_init(>sd.entity, 1, >pad);
+   if (ret < 0)
+   goto hdl_free;
+#endif
+
v4l2_ctrl_handler_setup(>hdl);
 
ret = v4l2_async_register_subdev(>sd);
if (ret < 0)
-   goto hdl_free;
+   goto entity_cleanup;
 
return 0;
 
+entity_cleanup:
+#if defined(CONFIG_MEDIA_CONTROLLER)
+   media_entity_cleanup(>sd.entity);
+#endif
 hdl_free:
v4l2_ctrl_handler_free(>hdl);
 clk_disable:
@@ -1712,6 +1728,9 @@ static int ov7670_remove(struct i2c_client *client)
v4l2_device_unregister_subdev(sd);
v4l2_ctrl_handler_free(>hdl);
clk_disable_unprepare(info->clk);
+#if defined(CONFIG_MEDIA_CONTROLLER)
+   media_entity_cleanup(>sd.entity);
+#endif
return 0;
 }
 
-- 
2.13.0

[PATCH v4 0/3] media: ov7670: Add entity init and power operation

[Wenyou Yang]

This patch set is to add the media entity pads initialization,
the s_power operation and get_fmt callback support.

Changes in v4:
 - Fix the build error when not enabling Media Controller API option.
 - Fix the build error when not enabling V4L2 sub-device userspace API option.

Changes in v3:
 - Keep tried format info in the try_fmt member of
   v4l2_subdev__pad_config struct.
 - Add the internal_ops callback to set default format.

Changes in v2:
 - Add the patch to support the get_fmt ops.
 - Remove the redundant invoking ov7670_init_gpio().

Wenyou Yang (3):
  media: ov7670: Add entity pads initialization
  media: ov7670: Add the get_fmt callback
  media: ov7670: Add the s_power operation

 drivers/media/i2c/ov7670.c | 128 ++---
 1 file changed, 121 insertions(+), 7 deletions(-)

-- 
2.13.0

[PATCH v2 5/5] media: atmel-isc: Rework the format list

[Wenyou Yang]

To improve the readability of code, split the format array into two,
one for the format description, other for the register configuration.
Meanwhile, add the flag member to indicate the format can be achieved
from the sensor or be produced by the controller, and rename members
related to the register configuration.

Also add more formats support: GREY, ARGB444, ARGB555 and ARGB32.

Signed-off-by: Wenyou Yang 
---

Changes in v2:
 - Add the new patch to remove the unnecessary member from
   isc_subdev_entity struct.
 - Rebase on the patch set,
[PATCH 0/6] [media] Atmel: Adjustments for seven function 
implementations
https://www.mail-archive.com/linux-media@vger.kernel.org/msg118342.html

 drivers/media/platform/atmel/atmel-isc.c | 524 ---
 1 file changed, 405 insertions(+), 119 deletions(-)

diff --git a/drivers/media/platform/atmel/atmel-isc.c 
b/drivers/media/platform/atmel/atmel-isc.c
index 2d876903da71..90bd0b28a975 100644
--- a/drivers/media/platform/atmel/atmel-isc.c
+++ b/drivers/media/platform/atmel/atmel-isc.c
@@ -89,34 +89,56 @@ struct isc_subdev_entity {
struct list_head list;
 };
 
+#define FMT_FLAG_FROM_SENSOR   BIT(0)
+#define FMT_FLAG_FROM_CONTROLLER   BIT(1)
+
 /*
  * struct isc_format - ISC media bus format information
  * @fourcc:Fourcc code for this format
  * @mbus_code: V4L2 media bus format code.
+ * flags:  Indicate format from sensor or converted by controller
  * @bpp:   Bits per pixel (when stored in memory)
- * @reg_bps:   reg value for bits per sample
  * (when transferred over a bus)
- * @pipeline:  pipeline switch
  * @sd_support:Subdev supports this format
  * @isc_support:   ISC can convert raw format to this format
  */
+
 struct isc_format {
u32 fourcc;
u32 mbus_code;
+   u32 flags;
u8  bpp;
 
-   u32 reg_bps;
-   u32 reg_bay_cfg;
-   u32 reg_rlp_mode;
-   u32 reg_dcfg_imode;
-   u32 reg_dctrl_dview;
-
-   u32 pipeline;
-
boolsd_support;
boolisc_support;
 };
 
+/* Pipeline bitmap */
+#define WB_ENABLE  BIT(0)
+#define CFA_ENABLE BIT(1)
+#define CC_ENABLE  BIT(2)
+#define GAM_ENABLE BIT(3)
+#define GAM_BENABLEBIT(4)
+#define GAM_GENABLEBIT(5)
+#define GAM_RENABLEBIT(6)
+#define CSC_ENABLE BIT(7)
+#define CBC_ENABLE BIT(8)
+#define SUB422_ENABLE  BIT(9)
+#define SUB420_ENABLE  BIT(10)
+
+#define GAM_ENABLES(GAM_RENABLE | GAM_GENABLE | GAM_BENABLE | GAM_ENABLE)
+
+struct fmt_config {
+   u32 fourcc;
+
+   u32 pfe_cfg0_bps;
+   u32 cfa_baycfg;
+   u32 rlp_cfg_mode;
+   u32 dcfg_imode;
+   u32 dctrl_dview;
+
+   u32 bits_pipeline;
+};
 
 #define HIST_ENTRIES   512
 #define HIST_BAYER (ISC_HIS_CFG_MODE_B + 1)
@@ -181,80 +203,321 @@ struct isc_device {
struct list_headsubdev_entities;
 };
 
-#define RAW_FMT_IND_START0
-#define RAW_FMT_IND_END  11
-#define ISC_FMT_IND_START12
-#define ISC_FMT_IND_END  14
-
-static struct isc_format isc_formats[] = {
-   { V4L2_PIX_FMT_SBGGR8, MEDIA_BUS_FMT_SBGGR8_1X8, 8,
- ISC_PFE_CFG0_BPS_EIGHT, ISC_BAY_CFG_BGBG, ISC_RLP_CFG_MODE_DAT8,
- ISC_DCFG_IMODE_PACKED8, ISC_DCTRL_DVIEW_PACKED, 0x0,
- false, false },
-   { V4L2_PIX_FMT_SGBRG8, MEDIA_BUS_FMT_SGBRG8_1X8, 8,
- ISC_PFE_CFG0_BPS_EIGHT, ISC_BAY_CFG_GBGB, ISC_RLP_CFG_MODE_DAT8,
- ISC_DCFG_IMODE_PACKED8, ISC_DCTRL_DVIEW_PACKED, 0x0,
- false, false },
-   { V4L2_PIX_FMT_SGRBG8, MEDIA_BUS_FMT_SGRBG8_1X8, 8,
- ISC_PFE_CFG0_BPS_EIGHT, ISC_BAY_CFG_GRGR, ISC_RLP_CFG_MODE_DAT8,
- ISC_DCFG_IMODE_PACKED8, ISC_DCTRL_DVIEW_PACKED, 0x0,
- false, false },
-   { V4L2_PIX_FMT_SRGGB8, MEDIA_BUS_FMT_SRGGB8_1X8, 8,
- ISC_PFE_CFG0_BPS_EIGHT, ISC_BAY_CFG_RGRG, ISC_RLP_CFG_MODE_DAT8,
- ISC_DCFG_IMODE_PACKED8, ISC_DCTRL_DVIEW_PACKED, 0x0,
- false, false },
-
-   { V4L2_PIX_FMT_SBGGR10, MEDIA_BUS_FMT_SBGGR10_1X10, 16,
- ISC_PFG_CFG0_BPS_TEN, ISC_BAY_CFG_BGBG, ISC_RLP_CFG_MODE_DAT10,
- ISC_DCFG_IMODE_PACKED16, ISC_DCTRL_DVIEW_PACKED, 0x0,
- false, false },
-   { V4L2_PIX_FMT_SGBRG10, MEDIA_BUS_FMT_SGBRG10_1X10, 16,
- ISC_PFG_CFG0_BPS_TEN, ISC_BAY_CFG_GBGB, ISC_RLP_CFG_MODE_DAT10,
- ISC_DCFG_IMODE_PACKED16, ISC_DCTRL_DVIEW_PACKED, 0x0,
- false, false },
-   { V4L2_PIX_FMT_SGRBG10, MEDIA_BUS_FMT_SGRBG10_1X10, 16,
- ISC_PFG_CFG0_BPS_TEN, ISC_BAY_CFG_GRGR, ISC_RLP_CFG_MODE_DAT10,
- ISC_DCFG_IMODE_PACKED16, ISC_DCTRL_DVIEW_PACKED, 0x0,
- false, false },
-   { V4L2_PIX_FMT_SRGGB10, MEDIA_BUS_FMT_SRGGB10_1X10, 16,
- ISC_PFG_CFG0_BPS_TEN, ISC_BAY_CFG_RGRG, 

[PATCH v2 4/5] media: atmel-isc: Remove unnecessary member

[Wenyou Yang]

Remove the memeber *config from the isc_subdev_entity struct,
the member is useless afterward.

Signed-off-by: Wenyou Yang 
---

Changes in v2: None

 drivers/media/platform/atmel/atmel-isc.c | 10 ++
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/drivers/media/platform/atmel/atmel-isc.c 
b/drivers/media/platform/atmel/atmel-isc.c
index 3ecd2bf016a8..2d876903da71 100644
--- a/drivers/media/platform/atmel/atmel-isc.c
+++ b/drivers/media/platform/atmel/atmel-isc.c
@@ -83,7 +83,6 @@ struct isc_subdev_entity {
struct v4l2_subdev  *sd;
struct v4l2_async_subdev*asd;
struct v4l2_async_notifier  notifier;
-   struct v4l2_subdev_pad_config   *config;
 
u32 pfe_cfg0;
 
@@ -983,6 +982,7 @@ static int isc_try_fmt(struct isc_device *isc, struct 
v4l2_format *f,
 {
struct isc_format *isc_fmt;
struct v4l2_pix_format *pixfmt = >fmt.pix;
+   struct v4l2_subdev_pad_config pad_cfg;
struct v4l2_subdev_format format = {
.which = V4L2_SUBDEV_FORMAT_TRY,
};
@@ -1013,7 +1013,7 @@ static int isc_try_fmt(struct isc_device *isc, struct 
v4l2_format *f,
 
v4l2_fill_mbus_format(, pixfmt, mbus_code);
ret = v4l2_subdev_call(isc->current_subdev->sd, pad, set_fmt,
-  isc->current_subdev->config, );
+  _cfg, );
if (ret < 0)
return ret;
 
@@ -1478,8 +1478,6 @@ static void isc_async_unbind(struct v4l2_async_notifier 
*notifier,
  struct isc_device, v4l2_dev);
cancel_work_sync(>awb_work);
video_unregister_device(>video_dev);
-   if (isc->current_subdev->config)
-   v4l2_subdev_free_pad_config(isc->current_subdev->config);
v4l2_ctrl_handler_free(>ctrls.handler);
 }
 
@@ -1632,10 +1630,6 @@ static int isc_async_complete(struct v4l2_async_notifier 
*notifier)
INIT_LIST_HEAD(>dma_queue);
spin_lock_init(>dma_queue_lock);
 
-   sd_entity->config = v4l2_subdev_alloc_pad_config(sd_entity->sd);
-   if (!sd_entity->config)
-   return -ENOMEM;
-
ret = isc_formats_init(isc);
if (ret < 0) {
v4l2_err(>v4l2_dev,
-- 
2.13.0

[PATCH v2 3/5] media: atmel-isc: Enable the clocks during probe

[Wenyou Yang]

To meet the relationship, enable the HCLOCK and ispck during the
device probe, "isc_pck frequency is less than or equal to isc_ispck,
and isc_ispck is greater than or equal to HCLOCK."
Meanwhile, call the pm_runtime_enable() in the right place.

Signed-off-by: Wenyou Yang 
---

Changes in v2: None

 drivers/media/platform/atmel/atmel-isc.c | 31 +--
 1 file changed, 25 insertions(+), 6 deletions(-)

diff --git a/drivers/media/platform/atmel/atmel-isc.c 
b/drivers/media/platform/atmel/atmel-isc.c
index 8b9c1cafa348..3ecd2bf016a8 100644
--- a/drivers/media/platform/atmel/atmel-isc.c
+++ b/drivers/media/platform/atmel/atmel-isc.c
@@ -1593,6 +1593,7 @@ static int isc_async_complete(struct v4l2_async_notifier 
*notifier)
struct isc_subdev_entity *sd_entity;
struct video_device *vdev = >video_dev;
struct vb2_queue *q = >vb2_vidq;
+   struct device *dev = isc->dev;
int ret;
 
ret = v4l2_device_register_subdev_nodes(>v4l2_dev);
@@ -1676,6 +1677,10 @@ static int isc_async_complete(struct v4l2_async_notifier 
*notifier)
return ret;
}
 
+   pm_runtime_set_active(dev);
+   pm_runtime_enable(dev);
+   pm_request_idle(dev);
+
return 0;
 }
 
@@ -1855,25 +1860,37 @@ static int atmel_isc_probe(struct platform_device *pdev)
return ret;
}
 
+   ret = clk_prepare_enable(isc->hclock);
+   if (ret) {
+   dev_err(dev, "failed to enable hclock: %d\n", ret);
+   return ret;
+   }
+
ret = isc_clk_init(isc);
if (ret) {
dev_err(dev, "failed to init isc clock: %d\n", ret);
-   goto clean_isc_clk;
+   goto unprepare_hclk;
}
 
isc->ispck = isc->isc_clks[ISC_ISPCK].clk;
 
+   ret = clk_prepare_enable(isc->ispck);
+   if (ret) {
+   dev_err(dev, "failed to enable ispck: %d\n", ret);
+   goto unprepare_hclk;
+   }
+
/* ispck should be greater or equal to hclock */
ret = clk_set_rate(isc->ispck, clk_get_rate(isc->hclock));
if (ret) {
dev_err(dev, "failed to set ispck rate: %d\n", ret);
-   goto clean_isc_clk;
+   goto unprepare_clk;
}
 
ret = v4l2_device_register(dev, >v4l2_dev);
if (ret) {
dev_err(dev, "unable to register v4l2 device.\n");
-   goto clean_isc_clk;
+   goto unprepare_clk;
}
 
ret = isc_parse_dt(dev, isc);
@@ -1906,8 +1923,6 @@ static int atmel_isc_probe(struct platform_device *pdev)
break;
}
 
-   pm_runtime_enable(dev);
-
return 0;
 
 cleanup_subdev:
@@ -1916,7 +1931,11 @@ static int atmel_isc_probe(struct platform_device *pdev)
 unregister_v4l2_device:
v4l2_device_unregister(>v4l2_dev);
 
-clean_isc_clk:
+unprepare_clk:
+   clk_disable_unprepare(isc->ispck);
+unprepare_hclk:
+   clk_disable_unprepare(isc->hclock);
+
isc_clk_cleanup(isc);
 
return ret;
-- 
2.13.0

[PATCH v2 2/5] media: atmel-isc: Add prepare and unprepare ops

[Wenyou Yang]

A software write operation to the ISC_CLKEN or ISC_CLKDIS register
requires double clock domain synchronization and is not permitted
when the ISC_SR.SIP is asserted. So add the .prepare and .unprepare
ops to make sure the ISC_CLKSR.SIP is unasserted before the write
operation to the ISC_CLKEN or ISC_CLKDIS register.

Signed-off-by: Wenyou Yang 
---

Changes in v2: None

 drivers/media/platform/atmel/atmel-isc-regs.h |  1 +
 drivers/media/platform/atmel/atmel-isc.c  | 30 +++
 2 files changed, 31 insertions(+)

diff --git a/drivers/media/platform/atmel/atmel-isc-regs.h 
b/drivers/media/platform/atmel/atmel-isc-regs.h
index 6936ac467609..93e58fcf1d5f 100644
--- a/drivers/media/platform/atmel/atmel-isc-regs.h
+++ b/drivers/media/platform/atmel/atmel-isc-regs.h
@@ -42,6 +42,7 @@
 
 /* ISC Clock Status Register */
 #define ISC_CLKSR   0x0020
+#define ISC_CLKSR_SIP  BIT(31)
 
 #define ISC_CLK(n) BIT(n)
 
diff --git a/drivers/media/platform/atmel/atmel-isc.c 
b/drivers/media/platform/atmel/atmel-isc.c
index 78114193af4c..8b9c1cafa348 100644
--- a/drivers/media/platform/atmel/atmel-isc.c
+++ b/drivers/media/platform/atmel/atmel-isc.c
@@ -308,6 +308,34 @@ module_param(sensor_preferred, uint, 0644);
 MODULE_PARM_DESC(sensor_preferred,
 "Sensor is preferred to output the specified format (1-on 
0-off), default 1");
 
+static int isc_wait_clk_stable(struct clk_hw *hw)
+{
+   struct isc_clk *isc_clk = to_isc_clk(hw);
+   struct regmap *regmap = isc_clk->regmap;
+   unsigned long timeout = jiffies + usecs_to_jiffies(1000);
+   unsigned int status;
+
+   while (time_before(jiffies, timeout)) {
+   regmap_read(regmap, ISC_CLKSR, );
+   if (!(status & ISC_CLKSR_SIP))
+   return 0;
+
+   usleep_range(10, 250);
+   }
+
+   return -ETIMEDOUT;
+}
+
+static int isc_clk_prepare(struct clk_hw *hw)
+{
+   return isc_wait_clk_stable(hw);
+}
+
+static void isc_clk_unprepare(struct clk_hw *hw)
+{
+   isc_wait_clk_stable(hw);
+}
+
 static int isc_clk_enable(struct clk_hw *hw)
 {
struct isc_clk *isc_clk = to_isc_clk(hw);
@@ -459,6 +487,8 @@ static int isc_clk_set_rate(struct clk_hw *hw,
 }
 
 static const struct clk_ops isc_clk_ops = {
+   .prepare= isc_clk_prepare,
+   .unprepare  = isc_clk_unprepare,
.enable = isc_clk_enable,
.disable= isc_clk_disable,
.is_enabled = isc_clk_is_enabled,
-- 
2.13.0

[PATCH v2 1/5] media: atmel_isc: Add spin lock for clock enable ops

[Wenyou Yang]

Add the spin lock for the clock enable and disable operations.

Signed-off-by: Wenyou Yang 
---

Changes in v2: None

 drivers/media/platform/atmel/atmel-isc.c | 14 +-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/media/platform/atmel/atmel-isc.c 
b/drivers/media/platform/atmel/atmel-isc.c
index 2f8e345d297e..78114193af4c 100644
--- a/drivers/media/platform/atmel/atmel-isc.c
+++ b/drivers/media/platform/atmel/atmel-isc.c
@@ -65,6 +65,7 @@ struct isc_clk {
struct clk_hw   hw;
struct clk  *clk;
struct regmap   *regmap;
+   spinlock_t  *lock;
u8  id;
u8  parent_id;
u32 div;
@@ -312,26 +313,37 @@ static int isc_clk_enable(struct clk_hw *hw)
struct isc_clk *isc_clk = to_isc_clk(hw);
u32 id = isc_clk->id;
struct regmap *regmap = isc_clk->regmap;
+   unsigned long flags;
+   unsigned int status;
 
dev_dbg(isc_clk->dev, "ISC CLK: %s, div = %d, parent id = %d\n",
__func__, isc_clk->div, isc_clk->parent_id);
 
+   spin_lock_irqsave(isc_clk->lock, flags);
regmap_update_bits(regmap, ISC_CLKCFG,
   ISC_CLKCFG_DIV_MASK(id) | ISC_CLKCFG_SEL_MASK(id),
   (isc_clk->div << ISC_CLKCFG_DIV_SHIFT(id)) |
   (isc_clk->parent_id << ISC_CLKCFG_SEL_SHIFT(id)));
 
regmap_write(regmap, ISC_CLKEN, ISC_CLK(id));
+   spin_unlock_irqrestore(isc_clk->lock, flags);
 
-   return 0;
+   regmap_read(regmap, ISC_CLKSR, );
+   if (status & ISC_CLK(id))
+   return 0;
+   else
+   return -EINVAL;
 }
 
 static void isc_clk_disable(struct clk_hw *hw)
 {
struct isc_clk *isc_clk = to_isc_clk(hw);
u32 id = isc_clk->id;
+   unsigned long flags;
 
+   spin_lock_irqsave(isc_clk->lock, flags);
regmap_write(isc_clk->regmap, ISC_CLKDIS, ISC_CLK(id));
+   spin_unlock_irqrestore(isc_clk->lock, flags);
 }
 
 static int isc_clk_is_enabled(struct clk_hw *hw)
-- 
2.13.0

[PATCH v2 0/5] media: atmel-isc: Rework the format list and the clock

[Wenyou Yang]

To improve the readability of code, rework the format list table,
split the format array into two.
Meanwhile, fix the clock operation issue.

Changes in v2:
 - Add the new patch to remove the unnecessary member from
   isc_subdev_entity struct.
 - Rebase on the patch set,
[PATCH 0/6] [media] Atmel: Adjustments for seven function 
implementations
https://www.mail-archive.com/linux-media@vger.kernel.org/msg118342.html

Wenyou Yang (5):
  media: atmel_isc: Add spin lock for clock enable ops
  media: atmel-isc: Add prepare and unprepare ops
  media: atmel-isc: Enable the clocks during probe
  media: atmel-isc: Remove unnecessary member
  media: atmel-isc: Rework the format list

 drivers/media/platform/atmel/atmel-isc-regs.h |   1 +
 drivers/media/platform/atmel/atmel-isc.c  | 609 --
 2 files changed, 476 insertions(+), 134 deletions(-)

-- 
2.13.0