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@xxxxxxx> > > > > > > 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&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&sdata=U > vIx% > > > > 2BAz9rx3Z8Oy7VSCRB90O8M5VICIUaUOiTmYw%2FeI%3D&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&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&sdata=W2iYPMJ6dn%2F4OTalTD2yqB > Hx%2Bo3% > > > 2BuBTuP%2BAe4bBz2Gc%3D&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 > > >
