RE: [PATCH V2 1/2] clk: imx6sx: add missing lvds2 clock to the clock tree

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




Anson Huang
Best Regards!


> -----Original Message-----
> From: Shawn Guo [mailto:shawnguo@xxxxxxxxxx]
> Sent: Thursday, April 19, 2018 10:57 PM
> To: Anson Huang <anson.huang@xxxxxxx>
> Cc: kernel@xxxxxxxxxxxxxx; Fabio Estevam <fabio.estevam@xxxxxxx>;
> robh+dt@xxxxxxxxxx; mark.rutland@xxxxxxx; linux@xxxxxxxxxxxxxxx;
> mturquette@xxxxxxxxxxxx; sboyd@xxxxxxxxxx; S.j. Wang
> <shengjiu.wang@xxxxxxx>; dl-linux-imx <linux-imx@xxxxxxx>;
> linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx;
> linux-kernel@xxxxxxxxxxxxxxx; linux-clk@xxxxxxxxxxxxxxx
> Subject: Re: [PATCH V2 1/2] clk: imx6sx: add missing lvds2 clock to the clock tree
> 
> On Mon, Mar 19, 2018 at 10:30:44AM +0800, Anson Huang wrote:
> > i.MX6SX has lvds2 (analog clock2), an I/O clock like lvds1.
> > And this lvds2, along with lvds1, can be used to provide external
> > clock source to the internal pll, such as pll4_audio and pll5_video.
> >
> > This patch mainly adds the lvds2 to the clock tree and fix its
> > relationship with pll accordingly.
> >
> > Signed-off-by: Anson Huang <Anson.Huang@xxxxxxx>
> > Signed-off-by: Shengjiu Wang <shengjiu.wang@xxxxxxx>
> > ---
> >  drivers/clk/imx/clk-imx6sx.c             | 8 ++++++--
> >  include/dt-bindings/clock/imx6sx-clock.h | 6 +++++-
> >  2 files changed, 11 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/clk/imx/clk-imx6sx.c
> > b/drivers/clk/imx/clk-imx6sx.c index e6d389e..478ad0d 100644
> > --- a/drivers/clk/imx/clk-imx6sx.c
> > +++ b/drivers/clk/imx/clk-imx6sx.c
> > @@ -80,7 +80,7 @@ static const char *lvds_sels[]	= {
> >  	"arm", "pll1_sys", "dummy", "dummy", "dummy", "dummy", "dummy",
> "pll5_video_div",
> >  	"dummy", "dummy", "pcie_ref_125m", "dummy", "usbphy1", "usbphy2",
> > }; -static const char *pll_bypass_src_sels[] = { "osc", "lvds1_in", };
> > +static const char *pll_bypass_src_sels[] = { "osc", "lvds1_in",
> > +"lvds2_in", "dummy", };
> >  static const char *pll1_bypass_sels[] = { "pll1", "pll1_bypass_src",
> > };  static const char *pll2_bypass_sels[] = { "pll2",
> > "pll2_bypass_src", };  static const char *pll3_bypass_sels[] = {
> > "pll3", "pll3_bypass_src", }; @@ -158,8 +158,9 @@ static void __init
> imx6sx_clocks_init(struct device_node *ccm_node)
> >  	clks[IMX6SX_CLK_IPP_DI0] = of_clk_get_by_name(ccm_node, "ipp_di0");
> >  	clks[IMX6SX_CLK_IPP_DI1] = of_clk_get_by_name(ccm_node, "ipp_di1");
> >
> > -	/* Clock source from external clock via CLK1 PAD */
> > +	/* Clock source from external clock via CLK1/2 PAD */
> >  	clks[IMX6SX_CLK_ANACLK1] = imx_obtain_fixed_clock("anaclk1", 0);
> > +	clks[IMX6SX_CLK_ANACLK2] = imx_obtain_fixed_clock("anaclk2", 0);
> 
> It seems to me that anaclk clocks are similar to ipp_di, and could be handled in
> the same way as ipp_di clocks.  If that's the case, I would suggest we do the
> following.
> 
> 1. Kill clocks container node by dropping 'reg' property and naming
>    clock nodes uniquely.  This is not strictly related to what we try
>    to do here, but just to address DT maintainers' concern on 'clocks'
>    container node.
> 
> 	clk_ckil: clock-ckil {
> 		compatible = "fixed-clock";
> 		#clock-cells = <0>;
> 		clock-frequency = <32768>;
> 		clock-output-names = "ckil";
> 	};
> 
> 	clk_osc: clock-osc {
> 		compatible = "fixed-clock";
> 		#clock-cells = <0>;
> 		clock-frequency = <24000000>;
> 		clock-output-names = "osc";
> 	};
> 
> 	clk_ipp_di0: clock-ipp-di0 {
> 		compatible = "fixed-clock";
> 		#clock-cells = <0>;
> 		clock-frequency = <0>;
> 		clock-output-names = "ipp_di0";
> 	};
> 
> 	clk_ipp_di1: clock-ipp-di1 {
> 		compatible = "fixed-clock";
> 		#clock-cells = <0>;
> 		clock-frequency = <0>;
> 		clock-output-names = "ipp_di1";
> 	};
> 
> 	clks: ccm@20c4000 {
> 		compatible = "fsl,imx6sx-ccm";
> 		reg = <0x020c4000 0x4000>;
> 		interrupts = <GIC_SPI 87 IRQ_TYPE_LEVEL_HIGH>,
> 			     <GIC_SPI 88 IRQ_TYPE_LEVEL_HIGH>;
> 		#clock-cells = <1>;
> 		clocks = <&clk_ckil>, <&clk_osc>, <&clk_ipp_di0>, <&clk_ipp_di1>;
> 		clock-names = "ckil", "osc", "ipp_di0", "ipp_di1";
> 	};
> 
> 2. Patch clock driver to have anaclk1 and anaclk2 handled in the same
>    way as ipp_di clocks.
> 
> 	clks[IMX6SX_CLK_ANACLK1] = of_clk_get_by_name(ccm_node, "anaclk1");
> 	clks[IMX6SX_CLK_ANACLK2] = of_clk_get_by_name(ccm_node, "anaclk2");
> 
> 3. Add anaclk1 and anaclk2 with clock-frequency being 0 by default, just
>    like ipp_di clocks.
> 
> 	clk_anaclk1: clock-anaclk1 {
> 		compatible = "fixed-clock";
> 		#clock-cells = <0>;
> 		clock-frequency = <0>;
> 		clock-output-names = "anaclk1";
> 	};
> 
> 	clk_anaclk2: clock-anaclk2 {
> 		compatible = "fixed-clock";
> 		#clock-cells = <0>;
> 		clock-frequency = <0>;
> 		clock-output-names = "anaclk2";
> 	};
> 
> 	clks: ccm@20c4000 {
> 		compatible = "fsl,imx6sx-ccm";
> 		reg = <0x020c4000 0x4000>;
> 		interrupts = <GIC_SPI 87 IRQ_TYPE_LEVEL_HIGH>,
> 			     <GIC_SPI 88 IRQ_TYPE_LEVEL_HIGH>;
> 		#clock-cells = <1>;
> 		clocks = <&clk_ckil>, <&clk_osc>,
> 			 <&clk_ipp_di0>, <&clk_ipp_di1>,
> 			 <&clk_anaclk1>, <&clk_anaclk2>;
> 		clock-names = "ckil", "osc",
> 			      "ipp_di0", "ipp_di1",
> 			      "anaclk1", "anaclk2";
> 	};
> 
> 4. Overwrite anaclk2 clock-frequency in imx6sx-sabreauto.dts.
> 
> &clk_anaclk2 {
> 	clock-frequency = <24576000>;
> };
> 
> Please test and let me know whether it works or not.  Thanks.
> 
> Shawn

It is working, see below clk tree dump, I will send a V3 patch, thanks.

Anson. 


root@imx6qpdlsolox:~# cat /sys/kernel/debug/clk/clk_summary
                                 enable  prepare  protect
   clock                          count    count    count        rate   accuracy   phase
----------------------------------------------------------------------------------------
 dummy                                3        3        0           0          0 0
    cko1_sel                          0        0        0           0          0 0
       cko1_podf                      0        0        0           0          0 0
          cko1                        0        0        0           0          0 0
             cko                      0        0        0           0          0 0
    usbphy2_gate                      1        1        0           0          0 0
    usbphy1_gate                      1        1        0           0          0 0
 anaclk2                              0        0        0    24576000          0 0
    lvds2_in                          0        0        0    24576000          0 0
 anaclk1                              0        0        0           0          0 0
    lvds1_in                          0        0        0           0          0 0
 ipp_di1                              0        0        0           0          0 0
 ipp_di0                              0        0        0           0          0 0
 osc                                  6        6        0    24000000          0 0

> 
> >
> >  	np = of_find_compatible_node(NULL, NULL, "fsl,imx6sx-anatop");
> >  	base = of_iomap(np, 0);
> > @@ -228,7 +229,9 @@ static void __init imx6sx_clocks_init(struct
> device_node *ccm_node)
> >  	clks[IMX6SX_CLK_PCIE_REF_125M] = imx_clk_gate("pcie_ref_125m",
> > "pcie_ref", base + 0xe0, 19);
> >
> >  	clks[IMX6SX_CLK_LVDS1_OUT] = imx_clk_gate_exclusive("lvds1_out",
> > "lvds1_sel", base + 0x160, 10, BIT(12));
> > +	clks[IMX6SX_CLK_LVDS2_OUT] = imx_clk_gate_exclusive("lvds2_out",
> > +"lvds2_sel", base + 0x160, 11, BIT(13));
> >  	clks[IMX6SX_CLK_LVDS1_IN]  = imx_clk_gate_exclusive("lvds1_in",
> "anaclk1",   base + 0x160, 12, BIT(10));
> > +	clks[IMX6SX_CLK_LVDS2_IN]  = imx_clk_gate_exclusive("lvds2_in",
> "anaclk2",   base + 0x160, 13, BIT(11));
> >
> >  	clks[IMX6SX_CLK_ENET_REF] = clk_register_divider_table(NULL,
> "enet_ref", "pll6_enet", 0,
> >  			base + 0xe0, 0, 2, 0, clk_enet_ref_table, @@ -270,6 +273,7 @@
> > static void __init imx6sx_clocks_init(struct device_node *ccm_node)
> >
> >  	/*                                                name
> reg           shift   width   parent_names       num_parents */
> >  	clks[IMX6SX_CLK_LVDS1_SEL]          = imx_clk_mux("lvds1_sel",
> base + 0x160, 0,      5,      lvds_sels,         ARRAY_SIZE(lvds_sels));
> > +	clks[IMX6SX_CLK_LVDS2_SEL]          = imx_clk_mux("lvds2_sel",
> base + 0x160, 5,      5,      lvds_sels,         ARRAY_SIZE(lvds_sels));
> >
> >  	np = ccm_node;
> >  	base = of_iomap(np, 0);
> > diff --git a/include/dt-bindings/clock/imx6sx-clock.h
> > b/include/dt-bindings/clock/imx6sx-clock.h
> > index 36f0324..cd2d6c5 100644
> > --- a/include/dt-bindings/clock/imx6sx-clock.h
> > +++ b/include/dt-bindings/clock/imx6sx-clock.h
> > @@ -275,6 +275,10 @@
> >  #define IMX6SX_PLL6_BYPASS		262
> >  #define IMX6SX_PLL7_BYPASS		263
> >  #define IMX6SX_CLK_SPDIF_GCLK		264
> > -#define IMX6SX_CLK_CLK_END		265
> > +#define IMX6SX_CLK_LVDS2_SEL		265
> > +#define IMX6SX_CLK_LVDS2_OUT		266
> > +#define IMX6SX_CLK_LVDS2_IN		267
> > +#define IMX6SX_CLK_ANACLK2		268
> > +#define IMX6SX_CLK_CLK_END		269
> >
> >  #endif /* __DT_BINDINGS_CLOCK_IMX6SX_H */
> > --
> > 2.7.4
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-clk"
> > in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo
> > info at
> > https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fvger
> > .kernel.org%2Fmajordomo-info.html&data=02%7C01%7CAnson.Huang%40nx
> p.com
> > %7Cfaec2b0e3f024446b4b908d5a605f71c%7C686ea1d3bc2b4c6fa92cd99c5c
> 301635
> > %7C0%7C0%7C636597466911971203&sdata=YXDSFv3JbKb26ncEJmpf0kUUp1
> DdVY6Fmc
> > utQZW1%2FM8%3D&reserved=0
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux