Re: [PATCH 6/9] soc: mediatek: extend bus protection API

From: Matthias Brugger
Date: Tue Oct 10 2017 - 11:45:13 EST

Next message: Steven Rostedt: "Re: ftrace: Save module init functions kallsyms symbols for tracing"
Previous message: Michael Lyle: "Re: [PATCH v2] bcache: explicitly destroy mutex while exiting"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

On 08/22/2017 12:28 PM, Weiyi Lu wrote:

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.
By improving the mtk-infracfg bus protection implementation to
support set/clear bus protection control method by IC configuration.

Signed-off-by: Weiyi Lu <weiyi.lu@xxxxxxxxxxxx>
---
drivers/soc/mediatek/mtk-infracfg.c | 32 ++++++++++----
drivers/soc/mediatek/mtk-scpsys.c | 81 +++++++++++++++++++++++++++--------
include/linux/soc/mediatek/infracfg.h | 12 ++++--
3 files changed, 96 insertions(+), 29 deletions(-)

diff --git a/drivers/soc/mediatek/mtk-infracfg.c b/drivers/soc/mediatek/mtk-infracfg.c
index dba3055..f55ceaa 100644
--- a/drivers/soc/mediatek/mtk-infracfg.c
+++ b/drivers/soc/mediatek/mtk-infracfg.c
@@ -17,30 +17,35 @@
#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
* @mask: The mask containing the protection bits to be enabled.
+ * @reg_set: The register used to enable protection bits.
+ * @reg_en: The register used to enable protection bits when there doesn't
+ * exist reg_set.
+ * @reg_sta: The register used to check the protection bits are enabled.
*
* 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(struct regmap *infracfg, u32 mask)
+int mtk_infracfg_set_bus_protection(struct regmap *infracfg, u32 mask,
+ u32 reg_set, u32 reg_en, u32 reg_sta)
{
unsigned long expired;
u32 val;
int ret;
- regmap_update_bits(infracfg, INFRA_TOPAXI_PROTECTEN, mask, mask);
+ if (reg_set)
+ regmap_write(infracfg, reg_set, mask);
+ else
+ regmap_update_bits(infracfg, reg_en, mask, mask);

This looks to me as if it's slightly over-engineered.
The reg_set and reg_en and reg_sta are the same on all SoCs, so no need to pass the value to the infra bus protection set and clear functions.
I think we could just add a boolean overwrite_mask for every SoC and check for this.

Do I miss something?

Regards,
Matthias

expired = jiffies + HZ;
while (1) {
- ret = regmap_read(infracfg, INFRA_TOPAXI_PROTECTSTA1, &val);
+ ret = regmap_read(infracfg, reg_sta, &val);
if (ret)
return ret;
@@ -59,23 +64,32 @@ int mtk_infracfg_set_bus_protection(struct regmap *infracfg, u32 mask)
* mtk_infracfg_clear_bus_protection - 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_en: The register used to disable protection bits when there doesn't
+ * exist reg_clr.
+ * @reg_sta: The register used to check the protection bits are disabled.
*
* This function disables the bus protection bits previously enabled with
* mtk_infracfg_set_bus_protection.
*/
-int mtk_infracfg_clear_bus_protection(struct regmap *infracfg, u32 mask)
+
+int mtk_infracfg_clear_bus_protection(struct regmap *infracfg, u32 mask,
+ u32 reg_clr, u32 reg_en, u32 reg_sta)
{
unsigned long expired;
int ret;
- regmap_update_bits(infracfg, INFRA_TOPAXI_PROTECTEN, mask, 0);
+ 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, INFRA_TOPAXI_PROTECTSTA1, &val);
+ ret = regmap_read(infracfg, reg_sta, &val);
if (ret)
return ret;
diff --git a/drivers/soc/mediatek/mtk-scpsys.c b/drivers/soc/mediatek/mtk-scpsys.c
index e1ce8b1..2569390 100644
--- a/drivers/soc/mediatek/mtk-scpsys.c
+++ b/drivers/soc/mediatek/mtk-scpsys.c
@@ -127,13 +127,25 @@ struct scp_ctrl_reg {
int pwr_sta2nd_offs;
};
+struct bus_prot_reg {
+ u32 set_offs;
+ u32 clr_offs;
+ u32 en_offs;
+ u32 sta_offs;
+};
+
+struct soc_reg {
+ struct scp_ctrl_reg scp_ctrl;
+ struct bus_prot_reg bus_prot;
+};
+
struct scp {
struct scp_domain *domains;
struct genpd_onecell_data pd_data;
struct device *dev;
void __iomem *base;
struct regmap *infracfg;
- struct scp_ctrl_reg ctrl_reg;
+ struct soc_reg soc_reg;
};
struct scp_subdomain {
@@ -146,16 +158,16 @@ struct scp_soc_data {
int num_domains;
const struct scp_subdomain *subdomains;
int num_subdomains;
- const struct scp_ctrl_reg regs;
+ const struct soc_reg regs;
};
static int scpsys_domain_is_on(struct scp_domain *scpd)
{
struct scp *scp = scpd->scp;
- u32 status = readl(scp->base + scp->ctrl_reg.pwr_sta_offs) &
+ u32 status = readl(scp->base + scp->soc_reg.scp_ctrl.pwr_sta_offs) &
scpd->data->sta_mask;
- u32 status2 = readl(scp->base + scp->ctrl_reg.pwr_sta2nd_offs) &
+ u32 status2 = readl(scp->base + scp->soc_reg.scp_ctrl.pwr_sta2nd_offs) &
scpd->data->sta_mask;
/*
@@ -254,7 +266,10 @@ 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);
+ scpd->data->bus_prot_mask,
+ scp->soc_reg.bus_prot.clr_offs,
+ scp->soc_reg.bus_prot.en_offs,
+ scp->soc_reg.bus_prot.sta_offs);
if (ret)
goto err_pwr_ack;
}
@@ -289,7 +304,10 @@ static int scpsys_power_off(struct generic_pm_domain *genpd)
if (scpd->data->bus_prot_mask) {
ret = mtk_infracfg_set_bus_protection(scp->infracfg,
- scpd->data->bus_prot_mask);
+ scpd->data->bus_prot_mask,
+ scp->soc_reg.bus_prot.set_offs,
+ scp->soc_reg.bus_prot.en_offs,
+ scp->soc_reg.bus_prot.sta_offs);
if (ret)
goto out;
}
@@ -382,7 +400,7 @@ static void init_clks(struct platform_device *pdev, struct clk **clk)
static struct scp *init_scp(struct platform_device *pdev,
const struct scp_domain_data *scp_domain_data, int num,
- const struct scp_ctrl_reg *scp_ctrl_reg)
+ const struct soc_reg *soc_reg)
{
struct genpd_onecell_data *pd_data;
struct resource *res;
@@ -394,8 +412,13 @@ static struct scp *init_scp(struct platform_device *pdev,
if (!scp)
return ERR_PTR(-ENOMEM);
- scp->ctrl_reg.pwr_sta_offs = scp_ctrl_reg->pwr_sta_offs;
- scp->ctrl_reg.pwr_sta2nd_offs = scp_ctrl_reg->pwr_sta2nd_offs;
+ scp->soc_reg.scp_ctrl.pwr_sta_offs = soc_reg->scp_ctrl.pwr_sta_offs;
+ scp->soc_reg.scp_ctrl.pwr_sta2nd_offs =
+ soc_reg->scp_ctrl.pwr_sta2nd_offs;
+ scp->soc_reg.bus_prot.set_offs = soc_reg->bus_prot.set_offs;
+ scp->soc_reg.bus_prot.clr_offs = soc_reg->bus_prot.clr_offs;
+ scp->soc_reg.bus_prot.en_offs = soc_reg->bus_prot.en_offs;
+ scp->soc_reg.bus_prot.sta_offs = soc_reg->bus_prot.sta_offs;
scp->dev = &pdev->dev;
@@ -814,8 +837,14 @@ static void mtk_register_power_domains(struct platform_device *pdev,
.domains = scp_domain_data_mt2701,
.num_domains = ARRAY_SIZE(scp_domain_data_mt2701),
.regs = {
- .pwr_sta_offs = SPM_PWR_STATUS,
- .pwr_sta2nd_offs = SPM_PWR_STATUS_2ND
+ .scp_ctrl = {
+ .pwr_sta_offs = SPM_PWR_STATUS,
+ .pwr_sta2nd_offs = SPM_PWR_STATUS_2ND
+ },
+ .bus_prot = {
+ .en_offs = INFRA_TOPAXI_PROTECTEN,
+ .sta_offs = INFRA_TOPAXI_PROTECTSTA1
+ },
}
};
@@ -825,8 +854,14 @@ static void mtk_register_power_domains(struct platform_device *pdev,
.subdomains = scp_subdomain_mt6797,
.num_subdomains = ARRAY_SIZE(scp_subdomain_mt6797),
.regs = {
- .pwr_sta_offs = SPM_PWR_STATUS_MT6797,
- .pwr_sta2nd_offs = SPM_PWR_STATUS_2ND_MT6797
+ .scp_ctrl = {
+ .pwr_sta_offs = SPM_PWR_STATUS_MT6797,
+ .pwr_sta2nd_offs = SPM_PWR_STATUS_2ND_MT6797
+ },
+ .bus_prot = {
+ .en_offs = INFRA_TOPAXI_PROTECTEN,
+ .sta_offs = INFRA_TOPAXI_PROTECTSTA1
+ },
}
};
@@ -834,8 +869,14 @@ static void mtk_register_power_domains(struct platform_device *pdev,
.domains = scp_domain_data_mt7622,
.num_domains = ARRAY_SIZE(scp_domain_data_mt7622),
.regs = {
- .pwr_sta_offs = SPM_PWR_STATUS,
- .pwr_sta2nd_offs = SPM_PWR_STATUS_2ND
+ .scp_ctrl = {
+ .pwr_sta_offs = SPM_PWR_STATUS,
+ .pwr_sta2nd_offs = SPM_PWR_STATUS_2ND
+ },
+ .bus_prot = {
+ .en_offs = INFRA_TOPAXI_PROTECTEN,
+ .sta_offs = INFRA_TOPAXI_PROTECTSTA1
+ },
}
};
@@ -845,8 +886,14 @@ static void mtk_register_power_domains(struct platform_device *pdev,
.subdomains = scp_subdomain_mt8173,
.num_subdomains = ARRAY_SIZE(scp_subdomain_mt8173),
.regs = {
- .pwr_sta_offs = SPM_PWR_STATUS,
- .pwr_sta2nd_offs = SPM_PWR_STATUS_2ND
+ .scp_ctrl = {
+ .pwr_sta_offs = SPM_PWR_STATUS,
+ .pwr_sta2nd_offs = SPM_PWR_STATUS_2ND
+ },
+ .bus_prot = {
+ .en_offs = INFRA_TOPAXI_PROTECTEN,
+ .sta_offs = INFRA_TOPAXI_PROTECTSTA1
+ },
}
};
diff --git a/include/linux/soc/mediatek/infracfg.h b/include/linux/soc/mediatek/infracfg.h
index a0182ec..c704af5 100644
--- a/include/linux/soc/mediatek/infracfg.h
+++ b/include/linux/soc/mediatek/infracfg.h
@@ -1,6 +1,11 @@
#ifndef __SOC_MEDIATEK_INFRACFG_H
#define __SOC_MEDIATEK_INFRACFG_H
+#define INFRA_TOPAXI_PROTECTEN 0x0220
+#define INFRA_TOPAXI_PROTECTSTA1 0x0228
+#define INFRA_TOPAXI_PROTECTEN_SET 0x0260
+#define INFRA_TOPAXI_PROTECTEN_CLR 0x0264

The last two a never used.

+
#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)
@@ -27,7 +32,8 @@
#define MT7622_TOP_AXI_PROT_EN_WB (BIT(2) | BIT(6) | \
BIT(7) | BIT(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(struct regmap *infracfg, u32 mask,
+ u32 reg_set, u32 reg_en, u32 reg_sta);
+int mtk_infracfg_clear_bus_protection(struct regmap *infracfg, u32 mask,
+ u32 reg_clr, u32 reg_en, u32 reg_sta);
#endif /* __SOC_MEDIATEK_INFRACFG_H */

Next message: Steven Rostedt: "Re: ftrace: Save module init functions kallsyms symbols for tracing"
Previous message: Michael Lyle: "Re: [PATCH v2] bcache: explicitly destroy mutex while exiting"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]