From cd34860bd02a5b121b03cdbc9fe9d7731aa56e50 Mon Sep 17 00:00:00 2001 From: Stephan Brunner Date: Sun, 16 Feb 2020 12:47:20 +0100 Subject: [PATCH 01/11] Add Eight-Bit-Adressing mode to I2CEEBlockDevice. When dealing with EEPROMs without a 16 bit adressing, the current implementation does not work, as it writes a 16 bit address to the chip. This may cause undefined behaviour. This change adds a new constructor argument to enable this new eight-bit mode. It defaults to false to not break existing code. This constructor argument should actually never be necessary to manually set, except when dealing with cheap devices. --- .../COMPONENT_I2CEE/I2CEEBlockDevice.cpp | 36 ++++++++++++----- .../COMPONENT_I2CEE/I2CEEBlockDevice.h | 40 ++++++++++++------- 2 files changed, 51 insertions(+), 25 deletions(-) diff --git a/components/storage/blockdevice/COMPONENT_I2CEE/I2CEEBlockDevice.cpp b/components/storage/blockdevice/COMPONENT_I2CEE/I2CEEBlockDevice.cpp index e0c466a1f2f..d46719aa0b5 100644 --- a/components/storage/blockdevice/COMPONENT_I2CEE/I2CEEBlockDevice.cpp +++ b/components/storage/blockdevice/COMPONENT_I2CEE/I2CEEBlockDevice.cpp @@ -22,8 +22,10 @@ using namespace mbed; I2CEEBlockDevice::I2CEEBlockDevice( PinName sda, PinName scl, uint8_t addr, - bd_size_t size, bd_size_t block, int freq) - : _i2c_addr(addr), _size(size), _block(block) + bd_size_t size, bd_size_t block, int freq, + bool address_is_eight_bit) + : _i2c_addr(addr), _size(size), _block(block), + _address_is_eight_bit(address_is_eight_bit) { _i2c = new (_i2c_buffer) I2C(sda, scl); _i2c->frequency(freq); @@ -31,8 +33,10 @@ I2CEEBlockDevice::I2CEEBlockDevice( I2CEEBlockDevice::I2CEEBlockDevice( I2C *i2c_obj, uint8_t addr, - bd_size_t size, bd_size_t block) - : _i2c_addr(addr), _size(size), _block(block) + bd_size_t size, bd_size_t block, + bool address_is_eight_bit) + : _i2c_addr(addr), _size(size), _block(block), + _address_is_eight_bit(address_is_eight_bit) { _i2c = i2c_obj; } @@ -60,9 +64,15 @@ int I2CEEBlockDevice::read(void *buffer, bd_addr_t addr, bd_size_t size) _i2c->start(); - if (!_i2c->write(_i2c_addr | 0) || - !_i2c->write((char)(addr >> 8)) || - !_i2c->write((char)(addr & 0xff))) { + if (!_i2c->write(_i2c_addr | 0)) { + return BD_ERROR_DEVICE_ERROR; + } + + if (!_address_is_eight_bit && !_i2c->write((char)(addr >> 8))) { + return BD_ERROR_DEVICE_ERROR; + } + + if (!_i2c->write((char)(addr & 0xff))) { return BD_ERROR_DEVICE_ERROR; } @@ -92,9 +102,15 @@ int I2CEEBlockDevice::program(const void *buffer, bd_addr_t addr, bd_size_t size _i2c->start(); - if (!_i2c->write(_i2c_addr | 0) || - !_i2c->write((char)(addr >> 8)) || - !_i2c->write((char)(addr & 0xff))) { + if (!_i2c->write(_i2c_addr | 0)) { + return BD_ERROR_DEVICE_ERROR; + } + + if (!_address_is_eight_bit && !_i2c->write((char)(addr >> 8))) { + return BD_ERROR_DEVICE_ERROR; + } + + if (!_i2c->write((char)(addr & 0xff))) { return BD_ERROR_DEVICE_ERROR; } diff --git a/components/storage/blockdevice/COMPONENT_I2CEE/I2CEEBlockDevice.h b/components/storage/blockdevice/COMPONENT_I2CEE/I2CEEBlockDevice.h index 4ba286676fa..b9eed811085 100644 --- a/components/storage/blockdevice/COMPONENT_I2CEE/I2CEEBlockDevice.h +++ b/components/storage/blockdevice/COMPONENT_I2CEE/I2CEEBlockDevice.h @@ -59,29 +59,37 @@ class I2CEEBlockDevice : public BlockDevice { public: /** Constructor to create an I2CEEBlockDevice on I2C pins * - * @param sda The pin name for the sda line of the I2C bus. - * @param scl The pin name for the scl line of the I2C bus. - * @param addr The 8bit I2C address of the chip, common range 0xa0 - 0xae. - * @param size The size of the device in bytes - * @param block The page size of the device in bytes, defaults to 32bytes - * @param freq The frequency of the I2C bus, defaults to 400K. + * @param sda The pin name for the sda line of the I2C bus. + * @param scl The pin name for the scl line of the I2C bus. + * @param addr The 8bit I2C address of the chip, common range 0xa0 - 0xae. + * @param size The size of the device in bytes + * @param block The page size of the device in bytes, defaults to 32bytes + * @param freq The frequency of the I2C bus, defaults to 400K. + * @param address_is_eight_bit Specifies whether the EEPROM device is using eight bit + * addresses instead of 16 bit addresses. This should not be needed + * unless dealing with very cheap devices. */ I2CEEBlockDevice( PinName sda, PinName scl, uint8_t address, bd_size_t size, bd_size_t block = 32, - int bus_speed = 400000); + int bus_speed = 400000, + bool address_is_eight_bit = false); /** Constructor to create an I2CEEBlockDevice on I2C pins - * - * @param i2c The I2C instance pointer - * @param addr The 8bit I2C address of the chip, common range 0xa0 - 0xae. - * @param size The size of the device in bytes - * @param block The page size of the device in bytes, defaults to 32bytes - * @param freq The frequency of the I2C bus, defaults to 400K. - */ + * + * @param i2c The I2C instance pointer + * @param addr The 8bit I2C address of the chip, common range 0xa0 - 0xae. + * @param size The size of the device in bytes + * @param block The page size of the device in bytes, defaults to 32bytes + * @param freq The frequency of the I2C bus, defaults to 400K. + * @param address_is_eight_bit Specifies whether the EEPROM device is using eight bit + * addresses instead of 16 bit addresses. This should not be needed + * unless dealing with very cheap devices. + */ I2CEEBlockDevice( mbed::I2C *i2c_obj, uint8_t address, - bd_size_t size, bd_size_t block = 32); + bd_size_t size, bd_size_t block = 32, + bool address_is_eight_bit = false); /** Destructor of I2CEEBlockDevice */ @@ -169,6 +177,8 @@ class I2CEEBlockDevice : public BlockDevice { uint32_t _size; uint32_t _block; + bool _address_is_eight_bit; + int _sync(); }; From 8d1978e05cef52b1fc3302a156dd987901dcd41a Mon Sep 17 00:00:00 2001 From: Stephan Brunner Date: Mon, 17 Feb 2020 20:22:18 +0100 Subject: [PATCH 02/11] I2CEEBlockDevice: Add paging to eight bit mode --- .../COMPONENT_I2CEE/I2CEEBlockDevice.cpp | 181 +++++++++++++----- .../COMPONENT_I2CEE/I2CEEBlockDevice.h | 35 +++- 2 files changed, 167 insertions(+), 49 deletions(-) diff --git a/components/storage/blockdevice/COMPONENT_I2CEE/I2CEEBlockDevice.cpp b/components/storage/blockdevice/COMPONENT_I2CEE/I2CEEBlockDevice.cpp index d46719aa0b5..148a318b0bb 100644 --- a/components/storage/blockdevice/COMPONENT_I2CEE/I2CEEBlockDevice.cpp +++ b/components/storage/blockdevice/COMPONENT_I2CEE/I2CEEBlockDevice.cpp @@ -62,32 +62,43 @@ int I2CEEBlockDevice::read(void *buffer, bd_addr_t addr, bd_size_t size) // Check the address and size fit onto the chip. MBED_ASSERT(is_valid_read(addr, size)); - _i2c->start(); + auto *charBuffer = reinterpret_cast(buffer); - if (!_i2c->write(_i2c_addr | 0)) { - return BD_ERROR_DEVICE_ERROR; - } + auto const handler = [&](const bd_addr_t &pagedStart, const bd_size_t &pagedLength, const uint8_t &page) -> int + { + _i2c->start(); - if (!_address_is_eight_bit && !_i2c->write((char)(addr >> 8))) { - return BD_ERROR_DEVICE_ERROR; - } + auto const pagedDeviceAddress = get_paged_device_address(page); - if (!_i2c->write((char)(addr & 0xff))) { - return BD_ERROR_DEVICE_ERROR; - } + if (!_i2c->write(pagedDeviceAddress)) { + return BD_ERROR_DEVICE_ERROR; + } - _i2c->stop(); + if (!_address_is_eight_bit && !_i2c->write((char)(pagedStart >> 8u))) { + return BD_ERROR_DEVICE_ERROR; + } - auto err = _sync(); - if (err) { - return err; - } + if (!_i2c->write((char)(pagedStart & 0xffu))) { + return BD_ERROR_DEVICE_ERROR; + } - if (0 != _i2c->read(_i2c_addr, static_cast(buffer), size)) { - return BD_ERROR_DEVICE_ERROR; - } + _i2c->stop(); - return 0; + auto err = _sync(); + if (err) { + return err; + } + + if (0 != _i2c->read(_i2c_addr, charBuffer, pagedLength)) { + return BD_ERROR_DEVICE_ERROR; + } + + charBuffer += size; + + return BD_ERROR_OK; + }; + + return do_paged(addr, size, handler); } int I2CEEBlockDevice::program(const void *buffer, bd_addr_t addr, bd_size_t size) @@ -95,43 +106,52 @@ int I2CEEBlockDevice::program(const void *buffer, bd_addr_t addr, bd_size_t size // Check the addr and size fit onto the chip. MBED_ASSERT(is_valid_program(addr, size)); - // While we have some more data to write. - while (size > 0) { - uint32_t off = addr % _block; - uint32_t chunk = (off + size < _block) ? size : (_block - off); + auto const *charBuffer = reinterpret_cast(buffer); - _i2c->start(); + auto const handler = [&](const bd_addr_t &pagedStart, const bd_size_t &pagedLength, const uint8_t &page) -> int + { + // While we have some more data to write. + while (size > 0) { + uint32_t off = addr % _block; + uint32_t chunk = (off + size < _block) ? size : (_block - off); - if (!_i2c->write(_i2c_addr | 0)) { - return BD_ERROR_DEVICE_ERROR; - } + _i2c->start(); - if (!_address_is_eight_bit && !_i2c->write((char)(addr >> 8))) { - return BD_ERROR_DEVICE_ERROR; - } + auto const pagedDeviceAddress = get_paged_device_address(page); - if (!_i2c->write((char)(addr & 0xff))) { - return BD_ERROR_DEVICE_ERROR; - } + if (!_i2c->write(pagedDeviceAddress)) { + return BD_ERROR_DEVICE_ERROR; + } - for (unsigned i = 0; i < chunk; i++) { - _i2c->write(static_cast(buffer)[i]); - } + if (!_address_is_eight_bit && !_i2c->write((char)(pagedStart >> 8u))) { + return BD_ERROR_DEVICE_ERROR; + } - _i2c->stop(); + if (!_i2c->write((char)(addr & 0xffu))) { + return BD_ERROR_DEVICE_ERROR; + } - int err = _sync(); + for (unsigned i = 0; i < chunk; i++) { + _i2c->write(charBuffer[i]); + } - if (err) { - return err; + _i2c->stop(); + + int err = _sync(); + + if (err) { + return err; + } + + addr += chunk; + size -= chunk; + charBuffer += chunk; } - addr += chunk; - size -= chunk; - buffer = static_cast(buffer) + chunk; - } + return BD_ERROR_OK; + }; - return 0; + return do_paged(addr, size, handler); } int I2CEEBlockDevice::erase(bd_addr_t addr, bd_size_t size) @@ -180,3 +200,74 @@ const char *I2CEEBlockDevice::get_type() const { return "I2CEE"; } + +int I2CEEBlockDevice::do_paged(const bd_addr_t &startAddress, + const bd_size_t &length, + const paged_handler &handler) +{ + // This helper is only used for eight bit mode. + if (!this->_address_is_eight_bit) { + return handler(startAddress, length, 0); + } + + auto currentStartAddress = startAddress; + + auto const pageSize = 256; + bd_size_t lengthDone = 0; + while (lengthDone != length) + { + /* Integer division => Round down */ + uint8_t const currentPage = currentStartAddress / 256; + bd_addr_t const nextPageBegin = (currentPage + 1) * pageSize; + bd_addr_t const currentReadEndAddressExclusive = std::min(nextPageBegin, startAddress + length); + bd_size_t const currentLength = currentReadEndAddressExclusive - currentStartAddress; + bd_addr_t const pagedBegin = currentStartAddress - (currentPage * pageSize); + + auto const handlerReturn = handler(pagedBegin, currentLength, currentPage); + if (handlerReturn != BD_ERROR_OK) + { + return handlerReturn; + } + + currentStartAddress = currentReadEndAddressExclusive; + lengthDone += currentLength; + } + + return BD_ERROR_OK; +} + +uint8_t I2CEEBlockDevice::get_paged_device_address(const uint8_t &page) +{ + if (!this->_address_is_eight_bit) + { + return this->_i2c_addr; + } + else + { + // This method uses a dynamically created bit mask for the page given. + // This ensures compatibility with all sizes of ICs. + // E. g. the 512K variants have two user address bits and one page bit. + // We don't want to forcefully override the two user address bits. + + // Create a mask to cover all bits required to set page + // i starts at one because the LSB is used for R/W in I2C + uint8_t i = 1; + uint8_t addressMask = 0; + auto p = page; + while (p != 0u) + { + addressMask |= (1u << i); + p >>= 1u; + i++; + } + + uint8_t pagedDeviceAddress = this->_i2c_addr & static_cast(~addressMask); + // Assert page < 0b111, because we don't have + // more bits for page encoding + // Don't actually write 0b111, this is a nonstandard extension. + MBED_ASSERT(page < 0x7); + pagedDeviceAddress |= static_cast(page << 1u); + + return pagedDeviceAddress; + } +} diff --git a/components/storage/blockdevice/COMPONENT_I2CEE/I2CEEBlockDevice.h b/components/storage/blockdevice/COMPONENT_I2CEE/I2CEEBlockDevice.h index b9eed811085..217bb49b788 100644 --- a/components/storage/blockdevice/COMPONENT_I2CEE/I2CEEBlockDevice.h +++ b/components/storage/blockdevice/COMPONENT_I2CEE/I2CEEBlockDevice.h @@ -66,8 +66,8 @@ class I2CEEBlockDevice : public BlockDevice { * @param block The page size of the device in bytes, defaults to 32bytes * @param freq The frequency of the I2C bus, defaults to 400K. * @param address_is_eight_bit Specifies whether the EEPROM device is using eight bit - * addresses instead of 16 bit addresses. This should not be needed - * unless dealing with very cheap devices. + * addresses instead of 16 bit addresses. This is used for example + * in AT24C series chips. */ I2CEEBlockDevice( PinName sda, PinName scl, uint8_t address, @@ -83,8 +83,8 @@ class I2CEEBlockDevice : public BlockDevice { * @param block The page size of the device in bytes, defaults to 32bytes * @param freq The frequency of the I2C bus, defaults to 400K. * @param address_is_eight_bit Specifies whether the EEPROM device is using eight bit - * addresses instead of 16 bit addresses. This should not be needed - * unless dealing with very cheap devices. + * addresses instead of 16 bit addresses. This is used for example + * in AT24C series chips. */ I2CEEBlockDevice( mbed::I2C *i2c_obj, uint8_t address, @@ -180,6 +180,33 @@ class I2CEEBlockDevice : public BlockDevice { bool _address_is_eight_bit; int _sync(); + + using paged_handler = std::function; + + /** + * Executes a handler across page boundaries for eight bit mode. + * When eight bit mode is disabled, the function does not do paging at all. + * When eight bit mode is enabled, this function splits the requested + * address space into multiple pages when needed. + * This is required when a read or write must be done across multiple pages. + * + * @param startAddress The address to start + * @param length The requested length of the operation + * @param handler The handler to execute + * @return Returns 0 when all calls to handler() return 0. Otherwise the + * error code from the first non-zero handler() call. + */ + int do_paged(const bd_addr_t &startAddress, const bd_size_t &length, const paged_handler &handler); + + /** + * Gets the device's I2C address with respect to the requested page. + * When eight bit mode is disabled, this function is a noop. + * When eight bit mode is enabled, it sets the bits required for this bit + * in the devices address. Other bits remain unchained. + * @param page The requested page + * @return The device's I2C address for that page + */ + uint8_t get_paged_device_address(const uint8_t &page); }; From 52aed229781cb5adcf03ce7bf46ab40e3143653f Mon Sep 17 00:00:00 2001 From: Stephan Brunner Date: Mon, 17 Feb 2020 20:49:43 +0100 Subject: [PATCH 03/11] Refactor paged_handler to directly give the paged device address. --- .../blockdevice/COMPONENT_I2CEE/I2CEEBlockDevice.cpp | 11 +++++++---- .../blockdevice/COMPONENT_I2CEE/I2CEEBlockDevice.h | 3 ++- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/components/storage/blockdevice/COMPONENT_I2CEE/I2CEEBlockDevice.cpp b/components/storage/blockdevice/COMPONENT_I2CEE/I2CEEBlockDevice.cpp index 148a318b0bb..b68e04567e6 100644 --- a/components/storage/blockdevice/COMPONENT_I2CEE/I2CEEBlockDevice.cpp +++ b/components/storage/blockdevice/COMPONENT_I2CEE/I2CEEBlockDevice.cpp @@ -64,7 +64,8 @@ int I2CEEBlockDevice::read(void *buffer, bd_addr_t addr, bd_size_t size) auto *charBuffer = reinterpret_cast(buffer); - auto const handler = [&](const bd_addr_t &pagedStart, const bd_size_t &pagedLength, const uint8_t &page) -> int + auto const handler = [&](const bd_addr_t &pagedStart, const bd_size_t &pagedLength, + const uint8_t &pagedDeviceAddress) -> int { _i2c->start(); @@ -108,7 +109,8 @@ int I2CEEBlockDevice::program(const void *buffer, bd_addr_t addr, bd_size_t size auto const *charBuffer = reinterpret_cast(buffer); - auto const handler = [&](const bd_addr_t &pagedStart, const bd_size_t &pagedLength, const uint8_t &page) -> int + auto const handler = [&](const bd_addr_t &pagedStart, const bd_size_t &pagedLength, + const uint8_t &pagedDeviceAddress) -> int { // While we have some more data to write. while (size > 0) { @@ -207,7 +209,7 @@ int I2CEEBlockDevice::do_paged(const bd_addr_t &startAddress, { // This helper is only used for eight bit mode. if (!this->_address_is_eight_bit) { - return handler(startAddress, length, 0); + return handler(startAddress, length, get_paged_device_address(0)); } auto currentStartAddress = startAddress; @@ -222,8 +224,9 @@ int I2CEEBlockDevice::do_paged(const bd_addr_t &startAddress, bd_addr_t const currentReadEndAddressExclusive = std::min(nextPageBegin, startAddress + length); bd_size_t const currentLength = currentReadEndAddressExclusive - currentStartAddress; bd_addr_t const pagedBegin = currentStartAddress - (currentPage * pageSize); + uint8_t const pagedDeviceAddress = get_paged_device_address(currentPage); - auto const handlerReturn = handler(pagedBegin, currentLength, currentPage); + auto const handlerReturn = handler(pagedBegin, currentLength, pagedDeviceAddress); if (handlerReturn != BD_ERROR_OK) { return handlerReturn; diff --git a/components/storage/blockdevice/COMPONENT_I2CEE/I2CEEBlockDevice.h b/components/storage/blockdevice/COMPONENT_I2CEE/I2CEEBlockDevice.h index 217bb49b788..a5c02599414 100644 --- a/components/storage/blockdevice/COMPONENT_I2CEE/I2CEEBlockDevice.h +++ b/components/storage/blockdevice/COMPONENT_I2CEE/I2CEEBlockDevice.h @@ -181,7 +181,8 @@ class I2CEEBlockDevice : public BlockDevice { int _sync(); - using paged_handler = std::function; + using paged_handler = std::function< + int(const bd_addr_t &address, const bd_size_t &length, const uint8_t &deviceAddress)>; /** * Executes a handler across page boundaries for eight bit mode. From e850984715d2f02ddcfd400695fb446d5a266f2c Mon Sep 17 00:00:00 2001 From: Stephan Brunner Date: Mon, 17 Feb 2020 20:50:49 +0100 Subject: [PATCH 04/11] Correctly check return codes from bytewise write function of I2C. --- .../COMPONENT_I2CEE/I2CEEBlockDevice.cpp | 33 +++++++++++-------- 1 file changed, 20 insertions(+), 13 deletions(-) diff --git a/components/storage/blockdevice/COMPONENT_I2CEE/I2CEEBlockDevice.cpp b/components/storage/blockdevice/COMPONENT_I2CEE/I2CEEBlockDevice.cpp index b68e04567e6..df3854e7def 100644 --- a/components/storage/blockdevice/COMPONENT_I2CEE/I2CEEBlockDevice.cpp +++ b/components/storage/blockdevice/COMPONENT_I2CEE/I2CEEBlockDevice.cpp @@ -69,17 +69,18 @@ int I2CEEBlockDevice::read(void *buffer, bd_addr_t addr, bd_size_t size) { _i2c->start(); - auto const pagedDeviceAddress = get_paged_device_address(page); - - if (!_i2c->write(pagedDeviceAddress)) { + if (1 != _i2c->write(pagedDeviceAddress)) + { return BD_ERROR_DEVICE_ERROR; } - if (!_address_is_eight_bit && !_i2c->write((char)(pagedStart >> 8u))) { + if (!_address_is_eight_bit && 1 != _i2c->write((char) (pagedStart >> 8u))) + { return BD_ERROR_DEVICE_ERROR; } - if (!_i2c->write((char)(pagedStart & 0xffu))) { + if (1 != _i2c->write((char) (pagedStart & 0xffu))) + { return BD_ERROR_DEVICE_ERROR; } @@ -113,28 +114,34 @@ int I2CEEBlockDevice::program(const void *buffer, bd_addr_t addr, bd_size_t size const uint8_t &pagedDeviceAddress) -> int { // While we have some more data to write. - while (size > 0) { + while (size > 0) + { uint32_t off = addr % _block; uint32_t chunk = (off + size < _block) ? size : (_block - off); _i2c->start(); - auto const pagedDeviceAddress = get_paged_device_address(page); - - if (!_i2c->write(pagedDeviceAddress)) { + if (1 != _i2c->write(pagedDeviceAddress)) + { return BD_ERROR_DEVICE_ERROR; } - if (!_address_is_eight_bit && !_i2c->write((char)(pagedStart >> 8u))) { + if (!_address_is_eight_bit && 1 != _i2c->write((char) (pagedStart >> 8u))) + { return BD_ERROR_DEVICE_ERROR; } - if (!_i2c->write((char)(addr & 0xffu))) { + if (1 != _i2c->write((char) (addr & 0xffu))) + { return BD_ERROR_DEVICE_ERROR; } - for (unsigned i = 0; i < chunk; i++) { - _i2c->write(charBuffer[i]); + for (unsigned i = 0; i < chunk; i++) + { + if (1 != _i2c->write(charBuffer[i])) + { + return BD_ERROR_DEVICE_ERROR; + } } _i2c->stop(); From 267d8cc223b289fdd36f199dc0ff06ef7262913d Mon Sep 17 00:00:00 2001 From: Stephan Brunner Date: Mon, 17 Feb 2020 20:57:36 +0100 Subject: [PATCH 05/11] Preserve original size as it is modified inside the handler function. The value is const reference bound inside do_paged and must not be modified, it is used inside the paging loop. --- .../storage/blockdevice/COMPONENT_I2CEE/I2CEEBlockDevice.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/components/storage/blockdevice/COMPONENT_I2CEE/I2CEEBlockDevice.cpp b/components/storage/blockdevice/COMPONENT_I2CEE/I2CEEBlockDevice.cpp index df3854e7def..e544cd73582 100644 --- a/components/storage/blockdevice/COMPONENT_I2CEE/I2CEEBlockDevice.cpp +++ b/components/storage/blockdevice/COMPONENT_I2CEE/I2CEEBlockDevice.cpp @@ -160,7 +160,8 @@ int I2CEEBlockDevice::program(const void *buffer, bd_addr_t addr, bd_size_t size return BD_ERROR_OK; }; - return do_paged(addr, size, handler); + auto const originalSize = size; + return do_paged(addr, originalSize, handler); } int I2CEEBlockDevice::erase(bd_addr_t addr, bd_size_t size) From 76a177fafa83df972a6a944083a8c7c45961b6b0 Mon Sep 17 00:00:00 2001 From: Stephan Brunner Date: Mon, 17 Feb 2020 21:25:47 +0100 Subject: [PATCH 06/11] Use pageSize-constant instead of hardcoded value --- .../storage/blockdevice/COMPONENT_I2CEE/I2CEEBlockDevice.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/storage/blockdevice/COMPONENT_I2CEE/I2CEEBlockDevice.cpp b/components/storage/blockdevice/COMPONENT_I2CEE/I2CEEBlockDevice.cpp index e544cd73582..4c7def036fe 100644 --- a/components/storage/blockdevice/COMPONENT_I2CEE/I2CEEBlockDevice.cpp +++ b/components/storage/blockdevice/COMPONENT_I2CEE/I2CEEBlockDevice.cpp @@ -227,7 +227,7 @@ int I2CEEBlockDevice::do_paged(const bd_addr_t &startAddress, while (lengthDone != length) { /* Integer division => Round down */ - uint8_t const currentPage = currentStartAddress / 256; + uint8_t const currentPage = currentStartAddress / pageSize; bd_addr_t const nextPageBegin = (currentPage + 1) * pageSize; bd_addr_t const currentReadEndAddressExclusive = std::min(nextPageBegin, startAddress + length); bd_size_t const currentLength = currentReadEndAddressExclusive - currentStartAddress; From 7c589ae17208020b53e3408a85189e73d5fc1dcd Mon Sep 17 00:00:00 2001 From: Stephan Brunner Date: Mon, 17 Feb 2020 22:58:45 +0100 Subject: [PATCH 07/11] Fix astyle --- .../COMPONENT_I2CEE/I2CEEBlockDevice.cpp | 49 +++++++------------ .../COMPONENT_I2CEE/I2CEEBlockDevice.h | 3 +- 2 files changed, 19 insertions(+), 33 deletions(-) diff --git a/components/storage/blockdevice/COMPONENT_I2CEE/I2CEEBlockDevice.cpp b/components/storage/blockdevice/COMPONENT_I2CEE/I2CEEBlockDevice.cpp index 4c7def036fe..511f708dbb2 100644 --- a/components/storage/blockdevice/COMPONENT_I2CEE/I2CEEBlockDevice.cpp +++ b/components/storage/blockdevice/COMPONENT_I2CEE/I2CEEBlockDevice.cpp @@ -64,9 +64,7 @@ int I2CEEBlockDevice::read(void *buffer, bd_addr_t addr, bd_size_t size) auto *charBuffer = reinterpret_cast(buffer); - auto const handler = [&](const bd_addr_t &pagedStart, const bd_size_t &pagedLength, - const uint8_t &pagedDeviceAddress) -> int - { + auto const handler = [&](const bd_addr_t &pagedStart, const bd_size_t &pagedLength, const uint8_t &pagedDeviceAddress) -> int { _i2c->start(); if (1 != _i2c->write(pagedDeviceAddress)) @@ -74,12 +72,12 @@ int I2CEEBlockDevice::read(void *buffer, bd_addr_t addr, bd_size_t size) return BD_ERROR_DEVICE_ERROR; } - if (!_address_is_eight_bit && 1 != _i2c->write((char) (pagedStart >> 8u))) + if (!_address_is_eight_bit && 1 != _i2c->write((char)(pagedStart >> 8u))) { return BD_ERROR_DEVICE_ERROR; } - if (1 != _i2c->write((char) (pagedStart & 0xffu))) + if (1 != _i2c->write((char)(pagedStart & 0xffu))) { return BD_ERROR_DEVICE_ERROR; } @@ -87,11 +85,13 @@ int I2CEEBlockDevice::read(void *buffer, bd_addr_t addr, bd_size_t size) _i2c->stop(); auto err = _sync(); - if (err) { + if (err) + { return err; } - if (0 != _i2c->read(_i2c_addr, charBuffer, pagedLength)) { + if (0 != _i2c->read(_i2c_addr, charBuffer, pagedLength)) + { return BD_ERROR_DEVICE_ERROR; } @@ -110,9 +110,7 @@ int I2CEEBlockDevice::program(const void *buffer, bd_addr_t addr, bd_size_t size auto const *charBuffer = reinterpret_cast(buffer); - auto const handler = [&](const bd_addr_t &pagedStart, const bd_size_t &pagedLength, - const uint8_t &pagedDeviceAddress) -> int - { + auto const handler = [&](const bd_addr_t &pagedStart, const bd_size_t &pagedLength, const uint8_t &pagedDeviceAddress) -> int { // While we have some more data to write. while (size > 0) { @@ -121,25 +119,20 @@ int I2CEEBlockDevice::program(const void *buffer, bd_addr_t addr, bd_size_t size _i2c->start(); - if (1 != _i2c->write(pagedDeviceAddress)) - { + if (1 != _i2c->write(pagedDeviceAddress)) { return BD_ERROR_DEVICE_ERROR; } - if (!_address_is_eight_bit && 1 != _i2c->write((char) (pagedStart >> 8u))) - { + if (!_address_is_eight_bit && 1 != _i2c->write((char)(pagedStart >> 8u))) { return BD_ERROR_DEVICE_ERROR; } - if (1 != _i2c->write((char) (addr & 0xffu))) - { + if (1 != _i2c->write((char)(addr & 0xffu))) { return BD_ERROR_DEVICE_ERROR; } - for (unsigned i = 0; i < chunk; i++) - { - if (1 != _i2c->write(charBuffer[i])) - { + for (unsigned i = 0; i < chunk; i++) { + if (1 != _i2c->write(charBuffer[i])) { return BD_ERROR_DEVICE_ERROR; } } @@ -224,8 +217,7 @@ int I2CEEBlockDevice::do_paged(const bd_addr_t &startAddress, auto const pageSize = 256; bd_size_t lengthDone = 0; - while (lengthDone != length) - { + while (lengthDone != length) { /* Integer division => Round down */ uint8_t const currentPage = currentStartAddress / pageSize; bd_addr_t const nextPageBegin = (currentPage + 1) * pageSize; @@ -235,8 +227,7 @@ int I2CEEBlockDevice::do_paged(const bd_addr_t &startAddress, uint8_t const pagedDeviceAddress = get_paged_device_address(currentPage); auto const handlerReturn = handler(pagedBegin, currentLength, pagedDeviceAddress); - if (handlerReturn != BD_ERROR_OK) - { + if (handlerReturn != BD_ERROR_OK) { return handlerReturn; } @@ -249,12 +240,9 @@ int I2CEEBlockDevice::do_paged(const bd_addr_t &startAddress, uint8_t I2CEEBlockDevice::get_paged_device_address(const uint8_t &page) { - if (!this->_address_is_eight_bit) - { + if (!this->_address_is_eight_bit) { return this->_i2c_addr; - } - else - { + } else { // This method uses a dynamically created bit mask for the page given. // This ensures compatibility with all sizes of ICs. // E. g. the 512K variants have two user address bits and one page bit. @@ -265,8 +253,7 @@ uint8_t I2CEEBlockDevice::get_paged_device_address(const uint8_t &page) uint8_t i = 1; uint8_t addressMask = 0; auto p = page; - while (p != 0u) - { + while (p != 0u) { addressMask |= (1u << i); p >>= 1u; i++; diff --git a/components/storage/blockdevice/COMPONENT_I2CEE/I2CEEBlockDevice.h b/components/storage/blockdevice/COMPONENT_I2CEE/I2CEEBlockDevice.h index a5c02599414..28897e681f9 100644 --- a/components/storage/blockdevice/COMPONENT_I2CEE/I2CEEBlockDevice.h +++ b/components/storage/blockdevice/COMPONENT_I2CEE/I2CEEBlockDevice.h @@ -181,8 +181,7 @@ class I2CEEBlockDevice : public BlockDevice { int _sync(); - using paged_handler = std::function< - int(const bd_addr_t &address, const bd_size_t &length, const uint8_t &deviceAddress)>; + using paged_handler = std::function; /** * Executes a handler across page boundaries for eight bit mode. From 532f3786a0ff61419811b4ccca44652f4ea3c8ee Mon Sep 17 00:00:00 2001 From: Stephan Brunner Date: Tue, 18 Feb 2020 18:40:29 +0100 Subject: [PATCH 08/11] Remove overcomplicated code from I2CEEBlockDevice --- .../COMPONENT_I2CEE/I2CEEBlockDevice.cpp | 160 ++++++------------ .../COMPONENT_I2CEE/I2CEEBlockDevice.h | 21 +-- 2 files changed, 51 insertions(+), 130 deletions(-) diff --git a/components/storage/blockdevice/COMPONENT_I2CEE/I2CEEBlockDevice.cpp b/components/storage/blockdevice/COMPONENT_I2CEE/I2CEEBlockDevice.cpp index 511f708dbb2..4a1eedabf62 100644 --- a/components/storage/blockdevice/COMPONENT_I2CEE/I2CEEBlockDevice.cpp +++ b/components/storage/blockdevice/COMPONENT_I2CEE/I2CEEBlockDevice.cpp @@ -62,45 +62,44 @@ int I2CEEBlockDevice::read(void *buffer, bd_addr_t addr, bd_size_t size) // Check the address and size fit onto the chip. MBED_ASSERT(is_valid_read(addr, size)); - auto *charBuffer = reinterpret_cast(buffer); + auto *pBuffer = reinterpret_cast(buffer); + + while (size > 0) { + uint32_t off = addr % _block; + uint32_t chunk = (off + size < _block) ? size : (_block - off); - auto const handler = [&](const bd_addr_t &pagedStart, const bd_size_t &pagedLength, const uint8_t &pagedDeviceAddress) -> int { _i2c->start(); - if (1 != _i2c->write(pagedDeviceAddress)) - { + if (1 != _i2c->write(get_paged_device_address(addr))) { return BD_ERROR_DEVICE_ERROR; } - if (!_address_is_eight_bit && 1 != _i2c->write((char)(pagedStart >> 8u))) - { + if (!_address_is_eight_bit && 1 != _i2c->write((char)(addr >> 8u))) { return BD_ERROR_DEVICE_ERROR; } - if (1 != _i2c->write((char)(pagedStart & 0xffu))) - { + if (1 != _i2c->write((char)(addr & 0xffu))) { return BD_ERROR_DEVICE_ERROR; } _i2c->stop(); auto err = _sync(); - if (err) - { + + if (err) { return err; } - if (0 != _i2c->read(_i2c_addr, charBuffer, pagedLength)) - { + if (0 != _i2c->read(_i2c_addr, pBuffer, chunk)) { return BD_ERROR_DEVICE_ERROR; } - charBuffer += size; - - return BD_ERROR_OK; - }; + addr += chunk; + size -= chunk; + pBuffer += chunk; + } - return do_paged(addr, size, handler); + return BD_ERROR_OK; } int I2CEEBlockDevice::program(const void *buffer, bd_addr_t addr, bd_size_t size) @@ -108,53 +107,47 @@ int I2CEEBlockDevice::program(const void *buffer, bd_addr_t addr, bd_size_t size // Check the addr and size fit onto the chip. MBED_ASSERT(is_valid_program(addr, size)); - auto const *charBuffer = reinterpret_cast(buffer); + auto const *pBuffer = reinterpret_cast(buffer); - auto const handler = [&](const bd_addr_t &pagedStart, const bd_size_t &pagedLength, const uint8_t &pagedDeviceAddress) -> int { - // While we have some more data to write. - while (size > 0) - { - uint32_t off = addr % _block; - uint32_t chunk = (off + size < _block) ? size : (_block - off); + // While we have some more data to write. + while (size > 0) { + uint32_t off = addr % _block; + uint32_t chunk = (off + size < _block) ? size : (_block - off); - _i2c->start(); + _i2c->start(); - if (1 != _i2c->write(pagedDeviceAddress)) { - return BD_ERROR_DEVICE_ERROR; - } + if (1 != _i2c->write(get_paged_device_address(addr))) { + return BD_ERROR_DEVICE_ERROR; + } - if (!_address_is_eight_bit && 1 != _i2c->write((char)(pagedStart >> 8u))) { - return BD_ERROR_DEVICE_ERROR; - } + if (!_address_is_eight_bit && 1 != _i2c->write((char)(addr >> 8u))) { + return BD_ERROR_DEVICE_ERROR; + } - if (1 != _i2c->write((char)(addr & 0xffu))) { - return BD_ERROR_DEVICE_ERROR; - } + if (1 != _i2c->write((char)(addr & 0xffu))) { + return BD_ERROR_DEVICE_ERROR; + } - for (unsigned i = 0; i < chunk; i++) { - if (1 != _i2c->write(charBuffer[i])) { - return BD_ERROR_DEVICE_ERROR; - } + for (unsigned i = 0; i < chunk; i++) { + if (1 != _i2c->write(pBuffer[i])) { + return BD_ERROR_DEVICE_ERROR; } + } - _i2c->stop(); - - int err = _sync(); + _i2c->stop(); - if (err) { - return err; - } + int err = _sync(); - addr += chunk; - size -= chunk; - charBuffer += chunk; + if (err) { + return err; } - return BD_ERROR_OK; - }; + addr += chunk; + size -= chunk; + pBuffer += chunk; + } - auto const originalSize = size; - return do_paged(addr, originalSize, handler); + return BD_ERROR_OK; } int I2CEEBlockDevice::erase(bd_addr_t addr, bd_size_t size) @@ -204,68 +197,13 @@ const char *I2CEEBlockDevice::get_type() const return "I2CEE"; } -int I2CEEBlockDevice::do_paged(const bd_addr_t &startAddress, - const bd_size_t &length, - const paged_handler &handler) -{ - // This helper is only used for eight bit mode. - if (!this->_address_is_eight_bit) { - return handler(startAddress, length, get_paged_device_address(0)); - } - - auto currentStartAddress = startAddress; - - auto const pageSize = 256; - bd_size_t lengthDone = 0; - while (lengthDone != length) { - /* Integer division => Round down */ - uint8_t const currentPage = currentStartAddress / pageSize; - bd_addr_t const nextPageBegin = (currentPage + 1) * pageSize; - bd_addr_t const currentReadEndAddressExclusive = std::min(nextPageBegin, startAddress + length); - bd_size_t const currentLength = currentReadEndAddressExclusive - currentStartAddress; - bd_addr_t const pagedBegin = currentStartAddress - (currentPage * pageSize); - uint8_t const pagedDeviceAddress = get_paged_device_address(currentPage); - - auto const handlerReturn = handler(pagedBegin, currentLength, pagedDeviceAddress); - if (handlerReturn != BD_ERROR_OK) { - return handlerReturn; - } - - currentStartAddress = currentReadEndAddressExclusive; - lengthDone += currentLength; - } - - return BD_ERROR_OK; -} - -uint8_t I2CEEBlockDevice::get_paged_device_address(const uint8_t &page) +uint8_t I2CEEBlockDevice::get_paged_device_address(const bd_addr_t &address) { if (!this->_address_is_eight_bit) { return this->_i2c_addr; } else { - // This method uses a dynamically created bit mask for the page given. - // This ensures compatibility with all sizes of ICs. - // E. g. the 512K variants have two user address bits and one page bit. - // We don't want to forcefully override the two user address bits. - - // Create a mask to cover all bits required to set page - // i starts at one because the LSB is used for R/W in I2C - uint8_t i = 1; - uint8_t addressMask = 0; - auto p = page; - while (p != 0u) { - addressMask |= (1u << i); - p >>= 1u; - i++; - } - - uint8_t pagedDeviceAddress = this->_i2c_addr & static_cast(~addressMask); - // Assert page < 0b111, because we don't have - // more bits for page encoding - // Don't actually write 0b111, this is a nonstandard extension. - MBED_ASSERT(page < 0x7); - pagedDeviceAddress |= static_cast(page << 1u); - - return pagedDeviceAddress; + // Use the three least significant bits of the 2nd byte as the page + // The page will be bits 2-4 of the user defined addresses. + return this->_i2c_addr | ((address & 0x0700u) >> 7u); } -} +} \ No newline at end of file diff --git a/components/storage/blockdevice/COMPONENT_I2CEE/I2CEEBlockDevice.h b/components/storage/blockdevice/COMPONENT_I2CEE/I2CEEBlockDevice.h index 28897e681f9..9bf97679391 100644 --- a/components/storage/blockdevice/COMPONENT_I2CEE/I2CEEBlockDevice.h +++ b/components/storage/blockdevice/COMPONENT_I2CEE/I2CEEBlockDevice.h @@ -181,32 +181,15 @@ class I2CEEBlockDevice : public BlockDevice { int _sync(); - using paged_handler = std::function; - - /** - * Executes a handler across page boundaries for eight bit mode. - * When eight bit mode is disabled, the function does not do paging at all. - * When eight bit mode is enabled, this function splits the requested - * address space into multiple pages when needed. - * This is required when a read or write must be done across multiple pages. - * - * @param startAddress The address to start - * @param length The requested length of the operation - * @param handler The handler to execute - * @return Returns 0 when all calls to handler() return 0. Otherwise the - * error code from the first non-zero handler() call. - */ - int do_paged(const bd_addr_t &startAddress, const bd_size_t &length, const paged_handler &handler); - /** * Gets the device's I2C address with respect to the requested page. * When eight bit mode is disabled, this function is a noop. * When eight bit mode is enabled, it sets the bits required for this bit * in the devices address. Other bits remain unchained. - * @param page The requested page + * @param address An address in the requested page. * @return The device's I2C address for that page */ - uint8_t get_paged_device_address(const uint8_t &page); + uint8_t get_paged_device_address(const bd_addr_t &address); }; From 3741440b77e09cd293c3abab013dfffcc5781286 Mon Sep 17 00:00:00 2001 From: Stephan Brunner Date: Wed, 19 Feb 2020 11:58:07 +0100 Subject: [PATCH 09/11] Fix typo and clarify documentation --- .../storage/blockdevice/COMPONENT_I2CEE/I2CEEBlockDevice.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/components/storage/blockdevice/COMPONENT_I2CEE/I2CEEBlockDevice.h b/components/storage/blockdevice/COMPONENT_I2CEE/I2CEEBlockDevice.h index 9bf97679391..cf07cc5ef50 100644 --- a/components/storage/blockdevice/COMPONENT_I2CEE/I2CEEBlockDevice.h +++ b/components/storage/blockdevice/COMPONENT_I2CEE/I2CEEBlockDevice.h @@ -184,8 +184,8 @@ class I2CEEBlockDevice : public BlockDevice { /** * Gets the device's I2C address with respect to the requested page. * When eight bit mode is disabled, this function is a noop. - * When eight bit mode is enabled, it sets the bits required for this bit - * in the devices address. Other bits remain unchained. + * When eight bit mode is enabled, it sets the bits required + * in the devices address. Other bits remain unchanged. * @param address An address in the requested page. * @return The device's I2C address for that page */ From 4d0c3463a07b26eb3ebc22ab381f3f1c483a9a7a Mon Sep 17 00:00:00 2001 From: Stephan Brunner Date: Wed, 19 Feb 2020 12:49:28 +0100 Subject: [PATCH 10/11] Fix Code style and reorder class members to get besser packing --- .../COMPONENT_I2CEE/I2CEEBlockDevice.cpp | 15 ++++++++------- .../COMPONENT_I2CEE/I2CEEBlockDevice.h | 9 ++++----- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/components/storage/blockdevice/COMPONENT_I2CEE/I2CEEBlockDevice.cpp b/components/storage/blockdevice/COMPONENT_I2CEE/I2CEEBlockDevice.cpp index 4a1eedabf62..ba0ad229a5b 100644 --- a/components/storage/blockdevice/COMPONENT_I2CEE/I2CEEBlockDevice.cpp +++ b/components/storage/blockdevice/COMPONENT_I2CEE/I2CEEBlockDevice.cpp @@ -62,7 +62,7 @@ int I2CEEBlockDevice::read(void *buffer, bd_addr_t addr, bd_size_t size) // Check the address and size fit onto the chip. MBED_ASSERT(is_valid_read(addr, size)); - auto *pBuffer = reinterpret_cast(buffer); + auto *pBuffer = static_cast(buffer); while (size > 0) { uint32_t off = addr % _block; @@ -107,7 +107,7 @@ int I2CEEBlockDevice::program(const void *buffer, bd_addr_t addr, bd_size_t size // Check the addr and size fit onto the chip. MBED_ASSERT(is_valid_program(addr, size)); - auto const *pBuffer = reinterpret_cast(buffer); + const auto *pBuffer = static_cast(buffer); // While we have some more data to write. while (size > 0) { @@ -197,13 +197,14 @@ const char *I2CEEBlockDevice::get_type() const return "I2CEE"; } -uint8_t I2CEEBlockDevice::get_paged_device_address(const bd_addr_t &address) +uint8_t I2CEEBlockDevice::get_paged_device_address(bd_addr_t address) { - if (!this->_address_is_eight_bit) { - return this->_i2c_addr; + if (!_address_is_eight_bit) { + return _i2c_addr; } else { // Use the three least significant bits of the 2nd byte as the page // The page will be bits 2-4 of the user defined addresses. - return this->_i2c_addr | ((address & 0x0700u) >> 7u); + return _i2c_addr | ((address & 0x0700u) >> 7u); } -} \ No newline at end of file +} + diff --git a/components/storage/blockdevice/COMPONENT_I2CEE/I2CEEBlockDevice.h b/components/storage/blockdevice/COMPONENT_I2CEE/I2CEEBlockDevice.h index cf07cc5ef50..2cf767e8e62 100644 --- a/components/storage/blockdevice/COMPONENT_I2CEE/I2CEEBlockDevice.h +++ b/components/storage/blockdevice/COMPONENT_I2CEE/I2CEEBlockDevice.h @@ -174,22 +174,21 @@ class I2CEEBlockDevice : public BlockDevice { mbed::I2C *_i2c; uint32_t _i2c_buffer[sizeof(mbed::I2C) / sizeof(uint32_t)]; uint8_t _i2c_addr; + bool _address_is_eight_bit; uint32_t _size; uint32_t _block; - bool _address_is_eight_bit; - int _sync(); /** * Gets the device's I2C address with respect to the requested page. - * When eight bit mode is disabled, this function is a noop. - * When eight bit mode is enabled, it sets the bits required + * When eight-bit mode is disabled, this function is a noop. + * When eight-bit mode is enabled, it sets the bits required * in the devices address. Other bits remain unchanged. * @param address An address in the requested page. * @return The device's I2C address for that page */ - uint8_t get_paged_device_address(const bd_addr_t &address); + uint8_t get_paged_device_address(bd_addr_t address); }; From 440fa4985b1bfbaa03dcb008cb16c76eabf4d1e9 Mon Sep 17 00:00:00 2001 From: Stephan Brunner Date: Wed, 19 Feb 2020 18:23:46 +0100 Subject: [PATCH 11/11] Remove paging and sync from I2CEEBlockDevice::read(), as it is not needed --- .../COMPONENT_I2CEE/I2CEEBlockDevice.cpp | 41 ++++++------------- 1 file changed, 13 insertions(+), 28 deletions(-) diff --git a/components/storage/blockdevice/COMPONENT_I2CEE/I2CEEBlockDevice.cpp b/components/storage/blockdevice/COMPONENT_I2CEE/I2CEEBlockDevice.cpp index ba0ad229a5b..aafc048e6fb 100644 --- a/components/storage/blockdevice/COMPONENT_I2CEE/I2CEEBlockDevice.cpp +++ b/components/storage/blockdevice/COMPONENT_I2CEE/I2CEEBlockDevice.cpp @@ -64,39 +64,24 @@ int I2CEEBlockDevice::read(void *buffer, bd_addr_t addr, bd_size_t size) auto *pBuffer = static_cast(buffer); - while (size > 0) { - uint32_t off = addr % _block; - uint32_t chunk = (off + size < _block) ? size : (_block - off); - - _i2c->start(); + _i2c->start(); - if (1 != _i2c->write(get_paged_device_address(addr))) { - return BD_ERROR_DEVICE_ERROR; - } - - if (!_address_is_eight_bit && 1 != _i2c->write((char)(addr >> 8u))) { - return BD_ERROR_DEVICE_ERROR; - } - - if (1 != _i2c->write((char)(addr & 0xffu))) { - return BD_ERROR_DEVICE_ERROR; - } - - _i2c->stop(); + if (1 != _i2c->write(get_paged_device_address(addr))) { + return BD_ERROR_DEVICE_ERROR; + } - auto err = _sync(); + if (!_address_is_eight_bit && 1 != _i2c->write((char)(addr >> 8u))) { + return BD_ERROR_DEVICE_ERROR; + } - if (err) { - return err; - } + if (1 != _i2c->write((char)(addr & 0xffu))) { + return BD_ERROR_DEVICE_ERROR; + } - if (0 != _i2c->read(_i2c_addr, pBuffer, chunk)) { - return BD_ERROR_DEVICE_ERROR; - } + _i2c->stop(); - addr += chunk; - size -= chunk; - pBuffer += chunk; + if (0 != _i2c->read(_i2c_addr, pBuffer, size)) { + return BD_ERROR_DEVICE_ERROR; } return BD_ERROR_OK;