[PATCH] udmabuf: fix odd_ptr_err.cocci warnings

2018-05-25 Thread Julia Lawall
From: kbuild test robot 

drivers/dma-buf/udmabuf.c:167:6-12: inconsistent IS_ERR and PTR_ERR on line 168.

 PTR_ERR should access the value just tested by IS_ERR

Semantic patch information:
 There can be false positives in the patch case, where it is the call to
 IS_ERR that is wrong.

Generated by: scripts/coccinelle/tests/odd_ptr_err.cocci

Fixes: cc2d0e91bc15 ("udmabuf: driver update")
Signed-off-by: kbuild test robot 
Signed-off-by: linux-ker...@vger.kernel.org
---

tree:   git://git.kraxel.org/linux udmabuf
head:   cc2d0e91bc15849baff695d175bfb8fba35f1465
commit: cc2d0e91bc15849baff695d175bfb8fba35f1465 [6/6] udmabuf: driver
update

 udmabuf.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- a/drivers/dma-buf/udmabuf.c
+++ b/drivers/dma-buf/udmabuf.c
@@ -165,7 +165,7 @@ static long udmabuf_ioctl_create(struct
page = shmem_read_mapping_page(
file_inode(ubuf->filp)->i_mapping, pgoff + pgidx);
if (IS_ERR(page)) {
-   ret = PTR_ERR(buf);
+   ret = PTR_ERR(page);
goto err_put_pages;
}
ubuf->pages[pgidx] = page;


Re: [PATCH 2/3] media: staging: atomisp: Fix an error handling path in 'lm3554_probe()'

2018-05-11 Thread Julia Lawall


On Fri, 11 May 2018, Christophe JAILLET wrote:

> The use of 'fail1' and 'fail2' is not correct. Reorder these calls to
> branch at the right place of the error handling path.

Maybe it would be good to improve the names at the same time?

julia

>
> Signed-off-by: Christophe JAILLET <christophe.jail...@wanadoo.fr>
> ---
>  drivers/staging/media/atomisp/i2c/atomisp-lm3554.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/staging/media/atomisp/i2c/atomisp-lm3554.c 
> b/drivers/staging/media/atomisp/i2c/atomisp-lm3554.c
> index 723fa74ff815..1e5f516f6e50 100644
> --- a/drivers/staging/media/atomisp/i2c/atomisp-lm3554.c
> +++ b/drivers/staging/media/atomisp/i2c/atomisp-lm3554.c
> @@ -871,7 +871,7 @@ static int lm3554_probe(struct i2c_client *client)
>ARRAY_SIZE(lm3554_controls));
>   if (err) {
>   dev_err(>dev, "error initialize a ctrl_handler.\n");
> - goto fail2;
> + goto fail1;
>   }
>
>   for (i = 0; i < ARRAY_SIZE(lm3554_controls); i++)
> @@ -879,7 +879,6 @@ static int lm3554_probe(struct i2c_client *client)
>NULL);
>
>   if (flash->ctrl_handler.error) {
> -
>   dev_err(>dev, "ctrl_handler error.\n");
>   goto fail2;
>   }
> @@ -888,7 +887,7 @@ static int lm3554_probe(struct i2c_client *client)
>   err = media_entity_pads_init(>sd.entity, 0, NULL);
>   if (err) {
>   dev_err(>dev, "error initialize a media entity.\n");
> - goto fail1;
> + goto fail2;
>   }
>
>   flash->sd.entity.function = MEDIA_ENT_F_FLASH;
> --
> 2.17.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>


[PATCH v2] [media] pvrusb2: delete unneeded include

2018-05-06 Thread Julia Lawall
pvrusb2-video-v4l.h only declares pvr2_saa7115_subdev_update and
includes pvrusb2-hdw-internal.h.  pvrusb2-cx2584x-v4l.c does not
use pvr2_saa7115_subdev_update and it explicitly includes
pvrusb2-hdw-internal.h.

Signed-off-by: Julia Lawall <julia.law...@lip6.fr>

---

v2: Make the subject line a bit less generic

 drivers/media/usb/pvrusb2/pvrusb2-cx2584x-v4l.c |1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/media/usb/pvrusb2/pvrusb2-cx2584x-v4l.c 
b/drivers/media/usb/pvrusb2/pvrusb2-cx2584x-v4l.c
index 242b213..d5bec0f 100644
--- a/drivers/media/usb/pvrusb2/pvrusb2-cx2584x-v4l.c
+++ b/drivers/media/usb/pvrusb2/pvrusb2-cx2584x-v4l.c
@@ -23,7 +23,6 @@
 */

 #include "pvrusb2-cx2584x-v4l.h"
-#include "pvrusb2-video-v4l.h"


 #include "pvrusb2-hdw-internal.h"

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


[PATCH] [media] media: delete unneeded include

2018-05-06 Thread Julia Lawall
pvrusb2-video-v4l.h only declares pvr2_saa7115_subdev_update and
includes pvrusb2-hdw-internal.h.  pvrusb2-cx2584x-v4l.c does not
use pvr2_saa7115_subdev_update and it explicitly includes
pvrusb2-hdw-internal.h.

Signed-off-by: Julia Lawall <julia.law...@lip6.fr>

---
 drivers/media/usb/pvrusb2/pvrusb2-cx2584x-v4l.c |1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/media/usb/pvrusb2/pvrusb2-cx2584x-v4l.c 
b/drivers/media/usb/pvrusb2/pvrusb2-cx2584x-v4l.c
index 242b213..d5bec0f 100644
--- a/drivers/media/usb/pvrusb2/pvrusb2-cx2584x-v4l.c
+++ b/drivers/media/usb/pvrusb2/pvrusb2-cx2584x-v4l.c
@@ -23,7 +23,6 @@
 */
 
 #include "pvrusb2-cx2584x-v4l.h"
-#include "pvrusb2-video-v4l.h"
 
 
 #include "pvrusb2-hdw-internal.h"



Re: [PATCH][RFC] kernel.h: provide array iterator

2018-03-16 Thread Julia Lawall

Le 16.03.2018 05:21, Kees Cook a écrit :

On Thu, Mar 15, 2018 at 3:00 AM, Kieran Bingham
<kieran.bing...@ideasonboard.com> wrote:
Simplify array iteration with a helper to iterate each entry in an 
array.
Utilise the existing ARRAY_SIZE macro to identify the length of the 
array

and pointer arithmetic to process each item as a for loop.

Signed-off-by: Kieran Bingham <kieran.bing...@ideasonboard.com>
---
 include/linux/kernel.h | 10 ++
 1 file changed, 10 insertions(+)

The use of static arrays to store data is a common use case throughout 
the

kernel. Along with that is the obvious need to iterate that data.

In fact there are just shy of 5000 instances of iterating a static 
array:

git grep "for .*ARRAY_SIZE" | wc -l
4943


I suspect the main question is "Does this macro make the code easier to 
read?"


I think it does, and we have other kinds of iterators like this in the
kernel already. Would it be worth building a Coccinelle script to do
the 5000 replacements?



Coccinelle should be able to replace the for loop header.  Coccinelle
could create the local macro.  Coccinelle might not put the definition 
in

exactly the right place.  Before the function of the first use would be
possible, or before any function.

I don't think that Coccinelle could figure out how to split one loop 
into

two as done here, unless that specific pattern is very common.  I guess
that the split is to add the flush_workqueue, and is not the main goal?

julia





-Kees



When working on the UVC driver - I found that I needed to split one 
such

iteration into two parts, and at the same time felt that this could be
refactored to be cleaner / easier to read.

I do however worry that this simple short patch might not be desired 
or could
also be heavily bikeshedded due to it's potential wide spread use 
(though
perhaps that would be a good thing to have more users) ...  but here 
it is,

along with an example usage below which is part of a separate series.

The aim is to simplify iteration on static arrays, in the same way 
that we have

iterators for lists. The use of the ARRAY_SIZE macro, provides all the
protections given by "__must_be_array(arr)" to this macro too.

Regards

Kieran

=
Example Usage from a pending UVC development:

+#define for_each_uvc_urb(uvc_urb, uvc_streaming) \
+   for_each_array_element(uvc_urb, uvc_streaming->uvc_urb)

 /*
  * Uninitialize isochronous/bulk URBs and free transfer buffers.
  */
 static void uvc_uninit_video(struct uvc_streaming *stream, int 
free_buffers)

 {
-   struct urb *urb;
-   unsigned int i;
+   struct uvc_urb *uvc_urb;

uvc_video_stats_stop(stream);

-   for (i = 0; i < UVC_URBS; ++i) {
-   struct uvc_urb *uvc_urb = >uvc_urb[i];
+   for_each_uvc_urb(uvc_urb, stream)
+   usb_kill_urb(uvc_urb->urb);

-   urb = uvc_urb->urb;
-   if (urb == NULL)
-   continue;
+   flush_workqueue(stream->async_wq);

-   usb_kill_urb(urb);
-   usb_free_urb(urb);
+   for_each_uvc_urb(uvc_urb, stream) {
+   usb_free_urb(uvc_urb->urb);
uvc_urb->urb = NULL;
}

if (free_buffers)
uvc_free_urb_buffers(stream);
 }
=




diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index ce51455e2adf..95d7dae248b7 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -70,6 +70,16 @@
  */
 #define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]) + 
__must_be_array(arr))


+/**
+ * for_each_array_element - Iterate all items in an array
+ * @elem: pointer of array type for iteration cursor
+ * @array: array to be iterated
+ */
+#define for_each_array_element(elem, array) \
+   for (elem = &(array)[0]; \
+elem < &(array)[ARRAY_SIZE(array)]; \
+++elem)
+
 #define u64_to_user_ptr(x) (   \
 {  \
typecheck(u64, x);  \
--
2.7.4



Re: [Outreachy kernel] [PATCH 0/3] staging: media: cleanup

2018-03-04 Thread Julia Lawall


On Sun, 4 Mar 2018, Arushi Singhal wrote:

> Spellcheck the comments.
> Remove the repeated, consecutive words with single word.

For the series:
Acked-by: Julia Lawall <julia.law...@lip6.fr>

But please look out for things to change in the code, not just in the
comments.

julia


>
> Arushi Singhal (3):
>   staging: media: Replace "be be" with "be"
>   staging: media: Replace "cant" with "can't"
>   staging: media: Replace "dont" with "don't"
>
>  drivers/staging/media/atomisp/pci/atomisp2/css2400/ia_css_pipe_public.h | 2 
> +-
>  drivers/staging/media/atomisp/pci/atomisp2/mmu/isp_mmu.c| 2 
> +-
>  drivers/staging/media/davinci_vpfe/vpfe_mc_capture.c| 2 
> +-
>  3 files changed, 3 insertions(+), 3 deletions(-)
>
> --
> 2.7.4
>
> --
> You received this message because you are subscribed to the Google Groups 
> "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an 
> email to outreachy-kernel+unsubscr...@googlegroups.com.
> To post to this group, send email to outreachy-ker...@googlegroups.com.
> To view this discussion on the web visit 
> https://groups.google.com/d/msgid/outreachy-kernel/1520178507-25141-1-git-send-email-arushisinghal19971997%40gmail.com.
> For more options, visit https://groups.google.com/d/optout.
>


Re: [Outreachy kernel] [PATCH] staging: media: Remove unnecessary semicolon

2018-03-03 Thread Julia Lawall


On Sun, 4 Mar 2018, Arushi Singhal wrote:

> Remove unnecessary semicolon found using semicolon.cocci Coccinelle
> script.
>
> Signed-off-by: Arushi Singhal <arushisinghal19971...@gmail.com>

Acked-by: Julia Lawall <julia.law...@lip6.fr>


> ---
>  .../media/atomisp/pci/atomisp2/css2400/runtime/frame/src/frame.c| 2 
> +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git 
> a/drivers/staging/media/atomisp/pci/atomisp2/css2400/runtime/frame/src/frame.c
>  
> b/drivers/staging/media/atomisp/pci/atomisp2/css2400/runtime/frame/src/frame.c
> index 5faa89a..7562bea 100644
> --- 
> a/drivers/staging/media/atomisp/pci/atomisp2/css2400/runtime/frame/src/frame.c
> +++ 
> b/drivers/staging/media/atomisp/pci/atomisp2/css2400/runtime/frame/src/frame.c
> @@ -196,7 +196,7 @@ enum ia_css_err ia_css_frame_map(struct ia_css_frame 
> **frame,
> attribute, context);
>   if (me->data == mmgr_NULL)
>   err = IA_CSS_ERR_INVALID_ARGUMENTS;
> - };
> + }
>
>   if (err != IA_CSS_SUCCESS) {
>   sh_css_free(me);
> --
> 2.7.4
>
> --
> You received this message because you are subscribed to the Google Groups 
> "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an 
> email to outreachy-kernel+unsubscr...@googlegroups.com.
> To post to this group, send email to outreachy-ker...@googlegroups.com.
> To view this discussion on the web visit 
> https://groups.google.com/d/msgid/outreachy-kernel/20180303192629.GA5198%40seema-Inspiron-15-3567.
> For more options, visit https://groups.google.com/d/optout.
>


Re: [PATCH 0/4] tree-wide: fix comparison to bitshift when dealing with a mask

2018-02-06 Thread Julia Lawall


On Tue, 6 Feb 2018, Dan Carpenter wrote:

> That found 4 that I think Wolfram's grep missed.
>
>  arch/um/drivers/vector_user.h |2 --
>  drivers/gpu/drm/mxsfb/mxsfb_regs.h|2 --
>  drivers/video/fbdev/mxsfb.c   |2 --
>  include/drm/drm_scdc_helper.h |3 ---
>
> But it didn't find the two bugs that Geert found where the right side
> wasn't a number literal.
>
> drivers/net/can/m_can/m_can.c:#define RXFC_FWM_MASK (0x7f < 
> RXFC_FWM_SHIFT)

OK, I can easily add this in - I've got rules to protect against reporting
it at the moment.  It may end up with false positives.

> drivers/usb/gadget/udc/goku_udc.h:#define INT_EPnNAK(n) (0x00100 < (n))   
>   /* 0 < n < 4 */

This is indeed harder, because one has to look at the usage site.

julia


Re: [PATCH 0/4] tree-wide: fix comparison to bitshift when dealing with a mask

2018-02-06 Thread Julia Lawall


On Tue, 6 Feb 2018, Wolfram Sang wrote:

> Hi Julia,
>
> > and got the results below.  I can make a version for the kernel shortly.
>
> It should probably take care of right-shifting, too?

I did that too but got no results.  Perhaps right shifting constants is
pretty uncommon.  I can put that in the complete rule though.

julia


Re: [PATCH 0/4] tree-wide: fix comparison to bitshift when dealing with a mask

2018-02-06 Thread Julia Lawall


On Tue, 6 Feb 2018, Dan Carpenter wrote:

> On Tue, Feb 06, 2018 at 02:15:51PM +0100, Julia Lawall wrote:
> >
> >
> > On Tue, 6 Feb 2018, Dan Carpenter wrote:
> >
> > > On Mon, Feb 05, 2018 at 09:09:57PM +0100, Wolfram Sang wrote:
> > > > In one Renesas driver, I found a typo which turned an intended bit 
> > > > shift ('<<')
> > > > into a comparison ('<'). Because this is a subtle issue, I looked tree 
> > > > wide for
> > > > similar patterns. This small patch series is the outcome.
> > > >
> > > > Buildbot and checkpatch are happy. Only compile-tested. To be applied
> > > > individually per sub-system, I think. I'd think only the net: amd: 
> > > > patch needs
> > > > to be conisdered for stable, but I leave this to people who actually 
> > > > know this
> > > > driver.
> > > >
> > > > CCing Dan. Maybe he has an idea how to add a test to smatch? In my 
> > > > setup, only
> > > > cppcheck reported a 'coding style' issue with a low prio.
> > > >
> > >
> > > Most of these are inside macros so it makes it complicated for Smatch
> > > to warn about them.  It might be easier in Coccinelle.  Julia the bugs
> > > look like this:
> > >
> > > - reissue_mask |= 0x < 4;
> > > + reissue_mask |= 0x << 4;
> >
> > Thanks.  I'll take a look.  Do you have an example of the macro issue
> > handy?
> >
>
> It's the same:
>
> #define EXYNOS_CIIMGEFF_PAT_CBCR_MASK  ((0xff < 13) | (0xff < 0))
>
> Smatch only sees the outside of the macro (where it is used in the code)
> and the pre-processed code.

I wrote the following rule:

@@
constant int x,y;
identifier i;
type T;
@@

(
i < x
|
x < i
|
 (T)i < x
|
x < (T)i
|
* x < y
)

and got the results below.  I can make a version for the kernel shortly.

julia

diff -u -p /run/shm/linux-next/drivers/gpu/drm/exynos/regs-fimc.h 
/tmp/nothing/drivers/gpu/drm/exynos/regs-fimc.h
--- /run/shm/linux-next/drivers/gpu/drm/exynos/regs-fimc.h
+++ /tmp/nothing/drivers/gpu/drm/exynos/regs-fimc.h
@@ -569,7 +569,6 @@
 #define EXYNOS_CIIMGEFF_FIN_EMBOSSING  (4 << 26)
 #define EXYNOS_CIIMGEFF_FIN_SILHOUETTE (5 << 26)
 #define EXYNOS_CIIMGEFF_FIN_MASK   (7 << 26)
-#define EXYNOS_CIIMGEFF_PAT_CBCR_MASK  ((0xff < 13) | (0xff < 0))

 /* Real input DMA size register */
 #define EXYNOS_CIREAL_ISIZE_AUTOLOAD_ENABLE(1 << 31)
diff -u -p /run/shm/linux-next/drivers/net/ethernet/amd/xgbe/xgbe-drv.c 
/tmp/nothing/drivers/net/ethernet/amd/xgbe/xgbe-drv.c
--- /run/shm/linux-next/drivers/net/ethernet/amd/xgbe/xgbe-drv.c
+++ /tmp/nothing/drivers/net/ethernet/amd/xgbe/xgbe-drv.c
@@ -595,7 +595,6 @@ isr_done:

reissue_mask = 1 << 0;
if (!pdata->per_channel_irq)
-   reissue_mask |= 0x < 4;

XP_IOWRITE(pdata, XP_INT_REISSUE_EN, reissue_mask);
}
diff -u -p /run/shm/linux-next/drivers/video/fbdev/mxsfb.c 
/tmp/nothing/drivers/video/fbdev/mxsfb.c
--- /run/shm/linux-next/drivers/video/fbdev/mxsfb.c
+++ /tmp/nothing/drivers/video/fbdev/mxsfb.c
@@ -133,8 +133,6 @@
 #define VDCTRL4_SYNC_SIGNALS_ON(1 << 18)
 #define SET_DOTCLK_H_VALID_DATA_CNT(x) ((x) & 0x3)

-#define DEBUG0_HSYNC   (1 < 26)
-#define DEBUG0_VSYNC   (1 < 25)

 #define MIN_XRES   120
 #define MIN_YRES   120
diff -u -p /run/shm/linux-next/include/drm/drm_scdc_helper.h 
/tmp/nothing/include/drm/drm_scdc_helper.h
--- /run/shm/linux-next/include/drm/drm_scdc_helper.h
+++ /tmp/nothing/include/drm/drm_scdc_helper.h
@@ -50,9 +50,6 @@
 #define  SCDC_READ_REQUEST_ENABLE (1 << 0)

 #define SCDC_STATUS_FLAGS_0 0x40
-#define  SCDC_CH2_LOCK (1 < 3)
-#define  SCDC_CH1_LOCK (1 < 2)
-#define  SCDC_CH0_LOCK (1 < 1)
 #define  SCDC_CH_LOCK_MASK (SCDC_CH2_LOCK | SCDC_CH1_LOCK | SCDC_CH0_LOCK)
 #define  SCDC_CLOCK_DETECT (1 << 0)

diff -u -p /run/shm/linux-next/arch/um/drivers/vector_user.h 
/tmp/nothing/arch/um/drivers/vector_user.h
--- /run/shm/linux-next/arch/um/drivers/vector_user.h
+++ /tmp/nothing/arch/um/drivers/vector_user.h
@@ -61,8 +61,6 @@ struct vector_fds {
 };

 #define VECTOR_READ1
-#define VECTOR_WRITE   (1 < 1)
-#define VECTOR_HEADERS (1 < 2)

 extern struct arglist *uml_parse_vector_ifspec(char *arg);

diff -u -p /run/shm/linux-next/drivers/gpu/drm/mxsfb/mxsfb_regs.h 
/tmp/nothing/drivers/gpu/drm/mxsfb/mxsfb_regs.h
--- /run/shm/linux-next/drivers/gpu/drm/mxsfb/mxsfb_regs.h
+++ /tmp/nothing/drivers/gpu/drm/mxsfb/mxsfb_regs.h
@@ -

Re: [PATCH 0/4] tree-wide: fix comparison to bitshift when dealing with a mask

2018-02-06 Thread Julia Lawall


On Tue, 6 Feb 2018, Dan Carpenter wrote:

> On Mon, Feb 05, 2018 at 09:09:57PM +0100, Wolfram Sang wrote:
> > In one Renesas driver, I found a typo which turned an intended bit shift 
> > ('<<')
> > into a comparison ('<'). Because this is a subtle issue, I looked tree wide 
> > for
> > similar patterns. This small patch series is the outcome.
> >
> > Buildbot and checkpatch are happy. Only compile-tested. To be applied
> > individually per sub-system, I think. I'd think only the net: amd: patch 
> > needs
> > to be conisdered for stable, but I leave this to people who actually know 
> > this
> > driver.
> >
> > CCing Dan. Maybe he has an idea how to add a test to smatch? In my setup, 
> > only
> > cppcheck reported a 'coding style' issue with a low prio.
> >
>
> Most of these are inside macros so it makes it complicated for Smatch
> to warn about them.  It might be easier in Coccinelle.  Julia the bugs
> look like this:
>
> - reissue_mask |= 0x < 4;
> + reissue_mask |= 0x << 4;

Thanks.  I'll take a look.  Do you have an example of the macro issue
handy?

julia

>
> regards,
> dan carpenter
>
> > Wolfram Sang (4):
> >   v4l: vsp1: fix mask creation for MULT_ALPHA_RATIO
> >   drm/exynos: fix comparison to bitshift when dealing with a mask
> >   v4l: dvb-frontends: stb0899: fix comparison to bitshift when dealing
> > with a mask
> >   net: amd-xgbe: fix comparison to bitshift when dealing with a mask
> >
> >  drivers/gpu/drm/exynos/regs-fimc.h| 2 +-
> >  drivers/media/dvb-frontends/stb0899_reg.h | 8 
> >  drivers/media/platform/vsp1/vsp1_regs.h   | 2 +-
> >  drivers/net/ethernet/amd/xgbe/xgbe-drv.c  | 2 +-
> >  4 files changed, 7 insertions(+), 7 deletions(-)
> >
> > --
> > 2.11.0
>


Re: [Cluster-devel] [PATCH 00/12] drop unneeded newline

2018-01-02 Thread Julia Lawall


On Tue, 2 Jan 2018, Bart Van Assche wrote:

> On Tue, 2018-01-02 at 15:00 +0100, Julia Lawall wrote:
> > On Tue, 2 Jan 2018, Bob Peterson wrote:
> > > - Original Message -
> > > > - Original Message -
> > > >
> > > Still, the GFS2 and DLM code has a plethora of broken-up printk messages,
> > > and I don't like the thought of re-combining them all.
> >
> > Actually, the point of the patch was to remove the unnecessary \n at the
> > end of the string, because log_print will add another one.  If you prefer
> > to keep the string broken up, I can resend the patch in that form, but
> > without the unnecessary \n.
>
> Please combine any user-visible strings into a single line for which the
> unneeded newline is dropped since these strings are modified anyway by
> your patch.

That is what the submitted patch (2/12 specifically) did.

julia


Re: [Cluster-devel] [PATCH 00/12] drop unneeded newline

2018-01-02 Thread Julia Lawall


On Tue, 2 Jan 2018, Bob Peterson wrote:

> - Original Message -
> | - Original Message -
> | | Drop newline at the end of a message string when the printing function 
> adds
> | | a newline.
> |
> | Hi Julia,
> |
> | NACK.
> |
> | As much as it's a pain when searching the source code for output strings,
> | this patch set goes against the accepted Linux coding style document. See:
> |
> | 
> https://www.kernel.org/doc/html/v4.10/process/coding-style.html#breaking-long-lines-and-strings
> |
> | Regards,
> |
> | Bob Peterson
> |
> |
> Hm. I guess I stand corrected. The document reads:
>
> "However, never break user-visible strings such as printk messages, because 
> that breaks the ability to grep for them."
>
> Still, the GFS2 and DLM code has a plethora of broken-up printk messages,
> and I don't like the thought of re-combining them all.

Actually, the point of the patch was to remove the unnecessary \n at the
end of the string, because log_print will add another one.  If you prefer
to keep the string broken up, I can resend the patch in that form, but
without the unnecessary \n.

julia


Re: [Cluster-devel] [PATCH 00/12] drop unneeded newline

2018-01-02 Thread Julia Lawall


On Tue, 2 Jan 2018, Bob Peterson wrote:

> - Original Message -
> | Drop newline at the end of a message string when the printing function adds
> | a newline.
>
> Hi Julia,
>
> NACK.
>
> As much as it's a pain when searching the source code for output strings,
> this patch set goes against the accepted Linux coding style document. See:
>
> https://www.kernel.org/doc/html/v4.10/process/coding-style.html#breaking-long-lines-and-strings

I don't think that's the case:

"However, never break user-visible strings such as printk messages,
because that breaks the ability to grep for them."

julia

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


Re: [PATCH v1 05/10] staging: atomisp: Remove non-ACPI leftovers

2018-01-02 Thread Julia Lawall


On Tue, 2 Jan 2018, Dan Carpenter wrote:

> On Wed, Dec 20, 2017 at 11:30:01AM +0100, Julia Lawall wrote:
> >
> >
> > On Wed, 20 Dec 2017, Dan Carpenter wrote:
> >
> > > On Tue, Dec 19, 2017 at 10:59:52PM +0200, Andy Shevchenko wrote:
> > > > @@ -914,9 +904,7 @@ static int lm3554_probe(struct i2c_client *client)
> > > > dev_err(>dev, "gpio request/direction_output 
> > > > fail");
> > > > goto fail2;
> > > > }
> > > > -   if (ACPI_HANDLE(>dev))
> > > > -   err = atomisp_register_i2c_module(>sd, NULL, 
> > > > LED_FLASH);
> > > > -   return 0;
> > > > +   return atomisp_register_i2c_module(>sd, NULL, LED_FLASH);
> > > >  fail2:
> > > > media_entity_cleanup(>sd.entity);
> > > > v4l2_ctrl_handler_free(>ctrl_handler);
> > >
> > > Actually every place where we directly return a function call is wrong
> > > and needs error handling added.  I've been meaning to write a Smatch
> > > check for this because it's a common anti-pattern we don't check the
> > > last function call for errors.
> > >
> > > Someone could probably do the same in Coccinelle if they want.
> >
> > I'm not sure what you are suggesting.  Is every case of return f(...);
> > for any f wrong?  Or is it a particular function that is of concern?  Or
> > would it be that every function call that has error handling somewhere
> > should have error handling everywhere?  Or is it related to what seems to
> > be the problem in the above code that err is initialized but nothing
> > happens to it?
> >
>
> I was just thinking that it's a common pattern to treat the last
> function call differently and one mistake I often see looks like this:
>
>   ret = frob();
>   if (ret) {
>   cleanup();
>   return ret;
>   }
>
>   return another_function();
>
> No error handling for the last function call.

OK, I see.  When there was error handling code along the way, a direct
return of a function that could fail needs error handling code too.

Thanks for the clarification,

julia


[PATCH 00/12] drop unneeded newline

2017-12-27 Thread Julia Lawall
Drop newline at the end of a message string when the printing function adds
a newline.

The complete semantic patch that detects this issue is as shown below
(http://coccinelle.lip6.fr/).  It works in two phases - the first phase
counts how many uses of a function involve a newline and how many don't,
and then the second phase removes newlines in the case of calls where a
newline is used one fourth of the times or less.

This approach is only moderately reliable, and all patches have been
checked to ensure that the newline is not needed.

This also converts some cases of string concatenation to single strings in
modified code, as this improves greppability.

// 
virtual after_start

@initialize:ocaml@
@@

let withnl = Hashtbl.create 101
let withoutnl = Hashtbl.create 101

let ignore =
  ["strcpy";"strlcpy";"strcat";"strlcat";"strcmp";"strncmp";"strcspn";
"strsep";"sprintf";"printf";"strncasecmp";"seq_printf";"strstr";"strspn";
"strlen";"strpbrk";"strtok_r";"memcmp";"memcpy"]

let dignore = ["tools";"samples"]

let inc tbl k =
  let cell =
try Hashtbl.find tbl k
with Not_found ->
  let cell = ref 0 in
  Hashtbl.add tbl k cell;
  cell in
  cell := 1 + !cell

let endnl c =
  let len = String.length c in
  try
String.get c (len-3) = '\\' && String.get c (len-2) = 'n' &&
String.get c (len-1) = '"'
  with _ -> false

let clean_string s extra =
  let pieces = Str.split (Str.regexp "\" \"") s in
  let nonempty s =
not (s = "") && String.get s 0 = '"' && not (String.get s 1 = '"') in
  let rec loop = function
  [] -> []
| [x] -> [x]
| x::y::rest ->
if nonempty x && nonempty y
then
  let xend = String.get x (String.length x - 2) = ' ' in
  let yend = String.get y 1 = ' ' in
  match (xend,yend) with
(true,false) | (false,true) -> x :: (loop (y::rest))
  | (true,true) ->
  x :: (loop (((String.sub y 0 (String.length y - 2))^"\"")::rest))
  | (false,false) ->
  ((String.sub x 0 (String.length x - 1)) ^ " \"") ::
  (loop (y::rest))
else x :: (loop (y::rest)) in
  (String.concat "" (loop pieces))^extra

@r depends on !after_start@
constant char[] c;
expression list[n] es;
identifier f;
position p;
@@

f@p(es,c,...)

@script:ocaml@
f << r.f;
n << r.n;
p << r.p;
c << r.c;
@@

let pieces = Str.split (Str.regexp "/") (List.hd p).file in
if not (List.mem f ignore) &&
  List.for_all (fun x -> not (List.mem x pieces)) dignore
then
  (if endnl c
  then inc withnl (f,n)
  else inc withoutnl (f,n))

@finalize:ocaml depends on !after_start@
w1 << merge.withnl;
w2 << merge.withoutnl;
@@

let names = ref [] in
let incn tbl k v =
  let cell =
try Hashtbl.find tbl k
with Not_found ->
  begin
let cell = ref 0 in
Hashtbl.add tbl k cell;
cell
  end in
  (if not (List.mem k !names) then names := k :: !names);
  cell := !v + !cell in
List.iter (function w -> Hashtbl.iter (incn withnl) w) w1;
List.iter (function w -> Hashtbl.iter (incn withoutnl) w) w2;

List.iter
  (function name ->
let wth = try !(Hashtbl.find withnl name) with _ -> 0 in
let wo = try !(Hashtbl.find withoutnl name) with _ -> 0 in
if wth > 0 && wth <= wo / 3 then Hashtbl.remove withnl name
else (Printf.eprintf "dropping %s %d %d\n" (fst name) wth wo; 
Hashtbl.remove withoutnl name; Hashtbl.remove withnl name))
  !names;

let it = new iteration() in
it#add_virtual_rule After_start;
it#register()

@s1 depends on after_start@
constant char[] c;
expression list[n] es;
identifier f;
position p;
@@

f(es,c@p,...)

@script:ocaml s2@
f << s1.f;
n << s1.n;
c << s1.c;
newc;
@@

try
  let _ = Hashtbl.find withnl (f,n) in
  if endnl c
  then Coccilib.include_match false
  else newc :=
make_expr(clean_string (String.sub c 0 (String.length c - 1)) "\\n\"")
with Not_found ->
try
  let _ = Hashtbl.find withoutnl (f,n) in
  if endnl c
  then newc :=
make_expr(clean_string (String.sub c 0 (String.length c - 3)) "\"")
  else Coccilib.include_match false
with Not_found -> Coccilib.include_match false

@@
constant char[] s1.c;
position s1.p;
expression s2.newc;
@@

- c@p
+ newc
// 

---

 arch/arm/mach-davinci/board-da850-evm.c |4 ++--
 drivers/block/DAC960.c  |4 ++--
 drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c|   12 
 drivers/gpu/drm/amd/powerplay/smumgr/fiji_smumgr.c  |2 +-
 drivers/gpu/drm/amd/powerplay/smumgr/iceland_smumgr.c   |2 +-
 drivers/gpu/drm/amd/powerplay/smumgr/polaris10_smumgr.c |2 +-
 drivers/gpu/drm/amd/powerplay/smumgr/tonga_smumgr.c |2 +-
 drivers/media/usb/pvrusb2/pvrusb2-hdw.c |3 ++-
 drivers/s390/block/dasd_diag.c  |3 +--
 drivers/scsi/hpsa.c |2 +-
 fs/dlm/plock.c  |3 +--
 fs/ext2/super.c 

[PATCH 09/12] [media] pvrusb2: drop unneeded newline

2017-12-27 Thread Julia Lawall
pvr2_trace prints a newline at the end of the message string, so the
message string does not need to include a newline explicitly.  Done
using Coccinelle.

Signed-off-by: Julia Lawall <julia.law...@lip6.fr>

---
 drivers/media/usb/pvrusb2/pvrusb2-hdw.c |3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/media/usb/pvrusb2/pvrusb2-hdw.c 
b/drivers/media/usb/pvrusb2/pvrusb2-hdw.c
index 09bd6c6..e035316 100644
--- a/drivers/media/usb/pvrusb2/pvrusb2-hdw.c
+++ b/drivers/media/usb/pvrusb2/pvrusb2-hdw.c
@@ -2351,7 +2351,8 @@ struct pvr2_hdw *pvr2_hdw_create(struct usb_interface 
*intf,
 
if (hdw_desc == NULL) {
pvr2_trace(PVR2_TRACE_INIT, "pvr2_hdw_create: No device 
description pointer, unable to continue.");
-   pvr2_trace(PVR2_TRACE_INIT, "If you have a new device type, 
please contact Mike Isely <is...@pobox.com> to get it included in the 
driver\n");
+   pvr2_trace(PVR2_TRACE_INIT,
+  "If you have a new device type, please contact Mike 
Isely <is...@pobox.com> to get it included in the driver");
goto fail;
}
 



Re: [PATCH v1 05/10] staging: atomisp: Remove non-ACPI leftovers

2017-12-20 Thread Julia Lawall


On Wed, 20 Dec 2017, Dan Carpenter wrote:

> On Tue, Dec 19, 2017 at 10:59:52PM +0200, Andy Shevchenko wrote:
> > @@ -914,9 +904,7 @@ static int lm3554_probe(struct i2c_client *client)
> > dev_err(>dev, "gpio request/direction_output fail");
> > goto fail2;
> > }
> > -   if (ACPI_HANDLE(>dev))
> > -   err = atomisp_register_i2c_module(>sd, NULL, LED_FLASH);
> > -   return 0;
> > +   return atomisp_register_i2c_module(>sd, NULL, LED_FLASH);
> >  fail2:
> > media_entity_cleanup(>sd.entity);
> > v4l2_ctrl_handler_free(>ctrl_handler);
>
> Actually every place where we directly return a function call is wrong
> and needs error handling added.  I've been meaning to write a Smatch
> check for this because it's a common anti-pattern we don't check the
> last function call for errors.
>
> Someone could probably do the same in Coccinelle if they want.

I'm not sure what you are suggesting.  Is every case of return f(...);
for any f wrong?  Or is it a particular function that is of concern?  Or
would it be that every function call that has error handling somewhere
should have error handling everywhere?  Or is it related to what seems to
be the problem in the above code that err is initialized but nothing
happens to it?

julia


Re: Adjustments for a lot of function implementations

2017-10-30 Thread Julia Lawall


On Mon, 30 Oct 2017, SF Markus Elfring wrote:

> > While we do not mind cleanup patches, the way you post them (one fix per 
> > file)
>
> I find it safer in this way  while I was browsing through the landscape of 
> Linux
> software components.
>
>
> > is really annoying and takes us too much time to review.
>
> It is just the case that there are so many remaining open issues.
>
>
> > I'll take the "Fix a possible null pointer" patch since it is an actual bug 
> > fix,
>
> Thanks for a bit of change acceptance.
>
>
> > but will reject the others, not just this driver but all of them that are 
> > currently
> > pending in our patchwork (https://patchwork.linuxtv.org).
>
> Will any chances evolve to integrate 146 patches in any other combination?
>
>
> > Feel free to repost, but only if you organize the patch as either fixing 
> > the same type of
> > issue for a whole subdirectory (media/usb, media/pci, etc)

Just for the record, while this may work for media, it won't work for all
subsystems.  One will quickly get a complaint that the big patch needs to
go into multiple trees.

julia

>
> Can we achieve an agreement on the shown change patterns?
>
> Is a consensus possible for involved update candidates?
>
>
> > or fixing all issues for a single driver.
>
> I find that I did this already.
>
>
> > Actual bug fixes (like the null pointer patch in this series) can still be 
> > posted as
> > separate patches, but cleanups shouldn't.
>
> I got an other software development opinion.
>
>
> > Just so you know, I'll reject any future patch series that do not follow 
> > these rules.
> > Just use common sense when posting these things in the future.
>
> Do we need to try any additional communication tools out?
>
>
> > I would also suggest that your time might be spent more productively if you 
> > would
> > work on some more useful projects.
>
> I hope that various change possibilities (from my selection) will become 
> useful
> for more Linux users.
> How will the clarification evolve further?
>
>
> Regards,
> Markus
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>


Re: [PATCH] [media] bdisp: remove redundant assignment to pix

2017-10-29 Thread Julia Lawall


On Sun, 29 Oct 2017, Colin King wrote:

> From: Colin Ian King <colin.k...@canonical.com>
>
> Pointer pix is being initialized to a value and a little later
> being assigned the same value again. Remove the redundant second
> duplicate assignment. Cleans up the clang warning:
>
> drivers/media/platform/sti/bdisp/bdisp-v4l2.c:726:26: warning: Value
> stored to 'pix' during its initialization is never read
>
> Signed-off-by: Colin Ian King <colin.k...@canonical.com>
> ---
>  drivers/media/platform/sti/bdisp/bdisp-v4l2.c | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/drivers/media/platform/sti/bdisp/bdisp-v4l2.c 
> b/drivers/media/platform/sti/bdisp/bdisp-v4l2.c
> index 939da6da7644..14e99aeae140 100644
> --- a/drivers/media/platform/sti/bdisp/bdisp-v4l2.c
> +++ b/drivers/media/platform/sti/bdisp/bdisp-v4l2.c
> @@ -731,7 +731,6 @@ static int bdisp_g_fmt(struct file *file, void *fh, 
> struct v4l2_format *f)
>   return PTR_ERR(frame);
>   }
>
> - pix = >fmt.pix;

Why not keep this one and drop the first one?  Maybe it would be nice to
keep all the initializations related to pix together?

julia

>   pix->width = frame->width;
>   pix->height = frame->height;
>   pix->pixelformat = frame->fmt->pixelformat;
> --
> 2.14.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>


Re: [Outreachy kernel] [PATCH v2 1/2] staging: atomisp2: cleanup null check on memory allocation

2017-10-14 Thread Julia Lawall


On Sat, 14 Oct 2017, Aishwarya Pant wrote:

> For memory allocation functions that fail with a NULL return value, it
> is preferred to use the (!x) test in place of (x == NULL).
>
> Changes in atomisp2/css2400/sh_css.c were done by hand.
>
> Done with the help of the following cocci script:
>
> @@
> type T;
> T* p;
> statement s,s1;
> @@
>
> p =
>   \(devm_kzalloc\|devm_ioremap\|usb_alloc_urb\|alloc_netdev\|dev_alloc_skb\|
>
> kmalloc\|kmalloc_array\|kzalloc\|kcalloc\|kmem_cache_alloc\|kmem_cache_zalloc\|
>kmem_cache_alloc_node\|kmalloc_node\|kzalloc_node\|devm_kzalloc\)(...)
> ...when != p
>
> if (
> - p == NULL
> + !p
>  ) s
>  else s1
>
> Signed-off-by: Aishwarya Pant <aishp...@gmail.com>

Acked-by: Julia Lawall <julia.law...@lip6.fr>

>
> --
> Changes in atomisp2/css2400/sh_css.c were done by hand, the above script
> was not able to match the pattern if (a->b != null).
>
> v2 changes:
> None, just rebase and re-send
> ---
>  .../media/atomisp/pci/atomisp2/css2400/sh_css.c| 36 
> +++---
>  .../atomisp/pci/atomisp2/css2400/sh_css_firmware.c |  6 ++--
>  .../pci/atomisp2/css2400/sh_css_param_shading.c|  2 +-
>  3 files changed, 22 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css.c 
> b/drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css.c
> index e882b5596813..56de641d8848 100644
> --- a/drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css.c
> +++ b/drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css.c
> @@ -5607,13 +5607,13 @@ static enum ia_css_err load_video_binaries(struct 
> ia_css_pipe *pipe)
>   mycs->num_yuv_scaler = cas_scaler_descr.num_stage;
>   mycs->yuv_scaler_binary = kzalloc(cas_scaler_descr.num_stage *
>   sizeof(struct ia_css_binary), GFP_KERNEL);
> - if (mycs->yuv_scaler_binary == NULL) {
> + if (!mycs->yuv_scaler_binary) {
>   err = IA_CSS_ERR_CANNOT_ALLOCATE_MEMORY;
>   return err;
>   }
>   mycs->is_output_stage = kzalloc(cas_scaler_descr.num_stage
>   * sizeof(bool), GFP_KERNEL);
> - if (mycs->is_output_stage == NULL) {
> + if (!mycs->is_output_stage) {
>   err = IA_CSS_ERR_CANNOT_ALLOCATE_MEMORY;
>   return err;
>   }
> @@ -6258,14 +6258,14 @@ static enum ia_css_err load_primary_binaries(
>   mycs->num_yuv_scaler = cas_scaler_descr.num_stage;
>   mycs->yuv_scaler_binary = kzalloc(cas_scaler_descr.num_stage *
>   sizeof(struct ia_css_binary), GFP_KERNEL);
> - if (mycs->yuv_scaler_binary == NULL) {
> + if (!mycs->yuv_scaler_binary) {
>   err = IA_CSS_ERR_CANNOT_ALLOCATE_MEMORY;
>   IA_CSS_LEAVE_ERR_PRIVATE(err);
>   return err;
>   }
>   mycs->is_output_stage = kzalloc(cas_scaler_descr.num_stage *
>   sizeof(bool), GFP_KERNEL);
> - if (mycs->is_output_stage == NULL) {
> + if (!mycs->is_output_stage) {
>   err = IA_CSS_ERR_CANNOT_ALLOCATE_MEMORY;
>   IA_CSS_LEAVE_ERR_PRIVATE(err);
>   return err;
> @@ -6982,27 +6982,27 @@ static enum ia_css_err 
> ia_css_pipe_create_cas_scaler_desc_single_output(
>   }
>
>   descr->in_info = kmalloc(descr->num_stage * sizeof(struct 
> ia_css_frame_info), GFP_KERNEL);
> - if (descr->in_info == NULL) {
> + if (!descr->in_info) {
>   err = IA_CSS_ERR_CANNOT_ALLOCATE_MEMORY;
>   goto ERR;
>   }
>   descr->internal_out_info = kmalloc(descr->num_stage * sizeof(struct 
> ia_css_frame_info), GFP_KERNEL);
> - if (descr->internal_out_info == NULL) {
> + if (!descr->internal_out_info) {
>   err = IA_CSS_ERR_CANNOT_ALLOCATE_MEMORY;
>   goto ERR;
>   }
>   descr->out_info = kmalloc(descr->num_stage * sizeof(struct 
> ia_css_frame_info), GFP_KERNEL);
> - if (descr->out_info == NULL) {
> + if (!descr->out_info) {
>   err = IA_CSS_ERR_CANNOT_ALLOCATE_MEMORY;
>   goto ERR;
>   }
>   descr->vf_info = kmalloc(descr->num_stage * sizeof(struct 
> ia_css_frame_info), GFP_KERNEL);
> - if (descr->vf_info == NULL) {
> + if (!descr->vf_info) {
>   err = IA_CSS_ERR_CANNOT_ALLOCATE_MEMORY;
>

Re: [Outreachy kernel] [PATCH] Staging: media: atomisp: pci: Eliminate use of typedefs for struct

2017-10-07 Thread Julia Lawall


On Sat, 7 Oct 2017, Srishti Sharma wrote:

> The use of typedefs for struct is discouraged, and hence can be
> eliminated. Done using the following semantic patch by coccinelle.
>
> @r1@
> type T;
> @@
>
> typedef struct {...} T;
>
> @script: python p@
> T << r1.T;
> T1;
> @@
>
> if T[-2:] == "_t" or T[-2:] == "_T":
> coccinelle.T1 = T[:-2]
> else:
> coccinelle.T1 = T
>
> print T, T1
> @r2@
> type r1.T;
> identifier p.T1;
> @@
>
> - typedef
> struct
> + T1
> {
> ...
> }
> - T
> ;
>
> @r3@
> type r1.T;
> identifier p.T1;
> @@
>
> - T
> + struct T1
>
> Signed-off-by: Srishti Sharma <srishtis...@gmail.com>

Acked-by: Julia Lawall <julia.law...@lip6.fr>

> ---
>  .../media/atomisp/pci/atomisp2/css2400/runtime/spctrl/src/spctrl.c  | 6 
> +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git 
> a/drivers/staging/media/atomisp/pci/atomisp2/css2400/runtime/spctrl/src/spctrl.c
>  
> b/drivers/staging/media/atomisp/pci/atomisp2/css2400/runtime/spctrl/src/spctrl.c
> index d9178e8..6d9bceb 100644
> --- 
> a/drivers/staging/media/atomisp/pci/atomisp2/css2400/runtime/spctrl/src/spctrl.c
> +++ 
> b/drivers/staging/media/atomisp/pci/atomisp2/css2400/runtime/spctrl/src/spctrl.c
> @@ -37,7 +37,7 @@ more details.
>  #include "ia_css_spctrl.h"
>  #include "ia_css_debug.h"
>
> -typedef struct {
> +struct spctrl_context_info {
>   struct ia_css_sp_init_dmem_cfg dmem_config;
>   uint32_tspctrl_config_dmem_addr; /** location of dmem_cfg  in 
> SP dmem */
>   uint32_tspctrl_state_dmem_addr;
> @@ -45,9 +45,9 @@ typedef struct {
>   hrt_vaddresscode_addr;  /* sp firmware location in host 
> mem-DDR*/
>   uint32_tcode_size;
>   char   *program_name;   /* used in case of PLATFORM_SIM */
> -} spctrl_context_info;
> +};
>
> -static spctrl_context_info spctrl_cofig_info[N_SP_ID];
> +static struct spctrl_context_info spctrl_cofig_info[N_SP_ID];
>  static bool spctrl_loaded[N_SP_ID] = {0};
>
>  /* Load firmware */
> --
> 2.7.4
>
> --
> You received this message because you are subscribed to the Google Groups 
> "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an 
> email to outreachy-kernel+unsubscr...@googlegroups.com.
> To post to this group, send email to outreachy-ker...@googlegroups.com.
> To view this discussion on the web visit 
> https://groups.google.com/d/msgid/outreachy-kernel/1507384322-16584-1-git-send-email-srishtishar%40gmail.com.
> For more options, visit https://groups.google.com/d/optout.
>


Re: [PATCH] [media] ov5645: I2C address change (fwd)

2017-10-04 Thread Julia Lawall
Hello,

It seems that an unlock is missing on line 764.

julia

-- Forwarded message --
Date: Wed, 4 Oct 2017 05:59:09 +0800
From: kbuild test robot <fengguang...@intel.com>
To: kbu...@01.org
Cc: Julia Lawall <julia.law...@lip6.fr>
Subject: Re: [PATCH] [media] ov5645: I2C address change

CC: kbuild-...@01.org
In-Reply-To: <1506950925-13924-1-git-send-email-todor.to...@linaro.org>
TO: Todor Tomov <todor.to...@linaro.org>
CC: mche...@kernel.org, sakari.ai...@linux.intel.com, hansv...@cisco.com, 
linux-media@vger.kernel.org, linux-ker...@vger.kernel.org, Todor Tomov 
<todor.to...@linaro.org>
CC: linux-media@vger.kernel.org, linux-ker...@vger.kernel.org, Todor Tomov 
<todor.to...@linaro.org>

Hi Todor,

[auto build test WARNING on linuxtv-media/master]
[also build test WARNING on v4.14-rc3 next-20170929]
[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/Todor-Tomov/ov5645-I2C-address-change/20171003-234231
base:   git://linuxtv.org/media_tree.git master
:: branch date: 6 hours ago
:: commit date: 6 hours ago

>> drivers/media/i2c/ov5645.c:806:1-7: preceding lock on line 760

# 
https://github.com/0day-ci/linux/commit/c222075023642217170e2ef95f48efef079f9bcd
git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout c222075023642217170e2ef95f48efef079f9bcd
vim +806 drivers/media/i2c/ov5645.c

9cae9722 Todor Tomov 2017-04-11  747
9cae9722 Todor Tomov 2017-04-11  748  static int ov5645_s_power(struct 
v4l2_subdev *sd, int on)
9cae9722 Todor Tomov 2017-04-11  749  {
9cae9722 Todor Tomov 2017-04-11  750struct ov5645 *ov5645 = to_ov5645(sd);
9cae9722 Todor Tomov 2017-04-11  751int ret = 0;
9cae9722 Todor Tomov 2017-04-11  752
9cae9722 Todor Tomov 2017-04-11  753mutex_lock(>power_lock);
9cae9722 Todor Tomov 2017-04-11  754
9cae9722 Todor Tomov 2017-04-11  755/* If the power count is modified from 
0 to != 0 or from != 0 to 0,
9cae9722 Todor Tomov 2017-04-11  756 * update the power state.
9cae9722 Todor Tomov 2017-04-11  757 */
9cae9722 Todor Tomov 2017-04-11  758if (ov5645->power_count == !on) {
9cae9722 Todor Tomov 2017-04-11  759if (on) {
c2220750 Todor Tomov 2017-10-02 @760
mutex_lock(_lock);
c2220750 Todor Tomov 2017-10-02  761
9cae9722 Todor Tomov 2017-04-11  762ret = 
ov5645_set_power_on(ov5645);
9cae9722 Todor Tomov 2017-04-11  763if (ret < 0)
9cae9722 Todor Tomov 2017-04-11  764goto exit;
9cae9722 Todor Tomov 2017-04-11  765
c2220750 Todor Tomov 2017-10-02  766ret = 
ov5645_write_reg_to(ov5645, 0x3100,
c2220750 Todor Tomov 2017-10-02  767
ov5645->i2c_client->addr, 0x78);
c2220750 Todor Tomov 2017-10-02  768if (ret < 0) {
c2220750 Todor Tomov 2017-10-02  769
dev_err(ov5645->dev,
c2220750 Todor Tomov 2017-10-02  770"could 
not change i2c address\n");
c2220750 Todor Tomov 2017-10-02  771
ov5645_set_power_off(ov5645);
c2220750 Todor Tomov 2017-10-02  772
mutex_unlock(_lock);
c2220750 Todor Tomov 2017-10-02  773goto exit;
c2220750 Todor Tomov 2017-10-02  774}
c2220750 Todor Tomov 2017-10-02  775
c2220750 Todor Tomov 2017-10-02  776
mutex_unlock(_lock);
c2220750 Todor Tomov 2017-10-02  777
9cae9722 Todor Tomov 2017-04-11  778ret = 
ov5645_set_register_array(ov5645,
9cae9722 Todor Tomov 2017-04-11  779
ov5645_global_init_setting,
9cae9722 Todor Tomov 2017-04-11  780
ARRAY_SIZE(ov5645_global_init_setting));
9cae9722 Todor Tomov 2017-04-11  781if (ret < 0) {
9cae9722 Todor Tomov 2017-04-11  782
dev_err(ov5645->dev,
9cae9722 Todor Tomov 2017-04-11  783"could 
not set init registers\n");
9cae9722 Todor Tomov 2017-04-11  784
ov5645_set_power_off(ov5645);
9cae9722 Todor Tomov 2017-04-11  785goto exit;
9cae9722 Todor Tomov 2017-04-11  786}
9cae9722 Todor Tomov 2017-04-11  787
9cae9722 Todor Tomov 2017-04-11  788ret = 
ov5645_write_reg(ov5645, OV5645_SYSTEM_CTRL0,
9cae9722 Todor Tomov 2017-04-11  789   
OV5645_SYSTEM_CTRL0_STOP);
9cae9722 Todor Tomov 2017-04-11  790if (ret < 0) {
9cae9722 Todor Tomov 2017-04-11  791
ov5645_set_power_off(ov5645);
9cae9722 Todor Tomov 2017-04-11  792goto exi

Re: [media] spca500: Use common error handling code in spca500_synch310()

2017-09-22 Thread Julia Lawall


On Fri, 22 Sep 2017, SF Markus Elfring wrote:

> >>return 0;
> >> -error:
> >> +
> >> +report_failure:
> >> +  PERR("Set packet size: set interface error");
> >>return -EBUSY;
> >>  }
> >
> > Why change the label name?
>
> I find the suggested variant a bi better.
>
>
> > They are both equally uninformative.
>
> Which identifier would you find appropriate there?

error was fine.

julia


Re: [PATCH] [media] spca500: Use common error handling code in spca500_synch310()

2017-09-22 Thread Julia Lawall


On Fri, 22 Sep 2017, SF Markus Elfring wrote:

> From: Markus Elfring <elfr...@users.sourceforge.net>
> Date: Fri, 22 Sep 2017 18:45:07 +0200
>
> Adjust 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 <elfr...@users.sourceforge.net>
> ---
>  drivers/media/usb/gspca/spca500.c | 21 ++---
>  1 file changed, 10 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/media/usb/gspca/spca500.c 
> b/drivers/media/usb/gspca/spca500.c
> index da2d9027914c..1f224f5e5b19 100644
> --- a/drivers/media/usb/gspca/spca500.c
> +++ b/drivers/media/usb/gspca/spca500.c
> @@ -501,7 +501,6 @@ static int spca500_full_reset(struct gspca_dev *gspca_dev)
>  static int spca500_synch310(struct gspca_dev *gspca_dev)
>  {
> - if (usb_set_interface(gspca_dev->dev, gspca_dev->iface, 0) < 0) {
> - PERR("Set packet size: set interface error");
> - goto error;
> - }
> + if (usb_set_interface(gspca_dev->dev, gspca_dev->iface, 0) < 0)
> + goto report_failure;
> +
>   spca500_ping310(gspca_dev);
> @@ -514,12 +513,12 @@ static int spca500_synch310(struct gspca_dev *gspca_dev)
>   /* Windoze use pipe with altsetting 6 why 7 here */
> - if (usb_set_interface(gspca_dev->dev,
> - gspca_dev->iface,
> - gspca_dev->alt) < 0) {
> - PERR("Set packet size: set interface error");
> - goto error;
> - }
> + if (usb_set_interface(gspca_dev->dev, gspca_dev->iface, gspca_dev->alt)
> + < 0)
> + goto report_failure;
> +
>   return 0;
> -error:
> +
> +report_failure:
> + PERR("Set packet size: set interface error");
>   return -EBUSY;
>  }

Why change the label name?  They are both equally uninformative.

julia


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

2017-09-12 Thread Julia Lawall


On Tue, 12 Sep 2017, 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 <srishtis...@gmail.com>

Acked-by: Julia Lawall <julia.law...@lip6.fr>


> ---
>  drivers/staging/media/atomisp/pci/atomisp2/hmm/hmm_bo.c | 6 +-
>  1 file changed, 1 insertion(+), 5 deletions(-)
>
> diff --git a/drivers/staging/media/atomisp/pci/atomisp2/hmm/hmm_bo.c 
> b/drivers/staging/media/atomisp/pci/atomisp2/hmm/hmm_bo.c
> index 11162f5..e6ddfbf 100644
> --- a/drivers/staging/media/atomisp/pci/atomisp2/hmm/hmm_bo.c
> +++ b/drivers/staging/media/atomisp/pci/atomisp2/hmm/hmm_bo.c
> @@ -1168,13 +1168,9 @@ void hmm_bo_free_pages(struct hmm_buffer_object *bo)
>
>  int hmm_bo_page_allocated(struct hmm_buffer_object *bo)
>  {
> - int ret;
> -
>   check_bo_null_return(bo, 0);
>
> - ret = bo->status & HMM_BO_PAGE_ALLOCED;
> -
> - return ret;
> + return bo->status & HMM_BO_PAGE_ALLOCED;
>  }
>
>  /*
> --
> 2.7.4
>
> --
> You received this message because you are subscribed to the Google Groups 
> "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an 
> email to outreachy-kernel+unsubscr...@googlegroups.com.
> To post to this group, send email to outreachy-ker...@googlegroups.com.
> To view this discussion on the web visit 
> https://groups.google.com/d/msgid/outreachy-kernel/1505226307-5119-1-git-send-email-srishtishar%40gmail.com.
> For more options, visit https://groups.google.com/d/optout.
>


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

2017-09-12 Thread Julia Lawall


On Tue, 12 Sep 2017, 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 <srishtis...@gmail.com>

Acked-by: Julia Lawall <julia.law...@lip6.fr>

> ---
>  drivers/staging/media/atomisp/i2c/ov5693/ov5693.c | 11 ++-
>  1 file changed, 2 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/staging/media/atomisp/i2c/ov5693/ov5693.c 
> b/drivers/staging/media/atomisp/i2c/ov5693/ov5693.c
> index 1236425..2195011 100644
> --- a/drivers/staging/media/atomisp/i2c/ov5693/ov5693.c
> +++ b/drivers/staging/media/atomisp/i2c/ov5693/ov5693.c
> @@ -945,12 +945,8 @@ static int ad5823_t_focus_vcm(struct v4l2_subdev *sd, 
> u16 val)
>
>  int ad5823_t_focus_abs(struct v4l2_subdev *sd, s32 value)
>  {
> - int ret;
> -
>   value = min(value, AD5823_MAX_FOCUS_POS);
> - ret = ad5823_t_focus_vcm(sd, value);
> -
> - return ret;
> + return ad5823_t_focus_vcm(sd, value);
>  }
>
>  static int ov5693_t_focus_abs(struct v4l2_subdev *sd, s32 value)
> @@ -1332,7 +1328,6 @@ static int power_ctrl(struct v4l2_subdev *sd, bool flag)
>
>  static int gpio_ctrl(struct v4l2_subdev *sd, bool flag)
>  {
> - int ret;
>   struct ov5693_device *dev = to_ov5693_sensor(sd);
>
>   if (!dev || !dev->platform_data)
> @@ -1342,9 +1337,7 @@ static int gpio_ctrl(struct v4l2_subdev *sd, bool flag)
>   if (dev->platform_data->gpio_ctrl)
>   return dev->platform_data->gpio_ctrl(sd, flag);
>
> - ret = dev->platform_data->gpio0_ctrl(sd, flag);
> -
> - return ret;
> + return dev->platform_data->gpio0_ctrl(sd, flag);
>  }
>
>  static int __power_up(struct v4l2_subdev *sd)
> --
> 2.7.4
>
> --
> You received this message because you are subscribed to the Google Groups 
> "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an 
> email to outreachy-kernel+unsubscr...@googlegroups.com.
> To post to this group, send email to outreachy-ker...@googlegroups.com.
> To view this discussion on the web visit 
> https://groups.google.com/d/msgid/outreachy-kernel/1505225549-4432-1-git-send-email-srishtishar%40gmail.com.
> For more options, visit https://groups.google.com/d/optout.
>


Re: [Outreachy kernel] [PATCH] Staging: media: omap4iss: Use WARN_ON() instead of BUG_ON().

2017-09-08 Thread Julia Lawall


On Fri, 8 Sep 2017, Srishti Sharma wrote:

> Use WARN_ON() instead of BUG_ON() to avoid crashing the kernel.
>
> Signed-off-by: Srishti Sharma <srishtis...@gmail.com>
> ---
>  drivers/staging/media/omap4iss/iss.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/staging/media/omap4iss/iss.c 
> b/drivers/staging/media/omap4iss/iss.c
> index c26c99fd..b1036ba 100644
> --- a/drivers/staging/media/omap4iss/iss.c
> +++ b/drivers/staging/media/omap4iss/iss.c
> @@ -893,7 +893,7 @@ void omap4iss_put(struct iss_device *iss)
>   return;
>
>   mutex_lock(>iss_mutex);
> - BUG_ON(iss->ref_count == 0);
> + WARN_ON(iss->ref_count == 0);
>   if (--iss->ref_count == 0) {

Won't this then infinite loop?

julia

>   iss_disable_interrupts(iss);
>   /* Reset the ISS if an entity has failed to stop. This is the
> --
> 2.7.4
>
> --
> You received this message because you are subscribed to the Google Groups 
> "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an 
> email to outreachy-kernel+unsubscr...@googlegroups.com.
> To post to this group, send email to outreachy-ker...@googlegroups.com.
> To view this discussion on the web visit 
> https://groups.google.com/d/msgid/outreachy-kernel/1504879698-5855-1-git-send-email-srishtishar%40gmail.com.
> For more options, visit https://groups.google.com/d/optout.
>


[PATCH v2] media: ddbridge: constify stv0910_p and lnbh25_cfg

2017-08-14 Thread Julia Lawall
These structures are only copied into other structures, so
they can be const.

Done with the help of Coccinelle.

Signed-off-by: Julia Lawall <julia.law...@lip6.fr>

---

v2: fix typo in the commit message

 drivers/media/pci/ddbridge/ddbridge-core.c |4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/pci/ddbridge/ddbridge-core.c 
b/drivers/media/pci/ddbridge/ddbridge-core.c
index 7e164a37..7ff570b 100644
--- a/drivers/media/pci/ddbridge/ddbridge-core.c
+++ b/drivers/media/pci/ddbridge/ddbridge-core.c
@@ -800,14 +800,14 @@ static int tuner_attach_stv6110(struct ddb_input *input, 
int type)
return 0;
 }
 
-static struct stv0910_cfg stv0910_p = {
+static const struct stv0910_cfg stv0910_p = {
.adr  = 0x68,
.parallel = 1,
.rptlvl   = 4,
.clk  = 3000,
 };
 
-static struct lnbh25_config lnbh25_cfg = {
+static const struct lnbh25_config lnbh25_cfg = {
.i2c_address = 0x0c << 1,
.data2_config = LNBH25_TEN
 };



[PATCH] media: ddbridge: constify stv0910_p and lnbh25_cfg

2017-08-14 Thread Julia Lawall
These structures are only copied into other stuructures, so
they can be const.

Done with the help of Coccinelle.

Signed-off-by: Julia Lawall <julia.law...@lip6.fr>

---
 drivers/media/pci/ddbridge/ddbridge-core.c |4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/pci/ddbridge/ddbridge-core.c 
b/drivers/media/pci/ddbridge/ddbridge-core.c
index 7e164a37..7ff570b 100644
--- a/drivers/media/pci/ddbridge/ddbridge-core.c
+++ b/drivers/media/pci/ddbridge/ddbridge-core.c
@@ -800,14 +800,14 @@ static int tuner_attach_stv6110(struct ddb_input *input, 
int type)
return 0;
 }
 
-static struct stv0910_cfg stv0910_p = {
+static const struct stv0910_cfg stv0910_p = {
.adr  = 0x68,
.parallel = 1,
.rptlvl   = 4,
.clk  = 3000,
 };
 
-static struct lnbh25_config lnbh25_cfg = {
+static const struct lnbh25_config lnbh25_cfg = {
.i2c_address = 0x0c << 1,
.data2_config = LNBH25_TEN
 };



[PATCH] [media] pxa_camera: constify v4l2_clk_ops structure

2017-08-14 Thread Julia Lawall
This v4l2_clk_ops structure is only passed as the first argument of
v4l2_clk_register, which is const, so the v4l2_clk_ops structure can
also be const.

Done with the help of Coccinelle.

Signed-off-by: Julia Lawall <julia.law...@lip6.fr>

---
 drivers/media/platform/pxa_camera.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/pxa_camera.c 
b/drivers/media/platform/pxa_camera.c
index 0d4af6d..4a800a4 100644
--- a/drivers/media/platform/pxa_camera.c
+++ b/drivers/media/platform/pxa_camera.c
@@ -2100,7 +2100,7 @@ static int pxac_fops_camera_release(struct file *filp)
.vidioc_unsubscribe_event   = v4l2_event_unsubscribe,
 };
 
-static struct v4l2_clk_ops pxa_camera_mclk_ops = {
+static const struct v4l2_clk_ops pxa_camera_mclk_ops = {
 };
 
 static const struct video_device pxa_camera_videodev_template = {



[PATCH] [media] v4l2: av7110_v4l: constify v4l2_audio structure

2017-08-14 Thread Julia Lawall
This v4l2_audio structure is only copied into other structures,
so it can be const.

Done with the help of Coccinelle.

Signed-off-by: Julia Lawall <julia.law...@lip6.fr>

---
 drivers/media/pci/ttpci/av7110_v4l.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/pci/ttpci/av7110_v4l.c 
b/drivers/media/pci/ttpci/av7110_v4l.c
index 397fe14..e4cf42c 100644
--- a/drivers/media/pci/ttpci/av7110_v4l.c
+++ b/drivers/media/pci/ttpci/av7110_v4l.c
@@ -218,7 +218,7 @@ static int stv0297_set_tv_freq(struct saa7146_dev *dev, u32 
freq)
 static struct saa7146_standard dvb_standard[];
 static struct saa7146_standard standard[];
 
-static struct v4l2_audio msp3400_v4l2_audio = {
+static const struct v4l2_audio msp3400_v4l2_audio = {
.index = 0,
.name = "Television",
.capability = V4L2_AUDCAP_STEREO



Re: [PATCH 6/6] [media] uvcvideo: constify video_subdev structures

2017-08-08 Thread Julia Lawall


On Tue, 8 Aug 2017, Laurent Pinchart wrote:

> Hi Julia,
>
> Thank you for the patch.
>
> On Tuesday 08 Aug 2017 12:58:32 Julia Lawall wrote:
> > uvc_subdev_ops is only passed as the second argument of
> > v4l2_subdev_init, which is const, so uvc_subdev_ops can be
> > const as well.
> >
> > Done with the help of Coccinelle.
> >
> > Signed-off-by: Julia Lawall <julia.law...@lip6.fr>
>
> Reviewed-by: Laurent Pinchart <laurent.pinch...@ideasonboard.com>
>
> and applied to my tree (with the first word of the commit message after the
> prefix capitalized to match the rest of the driver's commit messages, let me
> know if that's a problem :-)).

No, of course not.  I will try to watch out for that in the future.

julia

> >
> > ---
> >  drivers/media/usb/uvc/uvc_entity.c |2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/media/usb/uvc/uvc_entity.c
> > b/drivers/media/usb/uvc/uvc_entity.c index ac386bb..554063c 100644
> > --- a/drivers/media/usb/uvc/uvc_entity.c
> > +++ b/drivers/media/usb/uvc/uvc_entity.c
> > @@ -61,7 +61,7 @@ static int uvc_mc_create_links(struct uvc_video_chain
> > *chain, return 0;
> >  }
> >
> > -static struct v4l2_subdev_ops uvc_subdev_ops = {
> > +static const struct v4l2_subdev_ops uvc_subdev_ops = {
> >  };
> >
> >  void uvc_mc_cleanup_entity(struct uvc_entity *entity)
>
> --
> Regards,
>
> Laurent Pinchart
>
>


[PATCH 1/6] [media] v4l: mt9t001: constify video_subdev structures

2017-08-08 Thread Julia Lawall
The v4l2_subdev_ops structure is only passed as the third argument of
v4l2_i2c_subdev_init, which is const, so the v4l2_subdev_ops structure
can be const as well.  The other structures are only stored in the
v4l2_subdev_ops structure, all the fields of which are const, so these
structures can also be const.

Done with the help of Coccinelle.

Signed-off-by: Julia Lawall <julia.law...@lip6.fr>

---
 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,



[PATCH 6/6] [media] uvcvideo: constify video_subdev structures

2017-08-08 Thread Julia Lawall
uvc_subdev_ops is only passed as the second argument of
v4l2_subdev_init, which is const, so uvc_subdev_ops can be
const as well.

Done with the help of Coccinelle.

Signed-off-by: Julia Lawall <julia.law...@lip6.fr>

---
 drivers/media/usb/uvc/uvc_entity.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/usb/uvc/uvc_entity.c 
b/drivers/media/usb/uvc/uvc_entity.c
index ac386bb..554063c 100644
--- a/drivers/media/usb/uvc/uvc_entity.c
+++ b/drivers/media/usb/uvc/uvc_entity.c
@@ -61,7 +61,7 @@ static int uvc_mc_create_links(struct uvc_video_chain *chain,
return 0;
 }
 
-static struct v4l2_subdev_ops uvc_subdev_ops = {
+static const struct v4l2_subdev_ops uvc_subdev_ops = {
 };
 
 void uvc_mc_cleanup_entity(struct uvc_entity *entity)



[PATCH 3/6] staging: atomisp: constify video_subdev structures

2017-08-08 Thread Julia Lawall
These structures are both stored in fields of v4l2_subdev_ops
structures, all of which are const, so these structures can be
const as well.

Done with the help of Coccinelle.

Signed-off-by: Julia Lawall <julia.law...@lip6.fr>

---
 drivers/staging/media/atomisp/i2c/ap1302.c  |2 +-
 drivers/staging/media/atomisp/i2c/mt9m114.c |2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/media/atomisp/i2c/ap1302.c 
b/drivers/staging/media/atomisp/i2c/ap1302.c
index bacffbe..de687c6 100644
--- a/drivers/staging/media/atomisp/i2c/ap1302.c
+++ b/drivers/staging/media/atomisp/i2c/ap1302.c
@@ -1098,7 +1098,7 @@ static long ap1302_ioctl(struct v4l2_subdev *sd, unsigned 
int cmd, void *arg)
},
 };
 
-static struct v4l2_subdev_sensor_ops ap1302_sensor_ops = {
+static const struct v4l2_subdev_sensor_ops ap1302_sensor_ops = {
.g_skip_frames  = ap1302_g_skip_frames,
 };
 
diff --git a/drivers/staging/media/atomisp/i2c/mt9m114.c 
b/drivers/staging/media/atomisp/i2c/mt9m114.c
index 448e072..36a0636 100644
--- a/drivers/staging/media/atomisp/i2c/mt9m114.c
+++ b/drivers/staging/media/atomisp/i2c/mt9m114.c
@@ -1806,7 +1806,7 @@ static int mt9m114_g_skip_frames(struct v4l2_subdev *sd, 
u32 *frames)
.g_frame_interval = mt9m114_g_frame_interval,
 };
 
-static struct v4l2_subdev_sensor_ops mt9m114_sensor_ops = {
+static const struct v4l2_subdev_sensor_ops mt9m114_sensor_ops = {
.g_skip_frames  = mt9m114_g_skip_frames,
 };
 



[PATCH 0/6] constify video_subdev structures

2017-08-08 Thread Julia Lawall
The structures of type v4l2_subdev_ops are only passed as the second
argument of v4l2_subdev_init or as the third argument of
v4l2_i2c_subdev_init, both of which are const.  The structures of type
v4l2_subdev_core_ops, v4l2_subdev_pad_ops, v4l2_subdev_sensor_ops,
v4l2_subdev_video_ops are only stored in fields of v4l2_subdev_ops
structures, all of which are const.  Thus all of these structures can be
declared as const as well.

Done with the help of Coccinelle.

---

 drivers/media/i2c/mt9m111.c   |6 +++---
 drivers/media/i2c/mt9t001.c   |8 
 drivers/media/platform/exynos4-is/fimc-isp.c  |2 +-
 drivers/media/platform/exynos4-is/fimc-lite.c |2 +-
 drivers/media/platform/vimc/vimc-debayer.c|2 +-
 drivers/media/platform/vimc/vimc-scaler.c |2 +-
 drivers/media/platform/vimc/vimc-sensor.c |2 +-
 drivers/media/usb/uvc/uvc_entity.c|2 +-
 drivers/staging/media/atomisp/i2c/ap1302.c|2 +-
 drivers/staging/media/atomisp/i2c/mt9m114.c   |2 +-
 10 files changed, 15 insertions(+), 15 deletions(-)


[PATCH 2/6] [media] vimc: constify video_subdev structures

2017-08-08 Thread Julia Lawall
These structures are all only stored in fields of v4l2_subdev_ops
structures, all of which are const, so these structures can be const
as well.

Done with the help of Coccinelle.

Signed-off-by: Julia Lawall <julia.law...@lip6.fr>

---
 drivers/media/platform/vimc/vimc-debayer.c |2 +-
 drivers/media/platform/vimc/vimc-scaler.c  |2 +-
 drivers/media/platform/vimc/vimc-sensor.c  |2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/media/platform/vimc/vimc-debayer.c 
b/drivers/media/platform/vimc/vimc-debayer.c
index 033a131..4d663e8 100644
--- a/drivers/media/platform/vimc/vimc-debayer.c
+++ b/drivers/media/platform/vimc/vimc-debayer.c
@@ -373,7 +373,7 @@ static int vimc_deb_s_stream(struct v4l2_subdev *sd, int 
enable)
return 0;
 }
 
-static struct v4l2_subdev_video_ops vimc_deb_video_ops = {
+static const struct v4l2_subdev_video_ops vimc_deb_video_ops = {
.s_stream = vimc_deb_s_stream,
 };
 
diff --git a/drivers/media/platform/vimc/vimc-scaler.c 
b/drivers/media/platform/vimc/vimc-scaler.c
index 0a3e086..e1602e0 100644
--- a/drivers/media/platform/vimc/vimc-scaler.c
+++ b/drivers/media/platform/vimc/vimc-scaler.c
@@ -267,7 +267,7 @@ static int vimc_sca_s_stream(struct v4l2_subdev *sd, int 
enable)
return 0;
 }
 
-static struct v4l2_subdev_video_ops vimc_sca_video_ops = {
+static const struct v4l2_subdev_video_ops vimc_sca_video_ops = {
.s_stream = vimc_sca_s_stream,
 };
 
diff --git a/drivers/media/platform/vimc/vimc-sensor.c 
b/drivers/media/platform/vimc/vimc-sensor.c
index 615c2b1..02e68c8 100644
--- a/drivers/media/platform/vimc/vimc-sensor.c
+++ b/drivers/media/platform/vimc/vimc-sensor.c
@@ -282,7 +282,7 @@ static int vimc_sen_s_stream(struct v4l2_subdev *sd, int 
enable)
return 0;
 }
 
-static struct v4l2_subdev_video_ops vimc_sen_video_ops = {
+static const struct v4l2_subdev_video_ops vimc_sen_video_ops = {
.s_stream = vimc_sen_s_stream,
 };
 



[PATCH 5/6] [media] exynos4-is: constify video_subdev structures

2017-08-08 Thread Julia Lawall
The v4l2_subdev_ops structures are only passed as the second
argument of v4l2_subdev_init, which is const, so the
v4l2_subdev_ops structures can be const as well.

Done with the help of Coccinelle.

Signed-off-by: Julia Lawall <julia.law...@lip6.fr>

---
 drivers/media/platform/exynos4-is/fimc-isp.c  |2 +-
 drivers/media/platform/exynos4-is/fimc-lite.c |2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/exynos4-is/fimc-isp.c 
b/drivers/media/platform/exynos4-is/fimc-isp.c
index 8efe916..fd793d3 100644
--- a/drivers/media/platform/exynos4-is/fimc-isp.c
+++ b/drivers/media/platform/exynos4-is/fimc-isp.c
@@ -433,7 +433,7 @@ static void fimc_isp_subdev_unregistered(struct v4l2_subdev 
*sd)
.s_power = fimc_isp_subdev_s_power,
 };
 
-static struct v4l2_subdev_ops fimc_is_subdev_ops = {
+static const struct v4l2_subdev_ops fimc_is_subdev_ops = {
.core = _is_core_ops,
.video = _is_subdev_video_ops,
.pad = _is_subdev_pad_ops,
diff --git a/drivers/media/platform/exynos4-is/fimc-lite.c 
b/drivers/media/platform/exynos4-is/fimc-lite.c
index 7d3ec5c..30282c5 100644
--- a/drivers/media/platform/exynos4-is/fimc-lite.c
+++ b/drivers/media/platform/exynos4-is/fimc-lite.c
@@ -1361,7 +1361,7 @@ static void fimc_lite_subdev_unregistered(struct 
v4l2_subdev *sd)
.log_status = fimc_lite_log_status,
 };
 
-static struct v4l2_subdev_ops fimc_lite_subdev_ops = {
+static const struct v4l2_subdev_ops fimc_lite_subdev_ops = {
.core = _lite_core_ops,
.video = _lite_subdev_video_ops,
.pad = _lite_subdev_pad_ops,



[PATCH 4/6] [media] media: mt9m111: constify video_subdev structures

2017-08-08 Thread Julia Lawall
The v4l2_subdev_ops structure is only passed as the third argument
of v4l2_i2c_subdev_init, which is const, so the v4l2_subdev_ops
structure can be const as well.  The other structures are only
stored in the v4l2_subdev_ops structure, all the fields of which are
const, so these structures can also be const.

Done with the help of Coccinelle.

Signed-off-by: Julia Lawall <julia.law...@lip6.fr>

---
 drivers/media/i2c/mt9m111.c |6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/media/i2c/mt9m111.c b/drivers/media/i2c/mt9m111.c
index 72e71b7..99b992e 100644
--- a/drivers/media/i2c/mt9m111.c
+++ b/drivers/media/i2c/mt9m111.c
@@ -835,7 +835,7 @@ static int mt9m111_s_power(struct v4l2_subdev *sd, int on)
.s_ctrl = mt9m111_s_ctrl,
 };
 
-static struct v4l2_subdev_core_ops mt9m111_subdev_core_ops = {
+static const struct v4l2_subdev_core_ops mt9m111_subdev_core_ops = {
.s_power= mt9m111_s_power,
 #ifdef CONFIG_VIDEO_ADV_DEBUG
.g_register = mt9m111_g_register,
@@ -865,7 +865,7 @@ static int mt9m111_g_mbus_config(struct v4l2_subdev *sd,
return 0;
 }
 
-static struct v4l2_subdev_video_ops mt9m111_subdev_video_ops = {
+static const struct v4l2_subdev_video_ops mt9m111_subdev_video_ops = {
.g_mbus_config  = mt9m111_g_mbus_config,
 };
 
@@ -877,7 +877,7 @@ static int mt9m111_g_mbus_config(struct v4l2_subdev *sd,
.set_fmt= mt9m111_set_fmt,
 };
 
-static struct v4l2_subdev_ops mt9m111_subdev_ops = {
+static const struct v4l2_subdev_ops mt9m111_subdev_ops = {
.core   = _subdev_core_ops,
.video  = _subdev_video_ops,
.pad= _subdev_pad_ops,



[PATCH] [media] vs6624: constify vs6624_default_fmt

2017-08-07 Thread Julia Lawall
The structure vs6624_default_fmt is only copied into another
structure field, so it can be const.

Done with the help of Coccinelle.

Signed-off-by: Julia Lawall <julia.law...@lip6.fr>

---
 drivers/media/i2c/vs6624.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/i2c/vs6624.c b/drivers/media/i2c/vs6624.c
index f0741ab..5607382 100644
--- a/drivers/media/i2c/vs6624.c
+++ b/drivers/media/i2c/vs6624.c
@@ -58,7 +58,7 @@ struct vs6624 {
},
 };
 
-static struct v4l2_mbus_framefmt vs6624_default_fmt = {
+static const struct v4l2_mbus_framefmt vs6624_default_fmt = {
.width = VGA_WIDTH,
.height = VGA_HEIGHT,
.code = MEDIA_BUS_FMT_UYVY8_2X8,



[PATCH 02/12] [media] media: ti-vpe: vpe: constify v4l2_m2m_ops structures

2017-08-06 Thread Julia Lawall
The v4l2_m2m_ops structures are only passed as the only
argument to v4l2_m2m_init, which is declared as const.
Thus the v4l2_m2m_ops structures themselves can be const.

Done with the help of Coccinelle.

// 
@r disable optional_qualifier@
identifier i;
position p;
@@
static struct v4l2_m2m_ops i@p = { ... };

@ok1@
identifier r.i;
position p;
@@
v4l2_m2m_init(@p)

@bad@
position p != {r.p,ok1.p};
identifier r.i;
struct v4l2_m2m_ops e;
@@
e@i@p

@depends on !bad disable optional_qualifier@
identifier r.i;
@@
static
+const
 struct v4l2_m2m_ops i = { ... };
// 

Signed-off-by: Julia Lawall <julia.law...@lip6.fr>

---
 drivers/media/platform/ti-vpe/vpe.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/ti-vpe/vpe.c 
b/drivers/media/platform/ti-vpe/vpe.c
index c471514..2873c22 100644
--- a/drivers/media/platform/ti-vpe/vpe.c
+++ b/drivers/media/platform/ti-vpe/vpe.c
@@ -2430,7 +2430,7 @@ static int vpe_release(struct file *file)
.vfl_dir= VFL_DIR_M2M,
 };
 
-static struct v4l2_m2m_ops m2m_ops = {
+static const struct v4l2_m2m_ops m2m_ops = {
.device_run = device_run,
.job_ready  = job_ready,
.job_abort  = job_abort,



[PATCH 04/12] [media] V4L2: platform: rcar_jpu: constify v4l2_m2m_ops structures

2017-08-06 Thread Julia Lawall
The v4l2_m2m_ops structures are only passed as the only
argument to v4l2_m2m_init, which is declared as const.
Thus the v4l2_m2m_ops structures themselves can be const.

Done with the help of Coccinelle.

// 
@r disable optional_qualifier@
identifier i;
position p;
@@
static struct v4l2_m2m_ops i@p = { ... };

@ok1@
identifier r.i;
position p;
@@
v4l2_m2m_init(@p)

@bad@
position p != {r.p,ok1.p};
identifier r.i;
struct v4l2_m2m_ops e;
@@
e@i@p

@depends on !bad disable optional_qualifier@
identifier r.i;
@@
static
+const
 struct v4l2_m2m_ops i = { ... };
// 

Signed-off-by: Julia Lawall <julia.law...@lip6.fr>

---
 drivers/media/platform/rcar_jpu.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/rcar_jpu.c 
b/drivers/media/platform/rcar_jpu.c
index d1746ec..070bac3 100644
--- a/drivers/media/platform/rcar_jpu.c
+++ b/drivers/media/platform/rcar_jpu.c
@@ -1506,7 +1506,7 @@ static void jpu_job_abort(void *priv)
jpu_cleanup(ctx, true);
 }
 
-static struct v4l2_m2m_ops jpu_m2m_ops = {
+static const struct v4l2_m2m_ops jpu_m2m_ops = {
.device_run = jpu_device_run,
.job_ready  = jpu_job_ready,
.job_abort  = jpu_job_abort,



[PATCH 03/12] [media] s5p-g2d: constify v4l2_m2m_ops structures

2017-08-06 Thread Julia Lawall
The v4l2_m2m_ops structures are only passed as the only
argument to v4l2_m2m_init, which is declared as const.
Thus the v4l2_m2m_ops structures themselves can be const.

Done with the help of Coccinelle.

// 
@r disable optional_qualifier@
identifier i;
position p;
@@
static struct v4l2_m2m_ops i@p = { ... };

@ok1@
identifier r.i;
position p;
@@
v4l2_m2m_init(@p)

@bad@
position p != {r.p,ok1.p};
identifier r.i;
struct v4l2_m2m_ops e;
@@
e@i@p

@depends on !bad disable optional_qualifier@
identifier r.i;
@@
static
+const
 struct v4l2_m2m_ops i = { ... };
// 

Signed-off-by: Julia Lawall <julia.law...@lip6.fr>

---
 drivers/media/platform/s5p-g2d/g2d.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/s5p-g2d/g2d.c 
b/drivers/media/platform/s5p-g2d/g2d.c
index 81ed5cd..bd655b5 100644
--- a/drivers/media/platform/s5p-g2d/g2d.c
+++ b/drivers/media/platform/s5p-g2d/g2d.c
@@ -611,7 +611,7 @@ static irqreturn_t g2d_isr(int irq, void *prv)
.vfl_dir= VFL_DIR_M2M,
 };
 
-static struct v4l2_m2m_ops g2d_m2m_ops = {
+static const struct v4l2_m2m_ops g2d_m2m_ops = {
.device_run = device_run,
.job_abort  = job_abort,
 };



[PATCH 01/12] [media] st-delta: constify v4l2_m2m_ops structures

2017-08-06 Thread Julia Lawall
The v4l2_m2m_ops structures are only passed as the only
argument to v4l2_m2m_init, which is declared as const.
Thus the v4l2_m2m_ops structures themselves can be const.

Done with the help of Coccinelle.

// 
@r disable optional_qualifier@
identifier i;
position p;
@@
static struct v4l2_m2m_ops i@p = { ... };

@ok1@
identifier r.i;
position p;
@@
v4l2_m2m_init(@p)

@bad@
position p != {r.p,ok1.p};
identifier r.i;
struct v4l2_m2m_ops e;
@@
e@i@p

@depends on !bad disable optional_qualifier@
identifier r.i;
@@
static
+const
 struct v4l2_m2m_ops i = { ... };
// 

Signed-off-by: Julia Lawall <julia.law...@lip6.fr>

---
 drivers/media/platform/sti/delta/delta-v4l2.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/sti/delta/delta-v4l2.c 
b/drivers/media/platform/sti/delta/delta-v4l2.c
index ff9850e..b2dc3d2 100644
--- a/drivers/media/platform/sti/delta/delta-v4l2.c
+++ b/drivers/media/platform/sti/delta/delta-v4l2.c
@@ -1095,7 +1095,7 @@ static int delta_job_ready(void *priv)
 }
 
 /* mem-to-mem ops */
-static struct v4l2_m2m_ops delta_m2m_ops = {
+static const struct v4l2_m2m_ops delta_m2m_ops = {
.device_run = delta_device_run,
.job_ready  = delta_job_ready,
.job_abort  = delta_job_abort,



[PATCH 05/12] [media] vcodec: mediatek: constify v4l2_m2m_ops structures

2017-08-06 Thread Julia Lawall
The v4l2_m2m_ops structures are only passed as the only
argument to v4l2_m2m_init, which is declared as const.
Thus the v4l2_m2m_ops structures themselves can be const.

Done with the help of Coccinelle.

// 
@r disable optional_qualifier@
identifier i;
position p;
@@
static struct v4l2_m2m_ops i@p = { ... };

@ok1@
identifier r.i;
position p;
@@
v4l2_m2m_init(@p)

@bad@
position p != {r.p,ok1.p};
identifier r.i;
struct v4l2_m2m_ops e;
@@
e@i@p

@depends on !bad disable optional_qualifier@
identifier r.i;
@@
static
+const
 struct v4l2_m2m_ops i = { ... };
// 

Signed-off-by: Julia Lawall <julia.law...@lip6.fr>

---
 drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c 
b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
index f17a86b..226f908 100644
--- a/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
+++ b/drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c
@@ -865,7 +865,7 @@ static void mtk_jpeg_job_abort(void *priv)
 {
 }
 
-static struct v4l2_m2m_ops mtk_jpeg_m2m_ops = {
+static const struct v4l2_m2m_ops mtk_jpeg_m2m_ops = {
.device_run = mtk_jpeg_device_run,
.job_ready  = mtk_jpeg_job_ready,
.job_abort  = mtk_jpeg_job_abort,



[PATCH 00/12] constify v4l2_m2m_ops structures

2017-08-06 Thread Julia Lawall
The v4l2_m2m_ops structures are only passed as the only
argument to v4l2_m2m_init, which is declared as const.
Thus the v4l2_m2m_ops structures themselves can be const.

Done with the help of Coccinelle.

---

 drivers/media/platform/exynos-gsc/gsc-m2m.c |2 +-
 drivers/media/platform/exynos4-is/fimc-m2m.c|2 +-
 drivers/media/platform/m2m-deinterlace.c|2 +-
 drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c |2 +-
 drivers/media/platform/mtk-mdp/mtk_mdp_m2m.c|2 +-
 drivers/media/platform/mx2_emmaprp.c|2 +-
 drivers/media/platform/rcar_jpu.c   |2 +-
 drivers/media/platform/s5p-g2d/g2d.c|2 +-
 drivers/media/platform/sti/bdisp/bdisp-v4l2.c   |2 +-
 drivers/media/platform/sti/delta/delta-v4l2.c   |2 +-
 drivers/media/platform/ti-vpe/vpe.c |2 +-
 drivers/media/platform/vim2m.c  |2 +-
 12 files changed, 12 insertions(+), 12 deletions(-)


[PATCH 06/12] [media] exynos-gsc: constify v4l2_m2m_ops structures

2017-08-06 Thread Julia Lawall
The v4l2_m2m_ops structures are only passed as the only
argument to v4l2_m2m_init, which is declared as const.
Thus the v4l2_m2m_ops structures themselves can be const.

Done with the help of Coccinelle.

// 
@r disable optional_qualifier@
identifier i;
position p;
@@
static struct v4l2_m2m_ops i@p = { ... };

@ok1@
identifier r.i;
position p;
@@
v4l2_m2m_init(@p)

@bad@
position p != {r.p,ok1.p};
identifier r.i;
struct v4l2_m2m_ops e;
@@
e@i@p

@depends on !bad disable optional_qualifier@
identifier r.i;
@@
static
+const
 struct v4l2_m2m_ops i = { ... };
// 

Signed-off-by: Julia Lawall <julia.law...@lip6.fr>

---
 drivers/media/platform/exynos-gsc/gsc-m2m.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/exynos-gsc/gsc-m2m.c 
b/drivers/media/platform/exynos-gsc/gsc-m2m.c
index 33611a4..2a2994e 100644
--- a/drivers/media/platform/exynos-gsc/gsc-m2m.c
+++ b/drivers/media/platform/exynos-gsc/gsc-m2m.c
@@ -747,7 +747,7 @@ static int gsc_m2m_mmap(struct file *file, struct 
vm_area_struct *vma)
.mmap   = gsc_m2m_mmap,
 };
 
-static struct v4l2_m2m_ops gsc_m2m_ops = {
+static const struct v4l2_m2m_ops gsc_m2m_ops = {
.device_run = gsc_m2m_device_run,
.job_abort  = gsc_m2m_job_abort,
 };



[PATCH 07/12] [media] bdisp: constify v4l2_m2m_ops structures

2017-08-06 Thread Julia Lawall
The v4l2_m2m_ops structures are only passed as the only
argument to v4l2_m2m_init, which is declared as const.
Thus the v4l2_m2m_ops structures themselves can be const.

Done with the help of Coccinelle.

// 
@r disable optional_qualifier@
identifier i;
position p;
@@
static struct v4l2_m2m_ops i@p = { ... };

@ok1@
identifier r.i;
position p;
@@
v4l2_m2m_init(@p)

@bad@
position p != {r.p,ok1.p};
identifier r.i;
struct v4l2_m2m_ops e;
@@
e@i@p

@depends on !bad disable optional_qualifier@
identifier r.i;
@@
static
+const
 struct v4l2_m2m_ops i = { ... };
// 

Signed-off-by: Julia Lawall <julia.law...@lip6.fr>

---
 drivers/media/platform/sti/bdisp/bdisp-v4l2.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/sti/bdisp/bdisp-v4l2.c 
b/drivers/media/platform/sti/bdisp/bdisp-v4l2.c
index 7918b92..939da6d 100644
--- a/drivers/media/platform/sti/bdisp/bdisp-v4l2.c
+++ b/drivers/media/platform/sti/bdisp/bdisp-v4l2.c
@@ -360,7 +360,7 @@ static void bdisp_device_run(void *priv)
bdisp_job_finish(ctx, VB2_BUF_STATE_ERROR);
 }
 
-static struct v4l2_m2m_ops bdisp_m2m_ops = {
+static const struct v4l2_m2m_ops bdisp_m2m_ops = {
.device_run = bdisp_device_run,
.job_abort  = bdisp_job_abort,
 };



[PATCH 12/12] [media] mtk-mdp: constify v4l2_m2m_ops structures

2017-08-06 Thread Julia Lawall
The v4l2_m2m_ops structures are only passed as the only
argument to v4l2_m2m_init, which is declared as const.
Thus the v4l2_m2m_ops structures themselves can be const.

Done with the help of Coccinelle.

// 
@r disable optional_qualifier@
identifier i;
position p;
@@
static struct v4l2_m2m_ops i@p = { ... };

@ok1@
identifier r.i;
position p;
@@
v4l2_m2m_init(@p)

@bad@
position p != {r.p,ok1.p};
identifier r.i;
struct v4l2_m2m_ops e;
@@
e@i@p

@depends on !bad disable optional_qualifier@
identifier r.i;
@@
static
+const
 struct v4l2_m2m_ops i = { ... };
// 

Signed-off-by: Julia Lawall <julia.law...@lip6.fr>

---
 drivers/media/platform/mtk-mdp/mtk_mdp_m2m.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/mtk-mdp/mtk_mdp_m2m.c 
b/drivers/media/platform/mtk-mdp/mtk_mdp_m2m.c
index 3038d62..583d477 100644
--- a/drivers/media/platform/mtk-mdp/mtk_mdp_m2m.c
+++ b/drivers/media/platform/mtk-mdp/mtk_mdp_m2m.c
@@ -1225,7 +1225,7 @@ static int mtk_mdp_m2m_release(struct file *file)
.mmap   = v4l2_m2m_fop_mmap,
 };
 
-static struct v4l2_m2m_ops mtk_mdp_m2m_ops = {
+static const struct v4l2_m2m_ops mtk_mdp_m2m_ops = {
.device_run = mtk_mdp_m2m_device_run,
.job_abort  = mtk_mdp_m2m_job_abort,
 };



[PATCH 11/12] [media] exynos4-is: constify v4l2_m2m_ops structures

2017-08-06 Thread Julia Lawall
The v4l2_m2m_ops structures are only passed as the only
argument to v4l2_m2m_init, which is declared as const.
Thus the v4l2_m2m_ops structures themselves can be const.

Done with the help of Coccinelle.

// 
@r disable optional_qualifier@
identifier i;
position p;
@@
static struct v4l2_m2m_ops i@p = { ... };

@ok1@
identifier r.i;
position p;
@@
v4l2_m2m_init(@p)

@bad@
position p != {r.p,ok1.p};
identifier r.i;
struct v4l2_m2m_ops e;
@@
e@i@p

@depends on !bad disable optional_qualifier@
identifier r.i;
@@
static
+const
 struct v4l2_m2m_ops i = { ... };
// 

Signed-off-by: Julia Lawall <julia.law...@lip6.fr>

---
 drivers/media/platform/exynos4-is/fimc-m2m.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/exynos4-is/fimc-m2m.c 
b/drivers/media/platform/exynos4-is/fimc-m2m.c
index d8724fe..9027d0b 100644
--- a/drivers/media/platform/exynos4-is/fimc-m2m.c
+++ b/drivers/media/platform/exynos4-is/fimc-m2m.c
@@ -704,7 +704,7 @@ static int fimc_m2m_release(struct file *file)
.mmap   = v4l2_m2m_fop_mmap,
 };
 
-static struct v4l2_m2m_ops m2m_ops = {
+static const struct v4l2_m2m_ops m2m_ops = {
.device_run = fimc_device_run,
.job_abort  = fimc_job_abort,
 };



[PATCH 09/12] [media] media: mx2-emmaprp: constify v4l2_m2m_ops structures

2017-08-06 Thread Julia Lawall
The v4l2_m2m_ops structures are only passed as the only
argument to v4l2_m2m_init, which is declared as const.
Thus the v4l2_m2m_ops structures themselves can be const.

Done with the help of Coccinelle.

// 
@r disable optional_qualifier@
identifier i;
position p;
@@
static struct v4l2_m2m_ops i@p = { ... };

@ok1@
identifier r.i;
position p;
@@
v4l2_m2m_init(@p)

@bad@
position p != {r.p,ok1.p};
identifier r.i;
struct v4l2_m2m_ops e;
@@
e@i@p

@depends on !bad disable optional_qualifier@
identifier r.i;
@@
static
+const
 struct v4l2_m2m_ops i = { ... };
// 

Signed-off-by: Julia Lawall <julia.law...@lip6.fr>

---
 drivers/media/platform/mx2_emmaprp.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/mx2_emmaprp.c 
b/drivers/media/platform/mx2_emmaprp.c
index 03e47e0..7fd209e 100644
--- a/drivers/media/platform/mx2_emmaprp.c
+++ b/drivers/media/platform/mx2_emmaprp.c
@@ -882,7 +882,7 @@ static int emmaprp_mmap(struct file *file, struct 
vm_area_struct *vma)
.vfl_dir= VFL_DIR_M2M,
 };
 
-static struct v4l2_m2m_ops m2m_ops = {
+static const struct v4l2_m2m_ops m2m_ops = {
.device_run = emmaprp_device_run,
.job_abort  = emmaprp_job_abort,
.lock   = emmaprp_lock,



[PATCH 08/12] [media] m2m-deinterlace: constify v4l2_m2m_ops structures

2017-08-06 Thread Julia Lawall
The v4l2_m2m_ops structures are only passed as the only
argument to v4l2_m2m_init, which is declared as const.
Thus the v4l2_m2m_ops structures themselves can be const.

Done with the help of Coccinelle.

// 
@r disable optional_qualifier@
identifier i;
position p;
@@
static struct v4l2_m2m_ops i@p = { ... };

@ok1@
identifier r.i;
position p;
@@
v4l2_m2m_init(@p)

@bad@
position p != {r.p,ok1.p};
identifier r.i;
struct v4l2_m2m_ops e;
@@
e@i@p

@depends on !bad disable optional_qualifier@
identifier r.i;
@@
static
+const
 struct v4l2_m2m_ops i = { ... };
// 

Signed-off-by: Julia Lawall <julia.law...@lip6.fr>

---
 drivers/media/platform/m2m-deinterlace.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/m2m-deinterlace.c 
b/drivers/media/platform/m2m-deinterlace.c
index 980066b..98f6db2 100644
--- a/drivers/media/platform/m2m-deinterlace.c
+++ b/drivers/media/platform/m2m-deinterlace.c
@@ -988,7 +988,7 @@ static int deinterlace_mmap(struct file *file, struct 
vm_area_struct *vma)
.vfl_dir= VFL_DIR_M2M,
 };
 
-static struct v4l2_m2m_ops m2m_ops = {
+static const struct v4l2_m2m_ops m2m_ops = {
.device_run = deinterlace_device_run,
.job_ready  = deinterlace_job_ready,
.job_abort  = deinterlace_job_abort,



[PATCH 10/12] [media] vim2m: constify v4l2_m2m_ops structures

2017-08-06 Thread Julia Lawall
The v4l2_m2m_ops structures are only passed as the only
argument to v4l2_m2m_init, which is declared as const.
Thus the v4l2_m2m_ops structures themselves can be const.

Done with the help of Coccinelle.

// 
@r disable optional_qualifier@
identifier i;
position p;
@@
static struct v4l2_m2m_ops i@p = { ... };

@ok1@
identifier r.i;
position p;
@@
v4l2_m2m_init(@p)

@bad@
position p != {r.p,ok1.p};
identifier r.i;
struct v4l2_m2m_ops e;
@@
e@i@p

@depends on !bad disable optional_qualifier@
identifier r.i;
@@
static
+const
 struct v4l2_m2m_ops i = { ... };
// 

Signed-off-by: Julia Lawall <julia.law...@lip6.fr>

---
 drivers/media/platform/vim2m.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/vim2m.c b/drivers/media/platform/vim2m.c
index 970b9b6..afbaa35 100644
--- a/drivers/media/platform/vim2m.c
+++ b/drivers/media/platform/vim2m.c
@@ -983,7 +983,7 @@ static int vim2m_release(struct file *file)
.release= video_device_release_empty,
 };
 
-static struct v4l2_m2m_ops m2m_ops = {
+static const struct v4l2_m2m_ops m2m_ops = {
.device_run = device_run,
.job_ready  = job_ready,
.job_abort  = job_abort,



[PATCH 1/6] [media] v4l2-pci-skeleton: constify vb2_ops structures

2017-08-05 Thread Julia Lawall
These vb2_ops structures are only stored in the ops field of a
vb2_queue structure, which is declared as const.  Thus the vb2_ops
structures themselves can be const.

Done with the help of Coccinelle.

// 
@r disable optional_qualifier@
identifier i;
position p;
@@
static struct vb2_ops i@p = { ... };

@ok@
identifier r.i;
struct vb2_queue e;
position p;
@@
e.ops = @p;

@bad@
position p != {r.p,ok.p};
identifier r.i;
struct vb2_ops e;
@@
e@i@p

@depends on !bad disable optional_qualifier@
identifier r.i;
@@
static
+const
 struct vb2_ops i = { ... };
// 

Signed-off-by: Julia Lawall <julia.law...@lip6.fr>

---
There doesn't seem to be a maintainer for this file.

 samples/v4l/v4l2-pci-skeleton.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/samples/v4l/v4l2-pci-skeleton.c b/samples/v4l/v4l2-pci-skeleton.c
index 93b76c3..483e9bca 100644
--- a/samples/v4l/v4l2-pci-skeleton.c
+++ b/samples/v4l/v4l2-pci-skeleton.c
@@ -282,7 +282,7 @@ static void stop_streaming(struct vb2_queue *vq)
  * vb2_ops_wait_prepare/finish helper functions. If q->lock would be NULL,
  * then this driver would have to provide these ops.
  */
-static struct vb2_ops skel_qops = {
+static const struct vb2_ops skel_qops = {
.queue_setup= queue_setup,
.buf_prepare= buffer_prepare,
.buf_queue  = buffer_queue,



[PATCH 0/6] constify vb2_ops structures

2017-08-05 Thread Julia Lawall
These vb2_ops structures are only stored in the ops field of a
vb2_queue structure, which is declared as const.  Thus the vb2_ops
structures themselves can be const.

Done with the help of Coccinelle.

---

 drivers/media/platform/blackfin/bfin_capture.c|2 +-
 drivers/media/platform/davinci/vpbe_display.c |2 +-
 drivers/staging/media/davinci_vpfe/vpfe_video.c   |2 +-
 drivers/staging/media/imx/imx-media-capture.c |4 ++--
 drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c |2 +-
 samples/v4l/v4l2-pci-skeleton.c   |2 +-
 6 files changed, 7 insertions(+), 7 deletions(-)


[PATCH 2/6] [media] media: davinci: vpbe: constify vb2_ops structures

2017-08-05 Thread Julia Lawall
These vb2_ops structures are only stored in the ops field of a
vb2_queue structure, which is declared as const.  Thus the vb2_ops
structures themselves can be const.

Done with the help of Coccinelle.

// 
@r disable optional_qualifier@
identifier i;
position p;
@@
static struct vb2_ops i@p = { ... };

@ok@
identifier r.i;
struct vb2_queue e;
position p;
@@
e.ops = @p;

@bad@
position p != {r.p,ok.p};
identifier r.i;
struct vb2_ops e;
@@
e@i@p

@depends on !bad disable optional_qualifier@
identifier r.i;
@@
static
+const
 struct vb2_ops i = { ... };
// 

Signed-off-by: Julia Lawall <julia.law...@lip6.fr>

---
 drivers/media/platform/davinci/vpbe_display.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/davinci/vpbe_display.c 
b/drivers/media/platform/davinci/vpbe_display.c
index ca2adfa..13d0270 100644
--- a/drivers/media/platform/davinci/vpbe_display.c
+++ b/drivers/media/platform/davinci/vpbe_display.c
@@ -355,7 +355,7 @@ static void vpbe_stop_streaming(struct vb2_queue *vq)
spin_unlock_irqrestore(>dma_queue_lock, flags);
 }
 
-static struct vb2_ops video_qops = {
+static const struct vb2_ops video_qops = {
.queue_setup = vpbe_buffer_queue_setup,
.wait_prepare = vb2_ops_wait_prepare,
.wait_finish = vb2_ops_wait_finish,



[PATCH 4/6] [media] media: blackfin: bfin_capture: constify vb2_ops structures

2017-08-05 Thread Julia Lawall
These vb2_ops structures are only stored in the ops field of a
vb2_queue structure, which is declared as const.  Thus the vb2_ops
structures themselves can be const.

Done with the help of Coccinelle.

// 
@r disable optional_qualifier@
identifier i;
position p;
@@
static struct vb2_ops i@p = { ... };

@ok@
identifier r.i;
struct vb2_queue e;
position p;
@@
e.ops = @p;

@bad@
position p != {r.p,ok.p};
identifier r.i;
struct vb2_ops e;
@@
e@i@p

@depends on !bad disable optional_qualifier@
identifier r.i;
@@
static
+const
 struct vb2_ops i = { ... };
// 

Signed-off-by: Julia Lawall <julia.law...@lip6.fr>

---
 drivers/media/platform/blackfin/bfin_capture.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/blackfin/bfin_capture.c 
b/drivers/media/platform/blackfin/bfin_capture.c
index 294d696..41f1791 100644
--- a/drivers/media/platform/blackfin/bfin_capture.c
+++ b/drivers/media/platform/blackfin/bfin_capture.c
@@ -375,7 +375,7 @@ static void bcap_stop_streaming(struct vb2_queue *vq)
}
 }
 
-static struct vb2_ops bcap_video_qops = {
+static const struct vb2_ops bcap_video_qops = {
.queue_setup= bcap_queue_setup,
.buf_prepare= bcap_buffer_prepare,
.buf_cleanup= bcap_buffer_cleanup,



[PATCH 6/6] [media] media: imx: capture: constify vb2_ops structures

2017-08-05 Thread Julia Lawall
These vb2_ops structures are only stored in the ops field of a
vb2_queue structure, which is declared as const.  Thus the vb2_ops
structures themselves can be const.

Done with the help of Coccinelle.

// 
@r disable optional_qualifier@
identifier i;
position p;
@@
static struct vb2_ops i@p = { ... };

@ok@
identifier r.i;
struct vb2_queue e;
position p;
@@
e.ops = @p;

@bad@
position p != {r.p,ok.p};
identifier r.i;
struct vb2_ops e;
@@
e@i@p

@depends on !bad disable optional_qualifier@
identifier r.i;
@@
static
+const
 struct vb2_ops i = { ... };
// 

Signed-off-by: Julia Lawall <julia.law...@lip6.fr>

---
 drivers/staging/media/imx/imx-media-capture.c |4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/media/imx/imx-media-capture.c 
b/drivers/staging/media/imx/imx-media-capture.c
index ddab4c2..ea145ba 100644
--- a/drivers/staging/media/imx/imx-media-capture.c
+++ b/drivers/staging/media/imx/imx-media-capture.c
@@ -62,7 +62,7 @@ struct capture_priv {
 /* In bytes, per queue */
 #define VID_MEM_LIMIT  SZ_64M
 
-static struct vb2_ops capture_qops;
+static const struct vb2_ops capture_qops;
 
 /*
  * Video ioctls follow
@@ -503,7 +503,7 @@ static void capture_stop_streaming(struct vb2_queue *vq)
spin_unlock_irqrestore(>q_lock, flags);
 }
 
-static struct vb2_ops capture_qops = {
+static const struct vb2_ops capture_qops = {
.queue_setup = capture_queue_setup,
.buf_init= capture_buf_init,
.buf_prepare = capture_buf_prepare,



[PATCH 3/6] [media] staging: media: davinci_vpfe: constify vb2_ops structures

2017-08-05 Thread Julia Lawall
These vb2_ops structures are only stored in the ops field of a
vb2_queue structure, which is declared as const.  Thus the vb2_ops
structures themselves can be const.

Done with the help of Coccinelle.

// 
@r disable optional_qualifier@
identifier i;
position p;
@@
static struct vb2_ops i@p = { ... };

@ok@
identifier r.i;
struct vb2_queue e;
position p;
@@
e.ops = @p;

@bad@
position p != {r.p,ok.p};
identifier r.i;
struct vb2_ops e;
@@
e@i@p

@depends on !bad disable optional_qualifier@
identifier r.i;
@@
static
+const
 struct vb2_ops i = { ... };
// 

Signed-off-by: Julia Lawall <julia.law...@lip6.fr>

---
 drivers/staging/media/davinci_vpfe/vpfe_video.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/media/davinci_vpfe/vpfe_video.c 
b/drivers/staging/media/davinci_vpfe/vpfe_video.c
index 8b2117e..155e8c7 100644
--- a/drivers/staging/media/davinci_vpfe/vpfe_video.c
+++ b/drivers/staging/media/davinci_vpfe/vpfe_video.c
@@ -1304,7 +1304,7 @@ static void vpfe_buf_cleanup(struct vb2_buffer *vb)
list_del_init(>list);
 }
 
-static struct vb2_ops video_qops = {
+static const struct vb2_ops video_qops = {
.queue_setup= vpfe_buffer_queue_setup,
.buf_init   = vpfe_buffer_init,
.buf_prepare= vpfe_buffer_prepare,



[PATCH 1/5] [media] cx18: constify videobuf_queue_ops structures

2017-08-04 Thread Julia Lawall
These videobuf_queue_ops structures are only passed as the second
argument to videobuf_queue_vmalloc_init, which is declared as const.
Thus the videobuf_queue_ops structures themselves can be const.

Done with the help of Coccinelle.

// 
@r disable optional_qualifier@
identifier i;
position p;
@@
static struct videobuf_queue_ops i@p = { ... };

@ok1@
identifier r.i;
expression e1;
position p;
@@
videobuf_queue_vmalloc_init(e1,@p,...)

@bad@
position p != {r.p,ok1.p};
identifier r.i;
struct videobuf_queue_ops e;
@@
e@i@p

@depends on !bad disable optional_qualifier@
identifier r.i;
@@
static
+const
 struct videobuf_queue_ops i = { ... };
// 

Signed-off-by: Julia Lawall <julia.law...@lip6.fr>

---
 drivers/media/pci/cx18/cx18-streams.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/pci/cx18/cx18-streams.c 
b/drivers/media/pci/cx18/cx18-streams.c
index 3c45e007..81d06c1 100644
--- a/drivers/media/pci/cx18/cx18-streams.c
+++ b/drivers/media/pci/cx18/cx18-streams.c
@@ -240,7 +240,7 @@ static void buffer_queue(struct videobuf_queue *q, struct 
videobuf_buffer *vb)
list_add_tail(>vb.queue, >vb_capture);
 }
 
-static struct videobuf_queue_ops cx18_videobuf_qops = {
+static const struct videobuf_queue_ops cx18_videobuf_qops = {
.buf_setup= buffer_setup,
.buf_prepare  = buffer_prepare,
.buf_queue= buffer_queue,



[PATCH 2/5] [media] atomisp: constify videobuf_queue_ops structures

2017-08-04 Thread Julia Lawall
These videobuf_queue_ops structures are only passed as the second
argument to videobuf_queue_vmalloc_init, which is declared as const.
Thus the videobuf_queue_ops structures themselves can be const.

Done with the help of Coccinelle.

// 
@r disable optional_qualifier@
identifier i;
position p;
@@
static struct videobuf_queue_ops i@p = { ... };

@ok1@
identifier r.i;
expression e1;
position p;
@@
videobuf_queue_vmalloc_init(e1,@p,...)

@bad@
position p != {r.p,ok1.p};
identifier r.i;
struct videobuf_queue_ops e;
@@
e@i@p

@depends on !bad disable optional_qualifier@
identifier r.i;
@@
static
+const
 struct videobuf_queue_ops i = { ... };
// 

Signed-off-by: Julia Lawall <julia.law...@lip6.fr>

---
 drivers/staging/media/atomisp/pci/atomisp2/atomisp_fops.c |4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_fops.c 
b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_fops.c
index c151c84..d8cfed3 100644
--- a/drivers/staging/media/atomisp/pci/atomisp2/atomisp_fops.c
+++ b/drivers/staging/media/atomisp/pci/atomisp2/atomisp_fops.c
@@ -643,14 +643,14 @@ static void atomisp_buf_release_output(struct 
videobuf_queue *vq,
vb->state = VIDEOBUF_NEEDS_INIT;
 }
 
-static struct videobuf_queue_ops videobuf_qops = {
+static const struct videobuf_queue_ops videobuf_qops = {
.buf_setup  = atomisp_buf_setup,
.buf_prepare= atomisp_buf_prepare,
.buf_queue  = atomisp_buf_queue,
.buf_release= atomisp_buf_release,
 };
 
-static struct videobuf_queue_ops videobuf_qops_output = {
+static const struct videobuf_queue_ops videobuf_qops_output = {
.buf_setup  = atomisp_buf_setup_output,
.buf_prepare= atomisp_buf_prepare_output,
.buf_queue  = atomisp_buf_queue_output,



[PATCH 0/5] constify videobuf_queue_ops structures

2017-08-04 Thread Julia Lawall
These videobuf_queue_ops structures are only passed as the second
argument to videobuf_queue_vmalloc_init, which is declared as const.
Thus the videobuf_queue_ops structures themselves can be const.

Done with the help of Coccinelle.

---

 drivers/media/pci/cx18/cx18-streams.c |2 +-
 drivers/media/usb/cx231xx/cx231xx-417.c   |2 +-
 drivers/media/usb/cx231xx/cx231xx-video.c |2 +-
 drivers/media/usb/tm6000/tm6000-video.c   |2 +-
 drivers/media/usb/zr364xx/zr364xx.c   |2 +-
 drivers/staging/media/atomisp/pci/atomisp2/atomisp_fops.c |4 ++--
 6 files changed, 7 insertions(+), 7 deletions(-)


[PATCH 3/5] [media] cx231xx: constify videobuf_queue_ops structures

2017-08-04 Thread Julia Lawall
These videobuf_queue_ops structures are only passed as the second
argument to videobuf_queue_vmalloc_init, which is declared as const.
Thus the videobuf_queue_ops structures themselves can be const.

Done with the help of Coccinelle.

// 
@r disable optional_qualifier@
identifier i;
position p;
@@
static struct videobuf_queue_ops i@p = { ... };

@ok1@
identifier r.i;
expression e1;
position p;
@@
videobuf_queue_vmalloc_init(e1,@p,...)

@bad@
position p != {r.p,ok1.p};
identifier r.i;
struct videobuf_queue_ops e;
@@
e@i@p

@depends on !bad disable optional_qualifier@
identifier r.i;
@@
static
+const
 struct videobuf_queue_ops i = { ... };
// 

In the first case, there is a second commented call to
videobuf_queue_sg_init with the structure as the second argument.  If that
code will be uncommented, the const will remain correct, because the second
parameter of that function is also const.

Signed-off-by: Julia Lawall <julia.law...@lip6.fr>

---
 drivers/media/usb/cx231xx/cx231xx-417.c   |2 +-
 drivers/media/usb/cx231xx/cx231xx-video.c |2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/usb/cx231xx/cx231xx-417.c 
b/drivers/media/usb/cx231xx/cx231xx-417.c
index 8d5eb99..d538fa4 100644
--- a/drivers/media/usb/cx231xx/cx231xx-417.c
+++ b/drivers/media/usb/cx231xx/cx231xx-417.c
@@ -1490,7 +1490,7 @@ static void bb_buf_release(struct videobuf_queue *q,
free_buffer(q, buf);
 }
 
-static struct videobuf_queue_ops cx231xx_qops = {
+static const struct videobuf_queue_ops cx231xx_qops = {
.buf_setup= bb_buf_setup,
.buf_prepare  = bb_buf_prepare,
.buf_queue= bb_buf_queue,
diff --git a/drivers/media/usb/cx231xx/cx231xx-video.c 
b/drivers/media/usb/cx231xx/cx231xx-video.c
index f67f868..179b848 100644
--- a/drivers/media/usb/cx231xx/cx231xx-video.c
+++ b/drivers/media/usb/cx231xx/cx231xx-video.c
@@ -859,7 +859,7 @@ static void buffer_release(struct videobuf_queue *vq,
free_buffer(vq, buf);
 }
 
-static struct videobuf_queue_ops cx231xx_video_qops = {
+static const struct videobuf_queue_ops cx231xx_video_qops = {
.buf_setup = buffer_setup,
.buf_prepare = buffer_prepare,
.buf_queue = buffer_queue,



[PATCH 4/5] [media] tm6000: constify videobuf_queue_ops structures

2017-08-04 Thread Julia Lawall
These videobuf_queue_ops structures are only passed as the second
argument to videobuf_queue_vmalloc_init, which is declared as const.
Thus the videobuf_queue_ops structures themselves can be const.

Done with the help of Coccinelle.

// 
@r disable optional_qualifier@
identifier i;
position p;
@@
static struct videobuf_queue_ops i@p = { ... };

@ok1@
identifier r.i;
expression e1;
position p;
@@
videobuf_queue_vmalloc_init(e1,@p,...)

@bad@
position p != {r.p,ok1.p};
identifier r.i;
struct videobuf_queue_ops e;
@@
e@i@p

@depends on !bad disable optional_qualifier@
identifier r.i;
@@
static
+const
 struct videobuf_queue_ops i = { ... };
// 

Signed-off-by: Julia Lawall <julia.law...@lip6.fr>

---
 drivers/media/usb/tm6000/tm6000-video.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/usb/tm6000/tm6000-video.c 
b/drivers/media/usb/tm6000/tm6000-video.c
index cec1321..ec8c4d2 100644
--- a/drivers/media/usb/tm6000/tm6000-video.c
+++ b/drivers/media/usb/tm6000/tm6000-video.c
@@ -801,7 +801,7 @@ static void buffer_release(struct videobuf_queue *vq, 
struct videobuf_buffer *vb
free_buffer(vq, buf);
 }
 
-static struct videobuf_queue_ops tm6000_video_qops = {
+static const struct videobuf_queue_ops tm6000_video_qops = {
.buf_setup  = buffer_setup,
.buf_prepare= buffer_prepare,
.buf_queue  = buffer_queue,



[PATCH 5/5] [media] zr364xx: constify videobuf_queue_ops structures

2017-08-04 Thread Julia Lawall
These videobuf_queue_ops structures are only passed as the second
argument to videobuf_queue_vmalloc_init, which is declared as const.
Thus the videobuf_queue_ops structures themselves can be const.

Done with the help of Coccinelle.

// 
@r disable optional_qualifier@
identifier i;
position p;
@@
static struct videobuf_queue_ops i@p = { ... };

@ok1@
identifier r.i;
expression e1;
position p;
@@
videobuf_queue_vmalloc_init(e1,@p,...)

@bad@
position p != {r.p,ok1.p};
identifier r.i;
struct videobuf_queue_ops e;
@@
e@i@p

@depends on !bad disable optional_qualifier@
identifier r.i;
@@
static
+const
 struct videobuf_queue_ops i = { ... };
// 

Signed-off-by: Julia Lawall <julia.law...@lip6.fr>

---
 drivers/media/usb/zr364xx/zr364xx.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/usb/zr364xx/zr364xx.c 
b/drivers/media/usb/zr364xx/zr364xx.c
index efdcd5b..24d5860 100644
--- a/drivers/media/usb/zr364xx/zr364xx.c
+++ b/drivers/media/usb/zr364xx/zr364xx.c
@@ -439,7 +439,7 @@ static void buffer_release(struct videobuf_queue *vq,
free_buffer(vq, buf);
 }
 
-static struct videobuf_queue_ops zr364xx_video_qops = {
+static const struct videobuf_queue_ops zr364xx_video_qops = {
.buf_setup = buffer_setup,
.buf_prepare = buffer_prepare,
.buf_queue = buffer_queue,



[PATCH] staging: media: use relevant lock

2017-08-03 Thread Julia Lawall
The data protected is video_out2 and the lock that is released is
_out2->dma_queue_lock, so it seems that that lock should be
taken as well.

Signed-off-by: Julia Lawall <julia.law...@lip6.fr>

---
 drivers/staging/media/davinci_vpfe/dm365_resizer.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/media/davinci_vpfe/dm365_resizer.c 
b/drivers/staging/media/davinci_vpfe/dm365_resizer.c
index 857b0e8..4910cb7 100644
--- a/drivers/staging/media/davinci_vpfe/dm365_resizer.c
+++ b/drivers/staging/media/davinci_vpfe/dm365_resizer.c
@@ -1059,7 +1059,7 @@ static void resizer_ss_isr(struct vpfe_resizer_device 
*resizer)
/* If resizer B is enabled */
if (pipe->output_num > 1 && resizer->resizer_b.output ==
RESIZER_OUTPUT_MEMORY) {
-   spin_lock(_out->dma_queue_lock);
+   spin_lock(_out2->dma_queue_lock);
vpfe_video_process_buffer_complete(video_out2);
video_out2->state = VPFE_VIDEO_BUFFER_NOT_QUEUED;
vpfe_video_schedule_next_buffer(video_out2);



[PATCH] [media] DaVinci-VPBE: constify vpbe_dev_ops

2017-08-02 Thread Julia Lawall
vpbe_dev_ops is only copied into the ops field at the end of a vpbe_device
structure, so it can be const.

Signed-off-by: Julia Lawall <julia.law...@lip6.fr>

---

Does the ops field need to be inlined into the vpbe_device structure?

 drivers/media/platform/davinci/vpbe.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/davinci/vpbe.c 
b/drivers/media/platform/davinci/vpbe.c
index 3679b1e..7f64625 100644
--- a/drivers/media/platform/davinci/vpbe.c
+++ b/drivers/media/platform/davinci/vpbe.c
@@ -790,7 +790,7 @@ static void vpbe_deinitialize(struct device *dev, struct 
vpbe_device *vpbe_dev)
vpss_enable_clock(VPSS_VPBE_CLOCK, 0);
 }
 
-static struct vpbe_device_ops vpbe_dev_ops = {
+static const struct vpbe_device_ops vpbe_dev_ops = {
.g_cropcap = vpbe_g_cropcap,
.enum_outputs = vpbe_enum_outputs,
.set_output = vpbe_set_output,



[ragnatech:media-tree 2075/2144] drivers/media/dvb-frontends/stv0910.c:1185:2-8: preceding lock on line 1176 (fwd)

2017-07-23 Thread Julia Lawall
Is a lock release needed before line 1185?  Is the !enable in line 1189
correct?  If the code is correct as is, perhaps it could be good to add
some comments.

julia

-- Forwarded message --
Date: Mon, 24 Jul 2017 12:55:30 +0800
From: kbuild test robot <fengguang...@intel.com>
To: kbu...@01.org
Cc: Julia Lawall <julia.law...@lip6.fr>
Subject: [ragnatech:media-tree 2075/2144]
drivers/media/dvb-frontends/stv0910.c:1185:2-8: preceding lock on line 1176

CC: kbuild-...@01.org
TO: Daniel Scheller <d.schel...@gmx.net>
CC: Mauro Carvalho Chehab <m.che...@samsung.com>
CC: linux-media@vger.kernel.org

tree:   git://git.ragnatech.se/linux media-tree
head:   0e50e84a11f4854e9a7e3b7f4443ffb99e6be292
commit: cd21b334943719f880e707eb91895fc916a88000 [2075/2144] media: 
dvb-frontends: add ST STV0910 DVB-S/S2 demodulator frontend driver
:: branch date: 3 days ago
:: commit date: 4 days ago

>> drivers/media/dvb-frontends/stv0910.c:1185:2-8: preceding lock on line 1176

git remote add ragnatech git://git.ragnatech.se/linux
git remote update ragnatech
git checkout cd21b334943719f880e707eb91895fc916a88000
vim +1185 drivers/media/dvb-frontends/stv0910.c

cd21b334 Daniel Scheller 2017-07-03  1168
cd21b334 Daniel Scheller 2017-07-03  1169
cd21b334 Daniel Scheller 2017-07-03  1170  static int gate_ctrl(struct 
dvb_frontend *fe, int enable)
cd21b334 Daniel Scheller 2017-07-03  1171  {
cd21b334 Daniel Scheller 2017-07-03  1172   struct stv *state = 
fe->demodulator_priv;
cd21b334 Daniel Scheller 2017-07-03  1173   u8 i2crpt = state->i2crpt & 
~0x86;
cd21b334 Daniel Scheller 2017-07-03  1174
cd21b334 Daniel Scheller 2017-07-03  1175   if (enable)
cd21b334 Daniel Scheller 2017-07-03 @1176   
mutex_lock(>base->i2c_lock);
cd21b334 Daniel Scheller 2017-07-03  1177
cd21b334 Daniel Scheller 2017-07-03  1178   if (enable)
cd21b334 Daniel Scheller 2017-07-03  1179   i2crpt |= 0x80;
cd21b334 Daniel Scheller 2017-07-03  1180   else
cd21b334 Daniel Scheller 2017-07-03  1181   i2crpt |= 0x02;
cd21b334 Daniel Scheller 2017-07-03  1182
cd21b334 Daniel Scheller 2017-07-03  1183   if (write_reg(state, state->nr 
? RSTV0910_P2_I2CRPT :
cd21b334 Daniel Scheller 2017-07-03  1184 
RSTV0910_P1_I2CRPT, i2crpt) < 0)
cd21b334 Daniel Scheller 2017-07-03 @1185   return -EIO;
cd21b334 Daniel Scheller 2017-07-03  1186
cd21b334 Daniel Scheller 2017-07-03  1187   state->i2crpt = i2crpt;
cd21b334 Daniel Scheller 2017-07-03  1188
cd21b334 Daniel Scheller 2017-07-03  1189   if (!enable)
cd21b334 Daniel Scheller 2017-07-03  1190   
mutex_unlock(>base->i2c_lock);
cd21b334 Daniel Scheller 2017-07-03  1191   return 0;
cd21b334 Daniel Scheller 2017-07-03  1192  }
cd21b334 Daniel Scheller 2017-07-03  1193

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


Re: [PATCH 1/3] [media] uvcvideo: variable size controls (fwd)

2017-07-15 Thread Julia Lawall
There is a double free on data if the goto is taken on line 1815.

julia

-- Forwarded message --
Date: Sat, 15 Jul 2017 21:07:03 +0800
From: kbuild test robot <fengguang...@intel.com>
To: kbu...@01.org
Cc: Julia Lawall <julia.law...@lip6.fr>
Subject: Re: [PATCH 1/3] [media] uvcvideo: variable size controls

CC: kbuild-...@01.org
In-Reply-To: <20170714201424.23592-1-philipp.za...@gmail.com>
TO: Philipp Zabel <philipp.za...@gmail.com>
CC: linux-media@vger.kernel.org, Laurent Pinchart 
<laurent.pinch...@ideasonboard.com>, Philipp Zabel <philipp.za...@gmail.com>
CC: Laurent Pinchart <laurent.pinch...@ideasonboard.com>, Philipp Zabel 
<philipp.za...@gmail.com>

Hi Philipp,

[auto build test WARNING on linuxtv-media/master]
[also build test WARNING on v4.12 next-20170714]
[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/Philipp-Zabel/uvcvideo-variable-size-controls/20170715-193137
base:   git://linuxtv.org/media_tree.git master
:: branch date: 2 hours ago
:: commit date: 2 hours ago

>> drivers/media/usb/uvc/uvc_ctrl.c:1857:7-11: ERROR: reference preceded by 
>> free on line 1809

git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout f06e94cde314fba5859cd6c999dde48e94156c7a
vim +1857 drivers/media/usb/uvc/uvc_ctrl.c

52c58ad6f drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-29  1719
8e113595e drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2009-07-01  1720  
int uvc_xu_ctrl_query(struct uvc_video_chain *chain,
fe78d187f drivers/media/video/uvc/uvc_ctrl.c Martin Rubli 2010-10-02  1721  
struct uvc_xu_control_query *xqry)
c0efd2329 drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2008-06-30  1722  
{
c0efd2329 drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2008-06-30  1723  
struct uvc_entity *entity;
fe78d187f drivers/media/video/uvc/uvc_ctrl.c Martin Rubli 2010-10-02  1724  
struct uvc_control *ctrl;
c0efd2329 drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2008-06-30  1725  
unsigned int i, found = 0;
fe78d187f drivers/media/video/uvc/uvc_ctrl.c Martin Rubli 2010-10-02  1726  
__u32 reqflags;
fe78d187f drivers/media/video/uvc/uvc_ctrl.c Martin Rubli 2010-10-02  1727  
__u16 size;
fe78d187f drivers/media/video/uvc/uvc_ctrl.c Martin Rubli 2010-10-02  1728  
__u8 *data = NULL;
c0efd2329 drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2008-06-30  1729  
int ret;
c0efd2329 drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2008-06-30  1730
c0efd2329 drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2008-06-30  1731  
/* Find the extension unit. */
6241d8ca1 drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2009-11-25  1732  
list_for_each_entry(entity, >entities, chain) {
6241d8ca1 drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2009-11-25  1733  
if (UVC_ENTITY_TYPE(entity) == UVC_VC_EXTENSION_UNIT &&
fe78d187f drivers/media/video/uvc/uvc_ctrl.c Martin Rubli 2010-10-02  1734  
entity->id == xqry->unit)
c0efd2329 drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2008-06-30  1735  
break;
c0efd2329 drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2008-06-30  1736  
}
c0efd2329 drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2008-06-30  1737
fe78d187f drivers/media/video/uvc/uvc_ctrl.c Martin Rubli 2010-10-02  1738  
if (entity->id != xqry->unit) {
c0efd2329 drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2008-06-30  1739  
uvc_trace(UVC_TRACE_CONTROL, "Extension unit %u not found.\n",
fe78d187f drivers/media/video/uvc/uvc_ctrl.c Martin Rubli 2010-10-02  1740  
xqry->unit);
fe78d187f drivers/media/video/uvc/uvc_ctrl.c Martin Rubli 2010-10-02  1741  
return -ENOENT;
c0efd2329 drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2008-06-30  1742  
}
c0efd2329 drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2008-06-30  1743
52c58ad6f drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2010-09-29  1744  
/* Find the control and perform delayed initialization if needed. */
c0efd2329 drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2008-06-30  1745  
for (i = 0; i < entity->ncontrols; ++i) {
c0efd2329 drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2008-06-30  1746  
ctrl = >controls[i];
fe78d187f drivers/media/video/uvc/uvc_ctrl.c Martin Rubli 2010-10-02  1747  
if (ctrl->index == xqry->selector - 1) {
c0efd2329 drivers/media/video/uvc/uvc_ctrl.c Laurent Pinchart 2008-06-30  1748  
found = 1;
c0efd2329 drivers/media/video/uvc/uvc_ctrl.c Laurent 

Re: [PATCH 2/2] v4l: cadence: Add Cadence MIPI-CSI2 RX driver (fwd)

2017-07-06 Thread Julia Lawall
Please check on this (line 199).

julia

-- Forwarded message --
Date: Thu, 6 Jul 2017 13:58:29 +0800
From: kbuild test robot <fengguang...@intel.com>
To: kbu...@01.org
Cc: Julia Lawall <julia.law...@lip6.fr>
Subject: Re: [PATCH 2/2] v4l: cadence: Add Cadence MIPI-CSI2 RX driver

Hi Maxime,

[auto build test WARNING on linuxtv-media/master]
[also build test WARNING on v4.12]
[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/Maxime-Ripard/dt-bindings-media-Add-Cadence-MIPI-CSI2RX-Device-Tree-bindings/20170705-205643
base:   git://linuxtv.org/media_tree.git master
:: branch date: 17 hours ago
:: commit date: 17 hours ago

>> drivers/media/platform/cadence/cdns-csi2rx.c:199:5-23: WARNING: Unsigned 
>> expression compared with zero: csi2rx -> sensor_pad < 0

git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout fb3d2879ba0623bcdc83c99ba70229ec1713feaf
vim +199 drivers/media/platform/cadence/cdns-csi2rx.c

fb3d2879 Maxime Ripard 2017-07-03  183  return 
media_create_pad_link(>sensor_subdev->entity,
fb3d2879 Maxime Ripard 2017-07-03  184   
csi2rx->sensor_pad,
fb3d2879 Maxime Ripard 2017-07-03  185   
>subdev.entity, 0,
fb3d2879 Maxime Ripard 2017-07-03  186   
MEDIA_LNK_FL_ENABLED |
fb3d2879 Maxime Ripard 2017-07-03  187   
MEDIA_LNK_FL_IMMUTABLE);
fb3d2879 Maxime Ripard 2017-07-03  188  }
fb3d2879 Maxime Ripard 2017-07-03  189
fb3d2879 Maxime Ripard 2017-07-03  190  static int csi2rx_async_bound(struct 
v4l2_async_notifier *notifier,
fb3d2879 Maxime Ripard 2017-07-03  191struct 
v4l2_subdev *subdev,
fb3d2879 Maxime Ripard 2017-07-03  192struct 
v4l2_async_subdev *asd)
fb3d2879 Maxime Ripard 2017-07-03  193  {
fb3d2879 Maxime Ripard 2017-07-03  194  struct csi2rx_priv *csi2rx = 
v4l2_notifier_to_csi2rx(notifier);
fb3d2879 Maxime Ripard 2017-07-03  195
fb3d2879 Maxime Ripard 2017-07-03  196  csi2rx->sensor_pad = 
media_entity_get_fwnode_pad(>entity,
fb3d2879 Maxime Ripard 2017-07-03  197  
 >sensor_node->fwnode,
fb3d2879 Maxime Ripard 2017-07-03  198  
 MEDIA_PAD_FL_SOURCE);
fb3d2879 Maxime Ripard 2017-07-03 @199  if (csi2rx->sensor_pad < 0) {
fb3d2879 Maxime Ripard 2017-07-03  200  dev_err(csi2rx->dev, 
"Couldn't find output pad for subdev %s\n",
fb3d2879 Maxime Ripard 2017-07-03  201  subdev->name);
fb3d2879 Maxime Ripard 2017-07-03  202  return 
csi2rx->sensor_pad;
fb3d2879 Maxime Ripard 2017-07-03  203  }
fb3d2879 Maxime Ripard 2017-07-03  204
fb3d2879 Maxime Ripard 2017-07-03  205  csi2rx->sensor_subdev = subdev;
fb3d2879 Maxime Ripard 2017-07-03  206
fb3d2879 Maxime Ripard 2017-07-03  207  dev_dbg(csi2rx->dev, "Bound %s 
pad: %d\n", subdev->name,

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


Re: [PATCH v1 5/6] [media] ov9650: add multiple variant support (fwd)

2017-06-25 Thread Julia Lawall
Braces are probably missing aroud lines 1618-1620.

julia

-- Forwarded message --
Date: Sun, 25 Jun 2017 23:06:03 +0800
From: kbuild test robot <fengguang...@intel.com>
To: kbu...@01.org
Cc: Julia Lawall <julia.law...@lip6.fr>
Subject: Re: [PATCH v1 5/6] [media] ov9650: add multiple variant support

Hi Hugues,

[auto build test WARNING on linuxtv-media/master]
[also build test WARNING on v4.12-rc6 next-20170623]
[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/Hugues-Fruchet/Add-support-of-OV9655-camera/20170625-201153
base:   git://linuxtv.org/media_tree.git master
:: branch date: 3 hours ago
:: commit date: 3 hours ago

>> drivers/media/i2c/ov9650.c:1618:2-44: code aligned with following code on 
>> line 1619

git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout a9fe8c23240a7f8df39c6238d98e41f41fedb641
vim +1618 drivers/media/i2c/ov9650.c

a9fe8c23 Hugues Fruchet 2017-06-22  1612ov965x->set_params = 
__ov965x_set_params;
84a15ded Sylwester Nawrocki 2012-12-26  1613
a9fe8c23 Hugues Fruchet 2017-06-22  1614ov965x->frame_size = 
>framesizes[0];
a9fe8c23 Hugues Fruchet 2017-06-22  1615
ov965x_get_default_format(ov965x, >format);
a9fe8c23 Hugues Fruchet 2017-06-22  1616
a9fe8c23 Hugues Fruchet 2017-06-22  1617if (ov965x->initialize_controls)
a9fe8c23 Hugues Fruchet 2017-06-22 @1618ret = 
ov965x->initialize_controls(ov965x);
84a15ded Sylwester Nawrocki 2012-12-26 @1619if (ret < 0)
84a15ded Sylwester Nawrocki 2012-12-26  1620goto err_ctrls;
84a15ded Sylwester Nawrocki 2012-12-26  1621
84a15ded Sylwester Nawrocki 2012-12-26  1622/* Update exposure time min/max 
to match frame format */

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


[ragnatech:media-tree 1869/1907] drivers/media/dvb-frontends/stv0367.c:3127:3-16: duplicated argument to & or | (fwd)

2017-06-21 Thread Julia Lawall
It seems that some of the constants on lines 3127 and on are the same as
the ones on lines 3134 and on.

julia

-- Forwarded message --
Date: Wed, 21 Jun 2017 18:20:03 +0800
From: kbuild test robot <fengguang...@intel.com>
To: kbu...@01.org
Cc: Julia Lawall <julia.law...@lip6.fr>
Subject: [ragnatech:media-tree 1869/1907]
drivers/media/dvb-frontends/stv0367.c:3127:3-16: duplicated argument to & or
 |

CC: kbuild-...@01.org
TO: Daniel Scheller <d.schel...@gmx.net>
CC: Mauro Carvalho Chehab <m.che...@samsung.com>
CC: linux-media@vger.kernel.org

tree:   git://git.ragnatech.se/linux media-tree
head:   76724b30f222067faf00874dc277f6c99d03d800
commit: dbbac11e1de1250ad39bbc15490c8614ac7f9def [1869/1907] [media] 
dvb-frontends/stv0367: add Digital Devices compatibility
:: branch date: 20 hours ago
:: commit date: 22 hours ago

>> drivers/media/dvb-frontends/stv0367.c:3127:3-16: duplicated argument to & or 
>> |
   drivers/media/dvb-frontends/stv0367.c:3128:3-16: duplicated argument to & or 
|
   drivers/media/dvb-frontends/stv0367.c:3128:19-33: duplicated argument to & 
or |
   drivers/media/dvb-frontends/stv0367.c:3129:3-17: duplicated argument to & or 
|
   drivers/media/dvb-frontends/stv0367.c:3129:20-35: duplicated argument to & 
or |

git remote add ragnatech git://git.ragnatech.se/linux
git remote update ragnatech
git checkout dbbac11e1de1250ad39bbc15490c8614ac7f9def
vim +3127 drivers/media/dvb-frontends/stv0367.c

dbbac11e Daniel Scheller 2017-03-29  3111
dbbac11e Daniel Scheller 2017-03-29  3112   return 0;
dbbac11e Daniel Scheller 2017-03-29  3113  }
dbbac11e Daniel Scheller 2017-03-29  3114
dbbac11e Daniel Scheller 2017-03-29  3115  static const struct dvb_frontend_ops 
stv0367ddb_ops = {
dbbac11e Daniel Scheller 2017-03-29  3116   .delsys = { SYS_DVBC_ANNEX_A, 
SYS_DVBT },
dbbac11e Daniel Scheller 2017-03-29  3117   .info = {
dbbac11e Daniel Scheller 2017-03-29  3118   .name   
= "ST STV0367 DDB DVB-C/T",
dbbac11e Daniel Scheller 2017-03-29  3119   .frequency_min  
= 4700,
dbbac11e Daniel Scheller 2017-03-29  3120   .frequency_max  
= 86500,
dbbac11e Daniel Scheller 2017-03-29  3121   .frequency_stepsize 
= 17,
dbbac11e Daniel Scheller 2017-03-29  3122   .frequency_tolerance
= 0,
dbbac11e Daniel Scheller 2017-03-29  3123   .symbol_rate_min
= 87,
dbbac11e Daniel Scheller 2017-03-29  3124   .symbol_rate_max
= 1170,
dbbac11e Daniel Scheller 2017-03-29  3125   .caps = /* DVB-C */
dbbac11e Daniel Scheller 2017-03-29  3126   0x400 |/* 
FE_CAN_QAM_4 */
dbbac11e Daniel Scheller 2017-03-29 @3127   FE_CAN_QAM_16 | 
FE_CAN_QAM_32  |
dbbac11e Daniel Scheller 2017-03-29  3128   FE_CAN_QAM_64 | 
FE_CAN_QAM_128 |
dbbac11e Daniel Scheller 2017-03-29  3129   FE_CAN_QAM_256 
| FE_CAN_FEC_AUTO |
dbbac11e Daniel Scheller 2017-03-29  3130   /* DVB-T */
dbbac11e Daniel Scheller 2017-03-29  3131   FE_CAN_FEC_1_2 
| FE_CAN_FEC_2_3 |
dbbac11e Daniel Scheller 2017-03-29  3132   FE_CAN_FEC_3_4 
| FE_CAN_FEC_5_6 | FE_CAN_FEC_7_8 |
dbbac11e Daniel Scheller 2017-03-29  3133   FE_CAN_FEC_AUTO 
|
dbbac11e Daniel Scheller 2017-03-29  3134   FE_CAN_QPSK | 
FE_CAN_QAM_16 | FE_CAN_QAM_64 |
dbbac11e Daniel Scheller 2017-03-29  3135   FE_CAN_QAM_128 
| FE_CAN_QAM_256 | FE_CAN_QAM_AUTO |

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


Re: [PATCH v4 2/2] cec: add STM32 cec driver (fwd)

2017-05-29 Thread Julia Lawall
BRDNOGEN is duplicate in the #defined on line 46.

julia

-- Forwarded message --
Date: Mon, 29 May 2017 19:16:10 +0800
From: kbuild test robot <fengguang...@intel.com>
To: kbu...@01.org
Cc: Julia Lawall <julia.law...@lip6.fr>
Subject: Re: [PATCH v4 2/2] cec: add STM32 cec driver

CC: kbuild-...@01.org
In-Reply-To: <1496046855-5809-3-git-send-email-benjamin.gaign...@linaro.org>
TO: Benjamin Gaignard <benjamin.gaign...@linaro.org>
CC: yannick.fer...@st.com, alexandre.tor...@st.com, hverk...@xs4all.nl, 
devicet...@vger.kernel.org, linux-media@vger.kernel.org, r...@kernel.org, 
hans.verk...@cisco.com
CC: Benjamin Gaignard <benjamin.gaign...@linaro.org>

Hi Benjamin,

[auto build test WARNING on linuxtv-media/master]
[also build test WARNING on v4.12-rc3 next-20170529]
[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/Benjamin-Gaignard/cec-STM32-driver/20170529-172722
base:   git://linuxtv.org/media_tree.git master
:: branch date: 2 hours ago
:: commit date: 2 hours ago

>> drivers/media/platform/stm32/stm32-cec.c:46:33-41: duplicated argument to & 
>> or |

git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout 8864245090acf32561bbec305dd8be5cfe31f1e1
vim +46 drivers/media/platform/stm32/stm32-cec.c

88642450 Benjamin Gaignard 2017-05-29  30  #define CEC_ISR  0x0010 
/* Interrupt and status Register */
88642450 Benjamin Gaignard 2017-05-29  31  #define CEC_IER  0x0014 
/* Interrupt enable Register */
88642450 Benjamin Gaignard 2017-05-29  32
88642450 Benjamin Gaignard 2017-05-29  33  #define TXEOMBIT(2)
88642450 Benjamin Gaignard 2017-05-29  34  #define TXSOMBIT(1)
88642450 Benjamin Gaignard 2017-05-29  35  #define CECENBIT(0)
88642450 Benjamin Gaignard 2017-05-29  36
88642450 Benjamin Gaignard 2017-05-29  37  #define LSTN BIT(31)
88642450 Benjamin Gaignard 2017-05-29  38  #define OAR  GENMASK(30, 16)
88642450 Benjamin Gaignard 2017-05-29  39  #define SFTOPBIT(8)
88642450 Benjamin Gaignard 2017-05-29  40  #define BRDNOGEN BIT(7)
88642450 Benjamin Gaignard 2017-05-29  41  #define LBPEGEN  BIT(6)
88642450 Benjamin Gaignard 2017-05-29  42  #define BREGEN   BIT(5)
88642450 Benjamin Gaignard 2017-05-29  43  #define BRESTP   BIT(4)
88642450 Benjamin Gaignard 2017-05-29  44  #define RXTOLBIT(3)
88642450 Benjamin Gaignard 2017-05-29  45  #define SFT  GENMASK(2, 0)
88642450 Benjamin Gaignard 2017-05-29 @46  #define FULL_CFG (LSTN | SFTOP | 
BRDNOGEN | LBPEGEN | BREGEN | BRESTP \
88642450 Benjamin Gaignard 2017-05-29  47| RXTOL | 
BRDNOGEN)
88642450 Benjamin Gaignard 2017-05-29  48
88642450 Benjamin Gaignard 2017-05-29  49  #define TXACKE   BIT(12)
88642450 Benjamin Gaignard 2017-05-29  50  #define TXERRBIT(11)
88642450 Benjamin Gaignard 2017-05-29  51  #define TXUDRBIT(10)
88642450 Benjamin Gaignard 2017-05-29  52  #define TXENDBIT(9)
88642450 Benjamin Gaignard 2017-05-29  53  #define TXBR BIT(8)
88642450 Benjamin Gaignard 2017-05-29  54  #define ARBLST   BIT(7)

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


Re: [PATCH 04/22] target: Make use of the new sg_map function at 16 call sites (fwd)

2017-04-14 Thread Julia Lawall
It looks like >cmdr_lock should be released at line 512 if it has
not been released otherwise.  The lock was taken at line 438.

julia

-- Forwarded message --
Date: Fri, 14 Apr 2017 22:21:44 +0800
From: kbuild test robot <fengguang...@intel.com>
To: kbu...@01.org
Cc: Julia Lawall <julia.law...@lip6.fr>
Subject: Re: [PATCH 04/22] target: Make use of the new sg_map function at 16
call sites

Hi Logan,

[auto build test WARNING on scsi/for-next]
[also build test WARNING on v4.11-rc6]
[cannot apply to next-20170413]
[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/Logan-Gunthorpe/Introduce-common-scatterlist-map-function/20170414-142518
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi.git for-next
:: branch date: 8 hours ago
:: commit date: 8 hours ago

>> drivers/target/target_core_user.c:512:2-8: preceding lock on line 438
   drivers/target/target_core_user.c:512:2-8: preceding lock on line 471

git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout 78082134e7afdc59d744eb8d2def5c588e89c378
vim +512 drivers/target/target_core_user.c

7c9e7a6f Andy Grover  2014-10-01  432   
sizeof(struct tcmu_cmd_entry));
7c9e7a6f Andy Grover  2014-10-01  433   command_size = base_command_size
7c9e7a6f Andy Grover  2014-10-01  434   + 
round_up(scsi_command_size(se_cmd->t_task_cdb), TCMU_OP_ALIGN_SIZE);
7c9e7a6f Andy Grover  2014-10-01  435
7c9e7a6f Andy Grover  2014-10-01  436   WARN_ON(command_size & 
(TCMU_OP_ALIGN_SIZE-1));
7c9e7a6f Andy Grover  2014-10-01  437
7c9e7a6f Andy Grover  2014-10-01 @438   spin_lock_irq(>cmdr_lock);
7c9e7a6f Andy Grover  2014-10-01  439
7c9e7a6f Andy Grover  2014-10-01  440   mb = udev->mb_addr;
7c9e7a6f Andy Grover  2014-10-01  441   cmd_head = mb->cmd_head % 
udev->cmdr_size; /* UAM */
26418649 Sheng Yang   2016-02-26  442   data_length = 
se_cmd->data_length;
26418649 Sheng Yang   2016-02-26  443   if (se_cmd->se_cmd_flags & 
SCF_BIDI) {
26418649 Sheng Yang   2016-02-26  444   
BUG_ON(!(se_cmd->t_bidi_data_sg && se_cmd->t_bidi_data_nents));
26418649 Sheng Yang   2016-02-26  445   data_length += 
se_cmd->t_bidi_data_sg->length;
26418649 Sheng Yang   2016-02-26  446   }
554617b2 Andy Grover  2016-08-25  447   if ((command_size > 
(udev->cmdr_size / 2)) ||
554617b2 Andy Grover  2016-08-25  448   data_length > 
udev->data_size) {
554617b2 Andy Grover  2016-08-25  449   pr_warn("TCMU: Request 
of size %zu/%zu is too big for %u/%zu "
3d9b9555 Andy Grover  2016-08-25  450   "cmd ring/data 
area\n", command_size, data_length,
7c9e7a6f Andy Grover  2014-10-01  451   
udev->cmdr_size, udev->data_size);
554617b2 Andy Grover  2016-08-25  452   
spin_unlock_irq(>cmdr_lock);
554617b2 Andy Grover  2016-08-25  453   return 
TCM_INVALID_CDB_FIELD;
554617b2 Andy Grover  2016-08-25  454   }
7c9e7a6f Andy Grover  2014-10-01  455
26418649 Sheng Yang   2016-02-26  456   while 
(!is_ring_space_avail(udev, command_size, data_length)) {
7c9e7a6f Andy Grover  2014-10-01  457   int ret;
7c9e7a6f Andy Grover  2014-10-01  458   DEFINE_WAIT(__wait);
7c9e7a6f Andy Grover  2014-10-01  459
7c9e7a6f Andy Grover  2014-10-01  460   
prepare_to_wait(>wait_cmdr, &__wait, TASK_INTERRUPTIBLE);
7c9e7a6f Andy Grover  2014-10-01  461
7c9e7a6f Andy Grover  2014-10-01  462   pr_debug("sleeping for 
ring space\n");
7c9e7a6f Andy Grover  2014-10-01  463   
spin_unlock_irq(>cmdr_lock);
7c9e7a6f Andy Grover  2014-10-01  464   ret = 
schedule_timeout(msecs_to_jiffies(TCMU_TIME_OUT));
7c9e7a6f Andy Grover  2014-10-01  465   
finish_wait(>wait_cmdr, &__wait);
7c9e7a6f Andy Grover  2014-10-01  466   if (!ret) {
7c9e7a6f Andy Grover  2014-10-01  467   pr_warn("tcmu: 
command timed out\n");
02eb924f Andy Grover  2016-10-06  468   return 
TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE;
7c9e7a6f Andy Grover  2014-10-01  469   }
7c9e7a6f Andy Grover  2014-10-01  470
7c9e7a6f Andy Grover  2014-10-01  471   
spin_lock_irq(>cmdr_lock);
7c9e7a6f Andy Grover  2014-10-01  472
7c9e7a6f Andy Grover  2014-10-01  473   /* We dropped 
cmdr_lock, cmd_head is stale */
7c9e7a6f Andy Grover  2014-10-01  474   cmd_head = mb->cmd_head 
% udev->cmdr_size; /* UAM */
7c9e7a6f Andy Grover

Re: [Patch v3 10/11] [media] s5p-mfc: Add support for HEVC encoder (fwd)

2017-04-11 Thread Julia Lawall


On Tue, 11 Apr 2017, Sylwester Nawrocki wrote:

> Hi,
>
> On 04/03/2017 08:17 AM, Smitha T Murthy wrote:
> > On Mon, 2017-04-03 at 08:00 +0200, Julia Lawall wrote:
> > > See line 2101
> > >
> > > julia
> > >
> > Thank you for bringing it to my notice, I had not checked on this git.
> > I will upload the next version of patches soon corresponding to this
> > git.
>
> In general please use the media master branch as a base for your patches
> (git://linuxtv.org/media_tree.git master). Or latest branch in my
> git repository, currently it's "for-v4.12/media/next-2" as can be seen
> here: https://git.linuxtv.org/snawrocki/samsung.git

I'm not making the patch.  It comes to me from kbuild.  If you would
prefer some tree not to be included, you can notify Fengguang about this:

fengguang...@intel.com

julia

>
> --
> Thanks,
> Sylwester
>


Re: [Patch v3 10/11] [media] s5p-mfc: Add support for HEVC encoder (fwd)

2017-04-03 Thread Julia Lawall
See line 2101

julia

- Forwarded message --
Date: Mon, 3 Apr 2017 05:04:39 +0800
From: kbuild test robot <fengguang...@intel.com>
To: kbu...@01.org
Cc: Julia Lawall <julia.law...@lip6.fr>
Subject: Re: [Patch v3 10/11] [media] s5p-mfc: Add support for HEVC encoder

Hi Smitha,

[auto build test WARNING on linuxtv-media/master]
[also build test WARNING on v4.11-rc4 next-20170331]
[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/Smitha-T-Murthy/Add-MFC-v10-10-support/20170403-033620
base:   git://linuxtv.org/media_tree.git master
:: branch date: 88 minutes ago
:: commit date: 88 minutes ago

>> drivers/media/platform/s5p-mfc/s5p_mfc_enc.c:2101:6-25: WARNING: Unsigned 
>> expression compared with zero: p -> codec . hevc . level < 0

git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout bca072db65317d79554527391338ce8bc6fbde58
vim +2101 drivers/media/platform/s5p-mfc/s5p_mfc_enc.c

bca072db Smitha T Murthy 2017-03-31  2085   break;
bca072db Smitha T Murthy 2017-03-31  2086   case 
V4L2_CID_MPEG_VIDEO_HEVC_B_FRAME_QP:
bca072db Smitha T Murthy 2017-03-31  2087   
p->codec.hevc.rc_b_frame_qp = ctrl->val;
bca072db Smitha T Murthy 2017-03-31  2088   break;
bca072db Smitha T Murthy 2017-03-31  2089   case 
V4L2_CID_MPEG_VIDEO_HEVC_FRAME_RATE_RESOLUTION:
bca072db Smitha T Murthy 2017-03-31  2090   
p->codec.hevc.rc_framerate = ctrl->val;
bca072db Smitha T Murthy 2017-03-31  2091   break;
bca072db Smitha T Murthy 2017-03-31  2092   case 
V4L2_CID_MPEG_VIDEO_HEVC_MIN_QP:
bca072db Smitha T Murthy 2017-03-31  2093   p->codec.hevc.rc_min_qp 
= ctrl->val;
bca072db Smitha T Murthy 2017-03-31  2094   break;
bca072db Smitha T Murthy 2017-03-31  2095   case 
V4L2_CID_MPEG_VIDEO_HEVC_MAX_QP:
bca072db Smitha T Murthy 2017-03-31  2096   p->codec.hevc.rc_max_qp 
= ctrl->val;
bca072db Smitha T Murthy 2017-03-31  2097   break;
bca072db Smitha T Murthy 2017-03-31  2098   case 
V4L2_CID_MPEG_VIDEO_HEVC_LEVEL:
bca072db Smitha T Murthy 2017-03-31  2099   
p->codec.hevc.level_v4l2 = ctrl->val;
bca072db Smitha T Murthy 2017-03-31  2100   p->codec.hevc.level = 
hevc_level(ctrl->val);
bca072db Smitha T Murthy 2017-03-31 @2101   if (p->codec.hevc.level 
< 0) {
bca072db Smitha T Murthy 2017-03-31  2102   mfc_err("Level 
number is wrong\n");
bca072db Smitha T Murthy 2017-03-31  2103   ret = 
p->codec.hevc.level;
bca072db Smitha T Murthy 2017-03-31  2104   }
bca072db Smitha T Murthy 2017-03-31  2105   break;
bca072db Smitha T Murthy 2017-03-31  2106   case 
V4L2_CID_MPEG_VIDEO_HEVC_PROFILE:
bca072db Smitha T Murthy 2017-03-31  2107   switch (ctrl->val) {
bca072db Smitha T Murthy 2017-03-31  2108   case 
V4L2_MPEG_VIDEO_HEVC_PROFILE_MAIN:
bca072db Smitha T Murthy 2017-03-31  2109   ctrl->val = 
V4L2_MPEG_VIDEO_HEVC_PROFILE_MAIN;

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


Re: [Outreachy kernel] [PATCH v2] staging: media: davinci_vpfe:Replace a bit shift.

2017-03-24 Thread Julia Lawall
There should be a spac after every colon in the subject.  Please pay
attention to these small details, so you don't have to send the same patch
over and over.

julia

On Fri, 24 Mar 2017, Arushi Singhal wrote:

> This patch replaces bit shifting on 1 with the BIT(x) macro.
> This was done with coccinelle:
> @@
> constant c;
> @@
>
> -1 << c
> +BIT(c)
>
> Signed-off-by: Arushi Singhal <arushisinghal19971...@gmail.com>
> ---
> changes in v2
>  -Remove unnecessary parenthesis.
>
>  drivers/staging/media/davinci_vpfe/dm365_ipipe.c   |  2 +-
>  drivers/staging/media/davinci_vpfe/dm365_ipipeif.c |  2 +-
>  drivers/staging/media/davinci_vpfe/dm365_isif.c| 10 +-
>  drivers/staging/media/davinci_vpfe/dm365_resizer.c |  6 +++---
>  4 files changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/staging/media/davinci_vpfe/dm365_ipipe.c 
> b/drivers/staging/media/davinci_vpfe/dm365_ipipe.c
> index 6a3434cebd79..7eeb53217168 100644
> --- a/drivers/staging/media/davinci_vpfe/dm365_ipipe.c
> +++ b/drivers/staging/media/davinci_vpfe/dm365_ipipe.c
> @@ -1815,7 +1815,7 @@ vpfe_ipipe_init(struct vpfe_ipipe_device *ipipe, struct 
> platform_device *pdev)
>   v4l2_subdev_init(sd, _v4l2_ops);
>   sd->internal_ops = _v4l2_internal_ops;
>   strlcpy(sd->name, "DAVINCI IPIPE", sizeof(sd->name));
> - sd->grp_id = 1 << 16;   /* group ID for davinci subdevs */
> + sd->grp_id = BIT(16);   /* group ID for davinci subdevs */
>   v4l2_set_subdevdata(sd, ipipe);
>   sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
>
> diff --git a/drivers/staging/media/davinci_vpfe/dm365_ipipeif.c 
> b/drivers/staging/media/davinci_vpfe/dm365_ipipeif.c
> index 46fd2c7f69c3..c07f028dd6be 100644
> --- a/drivers/staging/media/davinci_vpfe/dm365_ipipeif.c
> +++ b/drivers/staging/media/davinci_vpfe/dm365_ipipeif.c
> @@ -1021,7 +1021,7 @@ int vpfe_ipipeif_init(struct vpfe_ipipeif_device 
> *ipipeif,
>
>   sd->internal_ops = _v4l2_internal_ops;
>   strlcpy(sd->name, "DAVINCI IPIPEIF", sizeof(sd->name));
> - sd->grp_id = 1 << 16;   /* group ID for davinci subdevs */
> + sd->grp_id = BIT(16);   /* group ID for davinci subdevs */
>
>   v4l2_set_subdevdata(sd, ipipeif);
>
> diff --git a/drivers/staging/media/davinci_vpfe/dm365_isif.c 
> b/drivers/staging/media/davinci_vpfe/dm365_isif.c
> index 569bcdc9ce2f..74b1247203b1 100644
> --- a/drivers/staging/media/davinci_vpfe/dm365_isif.c
> +++ b/drivers/staging/media/davinci_vpfe/dm365_isif.c
> @@ -821,7 +821,7 @@ isif_config_dfc(struct vpfe_isif_device *isif, struct 
> vpfe_isif_dfc *vdfc)
>
>   /* Correct whole line or partial */
>   if (vdfc->corr_whole_line)
> - val |= 1 << ISIF_VDFC_CORR_WHOLE_LN_SHIFT;
> + val |= BIT(ISIF_VDFC_CORR_WHOLE_LN_SHIFT);
>
>   /* level shift value */
>   val |= (vdfc->def_level_shift & ISIF_VDFC_LEVEL_SHFT_MASK) <<
> @@ -849,7 +849,7 @@ isif_config_dfc(struct vpfe_isif_device *isif, struct 
> vpfe_isif_dfc *vdfc)
>
>   val = isif_read(isif->isif_cfg.base_addr, DFCMEMCTL);
>   /* set DFCMARST and set DFCMWR */
> - val |= 1 << ISIF_DFCMEMCTL_DFCMARST_SHIFT;
> + val |= BIT(ISIF_DFCMEMCTL_DFCMARST_SHIFT);
>   val |= 1;
>   isif_write(isif->isif_cfg.base_addr, val, DFCMEMCTL);
>
> @@ -880,7 +880,7 @@ isif_config_dfc(struct vpfe_isif_device *isif, struct 
> vpfe_isif_dfc *vdfc)
>   }
>   val = isif_read(isif->isif_cfg.base_addr, DFCMEMCTL);
>   /* clear DFCMARST and set DFCMWR */
> - val &= ~(1 << ISIF_DFCMEMCTL_DFCMARST_SHIFT);
> + val &= ~BIT(ISIF_DFCMEMCTL_DFCMARST_SHIFT);
>   val |= 1;
>   isif_write(isif->isif_cfg.base_addr, val, DFCMEMCTL);
>
> @@ -1140,7 +1140,7 @@ static int isif_config_raw(struct v4l2_subdev *sd, int 
> mode)
>   isif_write(isif->isif_cfg.base_addr, val, CGAMMAWD);
>   /* Configure DPCM compression settings */
>   if (params->v4l2_pix_fmt == V4L2_PIX_FMT_SGRBG10DPCM8) {
> - val =  1 << ISIF_DPCM_EN_SHIFT;
> + val =  BIT(ISIF_DPCM_EN_SHIFT);
>   val |= (params->dpcm_predictor &
>   ISIF_DPCM_PREDICTOR_MASK) << ISIF_DPCM_PREDICTOR_SHIFT;
>   }
> @@ -2044,7 +2044,7 @@ int vpfe_isif_init(struct vpfe_isif_device *isif, 
> struct platform_device *pdev)
>   v4l2_subdev_init(sd, _v4l2_ops);
>   sd->internal_ops = _v4l2_internal_ops;
>   strlcpy(sd->name, "DAVINCI ISIF", sizeof(sd->name));
> - sd->grp_i

Re: [Outreachy kernel] Re: [PATCH v1] staging: media: Remove unused function atomisp_set_stop_timeout()

2017-03-12 Thread Julia Lawall


On Sun, 12 Mar 2017, SIMRAN SINGHAL wrote:

> On Sun, Mar 12, 2017 at 7:24 PM, Greg KH <gre...@linuxfoundation.org> wrote:
> > On Fri, Mar 10, 2017 at 07:05:05PM +0530, simran singhal wrote:
> >> The function atomisp_set_stop_timeout on being called, simply returns
> >> back. The function hasn't been mentioned in the TODO and doesn't have
> >> FIXME code around. Hence, atomisp_set_stop_timeout and its calls have been
> >> removed.
> >>
> >> This was done using Coccinelle.
> >>
> >> @@
> >> identifier f;
> >> @@
> >>
> >> void f(...) {
> >>
> >> -return;
> >>
> >> }
> >>
> >> Signed-off-by: simran singhal <singhalsimr...@gmail.com>
> >> ---
> >>  v1:
> >>-Change Subject to include name of function
> >>-change commit message to include the coccinelle script
> >
> > You should also cc: the developers doing all of the current work on this
> > driver, Alan Cox, to get their comment if this really is something that
> > can be removed or not.
> >
> > thanks,
> >
> Greg I have cc'd all the developers which script get_maintainer.pl showed:
>
> $ git show HEAD | perl scripts/get_maintainer.pl --separator ,
> --nokeywords --nogit --nogit-fallback --norolestats

Sometimes people do a lot of work on something without being the
maintainer.  You can see who has done this using git log.  Dropping some
of the "no" arguments might give you the same information, but in practice
the results tend to be an overapproximation...

julia

>
> Mauro Carvalho Chehab <mche...@kernel.org>,Greg Kroah-Hartman
> <gre...@linuxfoundation.org>,
> linux-media@vger.kernel.org,de...@driverdev.osuosl.org,linux-ker...@vger.kernel.org
>
> > greg k-h
>
> --
> You received this message because you are subscribed to the Google Groups 
> "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an 
> email to outreachy-kernel+unsubscr...@googlegroups.com.
> To post to this group, send email to outreachy-ker...@googlegroups.com.
> To view this discussion on the web visit 
> https://groups.google.com/d/msgid/outreachy-kernel/CALrZqyOdKmSF10Ba60_00OzzRMnKAt7XwjksmuQfGEKvBY-avg%40mail.gmail.com.
> For more options, visit https://groups.google.com/d/optout.
>


Re: [PATCH] staging: atomisp: clean up return logic, remove redunant code

2017-03-12 Thread Julia Lawall


On Sun, 12 Mar 2017, walter harms wrote:

>
>
> Am 11.03.2017 20:32, schrieb Colin King:
> > From: Colin Ian King <colin.k...@canonical.com>
> >
> > There is no need to check if ret is non-zero, remove this
> > redundant check and just return the error status from the call
> > to mt9m114_write_reg_array.
> >
> > Detected by CoverityScan, CID#1416577 ("Identical code for
> > different branches")
> >
> > Signed-off-by: Colin Ian King <colin.k...@canonical.com>
> > ---
> >  drivers/staging/media/atomisp/i2c/mt9m114.c | 6 +-
> >  1 file changed, 1 insertion(+), 5 deletions(-)
> >
> > diff --git a/drivers/staging/media/atomisp/i2c/mt9m114.c 
> > b/drivers/staging/media/atomisp/i2c/mt9m114.c
> > index 8762124..a555aec 100644
> > --- a/drivers/staging/media/atomisp/i2c/mt9m114.c
> > +++ b/drivers/staging/media/atomisp/i2c/mt9m114.c
> > @@ -444,12 +444,8 @@ static int mt9m114_set_suspend(struct v4l2_subdev *sd)
> >  static int mt9m114_init_common(struct v4l2_subdev *sd)
> >  {
> > struct i2c_client *client = v4l2_get_subdevdata(sd);
> > -   int ret;
> >
> > -   ret = mt9m114_write_reg_array(client, mt9m114_common, PRE_POLLING);
> > -   if (ret)
> > -   return ret;
> > -   return ret;
> > +   return mt9m114_write_reg_array(client, mt9m114_common, PRE_POLLING);
> >  }
>
>
> any use for "client" ?

I guess the code would be on two lines in any case.  It looks like a nice
decomposition as is.

julia


Re: [Outreachy kernel] Re: [PATCH 1/7] staging: media: Remove unnecessary typecast of c90 int constant

2017-03-03 Thread Julia Lawall


On Fri, 3 Mar 2017, SIMRAN SINGHAL wrote:

> On Fri, Mar 3, 2017 at 11:15 PM, Sakari Ailus <sakari.ai...@iki.fi> wrote:
> > Hi Simran,
> >
> > On Fri, Mar 03, 2017 at 01:21:56AM +0530, simran singhal wrote:
> >> This patch removes unnecessary typecast of c90 int constant.
> >>
> >> WARNING: Unnecessary typecast of c90 int constant
> >>
> >> Signed-off-by: simran singhal <singhalsimr...@gmail.com>
> >
> > Which tree are these patches based on?
> Can you please explain what's the problem with this patch. And
> please elaborate your question.

It is probably staging-testing.

julia


>
> >
> > --
> > Regards,
> >
> > Sakari Ailus
> > e-mail: sakari.ai...@iki.fi XMPP: sai...@retiisi.org.uk
>
> --
> You received this message because you are subscribed to the Google Groups 
> "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an 
> email to outreachy-kernel+unsubscr...@googlegroups.com.
> To post to this group, send email to outreachy-ker...@googlegroups.com.
> To view this discussion on the web visit 
> https://groups.google.com/d/msgid/outreachy-kernel/CALrZqyOeOK2k1K8Z2Yt3SmvJQ8A%2BvigNBsd39-paPwkRAY6CVQ%40mail.gmail.com.
> For more options, visit https://groups.google.com/d/optout.
>


Re: [Outreachy kernel] [PATCH] staging: media: Remove parentheses from return arguments

2017-03-03 Thread Julia Lawall


On Fri, 3 Mar 2017, simran singhal wrote:

> The sematic patch used for this is:
> @@
> identifier i;
> constant c;
> @@
> return
> - (
> \(i\|-i\|i(...)\|c\)
> - )
>   ;
>
> Signed-off-by: simran singhal <singhalsimr...@gmail.com>

Acked-by: Julia Lawall <julia.law...@lip6.fr>


> ---
>  .../media/atomisp/pci/atomisp2/css2400/sh_css.c  | 20 
> ++--
>  .../atomisp/pci/atomisp2/css2400/sh_css_firmware.c   |  2 +-
>  2 files changed, 11 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css.c 
> b/drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css.c
> index f39d6f5..1216efb 100644
> --- a/drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css.c
> +++ b/drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css.c
> @@ -2009,7 +2009,7 @@ enum ia_css_err ia_css_suspend(void)
>   for(i=0;i<MAX_ACTIVE_STREAMS;i++)
>   ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE, "==*> after 1: seed %d 
> (%p)\n", i, my_css_save.stream_seeds[i].stream);
>   ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE, "ia_css_suspend() leave\n");
> - return(IA_CSS_SUCCESS);
> + return IA_CSS_SUCCESS;
>  }
>
>  enum ia_css_err
> @@ -2021,10 +2021,10 @@ ia_css_resume(void)
>
>   err = ia_css_init(&(my_css_save.driver_env), my_css_save.loaded_fw, 
> my_css_save.mmu_base, my_css_save.irq_type);
>   if (err != IA_CSS_SUCCESS)
> - return(err);
> + return err;
>   err = ia_css_start_sp();
>   if (err != IA_CSS_SUCCESS)
> - return(err);
> + return err;
>   my_css_save.mode = sh_css_mode_resume;
>   for(i=0;i<MAX_ACTIVE_STREAMS;i++)
>   {
> @@ -2038,7 +2038,7 @@ ia_css_resume(void)
>   if (i)
>   for(j=0;j<i;j++)
>   
> ia_css_stream_unload(my_css_save.stream_seeds[j].stream);
> - return(err);
> + return err;
>   }
>   err = 
> ia_css_stream_start(my_css_save.stream_seeds[i].stream);
>   if (err != IA_CSS_SUCCESS)
> @@ -2048,7 +2048,7 @@ ia_css_resume(void)
>   
> ia_css_stream_stop(my_css_save.stream_seeds[j].stream);
>   
> ia_css_stream_unload(my_css_save.stream_seeds[j].stream);
>   }
> - return(err);
> + return err;
>   }
>   *my_css_save.stream_seeds[i].orig_stream = 
> my_css_save.stream_seeds[i].stream;
>   for(j=0;j<my_css_save.stream_seeds[i].num_pipes;j++)
> @@ -2057,7 +2057,7 @@ ia_css_resume(void)
>   }
>   my_css_save.mode = sh_css_mode_working;
>   ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE, "ia_css_resume() leave: 
> return_void\n");
> - return(IA_CSS_SUCCESS);
> + return IA_CSS_SUCCESS;
>  }
>
>  enum ia_css_err
> @@ -10261,7 +10261,7 @@ ia_css_stream_load(struct ia_css_stream *stream)
>   for(k=0;k<j;k++)
>   
> ia_css_pipe_destroy(my_css_save.stream_seeds[i].pipes[k]);
>   }
> - return(err);
> + return err;
>   }
>   err = 
> ia_css_stream_create(&(my_css_save.stream_seeds[i].stream_config), 
> my_css_save.stream_seeds[i].num_pipes,
>   
> my_css_save.stream_seeds[i].pipes, &(my_css_save.stream_seeds[i].stream));
> @@ -10270,12 +10270,12 @@ ia_css_stream_load(struct ia_css_stream *stream)
>   ia_css_stream_destroy(stream);
>   
> for(j=0;j<my_css_save.stream_seeds[i].num_pipes;j++)
>   
> ia_css_pipe_destroy(my_css_save.stream_seeds[i].pipes[j]);
> - return(err);
> + return err;
>   }
>   break;
>   }
>   ia_css_debug_dtrace(IA_CSS_DEBUG_TRACE, "ia_css_stream_load() exit, 
> \n");
> - return(IA_CSS_SUCCESS);
> + return IA_CSS_SUCCESS;
>  #else
>   /* TODO remove function - DEPRECATED */
>   (void)stream;
> @@ -10416,7 +10416,7 @@ ia_css_stream_unload(struct ia_css_stream *stream)
>

Re: [patch] [media] uvcvideo: freeing an error pointer

2016-11-28 Thread Julia Lawall
To put it another way, tools can figure out what is missing if they have
access to good exmples of what should be there.

To be clear, I can see both points of view.

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


Re: [patch] [media] uvcvideo: freeing an error pointer

2016-11-28 Thread Julia Lawall


On Mon, 28 Nov 2016, Dan Carpenter wrote:

> I understand the comparison, but I just think it's better if people
> always keep track of what has been allocated and what has not.  I tried
> so hard to get Markus to stop sending those hundreds of patches where
> he's like "this function has a sanity check so we can pass pointers
> that weren't allocated"...  It's garbage code.
>
> But I understand that other people don't agree.

In my opinion, it is good for code understanding to only do what is useful
to do.  It's not a hard and fast rule, but I think it is something to take
into account.

julia

>
> regards,
> dan carpenter
>
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.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


question about hva_hw_probe

2016-11-10 Thread Julia Lawall
The function hva_hw_probe in the file
drivers/media/platform/sti/hva/hva-hw.c contains the following code:

/* get memory for esram */
esram = platform_get_resource(pdev, IORESOURCE_MEM, 1);
if (IS_ERR_OR_NULL(esram)) {
dev_err(dev, "%s failed to get esram\n", HVA_PREFIX);
return PTR_ERR(esram);
}
hva->esram_addr = esram->start;
hva->esram_size = resource_size(esram);

platform_get_resource only returns NULL on failure, so the test of
IS_ERR_OR_NULL doesn't look appropriate.  Nor does the return value of
PTR_ERR(esram), which will be 0 on a NULL result.  But I wondered if it
was intended to have a call to devm_ioremap_resource between the
platform_get_resource and the IS_ERR_OR_NULL test (which should be just
IS_ERR; likewise for the call just above)?

thanks,
julia
--
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] Staging:media:davinci_vpfe: used devm_kzalloc in place of kzalloc

2016-11-02 Thread Julia Lawall


On Wed, 2 Nov 2016, Nadim Almas wrote:

> Switch to resource-managed function devm_kzalloc instead
> of kzolloc and remove unneeded kzfree
>
> Also, remove kzfree in probe function and  remove
> function,vpfe_remove as it is now has nothing to do.
> The Coccinelle semantic patch used to make this change is as follows:
> /
> @platform@
> identifier p, probefn, removefn;
> @@
> struct platform_driver p = {
> .probe = probefn,
> .remove = removefn,
> };
>
> @prb@
> identifier platform.probefn, pdev;
> expression e, e1, e2;
> @@
> probefn(struct platform_device *pdev, ...) {
> <+...
> - e = kzalloc(e1, e2)
> + e = devm_kzalloc(>dev, e1, e2)
> ...
> ?-kzfree(e);
> ...+>
> }
> @rem depends on prb@
> identifier platform.removefn;
> expression prb.e;
> @@
> removefn(...) {
> <...
> - kzfree(e);
> ...>
> }
> //
>
> Signed-off-by: Nadim Almas <nadim@gmail.com>
> ---
>  drivers/staging/media/davinci_vpfe/vpfe_mc_capture.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/staging/media/davinci_vpfe/vpfe_mc_capture.c 
> b/drivers/staging/media/davinci_vpfe/vpfe_mc_capture.c
> index bf077f8..cd44f0f 100644
> --- a/drivers/staging/media/davinci_vpfe/vpfe_mc_capture.c
> +++ b/drivers/staging/media/davinci_vpfe/vpfe_mc_capture.c
> @@ -579,7 +579,7 @@ static int vpfe_probe(struct platform_device *pdev)
>   struct resource *res1;
>   int ret = -ENOMEM;
>
> - vpfe_dev = kzalloc(sizeof(*vpfe_dev), GFP_KERNEL);
> + vpfe_dev = devm_kzalloc(>dev, sizeof(*vpfe_dev), GFP_KERNEL);
>   if (!vpfe_dev)
>   return ret;
>
> @@ -681,7 +681,6 @@ static int vpfe_probe(struct platform_device *pdev)
>  probe_disable_clock:
>   vpfe_disable_clock(vpfe_dev);
>  probe_free_dev_mem:
> - kzfree(vpfe_dev);

Kzfree zeroes the data before freeing it.  Devm_kzalloc only causes a
kfree to happen, not a kzfree.  If the kzfree is needed, then a memset 0
would need to replace the calls to kzfree.

There are some other minor issues with the patch.  In the subject line,
there is normally a space after each :.  There is a spelling mistake in
the commit message.  In the commit message there should be one space
betwee words within a sentence, and there should be a space after a comma,
or other puctuation.

julia

>
>   return ret;
>  }
> @@ -702,7 +701,6 @@ static int vpfe_remove(struct platform_device *pdev)
>   v4l2_device_unregister(_dev->v4l2_dev);
>   media_device_unregister(_dev->media_dev);
>   vpfe_disable_clock(vpfe_dev);
> - kzfree(vpfe_dev);
>
>   return 0;
>  }
> --
> 2.7.4
>
>
--
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 1/3] [media] au0828-video: Use kcalloc() in au0828_init_isoc()

2016-10-24 Thread Julia Lawall


On Mon, 24 Oct 2016, Mauro Carvalho Chehab wrote:

> Em Mon, 24 Oct 2016 23:28:44 +0100
> Andrey Utkin <andrey_ut...@fastmail.com> escreveu:
>
> > On Mon, Oct 24, 2016 at 10:59:24PM +0200, SF Markus Elfring wrote:
> > > From: Markus Elfring <elfr...@users.sourceforge.net>
> > > Date: Mon, 24 Oct 2016 22:08:47 +0200
> > >
> > > * Multiplications for the size determination of memory allocations
> > >   indicated that array data structures should be processed.
> > >   Thus use the corresponding function "kcalloc".
> > >
> > >   This issue was detected by using the Coccinelle software.
> > >
> > > * Replace the specification of data types by pointer dereferences
> > >   to make the corresponding size determination a bit safer according to
> > >   the Linux coding style convention.
> > >
> > > Signed-off-by: Markus Elfring <elfr...@users.sourceforge.net>
> > > ---
> > >  drivers/media/usb/au0828/au0828-video.c | 11 +++
> > >  1 file changed, 7 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/drivers/media/usb/au0828/au0828-video.c 
> > > b/drivers/media/usb/au0828/au0828-video.c
> > > index 85dd9a8..85b13c1 100644
> > > --- a/drivers/media/usb/au0828/au0828-video.c
> > > +++ b/drivers/media/usb/au0828/au0828-video.c
> > > @@ -221,15 +221,18 @@ static int au0828_init_isoc(struct au0828_dev *dev, 
> > > int max_packets,
> > >
> > >   dev->isoc_ctl.isoc_copy = isoc_copy;
> > >   dev->isoc_ctl.num_bufs = num_bufs;
> > > -
> >
> > > - dev->isoc_ctl.urb = kzalloc(sizeof(void *)*num_bufs,  GFP_KERNEL);
> > > + dev->isoc_ctl.urb = kcalloc(num_bufs,
> > > + sizeof(*dev->isoc_ctl.urb),
> > > + GFP_KERNEL);
> >
> > What about this (for both hunks)?
> >
> > -   dev->isoc_ctl.urb = kzalloc(sizeof(void *)*num_bufs,  GFP_KERNEL);
> > +   dev->isoc_ctl.urb =
> > +   kcalloc(num_bufs, sizeof(*dev->isoc_ctl.urb), GFP_KERNEL);
>
>
> That's worse :)
>
> The usual Kernel style is:
>
>   var = foo(bar1,
> bar2,
> bar3);

Isn't it more like

var = foo(bar1, bar2,
  bar3)

Otherwise, Markus is going to send millions of patches to put every
function argument on its own line...

julia

>
> instead of something like:
>
>   var =
>   foo(bar1,
>   bar2,
>   bar3);
>
> The places where it is different than that is because people ran
> ./scripts/Lindent to try to follow the Kernel coding style.
>
> On my experiences, at the end, using it caused more harm than
> good, IMHO, and cause very weird indentation on lines with
> more than 80 columns like the above.
>
> Thanks,
> Mauro
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.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: [linux-review:Mauro-Carvalho-Chehab/Don-t-use-stack-for-DMA-transers-on-media-usb-drivers/20161011-182408 3/31] drivers/media/usb/dvb-usb/cinergyT2-core.c:174:2-8: preceding lock on line 169

2016-10-11 Thread Julia Lawall


On Tue, 11 Oct 2016, Mauro Carvalho Chehab wrote:

> Em Tue, 11 Oct 2016 15:16:24 +0200 (CEST)
> Julia Lawall <julia.law...@lip6.fr> escreveu:
>
> > On Tue, 11 Oct 2016, Julia Lawall wrote:
> >
> > > It looks like a lock may be needed before line 174.
> >
> > Sorry, an unlock.
>
> I suspect that this is a false positive warning, as there is a
> mutex unlock on the same routine, at line 203. All exit
> conditions go to the unlock condition.

There is a direct exit in line 174.

julia

>
> Am I missing something?
>
> >
> > >
> > > julia
> > >
> > > -- Forwarded message --
> > > Date: Tue, 11 Oct 2016 21:06:18 +0800
> > > From: kbuild test robot <fengguang...@intel.com>
> > > To: kbu...@01.org
> > > Cc: Julia Lawall <julia.law...@lip6.fr>
> > > Subject:
> > > 
> > > [linux-review:Mauro-Carvalho-Chehab/Don-t-use-stack-for-DMA-transers-on-medi
> > > a-usb-drivers/20161011-182408 3/31]
> > > drivers/media/usb/dvb-usb/cinergyT2-core.c:174:2-8: preceding lock on 
> > > line
> > > 169
> > >
> > > CC: kbuild-...@01.org
> > > TO: Mauro Carvalho Chehab <m.che...@samsung.com>
> > > CC: linux-media@vger.kernel.org
> > > CC: 0day robot <fengguang...@intel.com>
> > >
> > > tree:   https://github.com/0day-ci/linux 
> > > Mauro-Carvalho-Chehab/Don-t-use-stack-for-DMA-transers-on-media-usb-drivers/20161011-182408
> > > head:   ff49f775552fe4ebe2944527cf882073679cb1e5
> > > commit: b38d98275e144aaea9db69ba2dcba58466046d9b [3/31] cinergyT2-core: 
> > > handle error code on RC query
> > > :: branch date: 3 hours ago
> > > :: commit date: 3 hours ago
> > >
> > > >> drivers/media/usb/dvb-usb/cinergyT2-core.c:174:2-8: preceding lock on 
> > > >> line 169
> > >
> > > git remote add linux-review https://github.com/0day-ci/linux
> > > git remote update linux-review
> > > git checkout b38d98275e144aaea9db69ba2dcba58466046d9b
> > > vim +174 drivers/media/usb/dvb-usb/cinergyT2-core.c
> > >
> > > 986bd1e5 drivers/media/dvb/dvb-usb/cinergyT2-core.c Tomi Orava
> > > 2008-09-19  163  {
> > > 7f987678 drivers/media/dvb/dvb-usb/cinergyT2-core.c Thierry MERLE 
> > > 2008-09-19  164 struct cinergyt2_state *st = d->priv;
> > > b38d9827 drivers/media/usb/dvb-usb/cinergyT2-core.c Mauro Carvalho Chehab 
> > > 2016-10-11  165 int i, ret;
> > > 7f987678 drivers/media/dvb/dvb-usb/cinergyT2-core.c Thierry MERLE 
> > > 2008-09-19  166
> > > 986bd1e5 drivers/media/dvb/dvb-usb/cinergyT2-core.c Tomi Orava
> > > 2008-09-19  167 *state = REMOTE_NO_KEY_PRESSED;
> > > 986bd1e5 drivers/media/dvb/dvb-usb/cinergyT2-core.c Tomi Orava
> > > 2008-09-19  168
> > > 48922468 drivers/media/usb/dvb-usb/cinergyT2-core.c Mauro Carvalho Chehab 
> > > 2016-10-11 @169 mutex_lock(>data_mutex);
> > > 48922468 drivers/media/usb/dvb-usb/cinergyT2-core.c Mauro Carvalho Chehab 
> > > 2016-10-11  170 st->data[0] = CINERGYT2_EP1_GET_RC_EVENTS;
> > > 48922468 drivers/media/usb/dvb-usb/cinergyT2-core.c Mauro Carvalho Chehab 
> > > 2016-10-11  171
> > > b38d9827 drivers/media/usb/dvb-usb/cinergyT2-core.c Mauro Carvalho Chehab 
> > > 2016-10-11  172 ret = dvb_usb_generic_rw(d, st->data, 1, 
> > > st->data, 5, 0);
> > > b38d9827 drivers/media/usb/dvb-usb/cinergyT2-core.c Mauro Carvalho Chehab 
> > > 2016-10-11  173 if (ret < 0)
> > > b38d9827 drivers/media/usb/dvb-usb/cinergyT2-core.c Mauro Carvalho Chehab 
> > > 2016-10-11 @174 return ret;
> > > 48922468 drivers/media/usb/dvb-usb/cinergyT2-core.c Mauro Carvalho Chehab 
> > > 2016-10-11  175
> > > 48922468 drivers/media/usb/dvb-usb/cinergyT2-core.c Mauro Carvalho Chehab 
> > > 2016-10-11  176 if (st->data[4] == 0xff) {
> > > 7f987678 drivers/media/dvb/dvb-usb/cinergyT2-core.c Thierry MERLE 
> > > 2008-09-19  177 /* key repeat */
> > >
> > > ---
> > > 0-DAY kernel test infrastructureOpen Source Technology 
> > > Center
> > > https://lists.01.org/pipermail/kbuild-all   Intel 
> > > Corporation
> > >
> >
> >
>
>
> --
> Thanks,
> Mauro
>
--
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: [linux-review:Mauro-Carvalho-Chehab/Don-t-use-stack-for-DMA-transers-on-media-usb-drivers/20161011-182408 3/31] drivers/media/usb/dvb-usb/cinergyT2-core.c:174:2-8: preceding lock on line 169

2016-10-11 Thread Julia Lawall


On Tue, 11 Oct 2016, Julia Lawall wrote:

> It looks like a lock may be needed before line 174.

Sorry, an unlock.

>
> julia
>
> -- Forwarded message --
> Date: Tue, 11 Oct 2016 21:06:18 +0800
> From: kbuild test robot <fengguang...@intel.com>
> To: kbu...@01.org
> Cc: Julia Lawall <julia.law...@lip6.fr>
> Subject:
> 
> [linux-review:Mauro-Carvalho-Chehab/Don-t-use-stack-for-DMA-transers-on-medi
> a-usb-drivers/20161011-182408 3/31]
> drivers/media/usb/dvb-usb/cinergyT2-core.c:174:2-8: preceding lock on line
> 169
>
> CC: kbuild-...@01.org
> TO: Mauro Carvalho Chehab <m.che...@samsung.com>
> CC: linux-media@vger.kernel.org
> CC: 0day robot <fengguang...@intel.com>
>
> tree:   https://github.com/0day-ci/linux 
> Mauro-Carvalho-Chehab/Don-t-use-stack-for-DMA-transers-on-media-usb-drivers/20161011-182408
> head:   ff49f775552fe4ebe2944527cf882073679cb1e5
> commit: b38d98275e144aaea9db69ba2dcba58466046d9b [3/31] cinergyT2-core: 
> handle error code on RC query
> :: branch date: 3 hours ago
> :: commit date: 3 hours ago
>
> >> drivers/media/usb/dvb-usb/cinergyT2-core.c:174:2-8: preceding lock on line 
> >> 169
>
> git remote add linux-review https://github.com/0day-ci/linux
> git remote update linux-review
> git checkout b38d98275e144aaea9db69ba2dcba58466046d9b
> vim +174 drivers/media/usb/dvb-usb/cinergyT2-core.c
>
> 986bd1e5 drivers/media/dvb/dvb-usb/cinergyT2-core.c Tomi Orava
> 2008-09-19  163  {
> 7f987678 drivers/media/dvb/dvb-usb/cinergyT2-core.c Thierry MERLE 
> 2008-09-19  164 struct cinergyt2_state *st = d->priv;
> b38d9827 drivers/media/usb/dvb-usb/cinergyT2-core.c Mauro Carvalho Chehab 
> 2016-10-11  165 int i, ret;
> 7f987678 drivers/media/dvb/dvb-usb/cinergyT2-core.c Thierry MERLE 
> 2008-09-19  166
> 986bd1e5 drivers/media/dvb/dvb-usb/cinergyT2-core.c Tomi Orava
> 2008-09-19  167 *state = REMOTE_NO_KEY_PRESSED;
> 986bd1e5 drivers/media/dvb/dvb-usb/cinergyT2-core.c Tomi Orava
> 2008-09-19  168
> 48922468 drivers/media/usb/dvb-usb/cinergyT2-core.c Mauro Carvalho Chehab 
> 2016-10-11 @169 mutex_lock(>data_mutex);
> 48922468 drivers/media/usb/dvb-usb/cinergyT2-core.c Mauro Carvalho Chehab 
> 2016-10-11  170 st->data[0] = CINERGYT2_EP1_GET_RC_EVENTS;
> 48922468 drivers/media/usb/dvb-usb/cinergyT2-core.c Mauro Carvalho Chehab 
> 2016-10-11  171
> b38d9827 drivers/media/usb/dvb-usb/cinergyT2-core.c Mauro Carvalho Chehab 
> 2016-10-11  172 ret = dvb_usb_generic_rw(d, st->data, 1, st->data, 5, 0);
> b38d9827 drivers/media/usb/dvb-usb/cinergyT2-core.c Mauro Carvalho Chehab 
> 2016-10-11  173 if (ret < 0)
> b38d9827 drivers/media/usb/dvb-usb/cinergyT2-core.c Mauro Carvalho Chehab 
> 2016-10-11 @174 return ret;
> 48922468 drivers/media/usb/dvb-usb/cinergyT2-core.c Mauro Carvalho Chehab 
> 2016-10-11  175
> 48922468 drivers/media/usb/dvb-usb/cinergyT2-core.c Mauro Carvalho Chehab 
> 2016-10-11  176 if (st->data[4] == 0xff) {
> 7f987678 drivers/media/dvb/dvb-usb/cinergyT2-core.c Thierry MERLE 
> 2008-09-19  177 /* key repeat */
>
> ---
> 0-DAY kernel test infrastructureOpen Source Technology Center
> https://lists.01.org/pipermail/kbuild-all   Intel Corporation
>
--
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


[linux-review:Mauro-Carvalho-Chehab/Don-t-use-stack-for-DMA-transers-on-media-usb-drivers/20161011-182408 3/31] drivers/media/usb/dvb-usb/cinergyT2-core.c:174:2-8: preceding lock on line 169

2016-10-11 Thread Julia Lawall
It looks like a lock may be needed before line 174.

julia

-- Forwarded message --
Date: Tue, 11 Oct 2016 21:06:18 +0800
From: kbuild test robot <fengguang...@intel.com>
To: kbu...@01.org
Cc: Julia Lawall <julia.law...@lip6.fr>
Subject:
[linux-review:Mauro-Carvalho-Chehab/Don-t-use-stack-for-DMA-transers-on-medi
a-usb-drivers/20161011-182408 3/31]
drivers/media/usb/dvb-usb/cinergyT2-core.c:174:2-8: preceding lock on line
169

CC: kbuild-...@01.org
TO: Mauro Carvalho Chehab <m.che...@samsung.com>
CC: linux-media@vger.kernel.org
CC: 0day robot <fengguang...@intel.com>

tree:   https://github.com/0day-ci/linux 
Mauro-Carvalho-Chehab/Don-t-use-stack-for-DMA-transers-on-media-usb-drivers/20161011-182408
head:   ff49f775552fe4ebe2944527cf882073679cb1e5
commit: b38d98275e144aaea9db69ba2dcba58466046d9b [3/31] cinergyT2-core: handle 
error code on RC query
:: branch date: 3 hours ago
:: commit date: 3 hours ago

>> drivers/media/usb/dvb-usb/cinergyT2-core.c:174:2-8: preceding lock on line 
>> 169

git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout b38d98275e144aaea9db69ba2dcba58466046d9b
vim +174 drivers/media/usb/dvb-usb/cinergyT2-core.c

986bd1e5 drivers/media/dvb/dvb-usb/cinergyT2-core.c Tomi Orava
2008-09-19  163  {
7f987678 drivers/media/dvb/dvb-usb/cinergyT2-core.c Thierry MERLE 
2008-09-19  164   struct cinergyt2_state *st = d->priv;
b38d9827 drivers/media/usb/dvb-usb/cinergyT2-core.c Mauro Carvalho Chehab 
2016-10-11  165   int i, ret;
7f987678 drivers/media/dvb/dvb-usb/cinergyT2-core.c Thierry MERLE 
2008-09-19  166
986bd1e5 drivers/media/dvb/dvb-usb/cinergyT2-core.c Tomi Orava
2008-09-19  167   *state = REMOTE_NO_KEY_PRESSED;
986bd1e5 drivers/media/dvb/dvb-usb/cinergyT2-core.c Tomi Orava
2008-09-19  168
48922468 drivers/media/usb/dvb-usb/cinergyT2-core.c Mauro Carvalho Chehab 
2016-10-11 @169   mutex_lock(>data_mutex);
48922468 drivers/media/usb/dvb-usb/cinergyT2-core.c Mauro Carvalho Chehab 
2016-10-11  170   st->data[0] = CINERGYT2_EP1_GET_RC_EVENTS;
48922468 drivers/media/usb/dvb-usb/cinergyT2-core.c Mauro Carvalho Chehab 
2016-10-11  171
b38d9827 drivers/media/usb/dvb-usb/cinergyT2-core.c Mauro Carvalho Chehab 
2016-10-11  172   ret = dvb_usb_generic_rw(d, st->data, 1, st->data, 5, 0);
b38d9827 drivers/media/usb/dvb-usb/cinergyT2-core.c Mauro Carvalho Chehab 
2016-10-11  173   if (ret < 0)
b38d9827 drivers/media/usb/dvb-usb/cinergyT2-core.c Mauro Carvalho Chehab 
2016-10-11 @174   return ret;
48922468 drivers/media/usb/dvb-usb/cinergyT2-core.c Mauro Carvalho Chehab 
2016-10-11  175
48922468 drivers/media/usb/dvb-usb/cinergyT2-core.c Mauro Carvalho Chehab 
2016-10-11  176   if (st->data[4] == 0xff) {
7f987678 drivers/media/dvb/dvb-usb/cinergyT2-core.c Thierry MERLE 
2008-09-19  177   /* key repeat */

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation
--
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: kernel-doc-rst-lint (was: Re: [PATCH 00/15] improve function-level documentation)

2016-10-05 Thread Julia Lawall


On Wed, 5 Oct 2016, Jani Nikula wrote:

> On Wed, 05 Oct 2016, Daniel Vetter <dan...@ffwll.ch> wrote:
> > Jani Nikula has a patch with a scrip to make the one kernel-doc parser
> > into a lint/checker pass over the entire kernel. I think that'd would
> > be more robust instead of trying to approximate the real kerneldoc
> > parser. Otoh that parser is a horror show of a perl/regex driven state
> > machine ;-)
> >
> > Jani, can you pls digg out these patches? Can't find them right now ...
>
> Expanding the massive Cc: with linux-doc list...
>
> Here goes. It's a quick hack from months ago, but still seems to
> somewhat work. At least for the kernel-doc parts. The reStructuredText
> lint part isn't all that great, and doesn't have mapping to line numbers
> like the Sphinx kernel-doc extension does. Anyway I'm happy how this
> integrates with kernel build CHECK and C=1/C=2.
>
> I guess Julia's goal is to automate the *fixing* of some of the error
> classes from kernel-doc. Not sure how well this could be made to
> integrate with any of that.

No, my work doesn't fix anything.  Coccinelle can't actually process
comments.  I just correlated the parsed comment with the function header.

julia

>
> BR,
> Jani.
>
>
> From 1244efa0f63a7b13795e8c37f81733a3c8bfc56a Mon Sep 17 00:00:00 2001
> From: Jani Nikula <jani.nik...@intel.com>
> Date: Tue, 31 May 2016 18:11:33 +0300
> Subject: [PATCH] kernel-doc-rst-lint: add tool to check kernel-doc and rst
>  correctness
> Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo
> Cc: Jani Nikula <jani.nik...@intel.com>
>
> Simple kernel-doc and reStructuredText lint tool that can be used
> independently and as a kernel build CHECK tool to validate kernel-doc
> comments.
>
> Independent usage:
> $ kernel-doc-rst-lint FILE
>
> Kernel CHECK usage:
> $ make CHECK=scripts/kernel-doc-rst-lint C=1  # (or C=2)
>
> Depends on docutils and the rst-lint package
> https://pypi.python.org/pypi/restructuredtext_lint
>
> Signed-off-by: Jani Nikula <jani.nik...@intel.com>
> ---
>  scripts/kernel-doc-rst-lint | 106 
> 
>  1 file changed, 106 insertions(+)
>  create mode 100755 scripts/kernel-doc-rst-lint
>
> diff --git a/scripts/kernel-doc-rst-lint b/scripts/kernel-doc-rst-lint
> new file mode 100755
> index ..7e0157679f83
> --- /dev/null
> +++ b/scripts/kernel-doc-rst-lint
> @@ -0,0 +1,106 @@
> +#!/usr/bin/env python
> +# coding=utf-8
> +#
> +# Copyright © 2016 Intel Corporation
> +#
> +# Permission is hereby granted, free of charge, to any person obtaining a
> +# copy of this software and associated documentation files (the "Software"),
> +# to deal in the Software without restriction, including without limitation
> +# the rights to use, copy, modify, merge, publish, distribute, sublicense,
> +# and/or sell copies of the Software, and to permit persons to whom the
> +# Software is furnished to do so, subject to the following conditions:
> +#
> +# The above copyright notice and this permission notice (including the next
> +# paragraph) shall be included in all copies or substantial portions of the
> +# Software.
> +#
> +# 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.
> +#
> +# Authors:
> +#Jani Nikula <jani.nik...@intel.com>
> +#
> +# Simple kernel-doc and reStructuredText lint tool that can be used
> +# independently and as a kernel build CHECK tool to validate kernel-doc
> +# comments.
> +#
> +# Independent usage:
> +# $ kernel-doc-rst-lint FILE
> +#
> +# Kernel CHECK usage:
> +# $ make CHECK=scripts/kernel-doc-rst-lint C=1   # (or C=2)
> +#
> +# Depends on docutils and the rst-lint package
> +# https://pypi.python.org/pypi/restructuredtext_lint
> +#
> +
> +import os
> +import subprocess
> +import sys
> +
> +from docutils.parsers.rst import directives
> +from docutils.parsers.rst import Directive
> +from docutils.parsers.rst import roles
> +from docutils import nodes, statemachine
> +import restructuredtext_lint
> +
> +class DummyDirective(Directive):
> +required_argument = 1
> +optional_arguments = 0
> +option

Re: [PATCH 00/15] improve function-level documentation

2016-10-05 Thread Julia Lawall


On Wed, 5 Oct 2016, Daniel Vetter wrote:

> Jani Nikula has a patch with a scrip to make the one kernel-doc parser
> into a lint/checker pass over the entire kernel. I think that'd would
> be more robust instead of trying to approximate the real kerneldoc
> parser. Otoh that parser is a horror show of a perl/regex driven state
> machine ;-)

Sure.  To my recollection, I found around 2000 issues.  Many I ignored, eg
functions that simply have no documentation abuot the parameters,
functions that document their local variables, when these were more
interesting than the parameters etc.  But the set of patches is not
exhaustive with respect to the remaining interesting ones either.

julia

>
> Jani, can you pls digg out these patches? Can't find them right now ...
> -Daniel
>
>
> On Sat, Oct 1, 2016 at 9:46 PM, Julia Lawall <julia.law...@lip6.fr> wrote:
> > These patches fix cases where the documentation above a function definition
> > is not consistent with the function header.  Issues are detected using the
> > semantic patch below (http://coccinelle.lip6.fr/).  Basically, the semantic
> > patch parses a file to find comments, then matches each function header,
> > and checks that the name and parameter list in the function header are
> > compatible with the comment that preceeds it most closely.
> >
> > // 
> > @initialize:ocaml@
> > @@
> >
> > let tbl = ref []
> > let fnstart = ref []
> > let success = Hashtbl.create 101
> > let thefile = ref ""
> > let parsed = ref []
> > let nea = ref []
> >
> > let parse file =
> >   thefile := List.nth (Str.split (Str.regexp "linux-next/") file) 1;
> >   let i = open_in file in
> >   let startline = ref 0 in
> >   let fn = ref "" in
> >   let ids = ref [] in
> >   let rec inside n =
> > let l = input_line i in
> > let n = n + 1 in
> > match Str.split_delim (Str.regexp_string "*/") l with
> >   before::after::_ ->
> > (if not (!fn = "")
> > then tbl := (!startline,n,!fn,List.rev !ids)::!tbl);
> > startline := 0;
> > fn := "";
> > ids := [];
> > outside n
> > | _ ->
> > (match Str.split (Str.regexp "[ \t]+") l with
> >   "*"::name::rest ->
> > let len = String.length name in
> > (if !fn = "" && len > 2 && String.sub name (len-2) 2 = "()"
> > then fn := String.sub name 0 (len-2)
> > else if !fn = "" && (not (rest = [])) && List.hd rest = "-"
> > then
> >   if String.get name (len-1) = ':'
> >   then fn := String.sub name 0 (len-1)
> >   else fn := name
> > else if not(!fn = "") && len > 2 &&
> >   String.get name 0 = '@' && String.get name (len-1) = ':'
> > then ids := (String.sub name 1 (len-2)) :: !ids);
> > | _ -> ());
> > inside n
> >   and outside n =
> > let l = input_line i in
> > let n = n + 1 in
> > if String.length l > 2 && String.sub l 0 3 = "/**"
> > then
> >   begin
> > startline := n;
> > inside n
> >   end
> > else outside n in
> >   try outside 0 with End_of_file -> ()
> >
> > let hashadd tbl k v =
> >   let cell =
> > try Hashtbl.find tbl k
> > with Not_found ->
> >   let cell = ref [] in
> >   Hashtbl.add tbl k cell;
> >   cell in
> >   cell := v :: !cell
> >
> > @script:ocaml@
> > @@
> >
> > tbl := [];
> > fnstart := [];
> > Hashtbl.clear success;
> > parsed := [];
> > nea := [];
> > parse (List.hd (Coccilib.files()))
> >
> > @r@
> > identifier f;
> > position p;
> > @@
> >
> > f@p(...) { ... }
> >
> > @script:ocaml@
> > p << r.p;
> > f << r.f;
> > @@
> >
> > parsed := f :: !parsed;
> > fnstart := (List.hd p).line :: !fnstart
> >
> > @param@
> > identifier f;
> > type T;
> > identifier i;
> > parameter list[n] ps;
> > parameter list[n1] ps1;
> > position p;
> > @@
> >
> > f@p(ps,T i,ps1) { ... }
> >
> > @script:ocaml@
> > @@
> >
> > tbl := List.rev (List.sort compare !tbl)
> >
> > @script:ocaml@
> &

Re: [PATCH 00/15] improve function-level documentation

2016-10-01 Thread Julia Lawall


On Sat, 1 Oct 2016, Joe Perches wrote:

> On Sat, 2016-10-01 at 21:46 +0200, Julia Lawall wrote:
> > These patches fix cases where the documentation above a function definition
> > is not consistent with the function header.  Issues are detected using the
> > semantic patch below (http://coccinelle.lip6.fr/).  Basically, the semantic
> > patch parses a file to find comments, then matches each function header,
> > and checks that the name and parameter list in the function header are
> > compatible with the comment that preceeds it most closely.
>
> Hi Julia.
>
> Would it be possible for a semantic patch to scan for
> function definitions where the types do not have
> identifiers and update the definitions to match the
> declarations?
>
> For instance, given:
>
> 
> int foo(int);
>
> 
> int foo(int bar)
> {
>   return baz;
> }
>
> Could coccinelle output:
>
> diff a/some.h b/some.h
> []
> -int foo(int);
> +int foo(int bar);

The following seems to work:

@r@
identifier f;
position p;
type T, t;
parameter list[n] ps;
@@

T f@p(ps,t,...);

@s@
identifier r.f,x;
type r.T, r.t;
parameter list[r.n] ps;
@@

T f(ps,t x,...) { ... }

@@
identifier r.f, s.x;
position r.p;
type r.T, r.t;
parameter list[r.n] ps;
@@

T f@p(ps,t
+ x
  ,...);

After letting it run for a few minutes without making any effort to
include .h files, I get over 2700 changed lines.

julia
--
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 00/15] improve function-level documentation

2016-10-01 Thread Julia Lawall
These patches fix cases where the documentation above a function definition
is not consistent with the function header.  Issues are detected using the
semantic patch below (http://coccinelle.lip6.fr/).  Basically, the semantic
patch parses a file to find comments, then matches each function header,
and checks that the name and parameter list in the function header are
compatible with the comment that preceeds it most closely.

// 
@initialize:ocaml@
@@

let tbl = ref []
let fnstart = ref []
let success = Hashtbl.create 101
let thefile = ref ""
let parsed = ref []
let nea = ref []

let parse file =
  thefile := List.nth (Str.split (Str.regexp "linux-next/") file) 1;
  let i = open_in file in
  let startline = ref 0 in
  let fn = ref "" in
  let ids = ref [] in
  let rec inside n =
let l = input_line i in
let n = n + 1 in
match Str.split_delim (Str.regexp_string "*/") l with
  before::after::_ ->
(if not (!fn = "")
then tbl := (!startline,n,!fn,List.rev !ids)::!tbl);
startline := 0;
fn := "";
ids := [];
outside n
| _ ->
(match Str.split (Str.regexp "[ \t]+") l with
  "*"::name::rest ->
let len = String.length name in
(if !fn = "" && len > 2 && String.sub name (len-2) 2 = "()"
then fn := String.sub name 0 (len-2)
else if !fn = "" && (not (rest = [])) && List.hd rest = "-"
then
  if String.get name (len-1) = ':'
  then fn := String.sub name 0 (len-1)
  else fn := name
else if not(!fn = "") && len > 2 &&
  String.get name 0 = '@' && String.get name (len-1) = ':'
then ids := (String.sub name 1 (len-2)) :: !ids);
| _ -> ());
inside n
  and outside n =
let l = input_line i in
let n = n + 1 in
if String.length l > 2 && String.sub l 0 3 = "/**"
then
  begin
startline := n;
inside n
  end
else outside n in
  try outside 0 with End_of_file -> ()

let hashadd tbl k v =
  let cell =
try Hashtbl.find tbl k
with Not_found ->
  let cell = ref [] in
  Hashtbl.add tbl k cell;
  cell in
  cell := v :: !cell

@script:ocaml@
@@

tbl := [];
fnstart := [];
Hashtbl.clear success;
parsed := [];
nea := [];
parse (List.hd (Coccilib.files()))

@r@
identifier f;
position p;
@@

f@p(...) { ... }

@script:ocaml@
p << r.p;
f << r.f;
@@

parsed := f :: !parsed;
fnstart := (List.hd p).line :: !fnstart

@param@
identifier f;
type T;
identifier i;
parameter list[n] ps;
parameter list[n1] ps1;
position p;
@@

f@p(ps,T i,ps1) { ... }

@script:ocaml@
@@

tbl := List.rev (List.sort compare !tbl)

@script:ocaml@
p << param.p;
f << param.f;
@@

let myline = (List.hd p).line in
let prevline =
  List.fold_left
(fun prev x ->
  if x < myline
  then max x prev
  else prev)
0 !fnstart in
let _ =
  List.exists
(function (st,fn,nm,ids) ->
  if prevline < st && myline > st && prevline < fn && myline > fn
  then
begin
  (if not (String.lowercase f = String.lowercase nm)
  then
Printf.printf "%s:%d %s doesn't match preceding comment: %s\n"
  !thefile myline f nm);
  true
end
  else false)
!tbl in
()

@script:ocaml@
p << param.p;
n << param.n;
n1 << param.n1;
i << param.i;
f << param.f;
@@

let myline = (List.hd p).line in
let prevline =
  List.fold_left
(fun prev x ->
  if x < myline
  then max x prev
  else prev)
0 !fnstart in
let _ =
  List.exists
(function (st,fn,nm,ids) ->
  if prevline < st && myline > st && prevline < fn && myline > fn
  then
begin
  (if List.mem i ids then hashadd success (st,fn,nm) i);
  (if ids = [] (* arg list seems not obligatory *)
  then ()
  else if not (List.mem i ids)
  then
Printf.printf "%s:%d %s doesn't appear in ids: %s\n"
  !thefile myline i (String.concat " " ids)
  else if List.length ids <= n || List.length ids <= n1
  then
(if not (List.mem f !nea)
then
  begin
nea := f :: !nea;
Printf.printf "%s:%d %s not enough args\n" !thefile myline f;
  end)
  else
let foundid = List.nth ids n in
let efoundid = List.nth (List.rev ids) n1 in
if not(foundid = i || efoundid = i)
then
  Printf.printf "%s:%d %s wrong arg in position %d: %s\n"
!thefile myline i n foundid);
  true
end
  else false)
!tbl in
()

@script:ocaml@
@@
List.iter
  (function (st,fn,nm,ids) ->
if List.mem nm !parsed
then
  let entry =
try !(Hashtbl.find success (st,fn,nm))
with Not_found -> [] in
  List.iter
(fun id ->
  if not (List.mem id entry) && not (id = "...")
  then Printf.printf "%s:%d %s not used\n" 

[PATCH 05/15] dma-buf/sw_sync: improve function-level documentation

2016-10-01 Thread Julia Lawall
Adjust the documentation to use the names that appear in the function
parameter list.

Issue detected using Coccinelle (http://coccinelle.lip6.fr/)

Signed-off-by: Julia Lawall <julia.law...@lip6.fr>

---
 drivers/dma-buf/sw_sync.c |6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/dma-buf/sw_sync.c b/drivers/dma-buf/sw_sync.c
index 62e8e6d..5d2b1b6 100644
--- a/drivers/dma-buf/sw_sync.c
+++ b/drivers/dma-buf/sw_sync.c
@@ -155,11 +155,11 @@ static void sync_timeline_signal(struct sync_timeline 
*obj, unsigned int inc)
 
 /**
  * sync_pt_create() - creates a sync pt
- * @parent:fence's parent sync_timeline
+ * @obj:   fence's parent sync_timeline
  * @size:  size to allocate for this pt
- * @inc:   value of the fence
+ * @value: value of the fence
  *
- * Creates a new sync_pt as a child of @parent.  @size bytes will be
+ * Creates a new sync_pt as a child of @obj.  @size bytes will be
  * allocated allowing for implementation specific data to be kept after
  * the generic sync_timeline struct. Returns the sync_pt object or
  * NULL in case of error.

--
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 00/26] constify local structures

2016-09-12 Thread Julia Lawall


On Mon, 12 Sep 2016, Jarkko Sakkinen wrote:

> On Mon, Sep 12, 2016 at 04:43:58PM +0300, Felipe Balbi wrote:
> >
> > Hi,
> >
> > Jarkko Sakkinen <jarkko.sakki...@linux.intel.com> writes:
> > > On Mon, Sep 12, 2016 at 10:54:07AM +0200, Julia Lawall wrote:
> > >>
> > >>
> > >> On Sun, 11 Sep 2016, Jarkko Sakkinen wrote:
> > >>
> > >> > On Sun, Sep 11, 2016 at 03:05:42PM +0200, Julia Lawall wrote:
> > >> > > Constify local structures.
> > >> > >
> > >> > > The semantic patch that makes this change is as follows:
> > >> > > (http://coccinelle.lip6.fr/)
> > >> >
> > >> > Just my two cents but:
> > >> >
> > >> > 1. You *can* use a static analysis too to find bugs or other issues.
> > >> > 2. However, you should manually do the commits and proper commit
> > >> >messages to subsystems based on your findings. And I generally think
> > >> >that if one contributes code one should also at least smoke test 
> > >> > changes
> > >> >somehow.
> > >> >
> > >> > I don't know if I'm alone with my opinion. I just think that one should
> > >> > also do the analysis part and not blindly create and submit patches.
> > >>
> > >> All of the patches are compile tested.  And the individual patches are
> > >
> > > Compile-testing is not testing. If you are not able to test a commit,
> > > you should explain why.
> >
> > Dude, Julia has been doing semantic patching for years already and
> > nobody has raised any concerns so far. There's already an expectation
> > that Coccinelle *works* and Julia's sematic patches are sound.
> >
> > Besides, adding 'const' is something that causes virtually no functional
> > changes to the point that build-testing is really all you need. Any
> > problems caused by adding 'const' to a definition will be seen by build
> > errors or warnings.
> >
> > Really, just stop with the pointless discussion and go read a bit about
> > Coccinelle and what semantic patches are giving you. The work done by
> > Julia and her peers are INRIA have measurable benefits.
> >
> > You're really making a thunderstorm in a glass of water.
>
> Hmm... I've been using coccinelle in cyclic basis for some time now.
> My comment was oversized but I didn't mean it to be impolite or attack
> of any kind for that matter.

No problem :)  Thanks for the feedback.

julia
--
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 00/26] constify local structures

2016-09-12 Thread Julia Lawall


On Mon, 12 Sep 2016, Felipe Balbi wrote:

>
> Hi,
>
> Jarkko Sakkinen <jarkko.sakki...@linux.intel.com> writes:
> > On Mon, Sep 12, 2016 at 10:54:07AM +0200, Julia Lawall wrote:
> >>
> >>
> >> On Sun, 11 Sep 2016, Jarkko Sakkinen wrote:
> >>
> >> > On Sun, Sep 11, 2016 at 03:05:42PM +0200, Julia Lawall wrote:
> >> > > Constify local structures.
> >> > >
> >> > > The semantic patch that makes this change is as follows:
> >> > > (http://coccinelle.lip6.fr/)
> >> >
> >> > Just my two cents but:
> >> >
> >> > 1. You *can* use a static analysis too to find bugs or other issues.
> >> > 2. However, you should manually do the commits and proper commit
> >> >messages to subsystems based on your findings. And I generally think
> >> >that if one contributes code one should also at least smoke test 
> >> > changes
> >> >somehow.
> >> >
> >> > I don't know if I'm alone with my opinion. I just think that one should
> >> > also do the analysis part and not blindly create and submit patches.
> >>
> >> All of the patches are compile tested.  And the individual patches are
> >
> > Compile-testing is not testing. If you are not able to test a commit,
> > you should explain why.
>
> Dude, Julia has been doing semantic patching for years already and
> nobody has raised any concerns so far. There's already an expectation
> that Coccinelle *works* and Julia's sematic patches are sound.
>
> Besides, adding 'const' is something that causes virtually no functional
> changes to the point that build-testing is really all you need. Any
> problems caused by adding 'const' to a definition will be seen by build
> errors or warnings.
>
> Really, just stop with the pointless discussion and go read a bit about
> Coccinelle and what semantic patches are giving you. The work done by
> Julia and her peers are INRIA have measurable benefits.
>
> You're really making a thunderstorm in a glass of water.

Thanks for the defense, but since a lot of these patches torned out to be
wrong, due to an incorrect parse by Coccinelle, combined with an
unpleasantly lax compiler, Jarkko does have a point that I should have
looked at the patches more carefully.  In any case, I have written to the
maintainers relevant to the patches that turned out to be incorrect.

julia
--
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 00/26] constify local structures

2016-09-12 Thread Julia Lawall


On Mon, 12 Sep 2016, Jarkko Sakkinen wrote:

> On Mon, Sep 12, 2016 at 10:54:07AM +0200, Julia Lawall wrote:
> >
> >
> > On Sun, 11 Sep 2016, Jarkko Sakkinen wrote:
> >
> > > On Sun, Sep 11, 2016 at 03:05:42PM +0200, Julia Lawall wrote:
> > > > Constify local structures.
> > > >
> > > > The semantic patch that makes this change is as follows:
> > > > (http://coccinelle.lip6.fr/)
> > >
> > > Just my two cents but:
> > >
> > > 1. You *can* use a static analysis too to find bugs or other issues.
> > > 2. However, you should manually do the commits and proper commit
> > >messages to subsystems based on your findings. And I generally think
> > >that if one contributes code one should also at least smoke test 
> > > changes
> > >somehow.
> > >
> > > I don't know if I'm alone with my opinion. I just think that one should
> > > also do the analysis part and not blindly create and submit patches.
> >
> > All of the patches are compile tested.  And the individual patches are
>
> Compile-testing is not testing. If you are not able to test a commit,
> you should explain why.
>
> > submitted to the relevant maintainers.  The individual commit messages
> > give a more detailed explanation of the strategy used to decide that the
> > structure was constifiable.  It seemed redundant to put that in the cover
> > letter, which will not be committed anyway.
>
> I don't mean to be harsh but I do not care about your thought process
> *that much* when I review a commit (sometimes it might make sense to
> explain that but it depends on the context).
>
> I mostly only care why a particular change makes sense for this
> particular subsystem. The report given by a static analysis tool can
> be a starting point for making a commit but it's not sufficient.
> Based on the report you should look subsystems as individuals.

OK, thanks for the feedback.

julia
--
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 00/26] constify local structures

2016-09-12 Thread Julia Lawall


On Sun, 11 Sep 2016, Jarkko Sakkinen wrote:

> On Sun, Sep 11, 2016 at 03:05:42PM +0200, Julia Lawall wrote:
> > Constify local structures.
> >
> > The semantic patch that makes this change is as follows:
> > (http://coccinelle.lip6.fr/)
>
> Just my two cents but:
>
> 1. You *can* use a static analysis too to find bugs or other issues.
> 2. However, you should manually do the commits and proper commit
>messages to subsystems based on your findings. And I generally think
>that if one contributes code one should also at least smoke test changes
>somehow.
>
> I don't know if I'm alone with my opinion. I just think that one should
> also do the analysis part and not blindly create and submit patches.

All of the patches are compile tested.  And the individual patches are
submitted to the relevant maintainers.  The individual commit messages
give a more detailed explanation of the strategy used to decide that the
structure was constifiable.  It seemed redundant to put that in the cover
letter, which will not be committed anyway.

julia

>
> Anyway, I'll apply the TPM change at some point. As I said they were
> for better. Thanks.
>
> /Jarkko
>
> > // 
> > // The first rule ignores some cases that posed problems
> > @r disable optional_qualifier@
> > identifier s != {peri_clk_data,threshold_attr,tracer_flags,tracer};
> > identifier i != {s5k5baf_cis_rect,smtcfb_fix};
> > position p;
> > @@
> > static struct s i@p = { ... };
> >
> > @lstruct@
> > identifier r.s;
> > @@
> > struct s { ... };
> >
> > @used depends on lstruct@
> > identifier r.i;
> > @@
> > i
> >
> > @bad1@
> > expression e;
> > identifier r.i;
> > assignment operator a;
> > @@
> >  (<+...i...+>) a e
> >
> > @bad2@
> > identifier r.i;
> > @@
> >  &(<+...i...+>)
> >
> > @bad3@
> > identifier r.i;
> > declarer d;
> > @@
> >  d(...,<+...i...+>,...);
> >
> > @bad4@
> > identifier r.i;
> > type T;
> > T[] e;
> > identifier f;
> > position p;
> > @@
> >
> > f@p(...,
> > (
> >   (<+...i...+>)
> > &
> >   e
> > )
> > ,...)
> >
> > @bad4a@
> > identifier r.i;
> > type T;
> > T *e;
> > identifier f;
> > position p;
> > @@
> >
> > f@p(...,
> > (
> >   (<+...i...+>)
> > &
> >   e
> > )
> > ,...)
> >
> > @ok5@
> > expression *e;
> > identifier r.i;
> > position p;
> > @@
> > e =@p i
> >
> > @bad5@
> > expression *e;
> > identifier r.i;
> > position p != ok5.p;
> > @@
> > e =@p (<+...i...+>)
> >
> > @rr depends on used && !bad1 && !bad2 && !bad3 && !bad4 && !bad4a && !bad5@
> > identifier s,r.i;
> > position r.p;
> > @@
> >
> > static
> > +const
> >  struct s i@p = { ... };
> >
> > @depends on used && !bad1 && !bad2 && !bad3 && !bad4 && !bad4a && !bad5
> >  disable optional_qualifier@
> > identifier rr.s,r.i;
> > @@
> >
> > static
> > +const
> >  struct s i;
> > // 
> >
> > ---
> >
> >  drivers/acpi/acpi_apd.c  |8 +++---
> >  drivers/char/tpm/tpm-interface.c |   10 
> >  drivers/char/tpm/tpm-sysfs.c |2 -
> >  drivers/cpufreq/intel_pstate.c   |8 +++---
> >  drivers/infiniband/hw/i40iw/i40iw_uk.c   |6 ++---
> >  drivers/media/i2c/tvp514x.c  |2 -
> >  drivers/media/pci/ddbridge/ddbridge-core.c   |   18 +++
> >  drivers/media/pci/ngene/ngene-cards.c|   14 ++--
> >  drivers/media/pci/smipcie/smipcie-main.c |8 +++---
> >  drivers/misc/sgi-xp/xpc_uv.c |2 -
> >  drivers/net/arcnet/com20020-pci.c|   10 
> >  drivers/net/can/c_can/c_can_pci.c|4 +--
> >  drivers/net/can/sja1000/plx_pci.c|   20 
> > -
> >  drivers/net/ethernet/mellanox/mlx4/main.c|4 +--
> >  drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c |2 -
> >  drivers/net/ethernet/renesas/sh_eth.c|   14 ++--
> >  drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c |2 -
> > 

Re: [PATCH 00/26] constify local structures

2016-09-11 Thread Julia Lawall

On Sun, 11 Sep 2016, Joe Perches wrote:

> On Sun, 2016-09-11 at 15:05 +0200, Julia Lawall wrote:
> > Constify local structures.
>
> Thanks Julia.
>
> A few suggestions & questions:
>
> Perhaps the script should go into scripts/coccinelle/
> so that future cases could be caught by the robot
> and commit message referenced by the patch instances.

OK.

> Can you please compile the files modified using the
> appropriate defconfig/allyesconfig and show the

I currently send patches for this issue only for files that compile using
the x86 allyesconfig.

> movement from data to const by using
>   $ size .new/old
> and include that in the changelogs (maybe next time)?

OK, thanks for the suggestion.

> Is it possible for a rule to trace the instances where
> an address of a struct or struct member is taken by
> locally defined and declared function call where the
> callee does not modify any dereferenced object?
>
> ie:
>
> struct foo {
>   int bar;
>   char *baz;
> };
>
> struct foo qux[] = {
>   { 1, "description 1" },
>   { 2, "dewcription 2" },
>   [ n, "etc" ]...,
> };
>
> void message(struct foo *msg)
> {
>   printk("%d %s\n", msg->bar, msg->baz);
> }
>
> where some code uses
>
>   message(qux[index]);
>
> So could a coccinelle script change:
>
> struct foo qux[] = { to const struct foo quz[] = {
>
> and
>
> void message(struct foo *msg) to void message(const struct foo *msg)

Yes, this could be possible too.

Thanks for the feedback.

julia
--
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


  1   2   3   4   >