Patchwork powerpc/pcm030.dts: add i2c eeprom and delete cruft

login
register
mail settings
Submitter Wolfram Sang
Date May 20, 2009, 8:07 a.m.
Message ID <1242806865-2334-1-git-send-email-w.sang@pengutronix.de>
Download mbox | patch
Permalink /patch/27451/
State Changes Requested, archived
Delegated to: Grant Likely
Headers show

Comments

Wolfram Sang - May 20, 2009, 8:07 a.m.
Add a node for the i2c eeprom and delete the superflous gpio-example.

Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>
Cc: Grant Likely <grant.likely@secretlab.ca>
Cc: linuxppc-dev@ozlabs.org
---
 arch/powerpc/boot/dts/pcm030.dts |   26 ++++----------------------
 1 files changed, 4 insertions(+), 22 deletions(-)
Jon Smirl - May 20, 2009, 3:17 p.m.
On Wed, May 20, 2009 at 4:07 AM, Wolfram Sang <w.sang@pengutronix.de> wrote:
> Add a node for the i2c eeprom and delete the superflous gpio-example.
>
> Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>
> Cc: Grant Likely <grant.likely@secretlab.ca>
> Cc: linuxppc-dev@ozlabs.org
> ---
>  arch/powerpc/boot/dts/pcm030.dts |   26 ++++----------------------
>  1 files changed, 4 insertions(+), 22 deletions(-)
>
> diff --git a/arch/powerpc/boot/dts/pcm030.dts b/arch/powerpc/boot/dts/pcm030.dts
> index 8958347..33ce488 100644
> --- a/arch/powerpc/boot/dts/pcm030.dts
> +++ b/arch/powerpc/boot/dts/pcm030.dts
> @@ -258,34 +258,16 @@
>                                compatible = "nxp,pcf8563";
>                                reg = <0x51>;
>                        };
> -                       /* FIXME: EEPROM */
> +                       eeprom@52 {
> +                               compatible = "at24,24c32";
> +                               reg = <0x52>;
> +                       };

Grant suggested this earlier...
			eeprom@52 {
				compatible = "atmel,24c32", "eeprom";
				reg = <0x52>;
			};




>                };
>
>                sram@8000 {
>                        compatible = "fsl,mpc5200b-sram","fsl,mpc5200-sram";
>                        reg = <0x8000 0x4000>;
>                };
> -
> -               /* This is only an example device to show the usage of gpios. It maps all available
> -                * gpios to the "gpio-provider" device.
> -                */
> -               gpio {
> -                       compatible = "gpio-provider";
> -
> -                                                   /* mpc52xx          exp.con         patchfield */
> -                       gpios = <&gpio_wkup     0 0 /* GPIO_WKUP_7      11d             jp13-3     */
> -                                &gpio_wkup     1 0 /* GPIO_WKUP_6      14c                        */
> -                                &gpio_wkup     6 0 /* PSC2_4           43c             x5-11      */
> -                                &gpio_simple   2 0 /* IRDA_1           24c             x7-6    set GPS_PORT_CONFIG[IRDA] = 0 */
> -                                &gpio_simple   3 0 /* IRDA_0                           x8-5    set GPS_PORT_CONFIG[IRDA] = 0 */
> -                                &gpt2          0 0 /* timer2           12d             x4-4       */
> -                                &gpt3          0 0 /* timer3           13d             x6-4       */
> -                                &gpt4          0 0 /* timer4           61c             x2-16      */
> -                                &gpt5          0 0 /* timer5           44c             x7-11      */
> -                                &gpt6          0 0 /* timer6           60c             x8-15      */
> -                                &gpt7          0 0 /* timer7           36a             x17-9      */
> -                                >;
> -               };
>        };
>
>        pci@f0000d00 {
> --
> 1.6.2
>
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@ozlabs.org
> https://ozlabs.org/mailman/listinfo/linuxppc-dev
>
Wolfram Sang - May 20, 2009, 3:53 p.m.
> > -                       /* FIXME: EEPROM */
> > +                       eeprom@52 {
> > +                               compatible = "at24,24c32";
> > +                               reg = <0x52>;
> > +                       };
> 
> Grant suggested this earlier...
> 			eeprom@52 {
> 				compatible = "atmel,24c32", "eeprom";
> 				reg = <0x52>;
> 			};

Can you give me a pointer? I just found this thread

http://ozlabs.org/pipermail/devicetree-discuss/2008-July/000008.html

but not the result you proposed.

Regards,

   Wolfram
Jon Smirl - May 20, 2009, 4:10 p.m.
On Wed, May 20, 2009 at 11:53 AM, Wolfram Sang <w.sang@pengutronix.de> wrote:
>> > -                       /* FIXME: EEPROM */
>> > +                       eeprom@52 {
>> > +                               compatible = "at24,24c32";
>> > +                               reg = <0x52>;
>> > +                       };
>>
>> Grant suggested this earlier...
>>                       eeprom@52 {
>>                               compatible = "atmel,24c32", "eeprom";
>>                               reg = <0x52>;
>>                       };
>
> Can you give me a pointer? I just found this thread


Grant, what do you want here?


> http://ozlabs.org/pipermail/devicetree-discuss/2008-July/000008.html
>
> but not the result you proposed.
>
> Regards,
>
>   Wolfram
>
> --
> Pengutronix e.K.                           | Wolfram Sang                |
> Industrial Linux Solutions                 | http://www.pengutronix.de/  |
>
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.9 (GNU/Linux)
>
> iEYEARECAAYFAkoUJ4oACgkQD27XaX1/VRsSbwCgo1o//DG1wjKGR7BY1lkRxOAi
> 8kIAoJghKuhKMNBDXUhA4sWj/vRfDoDV
> =Bmoy
> -----END PGP SIGNATURE-----
>
>
Wolfram Sang - May 20, 2009, 4:15 p.m.
On Wed, May 20, 2009 at 12:10:59PM -0400, Jon Smirl wrote:
> On Wed, May 20, 2009 at 11:53 AM, Wolfram Sang <w.sang@pengutronix.de> wrote:
> >> > -                       /* FIXME: EEPROM */
> >> > +                       eeprom@52 {
> >> > +                               compatible = "at24,24c32";
> >> > +                               reg = <0x52>;
> >> > +                       };
> >>
> >> Grant suggested this earlier...
> >>                       eeprom@52 {
> >>                               compatible = "atmel,24c32", "eeprom";
> >>                               reg = <0x52>;
> >>                       };
> >
> > Can you give me a pointer? I just found this thread
> 
> 
> Grant, what do you want here?

I fear an answer like: "a properly working at24" ;)
Grant Likely - May 20, 2009, 4:25 p.m.
On Wed, May 20, 2009 at 10:15 AM, Wolfram Sang <w.sang@pengutronix.de> wrote:
> On Wed, May 20, 2009 at 12:10:59PM -0400, Jon Smirl wrote:
>> On Wed, May 20, 2009 at 11:53 AM, Wolfram Sang <w.sang@pengutronix.de> wrote:
>> >> > -                       /* FIXME: EEPROM */
>> >> > +                       eeprom@52 {
>> >> > +                               compatible = "at24,24c32";
>> >> > +                               reg = <0x52>;
>> >> > +                       };
>> >>
>> >> Grant suggested this earlier...
>> >>                       eeprom@52 {
>> >>                               compatible = "atmel,24c32", "eeprom";
>> >>                               reg = <0x52>;
>> >>                       };
>> >
>> > Can you give me a pointer? I just found this thread
>>
>>
>> Grant, what do you want here?
>
> I fear an answer like: "a properly working at24" ;)
>

BWAHAHAHAHA!

g.
Grant Likely - May 20, 2009, 4:36 p.m.
On Wed, May 20, 2009 at 10:25 AM, Grant Likely
<grant.likely@secretlab.ca> wrote:
> On Wed, May 20, 2009 at 10:15 AM, Wolfram Sang <w.sang@pengutronix.de> wrote:
>> On Wed, May 20, 2009 at 12:10:59PM -0400, Jon Smirl wrote:
>>> On Wed, May 20, 2009 at 11:53 AM, Wolfram Sang <w.sang@pengutronix.de> wrote:
>>> >> > -                       /* FIXME: EEPROM */
>>> >> > +                       eeprom@52 {
>>> >> > +                               compatible = "at24,24c32";
>>> >> > +                               reg = <0x52>;
>>> >> > +                       };
>>> >>
>>> >> Grant suggested this earlier...
>>> >>                       eeprom@52 {
>>> >>                               compatible = "atmel,24c32", "eeprom";
>>> >>                               reg = <0x52>;
>>> >>                       };
>>> >
>>> > Can you give me a pointer? I just found this thread
>>>
>>>
>>> Grant, what do you want here?
>>
>> I fear an answer like: "a properly working at24" ;)
>>
>
> BWAHAHAHAHA!

Now that I've got that out of the way...

As the other thread states, "eeprom" is far too vague, and it is
certainly not documented, and does not say anything meaningful about
the protocol used to talk to the eeprom.  Sure, most i2c eeproms use
the same protocol, but an assumption cannot be made that that will
always be the case.  Plus, the namespace will collide with non-i2c
eeproms.  "i2c-eeprom" is better, but not great.  Before a value like
"i2c-eeprom" can be acceptable, it must be documented and reviewed as
to exactly what it means, and even then I'm uncomfortable with it.

However, on the other point, Jon is correct.  The first value in the
list should be "atmel,24c32", not "at24,24c32".

Cheers,
g.
Segher Boessenkool - May 21, 2009, 5:43 p.m.
> As the other thread states, "eeprom" is far too vague, and it is
> certainly not documented, and does not say anything meaningful about
> the protocol used to talk to the eeprom.  Sure, most i2c eeproms use
> the same protocol,

Not at all!  Pretty much every size of 24c has its own protocol;
and some manufacturers have special extensions for locking parts
of the array, etc.  A driver can ignore that last part, but not
the first.  So the SEEPROM size should be part of its "compatible"
name; simplest way for that is to use the model number.

> but an assumption cannot be made that that will
> always be the case.  Plus, the namespace will collide with non-i2c
> eeproms.  "i2c-eeprom" is better, but not great.  Before a value like
> "i2c-eeprom" can be acceptable, it must be documented and reviewed as
> to exactly what it means, and even then I'm uncomfortable with it.
>
> However, on the other point, Jon is correct.  The first value in the
> list should be "atmel,24c32", not "at24,24c32".

Yeah.  So perhaps "atmel,24c32","24c32" ?  I'm not terribly happy
with that last name, but these devices are _very_ common.


Segher
Grant Likely - May 21, 2009, 5:58 p.m.
On Thu, May 21, 2009 at 11:43 AM, Segher Boessenkool
<segher@kernel.crashing.org> wrote:
>> However, on the other point, Jon is correct.  The first value in the
>> list should be "atmel,24c32", not "at24,24c32".
>
> Yeah.  So perhaps "atmel,24c32","24c32" ?  I'm not terribly happy
> with that last name, but these devices are _very_ common.

I don't think the last name is necessary at all.  I'd leave it at
"atmel,24c32".  non-atmel parts can claim compatibility with the atmel
version if really necessary.  I don't like the 'generic' version
either.

g.

Patch

diff --git a/arch/powerpc/boot/dts/pcm030.dts b/arch/powerpc/boot/dts/pcm030.dts
index 8958347..33ce488 100644
--- a/arch/powerpc/boot/dts/pcm030.dts
+++ b/arch/powerpc/boot/dts/pcm030.dts
@@ -258,34 +258,16 @@ 
 				compatible = "nxp,pcf8563";
 				reg = <0x51>;
 			};
-			/* FIXME: EEPROM */
+			eeprom@52 {
+				compatible = "at24,24c32";
+				reg = <0x52>;
+			};
 		};
 
 		sram@8000 {
 			compatible = "fsl,mpc5200b-sram","fsl,mpc5200-sram";
 			reg = <0x8000 0x4000>;
 		};
-
-		/* This is only an example device to show the usage of gpios. It maps all available
-		 * gpios to the "gpio-provider" device.
-		 */
-		gpio {
-			compatible = "gpio-provider";
-
-						    /* mpc52xx		exp.con		patchfield */
-			gpios = <&gpio_wkup	0 0 /* GPIO_WKUP_7	11d		jp13-3     */
-				 &gpio_wkup	1 0 /* GPIO_WKUP_6	14c			   */
-				 &gpio_wkup	6 0 /* PSC2_4		43c		x5-11	   */
-				 &gpio_simple	2 0 /* IRDA_1		24c		x7-6	set GPS_PORT_CONFIG[IRDA] = 0 */
-				 &gpio_simple	3 0 /* IRDA_0				x8-5	set GPS_PORT_CONFIG[IRDA] = 0 */
-				 &gpt2		0 0 /* timer2		12d		x4-4	   */
-				 &gpt3		0 0 /* timer3		13d		x6-4	   */
-				 &gpt4		0 0 /* timer4		61c		x2-16	   */
-				 &gpt5		0 0 /* timer5		44c		x7-11	   */
-				 &gpt6		0 0 /* timer6		60c		x8-15	   */
-				 &gpt7		0 0 /* timer7		36a		x17-9	   */
-				 >;
-		};
 	};
 
 	pci@f0000d00 {