[ERD][APR-103] [COMMON]chub: ipc: fix ipc loss issue with ipc_read timeout
authorBoojin Kim <boojin.kim@samsung.com>
Thu, 7 Feb 2019 13:15:34 +0000 (22:15 +0900)
committerhskang <hs1218.kang@samsung.com>
Sun, 21 Apr 2019 09:09:54 +0000 (18:09 +0900)
Change-Id: I713acad026f9dbc1af401c24603ea9c74099ff84
Signed-off-by: Boojin Kim <boojin.kim@samsung.com>
drivers/staging/nanohub/chub.c
drivers/staging/nanohub/chub.h
drivers/staging/nanohub/chub_ipc.c
drivers/staging/nanohub/chub_ipc.h
drivers/staging/nanohub/comms.c

index 1e169aed9b86b6aaa54ea244ae4ab1e853dc6f03..b42115562127feb83af59a47cd6d9c3006ec4748 100644 (file)
@@ -70,7 +70,6 @@ static DEFINE_MUTEX(pmu_shutdown_mutex);
 static DEFINE_MUTEX(log_mutex);
 static DEFINE_MUTEX(wt_mutex);
 
-
 void chub_wake_event(struct chub_alive *event)
 {
        atomic_set(&event->flag, 1);
@@ -288,21 +287,6 @@ static inline int get_recv_channel(struct recv_ctrl *recv)
        return min_order_evt;
 }
 
-static inline bool read_is_locked(struct contexthub_ipc_info *ipc)
-{
-       return atomic_read(&ipc->read_lock.cnt) != 0;
-}
-
-static inline void read_get_locked(struct contexthub_ipc_info *ipc)
-{
-       atomic_inc(&ipc->read_lock.cnt);
-}
-
-static inline void read_put_unlocked(struct contexthub_ipc_info *ipc)
-{
-       atomic_dec(&ipc->read_lock.cnt);
-}
-
 /* simple alive check function : don't use ipc map */
 static bool contexthub_lowlevel_alive(struct contexthub_ipc_info *ipc)
 {
@@ -524,45 +508,48 @@ int contexthub_ipc_read(struct contexthub_ipc_info *ipc, uint8_t *rx, int max_le
        int size = 0;
        int ret;
        void *rxbuf;
+       u64 time = 0; /* for debug */
+
+       if (!atomic_read(&ipc->read_lock.cnt)) {
+               time = sched_clock();
 
-       if (!ipc->read_lock.flag) {
                spin_lock_irqsave(&ipc->read_lock.event.lock, flag);
-               read_get_locked(ipc);
+               atomic_inc(&ipc->read_lock.flag);
                ret =
                        wait_event_interruptible_timeout_locked(ipc->read_lock.event,
-                                                               ipc->read_lock.flag,
+                                                               atomic_read(&ipc->read_lock.cnt),
                                                                msecs_to_jiffies(timeout));
-               read_put_unlocked(ipc);
+               atomic_dec(&ipc->read_lock.flag);
                spin_unlock_irqrestore(&ipc->read_lock.event.lock, flag);
                if (ret < 0)
                        dev_warn(ipc->dev,
-                                "fails to get read ret:%d timeout:%d, flag:0x%x",
-                                ret, timeout, ipc->read_lock.flag);
-
-               if (!ipc->read_lock.flag)
-                       goto fail_get_channel;
+                                "fails to get read ret:%d timeout:%d\n", ret, timeout);
        }
 
-       ipc->read_lock.flag--;
-
        if (contexthub_get_token(ipc)) {
                dev_warn(ipc->dev, "no-active: read fails\n");
                return 0;
        }
 
-       rxbuf = ipc_read_data(IPC_DATA_C2A, &size);
-
-       if (size > 0)
-               ret = contexthub_read_process(rx, rxbuf, size);
+       if (atomic_read(&ipc->read_lock.cnt)) {
+               rxbuf = ipc_read_data(IPC_DATA_C2A, &size);
+               if (size > 0) {
+                       ret = contexthub_read_process(rx, rxbuf, size);
+                       atomic_dec(&ipc->read_lock.cnt);
+               }
+       } else {
+               dev_warn(ipc->dev, "%s: read timeout(%d): c2aq_cnt:%d, recv_cnt:%d during %lld ns\n",
+                       __func__, ipc->err_cnt[CHUB_ERR_READ_FAIL],
+                       ipc_get_data_cnt(IPC_DATA_C2A), atomic_read(&ipc->read_lock.cnt),
+                       sched_clock() - time);
+               if (ipc_get_data_cnt(IPC_DATA_C2A)) {
+                       ipc->err_cnt[CHUB_ERR_READ_FAIL]++;
+                       ipc_dump();
+               }
+               ret = -EINVAL;
+       }
        contexthub_put_token(ipc);
        return ret;
-
-fail_get_channel:
-       if (ipc_get_data_cnt(IPC_DATA_C2A)) {
-               dev_warn(ipc->dev, "%s: read timeout\n", __func__);
-               ipc->err_cnt[CHUB_ERR_READ_FAIL]++;
-       }
-       return -EINVAL;
 }
 
 int contexthub_ipc_write(struct contexthub_ipc_info *ipc,
@@ -625,7 +612,8 @@ static int contexthub_hw_reset(struct contexthub_ipc_info *ipc,
        /* clear ipc value */
        atomic_set(&ipc->wakeup_chub, CHUB_OFF);
        atomic_set(&ipc->irq1_apInt, C2A_OFF);
-       atomic_set(&ipc->read_lock.cnt, 0x0);
+       atomic_set(&ipc->read_lock.cnt, 0);
+       atomic_set(&ipc->read_lock.flag, 0);
        atomic_set(&ipc->log_work_active, 0);
 
        /* chub err init */
@@ -635,7 +623,6 @@ static int contexthub_hw_reset(struct contexthub_ipc_info *ipc,
                ipc->err_cnt[i] = 0;
        }
 
-       ipc->read_lock.flag = 0;
        ipc_hw_write_shared_reg(AP, ipc->os_load, SR_BOOT_MODE);
        ipc_set_chub_clk((u32)ipc->clkrate);
        ipc->chub_rt_log.loglevel = CHUB_RT_LOG_DUMP;
@@ -968,19 +955,19 @@ int contexthub_ipc_write_event(struct contexthub_ipc_info *ipc,
                case MAILBOX_EVT_WAKEUP_CLR:
                        if (atomic_read(&ipc->wakeup_chub) == CHUB_ON) {
                                ret = ipc_add_evt(IPC_EVT_A2C, IRQ_EVT_A2C_WAKEUP_CLR);
-                               if (!ret)
+                               if (ret >= 0)
                                        atomic_set(&ipc->wakeup_chub, CHUB_OFF);
                                else
-                                       dev_warn(ipc->dev, "%s: fails to set wakeup", __func__);
+                                       dev_warn(ipc->dev, "%s: fails to set wakeup. ret:%d", __func__, ret);
                        }
                        break;
                case MAILBOX_EVT_WAKEUP:
                        if (atomic_read(&ipc->wakeup_chub) == CHUB_OFF) {
                                ret = ipc_add_evt(IPC_EVT_A2C, IRQ_EVT_A2C_WAKEUP);
-                               if (!ret)
+                               if (ret >= 0)
                                        atomic_set(&ipc->wakeup_chub, CHUB_ON);
                                else
-                                       dev_warn(ipc->dev, "%s: fails to set wakeupclr", __func__);
+                                       dev_warn(ipc->dev, "%s: fails to set wakeupclr. ret:%d", __func__, ret);
                        }
                        break;
                default:
@@ -996,7 +983,7 @@ int contexthub_ipc_write_event(struct contexthub_ipc_info *ipc,
                }
                contexthub_put_token(ipc);
 
-               if (ret)
+               if (ret < 0)
                        ipc->err_cnt[CHUB_ERR_EVTQ_ADD]++;
        }
        return ret;
@@ -1240,10 +1227,10 @@ static void handle_irq(struct contexthub_ipc_info *ipc, enum irq_evt_chub evt)
                if (evt < IRQ_EVT_CH_MAX) {
                        int lock;
 
-                       ipc->read_lock.flag++;
+                       atomic_inc(&ipc->read_lock.cnt);
                        /* TODO: requered.. ? */
                        spin_lock(&ipc->read_lock.event.lock);
-                       lock = read_is_locked(ipc);
+                       lock = atomic_read(&ipc->read_lock.flag);
                        spin_unlock(&ipc->read_lock.event.lock);
                        if (lock)
                                wake_up_interruptible_sync(&ipc->read_lock.event);
index 91dc243d28891b89059103388a7b1633e4aba7a9..b2357f5c0a65477983762fe531e341efc9213561 100644 (file)
@@ -103,7 +103,7 @@ enum chub_status {
 
 struct read_wait {
        atomic_t cnt;
-       volatile u32 flag;
+       atomic_t flag;
        wait_queue_head_t event;
 };
 
index 3b860dcd0267d6a12338dee4ea2defb4ccde4fb6..1b09bcc05381c2da85a1bd7faea6bd6bacca754c 100644 (file)
@@ -220,7 +220,7 @@ u32 *ipc_get_chub_msp(void)
 }
 #endif
 
-static void dump_mailbox_sfr(void)
+static void ipc_dump_mailbox_sfr(void)
 {
        int i = 0;
        u32 val;
@@ -228,7 +228,7 @@ static void dump_mailbox_sfr(void)
        for (i = 0; i <= REG_MAILBOX_INTMSR1; i += 4) {
                val = __raw_readl(ipc_own[AP].base + i);
                if (val)
-                       CSP_PRINTF_ERROR("%s:sfr:+0x%x:0x%x\n", NAME_PREFIX, i, val);
+                       CSP_PRINTF_ERROR("%s: sfr:+0x%x:0x%x\n", __func__, i, val);
        }
 }
 
@@ -517,8 +517,8 @@ retry:
                cur_evt = &ipc_evt->data[ipc_evt->ctrl.eq];
                if (!cur_evt) {
                        CSP_PRINTF_ERROR("%s:%s: invalid cur_evt\n", NAME_PREFIX, __func__);
-                       ret = -EINVAL;
-                       goto out;
+                       ENABLE_IRQ(LOCK_ADD_EVT, &flag);
+                       return -EINVAL;
                }
 
                /* get evt channel */
@@ -711,17 +711,22 @@ int ipc_get_data_cnt(enum ipc_data_list dataq)
                return ipc_data->eq + (IPC_CH_BUF_NUM - ipc_data->dq);
 }
 
-void ipc_print_databuf(void)
+static void ipc_print_databuf(void)
 {
-       struct ipc_buf *ipc_data = ipc_get_base(IPC_REG_IPC_A2C);
-
-       CSP_PRINTF_INFO("%s: a2c: eq:%d dq:%d full:%d empty:%d\n",
-               NAME_PREFIX, ipc_data->eq, ipc_data->dq, ipc_data->full, ipc_data->empty);
+       struct ipc_buf *ipc_data;
+       int i;
+       int j;
 
-       ipc_data = ipc_get_base(IPC_REG_IPC_C2A);
+       for (j = 0; j < IPC_DATA_MAX; j++) {
+               ipc_data = (j == IPC_DATA_C2A) ? ipc_get_base(IPC_REG_IPC_C2A) : ipc_get_base(IPC_REG_IPC_A2C);
+               CSP_PRINTF_INFO("%s: %s: eq:%d dq:%d\n",
+                       __func__, (j == IPC_DATA_C2A) ? "c2a" : "a2c", ipc_data->eq, ipc_data->dq);
 
-       CSP_PRINTF_INFO("%s: c2a: eq:%d dq:%d full:%d empty:%d\n",
-               NAME_PREFIX, ipc_data->eq, ipc_data->dq, ipc_data->full, ipc_data->empty);
+               if (ipc_get_data_cnt(j))
+                       for (i = 0; i < IPC_CH_BUF_NUM; i++)
+                               CSP_PRINTF_INFO("%s: ch%d(size:0x%x)\n",
+                                               __func__, i, ipc_data->ch[i].size);
+       }
 }
 
 static void ipc_print_logbuf(void)
@@ -729,43 +734,14 @@ static void ipc_print_logbuf(void)
        struct ipc_logbuf *logbuf = &ipc_map->logbuf;
 
        CSP_PRINTF_INFO("%s: channel: eq:%d, dq:%d, size:%d, full:%d, dbg_full_cnt:%d, err(overwrite):%d, ipc-reqcnt:%d, fw:%d\n",
-               NAME_PREFIX, logbuf->eq, logbuf->dq, logbuf->size, logbuf->full, logbuf->dbg_full_cnt,
+               __func__, logbuf->eq, logbuf->dq, logbuf->size, logbuf->full, logbuf->dbg_full_cnt,
                logbuf->errcnt, logbuf->reqcnt, logbuf->fw_num);
        CSP_PRINTF_INFO("%s: raw: eq:%d, dq:%d, size:%d, full:%d\n",
-               NAME_PREFIX, logbuf->logbuf.eq, logbuf->logbuf.dq, logbuf->logbuf.size, logbuf->logbuf.full);
-}
+               __func__, logbuf->logbuf.eq, logbuf->logbuf.dq, logbuf->logbuf.size, logbuf->logbuf.full);
 
-int ipc_check_reset_valid()
-{
-       int i;
-       int ret = 0;
-       struct ipc_map_area *map = ipc_get_base(IPC_REG_IPC);
 
-       for (i = 0; i < IPC_DATA_MAX; i++)
-               if (map->data[i].dq || map->data[i].eq ||
-                       map->data[i].full || (map->data[i].empty != 1)) {
-                       CSP_PRINTF_INFO("%s: %s: ipc_data_%s invalid: eq:%d, dq:%d, full:%d, empty:%d\n",
-                       NAME_PREFIX, __func__, i ? "a2c" : "c2a",
-                       map->data[i].eq,
-                       map->data[i].dq,
-                       map->data[i].full,
-                       map->data[i].empty);
-                       ret = -EINVAL;
-               }
 
-       for (i = 0; i < IPC_EVT_MAX; i++)
-               if (map->evt[i].ctrl.eq || map->evt[i].ctrl.dq ||
-                       map->evt[i].ctrl.full || (map->evt[i].ctrl.empty != 1)) {
-                       CSP_PRINTF_INFO("%s: %s: ipc_evt_%s invalid: eq:%d, dq:%d, full:%d, empty:%d\n",
-                       NAME_PREFIX, __func__, i ? "a2c" : "c2a",
-                       map->evt[i].ctrl.eq,
-                       map->evt[i].ctrl.eq,
-                       map->evt[i].ctrl.full,
-                       map->evt[i].ctrl.empty);
-                       ret = -EINVAL;
-               }
 
-       return ret;
 }
 
 void ipc_init(void)
@@ -778,16 +754,12 @@ void ipc_init(void)
        for (i = 0; i < IPC_DATA_MAX; i++) {
                ipc_map->data[i].eq = 0;
                ipc_map->data[i].dq = 0;
-               ipc_map->data[i].full = 0;
-               ipc_map->data[i].empty = 1;
        }
 
        ipc_hw_clear_all_int_pend_reg(AP);
        for (j = 0; j < IPC_EVT_MAX; j++) {
                ipc_map->evt[j].ctrl.dq = 0;
                ipc_map->evt[j].ctrl.eq = 0;
-               ipc_map->evt[j].ctrl.full = 0;
-               ipc_map->evt[j].ctrl.empty = 1;
                ipc_map->evt[j].ctrl.irq = 0;
                for (i = 0 ; i < IRQ_MAX; i++)
                        ipc_map->evt[j].ctrl.pending[i] = 0;
@@ -806,19 +778,18 @@ void ipc_print_evt(enum ipc_evt_list evtq)
        struct ipc_evt *ipc_evt = &ipc_map->evt[evtq];
        int i;
 
-       CSP_PRINTF_INFO("%s: evt(%p)-%s: eq:%d dq:%d full:%d irq:%d\n",
-                       NAME_PREFIX, ipc_evt, IPC_GET_EVT_NAME(evtq), ipc_evt->ctrl.eq,
-                       ipc_evt->ctrl.dq, ipc_evt->ctrl.full,
-                       ipc_evt->ctrl.irq);
+       CSP_PRINTF_INFO("%s: evt(%p)-%s: eq:%d dq:%d irq:%d\n",
+                       __func__, ipc_evt, IPC_GET_EVT_NAME(evtq), ipc_evt->ctrl.eq,
+                       ipc_evt->ctrl.dq, ipc_evt->ctrl.irq);
 
-       for (i = 0; i < IRQ_MAX; i++) {
+       for (i = 0; i < IRQ_MAX; i++)
                if (ipc_evt->ctrl.pending[i])
-                       CSP_PRINTF_INFO("%s: irq:%d filled", __func__, i);
-       }
+                       CSP_PRINTF_INFO("%s: irq:%d filled\n", __func__, i);
+
        for (i = 0; i < IPC_EVT_NUM; i++) {
                if (ipc_evt->data[i].status)
                        CSP_PRINTF_INFO("%s: evt%d(evt:%d,irq:%d,f:%d)\n",
-                               NAME_PREFIX, i, ipc_evt->data[i].evt,
+                               __func__, i, ipc_evt->data[i].evt,
                                ipc_evt->data[i].irq, ipc_evt->data[i].status);
        }
 }
@@ -826,19 +797,16 @@ void ipc_print_evt(enum ipc_evt_list evtq)
 void ipc_dump(void)
 {
        if (strncmp(CHUB_IPC_MAGIC, ipc_map->magic, sizeof(CHUB_IPC_MAGIC))) {
-               CSP_PRINTF_INFO("%s: %s: ipc crash\n", NAME_PREFIX, __func__);
+               CSP_PRINTF_INFO("%s: [%s]: ipc crash\n", NAME_PREFIX, __func__);
                return;
        }
 
-       CSP_PRINTF_INFO("%s: %s: a2x event, magic:%s\n", NAME_PREFIX, __func__, ipc_map->magic);
+       CSP_PRINTF_INFO("[%s]: magic:%s\n", __func__, ipc_map->magic);
        ipc_print_evt(IPC_EVT_A2C);
-       CSP_PRINTF_INFO("%s: %s: c2a event\n", NAME_PREFIX, __func__);
        ipc_print_evt(IPC_EVT_C2A);
-       CSP_PRINTF_INFO("%s: %s: data buffer\n", NAME_PREFIX, __func__);
        ipc_print_databuf();
-       CSP_PRINTF_INFO("%s: %s: log buffer\n", NAME_PREFIX, __func__);
        ipc_print_logbuf();
-       dump_mailbox_sfr();
+       ipc_dump_mailbox_sfr();
 }
 
 #ifdef CHUB_IPC
index 0028fbab78f37d8b7beb78d99a00b6f206e0a596..687cdf68bc71b3e1b3153761419fcdded8caa12f 100644 (file)
@@ -537,7 +537,6 @@ void ipc_set_base(void *addr);
 void *ipc_get_base(enum ipc_region area);
 u32 ipc_get_offset(enum ipc_region area);
 void *ipc_get_addr(enum ipc_region area, int buf_num);
-int ipc_check_reset_valid(void);
 void ipc_init(void);
 int ipc_hw_read_int_start_index(enum ipc_owner owner);
 /* logbuf functions */
@@ -588,7 +587,6 @@ u16 ipc_get_chub_rtlogmode(void);
 u16 ipc_get_chub_bootmode(void);
 void ipc_set_ap_wake(u16 wake);
 u16 ipc_get_ap_wake(void);
-void ipc_print_databuf(void);
 void ipc_dump(void);
 #if defined(LOCAL_POWERGATE)
 u32 *ipc_get_chub_psp(void);
index 7e2ed8ba8b3c5906dbf4edf1b2bf5946e7c7085c..0c6053fdbb623ff84a6795490f781a9e4b72b701 100644 (file)
@@ -321,8 +321,13 @@ static int get_reply(struct nanohub_data *data, struct nanohub_packet *response,
                                       response->len);
                        ret =
                            read_msg(data, response, data->comms.timeout_reply);
+#ifdef CONFIG_NANOHUB_MAILBOX /* remove invalid error check */
                        if (ret < 0 || response->reason == CMD_COMMS_ACK)
                                ret = ERROR_NACK;
+#else
+                       if (ret < 0)
+                               ret = ERROR_NACK;
+#endif
                } else {
                        int i;
                        uint8_t *b = (uint8_t *) response;