Re: [Cocci] linux-kernel janitorial RFP: Mark static arrays as const
On Wednesday, March 3, 2021, Bernd Petrovitsch
wrote:
> Hi all!
>
> On 02/03/2021 18:42, Joe Perches wrote:
> [...]
> > - For instance: (head -10 of the git grep for file statics)
> >
> > drivers/accessibility/speakup/keyhelp.c:18:static u_short masks[] = {
> 32, 16, 8, 4, 2, 1 };
> > drivers/accessibility/speakup/keyhelp.c:26:static u_char funcvals[] = {
> > drivers/accessibility/speakup/main.c:2059:static spkup_hand
> spkup_handler[] = {
> > drivers/accessibility/speakup/speakup_acntpc.c:35:static unsigned int
> synth_portlist[] = { 0x2a8, 0 };
> > drivers/accessibility/speakup/speakup_decpc.c:133:static int
> synth_portlist[] = { 0x340, 0x350, 0x240, 0x250, 0 };
> > drivers/accessibility/speakup/speakup_dectlk.c:110:static int
> ap_defaults[] = {122, 89, 155, 110, 208, 240, 200, 106, 306};
> > drivers/accessibility/speakup/speakup_dectlk.c:111:static int
> g5_defaults[] = {86, 81, 86, 84, 81, 80, 83, 83, 73};
> > drivers/accessibility/speakup/speakup_dtlk.c:34:static unsigned int
> synth_portlist[] = {
> > drivers/accessibility/speakup/speakup_keypc.c:34:static unsigned int
> synth_portlist[] = { 0x2a8, 0 };
> > drivers/acpi/ac.c:137:static enum power_supply_property ac_props[] = {
> >
> > For drivers/accessibility/speakup/keyhelp.c:18:static u_short masks[] =
> { 32, 16, 8, 4, 2, 1 };
>
> Looking at the examples: Just s/^static /static const / in the lines
> reported by the grep's above and see if it compiles (and save space)?
I did two reverts and reported at least one issue with blind
constification. Besides that we have a lot of data structures that require
to drop const spécifier and the consumer won’t actually know if it’s
possible to write there or not. I’m talking about driver data fields where
they are defined as type of kernel_ulong_t. So, first you need to fix that,
>
> MfG,
> Bernd
> --
> Bernd Petrovitsch Email : be...@petrovitsch.priv.at
> There is NO CLOUD, just other people's computers. - FSFE
> LUGA : http://www.luga.at
>
--
With Best Regards,
Andy Shevchenko
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] coccinelle: api/devm_platform_ioremap_resource: remove useless script
On Fri, Oct 25, 2019 at 12:40:52AM +0900, Masahiro Yamada wrote: > On Sun, Oct 20, 2019 at 7:13 AM Joe Perches wrote: > > On Sat, 2019-10-19 at 21:43 +0100, Marc Zyngier wrote: > Alexandre Belloni used > https://lore.kernel.org/lkml/9bbcce19c777583815c92ce3c2ff2...@www.loen.fr/ > as a reference, but this is not the output from coccicheck. > The patch author just created a wrong patch by hand. Exactly. Removal of the script is a mistake. Like I said before is a healing (incorrect by the way!) by symptoms. > The deleted semantic patch supports MODE=patch, > which creates a correct patch, and is useful. Right! -- With Best Regards, Andy Shevchenko ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [PATCH] coccinelle: api: detect unnecessary le16_to_cpu
On Mon, Jul 3, 2017 at 4:36 PM, Sebastian Reichel <sebastian.reic...@collabora.co.uk> wrote: > On Sat, Jul 01, 2017 at 09:28:10PM +0200, Julia Lawall wrote: > * drivers/gpio/gpio-pca953x.c (line 190-192) It has double conversion there: 1. LE CPU: Read as LE and converted to LE (no-op), so, just u16 2. BE CPU: Read as BE and converted to LE, makes it __le16 Looks like the conversion is not needed, only get_unaligned() is necessary. P.S. What about lines 244-245 there? I think they are no-op. Interesting that those two parts were added in quite different commits. -- With Best Regards, Andy Shevchenko ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [PATCH] coccinelle: api: detect unnecessary le16_to_cpu
On Mon, Jul 3, 2017 at 8:14 PM, Sebastian Reichel <sebastian.reic...@collabora.co.uk> wrote: > Hi, > > On Mon, Jul 03, 2017 at 07:37:59PM +0300, Andy Shevchenko wrote: >> On Mon, Jul 3, 2017 at 4:36 PM, Sebastian Reichel >> <sebastian.reic...@collabora.co.uk> wrote: >> > On Sat, Jul 01, 2017 at 09:28:10PM +0200, Julia Lawall wrote: >> >> > * drivers/gpio/gpio-pca953x.c (line 190-192) >> >> It has double conversion there: >> 1. LE CPU: Read as LE and converted to LE (no-op), so, just u16 >> 2. BE CPU: Read as BE and converted to LE, makes it __le16 >> >> Looks like the conversion is not needed, only get_unaligned() is necessary. >> >> P.S. What about lines 244-245 there? I think they are no-op. >> Interesting that those two parts were added in quite different >> commits. > > val[0] = (u16)ret & 0xFF; > val[1] = (u16)ret >> 8; > > looks like cpu_to_be16()? cpu_to_le16(). No-op on LE CPU. Perhaps they should be replaced by put_unaligned(). -- With Best Regards, Andy Shevchenko ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
Re: [Cocci] [PATCH] coccinelle: api: detect unnecessary le16_to_cpu
On Tue, Jul 4, 2017 at 12:11 PM, Julia Lawall <julia.law...@lip6.fr> wrote: > Here is a revised version (not a patch because it doesn't support all of > the various modes) and the results. It doesn't return anything beyond > what was mentioned in previous mails. > > For the following code: > > ret = i2c_smbus_read_word_data(chip->client, reg << 1); > val[0] = (u16)ret & 0xFF; > val[1] = (u16)ret >> 8; > > do we want to see: > > put_unaligned(val,i2c_smbus_read_word_data(chip->client, reg << 1)); If and only if the type of the pointer is a byte type (u8 *, char *, or alike) _and_ we try to use it as a provider for 16-bit value (2 bytes). -- With Best Regards, Andy Shevchenko ___ Cocci mailing list Cocci@systeme.lip6.fr https://systeme.lip6.fr/mailman/listinfo/cocci
[Cocci] First try of coccinelle, needs advice
Hi!
I played first time with coccinelle.
What I have tried to do is to find all locations where pm_runtime_get()
is not followed by pm_runtime_put() in case of error path.
cocci script roughly does what I need, though I have several questions:
- how to shrink it (I see few duplications there, no idea what is the
best approach to eliminate them)?
- how handle goto:s properly? I mean how to clearly choose goto path
over the rest?
- if you look at the result (see below) in almost all goto cases I need
to avoid adding pm_runtime_put*() variants since it seems the error
handling is broken (needs to shuffle error labels). How to achieve this?
- any other recommendations or links to some good examples or
documentation would be much appreciated
From 825dc60eaa42af81d935ad4b3cb995d533c17657 Mon Sep 17 00:00:00 2001
From: Andy Shevchenko <andriy.shevche...@linux.intel.com>
Date: Sat, 17 Jun 2017 19:38:20 +0300
Subject: [PATCH 1/1] pm_runtime_put: First try coccinelle
pm_runtime_get() should followed by pm_runtime_put() in case of error.
Signed-off-by: Andy Shevchenko <andriy.shevche...@linux.intel.com>
---
drivers/dma/tegra210-adma.c | 1 +
drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 1 +
drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 2 +
drivers/i2c/busses/i2c-omap.c | 3 +-
drivers/i2c/busses/i2c-tegra.c | 2 +
drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c | 1 +
drivers/media/platform/sti/delta/delta-v4l2.c | 1 +
drivers/net/can/xilinx_can.c| 2 +
drivers/pci/dwc/pci-dra7xx.c| 5 +--
drivers/pci/host/pcie-rcar.c| 4 +-
drivers/soc/ti/knav_dma.c | 1 +
drivers/spi/spi-tegra114.c | 2 +
drivers/spi/spi-tegra20-sflash.c| 1 +
drivers/spi/spi-tegra20-slink.c | 2 +
drivers/spi/spi-ti-qspi.c | 1 +
drivers/usb/core/hub.c | 1 +
scripts/coccinelle/api/pm_runtime_put.cocci | 57
+
17 files changed, 78 insertions(+), 9 deletions(-)
create mode 100644 scripts/coccinelle/api/pm_runtime_put.cocci
diff --git a/drivers/dma/tegra210-adma.c b/drivers/dma/tegra210-adma.c
index b10cbaa82ff5..ad0473ab2360 100644
--- a/drivers/dma/tegra210-adma.c
+++ b/drivers/dma/tegra210-adma.c
@@ -582,6 +582,7 @@ static int tegra_adma_alloc_chan_resources(struct
dma_chan *dc)
ret = pm_runtime_get_sync(tdc2dev(tdc));
if (ret < 0) {
free_irq(tdc->irq, tdc);
+ pm_runtime_put(tdc2dev(tdc));
return ret;
}
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
index ada45fdd0eae..c77cade43de6 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
@@ -659,6 +659,7 @@ int etnaviv_gpu_init(struct etnaviv_gpu *gpu)
ret = pm_runtime_get_sync(gpu->dev);
if (ret < 0) {
dev_err(gpu->dev, "Failed to enable GPU power
domain\n");
+ pm_runtime_put(gpu->dev);
return ret;
}
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
index 5d450332c2fd..f930ca75f615 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
@@ -505,6 +505,7 @@ static int vop_enable(struct drm_crtc *crtc)
ret = pm_runtime_get_sync(vop->dev);
if (ret < 0) {
dev_err(vop->dev, "failed to get pm runtime: %d\n",
ret);
+ pm_runtime_put(vop->dev);
return ret;
}
@@ -1418,6 +1419,7 @@ static int vop_initial(struct vop *vop)
ret = pm_runtime_get_sync(vop->dev);
if (ret < 0) {
dev_err(vop->dev, "failed to get pm runtime: %d\n",
ret);
+ pm_runtime_put(vop->dev);
return ret;
}
diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-
omap.c
index 1ebb5e947e0b..b7ceefe44beb 100644
--- a/drivers/i2c/busses/i2c-omap.c
+++ b/drivers/i2c/busses/i2c-omap.c
@@ -1438,11 +1438,10 @@ omap_i2c_probe(struct platform_device *pdev)
err_unuse_clocks:
omap_i2c_write_reg(omap, OMAP_I2C_CON_REG, 0);
+err_free_mem:
pm_runtime_dont_use_autosuspend(omap->dev);
pm_runtime_put_sync(omap->dev);
pm_runtime_disable(>dev);
-err_free_mem:
-
return r;
}
diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-
tegra.c
index 4af9bbae20df..e6d40bef0475 100644
--- a/drivers/i2c/busses/i2c-tegra.c
+++ b/drivers/i2c/busses/i2c-tegra.c
@@ -484,6 +484,7 @@ static int tegra_i2c_init(struct tegra_i2c_dev
*i2c_dev)
err = pm_runtime_get_sync(i2c_dev->dev);
if (err < 0) {
de
Re: [Cocci] [PATCH] scripts/coccinelle/api/gpiod_get_index.cocci: use gpiod_get variant when possible
On Sun, 2017-06-11 at 15:53 +0200, Julia Lawall wrote:
> Coccinelle script to use the shorter gpiod_get function when the third
> argument to a gpiod_get_index function is 0.
>
> This rule avoids modifying wrapper functions and cases where there is
> a
> series of gpiod_get_index calls.
Thanks for the script.
One caution nevertheless (I'm not an expert in coccinelle below, so,
can't tell if it's already there):
If there are two calls to gpio_get_index() with different indices, we
are not going to replace the one which uses 0.
>
> Signed-off-by: Julia Lawall <julia.law...@lip6.fr>
>
> ---
> scripts/coccinelle/api/gpiod_get_index.cocci | 92
> +++
> 1 file changed, 92 insertions(+)
>
> diff --git a/scripts/coccinelle/api/gpiod_get_index.cocci
> b/scripts/coccinelle/api/gpiod_get_index.cocci
> new file mode 100644
> index 000..a670a29
> --- /dev/null
> +++ b/scripts/coccinelle/api/gpiod_get_index.cocci
> @@ -0,0 +1,92 @@
> +/// Use the shorter gpiod_get function when the third argument to a
> +/// gpiod_get_index function is 0.
> +//# The change might not be desired when there is a series of calls,
> for 0, 1,
> +//# 2, etc. This rule checks for another such call subsequently
> only.
> +///
> +// Confidence: Moderate
> +// Copyright: (C) 2017 Julia Lawall, Inria. GPLv2.
> +// URL: http://coccinelle.lip6.fr/
> +// Options: --no-includes --include-headers
> +// Keywords: gpiod_get_index
> +
> +virtual patch
> +virtual context
> +virtual org
> +virtual report
> +
> +@initialize:python@
> +@@
> +
> +def not_wrapper(p):
> + return (p[0].current_element != "devm_gpiod_get_optional" and
> + p[0].current_element != "devm_gpiod_get" and
> + p[0].current_element != "gpiod_get_optional" and
> + p[0].current_element != "gpiod_get");
> +
> +@r depends on patch && !context && !org && !report@
> +expression e1,e2,e3,e4,e5,e6,n != 0;
> +position p : script:python() { not_wrapper(p) };
> +@@
> +
> +(
> +- devm_gpiod_get_index_optional@p
> ++ devm_gpiod_get_optional
> +|
> +- devm_gpiod_get_index@p
> ++ devm_gpiod_get
> +|
> +- gpiod_get_index_optional@p
> ++ gpiod_get_optional
> +|
> +- gpiod_get_index@p
> ++ gpiod_get
> +)
> + (e1,e2,
> +- 0,
> +e3)
> + ... when != devm_gpiod_get_index_optional(e4,e5,n,e6)
> + when != devm_gpiod_get_index(e4,e5,n,e6)
> + when != gpiod_get_index_optional(e4,e5,n,e6)
> + when != gpiod_get_index(e4,e5,n,e6)
> +
> +// --
> --
> +
> +@r_context depends on !patch && (context || org || report) forall@
> +expression e1, e2, e3, e4, e5, e6, n != 0;
> +position p: script:python () { not_wrapper(p) };
> +@@
> +
> +(
> +* devm_gpiod_get_index_optional@p
> +|
> +* devm_gpiod_get_index@p
> +|
> +* gpiod_get_index_optional@p
> +|
> +* gpiod_get_index@p
> +)
> + (e1,e2,
> +*0,
> +e3)
> + ... when != devm_gpiod_get_index_optional(e4,e5,n,e6)
> + when != devm_gpiod_get_index(e4,e5,n,e6)
> + when != gpiod_get_index_optional(e4,e5,n,e6)
> + when != gpiod_get_index(e4,e5,n,e6)
> +
> +// --
> --
> +
> +@script:python r_org depends on org@
> +p << r_context.p;
> +@@
> +
> +msg = "Use non _index variant in 0 case."
> +coccilib.org.print_todo(p[0], msg)
> +
> +// --
> --
> +
> +@script:python r_report depends on report@
> +p << r_context.p;
> +@@
> +
> +msg = "Use non _index variant in 0 case around line %s." %
> (p[0].line)
> +coccilib.report.print_report(p[0], msg)
>
--
Andy Shevchenko <andriy.shevche...@linux.intel.com>
Intel Finland Oy
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci
