Re: [PATCH v2 8/8] arm64: dts: imx8m: Add NOC nodes

[Adam Ford <aford173@gmail.com>]

On Tue, Apr 7, 2020 at 4:10 AM Leonard Crestez <leonard.crestez@nxp.com> wrote:
>
> On 2020-04-04 4:25 PM, Adam Ford wrote:
> > On Wed, Apr 1, 2020 at 9:35 AM Leonard Crestez <leonard.crestez@nxp.com> wrote:
> >>
> >> Add nodes for the main interconnect of the imx8m series chips.
> >>
> >> These nodes are bound to by devfreq and interconnect drivers.
> >>
> >> Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com>
> >> ---
> >>   arch/arm64/boot/dts/freescale/imx8mm.dtsi | 24 +++++++++++++++++++++++
> >>   arch/arm64/boot/dts/freescale/imx8mn.dtsi | 24 +++++++++++++++++++++++
> >>   arch/arm64/boot/dts/freescale/imx8mq.dtsi | 24 +++++++++++++++++++++++
> >>   3 files changed, 72 insertions(+)
> >>
> >> diff --git a/arch/arm64/boot/dts/freescale/imx8mm.dtsi b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
> >> index 175c28ae10cf..41047b6709b6 100644
> >> --- a/arch/arm64/boot/dts/freescale/imx8mm.dtsi
> >> +++ b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
> >> @@ -6,10 +6,11 @@
> >>   #include <dt-bindings/clock/imx8mm-clock.h>
> >>   #include <dt-bindings/gpio/gpio.h>
> >>   #include <dt-bindings/input/input.h>
> >>   #include <dt-bindings/interrupt-controller/arm-gic.h>
> >>   #include <dt-bindings/thermal/thermal.h>
> >> +#include <dt-bindings/interconnect/imx8mm.h>
> >>
> >>   #include "imx8mm-pinfunc.h"
> >>
> >>   / {
> >>          interrupt-parent = <&gic>;
> >> @@ -860,10 +861,33 @@
> >>                                  status = "disabled";
> >>                          };
> >>
> >>                  };
> >>
> >> +               noc: interconnect@32700000 {
> >> +                       compatible = "fsl,imx8mm-noc", "fsl,imx8m-noc";
> >> +                       reg = <0x32700000 0x100000>;
> >> +                       clocks = <&clk IMX8MM_CLK_NOC>;
> >> +                       fsl,ddrc = <&ddrc>;
> >> +                       #interconnect-cells = <1>;
> >> +                       operating-points-v2 = <&noc_opp_table>;
> >> +
> >> +                       noc_opp_table: opp-table {
> >> +                               compatible = "operating-points-v2";
> >> +
> >> +                               opp-150M {
> >> +                                       opp-hz = /bits/ 64 <150000000>;
> >> +                               };
> >> +                               opp-375M {
> >> +                                       opp-hz = /bits/ 64 <375000000>;
> >> +                               };
> >> +                               opp-750M {
> >> +                                       opp-hz = /bits/ 64 <750000000>;
> >
> > Out of curiosity, the 8M Mini runs up to 750M, and  the 8M Nano and
> > 8MQ run up to 800.  The 8MQ had a patch to increase the assigned clock
> > speed for the NOC to 800MHz
> >
> > See: https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.kernel.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Fnext%2Flinux-next.git%2Fpatch%2Farch%2Farm64%2Fboot%2Fdts%2Ffreescale%3Fid%3D912b9dacf3f0ffad55e1a1b3c5af0e433ebdb5dd&amp;data=02%7C01%7Cleonard.crestez%40nxp.com%7C32c3655718e4459028e008d7d89baa31%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637216035403876452&amp;sdata=z%2B5afsPGbCk4HkRp4nR6QepOrm70Fi5B5dohyvaquxo%3D&amp;reserved=0)
> >
> > The 8M Mini and 8M Nano appear to be setting the default speed to 0.
>
> I'm not sure what you mean about this, the noc clock is required for
> mostly everything.

As an example, the i.MX8MM looks like this:

assigned-clocks = <&clk IMX8MM_CLK_NOC>,
                              <&clk IMX8MM_CLK_AUDIO_AHB>,
                              <&clk IMX8MM_CLK_IPG_AUDIO_ROOT>,
                              <&clk IMX8MM_SYS_PLL3>,
                              <&clk IMX8MM_VIDEO_PLL1>,
                              <&clk IMX8MM_AUDIO_PLL1>,
                              <&clk IMX8MM_AUDIO_PLL2>;
assigned-clock-parents = <&clk IMX8MM_SYS_PLL3_OUT>,
                                         <&clk IMX8MM_SYS_PLL1_800M>;
assigned-clock-rates = <0>,
                                    <400000000>,
                                    <400000000>,
                                    <750000000>,
                                    <594000000>,
                                    <393216000>,
                                    <361267200>;

If I am reading this correctly, it appears to me that IMX8MM_CLK_NOC
is set to 0.

The i.MX8MN is similar, but the patch above shows IMX8MQ_CLK_NOC used
to be 0, but was updated for better performance.

>
> > Should the 8M Mini or 8M Nano do something similar to what the 8MQ
> > did, or does this series negate the need for such a patch?
>
> Instead of doing assigned-clocks noc frequency needs to be tweaked by
> adjusting OPPs in this list. The devfreq device for noc will overwrite
> other frequencies set for the noc.

My question was whether or not we should consider a patch to made the
default assigned-clock-rate for IMX8MM_CLK_NOC to be 750000000.  Based
on your response, it sounds like the answer might be that it's not
necessary.

thanks

adam
>
> --
> Regards,
> Leonard

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel