Patchwork i2c: i2c-ibm_iic message can be confusing

login
register
mail settings
Submitter Sean MacLennan
Date Feb. 2, 2009, 5:01 p.m.
Message ID <20090202120159.2478d7af@lappy.seanm.ca>
Download mbox | patch
Permalink /patch/21576/
State Accepted, archived
Delegated to: Josh Boyer
Headers show

Comments

Sean MacLennan - Feb. 2, 2009, 5:01 p.m.
This is a trivial patch that does not need to be in 2.6.29. While
tracking down an EEPROM problem, I found the messages confusing... it
looked like the EEPROM was being started before the I2C driver!

Here is an example:

at24 0-0052: 512 byte 24c04 EEPROM (writable)
ibm-iic ef600700.i2c: using standard (100 kHz) mode
ad7414 0-004a: chip found

It looks like the at24 starts first, then the i2c driver, then the
ad7414. By moving the message to after the of scan, we always get the
driver, then the devices.

Cheers,
   Sean

Print the i2c driver message before scanning for devices so that the
logs show the driver, then the devices. Currently you can get
device(s), driver, device(s).

Signed-off-by: Sean MacLennan <smaclennan@pikatech.com>
---
Jean Delvare - Feb. 2, 2009, 5:29 p.m.
On Mon, 2 Feb 2009 12:01:59 -0500, Sean MacLennan wrote:
> This is a trivial patch that does not need to be in 2.6.29. While
> tracking down an EEPROM problem, I found the messages confusing... it
> looked like the EEPROM was being started before the I2C driver!
> 
> Here is an example:
> 
> at24 0-0052: 512 byte 24c04 EEPROM (writable)
> ibm-iic ef600700.i2c: using standard (100 kHz) mode
> ad7414 0-004a: chip found
> 
> It looks like the at24 starts first, then the i2c driver, then the
> ad7414. By moving the message to after the of scan, we always get the
> driver, then the devices.
> 
> Cheers,
>    Sean
> 
> Print the i2c driver message before scanning for devices so that the
> logs show the driver, then the devices. Currently you can get
> device(s), driver, device(s).
> 
> Signed-off-by: Sean MacLennan <smaclennan@pikatech.com>
> ---
> diff --git a/drivers/i2c/busses/i2c-ibm_iic.c b/drivers/i2c/busses/i2c-ibm_iic.c
> index 88f0db7..7fc0729 100644
> --- a/drivers/i2c/busses/i2c-ibm_iic.c
> +++ b/drivers/i2c/busses/i2c-ibm_iic.c
> @@ -756,12 +756,12 @@ static int __devinit iic_probe(struct of_device *ofdev,
>  		goto error_cleanup;
>  	}
>  
> -	/* Now register all the child nodes */
> -	of_register_i2c_devices(adap, np);
> -
>  	dev_info(&ofdev->dev, "using %s mode\n",
>  		 dev->fast_mode ? "fast (400 kHz)" : "standard (100 kHz)");
>  
> +	/* Now register all the child nodes */
> +	of_register_i2c_devices(adap, np);
> +
>  	return 0;
>  
>  error_cleanup:

Indeed.

Acked-by: Jean Delvare <khali@linux-fr.org>
Benjamin Herrenschmidt - Feb. 4, 2009, 3:55 a.m.
> Acked-by: Jean Delvare <khali@linux-fr.org>

Jean, you'll take that in your tree or should I take it in mine ?

Cheers,
Ben.
Jean Delvare - Feb. 4, 2009, 7:36 a.m.
Hi Ben,

On Wed, 04 Feb 2009 14:55:33 +1100, Benjamin Herrenschmidt wrote:
> 
> > Acked-by: Jean Delvare <khali@linux-fr.org>
> 
> Jean, you'll take that in your tree or should I take it in mine ?

No, I'm not taking it, i2c-ibm_iic is under Ben Dooks' jurisdiction. So
it's up to either him or you.
Sean MacLennan - April 18, 2009, 12:36 a.m.
Any update on the status of this patch? This patch was acked by Jean.

The patchwork entry is http://patchwork.ozlabs.org/patch/21576/ and the
original patch message is below.

Cheers,
   Sean

On Mon, 2 Feb 2009 12:01:59 -0500
Sean MacLennan <smaclennan@pikatech.com> wrote:

> This is a trivial patch that does not need to be in 2.6.29. While
> tracking down an EEPROM problem, I found the messages confusing... it
> looked like the EEPROM was being started before the I2C driver!
> 
> Here is an example:
> 
> at24 0-0052: 512 byte 24c04 EEPROM (writable)
> ibm-iic ef600700.i2c: using standard (100 kHz) mode
> ad7414 0-004a: chip found
> 
> It looks like the at24 starts first, then the i2c driver, then the
> ad7414. By moving the message to after the of scan, we always get the
> driver, then the devices.
> 
> Cheers,
>    Sean
> 
> Print the i2c driver message before scanning for devices so that the
> logs show the driver, then the devices. Currently you can get
> device(s), driver, device(s).
> 
> Signed-off-by: Sean MacLennan <smaclennan@pikatech.com>
> ---
> diff --git a/drivers/i2c/busses/i2c-ibm_iic.c
> b/drivers/i2c/busses/i2c-ibm_iic.c index 88f0db7..7fc0729 100644
> --- a/drivers/i2c/busses/i2c-ibm_iic.c
> +++ b/drivers/i2c/busses/i2c-ibm_iic.c
> @@ -756,12 +756,12 @@ static int __devinit iic_probe(struct of_device
> *ofdev, goto error_cleanup;
>  	}
>  
> -	/* Now register all the child nodes */
> -	of_register_i2c_devices(adap, np);
> -
>  	dev_info(&ofdev->dev, "using %s mode\n",
>  		 dev->fast_mode ? "fast (400 kHz)" : "standard (100
> kHz)"); 
> +	/* Now register all the child nodes */
> +	of_register_i2c_devices(adap, np);
> +
>  	return 0;
>  
>  error_cleanup:
>
Josh Boyer - April 24, 2009, 1:10 p.m.
On Fri, Apr 17, 2009 at 08:36:00PM -0400, Sean MacLennan wrote:
>Any update on the status of this patch? This patch was acked by Jean.
>
>The patchwork entry is http://patchwork.ozlabs.org/patch/21576/ and the
>original patch message is below.

Yeah, that's a bit annoying.  A case of too many trees again.

Ben,

Do you want to take this through your tree or should I take it through the
powerpc tree?  I'll be up-front and say silence will translate to 'powerpc tree'

josh

>
>Cheers,
>   Sean
>
>On Mon, 2 Feb 2009 12:01:59 -0500
>Sean MacLennan <smaclennan@pikatech.com> wrote:
>
>> This is a trivial patch that does not need to be in 2.6.29. While
>> tracking down an EEPROM problem, I found the messages confusing... it
>> looked like the EEPROM was being started before the I2C driver!
>> 
>> Here is an example:
>> 
>> at24 0-0052: 512 byte 24c04 EEPROM (writable)
>> ibm-iic ef600700.i2c: using standard (100 kHz) mode
>> ad7414 0-004a: chip found
>> 
>> It looks like the at24 starts first, then the i2c driver, then the
>> ad7414. By moving the message to after the of scan, we always get the
>> driver, then the devices.
>> 
>> Cheers,
>>    Sean
>> 
>> Print the i2c driver message before scanning for devices so that the
>> logs show the driver, then the devices. Currently you can get
>> device(s), driver, device(s).
>> 
>> Signed-off-by: Sean MacLennan <smaclennan@pikatech.com>
>> ---
>> diff --git a/drivers/i2c/busses/i2c-ibm_iic.c
>> b/drivers/i2c/busses/i2c-ibm_iic.c index 88f0db7..7fc0729 100644
>> --- a/drivers/i2c/busses/i2c-ibm_iic.c
>> +++ b/drivers/i2c/busses/i2c-ibm_iic.c
>> @@ -756,12 +756,12 @@ static int __devinit iic_probe(struct of_device
>> *ofdev, goto error_cleanup;
>>  	}
>>  
>> -	/* Now register all the child nodes */
>> -	of_register_i2c_devices(adap, np);
>> -
>>  	dev_info(&ofdev->dev, "using %s mode\n",
>>  		 dev->fast_mode ? "fast (400 kHz)" : "standard (100
>> kHz)"); 
>> +	/* Now register all the child nodes */
>> +	of_register_i2c_devices(adap, np);
>> +
>>  	return 0;
>>  
>>  error_cleanup:
>>  
>_______________________________________________
>Linuxppc-dev mailing list
>Linuxppc-dev@ozlabs.org
>https://ozlabs.org/mailman/listinfo/linuxppc-dev

Patch

diff --git a/drivers/i2c/busses/i2c-ibm_iic.c b/drivers/i2c/busses/i2c-ibm_iic.c
index 88f0db7..7fc0729 100644
--- a/drivers/i2c/busses/i2c-ibm_iic.c
+++ b/drivers/i2c/busses/i2c-ibm_iic.c
@@ -756,12 +756,12 @@  static int __devinit iic_probe(struct of_device *ofdev,
 		goto error_cleanup;
 	}
 
-	/* Now register all the child nodes */
-	of_register_i2c_devices(adap, np);
-
 	dev_info(&ofdev->dev, "using %s mode\n",
 		 dev->fast_mode ? "fast (400 kHz)" : "standard (100 kHz)");
 
+	/* Now register all the child nodes */
+	of_register_i2c_devices(adap, np);
+
 	return 0;
 
 error_cleanup: