Re: [PATCH v8, 3/4] mailbox: mtk-cmdq: add gce ddr enable support flow

[yongqiang.niu <yongqiang.niu@mediatek.com>]

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.



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel