linux-kernel - RE: [PATCH 2/3] clk: imx8mq: add hdmi_phy

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <DB3PR0402MB391694C610CA6B7027376E4CF54C0@DB3PR0402MB3916.eurprd04.prod.outlook.com>
Date:   Thu, 7 Mar 2019 12:56:04 +0000
From:   Anson Huang <anson.huang@....com>
To:     Lucas Stach <l.stach@...gutronix.de>,
        "shawnguo@...nel.org" <shawnguo@...nel.org>,
        "s.hauer@...gutronix.de" <s.hauer@...gutronix.de>,
        "kernel@...gutronix.de" <kernel@...gutronix.de>,
        Fabio Estevam <fabio.estevam@....com>,
        "robh+dt@...nel.org" <robh+dt@...nel.org>,
        "mark.rutland@....com" <mark.rutland@....com>,
        "mturquette@...libre.com" <mturquette@...libre.com>,
        "sboyd@...nel.org" <sboyd@...nel.org>,
        Abel Vesa <abel.vesa@....com>,
        "agx@...xcpu.org" <agx@...xcpu.org>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-clk@...r.kernel.org" <linux-clk@...r.kernel.org>
CC:     dl-linux-imx <linux-imx@....com>
Subject: RE: [PATCH 2/3] clk: imx8mq: add hdmi_phy_27m clock as pll's
 reference clock

Hi, Lucas

Best Regards!
Anson Huang

> -----Original Message-----
> From: Lucas Stach [mailto:l.stach@...gutronix.de]
> Sent: 2019年3月7日 20:06
> To: Anson Huang <anson.huang@....com>; shawnguo@...nel.org;
> s.hauer@...gutronix.de; kernel@...gutronix.de; Fabio Estevam
> <fabio.estevam@....com>; robh+dt@...nel.org; mark.rutland@....com;
> mturquette@...libre.com; sboyd@...nel.org; Abel Vesa
> <abel.vesa@....com>; agx@...xcpu.org; linux-arm-
> kernel@...ts.infradead.org; devicetree@...r.kernel.org; linux-
> kernel@...r.kernel.org; linux-clk@...r.kernel.org
> Cc: dl-linux-imx <linux-imx@....com>
> Subject: Re: [PATCH 2/3] clk: imx8mq: add hdmi_phy_27m clock as pll's
> reference clock
> 
> Am Donnerstag, den 07.03.2019, 03:41 +0000 schrieb Anson Huang:
> > There is another 27MHz OSC inside i.MX8MQ's display block and it can
> > be one of reference clocks of all PLLs, add it into clock tree and
> > also add it as PLL's reference clock.
> >
> > Signed-off-by: Anson Huang <Anson.Huang@....com>
> > ---
> >  drivers/clk/imx/clk-imx8mq.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/clk/imx/clk-imx8mq.c
> > b/drivers/clk/imx/clk-imx8mq.c index a9b3888..bb1bf9b 100644
> > --- a/drivers/clk/imx/clk-imx8mq.c
> > +++ b/drivers/clk/imx/clk-imx8mq.c
> > @@ -26,7 +26,7 @@ static u32 share_count_nand;
> >
> >  static struct clk *clks[IMX8MQ_CLK_END];
> >
> > -static const char * const pll_ref_sels[] = { "osc_25m", "osc_27m",
> > "dummy", "dummy", };
> > +static const char * const pll_ref_sels[] = { "osc_25m", "osc_27m",
> > +"osc_hdmi_phy_27m", "dummy", };
> >  static const char * const arm_pll_bypass_sels[] = {"arm_pll",
> > "arm_pll_ref_sel", };
> >  static const char * const gpu_pll_bypass_sels[] = {"gpu_pll",
> > "gpu_pll_ref_sel", };
> >  static const char * const vpu_pll_bypass_sels[] = {"vpu_pll",
> > "vpu_pll_ref_sel", }; @@ -281,6 +281,7 @@ static int
> imx8mq_clocks_probe(struct platform_device *pdev)
> >  	clks[IMX8MQ_CLK_32K] = of_clk_get_by_name(np, "ckil");
> >  	clks[IMX8MQ_CLK_25M] = of_clk_get_by_name(np, "osc_25m");
> >  	clks[IMX8MQ_CLK_27M] = of_clk_get_by_name(np, "osc_27m");
> > +	clks[IMX8MQ_CLK_HDMI_PHY_27M] = of_clk_get_by_name(np,
> > +"osc_hdmi_phy_27m");
> 
> This is not acceptable. This adds a new required clock input, without
> bothering to add the corresponding binding information or thinking about
> backwards compatibility. At this point there are existing DTs out there, which
> don't provide this required clock, which will cause a full boot regression. This
> can only be an optional clock input at this point.

What do you think if we don't get such clock from DT? Just register this fixed clock
in clock driver directly, then there will be no dependency of DT.

Anson.

> 
> Regards,
> Lucas
> 
> >  	clks[IMX8MQ_CLK_EXT1] = of_clk_get_by_name(np, "clk_ext1");
> >  	clks[IMX8MQ_CLK_EXT2] = of_clk_get_by_name(np, "clk_ext2");
> >  	clks[IMX8MQ_CLK_EXT3] = of_clk_get_by_name(np, "clk_ext3");