In preparation to add support for new generation SoCs like MT8196,
MT6991 and other variants, which require to set bus protection on
different busses than the ones found on legacy chips, and to also
simplify and reduce memory footprint of this driver, refactor the
mechanism to retrieve and use the bus protection regmaps.

This is done by removing the three pointers to struct regmap from
struct scpsys_domain (allocated for each power domain) and moving
them to the main struct scpsys (allocated per driver instance) as
an array of pointers to regmap named **bus_prot.

That deprecates the old devicetree properties to grab phandles to
the three predefined busses (infracfg, infracfg-nao and smi) and
replaces it with the base property "access-controllers" that is
meant to be an array of phandles holding the same busses where
required (for now - for legacy SoCs).

The new bus protection phandles are indexed by the bus_prot_index
member of struct scpsys, used to map "bus type" (ex.: infra, smi,
etc) to the specific *bus_prot[x] element.

While the old per-power-domain regmap pointers were removed, the
support for old devicetree was retained by still checking if the
new property (in DT) and new-style declaration (in SoC specific
platform data) are both present at probe time.

If those are not present, a lookup for the old properties will be
done in all of the children of the power controller, and pointers
to regmaps will be retrieved with the old properties, but then
will be internally remapped to follow the new style regmap anyway
as to let this driver benefit of the memory footprint reduction.

Finally, it was necessary to change macros in mtk-pm-domains.h and
in mt8365-pm-domains.h to make use of the new style bus protection
declaration, as the actual HW block is now recognized not by flags
but by its own scpsys_bus_prot_block enumeration.

The BUS_PROT_(STA)_COMPONENT_{INFRA,INFRA_NAO,SMI} flags were also
removed since they are now unused, and because that enumeration was
initially meant to vary the logic of bus protection and not the bus
where work is performed, anyway!

Reviewed-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
 drivers/pmdomain/mediatek/mt8365-pm-domains.h |   8 +-
 drivers/pmdomain/mediatek/mtk-pm-domains.c    | 188 ++++++++++++++----
 drivers/pmdomain/mediatek/mtk-pm-domains.h    |  53 +++--
 3 files changed, 187 insertions(+), 62 deletions(-)

diff --git a/drivers/pmdomain/mediatek/mt8365-pm-domains.h b/drivers/pmdomain/mediatek/mt8365-pm-domains.h
index 3d83d49eaa7c..6fbd5ef8d672 100644
--- a/drivers/pmdomain/mediatek/mt8365-pm-domains.h
+++ b/drivers/pmdomain/mediatek/mt8365-pm-domains.h
@@ -29,11 +29,9 @@
 			    MT8365_SMI_COMMON_CLAMP_EN)
 
 #define MT8365_BUS_PROT_WAY_EN(_set_mask, _set, _sta_mask, _sta)	\
-		_BUS_PROT(_set_mask, _set, _set, _sta_mask, _sta,	\
-			  BUS_PROT_COMPONENT_INFRA |			\
-			  BUS_PROT_STA_COMPONENT_INFRA_NAO |		\
-			  BUS_PROT_INVERTED |				\
-			  BUS_PROT_REG_UPDATE)
+		_BUS_PROT_STA(INFRA, INFRA_NAO, _set_mask, _set, _set,	\
+			      _sta_mask, _sta,				\
+			      BUS_PROT_INVERTED | BUS_PROT_REG_UPDATE)
 
 static const struct scpsys_domain_data scpsys_domain_data_mt8365[] = {
 	[MT8365_POWER_DOMAIN_MM] = {
diff --git a/drivers/pmdomain/mediatek/mtk-pm-domains.c b/drivers/pmdomain/mediatek/mtk-pm-domains.c
index a58ed7e2d9a4..48dc5f188438 100644
--- a/drivers/pmdomain/mediatek/mtk-pm-domains.c
+++ b/drivers/pmdomain/mediatek/mtk-pm-domains.c
@@ -47,9 +47,6 @@ 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;
 };
 
@@ -57,6 +54,8 @@ struct scpsys {
 	struct device *dev;
 	struct regmap *base;
 	const struct scpsys_soc_data *soc_data;
+	u8 bus_prot_index[BUS_PROT_BLOCK_COUNT];
+	struct regmap **bus_prot;
 	struct genpd_onecell_data pd_data;
 	struct generic_pm_domain *domains[];
 };
@@ -125,19 +124,19 @@ static int scpsys_sram_disable(struct scpsys_domain *pd)
 static struct regmap *scpsys_bus_protect_get_regmap(struct scpsys_domain *pd,
 						    const struct scpsys_bus_prot_data *bpd)
 {
-	if (bpd->flags & BUS_PROT_COMPONENT_SMI)
-		return pd->smi;
-	else
-		return pd->infracfg;
+	struct scpsys *scpsys = pd->scpsys;
+	unsigned short block_idx = scpsys->bus_prot_index[bpd->bus_prot_block];
+
+	return scpsys->bus_prot[block_idx];
 }
 
 static struct regmap *scpsys_bus_protect_get_sta_regmap(struct scpsys_domain *pd,
 							const struct scpsys_bus_prot_data *bpd)
 {
-	if (bpd->flags & BUS_PROT_STA_COMPONENT_INFRA_NAO)
-		return pd->infracfg_nao;
-	else
-		return scpsys_bus_protect_get_regmap(pd, bpd);
+	struct scpsys *scpsys = pd->scpsys;
+	int block_idx = scpsys->bus_prot_index[bpd->bus_prot_sta_block];
+
+	return scpsys->bus_prot[block_idx];
 }
 
 static int scpsys_bus_protect_clear(struct scpsys_domain *pd,
@@ -149,7 +148,7 @@ static int scpsys_bus_protect_clear(struct scpsys_domain *pd,
 	u32 expected_ack;
 	u32 val;
 
-	expected_ack = (bpd->flags & BUS_PROT_STA_COMPONENT_INFRA_NAO ? sta_mask : 0);
+	expected_ack = (bpd->bus_prot_sta_block == BUS_PROT_BLOCK_INFRA_NAO ? sta_mask : 0);
 
 	if (bpd->flags & BUS_PROT_REG_UPDATE)
 		regmap_clear_bits(regmap, bpd->bus_prot_clr, bpd->bus_prot_set_clr_mask);
@@ -355,7 +354,6 @@ generic_pm_domain *scpsys_add_one_domain(struct scpsys *scpsys, struct device_no
 {
 	const struct scpsys_domain_data *domain_data;
 	struct scpsys_domain *pd;
-	struct device_node *smi_node;
 	struct property *prop;
 	const char *clk_name;
 	int i, ret, num_clks;
@@ -396,32 +394,6 @@ generic_pm_domain *scpsys_add_one_domain(struct scpsys *scpsys, struct device_no
 				      node);
 	}
 
-	pd->infracfg = syscon_regmap_lookup_by_phandle_optional(node, "mediatek,infracfg");
-	if (IS_ERR(pd->infracfg))
-		return dev_err_cast_probe(scpsys->dev, pd->infracfg,
-					  "%pOF: failed to get infracfg regmap\n",
-					  node);
-
-	smi_node = of_parse_phandle(node, "mediatek,smi", 0);
-	if (smi_node) {
-		pd->smi = device_node_to_regmap(smi_node);
-		of_node_put(smi_node);
-		if (IS_ERR(pd->smi))
-			return dev_err_cast_probe(scpsys->dev, pd->smi,
-						  "%pOF: failed to get SMI regmap\n",
-						  node);
-	}
-
-	if (MTK_SCPD_CAPS(pd, MTK_SCPD_HAS_INFRA_NAO)) {
-		pd->infracfg_nao = syscon_regmap_lookup_by_phandle(node, "mediatek,infracfg-nao");
-		if (IS_ERR(pd->infracfg_nao))
-			return dev_err_cast_probe(scpsys->dev, pd->infracfg_nao,
-						  "%pOF: failed to get infracfg-nao regmap\n",
-						  node);
-	} else {
-		pd->infracfg_nao = NULL;
-	}
-
 	num_clks = of_clk_get_parent_count(node);
 	if (num_clks > 0) {
 		/* Calculate number of subsys_clks */
@@ -615,6 +587,136 @@ static void scpsys_domain_cleanup(struct scpsys *scpsys)
 	}
 }
 
+static int scpsys_get_bus_protection_legacy(struct device *dev, struct scpsys *scpsys)
+{
+	const u8 bp_blocks[3] = {
+		BUS_PROT_BLOCK_INFRA, BUS_PROT_BLOCK_SMI, BUS_PROT_BLOCK_INFRA_NAO
+	};
+	struct device_node *np = dev->of_node;
+	struct device_node *node, *smi_np;
+	int num_regmaps = 0, i, j;
+	struct regmap *regmap[3];
+
+	/*
+	 * Legacy code retrieves a maximum of three bus protection handles:
+	 * some may be optional, or may not be, so the array of bp blocks
+	 * that is normally passed in as platform data must be dynamically
+	 * built in this case.
+	 *
+	 * Here, try to retrieve all of the regmaps that the legacy code
+	 * supported and then count the number of the ones that are present,
+	 * this makes it then possible to allocate the array of bus_prot
+	 * regmaps and convert all to the new style handling.
+	 */
+	node = of_find_node_with_property(np, "mediatek,infracfg");
+	if (node) {
+		regmap[0] = syscon_regmap_lookup_by_phandle(node, "mediatek,infracfg");
+		of_node_put(node);
+		num_regmaps++;
+		if (IS_ERR(regmap[0]))
+			return dev_err_probe(dev, PTR_ERR(regmap[0]),
+					     "%pOF: failed to get infracfg regmap\n",
+					     node);
+	} else {
+		regmap[0] = NULL;
+	}
+
+	node = of_find_node_with_property(np, "mediatek,smi");
+	if (node) {
+		smi_np = of_parse_phandle(node, "mediatek,smi", 0);
+		of_node_put(node);
+		if (!smi_np)
+			return -ENODEV;
+
+		regmap[1] = device_node_to_regmap(smi_np);
+		num_regmaps++;
+		of_node_put(smi_np);
+		if (IS_ERR(regmap[1]))
+			return dev_err_probe(dev, PTR_ERR(regmap[1]),
+					     "%pOF: failed to get SMI regmap\n",
+					     node);
+	} else {
+		regmap[1] = NULL;
+	}
+
+	node = of_find_node_with_property(np, "mediatek,infracfg-nao");
+	if (node) {
+		regmap[2] = syscon_regmap_lookup_by_phandle(node, "mediatek,infracfg-nao");
+		num_regmaps++;
+		of_node_put(node);
+		if (IS_ERR(regmap[2]))
+			return dev_err_probe(dev, PTR_ERR(regmap[2]),
+					     "%pOF: failed to get infracfg regmap\n",
+					     node);
+	} else {
+		regmap[2] = NULL;
+	}
+
+	scpsys->bus_prot = devm_kmalloc_array(dev, num_regmaps,
+					      sizeof(*scpsys->bus_prot), GFP_KERNEL);
+	if (!scpsys->bus_prot)
+		return -ENOMEM;
+
+	for (i = 0, j = 0; i < ARRAY_SIZE(bp_blocks); i++) {
+		enum scpsys_bus_prot_block bp_type;
+
+		if (!regmap[i])
+			continue;
+
+		bp_type = bp_blocks[i];
+		scpsys->bus_prot_index[bp_type] = j;
+		scpsys->bus_prot[j] = regmap[i];
+
+		j++;
+	}
+
+	return 0;
+}
+
+static int scpsys_get_bus_protection(struct device *dev, struct scpsys *scpsys)
+{
+	const struct scpsys_soc_data *soc = scpsys->soc_data;
+	struct device_node *np = dev->of_node;
+	int i, num_handles;
+
+	num_handles = of_count_phandle_with_args(np, "access-controllers", NULL);
+	if (num_handles < 0 || num_handles != soc->num_bus_prot_blocks)
+		return dev_err_probe(dev, -EINVAL,
+				     "Cannot get access controllers: expected %u, got %d\n",
+				     soc->num_bus_prot_blocks, num_handles);
+
+	scpsys->bus_prot = devm_kmalloc_array(dev, soc->num_bus_prot_blocks,
+					      sizeof(*scpsys->bus_prot), GFP_KERNEL);
+	if (!scpsys->bus_prot)
+		return -ENOMEM;
+
+	for (i = 0; i < soc->num_bus_prot_blocks; i++) {
+		enum scpsys_bus_prot_block bp_type;
+		struct device_node *node;
+
+		node = of_parse_phandle(np, "access-controllers", i);
+		if (!node)
+			return -EINVAL;
+
+		/*
+		 * Index the bus protection regmaps so that we don't have to
+		 * find the right one by type with a loop at every execution
+		 * of power sequence(s).
+		 */
+		bp_type = soc->bus_prot_blocks[i];
+		scpsys->bus_prot_index[bp_type] = i;
+
+		scpsys->bus_prot[i] = device_node_to_regmap(node);
+		of_node_put(node);
+		if (IS_ERR_OR_NULL(scpsys->bus_prot[i]))
+			return dev_err_probe(dev, scpsys->bus_prot[i] ?
+					     PTR_ERR(scpsys->bus_prot[i]) : -ENXIO,
+					     "Cannot get regmap for access controller %d\n", i);
+	}
+
+	return 0;
+}
+
 static const struct of_device_id scpsys_of_match[] = {
 	{
 		.compatible = "mediatek,mt6735-power-controller",
@@ -701,6 +803,14 @@ static int scpsys_probe(struct platform_device *pdev)
 		return PTR_ERR(scpsys->base);
 	}
 
+	if (of_find_property(np, "access-controllers", NULL))
+		ret = scpsys_get_bus_protection(dev, scpsys);
+	else
+		ret = scpsys_get_bus_protection_legacy(dev, scpsys);
+
+	if (ret)
+		return ret;
+
 	ret = -ENODEV;
 	for_each_available_child_of_node(np, node) {
 		struct generic_pm_domain *domain;
diff --git a/drivers/pmdomain/mediatek/mtk-pm-domains.h b/drivers/pmdomain/mediatek/mtk-pm-domains.h
index 7085fa2976e9..4f2d331a866a 100644
--- a/drivers/pmdomain/mediatek/mtk-pm-domains.h
+++ b/drivers/pmdomain/mediatek/mtk-pm-domains.h
@@ -50,30 +50,43 @@ enum scpsys_bus_prot_flags {
 	BUS_PROT_REG_UPDATE = BIT(1),
 	BUS_PROT_IGNORE_CLR_ACK = BIT(2),
 	BUS_PROT_INVERTED = BIT(3),
-	BUS_PROT_COMPONENT_INFRA = BIT(4),
-	BUS_PROT_COMPONENT_SMI = BIT(5),
-	BUS_PROT_STA_COMPONENT_INFRA_NAO = BIT(6),
 };
 
-#define _BUS_PROT(_set_clr_mask, _set, _clr, _sta_mask, _sta, _flags) {	\
-		.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,				\
-		.flags = _flags					\
+enum scpsys_bus_prot_block {
+	BUS_PROT_BLOCK_INFRA,
+	BUS_PROT_BLOCK_INFRA_NAO,
+	BUS_PROT_BLOCK_SMI,
+	BUS_PROT_BLOCK_COUNT,
+};
+
+#define _BUS_PROT_STA(_hwip, _sta_hwip, _set_clr_mask, _set, _clr,	\
+		      _sta_mask, _sta, _flags)				\
+	{								\
+		.bus_prot_block = BUS_PROT_BLOCK_##_hwip,		\
+		.bus_prot_sta_block = BUS_PROT_BLOCK_##_sta_hwip,	\
+		.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,					\
+		.flags = _flags						\
 	}
 
-#define BUS_PROT_WR(_hwip, _mask, _set, _clr, _sta) \
-		_BUS_PROT(_mask, _set, _clr, _mask, _sta, BUS_PROT_COMPONENT_##_hwip)
+#define _BUS_PROT(_hwip, _set_clr_mask, _set, _clr, _sta_mask,		\
+		  _sta, _flags)						\
+	_BUS_PROT_STA(_hwip, _hwip, _set_clr_mask, _set, _clr,		\
+		      _sta_mask, _sta, _flags)
+
+#define BUS_PROT_WR(_hwip, _mask, _set, _clr, _sta)			\
+		_BUS_PROT(_hwip, _mask, _set, _clr,  _mask, _sta, 0)
 
-#define BUS_PROT_WR_IGN(_hwip, _mask, _set, _clr, _sta) \
-		_BUS_PROT(_mask, _set, _clr, _mask, _sta, \
-			  BUS_PROT_COMPONENT_##_hwip | BUS_PROT_IGNORE_CLR_ACK)
+#define BUS_PROT_WR_IGN(_hwip, _mask, _set, _clr, _sta)		\
+		_BUS_PROT(_hwip, _mask, _set, _clr, _mask, _sta,	\
+			  BUS_PROT_IGNORE_CLR_ACK)
 
-#define BUS_PROT_UPDATE(_hwip, _mask, _set, _clr, _sta) \
-		_BUS_PROT(_mask, _set, _clr, _mask, _sta, \
-			  BUS_PROT_COMPONENT_##_hwip | BUS_PROT_REG_UPDATE)
+#define BUS_PROT_UPDATE(_hwip, _mask, _set, _clr, _sta)			\
+		_BUS_PROT(_hwip, _mask, _set, _clr, _mask, _sta,	\
+			  BUS_PROT_REG_UPDATE)
 
 #define BUS_PROT_INFRA_UPDATE_TOPAXI(_mask)			\
 		BUS_PROT_UPDATE(INFRA, _mask,			\
@@ -82,6 +95,8 @@ enum scpsys_bus_prot_flags {
 				INFRA_TOPAXI_PROTECTSTA1)
 
 struct scpsys_bus_prot_data {
+	u8 bus_prot_block;
+	u8 bus_prot_sta_block;
 	u32 bus_prot_set_clr_mask;
 	u32 bus_prot_set;
 	u32 bus_prot_clr;
@@ -119,6 +134,8 @@ struct scpsys_domain_data {
 struct scpsys_soc_data {
 	const struct scpsys_domain_data *domains_data;
 	int num_domains;
+	enum scpsys_bus_prot_block *bus_prot_blocks;
+	int num_bus_prot_blocks;
 };
 
 #endif /* __SOC_MEDIATEK_MTK_PM_DOMAINS_H */
-- 
2.50.1

Hey,

On Tue, 2025-08-05 at 09:47 +0200, AngeloGioacchino Del Regno wrote:
> In preparation to add support for new generation SoCs like MT8196,
> MT6991 and other variants, which require to set bus protection on
> different busses than the ones found on legacy chips, and to also
> simplify and reduce memory footprint of this driver, refactor the
> mechanism to retrieve and use the bus protection regmaps.
> 
> This is done by removing the three pointers to struct regmap from
> struct scpsys_domain (allocated for each power domain) and moving
> them to the main struct scpsys (allocated per driver instance) as
> an array of pointers to regmap named **bus_prot.

Trying to boot v6.18.0-rc1 on a Genio 700 EVK using the arm64 defconfig,
ends up hanging at boot (seemingly when probing MTU3 and/or mmc, but that 
might be a red herring). 

Either reverting this patch *or* having CONFIG_MTK_MMSYS builtin rather
then a module seems to solve that. 

-- 
Sjoerd Simons <sjoerd@collabora.com>

Il 13/10/25 15:41, Sjoerd Simons ha scritto:
> Hey,
> 
> On Tue, 2025-08-05 at 09:47 +0200, AngeloGioacchino Del Regno wrote:
>> In preparation to add support for new generation SoCs like MT8196,
>> MT6991 and other variants, which require to set bus protection on
>> different busses than the ones found on legacy chips, and to also
>> simplify and reduce memory footprint of this driver, refactor the
>> mechanism to retrieve and use the bus protection regmaps.
>>
>> This is done by removing the three pointers to struct regmap from
>> struct scpsys_domain (allocated for each power domain) and moving
>> them to the main struct scpsys (allocated per driver instance) as
>> an array of pointers to regmap named **bus_prot.
> 
> Trying to boot v6.18.0-rc1 on a Genio 700 EVK using the arm64 defconfig,
> ends up hanging at boot (seemingly when probing MTU3 and/or mmc, but that
> might be a red herring).
> 
> Either reverting this patch *or* having CONFIG_MTK_MMSYS builtin rather
> then a module seems to solve that.
> 

Thanks for the report.

This is not a problem with this patch specifically, but surely some race condition
that was already present before and that does get uncovered with this one in some
conditions.

Without the devicetree updates (which are not upstream yet) this patch is
fully retaining the legacy functionality 1-to-1.

I'll check what's going on ASAP.

Cheers,
Angelo

On Tue, 2025-10-14 at 11:59 +0200, AngeloGioacchino Del Regno wrote:
> 
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
> 
> 
> Il 13/10/25 15:41, Sjoerd Simons ha scritto:
> > Hey,
> > 
> > On Tue, 2025-08-05 at 09:47 +0200, AngeloGioacchino Del Regno
> > wrote:
> > > In preparation to add support for new generation SoCs like
> > > MT8196,
> > > MT6991 and other variants, which require to set bus protection on
> > > different busses than the ones found on legacy chips, and to also
> > > simplify and reduce memory footprint of this driver, refactor the
> > > mechanism to retrieve and use the bus protection regmaps.
> > > 
> > > This is done by removing the three pointers to struct regmap from
> > > struct scpsys_domain (allocated for each power domain) and moving
> > > them to the main struct scpsys (allocated per driver instance) as
> > > an array of pointers to regmap named **bus_prot.
> > 
> > Trying to boot v6.18.0-rc1 on a Genio 700 EVK using the arm64
> > defconfig,
> > ends up hanging at boot (seemingly when probing MTU3 and/or mmc,
> > but that
> > might be a red herring).
> > 
> > Either reverting this patch *or* having CONFIG_MTK_MMSYS builtin
> > rather
> > then a module seems to solve that.
> > 
> 
> Thanks for the report.
> 
> This is not a problem with this patch specifically, but surely some
> race condition
> that was already present before and that does get uncovered with this
> one in some
> conditions.
> 
> Without the devicetree updates (which are not upstream yet) this
> patch is
> fully retaining the legacy functionality 1-to-1.
> 
> I'll check what's going on ASAP.
> 
> Cheers,
> Angelo
> 

I did a git bisect on linux-next master branch and
the result shows c29345fa5f66bea0790cf2219f57b974d4fc177b is the first
bad commit. This change also affect MT8195.

This patch couldn't be simply reverted because there are some dependent
commits follows this change.
I'm not sure it this refactor causes API or flag not synced with the
SCP firmware.
Just a remind that it is hard for MediaTek to update scp firmware for
an already in mass production state chip.
Each scp firmware and the pm-domain interface are designed specifically
for 'that' chip only. So it is difficult to adopt pm-domain refactor
by only reviewing the patch.

Here are the error logs with latest linux-next master on mt8395-genio-
1200-evk. Hope this could help on futher debugging.

[    1.291055] mtk-msdc 11240000.mmc: msdc_track_cmd_data: cmd=8
arg=000001AA; host->error=0x00000002
[    1.292775] mtk-msdc 11240000.mmc: msdc_track_cmd_data: cmd=55
arg=00000000; host->error=0x00000002
[    1.294539] mtk-msdc 11240000.mmc: msdc_track_cmd_data: cmd=55
arg=00000000; host->error=0x00000002
[    1.296293] mtk-msdc 11240000.mmc: msdc_track_cmd_data: cmd=55
arg=00000000; host->error=0x00000002
...
[    1.430408] mtk-msdc 11240000.mmc: msdc_track_cmd_data: cmd=55
arg=00000000; host->error=0x00000002
[    1.433766] mmc0: Failed to initialize a non-removable card
[   22.297240] rcu: INFO: rcu_preempt detected stalls on CPUs/tasks:
[   22.298723] rcu:     6-...0: (2 ticks this GP)
idle=104c/1/0x4000000000000000 softirq=45/45 fqs=37
[   22.299827] rcu:     (detected by 2, t=5256 jiffies, g=-1051, q=200
ncpus=8)
[   22.300689] Sending NMI from CPU 2 to CPUs 6:

Best regards,
Macpaul Lin

Il 29/10/25 05:11, Macpaul Lin (林智斌) ha scritto:
> On Tue, 2025-10-14 at 11:59 +0200, AngeloGioacchino Del Regno wrote:
>>
>> External email : Please do not click links or open attachments until
>> you have verified the sender or the content.
>>
>>
>> Il 13/10/25 15:41, Sjoerd Simons ha scritto:
>>> Hey,
>>>
>>> On Tue, 2025-08-05 at 09:47 +0200, AngeloGioacchino Del Regno
>>> wrote:
>>>> In preparation to add support for new generation SoCs like
>>>> MT8196,
>>>> MT6991 and other variants, which require to set bus protection on
>>>> different busses than the ones found on legacy chips, and to also
>>>> simplify and reduce memory footprint of this driver, refactor the
>>>> mechanism to retrieve and use the bus protection regmaps.
>>>>
>>>> This is done by removing the three pointers to struct regmap from
>>>> struct scpsys_domain (allocated for each power domain) and moving
>>>> them to the main struct scpsys (allocated per driver instance) as
>>>> an array of pointers to regmap named **bus_prot.
>>>
>>> Trying to boot v6.18.0-rc1 on a Genio 700 EVK using the arm64
>>> defconfig,
>>> ends up hanging at boot (seemingly when probing MTU3 and/or mmc,
>>> but that
>>> might be a red herring).
>>>
>>> Either reverting this patch *or* having CONFIG_MTK_MMSYS builtin
>>> rather
>>> then a module seems to solve that.
>>>
>>
>> Thanks for the report.
>>
>> This is not a problem with this patch specifically, but surely some
>> race condition
>> that was already present before and that does get uncovered with this
>> one in some
>> conditions.
>>
>> Without the devicetree updates (which are not upstream yet) this
>> patch is
>> fully retaining the legacy functionality 1-to-1.
>>
>> I'll check what's going on ASAP.
>>
>> Cheers,
>> Angelo
>>
> 

Hello Macpaul,

> I did a git bisect on linux-next master branch and
> the result shows c29345fa5f66bea0790cf2219f57b974d4fc177b is the first
> bad commit. This change also affect MT8195.
> 
> This patch couldn't be simply reverted because there are some dependent
> commits follows this change.
> I'm not sure it this refactor causes API or flag not synced with the
> SCP firmware.

There's no interaction with the SCP on this patch.

> Just a remind that it is hard for MediaTek to update scp firmware for
> an already in mass production state chip.
> Each scp firmware and the pm-domain interface are designed specifically
> for 'that' chip only. So it is difficult to adopt pm-domain refactor
> by only reviewing the patch.

There's no need to update the SCP firmware at all - the pmdomain driver
does not communicate with that - infact, you can keep using all of the
pmdomains without ever loading any SCP firmware from the kernel.

> 
> Here are the error logs with latest linux-next master on mt8395-genio-
> 1200-evk. Hope this could help on futher debugging.
> 

Thanks for the logs! Getting issues on MSDC is really odd because the MSDC
controller does *not* use nor have any power domain...

I'll take a look at this as soon as I can, anyway.

Thanks again,
Angelo

> [    1.291055] mtk-msdc 11240000.mmc: msdc_track_cmd_data: cmd=8
> arg=000001AA; host->error=0x00000002
> [    1.292775] mtk-msdc 11240000.mmc: msdc_track_cmd_data: cmd=55
> arg=00000000; host->error=0x00000002
> [    1.294539] mtk-msdc 11240000.mmc: msdc_track_cmd_data: cmd=55
> arg=00000000; host->error=0x00000002
> [    1.296293] mtk-msdc 11240000.mmc: msdc_track_cmd_data: cmd=55
> arg=00000000; host->error=0x00000002
> ...
> [    1.430408] mtk-msdc 11240000.mmc: msdc_track_cmd_data: cmd=55
> arg=00000000; host->error=0x00000002
> [    1.433766] mmc0: Failed to initialize a non-removable card
> [   22.297240] rcu: INFO: rcu_preempt detected stalls on CPUs/tasks:
> [   22.298723] rcu:     6-...0: (2 ticks this GP)
> idle=104c/1/0x4000000000000000 softirq=45/45 fqs=37
> [   22.299827] rcu:     (detected by 2, t=5256 jiffies, g=-1051, q=200
> ncpus=8)
> [   22.300689] Sending NMI from CPU 2 to CPUs 6:
> 
> Best regards,
> Macpaul Lin

On Wed, 2025-10-29 at 11:56 +0100, AngeloGioacchino Del Regno wrote:
> 

[snip]

> Thanks for the logs! Getting issues on MSDC is really odd because the
> MSDC
> controller does *not* use nor have any power domain...
> 
> I'll take a look at this as soon as I can, anyway.
> 
> Thanks again,
> Angelo
> 
> > [    1.291055] mtk-msdc 11240000.mmc: msdc_track_cmd_data: cmd=8
> > arg=000001AA; host->error=0x00000002
> > [    1.292775] mtk-msdc 11240000.mmc: msdc_track_cmd_data: cmd=55
> > arg=00000000; host->error=0x00000002
> > [    1.294539] mtk-msdc 11240000.mmc: msdc_track_cmd_data: cmd=55
> > arg=00000000; host->error=0x00000002
> > [    1.296293] mtk-msdc 11240000.mmc: msdc_track_cmd_data: cmd=55
> > arg=00000000; host->error=0x00000002
> > ...
> > [    1.430408] mtk-msdc 11240000.mmc: msdc_track_cmd_data: cmd=55
> > arg=00000000; host->error=0x00000002
> > [    1.433766] mmc0: Failed to initialize a non-removable card
> > [   22.297240] rcu: INFO: rcu_preempt detected stalls on
> > CPUs/tasks:
> > [   22.298723] rcu:     6-...0: (2 ticks this GP)
> > idle=104c/1/0x4000000000000000 softirq=45/45 fqs=37
> > [   22.299827] rcu:     (detected by 2, t=5256 jiffies, g=-1051,
> > q=200
> > ncpus=8)
> > [   22.300689] Sending NMI from CPU 2 to CPUs 6:
> > 
> > Best regards,
> > Macpaul Lin
> 
> 

The command retry only happens in the very beginning during linux boot.

The following rcu_preempt and cpu/task stall are keep appearing until
poewr off. It is more like cpu not responding or runtime-PM? I've not
try to disable any power related kernel option yet.

[22649.698912] rcu: INFO: rcu_preempt detected stalls on CPUs/tasks:
[22649.699687] rcu:     5-...0: (0 ticks this GP)
idle=1b7c/1/0x4000000000000000 softirq=32/32 fqs=976599
[22649.700835] rcu:     (detected by 1, t=5662102 jiffies, g=-1055,
q=216 ncpus=8)
[22649.701728] Sending NMI from CPU 1 to CPUs 5:
[22659.701734] rcu: rcu_preempt kthread starved for 2495 jiffies! g-
1055 f0x0 RCU_GP_DOING_FQS(6) ->state=0x0 ->cpu=0
[22659.703592] rcu:     Unless rcu_preempt kthread gets sufficient CPU
time, OOM is now expected behavior.
[22659.704746] rcu: RCU grace-period kthread stack dump:
[22659.705386] task:rcu_preempt     state:I stack:0     pid:15   
tgid:15    ppid:2      task_flags:0x208040 flags:0x00000010
[22659.706784] Call trace:
[22659.707097]  __switch_to+0xf0/0x1b0 (T)
[22659.707590]  __schedule+0x3b0/0xca8
[22659.708039]  schedule+0x3c/0x120
[22659.708453]  schedule_timeout+0x8c/0x130
[22659.708956]  rcu_gp_fqs_loop+0x140/0x6c0
[22659.709459]  rcu_gp_kthread+0x208/0x258
[22659.709951]  kthread+0x14c/0x230
[22659.710365]  ret_from_fork+0x10/0x20
[22659.710824] rcu: Stack dump where RCU GP kthread last ran:
[22659.711517] Sending NMI from CPU 1 to CPUs 0:
[22659.712109] NMI backtrace for cpu 0
[22659.712117] CPU: 0 UID: 0 PID: 0 Comm: swapper/0 Not tainted 6.18.0-
rc3-next-20251028-mtk+modified-04392-g5c921b2c5975 #1 PREEMPT
[22659.712123] Hardware name: MediaTek Genio 1200 EVK-P1V2-EMMC (DT)
[22659.712126] pstate: 40400009 (nZcv daif +PAN -UAO -TCO -DIT -SSBS
BTYPE=--)
[22659.712130] pc : cpuidle_enter_state+0x2bc/0x720
[22659.712137] lr : cpuidle_enter_state+0x2b8/0x720
[22659.712141] sp : ffff800084473d00
[22659.712143] x29: ffff800084473d00 x28: 0000000000000000 x27:
ffff0001fef7d8c0
[22659.712151] x26: 0000000000000001 x25: 0000149be00653eb x24:
0000149be01736ef
[22659.712157] x23: 0000000000000002 x22: ffff0000c2d38880 x21:
0000000000000002
[22659.712163] x20: ffff0000c2d38968 x19: ffff0001fee7e9f8 x18:
000000005bd3baae
[22659.712169] x17: 000000040044ffff x16: 005000f2b5503510 x15:
0000000000000000
[22659.712175] x14: ffff800084487540 x13: ffff80017b8c8000 x12:
ffff8000816f9830
[22659.712181] x11: 0000000000000000 x10: 0000000000000000 x9 :
ffff8000801dc64c
[22659.712187] x8 : 000000f2b5503510 x7 : 0000000000f0000f x6 :
00000000ff0ffff0
[22659.712193] x5 : ffff80017b8c8000 x4 : 0000000000000000 x3 :
ffff800084473c10
[22659.712199] x2 : 000000449d287e7f x1 : ffff800084487540 x0 :
0000000000000000
[22659.712205] Call trace:
[22659.712207]  cpuidle_enter_state+0x2bc/0x720 (P)
[22659.712212]  cpuidle_enter+0x40/0x60
[22659.712219]  do_idle+0x208/0x298
[22659.712226]  cpu_startup_entry+0x3c/0x50
[22659.712232]  rest_init+0xfc/0x120
[22659.712236]  start_kernel+0x7a4/0x920
[22659.712243]  __primary_switched+0x88/0x98

Regards,
Macpaul Lin
 

Il 29/10/25 16:01, Macpaul Lin (林智斌) ha scritto:
> On Wed, 2025-10-29 at 11:56 +0100, AngeloGioacchino Del Regno wrote:
>>
> 
> [snip]
> 
>> Thanks for the logs! Getting issues on MSDC is really odd because the
>> MSDC
>> controller does *not* use nor have any power domain...
>>
>> I'll take a look at this as soon as I can, anyway.
>>
>> Thanks again,
>> Angelo
>>
>>> [    1.291055] mtk-msdc 11240000.mmc: msdc_track_cmd_data: cmd=8
>>> arg=000001AA; host->error=0x00000002
>>> [    1.292775] mtk-msdc 11240000.mmc: msdc_track_cmd_data: cmd=55
>>> arg=00000000; host->error=0x00000002
>>> [    1.294539] mtk-msdc 11240000.mmc: msdc_track_cmd_data: cmd=55
>>> arg=00000000; host->error=0x00000002
>>> [    1.296293] mtk-msdc 11240000.mmc: msdc_track_cmd_data: cmd=55
>>> arg=00000000; host->error=0x00000002
>>> ...
>>> [    1.430408] mtk-msdc 11240000.mmc: msdc_track_cmd_data: cmd=55
>>> arg=00000000; host->error=0x00000002
>>> [    1.433766] mmc0: Failed to initialize a non-removable card
>>> [   22.297240] rcu: INFO: rcu_preempt detected stalls on
>>> CPUs/tasks:
>>> [   22.298723] rcu:     6-...0: (2 ticks this GP)
>>> idle=104c/1/0x4000000000000000 softirq=45/45 fqs=37
>>> [   22.299827] rcu:     (detected by 2, t=5256 jiffies, g=-1051,
>>> q=200
>>> ncpus=8)
>>> [   22.300689] Sending NMI from CPU 2 to CPUs 6:
>>>
>>> Best regards,
>>> Macpaul Lin
>>
>>
> 
> The command retry only happens in the very beginning during linux boot.
> 
> The following rcu_preempt and cpu/task stall are keep appearing until
> poewr off. It is more like cpu not responding or runtime-PM? I've not
> try to disable any power related kernel option yet.
> 

Hello Macpaul,

I have located the source of the issue that is showing on your machine, and
I have sent a fast patch to resolve that.

Please have a look:
https://lore.kernel.org/all/20251121125212.43093-1-angelogioacchino.delregno@collabora.com/

Cheers,
Angelo

> [22649.698912] rcu: INFO: rcu_preempt detected stalls on CPUs/tasks:
> [22649.699687] rcu:     5-...0: (0 ticks this GP)
> idle=1b7c/1/0x4000000000000000 softirq=32/32 fqs=976599
> [22649.700835] rcu:     (detected by 1, t=5662102 jiffies, g=-1055,
> q=216 ncpus=8)
> [22649.701728] Sending NMI from CPU 1 to CPUs 5:
> [22659.701734] rcu: rcu_preempt kthread starved for 2495 jiffies! g-
> 1055 f0x0 RCU_GP_DOING_FQS(6) ->state=0x0 ->cpu=0
> [22659.703592] rcu:     Unless rcu_preempt kthread gets sufficient CPU
> time, OOM is now expected behavior.
> [22659.704746] rcu: RCU grace-period kthread stack dump:
> [22659.705386] task:rcu_preempt     state:I stack:0     pid:15
> tgid:15    ppid:2      task_flags:0x208040 flags:0x00000010
> [22659.706784] Call trace:
> [22659.707097]  __switch_to+0xf0/0x1b0 (T)
> [22659.707590]  __schedule+0x3b0/0xca8
> [22659.708039]  schedule+0x3c/0x120
> [22659.708453]  schedule_timeout+0x8c/0x130
> [22659.708956]  rcu_gp_fqs_loop+0x140/0x6c0
> [22659.709459]  rcu_gp_kthread+0x208/0x258
> [22659.709951]  kthread+0x14c/0x230
> [22659.710365]  ret_from_fork+0x10/0x20
> [22659.710824] rcu: Stack dump where RCU GP kthread last ran:
> [22659.711517] Sending NMI from CPU 1 to CPUs 0:
> [22659.712109] NMI backtrace for cpu 0
> [22659.712117] CPU: 0 UID: 0 PID: 0 Comm: swapper/0 Not tainted 6.18.0-
> rc3-next-20251028-mtk+modified-04392-g5c921b2c5975 #1 PREEMPT
> [22659.712123] Hardware name: MediaTek Genio 1200 EVK-P1V2-EMMC (DT)
> [22659.712126] pstate: 40400009 (nZcv daif +PAN -UAO -TCO -DIT -SSBS
> BTYPE=--)
> [22659.712130] pc : cpuidle_enter_state+0x2bc/0x720
> [22659.712137] lr : cpuidle_enter_state+0x2b8/0x720
> [22659.712141] sp : ffff800084473d00
> [22659.712143] x29: ffff800084473d00 x28: 0000000000000000 x27:
> ffff0001fef7d8c0
> [22659.712151] x26: 0000000000000001 x25: 0000149be00653eb x24:
> 0000149be01736ef
> [22659.712157] x23: 0000000000000002 x22: ffff0000c2d38880 x21:
> 0000000000000002
> [22659.712163] x20: ffff0000c2d38968 x19: ffff0001fee7e9f8 x18:
> 000000005bd3baae
> [22659.712169] x17: 000000040044ffff x16: 005000f2b5503510 x15:
> 0000000000000000
> [22659.712175] x14: ffff800084487540 x13: ffff80017b8c8000 x12:
> ffff8000816f9830
> [22659.712181] x11: 0000000000000000 x10: 0000000000000000 x9 :
> ffff8000801dc64c
> [22659.712187] x8 : 000000f2b5503510 x7 : 0000000000f0000f x6 :
> 00000000ff0ffff0
> [22659.712193] x5 : ffff80017b8c8000 x4 : 0000000000000000 x3 :
> ffff800084473c10
> [22659.712199] x2 : 000000449d287e7f x1 : ffff800084487540 x0 :
> 0000000000000000
> [22659.712205] Call trace:
> [22659.712207]  cpuidle_enter_state+0x2bc/0x720 (P)
> [22659.712212]  cpuidle_enter+0x40/0x60
> [22659.712219]  do_idle+0x208/0x298
> [22659.712226]  cpu_startup_entry+0x3c/0x50
> [22659.712232]  rest_init+0xfc/0x120
> [22659.712236]  start_kernel+0x7a4/0x920
> [22659.712243]  __primary_switched+0x88/0x98
> 
> Regards,
> Macpaul Lin
>