* [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