linux-kernel - Re: [PATCH v19 3/7] soc: mediatek: SVS: introduce MTK SVS engine

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <0911434c39aa76a6312f19baa2a468c65589de3d.camel@mediatek.com>
Date:   Fri, 2 Jul 2021 17:19:06 +0800
From:   Roger Lu <roger.lu@...iatek.com>
To:     Philipp Zabel <p.zabel@...gutronix.de>,
        Matthias Brugger <matthias.bgg@...il.com>,
        Enric Balletbo Serra <eballetbo@...il.com>,
        "Kevin Hilman" <khilman@...nel.org>,
        Rob Herring <robh+dt@...nel.org>,
        "Nicolas Boichat" <drinkcat@...gle.com>,
        Stephen Boyd <sboyd@...nel.org>
CC:     Fan Chen <fan.chen@...iatek.com>,
        HenryC Chen <HenryC.Chen@...iatek.com>,
        YT Lee <yt.lee@...iatek.com>,
        Xiaoqing Liu <Xiaoqing.Liu@...iatek.com>,
        Charles Yang <Charles.Yang@...iatek.com>,
        Angus Lin <Angus.Lin@...iatek.com>,
        Mark Rutland <mark.rutland@....com>,
        Nishanth Menon <nm@...com>, <devicetree@...r.kernel.org>,
        <linux-arm-kernel@...ts.infradead.org>,
        <linux-mediatek@...ts.infradead.org>,
        <linux-kernel@...r.kernel.org>, <linux-pm@...r.kernel.org>,
        <Project_Global_Chrome_Upstream_Group@...iatek.com>,
        Guenter Roeck <linux@...ck-us.net>
Subject: Re: [PATCH v19 3/7] soc: mediatek: SVS: introduce MTK SVS engine

Hi Philipp,

Thanks for the advice.

On Fri, 2021-07-02 at 10:33 +0200, Philipp Zabel wrote:
> Hi Roger,
> 
> On Fri, 2021-07-02 at 11:12 +0800, Roger Lu wrote:
> [...]
> > diff --git a/drivers/soc/mediatek/mtk-svs.c b/drivers/soc/mediatek/mtk-svs.c
> > new file mode 100644
> > index 000000000000..c2fcbc204b1d
> > --- /dev/null
> > +++ b/drivers/soc/mediatek/mtk-svs.c
> > @@ -0,0 +1,1724 @@
> 
> [...]
> > +static int svs_suspend(struct device *dev)
> > +{
> > +	struct svs_platform *svsp = dev_get_drvdata(dev);
> > +	struct svs_bank *svsb;
> > +	unsigned long flags;
> > +	int ret;
> > +	u32 idx;
> > +
> > +	for (idx = 0; idx < svsp->bank_num; idx++) {
> > +		svsb = &svsp->banks[idx];
> > +
> > +		/* Wait if svs_isr() is still in process. */
> > +		spin_lock_irqsave(&mtk_svs_lock, flags);
> > +		svsp->pbank = svsb;
> > +		svs_switch_bank(svsp);
> > +		svs_writel(svsp, SVSB_EN_OFF, SVSEN);
> > +		svs_writel(svsp, SVSB_INTSTS_CLEAN, INTSTS);
> > +		spin_unlock_irqrestore(&mtk_svs_lock, flags);
> > +
> > +		svsb->suspended = true;
> > +		if (svsb->phase != SVSB_PHASE_INIT01) {
> > +			svsb->phase = SVSB_PHASE_ERROR;
> > +			svs_adjust_pm_opp_volts(svsb, true);
> > +		}
> > +	}
> > +
> > +	if (svsp->rst) {
> 
> This is not necessary, reset_control_assert() checks for (rstc == NULL)
> itself.
Okay. We'll remove "if (svsp->rst) {" in the next patch. Thanks.

> 
> > +		ret = reset_control_assert(svsp->rst);
> > +		if (ret) {
> > +			dev_err(svsp->dev, "cannot assert reset %d\n", ret);
> > +			return ret;
> > +		}
> > +	}
> > +
> > +	clk_disable_unprepare(svsp->main_clk);
> > +
> > +	return 0;
> > +}
> > +
> > +static int svs_resume(struct device *dev)
> > +{
> > +	struct svs_platform *svsp = dev_get_drvdata(dev);
> > +	struct svs_bank *svsb;
> > +	int ret;
> > +	u32 idx;
> > +
> > +	ret = clk_prepare_enable(svsp->main_clk);
> > +	if (ret) {
> > +		dev_err(svsp->dev, "cannot enable main_clk, disable svs\n");
> > +		return ret;
> > +	}
> > +
> > +	if (svsp->rst) {
> 
> Same as above, reset_control_deassert(NULL) will just return 0.
Okay. We'll remove "if (svsp->rst) {" in the next patch. Thanks.
> 
> > +		ret = reset_control_deassert(svsp->rst);
> > +		if (ret) {
> > +			dev_err(svsp->dev, "cannot deassert reset %d\n", ret);
> > +			return ret;
> > +		}
> > +	}
> > +
> > +	for (idx = 0; idx < svsp->bank_num; idx++) {
> > +		svsb = &svsp->banks[idx];
> > +		svsb->suspended = false;
> > +	}
> > +
> > +	ret = svs_init02(svsp);
> > +	if (ret)
> > +		return ret;
> > +
> > +	svs_mon_mode(svsp);
> > +
> > +	return 0;
> > +}
> 
> regards
> Philipp