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/09/21 15:33:20 UTC
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #7131: poll: add poll_notify() api and call it in all drivers.
xiaoxiang781216 commented on code in PR #7131:
URL: https://github.com/apache/incubator-nuttx/pull/7131#discussion_r976633836
##########
drivers/analog/comp.c:
##########
@@ -91,35 +91,12 @@ static const struct comp_callback_s g_comp_callback =
static void comp_pollnotify(FAR struct comp_dev_s *dev,
pollevent_t eventset)
{
- int i;
-
if (eventset & POLLERR)
{
eventset &= ~(POLLOUT | POLLIN);
Review Comment:
don't need, handled by poll_notify now. let's remove comp_pollnotify and call poll_notify directly
##########
drivers/can/can.c:
##########
@@ -175,22 +175,7 @@ static int can_takesem(FAR sem_t *sem)
static void can_pollnotify(FAR struct can_dev_s *dev, pollevent_t eventset)
Review Comment:
remove?
##########
drivers/pipes/pipe_common.c:
##########
@@ -80,35 +80,12 @@ static int pipecommon_semtake(FAR sem_t *sem)
static void pipecommon_pollnotify(FAR struct pipe_dev_s *dev,
pollevent_t eventset)
{
- int i;
-
if (eventset & POLLERR)
{
eventset &= ~(POLLOUT | POLLIN);
Review Comment:
remove
##########
drivers/ipcc/ipcc_poll.c:
##########
@@ -175,32 +175,5 @@ int ipcc_poll(FAR struct file *filep, FAR struct pollfd *fds,
void ipcc_pollnotify(FAR struct ipcc_driver_s *priv, pollevent_t eventset)
Review Comment:
remove
##########
drivers/usbhost/usbhost_cdcmbim.c:
##########
@@ -405,21 +405,7 @@ static int usbhost_ctrl_cmd(FAR struct usbhost_cdcmbim_s *priv,
static void usbhost_pollnotify(FAR struct usbhost_cdcmbim_s *priv)
{
- int i;
-
- for (i = 0; i < CONFIG_USBHOST_CDCMBIM_NPOLLWAITERS; i++)
- {
- struct pollfd *fds = priv->fds[i];
- if (fds)
- {
- fds->revents |= (fds->events & POLLIN);
- if (fds->revents != 0)
- {
- uinfo("Report events: %08" PRIx32 "\n", fds->revents);
- nxsem_post(fds->sem);
- }
- }
- }
+ poll_notify(priv->fds, CONFIG_USBHOST_CDCMBIM_NPOLLWAITERS, POLLIN);
Review Comment:
remove usbhost_pollnotify
##########
drivers/usbhost/usbhost_hidkbd.c:
##########
@@ -686,21 +686,7 @@ static void usbhost_forcetake(FAR sem_t *sem)
static void usbhost_pollnotify(FAR struct usbhost_state_s *priv)
{
- int i;
-
- for (i = 0; i < CONFIG_HIDKBD_NPOLLWAITERS; i++)
- {
- struct pollfd *fds = priv->fds[i];
- if (fds)
- {
- fds->revents |= (fds->events & POLLIN);
- if (fds->revents != 0)
- {
- uinfo("Report events: %08" PRIx32 "\n", fds->revents);
- nxsem_post(fds->sem);
- }
- }
- }
+ poll_notify(priv->fds, CONFIG_HIDKBD_NPOLLWAITERS, POLLIN);
Review Comment:
remove usbhost_pollnotify
##########
drivers/usbhost/usbhost_xboxcontroller.c:
##########
@@ -561,8 +561,6 @@ static void usbhost_destroy(FAR void *arg)
static void usbhost_pollnotify(FAR struct usbhost_state_s *priv)
{
- int i;
Review Comment:
remove usbhost_pollnotify
##########
drivers/usrsock/usrsock_dev.c:
##########
@@ -159,20 +159,7 @@ static bool usrsockdev_is_opened(FAR struct usrsockdev_s *dev)
static void usrsockdev_pollnotify(FAR struct usrsockdev_s *dev,
Review Comment:
remove
##########
fs/vfs/fs_timerfd.c:
##########
@@ -178,22 +178,7 @@ static timerfd_t timerfd_get_counter(FAR struct timerfd_priv_s *dev)
static void timerfd_pollnotify(FAR struct timerfd_priv_s *dev,
Review Comment:
remove
##########
net/tcp/tcp_netpoll.c:
##########
@@ -352,12 +351,7 @@ int tcp_pollsetup(FAR struct socket *psock, FAR struct pollfd *fds)
/* Check if any requested events are already in effect */
- if (fds->revents != 0)
- {
- /* Yes.. then signal the poll logic */
-
- nxsem_post(fds->sem);
- }
+ poll_notify(&fds, 1, 0);
Review Comment:
shouldn't change revents
##########
net/tcp/tcp_netpoll.c:
##########
@@ -167,8 +167,7 @@ static uint16_t tcp_poll_eventhandler(FAR struct net_driver_s *dev,
info->cb->priv = NULL;
info->cb->event = NULL;
- info->fds->revents |= eventset;
- nxsem_post(info->fds->sem);
+ poll_notify(&info->fds, 1, eventset);
Review Comment:
remove the check at line 162 and all `& info->fds->events`
##########
drivers/pipes/pipe_common.c:
##########
@@ -80,35 +80,12 @@ static int pipecommon_semtake(FAR sem_t *sem)
static void pipecommon_pollnotify(FAR struct pipe_dev_s *dev,
Review Comment:
remove
##########
drivers/input/keyboard_upper.c:
##########
@@ -105,22 +105,7 @@ static const struct file_operations g_keyboard_fops =
static void keyboard_notify(FAR struct keyboard_opriv_s *opriv)
Review Comment:
remove
##########
drivers/input/spq10kbd.c:
##########
@@ -446,21 +446,7 @@ static int spq10kbd_interrupt(int irq, FAR void *context, FAR void *arg)
static void spq10kbd_pollnotify(FAR struct spq10kbd_dev_s *priv)
Review Comment:
remove
##########
drivers/sensors/hc_sr04.c:
##########
@@ -275,24 +275,13 @@ static void hcsr04_notify(FAR struct hcsr04_dev_s *priv)
{
DEBUGASSERT(priv != NULL);
- int i;
-
/* If there are threads waiting on poll() for data to become available,
* then wake them up now. NOTE: we wake up all waiting threads because we
* do not know that they are going to do. If they all try to read the
* data, then some make end up blocking after all.
*/
- for (i = 0; i < CONFIG_HCSR04_NPOLLWAITERS; i++)
- {
- FAR struct pollfd *fds = priv->fds[i];
- if (fds)
- {
- fds->revents |= POLLIN;
- hcsr04_dbg("Report events: %08" PRIx32 "\n", fds->revents);
- nxsem_post(fds->sem);
- }
- }
+ poll_notify(priv->fds, CONFIG_HCSR04_NPOLLWAITERS, POLLIN);
Review Comment:
remove hcsr04_notify, call poll_notify directly
##########
net/local/local_netpoll.c:
##########
@@ -125,21 +126,7 @@ void local_event_pollnotify(FAR struct local_conn_s *conn,
pollevent_t eventset)
{
#ifdef CONFIG_NET_LOCAL_STREAM
- int i;
-
- for (i = 0; i < LOCAL_NPOLLWAITERS; i++)
- {
- struct pollfd *fds = conn->lc_event_fds[i];
- if (fds)
- {
- fds->revents |= (fds->events & eventset);
- if (fds->revents != 0)
- {
- ninfo("Report events: %08" PRIx32 "\n", fds->revents);
- nxsem_post(fds->sem);
- }
- }
- }
+ poll_notify(conn->lc_event_fds, LOCAL_NPOLLWAITERS, eventset);
Review Comment:
remove local_event_pollnotify
##########
drivers/sensors/hts221.c:
##########
@@ -1058,24 +1058,13 @@ static void hts221_notify(FAR struct hts221_dev_s *priv)
{
DEBUGASSERT(priv != NULL);
- int i;
-
/* If there are threads waiting on poll() for data to become available,
* then wake them up now. NOTE: we wake up all waiting threads because we
* do not know that they are going to do. If they all try to read the
* data, then some make end up blocking after all.
*/
- for (i = 0; i < CONFIG_HTS221_NPOLLWAITERS; i++)
- {
- FAR struct pollfd *fds = priv->fds[i];
- if (fds)
- {
- fds->revents |= POLLIN;
- hts221_dbg("Report events: %08" PRIx32 "\n", fds->revents);
- nxsem_post(fds->sem);
- }
- }
+ poll_notify(priv->fds, CONFIG_HTS221_NPOLLWAITERS, POLLIN);
Review Comment:
remove hts221_notify ditto
##########
drivers/sensors/lis2dh.c:
##########
@@ -740,24 +740,13 @@ static void lis2dh_notify(FAR struct lis2dh_dev_s *priv)
{
DEBUGASSERT(priv != NULL);
- int i;
-
/* If there are threads waiting on poll() for LIS2DH data to become
* available, then wake them up now. NOTE: we wake up all waiting threads
* because we do not know that they are going to do. If they all try to
* read the data, then some make end up blocking after all.
*/
- for (i = 0; i < CONFIG_LIS2DH_NPOLLWAITERS; i++)
- {
- struct pollfd *fds = priv->fds[i];
- if (fds)
- {
- fds->revents |= POLLIN;
- lis2dh_dbg("lis2dh: Report events: %08" PRIx32 "\n", fds->revents);
- nxsem_post(fds->sem);
- }
- }
+ poll_notify(priv->fds, CONFIG_LIS2DH_NPOLLWAITERS, POLLIN);
Review Comment:
remove lis2dh_notify
##########
drivers/serial/serial.c:
##########
@@ -171,32 +171,7 @@ static int uart_takesem(FAR sem_t *sem, bool errout)
static void uart_pollnotify(FAR uart_dev_t *dev, pollevent_t eventset)
{
- int i;
-
- for (i = 0; i < CONFIG_SERIAL_NPOLLWAITERS; i++)
- {
- struct pollfd *fds = dev->fds[i];
- if (fds)
- {
-#ifdef CONFIG_SERIAL_REMOVABLE
- fds->revents |= ((fds->events | (POLLERR | POLLHUP)) & eventset);
-#else
- fds->revents |= (fds->events & eventset);
-#endif
- if (fds->revents != 0)
- {
- int semcount;
-
- finfo("Report events: %08" PRIx32 "\n", fds->revents);
-
- nxsem_get_value(fds->sem, &semcount);
- if (semcount < 1)
- {
- nxsem_post(fds->sem);
- }
- }
- }
- }
+ poll_notify(dev->fds, CONFIG_SERIAL_NPOLLWAITERS, eventset);
Review Comment:
remove uart_pollnotify
##########
drivers/usbhost/usbhost_hidmouse.c:
##########
@@ -473,21 +473,7 @@ static void usbhost_forcetake(FAR sem_t *sem)
static void usbhost_pollnotify(FAR struct usbhost_state_s *priv)
{
- int i;
-
- for (i = 0; i < CONFIG_HIDMOUSE_NPOLLWAITERS; i++)
- {
- struct pollfd *fds = priv->fds[i];
- if (fds)
- {
- fds->revents |= (fds->events & POLLIN);
- if (fds->revents != 0)
- {
- uinfo("Report events: %08" PRIx32 "\n", fds->revents);
- nxsem_post(fds->sem);
- }
- }
- }
+ poll_notify(priv->fds, CONFIG_HIDMOUSE_NPOLLWAITERS, POLLIN);
Review Comment:
remove usbhost_pollnotify
##########
drivers/usbdev/adb.c:
##########
@@ -1553,22 +1553,7 @@ static void adb_char_notify_readers(FAR struct usbdev_adb_s *priv)
static void adb_char_pollnotify(FAR struct usbdev_adb_s *dev,
pollevent_t eventset)
{
- FAR struct pollfd *fds;
- int i;
-
- for (i = 0; i < CONFIG_USBADB_NPOLLWAITERS; i++)
- {
- fds = dev->fds[i];
- if (fds)
- {
- fds->revents |= eventset & (fds->events | POLLERR | POLLHUP);
-
- if (fds->revents != 0)
- {
- nxsem_post(fds->sem);
- }
- }
- }
+ poll_notify(dev->fds, CONFIG_USBADB_NPOLLWAITERS, eventset);
Review Comment:
remove adb_char_pollnotify
##########
drivers/usbmisc/fusb303.c:
##########
@@ -927,8 +927,6 @@ static int fusb303_poll(FAR struct file *filep, FAR struct pollfd *fds,
static void fusb303_notify(FAR struct fusb303_dev_s *priv)
Review Comment:
remove
##########
fs/mqueue/mq_open.c:
##########
@@ -373,28 +373,7 @@ static mqd_t nxmq_vopen(FAR const char *mq_name, int oflags, va_list ap)
#if CONFIG_FS_MQUEUE_NPOLLWAITERS > 0
void nxmq_pollnotify(FAR struct mqueue_inode_s *msgq, pollevent_t eventset)
Review Comment:
remove
##########
drivers/usbmisc/fusb301.c:
##########
@@ -741,24 +741,13 @@ static void fusb301_notify(FAR struct fusb301_dev_s *priv)
{
DEBUGASSERT(priv != NULL);
- int i;
Review Comment:
remove fusb301_notify
##########
fs/vfs/fs_eventfd.c:
##########
@@ -148,22 +148,7 @@ static void eventfd_destroy(FAR struct eventfd_priv_s *dev)
static void eventfd_pollnotify(FAR struct eventfd_priv_s *dev,
Review Comment:
remove
##########
net/udp/udp_netpoll.c:
##########
@@ -101,8 +101,7 @@ static uint16_t udp_poll_eventhandler(FAR struct net_driver_s *dev,
if (eventset)
Review Comment:
remove the check and ` & info->fds->events`
##########
net/usrsock/usrsock_poll.c:
##########
@@ -117,8 +117,7 @@ static uint16_t poll_event(FAR struct net_driver_s *dev,
if (eventset)
Review Comment:
remove the check, remove line 107-114
##########
net/udp/udp_netpoll.c:
##########
@@ -227,12 +226,7 @@ int udp_pollsetup(FAR struct socket *psock, FAR struct pollfd *fds)
/* Check if any requested events are already in effect */
- if (fds->revents != 0)
Review Comment:
add events and don't change revents
##########
net/usrsock/usrsock_poll.c:
##########
@@ -265,12 +264,7 @@ static int usrsock_pollsetup(FAR struct socket *psock,
/* Check if any requested events are already in effect */
- if (fds->revents != 0)
Review Comment:
let's create new variable(e.g. events), we shouldn't modify revents
##########
net/rpmsg/rpmsg_sockif.c:
##########
@@ -208,22 +208,7 @@ static inline void rpmsg_socket_post(FAR sem_t *sem)
static void rpmsg_socket_pollnotify(FAR struct rpmsg_socket_conn_s *conn,
Review Comment:
remove rpmsg_socket_pollnotify
##########
net/netlink/netlink_sockif.c:
##########
@@ -594,8 +592,7 @@ static int netlink_poll(FAR struct socket *psock, FAR struct pollfd *fds,
revents &= fds->events;
if (revents != 0)
Review Comment:
remove line 592-593
##########
fs/vfs/fs_poll.c:
##########
@@ -276,6 +277,60 @@ static inline int poll_teardown(FAR struct pollfd *fds, nfds_t nfds,
* Public Functions
****************************************************************************/
+/****************************************************************************
+ * Name: poll_notify
+ *
+ * Description:
+ * Notify the poll, this function should be called by drivers to notify
+ * the caller the poll is ready.
+ *
+ * Input Parameters:
+ * afds - The fds array
+ * nfds - Number of fds array
+ * eventset - List of events to check for activity
+ *
+ * Returned Value:
+ * None
+ *
+ ****************************************************************************/
+
+void poll_notify(FAR struct pollfd **afds, int nfds, pollevent_t eventset)
+{
+ int i;
+ int semcount;
+ FAR struct pollfd *fds;
+
+ DEBUGASSERT(afds != NULL && nfds >= 1);
+
+ for (i = 0; i < nfds; i++)
+ {
+ fds = afds[i];
+ if (fds != NULL)
+ {
+ /* The error event must be set in fds->revents */
+
+ fds->revents |= eventset & (fds->events | POLLERR | POLLHUP);
Review Comment:
remove the similar logic from all caller.
--
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