xiaoxiang781216 commented on a change in pull request #2717: URL: https://github.com/apache/incubator-nuttx/pull/2717#discussion_r559951014
########## File path: drivers/i2c/i2c_bitbang.c ########## @@ -0,0 +1,340 @@ +/**************************************************************************** + * arch/arm/src/nrf52/nrf52_i2c.c Review comment: correct the file name ########## File path: drivers/i2c/i2c_bitbang.c ########## @@ -0,0 +1,340 @@ +/**************************************************************************** + * arch/arm/src/nrf52/nrf52_i2c.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 <errno.h> +#include <debug.h> + +#include <nuttx/irq.h> +#include <nuttx/arch.h> +#include <nuttx/semaphore.h> +#include <nuttx/kmalloc.h> +#include <arch/board/board.h> +#include <nuttx/i2c/i2c_master.h> +#include <nuttx/i2c/i2c_bitbang.h> + +/**************************************************************************** + * Pre-processor Definitions + ****************************************************************************/ + +/**************************************************************************** + * Private Types + ****************************************************************************/ + +struct i2c_bitbang_dev_s +{ + struct i2c_master_s i2c; + struct i2c_bitbang_lower_dev_s *lower; + +#ifdef CONFIG_I2C_BITBANG_NO_DELAY + uint32_t delay; +#endif +}; + +/**************************************************************************** + * Private Function Prototypes + ****************************************************************************/ + +static int i2c_bitbang_transfer(FAR struct i2c_master_s *dev, + FAR struct i2c_msg_s *msgs, int count); + +static int i2c_bitbang_set_scl(FAR struct i2c_bitbang_dev_s *dev, + bool high); +static void i2c_bitbang_set_sda(FAR struct i2c_bitbang_dev_s *dev, + bool high); + +static int i2c_bitbang_wait_ack(FAR struct i2c_bitbang_dev_s *dev); +static void i2c_bitbang_send(FAR struct i2c_bitbang_dev_s *dev, + uint8_t data); + +/**************************************************************************** + * Private Data + ****************************************************************************/ + +struct i2c_ops_s g_i2c_ops = Review comment: add static const ########## File path: drivers/i2c/i2c_bitbang.c ########## @@ -0,0 +1,340 @@ +/**************************************************************************** + * arch/arm/src/nrf52/nrf52_i2c.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 <errno.h> +#include <debug.h> + +#include <nuttx/irq.h> +#include <nuttx/arch.h> +#include <nuttx/semaphore.h> +#include <nuttx/kmalloc.h> +#include <arch/board/board.h> Review comment: remove ########## File path: drivers/i2c/i2c_bitbang.c ########## @@ -0,0 +1,340 @@ +/**************************************************************************** + * arch/arm/src/nrf52/nrf52_i2c.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 <errno.h> +#include <debug.h> + +#include <nuttx/irq.h> +#include <nuttx/arch.h> +#include <nuttx/semaphore.h> +#include <nuttx/kmalloc.h> +#include <arch/board/board.h> +#include <nuttx/i2c/i2c_master.h> +#include <nuttx/i2c/i2c_bitbang.h> + +/**************************************************************************** + * Pre-processor Definitions + ****************************************************************************/ + +/**************************************************************************** + * Private Types + ****************************************************************************/ + +struct i2c_bitbang_dev_s +{ + struct i2c_master_s i2c; + struct i2c_bitbang_lower_dev_s *lower; + +#ifdef CONFIG_I2C_BITBANG_NO_DELAY + uint32_t delay; +#endif +}; + +/**************************************************************************** + * Private Function Prototypes + ****************************************************************************/ + +static int i2c_bitbang_transfer(FAR struct i2c_master_s *dev, + FAR struct i2c_msg_s *msgs, int count); + +static int i2c_bitbang_set_scl(FAR struct i2c_bitbang_dev_s *dev, + bool high); +static void i2c_bitbang_set_sda(FAR struct i2c_bitbang_dev_s *dev, + bool high); + +static int i2c_bitbang_wait_ack(FAR struct i2c_bitbang_dev_s *dev); +static void i2c_bitbang_send(FAR struct i2c_bitbang_dev_s *dev, + uint8_t data); + +/**************************************************************************** + * Private Data + ****************************************************************************/ + +struct i2c_ops_s g_i2c_ops = +{ + .transfer = i2c_bitbang_transfer +}; + +/**************************************************************************** + * Private Functions + ****************************************************************************/ + +/**************************************************************************** + * Name: i2c_bitbang_transfer + ****************************************************************************/ + +static int i2c_bitbang_transfer(FAR struct i2c_master_s *dev, + FAR struct i2c_msg_s *msgs, int count) +{ + FAR struct i2c_bitbang_dev_s *priv = (FAR struct i2c_bitbang_dev_s *)dev; + int ret = OK; + int i; + + for (i = 0; i < count; i++) + { + uint8_t addr; + FAR struct i2c_msg_s *msg = &msgs[i]; + +#ifdef CONFIG_I2C_BITBANG_NO_DELAY + /* Compute delay from frequency */ + + priv->delay = ((1000000 / msg->frequency) / 2) - Review comment: why not change delay to local variable and pass it to i2c_bitbang_set_xxx instead? ########## File path: drivers/i2c/i2c_bitbang.c ########## @@ -0,0 +1,340 @@ +/**************************************************************************** + * arch/arm/src/nrf52/nrf52_i2c.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 <errno.h> +#include <debug.h> + +#include <nuttx/irq.h> +#include <nuttx/arch.h> Review comment: remove? ########## File path: drivers/i2c/i2c_bitbang.c ########## @@ -0,0 +1,340 @@ +/**************************************************************************** + * arch/arm/src/nrf52/nrf52_i2c.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 <errno.h> +#include <debug.h> + +#include <nuttx/irq.h> +#include <nuttx/arch.h> +#include <nuttx/semaphore.h> +#include <nuttx/kmalloc.h> +#include <arch/board/board.h> +#include <nuttx/i2c/i2c_master.h> +#include <nuttx/i2c/i2c_bitbang.h> + +/**************************************************************************** + * Pre-processor Definitions + ****************************************************************************/ + +/**************************************************************************** + * Private Types + ****************************************************************************/ + +struct i2c_bitbang_dev_s +{ + struct i2c_master_s i2c; + struct i2c_bitbang_lower_dev_s *lower; + +#ifdef CONFIG_I2C_BITBANG_NO_DELAY + uint32_t delay; +#endif +}; + +/**************************************************************************** + * Private Function Prototypes + ****************************************************************************/ + +static int i2c_bitbang_transfer(FAR struct i2c_master_s *dev, + FAR struct i2c_msg_s *msgs, int count); + +static int i2c_bitbang_set_scl(FAR struct i2c_bitbang_dev_s *dev, + bool high); +static void i2c_bitbang_set_sda(FAR struct i2c_bitbang_dev_s *dev, + bool high); + +static int i2c_bitbang_wait_ack(FAR struct i2c_bitbang_dev_s *dev); +static void i2c_bitbang_send(FAR struct i2c_bitbang_dev_s *dev, + uint8_t data); + +/**************************************************************************** + * Private Data + ****************************************************************************/ + +struct i2c_ops_s g_i2c_ops = +{ + .transfer = i2c_bitbang_transfer +}; + +/**************************************************************************** + * Private Functions + ****************************************************************************/ + +/**************************************************************************** + * Name: i2c_bitbang_transfer + ****************************************************************************/ + +static int i2c_bitbang_transfer(FAR struct i2c_master_s *dev, + FAR struct i2c_msg_s *msgs, int count) +{ + FAR struct i2c_bitbang_dev_s *priv = (FAR struct i2c_bitbang_dev_s *)dev; + int ret = OK; + int i; + + for (i = 0; i < count; i++) + { + uint8_t addr; + FAR struct i2c_msg_s *msg = &msgs[i]; + +#ifdef CONFIG_I2C_BITBANG_NO_DELAY + /* Compute delay from frequency */ + + priv->delay = ((1000000 / msg->frequency) / 2) - + CONFIG_I2C_BITBANG_GPIO_OVERHEAD; + + if (priv->delay < 0) + { + priv->delay = 0; + } +#endif + + /* Send start bit */ + + i2c_bitbang_set_scl(priv, true); + i2c_bitbang_set_sda(priv, false); + i2c_bitbang_set_scl(priv, false); + + /* Send the address */ + + addr = (msg->flags & I2C_M_READ ? I2C_READADDR8(msg->addr) : + I2C_WRITEADDR8(msg->addr)); + + i2c_bitbang_send(priv, addr); + + /* Wait for ACK */ + + ret = i2c_bitbang_wait_ack(priv); + + if (ret < 0) + { + return ret; + } + + i2c_bitbang_set_scl(priv, false); + + if (msgs->flags & I2C_M_READ) + { + int j; + + for (j = 0; j < msg->length; j++) + { + /* Send the data */ + + i2c_bitbang_send(priv, msg->buffer[j]); + + ret = i2c_bitbang_wait_ack(priv); + + if (ret < 0) + { + return ret; + } + + i2c_bitbang_set_scl(priv, false); + } + } + else + { + int j; + int k; + + for (j = 0; j < msg->length; j++) + { + uint8_t data = 0; + + i2c_bitbang_set_sda(priv, true); + + msg->buffer[j] = 0; + + for (k = 0; k < 8; k++) + { + i2c_bitbang_set_scl(priv, true); + data |= priv->lower->ops->get_sda(priv->lower) << (7 - k); Review comment: add a wrapper function for get_sda? ########## File path: arch/arm/src/nrf52/nrf52_i2c_bitbang.c ########## @@ -0,0 +1,147 @@ +/**************************************************************************** + * arch/arm/src/nrf52/nrf52_i2c_bitbang.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 <nuttx/i2c/i2c_master.h> +#include <nuttx/i2c/i2c_bitbang.h> +#include <nuttx/kmalloc.h> +#include "nrf52_gpio.h" + +/**************************************************************************** + * Pre-processor Definitions + ****************************************************************************/ + +/**************************************************************************** + * Private Types + ****************************************************************************/ + +struct nrf52_i2c_bitbang_dev_s +{ + struct i2c_bitbang_lower_dev_s lower; + nrf52_pinset_t sda_pin; + nrf52_pinset_t scl_pin; +}; + +/**************************************************************************** + * Private Function Prototypes + ****************************************************************************/ + +static void i2c_bb_initialize(FAR struct i2c_bitbang_lower_dev_s *lower); + +static void i2c_bb_set_scl(FAR struct i2c_bitbang_lower_dev_s *lower, + bool high); +static void i2c_bb_set_sda(FAR struct i2c_bitbang_lower_dev_s *lower, + bool high); + +static bool i2c_bb_get_scl(FAR struct i2c_bitbang_lower_dev_s *lower); +static bool i2c_bb_get_sda(FAR struct i2c_bitbang_lower_dev_s *lower); + +/**************************************************************************** + * Private Data + ****************************************************************************/ + +const static struct i2c_bitbang_lower_ops_s g_ops = +{ + .initialize = i2c_bb_initialize, + .set_scl = i2c_bb_set_scl, + .set_sda = i2c_bb_set_sda, + .get_scl = i2c_bb_get_scl, + .get_sda = i2c_bb_get_sda +}; + +/**************************************************************************** + * Public Data + ****************************************************************************/ + +/**************************************************************************** + * Private Functions + ****************************************************************************/ + +static void i2c_bb_initialize(FAR struct i2c_bitbang_lower_dev_s *lower) +{ + struct nrf52_i2c_bitbang_dev_s *dev = lower->priv; + + nrf52_gpio_config(GPIO_INPUT | GPIO_DRIVE_S0D1 | dev->scl_pin); + nrf52_gpio_config(GPIO_INPUT | GPIO_DRIVE_S0D1 | dev->sda_pin); +} + +static void i2c_bb_set_scl(FAR struct i2c_bitbang_lower_dev_s *lower, + bool high) +{ + struct nrf52_i2c_bitbang_dev_s *dev = lower->priv; + + nrf52_gpio_config(GPIO_OUTPUT | GPIO_DRIVE_S0D1 | + (high ? GPIO_VALUE_ONE : GPIO_VALUE_ZERO) | + dev->scl_pin); +} + +static void i2c_bb_set_sda(FAR struct i2c_bitbang_lower_dev_s *lower, + bool high) +{ + struct nrf52_i2c_bitbang_dev_s *dev = lower->priv; + + nrf52_gpio_config(GPIO_OUTPUT | GPIO_DRIVE_S0D1 | + (high ? GPIO_VALUE_ONE : GPIO_VALUE_ZERO) | + dev->sda_pin); +} + +static bool i2c_bb_get_scl(FAR struct i2c_bitbang_lower_dev_s *lower) +{ + struct nrf52_i2c_bitbang_dev_s *dev = lower->priv; + + nrf52_gpio_config(GPIO_INPUT | GPIO_DRIVE_S0D1 | dev->scl_pin); Review comment: should we write a generic gpio lower half for i2c bitbang driver thorugh ioexpander interface? So, other chip can share the same implementation. ########## File path: drivers/i2c/i2c_bitbang.c ########## @@ -0,0 +1,340 @@ +/**************************************************************************** + * arch/arm/src/nrf52/nrf52_i2c.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 <errno.h> +#include <debug.h> + +#include <nuttx/irq.h> +#include <nuttx/arch.h> +#include <nuttx/semaphore.h> +#include <nuttx/kmalloc.h> +#include <arch/board/board.h> +#include <nuttx/i2c/i2c_master.h> +#include <nuttx/i2c/i2c_bitbang.h> + +/**************************************************************************** + * Pre-processor Definitions + ****************************************************************************/ + +/**************************************************************************** + * Private Types + ****************************************************************************/ + +struct i2c_bitbang_dev_s +{ + struct i2c_master_s i2c; + struct i2c_bitbang_lower_dev_s *lower; + +#ifdef CONFIG_I2C_BITBANG_NO_DELAY + uint32_t delay; +#endif +}; + +/**************************************************************************** + * Private Function Prototypes + ****************************************************************************/ + +static int i2c_bitbang_transfer(FAR struct i2c_master_s *dev, + FAR struct i2c_msg_s *msgs, int count); + +static int i2c_bitbang_set_scl(FAR struct i2c_bitbang_dev_s *dev, + bool high); +static void i2c_bitbang_set_sda(FAR struct i2c_bitbang_dev_s *dev, + bool high); + +static int i2c_bitbang_wait_ack(FAR struct i2c_bitbang_dev_s *dev); +static void i2c_bitbang_send(FAR struct i2c_bitbang_dev_s *dev, + uint8_t data); + +/**************************************************************************** + * Private Data + ****************************************************************************/ + +struct i2c_ops_s g_i2c_ops = +{ + .transfer = i2c_bitbang_transfer +}; + +/**************************************************************************** + * Private Functions + ****************************************************************************/ + +/**************************************************************************** + * Name: i2c_bitbang_transfer + ****************************************************************************/ + +static int i2c_bitbang_transfer(FAR struct i2c_master_s *dev, + FAR struct i2c_msg_s *msgs, int count) +{ + FAR struct i2c_bitbang_dev_s *priv = (FAR struct i2c_bitbang_dev_s *)dev; + int ret = OK; + int i; + + for (i = 0; i < count; i++) + { + uint8_t addr; + FAR struct i2c_msg_s *msg = &msgs[i]; + +#ifdef CONFIG_I2C_BITBANG_NO_DELAY + /* Compute delay from frequency */ + + priv->delay = ((1000000 / msg->frequency) / 2) - + CONFIG_I2C_BITBANG_GPIO_OVERHEAD; + + if (priv->delay < 0) + { + priv->delay = 0; + } +#endif + + /* Send start bit */ + + i2c_bitbang_set_scl(priv, true); + i2c_bitbang_set_sda(priv, false); + i2c_bitbang_set_scl(priv, false); + + /* Send the address */ + + addr = (msg->flags & I2C_M_READ ? I2C_READADDR8(msg->addr) : + I2C_WRITEADDR8(msg->addr)); + + i2c_bitbang_send(priv, addr); + + /* Wait for ACK */ + + ret = i2c_bitbang_wait_ack(priv); + + if (ret < 0) + { + return ret; + } + + i2c_bitbang_set_scl(priv, false); + + if (msgs->flags & I2C_M_READ) + { + int j; + + for (j = 0; j < msg->length; j++) + { + /* Send the data */ + + i2c_bitbang_send(priv, msg->buffer[j]); + + ret = i2c_bitbang_wait_ack(priv); + + if (ret < 0) + { + return ret; + } + + i2c_bitbang_set_scl(priv, false); + } + } + else + { + int j; + int k; + + for (j = 0; j < msg->length; j++) + { + uint8_t data = 0; + + i2c_bitbang_set_sda(priv, true); + + msg->buffer[j] = 0; + + for (k = 0; k < 8; k++) + { + i2c_bitbang_set_scl(priv, true); + data |= priv->lower->ops->get_sda(priv->lower) << (7 - k); + i2c_bitbang_set_scl(priv, false); + } + + msg->buffer[j] = data; + + if (j < msg->length - 1) + { + /* Send ACK */ + + i2c_bitbang_set_sda(priv, false); + i2c_bitbang_set_scl(priv, true); + i2c_bitbang_set_scl(priv, false); + } + else + { + /* On the last byte send NAK */ + + i2c_bitbang_set_sda(priv, true); + i2c_bitbang_set_scl(priv, true); + i2c_bitbang_set_scl(priv, false); + } + } + } + + /* Send stop */ + + i2c_bitbang_set_sda(priv, false); + i2c_bitbang_set_scl(priv, true); + i2c_bitbang_set_sda(priv, true); + } + + return ret; +} + +/**************************************************************************** + * Name: i2c_bitbang_wait_ack + ****************************************************************************/ + +static int i2c_bitbang_wait_ack(FAR struct i2c_bitbang_dev_s *priv) +{ + int ret = OK; + int i; + + /* Wait for ACK */ + + i2c_bitbang_set_sda(priv, true); + i2c_bitbang_set_scl(priv, true); + + for (i = 0; priv->lower->ops->get_sda(priv->lower) && Review comment: call the wrapper function mentioned at line 173 ---------------------------------------------------------------- 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: us...@infra.apache.org