Re: [PATCH v7 3/3] PCI: qcom: Add IPQ60xx support

Johan Hovold
2022-06-21T07:53:48Z

On Tue, Jun 21, 2022 at 06:39:45AM +0300, Baruch Siach wrote:
> Hi Johan,
>
> Thanks for your review comments.
>
> On Mon, Jun 20 2022, Johan Hovold wrote:
> > On Sun, Jun 12, 2022 at 01:18:35PM +0300, Baruch Siach wrote:
> >> From: Selvam Sathappan Periakaruppan <quic_speriaka@quicinc.com>
> >>
> >> IPQ60xx series of SoCs have one port of PCIe gen 3. Add support for that
> >> platform.
> >>
> >> The code is based on downstream[1] Codeaurora kernel v5.4 (branch
> >> win.linuxopenwrt.2.0).
> >>
> >> Split out the DBI registers access part from .init into .post_init. DBI
> >> registers are only accessible after phy_power_on().
> >>
> >> [1] https://source.codeaurora.org/quic/qsdk/oss/kernel/linux-ipq-5.4/
> >>
> >> Signed-off-by: Selvam Sathappan Periakaruppan <speriaka@codeaurora.org>
> >> Signed-off-by: Baruch Siach <baruch.siach@siklu.com>
> >> ---
> >
> >> +static void qcom_pcie_deinit_2_9_0(struct qcom_pcie *pcie)
> >> +{
> >> + struct qcom_pcie_resources_2_9_0 *res = &pcie->res.v2_9_0;
> >> +
> >> + clk_bulk_disable_unprepare(ARRAY_SIZE(res->clks), res->clks);
> >
> > Assert reset as you do in the init error path?
>
> Not sure about that. As I understand, the reset assert/deassert sequence
> on init is meant to ensure clean startup state. Deinit most likely does
> not need that. So maybe I should remove reset assert from init error
> path instead?

Yeah, I think the init error path and deinit should at least be
consistent.

> As always, this code sequence is from downstream kernel. I have no
> access to documentation.

I feel your pain. ;)

Johan