Skip to contentSkip to conversation

pinctrl: qcom: msm: constify gpio_chip structure

PATCH In review Gustavo A. R. Silva 3 participants 4 messages Jul 11, 2017 – Aug 1, 2017

Subsystems (5)
Arm Qualcomm mailing list GPIO Pin control Pin controller - Qualcomm +1 more subsystems
lore
Share & tools
lore

Gustavo A. R. Silva

author
#0 lore raw

PATCH 1/1 pinctrl: qcom: msm: constify gpio_chip structure

This structure is only used to copy into other structure, so declare it as const.

This issue was detected using Coccinelle and the following semantic patch:

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

@ok@
identifier r.i;
expression e;
position p;
@@
e = i@p;

@bad@
position p != {r.p,ok.p};
identifier r.i;
struct gpio_chip e;
@@
e@i@p

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

In the following log you can see a significant difference in the code size and data segment, hence in the dec segment. This log is the output of the size command, before and after the code change:

before:

   text    data     bss     dec     hex filename
  13129    2808     192   16129    3f01 drivers/pinctrl/qcom/pinctrl-msm.o

after:

   text    data     bss     dec     hex filename
  12839    2720     192   15751    3d87 drivers/pinctrl/qcom/pinctrl-msm.o
Signed-off-by Gustavo A. R. Silva <[email protected]>
---
 drivers/pinctrl/qcom/pinctrl-msm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/pinctrl/qcom/pinctrl-msm.c +1 -1
index 273badd..245ccfd 100644
@@ -521,7 +521,7 @@ static void msm_gpio_dbg_show(struct seq_file *s, struct gpio_chip *chip)
 #define msm_gpio_dbg_show NULL
 #endif
 
-static struct gpio_chip msm_gpio_template = {
+static const struct gpio_chip msm_gpio_template = {
 	.direction_input  = msm_gpio_direction_input,
 	.direction_output = msm_gpio_direction_output,
 	.get_direction    = msm_gpio_get_direction,
Signature · 1 line
2.5.0
Reviewed-by on v1 1/1
#1 lore raw

On Tue 11 Jul 11:34 PDT 2017, Gustavo A. R. Silva wrote:
[..]

Quoted 2 lines

In the following log you can see a significant difference in the code size and data segment, hence in the dec segment. This log is the output

"dec" is for "decimal" and is the sum of text, data and bss...

Quoted 11 lines

of the size command, before and after the code change:

before:

   text    data     bss     dec     hex filename
  13129    2808     192   16129    3f01 drivers/pinctrl/qcom/pinctrl-msm.o

after:

   text    data     bss     dec     hex filename
  12839    2720     192   15751    3d87 drivers/pinctrl/qcom/pinctrl-msm.o
Signed-off-by Gustavo A. R. Silva <[email protected]>

Good catch, thanks!

Reviewed-by Bjorn Andersson <[email protected]>

Regards,
Bjorn

Quoted 20 lines
---
 drivers/pinctrl/qcom/pinctrl-msm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/pinctrl/qcom/pinctrl-msm.c +1 -1
index 273badd..245ccfd 100644
@@ -521,7 +521,7 @@ static void msm_gpio_dbg_show(struct seq_file *s, struct gpio_chip *chip)
 #define msm_gpio_dbg_show NULL
 #endif
 
-static struct gpio_chip msm_gpio_template = {
+static const struct gpio_chip msm_gpio_template = {
 	.direction_input  = msm_gpio_direction_input,
 	.direction_output = msm_gpio_direction_output,
 	.get_direction    = msm_gpio_get_direction,
Signature · 1 line
2.5.0
author
#2 lore raw

Hi Bjorn,

Quoting Bjorn Andersson <[email protected]>:

Quoted 7 lines · Gustavo A. R. Silva

[..]

Quoted 2 lines

In the following log you can see a significant difference in the code size and data segment, hence in the dec segment. This log is the output

"dec" is for "decimal" and is the sum of text, data and bss...

You are right. :)

Quoted 6 lines
Quoted 6 lines

of the size command, before and after the code change:

before:

   text    data     bss     dec     hex filename
  13129    2808     192   16129    3f01 drivers/pinctrl/qcom/pinctrl-msm.o
Quoted 8 lines
Quoted 5 lines

after:

   text    data     bss     dec     hex filename
  12839    2720     192   15751    3d87 drivers/pinctrl/qcom/pinctrl-msm.o
Signed-off-by Gustavo A. R. Silva <[email protected]>

Good catch, thanks!

Absolutely, glad to help!

Quoted 18 lines
Reviewed-by Bjorn Andersson <[email protected]>

Regards,
Bjorn

Quoted 13 lines
---
 drivers/pinctrl/qcom/pinctrl-msm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/pinctrl/qcom/pinctrl-msm.c +0 -0
index 273badd..245ccfd 100644
@@ -521,7 +521,7 @@ static void msm_gpio_dbg_show(struct seq_file  
 *s, struct gpio_chip *chip)
 #define msm_gpio_dbg_show NULL
 #endif
Quoted 8 lines
Quoted 8 lines
drivers/pinctrl/qcom/pinctrl-msm.c +1 -1
-static struct gpio_chip msm_gpio_template = {
+static const struct gpio_chip msm_gpio_template = {
 	.direction_input  = msm_gpio_direction_input,
 	.direction_output = msm_gpio_direction_output,

.get_direction = msm_gpio_get_direction,
--
2.5.0

--
Gustavo A. R. Silva

#3 lore raw
In reply to Gustavo A. R. Silva
Quoted 37 lines · Gustavo A. R. Silva

This structure is only used to copy into other structure, so declare it as const.

This issue was detected using Coccinelle and the following semantic patch:

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

@ok@
identifier r.i;
expression e;
position p;
@@
e = i@p;

@bad@
position p != {r.p,ok.p};
identifier r.i;
struct gpio_chip e;
@@
e@i@p

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

In the following log you can see a significant difference in the code size and data segment, hence in the dec segment. This log is the output of the size command, before and after the code change:

before:

Quoted 8 lines
   text    data     bss     dec     hex filename
  13129    2808     192   16129    3f01 drivers/pinctrl/qcom/pinctrl-msm.o

after:

   text    data     bss     dec     hex filename
  12839    2720     192   15751    3d87 drivers/pinctrl/qcom/pinctrl-msm.o
Signed-off-by Gustavo A. R. Silva <[email protected]>

Patch applied with Björn's ACK.

Yours,
Linus Walleij