On 11/01/2022 07:39, Yong Wu wrote: > Sleep control means that when the larb goes to sleep, we should wait a bit > until all the current commands are finished. Thus, when the larb runtime > suspends, we need to enable this function to wait until all the existed > commands are finished. When the larb resumes, just disable this function. > This function only improves the safety of bus. Add a new flag for this > function. Prepare for mt8186. > > Signed-off-by: Anan Sun <anan....@mediatek.com> > Signed-off-by: Yong Wu <yong...@mediatek.com> > --- > drivers/memory/mtk-smi.c | 35 ++++++++++++++++++++++++++++++++++- > 1 file changed, 34 insertions(+), 1 deletion(-) > > diff --git a/drivers/memory/mtk-smi.c b/drivers/memory/mtk-smi.c > index e7b1a22b12ea..d8552f4caba4 100644 > --- a/drivers/memory/mtk-smi.c > +++ b/drivers/memory/mtk-smi.c > @@ -8,6 +8,7 @@ > #include <linux/device.h> > #include <linux/err.h> > #include <linux/io.h> > +#include <linux/iopoll.h> > #include <linux/module.h> > #include <linux/of.h> > #include <linux/of_platform.h> > @@ -32,6 +33,10 @@ > #define SMI_DUMMY 0x444 > > /* SMI LARB */ > +#define SMI_LARB_SLP_CON 0xc > +#define SLP_PROT_EN BIT(0) > +#define SLP_PROT_RDY BIT(16) > + > #define SMI_LARB_CMD_THRT_CON 0x24 > #define SMI_LARB_THRT_RD_NU_LMT_MSK GENMASK(7, 4) > #define SMI_LARB_THRT_RD_NU_LMT (5 << 4) > @@ -81,6 +86,7 @@ > > #define MTK_SMI_FLAG_THRT_UPDATE BIT(0) > #define MTK_SMI_FLAG_SW_FLAG BIT(1) > +#define MTK_SMI_FLAG_SLEEP_CTL BIT(2) > #define MTK_SMI_CAPS(flags, _x) (!!((flags) & (_x))) > > struct mtk_smi_reg_pair { > @@ -371,6 +377,26 @@ static const struct of_device_id mtk_smi_larb_of_ids[] = > { > {} > }; > > +static int mtk_smi_larb_sleep_ctrl_enable(struct mtk_smi_larb *larb) > +{ > + int ret; > + u32 tmp; > + > + writel_relaxed(SLP_PROT_EN, larb->base + SMI_LARB_SLP_CON); > + ret = readl_poll_timeout_atomic(larb->base + SMI_LARB_SLP_CON, > + tmp, !!(tmp & SLP_PROT_RDY), 10, 1000); > + if (ret) { > + /* TODO: Reset this larb if it fails here. */ > + dev_warn(larb->smi.dev, "sleep ctrl is not ready(0x%x).\n", > tmp); > + } > + return ret; > +} > + > +static void mtk_smi_larb_sleep_ctrl_disable(struct mtk_smi_larb *larb) > +{ > + writel_relaxed(0, larb->base + SMI_LARB_SLP_CON); > +} > + > static int mtk_smi_device_link_common(struct device *dev, struct device > **com_dev) > { > struct platform_device *smi_com_pdev; > @@ -483,6 +509,9 @@ static int __maybe_unused mtk_smi_larb_resume(struct > device *dev) > if (ret) > return ret; > > + if (MTK_SMI_CAPS(larb->larb_gen->flags_general, MTK_SMI_FLAG_SLEEP_CTL)) > + mtk_smi_larb_sleep_ctrl_disable(larb); > + > /* Configure the basic setting for this larb */ > larb_gen->config_port(dev); > > @@ -492,9 +521,13 @@ static int __maybe_unused mtk_smi_larb_resume(struct > device *dev) > static int __maybe_unused mtk_smi_larb_suspend(struct device *dev) > { > struct mtk_smi_larb *larb = dev_get_drvdata(dev); > + int ret = 0; > + > + if (MTK_SMI_CAPS(larb->larb_gen->flags_general, MTK_SMI_FLAG_SLEEP_CTL)) > + ret = mtk_smi_larb_sleep_ctrl_enable(larb); > > clk_bulk_disable_unprepare(larb->smi.clk_num, larb->smi.clks); > - return 0; > + return ret;
I am wondering whether disabling clocks in error case is a proper step. On suspend error, the PM core won't run any further callbacks on this device. This means, it won't be resumed and your clocks stay disabled. I think you should return early and leave the device in active state in case of error. Best regards, Krzysztof _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu