[PATCH] drivers: media: i2c: constify v4l2_subdev_* structures

[Bhumika Goyal]

v4l2_subdev_{core/pad/video}_ops structures are stored in the
fields of the v4l2_subdev_ops structure which are of type const.
Also, v4l2_subdev_ops structure is passed to a function
having its argument of type const. As these structures are never
modified, so declare them as const.
Done using Coccinelle: (One of the scripts used)

@r1 disable optional_qualifier @
identifier i;
position p;
@@
static struct v4l2_subdev_video_ops i@p = {...};

@ok1@
identifier r1.i;
position p;
struct v4l2_subdev_ops obj;
@@
obj.video=@p;

@bad@
position p!={r1.p,ok1.p};
identifier r1.i;
@@
i@p

@depends on !bad disable optional_qualifier@
identifier r1.i;
@@
+const
struct v4l2_subdev_video_ops i;

File sizes before:
  text data bss dec hex filename
   7810 736  1685622172 drivers/media/i2c/mt9p031.o
   9652 736  24   1041228ac drivers/media/i2c/mt9v032.o
   4613 552  2051851441 drivers/media/i2c/noon010pc30.o
   2615 552   83175 c67 drivers/media/i2c/s5k6a3.o

File sizes after:
  text data bss dec hex filename
   8322 232  168570217a drivers/media/i2c/mt9p031.o
  10164 232  24   1042028b4 drivers/media/i2c/mt9v032.o
   4933 232  2051851441 drivers/media/i2c/noon010pc30.o
   2935 232   83175 c67 drivers/media/i2c/s5k6a3.o

Signed-off-by: Bhumika Goyal 
---
 drivers/media/i2c/mt9p031.c | 8 
 drivers/media/i2c/mt9v032.c | 8 
 drivers/media/i2c/noon010pc30.c | 4 ++--
 drivers/media/i2c/s5k6a3.c  | 6 +++---
 4 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/drivers/media/i2c/mt9p031.c b/drivers/media/i2c/mt9p031.c
index 237737f..91d822f 100644
--- a/drivers/media/i2c/mt9p031.c
+++ b/drivers/media/i2c/mt9p031.c
@@ -972,15 +972,15 @@ static int mt9p031_close(struct v4l2_subdev *subdev, 
struct v4l2_subdev_fh *fh)
return mt9p031_set_power(subdev, 0);
 }
 
-static struct v4l2_subdev_core_ops mt9p031_subdev_core_ops = {
+static const struct v4l2_subdev_core_ops mt9p031_subdev_core_ops = {
.s_power= mt9p031_set_power,
 };
 
-static struct v4l2_subdev_video_ops mt9p031_subdev_video_ops = {
+static const struct v4l2_subdev_video_ops mt9p031_subdev_video_ops = {
.s_stream   = mt9p031_s_stream,
 };
 
-static struct v4l2_subdev_pad_ops mt9p031_subdev_pad_ops = {
+static const struct v4l2_subdev_pad_ops mt9p031_subdev_pad_ops = {
.enum_mbus_code = mt9p031_enum_mbus_code,
.enum_frame_size = mt9p031_enum_frame_size,
.get_fmt = mt9p031_get_format,
@@ -989,7 +989,7 @@ static int mt9p031_close(struct v4l2_subdev *subdev, struct 
v4l2_subdev_fh *fh)
.set_selection = mt9p031_set_selection,
 };
 
-static struct v4l2_subdev_ops mt9p031_subdev_ops = {
+static const struct v4l2_subdev_ops mt9p031_subdev_ops = {
.core   = _subdev_core_ops,
.video  = _subdev_video_ops,
.pad= _subdev_pad_ops,
diff --git a/drivers/media/i2c/mt9v032.c b/drivers/media/i2c/mt9v032.c
index 58eb62f..88b7890 100644
--- a/drivers/media/i2c/mt9v032.c
+++ b/drivers/media/i2c/mt9v032.c
@@ -936,15 +936,15 @@ static int mt9v032_close(struct v4l2_subdev *subdev, 
struct v4l2_subdev_fh *fh)
return mt9v032_set_power(subdev, 0);
 }
 
-static struct v4l2_subdev_core_ops mt9v032_subdev_core_ops = {
+static const struct v4l2_subdev_core_ops mt9v032_subdev_core_ops = {
.s_power= mt9v032_set_power,
 };
 
-static struct v4l2_subdev_video_ops mt9v032_subdev_video_ops = {
+static const struct v4l2_subdev_video_ops mt9v032_subdev_video_ops = {
.s_stream   = mt9v032_s_stream,
 };
 
-static struct v4l2_subdev_pad_ops mt9v032_subdev_pad_ops = {
+static const struct v4l2_subdev_pad_ops mt9v032_subdev_pad_ops = {
.enum_mbus_code = mt9v032_enum_mbus_code,
.enum_frame_size = mt9v032_enum_frame_size,
.get_fmt = mt9v032_get_format,
@@ -953,7 +953,7 @@ static int mt9v032_close(struct v4l2_subdev *subdev, struct 
v4l2_subdev_fh *fh)
.set_selection = mt9v032_set_selection,
 };
 
-static struct v4l2_subdev_ops mt9v032_subdev_ops = {
+static const struct v4l2_subdev_ops mt9v032_subdev_ops = {
.core   = _subdev_core_ops,
.video  = _subdev_video_ops,
.pad= _subdev_pad_ops,
diff --git a/drivers/media/i2c/noon010pc30.c b/drivers/media/i2c/noon010pc30.c
index 30cb90b..88c498a 100644
--- a/drivers/media/i2c/noon010pc30.c
+++ b/drivers/media/i2c/noon010pc30.c
@@ -664,13 +664,13 @@ static int noon010_open(struct v4l2_subdev *sd, struct 
v4l2_subdev_fh *fh)
.log_status = noon010_log_status,
 };
 
-static struct v4l2_subdev_pad_ops noon010_pad_ops = {
+static const struct v4l2_subdev_pad_ops noon010_pad_ops = {
.enum_mbus_code = noon010_enum_mbus_code,
.get_fmt= noon010_get_fmt,
.set_fmt= noon010_set_fmt,
 };
 
-static struct v4l2_subdev_video_ops noon010_video_ops = {

Re: [media] bt8xx: One function call less in bttv_input_init() after error detection

[Daniele Nicolodi]

On 12/12/16 00:33, SF Markus Elfring wrote:
>>> I would prefer a safer coding style for the corresponding
>>> exception handling.
>>
>> Can you please point out what is wrong in the current code
> 
> Is it useful to reconsider the software situation that another memory
> allocation is attempted when it could be determined that a previous one
> failed already?

No.

> Are two successful allocations finally needed to achieve the desired task?

Yes.

>> and how the changes you propose fix the problem?
> 
> I suggest to check return values immediately after each function call.
> An error situation can be detected earlier then and only the required
> clean-up functionality will be executed at the end.

Which improvement does this bring?

>> No one has expressed acceptance for the kind of change you propose with
>> this patch, or to previous patches you proposed changing similar constructs.
> 
> I got a mixed impression from the acceptance statistics about my
> published patches.

Have you proposed a similar patch that was accepted? I don't find record
of it, but I may be wrong.

>> The fact that you propose over and over again a class of changes that
>> has been already vocally rejected would suggest otherwise.
> 
> I dare to propose another look at results from source code search patterns.

Why?

Cheers,
Daniele

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [media] bt8xx: One function call less in bttv_input_init() after error detection

[SF Markus Elfring]

>> I would prefer a safer coding style for the corresponding
>> exception handling.
> 
> Can you please point out what is wrong in the current code

Is it useful to reconsider the software situation that another memory
allocation is attempted when it could be determined that a previous one
failed already?
Are two successful allocations finally needed to achieve the desired task?


> and how the changes you propose fix the problem?

I suggest to check return values immediately after each function call.
An error situation can be detected earlier then and only the required
clean-up functionality will be executed at the end.


> No one has expressed acceptance for the kind of change you propose with
> this patch, or to previous patches you proposed changing similar constructs.

I got a mixed impression from the acceptance statistics about my
published patches.


> The fact that you propose over and over again a class of changes that
> has been already vocally rejected would suggest otherwise.

I dare to propose another look at results from source code search patterns.


> The major achievement you obtained so far is that one of the maintainers
> of a large fraction of the kernel refuses to look at your patch submissions.

It can happen that some patterns are occasionally "too special"
to grow the popularity for such change possibilities and desired software
improvements quickly.
There are also different views about affected implementation details
by the software development community, aren't there?

Regards,
Markus
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v4l-utils] ir-ctl: `strndupa' undefined with --disable-nls

[Greg Whiteley]

Signed-off-by: Greg Whiteley 
---
 utils/ir-ctl/ir-ctl.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/utils/ir-ctl/ir-ctl.c b/utils/ir-ctl/ir-ctl.c
index f19bd05..707aa1f 100644
--- a/utils/ir-ctl/ir-ctl.c
+++ b/utils/ir-ctl/ir-ctl.c
@@ -37,6 +37,7 @@
 # include 
 # include 
 #else
+# include 
 # define _(string) string
 #endif
 
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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:   Mon Dec 12 05:00:18 CET 2016
media-tree git hash:365fe4e0ce218dc5ad10df17b150a366b6015499
media_build git hash:   1606032398b1d79149c1507be2029e1a00d8dff0
v4l-utils git hash: 188e604d57bec065078ff772c802b93ddb6def4b
gcc version:i686-linux-gcc (GCC) 6.2.0
sparse version: v0.5.0-3553-g78b2ea6
smatch version: v0.5.0-3553-g78b2ea6
host hardware:  x86_64
host os:4.8.0-164

linux-git-arm-at91: OK
linux-git-arm-davinci: OK
linux-git-arm-multi: OK
linux-git-arm-pxa: OK
linux-git-blackfin-bf561: OK
linux-git-i686: OK
linux-git-m32r: OK
linux-git-mips: OK
linux-git-powerpc64: OK
linux-git-sh: OK
linux-git-x86_64: OK
linux-2.6.36.4-i686: 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: OK
linux-3.12.67-i686: OK
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-rc5-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: OK
linux-3.12.67-x86_64: OK
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: OK
linux-4.9-rc5-x86_64: OK
apps: WARNINGS
spec-git: OK
sparse: WARNINGS

Detailed results are available here:

http://www.xs4all.nl/~hverkuil/logs/Monday.log

Full logs are available here:

http://www.xs4all.nl/~hverkuil/logs/Monday.tar.bz2

The Media Infrastructure API from this daily build is here:

http://www.xs4all.nl/~hverkuil/spec/index.html
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v8 2/4] vcodec: mediatek: Add Mediatek JPEG Decoder Driver

[Ricky Liang]

Hi Rick,

On Wed, Nov 30, 2016 at 11:08 AM, Rick Chang  wrote:
> Add v4l2 driver for Mediatek JPEG Decoder
>
> Signed-off-by: Rick Chang 
> Signed-off-by: Minghsiu Tsai 



> +static bool mtk_jpeg_check_resolution_change(struct mtk_jpeg_ctx *ctx,
> +struct mtk_jpeg_dec_param *param)
> +{
> +   struct mtk_jpeg_dev *jpeg = ctx->jpeg;
> +   struct mtk_jpeg_q_data *q_data;
> +
> +   q_data = >out_q;
> +   if (q_data->w != param->pic_w || q_data->h != param->pic_h) {
> +   v4l2_dbg(1, debug, >v4l2_dev, "Picture size change\n");
> +   return true;
> +   }
> +
> +   q_data = >cap_q;
> +   if (q_data->fmt != mtk_jpeg_find_format(ctx, param->dst_fourcc,
> +   MTK_JPEG_FMT_TYPE_CAPTURE)) {
> +   v4l2_dbg(1, debug, >v4l2_dev, "format change\n");
> +   return true;
> +   }
> +   return false;



> +static void mtk_jpeg_device_run(void *priv)
> +{
> +   struct mtk_jpeg_ctx *ctx = priv;
> +   struct mtk_jpeg_dev *jpeg = ctx->jpeg;
> +   struct vb2_buffer *src_buf, *dst_buf;
> +   enum vb2_buffer_state buf_state = VB2_BUF_STATE_ERROR;
> +   unsigned long flags;
> +   struct mtk_jpeg_src_buf *jpeg_src_buf;
> +   struct mtk_jpeg_bs bs;
> +   struct mtk_jpeg_fb fb;
> +   int i;
> +
> +   src_buf = v4l2_m2m_next_src_buf(ctx->fh.m2m_ctx);
> +   dst_buf = v4l2_m2m_next_dst_buf(ctx->fh.m2m_ctx);
> +   jpeg_src_buf = mtk_jpeg_vb2_to_srcbuf(src_buf);
> +
> +   if (jpeg_src_buf->flags & MTK_JPEG_BUF_FLAGS_LAST_FRAME) {
> +   for (i = 0; i < dst_buf->num_planes; i++)
> +   vb2_set_plane_payload(dst_buf, i, 0);
> +   buf_state = VB2_BUF_STATE_DONE;
> +   goto dec_end;
> +   }
> +
> +   if (mtk_jpeg_check_resolution_change(ctx, _src_buf->dec_param)) {
> +   mtk_jpeg_queue_src_chg_event(ctx);
> +   ctx->state = MTK_JPEG_SOURCE_CHANGE;
> +   v4l2_m2m_job_finish(jpeg->m2m_dev, ctx->fh.m2m_ctx);
> +   return;
> +   }

This only detects source change if multiple OUPUT buffers are queued.
It does not catch the source change in the following scenario:

- OUPUT buffers for jpeg1 enqueued
- OUTPUT queue STREAMON
- userspace creates CAPTURE buffers
- CAPTURE buffers enqueued
- CAPTURE queue STREAMON
- decode
- OUTPUT queue STREAMOFF
- userspace recreates OUTPUT buffers for jpeg2
- OUTPUT buffers for jpeg2 enqueued
- OUTPUT queue STREAMON

In the above sequence if jpeg2's decoded size is larger than jpeg1 the
function fails to detect that the existing CAPTURE buffers are not big
enough to hold the decoded data.

A possible fix is to pass *dst_buf to
mtk_jpeg_check_resolution_change(), and check in the function that all
the dst_buf planes are large enough to hold the decoded data.

> +
> +   mtk_jpeg_set_dec_src(ctx, src_buf, );
> +   if (mtk_jpeg_set_dec_dst(ctx, _src_buf->dec_param, dst_buf, ))
> +   goto dec_end;
> +
> +   spin_lock_irqsave(>hw_lock, flags);
> +   mtk_jpeg_dec_reset(jpeg->dec_reg_base);
> +   mtk_jpeg_dec_set_config(jpeg->dec_reg_base,
> +   _src_buf->dec_param, , );
> +
> +   mtk_jpeg_dec_start(jpeg->dec_reg_base);
> +   spin_unlock_irqrestore(>hw_lock, flags);
> +   return;
> +
> +dec_end:
> +   v4l2_m2m_src_buf_remove(ctx->fh.m2m_ctx);
> +   v4l2_m2m_dst_buf_remove(ctx->fh.m2m_ctx);
> +   v4l2_m2m_buf_done(to_vb2_v4l2_buffer(src_buf), buf_state);
> +   v4l2_m2m_buf_done(to_vb2_v4l2_buffer(dst_buf), buf_state);
> +   v4l2_m2m_job_finish(jpeg->m2m_dev, ctx->fh.m2m_ctx);
> +}


--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [media] bt8xx: One function call less in bttv_input_init() after error detection

[Daniele Nicolodi]

On 10/12/16 15:10, SF Markus Elfring wrote:
>> Despite that, you have found several instances of similar constructs:
> 
> Yes. - Special source code search pattern can point such places out
> for further considerations.

This is one of the things that makes reviewing the patches you submit
quire annoying: if a class of changes has already been turned town, why
do you insist in proposing it?

>> Didn't it occur to you that maybe those constructs are fine the way
>> they are and this is the idiomatic way to write that kind of code?
> 
> Such a programming approach might look convenient. - I would prefer
> a safer coding style for the corresponding exception handling.

Can you please point out what is wrong in the current code and how the
changes you propose fix the problem?  Clearly the people reading your
patches do not see the problem you are seeing.

>> Why are you submitting patches implementing changes that have already
>> been rejected?
> 
> The feedback to my update mixture is varying between acceptance and
> disagreements as usual.

No one has expressed acceptance for the kind of change you propose with
this patch, or to previous patches you proposed changing similar constructs.

>> Judging from your recent submissions, it seems that this process is not
>> working well for you. I'm probably not the only one that is wonderign
>> what are you trying to obtain with your patch submissions, other than
>> having your name in the git log.
> 
> I am picking some change possibilities up in the hope of related
> software improvements.

The fact that you propose over and over again a class of changes that
has been already vocally rejected would suggest otherwise. The major
achievement you obtained so far is that one of the maintainers of a
large fraction of the kernel refuses to look at your patch submissions.

Maybe you should revise how you contribute to Linux kernel development.
Apparently ignoring comments to your previous patch submission and not
showing that you have been learning from previous interactions, is not
going to help.

Cheers,
Daniele

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 3/4] stk1160: Add module param for setting the record gain.

[mahasler]

Sorry about the broken formatting. Here's the diff once more:

diff --git a/drivers/media/usb/stk1160/stk1160-ac97.c 
b/drivers/media/usb/stk1160/stk1160-ac97.c
index 95648ac..708792b 100644
--- a/drivers/media/usb/stk1160/stk1160-ac97.c
+++ b/drivers/media/usb/stk1160/stk1160-ac97.c
@@ -23,11 +23,30 @@
  *
  */
 
-#include 
+#include 
 
 #include "stk1160.h"
 #include "stk1160-reg.h"
 
+static int stk1160_ac97_wait_transfer_complete(struct stk1160 *dev)
+{
+   unsigned long timeout = jiffies + 
msecs_to_jiffies(STK1160_AC97_TIMEOUT);
+   u8 value;
+
+   /* Wait for AC97 transfer to complete */
+   while (time_is_after_jiffies(timeout)) {
+   stk1160_read_reg(dev, STK1160_AC97CTL_0, );
+
+   if (!(value & (STK1160_AC97CTL_0_CR | STK1160_AC97CTL_0_CW)))
+   return 0;
+
+   msleep(1);
+   }
+
+   stk1160_err("AC97 transfer took too long, this should never happen!");
+   return -EBUSY;
+}
+
 static void stk1160_write_ac97(struct stk1160 *dev, u16 reg, u16 value)
 {
/* Set codec register address */
@@ -37,11 +56,11 @@ static void stk1160_write_ac97(struct stk1160 *dev, u16 
reg, u16 value)
stk1160_write_reg(dev, STK1160_AC97_CMD, value & 0xff);
stk1160_write_reg(dev, STK1160_AC97_CMD + 1, (value & 0xff00) >> 8);
 
-   /*
-* Set command write bit to initiate write operation.
-* The bit will be cleared when transfer is done.
-*/
+   /* Set command write bit to initiate write operation */
stk1160_write_reg(dev, STK1160_AC97CTL_0, 0x8c);
+
+   /* Wait for command write bit to be cleared */
+   stk1160_ac97_wait_transfer_complete(dev);
 }
 
 #ifdef DEBUG
@@ -53,12 +72,14 @@ static u16 stk1160_read_ac97(struct stk1160 *dev, u16 reg)
/* Set codec register address */
stk1160_write_reg(dev, STK1160_AC97_ADDR, reg);
 
-   /*
-* Set command read bit to initiate read operation.
-* The bit will be cleared when transfer is done.
-*/
+   /* Set command read bit to initiate read operation */
stk1160_write_reg(dev, STK1160_AC97CTL_0, 0x8b);
 
+   /* Wait for command read bit to be cleared */
+   if (stk1160_ac97_wait_transfer_complete(dev) < 0) {
+   return 0;
+   }
+
/* Retrieve register value */
stk1160_read_reg(dev, STK1160_AC97_CMD, );
stk1160_read_reg(dev, STK1160_AC97_CMD + 1, );
diff --git a/drivers/media/usb/stk1160/stk1160-reg.h 
b/drivers/media/usb/stk1160/stk1160-reg.h
index 296a9e7..7b08a3c 100644
--- a/drivers/media/usb/stk1160/stk1160-reg.h
+++ b/drivers/media/usb/stk1160/stk1160-reg.h
@@ -122,6 +122,8 @@
 /* AC97 Audio Control */
 #define STK1160_AC97CTL_0  0x500
 #define STK1160_AC97CTL_1  0x504
+#define  STK1160_AC97CTL_0_CR  BIT(1)
+#define  STK1160_AC97CTL_0_CW  BIT(2)
 
 /* Use [0:6] bits of register 0x504 to set codec command address */
 #define STK1160_AC97_ADDR  0x504
diff --git a/drivers/media/usb/stk1160/stk1160.h 
b/drivers/media/usb/stk1160/stk1160.h
index e85e12e..acd1c81 100644
--- a/drivers/media/usb/stk1160/stk1160.h
+++ b/drivers/media/usb/stk1160/stk1160.h
@@ -50,6 +50,8 @@
 #define STK1160_MAX_INPUT 4
 #define STK1160_SVIDEO_INPUT 4
 
+#define STK1160_AC97_TIMEOUT 50
+
 #define STK1160_I2C_TIMEOUT 100
 
 

On Tue, Dec 06, 2016 at 10:56:26AM -0200, Mauro Carvalho Chehab wrote:
> Em Mon, 5 Dec 2016 22:06:59 +0100
> Marcel Hasler  escreveu:
> 
> > Hello
> > 
> > 2016-12-05 16:38 GMT+01:00 Ezequiel Garcia :
> > > On 5 December 2016 at 09:12, Mauro Carvalho Chehab
> > >  wrote:  
> > >> Em Sun, 4 Dec 2016 15:25:25 -0300
> > >> Ezequiel Garcia  escreveu:
> > >>  
> > >>> On 4 December 2016 at 10:01, Marcel Hasler  wrote:  
> > >>> > Hello
> > >>> >
> > >>> > 2016-12-03 21:46 GMT+01:00 Ezequiel Garcia 
> > >>> > :  
> > >>> >> On 2 December 2016 at 08:05, Mauro Carvalho Chehab
> > >>> >>  wrote:  
> > >>> >>> Em Sun, 27 Nov 2016 12:11:48 +0100
> > >>> >>> Marcel Hasler  escreveu:
> > >>> >>>  
> > >>>  Allow setting a custom record gain for the internal AC97 codec (if 
> > >>>  available). This can be
> > >>>  a value between 0 and 15, 8 is the default and should be suitable 
> > >>>  for most users. The Windows
> > >>>  driver also sets this to 8 without any possibility for changing 
> > >>>  it.  
> > >>> >>>
> > >>> >>> The problem of removing the mixer is that you need this kind of
> > >>> >>> crap to setup the volumes on a non-standard way.
> > >>> >>>  
> > >>> >>
> > >>> >> Right, that's a good point.
> > >>> >>  
> > >>> >>> NACK.
> > >>> >>>
> > >>> >>> Instead, keep the alsa mixer. The way other drivers do (for example,
> > >>> >>> em28xx) is that they 

Re: [PATCH v3 3/4] stk1160: Add module param for setting the record gain.

[Marcel Hasler]

Hello

2016-12-06 13:56 GMT+01:00 Mauro Carvalho Chehab :
> Em Mon, 5 Dec 2016 22:06:59 +0100
> Marcel Hasler  escreveu:
>
>> Hello
>>
>> 2016-12-05 16:38 GMT+01:00 Ezequiel Garcia :
>> > On 5 December 2016 at 09:12, Mauro Carvalho Chehab
>> >  wrote:
>> >> Em Sun, 4 Dec 2016 15:25:25 -0300
>> >> Ezequiel Garcia  escreveu:
>> >>
>> >>> On 4 December 2016 at 10:01, Marcel Hasler  wrote:
>> >>> > Hello
>> >>> >
>> >>> > 2016-12-03 21:46 GMT+01:00 Ezequiel Garcia 
>> >>> > :
>> >>> >> On 2 December 2016 at 08:05, Mauro Carvalho Chehab
>> >>> >>  wrote:
>> >>> >>> Em Sun, 27 Nov 2016 12:11:48 +0100
>> >>> >>> Marcel Hasler  escreveu:
>> >>> >>>
>> >>>  Allow setting a custom record gain for the internal AC97 codec (if 
>> >>>  available). This can be
>> >>>  a value between 0 and 15, 8 is the default and should be suitable 
>> >>>  for most users. The Windows
>> >>>  driver also sets this to 8 without any possibility for changing it.
>> >>> >>>
>> >>> >>> The problem of removing the mixer is that you need this kind of
>> >>> >>> crap to setup the volumes on a non-standard way.
>> >>> >>>
>> >>> >>
>> >>> >> Right, that's a good point.
>> >>> >>
>> >>> >>> NACK.
>> >>> >>>
>> >>> >>> Instead, keep the alsa mixer. The way other drivers do (for example,
>> >>> >>> em28xx) is that they configure the mixer when an input is selected,
>> >>> >>> increasing the volume of the active audio channel to 100% and muting
>> >>> >>> the other audio channels. Yet, as the alsa mixer is exported, users
>> >>> >>> can change the mixer settings in runtime using some alsa (or pa)
>> >>> >>> mixer application.
>> >>> >>>
>> >>> >>
>> >>> >> Yeah, the AC97 mixer we are currently leveraging
>> >>> >> exposes many controls that have no meaning in this device,
>> >>> >> so removing that still looks like an improvement.
>> >>> >>
>> >>> >> I guess the proper way is creating our own mixer
>> >>> >> (not using snd_ac97_mixer)  exposing only the record
>> >>> >> gain knob.
>> >>> >>
>> >>> >> Marcel, what do you think?
>> >>> >> --
>> >>> >> Ezequiel García, VanguardiaSur
>> >>> >> www.vanguardiasur.com.ar
>> >>> >
>> >>> > As I have written before, the recording gain isn't actually meant to
>> >>> > be changed by the user. In the official Windows driver this value is
>> >>> > hard-coded to 8 and cannot be changed in any way. And there really is
>> >>> > no good reason why anyone should need to mess with it in the first
>> >>> > place. The default value will give the best results in pretty much all
>> >>> > cases and produces approximately the same volume as the internal 8-bit
>> >>> > ADC whose gain cannot be changed at all, not even by a driver.
>> >>> >
>> >>> > I had considered writing a mixer but chose not to. If the gain setting
>> >>> > is openly exposed to mixer applications, how do you tell the users
>> >>> > that the value set by the driver already is the optimal and
>> >>> > recommended value and that they shouldn't mess with the controls
>> >>> > unless they really have to? By having a module parameter, this setting
>> >>> > is practically hidden from the normal user but still is available to
>> >>> > power-users if they think they really need it. In the end it's really
>> >>> > just a compromise between hiding it completely and exposing it openly.
>> >>> > Also, this way the driver guarantees reproducible results, since
>> >>> > there's no need to remember the positions of any volume sliders.
>> >>> >
>> >>>
>> >>> Hm, right. I've never changed the record gain, and it's true that it
>> >>> doens't really improve the volume. So, I would be OK with having
>> >>> a module parameter.
>> >>>
>> >>> On the other side, we are exposing it currently, through the "Capture"
>> >>> mixer control:
>> >>>
>> >>> Simple mixer control 'Capture',0
>> >>>   Capabilities: cvolume cswitch cswitch-joined
>> >>>   Capture channels: Front Left - Front Right
>> >>>   Limits: Capture 0 - 15
>> >>>   Front Left: Capture 10 [67%] [15.00dB] [on]
>> >>>   Front Right: Capture 8 [53%] [12.00dB] [on]
>> >>>
>> >>> So, it would be user-friendly to keep the user interface and continue
>> >>> to expose the same knob - even if the default is the optimal, etc.
>> >>>
>> >>> To be completely honest, I don't think any user is really relying
>> >>> on any REC_GAIN / Capture setting, and I'm completely OK
>> >>> with having a mixer control or a module parameter. It doesn't matter.
>> >>
>> >> If you're positive that *all* stk1160 use the ac97 mixer the
>> >> same way, and that there's no sense on having a mixer for it,
>> >> then it would be ok to remove it.
>> >>
>> >
>> > Let's remove it then!
>> >
>> >> In such case, then why you need a modprobe parameter to allow
>> >> setting the record level? If this mixer entry 

[linux-next:master 7186/11555] htmldocs: Documentation/doc-guide/sphinx.rst:110: ERROR: Unknown target name: "sphinx c domain".

[kbuild test robot]

Hi Mauro,

FYI, the error/warning still remains.

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git 
master
head:   4a71e4389b1f8bbf02f43522c234143fd571dcb8
commit: 1dc4bbf0b268246f6202c761016735933b6f0b99 [7186/11555] docs-rst: 
doc-guide: split the kernel-documentation.rst contents
reproduce: make htmldocs

All errors (new ones prefixed by >>):

   make[3]: warning: jobserver unavailable: using -j1.  Add '+' to parent make 
rule.
   include/linux/init.h:1: warning: no structured comments found
   include/linux/kthread.h:26: warning: Excess function parameter '...' 
description in 'kthread_create'
   kernel/sys.c:1: warning: no structured comments found
   Error: Cannot open file drivers/dma-buf/dma-fence.c
   Error: Cannot open file drivers/dma-buf/dma-fence.c
   WARNING: kernel-doc 'scripts/kernel-doc -rst -enable-lineno -export 
drivers/dma-buf/dma-fence.c' failed with return code 2
   Error: Cannot open file include/linux/dma-fence.h
   Error: Cannot open file include/linux/dma-fence.h
   WARNING: kernel-doc 'scripts/kernel-doc -rst -enable-lineno -internal 
include/linux/dma-fence.h' failed with return code 2
   drivers/dma-buf/seqno-fence.c:1: warning: no structured comments found
   Error: Cannot open file drivers/dma-buf/dma-fence-array.c
   Error: Cannot open file drivers/dma-buf/dma-fence-array.c
   WARNING: kernel-doc 'scripts/kernel-doc -rst -enable-lineno -export 
drivers/dma-buf/dma-fence-array.c' failed with return code 2
   Error: Cannot open file include/linux/dma-fence-array.h
   Error: Cannot open file include/linux/dma-fence-array.h
   WARNING: kernel-doc 'scripts/kernel-doc -rst -enable-lineno -internal 
include/linux/dma-fence-array.h' failed with return code 2
   include/uapi/linux/vtpm_proxy.h:1: warning: no structured comments found
   drivers/char/tpm/tpm_vtpm_proxy.c:71: warning: No description found for 
parameter 'filp'
   drivers/char/tpm/tpm_vtpm_proxy.c:71: warning: No description found for 
parameter 'buf'
   drivers/char/tpm/tpm_vtpm_proxy.c:71: warning: No description found for 
parameter 'count'
   drivers/char/tpm/tpm_vtpm_proxy.c:71: warning: No description found for 
parameter 'off'
   drivers/char/tpm/tpm_vtpm_proxy.c:121: warning: No description found for 
parameter 'filp'
   drivers/char/tpm/tpm_vtpm_proxy.c:121: warning: No description found for 
parameter 'buf'
   drivers/char/tpm/tpm_vtpm_proxy.c:121: warning: No description found for 
parameter 'count'
   drivers/char/tpm/tpm_vtpm_proxy.c:121: warning: No description found for 
parameter 'off'
   drivers/char/tpm/tpm_vtpm_proxy.c:201: warning: No description found for 
parameter 'proxy_dev'
   drivers/char/tpm/tpm_vtpm_proxy.c:1: warning: no structured comments found
   include/sound/compress_driver.h:162: warning: No description found for 
parameter 'id[64]'
   include/sound/compress_driver.h:162: warning: No description found for 
parameter 'proc_root'
   include/sound/compress_driver.h:162: warning: No description found for 
parameter 'proc_info_entry'
   include/net/mac80211.h:3207: ERROR: Unexpected indentation.
   include/net/mac80211.h:3210: WARNING: Block quote ends without a blank line; 
unexpected unindent.
   include/net/mac80211.h:3212: ERROR: Unexpected indentation.
   include/net/mac80211.h:3213: WARNING: Block quote ends without a blank line; 
unexpected unindent.
   include/net/mac80211.h:1772: ERROR: Unexpected indentation.
   include/net/mac80211.h:1776: WARNING: Block quote ends without a blank line; 
unexpected unindent.
>> Documentation/doc-guide/sphinx.rst:110: ERROR: Unknown target name: "sphinx 
>> c domain".
   kernel/sched/fair.c:7259: WARNING: Inline emphasis start-string without 
end-string.
   kernel/time/timer.c:1240: ERROR: Unexpected indentation.
   kernel/time/timer.c:1242: ERROR: Unexpected indentation.
   kernel/time/timer.c:1243: WARNING: Block quote ends without a blank line; 
unexpected unindent.
   include/linux/wait.h:121: WARNING: Block quote ends without a blank line; 
unexpected unindent.
   include/linux/wait.h:124: ERROR: Unexpected indentation.
   include/linux/wait.h:126: WARNING: Block quote ends without a blank line; 
unexpected unindent.
   kernel/time/hrtimer.c:1021: WARNING: Block quote ends without a blank line; 
unexpected unindent.
   kernel/signal.c:317: WARNING: Inline literal start-string without end-string.
   drivers/base/firmware_class.c:1348: WARNING: Bullet list ends without a 
blank line; unexpected unindent.
   drivers/message/fusion/mptbase.c:5054: WARNING: Definition list ends without 
a blank line; unexpected unindent.
   drivers/tty/serial/serial_core.c:1893: WARNING: Definition list ends without 
a blank line; unexpected unindent.
   include/linux/spi/spi.h:369: ERROR: Unexpected indentation.
   drivers/usb/core/message.c:478: ERROR: Unexpected indentation.
   drivers/usb/core/message.c:479: WARNING: Block quote ends without a blank 
line; unexpected unindent.
   Documentation/driver-api/usb.rst:623: ERROR: Unknown 

[PATCH] media: platform: xilinx: xilinx-tpg: constify v4l2_subdev_* structures

[Bhumika Goyal]

v4l2_subdev_{core/pad/video}_ops structures are stored in the
fields of the v4l2_subdev_ops structure which are of type const. 
Also, v4l2_subdev_ops structure is passed to a function
having its argument of type const. As these structures are never
modified, so declare them as const.
Done using Coccinelle: (One of the scripts used)

@r1 disable optional_qualifier @
identifier i;
position p;
@@
static struct v4l2_subdev_video_ops i@p = {...};

@ok1@
identifier r1.i;
position p;
struct v4l2_subdev_ops obj;
@@
obj.video=@p;

@bad@
position p!={r1.p,ok1.p};
identifier r1.i;
@@
i@p

@depends on !bad disable optional_qualifier@
identifier r1.i;
@@
+const
struct v4l2_subdev_video_ops i; 

File size before:
   textdata bss dec hex filename
   61702752 1449066236a media/platform/xilinx/xilinx-tpg.o

File size after:
   textdata bss dec hex filename
   2384   890582362 media/platform/xilinx/xilinx-tpg.o

Signed-off-by: Bhumika Goyal 
---
 drivers/media/platform/xilinx/xilinx-tpg.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/media/platform/xilinx/xilinx-tpg.c 
b/drivers/media/platform/xilinx/xilinx-tpg.c
index 2ec1f6c..9c49d1d 100644
--- a/drivers/media/platform/xilinx/xilinx-tpg.c
+++ b/drivers/media/platform/xilinx/xilinx-tpg.c
@@ -460,21 +460,21 @@ static int xtpg_s_ctrl(struct v4l2_ctrl *ctrl)
.s_ctrl = xtpg_s_ctrl,
 };
 
-static struct v4l2_subdev_core_ops xtpg_core_ops = {
+static const struct v4l2_subdev_core_ops xtpg_core_ops = {
 };
 
-static struct v4l2_subdev_video_ops xtpg_video_ops = {
+static const struct v4l2_subdev_video_ops xtpg_video_ops = {
.s_stream = xtpg_s_stream,
 };
 
-static struct v4l2_subdev_pad_ops xtpg_pad_ops = {
+static const struct v4l2_subdev_pad_ops xtpg_pad_ops = {
.enum_mbus_code = xvip_enum_mbus_code,
.enum_frame_size= xtpg_enum_frame_size,
.get_fmt= xtpg_get_format,
.set_fmt= xtpg_set_format,
 };
 
-static struct v4l2_subdev_ops xtpg_ops = {
+static const struct v4l2_subdev_ops xtpg_ops = {
.core   = _core_ops,
.video  = _video_ops,
.pad= _pad_ops,
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] drivers: media: i2c: mt9t001: constify v4l2_subdev_* structures

[Bhumika Goyal]

v4l2_subdev_{core/pad/video}_ops structures are stored in the 
fields of the v4l2_subdev_ops structure which are of
type const. Also, v4l2_subdev_ops structure is passed to a function 
having its argument of type const. As these structures are never 
modified, so declare them as const.
Done using Coccinelle: (One of the scripts used)

@r1 disable optional_qualifier @
identifier i;
position p;
@@
static struct v4l2_subdev_ops i@p = {...};

@ok1@
identifier r1.i;
position p;
expression e1,e2;
@@
v4l2_i2c_subdev_init(e1,e2,@p)

@bad@
position p!={r1.p,ok1.p};
identifier r1.i;
@@
i@p

@depends on !bad disable optional_qualifier@
identifier r1.i;
@@
+const
struct v4l2_subdev_ops i;

File size before:
   textdata bss dec hex filename
   6119 736  1668711ad7 drivers/media/i2c/mt9t001.o

File size after:
   textdata bss dec hex filename
   6631 232  1668791adf drivers/media/i2c/mt9t001.o

Signed-off-by: Bhumika Goyal 
---
 drivers/media/i2c/mt9t001.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/media/i2c/mt9t001.c b/drivers/media/i2c/mt9t001.c
index 842017f..9d981d9 100644
--- a/drivers/media/i2c/mt9t001.c
+++ b/drivers/media/i2c/mt9t001.c
@@ -822,15 +822,15 @@ static int mt9t001_close(struct v4l2_subdev *subdev, 
struct v4l2_subdev_fh *fh)
return mt9t001_set_power(subdev, 0);
 }
 
-static struct v4l2_subdev_core_ops mt9t001_subdev_core_ops = {
+static const struct v4l2_subdev_core_ops mt9t001_subdev_core_ops = {
.s_power = mt9t001_set_power,
 };
 
-static struct v4l2_subdev_video_ops mt9t001_subdev_video_ops = {
+static const struct v4l2_subdev_video_ops mt9t001_subdev_video_ops = {
.s_stream = mt9t001_s_stream,
 };
 
-static struct v4l2_subdev_pad_ops mt9t001_subdev_pad_ops = {
+static const struct v4l2_subdev_pad_ops mt9t001_subdev_pad_ops = {
.enum_mbus_code = mt9t001_enum_mbus_code,
.enum_frame_size = mt9t001_enum_frame_size,
.get_fmt = mt9t001_get_format,
@@ -839,7 +839,7 @@ static int mt9t001_close(struct v4l2_subdev *subdev, struct 
v4l2_subdev_fh *fh)
.set_selection = mt9t001_set_selection,
 };
 
-static struct v4l2_subdev_ops mt9t001_subdev_ops = {
+static const struct v4l2_subdev_ops mt9t001_subdev_ops = {
.core = _subdev_core_ops,
.video = _subdev_video_ops,
.pad = _subdev_pad_ops,
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html