lkml.org 
[lkml]   [2022]   [Oct]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH v9, 2/4] mailbox: mtk-cmdq: add gce software ddr enable private data
From
Il 07/10/22 03:51, yongqiang.niu ha scritto:
> On Thu, 2022-10-06 at 11:29 +0200, AngeloGioacchino Del Regno wrote:
>> Il 06/10/22 06:34, Yongqiang Niu ha scritto:
>>> if gce work control by software, we need set software enable
>>> for MT8186 Soc
>>>
>>> there is a handshake flow between gce and ddr hardware,
>>> if not set ddr enable flag of gce, ddr will fall into idle
>>> mode, then gce instructions will not process done.
>>> we need set this flag of gce to tell ddr when gce is idle or busy
>>> controlled by software flow.
>>>
>>> 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
>>>
>>> Signed-off-by: Yongqiang Niu <yongqiang.niu@mediatek.com>
>>> ---
>>> drivers/mailbox/mtk-cmdq-mailbox.c | 7 +++++++
>>> 1 file changed, 7 insertions(+)
>>>
>>> diff --git a/drivers/mailbox/mtk-cmdq-mailbox.c
>>> b/drivers/mailbox/mtk-cmdq-mailbox.c
>>> index c3cb24f51699..04eb44d89119 100644
>>> --- a/drivers/mailbox/mtk-cmdq-mailbox.c
>>> +++ b/drivers/mailbox/mtk-cmdq-mailbox.c
>>> @@ -39,6 +39,7 @@
>>>
>>> #define GCE_GCTL_VALUE 0x48
>>> #define GCE_CTRL_BY_SW GENMASK(2, 0)
>>> +#define GCE_DDR_EN GENMASK(18, 16)
>>>
>>> #define CMDQ_THR_ACTIVE_SLOT_CYCLES 0x3200
>>> #define CMDQ_THR_ENABLED 0x1
>>> @@ -81,6 +82,7 @@ struct cmdq {
>>> bool suspended;
>>> u8 shift_pa;
>>> bool control_by_sw;
>>> + bool sw_ddr_en;
>>> u32 gce_num;
>>> };
>>>
>>> @@ -88,6 +90,7 @@ struct gce_plat {
>>> u32 thread_nr;
>>> u8 shift;
>>> bool control_by_sw;
>>> + bool sw_ddr_en;
>>> u32 gce_num;
>>> };
>>>
>>> @@ -132,6 +135,9 @@ static void cmdq_init(struct cmdq *cmdq)
>>> if (cmdq->control_by_sw)
>>> writel(GCE_CTRL_BY_SW, cmdq->base + GCE_GCTL_VALUE);
>>>
>>> + if (cmdq->sw_ddr_en)
>>> + writel(GCE_DDR_EN | GCE_CTRL_BY_SW, cmdq->base +
>>> GCE_GCTL_VALUE);
>>> +
>>
>> No. That's redundant.
>> Here's a better way:
>>
>> u32 gctl_regval = 0;
>>
>> if (cmdq->control_by_sw)
>> gctl_regval = GCE_CTRL_BY_SW;
>> if (cmdq->sw_ddr_en)
>> gctl_regval |= GCE_DDR_EN;
>>
>> if (val)
>> writel(gctl_regval, cmdq->base + GCE_GCTL_VALUE);
>>
>> Regards,
>> Angelo
>
> thanks very much for your advise.
> shall i separate this into two patches?
> 1st one is
> u32 gctl_regval = 0;
> if (cmdq->control_by_sw)
>> gctl_regval = GCE_CTRL_BY_SW;
>> if (val)
>> writel(gctl_regval, cmdq->base + GCE_GCTL_VALUE);
>
>
>
> 2nd just add this
> if (cmdq->sw_ddr_en)
>> gctl_regval |= GCE_DDR_EN;
>
> or one patch is ok?
>

One patch is ok.

Thanks,
Angelo

>>
>>> writel(CMDQ_THR_ACTIVE_SLOT_CYCLES, cmdq->base +
>>> CMDQ_THR_SLOT_CYCLES);
>>> for (i = 0; i <= CMDQ_MAX_EVENT; i++)
>>> writel(i, cmdq->base + CMDQ_SYNC_TOKEN_UPDATE);
>>> @@ -545,6 +551,7 @@ static int cmdq_probe(struct platform_device
>>> *pdev)
>>> cmdq->thread_nr = plat_data->thread_nr;
>>> cmdq->shift_pa = plat_data->shift;
>>> cmdq->control_by_sw = plat_data->control_by_sw;
>>> + cmdq->sw_ddr_en = plat_data->sw_ddr_en;
>>> cmdq->gce_num = plat_data->gce_num;
>>> cmdq->irq_mask = GENMASK(cmdq->thread_nr - 1, 0);
>>> err = devm_request_irq(dev, cmdq->irq, cmdq_irq_handler,
>>> IRQF_SHARED,
>>
>>
>>
>

\
 
 \ /
  Last update: 2022-10-07 10:57    [W:0.041 / U:0.504 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site