Messages in this thread | | | Date | Fri, 19 Aug 2022 17:56:10 +0100 | Subject | Re: [RFC v4 06/10] pwm: dwc: split pci out of core driver | From | Ben Dooks <> |
| |
On 19/08/2022 14:38, Jarkko Nikula wrote: > Hi > > On 8/17/22 00:14, 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 <ben.dooks@sifive.com> >> --- > > I quickly tested this on Intel Elkhart and didn't notice any regression. > A few comments below. > >> drivers/pwm/Kconfig | 14 +++- >> 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, 207 insertions(+), 157 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..a9f1c554db2b 100644 >> --- a/drivers/pwm/Kconfig >> +++ b/drivers/pwm/Kconfig >> @@ -175,15 +175,23 @@ config PWM_CROS_EC >> Controller. >> config PWM_DWC >> - tristate "DesignWare PWM Controller" >> - depends on PCI || COMPILE_TEST >> + tristate "DesignWare PWM Controller core" >> 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_PCI >> + tristate "DesignWare PWM Controller core" > > Same text as core part has. How about "DesignWare PWM Controller PCI > driver"?
Thanks, did notice a couple of kconfig issues so will look at that.
> >> + depends on PWM_DWC && HAS_IOMEM && PCI >> + 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 <jarkko.nikula@linux.intel.com> >> + * Author: Raymond Tan <raymond.tan@intel.com> >> + * >> + * Limitations: >> + * - The hardware cannot generate a 0 % or 100 % duty cycle. Both >> high and low >> + * periods are one or more input clock periods long. >> + */ >> + > > I think this is more common limitation rather than PCI part.
The PCI is based off an core without the support, it is added as a build option as of (IIRC) the 2.13 core.
> >> --- 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 <jarkko.nikula@linux.intel.com> >> * Author: Raymond Tan <raymond.tan@intel.com> >> - * >> - * Limitations: >> - * - The hardware cannot generate a 0 % or 100 % duty cycle. Both >> high and low >> - * periods are one or more input clock periods long. >> */ > > Relates to previous comment, is there reason why this limitation is > removed from the core part?
See above.
> >> --- /dev/null >> +++ b/drivers/pwm/pwm-dwc.h >> +#define DWC_CLK_PERIOD_NS 10 > > Perhaps this addition can be removed if patch 7/10 goes before this > patch? It's anyway specific to PCI part only.
Will look into that
| |