eeprom: at2x: Add support for at24cXX by kurddt · Pull Request #29046 · zephyrproject-rtos/zephyr

kurddt · 2020-10-08T09:50:52Z

Those devices can use several I2C address in order to address more
than 256 Bytes using 8bit addressing. Also several physical component
can be used as a contiguous memory.

Alternative to #21170

Signed-off-by: Guillaume Lager guillaume.lager@gmail.com

dts/bindings/mtd/eeprom-spi-i2c.yaml

drivers/eeprom/eeprom_at2x.c

kurddt · 2020-10-21T09:13:51Z

I also made (what I think are) correction to some of the indentation

drivers/eeprom/eeprom_at2x.c

Those devices can use several I2C address in order to address more than 256 Bytes using 8bit addressing. Also several physical component can be used as a contiguous memory. Signed-off-by: Guillaume Lager <guillaume.lager@gmail.com>

KwonTae-young · 2020-10-26T07:57:38Z

I have a board with AT24C04 applied.

&i2c1 {
	clock-frequency = <400000>;
	status = "ok";

	eeprom: eeprom@50 {
		compatible = "atmel,at24";
		reg = <0x50>;
		label = "EEPROM";
		size = <512>;
		pagesize = <16>;
		address-width = <9>;
		timeout = <10>;
	};
};

When compiling, the following error occurs

/work/at24x_test/zephyr/include/toolchain/gcc.h:62:36: error: static assertion failed: "Unsupported address width"
   62 | #define BUILD_ASSERT(EXPR, MSG...) _Static_assert(EXPR, "" MSG)
      |                                    ^~~~~~~~~~~~~~

As of now, only AT24C02 seems to be supported.
Will AT24C04 support be excluded from this PR?

kurddt · 2020-10-26T08:10:01Z

That's expected, the address width is either 8 or 16. Setting the size to 512 is enough to handle the AT24C04 correctly

KwonTae-young · 2020-10-26T09:38:59Z

@kurddt I did a simple test.
There seems to be a bit of a problem in the current PR.
I tested it with the AT24C04.

test source

/*
 * Copyright (c) 2012-2014 Wind River Systems, Inc.
 *
 * SPDX-License-Identifier: Apache-2.0
 */

#include <zephyr.h>
#include <sys/printk.h>
#include <device.h>
#include <drivers/eeprom.h>

#define LOG_LEVEL CONFIG_LOG_DEFAULT_LEVEL
#include <logging/log.h>
LOG_MODULE_REGISTER(test);

void main(void)
{
	const struct device *eeprom;
	size_t eeprom_size;
	const uint8_t wr_buf[17] = {0x00, 0x01, 0x2, 0x03, 0x04, 0x05, 0x06, 0x07,
				0x08, 0x09, 0x0A, 0x0B, 0x0C, 0x0D, 0x0E, 0x0F,
				0x10};
	uint8_t rd_buf[17] = {0x00, };
	int ret;

	/* EEPROM */
	eeprom = device_get_binding(DT_LABEL(DT_ALIAS(eeprom_0)));
	if (!eeprom) {
		LOG_ERR("Cannot get EEPROM device");
		return;
	}

	eeprom_size = eeprom_get_size(eeprom);
	LOG_INF("eeprom_size: %d", eeprom_size);

	/* 8-biyte read/write */
	ret = eeprom_write(eeprom, 0, wr_buf, 8);
	if (ret) {
		LOG_ERR("eeprom_write() failed.");
		return;
	}

	memset(rd_buf, 0, sizeof(rd_buf));
	ret = eeprom_read(eeprom, 0, rd_buf, 8);
	if (ret) {
		LOG_ERR("eeprom_write() failed.");
		return;
	}
	
	if (memcmp(wr_buf, rd_buf, 8)) {
		LOG_ERR("8-byte write, read mismatch.");
		LOG_HEXDUMP_ERR(wr_buf, 8, "wr_buf");
		LOG_HEXDUMP_ERR(rd_buf, 8, "rd_buf");
	}

	/* 12-biyte read/write */
	ret = eeprom_write(eeprom, 0, wr_buf, 12);
	if (ret) {
		LOG_ERR("eeprom_write() failed.");
		return;
	}

	memset(rd_buf, 0, sizeof(rd_buf));
	ret = eeprom_read(eeprom, 0, rd_buf, 12);
	if (ret) {
		LOG_ERR("eeprom_write() failed.");
		return;
	}
	
	if (memcmp(wr_buf, rd_buf, 12)) {
		LOG_ERR("12-byte write, read mismatch.");
		LOG_HEXDUMP_ERR(wr_buf, 12, "wr_buf");
		LOG_HEXDUMP_ERR(rd_buf, 12, "rd_buf");
	}


	/* 16-biyte read/write */
	ret = eeprom_write(eeprom, 0, wr_buf, 16);
	if (ret) {
		LOG_ERR("eeprom_write() failed.");
		return;
	}

	memset(rd_buf, 0, sizeof(rd_buf));
	ret = eeprom_read(eeprom, 0, rd_buf, 16);
	if (ret) {
		LOG_ERR("eeprom_write() failed.");
		return;
	}
	
	if (memcmp(wr_buf, rd_buf, 16)) {
		LOG_ERR("16-byte write, read mismatch.");
		LOG_HEXDUMP_ERR(wr_buf, 16, "wr_buf");
		LOG_HEXDUMP_ERR(rd_buf, 16, "rd_buf");
	}

	/* 17-biyte read/write */
	ret = eeprom_write(eeprom, 0, wr_buf, 17);
	if (ret) {
		LOG_ERR("eeprom_write() failed.");
		return;
	}

	memset(rd_buf, 0, sizeof(rd_buf));
	ret = eeprom_read(eeprom, 0, rd_buf, 17);
	if (ret) {
		LOG_ERR("eeprom_write() failed.");
		return;
	}
	
	if (memcmp(wr_buf, rd_buf, 17)) {
		LOG_ERR("17-byte write, read mismatch.");
		LOG_HEXDUMP_ERR(wr_buf, 17, "wr_buf");
		LOG_HEXDUMP_ERR(rd_buf, 17, "rd_buf");
	}

	LOG_INF("EEPROM Test End!");

	return;
}

1. Current PR(address-width = <16>)

&i2c1 {
	clock-frequency = <400000>;
	status = "ok";

	eeprom: eeprom@50 {
		compatible = "atmel,at24";
		reg = <0x50>;
		label = "EEPROM";
		size = <512>;
		pagesize = <16>;
		address-width = <16>;
		timeout = <10>;
	};
};

console

[00:00:00.005,000] <inf> test: eeprom_size: 512                                                     
[00:00:00.017,000] <err> test: 16-byte write, read mismatch.                                        
[00:00:00.017,000] <err> test: wr_buf                                                               
00 01 02 03 04 05 06 07  08 09 0a 0b 0c 0d 0e 0f |........ ........                                 
[00:00:00.017,000] <err> test: rd_buf                                                               
00 01 02 03 04 05 06 07  08 09 0a 0b 0c 0d 0e 10 |........ ........                                 
[00:00:00.025,000] <err> test: 17-byte write, read mismatch.                                        
[00:00:00.025,000] <err> test: wr_buf                                                               
00 01 02 03 04 05 06 07  08 09 0a 0b 0c 0d 0e 0f |........ ........                                 
10                                               |.                                                 
[00:00:00.025,000] <err> test: rd_buf                                                               
10 01 02 03 04 05 06 07  08 09 0a 0b 0c 0d 0e 10 |........ ........                                 
10                                               |.                                                 
[00:00:00.025,000] <inf> test: EEPROM Test End!

2. Other PR(address-width = <9>)

PR: #21170

&i2c1 {
	clock-frequency = <400000>;
	status = "ok";

	eeprom: eeprom@50 {
		compatible = "atmel,at24";
		reg = <0x50>;
		label = "EEPROM";
		size = <512>;
		pagesize = <16>;
		address-width = <9>;
		timeout = <10>;
	};
};

console

[00:00:00.003,000] <inf> test: eeprom_size: 512                                                     
[00:00:00.023,000] <inf> test: EEPROM Test End!

kurddt · 2020-10-26T10:35:43Z

@KwonTae-young The address-width is meant to be used as the word address width, for AT24C04 it's 8 bit. It needs to be set to 16 for chips like AT24CM01.

Basically 8 means 1 byte address will be sent on the I2C bus after the device addressing. When set to 16, 2 bytes of address are sent

KwonTae-young · 2020-10-27T01:35:48Z

@kurddt I confirmed that it works with the address-width set to 8.

/ {
	aliases {
		eeprom-0 = &eeprom;
	};
};

&i2c1 {
	clock-frequency = <400000>;
	status = "ok";

	eeprom: eeprom@50 {
		compatible = "atmel,at24";
		reg = <0x50>;
		label = "EEPROM";
		size = <512>;
		pagesize = <16>;
		address-width = <8>;
		timeout = <10>;
	};
};

I thought the address-width should be 9-bit as shown in the datasheet.

Some people think it can be confusing.
First of all it works well. 😄

EDIT: I confirmed that Linux kenrel is also handling the same as this PR.
It was my misunderstanding.
https://github.com/torvalds/linux/blob/master/drivers/misc/eeprom/at24.c#L626-L642

kurddt · 2020-10-27T08:45:50Z

Yes I looked at the Linux kernel to get inspiration on how to deal with that.

@KwonTae-young If you have a suggestion to make the address-width description more clear please go ahead

henrikbrixandersen

Looks good. Thanks!

kurddt · 2020-10-28T08:06:59Z

Might be good to remove the "Device Tree" and "Device Tree bindings" tags since they are not relevant anymore. I seems that I can't do that myself

kurddt requested review from galak and henrikbrixandersen as code owners October 8, 2020 09:50

github-actions bot added area: Devicetree area: Devicetree Binding PR modifies or adds a Device Tree binding area: EEPROM labels Oct 8, 2020

kurddt force-pushed the eeprom_at2cXX branch from 0076268 to 4a18095 Compare October 8, 2020 09:59

henrikbrixandersen requested changes Oct 18, 2020

View reviewed changes

dts/bindings/mtd/eeprom-spi-i2c.yaml Outdated Show resolved Hide resolved

drivers/eeprom/eeprom_at2x.c Outdated Show resolved Hide resolved

KwonTae-young suggested changes Oct 21, 2020

View reviewed changes

drivers/eeprom/eeprom_at2x.c Outdated Show resolved Hide resolved

drivers/eeprom/eeprom_at2x.c Outdated Show resolved Hide resolved

kurddt force-pushed the eeprom_at2cXX branch from 4a18095 to 380169b Compare October 21, 2020 09:08

kurddt force-pushed the eeprom_at2cXX branch from 380169b to 903d5b9 Compare October 21, 2020 09:22

henrikbrixandersen self-requested a review October 21, 2020 12:21

henrikbrixandersen reviewed Oct 21, 2020

View reviewed changes

drivers/eeprom/eeprom_at2x.c Outdated Show resolved Hide resolved

eeprom: at2x: Add support for at24cXX

b82ac9a

Those devices can use several I2C address in order to address more than 256 Bytes using 8bit addressing. Also several physical component can be used as a contiguous memory. Signed-off-by: Guillaume Lager <guillaume.lager@gmail.com>

kurddt force-pushed the eeprom_at2cXX branch from 903d5b9 to b82ac9a Compare October 21, 2020 13:07

kurddt requested review from KwonTae-young and henrikbrixandersen October 23, 2020 14:47

KwonTae-young approved these changes Oct 27, 2020

View reviewed changes

henrikbrixandersen approved these changes Oct 27, 2020

View reviewed changes

henrikbrixandersen mentioned this pull request Oct 27, 2020

eeprom_at2x: Support at24c04, at24c08 and at24c16 #21170

Closed

henrikbrixandersen removed area: Devicetree area: Devicetree Binding PR modifies or adds a Device Tree binding labels Oct 28, 2020

carlescufi approved these changes Oct 28, 2020

View reviewed changes

carlescufi merged commit de1b7e4 into zephyrproject-rtos:master Oct 28, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

eeprom: at2x: Add support for at24cXX#29046

eeprom: at2x: Add support for at24cXX#29046
carlescufi merged 1 commit intozephyrproject-rtos:masterfrom
kurddt:eeprom_at2cXX

kurddt commented Oct 8, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kurddt commented Oct 21, 2020

Uh oh!

Uh oh!

KwonTae-young commented Oct 26, 2020

Uh oh!

kurddt commented Oct 26, 2020

Uh oh!

KwonTae-young commented Oct 26, 2020

Uh oh!

kurddt commented Oct 26, 2020

Uh oh!

KwonTae-young commented Oct 27, 2020 •

edited

Loading

Uh oh!

kurddt commented Oct 27, 2020 •

edited

Loading

Uh oh!

henrikbrixandersen left a comment

Uh oh!

kurddt commented Oct 28, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

kurddt commented Oct 8, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kurddt commented Oct 21, 2020

Uh oh!

Uh oh!

KwonTae-young commented Oct 26, 2020

Uh oh!

kurddt commented Oct 26, 2020

Uh oh!

KwonTae-young commented Oct 26, 2020

1. Current PR(address-width = <16>)

2. Other PR(address-width = <9>)

Uh oh!

kurddt commented Oct 26, 2020

Uh oh!

KwonTae-young commented Oct 27, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kurddt commented Oct 27, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

henrikbrixandersen left a comment

Choose a reason for hiding this comment

Uh oh!

kurddt commented Oct 28, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

KwonTae-young commented Oct 27, 2020 •

edited

Loading

kurddt commented Oct 27, 2020 •

edited

Loading