* [PATCH] ep93xx/core.c: remove redundant static variable @ 2010-03-16 17:17 H Hartley Sweeten 2010-03-16 17:54 ` H Hartley Sweeten 0 siblings, 1 reply; 4+ messages in thread From: H Hartley Sweeten @ 2010-03-16 17:17 UTC (permalink / raw) To: linux-arm-kernel Use the pointer to the ep93xx_eth_data passed by the board support code directly instead of copying it to a local static variable. While here, also document the function used to register the ethernet device. Signed-off-by: H Hartley Sweeten <hsweeten@visionengravers.com> Cc: Ryan Mallon <ryan@bluewatersys.com> --- diff --git a/arch/arm/mach-ep93xx/core.c b/arch/arm/mach-ep93xx/core.c index 90fb591..3dfef45 100644 --- a/arch/arm/mach-ep93xx/core.c +++ b/arch/arm/mach-ep93xx/core.c @@ -330,8 +330,10 @@ static struct platform_device ep93xx_ohci_device = { .resource = ep93xx_ohci_resources, }; -static struct ep93xx_eth_data ep93xx_eth_data; +/************************************************************************* + * EP93xx ethernet peripheral handling + *************************************************************************/ static struct resource ep93xx_eth_resource[] = { { .start = EP93XX_ETHERNET_PHYS_BASE, @@ -347,19 +349,22 @@ static struct resource ep93xx_eth_resource[] = { static struct platform_device ep93xx_eth_device = { .name = "ep93xx-eth", .id = -1, - .dev = { - .platform_data = &ep93xx_eth_data, - }, .num_resources = ARRAY_SIZE(ep93xx_eth_resource), .resource = ep93xx_eth_resource, }; +/** + * ep93xx_register_eth - Register the built-in ethernet platform device. + * @data: platform specific ethernet configuration + * @copy_addr: flag indicating that the MAC address should be copied + * from the IndAd registers (as programmed by the bootloader) + */ void __init ep93xx_register_eth(struct ep93xx_eth_data *data, int copy_addr) { if (copy_addr) memcpy_fromio(data->dev_addr, EP93XX_ETHERNET_BASE + 0x50, 6); - ep93xx_eth_data = *data; + ep93xx_eth_device.dev.platform_data = data; platform_device_register(&ep93xx_eth_device); } ^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH] ep93xx/core.c: remove redundant static variable 2010-03-16 17:17 [PATCH] ep93xx/core.c: remove redundant static variable H Hartley Sweeten @ 2010-03-16 17:54 ` H Hartley Sweeten 2010-03-17 7:52 ` Lothar Waßmann 0 siblings, 1 reply; 4+ messages in thread From: H Hartley Sweeten @ 2010-03-16 17:54 UTC (permalink / raw) To: linux-arm-kernel Use the pointer to the passed board support data directly instead of copying it to local static variables. This updates the ethernet, i2c, and framebuffer devices. While here, also document the functions used to register the devices. Signed-off-by: H Hartley Sweeten <hsweeten@visionengravers.com> Cc: Ryan Mallon <ryan@bluewatersys.com> --- V2: spotted a couple others in addition to the ethernet device. diff --git a/arch/arm/mach-ep93xx/core.c b/arch/arm/mach-ep93xx/core.c index 90fb591..5f5bc6d 100644 --- a/arch/arm/mach-ep93xx/core.c +++ b/arch/arm/mach-ep93xx/core.c @@ -330,8 +330,10 @@ static struct platform_device ep93xx_ohci_device = { .resource = ep93xx_ohci_resources, }; -static struct ep93xx_eth_data ep93xx_eth_data; +/************************************************************************* + * EP93xx ethernet peripheral handling + *************************************************************************/ static struct resource ep93xx_eth_resource[] = { { .start = EP93XX_ETHERNET_PHYS_BASE, @@ -347,19 +349,22 @@ static struct resource ep93xx_eth_resource[] = { static struct platform_device ep93xx_eth_device = { .name = "ep93xx-eth", .id = -1, - .dev = { - .platform_data = &ep93xx_eth_data, - }, .num_resources = ARRAY_SIZE(ep93xx_eth_resource), .resource = ep93xx_eth_resource, }; +/** + * ep93xx_register_eth - Register the built-in ethernet platform device. + * @data: platform specific ethernet configuration + * @copy_addr: flag indicating that the MAC address should be copied + * from the IndAd registers (as programmed by the bootloader) + */ void __init ep93xx_register_eth(struct ep93xx_eth_data *data, int copy_addr) { if (copy_addr) memcpy_fromio(data->dev_addr, EP93XX_ETHERNET_BASE + 0x50, 6); - ep93xx_eth_data = *data; + ep93xx_eth_device.dev.platform_data = data; platform_device_register(&ep93xx_eth_device); } @@ -367,14 +372,17 @@ void __init ep93xx_register_eth(struct ep93xx_eth_data *data, int copy_addr) /************************************************************************* * EP93xx i2c peripheral handling *************************************************************************/ -static struct i2c_gpio_platform_data ep93xx_i2c_data; - static struct platform_device ep93xx_i2c_device = { .name = "i2c-gpio", .id = 0, - .dev.platform_data = &ep93xx_i2c_data, }; +/** + * ep93xx_register_i2c - Register the i2c platform device. + * @data: platform specific i2c-gpio configurtion + * @devices: platform specific i2c bus device information + * @num: the number of devices on the i2c bus + */ void __init ep93xx_register_i2c(struct i2c_gpio_platform_data *data, struct i2c_board_info *devices, int num) { @@ -393,7 +401,7 @@ void __init ep93xx_register_i2c(struct i2c_gpio_platform_data *data, (data->scl_is_open_drain << 0), EP93XX_GPIO_EEDRIVE); - ep93xx_i2c_data = *data; + ep93xx_i2c_device.dev.platform_data = data; i2c_register_board_info(0, devices, num); platform_device_register(&ep93xx_i2c_device); } @@ -514,8 +522,6 @@ EXPORT_SYMBOL(ep93xx_pwm_release_gpio); /************************************************************************* * EP93xx video peripheral handling *************************************************************************/ -static struct ep93xxfb_mach_info ep93xxfb_data; - static struct resource ep93xx_fb_resource[] = { { .start = EP93XX_RASTER_PHYS_BASE, @@ -528,7 +534,6 @@ static struct platform_device ep93xx_fb_device = { .name = "ep93xx-fb", .id = -1, .dev = { - .platform_data = &ep93xxfb_data, .coherent_dma_mask = DMA_BIT_MASK(32), .dma_mask = &ep93xx_fb_device.dev.coherent_dma_mask, }, @@ -536,9 +541,13 @@ static struct platform_device ep93xx_fb_device = { .resource = ep93xx_fb_resource, }; +/** + * ep93xx_register_fb - Register the framebuffer platform device. + * @data: platform specific framebuffer configuration + */ void __init ep93xx_register_fb(struct ep93xxfb_mach_info *data) { - ep93xxfb_data = *data; + ep93xx_fb_device.dev.platform_data = data; platform_device_register(&ep93xx_fb_device); } ^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH] ep93xx/core.c: remove redundant static variable 2010-03-16 17:54 ` H Hartley Sweeten @ 2010-03-17 7:52 ` Lothar Waßmann 2010-03-17 16:39 ` H Hartley Sweeten 0 siblings, 1 reply; 4+ messages in thread From: Lothar Waßmann @ 2010-03-17 7:52 UTC (permalink / raw) To: linux-arm-kernel Hi, > Use the pointer to the passed board support data directly instead of > copying it to local static variables. > > This updates the ethernet, i2c, and framebuffer devices. > > While here, also document the functions used to register the devices. > > Signed-off-by: H Hartley Sweeten <hsweeten@visionengravers.com> > Cc: Ryan Mallon <ryan@bluewatersys.com> > > --- > > V2: spotted a couple others in addition to the ethernet device. > > diff --git a/arch/arm/mach-ep93xx/core.c b/arch/arm/mach-ep93xx/core.c > index 90fb591..5f5bc6d 100644 > --- a/arch/arm/mach-ep93xx/core.c > +++ b/arch/arm/mach-ep93xx/core.c > @@ -330,8 +330,10 @@ static struct platform_device ep93xx_ohci_device = { > .resource = ep93xx_ohci_resources, > }; > > -static struct ep93xx_eth_data ep93xx_eth_data; > > +/************************************************************************* > + * EP93xx ethernet peripheral handling > + *************************************************************************/ > static struct resource ep93xx_eth_resource[] = { > { > .start = EP93XX_ETHERNET_PHYS_BASE, > @@ -347,19 +349,22 @@ static struct resource ep93xx_eth_resource[] = { > static struct platform_device ep93xx_eth_device = { > .name = "ep93xx-eth", > .id = -1, > - .dev = { > - .platform_data = &ep93xx_eth_data, > - }, > .num_resources = ARRAY_SIZE(ep93xx_eth_resource), > .resource = ep93xx_eth_resource, > }; > > +/** > + * ep93xx_register_eth - Register the built-in ethernet platform device. > + * @data: platform specific ethernet configuration > + * @copy_addr: flag indicating that the MAC address should be copied > + * from the IndAd registers (as programmed by the bootloader) > + */ > void __init ep93xx_register_eth(struct ep93xx_eth_data *data, int copy_addr) > { > if (copy_addr) > memcpy_fromio(data->dev_addr, EP93XX_ETHERNET_BASE + 0x50, 6); > > - ep93xx_eth_data = *data; > + ep93xx_eth_device.dev.platform_data = data; > platform_device_register(&ep93xx_eth_device); > } > I don't think this is a good idea. In a multi platform kernel each platform that could potentially register the device needs to have the platform_data lying around in memory even if it's never used. In the old way the platforms could have the platform_data that is passed to the ep93xx_register_eth() function marked as __initdata and only one copy that is actually used would be kept. Lothar Wa?mann -- ___________________________________________________________ Ka-Ro electronics GmbH | Pascalstra?e 22 | D - 52076 Aachen Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10 Gesch?ftsf?hrer: Matthias Kaussen Handelsregistereintrag: Amtsgericht Aachen, HRB 4996 www.karo-electronics.de | info at karo-electronics.de ___________________________________________________________ ^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH] ep93xx/core.c: remove redundant static variable 2010-03-17 7:52 ` Lothar Waßmann @ 2010-03-17 16:39 ` H Hartley Sweeten 0 siblings, 0 replies; 4+ messages in thread From: H Hartley Sweeten @ 2010-03-17 16:39 UTC (permalink / raw) To: linux-arm-kernel On Wednesday, March 17, 2010 12:53 AM, Lothar Wa?mann wrote: > > Hi, Hello. >> Use the pointer to the passed board support data directly instead of >> copying it to local static variables. >> >> This updates the ethernet, i2c, and framebuffer devices. >> >> While here, also document the functions used to register the devices. >> >> Signed-off-by: H Hartley Sweeten <hsweeten@visionengravers.com> >> Cc: Ryan Mallon <ryan@bluewatersys.com> >> >> --- > I don't think this is a good idea. > In a multi platform kernel each platform that could potentially > register the device needs to have the platform_data lying around in > memory even if it's never used. > > In the old way the platforms could have the platform_data that is > passed to the ep93xx_register_eth() function marked as __initdata and > only one copy that is actually used would be kept. Ok. That makes sense. Based on that, all the ep93xx board init code needs to be checked to make sure any data that is passed to the core code and copied to a local static variable is marked as __initdata. I'll rework this and post a new patch in a bit. Thanks, Hartley ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2010-03-17 16:39 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2010-03-16 17:17 [PATCH] ep93xx/core.c: remove redundant static variable H Hartley Sweeten 2010-03-16 17:54 ` H Hartley Sweeten 2010-03-17 7:52 ` Lothar Waßmann 2010-03-17 16:39 ` H Hartley Sweeten
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox