Re: [PATCH] leds: tlc591xx: fix device_node_continue.cocci warnings (fwd)

2018-12-07 Thread Julia Lawall



On Fri, 7 Dec 2018, Jacek Anaszewski wrote:

> Hi Julia,
>
> Thank you for the patch, but it doesn't apply to LED tree.
>
> The patch causing the problem is out-of-LED-tree.

OK, I guess that the patch is in a TI-specific tree, given the name.

Thanks for looking into it.

julia

>
> Best regards,
> Jacek Anaszewski
>
> On 12/6/18 9:28 PM, Julia Lawall wrote:
> > Hello,
> >
> > The code seems to be wrong in several ways.  If the continue is wanted,
> > the of_node_put is not needed; it will happen on the next iteration.  If
> > the continue is not wanted, the of_node_put is needed, and the continue
> > should be dropped.
> >
> > julia
> >
> > -- Forwarded message --
> > Date: Thu, 6 Dec 2018 19:48:54 +0800
> > From: kbuild test robot 
> > To: kbu...@01.org
> > Cc: Julia Lawall 
> > Subject: [PATCH] leds: tlc591xx: fix device_node_continue.cocci warnings
> >
> > CC: kbuild-...@01.org
> > TO: Jyri Sarha 
> > CC: Peter Ujfalusi 
> > CC: Jacek Anaszewski 
> > CC: Pavel Machek 
> > CC: linux-l...@vger.kernel.org
> > CC: linux-kernel@vger.kernel.org
> >
> > From: kbuild test robot 
> >
> > drivers/leds/leds-tlc591xx.c:342:3-14: ERROR: probable double put.
> >
> >   Device node iterators put the previous value of the index variable, so an
> >   explicit put causes a double put.
> >
> > Generated by: scripts/coccinelle/iterators/device_node_continue.cocci
> >
> > Fixes: 7b2d34aaede7 ("leds: tlc591xx: Add gpio output support")
> > CC: Jyri Sarha 
> > Signed-off-by: kbuild test robot 
> > ---
> >
> > tree:   https://github.com/omap-audio/linux-audio peter/ti-linux-4.19.y/wip
> > head:   838f24e2deaf1229002bd6555eb7e889b09ac1f9
> > commit: 7b2d34aaede727b4abfc78061bbd2202fcd92bc8 [62/67] leds: tlc591xx: Add
> > gpio output support
> > :: branch date: 26 hours ago
> > :: commit date: 26 hours ago
> >
> > Please take the patch only if it's a positive warning. Thanks!
> >
> >   leds-tlc591xx.c |1 -
> >   1 file changed, 1 deletion(-)
> >
> > --- a/drivers/leds/leds-tlc591xx.c
> > +++ b/drivers/leds/leds-tlc591xx.c
> > @@ -339,7 +339,6 @@ tlc591xx_probe(struct i2c_client *client
> > for_each_child_of_node(np, child) {
> > err = of_property_read_u32(child, "reg", );
> > if (err) {
> > -   of_node_put(child);
> > continue;
> > return err;
> > }
> >
>
>
>


[PATCH] leds: tlc591xx: fix device_node_continue.cocci warnings (fwd)

2018-12-06 Thread Julia Lawall
Hello,

The code seems to be wrong in several ways.  If the continue is wanted,
the of_node_put is not needed; it will happen on the next iteration.  If
the continue is not wanted, the of_node_put is needed, and the continue
should be dropped.

julia

-- Forwarded message --
Date: Thu, 6 Dec 2018 19:48:54 +0800
From: kbuild test robot 
To: kbu...@01.org
Cc: Julia Lawall 
Subject: [PATCH] leds: tlc591xx: fix device_node_continue.cocci warnings

CC: kbuild-...@01.org
TO: Jyri Sarha 
CC: Peter Ujfalusi 
CC: Jacek Anaszewski 
CC: Pavel Machek 
CC: linux-l...@vger.kernel.org
CC: linux-kernel@vger.kernel.org

From: kbuild test robot 

drivers/leds/leds-tlc591xx.c:342:3-14: ERROR: probable double put.

 Device node iterators put the previous value of the index variable, so an
 explicit put causes a double put.

Generated by: scripts/coccinelle/iterators/device_node_continue.cocci

Fixes: 7b2d34aaede7 ("leds: tlc591xx: Add gpio output support")
CC: Jyri Sarha 
Signed-off-by: kbuild test robot 
---

tree:   https://github.com/omap-audio/linux-audio peter/ti-linux-4.19.y/wip
head:   838f24e2deaf1229002bd6555eb7e889b09ac1f9
commit: 7b2d34aaede727b4abfc78061bbd2202fcd92bc8 [62/67] leds: tlc591xx: Add 
gpio output support
:: branch date: 26 hours ago
:: commit date: 26 hours ago

Please take the patch only if it's a positive warning. Thanks!

 leds-tlc591xx.c |1 -
 1 file changed, 1 deletion(-)

--- a/drivers/leds/leds-tlc591xx.c
+++ b/drivers/leds/leds-tlc591xx.c
@@ -339,7 +339,6 @@ tlc591xx_probe(struct i2c_client *client
for_each_child_of_node(np, child) {
err = of_property_read_u32(child, "reg", );
if (err) {
-   of_node_put(child);
continue;
return err;
}


Re: [PATCH] locktorture: Fix assignment of boolean variables

2018-12-03 Thread Julia Lawall



On Mon, 3 Dec 2018, Peter Zijlstra wrote:

> On Mon, Dec 03, 2018 at 10:20:42AM +0100, Julia Lawall wrote:
> > Personally, I would prefer that assignments involving boolean variables
> > use true or false.  It seems more readable.  Potentially better for tools
> > as well.
>
> Then those tools are broken per the C spec.
>
> > But if the community really prefers 0 and 1, then the test can
> > be deleted.
>
> The C language spec, specifies _Bool as an integer type wide enough to
> at least store 0 and 1.
>
> IOW, 0 and 1 are perfectly valid valus to assign to a _Bool.
>
> And fundamentally that has to be so. That's how computers work. 0 is
> false, 1 is true.
>
> The kernel is not the place to try and abstract such stuff, C is our
> portable assembler. We muck with hardware, we'd better know how the heck
> it works.

How about it it were suggested only in files that already use true and
false somewhere?

julia


Re: [PATCH] locktorture: Fix assignment of boolean variables

2018-12-03 Thread Julia Lawall



On Mon, 3 Dec 2018, Peter Zijlstra wrote:

> On Mon, Dec 03, 2018 at 09:35:00AM +0100, Peter Zijlstra wrote:
> > On Sat, Dec 01, 2018 at 12:37:01PM -0800, Paul E. McKenney wrote:
> > > On Sat, Dec 01, 2018 at 04:31:49PM +0800, Wen Yang wrote:
> > > > Fix the following warnings reported by coccinelle:
> > > >
> > > > kernel/locking/locktorture.c:703:6-10: WARNING: Assignment of bool to 
> > > > 0/1
>
> Not to mention that WARN is gramatically incorrect. We're not assigning
> 'bool' to 0/1 but the other way around.
>
> What crap..
>
> > So I strongly disagree with this. Anybody that has trouble with 0/1 vs
> > false/true needs to stay the heck away from C.
> >
> > I would suggest we delete that stupid coccinelle scripts that generates
> > these pointless warns.

Personally, I would prefer that assignments involving boolean variables
use true or false.  It seems more readable.  Potentially better for tools
as well.  But if the community really prefers 0 and 1, then the test can
be deleted.

julia


Re: [PATCH] tty: serial: qcom_geni_serial: Fix softlock

2018-11-27 Thread Julia Lawall
Hello,

Since size_t is unsigned, avail will not be less than 0 on line 742.
Perhaps just reorganize the computation on line 740.

julia

-- Forwarded message --
Date: Tue, 27 Nov 2018 16:13:53 +0800
From: kbuild test robot 
To: kbu...@01.org
Cc: Julia Lawall 
Subject: Re: [PATCH] tty: serial: qcom_geni_serial: Fix softlock

CC: kbuild-...@01.org
In-Reply-To: <20181127022536.104663-1-ryandc...@chromium.org>
References: <20181127022536.104663-1-ryandc...@chromium.org>
TO: Ryan Case 
CC: Greg Kroah-Hartman , Jiri Slaby 
, Evan Green , Doug Anderson 
, linux-kernel@vger.kernel.org, 
linux-ser...@vger.kernel.org, Ryan Case 
CC: Evan Green , Doug Anderson , 
linux-kernel@vger.kernel.org, linux-ser...@vger.kernel.org, Ryan Case 


Hi Ryan,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on tty/tty-testing]
[also build test WARNING on v4.20-rc4 next-20181126]
[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/Ryan-Case/tty-serial-qcom_geni_serial-Fix-softlock/20181127-102810
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty.git 
tty-testing
:: branch date: 6 hours ago
:: commit date: 6 hours ago

>> drivers/tty/serial/qcom_geni_serial.c:742:5-10: WARNING: Unsigned expression 
>> compared with zero: avail < 0

# 
https://github.com/0day-ci/linux/commit/407559b41ed61fd8c95ebe39539677bc577c7c66
git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout 407559b41ed61fd8c95ebe39539677bc577c7c66
vim +742 drivers/tty/serial/qcom_geni_serial.c

c4f528795 Karthikeyan Ramasubramanian 2018-03-14  712
407559b41 Ryan Case   2018-11-26  713  static void 
qcom_geni_serial_handle_tx(struct uart_port *uport, bool done,
407559b41 Ryan Case   2018-11-26  714   bool active)
c4f528795 Karthikeyan Ramasubramanian 2018-03-14  715  {
c4f528795 Karthikeyan Ramasubramanian 2018-03-14  716   struct 
qcom_geni_serial_port *port = to_dev_port(uport, uport);
c4f528795 Karthikeyan Ramasubramanian 2018-03-14  717   struct circ_buf *xmit = 
>state->xmit;
c4f528795 Karthikeyan Ramasubramanian 2018-03-14  718   size_t avail;
c4f528795 Karthikeyan Ramasubramanian 2018-03-14  719   size_t remaining;
407559b41 Ryan Case   2018-11-26  720   size_t pending;
c4f528795 Karthikeyan Ramasubramanian 2018-03-14  721   int i;
c4f528795 Karthikeyan Ramasubramanian 2018-03-14  722   u32 status;
c4f528795 Karthikeyan Ramasubramanian 2018-03-14  723   unsigned int chunk;
c4f528795 Karthikeyan Ramasubramanian 2018-03-14  724   int tail;
c4f528795 Karthikeyan Ramasubramanian 2018-03-14  725
c4f528795 Karthikeyan Ramasubramanian 2018-03-14  726   status = 
readl_relaxed(uport->membase + SE_GENI_TX_FIFO_STATUS);
407559b41 Ryan Case   2018-11-26  727
407559b41 Ryan Case   2018-11-26  728   /* Complete the current 
tx command before taking newly added data */
407559b41 Ryan Case   2018-11-26  729   if (active)
407559b41 Ryan Case   2018-11-26  730   pending = 
port->cur_tx_remaining;
407559b41 Ryan Case   2018-11-26  731   else
407559b41 Ryan Case   2018-11-26  732   pending = 
uart_circ_chars_pending(xmit);
407559b41 Ryan Case   2018-11-26  733
407559b41 Ryan Case   2018-11-26  734   /* All data has been 
transmitted and acknowledged as received */
407559b41 Ryan Case   2018-11-26  735   if (!pending && !status 
&& done) {
c4f528795 Karthikeyan Ramasubramanian 2018-03-14  736   
qcom_geni_serial_stop_tx(uport);
c4f528795 Karthikeyan Ramasubramanian 2018-03-14  737   goto 
out_write_wakeup;
c4f528795 Karthikeyan Ramasubramanian 2018-03-14  738   }
c4f528795 Karthikeyan Ramasubramanian 2018-03-14  739
407559b41 Ryan Case   2018-11-26  740   avail = 
port->tx_fifo_depth - (status & TX_FIFO_WC);
407559b41 Ryan Case   2018-11-26  741   avail *= 
port->tx_bytes_pw;
407559b41 Ryan Case   2018-11-26 @742   if (avail < 0)
407559b41 Ryan Case   2018-11-26  743   avail = 0;
8a8a66a1a Girish Mahadevan2018-07-13  744
638a6f4eb Evan Green  2018-05-09  745   tail = xmit->tail;
407559b41 Ryan Case   2018-11-26  746   chunk = 
min3((size_t)pending, (size_t)(UART_XMIT_SIZE - tail), avail);
c4f528795 Karthikeyan Ramasubramanian 2018-03-14  747   if (!chunk)
c4f528795 Karthikeyan Ramasubramanian 2018-03-14  748   goto 
out_write_wakeup;
c4f528795 Karthikeyan Ramasubramanian 2018-03-14  749
407559b41 Ryan Case   2018-11-26  750   if 
(!port->cur_tx_remaining) {
407559b41 Ryan Case   2018-11-26  751

Re: [PATCH] of: add dtc annotations functionality to dtx_diff

2018-11-26 Thread Julia Lawall



On Mon, 26 Nov 2018, frowand.l...@gmail.com wrote:

> From: Frank Rowand 
>
> Add -T and --annotations command line arguments to dtx_diff.  These
> arguments will be passed through to dtc.  dtc will then add source
> location annotations to its output.
>
> Signed-off-by: Frank Rowand 

Tested-by: Julia Lawall 

> ---
>
> This feature depends upon commit 5667e7ef9a9a ("annotations: add the
> annotation functionality") in the dtc git repository.  To use the
> new flags before the new version of dtc is imported to the linux
> kernel, download the dtc repository, compile dtc with the make command,
> then add the path of the dtc repository to the shell PATH variable.
>
>  scripts/dtc/dtx_diff | 13 -
>  1 file changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/scripts/dtc/dtx_diff b/scripts/dtc/dtx_diff
> index 8c4fbad2055e..0d8572008729 100755
> --- a/scripts/dtc/dtx_diff
> +++ b/scripts/dtc/dtx_diff
> @@ -21,6 +21,7 @@ Usage:
>  diff DTx_1 and DTx_2
>
>
> +  --annotatesynonym for -T
> -f   print full dts in diff (--unified=9)
> -h   synonym for --help
> -helpsynonym for --help
> @@ -28,6 +29,7 @@ Usage:
> -s SRCTREE   linux kernel source tree is at path SRCTREE
>  (default is current directory)
> -S   linux kernel source tree is at root of current git repo
> +   -T   Annotate output .dts with input source file and line (-T 
> -T for more details)
> -u   unsorted, do not sort DTx
>
>
> @@ -174,6 +176,7 @@ compile_to_dts() {
>
>  # -  start of script
>
> +annotate=""
>  cmd_diff=0
>  diff_flags="-u"
>  dtx_file_1=""
> @@ -208,6 +211,14 @@ while [ $# -gt 0 ] ; do
>   shift
>   ;;
>
> + -T | --annotate )
> + if [ "${annotate}"  = "" ] ; then
> + annotate="-T"
> + elif [ "${annotate}"  = "-T" ] ; then
> + annotate="-T -T"
> + fi
> + shift
> + ;;
>   -u )
>   dtc_sort=""
>   shift
> @@ -327,7 +338,7 @@ cpp_flags="\
>  DTC="\
>   ${DTC} \
>   -i ${srctree}/scripts/dtc/include-prefixes \
> - -O dts -qq -f ${dtc_sort} -o -"
> + -O dts -qq -f ${dtc_sort} ${annotate} -o -"
>
>
>  # -  do the diff or decompile
> --
> Frank Rowand 
>
>


[PATCH] mailbox: hi3660: constify mbox_chan_ops structure

2018-11-02 Thread Julia Lawall
The mbox_chan_ops structure can be const as it is only stored in the
ops field of an mbox_controller structure and this field is const.

Done with the help of Coccinelle.

Signed-off-by: Julia Lawall 

---
 drivers/mailbox/hi3660-mailbox.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mailbox/hi3660-mailbox.c b/drivers/mailbox/hi3660-mailbox.c
index 3eea6b642484..c3fd1f2b797f 100644
--- a/drivers/mailbox/hi3660-mailbox.c
+++ b/drivers/mailbox/hi3660-mailbox.c
@@ -206,7 +206,7 @@ static int hi3660_mbox_send_data(struct mbox_chan *chan, 
void *msg)
return 0;
 }
 
-static struct mbox_chan_ops hi3660_mbox_ops = {
+static const struct mbox_chan_ops hi3660_mbox_ops = {
.startup= hi3660_mbox_startup,
.send_data  = hi3660_mbox_send_data,
 };



[PATCH] ASoC: smd845: constify snd_soc_ops structure

2018-11-02 Thread Julia Lawall
The snd_soc_ops structure can be const as it is only stored in the
ops field of a snd_soc_dai_link structure and this field is const.

Done with the help of Coccinelle.

Signed-off-by: Julia Lawall 

---
 sound/soc/qcom/sdm845.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/soc/qcom/sdm845.c b/sound/soc/qcom/sdm845.c
index 9effbecc571f..8d0cdff64377 100644
--- a/sound/soc/qcom/sdm845.c
+++ b/sound/soc/qcom/sdm845.c
@@ -171,7 +171,7 @@ static void  sdm845_snd_shutdown(struct snd_pcm_substream 
*substream)
}
 }
 
-static struct snd_soc_ops sdm845_be_ops = {
+static const struct snd_soc_ops sdm845_be_ops = {
.hw_params = sdm845_snd_hw_params,
.startup = sdm845_snd_startup,
.shutdown = sdm845_snd_shutdown,



Re: [Outreachy kernel] Re: [PATCH v2] staging: vboxvideo: Remove unnecessary parentheses

2018-10-31 Thread Julia Lawall



On Tue, 30 Oct 2018, Shayenne Moura wrote:

> On 10/30, Greg Kroah-Hartman wrote:
> > On Tue, Oct 23, 2018 at 02:43:04PM -0300, Shayenne da Luz Moura wrote:
> > > Remove unneeded parentheses around the arguments of ||. This reduces
> > > clutter and code behave in the same way.
> > > Change suggested by checkpatch.pl.
> > >
> > > vbox_main.c:119: CHECK: Unnecessary parentheses around 'rects[i].x2 <
> > > crtc->x'
> > >
> > > Signed-off-by: Shayenne da Luz Moura 
> > > Reviewed-by: Hans de Goede 
> > > ---
> > > Changes in v2:
> > >   - Make the commit message more clearer.
> >
> > This patch does not apply to the latest kernel tree at all :(
> >
> > Please fix up and resend.
> >
> > thanks,
> >
> > greg k-h
>
> Hi Greg!
>
> Sorry, I do not know what branch are expected to be used.
> I used the drm-misc-next to create this patch and I could not
> find any merge problem.
>
> Could you please tell me the details?

For staging drivers you should use the staging tree as indicated in the
outreachy tutorial.

julia


>
> Thanks,
>
> Shayenne
>
> --
> 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/20181030231757.24ooubt25oykmgkt%40smtp.gmail.com.
> For more options, visit https://groups.google.com/d/optout.
>


Re: [Outreachy kernel] [RESEND PATCH 2/2] staging: vboxvideo: Use unsigned int instead bool

2018-10-30 Thread Julia Lawall



On Tue, 30 Oct 2018, Shayenne Moura wrote:

> Hi,
>
> > On Sun, 28 Oct 2018, Himanshu Jha wrote:
> >
> > > On Sun, Oct 28, 2018 at 09:47:15AM +0100, Julia Lawall wrote:
> > > > > The "possible alignement issues" in CHECK report is difficult to 
> > > > > figure
> > > > > out by just doing a glance analysis. :)
> > > > >
> > > > > Linus also suggested to use bool as the base type i.e., `bool x:1` but
> > > > > again sizeof(_Bool) is implementation defined ranging from 1-4 bytes.
> > > >
> > > > If bool x:1 has the size of bool, then wouldn't int x:1 have the size of
> > > > int?  But my little experiments suggest that the size is the smallest 
> > > > that
> > > > fits the requested bits and alignment chosen by the compiler, 
> > > > regardless of
> > > > the type.
> > >
> > > Yes, correct!
> > > And we can't use sizeof on bitfields *directly*, nor reference it using a
> > > pointer.
> > >
> > > It can be applied only when these bitfields are wrapped in a structure.
> > >
> > > Testing:
> > >
> > > #include 
> > > #include 
> > >
> > > struct S {
> > > bool a:1;
> > > bool b:1;
> > > bool c:1;
> > > bool d:1;
> > > };
> > >
> > > int main(void)
> > > {
> > > printf("%zu\n", sizeof(struct S));
> > > }
> > >
> > > Output: 1
> > >
> > > If I change all bool to unsigned int, output is: *4*.
> > >
> > > So, conclusion is compiler doesn't squeeze the size less than
> > > native size of the datatype i.e., if we changed all members to
> > > unsigned int:1,
> > > total width = 4 bits
> > > padding = 4 bits
> > >
> > > Therefore, total size should have been = 1 byte!
> > > But since sizeof(unsigned int) == 4, it can't be squeezed to
> > > less than it.
> >
> > This conclusion does not seem to be correct, if you try the following
> > program.  I get 4 for everything, meaning that the four unsigned int bits
> > are getting squeezed into one byte when it is convenient.
> >
> > #include 
> > #include 
> >
> > struct S1 {
> > bool a:1;
> > bool b:1;
> > bool c:1;
> > bool d:1;
> > char a1;
> > char a2;
> > char a3;
> > };
> >
> > struct S2 {
> > unsigned int a:1;
> > unsigned int b:1;
> > unsigned int c:1;
> > unsigned int d:1;
> > char a1;
> > char a2;
> > char a3;
> > };
> >
> > int main(void)
> > {
> > printf("%zu\n", sizeof(struct S1));
> > printf("%zu\n", sizeof(struct S2));
> > printf("%zu\n", sizeof(unsigned int));
> > }
> >
> > > Well, int x:1 can either have 0..1 or -1..0 range due implementation
> > > defined behavior as I said in the previous reply.
> > >
> > > If you really want to consider negative values, then make it explicit
> > > using `signed int x:1` which make range guaranteed to be -1..0
> >
> > The code wants booleans, not negative values.
> >
> > julia
>
> Thank you all for the discussion!
>
> However, I think I do not understand the conclusion.
>
> It means that the best way is to use only boolean instead of use unsigned
> int with bitfield? I mean specifically in the case of my patch, where there
> are some boolean variables are mixed with other variables types.

To my recollection, your code had a bool with larger types on either side.
In that case, I think bool is fine.  The compiler it likely to align those
larger typed values such that the field with the bool type will get more
than one byte no matter what type you use.  If there are several fields
with very small types adjacent, there might be some benefit to thinking
about what the type should be.

julia


[PATCH] thermal: broadcom: use devm_thermal_zone_of_sensor_register

2018-10-30 Thread Julia Lawall
Using devm_thermal_zone_of_sensor_register allows to simplify some
error handling code, drop a label, and drop the remove function.

Signed-off-by: Julia Lawall 

---

This patch is completely orthogonal to the recent constification
patch.

 drivers/thermal/broadcom/brcmstb_thermal.c |   24 
 1 file changed, 4 insertions(+), 20 deletions(-)

diff --git a/drivers/thermal/broadcom/brcmstb_thermal.c 
b/drivers/thermal/broadcom/brcmstb_thermal.c
index 1919f91fa756..956eef8717bb 100644
--- a/drivers/thermal/broadcom/brcmstb_thermal.c
+++ b/drivers/thermal/broadcom/brcmstb_thermal.c
@@ -329,7 +329,8 @@ static int brcmstb_thermal_probe(struct platform_device 
*pdev)
priv->dev = >dev;
platform_set_drvdata(pdev, priv);
 
-   thermal = thermal_zone_of_sensor_register(>dev, 0, priv, _ops);
+   thermal = devm_thermal_zone_of_sensor_register(>dev, 0, priv,
+  _ops);
if (IS_ERR(thermal)) {
ret = PTR_ERR(thermal);
dev_err(>dev, "could not register sensor: %d\n", ret);
@@ -341,40 +342,23 @@ static int brcmstb_thermal_probe(struct platform_device 
*pdev)
irq = platform_get_irq(pdev, 0);
if (irq < 0) {
dev_err(>dev, "could not get IRQ\n");
-   ret = irq;
-   goto err;
+   return irq;
}
ret = devm_request_threaded_irq(>dev, irq, NULL,
brcmstb_tmon_irq_thread, IRQF_ONESHOT,
DRV_NAME, priv);
if (ret < 0) {
dev_err(>dev, "could not request IRQ: %d\n", ret);
-   goto err;
+   return ret;
}
 
dev_info(>dev, "registered AVS TMON of-sensor driver\n");
 
return 0;
-
-err:
-   thermal_zone_of_sensor_unregister(>dev, thermal);
-   return ret;
-}
-
-static int brcmstb_thermal_exit(struct platform_device *pdev)
-{
-   struct brcmstb_thermal_priv *priv = platform_get_drvdata(pdev);
-   struct thermal_zone_device *thermal = priv->thermal;
-
-   if (thermal)
-   thermal_zone_of_sensor_unregister(>dev, priv->thermal);
-
-   return 0;
 }
 
 static struct platform_driver brcmstb_thermal_driver = {
.probe = brcmstb_thermal_probe,
-   .remove = brcmstb_thermal_exit,
.driver = {
.name = DRV_NAME,
.of_match_table = brcmstb_thermal_id_table,



[PATCH 1/2] usb: gadget: uvc: constify vb2_ops structure

2018-10-30 Thread Julia Lawall
The vb2_ops structure can be const as it is only stored in the ops
field of a vb2_queue structure and this field is const.

Done with the help of Coccinelle.

Signed-off-by: Julia Lawall 

---
 drivers/usb/gadget/function/uvc_queue.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/function/uvc_queue.c 
b/drivers/usb/gadget/function/uvc_queue.c
index 9e33d5206d54..526a10a33094 100644
--- a/drivers/usb/gadget/function/uvc_queue.c
+++ b/drivers/usb/gadget/function/uvc_queue.c
@@ -102,7 +102,7 @@ static void uvc_buffer_queue(struct vb2_buffer *vb)
spin_unlock_irqrestore(>irqlock, flags);
 }
 
-static struct vb2_ops uvc_queue_qops = {
+static const struct vb2_ops uvc_queue_qops = {
.queue_setup = uvc_queue_setup,
.buf_prepare = uvc_buffer_prepare,
.buf_queue = uvc_buffer_queue,



[PATCH 2/2] thermal: broadcom: constify thermal_zone_of_device_ops structure

2018-10-30 Thread Julia Lawall
The thermal_zone_of_device_ops structure can be const as it is only
passed as the last argument of thermal_zone_of_sensor_register
and the corresponding parameter is declared as const.

Done with the help of Coccinelle.

Signed-off-by: Julia Lawall 

---

Unrelated to this change, is there a reason not to use
devm_thermal_zone_of_sensor_register?

 drivers/thermal/broadcom/brcmstb_thermal.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff -u -p a/drivers/thermal/broadcom/brcmstb_thermal.c 
b/drivers/thermal/broadcom/brcmstb_thermal.c
--- a/drivers/thermal/broadcom/brcmstb_thermal.c
+++ b/drivers/thermal/broadcom/brcmstb_thermal.c
@@ -299,7 +299,7 @@ static int brcmstb_set_trips(void *data,
return 0;
 }
 
-static struct thermal_zone_of_device_ops of_ops = {
+static const struct thermal_zone_of_device_ops of_ops = {
.get_temp   = brcmstb_get_temp,
.set_trips  = brcmstb_set_trips,
 };



[PATCH 1/2] thermal: armada: constify thermal_zone_of_device_ops structure

2018-10-30 Thread Julia Lawall
The thermal_zone_of_device_ops structure can be const as it is only
passed as the last argument of devm_thermal_zone_of_sensor_register
and the corresponding parameter is declared as const.

Done with the help of Coccinelle.

Signed-off-by: Julia Lawall 

---
 drivers/thermal/armada_thermal.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff -u -p a/drivers/thermal/armada_thermal.c b/drivers/thermal/armada_thermal.c
--- a/drivers/thermal/armada_thermal.c
+++ b/drivers/thermal/armada_thermal.c
@@ -395,7 +395,7 @@ unlock_mutex:
return ret;
 }
 
-static struct thermal_zone_of_device_ops of_ops = {
+static const struct thermal_zone_of_device_ops of_ops = {
.get_temp = armada_get_temp,
 };
 



[PATCH 0/2] constify thermal_zone_of_device_ops structures

2018-10-30 Thread Julia Lawall
The thermal_zone_of_device_ops structures can be const as they are
only passed as the last argument of a thermal_zone_of_sensor_register
function and the corresponding parameter is declared as const.

Done with the help of Coccinelle.

---

 drivers/thermal/armada_thermal.c   |2 +-
 drivers/thermal/broadcom/brcmstb_thermal.c |2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)


Re: [PATCH 2/8] ARM: vexpress/spc: constify clk_ops structure

2018-10-29 Thread Julia Lawall


On Mon, 29 Oct 2018, Liviu Dudau wrote:

> On Sat, Oct 27, 2018 at 07:47:36AM +0200, Julia Lawall wrote:
> > The clk_ops structure is only stored in the ops field of a
> > clk_init_data structure.  This field is const, so the clk_ops
> > structure can be const as well.
> >
> > Identified and transformed using Coccinelle.
> >
> > Signed-off-by: Julia Lawall 
>
> Acked-by: Liviu Dudau 
>
> Are you going to send the series to the arm-soc maintainers directly or
> you want us to cherry-pick this patch into the Versatile tree?

Please do with it whatever seems best.

Thanks,
julia

>
> Best regards,
> Liviu
>
> >
> > ---
> >  arch/arm/mach-vexpress/spc.c |2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/arch/arm/mach-vexpress/spc.c b/arch/arm/mach-vexpress/spc.c
> > index 0f5381d13494..611f22de48cb 100644
> > --- a/arch/arm/mach-vexpress/spc.c
> > +++ b/arch/arm/mach-vexpress/spc.c
> > @@ -521,7 +521,7 @@ static int spc_set_rate(struct clk_hw *hw, unsigned 
> > long rate,
> > return ve_spc_set_performance(spc->cluster, rate / 1000);
> >  }
> >
> > -static struct clk_ops clk_spc_ops = {
> > +static const struct clk_ops clk_spc_ops = {
> > .recalc_rate = spc_recalc_rate,
> > .round_rate = spc_round_rate,
> > .set_rate = spc_set_rate,
> >
>
> --
> 
> | I would like to |
> | fix the world,  |
> | but they're not |
> | giving me the   |
>  \ source code!  /
>   ---
> ¯\_(ツ)_/¯
>

Re: [Outreachy kernel] [RESEND PATCH 2/2] staging: vboxvideo: Use unsigned int instead bool

2018-10-28 Thread Julia Lawall



On Sun, 28 Oct 2018, Himanshu Jha wrote:

> On Sun, Oct 28, 2018 at 09:47:15AM +0100, Julia Lawall wrote:
> > > The "possible alignement issues" in CHECK report is difficult to figure
> > > out by just doing a glance analysis. :)
> > >
> > > Linus also suggested to use bool as the base type i.e., `bool x:1` but
> > > again sizeof(_Bool) is implementation defined ranging from 1-4 bytes.
> >
> > If bool x:1 has the size of bool, then wouldn't int x:1 have the size of
> > int?  But my little experiments suggest that the size is the smallest that
> > fits the requested bits and alignment chosen by the compiler, regardless of
> > the type.
>
> Yes, correct!
> And we can't use sizeof on bitfields *directly*, nor reference it using a
> pointer.
>
> It can be applied only when these bitfields are wrapped in a structure.
>
> Testing:
>
> #include 
> #include 
>
> struct S {
> bool a:1;
> bool b:1;
> bool c:1;
> bool d:1;
> };
>
> int main(void)
> {
> printf("%zu\n", sizeof(struct S));
> }
>
> Output: 1
>
> If I change all bool to unsigned int, output is: *4*.
>
> So, conclusion is compiler doesn't squeeze the size less than
> native size of the datatype i.e., if we changed all members to
> unsigned int:1,
> total width = 4 bits
> padding = 4 bits
>
> Therefore, total size should have been = 1 byte!
> But since sizeof(unsigned int) == 4, it can't be squeezed to
> less than it.

This conclusion does not seem to be correct, if you try the following
program.  I get 4 for everything, meaning that the four unsigned int bits
are getting squeezed into one byte when it is convenient.

#include 
#include 

struct S1 {
bool a:1;
bool b:1;
bool c:1;
bool d:1;
char a1;
char a2;
char a3;
};

struct S2 {
unsigned int a:1;
unsigned int b:1;
unsigned int c:1;
unsigned int d:1;
char a1;
char a2;
char a3;
};

int main(void)
{
printf("%zu\n", sizeof(struct S1));
printf("%zu\n", sizeof(struct S2));
printf("%zu\n", sizeof(unsigned int));
}

> Well, int x:1 can either have 0..1 or -1..0 range due implementation
> defined behavior as I said in the previous reply.
>
> If you really want to consider negative values, then make it explicit
> using `signed int x:1` which make range guaranteed to be -1..0

The code wants booleans, not negative values.

julia


Re: [Outreachy kernel] [RESEND PATCH 2/2] staging: vboxvideo: Use unsigned int instead bool

2018-10-28 Thread Julia Lawall
> The "possible alignement issues" in CHECK report is difficult to figure
> out by just doing a glance analysis. :)
>
> Linus also suggested to use bool as the base type i.e., `bool x:1` but
> again sizeof(_Bool) is implementation defined ranging from 1-4 bytes.

If bool x:1 has the size of bool, then wouldn't int x:1 have the size of
int?  But my little experiments suggest that the size is the smallest that
fits the requested bits and alignment chosen by the compiler, regardless of
the type.

bool x:1 has the advantage that anything that is not 0 is considered true.
So for bool x:1, x = 4 is true, while for int x:1, x = 4 is false.

But the :1 adds instructions, so at least for only one bool, where little
space is saved, it is probably not worth it.

julia


[PATCH] ASoC: AMD: constify regulator_desc structure

2018-10-28 Thread Julia Lawall
The regulator_desc structure can be const as it is only passed as the
second argument of devm_regulator_register and the corresponding
parameter is declared as const.

Done with the help of Coccinelle.

Signed-off-by: Julia Lawall 

---
 sound/soc/amd/acp-da7219-max98357a.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/soc/amd/acp-da7219-max98357a.c 
b/sound/soc/amd/acp-da7219-max98357a.c
index 3f813ea5210a..a5daad973ce5 100644
--- a/sound/soc/amd/acp-da7219-max98357a.c
+++ b/sound/soc/amd/acp-da7219-max98357a.c
@@ -403,7 +403,7 @@ static struct regulator_config acp_da7219_cfg = {
 static struct regulator_ops acp_da7219_ops = {
 };
 
-static struct regulator_desc acp_da7219_desc = {
+static const struct regulator_desc acp_da7219_desc = {
.name = "reg-fixed-1.8V",
.type = REGULATOR_VOLTAGE,
.owner = THIS_MODULE,



Re: [Outreachy kernel] [RESEND PATCH 2/2] staging: vboxvideo: Use unsigned int instead bool

2018-10-27 Thread Julia Lawall



On Sat, 27 Oct 2018, Joe Perches wrote:

> On Fri, 2018-10-26 at 22:54 +0200, Julia Lawall wrote:
> > [Adding Joe Perches]
> >
> > On Fri, 26 Oct 2018, Sasha Levin wrote:
> >
> > > On Fri, Oct 26, 2018 at 04:04:45PM -0300, Shayenne da Luz Moura wrote:
> > > > This change was suggested by checkpath.pl. Use unsigned int with 
> > > > bitfield
> > > > allocate only one bit to the boolean variable.
> > > >
> > > > CHECK: Avoid using bool structure members because of possible alignment
> > > > issues
> > > >
> > > > Signed-off-by: Shayenne da Luz Moura 
> > > > ---
> > > > drivers/staging/vboxvideo/vbox_drv.h| 14 +++---
> > > > drivers/staging/vboxvideo/vboxvideo_guest.h |  2 +-
> > > > 2 files changed, 8 insertions(+), 8 deletions(-)
> > > >
> > > > diff --git a/drivers/staging/vboxvideo/vbox_drv.h
> > > > b/drivers/staging/vboxvideo/vbox_drv.h
> > > > index 594f84272957..7d3e329a6b1c 100644
> > > > --- a/drivers/staging/vboxvideo/vbox_drv.h
> > > > +++ b/drivers/staging/vboxvideo/vbox_drv.h
> > > > @@ -81,7 +81,7 @@ struct vbox_private {
> > > > u8 __iomem *vbva_buffers;
> > > > struct gen_pool *guest_pool;
> > > > struct vbva_buf_ctx *vbva_info;
> > > > -   bool any_pitch;
> > > > +   unsigned int any_pitch:1;
> > > > u32 num_crtcs;
> > > > /** Amount of available VRAM, including space used for buffers. 
> > > > */
> > > > u32 full_vram_size;
> > >
> > > Using bitfields for booleans in these cases is less efficient than just
> > > using "regular" booleans for two reasons:
> > >
> > > 1. It will use the same amount of space. Due to alignment requirements,
> > > the compiler can't squeeze in anything into the 7 bits that are now
> > > "free". Each member, unless it's another bitfield, must start at a whole
> > > byte.

Since this is between a pointer and a u32, won't the compiler put a lot
more padding, in both cases?

> > >
> > > 2. This is actually less efficient (slower) for the compiler to work
> > > with. The smallest granularity we have to access memory is 1 byte; we
> > > can't set individual bits directly in memory. For the original code, the
> > > assembly for 'vbox_private.any_pitch = true' would look something like
> > > this:
> > >
> > >   movl   $0x1,-0x10(%rsp)
> > >
> > > As you can see, the compiler can directly write into the variable.
> > > However, when we switch to using bitfields, the compiler must preserve
> > > the original value of the other 7 bits, so it must first read them from
> > > memory, manipulate the value and write it back. The assembly would
> > > look something like this:
> > >
> > >   movzbl -0x10(%rsp),%eax
> > >   or $0x1,%eax
> > >   mov%al,-0x10(%rsp)
> > >
> > > Which is less efficient than what was previously happening.
> >
> > Maybe checkpatch could be more precise about what kind of bools should be
> > changed?
>
> Probably so, what verbiage would you suggest?

I don't know what are the conditions.  Sasha?

julia

> Also, any conversion from bool to int would
> have to take care than any assigment uses !!
> where appropriate.
>
>
>


[PATCH] PCI: histb: constify dw_pcie_host_ops structure

2018-10-27 Thread Julia Lawall
The dw_pcie_host_ops structure is only stored in the ops field
of a pcie_port structure, and this field is const, so make the
dw_pcie_host_ops structure const as well.

Done with the help of Coccinelle.

Signed-off-by: Julia Lawall 

---
 drivers/pci/controller/dwc/pcie-histb.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff -u -p a/drivers/pci/controller/dwc/pcie-histb.c 
b/drivers/pci/controller/dwc/pcie-histb.c
--- a/drivers/pci/controller/dwc/pcie-histb.c
+++ b/drivers/pci/controller/dwc/pcie-histb.c
@@ -202,7 +202,7 @@ static int histb_pcie_host_init(struct p
return 0;
 }
 
-static struct dw_pcie_host_ops histb_pcie_host_ops = {
+static const struct dw_pcie_host_ops histb_pcie_host_ops = {
.rd_own_conf = histb_pcie_rd_own_conf,
.wr_own_conf = histb_pcie_wr_own_conf,
.host_init = histb_pcie_host_init,



[PATCH 3/3] ASoC: codecs: constify snd_soc_dai_ops structures

2018-10-27 Thread Julia Lawall
The snd_soc_dai_ops structures are only stored in the ops field of a
snd_soc_dai_driver structure, so make the snd_soc_dai_ops structures
const as well.

Done with the help of Coccinelle.

Signed-off-by: Julia Lawall 

---
 sound/soc/codecs/ak4458.c   |2 +-
 sound/soc/codecs/ak5558.c   |2 +-
 sound/soc/codecs/hdac_hda.c |2 +-
 sound/soc/codecs/tas6424.c  |2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/sound/soc/codecs/ak4458.c b/sound/soc/codecs/ak4458.c
index 299ada4dfaa0..70d4c89bd6fc 100644
--- a/sound/soc/codecs/ak4458.c
+++ b/sound/soc/codecs/ak4458.c
@@ -456,7 +456,7 @@ static int ak4458_startup(struct snd_pcm_substream 
*substream,
return ret;
 }
 
-static struct snd_soc_dai_ops ak4458_dai_ops = {
+static const struct snd_soc_dai_ops ak4458_dai_ops = {
.startup= ak4458_startup,
.hw_params  = ak4458_hw_params,
.set_fmt= ak4458_set_dai_fmt,
diff --git a/sound/soc/codecs/ak5558.c b/sound/soc/codecs/ak5558.c
index 448bb90c9c8e..60f1f12c81ea 100644
--- a/sound/soc/codecs/ak5558.c
+++ b/sound/soc/codecs/ak5558.c
@@ -246,7 +246,7 @@ static int ak5558_startup(struct snd_pcm_substream 
*substream,
  _rate_constraints);
 }
 
-static struct snd_soc_dai_ops ak5558_dai_ops = {
+static const struct snd_soc_dai_ops ak5558_dai_ops = {
.startup= ak5558_startup,
.hw_params  = ak5558_hw_params,
 
diff --git a/sound/soc/codecs/hdac_hda.c b/sound/soc/codecs/hdac_hda.c
index 2aaa83028e55..ffecdaaa8cf2 100644
--- a/sound/soc/codecs/hdac_hda.c
+++ b/sound/soc/codecs/hdac_hda.c
@@ -46,7 +46,7 @@ static int hdac_hda_dai_set_tdm_slot(struct snd_soc_dai *dai,
 static struct hda_pcm *snd_soc_find_pcm_from_dai(struct hdac_hda_priv *hda_pvt,
 struct snd_soc_dai *dai);
 
-static struct snd_soc_dai_ops hdac_hda_dai_ops = {
+static const struct snd_soc_dai_ops hdac_hda_dai_ops = {
.startup = hdac_hda_dai_open,
.shutdown = hdac_hda_dai_close,
.prepare = hdac_hda_dai_prepare,
diff --git a/sound/soc/codecs/tas6424.c b/sound/soc/codecs/tas6424.c
index 36aebdb8f55c..aaba39295079 100644
--- a/sound/soc/codecs/tas6424.c
+++ b/sound/soc/codecs/tas6424.c
@@ -378,7 +378,7 @@ static struct snd_soc_component_driver 
soc_codec_dev_tas6424 = {
.non_legacy_dai_naming  = 1,
 };
 
-static struct snd_soc_dai_ops tas6424_speaker_dai_ops = {
+static const struct snd_soc_dai_ops tas6424_speaker_dai_ops = {
.hw_params  = tas6424_hw_params,
.set_fmt= tas6424_set_dai_fmt,
.set_tdm_slot   = tas6424_set_dai_tdm_slot,



[PATCH 1/3] soundwire: intel: constify snd_soc_dai_ops structures

2018-10-27 Thread Julia Lawall
The snd_soc_dai_ops structures are only stored in the ops field of a
snd_soc_dai_driver structure, so make the snd_soc_dai_ops structures
const as well.

Done with the help of Coccinelle.

Signed-off-by: Julia Lawall 

---
 drivers/soundwire/intel.c |4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c
index c5ee97ee7886..fd8d034cfec1 100644
--- a/drivers/soundwire/intel.c
+++ b/drivers/soundwire/intel.c
@@ -654,14 +654,14 @@ static int intel_pdm_set_sdw_stream(struct snd_soc_dai 
*dai,
return cdns_set_sdw_stream(dai, stream, false, direction);
 }
 
-static struct snd_soc_dai_ops intel_pcm_dai_ops = {
+static const struct snd_soc_dai_ops intel_pcm_dai_ops = {
.hw_params = intel_hw_params,
.hw_free = intel_hw_free,
.shutdown = sdw_cdns_shutdown,
.set_sdw_stream = intel_pcm_set_sdw_stream,
 };
 
-static struct snd_soc_dai_ops intel_pdm_dai_ops = {
+static const struct snd_soc_dai_ops intel_pdm_dai_ops = {
.hw_params = intel_hw_params,
.hw_free = intel_hw_free,
.shutdown = sdw_cdns_shutdown,



[PATCH 0/3] constify snd_soc_dai_ops structures

2018-10-27 Thread Julia Lawall
The snd_soc_dai_ops structures are only stored in the ops field of a
snd_soc_dai_driver structure, so make the snd_soc_dai_ops structures
const as well.

Done with the help of Coccinelle.

---

 drivers/soundwire/intel.c|4 ++--
 sound/soc/codecs/ak4458.c|2 +-
 sound/soc/codecs/ak5558.c|2 +-
 sound/soc/codecs/hdac_hda.c  |2 +-
 sound/soc/codecs/tas6424.c   |2 +-
 sound/soc/qcom/qdsp6/q6afe-dai.c |8 
 6 files changed, 10 insertions(+), 10 deletions(-)


[PATCH 2/3] ASoC: qdsp6: q6afe-dai: constify snd_soc_dai_ops structures

2018-10-27 Thread Julia Lawall
The snd_soc_dai_ops structures are only stored in the ops field of a
snd_soc_dai_driver structure, so make the snd_soc_dai_ops structures
const as well.

Done with the help of Coccinelle.

Signed-off-by: Julia Lawall 

---
 sound/soc/qcom/qdsp6/q6afe-dai.c |8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/sound/soc/qcom/qdsp6/q6afe-dai.c b/sound/soc/qcom/qdsp6/q6afe-dai.c
index 60ff4a2d3577..a07a4538d5cf 100644
--- a/sound/soc/qcom/qdsp6/q6afe-dai.c
+++ b/sound/soc/qcom/qdsp6/q6afe-dai.c
@@ -561,13 +561,13 @@ static const struct snd_soc_dapm_route 
q6afe_dapm_routes[] = {
{"QUAT_MI2S_TX", NULL, "Quaternary MI2S Capture"},
 };
 
-static struct snd_soc_dai_ops q6hdmi_ops = {
+static const struct snd_soc_dai_ops q6hdmi_ops = {
.prepare= q6afe_dai_prepare,
.hw_params  = q6hdmi_hw_params,
.shutdown   = q6afe_dai_shutdown,
 };
 
-static struct snd_soc_dai_ops q6i2s_ops = {
+static const struct snd_soc_dai_ops q6i2s_ops = {
.prepare= q6afe_dai_prepare,
.hw_params  = q6i2s_hw_params,
.set_fmt= q6i2s_set_fmt,
@@ -575,14 +575,14 @@ static struct snd_soc_dai_ops q6i2s_ops = {
.set_sysclk = q6afe_mi2s_set_sysclk,
 };
 
-static struct snd_soc_dai_ops q6slim_ops = {
+static const struct snd_soc_dai_ops q6slim_ops = {
.prepare= q6afe_dai_prepare,
.hw_params  = q6slim_hw_params,
.shutdown   = q6afe_dai_shutdown,
.set_channel_map = q6slim_set_channel_map,
 };
 
-static struct snd_soc_dai_ops q6tdm_ops = {
+static const struct snd_soc_dai_ops q6tdm_ops = {
.prepare= q6afe_dai_prepare,
.shutdown   = q6afe_dai_shutdown,
.set_sysclk = q6afe_mi2s_set_sysclk,



[PATCH 3/8] clk: palmas: constify clk_ops structure

2018-10-27 Thread Julia Lawall
The clk_ops structure is only stored in the ops field of clk_init_data
structures.  This field is const, so the clk_ops structure can be
const as well.

Identified and transformed using Coccinelle.

Signed-off-by: Julia Lawall 

---
 drivers/clk/clk-palmas.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/clk-palmas.c b/drivers/clk/clk-palmas.c
index e9612e7068e9..e41a3a9f7528 100644
--- a/drivers/clk/clk-palmas.c
+++ b/drivers/clk/clk-palmas.c
@@ -115,7 +115,7 @@ static int palmas_clks_is_prepared(struct clk_hw *hw)
return !!(val & cinfo->clk_desc->enable_mask);
 }
 
-static struct clk_ops palmas_clks_ops = {
+static const struct clk_ops palmas_clks_ops = {
.prepare= palmas_clks_prepare,
.unprepare  = palmas_clks_unprepare,
.is_prepared= palmas_clks_is_prepared,



[PATCH 6/8] clk: s2mps11: constify clk_ops structure

2018-10-27 Thread Julia Lawall
The clk_ops structure is only stored in the ops fields of
clk_init_data structures.  This field is const, so the clk_ops
structure can be const as well.

Identified and transformed using Coccinelle.

Signed-off-by: Julia Lawall 

---
 drivers/clk/clk-s2mps11.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/clk-s2mps11.c b/drivers/clk/clk-s2mps11.c
index 5b419b82f7ca..2ce370c804aa 100644
--- a/drivers/clk/clk-s2mps11.c
+++ b/drivers/clk/clk-s2mps11.c
@@ -71,7 +71,7 @@ static unsigned long s2mps11_clk_recalc_rate(struct clk_hw 
*hw,
return 32768;
 }
 
-static struct clk_ops s2mps11_clk_ops = {
+static const struct clk_ops s2mps11_clk_ops = {
.prepare= s2mps11_clk_prepare,
.unprepare  = s2mps11_clk_unprepare,
.is_prepared= s2mps11_clk_is_prepared,



[PATCH 5/8] clk: pxa: constify clk_ops structures

2018-10-27 Thread Julia Lawall
These clk_ops structures are only passed to a call to
clk_register_composite where the corresponding parameters
are const, so the clk_ops structure can be const as well.

Identified and transformed using Coccinelle.

Signed-off-by: Julia Lawall 

---
 drivers/clk/pxa/clk-pxa.c |4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/pxa/clk-pxa.c b/drivers/clk/pxa/clk-pxa.c
index b80dc9d5855c..42627bf8a09e 100644
--- a/drivers/clk/pxa/clk-pxa.c
+++ b/drivers/clk/pxa/clk-pxa.c
@@ -70,7 +70,7 @@ static unsigned long cken_recalc_rate(struct clk_hw *hw,
return clk_fixed_factor_ops.recalc_rate(>hw, parent_rate);
 }
 
-static struct clk_ops cken_rate_ops = {
+static const struct clk_ops cken_rate_ops = {
.recalc_rate = cken_recalc_rate,
 };
 
@@ -83,7 +83,7 @@ static u8 cken_get_parent(struct clk_hw *hw)
return pclk->is_in_low_power() ? 0 : 1;
 }
 
-static struct clk_ops cken_mux_ops = {
+static const struct clk_ops cken_mux_ops = {
.get_parent = cken_get_parent,
.set_parent = dummy_clk_set_parent,
 };



[PATCH 0/8] constify clk_ops structure

2018-10-27 Thread Julia Lawall
Declare as const clk_ops structures that are only stored in const fields or
passed to functions with const parameters.

Identified and transformed using Coccinelle.

Signed-off-by: Julia Lawall 

---

 arch/arm/mach-vexpress/spc.c |2 +-
 drivers/clk/clk-max77686.c   |2 +-
 drivers/clk/clk-palmas.c |2 +-
 drivers/clk/clk-s2mps11.c|2 +-
 drivers/clk/pistachio/clk-pll.c  |8 
 drivers/clk/pxa/clk-pxa.c|4 ++--
 drivers/gpu/drm/imx/imx-tve.c|2 +-
 drivers/gpu/drm/msm/hdmi/hdmi_phy_8996.c |2 +-
 8 files changed, 12 insertions(+), 12 deletions(-)


[PATCH 2/8] ARM: vexpress/spc: constify clk_ops structure

2018-10-27 Thread Julia Lawall
The clk_ops structure is only stored in the ops field of a
clk_init_data structure.  This field is const, so the clk_ops
structure can be const as well.

Identified and transformed using Coccinelle.

Signed-off-by: Julia Lawall 

---
 arch/arm/mach-vexpress/spc.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/mach-vexpress/spc.c b/arch/arm/mach-vexpress/spc.c
index 0f5381d13494..611f22de48cb 100644
--- a/arch/arm/mach-vexpress/spc.c
+++ b/arch/arm/mach-vexpress/spc.c
@@ -521,7 +521,7 @@ static int spc_set_rate(struct clk_hw *hw, unsigned long 
rate,
return ve_spc_set_performance(spc->cluster, rate / 1000);
 }
 
-static struct clk_ops clk_spc_ops = {
+static const struct clk_ops clk_spc_ops = {
.recalc_rate = spc_recalc_rate,
.round_rate = spc_round_rate,
.set_rate = spc_set_rate,



[PATCH 1/8] clk: max77686: constify clk_ops structure

2018-10-27 Thread Julia Lawall
The clk_ops structure is only stored in the ops field of a
clk_init_data structure.  This field is const, so the clk_ops
structure can be const as well.

Identified and transformed using Coccinelle.

Signed-off-by: Julia Lawall 

---
 drivers/clk/clk-max77686.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/clk-max77686.c b/drivers/clk/clk-max77686.c
index 02551fe4b87c..22c937644c93 100644
--- a/drivers/clk/clk-max77686.c
+++ b/drivers/clk/clk-max77686.c
@@ -137,7 +137,7 @@ static unsigned long max77686_recalc_rate(struct clk_hw *hw,
return 32768;
 }
 
-static struct clk_ops max77686_clk_ops = {
+static const struct clk_ops max77686_clk_ops = {
.prepare= max77686_clk_prepare,
.unprepare  = max77686_clk_unprepare,
.is_prepared= max77686_clk_is_prepared,



[PATCH 7/8] drm/imx: imx-tve: constify clk_ops structure

2018-10-27 Thread Julia Lawall
The clk_ops structure is only stored in the ops field of a
clk_init_data structure.  This field is const, so the clk_ops
structure can be const as well.

Identified and transformed using Coccinelle.

Signed-off-by: Julia Lawall 

---
 drivers/gpu/drm/imx/imx-tve.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/imx/imx-tve.c b/drivers/gpu/drm/imx/imx-tve.c
index 4bc3ead5c4e3..293dd5752583 100644
--- a/drivers/gpu/drm/imx/imx-tve.c
+++ b/drivers/gpu/drm/imx/imx-tve.c
@@ -434,7 +434,7 @@ static int clk_tve_di_set_rate(struct clk_hw *hw, unsigned 
long rate,
return 0;
 }
 
-static struct clk_ops clk_tve_di_ops = {
+static const struct clk_ops clk_tve_di_ops = {
.round_rate = clk_tve_di_round_rate,
.set_rate = clk_tve_di_set_rate,
.recalc_rate = clk_tve_di_recalc_rate,



[PATCH 4/8] clk: pistachio: constify clk_ops structures

2018-10-27 Thread Julia Lawall
These clk_ops structures are only stored in the ops field of a
clk_init_data structure.  This field is const, so the clk_ops
structures can be const as well.

Identified and transformed using Coccinelle.

Signed-off-by: Julia Lawall 

---
 drivers/clk/pistachio/clk-pll.c |8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/clk/pistachio/clk-pll.c b/drivers/clk/pistachio/clk-pll.c
index 7e8daab9025b..312c3580187f 100644
--- a/drivers/clk/pistachio/clk-pll.c
+++ b/drivers/clk/pistachio/clk-pll.c
@@ -298,7 +298,7 @@ static unsigned long pll_gf40lp_frac_recalc_rate(struct 
clk_hw *hw,
return rate;
 }
 
-static struct clk_ops pll_gf40lp_frac_ops = {
+static const struct clk_ops pll_gf40lp_frac_ops = {
.enable = pll_gf40lp_frac_enable,
.disable = pll_gf40lp_frac_disable,
.is_enabled = pll_gf40lp_frac_is_enabled,
@@ -307,7 +307,7 @@ static struct clk_ops pll_gf40lp_frac_ops = {
.set_rate = pll_gf40lp_frac_set_rate,
 };
 
-static struct clk_ops pll_gf40lp_frac_fixed_ops = {
+static const struct clk_ops pll_gf40lp_frac_fixed_ops = {
.enable = pll_gf40lp_frac_enable,
.disable = pll_gf40lp_frac_disable,
.is_enabled = pll_gf40lp_frac_is_enabled,
@@ -430,7 +430,7 @@ static unsigned long pll_gf40lp_laint_recalc_rate(struct 
clk_hw *hw,
return rate;
 }
 
-static struct clk_ops pll_gf40lp_laint_ops = {
+static const struct clk_ops pll_gf40lp_laint_ops = {
.enable = pll_gf40lp_laint_enable,
.disable = pll_gf40lp_laint_disable,
.is_enabled = pll_gf40lp_laint_is_enabled,
@@ -439,7 +439,7 @@ static struct clk_ops pll_gf40lp_laint_ops = {
.set_rate = pll_gf40lp_laint_set_rate,
 };
 
-static struct clk_ops pll_gf40lp_laint_fixed_ops = {
+static const struct clk_ops pll_gf40lp_laint_fixed_ops = {
.enable = pll_gf40lp_laint_enable,
.disable = pll_gf40lp_laint_disable,
.is_enabled = pll_gf40lp_laint_is_enabled,



Re: [Outreachy kernel] [RESEND PATCH 2/2] staging: vboxvideo: Use unsigned int instead bool

2018-10-26 Thread Julia Lawall
[Adding Joe Perches]

On Fri, 26 Oct 2018, Sasha Levin wrote:

> On Fri, Oct 26, 2018 at 04:04:45PM -0300, Shayenne da Luz Moura wrote:
> > This change was suggested by checkpath.pl. Use unsigned int with bitfield
> > allocate only one bit to the boolean variable.
> >
> > CHECK: Avoid using bool structure members because of possible alignment
> > issues
> >
> > Signed-off-by: Shayenne da Luz Moura 
> > ---
> > drivers/staging/vboxvideo/vbox_drv.h| 14 +++---
> > drivers/staging/vboxvideo/vboxvideo_guest.h |  2 +-
> > 2 files changed, 8 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/staging/vboxvideo/vbox_drv.h
> > b/drivers/staging/vboxvideo/vbox_drv.h
> > index 594f84272957..7d3e329a6b1c 100644
> > --- a/drivers/staging/vboxvideo/vbox_drv.h
> > +++ b/drivers/staging/vboxvideo/vbox_drv.h
> > @@ -81,7 +81,7 @@ struct vbox_private {
> > u8 __iomem *vbva_buffers;
> > struct gen_pool *guest_pool;
> > struct vbva_buf_ctx *vbva_info;
> > -   bool any_pitch;
> > +   unsigned int any_pitch:1;
> > u32 num_crtcs;
> > /** Amount of available VRAM, including space used for buffers. */
> > u32 full_vram_size;
>
> Using bitfields for booleans in these cases is less efficient than just
> using "regular" booleans for two reasons:
>
> 1. It will use the same amount of space. Due to alignment requirements,
> the compiler can't squeeze in anything into the 7 bits that are now
> "free". Each member, unless it's another bitfield, must start at a whole
> byte.
>
> 2. This is actually less efficient (slower) for the compiler to work
> with. The smallest granularity we have to access memory is 1 byte; we
> can't set individual bits directly in memory. For the original code, the
> assembly for 'vbox_private.any_pitch = true' would look something like
> this:
>
>   movl   $0x1,-0x10(%rsp)
>
> As you can see, the compiler can directly write into the variable.
> However, when we switch to using bitfields, the compiler must preserve
> the original value of the other 7 bits, so it must first read them from
> memory, manipulate the value and write it back. The assembly would
> look something like this:
>
>   movzbl -0x10(%rsp),%eax
>   or $0x1,%eax
>   mov%al,-0x10(%rsp)
>
> Which is less efficient than what was previously happening.

Maybe checkpatch could be more precise about what kind of bools should be
changed?

julia


Re: [PATCH] mm: convert totalram_pages, totalhigh_pages and managed_pages to atomic.

2018-10-23 Thread Julia Lawall



On Tue, 23 Oct 2018, Michal Hocko wrote:

> [Trimmed CC list + Julia - there is indeed no need to CC everybody maintain a
> file you are updating for the change like this]
>
> On Tue 23-10-18 10:16:51, Arun Sudhilal wrote:
> > On Mon, Oct 22, 2018 at 11:41 PM Michal Hocko  wrote:
> > >
> > > On Mon 22-10-18 22:53:22, Arun KS wrote:
> > > > Remove managed_page_count_lock spinlock and instead use atomic
> > > > variables.
> > >
> >
> > Hello Michal,
> > > I assume this has been auto-generated. If yes, it would be better to
> > > mention the script so that people can review it and regenerate for
> > > comparision. Such a large change is hard to review manually.
> >
> > Changes were made partially with script.  For totalram_pages and
> > totalhigh_pages,
> >
> > find dir -type f -exec sed -i
> > 's/totalram_pages/atomic_long_read(\_pages)/g' {} \;
> > find dir -type f -exec sed -i
> > 's/totalhigh_pages/atomic_long_read(\_pages)/g' {} \;
> >
> > For managed_pages it was mostly manual edits after using,
> > find mm/ -type f -exec sed -i
> > 's/zone->managed_pages/atomic_long_read(\>managed_pages)/g' {}
> > \;
>
> I guess we should be able to use coccinelle for this kind of change and
> reduce the amount of manual intervention to absolute minimum.

Coccinelle looks like it would be desirable, especially in case the word
zone is not always used.

Arun, please feel free to contact me if you want to try it and need help.

julia


Re: [Outreachy kernel] [PATCH] staging: vboxvideo: Removed unnecessary parentheses

2018-10-22 Thread Julia Lawall



On Mon, 22 Oct 2018, Shayenne da Luz Moura wrote:

> This patch fixes the checkpatch.pl check:
>
> vbox_main.c:119: CHECK: Unnecessary parentheses around 'rects[i].x2 <
> crtc->x'
> vbox_main.c:119: CHECK: Unnecessary parentheses around 'rects[i].y2 <
> crtc->y'

Please use the imperative, in the subject line and in the patch.  Also,
try to say something other than "fixes" about what you do, which is not
very descriptive.  Say what you actually did and why, eg remove unneeded
parentheses around the arguments of || to reduce clutter.

It is useful to mention that the change was suggested by checkpatch.  It
is not essential to copy the whole checkpatch message, or at least not all
of them.

thanks,
julia

>
> Signed-off-by: Shayenne da Luz Moura 
> ---
>  drivers/staging/vboxvideo/vbox_main.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/staging/vboxvideo/vbox_main.c 
> b/drivers/staging/vboxvideo/vbox_main.c
> index 429f6a453619..10a862674789 100644
> --- a/drivers/staging/vboxvideo/vbox_main.c
> +++ b/drivers/staging/vboxvideo/vbox_main.c
> @@ -116,10 +116,10 @@ void vbox_framebuffer_dirty_rectangles(struct 
> drm_framebuffer *fb,
>   struct vbva_cmd_hdr cmd_hdr;
>   unsigned int crtc_id = to_vbox_crtc(crtc)->crtc_id;
>
> - if ((rects[i].x1 > crtc->x + crtc->hwmode.hdisplay) ||
> - (rects[i].y1 > crtc->y + crtc->hwmode.vdisplay) ||
> - (rects[i].x2 < crtc->x) ||
> - (rects[i].y2 < crtc->y))
> + if (rects[i].x1 > crtc->x + crtc->hwmode.hdisplay ||
> + rects[i].y1 > crtc->y + crtc->hwmode.vdisplay ||
> + rects[i].x2 < crtc->x ||
> + rects[i].y2 < crtc->y)
>   continue;
>
>   cmd_hdr.x = (s16)rects[i].x1;
> --
> 2.19.1
>
> --
> 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/2018103126.smnsopiu4cavq6zh%40smtp.gmail.com.
> For more options, visit https://groups.google.com/d/optout.
>


Re: [PATCH 2/2] tty: serial: add driver for the SiFive UART (fwd)

2018-10-19 Thread Julia Lawall
Hello,

It looks like an unlock is needed before the return on line 562.

julia

-- Forwarded message --
Date: Fri, 19 Oct 2018 18:12:04 +0800
From: kbuild test robot 
To: kbu...@01.org
Cc: Julia Lawall 
Subject: Re: [PATCH 2/2] tty: serial: add driver for the SiFive UART

CC: kbuild-...@01.org
In-Reply-To: <20181018234352.26788-3-paul.walms...@sifive.com>
References: <20181018234352.26788-3-paul.walms...@sifive.com>
TO: Paul Walmsley 
CC: linux-ser...@vger.kernel.org
CC: Paul Walmsley , Greg Kroah-Hartman 
, Jiri Slaby , Palmer Dabbelt 
, Wesley Terpstra , 
linux-ri...@lists.infradead.org, linux-kernel@vger.kernel.org, Paul Walmsley 


Hi Paul,

I love your patch! Perhaps something to improve:

[auto build test WARNING on tty/tty-testing]
[also build test WARNING on v4.19-rc8 next-20181019]
[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/Paul-Walmsley/dt-bindings-serial-add-documentation-for-the-SiFive-UART-driver/20181019-165529
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty.git 
tty-testing
:: branch date: 77 minutes ago
:: commit date: 77 minutes ago

>> drivers/tty/serial/sifive.c:562:2-8: preceding lock on line 558

# 
https://github.com/0day-ci/linux/commit/15d0af7ab79ba53f984613f23e668cade495babb
git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout 15d0af7ab79ba53f984613f23e668cade495babb
vim +562 drivers/tty/serial/sifive.c

15d0af7a Paul Walmsley 2018-10-18  552
15d0af7a Paul Walmsley 2018-10-18  553  static irqreturn_t 
sifive_serial_irq(int irq, void *dev_id)
15d0af7a Paul Walmsley 2018-10-18  554  {
15d0af7a Paul Walmsley 2018-10-18  555  struct sifive_serial_port *ssp 
= dev_id;
15d0af7a Paul Walmsley 2018-10-18  556  u32 ip;
15d0af7a Paul Walmsley 2018-10-18  557
15d0af7a Paul Walmsley 2018-10-18 @558  spin_lock(>port.lock);
15d0af7a Paul Walmsley 2018-10-18  559
15d0af7a Paul Walmsley 2018-10-18  560  ip = __ssp_readl(ssp, 
SIFIVE_SERIAL_IP_OFFS);
15d0af7a Paul Walmsley 2018-10-18  561  if (!ip)
15d0af7a Paul Walmsley 2018-10-18 @562  return IRQ_NONE;
15d0af7a Paul Walmsley 2018-10-18  563
15d0af7a Paul Walmsley 2018-10-18  564  if (ip & 
SIFIVE_SERIAL_IP_RXWM_MASK)
15d0af7a Paul Walmsley 2018-10-18  565  
__ssp_receive_chars(ssp);
15d0af7a Paul Walmsley 2018-10-18  566  if (ip & 
SIFIVE_SERIAL_IP_TXWM_MASK)
15d0af7a Paul Walmsley 2018-10-18  567  
__ssp_transmit_chars(ssp);
15d0af7a Paul Walmsley 2018-10-18  568
15d0af7a Paul Walmsley 2018-10-18  569  spin_unlock(>port.lock);
15d0af7a Paul Walmsley 2018-10-18  570
15d0af7a Paul Walmsley 2018-10-18  571  return IRQ_HANDLED;
15d0af7a Paul Walmsley 2018-10-18  572  }
15d0af7a Paul Walmsley 2018-10-18  573

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


Re: [PATCH 1/2] eeprom: eeprom_93xx46: use resource management (fwd)

2018-09-10 Thread Julia Lawall
Line 494 should be dropped.  The whole fail label could be dropped as
well.

julia

-- Forwarded message --
Date: Mon, 10 Sep 2018 20:27:41 +0800
From: kbuild test robot 
To: kbu...@01.org
Cc: Julia Lawall 
Subject: Re: [PATCH 1/2] eeprom: eeprom_93xx46: use resource management

In-Reply-To: <20180910074404.8041-2-b...@bgdev.pl>
References: <20180910074404.8041-2-b...@bgdev.pl>

Hi Bartosz,

I love your patch! Perhaps something to improve:

[auto build test WARNING on char-misc/char-misc-testing]
[also build test WARNING on v4.19-rc2 next-20180906]
[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/Bartosz-Golaszewski/eeprom-eeprom_93xx46-use-resource-management/20180910-192124
:: branch date: 66 minutes ago
:: commit date: 66 minutes ago

>> drivers/misc/eeprom/eeprom_93xx46.c:494:1-6: WARNING: invalid free of devm_ 
>> allocated data

# 
https://github.com/0day-ci/linux/commit/d51b089674382bcc4f773d88ecd7c76d5ce472f7
git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout d51b089674382bcc4f773d88ecd7c76d5ce472f7
vim +494 drivers/misc/eeprom/eeprom_93xx46.c

c074abe02 Cory Tusar  2016-01-06  423
80c8ae289 Bill Pemberton  2012-11-19  424  static int 
eeprom_93xx46_probe(struct spi_device *spi)
06b4501e8 Anatolij Gustschin  2011-07-25  425  {
06b4501e8 Anatolij Gustschin  2011-07-25  426   struct 
eeprom_93xx46_platform_data *pd;
06b4501e8 Anatolij Gustschin  2011-07-25  427   struct eeprom_93xx46_dev *edev;
06b4501e8 Anatolij Gustschin  2011-07-25  428   int err;
06b4501e8 Anatolij Gustschin  2011-07-25  429
c074abe02 Cory Tusar  2016-01-06  430   if (spi->dev.of_node) {
c074abe02 Cory Tusar  2016-01-06  431   err = 
eeprom_93xx46_probe_dt(spi);
c074abe02 Cory Tusar  2016-01-06  432   if (err < 0)
c074abe02 Cory Tusar  2016-01-06  433   return err;
c074abe02 Cory Tusar  2016-01-06  434   }
c074abe02 Cory Tusar  2016-01-06  435
06b4501e8 Anatolij Gustschin  2011-07-25  436   pd = spi->dev.platform_data;
06b4501e8 Anatolij Gustschin  2011-07-25  437   if (!pd) {
06b4501e8 Anatolij Gustschin  2011-07-25  438   dev_err(>dev, 
"missing platform data\n");
06b4501e8 Anatolij Gustschin  2011-07-25  439   return -ENODEV;
06b4501e8 Anatolij Gustschin  2011-07-25  440   }
06b4501e8 Anatolij Gustschin  2011-07-25  441
d51b08967 Bartosz Golaszewski 2018-09-10  442   edev = devm_kzalloc(>dev, 
sizeof(*edev), GFP_KERNEL);
06b4501e8 Anatolij Gustschin  2011-07-25  443   if (!edev)
06b4501e8 Anatolij Gustschin  2011-07-25  444   return -ENOMEM;
06b4501e8 Anatolij Gustschin  2011-07-25  445
06b4501e8 Anatolij Gustschin  2011-07-25  446   if (pd->flags & EE_ADDR8)
06b4501e8 Anatolij Gustschin  2011-07-25  447   edev->addrlen = 7;
06b4501e8 Anatolij Gustschin  2011-07-25  448   else if (pd->flags & EE_ADDR16)
06b4501e8 Anatolij Gustschin  2011-07-25  449   edev->addrlen = 6;
06b4501e8 Anatolij Gustschin  2011-07-25  450   else {
06b4501e8 Anatolij Gustschin  2011-07-25  451   dev_err(>dev, 
"unspecified address type\n");
06b4501e8 Anatolij Gustschin  2011-07-25  452   err = -EINVAL;
06b4501e8 Anatolij Gustschin  2011-07-25  453   goto fail;
06b4501e8 Anatolij Gustschin  2011-07-25  454   }
06b4501e8 Anatolij Gustschin  2011-07-25  455
06b4501e8 Anatolij Gustschin  2011-07-25  456   mutex_init(>lock);
06b4501e8 Anatolij Gustschin  2011-07-25  457
dd69a18ae Mark Brown  2016-04-20  458   edev->spi = spi;
06b4501e8 Anatolij Gustschin  2011-07-25  459   edev->pdata = pd;
06b4501e8 Anatolij Gustschin  2011-07-25  460
1c4b6e2c7 Andrew Lunn 2016-02-26  461   edev->size = 128;
1c4b6e2c7 Andrew Lunn 2016-02-26  462   edev->nvmem_config.name = 
dev_name(>dev);
1c4b6e2c7 Andrew Lunn 2016-02-26  463   edev->nvmem_config.dev = 
>dev;
1c4b6e2c7 Andrew Lunn 2016-02-26  464   edev->nvmem_config.read_only = 
pd->flags & EE_READONLY;
1c4b6e2c7 Andrew Lunn 2016-02-26  465   edev->nvmem_config.root_only = 
true;
1c4b6e2c7 Andrew Lunn 2016-02-26  466   edev->nvmem_config.owner = 
THIS_MODULE;
1c4b6e2c7 Andrew Lunn 2016-02-26  467   edev->nvmem_config.compat = 
true;
1c4b6e2c7 Andrew Lunn 2016-02-26  468   edev->nvmem_config.base_dev = 
>dev;
a8ab316ab Srinivas Kandagatla 2016-04-24  469   edev->nvmem_config.reg_read = 
eeprom_93xx46_read;
a8ab316ab Srinivas Kandagatla 2016-04-24  470   edev->nvmem_config.reg_write = 
eeprom_93xx46_write;
a8ab316ab Srinivas Kandagatla 2016-04-24  471   edev->nvmem_config.priv = edev;
a8ab316ab Srinivas Kandagatla 2016-04-24  472   edev->nvmem_config.stride = 4;
a8ab316ab Srinivas 

drivers/gpu/drm/amd/display/dc/core/dc_resource.c:352:1-14: code aligned with following code on line 354 (fwd)

2018-09-06 Thread Julia Lawall
It looks like line 352 needs to be indented more.

julia

-- Forwarded message --
Date: Fri, 7 Sep 2018 05:47:25 +0800
From: kbuild test robot 
To: kbu...@01.org
Cc: Julia Lawall 
Subject: drivers/gpu/drm/amd/display/dc/core/dc_resource.c:352:1-14: code
aligned with following code on line 354

CC: kbuild-...@01.org
CC: linux-kernel@vger.kernel.org
TO: Mikita Lipski 
CC: Alex Deucher 
CC: Hersen Wu 

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 
master
head:   db44bf4b4768a0664d3c9d9000ecb747de31ded8
commit: 3e27e10e2ecee0d3a0083f8ae76354ac9c6ad15c drm/amd/display: Don't share 
clk source between DP and HDMI
date:   4 weeks ago
:: branch date: 5 hours ago
:: commit date: 4 weeks ago

>> drivers/gpu/drm/amd/display/dc/core/dc_resource.c:352:1-14: code aligned 
>> with following code on line 354

# 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=3e27e10e2ecee0d3a0083f8ae76354ac9c6ad15c
git remote add linus 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
git remote update linus
git checkout 3e27e10e2ecee0d3a0083f8ae76354ac9c6ad15c
vim +352 drivers/gpu/drm/amd/display/dc/core/dc_resource.c

4562236b Harry Wentland   2017-09-12  310
4562236b Harry Wentland   2017-09-12  311  bool 
resource_are_streams_timing_synchronizable(
0971c40e Harry Wentland   2017-07-27  312   struct dc_stream_state *stream1,
0971c40e Harry Wentland   2017-07-27  313   struct dc_stream_state *stream2)
4562236b Harry Wentland   2017-09-12  314  {
4fa086b9 Leo (Sunpeng  Li 2017-07-25  315)  if (stream1->timing.h_total != 
stream2->timing.h_total)
4562236b Harry Wentland   2017-09-12  316   return false;
4562236b Harry Wentland   2017-09-12  317
4fa086b9 Leo (Sunpeng  Li 2017-07-25  318)  if (stream1->timing.v_total != 
stream2->timing.v_total)
4562236b Harry Wentland   2017-09-12  319   return false;
4562236b Harry Wentland   2017-09-12  320
4fa086b9 Leo (Sunpeng  Li 2017-07-25  321)  if 
(stream1->timing.h_addressable
4fa086b9 Leo (Sunpeng  Li 2017-07-25  322)  != 
stream2->timing.h_addressable)
4562236b Harry Wentland   2017-09-12  323   return false;
4562236b Harry Wentland   2017-09-12  324
4fa086b9 Leo (Sunpeng  Li 2017-07-25  325)  if 
(stream1->timing.v_addressable
4fa086b9 Leo (Sunpeng  Li 2017-07-25  326)  != 
stream2->timing.v_addressable)
4562236b Harry Wentland   2017-09-12  327   return false;
4562236b Harry Wentland   2017-09-12  328
4fa086b9 Leo (Sunpeng  Li 2017-07-25  329)  if (stream1->timing.pix_clk_khz
4fa086b9 Leo (Sunpeng  Li 2017-07-25  330)  != 
stream2->timing.pix_clk_khz)
4562236b Harry Wentland   2017-09-12  331   return false;
4562236b Harry Wentland   2017-09-12  332
3e27e10e Mikita Lipski2018-07-12  333   if (stream1->clamping.c_depth 
!= stream2->clamping.c_depth)
3e27e10e Mikita Lipski2018-07-12  334   return false;
3e27e10e Mikita Lipski2018-07-12  335
4562236b Harry Wentland   2017-09-12  336   if (stream1->phy_pix_clk != 
stream2->phy_pix_clk
7e2fe319 Charlene Liu 2017-03-17  337   && 
(!dc_is_dp_signal(stream1->signal)
7e2fe319 Charlene Liu 2017-03-17  338   || 
!dc_is_dp_signal(stream2->signal)))
4562236b Harry Wentland   2017-09-12  339   return false;
4562236b Harry Wentland   2017-09-12  340
4562236b Harry Wentland   2017-09-12  341   return true;
4562236b Harry Wentland   2017-09-12  342  }
3e27e10e Mikita Lipski2018-07-12  343  static bool is_dp_and_hdmi_sharable(
3e27e10e Mikita Lipski2018-07-12  344   struct dc_stream_state 
*stream1,
3e27e10e Mikita Lipski2018-07-12  345   struct dc_stream_state 
*stream2)
3e27e10e Mikita Lipski2018-07-12  346  {
3e27e10e Mikita Lipski2018-07-12  347   if 
(stream1->ctx->dc->caps.disable_dp_clk_share)
3e27e10e Mikita Lipski2018-07-12  348   return false;
3e27e10e Mikita Lipski2018-07-12  349
3e27e10e Mikita Lipski2018-07-12  350   if (stream1->clamping.c_depth 
!= COLOR_DEPTH_888 ||
3e27e10e Mikita Lipski2018-07-12  351   stream2->clamping.c_depth 
!= COLOR_DEPTH_888)
3e27e10e Mikita Lipski2018-07-12 @352   return false;
3e27e10e Mikita Lipski2018-07-12  353
3e27e10e Mikita Lipski2018-07-12 @354   return true;
3e27e10e Mikita Lipski2018-07-12  355

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


net/sctp/socket.c:2681:6-11: ERROR: invalid reference to the index variable of the iterator on line 2661 (fwd)

2018-08-29 Thread Julia Lawall
Hello,

Are the ifs starting on lines 2655 and 2680 mutually exclusive?  If so,
perhaps add an else.  If not, and if the trans on line 2681 can come from
the trans initialized by the loop on line 2661, then there is a problem.

julia

-- Forwarded message --
Date: Wed, 29 Aug 2018 22:02:39 +0800
From: kbuild test robot 
To: kbu...@01.org
Cc: Julia Lawall 
Subject: net/sctp/socket.c:2681:6-11: ERROR: invalid reference to the index
variable of the iterator on line 2661

CC: kbuild-...@01.org
CC: linux-kernel@vger.kernel.org
TO: Xin Long 

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 
master
head:   3f16503b7d2274ac8cbab11163047ac0b4c66cfe
commit: 0b0dce7a36fb9f1a9dd8245ea82d3a268c6943fe sctp: add spp_ipv6_flowlabel 
and spp_dscp for sctp_paddrparams
date:   8 weeks ago
:: branch date: 15 hours ago
:: commit date: 8 weeks ago

>> net/sctp/socket.c:2681:6-11: ERROR: invalid reference to the index variable 
>> of the iterator on line 2661

# 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=0b0dce7a36fb9f1a9dd8245ea82d3a268c6943fe
git remote add linus 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
git remote update linus
git checkout 0b0dce7a36fb9f1a9dd8245ea82d3a268c6943fe
vim +2681 net/sctp/socket.c

^1da177e4 Linus Torvalds  2005-04-16  2378
^1da177e4 Linus Torvalds  2005-04-16  2379  /* 7.1.13 Peer Address 
Parameters (SCTP_PEER_ADDR_PARAMS)
^1da177e4 Linus Torvalds  2005-04-16  2380   *
^1da177e4 Linus Torvalds  2005-04-16  2381   * Applications can enable 
or disable heartbeats for any peer address of
^1da177e4 Linus Torvalds  2005-04-16  2382   * an association, modify 
an address's heartbeat interval, force a
^1da177e4 Linus Torvalds  2005-04-16  2383   * heartbeat to be sent 
immediately, and adjust the address's maximum
^1da177e4 Linus Torvalds  2005-04-16  2384   * number of 
retransmissions sent before an address is considered
^1da177e4 Linus Torvalds  2005-04-16  2385   * unreachable.  The 
following structure is used to access and modify an
^1da177e4 Linus Torvalds  2005-04-16  2386   * address's parameters:
^1da177e4 Linus Torvalds  2005-04-16  2387   *
^1da177e4 Linus Torvalds  2005-04-16  2388   *  struct sctp_paddrparams 
{
^1da177e4 Linus Torvalds  2005-04-16  2389   * sctp_assoc_t 
   spp_assoc_id;
^1da177e4 Linus Torvalds  2005-04-16  2390   * struct 
sockaddr_storage spp_address;
^1da177e4 Linus Torvalds  2005-04-16  2391   * uint32_t 
   spp_hbinterval;
^1da177e4 Linus Torvalds  2005-04-16  2392   * uint16_t 
   spp_pathmaxrxt;
52ccb8e90 Frank Filz  2005-12-22  2393   * uint32_t 
   spp_pathmtu;
52ccb8e90 Frank Filz  2005-12-22  2394   * uint32_t 
   spp_sackdelay;
52ccb8e90 Frank Filz  2005-12-22  2395   * uint32_t 
   spp_flags;
0b0dce7a3 Xin Long2018-07-02  2396   * uint32_t 
   spp_ipv6_flowlabel;
0b0dce7a3 Xin Long2018-07-02  2397   * uint8_t  
   spp_dscp;
^1da177e4 Linus Torvalds  2005-04-16  2398   * };
^1da177e4 Linus Torvalds  2005-04-16  2399   *
52ccb8e90 Frank Filz  2005-12-22  2400   *   spp_assoc_id- 
(one-to-many style socket) This is filled in the
52ccb8e90 Frank Filz  2005-12-22  2401   * 
application, and identifies the association for
52ccb8e90 Frank Filz  2005-12-22  2402   * this 
query.
^1da177e4 Linus Torvalds  2005-04-16  2403   *   spp_address - This 
specifies which address is of interest.
^1da177e4 Linus Torvalds  2005-04-16  2404   *   spp_hbinterval  - This 
contains the value of the heartbeat interval,
52ccb8e90 Frank Filz  2005-12-22  2405   * in 
milliseconds.  If a  value of zero
52ccb8e90 Frank Filz  2005-12-22  2406   * is 
present in this field then no changes are to
52ccb8e90 Frank Filz  2005-12-22  2407   * be 
made to this parameter.
^1da177e4 Linus Torvalds  2005-04-16  2408   *   spp_pathmaxrxt  - This 
contains the maximum number of
^1da177e4 Linus Torvalds  2005-04-16  2409   * 
retransmissions before this address shall be
52ccb8e90 Frank Filz  2005-12-22  2410   * 
considered unreachable. If a  value of zero
52ccb8e90 Frank Filz  2005-12-22  2411   * is 
present in this field then no changes are to
52ccb8e90 Frank Filz  2005-12-22  2412   * be 
made to this parameter.
52ccb8e90 Frank Filz  2005-12-22  2413   *   spp_pathmtu - When 
Path MTU discovery is disabl

Re: [PATCH] net: sched: Fix memory exposure from short TCA_U32_SEL

2018-08-26 Thread Julia Lawall



On Mon, 27 Aug 2018, Al Viro wrote:

> On Sun, Aug 26, 2018 at 11:35:17PM -0400, Julia Lawall wrote:
>
> > * x = \(kmalloc\|kzalloc\|devm_kmalloc\|devm_kzalloc\)(...)
>
> I can name several you've missed right off the top of my head -
> vmalloc, kvmalloc, kmem_cache_alloc, kmem_cache_zalloc, variants
> with _trace slapped on, and that is not to mention the things like
> get_free_page or

OK, maybe for a given type the set of functions would be smaller.

>
> void *my_k3wl_alloc(u64 n) // 'cause all artificial limits suck, that's why
> {
>   lots and lots of home-grown stats collection
>   some tracepoints thrown in just for fun
>   return kmalloc(n);
> }
>
> (and no, I'm not implying that net/sched folks had done anything of that
> sort; I have seen that and worse in drivers, though)
>
> > The * at the beginning of the line means to highlight what you are looking
> > for, which is done by making a diff in which the highlighted line
> > appears to be removed.
>
> Umm...  Does that cover return, BTW?  Or something like
>   T *barf;
>   extern void foo(T *p);
>   foo(kmalloc(sizeof(*barf)));

It only covers the pattern that is shown, ie an assignment.  For this,
another pattern would be needed.  It would be necessary to match first the
call that one is concerned with and then go find the function definition
or prototype to find the type of the associated parameter.  It is possible
to count the offset of the kmalloc call in the argument list and then get
the type at the corresponding offset in the parameter list of the function
declaration or prototype.

>
>
> > The limitation is the ability to figure out the type of x.  If it is a
> > local variable, Coccinelle should have no problem.  If it is a structure
> > field, it may be necessary to provide command line arguments like
> >
> > --all-includes --include-headers-for-types
> >
> > --all-includes means to try to find all include files that are mentioned
> > in the .c file.  The next stronger option is --recursive includes, which
> > means include what all of the mentioned files include as well,
> > recursively.  This tends to cause a major performance hit, because a lot
> > of code is being parsed.  --include-headers-for-types heals a bit with
> > that, as it only considers the header files when computing type
> > information, and now when applying the rules.
> >
> > With respect to ifdefs around variable declarations and structure field
> > declaration, in these cases Coccinelle considers that it cannot make the
> > ifdef have an if-like control flow, and so if considers the #ifdef, #else
> > and #endif to be comments.  Thus it takes into account only the last type
> > provided for a given variable.
>
> [snip]
>
> What about several variants of structure definition?  Because ifdefs around
> includes do occur in the wild...

Such ifdefs would be ignored completely.  I suspect that only the last
definition of the structure would be taken into account.

julia


Re: [PATCH] net: sched: Fix memory exposure from short TCA_U32_SEL

2018-08-26 Thread Julia Lawall



On Mon, 27 Aug 2018, Al Viro wrote:

> On Sun, Aug 26, 2018 at 10:00:46PM -0400, Julia Lawall wrote:
> >
> >
> > On Sun, 26 Aug 2018, Al Viro wrote:
> >
> > > On Sun, Aug 26, 2018 at 03:26:54PM -0700, Joe Perches wrote:
> > > > On Sun, 2018-08-26 at 22:24 +0100, Al Viro wrote:
> > > > > On Sun, Aug 26, 2018 at 11:57:57AM -0700, Joe Perches wrote:
> > > > >
> > > > > > > That, BTW, is why I hate the use of sizeof(*p) in kmalloc, etc.
> > > > > > > arguments.  typeof is even worse in that respect.
> > > > > >
> > > > > > True.  Semantic searches via tools like coccinelle could help here
> > > > > > but those searches are quite a bit slower than straightforward 
> > > > > > greps.
> > > > >
> > > > > Those searches are .config-sensitive as well, which can be much more
> > > > > unpleasant than being slow...
> > > >
> > > > Are they?  Julia?
> > >
> > > They work pretty much on preprocessor output level; if something it 
> > > ifdef'ed
> > > out on given config, it won't be seen...
> >
> > Coccinelle doesn't care what is ifdef'd out.  It only misses the things it
> > can't parse.  Very strange ifdefs could indeed cause that, but it should
> > be a minor problem.
>
> OK, but... what does it do when it sees two definitions of a structure
> in different branches of #if/#else/#endif?  I think I'm confused about
> what it can and cannot do; to restate the original problem:
>   * we need to find all places where instances of given type
> are created.  Assume it never is a member of struct/union/array and
> no static or auto duration instances exist - everything is dynamically
> allocated somewhere.
>
> Can coccinelle do that and if it can, what are the limitations?

Looking at the thread, it seems that what you are asking for is something
like:

@@
struct tcf_proto *x;
@@

* x = \(kmalloc\|kzalloc\|devm_kmalloc\|devm_kzalloc\)(...)

The * at the beginning of the line means to highlight what you are looking
for, which is done by making a diff in which the highlighted line
appears to be removed.

The limitation is the ability to figure out the type of x.  If it is a
local variable, Coccinelle should have no problem.  If it is a structure
field, it may be necessary to provide command line arguments like

--all-includes --include-headers-for-types

--all-includes means to try to find all include files that are mentioned
in the .c file.  The next stronger option is --recursive includes, which
means include what all of the mentioned files include as well,
recursively.  This tends to cause a major performance hit, because a lot
of code is being parsed.  --include-headers-for-types heals a bit with
that, as it only considers the header files when computing type
information, and now when applying the rules.

With respect to ifdefs around variable declarations and structure field
declaration, in these cases Coccinelle considers that it cannot make the
ifdef have an if-like control flow, and so if considers the #ifdef, #else
and #endif to be comments.  Thus it takes into account only the last type
provided for a given variable.  For example, in the following:

int main() {
#ifdef X
  int x;
#else
  char xx;
#endif

  return x;
}

int main2() {
#ifdef X
  char x;
#else
  int x;
#endif

  return x;
}

x is considered to have type int in both returns.  But if xx is replaced
by x in the definition of main, then x at the point of the return will
have type char.

Around a statement, such as x = kmalloc(...), it should be able to
consider that both branches of an ifdef are possible.

But there are no absolute guarantees.  If there is any problem in parsing
a line of some function, the whole function will be ignores.

julia


Re: [PATCH] net: sched: Fix memory exposure from short TCA_U32_SEL

2018-08-26 Thread Julia Lawall



On Sun, 26 Aug 2018, Al Viro wrote:

> On Sun, Aug 26, 2018 at 03:26:54PM -0700, Joe Perches wrote:
> > On Sun, 2018-08-26 at 22:24 +0100, Al Viro wrote:
> > > On Sun, Aug 26, 2018 at 11:57:57AM -0700, Joe Perches wrote:
> > >
> > > > > That, BTW, is why I hate the use of sizeof(*p) in kmalloc, etc.
> > > > > arguments.  typeof is even worse in that respect.
> > > >
> > > > True.  Semantic searches via tools like coccinelle could help here
> > > > but those searches are quite a bit slower than straightforward greps.
> > >
> > > Those searches are .config-sensitive as well, which can be much more
> > > unpleasant than being slow...
> >
> > Are they?  Julia?
>
> They work pretty much on preprocessor output level; if something it ifdef'ed
> out on given config, it won't be seen...

Coccinelle doesn't care what is ifdef'd out.  It only misses the things it
can't parse.  Very strange ifdefs could indeed cause that, but it should
be a minor problem.

julia


Re: [PATCH] net: sched: Fix memory exposure from short TCA_U32_SEL

2018-08-26 Thread Julia Lawall



On Sun, 26 Aug 2018, Joe Perches wrote:

> On Sun, 2018-08-26 at 22:24 +0100, Al Viro wrote:
> > On Sun, Aug 26, 2018 at 11:57:57AM -0700, Joe Perches wrote:
> >
> > > > That, BTW, is why I hate the use of sizeof(*p) in kmalloc, etc.
> > > > arguments.  typeof is even worse in that respect.
> > >
> > > True.  Semantic searches via tools like coccinelle could help here
> > > but those searches are quite a bit slower than straightforward greps.
> >
> > Those searches are .config-sensitive as well, which can be much more
> > unpleasant than being slow...
>
> Are they?  Julia?

I don't completely understand the question.  Coccinelle doens't know
anything about the configuration.

julia


Re: [PATCH] Coccinelle: remove pci_alloc_consistent semantic to dectect in zalloc-simple.cocci

2018-08-18 Thread Julia Lawall



On Sat, 18 Aug 2018, zhong jiang wrote:

> On 2018/8/18 20:52, Himanshu Jha wrote:
> > On Sat, Aug 18, 2018 at 08:01:40PM +0800, zhong jiang wrote:
> >> Because pci_alloc_consistent has been deprecated. We prefer to use
> >> dam_alloc_coherent directly. Therefore, we should remove 
> >> pci_alloc_consistent
> >   ^^^ typo "dma"
> >
> > Also, typo in the patch subject "dectect" -> "detect"
>  Thanks,  will fix it in v2.
> > Otherwise,
> >
> > Acked-by: Himanshu Jha 
> >
> > Thanks for cleaning up, we still have few more *_alloc/memset
> > in the mainline, especially in the scsi subsystem, even after
> > I cleaned up with two long patch series in the past.
> >
>I post same patches like *_alloc/memset in the scsi subsystem. 
> Unfortunately,
>Maintainer maybe do not care about the change.  therefore,  None of my 
> patches
>   are received and rarely feedback.

Maybe they will be picked up later.

julia


Re: [PATCH] Coccinelle: remove pci_alloc_consistent semantic to dectect in zalloc-simple.cocci

2018-08-18 Thread Julia Lawall



On Sat, 18 Aug 2018, zhong jiang wrote:

> Because pci_alloc_consistent has been deprecated. We prefer to use
> dam_alloc_coherent directly. Therefore, we should remove pci_alloc_consistent
> to increase the confidence.
>
> Signed-off-by: zhong jiang 

Thanks for the suggestion.

Acked-by: Julia Lawall 

> ---
>  scripts/coccinelle/api/alloc/zalloc-simple.cocci | 41 
> +---
>  1 file changed, 1 insertion(+), 40 deletions(-)
>
> diff --git a/scripts/coccinelle/api/alloc/zalloc-simple.cocci 
> b/scripts/coccinelle/api/alloc/zalloc-simple.cocci
> index 92b2091..d819275 100644
> --- a/scripts/coccinelle/api/alloc/zalloc-simple.cocci
> +++ b/scripts/coccinelle/api/alloc/zalloc-simple.cocci
> @@ -35,8 +35,7 @@ statement S;
>
>  * x = (T)\(kmalloc(E1, ...)\|vmalloc(E1)\|dma_alloc_coherent(...,E1,...)\|
>kmalloc_node(E1, ...)\|kmem_cache_alloc(...)\|kmem_alloc(E1, ...)\|
> -  devm_kmalloc(...,E1,...)\|kvmalloc(E1, 
> ...)\|pci_alloc_consistent(...,E1,...)\|
> -  kvmalloc_node(E1,...)\);
> +  devm_kmalloc(...,E1,...)\|kvmalloc(E1, ...)\|kvmalloc_node(E1,...)\);
>if ((x==NULL) || ...) S
>  * memset((T2)x,0,E1);
>
> @@ -124,15 +123,6 @@ statement S;
>  - x = (T)kvmalloc(E1,E2);
>  + x = (T)kvzalloc(E1,E2);
>  |
> -- x = pci_alloc_consistent(E2,E1,E3);
> -+ x = pci_zalloc_consistent(E2,E1,E3);
> -|
> -- x = (T *)pci_alloc_consistent(E2,E1,E3);
> -+ x = pci_zalloc_consistent(E2,E1,E3);
> -|
> -- x = (T)pci_alloc_consistent(E2,E1,E3);
> -+ x = (T)pci_zalloc_consistent(E2,E1,E3);
> -|
>  - x = kvmalloc_node(E1,E2,E3);
>  + x = kvzalloc_node(E1,E2,E3);
>  |
> @@ -389,35 +379,6 @@ msg="WARNING: kvzalloc should be used for %s, instead of 
> kvmalloc/memset" % (x)
>  coccilib.report.print_report(p[0], msg)
>
>  //-
> -@r8 depends on org || report@
> -type T, T2;
> -expression x;
> -expression E1,E2,E3;
> -statement S;
> -position p;
> -@@
> -
> - x = (T)pci_alloc_consistent@p(E2,E1,E3);
> - if ((x==NULL) || ...) S
> - memset((T2)x,0,E1);
> -
> -@script:python depends on org@
> -p << r8.p;
> -x << r8.x;
> -@@
> -
> -msg="%s" % (x)
> -msg_safe=msg.replace("[","@(").replace("]",")")
> -coccilib.org.print_todo(p[0], msg_safe)
> -
> -@script:python depends on report@
> -p << r8.p;
> -x << r8.x;
> -@@
> -
> -msg="WARNING: pci_zalloc_consistent should be used for %s, instead of 
> pci_alloc_consistent/memset" % (x)
> -coccilib.report.print_report(p[0], msg)
> -//-
>  @r9 depends on org || report@
>  type T, T2;
>  expression x;
> --
> 1.7.12.4
>
>


Re: [PATCH] fix ifnullfree.cocci warnings

2018-08-14 Thread Julia Lawall



On Tue, 14 Aug 2018, Mark Brown wrote:

> On Tue, Aug 14, 2018 at 12:57:57PM +0200, Julia Lawall wrote:
> > From: kbuild test robot 
> >
> >  NULL check before some freeing functions is not needed.
> >
> >  Based on checkpatch warning
> >  "kfree(NULL) is safe this check is probably not required"
> >  and kfreeaddr.cocci by Julia Lawall.
> >
> > Generated by: scripts/coccinelle/free/ifnullfree.cocci
> >
> > Fixes: 0099cc17a399 ("ASoC:topology:avoid error log and oops during 
> > topology free.")
>
> This doesn't apply against current code, please check and resend (the
> above commit isn't in my tree:
>
> $ git show 0099cc17a399
> fatal: ambiguous argument '0099cc17a399': unknown revision or path not in the 
> working tree.

Sorry, I didn't keep the tree information.

julia


[PATCH] fix ifnullfree.cocci warnings

2018-08-14 Thread Julia Lawall
From: kbuild test robot 

 NULL check before some freeing functions is not needed.

 Based on checkpatch warning
 "kfree(NULL) is safe this check is probably not required"
 and kfreeaddr.cocci by Julia Lawall.

Generated by: scripts/coccinelle/free/ifnullfree.cocci

Fixes: 0099cc17a399 ("ASoC:topology:avoid error log and oops during topology 
free.")
Signed-off-by: kbuild test robot 
Signed-off-by: Julia Lawall 
---

Feel free to ignore this if the null test is useful in some way.

 topology.c |3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

--- a/sound/soc/sof/topology.c
+++ b/sound/soc/sof/topology.c
@@ -2165,8 +2165,7 @@ void snd_sof_free_topology(struct snd_so
/* free sroute and its private data */
kfree(sroute->route->source);
kfree(sroute->route->sink);
-   if (sroute->route->control)
-   kfree(sroute->route->control);
+   kfree(sroute->route->control);
kfree(sroute->route);
kfree(sroute->private);
kfree(sroute);


Re: [PATCH v2] coccicheck: return proper error code on fail

2018-08-10 Thread Julia Lawall



On Fri, 10 Aug 2018, efre...@linux.com wrote:

> From: Denis Efremov 
>
> If coccicheck fails, it should return an error code distinct from zero
> to signal about an internal problem. Current code instead of exiting with
> the tool's error code returns the error code of 'echo "coccicheck failed"'
> which is almost always equals to zero, thus failing the original intention
> of alerting about a problem. This patch fixes the code.
>
> Found by Linux Driver Verification project (linuxtesting.org).
>
> Signed-off-by: Denis Efremov 

OK, I get it now.  Thanks.

Acked-by: Julia Lawall 

> ---
>  scripts/coccicheck | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/scripts/coccicheck b/scripts/coccicheck
> index 9fedca611b7f..e04d328210ac 100755
> --- a/scripts/coccicheck
> +++ b/scripts/coccicheck
> @@ -128,9 +128,10 @@ run_cmd_parmap() {
>   fi
>   echo $@ >>$DEBUG_FILE
>   $@ 2>>$DEBUG_FILE
> - if [[ $? -ne 0 ]]; then
> + err=$?
> + if [[ $err -ne 0 ]]; then
>   echo "coccicheck failed"
> - exit $?
> + exit $err
>   fi
>  }
>
> --
> 2.17.1
>
>


Re: [PATCH] coccicheck: return proper error code on check fail

2018-08-10 Thread Julia Lawall



On Fri, 10 Aug 2018, Denis Efremov wrote:

> > Do you mean that there is an error in the behavior of coccicheck or that 
> > coccicheck finds an error in the source code?
>
> An error in the source code.
>
> Here is an example of how the patch changes the behavior of 'make
> coccicheck' (my comments after the ###):
> Current behavior:
> $ make M=mymodule coccicheck
> mymodule/file1.c:97:4-14: ERROR: Assignment of bool to non-0/1 constant
> mymodule/file2.c:104:4-19: ERROR: Assignment of bool to non-0/1 constant
> mymodule/file2.c:577:1-15: code aligned with following code on line 583
> mymodule/file3.c:439:5-10: Unneeded variable: "error". Return "0" on line 449
> mymodule/file4.c:451:5-7: Unneeded variable: "rc". Return "0" on line 455
> mymodule/file5.c:433:5-8: Unneeded variable: "ret". Return "0" on line 607
> mymodule/file6.c:433:5-10: Unneeded variable: "error". Return "0" on line 440
> mymodule/file7.c:774:2-3: Unneeded semicolon
> coccicheck failed ### <-- Check failed

Are you sure that this coccicheck failed has any connection to the various
messages printed above it?  Normally Coccinelle has no idea about the
semantics of messages printed using python script code.  I'm not sure what
would cause it to return an error code because a particular script was
activated.

julia

> $ echo $?
> 0 ### <-- But error code signals that everthing is OK
>
> After this patch:
> $ make M=mymodule coccicheck
> ...
> coccicheck failed
> make: *** [Makefile:1636: coccicheck] Error 2
> $ echo $?
> 2 ### <-- The patch changes error code
>
> Why does this matter?
> 1) Because it's clear from the source code that the original intention
> was to return an error code of checking command, not the "echo
> 'coccicheck failed'" command.
> 2) Automated testing systems (CI, for example) rely on the return code.
>


Re: [PATCH] coccicheck: return proper error code on check fail

2018-08-10 Thread Julia Lawall



On Fri, 10 Aug 2018, efre...@linux.com wrote:

> If coccicheck finds errors,

What do you mean by finds errors?  Do you mean that there is an error in
the behavior of coccicheck or that coccicheck finds an error in the source
code?

To put it another way, can you give an example of the kind of error you
are concerned about?

thanks,
julia

> it should return an error code
> distinct from zero. Current code instead of exiting with an
> error code of coccinelle returns error code of
> 'echo "coccicheck failed"' which is almost always equals to zero,
> thus failing original intention of alerting about errors.
> This patch fixes the problem.
>
> Found by Linux Driver Verification project (linuxtesting.org).
>
> Signed-off-by: Denis Efremov 
> ---
>  scripts/coccicheck | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/scripts/coccicheck b/scripts/coccicheck
> index 9fedca611b7f..e04d328210ac 100755
> --- a/scripts/coccicheck
> +++ b/scripts/coccicheck
> @@ -128,9 +128,10 @@ run_cmd_parmap() {
>   fi
>   echo $@ >>$DEBUG_FILE
>   $@ 2>>$DEBUG_FILE
> - if [[ $? -ne 0 ]]; then
> + err=$?
> + if [[ $err -ne 0 ]]; then
>   echo "coccicheck failed"
> - exit $?
> + exit $err
>   fi
>  }
>
> --
> 2.17.1
>
>


Re: [PATCH] rtmutex: Drop pointless static qualifier in rt_mutex_adjust_prio_chain()

2018-08-07 Thread Julia Lawall



On Tue, 7 Aug 2018, Mao Wenan wrote:

> There is no need to have the 'T *v' variable static
> since new value always be assigned before use it.

The code is:

static int prev_max;

/*
 * Print this only once. If the admin changes the limit,
 * print a new message when reaching the limit again.
 */
if (prev_max != max_lock_depth) {

So it is referenced before it is initialized.

julia


>
> Signed-off-by: Mao Wenan 
> ---
>  kernel/locking/rtmutex.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/locking/rtmutex.c b/kernel/locking/rtmutex.c
> index 2823d41..ba70db8 100644
> --- a/kernel/locking/rtmutex.c
> +++ b/kernel/locking/rtmutex.c
> @@ -472,7 +472,7 @@ static int rt_mutex_adjust_prio_chain(struct task_struct 
> *task,
>* We limit the lock chain length for each invocation.
>*/
>   if (++depth > max_lock_depth) {
> - static int prev_max;
> + int prev_max;
>
>   /*
>* Print this only once. If the admin changes the limit,
>
>
>
> --
> 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] Staging: rtlwifi: base: Fixed line ending with parentheses

2018-08-04 Thread Julia Lawall



On Sat, 4 Aug 2018, Sohil Ladhani wrote:

> This patch fixes the "Lines should not end with a '('" problem reported by
> checkpatch

There is still no v2 in the subject line, or explanation of what has
changed under the --- (I assume this is still the same place and the same
code; I didn't keep the old versions to check).

julia

>
> Signed-off-by: Sohil Ladhani 
> ---
>  drivers/staging/rtlwifi/base.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/staging/rtlwifi/base.c b/drivers/staging/rtlwifi/base.c
> index 094827c1879a..654aa4e068ba 100644
> --- a/drivers/staging/rtlwifi/base.c
> +++ b/drivers/staging/rtlwifi/base.c
> @@ -685,9 +685,8 @@ static void _rtl_query_protection_mode(struct 
> ieee80211_hw *hw,
>   }
>  }
>
> -u8 rtl_mrate_idx_to_arfr_id(
> - struct ieee80211_hw *hw, u8 rate_index,
> - enum wireless_mode wirelessmode)
> +u8 rtl_mrate_idx_to_arfr_id(struct ieee80211_hw *hw, u8 rate_index,
> + enum wireless_mode wirelessmode)
>  {
>   struct rtl_priv *rtlpriv = rtl_priv(hw);
>   struct rtl_phy *rtlphy = >phy;
> --
> 2.18.0
>
>


Re: [PATCH] Staging: rtlwifi: base: fixed a brace coding style issue

2018-08-04 Thread Julia Lawall



On Sat, 4 Aug 2018, Sohil Ladhani wrote:

> Fixed a coding style issue

This seems to fix the header problem.  But it is a patch on the same code
at the same place doing the same thing as the previous message.  So you
should say [PATCH v2] in the subject line, and then below the --- explain
what has changed as compared to the previous submission.

julia

>
> Signed-off-by: Sohil Ladhani 
> ---
>  drivers/staging/rtlwifi/base.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/staging/rtlwifi/base.c b/drivers/staging/rtlwifi/base.c
> index 094827c1879a..654aa4e068ba 100644
> --- a/drivers/staging/rtlwifi/base.c
> +++ b/drivers/staging/rtlwifi/base.c
> @@ -685,9 +685,8 @@ static void _rtl_query_protection_mode(struct 
> ieee80211_hw *hw,
>   }
>  }
>
> -u8 rtl_mrate_idx_to_arfr_id(
> - struct ieee80211_hw *hw, u8 rate_index,
> - enum wireless_mode wirelessmode)
> +u8 rtl_mrate_idx_to_arfr_id(struct ieee80211_hw *hw, u8 rate_index,
> + enum wireless_mode wirelessmode)
>  {
>   struct rtl_priv *rtlpriv = rtl_priv(hw);
>   struct rtl_phy *rtlphy = >phy;
> --
> 2.18.0
>
>


Re: [RFC PATCH] checkpatch: check for function calls with struct or union on stack

2018-07-28 Thread Julia Lawall



On Sat, 28 Jul 2018, Joe Perches wrote:

> On Sat, 2018-07-28 at 08:25 +0200, Julia Lawall wrote:
> > On Fri, 27 Jul 2018, Joe Perches wrote:
> []
> > > It might make sense for this sort of check to be
> > > added to coccinelle or maybe as a compiler warning
> > > when the struct is larger than some size.
> > >
> > > Original thread for Julia:
> > > https://lore.kernel.org/patchwork/patch/967890/
> >
> > Coccinelle doesn't directly know the size of the structure, but it can
> > count the number of fields.  Maybe a case with an update in the function
> > body
>
> Perhaps this might be the most useful to check.
>
> >  or at least 3 fields is worth reporting on?
>
> a struct with 3 chars or bools might be faster
> by value, but maybe for structs with arrays or
> other structs.
>
> For instance:
>
> lib/vsprintf.c uses struct printf_spec which
> is 16 bytes and that fits nicely in a

This message seems to be cut off.  I got around 70 results, of which 23
are from vsprintf.  Perhaps the simplest would be to print the structure
declaration with the warning message, so the user could easily check the
results.

julia


Re: [RFC PATCH] checkpatch: check for function calls with struct or union on stack

2018-07-28 Thread Julia Lawall



On Fri, 27 Jul 2018, Joe Perches wrote:

> On Fri, 2018-07-27 at 10:21 +, David Laight wrote:
> > From: Joe Perches Sent: 27 July 2018 11:09
> > > On Fri, 2018-07-27 at 10:04 +, David Laight wrote:
> > > > From: Andrew Morton Sent: 26 July 2018 20:28
> > > > > On Thu, 26 Jul 2018 12:25:33 -0700 Andrew Morton 
> > > > >  wrote:
> > > > >
> > > > > > I'll give it a spin, see how noisy it is.
> > > > >
> > > > > Actually, I would prefer if the message, changelog and title
> > > > > used the term "passed by value".  It's a more familiar term
> > > > > and it is possible for a passed-by-value aggregate to in fact
> > > > > be passed in registers.
> > > >
> > > > You need to detect (and ignore) 'small' structures.
> > >
> > > checkpatch is stupid and basically can't do that
> > > as it has no context other than the current line.
> > >
> > > It would need a list of specific struct types to
> > > ignore.  Care to create and send that list to me?
> >
> > Does it even have the type?
>
> Yes, kinda.  But only on the line being matched.
>
> i.e.:  [struct or union] [type] [name]
>
> > If it has the prototype it could ignore aggregates that
> > are marked 'const'.
>
> checkpatch has no visibility of any prototype.
>
> It might make sense for this sort of check to be
> added to coccinelle or maybe as a compiler warning
> when the struct is larger than some size.
>
> Original thread for Julia:
> https://lore.kernel.org/patchwork/patch/967890/

Coccinelle doesn't directly know the size of the structure, but it can
count the number of fields.  Maybe a case with an update in the function
body or at least 3 fields is worth reporting on?

julia


[PATCH] NTB: fix debugfs_simple_attr.cocci warnings

2018-07-21 Thread Julia Lawall
From: kbuild test robot 

 Use DEFINE_DEBUGFS_ATTRIBUTE rather than DEFINE_SIMPLE_ATTRIBUTE
 for debugfs files.

 Rationale: DEFINE_SIMPLE_ATTRIBUTE + debugfs_create_file()
 imposes some significant overhead as compared to
 DEFINE_DEBUGFS_ATTRIBUTE + debugfs_create_file_unsafe().

Generated by: scripts/coccinelle/api/debugfs/debugfs_simple_attr.cocci

Fixes: c75153197d20 ("NTB: Introduce NTB MSI Test Client")
Signed-off-by: kbuild test robot 
Signed-off-by: Julia Lawall 
---

I don't know much about this issue, beyondwhat is explained by the
semantic patch.  Please check if the changes can be relevant.

 ntb_msi_test.c |   61 -
 1 file changed, 31 insertions(+), 30 deletions(-)

--- a/drivers/ntb/test/ntb_msi_test.c
+++ b/drivers/ntb/test/ntb_msi_test.c
@@ -170,8 +170,8 @@ static int ntb_msit_dbgfs_trigger(void *
>msi_desc[idx]);
 }

-DEFINE_SIMPLE_ATTRIBUTE(ntb_msit_trigger_fops, NULL,
-   ntb_msit_dbgfs_trigger, "%llu\n");
+DEFINE_DEBUGFS_ATTRIBUTE(ntb_msit_trigger_fops, NULL, ntb_msit_dbgfs_trigger,
+"%llu\n");

 static int ntb_msit_dbgfs_port_get(void *data, u64 *port)
 {
@@ -182,8 +182,8 @@ static int ntb_msit_dbgfs_port_get(void
return 0;
 }

-DEFINE_SIMPLE_ATTRIBUTE(ntb_msit_port_fops, ntb_msit_dbgfs_port_get,
-   NULL, "%llu\n");
+DEFINE_DEBUGFS_ATTRIBUTE(ntb_msit_port_fops, ntb_msit_dbgfs_port_get, NULL,
+"%llu\n");

 static int ntb_msit_dbgfs_count_get(void *data, u64 *count)
 {
@@ -194,8 +194,8 @@ static int ntb_msit_dbgfs_count_get(void
return 0;
 }

-DEFINE_SIMPLE_ATTRIBUTE(ntb_msit_count_fops, ntb_msit_dbgfs_count_get,
-   NULL, "%llu\n");
+DEFINE_DEBUGFS_ATTRIBUTE(ntb_msit_count_fops, ntb_msit_dbgfs_count_get, NULL,
+"%llu\n");

 static int ntb_msit_dbgfs_ready_get(void *data, u64 *ready)
 {
@@ -213,8 +213,8 @@ static int ntb_msit_dbgfs_ready_set(void
return wait_for_completion_interruptible(>init_comp);
 }

-DEFINE_SIMPLE_ATTRIBUTE(ntb_msit_ready_fops, ntb_msit_dbgfs_ready_get,
-   ntb_msit_dbgfs_ready_set, "%llu\n");
+DEFINE_DEBUGFS_ATTRIBUTE(ntb_msit_ready_fops, ntb_msit_dbgfs_ready_get,
+ntb_msit_dbgfs_ready_set, "%llu\n");

 static int ntb_msit_dbgfs_occurrences_get(void *data, u64 *occurrences)
 {
@@ -225,9 +225,8 @@ static int ntb_msit_dbgfs_occurrences_ge
return 0;
 }

-DEFINE_SIMPLE_ATTRIBUTE(ntb_msit_occurrences_fops,
-   ntb_msit_dbgfs_occurrences_get,
-   NULL, "%llu\n");
+DEFINE_DEBUGFS_ATTRIBUTE(ntb_msit_occurrences_fops,
+ntb_msit_dbgfs_occurrences_get, NULL, "%llu\n");

 static int ntb_msit_dbgfs_local_port_get(void *data, u64 *port)
 {
@@ -238,9 +237,8 @@ static int ntb_msit_dbgfs_local_port_get
return 0;
 }

-DEFINE_SIMPLE_ATTRIBUTE(ntb_msit_local_port_fops,
-   ntb_msit_dbgfs_local_port_get,
-   NULL, "%llu\n");
+DEFINE_DEBUGFS_ATTRIBUTE(ntb_msit_local_port_fops,
+ntb_msit_dbgfs_local_port_get, NULL, "%llu\n");

 static void ntb_msit_create_dbgfs(struct ntb_msit_ctx *nm)
 {
@@ -251,8 +249,8 @@ static void ntb_msit_create_dbgfs(struct

nm->dbgfs_dir = debugfs_create_dir(pci_name(pdev),
   ntb_msit_dbgfs_topdir);
-   debugfs_create_file("port", 0400, nm->dbgfs_dir, nm,
-   _msit_local_port_fops);
+   debugfs_create_file_unsafe("port", 0400, nm->dbgfs_dir, nm,
+  _msit_local_port_fops);

for (i = 0; i < ntb_peer_port_count(nm->ntb); i++) {
nm->peers[i].pidx = i;
@@ -262,24 +260,27 @@ static void ntb_msit_create_dbgfs(struct
snprintf(buf, sizeof(buf), "peer%d", i);
peer_dir = debugfs_create_dir(buf, nm->dbgfs_dir);

-   debugfs_create_file("trigger", 0200, peer_dir,
-   >peers[i], _msit_trigger_fops);
-
-   debugfs_create_file("port", 0400, peer_dir,
-   >peers[i], _msit_port_fops);
-
-   debugfs_create_file("count", 0400, peer_dir,
-   >peers[i], _msit_count_fops);
-
-   debugfs_create_file("ready", 0600, peer_dir,
-   >peers[i], _msit_ready_fops);
+   debugfs_create_file_unsafe("trigger", 0200, peer_dir,
+  >peers[i],
+  _msit_trigger_fops);
+
+   

Re: [PATCH] checkpatch: Add warnings for use of mdelay()

2018-07-06 Thread Julia Lawall



On Thu, 5 Jul 2018, Dan Carpenter wrote:

> Neither Smatch nor Coccinelle do a good job tracking when you're in
> atomic context.  I've wanted to add this to Smatch but even then it
> would be to warn that "We're holding a spinlock so we can't sleep".
> It's trickier to say for sure when you're not holding a lock...

Jia-Ju Bai is working on this.  The tool is available on github.  It's
still being improved, though, so perhaps it's not yet ready for eg 0-day
inclusion.  He can give more details.

julia


RE: [PATCH] mei: bus: type promotion bug in mei_nfc_if_version()

2018-07-04 Thread Julia Lawall



On Wed, 4 Jul 2018, Winkler, Tomas wrote:

>
> > On Wed, Jul 04, 2018 at 01:57:44PM +, Winkler, Tomas wrote:
> > > >
> > > > On Wed, Jul 04, 2018 at 01:59:14PM +0200, Julia Lawall wrote:
> > > > >
> > > > >
> > > > > On Wed, 4 Jul 2018, Dan Carpenter wrote:
> > > > >
> > > > > > We accidentally removed the check for negative returns without
> > > > > > considering the issue of type promotion.  The "if_version_length"
> > > > > > variable is type size_t so if __mei_cl_recv() returns a negative
> > > > > > then "bytes_recv" is type promoted to a high positive value and
> > > > > > treated as success.
> > > > > >
> > > > > > Fixes: 582ab27a063a ("mei: bus: fix received data size check in
> > > > > > NFC
> > > > > > fixup")
> > > > > > Signed-off-by: Dan Carpenter 
> > > > > >
> > > > > > diff --git a/drivers/misc/mei/bus-fixup.c
> > > > > > b/drivers/misc/mei/bus-fixup.c index 0208c4b027c5..fa0236a5e59a
> > > > > > 100644
> > > > > > --- a/drivers/misc/mei/bus-fixup.c
> > > > > > +++ b/drivers/misc/mei/bus-fixup.c
> > > > > > @@ -267,7 +267,7 @@ static int mei_nfc_if_version(struct mei_cl
> > > > > > *cl,
> > > > > >
> > > > > > ret = 0;
> > > > > > bytes_recv = __mei_cl_recv(cl, (u8 *)reply, if_version_length,
> > 0);
> > > > > > -   if (bytes_recv < if_version_length) {
> > > > > > +   if (bytes_recv < 0 || bytes_recv < if_version_length) {
> > > > >
> > > > > Is this preferred to adding an int cast?
> > > >
> > > > I don't think it matters.  I kind of like explicitly testing for
> > > > negative but maybe later people will just remove the check like we
> > > > did here?  You could do it a bunch of different ways:
> > > >
> > > > 1: if (ret < 0 || ret < ARRAY_SIZE(xxx))
> > > > 2: if (ret < (int)ARRAY_SIZE(xxx))
> > > > 3: if (ret != ARRAY_SIZE(xxx))
> > > >
> > > > They're all equivalent.  I guess I don't like casting too much.  My
> > > > first approach to fixing this was just to declare if_version_length
> > > > as an int, but then I saw that originally there was a "bytes_recv < 0"
> > > > check and decided to go that way instead.
> > >
> > > Actually bytes_recv should be probably of ssize_t type,  so could be the
> > if_version_length.
> > >
> > > How did you find this, I haven't seen it in reported by sparse, smatch 
> > > and I
> > believe -Wsign-compare is suppressed in compilation warnings.
> >
> > It's a new thing.  Julia noticed this kind of bug first

Actually, I got it from Joe Perches, for the sizeof case :)

julia

> > and I have been mucking
> > around with it in Smatch as well.  My Smatch check has too many false
> > positives to publish right now because it thinks a some common functions
> > like ffs() return negative error codes.
>
> I guess this is why it is suppressed in the compilation warnings  in the 
> first place.
> Maybe need to disable it selectively, like for fss, just not sure how bad is 
> that with false positive reports.
>
>
> Thanks
> Tomas
>
>


Re: [PATCH] mei: bus: type promotion bug in mei_nfc_if_version()

2018-07-04 Thread Julia Lawall



On Wed, 4 Jul 2018, Dan Carpenter wrote:

> We accidentally removed the check for negative returns without
> considering the issue of type promotion.  The "if_version_length"
> variable is type size_t so if __mei_cl_recv() returns a negative then
> "bytes_recv" is type promoted to a high positive value and treated as
> success.
>
> Fixes: 582ab27a063a ("mei: bus: fix received data size check in NFC fixup")
> Signed-off-by: Dan Carpenter 
>
> diff --git a/drivers/misc/mei/bus-fixup.c b/drivers/misc/mei/bus-fixup.c
> index 0208c4b027c5..fa0236a5e59a 100644
> --- a/drivers/misc/mei/bus-fixup.c
> +++ b/drivers/misc/mei/bus-fixup.c
> @@ -267,7 +267,7 @@ static int mei_nfc_if_version(struct mei_cl *cl,
>
>   ret = 0;
>   bytes_recv = __mei_cl_recv(cl, (u8 *)reply, if_version_length, 0);
> - if (bytes_recv < if_version_length) {
> + if (bytes_recv < 0 || bytes_recv < if_version_length) {

Is this preferred to adding an int cast?

julia

>   dev_err(bus->dev, "Could not read IF version\n");
>   ret = -EIO;
>   goto err;
> --
> 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] MIPS: Remove no-op cast in show_regs()

2018-07-02 Thread Julia Lawall



On Mon, 2 Jul 2018, Ralf Baechle wrote:

> On Fri, Jun 22, 2018 at 11:07:03AM -0700, Paul Burton wrote:
>
> > In show_regs() we have a regs argument of type struct pt_regs *, and we
> > explicitly cast it to that same type as part of calling __show_regs().
> >
> > Casting regs to the same type that it is declared as does nothing at
> > all, so remove the useless cast.
>
> Good catch but there's no dump_stack() in v4.18-rc3 so this doesn't apply.
> That's trivial to patch up but since pointless casts are one of my pet
> peeve I used a semantic patch from a dark local repository to hunt down a
> few more.
>
> @identitycast@
> type T;
> T *A;
> @@
> - (T *) A
> + A
>
> Julia, I guess this isn't bulletproof but maybe something similar should
> be considered for scripts/coccinelle?

Thanks for the suggestion.  I will try to put together something.  From
the examples, it looks like - (T *)(A) could be better.  With that
Coccinelle will also match cases with no parentheses.

julia

>
>   Ralf
>
> Signed-off-by: Ralf Baechle 
>
>  arch/mips/kernel/relocate.c   |  2 +-
>  arch/mips/kernel/traps.c  |  2 +-
>  arch/mips/loongson64/loongson-3/smp.c | 10 +-
>  arch/mips/pmcs-msp71xx/msp_usb.c  |  4 ++--
>  arch/mips/sgi-ip22/ip28-berr.c|  2 +-
>  5 files changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/arch/mips/kernel/relocate.c b/arch/mips/kernel/relocate.c
> index cbf4cc0b0b6c..ae7d9cf2c849 100644
> --- a/arch/mips/kernel/relocate.c
> +++ b/arch/mips/kernel/relocate.c
> @@ -146,7 +146,7 @@ int __init do_relocations(void *kbase_old, void 
> *kbase_new, long offset)
>   break;
>
>   type = (*r >> 24) & 0xff;
> - loc_orig = (void *)(kbase_old + ((*r & 0x00ff) << 2));
> + loc_orig = (kbase_old + ((*r & 0x00ff) << 2));
>   loc_new = RELOCATED(loc_orig);
>
>   if (reloc_handlers_rel[type] == NULL) {
> diff --git a/arch/mips/kernel/traps.c b/arch/mips/kernel/traps.c
> index d67fa74622ee..2935aa608d2f 100644
> --- a/arch/mips/kernel/traps.c
> +++ b/arch/mips/kernel/traps.c
> @@ -350,7 +350,7 @@ static void __show_regs(const struct pt_regs *regs)
>   */
>  void show_regs(struct pt_regs *regs)
>  {
> - __show_regs((struct pt_regs *)regs);
> + __show_regs(regs);
>  }
>
>  void show_registers(struct pt_regs *regs)
> diff --git a/arch/mips/loongson64/loongson-3/smp.c 
> b/arch/mips/loongson64/loongson-3/smp.c
> index 8501109bb0f0..e231c2cb4a64 100644
> --- a/arch/mips/loongson64/loongson-3/smp.c
> +++ b/arch/mips/loongson64/loongson-3/smp.c
> @@ -349,7 +349,7 @@ static void loongson3_smp_finish(void)
>   write_c0_compare(read_c0_count() + mips_hpt_frequency/HZ);
>   local_irq_enable();
>   loongson3_ipi_write64(0,
> - (void *)(ipi_mailbox_buf[cpu_logical_map(cpu)]+0x0));
> + (ipi_mailbox_buf[cpu_logical_map(cpu)] + 0x0));
>   pr_info("CPU#%d finished, CP0_ST=%x\n",
>   smp_processor_id(), read_c0_status());
>  }
> @@ -416,13 +416,13 @@ static int loongson3_boot_secondary(int cpu, struct 
> task_struct *idle)
>   cpu, startargs[0], startargs[1], startargs[2]);
>
>   loongson3_ipi_write64(startargs[3],
> - (void *)(ipi_mailbox_buf[cpu_logical_map(cpu)]+0x18));
> + (ipi_mailbox_buf[cpu_logical_map(cpu)] + 0x18));
>   loongson3_ipi_write64(startargs[2],
> - (void *)(ipi_mailbox_buf[cpu_logical_map(cpu)]+0x10));
> + (ipi_mailbox_buf[cpu_logical_map(cpu)] + 0x10));
>   loongson3_ipi_write64(startargs[1],
> - (void *)(ipi_mailbox_buf[cpu_logical_map(cpu)]+0x8));
> + (ipi_mailbox_buf[cpu_logical_map(cpu)] + 0x8));
>   loongson3_ipi_write64(startargs[0],
> - (void *)(ipi_mailbox_buf[cpu_logical_map(cpu)]+0x0));
> + (ipi_mailbox_buf[cpu_logical_map(cpu)] + 0x0));
>   return 0;
>  }
>
> diff --git a/arch/mips/pmcs-msp71xx/msp_usb.c 
> b/arch/mips/pmcs-msp71xx/msp_usb.c
> index c87c5f810cd1..d38ac70b5a2e 100644
> --- a/arch/mips/pmcs-msp71xx/msp_usb.c
> +++ b/arch/mips/pmcs-msp71xx/msp_usb.c
> @@ -133,13 +133,13 @@ static int __init msp_usb_setup(void)
>* "D" for device-mode.  If it works for Ethernet, why not USB...
>*  -- hammtrev, 2007/03/22
>*/
> - snprintf((char *)[0], sizeof(envstr), "usbmode");
> + snprintf([0], sizeof(envstr), "usbmode");
>
>   /* set default host mode */
>   val = 1;
>
>   /* get environment string */
> - strp = prom_getenv((char *)[0]);
> + strp = prom_getenv([0]);
>   if (strp) {
>   /* compare string */
>   if (!strcmp(strp, "device"))
> diff --git a/arch/mips/sgi-ip22/ip28-berr.c b/arch/mips/sgi-ip22/ip28-berr.c
> index 2ed8e4990b7a..082541d33161 100644
> --- a/arch/mips/sgi-ip22/ip28-berr.c
> +++ 

[PATCH 0/3] cast sizeof to int for comparison

2018-07-01 Thread Julia Lawall
Comparing an int to a size, which is unsigned, causes the int to become
unsigned, giving the wrong result.

The semantic match that finds this problem is as follows:
(http://coccinelle.lip6.fr/)

// 
@safe disable not_int2@
int x;
position p;
binary operator op = {<,<=};
expression e;
@@

(
x < 0 || (x@p op e)
|
x <= 0 || (x@p op e)
|
x > 0 && (x@p op e)
|
x >= 0 && (x@p op e)
)

@@
int x;
type t;
expression e,e1;
identifier f != {strlen,resource_size};
position p != safe.p;
binary operator op = {<,<=};
@@

*x = f(...);
... when != x = e1
when != if (x < 0 || ...) { ... return ...; }
(
*x@p op sizeof(e)
|
*x@p op sizeof(t)
)
// 

---

 drivers/input/mouse/elan_i2c_smbus.c |2 +-
 drivers/media/usb/gspca/kinect.c |2 +-
 drivers/usb/wusbcore/security.c  |2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)


[PATCH 1/3] Input: elan_i2c_smbus - cast sizeof to int for comparison

2018-07-01 Thread Julia Lawall
Comparing an int to a size, which is unsigned, causes the int to become
unsigned, giving the wrong result.  i2c_smbus_read_block_data can return the
result of i2c_smbus_xfer, whih can return a negative error code.

A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)

// 
@@
int x;
expression e,e1;
identifier f;
@@

*x = f(...);
... when != x = e1
when != if (x < 0 || ...) { ... return ...; }
*x < sizeof(e)
// 

Signed-off-by: Julia Lawall 

---
 drivers/input/mouse/elan_i2c_smbus.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/input/mouse/elan_i2c_smbus.c 
b/drivers/input/mouse/elan_i2c_smbus.c
index c060d27..88e315d 100644
--- a/drivers/input/mouse/elan_i2c_smbus.c
+++ b/drivers/input/mouse/elan_i2c_smbus.c
@@ -387,7 +387,7 @@ static int elan_smbus_prepare_fw_update(struct i2c_client 
*client)
len = i2c_smbus_read_block_data(client,
ETP_SMBUS_IAP_PASSWORD_READ,
val);
-   if (len < sizeof(u16)) {
+   if (len < (int)sizeof(u16)) {
error = len < 0 ? len : -EIO;
dev_err(dev, "failed to read iap password: %d\n",
error);



Re: [PATCH v3 12/16] treewide: Use array_size() for kmalloc()-family

2018-07-01 Thread Julia Lawall
> > // 2-factor product with sizeof(variable)
> > @@
> > identifier alloc =~ "kmalloc|kzalloc|kvmalloc|kvzalloc";
>
> * This regular expression could be optimised to the specification 
> “kv?[mz]alloc”.
>   Extensions will be useful for further function names.
>
> * The repetition of such a constraint in subsequent SmPL rules could be 
> avoided
>   if inheritance will be used for this metavariable.

This is quite incorrect.  Inheritance is only possible when a match of the
previous rule has succeeded.  If a rule never applies in a given file, the
rules that inherit from it won't apply either.  Furthermore, what is
inherited is the value, not the constraint.  If the original binding of
alloc only ever matches kmalloc, then the inherited references will only
match kmalloc too.

julia

Re: [PATCH] platform/x86: dell-smbios: make a function and a pointer static

2018-06-23 Thread Julia Lawall



On Fri, 22 Jun 2018, Darren Hart wrote:

> On Thu, Jun 21, 2018 at 07:15:24PM +0100, Colin King wrote:
> > From: Colin Ian King 
> >
> > The function dell_smbios_smm_call and pointer platform_device are
> > local to the source and do not need to be in global scope, so make
> > them static.
> >
> > Cleans up sparse warnings:
> > warning: symbol 'platform_device' was not declared. Should it be static?
> > warning: symbol 'dell_smbios_smm_call' was not declared. Should it be
> > static?
> >
> > Signed-off-by: Colin Ian King 
> > ---
> >  drivers/platform/x86/dell-smbios-smm.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/platform/x86/dell-smbios-smm.c 
> > b/drivers/platform/x86/dell-smbios-smm.c
> > index e9e9da556318..97a90bebc360 100644
> > --- a/drivers/platform/x86/dell-smbios-smm.c
> > +++ b/drivers/platform/x86/dell-smbios-smm.c
> > @@ -24,7 +24,7 @@
> >  static int da_command_address;
> >  static int da_command_code;
> >  static struct calling_interface_buffer *buffer;
> > -struct platform_device *platform_device;
> > +static struct platform_device *platform_device;
> >  static DEFINE_MUTEX(smm_mutex);
> >
> >  static const struct dmi_system_id dell_device_table[] __initconst = {
> > @@ -82,7 +82,7 @@ static void find_cmd_address(const struct dmi_header *dm, 
> > void *dummy)
> > }
> >  }
> >
> > -int dell_smbios_smm_call(struct calling_interface_buffer *input)
> > +static int dell_smbios_smm_call(struct calling_interface_buffer *input)
>
> Hrm. So these are passed by pointer to dell_smbios_register_device(), which 
> is in
> turn called by dell_smbios_call() from dell-smbios-base.c.
>
> So while it is valid to make these static, since we're not referencing the
> symbol, but the pointer value instead - I do worry about the "static" 
> suggesting
> to someone reading the code that this data is not used outside of this file,
> when it is.

Static protects the name.  The name in this case is very generic.

julia

>
> I'm not finding a position on this in coding-style.
>
> Andy, do you care to weigh in on this?
>
> --
> Darren Hart
> VMware Open Source Technology Center
> --
> 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 v3 2/2] IB/mad: Use ID allocator routines to allocate agent number (fwd)

2018-06-17 Thread Julia Lawall



On Sun, 17 Jun 2018, jackm wrote:

> On Sat, 16 Jun 2018 18:04:41 +0200 (CEST)
> Julia Lawall  wrote:
>
> > ib_mad_client_id is declared as u32, so it will not be < 0 (line 382).
> >
> > julia
> >
> Julia, your are correct.
> However, I was under the impression that this patch set was abandoned
> in favor of the one submitted by Matthew Wilcox! (Convert IB/mad to use
> an IDR for agent IDs)

No idea.  I just get the kbuild reports, and this was one that I didn't
get to in a timely manner.  So it's probably fine to ignore it.

julia

>
> Hans, Matthew?
>
> -Jack
> > -- Forwarded message --
> > Date: Fri, 8 Jun 2018 08:49:57 +0800
> > From: kbuild test robot 
> > To: kbu...@01.org
> > Cc: Julia Lawall 
> > Subject: Re: [PATCH v3 2/2] IB/mad: Use ID allocator routines to
> > allocate agent number
> >
> > Hi Hans,
> >
> > Thank you for the patch! Perhaps something to improve:
> >
> > [auto build test WARNING on linus/master]
> > [also build test WARNING on v4.17]
> > [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/Hans-Westgaard-Ry/IB-mad-Use-ID-allocator-routines-to-allocate-agent-number/20180608-022348
> >  ::
> > branch date: 6 hours ago :: commit date: 6 hours ago
> >
> > >> drivers/infiniband/core/mad.c:382:5-21: WARNING: Unsigned
> > >> expression compared with zero: ib_mad_client_id < 0
> >
> > #
> > https://github.com/0day-ci/linux/commit/74b1ba09003c9d132878734bf44f32a62bad31db
> > git remote add linux-review https://github.com/0day-ci/linux git
> > remote update linux-review git checkout
> > 74b1ba09003c9d132878734bf44f32a62bad31db vim +382
> > drivers/infiniband/core/mad.c
> >
> > 2527e681 Sean Hefty2006-07-20  190
> > ^1da177e Linus Torvalds2005-04-16  191  /*
> > ^1da177e Linus Torvalds2005-04-16  192   * ib_register_mad_agent
> > - Register to send/receive MADs ^1da177e Linus Torvalds
> > 2005-04-16  193   */ ^1da177e Linus Torvalds2005-04-16  194
> > struct ib_mad_agent *ib_register_mad_agent(struct ib_device *device,
> > ^1da177e Linus Torvalds2005-04-16  195
> >u8 port_num, ^1da177e
> > Linus Torvalds2005-04-16  196
> >enum ib_qp_type qp_type,
> > ^1da177e Linus Torvalds2005-04-16  197
> >struct ib_mad_reg_req
> > *mad_reg_req, ^1da177e Linus Torvalds2005-04-16  198
> >u8 rmpp_version, ^1da177e
> > Linus Torvalds2005-04-16  199
> >ib_mad_send_handler
> > send_handler, ^1da177e Linus Torvalds2005-04-16  200
> >ib_mad_recv_handler
> > recv_handler, 0f29b46d Ira Weiny 2014-08-08  201
> >void *context, 0f29b46d
> > Ira Weiny 2014-08-08  202
> >u32 registration_flags)
> > ^1da177e Linus Torvalds2005-04-16  203  { ^1da177e Linus
> > Torvalds2005-04-16  204 struct ib_mad_port_private
> > *port_priv; ^1da177e Linus Torvalds2005-04-16  205
> > struct ib_mad_agent *ret = ERR_PTR(-EINVAL); ^1da177e Linus
> > Torvalds2005-04-16  206 struct ib_mad_agent_private
> > *mad_agent_priv; ^1da177e Linus Torvalds2005-04-16  207
> > struct ib_mad_reg_req *reg_req = NULL; ^1da177e Linus Torvalds
> > 2005-04-16  208 struct ib_mad_mgmt_class_table *class;
> > ^1da177e Linus Torvalds2005-04-16  209  struct
> > ib_mad_mgmt_vendor_class_table *vendor; ^1da177e Linus Torvalds
> > 2005-04-16  210 struct ib_mad_mgmt_vendor_class
> > *vendor_class; ^1da177e Linus Torvalds2005-04-16  211
> > struct ib_mad_mgmt_method_table *method; ^1da177e Linus Torvalds
> > 2005-04-16  212 int ret2, qpn; ^1da177e Linus Torvalds
> > 2005-04-16  213 unsigned long flags; ^1da177e Linus
> > Torvalds2005-04-16  214 u8 mgmt_class, vclass; 74b1ba09
> > Hans Westgaard Ry 2018-06-07  215   u32 ib_mad_client_id;
> > ^1da177e Linus Torvalds2005-04-16  216  /* Validate
> > parameters */ ^1da177e Linus Torvalds2005-04-16  217qpn
> > = get_spl_qp_index(qp_type); 9ad13a42 Ira Weiny 2014-08-08
> > 218 if (qpn == -1) { 9ad13a42 Ira Weiny 2014-08-08
> > 219 dev_notice(>dev, 9ad13a42 

Re: [PATCH v3 2/2] IB/mad: Use ID allocator routines to allocate agent number (fwd)

2018-06-16 Thread Julia Lawall
ib_mad_client_id is declared as u32, so it will not be < 0 (line 382).

julia

-- Forwarded message --
Date: Fri, 8 Jun 2018 08:49:57 +0800
From: kbuild test robot 
To: kbu...@01.org
Cc: Julia Lawall 
Subject: Re: [PATCH v3 2/2] IB/mad: Use ID allocator routines to allocate agent
number

Hi Hans,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on linus/master]
[also build test WARNING on v4.17]
[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/Hans-Westgaard-Ry/IB-mad-Use-ID-allocator-routines-to-allocate-agent-number/20180608-022348
:: branch date: 6 hours ago
:: commit date: 6 hours ago

>> drivers/infiniband/core/mad.c:382:5-21: WARNING: Unsigned expression 
>> compared with zero: ib_mad_client_id < 0

# 
https://github.com/0day-ci/linux/commit/74b1ba09003c9d132878734bf44f32a62bad31db
git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout 74b1ba09003c9d132878734bf44f32a62bad31db
vim +382 drivers/infiniband/core/mad.c

2527e681 Sean Hefty2006-07-20  190
^1da177e Linus Torvalds2005-04-16  191  /*
^1da177e Linus Torvalds2005-04-16  192   * ib_register_mad_agent - Register 
to send/receive MADs
^1da177e Linus Torvalds2005-04-16  193   */
^1da177e Linus Torvalds2005-04-16  194  struct ib_mad_agent 
*ib_register_mad_agent(struct ib_device *device,
^1da177e Linus Torvalds2005-04-16  195  
   u8 port_num,
^1da177e Linus Torvalds2005-04-16  196  
   enum ib_qp_type qp_type,
^1da177e Linus Torvalds2005-04-16  197  
   struct ib_mad_reg_req *mad_reg_req,
^1da177e Linus Torvalds2005-04-16  198  
   u8 rmpp_version,
^1da177e Linus Torvalds2005-04-16  199  
   ib_mad_send_handler send_handler,
^1da177e Linus Torvalds2005-04-16  200  
   ib_mad_recv_handler recv_handler,
0f29b46d Ira Weiny 2014-08-08  201  
   void *context,
0f29b46d Ira Weiny 2014-08-08  202  
   u32 registration_flags)
^1da177e Linus Torvalds2005-04-16  203  {
^1da177e Linus Torvalds2005-04-16  204  struct ib_mad_port_private 
*port_priv;
^1da177e Linus Torvalds2005-04-16  205  struct ib_mad_agent *ret = 
ERR_PTR(-EINVAL);
^1da177e Linus Torvalds2005-04-16  206  struct ib_mad_agent_private 
*mad_agent_priv;
^1da177e Linus Torvalds2005-04-16  207  struct ib_mad_reg_req *reg_req 
= NULL;
^1da177e Linus Torvalds2005-04-16  208  struct ib_mad_mgmt_class_table 
*class;
^1da177e Linus Torvalds2005-04-16  209  struct 
ib_mad_mgmt_vendor_class_table *vendor;
^1da177e Linus Torvalds2005-04-16  210  struct ib_mad_mgmt_vendor_class 
*vendor_class;
^1da177e Linus Torvalds2005-04-16  211  struct ib_mad_mgmt_method_table 
*method;
^1da177e Linus Torvalds2005-04-16  212  int ret2, qpn;
^1da177e Linus Torvalds2005-04-16  213  unsigned long flags;
^1da177e Linus Torvalds2005-04-16  214  u8 mgmt_class, vclass;
74b1ba09 Hans Westgaard Ry 2018-06-07  215  u32 ib_mad_client_id;
^1da177e Linus Torvalds2005-04-16  216  /* Validate parameters */
^1da177e Linus Torvalds2005-04-16  217  qpn = get_spl_qp_index(qp_type);
9ad13a42 Ira Weiny 2014-08-08  218  if (qpn == -1) {
9ad13a42 Ira Weiny 2014-08-08  219  dev_notice(>dev,
9ad13a42 Ira Weiny 2014-08-08  220 
"ib_register_mad_agent: invalid QP Type %d\n",
9ad13a42 Ira Weiny 2014-08-08  221 qp_type);
^1da177e Linus Torvalds2005-04-16  222  goto error1;
9ad13a42 Ira Weiny 2014-08-08  223  }
^1da177e Linus Torvalds2005-04-16  224
9ad13a42 Ira Weiny 2014-08-08  225  if (rmpp_version && 
rmpp_version != IB_MGMT_RMPP_VERSION) {
9ad13a42 Ira Weiny 2014-08-08  226  dev_notice(>dev,
9ad13a42 Ira Weiny 2014-08-08  227 
"ib_register_mad_agent: invalid RMPP Version %u\n",
9ad13a42 Ira Weiny 2014-08-08  228 
rmpp_version);
fa619a77 Hal Rosenstock2005-07-27  229  goto error1;
9ad13a42 Ira Weiny 2014-08-08  230  }
^1da177e Linus Torvalds2005-04-16  231
^1da177e Linus Torvalds2005-04-16  232  /* Validate MAD registration 
request if supplied */
^1da177e Linus Torvalds2005-04-16  233  if (mad_reg_req) {
9ad13a42 Ira Weiny 2014-08-08  234  if 
(mad_reg_req->mgmt_class_version >= MAX_MGMT_VERSION) {
9ad13a42 Ira Weiny 2014-08-0

Re: [PATCH] staging: rtl8192u: fix line over 80 characters

2018-06-16 Thread Julia Lawall



On Sat, 16 Jun 2018, Hyunil Kim wrote:

> *fix checkpatch.pl warnings:
>  WARNING: line over 80 characters

Describe what you do and why, rather than just saying "Fix".  Often
something can be fixed in multiple ways.

julia

>
> Signed-off-by: Hyunil Kim 
> ---
>  drivers/staging/rtl8192u/r8192U_core.c | 5 -
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/staging/rtl8192u/r8192U_core.c 
> b/drivers/staging/rtl8192u/r8192U_core.c
> index 74c5865..2111e01 100644
> --- a/drivers/staging/rtl8192u/r8192U_core.c
> +++ b/drivers/staging/rtl8192u/r8192U_core.c
> @@ -2044,7 +2044,10 @@ static bool GetNmodeSupportBySecCfg8192(struct 
> net_device *dev)
>   return false;
>   } else if ((wpa_ie_len != 0)) {
>   /* parse pairwise key type */
> - if (((ieee->wpa_ie[0] == 0xdd) && (!memcmp(&(ieee->wpa_ie[14]), 
> ccmp_ie, 4))) || ((ieee->wpa_ie[0] == 0x30) && (!memcmp(>wpa_ie[10], 
> ccmp_rsn_ie, 4
> + if (((ieee->wpa_ie[0] == 0xdd) &&
> + (!memcmp(&(ieee->wpa_ie[14]), ccmp_ie, 4))) ||
> + ((ieee->wpa_ie[0] == 0x30) &&
> + (!memcmp(>wpa_ie[10], ccmp_rsn_ie, 4
>   return true;
>   else
>   return false;
> --
> 2.7.4
>
>
> --
> 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] RFC: bus: fix of_table.cocci warnings

2018-06-15 Thread Julia Lawall
From: kbuild test robot 

Make sure (of/i2c/platform)_device_id tables are NULL terminated

Generated by: scripts/coccinelle/misc/of_table.cocci

Fixes: 7d8c3a62781b ("RFC: bus: 96boards Low-Speed Connector")
Signed-off-by: kbuild test robot 
Signed-off-by: Julia Lawall 
---

tree:
https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-integrator.git
ultra96
head:   27803f286226e1b5b5893aabfd55feec08b73bae
commit: 7d8c3a62781bbd8b42119b3af2c90f4d9eef3748 [20/21] RFC: bus:
96boards Low-Speed Connector

 96boards-ls-connector.c |1 +
 1 file changed, 1 insertion(+)

--- a/drivers/bus/96boards-mezzanines/96boards-ls-connector.c
+++ b/drivers/bus/96boards-mezzanines/96boards-ls-connector.c
@@ -294,6 +294,7 @@ static const struct of_device_id lscon_o
{
.compatible = "96boards,low-speed-connector",
},
+   {},
 };

 static struct platform_driver lscon_driver = {


[PATCH] RFC: bus: fix semicolon.cocci warnings

2018-06-15 Thread Julia Lawall
From: kbuild test robot 

 Remove unneeded semicolon.

Generated by: scripts/coccinelle/misc/semicolon.cocci

Fixes: 7d8c3a62781b ("RFC: bus: 96boards Low-Speed Connector")
Signed-off-by: kbuild test robot 
Signed-off-by: Julia Lawall 
---
tree:
https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-integrator.git 
ultra96
head:   27803f286226e1b5b5893aabfd55feec08b73bae
commit: 7d8c3a62781bbd8b42119b3af2c90f4d9eef3748 [20/21] RFC: bus:
96boards Low-Speed Connector


 96boards-secure96.c |4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

--- a/drivers/bus/96boards-mezzanines/96boards-secure96.c
+++ b/drivers/bus/96boards-mezzanines/96boards-secure96.c
@@ -223,7 +223,7 @@ out_unreg_leds:
platform_device_unregister(sec->leds_device);
for (i = 0; i < ARRAY_SIZE(ledinfos); i++) {
mezzanine_ls_put_gpiod(ls, sec->secure96_leds[i].gpiod);
-   };
+   }
return ERR_PTR(ret);
 }
 EXPORT_SYMBOL_GPL(secure96_populate);
@@ -244,6 +244,6 @@ void secure96_depopulate(void *data)
platform_device_unregister(sec->leds_device);
for (i = 0; i < ARRAY_SIZE(ledinfos); i++) {
mezzanine_ls_put_gpiod(sec->ls, sec->secure96_leds[i].gpiod);
-   };
+   }
 }
 EXPORT_SYMBOL_GPL(secure96_depopulate);


[PATCH] mfd: kempld-core: constify variables that point to const structure

2018-06-12 Thread Julia Lawall
Add const to the declaration of various local variables of type
kempld_platform_data for which the referenced value is always only
dereferenced or passed to a const parameter, to record the fact that
kempld_platform_data_generic is declared as const.

The semantic match that finds this issue is as follows:
(http://coccinelle.lip6.fr/)

// 
@r@
identifier i,j;
@@
const struct i j = { ... };

@ok@
identifier r.i;
position p;
@@
const struct i@p *

@@
identifier r.i;
position p != ok.p;
@@
* struct i@p *
// 

Signed-off-by: Julia Lawall 

---
 drivers/mfd/kempld-core.c |   15 ---
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/mfd/kempld-core.c b/drivers/mfd/kempld-core.c
index 390b27c..fb5a10b 100644
--- a/drivers/mfd/kempld-core.c
+++ b/drivers/mfd/kempld-core.c
@@ -143,7 +143,7 @@ static int kempld_register_cells_generic(struct 
kempld_device_data *pld)
 
 static int kempld_create_platform_device(const struct dmi_system_id *id)
 {
-   struct kempld_platform_data *pdata = id->driver_data;
+   const struct kempld_platform_data *pdata = id->driver_data;
int ret;
 
kempld_pdev = platform_device_alloc("kempld", -1);
@@ -259,7 +259,7 @@ void kempld_write32(struct kempld_device_data *pld, u8 
index, u32 data)
  */
 void kempld_get_mutex(struct kempld_device_data *pld)
 {
-   struct kempld_platform_data *pdata = dev_get_platdata(pld->dev);
+   const struct kempld_platform_data *pdata = dev_get_platdata(pld->dev);
 
mutex_lock(>lock);
pdata->get_hardware_mutex(pld);
@@ -272,7 +272,7 @@ void kempld_get_mutex(struct kempld_device_data *pld)
  */
 void kempld_release_mutex(struct kempld_device_data *pld)
 {
-   struct kempld_platform_data *pdata = dev_get_platdata(pld->dev);
+   const struct kempld_platform_data *pdata = dev_get_platdata(pld->dev);
 
pdata->release_hardware_mutex(pld);
mutex_unlock(>lock);
@@ -290,7 +290,7 @@ void kempld_release_mutex(struct kempld_device_data *pld)
 static int kempld_get_info(struct kempld_device_data *pld)
 {
int ret;
-   struct kempld_platform_data *pdata = dev_get_platdata(pld->dev);
+   const struct kempld_platform_data *pdata = dev_get_platdata(pld->dev);
char major, minor;
 
ret = pdata->get_info(pld);
@@ -332,7 +332,7 @@ static int kempld_get_info(struct kempld_device_data *pld)
  */
 static int kempld_register_cells(struct kempld_device_data *pld)
 {
-   struct kempld_platform_data *pdata = dev_get_platdata(pld->dev);
+   const struct kempld_platform_data *pdata = dev_get_platdata(pld->dev);
 
return pdata->register_cells(pld);
 }
@@ -444,7 +444,8 @@ static int kempld_detect_device(struct kempld_device_data 
*pld)
 
 static int kempld_probe(struct platform_device *pdev)
 {
-   struct kempld_platform_data *pdata = dev_get_platdata(>dev);
+   const struct kempld_platform_data *pdata =
+   dev_get_platdata(>dev);
struct device *dev = >dev;
struct kempld_device_data *pld;
struct resource *ioport;
@@ -476,7 +477,7 @@ static int kempld_probe(struct platform_device *pdev)
 static int kempld_remove(struct platform_device *pdev)
 {
struct kempld_device_data *pld = platform_get_drvdata(pdev);
-   struct kempld_platform_data *pdata = dev_get_platdata(pld->dev);
+   const struct kempld_platform_data *pdata = dev_get_platdata(pld->dev);
 
sysfs_remove_group(>dev->kobj, _attr_group);
 



[PATCH] parport: sunbpp: fix error return code

2018-06-10 Thread Julia Lawall
Return an error code on failure.  Change leading spaces to tab on the
first if.

Problem found using Coccinelle.

Signed-off-by: Julia Lawall 

---
 drivers/parport/parport_sunbpp.c |8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/parport/parport_sunbpp.c b/drivers/parport/parport_sunbpp.c
index 01cf1c1..8de3295 100644
--- a/drivers/parport/parport_sunbpp.c
+++ b/drivers/parport/parport_sunbpp.c
@@ -286,12 +286,16 @@ static int bpp_probe(struct platform_device *op)
 
ops = kmemdup(_sunbpp_ops, sizeof(struct parport_operations),
  GFP_KERNEL);
-if (!ops)
+   if (!ops) {
+   err = -ENOMEM;
goto out_unmap;
+   }
 
dprintk(("register_port\n"));
-   if (!(p = parport_register_port((unsigned long)base, irq, dma, ops)))
+   if (!(p = parport_register_port((unsigned long)base, irq, dma, ops))) {
+   err = -ENOMEM;
goto out_free_ops;
+   }
 
p->size = size;
p->dev = >dev;



[PATCH] clocksource/drivers/stm32: fix error return code

2018-06-10 Thread Julia Lawall
Return an error code on failure.

Problem found using Coccinelle.

Signed-off-by: Julia Lawall 

---
 drivers/clocksource/timer-stm32.c |4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/clocksource/timer-stm32.c 
b/drivers/clocksource/timer-stm32.c
index e5cdc3a..2717f88 100644
--- a/drivers/clocksource/timer-stm32.c
+++ b/drivers/clocksource/timer-stm32.c
@@ -304,8 +304,10 @@ static int __init stm32_timer_init(struct device_node 
*node)
 
to->private_data = kzalloc(sizeof(struct stm32_timer_private),
   GFP_KERNEL);
-   if (!to->private_data)
+   if (!to->private_data) {
+   ret = -ENOMEM;
goto deinit;
+   }
 
rstc = of_reset_control_get(node, NULL);
if (!IS_ERR(rstc)) {



[PATCH] staging: rtl8723bs: drop test

2018-06-06 Thread Julia Lawall
The test selects between two identical values, so it doesn't look useful.
It turns out that the tested expression can only be true anyway, so drop
the test, the corresponding parameter, and the corresponding argument at
the only call site.

The semantic match that finds this problem is as follows:
(http://coccinelle.lip6.fr/)

// 
@@
expression e,e1;
@@

* e ? e1 : e1
// 

Signed-off-by: Julia Lawall 

---
 drivers/staging/rtl8723bs/hal/HalPhyRf_8723B.c |5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/rtl8723bs/hal/HalPhyRf_8723B.c 
b/drivers/staging/rtl8723bs/hal/HalPhyRf_8723B.c
index 2ee25b2..53d3bdf 100644
--- a/drivers/staging/rtl8723bs/hal/HalPhyRf_8723B.c
+++ b/drivers/staging/rtl8723bs/hal/HalPhyRf_8723B.c
@@ -1352,7 +1352,6 @@ static void _PHY_ReloadMACRegisters8723B(
 static void _PHY_PathADDAOn8723B(
struct adapter *padapter,
u32 *ADDAReg,
-   bool isPathAOn,
bool is2T
 )
 {
@@ -1363,7 +1362,7 @@ static void _PHY_PathADDAOn8723B(
 
ODM_RT_TRACE(pDM_Odm, ODM_COMP_CALIBRATION, ODM_DBG_LOUD, ("ADDA 
ON.\n"));
 
-   pathOn = isPathAOn ? 0x01c00014 : 0x01c00014;
+   pathOn = 0x01c00014;
if (false == is2T) {
pathOn = 0x01c00014;
PHY_SetBBReg(pDM_Odm->Adapter, ADDAReg[0], bMaskDWord, 
0x01c00014);
@@ -1556,7 +1555,7 @@ static void phy_IQCalibrate_8723B(
}
ODM_RT_TRACE(pDM_Odm, ODM_COMP_CALIBRATION, ODM_DBG_LOUD, ("IQ 
Calibration for %s for %d times\n", (is2T ? "2T2R" : "1T1R"), t));
 
-   _PHY_PathADDAOn8723B(padapter, ADDA_REG, true, is2T);
+   _PHY_PathADDAOn8723B(padapter, ADDA_REG, is2T);
 
 /* no serial mode */
 



Re: unnecessary test?

2018-06-06 Thread Julia Lawall



On Wed, 6 Jun 2018, Takashi Iwai wrote:

> On Wed, 06 Jun 2018 14:39:05 +0200,
> Julia Lawall wrote:
> >
> > In the file sound/pci/ctxfi/cthw20k1.c, the function daio_mgr_dao_init
> > contains:
> >
> > set_field(>spoctl, SPOCTL_OS << (idx*8),
> >   ((conf >> 3) & 0x1) ? 2 : 2); /* Raw */
> >
> > Could the second argument just be 2?  It's true that the preceeding call
> > contains conf >> ..., but in a more useful way, so perhaps it could be
> > useful for uniformity?
>
> I guess this is a typo of "2 : 0".  The code seems toggling the
> control bit depending on the S/PDIF passthru mode.  It might be
> reversed, but I bet 1 for non-audio from a common sense.
>
> Ditto for cthw20k1.c.  This one is likely 1, not 2, though.

OK, should I send a patch?  I have no way to test it.

julia

>
>
> thanks,
>
> Takashi
>


Re: unnecessary test?

2018-06-06 Thread Julia Lawall



On Wed, 6 Jun 2018, Julia Lawall wrote:

> In the file sound/pci/ctxfi/cthw20k1.c, the function daio_mgr_dao_init
> contains:
>
> set_field(>spoctl, SPOCTL_OS << (idx*8),
>   ((conf >> 3) & 0x1) ? 2 : 2); /* Raw */
>
> Could the second argument just be 2?  It's true that the preceeding call
> contains conf >> ..., but in a more useful way, so perhaps it could be
> useful for uniformity?

There is similar code in daio_mgr_dao_init in sound/pci/ctxfi/cthw20k2.c:

set_field(>txctl[idx], ATXCTL_RAW,
 ((conf >> 3) & 0x1) ? 0 : 0);


julia


unnecessary test?

2018-06-06 Thread Julia Lawall
In the file sound/pci/ctxfi/cthw20k1.c, the function daio_mgr_dao_init
contains:

set_field(>spoctl, SPOCTL_OS << (idx*8),
  ((conf >> 3) & 0x1) ? 2 : 2); /* Raw */

Could the second argument just be 2?  It's true that the preceeding call
contains conf >> ..., but in a more useful way, so perhaps it could be
useful for uniformity?

thanks,
julia



Re: [PATCH] fsl/qe: ucc: copy and paste bug in ucc_get_tdm_sync_shift()

2018-06-04 Thread Julia Lawall



On Mon, 4 Jun 2018, Dan Carpenter wrote:

> There is a copy and paste bug so we accidentally use the RX_ shift when
> we're in TX_ mode.
>
> Fixes: bb8b2062aff3 ("fsl/qe: setup clock source for TDM mode")
> Signed-off-by: Dan Carpenter 
> ---
> Static analysis work.  Not tested.  This affects the success path, so
> we should probably test it.

Maybe this is another one?  I don't have time to look into it at the
moment...

drivers/net/ethernet/broadcom/bnx2x/bnx2x_link.c

/* For strict priority entries defines the number of consecutive
 * slots for the highest priority.
 */
REG_WR(bp, (port) ? NIG_REG_P1_TX_ARB_NUM_STRICT_ARB_SLOTS :
   NIG_REG_P1_TX_ARB_NUM_STRICT_ARB_SLOTS, 0x100);
/* Mapping between the CREDIT_WEIGHT registers and actual client
 * numbers
 */

I find some others that choose between constants, such as ... ? 0 : 0.

julia


>
> diff --git a/drivers/soc/fsl/qe/ucc.c b/drivers/soc/fsl/qe/ucc.c
> index c646d8713861..681f7d4b7724 100644
> --- a/drivers/soc/fsl/qe/ucc.c
> +++ b/drivers/soc/fsl/qe/ucc.c
> @@ -626,7 +626,7 @@ static u32 ucc_get_tdm_sync_shift(enum comm_dir mode, u32 
> tdm_num)
>  {
>   u32 shift;
>
> - shift = (mode == COMM_DIR_RX) ? RX_SYNC_SHIFT_BASE : RX_SYNC_SHIFT_BASE;
> + shift = (mode == COMM_DIR_RX) ? RX_SYNC_SHIFT_BASE : TX_SYNC_SHIFT_BASE;
>   shift -= tdm_num * 2;
>
>   return shift;
> --
> 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] 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-kernel@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;


[PATCH 2/5] phy: add missing of_node_put

2018-05-23 Thread Julia Lawall
The device node iterators perform an of_node_get on each iteration, so a
jump out of the loop requires an of_node_put.

The semantic patch that fixes this problem in the break case is as follows
(http://coccinelle.lip6.fr):

// 
@@
expression root,e;
local idexpression child;
iterator name for_each_child_of_node;
@@

 for_each_child_of_node(root, child) {
   ... when != of_node_put(child)
   when != e = child
+  of_node_put(child);
?  break;
   ...
}
... when != child
// 

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

---
 drivers/phy/hisilicon/phy-hisi-inno-usb2.c |9 +++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/phy/hisilicon/phy-hisi-inno-usb2.c 
b/drivers/phy/hisilicon/phy-hisi-inno-usb2.c
index 5243812..0da14b6 100644
--- a/drivers/phy/hisilicon/phy-hisi-inno-usb2.c
+++ b/drivers/phy/hisilicon/phy-hisi-inno-usb2.c
@@ -154,14 +154,18 @@ static int hisi_inno_phy_probe(struct platform_device 
*pdev)
struct phy *phy;
 
rst = of_reset_control_get_exclusive(child, NULL);
-   if (IS_ERR(rst))
+   if (IS_ERR(rst)) {
+   of_node_put(child);
return PTR_ERR(rst);
+   }
priv->ports[i].utmi_rst = rst;
priv->ports[i].priv = priv;
 
phy = devm_phy_create(dev, child, _inno_phy_ops);
-   if (IS_ERR(phy))
+   if (IS_ERR(phy)) {
+   of_node_put(child);
return PTR_ERR(phy);
+   }
 
phy_set_bus_width(phy, 8);
phy_set_drvdata(phy, >ports[i]);
@@ -169,6 +173,7 @@ static int hisi_inno_phy_probe(struct platform_device *pdev)
 
if (i > INNO_PHY_PORT_NUM) {
dev_warn(dev, "Support %d ports in maximum\n", i);
+   of_node_put(child);
break;
}
}



[PATCH 1/5] pinctrl: at91-pio4: add missing of_node_put

2018-05-23 Thread Julia Lawall
The device node iterators perform an of_node_get on each iteration, so a
jump out of the loop requires an of_node_put.

The semantic patch that fixes this problem is as follows
(http://coccinelle.lip6.fr):

// 
@@
expression root,e;
local idexpression child;
iterator name for_each_child_of_node;
@@

 for_each_child_of_node(root, child) {
   ... when != of_node_put(child)
   when != e = child
+  of_node_put(child);
?  break;
   ...
}
... when != child
// 

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

---
 drivers/pinctrl/pinctrl-at91-pio4.c |4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/pinctrl/pinctrl-at91-pio4.c 
b/drivers/pinctrl/pinctrl-at91-pio4.c
index 4b57a13..bafb3d4 100644
--- a/drivers/pinctrl/pinctrl-at91-pio4.c
+++ b/drivers/pinctrl/pinctrl-at91-pio4.c
@@ -576,8 +576,10 @@ static int atmel_pctl_dt_node_to_map(struct pinctrl_dev 
*pctldev,
for_each_child_of_node(np_config, np) {
ret = atmel_pctl_dt_subnode_to_map(pctldev, np, map,
_maps, num_maps);
-   if (ret < 0)
+   if (ret < 0) {
+   of_node_put(np);
break;
+   }
}
}
 



[PATCH 5/5] drm/rockchip: lvds: add missing of_node_put

2018-05-23 Thread Julia Lawall
The device node iterators perform an of_node_get on each iteration, so a
jump out of the loop requires an of_node_put.

The semantic patch that fixes this problem is as follows
(http://coccinelle.lip6.fr):

// 
@@
expression root,e;
local idexpression child;
iterator name for_each_child_of_node;
@@

 for_each_child_of_node(root, child) {
   ... when != of_node_put(child)
   when != e = child
+  of_node_put(child);
?  break;
   ...
}
... when != child
// 

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

---
 drivers/gpu/drm/rockchip/rockchip_lvds.c |4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/rockchip/rockchip_lvds.c 
b/drivers/gpu/drm/rockchip/rockchip_lvds.c
index e67f4ea..051b8be 100644
--- a/drivers/gpu/drm/rockchip/rockchip_lvds.c
+++ b/drivers/gpu/drm/rockchip/rockchip_lvds.c
@@ -363,8 +363,10 @@ static int rockchip_lvds_bind(struct device *dev, struct 
device *master,
of_property_read_u32(endpoint, "reg", _id);
ret = drm_of_find_panel_or_bridge(dev->of_node, 1, endpoint_id,
  >panel, >bridge);
-   if (!ret)
+   if (!ret) {
+   of_node_put(endpoint);
break;
+   }
}
if (!child_count) {
DRM_DEV_ERROR(dev, "lvds port does not have any children\n");



[PATCH 3/5] soc: ti: knav_dma: add missing of_node_put

2018-05-23 Thread Julia Lawall
The device node iterators perform an of_node_get on each iteration, so a
jump out of the loop requires an of_node_put.

The semantic patch that fixes this problem is as follows
(http://coccinelle.lip6.fr):

// 
@@
expression root,e;
local idexpression child;
iterator name for_each_child_of_node;
@@

 for_each_child_of_node(root, child) {
   ... when != of_node_put(child)
   when != e = child
+  of_node_put(child);
?  break;
   ...
}
... when != child
// 

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

---
 drivers/soc/ti/knav_dma.c |1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/soc/ti/knav_dma.c b/drivers/soc/ti/knav_dma.c
index 224d7dd..bda3173 100644
--- a/drivers/soc/ti/knav_dma.c
+++ b/drivers/soc/ti/knav_dma.c
@@ -768,6 +768,7 @@ static int knav_dma_probe(struct platform_device *pdev)
ret = dma_init(node, child);
if (ret) {
dev_err(>dev, "init failed with %d\n", ret);
+   of_node_put(child);
break;
}
}



[PATCH 0/5] add missing of_node_put

2018-05-23 Thread Julia Lawall
The device node iterators perform an of_node_get on each iteration, so a
jump out of the loop requires an of_node_put.

---

 drivers/gpu/drm/rockchip/rockchip_lvds.c   |4 +++-
 drivers/pci/hotplug/pnv_php.c  |8 ++--
 drivers/phy/hisilicon/phy-hisi-inno-usb2.c |9 +++--
 drivers/pinctrl/pinctrl-at91-pio4.c|4 +++-
 drivers/soc/ti/knav_dma.c  |1 +
 5 files changed, 20 insertions(+), 6 deletions(-)


[PATCH 4/5] pci/hotplug/pnv-php: add missing of_node_put

2018-05-23 Thread Julia Lawall
The device node iterators perform an of_node_get on each iteration, so a
jump out of the loop requires an of_node_put.

The semantic patch that fixes this problem is as follows
(http://coccinelle.lip6.fr):

// 
@@
expression root,e;
local idexpression child;
iterator name for_each_child_of_node;
@@

 for_each_child_of_node(root, child) {
   ... when != of_node_put(child)
   when != e = child
+  of_node_put(child);
?  break;
   ...
}
... when != child
// 

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

---
 drivers/pci/hotplug/pnv_php.c |8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/hotplug/pnv_php.c b/drivers/pci/hotplug/pnv_php.c
index d441006..6c2e8d7 100644
--- a/drivers/pci/hotplug/pnv_php.c
+++ b/drivers/pci/hotplug/pnv_php.c
@@ -220,12 +220,16 @@ static int pnv_php_populate_changeset(struct of_changeset 
*ocs,
 
for_each_child_of_node(dn, child) {
ret = of_changeset_attach_node(ocs, child);
-   if (ret)
+   if (ret) {
+   of_node_put(child);
break;
+   }
 
ret = pnv_php_populate_changeset(ocs, child);
-   if (ret)
+   if (ret) {
+   of_node_put(child);
break;
+   }
}
 
return ret;



[PATCH 1/3] i2c: mux: pca954x: merge calls to of_match_device and of_device_get_match_data

2018-05-21 Thread Julia Lawall
Drop call to of_match_device, which is subsumed by the subsequent
call to of_device_get_match_data.  The code becomes simpler, and a
temporary variable can be dropped.

The semantic match that makes this change is as follows:
(http://coccinelle.lip6.fr/)

// 
@r@
local idexpression match;
identifier i;
expression x, dev, e, e1;
@@
-match@i = of_match_device(x, dev);
-if (match) e = of_device_get_match_data(dev);
-else e = e1;
+e = of_device_get_match_data(dev);
+if (!e) e = e1;

@@
identifier r.i;
@@
- const struct of_device_id *i;
... when != i
// 

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

---
 drivers/i2c/muxes/i2c-mux-pca954x.c |7 ++-
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/i2c/muxes/i2c-mux-pca954x.c 
b/drivers/i2c/muxes/i2c-mux-pca954x.c
index 09bafd3..ced840f 100644
--- a/drivers/i2c/muxes/i2c-mux-pca954x.c
+++ b/drivers/i2c/muxes/i2c-mux-pca954x.c
@@ -373,7 +373,6 @@ static int pca954x_probe(struct i2c_client *client,
int num, force, class;
struct i2c_mux_core *muxc;
struct pca954x *data;
-   const struct of_device_id *match;
int ret;
 
if (!i2c_check_functionality(adap, I2C_FUNC_SMBUS_BYTE))
@@ -394,10 +393,8 @@ static int pca954x_probe(struct i2c_client *client,
if (IS_ERR(gpio))
return PTR_ERR(gpio);
 
-   match = of_match_device(of_match_ptr(pca954x_of_match), >dev);
-   if (match)
-   data->chip = of_device_get_match_data(>dev);
-   else
+   data->chip = of_device_get_match_data(>dev);
+   if (!data->chip)
data->chip = [id->driver_data];
 
if (data->chip->id.manufacturer_id != I2C_DEVICE_ID_NONE) {



[PATCH 0/3] merge calls to of_match_device and of_device_get_match_data

2018-05-21 Thread Julia Lawall
of_device_get_match_data calls of_match_device and fails if the latter
fails, so both calls aren't needed.

---

 drivers/i2c/muxes/i2c-mux-pca954x.c |7 ++-
 drivers/iio/adc/max1363.c   |8 ++--
 drivers/iio/potentiometer/max5481.c |7 ++-
 drivers/iio/potentiometer/mcp4018.c |7 ++-
 drivers/iio/potentiometer/mcp4531.c |7 ++-
 5 files changed, 10 insertions(+), 26 deletions(-)


[PATCH 3/3] iio: potentiometer: merge calls to of_match_device and of_device_get_match_data

2018-05-21 Thread Julia Lawall
Drop call to of_match_device, which is subsumed by the subsequent
call to of_device_get_match_data.  The code becomes simpler, and a
temporary variable can be dropped.

The semantic match that makes this change is as follows:
(http://coccinelle.lip6.fr/)

// 
@r@
local idexpression match;
identifier i;
expression x, dev, e, e1;
@@
-match@i = of_match_device(x, dev);
-if (match) e = of_device_get_match_data(dev);
-else e = e1;
+e = of_device_get_match_data(dev);
+if (!e) e = e1;

@@
identifier r.i;
@@
- const struct of_device_id *i;
... when != i
// 

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

---
 drivers/iio/potentiometer/max5481.c |7 ++-
 drivers/iio/potentiometer/mcp4018.c |7 ++-
 drivers/iio/potentiometer/mcp4531.c |7 ++-
 3 files changed, 6 insertions(+), 15 deletions(-)

diff --git a/drivers/iio/potentiometer/mcp4018.c 
b/drivers/iio/potentiometer/mcp4018.c
index 320a7c9..c051ee0 100644
--- a/drivers/iio/potentiometer/mcp4018.c
+++ b/drivers/iio/potentiometer/mcp4018.c
@@ -147,7 +147,6 @@ static int mcp4018_probe(struct i2c_client *client)
struct device *dev = >dev;
struct mcp4018_data *data;
struct iio_dev *indio_dev;
-   const struct of_device_id *match;
 
if (!i2c_check_functionality(client->adapter,
 I2C_FUNC_SMBUS_BYTE)) {
@@ -162,10 +161,8 @@ static int mcp4018_probe(struct i2c_client *client)
i2c_set_clientdata(client, indio_dev);
data->client = client;
 
-   match = of_match_device(of_match_ptr(mcp4018_of_match), dev);
-   if (match)
-   data->cfg = of_device_get_match_data(dev);
-   else
+   data->cfg = of_device_get_match_data(dev);
+   if (!data->cfg)
data->cfg = _cfg[i2c_match_id(mcp4018_id, 
client)->driver_data];
 
indio_dev->dev.parent = dev;
diff --git a/drivers/iio/potentiometer/mcp4531.c 
b/drivers/iio/potentiometer/mcp4531.c
index df894af..d87ca85 100644
--- a/drivers/iio/potentiometer/mcp4531.c
+++ b/drivers/iio/potentiometer/mcp4531.c
@@ -360,7 +360,6 @@ static int mcp4531_probe(struct i2c_client *client)
struct device *dev = >dev;
struct mcp4531_data *data;
struct iio_dev *indio_dev;
-   const struct of_device_id *match;
 
if (!i2c_check_functionality(client->adapter,
 I2C_FUNC_SMBUS_WORD_DATA)) {
@@ -375,10 +374,8 @@ static int mcp4531_probe(struct i2c_client *client)
i2c_set_clientdata(client, indio_dev);
data->client = client;
 
-   match = of_match_device(of_match_ptr(mcp4531_of_match), dev);
-   if (match)
-   data->cfg = of_device_get_match_data(dev);
-   else
+   data->cfg = of_device_get_match_data(dev);
+   if (!data->cfg)
data->cfg = _cfg[i2c_match_id(mcp4531_id, 
client)->driver_data];
 
indio_dev->dev.parent = dev;
diff --git a/drivers/iio/potentiometer/max5481.c 
b/drivers/iio/potentiometer/max5481.c
index ffe2761..6d2f13f 100644
--- a/drivers/iio/potentiometer/max5481.c
+++ b/drivers/iio/potentiometer/max5481.c
@@ -137,7 +137,6 @@ static int max5481_probe(struct spi_device *spi)
struct iio_dev *indio_dev;
struct max5481_data *data;
const struct spi_device_id *id = spi_get_device_id(spi);
-   const struct of_device_id *match;
int ret;
 
indio_dev = devm_iio_device_alloc(>dev, sizeof(*data));
@@ -149,10 +148,8 @@ static int max5481_probe(struct spi_device *spi)
 
data->spi = spi;
 
-   match = of_match_device(of_match_ptr(max5481_match), >dev);
-   if (match)
-   data->cfg = of_device_get_match_data(>dev);
-   else
+   data->cfg = of_device_get_match_data(>dev);
+   if (!data->cfg)
data->cfg = _cfg[id->driver_data];
 
indio_dev->name = id->name;



[PATCH] coccinelle: deref_null: improve performance

2018-05-21 Thread Julia Lawall
Move rules looking for some special cases of safe dereferences before
the collection of NULL-tested values.  The special cases are fairly
rare, but somewhat costly to find, because isomorphisms create many
variants of the rules.  There is thus no need to search for them over
and over for each NULL tested expression.  Collecting them just once
is sufficient and more efficient.

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

---
 scripts/coccinelle/null/deref_null.cocci |   40 +++
 1 file changed, 20 insertions(+), 20 deletions(-)

diff --git a/scripts/coccinelle/null/deref_null.cocci 
b/scripts/coccinelle/null/deref_null.cocci
index b16ccb7..cbc6184 100644
--- a/scripts/coccinelle/null/deref_null.cocci
+++ b/scripts/coccinelle/null/deref_null.cocci
@@ -14,18 +14,10 @@ virtual context
 virtual org
 virtual report
 
-@ifm@
-expression *E;
-statement S1,S2;
-position p1;
-@@
-
-if@p1 ((E == NULL && ...) || ...) S1 else S2
-
 // The following two rules are separate, because both can match a single
 // expression in different ways
 @pr1 expression@
-expression *ifm.E;
+expression E;
 identifier f;
 position p1;
 @@
@@ -33,7 +25,7 @@ position p1;
  (E != NULL && ...) ? <+...E->f@p1...+> : ...
 
 @pr2 expression@
-expression *ifm.E;
+expression E;
 identifier f;
 position p2;
 @@
@@ -46,6 +38,14 @@ position p2;
  sizeof(<+...E->f@p2...+>)
 )
 
+@ifm@
+expression *E;
+statement S1,S2;
+position p1;
+@@
+
+if@p1 ((E == NULL && ...) || ...) S1 else S2
+
 // For org and report modes
 
 @r depends on !context && (org || report) exists@
@@ -212,16 +212,8 @@ else S3
 // The following three rules are duplicates of ifm, pr1 and pr2 respectively.
 // It is need because the previous rule as already made a "change".
 
-@ifm1 depends on context && !org && !report@
-expression *E;
-statement S1,S2;
-position p1;
-@@
-
-if@p1 ((E == NULL && ...) || ...) S1 else S2
-
 @pr11 depends on context && !org && !report expression@
-expression *ifm1.E;
+expression E;
 identifier f;
 position p1;
 @@
@@ -229,7 +221,7 @@ position p1;
  (E != NULL && ...) ? <+...E->f@p1...+> : ...
 
 @pr12 depends on context && !org && !report expression@
-expression *ifm1.E;
+expression E;
 identifier f;
 position p2;
 @@
@@ -242,6 +234,14 @@ position p2;
  sizeof(<+...E->f@p2...+>)
 )
 
+@ifm1 depends on context && !org && !report@
+expression *E;
+statement S1,S2;
+position p1;
+@@
+
+if@p1 ((E == NULL && ...) || ...) S1 else S2
+
 @depends on context && !org && !report exists@
 expression subE <= ifm1.E;
 expression *ifm1.E;



[PATCH] coccinelle: mini_lock: improve performance

2018-05-21 Thread Julia Lawall
Replace <+... ...+> by ... when any.  <+... ...+> is slow, and in some
obscure cases involving backward jumps it doesn't force the unlock to
actually come after the end of the if.

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

---
 scripts/coccinelle/locks/mini_lock.cocci |6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/scripts/coccinelle/locks/mini_lock.cocci 
b/scripts/coccinelle/locks/mini_lock.cocci
index 47f649b..19c6ee5 100644
--- a/scripts/coccinelle/locks/mini_lock.cocci
+++ b/scripts/coccinelle/locks/mini_lock.cocci
@@ -67,12 +67,14 @@ identifier lock,unlock;
 @@
 
 *lock(E1@p,...);
-<+... when != E1
+... when != E1
+when any
 if (...) {
   ... when != E1
 *  return@r ...;
 }
-...+>
+... when != E1
+when any
 *unlock@up(E1,...);
 
 @script:python depends on org@



drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c:1001:52-53: asic_setup: first occurrence line 1004, second occurrence line 1031 (fwd)

2018-05-18 Thread Julia Lawall
The structure has two initializations of the field asic_setup.

julia

-- Forwarded message --
Date: Sat, 19 May 2018 02:35:04 +0800
From: kbuild test robot <l...@intel.com>
To: kbu...@01.org
Cc: Julia Lawall <julia.law...@lip6.fr>
Subject: drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c:1001:52-53:
asic_setup: first occurrence line 1004, second occurrence line 1031

CC: kbuild-...@01.org
CC: linux-kernel@vger.kernel.org
TO: Rex Zhu <rex@amd.com>
CC: Alex Deucher <alexander.deuc...@amd.com>

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 
master
head:   3acf4e395260e3bd30a6fa29ba7eada4bf7566ca
commit: c425688520990d6cec769faaa97f4af45d361fd1 drm/amd/pp: Replace rv_* with 
smu10_*
date:   9 weeks ago
:: branch date: 20 hours ago
:: commit date: 9 weeks ago

>> drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c:1001:52-53: asic_setup: 
>> first occurrence line 1004, second occurrence line 1031

# 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=c425688520990d6cec769faaa97f4af45d361fd1
git remote add linus 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
git remote update linus
git checkout c425688520990d6cec769faaa97f4af45d361fd1
vim +1001 drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c

b01a4f48 drivers/gpu/drm/amd/powerplay/hwmgr/rv_hwmgr.cEric Huang 
2018-02-06  1000
c4256885 drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c Rex Zhu
2018-03-06 @1001  static const struct pp_hwmgr_func smu10_hwmgr_funcs = {
c4256885 drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c Rex Zhu
2018-03-06  1002  .backend_init = smu10_hwmgr_backend_init,
c4256885 drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c Rex Zhu
2018-03-06  1003  .backend_fini = smu10_hwmgr_backend_fini,
a960d61c drivers/gpu/drm/amd/powerplay/hwmgr/rv_hwmgr.cRex Zhu
2017-05-11 @1004  .asic_setup = NULL,
c4256885 drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c Rex Zhu
2018-03-06  1005  .apply_state_adjust_rules = 
smu10_apply_state_adjust_rules,
c4256885 drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c Rex Zhu
2018-03-06  1006  .force_dpm_level = smu10_dpm_force_dpm_level,
c4256885 drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c Rex Zhu
2018-03-06  1007  .get_power_state_size = smu10_get_power_state_size,
a960d61c drivers/gpu/drm/amd/powerplay/hwmgr/rv_hwmgr.cRex Zhu
2017-05-11  1008  .powerdown_uvd = NULL,
a960d61c drivers/gpu/drm/amd/powerplay/hwmgr/rv_hwmgr.cRex Zhu
2017-05-11  1009  .powergate_uvd = NULL,
a960d61c drivers/gpu/drm/amd/powerplay/hwmgr/rv_hwmgr.cRex Zhu
2017-05-11  1010  .powergate_vce = NULL,
c4256885 drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c Rex Zhu
2018-03-06  1011  .get_mclk = smu10_dpm_get_mclk,
c4256885 drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c Rex Zhu
2018-03-06  1012  .get_sclk = smu10_dpm_get_sclk,
c4256885 drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c Rex Zhu
2018-03-06  1013  .patch_boot_state = smu10_dpm_patch_boot_state,
c4256885 drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c Rex Zhu
2018-03-06  1014  .get_pp_table_entry = smu10_dpm_get_pp_table_entry,
c4256885 drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c Rex Zhu
2018-03-06  1015  .get_num_of_pp_table_entries = 
smu10_dpm_get_num_of_pp_table_entries,
c4256885 drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c Rex Zhu
2018-03-06  1016  .set_cpu_power_state = smu10_set_cpu_power_state,
c4256885 drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c Rex Zhu
2018-03-06  1017  .store_cc6_data = smu10_store_cc6_data,
c4256885 drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c Rex Zhu
2018-03-06  1018  .force_clock_level = smu10_force_clock_level,
c4256885 drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c Rex Zhu
2018-03-06  1019  .print_clock_levels = smu10_print_clock_levels,
c4256885 drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c Rex Zhu
2018-03-06  1020  .get_dal_power_level = smu10_get_dal_power_level,
c4256885 drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c Rex Zhu
2018-03-06  1021  .get_performance_level = smu10_get_performance_level,
c4256885 drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c Rex Zhu
2018-03-06  1022  .get_current_shallow_sleep_clocks = 
smu10_get_current_shallow_sleep_clocks,
c4256885 drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c Rex Zhu
2018-03-06  1023  .get_clock_by_type_with_latency = 
smu10_get_clock_by_type_with_latency,
c4256885 drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c Rex Zhu
2018-03-06  1024  .get_clock_by_type_with_voltage = 
smu10_get_clock_by_type_with_voltage,
c4256885 drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c Rex Zhu
2018-03-06

[PATCH] drm/nouveau/kms/nv50-: fix drm-get-put.cocci warnings

2018-05-18 Thread Julia Lawall
From: kbuild test robot <fengguang...@intel.com>

 Use drm_*_get() and drm_*_put() helpers instead of drm_*_reference() and
 drm_*_unreference() helpers.

Generated by: scripts/coccinelle/api/drm-get-put.cocci

Fixes: 30ed49b55b6e ("drm/nouveau/kms/nv50-: move code underneath dispnv50/")
Signed-off-by: kbuild test robot <fengguang...@intel.com>
Signed-off-by: Julia Lawall <julia.law...@lip6.fr>
---

tree:   https://github.com/skeggsb/linux linux-4.18
head:   6c46d01f25bcf74608d09645c27c35c3f3940ebe
commit: 30ed49b55b6e44e004c3095671e74fea93ee84cb [105/165]
drm/nouveau/kms/nv50-: move code underneath dispnv50/

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

--- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
@@ -3254,7 +3254,7 @@ nv50_mstm_destroy_connector(struct drm_d
mstc->port = NULL;
drm_modeset_unlock(>dev->mode_config.connection_mutex);

-   drm_connector_unreference(>connector);
+   drm_connector_put(>connector);
 }

 static void


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 
> ---
>  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] leds: lm3601x: fix semicolon.cocci warnings

2018-05-09 Thread Julia Lawall
From: Fengguang Wu <fengguang...@intel.com>

 Remove unneeded semicolon.

Generated by: scripts/coccinelle/misc/semicolon.cocci

Fixes: b550389fcb74 ("leds: lm3601x: Introduce the lm3601x LED driver")
CC: Dan Murphy <dmur...@ti.com>
Signed-off-by: Fengguang Wu <fengguang...@intel.com>
Signed-off-by: Julia Lawall <julia.law...@lip6.fr>
---

url:
https://github.com/0day-ci/linux/commits/Dan-Murphy/dt-bindings-lm3601x-Introduce-the-lm3601x-driver/20180509-033939
base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git
for-next
:: branch date: 5 hours ago
:: commit date: 5 hours ago

I also received the following comment, but no other details.  It would be
good to check what is going on on the mentioned lines.

>> drivers/leds/leds-lm3601x.c:315:2-8: preceding lock on line 311

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

--- a/drivers/leds/leds-lm3601x.c
+++ b/drivers/leds/leds-lm3601x.c
@@ -234,7 +234,7 @@ static int lm3601x_strobe_set(struct led

ret = -EINVAL;
goto out;
-   };
+   }

if (led->strobe_timeout != current_timeout)
ret = regmap_update_bits(led->regmap, LM3601X_CFG_REG,


[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"



[PATCH] mwifiex: delete unneeded include

2018-05-06 Thread Julia Lawall
Nothing that is defined in 11ac.h is referenced in cmdevt.c.

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

---
 drivers/net/wireless/marvell/mwifiex/cmdevt.c |1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/wireless/marvell/mwifiex/cmdevt.c 
b/drivers/net/wireless/marvell/mwifiex/cmdevt.c
index 7014f44..9cfcdf6 100644
--- a/drivers/net/wireless/marvell/mwifiex/cmdevt.c
+++ b/drivers/net/wireless/marvell/mwifiex/cmdevt.c
@@ -25,7 +25,6 @@
 #include "main.h"
 #include "wmm.h"
 #include "11n.h"
-#include "11ac.h"
 
 static void mwifiex_cancel_pending_ioctl(struct mwifiex_adapter *adapter);
 



Re: [PATCH 2/2] mm: Add kvmalloc_ab_c and kvzalloc_struct

2018-05-01 Thread Julia Lawall


On Tue, 1 May 2018, Kees Cook wrote:

> On Mon, Apr 30, 2018 at 2:29 PM, Rasmus Villemoes
>  wrote:
> > On 2018-04-30 22:16, Matthew Wilcox wrote:
> >> On Mon, Apr 30, 2018 at 12:02:14PM -0700, Kees Cook wrote:
> >>>
> >>> Getting the constant ordering right could be part of the macro
> >>> definition, maybe? i.e.:
> >>>
> >>> static inline void *kmalloc_ab(size_t a, size_t b, gfp_t flags)
> >>> {
> >>> if (__builtin_constant_p(a) && a != 0 && \
> >>> b > SIZE_MAX / a)
> >>> return NULL;
> >>> else if (__builtin_constant_p(b) && b != 0 && \
> >>>a > SIZE_MAX / b)
> >>> return NULL;
> >>>
> >>> return kmalloc(a * b, flags);
> >>> }
> >>
> >> Ooh, if neither a nor b is constant, it just didn't do a check ;-(  This
> >> stuff is hard.
> >>
> >>> (I just wish C had a sensible way to catch overflow...)
> >>
> >> Every CPU I ever worked with had an "overflow" bit ... do we have a
> >> friend on the C standards ctte who might figure out a way to let us
> >> write code that checks it?
> >
> > gcc 5.1+ (I think) have the __builtin_OP_overflow checks that should
> > generate reasonable code. Too bad there's no completely generic
> > check_all_ops_in_this_expression(a+b*c+d/e, or_jump_here). Though it's
> > hard to define what they should be checked against - probably would
> > require all subexpressions (including the variables themselves) to have
> > the same type.
> >
> > plug: https://lkml.org/lkml/2015/7/19/358
>
> That's a very nice series. Why did it never get taken? It seems to do
> the right things quite correctly.
>
> Daniel, while this isn't a perfect solution, is this something you'd
> use in graphics-land?

Opportunities for this, found with the following are shown below:

@@
expression a,b;
@@

*if (a + b < a)
 { ... return ...; }

- at the beginning of a line indicates an opportunity, not a suggestion
for removal.

I haven't checked the results carefully, but most look relevant.

julia



diff -u -p /var/linuxes/linux-next/lib/zstd/decompress.c 
/tmp/nothing/lib/zstd/decompress.c
--- /var/linuxes/linux-next/lib/zstd/decompress.c
+++ /tmp/nothing/lib/zstd/decompress.c
@@ -343,7 +343,6 @@ unsigned long long ZSTD_findDecompressed
return ret;

/* check for overflow */
-   if (totalDstSize + ret < totalDstSize)
return ZSTD_CONTENTSIZE_ERROR;
totalDstSize += ret;
}
diff -u -p /var/linuxes/linux-next/lib/scatterlist.c 
/tmp/nothing/lib/scatterlist.c
--- /var/linuxes/linux-next/lib/scatterlist.c
+++ /tmp/nothing/lib/scatterlist.c
@@ -503,7 +503,6 @@ struct scatterlist *sgl_alloc_order(unsi
nalloc = nent;
if (chainable) {
/* Check for integer overflow */
-   if (nalloc + 1 < nalloc)
return NULL;
nalloc++;
}
diff -u -p /var/linuxes/linux-next/drivers/dma-buf/dma-buf.c 
/tmp/nothing/drivers/dma-buf/dma-buf.c
--- /var/linuxes/linux-next/drivers/dma-buf/dma-buf.c
+++ /tmp/nothing/drivers/dma-buf/dma-buf.c
@@ -954,7 +954,6 @@ int dma_buf_mmap(struct dma_buf *dmabuf,
return -EINVAL;

/* check for offset overflow */
-   if (pgoff + vma_pages(vma) < pgoff)
return -EOVERFLOW;

/* check for overflowing the buffer's size */
diff -u -p /var/linuxes/linux-next/drivers/md/dm-verity-target.c 
/tmp/nothing/drivers/md/dm-verity-target.c
--- /var/linuxes/linux-next/drivers/md/dm-verity-target.c
+++ /tmp/nothing/drivers/md/dm-verity-target.c
@@ -1043,7 +1043,6 @@ static int verity_ctr(struct dm_target *
v->hash_level_block[i] = hash_position;
s = (v->data_blocks + ((sector_t)1 << ((i + 1) * 
v->hash_per_block_bits)) - 1)
>> ((i + 1) * v->hash_per_block_bits);
-   if (hash_position + s < hash_position) {
ti->error = "Hash device offset overflow";
r = -E2BIG;
goto bad;
diff -u -p /var/linuxes/linux-next/drivers/md/dm-flakey.c 
/tmp/nothing/drivers/md/dm-flakey.c
--- /var/linuxes/linux-next/drivers/md/dm-flakey.c
+++ /tmp/nothing/drivers/md/dm-flakey.c
@@ -233,7 +233,6 @@ static int flakey_ctr(struct dm_target *
goto bad;
}

-   if (fc->up_interval + fc->down_interval < fc->up_interval) {
ti->error = "Interval overflow";
r = -EINVAL;
goto bad;
diff -u -p /var/linuxes/linux-next/drivers/gpu/drm/vc4/vc4_validate.c 
/tmp/nothing/drivers/gpu/drm/vc4/vc4_validate.c
--- /var/linuxes/linux-next/drivers/gpu/drm/vc4/vc4_validate.c
+++ /tmp/nothing/drivers/gpu/drm/vc4/vc4_validate.c
@@ -306,7 +306,6 @@ validate_gl_array_primitive(VALIDATE_ARG
}
shader_state = 

Re: bug-introducing patches (or: -rc cycles suck)

2018-04-30 Thread Julia Lawall


On Mon, 30 Apr 2018, Sasha Levin wrote:

> Working on AUTOSEL, it became even more obvious to me how difficult it is for 
> a patch to get a proper review. Maintainers found it difficult to keep up 
> with the upstream work for their subsystem, and reviewing additional -stable 
> patches put even more load on them which some suggested would be more than 
> what they can handle.
>
> While AUTOSEL tries to understand if a patch fixes a bug, this was a bit 
> late: the bug was already introduced, folks already have to deal with it, and 
> the kernel is broken. I was wondering if I can do a similar process to 
> AUTOSEL, but teach the AI about bug-introducing patches.
>
> When someone fixes a bug, he would describe the patch differently than he 
> would if he was writing a new feature. This lets AUTOSEL build on different 
> commit message constructs, among various inputs, to recognize bug fixes. 
> However, people are unaware that they introduce a bug, so the commit message 
> for bug introducing patches is essentially the same as for commits that don't 
> introduce a bug. This meant that I had to try and source data out of 
> different sources.
>
> Few of the parameters I ended up using are:
>  - -next data (days spent in -next, changes in the patch between -next trees, 
> ...)
>  - Mailing list data (was this patch ever sent to a ML? How long before it 
> was merged? How many replies did it get? ...)
>  - Author/commiter/maintainer chain data. Just like sports, some folks are 
> more likely to produce better results than others. This goes beyond just 
> "skill", but also looks at things such as whether the author patches a 
> subsystem he's "familiar with" (== subsystem where most of his patches 
> usually go), or is he modifying a subsystem he never sent a patch for.
>  - Patch complexity metrics - various code metrics to indicate how "complex" 
> a patch is. Think 100 lines of whitespace fixes vs 100 lines that 
> significantly changes a subsystem.
>  - Kernel process correctness - I tried using "violations" of the kernel 
> process (patch formatting, correctness of the mailing to lkml, etc) as an 
> indicator of how familiar the author is with the kernel, with the presumption 
> that folks who are newer to kernel development are more likely to introduce 
> bugs

I'm not completely sure to understand what you are doing.  Is there also
some connection to things that are identified in some way as being bug
introducing patches?  Or are you just using these as metrics of low
quality?

I wonder how far one could get by just collecting the set of patches that
are referenced with fixes tags by stable patches, and then using machine
learning taking into account only the code to find other patches that make
similar changes.

julia

> Running an initial iteration on a set of commits made two things very obvious 
> to me:
>
> 1. -rc releases suck. seriously suck. The quality of commits that went in -rc 
> cycles was much worse that merge window commit:
>  - All commits had the same chance of introducing a bug whether they came in 
> a merge window or an -rc cycle. This means that -rc commits mostly end up 
> replacing obvious bugs with less obvious ones.
>  - While the average merge window commit changes, on average, 3x more lines 
> than an -rc commit, the chances of a bug introduced per patch is the same, 
> which means that bugs-per-line metric of code is much higher with -rc patches.
>  - A merge window commit spent 50% more days, on average, in -next than a -rc 
> commit.
>  - The number of -rc commits that never saw any mailing list or has never 
> been replied to on a mailing list was **way** higher than merge window 
> commits.
>  - For some reason, the odds of a -rc commit to be targetted for -stable is 
> over 20%, while for merge window commits it's about 3%. I can't quite explain 
> why that happens, but this would suggest that -rc commits end up hurting 
> -stable pretty badly.
>
> 2. Maintainers need to stop writing patches, commiting them, and pushing them 
> in without reviews.
> In -rc cycles there is quite a large number of commits that were either 
> written by maintainers, commited, and merged upstream the same day. These 
> patches are very likely to introduce a new bug.
>
>
> I don't really have a proposal beyond "tighten up -rc cycles", but I think 
> it's a discussion worth having. We have enough data to show what parts of 
> kernel development work, and what parts are just hurting us.


Re: [LSF/MM TOPIC NOTES] x86 ZONE_DMA love

2018-04-29 Thread Julia Lawall
Here are some improved results, also taking into account the pci
functions.

julia

too small: drivers/gpu/drm/i915/i915_drv.c:1138: 30
too small: drivers/hwtracing/coresight/coresight-tmc.c:335: 0
too small: drivers/media/pci/sta2x11/sta2x11_vip.c:859: 29
too small: drivers/media/pci/sta2x11/sta2x11_vip.c:983: 26
too small: drivers/net/ethernet/broadcom/b44.c:2389: 30
too small: drivers/net/wan/wanxl.c:585: 28
too small: drivers/net/wan/wanxl.c:586: 28
too small: drivers/net/wireless/broadcom/b43/dma.c:1068: 30
too small: drivers/net/wireless/broadcom/b43legacy/dma.c:809: 30
too small: drivers/scsi/aacraid/commsup.c:1581: 31
too small: drivers/scsi/aacraid/linit.c:1651: 31
too small: drivers/usb/host/ehci-pci.c:127: 31
too small: sound/pci/ali5451/ali5451.c:2110: 31
too small: sound/pci/ali5451/ali5451.c:2111: 31
too small: sound/pci/als300.c:661: 28
too small: sound/pci/als300.c:662: 28
too small: sound/pci/als4000.c:874: 24
too small: sound/pci/als4000.c:875: 24
too small: sound/pci/azt3328.c:2421: 24
too small: sound/pci/azt3328.c:2422: 24
too small: sound/pci/emu10k1/emu10k1x.c:916: 28
too small: sound/pci/emu10k1/emu10k1x.c:917: 28
too small: sound/pci/es1938.c:1600: 24
too small: sound/pci/es1938.c:1601: 24
too small: sound/pci/es1968.c:2692: 28
too small: sound/pci/es1968.c:2693: 28
too small: sound/pci/ice1712/ice1712.c:2533: 28
too small: sound/pci/ice1712/ice1712.c:2534: 28
too small: sound/pci/maestro3.c:2557: 28
too small: sound/pci/maestro3.c:2558: 28
too small: sound/pci/sis7019.c:1328: 30
too small: sound/pci/sonicvibes.c:1262: 24
too small: sound/pci/sonicvibes.c:1263: 24
too small: sound/pci/trident/trident_main.c:3552: 30
too small: sound/pci/trident/trident_main.c:3553: 30
unknown: arch/x86/pci/sta2x11-fixup.c:169: STA2X11_AMBA_SIZE-1
unknown: arch/x86/pci/sta2x11-fixup.c:170: STA2X11_AMBA_SIZE-1
unknown: drivers/ata/sata_nv.c:762: pp->adma_dma_mask
unknown: drivers/char/agp/intel-gtt.c:1409: DMA_BIT_MASK(mask)
unknown: drivers/char/agp/intel-gtt.c:1413: DMA_BIT_MASK(mask)
unknown: drivers/crypto/ccree/cc_driver.c:260: dma_mask
unknown: drivers/dma/mmp_pdma.c:1094: pdev->dev->coherent_dma_mask
unknown: drivers/dma/pxa_dma.c:1375: op->dev.coherent_dma_mask
unknown: drivers/dma/xilinx/xilinx_dma.c:2634: DMA_BIT_MASK(addr_width)
unknown: drivers/gpu/drm/ati_pcigart.c:117: gart_info->table_mask
unknown: drivers/gpu/drm/msm/msm_drv.c:1132: ~0
unknown: drivers/gpu/drm/nouveau/nvkm/engine/device/tegra.c:313: 
DMA_BIT_MASK(tdev->func->iommu_bit)
unknown: drivers/gpu/host1x/dev.c:199: host->info->dma_mask
unknown: drivers/hwtracing/intel_th/core.c:379: parent->coherent_dma_mask
unknown: drivers/iommu/arm-smmu.c:1848: DMA_BIT_MASK(size)
unknown: drivers/media/pci/intel/ipu3/ipu3-cio2.c:1759: CIO2_DMA_MASK
unknown: drivers/media/platform/qcom/venus/core.c:186: core->res->dma_mask
unknown: drivers/message/fusion/mptbase.c:4599: ioc->dma_mask
unknown: drivers/message/fusion/mptbase.c:4600: ioc->dma_mask
unknown: drivers/net/ethernet/altera/altera_tse_main.c:1449: 
DMA_BIT_MASK(priv->dmaops->dmamask)
unknown: drivers/net/ethernet/altera/altera_tse_main.c:1450: 
DMA_BIT_MASK(priv->dmaops->dmamask)
unknown: drivers/net/ethernet/amazon/ena/ena_netdev.c:2455: 
DMA_BIT_MASK(dma_width)
unknown: drivers/net/ethernet/amazon/ena/ena_netdev.c:2461: 
DMA_BIT_MASK(dma_width)
unknown: drivers/net/ethernet/amd/pcnet32.c:1558: PCNET32_DMA_MASK
unknown: drivers/net/ethernet/amd/xgbe/xgbe-main.c:294: 
DMA_BIT_MASK(pdata->hw_feat.dma_width)
unknown: drivers/net/ethernet/broadcom/bnx2.c:8234: persist_dma_mask
unknown: drivers/net/ethernet/broadcom/tg3.c:17781: persist_dma_mask
unknown: drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c:315: old_mask
unknown: drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c:316: old_cmask
unknown: drivers/net/ethernet/sfc/efx.c:1298: dma_mask
unknown: drivers/net/ethernet/sfc/falcon/efx.c:1251: dma_mask
unknown: drivers/net/ethernet/synopsys/dwc-xlgmac-common.c:96: 
DMA_BIT_MASK(pdata->hw_feat.dma_width)
unknown: drivers/net/wireless/ath/wil6210/pcie_bus.c:299: 
DMA_BIT_MASK(dma_addr_size[i])
unknown: drivers/net/wireless/ath/wil6210/pmc.c:132: 
DMA_BIT_MASK(wil->dma_addr_size)
unknown: drivers/net/wireless/ath/wil6210/txrx.c:200: 
DMA_BIT_MASK(wil->dma_addr_size)
unknown: drivers/scsi/3w-.c:2260: TW_DMA_MASK
unknown: drivers/scsi/hptiop.c:1312: DMA_BIT_MASK(iop_ops->hw_dma_bit_mask)
unknown: drivers/scsi/megaraid/megaraid_sas_base.c:6036: consistent_mask
unknown: drivers/scsi/sym53c8xx_2/sym_glue.c:1315: DMA_DAC_MASK
unknown: drivers/usb/gadget/udc/bdc/bdc_pci.c:86: pci->dev.coherent_dma_mask
unknown: sound/pci/emu10k1/emu10k1_main.c:1910: emu->dma_mask



@initialize:ocaml@
@@

let clean s = String.concat "" (Str.split (Str.regexp " ") s)

let shorten s = List.nth (Str.split (Str.regexp "linux-next/") s) 1

let ios s =
  match Str.split_delim (Str.regexp "ULL") s with
[n;""] -> int_of_string n
  | _ -> int_of_string s

let number x = try ignore(ios x); 

  1   2   3   4   5   6   7   8   9   10   >