linux-kernel - Re: [PATCH v3 2/8] drm/msm/dsi: 28nm 8960 PHY: Get ref clock from the DT

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: <20181204173549.GB14307@google.com>
Date:   Tue, 4 Dec 2018 09:35:49 -0800
From:   Matthias Kaehlcke <mka@...omium.org>
To:     Stephen Boyd <swboyd@...omium.org>
Cc:     Andy Gross <andy.gross@...aro.org>,
        David Airlie <airlied@...ux.ie>,
        David Brown <david.brown@...aro.org>,
        Mark Rutland <mark.rutland@....com>,
        Rob Clark <robdclark@...il.com>,
        Rob Herring <robh+dt@...nel.org>,
        Archit Taneja <architt@...eaurora.org>,
        Sean Paul <seanpaul@...omium.org>,
        Rajesh Yadav <ryadav@...eaurora.org>,
        Douglas Anderson <dianders@...omium.org>,
        Jeykumar Sankaran <jsanka@...eaurora.org>,
        linux-arm-msm@...r.kernel.org, dri-devel@...ts.freedesktop.org,
        freedreno@...ts.freedesktop.org, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 2/8] drm/msm/dsi: 28nm 8960 PHY: Get ref clock from
 the DT

On Tue, Dec 04, 2018 at 08:44:00AM -0800, Stephen Boyd wrote:
> Quoting Matthias Kaehlcke (2018-11-30 16:52:48)
> > Get the ref clock of the PHY from the device tree instead of
> > hardcoding its name and rate. Use default values if the ref
> > clock is not specified.
> > 
> > Signed-off-by: Matthias Kaehlcke <mka@...omium.org>
> > ---
> > Changes in v3:
> > - use default name and rate if the ref clock is not specified
> >   in the DT
> > - store vco_ref_clk_name instead of vco_ref_clk
> > - fixed check for EPROBE_DEFER
> > - renamed VCO_REF_CLK_RATE to VCO_REF_CLK_DEFAULT_RATE
> > 
> > Changes in v2:
> > - patch added to the series
> > ---
> >  .../gpu/drm/msm/dsi/pll/dsi_pll_28nm_8960.c   | 28 +++++++++++++++----
> >  1 file changed, 23 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/msm/dsi/pll/dsi_pll_28nm_8960.c b/drivers/gpu/drm/msm/dsi/pll/dsi_pll_28nm_8960.c
> > index 49008451085b8..3af678d3317f6 100644
> > --- a/drivers/gpu/drm/msm/dsi/pll/dsi_pll_28nm_8960.c
> > +++ b/drivers/gpu/drm/msm/dsi/pll/dsi_pll_28nm_8960.c
> > @@ -47,9 +47,9 @@
> >  
> >  #define NUM_PROVIDED_CLKS      2
> >  
> > -#define VCO_REF_CLK_RATE       27000000
> > -#define VCO_MIN_RATE           600000000
> > -#define VCO_MAX_RATE           1200000000
> > +#define VCO_REF_CLK_DEFAULT_RATE       27000000
> > +#define VCO_MIN_RATE                   600000000
> > +#define VCO_MAX_RATE                   1200000000
> >  
> >  #define DSI_BYTE_PLL_CLK       0
> >  #define DSI_PIXEL_PLL_CLK      1
> > @@ -75,6 +75,8 @@ struct dsi_pll_28nm {
> >         struct platform_device *pdev;
> >         void __iomem *mmio;
> >  
> > +       const char *vco_ref_clk_name;
> 
> Can this be passed around during clk registration so we don't have to
> store it away in the structure?

makes sense, will do

> > +
> >         /* custom byte clock divider */
> >         struct clk_bytediv *bytediv;
> >  
> > @@ -125,7 +127,10 @@ static int dsi_pll_28nm_clk_set_rate(struct clk_hw *hw, unsigned long rate,
> >         DBG("rate=%lu, parent's=%lu", rate, parent_rate);
> >  
> >         temp = rate / 10;
> > -       val = VCO_REF_CLK_RATE / 10;
> > +       if (parent_rate)
> > +               val = parent_rate / 10;
> > +       else
> > +               val = VCO_REF_CLK_DEFAULT_RATE / 10;
> 
> Is the clk not properly hooked up to a parent sometimes so parent_rate
> is 0? That sounds odd given the fact that it used to be 'pxo' and that
> has always existed on the system as 27 MHz. So I'd remove this and just
> use parent_rate all the time.

I wondered about this, but since I don't have hardware for testing I
kept the previous hardcoded rate. If we know for sure that 'pxo'
always exists it should indeed be fine to use the parent rate.