2016-12-17 11:30:08

by Bhumika Goyal

[permalink] [raw]
Subject: [PATCH] drivers: remoteproc: constify rproc_ops structures

Declare rproc_ops structures as const as they are only passed as an
argument to the function rproc_alloc. This argument is of type const, so
rproc_ops structures having this property can be declared const too.
Done using Coccinelle:

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

@ok1@
identifier r1.i;
position p;
@@
rproc_alloc(...,&i@p,...)

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

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

File sizes before:
text data bss dec hex filename
1258 416 0 1674 68a remoteproc/omap_remoteproc.o
2402 240 0 2642 a52 remoteproc/st_remoteproc.o
2064 272 0 2336 920 remoteproc/st_slim_rproc.o
2160 240 0 2400 960 remoteproc/wkup_m3_rproc.o

File sizes after:
text data bss dec hex filename
1297 368 0 1665 681 remoteproc/omap_remoteproc.o
2434 192 0 2626 a42 remoteproc/st_remoteproc.o
2112 240 0 2352 930 remoteproc/st_slim_rproc.o
2200 192 0 2392 958 remoteproc/wkup_m3_rproc.o

Signed-off-by: Bhumika Goyal <[email protected]>
---
drivers/remoteproc/omap_remoteproc.c | 2 +-
drivers/remoteproc/st_remoteproc.c | 2 +-
drivers/remoteproc/st_slim_rproc.c | 2 +-
drivers/remoteproc/wkup_m3_rproc.c | 2 +-
4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/remoteproc/omap_remoteproc.c b/drivers/remoteproc/omap_remoteproc.c
index fa63bf2..a96ce90 100644
--- a/drivers/remoteproc/omap_remoteproc.c
+++ b/drivers/remoteproc/omap_remoteproc.c
@@ -177,7 +177,7 @@ static int omap_rproc_stop(struct rproc *rproc)
return 0;
}

-static struct rproc_ops omap_rproc_ops = {
+static const struct rproc_ops omap_rproc_ops = {
.start = omap_rproc_start,
.stop = omap_rproc_stop,
.kick = omap_rproc_kick,
diff --git a/drivers/remoteproc/st_remoteproc.c b/drivers/remoteproc/st_remoteproc.c
index da4e152..f21787b 100644
--- a/drivers/remoteproc/st_remoteproc.c
+++ b/drivers/remoteproc/st_remoteproc.c
@@ -107,7 +107,7 @@ static int st_rproc_stop(struct rproc *rproc)
return sw_err ?: pwr_err;
}

-static struct rproc_ops st_rproc_ops = {
+static const struct rproc_ops st_rproc_ops = {
.start = st_rproc_start,
.stop = st_rproc_stop,
};
diff --git a/drivers/remoteproc/st_slim_rproc.c b/drivers/remoteproc/st_slim_rproc.c
index 507716c..6cfd862 100644
--- a/drivers/remoteproc/st_slim_rproc.c
+++ b/drivers/remoteproc/st_slim_rproc.c
@@ -200,7 +200,7 @@ static void *slim_rproc_da_to_va(struct rproc *rproc, u64 da, int len)
return va;
}

-static struct rproc_ops slim_rproc_ops = {
+static const struct rproc_ops slim_rproc_ops = {
.start = slim_rproc_start,
.stop = slim_rproc_stop,
.da_to_va = slim_rproc_da_to_va,
diff --git a/drivers/remoteproc/wkup_m3_rproc.c b/drivers/remoteproc/wkup_m3_rproc.c
index 18175d0..1ada0e5 100644
--- a/drivers/remoteproc/wkup_m3_rproc.c
+++ b/drivers/remoteproc/wkup_m3_rproc.c
@@ -111,7 +111,7 @@ static void *wkup_m3_rproc_da_to_va(struct rproc *rproc, u64 da, int len)
return va;
}

-static struct rproc_ops wkup_m3_rproc_ops = {
+static const struct rproc_ops wkup_m3_rproc_ops = {
.start = wkup_m3_rproc_start,
.stop = wkup_m3_rproc_stop,
.da_to_va = wkup_m3_rproc_da_to_va,
--
1.9.1


2016-12-22 21:22:40

by Suman Anna

[permalink] [raw]
Subject: Re: [PATCH] drivers: remoteproc: constify rproc_ops structures

On 12/17/2016 05:29 AM, Bhumika Goyal wrote:
> Declare rproc_ops structures as const as they are only passed as an
> argument to the function rproc_alloc. This argument is of type const, so
> rproc_ops structures having this property can be declared const too.
> Done using Coccinelle:
>
> @r1 disable optional_qualifier @
> identifier i;
> position p;
> @@
> static struct rproc_ops i@p = {...};
>
> @ok1@
> identifier r1.i;
> position p;
> @@
> rproc_alloc(...,&i@p,...)
>
> @bad@
> position p!={r1.p,ok1.p};
> identifier r1.i;
> @@
> i@p
>
> @depends on !bad disable optional_qualifier@
> identifier r1.i;
> @@
> +const
> struct rproc_ops i;
>
> File sizes before:
> text data bss dec hex filename
> 1258 416 0 1674 68a remoteproc/omap_remoteproc.o
> 2402 240 0 2642 a52 remoteproc/st_remoteproc.o
> 2064 272 0 2336 920 remoteproc/st_slim_rproc.o
> 2160 240 0 2400 960 remoteproc/wkup_m3_rproc.o
>
> File sizes after:
> text data bss dec hex filename
> 1297 368 0 1665 681 remoteproc/omap_remoteproc.o
> 2434 192 0 2626 a42 remoteproc/st_remoteproc.o
> 2112 240 0 2352 930 remoteproc/st_slim_rproc.o
> 2200 192 0 2392 958 remoteproc/wkup_m3_rproc.o
>
> Signed-off-by: Bhumika Goyal <[email protected]>

Thanks for the cleanup. The da8xx_remoteproc.c file could also use the
same fix.

regards
Suman

> ---
> drivers/remoteproc/omap_remoteproc.c | 2 +-
> drivers/remoteproc/st_remoteproc.c | 2 +-
> drivers/remoteproc/st_slim_rproc.c | 2 +-
> drivers/remoteproc/wkup_m3_rproc.c | 2 +-
> 4 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/remoteproc/omap_remoteproc.c b/drivers/remoteproc/omap_remoteproc.c
> index fa63bf2..a96ce90 100644
> --- a/drivers/remoteproc/omap_remoteproc.c
> +++ b/drivers/remoteproc/omap_remoteproc.c
> @@ -177,7 +177,7 @@ static int omap_rproc_stop(struct rproc *rproc)
> return 0;
> }
>
> -static struct rproc_ops omap_rproc_ops = {
> +static const struct rproc_ops omap_rproc_ops = {
> .start = omap_rproc_start,
> .stop = omap_rproc_stop,
> .kick = omap_rproc_kick,
> diff --git a/drivers/remoteproc/st_remoteproc.c b/drivers/remoteproc/st_remoteproc.c
> index da4e152..f21787b 100644
> --- a/drivers/remoteproc/st_remoteproc.c
> +++ b/drivers/remoteproc/st_remoteproc.c
> @@ -107,7 +107,7 @@ static int st_rproc_stop(struct rproc *rproc)
> return sw_err ?: pwr_err;
> }
>
> -static struct rproc_ops st_rproc_ops = {
> +static const struct rproc_ops st_rproc_ops = {
> .start = st_rproc_start,
> .stop = st_rproc_stop,
> };
> diff --git a/drivers/remoteproc/st_slim_rproc.c b/drivers/remoteproc/st_slim_rproc.c
> index 507716c..6cfd862 100644
> --- a/drivers/remoteproc/st_slim_rproc.c
> +++ b/drivers/remoteproc/st_slim_rproc.c
> @@ -200,7 +200,7 @@ static void *slim_rproc_da_to_va(struct rproc *rproc, u64 da, int len)
> return va;
> }
>
> -static struct rproc_ops slim_rproc_ops = {
> +static const struct rproc_ops slim_rproc_ops = {
> .start = slim_rproc_start,
> .stop = slim_rproc_stop,
> .da_to_va = slim_rproc_da_to_va,
> diff --git a/drivers/remoteproc/wkup_m3_rproc.c b/drivers/remoteproc/wkup_m3_rproc.c
> index 18175d0..1ada0e5 100644
> --- a/drivers/remoteproc/wkup_m3_rproc.c
> +++ b/drivers/remoteproc/wkup_m3_rproc.c
> @@ -111,7 +111,7 @@ static void *wkup_m3_rproc_da_to_va(struct rproc *rproc, u64 da, int len)
> return va;
> }
>
> -static struct rproc_ops wkup_m3_rproc_ops = {
> +static const struct rproc_ops wkup_m3_rproc_ops = {
> .start = wkup_m3_rproc_start,
> .stop = wkup_m3_rproc_stop,
> .da_to_va = wkup_m3_rproc_da_to_va,
>

2016-12-30 20:06:42

by Bjorn Andersson

[permalink] [raw]
Subject: Re: [PATCH] drivers: remoteproc: constify rproc_ops structures

On Sat 17 Dec 03:29 PST 2016, Bhumika Goyal wrote:

> Declare rproc_ops structures as const as they are only passed as an
> argument to the function rproc_alloc. This argument is of type const, so
> rproc_ops structures having this property can be declared const too.
> Done using Coccinelle:
>
> @r1 disable optional_qualifier @
> identifier i;
> position p;
> @@
> static struct rproc_ops i@p = {...};
>
> @ok1@
> identifier r1.i;
> position p;
> @@
> rproc_alloc(...,&i@p,...)
>
> @bad@
> position p!={r1.p,ok1.p};
> identifier r1.i;
> @@
> i@p
>
> @depends on !bad disable optional_qualifier@
> identifier r1.i;
> @@
> +const
> struct rproc_ops i;
>
> File sizes before:
> text data bss dec hex filename
> 1258 416 0 1674 68a remoteproc/omap_remoteproc.o
> 2402 240 0 2642 a52 remoteproc/st_remoteproc.o
> 2064 272 0 2336 920 remoteproc/st_slim_rproc.o
> 2160 240 0 2400 960 remoteproc/wkup_m3_rproc.o
>
> File sizes after:
> text data bss dec hex filename
> 1297 368 0 1665 681 remoteproc/omap_remoteproc.o
> 2434 192 0 2626 a42 remoteproc/st_remoteproc.o
> 2112 240 0 2352 930 remoteproc/st_slim_rproc.o
> 2200 192 0 2392 958 remoteproc/wkup_m3_rproc.o
>
> Signed-off-by: Bhumika Goyal <[email protected]>

Thanks Bhumika, this looks good.

But as Suman already asked. Is there any reason why da8xx_remoteproc.c
did not get updated? It looks like the same change would apply there.


Unless I'm missing something, please update the patch and I'll be happy
to apply it.

Regards,
Bjorn

2016-12-31 10:43:49

by Bhumika Goyal

[permalink] [raw]
Subject: Re: [PATCH] drivers: remoteproc: constify rproc_ops structures

On Fri, Dec 30, 2016 at 5:36 PM, Bjorn Andersson
<[email protected]> wrote:
> On Sat 17 Dec 03:29 PST 2016, Bhumika Goyal wrote:
>
>> Declare rproc_ops structures as const as they are only passed as an
>> argument to the function rproc_alloc. This argument is of type const, so
>> rproc_ops structures having this property can be declared const too.
>> Done using Coccinelle:
>>
>> @r1 disable optional_qualifier @
>> identifier i;
>> position p;
>> @@
>> static struct rproc_ops i@p = {...};
>>
>> @ok1@
>> identifier r1.i;
>> position p;
>> @@
>> rproc_alloc(...,&i@p,...)
>>
>> @bad@
>> position p!={r1.p,ok1.p};
>> identifier r1.i;
>> @@
>> i@p
>>
>> @depends on !bad disable optional_qualifier@
>> identifier r1.i;
>> @@
>> +const
>> struct rproc_ops i;
>>
>> File sizes before:
>> text data bss dec hex filename
>> 1258 416 0 1674 68a remoteproc/omap_remoteproc.o
>> 2402 240 0 2642 a52 remoteproc/st_remoteproc.o
>> 2064 272 0 2336 920 remoteproc/st_slim_rproc.o
>> 2160 240 0 2400 960 remoteproc/wkup_m3_rproc.o
>>
>> File sizes after:
>> text data bss dec hex filename
>> 1297 368 0 1665 681 remoteproc/omap_remoteproc.o
>> 2434 192 0 2626 a42 remoteproc/st_remoteproc.o
>> 2112 240 0 2352 930 remoteproc/st_slim_rproc.o
>> 2200 192 0 2392 958 remoteproc/wkup_m3_rproc.o
>>
>> Signed-off-by: Bhumika Goyal <[email protected]>
>
> Thanks Bhumika, this looks good.
>
> But as Suman already asked. Is there any reason why da8xx_remoteproc.c
> did not get updated? It looks like the same change would apply there.
>
>

The reason I did not sent a patch for that driver is because the .o
file is not obtained. This is output I get when I try to compile the
da8xx_remoteproc.o file.

drivers/remoteproc/da8xx_remoteproc.c:22:72: fatal error:
mach/clock.h: No such file or directory
#include <mach/clock.h> /* for davinci_clk_reset_assert/deassert() */
^
compilation terminated.
make[1]: *** [drivers/remoteproc/da8xx_remoteproc.o] Error 1
make: *** [drivers/remoteproc/da8xx_remoteproc.o] Error 2

I also tried running the commands make.cross ARCH=arm allyesconfig;
make.cross ARCH=arm drivers/remoteproc/da8xx_remoteproc.o but still
the error remains the same.

Could you please suggest me what to do in this case?

Thanks,
Bhumika

> Unless I'm missing something, please update the patch and I'll be happy
> to apply it.
>
> Regards,
> Bjorn

2016-12-31 17:54:23

by Bjorn Andersson

[permalink] [raw]
Subject: Re: [PATCH] drivers: remoteproc: constify rproc_ops structures

On Sat 31 Dec 02:43 PST 2016, Bhumika Goyal wrote:

> On Fri, Dec 30, 2016 at 5:36 PM, Bjorn Andersson
> <[email protected]> wrote:
> > On Sat 17 Dec 03:29 PST 2016, Bhumika Goyal wrote:
> >
> >> Declare rproc_ops structures as const as they are only passed as an
> >> argument to the function rproc_alloc. This argument is of type const, so
> >> rproc_ops structures having this property can be declared const too.
> >> Done using Coccinelle:
> >>
> >> @r1 disable optional_qualifier @
> >> identifier i;
> >> position p;
> >> @@
> >> static struct rproc_ops i@p = {...};
> >>
> >> @ok1@
> >> identifier r1.i;
> >> position p;
> >> @@
> >> rproc_alloc(...,&i@p,...)
> >>
> >> @bad@
> >> position p!={r1.p,ok1.p};
> >> identifier r1.i;
> >> @@
> >> i@p
> >>
> >> @depends on !bad disable optional_qualifier@
> >> identifier r1.i;
> >> @@
> >> +const
> >> struct rproc_ops i;
> >>
> >> File sizes before:
> >> text data bss dec hex filename
> >> 1258 416 0 1674 68a remoteproc/omap_remoteproc.o
> >> 2402 240 0 2642 a52 remoteproc/st_remoteproc.o
> >> 2064 272 0 2336 920 remoteproc/st_slim_rproc.o
> >> 2160 240 0 2400 960 remoteproc/wkup_m3_rproc.o
> >>
> >> File sizes after:
> >> text data bss dec hex filename
> >> 1297 368 0 1665 681 remoteproc/omap_remoteproc.o
> >> 2434 192 0 2626 a42 remoteproc/st_remoteproc.o
> >> 2112 240 0 2352 930 remoteproc/st_slim_rproc.o
> >> 2200 192 0 2392 958 remoteproc/wkup_m3_rproc.o
> >>
> >> Signed-off-by: Bhumika Goyal <[email protected]>
> >
> > Thanks Bhumika, this looks good.
> >
> > But as Suman already asked. Is there any reason why da8xx_remoteproc.c
> > did not get updated? It looks like the same change would apply there.
> >
> >
>
> The reason I did not sent a patch for that driver is because the .o
> file is not obtained. This is output I get when I try to compile the
> da8xx_remoteproc.o file.
>
> drivers/remoteproc/da8xx_remoteproc.c:22:72: fatal error:
> mach/clock.h: No such file or directory
> #include <mach/clock.h> /* for davinci_clk_reset_assert/deassert() */
> ^
> compilation terminated.
> make[1]: *** [drivers/remoteproc/da8xx_remoteproc.o] Error 1
> make: *** [drivers/remoteproc/da8xx_remoteproc.o] Error 2
>
> I also tried running the commands make.cross ARCH=arm allyesconfig;
> make.cross ARCH=arm drivers/remoteproc/da8xx_remoteproc.o but still
> the error remains the same.
>
> Could you please suggest me what to do in this case?
>

By using davinci_all_defconfig and then selecting
CONFIG_DA8XX_REMOTEPROC I managed to compile it, not sure what differs.

If you can't get it working please include the da8xx_remoteproc change
as well and just note that you didn't manage to test it.

Regards,
Bjorn