Subject: [PATCH 0/8] soc: mediatek: MT8365 power support

Hi,

sorry for the long delay, here is an update to the mt8365 power domain
series. Thanks for all your feedback, I hope I addressed everything. As
Matthias brought up some good points, I got back to try to make the
WAY_EN feature a bit more understandable with less code duplication. I
added a few patches to prepare the driver for the WAY_EN patch. I hope
it is better to understand now.

Thank you!

Best,
Markus

Changes in v4:
- Redesigned WAY_EN patch and split it up in smaller patches.
- Added two documentation patches.
- Added mediatek,infracfg-nao field to the binding.

Changes in v3:
- Mainly redesigned WAY_EN patch to be easier to understand
- Rebased onto v6.0-rc1
- Several other stuff that is described in the individual patches

Changes in v2:
- Updated error handling path for scpsys_power_on()
- Minor updates described in each patch

Previous versions:
v1 - https://lore.kernel.org/linux-mediatek/[email protected]/
v2 - https://lore.kernel.org/linux-mediatek/[email protected]/
v3 - https://lore.kernel.org/linux-mediatek/[email protected]/

Alexandre Bailon (2):
soc: mediatek: Add support for WAY_EN operations
soc: mediatek: Add support for MTK_SCPD_STRICT_BUS_PROTECTION cap

Fabien Parent (2):
dt-bindings: power: Add MT8365 power domains
soc: mediatek: pm-domains: Add support for MT8365

Markus Schneider-Pargmann (4):
soc: mediatek: pm-domains: Split bus_prot_mask
soc: mediatek: pm-domains: Create bus protection operation functions
soc: mediatek: pm-domains: Document scpsys_bus_prot_data
soc: mediatek: pm-domains: Fix caps field documentation

.../power/mediatek,power-controller.yaml | 6 +
drivers/soc/mediatek/mt8365-pm-domains.h | 147 ++++++++++++++++++
drivers/soc/mediatek/mtk-pm-domains.c | 135 +++++++++++-----
drivers/soc/mediatek/mtk-pm-domains.h | 37 ++++-
.../dt-bindings/power/mediatek,mt8365-power.h | 19 +++
5 files changed, 302 insertions(+), 42 deletions(-)
create mode 100644 drivers/soc/mediatek/mt8365-pm-domains.h
create mode 100644 include/dt-bindings/power/mediatek,mt8365-power.h

--
2.39.0


Subject: [PATCH 4/8] soc: mediatek: pm-domains: Document scpsys_bus_prot_data

Add a short documentation for the fields in struct scpsys_bus_prot_data.

Signed-off-by: Markus Schneider-Pargmann <[email protected]>
---
drivers/soc/mediatek/mtk-pm-domains.h | 12 ++++++++++++
1 file changed, 12 insertions(+)

diff --git a/drivers/soc/mediatek/mtk-pm-domains.h b/drivers/soc/mediatek/mtk-pm-domains.h
index 8aaed1c939d7..da827e91d462 100644
--- a/drivers/soc/mediatek/mtk-pm-domains.h
+++ b/drivers/soc/mediatek/mtk-pm-domains.h
@@ -66,6 +66,18 @@
INFRA_TOPAXI_PROTECTEN, \
INFRA_TOPAXI_PROTECTSTA1)

+/**
+ * struct scpsys_bus_prot_data - Bus protection setting
+ * @bus_prot_set_clr_mask: Bitmask used for the set and clear registers.
+ * @bus_prot_set: infracfg set register.
+ * @bus_prot_clr: infracfg clear register.
+ * @bus_prot_sta_mask: Bitmask used for the status register.
+ * @bus_prot_sta: infracfg status register.
+ * @bus_prot_reg_update: Only update the register bits given in the mask, do not
+ * write the whole register.
+ * @ignore_clk_ack: Ignore the result in the status register for clear
+ * operations.
+ */
struct scpsys_bus_prot_data {
u32 bus_prot_set_clr_mask;
u32 bus_prot_set;
--
2.39.0

Subject: [PATCH 6/8] soc: mediatek: Add support for WAY_EN operations

From: Alexandre Bailon <[email protected]>

This updates the power domain to support WAY_EN operations. These
operations enable a path between different units of the chip and are
labeled as 'way_en' in the register descriptions.

This operation is required by the mt8365 for the MM power domain.

Signed-off-by: Alexandre Bailon <[email protected]>
Signed-off-by: Fabien Parent <[email protected]>
Signed-off-by: Markus Schneider-Pargmann <[email protected]>
---

Notes:
Changes in v4:
- Redesigned the patch to be better to understand and have less code
duplication

Changes in v3:
- Separated the way_en functions for clarity
- Added some checks for infracfg_nao

Changes in v2:
- some minor style fixes.
- Renamed 'wayen' to 'way_en' to clarify the meaning
- Updated commit message

drivers/soc/mediatek/mtk-pm-domains.c | 57 ++++++++++++++++++++-------
drivers/soc/mediatek/mtk-pm-domains.h | 16 ++++++--
2 files changed, 54 insertions(+), 19 deletions(-)

diff --git a/drivers/soc/mediatek/mtk-pm-domains.c b/drivers/soc/mediatek/mtk-pm-domains.c
index 999e1f6c86b0..d53309f050ee 100644
--- a/drivers/soc/mediatek/mtk-pm-domains.c
+++ b/drivers/soc/mediatek/mtk-pm-domains.c
@@ -43,6 +43,7 @@ struct scpsys_domain {
struct clk_bulk_data *clks;
int num_subsys_clks;
struct clk_bulk_data *subsys_clks;
+ struct regmap *infracfg_nao;
struct regmap *infracfg;
struct regmap *smi;
struct regulator *supply;
@@ -118,10 +119,13 @@ static int scpsys_sram_disable(struct scpsys_domain *pd)
}

static int scpsys_bus_protect_clear(const struct scpsys_bus_prot_data *bpd,
- struct regmap *regmap)
+ struct regmap *regmap,
+ struct regmap *sta_regmap)
{
u32 val;
u32 sta_mask = bpd->bus_prot_sta_mask;
+ /* way_en acknowledges successful clear with the bit being set */
+ u32 expected_ack = (bpd->way_en ? sta_mask : 0);

if (bpd->bus_prot_reg_update)
regmap_clear_bits(regmap, bpd->bus_prot_clr, bpd->bus_prot_set_clr_mask);
@@ -131,13 +135,14 @@ static int scpsys_bus_protect_clear(const struct scpsys_bus_prot_data *bpd,
if (bpd->ignore_clr_ack)
return 0;

- return regmap_read_poll_timeout(regmap, bpd->bus_prot_sta,
- val, !(val & sta_mask),
+ return regmap_read_poll_timeout(sta_regmap, bpd->bus_prot_sta,
+ val, (val & sta_mask) == expected_ack,
MTK_POLL_DELAY_US, MTK_POLL_TIMEOUT);
}

static int scpsys_bus_protect_set(const struct scpsys_bus_prot_data *bpd,
- struct regmap *regmap)
+ struct regmap *regmap,
+ struct regmap *sta_regmap)
{
u32 val;
u32 sta_mask = bpd->bus_prot_sta_mask;
@@ -147,12 +152,13 @@ static int scpsys_bus_protect_set(const struct scpsys_bus_prot_data *bpd,
else
regmap_write(regmap, bpd->bus_prot_set, bpd->bus_prot_set_clr_mask);

- return regmap_read_poll_timeout(regmap, bpd->bus_prot_sta,
+ return regmap_read_poll_timeout(sta_regmap, bpd->bus_prot_sta,
val, (val & sta_mask) == sta_mask,
MTK_POLL_DELAY_US, MTK_POLL_TIMEOUT);
}

-static int _scpsys_bus_protect_enable(const struct scpsys_bus_prot_data *bpd, struct regmap *regmap)
+static int _scpsys_bus_protect_enable(const struct scpsys_bus_prot_data *bpd,
+ struct regmap *regmap, struct regmap *infracfg_nao)
{
int i, ret;

@@ -160,9 +166,14 @@ static int _scpsys_bus_protect_enable(const struct scpsys_bus_prot_data *bpd, st
if (!bpd[i].bus_prot_set_clr_mask)
break;

- ret = scpsys_bus_protect_set(&bpd[i], regmap);
- if (ret)
+ if (bpd[i].way_en)
+ ret = scpsys_bus_protect_clear(&bpd[i], regmap, infracfg_nao);
+ else
+ ret = scpsys_bus_protect_set(&bpd[i], regmap, regmap);
+ if (ret) {
+ pr_err("%s %d %d\n", __PRETTY_FUNCTION__, __LINE__, ret);
return ret;
+ }
}

return 0;
@@ -172,15 +183,17 @@ static int scpsys_bus_protect_enable(struct scpsys_domain *pd)
{
int ret;

- ret = _scpsys_bus_protect_enable(pd->data->bp_infracfg, pd->infracfg);
+ ret = _scpsys_bus_protect_enable(pd->data->bp_infracfg,
+ pd->infracfg, pd->infracfg_nao);
if (ret)
return ret;

- return _scpsys_bus_protect_enable(pd->data->bp_smi, pd->smi);
+ return _scpsys_bus_protect_enable(pd->data->bp_smi, pd->smi, NULL);
}

static int _scpsys_bus_protect_disable(const struct scpsys_bus_prot_data *bpd,
- struct regmap *regmap)
+ struct regmap *regmap,
+ struct regmap *infracfg_nao)
{
int i, ret;

@@ -188,9 +201,14 @@ static int _scpsys_bus_protect_disable(const struct scpsys_bus_prot_data *bpd,
if (!bpd[i].bus_prot_set_clr_mask)
continue;

- ret = scpsys_bus_protect_clear(&bpd[i], regmap);
- if (ret)
+ if (bpd[i].way_en)
+ ret = scpsys_bus_protect_set(&bpd[i], regmap, infracfg_nao);
+ else
+ ret = scpsys_bus_protect_clear(&bpd[i], regmap, regmap);
+ if (ret) {
+ pr_err("%s %d %d\n", __PRETTY_FUNCTION__, __LINE__, ret);
return ret;
+ }
}

return 0;
@@ -200,11 +218,12 @@ static int scpsys_bus_protect_disable(struct scpsys_domain *pd)
{
int ret;

- ret = _scpsys_bus_protect_disable(pd->data->bp_smi, pd->smi);
+ ret = _scpsys_bus_protect_disable(pd->data->bp_smi, pd->smi, NULL);
if (ret)
return ret;

- return _scpsys_bus_protect_disable(pd->data->bp_infracfg, pd->infracfg);
+ return _scpsys_bus_protect_disable(pd->data->bp_infracfg,
+ pd->infracfg, pd->infracfg_nao);
}

static int scpsys_regulator_enable(struct regulator *supply)
@@ -378,6 +397,14 @@ generic_pm_domain *scpsys_add_one_domain(struct scpsys *scpsys, struct device_no
return ERR_CAST(pd->smi);
}

+ pd->infracfg_nao = syscon_regmap_lookup_by_phandle(node, "mediatek,infracfg-nao");
+ if (IS_ERR(pd->infracfg_nao)) {
+ if (MTK_SCPD_CAPS(pd, MTK_SCPD_HAS_WAY_EN))
+ return ERR_CAST(pd->infracfg_nao);
+
+ pd->infracfg_nao = NULL;
+ }
+
num_clks = of_clk_get_parent_count(node);
if (num_clks > 0) {
/* Calculate number of subsys_clks */
diff --git a/drivers/soc/mediatek/mtk-pm-domains.h b/drivers/soc/mediatek/mtk-pm-domains.h
index 34642a279213..1fa634509db1 100644
--- a/drivers/soc/mediatek/mtk-pm-domains.h
+++ b/drivers/soc/mediatek/mtk-pm-domains.h
@@ -10,6 +10,7 @@
#define MTK_SCPD_DOMAIN_SUPPLY BIT(4)
/* can't set MTK_SCPD_KEEP_DEFAULT_OFF at the same time */
#define MTK_SCPD_ALWAYS_ON BIT(5)
+#define MTK_SCPD_HAS_WAY_EN BIT(6)
#define MTK_SCPD_CAPS(_scpd, _x) ((_scpd)->data->caps & (_x))

#define SPM_VDE_PWR_CON 0x0210
@@ -41,7 +42,7 @@

#define SPM_MAX_BUS_PROT_DATA 6

-#define _BUS_PROT(_set_clr_mask, _set, _clr, _sta_mask, _sta, _update, _ignore) { \
+#define _BUS_PROT(_set_clr_mask, _set, _clr, _sta_mask, _sta, _update, _ignore, _way_en) { \
.bus_prot_set_clr_mask = (_set_clr_mask), \
.bus_prot_set = _set, \
.bus_prot_clr = _clr, \
@@ -49,16 +50,20 @@
.bus_prot_sta = _sta, \
.bus_prot_reg_update = _update, \
.ignore_clr_ack = _ignore, \
+ .way_en = _way_en, \
}

#define BUS_PROT_WR(_mask, _set, _clr, _sta) \
- _BUS_PROT(_mask, _set, _clr, _mask, _sta, false, false)
+ _BUS_PROT(_mask, _set, _clr, _mask, _sta, false, false, false)

#define BUS_PROT_WR_IGN(_mask, _set, _clr, _sta) \
- _BUS_PROT(_mask, _set, _clr, _mask, _sta, false, true)
+ _BUS_PROT(_mask, _set, _clr, _mask, _sta, false, true, false)

#define BUS_PROT_UPDATE(_mask, _set, _clr, _sta) \
- _BUS_PROT(_mask, _set, _clr, _mask, _sta, true, false)
+ _BUS_PROT(_mask, _set, _clr, _mask, _sta, true, false, false)
+
+#define BUS_PROT_WAY_EN(_set_mask, _set, _sta_mask, _sta) \
+ _BUS_PROT(_set_mask, _set, _set, _sta_mask, _sta, true, false, true)

#define BUS_PROT_UPDATE_TOPAXI(_mask) \
BUS_PROT_UPDATE(_mask, \
@@ -77,6 +82,8 @@
* write the whole register.
* @ignore_clk_ack: Ignore the result in the status register for clear
* operations.
+ * @way_en: Enable a way in the SoC. For this type the status bit to acknowledge
+ * the change is always 1 on successful change.
*/
struct scpsys_bus_prot_data {
u32 bus_prot_set_clr_mask;
@@ -86,6 +93,7 @@ struct scpsys_bus_prot_data {
u32 bus_prot_sta;
bool bus_prot_reg_update;
bool ignore_clr_ack;
+ bool way_en;
};

/**
--
2.39.0

Subject: [PATCH 7/8] soc: mediatek: Add support for MTK_SCPD_STRICT_BUS_PROTECTION cap

From: Alexandre Bailon <[email protected]>

This adds support for MTK_SCPD_STRICT_BUS_PROTECTION capability. It is a
strict bus protection policy that requires the bus protection to be
disabled before accessing the bus.
This is required by the mt8365, for the MM power domain.

Signed-off-by: Alexandre Bailon <[email protected]>
Signed-off-by: Fabien Parent <[email protected]>
Signed-off-by: Markus Schneider-Pargmann <[email protected]>
---

Notes:
Changes in v4:
- Change name in title and commit message

Changes in v3:
- Rename MTK_SCPD_STRICT_BUSP to MTK_SCPD_STRICT_BUS_PROTECTION
- Remove extra bool variable reflecting the capability

Changes in v2:
- Fixup error handling path.

drivers/soc/mediatek/mtk-pm-domains.c | 27 +++++++++++++++++++++++----
drivers/soc/mediatek/mtk-pm-domains.h | 1 +
2 files changed, 24 insertions(+), 4 deletions(-)

diff --git a/drivers/soc/mediatek/mtk-pm-domains.c b/drivers/soc/mediatek/mtk-pm-domains.c
index d53309f050ee..29a9028dd9b3 100644
--- a/drivers/soc/mediatek/mtk-pm-domains.c
+++ b/drivers/soc/mediatek/mtk-pm-domains.c
@@ -265,9 +265,17 @@ static int scpsys_power_on(struct generic_pm_domain *genpd)
regmap_clear_bits(scpsys->base, pd->data->ctl_offs, PWR_ISO_BIT);
regmap_set_bits(scpsys->base, pd->data->ctl_offs, PWR_RST_B_BIT);

- ret = clk_bulk_prepare_enable(pd->num_subsys_clks, pd->subsys_clks);
- if (ret)
- goto err_pwr_ack;
+ /*
+ * In few Mediatek platforms(e.g. MT6779), the bus protect policy is
+ * stricter, which leads to bus protect release must be prior to bus
+ * access.
+ */
+ if (!MTK_SCPD_CAPS(pd, MTK_SCPD_STRICT_BUS_PROTECTION)) {
+ ret = clk_bulk_prepare_enable(pd->num_subsys_clks,
+ pd->subsys_clks);
+ if (ret)
+ goto err_pwr_ack;
+ }

ret = scpsys_sram_enable(pd);
if (ret < 0)
@@ -277,12 +285,23 @@ static int scpsys_power_on(struct generic_pm_domain *genpd)
if (ret < 0)
goto err_disable_sram;

+ if (MTK_SCPD_CAPS(pd, MTK_SCPD_STRICT_BUS_PROTECTION)) {
+ ret = clk_bulk_prepare_enable(pd->num_subsys_clks,
+ pd->subsys_clks);
+ if (ret)
+ goto err_enable_bus_protect;
+ }
+
return 0;

+err_enable_bus_protect:
+ scpsys_bus_protect_enable(pd);
err_disable_sram:
scpsys_sram_disable(pd);
err_disable_subsys_clks:
- clk_bulk_disable_unprepare(pd->num_subsys_clks, pd->subsys_clks);
+ if (!MTK_SCPD_CAPS(pd, MTK_SCPD_STRICT_BUS_PROTECTION))
+ clk_bulk_disable_unprepare(pd->num_subsys_clks,
+ pd->subsys_clks);
err_pwr_ack:
clk_bulk_disable_unprepare(pd->num_clks, pd->clks);
err_reg:
diff --git a/drivers/soc/mediatek/mtk-pm-domains.h b/drivers/soc/mediatek/mtk-pm-domains.h
index 1fa634509db1..e5aa2348a9db 100644
--- a/drivers/soc/mediatek/mtk-pm-domains.h
+++ b/drivers/soc/mediatek/mtk-pm-domains.h
@@ -11,6 +11,7 @@
/* can't set MTK_SCPD_KEEP_DEFAULT_OFF at the same time */
#define MTK_SCPD_ALWAYS_ON BIT(5)
#define MTK_SCPD_HAS_WAY_EN BIT(6)
+#define MTK_SCPD_STRICT_BUS_PROTECTION BIT(7)
#define MTK_SCPD_CAPS(_scpd, _x) ((_scpd)->data->caps & (_x))

#define SPM_VDE_PWR_CON 0x0210
--
2.39.0

Subject: [PATCH 8/8] soc: mediatek: pm-domains: Add support for MT8365

From: Fabien Parent <[email protected]>

Add the needed board data to support MT8365 SoC.

Signed-off-by: Fabien Parent <[email protected]>
Signed-off-by: Markus Schneider-Pargmann <[email protected]>
---
drivers/soc/mediatek/mt8365-pm-domains.h | 147 +++++++++++++++++++++++
drivers/soc/mediatek/mtk-pm-domains.c | 5 +
2 files changed, 152 insertions(+)
create mode 100644 drivers/soc/mediatek/mt8365-pm-domains.h

diff --git a/drivers/soc/mediatek/mt8365-pm-domains.h b/drivers/soc/mediatek/mt8365-pm-domains.h
new file mode 100644
index 000000000000..8735e833b15b
--- /dev/null
+++ b/drivers/soc/mediatek/mt8365-pm-domains.h
@@ -0,0 +1,147 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#ifndef __SOC_MEDIATEK_MT8365_PM_DOMAINS_H
+#define __SOC_MEDIATEK_MT8365_PM_DOMAINS_H
+
+#include "mtk-pm-domains.h"
+#include <dt-bindings/power/mediatek,mt8365-power.h>
+
+/*
+ * MT8365 power domain support
+ */
+
+static const struct scpsys_domain_data scpsys_domain_data_mt8365[] = {
+ [MT8365_POWER_DOMAIN_MM] = {
+ .name = "mm",
+ .sta_mask = PWR_STATUS_DISP,
+ .ctl_offs = 0x30c,
+ .pwr_sta_offs = 0x0180,
+ .pwr_sta2nd_offs = 0x0184,
+ .sram_pdn_bits = GENMASK(8, 8),
+ .sram_pdn_ack_bits = GENMASK(12, 12),
+ .caps = MTK_SCPD_STRICT_BUS_PROTECTION | MTK_SCPD_HAS_WAY_EN,
+ .bp_infracfg = {
+ BUS_PROT_WR(BIT(16) | BIT(17), 0x2a8, 0x2ac, 0x258),
+ BUS_PROT_WR(BIT(1) | BIT(2) | BIT(10) | BIT(11), 0x2a0, 0x2a4, 0x228),
+ BUS_PROT_WAY_EN(BIT(6), 0x200, BIT(24), 0x0),
+ BUS_PROT_WAY_EN(BIT(5), 0x234, BIT(14), 0x28),
+ BUS_PROT_WR(BIT(6), 0x2a0, 0x2a4, 0x228),
+ },
+ },
+ [MT8365_POWER_DOMAIN_VENC] = {
+ .name = "venc",
+ .sta_mask = PWR_STATUS_VENC,
+ .ctl_offs = 0x0304,
+ .pwr_sta_offs = 0x0180,
+ .pwr_sta2nd_offs = 0x0184,
+ .sram_pdn_bits = GENMASK(8, 8),
+ .sram_pdn_ack_bits = GENMASK(12, 12),
+ .bp_smi = {
+ BUS_PROT_WR(BIT(1), 0x3c4, 0x3c8, 0x3c0),
+ },
+ },
+ [MT8365_POWER_DOMAIN_AUDIO] = {
+ .name = "audio",
+ .sta_mask = PWR_STATUS_AUDIO,
+ .ctl_offs = 0x0314,
+ .pwr_sta_offs = 0x0180,
+ .pwr_sta2nd_offs = 0x0184,
+ .sram_pdn_bits = GENMASK(12, 8),
+ .sram_pdn_ack_bits = GENMASK(17, 13),
+ .bp_infracfg = {
+ BUS_PROT_WR(BIT(27) | BIT(28), 0x2a8, 0x2ac, 0x258),
+ },
+ .caps = MTK_SCPD_ACTIVE_WAKEUP,
+ },
+ [MT8365_POWER_DOMAIN_CONN] = {
+ .name = "conn",
+ .sta_mask = PWR_STATUS_CONN,
+ .ctl_offs = 0x032c,
+ .pwr_sta_offs = 0x0180,
+ .pwr_sta2nd_offs = 0x0184,
+ .sram_pdn_bits = 0,
+ .sram_pdn_ack_bits = 0,
+ .bp_infracfg = {
+ BUS_PROT_WR(BIT(13), 0x2a0, 0x2a4, 0x228),
+ BUS_PROT_WR(BIT(18), 0x2a8, 0x2ac, 0x258),
+ BUS_PROT_WR(BIT(14), 0x2a0, 0x2a4, 0x228),
+ BUS_PROT_WR(BIT(21), 0x2a8, 0x2ac, 0x258),
+ },
+ .caps = MTK_SCPD_ACTIVE_WAKEUP | MTK_SCPD_KEEP_DEFAULT_OFF,
+ },
+ [MT8365_POWER_DOMAIN_MFG] = {
+ .name = "mfg",
+ .sta_mask = PWR_STATUS_MFG,
+ .ctl_offs = 0x0338,
+ .pwr_sta_offs = 0x0180,
+ .pwr_sta2nd_offs = 0x0184,
+ .sram_pdn_bits = GENMASK(9, 8),
+ .sram_pdn_ack_bits = GENMASK(13, 12),
+ .bp_infracfg = {
+ BUS_PROT_WR(BIT(25), 0x2a0, 0x2a4, 0x228),
+ BUS_PROT_WR(BIT(21) | BIT(22), 0x2a0, 0x2a4, 0x228),
+ },
+ },
+ [MT8365_POWER_DOMAIN_CAM] = {
+ .name = "cam",
+ .sta_mask = BIT(25),
+ .ctl_offs = 0x0344,
+ .pwr_sta_offs = 0x0180,
+ .pwr_sta2nd_offs = 0x0184,
+ .sram_pdn_bits = GENMASK(9, 8),
+ .sram_pdn_ack_bits = GENMASK(13, 12),
+ .bp_infracfg = {
+ BUS_PROT_WR(BIT(19), 0x2a8, 0x2ac, 0x258),
+ },
+ .bp_smi = {
+ BUS_PROT_WR(BIT(2), 0x3c4, 0x3c8, 0x3c0),
+ },
+ },
+ [MT8365_POWER_DOMAIN_VDEC] = {
+ .name = "vdec",
+ .sta_mask = BIT(31),
+ .ctl_offs = 0x0370,
+ .pwr_sta_offs = 0x0180,
+ .pwr_sta2nd_offs = 0x0184,
+ .sram_pdn_bits = GENMASK(8, 8),
+ .sram_pdn_ack_bits = GENMASK(12, 12),
+ .bp_smi = {
+ BUS_PROT_WR(BIT(3), 0x3c4, 0x3c8, 0x3c0),
+ },
+ },
+ [MT8365_POWER_DOMAIN_APU] = {
+ .name = "apu",
+ .sta_mask = BIT(16),
+ .ctl_offs = 0x0378,
+ .pwr_sta_offs = 0x0180,
+ .pwr_sta2nd_offs = 0x0184,
+ .sram_pdn_bits = GENMASK(14, 8),
+ .sram_pdn_ack_bits = GENMASK(21, 15),
+ .bp_infracfg = {
+ BUS_PROT_WR(BIT(2) | BIT(20), 0x2a8, 0x2ac, 0x258),
+ },
+ .bp_smi = {
+ BUS_PROT_WR(BIT(4), 0x3c4, 0x3c8, 0x3c0),
+ },
+ },
+ [MT8365_POWER_DOMAIN_DSP] = {
+ .name = "dsp",
+ .sta_mask = BIT(17),
+ .ctl_offs = 0x037C,
+ .pwr_sta_offs = 0x0180,
+ .pwr_sta2nd_offs = 0x0184,
+ .sram_pdn_bits = GENMASK(11, 8),
+ .sram_pdn_ack_bits = GENMASK(15, 12),
+ .bp_infracfg = {
+ BUS_PROT_WR(BIT(24) | BIT(30) | BIT(31), 0x2a8, 0x2ac, 0x258),
+ },
+ .caps = MTK_SCPD_ACTIVE_WAKEUP,
+ },
+};
+
+static const struct scpsys_soc_data mt8365_scpsys_data = {
+ .domains_data = scpsys_domain_data_mt8365,
+ .num_domains = ARRAY_SIZE(scpsys_domain_data_mt8365),
+};
+
+#endif /* __SOC_MEDIATEK_MT8365_PM_DOMAINS_H */
diff --git a/drivers/soc/mediatek/mtk-pm-domains.c b/drivers/soc/mediatek/mtk-pm-domains.c
index 29a9028dd9b3..adb7716df0a8 100644
--- a/drivers/soc/mediatek/mtk-pm-domains.c
+++ b/drivers/soc/mediatek/mtk-pm-domains.c
@@ -23,6 +23,7 @@
#include "mt8186-pm-domains.h"
#include "mt8192-pm-domains.h"
#include "mt8195-pm-domains.h"
+#include "mt8365-pm-domains.h"

#define MTK_POLL_DELAY_US 10
#define MTK_POLL_TIMEOUT USEC_PER_SEC
@@ -647,6 +648,10 @@ static const struct of_device_id scpsys_of_match[] = {
.compatible = "mediatek,mt8195-power-controller",
.data = &mt8195_scpsys_data,
},
+ {
+ .compatible = "mediatek,mt8365-power-controller",
+ .data = &mt8365_scpsys_data,
+ },
{ }
};

--
2.39.0

Subject: [PATCH 5/8] soc: mediatek: pm-domains: Fix caps field documentation

This field is not for ACTIVE_WAKEUP exclusively. There are a lot of
other flags defined as well.

Fixes: 59b644b01cf4 ("soc: mediatek: Add MediaTek SCPSYS power domains")
Signed-off-by: Markus Schneider-Pargmann <[email protected]>
---
drivers/soc/mediatek/mtk-pm-domains.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/soc/mediatek/mtk-pm-domains.h b/drivers/soc/mediatek/mtk-pm-domains.h
index da827e91d462..34642a279213 100644
--- a/drivers/soc/mediatek/mtk-pm-domains.h
+++ b/drivers/soc/mediatek/mtk-pm-domains.h
@@ -95,7 +95,7 @@ struct scpsys_bus_prot_data {
* @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.
- * @caps: The flag for active wake-up action.
+ * @caps: MTK_SCPD_* capability flags.
* @bp_infracfg: bus protection for infracfg subsystem
* @bp_smi: bus protection for smi subsystem
*/
--
2.39.0

Subject: [PATCH 2/8] soc: mediatek: pm-domains: Split bus_prot_mask

bus_prot_mask is used for all operations, set clear and acknowledge. In
preparation of m8365 power domain support split this one mask into two,
one mask for set and clear, another one for acknowledge.

Signed-off-by: Markus Schneider-Pargmann <[email protected]>
---
drivers/soc/mediatek/mtk-pm-domains.c | 24 ++++++++++++++----------
drivers/soc/mediatek/mtk-pm-domains.h | 14 ++++++++------
2 files changed, 22 insertions(+), 16 deletions(-)

diff --git a/drivers/soc/mediatek/mtk-pm-domains.c b/drivers/soc/mediatek/mtk-pm-domains.c
index 474b272f9b02..4333cd297405 100644
--- a/drivers/soc/mediatek/mtk-pm-domains.c
+++ b/drivers/soc/mediatek/mtk-pm-domains.c
@@ -122,18 +122,20 @@ static int _scpsys_bus_protect_enable(const struct scpsys_bus_prot_data *bpd, st
int i, ret;

for (i = 0; i < SPM_MAX_BUS_PROT_DATA; i++) {
- u32 val, mask = bpd[i].bus_prot_mask;
+ u32 val;
+ u32 set_clr_mask = bpd[i].bus_prot_set_clr_mask;
+ u32 sta_mask = bpd[i].bus_prot_sta_mask;

- if (!mask)
+ if (!set_clr_mask)
break;

if (bpd[i].bus_prot_reg_update)
- regmap_set_bits(regmap, bpd[i].bus_prot_set, mask);
+ regmap_set_bits(regmap, bpd[i].bus_prot_set, set_clr_mask);
else
- regmap_write(regmap, bpd[i].bus_prot_set, mask);
+ regmap_write(regmap, bpd[i].bus_prot_set, set_clr_mask);

ret = regmap_read_poll_timeout(regmap, bpd[i].bus_prot_sta,
- val, (val & mask) == mask,
+ val, (val & sta_mask) == sta_mask,
MTK_POLL_DELAY_US, MTK_POLL_TIMEOUT);
if (ret)
return ret;
@@ -159,21 +161,23 @@ static int _scpsys_bus_protect_disable(const struct scpsys_bus_prot_data *bpd,
int i, ret;

for (i = SPM_MAX_BUS_PROT_DATA - 1; i >= 0; i--) {
- u32 val, mask = bpd[i].bus_prot_mask;
+ u32 val;
+ u32 set_clr_mask = bpd[i].bus_prot_set_clr_mask;
+ u32 sta_mask = bpd[i].bus_prot_sta_mask;

- if (!mask)
+ if (!set_clr_mask)
continue;

if (bpd[i].bus_prot_reg_update)
- regmap_clear_bits(regmap, bpd[i].bus_prot_clr, mask);
+ regmap_clear_bits(regmap, bpd[i].bus_prot_clr, set_clr_mask);
else
- regmap_write(regmap, bpd[i].bus_prot_clr, mask);
+ regmap_write(regmap, bpd[i].bus_prot_clr, set_clr_mask);

if (bpd[i].ignore_clr_ack)
continue;

ret = regmap_read_poll_timeout(regmap, bpd[i].bus_prot_sta,
- val, !(val & mask),
+ val, !(val & sta_mask),
MTK_POLL_DELAY_US, MTK_POLL_TIMEOUT);
if (ret)
return ret;
diff --git a/drivers/soc/mediatek/mtk-pm-domains.h b/drivers/soc/mediatek/mtk-pm-domains.h
index 7d3c0c36316c..8aaed1c939d7 100644
--- a/drivers/soc/mediatek/mtk-pm-domains.h
+++ b/drivers/soc/mediatek/mtk-pm-domains.h
@@ -41,23 +41,24 @@

#define SPM_MAX_BUS_PROT_DATA 6

-#define _BUS_PROT(_mask, _set, _clr, _sta, _update, _ignore) { \
- .bus_prot_mask = (_mask), \
+#define _BUS_PROT(_set_clr_mask, _set, _clr, _sta_mask, _sta, _update, _ignore) { \
+ .bus_prot_set_clr_mask = (_set_clr_mask), \
.bus_prot_set = _set, \
.bus_prot_clr = _clr, \
+ .bus_prot_sta_mask = (_sta_mask), \
.bus_prot_sta = _sta, \
.bus_prot_reg_update = _update, \
.ignore_clr_ack = _ignore, \
}

#define BUS_PROT_WR(_mask, _set, _clr, _sta) \
- _BUS_PROT(_mask, _set, _clr, _sta, false, false)
+ _BUS_PROT(_mask, _set, _clr, _mask, _sta, false, false)

#define BUS_PROT_WR_IGN(_mask, _set, _clr, _sta) \
- _BUS_PROT(_mask, _set, _clr, _sta, false, true)
+ _BUS_PROT(_mask, _set, _clr, _mask, _sta, false, true)

#define BUS_PROT_UPDATE(_mask, _set, _clr, _sta) \
- _BUS_PROT(_mask, _set, _clr, _sta, true, false)
+ _BUS_PROT(_mask, _set, _clr, _mask, _sta, true, false)

#define BUS_PROT_UPDATE_TOPAXI(_mask) \
BUS_PROT_UPDATE(_mask, \
@@ -66,9 +67,10 @@
INFRA_TOPAXI_PROTECTSTA1)

struct scpsys_bus_prot_data {
- u32 bus_prot_mask;
+ u32 bus_prot_set_clr_mask;
u32 bus_prot_set;
u32 bus_prot_clr;
+ u32 bus_prot_sta_mask;
u32 bus_prot_sta;
bool bus_prot_reg_update;
bool ignore_clr_ack;
--
2.39.0

Subject: [PATCH 3/8] soc: mediatek: pm-domains: Create bus protection operation functions

Separate the register access used for bus protection enable/disable into
their own functions. These will be used later for WAY_EN bits.

Signed-off-by: Markus Schneider-Pargmann <[email protected]>
---
drivers/soc/mediatek/mtk-pm-domains.c | 68 +++++++++++++++------------
1 file changed, 39 insertions(+), 29 deletions(-)

diff --git a/drivers/soc/mediatek/mtk-pm-domains.c b/drivers/soc/mediatek/mtk-pm-domains.c
index 4333cd297405..999e1f6c86b0 100644
--- a/drivers/soc/mediatek/mtk-pm-domains.c
+++ b/drivers/soc/mediatek/mtk-pm-domains.c
@@ -117,26 +117,50 @@ static int scpsys_sram_disable(struct scpsys_domain *pd)
MTK_POLL_TIMEOUT);
}

+static int scpsys_bus_protect_clear(const struct scpsys_bus_prot_data *bpd,
+ struct regmap *regmap)
+{
+ u32 val;
+ u32 sta_mask = bpd->bus_prot_sta_mask;
+
+ if (bpd->bus_prot_reg_update)
+ regmap_clear_bits(regmap, bpd->bus_prot_clr, bpd->bus_prot_set_clr_mask);
+ else
+ regmap_write(regmap, bpd->bus_prot_clr, bpd->bus_prot_set_clr_mask);
+
+ if (bpd->ignore_clr_ack)
+ return 0;
+
+ return regmap_read_poll_timeout(regmap, bpd->bus_prot_sta,
+ val, !(val & sta_mask),
+ MTK_POLL_DELAY_US, MTK_POLL_TIMEOUT);
+}
+
+static int scpsys_bus_protect_set(const struct scpsys_bus_prot_data *bpd,
+ struct regmap *regmap)
+{
+ u32 val;
+ u32 sta_mask = bpd->bus_prot_sta_mask;
+
+ if (bpd->bus_prot_reg_update)
+ regmap_set_bits(regmap, bpd->bus_prot_set, bpd->bus_prot_set_clr_mask);
+ else
+ regmap_write(regmap, bpd->bus_prot_set, bpd->bus_prot_set_clr_mask);
+
+ return regmap_read_poll_timeout(regmap, bpd->bus_prot_sta,
+ val, (val & sta_mask) == sta_mask,
+ MTK_POLL_DELAY_US, MTK_POLL_TIMEOUT);
+}
+
static int _scpsys_bus_protect_enable(const struct scpsys_bus_prot_data *bpd, struct regmap *regmap)
{
int i, ret;

for (i = 0; i < SPM_MAX_BUS_PROT_DATA; i++) {
- u32 val;
- u32 set_clr_mask = bpd[i].bus_prot_set_clr_mask;
- u32 sta_mask = bpd[i].bus_prot_sta_mask;
-
- if (!set_clr_mask)
+ if (!bpd[i].bus_prot_set_clr_mask)
break;

- if (bpd[i].bus_prot_reg_update)
- regmap_set_bits(regmap, bpd[i].bus_prot_set, set_clr_mask);
- else
- regmap_write(regmap, bpd[i].bus_prot_set, set_clr_mask);
-
- ret = regmap_read_poll_timeout(regmap, bpd[i].bus_prot_sta,
- val, (val & sta_mask) == sta_mask,
- MTK_POLL_DELAY_US, MTK_POLL_TIMEOUT);
+ ret = scpsys_bus_protect_set(&bpd[i], regmap);
if (ret)
return ret;
}
@@ -161,24 +185,10 @@ static int _scpsys_bus_protect_disable(const struct scpsys_bus_prot_data *bpd,
int i, ret;

for (i = SPM_MAX_BUS_PROT_DATA - 1; i >= 0; i--) {
- u32 val;
- u32 set_clr_mask = bpd[i].bus_prot_set_clr_mask;
- u32 sta_mask = bpd[i].bus_prot_sta_mask;
-
- if (!set_clr_mask)
- continue;
-
- if (bpd[i].bus_prot_reg_update)
- regmap_clear_bits(regmap, bpd[i].bus_prot_clr, set_clr_mask);
- else
- regmap_write(regmap, bpd[i].bus_prot_clr, set_clr_mask);
-
- if (bpd[i].ignore_clr_ack)
+ if (!bpd[i].bus_prot_set_clr_mask)
continue;

- ret = regmap_read_poll_timeout(regmap, bpd[i].bus_prot_sta,
- val, !(val & sta_mask),
- MTK_POLL_DELAY_US, MTK_POLL_TIMEOUT);
+ ret = scpsys_bus_protect_clear(&bpd[i], regmap);
if (ret)
return ret;
}
--
2.39.0

2023-02-03 11:14:51

by Matthias Brugger

[permalink] [raw]
Subject: Re: [PATCH 5/8] soc: mediatek: pm-domains: Fix caps field documentation



On 05/01/2023 18:07, Markus Schneider-Pargmann wrote:
> This field is not for ACTIVE_WAKEUP exclusively. There are a lot of
> other flags defined as well.
>
> Fixes: 59b644b01cf4 ("soc: mediatek: Add MediaTek SCPSYS power domains")

No bug here, nothing that needs to be fixed in the stable kernels. We can drop
the Fixes tag here.

> Signed-off-by: Markus Schneider-Pargmann <[email protected]>
> ---
> drivers/soc/mediatek/mtk-pm-domains.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/soc/mediatek/mtk-pm-domains.h b/drivers/soc/mediatek/mtk-pm-domains.h
> index da827e91d462..34642a279213 100644
> --- a/drivers/soc/mediatek/mtk-pm-domains.h
> +++ b/drivers/soc/mediatek/mtk-pm-domains.h
> @@ -95,7 +95,7 @@ struct scpsys_bus_prot_data {
> * @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.
> - * @caps: The flag for active wake-up action.
> + * @caps: MTK_SCPD_* capability flags.
> * @bp_infracfg: bus protection for infracfg subsystem
> * @bp_smi: bus protection for smi subsystem
> */

2023-02-03 12:23:03

by Matthias Brugger

[permalink] [raw]
Subject: Re: [PATCH 8/8] soc: mediatek: pm-domains: Add support for MT8365



On 05/01/2023 18:07, Markus Schneider-Pargmann wrote:
> From: Fabien Parent <[email protected]>
>
> Add the needed board data to support MT8365 SoC.
>
> Signed-off-by: Fabien Parent <[email protected]>
> Signed-off-by: Markus Schneider-Pargmann <[email protected]>
> ---
> drivers/soc/mediatek/mt8365-pm-domains.h | 147 +++++++++++++++++++++++
> drivers/soc/mediatek/mtk-pm-domains.c | 5 +
> 2 files changed, 152 insertions(+)
> create mode 100644 drivers/soc/mediatek/mt8365-pm-domains.h
>
> diff --git a/drivers/soc/mediatek/mt8365-pm-domains.h b/drivers/soc/mediatek/mt8365-pm-domains.h
> new file mode 100644
> index 000000000000..8735e833b15b
> --- /dev/null
> +++ b/drivers/soc/mediatek/mt8365-pm-domains.h
> @@ -0,0 +1,147 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +
> +#ifndef __SOC_MEDIATEK_MT8365_PM_DOMAINS_H
> +#define __SOC_MEDIATEK_MT8365_PM_DOMAINS_H
> +
> +#include "mtk-pm-domains.h"
> +#include <dt-bindings/power/mediatek,mt8365-power.h>
> +
> +/*
> + * MT8365 power domain support
> + */
> +
> +static const struct scpsys_domain_data scpsys_domain_data_mt8365[] = {
> + [MT8365_POWER_DOMAIN_MM] = {
> + .name = "mm",
> + .sta_mask = PWR_STATUS_DISP,
> + .ctl_offs = 0x30c,
> + .pwr_sta_offs = 0x0180,
> + .pwr_sta2nd_offs = 0x0184,
> + .sram_pdn_bits = GENMASK(8, 8),
> + .sram_pdn_ack_bits = GENMASK(12, 12),
> + .caps = MTK_SCPD_STRICT_BUS_PROTECTION | MTK_SCPD_HAS_WAY_EN,
> + .bp_infracfg = {
> + BUS_PROT_WR(BIT(16) | BIT(17), 0x2a8, 0x2ac, 0x258),
> + BUS_PROT_WR(BIT(1) | BIT(2) | BIT(10) | BIT(11), 0x2a0, 0x2a4, 0x228),
> + BUS_PROT_WAY_EN(BIT(6), 0x200, BIT(24), 0x0),
> + BUS_PROT_WAY_EN(BIT(5), 0x234, BIT(14), 0x28),
> + BUS_PROT_WR(BIT(6), 0x2a0, 0x2a4, 0x228),


BUS_PROT_WR(BIT(6), 0x2a0, 0x2a4, 0x228) repeates several times in the
definition. Would it make sense to create a new define like we did with
BUS_PROT_UPDATE_TOPAXI()? Are this offests are used in other SoCs.

In any case instead of magic numbers the values should be defined in
include/linux/soc/mediatek/infracfg.h or appropiate header files.

Regards,
Matthias

> + },
> + },
> + [MT8365_POWER_DOMAIN_VENC] = {
> + .name = "venc",
> + .sta_mask = PWR_STATUS_VENC,
> + .ctl_offs = 0x0304,
> + .pwr_sta_offs = 0x0180,
> + .pwr_sta2nd_offs = 0x0184,
> + .sram_pdn_bits = GENMASK(8, 8),
> + .sram_pdn_ack_bits = GENMASK(12, 12),
> + .bp_smi = {
> + BUS_PROT_WR(BIT(1), 0x3c4, 0x3c8, 0x3c0),
> + },
> + },
> + [MT8365_POWER_DOMAIN_AUDIO] = {
> + .name = "audio",
> + .sta_mask = PWR_STATUS_AUDIO,
> + .ctl_offs = 0x0314,
> + .pwr_sta_offs = 0x0180,
> + .pwr_sta2nd_offs = 0x0184,
> + .sram_pdn_bits = GENMASK(12, 8),
> + .sram_pdn_ack_bits = GENMASK(17, 13),
> + .bp_infracfg = {
> + BUS_PROT_WR(BIT(27) | BIT(28), 0x2a8, 0x2ac, 0x258),
> + },
> + .caps = MTK_SCPD_ACTIVE_WAKEUP,
> + },
> + [MT8365_POWER_DOMAIN_CONN] = {
> + .name = "conn",
> + .sta_mask = PWR_STATUS_CONN,
> + .ctl_offs = 0x032c,
> + .pwr_sta_offs = 0x0180,
> + .pwr_sta2nd_offs = 0x0184,
> + .sram_pdn_bits = 0,
> + .sram_pdn_ack_bits = 0,
> + .bp_infracfg = {
> + BUS_PROT_WR(BIT(13), 0x2a0, 0x2a4, 0x228),
> + BUS_PROT_WR(BIT(18), 0x2a8, 0x2ac, 0x258),
> + BUS_PROT_WR(BIT(14), 0x2a0, 0x2a4, 0x228),
> + BUS_PROT_WR(BIT(21), 0x2a8, 0x2ac, 0x258),
> + },
> + .caps = MTK_SCPD_ACTIVE_WAKEUP | MTK_SCPD_KEEP_DEFAULT_OFF,
> + },
> + [MT8365_POWER_DOMAIN_MFG] = {
> + .name = "mfg",
> + .sta_mask = PWR_STATUS_MFG,
> + .ctl_offs = 0x0338,
> + .pwr_sta_offs = 0x0180,
> + .pwr_sta2nd_offs = 0x0184,
> + .sram_pdn_bits = GENMASK(9, 8),
> + .sram_pdn_ack_bits = GENMASK(13, 12),
> + .bp_infracfg = {
> + BUS_PROT_WR(BIT(25), 0x2a0, 0x2a4, 0x228),
> + BUS_PROT_WR(BIT(21) | BIT(22), 0x2a0, 0x2a4, 0x228),
> + },
> + },
> + [MT8365_POWER_DOMAIN_CAM] = {
> + .name = "cam",
> + .sta_mask = BIT(25),
> + .ctl_offs = 0x0344,
> + .pwr_sta_offs = 0x0180,
> + .pwr_sta2nd_offs = 0x0184,
> + .sram_pdn_bits = GENMASK(9, 8),
> + .sram_pdn_ack_bits = GENMASK(13, 12),
> + .bp_infracfg = {
> + BUS_PROT_WR(BIT(19), 0x2a8, 0x2ac, 0x258),
> + },
> + .bp_smi = {
> + BUS_PROT_WR(BIT(2), 0x3c4, 0x3c8, 0x3c0),
> + },
> + },
> + [MT8365_POWER_DOMAIN_VDEC] = {
> + .name = "vdec",
> + .sta_mask = BIT(31),
> + .ctl_offs = 0x0370,
> + .pwr_sta_offs = 0x0180,
> + .pwr_sta2nd_offs = 0x0184,
> + .sram_pdn_bits = GENMASK(8, 8),
> + .sram_pdn_ack_bits = GENMASK(12, 12),
> + .bp_smi = {
> + BUS_PROT_WR(BIT(3), 0x3c4, 0x3c8, 0x3c0),
> + },
> + },
> + [MT8365_POWER_DOMAIN_APU] = {
> + .name = "apu",
> + .sta_mask = BIT(16),
> + .ctl_offs = 0x0378,
> + .pwr_sta_offs = 0x0180,
> + .pwr_sta2nd_offs = 0x0184,
> + .sram_pdn_bits = GENMASK(14, 8),
> + .sram_pdn_ack_bits = GENMASK(21, 15),
> + .bp_infracfg = {
> + BUS_PROT_WR(BIT(2) | BIT(20), 0x2a8, 0x2ac, 0x258),
> + },
> + .bp_smi = {
> + BUS_PROT_WR(BIT(4), 0x3c4, 0x3c8, 0x3c0),
> + },
> + },
> + [MT8365_POWER_DOMAIN_DSP] = {
> + .name = "dsp",
> + .sta_mask = BIT(17),
> + .ctl_offs = 0x037C,
> + .pwr_sta_offs = 0x0180,
> + .pwr_sta2nd_offs = 0x0184,
> + .sram_pdn_bits = GENMASK(11, 8),
> + .sram_pdn_ack_bits = GENMASK(15, 12),
> + .bp_infracfg = {
> + BUS_PROT_WR(BIT(24) | BIT(30) | BIT(31), 0x2a8, 0x2ac, 0x258),
> + },
> + .caps = MTK_SCPD_ACTIVE_WAKEUP,
> + },
> +};
> +
> +static const struct scpsys_soc_data mt8365_scpsys_data = {
> + .domains_data = scpsys_domain_data_mt8365,
> + .num_domains = ARRAY_SIZE(scpsys_domain_data_mt8365),
> +};
> +
> +#endif /* __SOC_MEDIATEK_MT8365_PM_DOMAINS_H */
> diff --git a/drivers/soc/mediatek/mtk-pm-domains.c b/drivers/soc/mediatek/mtk-pm-domains.c
> index 29a9028dd9b3..adb7716df0a8 100644
> --- a/drivers/soc/mediatek/mtk-pm-domains.c
> +++ b/drivers/soc/mediatek/mtk-pm-domains.c
> @@ -23,6 +23,7 @@
> #include "mt8186-pm-domains.h"
> #include "mt8192-pm-domains.h"
> #include "mt8195-pm-domains.h"
> +#include "mt8365-pm-domains.h"
>
> #define MTK_POLL_DELAY_US 10
> #define MTK_POLL_TIMEOUT USEC_PER_SEC
> @@ -647,6 +648,10 @@ static const struct of_device_id scpsys_of_match[] = {
> .compatible = "mediatek,mt8195-power-controller",
> .data = &mt8195_scpsys_data,
> },
> + {
> + .compatible = "mediatek,mt8365-power-controller",
> + .data = &mt8365_scpsys_data,
> + },
> { }
> };
>

2023-02-03 12:32:24

by Matthias Brugger

[permalink] [raw]
Subject: Re: [PATCH 3/8] soc: mediatek: pm-domains: Create bus protection operation functions



On 05/01/2023 18:07, Markus Schneider-Pargmann wrote:
> Separate the register access used for bus protection enable/disable into
> their own functions. These will be used later for WAY_EN bits.
>
> Signed-off-by: Markus Schneider-Pargmann <[email protected]>
> ---
> drivers/soc/mediatek/mtk-pm-domains.c | 68 +++++++++++++++------------
> 1 file changed, 39 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/soc/mediatek/mtk-pm-domains.c b/drivers/soc/mediatek/mtk-pm-domains.c
> index 4333cd297405..999e1f6c86b0 100644
> --- a/drivers/soc/mediatek/mtk-pm-domains.c
> +++ b/drivers/soc/mediatek/mtk-pm-domains.c
> @@ -117,26 +117,50 @@ static int scpsys_sram_disable(struct scpsys_domain *pd)
> MTK_POLL_TIMEOUT);
> }
>
> +static int scpsys_bus_protect_clear(const struct scpsys_bus_prot_data *bpd,
> + struct regmap *regmap)
> +{
> + u32 val;
> + u32 sta_mask = bpd->bus_prot_sta_mask;
> +
> + if (bpd->bus_prot_reg_update)
> + regmap_clear_bits(regmap, bpd->bus_prot_clr, bpd->bus_prot_set_clr_mask);
> + else
> + regmap_write(regmap, bpd->bus_prot_clr, bpd->bus_prot_set_clr_mask);
> +
> + if (bpd->ignore_clr_ack)
> + return 0;
> +
> + return regmap_read_poll_timeout(regmap, bpd->bus_prot_sta,
> + val, !(val & sta_mask),
> + MTK_POLL_DELAY_US, MTK_POLL_TIMEOUT);
> +}
> +

NIT: please move this function below
static int scpsys_bus_protect_enable(struct scpsys_domain *pd) {...}

This allows for better readability.

> +static int scpsys_bus_protect_set(const struct scpsys_bus_prot_data *bpd,
> + struct regmap *regmap)
> +{
> + u32 val;
> + u32 sta_mask = bpd->bus_prot_sta_mask;
> +
> + if (bpd->bus_prot_reg_update)
> + regmap_set_bits(regmap, bpd->bus_prot_set, bpd->bus_prot_set_clr_mask);
> + else
> + regmap_write(regmap, bpd->bus_prot_set, bpd->bus_prot_set_clr_mask);
> +
> + return regmap_read_poll_timeout(regmap, bpd->bus_prot_sta,
> + val, (val & sta_mask) == sta_mask,
> + MTK_POLL_DELAY_US, MTK_POLL_TIMEOUT);
> +}
> +
> static int _scpsys_bus_protect_enable(const struct scpsys_bus_prot_data *bpd, struct regmap *regmap)
> {
> int i, ret;
>
> for (i = 0; i < SPM_MAX_BUS_PROT_DATA; i++) {
> - u32 val;
> - u32 set_clr_mask = bpd[i].bus_prot_set_clr_mask;
> - u32 sta_mask = bpd[i].bus_prot_sta_mask;
> -
> - if (!set_clr_mask)
> + if (!bpd[i].bus_prot_set_clr_mask)
> break;
>
> - if (bpd[i].bus_prot_reg_update)
> - regmap_set_bits(regmap, bpd[i].bus_prot_set, set_clr_mask);
> - else
> - regmap_write(regmap, bpd[i].bus_prot_set, set_clr_mask);
> -
> - ret = regmap_read_poll_timeout(regmap, bpd[i].bus_prot_sta,
> - val, (val & sta_mask) == sta_mask,
> - MTK_POLL_DELAY_US, MTK_POLL_TIMEOUT);
> + ret = scpsys_bus_protect_set(&bpd[i], regmap);
> if (ret)
> return ret;
> }
> @@ -161,24 +185,10 @@ static int _scpsys_bus_protect_disable(const struct scpsys_bus_prot_data *bpd,
> int i, ret;
>
> for (i = SPM_MAX_BUS_PROT_DATA - 1; i >= 0; i--) {
> - u32 val;
> - u32 set_clr_mask = bpd[i].bus_prot_set_clr_mask;
> - u32 sta_mask = bpd[i].bus_prot_sta_mask;
> -
> - if (!set_clr_mask)
> - continue;
> -
> - if (bpd[i].bus_prot_reg_update)
> - regmap_clear_bits(regmap, bpd[i].bus_prot_clr, set_clr_mask);
> - else
> - regmap_write(regmap, bpd[i].bus_prot_clr, set_clr_mask);
> -
> - if (bpd[i].ignore_clr_ack)
> + if (!bpd[i].bus_prot_set_clr_mask)
> continue;
>
> - ret = regmap_read_poll_timeout(regmap, bpd[i].bus_prot_sta,
> - val, !(val & sta_mask),
> - MTK_POLL_DELAY_US, MTK_POLL_TIMEOUT);
> + ret = scpsys_bus_protect_clear(&bpd[i], regmap);
> if (ret)
> return ret;
> }

2023-02-03 12:49:46

by Matthias Brugger

[permalink] [raw]
Subject: Re: [PATCH 6/8] soc: mediatek: Add support for WAY_EN operations



On 05/01/2023 18:07, Markus Schneider-Pargmann wrote:
> From: Alexandre Bailon <[email protected]>
>
> This updates the power domain to support WAY_EN operations. These
> operations enable a path between different units of the chip and are
> labeled as 'way_en' in the register descriptions.
>
> This operation is required by the mt8365 for the MM power domain.
>
> Signed-off-by: Alexandre Bailon <[email protected]>
> Signed-off-by: Fabien Parent <[email protected]>
> Signed-off-by: Markus Schneider-Pargmann <[email protected]>
> ---
>
> Notes:
> Changes in v4:
> - Redesigned the patch to be better to understand and have less code
> duplication
>
> Changes in v3:
> - Separated the way_en functions for clarity
> - Added some checks for infracfg_nao
>
> Changes in v2:
> - some minor style fixes.
> - Renamed 'wayen' to 'way_en' to clarify the meaning
> - Updated commit message
>
> drivers/soc/mediatek/mtk-pm-domains.c | 57 ++++++++++++++++++++-------
> drivers/soc/mediatek/mtk-pm-domains.h | 16 ++++++--
> 2 files changed, 54 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/soc/mediatek/mtk-pm-domains.c b/drivers/soc/mediatek/mtk-pm-domains.c
> index 999e1f6c86b0..d53309f050ee 100644
> --- a/drivers/soc/mediatek/mtk-pm-domains.c
> +++ b/drivers/soc/mediatek/mtk-pm-domains.c
> @@ -43,6 +43,7 @@ struct scpsys_domain {
> struct clk_bulk_data *clks;
> int num_subsys_clks;
> struct clk_bulk_data *subsys_clks;
> + struct regmap *infracfg_nao;
> struct regmap *infracfg;
> struct regmap *smi;
> struct regulator *supply;
> @@ -118,10 +119,13 @@ static int scpsys_sram_disable(struct scpsys_domain *pd)
> }
>
> static int scpsys_bus_protect_clear(const struct scpsys_bus_prot_data *bpd,
> - struct regmap *regmap)
> + struct regmap *regmap,
> + struct regmap *sta_regmap)

I had a look at this and I don't like the idea to add another regmap that's used
based on flag in scpsys_bus_prot_data.

Up to now the assumption was that the different actions to enable/disable bus
protection were done using one regmap. For this reason the regmap was defined
once in scpsys_domain. It's not a good design to just pass more and more regmaps
or other members to the functions. We should try to find a better way to
structure the data so that it fulfills our needs.

Possible solutions I can think of right now:
a) Pass a struct of two regmaps to _scpsys_bus_protect_[disable,enable]()
We can pass the second regmap only if pd->way_en is set, otherwise it will be
ignored anyway.
It's less flexible if in the future we need another regmap. We will have two a
flag to mark we use way_en twice, in struct scpsys_domain and in struct
scpsys_domain_data. Please see my comment below. It's something else to think about.


b) Add a flag field to scpsys_bus_prot_data to say explicitely if we want to use
infracfg, smi with/without way_en. Then we can pass all three regmaps in a
struct, it will be easy to add new regmaps in the future.
Downsides:
1) We will overload even more the _BUS_PROT macros, which get's hard to read and
understand already. Maybe that can somehow be fixed. An idea would be by adding
a bit field to scpsys_bus_prot_data.
2) We will need to rework all the scpsys_domain_data structures for all SoCs
already supported. It's doable, but it's some extra work.

Any other ideas?

> {
> u32 val;
> u32 sta_mask = bpd->bus_prot_sta_mask;
> + /* way_en acknowledges successful clear with the bit being set */
> + u32 expected_ack = (bpd->way_en ? sta_mask : 0);
>
> if (bpd->bus_prot_reg_update)
> regmap_clear_bits(regmap, bpd->bus_prot_clr, bpd->bus_prot_set_clr_mask);
> @@ -131,13 +135,14 @@ static int scpsys_bus_protect_clear(const struct scpsys_bus_prot_data *bpd,
> if (bpd->ignore_clr_ack)
> return 0;
>
> - return regmap_read_poll_timeout(regmap, bpd->bus_prot_sta,
> - val, !(val & sta_mask),
> + return regmap_read_poll_timeout(sta_regmap, bpd->bus_prot_sta,
> + val, (val & sta_mask) == expected_ack,
> MTK_POLL_DELAY_US, MTK_POLL_TIMEOUT);
> }
>
> static int scpsys_bus_protect_set(const struct scpsys_bus_prot_data *bpd,
> - struct regmap *regmap)
> + struct regmap *regmap,
> + struct regmap *sta_regmap)
> {
> u32 val;
> u32 sta_mask = bpd->bus_prot_sta_mask;
> @@ -147,12 +152,13 @@ static int scpsys_bus_protect_set(const struct scpsys_bus_prot_data *bpd,
> else
> regmap_write(regmap, bpd->bus_prot_set, bpd->bus_prot_set_clr_mask);
>
> - return regmap_read_poll_timeout(regmap, bpd->bus_prot_sta,
> + return regmap_read_poll_timeout(sta_regmap, bpd->bus_prot_sta,
> val, (val & sta_mask) == sta_mask,
> MTK_POLL_DELAY_US, MTK_POLL_TIMEOUT);
> }
>
> -static int _scpsys_bus_protect_enable(const struct scpsys_bus_prot_data *bpd, struct regmap *regmap)
> +static int _scpsys_bus_protect_enable(const struct scpsys_bus_prot_data *bpd,
> + struct regmap *regmap, struct regmap *infracfg_nao)
> {
> int i, ret;
>
> @@ -160,9 +166,14 @@ static int _scpsys_bus_protect_enable(const struct scpsys_bus_prot_data *bpd, st
> if (!bpd[i].bus_prot_set_clr_mask)
> break;
>
> - ret = scpsys_bus_protect_set(&bpd[i], regmap);
> - if (ret)
> + if (bpd[i].way_en)
> + ret = scpsys_bus_protect_clear(&bpd[i], regmap, infracfg_nao);
> + else
> + ret = scpsys_bus_protect_set(&bpd[i], regmap, regmap);
> + if (ret) {
> + pr_err("%s %d %d\n", __PRETTY_FUNCTION__, __LINE__, ret);
> return ret;
> + }
> }
>
> return 0;
> @@ -172,15 +183,17 @@ static int scpsys_bus_protect_enable(struct scpsys_domain *pd)
> {
> int ret;
>
> - ret = _scpsys_bus_protect_enable(pd->data->bp_infracfg, pd->infracfg);
> + ret = _scpsys_bus_protect_enable(pd->data->bp_infracfg,
> + pd->infracfg, pd->infracfg_nao);
> if (ret)
> return ret;
>
> - return _scpsys_bus_protect_enable(pd->data->bp_smi, pd->smi);
> + return _scpsys_bus_protect_enable(pd->data->bp_smi, pd->smi, NULL);
> }
>
> static int _scpsys_bus_protect_disable(const struct scpsys_bus_prot_data *bpd,
> - struct regmap *regmap)
> + struct regmap *regmap,
> + struct regmap *infracfg_nao)
> {
> int i, ret;
>
> @@ -188,9 +201,14 @@ static int _scpsys_bus_protect_disable(const struct scpsys_bus_prot_data *bpd,
> if (!bpd[i].bus_prot_set_clr_mask)
> continue;
>
> - ret = scpsys_bus_protect_clear(&bpd[i], regmap);
> - if (ret)
> + if (bpd[i].way_en)
> + ret = scpsys_bus_protect_set(&bpd[i], regmap, infracfg_nao);
> + else
> + ret = scpsys_bus_protect_clear(&bpd[i], regmap, regmap);
> + if (ret) {
> + pr_err("%s %d %d\n", __PRETTY_FUNCTION__, __LINE__, ret);
> return ret;
> + }
> }
>
> return 0;
> @@ -200,11 +218,12 @@ static int scpsys_bus_protect_disable(struct scpsys_domain *pd)
> {
> int ret;
>
> - ret = _scpsys_bus_protect_disable(pd->data->bp_smi, pd->smi);
> + ret = _scpsys_bus_protect_disable(pd->data->bp_smi, pd->smi, NULL);
> if (ret)
> return ret;
>
> - return _scpsys_bus_protect_disable(pd->data->bp_infracfg, pd->infracfg);
> + return _scpsys_bus_protect_disable(pd->data->bp_infracfg,
> + pd->infracfg, pd->infracfg_nao);
> }
>
> static int scpsys_regulator_enable(struct regulator *supply)
> @@ -378,6 +397,14 @@ generic_pm_domain *scpsys_add_one_domain(struct scpsys *scpsys, struct device_no
> return ERR_CAST(pd->smi);
> }
>
> + pd->infracfg_nao = syscon_regmap_lookup_by_phandle(node, "mediatek,infracfg-nao");
> + if (IS_ERR(pd->infracfg_nao)) {
> + if (MTK_SCPD_CAPS(pd, MTK_SCPD_HAS_WAY_EN))
> + return ERR_CAST(pd->infracfg_nao);

I don't like the fact that we have a caps to show we have WAY_EN and then we
have a bool in scpsys_bus_prot_data to flag when we need to use the second
regmap and the expected_ack mask.
Can't we just make the lookup optional and let it all blow up if DTS is wrong
because no infracfg-nao was defined while there is at least one domain using way_en?

Regards,
Matthias

> +
> + pd->infracfg_nao = NULL;
> + }
> +
> num_clks = of_clk_get_parent_count(node);
> if (num_clks > 0) {
> /* Calculate number of subsys_clks */
> diff --git a/drivers/soc/mediatek/mtk-pm-domains.h b/drivers/soc/mediatek/mtk-pm-domains.h
> index 34642a279213..1fa634509db1 100644
> --- a/drivers/soc/mediatek/mtk-pm-domains.h
> +++ b/drivers/soc/mediatek/mtk-pm-domains.h
> @@ -10,6 +10,7 @@
> #define MTK_SCPD_DOMAIN_SUPPLY BIT(4)
> /* can't set MTK_SCPD_KEEP_DEFAULT_OFF at the same time */
> #define MTK_SCPD_ALWAYS_ON BIT(5)
> +#define MTK_SCPD_HAS_WAY_EN BIT(6)
> #define MTK_SCPD_CAPS(_scpd, _x) ((_scpd)->data->caps & (_x))
>
> #define SPM_VDE_PWR_CON 0x0210
> @@ -41,7 +42,7 @@
>
> #define SPM_MAX_BUS_PROT_DATA 6
>
> -#define _BUS_PROT(_set_clr_mask, _set, _clr, _sta_mask, _sta, _update, _ignore) { \
> +#define _BUS_PROT(_set_clr_mask, _set, _clr, _sta_mask, _sta, _update, _ignore, _way_en) { \
> .bus_prot_set_clr_mask = (_set_clr_mask), \
> .bus_prot_set = _set, \
> .bus_prot_clr = _clr, \
> @@ -49,16 +50,20 @@
> .bus_prot_sta = _sta, \
> .bus_prot_reg_update = _update, \
> .ignore_clr_ack = _ignore, \
> + .way_en = _way_en, \
> }
>
> #define BUS_PROT_WR(_mask, _set, _clr, _sta) \
> - _BUS_PROT(_mask, _set, _clr, _mask, _sta, false, false)
> + _BUS_PROT(_mask, _set, _clr, _mask, _sta, false, false, false)
>
> #define BUS_PROT_WR_IGN(_mask, _set, _clr, _sta) \
> - _BUS_PROT(_mask, _set, _clr, _mask, _sta, false, true)
> + _BUS_PROT(_mask, _set, _clr, _mask, _sta, false, true, false)
>
> #define BUS_PROT_UPDATE(_mask, _set, _clr, _sta) \
> - _BUS_PROT(_mask, _set, _clr, _mask, _sta, true, false)
> + _BUS_PROT(_mask, _set, _clr, _mask, _sta, true, false, false)
> +
> +#define BUS_PROT_WAY_EN(_set_mask, _set, _sta_mask, _sta) \
> + _BUS_PROT(_set_mask, _set, _set, _sta_mask, _sta, true, false, true)
>
> #define BUS_PROT_UPDATE_TOPAXI(_mask) \
> BUS_PROT_UPDATE(_mask, \
> @@ -77,6 +82,8 @@
> * write the whole register.
> * @ignore_clk_ack: Ignore the result in the status register for clear
> * operations.
> + * @way_en: Enable a way in the SoC. For this type the status bit to acknowledge
> + * the change is always 1 on successful change.
> */
> struct scpsys_bus_prot_data {
> u32 bus_prot_set_clr_mask;
> @@ -86,6 +93,7 @@ struct scpsys_bus_prot_data {
> u32 bus_prot_sta;
> bool bus_prot_reg_update;
> bool ignore_clr_ack;
> + bool way_en;
> };
>
> /**

Subject: Re: [PATCH 8/8] soc: mediatek: pm-domains: Add support for MT8365

Hi Matthias,

On Fri, Feb 03, 2023 at 01:22:38PM +0100, Matthias Brugger wrote:
>
>
> On 05/01/2023 18:07, Markus Schneider-Pargmann wrote:
> > From: Fabien Parent <[email protected]>
> >
> > Add the needed board data to support MT8365 SoC.
> >
> > Signed-off-by: Fabien Parent <[email protected]>
> > Signed-off-by: Markus Schneider-Pargmann <[email protected]>
> > ---
> > drivers/soc/mediatek/mt8365-pm-domains.h | 147 +++++++++++++++++++++++
> > drivers/soc/mediatek/mtk-pm-domains.c | 5 +
> > 2 files changed, 152 insertions(+)
> > create mode 100644 drivers/soc/mediatek/mt8365-pm-domains.h
> >
> > diff --git a/drivers/soc/mediatek/mt8365-pm-domains.h b/drivers/soc/mediatek/mt8365-pm-domains.h
> > new file mode 100644
> > index 000000000000..8735e833b15b
> > --- /dev/null
> > +++ b/drivers/soc/mediatek/mt8365-pm-domains.h
> > @@ -0,0 +1,147 @@
> > +/* SPDX-License-Identifier: GPL-2.0-only */
> > +
> > +#ifndef __SOC_MEDIATEK_MT8365_PM_DOMAINS_H
> > +#define __SOC_MEDIATEK_MT8365_PM_DOMAINS_H
> > +
> > +#include "mtk-pm-domains.h"
> > +#include <dt-bindings/power/mediatek,mt8365-power.h>
> > +
> > +/*
> > + * MT8365 power domain support
> > + */
> > +
> > +static const struct scpsys_domain_data scpsys_domain_data_mt8365[] = {
> > + [MT8365_POWER_DOMAIN_MM] = {
> > + .name = "mm",
> > + .sta_mask = PWR_STATUS_DISP,
> > + .ctl_offs = 0x30c,
> > + .pwr_sta_offs = 0x0180,
> > + .pwr_sta2nd_offs = 0x0184,
> > + .sram_pdn_bits = GENMASK(8, 8),
> > + .sram_pdn_ack_bits = GENMASK(12, 12),
> > + .caps = MTK_SCPD_STRICT_BUS_PROTECTION | MTK_SCPD_HAS_WAY_EN,
> > + .bp_infracfg = {
> > + BUS_PROT_WR(BIT(16) | BIT(17), 0x2a8, 0x2ac, 0x258),
> > + BUS_PROT_WR(BIT(1) | BIT(2) | BIT(10) | BIT(11), 0x2a0, 0x2a4, 0x228),
> > + BUS_PROT_WAY_EN(BIT(6), 0x200, BIT(24), 0x0),
> > + BUS_PROT_WAY_EN(BIT(5), 0x234, BIT(14), 0x28),
> > + BUS_PROT_WR(BIT(6), 0x2a0, 0x2a4, 0x228),
>
>
> BUS_PROT_WR(BIT(6), 0x2a0, 0x2a4, 0x228) repeates several times in the
> definition. Would it make sense to create a new define like we did with
> BUS_PROT_UPDATE_TOPAXI()? Are this offests are used in other SoCs.
>
> In any case instead of magic numbers the values should be defined in
> include/linux/soc/mediatek/infracfg.h or appropiate header files.

Thanks, you are right, I got rid of all the magic numbers and introduced
some helper defines as well.

Thank you,
Markus

Subject: Re: [PATCH 3/8] soc: mediatek: pm-domains: Create bus protection operation functions

Hi Matthias,

On Fri, Feb 03, 2023 at 01:32:11PM +0100, Matthias Brugger wrote:
>
>
> On 05/01/2023 18:07, Markus Schneider-Pargmann wrote:
> > Separate the register access used for bus protection enable/disable into
> > their own functions. These will be used later for WAY_EN bits.
> >
> > Signed-off-by: Markus Schneider-Pargmann <[email protected]>
> > ---
> > drivers/soc/mediatek/mtk-pm-domains.c | 68 +++++++++++++++------------
> > 1 file changed, 39 insertions(+), 29 deletions(-)
> >
> > diff --git a/drivers/soc/mediatek/mtk-pm-domains.c b/drivers/soc/mediatek/mtk-pm-domains.c
> > index 4333cd297405..999e1f6c86b0 100644
> > --- a/drivers/soc/mediatek/mtk-pm-domains.c
> > +++ b/drivers/soc/mediatek/mtk-pm-domains.c
> > @@ -117,26 +117,50 @@ static int scpsys_sram_disable(struct scpsys_domain *pd)
> > MTK_POLL_TIMEOUT);
> > }
> > +static int scpsys_bus_protect_clear(const struct scpsys_bus_prot_data *bpd,
> > + struct regmap *regmap)
> > +{
> > + u32 val;
> > + u32 sta_mask = bpd->bus_prot_sta_mask;
> > +
> > + if (bpd->bus_prot_reg_update)
> > + regmap_clear_bits(regmap, bpd->bus_prot_clr, bpd->bus_prot_set_clr_mask);
> > + else
> > + regmap_write(regmap, bpd->bus_prot_clr, bpd->bus_prot_set_clr_mask);
> > +
> > + if (bpd->ignore_clr_ack)
> > + return 0;
> > +
> > + return regmap_read_poll_timeout(regmap, bpd->bus_prot_sta,
> > + val, !(val & sta_mask),
> > + MTK_POLL_DELAY_US, MTK_POLL_TIMEOUT);
> > +}
> > +
>
> NIT: please move this function below
> static int scpsys_bus_protect_enable(struct scpsys_domain *pd) {...}
>
> This allows for better readability.

With the WAY_EN patch later in this series scpsys_bus_protect_enable()
is going to use scpsys_bus_protect_clear(). So I would need to add a
signature in that patch for scpsys_bus_protect_clear() or move it back
up. I would prefer to avoid adding a signature or moving so I added it
above these functions here. I could swap scpsys_bus_protect_set() and
scpscpsys_bus_protect_clear() if that helps readability.

Thank you,
Markus

>
> > +static int scpsys_bus_protect_set(const struct scpsys_bus_prot_data *bpd,
> > + struct regmap *regmap)
> > +{
> > + u32 val;
> > + u32 sta_mask = bpd->bus_prot_sta_mask;
> > +
> > + if (bpd->bus_prot_reg_update)
> > + regmap_set_bits(regmap, bpd->bus_prot_set, bpd->bus_prot_set_clr_mask);
> > + else
> > + regmap_write(regmap, bpd->bus_prot_set, bpd->bus_prot_set_clr_mask);
> > +
> > + return regmap_read_poll_timeout(regmap, bpd->bus_prot_sta,
> > + val, (val & sta_mask) == sta_mask,
> > + MTK_POLL_DELAY_US, MTK_POLL_TIMEOUT);
> > +}
> > +
> > static int _scpsys_bus_protect_enable(const struct scpsys_bus_prot_data *bpd, struct regmap *regmap)
> > {
> > int i, ret;
> > for (i = 0; i < SPM_MAX_BUS_PROT_DATA; i++) {
> > - u32 val;
> > - u32 set_clr_mask = bpd[i].bus_prot_set_clr_mask;
> > - u32 sta_mask = bpd[i].bus_prot_sta_mask;
> > -
> > - if (!set_clr_mask)
> > + if (!bpd[i].bus_prot_set_clr_mask)
> > break;
> > - if (bpd[i].bus_prot_reg_update)
> > - regmap_set_bits(regmap, bpd[i].bus_prot_set, set_clr_mask);
> > - else
> > - regmap_write(regmap, bpd[i].bus_prot_set, set_clr_mask);
> > -
> > - ret = regmap_read_poll_timeout(regmap, bpd[i].bus_prot_sta,
> > - val, (val & sta_mask) == sta_mask,
> > - MTK_POLL_DELAY_US, MTK_POLL_TIMEOUT);
> > + ret = scpsys_bus_protect_set(&bpd[i], regmap);
> > if (ret)
> > return ret;
> > }
> > @@ -161,24 +185,10 @@ static int _scpsys_bus_protect_disable(const struct scpsys_bus_prot_data *bpd,
> > int i, ret;
> > for (i = SPM_MAX_BUS_PROT_DATA - 1; i >= 0; i--) {
> > - u32 val;
> > - u32 set_clr_mask = bpd[i].bus_prot_set_clr_mask;
> > - u32 sta_mask = bpd[i].bus_prot_sta_mask;
> > -
> > - if (!set_clr_mask)
> > - continue;
> > -
> > - if (bpd[i].bus_prot_reg_update)
> > - regmap_clear_bits(regmap, bpd[i].bus_prot_clr, set_clr_mask);
> > - else
> > - regmap_write(regmap, bpd[i].bus_prot_clr, set_clr_mask);
> > -
> > - if (bpd[i].ignore_clr_ack)
> > + if (!bpd[i].bus_prot_set_clr_mask)
> > continue;
> > - ret = regmap_read_poll_timeout(regmap, bpd[i].bus_prot_sta,
> > - val, !(val & sta_mask),
> > - MTK_POLL_DELAY_US, MTK_POLL_TIMEOUT);
> > + ret = scpsys_bus_protect_clear(&bpd[i], regmap);
> > if (ret)
> > return ret;
> > }