linux-mediatek.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: Weiyi Lu <weiyi.lu@mediatek.com>
To: Matthias Brugger <matthias.bgg@gmail.com>
Cc: Rob Herring <robh@kernel.org>,
	Nicolas Boichat <drinkcat@chromium.org>,
	srv_heupstream@mediatek.com,
	James Liao <jamesjj.liao@mediatek.com>,
	linux-kernel@vger.kernel.org, Fan Chen <fan.chen@mediatek.com>,
	linux-mediatek@lists.infradead.org,
	Sascha Hauer <kernel@pengutronix.de>,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v11 04/10] soc: mediatek: Add multiple step bus protection control
Date: Thu, 13 Feb 2020 10:15:53 +0800	[thread overview]
Message-ID: <1581560153.19424.6.camel@mtksdaap41> (raw)
In-Reply-To: <28bcbd36-c2b9-d69e-55b2-508ad7b63887@gmail.com>

On Wed, 2020-02-12 at 10:23 +0100, Matthias Brugger wrote:
> 
> On 12/02/2020 03:55, Weiyi Lu wrote:
> > On Tue, 2020-02-11 at 18:49 +0100, Matthias Brugger wrote:
> >>
> >> On 20/12/2019 04:45, Weiyi Lu wrote:
> >>> 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. Also add new APIs for 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 <weiyi.lu@mediatek.com>
> >>> ---
> >>>  drivers/soc/mediatek/Makefile         |   2 +-
> >>>  drivers/soc/mediatek/mtk-scpsys-ext.c | 101 +++++++++++++++++++++++++++++
> >>>  drivers/soc/mediatek/mtk-scpsys.c     | 117 +++++++++++++++++++++-------------
> >>>  drivers/soc/mediatek/scpsys-ext.h     |  67 +++++++++++++++++++
> >>>  4 files changed, 240 insertions(+), 47 deletions(-)
> >>>  create mode 100644 drivers/soc/mediatek/mtk-scpsys-ext.c
> >>>  create mode 100644 drivers/soc/mediatek/scpsys-ext.h
> >>>
> >>> diff --git a/drivers/soc/mediatek/Makefile b/drivers/soc/mediatek/Makefile
> >>> index b017330..b442be9 100644
> >>> --- a/drivers/soc/mediatek/Makefile
> >>> +++ b/drivers/soc/mediatek/Makefile
> >>> @@ -1,5 +1,5 @@
> >>>  # SPDX-License-Identifier: GPL-2.0-only
> >>>  obj-$(CONFIG_MTK_CMDQ) += mtk-cmdq-helper.o
> >>> -obj-$(CONFIG_MTK_INFRACFG) += mtk-infracfg.o
> >>> +obj-$(CONFIG_MTK_INFRACFG) += mtk-infracfg.o mtk-scpsys-ext.o
> >>
> >> It seems that we would need another patch which get's rid of the mtk-infracfg
> >> first and then add stuff like the possibility to have different steps.
> >>
> > 
> > Actually I have a PATCH 05/11 to remove the mtk-infracfg.
> > In this patch, I have some changes, like calling
> > mtk_scpsys_ext_set_bus_protection(...) instead of
> > mtk_infracfg_set_bus_protection(...) in scpsys_bus_protect_enable(...)
> > and replacing bus_prot_mask by bp_table.
> > I thought I should introduce the new method first and remove useless one
> > later. What do you think?
> 
> Yes, but first patch would be a step to get rid of mtk-infracfg and a second
> patch add bp_table and the like.
> 

OK, I'll split into such sequence.

> > 
> >>>  obj-$(CONFIG_MTK_PMIC_WRAP) += mtk-pmic-wrap.o
> >>>  obj-$(CONFIG_MTK_SCPSYS) += mtk-scpsys.o
> >>> diff --git a/drivers/soc/mediatek/mtk-scpsys-ext.c b/drivers/soc/mediatek/mtk-scpsys-ext.c
> >>> new file mode 100644
> >>> index 0000000..df402ac
> >>> --- /dev/null
> >>> +++ b/drivers/soc/mediatek/mtk-scpsys-ext.c
> >>
> >> I'm not quite sure why we should put this into a new file. I suppose the
> >> rational behind it is the fact that we access other blocks through regmap.
> >>
> > 
> > Yes, those operation are accross infracfg and smi-common so we put these
> > into new files.
> 
> Are you exepct other drivers to use this functions? If not I will have to think
> again, but I don't see any reason to put it into a new file.
> 

I thought no other driver would use these functions unless there will
have drivers want to take over the bus protection process by themselves.

Do you prefer just putting these functions into mtk-scpsys.c?

> > 
> >>> @@ -0,0 +1,101 @@
> >>> +// SPDX-License-Identifier: GPL-2.0
> >>> +/*
> >>> + * Copyright (c) 2018 MediaTek Inc.
> >>> + * Author: Owen Chen <Owen.Chen@mediatek.com>
> >>> + */
> >>> +#include <linux/ktime.h>
> >>> +#include <linux/mfd/syscon.h>
> >>> +#include <linux/of_device.h>
> >>> +#include <linux/regmap.h>
> >>> +#include "scpsys-ext.h"
> >>> +
> >>> +#define MTK_POLL_DELAY_US   10
> >>> +#define MTK_POLL_TIMEOUT    USEC_PER_SEC
> >>> +
> >>> +static int set_bus_protection(struct regmap *map, u32 mask,
> >>> +		u32 reg_set, u32 reg_sta, u32 reg_en)
> >>> +{
> >>> +	u32 val;
> >>> +
> >>> +	if (reg_set)
> >>> +		regmap_write(map, reg_set, mask);
> >>> +	else
> >>> +		regmap_update_bits(map, reg_en, mask, mask);
> >>> +
> >>> +	return regmap_read_poll_timeout(map, reg_sta,
> >>> +			val, (val & mask) == mask,
> >>> +			MTK_POLL_DELAY_US, MTK_POLL_TIMEOUT);
> >>> +}
> >>> +
> >>> +static int clear_bus_protection(struct regmap *map, u32 mask,
> >>> +		u32 reg_clr, u32 reg_sta, u32 reg_en, bool ignore_ack)
> >>> +{
> >>> +	u32 val;
> >>> +
> >>> +	if (reg_clr)
> >>> +		regmap_write(map, reg_clr, mask);
> >>> +	else
> >>> +		regmap_update_bits(map, reg_en, mask, 0);
> >>> +
> >>> +	if (ignore_ack)
> >>> +		return 0;
> >>> +
> >>> +	return regmap_read_poll_timeout(map, reg_sta,
> >>> +			val, !(val & mask),
> >>> +			MTK_POLL_DELAY_US, MTK_POLL_TIMEOUT);
> >>> +}
> >>> +
> >>> +int mtk_scpsys_ext_set_bus_protection(const struct bus_prot *bp_table,
> >>> +	struct regmap *infracfg, struct regmap *smi_common)
> >>> +{
> >>> +	int i;
> >>> +
> >>> +	for (i = 0; i < MAX_STEPS; i++) {
> >>
> >> struct bus_prot bp_table = bp_table[i];
> >>
> > 
> > OK, got it.
> > 
> >>> +		struct regmap *map = NULL;
> >>> +		int ret;
> >>> +
> >>> +		if (bp_table[i].type == INVALID_TYPE)
> >>> +			break;
> >>> +		else if (bp_table[i].type == IFR_TYPE)
> >>> +			map = infracfg;
> >>> +		else if (bp_table[i].type == SMI_TYPE)
> >>> +			map = smi_common;
> >>
> >> if (bp_table.type == IFR_TYPE)
> >> 	map = infracfg;
> >> else if (bp_table.type == SMI_TYPE)
> >> 	map = smi_common;
> >> else
> >> 	break;
> >>
> > 
> > OK, got it.
> > 
> >>> +
> >>> +		ret = set_bus_protection(map,
> >>> +				bp_table[i].mask, bp_table[i].set_ofs,
> >>> +				bp_table[i].sta_ofs, bp_table[i].en_ofs);
> >>
> >> passing map and bp_table here makes code much more readable. I always end-up
> >> counting the variables when the names in the callee changes.
> >>
> > 
> > OK, I'll fix it.
> > 
> >>> +
> >>> +		if (ret)
> >>> +			return ret;
> >>> +	}
> >>> +
> >>> +	return 0;
> >>> +}
> >>> +
> >>> +int mtk_scpsys_ext_clear_bus_protection(const struct bus_prot *bp_table,
> >>> +	struct regmap *infracfg, struct regmap *smi_common)
> >>> +{
> >>> +	int i;
> >>> +
> >>> +	for (i = MAX_STEPS - 1; i >= 0; i--) {
> >>> +		struct regmap *map = NULL;
> >>> +		int ret;
> >>> +
> >>> +		if (bp_table[i].type == INVALID_TYPE)
> >>> +			continue;
> >>> +		else if (bp_table[i].type == IFR_TYPE)
> >>> +			map = infracfg;
> >>> +		else if (bp_table[i].type == SMI_TYPE)
> >>> +			map = smi_common;
> >>> +
> >>> +		ret = clear_bus_protection(map,
> >>> +				bp_table[i].mask, bp_table[i].clr_ofs,
> >>> +				bp_table[i].sta_ofs, bp_table[i].en_ofs,
> >>> +				bp_table[i].ignore_clr_ack);
> >>> +
> >>
> >> same here.
> >>
> > 
> > Got it.
> > 
> >>> +		if (ret)
> >>> +			return ret;
> >>> +	}
> >>> +
> >>> +	return 0;
> >>> +}
> >>> diff --git a/drivers/soc/mediatek/mtk-scpsys.c b/drivers/soc/mediatek/mtk-scpsys.c
> >>> index db35a28..763ca58 100644
> >>> --- a/drivers/soc/mediatek/mtk-scpsys.c
> >>> +++ b/drivers/soc/mediatek/mtk-scpsys.c
> >>> @@ -11,7 +11,7 @@
> >>>  #include <linux/platform_device.h>
> >>>  #include <linux/pm_domain.h>
> >>>  #include <linux/regulator/consumer.h>
> >>> -#include <linux/soc/mediatek/infracfg.h>
> >>> +#include "scpsys-ext.h"
> >>>  
> >>>  #include <dt-bindings/power/mt2701-power.h>
> >>>  #include <dt-bindings/power/mt2712-power.h>
> >>> @@ -87,9 +87,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;
> >>> @@ -97,9 +97,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;
> >>> @@ -123,8 +123,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 {
> >>> @@ -138,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)
> >>> @@ -252,24 +251,16 @@ static int scpsys_bus_protect_enable(struct scp_domain *scpd)
> >>>  {
> >>>  	struct scp *scp = scpd->scp;
> >>>  
> >>> -	if (!scpd->data->bus_prot_mask)
> >>> -		return 0;
> >>> -
> >>> -	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);
> >>
> >> Now that's just a wrapper which does add nothing but another indirection. We can
> >> call mtk_scpsys_ext_set_bus_protection() directly (also I don't like the name,
> >> actually why don't mtk_scpsys_set_bus_protection()?)
> >>
> > 
> > You're right. I'll called mtk_scpsys_ext_set_bus_protection() directly.
> > 
> >>>  }
> >>>  
> >>>  static int scpsys_bus_protect_disable(struct scp_domain *scpd)
> >>>  {
> >>>  	struct scp *scp = scpd->scp;
> >>>  
> >>> -	if (!scpd->data->bus_prot_mask)
> >>> -		return 0;
> >>> -
> >>> -	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);
> >>>  }
> >>>  
> >>>  static int scpsys_power_on(struct generic_pm_domain *genpd)
> >>> @@ -404,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;
> >>> @@ -419,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);
> >>> @@ -448,6 +436,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];
> >>> @@ -530,8 +529,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] = {
> >>> @@ -540,7 +542,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] = {
> >>> @@ -675,7 +680,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] = {
> >>> @@ -747,7 +755,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",
> >>> @@ -794,7 +805,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] = {
> >>> @@ -804,7 +818,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] = {
> >>> @@ -814,7 +831,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] = {
> >>> @@ -823,7 +843,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,
> >>>  	},
> >>>  };
> >>> @@ -837,8 +860,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] = {
> >>> @@ -903,8 +929,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",
> >>> @@ -950,10 +979,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),
> >>> +		},
> >>>  	},
> >>>  };
> >>>  
> >>> @@ -969,7 +1001,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 = {
> >>> @@ -981,7 +1012,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 = {
> >>> @@ -993,7 +1023,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 = {
> >>> @@ -1003,7 +1032,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 = {
> >>> @@ -1013,7 +1041,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 = {
> >>> @@ -1025,7 +1052,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,
> >>>  };
> >>>  
> >>>  /*
> >>> @@ -1066,8 +1092,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-ext.h b/drivers/soc/mediatek/scpsys-ext.h
> >>> new file mode 100644
> >>> index 0000000..458b2c5
> >>> --- /dev/null
> >>> +++ b/drivers/soc/mediatek/scpsys-ext.h
> >>> @@ -0,0 +1,67 @@
> >>> +/* SPDX-License-Identifier: GPL-2.0 */
> >>> +#ifndef __SOC_MEDIATEK_SCPSYS_EXT_H
> >>> +#define __SOC_MEDIATEK_SCPSYS_EXT_H
> >>> +
> >>> +#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)
> >>> +#define MT2701_TOP_AXI_PROT_EN_CONN_S		BIT(8)
> >>> +
> >>> +#define MT7622_TOP_AXI_PROT_EN_ETHSYS		(BIT(3) | BIT(17))
> >>> +#define MT7622_TOP_AXI_PROT_EN_HIF0		(BIT(24) | BIT(25))
> >>> +#define MT7622_TOP_AXI_PROT_EN_HIF1		(BIT(26) | BIT(27) | \
> >>> +						 BIT(28))
> >>> +#define MT7622_TOP_AXI_PROT_EN_WB		(BIT(2) | BIT(6) | \
> >>> +						 BIT(7) | BIT(8))
> >>> +
> >>> +#define MT8173_TOP_AXI_PROT_EN_MM_M0		BIT(1)
> >>> +#define MT8173_TOP_AXI_PROT_EN_MM_M1		BIT(2)
> >>> +#define MT8173_TOP_AXI_PROT_EN_MFG_S		BIT(14)
> >>> +#define MT8173_TOP_AXI_PROT_EN_MFG_M0		BIT(21)
> >>> +#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;
> >>
> >> Please try to keep variable names consistent. You change for example set_ofs to
> >> reg_set. That makes the code more difficult to read and understand.
> >>
> > 
> > OK, I'll keep those names consistent.
> > 
> >>> +	u32 en_ofs;
> >>> +	u32 sta_ofs;
> >>> +	u32 mask;
> >>> +	bool ignore_clr_ack;
> >>> +};
> >>> +
> >>> +int mtk_scpsys_ext_set_bus_protection(const struct bus_prot *bp_table,
> >>> +	struct regmap *infracfg, struct regmap *smi_common);
> >>> +int mtk_scpsys_ext_clear_bus_protection(const struct bus_prot *bp_table,
> >>> +	struct regmap *infracfg, struct regmap *smi_common);
> >>> +
> >>> +#endif /* __SOC_MEDIATEK_SCPSYS_EXT_H */
> >>>
> > 

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

  reply	other threads:[~2020-02-13  2:16 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-12-20  3:45 [PATCH v11 00/10] Mediatek MT8183 scpsys support Weiyi Lu
2019-12-20  3:45 ` [PATCH v11 01/10] dt-bindings: mediatek: Add property to mt8183 smi-common Weiyi Lu
2020-01-15  5:45   ` Weiyi Lu
2019-12-20  3:45 ` [PATCH v11 02/10] dt-bindings: soc: Add MT8183 power dt-bindings Weiyi Lu
2019-12-20  3:45 ` [PATCH v11 03/10] soc: mediatek: Add basic_clk_name to scp_power_data Weiyi Lu
2019-12-20  4:05   ` Nicolas Boichat
2019-12-20  3:45 ` [PATCH v11 04/10] soc: mediatek: Add multiple step bus protection control Weiyi Lu
2019-12-20  4:09   ` Nicolas Boichat
2020-02-11 17:49   ` Matthias Brugger
2020-02-12  2:55     ` Weiyi Lu
2020-02-12  9:23       ` Matthias Brugger
2020-02-13  2:15         ` Weiyi Lu [this message]
2020-02-13 11:11           ` Matthias Brugger
2019-12-20  3:45 ` [PATCH v11 05/10] soc: mediatek: Remove infracfg misc driver support Weiyi Lu
2019-12-20  4:11   ` Nicolas Boichat
2019-12-20  5:00     ` Weiyi Lu
2019-12-20  5:09       ` Nicolas Boichat
2019-12-20  3:46 ` [PATCH v11 06/10] soc: mediatek: Add subsys clock control for bus protection Weiyi Lu
2020-02-11 17:54   ` Matthias Brugger
2020-02-12  2:55     ` Weiyi Lu
2020-02-12 11:02       ` Matthias Brugger
2020-02-13  2:46         ` Weiyi Lu
2020-02-13 12:56           ` Matthias Brugger
2020-02-14  6:33             ` Weiyi Lu
2020-02-14  6:42             ` Weiyi Lu
2019-12-20  3:46 ` [PATCH v11 07/10] soc: mediatek: Add extra sram control Weiyi Lu
2020-02-11 17:04   ` Matthias Brugger
2020-02-12  2:56     ` Weiyi Lu
2019-12-20  3:46 ` [PATCH v11 08/10] soc: mediatek: Add MT8183 scpsys support Weiyi Lu
2019-12-20  5:11   ` Nicolas Boichat
2019-12-20  3:46 ` [PATCH v11 09/10] arm64: dts: Add power controller device node of MT8183 Weiyi Lu
2019-12-20  3:46 ` [PATCH v11 10/10] arm64: dts: Add power-domains properity to mfgcfg Weiyi Lu
2019-12-27  1:43 ` [PATCH v11 00/10] Mediatek MT8183 scpsys support Weiyi Lu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1581560153.19424.6.camel@mtksdaap41 \
    --to=weiyi.lu@mediatek.com \
    --cc=drinkcat@chromium.org \
    --cc=fan.chen@mediatek.com \
    --cc=jamesjj.liao@mediatek.com \
    --cc=kernel@pengutronix.de \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mediatek@lists.infradead.org \
    --cc=matthias.bgg@gmail.com \
    --cc=robh@kernel.org \
    --cc=srv_heupstream@mediatek.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).