Re: [PATCH v9 24/27] dmaengine: dw-edma: Relax driver config settings

Bjorn Helgaas
2023-01-20T23:01:03Z

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?

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().

> Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
>
> ---
>
> Changelog v8:
> - This is a new patch added on v8 stage of the series in order to fix
> the tbot-reported build issues. (@tbot)
> ---
> drivers/dma/dw-edma/Kconfig | 5 ++++-
> include/linux/dma/edma.h | 2 +-
> 2 files changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/dma/dw-edma/Kconfig b/drivers/dma/dw-edma/Kconfig
> index 7ff17b2db6a1..2b6f2679508d 100644
> --- a/drivers/dma/dw-edma/Kconfig
> +++ b/drivers/dma/dw-edma/Kconfig
> @@ -9,11 +9,14 @@ config DW_EDMA
> Support the Synopsys DesignWare eDMA controller, normally
> implemented on endpoints SoCs.
>
> +if DW_EDMA
> +
> config DW_EDMA_PCIE
> tristate "Synopsys DesignWare eDMA PCIe driver"
> depends on PCI && PCI_MSI
> - select DW_EDMA
> help
> Provides a glue-logic between the Synopsys DesignWare
> eDMA controller and an endpoint PCIe device. This also serves
> as a reference design to whom desires to use this IP.
> +
> +endif # DW_EDMA
> diff --git a/include/linux/dma/edma.h b/include/linux/dma/edma.h
> index 08833f12b386..c062c8db472c 100644
> --- a/include/linux/dma/edma.h
> +++ b/include/linux/dma/edma.h
> @@ -101,7 +101,7 @@ struct dw_edma_chip {
> };
>
> /* Export to the platform drivers */
> -#if IS_ENABLED(CONFIG_DW_EDMA)
> +#if IS_REACHABLE(CONFIG_DW_EDMA)
> int dw_edma_probe(struct dw_edma_chip *chip);
> int dw_edma_remove(struct dw_edma_chip *chip);
> #else
> --
> 2.39.0
>
>