Hello Frank,

Typo in subject:
s/opitimaze/optimize/


On Tue, Jan 28, 2025 at 05:07:33PM -0500, Frank Li wrote:

Bjorn's comments in https://lore.kernel.org/imx/20250123190900.GA650360@bhelgaas/ (local)

quoted

After all cpu_address_fixup() removed, we can remove use_parent_dt_ranges
in one clean up patches.

  ...

quoted

 dw_pcie_rd_other_conf
 dw_pcie_wr_other_conf
   dw_pcie_prog_outbound_atu() only called if pp->cfg0_io_shared,
   after an ECAM map via dw_pcie_other_conf_map_bus() and subsequent
   successful access; atu.cpu_addr came from pp->io_base, set by
   dw_pcie_host_init() from pci_pio_to_address(), which I'm pretty
   sure returns a CPU address.

io_base is parent_bus_address

quoted

   So this still looks wrong to me.  In addition, I think doing the
   ATU setup in *_map() and restore in *rd/wr_other_conf() is ugly
   and looks unreliable.

....

quoted

 dw_pcie_pme_turn_off
   atu.cpu_addr came from pp.msg_res, set by
   dw_pcie_host_request_msg_tlp_res() to a CPU address at the end of
   the first MMIO bridge window.  This one also looks wrong to me.

Fixed at this version.

I feel like it would have been easier if you replied to Bjorn's comments
directly in the thread instead of pasting them here (to a cover letter).


Please don't shoot the messenger, but I don't see any reply to (what I
assume is the biggest reason why Bjorn did not merge this series):

""
.cpu_addr_fixup() is a generic problem that affects dwc (dra7xx, imx6,
artpec6, intel-gw, visconti), cadence (cadence-plat), and now
apparently microchip.

I deferred these because I'm hoping we can come up with a more generic
solution that's easier to apply across all these cases.  I don't
really want to merge something that immediately needs to be reworked
for other drivers.
""

It should probably state in the cover letter how v9 addresses Bjorn's
concern when it comes to other PCI controller drivers, especially those
that are not DWC based.


Kind regards,
Niklas