linux-kernel - Re: [PATCH v3 2/2] phy: qcom: Introduce new eDP PHY driver

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 for Android: free password hash cracker in your pocket

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

Message-ID: <YXZYfh+yfNFkqY0a@matsya>
Date:   Mon, 25 Oct 2021 12:40:54 +0530
From:   Vinod Koul <vkoul@...nel.org>
To:     Bjorn Andersson <bjorn.andersson@...aro.org>
Cc:     Kishon Vijay Abraham I <kishon@...com>,
        Rob Herring <robh+dt@...nel.org>,
        Dmitry Baryshkov <dmitry.baryshkov@...aro.org>,
        linux-arm-msm@...r.kernel.org, linux-phy@...ts.infradead.org,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
        abhinavk@...eaurora.org, Stephen Boyd <sboyd@...nel.org>
Subject: Re: [PATCH v3 2/2] phy: qcom: Introduce new eDP PHY driver

On 22-10-21, 10:16, Bjorn Andersson wrote:
> On Thu 21 Oct 10:40 PDT 2021, Vinod Koul wrote:
> 
> > On 16-10-21, 16:21, Bjorn Andersson wrote:
> > > Many recent Qualcomm platforms comes with native DP and eDP support.
> > > This consists of a controller in the MDSS and a QMP-like PHY.
> > > 
> > > While similar to the well known QMP block, the eDP PHY only has TX lanes
> > > and the programming sequences are slightly different. Rather than
> > > continuing the trend of parameterize the QMP driver to pieces, this
> > > introduces the support as a new driver.
> > > 
> > > The registration of link and pixel clocks are borrowed from the QMP
> > > driver. The non-DP link frequencies are omitted for now.
> > > 
> > > The eDP PHY is very similar to the dedicated (non-USB) DP PHY, but only
> > > the prior is supported for now.
> > 
> > since this is QMP phy, pls add an explanation why common QMP driver
> > is not used here?
> 
> Looked at this again, doesn't the second paragraph answer that?

Hmmm, somehow this got missed by me! Yes sounds okay

> > > +static int qcom_edp_phy_init(struct phy *phy)
> > > +{
> [..]
> > > +	writel(0x00, edp->edp + DP_PHY_AUX_CFG0);
> > > +	writel(0x13, edp->edp + DP_PHY_AUX_CFG1);
> > > +	writel(0x24, edp->edp + DP_PHY_AUX_CFG2);
> > > +	writel(0x00, edp->edp + DP_PHY_AUX_CFG3);
> > > +	writel(0x0a, edp->edp + DP_PHY_AUX_CFG4);
> > > +	writel(0x26, edp->edp + DP_PHY_AUX_CFG5);
> > > +	writel(0x0a, edp->edp + DP_PHY_AUX_CFG6);
> > > +	writel(0x03, edp->edp + DP_PHY_AUX_CFG7);
> > > +	writel(0x37, edp->edp + DP_PHY_AUX_CFG8);
> > > +	writel(0x03, edp->edp + DP_PHY_AUX_CFG9);
> > 
> > In qmp phy we use a table for this, that looks very elegant and I am
> > sure next rev will have different magic numbers, so should we go the
> > table approach here on as well..?
> > 
> 
> Comparing the v3 and v4 USB/DP combo phy and this, the only number that
> differs is CFG_AUX2 and CFG_AUX8.
> 
> CFG_AUX8 is 0x37 for eDP and 0xb7 for DP and AUX_CFG2 seems better to
> mask together, but I don't fully understand the content yet.
> 
> I did check two other platforms and they have the same sequence, except
> one additional bit in AUX_CFG2. There also seem to be a few additional
> permutations of this value, so I don't think tables are the solution.
> 
> 
> So I think it's better if we leave this as proposed and then
> parameterize the two individual entries as needed when we go forward -
> or determine that I missed something.

okay sounds good to me

-- 
~Vinod