linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] w1: Constify w1_family_ops
@ 2020-10-04 19:31 Rikard Falkeborn
  2020-10-04 19:32 ` [PATCH 1/3] w1: Constify struct w1_family_ops Rikard Falkeborn
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Rikard Falkeborn @ 2020-10-04 19:31 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: linux-kernel, Rikard Falkeborn, Sebastian Reichel,
	Angelo Dureghello, Akira Shimahara, Evgeniy Polyakov, linux-pm

None of the current instances of struct w1_family_ops in the kernel is
modified. Constify these to let the compiler put them in read-only memory.

The first patch changes the fops field in w1_family struct to a pointer to
const and makes a local variable a pointer to const to avoid a compiler
warning. This patch is a prerequisite for the second and third patches
which constifies the static structs in drivers in w1 and power. These
changes was done with coccinelle (details in the commit messages).

With these changes applied, all instances of struct w1_family_ops in the
kernel are const.

Build-tested on x86 allmodconfig.

Rikard Falkeborn (3):
  w1: Constify struct w1_family_ops
  w1: Constify static w1_family_ops structs
  power: supply: Constify static w1_family_ops structs

 drivers/power/supply/bq27xxx_battery_hdq.c | 2 +-
 drivers/power/supply/ds2760_battery.c      | 2 +-
 drivers/power/supply/max1721x_battery.c    | 2 +-
 drivers/w1/slaves/w1_ds2405.c              | 2 +-
 drivers/w1/slaves/w1_ds2406.c              | 2 +-
 drivers/w1/slaves/w1_ds2408.c              | 2 +-
 drivers/w1/slaves/w1_ds2413.c              | 2 +-
 drivers/w1/slaves/w1_ds2423.c              | 2 +-
 drivers/w1/slaves/w1_ds2430.c              | 2 +-
 drivers/w1/slaves/w1_ds2431.c              | 2 +-
 drivers/w1/slaves/w1_ds2433.c              | 2 +-
 drivers/w1/slaves/w1_ds2438.c              | 2 +-
 drivers/w1/slaves/w1_ds250x.c              | 2 +-
 drivers/w1/slaves/w1_ds2780.c              | 2 +-
 drivers/w1/slaves/w1_ds2781.c              | 2 +-
 drivers/w1/slaves/w1_ds2805.c              | 2 +-
 drivers/w1/slaves/w1_ds28e04.c             | 2 +-
 drivers/w1/slaves/w1_ds28e17.c             | 2 +-
 drivers/w1/slaves/w1_therm.c               | 6 +++---
 drivers/w1/w1.c                            | 4 ++--
 include/linux/w1.h                         | 2 +-
 21 files changed, 24 insertions(+), 24 deletions(-)

-- 
2.28.0


^ permalink raw reply	[flat|nested] 6+ messages in thread
* [PATCH 1/3] w1: Constify struct w1_family_ops
  2020-10-04 19:31 [PATCH 0/3] w1: Constify w1_family_ops Rikard Falkeborn
@ 2020-10-04 19:32 ` Rikard Falkeborn
  2020-10-04 21:55   ` Sebastian Reichel
  2020-10-04 19:32 ` [PATCH 2/3] w1: Constify static w1_family_ops structs Rikard Falkeborn
  2020-10-04 19:32 ` [PATCH 3/3] power: supply: " Rikard Falkeborn
  2 siblings, 1 reply; 6+ messages in thread
From: Rikard Falkeborn @ 2020-10-04 19:32 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: linux-kernel, Rikard Falkeborn, Evgeniy Polyakov,
	Angelo Dureghello, Akira Shimahara, Sebastian Reichel

The fops field in the w1_family struct is never modified. Make it const
to indicate that. Constifying the pointer makes it possible for drivers
to declare static w1_family_ops structs const, which in turn will allow
the compiler to put it in read-only memory.

Signed-off-by: Rikard Falkeborn <rikard.falkeborn@gmail.com>
---
 drivers/w1/w1.c    | 2 +-
 include/linux/w1.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/w1/w1.c b/drivers/w1/w1.c
index e58c7592008d..6bd64bcb6316 100644
--- a/drivers/w1/w1.c
+++ b/drivers/w1/w1.c
@@ -613,7 +613,7 @@ static int w1_uevent(struct device *dev, struct kobj_uevent_env *env)
 
 static int w1_family_notify(unsigned long action, struct w1_slave *sl)
 {
-	struct w1_family_ops *fops;
+	const struct w1_family_ops *fops;
 	int err;
 
 	fops = sl->family->fops;
diff --git a/include/linux/w1.h b/include/linux/w1.h
index cebf3464bc03..949d3b10e531 100644
--- a/include/linux/w1.h
+++ b/include/linux/w1.h
@@ -269,7 +269,7 @@ struct w1_family {
 	struct list_head	family_entry;
 	u8			fid;
 
-	struct w1_family_ops	*fops;
+	const struct w1_family_ops *fops;
 
 	const struct of_device_id *of_match_table;
 
-- 
2.28.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [PATCH 2/3] w1: Constify static w1_family_ops structs
  2020-10-04 19:31 [PATCH 0/3] w1: Constify w1_family_ops Rikard Falkeborn
  2020-10-04 19:32 ` [PATCH 1/3] w1: Constify struct w1_family_ops Rikard Falkeborn
@ 2020-10-04 19:32 ` Rikard Falkeborn
  2020-10-04 19:32 ` [PATCH 3/3] power: supply: " Rikard Falkeborn
  2 siblings, 0 replies; 6+ messages in thread
From: Rikard Falkeborn @ 2020-10-04 19:32 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: linux-kernel, Rikard Falkeborn, Angelo Dureghello,
	Akira Shimahara, Evgeniy Polyakov

The only usage of these structs is to assign their address to the fops
field in the w1_family struct, which is a const pointer. Make them const
to allow the compiler to put them in read-only memory.

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

// <smpl>
@r1 disable optional_qualifier @
identifier i;
position p;
@@
static struct w1_family_ops i@p = {...};

@ok1@
identifier r1.i;
position p;
identifier s;
@@
static struct w1_family s = {
	.fops=&i@p,
};

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

@depends on !bad1 disable optional_qualifier@
identifier r1.i;
@@
static
+const
struct w1_family_ops i={};
// </smpl>

Signed-off-by: Rikard Falkeborn <rikard.falkeborn@gmail.com>
---
 drivers/w1/slaves/w1_ds2405.c  | 2 +-
 drivers/w1/slaves/w1_ds2406.c  | 2 +-
 drivers/w1/slaves/w1_ds2408.c  | 2 +-
 drivers/w1/slaves/w1_ds2413.c  | 2 +-
 drivers/w1/slaves/w1_ds2423.c  | 2 +-
 drivers/w1/slaves/w1_ds2430.c  | 2 +-
 drivers/w1/slaves/w1_ds2431.c  | 2 +-
 drivers/w1/slaves/w1_ds2433.c  | 2 +-
 drivers/w1/slaves/w1_ds2438.c  | 2 +-
 drivers/w1/slaves/w1_ds250x.c  | 2 +-
 drivers/w1/slaves/w1_ds2780.c  | 2 +-
 drivers/w1/slaves/w1_ds2781.c  | 2 +-
 drivers/w1/slaves/w1_ds2805.c  | 2 +-
 drivers/w1/slaves/w1_ds28e04.c | 2 +-
 drivers/w1/slaves/w1_ds28e17.c | 2 +-
 drivers/w1/slaves/w1_therm.c   | 6 +++---
 drivers/w1/w1.c                | 2 +-
 17 files changed, 19 insertions(+), 19 deletions(-)

diff --git a/drivers/w1/slaves/w1_ds2405.c b/drivers/w1/slaves/w1_ds2405.c
index 86cd97309d87..1d9a1183e83f 100644
--- a/drivers/w1/slaves/w1_ds2405.c
+++ b/drivers/w1/slaves/w1_ds2405.c
@@ -206,7 +206,7 @@ static struct attribute *w1_ds2405_attrs[] = {
 
 ATTRIBUTE_GROUPS(w1_ds2405);
 
-static struct w1_family_ops w1_ds2405_fops = {
+static const struct w1_family_ops w1_ds2405_fops = {
 	.groups = w1_ds2405_groups
 };
 
diff --git a/drivers/w1/slaves/w1_ds2406.c b/drivers/w1/slaves/w1_ds2406.c
index 762e5e4e2b48..6c269af73c80 100644
--- a/drivers/w1/slaves/w1_ds2406.c
+++ b/drivers/w1/slaves/w1_ds2406.c
@@ -138,7 +138,7 @@ static void w1_f12_remove_slave(struct w1_slave *sl)
 			&(w1_f12_sysfs_bin_files[i]));
 }
 
-static struct w1_family_ops w1_f12_fops = {
+static const struct w1_family_ops w1_f12_fops = {
 	.add_slave      = w1_f12_add_slave,
 	.remove_slave   = w1_f12_remove_slave,
 };
diff --git a/drivers/w1/slaves/w1_ds2408.c b/drivers/w1/slaves/w1_ds2408.c
index 83f8d94bb814..ad102c577122 100644
--- a/drivers/w1/slaves/w1_ds2408.c
+++ b/drivers/w1/slaves/w1_ds2408.c
@@ -336,7 +336,7 @@ static const struct attribute_group *w1_f29_groups[] = {
 	NULL,
 };
 
-static struct w1_family_ops w1_f29_fops = {
+static const struct w1_family_ops w1_f29_fops = {
 	.add_slave      = w1_f29_disable_test_mode,
 	.groups		= w1_f29_groups,
 };
diff --git a/drivers/w1/slaves/w1_ds2413.c b/drivers/w1/slaves/w1_ds2413.c
index f1fb18afbcea..c8cfac555b48 100644
--- a/drivers/w1/slaves/w1_ds2413.c
+++ b/drivers/w1/slaves/w1_ds2413.c
@@ -143,7 +143,7 @@ static const struct attribute_group *w1_f3a_groups[] = {
 	NULL,
 };
 
-static struct w1_family_ops w1_f3a_fops = {
+static const struct w1_family_ops w1_f3a_fops = {
 	.groups		= w1_f3a_groups,
 };
 
diff --git a/drivers/w1/slaves/w1_ds2423.c b/drivers/w1/slaves/w1_ds2423.c
index f4367282dcc1..b6bd18d5b3f6 100644
--- a/drivers/w1/slaves/w1_ds2423.c
+++ b/drivers/w1/slaves/w1_ds2423.c
@@ -117,7 +117,7 @@ static struct attribute *w1_f1d_attrs[] = {
 };
 ATTRIBUTE_GROUPS(w1_f1d);
 
-static struct w1_family_ops w1_f1d_fops = {
+static const struct w1_family_ops w1_f1d_fops = {
 	.groups		= w1_f1d_groups,
 };
 
diff --git a/drivers/w1/slaves/w1_ds2430.c b/drivers/w1/slaves/w1_ds2430.c
index 75bb8a88620b..0ea7d779d17a 100644
--- a/drivers/w1/slaves/w1_ds2430.c
+++ b/drivers/w1/slaves/w1_ds2430.c
@@ -279,7 +279,7 @@ static const struct attribute_group *w1_f14_groups[] = {
 	NULL,
 };
 
-static struct w1_family_ops w1_f14_fops = {
+static const struct w1_family_ops w1_f14_fops = {
 	.groups	= w1_f14_groups,
 };
 
diff --git a/drivers/w1/slaves/w1_ds2431.c b/drivers/w1/slaves/w1_ds2431.c
index e5bd7e2354d7..6856b1c29e17 100644
--- a/drivers/w1/slaves/w1_ds2431.c
+++ b/drivers/w1/slaves/w1_ds2431.c
@@ -278,7 +278,7 @@ static const struct attribute_group *w1_f2d_groups[] = {
 	NULL,
 };
 
-static struct w1_family_ops w1_f2d_fops = {
+static const struct w1_family_ops w1_f2d_fops = {
 	.groups		= w1_f2d_groups,
 };
 
diff --git a/drivers/w1/slaves/w1_ds2433.c b/drivers/w1/slaves/w1_ds2433.c
index 1f805c86517a..0f72df15a024 100644
--- a/drivers/w1/slaves/w1_ds2433.c
+++ b/drivers/w1/slaves/w1_ds2433.c
@@ -288,7 +288,7 @@ static void w1_f23_remove_slave(struct w1_slave *sl)
 #endif	/* CONFIG_W1_SLAVE_DS2433_CRC */
 }
 
-static struct w1_family_ops w1_f23_fops = {
+static const struct w1_family_ops w1_f23_fops = {
 	.add_slave      = w1_f23_add_slave,
 	.remove_slave   = w1_f23_remove_slave,
 	.groups		= w1_f23_groups,
diff --git a/drivers/w1/slaves/w1_ds2438.c b/drivers/w1/slaves/w1_ds2438.c
index d199e5a25cc0..5cfb0ae23e91 100644
--- a/drivers/w1/slaves/w1_ds2438.c
+++ b/drivers/w1/slaves/w1_ds2438.c
@@ -412,7 +412,7 @@ static const struct attribute_group *w1_ds2438_groups[] = {
 	NULL,
 };
 
-static struct w1_family_ops w1_ds2438_fops = {
+static const struct w1_family_ops w1_ds2438_fops = {
 	.groups		= w1_ds2438_groups,
 };
 
diff --git a/drivers/w1/slaves/w1_ds250x.c b/drivers/w1/slaves/w1_ds250x.c
index e507117444d8..7592c7050d1d 100644
--- a/drivers/w1/slaves/w1_ds250x.c
+++ b/drivers/w1/slaves/w1_ds250x.c
@@ -215,7 +215,7 @@ static int w1_eprom_add_slave(struct w1_slave *sl)
 	return PTR_ERR_OR_ZERO(nvmem);
 }
 
-static struct w1_family_ops w1_eprom_fops = {
+static const struct w1_family_ops w1_eprom_fops = {
 	.add_slave	= w1_eprom_add_slave,
 };
 
diff --git a/drivers/w1/slaves/w1_ds2780.c b/drivers/w1/slaves/w1_ds2780.c
index c689b1b987b8..c281fe5ed688 100644
--- a/drivers/w1/slaves/w1_ds2780.c
+++ b/drivers/w1/slaves/w1_ds2780.c
@@ -141,7 +141,7 @@ static void w1_ds2780_remove_slave(struct w1_slave *sl)
 	platform_device_unregister(pdev);
 }
 
-static struct w1_family_ops w1_ds2780_fops = {
+static const struct w1_family_ops w1_ds2780_fops = {
 	.add_slave    = w1_ds2780_add_slave,
 	.remove_slave = w1_ds2780_remove_slave,
 	.groups       = w1_ds2780_groups,
diff --git a/drivers/w1/slaves/w1_ds2781.c b/drivers/w1/slaves/w1_ds2781.c
index 84d6ceec5da5..f0d393ae070b 100644
--- a/drivers/w1/slaves/w1_ds2781.c
+++ b/drivers/w1/slaves/w1_ds2781.c
@@ -138,7 +138,7 @@ static void w1_ds2781_remove_slave(struct w1_slave *sl)
 	platform_device_unregister(pdev);
 }
 
-static struct w1_family_ops w1_ds2781_fops = {
+static const struct w1_family_ops w1_ds2781_fops = {
 	.add_slave    = w1_ds2781_add_slave,
 	.remove_slave = w1_ds2781_remove_slave,
 	.groups       = w1_ds2781_groups,
diff --git a/drivers/w1/slaves/w1_ds2805.c b/drivers/w1/slaves/w1_ds2805.c
index ccb753a474b1..206186db727d 100644
--- a/drivers/w1/slaves/w1_ds2805.c
+++ b/drivers/w1/slaves/w1_ds2805.c
@@ -281,7 +281,7 @@ static void w1_f0d_remove_slave(struct w1_slave *sl)
 	sysfs_remove_bin_file(&sl->dev.kobj, &w1_f0d_bin_attr);
 }
 
-static struct w1_family_ops w1_f0d_fops = {
+static const struct w1_family_ops w1_f0d_fops = {
 	.add_slave      = w1_f0d_add_slave,
 	.remove_slave   = w1_f0d_remove_slave,
 };
diff --git a/drivers/w1/slaves/w1_ds28e04.c b/drivers/w1/slaves/w1_ds28e04.c
index 8a640f159078..e4f336111edc 100644
--- a/drivers/w1/slaves/w1_ds28e04.c
+++ b/drivers/w1/slaves/w1_ds28e04.c
@@ -410,7 +410,7 @@ static void w1_f1C_remove_slave(struct w1_slave *sl)
 	sl->family_data = NULL;
 }
 
-static struct w1_family_ops w1_f1C_fops = {
+static const struct w1_family_ops w1_f1C_fops = {
 	.add_slave      = w1_f1C_add_slave,
 	.remove_slave   = w1_f1C_remove_slave,
 	.groups		= w1_f1C_groups,
diff --git a/drivers/w1/slaves/w1_ds28e17.c b/drivers/w1/slaves/w1_ds28e17.c
index 046ddda83df9..6b00db7169ab 100644
--- a/drivers/w1/slaves/w1_ds28e17.c
+++ b/drivers/w1/slaves/w1_ds28e17.c
@@ -741,7 +741,7 @@ static void w1_f19_remove_slave(struct w1_slave *sl)
 
 
 /* Declarations within the w1 subsystem. */
-static struct w1_family_ops w1_f19_fops = {
+static const struct w1_family_ops w1_f19_fops = {
 	.add_slave = w1_f19_add_slave,
 	.remove_slave = w1_f19_remove_slave,
 	.groups = w1_f19_groups,
diff --git a/drivers/w1/slaves/w1_therm.c b/drivers/w1/slaves/w1_therm.c
index c1b4eda16719..54f84f2d5064 100644
--- a/drivers/w1/slaves/w1_therm.c
+++ b/drivers/w1/slaves/w1_therm.c
@@ -409,21 +409,21 @@ static const struct hwmon_chip_info w1_chip_info = {
 
 /* Family operations */
 
-static struct w1_family_ops w1_therm_fops = {
+static const struct w1_family_ops w1_therm_fops = {
 	.add_slave	= w1_therm_add_slave,
 	.remove_slave	= w1_therm_remove_slave,
 	.groups		= w1_therm_groups,
 	.chip_info	= W1_CHIPINFO,
 };
 
-static struct w1_family_ops w1_ds18s20_fops = {
+static const struct w1_family_ops w1_ds18s20_fops = {
 	.add_slave	= w1_therm_add_slave,
 	.remove_slave	= w1_therm_remove_slave,
 	.groups		= w1_ds18s20_groups,
 	.chip_info	= W1_CHIPINFO,
 };
 
-static struct w1_family_ops w1_ds28ea00_fops = {
+static const struct w1_family_ops w1_ds28ea00_fops = {
 	.add_slave	= w1_therm_add_slave,
 	.remove_slave	= w1_therm_remove_slave,
 	.groups		= w1_ds28ea00_groups,
diff --git a/drivers/w1/w1.c b/drivers/w1/w1.c
index 6bd64bcb6316..15a2ee32f116 100644
--- a/drivers/w1/w1.c
+++ b/drivers/w1/w1.c
@@ -160,7 +160,7 @@ static const struct attribute_group *w1_slave_default_groups[] = {
 	NULL,
 };
 
-static struct w1_family_ops w1_default_fops = {
+static const struct w1_family_ops w1_default_fops = {
 	.groups		= w1_slave_default_groups,
 };
 
-- 
2.28.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [PATCH 3/3] power: supply: Constify static w1_family_ops structs
  2020-10-04 19:31 [PATCH 0/3] w1: Constify w1_family_ops Rikard Falkeborn
  2020-10-04 19:32 ` [PATCH 1/3] w1: Constify struct w1_family_ops Rikard Falkeborn
  2020-10-04 19:32 ` [PATCH 2/3] w1: Constify static w1_family_ops structs Rikard Falkeborn
@ 2020-10-04 19:32 ` Rikard Falkeborn
  2020-10-04 22:03   ` Sebastian Reichel
  2 siblings, 1 reply; 6+ messages in thread
From: Rikard Falkeborn @ 2020-10-04 19:32 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: linux-kernel, Rikard Falkeborn, Sebastian Reichel, linux-pm

The only usage of these structs is to assign their address to the fops
field in the w1_family struct, which is a const pointer. Make them const
to allow the compiler to put them in read-only memory.

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

// <smpl>
@r1 disable optional_qualifier @
identifier i;
position p;
@@
static struct w1_family_ops i@p = {...};

@ok1@
identifier r1.i;
position p;
identifier s;
@@
static struct w1_family s = {
	.fops=&i@p,
};

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

@depends on !bad1 disable optional_qualifier@
identifier r1.i;
@@
static
+const
struct w1_family_ops i={};
// </smpl>

Signed-off-by: Rikard Falkeborn <rikard.falkeborn@gmail.com>
---
 drivers/power/supply/bq27xxx_battery_hdq.c | 2 +-
 drivers/power/supply/ds2760_battery.c      | 2 +-
 drivers/power/supply/max1721x_battery.c    | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/power/supply/bq27xxx_battery_hdq.c b/drivers/power/supply/bq27xxx_battery_hdq.c
index 12b10dad77d3..922759ab2e04 100644
--- a/drivers/power/supply/bq27xxx_battery_hdq.c
+++ b/drivers/power/supply/bq27xxx_battery_hdq.c
@@ -97,7 +97,7 @@ static void bq27xxx_battery_hdq_remove_slave(struct w1_slave *sl)
 	bq27xxx_battery_teardown(di);
 }
 
-static struct w1_family_ops bq27xxx_battery_hdq_fops = {
+static const struct w1_family_ops bq27xxx_battery_hdq_fops = {
 	.add_slave	= bq27xxx_battery_hdq_add_slave,
 	.remove_slave	= bq27xxx_battery_hdq_remove_slave,
 };
diff --git a/drivers/power/supply/ds2760_battery.c b/drivers/power/supply/ds2760_battery.c
index 11bed88a89fa..695bb6747400 100644
--- a/drivers/power/supply/ds2760_battery.c
+++ b/drivers/power/supply/ds2760_battery.c
@@ -795,7 +795,7 @@ static const struct of_device_id w1_ds2760_of_ids[] = {
 };
 #endif
 
-static struct w1_family_ops w1_ds2760_fops = {
+static const struct w1_family_ops w1_ds2760_fops = {
 	.add_slave	= w1_ds2760_add_slave,
 	.remove_slave	= w1_ds2760_remove_slave,
 	.groups		= w1_ds2760_groups,
diff --git a/drivers/power/supply/max1721x_battery.c b/drivers/power/supply/max1721x_battery.c
index 9ca895b0dabb..1b1a36f8e929 100644
--- a/drivers/power/supply/max1721x_battery.c
+++ b/drivers/power/supply/max1721x_battery.c
@@ -431,7 +431,7 @@ static int devm_w1_max1721x_add_device(struct w1_slave *sl)
 	return 0;
 }
 
-static struct w1_family_ops w1_max1721x_fops = {
+static const struct w1_family_ops w1_max1721x_fops = {
 	.add_slave = devm_w1_max1721x_add_device,
 };
 
-- 
2.28.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [PATCH 1/3] w1: Constify struct w1_family_ops
  2020-10-04 19:32 ` [PATCH 1/3] w1: Constify struct w1_family_ops Rikard Falkeborn
@ 2020-10-04 21:55   ` Sebastian Reichel
  0 siblings, 0 replies; 6+ messages in thread
From: Sebastian Reichel @ 2020-10-04 21:55 UTC (permalink / raw)
  To: Rikard Falkeborn
  Cc: Greg Kroah-Hartman, linux-kernel, Evgeniy Polyakov,
	Angelo Dureghello, Akira Shimahara

[-- Attachment #1: Type: text/plain, Size: 1471 bytes --]

Hi,

On Sun, Oct 04, 2020 at 09:32:00PM +0200, Rikard Falkeborn wrote:
> The fops field in the w1_family struct is never modified. Make it const
> to indicate that. Constifying the pointer makes it possible for drivers
> to declare static w1_family_ops structs const, which in turn will allow
> the compiler to put it in read-only memory.
> 
> Signed-off-by: Rikard Falkeborn <rikard.falkeborn@gmail.com>
> ---

Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.com>

-- Sebastian

>  drivers/w1/w1.c    | 2 +-
>  include/linux/w1.h | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/w1/w1.c b/drivers/w1/w1.c
> index e58c7592008d..6bd64bcb6316 100644
> --- a/drivers/w1/w1.c
> +++ b/drivers/w1/w1.c
> @@ -613,7 +613,7 @@ static int w1_uevent(struct device *dev, struct kobj_uevent_env *env)
>  
>  static int w1_family_notify(unsigned long action, struct w1_slave *sl)
>  {
> -	struct w1_family_ops *fops;
> +	const struct w1_family_ops *fops;
>  	int err;
>  
>  	fops = sl->family->fops;
> diff --git a/include/linux/w1.h b/include/linux/w1.h
> index cebf3464bc03..949d3b10e531 100644
> --- a/include/linux/w1.h
> +++ b/include/linux/w1.h
> @@ -269,7 +269,7 @@ struct w1_family {
>  	struct list_head	family_entry;
>  	u8			fid;
>  
> -	struct w1_family_ops	*fops;
> +	const struct w1_family_ops *fops;
>  
>  	const struct of_device_id *of_match_table;
>  
> -- 
> 2.28.0
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH 3/3] power: supply: Constify static w1_family_ops structs
  2020-10-04 19:32 ` [PATCH 3/3] power: supply: " Rikard Falkeborn
@ 2020-10-04 22:03   ` Sebastian Reichel
  0 siblings, 0 replies; 6+ messages in thread
From: Sebastian Reichel @ 2020-10-04 22:03 UTC (permalink / raw)
  To: Rikard Falkeborn; +Cc: Greg Kroah-Hartman, linux-kernel, linux-pm

[-- Attachment #1: Type: text/plain, Size: 3032 bytes --]

Hi,

On Sun, Oct 04, 2020 at 09:32:02PM +0200, Rikard Falkeborn wrote:
> The only usage of these structs is to assign their address to the fops
> field in the w1_family struct, which is a const pointer. Make them const
> to allow the compiler to put them in read-only memory.
> 
> This was done with the following Coccinelle semantic patch
> (http://coccinelle.lip6.fr/):
> 
> // <smpl>
> @r1 disable optional_qualifier @
> identifier i;
> position p;
> @@
> static struct w1_family_ops i@p = {...};
> 
> @ok1@
> identifier r1.i;
> position p;
> identifier s;
> @@
> static struct w1_family s = {
> 	.fops=&i@p,
> };
> 
> @bad1@
> position p!={r1.p,ok1.p};
> identifier r1.i;
> @@
> i@p
> 
> @depends on !bad1 disable optional_qualifier@
> identifier r1.i;
> @@
> static
> +const
> struct w1_family_ops i={};
> // </smpl>
> 
> Signed-off-by: Rikard Falkeborn <rikard.falkeborn@gmail.com>
> ---

I suggest that this simply goes through the w1 tree together
with the other patches:

Acked-by: Sebastian Reichel <sre@kernel.org>

-- Sebastian

>  drivers/power/supply/bq27xxx_battery_hdq.c | 2 +-
>  drivers/power/supply/ds2760_battery.c      | 2 +-
>  drivers/power/supply/max1721x_battery.c    | 2 +-
>  3 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/power/supply/bq27xxx_battery_hdq.c b/drivers/power/supply/bq27xxx_battery_hdq.c
> index 12b10dad77d3..922759ab2e04 100644
> --- a/drivers/power/supply/bq27xxx_battery_hdq.c
> +++ b/drivers/power/supply/bq27xxx_battery_hdq.c
> @@ -97,7 +97,7 @@ static void bq27xxx_battery_hdq_remove_slave(struct w1_slave *sl)
>  	bq27xxx_battery_teardown(di);
>  }
>  
> -static struct w1_family_ops bq27xxx_battery_hdq_fops = {
> +static const struct w1_family_ops bq27xxx_battery_hdq_fops = {
>  	.add_slave	= bq27xxx_battery_hdq_add_slave,
>  	.remove_slave	= bq27xxx_battery_hdq_remove_slave,
>  };
> diff --git a/drivers/power/supply/ds2760_battery.c b/drivers/power/supply/ds2760_battery.c
> index 11bed88a89fa..695bb6747400 100644
> --- a/drivers/power/supply/ds2760_battery.c
> +++ b/drivers/power/supply/ds2760_battery.c
> @@ -795,7 +795,7 @@ static const struct of_device_id w1_ds2760_of_ids[] = {
>  };
>  #endif
>  
> -static struct w1_family_ops w1_ds2760_fops = {
> +static const struct w1_family_ops w1_ds2760_fops = {
>  	.add_slave	= w1_ds2760_add_slave,
>  	.remove_slave	= w1_ds2760_remove_slave,
>  	.groups		= w1_ds2760_groups,
> diff --git a/drivers/power/supply/max1721x_battery.c b/drivers/power/supply/max1721x_battery.c
> index 9ca895b0dabb..1b1a36f8e929 100644
> --- a/drivers/power/supply/max1721x_battery.c
> +++ b/drivers/power/supply/max1721x_battery.c
> @@ -431,7 +431,7 @@ static int devm_w1_max1721x_add_device(struct w1_slave *sl)
>  	return 0;
>  }
>  
> -static struct w1_family_ops w1_max1721x_fops = {
> +static const struct w1_family_ops w1_max1721x_fops = {
>  	.add_slave = devm_w1_max1721x_add_device,
>  };
>  
> -- 
> 2.28.0
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-10-04 22:03 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-04 19:31 [PATCH 0/3] w1: Constify w1_family_ops Rikard Falkeborn
2020-10-04 19:32 ` [PATCH 1/3] w1: Constify struct w1_family_ops Rikard Falkeborn
2020-10-04 21:55   ` Sebastian Reichel
2020-10-04 19:32 ` [PATCH 2/3] w1: Constify static w1_family_ops structs Rikard Falkeborn
2020-10-04 19:32 ` [PATCH 3/3] power: supply: " Rikard Falkeborn
2020-10-04 22:03   ` Sebastian Reichel

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).