* [PATCH v2 1/2] soc: mediatek: introduce a CAPS flag for scp_domain_data
@ 2018-04-23 8:36 sean.wang
2018-04-23 8:36 ` [PATCH v2 2/2] soc: mediatek: add a fixed wait for SRAM stable sean.wang
2018-04-27 9:43 ` [PATCH v2 1/2] soc: mediatek: introduce a CAPS flag for scp_domain_data Matthias Brugger
0 siblings, 2 replies; 9+ messages in thread
From: sean.wang @ 2018-04-23 8:36 UTC (permalink / raw)
To: matthias.bgg, rjw, khilman
Cc: ulf.hansson, linux-mediatek, linux-pm, linux-arm-kernel,
linux-kernel, Sean Wang
From: Sean Wang <sean.wang@mediatek.com>
Instead of adding more and more fields to scp_domain_data which get
checked in the code flow, add a caps field used for an indication the
characteristics for each SCP domain.
At present, type u8 for the caps field is selected which can satisfy the
current situation and doesn't take up extra space against type bool
previously used.
Suggested-by: Matthias Brugger <matthias.bgg@gmail.com>
Signed-off-by: Sean Wang <sean.wang@mediatek.com>
---
drivers/soc/mediatek/mtk-scpsys.c | 65 ++++++++++++++++++++-------------------
1 file changed, 34 insertions(+), 31 deletions(-)
diff --git a/drivers/soc/mediatek/mtk-scpsys.c b/drivers/soc/mediatek/mtk-scpsys.c
index f140e71..b1b45e4 100644
--- a/drivers/soc/mediatek/mtk-scpsys.c
+++ b/drivers/soc/mediatek/mtk-scpsys.c
@@ -31,6 +31,9 @@
#define MTK_POLL_DELAY_US 10
#define MTK_POLL_TIMEOUT (jiffies_to_usecs(HZ))
+#define MTK_SCPD_ACTIVE_WAKEUP BIT(0)
+#define MTK_SCPD_CAPS(_scpd, _x) ((_scpd)->data->caps & (_x))
+
#define SPM_VDE_PWR_CON 0x0210
#define SPM_MFG_PWR_CON 0x0214
#define SPM_VEN_PWR_CON 0x0230
@@ -120,7 +123,7 @@ struct scp_domain_data {
u32 sram_pdn_ack_bits;
u32 bus_prot_mask;
enum clk_id clk_id[MAX_CLKS];
- bool active_wakeup;
+ u8 caps;
};
struct scp;
@@ -424,7 +427,7 @@ static struct scp *init_scp(struct platform_device *pdev,
genpd->name = data->name;
genpd->power_off = scpsys_power_off;
genpd->power_on = scpsys_power_on;
- if (scpd->data->active_wakeup)
+ if (MTK_SCPD_CAPS(scpd, MTK_SCPD_ACTIVE_WAKEUP))
genpd->flags |= GENPD_FLAG_ACTIVE_WAKEUP;
}
@@ -477,7 +480,7 @@ static const struct scp_domain_data scp_domain_data_mt2701[] = {
.bus_prot_mask = MT2701_TOP_AXI_PROT_EN_CONN_M |
MT2701_TOP_AXI_PROT_EN_CONN_S,
.clk_id = {CLK_NONE},
- .active_wakeup = true,
+ .caps = MTK_SCPD_ACTIVE_WAKEUP,
},
[MT2701_POWER_DOMAIN_DISP] = {
.name = "disp",
@@ -486,7 +489,7 @@ static const struct scp_domain_data scp_domain_data_mt2701[] = {
.sram_pdn_bits = GENMASK(11, 8),
.clk_id = {CLK_MM},
.bus_prot_mask = MT2701_TOP_AXI_PROT_EN_MM_M0,
- .active_wakeup = true,
+ .caps = MTK_SCPD_ACTIVE_WAKEUP,
},
[MT2701_POWER_DOMAIN_MFG] = {
.name = "mfg",
@@ -495,7 +498,7 @@ static const struct scp_domain_data scp_domain_data_mt2701[] = {
.sram_pdn_bits = GENMASK(11, 8),
.sram_pdn_ack_bits = GENMASK(12, 12),
.clk_id = {CLK_MFG},
- .active_wakeup = true,
+ .caps = MTK_SCPD_ACTIVE_WAKEUP,
},
[MT2701_POWER_DOMAIN_VDEC] = {
.name = "vdec",
@@ -504,7 +507,7 @@ static const struct scp_domain_data scp_domain_data_mt2701[] = {
.sram_pdn_bits = GENMASK(11, 8),
.sram_pdn_ack_bits = GENMASK(12, 12),
.clk_id = {CLK_MM},
- .active_wakeup = true,
+ .caps = MTK_SCPD_ACTIVE_WAKEUP,
},
[MT2701_POWER_DOMAIN_ISP] = {
.name = "isp",
@@ -513,7 +516,7 @@ static const struct scp_domain_data scp_domain_data_mt2701[] = {
.sram_pdn_bits = GENMASK(11, 8),
.sram_pdn_ack_bits = GENMASK(13, 12),
.clk_id = {CLK_MM},
- .active_wakeup = true,
+ .caps = MTK_SCPD_ACTIVE_WAKEUP,
},
[MT2701_POWER_DOMAIN_BDP] = {
.name = "bdp",
@@ -521,7 +524,7 @@ static const struct scp_domain_data scp_domain_data_mt2701[] = {
.ctl_offs = SPM_BDP_PWR_CON,
.sram_pdn_bits = GENMASK(11, 8),
.clk_id = {CLK_NONE},
- .active_wakeup = true,
+ .caps = MTK_SCPD_ACTIVE_WAKEUP,
},
[MT2701_POWER_DOMAIN_ETH] = {
.name = "eth",
@@ -530,7 +533,7 @@ static const struct scp_domain_data scp_domain_data_mt2701[] = {
.sram_pdn_bits = GENMASK(11, 8),
.sram_pdn_ack_bits = GENMASK(15, 12),
.clk_id = {CLK_ETHIF},
- .active_wakeup = true,
+ .caps = MTK_SCPD_ACTIVE_WAKEUP,
},
[MT2701_POWER_DOMAIN_HIF] = {
.name = "hif",
@@ -539,14 +542,14 @@ static const struct scp_domain_data scp_domain_data_mt2701[] = {
.sram_pdn_bits = GENMASK(11, 8),
.sram_pdn_ack_bits = GENMASK(15, 12),
.clk_id = {CLK_ETHIF},
- .active_wakeup = true,
+ .caps = MTK_SCPD_ACTIVE_WAKEUP,
},
[MT2701_POWER_DOMAIN_IFR_MSC] = {
.name = "ifr_msc",
.sta_mask = PWR_STATUS_IFR_MSC,
.ctl_offs = SPM_IFR_MSC_PWR_CON,
.clk_id = {CLK_NONE},
- .active_wakeup = true,
+ .caps = MTK_SCPD_ACTIVE_WAKEUP,
},
};
@@ -561,7 +564,7 @@ static const struct scp_domain_data scp_domain_data_mt2712[] = {
.sram_pdn_bits = GENMASK(8, 8),
.sram_pdn_ack_bits = GENMASK(12, 12),
.clk_id = {CLK_MM},
- .active_wakeup = true,
+ .caps = MTK_SCPD_ACTIVE_WAKEUP,
},
[MT2712_POWER_DOMAIN_VDEC] = {
.name = "vdec",
@@ -570,7 +573,7 @@ static const struct scp_domain_data scp_domain_data_mt2712[] = {
.sram_pdn_bits = GENMASK(8, 8),
.sram_pdn_ack_bits = GENMASK(12, 12),
.clk_id = {CLK_MM, CLK_VDEC},
- .active_wakeup = true,
+ .caps = MTK_SCPD_ACTIVE_WAKEUP,
},
[MT2712_POWER_DOMAIN_VENC] = {
.name = "venc",
@@ -579,7 +582,7 @@ static const struct scp_domain_data scp_domain_data_mt2712[] = {
.sram_pdn_bits = GENMASK(11, 8),
.sram_pdn_ack_bits = GENMASK(15, 12),
.clk_id = {CLK_MM, CLK_VENC, CLK_JPGDEC},
- .active_wakeup = true,
+ .caps = MTK_SCPD_ACTIVE_WAKEUP,
},
[MT2712_POWER_DOMAIN_ISP] = {
.name = "isp",
@@ -588,7 +591,7 @@ static const struct scp_domain_data scp_domain_data_mt2712[] = {
.sram_pdn_bits = GENMASK(11, 8),
.sram_pdn_ack_bits = GENMASK(13, 12),
.clk_id = {CLK_MM},
- .active_wakeup = true,
+ .caps = MTK_SCPD_ACTIVE_WAKEUP,
},
[MT2712_POWER_DOMAIN_AUDIO] = {
.name = "audio",
@@ -597,7 +600,7 @@ static const struct scp_domain_data scp_domain_data_mt2712[] = {
.sram_pdn_bits = GENMASK(11, 8),
.sram_pdn_ack_bits = GENMASK(15, 12),
.clk_id = {CLK_AUDIO},
- .active_wakeup = true,
+ .caps = MTK_SCPD_ACTIVE_WAKEUP,
},
[MT2712_POWER_DOMAIN_USB] = {
.name = "usb",
@@ -606,7 +609,7 @@ static const struct scp_domain_data scp_domain_data_mt2712[] = {
.sram_pdn_bits = GENMASK(10, 8),
.sram_pdn_ack_bits = GENMASK(14, 12),
.clk_id = {CLK_NONE},
- .active_wakeup = true,
+ .caps = MTK_SCPD_ACTIVE_WAKEUP,
},
[MT2712_POWER_DOMAIN_USB2] = {
.name = "usb2",
@@ -615,7 +618,7 @@ static const struct scp_domain_data scp_domain_data_mt2712[] = {
.sram_pdn_bits = GENMASK(10, 8),
.sram_pdn_ack_bits = GENMASK(14, 12),
.clk_id = {CLK_NONE},
- .active_wakeup = true,
+ .caps = MTK_SCPD_ACTIVE_WAKEUP,
},
[MT2712_POWER_DOMAIN_MFG] = {
.name = "mfg",
@@ -625,7 +628,7 @@ static const struct scp_domain_data scp_domain_data_mt2712[] = {
.sram_pdn_ack_bits = GENMASK(16, 16),
.clk_id = {CLK_MFG},
.bus_prot_mask = BIT(14) | BIT(21) | BIT(23),
- .active_wakeup = true,
+ .caps = MTK_SCPD_ACTIVE_WAKEUP,
},
[MT2712_POWER_DOMAIN_MFG_SC1] = {
.name = "mfg_sc1",
@@ -634,7 +637,7 @@ static const struct scp_domain_data scp_domain_data_mt2712[] = {
.sram_pdn_bits = GENMASK(8, 8),
.sram_pdn_ack_bits = GENMASK(16, 16),
.clk_id = {CLK_NONE},
- .active_wakeup = true,
+ .caps = MTK_SCPD_ACTIVE_WAKEUP,
},
[MT2712_POWER_DOMAIN_MFG_SC2] = {
.name = "mfg_sc2",
@@ -643,7 +646,7 @@ static const struct scp_domain_data scp_domain_data_mt2712[] = {
.sram_pdn_bits = GENMASK(8, 8),
.sram_pdn_ack_bits = GENMASK(16, 16),
.clk_id = {CLK_NONE},
- .active_wakeup = true,
+ .caps = MTK_SCPD_ACTIVE_WAKEUP,
},
[MT2712_POWER_DOMAIN_MFG_SC3] = {
.name = "mfg_sc3",
@@ -652,7 +655,7 @@ static const struct scp_domain_data scp_domain_data_mt2712[] = {
.sram_pdn_bits = GENMASK(8, 8),
.sram_pdn_ack_bits = GENMASK(16, 16),
.clk_id = {CLK_NONE},
- .active_wakeup = true,
+ .caps = MTK_SCPD_ACTIVE_WAKEUP,
},
};
@@ -752,7 +755,7 @@ static const struct scp_domain_data scp_domain_data_mt7622[] = {
.sram_pdn_ack_bits = GENMASK(15, 12),
.clk_id = {CLK_NONE},
.bus_prot_mask = MT7622_TOP_AXI_PROT_EN_ETHSYS,
- .active_wakeup = true,
+ .caps = MTK_SCPD_ACTIVE_WAKEUP,
},
[MT7622_POWER_DOMAIN_HIF0] = {
.name = "hif0",
@@ -762,7 +765,7 @@ static const struct scp_domain_data scp_domain_data_mt7622[] = {
.sram_pdn_ack_bits = GENMASK(15, 12),
.clk_id = {CLK_HIFSEL},
.bus_prot_mask = MT7622_TOP_AXI_PROT_EN_HIF0,
- .active_wakeup = true,
+ .caps = MTK_SCPD_ACTIVE_WAKEUP,
},
[MT7622_POWER_DOMAIN_HIF1] = {
.name = "hif1",
@@ -772,7 +775,7 @@ static const struct scp_domain_data scp_domain_data_mt7622[] = {
.sram_pdn_ack_bits = GENMASK(15, 12),
.clk_id = {CLK_HIFSEL},
.bus_prot_mask = MT7622_TOP_AXI_PROT_EN_HIF1,
- .active_wakeup = true,
+ .caps = MTK_SCPD_ACTIVE_WAKEUP,
},
[MT7622_POWER_DOMAIN_WB] = {
.name = "wb",
@@ -782,7 +785,7 @@ static const struct scp_domain_data scp_domain_data_mt7622[] = {
.sram_pdn_ack_bits = 0,
.clk_id = {CLK_NONE},
.bus_prot_mask = MT7622_TOP_AXI_PROT_EN_WB,
- .active_wakeup = true,
+ .caps = MTK_SCPD_ACTIVE_WAKEUP,
},
};
@@ -798,7 +801,7 @@ static const struct scp_domain_data scp_domain_data_mt7623a[] = {
.bus_prot_mask = MT2701_TOP_AXI_PROT_EN_CONN_M |
MT2701_TOP_AXI_PROT_EN_CONN_S,
.clk_id = {CLK_NONE},
- .active_wakeup = true,
+ .caps = MTK_SCPD_ACTIVE_WAKEUP,
},
[MT7623A_POWER_DOMAIN_ETH] = {
.name = "eth",
@@ -807,7 +810,7 @@ static const struct scp_domain_data scp_domain_data_mt7623a[] = {
.sram_pdn_bits = GENMASK(11, 8),
.sram_pdn_ack_bits = GENMASK(15, 12),
.clk_id = {CLK_ETHIF},
- .active_wakeup = true,
+ .caps = MTK_SCPD_ACTIVE_WAKEUP,
},
[MT7623A_POWER_DOMAIN_HIF] = {
.name = "hif",
@@ -816,14 +819,14 @@ static const struct scp_domain_data scp_domain_data_mt7623a[] = {
.sram_pdn_bits = GENMASK(11, 8),
.sram_pdn_ack_bits = GENMASK(15, 12),
.clk_id = {CLK_ETHIF},
- .active_wakeup = true,
+ .caps = MTK_SCPD_ACTIVE_WAKEUP,
},
[MT7623A_POWER_DOMAIN_IFR_MSC] = {
.name = "ifr_msc",
.sta_mask = PWR_STATUS_IFR_MSC,
.ctl_offs = SPM_IFR_MSC_PWR_CON,
.clk_id = {CLK_NONE},
- .active_wakeup = true,
+ .caps = MTK_SCPD_ACTIVE_WAKEUP,
},
};
@@ -889,7 +892,7 @@ static const struct scp_domain_data scp_domain_data_mt8173[] = {
.sram_pdn_bits = GENMASK(11, 8),
.sram_pdn_ack_bits = GENMASK(15, 12),
.clk_id = {CLK_NONE},
- .active_wakeup = true,
+ .caps = MTK_SCPD_ACTIVE_WAKEUP,
},
[MT8173_POWER_DOMAIN_MFG_ASYNC] = {
.name = "mfg_async",
--
2.7.4
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH v2 2/2] soc: mediatek: add a fixed wait for SRAM stable
2018-04-23 8:36 [PATCH v2 1/2] soc: mediatek: introduce a CAPS flag for scp_domain_data sean.wang
@ 2018-04-23 8:36 ` sean.wang
2018-04-23 9:31 ` Matthias Brugger
2018-04-27 9:43 ` [PATCH v2 1/2] soc: mediatek: introduce a CAPS flag for scp_domain_data Matthias Brugger
1 sibling, 1 reply; 9+ messages in thread
From: sean.wang @ 2018-04-23 8:36 UTC (permalink / raw)
To: matthias.bgg, rjw, khilman
Cc: ulf.hansson, linux-mediatek, linux-pm, linux-arm-kernel,
linux-kernel, Sean Wang, Weiyi Lu
From: Sean Wang <sean.wang@mediatek.com>
MT7622_POWER_DOMAIN_WB doesn't send an ACK when its managed SRAM becomes
stable, which is not like the behavior the other power domains should
have. Therefore, it's necessary for such a power domain to have a fixed
and well-predefined duration to wait until its managed SRAM can be allowed
to access by all functions running on the top.
v1 -> v2:
- use MTK_SCPD_FWAIT_SRAM flag as an indication requiring force waiting.
Signed-off-by: Sean Wang <sean.wang@mediatek.com>
Cc: Matthias Brugger <matthias.bgg@gmail.com>
Cc: Ulf Hansson <ulf.hansson@linaro.org>
Cc: Weiyi Lu <weiyi.lu@mediatek.com>
---
drivers/soc/mediatek/mtk-scpsys.c | 24 ++++++++++++++++++------
1 file changed, 18 insertions(+), 6 deletions(-)
diff --git a/drivers/soc/mediatek/mtk-scpsys.c b/drivers/soc/mediatek/mtk-scpsys.c
index b1b45e4..d4f1a63 100644
--- a/drivers/soc/mediatek/mtk-scpsys.c
+++ b/drivers/soc/mediatek/mtk-scpsys.c
@@ -32,6 +32,7 @@
#define MTK_POLL_TIMEOUT (jiffies_to_usecs(HZ))
#define MTK_SCPD_ACTIVE_WAKEUP BIT(0)
+#define MTK_SCPD_FWAIT_SRAM BIT(1)
#define MTK_SCPD_CAPS(_scpd, _x) ((_scpd)->data->caps & (_x))
#define SPM_VDE_PWR_CON 0x0210
@@ -237,11 +238,22 @@ static int scpsys_power_on(struct generic_pm_domain *genpd)
val &= ~scpd->data->sram_pdn_bits;
writel(val, ctl_addr);
- /* wait until SRAM_PDN_ACK all 0 */
- ret = readl_poll_timeout(ctl_addr, tmp, (tmp & pdn_ack) == 0,
- MTK_POLL_DELAY_US, MTK_POLL_TIMEOUT);
- if (ret < 0)
- goto err_pwr_ack;
+ /* Either wait until SRAM_PDN_ACK all 0 or have a force wait */
+ if (!MTK_SCPD_CAPS(scpd, MTK_SCPD_FWAIT_SRAM)) {
+ ret = readl_poll_timeout(ctl_addr, tmp, (tmp & pdn_ack) == 0,
+ MTK_POLL_DELAY_US, MTK_POLL_TIMEOUT);
+ if (ret < 0)
+ goto err_pwr_ack;
+ } else {
+ /*
+ * Currently, MTK_SCPD_FWAIT_SRAM is necessary only for
+ * MT7622_POWER_DOMAIN_WB and thus just a trivial setup is
+ * applied here. If there're more domains which need to force
+ * waiting for its own pre-defined value, the duration should
+ * be coded in the caps field.
+ */
+ usleep_range(12000, 12100);
+ };
if (scpd->data->bus_prot_mask) {
ret = mtk_infracfg_clear_bus_protection(scp->infracfg,
@@ -785,7 +797,7 @@ static const struct scp_domain_data scp_domain_data_mt7622[] = {
.sram_pdn_ack_bits = 0,
.clk_id = {CLK_NONE},
.bus_prot_mask = MT7622_TOP_AXI_PROT_EN_WB,
- .caps = MTK_SCPD_ACTIVE_WAKEUP,
+ .caps = MTK_SCPD_ACTIVE_WAKEUP | MTK_SCPD_FWAIT_SRAM,
},
};
--
2.7.4
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH v2 2/2] soc: mediatek: add a fixed wait for SRAM stable
2018-04-23 8:36 ` [PATCH v2 2/2] soc: mediatek: add a fixed wait for SRAM stable sean.wang
@ 2018-04-23 9:31 ` Matthias Brugger
2018-04-23 9:39 ` Sean Wang
0 siblings, 1 reply; 9+ messages in thread
From: Matthias Brugger @ 2018-04-23 9:31 UTC (permalink / raw)
To: sean.wang, rjw, khilman
Cc: ulf.hansson, linux-mediatek, linux-pm, linux-arm-kernel,
linux-kernel, Weiyi Lu
On 04/23/2018 10:36 AM, sean.wang@mediatek.com wrote:
> From: Sean Wang <sean.wang@mediatek.com>
>
> MT7622_POWER_DOMAIN_WB doesn't send an ACK when its managed SRAM becomes
> stable, which is not like the behavior the other power domains should
> have. Therefore, it's necessary for such a power domain to have a fixed
> and well-predefined duration to wait until its managed SRAM can be allowed
> to access by all functions running on the top.
>
> v1 -> v2:
> - use MTK_SCPD_FWAIT_SRAM flag as an indication requiring force waiting.
>
> Signed-off-by: Sean Wang <sean.wang@mediatek.com>
> Cc: Matthias Brugger <matthias.bgg@gmail.com>
> Cc: Ulf Hansson <ulf.hansson@linaro.org>
> Cc: Weiyi Lu <weiyi.lu@mediatek.com>
> ---
> drivers/soc/mediatek/mtk-scpsys.c | 24 ++++++++++++++++++------
> 1 file changed, 18 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/soc/mediatek/mtk-scpsys.c b/drivers/soc/mediatek/mtk-scpsys.c
> index b1b45e4..d4f1a63 100644
> --- a/drivers/soc/mediatek/mtk-scpsys.c
> +++ b/drivers/soc/mediatek/mtk-scpsys.c
> @@ -32,6 +32,7 @@
> #define MTK_POLL_TIMEOUT (jiffies_to_usecs(HZ))
>
> #define MTK_SCPD_ACTIVE_WAKEUP BIT(0)
> +#define MTK_SCPD_FWAIT_SRAM BIT(1)
> #define MTK_SCPD_CAPS(_scpd, _x) ((_scpd)->data->caps & (_x))
>
> #define SPM_VDE_PWR_CON 0x0210
> @@ -237,11 +238,22 @@ static int scpsys_power_on(struct generic_pm_domain *genpd)
> val &= ~scpd->data->sram_pdn_bits;
> writel(val, ctl_addr);
>
> - /* wait until SRAM_PDN_ACK all 0 */
> - ret = readl_poll_timeout(ctl_addr, tmp, (tmp & pdn_ack) == 0,
> - MTK_POLL_DELAY_US, MTK_POLL_TIMEOUT);
> - if (ret < 0)
> - goto err_pwr_ack;
> + /* Either wait until SRAM_PDN_ACK all 0 or have a force wait */
> + if (!MTK_SCPD_CAPS(scpd, MTK_SCPD_FWAIT_SRAM)) {
> + ret = readl_poll_timeout(ctl_addr, tmp, (tmp & pdn_ack) == 0,
> + MTK_POLL_DELAY_US, MTK_POLL_TIMEOUT);
> + if (ret < 0)
> + goto err_pwr_ack;
> + } else {
> + /*
> + * Currently, MTK_SCPD_FWAIT_SRAM is necessary only for
> + * MT7622_POWER_DOMAIN_WB and thus just a trivial setup is
> + * applied here. If there're more domains which need to force
> + * waiting for its own pre-defined value, the duration should
> + * be coded in the caps field.
> + */
I would say, if necessary in the future we can add a switch statement here.
Other then that the patches look good. If you are OK, I'll just delete the last
sentence when applying the patch.
Regards,
Matthias
> + usleep_range(12000, 12100);
> + };
>
> if (scpd->data->bus_prot_mask) {
> ret = mtk_infracfg_clear_bus_protection(scp->infracfg,
> @@ -785,7 +797,7 @@ static const struct scp_domain_data scp_domain_data_mt7622[] = {
> .sram_pdn_ack_bits = 0,
> .clk_id = {CLK_NONE},
> .bus_prot_mask = MT7622_TOP_AXI_PROT_EN_WB,
> - .caps = MTK_SCPD_ACTIVE_WAKEUP,
> + .caps = MTK_SCPD_ACTIVE_WAKEUP | MTK_SCPD_FWAIT_SRAM,
> },
> };
>
>
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH v2 2/2] soc: mediatek: add a fixed wait for SRAM stable
2018-04-23 9:31 ` Matthias Brugger
@ 2018-04-23 9:39 ` Sean Wang
2018-04-27 9:46 ` Matthias Brugger
0 siblings, 1 reply; 9+ messages in thread
From: Sean Wang @ 2018-04-23 9:39 UTC (permalink / raw)
To: Matthias Brugger
Cc: rjw, khilman, ulf.hansson, linux-mediatek, linux-pm,
linux-arm-kernel, linux-kernel, Weiyi Lu
On Mon, 2018-04-23 at 11:31 +0200, Matthias Brugger wrote:
>
> On 04/23/2018 10:36 AM, sean.wang@mediatek.com wrote:
> > From: Sean Wang <sean.wang@mediatek.com>
> >
> > MT7622_POWER_DOMAIN_WB doesn't send an ACK when its managed SRAM becomes
> > stable, which is not like the behavior the other power domains should
> > have. Therefore, it's necessary for such a power domain to have a fixed
> > and well-predefined duration to wait until its managed SRAM can be allowed
> > to access by all functions running on the top.
> >
> > v1 -> v2:
> > - use MTK_SCPD_FWAIT_SRAM flag as an indication requiring force waiting.
> >
> > Signed-off-by: Sean Wang <sean.wang@mediatek.com>
> > Cc: Matthias Brugger <matthias.bgg@gmail.com>
> > Cc: Ulf Hansson <ulf.hansson@linaro.org>
> > Cc: Weiyi Lu <weiyi.lu@mediatek.com>
> > ---
> > drivers/soc/mediatek/mtk-scpsys.c | 24 ++++++++++++++++++------
> > 1 file changed, 18 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/soc/mediatek/mtk-scpsys.c b/drivers/soc/mediatek/mtk-scpsys.c
> > index b1b45e4..d4f1a63 100644
> > --- a/drivers/soc/mediatek/mtk-scpsys.c
> > +++ b/drivers/soc/mediatek/mtk-scpsys.c
> > @@ -32,6 +32,7 @@
> > #define MTK_POLL_TIMEOUT (jiffies_to_usecs(HZ))
> >
> > #define MTK_SCPD_ACTIVE_WAKEUP BIT(0)
> > +#define MTK_SCPD_FWAIT_SRAM BIT(1)
> > #define MTK_SCPD_CAPS(_scpd, _x) ((_scpd)->data->caps & (_x))
> >
> > #define SPM_VDE_PWR_CON 0x0210
> > @@ -237,11 +238,22 @@ static int scpsys_power_on(struct generic_pm_domain *genpd)
> > val &= ~scpd->data->sram_pdn_bits;
> > writel(val, ctl_addr);
> >
> > - /* wait until SRAM_PDN_ACK all 0 */
> > - ret = readl_poll_timeout(ctl_addr, tmp, (tmp & pdn_ack) == 0,
> > - MTK_POLL_DELAY_US, MTK_POLL_TIMEOUT);
> > - if (ret < 0)
> > - goto err_pwr_ack;
> > + /* Either wait until SRAM_PDN_ACK all 0 or have a force wait */
> > + if (!MTK_SCPD_CAPS(scpd, MTK_SCPD_FWAIT_SRAM)) {
> > + ret = readl_poll_timeout(ctl_addr, tmp, (tmp & pdn_ack) == 0,
> > + MTK_POLL_DELAY_US, MTK_POLL_TIMEOUT);
> > + if (ret < 0)
> > + goto err_pwr_ack;
> > + } else {
> > + /*
> > + * Currently, MTK_SCPD_FWAIT_SRAM is necessary only for
> > + * MT7622_POWER_DOMAIN_WB and thus just a trivial setup is
> > + * applied here. If there're more domains which need to force
> > + * waiting for its own pre-defined value, the duration should
> > + * be coded in the caps field.
> > + */
>
> I would say, if necessary in the future we can add a switch statement here.
> Other then that the patches look good. If you are OK, I'll just delete the last
> sentence when applying the patch.
>
yes, it's okay for me.
> Regards,
> Matthias
>
> > + usleep_range(12000, 12100);
> > + };
> >
> > if (scpd->data->bus_prot_mask) {
> > ret = mtk_infracfg_clear_bus_protection(scp->infracfg,
> > @@ -785,7 +797,7 @@ static const struct scp_domain_data scp_domain_data_mt7622[] = {
> > .sram_pdn_ack_bits = 0,
> > .clk_id = {CLK_NONE},
> > .bus_prot_mask = MT7622_TOP_AXI_PROT_EN_WB,
> > - .caps = MTK_SCPD_ACTIVE_WAKEUP,
> > + .caps = MTK_SCPD_ACTIVE_WAKEUP | MTK_SCPD_FWAIT_SRAM,
> > },
> > };
> >
> >
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH v2 2/2] soc: mediatek: add a fixed wait for SRAM stable
2018-04-23 9:39 ` Sean Wang
@ 2018-04-27 9:46 ` Matthias Brugger
2018-04-30 7:08 ` Sean Wang
0 siblings, 1 reply; 9+ messages in thread
From: Matthias Brugger @ 2018-04-27 9:46 UTC (permalink / raw)
To: Sean Wang
Cc: rjw, khilman, ulf.hansson, linux-mediatek, linux-pm,
linux-arm-kernel, linux-kernel, Weiyi Lu
Hi Sean,
On 04/23/2018 11:39 AM, Sean Wang wrote:
> On Mon, 2018-04-23 at 11:31 +0200, Matthias Brugger wrote:
>>
>> On 04/23/2018 10:36 AM, sean.wang@mediatek.com wrote:
>>> From: Sean Wang <sean.wang@mediatek.com>
>>>
>>> MT7622_POWER_DOMAIN_WB doesn't send an ACK when its managed SRAM becomes
>>> stable, which is not like the behavior the other power domains should
>>> have. Therefore, it's necessary for such a power domain to have a fixed
>>> and well-predefined duration to wait until its managed SRAM can be allowed
>>> to access by all functions running on the top.
>>>
>>> v1 -> v2:
>>> - use MTK_SCPD_FWAIT_SRAM flag as an indication requiring force waiting.
>>>
>>> Signed-off-by: Sean Wang <sean.wang@mediatek.com>
>>> Cc: Matthias Brugger <matthias.bgg@gmail.com>
>>> Cc: Ulf Hansson <ulf.hansson@linaro.org>
>>> Cc: Weiyi Lu <weiyi.lu@mediatek.com>
>>> ---
>>> drivers/soc/mediatek/mtk-scpsys.c | 24 ++++++++++++++++++------
>>> 1 file changed, 18 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/drivers/soc/mediatek/mtk-scpsys.c b/drivers/soc/mediatek/mtk-scpsys.c
>>> index b1b45e4..d4f1a63 100644
>>> --- a/drivers/soc/mediatek/mtk-scpsys.c
>>> +++ b/drivers/soc/mediatek/mtk-scpsys.c
>>> @@ -32,6 +32,7 @@
>>> #define MTK_POLL_TIMEOUT (jiffies_to_usecs(HZ))
>>>
>>> #define MTK_SCPD_ACTIVE_WAKEUP BIT(0)
>>> +#define MTK_SCPD_FWAIT_SRAM BIT(1)
>>> #define MTK_SCPD_CAPS(_scpd, _x) ((_scpd)->data->caps & (_x))
>>>
>>> #define SPM_VDE_PWR_CON 0x0210
>>> @@ -237,11 +238,22 @@ static int scpsys_power_on(struct generic_pm_domain *genpd)
>>> val &= ~scpd->data->sram_pdn_bits;
>>> writel(val, ctl_addr);
>>>
>>> - /* wait until SRAM_PDN_ACK all 0 */
>>> - ret = readl_poll_timeout(ctl_addr, tmp, (tmp & pdn_ack) == 0,
>>> - MTK_POLL_DELAY_US, MTK_POLL_TIMEOUT);
>>> - if (ret < 0)
>>> - goto err_pwr_ack;
>>> + /* Either wait until SRAM_PDN_ACK all 0 or have a force wait */
>>> + if (!MTK_SCPD_CAPS(scpd, MTK_SCPD_FWAIT_SRAM)) {
After having another look on the patch, could you change the order of the if:
So that we check for the existence of the MTK_SCPD_FWAIT_SRAM and sleep and in
the else branch we to the readl_poll_timeout.
I think in the future this will make the code easier to understand as you can
easily oversee the '!' negation in the if.
Regards,
Matthias
>>> + ret = readl_poll_timeout(ctl_addr, tmp, (tmp & pdn_ack) == 0,
>>> + MTK_POLL_DELAY_US, MTK_POLL_TIMEOUT);
>>> + if (ret < 0)
>>> + goto err_pwr_ack;
>>> + } else {
>>> + /*
>>> + * Currently, MTK_SCPD_FWAIT_SRAM is necessary only for
>>> + * MT7622_POWER_DOMAIN_WB and thus just a trivial setup is
>>> + * applied here. If there're more domains which need to force
>>> + * waiting for its own pre-defined value, the duration should
>>> + * be coded in the caps field.
>>> + */
>>
>> I would say, if necessary in the future we can add a switch statement here.
>> Other then that the patches look good. If you are OK, I'll just delete the last
>> sentence when applying the patch.
>>
>
> yes, it's okay for me.
>
>> Regards,
>> Matthias
>>
>>> + usleep_range(12000, 12100);
>>> + };
>>>
>>> if (scpd->data->bus_prot_mask) {
>>> ret = mtk_infracfg_clear_bus_protection(scp->infracfg,
>>> @@ -785,7 +797,7 @@ static const struct scp_domain_data scp_domain_data_mt7622[] = {
>>> .sram_pdn_ack_bits = 0,
>>> .clk_id = {CLK_NONE},
>>> .bus_prot_mask = MT7622_TOP_AXI_PROT_EN_WB,
>>> - .caps = MTK_SCPD_ACTIVE_WAKEUP,
>>> + .caps = MTK_SCPD_ACTIVE_WAKEUP | MTK_SCPD_FWAIT_SRAM,
>>> },
>>> };
>>>
>>>
>
>
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH v2 2/2] soc: mediatek: add a fixed wait for SRAM stable
2018-04-27 9:46 ` Matthias Brugger
@ 2018-04-30 7:08 ` Sean Wang
2018-04-30 9:10 ` Matthias Brugger
0 siblings, 1 reply; 9+ messages in thread
From: Sean Wang @ 2018-04-30 7:08 UTC (permalink / raw)
To: Matthias Brugger
Cc: rjw, khilman, ulf.hansson, linux-mediatek, linux-pm,
linux-arm-kernel, linux-kernel, Weiyi Lu
On Fri, 2018-04-27 at 11:46 +0200, Matthias Brugger wrote:
> Hi Sean,
>
> On 04/23/2018 11:39 AM, Sean Wang wrote:
> > On Mon, 2018-04-23 at 11:31 +0200, Matthias Brugger wrote:
> >>
> >> On 04/23/2018 10:36 AM, sean.wang@mediatek.com wrote:
> >>> From: Sean Wang <sean.wang@mediatek.com>
> >>>
> >>> MT7622_POWER_DOMAIN_WB doesn't send an ACK when its managed SRAM becomes
> >>> stable, which is not like the behavior the other power domains should
> >>> have. Therefore, it's necessary for such a power domain to have a fixed
> >>> and well-predefined duration to wait until its managed SRAM can be allowed
> >>> to access by all functions running on the top.
> >>>
> >>> v1 -> v2:
> >>> - use MTK_SCPD_FWAIT_SRAM flag as an indication requiring force waiting.
> >>>
> >>> Signed-off-by: Sean Wang <sean.wang@mediatek.com>
> >>> Cc: Matthias Brugger <matthias.bgg@gmail.com>
> >>> Cc: Ulf Hansson <ulf.hansson@linaro.org>
> >>> Cc: Weiyi Lu <weiyi.lu@mediatek.com>
> >>> ---
> >>> drivers/soc/mediatek/mtk-scpsys.c | 24 ++++++++++++++++++------
> >>> 1 file changed, 18 insertions(+), 6 deletions(-)
> >>>
> >>> diff --git a/drivers/soc/mediatek/mtk-scpsys.c b/drivers/soc/mediatek/mtk-scpsys.c
> >>> index b1b45e4..d4f1a63 100644
> >>> --- a/drivers/soc/mediatek/mtk-scpsys.c
> >>> +++ b/drivers/soc/mediatek/mtk-scpsys.c
> >>> @@ -32,6 +32,7 @@
> >>> #define MTK_POLL_TIMEOUT (jiffies_to_usecs(HZ))
> >>>
> >>> #define MTK_SCPD_ACTIVE_WAKEUP BIT(0)
> >>> +#define MTK_SCPD_FWAIT_SRAM BIT(1)
> >>> #define MTK_SCPD_CAPS(_scpd, _x) ((_scpd)->data->caps & (_x))
> >>>
> >>> #define SPM_VDE_PWR_CON 0x0210
> >>> @@ -237,11 +238,22 @@ static int scpsys_power_on(struct generic_pm_domain *genpd)
> >>> val &= ~scpd->data->sram_pdn_bits;
> >>> writel(val, ctl_addr);
> >>>
> >>> - /* wait until SRAM_PDN_ACK all 0 */
> >>> - ret = readl_poll_timeout(ctl_addr, tmp, (tmp & pdn_ack) == 0,
> >>> - MTK_POLL_DELAY_US, MTK_POLL_TIMEOUT);
> >>> - if (ret < 0)
> >>> - goto err_pwr_ack;
> >>> + /* Either wait until SRAM_PDN_ACK all 0 or have a force wait */
> >>> + if (!MTK_SCPD_CAPS(scpd, MTK_SCPD_FWAIT_SRAM)) {
>
> After having another look on the patch, could you change the order of the if:
> So that we check for the existence of the MTK_SCPD_FWAIT_SRAM and sleep and in
> the else branch we to the readl_poll_timeout.
>
> I think in the future this will make the code easier to understand as you can
> easily oversee the '!' negation in the if.
>
> Regards,
> Matthias
>
Initial thought on the patch is that I would like to save a branch
instruction for a most possibly executed block. Or would it be better to
add a compiler to branch prediction information? something like that
/* Either wait until SRAM_PDN_ACK all 0 or have a force wait */
if (unlikely(MTK_SCPD_CAPS(scpd, MTK_SCPD_FWAIT_SRAM))) {
/*
* Currently, MTK_SCPD_FWAIT_SRAM is necessary only for
* MT7622_POWER_DOMAIN_WB and thus just a trivial setup
is
* applied here.
*/
usleep_range(12000, 12100);
...
>
> >>> + ret = readl_poll_timeout(ctl_addr, tmp, (tmp & pdn_ack) == 0,
> >>> + MTK_POLL_DELAY_US, MTK_POLL_TIMEOUT);
> >>> + if (ret < 0)
> >>> + goto err_pwr_ack;
> >>> + } else {
> >>> + /*
> >>> + * Currently, MTK_SCPD_FWAIT_SRAM is necessary only for
> >>> + * MT7622_POWER_DOMAIN_WB and thus just a trivial setup is
> >>> + * applied here. If there're more domains which need to force
> >>> + * waiting for its own pre-defined value, the duration should
> >>> + * be coded in the caps field.
> >>> + */
> >>
> >> I would say, if necessary in the future we can add a switch statement here.
> >> Other then that the patches look good. If you are OK, I'll just delete the last
> >> sentence when applying the patch.
> >>
> >
> > yes, it's okay for me.
> >
> >> Regards,
> >> Matthias
> >>
> >>> + usleep_range(12000, 12100);
> >>> + };
> >>>
> >>> if (scpd->data->bus_prot_mask) {
> >>> ret = mtk_infracfg_clear_bus_protection(scp->infracfg,
> >>> @@ -785,7 +797,7 @@ static const struct scp_domain_data scp_domain_data_mt7622[] = {
> >>> .sram_pdn_ack_bits = 0,
> >>> .clk_id = {CLK_NONE},
> >>> .bus_prot_mask = MT7622_TOP_AXI_PROT_EN_WB,
> >>> - .caps = MTK_SCPD_ACTIVE_WAKEUP,
> >>> + .caps = MTK_SCPD_ACTIVE_WAKEUP | MTK_SCPD_FWAIT_SRAM,
> >>> },
> >>> };
> >>>
> >>>
> >
> >
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH v2 2/2] soc: mediatek: add a fixed wait for SRAM stable
2018-04-30 7:08 ` Sean Wang
@ 2018-04-30 9:10 ` Matthias Brugger
2018-04-30 9:59 ` Sean Wang
0 siblings, 1 reply; 9+ messages in thread
From: Matthias Brugger @ 2018-04-30 9:10 UTC (permalink / raw)
To: Sean Wang
Cc: rjw, khilman, ulf.hansson, linux-mediatek, linux-pm,
linux-arm-kernel, linux-kernel, Weiyi Lu
On 04/30/2018 09:08 AM, Sean Wang wrote:
> On Fri, 2018-04-27 at 11:46 +0200, Matthias Brugger wrote:
>> Hi Sean,
>>
>> On 04/23/2018 11:39 AM, Sean Wang wrote:
>>> On Mon, 2018-04-23 at 11:31 +0200, Matthias Brugger wrote:
>>>>
>>>> On 04/23/2018 10:36 AM, sean.wang@mediatek.com wrote:
>>>>> From: Sean Wang <sean.wang@mediatek.com>
>>>>>
>>>>> MT7622_POWER_DOMAIN_WB doesn't send an ACK when its managed SRAM becomes
>>>>> stable, which is not like the behavior the other power domains should
>>>>> have. Therefore, it's necessary for such a power domain to have a fixed
>>>>> and well-predefined duration to wait until its managed SRAM can be allowed
>>>>> to access by all functions running on the top.
>>>>>
>>>>> v1 -> v2:
>>>>> - use MTK_SCPD_FWAIT_SRAM flag as an indication requiring force waiting.
>>>>>
>>>>> Signed-off-by: Sean Wang <sean.wang@mediatek.com>
>>>>> Cc: Matthias Brugger <matthias.bgg@gmail.com>
>>>>> Cc: Ulf Hansson <ulf.hansson@linaro.org>
>>>>> Cc: Weiyi Lu <weiyi.lu@mediatek.com>
>>>>> ---
>>>>> drivers/soc/mediatek/mtk-scpsys.c | 24 ++++++++++++++++++------
>>>>> 1 file changed, 18 insertions(+), 6 deletions(-)
>>>>>
>>>>> diff --git a/drivers/soc/mediatek/mtk-scpsys.c b/drivers/soc/mediatek/mtk-scpsys.c
>>>>> index b1b45e4..d4f1a63 100644
>>>>> --- a/drivers/soc/mediatek/mtk-scpsys.c
>>>>> +++ b/drivers/soc/mediatek/mtk-scpsys.c
>>>>> @@ -32,6 +32,7 @@
>>>>> #define MTK_POLL_TIMEOUT (jiffies_to_usecs(HZ))
>>>>>
>>>>> #define MTK_SCPD_ACTIVE_WAKEUP BIT(0)
>>>>> +#define MTK_SCPD_FWAIT_SRAM BIT(1)
>>>>> #define MTK_SCPD_CAPS(_scpd, _x) ((_scpd)->data->caps & (_x))
>>>>>
>>>>> #define SPM_VDE_PWR_CON 0x0210
>>>>> @@ -237,11 +238,22 @@ static int scpsys_power_on(struct generic_pm_domain *genpd)
>>>>> val &= ~scpd->data->sram_pdn_bits;
>>>>> writel(val, ctl_addr);
>>>>>
>>>>> - /* wait until SRAM_PDN_ACK all 0 */
>>>>> - ret = readl_poll_timeout(ctl_addr, tmp, (tmp & pdn_ack) == 0,
>>>>> - MTK_POLL_DELAY_US, MTK_POLL_TIMEOUT);
>>>>> - if (ret < 0)
>>>>> - goto err_pwr_ack;
>>>>> + /* Either wait until SRAM_PDN_ACK all 0 or have a force wait */
>>>>> + if (!MTK_SCPD_CAPS(scpd, MTK_SCPD_FWAIT_SRAM)) {
>>
>> After having another look on the patch, could you change the order of the if:
>> So that we check for the existence of the MTK_SCPD_FWAIT_SRAM and sleep and in
>> the else branch we to the readl_poll_timeout.
>>
>> I think in the future this will make the code easier to understand as you can
>> easily oversee the '!' negation in the if.
>>
>> Regards,
>> Matthias
>>
>
> Initial thought on the patch is that I would like to save a branch
> instruction for a most possibly executed block. Or would it be better to
> add a compiler to branch prediction information? something like that
>
> /* Either wait until SRAM_PDN_ACK all 0 or have a force wait */
> if (unlikely(MTK_SCPD_CAPS(scpd, MTK_SCPD_FWAIT_SRAM))) {
> /*
> * Currently, MTK_SCPD_FWAIT_SRAM is necessary only for
> * MT7622_POWER_DOMAIN_WB and thus just a trivial setup
> is
> * applied here.
> */
> usleep_range(12000, 12100);
> ...
>
Is this a performance critical path? I thought if you turn on the power domain
for some peripherals, it does not matter if you need a few CPU cycles more or less.
Regards,
Matthias
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH v2 2/2] soc: mediatek: add a fixed wait for SRAM stable
2018-04-30 9:10 ` Matthias Brugger
@ 2018-04-30 9:59 ` Sean Wang
0 siblings, 0 replies; 9+ messages in thread
From: Sean Wang @ 2018-04-30 9:59 UTC (permalink / raw)
To: Matthias Brugger
Cc: rjw, khilman, ulf.hansson, linux-mediatek, linux-pm,
linux-arm-kernel, linux-kernel, Weiyi Lu
On Mon, 2018-04-30 at 11:10 +0200, Matthias Brugger wrote:
>
> On 04/30/2018 09:08 AM, Sean Wang wrote:
> > On Fri, 2018-04-27 at 11:46 +0200, Matthias Brugger wrote:
> >> Hi Sean,
> >>
> >> On 04/23/2018 11:39 AM, Sean Wang wrote:
> >>> On Mon, 2018-04-23 at 11:31 +0200, Matthias Brugger wrote:
> >>>>
> >>>> On 04/23/2018 10:36 AM, sean.wang@mediatek.com wrote:
> >>>>> From: Sean Wang <sean.wang@mediatek.com>
> >>>>>
> >>>>> MT7622_POWER_DOMAIN_WB doesn't send an ACK when its managed SRAM becomes
> >>>>> stable, which is not like the behavior the other power domains should
> >>>>> have. Therefore, it's necessary for such a power domain to have a fixed
> >>>>> and well-predefined duration to wait until its managed SRAM can be allowed
> >>>>> to access by all functions running on the top.
> >>>>>
> >>>>> v1 -> v2:
> >>>>> - use MTK_SCPD_FWAIT_SRAM flag as an indication requiring force waiting.
> >>>>>
> >>>>> Signed-off-by: Sean Wang <sean.wang@mediatek.com>
> >>>>> Cc: Matthias Brugger <matthias.bgg@gmail.com>
> >>>>> Cc: Ulf Hansson <ulf.hansson@linaro.org>
> >>>>> Cc: Weiyi Lu <weiyi.lu@mediatek.com>
> >>>>> ---
> >>>>> drivers/soc/mediatek/mtk-scpsys.c | 24 ++++++++++++++++++------
> >>>>> 1 file changed, 18 insertions(+), 6 deletions(-)
> >>>>>
> >>>>> diff --git a/drivers/soc/mediatek/mtk-scpsys.c b/drivers/soc/mediatek/mtk-scpsys.c
> >>>>> index b1b45e4..d4f1a63 100644
> >>>>> --- a/drivers/soc/mediatek/mtk-scpsys.c
> >>>>> +++ b/drivers/soc/mediatek/mtk-scpsys.c
> >>>>> @@ -32,6 +32,7 @@
> >>>>> #define MTK_POLL_TIMEOUT (jiffies_to_usecs(HZ))
> >>>>>
> >>>>> #define MTK_SCPD_ACTIVE_WAKEUP BIT(0)
> >>>>> +#define MTK_SCPD_FWAIT_SRAM BIT(1)
> >>>>> #define MTK_SCPD_CAPS(_scpd, _x) ((_scpd)->data->caps & (_x))
> >>>>>
> >>>>> #define SPM_VDE_PWR_CON 0x0210
> >>>>> @@ -237,11 +238,22 @@ static int scpsys_power_on(struct generic_pm_domain *genpd)
> >>>>> val &= ~scpd->data->sram_pdn_bits;
> >>>>> writel(val, ctl_addr);
> >>>>>
> >>>>> - /* wait until SRAM_PDN_ACK all 0 */
> >>>>> - ret = readl_poll_timeout(ctl_addr, tmp, (tmp & pdn_ack) == 0,
> >>>>> - MTK_POLL_DELAY_US, MTK_POLL_TIMEOUT);
> >>>>> - if (ret < 0)
> >>>>> - goto err_pwr_ack;
> >>>>> + /* Either wait until SRAM_PDN_ACK all 0 or have a force wait */
> >>>>> + if (!MTK_SCPD_CAPS(scpd, MTK_SCPD_FWAIT_SRAM)) {
> >>
> >> After having another look on the patch, could you change the order of the if:
> >> So that we check for the existence of the MTK_SCPD_FWAIT_SRAM and sleep and in
> >> the else branch we to the readl_poll_timeout.
> >>
> >> I think in the future this will make the code easier to understand as you can
> >> easily oversee the '!' negation in the if.
> >>
> >> Regards,
> >> Matthias
> >>
> >
> > Initial thought on the patch is that I would like to save a branch
> > instruction for a most possibly executed block. Or would it be better to
> > add a compiler to branch prediction information? something like that
> >
> > /* Either wait until SRAM_PDN_ACK all 0 or have a force wait */
> > if (unlikely(MTK_SCPD_CAPS(scpd, MTK_SCPD_FWAIT_SRAM))) {
> > /*
> > * Currently, MTK_SCPD_FWAIT_SRAM is necessary only for
> > * MT7622_POWER_DOMAIN_WB and thus just a trivial setup
> > is
> > * applied here.
> > */
> > usleep_range(12000, 12100);
> > ...
> >
>
> Is this a performance critical path? I thought if you turn on the power domain
> for some peripherals, it does not matter if you need a few CPU cycles more or less.
thanks for your advice
it's not a critical path.
i'll send a new patch according to the result.
> Regards,
> Matthias
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 1/2] soc: mediatek: introduce a CAPS flag for scp_domain_data
2018-04-23 8:36 [PATCH v2 1/2] soc: mediatek: introduce a CAPS flag for scp_domain_data sean.wang
2018-04-23 8:36 ` [PATCH v2 2/2] soc: mediatek: add a fixed wait for SRAM stable sean.wang
@ 2018-04-27 9:43 ` Matthias Brugger
1 sibling, 0 replies; 9+ messages in thread
From: Matthias Brugger @ 2018-04-27 9:43 UTC (permalink / raw)
To: sean.wang, rjw, khilman
Cc: ulf.hansson, linux-mediatek, linux-pm, linux-arm-kernel, linux-kernel
On 04/23/2018 10:36 AM, sean.wang@mediatek.com wrote:
> From: Sean Wang <sean.wang@mediatek.com>
>
> Instead of adding more and more fields to scp_domain_data which get
> checked in the code flow, add a caps field used for an indication the
> characteristics for each SCP domain.
>
> At present, type u8 for the caps field is selected which can satisfy the
> current situation and doesn't take up extra space against type bool
> previously used.
>
> Suggested-by: Matthias Brugger <matthias.bgg@gmail.com>
> Signed-off-by: Sean Wang <sean.wang@mediatek.com>
> ---
> drivers/soc/mediatek/mtk-scpsys.c | 65 ++++++++++++++++++++-------------------
> 1 file changed, 34 insertions(+), 31 deletions(-)
pushed to v4.17-next/soc
Thanks.
>
> diff --git a/drivers/soc/mediatek/mtk-scpsys.c b/drivers/soc/mediatek/mtk-scpsys.c
> index f140e71..b1b45e4 100644
> --- a/drivers/soc/mediatek/mtk-scpsys.c
> +++ b/drivers/soc/mediatek/mtk-scpsys.c
> @@ -31,6 +31,9 @@
> #define MTK_POLL_DELAY_US 10
> #define MTK_POLL_TIMEOUT (jiffies_to_usecs(HZ))
>
> +#define MTK_SCPD_ACTIVE_WAKEUP BIT(0)
> +#define MTK_SCPD_CAPS(_scpd, _x) ((_scpd)->data->caps & (_x))
> +
> #define SPM_VDE_PWR_CON 0x0210
> #define SPM_MFG_PWR_CON 0x0214
> #define SPM_VEN_PWR_CON 0x0230
> @@ -120,7 +123,7 @@ struct scp_domain_data {
> u32 sram_pdn_ack_bits;
> u32 bus_prot_mask;
> enum clk_id clk_id[MAX_CLKS];
> - bool active_wakeup;
> + u8 caps;
> };
>
> struct scp;
> @@ -424,7 +427,7 @@ static struct scp *init_scp(struct platform_device *pdev,
> genpd->name = data->name;
> genpd->power_off = scpsys_power_off;
> genpd->power_on = scpsys_power_on;
> - if (scpd->data->active_wakeup)
> + if (MTK_SCPD_CAPS(scpd, MTK_SCPD_ACTIVE_WAKEUP))
> genpd->flags |= GENPD_FLAG_ACTIVE_WAKEUP;
> }
>
> @@ -477,7 +480,7 @@ static const struct scp_domain_data scp_domain_data_mt2701[] = {
> .bus_prot_mask = MT2701_TOP_AXI_PROT_EN_CONN_M |
> MT2701_TOP_AXI_PROT_EN_CONN_S,
> .clk_id = {CLK_NONE},
> - .active_wakeup = true,
> + .caps = MTK_SCPD_ACTIVE_WAKEUP,
> },
> [MT2701_POWER_DOMAIN_DISP] = {
> .name = "disp",
> @@ -486,7 +489,7 @@ static const struct scp_domain_data scp_domain_data_mt2701[] = {
> .sram_pdn_bits = GENMASK(11, 8),
> .clk_id = {CLK_MM},
> .bus_prot_mask = MT2701_TOP_AXI_PROT_EN_MM_M0,
> - .active_wakeup = true,
> + .caps = MTK_SCPD_ACTIVE_WAKEUP,
> },
> [MT2701_POWER_DOMAIN_MFG] = {
> .name = "mfg",
> @@ -495,7 +498,7 @@ static const struct scp_domain_data scp_domain_data_mt2701[] = {
> .sram_pdn_bits = GENMASK(11, 8),
> .sram_pdn_ack_bits = GENMASK(12, 12),
> .clk_id = {CLK_MFG},
> - .active_wakeup = true,
> + .caps = MTK_SCPD_ACTIVE_WAKEUP,
> },
> [MT2701_POWER_DOMAIN_VDEC] = {
> .name = "vdec",
> @@ -504,7 +507,7 @@ static const struct scp_domain_data scp_domain_data_mt2701[] = {
> .sram_pdn_bits = GENMASK(11, 8),
> .sram_pdn_ack_bits = GENMASK(12, 12),
> .clk_id = {CLK_MM},
> - .active_wakeup = true,
> + .caps = MTK_SCPD_ACTIVE_WAKEUP,
> },
> [MT2701_POWER_DOMAIN_ISP] = {
> .name = "isp",
> @@ -513,7 +516,7 @@ static const struct scp_domain_data scp_domain_data_mt2701[] = {
> .sram_pdn_bits = GENMASK(11, 8),
> .sram_pdn_ack_bits = GENMASK(13, 12),
> .clk_id = {CLK_MM},
> - .active_wakeup = true,
> + .caps = MTK_SCPD_ACTIVE_WAKEUP,
> },
> [MT2701_POWER_DOMAIN_BDP] = {
> .name = "bdp",
> @@ -521,7 +524,7 @@ static const struct scp_domain_data scp_domain_data_mt2701[] = {
> .ctl_offs = SPM_BDP_PWR_CON,
> .sram_pdn_bits = GENMASK(11, 8),
> .clk_id = {CLK_NONE},
> - .active_wakeup = true,
> + .caps = MTK_SCPD_ACTIVE_WAKEUP,
> },
> [MT2701_POWER_DOMAIN_ETH] = {
> .name = "eth",
> @@ -530,7 +533,7 @@ static const struct scp_domain_data scp_domain_data_mt2701[] = {
> .sram_pdn_bits = GENMASK(11, 8),
> .sram_pdn_ack_bits = GENMASK(15, 12),
> .clk_id = {CLK_ETHIF},
> - .active_wakeup = true,
> + .caps = MTK_SCPD_ACTIVE_WAKEUP,
> },
> [MT2701_POWER_DOMAIN_HIF] = {
> .name = "hif",
> @@ -539,14 +542,14 @@ static const struct scp_domain_data scp_domain_data_mt2701[] = {
> .sram_pdn_bits = GENMASK(11, 8),
> .sram_pdn_ack_bits = GENMASK(15, 12),
> .clk_id = {CLK_ETHIF},
> - .active_wakeup = true,
> + .caps = MTK_SCPD_ACTIVE_WAKEUP,
> },
> [MT2701_POWER_DOMAIN_IFR_MSC] = {
> .name = "ifr_msc",
> .sta_mask = PWR_STATUS_IFR_MSC,
> .ctl_offs = SPM_IFR_MSC_PWR_CON,
> .clk_id = {CLK_NONE},
> - .active_wakeup = true,
> + .caps = MTK_SCPD_ACTIVE_WAKEUP,
> },
> };
>
> @@ -561,7 +564,7 @@ static const struct scp_domain_data scp_domain_data_mt2712[] = {
> .sram_pdn_bits = GENMASK(8, 8),
> .sram_pdn_ack_bits = GENMASK(12, 12),
> .clk_id = {CLK_MM},
> - .active_wakeup = true,
> + .caps = MTK_SCPD_ACTIVE_WAKEUP,
> },
> [MT2712_POWER_DOMAIN_VDEC] = {
> .name = "vdec",
> @@ -570,7 +573,7 @@ static const struct scp_domain_data scp_domain_data_mt2712[] = {
> .sram_pdn_bits = GENMASK(8, 8),
> .sram_pdn_ack_bits = GENMASK(12, 12),
> .clk_id = {CLK_MM, CLK_VDEC},
> - .active_wakeup = true,
> + .caps = MTK_SCPD_ACTIVE_WAKEUP,
> },
> [MT2712_POWER_DOMAIN_VENC] = {
> .name = "venc",
> @@ -579,7 +582,7 @@ static const struct scp_domain_data scp_domain_data_mt2712[] = {
> .sram_pdn_bits = GENMASK(11, 8),
> .sram_pdn_ack_bits = GENMASK(15, 12),
> .clk_id = {CLK_MM, CLK_VENC, CLK_JPGDEC},
> - .active_wakeup = true,
> + .caps = MTK_SCPD_ACTIVE_WAKEUP,
> },
> [MT2712_POWER_DOMAIN_ISP] = {
> .name = "isp",
> @@ -588,7 +591,7 @@ static const struct scp_domain_data scp_domain_data_mt2712[] = {
> .sram_pdn_bits = GENMASK(11, 8),
> .sram_pdn_ack_bits = GENMASK(13, 12),
> .clk_id = {CLK_MM},
> - .active_wakeup = true,
> + .caps = MTK_SCPD_ACTIVE_WAKEUP,
> },
> [MT2712_POWER_DOMAIN_AUDIO] = {
> .name = "audio",
> @@ -597,7 +600,7 @@ static const struct scp_domain_data scp_domain_data_mt2712[] = {
> .sram_pdn_bits = GENMASK(11, 8),
> .sram_pdn_ack_bits = GENMASK(15, 12),
> .clk_id = {CLK_AUDIO},
> - .active_wakeup = true,
> + .caps = MTK_SCPD_ACTIVE_WAKEUP,
> },
> [MT2712_POWER_DOMAIN_USB] = {
> .name = "usb",
> @@ -606,7 +609,7 @@ static const struct scp_domain_data scp_domain_data_mt2712[] = {
> .sram_pdn_bits = GENMASK(10, 8),
> .sram_pdn_ack_bits = GENMASK(14, 12),
> .clk_id = {CLK_NONE},
> - .active_wakeup = true,
> + .caps = MTK_SCPD_ACTIVE_WAKEUP,
> },
> [MT2712_POWER_DOMAIN_USB2] = {
> .name = "usb2",
> @@ -615,7 +618,7 @@ static const struct scp_domain_data scp_domain_data_mt2712[] = {
> .sram_pdn_bits = GENMASK(10, 8),
> .sram_pdn_ack_bits = GENMASK(14, 12),
> .clk_id = {CLK_NONE},
> - .active_wakeup = true,
> + .caps = MTK_SCPD_ACTIVE_WAKEUP,
> },
> [MT2712_POWER_DOMAIN_MFG] = {
> .name = "mfg",
> @@ -625,7 +628,7 @@ static const struct scp_domain_data scp_domain_data_mt2712[] = {
> .sram_pdn_ack_bits = GENMASK(16, 16),
> .clk_id = {CLK_MFG},
> .bus_prot_mask = BIT(14) | BIT(21) | BIT(23),
> - .active_wakeup = true,
> + .caps = MTK_SCPD_ACTIVE_WAKEUP,
> },
> [MT2712_POWER_DOMAIN_MFG_SC1] = {
> .name = "mfg_sc1",
> @@ -634,7 +637,7 @@ static const struct scp_domain_data scp_domain_data_mt2712[] = {
> .sram_pdn_bits = GENMASK(8, 8),
> .sram_pdn_ack_bits = GENMASK(16, 16),
> .clk_id = {CLK_NONE},
> - .active_wakeup = true,
> + .caps = MTK_SCPD_ACTIVE_WAKEUP,
> },
> [MT2712_POWER_DOMAIN_MFG_SC2] = {
> .name = "mfg_sc2",
> @@ -643,7 +646,7 @@ static const struct scp_domain_data scp_domain_data_mt2712[] = {
> .sram_pdn_bits = GENMASK(8, 8),
> .sram_pdn_ack_bits = GENMASK(16, 16),
> .clk_id = {CLK_NONE},
> - .active_wakeup = true,
> + .caps = MTK_SCPD_ACTIVE_WAKEUP,
> },
> [MT2712_POWER_DOMAIN_MFG_SC3] = {
> .name = "mfg_sc3",
> @@ -652,7 +655,7 @@ static const struct scp_domain_data scp_domain_data_mt2712[] = {
> .sram_pdn_bits = GENMASK(8, 8),
> .sram_pdn_ack_bits = GENMASK(16, 16),
> .clk_id = {CLK_NONE},
> - .active_wakeup = true,
> + .caps = MTK_SCPD_ACTIVE_WAKEUP,
> },
> };
>
> @@ -752,7 +755,7 @@ static const struct scp_domain_data scp_domain_data_mt7622[] = {
> .sram_pdn_ack_bits = GENMASK(15, 12),
> .clk_id = {CLK_NONE},
> .bus_prot_mask = MT7622_TOP_AXI_PROT_EN_ETHSYS,
> - .active_wakeup = true,
> + .caps = MTK_SCPD_ACTIVE_WAKEUP,
> },
> [MT7622_POWER_DOMAIN_HIF0] = {
> .name = "hif0",
> @@ -762,7 +765,7 @@ static const struct scp_domain_data scp_domain_data_mt7622[] = {
> .sram_pdn_ack_bits = GENMASK(15, 12),
> .clk_id = {CLK_HIFSEL},
> .bus_prot_mask = MT7622_TOP_AXI_PROT_EN_HIF0,
> - .active_wakeup = true,
> + .caps = MTK_SCPD_ACTIVE_WAKEUP,
> },
> [MT7622_POWER_DOMAIN_HIF1] = {
> .name = "hif1",
> @@ -772,7 +775,7 @@ static const struct scp_domain_data scp_domain_data_mt7622[] = {
> .sram_pdn_ack_bits = GENMASK(15, 12),
> .clk_id = {CLK_HIFSEL},
> .bus_prot_mask = MT7622_TOP_AXI_PROT_EN_HIF1,
> - .active_wakeup = true,
> + .caps = MTK_SCPD_ACTIVE_WAKEUP,
> },
> [MT7622_POWER_DOMAIN_WB] = {
> .name = "wb",
> @@ -782,7 +785,7 @@ static const struct scp_domain_data scp_domain_data_mt7622[] = {
> .sram_pdn_ack_bits = 0,
> .clk_id = {CLK_NONE},
> .bus_prot_mask = MT7622_TOP_AXI_PROT_EN_WB,
> - .active_wakeup = true,
> + .caps = MTK_SCPD_ACTIVE_WAKEUP,
> },
> };
>
> @@ -798,7 +801,7 @@ static const struct scp_domain_data scp_domain_data_mt7623a[] = {
> .bus_prot_mask = MT2701_TOP_AXI_PROT_EN_CONN_M |
> MT2701_TOP_AXI_PROT_EN_CONN_S,
> .clk_id = {CLK_NONE},
> - .active_wakeup = true,
> + .caps = MTK_SCPD_ACTIVE_WAKEUP,
> },
> [MT7623A_POWER_DOMAIN_ETH] = {
> .name = "eth",
> @@ -807,7 +810,7 @@ static const struct scp_domain_data scp_domain_data_mt7623a[] = {
> .sram_pdn_bits = GENMASK(11, 8),
> .sram_pdn_ack_bits = GENMASK(15, 12),
> .clk_id = {CLK_ETHIF},
> - .active_wakeup = true,
> + .caps = MTK_SCPD_ACTIVE_WAKEUP,
> },
> [MT7623A_POWER_DOMAIN_HIF] = {
> .name = "hif",
> @@ -816,14 +819,14 @@ static const struct scp_domain_data scp_domain_data_mt7623a[] = {
> .sram_pdn_bits = GENMASK(11, 8),
> .sram_pdn_ack_bits = GENMASK(15, 12),
> .clk_id = {CLK_ETHIF},
> - .active_wakeup = true,
> + .caps = MTK_SCPD_ACTIVE_WAKEUP,
> },
> [MT7623A_POWER_DOMAIN_IFR_MSC] = {
> .name = "ifr_msc",
> .sta_mask = PWR_STATUS_IFR_MSC,
> .ctl_offs = SPM_IFR_MSC_PWR_CON,
> .clk_id = {CLK_NONE},
> - .active_wakeup = true,
> + .caps = MTK_SCPD_ACTIVE_WAKEUP,
> },
> };
>
> @@ -889,7 +892,7 @@ static const struct scp_domain_data scp_domain_data_mt8173[] = {
> .sram_pdn_bits = GENMASK(11, 8),
> .sram_pdn_ack_bits = GENMASK(15, 12),
> .clk_id = {CLK_NONE},
> - .active_wakeup = true,
> + .caps = MTK_SCPD_ACTIVE_WAKEUP,
> },
> [MT8173_POWER_DOMAIN_MFG_ASYNC] = {
> .name = "mfg_async",
>
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2018-04-30 9:59 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-23 8:36 [PATCH v2 1/2] soc: mediatek: introduce a CAPS flag for scp_domain_data sean.wang
2018-04-23 8:36 ` [PATCH v2 2/2] soc: mediatek: add a fixed wait for SRAM stable sean.wang
2018-04-23 9:31 ` Matthias Brugger
2018-04-23 9:39 ` Sean Wang
2018-04-27 9:46 ` Matthias Brugger
2018-04-30 7:08 ` Sean Wang
2018-04-30 9:10 ` Matthias Brugger
2018-04-30 9:59 ` Sean Wang
2018-04-27 9:43 ` [PATCH v2 1/2] soc: mediatek: introduce a CAPS flag for scp_domain_data Matthias Brugger
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).