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/06/28 15:35:55 UTC
[GitHub] [incubator-nuttx] mlyszczek opened a new pull request, #6537: [API review request] drivers/ipcc: add Inter Processor Communication driver
mlyszczek opened a new pull request, #6537:
URL: https://github.com/apache/incubator-nuttx/pull/6537
This is proposal of new driver for IPCC (inter processor communication controller).
This is still work in progress but I would like to request small review now. Mainly I would like you to review interfaces as this is common code for all architectures.
The only test I did was to properly compile it.
I will be implementing lower half of the driver now, and it would be cool to have at least interfaces reviewed before I do all the stuff and will have to change half of the code :)
I've implemented IPCC as character device, very similar to ordinary UART with separate tx and rx (full duplex). It's interrupt driven - lower half driver will clear/fill buffers (if buffering is enabled) and notify blocked/polling threads. It should be enough to simply call ipcc_register(0); to register single full duplex /dev/ipcc0 device. Then simple open/read/write to get/send data.
It's a lower half driver responsibility to prepare (reserve) memory for IPCC. arch/ part will also have to take IPCC into account when passing heap memory for OS - reserved memory should not be exposed to OS as it's shared between CPUs which can have different OSes on them. Lower layer will also be responsible to send message in proper format, like
struct { uint32_t buflen; unsigned char buf[] };
--
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] mlyszczek commented on a diff in pull request #6537: drivers/ipcc: add Inter Processor Communication driver
Posted by GitBox <gi...@apache.org>.
mlyszczek commented on code in PR #6537:
URL: https://github.com/apache/incubator-nuttx/pull/6537#discussion_r923173997
##########
drivers/ipcc/ipcc_register.c:
##########
@@ -0,0 +1,208 @@
+/****************************************************************************
+ * drivers/ipcc/ipcc_register.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/ipcc.h>
+#include <nuttx/kmalloc.h>
+#include <nuttx/mm/circbuf.h>
+
+#include <assert.h>
+#include <errno.h>
+#include <stdbool.h>
+#include <stdio.h>
+#include <sys/types.h>
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/* Device naming ************************************************************/
+
+#define DEVNAME_FMT "/dev/ipcc%d"
+#define DEVNAME_FMTLEN (9 + 3 + 1)
+
+/****************************************************************************
+ * Private Function Prototypes
+ ****************************************************************************/
+
+#ifndef CONFIG_DISABLE_PSEUDOFS_OPERATIONS
+int ipcc_open(FAR struct file *filep);
+int ipcc_close(FAR struct file *filep);
+int ipcc_unlink(FAR struct inode *inode);
+#endif
+int ipcc_ioctl(FAR struct file *filep, int cmd, unsigned long arg);
+ssize_t ipcc_read(FAR struct file *filep, FAR char *buffer, size_t buflen);
+ssize_t ipcc_write(FAR struct file *filep, FAR const char *buffer,
+ size_t buflen);
+int ipcc_poll(FAR struct file *filep, FAR struct pollfd *fds,
+ bool setup);
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+static const struct file_operations ipcc_fops =
+{
+#ifdef CONFIG_DISABLE_PSEUDOFS_OPERATIONS
+ .open = NULL,
+ .close = NULL,
+ .unlink = NULL,
+#else /* CONFIG_DISABLE_PSEUDOFS_OPERATIONS */
+ .open = ipcc_open,
+ .close = ipcc_close,
+ .unlink = ipcc_unlink,
+#endif /* CONFIG_DISABLE_PSEUDOFS_OPERATIONS */
+ .poll = ipcc_poll,
+ .read = ipcc_read,
+ .write = ipcc_write,
+ .ioctl = NULL,
+ .seek = NULL
+};
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: ipcc_cleanup
+ *
+ * Description:
+ * Cleans up resources allocated by ipcc_register()
+ *
+ * Input Parameters:
+ * priv - ipcc driver instance to clean up
+ *
+ * Returned Value:
+ * None
+ *
+ * Assumptions/Limitations:
+ * This function should be called only when ipcc_register is run with
+ * success and all resources in driver instance are properly allocated.
+ *
+ ****************************************************************************/
+
+void ipcc_cleanup(FAR struct ipcc_driver_s *priv)
+{
+#ifdef CONFIG_IPCC_BUFFERED
+ circbuf_uninit(&priv->ipcc.rxbuf);
+ circbuf_uninit(&priv->ipcc.txbuf);
+#endif
+ nxsem_destroy(&priv->ipcc.rxsem);
+ nxsem_destroy(&priv->ipcc.txsem);
+ up_ipcc_cleanup(&priv->ipcc);
+ nxsem_destroy(&priv->exclsem);
+ kmm_free(priv);
+}
+
+/****************************************************************************
+ * Name: ipcc_register
+ *
+ * Description:
+ * Create and register the IPCC character driver.
+ *
+ * IPCC is a simple character driver that supports inter processor
+ * communication.
+ *
+ * Input Parameters:
+ * ipcc - An instance of the lower half IPCC driver
+ * chan - IPCC channel. This will be used ad IPCC minor number.
+ * IPPC will be registered as /dev/ipccN where N is the minor number.
+ * buflen - Length of tx and rx buffers, 0 for unbuffered communication.
+ *
+ * Returned Value:
+ * OK if the driver was successfully registered, or negated errno on
+ * failure.
+ *
+ * Assumptions/Limitations:
+ *
+ ****************************************************************************/
+
+#ifdef CONFIG_IPCC_BUFFERED
+int ipcc_register(int chan, size_t rxbuflen, size_t txbuflen)
+#else
+int ipcc_register(int chan)
+#endif
+{
+ FAR struct ipcc_driver_s *priv;
+ char devname[DEVNAME_FMTLEN];
+ int ret;
+
+ /* Allocate a IPCC character device structure */
+
+ if ((priv = kmm_zalloc(sizeof(*priv))) == NULL)
+ {
+ return -ENOMEM;
+ }
+
+ if ((ret = up_ipcc_init(&priv->ipcc, chan)))
Review Comment:
No, up_ipcc_init() is supposed to be implemented in arch/ not board/. User only has to call ipcc_register(0) and that's all.
--
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] mlyszczek commented on a diff in pull request #6537: drivers/ipcc: add Inter Processor Communication driver
Posted by GitBox <gi...@apache.org>.
mlyszczek commented on code in PR #6537:
URL: https://github.com/apache/incubator-nuttx/pull/6537#discussion_r923159342
##########
drivers/ipcc/ipcc_register.c:
##########
@@ -0,0 +1,208 @@
+/****************************************************************************
+ * drivers/ipcc/ipcc_register.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/ipcc.h>
+#include <nuttx/kmalloc.h>
+#include <nuttx/mm/circbuf.h>
+
+#include <assert.h>
+#include <errno.h>
+#include <stdbool.h>
+#include <stdio.h>
+#include <sys/types.h>
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/* Device naming ************************************************************/
+
+#define DEVNAME_FMT "/dev/ipcc%d"
+#define DEVNAME_FMTLEN (9 + 3 + 1)
+
+/****************************************************************************
+ * Private Function Prototypes
+ ****************************************************************************/
+
+#ifndef CONFIG_DISABLE_PSEUDOFS_OPERATIONS
+int ipcc_open(FAR struct file *filep);
+int ipcc_close(FAR struct file *filep);
+int ipcc_unlink(FAR struct inode *inode);
+#endif
+int ipcc_ioctl(FAR struct file *filep, int cmd, unsigned long arg);
+ssize_t ipcc_read(FAR struct file *filep, FAR char *buffer, size_t buflen);
+ssize_t ipcc_write(FAR struct file *filep, FAR const char *buffer,
+ size_t buflen);
+int ipcc_poll(FAR struct file *filep, FAR struct pollfd *fds,
+ bool setup);
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+static const struct file_operations ipcc_fops =
+{
+#ifdef CONFIG_DISABLE_PSEUDOFS_OPERATIONS
+ .open = NULL,
+ .close = NULL,
Review Comment:
i2c driver only implements open/close when this macro is undefined. I take it, i2c driver is wrong in that regard as well?
--
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 diff in pull request #6537: drivers/ipcc: add Inter Processor Communication driver
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #6537:
URL: https://github.com/apache/incubator-nuttx/pull/6537#discussion_r928692499
##########
include/nuttx/ipcc.h:
##########
@@ -0,0 +1,212 @@
+/****************************************************************************
+ * include/nuttx/ipcc.h
+ *
+ * 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.
+ *
+ ****************************************************************************/
+
+#ifndef __INCLUDE_NUTTX_IPCC_H
+#define __INCLUDE_NUTTX_IPCC_H
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+#include <nuttx/mm/circbuf.h>
+#include <nuttx/semaphore.h>
+
+#include <poll.h>
+
+/****************************************************************************
+ * Public Types
+ ****************************************************************************/
+
+/* IPCC is a two-part driver:
+ *
+ * 1) A common upper half driver, that provides the character device
+ * interface for the user
+ * 2) Platform-specific lower half drivers that provide interface
+ * between upper and lower layer. Lower half also contains
+ * common objects like semaphore to inform
+ */
+
+/* This structure defines all of the operations provided by the
+ * architecture specific logic. All fields must be provided with
+ * non-NULL function pointers.
+ */
+
+struct ipcc_lower_s;
+struct ipcc_ops_s
+{
+ /**************************************************************************
+ * Name: read
+ *
+ * Description:
+ * Reads data directly from IPCC mbox, function shall not block.
+ * If no data is available, 0 shall be returned. Partial read
+ * is possible. In that case function can return value less
+ * than buflen.
+ *
+ * Input Parameters:
+ * ipcc - ipcc channel to read data from
+ * buffer - location where data shall be stored
+ * buflen - size of the buffer
+ *
+ * Returned Value:
+ * Number of bytes actually written to buffer.
+ *
+ **************************************************************************/
+
+ CODE ssize_t (*read)(FAR struct ipcc_lower_s *ipcc,
+ FAR char *buffer, size_t buflen);
+
+ /**************************************************************************
+ * Name: write
+ *
+ * Description:
+ * Writes data directly to IPCC mbox, function shall not block.
+ * If IPCC memory is full, or busy or otherwise unavailable for
+ * write, 0 shall be returned. Partial write is possible. In
+ * that case function can return value less then buflen.
+ *
+ * Input Parameters:
+ * ipcc - ipcc channel to write data to
+ * buffer - location to read data from
+ * buflen - number of bytes to write from buffer
+ *
+ * Returned Value:
+ * Number of actually written data to IPCC channel.
+ *
+ **************************************************************************/
+
+ CODE ssize_t (*write)(FAR struct ipcc_lower_s *ipcc,
+ FAR const char *buffer, size_t buflen);
+
+ /**************************************************************************
+ * Name: buffer_data
+ *
+ * Description:
+ * Lower driver shall copy data from IPCC to cirbuf on demand.
+ * It does not have to copy all data (in case rxbuf is full), but
+ * it must keep internal pointer to track uncopied data, and copy
+ * what is left on the next call to this function.
+ *
+ * Input Parameters:
+ * ipcc - channel to buffer data
+ * rxbuf - circural buffer where data should be written to
+ *
+ * Returned Value:
+ * Number of bytes that were successfully written to rxbuf.
+ *
+ **************************************************************************/
+
+ CODE ssize_t (*buffer_data)(FAR struct ipcc_lower_s *ipcc,
+ FAR struct circbuf_s *rxbuf);
+
+ /**************************************************************************
+ * Name: write_notify
+ *
+ * Description:
+ * Upper half driver notifies lower half driver that new data has been
+ * copied to circ buffer. This can be used by lower half to transfer
+ * data from buffer to IPCC memory.
+ *
+ * Input Parameters:
+ * ipcc - ipcc channel that wrote data.
+ *
+ * Returned Value:
+ * 0 on success, or negated errno otherwise
+ *
+ **************************************************************************/
+
+ CODE ssize_t (*write_notify)(FAR struct ipcc_lower_s *ipcc);
+
+ /**************************************************************************
+ * Name: cleanup
+ *
+ * Description:
+ * Cleans up resources initialized by <arch>_ipcc_init(). If arch code
+ * malloc()ed any memory, this funciton is responsible of freeing it.
+ * After this function is called, ipcc driver will not access lower
+ * half pointer anymore.
+ *
+ * Input Parameters:
+ * ipcc - ipcc channel to cleanup
+ *
+ * Returned Value:
+ * Always OK
+ *
+ **************************************************************************/
+
+ CODE int (*cleanup)(FAR struct ipcc_lower_s *ipcc);
+};
+
+/* This structure defines the interface between upper and lower halves.
+ * Such instance is passed to ipcc_register() function after driver
+ * has been initialized, binding the upper and lower halves into one
+ * driver.
+ */
+
+struct ipcc_lower_s
+{
+ FAR struct pollfd *fds[CONFIG_IPCC_NPOLLWAITERS];
Review Comment:
I mean that the fields which is only used by upper half.
--
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] mlyszczek commented on a diff in pull request #6537: drivers/ipcc: add Inter Processor Communication driver
Posted by GitBox <gi...@apache.org>.
mlyszczek commented on code in PR #6537:
URL: https://github.com/apache/incubator-nuttx/pull/6537#discussion_r928626646
##########
drivers/ipcc/ipcc_register.c:
##########
@@ -0,0 +1,207 @@
+/****************************************************************************
+ * drivers/ipcc/ipcc_register.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/ipcc.h>
+#include <nuttx/kmalloc.h>
+#include <nuttx/mm/circbuf.h>
+
+#include <assert.h>
+#include <errno.h>
+#include <stdbool.h>
+#include <stdio.h>
+#include <sys/types.h>
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/* Device naming ************************************************************/
+
+#define DEVNAME_FMT "/dev/ipcc%d"
+#define DEVNAME_FMTLEN (9 + 3 + 1)
+
+/****************************************************************************
+ * Private Function Prototypes
+ ****************************************************************************/
+
+int ipcc_open(FAR struct file *filep);
+int ipcc_close(FAR struct file *filep);
+#ifndef CONFIG_DISABLE_PSEUDOFS_OPERATIONS
+int ipcc_unlink(FAR struct inode *inode);
+#endif
+int ipcc_ioctl(FAR struct file *filep, int cmd, unsigned long arg);
+ssize_t ipcc_read(FAR struct file *filep, FAR char *buffer, size_t buflen);
+ssize_t ipcc_write(FAR struct file *filep, FAR const char *buffer,
+ size_t buflen);
+int ipcc_poll(FAR struct file *filep, FAR struct pollfd *fds,
+ bool setup);
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+static const struct file_operations ipcc_fops =
+{
+#ifdef CONFIG_DISABLE_PSEUDOFS_OPERATIONS
+ .unlink = NULL,
+#else /* CONFIG_DISABLE_PSEUDOFS_OPERATIONS */
+ .unlink = ipcc_unlink,
+#endif /* CONFIG_DISABLE_PSEUDOFS_OPERATIONS */
+ .open = ipcc_open,
+ .close = ipcc_close,
+ .poll = ipcc_poll,
+ .read = ipcc_read,
+ .write = ipcc_write,
+ .ioctl = NULL,
+ .seek = NULL
+};
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: ipcc_cleanup
+ *
+ * Description:
+ * Cleans up resources allocated by ipcc_register()
+ *
+ * Input Parameters:
+ * priv - ipcc driver instance to clean up
+ *
+ * Returned Value:
+ * None
+ *
+ * Assumptions/Limitations:
+ * This function should be called only when ipcc_register is run with
+ * success and all resources in driver instance are properly allocated.
+ *
+ ****************************************************************************/
+
+void ipcc_cleanup(FAR struct ipcc_driver_s *priv)
+{
+#ifdef CONFIG_IPCC_BUFFERED
+ circbuf_uninit(&priv->ipcc->rxbuf);
+ circbuf_uninit(&priv->ipcc->txbuf);
+#endif
+ nxsem_destroy(&priv->ipcc->rxsem);
+ nxsem_destroy(&priv->ipcc->txsem);
+ priv->ipcc->ops.cleanup(priv->ipcc);
+ nxsem_destroy(&priv->exclsem);
+ kmm_free(priv);
+}
+
+/****************************************************************************
+ * Name: ipcc_register
+ *
+ * Description:
+ * Create and register the IPCC character driver.
+ *
+ * IPCC is a simple character driver that supports inter processor
+ * communication.
+ *
+ * Input Parameters:
+ * ipcc - An instance of the lower half IPCC driver
+ * chan - IPCC channel. This will be used ad IPCC minor number.
+ * IPPC will be registered as /dev/ipccN where N is the minor number.
+ * buflen - Length of tx and rx buffers, 0 for unbuffered communication.
+ *
+ * Returned Value:
+ * OK if the driver was successfully registered, or negated errno on
+ * failure.
+ *
+ * Assumptions/Limitations:
+ * ipcc is already allocated and initialized by architecture code.
+ *
+ ****************************************************************************/
+
+#ifdef CONFIG_IPCC_BUFFERED
+int ipcc_register(FAR struct ipcc_lower_s *ipcc, size_t rxbuflen,
+ size_t txbuflen)
+#else
+int ipcc_register(FAR struct ipcc_lower_s *ipcc)
+#endif
+{
+ FAR struct ipcc_driver_s *priv;
+ char devname[DEVNAME_FMTLEN];
+ int ret;
+
+ /* Allocate a IPCC character device structure */
+
+ if ((priv = kmm_zalloc(sizeof(*priv))) == NULL)
+ {
+ return -ENOMEM;
+ }
+
+ /* Link upper and lower driver together */
+
+ priv->ipcc = ipcc;
+
+#ifdef CONFIG_IPCC_BUFFERED
+ /* allocate buffers for reading and writing data to IPCC memory */
+
+ if (rxbuflen)
+ {
+ if ((ret = circbuf_init(&priv->ipcc->rxbuf, NULL, rxbuflen)))
+ {
+ goto error;
+ }
+ }
+
+ if (txbuflen)
+ {
+ if ((ret = circbuf_init(&priv->ipcc->txbuf, NULL, txbuflen)))
+ {
+ goto error;
+ }
+ }
+#endif /* CONFIG_IPCC_BUFFERED */
+
+ /* Create the character device name */
+
+ snprintf(devname, DEVNAME_FMTLEN, DEVNAME_FMT, ipcc->chan);
+ if ((ret = register_driver(devname, &ipcc_fops, 0666, priv)))
+ {
+ goto error;
+ }
+
+ /* nxsem_init can't really fail us if we provide it with valid params */
+
+ nxsem_init(&priv->exclsem, 0, 1);
+ nxsem_init(&priv->ipcc->rxsem, 0, 0);
+ nxsem_init(&priv->ipcc->txsem, 0, 1);
+
+ return OK;
+
+error:
+#ifdef CONFIG_IPCC_BUFFERED
+ circbuf_uninit(&priv->ipcc->rxbuf);
+ circbuf_uninit(&priv->ipcc->txbuf);
+#endif
+ ipcc_cleanup(priv);
+ kmm_free(priv);
Review Comment:
Oh my... I did miss that one now, didn't I?
--
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 diff in pull request #6537: drivers/ipcc: add Inter Processor Communication driver
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #6537:
URL: https://github.com/apache/incubator-nuttx/pull/6537#discussion_r931947509
##########
drivers/ipcc/ipcc_register.c:
##########
@@ -0,0 +1,205 @@
+/****************************************************************************
+ * drivers/ipcc/ipcc_register.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/ipcc.h>
+#include <nuttx/kmalloc.h>
+#include <nuttx/mm/circbuf.h>
+
+#include <assert.h>
+#include <errno.h>
+#include <stdbool.h>
+#include <stdio.h>
+#include <sys/types.h>
+
+#include "ipcc_priv.h"
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/* Device naming ************************************************************/
+
+#define DEVNAME_FMT "/dev/ipcc%d"
+#define DEVNAME_FMTLEN (9 + 3 + 1)
+
+/****************************************************************************
+ * Private Function Prototypes
+ ****************************************************************************/
+
+int ipcc_open(FAR struct file *filep);
+int ipcc_close(FAR struct file *filep);
+#ifndef CONFIG_DISABLE_PSEUDOFS_OPERATIONS
+int ipcc_unlink(FAR struct inode *inode);
+#endif
+int ipcc_ioctl(FAR struct file *filep, int cmd, unsigned long arg);
Review Comment:
let's move all prototype to ipcc_priv.h?
--
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 merged pull request #6537: drivers/ipcc: add Inter Processor Communication driver
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 merged PR #6537:
URL: https://github.com/apache/incubator-nuttx/pull/6537
--
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] mlyszczek commented on a diff in pull request #6537: drivers/ipcc: add Inter Processor Communication driver
Posted by GitBox <gi...@apache.org>.
mlyszczek commented on code in PR #6537:
URL: https://github.com/apache/incubator-nuttx/pull/6537#discussion_r923646642
##########
arch/arm/src/stm32wl5/stm32wl5_ipcc.c:
##########
@@ -0,0 +1,801 @@
+/****************************************************************************
+ * arch/arm/src/stm32wl5/stm32wl5_ipcc.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/ipcc.h>
+#include <nuttx/irq.h>
+
+#include <arm_internal.h>
+#include <assert.h>
+#include <debug.h>
+
+#include <arch/stm32wl5/stm32wl5xxx_cpu1_irq.h>
+#include "hardware/stm32wl5_ipcc.h"
+#include "stm32wl5.h"
+#include "stm32wl5_ipcc.h"
+
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+/* This structure describes tx or rx of single channel in memory */
+
+struct stm32wl5_ipcc_chan_mem_s
+{
+ unsigned len; /* Number of valid bytes in data[] */
+ char data[]; /* Data in IPCC memory */
+};
+
+/* Internal stm32wl5 ipcc structure describing channel state. */
+
+struct stm32wl5_ipcc_s
+{
+ /* Pointer to API connecting upper and lower half of the driver */
+
+ struct ipcc_lower_s *ipcc;
+
+ /* Physical memory address where second CPU will write data for us,
+ * we will be reading from this memory.
+ */
+
+ char *rxmem;
+
+ /* Maximum length of data that rxmem can hold. It is size of the
+ * reserved space for rxmem minus sizeof(stm32wl5_ipcc_chan_mem_s.len)
+ */
+
+ unsigned rxlen;
+
+#if CONFIG_IPCC_BUFFERED
+ /* Number of bytes copied from IPCC memory to buffer. Can be less than
+ * stm32wl5_ipcc_chan_mem_s.len after copy operation when buffer is full.
+ * Value can persist between multiple ISR and stm32wl5_ipcc_buffer_data()
+ * calls, until all data from IPCC memory is successfully buffered.
+ */
+
+ unsigned rxcopied;
+#endif
+
+ /* Physical memory address where we will write data for the second
+ * CPU to read.
+ */
+
+ char *txmem;
+
+ /* Maximum length of data that txmem can hold. It is size of the
+ * reserved space for txmem minus sizeof(stm32wl5_ipcc_chan_mem_s.len)
+ */
+
+ unsigned txlen;
+};
+
+/****************************************************************************
+ * Private Function Prototypes
+ ****************************************************************************/
+
+static ssize_t stm32wl5_ipcc_read(struct ipcc_lower_s *ipcc,
+ char *buffer, size_t buflen);
+static ssize_t stm32wl5_ipcc_write(struct ipcc_lower_s *ipcc,
+ const char *buffer, size_t buflen);
+static ssize_t stm32wl5_ipcc_buffer_data(struct ipcc_lower_s *ipcc,
+ struct circbuf_s *rxbuf);
+static ssize_t stm32wl5_ipcc_copy_to_buffer(int chan,
+ struct circbuf_s *rxbuf);
+static int stm32wl5_ipcc_rx_isr(int irq, void *context, void *arg);
+static int stm32wl5_ipcc_tx_isr(int irq, void *context, void *arg);
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+struct stm32wl5_ipcc_s g_ipccpriv[IPCC_NCHAN] =
+{
+ /* Channel 1 is always enabled when IPCC is enabled */
+
+ {
+ .rxmem = (char *)(IPCC_CHAN1_START),
+ .rxlen = IPCC_CHAN1_RX_SIZE - sizeof(unsigned),
+ .txmem = (char *)(IPCC_CHAN1_START + IPCC_CHAN1_RX_SIZE),
+ .txlen = IPCC_CHAN1_TX_SIZE - sizeof(unsigned)
+ }
+
+#if IPCC_CHAN2
+ ,
+ {
+ .rxmem = (char *)(IPCC_CHAN2_START),
+ .rxlen = IPCC_CHAN2_RX_SIZE - sizeof(unsigned),
+ .txmem = (char *)(IPCC_CHAN2_START + IPCC_CHAN2_RX_SIZE),
+ .txlen = IPCC_CHAN2_TX_SIZE - sizeof(unsigned)
+ }
+#endif
+
+#if IPCC_CHAN3
+ ,
+ {
+ .rxmem = (char *)(IPCC_CHAN3_START),
+ .rxlen = IPCC_CHAN3_RX_SIZE - sizeof(unsigned),
+ .txmem = (char *)(IPCC_CHAN3_START + IPCC_CHAN3_RX_SIZE),
+ .txlen = IPCC_CHAN3_TX_SIZE - sizeof(unsigned)
+ }
+#endif
+
+#if IPCC_CHAN4
+ ,
+ {
+ .rxmem = (char *)(IPCC_CHAN4_START),
+ .rxlen = IPCC_CHAN4_RX_SIZE - sizeof(unsigned),
+ .txmem = (char *)(IPCC_CHAN4_START + IPCC_CHAN4_RX_SIZE),
+ .txlen = IPCC_CHAN4_TX_SIZE - sizeof(unsigned)
+ }
+#endif
+
+#if IPCC_CHAN5
+ ,
+ {
+ .rxmem = (char *)(IPCC_CHAN5_START),
+ .rxlen = IPCC_CHAN5_RX_SIZE - sizeof(unsigned),
+ .txmem = (char *)(IPCC_CHAN5_START + IPCC_CHAN5_RX_SIZE),
+ .txlen = IPCC_CHAN5_TX_SIZE - sizeof(unsigned)
+ }
+#endif
+
+#if IPCC_CHAN6
+ ,
+ {
+ .rxmem = (char *)(IPCC_CHAN6_START),
+ .rxlen = IPCC_CHAN6_RX_SIZE - sizeof(unsigned),
+ .txmem = (char *)(IPCC_CHAN6_START + IPCC_CHAN6_RX_SIZE),
+ .txlen = IPCC_CHAN6_TX_SIZE - sizeof(unsigned)
+ }
+#endif
+};
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: stm32wl5_ipcc_tx_isr
+ *
+ * Description:
+ * IPCC TX interrupt service routine. This interrupt is called when
+ * second CPU read all data from IPCC TX memory and we are free to
+ * write new data to that memory.
+ *
+ * For buffered IPCC, we will immediately write data from buffer to
+ * IPCC memory and notify second CPU. Also blocked writer will be
+ * notified that there is free space on tx buffer.
+ *
+ * For unbuffered IPCC, we won't copy anything but only notify blocked
+ * writers that IPCC TX memory is free.
+ *
+ * Input Parameters:
+ * irq - Number of IRQ that generated interrupt
+ * context - Interrupt register state save info (architecture-specific)
+ * arg - user data (not used by us)
+ *
+ * Returned Value:
+ * Always OK
+ *
+ * Assumptions/Limitations:
+ *
+ ****************************************************************************/
+
+static int stm32wl5_ipcc_tx_isr(int irq, void *context, void *arg)
+{
+ int chan;
+ size_t nwritten;
+ uint32_t mr;
+ uint32_t sr;
+ uint32_t status;
+ struct stm32wl5_ipcc_s *priv;
+ struct stm32wl5_ipcc_chan_mem_s *txmem;
+
+ (void)context;
+ (void)arg;
+ (void)irq;
+
+ /* Disable TX interrupt, we don't want two "threads" to concurently
+ * modify circbuffers as we don't use any guard to them
+ */
+
+ up_disable_irq(STM32WL5_IRQ_IPCC_C1_TX_IT);
+
+ mr = getreg32(STM32WL5_IPCC_C1MR) >> STM32WL5_IPCC_TX_SHIFT;
+ sr = getreg32(STM32WL5_IPCC_C1TOC2SR);
+
+ /* Consider only channels that have tx memory free and are unmasked */
+
+ status = sr | mr;
+
+ /* Check which channels have data ready to read */
+
+ for (chan = 0; chan != IPCC_NCHAN; chan++)
+ {
+ if (status & (1 << chan))
+ {
+ /* Transmit on channel is either masked or TX memory is not
+ * ready to write (second CPU is still read data from it,
+ * CHnF == 1).
+ */
+
+ continue;
+ }
+
+ /* Get internal data structure for current chan */
+
+ priv = &g_ipccpriv[chan];
+ txmem = (struct stm32wl5_ipcc_chan_mem_s *)priv->txmem;
+
+#if CONFIG_IPCC_BUFFERED
+ /* Copy as much as we can into IPCC memory, circbuf won't copy
+ * more than there is in the buffer.
+ */
+
+ nwritten = circbuf_read(&priv->ipcc->txbuf, txmem->data, priv->txlen);
Review Comment:
Circbuf is very lightweight, it does couple int comparision then call memcpy(). Is copying couple of hundreds bytes that much for interrupt handler? I guess it might be if you copy a lot of data. I guess it would be best to install DMA handler here, which I might consider if I hit bottleneck here.
--
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 diff in pull request #6537: drivers/ipcc: add Inter Processor Communication driver
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #6537:
URL: https://github.com/apache/incubator-nuttx/pull/6537#discussion_r923176443
##########
include/nuttx/ipcc.h:
##########
@@ -0,0 +1,188 @@
+/****************************************************************************
+ * include/nuttx/ipcc.h
+ *
+ * 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/mm/circbuf.h>
+#include <nuttx/semaphore.h>
+
+#include <poll.h>
+
+/****************************************************************************
+ * Public Types
+ ****************************************************************************/
+
+/* IPCC is a two-part driver:
+ *
+ * 1) A common upper half driver, that provides the character device
+ * interface for the user
+ * 2) Platform-specific lower half drivers that provide interface
+ * between upper and lower layer. Lower half also contains
+ * common objects like semaphore to inform
+ */
+
+/* This structure defines all of the operations provided by the
+ * architecture specific logic. All fields must be provided with
+ * non-NULL function pointers.
+ */
+
+struct ipcc_lower_s;
+struct ipcc_ops_s
+{
+ /* Reads data directly from IPCC mbox, should not block, ever.
+ * If no data is available, 0 should be returned. Partial read
+ * is possible. In that case function will return value less
+ * than buflen.
+ */
+
+ CODE ssize_t (*read)(FAR struct ipcc_lower_s *ipcc,
+ FAR char *buffer, size_t buflen);
+
+ /* Writes data directly to IPCC mbox, should not block, ever.
+ * If IPCC memory is full, or busy or otherwise unavailable for
+ * write, 0 should be returned. Partial write is possible. In
+ * that case function will return value less then buflen.
+ */
+
+ CODE ssize_t (*write)(FAR struct ipcc_lower_s *ipcc,
+ FAR const char *buffer, size_t buflen);
+
+ /* Lower driver should copy data from IPCC to cirbuf on demand.
+ * It does not have to copy all data (in case rxbuf is full), but
+ * it should keep internal pointer to track uncopied data, and copy
+ * what is left on the next call to this function.
+ */
+
+ CODE ssize_t (*buffer_data)(FAR struct ipcc_lower_s *ipcc,
+ FAR struct circbuf_s *rxbuf);
+
+ /* Upper half notifies lower half driver that new data has been
+ * copied to circ buffer
+ */
+
+ CODE ssize_t (*write_notify)(FAR struct ipcc_lower_s *ipcc);
+};
+
+/* This structure defines the interface between upper and lower halves.
+ * Such instance is passed to ipcc_register() function after driver
+ * has been initialized, binding the upper and lower halves into one
+ * driver.
+ */
+
+struct ipcc_lower_s
+{
+ FAR struct pollfd *fds[CONFIG_IPCC_NPOLLWAITERS];
+ FAR struct ipcc_ops_s ops; /* Arch specific functions */
+ sem_t rxsem; /* Data ready to read semaphore */
+ sem_t txsem; /* Data ready to send semaphore */
+ int chan; /* IPCC channel */
+
+#ifdef CONFIG_IPCC_BUFFERED
+ struct circbuf_s rxbuf; /* Receive buffer */
+ struct circbuf_s txbuf; /* Transmit buffer */
+
+ /* 1- not all data could fit into buffer, some unread data is still
+ * in IPCC mbox. State of this variable is maintained by lower half,
+ * upper half only reads it to know when to inform lower driver that
+ * buffer got free.
+ */
+
+ int overflow;
+#endif /* CONFIG_IPCC_BUFFERED */
+};
+
+/* Driver state structure */
+
+struct ipcc_driver_s
+{
+ FAR struct ipcc_lower_s ipcc; /* Lower half driver state */
+ int crefs; /* Count number of open references */
+ int unlinked; /* 1 - driver has been unlinked */
+ sem_t exclsem; /* Mutual exclusion for driver */
+};
+
+/****************************************************************************
+ * Public Functions Prototypes
+ ****************************************************************************/
+
+void ipcc_pollnotify(FAR struct ipcc_lower_s *ipcc, pollevent_t eventset);
+void ipcc_cleanup(FAR struct ipcc_driver_s *priv);
+void ipcc_txfree_notify(FAR struct ipcc_lower_s *priv);
+void ipcc_rxfree_notify(FAR struct ipcc_lower_s *ipcc);
+
+#ifdef CONFIG_IPCC_BUFFERED
+int ipcc_register(int chan, size_t rxbuflen, size_t txbuflen);
+#else
+int ipcc_register(int chan);
+#endif
+
+/****************************************************************************
+ * Name: up_ipcc_init
+ *
+ * Description:
+ * Initializes lower half of the driver. This function should be
+ * implemented by lower half driver in microcontroller code. Function
+ * should not initialize ipcc_lower_s structure with the exception of
+ * explicitly specified fields like setting pointer to read and write
+ * function pointers. Rest of the fields are initialized in upper half
+ * driver.
+ *
+ * This function is called once in ipcc_register() function.
+ *
+ * Input Parameters:
+ * ipcc - interface between upper and lower half drivers. Only some
+ * of the fields should be initialized in this function.
+ * chan - channel number to initialize.
+ *
+ * Returned Value:
+ * 0 on success or negated errno otherwise.
+ *
+ * Assumptions/Limitations:
+ *
+ ****************************************************************************/
+
+int up_ipcc_init(FAR struct ipcc_lower_s *ipcc, int chan);
Review Comment:
> Oh wait, but to what should I change it to? I cannot move it to ops struct, it needs to be known name so that upper half can call it so that lower half can initialize that ops struct.
you can pass the arguments you need to ipcc_register like other driver.
##########
include/nuttx/ipcc.h:
##########
@@ -0,0 +1,188 @@
+/****************************************************************************
+ * include/nuttx/ipcc.h
+ *
+ * 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/mm/circbuf.h>
+#include <nuttx/semaphore.h>
+
+#include <poll.h>
+
+/****************************************************************************
+ * Public Types
+ ****************************************************************************/
+
+/* IPCC is a two-part driver:
+ *
+ * 1) A common upper half driver, that provides the character device
+ * interface for the user
+ * 2) Platform-specific lower half drivers that provide interface
+ * between upper and lower layer. Lower half also contains
+ * common objects like semaphore to inform
+ */
+
+/* This structure defines all of the operations provided by the
+ * architecture specific logic. All fields must be provided with
+ * non-NULL function pointers.
+ */
+
+struct ipcc_lower_s;
+struct ipcc_ops_s
+{
+ /* Reads data directly from IPCC mbox, should not block, ever.
+ * If no data is available, 0 should be returned. Partial read
+ * is possible. In that case function will return value less
+ * than buflen.
+ */
+
+ CODE ssize_t (*read)(FAR struct ipcc_lower_s *ipcc,
+ FAR char *buffer, size_t buflen);
+
+ /* Writes data directly to IPCC mbox, should not block, ever.
+ * If IPCC memory is full, or busy or otherwise unavailable for
+ * write, 0 should be returned. Partial write is possible. In
+ * that case function will return value less then buflen.
+ */
+
+ CODE ssize_t (*write)(FAR struct ipcc_lower_s *ipcc,
+ FAR const char *buffer, size_t buflen);
+
+ /* Lower driver should copy data from IPCC to cirbuf on demand.
+ * It does not have to copy all data (in case rxbuf is full), but
+ * it should keep internal pointer to track uncopied data, and copy
+ * what is left on the next call to this function.
+ */
+
+ CODE ssize_t (*buffer_data)(FAR struct ipcc_lower_s *ipcc,
+ FAR struct circbuf_s *rxbuf);
+
+ /* Upper half notifies lower half driver that new data has been
+ * copied to circ buffer
+ */
+
+ CODE ssize_t (*write_notify)(FAR struct ipcc_lower_s *ipcc);
+};
+
+/* This structure defines the interface between upper and lower halves.
+ * Such instance is passed to ipcc_register() function after driver
+ * has been initialized, binding the upper and lower halves into one
+ * driver.
+ */
+
+struct ipcc_lower_s
+{
+ FAR struct pollfd *fds[CONFIG_IPCC_NPOLLWAITERS];
+ FAR struct ipcc_ops_s ops; /* Arch specific functions */
+ sem_t rxsem; /* Data ready to read semaphore */
+ sem_t txsem; /* Data ready to send semaphore */
+ int chan; /* IPCC channel */
+
+#ifdef CONFIG_IPCC_BUFFERED
+ struct circbuf_s rxbuf; /* Receive buffer */
+ struct circbuf_s txbuf; /* Transmit buffer */
+
+ /* 1- not all data could fit into buffer, some unread data is still
+ * in IPCC mbox. State of this variable is maintained by lower half,
+ * upper half only reads it to know when to inform lower driver that
+ * buffer got free.
+ */
+
+ int overflow;
+#endif /* CONFIG_IPCC_BUFFERED */
+};
+
+/* Driver state structure */
+
+struct ipcc_driver_s
+{
+ FAR struct ipcc_lower_s ipcc; /* Lower half driver state */
+ int crefs; /* Count number of open references */
+ int unlinked; /* 1 - driver has been unlinked */
+ sem_t exclsem; /* Mutual exclusion for driver */
+};
+
+/****************************************************************************
+ * Public Functions Prototypes
+ ****************************************************************************/
+
+void ipcc_pollnotify(FAR struct ipcc_lower_s *ipcc, pollevent_t eventset);
+void ipcc_cleanup(FAR struct ipcc_driver_s *priv);
+void ipcc_txfree_notify(FAR struct ipcc_lower_s *priv);
+void ipcc_rxfree_notify(FAR struct ipcc_lower_s *ipcc);
+
+#ifdef CONFIG_IPCC_BUFFERED
+int ipcc_register(int chan, size_t rxbuflen, size_t txbuflen);
+#else
+int ipcc_register(int chan);
+#endif
+
+/****************************************************************************
+ * Name: up_ipcc_init
+ *
+ * Description:
+ * Initializes lower half of the driver. This function should be
+ * implemented by lower half driver in microcontroller code. Function
+ * should not initialize ipcc_lower_s structure with the exception of
+ * explicitly specified fields like setting pointer to read and write
+ * function pointers. Rest of the fields are initialized in upper half
+ * driver.
+ *
+ * This function is called once in ipcc_register() function.
+ *
+ * Input Parameters:
+ * ipcc - interface between upper and lower half drivers. Only some
+ * of the fields should be initialized in this function.
+ * chan - channel number to initialize.
+ *
+ * Returned Value:
+ * 0 on success or negated errno otherwise.
+ *
+ * Assumptions/Limitations:
+ *
+ ****************************************************************************/
+
+int up_ipcc_init(FAR struct ipcc_lower_s *ipcc, int chan);
Review Comment:
> Oh wait, but to what should I change it to? I cannot move it to ops struct, it needs to be known name so that upper half can call it so that lower half can initialize that ops struct.
you can pass the arguments you need to ipcc_register like other drivers.
--
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] onegray commented on a diff in pull request #6537: drivers/ipcc: add Inter Processor Communication driver
Posted by GitBox <gi...@apache.org>.
onegray commented on code in PR #6537:
URL: https://github.com/apache/incubator-nuttx/pull/6537#discussion_r923661724
##########
arch/arm/src/stm32wl5/stm32wl5_ipcc.c:
##########
@@ -0,0 +1,801 @@
+/****************************************************************************
+ * arch/arm/src/stm32wl5/stm32wl5_ipcc.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/ipcc.h>
+#include <nuttx/irq.h>
+
+#include <arm_internal.h>
+#include <assert.h>
+#include <debug.h>
+
+#include <arch/stm32wl5/stm32wl5xxx_cpu1_irq.h>
+#include "hardware/stm32wl5_ipcc.h"
+#include "stm32wl5.h"
+#include "stm32wl5_ipcc.h"
+
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+/* This structure describes tx or rx of single channel in memory */
+
+struct stm32wl5_ipcc_chan_mem_s
+{
+ unsigned len; /* Number of valid bytes in data[] */
+ char data[]; /* Data in IPCC memory */
+};
+
+/* Internal stm32wl5 ipcc structure describing channel state. */
+
+struct stm32wl5_ipcc_s
+{
+ /* Pointer to API connecting upper and lower half of the driver */
+
+ struct ipcc_lower_s *ipcc;
+
+ /* Physical memory address where second CPU will write data for us,
+ * we will be reading from this memory.
+ */
+
+ char *rxmem;
+
+ /* Maximum length of data that rxmem can hold. It is size of the
+ * reserved space for rxmem minus sizeof(stm32wl5_ipcc_chan_mem_s.len)
+ */
+
+ unsigned rxlen;
+
+#if CONFIG_IPCC_BUFFERED
+ /* Number of bytes copied from IPCC memory to buffer. Can be less than
+ * stm32wl5_ipcc_chan_mem_s.len after copy operation when buffer is full.
+ * Value can persist between multiple ISR and stm32wl5_ipcc_buffer_data()
+ * calls, until all data from IPCC memory is successfully buffered.
+ */
+
+ unsigned rxcopied;
+#endif
+
+ /* Physical memory address where we will write data for the second
+ * CPU to read.
+ */
+
+ char *txmem;
+
+ /* Maximum length of data that txmem can hold. It is size of the
+ * reserved space for txmem minus sizeof(stm32wl5_ipcc_chan_mem_s.len)
+ */
+
+ unsigned txlen;
+};
+
+/****************************************************************************
+ * Private Function Prototypes
+ ****************************************************************************/
+
+static ssize_t stm32wl5_ipcc_read(struct ipcc_lower_s *ipcc,
+ char *buffer, size_t buflen);
+static ssize_t stm32wl5_ipcc_write(struct ipcc_lower_s *ipcc,
+ const char *buffer, size_t buflen);
+static ssize_t stm32wl5_ipcc_buffer_data(struct ipcc_lower_s *ipcc,
+ struct circbuf_s *rxbuf);
+static ssize_t stm32wl5_ipcc_copy_to_buffer(int chan,
+ struct circbuf_s *rxbuf);
+static int stm32wl5_ipcc_rx_isr(int irq, void *context, void *arg);
+static int stm32wl5_ipcc_tx_isr(int irq, void *context, void *arg);
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+struct stm32wl5_ipcc_s g_ipccpriv[IPCC_NCHAN] =
+{
+ /* Channel 1 is always enabled when IPCC is enabled */
+
+ {
+ .rxmem = (char *)(IPCC_CHAN1_START),
+ .rxlen = IPCC_CHAN1_RX_SIZE - sizeof(unsigned),
+ .txmem = (char *)(IPCC_CHAN1_START + IPCC_CHAN1_RX_SIZE),
+ .txlen = IPCC_CHAN1_TX_SIZE - sizeof(unsigned)
+ }
+
+#if IPCC_CHAN2
+ ,
+ {
+ .rxmem = (char *)(IPCC_CHAN2_START),
+ .rxlen = IPCC_CHAN2_RX_SIZE - sizeof(unsigned),
+ .txmem = (char *)(IPCC_CHAN2_START + IPCC_CHAN2_RX_SIZE),
+ .txlen = IPCC_CHAN2_TX_SIZE - sizeof(unsigned)
+ }
+#endif
+
+#if IPCC_CHAN3
+ ,
+ {
+ .rxmem = (char *)(IPCC_CHAN3_START),
+ .rxlen = IPCC_CHAN3_RX_SIZE - sizeof(unsigned),
+ .txmem = (char *)(IPCC_CHAN3_START + IPCC_CHAN3_RX_SIZE),
+ .txlen = IPCC_CHAN3_TX_SIZE - sizeof(unsigned)
+ }
+#endif
+
+#if IPCC_CHAN4
+ ,
+ {
+ .rxmem = (char *)(IPCC_CHAN4_START),
+ .rxlen = IPCC_CHAN4_RX_SIZE - sizeof(unsigned),
+ .txmem = (char *)(IPCC_CHAN4_START + IPCC_CHAN4_RX_SIZE),
+ .txlen = IPCC_CHAN4_TX_SIZE - sizeof(unsigned)
+ }
+#endif
+
+#if IPCC_CHAN5
+ ,
+ {
+ .rxmem = (char *)(IPCC_CHAN5_START),
+ .rxlen = IPCC_CHAN5_RX_SIZE - sizeof(unsigned),
+ .txmem = (char *)(IPCC_CHAN5_START + IPCC_CHAN5_RX_SIZE),
+ .txlen = IPCC_CHAN5_TX_SIZE - sizeof(unsigned)
+ }
+#endif
+
+#if IPCC_CHAN6
+ ,
+ {
+ .rxmem = (char *)(IPCC_CHAN6_START),
+ .rxlen = IPCC_CHAN6_RX_SIZE - sizeof(unsigned),
+ .txmem = (char *)(IPCC_CHAN6_START + IPCC_CHAN6_RX_SIZE),
+ .txlen = IPCC_CHAN6_TX_SIZE - sizeof(unsigned)
+ }
+#endif
+};
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: stm32wl5_ipcc_tx_isr
+ *
+ * Description:
+ * IPCC TX interrupt service routine. This interrupt is called when
+ * second CPU read all data from IPCC TX memory and we are free to
+ * write new data to that memory.
+ *
+ * For buffered IPCC, we will immediately write data from buffer to
+ * IPCC memory and notify second CPU. Also blocked writer will be
+ * notified that there is free space on tx buffer.
+ *
+ * For unbuffered IPCC, we won't copy anything but only notify blocked
+ * writers that IPCC TX memory is free.
+ *
+ * Input Parameters:
+ * irq - Number of IRQ that generated interrupt
+ * context - Interrupt register state save info (architecture-specific)
+ * arg - user data (not used by us)
+ *
+ * Returned Value:
+ * Always OK
+ *
+ * Assumptions/Limitations:
+ *
+ ****************************************************************************/
+
+static int stm32wl5_ipcc_tx_isr(int irq, void *context, void *arg)
+{
+ int chan;
+ size_t nwritten;
+ uint32_t mr;
+ uint32_t sr;
+ uint32_t status;
+ struct stm32wl5_ipcc_s *priv;
+ struct stm32wl5_ipcc_chan_mem_s *txmem;
+
+ (void)context;
+ (void)arg;
+ (void)irq;
+
+ /* Disable TX interrupt, we don't want two "threads" to concurently
+ * modify circbuffers as we don't use any guard to them
+ */
+
+ up_disable_irq(STM32WL5_IRQ_IPCC_C1_TX_IT);
+
+ mr = getreg32(STM32WL5_IPCC_C1MR) >> STM32WL5_IPCC_TX_SHIFT;
+ sr = getreg32(STM32WL5_IPCC_C1TOC2SR);
+
+ /* Consider only channels that have tx memory free and are unmasked */
+
+ status = sr | mr;
+
+ /* Check which channels have data ready to read */
+
+ for (chan = 0; chan != IPCC_NCHAN; chan++)
+ {
+ if (status & (1 << chan))
+ {
+ /* Transmit on channel is either masked or TX memory is not
+ * ready to write (second CPU is still read data from it,
+ * CHnF == 1).
+ */
+
+ continue;
+ }
+
+ /* Get internal data structure for current chan */
+
+ priv = &g_ipccpriv[chan];
+ txmem = (struct stm32wl5_ipcc_chan_mem_s *)priv->txmem;
+
+#if CONFIG_IPCC_BUFFERED
+ /* Copy as much as we can into IPCC memory, circbuf won't copy
+ * more than there is in the buffer.
+ */
+
+ nwritten = circbuf_read(&priv->ipcc->txbuf, txmem->data, priv->txlen);
Review Comment:
I think the high priority work queue is more suitable for this, but consider this just as an option. Also I understand it needs more efforts to implement this, so let's keep it as is.
--
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] mlyszczek commented on a diff in pull request #6537: drivers/ipcc: add Inter Processor Communication driver
Posted by GitBox <gi...@apache.org>.
mlyszczek commented on code in PR #6537:
URL: https://github.com/apache/incubator-nuttx/pull/6537#discussion_r923165355
##########
include/nuttx/ipcc.h:
##########
@@ -0,0 +1,188 @@
+/****************************************************************************
+ * include/nuttx/ipcc.h
+ *
+ * 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/mm/circbuf.h>
+#include <nuttx/semaphore.h>
+
+#include <poll.h>
+
+/****************************************************************************
+ * Public Types
+ ****************************************************************************/
+
+/* IPCC is a two-part driver:
+ *
+ * 1) A common upper half driver, that provides the character device
+ * interface for the user
+ * 2) Platform-specific lower half drivers that provide interface
+ * between upper and lower layer. Lower half also contains
+ * common objects like semaphore to inform
+ */
+
+/* This structure defines all of the operations provided by the
+ * architecture specific logic. All fields must be provided with
+ * non-NULL function pointers.
+ */
+
+struct ipcc_lower_s;
+struct ipcc_ops_s
+{
+ /* Reads data directly from IPCC mbox, should not block, ever.
+ * If no data is available, 0 should be returned. Partial read
+ * is possible. In that case function will return value less
+ * than buflen.
+ */
+
+ CODE ssize_t (*read)(FAR struct ipcc_lower_s *ipcc,
+ FAR char *buffer, size_t buflen);
+
+ /* Writes data directly to IPCC mbox, should not block, ever.
+ * If IPCC memory is full, or busy or otherwise unavailable for
+ * write, 0 should be returned. Partial write is possible. In
+ * that case function will return value less then buflen.
+ */
+
+ CODE ssize_t (*write)(FAR struct ipcc_lower_s *ipcc,
+ FAR const char *buffer, size_t buflen);
+
+ /* Lower driver should copy data from IPCC to cirbuf on demand.
+ * It does not have to copy all data (in case rxbuf is full), but
+ * it should keep internal pointer to track uncopied data, and copy
+ * what is left on the next call to this function.
+ */
+
+ CODE ssize_t (*buffer_data)(FAR struct ipcc_lower_s *ipcc,
+ FAR struct circbuf_s *rxbuf);
+
+ /* Upper half notifies lower half driver that new data has been
+ * copied to circ buffer
+ */
+
+ CODE ssize_t (*write_notify)(FAR struct ipcc_lower_s *ipcc);
+};
+
+/* This structure defines the interface between upper and lower halves.
+ * Such instance is passed to ipcc_register() function after driver
+ * has been initialized, binding the upper and lower halves into one
+ * driver.
+ */
+
+struct ipcc_lower_s
+{
+ FAR struct pollfd *fds[CONFIG_IPCC_NPOLLWAITERS];
+ FAR struct ipcc_ops_s ops; /* Arch specific functions */
+ sem_t rxsem; /* Data ready to read semaphore */
+ sem_t txsem; /* Data ready to send semaphore */
+ int chan; /* IPCC channel */
+
+#ifdef CONFIG_IPCC_BUFFERED
+ struct circbuf_s rxbuf; /* Receive buffer */
+ struct circbuf_s txbuf; /* Transmit buffer */
+
+ /* 1- not all data could fit into buffer, some unread data is still
+ * in IPCC mbox. State of this variable is maintained by lower half,
+ * upper half only reads it to know when to inform lower driver that
+ * buffer got free.
+ */
+
+ int overflow;
+#endif /* CONFIG_IPCC_BUFFERED */
+};
+
+/* Driver state structure */
+
+struct ipcc_driver_s
+{
+ FAR struct ipcc_lower_s ipcc; /* Lower half driver state */
+ int crefs; /* Count number of open references */
+ int unlinked; /* 1 - driver has been unlinked */
+ sem_t exclsem; /* Mutual exclusion for driver */
+};
+
+/****************************************************************************
+ * Public Functions Prototypes
+ ****************************************************************************/
+
+void ipcc_pollnotify(FAR struct ipcc_lower_s *ipcc, pollevent_t eventset);
+void ipcc_cleanup(FAR struct ipcc_driver_s *priv);
+void ipcc_txfree_notify(FAR struct ipcc_lower_s *priv);
+void ipcc_rxfree_notify(FAR struct ipcc_lower_s *ipcc);
+
+#ifdef CONFIG_IPCC_BUFFERED
+int ipcc_register(int chan, size_t rxbuflen, size_t txbuflen);
+#else
+int ipcc_register(int chan);
+#endif
+
+/****************************************************************************
+ * Name: up_ipcc_init
+ *
+ * Description:
+ * Initializes lower half of the driver. This function should be
+ * implemented by lower half driver in microcontroller code. Function
+ * should not initialize ipcc_lower_s structure with the exception of
+ * explicitly specified fields like setting pointer to read and write
+ * function pointers. Rest of the fields are initialized in upper half
+ * driver.
+ *
+ * This function is called once in ipcc_register() function.
+ *
+ * Input Parameters:
+ * ipcc - interface between upper and lower half drivers. Only some
+ * of the fields should be initialized in this function.
+ * chan - channel number to initialize.
+ *
+ * Returned Value:
+ * 0 on success or negated errno otherwise.
+ *
+ * Assumptions/Limitations:
+ *
+ ****************************************************************************/
+
+int up_ipcc_init(FAR struct ipcc_lower_s *ipcc, int chan);
Review Comment:
Ok, sure, I'll change this.
--
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] mlyszczek commented on pull request #6537: [API review request] drivers/ipcc: add Inter Processor Communication driver
Posted by GitBox <gi...@apache.org>.
mlyszczek commented on PR #6537:
URL: https://github.com/apache/incubator-nuttx/pull/6537#issuecomment-1169203972
That OpenAMP looks promising and nice but I have some concerns. Mainly about memory. It seems quite complex (and definately way more complex than simple character device stream) so I suspect it requires quite a lot of memory (for an mcu)? Looking around I noticed that it requires at least one extra thread (for dispatching) that by default uses 4kB of ram. That is *a lot* considering that this will have to work on two of the CPUs. Not to mention extra code size (that I did not confirm in any way).
ST promotes openamp yes, but I've seen it's on their stm32mp* processors which have arm A7 and M4 - so there is plenty of RAM. On stm32wl5 you have 64kB, and I did not see way to enable openamp in their stm32cube.
I want to use this to communicate via LORA and draw stuff on LCD - this will take it's own share of memory, so I don't want to add extra features to the plate (even tough they seem nice) as I already am a little afraid of exhausing RAM.
So do you know how much extra memory would openamp take (ram/flash)?
--
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] onegray commented on pull request #6537: [API review request] drivers/ipcc: add Inter Processor Communication driver
Posted by GitBox <gi...@apache.org>.
onegray commented on PR #6537:
URL: https://github.com/apache/incubator-nuttx/pull/6537#issuecomment-1175568652
>> just two of them are needed for BLE
I was wrong on this, more IPCC channels are used for specific purpose. But it makes the communication protocol even more custom.
--
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] acassis commented on pull request #6537: drivers/ipcc: add Inter Processor Communication driver
Posted by GitBox <gi...@apache.org>.
acassis commented on PR #6537:
URL: https://github.com/apache/incubator-nuttx/pull/6537#issuecomment-1197060783
@mlyszczek I noticed you removed the Draft tag, is it done for mainline integration? BTW, did you see this project: https://github.com/nickbjohnson4224/fastpub maybe could give more some ideas/inspiration.
--
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] mlyszczek commented on pull request #6537: [API review request] drivers/ipcc: add Inter Processor Communication driver
Posted by GitBox <gi...@apache.org>.
mlyszczek commented on PR #6537:
URL: https://github.com/apache/incubator-nuttx/pull/6537#issuecomment-1180376334
@acassis I wouldn't remove the draft for now - I still want to write some tests for second CPU and confirm that the code does indeed work. When I confirm it - and fix possible bugs, I will rebase and upload new patch.
--
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 pull request #6537: [API review request] drivers/ipcc: add Inter Processor Communication driver
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on PR #6537:
URL: https://github.com/apache/incubator-nuttx/pull/6537#issuecomment-1170402526
> That OpenAMP looks promising and nice but I have some concerns. Mainly about memory. It seems quite complex (and definately way more complex than simple character device stream) so I suspect it requires quite a lot of memory (for an mcu)?
The major memory consumption come from tx/rx virtio queue, here is a demo config from sim:
https://github.com/apache/incubator-nuttx/blob/master/arch/sim/src/sim/up_rptun.c#L115-L121
which is quit large, but you can change both the number and size of buffer to reduce the total size less than 1KB.
> Looking around I noticed that it requires at least one extra thread (for dispatching) that by default uses 4kB of ram. That is _a lot_ considering that this will have to work on two of the CPUs. Not to mention extra code size (that I did not confirm in any way).
>
There is an option to reuse the work queue for dispatching:
https://github.com/apache/incubator-nuttx/blob/master/drivers/rptun/Kconfig#L21-L24
so the additional overhead can be shared with other component.
> ST promotes openamp yes, but I've seen it's on their stm32mp* processors which have arm A7 and M4 - so there is plenty of RAM. On stm32wl5 you have 64kB, and I did not see way to enable openamp in their stm32cube.
>
> I want to use this to communicate via LORA and draw stuff on LCD - this will take it's own share of memory, so I don't want to add extra features to the plate (even tough they seem nice) as I already am a little afraid of exhausing RAM.
>
All service is disabled by default, so the overhead is major overhead come from:
1. virtio and remoteproc code
2. IPC buffer depends on the config
3. Runtime house keeping data depends on how many endpoint you create
> So do you know how much extra memory would openamp take (ram/flash)?
My rough estimation: RAM ~2KB, Flash ~20KB
Anyway, you can provide a dummy rptun driver and get the real result quickly.
--
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] acassis commented on pull request #6537: [API review request] drivers/ipcc: add Inter Processor Communication driver
Posted by GitBox <gi...@apache.org>.
acassis commented on PR #6537:
URL: https://github.com/apache/incubator-nuttx/pull/6537#issuecomment-1180357100
@mlyszczek could you please fix these conflicts? Suggestion: maybe you can keep it depending on CONFIG_EXPERIMENTAL before more people starting using it. @xiaoxiang781216 and @onegray do you agree to let him remove the Draft flag and keep it as Experimental for now?
--
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] mlyszczek commented on a diff in pull request #6537: drivers/ipcc: add Inter Processor Communication driver
Posted by GitBox <gi...@apache.org>.
mlyszczek commented on code in PR #6537:
URL: https://github.com/apache/incubator-nuttx/pull/6537#discussion_r923641395
##########
arch/arm/src/stm32wl5/stm32wl5_ipcc.c:
##########
@@ -0,0 +1,801 @@
+/****************************************************************************
+ * arch/arm/src/stm32wl5/stm32wl5_ipcc.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/ipcc.h>
+#include <nuttx/irq.h>
+
+#include <arm_internal.h>
+#include <assert.h>
+#include <debug.h>
+
+#include <arch/stm32wl5/stm32wl5xxx_cpu1_irq.h>
+#include "hardware/stm32wl5_ipcc.h"
+#include "stm32wl5.h"
+#include "stm32wl5_ipcc.h"
+
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+/* This structure describes tx or rx of single channel in memory */
+
+struct stm32wl5_ipcc_chan_mem_s
+{
+ unsigned len; /* Number of valid bytes in data[] */
+ char data[]; /* Data in IPCC memory */
+};
+
+/* Internal stm32wl5 ipcc structure describing channel state. */
+
+struct stm32wl5_ipcc_s
+{
+ /* Pointer to API connecting upper and lower half of the driver */
+
+ struct ipcc_lower_s *ipcc;
+
+ /* Physical memory address where second CPU will write data for us,
+ * we will be reading from this memory.
+ */
+
+ char *rxmem;
+
+ /* Maximum length of data that rxmem can hold. It is size of the
+ * reserved space for rxmem minus sizeof(stm32wl5_ipcc_chan_mem_s.len)
+ */
+
+ unsigned rxlen;
+
+#if CONFIG_IPCC_BUFFERED
+ /* Number of bytes copied from IPCC memory to buffer. Can be less than
+ * stm32wl5_ipcc_chan_mem_s.len after copy operation when buffer is full.
+ * Value can persist between multiple ISR and stm32wl5_ipcc_buffer_data()
+ * calls, until all data from IPCC memory is successfully buffered.
+ */
+
+ unsigned rxcopied;
+#endif
+
+ /* Physical memory address where we will write data for the second
+ * CPU to read.
+ */
+
+ char *txmem;
+
+ /* Maximum length of data that txmem can hold. It is size of the
+ * reserved space for txmem minus sizeof(stm32wl5_ipcc_chan_mem_s.len)
+ */
+
+ unsigned txlen;
+};
+
+/****************************************************************************
+ * Private Function Prototypes
+ ****************************************************************************/
+
+static ssize_t stm32wl5_ipcc_read(struct ipcc_lower_s *ipcc,
+ char *buffer, size_t buflen);
+static ssize_t stm32wl5_ipcc_write(struct ipcc_lower_s *ipcc,
+ const char *buffer, size_t buflen);
+static ssize_t stm32wl5_ipcc_buffer_data(struct ipcc_lower_s *ipcc,
+ struct circbuf_s *rxbuf);
+static ssize_t stm32wl5_ipcc_copy_to_buffer(int chan,
+ struct circbuf_s *rxbuf);
+static int stm32wl5_ipcc_rx_isr(int irq, void *context, void *arg);
+static int stm32wl5_ipcc_tx_isr(int irq, void *context, void *arg);
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+struct stm32wl5_ipcc_s g_ipccpriv[IPCC_NCHAN] =
+{
+ /* Channel 1 is always enabled when IPCC is enabled */
+
+ {
+ .rxmem = (char *)(IPCC_CHAN1_START),
+ .rxlen = IPCC_CHAN1_RX_SIZE - sizeof(unsigned),
+ .txmem = (char *)(IPCC_CHAN1_START + IPCC_CHAN1_RX_SIZE),
+ .txlen = IPCC_CHAN1_TX_SIZE - sizeof(unsigned)
+ }
+
+#if IPCC_CHAN2
+ ,
+ {
+ .rxmem = (char *)(IPCC_CHAN2_START),
+ .rxlen = IPCC_CHAN2_RX_SIZE - sizeof(unsigned),
+ .txmem = (char *)(IPCC_CHAN2_START + IPCC_CHAN2_RX_SIZE),
+ .txlen = IPCC_CHAN2_TX_SIZE - sizeof(unsigned)
+ }
+#endif
+
+#if IPCC_CHAN3
+ ,
+ {
+ .rxmem = (char *)(IPCC_CHAN3_START),
+ .rxlen = IPCC_CHAN3_RX_SIZE - sizeof(unsigned),
+ .txmem = (char *)(IPCC_CHAN3_START + IPCC_CHAN3_RX_SIZE),
+ .txlen = IPCC_CHAN3_TX_SIZE - sizeof(unsigned)
+ }
+#endif
+
+#if IPCC_CHAN4
+ ,
+ {
+ .rxmem = (char *)(IPCC_CHAN4_START),
+ .rxlen = IPCC_CHAN4_RX_SIZE - sizeof(unsigned),
+ .txmem = (char *)(IPCC_CHAN4_START + IPCC_CHAN4_RX_SIZE),
+ .txlen = IPCC_CHAN4_TX_SIZE - sizeof(unsigned)
+ }
+#endif
+
+#if IPCC_CHAN5
+ ,
+ {
+ .rxmem = (char *)(IPCC_CHAN5_START),
+ .rxlen = IPCC_CHAN5_RX_SIZE - sizeof(unsigned),
+ .txmem = (char *)(IPCC_CHAN5_START + IPCC_CHAN5_RX_SIZE),
+ .txlen = IPCC_CHAN5_TX_SIZE - sizeof(unsigned)
+ }
+#endif
+
+#if IPCC_CHAN6
+ ,
+ {
+ .rxmem = (char *)(IPCC_CHAN6_START),
+ .rxlen = IPCC_CHAN6_RX_SIZE - sizeof(unsigned),
+ .txmem = (char *)(IPCC_CHAN6_START + IPCC_CHAN6_RX_SIZE),
+ .txlen = IPCC_CHAN6_TX_SIZE - sizeof(unsigned)
+ }
+#endif
+};
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: stm32wl5_ipcc_tx_isr
+ *
+ * Description:
+ * IPCC TX interrupt service routine. This interrupt is called when
+ * second CPU read all data from IPCC TX memory and we are free to
+ * write new data to that memory.
+ *
+ * For buffered IPCC, we will immediately write data from buffer to
+ * IPCC memory and notify second CPU. Also blocked writer will be
+ * notified that there is free space on tx buffer.
+ *
+ * For unbuffered IPCC, we won't copy anything but only notify blocked
+ * writers that IPCC TX memory is free.
+ *
+ * Input Parameters:
+ * irq - Number of IRQ that generated interrupt
+ * context - Interrupt register state save info (architecture-specific)
+ * arg - user data (not used by us)
+ *
+ * Returned Value:
+ * Always OK
+ *
+ * Assumptions/Limitations:
+ *
+ ****************************************************************************/
+
+static int stm32wl5_ipcc_tx_isr(int irq, void *context, void *arg)
+{
+ int chan;
+ size_t nwritten;
+ uint32_t mr;
+ uint32_t sr;
+ uint32_t status;
+ struct stm32wl5_ipcc_s *priv;
+ struct stm32wl5_ipcc_chan_mem_s *txmem;
+
+ (void)context;
+ (void)arg;
+ (void)irq;
+
+ /* Disable TX interrupt, we don't want two "threads" to concurently
+ * modify circbuffers as we don't use any guard to them
+ */
+
+ up_disable_irq(STM32WL5_IRQ_IPCC_C1_TX_IT);
+
+ mr = getreg32(STM32WL5_IPCC_C1MR) >> STM32WL5_IPCC_TX_SHIFT;
+ sr = getreg32(STM32WL5_IPCC_C1TOC2SR);
+
+ /* Consider only channels that have tx memory free and are unmasked */
+
+ status = sr | mr;
Review Comment:
Actually I did mean OR here.
If status is ```true``` then transmit channel is either masked or memory is not ready to be written. sr == 1, means "Channel occupied data can be read by second cpu" - so we cannot write to it. mr == 1, means channel is masked. So if any of these values are 1, then we cannot write to the memory.
--
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 diff in pull request #6537: drivers/ipcc: add Inter Processor Communication driver
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #6537:
URL: https://github.com/apache/incubator-nuttx/pull/6537#discussion_r928107217
##########
drivers/ipcc/ipcc_close.c:
##########
@@ -0,0 +1,92 @@
+/****************************************************************************
+ * drivers/ipcc/ipcc_close.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/ipcc.h>
+#include <nuttx/kmalloc.h>
+#include <nuttx/semaphore.h>
+
+#include <assert.h>
+#include <errno.h>
+#include <stdbool.h>
+#include <stdio.h>
+#include <sys/types.h>
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: ipcc_close
+ *
+ * Description:
+ * Closes the driver device. If this is last reference and file has been
+ * unlinked, we will also free resources allocated by ipcc_register()
+ *
+ * Input Parameters:
+ * filep - pointer to a file structure to close.
+ *
+ * Returned Value:
+ * OK on successfull close, or negated errno on failure.
+ *
+ * Assumptions/Limitations:
+ *
+ ****************************************************************************/
+
+int ipcc_close(FAR struct file *filep)
+{
+ FAR struct ipcc_driver_s *priv;
+ int ret;
+
+ /* Get our private data structure */
+
+ DEBUGASSERT(filep != NULL && filep->f_inode != NULL);
+ priv = filep->f_inode->i_private;
+
+ /* Get exclusive access to the IPCC driver state structure */
+
+ if ((ret = nxsem_wait(&priv->exclsem)))
Review Comment:
< 0
##########
include/nuttx/ipcc.h:
##########
@@ -0,0 +1,212 @@
+/****************************************************************************
+ * include/nuttx/ipcc.h
+ *
+ * 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.
+ *
+ ****************************************************************************/
+
+#ifndef __INCLUDE_NUTTX_IPCC_H
+#define __INCLUDE_NUTTX_IPCC_H
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+#include <nuttx/mm/circbuf.h>
+#include <nuttx/semaphore.h>
+
+#include <poll.h>
+
+/****************************************************************************
+ * Public Types
+ ****************************************************************************/
+
+/* IPCC is a two-part driver:
+ *
+ * 1) A common upper half driver, that provides the character device
+ * interface for the user
+ * 2) Platform-specific lower half drivers that provide interface
+ * between upper and lower layer. Lower half also contains
+ * common objects like semaphore to inform
+ */
+
+/* This structure defines all of the operations provided by the
+ * architecture specific logic. All fields must be provided with
+ * non-NULL function pointers.
+ */
+
+struct ipcc_lower_s;
+struct ipcc_ops_s
+{
+ /**************************************************************************
+ * Name: read
+ *
+ * Description:
+ * Reads data directly from IPCC mbox, function shall not block.
+ * If no data is available, 0 shall be returned. Partial read
+ * is possible. In that case function can return value less
+ * than buflen.
+ *
+ * Input Parameters:
+ * ipcc - ipcc channel to read data from
+ * buffer - location where data shall be stored
+ * buflen - size of the buffer
+ *
+ * Returned Value:
+ * Number of bytes actually written to buffer.
+ *
+ **************************************************************************/
+
+ CODE ssize_t (*read)(FAR struct ipcc_lower_s *ipcc,
+ FAR char *buffer, size_t buflen);
+
+ /**************************************************************************
+ * Name: write
+ *
+ * Description:
+ * Writes data directly to IPCC mbox, function shall not block.
+ * If IPCC memory is full, or busy or otherwise unavailable for
+ * write, 0 shall be returned. Partial write is possible. In
+ * that case function can return value less then buflen.
+ *
+ * Input Parameters:
+ * ipcc - ipcc channel to write data to
+ * buffer - location to read data from
+ * buflen - number of bytes to write from buffer
+ *
+ * Returned Value:
+ * Number of actually written data to IPCC channel.
+ *
+ **************************************************************************/
+
+ CODE ssize_t (*write)(FAR struct ipcc_lower_s *ipcc,
+ FAR const char *buffer, size_t buflen);
+
+ /**************************************************************************
+ * Name: buffer_data
+ *
+ * Description:
+ * Lower driver shall copy data from IPCC to cirbuf on demand.
+ * It does not have to copy all data (in case rxbuf is full), but
+ * it must keep internal pointer to track uncopied data, and copy
+ * what is left on the next call to this function.
+ *
+ * Input Parameters:
+ * ipcc - channel to buffer data
+ * rxbuf - circural buffer where data should be written to
+ *
+ * Returned Value:
+ * Number of bytes that were successfully written to rxbuf.
+ *
+ **************************************************************************/
+
+ CODE ssize_t (*buffer_data)(FAR struct ipcc_lower_s *ipcc,
+ FAR struct circbuf_s *rxbuf);
+
+ /**************************************************************************
+ * Name: write_notify
+ *
+ * Description:
+ * Upper half driver notifies lower half driver that new data has been
+ * copied to circ buffer. This can be used by lower half to transfer
+ * data from buffer to IPCC memory.
+ *
+ * Input Parameters:
+ * ipcc - ipcc channel that wrote data.
+ *
+ * Returned Value:
+ * 0 on success, or negated errno otherwise
+ *
+ **************************************************************************/
+
+ CODE ssize_t (*write_notify)(FAR struct ipcc_lower_s *ipcc);
+
+ /**************************************************************************
+ * Name: cleanup
+ *
+ * Description:
+ * Cleans up resources initialized by <arch>_ipcc_init(). If arch code
+ * malloc()ed any memory, this funciton is responsible of freeing it.
+ * After this function is called, ipcc driver will not access lower
+ * half pointer anymore.
+ *
+ * Input Parameters:
+ * ipcc - ipcc channel to cleanup
+ *
+ * Returned Value:
+ * Always OK
+ *
+ **************************************************************************/
+
+ CODE int (*cleanup)(FAR struct ipcc_lower_s *ipcc);
+};
+
+/* This structure defines the interface between upper and lower halves.
+ * Such instance is passed to ipcc_register() function after driver
+ * has been initialized, binding the upper and lower halves into one
+ * driver.
+ */
+
+struct ipcc_lower_s
+{
+ FAR struct pollfd *fds[CONFIG_IPCC_NPOLLWAITERS];
Review Comment:
most fields in ipcc_lower_s except ops should be part of ipcc_driver_s instead
##########
drivers/ipcc/ipcc_register.c:
##########
@@ -0,0 +1,207 @@
+/****************************************************************************
+ * drivers/ipcc/ipcc_register.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/ipcc.h>
+#include <nuttx/kmalloc.h>
+#include <nuttx/mm/circbuf.h>
+
+#include <assert.h>
+#include <errno.h>
+#include <stdbool.h>
+#include <stdio.h>
+#include <sys/types.h>
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/* Device naming ************************************************************/
+
+#define DEVNAME_FMT "/dev/ipcc%d"
+#define DEVNAME_FMTLEN (9 + 3 + 1)
+
+/****************************************************************************
+ * Private Function Prototypes
+ ****************************************************************************/
+
+int ipcc_open(FAR struct file *filep);
+int ipcc_close(FAR struct file *filep);
+#ifndef CONFIG_DISABLE_PSEUDOFS_OPERATIONS
+int ipcc_unlink(FAR struct inode *inode);
+#endif
+int ipcc_ioctl(FAR struct file *filep, int cmd, unsigned long arg);
+ssize_t ipcc_read(FAR struct file *filep, FAR char *buffer, size_t buflen);
+ssize_t ipcc_write(FAR struct file *filep, FAR const char *buffer,
+ size_t buflen);
+int ipcc_poll(FAR struct file *filep, FAR struct pollfd *fds,
+ bool setup);
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+static const struct file_operations ipcc_fops =
+{
+#ifdef CONFIG_DISABLE_PSEUDOFS_OPERATIONS
+ .unlink = NULL,
+#else /* CONFIG_DISABLE_PSEUDOFS_OPERATIONS */
+ .unlink = ipcc_unlink,
+#endif /* CONFIG_DISABLE_PSEUDOFS_OPERATIONS */
+ .open = ipcc_open,
+ .close = ipcc_close,
+ .poll = ipcc_poll,
+ .read = ipcc_read,
+ .write = ipcc_write,
+ .ioctl = NULL,
+ .seek = NULL
+};
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: ipcc_cleanup
+ *
+ * Description:
+ * Cleans up resources allocated by ipcc_register()
+ *
+ * Input Parameters:
+ * priv - ipcc driver instance to clean up
+ *
+ * Returned Value:
+ * None
+ *
+ * Assumptions/Limitations:
+ * This function should be called only when ipcc_register is run with
+ * success and all resources in driver instance are properly allocated.
+ *
+ ****************************************************************************/
+
+void ipcc_cleanup(FAR struct ipcc_driver_s *priv)
+{
+#ifdef CONFIG_IPCC_BUFFERED
+ circbuf_uninit(&priv->ipcc->rxbuf);
+ circbuf_uninit(&priv->ipcc->txbuf);
+#endif
+ nxsem_destroy(&priv->ipcc->rxsem);
+ nxsem_destroy(&priv->ipcc->txsem);
+ priv->ipcc->ops.cleanup(priv->ipcc);
+ nxsem_destroy(&priv->exclsem);
+ kmm_free(priv);
+}
+
+/****************************************************************************
+ * Name: ipcc_register
+ *
+ * Description:
+ * Create and register the IPCC character driver.
+ *
+ * IPCC is a simple character driver that supports inter processor
+ * communication.
+ *
+ * Input Parameters:
+ * ipcc - An instance of the lower half IPCC driver
+ * chan - IPCC channel. This will be used ad IPCC minor number.
+ * IPPC will be registered as /dev/ipccN where N is the minor number.
+ * buflen - Length of tx and rx buffers, 0 for unbuffered communication.
+ *
+ * Returned Value:
+ * OK if the driver was successfully registered, or negated errno on
+ * failure.
+ *
+ * Assumptions/Limitations:
+ * ipcc is already allocated and initialized by architecture code.
+ *
+ ****************************************************************************/
+
+#ifdef CONFIG_IPCC_BUFFERED
+int ipcc_register(FAR struct ipcc_lower_s *ipcc, size_t rxbuflen,
+ size_t txbuflen)
+#else
+int ipcc_register(FAR struct ipcc_lower_s *ipcc)
+#endif
+{
+ FAR struct ipcc_driver_s *priv;
+ char devname[DEVNAME_FMTLEN];
+ int ret;
+
+ /* Allocate a IPCC character device structure */
+
+ if ((priv = kmm_zalloc(sizeof(*priv))) == NULL)
+ {
+ return -ENOMEM;
+ }
+
+ /* Link upper and lower driver together */
+
+ priv->ipcc = ipcc;
+
+#ifdef CONFIG_IPCC_BUFFERED
+ /* allocate buffers for reading and writing data to IPCC memory */
+
+ if (rxbuflen)
+ {
+ if ((ret = circbuf_init(&priv->ipcc->rxbuf, NULL, rxbuflen)))
+ {
+ goto error;
+ }
+ }
+
+ if (txbuflen)
+ {
+ if ((ret = circbuf_init(&priv->ipcc->txbuf, NULL, txbuflen)))
+ {
+ goto error;
+ }
+ }
+#endif /* CONFIG_IPCC_BUFFERED */
+
+ /* Create the character device name */
+
+ snprintf(devname, DEVNAME_FMTLEN, DEVNAME_FMT, ipcc->chan);
+ if ((ret = register_driver(devname, &ipcc_fops, 0666, priv)))
+ {
+ goto error;
+ }
+
+ /* nxsem_init can't really fail us if we provide it with valid params */
+
+ nxsem_init(&priv->exclsem, 0, 1);
+ nxsem_init(&priv->ipcc->rxsem, 0, 0);
+ nxsem_init(&priv->ipcc->txsem, 0, 1);
+
+ return OK;
+
+error:
+#ifdef CONFIG_IPCC_BUFFERED
+ circbuf_uninit(&priv->ipcc->rxbuf);
+ circbuf_uninit(&priv->ipcc->txbuf);
+#endif
+ ipcc_cleanup(priv);
+ kmm_free(priv);
Review Comment:
call kmm_free and circbuf_uninit dobule time
##########
drivers/ipcc/ipcc_register.c:
##########
@@ -0,0 +1,207 @@
+/****************************************************************************
+ * drivers/ipcc/ipcc_register.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/ipcc.h>
+#include <nuttx/kmalloc.h>
+#include <nuttx/mm/circbuf.h>
+
+#include <assert.h>
+#include <errno.h>
+#include <stdbool.h>
+#include <stdio.h>
+#include <sys/types.h>
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/* Device naming ************************************************************/
+
+#define DEVNAME_FMT "/dev/ipcc%d"
+#define DEVNAME_FMTLEN (9 + 3 + 1)
+
+/****************************************************************************
+ * Private Function Prototypes
+ ****************************************************************************/
+
+int ipcc_open(FAR struct file *filep);
+int ipcc_close(FAR struct file *filep);
+#ifndef CONFIG_DISABLE_PSEUDOFS_OPERATIONS
+int ipcc_unlink(FAR struct inode *inode);
+#endif
+int ipcc_ioctl(FAR struct file *filep, int cmd, unsigned long arg);
+ssize_t ipcc_read(FAR struct file *filep, FAR char *buffer, size_t buflen);
+ssize_t ipcc_write(FAR struct file *filep, FAR const char *buffer,
+ size_t buflen);
+int ipcc_poll(FAR struct file *filep, FAR struct pollfd *fds,
Review Comment:
move to drivers/ipcc/ipcc.h
##########
include/nuttx/ipcc.h:
##########
@@ -0,0 +1,212 @@
+/****************************************************************************
+ * include/nuttx/ipcc.h
+ *
+ * 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.
+ *
+ ****************************************************************************/
+
+#ifndef __INCLUDE_NUTTX_IPCC_H
+#define __INCLUDE_NUTTX_IPCC_H
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+#include <nuttx/mm/circbuf.h>
+#include <nuttx/semaphore.h>
+
+#include <poll.h>
+
+/****************************************************************************
+ * Public Types
+ ****************************************************************************/
+
+/* IPCC is a two-part driver:
+ *
+ * 1) A common upper half driver, that provides the character device
+ * interface for the user
+ * 2) Platform-specific lower half drivers that provide interface
+ * between upper and lower layer. Lower half also contains
+ * common objects like semaphore to inform
+ */
+
+/* This structure defines all of the operations provided by the
+ * architecture specific logic. All fields must be provided with
+ * non-NULL function pointers.
+ */
+
+struct ipcc_lower_s;
+struct ipcc_ops_s
+{
+ /**************************************************************************
+ * Name: read
+ *
+ * Description:
+ * Reads data directly from IPCC mbox, function shall not block.
+ * If no data is available, 0 shall be returned. Partial read
+ * is possible. In that case function can return value less
+ * than buflen.
+ *
+ * Input Parameters:
+ * ipcc - ipcc channel to read data from
+ * buffer - location where data shall be stored
+ * buflen - size of the buffer
+ *
+ * Returned Value:
+ * Number of bytes actually written to buffer.
+ *
+ **************************************************************************/
+
+ CODE ssize_t (*read)(FAR struct ipcc_lower_s *ipcc,
+ FAR char *buffer, size_t buflen);
+
+ /**************************************************************************
+ * Name: write
+ *
+ * Description:
+ * Writes data directly to IPCC mbox, function shall not block.
+ * If IPCC memory is full, or busy or otherwise unavailable for
+ * write, 0 shall be returned. Partial write is possible. In
+ * that case function can return value less then buflen.
+ *
+ * Input Parameters:
+ * ipcc - ipcc channel to write data to
+ * buffer - location to read data from
+ * buflen - number of bytes to write from buffer
+ *
+ * Returned Value:
+ * Number of actually written data to IPCC channel.
+ *
+ **************************************************************************/
+
+ CODE ssize_t (*write)(FAR struct ipcc_lower_s *ipcc,
+ FAR const char *buffer, size_t buflen);
+
+ /**************************************************************************
+ * Name: buffer_data
+ *
+ * Description:
+ * Lower driver shall copy data from IPCC to cirbuf on demand.
+ * It does not have to copy all data (in case rxbuf is full), but
+ * it must keep internal pointer to track uncopied data, and copy
+ * what is left on the next call to this function.
+ *
+ * Input Parameters:
+ * ipcc - channel to buffer data
+ * rxbuf - circural buffer where data should be written to
+ *
+ * Returned Value:
+ * Number of bytes that were successfully written to rxbuf.
+ *
+ **************************************************************************/
+
+ CODE ssize_t (*buffer_data)(FAR struct ipcc_lower_s *ipcc,
+ FAR struct circbuf_s *rxbuf);
+
+ /**************************************************************************
+ * Name: write_notify
+ *
+ * Description:
+ * Upper half driver notifies lower half driver that new data has been
+ * copied to circ buffer. This can be used by lower half to transfer
+ * data from buffer to IPCC memory.
+ *
+ * Input Parameters:
+ * ipcc - ipcc channel that wrote data.
+ *
+ * Returned Value:
+ * 0 on success, or negated errno otherwise
+ *
+ **************************************************************************/
+
+ CODE ssize_t (*write_notify)(FAR struct ipcc_lower_s *ipcc);
+
+ /**************************************************************************
+ * Name: cleanup
+ *
+ * Description:
+ * Cleans up resources initialized by <arch>_ipcc_init(). If arch code
+ * malloc()ed any memory, this funciton is responsible of freeing it.
+ * After this function is called, ipcc driver will not access lower
+ * half pointer anymore.
+ *
+ * Input Parameters:
+ * ipcc - ipcc channel to cleanup
+ *
+ * Returned Value:
+ * Always OK
+ *
+ **************************************************************************/
+
+ CODE int (*cleanup)(FAR struct ipcc_lower_s *ipcc);
+};
+
+/* This structure defines the interface between upper and lower halves.
+ * Such instance is passed to ipcc_register() function after driver
+ * has been initialized, binding the upper and lower halves into one
+ * driver.
+ */
+
+struct ipcc_lower_s
+{
+ FAR struct pollfd *fds[CONFIG_IPCC_NPOLLWAITERS];
+ FAR struct ipcc_ops_s ops; /* Arch specific functions */
+ sem_t rxsem; /* Data ready to read semaphore */
+ sem_t txsem; /* Data ready to send semaphore */
+ int chan; /* IPCC channel */
+
+#ifdef CONFIG_IPCC_BUFFERED
+ struct circbuf_s rxbuf; /* Receive buffer */
+ struct circbuf_s txbuf; /* Transmit buffer */
+
+ /* 1- not all data could fit into buffer, some unread data is still
+ * in IPCC mbox. State of this variable is maintained by lower half,
+ * upper half only reads it to know when to inform lower driver that
+ * buffer got free.
+ */
+
+ int overflow;
+#endif /* CONFIG_IPCC_BUFFERED */
+};
+
+/* Driver state structure */
+
+struct ipcc_driver_s
Review Comment:
move to the source file
##########
drivers/ipcc/ipcc_open.c:
##########
@@ -0,0 +1,78 @@
+/****************************************************************************
+ * drivers/ipcc/ipcc_open.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/ipcc.h>
+
+#include <assert.h>
+#include <errno.h>
+#include <stdbool.h>
+#include <stdio.h>
+#include <sys/types.h>
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: ipcc_open
+ *
+ * Description:
+ * Opens driver for use by userspace applications.
+ *
+ * Input Parameters:
+ * filep - pointer to a file structure to open.
+ *
+ * Returned Value:
+ * OK on successfull open, or negated errno on failure.
+ *
+ * Assumptions/Limitations:
+ *
+ ****************************************************************************/
+
+int ipcc_open(FAR struct file *filep)
+{
+ FAR struct ipcc_driver_s *priv;
+ int ret;
+
+ /* Get our private data structure */
+
+ DEBUGASSERT(filep != NULL && filep->f_inode != NULL);
+ priv = filep->f_inode->i_private;
+
+ /* Get exclusive access to the IPCC driver state structure */
+
+ if ((ret = nxsem_wait(&priv->exclsem)))
Review Comment:
< 0
##########
drivers/ipcc/ipcc_unlink.c:
##########
@@ -0,0 +1,94 @@
+/****************************************************************************
+ * drivers/ipcc/ipcc_unlink.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/ipcc.h>
+#include <nuttx/kmalloc.h>
+#include <nuttx/semaphore.h>
+
+#include <assert.h>
+#include <errno.h>
+#include <stdbool.h>
+#include <stdio.h>
+#include <sys/types.h>
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: ipcc_unlink
+ *
+ * Description:
+ * Action to take upon file unlinking. Function will free resources if
+ * noone is using the driver when unlinking occured. If driver is still
+ * in use, it will be marked as unlinked and resource freeing will take
+ * place in ipcc_close() function instead, once last reference is closed.
+ *
+ * Input Parameters:
+ * inode - driver inode that is being unlinked
+ *
+ * Returned Value:
+ * OK on successfull close, or negated errno on failure.
+ *
+ * Assumptions/Limitations:
+ *
+ ****************************************************************************/
+
+int ipcc_unlink(FAR struct inode *inode)
+{
+ FAR struct ipcc_driver_s *priv;
+ int ret;
+
+ /* Get our private data structure */
+
+ DEBUGASSERT(inode);
+ priv = inode->i_private;
+
+ /* Get exclusive access to the IPCC driver state structure */
+
+ if ((ret = nxsem_wait(&priv->exclsem)))
Review Comment:
< 0
##########
include/nuttx/ipcc.h:
##########
@@ -0,0 +1,212 @@
+/****************************************************************************
+ * include/nuttx/ipcc.h
+ *
+ * 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.
+ *
+ ****************************************************************************/
+
+#ifndef __INCLUDE_NUTTX_IPCC_H
+#define __INCLUDE_NUTTX_IPCC_H
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+#include <nuttx/mm/circbuf.h>
+#include <nuttx/semaphore.h>
+
+#include <poll.h>
+
+/****************************************************************************
+ * Public Types
+ ****************************************************************************/
+
+/* IPCC is a two-part driver:
+ *
+ * 1) A common upper half driver, that provides the character device
+ * interface for the user
+ * 2) Platform-specific lower half drivers that provide interface
+ * between upper and lower layer. Lower half also contains
+ * common objects like semaphore to inform
+ */
+
+/* This structure defines all of the operations provided by the
+ * architecture specific logic. All fields must be provided with
+ * non-NULL function pointers.
+ */
+
+struct ipcc_lower_s;
+struct ipcc_ops_s
+{
+ /**************************************************************************
+ * Name: read
+ *
+ * Description:
+ * Reads data directly from IPCC mbox, function shall not block.
+ * If no data is available, 0 shall be returned. Partial read
+ * is possible. In that case function can return value less
+ * than buflen.
+ *
+ * Input Parameters:
+ * ipcc - ipcc channel to read data from
+ * buffer - location where data shall be stored
+ * buflen - size of the buffer
+ *
+ * Returned Value:
+ * Number of bytes actually written to buffer.
+ *
+ **************************************************************************/
+
+ CODE ssize_t (*read)(FAR struct ipcc_lower_s *ipcc,
+ FAR char *buffer, size_t buflen);
+
+ /**************************************************************************
+ * Name: write
+ *
+ * Description:
+ * Writes data directly to IPCC mbox, function shall not block.
+ * If IPCC memory is full, or busy or otherwise unavailable for
+ * write, 0 shall be returned. Partial write is possible. In
+ * that case function can return value less then buflen.
+ *
+ * Input Parameters:
+ * ipcc - ipcc channel to write data to
+ * buffer - location to read data from
+ * buflen - number of bytes to write from buffer
+ *
+ * Returned Value:
+ * Number of actually written data to IPCC channel.
+ *
+ **************************************************************************/
+
+ CODE ssize_t (*write)(FAR struct ipcc_lower_s *ipcc,
+ FAR const char *buffer, size_t buflen);
+
+ /**************************************************************************
+ * Name: buffer_data
+ *
+ * Description:
+ * Lower driver shall copy data from IPCC to cirbuf on demand.
+ * It does not have to copy all data (in case rxbuf is full), but
+ * it must keep internal pointer to track uncopied data, and copy
+ * what is left on the next call to this function.
+ *
+ * Input Parameters:
+ * ipcc - channel to buffer data
+ * rxbuf - circural buffer where data should be written to
+ *
+ * Returned Value:
+ * Number of bytes that were successfully written to rxbuf.
+ *
+ **************************************************************************/
+
+ CODE ssize_t (*buffer_data)(FAR struct ipcc_lower_s *ipcc,
+ FAR struct circbuf_s *rxbuf);
+
+ /**************************************************************************
+ * Name: write_notify
+ *
+ * Description:
+ * Upper half driver notifies lower half driver that new data has been
+ * copied to circ buffer. This can be used by lower half to transfer
+ * data from buffer to IPCC memory.
+ *
+ * Input Parameters:
+ * ipcc - ipcc channel that wrote data.
+ *
+ * Returned Value:
+ * 0 on success, or negated errno otherwise
+ *
+ **************************************************************************/
+
+ CODE ssize_t (*write_notify)(FAR struct ipcc_lower_s *ipcc);
+
+ /**************************************************************************
+ * Name: cleanup
+ *
+ * Description:
+ * Cleans up resources initialized by <arch>_ipcc_init(). If arch code
+ * malloc()ed any memory, this funciton is responsible of freeing it.
+ * After this function is called, ipcc driver will not access lower
+ * half pointer anymore.
+ *
+ * Input Parameters:
+ * ipcc - ipcc channel to cleanup
+ *
+ * Returned Value:
+ * Always OK
+ *
+ **************************************************************************/
+
+ CODE int (*cleanup)(FAR struct ipcc_lower_s *ipcc);
+};
+
+/* This structure defines the interface between upper and lower halves.
+ * Such instance is passed to ipcc_register() function after driver
+ * has been initialized, binding the upper and lower halves into one
+ * driver.
+ */
+
+struct ipcc_lower_s
+{
+ FAR struct pollfd *fds[CONFIG_IPCC_NPOLLWAITERS];
+ FAR struct ipcc_ops_s ops; /* Arch specific functions */
+ sem_t rxsem; /* Data ready to read semaphore */
+ sem_t txsem; /* Data ready to send semaphore */
+ int chan; /* IPCC channel */
+
+#ifdef CONFIG_IPCC_BUFFERED
+ struct circbuf_s rxbuf; /* Receive buffer */
+ struct circbuf_s txbuf; /* Transmit buffer */
+
+ /* 1- not all data could fit into buffer, some unread data is still
+ * in IPCC mbox. State of this variable is maintained by lower half,
+ * upper half only reads it to know when to inform lower driver that
+ * buffer got free.
+ */
+
+ int overflow;
+#endif /* CONFIG_IPCC_BUFFERED */
+};
+
+/* Driver state structure */
+
+struct ipcc_driver_s
+{
+ FAR struct ipcc_lower_s *ipcc; /* Lower half driver state */
+ int crefs; /* Count number of open references */
+ int unlinked; /* 1 - driver has been unlinked */
+ sem_t exclsem; /* Mutual exclusion for driver */
+};
+
+/****************************************************************************
+ * Public Functions Prototypes
+ ****************************************************************************/
+
+void ipcc_pollnotify(FAR struct ipcc_lower_s *ipcc, pollevent_t eventset);
+void ipcc_cleanup(FAR struct ipcc_driver_s *priv);
+void ipcc_txfree_notify(FAR struct ipcc_lower_s *priv);
Review Comment:
priv->ipcc
##########
drivers/ipcc/ipcc_poll.c:
##########
@@ -0,0 +1,201 @@
+/****************************************************************************
+ * drivers/ipcc/ipcc_poll.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/ipcc.h>
+#include <nuttx/kmalloc.h>
+#include <nuttx/semaphore.h>
+
+#include <assert.h>
+#include <debug.h>
+#include <errno.h>
+#include <fcntl.h>
+#include <poll.h>
+#include <stdbool.h>
+#include <stdio.h>
+#include <sys/types.h>
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: ipcc_poll
+ *
+ * Description:
+ * Sets up or tears down poll for the driver associated with filep pointer
+ *
+ * Input Parameters:
+ * filep - file associated with the driver instance
+ * fds - The structure describing the events to be monitored, OR NULL if
+ * this is a request to stop monitoring events.
+ * setup - true: Setup up the poll; false: Teardown the poll
+ *
+ * Returned Value:
+ * 0: Success; Negated errno on failure
+ *
+ * Assumptions/Limitations:
+ *
+ ****************************************************************************/
+
+int ipcc_poll(FAR struct file *filep, FAR struct pollfd *fds,
+ bool setup)
+{
+ FAR struct ipcc_driver_s *priv;
+ FAR struct pollfd **slot;
+ pollevent_t eventset;
+ int ret;
+ int i;
+
+ /* Get our private data structure */
+
+ DEBUGASSERT(filep != NULL && filep->f_inode != NULL);
+ priv = filep->f_inode->i_private;
+
+ /* Get exclusive access to driver */
+
+ if ((ret = nxsem_wait(&priv->exclsem)))
Review Comment:
< 0
##########
drivers/ipcc/ipcc_write.c:
##########
@@ -0,0 +1,241 @@
+/****************************************************************************
+ * drivers/ipcc/ipcc_write.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/ipcc.h>
+#include <nuttx/kmalloc.h>
+#include <nuttx/semaphore.h>
+
+#include <assert.h>
+#include <errno.h>
+#include <fcntl.h>
+#include <stdbool.h>
+#include <stdio.h>
+#include <sys/types.h>
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: ipcc_txfree_notify
+ *
+ * Description:
+ * Notifies all blocked threads or those waiting in poll/select that
+ * there is place on buffer to perform writing.
+ *
+ * Input Parameters:
+ * ipcc - pointer to driver instance
+ *
+ * Returned Value:
+ * None
+ *
+ * Assumptions/Limitations:
+ * This function can be called from interrupt handler from lower half.
+ *
+ ****************************************************************************/
+
+void ipcc_txfree_notify(FAR struct ipcc_lower_s *ipcc)
+{
+ int semval;
+
+ if ((nxsem_get_value(&ipcc->txsem, &semval) == 0) && semval > 0)
+ {
+ /* Notify all poll/select waiters that they can write to the driver.
+ * Do it only when there are no already blocked writers to avoid
+ * situation where thread that is polling gets notified only to
+ * be blocked in write() because another thread have written to
+ * buffer before polling thread could.
+ */
+
+ ipcc_pollnotify(ipcc, POLLOUT);
+ return;
+ }
+
+ /* Notify all blocked writers that data is available to write */
+
+ do
+ {
+ nxsem_post(&ipcc->txsem);
+ }
+ while (nxsem_get_value(&ipcc->txsem, &semval) == 0 && semval <= 0);
+}
+
+/****************************************************************************
+ * Name: ipcc_write
+ *
+ * Description:
+ * Writes data to IPCC memory so that another CPU can read the contents.
+ * Will block untill whole buffer is copied unless signal is received
+ * or O_NONBLOCK flag is set.
+ *
+ * Input Parameters:
+ * filep - file on vfs associated with the driver
+ * buffer - buffer to copy to IPCC memory
+ * buflen - size of the buffer... buffer
+ *
+ * Returned Value:
+ * Number of successfully written bytes into the IPCC memory or netagted
+ * errno when no data could be written.
+ *
+ * Assumptions/Limitations:
+ *
+ ****************************************************************************/
+
+ssize_t ipcc_write(FAR struct file *filep, FAR const char *buffer,
+ size_t buflen)
+{
+ FAR struct ipcc_driver_s *priv;
+ ssize_t nwritten;
+ int ret;
+
+ /* Get our private data structure */
+
+ DEBUGASSERT(filep != NULL && filep->f_inode != NULL);
+ priv = filep->f_inode->i_private;
+
+ /* Get exclusive access to driver */
+
+ if ((ret = nxsem_wait(&priv->exclsem)))
+ {
+ /* nxsem_wait() will return on signal, we did not start
+ * any transfer yet, so we can safely return with error
+ */
+
+ return ret;
+ }
+
+ for (nwritten = 0; ; )
+ {
+#ifdef CONFIG_IPCC_BUFFERED
+ /* Buffered write, if buffer is empty try to write directly to
+ * IPCC memory, else buffer data in circbuf - it will be written
+ * in interrupt handler when IPCC tx channel is free.
+ */
+
+ if (circbuf_used(&priv->ipcc->txbuf) == 0)
+ {
+ /* Write buffer is empty, we can try and write data directly
+ * to IPCC memory thus omitting copying to buffer.
+ */
+
+ nwritten += priv->ipcc->ops.write(priv->ipcc, buffer + nwritten,
+ buflen - nwritten);
+ if (nwritten == (ssize_t)buflen || nwritten < 0)
+ {
+ /* We've managed to write whole buffer to IPCC memory,
+ * there is nothing else for use to do
+ * --or--
+ * lower half driver returned error during write,
+ *
+ * either way we return with nwritten which will either
+ * be number of bytes written or negated errno.
+ */
+
+ nxsem_post(&priv->exclsem);
+ return nwritten;
+ }
+ }
+
+ /* Either, there is already some data on the txbuffer, which
+ * means IPCC is occupied, or txbuffer is empty, but we could
+ * not write whole buffer to IPCC memory. In either case we
+ * copy what is left in data to buffer.
+ */
+
+ nwritten += circbuf_write(&priv->ipcc->txbuf, buffer + nwritten,
+ (ssize_t)buflen - nwritten);
+
+ /* Notify lower half that new data on circ buffer is available */
+
+ priv->ipcc->ops.write_notify(priv->ipcc);
+
+ if (nwritten == (ssize_t)buflen)
+ {
+ /* All outstanding data has been copied to txbuffer, we're done */
+
+ nxsem_post(&priv->exclsem);
+ return nwritten;
+ }
+
+#else /* CONFIG_IPCC_BUFFERED */
+ /* Unbuffered write, write data directly to lower driver */
+
+ nwritten += priv->ipcc->ops.write(&priv->ipcc, buffer, buflen);
Review Comment:
how to handle the fail at the 2nd or 3rd loop?
##########
include/nuttx/ipcc.h:
##########
@@ -0,0 +1,212 @@
+/****************************************************************************
+ * include/nuttx/ipcc.h
+ *
+ * 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.
+ *
+ ****************************************************************************/
+
+#ifndef __INCLUDE_NUTTX_IPCC_H
+#define __INCLUDE_NUTTX_IPCC_H
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+#include <nuttx/mm/circbuf.h>
+#include <nuttx/semaphore.h>
+
+#include <poll.h>
+
+/****************************************************************************
+ * Public Types
+ ****************************************************************************/
+
+/* IPCC is a two-part driver:
+ *
+ * 1) A common upper half driver, that provides the character device
+ * interface for the user
+ * 2) Platform-specific lower half drivers that provide interface
+ * between upper and lower layer. Lower half also contains
+ * common objects like semaphore to inform
+ */
+
+/* This structure defines all of the operations provided by the
+ * architecture specific logic. All fields must be provided with
+ * non-NULL function pointers.
+ */
+
+struct ipcc_lower_s;
+struct ipcc_ops_s
+{
+ /**************************************************************************
+ * Name: read
+ *
+ * Description:
+ * Reads data directly from IPCC mbox, function shall not block.
+ * If no data is available, 0 shall be returned. Partial read
+ * is possible. In that case function can return value less
+ * than buflen.
+ *
+ * Input Parameters:
+ * ipcc - ipcc channel to read data from
+ * buffer - location where data shall be stored
+ * buflen - size of the buffer
+ *
+ * Returned Value:
+ * Number of bytes actually written to buffer.
+ *
+ **************************************************************************/
+
+ CODE ssize_t (*read)(FAR struct ipcc_lower_s *ipcc,
+ FAR char *buffer, size_t buflen);
+
+ /**************************************************************************
+ * Name: write
+ *
+ * Description:
+ * Writes data directly to IPCC mbox, function shall not block.
+ * If IPCC memory is full, or busy or otherwise unavailable for
+ * write, 0 shall be returned. Partial write is possible. In
+ * that case function can return value less then buflen.
+ *
+ * Input Parameters:
+ * ipcc - ipcc channel to write data to
+ * buffer - location to read data from
+ * buflen - number of bytes to write from buffer
+ *
+ * Returned Value:
+ * Number of actually written data to IPCC channel.
+ *
+ **************************************************************************/
+
+ CODE ssize_t (*write)(FAR struct ipcc_lower_s *ipcc,
+ FAR const char *buffer, size_t buflen);
+
+ /**************************************************************************
+ * Name: buffer_data
+ *
+ * Description:
+ * Lower driver shall copy data from IPCC to cirbuf on demand.
+ * It does not have to copy all data (in case rxbuf is full), but
+ * it must keep internal pointer to track uncopied data, and copy
+ * what is left on the next call to this function.
+ *
+ * Input Parameters:
+ * ipcc - channel to buffer data
+ * rxbuf - circural buffer where data should be written to
+ *
+ * Returned Value:
+ * Number of bytes that were successfully written to rxbuf.
+ *
+ **************************************************************************/
+
+ CODE ssize_t (*buffer_data)(FAR struct ipcc_lower_s *ipcc,
+ FAR struct circbuf_s *rxbuf);
+
+ /**************************************************************************
+ * Name: write_notify
+ *
+ * Description:
+ * Upper half driver notifies lower half driver that new data has been
+ * copied to circ buffer. This can be used by lower half to transfer
+ * data from buffer to IPCC memory.
+ *
+ * Input Parameters:
+ * ipcc - ipcc channel that wrote data.
+ *
+ * Returned Value:
+ * 0 on success, or negated errno otherwise
+ *
+ **************************************************************************/
+
+ CODE ssize_t (*write_notify)(FAR struct ipcc_lower_s *ipcc);
+
+ /**************************************************************************
+ * Name: cleanup
+ *
+ * Description:
+ * Cleans up resources initialized by <arch>_ipcc_init(). If arch code
+ * malloc()ed any memory, this funciton is responsible of freeing it.
+ * After this function is called, ipcc driver will not access lower
+ * half pointer anymore.
+ *
+ * Input Parameters:
+ * ipcc - ipcc channel to cleanup
+ *
+ * Returned Value:
+ * Always OK
+ *
+ **************************************************************************/
+
+ CODE int (*cleanup)(FAR struct ipcc_lower_s *ipcc);
+};
+
+/* This structure defines the interface between upper and lower halves.
+ * Such instance is passed to ipcc_register() function after driver
+ * has been initialized, binding the upper and lower halves into one
+ * driver.
+ */
+
+struct ipcc_lower_s
+{
+ FAR struct pollfd *fds[CONFIG_IPCC_NPOLLWAITERS];
+ FAR struct ipcc_ops_s ops; /* Arch specific functions */
+ sem_t rxsem; /* Data ready to read semaphore */
+ sem_t txsem; /* Data ready to send semaphore */
+ int chan; /* IPCC channel */
+
+#ifdef CONFIG_IPCC_BUFFERED
+ struct circbuf_s rxbuf; /* Receive buffer */
+ struct circbuf_s txbuf; /* Transmit buffer */
+
+ /* 1- not all data could fit into buffer, some unread data is still
+ * in IPCC mbox. State of this variable is maintained by lower half,
+ * upper half only reads it to know when to inform lower driver that
+ * buffer got free.
+ */
+
+ int overflow;
+#endif /* CONFIG_IPCC_BUFFERED */
+};
+
+/* Driver state structure */
+
+struct ipcc_driver_s
+{
+ FAR struct ipcc_lower_s *ipcc; /* Lower half driver state */
+ int crefs; /* Count number of open references */
+ int unlinked; /* 1 - driver has been unlinked */
+ sem_t exclsem; /* Mutual exclusion for driver */
+};
+
+/****************************************************************************
+ * Public Functions Prototypes
+ ****************************************************************************/
+
+void ipcc_pollnotify(FAR struct ipcc_lower_s *ipcc, pollevent_t eventset);
Review Comment:
it's better to move these function to:
drivers/ipcc/ipcc.h
--
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] mlyszczek commented on a diff in pull request #6537: drivers/ipcc: add Inter Processor Communication driver
Posted by GitBox <gi...@apache.org>.
mlyszczek commented on code in PR #6537:
URL: https://github.com/apache/incubator-nuttx/pull/6537#discussion_r929080036
##########
include/nuttx/ipcc.h:
##########
@@ -0,0 +1,212 @@
+/****************************************************************************
+ * include/nuttx/ipcc.h
+ *
+ * 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.
+ *
+ ****************************************************************************/
+
+#ifndef __INCLUDE_NUTTX_IPCC_H
+#define __INCLUDE_NUTTX_IPCC_H
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+#include <nuttx/mm/circbuf.h>
+#include <nuttx/semaphore.h>
+
+#include <poll.h>
+
+/****************************************************************************
+ * Public Types
+ ****************************************************************************/
+
+/* IPCC is a two-part driver:
+ *
+ * 1) A common upper half driver, that provides the character device
+ * interface for the user
+ * 2) Platform-specific lower half drivers that provide interface
+ * between upper and lower layer. Lower half also contains
+ * common objects like semaphore to inform
+ */
+
+/* This structure defines all of the operations provided by the
+ * architecture specific logic. All fields must be provided with
+ * non-NULL function pointers.
+ */
+
+struct ipcc_lower_s;
+struct ipcc_ops_s
+{
+ /**************************************************************************
+ * Name: read
+ *
+ * Description:
+ * Reads data directly from IPCC mbox, function shall not block.
+ * If no data is available, 0 shall be returned. Partial read
+ * is possible. In that case function can return value less
+ * than buflen.
+ *
+ * Input Parameters:
+ * ipcc - ipcc channel to read data from
+ * buffer - location where data shall be stored
+ * buflen - size of the buffer
+ *
+ * Returned Value:
+ * Number of bytes actually written to buffer.
+ *
+ **************************************************************************/
+
+ CODE ssize_t (*read)(FAR struct ipcc_lower_s *ipcc,
+ FAR char *buffer, size_t buflen);
+
+ /**************************************************************************
+ * Name: write
+ *
+ * Description:
+ * Writes data directly to IPCC mbox, function shall not block.
+ * If IPCC memory is full, or busy or otherwise unavailable for
+ * write, 0 shall be returned. Partial write is possible. In
+ * that case function can return value less then buflen.
+ *
+ * Input Parameters:
+ * ipcc - ipcc channel to write data to
+ * buffer - location to read data from
+ * buflen - number of bytes to write from buffer
+ *
+ * Returned Value:
+ * Number of actually written data to IPCC channel.
+ *
+ **************************************************************************/
+
+ CODE ssize_t (*write)(FAR struct ipcc_lower_s *ipcc,
+ FAR const char *buffer, size_t buflen);
+
+ /**************************************************************************
+ * Name: buffer_data
+ *
+ * Description:
+ * Lower driver shall copy data from IPCC to cirbuf on demand.
+ * It does not have to copy all data (in case rxbuf is full), but
+ * it must keep internal pointer to track uncopied data, and copy
+ * what is left on the next call to this function.
+ *
+ * Input Parameters:
+ * ipcc - channel to buffer data
+ * rxbuf - circural buffer where data should be written to
+ *
+ * Returned Value:
+ * Number of bytes that were successfully written to rxbuf.
+ *
+ **************************************************************************/
+
+ CODE ssize_t (*buffer_data)(FAR struct ipcc_lower_s *ipcc,
+ FAR struct circbuf_s *rxbuf);
+
+ /**************************************************************************
+ * Name: write_notify
+ *
+ * Description:
+ * Upper half driver notifies lower half driver that new data has been
+ * copied to circ buffer. This can be used by lower half to transfer
+ * data from buffer to IPCC memory.
+ *
+ * Input Parameters:
+ * ipcc - ipcc channel that wrote data.
+ *
+ * Returned Value:
+ * 0 on success, or negated errno otherwise
+ *
+ **************************************************************************/
+
+ CODE ssize_t (*write_notify)(FAR struct ipcc_lower_s *ipcc);
+
+ /**************************************************************************
+ * Name: cleanup
+ *
+ * Description:
+ * Cleans up resources initialized by <arch>_ipcc_init(). If arch code
+ * malloc()ed any memory, this funciton is responsible of freeing it.
+ * After this function is called, ipcc driver will not access lower
+ * half pointer anymore.
+ *
+ * Input Parameters:
+ * ipcc - ipcc channel to cleanup
+ *
+ * Returned Value:
+ * Always OK
+ *
+ **************************************************************************/
+
+ CODE int (*cleanup)(FAR struct ipcc_lower_s *ipcc);
+};
+
+/* This structure defines the interface between upper and lower halves.
+ * Such instance is passed to ipcc_register() function after driver
+ * has been initialized, binding the upper and lower halves into one
+ * driver.
+ */
+
+struct ipcc_lower_s
+{
+ FAR struct pollfd *fds[CONFIG_IPCC_NPOLLWAITERS];
Review Comment:
Ok, this should be fixed now. Data that is directly used in lower half has been left in ipcc_lower_s, and data that is used (code reference) in upper half has been moved to ipcc_driver_s. ipcc_lower_s has now pointer to ipcc_driver_s structure which should only be used to call upper half functions from lower half driver.
--
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] mlyszczek commented on pull request #6537: drivers/ipcc: add Inter Processor Communication driver
Posted by GitBox <gi...@apache.org>.
mlyszczek commented on PR #6537:
URL: https://github.com/apache/incubator-nuttx/pull/6537#issuecomment-1188088408
@onegray could you please elaborate more about these performance issues?
Is this because of multiple copying? cpu2 wifi buffer -> ipcc memory -> cpu1 circ buffer -> cpu1 user space app? circ buf part can be optimized out by disabling CONFIG_IPCC_BUFFERED.
Too many interrupts? This also could be optimized by increasing IPCC memory and trigger interrupt once specific threshold of memory is occupied. This could be done with some open flags, and ioctl(TRIGGER_TX_TRANSACTION) or similar.
We could also do some black magic by exposing pointer to raw IPCC memory via ioctl() so userapp could work directly on IPCC memory without copying anything. But this is no unix way.
I wrote this driver with ease of use in mind, and character device is the easiest I could think of:) I really don't know what is the performance of the driver. I am yet to see it when testing with passing lora traffic through it. I'm fine with renaming this to ipcc-serial though.
--
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] onegray commented on a diff in pull request #6537: drivers/ipcc: add Inter Processor Communication driver
Posted by GitBox <gi...@apache.org>.
onegray commented on code in PR #6537:
URL: https://github.com/apache/incubator-nuttx/pull/6537#discussion_r923544998
##########
arch/arm/src/stm32wl5/stm32wl5_ipcc.c:
##########
@@ -0,0 +1,801 @@
+/****************************************************************************
+ * arch/arm/src/stm32wl5/stm32wl5_ipcc.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/ipcc.h>
+#include <nuttx/irq.h>
+
+#include <arm_internal.h>
+#include <assert.h>
+#include <debug.h>
+
+#include <arch/stm32wl5/stm32wl5xxx_cpu1_irq.h>
+#include "hardware/stm32wl5_ipcc.h"
+#include "stm32wl5.h"
+#include "stm32wl5_ipcc.h"
+
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+/* This structure describes tx or rx of single channel in memory */
+
+struct stm32wl5_ipcc_chan_mem_s
+{
+ unsigned len; /* Number of valid bytes in data[] */
+ char data[]; /* Data in IPCC memory */
+};
+
+/* Internal stm32wl5 ipcc structure describing channel state. */
+
+struct stm32wl5_ipcc_s
+{
+ /* Pointer to API connecting upper and lower half of the driver */
+
+ struct ipcc_lower_s *ipcc;
+
+ /* Physical memory address where second CPU will write data for us,
+ * we will be reading from this memory.
+ */
+
+ char *rxmem;
+
+ /* Maximum length of data that rxmem can hold. It is size of the
+ * reserved space for rxmem minus sizeof(stm32wl5_ipcc_chan_mem_s.len)
+ */
+
+ unsigned rxlen;
+
+#if CONFIG_IPCC_BUFFERED
+ /* Number of bytes copied from IPCC memory to buffer. Can be less than
+ * stm32wl5_ipcc_chan_mem_s.len after copy operation when buffer is full.
+ * Value can persist between multiple ISR and stm32wl5_ipcc_buffer_data()
+ * calls, until all data from IPCC memory is successfully buffered.
+ */
+
+ unsigned rxcopied;
+#endif
+
+ /* Physical memory address where we will write data for the second
+ * CPU to read.
+ */
+
+ char *txmem;
+
+ /* Maximum length of data that txmem can hold. It is size of the
+ * reserved space for txmem minus sizeof(stm32wl5_ipcc_chan_mem_s.len)
+ */
+
+ unsigned txlen;
+};
+
+/****************************************************************************
+ * Private Function Prototypes
+ ****************************************************************************/
+
+static ssize_t stm32wl5_ipcc_read(struct ipcc_lower_s *ipcc,
+ char *buffer, size_t buflen);
+static ssize_t stm32wl5_ipcc_write(struct ipcc_lower_s *ipcc,
+ const char *buffer, size_t buflen);
+static ssize_t stm32wl5_ipcc_buffer_data(struct ipcc_lower_s *ipcc,
+ struct circbuf_s *rxbuf);
+static ssize_t stm32wl5_ipcc_copy_to_buffer(int chan,
+ struct circbuf_s *rxbuf);
+static int stm32wl5_ipcc_rx_isr(int irq, void *context, void *arg);
+static int stm32wl5_ipcc_tx_isr(int irq, void *context, void *arg);
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+struct stm32wl5_ipcc_s g_ipccpriv[IPCC_NCHAN] =
+{
+ /* Channel 1 is always enabled when IPCC is enabled */
+
+ {
+ .rxmem = (char *)(IPCC_CHAN1_START),
+ .rxlen = IPCC_CHAN1_RX_SIZE - sizeof(unsigned),
+ .txmem = (char *)(IPCC_CHAN1_START + IPCC_CHAN1_RX_SIZE),
+ .txlen = IPCC_CHAN1_TX_SIZE - sizeof(unsigned)
+ }
+
+#if IPCC_CHAN2
+ ,
+ {
+ .rxmem = (char *)(IPCC_CHAN2_START),
+ .rxlen = IPCC_CHAN2_RX_SIZE - sizeof(unsigned),
+ .txmem = (char *)(IPCC_CHAN2_START + IPCC_CHAN2_RX_SIZE),
+ .txlen = IPCC_CHAN2_TX_SIZE - sizeof(unsigned)
+ }
+#endif
+
+#if IPCC_CHAN3
+ ,
+ {
+ .rxmem = (char *)(IPCC_CHAN3_START),
+ .rxlen = IPCC_CHAN3_RX_SIZE - sizeof(unsigned),
+ .txmem = (char *)(IPCC_CHAN3_START + IPCC_CHAN3_RX_SIZE),
+ .txlen = IPCC_CHAN3_TX_SIZE - sizeof(unsigned)
+ }
+#endif
+
+#if IPCC_CHAN4
+ ,
+ {
+ .rxmem = (char *)(IPCC_CHAN4_START),
+ .rxlen = IPCC_CHAN4_RX_SIZE - sizeof(unsigned),
+ .txmem = (char *)(IPCC_CHAN4_START + IPCC_CHAN4_RX_SIZE),
+ .txlen = IPCC_CHAN4_TX_SIZE - sizeof(unsigned)
+ }
+#endif
+
+#if IPCC_CHAN5
+ ,
+ {
+ .rxmem = (char *)(IPCC_CHAN5_START),
+ .rxlen = IPCC_CHAN5_RX_SIZE - sizeof(unsigned),
+ .txmem = (char *)(IPCC_CHAN5_START + IPCC_CHAN5_RX_SIZE),
+ .txlen = IPCC_CHAN5_TX_SIZE - sizeof(unsigned)
+ }
+#endif
+
+#if IPCC_CHAN6
+ ,
+ {
+ .rxmem = (char *)(IPCC_CHAN6_START),
+ .rxlen = IPCC_CHAN6_RX_SIZE - sizeof(unsigned),
+ .txmem = (char *)(IPCC_CHAN6_START + IPCC_CHAN6_RX_SIZE),
+ .txlen = IPCC_CHAN6_TX_SIZE - sizeof(unsigned)
+ }
+#endif
+};
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: stm32wl5_ipcc_tx_isr
+ *
+ * Description:
+ * IPCC TX interrupt service routine. This interrupt is called when
+ * second CPU read all data from IPCC TX memory and we are free to
+ * write new data to that memory.
+ *
+ * For buffered IPCC, we will immediately write data from buffer to
+ * IPCC memory and notify second CPU. Also blocked writer will be
+ * notified that there is free space on tx buffer.
+ *
+ * For unbuffered IPCC, we won't copy anything but only notify blocked
+ * writers that IPCC TX memory is free.
+ *
+ * Input Parameters:
+ * irq - Number of IRQ that generated interrupt
+ * context - Interrupt register state save info (architecture-specific)
+ * arg - user data (not used by us)
+ *
+ * Returned Value:
+ * Always OK
+ *
+ * Assumptions/Limitations:
+ *
+ ****************************************************************************/
+
+static int stm32wl5_ipcc_tx_isr(int irq, void *context, void *arg)
+{
+ int chan;
+ size_t nwritten;
+ uint32_t mr;
+ uint32_t sr;
+ uint32_t status;
+ struct stm32wl5_ipcc_s *priv;
+ struct stm32wl5_ipcc_chan_mem_s *txmem;
+
+ (void)context;
+ (void)arg;
+ (void)irq;
+
+ /* Disable TX interrupt, we don't want two "threads" to concurently
+ * modify circbuffers as we don't use any guard to them
+ */
+
+ up_disable_irq(STM32WL5_IRQ_IPCC_C1_TX_IT);
+
+ mr = getreg32(STM32WL5_IPCC_C1MR) >> STM32WL5_IPCC_TX_SHIFT;
+ sr = getreg32(STM32WL5_IPCC_C1TOC2SR);
+
+ /* Consider only channels that have tx memory free and are unmasked */
+
+ status = sr | mr;
+
+ /* Check which channels have data ready to read */
+
+ for (chan = 0; chan != IPCC_NCHAN; chan++)
+ {
+ if (status & (1 << chan))
+ {
+ /* Transmit on channel is either masked or TX memory is not
+ * ready to write (second CPU is still read data from it,
+ * CHnF == 1).
+ */
+
+ continue;
+ }
+
+ /* Get internal data structure for current chan */
+
+ priv = &g_ipccpriv[chan];
+ txmem = (struct stm32wl5_ipcc_chan_mem_s *)priv->txmem;
+
+#if CONFIG_IPCC_BUFFERED
+ /* Copy as much as we can into IPCC memory, circbuf won't copy
+ * more than there is in the buffer.
+ */
+
+ nwritten = circbuf_read(&priv->ipcc->txbuf, txmem->data, priv->txlen);
Review Comment:
Isn't it too much stuff to do in interrupt handler? Optionally we can move something to `work_queue`. But I think it's ok for now.
--
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] mlyszczek commented on pull request #6537: [API review request] drivers/ipcc: add Inter Processor Communication driver
Posted by GitBox <gi...@apache.org>.
mlyszczek commented on PR #6537:
URL: https://github.com/apache/incubator-nuttx/pull/6537#issuecomment-1171050306
Yeah, so I've compiled stub code and my flash increased by 18kB. It will probably take similar ammount of memory to compile and I don't suppose there is some trickery to share flash code between CPUs ;) And I don't know what else will have to be compiled for another CPU to support OpenAMP there, so it might be another extra tens of kB of flash.
And my project requires substential ammount of flash as it is so adding around 40kB just for IPCC is hard to swallow.
So I've decieded that I will continue with that simple character device stream. The question is, are you going to accept this into upstream or should I not even bother creating pull request for this and just keep it internally?
--
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] mlyszczek commented on a diff in pull request #6537: drivers/ipcc: add Inter Processor Communication driver
Posted by GitBox <gi...@apache.org>.
mlyszczek commented on code in PR #6537:
URL: https://github.com/apache/incubator-nuttx/pull/6537#discussion_r923677177
##########
drivers/ipcc/ipcc_unlink.c:
##########
@@ -0,0 +1,94 @@
+/****************************************************************************
+ * drivers/ipcc/ipcc_unlink.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/ipcc.h>
+#include <nuttx/kmalloc.h>
+#include <nuttx/semaphore.h>
+
+#include <assert.h>
+#include <errno.h>
+#include <stdbool.h>
+#include <stdio.h>
+#include <sys/types.h>
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: ipcc_unlink
+ *
+ * Description:
+ * Action to take upon file unlinking. Function will free resources if
+ * noone is using the driver when unlinking occured. If driver is still
+ * in use, it will be marked as unlinked and resource freeing will take
+ * place in ipcc_close() function instead, once last reference is closed.
+ *
+ * Input Parameters:
+ * inode - driver inode that is being unlinked
+ *
+ * Returned Value:
+ * OK on successfull close, or negated errno on failure.
+ *
+ * Assumptions/Limitations:
+ *
+ ****************************************************************************/
+
+int ipcc_unlink(FAR struct inode *inode)
Review Comment:
Well, since unix idea of drivers is that "everything is a file" unlink() seems like a good function to remove driver. And doesn't unregister_driver() simply remove inode from the file system? Without freeing resources?
--
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] mlyszczek commented on a diff in pull request #6537: drivers/ipcc: add Inter Processor Communication driver
Posted by GitBox <gi...@apache.org>.
mlyszczek commented on code in PR #6537:
URL: https://github.com/apache/incubator-nuttx/pull/6537#discussion_r923188088
##########
include/nuttx/ipcc.h:
##########
@@ -0,0 +1,188 @@
+/****************************************************************************
+ * include/nuttx/ipcc.h
+ *
+ * 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/mm/circbuf.h>
+#include <nuttx/semaphore.h>
+
+#include <poll.h>
+
+/****************************************************************************
+ * Public Types
+ ****************************************************************************/
+
+/* IPCC is a two-part driver:
+ *
+ * 1) A common upper half driver, that provides the character device
+ * interface for the user
+ * 2) Platform-specific lower half drivers that provide interface
+ * between upper and lower layer. Lower half also contains
+ * common objects like semaphore to inform
+ */
+
+/* This structure defines all of the operations provided by the
+ * architecture specific logic. All fields must be provided with
+ * non-NULL function pointers.
+ */
+
+struct ipcc_lower_s;
+struct ipcc_ops_s
+{
+ /* Reads data directly from IPCC mbox, should not block, ever.
+ * If no data is available, 0 should be returned. Partial read
+ * is possible. In that case function will return value less
+ * than buflen.
+ */
+
+ CODE ssize_t (*read)(FAR struct ipcc_lower_s *ipcc,
+ FAR char *buffer, size_t buflen);
+
+ /* Writes data directly to IPCC mbox, should not block, ever.
+ * If IPCC memory is full, or busy or otherwise unavailable for
+ * write, 0 should be returned. Partial write is possible. In
+ * that case function will return value less then buflen.
+ */
+
+ CODE ssize_t (*write)(FAR struct ipcc_lower_s *ipcc,
+ FAR const char *buffer, size_t buflen);
+
+ /* Lower driver should copy data from IPCC to cirbuf on demand.
+ * It does not have to copy all data (in case rxbuf is full), but
+ * it should keep internal pointer to track uncopied data, and copy
+ * what is left on the next call to this function.
+ */
+
+ CODE ssize_t (*buffer_data)(FAR struct ipcc_lower_s *ipcc,
+ FAR struct circbuf_s *rxbuf);
+
+ /* Upper half notifies lower half driver that new data has been
+ * copied to circ buffer
+ */
+
+ CODE ssize_t (*write_notify)(FAR struct ipcc_lower_s *ipcc);
+};
+
+/* This structure defines the interface between upper and lower halves.
+ * Such instance is passed to ipcc_register() function after driver
+ * has been initialized, binding the upper and lower halves into one
+ * driver.
+ */
+
+struct ipcc_lower_s
+{
+ FAR struct pollfd *fds[CONFIG_IPCC_NPOLLWAITERS];
+ FAR struct ipcc_ops_s ops; /* Arch specific functions */
+ sem_t rxsem; /* Data ready to read semaphore */
+ sem_t txsem; /* Data ready to send semaphore */
+ int chan; /* IPCC channel */
+
+#ifdef CONFIG_IPCC_BUFFERED
+ struct circbuf_s rxbuf; /* Receive buffer */
+ struct circbuf_s txbuf; /* Transmit buffer */
+
+ /* 1- not all data could fit into buffer, some unread data is still
+ * in IPCC mbox. State of this variable is maintained by lower half,
+ * upper half only reads it to know when to inform lower driver that
+ * buffer got free.
+ */
+
+ int overflow;
+#endif /* CONFIG_IPCC_BUFFERED */
+};
+
+/* Driver state structure */
+
+struct ipcc_driver_s
+{
+ FAR struct ipcc_lower_s ipcc; /* Lower half driver state */
+ int crefs; /* Count number of open references */
+ int unlinked; /* 1 - driver has been unlinked */
+ sem_t exclsem; /* Mutual exclusion for driver */
+};
+
+/****************************************************************************
+ * Public Functions Prototypes
+ ****************************************************************************/
+
+void ipcc_pollnotify(FAR struct ipcc_lower_s *ipcc, pollevent_t eventset);
+void ipcc_cleanup(FAR struct ipcc_driver_s *priv);
+void ipcc_txfree_notify(FAR struct ipcc_lower_s *priv);
+void ipcc_rxfree_notify(FAR struct ipcc_lower_s *ipcc);
+
+#ifdef CONFIG_IPCC_BUFFERED
+int ipcc_register(int chan, size_t rxbuflen, size_t txbuflen);
+#else
+int ipcc_register(int chan);
+#endif
+
+/****************************************************************************
+ * Name: up_ipcc_init
+ *
+ * Description:
+ * Initializes lower half of the driver. This function should be
+ * implemented by lower half driver in microcontroller code. Function
+ * should not initialize ipcc_lower_s structure with the exception of
+ * explicitly specified fields like setting pointer to read and write
+ * function pointers. Rest of the fields are initialized in upper half
+ * driver.
+ *
+ * This function is called once in ipcc_register() function.
+ *
+ * Input Parameters:
+ * ipcc - interface between upper and lower half drivers. Only some
+ * of the fields should be initialized in this function.
+ * chan - channel number to initialize.
+ *
+ * Returned Value:
+ * 0 on success or negated errno otherwise.
+ *
+ * Assumptions/Limitations:
+ *
+ ****************************************************************************/
+
+int up_ipcc_init(FAR struct ipcc_lower_s *ipcc, int chan);
Review Comment:
Ok, more work for the user but it makes sense. Will change that.
--
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 pull request #6537: [API review request] drivers/ipcc: add Inter Processor Communication driver
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on PR #6537:
URL: https://github.com/apache/incubator-nuttx/pull/6537#issuecomment-1171514627
Linux contain several subsystem for IPC:
hardware spinlock:
https://docs.kernel.org/locking/hwspinlock.html
mailbox:
https://www.kernel.org/doc/html/latest/driver-api/mailbox.html
virtio:
https://docs.oasis-open.org/virtio/virtio/v1.1/csprd01/virtio-v1.1-csprd01.html
rpmsg:
https://docs.kernel.org/staging/rpmsg.html
remoteproc:
https://docs.kernel.org/staging/remoteproc.html
Item 1 and 2 directly map to the hardware block, and easy to define the driver interface
Item 3 and 4 define the share memory layout which is purely defined by software not couple with the hardware
Item 5 is used to control the life cycle(download firmware, start, stop) of the remote cpu
You may get some ideal from them.
--
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 pull request #6537: [API review request] drivers/ipcc: add Inter Processor Communication driver
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on PR #6537:
URL: https://github.com/apache/incubator-nuttx/pull/6537#issuecomment-1168886439
What's the hardware infrastructure plan to work with the IPCC driver model?
--
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] onegray commented on a diff in pull request #6537: drivers/ipcc: add Inter Processor Communication driver
Posted by GitBox <gi...@apache.org>.
onegray commented on code in PR #6537:
URL: https://github.com/apache/incubator-nuttx/pull/6537#discussion_r923527130
##########
arch/arm/src/stm32wl5/stm32wl5_ipcc.c:
##########
@@ -0,0 +1,801 @@
+/****************************************************************************
+ * arch/arm/src/stm32wl5/stm32wl5_ipcc.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/ipcc.h>
+#include <nuttx/irq.h>
+
+#include <arm_internal.h>
+#include <assert.h>
+#include <debug.h>
+
+#include <arch/stm32wl5/stm32wl5xxx_cpu1_irq.h>
+#include "hardware/stm32wl5_ipcc.h"
+#include "stm32wl5.h"
+#include "stm32wl5_ipcc.h"
+
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+/* This structure describes tx or rx of single channel in memory */
+
+struct stm32wl5_ipcc_chan_mem_s
+{
+ unsigned len; /* Number of valid bytes in data[] */
+ char data[]; /* Data in IPCC memory */
+};
+
+/* Internal stm32wl5 ipcc structure describing channel state. */
+
+struct stm32wl5_ipcc_s
+{
+ /* Pointer to API connecting upper and lower half of the driver */
+
+ struct ipcc_lower_s *ipcc;
+
+ /* Physical memory address where second CPU will write data for us,
+ * we will be reading from this memory.
+ */
+
+ char *rxmem;
+
+ /* Maximum length of data that rxmem can hold. It is size of the
+ * reserved space for rxmem minus sizeof(stm32wl5_ipcc_chan_mem_s.len)
+ */
+
+ unsigned rxlen;
+
+#if CONFIG_IPCC_BUFFERED
+ /* Number of bytes copied from IPCC memory to buffer. Can be less than
+ * stm32wl5_ipcc_chan_mem_s.len after copy operation when buffer is full.
+ * Value can persist between multiple ISR and stm32wl5_ipcc_buffer_data()
+ * calls, until all data from IPCC memory is successfully buffered.
+ */
+
+ unsigned rxcopied;
+#endif
+
+ /* Physical memory address where we will write data for the second
+ * CPU to read.
+ */
+
+ char *txmem;
+
+ /* Maximum length of data that txmem can hold. It is size of the
+ * reserved space for txmem minus sizeof(stm32wl5_ipcc_chan_mem_s.len)
+ */
+
+ unsigned txlen;
+};
+
+/****************************************************************************
+ * Private Function Prototypes
+ ****************************************************************************/
+
+static ssize_t stm32wl5_ipcc_read(struct ipcc_lower_s *ipcc,
+ char *buffer, size_t buflen);
+static ssize_t stm32wl5_ipcc_write(struct ipcc_lower_s *ipcc,
+ const char *buffer, size_t buflen);
+static ssize_t stm32wl5_ipcc_buffer_data(struct ipcc_lower_s *ipcc,
+ struct circbuf_s *rxbuf);
+static ssize_t stm32wl5_ipcc_copy_to_buffer(int chan,
+ struct circbuf_s *rxbuf);
+static int stm32wl5_ipcc_rx_isr(int irq, void *context, void *arg);
+static int stm32wl5_ipcc_tx_isr(int irq, void *context, void *arg);
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+struct stm32wl5_ipcc_s g_ipccpriv[IPCC_NCHAN] =
+{
+ /* Channel 1 is always enabled when IPCC is enabled */
+
+ {
+ .rxmem = (char *)(IPCC_CHAN1_START),
+ .rxlen = IPCC_CHAN1_RX_SIZE - sizeof(unsigned),
+ .txmem = (char *)(IPCC_CHAN1_START + IPCC_CHAN1_RX_SIZE),
+ .txlen = IPCC_CHAN1_TX_SIZE - sizeof(unsigned)
+ }
+
+#if IPCC_CHAN2
+ ,
+ {
+ .rxmem = (char *)(IPCC_CHAN2_START),
+ .rxlen = IPCC_CHAN2_RX_SIZE - sizeof(unsigned),
+ .txmem = (char *)(IPCC_CHAN2_START + IPCC_CHAN2_RX_SIZE),
+ .txlen = IPCC_CHAN2_TX_SIZE - sizeof(unsigned)
+ }
+#endif
+
+#if IPCC_CHAN3
+ ,
+ {
+ .rxmem = (char *)(IPCC_CHAN3_START),
+ .rxlen = IPCC_CHAN3_RX_SIZE - sizeof(unsigned),
+ .txmem = (char *)(IPCC_CHAN3_START + IPCC_CHAN3_RX_SIZE),
+ .txlen = IPCC_CHAN3_TX_SIZE - sizeof(unsigned)
+ }
+#endif
+
+#if IPCC_CHAN4
+ ,
+ {
+ .rxmem = (char *)(IPCC_CHAN4_START),
+ .rxlen = IPCC_CHAN4_RX_SIZE - sizeof(unsigned),
+ .txmem = (char *)(IPCC_CHAN4_START + IPCC_CHAN4_RX_SIZE),
+ .txlen = IPCC_CHAN4_TX_SIZE - sizeof(unsigned)
+ }
+#endif
+
+#if IPCC_CHAN5
+ ,
+ {
+ .rxmem = (char *)(IPCC_CHAN5_START),
+ .rxlen = IPCC_CHAN5_RX_SIZE - sizeof(unsigned),
+ .txmem = (char *)(IPCC_CHAN5_START + IPCC_CHAN5_RX_SIZE),
+ .txlen = IPCC_CHAN5_TX_SIZE - sizeof(unsigned)
+ }
+#endif
+
+#if IPCC_CHAN6
+ ,
+ {
+ .rxmem = (char *)(IPCC_CHAN6_START),
+ .rxlen = IPCC_CHAN6_RX_SIZE - sizeof(unsigned),
+ .txmem = (char *)(IPCC_CHAN6_START + IPCC_CHAN6_RX_SIZE),
+ .txlen = IPCC_CHAN6_TX_SIZE - sizeof(unsigned)
+ }
+#endif
+};
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: stm32wl5_ipcc_tx_isr
+ *
+ * Description:
+ * IPCC TX interrupt service routine. This interrupt is called when
+ * second CPU read all data from IPCC TX memory and we are free to
+ * write new data to that memory.
+ *
+ * For buffered IPCC, we will immediately write data from buffer to
+ * IPCC memory and notify second CPU. Also blocked writer will be
+ * notified that there is free space on tx buffer.
+ *
+ * For unbuffered IPCC, we won't copy anything but only notify blocked
+ * writers that IPCC TX memory is free.
+ *
+ * Input Parameters:
+ * irq - Number of IRQ that generated interrupt
+ * context - Interrupt register state save info (architecture-specific)
+ * arg - user data (not used by us)
+ *
+ * Returned Value:
+ * Always OK
+ *
+ * Assumptions/Limitations:
+ *
+ ****************************************************************************/
+
+static int stm32wl5_ipcc_tx_isr(int irq, void *context, void *arg)
+{
+ int chan;
+ size_t nwritten;
+ uint32_t mr;
+ uint32_t sr;
+ uint32_t status;
+ struct stm32wl5_ipcc_s *priv;
+ struct stm32wl5_ipcc_chan_mem_s *txmem;
+
+ (void)context;
+ (void)arg;
+ (void)irq;
+
+ /* Disable TX interrupt, we don't want two "threads" to concurently
+ * modify circbuffers as we don't use any guard to them
+ */
+
+ up_disable_irq(STM32WL5_IRQ_IPCC_C1_TX_IT);
+
+ mr = getreg32(STM32WL5_IPCC_C1MR) >> STM32WL5_IPCC_TX_SHIFT;
+ sr = getreg32(STM32WL5_IPCC_C1TOC2SR);
+
+ /* Consider only channels that have tx memory free and are unmasked */
+
+ status = sr | mr;
Review Comment:
Looks like you are expecting `AND` logic, but the written is `OR`.
--
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] mlyszczek commented on a diff in pull request #6537: drivers/ipcc: add Inter Processor Communication driver
Posted by GitBox <gi...@apache.org>.
mlyszczek commented on code in PR #6537:
URL: https://github.com/apache/incubator-nuttx/pull/6537#discussion_r928625327
##########
include/nuttx/ipcc.h:
##########
@@ -0,0 +1,212 @@
+/****************************************************************************
+ * include/nuttx/ipcc.h
+ *
+ * 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.
+ *
+ ****************************************************************************/
+
+#ifndef __INCLUDE_NUTTX_IPCC_H
+#define __INCLUDE_NUTTX_IPCC_H
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+#include <nuttx/mm/circbuf.h>
+#include <nuttx/semaphore.h>
+
+#include <poll.h>
+
+/****************************************************************************
+ * Public Types
+ ****************************************************************************/
+
+/* IPCC is a two-part driver:
+ *
+ * 1) A common upper half driver, that provides the character device
+ * interface for the user
+ * 2) Platform-specific lower half drivers that provide interface
+ * between upper and lower layer. Lower half also contains
+ * common objects like semaphore to inform
+ */
+
+/* This structure defines all of the operations provided by the
+ * architecture specific logic. All fields must be provided with
+ * non-NULL function pointers.
+ */
+
+struct ipcc_lower_s;
+struct ipcc_ops_s
+{
+ /**************************************************************************
+ * Name: read
+ *
+ * Description:
+ * Reads data directly from IPCC mbox, function shall not block.
+ * If no data is available, 0 shall be returned. Partial read
+ * is possible. In that case function can return value less
+ * than buflen.
+ *
+ * Input Parameters:
+ * ipcc - ipcc channel to read data from
+ * buffer - location where data shall be stored
+ * buflen - size of the buffer
+ *
+ * Returned Value:
+ * Number of bytes actually written to buffer.
+ *
+ **************************************************************************/
+
+ CODE ssize_t (*read)(FAR struct ipcc_lower_s *ipcc,
+ FAR char *buffer, size_t buflen);
+
+ /**************************************************************************
+ * Name: write
+ *
+ * Description:
+ * Writes data directly to IPCC mbox, function shall not block.
+ * If IPCC memory is full, or busy or otherwise unavailable for
+ * write, 0 shall be returned. Partial write is possible. In
+ * that case function can return value less then buflen.
+ *
+ * Input Parameters:
+ * ipcc - ipcc channel to write data to
+ * buffer - location to read data from
+ * buflen - number of bytes to write from buffer
+ *
+ * Returned Value:
+ * Number of actually written data to IPCC channel.
+ *
+ **************************************************************************/
+
+ CODE ssize_t (*write)(FAR struct ipcc_lower_s *ipcc,
+ FAR const char *buffer, size_t buflen);
+
+ /**************************************************************************
+ * Name: buffer_data
+ *
+ * Description:
+ * Lower driver shall copy data from IPCC to cirbuf on demand.
+ * It does not have to copy all data (in case rxbuf is full), but
+ * it must keep internal pointer to track uncopied data, and copy
+ * what is left on the next call to this function.
+ *
+ * Input Parameters:
+ * ipcc - channel to buffer data
+ * rxbuf - circural buffer where data should be written to
+ *
+ * Returned Value:
+ * Number of bytes that were successfully written to rxbuf.
+ *
+ **************************************************************************/
+
+ CODE ssize_t (*buffer_data)(FAR struct ipcc_lower_s *ipcc,
+ FAR struct circbuf_s *rxbuf);
+
+ /**************************************************************************
+ * Name: write_notify
+ *
+ * Description:
+ * Upper half driver notifies lower half driver that new data has been
+ * copied to circ buffer. This can be used by lower half to transfer
+ * data from buffer to IPCC memory.
+ *
+ * Input Parameters:
+ * ipcc - ipcc channel that wrote data.
+ *
+ * Returned Value:
+ * 0 on success, or negated errno otherwise
+ *
+ **************************************************************************/
+
+ CODE ssize_t (*write_notify)(FAR struct ipcc_lower_s *ipcc);
+
+ /**************************************************************************
+ * Name: cleanup
+ *
+ * Description:
+ * Cleans up resources initialized by <arch>_ipcc_init(). If arch code
+ * malloc()ed any memory, this funciton is responsible of freeing it.
+ * After this function is called, ipcc driver will not access lower
+ * half pointer anymore.
+ *
+ * Input Parameters:
+ * ipcc - ipcc channel to cleanup
+ *
+ * Returned Value:
+ * Always OK
+ *
+ **************************************************************************/
+
+ CODE int (*cleanup)(FAR struct ipcc_lower_s *ipcc);
+};
+
+/* This structure defines the interface between upper and lower halves.
+ * Such instance is passed to ipcc_register() function after driver
+ * has been initialized, binding the upper and lower halves into one
+ * driver.
+ */
+
+struct ipcc_lower_s
+{
+ FAR struct pollfd *fds[CONFIG_IPCC_NPOLLWAITERS];
Review Comment:
But does it really make sens to do so? ipcc_lower_s is a shared interface between lower and upper halfs of the driver.
We have 3 structs in that driver
**ipcc_driver_s** - accessible only by upper half of the driver
**ipcc_lower_s** - shared struct, accessible by both upper and lower - used to communicate two parts
**stm32wl5_ipcc_s** - accessible only by lower half of the driver
So I think it makes a lot of sense to leave all these fields in lower_s, because lower driver does access them - directly or indirectly. Even pollfd is used by lower half by calling ipcc_rxfree_notify() which then calls ipcc_pollnotify() that actually uses pollfd field. If I were to move these fields to **ipcc_driver_s** then lower half of the driver would have to have acess to **ipcc_drver_s** which does not really make much sense to me as lower part does not really have to know what the driver is doing. Lower half just has to implement all interfaces and use all fields in **ipcc_lower_s** as described, and that is all that lower half cares about.
What do you think now?
--
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] mlyszczek commented on a diff in pull request #6537: drivers/ipcc: add Inter Processor Communication driver
Posted by GitBox <gi...@apache.org>.
mlyszczek commented on code in PR #6537:
URL: https://github.com/apache/incubator-nuttx/pull/6537#discussion_r928636589
##########
drivers/ipcc/ipcc_write.c:
##########
@@ -0,0 +1,241 @@
+/****************************************************************************
+ * drivers/ipcc/ipcc_write.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/ipcc.h>
+#include <nuttx/kmalloc.h>
+#include <nuttx/semaphore.h>
+
+#include <assert.h>
+#include <errno.h>
+#include <fcntl.h>
+#include <stdbool.h>
+#include <stdio.h>
+#include <sys/types.h>
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: ipcc_txfree_notify
+ *
+ * Description:
+ * Notifies all blocked threads or those waiting in poll/select that
+ * there is place on buffer to perform writing.
+ *
+ * Input Parameters:
+ * ipcc - pointer to driver instance
+ *
+ * Returned Value:
+ * None
+ *
+ * Assumptions/Limitations:
+ * This function can be called from interrupt handler from lower half.
+ *
+ ****************************************************************************/
+
+void ipcc_txfree_notify(FAR struct ipcc_lower_s *ipcc)
+{
+ int semval;
+
+ if ((nxsem_get_value(&ipcc->txsem, &semval) == 0) && semval > 0)
+ {
+ /* Notify all poll/select waiters that they can write to the driver.
+ * Do it only when there are no already blocked writers to avoid
+ * situation where thread that is polling gets notified only to
+ * be blocked in write() because another thread have written to
+ * buffer before polling thread could.
+ */
+
+ ipcc_pollnotify(ipcc, POLLOUT);
+ return;
+ }
+
+ /* Notify all blocked writers that data is available to write */
+
+ do
+ {
+ nxsem_post(&ipcc->txsem);
+ }
+ while (nxsem_get_value(&ipcc->txsem, &semval) == 0 && semval <= 0);
+}
+
+/****************************************************************************
+ * Name: ipcc_write
+ *
+ * Description:
+ * Writes data to IPCC memory so that another CPU can read the contents.
+ * Will block untill whole buffer is copied unless signal is received
+ * or O_NONBLOCK flag is set.
+ *
+ * Input Parameters:
+ * filep - file on vfs associated with the driver
+ * buffer - buffer to copy to IPCC memory
+ * buflen - size of the buffer... buffer
+ *
+ * Returned Value:
+ * Number of successfully written bytes into the IPCC memory or netagted
+ * errno when no data could be written.
+ *
+ * Assumptions/Limitations:
+ *
+ ****************************************************************************/
+
+ssize_t ipcc_write(FAR struct file *filep, FAR const char *buffer,
+ size_t buflen)
+{
+ FAR struct ipcc_driver_s *priv;
+ ssize_t nwritten;
+ int ret;
+
+ /* Get our private data structure */
+
+ DEBUGASSERT(filep != NULL && filep->f_inode != NULL);
+ priv = filep->f_inode->i_private;
+
+ /* Get exclusive access to driver */
+
+ if ((ret = nxsem_wait(&priv->exclsem)))
+ {
+ /* nxsem_wait() will return on signal, we did not start
+ * any transfer yet, so we can safely return with error
+ */
+
+ return ret;
+ }
+
+ for (nwritten = 0; ; )
+ {
+#ifdef CONFIG_IPCC_BUFFERED
+ /* Buffered write, if buffer is empty try to write directly to
+ * IPCC memory, else buffer data in circbuf - it will be written
+ * in interrupt handler when IPCC tx channel is free.
+ */
+
+ if (circbuf_used(&priv->ipcc->txbuf) == 0)
+ {
+ /* Write buffer is empty, we can try and write data directly
+ * to IPCC memory thus omitting copying to buffer.
+ */
+
+ nwritten += priv->ipcc->ops.write(priv->ipcc, buffer + nwritten,
+ buflen - nwritten);
+ if (nwritten == (ssize_t)buflen || nwritten < 0)
+ {
+ /* We've managed to write whole buffer to IPCC memory,
+ * there is nothing else for use to do
+ * --or--
+ * lower half driver returned error during write,
+ *
+ * either way we return with nwritten which will either
+ * be number of bytes written or negated errno.
+ */
+
+ nxsem_post(&priv->exclsem);
+ return nwritten;
+ }
+ }
+
+ /* Either, there is already some data on the txbuffer, which
+ * means IPCC is occupied, or txbuffer is empty, but we could
+ * not write whole buffer to IPCC memory. In either case we
+ * copy what is left in data to buffer.
+ */
+
+ nwritten += circbuf_write(&priv->ipcc->txbuf, buffer + nwritten,
+ (ssize_t)buflen - nwritten);
+
+ /* Notify lower half that new data on circ buffer is available */
+
+ priv->ipcc->ops.write_notify(priv->ipcc);
+
+ if (nwritten == (ssize_t)buflen)
+ {
+ /* All outstanding data has been copied to txbuffer, we're done */
+
+ nxsem_post(&priv->exclsem);
+ return nwritten;
+ }
+
+#else /* CONFIG_IPCC_BUFFERED */
+ /* Unbuffered write, write data directly to lower driver */
+
+ nwritten += priv->ipcc->ops.write(&priv->ipcc, buffer, buflen);
Review Comment:
You don't. IPCC write() can't really fail. IPCC uses only RAM and some registers to send data. These things cannot really fail, and if they do, then it's a critical hardware failure that I don't think we should care about.
There is possibility that this will always return 0, but in that case second CPU simply does not consume data or never informs us that data has been read - it's a bug in IPCC implementation of second CPU. With that bug we can also hang in semaphore waiting for the interrupt.
If user expects such situations, O_NONBLOCK then should be used.
That said, that nwritten < 0 check is useless and I will remove it.
--
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] acassis commented on pull request #6537: [API review request] drivers/ipcc: add Inter Processor Communication driver
Posted by GitBox <gi...@apache.org>.
acassis commented on PR #6537:
URL: https://github.com/apache/incubator-nuttx/pull/6537#issuecomment-1171634112
@xiaoxiang781216 @mlyszczek I think it is good to have a light IPC on NuttX and of course the fact of having it doesn't mean people cannot use rpmsg or openamp (or their both integration). For some people having more options could be confusing, but I think it is a good thing, it means flexibility. Same way people can use NuttX native NX Graphics libs or LVGL or just framebuffer or whatever.
--
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 pull request #6537: [API review request] drivers/ipcc: add Inter Processor Communication driver
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on PR #6537:
URL: https://github.com/apache/incubator-nuttx/pull/6537#issuecomment-1171520018
> Just to note, as for STM32WB (which uses same IPCC hardware) it is not possible to run user code on CPU2, only vendor encrypted binaries. For BLE we have `bt_driver`, but for ZigBee and Thread we still need a solution. I am not sure whether it should be a generic IPCC driver, or IPCC as a part of a more specific `lora_driver`/`zigbee_driver`/`thread_driver` or just an app-side library.
people can define many different and incompatible protocol on top of share memory. It's very hard to reuse the definition or code on the different hardware or even the same hardware but different firmware.
Virtio/rpmsg is only the solution accept widely by many platform(Linux/FreeBSD/RTOS) and maintained by the committee as far as I know.
--
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] onegray commented on pull request #6537: [API review request] drivers/ipcc: add Inter Processor Communication driver
Posted by GitBox <gi...@apache.org>.
onegray commented on PR #6537:
URL: https://github.com/apache/incubator-nuttx/pull/6537#issuecomment-1171451344
Just to note, as for STM32WB (which uses same IPCC hardware) it is not possible to run user code on CPU2, only vendor encrypted binaries. For BLE we have `bt_driver`, but for ZigBee and Thread we still need a solution. I am not sure whether it should be a generic IPCC driver, or IPCC as a part of a more specific `lora_driver`/`zigbee_driver`/`thread_driver` or just an app-side library.
--
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] onegray commented on pull request #6537: [API review request] drivers/ipcc: add Inter Processor Communication driver
Posted by GitBox <gi...@apache.org>.
onegray commented on PR #6537:
URL: https://github.com/apache/incubator-nuttx/pull/6537#issuecomment-1175299311
Yes, for stm32wb we have to follow the CPU2 firmware logic which is quite specific. The communication for each IPCC channel is represented by async event queue shared in memory as a linked list. The channels have fixed assignment and just two of them are needed for BLE (for system commands and for BLE commands).
Fortunately we have the [`bt_driver_s`](https://github.com/apache/incubator-nuttx/blob/master/include/nuttx/wireless/bluetooth/bt_driver.h) which suits for BLE in HCI mode. So for stm32wb it looks like a way to go, similar to `nrf52`.
--
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] mlyszczek commented on pull request #6537: [API review request] drivers/ipcc: add Inter Processor Communication driver
Posted by GitBox <gi...@apache.org>.
mlyszczek commented on PR #6537:
URL: https://github.com/apache/incubator-nuttx/pull/6537#issuecomment-1175233822
I've added lower half driver for the IPCC. The only test I've done is that I compiled it. I am going to write some tests for second cpu with stm32cube and fix any bugs that I find.
So right now I would like to only request design review
--
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] onegray commented on a diff in pull request #6537: drivers/ipcc: add Inter Processor Communication driver
Posted by GitBox <gi...@apache.org>.
onegray commented on code in PR #6537:
URL: https://github.com/apache/incubator-nuttx/pull/6537#discussion_r923529376
##########
arch/arm/src/stm32wl5/stm32wl5_ipcc.c:
##########
@@ -0,0 +1,801 @@
+/****************************************************************************
+ * arch/arm/src/stm32wl5/stm32wl5_ipcc.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/ipcc.h>
+#include <nuttx/irq.h>
+
+#include <arm_internal.h>
+#include <assert.h>
+#include <debug.h>
+
+#include <arch/stm32wl5/stm32wl5xxx_cpu1_irq.h>
+#include "hardware/stm32wl5_ipcc.h"
+#include "stm32wl5.h"
+#include "stm32wl5_ipcc.h"
+
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+/* This structure describes tx or rx of single channel in memory */
+
+struct stm32wl5_ipcc_chan_mem_s
+{
+ unsigned len; /* Number of valid bytes in data[] */
+ char data[]; /* Data in IPCC memory */
+};
+
+/* Internal stm32wl5 ipcc structure describing channel state. */
+
+struct stm32wl5_ipcc_s
+{
+ /* Pointer to API connecting upper and lower half of the driver */
+
+ struct ipcc_lower_s *ipcc;
+
+ /* Physical memory address where second CPU will write data for us,
+ * we will be reading from this memory.
+ */
+
+ char *rxmem;
+
+ /* Maximum length of data that rxmem can hold. It is size of the
+ * reserved space for rxmem minus sizeof(stm32wl5_ipcc_chan_mem_s.len)
+ */
+
+ unsigned rxlen;
+
+#if CONFIG_IPCC_BUFFERED
+ /* Number of bytes copied from IPCC memory to buffer. Can be less than
+ * stm32wl5_ipcc_chan_mem_s.len after copy operation when buffer is full.
+ * Value can persist between multiple ISR and stm32wl5_ipcc_buffer_data()
+ * calls, until all data from IPCC memory is successfully buffered.
+ */
+
+ unsigned rxcopied;
+#endif
+
+ /* Physical memory address where we will write data for the second
+ * CPU to read.
+ */
+
+ char *txmem;
+
+ /* Maximum length of data that txmem can hold. It is size of the
+ * reserved space for txmem minus sizeof(stm32wl5_ipcc_chan_mem_s.len)
+ */
+
+ unsigned txlen;
+};
+
+/****************************************************************************
+ * Private Function Prototypes
+ ****************************************************************************/
+
+static ssize_t stm32wl5_ipcc_read(struct ipcc_lower_s *ipcc,
+ char *buffer, size_t buflen);
+static ssize_t stm32wl5_ipcc_write(struct ipcc_lower_s *ipcc,
+ const char *buffer, size_t buflen);
+static ssize_t stm32wl5_ipcc_buffer_data(struct ipcc_lower_s *ipcc,
+ struct circbuf_s *rxbuf);
+static ssize_t stm32wl5_ipcc_copy_to_buffer(int chan,
+ struct circbuf_s *rxbuf);
+static int stm32wl5_ipcc_rx_isr(int irq, void *context, void *arg);
+static int stm32wl5_ipcc_tx_isr(int irq, void *context, void *arg);
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+struct stm32wl5_ipcc_s g_ipccpriv[IPCC_NCHAN] =
+{
+ /* Channel 1 is always enabled when IPCC is enabled */
+
+ {
+ .rxmem = (char *)(IPCC_CHAN1_START),
+ .rxlen = IPCC_CHAN1_RX_SIZE - sizeof(unsigned),
+ .txmem = (char *)(IPCC_CHAN1_START + IPCC_CHAN1_RX_SIZE),
+ .txlen = IPCC_CHAN1_TX_SIZE - sizeof(unsigned)
+ }
+
+#if IPCC_CHAN2
+ ,
+ {
+ .rxmem = (char *)(IPCC_CHAN2_START),
+ .rxlen = IPCC_CHAN2_RX_SIZE - sizeof(unsigned),
+ .txmem = (char *)(IPCC_CHAN2_START + IPCC_CHAN2_RX_SIZE),
+ .txlen = IPCC_CHAN2_TX_SIZE - sizeof(unsigned)
+ }
+#endif
+
+#if IPCC_CHAN3
+ ,
+ {
+ .rxmem = (char *)(IPCC_CHAN3_START),
+ .rxlen = IPCC_CHAN3_RX_SIZE - sizeof(unsigned),
+ .txmem = (char *)(IPCC_CHAN3_START + IPCC_CHAN3_RX_SIZE),
+ .txlen = IPCC_CHAN3_TX_SIZE - sizeof(unsigned)
+ }
+#endif
+
+#if IPCC_CHAN4
+ ,
+ {
+ .rxmem = (char *)(IPCC_CHAN4_START),
+ .rxlen = IPCC_CHAN4_RX_SIZE - sizeof(unsigned),
+ .txmem = (char *)(IPCC_CHAN4_START + IPCC_CHAN4_RX_SIZE),
+ .txlen = IPCC_CHAN4_TX_SIZE - sizeof(unsigned)
+ }
+#endif
+
+#if IPCC_CHAN5
+ ,
+ {
+ .rxmem = (char *)(IPCC_CHAN5_START),
+ .rxlen = IPCC_CHAN5_RX_SIZE - sizeof(unsigned),
+ .txmem = (char *)(IPCC_CHAN5_START + IPCC_CHAN5_RX_SIZE),
+ .txlen = IPCC_CHAN5_TX_SIZE - sizeof(unsigned)
+ }
+#endif
+
+#if IPCC_CHAN6
+ ,
+ {
+ .rxmem = (char *)(IPCC_CHAN6_START),
+ .rxlen = IPCC_CHAN6_RX_SIZE - sizeof(unsigned),
+ .txmem = (char *)(IPCC_CHAN6_START + IPCC_CHAN6_RX_SIZE),
+ .txlen = IPCC_CHAN6_TX_SIZE - sizeof(unsigned)
+ }
+#endif
+};
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: stm32wl5_ipcc_tx_isr
+ *
+ * Description:
+ * IPCC TX interrupt service routine. This interrupt is called when
+ * second CPU read all data from IPCC TX memory and we are free to
+ * write new data to that memory.
+ *
+ * For buffered IPCC, we will immediately write data from buffer to
+ * IPCC memory and notify second CPU. Also blocked writer will be
+ * notified that there is free space on tx buffer.
+ *
+ * For unbuffered IPCC, we won't copy anything but only notify blocked
+ * writers that IPCC TX memory is free.
+ *
+ * Input Parameters:
+ * irq - Number of IRQ that generated interrupt
+ * context - Interrupt register state save info (architecture-specific)
+ * arg - user data (not used by us)
+ *
+ * Returned Value:
+ * Always OK
+ *
+ * Assumptions/Limitations:
+ *
+ ****************************************************************************/
+
+static int stm32wl5_ipcc_tx_isr(int irq, void *context, void *arg)
+{
+ int chan;
+ size_t nwritten;
+ uint32_t mr;
+ uint32_t sr;
+ uint32_t status;
+ struct stm32wl5_ipcc_s *priv;
+ struct stm32wl5_ipcc_chan_mem_s *txmem;
+
+ (void)context;
+ (void)arg;
+ (void)irq;
+
+ /* Disable TX interrupt, we don't want two "threads" to concurently
+ * modify circbuffers as we don't use any guard to them
+ */
+
+ up_disable_irq(STM32WL5_IRQ_IPCC_C1_TX_IT);
Review Comment:
Looks not needed since we are already in interrupt.
--
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] mlyszczek commented on a diff in pull request #6537: drivers/ipcc: add Inter Processor Communication driver
Posted by GitBox <gi...@apache.org>.
mlyszczek commented on code in PR #6537:
URL: https://github.com/apache/incubator-nuttx/pull/6537#discussion_r923627203
##########
arch/arm/src/stm32wl5/stm32wl5_ipcc.c:
##########
@@ -0,0 +1,801 @@
+/****************************************************************************
+ * arch/arm/src/stm32wl5/stm32wl5_ipcc.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/ipcc.h>
+#include <nuttx/irq.h>
+
+#include <arm_internal.h>
+#include <assert.h>
+#include <debug.h>
+
+#include <arch/stm32wl5/stm32wl5xxx_cpu1_irq.h>
+#include "hardware/stm32wl5_ipcc.h"
+#include "stm32wl5.h"
+#include "stm32wl5_ipcc.h"
+
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+/* This structure describes tx or rx of single channel in memory */
+
+struct stm32wl5_ipcc_chan_mem_s
+{
+ unsigned len; /* Number of valid bytes in data[] */
+ char data[]; /* Data in IPCC memory */
+};
+
+/* Internal stm32wl5 ipcc structure describing channel state. */
+
+struct stm32wl5_ipcc_s
+{
+ /* Pointer to API connecting upper and lower half of the driver */
+
+ struct ipcc_lower_s *ipcc;
+
+ /* Physical memory address where second CPU will write data for us,
+ * we will be reading from this memory.
+ */
+
+ char *rxmem;
+
+ /* Maximum length of data that rxmem can hold. It is size of the
+ * reserved space for rxmem minus sizeof(stm32wl5_ipcc_chan_mem_s.len)
+ */
+
+ unsigned rxlen;
+
+#if CONFIG_IPCC_BUFFERED
+ /* Number of bytes copied from IPCC memory to buffer. Can be less than
+ * stm32wl5_ipcc_chan_mem_s.len after copy operation when buffer is full.
+ * Value can persist between multiple ISR and stm32wl5_ipcc_buffer_data()
+ * calls, until all data from IPCC memory is successfully buffered.
+ */
+
+ unsigned rxcopied;
+#endif
+
+ /* Physical memory address where we will write data for the second
+ * CPU to read.
+ */
+
+ char *txmem;
+
+ /* Maximum length of data that txmem can hold. It is size of the
+ * reserved space for txmem minus sizeof(stm32wl5_ipcc_chan_mem_s.len)
+ */
+
+ unsigned txlen;
+};
+
+/****************************************************************************
+ * Private Function Prototypes
+ ****************************************************************************/
+
+static ssize_t stm32wl5_ipcc_read(struct ipcc_lower_s *ipcc,
+ char *buffer, size_t buflen);
+static ssize_t stm32wl5_ipcc_write(struct ipcc_lower_s *ipcc,
+ const char *buffer, size_t buflen);
+static ssize_t stm32wl5_ipcc_buffer_data(struct ipcc_lower_s *ipcc,
+ struct circbuf_s *rxbuf);
+static ssize_t stm32wl5_ipcc_copy_to_buffer(int chan,
+ struct circbuf_s *rxbuf);
+static int stm32wl5_ipcc_rx_isr(int irq, void *context, void *arg);
+static int stm32wl5_ipcc_tx_isr(int irq, void *context, void *arg);
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+struct stm32wl5_ipcc_s g_ipccpriv[IPCC_NCHAN] =
+{
+ /* Channel 1 is always enabled when IPCC is enabled */
+
+ {
+ .rxmem = (char *)(IPCC_CHAN1_START),
+ .rxlen = IPCC_CHAN1_RX_SIZE - sizeof(unsigned),
+ .txmem = (char *)(IPCC_CHAN1_START + IPCC_CHAN1_RX_SIZE),
+ .txlen = IPCC_CHAN1_TX_SIZE - sizeof(unsigned)
+ }
+
+#if IPCC_CHAN2
+ ,
+ {
+ .rxmem = (char *)(IPCC_CHAN2_START),
+ .rxlen = IPCC_CHAN2_RX_SIZE - sizeof(unsigned),
+ .txmem = (char *)(IPCC_CHAN2_START + IPCC_CHAN2_RX_SIZE),
+ .txlen = IPCC_CHAN2_TX_SIZE - sizeof(unsigned)
+ }
+#endif
+
+#if IPCC_CHAN3
+ ,
+ {
+ .rxmem = (char *)(IPCC_CHAN3_START),
+ .rxlen = IPCC_CHAN3_RX_SIZE - sizeof(unsigned),
+ .txmem = (char *)(IPCC_CHAN3_START + IPCC_CHAN3_RX_SIZE),
+ .txlen = IPCC_CHAN3_TX_SIZE - sizeof(unsigned)
+ }
+#endif
+
+#if IPCC_CHAN4
+ ,
+ {
+ .rxmem = (char *)(IPCC_CHAN4_START),
+ .rxlen = IPCC_CHAN4_RX_SIZE - sizeof(unsigned),
+ .txmem = (char *)(IPCC_CHAN4_START + IPCC_CHAN4_RX_SIZE),
+ .txlen = IPCC_CHAN4_TX_SIZE - sizeof(unsigned)
+ }
+#endif
+
+#if IPCC_CHAN5
+ ,
+ {
+ .rxmem = (char *)(IPCC_CHAN5_START),
+ .rxlen = IPCC_CHAN5_RX_SIZE - sizeof(unsigned),
+ .txmem = (char *)(IPCC_CHAN5_START + IPCC_CHAN5_RX_SIZE),
+ .txlen = IPCC_CHAN5_TX_SIZE - sizeof(unsigned)
+ }
+#endif
+
+#if IPCC_CHAN6
+ ,
+ {
+ .rxmem = (char *)(IPCC_CHAN6_START),
+ .rxlen = IPCC_CHAN6_RX_SIZE - sizeof(unsigned),
+ .txmem = (char *)(IPCC_CHAN6_START + IPCC_CHAN6_RX_SIZE),
+ .txlen = IPCC_CHAN6_TX_SIZE - sizeof(unsigned)
+ }
+#endif
+};
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: stm32wl5_ipcc_tx_isr
+ *
+ * Description:
+ * IPCC TX interrupt service routine. This interrupt is called when
+ * second CPU read all data from IPCC TX memory and we are free to
+ * write new data to that memory.
+ *
+ * For buffered IPCC, we will immediately write data from buffer to
+ * IPCC memory and notify second CPU. Also blocked writer will be
+ * notified that there is free space on tx buffer.
+ *
+ * For unbuffered IPCC, we won't copy anything but only notify blocked
+ * writers that IPCC TX memory is free.
+ *
+ * Input Parameters:
+ * irq - Number of IRQ that generated interrupt
+ * context - Interrupt register state save info (architecture-specific)
+ * arg - user data (not used by us)
+ *
+ * Returned Value:
+ * Always OK
+ *
+ * Assumptions/Limitations:
+ *
+ ****************************************************************************/
+
+static int stm32wl5_ipcc_tx_isr(int irq, void *context, void *arg)
+{
+ int chan;
+ size_t nwritten;
+ uint32_t mr;
+ uint32_t sr;
+ uint32_t status;
+ struct stm32wl5_ipcc_s *priv;
+ struct stm32wl5_ipcc_chan_mem_s *txmem;
+
+ (void)context;
+ (void)arg;
+ (void)irq;
+
+ /* Disable TX interrupt, we don't want two "threads" to concurently
+ * modify circbuffers as we don't use any guard to them
+ */
+
+ up_disable_irq(STM32WL5_IRQ_IPCC_C1_TX_IT);
Review Comment:
You are right. Removed.
--
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] mlyszczek commented on pull request #6537: [API review request] drivers/ipcc: add Inter Processor Communication driver
Posted by GitBox <gi...@apache.org>.
mlyszczek commented on PR #6537:
URL: https://github.com/apache/incubator-nuttx/pull/6537#issuecomment-1186629241
Ok, this should be ready to review now. As @acassis suggested I marked this driver as EXPERIMENTAL, as I don't feel I've tested it enough. I did perform tens of thousands tests - mainly write/read with different buffer sizes (less than, equal and n-times bigger than IPCC memory can hold) and it all worked fine, but still, I would like to keep this EXPERIMENTAL.
In the next few months I will test this in real live scenarios so I will fix any lurking bugs that - well, are probably there. When I move my code to production and decide I am happy with the quality of the driver I will make a commit to remove EXPERIMENTAL.
EXPERIMENTAL flag also gives me a little room to change API/design if I need to.
--
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] mlyszczek commented on pull request #6537: [API review request] drivers/ipcc: add Inter Processor Communication driver
Posted by GitBox <gi...@apache.org>.
mlyszczek commented on PR #6537:
URL: https://github.com/apache/incubator-nuttx/pull/6537#issuecomment-1168891407
I apologize but I am not really sure I understand the question.
I need IPCC to communicate 2 CPUs on stm32wl5. It's a trivial controller, software is responsible to reserve memory, and hardware simply send iterrupts when software says "I copied data from read buffer - you can put new data there" or "I left you something in a memory, go check it out". I did not work with other inter processor communication controllers but I image it will not be that different and I tried to keep this upper half as generic as possible.
--
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 pull request #6537: [API review request] drivers/ipcc: add Inter Processor Communication driver
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on PR #6537:
URL: https://github.com/apache/incubator-nuttx/pull/6537#issuecomment-1171903202
> @xiaoxiang781216 did you see that: https://www.redhat.com/en/blog/deep-dive-virtio-networking-and-vhost-net ? So I'll include eventfd as another IPC supported on Linux (and NuttX)
OpenAMP is trying add virtio support, once it's ready we plan to add virito driver to NuttX:
https://github.com/OpenAMP/open-amp/issues/390
--
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 pull request #6537: [API review request] drivers/ipcc: add Inter Processor Communication driver
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on PR #6537:
URL: https://github.com/apache/incubator-nuttx/pull/6537#issuecomment-1171480823
> Yeah, so I've compiled stub code and my flash increased by 18kB. It will probably take similar ammount of memory to compile on second CPU and I don't suppose there is some trickery to share flash code between CPUs ;) And I don't know what else will have to be compiled for another CPU to support OpenAMP there, so it might be another extra tens of kB of flash.
>
Yes, the second cpu need consume the same size of flash
> And my project requires substential ammount of flash as it is so adding around 40kB just for IPCC is hard to swallow.
>
> So I've decieded that I will continue with that simple character device stream. The question is, are you going to accept this into upstream or should I not even bother creating pull request for this and just keep it internally?
Yes, OpenAMP just give you a choice to see whether it can save your development effort and reuse other's work.
--
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] mlyszczek commented on a diff in pull request #6537: drivers/ipcc: add Inter Processor Communication driver
Posted by GitBox <gi...@apache.org>.
mlyszczek commented on code in PR #6537:
URL: https://github.com/apache/incubator-nuttx/pull/6537#discussion_r923160607
##########
drivers/ipcc/ipcc_register.c:
##########
@@ -0,0 +1,208 @@
+/****************************************************************************
+ * drivers/ipcc/ipcc_register.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/ipcc.h>
+#include <nuttx/kmalloc.h>
+#include <nuttx/mm/circbuf.h>
+
+#include <assert.h>
+#include <errno.h>
+#include <stdbool.h>
+#include <stdio.h>
+#include <sys/types.h>
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/* Device naming ************************************************************/
+
+#define DEVNAME_FMT "/dev/ipcc%d"
+#define DEVNAME_FMTLEN (9 + 3 + 1)
+
+/****************************************************************************
+ * Private Function Prototypes
+ ****************************************************************************/
+
+#ifndef CONFIG_DISABLE_PSEUDOFS_OPERATIONS
+int ipcc_open(FAR struct file *filep);
+int ipcc_close(FAR struct file *filep);
+int ipcc_unlink(FAR struct inode *inode);
+#endif
+int ipcc_ioctl(FAR struct file *filep, int cmd, unsigned long arg);
+ssize_t ipcc_read(FAR struct file *filep, FAR char *buffer, size_t buflen);
+ssize_t ipcc_write(FAR struct file *filep, FAR const char *buffer,
+ size_t buflen);
+int ipcc_poll(FAR struct file *filep, FAR struct pollfd *fds,
+ bool setup);
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+static const struct file_operations ipcc_fops =
+{
+#ifdef CONFIG_DISABLE_PSEUDOFS_OPERATIONS
+ .open = NULL,
+ .close = NULL,
+ .unlink = NULL,
+#else /* CONFIG_DISABLE_PSEUDOFS_OPERATIONS */
+ .open = ipcc_open,
+ .close = ipcc_close,
+ .unlink = ipcc_unlink,
+#endif /* CONFIG_DISABLE_PSEUDOFS_OPERATIONS */
+ .poll = ipcc_poll,
+ .read = ipcc_read,
+ .write = ipcc_write,
+ .ioctl = NULL,
+ .seek = NULL
+};
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: ipcc_cleanup
+ *
+ * Description:
+ * Cleans up resources allocated by ipcc_register()
+ *
+ * Input Parameters:
+ * priv - ipcc driver instance to clean up
+ *
+ * Returned Value:
+ * None
+ *
+ * Assumptions/Limitations:
+ * This function should be called only when ipcc_register is run with
+ * success and all resources in driver instance are properly allocated.
+ *
+ ****************************************************************************/
+
+void ipcc_cleanup(FAR struct ipcc_driver_s *priv)
+{
+#ifdef CONFIG_IPCC_BUFFERED
+ circbuf_uninit(&priv->ipcc.rxbuf);
+ circbuf_uninit(&priv->ipcc.txbuf);
+#endif
+ nxsem_destroy(&priv->ipcc.rxsem);
+ nxsem_destroy(&priv->ipcc.txsem);
+ up_ipcc_cleanup(&priv->ipcc);
+ nxsem_destroy(&priv->exclsem);
+ kmm_free(priv);
+}
+
+/****************************************************************************
+ * Name: ipcc_register
+ *
+ * Description:
+ * Create and register the IPCC character driver.
+ *
+ * IPCC is a simple character driver that supports inter processor
+ * communication.
+ *
+ * Input Parameters:
+ * ipcc - An instance of the lower half IPCC driver
+ * chan - IPCC channel. This will be used ad IPCC minor number.
+ * IPPC will be registered as /dev/ipccN where N is the minor number.
+ * buflen - Length of tx and rx buffers, 0 for unbuffered communication.
+ *
+ * Returned Value:
+ * OK if the driver was successfully registered, or negated errno on
+ * failure.
+ *
+ * Assumptions/Limitations:
+ *
+ ****************************************************************************/
+
+#ifdef CONFIG_IPCC_BUFFERED
+int ipcc_register(int chan, size_t rxbuflen, size_t txbuflen)
+#else
+int ipcc_register(int chan)
+#endif
+{
+ FAR struct ipcc_driver_s *priv;
+ char devname[DEVNAME_FMTLEN];
+ int ret;
+
+ /* Allocate a IPCC character device structure */
+
+ if ((priv = kmm_zalloc(sizeof(*priv))) == NULL)
+ {
+ return -ENOMEM;
+ }
+
+ if ((ret = up_ipcc_init(&priv->ipcc, chan)))
Review Comment:
Less work for the implementor I guess. He just has to pass channel number to initialize. Do you see any drawback of that? Would it be better for implementor to pass ipcc_driver_s directly?
--
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 diff in pull request #6537: drivers/ipcc: add Inter Processor Communication driver
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #6537:
URL: https://github.com/apache/incubator-nuttx/pull/6537#discussion_r923165348
##########
drivers/ipcc/ipcc_register.c:
##########
@@ -0,0 +1,208 @@
+/****************************************************************************
+ * drivers/ipcc/ipcc_register.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/ipcc.h>
+#include <nuttx/kmalloc.h>
+#include <nuttx/mm/circbuf.h>
+
+#include <assert.h>
+#include <errno.h>
+#include <stdbool.h>
+#include <stdio.h>
+#include <sys/types.h>
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/* Device naming ************************************************************/
+
+#define DEVNAME_FMT "/dev/ipcc%d"
+#define DEVNAME_FMTLEN (9 + 3 + 1)
+
+/****************************************************************************
+ * Private Function Prototypes
+ ****************************************************************************/
+
+#ifndef CONFIG_DISABLE_PSEUDOFS_OPERATIONS
+int ipcc_open(FAR struct file *filep);
+int ipcc_close(FAR struct file *filep);
+int ipcc_unlink(FAR struct inode *inode);
+#endif
+int ipcc_ioctl(FAR struct file *filep, int cmd, unsigned long arg);
+ssize_t ipcc_read(FAR struct file *filep, FAR char *buffer, size_t buflen);
+ssize_t ipcc_write(FAR struct file *filep, FAR const char *buffer,
+ size_t buflen);
+int ipcc_poll(FAR struct file *filep, FAR struct pollfd *fds,
+ bool setup);
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+static const struct file_operations ipcc_fops =
+{
+#ifdef CONFIG_DISABLE_PSEUDOFS_OPERATIONS
+ .open = NULL,
+ .close = NULL,
+ .unlink = NULL,
+#else /* CONFIG_DISABLE_PSEUDOFS_OPERATIONS */
+ .open = ipcc_open,
+ .close = ipcc_close,
+ .unlink = ipcc_unlink,
+#endif /* CONFIG_DISABLE_PSEUDOFS_OPERATIONS */
+ .poll = ipcc_poll,
+ .read = ipcc_read,
+ .write = ipcc_write,
+ .ioctl = NULL,
+ .seek = NULL
+};
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: ipcc_cleanup
+ *
+ * Description:
+ * Cleans up resources allocated by ipcc_register()
+ *
+ * Input Parameters:
+ * priv - ipcc driver instance to clean up
+ *
+ * Returned Value:
+ * None
+ *
+ * Assumptions/Limitations:
+ * This function should be called only when ipcc_register is run with
+ * success and all resources in driver instance are properly allocated.
+ *
+ ****************************************************************************/
+
+void ipcc_cleanup(FAR struct ipcc_driver_s *priv)
+{
+#ifdef CONFIG_IPCC_BUFFERED
+ circbuf_uninit(&priv->ipcc.rxbuf);
+ circbuf_uninit(&priv->ipcc.txbuf);
+#endif
+ nxsem_destroy(&priv->ipcc.rxsem);
+ nxsem_destroy(&priv->ipcc.txsem);
+ up_ipcc_cleanup(&priv->ipcc);
+ nxsem_destroy(&priv->exclsem);
+ kmm_free(priv);
+}
+
+/****************************************************************************
+ * Name: ipcc_register
+ *
+ * Description:
+ * Create and register the IPCC character driver.
+ *
+ * IPCC is a simple character driver that supports inter processor
+ * communication.
+ *
+ * Input Parameters:
+ * ipcc - An instance of the lower half IPCC driver
+ * chan - IPCC channel. This will be used ad IPCC minor number.
+ * IPPC will be registered as /dev/ipccN where N is the minor number.
+ * buflen - Length of tx and rx buffers, 0 for unbuffered communication.
+ *
+ * Returned Value:
+ * OK if the driver was successfully registered, or negated errno on
+ * failure.
+ *
+ * Assumptions/Limitations:
+ *
+ ****************************************************************************/
+
+#ifdef CONFIG_IPCC_BUFFERED
+int ipcc_register(int chan, size_t rxbuflen, size_t txbuflen)
+#else
+int ipcc_register(int chan)
+#endif
+{
+ FAR struct ipcc_driver_s *priv;
+ char devname[DEVNAME_FMTLEN];
+ int ret;
+
+ /* Allocate a IPCC character device structure */
+
+ if ((priv = kmm_zalloc(sizeof(*priv))) == NULL)
+ {
+ return -ENOMEM;
+ }
+
+ if ((ret = up_ipcc_init(&priv->ipcc, chan)))
Review Comment:
This approach require the user implement up_ipcc_init which isn't consistent with ipcc_ops.
--
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] mlyszczek commented on a diff in pull request #6537: drivers/ipcc: add Inter Processor Communication driver
Posted by GitBox <gi...@apache.org>.
mlyszczek commented on code in PR #6537:
URL: https://github.com/apache/incubator-nuttx/pull/6537#discussion_r923164475
##########
drivers/ipcc/ipcc_unlink.c:
##########
@@ -0,0 +1,94 @@
+/****************************************************************************
+ * drivers/ipcc/ipcc_unlink.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/ipcc.h>
+#include <nuttx/kmalloc.h>
+#include <nuttx/semaphore.h>
+
+#include <assert.h>
+#include <errno.h>
+#include <stdbool.h>
+#include <stdio.h>
+#include <sys/types.h>
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: ipcc_unlink
+ *
+ * Description:
+ * Action to take upon file unlinking. Function will free resources if
+ * noone is using the driver when unlinking occured. If driver is still
+ * in use, it will be marked as unlinked and resource freeing will take
+ * place in ipcc_close() function instead, once last reference is closed.
+ *
+ * Input Parameters:
+ * inode - driver inode that is being unlinked
+ *
+ * Returned Value:
+ * OK on successfull close, or negated errno on failure.
+ *
+ * Assumptions/Limitations:
+ *
+ ****************************************************************************/
+
+int ipcc_unlink(FAR struct inode *inode)
Review Comment:
Not sure about that. What about case, when you want to send some stuff over channel during initialization, and then free resources for other things for the rest of the project live? I understand it's a corner case and freeing is not good thing to do in embedded. But since the code is here, why not let it stay so the driver can be more complete?
--
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] mlyszczek commented on a diff in pull request #6537: drivers/ipcc: add Inter Processor Communication driver
Posted by GitBox <gi...@apache.org>.
mlyszczek commented on code in PR #6537:
URL: https://github.com/apache/incubator-nuttx/pull/6537#discussion_r923169301
##########
include/nuttx/ipcc.h:
##########
@@ -0,0 +1,188 @@
+/****************************************************************************
+ * include/nuttx/ipcc.h
+ *
+ * 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/mm/circbuf.h>
+#include <nuttx/semaphore.h>
+
+#include <poll.h>
+
+/****************************************************************************
+ * Public Types
+ ****************************************************************************/
+
+/* IPCC is a two-part driver:
+ *
+ * 1) A common upper half driver, that provides the character device
+ * interface for the user
+ * 2) Platform-specific lower half drivers that provide interface
+ * between upper and lower layer. Lower half also contains
+ * common objects like semaphore to inform
+ */
+
+/* This structure defines all of the operations provided by the
+ * architecture specific logic. All fields must be provided with
+ * non-NULL function pointers.
+ */
+
+struct ipcc_lower_s;
+struct ipcc_ops_s
+{
+ /* Reads data directly from IPCC mbox, should not block, ever.
+ * If no data is available, 0 should be returned. Partial read
+ * is possible. In that case function will return value less
+ * than buflen.
+ */
+
+ CODE ssize_t (*read)(FAR struct ipcc_lower_s *ipcc,
+ FAR char *buffer, size_t buflen);
+
+ /* Writes data directly to IPCC mbox, should not block, ever.
+ * If IPCC memory is full, or busy or otherwise unavailable for
+ * write, 0 should be returned. Partial write is possible. In
+ * that case function will return value less then buflen.
+ */
+
+ CODE ssize_t (*write)(FAR struct ipcc_lower_s *ipcc,
+ FAR const char *buffer, size_t buflen);
+
+ /* Lower driver should copy data from IPCC to cirbuf on demand.
+ * It does not have to copy all data (in case rxbuf is full), but
+ * it should keep internal pointer to track uncopied data, and copy
+ * what is left on the next call to this function.
+ */
+
+ CODE ssize_t (*buffer_data)(FAR struct ipcc_lower_s *ipcc,
+ FAR struct circbuf_s *rxbuf);
+
+ /* Upper half notifies lower half driver that new data has been
+ * copied to circ buffer
+ */
+
+ CODE ssize_t (*write_notify)(FAR struct ipcc_lower_s *ipcc);
+};
+
+/* This structure defines the interface between upper and lower halves.
+ * Such instance is passed to ipcc_register() function after driver
+ * has been initialized, binding the upper and lower halves into one
+ * driver.
+ */
+
+struct ipcc_lower_s
+{
+ FAR struct pollfd *fds[CONFIG_IPCC_NPOLLWAITERS];
+ FAR struct ipcc_ops_s ops; /* Arch specific functions */
+ sem_t rxsem; /* Data ready to read semaphore */
+ sem_t txsem; /* Data ready to send semaphore */
+ int chan; /* IPCC channel */
+
+#ifdef CONFIG_IPCC_BUFFERED
+ struct circbuf_s rxbuf; /* Receive buffer */
+ struct circbuf_s txbuf; /* Transmit buffer */
+
+ /* 1- not all data could fit into buffer, some unread data is still
+ * in IPCC mbox. State of this variable is maintained by lower half,
+ * upper half only reads it to know when to inform lower driver that
+ * buffer got free.
+ */
+
+ int overflow;
+#endif /* CONFIG_IPCC_BUFFERED */
+};
+
+/* Driver state structure */
+
+struct ipcc_driver_s
+{
+ FAR struct ipcc_lower_s ipcc; /* Lower half driver state */
+ int crefs; /* Count number of open references */
+ int unlinked; /* 1 - driver has been unlinked */
+ sem_t exclsem; /* Mutual exclusion for driver */
+};
+
+/****************************************************************************
+ * Public Functions Prototypes
+ ****************************************************************************/
+
+void ipcc_pollnotify(FAR struct ipcc_lower_s *ipcc, pollevent_t eventset);
+void ipcc_cleanup(FAR struct ipcc_driver_s *priv);
+void ipcc_txfree_notify(FAR struct ipcc_lower_s *priv);
+void ipcc_rxfree_notify(FAR struct ipcc_lower_s *ipcc);
+
+#ifdef CONFIG_IPCC_BUFFERED
+int ipcc_register(int chan, size_t rxbuflen, size_t txbuflen);
+#else
+int ipcc_register(int chan);
+#endif
+
+/****************************************************************************
+ * Name: up_ipcc_init
+ *
+ * Description:
+ * Initializes lower half of the driver. This function should be
+ * implemented by lower half driver in microcontroller code. Function
+ * should not initialize ipcc_lower_s structure with the exception of
+ * explicitly specified fields like setting pointer to read and write
+ * function pointers. Rest of the fields are initialized in upper half
+ * driver.
+ *
+ * This function is called once in ipcc_register() function.
+ *
+ * Input Parameters:
+ * ipcc - interface between upper and lower half drivers. Only some
+ * of the fields should be initialized in this function.
+ * chan - channel number to initialize.
+ *
+ * Returned Value:
+ * 0 on success or negated errno otherwise.
+ *
+ * Assumptions/Limitations:
+ *
+ ****************************************************************************/
+
+int up_ipcc_init(FAR struct ipcc_lower_s *ipcc, int chan);
Review Comment:
Oh wait, but to what should I change it to? I cannot move it to ops struct, it needs to be known name so that upper half can call it so that lower half can initialize that ops struct.
--
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] mlyszczek commented on a diff in pull request #6537: drivers/ipcc: add Inter Processor Communication driver
Posted by GitBox <gi...@apache.org>.
mlyszczek commented on code in PR #6537:
URL: https://github.com/apache/incubator-nuttx/pull/6537#discussion_r923172915
##########
drivers/ipcc/ipcc_register.c:
##########
@@ -0,0 +1,208 @@
+/****************************************************************************
+ * drivers/ipcc/ipcc_register.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/ipcc.h>
+#include <nuttx/kmalloc.h>
+#include <nuttx/mm/circbuf.h>
+
+#include <assert.h>
+#include <errno.h>
+#include <stdbool.h>
+#include <stdio.h>
+#include <sys/types.h>
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/* Device naming ************************************************************/
+
+#define DEVNAME_FMT "/dev/ipcc%d"
+#define DEVNAME_FMTLEN (9 + 3 + 1)
+
+/****************************************************************************
+ * Private Function Prototypes
+ ****************************************************************************/
+
+#ifndef CONFIG_DISABLE_PSEUDOFS_OPERATIONS
+int ipcc_open(FAR struct file *filep);
+int ipcc_close(FAR struct file *filep);
+int ipcc_unlink(FAR struct inode *inode);
+#endif
+int ipcc_ioctl(FAR struct file *filep, int cmd, unsigned long arg);
+ssize_t ipcc_read(FAR struct file *filep, FAR char *buffer, size_t buflen);
+ssize_t ipcc_write(FAR struct file *filep, FAR const char *buffer,
+ size_t buflen);
+int ipcc_poll(FAR struct file *filep, FAR struct pollfd *fds,
+ bool setup);
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+static const struct file_operations ipcc_fops =
+{
+#ifdef CONFIG_DISABLE_PSEUDOFS_OPERATIONS
+ .open = NULL,
+ .close = NULL,
Review Comment:
Got it, will fix it.
--
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] mlyszczek commented on a diff in pull request #6537: drivers/ipcc: add Inter Processor Communication driver
Posted by GitBox <gi...@apache.org>.
mlyszczek commented on code in PR #6537:
URL: https://github.com/apache/incubator-nuttx/pull/6537#discussion_r923171794
##########
include/nuttx/ipcc.h:
##########
@@ -0,0 +1,188 @@
+/****************************************************************************
+ * include/nuttx/ipcc.h
+ *
+ * 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/mm/circbuf.h>
+#include <nuttx/semaphore.h>
+
+#include <poll.h>
+
+/****************************************************************************
+ * Public Types
+ ****************************************************************************/
+
+/* IPCC is a two-part driver:
+ *
+ * 1) A common upper half driver, that provides the character device
+ * interface for the user
+ * 2) Platform-specific lower half drivers that provide interface
+ * between upper and lower layer. Lower half also contains
+ * common objects like semaphore to inform
+ */
+
+/* This structure defines all of the operations provided by the
+ * architecture specific logic. All fields must be provided with
+ * non-NULL function pointers.
+ */
+
+struct ipcc_lower_s;
+struct ipcc_ops_s
+{
+ /* Reads data directly from IPCC mbox, should not block, ever.
+ * If no data is available, 0 should be returned. Partial read
+ * is possible. In that case function will return value less
+ * than buflen.
+ */
+
+ CODE ssize_t (*read)(FAR struct ipcc_lower_s *ipcc,
+ FAR char *buffer, size_t buflen);
+
+ /* Writes data directly to IPCC mbox, should not block, ever.
+ * If IPCC memory is full, or busy or otherwise unavailable for
+ * write, 0 should be returned. Partial write is possible. In
+ * that case function will return value less then buflen.
+ */
+
+ CODE ssize_t (*write)(FAR struct ipcc_lower_s *ipcc,
+ FAR const char *buffer, size_t buflen);
+
+ /* Lower driver should copy data from IPCC to cirbuf on demand.
+ * It does not have to copy all data (in case rxbuf is full), but
+ * it should keep internal pointer to track uncopied data, and copy
+ * what is left on the next call to this function.
+ */
+
+ CODE ssize_t (*buffer_data)(FAR struct ipcc_lower_s *ipcc,
+ FAR struct circbuf_s *rxbuf);
+
+ /* Upper half notifies lower half driver that new data has been
+ * copied to circ buffer
+ */
+
+ CODE ssize_t (*write_notify)(FAR struct ipcc_lower_s *ipcc);
+};
+
+/* This structure defines the interface between upper and lower halves.
+ * Such instance is passed to ipcc_register() function after driver
+ * has been initialized, binding the upper and lower halves into one
+ * driver.
+ */
+
+struct ipcc_lower_s
+{
+ FAR struct pollfd *fds[CONFIG_IPCC_NPOLLWAITERS];
+ FAR struct ipcc_ops_s ops; /* Arch specific functions */
+ sem_t rxsem; /* Data ready to read semaphore */
+ sem_t txsem; /* Data ready to send semaphore */
+ int chan; /* IPCC channel */
+
+#ifdef CONFIG_IPCC_BUFFERED
+ struct circbuf_s rxbuf; /* Receive buffer */
+ struct circbuf_s txbuf; /* Transmit buffer */
+
+ /* 1- not all data could fit into buffer, some unread data is still
+ * in IPCC mbox. State of this variable is maintained by lower half,
+ * upper half only reads it to know when to inform lower driver that
+ * buffer got free.
+ */
+
+ int overflow;
+#endif /* CONFIG_IPCC_BUFFERED */
+};
+
+/* Driver state structure */
+
+struct ipcc_driver_s
+{
+ FAR struct ipcc_lower_s ipcc; /* Lower half driver state */
+ int crefs; /* Count number of open references */
+ int unlinked; /* 1 - driver has been unlinked */
+ sem_t exclsem; /* Mutual exclusion for driver */
+};
+
+/****************************************************************************
+ * Public Functions Prototypes
+ ****************************************************************************/
+
+void ipcc_pollnotify(FAR struct ipcc_lower_s *ipcc, pollevent_t eventset);
+void ipcc_cleanup(FAR struct ipcc_driver_s *priv);
+void ipcc_txfree_notify(FAR struct ipcc_lower_s *priv);
+void ipcc_rxfree_notify(FAR struct ipcc_lower_s *ipcc);
+
+#ifdef CONFIG_IPCC_BUFFERED
+int ipcc_register(int chan, size_t rxbuflen, size_t txbuflen);
+#else
+int ipcc_register(int chan);
+#endif
+
+/****************************************************************************
+ * Name: up_ipcc_init
+ *
+ * Description:
+ * Initializes lower half of the driver. This function should be
+ * implemented by lower half driver in microcontroller code. Function
+ * should not initialize ipcc_lower_s structure with the exception of
+ * explicitly specified fields like setting pointer to read and write
+ * function pointers. Rest of the fields are initialized in upper half
+ * driver.
+ *
+ * This function is called once in ipcc_register() function.
+ *
+ * Input Parameters:
+ * ipcc - interface between upper and lower half drivers. Only some
+ * of the fields should be initialized in this function.
+ * chan - channel number to initialize.
+ *
+ * Returned Value:
+ * 0 on success or negated errno otherwise.
+ *
+ * Assumptions/Limitations:
+ *
+ ****************************************************************************/
+
+int up_ipcc_init(FAR struct ipcc_lower_s *ipcc, int chan);
Review Comment:
ipcc_lower_init() maybe? and ipcc_lower_cleanup() for cleanup function?
--
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 diff in pull request #6537: drivers/ipcc: add Inter Processor Communication driver
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #6537:
URL: https://github.com/apache/incubator-nuttx/pull/6537#discussion_r923169219
##########
drivers/ipcc/ipcc_register.c:
##########
@@ -0,0 +1,208 @@
+/****************************************************************************
+ * drivers/ipcc/ipcc_register.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/ipcc.h>
+#include <nuttx/kmalloc.h>
+#include <nuttx/mm/circbuf.h>
+
+#include <assert.h>
+#include <errno.h>
+#include <stdbool.h>
+#include <stdio.h>
+#include <sys/types.h>
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/* Device naming ************************************************************/
+
+#define DEVNAME_FMT "/dev/ipcc%d"
+#define DEVNAME_FMTLEN (9 + 3 + 1)
+
+/****************************************************************************
+ * Private Function Prototypes
+ ****************************************************************************/
+
+#ifndef CONFIG_DISABLE_PSEUDOFS_OPERATIONS
+int ipcc_open(FAR struct file *filep);
+int ipcc_close(FAR struct file *filep);
+int ipcc_unlink(FAR struct inode *inode);
+#endif
+int ipcc_ioctl(FAR struct file *filep, int cmd, unsigned long arg);
+ssize_t ipcc_read(FAR struct file *filep, FAR char *buffer, size_t buflen);
+ssize_t ipcc_write(FAR struct file *filep, FAR const char *buffer,
+ size_t buflen);
+int ipcc_poll(FAR struct file *filep, FAR struct pollfd *fds,
+ bool setup);
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+static const struct file_operations ipcc_fops =
+{
+#ifdef CONFIG_DISABLE_PSEUDOFS_OPERATIONS
+ .open = NULL,
+ .close = NULL,
Review Comment:
That's because i2c is a bus driver and most user call i2c_ops directly from other kernel driver, the file interface(/dev/i2cxx) is used for debugging: https://github.com/apache/incubator-nuttx-apps/tree/master/system/i2c.
I don't see ipcc kernel interface, so I assume the client of ipcc always come from userspace and then open/close isn't optonal.
--
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] mlyszczek commented on a diff in pull request #6537: drivers/ipcc: add Inter Processor Communication driver
Posted by GitBox <gi...@apache.org>.
mlyszczek commented on code in PR #6537:
URL: https://github.com/apache/incubator-nuttx/pull/6537#discussion_r923173997
##########
drivers/ipcc/ipcc_register.c:
##########
@@ -0,0 +1,208 @@
+/****************************************************************************
+ * drivers/ipcc/ipcc_register.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/ipcc.h>
+#include <nuttx/kmalloc.h>
+#include <nuttx/mm/circbuf.h>
+
+#include <assert.h>
+#include <errno.h>
+#include <stdbool.h>
+#include <stdio.h>
+#include <sys/types.h>
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/* Device naming ************************************************************/
+
+#define DEVNAME_FMT "/dev/ipcc%d"
+#define DEVNAME_FMTLEN (9 + 3 + 1)
+
+/****************************************************************************
+ * Private Function Prototypes
+ ****************************************************************************/
+
+#ifndef CONFIG_DISABLE_PSEUDOFS_OPERATIONS
+int ipcc_open(FAR struct file *filep);
+int ipcc_close(FAR struct file *filep);
+int ipcc_unlink(FAR struct inode *inode);
+#endif
+int ipcc_ioctl(FAR struct file *filep, int cmd, unsigned long arg);
+ssize_t ipcc_read(FAR struct file *filep, FAR char *buffer, size_t buflen);
+ssize_t ipcc_write(FAR struct file *filep, FAR const char *buffer,
+ size_t buflen);
+int ipcc_poll(FAR struct file *filep, FAR struct pollfd *fds,
+ bool setup);
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+static const struct file_operations ipcc_fops =
+{
+#ifdef CONFIG_DISABLE_PSEUDOFS_OPERATIONS
+ .open = NULL,
+ .close = NULL,
+ .unlink = NULL,
+#else /* CONFIG_DISABLE_PSEUDOFS_OPERATIONS */
+ .open = ipcc_open,
+ .close = ipcc_close,
+ .unlink = ipcc_unlink,
+#endif /* CONFIG_DISABLE_PSEUDOFS_OPERATIONS */
+ .poll = ipcc_poll,
+ .read = ipcc_read,
+ .write = ipcc_write,
+ .ioctl = NULL,
+ .seek = NULL
+};
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: ipcc_cleanup
+ *
+ * Description:
+ * Cleans up resources allocated by ipcc_register()
+ *
+ * Input Parameters:
+ * priv - ipcc driver instance to clean up
+ *
+ * Returned Value:
+ * None
+ *
+ * Assumptions/Limitations:
+ * This function should be called only when ipcc_register is run with
+ * success and all resources in driver instance are properly allocated.
+ *
+ ****************************************************************************/
+
+void ipcc_cleanup(FAR struct ipcc_driver_s *priv)
+{
+#ifdef CONFIG_IPCC_BUFFERED
+ circbuf_uninit(&priv->ipcc.rxbuf);
+ circbuf_uninit(&priv->ipcc.txbuf);
+#endif
+ nxsem_destroy(&priv->ipcc.rxsem);
+ nxsem_destroy(&priv->ipcc.txsem);
+ up_ipcc_cleanup(&priv->ipcc);
+ nxsem_destroy(&priv->exclsem);
+ kmm_free(priv);
+}
+
+/****************************************************************************
+ * Name: ipcc_register
+ *
+ * Description:
+ * Create and register the IPCC character driver.
+ *
+ * IPCC is a simple character driver that supports inter processor
+ * communication.
+ *
+ * Input Parameters:
+ * ipcc - An instance of the lower half IPCC driver
+ * chan - IPCC channel. This will be used ad IPCC minor number.
+ * IPPC will be registered as /dev/ipccN where N is the minor number.
+ * buflen - Length of tx and rx buffers, 0 for unbuffered communication.
+ *
+ * Returned Value:
+ * OK if the driver was successfully registered, or negated errno on
+ * failure.
+ *
+ * Assumptions/Limitations:
+ *
+ ****************************************************************************/
+
+#ifdef CONFIG_IPCC_BUFFERED
+int ipcc_register(int chan, size_t rxbuflen, size_t txbuflen)
+#else
+int ipcc_register(int chan)
+#endif
+{
+ FAR struct ipcc_driver_s *priv;
+ char devname[DEVNAME_FMTLEN];
+ int ret;
+
+ /* Allocate a IPCC character device structure */
+
+ if ((priv = kmm_zalloc(sizeof(*priv))) == NULL)
+ {
+ return -ENOMEM;
+ }
+
+ if ((ret = up_ipcc_init(&priv->ipcc, chan)))
Review Comment:
No, up_ipcc_init() is supposed to be implemented in arch/ not board/. User only has to call ipcc_register(IPCC_CHANNEL_NUMBER) and that's all.
--
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] onegray commented on pull request #6537: drivers/ipcc: add Inter Processor Communication driver
Posted by GitBox <gi...@apache.org>.
onegray commented on PR #6537:
URL: https://github.com/apache/incubator-nuttx/pull/6537#issuecomment-1187760977
Great to see the working way how the IPCC communication could be done! This is definitely can be useful in some cases, especially if we need to transfer some meaning amount of data. But also I feel the proposed implementation would not be the most efficient way for the small events oriented protocol, like some wireless are. Small packets can be transferred by simple 'place & get' logic, without streaming data via serial-like interface. I do not detract the proposed solution, but may be we could consider to call it in a more specific way, like ipcc-serial driver (or similar)?
--
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] mlyszczek commented on a diff in pull request #6537: drivers/ipcc: add Inter Processor Communication driver
Posted by GitBox <gi...@apache.org>.
mlyszczek commented on code in PR #6537:
URL: https://github.com/apache/incubator-nuttx/pull/6537#discussion_r928981092
##########
include/nuttx/ipcc.h:
##########
@@ -0,0 +1,212 @@
+/****************************************************************************
+ * include/nuttx/ipcc.h
+ *
+ * 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.
+ *
+ ****************************************************************************/
+
+#ifndef __INCLUDE_NUTTX_IPCC_H
+#define __INCLUDE_NUTTX_IPCC_H
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+#include <nuttx/mm/circbuf.h>
+#include <nuttx/semaphore.h>
+
+#include <poll.h>
+
+/****************************************************************************
+ * Public Types
+ ****************************************************************************/
+
+/* IPCC is a two-part driver:
+ *
+ * 1) A common upper half driver, that provides the character device
+ * interface for the user
+ * 2) Platform-specific lower half drivers that provide interface
+ * between upper and lower layer. Lower half also contains
+ * common objects like semaphore to inform
+ */
+
+/* This structure defines all of the operations provided by the
+ * architecture specific logic. All fields must be provided with
+ * non-NULL function pointers.
+ */
+
+struct ipcc_lower_s;
+struct ipcc_ops_s
+{
+ /**************************************************************************
+ * Name: read
+ *
+ * Description:
+ * Reads data directly from IPCC mbox, function shall not block.
+ * If no data is available, 0 shall be returned. Partial read
+ * is possible. In that case function can return value less
+ * than buflen.
+ *
+ * Input Parameters:
+ * ipcc - ipcc channel to read data from
+ * buffer - location where data shall be stored
+ * buflen - size of the buffer
+ *
+ * Returned Value:
+ * Number of bytes actually written to buffer.
+ *
+ **************************************************************************/
+
+ CODE ssize_t (*read)(FAR struct ipcc_lower_s *ipcc,
+ FAR char *buffer, size_t buflen);
+
+ /**************************************************************************
+ * Name: write
+ *
+ * Description:
+ * Writes data directly to IPCC mbox, function shall not block.
+ * If IPCC memory is full, or busy or otherwise unavailable for
+ * write, 0 shall be returned. Partial write is possible. In
+ * that case function can return value less then buflen.
+ *
+ * Input Parameters:
+ * ipcc - ipcc channel to write data to
+ * buffer - location to read data from
+ * buflen - number of bytes to write from buffer
+ *
+ * Returned Value:
+ * Number of actually written data to IPCC channel.
+ *
+ **************************************************************************/
+
+ CODE ssize_t (*write)(FAR struct ipcc_lower_s *ipcc,
+ FAR const char *buffer, size_t buflen);
+
+ /**************************************************************************
+ * Name: buffer_data
+ *
+ * Description:
+ * Lower driver shall copy data from IPCC to cirbuf on demand.
+ * It does not have to copy all data (in case rxbuf is full), but
+ * it must keep internal pointer to track uncopied data, and copy
+ * what is left on the next call to this function.
+ *
+ * Input Parameters:
+ * ipcc - channel to buffer data
+ * rxbuf - circural buffer where data should be written to
+ *
+ * Returned Value:
+ * Number of bytes that were successfully written to rxbuf.
+ *
+ **************************************************************************/
+
+ CODE ssize_t (*buffer_data)(FAR struct ipcc_lower_s *ipcc,
+ FAR struct circbuf_s *rxbuf);
+
+ /**************************************************************************
+ * Name: write_notify
+ *
+ * Description:
+ * Upper half driver notifies lower half driver that new data has been
+ * copied to circ buffer. This can be used by lower half to transfer
+ * data from buffer to IPCC memory.
+ *
+ * Input Parameters:
+ * ipcc - ipcc channel that wrote data.
+ *
+ * Returned Value:
+ * 0 on success, or negated errno otherwise
+ *
+ **************************************************************************/
+
+ CODE ssize_t (*write_notify)(FAR struct ipcc_lower_s *ipcc);
+
+ /**************************************************************************
+ * Name: cleanup
+ *
+ * Description:
+ * Cleans up resources initialized by <arch>_ipcc_init(). If arch code
+ * malloc()ed any memory, this funciton is responsible of freeing it.
+ * After this function is called, ipcc driver will not access lower
+ * half pointer anymore.
+ *
+ * Input Parameters:
+ * ipcc - ipcc channel to cleanup
+ *
+ * Returned Value:
+ * Always OK
+ *
+ **************************************************************************/
+
+ CODE int (*cleanup)(FAR struct ipcc_lower_s *ipcc);
+};
+
+/* This structure defines the interface between upper and lower halves.
+ * Such instance is passed to ipcc_register() function after driver
+ * has been initialized, binding the upper and lower halves into one
+ * driver.
+ */
+
+struct ipcc_lower_s
+{
+ FAR struct pollfd *fds[CONFIG_IPCC_NPOLLWAITERS];
Review Comment:
In that case all of these fields are used by both upper and lower half.
Buffers are directly used in lower half. fds and notify semaphores are used indirectly, that is lower half calls functions from upper half which accept ipcc_lower_s. As I understand you want these upper half functions to accept ipcc_driver_s instead? In that case I would have to add pointer to ipcc_driver_s to ipcc_lower_s, thus exposing upper driver fields to lower half.
But I guess it does make sense. And since all of these are internal APIs anyway, we could "restrict" access to upper half from lower half by comment:)
--
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 pull request #6537: [API review request] drivers/ipcc: add Inter Processor Communication driver
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on PR #6537:
URL: https://github.com/apache/incubator-nuttx/pull/6537#issuecomment-1168914571
> I apologize but I am not really sure I understand the question.
>
> I need IPCC to communicate 2 CPUs on stm32wl5. It's a trivial controller, software is responsible to reserve memory, and hardware simply send iterrupts when software says "I copied data from read buffer - you can put new data there" or "I left you something in a memory, go check it out". I did not work with other inter processor communication controllers but I image it will not be that different and I tried to keep this upper half as generic as possible.
Ok, stm32wl5 doesn't create a very special hardware for IPC(just share memory plus software interrupt).
I would suggest you look at OpenAMP(RTOS side) and remoteproc/rpmsg(Linux side):
https://static.linaro.org/connect/hkg18/presentations/hkg18-411.pdf
https://www.kernel.org/doc/Documentation/remoteproc.txt
https://www.kernel.org/doc/Documentation/rpmsg.txt
NuttX fully incorporate OpenAMP stack and provide many essential service on top of them:
https://cwiki.apache.org/confluence/display/NUTTX/NuttX+And+OpenAMP
Actually, ST also promote OpenAMP framework on their hardware:
https://elinux.org/images/6/63/ELC_EU19_rpmsg.pdf
--
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] acassis commented on pull request #6537: [API review request] drivers/ipcc: add Inter Processor Communication driver
Posted by GitBox <gi...@apache.org>.
acassis commented on PR #6537:
URL: https://github.com/apache/incubator-nuttx/pull/6537#issuecomment-1171635063
@xiaoxiang781216 did you see that: https://www.redhat.com/en/blog/deep-dive-virtio-networking-and-vhost-net ?
So I'll include eventfd as another IPC supported on Linux (and NuttX)
--
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] mlyszczek commented on pull request #6537: drivers/ipcc: add Inter Processor Communication driver
Posted by GitBox <gi...@apache.org>.
mlyszczek commented on PR #6537:
URL: https://github.com/apache/incubator-nuttx/pull/6537#issuecomment-1197219952
@acassis yes, as far as I am concerned this can be merged. It's still marked as CONFIG_EXPERIMENTAL as you, rightfully suggested, because I belive there are still bugs lurking there - which I will be fixing while using this driver in my project.
I meant for this ipcc driver to be as flexible as possible and as unixy as possible, performance was not my first goal. I belive zero copy driver can be implemented using ioctl(), but again, that was not my goal here.
If it turns out that this driver is a bottleneck in my application, I will probably investigate other options:)
--
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] mlyszczek commented on a diff in pull request #6537: drivers/ipcc: add Inter Processor Communication driver
Posted by GitBox <gi...@apache.org>.
mlyszczek commented on code in PR #6537:
URL: https://github.com/apache/incubator-nuttx/pull/6537#discussion_r932070668
##########
drivers/ipcc/ipcc_register.c:
##########
@@ -0,0 +1,205 @@
+/****************************************************************************
+ * drivers/ipcc/ipcc_register.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/ipcc.h>
+#include <nuttx/kmalloc.h>
+#include <nuttx/mm/circbuf.h>
+
+#include <assert.h>
+#include <errno.h>
+#include <stdbool.h>
+#include <stdio.h>
+#include <sys/types.h>
+
+#include "ipcc_priv.h"
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/* Device naming ************************************************************/
+
+#define DEVNAME_FMT "/dev/ipcc%d"
+#define DEVNAME_FMTLEN (9 + 3 + 1)
+
+/****************************************************************************
+ * Private Function Prototypes
+ ****************************************************************************/
+
+int ipcc_open(FAR struct file *filep);
+int ipcc_close(FAR struct file *filep);
+#ifndef CONFIG_DISABLE_PSEUDOFS_OPERATIONS
+int ipcc_unlink(FAR struct inode *inode);
+#endif
+int ipcc_ioctl(FAR struct file *filep, int cmd, unsigned long arg);
Review Comment:
done
--
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 diff in pull request #6537: drivers/ipcc: add Inter Processor Communication driver
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #6537:
URL: https://github.com/apache/incubator-nuttx/pull/6537#discussion_r922983914
##########
drivers/ipcc/Make.defs:
##########
@@ -0,0 +1,41 @@
+############################################################################
+# drivers/ipcc/Make.defs
+#
+# 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.
+#
+############################################################################
+
+ifeq ($(CONFIG_IPCC),y)
+
+# Include ipcc driver
+
+CSRCS += ipcc_poll.c
+CSRCS += ipcc_read.c
+CSRCS += ipcc_write.c
+CSRCS += ipcc_register.c
+
+ifneq ($(CONFIG_DISABLE_PSEUDOFS_OPERATIONS),y)
+CSRCS += ipcc_open.c
+CSRCS += ipcc_close.c
Review Comment:
open/close is always required?
##########
include/nuttx/ipcc.h:
##########
@@ -0,0 +1,188 @@
+/****************************************************************************
+ * include/nuttx/ipcc.h
+ *
+ * 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/mm/circbuf.h>
+#include <nuttx/semaphore.h>
+
+#include <poll.h>
+
+/****************************************************************************
+ * Public Types
+ ****************************************************************************/
+
+/* IPCC is a two-part driver:
+ *
+ * 1) A common upper half driver, that provides the character device
+ * interface for the user
+ * 2) Platform-specific lower half drivers that provide interface
+ * between upper and lower layer. Lower half also contains
+ * common objects like semaphore to inform
+ */
+
+/* This structure defines all of the operations provided by the
+ * architecture specific logic. All fields must be provided with
+ * non-NULL function pointers.
+ */
+
+struct ipcc_lower_s;
+struct ipcc_ops_s
+{
+ /* Reads data directly from IPCC mbox, should not block, ever.
+ * If no data is available, 0 should be returned. Partial read
+ * is possible. In that case function will return value less
+ * than buflen.
+ */
+
+ CODE ssize_t (*read)(FAR struct ipcc_lower_s *ipcc,
+ FAR char *buffer, size_t buflen);
+
+ /* Writes data directly to IPCC mbox, should not block, ever.
+ * If IPCC memory is full, or busy or otherwise unavailable for
+ * write, 0 should be returned. Partial write is possible. In
+ * that case function will return value less then buflen.
+ */
+
+ CODE ssize_t (*write)(FAR struct ipcc_lower_s *ipcc,
+ FAR const char *buffer, size_t buflen);
+
+ /* Lower driver should copy data from IPCC to cirbuf on demand.
+ * It does not have to copy all data (in case rxbuf is full), but
+ * it should keep internal pointer to track uncopied data, and copy
+ * what is left on the next call to this function.
+ */
+
+ CODE ssize_t (*buffer_data)(FAR struct ipcc_lower_s *ipcc,
+ FAR struct circbuf_s *rxbuf);
+
+ /* Upper half notifies lower half driver that new data has been
+ * copied to circ buffer
+ */
+
+ CODE ssize_t (*write_notify)(FAR struct ipcc_lower_s *ipcc);
+};
+
+/* This structure defines the interface between upper and lower halves.
+ * Such instance is passed to ipcc_register() function after driver
+ * has been initialized, binding the upper and lower halves into one
+ * driver.
+ */
+
+struct ipcc_lower_s
+{
+ FAR struct pollfd *fds[CONFIG_IPCC_NPOLLWAITERS];
+ FAR struct ipcc_ops_s ops; /* Arch specific functions */
+ sem_t rxsem; /* Data ready to read semaphore */
+ sem_t txsem; /* Data ready to send semaphore */
+ int chan; /* IPCC channel */
+
+#ifdef CONFIG_IPCC_BUFFERED
+ struct circbuf_s rxbuf; /* Receive buffer */
+ struct circbuf_s txbuf; /* Transmit buffer */
+
+ /* 1- not all data could fit into buffer, some unread data is still
+ * in IPCC mbox. State of this variable is maintained by lower half,
+ * upper half only reads it to know when to inform lower driver that
+ * buffer got free.
+ */
+
+ int overflow;
+#endif /* CONFIG_IPCC_BUFFERED */
+};
+
+/* Driver state structure */
+
+struct ipcc_driver_s
+{
+ FAR struct ipcc_lower_s ipcc; /* Lower half driver state */
+ int crefs; /* Count number of open references */
+ int unlinked; /* 1 - driver has been unlinked */
+ sem_t exclsem; /* Mutual exclusion for driver */
+};
+
+/****************************************************************************
+ * Public Functions Prototypes
+ ****************************************************************************/
+
+void ipcc_pollnotify(FAR struct ipcc_lower_s *ipcc, pollevent_t eventset);
+void ipcc_cleanup(FAR struct ipcc_driver_s *priv);
+void ipcc_txfree_notify(FAR struct ipcc_lower_s *priv);
+void ipcc_rxfree_notify(FAR struct ipcc_lower_s *ipcc);
+
+#ifdef CONFIG_IPCC_BUFFERED
+int ipcc_register(int chan, size_t rxbuflen, size_t txbuflen);
+#else
+int ipcc_register(int chan);
+#endif
+
+/****************************************************************************
+ * Name: up_ipcc_init
+ *
+ * Description:
+ * Initializes lower half of the driver. This function should be
+ * implemented by lower half driver in microcontroller code. Function
+ * should not initialize ipcc_lower_s structure with the exception of
+ * explicitly specified fields like setting pointer to read and write
+ * function pointers. Rest of the fields are initialized in upper half
+ * driver.
+ *
+ * This function is called once in ipcc_register() function.
+ *
+ * Input Parameters:
+ * ipcc - interface between upper and lower half drivers. Only some
+ * of the fields should be initialized in this function.
+ * chan - channel number to initialize.
+ *
+ * Returned Value:
+ * 0 on success or negated errno otherwise.
+ *
+ * Assumptions/Limitations:
+ *
+ ****************************************************************************/
+
+int up_ipcc_init(FAR struct ipcc_lower_s *ipcc, int chan);
Review Comment:
up_xxx is reserved to the core os interact with the arch/chip specific code, it isn't good to use up api for the normal driver communication.
##########
drivers/ipcc/ipcc_unlink.c:
##########
@@ -0,0 +1,94 @@
+/****************************************************************************
+ * drivers/ipcc/ipcc_unlink.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/ipcc.h>
+#include <nuttx/kmalloc.h>
+#include <nuttx/semaphore.h>
+
+#include <assert.h>
+#include <errno.h>
+#include <stdbool.h>
+#include <stdio.h>
+#include <sys/types.h>
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: ipcc_unlink
+ *
+ * Description:
+ * Action to take upon file unlinking. Function will free resources if
+ * noone is using the driver when unlinking occured. If driver is still
+ * in use, it will be marked as unlinked and resource freeing will take
+ * place in ipcc_close() function instead, once last reference is closed.
+ *
+ * Input Parameters:
+ * inode - driver inode that is being unlinked
+ *
+ * Returned Value:
+ * OK on successfull close, or negated errno on failure.
+ *
+ * Assumptions/Limitations:
+ *
+ ****************************************************************************/
+
+int ipcc_unlink(FAR struct inode *inode)
Review Comment:
since the IPC existence is determined statically, it doesn't make sense to let user unlink it.
##########
drivers/ipcc/ipcc_register.c:
##########
@@ -0,0 +1,208 @@
+/****************************************************************************
+ * drivers/ipcc/ipcc_register.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/ipcc.h>
+#include <nuttx/kmalloc.h>
+#include <nuttx/mm/circbuf.h>
+
+#include <assert.h>
+#include <errno.h>
+#include <stdbool.h>
+#include <stdio.h>
+#include <sys/types.h>
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/* Device naming ************************************************************/
+
+#define DEVNAME_FMT "/dev/ipcc%d"
+#define DEVNAME_FMTLEN (9 + 3 + 1)
+
+/****************************************************************************
+ * Private Function Prototypes
+ ****************************************************************************/
+
+#ifndef CONFIG_DISABLE_PSEUDOFS_OPERATIONS
+int ipcc_open(FAR struct file *filep);
+int ipcc_close(FAR struct file *filep);
+int ipcc_unlink(FAR struct inode *inode);
+#endif
+int ipcc_ioctl(FAR struct file *filep, int cmd, unsigned long arg);
+ssize_t ipcc_read(FAR struct file *filep, FAR char *buffer, size_t buflen);
+ssize_t ipcc_write(FAR struct file *filep, FAR const char *buffer,
+ size_t buflen);
+int ipcc_poll(FAR struct file *filep, FAR struct pollfd *fds,
+ bool setup);
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+static const struct file_operations ipcc_fops =
+{
+#ifdef CONFIG_DISABLE_PSEUDOFS_OPERATIONS
+ .open = NULL,
+ .close = NULL,
+ .unlink = NULL,
+#else /* CONFIG_DISABLE_PSEUDOFS_OPERATIONS */
+ .open = ipcc_open,
+ .close = ipcc_close,
+ .unlink = ipcc_unlink,
+#endif /* CONFIG_DISABLE_PSEUDOFS_OPERATIONS */
+ .poll = ipcc_poll,
+ .read = ipcc_read,
+ .write = ipcc_write,
+ .ioctl = NULL,
+ .seek = NULL
+};
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: ipcc_cleanup
+ *
+ * Description:
+ * Cleans up resources allocated by ipcc_register()
+ *
+ * Input Parameters:
+ * priv - ipcc driver instance to clean up
+ *
+ * Returned Value:
+ * None
+ *
+ * Assumptions/Limitations:
+ * This function should be called only when ipcc_register is run with
+ * success and all resources in driver instance are properly allocated.
+ *
+ ****************************************************************************/
+
+void ipcc_cleanup(FAR struct ipcc_driver_s *priv)
+{
+#ifdef CONFIG_IPCC_BUFFERED
+ circbuf_uninit(&priv->ipcc.rxbuf);
+ circbuf_uninit(&priv->ipcc.txbuf);
+#endif
+ nxsem_destroy(&priv->ipcc.rxsem);
+ nxsem_destroy(&priv->ipcc.txsem);
+ up_ipcc_cleanup(&priv->ipcc);
+ nxsem_destroy(&priv->exclsem);
+ kmm_free(priv);
+}
+
+/****************************************************************************
+ * Name: ipcc_register
+ *
+ * Description:
+ * Create and register the IPCC character driver.
+ *
+ * IPCC is a simple character driver that supports inter processor
+ * communication.
+ *
+ * Input Parameters:
+ * ipcc - An instance of the lower half IPCC driver
+ * chan - IPCC channel. This will be used ad IPCC minor number.
+ * IPPC will be registered as /dev/ipccN where N is the minor number.
+ * buflen - Length of tx and rx buffers, 0 for unbuffered communication.
+ *
+ * Returned Value:
+ * OK if the driver was successfully registered, or negated errno on
+ * failure.
+ *
+ * Assumptions/Limitations:
+ *
+ ****************************************************************************/
+
+#ifdef CONFIG_IPCC_BUFFERED
+int ipcc_register(int chan, size_t rxbuflen, size_t txbuflen)
+#else
+int ipcc_register(int chan)
+#endif
+{
+ FAR struct ipcc_driver_s *priv;
+ char devname[DEVNAME_FMTLEN];
+ int ret;
+
+ /* Allocate a IPCC character device structure */
+
+ if ((priv = kmm_zalloc(sizeof(*priv))) == NULL)
+ {
+ return -ENOMEM;
+ }
+
+ if ((ret = up_ipcc_init(&priv->ipcc, chan)))
Review Comment:
why not let the implementor pass ipcc_driver_s directly to ipcc_register?
##########
include/nuttx/ipcc.h:
##########
@@ -0,0 +1,188 @@
+/****************************************************************************
+ * include/nuttx/ipcc.h
+ *
+ * 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/mm/circbuf.h>
+#include <nuttx/semaphore.h>
+
+#include <poll.h>
+
+/****************************************************************************
+ * Public Types
+ ****************************************************************************/
+
+/* IPCC is a two-part driver:
+ *
+ * 1) A common upper half driver, that provides the character device
+ * interface for the user
+ * 2) Platform-specific lower half drivers that provide interface
+ * between upper and lower layer. Lower half also contains
+ * common objects like semaphore to inform
+ */
+
+/* This structure defines all of the operations provided by the
+ * architecture specific logic. All fields must be provided with
+ * non-NULL function pointers.
+ */
+
+struct ipcc_lower_s;
+struct ipcc_ops_s
+{
+ /* Reads data directly from IPCC mbox, should not block, ever.
+ * If no data is available, 0 should be returned. Partial read
+ * is possible. In that case function will return value less
+ * than buflen.
+ */
+
+ CODE ssize_t (*read)(FAR struct ipcc_lower_s *ipcc,
+ FAR char *buffer, size_t buflen);
+
+ /* Writes data directly to IPCC mbox, should not block, ever.
+ * If IPCC memory is full, or busy or otherwise unavailable for
+ * write, 0 should be returned. Partial write is possible. In
+ * that case function will return value less then buflen.
+ */
+
+ CODE ssize_t (*write)(FAR struct ipcc_lower_s *ipcc,
+ FAR const char *buffer, size_t buflen);
+
+ /* Lower driver should copy data from IPCC to cirbuf on demand.
+ * It does not have to copy all data (in case rxbuf is full), but
+ * it should keep internal pointer to track uncopied data, and copy
+ * what is left on the next call to this function.
+ */
+
+ CODE ssize_t (*buffer_data)(FAR struct ipcc_lower_s *ipcc,
+ FAR struct circbuf_s *rxbuf);
+
+ /* Upper half notifies lower half driver that new data has been
+ * copied to circ buffer
+ */
+
+ CODE ssize_t (*write_notify)(FAR struct ipcc_lower_s *ipcc);
+};
+
+/* This structure defines the interface between upper and lower halves.
+ * Such instance is passed to ipcc_register() function after driver
+ * has been initialized, binding the upper and lower halves into one
+ * driver.
+ */
+
+struct ipcc_lower_s
+{
+ FAR struct pollfd *fds[CONFIG_IPCC_NPOLLWAITERS];
+ FAR struct ipcc_ops_s ops; /* Arch specific functions */
+ sem_t rxsem; /* Data ready to read semaphore */
+ sem_t txsem; /* Data ready to send semaphore */
+ int chan; /* IPCC channel */
+
+#ifdef CONFIG_IPCC_BUFFERED
+ struct circbuf_s rxbuf; /* Receive buffer */
+ struct circbuf_s txbuf; /* Transmit buffer */
+
+ /* 1- not all data could fit into buffer, some unread data is still
+ * in IPCC mbox. State of this variable is maintained by lower half,
+ * upper half only reads it to know when to inform lower driver that
+ * buffer got free.
+ */
+
+ int overflow;
+#endif /* CONFIG_IPCC_BUFFERED */
+};
+
+/* Driver state structure */
+
+struct ipcc_driver_s
+{
+ FAR struct ipcc_lower_s ipcc; /* Lower half driver state */
+ int crefs; /* Count number of open references */
+ int unlinked; /* 1 - driver has been unlinked */
+ sem_t exclsem; /* Mutual exclusion for driver */
+};
+
+/****************************************************************************
+ * Public Functions Prototypes
+ ****************************************************************************/
+
+void ipcc_pollnotify(FAR struct ipcc_lower_s *ipcc, pollevent_t eventset);
+void ipcc_cleanup(FAR struct ipcc_driver_s *priv);
+void ipcc_txfree_notify(FAR struct ipcc_lower_s *priv);
+void ipcc_rxfree_notify(FAR struct ipcc_lower_s *ipcc);
+
+#ifdef CONFIG_IPCC_BUFFERED
+int ipcc_register(int chan, size_t rxbuflen, size_t txbuflen);
+#else
+int ipcc_register(int chan);
+#endif
+
+/****************************************************************************
+ * Name: up_ipcc_init
+ *
+ * Description:
+ * Initializes lower half of the driver. This function should be
+ * implemented by lower half driver in microcontroller code. Function
+ * should not initialize ipcc_lower_s structure with the exception of
+ * explicitly specified fields like setting pointer to read and write
+ * function pointers. Rest of the fields are initialized in upper half
+ * driver.
+ *
+ * This function is called once in ipcc_register() function.
+ *
+ * Input Parameters:
+ * ipcc - interface between upper and lower half drivers. Only some
+ * of the fields should be initialized in this function.
+ * chan - channel number to initialize.
+ *
+ * Returned Value:
+ * 0 on success or negated errno otherwise.
+ *
+ * Assumptions/Limitations:
+ *
+ ****************************************************************************/
+
+int up_ipcc_init(FAR struct ipcc_lower_s *ipcc, int chan);
+
+/****************************************************************************
+ * Name: up_ipcc_cleanup
+ *
+ * Description:
+ * Cleans up lower half of the driver. This function should be
+ * implemented by lower half driver in microcontroller code. Function
+ * should not deinitialize anyting in ipcc structure - upper half driver
+ * will handle that.
+ *
+ * Function is called once driver has been unlinked and all descriptors
+ * to ipcc instance are closed.
+ *
+ * Input Parameters:
+ * ipcc - driver instance that is being cleaned up.
+ *
+ * Returned Value:
+ * 0 on success or negated errno otherwise.
+ *
+ * Assumptions/Limitations:
+ *
+ ****************************************************************************/
+
+int up_ipcc_cleanup(FAR struct ipcc_lower_s *ipcc);
Review Comment:
let's change to a callback in ipcc_lower_s instead
##########
drivers/ipcc/ipcc_register.c:
##########
@@ -0,0 +1,208 @@
+/****************************************************************************
+ * drivers/ipcc/ipcc_register.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/ipcc.h>
+#include <nuttx/kmalloc.h>
+#include <nuttx/mm/circbuf.h>
+
+#include <assert.h>
+#include <errno.h>
+#include <stdbool.h>
+#include <stdio.h>
+#include <sys/types.h>
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/* Device naming ************************************************************/
+
+#define DEVNAME_FMT "/dev/ipcc%d"
+#define DEVNAME_FMTLEN (9 + 3 + 1)
+
+/****************************************************************************
+ * Private Function Prototypes
+ ****************************************************************************/
+
+#ifndef CONFIG_DISABLE_PSEUDOFS_OPERATIONS
+int ipcc_open(FAR struct file *filep);
Review Comment:
why not implement ipcc_xxx in one file like other drivers?
##########
drivers/ipcc/ipcc_register.c:
##########
@@ -0,0 +1,208 @@
+/****************************************************************************
+ * drivers/ipcc/ipcc_register.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/ipcc.h>
+#include <nuttx/kmalloc.h>
+#include <nuttx/mm/circbuf.h>
+
+#include <assert.h>
+#include <errno.h>
+#include <stdbool.h>
+#include <stdio.h>
+#include <sys/types.h>
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/* Device naming ************************************************************/
+
+#define DEVNAME_FMT "/dev/ipcc%d"
+#define DEVNAME_FMTLEN (9 + 3 + 1)
+
+/****************************************************************************
+ * Private Function Prototypes
+ ****************************************************************************/
+
+#ifndef CONFIG_DISABLE_PSEUDOFS_OPERATIONS
+int ipcc_open(FAR struct file *filep);
+int ipcc_close(FAR struct file *filep);
+int ipcc_unlink(FAR struct inode *inode);
+#endif
+int ipcc_ioctl(FAR struct file *filep, int cmd, unsigned long arg);
+ssize_t ipcc_read(FAR struct file *filep, FAR char *buffer, size_t buflen);
+ssize_t ipcc_write(FAR struct file *filep, FAR const char *buffer,
+ size_t buflen);
+int ipcc_poll(FAR struct file *filep, FAR struct pollfd *fds,
+ bool setup);
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+static const struct file_operations ipcc_fops =
+{
+#ifdef CONFIG_DISABLE_PSEUDOFS_OPERATIONS
+ .open = NULL,
+ .close = NULL,
Review Comment:
open/close must always implement, only unlink is optional.
--
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 diff in pull request #6537: drivers/ipcc: add Inter Processor Communication driver
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #6537:
URL: https://github.com/apache/incubator-nuttx/pull/6537#discussion_r923175056
##########
drivers/ipcc/ipcc_unlink.c:
##########
@@ -0,0 +1,94 @@
+/****************************************************************************
+ * drivers/ipcc/ipcc_unlink.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/ipcc.h>
+#include <nuttx/kmalloc.h>
+#include <nuttx/semaphore.h>
+
+#include <assert.h>
+#include <errno.h>
+#include <stdbool.h>
+#include <stdio.h>
+#include <sys/types.h>
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: ipcc_unlink
+ *
+ * Description:
+ * Action to take upon file unlinking. Function will free resources if
+ * noone is using the driver when unlinking occured. If driver is still
+ * in use, it will be marked as unlinked and resource freeing will take
+ * place in ipcc_close() function instead, once last reference is closed.
+ *
+ * Input Parameters:
+ * inode - driver inode that is being unlinked
+ *
+ * Returned Value:
+ * OK on successfull close, or negated errno on failure.
+ *
+ * Assumptions/Limitations:
+ *
+ ****************************************************************************/
+
+int ipcc_unlink(FAR struct inode *inode)
Review Comment:
unlink is normally used for the file, this case is better to go through unregister_driver.
--
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] mlyszczek commented on pull request #6537: [API review request] drivers/ipcc: add Inter Processor Communication driver
Posted by GitBox <gi...@apache.org>.
mlyszczek commented on PR #6537:
URL: https://github.com/apache/incubator-nuttx/pull/6537#issuecomment-1175238373
I've added lower half driver for the IPCC. The only test I've done is that I compiled it. I am going to write some tests for second cpu with stm32cube and fix any bugs that I find.
So right now I would like to only request design review - don't look for bugs.
@onegray If you can't flash cm0 with custom software then I don't think you will be able to take advantage of this driver. As Xiao said, with shared memory there is no standard (with the except of OpenAMP) and my implementation is ultra simple (4 bytes of length + data[]) but still custom one, and that most likely won't work with stm complex communication.
For lora driver I plan to write BSD socket interface on top of that IPCC and use Lora software/api provided by stm32cube.
--
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] acassis commented on pull request #6537: [API review request] drivers/ipcc: add Inter Processor Communication driver
Posted by GitBox <gi...@apache.org>.
acassis commented on PR #6537:
URL: https://github.com/apache/incubator-nuttx/pull/6537#issuecomment-1177843538
@mlyszczek I think we can keep your design as it is. "Being simple isn't banal. It's elegant" -- Leonardo Da Vinci
--
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] mlyszczek commented on a diff in pull request #6537: drivers/ipcc: add Inter Processor Communication driver
Posted by GitBox <gi...@apache.org>.
mlyszczek commented on code in PR #6537:
URL: https://github.com/apache/incubator-nuttx/pull/6537#discussion_r923156225
##########
drivers/ipcc/ipcc_register.c:
##########
@@ -0,0 +1,208 @@
+/****************************************************************************
+ * drivers/ipcc/ipcc_register.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/ipcc.h>
+#include <nuttx/kmalloc.h>
+#include <nuttx/mm/circbuf.h>
+
+#include <assert.h>
+#include <errno.h>
+#include <stdbool.h>
+#include <stdio.h>
+#include <sys/types.h>
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/* Device naming ************************************************************/
+
+#define DEVNAME_FMT "/dev/ipcc%d"
+#define DEVNAME_FMTLEN (9 + 3 + 1)
+
+/****************************************************************************
+ * Private Function Prototypes
+ ****************************************************************************/
+
+#ifndef CONFIG_DISABLE_PSEUDOFS_OPERATIONS
+int ipcc_open(FAR struct file *filep);
Review Comment:
No all drivers has everything in single file. i2c and 1wire drivers are examples of it. Maybe they did not split everything - like open/close is in one file, but still there is split.
I just think it's easier to move around code when these functions are split. Also it's easier to logicaly split things into modules. Like, ipcc_read() has extern ipcc_rxfree_notify() function that is read() related so it's closed in ipcc_read.c file. Also register has code to initialize things, so there is also code to free it.
I think it's just a matter of taste. Anyway, there are other drivers that also split things into separate files.
--
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] onegray commented on pull request #6537: drivers/ipcc: add Inter Processor Communication driver
Posted by GitBox <gi...@apache.org>.
onegray commented on PR #6537:
URL: https://github.com/apache/incubator-nuttx/pull/6537#issuecomment-1188241535
Yes, I am concerning about multiple copying and then parsing back from bytes stream to a packet. But probably it is a reasonable fee for a such universal interface. Telling the truth I do not have exact feeling where is [the place for a such driver](https://cwiki.apache.org/confluence/display/NUTTX/Device+Drivers+vs.+Bus+Drivers+and+GPIO+Drivers). So ignore my doubts and let's go with your solution :)
--
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