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/11/05 07:45:01 UTC

[GitHub] [incubator-nuttx] yunkya2 opened a new pull request #4786: rp2040: support I2C_RESET

yunkya2 opened a new pull request #4786:
URL: https://github.com/apache/incubator-nuttx/pull/4786


   ## Summary
   Support I2C bus reset for RP2040 to recover from bus stuck conditions.
   (Related discussion: https://github.com/apache/incubator-nuttx/issues/4746)
   
   ## Impact
   RP2040 only (CONFIG_I2C_RESET=y)
   
   ## Testing
   1. Set the following configs to enable the feature:
   ```
   CONFIG_I2C_RESET=y
   CONFIG_RP2040_I2C0=y
   CONFIG_RP2040_I2C0_GPIO=4
   CONFIG_RP2040_I2C=y
   CONFIG_RP2040_I2C_DRIVER=y
   CONFIG_SYSTEM_I2CTOOL=y
   ```
   2. Boot kernel with connecting some I2C device to the bus.
   3. Execute `i2c dev 00 7f` to confirm that the I2C device responds.
   4. Execute `i2c reset`  (Observe I2C bus with the logic analyzer to confirm the recovery sequence.)
   5. Execute `i2c dev 00 7f` again to confirm the device is still alive.
   


-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

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



[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #4786: rp2040: support I2C_RESET

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #4786:
URL: https://github.com/apache/incubator-nuttx/pull/4786#discussion_r743474380



##########
File path: arch/arm/Kconfig
##########
@@ -232,6 +232,7 @@ config ARCH_CHIP_RP2040
 	select ARCH_HAVE_MULTICPU
 	select ARCH_HAVE_TESTSET
 	select ARM_HAVE_WFE_SEV
+	select ARCH_HAVE_I2CRESET

Review comment:
       move before line 234?

##########
File path: arch/arm/Kconfig
##########
@@ -232,6 +232,7 @@ config ARCH_CHIP_RP2040
 	select ARCH_HAVE_MULTICPU
 	select ARCH_HAVE_TESTSET
 	select ARM_HAVE_WFE_SEV
+	select ARCH_HAVE_I2CRESET

Review comment:
       move before line 234?




-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

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



[GitHub] [incubator-nuttx] davids5 commented on a change in pull request #4786: rp2040: support I2C_RESET

Posted by GitBox <gi...@apache.org>.
davids5 commented on a change in pull request #4786:
URL: https://github.com/apache/incubator-nuttx/pull/4786#discussion_r743634081



##########
File path: arch/arm/src/rp2040/rp2040_i2c.c
##########
@@ -639,7 +642,158 @@ static int rp2040_i2c_transfer(FAR struct i2c_master_s *dev,
 #ifdef CONFIG_I2C_RESET
 static int rp2040_i2c_reset(FAR struct i2c_master_s *dev)
 {
-  return OK;
+  FAR struct rp2040_i2cdev_s *priv = (struct rp2040_i2cdev_s *)dev;
+  unsigned int clock_count;
+  unsigned int stretch_count;
+  uint32_t scl_gpio;
+  uint32_t sda_gpio;
+  uint32_t subsys;
+  uint32_t frequency;
+  int ret;
+
+  DEBUGASSERT(dev);
+
+  /* Our caller must own a ref */
+
+  DEBUGASSERT(priv->refs > 0);
+
+  /* Lock out other clients */
+
+  i2c_takesem(&priv->mutex);
+
+  ret = -EIO;
+
+  /* De-init the port */
+
+  rp2040_i2c_disable(priv);
+
+  /* Use GPIO configuration to un-wedge the bus */
+
+  switch (priv->port)
+    {
+      case 0:
+#if defined(CONFIG_RP2040_I2C0) && CONFIG_RP2040_I2C0_GPIO >= 0
+        sda_gpio = CONFIG_RP2040_I2C0_GPIO;

Review comment:
       Ok that makes sense. Yes as it is the GPIO interface seems rather limiting. 




-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

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



[GitHub] [incubator-nuttx] davids5 commented on a change in pull request #4786: rp2040: support I2C_RESET

Posted by GitBox <gi...@apache.org>.
davids5 commented on a change in pull request #4786:
URL: https://github.com/apache/incubator-nuttx/pull/4786#discussion_r743592637



##########
File path: arch/arm/src/rp2040/rp2040_i2c.c
##########
@@ -639,7 +642,158 @@ static int rp2040_i2c_transfer(FAR struct i2c_master_s *dev,
 #ifdef CONFIG_I2C_RESET
 static int rp2040_i2c_reset(FAR struct i2c_master_s *dev)
 {
-  return OK;
+  FAR struct rp2040_i2cdev_s *priv = (struct rp2040_i2cdev_s *)dev;
+  unsigned int clock_count;
+  unsigned int stretch_count;
+  uint32_t scl_gpio;
+  uint32_t sda_gpio;
+  uint32_t subsys;
+  uint32_t frequency;
+  int ret;
+
+  DEBUGASSERT(dev);
+
+  /* Our caller must own a ref */
+
+  DEBUGASSERT(priv->refs > 0);
+
+  /* Lock out other clients */
+
+  i2c_takesem(&priv->mutex);
+
+  ret = -EIO;
+
+  /* De-init the port */
+
+  rp2040_i2c_disable(priv);
+
+  /* Use GPIO configuration to un-wedge the bus */
+
+  switch (priv->port)
+    {
+      case 0:
+#if defined(CONFIG_RP2040_I2C0) && CONFIG_RP2040_I2C0_GPIO >= 0
+        sda_gpio = CONFIG_RP2040_I2C0_GPIO;

Review comment:
       Is something like this https://github.com/apache/incubator-nuttx/blob/master/arch/arm/src/stm32h7/stm32_i2c.c#L2596-L2597 possible this the GPIO IP on the RP240.
   
   It would make this have less config option and code.

##########
File path: arch/arm/src/rp2040/rp2040_i2c.c
##########
@@ -639,7 +642,158 @@ static int rp2040_i2c_transfer(FAR struct i2c_master_s *dev,
 #ifdef CONFIG_I2C_RESET
 static int rp2040_i2c_reset(FAR struct i2c_master_s *dev)
 {
-  return OK;
+  FAR struct rp2040_i2cdev_s *priv = (struct rp2040_i2cdev_s *)dev;
+  unsigned int clock_count;
+  unsigned int stretch_count;
+  uint32_t scl_gpio;
+  uint32_t sda_gpio;
+  uint32_t subsys;
+  uint32_t frequency;
+  int ret;
+
+  DEBUGASSERT(dev);
+
+  /* Our caller must own a ref */
+
+  DEBUGASSERT(priv->refs > 0);
+
+  /* Lock out other clients */
+
+  i2c_takesem(&priv->mutex);
+
+  ret = -EIO;
+
+  /* De-init the port */
+
+  rp2040_i2c_disable(priv);
+
+  /* Use GPIO configuration to un-wedge the bus */
+
+  switch (priv->port)
+    {
+      case 0:
+#if defined(CONFIG_RP2040_I2C0) && CONFIG_RP2040_I2C0_GPIO >= 0
+        sda_gpio = CONFIG_RP2040_I2C0_GPIO;

Review comment:
       Is something like this https://github.com/apache/incubator-nuttx/blob/master/arch/arm/src/stm32h7/stm32_i2c.c#L2596-L2597 possible with the GPIO IP on the RP240.
   
   It would make this have less config option and code.

##########
File path: arch/arm/src/rp2040/rp2040_i2c.c
##########
@@ -639,7 +642,158 @@ static int rp2040_i2c_transfer(FAR struct i2c_master_s *dev,
 #ifdef CONFIG_I2C_RESET
 static int rp2040_i2c_reset(FAR struct i2c_master_s *dev)
 {
-  return OK;
+  FAR struct rp2040_i2cdev_s *priv = (struct rp2040_i2cdev_s *)dev;
+  unsigned int clock_count;
+  unsigned int stretch_count;
+  uint32_t scl_gpio;
+  uint32_t sda_gpio;
+  uint32_t subsys;
+  uint32_t frequency;
+  int ret;
+
+  DEBUGASSERT(dev);
+
+  /* Our caller must own a ref */
+
+  DEBUGASSERT(priv->refs > 0);
+
+  /* Lock out other clients */
+
+  i2c_takesem(&priv->mutex);
+
+  ret = -EIO;
+
+  /* De-init the port */
+
+  rp2040_i2c_disable(priv);
+
+  /* Use GPIO configuration to un-wedge the bus */
+
+  switch (priv->port)
+    {
+      case 0:
+#if defined(CONFIG_RP2040_I2C0) && CONFIG_RP2040_I2C0_GPIO >= 0
+        sda_gpio = CONFIG_RP2040_I2C0_GPIO;

Review comment:
       Ok that makes sense. Yes as it is the GPIO interface seems rather limiting. 




-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

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



[GitHub] [incubator-nuttx] yunkya2 commented on a change in pull request #4786: rp2040: support I2C_RESET

Posted by GitBox <gi...@apache.org>.
yunkya2 commented on a change in pull request #4786:
URL: https://github.com/apache/incubator-nuttx/pull/4786#discussion_r743631676



##########
File path: arch/arm/src/rp2040/rp2040_i2c.c
##########
@@ -639,7 +642,158 @@ static int rp2040_i2c_transfer(FAR struct i2c_master_s *dev,
 #ifdef CONFIG_I2C_RESET
 static int rp2040_i2c_reset(FAR struct i2c_master_s *dev)
 {
-  return OK;
+  FAR struct rp2040_i2cdev_s *priv = (struct rp2040_i2cdev_s *)dev;
+  unsigned int clock_count;
+  unsigned int stretch_count;
+  uint32_t scl_gpio;
+  uint32_t sda_gpio;
+  uint32_t subsys;
+  uint32_t frequency;
+  int ret;
+
+  DEBUGASSERT(dev);
+
+  /* Our caller must own a ref */
+
+  DEBUGASSERT(priv->refs > 0);
+
+  /* Lock out other clients */
+
+  i2c_takesem(&priv->mutex);
+
+  ret = -EIO;
+
+  /* De-init the port */
+
+  rp2040_i2c_disable(priv);
+
+  /* Use GPIO configuration to un-wedge the bus */
+
+  switch (priv->port)
+    {
+      case 0:
+#if defined(CONFIG_RP2040_I2C0) && CONFIG_RP2040_I2C0_GPIO >= 0
+        sda_gpio = CONFIG_RP2040_I2C0_GPIO;

Review comment:
       Currently all RP2040 GPIO pins are assigned in https://github.com/apache/incubator-nuttx/blob/master/boards/arm/rp2040/raspberrypi-pico/src/rp2040_boardinitialize.c
   The functions, such as I2C and SPI, are assigned here according to the kernel configurations, but the current GPIO API design have no assumption that the assignment may be changed after the boot.  This is the reason of the dirty design in rp2040_i2c_reset().
   
   I think it is better to provide the APIs to retrieve the current GPIO function assignment to make it clean.




-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

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



[GitHub] [incubator-nuttx] davids5 commented on pull request #4786: rp2040: support I2C_RESET

Posted by GitBox <gi...@apache.org>.
davids5 commented on pull request #4786:
URL: https://github.com/apache/incubator-nuttx/pull/4786#issuecomment-962595460


   Let's ignore the doc build CI issue


-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

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



[GitHub] [incubator-nuttx] davids5 merged pull request #4786: rp2040: support I2C_RESET

Posted by GitBox <gi...@apache.org>.
davids5 merged pull request #4786:
URL: https://github.com/apache/incubator-nuttx/pull/4786


   


-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

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



[GitHub] [incubator-nuttx] davids5 commented on a change in pull request #4786: rp2040: support I2C_RESET

Posted by GitBox <gi...@apache.org>.
davids5 commented on a change in pull request #4786:
URL: https://github.com/apache/incubator-nuttx/pull/4786#discussion_r743592637



##########
File path: arch/arm/src/rp2040/rp2040_i2c.c
##########
@@ -639,7 +642,158 @@ static int rp2040_i2c_transfer(FAR struct i2c_master_s *dev,
 #ifdef CONFIG_I2C_RESET
 static int rp2040_i2c_reset(FAR struct i2c_master_s *dev)
 {
-  return OK;
+  FAR struct rp2040_i2cdev_s *priv = (struct rp2040_i2cdev_s *)dev;
+  unsigned int clock_count;
+  unsigned int stretch_count;
+  uint32_t scl_gpio;
+  uint32_t sda_gpio;
+  uint32_t subsys;
+  uint32_t frequency;
+  int ret;
+
+  DEBUGASSERT(dev);
+
+  /* Our caller must own a ref */
+
+  DEBUGASSERT(priv->refs > 0);
+
+  /* Lock out other clients */
+
+  i2c_takesem(&priv->mutex);
+
+  ret = -EIO;
+
+  /* De-init the port */
+
+  rp2040_i2c_disable(priv);
+
+  /* Use GPIO configuration to un-wedge the bus */
+
+  switch (priv->port)
+    {
+      case 0:
+#if defined(CONFIG_RP2040_I2C0) && CONFIG_RP2040_I2C0_GPIO >= 0
+        sda_gpio = CONFIG_RP2040_I2C0_GPIO;

Review comment:
       Is something like this https://github.com/apache/incubator-nuttx/blob/master/arch/arm/src/stm32h7/stm32_i2c.c#L2596-L2597 possible this the GPIO IP on the RP240.
   
   It would make this have less config option and code.

##########
File path: arch/arm/src/rp2040/rp2040_i2c.c
##########
@@ -639,7 +642,158 @@ static int rp2040_i2c_transfer(FAR struct i2c_master_s *dev,
 #ifdef CONFIG_I2C_RESET
 static int rp2040_i2c_reset(FAR struct i2c_master_s *dev)
 {
-  return OK;
+  FAR struct rp2040_i2cdev_s *priv = (struct rp2040_i2cdev_s *)dev;
+  unsigned int clock_count;
+  unsigned int stretch_count;
+  uint32_t scl_gpio;
+  uint32_t sda_gpio;
+  uint32_t subsys;
+  uint32_t frequency;
+  int ret;
+
+  DEBUGASSERT(dev);
+
+  /* Our caller must own a ref */
+
+  DEBUGASSERT(priv->refs > 0);
+
+  /* Lock out other clients */
+
+  i2c_takesem(&priv->mutex);
+
+  ret = -EIO;
+
+  /* De-init the port */
+
+  rp2040_i2c_disable(priv);
+
+  /* Use GPIO configuration to un-wedge the bus */
+
+  switch (priv->port)
+    {
+      case 0:
+#if defined(CONFIG_RP2040_I2C0) && CONFIG_RP2040_I2C0_GPIO >= 0
+        sda_gpio = CONFIG_RP2040_I2C0_GPIO;

Review comment:
       Is something like this https://github.com/apache/incubator-nuttx/blob/master/arch/arm/src/stm32h7/stm32_i2c.c#L2596-L2597 possible with the GPIO IP on the RP240.
   
   It would make this have less config option and code.

##########
File path: arch/arm/src/rp2040/rp2040_i2c.c
##########
@@ -639,7 +642,158 @@ static int rp2040_i2c_transfer(FAR struct i2c_master_s *dev,
 #ifdef CONFIG_I2C_RESET
 static int rp2040_i2c_reset(FAR struct i2c_master_s *dev)
 {
-  return OK;
+  FAR struct rp2040_i2cdev_s *priv = (struct rp2040_i2cdev_s *)dev;
+  unsigned int clock_count;
+  unsigned int stretch_count;
+  uint32_t scl_gpio;
+  uint32_t sda_gpio;
+  uint32_t subsys;
+  uint32_t frequency;
+  int ret;
+
+  DEBUGASSERT(dev);
+
+  /* Our caller must own a ref */
+
+  DEBUGASSERT(priv->refs > 0);
+
+  /* Lock out other clients */
+
+  i2c_takesem(&priv->mutex);
+
+  ret = -EIO;
+
+  /* De-init the port */
+
+  rp2040_i2c_disable(priv);
+
+  /* Use GPIO configuration to un-wedge the bus */
+
+  switch (priv->port)
+    {
+      case 0:
+#if defined(CONFIG_RP2040_I2C0) && CONFIG_RP2040_I2C0_GPIO >= 0
+        sda_gpio = CONFIG_RP2040_I2C0_GPIO;

Review comment:
       Ok that makes sense. Yes as it is the GPIO interface seems rather limiting. 




-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

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



[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #4786: rp2040: support I2C_RESET

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #4786:
URL: https://github.com/apache/incubator-nuttx/pull/4786#discussion_r743474380



##########
File path: arch/arm/Kconfig
##########
@@ -232,6 +232,7 @@ config ARCH_CHIP_RP2040
 	select ARCH_HAVE_MULTICPU
 	select ARCH_HAVE_TESTSET
 	select ARM_HAVE_WFE_SEV
+	select ARCH_HAVE_I2CRESET

Review comment:
       move before line 234?




-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

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



[GitHub] [incubator-nuttx] davids5 commented on a change in pull request #4786: rp2040: support I2C_RESET

Posted by GitBox <gi...@apache.org>.
davids5 commented on a change in pull request #4786:
URL: https://github.com/apache/incubator-nuttx/pull/4786#discussion_r743592637



##########
File path: arch/arm/src/rp2040/rp2040_i2c.c
##########
@@ -639,7 +642,158 @@ static int rp2040_i2c_transfer(FAR struct i2c_master_s *dev,
 #ifdef CONFIG_I2C_RESET
 static int rp2040_i2c_reset(FAR struct i2c_master_s *dev)
 {
-  return OK;
+  FAR struct rp2040_i2cdev_s *priv = (struct rp2040_i2cdev_s *)dev;
+  unsigned int clock_count;
+  unsigned int stretch_count;
+  uint32_t scl_gpio;
+  uint32_t sda_gpio;
+  uint32_t subsys;
+  uint32_t frequency;
+  int ret;
+
+  DEBUGASSERT(dev);
+
+  /* Our caller must own a ref */
+
+  DEBUGASSERT(priv->refs > 0);
+
+  /* Lock out other clients */
+
+  i2c_takesem(&priv->mutex);
+
+  ret = -EIO;
+
+  /* De-init the port */
+
+  rp2040_i2c_disable(priv);
+
+  /* Use GPIO configuration to un-wedge the bus */
+
+  switch (priv->port)
+    {
+      case 0:
+#if defined(CONFIG_RP2040_I2C0) && CONFIG_RP2040_I2C0_GPIO >= 0
+        sda_gpio = CONFIG_RP2040_I2C0_GPIO;

Review comment:
       Is something like this https://github.com/apache/incubator-nuttx/blob/master/arch/arm/src/stm32h7/stm32_i2c.c#L2596-L2597 possible this the GPIO IP on the RP240.
   
   It would make this have less config option and code.




-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

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



[GitHub] [incubator-nuttx] davids5 commented on a change in pull request #4786: rp2040: support I2C_RESET

Posted by GitBox <gi...@apache.org>.
davids5 commented on a change in pull request #4786:
URL: https://github.com/apache/incubator-nuttx/pull/4786#discussion_r743592637



##########
File path: arch/arm/src/rp2040/rp2040_i2c.c
##########
@@ -639,7 +642,158 @@ static int rp2040_i2c_transfer(FAR struct i2c_master_s *dev,
 #ifdef CONFIG_I2C_RESET
 static int rp2040_i2c_reset(FAR struct i2c_master_s *dev)
 {
-  return OK;
+  FAR struct rp2040_i2cdev_s *priv = (struct rp2040_i2cdev_s *)dev;
+  unsigned int clock_count;
+  unsigned int stretch_count;
+  uint32_t scl_gpio;
+  uint32_t sda_gpio;
+  uint32_t subsys;
+  uint32_t frequency;
+  int ret;
+
+  DEBUGASSERT(dev);
+
+  /* Our caller must own a ref */
+
+  DEBUGASSERT(priv->refs > 0);
+
+  /* Lock out other clients */
+
+  i2c_takesem(&priv->mutex);
+
+  ret = -EIO;
+
+  /* De-init the port */
+
+  rp2040_i2c_disable(priv);
+
+  /* Use GPIO configuration to un-wedge the bus */
+
+  switch (priv->port)
+    {
+      case 0:
+#if defined(CONFIG_RP2040_I2C0) && CONFIG_RP2040_I2C0_GPIO >= 0
+        sda_gpio = CONFIG_RP2040_I2C0_GPIO;

Review comment:
       Is something like this https://github.com/apache/incubator-nuttx/blob/master/arch/arm/src/stm32h7/stm32_i2c.c#L2596-L2597 possible with the GPIO IP on the RP240.
   
   It would make this have less config option and code.




-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

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



[GitHub] [incubator-nuttx] yunkya2 commented on a change in pull request #4786: rp2040: support I2C_RESET

Posted by GitBox <gi...@apache.org>.
yunkya2 commented on a change in pull request #4786:
URL: https://github.com/apache/incubator-nuttx/pull/4786#discussion_r743631676



##########
File path: arch/arm/src/rp2040/rp2040_i2c.c
##########
@@ -639,7 +642,158 @@ static int rp2040_i2c_transfer(FAR struct i2c_master_s *dev,
 #ifdef CONFIG_I2C_RESET
 static int rp2040_i2c_reset(FAR struct i2c_master_s *dev)
 {
-  return OK;
+  FAR struct rp2040_i2cdev_s *priv = (struct rp2040_i2cdev_s *)dev;
+  unsigned int clock_count;
+  unsigned int stretch_count;
+  uint32_t scl_gpio;
+  uint32_t sda_gpio;
+  uint32_t subsys;
+  uint32_t frequency;
+  int ret;
+
+  DEBUGASSERT(dev);
+
+  /* Our caller must own a ref */
+
+  DEBUGASSERT(priv->refs > 0);
+
+  /* Lock out other clients */
+
+  i2c_takesem(&priv->mutex);
+
+  ret = -EIO;
+
+  /* De-init the port */
+
+  rp2040_i2c_disable(priv);
+
+  /* Use GPIO configuration to un-wedge the bus */
+
+  switch (priv->port)
+    {
+      case 0:
+#if defined(CONFIG_RP2040_I2C0) && CONFIG_RP2040_I2C0_GPIO >= 0
+        sda_gpio = CONFIG_RP2040_I2C0_GPIO;

Review comment:
       Currently all RP2040 GPIO pins are assigned in https://github.com/apache/incubator-nuttx/blob/master/boards/arm/rp2040/raspberrypi-pico/src/rp2040_boardinitialize.c
   The functions, such as I2C and SPI, are assigned here according to the kernel configurations, but the current GPIO API design have no assumption that the assignment may be changed after the boot.  This is the reason of the dirty design in rp2040_i2c_reset().
   
   I think it is better to provide the APIs to retrieve the current GPIO function assignment to make it clean.

##########
File path: arch/arm/src/rp2040/rp2040_i2c.c
##########
@@ -639,7 +642,158 @@ static int rp2040_i2c_transfer(FAR struct i2c_master_s *dev,
 #ifdef CONFIG_I2C_RESET
 static int rp2040_i2c_reset(FAR struct i2c_master_s *dev)
 {
-  return OK;
+  FAR struct rp2040_i2cdev_s *priv = (struct rp2040_i2cdev_s *)dev;
+  unsigned int clock_count;
+  unsigned int stretch_count;
+  uint32_t scl_gpio;
+  uint32_t sda_gpio;
+  uint32_t subsys;
+  uint32_t frequency;
+  int ret;
+
+  DEBUGASSERT(dev);
+
+  /* Our caller must own a ref */
+
+  DEBUGASSERT(priv->refs > 0);
+
+  /* Lock out other clients */
+
+  i2c_takesem(&priv->mutex);
+
+  ret = -EIO;
+
+  /* De-init the port */
+
+  rp2040_i2c_disable(priv);
+
+  /* Use GPIO configuration to un-wedge the bus */
+
+  switch (priv->port)
+    {
+      case 0:
+#if defined(CONFIG_RP2040_I2C0) && CONFIG_RP2040_I2C0_GPIO >= 0
+        sda_gpio = CONFIG_RP2040_I2C0_GPIO;

Review comment:
       Currently all RP2040 GPIO pins are assigned in https://github.com/apache/incubator-nuttx/blob/master/boards/arm/rp2040/raspberrypi-pico/src/rp2040_boardinitialize.c
   The functions, such as I2C and SPI, are assigned here according to the kernel configurations, but the current GPIO API design have no assumption that the assignment may be changed after the boot.  This is the reason of the dirty design in rp2040_i2c_reset().
   
   I think it is better to provide the APIs to retrieve the current GPIO function assignment to make it clean.




-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

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