Commit 56018056 authored by Andrzej Głąbek's avatar Andrzej Głąbek Committed by Carles Cufi
Browse files

drivers: i2c_nrfx_twim: Add handling of buffers located in flash



TWIM peripherals cannot perform write transactions from buffers
located in flash. The content of such buffers needs to be copied
to RAM before the actual transfer can be requested.
This commits adds a new property (zephyr,flash-buf-max-size) that
informs the driver how much space in RAM needs to be reserved for
such copying and adds proper handling of buffers located in flash.
This fixes an issue that caused that e.g. the DPS310 sensor driver
did not work on nRF SoCs that only have TWIM, not TWI peripherals.

Signed-off-by: default avatarAndrzej Głąbek <andrzej.glabek@nordicsemi.no>
parent 99ce264d
Loading
Loading
Loading
Loading
+73 −44
Original line number Diff line number Diff line
@@ -20,13 +20,14 @@ struct i2c_nrfx_twim_data {
	struct k_sem completion_sync;
	volatile nrfx_err_t res;
	uint32_t dev_config;
	uint16_t concat_buf_size;
	uint8_t *concat_buf;
	uint8_t *msg_buf;
};

struct i2c_nrfx_twim_config {
	nrfx_twim_t twim;
	nrfx_twim_config_t config;
	uint16_t concat_buf_size;
	uint16_t flash_buf_max_size;
};

static inline struct i2c_nrfx_twim_data *get_dev_data(const struct device *dev)
@@ -44,20 +45,22 @@ static int i2c_nrfx_twim_transfer(const struct device *dev,
				  struct i2c_msg *msgs,
				  uint8_t num_msgs, uint16_t addr)
{
	struct i2c_nrfx_twim_data *dev_data = get_dev_data(dev);
	const struct i2c_nrfx_twim_config *dev_config = get_dev_config(dev);
	int ret = 0;
	uint32_t concat_len = 0;
	uint8_t *concat_buf = get_dev_data(dev)->concat_buf;
	uint16_t concat_buf_size = get_dev_data(dev)->concat_buf_size;
	uint8_t *msg_buf = dev_data->msg_buf;
	uint16_t msg_buf_used = 0;
	uint16_t concat_buf_size = dev_config->concat_buf_size;
	nrfx_twim_xfer_desc_t cur_xfer = {
		.address = addr
	};

	k_sem_take(&(get_dev_data(dev)->transfer_sync), K_FOREVER);
	k_sem_take(&dev_data->transfer_sync, K_FOREVER);

	/* Dummy take on completion_sync sem to be sure that it is empty */
	k_sem_take(&(get_dev_data(dev)->completion_sync), K_NO_WAIT);
	k_sem_take(&dev_data->completion_sync, K_NO_WAIT);

	nrfx_twim_enable(&get_dev_config(dev)->twim);
	nrfx_twim_enable(&dev_config->twim);

	for (size_t i = 0; i < num_msgs; i++) {
		if (I2C_MSG_ADDR_10_BITS & msgs[i].flags) {
@@ -81,41 +84,60 @@ static int i2c_nrfx_twim_transfer(const struct device *dev,
		 * already committed to concatenate this message, add it to
		 * the buffer after verifying there's room.
		 */
		if (concat_next || (concat_len != 0)) {
			if ((concat_len + msgs[i].len) > concat_buf_size) {
		if (concat_next || (msg_buf_used != 0)) {
			if ((msg_buf_used + msgs[i].len) > concat_buf_size) {
				LOG_ERR("Need to use concatenation buffer and "
					"provided size is insufficient "
					"(%u + %u > %u). "
					"Adjust the zephyr,concat-buf-size "
					"property in the \"%s\" node.",
					concat_len, msgs[i].len,
					msg_buf_used, msgs[i].len,
					concat_buf_size, dev->name);
				ret = -ENOSPC;
				break;
			}
			if (!(msgs[i].flags & I2C_MSG_READ)) {
				memcpy(concat_buf + concat_len,
				memcpy(msg_buf + msg_buf_used,
				       msgs[i].buf,
				       msgs[i].len);
			}
			concat_len += msgs[i].len;
			msg_buf_used += msgs[i].len;

		/* TWIM peripherals cannot transfer data directly from
		 * flash. If a buffer located in flash is provided for
		 * a write transaction, its content must be copied to
		 * RAM before the transfer can be requested.
		 */
		} else if (!(msgs[i].flags & I2C_MSG_READ) &&
			   !nrfx_is_in_ram(msgs[i].buf)) {
			if (msgs[i].len > dev_config->flash_buf_max_size) {
				LOG_ERR("Cannot copy flash buffer of size: %u. "
					"Adjust the zephyr,flash-buf-max-size "
					"property in the \"%s\" node.",
					msgs[i].len, dev->name);
				ret = -EINVAL;
				break;
			}

			memcpy(msg_buf, msgs[i].buf, msgs[i].len);
			msg_buf_used = msgs[i].len;
		}

		if (concat_next) {
			continue;
		}

		if (concat_len == 0) {
		if (msg_buf_used == 0) {
			cur_xfer.p_primary_buf = msgs[i].buf;
			cur_xfer.primary_length = msgs[i].len;
		} else {
			cur_xfer.p_primary_buf = concat_buf;
			cur_xfer.primary_length = concat_len;
			cur_xfer.p_primary_buf = msg_buf;
			cur_xfer.primary_length = msg_buf_used;
		}
		cur_xfer.type = (msgs[i].flags & I2C_MSG_READ) ?
			NRFX_TWIM_XFER_RX : NRFX_TWIM_XFER_TX;

		nrfx_err_t res = nrfx_twim_xfer(&get_dev_config(dev)->twim,
		nrfx_err_t res = nrfx_twim_xfer(&dev_config->twim,
						&cur_xfer,
						(msgs[i].flags & I2C_MSG_STOP) ?
						 0 : NRFX_TWIM_FLAG_TX_NO_STOP);
@@ -129,7 +151,7 @@ static int i2c_nrfx_twim_transfer(const struct device *dev,
			}
		}

		ret = k_sem_take(&(get_dev_data(dev)->completion_sync),
		ret = k_sem_take(&dev_data->completion_sync,
				 I2C_TRANSFER_TIMEOUT_MSEC);
		if (ret != 0) {
			/* Whatever the frequency, completion_sync should have
@@ -149,14 +171,14 @@ static int i2c_nrfx_twim_transfer(const struct device *dev,
			 * bus from this error.
			 */
			LOG_ERR("Error on I2C line occurred for message %d", i);
			nrfx_twim_disable(&get_dev_config(dev)->twim);
			nrfx_twim_bus_recover(get_dev_config(dev)->config.scl,
					      get_dev_config(dev)->config.sda);
			nrfx_twim_disable(&dev_config->twim);
			nrfx_twim_bus_recover(dev_config->config.scl,
					      dev_config->config.sda);
			ret = -EIO;
			break;
		}

		res = get_dev_data(dev)->res;
		res = dev_data->res;

		if (res != NRFX_SUCCESS) {
			LOG_ERR("Error 0x%08X occurred for message %d", res, i);
@@ -168,24 +190,24 @@ static int i2c_nrfx_twim_transfer(const struct device *dev,
		 * content of concatenation buffer has to be copied back into
		 * buffers provided by user. */
		if ((msgs[i].flags & I2C_MSG_READ)
		    && cur_xfer.p_primary_buf == concat_buf) {
		    && cur_xfer.p_primary_buf == msg_buf) {
			int j = i;

			while (concat_len >= msgs[j].len) {
				concat_len -= msgs[j].len;
			while (msg_buf_used >= msgs[j].len) {
				msg_buf_used -= msgs[j].len;
				memcpy(msgs[j].buf,
				       concat_buf + concat_len,
				       msg_buf + msg_buf_used,
				       msgs[j].len);
				j--;
			}

		}

		concat_len = 0;
		msg_buf_used = 0;
	}

	nrfx_twim_disable(&get_dev_config(dev)->twim);
	k_sem_give(&(get_dev_data(dev)->transfer_sync));
	nrfx_twim_disable(&dev_config->twim);
	k_sem_give(&dev_data->transfer_sync);

	return ret;
}
@@ -296,12 +318,18 @@ static int twim_nrfx_pm_control(const struct device *dev,
#define I2C_FREQUENCY(idx)						       \
	I2C_NRFX_TWIM_FREQUENCY(DT_PROP(I2C(idx), clock_frequency))

#define CONCAT_BUF_SIZE(idx) DT_PROP(I2C(idx), zephyr_concat_buf_size)

#define I2C_CONCAT_BUF(idx)						       \
#define CONCAT_BUF_SIZE(idx)						       \
	COND_CODE_1(DT_NODE_HAS_PROP(I2C(idx), zephyr_concat_buf_size),	       \
	(.concat_buf = twim_##idx##_concat_buf,				       \
	.concat_buf_size = CONCAT_BUF_SIZE(idx),), ())
		    (DT_PROP(I2C(idx), zephyr_concat_buf_size)), (0))
#define FLASH_BUF_MAX_SIZE(idx)						       \
	COND_CODE_1(DT_NODE_HAS_PROP(I2C(idx), zephyr_flash_buf_max_size),     \
		    (DT_PROP(I2C(idx), zephyr_flash_buf_max_size)), (0))

#define USES_MSG_BUF(idx)						       \
	COND_CODE_0(CONCAT_BUF_SIZE(idx),				       \
		(COND_CODE_0(FLASH_BUF_MAX_SIZE(idx), (0), (1))),	       \
		(1))
#define MSG_BUF_SIZE(idx)  MAX(CONCAT_BUF_SIZE(idx), FLASH_BUF_MAX_SIZE(idx))

#define I2C_NRFX_TWIM_DEVICE(idx)					       \
	BUILD_ASSERT(I2C_FREQUENCY(idx) !=				       \
@@ -313,16 +341,15 @@ static int twim_nrfx_pm_control(const struct device *dev,
			    nrfx_isr, nrfx_twim_##idx##_irq_handler, 0);       \
		return init_twim(dev);					       \
	}								       \
	COND_CODE_1(DT_NODE_HAS_PROP(I2C(idx), zephyr_concat_buf_size),	       \
	(static uint8_t twim_##idx##_concat_buf[CONCAT_BUF_SIZE(idx)];),       \
	())								       \
									       \
	IF_ENABLED(USES_MSG_BUF(idx),					       \
		(static uint8_t twim_##idx##_msg_buf[MSG_BUF_SIZE(idx)];))     \
	static struct i2c_nrfx_twim_data twim_##idx##_data = {		       \
		.transfer_sync = Z_SEM_INITIALIZER(			       \
			twim_##idx##_data.transfer_sync, 1, 1),		       \
		.completion_sync = Z_SEM_INITIALIZER(			       \
			twim_##idx##_data.completion_sync, 0, 1),	       \
		I2C_CONCAT_BUF(idx)					       \
		IF_ENABLED(USES_MSG_BUF(idx),				       \
			(.msg_buf = twim_##idx##_msg_buf,))		       \
	};								       \
	static const struct i2c_nrfx_twim_config twim_##idx##z_config = {      \
		.twim = NRFX_TWIM_INSTANCE(idx),			       \
@@ -330,7 +357,9 @@ static int twim_nrfx_pm_control(const struct device *dev,
			.scl       = DT_PROP(I2C(idx), scl_pin),	       \
			.sda       = DT_PROP(I2C(idx), sda_pin),	       \
			.frequency = I2C_FREQUENCY(idx),		       \
		}							       \
		},							       \
		.concat_buf_size = CONCAT_BUF_SIZE(idx),		       \
		.flash_buf_max_size = FLASH_BUF_MAX_SIZE(idx),		       \
	};								       \
	DEVICE_DT_DEFINE(I2C(idx),					       \
		      twim_##idx##_init,				       \
+21 −0
Original line number Diff line number Diff line
@@ -40,3 +40,24 @@ properties:
            the SSD1306 display that cannot tolerate a repeated start and
            address appearing on the bus between message fragments. For many
            devices a concatenation buffer is not necessary.

    zephyr,flash-buf-max-size:
        type: int
        required: false
        default: 16
        description: |
            TWIM peripherals cannot perform write transactions from buffers
            located in flash. If such buffers are expected to be used with
            a given instance of the TWIM peripheral, this property must be
            set to the maximum possible size of those buffers, so that the
            driver can reserve enough space in RAM to copy there the contents
            of particular buffers before requesting the actual transfers.

            If this property is not set to a value adequate for a given
            application, write transactions may fail for buffers that are
            located in flash, what in turn may cause certain components,
            like the DPS310 sensor driver, to not work.

            It is recommended to use the same value for this property and for
            the zephyr,concat-buf-size one, as both these buffering mechanisms
            can utilize the same space in RAM.