On Sun, Jan 22, 2023 at 03:11:16AM +0300, Serge Semin wrote: > On Fri, Jan 20, 2023 at 04:50:36PM -0600, Bjorn Helgaas wrote: > > On Fri, Jan 13, 2023 at 08:14:06PM +0300, Serge Semin wrote: > > > Since the DW PCIe RP/EP driver is about to be updated to register the DW > > > eDMA-based DMA-engine the drivers build modes must be synchronized. > > > Currently the DW PCIe RP/EP driver is always built as a builtin module. > > > Meanwhile the DW eDMA driver can be built as a loadable module. Thus in > > > the later case the kernel with DW PCIe controllers support will fail to be > > > linked due to lacking the DW eDMA probe/remove symbols. At the same time > > > forcibly selecting the DW eDMA driver from the DW PCIe RP/EP kconfig will > > > effectively eliminate the tristate type of the former driver fixing it to > > > just the builtin kernel module. > > > > > > Seeing the DW eDMA engine isn't that often met built into the DW PCIe > > > Root-ports and End-points let's convert the DW eDMA driver config to being > > > more flexible instead of just forcibly selecting the DW eDMA kconfig. In > > > order to do that first the DW eDMA PCIe driver config should be converted > > > to being depended from the DW eDMA core config instead of selecting the > > > one. Second the DW eDMA probe and remove symbols should be referenced only > > > if they are reachable by the caller. Thus the user will be able to build > > > the DW eDMA core driver with any type, meanwhile the dependent code will > > > be either restricted to the same build type (e.g. DW eDMA PCIe driver if > > > DW eDMA driver is built as a loadable module) or just won't be able to use > > > the eDMA engine registration/de-registration functionality (e.g. DW PCIe > > > RP/EP driver if DW eDMA driver is built as a loadable module). > > > > I'm trying to write the merge commit log, and I understand the linking > > issue, but I'm having a hard time figuring out what the user-visible > > scenarios are here. > > > > I assume there's something that works when CONFIG_PCIE_DW=y and > > CONFIG_DW_EDMA_PCIE=y but does *not* work when CONFIG_PCIE_DW=y and > > CONFIG_DW_EDMA_PCIE=m? > > No. The DW eDMA code availability (in other words the CONFIG_DW_EDMA > config value) determines whether the corresponding driver (DW PCIe > RP/EP or DW eDMA PCI) is capable to perform the eDMA engine probe > procedure. Additionally both drivers has the opposite dependency from > the DW eDMA code. > | | DW PCIe RP/EP | DW eDMA PCIe | > | CONFIG_DW_EDMA +----------------------+----------------------+ > | | Probe eDMA | KConfig | Probe eDMA | Kconfig | > +----------------+------------+---------+------------+---------+ > | y | YES | y,n | YES | y,m,n | > | m | NO | y,n | YES | m,n | > | n | NO | y,n | NO | n | > +--------------------------------------------------------------+ > > Basically it means the DW PCIe RP/EP driver will be able to probe the > DW eDMA engine only if the corresponding driver is built into the > kernel. At the same time the DW PCIe RP/EP driver doesn't depend on > the DW eDMA core module config state. The DW eDMA PCIe driver in > opposite depends on the DW eDMA code config state, but will always be > able to probe the DW eDMA engine as long as the corresponding code is > loaded as either a part of the kernel or as a loadable module. > > > If both scenarios worked the same, I would think the existing > > dw_edma_pcie_probe() would be enough, and you wouldn't need to call > > dw_pcie_edma_detect() from dw_pcie_host_init() and dw_pcie_ep_init(). > > No. These methods have been implemented for the absolutely different > drivers. > dw_edma_pcie_probe() is called for an end-point PCIe-device found on a > PCIe-bus. > dw_pcie_host_init()/dw_pcie_ep_init() and dw_pcie_edma_detect() are > called for a platform-device representing a DW PCIe RP/EP controller. > In other words dw_pcie_edma_detect() and dw_edma_pcie_probe() are in > no means interchangeable. The question is what the user-visible difference between CONFIG_DW_EDMA_PCIE=y and CONFIG_DW_EDMA_PCIE=m is. If there were no difference, dw_pcie_host_init() would not need to call dw_pcie_edma_detect(). Can you give me a one- or two-sentence merge commit comment that explains why we want to merge this? "Relax driver config settings" doesn't tell us that. Bjorn
