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 2020/11/03 13:48:22 UTC
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #2158: Linux I2C bus support in sim
xiaoxiang781216 commented on a change in pull request #2158:
URL: https://github.com/apache/incubator-nuttx/pull/2158#discussion_r516055578
##########
File path: arch/sim/src/sim/up_linuxi2cbus.c
##########
@@ -0,0 +1,332 @@
+/****************************************************************************
+ * arch/sim/src/sim/up_linuxi2cbus.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 <sys/types.h>
+#include <sys/ioctl.h>
+
+#include <stdbool.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <syslog.h>
+#include <unistd.h>
+#include <fcntl.h>
+
+#include "up_linuxi2cbus.h"
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+#define ERROR(fmt, ...) \
+ syslog(LOG_ERR, "up_linuxi2c_host: " fmt "\n", ##__VA_ARGS__)
+#define INFO(fmt, ...) \
+ syslog(LOG_ERR, "up_linuxi2c_host: " fmt "\n", ##__VA_ARGS__)
+#define DEBUG(fmt, ...)
+
+/* Define the Linux IOCTL interfaces again here so we do not require having
+ * the kernel headers to build this. This interface should be fairly
+ * stable so it should not cause any issues. The naming is preserved in case
+ * we want to change it in the future.
+ */
+
+/* Linux msg flags */
Review comment:
it's better to include linux/i2c-dev.h to avoid the duplicated definition
##########
File path: arch/sim/src/sim/up_i2cbus.c
##########
@@ -0,0 +1,153 @@
+/****************************************************************************
+ * arch/sim/src/sim/up_i2cbus.c
Review comment:
don't need up_i2cbus.c, let up_linuxi2cbus.c implement sim_i2cbus_initialize/sim_i2cbus_uninitialize directly.
##########
File path: boards/sim/sim/sim/Kconfig
##########
@@ -54,3 +54,20 @@ config SIM_WTGAHRS2_UARTN
We can select the number accoding to which SIM_UARTX_NAME is uesd to sensor.
This range is 0-4.
endif
+
+config SIM_I2CBUS
Review comment:
should we follow SIM_NETDEV?
```
config SIM_I2CBUS
bool
if SIM_I2CBUS
choice
default CONFIG_SIM_LINUXI2CBUS
config CONFIG_SIM_LINUXI2CBUS
bool
endchoice
endif
```
##########
File path: arch/sim/src/Makefile
##########
@@ -198,6 +198,14 @@ ifeq ($(CONFIG_SIM_HCISOCKET),y)
CSRCS += up_hcisocket.c
endif
+ifeq ($(CONFIG_SIM_LINUXI2CBUS),y)
+ HOSTSRCS += up_linuxi2cbus.c
Review comment:
change to up_i2cbuslinux.c?
##########
File path: arch/sim/src/sim/up_linuxi2cbus.c
##########
@@ -0,0 +1,332 @@
+/****************************************************************************
+ * arch/sim/src/sim/up_linuxi2cbus.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 <sys/types.h>
+#include <sys/ioctl.h>
+
+#include <stdbool.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <syslog.h>
+#include <unistd.h>
+#include <fcntl.h>
+
+#include "up_linuxi2cbus.h"
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+#define ERROR(fmt, ...) \
+ syslog(LOG_ERR, "up_linuxi2c_host: " fmt "\n", ##__VA_ARGS__)
+#define INFO(fmt, ...) \
+ syslog(LOG_ERR, "up_linuxi2c_host: " fmt "\n", ##__VA_ARGS__)
+#define DEBUG(fmt, ...)
+
+/* Define the Linux IOCTL interfaces again here so we do not require having
+ * the kernel headers to build this. This interface should be fairly
+ * stable so it should not cause any issues. The naming is preserved in case
+ * we want to change it in the future.
+ */
+
+/* Linux msg flags */
+
+#define I2C_M_RD 0x0001
+#define I2C_M_TEN 0x0010
+#define I2C_M_DMA_SAFE 0x0200
+#define I2C_M_RECV_LEN 0x0400
+#define I2C_M_NO_RD_ACK 0x0800
+#define I2C_M_IGNORE_NAK 0x1000
+#define I2C_M_REV_DIR_ADDR 0x2000
+#define I2C_M_NOSTART 0x4000
+#define I2C_M_STOP 0x8000
+
+/* Linux ioctl */
+
+#define I2C_RETRIES 0x0701
+#define I2C_TIMEOUT 0x0702
+#define I2C_SLAVE 0x0703
+#define I2C_SLAVE_FORCE 0x0706
+#define I2C_TENBIT 0x0704
+#define I2C_FUNCS 0x0705
+#define I2C_RDWR 0x0707
+#define I2C_PEC 0x0708
+#define I2C_SMBUS 0x0720
+
+/* NuttX msg flags (see i2c_master.h) */
Review comment:
it's better to create a header file and copy the required info from nuttx/i2c/i2c_master.h to it, then other host can reuse the definition.
##########
File path: arch/sim/Kconfig
##########
@@ -504,6 +504,21 @@ config SIM_HCISOCKET
control of the device, but is abstracted from the
physical interface which is still handled by Linux.
+config SIM_LINUXI2CBUS
Review comment:
change to SIM_I2CBUS_LINUX?
##########
File path: arch/sim/src/sim/up_i2cbus.c
##########
@@ -0,0 +1,153 @@
+/****************************************************************************
+ * arch/sim/src/sim/up_i2cbus.c
Review comment:
Actually, the implementation isn't much difference between an i2c hardware driver and an i2c linux/macOS wrapper driver. The key point is that i2c linux/macOS wrapper driver can't include <nuttx/i2c/i2c_master.h> directly, so the natural solution is:
1. Create a header file contain required i2c macro/callback/strcuture
2. Let up_i2cbuslinux.c or up_i2cbusmacos.c include this header file and implement the callback
3. Implement sim_i2c_initialize and sim_i2c_uninitialize
4. Add Kconfig option to select the source file to be built
In step 2, we normally include the host specific header file(e.g. linux/i2c-dev.h) and map NuttX API to host API. Since you can do what ever you want in up_i2cbuslinux.c or up_i2cbusmacos.c, it's redundant to add another layer(e.g. up_i2cbus.c).
##########
File path: arch/sim/src/sim/up_i2cbus.c
##########
@@ -0,0 +1,153 @@
+/****************************************************************************
+ * arch/sim/src/sim/up_i2cbus.c
Review comment:
Actually, the implementation isn't much difference between an i2c hardware driver and an i2c linux/macOS wrapper driver. The key point is that i2c linux/macOS wrapper driver can't include <nuttx/i2c/i2c_master.h> directly, so the natural solution is:
1. Create a header file duplicate the required i2c macro/callback/strcuture
2. Let up_i2cbuslinux.c or up_i2cbusmacos.c include this header file and implement the callback
3. Implement sim_i2c_initialize and sim_i2c_uninitialize
4. Add Kconfig option to select the source file to be built
In step 2, we normally include the host specific header file(e.g. linux/i2c-dev.h) and map NuttX API to host API. Since you can do what ever you want in up_i2cbuslinux.c or up_i2cbusmacos.c, it's redundant to add another layer(e.g. up_i2cbus.c).
##########
File path: arch/sim/src/sim/up_linuxi2cbus.c
##########
@@ -0,0 +1,332 @@
+/****************************************************************************
+ * arch/sim/src/sim/up_linuxi2cbus.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 <sys/types.h>
+#include <sys/ioctl.h>
+
+#include <stdbool.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <syslog.h>
+#include <unistd.h>
+#include <fcntl.h>
+
+#include "up_linuxi2cbus.h"
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+#define ERROR(fmt, ...) \
+ syslog(LOG_ERR, "up_linuxi2c_host: " fmt "\n", ##__VA_ARGS__)
+#define INFO(fmt, ...) \
+ syslog(LOG_ERR, "up_linuxi2c_host: " fmt "\n", ##__VA_ARGS__)
+#define DEBUG(fmt, ...)
+
+/* Define the Linux IOCTL interfaces again here so we do not require having
+ * the kernel headers to build this. This interface should be fairly
+ * stable so it should not cause any issues. The naming is preserved in case
+ * we want to change it in the future.
+ */
+
+/* Linux msg flags */
+
+#define I2C_M_RD 0x0001
+#define I2C_M_TEN 0x0010
+#define I2C_M_DMA_SAFE 0x0200
+#define I2C_M_RECV_LEN 0x0400
+#define I2C_M_NO_RD_ACK 0x0800
+#define I2C_M_IGNORE_NAK 0x1000
+#define I2C_M_REV_DIR_ADDR 0x2000
+#define I2C_M_NOSTART 0x4000
+#define I2C_M_STOP 0x8000
+
+/* Linux ioctl */
+
+#define I2C_RETRIES 0x0701
+#define I2C_TIMEOUT 0x0702
+#define I2C_SLAVE 0x0703
+#define I2C_SLAVE_FORCE 0x0706
+#define I2C_TENBIT 0x0704
+#define I2C_FUNCS 0x0705
+#define I2C_RDWR 0x0707
+#define I2C_PEC 0x0708
+#define I2C_SMBUS 0x0720
+
+/* NuttX msg flags (see i2c_master.h) */
Review comment:
Sorry, I mean Linux specific macro or function, not NuttX specific. NuttX definition could be shared by different host implementation, a common header file coud avoid the duplication. On the other hand, Linux definition is only used by up_linuxi2cbus.c, so it's better to put in source file directly.
##########
File path: arch/sim/src/sim/up_linuxi2cbus.c
##########
@@ -0,0 +1,332 @@
+/****************************************************************************
+ * arch/sim/src/sim/up_linuxi2cbus.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 <sys/types.h>
+#include <sys/ioctl.h>
+
+#include <stdbool.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <syslog.h>
+#include <unistd.h>
+#include <fcntl.h>
+
+#include "up_linuxi2cbus.h"
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+#define ERROR(fmt, ...) \
+ syslog(LOG_ERR, "up_linuxi2c_host: " fmt "\n", ##__VA_ARGS__)
+#define INFO(fmt, ...) \
+ syslog(LOG_ERR, "up_linuxi2c_host: " fmt "\n", ##__VA_ARGS__)
+#define DEBUG(fmt, ...)
+
+/* Define the Linux IOCTL interfaces again here so we do not require having
+ * the kernel headers to build this. This interface should be fairly
+ * stable so it should not cause any issues. The naming is preserved in case
+ * we want to change it in the future.
+ */
+
+/* Linux msg flags */
+
+#define I2C_M_RD 0x0001
+#define I2C_M_TEN 0x0010
+#define I2C_M_DMA_SAFE 0x0200
+#define I2C_M_RECV_LEN 0x0400
+#define I2C_M_NO_RD_ACK 0x0800
+#define I2C_M_IGNORE_NAK 0x1000
+#define I2C_M_REV_DIR_ADDR 0x2000
+#define I2C_M_NOSTART 0x4000
+#define I2C_M_STOP 0x8000
+
+/* Linux ioctl */
+
+#define I2C_RETRIES 0x0701
+#define I2C_TIMEOUT 0x0702
+#define I2C_SLAVE 0x0703
+#define I2C_SLAVE_FORCE 0x0706
+#define I2C_TENBIT 0x0704
+#define I2C_FUNCS 0x0705
+#define I2C_RDWR 0x0707
+#define I2C_PEC 0x0708
+#define I2C_SMBUS 0x0720
+
+/* NuttX msg flags (see i2c_master.h) */
Review comment:
Sorry, I mean Linux specific macro or function, not NuttX specific. NuttX definition could be shared by different host implementation, a common header file can avoid the duplication. On the other hand, Linux definition is only used by up_linuxi2cbus.c, so it's better to put in source file directly.
##########
File path: arch/sim/src/sim/up_linuxi2cbus.c
##########
@@ -0,0 +1,332 @@
+/****************************************************************************
+ * arch/sim/src/sim/up_linuxi2cbus.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 <sys/types.h>
+#include <sys/ioctl.h>
+
+#include <stdbool.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <syslog.h>
+#include <unistd.h>
+#include <fcntl.h>
+
+#include "up_linuxi2cbus.h"
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+#define ERROR(fmt, ...) \
+ syslog(LOG_ERR, "up_linuxi2c_host: " fmt "\n", ##__VA_ARGS__)
+#define INFO(fmt, ...) \
+ syslog(LOG_ERR, "up_linuxi2c_host: " fmt "\n", ##__VA_ARGS__)
+#define DEBUG(fmt, ...)
+
+/* Define the Linux IOCTL interfaces again here so we do not require having
+ * the kernel headers to build this. This interface should be fairly
+ * stable so it should not cause any issues. The naming is preserved in case
+ * we want to change it in the future.
+ */
+
+/* Linux msg flags */
Review comment:
Ok, but as you said this interface is very stable, why not include it directly:)? Since i2c interface isn't too complex and stable, the manual redeclaration is workable, but how about XWindow or ALSA which have many API and complex struct?
##########
File path: arch/sim/src/sim/up_linuxi2cbus.c
##########
@@ -0,0 +1,332 @@
+/****************************************************************************
+ * arch/sim/src/sim/up_linuxi2cbus.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 <sys/types.h>
+#include <sys/ioctl.h>
+
+#include <stdbool.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <syslog.h>
+#include <unistd.h>
+#include <fcntl.h>
+
+#include "up_linuxi2cbus.h"
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+#define ERROR(fmt, ...) \
+ syslog(LOG_ERR, "up_linuxi2c_host: " fmt "\n", ##__VA_ARGS__)
+#define INFO(fmt, ...) \
+ syslog(LOG_ERR, "up_linuxi2c_host: " fmt "\n", ##__VA_ARGS__)
+#define DEBUG(fmt, ...)
+
+/* Define the Linux IOCTL interfaces again here so we do not require having
+ * the kernel headers to build this. This interface should be fairly
+ * stable so it should not cause any issues. The naming is preserved in case
+ * we want to change it in the future.
+ */
+
+/* Linux msg flags */
Review comment:
Ok, but as you said this interface is very stable, why not include it directly to save the typing:)? Since i2c interface isn't too complex and stable, the manual redeclaration is workable, but how about XWindow or ALSA which have many API and complex struct?
##########
File path: arch/sim/src/sim/up_linuxi2cbus.c
##########
@@ -0,0 +1,332 @@
+/****************************************************************************
+ * arch/sim/src/sim/up_linuxi2cbus.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 <sys/types.h>
+#include <sys/ioctl.h>
+
+#include <stdbool.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <syslog.h>
+#include <unistd.h>
+#include <fcntl.h>
+
+#include "up_linuxi2cbus.h"
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+#define ERROR(fmt, ...) \
+ syslog(LOG_ERR, "up_linuxi2c_host: " fmt "\n", ##__VA_ARGS__)
+#define INFO(fmt, ...) \
+ syslog(LOG_ERR, "up_linuxi2c_host: " fmt "\n", ##__VA_ARGS__)
+#define DEBUG(fmt, ...)
+
+/* Define the Linux IOCTL interfaces again here so we do not require having
+ * the kernel headers to build this. This interface should be fairly
+ * stable so it should not cause any issues. The naming is preserved in case
+ * we want to change it in the future.
+ */
+
+/* Linux msg flags */
+
+#define I2C_M_RD 0x0001
+#define I2C_M_TEN 0x0010
+#define I2C_M_DMA_SAFE 0x0200
+#define I2C_M_RECV_LEN 0x0400
+#define I2C_M_NO_RD_ACK 0x0800
+#define I2C_M_IGNORE_NAK 0x1000
+#define I2C_M_REV_DIR_ADDR 0x2000
+#define I2C_M_NOSTART 0x4000
+#define I2C_M_STOP 0x8000
+
+/* Linux ioctl */
+
+#define I2C_RETRIES 0x0701
+#define I2C_TIMEOUT 0x0702
+#define I2C_SLAVE 0x0703
+#define I2C_SLAVE_FORCE 0x0706
+#define I2C_TENBIT 0x0704
+#define I2C_FUNCS 0x0705
+#define I2C_RDWR 0x0707
+#define I2C_PEC 0x0708
+#define I2C_SMBUS 0x0720
+
+/* NuttX msg flags (see i2c_master.h) */
Review comment:
Yes, that's one of reason we can't directly use nuttx/i2c/i2c_master.h, here is the same approach for hostfs:
https://github.com/apache/incubator-nuttx/blob/master/include/nuttx/fs/hostfs.h
##########
File path: arch/sim/src/sim/up_i2cbuslinux.c
##########
@@ -0,0 +1,288 @@
+/****************************************************************************
+ * arch/sim/src/sim/up_i2cbuslinux.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 <sys/types.h>
+#include <sys/ioctl.h>
+
+#include <stdbool.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <syslog.h>
+#include <unistd.h>
+#include <fcntl.h>
+
+#include <linux/i2c-dev.h>
+#include <linux/i2c.h>
+
+#include "up_i2cbus.h"
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+#define ERROR(fmt, ...) \
+ syslog(LOG_ERR, "up_i2cbuslinux: " fmt "\n", ##__VA_ARGS__)
+#define INFO(fmt, ...) \
+ syslog(LOG_ERR, "up_i2cbuslinux: " fmt "\n", ##__VA_ARGS__)
+#define DEBUG(fmt, ...)
+
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+struct linux_i2cbus_master_s
+{
+ const struct i2c_ops_s *ops; /* I2C vtable */
+ int file;
+};
+
+/****************************************************************************
+ * Private Function Prototypes
+ ****************************************************************************/
+
+int linux_i2cbus_transfer_host(struct i2c_master_s *dev,
Review comment:
add static
##########
File path: arch/sim/Kconfig
##########
@@ -515,6 +515,36 @@ config SIM_HCISOCKET
control of the device, but is abstracted from the
physical interface which is still handled by Linux.
+config SIM_I2CBUS
+ bool "Simulated I2C Bus"
+ default y
Review comment:
change to n, or depeend on I2C
##########
File path: arch/sim/src/sim/up_i2cbuslinux.c
##########
@@ -0,0 +1,288 @@
+/****************************************************************************
+ * arch/sim/src/sim/up_i2cbuslinux.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 <sys/types.h>
+#include <sys/ioctl.h>
+
+#include <stdbool.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <syslog.h>
+#include <unistd.h>
+#include <fcntl.h>
+
+#include <linux/i2c-dev.h>
+#include <linux/i2c.h>
+
+#include "up_i2cbus.h"
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+#define ERROR(fmt, ...) \
+ syslog(LOG_ERR, "up_i2cbuslinux: " fmt "\n", ##__VA_ARGS__)
+#define INFO(fmt, ...) \
+ syslog(LOG_ERR, "up_i2cbuslinux: " fmt "\n", ##__VA_ARGS__)
+#define DEBUG(fmt, ...)
+
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+struct linux_i2cbus_master_s
+{
+ const struct i2c_ops_s *ops; /* I2C vtable */
+ int file;
+};
+
+/****************************************************************************
+ * Private Function Prototypes
+ ****************************************************************************/
+
+int linux_i2cbus_transfer_host(struct i2c_master_s *dev,
+ struct i2c_msg_s *msgs, int count);
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+struct i2c_ops_s i2c_linux_ops =
+{
+ .transfer = linux_i2cbus_transfer_host,
+};
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: linux_i2cbus_transfer_host
+ *
+ * Description:
+ * Provide i2c transfer
+ *
+ ****************************************************************************/
+
+int linux_i2cbus_transfer_host(struct i2c_master_s *dev,
Review comment:
add static move to private function section
##########
File path: arch/sim/src/sim/up_i2cbus.h
##########
@@ -0,0 +1,76 @@
+/****************************************************************************
+ * arch/sim/src/sim/up_linuxi2cbus.h
Review comment:
change to up_i2cbus.h
##########
File path: arch/sim/src/sim/up_i2cbuslinux.c
##########
@@ -0,0 +1,288 @@
+/****************************************************************************
+ * arch/sim/src/sim/up_i2cbuslinux.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 <sys/types.h>
+#include <sys/ioctl.h>
+
+#include <stdbool.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <syslog.h>
+#include <unistd.h>
+#include <fcntl.h>
+
+#include <linux/i2c-dev.h>
+#include <linux/i2c.h>
+
+#include "up_i2cbus.h"
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+#define ERROR(fmt, ...) \
+ syslog(LOG_ERR, "up_i2cbuslinux: " fmt "\n", ##__VA_ARGS__)
+#define INFO(fmt, ...) \
+ syslog(LOG_ERR, "up_i2cbuslinux: " fmt "\n", ##__VA_ARGS__)
+#define DEBUG(fmt, ...)
+
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+struct linux_i2cbus_master_s
+{
+ const struct i2c_ops_s *ops; /* I2C vtable */
+ int file;
+};
+
+/****************************************************************************
+ * Private Function Prototypes
+ ****************************************************************************/
+
+int linux_i2cbus_transfer_host(struct i2c_master_s *dev,
+ struct i2c_msg_s *msgs, int count);
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+struct i2c_ops_s i2c_linux_ops =
+{
+ .transfer = linux_i2cbus_transfer_host,
+};
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: linux_i2cbus_transfer_host
+ *
+ * Description:
+ * Provide i2c transfer
+ *
+ ****************************************************************************/
+
+int linux_i2cbus_transfer_host(struct i2c_master_s *dev,
+ struct i2c_msg_s *msgs, int count)
+{
+ int ret = 0;
+ struct linux_i2cbus_master_s *priv = (struct linux_i2cbus_master_s *)dev;
+ int file = priv->file;
+ uint8_t *pack_buf = NULL;
+ struct i2c_rdwr_ioctl_data ioctl_data;
+ struct i2c_msg l_msgs[2]; /* We only support up to 2 messages */
Review comment:
there are one to one mapping between Linux and NuttX i2c msg defintion, why not fully convert like this?
```
struct i2c_msg l_msgs[count];
for (i = 0; i < count; i++)
{
l_msgs[i].addr = msgs[i].addr;
l_msgs[i].buf = msgs[i].buffer;
l_msgs[i].len = msgs[i].length;
l_msgs[i].flags = 0;
if (msgs[i].flags & NUTTX_I2C_M_READ)
{
l_msgs[i].flags |= I2C_M_RD;
}
if (msgs[i].flags & NUTTX_I2C_M_TEN)
{
l_msgs[i].flags |= I2C_M_TEN;
}
if (msgs[i].flags & NUTTX_I2C_M_NOSTART)
{
l_msgs[i].flags |= I2C_M_NOSTART;
}
if (!(msgs[i].flags & I2C_M_NOSTOP))
{
l_msgs[i].flags |= I2C_M_STOP;
}
}
```
##########
File path: arch/sim/src/sim/up_i2cbuslinux.c
##########
@@ -0,0 +1,288 @@
+/****************************************************************************
+ * arch/sim/src/sim/up_i2cbuslinux.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 <sys/types.h>
+#include <sys/ioctl.h>
+
+#include <stdbool.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <syslog.h>
+#include <unistd.h>
+#include <fcntl.h>
+
+#include <linux/i2c-dev.h>
+#include <linux/i2c.h>
+
+#include "up_i2cbus.h"
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+#define ERROR(fmt, ...) \
+ syslog(LOG_ERR, "up_i2cbuslinux: " fmt "\n", ##__VA_ARGS__)
+#define INFO(fmt, ...) \
+ syslog(LOG_ERR, "up_i2cbuslinux: " fmt "\n", ##__VA_ARGS__)
+#define DEBUG(fmt, ...)
+
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+struct linux_i2cbus_master_s
+{
+ const struct i2c_ops_s *ops; /* I2C vtable */
+ int file;
+};
+
+/****************************************************************************
+ * Private Function Prototypes
+ ****************************************************************************/
+
+int linux_i2cbus_transfer_host(struct i2c_master_s *dev,
+ struct i2c_msg_s *msgs, int count);
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+struct i2c_ops_s i2c_linux_ops =
+{
+ .transfer = linux_i2cbus_transfer_host,
+};
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: linux_i2cbus_transfer_host
+ *
+ * Description:
+ * Provide i2c transfer
+ *
+ ****************************************************************************/
+
+int linux_i2cbus_transfer_host(struct i2c_master_s *dev,
+ struct i2c_msg_s *msgs, int count)
+{
+ int ret = 0;
+ struct linux_i2cbus_master_s *priv = (struct linux_i2cbus_master_s *)dev;
+ int file = priv->file;
+ uint8_t *pack_buf = NULL;
+ struct i2c_rdwr_ioctl_data ioctl_data;
+ struct i2c_msg l_msgs[2]; /* We only support up to 2 messages */
+ ioctl_data.msgs = l_msgs;
+
+ /* Many i2c bus do not play well with combined messages via the Linux
+ * interface, this makes stitching things together a little harder
+ * because NuttX provides the ability to hold the bus without ending
+ * with a STOP which is not ideal in general, and not possible with
+ * Linux.
+ */
+
+ if ((msgs[0].flags & NUTTX_I2C_M_TEN) || (msgs[1].flags & NUTTX_I2C_M_TEN))
+ {
+ /* Linux also has somewhat poor support for 10bit addresses and they
+ * are quite rare so we just don't support them for now here
+ */
+
+ return -1;
+ }
+
+ ioctl_data.msgs = l_msgs;
+
+ if (count == 1)
+ {
+ if (msgs[0].flags & NUTTX_I2C_M_NOSTOP || \
+ msgs[0].flags & NUTTX_I2C_M_NOSTART)
+ {
+ /* Do not support leaving the bus hanging or try to send
+ * without first starting.
+ */
+
+ return -1;
+ }
+
+ l_msgs[0].addr = msgs[0].addr;
+ l_msgs[0].buf = msgs[0].buffer;
+ l_msgs[0].len = msgs[0].length;
+ l_msgs[0].flags = 0;
+ if (msgs[0].flags & NUTTX_I2C_M_READ)
+ {
+ l_msgs[0].flags |= I2C_M_RD;
+ }
+
+ ioctl_data.nmsgs = 1;
+ }
+ else if(count == 2)
+ {
+ /* Addresses should be the same */
+
+ if (msgs[0].addr != msgs[1].addr)
+ {
+ return -1;
+ }
+
+ /* Check if we are about to do a read of a register
+ * NuttX interface represents this as WRITE(NOSTOP) + READ
+ * Linux interface represents this as READ + READ
+ */
+
+ if (msgs[0].flags & NUTTX_I2C_M_NOSTOP && \
+ msgs[1].flags & NUTTX_I2C_M_READ)
+ {
+ l_msgs[0].addr = msgs[0].addr;
+ l_msgs[0].flags = 0;
+ l_msgs[0].buf = msgs[0].buffer;
+ l_msgs[0].len = msgs[0].length;
+
+ l_msgs[1].addr = msgs[1].addr;
+ l_msgs[1].flags = I2C_M_RD;
+ l_msgs[1].buf = msgs[1].buffer;
+ l_msgs[1].len = msgs[1].length;
+ ioctl_data.nmsgs = 2;
+ }
+ else if (!(msgs[0].flags & NUTTX_I2C_M_READ) && \
+ !(msgs[1].flags & NUTTX_I2C_M_READ) && \
+ (msgs[0].flags & NUTTX_I2C_M_NOSTOP) && \
+ (msgs[1].flags & NUTTX_I2C_M_NOSTART))
+ {
+ /* These writes are actually just a single write in Linux
+ * so we pack the data in a single buffer and the unpack
+ * it at the end. This could support for for more than just 2
+ * messages, but in most cases it is just two because it is
+ * connivent to write the register address and the data into two
+ * different buffers.
+ */
+
+ pack_buf = malloc(msgs[0].length + msgs[1].length);
+ if (pack_buf == NULL)
+ {
+ return -1;
+ }
+
+ memcpy(pack_buf, msgs[0].buffer, msgs[0].length);
+ memcpy(pack_buf + msgs[0].length, msgs[1].buffer, msgs[1].length);
+ l_msgs[0].len = msgs[0].length + msgs[1].length;
+ l_msgs[0].flags = 0;
+ l_msgs[0].addr = msgs[0].addr;
+ ioctl_data.msgs[0].buf = pack_buf;
+ ioctl_data.nmsgs = 1;
+ }
+ else
+ {
+ /* Many busses cannot handle more than 2 messages */
+
+ return -1;
+ }
+ }
+ else
+ {
+ return -1;
+ }
+
+ if (ioctl(file, I2C_RDWR, &ioctl_data) < 1)
+ {
+ ret = -1;
+ }
+
+ /* Unpack from buffer back to msg buffers if needed */
+
+ if (pack_buf != NULL)
+ {
+ if (ret == 0)
+ {
+ int idx;
+ uint8_t *msg_p = pack_buf;
+ for (idx = 0; idx < count; idx++)
+ {
+ memcpy(msgs[idx].buffer, msg_p, msgs[idx].length);
+ msg_p += msgs[idx].length;
+ }
+ }
+
+ free(pack_buf);
+ }
+
+ return ret;
+}
+
+/****************************************************************************
+ * Name: sim_i2cbus_initialize
+ *
+ * Description:
+ * Initialize one I2C bus
+ *
+ ****************************************************************************/
+
+struct i2c_master_s *sim_i2cbus_initialize(int bus)
+{
+ struct linux_i2cbus_master_s *priv;
+ char filename[20];
+
+ priv = (struct linux_i2cbus_master_s *)malloc(sizeof(priv));
+ if (priv == NULL)
+ {
+ ERROR("Failed to allocate private i2c master driver");
+ return NULL;
+ }
+
+ snprintf(filename, 19, "/dev/i2c-%d", bus);
+ priv->file = open(filename, O_RDWR);
+ if (priv->file < 0)
+ {
+ ERROR("Failed to open %s: %d", filename, priv->file);
+ free(priv);
+ return NULL;
+ }
+
+ priv->ops = &i2c_linux_ops;
+ return (struct i2c_master_s *)priv;
+}
+
+/****************************************************************************
+ * Name: sim_i2cbus_uninitialize
+ *
+ * Description:
+ * Uninitialize an I2C bus
+ *
+ ****************************************************************************/
+
+int sim_i2cbus_uninitialize(struct i2c_master_s *dev)
+{
+ struct linux_i2cbus_master_s *priv = (struct linux_i2cbus_master_s *)dev;
+ if (priv->file >= 0)
+ {
+ close(priv->file);
+ }
+
Review comment:
free(priv)
##########
File path: arch/sim/src/sim/up_i2cbuslinux.c
##########
@@ -0,0 +1,288 @@
+/****************************************************************************
+ * arch/sim/src/sim/up_i2cbuslinux.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 <sys/types.h>
+#include <sys/ioctl.h>
+
+#include <stdbool.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <syslog.h>
+#include <unistd.h>
+#include <fcntl.h>
+
+#include <linux/i2c-dev.h>
+#include <linux/i2c.h>
+
+#include "up_i2cbus.h"
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+#define ERROR(fmt, ...) \
+ syslog(LOG_ERR, "up_i2cbuslinux: " fmt "\n", ##__VA_ARGS__)
+#define INFO(fmt, ...) \
+ syslog(LOG_ERR, "up_i2cbuslinux: " fmt "\n", ##__VA_ARGS__)
+#define DEBUG(fmt, ...)
+
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+struct linux_i2cbus_master_s
+{
+ const struct i2c_ops_s *ops; /* I2C vtable */
+ int file;
+};
+
+/****************************************************************************
+ * Private Function Prototypes
+ ****************************************************************************/
+
+int linux_i2cbus_transfer_host(struct i2c_master_s *dev,
+ struct i2c_msg_s *msgs, int count);
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+struct i2c_ops_s i2c_linux_ops =
Review comment:
add static
----------------------------------------------------------------
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