Re: [0/2] powerpc/powernv/vas: Adjustments for two function implementations

[Julia Lawall]

On Tue, 16 Apr 2024, Christophe Leroy wrote:

>
>
> Le 16/04/2024 à 14:14, Markus Elfring a écrit :
> >> This is explicit in Kernel documentation:
> >>
> >> /**
> >>* kfree - free previously allocated memory
> >>* @object: pointer returned by kmalloc() or kmem_cache_alloc()
> >>*
> >>* If @object is NULL, no operation is performed.
> >>*/
> >>
> >> That's exactly the same behaviour as free() in libc.
> >>
> >> So Coccinelle should be fixed if it reports an error for that.
> >
> > Redundant function calls can occasionally be avoided accordingly,
> > can't they?
>
> Sure they can, but is that worth it here ?

Coccinelle does what the developer of the semantic patch tells it to do.
It doesn't spontaneously report errors for anything.

julia

[PATCH 06/11] powerpc/kexec_file: add missing of_node_put

[Julia Lawall]

for_each_node_with_property performs an of_node_get on each
iteration, so a break out of the loop requires an
of_node_put.

This was done using the Coccinelle semantic patch
iterators/for_each_child.cocci

Signed-off-by: Julia Lawall 

---
 arch/powerpc/kexec/file_load_64.c |8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff -u -p a/arch/powerpc/kexec/file_load_64.c 
b/arch/powerpc/kexec/file_load_64.c
--- a/arch/powerpc/kexec/file_load_64.c
+++ b/arch/powerpc/kexec/file_load_64.c
@@ -1138,11 +1138,15 @@ static int update_pci_dma_nodes(void *fd
continue;
 
ret = copy_property(fdt, pci_offset, dn, "ibm,dma-window");
-   if (ret < 0)
+   if (ret < 0) {
+   of_node_put(dn);
break;
+   }
ret = copy_property(fdt, pci_offset, dn, dmapropname);
-   if (ret < 0)
+   if (ret < 0) {
+   of_node_put(dn);
break;
+   }
}
 
return ret;

[PATCH 03/11] powerpc/powermac: add missing of_node_put

[Julia Lawall]

for_each_node_by_name performs an of_node_get on each
iteration, so a break out of the loop requires an
of_node_put.

This was done using the Coccinelle semantic patch
iterators/for_each_child.cocci

Signed-off-by: Julia Lawall 

---
 arch/powerpc/platforms/powermac/low_i2c.c |4 +++-
 arch/powerpc/platforms/powermac/smp.c |4 +++-
 2 files changed, 6 insertions(+), 2 deletions(-)

diff -u -p a/arch/powerpc/platforms/powermac/smp.c 
b/arch/powerpc/platforms/powermac/smp.c
--- a/arch/powerpc/platforms/powermac/smp.c
+++ b/arch/powerpc/platforms/powermac/smp.c
@@ -598,8 +598,10 @@ static void __init smp_core99_setup_i2c_
name = "Pulsar";
break;
}
-   if (pmac_tb_freeze != NULL)
+   if (pmac_tb_freeze != NULL) {
+   of_node_put(cc);
break;
+   }
}
if (pmac_tb_freeze != NULL) {
/* Open i2c bus for synchronous access */
diff -u -p a/arch/powerpc/platforms/powermac/low_i2c.c 
b/arch/powerpc/platforms/powermac/low_i2c.c
--- a/arch/powerpc/platforms/powermac/low_i2c.c
+++ b/arch/powerpc/platforms/powermac/low_i2c.c
@@ -925,8 +925,10 @@ static void __init smu_i2c_probe(void)
 
sz = sizeof(struct pmac_i2c_bus) + sizeof(struct smu_i2c_cmd);
bus = kzalloc(sz, GFP_KERNEL);
-   if (bus == NULL)
+   if (bus == NULL) {
+   of_node_put(busnode);
return;
+   }
 
bus->controller = controller;
bus->busnode = of_node_get(busnode);

[PATCH 00/11] add missing of_node_put

[Julia Lawall]

Add of_node_put on a break out of an of_node loop.

---

 arch/powerpc/kexec/file_load_64.c|8 ++--
 arch/powerpc/platforms/powermac/low_i2c.c|4 +++-
 arch/powerpc/platforms/powermac/smp.c|4 +++-
 drivers/bus/arm-cci.c|4 +++-
 drivers/genpd/ti/ti_sci_pm_domains.c |8 ++--
 drivers/gpu/drm/mediatek/mtk_disp_ovl_adaptor.c  |4 +++-
 drivers/gpu/drm/mediatek/mtk_drm_drv.c   |4 +++-
 drivers/media/platform/mediatek/mdp3/mtk-mdp3-comp.c |1 +
 drivers/mmc/host/atmel-mci.c |8 ++--
 drivers/net/ethernet/broadcom/asp2/bcmasp.c  |1 +
 drivers/soc/dove/pmu.c   |5 -
 drivers/thermal/thermal_of.c |8 ++--
 sound/soc/sh/rcar/core.c |1 +
 13 files changed, 46 insertions(+), 14 deletions(-)

Re: [PATCH v4 2/6] treewide: use prandom_u32_max() when possible

[Julia Lawall]

> >> @minus_one@
> >> expression FULL;
> >> @@
> >>
> >> - (get_random_int() & ((FULL) - 1)
> >> + prandom_u32_max(FULL)
> >
> >Ahh, well, okay, this is the example I mentioned above. Only works if
> >FULL is saturated. Any clever way to get coccinelle to prove that? Can
> >it look at the value of constants?
>
> I'm not sure if Cocci will do that without a lot of work. The literals trick 
> I used below would need a lot of fanciness. :)

If FULL is an arbitrary expression, it would not be easy to automate.  If
it is a constant then you can use python/ocaml to analyze its value.  But
if it's a #define constant then you would need a previous rule to match the
#define and find that value.

For LITERAL, I think you could just do constant int LITERAL; for the
metavariable declaration.

julia

Re: [PATCH] powerpc/kvm: fix repeated words in comments Delete the redundant word 'that'.

[Julia Lawall]

On Mon, 25 Jul 2022, Michael Ellerman wrote:

> wangjianli  writes:
> > Signed-off-by: wangjianli 
> > ---
> >  arch/powerpc/kvm/book3s_64_mmu_hv.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/arch/powerpc/kvm/book3s_64_mmu_hv.c 
> > b/arch/powerpc/kvm/book3s_64_mmu_hv.c
> > index 514fd45c1994..73c6db20cd8a 100644
> > --- a/arch/powerpc/kvm/book3s_64_mmu_hv.c
> > +++ b/arch/powerpc/kvm/book3s_64_mmu_hv.c
> > @@ -1601,7 +1601,7 @@ long kvm_vm_ioctl_resize_hpt_commit(struct kvm *kvm,
> >   * is valid, it is written to the HPT as if an H_ENTER with the
> >   * exact flag set was done.  When the invalid count is non-zero
> >   * in the header written to the stream, the kernel will make
> > - * sure that that many HPTEs are invalid, and invalidate them
> > + * sure that many HPTEs are invalid, and invalidate them
> >   * if not.
>
> The existing wording is correct:
>
>  "the kernel will make sure that ... that many HPTEs are invalid"

Maybe it would be better as "that the number of invalid HPTEs is the same
as the invalid count"?

julia

[PATCH] cxl: fix typo in comment

[Julia Lawall]

Spelling mistake (triple letters) in comment.
Detected with the help of Coccinelle.

Signed-off-by: Julia Lawall 

---
 include/misc/cxl.h |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/misc/cxl.h b/include/misc/cxl.h
index 0410412de16b..d8044299d654 100644
--- a/include/misc/cxl.h
+++ b/include/misc/cxl.h
@@ -30,7 +30,7 @@ unsigned int cxl_pci_to_cfg_record(struct pci_dev *dev);
 /*
  * Context lifetime overview:
  *
- * An AFU context may be inited and then started and stoppped multiple times
+ * An AFU context may be inited and then started and stopped multiple times
  * before it's released. ie.
  *- cxl_dev_context_init()
  *  - cxl_start_context()

Re: [PATCH] powerpc: fix typos in comments

[Julia Lawall]

On Thu, 5 May 2022, Joel Stanley wrote:

> On Sat, 30 Apr 2022 at 18:58, Julia Lawall  wrote:
> >
> > Various spelling mistakes in comments.
> > Detected with the help of Coccinelle.
> >
> > Signed-off-by: Julia Lawall 
>
> I read the patch and it appears that all of the corrections are good.
> Thanks for sending it Julia.
>
> I'm not sure that one mega patch is the right way to go or not, so
> I'll leave that to mpe.

I tried some other options, but wasn't satisfied with any of them.  But
just let me know if you want something else (by file? by subdirectory?)

julia

>
> Reviewed-by: Joel Stanley 
>
> >
> > ---
> >  arch/powerpc/boot/cuboot-hotfoot.c  |2 +-
> >  arch/powerpc/crypto/aes-spe-glue.c  |2 +-
> >  arch/powerpc/kernel/cputable.c  |2 +-
> >  arch/powerpc/kernel/dawr.c  |2 +-
> >  arch/powerpc/kernel/eeh.c   |4 ++--
> >  arch/powerpc/kernel/eeh_event.c |2 +-
> >  arch/powerpc/kernel/fadump.c|   10 +-
> >  arch/powerpc/kernel/module_32.c |2 +-
> >  arch/powerpc/kernel/module_64.c |4 ++--
> >  arch/powerpc/kernel/pci-common.c|2 +-
> >  arch/powerpc/kernel/pci_of_scan.c   |2 +-
> >  arch/powerpc/kernel/process.c   |4 ++--
> >  arch/powerpc/kernel/prom_init.c |2 +-
> >  arch/powerpc/kernel/ptrace/ptrace-view.c|2 +-
> >  arch/powerpc/kernel/rtas_flash.c|2 +-
> >  arch/powerpc/kernel/setup-common.c  |2 +-
> >  arch/powerpc/kernel/signal_64.c |2 +-
> >  arch/powerpc/kernel/smp.c   |2 +-
> >  arch/powerpc/kernel/time.c  |4 ++--
> >  arch/powerpc/kernel/watchdog.c  |2 +-
> >  arch/powerpc/kexec/core_64.c|2 +-
> >  arch/powerpc/kvm/book3s_64_mmu_hv.c |2 +-
> >  arch/powerpc/kvm/book3s_64_vio_hv.c |2 +-
> >  arch/powerpc/kvm/book3s_emulate.c   |2 +-
> >  arch/powerpc/kvm/book3s_hv_p9_entry.c   |2 +-
> >  arch/powerpc/kvm/book3s_hv_uvmem.c  |2 +-
> >  arch/powerpc/kvm/book3s_pr.c|2 +-
> >  arch/powerpc/kvm/book3s_xics.c  |2 +-
> >  arch/powerpc/kvm/book3s_xive.c  |6 +++---
> >  arch/powerpc/kvm/e500mc.c   |2 +-
> >  arch/powerpc/mm/book3s64/hash_pgtable.c |2 +-
> >  arch/powerpc/mm/book3s64/hash_utils.c   |4 ++--
> >  arch/powerpc/mm/book3s64/pgtable.c  |2 +-
> >  arch/powerpc/mm/book3s64/radix_pgtable.c|2 +-
> >  arch/powerpc/mm/book3s64/radix_tlb.c|2 +-
> >  arch/powerpc/mm/book3s64/slb.c  |4 ++--
> >  arch/powerpc/mm/init_64.c   |4 ++--
> >  arch/powerpc/mm/nohash/book3e_hugetlbpage.c |2 +-
> >  arch/powerpc/mm/nohash/kaslr_booke.c|2 +-
> >  arch/powerpc/mm/pgtable-frag.c  |2 +-
> >  arch/powerpc/perf/8xx-pmu.c |2 +-
> >  arch/powerpc/perf/core-book3s.c |6 +++---
> >  arch/powerpc/perf/imc-pmu.c |4 ++--
> >  arch/powerpc/perf/isa207-common.c   |6 +++---
> >  arch/powerpc/platforms/512x/clock-commonclk.c   |2 +-
> >  arch/powerpc/platforms/512x/mpc512x_shared.c|2 +-
> >  arch/powerpc/platforms/52xx/mpc52xx_common.c|2 +-
> >  arch/powerpc/platforms/52xx/mpc52xx_gpt.c   |2 +-
> >  arch/powerpc/platforms/52xx/mpc52xx_lpbfifo.c   |2 +-
> >  arch/powerpc/platforms/85xx/mpc85xx_cds.c   |2 +-
> >  arch/powerpc/platforms/86xx/gef_ppc9a.c |2 +-
> >  arch/powerpc/platforms/86xx/gef_sbc310.c|2 +-
> >  arch/powerpc/platforms/86xx/gef_sbc610.c|2 +-
> >  arch/powerpc/platforms/book3s/vas-api.c |2 +-
> >  arch/powerpc/platforms/cell/cbe_regs.c  |2 +-
> >  arch/powerpc/platforms/cell/iommu.c |2 +-
> >  arch/powerpc/platforms/cell/spider-pci.c|2 +-
> >  arch/powerpc/platforms/cell/spu_manage.c|2 +-
> >  arch/powerpc/platform

[PATCH] powerpc: fix typos in comments

[Julia Lawall]

Various spelling mistakes in comments.
Detected with the help of Coccinelle.

Signed-off-by: Julia Lawall 

---
 arch/powerpc/boot/cuboot-hotfoot.c  |2 +-
 arch/powerpc/crypto/aes-spe-glue.c  |2 +-
 arch/powerpc/kernel/cputable.c  |2 +-
 arch/powerpc/kernel/dawr.c  |2 +-
 arch/powerpc/kernel/eeh.c   |4 ++--
 arch/powerpc/kernel/eeh_event.c |2 +-
 arch/powerpc/kernel/fadump.c|   10 +-
 arch/powerpc/kernel/module_32.c |2 +-
 arch/powerpc/kernel/module_64.c |4 ++--
 arch/powerpc/kernel/pci-common.c|2 +-
 arch/powerpc/kernel/pci_of_scan.c   |2 +-
 arch/powerpc/kernel/process.c   |4 ++--
 arch/powerpc/kernel/prom_init.c |2 +-
 arch/powerpc/kernel/ptrace/ptrace-view.c|2 +-
 arch/powerpc/kernel/rtas_flash.c|2 +-
 arch/powerpc/kernel/setup-common.c  |2 +-
 arch/powerpc/kernel/signal_64.c |2 +-
 arch/powerpc/kernel/smp.c   |2 +-
 arch/powerpc/kernel/time.c  |4 ++--
 arch/powerpc/kernel/watchdog.c  |2 +-
 arch/powerpc/kexec/core_64.c|2 +-
 arch/powerpc/kvm/book3s_64_mmu_hv.c |2 +-
 arch/powerpc/kvm/book3s_64_vio_hv.c |2 +-
 arch/powerpc/kvm/book3s_emulate.c   |2 +-
 arch/powerpc/kvm/book3s_hv_p9_entry.c   |2 +-
 arch/powerpc/kvm/book3s_hv_uvmem.c  |2 +-
 arch/powerpc/kvm/book3s_pr.c|2 +-
 arch/powerpc/kvm/book3s_xics.c  |2 +-
 arch/powerpc/kvm/book3s_xive.c  |6 +++---
 arch/powerpc/kvm/e500mc.c   |2 +-
 arch/powerpc/mm/book3s64/hash_pgtable.c |2 +-
 arch/powerpc/mm/book3s64/hash_utils.c   |4 ++--
 arch/powerpc/mm/book3s64/pgtable.c  |2 +-
 arch/powerpc/mm/book3s64/radix_pgtable.c|2 +-
 arch/powerpc/mm/book3s64/radix_tlb.c|2 +-
 arch/powerpc/mm/book3s64/slb.c  |4 ++--
 arch/powerpc/mm/init_64.c   |4 ++--
 arch/powerpc/mm/nohash/book3e_hugetlbpage.c |2 +-
 arch/powerpc/mm/nohash/kaslr_booke.c|2 +-
 arch/powerpc/mm/pgtable-frag.c  |2 +-
 arch/powerpc/perf/8xx-pmu.c |2 +-
 arch/powerpc/perf/core-book3s.c |6 +++---
 arch/powerpc/perf/imc-pmu.c |4 ++--
 arch/powerpc/perf/isa207-common.c   |6 +++---
 arch/powerpc/platforms/512x/clock-commonclk.c   |2 +-
 arch/powerpc/platforms/512x/mpc512x_shared.c|2 +-
 arch/powerpc/platforms/52xx/mpc52xx_common.c|2 +-
 arch/powerpc/platforms/52xx/mpc52xx_gpt.c   |2 +-
 arch/powerpc/platforms/52xx/mpc52xx_lpbfifo.c   |2 +-
 arch/powerpc/platforms/85xx/mpc85xx_cds.c   |2 +-
 arch/powerpc/platforms/86xx/gef_ppc9a.c |2 +-
 arch/powerpc/platforms/86xx/gef_sbc310.c|2 +-
 arch/powerpc/platforms/86xx/gef_sbc610.c|2 +-
 arch/powerpc/platforms/book3s/vas-api.c |2 +-
 arch/powerpc/platforms/cell/cbe_regs.c  |2 +-
 arch/powerpc/platforms/cell/iommu.c |2 +-
 arch/powerpc/platforms/cell/spider-pci.c|2 +-
 arch/powerpc/platforms/cell/spu_manage.c|2 +-
 arch/powerpc/platforms/powermac/low_i2c.c   |2 +-
 arch/powerpc/platforms/powernv/eeh-powernv.c|   10 +-
 arch/powerpc/platforms/powernv/idle.c   |4 ++--
 arch/powerpc/platforms/powernv/ocxl.c   |2 +-
 arch/powerpc/platforms/powernv/opal-fadump.c|2 +-
 arch/powerpc/platforms/powernv/opal-lpc.c   |2 +-
 arch/powerpc/platforms/powernv/opal-memory-errors.c |2 +-
 arch/powerpc/platforms/powernv/pci-sriov.c  |2 +-
 arch/powerpc/platforms/ps3/mm.c |2 +-
 arch/powerpc/platforms/ps3/system-bus.c |2 +-
 arch/powerpc/platforms/pseries/eeh_pseries.c|2 +-
 arch/powerpc/platforms/pseries/iommu.c  |2 +-
 arch/powerpc/platforms/pseries/setup.c  |4 ++--
 arch/powerpc/platforms/pseries/vas-sysfs.c  |2 +-
 arch/powerpc/platforms/pseries/vas.c|2 +-
 arch/powerpc/sysdev/fsl_lbc.c   |2 +-
 arch/powerpc/sysdev/fsl_pci.c   |2 +-
 arch/powerpc/sysdev/ge/ge_pic.c |2 +-
 arch/powerpc/sysdev/mpic_msgr.c

[PATCH] powerpc/kexec: fix for_each_child.cocci warning

[Julia Lawall]

From: kernel test robot 

for_each_node_by_type should have of_node_put() before return.

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

CC: Sumera Priyadarsini 
Reported-by: kernel test robot 
Signed-off-by: kernel test robot 
---

The code seems to have been this way since the beginning of time.  Perhaps
the of_node_put is a no op for this code?

 core_64.c |4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

--- a/arch/powerpc/kexec/core_64.c
+++ b/arch/powerpc/kexec/core_64.c
@@ -64,8 +64,10 @@ int default_machine_kexec_prepare(struct
begin = image->segment[i].mem;
end = begin + image->segment[i].memsz;

-   if ((begin < high) && (end > low))
+   if ((begin < high) && (end > low)) {
+   of_node_put(node);
return -ETXTBSY;
+   }
}
}

[PATCH 4/4] ASoC: fsl: drop unneeded snd_soc_dai_set_drvdata

[Julia Lawall]

snd_soc_dai_set_drvdata is not needed when the set data comes from
snd_soc_dai_get_drvdata or dev_get_drvdata.  The problem was fixed
usingthe following semantic patch: (http://coccinelle.lip6.fr/)

// 
@@
expression x,y,e;
@@
x = dev_get_drvdata(y->dev)
... when != x = e
-   snd_soc_dai_set_drvdata(y,x);

@@
expression x,y,e;
@@
x = snd_soc_dai_get_drvdata(y)
... when != x = e
-   snd_soc_dai_set_drvdata(y,x);
// 

Signed-off-by: Julia Lawall 

---
 sound/soc/fsl/fsl_micfil.c |2 --
 sound/soc/fsl/fsl_sai.c|2 --
 sound/soc/fsl/fsl_xcvr.c   |1 -
 3 files changed, 5 deletions(-)

diff --git a/sound/soc/fsl/fsl_micfil.c b/sound/soc/fsl/fsl_micfil.c
index 8aedf6590b28..fd21017fa2bd 100644
--- a/sound/soc/fsl/fsl_micfil.c
+++ b/sound/soc/fsl/fsl_micfil.c
@@ -423,8 +423,6 @@ static int fsl_micfil_dai_probe(struct snd_soc_dai *cpu_dai)
return ret;
}
 
-   snd_soc_dai_set_drvdata(cpu_dai, micfil);
-
return 0;
 }
 
diff --git a/sound/soc/fsl/fsl_sai.c b/sound/soc/fsl/fsl_sai.c
index 5e65b456d3e2..8876d0ed37d9 100644
--- a/sound/soc/fsl/fsl_sai.c
+++ b/sound/soc/fsl/fsl_sai.c
@@ -727,8 +727,6 @@ static int fsl_sai_dai_probe(struct snd_soc_dai *cpu_dai)
snd_soc_dai_init_dma_data(cpu_dai, >dma_params_tx,
>dma_params_rx);
 
-   snd_soc_dai_set_drvdata(cpu_dai, sai);
-
return 0;
 }
 
diff --git a/sound/soc/fsl/fsl_xcvr.c b/sound/soc/fsl/fsl_xcvr.c
index dd228b421e2c..8a3bde718697 100644
--- a/sound/soc/fsl/fsl_xcvr.c
+++ b/sound/soc/fsl/fsl_xcvr.c
@@ -869,7 +869,6 @@ static int fsl_xcvr_dai_probe(struct snd_soc_dai *dai)
struct fsl_xcvr *xcvr = snd_soc_dai_get_drvdata(dai);
 
snd_soc_dai_init_dma_data(dai, >dma_prms_tx, >dma_prms_rx);
-   snd_soc_dai_set_drvdata(dai, xcvr);
 
snd_soc_add_dai_controls(dai, _xcvr_mode_kctl, 1);
snd_soc_add_dai_controls(dai, _xcvr_arc_mode_kctl, 1);

Documentation/powerpc: Ultravisor API

[Julia Lawall]

The file Documentation/powerpc/ultravisor.rst contains:

Only valid value(s) in ``flags`` are:

* H_PAGE_IN_SHARED which indicates that the page is to be shared
  with the Ultravisor.

* H_PAGE_IN_NONSHARED indicates that the UV is not anymore
  interested in the page. Applicable if the page is a shared page.

The flag H_PAGE_IN_SHARED exists in the Linux kernel
(arch/powerpc/include/asm/hvcall.h), but the flag H_PAGE_IN_NONSHARED does
not.  Should the documentation be changed in some way?

julia

Re: [PATCH] powerpc/spufs: adjust list element pointer type

[Julia Lawall]

On Fri, 8 May 2020, Jeremy Kerr wrote:

> Hi Julia,
>
> > Other uses of >aff_list_head, eg in spufs_assert_affinity, indicate
> > that the list elements have type spu_context, not spu as used here.  Change
> > the type of tmp accordingly.
>
> Looks good to me; we could even use ctx there, rather than the separate
> tmp variable.

I thought about that, but it seemed a little bit abusive, since ctx is
used in an iteration over another list.  But if you prefer that I can
change it.

julia

>
> Reviewed-by: Jeremy Kerr 
>
> Cheers,
>
>
> Jeremy
>
>

[PATCH] powerpc/spufs: adjust list element pointer type

[Julia Lawall]

Other uses of >aff_list_head, eg in spufs_assert_affinity, indicate
that the list elements have type spu_context, not spu as used here.  Change
the type of tmp accordingly.

This has no impact on the execution, because tmp is not used in the body of
the loop.

Fixes: c5fc8d2a92461 ("[CELL] cell: add placement computation for scheduling of 
affinity contexts")
Signed-off-by: Julia Lawall 

---
 arch/powerpc/platforms/cell/spufs/sched.c |3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/powerpc/platforms/cell/spufs/sched.c 
b/arch/powerpc/platforms/cell/spufs/sched.c
index f18d5067cd0f..487fcb47f10d 100644
--- a/arch/powerpc/platforms/cell/spufs/sched.c
+++ b/arch/powerpc/platforms/cell/spufs/sched.c
@@ -344,8 +344,7 @@ static struct spu *aff_ref_location(struct spu_context 
*ctx, int mem_aff,
 static void aff_set_ref_point_location(struct spu_gang *gang)
 {
int mem_aff, gs, lowest_offset;
-   struct spu_context *ctx;
-   struct spu *tmp;
+   struct spu_context *tmp, *ctx;
 
mem_aff = gang->aff_ref_ctx->flags & SPU_CREATE_AFFINITY_MEM;
lowest_offset = 0;

Re: [PATCH 0/4] use mmgrab

[Julia Lawall]

On Fri, 3 Jan 2020, Dan Carpenter wrote:

> On Sun, Dec 29, 2019 at 04:42:54PM +0100, Julia Lawall wrote:
> > Mmgrab was introduced in commit f1f1007644ff ("mm: add new mmgrab()
> > helper") and most of the kernel was updated to use it. Update a few
> > remaining files.
>
> I wonder if there is an automatic way to generate these kind of
> Coccinelle scripts which use inlines instead of open coding.  Like maybe
> make a list of one line functions, and then auto generate a recipe.  Or
> the mmgrab() function could have multiple lines if the first few were
> just sanity checks for NULL or something...

I tried this at one point (10 years ago!):

https://pages.lip6.fr/Julia.Lawall/acp4is09-lawall.pdf

Perhaps it is worth reviving.

julia

[PATCH 10/10] powerpc/powernv: use resource_size

[Julia Lawall]

Use resource_size rather than a verbose computation on
the end and start fields.

The semantic patch that makes these changes is as follows:
(http://coccinelle.lip6.fr/)


@@ struct resource ptr; @@
- (ptr.end - ptr.start + 1)
+ resource_size()


Signed-off-by: Julia Lawall 

---
 arch/powerpc/platforms/powernv/pci-ioda.c |4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c 
b/arch/powerpc/platforms/powernv/pci-ioda.c
index da1068a9c263..364140145ce0 100644
--- a/arch/powerpc/platforms/powernv/pci-ioda.c
+++ b/arch/powerpc/platforms/powernv/pci-ioda.c
@@ -792,7 +792,7 @@ static int pnv_ioda_deconfigure_pe(struct pnv_phb *phb, 
struct pnv_ioda_pe *pe)
fcomp = OPAL_IGNORE_RID_FUNCTION_NUMBER;
parent = pe->pbus->self;
if (pe->flags & PNV_IODA_PE_BUS_ALL)
-   count = pe->pbus->busn_res.end - 
pe->pbus->busn_res.start + 1;
+   count = resource_size(>pbus->busn_res);
else
count = 1;
 
@@ -874,7 +874,7 @@ static int pnv_ioda_configure_pe(struct pnv_phb *phb, 
struct pnv_ioda_pe *pe)
fcomp = OPAL_IGNORE_RID_FUNCTION_NUMBER;
parent = pe->pbus->self;
if (pe->flags & PNV_IODA_PE_BUS_ALL)
-   count = pe->pbus->busn_res.end - 
pe->pbus->busn_res.start + 1;
+   count = resource_size(>pbus->busn_res);
else
count = 1;
 

[PATCH 05/10] powerpc/83xx: use resource_size

[Julia Lawall]

Use resource_size rather than a verbose computation on
the end and start fields.

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


@@ struct resource ptr; @@
- (ptr.end - ptr.start + 1)
+ resource_size()


Signed-off-by: Julia Lawall 

---
 arch/powerpc/platforms/83xx/km83xx.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/platforms/83xx/km83xx.c 
b/arch/powerpc/platforms/83xx/km83xx.c
index 3d89569e9e71..ada42f03915a 100644
--- a/arch/powerpc/platforms/83xx/km83xx.c
+++ b/arch/powerpc/platforms/83xx/km83xx.c
@@ -63,7 +63,7 @@ static void quirk_mpc8360e_qe_enet10(void)
return;
}
 
-   base = ioremap(res.start, res.end - res.start + 1);
+   base = ioremap(res.start, resource_size());
 
/*
 * set output delay adjustments to default values according

[PATCH 00/10] use resource_size

[Julia Lawall]

Use resource_size rather than a verbose computation on
the end and start fields.

The semantic patch that makes these changes is as follows:
(http://coccinelle.lip6.fr/)


@@
struct resource ptr;
@@

- ((ptr.end) - (ptr.start) + 1)
+ resource_size()

@@
struct resource *ptr;
@@

- ((ptr->end) - (ptr->start) + 1)
+ resource_size(ptr)

@@
struct resource ptr;
@@

- ((ptr.end) + 1 - (ptr.start))
+ resource_size()

@@
struct resource *ptr;
@@

- ((ptr->end) + 1 - (ptr->start))
+ resource_size(ptr)


---

 arch/mips/kernel/setup.c  |6 ++
 arch/powerpc/platforms/83xx/km83xx.c  |2 +-
 arch/powerpc/platforms/powernv/pci-ioda.c |4 ++--
 arch/x86/kernel/crash.c   |2 +-
 drivers/net/ethernet/freescale/fman/mac.c |4 ++--
 drivers/usb/gadget/udc/omap_udc.c |6 +++---
 drivers/video/fbdev/cg14.c|3 +--
 drivers/video/fbdev/s1d13xxxfb.c  |   16 
 sound/drivers/ml403-ac97cr.c  |4 +---
 sound/soc/sof/imx/imx8.c  |3 +--
 10 files changed, 22 insertions(+), 28 deletions(-)

[PATCH 09/16] powerpc/mpic: constify copied structure

[Julia Lawall]

The mpic_ipi_chip and mpic_irq_ht_chip structures are only copied
into other structures, so make them const.

The opportunity for this change was found using Coccinelle.

Signed-off-by: Julia Lawall 

---
 arch/powerpc/sysdev/mpic.c |4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/sysdev/mpic.c b/arch/powerpc/sysdev/mpic.c
index 934a77324f6b..a3a72b780e67 100644
--- a/arch/powerpc/sysdev/mpic.c
+++ b/arch/powerpc/sysdev/mpic.c
@@ -964,7 +964,7 @@ static struct irq_chip mpic_irq_chip = {
 };
 
 #ifdef CONFIG_SMP
-static struct irq_chip mpic_ipi_chip = {
+static const struct irq_chip mpic_ipi_chip = {
.irq_mask   = mpic_mask_ipi,
.irq_unmask = mpic_unmask_ipi,
.irq_eoi= mpic_end_ipi,
@@ -978,7 +978,7 @@ static struct irq_chip mpic_tm_chip = {
 };
 
 #ifdef CONFIG_MPIC_U3_HT_IRQS
-static struct irq_chip mpic_irq_ht_chip = {
+static const struct irq_chip mpic_irq_ht_chip = {
.irq_startup= mpic_startup_ht_irq,
.irq_shutdown   = mpic_shutdown_ht_irq,
.irq_mask   = mpic_mask_irq,

[PATCH 00/16] constify copied structure

[Julia Lawall]

Make const static structures that are just copied into other structures.

The semantic patch that detects the opportunity for this change is as
follows: (http://coccinelle.lip6.fr/)


@r disable optional_qualifier@
identifier i,j;
position p;
@@
static struct i j@p = { ... };

@upd@
position p1;
identifier r.j;
expression e;
@@
e = j@p1

@ref@
position p2 != {r.p,upd.p1};
identifier r.j;
@@
j@p2

@script:ocaml depends on upd && !ref@
i << r.i;
j << r.j;
p << r.p;
@@
if j = (List.hd p).current_element
then Coccilib.print_main i p


---

 arch/powerpc/sysdev/mpic.c  |4 ++--
 drivers/gpu/drm/bridge/synopsys/dw-hdmi-ahb-audio.c |2 +-
 drivers/media/i2c/mt9v111.c |2 +-
 drivers/media/platform/davinci/isif.c   |2 +-
 drivers/media/usb/cx231xx/cx231xx-dvb.c |2 +-
 drivers/media/usb/dvb-usb-v2/anysee.c   |4 ++--
 drivers/pinctrl/nuvoton/pinctrl-npcm7xx.c   |2 +-
 drivers/pinctrl/qcom/pinctrl-ssbi-gpio.c|2 +-
 drivers/pinctrl/qcom/pinctrl-ssbi-mpp.c |2 +-
 drivers/ptp/ptp_clockmatrix.c   |2 +-
 drivers/usb/gadget/udc/atmel_usba_udc.c |2 +-
 drivers/video/fbdev/sa1100fb.c  |2 +-
 net/sunrpc/xdr.c|2 +-
 sound/isa/ad1816a/ad1816a_lib.c |2 +-
 sound/pci/hda/hda_controller.c  |2 +-
 sound/soc/qcom/qdsp6/q6asm-dai.c|2 +-
 16 files changed, 18 insertions(+), 18 deletions(-)

[PATCH 1/4] misc: cxl: use mmgrab

[Julia Lawall]

Mmgrab was introduced in commit f1f1007644ff ("mm: add new mmgrab()
helper") and most of the kernel was updated to use it. Update a
remaining file.

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


@@ expression e; @@
- atomic_inc(>mm_count);
+ mmgrab(e);


Signed-off-by: Julia Lawall 

---
 drivers/misc/cxl/context.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/misc/cxl/context.c b/drivers/misc/cxl/context.c
index aed9c445d1e2..fb2eff69e449 100644
--- a/drivers/misc/cxl/context.c
+++ b/drivers/misc/cxl/context.c
@@ -352,7 +352,7 @@ void cxl_context_free(struct cxl_context *ctx)
 void cxl_context_mm_count_get(struct cxl_context *ctx)
 {
if (ctx->mm)
-   atomic_inc(>mm->mm_count);
+   mmgrab(ctx->mm);
 }
 
 void cxl_context_mm_count_put(struct cxl_context *ctx)

[PATCH 0/4] use mmgrab

[Julia Lawall]

Mmgrab was introduced in commit f1f1007644ff ("mm: add new mmgrab()
helper") and most of the kernel was updated to use it. Update a few
remaining files.

---

 arch/openrisc/kernel/smp.c  |2 +-
 drivers/misc/cxl/context.c  |2 +-
 drivers/vfio/pci/vfio_pci_nvlink2.c |2 +-
 drivers/vfio/vfio_iommu_spapr_tce.c |2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

Re: Coccinelle: Checking of_node_put() calls with SmPL

[Julia Lawall]

On Thu, 11 Jul 2019, wen.yan...@zte.com.cn wrote:

> > > we developed a coccinelle script to detect such problems.
> >
> > Would you find the implementation of the function “dt_init_idle_driver”
> > suspicious according to discussed source code search patterns?
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/cpuidle/dt_idle_states.c?id=e9a83bd2322035ed9d7dcf35753d3f984d76c6a5#n208
> > https://elixir.bootlin.com/linux/v5.2/source/drivers/cpuidle/dt_idle_states.c#L208
> >
> >
> > > This script is still being improved.
> >
> > Will corresponding software development challenges become more interesting?
>
> Hello Markus,
> This is the simplified code pattern for it:
>
> 172 for (i = 0; ; i++) {
> 173 state_node = of_parse_phandle(...); ---> Obtain here
> ...
> 177 match_id = of_match_node(matches, state_node);
> 178 if (!match_id) {
> 179 err = -ENODEV;
> 180 break; --->  Jump out of 
> the loop without releasing it
> 181 }
> 182
> 183 if (!of_device_is_available(state_node)) {
> 184 of_node_put(state_node);
> 185 continue;--->  Release the 
> object references within a loop
> 186 }
> ...
> 208 of_node_put(state_node);  -->  Release the object 
> references within a loop
> 209 }
> 210
> 211 of_node_put(state_node);   -->There may be double free 
> here.
>
> This code pattern is very interesting and the coccinelle software should also 
> recognize this pattern.

In my experience, when you start looking at these of_node_put things, all
sorts of strange things appear...

julia

[PATCH 03/12] PowerPC-83xx: add missing of_node_put after of_device_is_available

[Julia Lawall]

Add an of_node_put when a tested device node is not available.

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

// 
@@
identifier f;
local idexpression e;
expression x;
@@

e = f(...);
... when != of_node_put(e)
when != x = e
when != e = x
when any
if (<+...of_device_is_available(e)...+>) {
  ... when != of_node_put(e)
(
  return e;
|
+ of_node_put(e);
  return ...;
)
}
// 

Fixes: c026c98739c7e ("powerpc/83xx: Do not configure or probe disabled FSL DR 
USB controllers")
Signed-off-by: Julia Lawall 

---
 arch/powerpc/platforms/83xx/usb.c |4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff -u -p a/arch/powerpc/platforms/83xx/usb.c 
b/arch/powerpc/platforms/83xx/usb.c
--- a/arch/powerpc/platforms/83xx/usb.c
+++ b/arch/powerpc/platforms/83xx/usb.c
@@ -221,8 +221,10 @@ int mpc837x_usb_cfg(void)
int ret = 0;
 
np = of_find_compatible_node(NULL, NULL, "fsl-usb2-dr");
-   if (!np || !of_device_is_available(np))
+   if (!np || !of_device_is_available(np)) {
+   of_node_put(np);
return -ENODEV;
+   }
prop = of_get_property(np, "phy_type", NULL);
 
if (!prop || (strcmp(prop, "ulpi") && strcmp(prop, "serial"))) {

[PATCH 00/12] add missing of_node_put after of_device_is_available

[Julia Lawall]

Failure of of_device_is_available implies that the device node
should be put, if it is not used otherwise.

---

 arch/arm/mach-omap2/display.c|4 +++-
 arch/powerpc/platforms/83xx/usb.c|4 +++-
 drivers/bus/arm-cci.c|4 +++-
 drivers/cpufreq/armada-8k-cpufreq.c  |4 +++-
 drivers/crypto/amcc/crypto4xx_trng.c |4 +++-
 drivers/firmware/psci.c  |4 +++-
 drivers/gpu/drm/omapdrm/dss/omapdss-boot-init.c  |4 +++-
 drivers/gpu/drm/tegra/rgb.c  |4 +++-
 drivers/phy/tegra/xusb.c |4 +++-
 drivers/soc/amlogic/meson-gx-socinfo.c   |4 +++-
 drivers/tee/optee/core.c |4 +++-
 drivers/video/fbdev/omap2/omapfb/dss/omapdss-boot-init.c |4 +++-
 12 files changed, 36 insertions(+), 12 deletions(-)

Re: [PATCH -next] powerpc/pseries: Fix platform_no_drv_owner.cocci warnings

[Julia Lawall]

On Mon, 18 Feb 2019, YueHaibing wrote:

> Remove .owner field if calls are used which set it automatically
> Generated by: scripts/coccinelle/api/platform_no_drv_owner.cocci
>
> Signed-off-by: YueHaibing 
> ---
>  arch/powerpc/platforms/pseries/papr_scm.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/platforms/pseries/papr_scm.c 
> b/arch/powerpc/platforms/pseries/papr_scm.c
> index bba281b1fe1b..e3f5c1a01950 100644
> --- a/arch/powerpc/platforms/pseries/papr_scm.c
> +++ b/arch/powerpc/platforms/pseries/papr_scm.c
> @@ -21,6 +21,7 @@
>(1ul << ND_CMD_GET_CONFIG_DATA) | \
>(1ul << ND_CMD_SET_CONFIG_DATA))
>
> +

No need to add a blank line.

julia

>  struct papr_scm_priv {
>   struct platform_device *pdev;
>   struct device_node *dn;
> @@ -358,7 +359,6 @@ static struct platform_driver papr_scm_driver = {
>   .remove = papr_scm_remove,
>   .driver = {
>   .name = "papr_scm",
> - .owner = THIS_MODULE,
>   .of_match_table = papr_scm_match,
>   },
>  };
>
>
>
>

Re: [PATCH] powerpc/ipic: Fix a bounds check in ipic_set_priority()

[Julia Lawall]

On Thu, 6 Dec 2018, Christophe LEROY wrote:

>
>
> Le 05/12/2018 à 04:26, Michael Ellerman a écrit :
> > Hi Dan,
> >
> > Thanks for the patch.
> >
> > Dan Carpenter  writes:
> > > The ipic_info[] array only has 95 elements so I have made the bounds
> > > check smaller to prevent a read overflow.  It was Smatch that found
> > > this issue:
> > >
> > >  arch/powerpc/sysdev/ipic.c:784 ipic_set_priority()
> > >  error: buffer overflow 'ipic_info' 95 <= 127
> > >
> > > Signed-off-by: Dan Carpenter 
> > > ---
> > > I wasn't able to find any callers of this code.  Maybe we removed the
> > > last one in commit b9f0f1bb2bca ("[POWERPC] Adapt ipic driver to new
> > > host_ops interface, add set_irq_type to set IRQ sense").  So perhaps we
> > > should just remove it.  I'm not really comfortable doing that myself,
> > > because I don't know the code well enough and can't build test
> > > it properly.
> >
> > Hah wow, last usage removed in 2006!
> >
> > I don't see any mention of it since then, so I'll remove it. If it
> > breaks something we can put it back.
> >
> > Can smatch help us find things like this that are defined non-static but
> > never used?
> >
>
> I think we have to do that carrefully. Some of those functions might be used
> by out-of-tree boards.
>
> I'm thinking especially at ipic_get_mcp_status() and ipic_set_mcp_status().
> They are used in my 832x boards's machine check handler to know when a machine
> check is a timeout from the 832x watchdog.

The message I have gotten in the past is that the Linux kernel doesn't
support code that is not used in the Linux kernel.  However, if I were to
do this, I would send the code to the individual maintainers, who
presumably would know what is actually needed and what is not.

Perhaps a good sanity check would be if the code has been used in the
past.  If there was a use in the past that has been removed, then perhaps
it is more likely that the function was intended for internal kernel use
rather than the case that you are describing.

julia

Re: [PATCH] powerpc/ipic: Fix a bounds check in ipic_set_priority()

[Julia Lawall]

On Wed, 5 Dec 2018, Michael Ellerman wrote:

> Julia Lawall  writes:
> > On Wed, 5 Dec 2018, Michael Ellerman wrote:
> >
> >> Hi Dan,
> >>
> >> Thanks for the patch.
> >>
> >> Dan Carpenter  writes:
> >> > The ipic_info[] array only has 95 elements so I have made the bounds
> >> > check smaller to prevent a read overflow.  It was Smatch that found
> >> > this issue:
> >> >
> >> > arch/powerpc/sysdev/ipic.c:784 ipic_set_priority()
> >> > error: buffer overflow 'ipic_info' 95 <= 127
> >> >
> >> > Signed-off-by: Dan Carpenter 
> >> > ---
> >> > I wasn't able to find any callers of this code.  Maybe we removed the
> >> > last one in commit b9f0f1bb2bca ("[POWERPC] Adapt ipic driver to new
> >> > host_ops interface, add set_irq_type to set IRQ sense").  So perhaps we
> >> > should just remove it.  I'm not really comfortable doing that myself,
> >> > because I don't know the code well enough and can't build test
> >> > it properly.
> >>
> >> Hah wow, last usage removed in 2006!
> >>
> >> I don't see any mention of it since then, so I'll remove it. If it
> >> breaks something we can put it back.
> >>
> >> Can smatch help us find things like this that are defined non-static but
> >> never used?
> >
> > I wrote a Coccinelle script for this, that just uses grep.  Of course the
> > results need checking because uses can be constructed within macros using
> > #.
>
> That would be cool. I can't immediately see it in scripts/coccinelle, is
> it somewhere else?

No, it needs improvement...  I'll try to do something with it soon.  I
don't think it  is well suited to scrips/coccinelle, because it needs to
know where the kernel tree is to do the grep.

julia

>
> > Are things that are defined static but are never used useful to keep
> > around?
>
> No, but the compiler will usually tell us about them via -Wunused-function.
>
> cheers
>

Re: [PATCH] powerpc/ipic: Fix a bounds check in ipic_set_priority()

[Julia Lawall]

On Wed, 5 Dec 2018, Michael Ellerman wrote:

> Hi Dan,
>
> Thanks for the patch.
>
> Dan Carpenter  writes:
> > The ipic_info[] array only has 95 elements so I have made the bounds
> > check smaller to prevent a read overflow.  It was Smatch that found
> > this issue:
> >
> > arch/powerpc/sysdev/ipic.c:784 ipic_set_priority()
> > error: buffer overflow 'ipic_info' 95 <= 127
> >
> > Signed-off-by: Dan Carpenter 
> > ---
> > I wasn't able to find any callers of this code.  Maybe we removed the
> > last one in commit b9f0f1bb2bca ("[POWERPC] Adapt ipic driver to new
> > host_ops interface, add set_irq_type to set IRQ sense").  So perhaps we
> > should just remove it.  I'm not really comfortable doing that myself,
> > because I don't know the code well enough and can't build test
> > it properly.
>
> Hah wow, last usage removed in 2006!
>
> I don't see any mention of it since then, so I'll remove it. If it
> breaks something we can put it back.
>
> Can smatch help us find things like this that are defined non-static but
> never used?

I wrote a Coccinelle script for this, that just uses grep.  Of course the
results need checking because uses can be constructed within macros using
#.

Are things that are defined static but are never used useful to keep
around?

julia

Re: [PATCH v2 1/2] treewide: remove unused address argument from pte_alloc functions

[Julia Lawall]

> I wrote something like this as below but it failed to compile, Julia any
> suggestions on how to express this?
>
> @pte_alloc_func_proto depends on patch exists@
> type T1, T2, T3, T4;
> identifier fn =~
> "^(__pte_alloc|pte_alloc_one|pte_alloc|__pte_alloc_kernel|pte_alloc_one_kernel)$";
> @@
>
> (
> - T3 fn(T1, T2);
> + T3 fn(T1);
> |
> - T3 fn(T1, T2, T4);
> + T3 fn(T1, T2);
> )

What goes wrong?  It seems fine to me.

julia

Re: [PATCH v5 2/4] resource: Use list_head to link sibling resource

[Julia Lawall]

This looks wrong.  After a list iterator, the index variable points to a
dummy structure.

julia

url:
https://github.com/0day-ci/linux/commits/Baoquan-He/resource-Use-list_head-to-link-sibling-resource/20180612-113600
:: branch date: 7 hours ago
:: commit date: 7 hours ago

>> kernel/resource.c:265:17-20: ERROR: invalid reference to the index variable 
>> of the iterator on line 253

# 
https://github.com/0day-ci/linux/commit/e906f15906750a86913ba2b1f08bad99129d3dfc
git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout e906f15906750a86913ba2b1f08bad99129d3dfc
vim +265 kernel/resource.c

^1da177e4 Linus Torvalds 2005-04-16  247
5eeec0ec9 Yinghai Lu 2009-12-22  248  static void 
__release_child_resources(struct resource *r)
5eeec0ec9 Yinghai Lu 2009-12-22  249  {
e906f1590 Baoquan He 2018-06-12  250struct resource *tmp, *next;
5eeec0ec9 Yinghai Lu 2009-12-22  251resource_size_t size;
5eeec0ec9 Yinghai Lu 2009-12-22  252
e906f1590 Baoquan He 2018-06-12 @253list_for_each_entry_safe(tmp, 
next, >child, sibling) {
5eeec0ec9 Yinghai Lu 2009-12-22  254tmp->parent = NULL;
e906f1590 Baoquan He 2018-06-12  255
INIT_LIST_HEAD(>sibling);
5eeec0ec9 Yinghai Lu 2009-12-22  256
__release_child_resources(tmp);
5eeec0ec9 Yinghai Lu 2009-12-22  257
5eeec0ec9 Yinghai Lu 2009-12-22  258printk(KERN_DEBUG 
"release child resource %pR\n", tmp);
5eeec0ec9 Yinghai Lu 2009-12-22  259/* need to restore 
size, and keep flags */
5eeec0ec9 Yinghai Lu 2009-12-22  260size = 
resource_size(tmp);
5eeec0ec9 Yinghai Lu 2009-12-22  261tmp->start = 0;
5eeec0ec9 Yinghai Lu 2009-12-22  262tmp->end = size - 1;
5eeec0ec9 Yinghai Lu 2009-12-22  263}
e906f1590 Baoquan He 2018-06-12  264
e906f1590 Baoquan He 2018-06-12 @265INIT_LIST_HEAD(>child);
5eeec0ec9 Yinghai Lu 2009-12-22  266  }
5eeec0ec9 Yinghai Lu 2009-12-22  267

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

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

[Julia Lawall]

On Mon, 4 Jun 2018, Dan Carpenter wrote:

> On Mon, Jun 04, 2018 at 10:25:14PM +0900, Julia Lawall wrote:
> >
> >
> > 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.
>
> I feel like it should warn about all of those because people shouldn't
> be submitting unfinished written code to the kernel.  Coccinelle is a
> lot better for this than Smatch is because it's pre-processor stuff.

OK, maybe I can report these in the next few days.

thanks,
julia

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

[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 4/5] pci/hotplug/pnv-php: add missing of_node_put

[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 0/5] add missing of_node_put

[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(-)

Re: [PATCH] crypto/nx: fix spelling mistake: "availavle" -> "available"

[Julia Lawall]

On Tue, 14 Nov 2017, Colin King wrote:

> From: Colin Ian King 
>
> Trivial fix to spelling mistake in pr_err error message text
>
> Signed-off-by: Colin Ian King 
> ---
>  drivers/crypto/nx/nx-842-powernv.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/crypto/nx/nx-842-powernv.c 
> b/drivers/crypto/nx/nx-842-powernv.c
> index f2246a5abcf6..9c01d11f255a 100644
> --- a/drivers/crypto/nx/nx-842-powernv.c
> +++ b/drivers/crypto/nx/nx-842-powernv.c
> @@ -744,7 +744,7 @@ static int nx842_open_percpu_txwins(void)
>
>   if (!per_cpu(cpu_txwin, i)) {
>   /* shoudn't happen, Each chip will have NX engine */

It could be nice to fix the comment as well: shoud -> should

julia

> - pr_err("NX engine is not availavle for CPU %d\n", i);
> + pr_err("NX engine is not available for CPU %d\n", i);
>   return -EINVAL;
>   }
>   }
> --
> 2.14.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

RE: Adjusting further size determinations?

[Julia Lawall]

On Wed, 18 Oct 2017, David Laight wrote:

> From: SF Markus Elfring
> >  Unpleasant consequences are possible in both cases.
> > >> How much do you care to reduce the failure probability further?
> > >
> > > Zero.
> >
> > I am interested to improve the software situation a bit more here.
>
> There are probably better places to spend your time!
>
> If you want 'security' for kmalloc() then:
>
> #define KMALLOC_TYPE(flags) (type *)kmalloc(sizeof (type), flags)
> #define KMALLOC(ptr, flags) *(ptr) = KMALLOC_TYPE(typeof *(ptr), flags)
>
> and change:
>   ptr = kmalloc(sizeof *ptr, flags);
> to:
>   KMALLOC(, flags);
>
> But it is all churn for churn's sake.

Please don't.  Coccinelle won't find real problems with kmalloc any more
if this is done.

julia

RE: char-TPM: Adjustments for ten function implementations

[Julia Lawall]

On Wed, 18 Oct 2017, alexander.stef...@infineon.com wrote:

> > On Wed, 2017-10-18 at 11:00 +0200, SF Markus Elfring wrote:
> > > > The printk removals do change the objects.
> > > >
> > > > The value of that type of change is only for resource limited systems.
> > >
> > > I imagine that such small code adjustments are also useful for other
> > systems.
> >
> > Your imagination and mine differ.
> > Where do you _think_ it matters?
> >
> > For instance, nothing about
> >
> > sizeof(type)
> > vs
> > sizeof(*ptr)
> >
> > makes it easier for a human to read the code.
>
> If it does not make it easier to read the code for you, then maybe you
> should consider that this might not be true for all humans. For me, it
> makes it much easier to see at a glance, that code like
> ptr=malloc(sizeof(*ptr)) is correct.

I don't think there is a perfect solution.  The type argument to sizeof
could have the wrong type.  The expression argument to sizeof could be
missing the *.  Unpleasant consequences are possible in both cases.
Probably each maintainer has a style they prefer.  Perhaps it could be
useful to adjust the code to follow the dominant strategy, in cases where
there are a inconsistencies.  For example

if (...)
  x = foo1(sizeof(struct xtype));
else
  x = foo2(sizeof(*x));

might at least cause some unnecessary mental effort to process.

julia

Re: [PATCH 3/4] char/tpm: Improve a size determination in nine functions

[Julia Lawall]

On Tue, 17 Oct 2017, Mimi Zohar wrote:

> On Tue, 2017-10-17 at 14:58 +0200, Julia Lawall wrote:
> >
> > On Tue, 17 Oct 2017, Mimi Zohar wrote:
> >
> > > On Tue, 2017-10-17 at 11:50 +, alexander.stef...@infineon.com
> > > wrote:
> > > > > > Replace the specification of data structures by pointer dereferences
> > > > > > as the parameter for the operator "sizeof" to make the corresponding
> > > > > > size
> > > > > > determination a bit safer according to the Linux coding style
> > > > > > convention.
> > > > >
> > > > >
> > > > > This patch does one style in favor of the other.
> > > >
> > > > I actually prefer that style, so I'd welcome this change :)
> > >
> > > Style changes should be reviewed and documented, like any other code
> > > change, and added to Documentation/process/coding-style.rst or an
> > > equivalent file.
> >
> > Actually, it has been there for many years:
> >
> > 14) Allocating memory
> > -
> > ...
> > The preferred form for passing a size of a struct is the following:
> >
> > .. code-block:: c
> >
> > p = kmalloc(sizeof(*p), ...);
> >
> > The alternative form where struct name is spelled out hurts readability and
> > introduces an opportunity for a bug when the pointer variable type is 
> > changed
> > but the corresponding sizeof that is passed to a memory allocator is not.
>
> True, thanks for the reminder.  Is this common in new code?  Is there
> a script/ or some other automated way of catching this usage before
> patches are upstreamed?
>
> Just as you're doing here, the patch description should reference this
> in the patch description.

The comment in the documentation seems have been there since Linux 2.6.14,
ie 2005.  The fact that a lot of code still doesn't use that style, 12
years later, suggests that actually it is not preferred, or not preferred
by everyone.  Perhaps the paragraph in coding style should just be
dropped.

julia

Re: [PATCH 3/4] char/tpm: Improve a size determination in nine functions

[Julia Lawall]

On Tue, 17 Oct 2017, Mimi Zohar wrote:

> On Tue, 2017-10-17 at 11:50 +, alexander.stef...@infineon.com
> wrote:
> > > > Replace the specification of data structures by pointer dereferences
> > > > as the parameter for the operator "sizeof" to make the corresponding
> > > > size
> > > > determination a bit safer according to the Linux coding style
> > > > convention.
> > >
> > >
> > > This patch does one style in favor of the other.
> >
> > I actually prefer that style, so I'd welcome this change :)
>
> Style changes should be reviewed and documented, like any other code
> change, and added to Documentation/process/coding-style.rst or an
> equivalent file.

Actually, it has been there for many years:

14) Allocating memory
-
...
The preferred form for passing a size of a struct is the following:

.. code-block:: c

p = kmalloc(sizeof(*p), ...);

The alternative form where struct name is spelled out hurts readability and
introduces an opportunity for a bug when the pointer variable type is changed
but the corresponding sizeof that is passed to a memory allocator is not.

julia

>
> > > At the end it's Jarkko's call, though I would NAK this as I think some
> > > one already told this to you for some other similar patch(es).
> > >
> > >
> > > I even would suggest to stop doing this noisy stuff, which keeps people
> > > busy for nothing.
> >
> > Cleaning up old code is also worth something, even if does not
> > change one bit in the assembly output in the end...
>
> Wow, you're opening the door really wide for all sorts of trivial
> changes!  Hope you have the time and inclination to review and comment
> on all of them.  I certainly don't.
>
> There is a major difference between adding these sorts of checks to
> the tools in the scripts directory or even to the zero day bots that
> catch different sorts of errors, BEFORE code is upstreamed, and
> patches like these, after the fact.
>
> After the code has been upstreamed, it is a lot more difficult to
> justify changes like this.  It impacts both code that is being
> developed AND backporting bug fixes.
>
> Mimi
>
> --
> 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 0/4] char-TPM: Adjustments for ten function implementations

[Julia Lawall]

On Tue, 17 Oct 2017, Dan Carpenter wrote:

> On Tue, Oct 17, 2017 at 10:56:42AM +0200, Julia Lawall wrote:
> >
> >
> > On Tue, 17 Oct 2017, Dan Carpenter wrote:
> >
> > > On Mon, Oct 16, 2017 at 09:35:12PM +0300, Jarkko Sakkinen wrote:
> > > >
> > > > A minor complaint: all commits are missing "Fixes:" tag.
> > > >
> > >
> > > Fixes is only for bug fixes.  These don't fix any bugs.
> >
> > 0-day seems to put Fixes for everything.  Should they be removed when the
> > old code is undesirable but doesn't actually cause a crash, eg out of date
> > API.
>
> Yeah, I feel like Fixes tags don't belong for API updates and cleanups.

OK, I will remove them from the patches that go through me where they
don't seem appropriate.

thanks,
julia

Re: [PATCH 0/4] char-TPM: Adjustments for ten function implementations

[Julia Lawall]

On Tue, 17 Oct 2017, Dan Carpenter wrote:

> On Mon, Oct 16, 2017 at 09:35:12PM +0300, Jarkko Sakkinen wrote:
> >
> > A minor complaint: all commits are missing "Fixes:" tag.
> >
>
> Fixes is only for bug fixes.  These don't fix any bugs.

0-day seems to put Fixes for everything.  Should they be removed when the
old code is undesirable but doesn't actually cause a crash, eg out of date
API.

julia


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

Re: [PATCH] powerpc/eeh: make eeh_ops structures _ro_after_init

[Julia Lawall]

On Fri, 13 Oct 2017, Bhumika Goyal wrote:

> These structures are passed to the eeh_ops_register function during the
> initialization phase. There they get stored in a structure variable
> which only makes function calls through function pointers. There is no
> other usage of these eeh_ops structures and their fields are never
> modified after init phase. So, make them __ro_after_init.

I think they could be const.

julia

> Signed-off-by: Bhumika Goyal 
> ---
>  arch/powerpc/platforms/powernv/eeh-powernv.c | 2 +-
>  arch/powerpc/platforms/pseries/eeh_pseries.c | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/powerpc/platforms/powernv/eeh-powernv.c 
> b/arch/powerpc/platforms/powernv/eeh-powernv.c
> index 4650fb2..d2a53df 100644
> --- a/arch/powerpc/platforms/powernv/eeh-powernv.c
> +++ b/arch/powerpc/platforms/powernv/eeh-powernv.c
> @@ -1731,7 +1731,7 @@ static int pnv_eeh_restore_config(struct pci_dn *pdn)
>   return 0;
>  }
>
> -static struct eeh_ops pnv_eeh_ops = {
> +static struct eeh_ops pnv_eeh_ops __ro_after_init = {
>   .name   = "powernv",
>   .init   = pnv_eeh_init,
>   .probe  = pnv_eeh_probe,
> diff --git a/arch/powerpc/platforms/pseries/eeh_pseries.c 
> b/arch/powerpc/platforms/pseries/eeh_pseries.c
> index 6b812ad..6fedfc9 100644
> --- a/arch/powerpc/platforms/pseries/eeh_pseries.c
> +++ b/arch/powerpc/platforms/pseries/eeh_pseries.c
> @@ -684,7 +684,7 @@ static int pseries_eeh_write_config(struct pci_dn *pdn, 
> int where, int size, u32
>   return rtas_write_config(pdn, where, size, val);
>  }
>
> -static struct eeh_ops pseries_eeh_ops = {
> +static struct eeh_ops pseries_eeh_ops __ro_after_init = {
>   .name   = "pseries",
>   .init   = pseries_eeh_init,
>   .probe  = pseries_eeh_probe,
> --
> 1.9.1
>
>

Re: [PATCH 1/2] powerpc/platforms/cell: Delete an error message for a failed memory allocation in three functions

[Julia Lawall]

On Thu, 5 Oct 2017, Michal Suchánek wrote:

> Hello,
>
> On Thu, 5 Oct 2017 21:36:26 +0200
> SF Markus Elfring  wrote:
>
> > From: Markus Elfring 
> > Date: Thu, 5 Oct 2017 21:04:30 +0200
> >
> > Omit extra messages for a memory allocation failure in these
> > functions.
>
> this is bogus. All these functions return -1 on any error. Until they
> reflect the error in their return value (and it is properly propagated
> to the user) there is no way to tell WTF failed without the message.

A backtrace will be generated.

julia

>
> >
> > This issue was detected by using the Coccinelle software.
> >
>
> ... which provides only a hint which should be evaluated by the user.
>
> Thanks
>
> Michal
> --
> 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] powerpc: make irq_chip const, __initdata and __initconst

[Julia Lawall]

On Wed, 20 Sep 2017, Michael Ellerman wrote:

> Bhumika Goyal  writes:
>
> > Make ehv_pic_irq_chip, mpic_ipi_chip and mpic_tm_chip const as they are
> > used only as a copy operation. This usage is during init, so make them
> > __initconst too.
> > Make mpic_ipi_chip __initdata as it is only modified during the init
> > phase and there is no reference of it anywhere after init.
>
> The change log doesn't seem to match the code.
>
> It's mpic_tm_chip which you marked __initdata, and that looks wrong, as
> we keep a pointer to it here:
>
>   mpic->hc_tm = mpic_tm_chip;

This looks like a memory copy.

julia

qustion about eeh_add_virt_device

[Julia Lawall]

Hello,

At the suggestion of Christoph Hellwig, I am working on inlining the
functions stored in the err_handler field of a pci_driver structure into
the pci_driver structure itself.  A number of functions in the file
arch/powerpc/kernel/eeh_driver.c have code like:

if (!driver->err_handler ||
!driver->err_handler->error_detected) {
eeh_pcid_put(dev);
return NULL;
}

This I would just convert to:

if (!driver->error_detected) {
eeh_pcid_put(dev);
return NULL;
}

But I am not sure what is best to do about eeh_add_virt_device, which
contains:

if (driver->err_handler)
return NULL;

Should I try to find a subfield of the err_handler that is guaranteed to
be there if anything is there?  Or could the test just be dropped, leaving
a direct return NULL?

thanks,
julia

Re: [PATCH] powerpc: store the intended structure

[Julia Lawall]

On Sun, 13 Aug 2017, Joe Perches wrote:

> On Sun, 2017-08-13 at 15:24 +0200, Julia Lawall wrote:
> > Normally the values in the resource field and the argument to ARRAY_SIZE
> > in the num_resources are the same.  In this case, the value in the reousrce
> > field is the same as the one in the previous platform_device structure, and
> > appears to be a copy-paste error.  Replace the value in the resource field
> > with the argument to the local call to ARRAY_SIZE.
>
> found by a script or eyeballs?

A script that was looking for something else.  But I wrote a script for
this specific issue and this was the only match.  I am currently checking
in a more general way.

julia

[PATCH] powerpc: store the intended structure

[Julia Lawall]

Normally the values in the resource field and the argument to ARRAY_SIZE
in the num_resources are the same.  In this case, the value in the reousrce
field is the same as the one in the previous platform_device structure, and
appears to be a copy-paste error.  Replace the value in the resource field
with the argument to the local call to ARRAY_SIZE.

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

---
 arch/powerpc/platforms/chrp/pegasos_eth.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/platforms/chrp/pegasos_eth.c 
b/arch/powerpc/platforms/chrp/pegasos_eth.c
index 2b4dc6a..1976071 100644
--- a/arch/powerpc/platforms/chrp/pegasos_eth.c
+++ b/arch/powerpc/platforms/chrp/pegasos_eth.c
@@ -63,7 +63,7 @@
.name   = "orion-mdio",
.id = -1,
.num_resources  = ARRAY_SIZE(mv643xx_eth_mvmdio_resources),
-   .resource   = mv643xx_eth_shared_resources,
+   .resource   = mv643xx_eth_mvmdio_resources,
 };
 
 static struct resource mv643xx_eth_port1_resources[] = {

[PATCH 05/11] serial: uuc_uart: constify uart_ops structures

[Julia Lawall]

These uart_ops structures are only stored in the ops field of a
uart_port structure and this fields is const, so the uart_ops
structures can also be const.

Done with the help of Coccinelle.

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

---
 drivers/tty/serial/ucc_uart.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/tty/serial/ucc_uart.c b/drivers/tty/serial/ucc_uart.c
index 481eb29..55b7027 100644
--- a/drivers/tty/serial/ucc_uart.c
+++ b/drivers/tty/serial/ucc_uart.c
@@ -1085,7 +1085,7 @@ static int qe_uart_verify_port(struct uart_port *port,
  *
  * Details on these functions can be found in Documentation/serial/driver
  */
-static struct uart_ops qe_uart_pops = {
+static const struct uart_ops qe_uart_pops = {
.tx_empty   = qe_uart_tx_empty,
.set_mctrl  = qe_uart_set_mctrl,
.get_mctrl  = qe_uart_get_mctrl,

[PATCH 00/11] constify uart_ops structures

[Julia Lawall]

These uart_ops structures are only stored in the ops field of a
uart_port structure and this fields is const, so the uart_ops
structures can also be const.

Done with the help of Coccinelle.

---

 drivers/tty/serial/21285.c  |2 +-
 drivers/tty/serial/apbuart.c|2 +-
 drivers/tty/serial/cpm_uart/cpm_uart_core.c |2 +-
 drivers/tty/serial/m32r_sio.c   |2 +-
 drivers/tty/serial/meson_uart.c |2 +-
 drivers/tty/serial/mpc52xx_uart.c   |2 +-
 drivers/tty/serial/mux.c|2 +-
 drivers/tty/serial/owl-uart.c   |2 +-
 drivers/tty/serial/sunsab.c |2 +-
 drivers/tty/serial/sunsu.c  |2 +-
 drivers/tty/serial/ucc_uart.c   |2 +-
 11 files changed, 11 insertions(+), 11 deletions(-)

[PATCH] powerpc/macintosh: constify wf_sensor_ops structures

[Julia Lawall]

The wf_sensor_ops structures are only stored in the ops field of a
wf_sensor structure, which is declared as const.  Thus the
wf_sensor_ops structures themselves can be const.

Done with the help of Coccinelle.

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

@ok1@
identifier r.i;
struct wf_sensor s;
position p;
@@
s.ops = @p

@ok2@
identifier r.i;
struct wf_sat_sensor s;
position p;
@@
s.sens.ops = @p

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

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

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

---
 drivers/macintosh/windfarm_lm75_sensor.c|2 +-
 drivers/macintosh/windfarm_lm87_sensor.c|2 +-
 drivers/macintosh/windfarm_max6690_sensor.c |2 +-
 drivers/macintosh/windfarm_smu_sat.c|2 +-
 drivers/macintosh/windfarm_smu_sensors.c|   10 +-
 5 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/macintosh/windfarm_lm75_sensor.c 
b/drivers/macintosh/windfarm_lm75_sensor.c
index 590214b..6cdfe71 100644
--- a/drivers/macintosh/windfarm_lm75_sensor.c
+++ b/drivers/macintosh/windfarm_lm75_sensor.c
@@ -82,7 +82,7 @@ static void wf_lm75_release(struct wf_sensor *sr)
kfree(lm);
 }
 
-static struct wf_sensor_ops wf_lm75_ops = {
+static const struct wf_sensor_ops wf_lm75_ops = {
.get_value  = wf_lm75_get,
.release= wf_lm75_release,
.owner  = THIS_MODULE,
diff --git a/drivers/macintosh/windfarm_lm87_sensor.c 
b/drivers/macintosh/windfarm_lm87_sensor.c
index c071aab..273d7d4 100644
--- a/drivers/macintosh/windfarm_lm87_sensor.c
+++ b/drivers/macintosh/windfarm_lm87_sensor.c
@@ -91,7 +91,7 @@ static void wf_lm87_release(struct wf_sensor *sr)
kfree(lm);
 }
 
-static struct wf_sensor_ops wf_lm87_ops = {
+static const struct wf_sensor_ops wf_lm87_ops = {
.get_value  = wf_lm87_get,
.release= wf_lm87_release,
.owner  = THIS_MODULE,
diff --git a/drivers/macintosh/windfarm_max6690_sensor.c 
b/drivers/macintosh/windfarm_max6690_sensor.c
index 87e439b..6ad035e 100644
--- a/drivers/macintosh/windfarm_max6690_sensor.c
+++ b/drivers/macintosh/windfarm_max6690_sensor.c
@@ -55,7 +55,7 @@ static void wf_max6690_release(struct wf_sensor *sr)
kfree(max);
 }
 
-static struct wf_sensor_ops wf_max6690_ops = {
+static const struct wf_sensor_ops wf_max6690_ops = {
.get_value  = wf_max6690_get,
.release= wf_max6690_release,
.owner  = THIS_MODULE,
diff --git a/drivers/macintosh/windfarm_smu_sat.c 
b/drivers/macintosh/windfarm_smu_sat.c
index ad6223e..5a58fc2 100644
--- a/drivers/macintosh/windfarm_smu_sat.c
+++ b/drivers/macintosh/windfarm_smu_sat.c
@@ -195,7 +195,7 @@ static void wf_sat_sensor_release(struct wf_sensor *sr)
kref_put(>ref, wf_sat_release);
 }
 
-static struct wf_sensor_ops wf_sat_ops = {
+static const struct wf_sensor_ops wf_sat_ops = {
.get_value  = wf_sat_sensor_get,
.release= wf_sat_sensor_release,
.owner  = THIS_MODULE,
diff --git a/drivers/macintosh/windfarm_smu_sensors.c 
b/drivers/macintosh/windfarm_smu_sensors.c
index 1cc4e49..172fd26 100644
--- a/drivers/macintosh/windfarm_smu_sensors.c
+++ b/drivers/macintosh/windfarm_smu_sensors.c
@@ -172,22 +172,22 @@ static int smu_slotspow_get(struct wf_sensor *sr, s32 
*value)
 }
 
 
-static struct wf_sensor_ops smu_cputemp_ops = {
+static const struct wf_sensor_ops smu_cputemp_ops = {
.get_value  = smu_cputemp_get,
.release= smu_ads_release,
.owner  = THIS_MODULE,
 };
-static struct wf_sensor_ops smu_cpuamp_ops = {
+static const struct wf_sensor_ops smu_cpuamp_ops = {
.get_value  = smu_cpuamp_get,
.release= smu_ads_release,
.owner  = THIS_MODULE,
 };
-static struct wf_sensor_ops smu_cpuvolt_ops = {
+static const struct wf_sensor_ops smu_cpuvolt_ops = {
.get_value  = smu_cpuvolt_get,
.release= smu_ads_release,
.owner  = THIS_MODULE,
 };
-static struct wf_sensor_ops smu_slotspow_ops = {
+static const struct wf_sensor_ops smu_slotspow_ops = {
.get_value  = smu_slotspow_get,
.release= smu_ads_release,
.owner  = THIS_MODULE,
@@ -327,7 +327,7 @@ static int smu_cpu_power_get(struct wf_sensor *sr, s32 
*value)
return 0;
 }
 
-static struct wf_sensor_ops smu_cpu_power_ops = {
+static const struct wf_sensor_ops smu_cpu_power_ops = {
.get_value  = smu_cpu_power_get,
.release= smu_cpu_power_release,
.owner  = THIS_MODULE,

[PATCH] usb: gadget: fsl_qe_udc: constify qe_ep0_desc

[Julia Lawall]

qe_ep0_desc is only passed as the second argument to qe_ep_init, which is
const, so qe_ep0_desc can be const too.

Done with the help of Coccinelle.

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

---
I got a lot of warnings when compiling this file, but none seemed to be
related to the change.

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

diff --git a/drivers/usb/gadget/udc/fsl_qe_udc.c 
b/drivers/usb/gadget/udc/fsl_qe_udc.c
index 303328ce..a3e72d6 100644
--- a/drivers/usb/gadget/udc/fsl_qe_udc.c
+++ b/drivers/usb/gadget/udc/fsl_qe_udc.c
@@ -62,7 +62,7 @@
"ep3",
 };
 
-static struct usb_endpoint_descriptor qe_ep0_desc = {
+static const struct usb_endpoint_descriptor qe_ep0_desc = {
.bLength =  USB_DT_ENDPOINT_SIZE,
.bDescriptorType =  USB_DT_ENDPOINT,
 

Re: [PATCH] ALSA: declare snd_kcontrol_new structures as const

[Julia Lawall]

On Tue, 30 May 2017, Takashi Iwai wrote:

> On Sat, 27 May 2017 16:46:15 +0200,
> Bhumika Goyal wrote:
> >
> > Declare snd_kcontrol_new structures as const as they are only passed an
> > argument to the function snd_ctl_new1. This argument is of type const,
> > so snd_kcontrol_new structures having this property can be made const.
> > Done using Coccinelle:
> >
> > @r disable optional_qualifier@
> > identifier x;
> > position p;
> > @@
> > static struct snd_kcontrol_new x@p={...};
> >
> > @ok@
> > identifier r.x;
> > position p;
> > @@
> > snd_ctl_new1(@p,...)
> >
> > @bad@
> > position p != {r.p,ok.p};
> > identifier r.x;
> > @@
> > x@p
> >
> > @depends on !bad disable optional_qualifier@
> > identifier r.x;
> > @@
> > +const
> > struct snd_kcontrol_new x;
> >
> > Cross compiled these files:
> > sound/aoa/codecs/tas.c - powerpc
> > sound/mips/{hal2.c/sgio2audio.c} - mips
> > sound/ppc/{awacs.c/beep.c/tumbler.c} - powerpc
> > sound/soc/sh/siu_dai.c - sh
> > Could not find an architecture to compile sound/sh/aica.c.
> >
> > Signed-off-by: Bhumika Goyal 
>
> Applied now, as it looks safe enough.  But I prefer splitting to
> individual patches, so please try to do that at the next time.

Individual patches in a series?

julia

[PATCH 08/15] powerpc/iommu: use permission-specific DEVICE_ATTR variants

[Julia Lawall]

Use DEVICE_ATTR_RW for read-write attributes.  This simplifies the
source code, improves readbility, and reduces the chance of
inconsistencies.

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

// 
@rw@
declarer name DEVICE_ATTR;
identifier x,x_show,x_store;
@@

DEVICE_ATTR(x, \(0644\|S_IRUGO|S_IWUSR\), x_show, x_store);

@script:ocaml@
x << rw.x;
x_show << rw.x_show;
x_store << rw.x_store;
@@

if not (x^"_show" = x_show && x^"_store" = x_store)
then Coccilib.include_match false

@@
declarer name DEVICE_ATTR_RW;
identifier rw.x,rw.x_show,rw.x_store;
@@

- DEVICE_ATTR(x, \(0644\|S_IRUGO|S_IWUSR\), x_show, x_store);
+ DEVICE_ATTR_RW(x);
// 

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

---
 arch/powerpc/kernel/iommu.c |3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/powerpc/kernel/iommu.c b/arch/powerpc/kernel/iommu.c
index 5f202a5..32f18b5 100644
--- a/arch/powerpc/kernel/iommu.c
+++ b/arch/powerpc/kernel/iommu.c
@@ -127,8 +127,7 @@ static ssize_t fail_iommu_store(struct device *dev,
return count;
 }
 
-static DEVICE_ATTR(fail_iommu, S_IRUGO|S_IWUSR, fail_iommu_show,
-  fail_iommu_store);
+static DEVICE_ATTR_RW(fail_iommu);
 
 static int fail_iommu_bus_notify(struct notifier_block *nb,
 unsigned long action, void *data)

[PATCH 00/15] use permission-specific DEVICE_ATTR variants

[Julia Lawall]

Use DEVICE_ATTR_RO etc. for read only attributes etc.  This simplifies the
source code, improves readbility, and reduces the chance of
inconsistencies.

The complete semantic patch is as follows:
(http://coccinelle.lip6.fr/)

// 
@ro@
declarer name DEVICE_ATTR;
identifier x,x_show;
@@

DEVICE_ATTR(x, \(0444\|S_IRUGO\), x_show, NULL);

@wo@
declarer name DEVICE_ATTR;
identifier x,x_store;
@@

DEVICE_ATTR(x, \(0200\|S_IWUSR\), NULL, x_store);

@rw@
declarer name DEVICE_ATTR;
identifier x,x_show,x_store;
@@

DEVICE_ATTR(x, \(0644\|S_IRUGO|S_IWUSR\), x_show, x_store);

@script:ocaml@
x << ro.x;
x_show << ro.x_show;
@@

if not (x^"_show" = x_show) then Coccilib.include_match false

@script:ocaml@
x << wo.x;
x_store << wo.x_store;
@@

if not (x^"_store" = x_store) then Coccilib.include_match false

@script:ocaml@
x << rw.x;
x_show << rw.x_show;
x_store << rw.x_store;
@@

if not (x^"_show" = x_show && x^"_store" = x_store)
then Coccilib.include_match false

@@
declarer name DEVICE_ATTR_RO;
identifier ro.x,ro.x_show;
@@

- DEVICE_ATTR(x, \(0444\|S_IRUGO\), x_show, NULL);
+ DEVICE_ATTR_RO(x);

@@
declarer name DEVICE_ATTR_WO;
identifier wo.x,wo.x_store;
@@

- DEVICE_ATTR(x, \(0200\|S_IWUSR\), NULL, x_store);
+ DEVICE_ATTR_WO(x);

@@
declarer name DEVICE_ATTR_RW;
identifier rw.x,rw.x_show,rw.x_store;
@@

- DEVICE_ATTR(x, \(0644\|S_IRUGO|S_IWUSR\), x_show, x_store);
+ DEVICE_ATTR_RW(x);
// 

---

 arch/mips/txx9/generic/7segled.c  |4 ++--
 arch/powerpc/kernel/iommu.c   |3 +--
 arch/tile/kernel/sysfs.c  |   14 +++---
 drivers/atm/solos-pci.c   |2 +-
 drivers/pci/pcie/aspm.c   |4 ++--
 drivers/power/supply/wm8350_power.c   |2 +-
 drivers/ptp/ptp_sysfs.c   |2 +-
 drivers/thermal/int340x_thermal/int3400_thermal.c |2 +-
 drivers/thermal/thermal_hwmon.c   |2 +-
 drivers/tty/nozomi.c  |4 ++--
 drivers/usb/wusbcore/dev-sysfs.c  |6 +++---
 drivers/usb/wusbcore/wusbhc.c |   13 +
 drivers/video/fbdev/wm8505fb.c|2 +-
 sound/soc/omap/mcbsp.c|4 ++--
 sound/soc/soc-dapm.c  |2 +-
 15 files changed, 31 insertions(+), 35 deletions(-)

[PATCH 6/7] ASoC: constify snd_soc_ops structures

[Julia Lawall]

Check for snd_soc_ops structures that are only stored in the ops field of a
snd_soc_dai_link structure.  This field is declared const, so snd_soc_ops
structures that have this property can be declared as const also.

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

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

@ok1@
identifier r.i;
struct snd_soc_dai_link e;
position p;
@@
e.ops = @p;

@ok2@
identifier r.i, e;
position p;
@@
struct snd_soc_dai_link e[] = { ..., { .ops = @p, }, ..., };

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

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

The effect on the layout of the .o files is shown by the following output of
the size command, first before then after the transformation:

  textdata bss dec hex filename
   87481024   09772262c sound/soc/fsl/fsl-asoc-card.o
   8812 952   097642624 sound/soc/fsl/fsl-asoc-card.o

   textdata bss dec hex filename
   4165 264   844371155 sound/soc/fsl/imx-wm8962.o
   4229 200   844371155 sound/soc/fsl/imx-wm8962.o

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

---
 sound/soc/fsl/fsl-asoc-card.c |2 +-
 sound/soc/fsl/imx-wm8962.c|2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff -u -p a/sound/soc/fsl/fsl-asoc-card.c b/sound/soc/fsl/fsl-asoc-card.c
--- a/sound/soc/fsl/fsl-asoc-card.c
+++ b/sound/soc/fsl/fsl-asoc-card.c
@@ -183,7 +183,7 @@ static int fsl_asoc_card_hw_params(struc
return 0;
 }
 
-static struct snd_soc_ops fsl_asoc_card_ops = {
+static const struct snd_soc_ops fsl_asoc_card_ops = {
.hw_params = fsl_asoc_card_hw_params,
 };
 
diff -u -p a/sound/soc/fsl/imx-wm8962.c b/sound/soc/fsl/imx-wm8962.c
--- a/sound/soc/fsl/imx-wm8962.c
+++ b/sound/soc/fsl/imx-wm8962.c
@@ -61,7 +61,7 @@ static int imx_hifi_hw_params(struct snd
return 0;
 }
 
-static struct snd_soc_ops imx_hifi_ops = {
+static const struct snd_soc_ops imx_hifi_ops = {
.hw_params = imx_hifi_hw_params,
 };
 

[PATCH] ASoC: constify snd_pcm_ops structures

[Julia Lawall]

Check for snd_pcm_ops structures that are only stored in the ops field of a
snd_soc_platform_driver structure or passed as the third argument to
snd_pcm_set_ops.  The corresponding field or parameter is declared const,
so snd_pcm_ops structures that have this property can be declared as const
also.

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

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

@ok1@
identifier r.i;
struct snd_soc_platform_driver e;
position p;
@@
e.ops = @p;

@ok2@
identifier r.i;
expression e1, e2;
position p;
@@
snd_pcm_set_ops(e1, e2, @p)

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

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

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

---
 sound/soc/amd/acp-pcm-dma.c  |2 +-
 sound/soc/atmel/atmel-pcm-pdc.c  |2 +-
 sound/soc/codecs/rt5514-spi.c|2 +-
 sound/soc/fsl/fsl_asrc_dma.c |2 +-
 sound/soc/intel/atom/sst-mfld-platform-pcm.c |2 +-
 sound/soc/intel/haswell/sst-haswell-pcm.c|2 +-
 sound/soc/intel/skylake/skl-pcm.c|2 +-
 sound/soc/kirkwood/kirkwood-dma.c|2 +-
 sound/soc/qcom/lpass-platform.c  |2 +-
 sound/soc/soc-utils.c|2 +-
 10 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/sound/soc/intel/haswell/sst-haswell-pcm.c 
b/sound/soc/intel/haswell/sst-haswell-pcm.c
index 3154525..9e4094e 100644
--- a/sound/soc/intel/haswell/sst-haswell-pcm.c
+++ b/sound/soc/intel/haswell/sst-haswell-pcm.c
@@ -871,7 +871,7 @@ out:
return ret;
 }
 
-static struct snd_pcm_ops hsw_pcm_ops = {
+static const struct snd_pcm_ops hsw_pcm_ops = {
.open   = hsw_pcm_open,
.close  = hsw_pcm_close,
.ioctl  = snd_pcm_lib_ioctl,
diff --git a/sound/soc/atmel/atmel-pcm-pdc.c b/sound/soc/atmel/atmel-pcm-pdc.c
index da861b4..91b7069 100644
--- a/sound/soc/atmel/atmel-pcm-pdc.c
+++ b/sound/soc/atmel/atmel-pcm-pdc.c
@@ -381,7 +381,7 @@ static int atmel_pcm_close(struct snd_pcm_substream 
*substream)
return 0;
 }
 
-static struct snd_pcm_ops atmel_pcm_ops = {
+static const struct snd_pcm_ops atmel_pcm_ops = {
.open   = atmel_pcm_open,
.close  = atmel_pcm_close,
.ioctl  = snd_pcm_lib_ioctl,
diff --git a/sound/soc/qcom/lpass-platform.c b/sound/soc/qcom/lpass-platform.c
index a144c14..e2ff538 100644
--- a/sound/soc/qcom/lpass-platform.c
+++ b/sound/soc/qcom/lpass-platform.c
@@ -372,7 +372,7 @@ static int lpass_platform_pcmops_mmap(struct 
snd_pcm_substream *substream,
runtime->dma_bytes);
 }
 
-static struct snd_pcm_ops lpass_platform_pcm_ops = {
+static const struct snd_pcm_ops lpass_platform_pcm_ops = {
.open   = lpass_platform_pcmops_open,
.ioctl  = snd_pcm_lib_ioctl,
.hw_params  = lpass_platform_pcmops_hw_params,
diff --git a/sound/soc/codecs/rt5514-spi.c b/sound/soc/codecs/rt5514-spi.c
index 77ff8eb..09103aa 100644
--- a/sound/soc/codecs/rt5514-spi.c
+++ b/sound/soc/codecs/rt5514-spi.c
@@ -236,7 +236,7 @@ static snd_pcm_uframes_t rt5514_spi_pcm_pointer(
return bytes_to_frames(runtime, rt5514_dsp->dma_offset);
 }
 
-static struct snd_pcm_ops rt5514_spi_pcm_ops = {
+static const struct snd_pcm_ops rt5514_spi_pcm_ops = {
.open   = rt5514_spi_pcm_open,
.hw_params  = rt5514_spi_hw_params,
.hw_free= rt5514_spi_hw_free,
diff --git a/sound/soc/amd/acp-pcm-dma.c b/sound/soc/amd/acp-pcm-dma.c
index d1fb035..504c7cd 100644
--- a/sound/soc/amd/acp-pcm-dma.c
+++ b/sound/soc/amd/acp-pcm-dma.c
@@ -897,7 +897,7 @@ static int acp_dma_close(struct snd_pcm_substream 
*substream)
return 0;
 }
 
-static struct snd_pcm_ops acp_dma_ops = {
+static const struct snd_pcm_ops acp_dma_ops = {
.open = acp_dma_open,
.close = acp_dma_close,
.ioctl = snd_pcm_lib_ioctl,
diff --git a/sound/soc/intel/atom/sst-mfld-platform-pcm.c 
b/sound/soc/intel/atom/sst-mfld-platform-pcm.c
index 52ed434..25c6d87 100644
--- a/sound/soc/intel/atom/sst-mfld-platform-pcm.c
+++ b/sound/soc/intel/atom/sst-mfld-platform-pcm.c
@@ -670,7 +670,7 @@ static snd_pcm_uframes_t sst_platform_pcm_pointer
return str_info->buffer_ptr;
 }
 
-static struct snd_pcm_ops sst_platform_ops = {
+static const struct snd_pcm_ops sst_platform_ops = {
.open = sst_platform_open,
.ioctl = snd_pcm_lib_ioctl,
.trigger = sst_platform_pcm_trigger,
diff --git a/sound/soc/intel/skylake/skl-pcm.c 
b/sound/soc/intel/skylake/skl-pcm.c
index 77bfd40..86c125f 100644
--- a/sound/soc/intel/skylake/skl-pcm.c
+++ b/sound/soc/intel/skylake/skl-pcm.c
@@ -1093,7 +1093,7 @@ static int sk

Re: [Ksummit-discuss] [CORE TOPIC] (group) maintainership models

[Julia Lawall]

On Fri, 2 Sep 2016, Linus Torvalds wrote:

> On Fri, Sep 2, 2016 at 1:06 PM, Arnd Bergmann  wrote:
> >
> > When I once looked, I thought all drivers using NO_IRQ were specific
> > to powerpc or one of the less common architectures.
>
> powerpc definitely does seem to be the biggest case, with about half
> the instances of NO_IRQ being under arch/powerpc/ (and a few more in
> ppc-specific drivers).
>
> Adding the powerpc maintainers to the list - because it would really
> be nice to get rid of it, or at least make it *so* rare that we don't
> have people re-introducing it again because they thought it was the
> right thing to do.
>
> A fair amount of of it could even be done by some trivial scripting.
> Something like
>
>   git grep -wl NO_IRQ arch/powerpc/ | while read a
>   do
>   sed 's/(\([a-z_]*irq\) != NO_IRQ)/(\1)/' < $a > $a.new
>   sed 's/(\([a-z_]*irq\) == NO_IRQ)/(!\1)/' < $a.new > $a
>   done
>
> does fix at least a few of the cases. It still leaves several
> assignments and "return NO_IRQ;" statements, but a few more
> sed-scripts would take care of most of it. Then remove the #define,
> and do a full build to find any straggling cases.

Like this?

@@
expression e;
@@

e
- != NO_IRQ

@@
expression e;
@@

+!
e
- == NO_IRQ

@@
@@

- NO_IRQ
+ 0

---

Is it always correct to replace return NO_IRQ by return 0?

Completely untested patch below.

julia

diff -u -p a/arch/powerpc/platforms/52xx/mpc52xx_pic.c 
b/arch/powerpc/platforms/52xx/mpc52xx_pic.c
--- a/arch/powerpc/platforms/52xx/mpc52xx_pic.c
+++ b/arch/powerpc/platforms/52xx/mpc52xx_pic.c
@@ -511,7 +511,7 @@ unsigned int mpc52xx_get_irq(void)
irq |= (MPC52xx_IRQ_L1_PERP << MPC52xx_IRQ_L1_OFFSET);
}
} else {
-   return NO_IRQ;
+   return 0;
}

return irq_linear_revmap(mpc52xx_irqhost, irq);
diff -u -p a/arch/powerpc/platforms/chrp/setup.c 
b/arch/powerpc/platforms/chrp/setup.c
--- a/arch/powerpc/platforms/chrp/setup.c
+++ b/arch/powerpc/platforms/chrp/setup.c
@@ -368,7 +368,7 @@ static void chrp_8259_cascade(struct irq
struct irq_chip *chip = irq_desc_get_chip(desc);
unsigned int cascade_irq = i8259_irq();

-   if (cascade_irq != NO_IRQ)
+   if (cascade_irq)
generic_handle_irq(cascade_irq);

chip->irq_eoi(>irq_data);
@@ -514,7 +514,7 @@ static void __init chrp_find_8259(void)
}
if (chrp_mpic != NULL) {
cascade_irq = irq_of_parse_and_map(pic, 0);
-   if (cascade_irq == NO_IRQ)
+   if (!cascade_irq)
printk(KERN_ERR "i8259: failed to map cascade irq\n");
else
irq_set_chained_handler(cascade_irq,
diff -u -p a/arch/powerpc/platforms/maple/pci.c 
b/arch/powerpc/platforms/maple/pci.c
--- a/arch/powerpc/platforms/maple/pci.c
+++ b/arch/powerpc/platforms/maple/pci.c
@@ -552,7 +552,7 @@ void maple_pci_irq_fixup(struct pci_dev
pci_bus_to_host(dev->bus) == u4_pcie) {
printk(KERN_DEBUG "Fixup U4 PCIe IRQ\n");
dev->irq = irq_create_mapping(NULL, 1);
-   if (dev->irq != NO_IRQ)
+   if (dev->irq)
irq_set_irq_type(dev->irq, IRQ_TYPE_LEVEL_LOW);
}

@@ -562,7 +562,7 @@ void maple_pci_irq_fixup(struct pci_dev
if (dev->vendor == PCI_VENDOR_ID_AMD &&
dev->device == PCI_DEVICE_ID_AMD_8111_IDE &&
(dev->class & 5) != 5) {
-   dev->irq = NO_IRQ;
+   dev->irq = 0;
}

DBG(" <- maple_pci_irq_fixup\n");
@@ -648,7 +648,7 @@ int maple_pci_get_legacy_ide_irq(struct
return defirq;
}
irq = irq_of_parse_and_map(np, channel & 0x1);
-   if (irq == NO_IRQ) {
+   if (!irq) {
printk("Failed to map onboard IDE interrupt for channel %d\n",
   channel);
return defirq;
diff -u -p a/arch/powerpc/platforms/embedded6xx/flipper-pic.c 
b/arch/powerpc/platforms/embedded6xx/flipper-pic.c
--- a/arch/powerpc/platforms/embedded6xx/flipper-pic.c
+++ b/arch/powerpc/platforms/embedded6xx/flipper-pic.c
@@ -181,7 +181,7 @@ unsigned int flipper_pic_get_irq(void)
irq_status = in_be32(io_base + FLIPPER_ICR) &
 in_be32(io_base + FLIPPER_IMR);
if (irq_status == 0)
-   return NO_IRQ;  /* no more IRQs pending */
+   return 0;   /* no more IRQs pending */

irq = __ffs(irq_status);
return irq_linear_revmap(flipper_irq_host, irq);
diff -u -p a/arch/powerpc/platforms/embedded6xx/hlwd-pic.c 
b/arch/powerpc/platforms/embedded6xx/hlwd-pic.c
--- a/arch/powerpc/platforms/embedded6xx/hlwd-pic.c
+++ b/arch/powerpc/platforms/embedded6xx/hlwd-pic.c
@@ -114,7 +114,7 @@ static unsigned int __hlwd_pic_get_irq(s
irq_status = in_be32(io_base + HW_BROADWAY_ICR) &
 in_be32(io_base + 

Re: [PATCH 6/6] KVM: PPC: e500: Rename jump labels in kvmppc_e500_tlb_init()

[Julia Lawall]

On Sun, 28 Aug 2016, SF Markus Elfring wrote:

> From: Markus Elfring 
> Date: Sun, 28 Aug 2016 18:45:26 +0200
>
> Adjust jump labels according to the current Linux coding style convention.
>
> Signed-off-by: Markus Elfring 
> ---
>  arch/powerpc/kvm/e500_mmu.c | 9 -
>  1 file changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/arch/powerpc/kvm/e500_mmu.c b/arch/powerpc/kvm/e500_mmu.c
> index 0a2eeb1..da8f22b 100644
> --- a/arch/powerpc/kvm/e500_mmu.c
> +++ b/arch/powerpc/kvm/e500_mmu.c
> @@ -933,26 +933,25 @@ int kvmppc_e500_tlb_init(struct kvmppc_vcpu_e500 
> *vcpu_e500)
> sizeof(struct tlbe_ref),
> GFP_KERNEL);
>   if (!vcpu_e500->gtlb_priv[0])
> - goto err;
> + goto free_vcpu;
>
>   vcpu_e500->gtlb_priv[1] = kcalloc(vcpu_e500->gtlb_params[1].entries,
> sizeof(struct tlbe_ref),
> GFP_KERNEL);
>   if (!vcpu_e500->gtlb_priv[1])
> - goto err;
> + goto free_vcpu;
>
>   vcpu_e500->g2h_tlb1_map = kcalloc(vcpu_e500->gtlb_params[1].entries,
> sizeof(*vcpu_e500->g2h_tlb1_map),
> GFP_KERNEL);
>   if (!vcpu_e500->g2h_tlb1_map)
> - goto err;
> + goto free_vcpu;
>
>   vcpu_mmu_init(vcpu, vcpu_e500->gtlb_params);
>
>   kvmppc_recalc_tlb1map_range(vcpu_e500);
>   return 0;
> -
> -err:
> + free_vcpu:
>   free_gtlb(vcpu_e500);
>   return -1;

I doubt that -1 is the best return value.  One could guess that it should
be -ENOMEM.  But see what the call sites expect.

julia

>  }
> --
> 2.9.3
>
> --
> 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 5/6] KVM: PPC: e500: Use kmalloc_array() in kvmppc_e500_tlb_init()

[Julia Lawall]

On Sun, 28 Aug 2016, SF Markus Elfring wrote:

> From: Markus Elfring 
> Date: Sun, 28 Aug 2016 18:40:08 +0200
>
> * A multiplication for the size determination of a memory allocation
>   indicated that an array data structure should be processed.
>   Thus use the corresponding function "kmalloc_array".
>
> * Replace the specification of a data structure by a pointer dereference
>   to make the corresponding size determination a bit safer according to
>   the Linux coding style convention.
>
> Signed-off-by: Markus Elfring 
> ---
>  arch/powerpc/kvm/e500_mmu.c | 7 ---
>  1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/arch/powerpc/kvm/e500_mmu.c b/arch/powerpc/kvm/e500_mmu.c
> index 2be2afc4..0a2eeb1 100644
> --- a/arch/powerpc/kvm/e500_mmu.c
> +++ b/arch/powerpc/kvm/e500_mmu.c
> @@ -905,8 +905,6 @@ static int vcpu_mmu_init(struct kvm_vcpu *vcpu,
>  int kvmppc_e500_tlb_init(struct kvmppc_vcpu_e500 *vcpu_e500)
>  {
>   struct kvm_vcpu *vcpu = _e500->vcpu;
> - int entry_size = sizeof(struct kvm_book3e_206_tlb_entry);
> - int entries = KVM_E500_TLB0_SIZE + KVM_E500_TLB1_SIZE;
>
>   if (e500_mmu_host_init(vcpu_e500))
>   goto err;
> @@ -921,7 +919,10 @@ int kvmppc_e500_tlb_init(struct kvmppc_vcpu_e500 
> *vcpu_e500)
>   vcpu_e500->gtlb_params[1].ways = KVM_E500_TLB1_SIZE;
>   vcpu_e500->gtlb_params[1].sets = 1;
>
> - vcpu_e500->gtlb_arch = kmalloc(entries * entry_size, GFP_KERNEL);
> + vcpu_e500->gtlb_arch = kmalloc_array(KVM_E500_TLB0_SIZE +
> +  KVM_E500_TLB1_SIZE,
> +  sizeof(*vcpu_e500->gtlb_arch),
> +  GFP_KERNEL);

There are changes here that are not mentioned in the commit log.

julia

>   if (!vcpu_e500->gtlb_arch)
>   return -ENOMEM;
>
> --
> 2.9.3
>
> --
> 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 10/11] soc: ti: knav_qmss_queue: use of_property_read_bool

[Julia Lawall]

On Fri, 5 Aug 2016, Robin Murphy wrote:

> Hi Julia,
>
> On 05/08/16 09:56, Julia Lawall wrote:
> > Use of_property_read_bool to check for the existence of a property.
>
> This caught my eye since Rob told me off for doing the same recently[1].
>
> > The semantic patch that makes this change is as follows:
> > (http://coccinelle.lip6.fr/)
> >
> > // 
> > @@
> > expression e1,e2;
> > statement S2,S1;
> > @@
> > -   if (of_get_property(e1,e2,NULL))
> > +   if (of_property_read_bool(e1,e2))
> > S1 else S2
> > // 
> >
> > Signed-off-by: Julia Lawall <julia.law...@lip6.fr>
> >
> > ---
> >  drivers/soc/ti/knav_qmss_queue.c |2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/soc/ti/knav_qmss_queue.c 
> > b/drivers/soc/ti/knav_qmss_queue.c
> > index b73e353..56b5d7c 100644
> > --- a/drivers/soc/ti/knav_qmss_queue.c
> > +++ b/drivers/soc/ti/knav_qmss_queue.c
> > @@ -1240,7 +1240,7 @@ static int knav_setup_queue_range(struct knav_device 
> > *kdev,
> > if (of_get_property(node, "qalloc-by-id", NULL))
>
> According to the binding, "qalloc-by-id" _is_ a boolean property, so
> this one really does deserve to be of_property_read_bool()...
>
> > range->flags |= RANGE_RESERVED;
> >
> > -   if (of_get_property(node, "accumulator", NULL)) {
> > +   if (of_property_read_bool(node, "accumulator")) {
>
> ...whereas "accumulator" must have a value, so this isn't technically
> appropriate. In general, most of these "if the property exists, read the
> property and do stuff" checks are probably a sign of code that could be
> simplified by refactoring the "do stuff" step to just specifically
> handle the "read the property" step returning -EINVAL when it's not present.

Thanks for the very helpful feedback.  I will rethink the patch set in
light of this information.

julia

> Robin.
>
> [1]:https://www.mail-archive.com/iommu@lists.linux-foundation.org/msg13375.html
>
> > ret = knav_init_acc_range(kdev, node, range);
> > if (ret < 0) {
> > devm_kfree(dev, range);
> >
> >
> > ___
> > linux-arm-kernel mailing list
> > linux-arm-ker...@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> >
>
> --
> 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 09/11] powerpc/mpic: use of_property_read_bool

[Julia Lawall]

Use of_property_read_bool to check for the existence of a property.

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

// 
@@
expression e1,e2;
statement S2,S1;
@@
-   if (of_get_property(e1,e2,NULL))
+   if (of_property_read_bool(e1,e2))
S1 else S2
// 

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

---
 arch/powerpc/sysdev/mpic.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/sysdev/mpic.c b/arch/powerpc/sysdev/mpic.c
index 7de45b2..26d9c3f 100644
--- a/arch/powerpc/sysdev/mpic.c
+++ b/arch/powerpc/sysdev/mpic.c
@@ -1249,7 +1249,7 @@ struct mpic * __init mpic_alloc(struct device_node *node,
/* Pick the physical address from the device tree if unspecified */
if (!phys_addr) {
/* Check if it is DCR-based */
-   if (of_get_property(node, "dcr-reg", NULL)) {
+   if (of_property_read_bool(node, "dcr-reg")) {
flags |= MPIC_USES_DCR;
} else {
struct resource r;

[PATCH 01/11] fsl/qe: use of_property_read_bool

[Julia Lawall]

Use of_property_read_bool to check for the existence of a property.

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

// 
@@
expression e1,e2;
statement S2,S1;
@@
-   if (of_get_property(e1,e2,NULL))
+   if (of_property_read_bool(e1,e2))
S1 else S2
// 

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

---
 drivers/soc/fsl/qe/qe_tdm.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/soc/fsl/qe/qe_tdm.c b/drivers/soc/fsl/qe/qe_tdm.c
index 5e48b14..0ac98e6 100644
--- a/drivers/soc/fsl/qe/qe_tdm.c
+++ b/drivers/soc/fsl/qe/qe_tdm.c
@@ -99,7 +99,7 @@ int ucc_of_parse_tdm(struct device_node *np, struct ucc_tdm 
*utdm,
utdm->tdm_port = val;
ut_info->uf_info.tdm_num = utdm->tdm_port;
 
-   if (of_get_property(np, "fsl,tdm-internal-loopback", NULL))
+   if (of_property_read_bool(np, "fsl,tdm-internal-loopback"))
utdm->tdm_mode = TDM_INTERNAL_LOOPBACK;
else
utdm->tdm_mode = TDM_NORMAL;

[PATCH 00/11] use of_property_read_bool

[Julia Lawall]

Use of_property_read_bool to check for the existence of a property.

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

// 
@@
expression e1,e2;
statement S2,S1;
@@
-   if (of_get_property(e1,e2,NULL))
+   if (of_property_read_bool(e1,e2))
S1 else S2
// 

---

 arch/powerpc/sysdev/mpic.c  |2 +-
 drivers/i2c/busses/i2c-mpc.c|2 +-
 drivers/mmc/host/sdhci-of-esdhc.c   |2 +-
 drivers/net/ethernet/freescale/xgmac_mdio.c |3 +--
 drivers/phy/phy-qcom-ufs.c  |2 +-
 drivers/pinctrl/nomadik/pinctrl-nomadik.c   |2 +-
 drivers/soc/fsl/qe/qe_tdm.c |2 +-
 drivers/soc/ti/knav_qmss_queue.c|2 +-
 drivers/tty/serial/atmel_serial.c   |8 
 drivers/usb/host/fsl-mph-dr-of.c|6 +++---
 sound/soc/codecs/ab8500-codec.c |   10 +-
 sound/soc/sh/rcar/ssi.c |2 +-
 sound/soc/soc-core.c|2 +-
 13 files changed, 22 insertions(+), 23 deletions(-)

Re: [PATCH] cxl: replace loop with for_each_child_of_node(), remove unneeded of_node_put()

[Julia Lawall]

On Fri, 29 Jul 2016, walter harms wrote:

>
>
> Am 29.07.2016 05:55, schrieb Andrew Donnellan:
> > Rewrite the cxl_guest_init_afu() loop in cxl_of_probe() to use
> > for_each_child_of_node() rather than a hand-coded for loop.
> >
> > Remove the useless of_node_put(afu_np) call after the loop, where it's
> > guaranteed that afu_np == NULL.
> >
> > Reported-by: SF Markus Elfring <elfr...@users.sourceforge.net>
> > Reported-by: Julia Lawall <julia.law...@lip6.fr>
> > Signed-off-by: Andrew Donnellan <andrew.donnel...@au1.ibm.com>
> >
> > ---
> >
> > Checked the of_node_put() with Fred, he thinks it was probably just left
> > over from an earlier private version of the code and we can just get rid of
> > it.
> > ---
> >  drivers/misc/cxl/of.c | 8 +++-
> >  1 file changed, 3 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/misc/cxl/of.c b/drivers/misc/cxl/of.c
> > index edc4583..ec175ea 100644
> > --- a/drivers/misc/cxl/of.c
> > +++ b/drivers/misc/cxl/of.c
> > @@ -460,7 +460,7 @@ int cxl_of_probe(struct platform_device *pdev)
> > struct device_node *afu_np = NULL;
> > struct cxl *adapter = NULL;
> > int ret;
> > -   int slice, slice_ok;
> > +   int slice = 0, slice_ok = 0;
> >
> > pr_devel("in %s\n", __func__);
> >
> > @@ -476,13 +476,13 @@ int cxl_of_probe(struct platform_device *pdev)
> > }
> >
> > /* init afu */
> > -   slice_ok = 0;
> > -   for (afu_np = NULL, slice = 0; (afu_np = of_get_next_child(np, 
> > afu_np)); slice++) {
> > +   for_each_child_of_node(np, afu_np) {
> > if ((ret = cxl_guest_init_afu(adapter, slice, afu_np)))
> > dev_err(>dev, "AFU %i failed to initialise: %i\n",
> > slice, ret);
> > else
> > slice_ok++;
> > +   slice++;
> > }
>
> while you are here ..
> you could move the assign out of the condition..
>
> ret = cxl_guest_init_afu(adapter, slice, afu_np);
> if (ret) 

Yes, please.

julia


>
> just my 2 cents,
>
> re,
>  wh
>
> >
> > if (slice_ok == 0) {
> > @@ -490,8 +490,6 @@ int cxl_of_probe(struct platform_device *pdev)
> > adapter->slices = 0;
> > }
> >
> > -   if (afu_np)
> > -   of_node_put(afu_np);
> > return 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
>
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH] cxl: Delete an unnecessary check before the function call "of_node_put"

[Julia Lawall]

On Wed, 20 Jul 2016, SF Markus Elfring wrote:

> From: Markus Elfring 
> Date: Wed, 20 Jul 2016 15:10:32 +0200
>
> The of_node_put() function tests whether its argument is NULL
> and then returns immediately.
> Thus the test around the call is not needed.
>
> This issue was detected by using the Coccinelle software.
>
> Signed-off-by: Markus Elfring 
> ---
>  drivers/misc/cxl/of.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/misc/cxl/of.c b/drivers/misc/cxl/of.c
> index edc4583..333256a 100644
> --- a/drivers/misc/cxl/of.c
> +++ b/drivers/misc/cxl/of.c
> @@ -490,8 +490,7 @@ int cxl_of_probe(struct platform_device *pdev)
>   adapter->slices = 0;
>   }
>
> - if (afu_np)
> - of_node_put(afu_np);
> + of_node_put(afu_np);
>   return 0;
>  }

I don't think that the call should be there at all.  The loop only exits
when afu_np is NULL.  Furthermore, the loop should not be written as a for
loop, but rather with for_each_child_of_node.

julia
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH 0/5] Use dma_pool_zalloc

[Julia Lawall]

Dma_pool_zalloc combines dma_pool_alloc and memset 0.  The semantic patch
that makes this transformation is as follows: (http://coccinelle.lip6.fr/)

// 
@@
type T;
T *d;
expression e;
statement S;
@@

d =
-dma_pool_alloc
+dma_pool_zalloc
 (...);
if (!d) S
-   memset(d, 0, sizeof(T));

@@
expression d,e;
statement S;
@@

d =
-dma_pool_alloc
+dma_pool_zalloc
 (...);
if (!d) S
-   memset(d, 0, sizeof(*d));
// 

---

 drivers/crypto/marvell/tdma.c|5 ++---
 drivers/dma/fsldma.c |3 +--
 drivers/dma/ioat/init.c  |5 ++---
 drivers/dma/mmp_pdma.c   |3 +--
 drivers/dma/xilinx/xilinx_vdma.c |3 +--
 5 files changed, 7 insertions(+), 12 deletions(-)
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH 5/5] dmaengine: fsldma: Use dma_pool_zalloc

[Julia Lawall]

Dma_pool_zalloc combines dma_pool_alloc and memset 0.  The semantic patch
that makes this transformation is as follows: (http://coccinelle.lip6.fr/)

// 
@@
expression d,e;
statement S;
@@

d =
-dma_pool_alloc
+dma_pool_zalloc
 (...);
if (!d) S
-   memset(d, 0, sizeof(*d));
// 

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

---
 drivers/dma/fsldma.c |3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c
index aac85c3..a8828ed 100644
--- a/drivers/dma/fsldma.c
+++ b/drivers/dma/fsldma.c
@@ -462,13 +462,12 @@ static struct fsl_desc_sw 
*fsl_dma_alloc_descriptor(struct fsldma_chan *chan)
struct fsl_desc_sw *desc;
dma_addr_t pdesc;
 
-   desc = dma_pool_alloc(chan->desc_pool, GFP_ATOMIC, );
+   desc = dma_pool_zalloc(chan->desc_pool, GFP_ATOMIC, );
if (!desc) {
chan_dbg(chan, "out of memory for link descriptor\n");
return NULL;
}
 
-   memset(desc, 0, sizeof(*desc));
INIT_LIST_HEAD(>tx_list);
dma_async_tx_descriptor_init(>async_tx, >common);
desc->async_tx.tx_submit = fsl_dma_tx_submit;

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH] xen/hvc: constify hv_ops structures

[Julia Lawall]

These hv_ops structures are never modified, so declare them as const.  Most
were const already.

Done with the help of Coccinelle.

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

---
 drivers/tty/hvc/hvc_xen.c |4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/hvc/hvc_xen.c b/drivers/tty/hvc/hvc_xen.c
index fa816b7..1505056 100644
--- a/drivers/tty/hvc/hvc_xen.c
+++ b/drivers/tty/hvc/hvc_xen.c
@@ -162,7 +162,7 @@ static int domU_read_console(uint32_t vtermno, char *buf, 
int len)
return recv;
 }
 
-static struct hv_ops domU_hvc_ops = {
+static const struct hv_ops domU_hvc_ops = {
.get_chars = domU_read_console,
.put_chars = domU_write_console,
.notifier_add = notifier_add_irq,
@@ -188,7 +188,7 @@ static int dom0_write_console(uint32_t vtermno, const char 
*str, int len)
return len;
 }
 
-static struct hv_ops dom0_hvc_ops = {
+static const struct hv_ops dom0_hvc_ops = {
.get_chars = dom0_read_console,
.put_chars = dom0_write_console,
.notifier_add = notifier_add_irq,

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH 0/4] add NULL test

[Julia Lawall]

Add NULL tests on various calls to kzalloc and devm_kzalloc.

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

// 
@@
expression x,y;
identifier fld;
@@

(
x = \(vmalloc\|kmalloc\|kzalloc\|kcalloc\|kmem_cache_alloc\|krealloc\|
kmemdup\|kstrdup\|
devm_kzalloc\|devm_kmalloc\|devm_kcalloc\|devm_kasprintf\|
kmalloc_array\)(...,<+... __GFP_NOFAIL ...+>,...);
|
* x = \(vmalloc\|kmalloc\|kzalloc\|kcalloc\|kmem_cache_alloc\|krealloc\|
kmemdup\|kstrdup\|
devm_kzalloc\|devm_kmalloc\|devm_kcalloc\|devm_kasprintf\|
kmalloc_array\)(...);
)
  ... when != (x) == NULL
  when != (x) != NULL
  when != (x) == 0
  when != (x) != 0
  when != x = y
(
  x->fld
|
  *x
|
  x[...]
)
// 

---

 drivers/s390/char/con3215.c |2 ++
 drivers/s390/char/raw3270.c |2 ++
 sound/soc/fsl/imx-pcm-dma.c |2 ++
 sound/soc/intel/baytrail/sst-baytrail-pcm.c |2 ++
 sound/soc/omap/omap-hdmi-audio.c|2 ++
 5 files changed, 10 insertions(+)
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH 1/4] ASoC: imx-pcm-dma: add NULL test

[Julia Lawall]

Add NULL test on call to devm_kzalloc.

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

// 
@@
expression x;
@@

* x = devm_kzalloc(...);
  ... when != x == NULL
  *x
// 

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

---
 sound/soc/fsl/imx-pcm-dma.c |2 ++
 1 file changed, 2 insertions(+)

diff --git a/sound/soc/fsl/imx-pcm-dma.c b/sound/soc/fsl/imx-pcm-dma.c
index 1fc01ed..f3d3d1f 100644
--- a/sound/soc/fsl/imx-pcm-dma.c
+++ b/sound/soc/fsl/imx-pcm-dma.c
@@ -62,6 +62,8 @@ int imx_pcm_dma_init(struct platform_device *pdev, size_t 
size)
 
config = devm_kzalloc(>dev,
sizeof(struct snd_dmaengine_pcm_config), GFP_KERNEL);
+   if (!config)
+   return -ENOMEM;
*config = imx_dmaengine_pcm_config;
if (size)
config->prealloc_buffer_size = size;

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH 3/6] powerpc/powernv: add missing of_node_put

[Julia Lawall]

for_each_compatible_node performs an of_node_get on each iteration, so
a break out of the loop requires an of_node_put.

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

// 
@@
local idexpression n;
expression e;
@@

 for_each_compatible_node(n,...) {
   ...
(
   of_node_put(n);
|
   e = n
|
+  of_node_put(n);
?  break;
)
   ...
 }
... when != n
// 

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

---
 arch/powerpc/platforms/powernv/opal-lpc.c |1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/powerpc/platforms/powernv/opal-lpc.c 
b/arch/powerpc/platforms/powernv/opal-lpc.c
index e4169d6..d28c4a9 100644
--- a/arch/powerpc/platforms/powernv/opal-lpc.c
+++ b/arch/powerpc/platforms/powernv/opal-lpc.c
@@ -401,6 +401,7 @@ void opal_lpc_init(void)
if (!of_get_property(np, "primary", NULL))
continue;
opal_lpc_chip_id = of_get_ibm_chip_id(np);
+   of_node_put(np);
break;
}
if (opal_lpc_chip_id < 0)

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH 2/6] powerpc/pseries: add missing of_node_put

[Julia Lawall]

for_each_node_by_type performs an of_node_get on each iteration, so a break
out of the loop requires an of_node_put.  In this code, there are two paths
that break out of the loop.  In the first one (open-pic case), the code
originally contained an of_node_get.  That increments the reference count,
which cancels out against the decrement of the reference count that is
needed here, and thus the patch drops the of_node_get.  In the second one
(ppc-xicp case), there is no need to save the node value, and so an
of_node_put is added as described by the following semantic patch rule
(http://coccinelle.lip6.fr):

// 
@@
expression e,e1;
local idexpression n;
@@

 for_each_node_by_type(n, e1) {
   ... when != of_node_put(n)
   when != e = n
(
   return n;
|
+  of_node_put(n);
?  return ...;
)
   ...
 }
// 

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

---
 arch/powerpc/platforms/pseries/setup.c |3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/platforms/pseries/setup.c 
b/arch/powerpc/platforms/pseries/setup.c
index 36df46e..7185cd3 100644
--- a/arch/powerpc/platforms/pseries/setup.c
+++ b/arch/powerpc/platforms/pseries/setup.c
@@ -236,7 +236,7 @@ static void __init pseries_discover_pic(void)
for_each_node_by_name(np, "interrupt-controller") {
typep = of_get_property(np, "compatible", NULL);
if (strstr(typep, "open-pic")) {
-   pSeries_mpic_node = of_node_get(np);
+   pSeries_mpic_node = np;
ppc_md.init_IRQ   = pseries_mpic_init_IRQ;
setup_kexec_cpu_down_mpic();
smp_init_pseries_mpic();
@@ -245,6 +245,7 @@ static void __init pseries_discover_pic(void)
ppc_md.init_IRQ   = pseries_xics_init_IRQ;
setup_kexec_cpu_down_xics();
smp_init_pseries_xics();
+   of_node_put(np);
return;
}
}

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH 0/6] add missing of_node_put

[Julia Lawall]

The various for_each device_node iterators performs an of_node_get on each
iteration, so a break out of the loop requires an of_node_put.

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

// 
@r@
local idexpression n;
expression e1,e2;
iterator name for_each_node_by_name, for_each_node_by_type,
for_each_compatible_node, for_each_matching_node,
for_each_matching_node_and_match, for_each_child_of_node,
for_each_available_child_of_node, for_each_node_with_property;
iterator i;
statement S;
expression list [n1] es;
@@

(
(
for_each_node_by_name(n,e1) S
|
for_each_node_by_type(n,e1) S
|
for_each_compatible_node(n,e1,e2) S
|
for_each_matching_node(n,e1) S
|
for_each_matching_node_and_match(n,e1,e2) S
|
for_each_child_of_node(e1,n) S
|
for_each_available_child_of_node(e1,n) S
|
for_each_node_with_property(n,e1) S
)
&
i(es,n,...) S
)

@@
local idexpression r.n;
iterator r.i;
expression e;
expression list [r.n1] es;
@@

 i(es,n,...) {
   ...
(
   of_node_put(n);
|
   e = n
|
   return n;
|
+  of_node_put(n);
?  return ...;
)
   ...
 }

@@
local idexpression r.n;
iterator r.i;
expression e;
expression list [r.n1] es;
@@

 i(es,n,...) {
   ...
(
   of_node_put(n);
|
   e = n
|
+  of_node_put(n);
?  break;
)
   ...
 }
... when != n

@@
local idexpression r.n;
iterator r.i;
expression e;
identifier l;
expression list [r.n1] es;
@@

 i(es,n,...) {
   ...
(
   of_node_put(n);
|
   e = n
|
+  of_node_put(n);
?  goto l;
)
   ...
 }
...
l: ... when != n// 

---

 arch/powerpc/kernel/btext.c   |4 +++-
 arch/powerpc/kernel/machine_kexec_64.c|4 +++-
 arch/powerpc/platforms/cell/iommu.c   |1 +
 arch/powerpc/platforms/embedded6xx/hlwd-pic.c |1 +
 arch/powerpc/platforms/powernv/opal-lpc.c |1 +
 arch/powerpc/platforms/pseries/setup.c|3 ++-
 6 files changed, 11 insertions(+), 3 deletions(-)
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH 1/6] powerpc/6xx: add missing of_node_put

[Julia Lawall]

for_each_compatible_node performs an of_node_get on each iteration, so
a break out of the loop requires an of_node_put.

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

// 
@@
expression e;
local idexpression n;
@@

@@
local idexpression n;
expression e;
@@

 for_each_compatible_node(n,...) {
   ...
(
   of_node_put(n);
|
   e = n
|
+  of_node_put(n);
?  break;
)
   ...
 }
... when != n
// 

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

---
 arch/powerpc/platforms/embedded6xx/hlwd-pic.c |1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/powerpc/platforms/embedded6xx/hlwd-pic.c 
b/arch/powerpc/platforms/embedded6xx/hlwd-pic.c
index 9b79757..b581b42 100644
--- a/arch/powerpc/platforms/embedded6xx/hlwd-pic.c
+++ b/arch/powerpc/platforms/embedded6xx/hlwd-pic.c
@@ -215,6 +215,7 @@ void hlwd_pic_probe(void)
irq_set_chained_handler(cascade_virq,
hlwd_pic_irq_cascade);
hlwd_irq_host = host;
+   of_node_put(np);
break;
}
}

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH 5/6] powerpc/btext: add missing of_node_put

[Julia Lawall]

for_each_node_by_type performs an of_node_get on each iteration, so
a break out of the loop requires an of_node_put.

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

// 
@@
local idexpression n;
expression e;
@@

 for_each_node_by_type(n,...) {
   ...
(
   of_node_put(n);
|
   e = n
|
+  of_node_put(n);
?  break;
)
   ...
 }
... when != n
// 

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

---
 arch/powerpc/kernel/btext.c |4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/kernel/btext.c b/arch/powerpc/kernel/btext.c
index 41c011c..8d05ef2 100644
--- a/arch/powerpc/kernel/btext.c
+++ b/arch/powerpc/kernel/btext.c
@@ -257,8 +257,10 @@ int __init btext_find_display(int allow_nonstdout)
rc = btext_initialize(np);
printk("result: %d\n", rc);
}
-   if (rc == 0)
+   if (rc == 0) {
+   of_node_put(np);
break;
+   }
}
return rc;
 }

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH 4/6] powerpc/kexec: add missing of_node_put

[Julia Lawall]

for_each_node_by_type performs an of_node_get on each iteration, so
a break out of the loop requires an of_node_put.

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

// 
@@
expression e,e1;
local idexpression n;
@@

 for_each_node_by_type(n, e1) {
   ... when != of_node_put(n)
   when != e = n
(
   return n;
|
+  of_node_put(n);
?  return ...;
)
   ...
 }
// 

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

---
 arch/powerpc/kernel/machine_kexec_64.c |4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/kernel/machine_kexec_64.c 
b/arch/powerpc/kernel/machine_kexec_64.c
index 0fbd75d..4ba61ce 100644
--- a/arch/powerpc/kernel/machine_kexec_64.c
+++ b/arch/powerpc/kernel/machine_kexec_64.c
@@ -103,8 +103,10 @@ int default_machine_kexec_prepare(struct kimage *image)
begin = image->segment[i].mem;
end = begin + image->segment[i].memsz;
 
-   if ((begin < high) && (end > low))
+   if ((begin < high) && (end > low)) {
+   of_node_put(node);
return -ETXTBSY;
+   }
}
}
 

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH 6/6] powerpc/cell: add missing of_node_put

[Julia Lawall]

for_each_node_by_name performs an of_node_get on each iteration, so
a break out of the loop requires an of_node_put.

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

// 
@@
expression e,e1;
local idexpression n;
@@

 for_each_node_by_name(n, e1) {
   ... when != of_node_put(n)
   when != e = n
(
   return n;
|
+  of_node_put(n);
?  return ...;
)
   ...
 }
// 

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

---
 arch/powerpc/platforms/cell/iommu.c |1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/powerpc/platforms/cell/iommu.c 
b/arch/powerpc/platforms/cell/iommu.c
index 14a582b..4edceff 100644
--- a/arch/powerpc/platforms/cell/iommu.c
+++ b/arch/powerpc/platforms/cell/iommu.c
@@ -1107,6 +1107,7 @@ static int __init cell_iommu_fixed_mapping_init(void)
if (hbase < dbase || (hend > (dbase + dsize))) {
pr_debug("iommu: hash window doesn't fit in"
 "real DMA window\n");
+   of_node_put(np);
return -1;
}
}

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH] powerpc/mpc5xxx: Use of_get_next_parent to simplify code

[Julia Lawall]

On Sun, 11 Oct 2015, Christophe JAILLET wrote:

> of_get_next_parent can be used to simplify the while() loop and
> avoid the need of a temp variable.

Can you do something with the loop in __of_translate_address, in 
drivers/of/address.c?  Is there not an iterator for this?

julia


> 
> Signed-off-by: Christophe JAILLET 
> ---
>  arch/powerpc/sysdev/mpc5xxx_clocks.c | 5 +
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/arch/powerpc/sysdev/mpc5xxx_clocks.c 
> b/arch/powerpc/sysdev/mpc5xxx_clocks.c
> index f4f0301..5732926 100644
> --- a/arch/powerpc/sysdev/mpc5xxx_clocks.c
> +++ b/arch/powerpc/sysdev/mpc5xxx_clocks.c
> @@ -13,7 +13,6 @@
>  
>  unsigned long mpc5xxx_get_bus_frequency(struct device_node *node)
>  {
> - struct device_node *np;
>   const unsigned int *p_bus_freq = NULL;
>  
>   of_node_get(node);
> @@ -22,9 +21,7 @@ unsigned long mpc5xxx_get_bus_frequency(struct device_node 
> *node)
>   if (p_bus_freq)
>   break;
>  
> - np = of_get_parent(node);
> - of_node_put(node);
> - node = np;
> + node = of_get_next_parent(node);
>   }
>   of_node_put(node);
>  
> -- 
> 2.1.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
> 
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH 9/9] sound/ppc/snd_ps3.c: fix error return code

[Julia Lawall]

Please ignore. Wrong patch set.

On Sun, 5 Apr 2015, Julia Lawall wrote:

 From: Julia Lawall julia.law...@lip6.fr
 
 Initialize ret before returning on failure, as done elsewhere in the
 function.
 
 A simplified version of the semantic match that finds this problem is as
 follows: (http://coccinelle.lip6.fr/)
 
 // smpl
 (
 if@p1 (\(ret  0\|ret != 0\))
  { ... return ret; }
 |
 ret@p1 = 0
 )
 ... when != ret = e1
 when != ret
 *if(...)
 {
   ... when != ret = e2
   when forall
  return ret;
 }
 
 // /smpl
 
 Signed-off-by: Julia Lawall julia.law...@lip6.fr
 
 ---
  sound/ppc/snd_ps3.c |1 +
  1 file changed, 1 insertion(+)
 
 diff --git a/sound/ppc/snd_ps3.c b/sound/ppc/snd_ps3.c
 index 1aa52ef..9b18b52 100644
 --- a/sound/ppc/snd_ps3.c
 +++ b/sound/ppc/snd_ps3.c
 @@ -1040,6 +1040,7 @@ static int __devinit snd_ps3_driver_probe(struct 
 ps3_system_bus_device *dev)
  GFP_KERNEL);
   if (!the_card.null_buffer_start_vaddr) {
   pr_info(%s: nullbuffer alloc failed\n, __func__);
 + ret = -ENOMEM;
   goto clean_preallocate;
   }
   pr_debug(%s: null vaddr=%p dma=%#llx\n, __func__,
 
 --
 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
 
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH 9/9] sound/ppc/snd_ps3.c: fix error return code

[Julia Lawall]

From: Julia Lawall julia.law...@lip6.fr

Initialize ret before returning on failure, as done elsewhere in the
function.

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

// smpl
(
if@p1 (\(ret  0\|ret != 0\))
 { ... return ret; }
|
ret@p1 = 0
)
... when != ret = e1
when != ret
*if(...)
{
  ... when != ret = e2
  when forall
 return ret;
}

// /smpl

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

---
 sound/ppc/snd_ps3.c |1 +
 1 file changed, 1 insertion(+)

diff --git a/sound/ppc/snd_ps3.c b/sound/ppc/snd_ps3.c
index 1aa52ef..9b18b52 100644
--- a/sound/ppc/snd_ps3.c
+++ b/sound/ppc/snd_ps3.c
@@ -1040,6 +1040,7 @@ static int __devinit snd_ps3_driver_probe(struct 
ps3_system_bus_device *dev)
   GFP_KERNEL);
if (!the_card.null_buffer_start_vaddr) {
pr_info(%s: nullbuffer alloc failed\n, __func__);
+   ret = -ENOMEM;
goto clean_preallocate;
}
pr_debug(%s: null vaddr=%p dma=%#llx\n, __func__,

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH 2/15] powerpc: don't export static symbol

[Julia Lawall]

From: Julia Lawall julia.law...@lip6.fr

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

// smpl
@r@
type T;
identifier f;
@@

static T f (...) { ... }

@@
identifier r.f;
declarer name EXPORT_SYMBOL;
@@

-EXPORT_SYMBOL(f);
// /smpl

Furthermore, the function is never used, so its definition is dropped as
well.

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

---
 arch/powerpc/sysdev/qe_lib/qe_io.c |   25 -
 1 file changed, 25 deletions(-)

diff --git a/arch/powerpc/sysdev/qe_lib/qe_io.c 
b/arch/powerpc/sysdev/qe_lib/qe_io.c
index d099941..7ea0174 100644
--- a/arch/powerpc/sysdev/qe_lib/qe_io.c
+++ b/arch/powerpc/sysdev/qe_lib/qe_io.c
@@ -190,28 +190,3 @@ int par_io_of_config(struct device_node *np)
return 0;
 }
 EXPORT_SYMBOL(par_io_of_config);
-
-#ifdef DEBUG
-static void dump_par_io(void)
-{
-   unsigned int i;
-
-   printk(KERN_INFO %s: par_io=%p\n, __func__, par_io);
-   for (i = 0; i  num_par_io_ports; i++) {
-   printk(KERN_INFO   cpodr[%u]=%08x\n, i,
-   in_be32(par_io[i].cpodr));
-   printk(KERN_INFO   cpdata[%u]=%08x\n, i,
-   in_be32(par_io[i].cpdata));
-   printk(KERN_INFO   cpdir1[%u]=%08x\n, i,
-   in_be32(par_io[i].cpdir1));
-   printk(KERN_INFO   cpdir2[%u]=%08x\n, i,
-   in_be32(par_io[i].cpdir2));
-   printk(KERN_INFO   cppar1[%u]=%08x\n, i,
-   in_be32(par_io[i].cppar1));
-   printk(KERN_INFO   cppar2[%u]=%08x\n, i,
-   in_be32(par_io[i].cppar2));
-   }
-
-}
-EXPORT_SYMBOL(dump_par_io);
-#endif /* DEBUG */
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH 0/15] don't export static symbol

[Julia Lawall]

These patches remove EXPORT_SYMBOL or EXPORT_SYMBOL_GPL declarations on
static functions.

This was done using the following semantic patch:
(http://coccinelle.lip6.fr/)

// smpl
@r@
type T;
identifier f;
@@

static T f (...) { ... }

@@
identifier r.f;
declarer name EXPORT_SYMBOL;
@@

-EXPORT_SYMBOL(f);

@@
identifier r.f;
declarer name EXPORT_SYMBOL_GPL;
@@

-EXPORT_SYMBOL_GPL(f);
// /smpl

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH 0/8] replace memset by memzero_explicit

[Julia Lawall]

Memset on a local variable may be removed when it is called just before the
variable goes out of scope.  Using memzero_explicit defeats this
optimization.  The complete semantic patch that makes this change is as
follows: (http://coccinelle.lip6.fr/)

// smpl
@@
identifier x;
local idexpression e;
type T,T1;
@@

{
... when any
T x[...];
... when any
when exists
(
e = (T1)x
|
e = (T1)x[0]
)
... when any
when exists
- memset
+ memzero_explicit
  (x,
-0,
  ...)
... when != x
when != e
when strict
}

@@
identifier i,x;
local idexpression e;
type T;
@@

{
... when any
struct i x;
... when any
when exists
e = (T)x
... when any
when exists
- memset
+ memzero_explicit
  (x,
-0,
  ...)
... when != x
when != e
when strict
}

// 

@@
identifier x;
type T,T1;
expression e;
@@

{
... when any
T x[...];
... when any
when exists
when != e = (T1)x
when != e = (T1)x[0]
- memset
+ memzero_explicit
  (x,
-0,
  ...)
... when != x
when strict
}

@@
identifier i,x;
expression e;
type T;
@@

{
... when any
struct i x;
... when any
when exists
when != e = (T)x
- memset
+ memzero_explicit
  (x,
-0,
  ...)
... when != x
when strict
}
// /smpl

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH 6/8] crypto: replace memset by memzero_explicit

[Julia Lawall]

From: Julia Lawall ju...@diku.dk

Memset on a local variable may be removed when it is called just before the
variable goes out of scope.  Using memzero_explicit defeats this
optimization.  A simplified version of the semantic patch that makes this
change is as follows: (http://coccinelle.lip6.fr/)

// smpl
@@
identifier x;
type T;
@@

{
... when any
T x[...];
... when any
when exists
- memset
+ memzero_explicit
  (x,
-0,
  ...)
... when != x
when strict
}
// /smpl

This change was suggested by Daniel Borkmann dbork...@redhat.com

Signed-off-by: Julia Lawall ju...@diku.dk

---
Daniel Borkmann suggested that these patches could go through Herbert Xu's
cryptodev tree.

 arch/powerpc/crypto/sha1.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/crypto/sha1.c b/arch/powerpc/crypto/sha1.c
index 0f88c7b..d3feba5 100644
--- a/arch/powerpc/crypto/sha1.c
+++ b/arch/powerpc/crypto/sha1.c
@@ -66,7 +66,7 @@ static int sha1_update(struct shash_desc *desc, const u8 
*data,
src = data + done;
} while (done + 63  len);
 
-   memset(temp, 0, sizeof(temp));
+   memzero_explicit(temp, sizeof(temp));
partial = 0;
}
memcpy(sctx-buffer + partial, src, len - done);

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH 0/8] replace memset by memzero_explicit

[Julia Lawall]

Memset on a local variable may be removed when it is called just before the
variable goes out of scope.  Using memzero_explicit defeats this
optimization.  The complete semantic patch that makes this change is as
follows: (http://coccinelle.lip6.fr/)

// smpl
@@
identifier x;
local idexpression e;
type T,T1;
@@

{
... when any
T x[...];
... when any
when exists
(
e = (T1)x
|
e = (T1)x[0]
)
... when any
when exists
- memset
+ memzero_explicit
  (x,
-0,
  ...)
... when != x
when != e
when strict
}

@@
identifier i,x;
local idexpression e;
type T;
@@

{
... when any
struct i x;
... when any
when exists
e = (T)x
... when any
when exists
- memset
+ memzero_explicit
  (x,
-0,
  ...)
... when != x
when != e
when strict
}

// 

@@
identifier x;
type T,T1;
expression e;
@@

{
... when any
T x[...];
... when any
when exists
when != e = (T1)x
when != e = (T1)x[0]
- memset
+ memzero_explicit
  (x,
-0,
  ...)
... when != x
when strict
}

@@
identifier i,x;
expression e;
type T;
@@

{
... when any
struct i x;
... when any
when exists
when != e = (T)x
- memset
+ memzero_explicit
  (x,
-0,
  ...)
... when != x
when strict
}
// /smpl

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH 6/8 v2] crypto: replace memset by memzero_explicit

[Julia Lawall]

From: Julia Lawall julia.law...@lip6.fr

Memset on a local variable may be removed when it is called just before the
variable goes out of scope.  Using memzero_explicit defeats this
optimization.  A simplified version of the semantic patch that makes this
change is as follows: (http://coccinelle.lip6.fr/)

// smpl
@@
identifier x;
type T;
@@

{
... when any
T x[...];
... when any
when exists
- memset
+ memzero_explicit
  (x,
-0,
  ...)
... when != x
when strict
}
// /smpl

This change was suggested by Daniel Borkmann dbork...@redhat.com

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

---
Daniel Borkmann suggested that these patches could go through Herbert Xu's
cryptodev tree.

v2: fixed email address

 arch/powerpc/crypto/sha1.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/crypto/sha1.c b/arch/powerpc/crypto/sha1.c
index 0f88c7b..d3feba5 100644
--- a/arch/powerpc/crypto/sha1.c
+++ b/arch/powerpc/crypto/sha1.c
@@ -66,7 +66,7 @@ static int sha1_update(struct shash_desc *desc, const u8 
*data,
src = data + done;
} while (done + 63  len);
 
-   memset(temp, 0, sizeof(temp));
+   memzero_explicit(temp, sizeof(temp));
partial = 0;
}
memcpy(sctx-buffer + partial, src, len - done);

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH 9/9] sound/ppc/snd_ps3.c: fix error return code

[Julia Lawall]

From: Julia Lawall julia.law...@lip6.fr

Initialize ret before returning on failure, as done elsewhere in the
function.

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

// smpl
(
if@p1 (\(ret  0\|ret != 0\))
 { ... return ret; }
|
ret@p1 = 0
)
... when != ret = e1
when != ret
*if(...)
{
  ... when != ret = e2
  when forall
 return ret;
}

// /smpl

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

---
 sound/ppc/snd_ps3.c |1 +
 1 file changed, 1 insertion(+)

diff --git a/sound/ppc/snd_ps3.c b/sound/ppc/snd_ps3.c
index 1aa52ef..9b18b52 100644
--- a/sound/ppc/snd_ps3.c
+++ b/sound/ppc/snd_ps3.c
@@ -1040,6 +1040,7 @@ static int __devinit snd_ps3_driver_probe(struct 
ps3_system_bus_device *dev)
   GFP_KERNEL);
if (!the_card.null_buffer_start_vaddr) {
pr_info(%s: nullbuffer alloc failed\n, __func__);
+   ret = -ENOMEM;
goto clean_preallocate;
}
pr_debug(%s: null vaddr=%p dma=%#llx\n, __func__,

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH 9/9] sound/ppc/snd_ps3.c: fix error return code

[Julia Lawall]

Wrong patch, please ignore.

julia

On Sun, 23 Nov 2014, Julia Lawall wrote:

 From: Julia Lawall julia.law...@lip6.fr
 
 Initialize ret before returning on failure, as done elsewhere in the
 function.
 
 A simplified version of the semantic match that finds this problem is as
 follows: (http://coccinelle.lip6.fr/)
 
 // smpl
 (
 if@p1 (\(ret  0\|ret != 0\))
  { ... return ret; }
 |
 ret@p1 = 0
 )
 ... when != ret = e1
 when != ret
 *if(...)
 {
   ... when != ret = e2
   when forall
  return ret;
 }
 
 // /smpl
 
 Signed-off-by: Julia Lawall julia.law...@lip6.fr
 
 ---
  sound/ppc/snd_ps3.c |1 +
  1 file changed, 1 insertion(+)
 
 diff --git a/sound/ppc/snd_ps3.c b/sound/ppc/snd_ps3.c
 index 1aa52ef..9b18b52 100644
 --- a/sound/ppc/snd_ps3.c
 +++ b/sound/ppc/snd_ps3.c
 @@ -1040,6 +1040,7 @@ static int __devinit snd_ps3_driver_probe(struct 
 ps3_system_bus_device *dev)
  GFP_KERNEL);
   if (!the_card.null_buffer_start_vaddr) {
   pr_info(%s: nullbuffer alloc failed\n, __func__);
 + ret = -ENOMEM;
   goto clean_preallocate;
   }
   pr_debug(%s: null vaddr=%p dma=%#llx\n, __func__,
 
 --
 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
 
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH 7/14 v2] powerpc/4xx/cpm: delete unneeded test before of_node_put

[Julia Lawall]

From: Julia Lawall julia.law...@lip6.fr

Simplify the error path to avoid calling of_node_put when it is not needed.

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

// smpl
@@
expression e;
@@

-if (e)
   of_node_put(e);
// /smpl

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

---
v2: new subject

 arch/powerpc/sysdev/ppc4xx_cpm.c |8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/powerpc/sysdev/ppc4xx_cpm.c b/arch/powerpc/sysdev/ppc4xx_cpm.c
index 82e2cfe..ba95adf 100644
--- a/arch/powerpc/sysdev/ppc4xx_cpm.c
+++ b/arch/powerpc/sysdev/ppc4xx_cpm.c
@@ -281,7 +281,7 @@ static int __init cpm_init(void)
printk(KERN_ERR cpm: could not parse dcr property for %s\n,
   np-full_name);
ret = -EINVAL;
-   goto out;
+   goto node_put;
}

cpm.dcr_host = dcr_map(np, dcr_base, dcr_len);
@@ -290,7 +290,7 @@ static int __init cpm_init(void)
printk(KERN_ERR cpm: failed to map dcr property for %s\n,
   np-full_name);
ret = -EINVAL;
-   goto out;
+   goto node_put;
}

/* All 4xx SoCs with a CPM controller have one of two
@@ -330,9 +330,9 @@ static int __init cpm_init(void)

if (cpm.standby || cpm.suspend)
suspend_set_ops(cpm_suspend_ops);
+node_put:
+   of_node_put(np);
 out:
-   if (np)
-   of_node_put(np);
return ret;
 }


___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH 7/14] cpu: delete unneeded test before of_node_put

[Julia Lawall]

From: Julia Lawall julia.law...@lip6.fr

Simplify the error path to avoid calling of_node_put when it is not needed.

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

// smpl
@@
expression e;
@@

-if (e)
   of_node_put(e);
// /smpl

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

---
 arch/powerpc/sysdev/ppc4xx_cpm.c |8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/powerpc/sysdev/ppc4xx_cpm.c b/arch/powerpc/sysdev/ppc4xx_cpm.c
index 82e2cfe..ba95adf 100644
--- a/arch/powerpc/sysdev/ppc4xx_cpm.c
+++ b/arch/powerpc/sysdev/ppc4xx_cpm.c
@@ -281,7 +281,7 @@ static int __init cpm_init(void)
printk(KERN_ERR cpm: could not parse dcr property for %s\n,
   np-full_name);
ret = -EINVAL;
-   goto out;
+   goto node_put;
}
 
cpm.dcr_host = dcr_map(np, dcr_base, dcr_len);
@@ -290,7 +290,7 @@ static int __init cpm_init(void)
printk(KERN_ERR cpm: failed to map dcr property for %s\n,
   np-full_name);
ret = -EINVAL;
-   goto out;
+   goto node_put;
}
 
/* All 4xx SoCs with a CPM controller have one of two
@@ -330,9 +330,9 @@ static int __init cpm_init(void)
 
if (cpm.standby || cpm.suspend)
suspend_set_ops(cpm_suspend_ops);
+node_put:
+   of_node_put(np);
 out:
-   if (np)
-   of_node_put(np);
return ret;
 }
 

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH 6/14] powerpc/fsl: fsl_soc: delete unneeded test before of_node_put

[Julia Lawall]

From: Julia Lawall julia.law...@lip6.fr

Of_node_put supports NULL as its argument, so the initial test is not
necessary.

Suggested by Uwe Kleine-König.

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

// smpl
@@
expression e;
@@

-if (e)
   of_node_put(e);
// /smpl

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

---
 arch/powerpc/sysdev/fsl_soc.c |3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/powerpc/sysdev/fsl_soc.c b/arch/powerpc/sysdev/fsl_soc.c
index ffd1169..bfb5b6c 100644
--- a/arch/powerpc/sysdev/fsl_soc.c
+++ b/arch/powerpc/sysdev/fsl_soc.c
@@ -197,8 +197,7 @@ static int __init setup_rstcr(void)
if (!rstcr  ppc_md.restart == fsl_rstcr_restart)
printk(KERN_ERR No RSTCR register, warm reboot won't work\n);
 
-   if (np)
-   of_node_put(np);
+   of_node_put(np);
 
return 0;
 }

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH 5/14] powerpc/mpc5xxx: delete unneeded test before of_node_put

[Julia Lawall]

From: Julia Lawall julia.law...@lip6.fr

Of_node_put supports NULL as its argument, so the initial test is not
necessary.

Suggested by Uwe Kleine-König.

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

// smpl
@@
expression e;
@@

-if (e)
   of_node_put(e);
// /smpl

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

---
 arch/powerpc/sysdev/mpc5xxx_clocks.c |3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/powerpc/sysdev/mpc5xxx_clocks.c 
b/arch/powerpc/sysdev/mpc5xxx_clocks.c
index 5492dc5..f4f0301 100644
--- a/arch/powerpc/sysdev/mpc5xxx_clocks.c
+++ b/arch/powerpc/sysdev/mpc5xxx_clocks.c
@@ -26,8 +26,7 @@ unsigned long mpc5xxx_get_bus_frequency(struct device_node 
*node)
of_node_put(node);
node = np;
}
-   if (node)
-   of_node_put(node);
+   of_node_put(node);
 
return p_bus_freq ? *p_bus_freq : 0;
 }

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH 4/14] powerpc/pseries: delete unneeded test before of_node_put

[Julia Lawall]

From: Julia Lawall julia.law...@lip6.fr

Of_node_put supports NULL as its argument, so the initial test is not
necessary.

Suggested by Uwe Kleine-König.

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

// smpl
@@
expression e;
@@

-if (e)
   of_node_put(e);
// /smpl

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

---
 arch/powerpc/platforms/pseries/iommu.c |3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/iommu.c 
b/arch/powerpc/platforms/pseries/iommu.c
index 33b552f..2ea6831 100644
--- a/arch/powerpc/platforms/pseries/iommu.c
+++ b/arch/powerpc/platforms/pseries/iommu.c
@@ -575,8 +575,7 @@ static void pci_dma_bus_setup_pSeries(struct pci_bus *bus)
while (isa_dn  isa_dn != dn)
isa_dn = isa_dn-parent;
 
-   if (isa_dn_orig)
-   of_node_put(isa_dn_orig);
+   of_node_put(isa_dn_orig);
 
/* Count number of direct PCI children of the PHB. */
for (children = 0, tmp = dn-child; tmp; tmp = tmp-sibling)

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH 3/14] powerpc: gamecube/wii: delete unneeded test before of_node_put

[Julia Lawall]

From: Julia Lawall julia.law...@lip6.fr

Simplify the error path to avoid calling of_node_put when it is not needed.

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

// smpl
@@
expression e;
@@

-if (e)
   of_node_put(e);
// /smpl

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

---
 arch/powerpc/platforms/embedded6xx/usbgecko_udbg.c |6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/platforms/embedded6xx/usbgecko_udbg.c 
b/arch/powerpc/platforms/embedded6xx/usbgecko_udbg.c
index 20a8ed9..7feb325 100644
--- a/arch/powerpc/platforms/embedded6xx/usbgecko_udbg.c
+++ b/arch/powerpc/platforms/embedded6xx/usbgecko_udbg.c
@@ -247,7 +247,7 @@ void __init ug_udbg_init(void)
np = of_find_compatible_node(NULL, NULL, nintendo,flipper-exi);
if (!np) {
udbg_printf(%s: EXI node not found\n, __func__);
-   goto done;
+   goto out;
}
 
exi_io_base = ug_udbg_setup_exi_io_base(np);
@@ -267,8 +267,8 @@ void __init ug_udbg_init(void)
}
 
 done:
-   if (np)
-   of_node_put(np);
+   of_node_put(np);
+out:
return;
 }
 

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH 0/25] fix error return code

[Julia Lawall]

These patches fix cases where the return variable is not set to an error
code in an error case.

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH 19/25] fix error return code

[Julia Lawall]

From: Julia Lawall julia.law...@lip6.fr

Set the return variable to an error code as done elsewhere in the function.

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

// smpl
(
if@p1 (\(ret  0\|ret != 0\))
 { ... return ret; }
|
ret@p1 = 0
)
... when != ret = e1
when != ret
*if(...)
{
  ... when != ret = e2
  when forall
 return ret;
}

// /smpl

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

---
Not tested.

 drivers/scsi/ps3rom.c |1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/scsi/ps3rom.c b/drivers/scsi/ps3rom.c
index e6e2a30..04d77ce 100644
--- a/drivers/scsi/ps3rom.c
+++ b/drivers/scsi/ps3rom.c
@@ -387,6 +387,7 @@ static int ps3rom_probe(struct ps3_system_bus_device *_dev)
if (!host) {
dev_err(dev-sbd.core, %s:%u: scsi_host_alloc failed\n,
__func__, __LINE__);
+   error = -ENOMEM;
goto fail_teardown;
}
 

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH 8/15] arch/powerpc/platforms/85xx/p1022_ds.c: adjust duplicate test

[Julia Lawall]

From: Julia Lawall julia.law...@lip6.fr

Delete successive tests to the same location.  The code tested the result
of a previous call, that itself was already tested.  It is changed to test
the result of the most recent call.

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

// smpl
@s exists@
local idexpression y;
expression x,e;
@@

*if ( \(x == NULL\|IS_ERR(x)\|y != 0\) )
 { ... when forall
   return ...; }
... when != \(y = e\|y += e\|y -= e\|y |= e\|y = e\|y++\|y--\|y\)
when != \(XT_GETPAGE(...,y)\|WMI_CMD_BUF(...)\)
*if ( \(x == NULL\|IS_ERR(x)\|y != 0\) )
 { ... when forall
   return ...; }
// /smpl

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

---
 arch/powerpc/platforms/85xx/p1022_ds.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/platforms/85xx/p1022_ds.c 
b/arch/powerpc/platforms/85xx/p1022_ds.c
index e346edf..af5853c 100644
--- a/arch/powerpc/platforms/85xx/p1022_ds.c
+++ b/arch/powerpc/platforms/85xx/p1022_ds.c
@@ -302,7 +302,7 @@ static void p1022ds_set_monitor_port(enum 
fsl_diu_monitor_port port)
goto exit;
}
cs1_addr = lbc_br_to_phys(ecm, num_laws, br1);
-   if (!cs0_addr) {
+   if (!cs1_addr) {
pr_err(p1022ds: could not determine physical address for CS1
(BR1=%08x)\n, br1);
goto exit;

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH 3/5] arch/powerpc/kernel/rtas_flash.c: eliminate possible double free

[Julia Lawall]

From: Julia Lawall julia.law...@lip6.fr

The function initialize_flash_pde_data is only called four times.  All four
calls are in the function rtas_flash_init, and on the failure of any of the
calls, remove_flash_pde is called on the third argument of each of the
calls.  There is thus no need for initialize_flash_pde_data to call
remove_flash_pde on the same argument.  remove_flash_pde kfrees the data
field of its argument, and does not clear that field, so this amounts ot a
possible double free.

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

// smpl
@r@
identifier f,free,a;
parameter list[n] ps;
type T;
expression e;
@@

f(ps,T a,...) {
  ... when any
  when != a = e
  if(...) { ... free(a); ... return ...; }
  ... when any
}

@@
identifier r.f,r.free;
expression x,a;
expression list[r.n] xs;
@@

* x = f(xs,a,...);
  if (...) { ... free(a); ... return ...; }
// /smpl

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

---
Not tested.

 arch/powerpc/kernel/rtas_flash.c |4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/arch/powerpc/kernel/rtas_flash.c b/arch/powerpc/kernel/rtas_flash.c
index 20b0120..8329190 100644
--- a/arch/powerpc/kernel/rtas_flash.c
+++ b/arch/powerpc/kernel/rtas_flash.c
@@ -650,10 +650,8 @@ static int initialize_flash_pde_data(const char 
*rtas_call_name,
int token;
 
dp-data = kzalloc(buf_size, GFP_KERNEL);
-   if (dp-data == NULL) {
-   remove_flash_pde(dp);
+   if (dp-data == NULL)
return -ENOMEM;
-   }
 
/*
 * This code assumes that the status int is the first member of the

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH 4/4] drivers/mtd/nand/mpc5121_nfc.c: some devm_ cleanups

[Julia Lawall]

On Tue, 4 Sep 2012, Lars-Peter Clausen wrote:

 On 09/04/2012 10:42 AM, Artem Bityutskiy wrote:
  Aiaiai! :-) [1] [2]
 
  I've build-tested this using aiaiai and it reports that this change breaks 
  the build:
 
  dedekind@blue:~/git/maintaining$ ./verify ../l2-mtd/ mpc5121_nfc  
  ~/tmp/julia2.mbox
  Tested the patch(es) on top of the following commits:
  ba64756 Quick fixes - applied by aiaiai
  651c6fa JFFS2: don't fail on bitflips in OOB
  e22ac84 mtd: autcpu12-nvram: drop frees of devm_ alloc'd data
  ea9d312 mtd: cmdlinepart: minor cleanups
 
  
  Failed to build the following commit for configuration 
  powerpc-mpc512x_defconfig (architecture powerpc):
 
  0fe13ab drivers/mtd/nand/mpc5121_nfc.c: some devm_ cleanups
 
  ...
  drivers/mtd/nand/mpc5121_nfc.c: In function 'mpc5121_nfc_probe':
  drivers/mtd/nand/mpc5121_nfc.c:622:28: warning: variable 'regs_size' set 
  but not used [-Wunused-but-set-variable]
  drivers/mtd/nand/mpc5121_nfc.c:622:16: warning: variable 'regs_paddr' set 
  but not used [-Wunused-but-set-variable]
  drivers/built-in.o: In function `mpc5121_nfc_probe':
  mpc5121_nfc.c:(.devinit.text+0x2a14): undefined reference to `devm_clk_get'
  make[1]: *** [vmlinux] Error 1
 
  
 
  I do not really know why, but it seems that clock framework is not 
  supported for powerpc. CCing the PPC mailing list. Preserved the context 
  below for the PPC people.
 

 I've been bitten by the same issue recently, also cause by one of these
 cocci devm patches. devm_clk_get is only available if the generic
 clk_get/clk_put implementation is used. Not all architectures do this and
 some implement their own clk_get/clk_put, etc functions. Since devm_clk_get
 is merely a wrapper around clk_get/clk_put there is no reason why it should
 depend CLKDEV_LOOKUP. I've prepared a patch which makes them generically
 available if the clk_get/clk_put are implemented (i.e. if HAVE_CLK is set),
 but it is on a different machine right now, will try to submit it later today.

Sorry about this.  I wasn't aware that devm_clk_get wasn't supported by
all architectures, and I have no way of compiling code for these
architectures...  But I wonder why it is not, since devm-ness doesn't seem
to have anything to do with architecture-specific details?  It would be
really nice to have it for all architectures, because the clock functions
are just as (or at least almost as) common as kzalloc, ioremap, etc.

thanks,
julia
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev