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

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

[Peng Fan (OSS)]

From: Peng Fan <peng.fan@nxp.com>

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

[PATCH 1/5] dt-bindings: interconnect: imx8m: Add bindings for imx8mp noc

[Peng Fan (OSS)]

From: Peng Fan <peng.fan@nxp.com>

i.MX8MP features same NoC/NIC as i.MX8MM/N/Q, and use two compatible
strings.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 .../devicetree/bindings/interconnect/fsl,imx8m-noc.yaml     | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/interconnect/fsl,imx8m-noc.yaml b/Documentation/devicetree/bindings/interconnect/fsl,imx8m-noc.yaml
index b8204ed22dd5..0923cd28d6c6 100644
--- a/Documentation/devicetree/bindings/interconnect/fsl,imx8m-noc.yaml
+++ b/Documentation/devicetree/bindings/interconnect/fsl,imx8m-noc.yaml
@@ -26,16 +26,22 @@ properties:
     oneOf:
       - items:
           - enum:
+              - fsl,imx8mp-nic
               - fsl,imx8mn-nic
               - fsl,imx8mm-nic
               - fsl,imx8mq-nic
           - const: fsl,imx8m-nic
       - items:
           - enum:
+              - fsl,imx8mp-noc
               - fsl,imx8mn-noc
               - fsl,imx8mm-noc
               - fsl,imx8mq-noc
           - const: fsl,imx8m-noc
+      - items:
+          - const: fsl,imx8mp-noc
+          - const: fsl,imx8m-noc
+          - const: syscon
       - const: fsl,imx8m-nic
 
   reg:
-- 
2.25.1

Re: [PATCH 1/5] dt-bindings: interconnect: imx8m: Add bindings for imx8mp noc

[Laurent Pinchart]

Hi Peng,

Thank you for the patch.

On Wed, Apr 06, 2022 at 04:23:26PM +0800, Peng Fan (OSS) wrote:
> From: Peng Fan <peng.fan@nxp.com>
> 
> i.MX8MP features same NoC/NIC as i.MX8MM/N/Q, and use two compatible
> strings.
> 
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
>  .../devicetree/bindings/interconnect/fsl,imx8m-noc.yaml     | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/interconnect/fsl,imx8m-noc.yaml b/Documentation/devicetree/bindings/interconnect/fsl,imx8m-noc.yaml
> index b8204ed22dd5..0923cd28d6c6 100644
> --- a/Documentation/devicetree/bindings/interconnect/fsl,imx8m-noc.yaml
> +++ b/Documentation/devicetree/bindings/interconnect/fsl,imx8m-noc.yaml
> @@ -26,16 +26,22 @@ properties:
>      oneOf:
>        - items:
>            - enum:
> +              - fsl,imx8mp-nic
>                - fsl,imx8mn-nic
>                - fsl,imx8mm-nic
>                - fsl,imx8mq-nic

Alphabetical order would be nice here, maybe you could sort those
entries while at it ?

>            - const: fsl,imx8m-nic

Why do we need both -nic and -noc versions of the compatible string btw
? The imx-bus driver matches on

	{ .compatible = "fsl,imx8mq-noc", .data = "imx8mq-interconnect", },
	{ .compatible = "fsl,imx8mm-noc", .data = "imx8mm-interconnect", },
	{ .compatible = "fsl,imx8mn-noc", .data = "imx8mn-interconnect", },
	{ .compatible = "fsl,imx8m-noc", },
	{ .compatible = "fsl,imx8m-nic", },

>        - items:
>            - enum:
> +              - fsl,imx8mp-noc
>                - fsl,imx8mn-noc
>                - fsl,imx8mm-noc
>                - fsl,imx8mq-noc

Same here.

>            - const: fsl,imx8m-noc
> +      - items:
> +          - const: fsl,imx8mp-noc
> +          - const: fsl,imx8m-noc
> +          - const: syscon

Do we want to support both

	compatible = "fsl,imx8mp-noc", "fsl,imx8m-noc";

and

	compatible = "fsl,imx8mp-noc", "fsl,imx8m-noc", "syscon";

or we can pick one of the two (the latter one in that case I suppose) ?

>        - const: fsl,imx8m-nic
>  
>    reg:

-- 
Regards,

Laurent Pinchart

RE: [PATCH 1/5] dt-bindings: interconnect: imx8m: Add bindings for imx8mp noc

[Peng Fan]

> Subject: Re: [PATCH 1/5] dt-bindings: interconnect: imx8m: Add bindings for
> imx8mp noc
> 
> Hi Peng,
> 
> Thank you for the patch.
> 
> On Wed, Apr 06, 2022 at 04:23:26PM +0800, Peng Fan (OSS) wrote:
> > From: Peng Fan <peng.fan@nxp.com>
> >
> > i.MX8MP features same NoC/NIC as i.MX8MM/N/Q, and use two
> compatible
> > strings.
> >
> > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > ---
> >  .../devicetree/bindings/interconnect/fsl,imx8m-noc.yaml     | 6 ++++++
> >  1 file changed, 6 insertions(+)
> >
> > diff --git
> > a/Documentation/devicetree/bindings/interconnect/fsl,imx8m-noc.yaml
> > b/Documentation/devicetree/bindings/interconnect/fsl,imx8m-noc.yaml
> > index b8204ed22dd5..0923cd28d6c6 100644
> > ---
> > a/Documentation/devicetree/bindings/interconnect/fsl,imx8m-noc.yaml
> > +++
> b/Documentation/devicetree/bindings/interconnect/fsl,imx8m-noc.yam
> > +++ l
> > @@ -26,16 +26,22 @@ properties:
> >      oneOf:
> >        - items:
> >            - enum:
> > +              - fsl,imx8mp-nic
> >                - fsl,imx8mn-nic
> >                - fsl,imx8mm-nic
> >                - fsl,imx8mq-nic
> 
> Alphabetical order would be nice here, maybe you could sort those entries
> while at it ?

Sure.

> 
> >            - const: fsl,imx8m-nic
> 
> Why do we need both -nic and -noc versions of the compatible string btw ?
> The imx-bus driver matches on

There is NoC and PL301 NIC in i.MX8M*.

> 
> 	{ .compatible = "fsl,imx8mq-noc", .data = "imx8mq-interconnect", },
> 	{ .compatible = "fsl,imx8mm-noc", .data = "imx8mm-interconnect", },
> 	{ .compatible = "fsl,imx8mn-noc", .data = "imx8mn-interconnect", },
> 	{ .compatible = "fsl,imx8m-noc", },
> 	{ .compatible = "fsl,imx8m-nic", },
> 
> >        - items:
> >            - enum:
> > +              - fsl,imx8mp-noc
> >                - fsl,imx8mn-noc
> >                - fsl,imx8mm-noc
> >                - fsl,imx8mq-noc
> 
> Same here.
> 
> >            - const: fsl,imx8m-noc
> > +      - items:
> > +          - const: fsl,imx8mp-noc
> > +          - const: fsl,imx8m-noc
> > +          - const: syscon
> 
> Do we want to support both
> 
> 	compatible = "fsl,imx8mp-noc", "fsl,imx8m-noc";
> 
> and
> 
> 	compatible = "fsl,imx8mp-noc", "fsl,imx8m-noc", "syscon";
> 
> or we can pick one of the two (the latter one in that case I suppose) ?

Latter one should be ok.

Thanks
Peng.

> 
> >        - const: fsl,imx8m-nic
> >
> >    reg:
> 
> --
> Regards,
> 
> Laurent Pinchart

[PATCH 2/5] arm64: dts: imx8mp: add noc node

[Peng Fan (OSS)]

From: Peng Fan <peng.fan@nxp.com>

Add i.MX8MP main noc node

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 arch/arm64/boot/dts/freescale/imx8mp.dtsi | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
index e9e55fdd7652..be902f8155e8 100644
--- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
@@ -1015,6 +1015,13 @@ gpu2d: gpu@38008000 {
 			power-domains = <&pgc_gpu2d>;
 		};
 
+		noc: interconnect@32700000 {
+			compatible = "fsl,imx8mp-noc", "fsl,imx8m-noc", "syscon";
+			reg = <0x32700000 0x100000>;
+			clocks = <&clk IMX8MP_CLK_NOC>;
+			#interconnect-cells = <1>;
+		};
+
 		aips4 {
 			compatible = "fsl,aips-bus", "simple-bus";
 			reg = <0x32c00000 0x400000>;
-- 
2.25.1

Re: [PATCH 2/5] arm64: dts: imx8mp: add noc node

[Laurent Pinchart]

Hi Peng,

Thank you for the patch.

On Wed, Apr 06, 2022 at 04:23:27PM +0800, Peng Fan (OSS) wrote:
> From: Peng Fan <peng.fan@nxp.com>
> 
> Add i.MX8MP main noc node
> 
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
>  arch/arm64/boot/dts/freescale/imx8mp.dtsi | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> index e9e55fdd7652..be902f8155e8 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> @@ -1015,6 +1015,13 @@ gpu2d: gpu@38008000 {
>  			power-domains = <&pgc_gpu2d>;
>  		};
>  
> +		noc: interconnect@32700000 {
> +			compatible = "fsl,imx8mp-noc", "fsl,imx8m-noc", "syscon";
> +			reg = <0x32700000 0x100000>;

I can't comment on this, as the memory is documented as reserved in the
reference manual, but I have no reason not to trust you :-)

> +			clocks = <&clk IMX8MP_CLK_NOC>;

There's also a NOC_WRAPPER clock documented in the reference manual, and
also a NOC_IO clock. Are those related, do we need to care about them ?

> +			#interconnect-cells = <1>;
> +		};
> +
>  		aips4 {
>  			compatible = "fsl,aips-bus", "simple-bus";
>  			reg = <0x32c00000 0x400000>;

-- 
Regards,

Laurent Pinchart

RE: [PATCH 2/5] arm64: dts: imx8mp: add noc node

[Peng Fan]

> Subject: Re: [PATCH 2/5] arm64: dts: imx8mp: add noc node
> 
> Hi Peng,
> 
> Thank you for the patch.
> 
> On Wed, Apr 06, 2022 at 04:23:27PM +0800, Peng Fan (OSS) wrote:
> > From: Peng Fan <peng.fan@nxp.com>
> >
> > Add i.MX8MP main noc node
> >
> > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > ---
> >  arch/arm64/boot/dts/freescale/imx8mp.dtsi | 7 +++++++
> >  1 file changed, 7 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> > b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> > index e9e55fdd7652..be902f8155e8 100644
> > --- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> > +++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> > @@ -1015,6 +1015,13 @@ gpu2d: gpu@38008000 {
> >  			power-domains = <&pgc_gpu2d>;
> >  		};
> >
> > +		noc: interconnect@32700000 {
> > +			compatible = "fsl,imx8mp-noc", "fsl,imx8m-noc", "syscon";
> > +			reg = <0x32700000 0x100000>;
> 
> I can't comment on this, as the memory is documented as reserved in the
> reference manual, but I have no reason not to trust you :-)

thanks. Sadly, I have asked, but the NoC doc will not be public.

> 
> > +			clocks = <&clk IMX8MP_CLK_NOC>;
> 
> There's also a NOC_WRAPPER clock documented in the reference manual,
> and also a NOC_IO clock. Are those related, do we need to care about them ?

Thanks for pointing this out, yes, we need NOC_IO clk.
But since NOC and NOC_IO clk are both critical clk, I not met issue :)

Thanks,
Peng.

> 
> > +			#interconnect-cells = <1>;
> > +		};
> > +
> >  		aips4 {
> >  			compatible = "fsl,aips-bus", "simple-bus";
> >  			reg = <0x32c00000 0x400000>;
> 
> --
> Regards,
> 
> Laurent Pinchart

[PATCH 3/5] soc: imx: gpcv2: support i.MX8MP NoC settings

[Peng Fan (OSS)]

From: Peng Fan <peng.fan@nxp.com>

The NoC reset value for GPU and MLMIX is not valid, need to set
it to a valid value after power up.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/soc/imx/gpcv2.c | 56 ++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 55 insertions(+), 1 deletion(-)

diff --git a/drivers/soc/imx/gpcv2.c b/drivers/soc/imx/gpcv2.c
index 85aa86e1338a..7199cf8e148e 100644
--- a/drivers/soc/imx/gpcv2.c
+++ b/drivers/soc/imx/gpcv2.c
@@ -9,6 +9,7 @@
  */
 
 #include <linux/clk.h>
+#include <linux/mfd/syscon.h>
 #include <linux/of_device.h>
 #include <linux/platform_device.h>
 #include <linux/pm_domain.h>
@@ -274,9 +275,17 @@ struct imx_pgc_regs {
 	u16 hsk;
 };
 
+struct imx_pgc_noc_data {
+	u32 off;
+	u32 priority;
+	u32 mode;
+	u32 extctrl;
+};
+
 struct imx_pgc_domain {
 	struct generic_pm_domain genpd;
 	struct regmap *regmap;
+	struct regmap *noc_regmap;
 	const struct imx_pgc_regs *regs;
 	struct regulator *regulator;
 	struct reset_control *reset;
@@ -298,6 +307,7 @@ struct imx_pgc_domain {
 
 	unsigned int pgc_sw_pup_reg;
 	unsigned int pgc_sw_pdn_reg;
+	const struct imx_pgc_noc_data *noc_data;
 };
 
 struct imx_pgc_domain_data {
@@ -313,6 +323,21 @@ to_imx_pgc_domain(struct generic_pm_domain *genpd)
 	return container_of(genpd, struct imx_pgc_domain, genpd);
 }
 
+static int imx_pgc_noc_set(struct imx_pgc_domain *domain)
+{
+	const struct imx_pgc_noc_data *data = domain->noc_data;
+	struct regmap *regmap = domain->noc_regmap;
+
+	if (!data || !regmap)
+		return 0;
+
+	regmap_write(regmap, data->off + 0x8, data->priority);
+	regmap_write(regmap, data->off + 0xc, data->mode);
+	regmap_write(regmap, data->off + 0x18, data->extctrl);
+
+	return 0;
+}
+
 static int imx_pgc_power_up(struct generic_pm_domain *genpd)
 {
 	struct imx_pgc_domain *domain = to_imx_pgc_domain(genpd);
@@ -394,6 +419,8 @@ static int imx_pgc_power_up(struct generic_pm_domain *genpd)
 	if (!domain->keep_clocks)
 		clk_bulk_disable_unprepare(domain->num_clks, domain->clks);
 
+	imx_pgc_noc_set(domain);
+
 	return 0;
 
 out_clk_disable:
@@ -911,6 +938,25 @@ static const struct imx_pgc_domain_data imx8mm_pgc_domain_data = {
 	.pgc_regs = &imx7_pgc_regs,
 };
 
+#define IMX8MP_MLMIX	0
+#define IMX8MP_GPU2D	1
+#define IMX8MP_GPU3D	2
+
+static const struct imx_pgc_noc_data imx8mp_pgc_noc_data[] = {
+	[IMX8MP_MLMIX] = {
+		.off = 0x180,
+		.priority = 0x80000303,
+	},
+	[IMX8MP_GPU2D] = {
+		.off = 0x500,
+		.priority = 0x80000303,
+	},
+	[IMX8MP_GPU3D] = {
+		.off = 0x580,
+		.priority = 0x80000303,
+	},
+};
+
 static const struct imx_pgc_domain imx8mp_pgc_domains[] = {
 	[IMX8MP_POWER_DOMAIN_MIPI_PHY1] = {
 		.genpd = {
@@ -968,6 +1014,7 @@ static const struct imx_pgc_domain imx8mp_pgc_domains[] = {
 		},
 		.pgc = BIT(IMX8MP_PGC_MLMIX),
 		.keep_clocks = true,
+		.noc_data = &imx8mp_pgc_noc_data[IMX8MP_MLMIX],
 	},
 
 	[IMX8MP_POWER_DOMAIN_AUDIOMIX] = {
@@ -993,6 +1040,7 @@ static const struct imx_pgc_domain imx8mp_pgc_domains[] = {
 			.map = IMX8MP_GPU2D_A53_DOMAIN,
 		},
 		.pgc = BIT(IMX8MP_PGC_GPU2D),
+		.noc_data = &imx8mp_pgc_noc_data[IMX8MP_GPU2D],
 	},
 
 	[IMX8MP_POWER_DOMAIN_GPUMIX] = {
@@ -1032,6 +1080,7 @@ static const struct imx_pgc_domain imx8mp_pgc_domains[] = {
 			.map = IMX8MP_GPU3D_A53_DOMAIN,
 		},
 		.pgc = BIT(IMX8MP_PGC_GPU3D),
+		.noc_data = &imx8mp_pgc_noc_data[IMX8MP_GPU3D],
 	},
 
 	[IMX8MP_POWER_DOMAIN_MEDIAMIX] = {
@@ -1440,7 +1489,7 @@ static int imx_gpcv2_probe(struct platform_device *pdev)
 	};
 	struct device *dev = &pdev->dev;
 	struct device_node *pgc_np, *np;
-	struct regmap *regmap;
+	struct regmap *regmap, *noc_regmap;
 	void __iomem *base;
 	int ret;
 
@@ -1461,6 +1510,8 @@ static int imx_gpcv2_probe(struct platform_device *pdev)
 		return ret;
 	}
 
+	noc_regmap = syscon_regmap_lookup_by_compatible("fsl,imx8m-noc");
+
 	for_each_child_of_node(pgc_np, np) {
 		struct platform_device *pd_pdev;
 		struct imx_pgc_domain *domain;
@@ -1503,6 +1554,9 @@ static int imx_gpcv2_probe(struct platform_device *pdev)
 		domain = pd_pdev->dev.platform_data;
 		domain->regmap = regmap;
 		domain->regs = domain_data->pgc_regs;
+		domain->noc_data = domain_data->domains[domain_index].noc_data;
+		if (!IS_ERR(noc_regmap))
+			domain->noc_regmap = noc_regmap;
 
 		domain->genpd.power_on  = imx_pgc_power_up;
 		domain->genpd.power_off = imx_pgc_power_down;
-- 
2.25.1

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

[Peng Fan (OSS)]

From: Peng Fan <peng.fan@nxp.com>

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 <peng.fan@nxp.com>
---
 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

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

[Laurent Pinchart]

Hi Peng,

Thank you for the patch.

On Wed, Apr 06, 2022 at 04:23:29PM +0800, Peng Fan (OSS) wrote:
> From: Peng Fan <peng.fan@nxp.com>
> 
> 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 <peng.fan@nxp.com>
> ---
>  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 <laurent.pinchart@ideasonboard.com>

> +};
> +
>  /*
>   * 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

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

[Peng Fan (OSS)]

From: Peng Fan <peng.fan@nxp.com>

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 <peng.fan@nxp.com>
---
 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

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

[Laurent Pinchart]

Hi Peng,

Thank you for the patch.

On Wed, Apr 06, 2022 at 04:23:30PM +0800, Peng Fan (OSS) wrote:
> From: Peng Fan <peng.fan@nxp.com>
> 
> 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 <peng.fan@nxp.com>
> ---
>  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

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

[Lucas Stach]

Hi Peng,

Am Mittwoch, dem 06.04.2022 um 16:23 +0800 schrieb Peng Fan (OSS):
> From: Peng Fan <peng.fan@nxp.com>
> 
> 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(-)
> 

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

[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 <peng.fan@nxp.com>
> >
> > 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(-)
> >
> 

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

[Lucas Stach]

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 <peng.fan@nxp.com>
> > > 
> > > 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