Both MT8183 & MT6765 have more control steps of bus protection
than previous project. And there add more bus protection registers
reside at infracfg & smi-common.
Extend function to support multiple step bus protection control
with more customized arguments.
And then use bp_table for bus protection of all compatibles,
instead of mixing bus_prot_mask and bus_prot_reg_update.
Signed-off-by: Weiyi Lu <[email protected]>
---
drivers/soc/mediatek/mtk-scpsys.c | 206 ++++++++++++++++++++++++++------------
drivers/soc/mediatek/scpsys.h | 42 +++++++-
2 files changed, 182 insertions(+), 66 deletions(-)
diff --git a/drivers/soc/mediatek/mtk-scpsys.c b/drivers/soc/mediatek/mtk-scpsys.c
index e50a568..a4fb0b23 100644
--- a/drivers/soc/mediatek/mtk-scpsys.c
+++ b/drivers/soc/mediatek/mtk-scpsys.c
@@ -88,9 +88,9 @@
* @ctl_offs: The offset for main power control register.
* @sram_pdn_bits: The mask for sram power control bits.
* @sram_pdn_ack_bits: The mask for sram power control acked bits.
- * @bus_prot_mask: The mask for single step bus protection.
* @basic_clk_name: The basic clocks required by this power domain.
* @caps: The flag for active wake-up action.
+ * @bp_table: The mask table for multiple step bus protection.
*/
struct scp_domain_data {
const char *name;
@@ -98,9 +98,9 @@ struct scp_domain_data {
int ctl_offs;
u32 sram_pdn_bits;
u32 sram_pdn_ack_bits;
- u32 bus_prot_mask;
const char *basic_clk_name[MAX_CLKS];
u8 caps;
+ struct bus_prot bp_table[MAX_STEPS];
};
struct scp;
@@ -124,8 +124,8 @@ struct scp {
struct device *dev;
void __iomem *base;
struct regmap *infracfg;
+ struct regmap *smi_common;
struct scp_ctrl_reg ctrl_reg;
- bool bus_prot_reg_update;
};
struct scp_subdomain {
@@ -139,7 +139,6 @@ struct scp_soc_data {
const struct scp_subdomain *subdomains;
int num_subdomains;
const struct scp_ctrl_reg regs;
- bool bus_prot_reg_update;
};
static int scpsys_domain_is_on(struct scp_domain *scpd)
@@ -249,53 +248,102 @@ static int scpsys_sram_disable(struct scp_domain *scpd, void __iomem *ctl_addr)
MTK_POLL_DELAY_US, MTK_POLL_TIMEOUT);
}
+static int set_bus_protection(struct regmap *map, struct bus_prot *bp)
+{
+ u32 val;
+ u32 set_ofs = bp->set_ofs;
+ u32 en_ofs = bp->en_ofs;
+ u32 sta_ofs = bp->sta_ofs;
+ u32 mask = bp->mask;
+
+ if (set_ofs)
+ regmap_write(map, set_ofs, mask);
+ else
+ regmap_update_bits(map, en_ofs, mask, mask);
+
+ return regmap_read_poll_timeout(map, sta_ofs,
+ val, (val & mask) == mask,
+ MTK_POLL_DELAY_US, MTK_POLL_TIMEOUT);
+}
+
+static int clear_bus_protection(struct regmap *map, struct bus_prot *bp)
+{
+ u32 val;
+ u32 clr_ofs = bp->clr_ofs;
+ u32 en_ofs = bp->en_ofs;
+ u32 sta_ofs = bp->sta_ofs;
+ u32 mask = bp->mask;
+ bool ignore_ack = bp->ignore_clr_ack;
+
+ if (clr_ofs)
+ regmap_write(map, clr_ofs, mask);
+ else
+ regmap_update_bits(map, en_ofs, mask, 0);
+
+ if (ignore_ack)
+ return 0;
+
+ return regmap_read_poll_timeout(map, sta_ofs,
+ val, !(val & mask),
+ MTK_POLL_DELAY_US, MTK_POLL_TIMEOUT);
+}
+
static int scpsys_bus_protect_enable(struct scp_domain *scpd)
{
struct scp *scp = scpd->scp;
+ const struct bus_prot *bp_table = scpd->data->bp_table;
struct regmap *infracfg = scp->infracfg;
- u32 mask = scpd->data->bus_prot_mask;
- bool reg_update = scp->bus_prot_reg_update;
- u32 val;
- int ret;
+ struct regmap *smi_common = scp->smi_common;
+ int i;
- if (!mask)
- return 0;
+ for (i = 0; i < MAX_STEPS; i++) {
+ struct regmap *map = NULL;
+ int ret;
+ struct bus_prot bp = bp_table[i];
- if (reg_update)
- regmap_update_bits(infracfg, INFRA_TOPAXI_PROTECTEN, mask,
- mask);
- else
- regmap_write(infracfg, INFRA_TOPAXI_PROTECTEN_SET, mask);
+ if (bp.type == IFR_TYPE)
+ map = infracfg;
+ else if (bp.type == SMI_TYPE)
+ map = smi_common;
+ else
+ break;
- ret = regmap_read_poll_timeout(infracfg, INFRA_TOPAXI_PROTECTSTA1,
- val, (val & mask) == mask,
- MTK_POLL_DELAY_US, MTK_POLL_TIMEOUT);
+ ret = set_bus_protection(map, &bp);
- return ret;
+ if (ret)
+ return ret;
+ }
+
+ return 0;
}
static int scpsys_bus_protect_disable(struct scp_domain *scpd)
{
struct scp *scp = scpd->scp;
+ const struct bus_prot *bp_table = scpd->data->bp_table;
struct regmap *infracfg = scp->infracfg;
- u32 mask = scpd->data->bus_prot_mask;
- bool reg_update = scp->bus_prot_reg_update;
- u32 val;
- int ret;
+ struct regmap *smi_common = scp->smi_common;
+ int i;
- if (!mask)
- return 0;
+ for (i = MAX_STEPS - 1; i >= 0; i--) {
+ struct regmap *map = NULL;
+ int ret;
+ struct bus_prot bp = bp_table[i];
- if (reg_update)
- regmap_update_bits(infracfg, INFRA_TOPAXI_PROTECTEN, mask, 0);
- else
- regmap_write(infracfg, INFRA_TOPAXI_PROTECTEN_CLR, mask);
+ if (bp.type == IFR_TYPE)
+ map = infracfg;
+ else if (bp.type == SMI_TYPE)
+ map = smi_common;
+ else
+ continue;
- ret = regmap_read_poll_timeout(infracfg, INFRA_TOPAXI_PROTECTSTA1,
- val, !(val & mask),
- MTK_POLL_DELAY_US, MTK_POLL_TIMEOUT);
+ ret = clear_bus_protection(map, &bp);
- return ret;
+ if (ret)
+ return ret;
+ }
+
+ return 0;
}
static int scpsys_power_on(struct generic_pm_domain *genpd)
@@ -430,8 +478,7 @@ static int init_basic_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,
- bool bus_prot_reg_update)
+ const struct scp_ctrl_reg *scp_ctrl_reg)
{
struct genpd_onecell_data *pd_data;
struct resource *res;
@@ -445,8 +492,6 @@ static struct scp *init_scp(struct platform_device *pdev,
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->bus_prot_reg_update = bus_prot_reg_update;
-
scp->dev = &pdev->dev;
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
@@ -474,6 +519,17 @@ static struct scp *init_scp(struct platform_device *pdev,
return ERR_CAST(scp->infracfg);
}
+ scp->smi_common = syscon_regmap_lookup_by_phandle(pdev->dev.of_node,
+ "smi_comm");
+
+ if (scp->smi_common == ERR_PTR(-ENODEV)) {
+ scp->smi_common = NULL;
+ } else if (IS_ERR(scp->smi_common)) {
+ dev_err(&pdev->dev, "Cannot find smi_common controller: %ld\n",
+ PTR_ERR(scp->smi_common));
+ return ERR_CAST(scp->smi_common);
+ }
+
for (i = 0; i < num; i++) {
struct scp_domain *scpd = &scp->domains[i];
const struct scp_domain_data *data = &scp_domain_data[i];
@@ -556,8 +612,11 @@ static void mtk_register_power_domains(struct platform_device *pdev,
.name = "conn",
.sta_mask = PWR_STATUS_CONN,
.ctl_offs = SPM_CONN_PWR_CON,
- .bus_prot_mask = MT2701_TOP_AXI_PROT_EN_CONN_M |
- MT2701_TOP_AXI_PROT_EN_CONN_S,
+ .bp_table = {
+ BUS_PROT(IFR_TYPE, 0, 0, 0x220, 0x228,
+ MT2701_TOP_AXI_PROT_EN_CONN_M |
+ MT2701_TOP_AXI_PROT_EN_CONN_S),
+ },
.caps = MTK_SCPD_ACTIVE_WAKEUP,
},
[MT2701_POWER_DOMAIN_DISP] = {
@@ -566,7 +625,10 @@ static void mtk_register_power_domains(struct platform_device *pdev,
.ctl_offs = SPM_DIS_PWR_CON,
.sram_pdn_bits = GENMASK(11, 8),
.basic_clk_name = {"mm"},
- .bus_prot_mask = MT2701_TOP_AXI_PROT_EN_MM_M0,
+ .bp_table = {
+ BUS_PROT(IFR_TYPE, 0, 0, 0x220, 0x228,
+ MT2701_TOP_AXI_PROT_EN_MM_M0),
+ },
.caps = MTK_SCPD_ACTIVE_WAKEUP,
},
[MT2701_POWER_DOMAIN_MFG] = {
@@ -701,7 +763,10 @@ static void mtk_register_power_domains(struct platform_device *pdev,
.sram_pdn_bits = GENMASK(8, 8),
.sram_pdn_ack_bits = GENMASK(16, 16),
.basic_clk_name = {"mfg"},
- .bus_prot_mask = BIT(14) | BIT(21) | BIT(23),
+ .bp_table = {
+ BUS_PROT(IFR_TYPE, 0x260, 0x264, 0x220, 0x228,
+ BIT(14) | BIT(21) | BIT(23)),
+ },
.caps = MTK_SCPD_ACTIVE_WAKEUP,
},
[MT2712_POWER_DOMAIN_MFG_SC1] = {
@@ -773,7 +838,10 @@ static void mtk_register_power_domains(struct platform_device *pdev,
.sram_pdn_bits = GENMASK(8, 8),
.sram_pdn_ack_bits = GENMASK(12, 12),
.basic_clk_name = {"mm"},
- .bus_prot_mask = (BIT(1) | BIT(2)),
+ .bp_table = {
+ BUS_PROT(IFR_TYPE, 0, 0, 0x220, 0x228,
+ BIT(1) | BIT(2)),
+ },
},
[MT6797_POWER_DOMAIN_AUDIO] = {
.name = "audio",
@@ -820,7 +888,10 @@ static void mtk_register_power_domains(struct platform_device *pdev,
.ctl_offs = SPM_ETHSYS_PWR_CON,
.sram_pdn_bits = GENMASK(11, 8),
.sram_pdn_ack_bits = GENMASK(15, 12),
- .bus_prot_mask = MT7622_TOP_AXI_PROT_EN_ETHSYS,
+ .bp_table = {
+ BUS_PROT(IFR_TYPE, 0, 0, 0x220, 0x228,
+ MT7622_TOP_AXI_PROT_EN_ETHSYS),
+ },
.caps = MTK_SCPD_ACTIVE_WAKEUP,
},
[MT7622_POWER_DOMAIN_HIF0] = {
@@ -830,7 +901,10 @@ static void mtk_register_power_domains(struct platform_device *pdev,
.sram_pdn_bits = GENMASK(11, 8),
.sram_pdn_ack_bits = GENMASK(15, 12),
.basic_clk_name = {"hif_sel"},
- .bus_prot_mask = MT7622_TOP_AXI_PROT_EN_HIF0,
+ .bp_table = {
+ BUS_PROT(IFR_TYPE, 0, 0, 0x220, 0x228,
+ MT7622_TOP_AXI_PROT_EN_HIF0),
+ },
.caps = MTK_SCPD_ACTIVE_WAKEUP,
},
[MT7622_POWER_DOMAIN_HIF1] = {
@@ -840,7 +914,10 @@ static void mtk_register_power_domains(struct platform_device *pdev,
.sram_pdn_bits = GENMASK(11, 8),
.sram_pdn_ack_bits = GENMASK(15, 12),
.basic_clk_name = {"hif_sel"},
- .bus_prot_mask = MT7622_TOP_AXI_PROT_EN_HIF1,
+ .bp_table = {
+ BUS_PROT(IFR_TYPE, 0, 0, 0x220, 0x228,
+ MT7622_TOP_AXI_PROT_EN_HIF1),
+ },
.caps = MTK_SCPD_ACTIVE_WAKEUP,
},
[MT7622_POWER_DOMAIN_WB] = {
@@ -849,7 +926,10 @@ static void mtk_register_power_domains(struct platform_device *pdev,
.ctl_offs = SPM_WB_PWR_CON,
.sram_pdn_bits = 0,
.sram_pdn_ack_bits = 0,
- .bus_prot_mask = MT7622_TOP_AXI_PROT_EN_WB,
+ .bp_table = {
+ BUS_PROT(IFR_TYPE, 0, 0, 0x220, 0x228,
+ MT7622_TOP_AXI_PROT_EN_WB),
+ },
.caps = MTK_SCPD_ACTIVE_WAKEUP | MTK_SCPD_FWAIT_SRAM,
},
};
@@ -863,8 +943,11 @@ static void mtk_register_power_domains(struct platform_device *pdev,
.name = "conn",
.sta_mask = PWR_STATUS_CONN,
.ctl_offs = SPM_CONN_PWR_CON,
- .bus_prot_mask = MT2701_TOP_AXI_PROT_EN_CONN_M |
- MT2701_TOP_AXI_PROT_EN_CONN_S,
+ .bp_table = {
+ BUS_PROT(IFR_TYPE, 0, 0, 0x220, 0x228,
+ MT2701_TOP_AXI_PROT_EN_CONN_M |
+ MT2701_TOP_AXI_PROT_EN_CONN_S),
+ },
.caps = MTK_SCPD_ACTIVE_WAKEUP,
},
[MT7623A_POWER_DOMAIN_ETH] = {
@@ -929,8 +1012,11 @@ static void mtk_register_power_domains(struct platform_device *pdev,
.sram_pdn_bits = GENMASK(11, 8),
.sram_pdn_ack_bits = GENMASK(12, 12),
.basic_clk_name = {"mm"},
- .bus_prot_mask = MT8173_TOP_AXI_PROT_EN_MM_M0 |
- MT8173_TOP_AXI_PROT_EN_MM_M1,
+ .bp_table = {
+ BUS_PROT(IFR_TYPE, 0, 0, 0x220, 0x228,
+ MT8173_TOP_AXI_PROT_EN_MM_M0 |
+ MT8173_TOP_AXI_PROT_EN_MM_M1),
+ },
},
[MT8173_POWER_DOMAIN_VENC_LT] = {
.name = "venc_lt",
@@ -976,10 +1062,13 @@ static void mtk_register_power_domains(struct platform_device *pdev,
.ctl_offs = SPM_MFG_PWR_CON,
.sram_pdn_bits = GENMASK(13, 8),
.sram_pdn_ack_bits = GENMASK(21, 16),
- .bus_prot_mask = MT8173_TOP_AXI_PROT_EN_MFG_S |
- MT8173_TOP_AXI_PROT_EN_MFG_M0 |
- MT8173_TOP_AXI_PROT_EN_MFG_M1 |
- MT8173_TOP_AXI_PROT_EN_MFG_SNOOP_OUT,
+ .bp_table = {
+ BUS_PROT(IFR_TYPE, 0, 0, 0x220, 0x228,
+ MT8173_TOP_AXI_PROT_EN_MFG_S |
+ MT8173_TOP_AXI_PROT_EN_MFG_M0 |
+ MT8173_TOP_AXI_PROT_EN_MFG_M1 |
+ MT8173_TOP_AXI_PROT_EN_MFG_SNOOP_OUT),
+ },
},
};
@@ -995,7 +1084,6 @@ static void mtk_register_power_domains(struct platform_device *pdev,
.pwr_sta_offs = SPM_PWR_STATUS,
.pwr_sta2nd_offs = SPM_PWR_STATUS_2ND
},
- .bus_prot_reg_update = true,
};
static const struct scp_soc_data mt2712_data = {
@@ -1007,7 +1095,6 @@ static void mtk_register_power_domains(struct platform_device *pdev,
.pwr_sta_offs = SPM_PWR_STATUS,
.pwr_sta2nd_offs = SPM_PWR_STATUS_2ND
},
- .bus_prot_reg_update = false,
};
static const struct scp_soc_data mt6797_data = {
@@ -1019,7 +1106,6 @@ static void mtk_register_power_domains(struct platform_device *pdev,
.pwr_sta_offs = SPM_PWR_STATUS_MT6797,
.pwr_sta2nd_offs = SPM_PWR_STATUS_2ND_MT6797
},
- .bus_prot_reg_update = true,
};
static const struct scp_soc_data mt7622_data = {
@@ -1029,7 +1115,6 @@ static void mtk_register_power_domains(struct platform_device *pdev,
.pwr_sta_offs = SPM_PWR_STATUS,
.pwr_sta2nd_offs = SPM_PWR_STATUS_2ND
},
- .bus_prot_reg_update = true,
};
static const struct scp_soc_data mt7623a_data = {
@@ -1039,7 +1124,6 @@ static void mtk_register_power_domains(struct platform_device *pdev,
.pwr_sta_offs = SPM_PWR_STATUS,
.pwr_sta2nd_offs = SPM_PWR_STATUS_2ND
},
- .bus_prot_reg_update = true,
};
static const struct scp_soc_data mt8173_data = {
@@ -1051,7 +1135,6 @@ static void mtk_register_power_domains(struct platform_device *pdev,
.pwr_sta_offs = SPM_PWR_STATUS,
.pwr_sta2nd_offs = SPM_PWR_STATUS_2ND
},
- .bus_prot_reg_update = true,
};
/*
@@ -1092,8 +1175,7 @@ static int scpsys_probe(struct platform_device *pdev)
soc = of_device_get_match_data(&pdev->dev);
- scp = init_scp(pdev, soc->domains, soc->num_domains, &soc->regs,
- soc->bus_prot_reg_update);
+ scp = init_scp(pdev, soc->domains, soc->num_domains, &soc->regs);
if (IS_ERR(scp))
return PTR_ERR(scp);
diff --git a/drivers/soc/mediatek/scpsys.h b/drivers/soc/mediatek/scpsys.h
index ad2559b..45bc644 100644
--- a/drivers/soc/mediatek/scpsys.h
+++ b/drivers/soc/mediatek/scpsys.h
@@ -2,10 +2,28 @@
#ifndef __SOC_MEDIATEK_SCPSYS_H
#define __SOC_MEDIATEK_SCPSYS_H
-#define INFRA_TOPAXI_PROTECTEN 0x0220
-#define INFRA_TOPAXI_PROTECTSTA1 0x0228
-#define INFRA_TOPAXI_PROTECTEN_SET 0x0260
-#define INFRA_TOPAXI_PROTECTEN_CLR 0x0264
+#define MAX_STEPS 4
+
+#define _BUS_PROT(_type, _set_ofs, _clr_ofs, \
+ _en_ofs, _sta_ofs, _mask, _ignore_clr_ack) { \
+ .type = _type, \
+ .set_ofs = _set_ofs, \
+ .clr_ofs = _clr_ofs, \
+ .en_ofs = _en_ofs, \
+ .sta_ofs = _sta_ofs, \
+ .mask = _mask, \
+ .ignore_clr_ack = _ignore_clr_ack, \
+ }
+
+#define BUS_PROT(_type, _set_ofs, _clr_ofs, \
+ _en_ofs, _sta_ofs, _mask) \
+ _BUS_PROT(_type, _set_ofs, _clr_ofs, \
+ _en_ofs, _sta_ofs, _mask, false)
+
+#define BUS_PROT_IGN(_type, _set_ofs, _clr_ofs, \
+ _en_ofs, _sta_ofs, _mask) \
+ _BUS_PROT(_type, _set_ofs, _clr_ofs, \
+ _en_ofs, _sta_ofs, _mask, true)
#define MT2701_TOP_AXI_PROT_EN_MM_M0 BIT(1)
#define MT2701_TOP_AXI_PROT_EN_CONN_M BIT(2)
@@ -25,4 +43,20 @@
#define MT8173_TOP_AXI_PROT_EN_MFG_M1 BIT(22)
#define MT8173_TOP_AXI_PROT_EN_MFG_SNOOP_OUT BIT(23)
+enum regmap_type {
+ INVALID_TYPE = 0,
+ IFR_TYPE,
+ SMI_TYPE,
+};
+
+struct bus_prot {
+ enum regmap_type type;
+ u32 set_ofs;
+ u32 clr_ofs;
+ u32 en_ofs;
+ u32 sta_ofs;
+ u32 mask;
+ bool ignore_clr_ack;
+};
+
#endif /* __SOC_MEDIATEK_SCPSYS_H */
--
1.8.1.1.dirty
Hi Wiyi Lu,
Thank you for your patch.
Missatge de Weiyi Lu <[email protected]> del dia dv., 20 de març
2020 a les 8:33:
>
> Both MT8183 & MT6765 have more control steps of bus protection
> than previous project. And there add more bus protection registers
> reside at infracfg & smi-common.
> Extend function to support multiple step bus protection control
> with more customized arguments.
> And then use bp_table for bus protection of all compatibles,
> instead of mixing bus_prot_mask and bus_prot_reg_update.
>
> Signed-off-by: Weiyi Lu <[email protected]>
> ---
> drivers/soc/mediatek/mtk-scpsys.c | 206 ++++++++++++++++++++++++++------------
> drivers/soc/mediatek/scpsys.h | 42 +++++++-
> 2 files changed, 182 insertions(+), 66 deletions(-)
>
> diff --git a/drivers/soc/mediatek/mtk-scpsys.c b/drivers/soc/mediatek/mtk-scpsys.c
> index e50a568..a4fb0b23 100644
> --- a/drivers/soc/mediatek/mtk-scpsys.c
> +++ b/drivers/soc/mediatek/mtk-scpsys.c
> @@ -88,9 +88,9 @@
> * @ctl_offs: The offset for main power control register.
> * @sram_pdn_bits: The mask for sram power control bits.
> * @sram_pdn_ack_bits: The mask for sram power control acked bits.
> - * @bus_prot_mask: The mask for single step bus protection.
> * @basic_clk_name: The basic clocks required by this power domain.
> * @caps: The flag for active wake-up action.
> + * @bp_table: The mask table for multiple step bus protection.
> */
> struct scp_domain_data {
> const char *name;
> @@ -98,9 +98,9 @@ struct scp_domain_data {
> int ctl_offs;
> u32 sram_pdn_bits;
> u32 sram_pdn_ack_bits;
> - u32 bus_prot_mask;
> const char *basic_clk_name[MAX_CLKS];
> u8 caps;
> + struct bus_prot bp_table[MAX_STEPS];
> };
>
> struct scp;
> @@ -124,8 +124,8 @@ struct scp {
> struct device *dev;
> void __iomem *base;
> struct regmap *infracfg;
> + struct regmap *smi_common;
> struct scp_ctrl_reg ctrl_reg;
> - bool bus_prot_reg_update;
> };
>
> struct scp_subdomain {
> @@ -139,7 +139,6 @@ struct scp_soc_data {
> const struct scp_subdomain *subdomains;
> int num_subdomains;
> const struct scp_ctrl_reg regs;
> - bool bus_prot_reg_update;
> };
>
> static int scpsys_domain_is_on(struct scp_domain *scpd)
> @@ -249,53 +248,102 @@ static int scpsys_sram_disable(struct scp_domain *scpd, void __iomem *ctl_addr)
> MTK_POLL_DELAY_US, MTK_POLL_TIMEOUT);
> }
>
> +static int set_bus_protection(struct regmap *map, struct bus_prot *bp)
> +{
> + u32 val;
> + u32 set_ofs = bp->set_ofs;
> + u32 en_ofs = bp->en_ofs;
> + u32 sta_ofs = bp->sta_ofs;
> + u32 mask = bp->mask;
Remove unnecessary local variables (set_ofs, en_ofs, sta_ofs and
mask), and use their bp-> form directly.
> +
> + if (set_ofs)
> + regmap_write(map, set_ofs, mask);
> + else
> + regmap_update_bits(map, en_ofs, mask, mask);
> +
> + return regmap_read_poll_timeout(map, sta_ofs,
> + val, (val & mask) == mask,
> + MTK_POLL_DELAY_US, MTK_POLL_TIMEOUT);
> +}
> +
> +static int clear_bus_protection(struct regmap *map, struct bus_prot *bp)
> +{
> + u32 val;
> + u32 clr_ofs = bp->clr_ofs;
> + u32 en_ofs = bp->en_ofs;
> + u32 sta_ofs = bp->sta_ofs;
> + u32 mask = bp->mask;
> + bool ignore_ack = bp->ignore_clr_ack;
Remove unnecessary local variables (clr_ofs, en_ofs, sta_ofs, mask and
ignore_ack), and use their bp-> form directly.
> +
> + if (clr_ofs)
> + regmap_write(map, clr_ofs, mask);
> + else
> + regmap_update_bits(map, en_ofs, mask, 0);
> +
> + if (ignore_ack)
> + return 0;
> +
> + return regmap_read_poll_timeout(map, sta_ofs,
> + val, !(val & mask),
> + MTK_POLL_DELAY_US, MTK_POLL_TIMEOUT);
> +}
> +
> static int scpsys_bus_protect_enable(struct scp_domain *scpd)
> {
> struct scp *scp = scpd->scp;
> + const struct bus_prot *bp_table = scpd->data->bp_table;
> struct regmap *infracfg = scp->infracfg;
> - u32 mask = scpd->data->bus_prot_mask;
> - bool reg_update = scp->bus_prot_reg_update;
> - u32 val;
> - int ret;
No need to remove ret if you're going to add anyway later.
> + struct regmap *smi_common = scp->smi_common;
> + int i;
>
> - if (!mask)
> - return 0;
> + for (i = 0; i < MAX_STEPS; i++) {
> + struct regmap *map = NULL;
Declare this on top of the function and no need to initialize to NUlL
> + int ret;
> + struct bus_prot bp = bp_table[i];
Unneeded local variable ...
>
> - if (reg_update)
> - regmap_update_bits(infracfg, INFRA_TOPAXI_PROTECTEN, mask,
> - mask);
> - else
> - regmap_write(infracfg, INFRA_TOPAXI_PROTECTEN_SET, mask);
> + if (bp.type == IFR_TYPE)
bp_table[i].type?
> + map = infracfg;
> + else if (bp.type == SMI_TYPE)
ditto
> + map = smi_common;
> + else
> + break;
So if the type is invalid you break the loop, this is different to
what you are doing below with the scpsys_bus_protect_protect_disable()
function where you are "ignoring" instead of beaking the loop. I am
wondering why? Looks to me that, or you break and print an error
because something was really wrong? or you ignore in both cases?
>
> - ret = regmap_read_poll_timeout(infracfg, INFRA_TOPAXI_PROTECTSTA1,
> - val, (val & mask) == mask,
> - MTK_POLL_DELAY_US, MTK_POLL_TIMEOUT);
> + ret = set_bus_protection(map, &bp);
>
> - return ret;
> + if (ret)
> + return ret;
> + }
> +
> + return 0;
> }
>
> static int scpsys_bus_protect_disable(struct scp_domain *scpd)
> {
> struct scp *scp = scpd->scp;
> + const struct bus_prot *bp_table = scpd->data->bp_table;
> struct regmap *infracfg = scp->infracfg;
> - u32 mask = scpd->data->bus_prot_mask;
> - bool reg_update = scp->bus_prot_reg_update;
> - u32 val;
> - int ret;
> + struct regmap *smi_common = scp->smi_common;
> + int i;
>
> - if (!mask)
> - return 0;
> + for (i = MAX_STEPS - 1; i >= 0; i--) {
> + struct regmap *map = NULL;
> + int ret;
> + struct bus_prot bp = bp_table[i];
The same comments done for scpsys_bus_protect_disable apply here.
>
> - if (reg_update)
> - regmap_update_bits(infracfg, INFRA_TOPAXI_PROTECTEN, mask, 0);
> - else
> - regmap_write(infracfg, INFRA_TOPAXI_PROTECTEN_CLR, mask);
> + if (bp.type == IFR_TYPE)
> + map = infracfg;
> + else if (bp.type == SMI_TYPE)
> + map = smi_common;
> + else
> + continue;
>
> - ret = regmap_read_poll_timeout(infracfg, INFRA_TOPAXI_PROTECTSTA1,
> - val, !(val & mask),
> - MTK_POLL_DELAY_US, MTK_POLL_TIMEOUT);
> + ret = clear_bus_protection(map, &bp);
>
> - return ret;
> + if (ret)
> + return ret;
> + }
> +
> + return 0;
> }
>
> static int scpsys_power_on(struct generic_pm_domain *genpd)
> @@ -430,8 +478,7 @@ static int init_basic_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,
> - bool bus_prot_reg_update)
> + const struct scp_ctrl_reg *scp_ctrl_reg)
> {
> struct genpd_onecell_data *pd_data;
> struct resource *res;
> @@ -445,8 +492,6 @@ static struct scp *init_scp(struct platform_device *pdev,
> 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->bus_prot_reg_update = bus_prot_reg_update;
> -
> scp->dev = &pdev->dev;
>
> res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> @@ -474,6 +519,17 @@ static struct scp *init_scp(struct platform_device *pdev,
> return ERR_CAST(scp->infracfg);
> }
>
> + scp->smi_common = syscon_regmap_lookup_by_phandle(pdev->dev.of_node,
> + "smi_comm");
Hmm, is this property documented? I couldn't find it.
> +
> + if (scp->smi_common == ERR_PTR(-ENODEV)) {
> + scp->smi_common = NULL;
> + } else if (IS_ERR(scp->smi_common)) {
> + dev_err(&pdev->dev, "Cannot find smi_common controller: %ld\n",
> + PTR_ERR(scp->smi_common));
> + return ERR_CAST(scp->smi_common);
> + }
> +
> for (i = 0; i < num; i++) {
> struct scp_domain *scpd = &scp->domains[i];
> const struct scp_domain_data *data = &scp_domain_data[i];
> @@ -556,8 +612,11 @@ static void mtk_register_power_domains(struct platform_device *pdev,
> .name = "conn",
> .sta_mask = PWR_STATUS_CONN,
> .ctl_offs = SPM_CONN_PWR_CON,
> - .bus_prot_mask = MT2701_TOP_AXI_PROT_EN_CONN_M |
> - MT2701_TOP_AXI_PROT_EN_CONN_S,
> + .bp_table = {
> + BUS_PROT(IFR_TYPE, 0, 0, 0x220, 0x228,
> + MT2701_TOP_AXI_PROT_EN_CONN_M |
> + MT2701_TOP_AXI_PROT_EN_CONN_S),
> + },
> .caps = MTK_SCPD_ACTIVE_WAKEUP,
> },
> [MT2701_POWER_DOMAIN_DISP] = {
> @@ -566,7 +625,10 @@ static void mtk_register_power_domains(struct platform_device *pdev,
> .ctl_offs = SPM_DIS_PWR_CON,
> .sram_pdn_bits = GENMASK(11, 8),
> .basic_clk_name = {"mm"},
> - .bus_prot_mask = MT2701_TOP_AXI_PROT_EN_MM_M0,
> + .bp_table = {
> + BUS_PROT(IFR_TYPE, 0, 0, 0x220, 0x228,
> + MT2701_TOP_AXI_PROT_EN_MM_M0),
> + },
> .caps = MTK_SCPD_ACTIVE_WAKEUP,
> },
> [MT2701_POWER_DOMAIN_MFG] = {
> @@ -701,7 +763,10 @@ static void mtk_register_power_domains(struct platform_device *pdev,
> .sram_pdn_bits = GENMASK(8, 8),
> .sram_pdn_ack_bits = GENMASK(16, 16),
> .basic_clk_name = {"mfg"},
> - .bus_prot_mask = BIT(14) | BIT(21) | BIT(23),
> + .bp_table = {
> + BUS_PROT(IFR_TYPE, 0x260, 0x264, 0x220, 0x228,
> + BIT(14) | BIT(21) | BIT(23)),
> + },
> .caps = MTK_SCPD_ACTIVE_WAKEUP,
> },
> [MT2712_POWER_DOMAIN_MFG_SC1] = {
> @@ -773,7 +838,10 @@ static void mtk_register_power_domains(struct platform_device *pdev,
> .sram_pdn_bits = GENMASK(8, 8),
> .sram_pdn_ack_bits = GENMASK(12, 12),
> .basic_clk_name = {"mm"},
> - .bus_prot_mask = (BIT(1) | BIT(2)),
> + .bp_table = {
> + BUS_PROT(IFR_TYPE, 0, 0, 0x220, 0x228,
> + BIT(1) | BIT(2)),
> + },
> },
> [MT6797_POWER_DOMAIN_AUDIO] = {
> .name = "audio",
> @@ -820,7 +888,10 @@ static void mtk_register_power_domains(struct platform_device *pdev,
> .ctl_offs = SPM_ETHSYS_PWR_CON,
> .sram_pdn_bits = GENMASK(11, 8),
> .sram_pdn_ack_bits = GENMASK(15, 12),
> - .bus_prot_mask = MT7622_TOP_AXI_PROT_EN_ETHSYS,
> + .bp_table = {
> + BUS_PROT(IFR_TYPE, 0, 0, 0x220, 0x228,
> + MT7622_TOP_AXI_PROT_EN_ETHSYS),
> + },
> .caps = MTK_SCPD_ACTIVE_WAKEUP,
> },
> [MT7622_POWER_DOMAIN_HIF0] = {
> @@ -830,7 +901,10 @@ static void mtk_register_power_domains(struct platform_device *pdev,
> .sram_pdn_bits = GENMASK(11, 8),
> .sram_pdn_ack_bits = GENMASK(15, 12),
> .basic_clk_name = {"hif_sel"},
> - .bus_prot_mask = MT7622_TOP_AXI_PROT_EN_HIF0,
> + .bp_table = {
> + BUS_PROT(IFR_TYPE, 0, 0, 0x220, 0x228,
> + MT7622_TOP_AXI_PROT_EN_HIF0),
> + },
> .caps = MTK_SCPD_ACTIVE_WAKEUP,
> },
> [MT7622_POWER_DOMAIN_HIF1] = {
> @@ -840,7 +914,10 @@ static void mtk_register_power_domains(struct platform_device *pdev,
> .sram_pdn_bits = GENMASK(11, 8),
> .sram_pdn_ack_bits = GENMASK(15, 12),
> .basic_clk_name = {"hif_sel"},
> - .bus_prot_mask = MT7622_TOP_AXI_PROT_EN_HIF1,
> + .bp_table = {
> + BUS_PROT(IFR_TYPE, 0, 0, 0x220, 0x228,
> + MT7622_TOP_AXI_PROT_EN_HIF1),
> + },
> .caps = MTK_SCPD_ACTIVE_WAKEUP,
> },
> [MT7622_POWER_DOMAIN_WB] = {
> @@ -849,7 +926,10 @@ static void mtk_register_power_domains(struct platform_device *pdev,
> .ctl_offs = SPM_WB_PWR_CON,
> .sram_pdn_bits = 0,
> .sram_pdn_ack_bits = 0,
> - .bus_prot_mask = MT7622_TOP_AXI_PROT_EN_WB,
> + .bp_table = {
> + BUS_PROT(IFR_TYPE, 0, 0, 0x220, 0x228,
> + MT7622_TOP_AXI_PROT_EN_WB),
> + },
> .caps = MTK_SCPD_ACTIVE_WAKEUP | MTK_SCPD_FWAIT_SRAM,
> },
> };
> @@ -863,8 +943,11 @@ static void mtk_register_power_domains(struct platform_device *pdev,
> .name = "conn",
> .sta_mask = PWR_STATUS_CONN,
> .ctl_offs = SPM_CONN_PWR_CON,
> - .bus_prot_mask = MT2701_TOP_AXI_PROT_EN_CONN_M |
> - MT2701_TOP_AXI_PROT_EN_CONN_S,
> + .bp_table = {
> + BUS_PROT(IFR_TYPE, 0, 0, 0x220, 0x228,
> + MT2701_TOP_AXI_PROT_EN_CONN_M |
> + MT2701_TOP_AXI_PROT_EN_CONN_S),
> + },
> .caps = MTK_SCPD_ACTIVE_WAKEUP,
> },
> [MT7623A_POWER_DOMAIN_ETH] = {
> @@ -929,8 +1012,11 @@ static void mtk_register_power_domains(struct platform_device *pdev,
> .sram_pdn_bits = GENMASK(11, 8),
> .sram_pdn_ack_bits = GENMASK(12, 12),
> .basic_clk_name = {"mm"},
> - .bus_prot_mask = MT8173_TOP_AXI_PROT_EN_MM_M0 |
> - MT8173_TOP_AXI_PROT_EN_MM_M1,
> + .bp_table = {
> + BUS_PROT(IFR_TYPE, 0, 0, 0x220, 0x228,
> + MT8173_TOP_AXI_PROT_EN_MM_M0 |
> + MT8173_TOP_AXI_PROT_EN_MM_M1),
> + },
> },
> [MT8173_POWER_DOMAIN_VENC_LT] = {
> .name = "venc_lt",
> @@ -976,10 +1062,13 @@ static void mtk_register_power_domains(struct platform_device *pdev,
> .ctl_offs = SPM_MFG_PWR_CON,
> .sram_pdn_bits = GENMASK(13, 8),
> .sram_pdn_ack_bits = GENMASK(21, 16),
> - .bus_prot_mask = MT8173_TOP_AXI_PROT_EN_MFG_S |
> - MT8173_TOP_AXI_PROT_EN_MFG_M0 |
> - MT8173_TOP_AXI_PROT_EN_MFG_M1 |
> - MT8173_TOP_AXI_PROT_EN_MFG_SNOOP_OUT,
> + .bp_table = {
> + BUS_PROT(IFR_TYPE, 0, 0, 0x220, 0x228,
> + MT8173_TOP_AXI_PROT_EN_MFG_S |
> + MT8173_TOP_AXI_PROT_EN_MFG_M0 |
> + MT8173_TOP_AXI_PROT_EN_MFG_M1 |
> + MT8173_TOP_AXI_PROT_EN_MFG_SNOOP_OUT),
> + },
> },
> };
>
> @@ -995,7 +1084,6 @@ static void mtk_register_power_domains(struct platform_device *pdev,
> .pwr_sta_offs = SPM_PWR_STATUS,
> .pwr_sta2nd_offs = SPM_PWR_STATUS_2ND
> },
> - .bus_prot_reg_update = true,
> };
>
> static const struct scp_soc_data mt2712_data = {
> @@ -1007,7 +1095,6 @@ static void mtk_register_power_domains(struct platform_device *pdev,
> .pwr_sta_offs = SPM_PWR_STATUS,
> .pwr_sta2nd_offs = SPM_PWR_STATUS_2ND
> },
> - .bus_prot_reg_update = false,
> };
>
> static const struct scp_soc_data mt6797_data = {
> @@ -1019,7 +1106,6 @@ static void mtk_register_power_domains(struct platform_device *pdev,
> .pwr_sta_offs = SPM_PWR_STATUS_MT6797,
> .pwr_sta2nd_offs = SPM_PWR_STATUS_2ND_MT6797
> },
> - .bus_prot_reg_update = true,
> };
>
> static const struct scp_soc_data mt7622_data = {
> @@ -1029,7 +1115,6 @@ static void mtk_register_power_domains(struct platform_device *pdev,
> .pwr_sta_offs = SPM_PWR_STATUS,
> .pwr_sta2nd_offs = SPM_PWR_STATUS_2ND
> },
> - .bus_prot_reg_update = true,
> };
>
> static const struct scp_soc_data mt7623a_data = {
> @@ -1039,7 +1124,6 @@ static void mtk_register_power_domains(struct platform_device *pdev,
> .pwr_sta_offs = SPM_PWR_STATUS,
> .pwr_sta2nd_offs = SPM_PWR_STATUS_2ND
> },
> - .bus_prot_reg_update = true,
> };
>
> static const struct scp_soc_data mt8173_data = {
> @@ -1051,7 +1135,6 @@ static void mtk_register_power_domains(struct platform_device *pdev,
> .pwr_sta_offs = SPM_PWR_STATUS,
> .pwr_sta2nd_offs = SPM_PWR_STATUS_2ND
> },
> - .bus_prot_reg_update = true,
> };
>
> /*
> @@ -1092,8 +1175,7 @@ static int scpsys_probe(struct platform_device *pdev)
>
> soc = of_device_get_match_data(&pdev->dev);
>
> - scp = init_scp(pdev, soc->domains, soc->num_domains, &soc->regs,
> - soc->bus_prot_reg_update);
> + scp = init_scp(pdev, soc->domains, soc->num_domains, &soc->regs);
> if (IS_ERR(scp))
> return PTR_ERR(scp);
>
> diff --git a/drivers/soc/mediatek/scpsys.h b/drivers/soc/mediatek/scpsys.h
> index ad2559b..45bc644 100644
> --- a/drivers/soc/mediatek/scpsys.h
> +++ b/drivers/soc/mediatek/scpsys.h
> @@ -2,10 +2,28 @@
> #ifndef __SOC_MEDIATEK_SCPSYS_H
> #define __SOC_MEDIATEK_SCPSYS_H
>
> -#define INFRA_TOPAXI_PROTECTEN 0x0220
> -#define INFRA_TOPAXI_PROTECTSTA1 0x0228
> -#define INFRA_TOPAXI_PROTECTEN_SET 0x0260
> -#define INFRA_TOPAXI_PROTECTEN_CLR 0x0264
> +#define MAX_STEPS 4
> +
> +#define _BUS_PROT(_type, _set_ofs, _clr_ofs, \
> + _en_ofs, _sta_ofs, _mask, _ignore_clr_ack) { \
> + .type = _type, \
> + .set_ofs = _set_ofs, \
> + .clr_ofs = _clr_ofs, \
> + .en_ofs = _en_ofs, \
> + .sta_ofs = _sta_ofs, \
> + .mask = _mask, \
> + .ignore_clr_ack = _ignore_clr_ack, \
> + }
> +
> +#define BUS_PROT(_type, _set_ofs, _clr_ofs, \
> + _en_ofs, _sta_ofs, _mask) \
> + _BUS_PROT(_type, _set_ofs, _clr_ofs, \
> + _en_ofs, _sta_ofs, _mask, false)
> +
> +#define BUS_PROT_IGN(_type, _set_ofs, _clr_ofs, \
> + _en_ofs, _sta_ofs, _mask) \
> + _BUS_PROT(_type, _set_ofs, _clr_ofs, \
> + _en_ofs, _sta_ofs, _mask, true)
>
> #define MT2701_TOP_AXI_PROT_EN_MM_M0 BIT(1)
> #define MT2701_TOP_AXI_PROT_EN_CONN_M BIT(2)
> @@ -25,4 +43,20 @@
> #define MT8173_TOP_AXI_PROT_EN_MFG_M1 BIT(22)
> #define MT8173_TOP_AXI_PROT_EN_MFG_SNOOP_OUT BIT(23)
>
> +enum regmap_type {
> + INVALID_TYPE = 0,
I didn't see any use of that type, is this an option?
> + IFR_TYPE,
> + SMI_TYPE,
> +};
> +
> +struct bus_prot {
> + enum regmap_type type;
> + u32 set_ofs;
> + u32 clr_ofs;
> + u32 en_ofs;
> + u32 sta_ofs;
> + u32 mask;
> + bool ignore_clr_ack;
> +};
> +
> #endif /* __SOC_MEDIATEK_SCPSYS_H */
> --
> 1.8.1.1.dirty
> _______________________________________________
> Linux-mediatek mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-mediatek
On Fri, 2020-04-24 at 02:19 +0800, Enric Balletbo Serra wrote:
> Hi Wiyi Lu,
>
> Thank you for your patch.
>
> Missatge de Weiyi Lu <[email protected]> del dia dv., 20 de març
> 2020 a les 8:33:
> >
> > Both MT8183 & MT6765 have more control steps of bus protection
> > than previous project. And there add more bus protection registers
> > reside at infracfg & smi-common.
> > Extend function to support multiple step bus protection control
> > with more customized arguments.
> > And then use bp_table for bus protection of all compatibles,
> > instead of mixing bus_prot_mask and bus_prot_reg_update.
> >
> > Signed-off-by: Weiyi Lu <[email protected]>
> > ---
> > drivers/soc/mediatek/mtk-scpsys.c | 206 ++++++++++++++++++++++++++------------
> > drivers/soc/mediatek/scpsys.h | 42 +++++++-
> > 2 files changed, 182 insertions(+), 66 deletions(-)
> >
> > diff --git a/drivers/soc/mediatek/mtk-scpsys.c b/drivers/soc/mediatek/mtk-scpsys.c
> > index e50a568..a4fb0b23 100644
> > --- a/drivers/soc/mediatek/mtk-scpsys.c
> > +++ b/drivers/soc/mediatek/mtk-scpsys.c
> > @@ -88,9 +88,9 @@
> > * @ctl_offs: The offset for main power control register.
> > * @sram_pdn_bits: The mask for sram power control bits.
> > * @sram_pdn_ack_bits: The mask for sram power control acked bits.
> > - * @bus_prot_mask: The mask for single step bus protection.
> > * @basic_clk_name: The basic clocks required by this power domain.
> > * @caps: The flag for active wake-up action.
> > + * @bp_table: The mask table for multiple step bus protection.
> > */
> > struct scp_domain_data {
> > const char *name;
> > @@ -98,9 +98,9 @@ struct scp_domain_data {
> > int ctl_offs;
> > u32 sram_pdn_bits;
> > u32 sram_pdn_ack_bits;
> > - u32 bus_prot_mask;
> > const char *basic_clk_name[MAX_CLKS];
> > u8 caps;
> > + struct bus_prot bp_table[MAX_STEPS];
> > };
> >
> > struct scp;
> > @@ -124,8 +124,8 @@ struct scp {
> > struct device *dev;
> > void __iomem *base;
> > struct regmap *infracfg;
> > + struct regmap *smi_common;
> > struct scp_ctrl_reg ctrl_reg;
> > - bool bus_prot_reg_update;
> > };
> >
> > struct scp_subdomain {
> > @@ -139,7 +139,6 @@ struct scp_soc_data {
> > const struct scp_subdomain *subdomains;
> > int num_subdomains;
> > const struct scp_ctrl_reg regs;
> > - bool bus_prot_reg_update;
> > };
> >
> > static int scpsys_domain_is_on(struct scp_domain *scpd)
> > @@ -249,53 +248,102 @@ static int scpsys_sram_disable(struct scp_domain *scpd, void __iomem *ctl_addr)
> > MTK_POLL_DELAY_US, MTK_POLL_TIMEOUT);
> > }
> >
> > +static int set_bus_protection(struct regmap *map, struct bus_prot *bp)
> > +{
> > + u32 val;
> > + u32 set_ofs = bp->set_ofs;
> > + u32 en_ofs = bp->en_ofs;
> > + u32 sta_ofs = bp->sta_ofs;
> > + u32 mask = bp->mask;
>
> Remove unnecessary local variables (set_ofs, en_ofs, sta_ofs and
> mask), and use their bp-> form directly.
>
OK, I'll fix it.
> > +
> > + if (set_ofs)
> > + regmap_write(map, set_ofs, mask);
> > + else
> > + regmap_update_bits(map, en_ofs, mask, mask);
> > +
> > + return regmap_read_poll_timeout(map, sta_ofs,
> > + val, (val & mask) == mask,
> > + MTK_POLL_DELAY_US, MTK_POLL_TIMEOUT);
> > +}
> > +
> > +static int clear_bus_protection(struct regmap *map, struct bus_prot *bp)
> > +{
> > + u32 val;
> > + u32 clr_ofs = bp->clr_ofs;
> > + u32 en_ofs = bp->en_ofs;
> > + u32 sta_ofs = bp->sta_ofs;
> > + u32 mask = bp->mask;
> > + bool ignore_ack = bp->ignore_clr_ack;
>
> Remove unnecessary local variables (clr_ofs, en_ofs, sta_ofs, mask and
> ignore_ack), and use their bp-> form directly.
>
OK, I'll fix it.
> > +
> > + if (clr_ofs)
> > + regmap_write(map, clr_ofs, mask);
> > + else
> > + regmap_update_bits(map, en_ofs, mask, 0);
> > +
> > + if (ignore_ack)
> > + return 0;
> > +
> > + return regmap_read_poll_timeout(map, sta_ofs,
> > + val, !(val & mask),
> > + MTK_POLL_DELAY_US, MTK_POLL_TIMEOUT);
> > +}
> > +
> > static int scpsys_bus_protect_enable(struct scp_domain *scpd)
> > {
> > struct scp *scp = scpd->scp;
> > + const struct bus_prot *bp_table = scpd->data->bp_table;
> > struct regmap *infracfg = scp->infracfg;
> > - u32 mask = scpd->data->bus_prot_mask;
> > - bool reg_update = scp->bus_prot_reg_update;
> > - u32 val;
> > - int ret;
>
> No need to remove ret if you're going to add anyway later.
>
Got it.
> > + struct regmap *smi_common = scp->smi_common;
> > + int i;
> >
> > - if (!mask)
> > - return 0;
> > + for (i = 0; i < MAX_STEPS; i++) {
> > + struct regmap *map = NULL;
>
> Declare this on top of the function and no need to initialize to NUlL
>
Got it.
> > + int ret;
> > + struct bus_prot bp = bp_table[i];
>
> Unneeded local variable ...
>
Actually I followed Nicolas's suggestion in [1]
But, it's okay to me to change it back.
[1] https://patchwork.kernel.org/patch/11304709/
> >
> > - if (reg_update)
> > - regmap_update_bits(infracfg, INFRA_TOPAXI_PROTECTEN, mask,
> > - mask);
> > - else
> > - regmap_write(infracfg, INFRA_TOPAXI_PROTECTEN_SET, mask);
> > + if (bp.type == IFR_TYPE)
>
> bp_table[i].type?
>
Got it.
> > + map = infracfg;
> > + else if (bp.type == SMI_TYPE)
>
> ditto
>
Got it.
> > + map = smi_common;
> > + else
> > + break;
>
> So if the type is invalid you break the loop, this is different to
> what you are doing below with the scpsys_bus_protect_protect_disable()
> function where you are "ignoring" instead of beaking the loop. I am
> wondering why? Looks to me that, or you break and print an error
> because something was really wrong? or you ignore in both cases?
>
The bp_table inside the scp_domain_data_mt8183 is static variable with
maximum MAX_STEPS member count. So, for those power domains with less
bus protection steps, invalid type will be used.
> >
> > - ret = regmap_read_poll_timeout(infracfg, INFRA_TOPAXI_PROTECTSTA1,
> > - val, (val & mask) == mask,
> > - MTK_POLL_DELAY_US, MTK_POLL_TIMEOUT);
> > + ret = set_bus_protection(map, &bp);
> >
> > - return ret;
> > + if (ret)
> > + return ret;
> > + }
> > +
> > + return 0;
> > }
> >
> > static int scpsys_bus_protect_disable(struct scp_domain *scpd)
> > {
> > struct scp *scp = scpd->scp;
> > + const struct bus_prot *bp_table = scpd->data->bp_table;
> > struct regmap *infracfg = scp->infracfg;
> > - u32 mask = scpd->data->bus_prot_mask;
> > - bool reg_update = scp->bus_prot_reg_update;
> > - u32 val;
> > - int ret;
> > + struct regmap *smi_common = scp->smi_common;
> > + int i;
> >
> > - if (!mask)
> > - return 0;
> > + for (i = MAX_STEPS - 1; i >= 0; i--) {
> > + struct regmap *map = NULL;
> > + int ret;
> > + struct bus_prot bp = bp_table[i];
>
> The same comments done for scpsys_bus_protect_disable apply here.
>
Got it.
> >
> > - if (reg_update)
> > - regmap_update_bits(infracfg, INFRA_TOPAXI_PROTECTEN, mask, 0);
> > - else
> > - regmap_write(infracfg, INFRA_TOPAXI_PROTECTEN_CLR, mask);
> > + if (bp.type == IFR_TYPE)
> > + map = infracfg;
> > + else if (bp.type == SMI_TYPE)
> > + map = smi_common;
> > + else
> > + continue;
> >
> > - ret = regmap_read_poll_timeout(infracfg, INFRA_TOPAXI_PROTECTSTA1,
> > - val, !(val & mask),
> > - MTK_POLL_DELAY_US, MTK_POLL_TIMEOUT);
> > + ret = clear_bus_protection(map, &bp);
> >
> > - return ret;
> > + if (ret)
> > + return ret;
> > + }
> > +
> > + return 0;
> > }
> >
> > static int scpsys_power_on(struct generic_pm_domain *genpd)
> > @@ -430,8 +478,7 @@ static int init_basic_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,
> > - bool bus_prot_reg_update)
> > + const struct scp_ctrl_reg *scp_ctrl_reg)
> > {
> > struct genpd_onecell_data *pd_data;
> > struct resource *res;
> > @@ -445,8 +492,6 @@ static struct scp *init_scp(struct platform_device *pdev,
> > 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->bus_prot_reg_update = bus_prot_reg_update;
> > -
> > scp->dev = &pdev->dev;
> >
> > res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > @@ -474,6 +519,17 @@ static struct scp *init_scp(struct platform_device *pdev,
> > return ERR_CAST(scp->infracfg);
> > }
> >
> > + scp->smi_common = syscon_regmap_lookup_by_phandle(pdev->dev.of_node,
> > + "smi_comm");
>
> Hmm, is this property documented? I couldn't find it.
>
Thanks, I'll add in the dt-binding document in PATCH 02.
> > +
> > + if (scp->smi_common == ERR_PTR(-ENODEV)) {
> > + scp->smi_common = NULL;
> > + } else if (IS_ERR(scp->smi_common)) {
> > + dev_err(&pdev->dev, "Cannot find smi_common controller: %ld\n",
> > + PTR_ERR(scp->smi_common));
> > + return ERR_CAST(scp->smi_common);
> > + }
> > +
> > for (i = 0; i < num; i++) {
> > struct scp_domain *scpd = &scp->domains[i];
> > const struct scp_domain_data *data = &scp_domain_data[i];
> > @@ -556,8 +612,11 @@ static void mtk_register_power_domains(struct platform_device *pdev,
> > .name = "conn",
> > .sta_mask = PWR_STATUS_CONN,
> > .ctl_offs = SPM_CONN_PWR_CON,
> > - .bus_prot_mask = MT2701_TOP_AXI_PROT_EN_CONN_M |
> > - MT2701_TOP_AXI_PROT_EN_CONN_S,
> > + .bp_table = {
> > + BUS_PROT(IFR_TYPE, 0, 0, 0x220, 0x228,
> > + MT2701_TOP_AXI_PROT_EN_CONN_M |
> > + MT2701_TOP_AXI_PROT_EN_CONN_S),
> > + },
> > .caps = MTK_SCPD_ACTIVE_WAKEUP,
> > },
> > [MT2701_POWER_DOMAIN_DISP] = {
> > @@ -566,7 +625,10 @@ static void mtk_register_power_domains(struct platform_device *pdev,
> > .ctl_offs = SPM_DIS_PWR_CON,
> > .sram_pdn_bits = GENMASK(11, 8),
> > .basic_clk_name = {"mm"},
> > - .bus_prot_mask = MT2701_TOP_AXI_PROT_EN_MM_M0,
> > + .bp_table = {
> > + BUS_PROT(IFR_TYPE, 0, 0, 0x220, 0x228,
> > + MT2701_TOP_AXI_PROT_EN_MM_M0),
> > + },
> > .caps = MTK_SCPD_ACTIVE_WAKEUP,
> > },
> > [MT2701_POWER_DOMAIN_MFG] = {
> > @@ -701,7 +763,10 @@ static void mtk_register_power_domains(struct platform_device *pdev,
> > .sram_pdn_bits = GENMASK(8, 8),
> > .sram_pdn_ack_bits = GENMASK(16, 16),
> > .basic_clk_name = {"mfg"},
> > - .bus_prot_mask = BIT(14) | BIT(21) | BIT(23),
> > + .bp_table = {
> > + BUS_PROT(IFR_TYPE, 0x260, 0x264, 0x220, 0x228,
> > + BIT(14) | BIT(21) | BIT(23)),
> > + },
> > .caps = MTK_SCPD_ACTIVE_WAKEUP,
> > },
> > [MT2712_POWER_DOMAIN_MFG_SC1] = {
> > @@ -773,7 +838,10 @@ static void mtk_register_power_domains(struct platform_device *pdev,
> > .sram_pdn_bits = GENMASK(8, 8),
> > .sram_pdn_ack_bits = GENMASK(12, 12),
> > .basic_clk_name = {"mm"},
> > - .bus_prot_mask = (BIT(1) | BIT(2)),
> > + .bp_table = {
> > + BUS_PROT(IFR_TYPE, 0, 0, 0x220, 0x228,
> > + BIT(1) | BIT(2)),
> > + },
> > },
> > [MT6797_POWER_DOMAIN_AUDIO] = {
> > .name = "audio",
> > @@ -820,7 +888,10 @@ static void mtk_register_power_domains(struct platform_device *pdev,
> > .ctl_offs = SPM_ETHSYS_PWR_CON,
> > .sram_pdn_bits = GENMASK(11, 8),
> > .sram_pdn_ack_bits = GENMASK(15, 12),
> > - .bus_prot_mask = MT7622_TOP_AXI_PROT_EN_ETHSYS,
> > + .bp_table = {
> > + BUS_PROT(IFR_TYPE, 0, 0, 0x220, 0x228,
> > + MT7622_TOP_AXI_PROT_EN_ETHSYS),
> > + },
> > .caps = MTK_SCPD_ACTIVE_WAKEUP,
> > },
> > [MT7622_POWER_DOMAIN_HIF0] = {
> > @@ -830,7 +901,10 @@ static void mtk_register_power_domains(struct platform_device *pdev,
> > .sram_pdn_bits = GENMASK(11, 8),
> > .sram_pdn_ack_bits = GENMASK(15, 12),
> > .basic_clk_name = {"hif_sel"},
> > - .bus_prot_mask = MT7622_TOP_AXI_PROT_EN_HIF0,
> > + .bp_table = {
> > + BUS_PROT(IFR_TYPE, 0, 0, 0x220, 0x228,
> > + MT7622_TOP_AXI_PROT_EN_HIF0),
> > + },
> > .caps = MTK_SCPD_ACTIVE_WAKEUP,
> > },
> > [MT7622_POWER_DOMAIN_HIF1] = {
> > @@ -840,7 +914,10 @@ static void mtk_register_power_domains(struct platform_device *pdev,
> > .sram_pdn_bits = GENMASK(11, 8),
> > .sram_pdn_ack_bits = GENMASK(15, 12),
> > .basic_clk_name = {"hif_sel"},
> > - .bus_prot_mask = MT7622_TOP_AXI_PROT_EN_HIF1,
> > + .bp_table = {
> > + BUS_PROT(IFR_TYPE, 0, 0, 0x220, 0x228,
> > + MT7622_TOP_AXI_PROT_EN_HIF1),
> > + },
> > .caps = MTK_SCPD_ACTIVE_WAKEUP,
> > },
> > [MT7622_POWER_DOMAIN_WB] = {
> > @@ -849,7 +926,10 @@ static void mtk_register_power_domains(struct platform_device *pdev,
> > .ctl_offs = SPM_WB_PWR_CON,
> > .sram_pdn_bits = 0,
> > .sram_pdn_ack_bits = 0,
> > - .bus_prot_mask = MT7622_TOP_AXI_PROT_EN_WB,
> > + .bp_table = {
> > + BUS_PROT(IFR_TYPE, 0, 0, 0x220, 0x228,
> > + MT7622_TOP_AXI_PROT_EN_WB),
> > + },
> > .caps = MTK_SCPD_ACTIVE_WAKEUP | MTK_SCPD_FWAIT_SRAM,
> > },
> > };
> > @@ -863,8 +943,11 @@ static void mtk_register_power_domains(struct platform_device *pdev,
> > .name = "conn",
> > .sta_mask = PWR_STATUS_CONN,
> > .ctl_offs = SPM_CONN_PWR_CON,
> > - .bus_prot_mask = MT2701_TOP_AXI_PROT_EN_CONN_M |
> > - MT2701_TOP_AXI_PROT_EN_CONN_S,
> > + .bp_table = {
> > + BUS_PROT(IFR_TYPE, 0, 0, 0x220, 0x228,
> > + MT2701_TOP_AXI_PROT_EN_CONN_M |
> > + MT2701_TOP_AXI_PROT_EN_CONN_S),
> > + },
> > .caps = MTK_SCPD_ACTIVE_WAKEUP,
> > },
> > [MT7623A_POWER_DOMAIN_ETH] = {
> > @@ -929,8 +1012,11 @@ static void mtk_register_power_domains(struct platform_device *pdev,
> > .sram_pdn_bits = GENMASK(11, 8),
> > .sram_pdn_ack_bits = GENMASK(12, 12),
> > .basic_clk_name = {"mm"},
> > - .bus_prot_mask = MT8173_TOP_AXI_PROT_EN_MM_M0 |
> > - MT8173_TOP_AXI_PROT_EN_MM_M1,
> > + .bp_table = {
> > + BUS_PROT(IFR_TYPE, 0, 0, 0x220, 0x228,
> > + MT8173_TOP_AXI_PROT_EN_MM_M0 |
> > + MT8173_TOP_AXI_PROT_EN_MM_M1),
> > + },
> > },
> > [MT8173_POWER_DOMAIN_VENC_LT] = {
> > .name = "venc_lt",
> > @@ -976,10 +1062,13 @@ static void mtk_register_power_domains(struct platform_device *pdev,
> > .ctl_offs = SPM_MFG_PWR_CON,
> > .sram_pdn_bits = GENMASK(13, 8),
> > .sram_pdn_ack_bits = GENMASK(21, 16),
> > - .bus_prot_mask = MT8173_TOP_AXI_PROT_EN_MFG_S |
> > - MT8173_TOP_AXI_PROT_EN_MFG_M0 |
> > - MT8173_TOP_AXI_PROT_EN_MFG_M1 |
> > - MT8173_TOP_AXI_PROT_EN_MFG_SNOOP_OUT,
> > + .bp_table = {
> > + BUS_PROT(IFR_TYPE, 0, 0, 0x220, 0x228,
> > + MT8173_TOP_AXI_PROT_EN_MFG_S |
> > + MT8173_TOP_AXI_PROT_EN_MFG_M0 |
> > + MT8173_TOP_AXI_PROT_EN_MFG_M1 |
> > + MT8173_TOP_AXI_PROT_EN_MFG_SNOOP_OUT),
> > + },
> > },
> > };
> >
> > @@ -995,7 +1084,6 @@ static void mtk_register_power_domains(struct platform_device *pdev,
> > .pwr_sta_offs = SPM_PWR_STATUS,
> > .pwr_sta2nd_offs = SPM_PWR_STATUS_2ND
> > },
> > - .bus_prot_reg_update = true,
> > };
> >
> > static const struct scp_soc_data mt2712_data = {
> > @@ -1007,7 +1095,6 @@ static void mtk_register_power_domains(struct platform_device *pdev,
> > .pwr_sta_offs = SPM_PWR_STATUS,
> > .pwr_sta2nd_offs = SPM_PWR_STATUS_2ND
> > },
> > - .bus_prot_reg_update = false,
> > };
> >
> > static const struct scp_soc_data mt6797_data = {
> > @@ -1019,7 +1106,6 @@ static void mtk_register_power_domains(struct platform_device *pdev,
> > .pwr_sta_offs = SPM_PWR_STATUS_MT6797,
> > .pwr_sta2nd_offs = SPM_PWR_STATUS_2ND_MT6797
> > },
> > - .bus_prot_reg_update = true,
> > };
> >
> > static const struct scp_soc_data mt7622_data = {
> > @@ -1029,7 +1115,6 @@ static void mtk_register_power_domains(struct platform_device *pdev,
> > .pwr_sta_offs = SPM_PWR_STATUS,
> > .pwr_sta2nd_offs = SPM_PWR_STATUS_2ND
> > },
> > - .bus_prot_reg_update = true,
> > };
> >
> > static const struct scp_soc_data mt7623a_data = {
> > @@ -1039,7 +1124,6 @@ static void mtk_register_power_domains(struct platform_device *pdev,
> > .pwr_sta_offs = SPM_PWR_STATUS,
> > .pwr_sta2nd_offs = SPM_PWR_STATUS_2ND
> > },
> > - .bus_prot_reg_update = true,
> > };
> >
> > static const struct scp_soc_data mt8173_data = {
> > @@ -1051,7 +1135,6 @@ static void mtk_register_power_domains(struct platform_device *pdev,
> > .pwr_sta_offs = SPM_PWR_STATUS,
> > .pwr_sta2nd_offs = SPM_PWR_STATUS_2ND
> > },
> > - .bus_prot_reg_update = true,
> > };
> >
> > /*
> > @@ -1092,8 +1175,7 @@ static int scpsys_probe(struct platform_device *pdev)
> >
> > soc = of_device_get_match_data(&pdev->dev);
> >
> > - scp = init_scp(pdev, soc->domains, soc->num_domains, &soc->regs,
> > - soc->bus_prot_reg_update);
> > + scp = init_scp(pdev, soc->domains, soc->num_domains, &soc->regs);
> > if (IS_ERR(scp))
> > return PTR_ERR(scp);
> >
> > diff --git a/drivers/soc/mediatek/scpsys.h b/drivers/soc/mediatek/scpsys.h
> > index ad2559b..45bc644 100644
> > --- a/drivers/soc/mediatek/scpsys.h
> > +++ b/drivers/soc/mediatek/scpsys.h
> > @@ -2,10 +2,28 @@
> > #ifndef __SOC_MEDIATEK_SCPSYS_H
> > #define __SOC_MEDIATEK_SCPSYS_H
> >
> > -#define INFRA_TOPAXI_PROTECTEN 0x0220
> > -#define INFRA_TOPAXI_PROTECTSTA1 0x0228
> > -#define INFRA_TOPAXI_PROTECTEN_SET 0x0260
> > -#define INFRA_TOPAXI_PROTECTEN_CLR 0x0264
> > +#define MAX_STEPS 4
> > +
> > +#define _BUS_PROT(_type, _set_ofs, _clr_ofs, \
> > + _en_ofs, _sta_ofs, _mask, _ignore_clr_ack) { \
> > + .type = _type, \
> > + .set_ofs = _set_ofs, \
> > + .clr_ofs = _clr_ofs, \
> > + .en_ofs = _en_ofs, \
> > + .sta_ofs = _sta_ofs, \
> > + .mask = _mask, \
> > + .ignore_clr_ack = _ignore_clr_ack, \
> > + }
> > +
> > +#define BUS_PROT(_type, _set_ofs, _clr_ofs, \
> > + _en_ofs, _sta_ofs, _mask) \
> > + _BUS_PROT(_type, _set_ofs, _clr_ofs, \
> > + _en_ofs, _sta_ofs, _mask, false)
> > +
> > +#define BUS_PROT_IGN(_type, _set_ofs, _clr_ofs, \
> > + _en_ofs, _sta_ofs, _mask) \
> > + _BUS_PROT(_type, _set_ofs, _clr_ofs, \
> > + _en_ofs, _sta_ofs, _mask, true)
> >
> > #define MT2701_TOP_AXI_PROT_EN_MM_M0 BIT(1)
> > #define MT2701_TOP_AXI_PROT_EN_CONN_M BIT(2)
> > @@ -25,4 +43,20 @@
> > #define MT8173_TOP_AXI_PROT_EN_MFG_M1 BIT(22)
> > #define MT8173_TOP_AXI_PROT_EN_MFG_SNOOP_OUT BIT(23)
> >
> > +enum regmap_type {
> > + INVALID_TYPE = 0,
>
> I didn't see any use of that type, is this an option?
>
Answered above.
>
> > + IFR_TYPE,
> > + SMI_TYPE,
> > +};
> > +
> > +struct bus_prot {
> > + enum regmap_type type;
> > + u32 set_ofs;
> > + u32 clr_ofs;
> > + u32 en_ofs;
> > + u32 sta_ofs;
> > + u32 mask;
> > + bool ignore_clr_ack;
> > +};
> > +
> > #endif /* __SOC_MEDIATEK_SCPSYS_H */
> > --
> > 1.8.1.1.dirty
> > _______________________________________________
> > Linux-mediatek mailing list
> > [email protected]
> > http://lists.infradead.org/mailman/listinfo/linux-mediatek
>
> _______________________________________________
> Linux-mediatek mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-mediatek