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@xxxxxxxxxxx> > >> > >> 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@xxxxxxxxxxxxxx> > >> Signed-off-by: Baruch Siach <baruch.siach@xxxxxxxxx> > >> --- > > > >> +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