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 2022/07/15 09:53:28 UTC
[GitHub] [incubator-nuttx] pkarashchenko commented on a diff in pull request #6615: Add I2C slave mode for RP2040
pkarashchenko commented on code in PR #6615:
URL: https://github.com/apache/incubator-nuttx/pull/6615#discussion_r922002998
##########
arch/arm/src/rp2040/rp2040_i2c_slave.c:
##########
@@ -0,0 +1,661 @@
+/****************************************************************************
+ * arch/arm/src/rp2040/rp2040_i2c_slave.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership. The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <sys/types.h>
+#include <stdint.h>
+#include <stdbool.h>
+#include <stdlib.h>
+#include <string.h>
+#include <errno.h>
+#include <debug.h>
+#include <assert.h>
+
+#include <nuttx/arch.h>
+#include <nuttx/i2c/i2c_slave.h>
+#include <nuttx/irq.h>
+#include <nuttx/kmalloc.h>
+
+#include <arch/chip/i2c_slave.h>
+#include <arch/board/board.h>
+
+#include "chip.h"
+#include "arm_internal.h"
+#include "rp2040_i2c.h"
+#include "hardware/rp2040_i2c.h"
+#include "hardware/rp2040_resets.h"
+#include "rp2040_gpio.h"
+
+#ifdef CONFIG_RP2040_I2C_SLAVE
+
+#define FIFO_LENGTH 16
+
+#define TX_BUF_LEN 8
+#define RX_BUF_LEN 8
+
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+typedef struct rp2040_i2cdev_s
+{
+ struct i2c_slave_s dev; /* Generic I2C device */
+ int8_t controller; /* I2C controller number */
+ int error; /* Error value */
+
+ sem_t wait; /* Wait for transfer done */
+
+ uint8_t *rx_buffer;
+ uint8_t *rx_buf_ptr;
+ uint8_t *rx_buf_end;
+
+ const uint8_t *tx_buffer;
+ const uint8_t *tx_buf_ptr;
+ const uint8_t *tx_buf_end;
+
+ rp2040_i2c_callback_t *callback; /* Callback function */
+ rp2040_i2c_callback_len_t *callback_len;
+ void *callback_arg; /* Argument for callback */
+} rp2040_i2cdev_t;
+
+/****************************************************************************
+ * Private Function Prototypes
+ ****************************************************************************/
+
+static int i2c_interrupt(int irq,
+ void *context,
+ void *arg);
+
+static int my_set_own_address(struct i2c_slave_s *dev,
+ int address,
+ int nbits);
+
+static int my_write(struct i2c_slave_s *dev,
+ const uint8_t *buffer,
+ int length);
+
+static int my_read(struct i2c_slave_s *dev,
+ uint8_t *buffer,
+ int length);
+
+static int my_register_callback(struct i2c_slave_s *dev,
+ rp2040_i2c_callback_t *callback,
+ void *arg);
+
+static int my_register_callback_len(struct i2c_slave_s *dev,
+ rp2040_i2c_callback_len_t *callback,
+ void *arg);
+
+static void enable_i2c_slave(struct i2c_slave_s *dev);
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+struct i2c_slaveops_s i2c_slaveops =
+{
+ .setownaddress = my_set_own_address,
+ .write = my_write,
+ .read = my_read,
+ .registercallback = my_register_callback,
+ .registercallback_len = my_register_callback_len
+};
+
+#ifdef CONFIG_RP2040_I2C0_SLAVE
+
+rp2040_i2cdev_t i2c0_slave_dev =
+{
+ .dev.ops = &i2c_slaveops, /* Slave operations */
+ .controller = 0, /* I2C controller number */
+ .error = OK, /* Error value */
+ .rx_buffer = NULL, /* Receive buffer */
+ .rx_buf_ptr = NULL, /* Receive buffer location pointer */
+ .rx_buf_end = NULL, /* Receive buffer end pointer */
+ .tx_buffer = NULL, /* Transmit buffer */
+ .tx_buf_ptr = NULL, /* Transmit buffer location pointer */
+ .tx_buf_end = NULL, /* Transmit buffer end pointer */
+ .callback = NULL, /* Callback function */
+ .callback_len = NULL, /* Callback function with len */
+ .callback_arg = NULL /* Argument for callback */
Review Comment:
Optional
```suggestion
.controller = 0, /* I2C controller number */
```
##########
arch/arm/src/rp2040/rp2040_i2c_slave.c:
##########
@@ -0,0 +1,661 @@
+/****************************************************************************
+ * arch/arm/src/rp2040/rp2040_i2c_slave.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership. The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <sys/types.h>
+#include <stdint.h>
+#include <stdbool.h>
+#include <stdlib.h>
+#include <string.h>
+#include <errno.h>
+#include <debug.h>
+#include <assert.h>
+
+#include <nuttx/arch.h>
+#include <nuttx/i2c/i2c_slave.h>
+#include <nuttx/irq.h>
+#include <nuttx/kmalloc.h>
+
+#include <arch/chip/i2c_slave.h>
+#include <arch/board/board.h>
+
+#include "chip.h"
+#include "arm_internal.h"
+#include "rp2040_i2c.h"
+#include "hardware/rp2040_i2c.h"
+#include "hardware/rp2040_resets.h"
+#include "rp2040_gpio.h"
+
+#ifdef CONFIG_RP2040_I2C_SLAVE
+
+#define FIFO_LENGTH 16
+
+#define TX_BUF_LEN 8
+#define RX_BUF_LEN 8
+
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+typedef struct rp2040_i2cdev_s
+{
+ struct i2c_slave_s dev; /* Generic I2C device */
+ int8_t controller; /* I2C controller number */
+ int error; /* Error value */
+
+ sem_t wait; /* Wait for transfer done */
+
+ uint8_t *rx_buffer;
+ uint8_t *rx_buf_ptr;
+ uint8_t *rx_buf_end;
+
+ const uint8_t *tx_buffer;
+ const uint8_t *tx_buf_ptr;
+ const uint8_t *tx_buf_end;
+
+ rp2040_i2c_callback_t *callback; /* Callback function */
+ rp2040_i2c_callback_len_t *callback_len;
+ void *callback_arg; /* Argument for callback */
+} rp2040_i2cdev_t;
+
+/****************************************************************************
+ * Private Function Prototypes
+ ****************************************************************************/
+
+static int i2c_interrupt(int irq,
+ void *context,
+ void *arg);
+
+static int my_set_own_address(struct i2c_slave_s *dev,
+ int address,
+ int nbits);
+
+static int my_write(struct i2c_slave_s *dev,
+ const uint8_t *buffer,
+ int length);
+
+static int my_read(struct i2c_slave_s *dev,
+ uint8_t *buffer,
+ int length);
+
+static int my_register_callback(struct i2c_slave_s *dev,
+ rp2040_i2c_callback_t *callback,
+ void *arg);
+
+static int my_register_callback_len(struct i2c_slave_s *dev,
+ rp2040_i2c_callback_len_t *callback,
+ void *arg);
+
+static void enable_i2c_slave(struct i2c_slave_s *dev);
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+struct i2c_slaveops_s i2c_slaveops =
+{
+ .setownaddress = my_set_own_address,
+ .write = my_write,
+ .read = my_read,
+ .registercallback = my_register_callback,
+ .registercallback_len = my_register_callback_len
+};
+
+#ifdef CONFIG_RP2040_I2C0_SLAVE
+
+rp2040_i2cdev_t i2c0_slave_dev =
+{
+ .dev.ops = &i2c_slaveops, /* Slave operations */
+ .controller = 0, /* I2C controller number */
+ .error = OK, /* Error value */
+ .rx_buffer = NULL, /* Receive buffer */
+ .rx_buf_ptr = NULL, /* Receive buffer location pointer */
+ .rx_buf_end = NULL, /* Receive buffer end pointer */
+ .tx_buffer = NULL, /* Transmit buffer */
+ .tx_buf_ptr = NULL, /* Transmit buffer location pointer */
+ .tx_buf_end = NULL, /* Transmit buffer end pointer */
+ .callback = NULL, /* Callback function */
+ .callback_len = NULL, /* Callback function with len */
+ .callback_arg = NULL /* Argument for callback */
+};
+
+#endif
+
+#ifdef CONFIG_RP2040_I2C1_SLAVE
+
+rp2040_i2cdev_t i2c1_slave_dev =
+{
+ .dev.ops = &i2c_slaveops, /* Slave operations */
+ .controller = 1, /* I2C controller number */
+ .error = OK, /* Error value */
+ .rx_buffer = NULL, /* Receive buffer */
+ .rx_buf_ptr = NULL, /* Receive buffer location pointer */
+ .rx_buf_end = NULL, /* Receive buffer end pointer */
+ .tx_buffer = NULL, /* Transmit buffer */
+ .tx_buf_ptr = NULL, /* Transmit buffer location pointer */
+ .tx_buf_end = NULL, /* Transmit buffer end pointer */
+ .callback = NULL, /* Callback function */
+ .callback_len = NULL, /* Callback function with len */
+ .callback_arg = NULL /* Argument for callback */
+};
+
+#endif
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: i2c_interrupt
+ *
+ * Description:
+ * The I2C Interrupt Handler
+ *
+ ****************************************************************************/
+
+static int i2c_interrupt(int irq, void *context, void *arg)
+{
+ struct rp2040_i2cdev_s *priv = (struct rp2040_i2cdev_s *)arg;
+ uint32_t data_cmd;
+ uint32_t state;
+
+ state = getreg32(RP2040_I2C_IC_INTR_STAT(priv->controller));
+
+ /* -- We need to transmit data (Read Request) -- */
+
+ if (state & RP2040_I2C_IC_INTR_STAT_R_RD_REQ)
+ {
+ if (priv->tx_buf_ptr < priv->tx_buf_end)
+ {
+ while (priv->tx_buf_ptr < priv->tx_buf_end
+ && getreg32(RP2040_I2C_IC_TXFLR(priv->controller))
+ < FIFO_LENGTH)
+ {
+ putreg32(*priv->tx_buf_ptr++,
+ RP2040_I2C_IC_DATA_CMD(priv->controller));
+ }
+ }
+ else
+ {
+ putreg32(0, RP2040_I2C_IC_DATA_CMD(priv->controller));
+ }
+
+ getreg32(RP2040_I2C_IC_CLR_RD_REQ(priv->controller));
+ }
+
+ /* -- We are receiving data (Write Request) -- */
+
+ if (state & RP2040_I2C_IC_INTR_STAT_R_RX_FULL)
+ {
+ while (getreg32(RP2040_I2C_IC_RXFLR(priv->controller)) > 0)
+ {
+ data_cmd = getreg32(RP2040_I2C_IC_DATA_CMD(priv->controller));
+
+ if (data_cmd & RP2040_I2C_IC_DATA_CMD_FIRST_DATA_BYTE)
+ {
+ priv->rx_buf_ptr = priv->rx_buffer;
+ }
+
+ if (priv->rx_buf_ptr < priv->rx_buf_end)
+ {
+ *priv->rx_buf_ptr++ = (uint8_t) data_cmd;
+ }
+ }
+ }
+
+ /* -- Restart -- */
+
+ if (state & RP2040_I2C_IC_INTR_STAT_R_RESTART_DET)
+ {
+ if (priv->rx_buf_ptr > priv->rx_buffer)
+ {
+ if (priv->callback != NULL)
+ {
+ priv->callback(priv);
+ }
+
+ if (priv->callback_len != NULL)
+ {
+ priv->callback_len(priv, priv->rx_buf_ptr - priv->rx_buffer);
+ }
+
+ priv->rx_buf_ptr = priv->rx_buffer;
+ }
+
+ getreg32(RP2040_I2C_IC_CLR_RESTART_DET(priv->controller));
+ }
+
+ /* -- End of transfer -- */
+
+ if (state & RP2040_I2C_IC_INTR_STAT_R_STOP_DET)
+ {
+ if (priv->rx_buf_ptr > priv->rx_buffer)
+ {
+ if (priv->callback != NULL)
+ {
+ priv->callback(priv);
+ }
+
+ if (priv->callback_len != NULL)
+ {
+ priv->callback_len(priv, priv->rx_buf_ptr - priv->rx_buffer);
+ }
+
+ priv->rx_buf_ptr = priv->rx_buffer;
+ }
+
+ getreg32(RP2040_I2C_IC_CLR_STOP_DET(priv->controller));
+ }
+
+ /* -- Transmit Abort -- */
+
+ if (state & RP2040_I2C_IC_INTR_STAT_R_TX_ABRT)
+ {
+ getreg32(RP2040_I2C_IC_CLR_TX_ABRT(priv->controller));
+ priv->error = -ENODEV;
+ }
+
+ if (state & RP2040_I2C_IC_INTR_STAT_R_TX_OVER)
+ {
+ getreg32(RP2040_I2C_IC_CLR_TX_OVER(priv->controller));
+ priv->error = -EIO;
+ }
+
+ if (state & RP2040_I2C_IC_INTR_STAT_R_RX_OVER)
+ {
+ getreg32(RP2040_I2C_IC_CLR_RX_OVER(priv->controller));
+ priv->error = -EIO;
+ }
+
+ if (state & RP2040_I2C_IC_INTR_STAT_R_RX_UNDER)
+ {
+ getreg32(RP2040_I2C_IC_CLR_RX_UNDER(priv->controller));
+ priv->error = -EIO;
+ }
+
+#ifdef NEEDED_FOR_MASTER_MODE_
+ if (state & RP2040_I2C_IC_INTR_STAT_R_TX_EMPTY)
+ {
+ /* TX_EMPTY is automatically cleared by hardware
+ * when the buffer level goes above the threshold.
+ */
+
+ modbits_reg32(RP2040_I2C_IC_INTR_MASK(priv->controller),
+ 0
+ RP2040_I2C_IC_INTR_MASK_M_TX_EMPTY);
+ }
+
+ if (state & RP2040_I2C_IC_INTR_STAT_R_RX_FULL)
+ {
+ /* RX_FULL is automatically cleared by hardware
+ * when the buffer level goes below the threshold.
+ */
+
+ modbits_reg32(RP2040_I2C_IC_INTR_MASK(priv->controller),
+ 0,
+ RP2040_I2C_IC_INTR_MASK_M_RX_FULL);
+
+ rp2040_i2c_drainrxfifo(priv);
+ }
+
+ if ((priv->error) || (state & RP2040_I2C_IC_INTR_STAT_R_TX_EMPTY)
+ || (state & RP2040_I2C_IC_INTR_STAT_R_RX_FULL))
Review Comment:
Optional
```suggestion
if (priv->error != OK ||
(state & RP2040_I2C_IC_INTR_STAT_R_TX_EMPTY) ||
(state & RP2040_I2C_IC_INTR_STAT_R_RX_FULL))
```
##########
include/nuttx/i2c/i2c_slave.h:
##########
@@ -164,6 +208,8 @@ struct i2c_slaveops_s
int buflen);
int (*registercallback)(FAR struct i2c_slave_s *dev,
int (*callback)(FAR void *arg), FAR void *arg);
+ int (*registercallback_len)(FAR struct i2c_slave_s *dev,
+ int (*callback_len)(FAR void *arg, size_t rx_len), FAR void *arg);
Review Comment:
why do we need both `registercallback` and `registercallback_len`? Maybe it is reasonable to extend `registercallback` with length parameter? Or I'm missing use cases?
##########
arch/arm/src/rp2040/rp2040_i2c_slave.c:
##########
@@ -0,0 +1,661 @@
+/****************************************************************************
+ * arch/arm/src/rp2040/rp2040_i2c_slave.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership. The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <sys/types.h>
+#include <stdint.h>
+#include <stdbool.h>
+#include <stdlib.h>
+#include <string.h>
+#include <errno.h>
+#include <debug.h>
+#include <assert.h>
+
+#include <nuttx/arch.h>
+#include <nuttx/i2c/i2c_slave.h>
+#include <nuttx/irq.h>
+#include <nuttx/kmalloc.h>
+
+#include <arch/chip/i2c_slave.h>
+#include <arch/board/board.h>
+
+#include "chip.h"
+#include "arm_internal.h"
+#include "rp2040_i2c.h"
+#include "hardware/rp2040_i2c.h"
+#include "hardware/rp2040_resets.h"
+#include "rp2040_gpio.h"
+
+#ifdef CONFIG_RP2040_I2C_SLAVE
+
+#define FIFO_LENGTH 16
+
+#define TX_BUF_LEN 8
+#define RX_BUF_LEN 8
+
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+typedef struct rp2040_i2cdev_s
+{
+ struct i2c_slave_s dev; /* Generic I2C device */
+ int8_t controller; /* I2C controller number */
+ int error; /* Error value */
+
+ sem_t wait; /* Wait for transfer done */
+
+ uint8_t *rx_buffer;
+ uint8_t *rx_buf_ptr;
+ uint8_t *rx_buf_end;
+
+ const uint8_t *tx_buffer;
+ const uint8_t *tx_buf_ptr;
+ const uint8_t *tx_buf_end;
+
+ rp2040_i2c_callback_t *callback; /* Callback function */
+ rp2040_i2c_callback_len_t *callback_len;
+ void *callback_arg; /* Argument for callback */
+} rp2040_i2cdev_t;
+
+/****************************************************************************
+ * Private Function Prototypes
+ ****************************************************************************/
+
+static int i2c_interrupt(int irq,
+ void *context,
+ void *arg);
+
+static int my_set_own_address(struct i2c_slave_s *dev,
+ int address,
+ int nbits);
+
+static int my_write(struct i2c_slave_s *dev,
+ const uint8_t *buffer,
+ int length);
+
+static int my_read(struct i2c_slave_s *dev,
+ uint8_t *buffer,
+ int length);
+
+static int my_register_callback(struct i2c_slave_s *dev,
+ rp2040_i2c_callback_t *callback,
+ void *arg);
+
+static int my_register_callback_len(struct i2c_slave_s *dev,
+ rp2040_i2c_callback_len_t *callback,
+ void *arg);
+
+static void enable_i2c_slave(struct i2c_slave_s *dev);
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+struct i2c_slaveops_s i2c_slaveops =
+{
+ .setownaddress = my_set_own_address,
+ .write = my_write,
+ .read = my_read,
+ .registercallback = my_register_callback,
+ .registercallback_len = my_register_callback_len
+};
+
+#ifdef CONFIG_RP2040_I2C0_SLAVE
+
+rp2040_i2cdev_t i2c0_slave_dev =
+{
+ .dev.ops = &i2c_slaveops, /* Slave operations */
+ .controller = 0, /* I2C controller number */
+ .error = OK, /* Error value */
+ .rx_buffer = NULL, /* Receive buffer */
+ .rx_buf_ptr = NULL, /* Receive buffer location pointer */
+ .rx_buf_end = NULL, /* Receive buffer end pointer */
+ .tx_buffer = NULL, /* Transmit buffer */
+ .tx_buf_ptr = NULL, /* Transmit buffer location pointer */
+ .tx_buf_end = NULL, /* Transmit buffer end pointer */
+ .callback = NULL, /* Callback function */
+ .callback_len = NULL, /* Callback function with len */
+ .callback_arg = NULL /* Argument for callback */
+};
+
+#endif
+
+#ifdef CONFIG_RP2040_I2C1_SLAVE
+
+rp2040_i2cdev_t i2c1_slave_dev =
+{
+ .dev.ops = &i2c_slaveops, /* Slave operations */
+ .controller = 1, /* I2C controller number */
+ .error = OK, /* Error value */
+ .rx_buffer = NULL, /* Receive buffer */
+ .rx_buf_ptr = NULL, /* Receive buffer location pointer */
+ .rx_buf_end = NULL, /* Receive buffer end pointer */
+ .tx_buffer = NULL, /* Transmit buffer */
+ .tx_buf_ptr = NULL, /* Transmit buffer location pointer */
+ .tx_buf_end = NULL, /* Transmit buffer end pointer */
+ .callback = NULL, /* Callback function */
+ .callback_len = NULL, /* Callback function with len */
+ .callback_arg = NULL /* Argument for callback */
Review Comment:
Optional
```suggestion
.controller = 1, /* I2C controller number */
```
--
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