Messages in this thread | | | From | Nicolas Boichat <> | Date | Thu, 19 Dec 2019 11:54:18 +0800 | Subject | Re: [PATCH v10 06/12] soc: mediatek: Use bp_table for all compatibles |
| |
On Wed, Dec 18, 2019 at 4:31 PM Weiyi Lu <weiyi.lu@mediatek.com> wrote: > > 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.
> Signed-off-by: Weiyi Lu <weiyi.lu@mediatek.com> > --- > 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.
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.
[snip, many similar occurences below]
| |