Skip to content

Commit

Permalink
PORT API I2C CHANGE, fixing Zephyr for STM32: uPortI2cControllerExcha…
Browse files Browse the repository at this point in the history 
…nge() replaces uPortI2cControllerSend()/uPortI2cControllerSendReceive(). (#1105)

The port API for I2C had functions uPortI2cControllerSend(), where the stop bit could optionally be omitted, and uPortI2cControllerSendReceive(), for normal communications.  Sending without a stop bit is required to allow the register address within a u-blox GNSS chip to be set, i.e. a write of 0xFD with no stop bit causes the register address in the GNSS chip to be set to 0xFD, and that is the register address from which the number of bytes the GNSS chip has ready for transmission can be read.

However, there is driver software out there (e.g. the STM32 I2C drivers within Zephyr, and potentially Zephyr in general aside from the Nordic I2C drivers) which FORCES a stop bit at the end of each I2C transfer.

In order to be maximally compatible with all platforms, uPortI2cControllerSend()/uPortI2cControllerSendReceive() is now replaced with a single uPortI2cControllerExchange() function, with the option of leaving off the stop bit between the send and the receive.  The uPortI2cControllerSend() and uPortI2cControllerSendReceive() functions are deprecated but remain for now: a weakly-linked implementation of uPortI2cControllerExchange() is provided which calls through to uPortI2cControllerSend()/uPortI2cControllerSendReceive() to ensure backwards compatibility.

For Zephyr, uPortI2cControllerExchange() is implemented such that I2C transactions work with STM32 MCUs as well as nRF52/nRF53.

Our thanks to AarC10/Naquino14 and djfurie for highlighting this issue.

P.S. the number of I2C ports supported for Zephyr is also increased from 2 to 4 to match the capabilities of the STM32 family.
  • Loading branch information
@RobMeades
RobMeades committed Feb 27, 2024
1 parent 3c8d2d9 commit 6d12d9f
Show file tree
Hide file tree
Showing 7 changed files with 269 additions and 56 deletions.
6 changes: 3 additions & 3 deletions gnss/src/u_gnss_msg.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -528,9 +528,9 @@ int32_t uGnssMsgSend(uDeviceHandle_t gnssHandle, const char *pBuffer, size_t siz
}
break;
case U_GNSS_PRIVATE_STREAM_TYPE_I2C: {
errorCodeOrLength = uPortI2cControllerSend(pInstance->transportHandle.i2c,
pInstance->i2cAddress,
pBuffer, size, false);
errorCodeOrLength = uPortI2cControllerExchange(pInstance->transportHandle.i2c,
pInstance->i2cAddress,
pBuffer, size, NULL, 0, false);
if (errorCodeOrLength == 0) {
errorCodeOrLength = (int32_t) size;
}
Expand Down
36 changes: 17 additions & 19 deletions gnss/src/u_gnss_private.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -373,9 +373,10 @@ static int32_t sendMessageStream(uGnssPrivateInstance_t *pInstance,
}
break;
case U_GNSS_PRIVATE_STREAM_TYPE_I2C: {
errorCodeOrSentLength = uPortI2cControllerSend(pInstance->transportHandle.i2c,
pInstance->i2cAddress,
pMessage, messageLengthBytes, false);
errorCodeOrSentLength = uPortI2cControllerExchange(pInstance->transportHandle.i2c,
pInstance->i2cAddress,
pMessage, messageLengthBytes,
NULL, 0, false);
if (errorCodeOrSentLength == 0) {
errorCodeOrSentLength = messageLengthBytes;
}
Expand Down Expand Up @@ -1981,7 +1982,7 @@ int32_t uGnssPrivateStreamGetReceiveSize(uGnssPrivateInstance_t *pInstance)
{
int32_t errorCodeOrReceiveSize = (int32_t) U_ERROR_COMMON_INVALID_PARAMETER;
int32_t privateStreamTypeOrError;
char buffer[2];
char buffer[2] = {0};

if (pInstance != NULL) {
privateStreamTypeOrError = uGnssPrivateGetStreamType(pInstance->transportType);
Expand All @@ -1998,16 +1999,13 @@ int32_t uGnssPrivateStreamGetReceiveSize(uGnssPrivateInstance_t *pInstance)
// 0xFD, with no stop bit, and then a read request for two bytes
// should get us the [big-endian] length
buffer[0] = 0xFD;
errorCodeOrReceiveSize = uPortI2cControllerSend(pInstance->transportHandle.i2c,
(uint16_t) i2cAddress,
buffer, 1, true);
if (errorCodeOrReceiveSize == 0) {
errorCodeOrReceiveSize = uPortI2cControllerSendReceive(pInstance->transportHandle.i2c,
(uint16_t) i2cAddress,
NULL, 0, buffer, sizeof(buffer));
if (errorCodeOrReceiveSize == sizeof(buffer)) {
errorCodeOrReceiveSize = (int32_t) ((((uint32_t) buffer[0]) << 8) + (uint32_t) buffer[1]);
}
errorCodeOrReceiveSize = uPortI2cControllerExchange(pInstance->transportHandle.i2c,
(uint16_t) i2cAddress,
buffer, 1,
buffer, sizeof(buffer),
true);
if (errorCodeOrReceiveSize == sizeof(buffer)) {
errorCodeOrReceiveSize = (int32_t) ((((uint32_t) buffer[0]) << 8) + (uint32_t) buffer[1]);
}
}
break;
Expand Down Expand Up @@ -2186,11 +2184,11 @@ int32_t uGnssPrivateStreamFillRingBuffer(uGnssPrivateInstance_t *pInstance,
// the I2C buffer is effectively on the GNSS chip and I2C drivers
// often don't say how much they've read, just giving us back
// the number we asked for on a successful read
receiveSize = uPortI2cControllerSendReceive(pInstance->transportHandle.i2c,
pInstance->i2cAddress,
NULL, 0,
pTemporaryBuffer,
receiveSize);
receiveSize = uPortI2cControllerExchange(pInstance->transportHandle.i2c,
pInstance->i2cAddress,
NULL, 0,
pTemporaryBuffer,
receiveSize, false);
break;
case U_GNSS_PRIVATE_STREAM_TYPE_SPI:
// For the SPI case, we need to pull the data that was
Expand Down
17 changes: 9 additions & 8 deletions gnss/src/u_gnss_util.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -130,11 +130,11 @@ int32_t uGnssUtilUbxTransparentSendReceive(uDeviceHandle_t gnssHandle,
commandLengthBytes);
break;
case U_GNSS_PRIVATE_STREAM_TYPE_I2C:
errorCodeOrResponseLength = uPortI2cControllerSend(pInstance->transportHandle.i2c,
pInstance->i2cAddress,
pCommand,
commandLengthBytes,
false);
errorCodeOrResponseLength = uPortI2cControllerExchange(pInstance->transportHandle.i2c,
pInstance->i2cAddress,
pCommand,
commandLengthBytes,
NULL, 0, false);
if (errorCodeOrResponseLength == 0) {
errorCodeOrResponseLength = commandLengthBytes;
}
Expand Down Expand Up @@ -197,9 +197,10 @@ int32_t uGnssUtilUbxTransparentSendReceive(uDeviceHandle_t gnssHandle,
pResponse + bytesRead, x);
break;
case U_GNSS_PRIVATE_STREAM_TYPE_I2C:
x = uPortI2cControllerSendReceive(pInstance->transportHandle.i2c,
pInstance->i2cAddress,
NULL, 0, pResponse + bytesRead, x);
x = uPortI2cControllerExchange(pInstance->transportHandle.i2c,
pInstance->i2cAddress,
NULL, 0, pResponse + bytesRead, x,
false);
break;
case U_GNSS_PRIVATE_STREAM_TYPE_SPI:
// For the SPI case, we need to pull the data that was
Expand Down
92 changes: 88 additions & 4 deletions port/api/u_port_i2c.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,85 @@ int32_t uPortI2cSetMaxSegmentSize(int32_t handle, size_t maxSegmentSize);
int32_t uPortI2cGetMaxSegmentSize(int32_t handle);

/** Send and/or receive over the I2C interface as a controller.
* Note that the NRF52 and NRF53 chips require all buffers to
* be in RAM.
*
* Note: where this function is not implemented a weakly-linked
* version will currently do the following:
*
* (a) call uPortI2cControllerSend() if pReceive is NULL or
* noInterveningStop is true,
* (b) call uPortI2cControllerSend() followed by
* uPortI2cControllerSendReceive() if pReceive is non-NULL
* and noInterveningStop is true,
* (c) otherwise call uPortI2cControllerSendReceive().
*
* This weakly-linked function will be removed when the deprecated
* uPortI2cControllerSend()/uPortI2cControllerSendReceive() functions
* are removed.
*
* Note also that the uPortI2cSetTimeout() (or the equivalent set
* by a platform at compile-time) applies for the whole of this
* transaction, i.e. the peripheral must begin responding within
* that time; if you wish to allow the peripheral longer to respond
* you should take control of the time allowed yourself by calling
* uPortI2cControllerExchange() twice, once with only a send buffer
* and again with only a receive buffer.
*
* @param handle the handle of the I2C instance.
* @param address the I2C address to send to; only the lower
* 7 bits are used unless the platform supports
* 10-bit addressing. Note that the NRF5 SDK,
* and hence Zephyr on NRF52/53 (which uses the NRF5
* SDK under the hood) does not support 10-bit
* addressing and, in any case, we've not yet found
* a device that supports 10-bit addressing to test
* against.
* @param[in] pSend a pointer to the data to send, use NULL
* if only receive is required; setting this and
* pReceive to NULL will return success only if a
* device with the given address is present on the I2C
* bus; however note that the NRFX drivers used on nRF52
* and nRF53 by NRF-SDK and Zephyr don't support
* sending only the address, data must follow.
* @param bytesToSend the number of bytes to send, must be zero if pSend
* is NULL.
* @param[out] pReceive a pointer to a buffer in which to store received
* data; use NULL if only send is required.
* @param bytesToReceive the size of buffer pointed to by pReceive, must
* be zero if pReceive is NULL.
* @param noInterveningStop if true then no stop is sent between the send
* and the receive; this is useful for devices such
* as EEPROMs or, in certain situations, u-blox GNSS
* modules, which allow writing of a memory address
* byte or bytes, followed by no stop bit; the data
* from that memory address may then be received
* into pReceive. This is sometimes called using a
* "repeated start bit". Note that if pReceive
* is NULL, depending on the platform, this _may_
* be ignored and a stop bit added in any case;
* e.g. the STM32 drivers within Zephyr will do this;
* they require a stop bit at the end of an I2C
* transaction.
* @return if pReceive is not NULL the number of bytes
* received or negative error code; if pReceive is
* NULL then zero on success else negative error code.
* Note that the underlying platform drivers often
* do not report the number of bytes received and
* hence the return value may just be either an
* error code or bytesToReceive copied back to you.
*/
int32_t uPortI2cControllerExchange(int32_t handle, uint16_t address,
const char *pSend, size_t bytesToSend,
char *pReceive, size_t bytesToReceive,
bool noInterveningStop);

/** \deprecated this function is deprecated, please use
* uPortI2cControllerExchange() instead; if you are making your own
* I2C port, please implement uPortI2cControllerExchange() and
* not this function.
*
* Send and/or receive over the I2C interface as a controller.
* Note that the NRF52 and NRF53 chips require all buffers to
* be in RAM.
*
Expand All @@ -275,7 +354,7 @@ int32_t uPortI2cGetMaxSegmentSize(int32_t handle);
* addressing and, in any case, we've not yet found
* a device that supports 10-bit addressing to test
* against.
* @param pSend a pointer to the data to send, use NULL
* @param[in] pSend a pointer to the data to send, use NULL
* if only receive is required. This function
* will do nothing, and return success, if both
* pSend and pReceive are NULL; if you want to do
Expand All @@ -284,7 +363,7 @@ int32_t uPortI2cGetMaxSegmentSize(int32_t handle);
* though note that not all platforms support this.
* @param bytesToSend the number of bytes to send, must be zero if pSend
* is NULL.
* @param pReceive a pointer to a buffer in which to store received
* @param[out] pReceive a pointer to a buffer in which to store received
* data; use NULL if only send is required.
* @param bytesToReceive the size of buffer pointed to by pReceive, must
* be zero if pReceive is NULL.
Expand All @@ -300,7 +379,12 @@ int32_t uPortI2cControllerSendReceive(int32_t handle, uint16_t address,
const char *pSend, size_t bytesToSend,
char *pReceive, size_t bytesToReceive);

/** Perform just a send over the I2C interface as a controller, with the
/** \deprecated this function is deprecated, please use
* uPortI2cControllerExchange() instead; if you are making your own
* I2C port, please implement uPortI2cControllerExchange() and
* not this function.
*
* Perform just a send over the I2C interface as a controller, with the
* option of omitting the stop marker on the end.
* Note that the NRF52 and NRF53 chips require the buffer to be in RAM.
*
Expand All @@ -313,7 +397,7 @@ int32_t uPortI2cControllerSendReceive(int32_t handle, uint16_t address,
* addressing and, in any case, we've not yet found
* a device that supports 10-bit addressing to test
* against.
* @param pSend a pointer to the data to send; setting this to
* @param[in] pSend a pointer to the data to send; setting this to
* NULL will return success only if a device with
* the given address is present on the I2C bus;
* however note that the NRFX drivers used on nRF52
Expand Down
Loading

0 comments on commit 6d12d9f

Please sign in to comment.