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.
>
> Created and tested on top of current linux-omap/omap1, commit
> 967809bd7faf71ddc29c8081e0f21db8b201a0f4.
>
> Signed-off-by: Janusz Krzysztofik <jkrzyszt@xxxxxxxxxxxx>
> ---
> 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.
> /* 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.
> .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.
> [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.
> [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.
> .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.
> {
> 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.
> .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.
> &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.
> {
> .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 = {
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
