2022-06-08 15:16:41

by Chun-Kuang Hu

[permalink] [raw]
Subject: [PATCH] mailbox: mtk-cmdq: Remove proprietary cmdq_task_cb

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. Client driver has changed to use standard
rx_callback, so remove proprietary cmdq_task_cb.

Signed-off-by: Chun-Kuang Hu <[email protected]>
---
drivers/mailbox/mtk-cmdq-mailbox.c | 11 -----------
include/linux/mailbox/mtk-cmdq-mailbox.h | 10 ----------
2 files changed, 21 deletions(-)

diff --git a/drivers/mailbox/mtk-cmdq-mailbox.c b/drivers/mailbox/mtk-cmdq-mailbox.c
index 2578e5aaa935..9465f9081515 100644
--- a/drivers/mailbox/mtk-cmdq-mailbox.c
+++ b/drivers/mailbox/mtk-cmdq-mailbox.c
@@ -192,15 +192,10 @@ static bool cmdq_thread_is_in_wfe(struct cmdq_thread *thread)

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;

data.sta = sta;
- data.data = cb->data;
data.pkt = task->pkt;
- if (cb->cb)
- cb->cb(data);
-
mbox_chan_received_data(task->thread->chan, &data);

list_del(&task->list_entry);
@@ -448,7 +443,6 @@ static void cmdq_mbox_shutdown(struct mbox_chan *chan)
static int cmdq_mbox_flush(struct mbox_chan *chan, unsigned long timeout)
{
struct cmdq_thread *thread = (struct cmdq_thread *)chan->con_priv;
- struct cmdq_task_cb *cb;
struct cmdq_cb_data data;
struct cmdq *cmdq = dev_get_drvdata(chan->mbox->dev);
struct cmdq_task *task, *tmp;
@@ -465,13 +459,8 @@ 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) {
- cb = &task->pkt->async_cb;
data.sta = -ECONNABORTED;
- data.data = cb->data;
data.pkt = task->pkt;
- if (cb->cb)
- cb->cb(data);
-
mbox_chan_received_data(task->thread->chan, &data);
list_del(&task->list_entry);
kfree(task);
diff --git a/include/linux/mailbox/mtk-cmdq-mailbox.h b/include/linux/mailbox/mtk-cmdq-mailbox.h
index 44365aab043c..a8f0070c7aa9 100644
--- a/include/linux/mailbox/mtk-cmdq-mailbox.h
+++ b/include/linux/mailbox/mtk-cmdq-mailbox.h
@@ -67,24 +67,14 @@ 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);
-
-struct cmdq_task_cb {
- cmdq_async_flush_cb cb;
- void *data;
-};
-
struct cmdq_pkt {
void *va_base;
dma_addr_t pa_base;
size_t cmd_buf_size; /* command occupied size */
size_t buf_size; /* real buffer size */
- struct cmdq_task_cb cb;
- struct cmdq_task_cb async_cb;
void *cl;
};

--
2.25.1


2022-06-13 18:37:21

by Matthias Brugger

[permalink] [raw]
Subject: Re: [PATCH] mailbox: mtk-cmdq: Remove proprietary cmdq_task_cb



On 08/06/2022 16:40, Chun-Kuang Hu wrote:
> 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. Client driver has changed to use standard
> rx_callback, so remove proprietary cmdq_task_cb.
>
> Signed-off-by: Chun-Kuang Hu <[email protected]>

Reviewed-by: Matthias Brugger <[email protected]>

> ---
> drivers/mailbox/mtk-cmdq-mailbox.c | 11 -----------
> include/linux/mailbox/mtk-cmdq-mailbox.h | 10 ----------
> 2 files changed, 21 deletions(-)
>
> diff --git a/drivers/mailbox/mtk-cmdq-mailbox.c b/drivers/mailbox/mtk-cmdq-mailbox.c
> index 2578e5aaa935..9465f9081515 100644
> --- a/drivers/mailbox/mtk-cmdq-mailbox.c
> +++ b/drivers/mailbox/mtk-cmdq-mailbox.c
> @@ -192,15 +192,10 @@ static bool cmdq_thread_is_in_wfe(struct cmdq_thread *thread)
>
> 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;
>
> data.sta = sta;
> - data.data = cb->data;
> data.pkt = task->pkt;
> - if (cb->cb)
> - cb->cb(data);
> -
> mbox_chan_received_data(task->thread->chan, &data);
>
> list_del(&task->list_entry);
> @@ -448,7 +443,6 @@ static void cmdq_mbox_shutdown(struct mbox_chan *chan)
> static int cmdq_mbox_flush(struct mbox_chan *chan, unsigned long timeout)
> {
> struct cmdq_thread *thread = (struct cmdq_thread *)chan->con_priv;
> - struct cmdq_task_cb *cb;
> struct cmdq_cb_data data;
> struct cmdq *cmdq = dev_get_drvdata(chan->mbox->dev);
> struct cmdq_task *task, *tmp;
> @@ -465,13 +459,8 @@ 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) {
> - cb = &task->pkt->async_cb;
> data.sta = -ECONNABORTED;
> - data.data = cb->data;
> data.pkt = task->pkt;
> - if (cb->cb)
> - cb->cb(data);
> -
> mbox_chan_received_data(task->thread->chan, &data);
> list_del(&task->list_entry);
> kfree(task);
> diff --git a/include/linux/mailbox/mtk-cmdq-mailbox.h b/include/linux/mailbox/mtk-cmdq-mailbox.h
> index 44365aab043c..a8f0070c7aa9 100644
> --- a/include/linux/mailbox/mtk-cmdq-mailbox.h
> +++ b/include/linux/mailbox/mtk-cmdq-mailbox.h
> @@ -67,24 +67,14 @@ 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);
> -
> -struct cmdq_task_cb {
> - cmdq_async_flush_cb cb;
> - void *data;
> -};
> -
> struct cmdq_pkt {
> void *va_base;
> dma_addr_t pa_base;
> size_t cmd_buf_size; /* command occupied size */
> size_t buf_size; /* real buffer size */
> - struct cmdq_task_cb cb;
> - struct cmdq_task_cb async_cb;
> void *cl;
> };
>

2022-06-13 18:59:01

by Matthias Brugger

[permalink] [raw]
Subject: Re: [PATCH] mailbox: mtk-cmdq: Remove proprietary cmdq_task_cb



On 10/06/2022 04:49, moudy.ho wrote:
> On Wed, 2022-06-08 at 22:40 +0800, Chun-Kuang Hu wrote:
>> 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. Client driver has changed to use standard
>> rx_callback, so remove proprietary cmdq_task_cb.
>>
>> Signed-off-by: Chun-Kuang Hu <[email protected]>
>
> Depends on:
>
> https://patchwork.kernel.org/project/linux-mediatek/patch/[email protected]/
>
> Tested-by: Moudy Ho <[email protected]>
>

Thanks for helping by testing this, although:

Your message makes me think that you didn't test against v5.19-rc1 nor against
v5.18 (!) based kernel but something older. Giving a Tested-by tag to an older
kernel with lots of out-of-tree patches on top can be critical as the kernel
evolves and changes pretty much over time.
Upstream development should always be done against an upstream kernel, as
otherwise bugs in the upstream kernel could stay unnoticed and show up as
"regressions" later on, when you update your base kernel version.

Best regards,
Matthias

>> ---
>> drivers/mailbox/mtk-cmdq-mailbox.c | 11 -----------
>> include/linux/mailbox/mtk-cmdq-mailbox.h | 10 ----------
>> 2 files changed, 21 deletions(-)
>>
>> diff --git a/drivers/mailbox/mtk-cmdq-mailbox.c
>> b/drivers/mailbox/mtk-cmdq-mailbox.c
>> index 2578e5aaa935..9465f9081515 100644
>> --- a/drivers/mailbox/mtk-cmdq-mailbox.c
>> +++ b/drivers/mailbox/mtk-cmdq-mailbox.c
>> @@ -192,15 +192,10 @@ static bool cmdq_thread_is_in_wfe(struct
>> cmdq_thread *thread)
>>
>> 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;
>>
>> data.sta = sta;
>> - data.data = cb->data;
>> data.pkt = task->pkt;
>> - if (cb->cb)
>> - cb->cb(data);
>> -
>> mbox_chan_received_data(task->thread->chan, &data);
>>
>> list_del(&task->list_entry);
>> @@ -448,7 +443,6 @@ static void cmdq_mbox_shutdown(struct mbox_chan
>> *chan)
>> static int cmdq_mbox_flush(struct mbox_chan *chan, unsigned long
>> timeout)
>> {
>> struct cmdq_thread *thread = (struct cmdq_thread *)chan-
>>> con_priv;
>> - struct cmdq_task_cb *cb;
>> struct cmdq_cb_data data;
>> struct cmdq *cmdq = dev_get_drvdata(chan->mbox->dev);
>> struct cmdq_task *task, *tmp;
>> @@ -465,13 +459,8 @@ 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) {
>> - cb = &task->pkt->async_cb;
>> data.sta = -ECONNABORTED;
>> - data.data = cb->data;
>> data.pkt = task->pkt;
>> - if (cb->cb)
>> - cb->cb(data);
>> -
>> mbox_chan_received_data(task->thread->chan, &data);
>> list_del(&task->list_entry);
>> kfree(task);
>> diff --git a/include/linux/mailbox/mtk-cmdq-mailbox.h
>> b/include/linux/mailbox/mtk-cmdq-mailbox.h
>> index 44365aab043c..a8f0070c7aa9 100644
>> --- a/include/linux/mailbox/mtk-cmdq-mailbox.h
>> +++ b/include/linux/mailbox/mtk-cmdq-mailbox.h
>> @@ -67,24 +67,14 @@ 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);
>> -
>> -struct cmdq_task_cb {
>> - cmdq_async_flush_cb cb;
>> - void *data;
>> -};
>> -
>> struct cmdq_pkt {
>> void *va_base;
>> dma_addr_t pa_base;
>> size_t cmd_buf_size; /* command occupied
>> size */
>> size_t buf_size; /* real buffer size */
>> - struct cmdq_task_cb cb;
>> - struct cmdq_task_cb async_cb;
>> void *cl;
>> };
>>
>