[PATCH 0/8] interconnect: support i.MX8MP

[PATCH 0/8] interconnect: support i.MX8MP

[Peng Fan (OSS)]

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

This patchset is to support i.MX8MP NoC settings, i.MX8MP NoC initial
value after power up is invalid, need set a valid value after related
power domain up.

This patchset also includes two patch[1,2] during my development to enable
the ICC feature for i.MX8MP.

I not include ddrc DVFS in this patchset, ths patchset is only to
support NoC value mode/priority/ext_control being set to a valid value
that suggested by i.MX Chip Design Team. The value is same as NXP
downstream one inside Arm Trusted Firmware:
https://source.codeaurora.org/external/imx/imx-atf/tree/plat/imx/imx8m/imx8mp/gpc.c?h=lf_v2.4#n97

A repo created here: https://github.com/MrVan/linux/tree/imx8mp-interconnect

Peng Fan (8):
  dt-bindings: interconnect: imx8m: Add bindings for imx8mp noc
  interconnect: add device managed bulk API
  interconnect: imx: fix max_node_id
  interconnect: imx: set src node
  interconnect: imx: introduce imx_icc_provider
  interconnect: imx: set of_node for interconnect provider
  interconnect: imx: configure NoC mode/prioriry/ext_control
  interconnect: imx: Add platform driver for imx8mp

 .../bindings/interconnect/fsl,imx8m-noc.yaml  |   6 +
 drivers/interconnect/bulk.c                   |  34 +++
 drivers/interconnect/imx/Kconfig              |   4 +
 drivers/interconnect/imx/Makefile             |   2 +
 drivers/interconnect/imx/imx.c                |  68 +++--
 drivers/interconnect/imx/imx.h                |  25 +-
 drivers/interconnect/imx/imx8mm.c             |   2 +-
 drivers/interconnect/imx/imx8mn.c             |   2 +-
 drivers/interconnect/imx/imx8mp.c             | 232 ++++++++++++++++++
 drivers/interconnect/imx/imx8mq.c             |   2 +-
 include/dt-bindings/interconnect/fsl,imx8mp.h |  59 +++++
 include/linux/interconnect.h                  |   6 +
 12 files changed, 424 insertions(+), 18 deletions(-)
 create mode 100644 drivers/interconnect/imx/imx8mp.c
 create mode 100644 include/dt-bindings/interconnect/fsl,imx8mp.h

-- 
2.25.1

[PATCH 1/8] 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/8] dt-bindings: interconnect: imx8m: Add bindings for imx8mp noc

[Krzysztof Kozlowski]

On 01/06/2022 11:41, 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

Please order the entries alphabetically, so 8mp goes after 8mm.

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

ditto

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

This is a bit confusing - why this is also fallbacked as syscon?

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


Best regards,
Krzysztof

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

[Peng Fan]

> Subject: Re: [PATCH 1/8] dt-bindings: interconnect: imx8m: Add bindings for
> imx8mp noc
> 
> On 01/06/2022 11:41, 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
> 
> Please order the entries alphabetically, so 8mp goes after 8mm.
> 
> >                - fsl,imx8mn-nic
> >                - fsl,imx8mm-nic
> >                - fsl,imx8mq-nic
> >            - const: fsl,imx8m-nic
> >        - items:
> >            - enum:
> > +              - fsl,imx8mp-noc
> 
> ditto
> 
> >                - fsl,imx8mn-noc
> >                - fsl,imx8mm-noc
> >                - fsl,imx8mq-noc
> >            - const: fsl,imx8m-noc
> > +      - items:
> > +          - const: fsl,imx8mp-noc
> > +          - const: fsl,imx8m-noc
> > +          - const: syscon
> 
> This is a bit confusing - why this is also fallbacked as syscon?

I thought to give some flexibility for drivers to access the
address through syscon. But it could be removed, I could
fix in V2.

Thanks,
Peng. 

> 
> >        - const: fsl,imx8m-nic
> >
> >    reg:
> 
> 
> Best regards,
> Krzysztof

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

[Krzysztof Kozlowski]

On 01/06/2022 14:06, Peng Fan wrote:
>> Subject: Re: [PATCH 1/8] dt-bindings: interconnect: imx8m: Add bindings for
>> imx8mp noc
>>
>> On 01/06/2022 11:41, 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
>>
>> Please order the entries alphabetically, so 8mp goes after 8mm.
>>
>>>                - fsl,imx8mn-nic
>>>                - fsl,imx8mm-nic
>>>                - fsl,imx8mq-nic
>>>            - const: fsl,imx8m-nic
>>>        - items:
>>>            - enum:
>>> +              - fsl,imx8mp-noc
>>
>> ditto
>>
>>>                - fsl,imx8mn-noc
>>>                - fsl,imx8mm-noc
>>>                - fsl,imx8mq-noc
>>>            - const: fsl,imx8m-noc
>>> +      - items:
>>> +          - const: fsl,imx8mp-noc
>>> +          - const: fsl,imx8m-noc
>>> +          - const: syscon
>>
>> This is a bit confusing - why this is also fallbacked as syscon?
> 
> I thought to give some flexibility for drivers to access the
> address through syscon. But it could be removed, I could
> fix in V2.

Please remove, unless you really need it. No one should poke someone's
else registers in general :)


Best regards,
Krzysztof

[PATCH 2/8] interconnect: add device managed bulk API

[Peng Fan (OSS)]

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

Add device managed bulk API to simplify driver.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/interconnect/bulk.c  | 34 ++++++++++++++++++++++++++++++++++
 include/linux/interconnect.h |  6 ++++++
 2 files changed, 40 insertions(+)

diff --git a/drivers/interconnect/bulk.c b/drivers/interconnect/bulk.c
index 448cc536aa79..4918844bfe0d 100644
--- a/drivers/interconnect/bulk.c
+++ b/drivers/interconnect/bulk.c
@@ -115,3 +115,37 @@ void icc_bulk_disable(int num_paths, const struct icc_bulk_data *paths)
 		icc_disable(paths[num_paths].path);
 }
 EXPORT_SYMBOL_GPL(icc_bulk_disable);
+
+struct icc_bulk_devres {
+	struct icc_bulk_data *paths;
+	int num_paths;
+};
+
+static void devm_icc_bulk_release(struct device *dev, void *res)
+{
+	struct icc_bulk_devres *devres = res;
+
+	icc_bulk_put(devres->num_paths, devres->paths);
+}
+
+int devm_of_icc_bulk_get(struct device *dev, int num_paths, struct icc_bulk_data *paths)
+{
+	struct icc_bulk_devres *devres;
+	int ret;
+
+	devres = devres_alloc(devm_icc_bulk_release, sizeof(*devres), GFP_KERNEL);
+	if (!devres)
+		return -ENOMEM;
+
+	ret = of_icc_bulk_get(dev, num_paths, paths);
+	if (!ret) {
+		devres->paths = paths;
+		devres->num_paths = num_paths;
+		devres_add(dev, devres);
+	} else {
+		devres_free(devres);
+	}
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(devm_of_icc_bulk_get);
diff --git a/include/linux/interconnect.h b/include/linux/interconnect.h
index f685777b875e..1a5fdf049edd 100644
--- a/include/linux/interconnect.h
+++ b/include/linux/interconnect.h
@@ -44,6 +44,7 @@ struct icc_path *icc_get(struct device *dev, const int src_id,
 			 const int dst_id);
 struct icc_path *of_icc_get(struct device *dev, const char *name);
 struct icc_path *devm_of_icc_get(struct device *dev, const char *name);
+int devm_of_icc_bulk_get(struct device *dev, int num_paths, struct icc_bulk_data *paths);
 struct icc_path *of_icc_get_by_index(struct device *dev, int idx);
 void icc_put(struct icc_path *path);
 int icc_enable(struct icc_path *path);
@@ -116,6 +117,11 @@ static inline int of_icc_bulk_get(struct device *dev, int num_paths, struct icc_
 	return 0;
 }
 
+int devm_of_icc_bulk_get(struct device *dev, int num_paths, struct icc_bulk_data *paths)
+{
+	return 0;
+}
+
 static inline void icc_bulk_put(int num_paths, struct icc_bulk_data *paths)
 {
 }
-- 
2.25.1

Re: [PATCH 2/8] interconnect: add device managed bulk API

[kernel test robot]

Hi "Peng,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on shawnguo/for-next]
[also build test WARNING on robh/for-next linus/master v5.18 next-20220601]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/intel-lab-lkp/linux/commits/Peng-Fan-OSS/interconnect-support-i-MX8MP/20220601-174431
base:   https://git.kernel.org/pub/scm/linux/kernel/git/shawnguo/linux.git for-next
config: hexagon-randconfig-r041-20220531 (https://download.01.org/0day-ci/archive/20220601/202206012000.wFYpOdWH-lkp@intel.com/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project c825abd6b0198fb088d9752f556a70705bc99dfd)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/23ecbba75b21962f25975cb014cf981a0420dae1
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Peng-Fan-OSS/interconnect-support-i-MX8MP/20220601-174431
        git checkout 23ecbba75b21962f25975cb014cf981a0420dae1
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=hexagon SHELL=/bin/bash drivers/opp/

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   In file included from drivers/opp/core.c:22:
   In file included from drivers/opp/opp.h:15:
>> include/linux/interconnect.h:120:5: warning: no previous prototype for function 'devm_of_icc_bulk_get' [-Wmissing-prototypes]
   int devm_of_icc_bulk_get(struct device *dev, int num_paths, struct icc_bulk_data *paths)
       ^
   include/linux/interconnect.h:120:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
   int devm_of_icc_bulk_get(struct device *dev, int num_paths, struct icc_bulk_data *paths)
   ^
   static 
   1 warning generated.


vim +/devm_of_icc_bulk_get +120 include/linux/interconnect.h

   119	
 > 120	int devm_of_icc_bulk_get(struct device *dev, int num_paths, struct icc_bulk_data *paths)
   121	{
   122		return 0;
   123	}
   124	

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

Re: [PATCH 2/8] interconnect: add device managed bulk API

[kernel test robot]

Hi "Peng,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on shawnguo/for-next]
[also build test ERROR on robh/for-next linus/master v5.18 next-20220601]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/intel-lab-lkp/linux/commits/Peng-Fan-OSS/interconnect-support-i-MX8MP/20220601-174431
base:   https://git.kernel.org/pub/scm/linux/kernel/git/shawnguo/linux.git for-next
config: x86_64-kexec (https://download.01.org/0day-ci/archive/20220601/202206012137.3VySlLwf-lkp@intel.com/config)
compiler: gcc-11 (Debian 11.3.0-1) 11.3.0
reproduce (this is a W=1 build):
        # https://github.com/intel-lab-lkp/linux/commit/23ecbba75b21962f25975cb014cf981a0420dae1
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Peng-Fan-OSS/interconnect-support-i-MX8MP/20220601-174431
        git checkout 23ecbba75b21962f25975cb014cf981a0420dae1
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        make W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   ld: drivers/opp/cpu.o: in function `devm_of_icc_bulk_get':
>> cpu.c:(.text+0x2ad): multiple definition of `devm_of_icc_bulk_get'; drivers/opp/core.o:core.c:(.text+0xa4a): first defined here
   ld: drivers/opp/debugfs.o: in function `devm_of_icc_bulk_get':
   debugfs.c:(.text+0x13c): multiple definition of `devm_of_icc_bulk_get'; drivers/opp/core.o:core.c:(.text+0xa4a): first defined here

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

Re: [PATCH 2/8] interconnect: add device managed bulk API

[kernel test robot]

Hi "Peng,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on shawnguo/for-next]
[also build test ERROR on robh/for-next linus/master v5.18 next-20220601]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/intel-lab-lkp/linux/commits/Peng-Fan-OSS/interconnect-support-i-MX8MP/20220601-174431
base:   https://git.kernel.org/pub/scm/linux/kernel/git/shawnguo/linux.git for-next
config: riscv-randconfig-r042-20220531 (https://download.01.org/0day-ci/archive/20220601/202206012118.zBGUugaB-lkp@intel.com/config)
compiler: riscv64-linux-gcc (GCC) 11.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/23ecbba75b21962f25975cb014cf981a0420dae1
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Peng-Fan-OSS/interconnect-support-i-MX8MP/20220601-174431
        git checkout 23ecbba75b21962f25975cb014cf981a0420dae1
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.3.0 make.cross W=1 O=build_dir ARCH=riscv SHELL=/bin/bash

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   riscv64-linux-ld: drivers/opp/core.o: in function `devm_of_icc_bulk_get':
>> include/linux/interconnect.h:121: multiple definition of `devm_of_icc_bulk_get'; drivers/usb/dwc3/dwc3-qcom.o:include/linux/interconnect.h:121: first defined here
   riscv64-linux-ld: drivers/opp/cpu.o: in function `devm_of_icc_bulk_get':
>> include/linux/interconnect.h:121: multiple definition of `devm_of_icc_bulk_get'; drivers/usb/dwc3/dwc3-qcom.o:include/linux/interconnect.h:121: first defined here
   riscv64-linux-ld: drivers/opp/of.o: in function `devm_of_icc_bulk_get':
>> include/linux/interconnect.h:121: multiple definition of `devm_of_icc_bulk_get'; drivers/usb/dwc3/dwc3-qcom.o:include/linux/interconnect.h:121: first defined here
   riscv64-linux-ld: drivers/opp/debugfs.o: in function `devm_of_icc_bulk_get':
>> include/linux/interconnect.h:121: multiple definition of `devm_of_icc_bulk_get'; drivers/usb/dwc3/dwc3-qcom.o:include/linux/interconnect.h:121: first defined here

Kconfig warnings: (for reference only)
   WARNING: unmet direct dependencies detected for DRM_GEM_SHMEM_HELPER
   Depends on HAS_IOMEM && DRM && MMU
   Selected by
   - DRM_SSD130X && HAS_IOMEM && DRM


vim +121 include/linux/interconnect.h

   119	
   120	int devm_of_icc_bulk_get(struct device *dev, int num_paths, struct icc_bulk_data *paths)
 > 121	{
   122		return 0;
   123	}
   124	

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

Re: [PATCH 2/8] interconnect: add device managed bulk API

[kernel test robot]

Hi "Peng,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on shawnguo/for-next]
[also build test WARNING on robh/for-next linus/master v5.18 next-20220601]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/intel-lab-lkp/linux/commits/Peng-Fan-OSS/interconnect-support-i-MX8MP/20220601-174431
base:   https://git.kernel.org/pub/scm/linux/kernel/git/shawnguo/linux.git for-next
config: parisc-randconfig-r014-20220531 (https://download.01.org/0day-ci/archive/20220601/202206012228.VPwcFQ5b-lkp@intel.com/config)
compiler: hppa-linux-gcc (GCC) 11.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/23ecbba75b21962f25975cb014cf981a0420dae1
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Peng-Fan-OSS/interconnect-support-i-MX8MP/20220601-174431
        git checkout 23ecbba75b21962f25975cb014cf981a0420dae1
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.3.0 make.cross W=1 O=build_dir ARCH=parisc SHELL=/bin/bash drivers/opp/

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

   In file included from drivers/opp/opp.h:15,
                    from drivers/opp/core.c:22:
>> include/linux/interconnect.h:120:5: warning: no previous prototype for 'devm_of_icc_bulk_get' [-Wmissing-prototypes]
     120 | int devm_of_icc_bulk_get(struct device *dev, int num_paths, struct icc_bulk_data *paths)
         |     ^~~~~~~~~~~~~~~~~~~~


vim +/devm_of_icc_bulk_get +120 include/linux/interconnect.h

   119	
 > 120	int devm_of_icc_bulk_get(struct device *dev, int num_paths, struct icc_bulk_data *paths)
   121	{
   122		return 0;
   123	}
   124	

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

Re: [PATCH 2/8] interconnect: add device managed bulk API

[Georgi Djakov]

Hi Peng,

Thanks for the patches!

On 1.06.22 12:41, Peng Fan (OSS) wrote:
> From: Peng Fan <peng.fan@nxp.com>
> 
> Add device managed bulk API to simplify driver.
> 
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
>   drivers/interconnect/bulk.c  | 34 ++++++++++++++++++++++++++++++++++
>   include/linux/interconnect.h |  6 ++++++
>   2 files changed, 40 insertions(+)
> 
> diff --git a/drivers/interconnect/bulk.c b/drivers/interconnect/bulk.c
> index 448cc536aa79..4918844bfe0d 100644
> --- a/drivers/interconnect/bulk.c
> +++ b/drivers/interconnect/bulk.c
> @@ -115,3 +115,37 @@ void icc_bulk_disable(int num_paths, const struct icc_bulk_data *paths)
>   		icc_disable(paths[num_paths].path);
>   }
>   EXPORT_SYMBOL_GPL(icc_bulk_disable);
> +
> +struct icc_bulk_devres {
> +	struct icc_bulk_data *paths;
> +	int num_paths;
> +};
> +
> +static void devm_icc_bulk_release(struct device *dev, void *res)
> +{
> +	struct icc_bulk_devres *devres = res;
> +
> +	icc_bulk_put(devres->num_paths, devres->paths);
> +}
> +
> +int devm_of_icc_bulk_get(struct device *dev, int num_paths, struct icc_bulk_data *paths)

Adding a kerneldoc would be nice.

> +{
> +	struct icc_bulk_devres *devres;
> +	int ret;
> +
> +	devres = devres_alloc(devm_icc_bulk_release, sizeof(*devres), GFP_KERNEL);
> +	if (!devres)
> +		return -ENOMEM;
> +
> +	ret = of_icc_bulk_get(dev, num_paths, paths);
> +	if (!ret) {
> +		devres->paths = paths;
> +		devres->num_paths = num_paths;
> +		devres_add(dev, devres);
> +	} else {
> +		devres_free(devres);
> +	}
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(devm_of_icc_bulk_get);
> diff --git a/include/linux/interconnect.h b/include/linux/interconnect.h
> index f685777b875e..1a5fdf049edd 100644
> --- a/include/linux/interconnect.h
> +++ b/include/linux/interconnect.h
> @@ -44,6 +44,7 @@ struct icc_path *icc_get(struct device *dev, const int src_id,
>   			 const int dst_id);
>   struct icc_path *of_icc_get(struct device *dev, const char *name);
>   struct icc_path *devm_of_icc_get(struct device *dev, const char *name);
> +int devm_of_icc_bulk_get(struct device *dev, int num_paths, struct icc_bulk_data *paths);
>   struct icc_path *of_icc_get_by_index(struct device *dev, int idx);
>   void icc_put(struct icc_path *path);
>   int icc_enable(struct icc_path *path);
> @@ -116,6 +117,11 @@ static inline int of_icc_bulk_get(struct device *dev, int num_paths, struct icc_
>   	return 0;
>   }
>   
> +int devm_of_icc_bulk_get(struct device *dev, int num_paths, struct icc_bulk_data *paths)

Please make this static inline. The rest looks good!

Thanks,
Georgi

> +{
> +	return 0;
> +}
> +
>   static inline void icc_bulk_put(int num_paths, struct icc_bulk_data *paths)
>   {
>   }

[PATCH 3/8] interconnect: imx: fix max_node_id

[Peng Fan (OSS)]

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

max_node_id not equal to the ARRAY_SIZE of node array, need increase 1,
otherwise xlate will fail for the last entry.

Fixes: f0d8048525d7d("interconnect: Add imx core driver")
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/interconnect/imx/imx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/interconnect/imx/imx.c b/drivers/interconnect/imx/imx.c
index 249ca25d1d55..3c074933ed0c 100644
--- a/drivers/interconnect/imx/imx.c
+++ b/drivers/interconnect/imx/imx.c
@@ -238,7 +238,7 @@ int imx_icc_register(struct platform_device *pdev,
 	int ret;
 
 	/* icc_onecell_data is indexed by node_id, unlike nodes param */
-	max_node_id = get_max_node_id(nodes, nodes_count);
+	max_node_id = get_max_node_id(nodes, nodes_count) + 1;
 	data = devm_kzalloc(dev, struct_size(data, nodes, max_node_id),
 			    GFP_KERNEL);
 	if (!data)
-- 
2.25.1

Re: [PATCH 3/8] interconnect: imx: fix max_node_id

[Laurent Pinchart]

Hi Peng,

Thank you for the patch.

On Wed, Jun 01, 2022 at 05:41:51PM +0800, Peng Fan (OSS) wrote:
> From: Peng Fan <peng.fan@nxp.com>
> 
> max_node_id not equal to the ARRAY_SIZE of node array, need increase 1,
> otherwise xlate will fail for the last entry.
> 
> Fixes: f0d8048525d7d("interconnect: Add imx core driver")
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
>  drivers/interconnect/imx/imx.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/interconnect/imx/imx.c b/drivers/interconnect/imx/imx.c
> index 249ca25d1d55..3c074933ed0c 100644
> --- a/drivers/interconnect/imx/imx.c
> +++ b/drivers/interconnect/imx/imx.c
> @@ -238,7 +238,7 @@ int imx_icc_register(struct platform_device *pdev,
>  	int ret;
>  
>  	/* icc_onecell_data is indexed by node_id, unlike nodes param */
> -	max_node_id = get_max_node_id(nodes, nodes_count);
> +	max_node_id = get_max_node_id(nodes, nodes_count) + 1;

I'd rename the max_node_id variable to num_nodes, as writing

	max_node_id = get_max_node_id(...) + 1;

looks weird.

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

>  	data = devm_kzalloc(dev, struct_size(data, nodes, max_node_id),
>  			    GFP_KERNEL);
>  	if (!data)

-- 
Regards,

Laurent Pinchart

RE: [PATCH 3/8] interconnect: imx: fix max_node_id

[Peng Fan]

> Subject: Re: [PATCH 3/8] interconnect: imx: fix max_node_id
> 
> Hi Peng,
> 
> Thank you for the patch.
> 
> On Wed, Jun 01, 2022 at 05:41:51PM +0800, Peng Fan (OSS) wrote:
> > From: Peng Fan <peng.fan@nxp.com>
> >
> > max_node_id not equal to the ARRAY_SIZE of node array, need increase
> > 1, otherwise xlate will fail for the last entry.
> >
> > Fixes: f0d8048525d7d("interconnect: Add imx core driver")
> > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > ---
> >  drivers/interconnect/imx/imx.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/interconnect/imx/imx.c
> > b/drivers/interconnect/imx/imx.c index 249ca25d1d55..3c074933ed0c
> > 100644
> > --- a/drivers/interconnect/imx/imx.c
> > +++ b/drivers/interconnect/imx/imx.c
> > @@ -238,7 +238,7 @@ int imx_icc_register(struct platform_device *pdev,
> >  	int ret;
> >
> >  	/* icc_onecell_data is indexed by node_id, unlike nodes param */
> > -	max_node_id = get_max_node_id(nodes, nodes_count);
> > +	max_node_id = get_max_node_id(nodes, nodes_count) + 1;
> 
> I'd rename the max_node_id variable to num_nodes, as writing
> 
> 	max_node_id = get_max_node_id(...) + 1;
> 
> looks weird.

Thanks, will follow your suggestion in V2.

Thanks,
Peng.

> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> >  	data = devm_kzalloc(dev, struct_size(data, nodes, max_node_id),
> >  			    GFP_KERNEL);
> >  	if (!data)
> 
> --
> Regards,
> 
> Laurent Pinchart

[PATCH 4/8] interconnect: imx: set src node

[Peng Fan (OSS)]

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

When set QoS for a icc path, only set dst icc node is not enough,
also need to set src icc node.

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

diff --git a/drivers/interconnect/imx/imx.c b/drivers/interconnect/imx/imx.c
index 3c074933ed0c..1ba906822b7e 100644
--- a/drivers/interconnect/imx/imx.c
+++ b/drivers/interconnect/imx/imx.c
@@ -61,6 +61,12 @@ static int imx_icc_node_set(struct icc_node *node)
 
 static int imx_icc_set(struct icc_node *src, struct icc_node *dst)
 {
+	int ret;
+
+	ret = imx_icc_node_set(src);
+	if (ret)
+		return ret;
+
 	return imx_icc_node_set(dst);
 }
 
-- 
2.25.1

[PATCH 5/8] interconnect: imx: introduce imx_icc_provider

[Peng Fan (OSS)]

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

Introduce imx_icc_provider as a wrapper of icc_provider to
add i.MX specific information.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/interconnect/imx/imx.c | 24 ++++++++++++++----------
 drivers/interconnect/imx/imx.h |  6 ++++++
 2 files changed, 20 insertions(+), 10 deletions(-)

diff --git a/drivers/interconnect/imx/imx.c b/drivers/interconnect/imx/imx.c
index 1ba906822b7e..f381697fb23e 100644
--- a/drivers/interconnect/imx/imx.c
+++ b/drivers/interconnect/imx/imx.c
@@ -134,9 +134,10 @@ static int imx_icc_node_init_qos(struct icc_provider *provider,
 				      DEV_PM_QOS_MIN_FREQUENCY, 0);
 }
 
-static struct icc_node *imx_icc_node_add(struct icc_provider *provider,
+static struct icc_node *imx_icc_node_add(struct imx_icc_provider *imx_provider,
 					 const struct imx_icc_node_desc *node_desc)
 {
+	struct icc_provider *provider = &imx_provider->provider;
 	struct device *dev = provider->dev;
 	struct imx_icc_node *node_data;
 	struct icc_node *node;
@@ -184,10 +185,11 @@ static void imx_icc_unregister_nodes(struct icc_provider *provider)
 		imx_icc_node_destroy(node);
 }
 
-static int imx_icc_register_nodes(struct icc_provider *provider,
+static int imx_icc_register_nodes(struct imx_icc_provider *imx_provider,
 				  const struct imx_icc_node_desc *descs,
 				  int count)
 {
+	struct icc_provider *provider = &imx_provider->provider;
 	struct icc_onecell_data *provider_data = provider->data;
 	int ret;
 	int i;
@@ -197,7 +199,7 @@ static int imx_icc_register_nodes(struct icc_provider *provider,
 		const struct imx_icc_node_desc *node_desc = &descs[i];
 		size_t j;
 
-		node = imx_icc_node_add(provider, node_desc);
+		node = imx_icc_node_add(imx_provider, node_desc);
 		if (IS_ERR(node)) {
 			ret = dev_err_probe(provider->dev, PTR_ERR(node),
 					    "failed to add %s\n", node_desc->name);
@@ -239,6 +241,7 @@ int imx_icc_register(struct platform_device *pdev,
 {
 	struct device *dev = &pdev->dev;
 	struct icc_onecell_data *data;
+	struct imx_icc_provider *imx_provider;
 	struct icc_provider *provider;
 	int max_node_id;
 	int ret;
@@ -251,16 +254,17 @@ int imx_icc_register(struct platform_device *pdev,
 		return -ENOMEM;
 	data->num_nodes = max_node_id;
 
-	provider = devm_kzalloc(dev, sizeof(*provider), GFP_KERNEL);
-	if (!provider)
+	imx_provider = devm_kzalloc(dev, sizeof(*imx_provider), GFP_KERNEL);
+	if (!imx_provider)
 		return -ENOMEM;
+	provider = &imx_provider->provider;
 	provider->set = imx_icc_set;
 	provider->get_bw = imx_icc_get_bw;
 	provider->aggregate = icc_std_aggregate;
 	provider->xlate = of_icc_xlate_onecell;
 	provider->data = data;
 	provider->dev = dev->parent;
-	platform_set_drvdata(pdev, provider);
+	platform_set_drvdata(pdev, imx_provider);
 
 	ret = icc_provider_add(provider);
 	if (ret) {
@@ -268,7 +272,7 @@ int imx_icc_register(struct platform_device *pdev,
 		return ret;
 	}
 
-	ret = imx_icc_register_nodes(provider, nodes, nodes_count);
+	ret = imx_icc_register_nodes(imx_provider, nodes, nodes_count);
 	if (ret)
 		goto provider_del;
 
@@ -282,11 +286,11 @@ EXPORT_SYMBOL_GPL(imx_icc_register);
 
 int imx_icc_unregister(struct platform_device *pdev)
 {
-	struct icc_provider *provider = platform_get_drvdata(pdev);
+	struct imx_icc_provider *imx_provider = platform_get_drvdata(pdev);
 
-	imx_icc_unregister_nodes(provider);
+	imx_icc_unregister_nodes(&imx_provider->provider);
 
-	return icc_provider_del(provider);
+	return icc_provider_del(&imx_provider->provider);
 }
 EXPORT_SYMBOL_GPL(imx_icc_unregister);
 
diff --git a/drivers/interconnect/imx/imx.h b/drivers/interconnect/imx/imx.h
index 75da51076c68..0ad2c654c222 100644
--- a/drivers/interconnect/imx/imx.h
+++ b/drivers/interconnect/imx/imx.h
@@ -10,10 +10,16 @@
 #ifndef __DRIVERS_INTERCONNECT_IMX_H
 #define __DRIVERS_INTERCONNECT_IMX_H
 
+#include <linux/interconnect-provider.h>
 #include <linux/kernel.h>
 
 #define IMX_ICC_MAX_LINKS	4
 
+struct imx_icc_provider {
+	void __iomem *noc_base;
+	struct icc_provider provider;
+};
+
 /*
  * struct imx_icc_node_adj - Describe a dynamic adjustable node
  */
-- 
2.25.1

[PATCH 6/8] interconnect: imx: set of_node for interconnect provider

[Peng Fan (OSS)]

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

The provider device is created using platform_device_register_data in
imx-bus driver, which not has of_node. With of_node set, it will be
easy to support QoS settings.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/interconnect/imx/imx.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/interconnect/imx/imx.c b/drivers/interconnect/imx/imx.c
index f381697fb23e..08e3a91d2543 100644
--- a/drivers/interconnect/imx/imx.c
+++ b/drivers/interconnect/imx/imx.c
@@ -264,6 +264,7 @@ int imx_icc_register(struct platform_device *pdev,
 	provider->xlate = of_icc_xlate_onecell;
 	provider->data = data;
 	provider->dev = dev->parent;
+	provider->dev->of_node = dev->parent->of_node;
 	platform_set_drvdata(pdev, imx_provider);
 
 	ret = icc_provider_add(provider);
-- 
2.25.1

[PATCH 7/8] interconnect: imx: configure NoC mode/prioriry/ext_control

[Peng Fan (OSS)]

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

Introduce imx_icc_noc_setting structure to describe a master port setting
Pass imx_icc_noc_setting as a parameter from specific driver
Set priority level, mode, ext control in imx_icc_node_set

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/interconnect/imx/imx.c    | 39 +++++++++++++++++++++++++++----
 drivers/interconnect/imx/imx.h    | 19 ++++++++++++++-
 drivers/interconnect/imx/imx8mm.c |  2 +-
 drivers/interconnect/imx/imx8mn.c |  2 +-
 drivers/interconnect/imx/imx8mq.c |  2 +-
 5 files changed, 55 insertions(+), 9 deletions(-)

diff --git a/drivers/interconnect/imx/imx.c b/drivers/interconnect/imx/imx.c
index 08e3a91d2543..bbb6f4b32b66 100644
--- a/drivers/interconnect/imx/imx.c
+++ b/drivers/interconnect/imx/imx.c
@@ -10,6 +10,7 @@
 
 #include <linux/device.h>
 #include <linux/interconnect-provider.h>
+#include <linux/io.h>
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/of_platform.h>
@@ -18,11 +19,17 @@
 
 #include "imx.h"
 
+#define NOC_PRIO_REG	0x8
+#define NOC_MODE_REG	0xC
+#define NOC_EXT_CTL_REG	0x18
+
 /* private icc_node data */
 struct imx_icc_node {
 	const struct imx_icc_node_desc *desc;
+	const struct imx_icc_noc_setting *setting;
 	struct device *qos_dev;
 	struct dev_pm_qos_request qos_req;
+	struct imx_icc_provider *imx_provider;
 };
 
 static int imx_icc_get_bw(struct icc_node *node, u32 *avg, u32 *peak)
@@ -37,8 +44,16 @@ static int imx_icc_node_set(struct icc_node *node)
 {
 	struct device *dev = node->provider->dev;
 	struct imx_icc_node *node_data = node->data;
+	void __iomem *base;
 	u64 freq;
 
+	if (node_data->setting && !node_data->setting->ignore && node->peak_bw) {
+		base = node_data->setting->reg + node_data->imx_provider->noc_base;
+		writel(node_data->setting->prio_level, base + NOC_PRIO_REG);
+		writel(node_data->setting->mode, base + NOC_MODE_REG);
+		writel(node_data->setting->ext_control, base + NOC_EXT_CTL_REG);
+	}
+
 	if (!node_data->qos_dev)
 		return 0;
 
@@ -135,7 +150,8 @@ static int imx_icc_node_init_qos(struct icc_provider *provider,
 }
 
 static struct icc_node *imx_icc_node_add(struct imx_icc_provider *imx_provider,
-					 const struct imx_icc_node_desc *node_desc)
+					 const struct imx_icc_node_desc *node_desc,
+					 const struct imx_icc_noc_setting *setting)
 {
 	struct icc_provider *provider = &imx_provider->provider;
 	struct device *dev = provider->dev;
@@ -164,6 +180,8 @@ static struct icc_node *imx_icc_node_add(struct imx_icc_provider *imx_provider,
 	node->name = node_desc->name;
 	node->data = node_data;
 	node_data->desc = node_desc;
+	node_data->setting = setting;
+	node_data->imx_provider = imx_provider;
 	icc_node_add(node, provider);
 
 	if (node_desc->adj) {
@@ -187,7 +205,8 @@ static void imx_icc_unregister_nodes(struct icc_provider *provider)
 
 static int imx_icc_register_nodes(struct imx_icc_provider *imx_provider,
 				  const struct imx_icc_node_desc *descs,
-				  int count)
+				  int count,
+				  const struct imx_icc_noc_setting *settings)
 {
 	struct icc_provider *provider = &imx_provider->provider;
 	struct icc_onecell_data *provider_data = provider->data;
@@ -199,7 +218,10 @@ static int imx_icc_register_nodes(struct imx_icc_provider *imx_provider,
 		const struct imx_icc_node_desc *node_desc = &descs[i];
 		size_t j;
 
-		node = imx_icc_node_add(imx_provider, node_desc);
+		if (settings)
+			node = imx_icc_node_add(imx_provider, node_desc, &settings[node_desc->id]);
+		else
+			node = imx_icc_node_add(imx_provider, node_desc, NULL);
 		if (IS_ERR(node)) {
 			ret = dev_err_probe(provider->dev, PTR_ERR(node),
 					    "failed to add %s\n", node_desc->name);
@@ -237,7 +259,8 @@ static int get_max_node_id(struct imx_icc_node_desc *nodes, int nodes_count)
 }
 
 int imx_icc_register(struct platform_device *pdev,
-		     struct imx_icc_node_desc *nodes, int nodes_count)
+		     struct imx_icc_node_desc *nodes, int nodes_count,
+		     struct imx_icc_noc_setting *settings)
 {
 	struct device *dev = &pdev->dev;
 	struct icc_onecell_data *data;
@@ -267,13 +290,19 @@ int imx_icc_register(struct platform_device *pdev,
 	provider->dev->of_node = dev->parent->of_node;
 	platform_set_drvdata(pdev, imx_provider);
 
+	if (settings) {
+		imx_provider->noc_base = devm_of_iomap(dev, provider->dev->of_node, 0, NULL);
+		if (!imx_provider->noc_base)
+			return PTR_ERR(imx_provider->noc_base);
+	}
+
 	ret = icc_provider_add(provider);
 	if (ret) {
 		dev_err(dev, "error adding interconnect provider: %d\n", ret);
 		return ret;
 	}
 
-	ret = imx_icc_register_nodes(imx_provider, nodes, nodes_count);
+	ret = imx_icc_register_nodes(imx_provider, nodes, nodes_count, settings);
 	if (ret)
 		goto provider_del;
 
diff --git a/drivers/interconnect/imx/imx.h b/drivers/interconnect/imx/imx.h
index 0ad2c654c222..5fd650e61186 100644
--- a/drivers/interconnect/imx/imx.h
+++ b/drivers/interconnect/imx/imx.h
@@ -44,6 +44,22 @@ struct imx_icc_node_desc {
 	const struct imx_icc_node_adj_desc *adj;
 };
 
+/*
+ * struct imx_icc_noc_setting - Describe an interconnect node setting
+ * @ignore: indicate whether need apply this setting
+ * @reg: register offset inside the NoC
+ * @prio_level: priority level
+ * @mode: functional mode
+ * @ext_control: external input control
+ */
+struct imx_icc_noc_setting {
+	bool ignore;
+	u32 reg;
+	u32 prio_level;
+	u32 mode;
+	u32 ext_control;
+};
+
 #define DEFINE_BUS_INTERCONNECT(_name, _id, _adj, ...)			\
 	{								\
 		.id = _id,						\
@@ -61,7 +77,8 @@ struct imx_icc_node_desc {
 
 int imx_icc_register(struct platform_device *pdev,
 		     struct imx_icc_node_desc *nodes,
-		     int nodes_count);
+		     int nodes_count,
+		     struct imx_icc_noc_setting *noc_settings);
 int imx_icc_unregister(struct platform_device *pdev);
 
 #endif /* __DRIVERS_INTERCONNECT_IMX_H */
diff --git a/drivers/interconnect/imx/imx8mm.c b/drivers/interconnect/imx/imx8mm.c
index 1083490bb391..ae797412db96 100644
--- a/drivers/interconnect/imx/imx8mm.c
+++ b/drivers/interconnect/imx/imx8mm.c
@@ -83,7 +83,7 @@ static struct imx_icc_node_desc nodes[] = {
 
 static int imx8mm_icc_probe(struct platform_device *pdev)
 {
-	return imx_icc_register(pdev, nodes, ARRAY_SIZE(nodes));
+	return imx_icc_register(pdev, nodes, ARRAY_SIZE(nodes), NULL);
 }
 
 static int imx8mm_icc_remove(struct platform_device *pdev)
diff --git a/drivers/interconnect/imx/imx8mn.c b/drivers/interconnect/imx/imx8mn.c
index ad97e55fd4e5..1ce94c5bdd8c 100644
--- a/drivers/interconnect/imx/imx8mn.c
+++ b/drivers/interconnect/imx/imx8mn.c
@@ -72,7 +72,7 @@ static struct imx_icc_node_desc nodes[] = {
 
 static int imx8mn_icc_probe(struct platform_device *pdev)
 {
-	return imx_icc_register(pdev, nodes, ARRAY_SIZE(nodes));
+	return imx_icc_register(pdev, nodes, ARRAY_SIZE(nodes), NULL);
 }
 
 static int imx8mn_icc_remove(struct platform_device *pdev)
diff --git a/drivers/interconnect/imx/imx8mq.c b/drivers/interconnect/imx/imx8mq.c
index d7768d3c6d8a..7f00a0511c6e 100644
--- a/drivers/interconnect/imx/imx8mq.c
+++ b/drivers/interconnect/imx/imx8mq.c
@@ -82,7 +82,7 @@ static struct imx_icc_node_desc nodes[] = {
 
 static int imx8mq_icc_probe(struct platform_device *pdev)
 {
-	return imx_icc_register(pdev, nodes, ARRAY_SIZE(nodes));
+	return imx_icc_register(pdev, nodes, ARRAY_SIZE(nodes), NULL);
 }
 
 static int imx8mq_icc_remove(struct platform_device *pdev)
-- 
2.25.1

[PATCH 8/8] interconnect: imx: Add platform driver for imx8mp

[Peng Fan (OSS)]

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

Add a platform driver for the i.MX8MP SoC describing bus topology, based
on internal documentation.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/interconnect/imx/Kconfig              |   4 +
 drivers/interconnect/imx/Makefile             |   2 +
 drivers/interconnect/imx/imx8mp.c             | 232 ++++++++++++++++++
 include/dt-bindings/interconnect/fsl,imx8mp.h |  59 +++++
 4 files changed, 297 insertions(+)
 create mode 100644 drivers/interconnect/imx/imx8mp.c
 create mode 100644 include/dt-bindings/interconnect/fsl,imx8mp.h

diff --git a/drivers/interconnect/imx/Kconfig b/drivers/interconnect/imx/Kconfig
index be2928362bb7..c772552431f5 100644
--- a/drivers/interconnect/imx/Kconfig
+++ b/drivers/interconnect/imx/Kconfig
@@ -15,3 +15,7 @@ config INTERCONNECT_IMX8MN
 config INTERCONNECT_IMX8MQ
 	tristate "i.MX8MQ interconnect driver"
 	depends on INTERCONNECT_IMX
+
+config INTERCONNECT_IMX8MP
+	tristate "i.MX8MP interconnect driver"
+	depends on INTERCONNECT_IMX
diff --git a/drivers/interconnect/imx/Makefile b/drivers/interconnect/imx/Makefile
index 21fd5233754f..16d256cdeab4 100644
--- a/drivers/interconnect/imx/Makefile
+++ b/drivers/interconnect/imx/Makefile
@@ -2,8 +2,10 @@ imx-interconnect-objs			:= imx.o
 imx8mm-interconnect-objs       		:= imx8mm.o
 imx8mq-interconnect-objs       		:= imx8mq.o
 imx8mn-interconnect-objs       		:= imx8mn.o
+imx8mp-interconnect-objs       		:= imx8mp.o
 
 obj-$(CONFIG_INTERCONNECT_IMX)		+= imx-interconnect.o
 obj-$(CONFIG_INTERCONNECT_IMX8MM)	+= imx8mm-interconnect.o
 obj-$(CONFIG_INTERCONNECT_IMX8MQ)	+= imx8mq-interconnect.o
 obj-$(CONFIG_INTERCONNECT_IMX8MN)	+= imx8mn-interconnect.o
+obj-$(CONFIG_INTERCONNECT_IMX8MP)	+= imx8mp-interconnect.o
diff --git a/drivers/interconnect/imx/imx8mp.c b/drivers/interconnect/imx/imx8mp.c
new file mode 100644
index 000000000000..f13683ac941c
--- /dev/null
+++ b/drivers/interconnect/imx/imx8mp.c
@@ -0,0 +1,232 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Interconnect framework driver for i.MX8MP SoC
+ *
+ * Copyright 2022 NXP
+ * Peng Fan <peng.fan@nxp.com>
+ */
+
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <dt-bindings/interconnect/fsl,imx8mp.h>
+
+#include "imx.h"
+
+static const struct imx_icc_node_adj_desc imx8mp_noc_adj = {
+	.bw_mul = 1,
+	.bw_div = 16,
+	.main_noc = true,
+};
+
+static struct imx_icc_noc_setting noc_setting_nodes[] = {
+	[IMX8MP_ICM_MLMIX] = {
+		.reg = 0x180,
+		.prio_level = 0x80000303,
+	},
+	[IMX8MP_ICM_DSP] = {
+		.reg = 0x200,
+		.prio_level = 0x80000303,
+	},
+	[IMX8MP_ICM_SDMA2PER] = {
+		.reg = 0x280,
+		.prio_level = 0x80000404,
+	},
+	[IMX8MP_ICM_SDMA2BURST] = {
+		.reg = 0x300,
+		.prio_level = 0x80000404,
+	},
+	[IMX8MP_ICM_SDMA3PER] = {
+		.reg = 0x380,
+		.prio_level = 0x80000404,
+	},
+	[IMX8MP_ICM_SDMA3BURST] = {
+		.reg = 0x400,
+		.prio_level = 0x80000404,
+	},
+	[IMX8MP_ICM_EDMA] = {
+		.reg = 0x480,
+		.prio_level = 0x80000404,
+	},
+	[IMX8MP_ICM_GPU3D] = {
+		.reg = 0x500,
+		.prio_level = 0x80000303,
+	},
+	[IMX8MP_ICM_GPU2D] = {
+		.reg = 0x580,
+		.prio_level = 0x80000303,
+	},
+	[IMX8MP_ICM_HRV] = {
+		.reg = 0x600,
+		.prio_level = 0x80000202,
+		.ext_control = 1,
+	},
+	[IMX8MP_ICM_LCDIF_HDMI] = {
+		.reg = 0x680,
+		.prio_level = 0x80000202,
+		.ext_control = 1,
+	},
+	[IMX8MP_ICM_HDCP] = {
+		.reg = 0x700,
+		.prio_level = 0x80000505,
+	},
+	[IMX8MP_ICM_NOC_PCIE] = {
+		.reg = 0x780,
+		.prio_level = 0x80000303,
+	},
+	[IMX8MP_ICM_USB1] = {
+		.reg = 0x800,
+		.prio_level = 0x80000303,
+	},
+	[IMX8MP_ICM_USB2] = {
+		.reg = 0x880,
+		.prio_level = 0x80000303,
+	},
+	[IMX8MP_ICM_PCIE] = {
+		.reg = 0x900,
+		.prio_level = 0x80000303,
+	},
+	[IMX8MP_ICM_LCDIF_RD] = {
+		.reg = 0x980,
+		.prio_level = 0x80000202,
+		.ext_control = 1,
+	},
+	[IMX8MP_ICM_LCDIF_WR] = {
+		.reg = 0xa00,
+		.prio_level = 0x80000202,
+		.ext_control = 1,
+	},
+	[IMX8MP_ICM_ISI0] = {
+		.reg = 0xa80,
+		.prio_level = 0x80000202,
+		.ext_control = 1,
+	},
+	[IMX8MP_ICM_ISI1] = {
+		.reg = 0xb00,
+		.prio_level = 0x80000202,
+		.ext_control = 1,
+	},
+	[IMX8MP_ICM_ISI2] = {
+		.reg = 0xb80,
+		.prio_level = 0x80000202,
+		.ext_control = 1,
+	},
+	[IMX8MP_ICM_ISP0] = {
+		.reg = 0xc00,
+		.prio_level = 0x80000707,
+	},
+	[IMX8MP_ICM_ISP1] = {
+		.reg = 0xc80,
+		.prio_level = 0x80000707,
+	},
+	[IMX8MP_ICM_DWE] = {
+		.reg = 0xd00,
+		.prio_level = 0x80000707,
+	},
+	[IMX8MP_ICM_VPU_G1] = {
+		.reg = 0xd80,
+		.prio_level = 0x80000303,
+	},
+	[IMX8MP_ICM_VPU_G2] = {
+		.reg = 0xe00,
+		.prio_level = 0x80000303,
+	},
+	[IMX8MP_ICM_VPU_H1] = {
+		.reg = 0xe80,
+		.prio_level = 0x80000303,
+	},
+	[IMX8MP_ICN_MEDIA] = {
+		.ignore = true,
+	},
+	[IMX8MP_ICN_VIDEO] = {
+		.ignore = true,
+	},
+	[IMX8MP_ICN_AUDIO] = {
+		.ignore = true,
+	},
+	[IMX8MP_ICN_HDMI] = {
+		.ignore = true,
+	},
+	[IMX8MP_ICN_GPU] = {
+		.ignore = true,
+	},
+	[IMX8MP_ICN_HSIO] = {
+		.ignore = true,
+	},
+};
+
+/* Describe bus masters, slaves and connections between them */
+static struct imx_icc_node_desc nodes[] = {
+	DEFINE_BUS_INTERCONNECT("NOC", IMX8MP_ICN_NOC, &imx8mp_noc_adj,
+				IMX8MP_ICS_DRAM, IMX8MP_ICN_MAIN),
+
+	DEFINE_BUS_SLAVE("OCRAM", IMX8MP_ICS_OCRAM, NULL),
+	DEFINE_BUS_SLAVE("DRAM", IMX8MP_ICS_DRAM, NULL),
+	DEFINE_BUS_MASTER("A53", IMX8MP_ICM_A53, IMX8MP_ICN_NOC),
+	DEFINE_BUS_MASTER("SUPERMIX", IMX8MP_ICM_SUPERMIX, IMX8MP_ICN_NOC),
+	DEFINE_BUS_MASTER("GIC", IMX8MP_ICM_GIC, IMX8MP_ICN_NOC),
+	DEFINE_BUS_MASTER("MLMIX", IMX8MP_ICM_MLMIX, IMX8MP_ICN_NOC),
+
+	DEFINE_BUS_INTERCONNECT("NOC_AUDIO", IMX8MP_ICN_AUDIO, NULL, IMX8MP_ICN_NOC),
+	DEFINE_BUS_MASTER("DSP", IMX8MP_ICM_DSP, IMX8MP_ICN_AUDIO),
+	DEFINE_BUS_MASTER("SDMA2PER", IMX8MP_ICM_SDMA2PER, IMX8MP_ICN_AUDIO),
+	DEFINE_BUS_MASTER("SDMA2BURST", IMX8MP_ICM_SDMA2BURST, IMX8MP_ICN_AUDIO),
+	DEFINE_BUS_MASTER("SDMA3PER", IMX8MP_ICM_SDMA3PER, IMX8MP_ICN_AUDIO),
+	DEFINE_BUS_MASTER("SDMA3BURST", IMX8MP_ICM_SDMA3BURST, IMX8MP_ICN_AUDIO),
+	DEFINE_BUS_MASTER("EDMA", IMX8MP_ICM_EDMA, IMX8MP_ICN_AUDIO),
+
+	DEFINE_BUS_INTERCONNECT("NOC_GPU", IMX8MP_ICN_GPU, NULL, IMX8MP_ICN_NOC),
+	DEFINE_BUS_MASTER("GPU 2D", IMX8MP_ICM_GPU2D, IMX8MP_ICN_GPU),
+	DEFINE_BUS_MASTER("GPU 3D", IMX8MP_ICM_GPU3D, IMX8MP_ICN_GPU),
+
+	DEFINE_BUS_INTERCONNECT("NOC_HDMI", IMX8MP_ICN_HDMI, NULL, IMX8MP_ICN_NOC),
+	DEFINE_BUS_MASTER("HRV", IMX8MP_ICM_HRV, IMX8MP_ICN_HDMI),
+	DEFINE_BUS_MASTER("LCDIF_HDMI", IMX8MP_ICM_LCDIF_HDMI, IMX8MP_ICN_HDMI),
+	DEFINE_BUS_MASTER("HDCP", IMX8MP_ICM_HDCP, IMX8MP_ICN_HDMI),
+
+	DEFINE_BUS_INTERCONNECT("NOC_HSIO", IMX8MP_ICN_HSIO, NULL, IMX8MP_ICN_NOC),
+	DEFINE_BUS_MASTER("NOC_PCIE", IMX8MP_ICM_NOC_PCIE, IMX8MP_ICN_HSIO),
+	DEFINE_BUS_MASTER("USB1", IMX8MP_ICM_USB1, IMX8MP_ICN_HSIO),
+	DEFINE_BUS_MASTER("USB2", IMX8MP_ICM_USB2, IMX8MP_ICN_HSIO),
+	DEFINE_BUS_MASTER("PCIE", IMX8MP_ICM_PCIE, IMX8MP_ICN_HSIO),
+
+	DEFINE_BUS_INTERCONNECT("NOC_MEDIA", IMX8MP_ICN_MEDIA, NULL, IMX8MP_ICN_NOC),
+	DEFINE_BUS_MASTER("LCDIF_RD", IMX8MP_ICM_LCDIF_RD, IMX8MP_ICN_MEDIA),
+	DEFINE_BUS_MASTER("LCDIF_WR", IMX8MP_ICM_LCDIF_WR, IMX8MP_ICN_MEDIA),
+	DEFINE_BUS_MASTER("ISI0", IMX8MP_ICM_ISI0, IMX8MP_ICN_MEDIA),
+	DEFINE_BUS_MASTER("ISI1", IMX8MP_ICM_ISI1, IMX8MP_ICN_MEDIA),
+	DEFINE_BUS_MASTER("ISI2", IMX8MP_ICM_ISI2, IMX8MP_ICN_MEDIA),
+	DEFINE_BUS_MASTER("ISP0", IMX8MP_ICM_ISP0, IMX8MP_ICN_MEDIA),
+	DEFINE_BUS_MASTER("ISP1", IMX8MP_ICM_ISP1, IMX8MP_ICN_MEDIA),
+	DEFINE_BUS_MASTER("DWE", IMX8MP_ICM_DWE, IMX8MP_ICN_MEDIA),
+
+	DEFINE_BUS_INTERCONNECT("NOC_VIDEO", IMX8MP_ICN_VIDEO, NULL, IMX8MP_ICN_NOC),
+	DEFINE_BUS_MASTER("VPU G1", IMX8MP_ICM_VPU_G1, IMX8MP_ICN_VIDEO),
+	DEFINE_BUS_MASTER("VPU G2", IMX8MP_ICM_VPU_G2, IMX8MP_ICN_VIDEO),
+	DEFINE_BUS_MASTER("VPU H1", IMX8MP_ICM_VPU_H1, IMX8MP_ICN_VIDEO),
+	DEFINE_BUS_INTERCONNECT("PL301_MAIN", IMX8MP_ICN_MAIN, NULL,
+				IMX8MP_ICN_NOC, IMX8MP_ICS_OCRAM),
+};
+
+static int imx8mp_icc_probe(struct platform_device *pdev)
+{
+	return imx_icc_register(pdev, nodes, ARRAY_SIZE(nodes), noc_setting_nodes);
+}
+
+static int imx8mp_icc_remove(struct platform_device *pdev)
+{
+	return imx_icc_unregister(pdev);
+}
+
+static struct platform_driver imx8mp_icc_driver = {
+	.probe = imx8mp_icc_probe,
+	.remove = imx8mp_icc_remove,
+	.driver = {
+		.name = "imx8mp-interconnect",
+	},
+};
+
+module_platform_driver(imx8mp_icc_driver);
+MODULE_AUTHOR("Peng Fan <peng.fan@nxp.com>");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:imx8mp-interconnect");
diff --git a/include/dt-bindings/interconnect/fsl,imx8mp.h b/include/dt-bindings/interconnect/fsl,imx8mp.h
new file mode 100644
index 000000000000..732547577c76
--- /dev/null
+++ b/include/dt-bindings/interconnect/fsl,imx8mp.h
@@ -0,0 +1,59 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Interconnect framework driver for i.MX SoC
+ *
+ * Copyright 2022 NXP
+ * Peng Fan <peng.fan@nxp.com>
+ */
+
+#ifndef __DT_BINDINGS_INTERCONNECT_IMX8MP_H
+#define __DT_BINDINGS_INTERCONNECT_IMX8MP_H
+
+#define IMX8MP_ICN_NOC		0
+#define IMX8MP_ICN_MAIN		1
+#define IMX8MP_ICS_DRAM		2
+#define IMX8MP_ICS_OCRAM	3
+#define IMX8MP_ICM_A53		4
+#define IMX8MP_ICM_SUPERMIX	5
+#define IMX8MP_ICM_GIC		6
+#define IMX8MP_ICM_MLMIX	7
+
+#define IMX8MP_ICN_AUDIO	8
+#define IMX8MP_ICM_DSP		9
+#define IMX8MP_ICM_SDMA2PER	10
+#define IMX8MP_ICM_SDMA2BURST	11
+#define IMX8MP_ICM_SDMA3PER	12
+#define IMX8MP_ICM_SDMA3BURST	13
+#define IMX8MP_ICM_EDMA		14
+
+#define IMX8MP_ICN_GPU		15
+#define IMX8MP_ICM_GPU2D	16
+#define IMX8MP_ICM_GPU3D	17
+
+#define IMX8MP_ICN_HDMI		18
+#define IMX8MP_ICM_HRV		19
+#define IMX8MP_ICM_LCDIF_HDMI	20
+#define IMX8MP_ICM_HDCP		21
+
+#define IMX8MP_ICN_HSIO		22
+#define IMX8MP_ICM_NOC_PCIE	23
+#define IMX8MP_ICM_USB1		24
+#define IMX8MP_ICM_USB2		25
+#define IMX8MP_ICM_PCIE		26
+
+#define IMX8MP_ICN_MEDIA	27
+#define IMX8MP_ICM_LCDIF_RD	28
+#define IMX8MP_ICM_LCDIF_WR	29
+#define IMX8MP_ICM_ISI0		30
+#define IMX8MP_ICM_ISI1		31
+#define IMX8MP_ICM_ISI2		32
+#define IMX8MP_ICM_ISP0		33
+#define IMX8MP_ICM_ISP1		34
+#define IMX8MP_ICM_DWE		35
+
+#define IMX8MP_ICN_VIDEO	36
+#define IMX8MP_ICM_VPU_G1	37
+#define IMX8MP_ICM_VPU_G2	38
+#define IMX8MP_ICM_VPU_H1	39
+
+#endif /* __DT_BINDINGS_INTERCONNECT_IMX8MP_H */
-- 
2.25.1

Re: [PATCH 8/8] interconnect: imx: Add platform driver for imx8mp

[Krzysztof Kozlowski]

On 01/06/2022 11:41, Peng Fan (OSS) wrote:
> From: Peng Fan <peng.fan@nxp.com>
> 
> Add a platform driver for the i.MX8MP SoC describing bus topology, based
> on internal documentation.
> 
> Signed-off-by: Peng Fan <peng.fan@nxp.com>

> +
> +static struct platform_driver imx8mp_icc_driver = {
> +	.probe = imx8mp_icc_probe,
> +	.remove = imx8mp_icc_remove,
> +	.driver = {
> +		.name = "imx8mp-interconnect",

How do you match/bind the driver?

> +	},
> +};
> +
> +module_platform_driver(imx8mp_icc_driver);
> +MODULE_AUTHOR("Peng Fan <peng.fan@nxp.com>");
> +MODULE_LICENSE("GPL v2");
> +MODULE_ALIAS("platform:imx8mp-interconnect");
> diff --git a/include/dt-bindings/interconnect/fsl,imx8mp.h b/include/dt-bindings/interconnect/fsl,imx8mp.h
> new file mode 100644

Bindings are separate patch.

> index 000000000000..732547577c76
> --- /dev/null
> +++ b/include/dt-bindings/interconnect/fsl,imx8mp.h
> @@ -0,0 +1,59 @@
> +/* SPDX-License-Identifier: GPL-2.0 */

Dual license.

> +/*
> + * Interconnect framework driver for i.MX SoC
> + *
> + * Copyright 2022 NXP
> + * Peng Fan <peng.fan@nxp.com>
> + */
> +



Best regards,
Krzysztof

RE: [PATCH 8/8] interconnect: imx: Add platform driver for imx8mp

[Peng Fan]

> Subject: Re: [PATCH 8/8] interconnect: imx: Add platform driver for imx8mp
> 
> On 01/06/2022 11:41, Peng Fan (OSS) wrote:
> > From: Peng Fan <peng.fan@nxp.com>
> >
> > Add a platform driver for the i.MX8MP SoC describing bus topology,
> > based on internal documentation.
> >
> > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> 
> > +
> > +static struct platform_driver imx8mp_icc_driver = {
> > +	.probe = imx8mp_icc_probe,
> > +	.remove = imx8mp_icc_remove,
> > +	.driver = {
> > +		.name = "imx8mp-interconnect",
> 
> How do you match/bind the driver?

drivers/devfreq/imx-bus.c will create a platform device
that match this driver. I have not posted the patch to add
imx8mp support in devfreq driver.

Thanks,
Peng.

> 
> > +	},
> > +};
> > +
> > +module_platform_driver(imx8mp_icc_driver);
> > +MODULE_AUTHOR("Peng Fan <peng.fan@nxp.com>");
> MODULE_LICENSE("GPL
> > +v2"); MODULE_ALIAS("platform:imx8mp-interconnect");
> > diff --git a/include/dt-bindings/interconnect/fsl,imx8mp.h
> > b/include/dt-bindings/interconnect/fsl,imx8mp.h
> > new file mode 100644
> 
> Bindings are separate patch.
> 
> > index 000000000000..732547577c76
> > --- /dev/null
> > +++ b/include/dt-bindings/interconnect/fsl,imx8mp.h
> > @@ -0,0 +1,59 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> 
> Dual license.
> 
> > +/*
> > + * Interconnect framework driver for i.MX SoC
> > + *
> > + * Copyright 2022 NXP
> > + * Peng Fan <peng.fan@nxp.com>
> > + */
> > +
> 
> 
> 
> Best regards,
> Krzysztof

RE: [PATCH 0/8] interconnect: support i.MX8MP

[Peng Fan]

All,

> Subject: [PATCH 0/8] interconnect: support i.MX8MP

I am going to send out V2 this week to address the comments until now.
But before that I would like to see if any one has any comments on the
design here.

Georgi, do you have comments on Patch 2 " interconnect: add device
managed bulk API"

Lucas, since you had comments when I first use syscon to configure NoC,
are you ok with the design to use interconnect in this patchset?

Thanks,
Peng.

> 
> From: Peng Fan <peng.fan@nxp.com>
> 
> This patchset is to support i.MX8MP NoC settings, i.MX8MP NoC initial value
> after power up is invalid, need set a valid value after related power domain up.
> 
> This patchset also includes two patch[1,2] during my development to enable the
> ICC feature for i.MX8MP.
> 
> I not include ddrc DVFS in this patchset, ths patchset is only to support NoC
> value mode/priority/ext_control being set to a valid value that suggested by
> i.MX Chip Design Team. The value is same as NXP downstream one inside Arm
> Trusted Firmware:
> https://source.codeaurora.org/external/imx/imx-atf/tree/plat/imx/imx8m/imx
> 8mp/gpc.c?h=lf_v2.4#n97
> 
> A repo created here:
> https://github.com/MrVan/linux/tree/imx8mp-interconnect
> 
> Peng Fan (8):
>   dt-bindings: interconnect: imx8m: Add bindings for imx8mp noc
>   interconnect: add device managed bulk API
>   interconnect: imx: fix max_node_id
>   interconnect: imx: set src node
>   interconnect: imx: introduce imx_icc_provider
>   interconnect: imx: set of_node for interconnect provider
>   interconnect: imx: configure NoC mode/prioriry/ext_control
>   interconnect: imx: Add platform driver for imx8mp
> 
>  .../bindings/interconnect/fsl,imx8m-noc.yaml  |   6 +
>  drivers/interconnect/bulk.c                   |  34 +++
>  drivers/interconnect/imx/Kconfig              |   4 +
>  drivers/interconnect/imx/Makefile             |   2 +
>  drivers/interconnect/imx/imx.c                |  68 +++--
>  drivers/interconnect/imx/imx.h                |  25 +-
>  drivers/interconnect/imx/imx8mm.c             |   2 +-
>  drivers/interconnect/imx/imx8mn.c             |   2 +-
>  drivers/interconnect/imx/imx8mp.c             | 232
> ++++++++++++++++++
>  drivers/interconnect/imx/imx8mq.c             |   2 +-
>  include/dt-bindings/interconnect/fsl,imx8mp.h |  59 +++++
>  include/linux/interconnect.h                  |   6 +
>  12 files changed, 424 insertions(+), 18 deletions(-)  create mode 100644
> drivers/interconnect/imx/imx8mp.c  create mode 100644
> include/dt-bindings/interconnect/fsl,imx8mp.h
> 
> --
> 2.25.1

Re: [PATCH 0/8] interconnect: support i.MX8MP

[Lucas Stach]

Hi Peng,

Am Montag, dem 13.06.2022 um 01:23 +0000 schrieb Peng Fan:
> All,
> 
> > Subject: [PATCH 0/8] interconnect: support i.MX8MP
> 
> I am going to send out V2 this week to address the comments until now.
> But before that I would like to see if any one has any comments on the
> design here.
> 
> Georgi, do you have comments on Patch 2 " interconnect: add device
> managed bulk API"
> 
> Lucas, since you had comments when I first use syscon to configure NoC,
> are you ok with the design to use interconnect in this patchset?
> 
I'm still not 100% convinced that the blk-ctrl is the right consumer
for the interconnect, since it doesn't do any busmastering. However,
the design looks much better than the syscon based one.

I mostly worry about being able to extend this to do more than the
current static configuration if/when NXP decides to release more
information about the NoC configuration options or someone reverse
engineers this part of the SoC. I still hope that we could optimize NoC
usage by setting real bandwidth and latency limits for the devices
connected to the NoC. As the blk-ctrl doesn't have any clue about this
right now, we can't really set any more specific requests than the
current INT_MAX ones.
I guess we could extend things in this way by making the blk-ctrl not
only be a simple consumer of the interconnect, but aggregate requests
from the devices in the blk-ctrl domain and forward them to the NOC
provider, right?

Regards,
Lucas

> Thanks,
> Peng.
> 
> > 
> > From: Peng Fan <peng.fan@nxp.com>
> > 
> > This patchset is to support i.MX8MP NoC settings, i.MX8MP NoC initial value
> > after power up is invalid, need set a valid value after related power domain up.
> > 
> > This patchset also includes two patch[1,2] during my development to enable the
> > ICC feature for i.MX8MP.
> > 
> > I not include ddrc DVFS in this patchset, ths patchset is only to support NoC
> > value mode/priority/ext_control being set to a valid value that suggested by
> > i.MX Chip Design Team. The value is same as NXP downstream one inside Arm
> > Trusted Firmware:
> > https://source.codeaurora.org/external/imx/imx-atf/tree/plat/imx/imx8m/imx
> > 8mp/gpc.c?h=lf_v2.4#n97
> > 
> > A repo created here:
> > https://github.com/MrVan/linux/tree/imx8mp-interconnect
> > 
> > Peng Fan (8):
> >   dt-bindings: interconnect: imx8m: Add bindings for imx8mp noc
> >   interconnect: add device managed bulk API
> >   interconnect: imx: fix max_node_id
> >   interconnect: imx: set src node
> >   interconnect: imx: introduce imx_icc_provider
> >   interconnect: imx: set of_node for interconnect provider
> >   interconnect: imx: configure NoC mode/prioriry/ext_control
> >   interconnect: imx: Add platform driver for imx8mp
> > 
> >  .../bindings/interconnect/fsl,imx8m-noc.yaml  |   6 +
> >  drivers/interconnect/bulk.c                   |  34 +++
> >  drivers/interconnect/imx/Kconfig              |   4 +
> >  drivers/interconnect/imx/Makefile             |   2 +
> >  drivers/interconnect/imx/imx.c                |  68 +++--
> >  drivers/interconnect/imx/imx.h                |  25 +-
> >  drivers/interconnect/imx/imx8mm.c             |   2 +-
> >  drivers/interconnect/imx/imx8mn.c             |   2 +-
> >  drivers/interconnect/imx/imx8mp.c             | 232
> > ++++++++++++++++++
> >  drivers/interconnect/imx/imx8mq.c             |   2 +-
> >  include/dt-bindings/interconnect/fsl,imx8mp.h |  59 +++++
> >  include/linux/interconnect.h                  |   6 +
> >  12 files changed, 424 insertions(+), 18 deletions(-)  create mode 100644
> > drivers/interconnect/imx/imx8mp.c  create mode 100644
> > include/dt-bindings/interconnect/fsl,imx8mp.h
> > 
> > --
> > 2.25.1
> 

RE: [PATCH 0/8] interconnect: support i.MX8MP

[Peng Fan]

Hi Lucas,

> Subject: Re: [PATCH 0/8] interconnect: support i.MX8MP
> 
> Hi Peng,
> 
> Am Montag, dem 13.06.2022 um 01:23 +0000 schrieb Peng Fan:
> > All,
> >
> > > Subject: [PATCH 0/8] interconnect: support i.MX8MP
> >
> > I am going to send out V2 this week to address the comments until now.
> > But before that I would like to see if any one has any comments on the
> > design here.
> >
> > Georgi, do you have comments on Patch 2 " interconnect: add device
> > managed bulk API"
> >
> > Lucas, since you had comments when I first use syscon to configure
> > NoC, are you ok with the design to use interconnect in this patchset?
> >
> I'm still not 100% convinced that the blk-ctrl is the right consumer for the
> interconnect, since it doesn't do any busmastering. However, the design looks
> much better than the syscon based one.
> 
> I mostly worry about being able to extend this to do more than the current
> static configuration if/when NXP decides to release more information about the
> NoC configuration options or someone reverse engineers this part of the SoC. 

I have asked internally, NoC documentation for i.MX8M* is not allowed to public.

I
> still hope that we could optimize NoC usage by setting real bandwidth and
> latency limits for the devices connected to the NoC. As the blk-ctrl doesn't have
> any clue about this right now, we can't really set any more specific requests
> than the current INT_MAX ones.

Actually looking at ATF NoC settings, the values are suggested by Design team,
Design team give SW team such a group of value and not suggest SW team
to change it. And the value in ATF not touch bandwidth registers, as you
could see from the patchset, only mode,priority,ext_control are configured.

Similar to qcom using static settings:
./drivers/interconnect/qcom/qcm2290.c:668.
.qos.qos_mode = NOC_QOS_MODE_FIXED,

I understand that people wanna tune the settings at runtime on demand.

> I guess we could extend things in this way by making the blk-ctrl not only be a
> simple consumer of the interconnect, but aggregate requests from the devices
> in the blk-ctrl domain and forward them to the NOC provider, right?

I am not sure. This patchset is actually only for init NoC settings after
power on, because the initial value is invalid.

I could think how to resolve the INT_MAX settings in next version,
For your upper suggestion, could we start after this version approved for land
in tree?


Thanks,
Peng


> 
> Regards,
> Lucas
> 
> > Thanks,
> > Peng.
> >
> > >
> > > From: Peng Fan <peng.fan@nxp.com>
> > >
> > > This patchset is to support i.MX8MP NoC settings, i.MX8MP NoC
> > > initial value after power up is invalid, need set a valid value after related
> power domain up.
> > >
> > > This patchset also includes two patch[1,2] during my development to
> > > enable the ICC feature for i.MX8MP.
> > >
> > > I not include ddrc DVFS in this patchset, ths patchset is only to
> > > support NoC value mode/priority/ext_control being set to a valid
> > > value that suggested by i.MX Chip Design Team. The value is same as
> > > NXP downstream one inside Arm Trusted Firmware:
> > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fso
> > >
> urce.codeaurora.org%2Fexternal%2Fimx%2Fimx-atf%2Ftree%2Fplat%2Fimx%2
> > >
> Fimx8m%2Fimx&amp;data=05%7C01%7Cpeng.fan%40nxp.com%7C6cfad0fcec
> 0d472
> > >
> 408a208da4e2cd96d%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7
> C63790
> > >
> 8251778425186%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJ
> QIjoiV2
> > >
> luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=U
> vIx%
> > >
> 2BAz9rx3Z8Oy7VSCRB90O8M5VICIUaUOiTmYw%2FeI%3D&amp;reserved=0
> > > 8mp/gpc.c?h=lf_v2.4#n97
> > >
> > > A repo created here:
> > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgi
> > >
> thub.com%2FMrVan%2Flinux%2Ftree%2Fimx8mp-interconnect&amp;data=05
> %7C
> > >
> 01%7Cpeng.fan%40nxp.com%7C6cfad0fcec0d472408a208da4e2cd96d%7C68
> 6ea1d
> > >
> 3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637908251778425186%7CUnkn
> own%7CT
> > >
> WFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJX
> V
> > >
> CI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=W2iYPMJ6dn%2F4OTalTD2yqB
> Hx%2Bo3%
> > > 2BuBTuP%2BAe4bBz2Gc%3D&amp;reserved=0
> > >
> > > Peng Fan (8):
> > >   dt-bindings: interconnect: imx8m: Add bindings for imx8mp noc
> > >   interconnect: add device managed bulk API
> > >   interconnect: imx: fix max_node_id
> > >   interconnect: imx: set src node
> > >   interconnect: imx: introduce imx_icc_provider
> > >   interconnect: imx: set of_node for interconnect provider
> > >   interconnect: imx: configure NoC mode/prioriry/ext_control
> > >   interconnect: imx: Add platform driver for imx8mp
> > >
> > >  .../bindings/interconnect/fsl,imx8m-noc.yaml  |   6 +
> > >  drivers/interconnect/bulk.c                   |  34 +++
> > >  drivers/interconnect/imx/Kconfig              |   4 +
> > >  drivers/interconnect/imx/Makefile             |   2 +
> > >  drivers/interconnect/imx/imx.c                |  68 +++--
> > >  drivers/interconnect/imx/imx.h                |  25 +-
> > >  drivers/interconnect/imx/imx8mm.c             |   2 +-
> > >  drivers/interconnect/imx/imx8mn.c             |   2 +-
> > >  drivers/interconnect/imx/imx8mp.c             | 232
> > > ++++++++++++++++++
> > >  drivers/interconnect/imx/imx8mq.c             |   2 +-
> > >  include/dt-bindings/interconnect/fsl,imx8mp.h |  59 +++++
> > >  include/linux/interconnect.h                  |   6 +
> > >  12 files changed, 424 insertions(+), 18 deletions(-)  create mode
> > > 100644 drivers/interconnect/imx/imx8mp.c  create mode 100644
> > > include/dt-bindings/interconnect/fsl,imx8mp.h
> > >
> > > --
> > > 2.25.1
> >
> 

Re: [PATCH 0/8] interconnect: support i.MX8MP

[Lucas Stach]

Hi Peng,

Am Dienstag, dem 14.06.2022 um 23:38 +0000 schrieb Peng Fan:
> Hi Lucas,
> 
> > Subject: Re: [PATCH 0/8] interconnect: support i.MX8MP
> > 
> > Hi Peng,
> > 
> > Am Montag, dem 13.06.2022 um 01:23 +0000 schrieb Peng Fan:
> > > All,
> > > 
> > > > Subject: [PATCH 0/8] interconnect: support i.MX8MP
> > > 
> > > I am going to send out V2 this week to address the comments until now.
> > > But before that I would like to see if any one has any comments on the
> > > design here.
> > > 
> > > Georgi, do you have comments on Patch 2 " interconnect: add device
> > > managed bulk API"
> > > 
> > > Lucas, since you had comments when I first use syscon to configure
> > > NoC, are you ok with the design to use interconnect in this patchset?
> > > 
> > I'm still not 100% convinced that the blk-ctrl is the right consumer for the
> > interconnect, since it doesn't do any busmastering. However, the design looks
> > much better than the syscon based one.
> > 
> > I mostly worry about being able to extend this to do more than the current
> > static configuration if/when NXP decides to release more information about the
> > NoC configuration options or someone reverse engineers this part of the SoC. 
> 
> I have asked internally, NoC documentation for i.MX8M* is not allowed to public.
> 
Yea, sadly I've heard this many times from NXP.

> I
> > still hope that we could optimize NoC usage by setting real bandwidth and
> > latency limits for the devices connected to the NoC. As the blk-ctrl doesn't have
> > any clue about this right now, we can't really set any more specific requests
> > than the current INT_MAX ones.
> 
> Actually looking at ATF NoC settings, the values are suggested by Design team,
> Design team give SW team such a group of value and not suggest SW team
> to change it. And the value in ATF not touch bandwidth registers, as you
> could see from the patchset, only mode,priority,ext_control are configured.
> 
> Similar to qcom using static settings:
> ./drivers/interconnect/qcom/qcm2290.c:668.
> .qos.qos_mode = NOC_QOS_MODE_FIXED,
> 
> I understand that people wanna tune the settings at runtime on demand.
> 
Right. We had the same situation with QoS settings on the i.MX6, where
Freescale/NXP claimed that the values from the design team are optimal
and should not be changed, but we actually had some cases where tuning
those values to the specific use-case of a board was beneficial. With
the i.MX6 we could do this on our own, as things were documented, at
least partially.

I don't request you or anyone from the NXP open source team to do
something here, as I understand that the no documentation policy is an
outside decision that you can not really change. I just want to make
sure that if someone was to do something about this situation, we don't
make that change harder than necessary by locking us into a DT binding
and design that might be hard to change later on.

> > I guess we could extend things in this way by making the blk-ctrl not only be a
> > simple consumer of the interconnect, but aggregate requests from the devices
> > in the blk-ctrl domain and forward them to the NOC provider, right?
> 
> I am not sure. This patchset is actually only for init NoC settings after
> power on, because the initial value is invalid.
> 
> I could think how to resolve the INT_MAX settings in next version,
> For your upper suggestion, could we start after this version approved for land
> in tree?
> 
I just want you to think about how we could extend the design laid down
in this patchset if/when the peripheral drivers are starting to request
their actual bandwidth usage. If the answer to this question is "we'll
simply make the blk-ctl part of the interconnect hierarchy and let it
aggregate the bandwidth requests and forward them to the NoC driver"
then I'm fine with this patchset landing in upstream as-is. I'm just
not sure if I'm overlooking something here which would prevent such an
extension of the design, as I'm not a expert in the interconnect
framework.

Regards,
Lucas

> 
> Thanks,
> Peng
> 
> 
> > 
> > Regards,
> > Lucas
> > 
> > > Thanks,
> > > Peng.
> > > 
> > > > 
> > > > From: Peng Fan <peng.fan@nxp.com>
> > > > 
> > > > This patchset is to support i.MX8MP NoC settings, i.MX8MP NoC
> > > > initial value after power up is invalid, need set a valid value after related
> > power domain up.
> > > > 
> > > > This patchset also includes two patch[1,2] during my development to
> > > > enable the ICC feature for i.MX8MP.
> > > > 
> > > > I not include ddrc DVFS in this patchset, ths patchset is only to
> > > > support NoC value mode/priority/ext_control being set to a valid
> > > > value that suggested by i.MX Chip Design Team. The value is same as
> > > > NXP downstream one inside Arm Trusted Firmware:
> > > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fso
> > > > 
> > urce.codeaurora.org%2Fexternal%2Fimx%2Fimx-atf%2Ftree%2Fplat%2Fimx%2
> > > > 
> > Fimx8m%2Fimx&amp;data=05%7C01%7Cpeng.fan%40nxp.com%7C6cfad0fcec
> > 0d472
> > > > 
> > 408a208da4e2cd96d%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7
> > C63790
> > > > 
> > 8251778425186%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJ
> > QIjoiV2
> > > > 
> > luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=U
> > vIx%
> > > > 
> > 2BAz9rx3Z8Oy7VSCRB90O8M5VICIUaUOiTmYw%2FeI%3D&amp;reserved=0
> > > > 8mp/gpc.c?h=lf_v2.4#n97
> > > > 
> > > > A repo created here:
> > > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgi
> > > > 
> > thub.com%2FMrVan%2Flinux%2Ftree%2Fimx8mp-interconnect&amp;data=05
> > %7C
> > > > 
> > 01%7Cpeng.fan%40nxp.com%7C6cfad0fcec0d472408a208da4e2cd96d%7C68
> > 6ea1d
> > > > 
> > 3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637908251778425186%7CUnkn
> > own%7CT
> > > > 
> > WFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJX
> > V
> > > > 
> > CI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=W2iYPMJ6dn%2F4OTalTD2yqB
> > Hx%2Bo3%
> > > > 2BuBTuP%2BAe4bBz2Gc%3D&amp;reserved=0
> > > > 
> > > > Peng Fan (8):
> > > >   dt-bindings: interconnect: imx8m: Add bindings for imx8mp noc
> > > >   interconnect: add device managed bulk API
> > > >   interconnect: imx: fix max_node_id
> > > >   interconnect: imx: set src node
> > > >   interconnect: imx: introduce imx_icc_provider
> > > >   interconnect: imx: set of_node for interconnect provider
> > > >   interconnect: imx: configure NoC mode/prioriry/ext_control
> > > >   interconnect: imx: Add platform driver for imx8mp
> > > > 
> > > >  .../bindings/interconnect/fsl,imx8m-noc.yaml  |   6 +
> > > >  drivers/interconnect/bulk.c                   |  34 +++
> > > >  drivers/interconnect/imx/Kconfig              |   4 +
> > > >  drivers/interconnect/imx/Makefile             |   2 +
> > > >  drivers/interconnect/imx/imx.c                |  68 +++--
> > > >  drivers/interconnect/imx/imx.h                |  25 +-
> > > >  drivers/interconnect/imx/imx8mm.c             |   2 +-
> > > >  drivers/interconnect/imx/imx8mn.c             |   2 +-
> > > >  drivers/interconnect/imx/imx8mp.c             | 232
> > > > ++++++++++++++++++
> > > >  drivers/interconnect/imx/imx8mq.c             |   2 +-
> > > >  include/dt-bindings/interconnect/fsl,imx8mp.h |  59 +++++
> > > >  include/linux/interconnect.h                  |   6 +
> > > >  12 files changed, 424 insertions(+), 18 deletions(-)  create mode
> > > > 100644 drivers/interconnect/imx/imx8mp.c  create mode 100644
> > > > include/dt-bindings/interconnect/fsl,imx8mp.h
> > > > 
> > > > --
> > > > 2.25.1
> > > 
> > 
> 

RE: [PATCH 0/8] interconnect: support i.MX8MP

[Peng Fan]

Hi Lucas,

> Subject: Re: [PATCH 0/8] interconnect: support i.MX8MP
> 
> Hi Peng,
> 
> Am Dienstag, dem 14.06.2022 um 23:38 +0000 schrieb Peng Fan:
> > Hi Lucas,
> >
> > > Subject: Re: [PATCH 0/8] interconnect: support i.MX8MP
> > >
> > > Hi Peng,
> > >
> > > Am Montag, dem 13.06.2022 um 01:23 +0000 schrieb Peng Fan:
> > > > All,
> > > >
> > > > > Subject: [PATCH 0/8] interconnect: support i.MX8MP
> > > >
> > > > I am going to send out V2 this week to address the comments until now.
> > > > But before that I would like to see if any one has any comments on
> > > > the design here.
> > > >
> > > > Georgi, do you have comments on Patch 2 " interconnect: add device
> > > > managed bulk API"
> > > >
> > > > Lucas, since you had comments when I first use syscon to configure
> > > > NoC, are you ok with the design to use interconnect in this patchset?
> > > >
> > > I'm still not 100% convinced that the blk-ctrl is the right consumer
> > > for the interconnect, since it doesn't do any busmastering. However,
> > > the design looks much better than the syscon based one.
> > >
> > > I mostly worry about being able to extend this to do more than the
> > > current static configuration if/when NXP decides to release more
> > > information about the NoC configuration options or someone reverse
> engineers this part of the SoC.
> >
> > I have asked internally, NoC documentation for i.MX8M* is not allowed to
> public.
> >
> Yea, sadly I've heard this many times from NXP.
> 
> > I
> > > still hope that we could optimize NoC usage by setting real
> > > bandwidth and latency limits for the devices connected to the NoC.
> > > As the blk-ctrl doesn't have any clue about this right now, we can't
> > > really set any more specific requests than the current INT_MAX ones.
> >
> > Actually looking at ATF NoC settings, the values are suggested by
> > Design team, Design team give SW team such a group of value and not
> > suggest SW team to change it. And the value in ATF not touch bandwidth
> > registers, as you could see from the patchset, only mode,priority,ext_control
> are configured.
> >
> > Similar to qcom using static settings:
> > ./drivers/interconnect/qcom/qcm2290.c:668.
> > .qos.qos_mode = NOC_QOS_MODE_FIXED,
> >
> > I understand that people wanna tune the settings at runtime on demand.
> >
> Right. We had the same situation with QoS settings on the i.MX6, where
> Freescale/NXP claimed that the values from the design team are optimal and
> should not be changed, but we actually had some cases where tuning those
> values to the specific use-case of a board was beneficial. With the i.MX6 we
> could do this on our own, as things were documented, at least partially.
> 
> I don't request you or anyone from the NXP open source team to do something
> here, as I understand that the no documentation policy is an outside decision
> that you can not really change. I just want to make sure that if someone was to
> do something about this situation, we don't make that change harder than
> necessary by locking us into a DT binding and design that might be hard to
> change later on.

Hope I could help on this if you have suggestion on technical direction without
breaking NXP policy.

> 
> > > I guess we could extend things in this way by making the blk-ctrl
> > > not only be a simple consumer of the interconnect, but aggregate
> > > requests from the devices in the blk-ctrl domain and forward them to the
> NOC provider, right?
> >
> > I am not sure. This patchset is actually only for init NoC settings
> > after power on, because the initial value is invalid.
> >
> > I could think how to resolve the INT_MAX settings in next version, For
> > your upper suggestion, could we start after this version approved for
> > land in tree?
> >
> I just want you to think about how we could extend the design laid down in this
> patchset if/when the peripheral drivers are starting to request their actual
> bandwidth usage. If the answer to this question is "we'll simply make the blk-ctl
> part of the interconnect hierarchy and let it aggregate the bandwidth requests
> and forward them to the NoC driver"
> then I'm fine with this patchset landing in upstream as-is. I'm just not sure if I'm
> overlooking something here which would prevent such an extension of the
> design, as I'm not a expert in the interconnect framework.

There is only priority level as I write in the driver, the priority level is not suggested
to runtime change according to i.MX design team.
For media related settings, there is GPR hurry level settings which is written
in RM and ATF code.

In normal case, if you really wanna change priority level, you could use
other value. I'll update driver to describe priority bit mask.
In some heavy load mode, you could pre-set a GPR hurry level value to boost
the transaction.

But since design team not suggest SW to runtime update the value, also no
idea to match priority level with SW bandwidth. from driver debug, what
I could improve is the describe the priority register, then people wanna
to change, could update it.

Is this ok?

Thanks,
Peng.

> 
> Regards,
> Lucas
> 
> >
> > Thanks,
> > Peng
> >
> >
> > >
> > > Regards,
> > > Lucas
> > >
> > > > Thanks,
> > > > Peng.
> > > >
> > > > >
> > > > > From: Peng Fan <peng.fan@nxp.com>
> > > > >
> > > > > This patchset is to support i.MX8MP NoC settings, i.MX8MP NoC
> > > > > initial value after power up is invalid, need set a valid value
> > > > > after related
> > > power domain up.
> > > > >
> > > > > This patchset also includes two patch[1,2] during my development
> > > > > to enable the ICC feature for i.MX8MP.
> > > > >
> > > > > I not include ddrc DVFS in this patchset, ths patchset is only
> > > > > to support NoC value mode/priority/ext_control being set to a
> > > > > valid value that suggested by i.MX Chip Design Team. The value
> > > > > is same as NXP downstream one inside Arm Trusted Firmware:
> > > > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%
> > > > > 2Fso
> > > > >
> > >
> urce.codeaurora.org%2Fexternal%2Fimx%2Fimx-atf%2Ftree%2Fplat%2Fimx%2
> > > > >
> > >
> Fimx8m%2Fimx&amp;data=05%7C01%7Cpeng.fan%40nxp.com%7C6cfad0fcec
> > > 0d472
> > > > >
> > >
> 408a208da4e2cd96d%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7
> > > C63790
> > > > >
> > >
> 8251778425186%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJ
> > > QIjoiV2
> > > > >
> > >
> luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=U
> > > vIx%
> > > > >
> > >
> 2BAz9rx3Z8Oy7VSCRB90O8M5VICIUaUOiTmYw%2FeI%3D&amp;reserved=0
> > > > > 8mp/gpc.c?h=lf_v2.4#n97
> > > > >
> > > > > A repo created here:
> > > > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%
> > > > > 2Fgi
> > > > >
> > >
> thub.com%2FMrVan%2Flinux%2Ftree%2Fimx8mp-interconnect&amp;data=05
> > > %7C
> > > > >
> > >
> 01%7Cpeng.fan%40nxp.com%7C6cfad0fcec0d472408a208da4e2cd96d%7C68
> > > 6ea1d
> > > > >
> > >
> 3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637908251778425186%7CUnkn
> > > own%7CT
> > > > >
> > >
> WFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJX
> > > V
> > > > >
> > >
> CI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=W2iYPMJ6dn%2F4OTalTD2yqB
> > > Hx%2Bo3%
> > > > > 2BuBTuP%2BAe4bBz2Gc%3D&amp;reserved=0
> > > > >
> > > > > Peng Fan (8):
> > > > >   dt-bindings: interconnect: imx8m: Add bindings for imx8mp noc
> > > > >   interconnect: add device managed bulk API
> > > > >   interconnect: imx: fix max_node_id
> > > > >   interconnect: imx: set src node
> > > > >   interconnect: imx: introduce imx_icc_provider
> > > > >   interconnect: imx: set of_node for interconnect provider
> > > > >   interconnect: imx: configure NoC mode/prioriry/ext_control
> > > > >   interconnect: imx: Add platform driver for imx8mp
> > > > >
> > > > >  .../bindings/interconnect/fsl,imx8m-noc.yaml  |   6 +
> > > > >  drivers/interconnect/bulk.c                   |  34 +++
> > > > >  drivers/interconnect/imx/Kconfig              |   4 +
> > > > >  drivers/interconnect/imx/Makefile             |   2 +
> > > > >  drivers/interconnect/imx/imx.c                |  68 +++--
> > > > >  drivers/interconnect/imx/imx.h                |  25 +-
> > > > >  drivers/interconnect/imx/imx8mm.c             |   2 +-
> > > > >  drivers/interconnect/imx/imx8mn.c             |   2 +-
> > > > >  drivers/interconnect/imx/imx8mp.c             | 232
> > > > > ++++++++++++++++++
> > > > >  drivers/interconnect/imx/imx8mq.c             |   2 +-
> > > > >  include/dt-bindings/interconnect/fsl,imx8mp.h |  59 +++++
> > > > >  include/linux/interconnect.h                  |   6 +
> > > > >  12 files changed, 424 insertions(+), 18 deletions(-)  create
> > > > > mode
> > > > > 100644 drivers/interconnect/imx/imx8mp.c  create mode 100644
> > > > > include/dt-bindings/interconnect/fsl,imx8mp.h
> > > > >
> > > > > --
> > > > > 2.25.1
> > > >
> > >
> >
> 

Re: [PATCH 0/8] interconnect: support i.MX8MP

[Lucas Stach]

Am Mittwoch, dem 15.06.2022 um 09:28 +0000 schrieb Peng Fan:
> Hi Lucas,
> 
> > Subject: Re: [PATCH 0/8] interconnect: support i.MX8MP
> > 
> > Hi Peng,
> > 
> > Am Dienstag, dem 14.06.2022 um 23:38 +0000 schrieb Peng Fan:
> > > Hi Lucas,
> > > 
> > > > Subject: Re: [PATCH 0/8] interconnect: support i.MX8MP
> > > > 
> > > > Hi Peng,
> > > > 
> > > > Am Montag, dem 13.06.2022 um 01:23 +0000 schrieb Peng Fan:
> > > > > All,
> > > > > 
> > > > > > Subject: [PATCH 0/8] interconnect: support i.MX8MP
> > > > > 
> > > > > I am going to send out V2 this week to address the comments until now.
> > > > > But before that I would like to see if any one has any comments on
> > > > > the design here.
> > > > > 
> > > > > Georgi, do you have comments on Patch 2 " interconnect: add device
> > > > > managed bulk API"
> > > > > 
> > > > > Lucas, since you had comments when I first use syscon to configure
> > > > > NoC, are you ok with the design to use interconnect in this patchset?
> > > > > 
> > > > I'm still not 100% convinced that the blk-ctrl is the right consumer
> > > > for the interconnect, since it doesn't do any busmastering. However,
> > > > the design looks much better than the syscon based one.
> > > > 
> > > > I mostly worry about being able to extend this to do more than the
> > > > current static configuration if/when NXP decides to release more
> > > > information about the NoC configuration options or someone reverse
> > engineers this part of the SoC.
> > > 
> > > I have asked internally, NoC documentation for i.MX8M* is not allowed to
> > public.
> > > 
> > Yea, sadly I've heard this many times from NXP.
> > 
> > > I
> > > > still hope that we could optimize NoC usage by setting real
> > > > bandwidth and latency limits for the devices connected to the NoC.
> > > > As the blk-ctrl doesn't have any clue about this right now, we can't
> > > > really set any more specific requests than the current INT_MAX ones.
> > > 
> > > Actually looking at ATF NoC settings, the values are suggested by
> > > Design team, Design team give SW team such a group of value and not
> > > suggest SW team to change it. And the value in ATF not touch bandwidth
> > > registers, as you could see from the patchset, only mode,priority,ext_control
> > are configured.
> > > 
> > > Similar to qcom using static settings:
> > > ./drivers/interconnect/qcom/qcm2290.c:668.
> > > .qos.qos_mode = NOC_QOS_MODE_FIXED,
> > > 
> > > I understand that people wanna tune the settings at runtime on demand.
> > > 
> > Right. We had the same situation with QoS settings on the i.MX6, where
> > Freescale/NXP claimed that the values from the design team are optimal and
> > should not be changed, but we actually had some cases where tuning those
> > values to the specific use-case of a board was beneficial. With the i.MX6 we
> > could do this on our own, as things were documented, at least partially.
> > 
> > I don't request you or anyone from the NXP open source team to do something
> > here, as I understand that the no documentation policy is an outside decision
> > that you can not really change. I just want to make sure that if someone was to
> > do something about this situation, we don't make that change harder than
> > necessary by locking us into a DT binding and design that might be hard to
> > change later on.
> 
> Hope I could help on this if you have suggestion on technical direction without
> breaking NXP policy.
> 
> > 
> > > > I guess we could extend things in this way by making the blk-ctrl
> > > > not only be a simple consumer of the interconnect, but aggregate
> > > > requests from the devices in the blk-ctrl domain and forward them to the
> > NOC provider, right?
> > > 
> > > I am not sure. This patchset is actually only for init NoC settings
> > > after power on, because the initial value is invalid.
> > > 
> > > I could think how to resolve the INT_MAX settings in next version, For
> > > your upper suggestion, could we start after this version approved for
> > > land in tree?
> > > 
> > I just want you to think about how we could extend the design laid down in this
> > patchset if/when the peripheral drivers are starting to request their actual
> > bandwidth usage. If the answer to this question is "we'll simply make the blk-ctl
> > part of the interconnect hierarchy and let it aggregate the bandwidth requests
> > and forward them to the NoC driver"
> > then I'm fine with this patchset landing in upstream as-is. I'm just not sure if I'm
> > overlooking something here which would prevent such an extension of the
> > design, as I'm not a expert in the interconnect framework.
> 
> There is only priority level as I write in the driver, the priority level is not suggested
> to runtime change according to i.MX design team.
> For media related settings, there is GPR hurry level settings which is written
> in RM and ATF code.
> 
> In normal case, if you really wanna change priority level, you could use
> other value. I'll update driver to describe priority bit mask.
> In some heavy load mode, you could pre-set a GPR hurry level value to boost
> the transaction.
> 
> But since design team not suggest SW to runtime update the value, also no
> idea to match priority level with SW bandwidth. from driver debug, what
> I could improve is the describe the priority register, then people wanna
> to change, could update it.

But we all know that you could do more interesting things in regulator
mode with bandwidth setting changed according to use-case, right? ;)

> 
> Is this ok?
> 
I don't worry too much about the specific implementation in the drivers
right now, as this is something we can change later on. What I'm
worried about is more the DT description. With the description in this
patchset we put the blk-ctrl in charge of requesting the bandwidth
limits, as it's the consumer of the interconnect. But it doesn't really
have any information about the real bandwidth/latency/whatever other
QoS settings of the peripherals, so it can't do a proper request.

If it is possible to extend the current design in the future by making
the blk-ctrl be a interconnect provider to the peripherals in the power
domains _without_ changing the DT description between the blk-ctrl and
NoC node, then I'm fine with the design in this patchset.

Regards,
Lucas

> Thanks,
> Peng.
> 
> > 
> > Regards,
> > Lucas
> > 
> > > 
> > > Thanks,
> > > Peng
> > > 
> > > 
> > > > 
> > > > Regards,
> > > > Lucas
> > > > 
> > > > > Thanks,
> > > > > Peng.
> > > > > 
> > > > > > 
> > > > > > From: Peng Fan <peng.fan@nxp.com>
> > > > > > 
> > > > > > This patchset is to support i.MX8MP NoC settings, i.MX8MP NoC
> > > > > > initial value after power up is invalid, need set a valid value
> > > > > > after related
> > > > power domain up.
> > > > > > 
> > > > > > This patchset also includes two patch[1,2] during my development
> > > > > > to enable the ICC feature for i.MX8MP.
> > > > > > 
> > > > > > I not include ddrc DVFS in this patchset, ths patchset is only
> > > > > > to support NoC value mode/priority/ext_control being set to a
> > > > > > valid value that suggested by i.MX Chip Design Team. The value
> > > > > > is same as NXP downstream one inside Arm Trusted Firmware:
> > > > > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%
> > > > > > 2Fso
> > > > > > 
> > > > 
> > urce.codeaurora.org%2Fexternal%2Fimx%2Fimx-atf%2Ftree%2Fplat%2Fimx%2
> > > > > > 
> > > > 
> > Fimx8m%2Fimx&amp;data=05%7C01%7Cpeng.fan%40nxp.com%7C6cfad0fcec
> > > > 0d472
> > > > > > 
> > > > 
> > 408a208da4e2cd96d%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7
> > > > C63790
> > > > > > 
> > > > 
> > 8251778425186%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJ
> > > > QIjoiV2
> > > > > > 
> > > > 
> > luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=U
> > > > vIx%
> > > > > > 
> > > > 
> > 2BAz9rx3Z8Oy7VSCRB90O8M5VICIUaUOiTmYw%2FeI%3D&amp;reserved=0
> > > > > > 8mp/gpc.c?h=lf_v2.4#n97
> > > > > > 
> > > > > > A repo created here:
> > > > > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%
> > > > > > 2Fgi
> > > > > > 
> > > > 
> > thub.com%2FMrVan%2Flinux%2Ftree%2Fimx8mp-interconnect&amp;data=05
> > > > %7C
> > > > > > 
> > > > 
> > 01%7Cpeng.fan%40nxp.com%7C6cfad0fcec0d472408a208da4e2cd96d%7C68
> > > > 6ea1d
> > > > > > 
> > > > 
> > 3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637908251778425186%7CUnkn
> > > > own%7CT
> > > > > > 
> > > > 
> > WFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJX
> > > > V
> > > > > > 
> > > > 
> > CI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=W2iYPMJ6dn%2F4OTalTD2yqB
> > > > Hx%2Bo3%
> > > > > > 2BuBTuP%2BAe4bBz2Gc%3D&amp;reserved=0
> > > > > > 
> > > > > > Peng Fan (8):
> > > > > >   dt-bindings: interconnect: imx8m: Add bindings for imx8mp noc
> > > > > >   interconnect: add device managed bulk API
> > > > > >   interconnect: imx: fix max_node_id
> > > > > >   interconnect: imx: set src node
> > > > > >   interconnect: imx: introduce imx_icc_provider
> > > > > >   interconnect: imx: set of_node for interconnect provider
> > > > > >   interconnect: imx: configure NoC mode/prioriry/ext_control
> > > > > >   interconnect: imx: Add platform driver for imx8mp
> > > > > > 
> > > > > >  .../bindings/interconnect/fsl,imx8m-noc.yaml  |   6 +
> > > > > >  drivers/interconnect/bulk.c                   |  34 +++
> > > > > >  drivers/interconnect/imx/Kconfig              |   4 +
> > > > > >  drivers/interconnect/imx/Makefile             |   2 +
> > > > > >  drivers/interconnect/imx/imx.c                |  68 +++--
> > > > > >  drivers/interconnect/imx/imx.h                |  25 +-
> > > > > >  drivers/interconnect/imx/imx8mm.c             |   2 +-
> > > > > >  drivers/interconnect/imx/imx8mn.c             |   2 +-
> > > > > >  drivers/interconnect/imx/imx8mp.c             | 232
> > > > > > ++++++++++++++++++
> > > > > >  drivers/interconnect/imx/imx8mq.c             |   2 +-
> > > > > >  include/dt-bindings/interconnect/fsl,imx8mp.h |  59 +++++
> > > > > >  include/linux/interconnect.h                  |   6 +
> > > > > >  12 files changed, 424 insertions(+), 18 deletions(-)  create
> > > > > > mode
> > > > > > 100644 drivers/interconnect/imx/imx8mp.c  create mode 100644
> > > > > > include/dt-bindings/interconnect/fsl,imx8mp.h
> > > > > > 
> > > > > > --
> > > > > > 2.25.1
> > > > > 
> > > > 
> > > 
> > 
>