On 22/06/2022 00:16, Bjorn Helgaas wrote:
On Tue, Jun 21, 2022 at 11:45:12PM +0300, Dmitry Baryshkov wrote:
On Tue, 21 Jun 2022 at 23:32, Bjorn Helgaas <helgaas@xxxxxxxxxx> wrote:
On Tue, Jun 21, 2022 at 01:23:30PM +0200, Robert Marko wrote:
IPQ8074 has one Gen2 and one Gen3 port, currently the Gen2 port will
cause the system to hang as its using DBI registers in the .init
and those are only accesible after phy_power_on().
Is the fact that IPQ8074 has both a Gen2 and a Gen3 port relevant to
this patch? I don't see the connection.
I see that qcom_pcie_host_init() does:
qcom_pcie_host_init
pcie->cfg->ops->init(pcie)
phy_power_on(pcie->phy)
pcie->cfg->ops->post_init(pcie)
and that you're moving DBI register accesses from
qcom_pcie_init_2_3_3() to qcom_pcie_post_init_2_3_3().
But I also see DBI register accesses in other .init() functions:
qcom_pcie_init_2_1_0
qcom_pcie_init_1_0_0 (oddly out of order)
qcom_pcie_init_2_3_2
qcom_pcie_init_2_4_0
Why do these accesses not need to be moved? I assume it's because
pcie->phy is an optional PHY and phy_power_on() does nothing on those
controllers?
Whatever the reason, I think the DBI accesses should be done
consistently in .post_init(). I see that Dmitry's previous patches
removed all those .post_init() functions, but I think the consistency
is worth having.
Perhaps we could reorder the patches so this patch comes first, moves
the DBI accesses into .post_init(), then Dmitry's patches could be
rebased on top to drop the clock handling?
I don't think there is a need to reorder patches. My patches do not
remove support for post_init(), they drop the callbacks code. Thus one
can reinstate necessary code back.
There's not a *need* to reorder them, but I think it would make the
patches smaller and more readable because we wouldn't be removing and
then re-adding the functions.
Ack. I'm fine then with rebasing my patches on top of Robert's patchset.
I'll send the next revision after getting this patchset into the form.
--
With best wishes
Dmitry
