Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <606c3409-0e27-e993-8657-0c8d36a16a97@gmail.com>
Date:   Mon, 13 Jun 2022 17:52:42 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.8.1
Content-Language: en-US
To:     "moudy.ho" <moudy.ho@mediatek.com>,
        Chun-Kuang Hu <chunkuang.hu@kernel.org>,
        Jassi Brar <jassisinghbrar@gmail.com>,
        linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
        linux-mediatek@lists.infradead.org
References: <20220608144055.27562-1-chunkuang.hu@kernel.org>
 <55b84b36832b0da24c267d06a43b9db4ee345176.camel@mediatek.com>
From:   Matthias Brugger <matthias.bgg@gmail.com>
Subject: Re: [PATCH] mailbox: mtk-cmdq: Remove proprietary cmdq_task_cb
In-Reply-To: <55b84b36832b0da24c267d06a43b9db4ee345176.camel@mediatek.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk


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 <chunkuang.hu@kernel.org>
> 
> Depends on:
> 
> https://patchwork.kernel.org/project/linux-mediatek/patch/1650102868-26219-1-git-send-email-chunkuang.hu@kernel.org/
> 
> Tested-by: Moudy Ho <Moudy.Ho@mediatek.com>
> 

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;
>>   };
>>   
>