| Message ID | 20220111063904.7583-6-yong.wu@mediatek.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | MT8186 SMI SUPPORT | expand |
Il 11/01/22 07:39, Yong Wu ha scritto: > 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.sun@mediatek.com> > Signed-off-by: Yong Wu <yong.wu@mediatek.com> Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
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.sun@mediatek.com> > Signed-off-by: Yong Wu <yong.wu@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
On Wed, 2022-01-12 at 11:27 +0100, Krzysztof Kozlowski wrote: > 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.sun@mediatek.com> > > Signed-off-by: Yong Wu <yong.wu@mediatek.com> > > --- > > drivers/memory/mtk-smi.c | 35 ++++++++++++++++++++++++++++++++++- > > 1 file changed, 34 insertions(+), 1 deletion(-) [...] > > @@ -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. oh, Yes. Thanks for pointing this out. I will fix this in next version. > > > Best regards, > Krzysztof > > _______________________________________________ > Linux-mediatek mailing list > Linux-mediatek@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-mediatek
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; } static const struct dev_pm_ops smi_larb_pm_ops = {