From: Matthias Brugger <matthias.bgg@gmail.com>
To: Weiyi Lu <weiyi.lu@mediatek.com>
Cc: Nicolas Boichat <drinkcat@chromium.org>,
Rob Herring <robh@kernel.org>,
Sascha Hauer <kernel@pengutronix.de>,
James Liao <jamesjj.liao@mediatek.com>,
Fan Chen <fan.chen@mediatek.com>,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, linux-mediatek@lists.infradead.org,
srv_heupstream@mediatek.com
Subject: Re: [PATCH v11 04/10] soc: mediatek: Add multiple step bus protection control
Date: Wed, 12 Feb 2020 10:23:45 +0100 [thread overview]
Message-ID: <28bcbd36-c2b9-d69e-55b2-508ad7b63887@gmail.com> (raw)
In-Reply-To: <1581476150.22901.35.camel@mtksdaap41>
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.
>
>>> 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.
>
>>> @@ -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 */
>>>
>
next prev parent reply other threads:[~2020-02-12 9:23 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 [this message]
2020-02-13 2:15 ` Weiyi Lu
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=28bcbd36-c2b9-d69e-55b2-508ad7b63887@gmail.com \
--to=matthias.bgg@gmail.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=robh@kernel.org \
--cc=srv_heupstream@mediatek.com \
--cc=weiyi.lu@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