| 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: <a179cf17-0380-f47e-645e-92f38b0e1917@gmail.com>
Date: Thu, 17 Aug 2017 12:57:44 +0200
From: Matthias Brugger <matthias.bgg@...il.com>
To: weiyi.lu@...iatek.com, Stephen Boyd <sboyd@...eaurora.org>
Cc: James Liao <jamesjj.liao@...iatek.com>,
Fan Chen <fan.chen@...iatek.com>, devicetree@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
linux-mediatek@...ts.infradead.org, linux-clk@...r.kernel.org,
srv_heupstream@...iatek.com
Subject: Re: [PATCH v1 6/9] soc: mediatek: add bus protection extend API
On 08/15/2017 08:42 AM, weiyi.lu@...iatek.com wrote:
> From: Weiyi Lu <weiyi.lu@...iatek.com>
>
> MT2712 add "set/clear" bus control register to each control register set
> instead of providing only one "enable" control register, we could avoid
> the read-modify-write racing by using extend API with such new design.
> Also there exists the second bus control register set, we could not access
> this setting via original API rather than the extend API.
> By improving the mtk-infracfg bus protection implementation to
> support set/clear bus protection control method and second
> register set while power on/off scpsys.
>
> Signed-off-by: Weiyi Lu <weiyi.lu@...iatek.com>
> ---
> drivers/soc/mediatek/mtk-infracfg.c | 90 +++++++++++++++++++++++++++++++++--
> drivers/soc/mediatek/mtk-scpsys.c | 36 ++++++++++++--
> include/linux/soc/mediatek/infracfg.h | 14 +++++-
> 3 files changed, 132 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/soc/mediatek/mtk-infracfg.c b/drivers/soc/mediatek/mtk-infracfg.c
> index dba3055..63898e4 100644
> --- a/drivers/soc/mediatek/mtk-infracfg.c
> +++ b/drivers/soc/mediatek/mtk-infracfg.c
> @@ -17,9 +17,6 @@
> #include <linux/soc/mediatek/infracfg.h>
> #include <asm/processor.h>
>
> -#define INFRA_TOPAXI_PROTECTEN 0x0220
> -#define INFRA_TOPAXI_PROTECTSTA1 0x0228
> -
> /**
> * mtk_infracfg_set_bus_protection - enable bus protection
> * @regmap: The infracfg regmap
> @@ -89,3 +86,90 @@ int mtk_infracfg_clear_bus_protection(struct regmap *infracfg, u32 mask)
>
> return 0;
> }
> +
> +/**
> + * mtk_infracfg_set_bus_protection_ext - enable bus protection
> + * @regmap: The infracfg regmap
> + * @mask: The mask containing the protection bits to be enabled.
> + * @reg_set: The register used to enable protection bits.
> + * @reg_sta: The register used to check the protection bits are enabled.
> + * @reg_en: The register used to enable protection bits when there doesn't
> + * exist reg_set.
> + *
> + * This function enables the bus protection bits for disabled power
> + * domains so that the system does not hang when some unit accesses the
> + * bus while in power down.
> + */
> +int mtk_infracfg_set_bus_protection_ext(struct regmap *infracfg, u32 mask,
> + u32 reg_set, u32 reg_sta, u32 reg_en)
There is no need to add a new function. The only differences are:
1. it passes the register offsets instead of assuming default ones
2. it checks if reg_set/reg_clr and enables/disable.
We should change the original function instead of adding a new one.
From what I see, we are all done if we provide a boolean which tells infracfg
if it should use set/clear or update register.
For now we don't have any user of INFRA_TOPAXI_PROTECTSTA3 (same for
*PROTECTEN1*). Do you expect one in the near future? If so, we could pass a
flags value instead of bool which points that out. It seems there is no need for
now to pass the register value.
Regards,
Matthias
> +{
> + unsigned long expired;
> + u32 val;
> + int ret;
> +
> + if (reg_set)
> + regmap_write(infracfg, reg_set, mask);
> + else
> + regmap_update_bits(infracfg, reg_en, mask, mask);
> +
> + expired = jiffies + HZ;
> +
> + while (1) {
> + ret = regmap_read(infracfg, reg_sta, &val);
> + if (ret)
> + return ret;
> +
> + if ((val & mask) == mask)
> + break;
> +
> + cpu_relax();
> + if (time_after(jiffies, expired))
> + return -EIO;
> + }
> +
> + return 0;
> +}
> +
> +/**
> + * mtk_infracfg_clear_bus_protection_ext - disable bus protection
> + * @regmap: The infracfg regmap
> + * @mask: The mask containing the protection bits to be disabled.
> + * @reg_clr: The register used to disable protection bits.
> + * @reg_sta: The register used to check the protection bits are disabled.
> + * @reg_en: The register used to disable protection bits when there doesn't
> + * exist reg_clr.
> + *
> + * This function disables the bus protection bits previously enabled with
> + * mtk_infracfg_set_bus_protection.
> + */
> +
> +int mtk_infracfg_clear_bus_protection_ext(struct regmap *infracfg, u32 mask,
> + u32 reg_clr, u32 reg_sta, u32 reg_en)
> +{
> + unsigned long expired;
> + int ret;
> +
> + if (reg_clr)
> + regmap_write(infracfg, reg_clr, mask);
> + else
> + regmap_update_bits(infracfg, reg_en, mask, 0);
> +
> + expired = jiffies + HZ;
> +
> + while (1) {
> + u32 val;
> +
> + ret = regmap_read(infracfg, reg_sta, &val);
> + if (ret)
> + return ret;
> +
> + if (!(val & mask))
> + break;
> +
> + cpu_relax();
> + if (time_after(jiffies, expired))
> + return -EIO;
> + }
> +
> + return 0;
> +}
> diff --git a/drivers/soc/mediatek/mtk-scpsys.c b/drivers/soc/mediatek/mtk-scpsys.c
> index ceb2cc4..b181ef2 100644
> --- a/drivers/soc/mediatek/mtk-scpsys.c
> +++ b/drivers/soc/mediatek/mtk-scpsys.c
> @@ -89,6 +89,13 @@ enum clk_id {
>
> #define MAX_CLKS 2
>
> +struct bus_prot_ext {
> + u32 set_ofs;
> + u32 clr_ofs;
> + u32 en_ofs;
> + u32 sta_ofs;
> +};
> +
> struct scp_domain_data {
> const char *name;
> u32 sta_mask;
> @@ -96,6 +103,7 @@ struct scp_domain_data {
> u32 sram_pdn_bits;
> u32 sram_pdn_ack_bits;
> u32 bus_prot_mask;
> + struct bus_prot_ext bp_ext;
> enum clk_id clk_id[MAX_CLKS];
> bool active_wakeup;
> };
> @@ -228,8 +236,18 @@ static int scpsys_power_on(struct generic_pm_domain *genpd)
> }
>
> if (scpd->data->bus_prot_mask) {
> - ret = mtk_infracfg_clear_bus_protection(scp->infracfg,
> - scpd->data->bus_prot_mask);
> + if (((scpd->data->bp_ext.set_ofs && scpd->data->bp_ext.clr_ofs)
> + || scpd->data->bp_ext.en_ofs)
> + && scpd->data->bp_ext.sta_ofs)
> + ret = mtk_infracfg_clear_bus_protection_ext(
> + scp->infracfg,
> + scpd->data->bus_prot_mask,
> + scpd->data->bp_ext.clr_ofs,
> + scpd->data->bp_ext.sta_ofs,
> + scpd->data->bp_ext.en_ofs);
> + else
> + ret = mtk_infracfg_clear_bus_protection(scp->infracfg,
> + scpd->data->bus_prot_mask);
> if (ret)
> goto err_pwr_ack;
> }
> @@ -263,8 +281,18 @@ static int scpsys_power_off(struct generic_pm_domain *genpd)
> int i;
>
> if (scpd->data->bus_prot_mask) {
> - ret = mtk_infracfg_set_bus_protection(scp->infracfg,
> - scpd->data->bus_prot_mask);
> + if (((scpd->data->bp_ext.set_ofs && scpd->data->bp_ext.clr_ofs)
> + || scpd->data->bp_ext.en_ofs)
> + && scpd->data->bp_ext.sta_ofs)
> + ret = mtk_infracfg_set_bus_protection_ext(
> + scp->infracfg,
> + scpd->data->bus_prot_mask,
> + scpd->data->bp_ext.set_ofs,
> + scpd->data->bp_ext.sta_ofs,
> + scpd->data->bp_ext.en_ofs);
> + else
> + ret = mtk_infracfg_set_bus_protection(scp->infracfg,
> + scpd->data->bus_prot_mask);
> if (ret)
> goto out;
> }
> diff --git a/include/linux/soc/mediatek/infracfg.h b/include/linux/soc/mediatek/infracfg.h
> index a5714e9..d3fbb45 100644
> --- a/include/linux/soc/mediatek/infracfg.h
> +++ b/include/linux/soc/mediatek/infracfg.h
> @@ -1,6 +1,15 @@
> #ifndef __SOC_MEDIATEK_INFRACFG_H
> #define __SOC_MEDIATEK_INFRACFG_H
>
> +#define INFRA_TOPAXI_PROTECTEN 0x0220
> +#define INFRA_TOPAXI_PROTECTSTA1 0x0228
> +#define INFRA_TOPAXI_PROTECTEN1 0x0250
> +#define INFRA_TOPAXI_PROTECTSTA3 0x0258
> +#define INFRA_TOPAXI_PROTECTEN_SET 0x0260
> +#define INFRA_TOPAXI_PROTECTEN_CLR 0x0264
> +#define INFRA_TOPAXI_PROTECTEN1_SET 0x0270
> +#define INFRA_TOPAXI_PROTECTEN1_CLR 0x0274
> +
> #define MT8173_TOP_AXI_PROT_EN_MCI_M2 BIT(0)
> #define MT8173_TOP_AXI_PROT_EN_MM_M0 BIT(1)
> #define MT8173_TOP_AXI_PROT_EN_MM_M1 BIT(2)
> @@ -22,5 +31,8 @@
>
> int mtk_infracfg_set_bus_protection(struct regmap *infracfg, u32 mask);
> int mtk_infracfg_clear_bus_protection(struct regmap *infracfg, u32 mask);
> -
> +int mtk_infracfg_set_bus_protection_ext(struct regmap *infracfg, u32 mask,
> + u32 reg_set, u32 reg_sta, u32 reg_en);
> +int mtk_infracfg_clear_bus_protection_ext(struct regmap *infracfg, u32 mask,
> + u32 reg_clr, u32 reg_sta, u32 reg_en);
> #endif /* __SOC_MEDIATEK_INFRACFG_H */
>
Powered by blists - more mailing lists