Re: [PATCH v10 06/12] soc: mediatek: Use bp_table for all compatibles

On Thu, 2019-12-19 at 11:54 +0800, Nicolas Boichat wrote:

On Wed, Dec 18, 2019 at 4:31 PM Weiyi Lu [off-list ref] wrote:

quoted

Only use bp_table for bus protection of all compatibles,
instead of mixing bus_prot_mask and bus_prot_reg_update.

ditto, I'd just squash in the previous patch.

OK, I'll update in next version.

quoted

Signed-off-by: Weiyi Lu <redacted>
---
 drivers/soc/mediatek/mtk-scpsys.c | 94 ++++++++++++++++++++-------------------
 1 file changed, 48 insertions(+), 46 deletions(-)

diff --git a/drivers/soc/mediatek/mtk-scpsys.c b/drivers/soc/mediatek/mtk-scpsys.c
index 5699d9f..c438c53 100644
--- a/drivers/soc/mediatek/mtk-scpsys.c
+++ b/drivers/soc/mediatek/mtk-scpsys.c

@@ -11,7 +11,6 @@
 #include <linux/platform_device.h>
 #include <linux/pm_domain.h>
 #include <linux/regulator/consumer.h>
-#include <linux/soc/mediatek/infracfg.h>
 #include <linux/soc/mediatek/scpsys-ext.h>

 #include <dt-bindings/power/mt2701-power.h>

@@ -88,7 +87,6 @@
  * @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.

@@ -99,7 +97,6 @@ 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];

@@ -128,7 +125,6 @@ struct scp {
        struct regmap *infracfg;
        struct regmap *smi_common;
        struct scp_ctrl_reg ctrl_reg;
-       bool bus_prot_reg_update;
 };

 struct scp_subdomain {

@@ -142,7 +138,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)

@@ -256,12 +251,6 @@ static int scpsys_bus_protect_enable(struct scp_domain *scpd)
 {
        struct scp *scp = scpd->scp;

-       if (scpd->data->bus_prot_mask) {
-               return mtk_infracfg_set_bus_protection(scp->infracfg,
-                               scpd->data->bus_prot_mask,
-                               scp->bus_prot_reg_update);
-       }
-
        return mtk_scpsys_ext_set_bus_protection(scpd->data->bp_table,
                        scp->infracfg, scp->smi_common);
 }

@@ -270,12 +259,6 @@ static int scpsys_bus_protect_disable(struct scp_domain *scpd)
 {
        struct scp *scp = scpd->scp;

-       if (scpd->data->bus_prot_mask) {
-               return mtk_infracfg_clear_bus_protection(scp->infracfg,
-                               scpd->data->bus_prot_mask,
-                               scp->bus_prot_reg_update);
-       }
-
        return mtk_scpsys_ext_clear_bus_protection(scpd->data->bp_table,
                        scp->infracfg, scp->smi_common);
 }

@@ -412,8 +395,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;

@@ -427,8 +409,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);

@@ -549,8 +529,10 @@ 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,
+                               BIT(2) | BIT(8), BIT(2) | BIT(8)),
+               },

I'm a bit sad we lose the information about the BIT meaning.

I'll keep those information in next version.

Of course this looks ugly and verbose:
                      BUS_PROT(IFR_TYPE, 0, 0, 0x220, 0x228,
                               MT2701_TOP_AXI_PROT_EN_CONN_M |
MT2701_TOP_AXI_PROT_EN_CONN_S,
                               MT2701_TOP_AXI_PROT_EN_CONN_M |
MT2701_TOP_AXI_PROT_EN_CONN_S),

But if you make "check_clr_mask" a boolean, you wouldn't have to
repeat the mask twice and you could keep the nice register bit
definitions.

Thanks for the suggestion, I'll add "ignore_clr_ack" for it in next
version.

[snip, many similar occurences below]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel