All of lore.kernel.org
 help / color / mirror / Atom feed
From: Johan Hovold <johan@kernel.org>
To: Konrad Dybcio <konrad.dybcio@linaro.org>
Cc: "Manivannan Sadhasivam" <mani@kernel.org>,
	"Bjorn Andersson" <andersson@kernel.org>,
	"Lorenzo Pieralisi" <lpieralisi@kernel.org>,
	"Krzysztof Wilczyński" <kw@linux.com>,
	"Rob Herring" <robh@kernel.org>,
	"Bjorn Helgaas" <bhelgaas@google.com>,
	"Philipp Zabel" <p.zabel@pengutronix.de>,
	"Stanimir Varbanov" <svarbanov@mm-sol.com>,
	"Andrew Murray" <amurray@thegoodpenguin.co.uk>,
	"Vinod Koul" <vkoul@kernel.org>,
	"Marijn Suijten" <marijn.suijten@somainline.org>,
	linux-arm-msm@vger.kernel.org, linux-pci@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/4] PCI: qcom: Reshuffle reset logic in 2_7_0 .init
Date: Fri, 29 Dec 2023 15:04:23 +0100	[thread overview]
Message-ID: <ZY7R581pgn3uO6kk@hovoldconsulting.com> (raw)
In-Reply-To: <20231227-topic-8280_pcie-v1-1-095491baf9e4@linaro.org>

On Wed, Dec 27, 2023 at 11:17:19PM +0100, Konrad Dybcio wrote:
> At least on SC8280XP, if the PCIe reset is asserted, the corresponding
> AUX_CLK will be stuck at 'off'.

No, this path is exercised on every boot without the aux clock ever
being stuck at off. So something is clearly missing in this description.
 
> Assert the reset (which may end up being a NOP if it was previously
> asserted) and de-assert it back *before* turning on the clocks to avoid
> such cases.
> 
> In addition to that, in case the clock bulk enable fails, assert the
> RC reset back, as the hardware is in an unknown state at best.

This is arguably a separate change, and not necessarily one that is
correct either, so should at least go in a separate patch if it should
be done at all.

> Fixes: ed8cc3b1fc84 ("PCI: qcom: Add support for SDM845 PCIe controller")

I think you're being way to liberal with your use of Fixes tags. To
claim that this is a bug, you need to make a more convincing case for
why you think so.

Also note Qualcomm's vendor driver is similarly asserting reset after
enabling the clocks.

That driver does not seem to reset the controller on resume, though, in
case that is relevant for your current experiments.

Johan

  reply	other threads:[~2023-12-29 14:04 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-12-27 22:17 [PATCH 0/4] Qualcomm PCIe RC shutdown & reinit Konrad Dybcio
2023-12-27 22:17 ` [PATCH 1/4] PCI: qcom: Reshuffle reset logic in 2_7_0 .init Konrad Dybcio
2023-12-29 14:04   ` Johan Hovold [this message]
2023-12-29 15:01     ` Konrad Dybcio
2023-12-29 15:29       ` Johan Hovold
2023-12-30  1:16         ` Konrad Dybcio
2024-01-01 17:31           ` Manivannan Sadhasivam
2024-01-02 10:17           ` Johan Hovold
2024-01-02 17:03             ` Konrad Dybcio
2024-01-03 10:40               ` Johan Hovold
2024-01-03 12:11                 ` Konrad Dybcio
2023-12-29 15:46     ` Bjorn Helgaas
2023-12-30 22:11       ` Konrad Dybcio
2023-12-27 22:17 ` [PATCH 2/4] PCI: qcom: Cache last icc bandwidth Konrad Dybcio
2023-12-27 22:17 ` [PATCH 3/4] PCI: qcom: Read back PARF_LTSSM register Konrad Dybcio
2024-01-02 17:05   ` Manivannan Sadhasivam
2023-12-27 22:17 ` [PATCH 4/4] PCI: qcom: Implement RC shutdown/power up Konrad Dybcio
2023-12-28 15:12   ` kernel test robot
2023-12-29  3:42   ` kernel test robot
2024-01-02 17:07   ` Manivannan Sadhasivam

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ZY7R581pgn3uO6kk@hovoldconsulting.com \
    --to=johan@kernel.org \
    --cc=amurray@thegoodpenguin.co.uk \
    --cc=andersson@kernel.org \
    --cc=bhelgaas@google.com \
    --cc=konrad.dybcio@linaro.org \
    --cc=kw@linux.com \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=lpieralisi@kernel.org \
    --cc=mani@kernel.org \
    --cc=marijn.suijten@somainline.org \
    --cc=p.zabel@pengutronix.de \
    --cc=robh@kernel.org \
    --cc=svarbanov@mm-sol.com \
    --cc=vkoul@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.