RE: [PATCHv6 00/28] PCI: mobiveil: fixes for Mobiveil PCIe Host Bridge IP driver

From: Z.q. Hou
Date: Wed Jul 10 2019 - 07:00:01 EST


Hi Lorenzo,

Thanks for your comments!

> -----Original Message-----
> From: Lorenzo Pieralisi <lorenzo.pieralisi@xxxxxxx>
> Sent: 2019年7月8日 19:35
> To: Z.q. Hou <zhiqiang.hou@xxxxxxx>
> Cc: linux-pci@xxxxxxxxxxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx;
> devicetree@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx;
> bhelgaas@xxxxxxxxxx; robh+dt@xxxxxxxxxx; mark.rutland@xxxxxxx;
> l.subrahmanya@xxxxxxxxxxxxxx; shawnguo@xxxxxxxxxx; Leo Li
> <leoyang.li@xxxxxxx>; catalin.marinas@xxxxxxx; will.deacon@xxxxxxx;
> Mingkai Hu <mingkai.hu@xxxxxxx>; M.h. Lian <minghuan.lian@xxxxxxx>;
> Xiaowei Bao <xiaowei.bao@xxxxxxx>
> Subject: Re: [PATCHv6 00/28] PCI: mobiveil: fixes for Mobiveil PCIe Host
> Bridge IP driver
>
> On Fri, Jul 05, 2019 at 05:56:28PM +0800, Hou Zhiqiang wrote:
> > This patch set is to add fixes for Mobiveil PCIe Host driver.
> > Splited #2, #3, #9 and #10 of v5 patches.
> >
> > Hou Zhiqiang (28):
> > PCI: mobiveil: Unify register accessors
> > PCI: mobiveil: Remove the flag MSI_FLAG_MULTI_PCI_MSI
> > PCI: mobiveil: Fix PCI base address in MEM/IO outbound windows
> > PCI: mobiveil: Update the resource list traversal function
> > PCI: mobiveil: Use WIN_NUM_0 explicitly for CFG outbound window
> > PCI: mobiveil: Use the 1st inbound window for MEM inbound
> > transactions
> > PCI: mobiveil: Fix the Class Code field
> > PCI: mobiveil: Move the link up waiting out of mobiveil_host_init()
> > PCI: mobiveil: Move IRQ chained handler setup out of DT parse
> > PCI: mobiveil: Initialize Primary/Secondary/Subordinate bus numbers
> > PCI: mobiveil: Fix devfn check in mobiveil_pcie_valid_device()
> > dt-bindings: PCI: mobiveil: Change gpio_slave and apb_csr to optional
> > PCI: mobiveil: Reformat the code for readability
> > PCI: mobiveil: Make the register updating more readable
> > PCI: mobiveil: Revise the MEM/IO outbound window initialization
> > PCI: mobiveil: Fix the returned error number
> > PCI: mobiveil: Remove an unnecessary return value check
> > PCI: mobiveil: Remove redundant var definitions and register read
> > operations
> > PCI: mobiveil: Fix the valid check for inbound and outbound window
> > PCI: mobiveil: Add the statistic of initialized inbound windows
> > PCI: mobiveil: Clear the target fields before updating the register
> > PCI: mobiveil: Mask out the lower 10-bit hardcode window size
> > PCI: mobiveil: Add upper 32-bit CPU base address setup in outbound
> > window
> > PCI: mobiveil: Add upper 32-bit PCI base address setup in inbound
> > window
> > PCI: mobiveil: Fix the CPU base address setup in inbound window
> > PCI: mobiveil: Move PCIe PIO enablement out of inbound window
> routine
> > PCI: mobiveil: Fix infinite-loop in the INTx process
> > PCI: mobiveil: Fix the potential INTx missing problem
> >
> > .../devicetree/bindings/pci/mobiveil-pcie.txt | 2 +
> > drivers/pci/controller/pcie-mobiveil.c | 529
> ++++++++++++--------
> > 2 files changed, 318 insertions(+), 213 deletions(-)
> >
>
> OK, I rewrote most of commit logs, dropped patch 25 since I do not
> understand the commit log, pushed to pci/mobiveil tentatively for v5.3.

The patch #25 is to fix the wrongly programming of the CPU base address of
the inbound windows. The current code set the CPU base address with the
PCI base address parameter which is the caller passed to set the PCI base
address of the inbound window, and the upper 32-bit of the CPU base address
of the inbound window is not set in current code.

So it means the current code only support 1:1 inbound window setting & the
CPU base address must be < 4GB. It won't work if in future someone change
it to non 1:1 mapping or the CPU base address of the inbound window > 4GB.

So, I will resend it separately.

> Having said that, you should improve commit logs writing it took me too
> much time to check them all and rewrite them.
>
> Never ever again post a massive series like this mixing refactoring fixes and
> clean-ups it was painful to review/rebase, please split patch series into small
> chunks to make my life much easier.

I understand it is wearisome, but I just want to make it better and easier to
add new driver for other developers. I will try to not mix the distinct changes
so that easy to maintain.
Thank you so much for recomposing of these changelogs.

> Please check my pci/mobiveil branch and report back if something is not in
> order.

Yes, and thanks again.

B.R,
Zhiqiang

> Lorenzo