2022-04-06 14:17:55

by Peng Fan (OSS)

[permalink] [raw]
Subject: [PATCH 0/5] imx: support noc settings with power domain

From: Peng Fan <[email protected]>

i.MX8MP has a design that NoC(Not main NoC) is distributed in multiple
blocks, such as vpumix, hsiomix and etc. The access to NoC requires
power domain on and blk ctrl settings configured.

So the design here is for mixes that not have blk-ctrl, configure
the NoC in gpcv2 driver, for mixes that have blk-ctrl, configure
the NoC in blk-ctrl drivers.

This v1 patchset not apply on Shawn's tree, I picked up Lucas's HSIO
and Laurent's mediablk patches, then worked out this patchset:
https://github.com/MrVan/linux/tree/noc-imx8mp

Note: This interconnect related functions not added. This patchset
is only to replace the function did in NXP downstream:
https://source.codeaurora.org/external/imx/imx-atf/tree/plat/imx/imx8m/imx8mp/gpc.c?h=lf_v2.4#n157

Peng Fan (5):
dt-bindings: interconnect: imx8m: Add bindings for imx8mp noc
arm64: dts: imx8mp: add noc node
soc: imx: gpcv2: support i.MX8MP NoC settings
soc: imx: imx8m-blk-ctrl: support i.MX8MP media blk ctrl noc settings
soc: imx: imx8mp-blk-ctrl: introduce HSIO blk ctrl noc settings

.../bindings/interconnect/fsl,imx8m-noc.yaml | 6 +
arch/arm64/boot/dts/freescale/imx8mp.dtsi | 7 ++
drivers/soc/imx/gpcv2.c | 56 ++++++++-
drivers/soc/imx/imx8m-blk-ctrl.c | 109 ++++++++++++++++++
drivers/soc/imx/imx8mp-blk-ctrl.c | 74 ++++++++++++
5 files changed, 251 insertions(+), 1 deletion(-)

--
2.25.1


2022-04-06 14:45:24

by Peng Fan (OSS)

[permalink] [raw]
Subject: [PATCH 4/5] soc: imx: imx8m-blk-ctrl: support i.MX8MP media blk ctrl noc settings

From: Peng Fan <[email protected]>

The out of reset value of NoC is not a valid value, we need
set it to a correct value. We only need to set it after power on.

Signed-off-by: Peng Fan <[email protected]>
---
drivers/soc/imx/imx8m-blk-ctrl.c | 109 +++++++++++++++++++++++++++++++
1 file changed, 109 insertions(+)

diff --git a/drivers/soc/imx/imx8m-blk-ctrl.c b/drivers/soc/imx/imx8m-blk-ctrl.c
index 3a6abd70520c..5b382b4e6a64 100644
--- a/drivers/soc/imx/imx8m-blk-ctrl.c
+++ b/drivers/soc/imx/imx8m-blk-ctrl.c
@@ -12,6 +12,7 @@
#include <linux/pm_runtime.h>
#include <linux/regmap.h>
#include <linux/clk.h>
+#include <linux/mfd/syscon.h>

#include <dt-bindings/power/imx8mm-power.h>
#include <dt-bindings/power/imx8mn-power.h>
@@ -29,10 +30,19 @@ struct imx8m_blk_ctrl {
struct notifier_block power_nb;
struct device *bus_power_dev;
struct regmap *regmap;
+ struct regmap *noc_regmap;
struct imx8m_blk_ctrl_domain *domains;
struct genpd_onecell_data onecell_data;
};

+struct imx8m_blk_ctrl_noc_data {
+ u32 off;
+ u32 priority;
+ u32 mode;
+ u32 extctrl;
+};
+
+#define DOMAIN_MAX_NOC 4
struct imx8m_blk_ctrl_domain_data {
const char *name;
const char * const *clk_names;
@@ -49,6 +59,7 @@ struct imx8m_blk_ctrl_domain_data {
* register.
*/
u32 mipi_phy_rst_mask;
+ const struct imx8m_blk_ctrl_noc_data *noc_data[DOMAIN_MAX_NOC];
};

#define DOMAIN_MAX_CLKS 3
@@ -66,6 +77,7 @@ struct imx8m_blk_ctrl_data {
notifier_fn_t power_notifier_fn;
const struct imx8m_blk_ctrl_domain_data *domains;
int num_domains;
+ bool has_noc;
};

static inline struct imx8m_blk_ctrl_domain *
@@ -74,6 +86,27 @@ to_imx8m_blk_ctrl_domain(struct generic_pm_domain *genpd)
return container_of(genpd, struct imx8m_blk_ctrl_domain, genpd);
}

+static int imx8m_blk_ctrl_noc_set(struct imx8m_blk_ctrl_domain *domain)
+{
+ const struct imx8m_blk_ctrl_domain_data *data = domain->data;
+ struct imx8m_blk_ctrl *bc = domain->bc;
+ struct regmap *regmap = bc->noc_regmap;
+ int i;
+
+ if (!data || !regmap)
+ return 0;
+
+ for (i = 0; i < DOMAIN_MAX_NOC; i++) {
+ if (!data->noc_data[i])
+ continue;
+ regmap_write(regmap, data->noc_data[i]->off + 0x8, data->noc_data[i]->priority);
+ regmap_write(regmap, data->noc_data[i]->off + 0xc, data->noc_data[i]->mode);
+ regmap_write(regmap, data->noc_data[i]->off + 0x18, data->noc_data[i]->extctrl);
+ }
+
+ return 0;
+}
+
static int imx8m_blk_ctrl_power_on(struct generic_pm_domain *genpd)
{
struct imx8m_blk_ctrl_domain *domain = to_imx8m_blk_ctrl_domain(genpd);
@@ -117,6 +150,8 @@ static int imx8m_blk_ctrl_power_on(struct generic_pm_domain *genpd)
if (data->mipi_phy_rst_mask)
regmap_set_bits(bc->regmap, BLK_MIPI_RESET_DIV, data->mipi_phy_rst_mask);

+ imx8m_blk_ctrl_noc_set(domain);
+
/* disable upstream clocks */
clk_bulk_disable_unprepare(data->num_clks, domain->clks);

@@ -172,6 +207,7 @@ static int imx8m_blk_ctrl_probe(struct platform_device *pdev)
const struct imx8m_blk_ctrl_data *bc_data;
struct device *dev = &pdev->dev;
struct imx8m_blk_ctrl *bc;
+ struct regmap *regmap;
void __iomem *base;
int i, ret;

@@ -218,6 +254,10 @@ static int imx8m_blk_ctrl_probe(struct platform_device *pdev)
return dev_err_probe(dev, PTR_ERR(bc->bus_power_dev),
"failed to attach power domain\n");

+ regmap = syscon_regmap_lookup_by_compatible("fsl,imx8m-noc");
+ if (!IS_ERR(regmap))
+ bc->noc_regmap = regmap;
+
for (i = 0; i < bc_data->num_domains; i++) {
const struct imx8m_blk_ctrl_domain_data *data = &bc_data->domains[i];
struct imx8m_blk_ctrl_domain *domain = &bc->domains[i];
@@ -677,6 +717,55 @@ static int imx8mp_media_power_notifier(struct notifier_block *nb,
return NOTIFY_OK;
}

+#define IMX8MP_MEDIABLK_LCDIF_RD 0
+#define IMX8MP_MEDIABLK_LCDIF_WR 1
+#define IMX8MP_MEDIABLK_ISI0 2
+#define IMX8MP_MEDIABLK_ISI1 3
+#define IMX8MP_MEDIABLK_ISI2 4
+#define IMX8MP_MEDIABLK_ISP0 5
+#define IMX8MP_MEDIABLK_ISP1 6
+#define IMX8MP_MEDIABLK_DWE 7
+
+static const struct imx8m_blk_ctrl_noc_data imx8mp_media_noc_data[] = {
+ [IMX8MP_MEDIABLK_LCDIF_RD] = {
+ .off = 0x980,
+ .priority = 0x80000202,
+ .extctrl = 1,
+ },
+ [IMX8MP_MEDIABLK_LCDIF_WR] = {
+ .off = 0xa00,
+ .priority = 0x80000202,
+ .extctrl = 1,
+ },
+ [IMX8MP_MEDIABLK_ISI0] = {
+ .off = 0xa80,
+ .priority = 0x80000202,
+ .extctrl = 1,
+ },
+ [IMX8MP_MEDIABLK_ISI1] = {
+ .off = 0xb00,
+ .priority = 0x80000202,
+ .extctrl = 1,
+ },
+ [IMX8MP_MEDIABLK_ISI2] = {
+ .off = 0xb80,
+ .priority = 0x80000202,
+ .extctrl = 1,
+ },
+ [IMX8MP_MEDIABLK_ISP0] = {
+ .off = 0xc00,
+ .priority = 0x80000707,
+ },
+ [IMX8MP_MEDIABLK_ISP1] = {
+ .off = 0xc80,
+ .priority = 0x80000707,
+ },
+ [IMX8MP_MEDIABLK_DWE] = {
+ .off = 0xd00,
+ .priority = 0x80000707,
+ },
+};
+
/*
* From i.MX 8M Plus Applications Processor Reference Manual, Rev. 1,
* section 13.2.2, 13.2.3
@@ -708,6 +797,10 @@ static const struct imx8m_blk_ctrl_domain_data imx8mp_media_blk_ctl_domain_data[
.gpc_name = "lcdif1",
.rst_mask = BIT(4) | BIT(5) | BIT(23),
.clk_mask = BIT(4) | BIT(5) | BIT(23),
+ .noc_data = {
+ &imx8mp_media_noc_data[IMX8MP_MEDIABLK_LCDIF_RD],
+ &imx8mp_media_noc_data[IMX8MP_MEDIABLK_LCDIF_WR],
+ },
},
[IMX8MP_MEDIABLK_PD_ISI] = {
.name = "mediablk-isi",
@@ -716,6 +809,11 @@ static const struct imx8m_blk_ctrl_domain_data imx8mp_media_blk_ctl_domain_data[
.gpc_name = "isi",
.rst_mask = BIT(6) | BIT(7),
.clk_mask = BIT(6) | BIT(7),
+ .noc_data = {
+ &imx8mp_media_noc_data[IMX8MP_MEDIABLK_ISI0],
+ &imx8mp_media_noc_data[IMX8MP_MEDIABLK_ISI1],
+ &imx8mp_media_noc_data[IMX8MP_MEDIABLK_ISI2],
+ },
},
[IMX8MP_MEDIABLK_PD_MIPI_CSI2_2] = {
.name = "mediablk-mipi-csi2-2",
@@ -733,6 +831,10 @@ static const struct imx8m_blk_ctrl_domain_data imx8mp_media_blk_ctl_domain_data[
.gpc_name = "lcdif2",
.rst_mask = BIT(11) | BIT(12) | BIT(24),
.clk_mask = BIT(11) | BIT(12) | BIT(24),
+ .noc_data = {
+ &imx8mp_media_noc_data[IMX8MP_MEDIABLK_LCDIF_RD],
+ &imx8mp_media_noc_data[IMX8MP_MEDIABLK_LCDIF_WR],
+ },
},
[IMX8MP_MEDIABLK_PD_ISP2] = {
.name = "mediablk-isp2",
@@ -749,6 +851,10 @@ static const struct imx8m_blk_ctrl_domain_data imx8mp_media_blk_ctl_domain_data[
.gpc_name = "isp1",
.rst_mask = BIT(16) | BIT(17) | BIT(18),
.clk_mask = BIT(16) | BIT(17) | BIT(18),
+ .noc_data = {
+ &imx8mp_media_noc_data[IMX8MP_MEDIABLK_ISP0],
+ &imx8mp_media_noc_data[IMX8MP_MEDIABLK_ISP1],
+ },
},
[IMX8MP_MEDIABLK_PD_DWE] = {
.name = "mediablk-dwe",
@@ -757,6 +863,9 @@ static const struct imx8m_blk_ctrl_domain_data imx8mp_media_blk_ctl_domain_data[
.gpc_name = "dwe",
.rst_mask = BIT(19) | BIT(20) | BIT(21),
.clk_mask = BIT(19) | BIT(20) | BIT(21),
+ .noc_data = {
+ &imx8mp_media_noc_data[IMX8MP_MEDIABLK_DWE],
+ },
},
[IMX8MP_MEDIABLK_PD_MIPI_DSI_2] = {
.name = "mediablk-mipi-dsi-2",
--
2.25.1

2022-04-06 15:06:23

by Peng Fan (OSS)

[permalink] [raw]
Subject: [PATCH 5/5] soc: imx: imx8mp-blk-ctrl: introduce HSIO blk ctrl noc settings

From: Peng Fan <[email protected]>

The out of reset value of HSIO NoC is not a valid value, we need
set it to a correct value. We only need to set it after power on.

Signed-off-by: Peng Fan <[email protected]>
---
drivers/soc/imx/imx8mp-blk-ctrl.c | 74 +++++++++++++++++++++++++++++++
1 file changed, 74 insertions(+)

diff --git a/drivers/soc/imx/imx8mp-blk-ctrl.c b/drivers/soc/imx/imx8mp-blk-ctrl.c
index e832c007b063..929fd9b770ae 100644
--- a/drivers/soc/imx/imx8mp-blk-ctrl.c
+++ b/drivers/soc/imx/imx8mp-blk-ctrl.c
@@ -12,6 +12,7 @@
#include <linux/pm_domain.h>
#include <linux/pm_runtime.h>
#include <linux/regmap.h>
+#include <linux/mfd/syscon.h>

#include <dt-bindings/power/imx8mp-power.h>

@@ -26,14 +27,24 @@ struct imx8mp_hsio_blk_ctrl {
struct notifier_block power_nb;
struct device *bus_power_dev;
struct regmap *regmap;
+ struct regmap *noc_regmap;
struct imx8mp_hsio_blk_ctrl_domain *domains;
struct genpd_onecell_data onecell_data;
};

+struct imx8mp_hsio_blk_ctrl_noc_data {
+ u32 off;
+ u32 priority;
+ u32 mode;
+ u32 extctrl;
+};
+
+#define DOMAIN_MAX_NOC 2
struct imx8mp_hsio_blk_ctrl_domain_data {
const char *name;
const char *clk_name;
const char *gpc_name;
+ const struct imx8mp_hsio_blk_ctrl_noc_data *noc_data[DOMAIN_MAX_NOC];
};

struct imx8mp_hsio_blk_ctrl_domain {
@@ -41,6 +52,7 @@ struct imx8mp_hsio_blk_ctrl_domain {
struct clk *clk;
struct device *power_dev;
struct imx8mp_hsio_blk_ctrl *bc;
+ const struct imx8mp_hsio_blk_ctrl_domain_data *data;
int id;
};

@@ -50,6 +62,27 @@ to_imx8mp_hsio_blk_ctrl_domain(struct generic_pm_domain *genpd)
return container_of(genpd, struct imx8mp_hsio_blk_ctrl_domain, genpd);
}

+static int imx8mp_hsio_noc_set(struct imx8mp_hsio_blk_ctrl_domain *domain)
+{
+ const struct imx8mp_hsio_blk_ctrl_domain_data *data = domain->data;
+ struct imx8mp_hsio_blk_ctrl *bc = domain->bc;
+ struct regmap *regmap = bc->noc_regmap;
+ int i;
+
+ if (!data || !regmap)
+ return 0;
+
+ for (i = 0; i < DOMAIN_MAX_NOC; i++) {
+ if (!data->noc_data[i])
+ continue;
+ regmap_write(regmap, data->noc_data[i]->off + 0x8, data->noc_data[i]->priority);
+ regmap_write(regmap, data->noc_data[i]->off + 0xc, data->noc_data[i]->mode);
+ regmap_write(regmap, data->noc_data[i]->off + 0x18, data->noc_data[i]->extctrl);
+ }
+
+ return 0;
+}
+
static int imx8mp_hsio_blk_ctrl_power_on(struct generic_pm_domain *genpd)
{
struct imx8mp_hsio_blk_ctrl_domain *domain =
@@ -89,6 +122,8 @@ static int imx8mp_hsio_blk_ctrl_power_on(struct generic_pm_domain *genpd)
goto clk_disable;
}

+ imx8mp_hsio_noc_set(domain);
+
return 0;

clk_disable:
@@ -143,11 +178,39 @@ imx8m_blk_ctrl_xlate(struct of_phandle_args *args, void *data)

static struct lock_class_key blk_ctrl_genpd_lock_class;

+#define IMX8MP_HSIOBLK_NOC_PCIE 0
+#define IMX8MP_HSIOBLK_USB1 1
+#define IMX8MP_HSIOBLK_USB2 2
+#define IMX8MP_HSIOBLK_PCIE 3
+
+static const struct imx8mp_hsio_blk_ctrl_noc_data imx8mp_hsio_noc_data[] = {
+ [IMX8MP_HSIOBLK_NOC_PCIE] = {
+ .off = 0x780,
+ .priority = 0x80000303,
+ },
+ [IMX8MP_HSIOBLK_USB1] = {
+ .off = 0x800,
+ .priority = 0x80000303,
+ },
+ [IMX8MP_HSIOBLK_USB2] = {
+ .off = 0x880,
+ .priority = 0x80000303,
+ },
+ [IMX8MP_HSIOBLK_PCIE] = {
+ .off = 0x900,
+ .priority = 0x80000303,
+ },
+};
+
static const struct imx8mp_hsio_blk_ctrl_domain_data imx8mp_hsio_domain_data[] = {
[IMX8MP_HSIOBLK_PD_USB] = {
.name = "hsioblk-usb",
.clk_name = "usb",
.gpc_name = "usb",
+ .noc_data = {
+ &imx8mp_hsio_noc_data[IMX8MP_HSIOBLK_USB1],
+ &imx8mp_hsio_noc_data[IMX8MP_HSIOBLK_USB2]
+ },
},
[IMX8MP_HSIOBLK_PD_USB_PHY1] = {
.name = "hsioblk-usb-phy1",
@@ -161,6 +224,10 @@ static const struct imx8mp_hsio_blk_ctrl_domain_data imx8mp_hsio_domain_data[] =
.name = "hsioblk-pcie",
.clk_name = "pcie",
.gpc_name = "pcie",
+ .noc_data = {
+ &imx8mp_hsio_noc_data[IMX8MP_HSIOBLK_NOC_PCIE],
+ &imx8mp_hsio_noc_data[IMX8MP_HSIOBLK_PCIE]
+ },
},
[IMX8MP_HSIOBLK_PD_PCIE_PHY] = {
.name = "hsioblk-pcie-phy",
@@ -215,6 +282,7 @@ static int imx8mp_hsio_blk_ctrl_probe(struct platform_device *pdev)
int num_domains = ARRAY_SIZE(imx8mp_hsio_domain_data);
struct device *dev = &pdev->dev;
struct imx8mp_hsio_blk_ctrl *bc;
+ struct regmap *regmap;
void __iomem *base;
int i, ret;

@@ -259,11 +327,17 @@ static int imx8mp_hsio_blk_ctrl_probe(struct platform_device *pdev)
return dev_err_probe(dev, PTR_ERR(bc->bus_power_dev),
"failed to attach bus power domain\n");

+ regmap = syscon_regmap_lookup_by_compatible("fsl,imx8m-noc");
+ if (!IS_ERR(regmap))
+ bc->noc_regmap = regmap;
+
for (i = 0; i < num_domains; i++) {
const struct imx8mp_hsio_blk_ctrl_domain_data *data =
&imx8mp_hsio_domain_data[i];
struct imx8mp_hsio_blk_ctrl_domain *domain = &bc->domains[i];

+ domain->data = data;
+
if (data->clk_name) {
domain->clk = devm_clk_get(dev, data->clk_name);
if (IS_ERR(domain->clk)) {
--
2.25.1

2022-04-06 15:17:58

by Laurent Pinchart

[permalink] [raw]
Subject: Re: [PATCH 5/5] soc: imx: imx8mp-blk-ctrl: introduce HSIO blk ctrl noc settings

Hi Peng,

Thank you for the patch.

On Wed, Apr 06, 2022 at 04:23:30PM +0800, Peng Fan (OSS) wrote:
> From: Peng Fan <[email protected]>
>
> The out of reset value of HSIO NoC is not a valid value, we need
> set it to a correct value. We only need to set it after power on.
>
> Signed-off-by: Peng Fan <[email protected]>
> ---
> drivers/soc/imx/imx8mp-blk-ctrl.c | 74 +++++++++++++++++++++++++++++++
> 1 file changed, 74 insertions(+)
>
> diff --git a/drivers/soc/imx/imx8mp-blk-ctrl.c b/drivers/soc/imx/imx8mp-blk-ctrl.c
> index e832c007b063..929fd9b770ae 100644
> --- a/drivers/soc/imx/imx8mp-blk-ctrl.c
> +++ b/drivers/soc/imx/imx8mp-blk-ctrl.c
> @@ -12,6 +12,7 @@
> #include <linux/pm_domain.h>
> #include <linux/pm_runtime.h>
> #include <linux/regmap.h>
> +#include <linux/mfd/syscon.h>
>
> #include <dt-bindings/power/imx8mp-power.h>
>
> @@ -26,14 +27,24 @@ struct imx8mp_hsio_blk_ctrl {
> struct notifier_block power_nb;
> struct device *bus_power_dev;
> struct regmap *regmap;
> + struct regmap *noc_regmap;
> struct imx8mp_hsio_blk_ctrl_domain *domains;
> struct genpd_onecell_data onecell_data;
> };
>
> +struct imx8mp_hsio_blk_ctrl_noc_data {
> + u32 off;
> + u32 priority;
> + u32 mode;
> + u32 extctrl;
> +};

Maybe the data structure, and possibly the imx8mp_hsio_noc_set()
function, could be shared with imx8m-blk-ctrl.c ? It's much code, so if
it causes too much trouble, feel free to ignore.

The other comments on patch 4/5 apply to this one too.

> +
> +#define DOMAIN_MAX_NOC 2
> struct imx8mp_hsio_blk_ctrl_domain_data {
> const char *name;
> const char *clk_name;
> const char *gpc_name;
> + const struct imx8mp_hsio_blk_ctrl_noc_data *noc_data[DOMAIN_MAX_NOC];
> };
>
> struct imx8mp_hsio_blk_ctrl_domain {
> @@ -41,6 +52,7 @@ struct imx8mp_hsio_blk_ctrl_domain {
> struct clk *clk;
> struct device *power_dev;
> struct imx8mp_hsio_blk_ctrl *bc;
> + const struct imx8mp_hsio_blk_ctrl_domain_data *data;
> int id;
> };
>
> @@ -50,6 +62,27 @@ to_imx8mp_hsio_blk_ctrl_domain(struct generic_pm_domain *genpd)
> return container_of(genpd, struct imx8mp_hsio_blk_ctrl_domain, genpd);
> }
>
> +static int imx8mp_hsio_noc_set(struct imx8mp_hsio_blk_ctrl_domain *domain)
> +{
> + const struct imx8mp_hsio_blk_ctrl_domain_data *data = domain->data;
> + struct imx8mp_hsio_blk_ctrl *bc = domain->bc;
> + struct regmap *regmap = bc->noc_regmap;
> + int i;
> +
> + if (!data || !regmap)
> + return 0;
> +
> + for (i = 0; i < DOMAIN_MAX_NOC; i++) {
> + if (!data->noc_data[i])
> + continue;
> + regmap_write(regmap, data->noc_data[i]->off + 0x8, data->noc_data[i]->priority);
> + regmap_write(regmap, data->noc_data[i]->off + 0xc, data->noc_data[i]->mode);
> + regmap_write(regmap, data->noc_data[i]->off + 0x18, data->noc_data[i]->extctrl);
> + }
> +
> + return 0;
> +}
> +
> static int imx8mp_hsio_blk_ctrl_power_on(struct generic_pm_domain *genpd)
> {
> struct imx8mp_hsio_blk_ctrl_domain *domain =
> @@ -89,6 +122,8 @@ static int imx8mp_hsio_blk_ctrl_power_on(struct generic_pm_domain *genpd)
> goto clk_disable;
> }
>
> + imx8mp_hsio_noc_set(domain);
> +
> return 0;
>
> clk_disable:
> @@ -143,11 +178,39 @@ imx8m_blk_ctrl_xlate(struct of_phandle_args *args, void *data)
>
> static struct lock_class_key blk_ctrl_genpd_lock_class;
>
> +#define IMX8MP_HSIOBLK_NOC_PCIE 0
> +#define IMX8MP_HSIOBLK_USB1 1
> +#define IMX8MP_HSIOBLK_USB2 2
> +#define IMX8MP_HSIOBLK_PCIE 3
> +
> +static const struct imx8mp_hsio_blk_ctrl_noc_data imx8mp_hsio_noc_data[] = {
> + [IMX8MP_HSIOBLK_NOC_PCIE] = {
> + .off = 0x780,
> + .priority = 0x80000303,
> + },
> + [IMX8MP_HSIOBLK_USB1] = {
> + .off = 0x800,
> + .priority = 0x80000303,
> + },
> + [IMX8MP_HSIOBLK_USB2] = {
> + .off = 0x880,
> + .priority = 0x80000303,
> + },
> + [IMX8MP_HSIOBLK_PCIE] = {
> + .off = 0x900,
> + .priority = 0x80000303,
> + },
> +};
> +
> static const struct imx8mp_hsio_blk_ctrl_domain_data imx8mp_hsio_domain_data[] = {
> [IMX8MP_HSIOBLK_PD_USB] = {
> .name = "hsioblk-usb",
> .clk_name = "usb",
> .gpc_name = "usb",
> + .noc_data = {
> + &imx8mp_hsio_noc_data[IMX8MP_HSIOBLK_USB1],
> + &imx8mp_hsio_noc_data[IMX8MP_HSIOBLK_USB2]
> + },
> },
> [IMX8MP_HSIOBLK_PD_USB_PHY1] = {
> .name = "hsioblk-usb-phy1",
> @@ -161,6 +224,10 @@ static const struct imx8mp_hsio_blk_ctrl_domain_data imx8mp_hsio_domain_data[] =
> .name = "hsioblk-pcie",
> .clk_name = "pcie",
> .gpc_name = "pcie",
> + .noc_data = {
> + &imx8mp_hsio_noc_data[IMX8MP_HSIOBLK_NOC_PCIE],
> + &imx8mp_hsio_noc_data[IMX8MP_HSIOBLK_PCIE]
> + },
> },
> [IMX8MP_HSIOBLK_PD_PCIE_PHY] = {
> .name = "hsioblk-pcie-phy",
> @@ -215,6 +282,7 @@ static int imx8mp_hsio_blk_ctrl_probe(struct platform_device *pdev)
> int num_domains = ARRAY_SIZE(imx8mp_hsio_domain_data);
> struct device *dev = &pdev->dev;
> struct imx8mp_hsio_blk_ctrl *bc;
> + struct regmap *regmap;
> void __iomem *base;
> int i, ret;
>
> @@ -259,11 +327,17 @@ static int imx8mp_hsio_blk_ctrl_probe(struct platform_device *pdev)
> return dev_err_probe(dev, PTR_ERR(bc->bus_power_dev),
> "failed to attach bus power domain\n");
>
> + regmap = syscon_regmap_lookup_by_compatible("fsl,imx8m-noc");
> + if (!IS_ERR(regmap))
> + bc->noc_regmap = regmap;
> +
> for (i = 0; i < num_domains; i++) {
> const struct imx8mp_hsio_blk_ctrl_domain_data *data =
> &imx8mp_hsio_domain_data[i];
> struct imx8mp_hsio_blk_ctrl_domain *domain = &bc->domains[i];
>
> + domain->data = data;
> +
> if (data->clk_name) {
> domain->clk = devm_clk_get(dev, data->clk_name);
> if (IS_ERR(domain->clk)) {
> --
> 2.25.1
>

--
Regards,

Laurent Pinchart

2022-04-06 15:32:34

by Peng Fan

[permalink] [raw]
Subject: RE: [PATCH 0/5] imx: support noc settings with power domain

> Subject: Re: [PATCH 0/5] imx: support noc settings with power domain
>
> Hi Peng,
>
> Am Mittwoch, dem 06.04.2022 um 16:23 +0800 schrieb Peng Fan (OSS):
> > From: Peng Fan <[email protected]>
> >
> > i.MX8MP has a design that NoC(Not main NoC) is distributed in multiple
> > blocks, such as vpumix, hsiomix and etc. The access to NoC requires
> > power domain on and blk ctrl settings configured.
> >
> > So the design here is for mixes that not have blk-ctrl, configure the
> > NoC in gpcv2 driver, for mixes that have blk-ctrl, configure the NoC
> > in blk-ctrl drivers.
> >
> > This v1 patchset not apply on Shawn's tree, I picked up Lucas's HSIO
> > and Laurent's mediablk patches, then worked out this patchset:
> > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgith
> >
> ub.com%2FMrVan%2Flinux%2Ftree%2Fnoc-imx8mp&amp;data=04%7C01%7
> Cpeng.fan
> > %40nxp.com%7C3bd1d020ad8f4a68efc808da17b28ac8%7C686ea1d3bc2b
> 4c6fa92cd9
> >
> 9c5c301635%7C0%7C0%7C637848352908363591%7CUnknown%7CTWFpbG
> Zsb3d8eyJWIj
> >
> oiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3
> 000&am
> >
> p;sdata=ZVeHFy%2FEaWPhAj%2BURGIDXoWYdX5eeQoEIeZYZoxPPNo%3D&a
> mp;reserve
> > d=0
> >
> > Note: This interconnect related functions not added. This patchset is
> > only to replace the function did in NXP downstream:
> > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fsour
> >
> ce.codeaurora.org%2Fexternal%2Fimx%2Fimx-atf%2Ftree%2Fplat%2Fimx%2
> Fimx
> >
> 8m%2Fimx8mp%2Fgpc.c%3Fh%3Dlf_v2.4%23n157&amp;data=04%7C01%7C
> peng.fan%4
> >
> 0nxp.com%7C3bd1d020ad8f4a68efc808da17b28ac8%7C686ea1d3bc2b4c6fa
> 92cd99c
> >
> 5c301635%7C0%7C0%7C637848352908363591%7CUnknown%7CTWFpbGZs
> b3d8eyJWIjoi
> >
> MC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C30
> 00&amp;
> >
> sdata=eLawc3SJQBRmVwcOA2%2B6u6d2ZaYxqcO4Gm%2FqEJpqxFE%3D&a
> mp;reserved=
> > 0
>
> As a general comment I think this is implemented the wrong way around.
>
> Neither GPC, nor the blk-ctrl should poke into the NoC registers directly. The
> NoC driver should attach itself to the power domain via a notifier (same as
> the blk-ctrl does with the GPC domains) and should do the necessary NoC
> configuration when the power domain is powered up.

If separate NoC in a standalone driver, NoC may be configured not as early as
power domain up. Saying lcdif is running, NoC driver probe starts w/o defer
probe.

Thanks,
Peng.


>
> Regards,
> Lucas
> >
> > Peng Fan (5):
> > dt-bindings: interconnect: imx8m: Add bindings for imx8mp noc
> > arm64: dts: imx8mp: add noc node
> > soc: imx: gpcv2: support i.MX8MP NoC settings
> > soc: imx: imx8m-blk-ctrl: support i.MX8MP media blk ctrl noc settings
> > soc: imx: imx8mp-blk-ctrl: introduce HSIO blk ctrl noc settings
> >
> > .../bindings/interconnect/fsl,imx8m-noc.yaml | 6 +
> > arch/arm64/boot/dts/freescale/imx8mp.dtsi | 7 ++
> > drivers/soc/imx/gpcv2.c | 56 ++++++++-
> > drivers/soc/imx/imx8m-blk-ctrl.c | 109
> ++++++++++++++++++
> > drivers/soc/imx/imx8mp-blk-ctrl.c | 74 ++++++++++++
> > 5 files changed, 251 insertions(+), 1 deletion(-)
> >
>

2022-04-06 15:40:17

by Lucas Stach

[permalink] [raw]
Subject: Re: [PATCH 0/5] imx: support noc settings with power domain

Am Mittwoch, dem 06.04.2022 um 10:55 +0000 schrieb Peng Fan:
> > Subject: Re: [PATCH 0/5] imx: support noc settings with power domain
> >
> > Hi Peng,
> >
> > Am Mittwoch, dem 06.04.2022 um 16:23 +0800 schrieb Peng Fan (OSS):
> > > From: Peng Fan <[email protected]>
> > >
> > > i.MX8MP has a design that NoC(Not main NoC) is distributed in multiple
> > > blocks, such as vpumix, hsiomix and etc. The access to NoC requires
> > > power domain on and blk ctrl settings configured.
> > >
> > > So the design here is for mixes that not have blk-ctrl, configure the
> > > NoC in gpcv2 driver, for mixes that have blk-ctrl, configure the NoC
> > > in blk-ctrl drivers.
> > >
> > > This v1 patchset not apply on Shawn's tree, I picked up Lucas's HSIO
> > > and Laurent's mediablk patches, then worked out this patchset:
> > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgith
> > >
> > ub.com%2FMrVan%2Flinux%2Ftree%2Fnoc-imx8mp&amp;data=04%7C01%7
> > Cpeng.fan
> > > %40nxp.com%7C3bd1d020ad8f4a68efc808da17b28ac8%7C686ea1d3bc2b
> > 4c6fa92cd9
> > >
> > 9c5c301635%7C0%7C0%7C637848352908363591%7CUnknown%7CTWFpbG
> > Zsb3d8eyJWIj
> > >
> > oiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3
> > 000&am
> > >
> > p;sdata=ZVeHFy%2FEaWPhAj%2BURGIDXoWYdX5eeQoEIeZYZoxPPNo%3D&a
> > mp;reserve
> > > d=0
> > >
> > > Note: This interconnect related functions not added. This patchset is
> > > only to replace the function did in NXP downstream:
> > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fsour
> > >
> > ce.codeaurora.org%2Fexternal%2Fimx%2Fimx-atf%2Ftree%2Fplat%2Fimx%2
> > Fimx
> > >
> > 8m%2Fimx8mp%2Fgpc.c%3Fh%3Dlf_v2.4%23n157&amp;data=04%7C01%7C
> > peng.fan%4
> > >
> > 0nxp.com%7C3bd1d020ad8f4a68efc808da17b28ac8%7C686ea1d3bc2b4c6fa
> > 92cd99c
> > >
> > 5c301635%7C0%7C0%7C637848352908363591%7CUnknown%7CTWFpbGZs
> > b3d8eyJWIjoi
> > >
> > MC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C30
> > 00&amp;
> > >
> > sdata=eLawc3SJQBRmVwcOA2%2B6u6d2ZaYxqcO4Gm%2FqEJpqxFE%3D&a
> > mp;reserved=
> > > 0
> >
> > As a general comment I think this is implemented the wrong way around.
> >
> > Neither GPC, nor the blk-ctrl should poke into the NoC registers directly. The
> > NoC driver should attach itself to the power domain via a notifier (same as
> > the blk-ctrl does with the GPC domains) and should do the necessary NoC
> > configuration when the power domain is powered up.
>
> If separate NoC in a standalone driver, NoC may be configured not as early as
> power domain up. Saying lcdif is running, NoC driver probe starts w/o defer
> probe.

The right way to solve this would be to actually implement the
interconnect bits, so that consumers like the LCDIF that have specific
NOC bandwidth/latency requirements could request them from the NoC
driver. Proper probe deferral would come naturally with this.

The static NoC configuration per domain is quite a cludge IHMO, maybe
due to the decision to not open up any information about this part of
the SoC. Spreading support for this hack into multiple drivers doesn't
sound like a direction we want to take for upstream. At minimum we
could try to define the interconnect DT bits, so that the LCDIF driver,
etc. could attach to the NoC driver, giving us proper probe defer
behavior, even if the actual configuration is still static.

Regards,
Lucas

2022-04-06 15:47:22

by Laurent Pinchart

[permalink] [raw]
Subject: Re: [PATCH 4/5] soc: imx: imx8m-blk-ctrl: support i.MX8MP media blk ctrl noc settings

Hi Peng,

Thank you for the patch.

On Wed, Apr 06, 2022 at 04:23:29PM +0800, Peng Fan (OSS) wrote:
> From: Peng Fan <[email protected]>
>
> The out of reset value of NoC is not a valid value, we need
> set it to a correct value. We only need to set it after power on.
>
> Signed-off-by: Peng Fan <[email protected]>
> ---
> drivers/soc/imx/imx8m-blk-ctrl.c | 109 +++++++++++++++++++++++++++++++
> 1 file changed, 109 insertions(+)
>
> diff --git a/drivers/soc/imx/imx8m-blk-ctrl.c b/drivers/soc/imx/imx8m-blk-ctrl.c
> index 3a6abd70520c..5b382b4e6a64 100644
> --- a/drivers/soc/imx/imx8m-blk-ctrl.c
> +++ b/drivers/soc/imx/imx8m-blk-ctrl.c
> @@ -12,6 +12,7 @@
> #include <linux/pm_runtime.h>
> #include <linux/regmap.h>
> #include <linux/clk.h>
> +#include <linux/mfd/syscon.h>
>
> #include <dt-bindings/power/imx8mm-power.h>
> #include <dt-bindings/power/imx8mn-power.h>
> @@ -29,10 +30,19 @@ struct imx8m_blk_ctrl {
> struct notifier_block power_nb;
> struct device *bus_power_dev;
> struct regmap *regmap;
> + struct regmap *noc_regmap;
> struct imx8m_blk_ctrl_domain *domains;
> struct genpd_onecell_data onecell_data;
> };
>
> +struct imx8m_blk_ctrl_noc_data {
> + u32 off;

Maybe "offset" ?

> + u32 priority;
> + u32 mode;
> + u32 extctrl;
> +};
> +
> +#define DOMAIN_MAX_NOC 4

And a blank line here.

We have 3 NoC entries at most below, so this could be lowered, but I
wonder if it wouldn't be nicer to actually drop the macro (see below).

> struct imx8m_blk_ctrl_domain_data {
> const char *name;
> const char * const *clk_names;
> @@ -49,6 +59,7 @@ struct imx8m_blk_ctrl_domain_data {
> * register.
> */
> u32 mipi_phy_rst_mask;
> + const struct imx8m_blk_ctrl_noc_data *noc_data[DOMAIN_MAX_NOC];

This would become just

const struct imx8m_blk_ctrl_noc_data **noc_data;

> };
>
> #define DOMAIN_MAX_CLKS 3
> @@ -66,6 +77,7 @@ struct imx8m_blk_ctrl_data {
> notifier_fn_t power_notifier_fn;
> const struct imx8m_blk_ctrl_domain_data *domains;
> int num_domains;
> + bool has_noc;
> };
>
> static inline struct imx8m_blk_ctrl_domain *
> @@ -74,6 +86,27 @@ to_imx8m_blk_ctrl_domain(struct generic_pm_domain *genpd)
> return container_of(genpd, struct imx8m_blk_ctrl_domain, genpd);
> }
>
> +static int imx8m_blk_ctrl_noc_set(struct imx8m_blk_ctrl_domain *domain)
> +{
> + const struct imx8m_blk_ctrl_domain_data *data = domain->data;
> + struct imx8m_blk_ctrl *bc = domain->bc;
> + struct regmap *regmap = bc->noc_regmap;
> + int i;

As it can never be negative, you can make i an unsigned int.

> +
> + if (!data || !regmap)
> + return 0;
> +
> + for (i = 0; i < DOMAIN_MAX_NOC; i++) {
> + if (!data->noc_data[i])
> + continue;

With

const struct imx8m_blk_ctrl_noc_data *noc_data = &data->noc_data[i];

if (!noc_data)
continue;

you could then write

regmap_write(regmap, noc_data->off + 0x8, noc_data->priority);
regmap_write(regmap, noc_data->off + 0xc, noc_data->mode);
regmap_write(regmap, noc_data->off + 0x18, noc_data->extctrl);

which is a bit nicer.

If we drop the harcoded number of NoC entries, the loop could become

const struct imx8m_blk_ctrl_noc_data *noc_data;

for (i = 0; data->noc_data[i]; i++) {
const struct imx8m_blk_ctrl_noc_data *noc_data = data->noc_data[i];

regmap_write(regmap, noc_data->off + 0x8, noc_data->priority);
regmap_write(regmap, noc_data->off + 0xc, noc_data->mode);
regmap_write(regmap, noc_data->off + 0x18, noc_data->extctrl);
}

You would then need a sentinel entry at the end of the noc_data array,
initialized to NULL.

> + regmap_write(regmap, data->noc_data[i]->off + 0x8, data->noc_data[i]->priority);
> + regmap_write(regmap, data->noc_data[i]->off + 0xc, data->noc_data[i]->mode);
> + regmap_write(regmap, data->noc_data[i]->off + 0x18, data->noc_data[i]->extctrl);
> + }
> +
> + return 0;
> +}
> +
> static int imx8m_blk_ctrl_power_on(struct generic_pm_domain *genpd)
> {
> struct imx8m_blk_ctrl_domain *domain = to_imx8m_blk_ctrl_domain(genpd);
> @@ -117,6 +150,8 @@ static int imx8m_blk_ctrl_power_on(struct generic_pm_domain *genpd)
> if (data->mipi_phy_rst_mask)
> regmap_set_bits(bc->regmap, BLK_MIPI_RESET_DIV, data->mipi_phy_rst_mask);
>
> + imx8m_blk_ctrl_noc_set(domain);
> +
> /* disable upstream clocks */
> clk_bulk_disable_unprepare(data->num_clks, domain->clks);
>
> @@ -172,6 +207,7 @@ static int imx8m_blk_ctrl_probe(struct platform_device *pdev)
> const struct imx8m_blk_ctrl_data *bc_data;
> struct device *dev = &pdev->dev;
> struct imx8m_blk_ctrl *bc;
> + struct regmap *regmap;
> void __iomem *base;
> int i, ret;
>
> @@ -218,6 +254,10 @@ static int imx8m_blk_ctrl_probe(struct platform_device *pdev)
> return dev_err_probe(dev, PTR_ERR(bc->bus_power_dev),
> "failed to attach power domain\n");
>
> + regmap = syscon_regmap_lookup_by_compatible("fsl,imx8m-noc");
> + if (!IS_ERR(regmap))
> + bc->noc_regmap = regmap;
> +
> for (i = 0; i < bc_data->num_domains; i++) {
> const struct imx8m_blk_ctrl_domain_data *data = &bc_data->domains[i];
> struct imx8m_blk_ctrl_domain *domain = &bc->domains[i];
> @@ -677,6 +717,55 @@ static int imx8mp_media_power_notifier(struct notifier_block *nb,
> return NOTIFY_OK;
> }
>
> +#define IMX8MP_MEDIABLK_LCDIF_RD 0
> +#define IMX8MP_MEDIABLK_LCDIF_WR 1
> +#define IMX8MP_MEDIABLK_ISI0 2
> +#define IMX8MP_MEDIABLK_ISI1 3
> +#define IMX8MP_MEDIABLK_ISI2 4

Would these be the two AXI write and the AXI read ports of the ISI ?
Could they then be named accordingly (e.g. ISI_WR0, ISI_WR1, ISI_RD) ? I
don't know which is which though.

> +#define IMX8MP_MEDIABLK_ISP0 5
> +#define IMX8MP_MEDIABLK_ISP1 6
> +#define IMX8MP_MEDIABLK_DWE 7
> +
> +static const struct imx8m_blk_ctrl_noc_data imx8mp_media_noc_data[] = {
> + [IMX8MP_MEDIABLK_LCDIF_RD] = {
> + .off = 0x980,
> + .priority = 0x80000202,
> + .extctrl = 1,
> + },
> + [IMX8MP_MEDIABLK_LCDIF_WR] = {
> + .off = 0xa00,
> + .priority = 0x80000202,
> + .extctrl = 1,
> + },
> + [IMX8MP_MEDIABLK_ISI0] = {
> + .off = 0xa80,
> + .priority = 0x80000202,
> + .extctrl = 1,
> + },
> + [IMX8MP_MEDIABLK_ISI1] = {
> + .off = 0xb00,
> + .priority = 0x80000202,
> + .extctrl = 1,
> + },
> + [IMX8MP_MEDIABLK_ISI2] = {
> + .off = 0xb80,
> + .priority = 0x80000202,
> + .extctrl = 1,
> + },
> + [IMX8MP_MEDIABLK_ISP0] = {
> + .off = 0xc00,
> + .priority = 0x80000707,
> + },
> + [IMX8MP_MEDIABLK_ISP1] = {
> + .off = 0xc80,
> + .priority = 0x80000707,
> + },
> + [IMX8MP_MEDIABLK_DWE] = {
> + .off = 0xd00,
> + .priority = 0x80000707,
> + },

This matches the TF-A implementation.

With the above comments taken into account,

Reviewed-by: Laurent Pinchart <[email protected]>

> +};
> +
> /*
> * From i.MX 8M Plus Applications Processor Reference Manual, Rev. 1,
> * section 13.2.2, 13.2.3
> @@ -708,6 +797,10 @@ static const struct imx8m_blk_ctrl_domain_data imx8mp_media_blk_ctl_domain_data[
> .gpc_name = "lcdif1",
> .rst_mask = BIT(4) | BIT(5) | BIT(23),
> .clk_mask = BIT(4) | BIT(5) | BIT(23),
> + .noc_data = {
> + &imx8mp_media_noc_data[IMX8MP_MEDIABLK_LCDIF_RD],
> + &imx8mp_media_noc_data[IMX8MP_MEDIABLK_LCDIF_WR],
> + },
> },
> [IMX8MP_MEDIABLK_PD_ISI] = {
> .name = "mediablk-isi",
> @@ -716,6 +809,11 @@ static const struct imx8m_blk_ctrl_domain_data imx8mp_media_blk_ctl_domain_data[
> .gpc_name = "isi",
> .rst_mask = BIT(6) | BIT(7),
> .clk_mask = BIT(6) | BIT(7),
> + .noc_data = {
> + &imx8mp_media_noc_data[IMX8MP_MEDIABLK_ISI0],
> + &imx8mp_media_noc_data[IMX8MP_MEDIABLK_ISI1],
> + &imx8mp_media_noc_data[IMX8MP_MEDIABLK_ISI2],
> + },
> },
> [IMX8MP_MEDIABLK_PD_MIPI_CSI2_2] = {
> .name = "mediablk-mipi-csi2-2",
> @@ -733,6 +831,10 @@ static const struct imx8m_blk_ctrl_domain_data imx8mp_media_blk_ctl_domain_data[
> .gpc_name = "lcdif2",
> .rst_mask = BIT(11) | BIT(12) | BIT(24),
> .clk_mask = BIT(11) | BIT(12) | BIT(24),
> + .noc_data = {
> + &imx8mp_media_noc_data[IMX8MP_MEDIABLK_LCDIF_RD],
> + &imx8mp_media_noc_data[IMX8MP_MEDIABLK_LCDIF_WR],
> + },
> },
> [IMX8MP_MEDIABLK_PD_ISP2] = {
> .name = "mediablk-isp2",
> @@ -749,6 +851,10 @@ static const struct imx8m_blk_ctrl_domain_data imx8mp_media_blk_ctl_domain_data[
> .gpc_name = "isp1",
> .rst_mask = BIT(16) | BIT(17) | BIT(18),
> .clk_mask = BIT(16) | BIT(17) | BIT(18),
> + .noc_data = {
> + &imx8mp_media_noc_data[IMX8MP_MEDIABLK_ISP0],
> + &imx8mp_media_noc_data[IMX8MP_MEDIABLK_ISP1],
> + },
> },
> [IMX8MP_MEDIABLK_PD_DWE] = {
> .name = "mediablk-dwe",
> @@ -757,6 +863,9 @@ static const struct imx8m_blk_ctrl_domain_data imx8mp_media_blk_ctl_domain_data[
> .gpc_name = "dwe",
> .rst_mask = BIT(19) | BIT(20) | BIT(21),
> .clk_mask = BIT(19) | BIT(20) | BIT(21),
> + .noc_data = {
> + &imx8mp_media_noc_data[IMX8MP_MEDIABLK_DWE],
> + },
> },
> [IMX8MP_MEDIABLK_PD_MIPI_DSI_2] = {
> .name = "mediablk-mipi-dsi-2",

--
Regards,

Laurent Pinchart

2022-04-06 15:50:29

by Lucas Stach

[permalink] [raw]
Subject: Re: [PATCH 0/5] imx: support noc settings with power domain

Hi Peng,

Am Mittwoch, dem 06.04.2022 um 16:23 +0800 schrieb Peng Fan (OSS):
> From: Peng Fan <[email protected]>
>
> i.MX8MP has a design that NoC(Not main NoC) is distributed in multiple
> blocks, such as vpumix, hsiomix and etc. The access to NoC requires
> power domain on and blk ctrl settings configured.
>
> So the design here is for mixes that not have blk-ctrl, configure
> the NoC in gpcv2 driver, for mixes that have blk-ctrl, configure
> the NoC in blk-ctrl drivers.
>
> This v1 patchset not apply on Shawn's tree, I picked up Lucas's HSIO
> and Laurent's mediablk patches, then worked out this patchset:
> https://github.com/MrVan/linux/tree/noc-imx8mp
>
> Note: This interconnect related functions not added. This patchset
> is only to replace the function did in NXP downstream:
> https://source.codeaurora.org/external/imx/imx-atf/tree/plat/imx/imx8m/imx8mp/gpc.c?h=lf_v2.4#n157

As a general comment I think this is implemented the wrong way around.

Neither GPC, nor the blk-ctrl should poke into the NoC registers
directly. The NoC driver should attach itself to the power domain via a
notifier (same as the blk-ctrl does with the GPC domains) and should do
the necessary NoC configuration when the power domain is powered up.

Regards,
Lucas
>
> Peng Fan (5):
> dt-bindings: interconnect: imx8m: Add bindings for imx8mp noc
> arm64: dts: imx8mp: add noc node
> soc: imx: gpcv2: support i.MX8MP NoC settings
> soc: imx: imx8m-blk-ctrl: support i.MX8MP media blk ctrl noc settings
> soc: imx: imx8mp-blk-ctrl: introduce HSIO blk ctrl noc settings
>
> .../bindings/interconnect/fsl,imx8m-noc.yaml | 6 +
> arch/arm64/boot/dts/freescale/imx8mp.dtsi | 7 ++
> drivers/soc/imx/gpcv2.c | 56 ++++++++-
> drivers/soc/imx/imx8m-blk-ctrl.c | 109 ++++++++++++++++++
> drivers/soc/imx/imx8mp-blk-ctrl.c | 74 ++++++++++++
> 5 files changed, 251 insertions(+), 1 deletion(-)
>