diff for duplicates of <20180309170221.1c8bf8a1@karo-electronics.de> diff --git a/a/1.txt b/N1/1.txt index 3d56133..5e47c39 100644 --- a/a/1.txt +++ b/N1/1.txt @@ -2,131 +2,109 @@ Hi, On Thu, 8 Mar 2018 17:46:54 +0100 Martin Kaiser wrote: > Hi Fabio, ->=20 +> > thanks for the quick response. ->=20 -> Thus wrote Fabio Estevam (festevam@gmail.com): ->=20 +> +> Thus wrote Fabio Estevam (festevam at gmail.com): +> > > Hi Martin, ->=20 +> > > On Thu, Mar 8, 2018 at 11:08 AM, Martin Kaiser <martin@kaiser.cx> wrote: > > > Hi Fabio, ->=20 -> > > Thus wrote Fabio Estevam (festevam@gmail.com): ->=20 +> +> > > Thus wrote Fabio Estevam (festevam at gmail.com): +> > > >> I get audio working from SSI1, but I guess this is due to the fact > > >> that the bootloader enables the SSI clock: ->=20 +> > > >> I have the following in U-Boot: ->=20 +> > > >> /* Enable the clocks */ > > >> DATA 4 0x53f8000c 0x1fffffff > > >> DATA 4 0x53f80010 0xffffffff > > >> DATA 4 0x53f80014 0xfdfff ->=20 +> > > > I'm using the same initial settings. ->=20 +> > > > Nevertheless, ssi1_ipg_per is disbled after loading the kernel. ->=20 +> > > > Digging into this a bit more, it turned out that without my patch, -> > > clk_disable_unused() recognizes ssi1_ipg_per as unused and disables i= -t. ->=20 +> > > clk_disable_unused() recognizes ssi1_ipg_per as unused and disables it. +> > > > If my patch is applied and ssi1_ipg_per is declared as parent of -> > > ssi1_ipg, clk_disable_unused() will not disable it and fsl_ssi_startu= -p() +> > > ssi1_ipg, clk_disable_unused() will not disable it and fsl_ssi_startup() > > > will enable both ssi1_ipg_per and ssi1_ipg before playing sound. ->=20 +> > > I can get audio to work fine without your patch on a mx25pdk. ->=20 +> > this is surprising. How come the ssi1_ipg_per clock is not turned off by > clk_disable_unused()? Where is it used? Do you have ->=20 +> > <&clks 55> ->=20 +> > anywhere in your DT? ->=20 +> > > In the other i.MX clock drivers we have this same pattern: ->=20 -> > clks[IMX6SL_CLK_SSI1_IPG] =3D imx_clk_gate2_shared("ssi1_ipg", = -"ipg", ->=20 -> It seems to the that imx25 uses a different clock hierarchy for ssi than = -other +> +> > clks[IMX6SL_CLK_SSI1_IPG] = imx_clk_gate2_shared("ssi1_ipg", "ipg", +> +> It seems to the that imx25 uses a different clock hierarchy for ssi than other > imx chips. ->=20 -> Documentation/devicetree/bindings/clock/imx25-clock.txt: ssi1_ipg_per = - 55 -> Documentation/devicetree/bindings/clock/imx25-clock.txt: ssi2_ipg_per = - 56 +> +> Documentation/devicetree/bindings/clock/imx25-clock.txt: ssi1_ipg_per 55 +> Documentation/devicetree/bindings/clock/imx25-clock.txt: ssi2_ipg_per 56 > Documentation/devicetree/bindings/clock/imx25-clock.txt: ssi1_ipg 117 > Documentation/devicetree/bindings/clock/imx25-clock.txt: ssi2_ipg 118 -> Documentation/devicetree/bindings/clock/imx31-clock.txt: ssi1_gate = - 32 -> Documentation/devicetree/bindings/clock/imx31-clock.txt: ssi2_gate = - 52 +> Documentation/devicetree/bindings/clock/imx31-clock.txt: ssi1_gate 32 +> Documentation/devicetree/bindings/clock/imx31-clock.txt: ssi2_gate 52 > Documentation/devicetree/bindings/clock/imx35-clock.txt: ssi_sel 22 -> Documentation/devicetree/bindings/clock/imx35-clock.txt: ssi1_div_pre = - 23 -> Documentation/devicetree/bindings/clock/imx35-clock.txt: ssi1_div_post = - 24 -> Documentation/devicetree/bindings/clock/imx35-clock.txt: ssi2_div_pre = - 25 -> Documentation/devicetree/bindings/clock/imx35-clock.txt: ssi2_div_post = - 26 +> Documentation/devicetree/bindings/clock/imx35-clock.txt: ssi1_div_pre 23 +> Documentation/devicetree/bindings/clock/imx35-clock.txt: ssi1_div_post 24 +> Documentation/devicetree/bindings/clock/imx35-clock.txt: ssi2_div_pre 25 +> Documentation/devicetree/bindings/clock/imx35-clock.txt: ssi2_div_post 26 > Documentation/devicetree/bindings/clock/imx35-clock.txt: ssi1_gate 68 > Documentation/devicetree/bindings/clock/imx35-clock.txt: ssi2_gate 69 ->=20 +> > Others don't have ssiX_ipg_per. ->=20 -> > It is not clear to me what is the real issue this patch is trying to fi= -x. ->=20 +> +> > It is not clear to me what is the real issue this patch is trying to fix. +> > True. This needs clarification. ->=20 -> I found that, in oder to get a tx clock out of the SSI, both ssi1_ipg_per= - and +> +> I found that, in oder to get a tx clock out of the SSI, both ssi1_ipg_per and > ssi1_ipg clocks must be active. ->=20 +> > The fsl_ssi driver only activates ssi1_ipg. ->=20 -> If I activate ssi1_ipg_per in the bootloader, clk_disable_unused() deacti= -vates it. ->=20 -> (My codec chip does not use a dedicated clock line. It takes the bit cloc= -k that +> +> If I activate ssi1_ipg_per in the bootloader, clk_disable_unused() deactivates it. +> +> (My codec chip does not use a dedicated clock line. It takes the bit clock that > is the output of SSI. Are you maybe using ssi1_ipg_per for your codec and > enable it there?) ->=20 +> > In my first mail, I was wondering about imx25 uart1, where we also have > uart1_ipg and uart_ipg_per and the clock seeting is ->=20 -> clk[uart1_ipg] =3D imx_clk_gate("uart1_ipg", "ipg", ccm(CCM_CGCR2), 14= -); ->=20 +> +> clk[uart1_ipg] = imx_clk_gate("uart1_ipg", "ipg", ccm(CCM_CGCR2), 14); +> > In this case, both uart1 and uart_ipg_per are listed in the device tree ->=20 -> uart1: serial@43f90000 { = - =20 +> +> uart1: serial at 43f90000 { > ... -> clocks =3D <&clks 120>, <&clks 57>; = - =20 -> clock-names =3D "ipg", "per"; = - =20 -> }; = - =20 ->=20 +> clocks = <&clks 120>, <&clks 57>; +> clock-names = "ipg", "per"; +> }; +> > Documentation/devicetree/bindings/clock/imx25-clock.txt > uart_ipg_per 57 > uart1_ipg 120 ->=20 +> > and the driver enables both clocks explicitly. So they are not unused. ->=20 ->=20 -> Doing something like this is not an option for ssi, this will not work wi= -th +> +> +> Doing something like this is not an option for ssi, this will not work with > imx31, 35 etc. ->=20 +> > Therefore, I suggest setting ssi1_ipg_per as parent of ssi1_ipg. > The right wayto fix this is to add the missing ipg_per clock to the DTB @@ -135,9 +113,9 @@ in hardware. The sound/soc/fsl/fsl_ssi.c driver does already handle a second clock as bitclock. It only needs to be specified in the DTB: &ssi1 { - clocks =3D <&clks 117>, <&clk 55>; - clock-names =3D "ipg", "baud"; + clocks = <&clks 117>, <&clk 55>; + clock-names = "ipg", "baud"; }; -Lothar Wa=C3=9Fmann +Lothar Wa?mann diff --git a/a/content_digest b/N1/content_digest index e636567..9d38cb3 100644 --- a/a/content_digest +++ b/N1/content_digest @@ -3,150 +3,119 @@ "ref\020180308140851.GA25601@botnar.kaiser.cx\0" "ref\0CAOMZO5BdBWExt9FvPrfUG74Y62e4EUs6cqFyfVs5JuUtym00hQ@mail.gmail.com\0" "ref\020180308164654.GA31199@botnar.kaiser.cx\0" - "From\0Lothar Wa\303\237mann <LW@karo-electronics.de>\0" - "Subject\0Re: [PATCH] clk: imx25: set correct parents for ssi ipg clocks\0" + "From\0LW@karo-electronics.de (Lothar Wa\303\237mann)\0" + "Subject\0[PATCH] clk: imx25: set correct parents for ssi ipg clocks\0" "Date\0Fri, 9 Mar 2018 17:02:21 +0100\0" - "To\0Martin Kaiser <martin@kaiser.cx>\0" - "Cc\0Fabio Estevam <festevam@gmail.com>" - Stephen Boyd <sboyd@kernel.org> - Michael Turquette <mturquette@baylibre.com> - linux-kernel <linux-kernel@vger.kernel.org> - Sascha Hauer <kernel@pengutronix.de> - Fabio Estevam <fabio.estevam@nxp.com> - Shawn Guo <shawnguo@kernel.org> - linux-clk <linux-clk@vger.kernel.org> - " moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE <linux-arm-kernel@lists.infradead.org>\0" + "To\0linux-arm-kernel@lists.infradead.org\0" "\00:1\0" "b\0" "Hi,\n" "\n" "On Thu, 8 Mar 2018 17:46:54 +0100 Martin Kaiser wrote:\n" "> Hi Fabio,\n" - ">=20\n" + "> \n" "> thanks for the quick response.\n" - ">=20\n" - "> Thus wrote Fabio Estevam (festevam@gmail.com):\n" - ">=20\n" + "> \n" + "> Thus wrote Fabio Estevam (festevam at gmail.com):\n" + "> \n" "> > Hi Martin,\n" - ">=20\n" + "> \n" "> > On Thu, Mar 8, 2018 at 11:08 AM, Martin Kaiser <martin@kaiser.cx> wrote:\n" "> > > Hi Fabio,\n" - ">=20\n" - "> > > Thus wrote Fabio Estevam (festevam@gmail.com):\n" - ">=20\n" + "> \n" + "> > > Thus wrote Fabio Estevam (festevam at gmail.com):\n" + "> \n" "> > >> I get audio working from SSI1, but I guess this is due to the fact\n" "> > >> that the bootloader enables the SSI clock:\n" - ">=20\n" + "> \n" "> > >> I have the following in U-Boot:\n" - ">=20\n" + "> \n" "> > >> /* Enable the clocks */\n" "> > >> DATA 4 0x53f8000c 0x1fffffff\n" "> > >> DATA 4 0x53f80010 0xffffffff\n" "> > >> DATA 4 0x53f80014 0xfdfff\n" - ">=20\n" + "> \n" "> > > I'm using the same initial settings.\n" - ">=20\n" + "> \n" "> > > Nevertheless, ssi1_ipg_per is disbled after loading the kernel.\n" - ">=20\n" + "> \n" "> > > Digging into this a bit more, it turned out that without my patch,\n" - "> > > clk_disable_unused() recognizes ssi1_ipg_per as unused and disables i=\n" - "t.\n" - ">=20\n" + "> > > clk_disable_unused() recognizes ssi1_ipg_per as unused and disables it.\n" + "> \n" "> > > If my patch is applied and ssi1_ipg_per is declared as parent of\n" - "> > > ssi1_ipg, clk_disable_unused() will not disable it and fsl_ssi_startu=\n" - "p()\n" + "> > > ssi1_ipg, clk_disable_unused() will not disable it and fsl_ssi_startup()\n" "> > > will enable both ssi1_ipg_per and ssi1_ipg before playing sound.\n" - ">=20\n" + "> \n" "> > I can get audio to work fine without your patch on a mx25pdk.\n" - ">=20\n" + "> \n" "> this is surprising. How come the ssi1_ipg_per clock is not turned off by\n" "> clk_disable_unused()? Where is it used? Do you have\n" - ">=20\n" + "> \n" "> <&clks 55>\n" - ">=20\n" + "> \n" "> anywhere in your DT?\n" - ">=20\n" + "> \n" "> > In the other i.MX clock drivers we have this same pattern:\n" - ">=20\n" - "> > clks[IMX6SL_CLK_SSI1_IPG] =3D imx_clk_gate2_shared(\"ssi1_ipg\", =\n" - "\"ipg\",\n" - ">=20\n" - "> It seems to the that imx25 uses a different clock hierarchy for ssi than =\n" - "other\n" + "> \n" + "> > clks[IMX6SL_CLK_SSI1_IPG] = imx_clk_gate2_shared(\"ssi1_ipg\", \"ipg\",\n" + "> \n" + "> It seems to the that imx25 uses a different clock hierarchy for ssi than other\n" "> imx chips.\n" - ">=20\n" - "> Documentation/devicetree/bindings/clock/imx25-clock.txt: ssi1_ipg_per =\n" - " 55\n" - "> Documentation/devicetree/bindings/clock/imx25-clock.txt: ssi2_ipg_per =\n" - " 56\n" + "> \n" + "> Documentation/devicetree/bindings/clock/imx25-clock.txt: ssi1_ipg_per 55\n" + "> Documentation/devicetree/bindings/clock/imx25-clock.txt: ssi2_ipg_per 56\n" "> Documentation/devicetree/bindings/clock/imx25-clock.txt: ssi1_ipg 117\n" "> Documentation/devicetree/bindings/clock/imx25-clock.txt: ssi2_ipg 118\n" - "> Documentation/devicetree/bindings/clock/imx31-clock.txt: ssi1_gate =\n" - " 32\n" - "> Documentation/devicetree/bindings/clock/imx31-clock.txt: ssi2_gate =\n" - " 52\n" + "> Documentation/devicetree/bindings/clock/imx31-clock.txt: ssi1_gate 32\n" + "> Documentation/devicetree/bindings/clock/imx31-clock.txt: ssi2_gate 52\n" "> Documentation/devicetree/bindings/clock/imx35-clock.txt: ssi_sel 22\n" - "> Documentation/devicetree/bindings/clock/imx35-clock.txt: ssi1_div_pre =\n" - " 23\n" - "> Documentation/devicetree/bindings/clock/imx35-clock.txt: ssi1_div_post =\n" - " 24\n" - "> Documentation/devicetree/bindings/clock/imx35-clock.txt: ssi2_div_pre =\n" - " 25\n" - "> Documentation/devicetree/bindings/clock/imx35-clock.txt: ssi2_div_post =\n" - " 26\n" + "> Documentation/devicetree/bindings/clock/imx35-clock.txt: ssi1_div_pre 23\n" + "> Documentation/devicetree/bindings/clock/imx35-clock.txt: ssi1_div_post 24\n" + "> Documentation/devicetree/bindings/clock/imx35-clock.txt: ssi2_div_pre 25\n" + "> Documentation/devicetree/bindings/clock/imx35-clock.txt: ssi2_div_post 26\n" "> Documentation/devicetree/bindings/clock/imx35-clock.txt: ssi1_gate 68\n" "> Documentation/devicetree/bindings/clock/imx35-clock.txt: ssi2_gate 69\n" - ">=20\n" + "> \n" "> Others don't have ssiX_ipg_per.\n" - ">=20\n" - "> > It is not clear to me what is the real issue this patch is trying to fi=\n" - "x.\n" - ">=20\n" + "> \n" + "> > It is not clear to me what is the real issue this patch is trying to fix.\n" + "> \n" "> True. This needs clarification.\n" - ">=20\n" - "> I found that, in oder to get a tx clock out of the SSI, both ssi1_ipg_per=\n" - " and\n" + "> \n" + "> I found that, in oder to get a tx clock out of the SSI, both ssi1_ipg_per and\n" "> ssi1_ipg clocks must be active.\n" - ">=20\n" + "> \n" "> The fsl_ssi driver only activates ssi1_ipg.\n" - ">=20\n" - "> If I activate ssi1_ipg_per in the bootloader, clk_disable_unused() deacti=\n" - "vates it.\n" - ">=20\n" - "> (My codec chip does not use a dedicated clock line. It takes the bit cloc=\n" - "k that\n" + "> \n" + "> If I activate ssi1_ipg_per in the bootloader, clk_disable_unused() deactivates it.\n" + "> \n" + "> (My codec chip does not use a dedicated clock line. It takes the bit clock that\n" "> is the output of SSI. Are you maybe using ssi1_ipg_per for your codec and\n" "> enable it there?)\n" - ">=20\n" + "> \n" "> In my first mail, I was wondering about imx25 uart1, where we also have\n" "> uart1_ipg and uart_ipg_per and the clock seeting is\n" - ">=20\n" - "> clk[uart1_ipg] =3D imx_clk_gate(\"uart1_ipg\", \"ipg\", ccm(CCM_CGCR2), 14=\n" - ");\n" - ">=20\n" + "> \n" + "> clk[uart1_ipg] = imx_clk_gate(\"uart1_ipg\", \"ipg\", ccm(CCM_CGCR2), 14);\n" + "> \n" "> In this case, both uart1 and uart_ipg_per are listed in the device tree\n" - ">=20\n" - "> uart1: serial@43f90000 { =\n" - " =20\n" + "> \n" + "> uart1: serial at 43f90000 { \n" "> ...\n" - "> clocks =3D <&clks 120>, <&clks 57>; =\n" - " =20\n" - "> clock-names =3D \"ipg\", \"per\"; =\n" - " =20\n" - "> }; =\n" - " =20\n" - ">=20\n" + "> clocks = <&clks 120>, <&clks 57>; \n" + "> clock-names = \"ipg\", \"per\"; \n" + "> }; \n" + "> \n" "> Documentation/devicetree/bindings/clock/imx25-clock.txt\n" "> uart_ipg_per 57\n" "> uart1_ipg 120\n" - ">=20\n" + "> \n" "> and the driver enables both clocks explicitly. So they are not unused.\n" - ">=20\n" - ">=20\n" - "> Doing something like this is not an option for ssi, this will not work wi=\n" - "th\n" + "> \n" + "> \n" + "> Doing something like this is not an option for ssi, this will not work with\n" "> imx31, 35 etc.\n" - ">=20\n" + "> \n" "> Therefore, I suggest setting ssi1_ipg_per as parent of ssi1_ipg.\n" ">\n" "The right wayto fix this is to add the missing ipg_per clock to the DTB\n" @@ -155,11 +124,11 @@ "The sound/soc/fsl/fsl_ssi.c driver does already handle a second clock\n" "as bitclock. It only needs to be specified in the DTB:\n" "&ssi1 {\n" - "\tclocks =3D <&clks 117>, <&clk 55>;\n" - "\tclock-names =3D \"ipg\", \"baud\";\n" + "\tclocks = <&clks 117>, <&clk 55>;\n" + "\tclock-names = \"ipg\", \"baud\";\n" "};\n" "\n" "\n" - Lothar Wa=C3=9Fmann + Lothar Wa?mann -9e6b407861d55eefeb71e8d0a1b31d59b06392903a46957ab461da16fa71ad7f +550282e3fe7be7597f9a441d615f92eeae2e4e64f76378a1ca0262b103d3ae6d
diff --git a/a/1.txt b/N2/1.txt index 3d56133..4f489dd 100644 --- a/a/1.txt +++ b/N2/1.txt @@ -2,131 +2,109 @@ Hi, On Thu, 8 Mar 2018 17:46:54 +0100 Martin Kaiser wrote: > Hi Fabio, ->=20 +> > thanks for the quick response. ->=20 +> > Thus wrote Fabio Estevam (festevam@gmail.com): ->=20 +> > > Hi Martin, ->=20 +> > > On Thu, Mar 8, 2018 at 11:08 AM, Martin Kaiser <martin@kaiser.cx> wrote: > > > Hi Fabio, ->=20 +> > > > Thus wrote Fabio Estevam (festevam@gmail.com): ->=20 +> > > >> I get audio working from SSI1, but I guess this is due to the fact > > >> that the bootloader enables the SSI clock: ->=20 +> > > >> I have the following in U-Boot: ->=20 +> > > >> /* Enable the clocks */ > > >> DATA 4 0x53f8000c 0x1fffffff > > >> DATA 4 0x53f80010 0xffffffff > > >> DATA 4 0x53f80014 0xfdfff ->=20 +> > > > I'm using the same initial settings. ->=20 +> > > > Nevertheless, ssi1_ipg_per is disbled after loading the kernel. ->=20 +> > > > Digging into this a bit more, it turned out that without my patch, -> > > clk_disable_unused() recognizes ssi1_ipg_per as unused and disables i= -t. ->=20 +> > > clk_disable_unused() recognizes ssi1_ipg_per as unused and disables it. +> > > > If my patch is applied and ssi1_ipg_per is declared as parent of -> > > ssi1_ipg, clk_disable_unused() will not disable it and fsl_ssi_startu= -p() +> > > ssi1_ipg, clk_disable_unused() will not disable it and fsl_ssi_startup() > > > will enable both ssi1_ipg_per and ssi1_ipg before playing sound. ->=20 +> > > I can get audio to work fine without your patch on a mx25pdk. ->=20 +> > this is surprising. How come the ssi1_ipg_per clock is not turned off by > clk_disable_unused()? Where is it used? Do you have ->=20 +> > <&clks 55> ->=20 +> > anywhere in your DT? ->=20 +> > > In the other i.MX clock drivers we have this same pattern: ->=20 -> > clks[IMX6SL_CLK_SSI1_IPG] =3D imx_clk_gate2_shared("ssi1_ipg", = -"ipg", ->=20 -> It seems to the that imx25 uses a different clock hierarchy for ssi than = -other +> +> > clks[IMX6SL_CLK_SSI1_IPG] = imx_clk_gate2_shared("ssi1_ipg", "ipg", +> +> It seems to the that imx25 uses a different clock hierarchy for ssi than other > imx chips. ->=20 -> Documentation/devicetree/bindings/clock/imx25-clock.txt: ssi1_ipg_per = - 55 -> Documentation/devicetree/bindings/clock/imx25-clock.txt: ssi2_ipg_per = - 56 +> +> Documentation/devicetree/bindings/clock/imx25-clock.txt: ssi1_ipg_per 55 +> Documentation/devicetree/bindings/clock/imx25-clock.txt: ssi2_ipg_per 56 > Documentation/devicetree/bindings/clock/imx25-clock.txt: ssi1_ipg 117 > Documentation/devicetree/bindings/clock/imx25-clock.txt: ssi2_ipg 118 -> Documentation/devicetree/bindings/clock/imx31-clock.txt: ssi1_gate = - 32 -> Documentation/devicetree/bindings/clock/imx31-clock.txt: ssi2_gate = - 52 +> Documentation/devicetree/bindings/clock/imx31-clock.txt: ssi1_gate 32 +> Documentation/devicetree/bindings/clock/imx31-clock.txt: ssi2_gate 52 > Documentation/devicetree/bindings/clock/imx35-clock.txt: ssi_sel 22 -> Documentation/devicetree/bindings/clock/imx35-clock.txt: ssi1_div_pre = - 23 -> Documentation/devicetree/bindings/clock/imx35-clock.txt: ssi1_div_post = - 24 -> Documentation/devicetree/bindings/clock/imx35-clock.txt: ssi2_div_pre = - 25 -> Documentation/devicetree/bindings/clock/imx35-clock.txt: ssi2_div_post = - 26 +> Documentation/devicetree/bindings/clock/imx35-clock.txt: ssi1_div_pre 23 +> Documentation/devicetree/bindings/clock/imx35-clock.txt: ssi1_div_post 24 +> Documentation/devicetree/bindings/clock/imx35-clock.txt: ssi2_div_pre 25 +> Documentation/devicetree/bindings/clock/imx35-clock.txt: ssi2_div_post 26 > Documentation/devicetree/bindings/clock/imx35-clock.txt: ssi1_gate 68 > Documentation/devicetree/bindings/clock/imx35-clock.txt: ssi2_gate 69 ->=20 +> > Others don't have ssiX_ipg_per. ->=20 -> > It is not clear to me what is the real issue this patch is trying to fi= -x. ->=20 +> +> > It is not clear to me what is the real issue this patch is trying to fix. +> > True. This needs clarification. ->=20 -> I found that, in oder to get a tx clock out of the SSI, both ssi1_ipg_per= - and +> +> I found that, in oder to get a tx clock out of the SSI, both ssi1_ipg_per and > ssi1_ipg clocks must be active. ->=20 +> > The fsl_ssi driver only activates ssi1_ipg. ->=20 -> If I activate ssi1_ipg_per in the bootloader, clk_disable_unused() deacti= -vates it. ->=20 -> (My codec chip does not use a dedicated clock line. It takes the bit cloc= -k that +> +> If I activate ssi1_ipg_per in the bootloader, clk_disable_unused() deactivates it. +> +> (My codec chip does not use a dedicated clock line. It takes the bit clock that > is the output of SSI. Are you maybe using ssi1_ipg_per for your codec and > enable it there?) ->=20 +> > In my first mail, I was wondering about imx25 uart1, where we also have > uart1_ipg and uart_ipg_per and the clock seeting is ->=20 -> clk[uart1_ipg] =3D imx_clk_gate("uart1_ipg", "ipg", ccm(CCM_CGCR2), 14= -); ->=20 +> +> clk[uart1_ipg] = imx_clk_gate("uart1_ipg", "ipg", ccm(CCM_CGCR2), 14); +> > In this case, both uart1 and uart_ipg_per are listed in the device tree ->=20 -> uart1: serial@43f90000 { = - =20 +> +> uart1: serial@43f90000 { > ... -> clocks =3D <&clks 120>, <&clks 57>; = - =20 -> clock-names =3D "ipg", "per"; = - =20 -> }; = - =20 ->=20 +> clocks = <&clks 120>, <&clks 57>; +> clock-names = "ipg", "per"; +> }; +> > Documentation/devicetree/bindings/clock/imx25-clock.txt > uart_ipg_per 57 > uart1_ipg 120 ->=20 +> > and the driver enables both clocks explicitly. So they are not unused. ->=20 ->=20 -> Doing something like this is not an option for ssi, this will not work wi= -th +> +> +> Doing something like this is not an option for ssi, this will not work with > imx31, 35 etc. ->=20 +> > Therefore, I suggest setting ssi1_ipg_per as parent of ssi1_ipg. > The right wayto fix this is to add the missing ipg_per clock to the DTB @@ -135,9 +113,9 @@ in hardware. The sound/soc/fsl/fsl_ssi.c driver does already handle a second clock as bitclock. It only needs to be specified in the DTB: &ssi1 { - clocks =3D <&clks 117>, <&clk 55>; - clock-names =3D "ipg", "baud"; + clocks = <&clks 117>, <&clk 55>; + clock-names = "ipg", "baud"; }; -Lothar Wa=C3=9Fmann +Lothar Waßmann diff --git a/a/content_digest b/N2/content_digest index e636567..0d55b38 100644 --- a/a/content_digest +++ b/N2/content_digest @@ -22,131 +22,109 @@ "\n" "On Thu, 8 Mar 2018 17:46:54 +0100 Martin Kaiser wrote:\n" "> Hi Fabio,\n" - ">=20\n" + "> \n" "> thanks for the quick response.\n" - ">=20\n" + "> \n" "> Thus wrote Fabio Estevam (festevam@gmail.com):\n" - ">=20\n" + "> \n" "> > Hi Martin,\n" - ">=20\n" + "> \n" "> > On Thu, Mar 8, 2018 at 11:08 AM, Martin Kaiser <martin@kaiser.cx> wrote:\n" "> > > Hi Fabio,\n" - ">=20\n" + "> \n" "> > > Thus wrote Fabio Estevam (festevam@gmail.com):\n" - ">=20\n" + "> \n" "> > >> I get audio working from SSI1, but I guess this is due to the fact\n" "> > >> that the bootloader enables the SSI clock:\n" - ">=20\n" + "> \n" "> > >> I have the following in U-Boot:\n" - ">=20\n" + "> \n" "> > >> /* Enable the clocks */\n" "> > >> DATA 4 0x53f8000c 0x1fffffff\n" "> > >> DATA 4 0x53f80010 0xffffffff\n" "> > >> DATA 4 0x53f80014 0xfdfff\n" - ">=20\n" + "> \n" "> > > I'm using the same initial settings.\n" - ">=20\n" + "> \n" "> > > Nevertheless, ssi1_ipg_per is disbled after loading the kernel.\n" - ">=20\n" + "> \n" "> > > Digging into this a bit more, it turned out that without my patch,\n" - "> > > clk_disable_unused() recognizes ssi1_ipg_per as unused and disables i=\n" - "t.\n" - ">=20\n" + "> > > clk_disable_unused() recognizes ssi1_ipg_per as unused and disables it.\n" + "> \n" "> > > If my patch is applied and ssi1_ipg_per is declared as parent of\n" - "> > > ssi1_ipg, clk_disable_unused() will not disable it and fsl_ssi_startu=\n" - "p()\n" + "> > > ssi1_ipg, clk_disable_unused() will not disable it and fsl_ssi_startup()\n" "> > > will enable both ssi1_ipg_per and ssi1_ipg before playing sound.\n" - ">=20\n" + "> \n" "> > I can get audio to work fine without your patch on a mx25pdk.\n" - ">=20\n" + "> \n" "> this is surprising. How come the ssi1_ipg_per clock is not turned off by\n" "> clk_disable_unused()? Where is it used? Do you have\n" - ">=20\n" + "> \n" "> <&clks 55>\n" - ">=20\n" + "> \n" "> anywhere in your DT?\n" - ">=20\n" + "> \n" "> > In the other i.MX clock drivers we have this same pattern:\n" - ">=20\n" - "> > clks[IMX6SL_CLK_SSI1_IPG] =3D imx_clk_gate2_shared(\"ssi1_ipg\", =\n" - "\"ipg\",\n" - ">=20\n" - "> It seems to the that imx25 uses a different clock hierarchy for ssi than =\n" - "other\n" + "> \n" + "> > clks[IMX6SL_CLK_SSI1_IPG] = imx_clk_gate2_shared(\"ssi1_ipg\", \"ipg\",\n" + "> \n" + "> It seems to the that imx25 uses a different clock hierarchy for ssi than other\n" "> imx chips.\n" - ">=20\n" - "> Documentation/devicetree/bindings/clock/imx25-clock.txt: ssi1_ipg_per =\n" - " 55\n" - "> Documentation/devicetree/bindings/clock/imx25-clock.txt: ssi2_ipg_per =\n" - " 56\n" + "> \n" + "> Documentation/devicetree/bindings/clock/imx25-clock.txt: ssi1_ipg_per 55\n" + "> Documentation/devicetree/bindings/clock/imx25-clock.txt: ssi2_ipg_per 56\n" "> Documentation/devicetree/bindings/clock/imx25-clock.txt: ssi1_ipg 117\n" "> Documentation/devicetree/bindings/clock/imx25-clock.txt: ssi2_ipg 118\n" - "> Documentation/devicetree/bindings/clock/imx31-clock.txt: ssi1_gate =\n" - " 32\n" - "> Documentation/devicetree/bindings/clock/imx31-clock.txt: ssi2_gate =\n" - " 52\n" + "> Documentation/devicetree/bindings/clock/imx31-clock.txt: ssi1_gate 32\n" + "> Documentation/devicetree/bindings/clock/imx31-clock.txt: ssi2_gate 52\n" "> Documentation/devicetree/bindings/clock/imx35-clock.txt: ssi_sel 22\n" - "> Documentation/devicetree/bindings/clock/imx35-clock.txt: ssi1_div_pre =\n" - " 23\n" - "> Documentation/devicetree/bindings/clock/imx35-clock.txt: ssi1_div_post =\n" - " 24\n" - "> Documentation/devicetree/bindings/clock/imx35-clock.txt: ssi2_div_pre =\n" - " 25\n" - "> Documentation/devicetree/bindings/clock/imx35-clock.txt: ssi2_div_post =\n" - " 26\n" + "> Documentation/devicetree/bindings/clock/imx35-clock.txt: ssi1_div_pre 23\n" + "> Documentation/devicetree/bindings/clock/imx35-clock.txt: ssi1_div_post 24\n" + "> Documentation/devicetree/bindings/clock/imx35-clock.txt: ssi2_div_pre 25\n" + "> Documentation/devicetree/bindings/clock/imx35-clock.txt: ssi2_div_post 26\n" "> Documentation/devicetree/bindings/clock/imx35-clock.txt: ssi1_gate 68\n" "> Documentation/devicetree/bindings/clock/imx35-clock.txt: ssi2_gate 69\n" - ">=20\n" + "> \n" "> Others don't have ssiX_ipg_per.\n" - ">=20\n" - "> > It is not clear to me what is the real issue this patch is trying to fi=\n" - "x.\n" - ">=20\n" + "> \n" + "> > It is not clear to me what is the real issue this patch is trying to fix.\n" + "> \n" "> True. This needs clarification.\n" - ">=20\n" - "> I found that, in oder to get a tx clock out of the SSI, both ssi1_ipg_per=\n" - " and\n" + "> \n" + "> I found that, in oder to get a tx clock out of the SSI, both ssi1_ipg_per and\n" "> ssi1_ipg clocks must be active.\n" - ">=20\n" + "> \n" "> The fsl_ssi driver only activates ssi1_ipg.\n" - ">=20\n" - "> If I activate ssi1_ipg_per in the bootloader, clk_disable_unused() deacti=\n" - "vates it.\n" - ">=20\n" - "> (My codec chip does not use a dedicated clock line. It takes the bit cloc=\n" - "k that\n" + "> \n" + "> If I activate ssi1_ipg_per in the bootloader, clk_disable_unused() deactivates it.\n" + "> \n" + "> (My codec chip does not use a dedicated clock line. It takes the bit clock that\n" "> is the output of SSI. Are you maybe using ssi1_ipg_per for your codec and\n" "> enable it there?)\n" - ">=20\n" + "> \n" "> In my first mail, I was wondering about imx25 uart1, where we also have\n" "> uart1_ipg and uart_ipg_per and the clock seeting is\n" - ">=20\n" - "> clk[uart1_ipg] =3D imx_clk_gate(\"uart1_ipg\", \"ipg\", ccm(CCM_CGCR2), 14=\n" - ");\n" - ">=20\n" + "> \n" + "> clk[uart1_ipg] = imx_clk_gate(\"uart1_ipg\", \"ipg\", ccm(CCM_CGCR2), 14);\n" + "> \n" "> In this case, both uart1 and uart_ipg_per are listed in the device tree\n" - ">=20\n" - "> uart1: serial@43f90000 { =\n" - " =20\n" + "> \n" + "> uart1: serial@43f90000 { \n" "> ...\n" - "> clocks =3D <&clks 120>, <&clks 57>; =\n" - " =20\n" - "> clock-names =3D \"ipg\", \"per\"; =\n" - " =20\n" - "> }; =\n" - " =20\n" - ">=20\n" + "> clocks = <&clks 120>, <&clks 57>; \n" + "> clock-names = \"ipg\", \"per\"; \n" + "> }; \n" + "> \n" "> Documentation/devicetree/bindings/clock/imx25-clock.txt\n" "> uart_ipg_per 57\n" "> uart1_ipg 120\n" - ">=20\n" + "> \n" "> and the driver enables both clocks explicitly. So they are not unused.\n" - ">=20\n" - ">=20\n" - "> Doing something like this is not an option for ssi, this will not work wi=\n" - "th\n" + "> \n" + "> \n" + "> Doing something like this is not an option for ssi, this will not work with\n" "> imx31, 35 etc.\n" - ">=20\n" + "> \n" "> Therefore, I suggest setting ssi1_ipg_per as parent of ssi1_ipg.\n" ">\n" "The right wayto fix this is to add the missing ipg_per clock to the DTB\n" @@ -155,11 +133,11 @@ "The sound/soc/fsl/fsl_ssi.c driver does already handle a second clock\n" "as bitclock. It only needs to be specified in the DTB:\n" "&ssi1 {\n" - "\tclocks =3D <&clks 117>, <&clk 55>;\n" - "\tclock-names =3D \"ipg\", \"baud\";\n" + "\tclocks = <&clks 117>, <&clk 55>;\n" + "\tclock-names = \"ipg\", \"baud\";\n" "};\n" "\n" "\n" - Lothar Wa=C3=9Fmann + "Lothar Wa\303\237mann" -9e6b407861d55eefeb71e8d0a1b31d59b06392903a46957ab461da16fa71ad7f +1bbab15309c986faef956298643aece64f3db90aef8338d5ef9459f304970a53
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.