lkml.org 
[lkml]   [2022]   [Oct]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH v8, 3/4] mailbox: mtk-cmdq: add gce ddr enable support flow
From
Il 04/10/22 11:39, yongqiang.niu ha scritto:
> On Mon, 2022-10-03 at 16:54 +0200, AngeloGioacchino Del Regno wrote:
>> Il 30/09/22 18:06, Yongqiang Niu ha scritto:
>>> add gce ddr enable control flow when gce suspend/resume
>>>
>>> Signed-off-by: Yongqiang Niu <yongqiang.niu@mediatek.com>
>>> ---
>>> drivers/mailbox/mtk-cmdq-mailbox.c | 22 ++++++++++++++++++++++
>>> 1 file changed, 22 insertions(+)
>>>
>>> diff --git a/drivers/mailbox/mtk-cmdq-mailbox.c
>>> b/drivers/mailbox/mtk-cmdq-mailbox.c
>>> index 04eb44d89119..2db82ff838ed 100644
>>> --- a/drivers/mailbox/mtk-cmdq-mailbox.c
>>> +++ b/drivers/mailbox/mtk-cmdq-mailbox.c
>>> @@ -94,6 +94,18 @@ struct gce_plat {
>>> u32 gce_num;
>>> };
>>>
>>> +static void cmdq_sw_ddr_enable(struct cmdq *cmdq, bool enable)
>>> +{
>>> + WARN_ON(clk_bulk_enable(cmdq->gce_num, cmdq->clocks));
>>> +
>>> + if (enable)
>>> + writel(GCE_DDR_EN | GCE_CTRL_BY_SW, cmdq->base +
>>> GCE_GCTL_VALUE);
>>
>> My only concern here is about the previous value stored in the
>> GCE_GCTL_VALUE
>> register, as you're overwriting it in its entirety with
>> GCE_DDR_EN | GCE_CTRL_BY_SW.
>>
>> Can you guarantee that this register is not pre-initialized with some
>> value,
>> and that these are the only bits to be `1` in this register?
>>
>> Otherwise, you will have to readl and modify the bits instead... by
>> the way,
>> if this register doesn't get any changes during runtime, you may
>> cache it
>> at probe time to avoid reading it for every suspend/resume operation.
>>
>> Regards,
>> Angelo
>>
>>
>
> 0x48[2:0] means control by software
> 0x48[18:16] means ddr enable
> 0x48[2:0] is pre-condition of 0x48[18:16].
> if we want set 0x48[18:16] ddr enable, 0x48[2:0] must be set at same
> time.
> and only these bits is useful, other bits is useless bits
>
> we need set 0x48[18:16] to 0 disable gce access ddr when suspend.
> and set 0x48[18:16] to 0x7 enable gce access ddr when resume, there
> will be cmdq client send task to process.
> this control flow should controlled in suspend/resume flow.
>
>

That's perfect. Thanks for the explanation.

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>


\
 
 \ /
  Last update: 2022-10-04 11:56    [W:0.181 / U:0.164 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site