Skip to contentSkip to conversation

ARM: mx28evk: remove flexcan_pdata __initconst attribute

PATCH New Dong Aisheng 3 participants 9 messages Nov 11, 2011

Subsystems (4)
Arm (32-bit) Arm sub-architectures Arm i.MX and Layerscape Arm STM32
lore
Share & tools
lore

Dong Aisheng

author
#0 lore raw

PATCH 1/2 ARM: mx28evk: remove flexcan_pdata __initconst attribute

The flexcan driver still uses it after init.

Signed-off-by Dong Aisheng <[email protected]>
Cc Sascha Hauer <[email protected]>
Cc Shawn Guo <[email protected]>
Cc Wolfram Sang <[email protected]>
Cc Uwe Kleine-K?nig <[email protected]>
---
 arch/arm/mach-mxs/mach-mx28evk.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
arch/arm/mach-mxs/mach-mx28evk.c +1 -1
index 4a3cca3..226ba64 100644
@@ -327,7 +327,7 @@ static void mx28evk_flexcan1_switch(int enable)
 }
 
 static const struct flexcan_platform_data
-		mx28evk_flexcan_pdata[] __initconst = {
+		mx28evk_flexcan_pdata[] = {
 	{
 		.transceiver_switch = mx28evk_flexcan0_switch,
 	}, {
Signature · 1 line
1.7.0.4
#1 lore raw

PATCH 1/2 ARM: mx28evk: remove flexcan_pdata __initconst attribute

Quoted 1 line · Dong Aisheng

The flexcan driver still uses it after init.

As "it" is a copy of flexcan_pdata it's completely OK for the driver to use it.

What is the exact problem you want to address?

Uwe

Signature · 2 lines
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
author
#2 lore raw

PATCH 1/2 ARM: mx28evk: remove flexcan_pdata __initconst attribute

Quoted 9 lines

-----Original Message-----
From: Uwe Kleine-K?nig [mailto:u.kleine-koenig at pengutronix.de]
Sent: Friday, November 11, 2011 11:06 PM
To: Dong Aisheng-B29396
Cc: linux-arm-kernel at lists.infradead.org; kernel at pengutronix.de; s.hauer at pengutronix.de; w.sang at pengutronix.de; Guo Shawn-R65073
Subject: Re: [PATCH 1/2] ARM: mx28evk: remove flexcan_pdata __initconst attribute

Quoted 7 lines
Quoted 1 line · Dong Aisheng

The flexcan driver still uses it after init.

As "it" is a copy of flexcan_pdata it's completely OK for the driver to use it.

What is the exact problem you want to address?

Sorry, I just checked the code and found platform_device_add_data will Add a copy of platform specific data.
Originally I thought the driver may access the freed pdata after init.

So, does it mean that every pdata of pdevice added by mxs_add_platform_device_dmamask can be prefixed by __init*?

And, below change seems correct?

arch/arm/mach-mxs/module-tx28.c +2 -2
index 0fcff47..a096cca 100644
@@ -66,11 +66,11 @@ static const iomux_cfg_t tx28_fec1_pads[] __initconst = {
        MX28_PAD_ENET0_CRS__ENET1_RX_EN,
 };
 
-static struct fec_platform_data tx28_fec0_data = {
+static struct fec_platform_data __initconst tx28_fec0_data = {
        .phy = PHY_INTERFACE_MODE_RMII,
 };
 
-static struct fec_platform_data tx28_fec1_data = {
+static struct fec_platform_data __initconst tx28_fec1_data = {
        .phy = PHY_INTERFACE_MODE_RMII,
 };

Regards
Dong Aisheng

#3 lore raw

PATCH 1/2 ARM: mx28evk: remove flexcan_pdata __initconst attribute

arch/arm/mach-mxs/module-tx28.c static const iomux_cfg_t tx28_fec1_pads[] __initconst = { · line 69

Quoted 2 lines · On Fri, Nov 11, 2011 at 03:25:01PM +0000, Dong Aisheng-B29396 wrote:
arch/arm/mach-mxs/module-tx28.c +1 -1
-static struct fec_platform_data tx28_fec0_data = {
+static struct fec_platform_data __initconst tx28_fec0_data = {

No. It's spelt:

static const struct fec_platform_data tx28_fec0_data __initconst = {

author
#4 lore raw

PATCH 1/2 ARM: mx28evk: remove flexcan_pdata __initconst attribute

Quoted 10 lines

-----Original Message-----
From: Russell King - ARM Linux [mailto:linux at arm.linux.org.uk]
Sent: Friday, November 11, 2011 11:28 PM
To: Dong Aisheng-B29396
Cc: Uwe Kleine-K?nig; Guo Shawn-R65073; s.hauer at pengutronix.de; kernel at pengutronix.de; linux-arm-kernel at lists.infradead.org; w.sang at pengutronix.de
Subject: Re: [PATCH 1/2] ARM: mx28evk: remove flexcan_pdata __initconst attribute

Quoted 7 lines

arch/arm/mach-mxs/module-tx28.c static const iomux_cfg_t tx28_fec1_pads[] __initconst = { · line 69

Quoted 2 lines · On Fri, Nov 11, 2011 at 03:25:01PM +0000, Dong Aisheng-B29396 wrote:
arch/arm/mach-mxs/module-tx28.c +1 -1
-static struct fec_platform_data tx28_fec0_data = {
+static struct fec_platform_data __initconst tx28_fec0_data = {

No. It's spelt:

static const struct fec_platform_data tx28_fec0_data __initconst = {

Thanks for reminder, I missed the first const. :-)

Regards
Dong Aisheng

#5 lore raw

PATCH 1/2 ARM: mx28evk: remove flexcan_pdata __initconst attribute

Quoted 10 lines · On Fri, Nov 11, 2011 at 03:30:04PM +0000, Dong Aisheng-B29396 wrote:
Quoted 10 lines

-----Original Message-----
From: Russell King - ARM Linux [mailto:linux at arm.linux.org.uk]
Sent: Friday, November 11, 2011 11:28 PM
To: Dong Aisheng-B29396
Cc: Uwe Kleine-K?nig; Guo Shawn-R65073; s.hauer at pengutronix.de; kernel at pengutronix.de; linux-arm-kernel at lists.infradead.org; w.sang at pengutronix.de
Subject: Re: [PATCH 1/2] ARM: mx28evk: remove flexcan_pdata __initconst attribute

Quoted 8 lines
Quoted 7 lines · On Fri, Nov 11, 2011 at 03:25:01PM +0000, Dong Aisheng-B29396 wrote:

arch/arm/mach-mxs/module-tx28.c static const iomux_cfg_t tx28_fec1_pads[] __initconst = { · line 69

Quoted 2 lines
arch/arm/mach-mxs/module-tx28.c +1 -1
-static struct fec_platform_data tx28_fec0_data = {
+static struct fec_platform_data __initconst tx28_fec0_data = {

No. It's spelt:

static const struct fec_platform_data tx28_fec0_data __initconst = {

Thanks for reminder, I missed the first const. :-)

That's not the only thing. Note the placement of the __initconst too.

author
#6 lore raw

PATCH 1/2 ARM: mx28evk: remove flexcan_pdata __initconst attribute

Quoted 27 lines

-----Original Message-----
From: Russell King - ARM Linux [mailto:linux at arm.linux.org.uk]
Sent: Friday, November 11, 2011 11:33 PM
To: Dong Aisheng-B29396
Cc: Uwe Kleine-K?nig; Guo Shawn-R65073; s.hauer at pengutronix.de; kernel at pengutronix.de; linux-arm-kernel at lists.infradead.org; w.sang at pengutronix.de
Subject: Re: [PATCH 1/2] ARM: mx28evk: remove flexcan_pdata __initconst attribute

Quoted 16 lines · On Fri, Nov 11, 2011 at 03:30:04PM +0000, Dong Aisheng-B29396 wrote:
Quoted 16 lines

-----Original Message-----
From: Russell King - ARM Linux [mailto:linux at arm.linux.org.uk]
Sent: Friday, November 11, 2011 11:28 PM
To: Dong Aisheng-B29396
Cc: Uwe Kleine-K?nig; Guo Shawn-R65073; s.hauer at pengutronix.de; kernel at pengutronix.de; linux-arm-kernel at lists.infradead.org; w.sang at pengutronix.de
Subject: Re: [PATCH 1/2] ARM: mx28evk: remove flexcan_pdata __initconst attribute

arch/arm/mach-mxs/module-tx28.c static const iomux_cfg_t tx28_fec1_pads[] __initconst = { · line 69

Quoted 2 lines · On Fri, Nov 11, 2011 at 03:25:01PM +0000, Dong Aisheng-B29396 wrote:
arch/arm/mach-mxs/module-tx28.c +1 -1
-static struct fec_platform_data tx28_fec0_data = {
+static struct fec_platform_data __initconst tx28_fec0_data = {

No. It's spelt:

Quoted 4 lines
Quoted 2 lines
Quoted 1 line

static const struct fec_platform_data tx28_fec0_data __initconst = {

Thanks for reminder, I missed the first const. :-)

That's not the only thing. Note the placement of the __initconst too.

Got it, thanks for your carefully review. :-)

Regards
Dong Aisheng

#7 lore raw

PATCH 1/2 ARM: mx28evk: remove flexcan_pdata __initconst attribute

In reply to Dong Aisheng-B29396
Quoted 31 lines · On Fri, Nov 11, 2011 at 03:25:01PM +0000, Dong Aisheng-B29396 wrote:
Quoted 9 lines

-----Original Message-----
From: Uwe Kleine-K?nig [mailto:u.kleine-koenig at pengutronix.de]
Sent: Friday, November 11, 2011 11:06 PM
To: Dong Aisheng-B29396
Cc: linux-arm-kernel at lists.infradead.org; kernel at pengutronix.de; s.hauer at pengutronix.de; w.sang at pengutronix.de; Guo Shawn-R65073
Subject: Re: [PATCH 1/2] ARM: mx28evk: remove flexcan_pdata __initconst attribute

Quoted 7 lines
Quoted 1 line · Dong Aisheng

The flexcan driver still uses it after init.

As "it" is a copy of flexcan_pdata it's completely OK for the driver to use it.

What is the exact problem you want to address?

Sorry, I just checked the code and found platform_device_add_data will Add a copy of platform specific data.
Originally I thought the driver may access the freed pdata after init.

So, does it mean that every pdata of pdevice added by mxs_add_platform_device_dmamask can be prefixed by __init*?

And, below change seems correct?

arch/arm/mach-mxs/module-tx28.c +0 -0
index 0fcff47..a096cca 100644
@@ -66,11 +66,11 @@ static const iomux_cfg_t tx28_fec1_pads[] __initconst = {
        MX28_PAD_ENET0_CRS__ENET1_RX_EN,
 };

arch/arm/mach-mxs/module-tx28.c static const iomux_cfg_t tx28_fec1_pads[] __initconst = { · line 75

Quoted 10 lines
arch/arm/mach-mxs/module-tx28.c +2 -2
 
-static struct fec_platform_data tx28_fec0_data = {
+static struct fec_platform_data __initconst tx28_fec0_data = {
        .phy = PHY_INTERFACE_MODE_RMII,
 };
 
-static struct fec_platform_data tx28_fec1_data = {
+static struct fec_platform_data __initconst tx28_fec1_data = {
        .phy = PHY_INTERFACE_MODE_RMII,
 };

Apart from Russell's objections, yes.

Best regards
Uwe

Signature · 2 lines
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
author
#8 lore raw

PATCH 2/2 ARM: mxs: remove mmc_platform_data __initdata attribute

In reply to Dong Aisheng

The mmc driver still uses it after init.

Signed-off-by Dong Aisheng <[email protected]>
Cc Sascha Hauer <[email protected]>
Cc Shawn Guo <[email protected]>
Cc Wolfram Sang <[email protected]>
Cc Uwe Kleine-K?nig <[email protected]>
---
 arch/arm/mach-mxs/mach-mx23evk.c       |    2 +-
 arch/arm/mach-mxs/mach-mx28evk.c       |    2 +-
 arch/arm/mach-mxs/mach-stmp378x_devb.c |    2 +-
 arch/arm/mach-mxs/mach-tx28.c          |    2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)
arch/arm/mach-mxs/mach-mx23evk.c +1 -1
index 2c8eb7c..9a0c568 100644
@@ -133,7 +133,7 @@ static const struct mxsfb_platform_data mx23evk_mxsfb_pdata __initconst = {
 	.ld_intf_width	= STMLCDIF_24BIT,
 };
 
-static struct mxs_mmc_platform_data mx23evk_mmc_pdata __initdata = {
+static struct mxs_mmc_platform_data mx23evk_mmc_pdata = {
 	.wp_gpio = MX23EVK_MMC0_WRITE_PROTECT,
 	.flags = SLOTF_8_BIT_CAPABLE,
 };
arch/arm/mach-mxs/mach-mx28evk.c +1 -1
index 226ba64..9a57da9 100644
@@ -361,7 +361,7 @@ static const struct mxsfb_platform_data mx28evk_mxsfb_pdata __initconst = {
 	.ld_intf_width	= STMLCDIF_24BIT,
 };
 
-static struct mxs_mmc_platform_data mx28evk_mmc_pdata[] __initdata = {
+static struct mxs_mmc_platform_data mx28evk_mmc_pdata[] = {
 	{
 		/* mmc0 */
 		.wp_gpio = MX28EVK_MMC0_WRITE_PROTECT,
arch/arm/mach-mxs/mach-stmp378x_devb.c +1 -1
index afec337..aaf06bf 100644
@@ -66,7 +66,7 @@ static const iomux_cfg_t stmp378x_dvb_pads[] __initconst = {
 	MX23_PAD_PWM3__GPIO_1_29 | MXS_PAD_CTRL, /* power enable */
 };
 
-static struct mxs_mmc_platform_data stmp378x_dvb_mmc_pdata __initdata = {
+static struct mxs_mmc_platform_data stmp378x_dvb_mmc_pdata = {
 	.wp_gpio = STMP378X_DEVB_MMC0_WRITE_PROTECT,
 };
arch/arm/mach-mxs/mach-tx28.c +1 -1
index ce5d563..ec8337e 100644
@@ -139,7 +139,7 @@ static struct i2c_board_info tx28_stk5v3_i2c_boardinfo[] __initdata = {
 	},
 };
 
-static struct mxs_mmc_platform_data tx28_mmc0_pdata __initdata = {
+static struct mxs_mmc_platform_data tx28_mmc0_pdata = {
        .wp_gpio = -EINVAL,
        .flags = SLOTF_4_BIT_CAPABLE,
 };
Signature · 1 line
1.7.0.4