2021-03-14 23:35:52

by Chun-Kuang Hu

[permalink] [raw]
Subject: [PATCH 0/3] Refine mtk-cmdq-mailbox callback mechanism

mtk-cmdq-mailbox use proprietary callback mechanism and proprietary
error number, but these could be replaced by standard callback
mechanism and standard error number. In addition, use cmdq_pkt as
callback data to prevent redundnat assignment.

Because client driver still use proprietary mechanism, so keep
proprietary mechanism until client driver use the standard one.

Chun-Kuang Hu (3):
mailbox: mtk-cmdq: Remove cmdq_cb_status
mailbox: mtk-cmdq: Use mailbox rx_callback
mailbox: mtk-cmdq: Add struct cmdq_pkt in struct cmdq_cb_data

drivers/mailbox/mtk-cmdq-mailbox.c | 24 +++++++++++++++---------
include/linux/mailbox/mtk-cmdq-mailbox.h | 8 ++------
2 files changed, 17 insertions(+), 15 deletions(-)

--
2.17.1


2021-03-14 23:36:43

by Chun-Kuang Hu

[permalink] [raw]
Subject: [PATCH 1/3] mailbox: mtk-cmdq: Remove cmdq_cb_status

cmdq_cb_status is an error status. Use the standard error number
instead of cmdq_cb_status to prevent status duplication.

Cc: Jassi Brar <[email protected]>
Cc: Matthias Brugger <[email protected]>
Cc: Houlong Wei <[email protected]>
Cc: Bibby Hsieh <[email protected]>
Cc: Dennis YC Hsieh <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Chun-Kuang Hu <[email protected]>
---
drivers/mailbox/mtk-cmdq-mailbox.c | 10 +++++-----
include/linux/mailbox/mtk-cmdq-mailbox.h | 7 +------
2 files changed, 6 insertions(+), 11 deletions(-)

diff --git a/drivers/mailbox/mtk-cmdq-mailbox.c b/drivers/mailbox/mtk-cmdq-mailbox.c
index 5665b6ea8119..3d37c1cd40f1 100644
--- a/drivers/mailbox/mtk-cmdq-mailbox.c
+++ b/drivers/mailbox/mtk-cmdq-mailbox.c
@@ -180,7 +180,7 @@ static bool cmdq_thread_is_in_wfe(struct cmdq_thread *thread)
return readl(thread->base + CMDQ_THR_WAIT_TOKEN) & CMDQ_THR_IS_WAITING;
}

-static void cmdq_task_exec_done(struct cmdq_task *task, enum cmdq_cb_status sta)
+static void cmdq_task_exec_done(struct cmdq_task *task, int sta)
{
struct cmdq_task_cb *cb = &task->pkt->async_cb;
struct cmdq_cb_data data;
@@ -244,10 +244,10 @@ static void cmdq_thread_irq_handler(struct cmdq *cmdq,
curr_task = task;

if (!curr_task || curr_pa == task_end_pa - CMDQ_INST_SIZE) {
- cmdq_task_exec_done(task, CMDQ_CB_NORMAL);
+ cmdq_task_exec_done(task, 0);
kfree(task);
} else if (err) {
- cmdq_task_exec_done(task, CMDQ_CB_ERROR);
+ cmdq_task_exec_done(task, -ENOEXEC);
cmdq_task_handle_error(curr_task);
kfree(task);
}
@@ -415,7 +415,7 @@ static void cmdq_mbox_shutdown(struct mbox_chan *chan)

list_for_each_entry_safe(task, tmp, &thread->task_busy_list,
list_entry) {
- cmdq_task_exec_done(task, CMDQ_CB_ERROR);
+ cmdq_task_exec_done(task, -ECONNABORTED);
kfree(task);
}

@@ -453,7 +453,7 @@ static int cmdq_mbox_flush(struct mbox_chan *chan, unsigned long timeout)
list_entry) {
cb = &task->pkt->async_cb;
if (cb->cb) {
- data.sta = CMDQ_CB_ERROR;
+ data.sta = -ECONNABORTED;
data.data = cb->data;
cb->cb(data);
}
diff --git a/include/linux/mailbox/mtk-cmdq-mailbox.h b/include/linux/mailbox/mtk-cmdq-mailbox.h
index d5a983d65f05..2f7d9a37d611 100644
--- a/include/linux/mailbox/mtk-cmdq-mailbox.h
+++ b/include/linux/mailbox/mtk-cmdq-mailbox.h
@@ -65,13 +65,8 @@ enum cmdq_code {
CMDQ_CODE_LOGIC = 0xa0,
};

-enum cmdq_cb_status {
- CMDQ_CB_NORMAL = 0,
- CMDQ_CB_ERROR
-};
-
struct cmdq_cb_data {
- enum cmdq_cb_status sta;
+ int sta;
void *data;
};

--
2.17.1

2021-03-14 23:37:44

by Chun-Kuang Hu

[permalink] [raw]
Subject: [PATCH 2/3] mailbox: mtk-cmdq: Use mailbox rx_callback

rx_callback is a standard mailbox callback mechanism and could cover the
function of proprietary cmdq_task_cb, so use the standard one instead of
the proprietary one. But the client driver has already used cmdq_task_cb,
so keep cmdq_task_cb until all client driver use rx_callback instead of
cmdq_task_cb.

Cc: Jassi Brar <[email protected]>
Cc: Matthias Brugger <[email protected]>
Cc: Houlong Wei <[email protected]>
Cc: Bibby Hsieh <[email protected]>
Cc: Dennis YC Hsieh <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Chun-Kuang Hu <[email protected]>
---
drivers/mailbox/mtk-cmdq-mailbox.c | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/drivers/mailbox/mtk-cmdq-mailbox.c b/drivers/mailbox/mtk-cmdq-mailbox.c
index 3d37c1cd40f1..ef59e2234f22 100644
--- a/drivers/mailbox/mtk-cmdq-mailbox.c
+++ b/drivers/mailbox/mtk-cmdq-mailbox.c
@@ -188,7 +188,10 @@ static void cmdq_task_exec_done(struct cmdq_task *task, int sta)
WARN_ON(cb->cb == (cmdq_async_flush_cb)NULL);
data.sta = sta;
data.data = cb->data;
- cb->cb(data);
+ if (cb->cb)
+ cb->cb(data);
+
+ mbox_chan_received_data(task->thread->chan, &data);

list_del(&task->list_entry);
}
@@ -451,12 +454,13 @@ static int cmdq_mbox_flush(struct mbox_chan *chan, unsigned long timeout)

list_for_each_entry_safe(task, tmp, &thread->task_busy_list,
list_entry) {
+ data.sta = -ECONNABORTED;
+ data.data = cb->data;
cb = &task->pkt->async_cb;
- if (cb->cb) {
- data.sta = -ECONNABORTED;
- data.data = cb->data;
+ if (cb->cb)
cb->cb(data);
- }
+
+ mbox_chan_received_data(task->thread->chan, &data);
list_del(&task->list_entry);
kfree(task);
}
--
2.17.1

2021-03-14 23:38:57

by Chun-Kuang Hu

[permalink] [raw]
Subject: [PATCH 3/3] mailbox: mtk-cmdq: Add struct cmdq_pkt in struct cmdq_cb_data

Current client use 'struct cmdq_pkt' as callback data, so
change 'void *data' to 'struct cmdq_pkt *pkt'. Keep data
until client use pkt instead of data.

Cc: Jassi Brar <[email protected]>
Cc: Matthias Brugger <[email protected]>
Cc: Houlong Wei <[email protected]>
Cc: Bibby Hsieh <[email protected]>
Cc: Dennis YC Hsieh <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Chun-Kuang Hu <[email protected]>
---
drivers/mailbox/mtk-cmdq-mailbox.c | 2 ++
include/linux/mailbox/mtk-cmdq-mailbox.h | 1 +
2 files changed, 3 insertions(+)

diff --git a/drivers/mailbox/mtk-cmdq-mailbox.c b/drivers/mailbox/mtk-cmdq-mailbox.c
index ef59e2234f22..99a9e0787501 100644
--- a/drivers/mailbox/mtk-cmdq-mailbox.c
+++ b/drivers/mailbox/mtk-cmdq-mailbox.c
@@ -188,6 +188,7 @@ static void cmdq_task_exec_done(struct cmdq_task *task, int sta)
WARN_ON(cb->cb == (cmdq_async_flush_cb)NULL);
data.sta = sta;
data.data = cb->data;
+ data.pkt = task->pkt;
if (cb->cb)
cb->cb(data);

@@ -456,6 +457,7 @@ static int cmdq_mbox_flush(struct mbox_chan *chan, unsigned long timeout)
list_entry) {
data.sta = -ECONNABORTED;
data.data = cb->data;
+ data.pkt = task->pkt;
cb = &task->pkt->async_cb;
if (cb->cb)
cb->cb(data);
diff --git a/include/linux/mailbox/mtk-cmdq-mailbox.h b/include/linux/mailbox/mtk-cmdq-mailbox.h
index 2f7d9a37d611..44365aab043c 100644
--- a/include/linux/mailbox/mtk-cmdq-mailbox.h
+++ b/include/linux/mailbox/mtk-cmdq-mailbox.h
@@ -68,6 +68,7 @@ enum cmdq_code {
struct cmdq_cb_data {
int sta;
void *data;
+ struct cmdq_pkt *pkt;
};

typedef void (*cmdq_async_flush_cb)(struct cmdq_cb_data data);
--
2.17.1

2021-04-03 02:45:59

by Chun-Kuang Hu

[permalink] [raw]
Subject: Re: [PATCH 0/3] Refine mtk-cmdq-mailbox callback mechanism

Hi, Jassi:

Chun-Kuang Hu <[email protected]> 於 2021年3月15日 週一 上午7:33寫道:
>
> mtk-cmdq-mailbox use proprietary callback mechanism and proprietary
> error number, but these could be replaced by standard callback
> mechanism and standard error number. In addition, use cmdq_pkt as
> callback data to prevent redundnat assignment.
>
> Because client driver still use proprietary mechanism, so keep
> proprietary mechanism until client driver use the standard one.

How do you think about this series?

Regards,
Chun-Kuang.

>
> Chun-Kuang Hu (3):
> mailbox: mtk-cmdq: Remove cmdq_cb_status
> mailbox: mtk-cmdq: Use mailbox rx_callback
> mailbox: mtk-cmdq: Add struct cmdq_pkt in struct cmdq_cb_data
>
> drivers/mailbox/mtk-cmdq-mailbox.c | 24 +++++++++++++++---------
> include/linux/mailbox/mtk-cmdq-mailbox.h | 8 ++------
> 2 files changed, 17 insertions(+), 15 deletions(-)
>
> --
> 2.17.1
>

2021-05-19 19:31:42

by Chun-Kuang Hu

[permalink] [raw]
Subject: Re: [PATCH 1/3] mailbox: mtk-cmdq: Remove cmdq_cb_status

Hi, Jassi:

Chun-Kuang Hu <[email protected]> 於 2021年3月15日 週一 上午7:33寫道:
>
> cmdq_cb_status is an error status. Use the standard error number
> instead of cmdq_cb_status to prevent status duplication.

How do you think about this series?

Regards,
Chun-Kuang.

>
> Cc: Jassi Brar <[email protected]>
> Cc: Matthias Brugger <[email protected]>
> Cc: Houlong Wei <[email protected]>
> Cc: Bibby Hsieh <[email protected]>
> Cc: Dennis YC Hsieh <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Signed-off-by: Chun-Kuang Hu <[email protected]>
> ---
> drivers/mailbox/mtk-cmdq-mailbox.c | 10 +++++-----
> include/linux/mailbox/mtk-cmdq-mailbox.h | 7 +------
> 2 files changed, 6 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/mailbox/mtk-cmdq-mailbox.c b/drivers/mailbox/mtk-cmdq-mailbox.c
> index 5665b6ea8119..3d37c1cd40f1 100644
> --- a/drivers/mailbox/mtk-cmdq-mailbox.c
> +++ b/drivers/mailbox/mtk-cmdq-mailbox.c
> @@ -180,7 +180,7 @@ static bool cmdq_thread_is_in_wfe(struct cmdq_thread *thread)
> return readl(thread->base + CMDQ_THR_WAIT_TOKEN) & CMDQ_THR_IS_WAITING;
> }
>
> -static void cmdq_task_exec_done(struct cmdq_task *task, enum cmdq_cb_status sta)
> +static void cmdq_task_exec_done(struct cmdq_task *task, int sta)
> {
> struct cmdq_task_cb *cb = &task->pkt->async_cb;
> struct cmdq_cb_data data;
> @@ -244,10 +244,10 @@ static void cmdq_thread_irq_handler(struct cmdq *cmdq,
> curr_task = task;
>
> if (!curr_task || curr_pa == task_end_pa - CMDQ_INST_SIZE) {
> - cmdq_task_exec_done(task, CMDQ_CB_NORMAL);
> + cmdq_task_exec_done(task, 0);
> kfree(task);
> } else if (err) {
> - cmdq_task_exec_done(task, CMDQ_CB_ERROR);
> + cmdq_task_exec_done(task, -ENOEXEC);
> cmdq_task_handle_error(curr_task);
> kfree(task);
> }
> @@ -415,7 +415,7 @@ static void cmdq_mbox_shutdown(struct mbox_chan *chan)
>
> list_for_each_entry_safe(task, tmp, &thread->task_busy_list,
> list_entry) {
> - cmdq_task_exec_done(task, CMDQ_CB_ERROR);
> + cmdq_task_exec_done(task, -ECONNABORTED);
> kfree(task);
> }
>
> @@ -453,7 +453,7 @@ static int cmdq_mbox_flush(struct mbox_chan *chan, unsigned long timeout)
> list_entry) {
> cb = &task->pkt->async_cb;
> if (cb->cb) {
> - data.sta = CMDQ_CB_ERROR;
> + data.sta = -ECONNABORTED;
> data.data = cb->data;
> cb->cb(data);
> }
> diff --git a/include/linux/mailbox/mtk-cmdq-mailbox.h b/include/linux/mailbox/mtk-cmdq-mailbox.h
> index d5a983d65f05..2f7d9a37d611 100644
> --- a/include/linux/mailbox/mtk-cmdq-mailbox.h
> +++ b/include/linux/mailbox/mtk-cmdq-mailbox.h
> @@ -65,13 +65,8 @@ enum cmdq_code {
> CMDQ_CODE_LOGIC = 0xa0,
> };
>
> -enum cmdq_cb_status {
> - CMDQ_CB_NORMAL = 0,
> - CMDQ_CB_ERROR
> -};
> -
> struct cmdq_cb_data {
> - enum cmdq_cb_status sta;
> + int sta;
> void *data;
> };
>
> --
> 2.17.1
>

2021-05-20 13:58:03

by Jassi Brar

[permalink] [raw]
Subject: Re: [PATCH 1/3] mailbox: mtk-cmdq: Remove cmdq_cb_status

On Wed, May 19, 2021 at 6:21 AM Chun-Kuang Hu <[email protected]> wrote:
>
> Hi, Jassi:
>
> Chun-Kuang Hu <[email protected]> 於 2021年3月15日 週一 上午7:33寫道:
> >
> > cmdq_cb_status is an error status. Use the standard error number
> > instead of cmdq_cb_status to prevent status duplication.
>
> How do you think about this series?
>
Hmm, I can't remember the reason I didn't pick. But it was definitely
under "todo" label. I will pick it.
BTW, it helps to see Acked-by's from someone working for the h/w vendor.

thanks.

2021-05-26 06:31:39

by Chun-Kuang Hu

[permalink] [raw]
Subject: Re: [PATCH 0/3] Refine mtk-cmdq-mailbox callback mechanism

+ Yongqiang.

Chun-Kuang Hu <[email protected]> 於 2021年3月15日 週一 上午7:33寫道:
>
> mtk-cmdq-mailbox use proprietary callback mechanism and proprietary
> error number, but these could be replaced by standard callback
> mechanism and standard error number. In addition, use cmdq_pkt as
> callback data to prevent redundnat assignment.
>
> Because client driver still use proprietary mechanism, so keep
> proprietary mechanism until client driver use the standard one.
>
> Chun-Kuang Hu (3):
> mailbox: mtk-cmdq: Remove cmdq_cb_status
> mailbox: mtk-cmdq: Use mailbox rx_callback
> mailbox: mtk-cmdq: Add struct cmdq_pkt in struct cmdq_cb_data
>
> drivers/mailbox/mtk-cmdq-mailbox.c | 24 +++++++++++++++---------
> include/linux/mailbox/mtk-cmdq-mailbox.h | 8 ++------
> 2 files changed, 17 insertions(+), 15 deletions(-)
>
> --
> 2.17.1
>

2021-05-26 06:43:09

by Yongqiang Niu

[permalink] [raw]
Subject: Re: [PATCH 0/3] Refine mtk-cmdq-mailbox callback mechanism


Reviewed-by: Yongqiang Niu <[email protected]>

On Wed, 2021-05-26 at 14:13 +0800, Chun-Kuang Hu wrote:
> + Yongqiang.
>
> Chun-Kuang Hu <[email protected]> 於 2021年3月15日 週一 上午7:33寫道:
> >
> > mtk-cmdq-mailbox use proprietary callback mechanism and proprietary
> > error number, but these could be replaced by standard callback
> > mechanism and standard error number. In addition, use cmdq_pkt as
> > callback data to prevent redundnat assignment.
> >
> > Because client driver still use proprietary mechanism, so keep
> > proprietary mechanism until client driver use the standard one.
> >
> > Chun-Kuang Hu (3):
> > mailbox: mtk-cmdq: Remove cmdq_cb_status
> > mailbox: mtk-cmdq: Use mailbox rx_callback
> > mailbox: mtk-cmdq: Add struct cmdq_pkt in struct cmdq_cb_data
> >
> > drivers/mailbox/mtk-cmdq-mailbox.c | 24 +++++++++++++++---------
> > include/linux/mailbox/mtk-cmdq-mailbox.h | 8 ++------
> > 2 files changed, 17 insertions(+), 15 deletions(-)
> >
> > --
> > 2.17.1
> >

2021-05-26 06:53:36

by Yongqiang Niu

[permalink] [raw]
Subject: Re: [PATCH 3/3] mailbox: mtk-cmdq: Add struct cmdq_pkt in struct cmdq_cb_data

Reviewed-by: Yongqiang Niu <[email protected]>

On Wed, 2021-05-26 at 14:18 +0800, Chun-Kuang Hu wrote:
> + Yongqiang
>
> Chun-Kuang Hu <[email protected]> 於 2021年3月15日 週一 上午7:33寫道:
> >
> > Current client use 'struct cmdq_pkt' as callback data, so
> > change 'void *data' to 'struct cmdq_pkt *pkt'. Keep data
> > until client use pkt instead of data.
> >
> > Cc: Jassi Brar <[email protected]>
> > Cc: Matthias Brugger <[email protected]>
> > Cc: Houlong Wei <[email protected]>
> > Cc: Bibby Hsieh <[email protected]>
> > Cc: Dennis YC Hsieh <[email protected]>
> > Cc: [email protected]
> > Cc: [email protected]
> > Cc: [email protected]
> > Signed-off-by: Chun-Kuang Hu <[email protected]>
> > ---
> > drivers/mailbox/mtk-cmdq-mailbox.c | 2 ++
> > include/linux/mailbox/mtk-cmdq-mailbox.h | 1 +
> > 2 files changed, 3 insertions(+)
> >
> > diff --git a/drivers/mailbox/mtk-cmdq-mailbox.c b/drivers/mailbox/mtk-cmdq-mailbox.c
> > index ef59e2234f22..99a9e0787501 100644
> > --- a/drivers/mailbox/mtk-cmdq-mailbox.c
> > +++ b/drivers/mailbox/mtk-cmdq-mailbox.c
> > @@ -188,6 +188,7 @@ static void cmdq_task_exec_done(struct cmdq_task *task, int sta)
> > WARN_ON(cb->cb == (cmdq_async_flush_cb)NULL);
> > data.sta = sta;
> > data.data = cb->data;
> > + data.pkt = task->pkt;
> > if (cb->cb)
> > cb->cb(data);
> >
> > @@ -456,6 +457,7 @@ static int cmdq_mbox_flush(struct mbox_chan *chan, unsigned long timeout)
> > list_entry) {
> > data.sta = -ECONNABORTED;
> > data.data = cb->data;
> > + data.pkt = task->pkt;
> > cb = &task->pkt->async_cb;
> > if (cb->cb)
> > cb->cb(data);
> > diff --git a/include/linux/mailbox/mtk-cmdq-mailbox.h b/include/linux/mailbox/mtk-cmdq-mailbox.h
> > index 2f7d9a37d611..44365aab043c 100644
> > --- a/include/linux/mailbox/mtk-cmdq-mailbox.h
> > +++ b/include/linux/mailbox/mtk-cmdq-mailbox.h
> > @@ -68,6 +68,7 @@ enum cmdq_code {
> > struct cmdq_cb_data {
> > int sta;
> > void *data;
> > + struct cmdq_pkt *pkt;
> > };
> >
> > typedef void (*cmdq_async_flush_cb)(struct cmdq_cb_data data);
> > --
> > 2.17.1
> >

2021-05-26 07:56:45

by Chun-Kuang Hu

[permalink] [raw]
Subject: Re: [PATCH 2/3] mailbox: mtk-cmdq: Use mailbox rx_callback

+ Yongqiang

Chun-Kuang Hu <[email protected]> 於 2021年3月15日 週一 上午7:33寫道:
>
> rx_callback is a standard mailbox callback mechanism and could cover the
> function of proprietary cmdq_task_cb, so use the standard one instead of
> the proprietary one. But the client driver has already used cmdq_task_cb,
> so keep cmdq_task_cb until all client driver use rx_callback instead of
> cmdq_task_cb.
>
> Cc: Jassi Brar <[email protected]>
> Cc: Matthias Brugger <[email protected]>
> Cc: Houlong Wei <[email protected]>
> Cc: Bibby Hsieh <[email protected]>
> Cc: Dennis YC Hsieh <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Signed-off-by: Chun-Kuang Hu <[email protected]>
> ---
> drivers/mailbox/mtk-cmdq-mailbox.c | 14 +++++++++-----
> 1 file changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/mailbox/mtk-cmdq-mailbox.c b/drivers/mailbox/mtk-cmdq-mailbox.c
> index 3d37c1cd40f1..ef59e2234f22 100644
> --- a/drivers/mailbox/mtk-cmdq-mailbox.c
> +++ b/drivers/mailbox/mtk-cmdq-mailbox.c
> @@ -188,7 +188,10 @@ static void cmdq_task_exec_done(struct cmdq_task *task, int sta)
> WARN_ON(cb->cb == (cmdq_async_flush_cb)NULL);
> data.sta = sta;
> data.data = cb->data;
> - cb->cb(data);
> + if (cb->cb)
> + cb->cb(data);
> +
> + mbox_chan_received_data(task->thread->chan, &data);
>
> list_del(&task->list_entry);
> }
> @@ -451,12 +454,13 @@ static int cmdq_mbox_flush(struct mbox_chan *chan, unsigned long timeout)
>
> list_for_each_entry_safe(task, tmp, &thread->task_busy_list,
> list_entry) {
> + data.sta = -ECONNABORTED;
> + data.data = cb->data;
> cb = &task->pkt->async_cb;
> - if (cb->cb) {
> - data.sta = -ECONNABORTED;
> - data.data = cb->data;
> + if (cb->cb)
> cb->cb(data);
> - }
> +
> + mbox_chan_received_data(task->thread->chan, &data);
> list_del(&task->list_entry);
> kfree(task);
> }
> --
> 2.17.1
>

2021-05-26 08:02:32

by Yongqiang Niu

[permalink] [raw]
Subject: Re: [PATCH 2/3] mailbox: mtk-cmdq: Use mailbox rx_callback

Reviewed-by: Yongqiang Niu <[email protected]>

On Wed, 2021-05-26 at 14:17 +0800, Chun-Kuang Hu wrote:
> + Yongqiang
>
> Chun-Kuang Hu <[email protected]> 於 2021年3月15日 週一 上午7:33寫道:
> >
> > rx_callback is a standard mailbox callback mechanism and could cover the
> > function of proprietary cmdq_task_cb, so use the standard one instead of
> > the proprietary one. But the client driver has already used cmdq_task_cb,
> > so keep cmdq_task_cb until all client driver use rx_callback instead of
> > cmdq_task_cb.
> >
> > Cc: Jassi Brar <[email protected]>
> > Cc: Matthias Brugger <[email protected]>
> > Cc: Houlong Wei <[email protected]>
> > Cc: Bibby Hsieh <[email protected]>
> > Cc: Dennis YC Hsieh <[email protected]>
> > Cc: [email protected]
> > Cc: [email protected]
> > Cc: [email protected]
> > Signed-off-by: Chun-Kuang Hu <[email protected]>
> > ---
> > drivers/mailbox/mtk-cmdq-mailbox.c | 14 +++++++++-----
> > 1 file changed, 9 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/mailbox/mtk-cmdq-mailbox.c b/drivers/mailbox/mtk-cmdq-mailbox.c
> > index 3d37c1cd40f1..ef59e2234f22 100644
> > --- a/drivers/mailbox/mtk-cmdq-mailbox.c
> > +++ b/drivers/mailbox/mtk-cmdq-mailbox.c
> > @@ -188,7 +188,10 @@ static void cmdq_task_exec_done(struct cmdq_task *task, int sta)
> > WARN_ON(cb->cb == (cmdq_async_flush_cb)NULL);
> > data.sta = sta;
> > data.data = cb->data;
> > - cb->cb(data);
> > + if (cb->cb)
> > + cb->cb(data);
> > +
> > + mbox_chan_received_data(task->thread->chan, &data);
> >
> > list_del(&task->list_entry);
> > }
> > @@ -451,12 +454,13 @@ static int cmdq_mbox_flush(struct mbox_chan *chan, unsigned long timeout)
> >
> > list_for_each_entry_safe(task, tmp, &thread->task_busy_list,
> > list_entry) {
> > + data.sta = -ECONNABORTED;
> > + data.data = cb->data;
> > cb = &task->pkt->async_cb;
> > - if (cb->cb) {
> > - data.sta = -ECONNABORTED;
> > - data.data = cb->data;
> > + if (cb->cb)
> > cb->cb(data);
> > - }
> > +
> > + mbox_chan_received_data(task->thread->chan, &data);
> > list_del(&task->list_entry);
> > kfree(task);
> > }
> > --
> > 2.17.1
> >

2021-05-26 11:25:54

by Chun-Kuang Hu

[permalink] [raw]
Subject: Re: [PATCH 3/3] mailbox: mtk-cmdq: Add struct cmdq_pkt in struct cmdq_cb_data

+ Yongqiang

Chun-Kuang Hu <[email protected]> 於 2021年3月15日 週一 上午7:33寫道:
>
> Current client use 'struct cmdq_pkt' as callback data, so
> change 'void *data' to 'struct cmdq_pkt *pkt'. Keep data
> until client use pkt instead of data.
>
> Cc: Jassi Brar <[email protected]>
> Cc: Matthias Brugger <[email protected]>
> Cc: Houlong Wei <[email protected]>
> Cc: Bibby Hsieh <[email protected]>
> Cc: Dennis YC Hsieh <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Signed-off-by: Chun-Kuang Hu <[email protected]>
> ---
> drivers/mailbox/mtk-cmdq-mailbox.c | 2 ++
> include/linux/mailbox/mtk-cmdq-mailbox.h | 1 +
> 2 files changed, 3 insertions(+)
>
> diff --git a/drivers/mailbox/mtk-cmdq-mailbox.c b/drivers/mailbox/mtk-cmdq-mailbox.c
> index ef59e2234f22..99a9e0787501 100644
> --- a/drivers/mailbox/mtk-cmdq-mailbox.c
> +++ b/drivers/mailbox/mtk-cmdq-mailbox.c
> @@ -188,6 +188,7 @@ static void cmdq_task_exec_done(struct cmdq_task *task, int sta)
> WARN_ON(cb->cb == (cmdq_async_flush_cb)NULL);
> data.sta = sta;
> data.data = cb->data;
> + data.pkt = task->pkt;
> if (cb->cb)
> cb->cb(data);
>
> @@ -456,6 +457,7 @@ static int cmdq_mbox_flush(struct mbox_chan *chan, unsigned long timeout)
> list_entry) {
> data.sta = -ECONNABORTED;
> data.data = cb->data;
> + data.pkt = task->pkt;
> cb = &task->pkt->async_cb;
> if (cb->cb)
> cb->cb(data);
> diff --git a/include/linux/mailbox/mtk-cmdq-mailbox.h b/include/linux/mailbox/mtk-cmdq-mailbox.h
> index 2f7d9a37d611..44365aab043c 100644
> --- a/include/linux/mailbox/mtk-cmdq-mailbox.h
> +++ b/include/linux/mailbox/mtk-cmdq-mailbox.h
> @@ -68,6 +68,7 @@ enum cmdq_code {
> struct cmdq_cb_data {
> int sta;
> void *data;
> + struct cmdq_pkt *pkt;
> };
>
> typedef void (*cmdq_async_flush_cb)(struct cmdq_cb_data data);
> --
> 2.17.1
>

2021-05-26 11:25:54

by Chun-Kuang Hu

[permalink] [raw]
Subject: Re: [PATCH 1/3] mailbox: mtk-cmdq: Remove cmdq_cb_status

+ Yongqiang

Chun-Kuang Hu <[email protected]> 於 2021年3月15日 週一 上午7:33寫道:
>
> cmdq_cb_status is an error status. Use the standard error number
> instead of cmdq_cb_status to prevent status duplication.
>
> Cc: Jassi Brar <[email protected]>
> Cc: Matthias Brugger <[email protected]>
> Cc: Houlong Wei <[email protected]>
> Cc: Bibby Hsieh <[email protected]>
> Cc: Dennis YC Hsieh <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Signed-off-by: Chun-Kuang Hu <[email protected]>
> ---
> drivers/mailbox/mtk-cmdq-mailbox.c | 10 +++++-----
> include/linux/mailbox/mtk-cmdq-mailbox.h | 7 +------
> 2 files changed, 6 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/mailbox/mtk-cmdq-mailbox.c b/drivers/mailbox/mtk-cmdq-mailbox.c
> index 5665b6ea8119..3d37c1cd40f1 100644
> --- a/drivers/mailbox/mtk-cmdq-mailbox.c
> +++ b/drivers/mailbox/mtk-cmdq-mailbox.c
> @@ -180,7 +180,7 @@ static bool cmdq_thread_is_in_wfe(struct cmdq_thread *thread)
> return readl(thread->base + CMDQ_THR_WAIT_TOKEN) & CMDQ_THR_IS_WAITING;
> }
>
> -static void cmdq_task_exec_done(struct cmdq_task *task, enum cmdq_cb_status sta)
> +static void cmdq_task_exec_done(struct cmdq_task *task, int sta)
> {
> struct cmdq_task_cb *cb = &task->pkt->async_cb;
> struct cmdq_cb_data data;
> @@ -244,10 +244,10 @@ static void cmdq_thread_irq_handler(struct cmdq *cmdq,
> curr_task = task;
>
> if (!curr_task || curr_pa == task_end_pa - CMDQ_INST_SIZE) {
> - cmdq_task_exec_done(task, CMDQ_CB_NORMAL);
> + cmdq_task_exec_done(task, 0);
> kfree(task);
> } else if (err) {
> - cmdq_task_exec_done(task, CMDQ_CB_ERROR);
> + cmdq_task_exec_done(task, -ENOEXEC);
> cmdq_task_handle_error(curr_task);
> kfree(task);
> }
> @@ -415,7 +415,7 @@ static void cmdq_mbox_shutdown(struct mbox_chan *chan)
>
> list_for_each_entry_safe(task, tmp, &thread->task_busy_list,
> list_entry) {
> - cmdq_task_exec_done(task, CMDQ_CB_ERROR);
> + cmdq_task_exec_done(task, -ECONNABORTED);
> kfree(task);
> }
>
> @@ -453,7 +453,7 @@ static int cmdq_mbox_flush(struct mbox_chan *chan, unsigned long timeout)
> list_entry) {
> cb = &task->pkt->async_cb;
> if (cb->cb) {
> - data.sta = CMDQ_CB_ERROR;
> + data.sta = -ECONNABORTED;
> data.data = cb->data;
> cb->cb(data);
> }
> diff --git a/include/linux/mailbox/mtk-cmdq-mailbox.h b/include/linux/mailbox/mtk-cmdq-mailbox.h
> index d5a983d65f05..2f7d9a37d611 100644
> --- a/include/linux/mailbox/mtk-cmdq-mailbox.h
> +++ b/include/linux/mailbox/mtk-cmdq-mailbox.h
> @@ -65,13 +65,8 @@ enum cmdq_code {
> CMDQ_CODE_LOGIC = 0xa0,
> };
>
> -enum cmdq_cb_status {
> - CMDQ_CB_NORMAL = 0,
> - CMDQ_CB_ERROR
> -};
> -
> struct cmdq_cb_data {
> - enum cmdq_cb_status sta;
> + int sta;
> void *data;
> };
>
> --
> 2.17.1
>

2021-05-26 11:26:25

by Yongqiang Niu

[permalink] [raw]
Subject: Re: [PATCH 1/3] mailbox: mtk-cmdq: Remove cmdq_cb_status


Reviewed-by: Yongqiang Niu <[email protected]>

On Wed, 2021-05-26 at 14:15 +0800, Chun-Kuang Hu wrote:
> + Yongqiang
>
> Chun-Kuang Hu <[email protected]> 於 2021年3月15日 週一 上午7:33寫道:
> >
> > cmdq_cb_status is an error status. Use the standard error number
> > instead of cmdq_cb_status to prevent status duplication.
> >
> > Cc: Jassi Brar <[email protected]>
> > Cc: Matthias Brugger <[email protected]>
> > Cc: Houlong Wei <[email protected]>
> > Cc: Bibby Hsieh <[email protected]>
> > Cc: Dennis YC Hsieh <[email protected]>
> > Cc: [email protected]
> > Cc: [email protected]
> > Cc: [email protected]
> > Signed-off-by: Chun-Kuang Hu <[email protected]>
> > ---
> > drivers/mailbox/mtk-cmdq-mailbox.c | 10 +++++-----
> > include/linux/mailbox/mtk-cmdq-mailbox.h | 7 +------
> > 2 files changed, 6 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/mailbox/mtk-cmdq-mailbox.c b/drivers/mailbox/mtk-cmdq-mailbox.c
> > index 5665b6ea8119..3d37c1cd40f1 100644
> > --- a/drivers/mailbox/mtk-cmdq-mailbox.c
> > +++ b/drivers/mailbox/mtk-cmdq-mailbox.c
> > @@ -180,7 +180,7 @@ static bool cmdq_thread_is_in_wfe(struct cmdq_thread *thread)
> > return readl(thread->base + CMDQ_THR_WAIT_TOKEN) & CMDQ_THR_IS_WAITING;
> > }
> >
> > -static void cmdq_task_exec_done(struct cmdq_task *task, enum cmdq_cb_status sta)
> > +static void cmdq_task_exec_done(struct cmdq_task *task, int sta)
> > {
> > struct cmdq_task_cb *cb = &task->pkt->async_cb;
> > struct cmdq_cb_data data;
> > @@ -244,10 +244,10 @@ static void cmdq_thread_irq_handler(struct cmdq *cmdq,
> > curr_task = task;
> >
> > if (!curr_task || curr_pa == task_end_pa - CMDQ_INST_SIZE) {
> > - cmdq_task_exec_done(task, CMDQ_CB_NORMAL);
> > + cmdq_task_exec_done(task, 0);
> > kfree(task);
> > } else if (err) {
> > - cmdq_task_exec_done(task, CMDQ_CB_ERROR);
> > + cmdq_task_exec_done(task, -ENOEXEC);
> > cmdq_task_handle_error(curr_task);
> > kfree(task);
> > }
> > @@ -415,7 +415,7 @@ static void cmdq_mbox_shutdown(struct mbox_chan *chan)
> >
> > list_for_each_entry_safe(task, tmp, &thread->task_busy_list,
> > list_entry) {
> > - cmdq_task_exec_done(task, CMDQ_CB_ERROR);
> > + cmdq_task_exec_done(task, -ECONNABORTED);
> > kfree(task);
> > }
> >
> > @@ -453,7 +453,7 @@ static int cmdq_mbox_flush(struct mbox_chan *chan, unsigned long timeout)
> > list_entry) {
> > cb = &task->pkt->async_cb;
> > if (cb->cb) {
> > - data.sta = CMDQ_CB_ERROR;
> > + data.sta = -ECONNABORTED;
> > data.data = cb->data;
> > cb->cb(data);
> > }
> > diff --git a/include/linux/mailbox/mtk-cmdq-mailbox.h b/include/linux/mailbox/mtk-cmdq-mailbox.h
> > index d5a983d65f05..2f7d9a37d611 100644
> > --- a/include/linux/mailbox/mtk-cmdq-mailbox.h
> > +++ b/include/linux/mailbox/mtk-cmdq-mailbox.h
> > @@ -65,13 +65,8 @@ enum cmdq_code {
> > CMDQ_CODE_LOGIC = 0xa0,
> > };
> >
> > -enum cmdq_cb_status {
> > - CMDQ_CB_NORMAL = 0,
> > - CMDQ_CB_ERROR
> > -};
> > -
> > struct cmdq_cb_data {
> > - enum cmdq_cb_status sta;
> > + int sta;
> > void *data;
> > };
> >
> > --
> > 2.17.1
> >

2021-06-17 09:07:27

by Chun-Kuang Hu

[permalink] [raw]
Subject: Re: [PATCH 1/3] mailbox: mtk-cmdq: Remove cmdq_cb_status

Hi, Jassi:

Jassi Brar <[email protected]> 於 2021年5月20日 週四 下午9:55寫道:
>
> On Wed, May 19, 2021 at 6:21 AM Chun-Kuang Hu <[email protected]> wrote:
> >
> > Hi, Jassi:
> >
> > Chun-Kuang Hu <[email protected]> 於 2021年3月15日 週一 上午7:33寫道:
> > >
> > > cmdq_cb_status is an error status. Use the standard error number
> > > instead of cmdq_cb_status to prevent status duplication.
> >
> > How do you think about this series?
> >
> Hmm, I can't remember the reason I didn't pick. But it was definitely
> under "todo" label. I will pick it.
> BTW, it helps to see Acked-by's from someone working for the h/w vendor.

Yongqiang is working for the h/w vendor and have reviewed this series,
does this get any help?

Regards,
Chun-Kuang.

>
> thanks.