Skip to contentSkip to conversation

OMAP: drop __initdata tags from static struct platform_device declarations

PATCH Superseded Janusz Krzysztofik 3 participants 4 messages Jun 16, 2011 – Jul 1, 2011

Subsystems (4)
Arm (32-bit) Arm sub-architectures OMAP2+ support OMAP1 support
lore
Share & tools
lore

Janusz Krzysztofik

author
#0 lore raw

PATCH 1/1 OMAP: drop __initdata tags from static struct platform_device declarations

Pointers to statically declared platform device structures which are registered with platform_device_register() are then used during run time to access these structure members, for example from platform_uevent() and much more. Therefore, these structures should never be placed inside sections which are dropped after boot. Fix platform devices incorrectly tagged with __initdata which happen to exist inside OMAP sub-trees.

This bug has exhibited itself on my ARM/OMAP1 based Amstrad Delta videophone after commit 6d3163ce86dd386b4f7bda80241d7fea2bc0bb1d, "mm:
check if any page in a pageblock is reserved before marking it MIGRATE_RESERVE", resulting in reading from several /sys/device/platform/*/uevent files always ending up with segmentation faults.

Signed-off-by Janusz Krzysztofik <[email protected]>
Acked-by Felipe Balbi <[email protected]>
---
 arch/arm/mach-omap1/board-ams-delta.c        |    8 ++++----
 arch/arm/mach-omap1/gpio15xx.c               |    4 ++--
 arch/arm/mach-omap1/gpio16xx.c               |   10 +++++-----
 arch/arm/mach-omap1/gpio7xx.c                |   14 +++++++-------
 arch/arm/mach-omap2/board-rx51-peripherals.c |    2 +-
 5 files changed, 19 insertions(+), 19 deletions(-)
arch/arm/mach-omap1/board-ams-delta.c renamed from arch/arm/mach-omap1/board-ams-delta.c.orig +4 -4
@@ -215,7 +215,7 @@ static struct omap_kp_platform_data ams_
 	.delay		= 9,
 };
 
-static struct platform_device ams_delta_kp_device __initdata = {
+static struct platform_device ams_delta_kp_device = {
 	.name		= "omap-keypad",
 	.id		= -1,
 	.dev		= {

@@ -225,12 +225,12 @@ static struct platform_device ams_delta_
 	.resource	= ams_delta_kp_resources,
 };
 
-static struct platform_device ams_delta_lcd_device __initdata = {
+static struct platform_device ams_delta_lcd_device = {
 	.name	= "lcd_ams_delta",
 	.id	= -1,
 };
 
-static struct platform_device ams_delta_led_device __initdata = {
+static struct platform_device ams_delta_led_device = {
 	.name	= "ams-delta-led",
 	.id	= -1
 };

@@ -267,7 +267,7 @@ static struct soc_camera_link ams_delta_
 	.power		= ams_delta_camera_power,
 };
 
-static struct platform_device ams_delta_camera_device __initdata = {
+static struct platform_device ams_delta_camera_device = {
 	.name   = "soc-camera-pdrv",
 	.id     = 0,
 	.dev    = {
arch/arm/mach-omap1/gpio7xx.c renamed from arch/arm/mach-omap1/gpio7xx.c.orig +7 -7
@@ -46,7 +46,7 @@ static struct __initdata omap_gpio_platf
 	.bank_stride		= 2,
 };
 
-static struct __initdata platform_device omap7xx_mpu_gpio = {
+static struct platform_device omap7xx_mpu_gpio = {
 	.name           = "omap_gpio",
 	.id             = 0,
 	.dev            = {

@@ -75,7 +75,7 @@ static struct __initdata omap_gpio_platf
 	.bank_width		= 32,
 };
 
-static struct __initdata platform_device omap7xx_gpio1 = {
+static struct platform_device omap7xx_gpio1 = {
 	.name           = "omap_gpio",
 	.id             = 1,
 	.dev            = {

@@ -104,7 +104,7 @@ static struct __initdata omap_gpio_platf
 	.bank_width		= 32,
 };
 
-static struct __initdata platform_device omap7xx_gpio2 = {
+static struct platform_device omap7xx_gpio2 = {
 	.name           = "omap_gpio",
 	.id             = 2,
 	.dev            = {

@@ -133,7 +133,7 @@ static struct __initdata omap_gpio_platf
 	.bank_width		= 32,
 };
 
-static struct __initdata platform_device omap7xx_gpio3 = {
+static struct platform_device omap7xx_gpio3 = {
 	.name           = "omap_gpio",
 	.id             = 3,
 	.dev            = {

@@ -162,7 +162,7 @@ static struct __initdata omap_gpio_platf
 	.bank_width		= 32,
 };
 
-static struct __initdata platform_device omap7xx_gpio4 = {
+static struct platform_device omap7xx_gpio4 = {
 	.name           = "omap_gpio",
 	.id             = 4,
 	.dev            = {

@@ -191,7 +191,7 @@ static struct __initdata omap_gpio_platf
 	.bank_width		= 32,
 };
 
-static struct __initdata platform_device omap7xx_gpio5 = {
+static struct platform_device omap7xx_gpio5 = {
 	.name           = "omap_gpio",
 	.id             = 5,
 	.dev            = {

@@ -220,7 +220,7 @@ static struct __initdata omap_gpio_platf
 	.bank_width		= 32,
 };
 
-static struct __initdata platform_device omap7xx_gpio6 = {
+static struct platform_device omap7xx_gpio6 = {
 	.name           = "omap_gpio",
 	.id             = 6,
 	.dev            = {
arch/arm/mach-omap1/gpio15xx.c renamed from arch/arm/mach-omap1/gpio15xx.c.orig +2 -2
@@ -41,7 +41,7 @@ static struct __initdata omap_gpio_platf
 	.bank_stride		= 1,
 };
 
-static struct __initdata platform_device omap15xx_mpu_gpio = {
+static struct platform_device omap15xx_mpu_gpio = {
 	.name           = "omap_gpio",
 	.id             = 0,
 	.dev            = {

@@ -70,7 +70,7 @@ static struct __initdata omap_gpio_platf
 	.bank_width		= 16,
 };
 
-static struct __initdata platform_device omap15xx_gpio = {
+static struct platform_device omap15xx_gpio = {
 	.name           = "omap_gpio",
 	.id             = 1,
 	.dev            = {
arch/arm/mach-omap1/gpio16xx.c renamed from arch/arm/mach-omap1/gpio16xx.c.orig +5 -5
@@ -44,7 +44,7 @@ static struct __initdata omap_gpio_platf
 	.bank_stride		= 1,
 };
 
-static struct __initdata platform_device omap16xx_mpu_gpio = {
+static struct platform_device omap16xx_mpu_gpio = {
 	.name           = "omap_gpio",
 	.id             = 0,
 	.dev            = {

@@ -73,7 +73,7 @@ static struct __initdata omap_gpio_platf
 	.bank_width		= 16,
 };
 
-static struct __initdata platform_device omap16xx_gpio1 = {
+static struct platform_device omap16xx_gpio1 = {
 	.name           = "omap_gpio",
 	.id             = 1,
 	.dev            = {

@@ -102,7 +102,7 @@ static struct __initdata omap_gpio_platf
 	.bank_width		= 16,
 };
 
-static struct __initdata platform_device omap16xx_gpio2 = {
+static struct platform_device omap16xx_gpio2 = {
 	.name           = "omap_gpio",
 	.id             = 2,
 	.dev            = {

@@ -131,7 +131,7 @@ static struct __initdata omap_gpio_platf
 	.bank_width		= 16,
 };
 
-static struct __initdata platform_device omap16xx_gpio3 = {
+static struct platform_device omap16xx_gpio3 = {
 	.name           = "omap_gpio",
 	.id             = 3,
 	.dev            = {

@@ -160,7 +160,7 @@ static struct __initdata omap_gpio_platf
 	.bank_width		= 16,
 };
 
-static struct __initdata platform_device omap16xx_gpio4 = {
+static struct platform_device omap16xx_gpio4 = {
 	.name           = "omap_gpio",
 	.id             = 4,
 	.dev            = {
arch/arm/mach-omap2/board-rx51-peripherals.c renamed from arch/arm/mach-omap2/board-rx51-peripherals.c.orig +1 -1
@@ -558,7 +558,7 @@ static struct radio_si4713_platform_data
 	.subdev_board_info = &rx51_si4713_board_info,
 };
 
-static struct platform_device rx51_si4713_dev __initdata_or_module = {
+static struct platform_device rx51_si4713_dev = {
 	.name	= "radio-si4713",
 	.id	= -1,
 	.dev	= {
#1 lore raw

PATCH 1/1 OMAP: drop __initdata tags from static struct platform_device declarations

Quoted 6 lines · Janusz Krzysztofik

Pointers to statically declared platform device structures which are registered with platform_device_register() are then used during run time to access these structure members, for example from platform_uevent() and much more. Therefore, these structures should never be placed inside sections which are dropped after boot. Fix platform devices incorrectly tagged with __initdata which happen to exist inside OMAP sub-trees.

It would be a good idea to cc the authors of the patches which introduced this stuff, so that they learn that registering platform devices marked with __initdata is a bad idea. We live in hope that doing so will prevent it happening in the future.

author
#2 lore raw

PATCH RESEND 1/1 OMAP: drop __initdata tags from static struct platform_device declarations

Pointers to statically declared platform device structures which are registered with platform_device_register() are then used during run time to access these structure members, for example from platform_uevent() and much more. Therefore, these structures should never be placed inside sections which are dropped after boot. Fix platform devices incorrectly tagged with __initdata which happen to exist inside OMAP sub-trees.

This bug has exhibited itself on my ARM/OMAP1 based Amstrad Delta videophone after commit 6d3163ce86dd386b4f7bda80241d7fea2bc0bb1d, "mm:
check if any page in a pageblock is reserved before marking it MIGRATE_RESERVE", resulting in reading from several /sys/device/platform/*/uevent files always ending up with segmentation faults.

Signed-off-by Janusz Krzysztofik <[email protected]>
Acked-by Felipe Balbi <[email protected]>
Cc Varadarajan, Charulatha <[email protected]>
Cc Jarkko Nikula <[email protected]>

---

Quoted 4 lines · On Fri 17 Jun 2011 at 00:18:32 CET Russell King - ARM Linux wrote:

It would be a good idea to cc the authors of the patches which introduced this stuff, so that they learn that registering platform devices marked with __initdata is a bad idea. We live in hope that doing so will prevent it happening in the future.

Resending with Cc: as requested.

Thanks,
Janusz

 arch/arm/mach-omap1/board-ams-delta.c        |    8 ++++----
 arch/arm/mach-omap1/gpio15xx.c               |    4 ++--
 arch/arm/mach-omap1/gpio16xx.c               |   10 +++++-----
 arch/arm/mach-omap1/gpio7xx.c                |   14 +++++++-------
 arch/arm/mach-omap2/board-rx51-peripherals.c |    2 +-
 5 files changed, 19 insertions(+), 19 deletions(-)
arch/arm/mach-omap1/board-ams-delta.c renamed from arch/arm/mach-omap1/board-ams-delta.c.orig +4 -4
@@ -215,7 +215,7 @@ static struct omap_kp_platform_data ams_
 	.delay		= 9,
 };
 
-static struct platform_device ams_delta_kp_device __initdata = {
+static struct platform_device ams_delta_kp_device = {
 	.name		= "omap-keypad",
 	.id		= -1,
 	.dev		= {

@@ -225,12 +225,12 @@ static struct platform_device ams_delta_
 	.resource	= ams_delta_kp_resources,
 };
 
-static struct platform_device ams_delta_lcd_device __initdata = {
+static struct platform_device ams_delta_lcd_device = {
 	.name	= "lcd_ams_delta",
 	.id	= -1,
 };
 
-static struct platform_device ams_delta_led_device __initdata = {
+static struct platform_device ams_delta_led_device = {
 	.name	= "ams-delta-led",
 	.id	= -1
 };

@@ -267,7 +267,7 @@ static struct soc_camera_link ams_delta_
 	.power		= ams_delta_camera_power,
 };
 
-static struct platform_device ams_delta_camera_device __initdata = {
+static struct platform_device ams_delta_camera_device = {
 	.name   = "soc-camera-pdrv",
 	.id     = 0,
 	.dev    = {
arch/arm/mach-omap1/gpio7xx.c renamed from arch/arm/mach-omap1/gpio7xx.c.orig +7 -7
@@ -46,7 +46,7 @@ static struct __initdata omap_gpio_platf
 	.bank_stride		= 2,
 };
 
-static struct __initdata platform_device omap7xx_mpu_gpio = {
+static struct platform_device omap7xx_mpu_gpio = {
 	.name           = "omap_gpio",
 	.id             = 0,
 	.dev            = {

@@ -75,7 +75,7 @@ static struct __initdata omap_gpio_platf
 	.bank_width		= 32,
 };
 
-static struct __initdata platform_device omap7xx_gpio1 = {
+static struct platform_device omap7xx_gpio1 = {
 	.name           = "omap_gpio",
 	.id             = 1,
 	.dev            = {

@@ -104,7 +104,7 @@ static struct __initdata omap_gpio_platf
 	.bank_width		= 32,
 };
 
-static struct __initdata platform_device omap7xx_gpio2 = {
+static struct platform_device omap7xx_gpio2 = {
 	.name           = "omap_gpio",
 	.id             = 2,
 	.dev            = {

@@ -133,7 +133,7 @@ static struct __initdata omap_gpio_platf
 	.bank_width		= 32,
 };
 
-static struct __initdata platform_device omap7xx_gpio3 = {
+static struct platform_device omap7xx_gpio3 = {
 	.name           = "omap_gpio",
 	.id             = 3,
 	.dev            = {

@@ -162,7 +162,7 @@ static struct __initdata omap_gpio_platf
 	.bank_width		= 32,
 };
 
-static struct __initdata platform_device omap7xx_gpio4 = {
+static struct platform_device omap7xx_gpio4 = {
 	.name           = "omap_gpio",
 	.id             = 4,
 	.dev            = {

@@ -191,7 +191,7 @@ static struct __initdata omap_gpio_platf
 	.bank_width		= 32,
 };
 
-static struct __initdata platform_device omap7xx_gpio5 = {
+static struct platform_device omap7xx_gpio5 = {
 	.name           = "omap_gpio",
 	.id             = 5,
 	.dev            = {

@@ -220,7 +220,7 @@ static struct __initdata omap_gpio_platf
 	.bank_width		= 32,
 };
 
-static struct __initdata platform_device omap7xx_gpio6 = {
+static struct platform_device omap7xx_gpio6 = {
 	.name           = "omap_gpio",
 	.id             = 6,
 	.dev            = {
arch/arm/mach-omap1/gpio15xx.c renamed from arch/arm/mach-omap1/gpio15xx.c.orig +2 -2
@@ -41,7 +41,7 @@ static struct __initdata omap_gpio_platf
 	.bank_stride		= 1,
 };
 
-static struct __initdata platform_device omap15xx_mpu_gpio = {
+static struct platform_device omap15xx_mpu_gpio = {
 	.name           = "omap_gpio",
 	.id             = 0,
 	.dev            = {

@@ -70,7 +70,7 @@ static struct __initdata omap_gpio_platf
 	.bank_width		= 16,
 };
 
-static struct __initdata platform_device omap15xx_gpio = {
+static struct platform_device omap15xx_gpio = {
 	.name           = "omap_gpio",
 	.id             = 1,
 	.dev            = {
arch/arm/mach-omap1/gpio16xx.c renamed from arch/arm/mach-omap1/gpio16xx.c.orig +5 -5
@@ -44,7 +44,7 @@ static struct __initdata omap_gpio_platf
 	.bank_stride		= 1,
 };
 
-static struct __initdata platform_device omap16xx_mpu_gpio = {
+static struct platform_device omap16xx_mpu_gpio = {
 	.name           = "omap_gpio",
 	.id             = 0,
 	.dev            = {

@@ -73,7 +73,7 @@ static struct __initdata omap_gpio_platf
 	.bank_width		= 16,
 };
 
-static struct __initdata platform_device omap16xx_gpio1 = {
+static struct platform_device omap16xx_gpio1 = {
 	.name           = "omap_gpio",
 	.id             = 1,
 	.dev            = {

@@ -102,7 +102,7 @@ static struct __initdata omap_gpio_platf
 	.bank_width		= 16,
 };
 
-static struct __initdata platform_device omap16xx_gpio2 = {
+static struct platform_device omap16xx_gpio2 = {
 	.name           = "omap_gpio",
 	.id             = 2,
 	.dev            = {

@@ -131,7 +131,7 @@ static struct __initdata omap_gpio_platf
 	.bank_width		= 16,
 };
 
-static struct __initdata platform_device omap16xx_gpio3 = {
+static struct platform_device omap16xx_gpio3 = {
 	.name           = "omap_gpio",
 	.id             = 3,
 	.dev            = {

@@ -160,7 +160,7 @@ static struct __initdata omap_gpio_platf
 	.bank_width		= 16,
 };
 
-static struct __initdata platform_device omap16xx_gpio4 = {
+static struct platform_device omap16xx_gpio4 = {
 	.name           = "omap_gpio",
 	.id             = 4,
 	.dev            = {
arch/arm/mach-omap2/board-rx51-peripherals.c renamed from arch/arm/mach-omap2/board-rx51-peripherals.c.orig +1 -1
@@ -558,7 +558,7 @@ static struct radio_si4713_platform_data
 	.subdev_board_info = &rx51_si4713_board_info,
 };
 
-static struct platform_device rx51_si4713_dev __initdata_or_module = {
+static struct platform_device rx51_si4713_dev = {
 	.name	= "radio-si4713",
 	.id	= -1,
 	.dev	= {
maintainer
#3 lore raw

PATCH RESEND 1/1 OMAP: drop __initdata tags from static struct platform_device declarations

Quoted 7 lines · Janusz Krzysztofik

Pointers to statically declared platform device structures which are registered with platform_device_register() are then used during run time to access these structure members, for example from platform_uevent() and much more. Therefore, these structures should never be placed inside sections which are dropped after boot. Fix platform devices incorrectly tagged with __initdata which happen to exist inside OMAP sub-trees.

Quoted 6 lines

This bug has exhibited itself on my ARM/OMAP1 based Amstrad Delta videophone after commit 6d3163ce86dd386b4f7bda80241d7fea2bc0bb1d, "mm:
check if any page in a pageblock is reserved before marking it MIGRATE_RESERVE", resulting in reading from several /sys/device/platform/*/uevent files always ending up with segmentation faults.

Thanks applying into devel-fixes.

Tony