linux-kernel - Re: [PATCH 06/13] phy: qcom-qmp-pcie: drop bogus register update

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: <YzVU0gC4gT9FwBL+@hovoldconsulting.com>
Date:   Thu, 29 Sep 2022 10:18:26 +0200
From:   Johan Hovold <johan@...nel.org>
To:     Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
Cc:     Johan Hovold <johan+linaro@...nel.org>,
        Vinod Koul <vkoul@...nel.org>, Andy Gross <agross@...nel.org>,
        Bjorn Andersson <andersson@...nel.org>,
        Konrad Dybcio <konrad.dybcio@...ainline.org>,
        Kishon Vijay Abraham I <kishon@...com>,
        linux-arm-msm@...r.kernel.org, linux-phy@...ts.infradead.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 06/13] phy: qcom-qmp-pcie: drop bogus register update

On Thu, Sep 29, 2022 at 10:56:11AM +0300, Dmitry Baryshkov wrote:
> On 29/09/2022 10:12, Johan Hovold wrote:
> > On Wed, Sep 28, 2022 at 10:48:40PM +0300, Dmitry Baryshkov wrote:
> >> On 28/09/2022 22:10, Dmitry Baryshkov wrote:

> >> The programmings starts with <0x804 0x1 0x0>, writing 1 to
> >> PCS_POWER_DOWN_CONTROL (which if I'm not mistaken we do not do at this
> >> moment). Then after writing all the serdes/tx/rx/pcs/pcs_misc tables
> >> comes the write <0x804 0x3 0x0> (which you are trying to remove here).
> > 
> > The PHY would already have been powered on with the mainline driver, that
> > write has already happened.
> > 
> > Whether or not PCIe support for SDM845 has been broken since it was
> > first mainlined almost three years ago is a separate issue. I assume
> > Bjorn tested it before sending it upstream.
> > 
> > 	421c9a0e9731 ("phy: qcom: qmp: Add SDM845 PCIe QMP PHY support")
> 
> On SDM845 PCIe0 is used for the WiFi, e.g. on the RB3 (dragonboard845) 
> device. The PHY definitely works with the upstream kernels.

Ok, good.

> >> Same sequence applies to the PCIe PHY on msm8998.
> >>
> >> Most newer PHYs have the expected sequence (of writing 0x3 to
> >> PCS_POWER_DOWN_CONTROL) before writing all registers.
> >>
> >> As a short summary: unless we get any additional information that 8998
> >> and sdm845 tables are incorrect, I'd suggest adding a conditional here
> >> (ugh) and using it here and in qmp_pcie_init() call.
> > 
> > I see little point in doing that unless you dig out an SDM845, confirm
> > that it has never worked with upstream, and update the init sequence
> > first.
> 
> Digging out an sdm845 is not a problem per se. It works, but it also has 
> an additional regwrite that this patch tries to remove. I will try 
> checking whether it works with this register write being removed or not.

Right, but as I already mentioned that register already holds the same
value (0x3) when the second redundant write happens. So this should be a
no-op on sdm845 currently.

(Only MSM8996 had an intermediate update of that register in its PCS
table and therefore needs this second update.)

> >> Vinod, Bjorn, do you have any additional info?
> > 
> > An explanation of the split POWER_DOWN_CONTROL updates on MSM8996 would
> > be good to have either way

Johan