diff mbox

[1/2] misc/at24: Add at24c512b eeprom support

Message ID 1358922764-31654-1-git-send-email-Ying.Liu@freescale.com (mailing list archive)
State New, archived
Headers show

Commit Message

Liu Ying Jan. 23, 2013, 6:32 a.m. UTC
This patch adds at24c512b eeprom support.
The datasheet of at24c512b can be found at:
http://www.alldatasheet.com/datasheet-pdf/pdf/
256958/ATMEL/AT24C512B-TH-B.html

Signed-off-by: Liu Ying <Ying.Liu@freescale.com>
---
 Documentation/devicetree/bindings/eeprom.txt |    2 +-
 drivers/misc/eeprom/at24.c                   |    1 +
 2 files changed, 2 insertions(+), 1 deletions(-)

Comments

Linus Walleij Jan. 23, 2013, 12:24 p.m. UTC | #1
On Wed, Jan 23, 2013 at 7:32 AM, Liu Ying <Ying.Liu@freescale.com> wrote:

> This patch adds at24c512b eeprom support.
> The datasheet of at24c512b can be found at:
> http://www.alldatasheet.com/datasheet-pdf/pdf/
> 256958/ATMEL/AT24C512B-TH-B.html
>
> Signed-off-by: Liu Ying <Ying.Liu@freescale.com>

Arnd Bergmann is the misc maintainer, route this by him.

Yours,
Linus Walleij
Wolfram Sang Jan. 23, 2013, 12:50 p.m. UTC | #2
On Wed, Jan 23, 2013 at 01:24:52PM +0100, Linus Walleij wrote:
> On Wed, Jan 23, 2013 at 7:32 AM, Liu Ying <Ying.Liu@freescale.com> wrote:
> 
> > This patch adds at24c512b eeprom support.
> > The datasheet of at24c512b can be found at:
> > http://www.alldatasheet.com/datasheet-pdf/pdf/
> > 256958/ATMEL/AT24C512B-TH-B.html
> >
> > Signed-off-by: Liu Ying <Ying.Liu@freescale.com>
> 
> Arnd Bergmann is the misc maintainer, route this by him.

I usually take at24 patches via my I2C tree.

But not this one, though. The 512b can equally use the 512 entry. The
devicetree should contain both entries, the 512 one as a fallback. (And
the vendor is not "at24"!)

Regards,

   Wolfram
Liu Ying Jan. 23, 2013, 1:40 p.m. UTC | #3
2013/1/23 Wolfram Sang <w.sang@pengutronix.de>:
> On Wed, Jan 23, 2013 at 01:24:52PM +0100, Linus Walleij wrote:
>> On Wed, Jan 23, 2013 at 7:32 AM, Liu Ying <Ying.Liu@freescale.com> wrote:
>>
>> > This patch adds at24c512b eeprom support.
>> > The datasheet of at24c512b can be found at:
>> > http://www.alldatasheet.com/datasheet-pdf/pdf/
>> > 256958/ATMEL/AT24C512B-TH-B.html
>> >
>> > Signed-off-by: Liu Ying <Ying.Liu@freescale.com>
>>
>> Arnd Bergmann is the misc maintainer, route this by him.
>
> I usually take at24 patches via my I2C tree.
>
> But not this one, though. The 512b can equally use the 512 entry. The
> devicetree should contain both entries, the 512 one as a fallback. (And
> the vendor is not "at24"!)

There are some difference between 24c512 and 24c512b about the system
reset procedure, according to the two devices' spec:
24c512b:(a) Create a start bit condition, (b)clock 9 cycles, (c)
create another start bit followed by stop bit condition.
24c512:(a) Clock up to 9 cycles, (b) look for SDA high in each cycle
while SCL is high and then, (c) create a start condition as SDA is
high.
Could this be a reason to add an entry for 24c512b?

About the vendor name, I took the at24c32 node in
arch/arm/boot/dts/imx28-evk.dts as a reference:

                                at24@51 {
                                        compatible = "at24,24c32";
                                        pagesize = <32>;
                                        reg = <0x51>;
                                };

Now, I think the correct vendor name should be "at" or "atmel".

Thanks.

>
> Regards,
>
>    Wolfram
>
> --
> Pengutronix e.K.                           | Wolfram Sang                |
> Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Wolfram Sang Jan. 23, 2013, 1:49 p.m. UTC | #4
Hi,

> There are some difference between 24c512 and 24c512b about the system
> reset procedure, according to the two devices' spec:
> 24c512b:(a) Create a start bit condition, (b)clock 9 cycles, (c)
> create another start bit followed by stop bit condition.
> 24c512:(a) Clock up to 9 cycles, (b) look for SDA high in each cycle
> while SCL is high and then, (c) create a start condition as SDA is
> high.
> Could this be a reason to add an entry for 24c512b?

Since the entries in at24_ids[] are the same, no. If they would differ,
that is a reason.

> Now, I think the correct vendor name should be "at" or "atmel".

"atmel" would be better, but the MX28EVK doesn't even have an I2C eeprom
by default IIRC? But that is unrelated to your patch.
Liu Ying Jan. 23, 2013, 2:08 p.m. UTC | #5
2013/1/23 Wolfram Sang <w.sang@pengutronix.de>:
> Hi,
>
>> There are some difference between 24c512 and 24c512b about the system
>> reset procedure, according to the two devices' spec:
>> 24c512b:(a) Create a start bit condition, (b)clock 9 cycles, (c)
>> create another start bit followed by stop bit condition.
>> 24c512:(a) Clock up to 9 cycles, (b) look for SDA high in each cycle
>> while SCL is high and then, (c) create a start condition as SDA is
>> high.
>> Could this be a reason to add an entry for 24c512b?
>
> Since the entries in at24_ids[] are the same, no. If they would differ,
> that is a reason.

The names of the two entries are not the same, one is "24c512", the
other is "24c512b".
My original idea to add the new entry is to count on the name
difference to do different system reset operations.

>
>> Now, I think the correct vendor name should be "at" or "atmel".
>
> "atmel" would be better, but the MX28EVK doesn't even have an I2C eeprom
> by default IIRC? But that is unrelated to your patch.
The commit message of the patch which adds 24c32 support for MX28EVK
tells that "mx28evk has a free slot U50 that can be used to populate
an I2C EEPROM.".

>
> --
> Pengutronix e.K.                           | Wolfram Sang                |
> Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Wolfram Sang Jan. 23, 2013, 2:14 p.m. UTC | #6
> The names of the two entries are not the same, one is "24c512", the
> other is "24c512b".

I do understand that. There are TONS of 24c512 variants out there.

> My original idea to add the new entry is to count on the name
> difference to do different system reset operations.

Your devicetree may have both names, but at24 doesn't need it since
there is no code involved handling the difference.

> The commit message of the patch which adds 24c32 support for MX28EVK
> tells that "mx28evk has a free slot U50 that can be used to populate
> an I2C EEPROM.".

I do know that, too. You can put other things there, too. But well, not
worth the hazzle...
Liu Ying Jan. 23, 2013, 2:23 p.m. UTC | #7
2013/1/23 Wolfram Sang <w.sang@pengutronix.de>:
>
>> The names of the two entries are not the same, one is "24c512", the
>> other is "24c512b".
>
> I do understand that. There are TONS of 24c512 variants out there.
>
>> My original idea to add the new entry is to count on the name
>> difference to do different system reset operations.
>
> Your devicetree may have both names, but at24 doesn't need it since
> there is no code involved handling the difference.
>
>> The commit message of the patch which adds 24c32 support for MX28EVK
>> tells that "mx28evk has a free slot U50 that can be used to populate
>> an I2C EEPROM.".
>
> I do know that, too. You can put other things there, too. But well, not
> worth the hazzle...

I accept your comments on this patch. Thanks for your review!

>
> --
> Pengutronix e.K.                           | Wolfram Sang                |
> Industrial Linux Solutions                 | http://www.pengutronix.de/  |
>
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.10 (GNU/Linux)
>
> iEYEARECAAYFAlD/8FUACgkQD27XaX1/VRsmGgCeJZCaByj0gw37lr753wxW0npY
> 06oAn0WAmYiUabRNgQ6eAmcO7Ky3f1U4
> =JSKP
> -----END PGP SIGNATURE-----
>
Linus Walleij Jan. 24, 2013, 2:41 p.m. UTC | #8
On Wed, Jan 23, 2013 at 1:50 PM, Wolfram Sang <w.sang@pengutronix.de> wrote:
> On Wed, Jan 23, 2013 at 01:24:52PM +0100, Linus Walleij wrote:
>> On Wed, Jan 23, 2013 at 7:32 AM, Liu Ying <Ying.Liu@freescale.com> wrote:
>>
>> > This patch adds at24c512b eeprom support.
>> > The datasheet of at24c512b can be found at:
>> > http://www.alldatasheet.com/datasheet-pdf/pdf/
>> > 256958/ATMEL/AT24C512B-TH-B.html
>> >
>> > Signed-off-by: Liu Ying <Ying.Liu@freescale.com>
>>
>> Arnd Bergmann is the misc maintainer, route this by him.
>
> I usually take at24 patches via my I2C tree.

Oh I didn't mean he'd merge it, I meant route it by him as
in "let him have a look at it" :-)

Yours,
Linus Walleij
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/eeprom.txt b/Documentation/devicetree/bindings/eeprom.txt
index 4342c10..fcea214 100644
--- a/Documentation/devicetree/bindings/eeprom.txt
+++ b/Documentation/devicetree/bindings/eeprom.txt
@@ -6,7 +6,7 @@  Required properties:
 		 If there is no specific driver for <manufacturer>, a generic
 		 driver based on <type> is selected. Possible types are:
 		 24c00, 24c01, 24c02, 24c04, 24c08, 24c16, 24c32, 24c64,
-		 24c128, 24c256, 24c512, 24c1024, spd
+		 24c128, 24c256, 24c512, 24c512b, 24c1024, spd
 
   - reg : the I2C address of the EEPROM
 
diff --git a/drivers/misc/eeprom/at24.c b/drivers/misc/eeprom/at24.c
index 2baeec5..0df4cb0 100644
--- a/drivers/misc/eeprom/at24.c
+++ b/drivers/misc/eeprom/at24.c
@@ -124,6 +124,7 @@  static const struct i2c_device_id at24_ids[] = {
 	{ "24c128", AT24_DEVICE_MAGIC(131072 / 8, AT24_FLAG_ADDR16) },
 	{ "24c256", AT24_DEVICE_MAGIC(262144 / 8, AT24_FLAG_ADDR16) },
 	{ "24c512", AT24_DEVICE_MAGIC(524288 / 8, AT24_FLAG_ADDR16) },
+	{ "24c512b", AT24_DEVICE_MAGIC(524288 / 8, AT24_FLAG_ADDR16) },
 	{ "24c1024", AT24_DEVICE_MAGIC(1048576 / 8, AT24_FLAG_ADDR16) },
 	{ "at24", 0 },
 	{ /* END OF LIST */ }