* [PATCH] mailbox: mtk-cmdq: Refine GCE_GCTL_VALUE setting
@ 2025-02-24 5:01 Jason-JH Lin
2025-02-24 6:04 ` CK Hu (胡俊光)
0 siblings, 1 reply; 3+ messages in thread
From: Jason-JH Lin @ 2025-02-24 5:01 UTC (permalink / raw)
To: Jassi Brar, Chun-Kuang Hu, AngeloGioacchino Del Regno
Cc: Jason-JH . Lin, Nancy Lin, Singo Chang, Yongqiang Niu,
Xiandong Wang, Sirius Wang, Xavier Chang, Fei Shao, Chen-yu Tsai,
Pin-yen Lin, linux-kernel, linux-mediatek, linux-arm-kernel,
Project_Global_Chrome_Upstream_Group
Add cmdq_gctl_value_toggle() to configure GCE_CTRL_BY_SW and GCE_DDR_EN
together in same the GCE_GCTL_VALUE register.
Move this function into cmdq_runtime_resume() and cmdq_runtime_suspend()
to ensure it can be called when the GCE clock is enabled.
Fixes: 7abd037aa581 ("mailbox: mtk-cmdq: add gce ddr enable support flow")
Signed-off-by: Jason-JH Lin <jason-jh.lin@mediatek.com>
---
drivers/mailbox/mtk-cmdq-mailbox.c | 41 +++++++++++++-----------------
1 file changed, 17 insertions(+), 24 deletions(-)
diff --git a/drivers/mailbox/mtk-cmdq-mailbox.c b/drivers/mailbox/mtk-cmdq-mailbox.c
index d186865b8dce..be17697d7785 100644
--- a/drivers/mailbox/mtk-cmdq-mailbox.c
+++ b/drivers/mailbox/mtk-cmdq-mailbox.c
@@ -92,16 +92,17 @@ struct gce_plat {
u32 gce_num;
};
-static void cmdq_sw_ddr_enable(struct cmdq *cmdq, bool enable)
+static void cmdq_gctl_value_toggle(struct cmdq *cmdq, bool ddr_enable)
{
- WARN_ON(clk_bulk_enable(cmdq->pdata->gce_num, cmdq->clocks));
+ u32 val = (cmdq->pdata->control_by_sw) ? GCE_CTRL_BY_SW : 0;
- if (enable)
- writel(GCE_DDR_EN | GCE_CTRL_BY_SW, cmdq->base + GCE_GCTL_VALUE);
- else
- writel(GCE_CTRL_BY_SW, cmdq->base + GCE_GCTL_VALUE);
+ if (!cmdq->pdata->control_by_sw && !cmdq->pdata->sw_ddr_en)
+ return;
- clk_bulk_disable(cmdq->pdata->gce_num, cmdq->clocks);
+ if (cmdq->pdata->sw_ddr_en && ddr_enable)
+ val |= GCE_DDR_EN;
+
+ writel(val, cmdq->base + GCE_GCTL_VALUE);
}
u8 cmdq_get_shift_pa(struct mbox_chan *chan)
@@ -140,16 +141,10 @@ static void cmdq_thread_resume(struct cmdq_thread *thread)
static void cmdq_init(struct cmdq *cmdq)
{
int i;
- u32 gctl_regval = 0;
WARN_ON(clk_bulk_enable(cmdq->pdata->gce_num, cmdq->clocks));
- if (cmdq->pdata->control_by_sw)
- gctl_regval = GCE_CTRL_BY_SW;
- if (cmdq->pdata->sw_ddr_en)
- gctl_regval |= GCE_DDR_EN;
- if (gctl_regval)
- writel(gctl_regval, cmdq->base + GCE_GCTL_VALUE);
+ cmdq_gctl_value_toggle(cmdq, true);
writel(CMDQ_THR_ACTIVE_SLOT_CYCLES, cmdq->base + CMDQ_THR_SLOT_CYCLES);
for (i = 0; i <= CMDQ_MAX_EVENT; i++)
@@ -315,14 +310,21 @@ static irqreturn_t cmdq_irq_handler(int irq, void *dev)
static int cmdq_runtime_resume(struct device *dev)
{
struct cmdq *cmdq = dev_get_drvdata(dev);
+ int ret;
- return clk_bulk_enable(cmdq->pdata->gce_num, cmdq->clocks);
+ ret = clk_bulk_enable(cmdq->pdata->gce_num, cmdq->clocks);
+ if (ret)
+ return ret;
+
+ cmdq_gctl_value_toggle(cmdq, true);
+ return 0;
}
static int cmdq_runtime_suspend(struct device *dev)
{
struct cmdq *cmdq = dev_get_drvdata(dev);
+ cmdq_gctl_value_toggle(cmdq, false);
clk_bulk_disable(cmdq->pdata->gce_num, cmdq->clocks);
return 0;
}
@@ -347,9 +349,6 @@ static int cmdq_suspend(struct device *dev)
if (task_running)
dev_warn(dev, "exist running task(s) in suspend\n");
- if (cmdq->pdata->sw_ddr_en)
- cmdq_sw_ddr_enable(cmdq, false);
-
return pm_runtime_force_suspend(dev);
}
@@ -360,9 +359,6 @@ static int cmdq_resume(struct device *dev)
WARN_ON(pm_runtime_force_resume(dev));
cmdq->suspended = false;
- if (cmdq->pdata->sw_ddr_en)
- cmdq_sw_ddr_enable(cmdq, true);
-
return 0;
}
@@ -370,9 +366,6 @@ static void cmdq_remove(struct platform_device *pdev)
{
struct cmdq *cmdq = platform_get_drvdata(pdev);
- if (cmdq->pdata->sw_ddr_en)
- cmdq_sw_ddr_enable(cmdq, false);
-
if (!IS_ENABLED(CONFIG_PM))
cmdq_runtime_suspend(&pdev->dev);
--
2.43.0
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] mailbox: mtk-cmdq: Refine GCE_GCTL_VALUE setting
2025-02-24 5:01 [PATCH] mailbox: mtk-cmdq: Refine GCE_GCTL_VALUE setting Jason-JH Lin
@ 2025-02-24 6:04 ` CK Hu (胡俊光)
2025-02-24 9:57 ` Jason-JH Lin (林睿祥)
0 siblings, 1 reply; 3+ messages in thread
From: CK Hu (胡俊光) @ 2025-02-24 6:04 UTC (permalink / raw)
To: jassisinghbrar, Jason-JH Lin (林睿祥),
chunkuang.hu, AngeloGioacchino Del Regno
Cc: Singo Chang (張興國),
treapking, Yongqiang Niu (牛永强),
Project_Global_Chrome_Upstream_Group,
Nancy Lin (林欣螢),
Xavier Chang (張獻文),
linux-kernel, Sirius Wang (王皓昱),
wenst, Xiandong Wang (王先冬),
linux-mediatek, linux-arm-kernel
On Mon, 2025-02-24 at 13:01 +0800, Jason-JH Lin wrote:
> Add cmdq_gctl_value_toggle() to configure GCE_CTRL_BY_SW and GCE_DDR_EN
> together in same the GCE_GCTL_VALUE register.
>
> Move this function into cmdq_runtime_resume() and cmdq_runtime_suspend()
> to ensure it can be called when the GCE clock is enabled.
Why need GCE clock to be enabled when toggle GCE_GCTL_VALUE register?
In some hardware, just need power on to access register. It's not necessary to enable clock.
If GCE need to enable clock to access register, add information here.
>
> Fixes: 7abd037aa581 ("mailbox: mtk-cmdq: add gce ddr enable support flow")
> Signed-off-by: Jason-JH Lin <jason-jh.lin@mediatek.com>
> ---
> drivers/mailbox/mtk-cmdq-mailbox.c | 41 +++++++++++++-----------------
> 1 file changed, 17 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/mailbox/mtk-cmdq-mailbox.c b/drivers/mailbox/mtk-cmdq-mailbox.c
> index d186865b8dce..be17697d7785 100644
> --- a/drivers/mailbox/mtk-cmdq-mailbox.c
> +++ b/drivers/mailbox/mtk-cmdq-mailbox.c
> @@ -92,16 +92,17 @@ struct gce_plat {
> u32 gce_num;
> };
>
> -static void cmdq_sw_ddr_enable(struct cmdq *cmdq, bool enable)
> +static void cmdq_gctl_value_toggle(struct cmdq *cmdq, bool ddr_enable)
> {
> - WARN_ON(clk_bulk_enable(cmdq->pdata->gce_num, cmdq->clocks));
> + u32 val = (cmdq->pdata->control_by_sw) ? GCE_CTRL_BY_SW : 0;
u32 val = cmdq->pdata->control_by_sw ? GCE_CTRL_BY_SW : 0;
>
> - if (enable)
> - writel(GCE_DDR_EN | GCE_CTRL_BY_SW, cmdq->base + GCE_GCTL_VALUE);
> - else
> - writel(GCE_CTRL_BY_SW, cmdq->base + GCE_GCTL_VALUE);
> + if (!cmdq->pdata->control_by_sw && !cmdq->pdata->sw_ddr_en)
> + return;
>
> - clk_bulk_disable(cmdq->pdata->gce_num, cmdq->clocks);
> + if (cmdq->pdata->sw_ddr_en && ddr_enable)
> + val |= GCE_DDR_EN;
> +
> + writel(val, cmdq->base + GCE_GCTL_VALUE);
> }
>
> u8 cmdq_get_shift_pa(struct mbox_chan *chan)
> @@ -140,16 +141,10 @@ static void cmdq_thread_resume(struct cmdq_thread *thread)
> static void cmdq_init(struct cmdq *cmdq)
> {
> int i;
> - u32 gctl_regval = 0;
>
> WARN_ON(clk_bulk_enable(cmdq->pdata->gce_num, cmdq->clocks));
> - if (cmdq->pdata->control_by_sw)
> - gctl_regval = GCE_CTRL_BY_SW;
> - if (cmdq->pdata->sw_ddr_en)
> - gctl_regval |= GCE_DDR_EN;
>
> - if (gctl_regval)
> - writel(gctl_regval, cmdq->base + GCE_GCTL_VALUE);
> + cmdq_gctl_value_toggle(cmdq, true);
>
> writel(CMDQ_THR_ACTIVE_SLOT_CYCLES, cmdq->base + CMDQ_THR_SLOT_CYCLES);
> for (i = 0; i <= CMDQ_MAX_EVENT; i++)
> @@ -315,14 +310,21 @@ static irqreturn_t cmdq_irq_handler(int irq, void *dev)
> static int cmdq_runtime_resume(struct device *dev)
> {
> struct cmdq *cmdq = dev_get_drvdata(dev);
> + int ret;
>
> - return clk_bulk_enable(cmdq->pdata->gce_num, cmdq->clocks);
> + ret = clk_bulk_enable(cmdq->pdata->gce_num, cmdq->clocks);
> + if (ret)
> + return ret;
> +
> + cmdq_gctl_value_toggle(cmdq, true);
> + return 0;
> }
>
> static int cmdq_runtime_suspend(struct device *dev)
> {
> struct cmdq *cmdq = dev_get_drvdata(dev);
>
> + cmdq_gctl_value_toggle(cmdq, false);
> clk_bulk_disable(cmdq->pdata->gce_num, cmdq->clocks);
> return 0;
> }
> @@ -347,9 +349,6 @@ static int cmdq_suspend(struct device *dev)
> if (task_running)
> dev_warn(dev, "exist running task(s) in suspend\n");
>
> - if (cmdq->pdata->sw_ddr_en)
> - cmdq_sw_ddr_enable(cmdq, false);
> -
> return pm_runtime_force_suspend(dev);
> }
>
> @@ -360,9 +359,6 @@ static int cmdq_resume(struct device *dev)
> WARN_ON(pm_runtime_force_resume(dev));
> cmdq->suspended = false;
>
> - if (cmdq->pdata->sw_ddr_en)
> - cmdq_sw_ddr_enable(cmdq, true);
> -
> return 0;
> }
>
> @@ -370,9 +366,6 @@ static void cmdq_remove(struct platform_device *pdev)
> {
> struct cmdq *cmdq = platform_get_drvdata(pdev);
>
> - if (cmdq->pdata->sw_ddr_en)
> - cmdq_sw_ddr_enable(cmdq, false);
> -
> if (!IS_ENABLED(CONFIG_PM))
> cmdq_runtime_suspend(&pdev->dev);
>
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] mailbox: mtk-cmdq: Refine GCE_GCTL_VALUE setting
2025-02-24 6:04 ` CK Hu (胡俊光)
@ 2025-02-24 9:57 ` Jason-JH Lin (林睿祥)
0 siblings, 0 replies; 3+ messages in thread
From: Jason-JH Lin (林睿祥) @ 2025-02-24 9:57 UTC (permalink / raw)
To: CK Hu (胡俊光),
jassisinghbrar, chunkuang.hu, AngeloGioacchino Del Regno
Cc: treapking, Singo Chang (張興國),
Yongqiang Niu (牛永强),
Project_Global_Chrome_Upstream_Group,
Nancy Lin (林欣螢),
Xavier Chang (張獻文),
Sirius Wang (王皓昱),
linux-arm-kernel, linux-kernel, wenst, linux-mediatek,
Xiandong Wang (王先冬)
Hi CK,
Thanks for the reviews.
On Mon, 2025-02-24 at 06:04 +0000, CK Hu (胡俊光) wrote:
> On Mon, 2025-02-24 at 13:01 +0800, Jason-JH Lin wrote:
> > Add cmdq_gctl_value_toggle() to configure GCE_CTRL_BY_SW and
> > GCE_DDR_EN
> > together in same the GCE_GCTL_VALUE register.
> >
> > Move this function into cmdq_runtime_resume() and
> > cmdq_runtime_suspend()
> > to ensure it can be called when the GCE clock is enabled.
>
> Why need GCE clock to be enabled when toggle GCE_GCTL_VALUE register?
All the GCE registers should be written after GCE clocks enabled.
> In some hardware, just need power on to access register. It's not
> necessary to enable clock.
MT8196 GCE is placed in MMINFRA and using MMINFRA_AO power, so it can
be written without enabling the clocks.
> If GCE need to enable clock to access register, add information here.
OK, I'll add the description above into the commit message.
>
> >
> > Fixes: 7abd037aa581 ("mailbox: mtk-cmdq: add gce ddr enable support
> > flow")
> > Signed-off-by: Jason-JH Lin <jason-jh.lin@mediatek.com>
> > ---
> > drivers/mailbox/mtk-cmdq-mailbox.c | 41 +++++++++++++-------------
> > ----
> > 1 file changed, 17 insertions(+), 24 deletions(-)
> >
> > diff --git a/drivers/mailbox/mtk-cmdq-mailbox.c
> > b/drivers/mailbox/mtk-cmdq-mailbox.c
> > index d186865b8dce..be17697d7785 100644
> > --- a/drivers/mailbox/mtk-cmdq-mailbox.c
> > +++ b/drivers/mailbox/mtk-cmdq-mailbox.c
> > @@ -92,16 +92,17 @@ struct gce_plat {
> > u32 gce_num;
> > };
> >
> > -static void cmdq_sw_ddr_enable(struct cmdq *cmdq, bool enable)
> > +static void cmdq_gctl_value_toggle(struct cmdq *cmdq, bool
> > ddr_enable)
> > {
> > - WARN_ON(clk_bulk_enable(cmdq->pdata->gce_num, cmdq-
> > >clocks));
> > + u32 val = (cmdq->pdata->control_by_sw) ? GCE_CTRL_BY_SW :
> > 0;
>
> u32 val = cmdq->pdata->control_by_sw ? GCE_CTRL_BY_SW : 0;
>
OK, I'll change to this.
Regards,
Jason-JH Lin
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2025-02-24 9:57 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-02-24 5:01 [PATCH] mailbox: mtk-cmdq: Refine GCE_GCTL_VALUE setting Jason-JH Lin
2025-02-24 6:04 ` CK Hu (胡俊光)
2025-02-24 9:57 ` Jason-JH Lin (林睿祥)
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®