| Message ID | 20201022150808.763082-1-aford173@gmail.com |
|---|---|
| Headers | show |
| Series | Add i.MX8MN power domain and fix USB | expand |
On Thu, Oct 22, 2020 at 10:08:05AM -0500, Adam Ford wrote: > This adds support for the power domains founds on i.MX8MN. The Nano > has fewer domains than the Mini, and the access to some of these domains > is different than that of the Mini, the Mini power domains cannot be > reused. > > Signed-off-by: Adam Ford <aford173@gmail.com> Acked-by: Krzysztof Kozlowski <krzk@kernel.org> Best regards, Krzysztof
On Thu, Oct 22, 2020 at 10:08:06AM -0500, Adam Ford wrote: > This adds the DT nodes to describe the power domains available on the > i.MX8MN. There are four power domains, but the displaymix and mipi > power domains need a separate clock block controller which is also > pending for 8MP and 8MM. Once the path for those is clear, Nano will > need something similar, but the registers for Nano differ. For now, > the dispmix and mipi are placeholders. > > Signed-off-by: Adam Ford <aford173@gmail.com> > > diff --git a/arch/arm64/boot/dts/freescale/imx8mn.dtsi b/arch/arm64/boot/dts/freescale/imx8mn.dtsi > index 9b4baf7bdfb1..27733fbe87e9 100644 > --- a/arch/arm64/boot/dts/freescale/imx8mn.dtsi > +++ b/arch/arm64/boot/dts/freescale/imx8mn.dtsi > @@ -596,6 +596,55 @@ src: reset-controller@30390000 { > interrupts = <GIC_SPI 89 IRQ_TYPE_LEVEL_HIGH>; > #reset-cells = <1>; > }; > + > + gpc: gpc@303a0000 { > + compatible = "fsl,imx8mn-gpc"; > + reg = <0x303a0000 0x10000>; > + interrupt-parent = <&gic>; > + interrupt-controller; > + #interrupt-cells = <3>; Missing interrupts. > + > + pgc { > + #address-cells = <1>; > + #size-cells = <0>; > + > + pgc_hsiomix: power-domain@0 { > + #power-domain-cells = <0>; > + reg = <IMX8MN_POWER_DOMAIN_HSIOMIX>; > + clocks = <&clk IMX8MN_CLK_USB_BUS>; > + }; > + > + pgc_otg1: power-domain@1 { > + #power-domain-cells = <0>; > + reg = <IMX8MN_POWER_DOMAIN_OTG1>; > + power-domains = <&pgc_hsiomix>; > + }; > + > + pgc_gpumix: power-domain@2 { > + #power-domain-cells = <0>; > + reg = <IMX8MN_POWER_DOMAIN_GPUMIX>; > + clocks = <&clk IMX8MN_CLK_GPU_CORE_ROOT>, > + <&clk IMX8MN_CLK_GPU_SHADER_DIV>, > + <&clk IMX8MN_CLK_GPU_BUS_ROOT>, > + <&clk IMX8MN_CLK_GPU_AHB>; > + resets = <&src IMX8MQ_RESET_GPU_RESET>; Does it compile without include? Did the include come via dependencies of this patch? Best regards, Krzysztof
On Thu, Oct 22, 2020 at 10:08:07AM -0500, Adam Ford wrote: > The USB OTG controller cannot be used until the power-domain is enabled > unless it was started in the bootloader. > > Adding the power-domain reference to the OTG node allows the OTG > controller to operate. > > Signed-off-by: Adam Ford <aford173@gmail.com> > I wonder, why your patches do not have usual Git trailer with summary of changes (after '---')? > diff --git a/arch/arm64/boot/dts/freescale/imx8mn.dtsi b/arch/arm64/boot/dts/freescale/imx8mn.dtsi > index 27733fbe87e9..605e6dbd2c6f 100644 > --- a/arch/arm64/boot/dts/freescale/imx8mn.dtsi > +++ b/arch/arm64/boot/dts/freescale/imx8mn.dtsi > @@ -979,6 +979,7 @@ usbotg1: usb@32e40000 { > assigned-clock-parents = <&clk IMX8MN_SYS_PLL2_500M>; > fsl,usbphy = <&usbphynop1>; > fsl,usbmisc = <&usbmisc1 0>; > + power-domains = <&pgc_otg1>; I guess you need it also for the usbphynop1 and usbmisc1. Best regards, Krzysztof
On Fri, Oct 23, 2020 at 4:52 AM Krzysztof Kozlowski <krzk@kernel.org> wrote: > > On Thu, Oct 22, 2020 at 10:08:06AM -0500, Adam Ford wrote: > > This adds the DT nodes to describe the power domains available on the > > i.MX8MN. There are four power domains, but the displaymix and mipi > > power domains need a separate clock block controller which is also > > pending for 8MP and 8MM. Once the path for those is clear, Nano will > > need something similar, but the registers for Nano differ. For now, > > the dispmix and mipi are placeholders. > > > > Signed-off-by: Adam Ford <aford173@gmail.com> > > > > diff --git a/arch/arm64/boot/dts/freescale/imx8mn.dtsi b/arch/arm64/boot/dts/freescale/imx8mn.dtsi > > index 9b4baf7bdfb1..27733fbe87e9 100644 > > --- a/arch/arm64/boot/dts/freescale/imx8mn.dtsi > > +++ b/arch/arm64/boot/dts/freescale/imx8mn.dtsi > > @@ -596,6 +596,55 @@ src: reset-controller@30390000 { > > interrupts = <GIC_SPI 89 IRQ_TYPE_LEVEL_HIGH>; > > #reset-cells = <1>; > > }; > > + > > + gpc: gpc@303a0000 { > > + compatible = "fsl,imx8mn-gpc"; > > + reg = <0x303a0000 0x10000>; > > + interrupt-parent = <&gic>; > > + interrupt-controller; > > + #interrupt-cells = <3>; > > Missing interrupts. Oops. I'll go back and review this. > > > + > > + pgc { > > + #address-cells = <1>; > > + #size-cells = <0>; > > + > > + pgc_hsiomix: power-domain@0 { > > + #power-domain-cells = <0>; > > + reg = <IMX8MN_POWER_DOMAIN_HSIOMIX>; > > + clocks = <&clk IMX8MN_CLK_USB_BUS>; > > + }; > > + > > + pgc_otg1: power-domain@1 { > > + #power-domain-cells = <0>; > > + reg = <IMX8MN_POWER_DOMAIN_OTG1>; > > + power-domains = <&pgc_hsiomix>; > > + }; > > + > > + pgc_gpumix: power-domain@2 { > > + #power-domain-cells = <0>; > > + reg = <IMX8MN_POWER_DOMAIN_GPUMIX>; > > + clocks = <&clk IMX8MN_CLK_GPU_CORE_ROOT>, > > + <&clk IMX8MN_CLK_GPU_SHADER_DIV>, > > + <&clk IMX8MN_CLK_GPU_BUS_ROOT>, > > + <&clk IMX8MN_CLK_GPU_AHB>; > > + resets = <&src IMX8MQ_RESET_GPU_RESET>; > > Does it compile without include? Did the include come via dependencies > of this patch? Oops. I cherry picked the git commits, but I forgot to include the includes which is why it compiles for me. I'll fix in a V2. Thanks for all the reviews. adam > > Best regards, > Krzysztof >
On Fri, Oct 23, 2020 at 4:55 AM Krzysztof Kozlowski <krzk@kernel.org> wrote: > > On Thu, Oct 22, 2020 at 10:08:07AM -0500, Adam Ford wrote: > > The USB OTG controller cannot be used until the power-domain is enabled > > unless it was started in the bootloader. > > > > Adding the power-domain reference to the OTG node allows the OTG > > controller to operate. > > > > Signed-off-by: Adam Ford <aford173@gmail.com> > > > > I wonder, why your patches do not have usual Git trailer with summary of > changes (after '---')? I historically have just done 'git format-patch -pX' but I haven't seen complaints that the summary was missing. I can add them going forward. Most of these individual patches have only touched single files or created new files all together. > > > diff --git a/arch/arm64/boot/dts/freescale/imx8mn.dtsi b/arch/arm64/boot/dts/freescale/imx8mn.dtsi > > index 27733fbe87e9..605e6dbd2c6f 100644 > > --- a/arch/arm64/boot/dts/freescale/imx8mn.dtsi > > +++ b/arch/arm64/boot/dts/freescale/imx8mn.dtsi > > @@ -979,6 +979,7 @@ usbotg1: usb@32e40000 { > > assigned-clock-parents = <&clk IMX8MN_SYS_PLL2_500M>; > > fsl,usbphy = <&usbphynop1>; > > fsl,usbmisc = <&usbmisc1 0>; > > + power-domains = <&pgc_otg1>; > > I guess you need it also for the usbphynop1 and usbmisc1. From what I can see looking at the IP blocks and the vendor's code repo, only the usbotg IP block needs the power-domain to enable the clocking going to it. AFAICT, neither the usbphynop nor the usbmisc are using clocks gated by the power-domain. Thanks again for all your reviews. adam > > Best regards, > Krzysztof
On Fri, Oct 23, 2020 at 06:03:32AM -0500, Adam Ford wrote: > On Fri, Oct 23, 2020 at 4:55 AM Krzysztof Kozlowski <krzk@kernel.org> wrote: > > > > On Thu, Oct 22, 2020 at 10:08:07AM -0500, Adam Ford wrote: > > > The USB OTG controller cannot be used until the power-domain is enabled > > > unless it was started in the bootloader. > > > > > > Adding the power-domain reference to the OTG node allows the OTG > > > controller to operate. > > > > > > Signed-off-by: Adam Ford <aford173@gmail.com> > > > > > > > I wonder, why your patches do not have usual Git trailer with summary of > > changes (after '---')? > > I historically have just done 'git format-patch -pX' but I haven't > seen complaints that the summary was missing. I can add them going > forward. Most of these individual patches have only touched single > files or created new files all together. The patch stat is useful, especially if it touches multiple files/subsystems. However it's not needed, so I was just wondering :) > > > > > > diff --git a/arch/arm64/boot/dts/freescale/imx8mn.dtsi b/arch/arm64/boot/dts/freescale/imx8mn.dtsi > > > index 27733fbe87e9..605e6dbd2c6f 100644 > > > --- a/arch/arm64/boot/dts/freescale/imx8mn.dtsi > > > +++ b/arch/arm64/boot/dts/freescale/imx8mn.dtsi > > > @@ -979,6 +979,7 @@ usbotg1: usb@32e40000 { > > > assigned-clock-parents = <&clk IMX8MN_SYS_PLL2_500M>; > > > fsl,usbphy = <&usbphynop1>; > > > fsl,usbmisc = <&usbmisc1 0>; > > > + power-domains = <&pgc_otg1>; > > > > I guess you need it also for the usbphynop1 and usbmisc1. > > From what I can see looking at the IP blocks and the vendor's code > repo, only the usbotg IP block needs the power-domain to enable the > clocking going to it. AFAICT, neither the usbphynop nor the usbmisc > are using clocks gated by the power-domain. > Thanks again for all your reviews. Mhm, ok then. Thanks for checking. Best regards, Krzysztof