diff mbox

drivers: remoteproc: constify rproc_ops structures

Message ID 1481974176-19912-1-git-send-email-bhumirks@gmail.com (mailing list archive)
State Accepted
Headers show

Commit Message

Bhumika Goyal Dec. 17, 2016, 11:29 a.m. UTC
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 <bhumirks@gmail.com>
---
 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(-)

Comments

Suman Anna Dec. 22, 2016, 9:22 p.m. UTC | #1
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 <bhumirks@gmail.com>

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

--
To unsubscribe from this list: send the line "unsubscribe linux-remoteproc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Bjorn Andersson Dec. 30, 2016, 12:06 p.m. UTC | #2
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 <bhumirks@gmail.com>

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
--
To unsubscribe from this list: send the line "unsubscribe linux-remoteproc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Bhumika Goyal Dec. 31, 2016, 10:43 a.m. UTC | #3
On Fri, Dec 30, 2016 at 5:36 PM, Bjorn Andersson
<bjorn.andersson@linaro.org> 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 <bhumirks@gmail.com>
>
> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-remoteproc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Bjorn Andersson Dec. 31, 2016, 5:54 p.m. UTC | #4
On Sat 31 Dec 02:43 PST 2016, Bhumika Goyal wrote:

> On Fri, Dec 30, 2016 at 5:36 PM, Bjorn Andersson
> <bjorn.andersson@linaro.org> 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 <bhumirks@gmail.com>
> >
> > 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
--
To unsubscribe from this list: send the line "unsubscribe linux-remoteproc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Bhumika Goyal Jan. 1, 2017, 6:23 a.m. UTC | #5
On Sat, Dec 31, 2016 at 11:24 PM, Bjorn Andersson
<bjorn.andersson@linaro.org> wrote:
> On Sat 31 Dec 02:43 PST 2016, Bhumika Goyal wrote:
>
>> On Fri, Dec 30, 2016 at 5:36 PM, Bjorn Andersson
>> <bjorn.andersson@linaro.org> 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 <bhumirks@gmail.com>
>> >
>> > 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.
>

Yes, it compiled. Thanks for the input. Now I will include this driver
as well and send a v2.

Thanks,
Bhumika

> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-remoteproc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

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,