Patchwork Drop superfluous setting of i2c_board_info.type

login
register
mail settings
Submitter Paul Bolle
Date May 24, 2012, 2:38 p.m.
Message ID <1337870303.22505.20.camel@x61.thuisdomein>
Download mbox | patch
Permalink /patch/161146/
State New
Headers show

Comments

Paul Bolle - May 24, 2012, 2:38 p.m.
Three instances of struct i2c_board_info have their "type" member set to
"tsc2007" twice. First through the I2C_BOARD_INFO macro and then
directly. Drop the superfluous second setting.

Signed-off-by: Paul Bolle <pebolle@tiscali.nl>
---
0) Entirely untested. I'm unsure what toolchain(s) is (are) needed to
compile this and I am certain that I don't have the hardware.

1) This is apparently legal. Doesn't gcc issue a warning for this?

 arch/arm/mach-imx/mach-cpuimx35.c     |    1 -
 arch/arm/mach-imx/mach-cpuimx51sd.c   |    1 -
 arch/arm/mach-shmobile/board-ap4evb.c |    1 -
 3 files changed, 0 insertions(+), 3 deletions(-)
Paul Mundt - May 24, 2012, 2:59 p.m.
On Thu, May 24, 2012 at 04:38:23PM +0200, Paul Bolle wrote:
> Three instances of struct i2c_board_info have their "type" member set to
> "tsc2007" twice. First through the I2C_BOARD_INFO macro and then
> directly. Drop the superfluous second setting.
> 
> Signed-off-by: Paul Bolle <pebolle@tiscali.nl>
> ---
> 0) Entirely untested. I'm unsure what toolchain(s) is (are) needed to
> compile this and I am certain that I don't have the hardware.
> 
> 1) This is apparently legal. Doesn't gcc issue a warning for this?
> 
That's a new one to me. Seems legal enough. Using this feature you can
use macro initializion for the bulk of structure elements and then just
overload the ones you disagree with to save time, neat yet revolting at
the same time, there was definitely a committee involved in this.
Uwe Kleine-König - May 24, 2012, 4:22 p.m.
On Thu, May 24, 2012 at 04:38:23PM +0200, Paul Bolle wrote:
> Three instances of struct i2c_board_info have their "type" member set to
> "tsc2007" twice. First through the I2C_BOARD_INFO macro and then
> directly. Drop the superfluous second setting.
> 
> Signed-off-by: Paul Bolle <pebolle@tiscali.nl>
> ---
> 0) Entirely untested. I'm unsure what toolchain(s) is (are) needed to
> compile this and I am certain that I don't have the hardware.
> 
> 1) This is apparently legal. Doesn't gcc issue a warning for this?
I wondered about that too, gcc doesn't even warn when the two values are
different. See for example commit
bd9e310dca15c9987256f67af19f9f42426e7493.
> 
>  arch/arm/mach-imx/mach-cpuimx35.c     |    1 -
>  arch/arm/mach-imx/mach-cpuimx51sd.c   |    1 -
I already submitted a patch for the imx bits with Message id
1335989287-10094-1-git-send-email-u.kleine-koenig@pengutronix.de.
(e.g.
http://mid.gmane.org/1335989287-10094-1-git-send-email-u.kleine-koenig@pengutronix.de)

Don't know if Sascha took it already. If he did, he didn't tell me.

Best regards
Uwe
>  arch/arm/mach-shmobile/board-ap4evb.c |    1 -
>  3 files changed, 0 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm/mach-imx/mach-cpuimx35.c b/arch/arm/mach-imx/mach-cpuimx35.c
> index 8ecc872..b1a4796 100644
> --- a/arch/arm/mach-imx/mach-cpuimx35.c
> +++ b/arch/arm/mach-imx/mach-cpuimx35.c
> @@ -70,7 +70,6 @@ static struct i2c_board_info eukrea_cpuimx35_i2c_devices[] = {
>  		I2C_BOARD_INFO("pcf8563", 0x51),
>  	}, {
>  		I2C_BOARD_INFO("tsc2007", 0x48),
> -		.type		= "tsc2007",
>  		.platform_data	= &tsc2007_info,
>  		.irq		= IMX_GPIO_TO_IRQ(TSC2007_IRQGPIO),
>  	},
> diff --git a/arch/arm/mach-imx/mach-cpuimx51sd.c b/arch/arm/mach-imx/mach-cpuimx51sd.c
> index 9fbe923..89eb93c 100644
> --- a/arch/arm/mach-imx/mach-cpuimx51sd.c
> +++ b/arch/arm/mach-imx/mach-cpuimx51sd.c
> @@ -124,7 +124,6 @@ static struct i2c_board_info eukrea_cpuimx51sd_i2c_devices[] = {
>  		I2C_BOARD_INFO("pcf8563", 0x51),
>  	}, {
>  		I2C_BOARD_INFO("tsc2007", 0x49),
> -		.type		= "tsc2007",
>  		.platform_data	= &tsc2007_info,
>  		.irq		= IMX_GPIO_TO_IRQ(TSC2007_IRQGPIO),
>  	},
> diff --git a/arch/arm/mach-shmobile/board-ap4evb.c b/arch/arm/mach-shmobile/board-ap4evb.c
> index b56dde2..801de4b 100644
> --- a/arch/arm/mach-shmobile/board-ap4evb.c
> +++ b/arch/arm/mach-shmobile/board-ap4evb.c
> @@ -1168,7 +1168,6 @@ static struct tsc2007_platform_data tsc2007_info = {
>  
>  static struct i2c_board_info tsc_device = {
>  	I2C_BOARD_INFO("tsc2007", 0x48),
> -	.type		= "tsc2007",
>  	.platform_data	= &tsc2007_info,
>  	/*.irq is selected on ap4evb_init */
>  };
> -- 
> 1.7.7.6
> 
>
Paul Bolle - May 24, 2012, 4:40 p.m.
On Thu, 2012-05-24 at 18:22 +0200, Uwe Kleine-König wrote:
> On Thu, May 24, 2012 at 04:38:23PM +0200, Paul Bolle wrote:
> > 1) This is apparently legal. Doesn't gcc issue a warning for this?
> I wondered about that too, gcc doesn't even warn when the two values are
> different. See for example commit
> bd9e310dca15c9987256f67af19f9f42426e7493.

In that case it were two strings ("rtc-pcf8563" versus "pcf8563"). Why
should gcc care? Or is there something going on behind the scenes which
somehow depends on the contents of the string?
 
> >  arch/arm/mach-imx/mach-cpuimx35.c     |    1 -
> >  arch/arm/mach-imx/mach-cpuimx51sd.c   |    1 -
> I already submitted a patch for the imx bits with Message id
> 1335989287-10094-1-git-send-email-u.kleine-koenig@pengutronix.de.
> (e.g.
> http://mid.gmane.org/1335989287-10094-1-git-send-email-u.kleine-koenig@pengutronix.de)

Thanks. I missed that one, obviously. Should I just drop that part of
the patch and resend?

By the way, together with my previous patch (Message-ID
1337869829.22505.15.camel@x61.thuisdomein , probably not yet archived on
the web) we have found all instances of this issue, haven't we?

> Don't know if Sascha took it already. If he did, he didn't tell me.


Paul Bolle
Sascha Hauer - May 29, 2012, 5:22 p.m.
On Thu, May 24, 2012 at 06:40:26PM +0200, Paul Bolle wrote:
> On Thu, 2012-05-24 at 18:22 +0200, Uwe Kleine-König wrote:
> > On Thu, May 24, 2012 at 04:38:23PM +0200, Paul Bolle wrote:
> > > 1) This is apparently legal. Doesn't gcc issue a warning for this?
> > I wondered about that too, gcc doesn't even warn when the two values are
> > different. See for example commit
> > bd9e310dca15c9987256f67af19f9f42426e7493.
> 
> In that case it were two strings ("rtc-pcf8563" versus "pcf8563"). Why
> should gcc care? Or is there something going on behind the scenes which
> somehow depends on the contents of the string?
>  
> > >  arch/arm/mach-imx/mach-cpuimx35.c     |    1 -
> > >  arch/arm/mach-imx/mach-cpuimx51sd.c   |    1 -
> > I already submitted a patch for the imx bits with Message id
> > 1335989287-10094-1-git-send-email-u.kleine-koenig@pengutronix.de.
> > (e.g.
> > http://mid.gmane.org/1335989287-10094-1-git-send-email-u.kleine-koenig@pengutronix.de)
> 
> Thanks. I missed that one, obviously. Should I just drop that part of
> the patch and resend?

I just merged Uwes patch, this leaves the sh-mobile hunk still to be
fixed.

Sascha

Patch

diff --git a/arch/arm/mach-imx/mach-cpuimx35.c b/arch/arm/mach-imx/mach-cpuimx35.c
index 8ecc872..b1a4796 100644
--- a/arch/arm/mach-imx/mach-cpuimx35.c
+++ b/arch/arm/mach-imx/mach-cpuimx35.c
@@ -70,7 +70,6 @@  static struct i2c_board_info eukrea_cpuimx35_i2c_devices[] = {
 		I2C_BOARD_INFO("pcf8563", 0x51),
 	}, {
 		I2C_BOARD_INFO("tsc2007", 0x48),
-		.type		= "tsc2007",
 		.platform_data	= &tsc2007_info,
 		.irq		= IMX_GPIO_TO_IRQ(TSC2007_IRQGPIO),
 	},
diff --git a/arch/arm/mach-imx/mach-cpuimx51sd.c b/arch/arm/mach-imx/mach-cpuimx51sd.c
index 9fbe923..89eb93c 100644
--- a/arch/arm/mach-imx/mach-cpuimx51sd.c
+++ b/arch/arm/mach-imx/mach-cpuimx51sd.c
@@ -124,7 +124,6 @@  static struct i2c_board_info eukrea_cpuimx51sd_i2c_devices[] = {
 		I2C_BOARD_INFO("pcf8563", 0x51),
 	}, {
 		I2C_BOARD_INFO("tsc2007", 0x49),
-		.type		= "tsc2007",
 		.platform_data	= &tsc2007_info,
 		.irq		= IMX_GPIO_TO_IRQ(TSC2007_IRQGPIO),
 	},
diff --git a/arch/arm/mach-shmobile/board-ap4evb.c b/arch/arm/mach-shmobile/board-ap4evb.c
index b56dde2..801de4b 100644
--- a/arch/arm/mach-shmobile/board-ap4evb.c
+++ b/arch/arm/mach-shmobile/board-ap4evb.c
@@ -1168,7 +1168,6 @@  static struct tsc2007_platform_data tsc2007_info = {
 
 static struct i2c_board_info tsc_device = {
 	I2C_BOARD_INFO("tsc2007", 0x48),
-	.type		= "tsc2007",
 	.platform_data	= &tsc2007_info,
 	/*.irq is selected on ap4evb_init */
 };