You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2021/01/17 22:32:26 UTC

[GitHub] [incubator-nuttx] v01d commented on a change in pull request #2705: nrf52: Add a static copy buffer for i2c NOSTART transfers

v01d commented on a change in pull request #2705:
URL: https://github.com/apache/incubator-nuttx/pull/2705#discussion_r559248318



##########
File path: arch/arm/src/nrf52/nrf52_i2c.c
##########
@@ -308,11 +313,20 @@ static int nrf52_i2c_transfer(FAR struct i2c_master_s *dev,
 
                   /* Combine buffers */
 
-                  pack_buf = kmm_malloc(priv->msgv[0].length +
-                                        priv->msgv[1].length);
-                  if (pack_buf == NULL)
+                  if ((priv->msgv[0].length +
+                       priv->msgv[1].length) <=
+                      CONFIG_NRF52_I2C_MASTER_COPY_BUF_SIZE)
+                    {
+                      pack_buf = (uint8_t *)priv->copy_buf;
+                    }
+                  else
                     {
-                      return -1;
+                      pack_buf = kmm_malloc(priv->msgv[0].length +
+                                            priv->msgv[1].length);
+                      if (pack_buf == NULL)
+                        {
+                          return -1;

Review comment:
       maybe better -ENOMEM?

##########
File path: arch/arm/src/nrf52/Kconfig
##########
@@ -633,4 +633,16 @@ config NRF52_I2C_MASTER_DISABLE_NOSTART
 	bool "Disable the I2C Master NOSTART flag support"
 	default n
 
+config NRF52_I2C_MASTER_COPY_BUF_SIZE
+	int "Static buffer size for NOSTART flag support"
+	depends on !NRF52_I2C_MASTER_DISABLE_NOSTART
+	default 4
+	---help---
+		To combine two i2c messages that are part of a
+		single transaction (NO_STOP-NO_START) the nrf52
+		hardware requires these be joined into a single
+		transfer. This static buffer will be used if the
+		transaction will fit otherwise it will fall back
+		on malloc.

Review comment:
       Could you move part of this explanation to `NRF52_I2C_MASTER_DISABLE_NOSTART` (to explain why this option exists)?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org