| 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
| ||
|
Message-ID: <CAHQ1cqEq3LBvJxNBRC-vVsB7=Uj9qTgBXzuAB5vN92qtxo=7bw@mail.gmail.com>
Date: Mon, 12 Apr 2021 10:16:27 -0700
From: Andrey Smirnov <andrew.smirnov@...il.com>
To: Adrien Grassein <adrien.grassein@...il.com>
Cc: Rob Herring <robh+dt@...nel.org>, Shawn Guo <shawnguo@...nel.org>,
Sascha Hauer <s.hauer@...gutronix.de>,
Sascha Hauer <kernel@...gutronix.de>,
Fabio Estevam <festevam@...il.com>,
dl-linux-imx <linux-imx@....com>,
Lucas Stach <l.stach@...gutronix.de>,
Anson Huang <Anson.Huang@....com>, krzk@...nel.org,
peng.fan@....com, Dong Aisheng <aisheng.dong@....com>,
qiangqing.zhang@....com, alice.guo@....com, aford173@...il.com,
agx@...xcpu.org,
"open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS"
<devicetree@...r.kernel.org>,
linux-arm-kernel <linux-arm-kernel@...ts.infradead.org>,
linux-kernel <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v1 1/7] soc: imx: gpcv2: check for errors when r/w registers
On Wed, Apr 7, 2021 at 2:21 PM Adrien Grassein
<adrien.grassein@...il.com> wrote:
>
> Errors were not checked after each access to registers
FWIW, I didn't write any error checking code on purpose since all of
those are memory mapped registers and I don't think there's a case for
those to error out. Don't have a strong opinion on this though.
> and clocks initialisation.
>
> Signed-off-by: Adrien Grassein <adrien.grassein@...il.com>
> ---
> drivers/soc/imx/gpcv2.c | 62 ++++++++++++++++++++++++++++++-----------
> 1 file changed, 45 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/soc/imx/gpcv2.c b/drivers/soc/imx/gpcv2.c
> index db7e7fc321b1..8ec5b1b817c7 100644
> --- a/drivers/soc/imx/gpcv2.c
> +++ b/drivers/soc/imx/gpcv2.c
> @@ -140,8 +140,12 @@ static int imx_gpc_pu_pgc_sw_pxx_req(struct generic_pm_domain *genpd,
> int i, ret = 0;
> u32 pxx_req;
>
> - regmap_update_bits(domain->regmap, GPC_PGC_CPU_MAPPING,
> - domain->bits.map, domain->bits.map);
> + ret = regmap_update_bits(domain->regmap, GPC_PGC_CPU_MAPPING,
> + domain->bits.map, domain->bits.map);
> + if (ret) {
> + dev_err(domain->dev, "failed to map GPC PGC domain\n");
> + return ret;
> + }
>
> if (has_regulator && on) {
> ret = regulator_enable(domain->regulator);
> @@ -152,19 +156,39 @@ static int imx_gpc_pu_pgc_sw_pxx_req(struct generic_pm_domain *genpd,
> }
>
> /* Enable reset clocks for all devices in the domain */
> - for (i = 0; i < domain->num_clks; i++)
> - clk_prepare_enable(domain->clk[i]);
> + for (i = 0; i < domain->num_clks; i++) {
> + ret = clk_prepare_enable(domain->clk[i]);
> + if (ret) {
> + dev_err(domain->dev, "failed to enable clocks\n");
> + goto disable_clocks;
> + }
> + }
>
> - if (enable_power_control)
> - regmap_update_bits(domain->regmap, GPC_PGC_CTRL(domain->pgc),
> - GPC_PGC_CTRL_PCR, GPC_PGC_CTRL_PCR);
> + if (enable_power_control) {
> + ret = regmap_update_bits(domain->regmap, GPC_PGC_CTRL(domain->pgc),
> + GPC_PGC_CTRL_PCR, GPC_PGC_CTRL_PCR);
> + if (ret) {
> + dev_err(domain->dev, "failed to enable power control\n");
> + goto disable_clocks;
> + }
> + }
>
> - if (domain->bits.hsk)
> - regmap_update_bits(domain->regmap, GPC_PU_PWRHSK,
> - domain->bits.hsk, on ? domain->bits.hsk : 0);
> + if (domain->bits.hsk) {
> + ret = regmap_update_bits(domain->regmap, GPC_PU_PWRHSK,
> + domain->bits.hsk,
> + on ? domain->bits.hsk : 0);
> + if (ret) {
> + dev_err(domain->dev, "Failed to initiate handshake\n");
> + goto disable_power_control;
> + }
> + }
>
> - regmap_update_bits(domain->regmap, offset,
> - domain->bits.pxx, domain->bits.pxx);
> + ret = regmap_update_bits(domain->regmap, offset,
> + domain->bits.pxx, domain->bits.pxx);
> + if (ret) {
> + dev_err(domain->dev, "failed to command PGC\n");
> + goto disable_power_control;
> + }
>
> /*
> * As per "5.5.9.4 Example Code 4" in IMX7DRM.pdf wait
> @@ -173,8 +197,15 @@ static int imx_gpc_pu_pgc_sw_pxx_req(struct generic_pm_domain *genpd,
> ret = regmap_read_poll_timeout(domain->regmap, offset, pxx_req,
> !(pxx_req & domain->bits.pxx),
> 0, USEC_PER_MSEC);
> - if (ret) {
> + if (ret)
> dev_err(domain->dev, "failed to command PGC\n");
> +
> +disable_power_control:
> + if (enable_power_control)
> + regmap_update_bits(domain->regmap, GPC_PGC_CTRL(domain->pgc),
> + GPC_PGC_CTRL_PCR, 0);
> +
> + if (ret) {
> /*
> * If we were in a process of enabling a
> * domain and failed we might as well disable
> @@ -185,10 +216,7 @@ static int imx_gpc_pu_pgc_sw_pxx_req(struct generic_pm_domain *genpd,
> on = !on;
> }
>
> - if (enable_power_control)
> - regmap_update_bits(domain->regmap, GPC_PGC_CTRL(domain->pgc),
> - GPC_PGC_CTRL_PCR, 0);
> -
> +disable_clocks:
> /* Disable reset clocks for all devices in the domain */
> for (i = 0; i < domain->num_clks; i++)
> clk_disable_unprepare(domain->clk[i]);
> --
> 2.25.1
>
Powered by blists - more mailing lists