[PATCH] ARM: OMAP1: ams-delta: clean up init data section assignments
From: Russell King - ARM Linux <hidden>
Date: 2012-02-09 14:48:53
Also in:
linux-fbdev, linux-input, linux-omap
On Thu, Feb 09, 2012 at 12:18:22PM +0100, Janusz Krzysztofik wrote:
The main purpose of this patch is to fix several section mismatch warnings from the board file and a few board specific drivers, introduced mostly with recent Amstrad Delta patch series, some of them rising up only when building with CONFIG_MODULES not set. While being at it, section assignments of all init data found in the board file have been revised and hopefully optimised.
There is no optimisation to adding __refdata to stuff. That's screaming that you have lots of bugs here.
quoted hunk ↗ jump to hunk
Created and tested on top of current linux-omap/omap1, commit 967809bd7faf71ddc29c8081e0f21db8b201a0f4. Signed-off-by: Janusz Krzysztofik <redacted> --- arch/arm/mach-omap1/board-ams-delta.c | 35 +++++++++++++++++---------------- drivers/input/serio/ams_delta_serio.c | 2 +- drivers/mtd/nand/ams-delta.c | 2 +- drivers/video/omap/lcd_ams_delta.c | 2 +- 4 files changed, 21 insertions(+), 20 deletions(-)diff --git a/arch/arm/mach-omap1/board-ams-delta.c b/arch/arm/mach-omap1/board-ams-delta.c index 87b1303..dc2455f 100644 --- a/arch/arm/mach-omap1/board-ams-delta.c +++ b/arch/arm/mach-omap1/board-ams-delta.c@@ -126,7 +126,7 @@ static const unsigned int ams_delta_keymap[] = { #define LATCH2_PHYS 0x08000000 #define LATCH2_VIRT 0xEC000000 -static struct map_desc ams_delta_io_desc[] __initdata = { +static struct map_desc ams_delta_io_desc[] __initconst = {
You're not making this comst so don't change it to __initconst.
quoted hunk ↗ jump to hunk
/* AMS_DELTA_LATCH1 */ { .virtual = LATCH1_VIRT,@@ -150,17 +150,17 @@ static struct map_desc ams_delta_io_desc[] __initdata = { } }; -static struct omap_lcd_config ams_delta_lcd_config = { +static struct omap_lcd_config ams_delta_lcd_config __initconst = {
This change probably adds a bug. Are you sure this will never be used outside init context?
.ctrl_name = "internal",
};
-static struct omap_usb_config ams_delta_usb_config __initdata = {
+static struct omap_usb_config ams_delta_usb_config __initdata_or_module = {
Even if you don't have modules enabled, have you checked whether the
driver can be bound and unbound via
/sys/bus/platform/driver/<name>/{bind,unbind} ?
I suspect this is a bug.
quoted hunk ↗ jump to hunk
.register_host = 1, .hmc_mode = 16, .pins[0] = 2, }; -static struct omap_board_config_kernel ams_delta_config[] __initdata = { +static struct omap_board_config_kernel ams_delta_config[] __initconst = { { OMAP_TAG_LCD, &ams_delta_lcd_config }, };@@ -181,7 +181,7 @@ static struct bgpio_pdata latch1_pdata __initconst = { .ngpio = LATCH1_NGPIO, }; -static struct platform_device latch1_gpio_device = { +static struct platform_device latch1_gpio_device __refdata = { .name = "basic-mmio-gpio", .id = 0, .resource = latch1_resources,@@ -205,7 +205,7 @@ static struct bgpio_pdata latch2_pdata __initconst = { .ngpio = AMS_DELTA_LATCH2_NGPIO, }; -static struct platform_device latch2_gpio_device = { +static struct platform_device latch2_gpio_device __refdata = { .name = "basic-mmio-gpio", .id = 1, .resource = latch2_resources,@@ -271,7 +271,7 @@ void ams_delta_latch_write(int base, int ngpio, u16 mask, u16 value) } EXPORT_SYMBOL(ams_delta_latch_write); -static struct resource ams_delta_nand_resources[] = { +static struct resource ams_delta_nand_resources[] __initconst_or_module = {
This change definitely adds a bug. The resources are _used_ _all_ _the_ _time_ _the_ _device_ _is_ _registered_. Try catting /proc/iomem after boot.
quoted hunk ↗ jump to hunk
[0] = { .start = OMAP1_MPUIO_BASE, .end = OMAP1_MPUIO_BASE +@@ -280,14 +280,14 @@ static struct resource ams_delta_nand_resources[] = { }, }; -static struct platform_device ams_delta_nand_device = { +static struct platform_device ams_delta_nand_device __refdata = {
Therefore, bug.
.name = "ams-delta-nand",
.id = -1,
.num_resources = ARRAY_SIZE(ams_delta_nand_resources),
.resource = ams_delta_nand_resources,
};
-static struct resource ams_delta_kp_resources[] = {
+static struct resource ams_delta_kp_resources[] __initconst_or_module = {Bug.
quoted hunk ↗ jump to hunk
[0] = { .start = INT_KEYBOARD, .end = INT_KEYBOARD,@@ -300,14 +300,14 @@ static const struct matrix_keymap_data ams_delta_keymap_data = { .keymap_size = ARRAY_SIZE(ams_delta_keymap), }; -static struct omap_kp_platform_data ams_delta_kp_data __initdata = { +static struct omap_kp_platform_data ams_delta_kp_data __initconst_or_module = {
Probably a bug if you unbind/rebind the associated driver with modules disabled.
quoted hunk ↗ jump to hunk
.rows = 8, .cols = 8, .keymap_data = &ams_delta_keymap_data, .delay = 9, }; -static struct platform_device ams_delta_kp_device = { +static struct platform_device ams_delta_kp_device __refdata = { .name = "omap-keypad", .id = -1, .dev = {@@ -363,7 +363,8 @@ static struct gpio_led_platform_data leds_pdata __initconst = { .num_leds = ARRAY_SIZE(gpio_leds), }; -static struct i2c_board_info ams_delta_camera_board_info[] = { +static struct i2c_board_info __initconst_or_module +ams_delta_camera_board_info[] = {
No. It's static struct foo blah[] __whatever_init_attribute not static struct foo __whatever_init_attribute blah[] I've no idea where this fucked idea came from but it's something that's wasting a lot of review time with complaints like this about it.
quoted hunk ↗ jump to hunk
{ I2C_BOARD_INFO("ov6650", 0x60), },@@ -387,7 +388,7 @@ static int ams_delta_camera_power(struct device *dev, int power) #define ams_delta_camera_power NULL #endif -static struct soc_camera_link ams_delta_iclink = { +static struct soc_camera_link ams_delta_iclink __initconst_or_module = {
Probably buggy.
quoted hunk ↗ jump to hunk
.bus_id = 0, /* OMAP1 SoC camera bus */ .i2c_adapter_id = 1, .board_info = &ams_delta_camera_board_info[0],@@ -395,7 +396,7 @@ static struct soc_camera_link ams_delta_iclink = { .power = ams_delta_camera_power, }; -static struct platform_device ams_delta_camera_device = { +static struct platform_device ams_delta_camera_device __refdata = { .name = "soc-camera-pdrv", .id = 0, .dev = {@@ -408,7 +409,7 @@ static struct omap1_cam_platform_data ams_delta_camera_platform_data = { .lclk_khz_max = 1334, /* results in 5fps CIF, 10fps QCIF */ }; -static struct platform_device *ams_delta_devices[] __initdata = { +static struct platform_device *ams_delta_devices[] __initconst = {
Missing const. If you can't const it, don't put it in __initconst.
quoted hunk ↗ jump to hunk
&latch1_gpio_device, &latch2_gpio_device, &ams_delta_kp_device,@@ -459,7 +460,7 @@ static void __init ams_delta_init(void) omap_writew(omap_readw(ARM_RSTCT1) | 0x0004, ARM_RSTCT1); } -static struct plat_serial8250_port ams_delta_modem_ports[] = { +static struct plat_serial8250_port ams_delta_modem_ports[] __initdata = {
Buggy.
quoted hunk ↗ jump to hunk
{ .membase = IOMEM(MODEM_VIRT), .mapbase = MODEM_PHYS,@@ -473,7 +474,7 @@ static struct plat_serial8250_port ams_delta_modem_ports[] = { { }, }; -static struct platform_device ams_delta_modem_device = { +static struct platform_device ams_delta_modem_device __refdata = { .name = "serial8250", .id = PLAT8250_DEV_PLATFORM1, .dev = {