On Thu, Feb 01, 2024 at 11:13:30AM -0500, Frank Li wrote: > Set outbound ATU map memory write to send PCI message. So one MMIO write > can trigger a PCI message, such as PME_Turn_Off. > > Add common dw_pcie_send_pme_turn_off_by_atu() function. > ... > - if (!pci->pp.ops->pme_turn_off) > - return 0; > + if (pci->pp.ops->pme_turn_off) > + pci->pp.ops->pme_turn_off(&pci->pp); > + else > + ret = dw_pcie_send_pme_turn_off_by_atu(pci); I think it's nice if function names match the function pointer names. E.g., we currently already have: .pme_turn_off = ls_pcie_send_turnoff_msg, .pme_turn_off = ls1021a_pcie_send_turnoff_msg, .pme_turn_off = ls1043a_pcie_send_turnoff_msg, which is slightly annoying because it's always useful to compare implementations, but "git grep pme_turn_off" doesn't find the actual functions, so I wish these were named "ls_pcie_pme_turn_off()", etc. You don't have to fix those existing layerscape ones now, but I think the same applies to dw_pcie_send_pme_turn_off_by_atu(): it would be nice if it were named something like "dw_pcie_pme_turn_off()" so grep/cscope would find it easily. Bjorn
