Hello, On Fri, Dec 23, 2022 at 03:38:16PM +0000, Ben Dooks wrote: > Moving towards adding non-pci support for the driver, move the pci > parts out of the core into their own module. This is partly due to > the module_driver() code only being allowed once in a module and also > to avoid a number of #ifdef if we build a single file in a system > without pci support. > > Signed-off-by: Ben Dooks > --- > v7: > - re-order kconfig to make dwc core be selected by PCI driver > v6: > - put DWC_PERIOD_NS back to avoid bisect issues > v4: > - removed DWC_PERIOD_NS as not needed > --- > drivers/pwm/Kconfig | 17 +++- > drivers/pwm/Makefile | 1 + > drivers/pwm/pwm-dwc-pci.c | 133 ++++++++++++++++++++++++++++++++ > drivers/pwm/pwm-dwc.c | 158 +------------------------------------- > drivers/pwm/pwm-dwc.h | 58 ++++++++++++++ > 5 files changed, 209 insertions(+), 158 deletions(-) > create mode 100644 drivers/pwm/pwm-dwc-pci.c > create mode 100644 drivers/pwm/pwm-dwc.h > > diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig > index 3f3c53af4a56..8c5ef388a981 100644 > --- a/drivers/pwm/Kconfig > +++ b/drivers/pwm/Kconfig > @@ -174,16 +174,25 @@ config PWM_CROS_EC > PWM driver for exposing a PWM attached to the ChromeOS Embedded > Controller. > > -config PWM_DWC > - tristate "DesignWare PWM Controller" > - depends on PCI || COMPILE_TEST You're loosing COMPILE_TEST here, as it's not present for the new PWM_DWC. > +config PWM_DWC_CORE > + tristate > depends on HAS_IOMEM > help > - PWM driver for Synopsys DWC PWM Controller attached to a PCI bus. > + PWM driver for Synopsys DWC PWM Controller. > > To compile this driver as a module, choose M here: the module > will be called pwm-dwc. > > +config PWM_DWC > + tristate "DesignWare PWM Controller (PCI bus)" > + depends on HAS_IOMEM && PCI > + select PWM_DWC_CORE > + help > + PWM driver for Synopsys DWC PWM Controller attached to a PCI bus. > + > + To compile this driver as a module, choose M here: the module > + will be called pwm-dwc-pci. > + > config PWM_EP93XX > tristate "Cirrus Logic EP93xx PWM support" > depends on ARCH_EP93XX || COMPILE_TEST > diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile > index 7bf1a29f02b8..a70d36623129 100644 > --- a/drivers/pwm/Makefile > +++ b/drivers/pwm/Makefile > @@ -15,6 +15,7 @@ obj-$(CONFIG_PWM_CLPS711X) += pwm-clps711x.o > obj-$(CONFIG_PWM_CRC) += pwm-crc.o > obj-$(CONFIG_PWM_CROS_EC) += pwm-cros-ec.o > obj-$(CONFIG_PWM_DWC) += pwm-dwc.o > +obj-$(CONFIG_PWM_DWC_PCI) += pwm-dwc-pci.o > obj-$(CONFIG_PWM_EP93XX) += pwm-ep93xx.o > obj-$(CONFIG_PWM_FSL_FTM) += pwm-fsl-ftm.o > obj-$(CONFIG_PWM_HIBVT) += pwm-hibvt.o > diff --git a/drivers/pwm/pwm-dwc-pci.c b/drivers/pwm/pwm-dwc-pci.c > new file mode 100644 > index 000000000000..2213d0e7f3c8 > --- /dev/null > +++ b/drivers/pwm/pwm-dwc-pci.c > @@ -0,0 +1,133 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * DesignWare PWM Controller driver (PCI part) > + * > + * Copyright (C) 2018-2020 Intel Corporation > + * > + * Author: Felipe Balbi (Intel) > + * Author: Jarkko Nikula > + * Author: Raymond Tan > + * > + * Limitations: > + * - The hardware cannot generate a 0 % or 100 % duty cycle. Both high and low > + * periods are one or more input clock periods long. > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#include "pwm-dwc.h" > + > +static int dwc_pwm_probe(struct pci_dev *pci, const struct pci_device_id *id) > +{ > + struct device *dev = &pci->dev; > + struct dwc_pwm *dwc; > + int ret; > + > + dwc = dwc_pwm_alloc(dev); > + if (!dwc) > + return -ENOMEM; > + > + ret = pcim_enable_device(pci); > + if (ret) { > + dev_err(dev, "Failed to enable device (%pe)\n", ERR_PTR(ret)); > + return ret; > + } > + > + pci_set_master(pci); > + > + ret = pcim_iomap_regions(pci, BIT(0), pci_name(pci)); > + if (ret) { > + dev_err(dev, "Failed to iomap PCI BAR (%pe)\n", ERR_PTR(ret)); > + return ret; > + } > + > + dwc->base = pcim_iomap_table(pci)[0]; > + if (!dwc->base) { > + dev_err(dev, "Base address missing\n"); > + return -ENOMEM; > + } > + > + ret = devm_pwmchip_add(dev, &dwc->chip); > + if (ret) > + return ret; > + > + pm_runtime_put(dev); > + pm_runtime_allow(dev); > + > + return 0; > +} > + > +static void dwc_pwm_remove(struct pci_dev *pci) > +{ > + pm_runtime_forbid(&pci->dev); > + pm_runtime_get_noresume(&pci->dev); > +} > + > +#ifdef CONFIG_PM_SLEEP > +static int dwc_pwm_suspend(struct device *dev) > +{ > + struct pci_dev *pdev = container_of(dev, struct pci_dev, dev); > + struct dwc_pwm *dwc = pci_get_drvdata(pdev); > + int i; > + > + for (i = 0; i < DWC_TIMERS_TOTAL; i++) { > + if (dwc->chip.pwms[i].state.enabled) { > + dev_err(dev, "PWM %u in use by consumer (%s)\n", > + i, dwc->chip.pwms[i].label); > + return -EBUSY; > + } > + dwc->ctx[i].cnt = dwc_pwm_readl(dwc, DWC_TIM_LD_CNT(i)); > + dwc->ctx[i].cnt2 = dwc_pwm_readl(dwc, DWC_TIM_LD_CNT2(i)); > + dwc->ctx[i].ctrl = dwc_pwm_readl(dwc, DWC_TIM_CTRL(i)); > + } > + > + return 0; > +} > + > +static int dwc_pwm_resume(struct device *dev) > +{ > + struct pci_dev *pdev = container_of(dev, struct pci_dev, dev); > + struct dwc_pwm *dwc = pci_get_drvdata(pdev); > + int i; > + > + for (i = 0; i < DWC_TIMERS_TOTAL; i++) { > + dwc_pwm_writel(dwc, dwc->ctx[i].cnt, DWC_TIM_LD_CNT(i)); > + dwc_pwm_writel(dwc, dwc->ctx[i].cnt2, DWC_TIM_LD_CNT2(i)); > + dwc_pwm_writel(dwc, dwc->ctx[i].ctrl, DWC_TIM_CTRL(i)); > + } > + > + return 0; > +} > +#endif > + > +static SIMPLE_DEV_PM_OPS(dwc_pwm_pm_ops, dwc_pwm_suspend, dwc_pwm_resume); > + > +static const struct pci_device_id dwc_pwm_id_table[] = { > + { PCI_VDEVICE(INTEL, 0x4bb7) }, /* Elkhart Lake */ > + { } /* Terminating Entry */ > +}; > +MODULE_DEVICE_TABLE(pci, dwc_pwm_id_table); > + > +static struct pci_driver dwc_pwm_driver = { > + .name = "pwm-dwc", > + .probe = dwc_pwm_probe, > + .remove = dwc_pwm_remove, > + .id_table = dwc_pwm_id_table, > + .driver = { > + .pm = &dwc_pwm_pm_ops, > + }, > +}; > + > +module_pci_driver(dwc_pwm_driver); > + > +MODULE_AUTHOR("Felipe Balbi (Intel)"); > +MODULE_AUTHOR("Jarkko Nikula "); > +MODULE_AUTHOR("Raymond Tan "); > +MODULE_DESCRIPTION("DesignWare PWM Controller"); > +MODULE_LICENSE("GPL"); > diff --git a/drivers/pwm/pwm-dwc.c b/drivers/pwm/pwm-dwc.c > index 56cde9da2c0e..90a8ae1252a1 100644 > --- a/drivers/pwm/pwm-dwc.c > +++ b/drivers/pwm/pwm-dwc.c > @@ -1,16 +1,12 @@ > // SPDX-License-Identifier: GPL-2.0 > /* > - * DesignWare PWM Controller driver > + * DesignWare PWM Controller driver core > * > * Copyright (C) 2018-2020 Intel Corporation > * > * Author: Felipe Balbi (Intel) > * Author: Jarkko Nikula > * Author: Raymond Tan > - * > - * Limitations: > - * - The hardware cannot generate a 0 % or 100 % duty cycle. Both high and low > - * periods are one or more input clock periods long. > */ > > #include > @@ -21,51 +17,7 @@ > #include > #include > > -#define DWC_TIM_LD_CNT(n) ((n) * 0x14) > -#define DWC_TIM_LD_CNT2(n) (((n) * 4) + 0xb0) > -#define DWC_TIM_CUR_VAL(n) (((n) * 0x14) + 0x04) > -#define DWC_TIM_CTRL(n) (((n) * 0x14) + 0x08) > -#define DWC_TIM_EOI(n) (((n) * 0x14) + 0x0c) > -#define DWC_TIM_INT_STS(n) (((n) * 0x14) + 0x10) > - > -#define DWC_TIMERS_INT_STS 0xa0 > -#define DWC_TIMERS_EOI 0xa4 > -#define DWC_TIMERS_RAW_INT_STS 0xa8 > -#define DWC_TIMERS_COMP_VERSION 0xac > - > -#define DWC_TIMERS_TOTAL 8 > -#define DWC_CLK_PERIOD_NS 10 > - > -/* Timer Control Register */ > -#define DWC_TIM_CTRL_EN BIT(0) > -#define DWC_TIM_CTRL_MODE BIT(1) > -#define DWC_TIM_CTRL_MODE_FREE (0 << 1) > -#define DWC_TIM_CTRL_MODE_USER (1 << 1) > -#define DWC_TIM_CTRL_INT_MASK BIT(2) > -#define DWC_TIM_CTRL_PWM BIT(3) > - > -struct dwc_pwm_ctx { > - u32 cnt; > - u32 cnt2; > - u32 ctrl; > -}; > - > -struct dwc_pwm { > - struct pwm_chip chip; > - void __iomem *base; > - struct dwc_pwm_ctx ctx[DWC_TIMERS_TOTAL]; > -}; > -#define to_dwc_pwm(p) (container_of((p), struct dwc_pwm, chip)) > - > -static inline u32 dwc_pwm_readl(struct dwc_pwm *dwc, u32 offset) > -{ > - return readl(dwc->base + offset); > -} > - > -static inline void dwc_pwm_writel(struct dwc_pwm *dwc, u32 value, u32 offset) > -{ > - writel(value, dwc->base + offset); > -} > +#include "pwm-dwc.h" > > static void __dwc_pwm_set_enable(struct dwc_pwm *dwc, int pwm, int enabled) > { > @@ -196,7 +148,7 @@ static const struct pwm_ops dwc_pwm_ops = { > .owner = THIS_MODULE, > }; > > -static struct dwc_pwm *dwc_pwm_alloc(struct device *dev) > +struct dwc_pwm *dwc_pwm_alloc(struct device *dev) > { > struct dwc_pwm *dwc; > > @@ -211,109 +163,7 @@ static struct dwc_pwm *dwc_pwm_alloc(struct device *dev) > dev_set_drvdata(dev, dwc); > return dwc; > } > - > -static int dwc_pwm_probe(struct pci_dev *pci, const struct pci_device_id *id) > -{ > - struct device *dev = &pci->dev; > - struct dwc_pwm *dwc; > - int ret; > - > - dwc = dwc_pwm_alloc(dev); > - if (!dwc) > - return -ENOMEM; > - > - ret = pcim_enable_device(pci); > - if (ret) { > - dev_err(dev, "Failed to enable device (%pe)\n", ERR_PTR(ret)); > - return ret; > - } > - > - pci_set_master(pci); > - > - ret = pcim_iomap_regions(pci, BIT(0), pci_name(pci)); > - if (ret) { > - dev_err(dev, "Failed to iomap PCI BAR (%pe)\n", ERR_PTR(ret)); > - return ret; > - } > - > - dwc->base = pcim_iomap_table(pci)[0]; > - if (!dwc->base) { > - dev_err(dev, "Base address missing\n"); > - return -ENOMEM; > - } > - > - ret = devm_pwmchip_add(dev, &dwc->chip); > - if (ret) > - return ret; > - > - pm_runtime_put(dev); > - pm_runtime_allow(dev); > - > - return 0; > -} > - > -static void dwc_pwm_remove(struct pci_dev *pci) > -{ > - pm_runtime_forbid(&pci->dev); > - pm_runtime_get_noresume(&pci->dev); > -} > - > -#ifdef CONFIG_PM_SLEEP > -static int dwc_pwm_suspend(struct device *dev) > -{ > - struct pci_dev *pdev = container_of(dev, struct pci_dev, dev); > - struct dwc_pwm *dwc = pci_get_drvdata(pdev); > - int i; > - > - for (i = 0; i < DWC_TIMERS_TOTAL; i++) { > - if (dwc->chip.pwms[i].state.enabled) { > - dev_err(dev, "PWM %u in use by consumer (%s)\n", > - i, dwc->chip.pwms[i].label); > - return -EBUSY; > - } > - dwc->ctx[i].cnt = dwc_pwm_readl(dwc, DWC_TIM_LD_CNT(i)); > - dwc->ctx[i].cnt2 = dwc_pwm_readl(dwc, DWC_TIM_LD_CNT2(i)); > - dwc->ctx[i].ctrl = dwc_pwm_readl(dwc, DWC_TIM_CTRL(i)); > - } > - > - return 0; > -} > - > -static int dwc_pwm_resume(struct device *dev) > -{ > - struct pci_dev *pdev = container_of(dev, struct pci_dev, dev); > - struct dwc_pwm *dwc = pci_get_drvdata(pdev); > - int i; > - > - for (i = 0; i < DWC_TIMERS_TOTAL; i++) { > - dwc_pwm_writel(dwc, dwc->ctx[i].cnt, DWC_TIM_LD_CNT(i)); > - dwc_pwm_writel(dwc, dwc->ctx[i].cnt2, DWC_TIM_LD_CNT2(i)); > - dwc_pwm_writel(dwc, dwc->ctx[i].ctrl, DWC_TIM_CTRL(i)); > - } > - > - return 0; > -} > -#endif > - > -static SIMPLE_DEV_PM_OPS(dwc_pwm_pm_ops, dwc_pwm_suspend, dwc_pwm_resume); > - > -static const struct pci_device_id dwc_pwm_id_table[] = { > - { PCI_VDEVICE(INTEL, 0x4bb7) }, /* Elkhart Lake */ > - { } /* Terminating Entry */ > -}; > -MODULE_DEVICE_TABLE(pci, dwc_pwm_id_table); > - > -static struct pci_driver dwc_pwm_driver = { > - .name = "pwm-dwc", > - .probe = dwc_pwm_probe, > - .remove = dwc_pwm_remove, > - .id_table = dwc_pwm_id_table, > - .driver = { > - .pm = &dwc_pwm_pm_ops, > - }, > -}; > - > -module_pci_driver(dwc_pwm_driver); > +EXPORT_SYMBOL_GPL(dwc_pwm_alloc); > > MODULE_AUTHOR("Felipe Balbi (Intel)"); > MODULE_AUTHOR("Jarkko Nikula "); > diff --git a/drivers/pwm/pwm-dwc.h b/drivers/pwm/pwm-dwc.h > new file mode 100644 > index 000000000000..68f98eb76152 > --- /dev/null > +++ b/drivers/pwm/pwm-dwc.h > @@ -0,0 +1,58 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * DesignWare PWM Controller driver > + * > + * Copyright (C) 2018-2020 Intel Corporation > + * > + * Author: Felipe Balbi (Intel) > + * Author: Jarkko Nikula > + * Author: Raymond Tan > + */ > + > +#define DWC_TIM_LD_CNT(n) ((n) * 0x14) > +#define DWC_TIM_LD_CNT2(n) (((n) * 4) + 0xb0) > +#define DWC_TIM_CUR_VAL(n) (((n) * 0x14) + 0x04) > +#define DWC_TIM_CTRL(n) (((n) * 0x14) + 0x08) > +#define DWC_TIM_EOI(n) (((n) * 0x14) + 0x0c) > +#define DWC_TIM_INT_STS(n) (((n) * 0x14) + 0x10) > + > +#define DWC_TIMERS_INT_STS 0xa0 > +#define DWC_TIMERS_EOI 0xa4 > +#define DWC_TIMERS_RAW_INT_STS 0xa8 > +#define DWC_TIMERS_COMP_VERSION 0xac > + > +#define DWC_TIMERS_TOTAL 8 > +#define DWC_CLK_PERIOD_NS 10 > + > +/* Timer Control Register */ > +#define DWC_TIM_CTRL_EN BIT(0) > +#define DWC_TIM_CTRL_MODE BIT(1) > +#define DWC_TIM_CTRL_MODE_FREE (0 << 1) > +#define DWC_TIM_CTRL_MODE_USER (1 << 1) > +#define DWC_TIM_CTRL_INT_MASK BIT(2) > +#define DWC_TIM_CTRL_PWM BIT(3) > + > +struct dwc_pwm_ctx { > + u32 cnt; > + u32 cnt2; > + u32 ctrl; > +}; > + > +struct dwc_pwm { > + struct pwm_chip chip; > + void __iomem *base; > + struct dwc_pwm_ctx ctx[DWC_TIMERS_TOTAL]; > +}; > +#define to_dwc_pwm(p) (container_of((p), struct dwc_pwm, chip)) > + > +static inline u32 dwc_pwm_readl(struct dwc_pwm *dwc, u32 offset) > +{ > + return readl(dwc->base + offset); > +} > + > +static inline void dwc_pwm_writel(struct dwc_pwm *dwc, u32 value, u32 offset) > +{ > + writel(value, dwc->base + offset); > +} > + > +extern struct dwc_pwm *dwc_pwm_alloc(struct device *dev); If you respin this patch for the COMPILE_TEST issue I pointed out above, would you mind using a module namespace? That would work by adding e.g. #define DEFAULT_SYMBOL_NAMESPACE dwc-pwm to drivers/pwm/pwm-dwc.c (before the includes) and MODULE_IMPORT_NS(dwc-pwm) to drivers/pwm/pwm-dwc.h. Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-König | Industrial Linux Solutions | https://www.pengutronix.de/ |