* [PATCH v8 1/5] pwm: dwc: split pci out of core driver
2023-06-14 17:14 [PATCH v8 0/5] DesignWare PWM driver updates Ben Dooks
@ 2023-06-14 17:14 ` Ben Dooks
2023-07-15 19:28 ` Uwe Kleine-König
2023-06-14 17:14 ` [PATCH v8 2/5] pwm: dwc: make timer clock configurable Ben Dooks
` (4 subsequent siblings)
5 siblings, 1 reply; 14+ messages in thread
From: Ben Dooks @ 2023-06-14 17:14 UTC (permalink / raw)
To: linux-pwm
Cc: devicetree, linux-kernel, ben.dooks, u.kleine-koenig,
Thierry Reding, Krzysztof Kozlowski, Greentime Hu, jarkko.nikula,
William Salmon, Jude Onyenegecha, Ben Dooks
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>
---
v8:
- add module namespace
- remove compile-test for pci case, doesn't make sense
- fix makefile, missed config symbol changes
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 | 14 ++-
drivers/pwm/Makefile | 1 +
drivers/pwm/pwm-dwc-core.c | 176 +++++++++++++++++++++++++++++++++
drivers/pwm/pwm-dwc.c | 197 +------------------------------------
drivers/pwm/pwm-dwc.h | 60 +++++++++++
5 files changed, 253 insertions(+), 195 deletions(-)
create mode 100644 drivers/pwm/pwm-dwc-core.c
create mode 100644 drivers/pwm/pwm-dwc.h
diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
index 8df861b1f4a3..7c54cdcb97a0 100644
--- a/drivers/pwm/Kconfig
+++ b/drivers/pwm/Kconfig
@@ -186,9 +186,19 @@ config PWM_CROS_EC
PWM driver for exposing a PWM attached to the ChromeOS Embedded
Controller.
+config PWM_DWC_CORE
+ tristate
+ depends on HAS_IOMEM
+ help
+ PWM driver for Synopsys DWC PWM Controller.
+
+ To compile this driver as a module, build the dependecies as
+ modules, this will be called pwm-dwc-core.
+
config PWM_DWC
- tristate "DesignWare PWM Controller"
- depends on PCI
+ 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.
diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile
index 19899b912e00..de3ed77e8d7c 100644
--- a/drivers/pwm/Makefile
+++ b/drivers/pwm/Makefile
@@ -15,6 +15,7 @@ obj-$(CONFIG_PWM_CLK) += pwm-clk.o
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_CORE) += pwm-dwc-core.o
obj-$(CONFIG_PWM_DWC) += pwm-dwc.o
obj-$(CONFIG_PWM_EP93XX) += pwm-ep93xx.o
obj-$(CONFIG_PWM_FSL_FTM) += pwm-fsl-ftm.o
diff --git a/drivers/pwm/pwm-dwc-core.c b/drivers/pwm/pwm-dwc-core.c
new file mode 100644
index 000000000000..b693cb7fa812
--- /dev/null
+++ b/drivers/pwm/pwm-dwc-core.c
@@ -0,0 +1,176 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * 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>
+ */
+
+#define DEFAULT_SYMBOL_NAMESPACE dwc_pwm
+
+#include <linux/bitops.h>
+#include <linux/export.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/pci.h>
+#include <linux/pm_runtime.h>
+#include <linux/pwm.h>
+
+#include "pwm-dwc.h"
+
+static void __dwc_pwm_set_enable(struct dwc_pwm *dwc, int pwm, int enabled)
+{
+ u32 reg;
+
+ reg = dwc_pwm_readl(dwc, DWC_TIM_CTRL(pwm));
+
+ if (enabled)
+ reg |= DWC_TIM_CTRL_EN;
+ else
+ reg &= ~DWC_TIM_CTRL_EN;
+
+ dwc_pwm_writel(dwc, reg, DWC_TIM_CTRL(pwm));
+}
+
+static int __dwc_pwm_configure_timer(struct dwc_pwm *dwc,
+ struct pwm_device *pwm,
+ const struct pwm_state *state)
+{
+ u64 tmp;
+ u32 ctrl;
+ u32 high;
+ u32 low;
+
+ /*
+ * Calculate width of low and high period in terms of input clock
+ * periods and check are the result within HW limits between 1 and
+ * 2^32 periods.
+ */
+ tmp = DIV_ROUND_CLOSEST_ULL(state->duty_cycle, DWC_CLK_PERIOD_NS);
+ if (tmp < 1 || tmp > (1ULL << 32))
+ return -ERANGE;
+ low = tmp - 1;
+
+ tmp = DIV_ROUND_CLOSEST_ULL(state->period - state->duty_cycle,
+ DWC_CLK_PERIOD_NS);
+ if (tmp < 1 || tmp > (1ULL << 32))
+ return -ERANGE;
+ high = tmp - 1;
+
+ /*
+ * Specification says timer usage flow is to disable timer, then
+ * program it followed by enable. It also says Load Count is loaded
+ * into timer after it is enabled - either after a disable or
+ * a reset. Based on measurements it happens also without disable
+ * whenever Load Count is updated. But follow the specification.
+ */
+ __dwc_pwm_set_enable(dwc, pwm->hwpwm, false);
+
+ /*
+ * Write Load Count and Load Count 2 registers. Former defines the
+ * width of low period and latter the width of high period in terms
+ * multiple of input clock periods:
+ * Width = ((Count + 1) * input clock period).
+ */
+ dwc_pwm_writel(dwc, low, DWC_TIM_LD_CNT(pwm->hwpwm));
+ dwc_pwm_writel(dwc, high, DWC_TIM_LD_CNT2(pwm->hwpwm));
+
+ /*
+ * Set user-defined mode, timer reloads from Load Count registers
+ * when it counts down to 0.
+ * Set PWM mode, it makes output to toggle and width of low and high
+ * periods are set by Load Count registers.
+ */
+ ctrl = DWC_TIM_CTRL_MODE_USER | DWC_TIM_CTRL_PWM;
+ dwc_pwm_writel(dwc, ctrl, DWC_TIM_CTRL(pwm->hwpwm));
+
+ /*
+ * Enable timer. Output starts from low period.
+ */
+ __dwc_pwm_set_enable(dwc, pwm->hwpwm, state->enabled);
+
+ return 0;
+}
+
+static int dwc_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
+ const struct pwm_state *state)
+{
+ struct dwc_pwm *dwc = to_dwc_pwm(chip);
+
+ if (state->polarity != PWM_POLARITY_INVERSED)
+ return -EINVAL;
+
+ if (state->enabled) {
+ if (!pwm->state.enabled)
+ pm_runtime_get_sync(chip->dev);
+ return __dwc_pwm_configure_timer(dwc, pwm, state);
+ } else {
+ if (pwm->state.enabled) {
+ __dwc_pwm_set_enable(dwc, pwm->hwpwm, false);
+ pm_runtime_put_sync(chip->dev);
+ }
+ }
+
+ return 0;
+}
+
+static int dwc_pwm_get_state(struct pwm_chip *chip, struct pwm_device *pwm,
+ struct pwm_state *state)
+{
+ struct dwc_pwm *dwc = to_dwc_pwm(chip);
+ u64 duty, period;
+
+ pm_runtime_get_sync(chip->dev);
+
+ state->enabled = !!(dwc_pwm_readl(dwc,
+ DWC_TIM_CTRL(pwm->hwpwm)) & DWC_TIM_CTRL_EN);
+
+ duty = dwc_pwm_readl(dwc, DWC_TIM_LD_CNT(pwm->hwpwm));
+ duty += 1;
+ duty *= DWC_CLK_PERIOD_NS;
+ state->duty_cycle = duty;
+
+ period = dwc_pwm_readl(dwc, DWC_TIM_LD_CNT2(pwm->hwpwm));
+ period += 1;
+ period *= DWC_CLK_PERIOD_NS;
+ period += duty;
+ state->period = period;
+
+ state->polarity = PWM_POLARITY_INVERSED;
+
+ pm_runtime_put_sync(chip->dev);
+
+ return 0;
+}
+
+static const struct pwm_ops dwc_pwm_ops = {
+ .apply = dwc_pwm_apply,
+ .get_state = dwc_pwm_get_state,
+ .owner = THIS_MODULE,
+};
+
+struct dwc_pwm *dwc_pwm_alloc(struct device *dev)
+{
+ struct dwc_pwm *dwc;
+
+ dwc = devm_kzalloc(dev, sizeof(*dwc), GFP_KERNEL);
+ if (!dwc)
+ return NULL;
+
+ dwc->chip.dev = dev;
+ dwc->chip.ops = &dwc_pwm_ops;
+ dwc->chip.npwm = DWC_TIMERS_TOTAL;
+
+ dev_set_drvdata(dev, dwc);
+ return dwc;
+}
+EXPORT_SYMBOL_GPL(dwc_pwm_alloc);
+
+MODULE_AUTHOR("Felipe Balbi (Intel)");
+MODULE_AUTHOR("Jarkko Nikula <jarkko.nikula@linux.intel.com>");
+MODULE_AUTHOR("Raymond Tan <raymond.tan@intel.com>");
+MODULE_DESCRIPTION("DesignWare PWM Controller");
+MODULE_LICENSE("GPL");
diff --git a/drivers/pwm/pwm-dwc.c b/drivers/pwm/pwm-dwc.c
index 3bbb26c862c3..bd9cadb497d7 100644
--- a/drivers/pwm/pwm-dwc.c
+++ b/drivers/pwm/pwm-dwc.c
@@ -1,6 +1,6 @@
// SPDX-License-Identifier: GPL-2.0
/*
- * DesignWare PWM Controller driver
+ * DesignWare PWM Controller driver (PCI part)
*
* Copyright (C) 2018-2020 Intel Corporation
*
@@ -13,6 +13,8 @@
* periods are one or more input clock periods long.
*/
+#define DEFAULT_MOUDLE_NAMESPACE dwc_pwm
+
#include <linux/bitops.h>
#include <linux/export.h>
#include <linux/kernel.h>
@@ -21,198 +23,7 @@
#include <linux/pm_runtime.h>
#include <linux/pwm.h>
-#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);
-}
-
-static void __dwc_pwm_set_enable(struct dwc_pwm *dwc, int pwm, int enabled)
-{
- u32 reg;
-
- reg = dwc_pwm_readl(dwc, DWC_TIM_CTRL(pwm));
-
- if (enabled)
- reg |= DWC_TIM_CTRL_EN;
- else
- reg &= ~DWC_TIM_CTRL_EN;
-
- dwc_pwm_writel(dwc, reg, DWC_TIM_CTRL(pwm));
-}
-
-static int __dwc_pwm_configure_timer(struct dwc_pwm *dwc,
- struct pwm_device *pwm,
- const struct pwm_state *state)
-{
- u64 tmp;
- u32 ctrl;
- u32 high;
- u32 low;
-
- /*
- * Calculate width of low and high period in terms of input clock
- * periods and check are the result within HW limits between 1 and
- * 2^32 periods.
- */
- tmp = DIV_ROUND_CLOSEST_ULL(state->duty_cycle, DWC_CLK_PERIOD_NS);
- if (tmp < 1 || tmp > (1ULL << 32))
- return -ERANGE;
- low = tmp - 1;
-
- tmp = DIV_ROUND_CLOSEST_ULL(state->period - state->duty_cycle,
- DWC_CLK_PERIOD_NS);
- if (tmp < 1 || tmp > (1ULL << 32))
- return -ERANGE;
- high = tmp - 1;
-
- /*
- * Specification says timer usage flow is to disable timer, then
- * program it followed by enable. It also says Load Count is loaded
- * into timer after it is enabled - either after a disable or
- * a reset. Based on measurements it happens also without disable
- * whenever Load Count is updated. But follow the specification.
- */
- __dwc_pwm_set_enable(dwc, pwm->hwpwm, false);
-
- /*
- * Write Load Count and Load Count 2 registers. Former defines the
- * width of low period and latter the width of high period in terms
- * multiple of input clock periods:
- * Width = ((Count + 1) * input clock period).
- */
- dwc_pwm_writel(dwc, low, DWC_TIM_LD_CNT(pwm->hwpwm));
- dwc_pwm_writel(dwc, high, DWC_TIM_LD_CNT2(pwm->hwpwm));
-
- /*
- * Set user-defined mode, timer reloads from Load Count registers
- * when it counts down to 0.
- * Set PWM mode, it makes output to toggle and width of low and high
- * periods are set by Load Count registers.
- */
- ctrl = DWC_TIM_CTRL_MODE_USER | DWC_TIM_CTRL_PWM;
- dwc_pwm_writel(dwc, ctrl, DWC_TIM_CTRL(pwm->hwpwm));
-
- /*
- * Enable timer. Output starts from low period.
- */
- __dwc_pwm_set_enable(dwc, pwm->hwpwm, state->enabled);
-
- return 0;
-}
-
-static int dwc_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
- const struct pwm_state *state)
-{
- struct dwc_pwm *dwc = to_dwc_pwm(chip);
-
- if (state->polarity != PWM_POLARITY_INVERSED)
- return -EINVAL;
-
- if (state->enabled) {
- if (!pwm->state.enabled)
- pm_runtime_get_sync(chip->dev);
- return __dwc_pwm_configure_timer(dwc, pwm, state);
- } else {
- if (pwm->state.enabled) {
- __dwc_pwm_set_enable(dwc, pwm->hwpwm, false);
- pm_runtime_put_sync(chip->dev);
- }
- }
-
- return 0;
-}
-
-static int dwc_pwm_get_state(struct pwm_chip *chip, struct pwm_device *pwm,
- struct pwm_state *state)
-{
- struct dwc_pwm *dwc = to_dwc_pwm(chip);
- u64 duty, period;
-
- pm_runtime_get_sync(chip->dev);
-
- state->enabled = !!(dwc_pwm_readl(dwc,
- DWC_TIM_CTRL(pwm->hwpwm)) & DWC_TIM_CTRL_EN);
-
- duty = dwc_pwm_readl(dwc, DWC_TIM_LD_CNT(pwm->hwpwm));
- duty += 1;
- duty *= DWC_CLK_PERIOD_NS;
- state->duty_cycle = duty;
-
- period = dwc_pwm_readl(dwc, DWC_TIM_LD_CNT2(pwm->hwpwm));
- period += 1;
- period *= DWC_CLK_PERIOD_NS;
- period += duty;
- state->period = period;
-
- state->polarity = PWM_POLARITY_INVERSED;
-
- pm_runtime_put_sync(chip->dev);
-
- return 0;
-}
-
-static const struct pwm_ops dwc_pwm_ops = {
- .apply = dwc_pwm_apply,
- .get_state = dwc_pwm_get_state,
- .owner = THIS_MODULE,
-};
-
-static struct dwc_pwm *dwc_pwm_alloc(struct device *dev)
-{
- struct dwc_pwm *dwc;
-
- dwc = devm_kzalloc(dev, sizeof(*dwc), GFP_KERNEL);
- if (!dwc)
- return NULL;
-
- dwc->chip.dev = dev;
- dwc->chip.ops = &dwc_pwm_ops;
- dwc->chip.npwm = DWC_TIMERS_TOTAL;
-
- dev_set_drvdata(dev, dwc);
- return dwc;
-}
+#include "pwm-dwc.h"
static int dwc_pwm_probe(struct pci_dev *pci, const struct pci_device_id *id)
{
diff --git a/drivers/pwm/pwm-dwc.h b/drivers/pwm/pwm-dwc.h
new file mode 100644
index 000000000000..56deab4e28ec
--- /dev/null
+++ b/drivers/pwm/pwm-dwc.h
@@ -0,0 +1,60 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * DesignWare PWM Controller driver
+ *
+ * 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>
+ */
+
+MODULE_IMPORT_NS(dwc_pwm);
+
+#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);
--
2.39.2
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH v8 1/5] pwm: dwc: split pci out of core driver
2023-06-14 17:14 ` [PATCH v8 1/5] pwm: dwc: split pci out of core driver Ben Dooks
@ 2023-07-15 19:28 ` Uwe Kleine-König
2023-07-17 7:07 ` Ben Dooks
0 siblings, 1 reply; 14+ messages in thread
From: Uwe Kleine-König @ 2023-07-15 19:28 UTC (permalink / raw)
To: Ben Dooks
Cc: linux-pwm, devicetree, linux-kernel, ben.dooks, Thierry Reding,
Krzysztof Kozlowski, Greentime Hu, jarkko.nikula, William Salmon,
Jude Onyenegecha
[-- Attachment #1: Type: text/plain, Size: 2909 bytes --]
On Wed, Jun 14, 2023 at 06:14:53PM +0100, 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>
> ---
> v8:
> - add module namespace
> - remove compile-test for pci case, doesn't make sense
> - fix makefile, missed config symbol changes
> 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 | 14 ++-
> drivers/pwm/Makefile | 1 +
> drivers/pwm/pwm-dwc-core.c | 176 +++++++++++++++++++++++++++++++++
> drivers/pwm/pwm-dwc.c | 197 +------------------------------------
> drivers/pwm/pwm-dwc.h | 60 +++++++++++
> 5 files changed, 253 insertions(+), 195 deletions(-)
> create mode 100644 drivers/pwm/pwm-dwc-core.c
> create mode 100644 drivers/pwm/pwm-dwc.h
>
> diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
> index 8df861b1f4a3..7c54cdcb97a0 100644
> --- a/drivers/pwm/Kconfig
> +++ b/drivers/pwm/Kconfig
> @@ -186,9 +186,19 @@ config PWM_CROS_EC
> PWM driver for exposing a PWM attached to the ChromeOS Embedded
> Controller.
>
> +config PWM_DWC_CORE
> + tristate
> + depends on HAS_IOMEM
> + help
> + PWM driver for Synopsys DWC PWM Controller.
> +
> + To compile this driver as a module, build the dependecies as
> + modules, this will be called pwm-dwc-core.
> +
> config PWM_DWC
> - tristate "DesignWare PWM Controller"
> - depends on PCI
> + 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.
>
> diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile
> index 19899b912e00..de3ed77e8d7c 100644
> --- a/drivers/pwm/Makefile
> +++ b/drivers/pwm/Makefile
> @@ -15,6 +15,7 @@ obj-$(CONFIG_PWM_CLK) += pwm-clk.o
> 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_CORE) += pwm-dwc-core.o
> obj-$(CONFIG_PWM_DWC) += pwm-dwc.o
Would it make sense to call this pwm-dwc-pci.o? And the symbol
CONFIG_PWM_DWC_PCI? (The latter would break make oldconfig. Hmm, I'm
unsure myself.)
I didn't check all the details, but assuming that this is a split
without further changes it looks ok to me.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | https://www.pengutronix.de/ |
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v8 1/5] pwm: dwc: split pci out of core driver
2023-07-15 19:28 ` Uwe Kleine-König
@ 2023-07-17 7:07 ` Ben Dooks
2023-07-17 7:46 ` Uwe Kleine-König
0 siblings, 1 reply; 14+ messages in thread
From: Ben Dooks @ 2023-07-17 7:07 UTC (permalink / raw)
To: Uwe Kleine-König, Ben Dooks
Cc: linux-pwm, devicetree, linux-kernel, Thierry Reding,
Krzysztof Kozlowski, Greentime Hu, jarkko.nikula, William Salmon,
Jude Onyenegecha
On 15/07/2023 20:28, Uwe Kleine-König wrote:
> On Wed, Jun 14, 2023 at 06:14:53PM +0100, 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>
>> ---
>> v8:
>> - add module namespace
>> - remove compile-test for pci case, doesn't make sense
>> - fix makefile, missed config symbol changes
>> 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 | 14 ++-
>> drivers/pwm/Makefile | 1 +
>> drivers/pwm/pwm-dwc-core.c | 176 +++++++++++++++++++++++++++++++++
>> drivers/pwm/pwm-dwc.c | 197 +------------------------------------
>> drivers/pwm/pwm-dwc.h | 60 +++++++++++
>> 5 files changed, 253 insertions(+), 195 deletions(-)
>> create mode 100644 drivers/pwm/pwm-dwc-core.c
>> create mode 100644 drivers/pwm/pwm-dwc.h
>>
>> diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
>> index 8df861b1f4a3..7c54cdcb97a0 100644
>> --- a/drivers/pwm/Kconfig
>> +++ b/drivers/pwm/Kconfig
>> @@ -186,9 +186,19 @@ config PWM_CROS_EC
>> PWM driver for exposing a PWM attached to the ChromeOS Embedded
>> Controller.
>>
>> +config PWM_DWC_CORE
>> + tristate
>> + depends on HAS_IOMEM
>> + help
>> + PWM driver for Synopsys DWC PWM Controller.
>> +
>> + To compile this driver as a module, build the dependecies as
>> + modules, this will be called pwm-dwc-core.
>> +
>> config PWM_DWC
>> - tristate "DesignWare PWM Controller"
>> - depends on PCI
>> + 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.
>>
>> diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile
>> index 19899b912e00..de3ed77e8d7c 100644
>> --- a/drivers/pwm/Makefile
>> +++ b/drivers/pwm/Makefile
>> @@ -15,6 +15,7 @@ obj-$(CONFIG_PWM_CLK) += pwm-clk.o
>> 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_CORE) += pwm-dwc-core.o
>> obj-$(CONFIG_PWM_DWC) += pwm-dwc.o
>
> Would it make sense to call this pwm-dwc-pci.o? And the symbol
> CONFIG_PWM_DWC_PCI? (The latter would break make oldconfig. Hmm, I'm
> unsure myself.)
i left the pci as the pwm-dwc so that anyone moving up and using
this as a module won't have to change config or their module loading
if they're not autoloading modules.
> I didn't check all the details, but assuming that this is a split
> without further changes it looks ok to me.
>
> Best regards
> Uwe
>
--
Ben Dooks http://www.codethink.co.uk/
Senior Engineer Codethink - Providing Genius
https://www.codethink.co.uk/privacy.html
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v8 1/5] pwm: dwc: split pci out of core driver
2023-07-17 7:07 ` Ben Dooks
@ 2023-07-17 7:46 ` Uwe Kleine-König
0 siblings, 0 replies; 14+ messages in thread
From: Uwe Kleine-König @ 2023-07-17 7:46 UTC (permalink / raw)
To: Ben Dooks
Cc: Ben Dooks, linux-pwm, devicetree, linux-kernel, Thierry Reding,
Krzysztof Kozlowski, Greentime Hu, jarkko.nikula, William Salmon,
Jude Onyenegecha
[-- Attachment #1: Type: text/plain, Size: 3517 bytes --]
On Mon, Jul 17, 2023 at 08:07:10AM +0100, Ben Dooks wrote:
> On 15/07/2023 20:28, Uwe Kleine-König wrote:
> > On Wed, Jun 14, 2023 at 06:14:53PM +0100, 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>
> > > ---
> > > v8:
> > > - add module namespace
> > > - remove compile-test for pci case, doesn't make sense
> > > - fix makefile, missed config symbol changes
> > > 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 | 14 ++-
> > > drivers/pwm/Makefile | 1 +
> > > drivers/pwm/pwm-dwc-core.c | 176 +++++++++++++++++++++++++++++++++
> > > drivers/pwm/pwm-dwc.c | 197 +------------------------------------
> > > drivers/pwm/pwm-dwc.h | 60 +++++++++++
> > > 5 files changed, 253 insertions(+), 195 deletions(-)
> > > create mode 100644 drivers/pwm/pwm-dwc-core.c
> > > create mode 100644 drivers/pwm/pwm-dwc.h
> > >
> > > diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
> > > index 8df861b1f4a3..7c54cdcb97a0 100644
> > > --- a/drivers/pwm/Kconfig
> > > +++ b/drivers/pwm/Kconfig
> > > @@ -186,9 +186,19 @@ config PWM_CROS_EC
> > > PWM driver for exposing a PWM attached to the ChromeOS Embedded
> > > Controller.
> > > +config PWM_DWC_CORE
> > > + tristate
> > > + depends on HAS_IOMEM
> > > + help
> > > + PWM driver for Synopsys DWC PWM Controller.
> > > +
> > > + To compile this driver as a module, build the dependecies as
> > > + modules, this will be called pwm-dwc-core.
> > > +
> > > config PWM_DWC
> > > - tristate "DesignWare PWM Controller"
> > > - depends on PCI
> > > + 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.
> > > diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile
> > > index 19899b912e00..de3ed77e8d7c 100644
> > > --- a/drivers/pwm/Makefile
> > > +++ b/drivers/pwm/Makefile
> > > @@ -15,6 +15,7 @@ obj-$(CONFIG_PWM_CLK) += pwm-clk.o
> > > 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_CORE) += pwm-dwc-core.o
> > > obj-$(CONFIG_PWM_DWC) += pwm-dwc.o
> >
> > Would it make sense to call this pwm-dwc-pci.o? And the symbol
> > CONFIG_PWM_DWC_PCI? (The latter would break make oldconfig. Hmm, I'm
> > unsure myself.)
>
> i left the pci as the pwm-dwc so that anyone moving up and using
> this as a module won't have to change config or their module loading
> if they're not autoloading modules.
Yeah, I thought so. I don't care much either way, but maybe that's worth
mentioning in the commit log or even a comment in drivers/pwm/Makefile.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | https://www.pengutronix.de/ |
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v8 2/5] pwm: dwc: make timer clock configurable
2023-06-14 17:14 [PATCH v8 0/5] DesignWare PWM driver updates Ben Dooks
2023-06-14 17:14 ` [PATCH v8 1/5] pwm: dwc: split pci out of core driver Ben Dooks
@ 2023-06-14 17:14 ` Ben Dooks
2023-06-14 17:14 ` [PATCH v8 3/5] pwm: dwc: add PWM bit unset in get_state call Ben Dooks
` (3 subsequent siblings)
5 siblings, 0 replies; 14+ messages in thread
From: Ben Dooks @ 2023-06-14 17:14 UTC (permalink / raw)
To: linux-pwm
Cc: devicetree, linux-kernel, ben.dooks, u.kleine-koenig,
Thierry Reding, Krzysztof Kozlowski, Greentime Hu, jarkko.nikula,
William Salmon, Jude Onyenegecha, Ben Dooks
Add a configurable clock base rate for the pwm as when being built
for non-PCI the block may be sourced from an internal clock.
Signed-off-by: Ben Dooks <ben.dooks@sifive.com>
Reviewed-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
v8:
- add reviewed by, fixed issue with previous renames.
v7:
- remove the "struct clk *" clk field from dwc_pwm_ctx, not used here,
v6:
- removed DWC_CLK_PERIOD_NS as it is now not needed
v4:
- moved earlier before the of changes to make the of changes one patch
v2:
- removed the ifdef and merged the other clock patch in here
---
drivers/pwm/pwm-dwc-core.c | 9 +++++----
drivers/pwm/pwm-dwc.h | 2 +-
2 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/pwm/pwm-dwc-core.c b/drivers/pwm/pwm-dwc-core.c
index b693cb7fa812..4b4b7b9e1d82 100644
--- a/drivers/pwm/pwm-dwc-core.c
+++ b/drivers/pwm/pwm-dwc-core.c
@@ -49,13 +49,13 @@ static int __dwc_pwm_configure_timer(struct dwc_pwm *dwc,
* periods and check are the result within HW limits between 1 and
* 2^32 periods.
*/
- tmp = DIV_ROUND_CLOSEST_ULL(state->duty_cycle, DWC_CLK_PERIOD_NS);
+ tmp = DIV_ROUND_CLOSEST_ULL(state->duty_cycle, dwc->clk_ns);
if (tmp < 1 || tmp > (1ULL << 32))
return -ERANGE;
low = tmp - 1;
tmp = DIV_ROUND_CLOSEST_ULL(state->period - state->duty_cycle,
- DWC_CLK_PERIOD_NS);
+ dwc->clk_ns);
if (tmp < 1 || tmp > (1ULL << 32))
return -ERANGE;
high = tmp - 1;
@@ -130,12 +130,12 @@ static int dwc_pwm_get_state(struct pwm_chip *chip, struct pwm_device *pwm,
duty = dwc_pwm_readl(dwc, DWC_TIM_LD_CNT(pwm->hwpwm));
duty += 1;
- duty *= DWC_CLK_PERIOD_NS;
+ duty *= dwc->clk_ns;
state->duty_cycle = duty;
period = dwc_pwm_readl(dwc, DWC_TIM_LD_CNT2(pwm->hwpwm));
period += 1;
- period *= DWC_CLK_PERIOD_NS;
+ period *= dwc->clk_ns;
period += duty;
state->period = period;
@@ -160,6 +160,7 @@ struct dwc_pwm *dwc_pwm_alloc(struct device *dev)
if (!dwc)
return NULL;
+ dwc->clk_ns = 10;
dwc->chip.dev = dev;
dwc->chip.ops = &dwc_pwm_ops;
dwc->chip.npwm = DWC_TIMERS_TOTAL;
diff --git a/drivers/pwm/pwm-dwc.h b/drivers/pwm/pwm-dwc.h
index 56deab4e28ec..64795247c54c 100644
--- a/drivers/pwm/pwm-dwc.h
+++ b/drivers/pwm/pwm-dwc.h
@@ -24,7 +24,6 @@ MODULE_IMPORT_NS(dwc_pwm);
#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)
@@ -43,6 +42,7 @@ struct dwc_pwm_ctx {
struct dwc_pwm {
struct pwm_chip chip;
void __iomem *base;
+ unsigned int clk_ns;
struct dwc_pwm_ctx ctx[DWC_TIMERS_TOTAL];
};
#define to_dwc_pwm(p) (container_of((p), struct dwc_pwm, chip))
--
2.39.2
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH v8 3/5] pwm: dwc: add PWM bit unset in get_state call
2023-06-14 17:14 [PATCH v8 0/5] DesignWare PWM driver updates Ben Dooks
2023-06-14 17:14 ` [PATCH v8 1/5] pwm: dwc: split pci out of core driver Ben Dooks
2023-06-14 17:14 ` [PATCH v8 2/5] pwm: dwc: make timer clock configurable Ben Dooks
@ 2023-06-14 17:14 ` Ben Dooks
2023-07-15 19:33 ` Uwe Kleine-König
2023-06-14 17:14 ` [PATCH v8 4/5] pwm: dwc: use clock rate in hz to avoid rounding issues Ben Dooks
` (2 subsequent siblings)
5 siblings, 1 reply; 14+ messages in thread
From: Ben Dooks @ 2023-06-14 17:14 UTC (permalink / raw)
To: linux-pwm
Cc: devicetree, linux-kernel, ben.dooks, u.kleine-koenig,
Thierry Reding, Krzysztof Kozlowski, Greentime Hu, jarkko.nikula,
William Salmon, Jude Onyenegecha, Ben Dooks
If we are not in PWM mode, then the output is technically a 50%
output based on a single timer instead of the high-low based on
the two counters. Add a check for the PWM mode in dwc_pwm_get_state()
and if DWC_TIM_CTRL_PWM is not set, then return a 50% cycle.
This may only be an issue on initialisation, as the rest of the
code currently assumes we're always going to have the extended
PWM mode using two counters.
Signed-off-by: Ben Dooks <ben.dooks@sifive.com>
---
v8:
- fixed rename issues
v4:
- fixed review comment on mulit-line calculations
---
drivers/pwm/pwm-dwc-core.c | 29 ++++++++++++++++++-----------
1 file changed, 18 insertions(+), 11 deletions(-)
diff --git a/drivers/pwm/pwm-dwc-core.c b/drivers/pwm/pwm-dwc-core.c
index 4b4b7b9e1d82..38cd2163fe01 100644
--- a/drivers/pwm/pwm-dwc-core.c
+++ b/drivers/pwm/pwm-dwc-core.c
@@ -122,24 +122,31 @@ static int dwc_pwm_get_state(struct pwm_chip *chip, struct pwm_device *pwm,
{
struct dwc_pwm *dwc = to_dwc_pwm(chip);
u64 duty, period;
+ u32 ctrl, ld, ld2;
pm_runtime_get_sync(chip->dev);
- state->enabled = !!(dwc_pwm_readl(dwc,
- DWC_TIM_CTRL(pwm->hwpwm)) & DWC_TIM_CTRL_EN);
+ ctrl = dwc_pwm_readl(dwc, DWC_TIM_CTRL(pwm->hwpwm));
+ ld = dwc_pwm_readl(dwc, DWC_TIM_LD_CNT(pwm->hwpwm));
+ ld2 = dwc_pwm_readl(dwc, DWC_TIM_LD_CNT2(pwm->hwpwm));
- duty = dwc_pwm_readl(dwc, DWC_TIM_LD_CNT(pwm->hwpwm));
- duty += 1;
- duty *= dwc->clk_ns;
- state->duty_cycle = duty;
+ state->enabled = !!(ctrl & DWC_TIM_CTRL_EN);
- period = dwc_pwm_readl(dwc, DWC_TIM_LD_CNT2(pwm->hwpwm));
- period += 1;
- period *= dwc->clk_ns;
- period += duty;
- state->period = period;
+ /* If we're not in PWM, technically the output is a 50-50
+ * based on the timer load-count only.
+ */
+ if (ctrl & DWC_TIM_CTRL_PWM) {
+ duty = (ld + 1) * dwc->clk_ns;
+ period = (ld2 + 1) * dwc->clk_ns;
+ period += duty;
+ } else {
+ duty = (ld + 1) * dwc->clk_ns;
+ period = duty * 2;
+ }
state->polarity = PWM_POLARITY_INVERSED;
+ state->period = period;
+ state->duty_cycle = duty;
pm_runtime_put_sync(chip->dev);
--
2.39.2
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH v8 3/5] pwm: dwc: add PWM bit unset in get_state call
2023-06-14 17:14 ` [PATCH v8 3/5] pwm: dwc: add PWM bit unset in get_state call Ben Dooks
@ 2023-07-15 19:33 ` Uwe Kleine-König
0 siblings, 0 replies; 14+ messages in thread
From: Uwe Kleine-König @ 2023-07-15 19:33 UTC (permalink / raw)
To: Ben Dooks
Cc: linux-pwm, devicetree, linux-kernel, ben.dooks, Thierry Reding,
Krzysztof Kozlowski, Greentime Hu, jarkko.nikula, William Salmon,
Jude Onyenegecha
[-- Attachment #1: Type: text/plain, Size: 2588 bytes --]
On Wed, Jun 14, 2023 at 06:14:55PM +0100, Ben Dooks wrote:
> If we are not in PWM mode, then the output is technically a 50%
> output based on a single timer instead of the high-low based on
> the two counters. Add a check for the PWM mode in dwc_pwm_get_state()
> and if DWC_TIM_CTRL_PWM is not set, then return a 50% cycle.
>
> This may only be an issue on initialisation, as the rest of the
> code currently assumes we're always going to have the extended
> PWM mode using two counters.
>
> Signed-off-by: Ben Dooks <ben.dooks@sifive.com>
> ---
> v8:
> - fixed rename issues
> v4:
> - fixed review comment on mulit-line calculations
> ---
> drivers/pwm/pwm-dwc-core.c | 29 ++++++++++++++++++-----------
> 1 file changed, 18 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/pwm/pwm-dwc-core.c b/drivers/pwm/pwm-dwc-core.c
> index 4b4b7b9e1d82..38cd2163fe01 100644
> --- a/drivers/pwm/pwm-dwc-core.c
> +++ b/drivers/pwm/pwm-dwc-core.c
> @@ -122,24 +122,31 @@ static int dwc_pwm_get_state(struct pwm_chip *chip, struct pwm_device *pwm,
> {
> struct dwc_pwm *dwc = to_dwc_pwm(chip);
> u64 duty, period;
> + u32 ctrl, ld, ld2;
>
> pm_runtime_get_sync(chip->dev);
>
> - state->enabled = !!(dwc_pwm_readl(dwc,
> - DWC_TIM_CTRL(pwm->hwpwm)) & DWC_TIM_CTRL_EN);
> + ctrl = dwc_pwm_readl(dwc, DWC_TIM_CTRL(pwm->hwpwm));
> + ld = dwc_pwm_readl(dwc, DWC_TIM_LD_CNT(pwm->hwpwm));
> + ld2 = dwc_pwm_readl(dwc, DWC_TIM_LD_CNT2(pwm->hwpwm));
>
> - duty = dwc_pwm_readl(dwc, DWC_TIM_LD_CNT(pwm->hwpwm));
> - duty += 1;
> - duty *= dwc->clk_ns;
> - state->duty_cycle = duty;
> + state->enabled = !!(ctrl & DWC_TIM_CTRL_EN);
>
> - period = dwc_pwm_readl(dwc, DWC_TIM_LD_CNT2(pwm->hwpwm));
> - period += 1;
> - period *= dwc->clk_ns;
> - period += duty;
> - state->period = period;
> + /* If we're not in PWM, technically the output is a 50-50
Multiline comments should have /* on a line for itself.
> + * based on the timer load-count only.
> + */
> + if (ctrl & DWC_TIM_CTRL_PWM) {
> + duty = (ld + 1) * dwc->clk_ns;
> + period = (ld2 + 1) * dwc->clk_ns;
s/ / /
> + period += duty;
> + } else {
> + duty = (ld + 1) * dwc->clk_ns;
> + period = duty * 2;
> + }
>
> state->polarity = PWM_POLARITY_INVERSED;
> + state->period = period;
> + state->duty_cycle = duty;
>
> pm_runtime_put_sync(chip->dev);
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | https://www.pengutronix.de/ |
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v8 4/5] pwm: dwc: use clock rate in hz to avoid rounding issues
2023-06-14 17:14 [PATCH v8 0/5] DesignWare PWM driver updates Ben Dooks
` (2 preceding siblings ...)
2023-06-14 17:14 ` [PATCH v8 3/5] pwm: dwc: add PWM bit unset in get_state call Ben Dooks
@ 2023-06-14 17:14 ` Ben Dooks
2023-07-15 19:42 ` Uwe Kleine-König
2023-06-14 17:14 ` [PATCH v8 5/5] pwm: dwc: add of/platform support Ben Dooks
2023-06-20 12:59 ` [PATCH v8 0/5] DesignWare PWM driver updates Jarkko Nikula
5 siblings, 1 reply; 14+ messages in thread
From: Ben Dooks @ 2023-06-14 17:14 UTC (permalink / raw)
To: linux-pwm
Cc: devicetree, linux-kernel, ben.dooks, u.kleine-koenig,
Thierry Reding, Krzysztof Kozlowski, Greentime Hu, jarkko.nikula,
William Salmon, Jude Onyenegecha, Ben Dooks
As noted, the clock-rate when not a nice multiple of ns is probably
going to end up with inacurate caculations, as well as on a non pci
system the rate may change (although we've not put a clock rate
change notifier in this code yet) so we also add some quick checks
of the rate when we do any calculations with it.
Signed-off-by; Ben Dooks <ben.dooks@sifive.com>
Reported-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
v8:
- fixup post rename
- move to earlier in series
---
drivers/pwm/pwm-dwc-core.c | 24 +++++++++++++++---------
drivers/pwm/pwm-dwc.h | 2 +-
2 files changed, 16 insertions(+), 10 deletions(-)
diff --git a/drivers/pwm/pwm-dwc-core.c b/drivers/pwm/pwm-dwc-core.c
index 38cd2163fe01..0f07e26e6c30 100644
--- a/drivers/pwm/pwm-dwc-core.c
+++ b/drivers/pwm/pwm-dwc-core.c
@@ -49,13 +49,14 @@ static int __dwc_pwm_configure_timer(struct dwc_pwm *dwc,
* periods and check are the result within HW limits between 1 and
* 2^32 periods.
*/
- tmp = DIV_ROUND_CLOSEST_ULL(state->duty_cycle, dwc->clk_ns);
+ tmp = state->duty_cycle * dwc->clk_rate;
+ tmp = DIV_ROUND_CLOSEST_ULL(tmp, NSEC_PER_SEC);
if (tmp < 1 || tmp > (1ULL << 32))
return -ERANGE;
low = tmp - 1;
- tmp = DIV_ROUND_CLOSEST_ULL(state->period - state->duty_cycle,
- dwc->clk_ns);
+ tmp = (state->period - state->duty_cycle) * dwc->clk_rate;
+ tmp = DIV_ROUND_CLOSEST_ULL(tmp, NSEC_PER_SEC);
if (tmp < 1 || tmp > (1ULL << 32))
return -ERANGE;
high = tmp - 1;
@@ -121,11 +122,14 @@ static int dwc_pwm_get_state(struct pwm_chip *chip, struct pwm_device *pwm,
struct pwm_state *state)
{
struct dwc_pwm *dwc = to_dwc_pwm(chip);
+ unsigned long clk_rate;
u64 duty, period;
u32 ctrl, ld, ld2;
pm_runtime_get_sync(chip->dev);
+ clk_rate = dwc->clk_rate;
+
ctrl = dwc_pwm_readl(dwc, DWC_TIM_CTRL(pwm->hwpwm));
ld = dwc_pwm_readl(dwc, DWC_TIM_LD_CNT(pwm->hwpwm));
ld2 = dwc_pwm_readl(dwc, DWC_TIM_LD_CNT2(pwm->hwpwm));
@@ -136,17 +140,19 @@ static int dwc_pwm_get_state(struct pwm_chip *chip, struct pwm_device *pwm,
* based on the timer load-count only.
*/
if (ctrl & DWC_TIM_CTRL_PWM) {
- duty = (ld + 1) * dwc->clk_ns;
- period = (ld2 + 1) * dwc->clk_ns;
+ duty = ld + 1;
+ period = ld2 + 1;
period += duty;
} else {
- duty = (ld + 1) * dwc->clk_ns;
+ duty = ld + 1;
period = duty * 2;
}
+ duty *= NSEC_PER_SEC;
+ period *= NSEC_PER_SEC;
+ state->period = DIV_ROUND_CLOSEST_ULL(period, clk_rate);
+ state->duty_cycle = DIV_ROUND_CLOSEST_ULL(duty, clk_rate);
state->polarity = PWM_POLARITY_INVERSED;
- state->period = period;
- state->duty_cycle = duty;
pm_runtime_put_sync(chip->dev);
@@ -167,7 +173,7 @@ struct dwc_pwm *dwc_pwm_alloc(struct device *dev)
if (!dwc)
return NULL;
- dwc->clk_ns = 10;
+ dwc->clk_rate = NSEC_PER_SEC / 10;
dwc->chip.dev = dev;
dwc->chip.ops = &dwc_pwm_ops;
dwc->chip.npwm = DWC_TIMERS_TOTAL;
diff --git a/drivers/pwm/pwm-dwc.h b/drivers/pwm/pwm-dwc.h
index 64795247c54c..e0a940fd6e87 100644
--- a/drivers/pwm/pwm-dwc.h
+++ b/drivers/pwm/pwm-dwc.h
@@ -42,7 +42,7 @@ struct dwc_pwm_ctx {
struct dwc_pwm {
struct pwm_chip chip;
void __iomem *base;
- unsigned int clk_ns;
+ unsigned long clk_rate;
struct dwc_pwm_ctx ctx[DWC_TIMERS_TOTAL];
};
#define to_dwc_pwm(p) (container_of((p), struct dwc_pwm, chip))
--
2.39.2
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH v8 4/5] pwm: dwc: use clock rate in hz to avoid rounding issues
2023-06-14 17:14 ` [PATCH v8 4/5] pwm: dwc: use clock rate in hz to avoid rounding issues Ben Dooks
@ 2023-07-15 19:42 ` Uwe Kleine-König
0 siblings, 0 replies; 14+ messages in thread
From: Uwe Kleine-König @ 2023-07-15 19:42 UTC (permalink / raw)
To: Ben Dooks
Cc: linux-pwm, devicetree, linux-kernel, ben.dooks, Thierry Reding,
Krzysztof Kozlowski, Greentime Hu, jarkko.nikula, William Salmon,
Jude Onyenegecha
[-- Attachment #1: Type: text/plain, Size: 4552 bytes --]
On Wed, Jun 14, 2023 at 06:14:56PM +0100, Ben Dooks wrote:
> As noted, the clock-rate when not a nice multiple of ns is probably
> going to end up with inacurate caculations, as well as on a non pci
s/caculation/calculations/
> system the rate may change (although we've not put a clock rate
> change notifier in this code yet) so we also add some quick checks
> of the rate when we do any calculations with it.
An externally triggered clock rate change is bad. If you drive a motor
you probably want to prevent an uncontrolled change here. I already
considered to add a call to clk_rate_exclusive_get() in various pwm
drivers for that reason, but didn't come around yet.
> Signed-off-by; Ben Dooks <ben.dooks@sifive.com>
> Reported-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
> v8:
> - fixup post rename
> - move to earlier in series
> ---
> drivers/pwm/pwm-dwc-core.c | 24 +++++++++++++++---------
> drivers/pwm/pwm-dwc.h | 2 +-
> 2 files changed, 16 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/pwm/pwm-dwc-core.c b/drivers/pwm/pwm-dwc-core.c
> index 38cd2163fe01..0f07e26e6c30 100644
> --- a/drivers/pwm/pwm-dwc-core.c
> +++ b/drivers/pwm/pwm-dwc-core.c
> @@ -49,13 +49,14 @@ static int __dwc_pwm_configure_timer(struct dwc_pwm *dwc,
> * periods and check are the result within HW limits between 1 and
> * 2^32 periods.
> */
> - tmp = DIV_ROUND_CLOSEST_ULL(state->duty_cycle, dwc->clk_ns);
> + tmp = state->duty_cycle * dwc->clk_rate;
> + tmp = DIV_ROUND_CLOSEST_ULL(tmp, NSEC_PER_SEC);
New drivers should implement round-down behaviour (i.e. pick the biggest
period (and duty_cycle) that is not bigger than the requested value.
With clk_ns = 10 (which it is the hardcoded value up to now) it doesn't
matter much how you round the division. I suggest to use the opportunity
to align to how new drivers should round. (That would be a separate
patch.)
> if (tmp < 1 || tmp > (1ULL << 32))
> return -ERANGE;
> low = tmp - 1;
>
> - tmp = DIV_ROUND_CLOSEST_ULL(state->period - state->duty_cycle,
> - dwc->clk_ns);
> + tmp = (state->period - state->duty_cycle) * dwc->clk_rate;
> + tmp = DIV_ROUND_CLOSEST_ULL(tmp, NSEC_PER_SEC);
> if (tmp < 1 || tmp > (1ULL << 32))
> return -ERANGE;
> high = tmp - 1;
> @@ -121,11 +122,14 @@ static int dwc_pwm_get_state(struct pwm_chip *chip, struct pwm_device *pwm,
> struct pwm_state *state)
> {
> struct dwc_pwm *dwc = to_dwc_pwm(chip);
> + unsigned long clk_rate;
> u64 duty, period;
> u32 ctrl, ld, ld2;
>
> pm_runtime_get_sync(chip->dev);
>
> + clk_rate = dwc->clk_rate;
> +
> ctrl = dwc_pwm_readl(dwc, DWC_TIM_CTRL(pwm->hwpwm));
> ld = dwc_pwm_readl(dwc, DWC_TIM_LD_CNT(pwm->hwpwm));
> ld2 = dwc_pwm_readl(dwc, DWC_TIM_LD_CNT2(pwm->hwpwm));
> @@ -136,17 +140,19 @@ static int dwc_pwm_get_state(struct pwm_chip *chip, struct pwm_device *pwm,
> * based on the timer load-count only.
> */
> if (ctrl & DWC_TIM_CTRL_PWM) {
> - duty = (ld + 1) * dwc->clk_ns;
> - period = (ld2 + 1) * dwc->clk_ns;
> + duty = ld + 1;
> + period = ld2 + 1;
> period += duty;
> } else {
> - duty = (ld + 1) * dwc->clk_ns;
> + duty = ld + 1;
> period = duty * 2;
> }
>
> + duty *= NSEC_PER_SEC;
> + period *= NSEC_PER_SEC;
> + state->period = DIV_ROUND_CLOSEST_ULL(period, clk_rate);
> + state->duty_cycle = DIV_ROUND_CLOSEST_ULL(duty, clk_rate);
> state->polarity = PWM_POLARITY_INVERSED;
> - state->period = period;
> - state->duty_cycle = duty;
>
> pm_runtime_put_sync(chip->dev);
>
> @@ -167,7 +173,7 @@ struct dwc_pwm *dwc_pwm_alloc(struct device *dev)
> if (!dwc)
> return NULL;
>
> - dwc->clk_ns = 10;
> + dwc->clk_rate = NSEC_PER_SEC / 10;
> dwc->chip.dev = dev;
> dwc->chip.ops = &dwc_pwm_ops;
> dwc->chip.npwm = DWC_TIMERS_TOTAL;
> diff --git a/drivers/pwm/pwm-dwc.h b/drivers/pwm/pwm-dwc.h
> index 64795247c54c..e0a940fd6e87 100644
> --- a/drivers/pwm/pwm-dwc.h
> +++ b/drivers/pwm/pwm-dwc.h
> @@ -42,7 +42,7 @@ struct dwc_pwm_ctx {
> struct dwc_pwm {
> struct pwm_chip chip;
> void __iomem *base;
> - unsigned int clk_ns;
> + unsigned long clk_rate;
> struct dwc_pwm_ctx ctx[DWC_TIMERS_TOTAL];
> };
> #define to_dwc_pwm(p) (container_of((p), struct dwc_pwm, chip))
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | https://www.pengutronix.de/ |
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v8 5/5] pwm: dwc: add of/platform support
2023-06-14 17:14 [PATCH v8 0/5] DesignWare PWM driver updates Ben Dooks
` (3 preceding siblings ...)
2023-06-14 17:14 ` [PATCH v8 4/5] pwm: dwc: use clock rate in hz to avoid rounding issues Ben Dooks
@ 2023-06-14 17:14 ` Ben Dooks
2023-07-15 19:51 ` Uwe Kleine-König
2023-06-20 12:59 ` [PATCH v8 0/5] DesignWare PWM driver updates Jarkko Nikula
5 siblings, 1 reply; 14+ messages in thread
From: Ben Dooks @ 2023-06-14 17:14 UTC (permalink / raw)
To: linux-pwm
Cc: devicetree, linux-kernel, ben.dooks, u.kleine-koenig,
Thierry Reding, Krzysztof Kozlowski, Greentime Hu, jarkko.nikula,
William Salmon, Jude Onyenegecha, Ben Dooks
The dwc pwm controller can be used in non-PCI systems, so allow
either platform or OF based probing.
Signed-off-by: Ben Dooks <ben.dooks@sifive.com>
---
v8:
- add compile test for of-case
- add module namespace
- move later in the series
v7:
- fixup kconfig from previous pcie changes
v5:
- fix missing " in kconfig
- remove .remove method, devm already sorts this.
- merge pwm-number code
- split the of code out of the core
- get bus clock
v4:
- moved the compile test code earlier
- fixed review comments
- used NS_PER_SEC
- use devm_clk_get_enabled
- ensure we get the bus clock
v3:
- changed compatible name
---
drivers/pwm/Kconfig | 10 +++++
drivers/pwm/Makefile | 1 +
drivers/pwm/pwm-dwc-core.c | 6 +++
drivers/pwm/pwm-dwc-of.c | 78 ++++++++++++++++++++++++++++++++++++++
drivers/pwm/pwm-dwc.c | 1 +
drivers/pwm/pwm-dwc.h | 1 +
6 files changed, 97 insertions(+)
create mode 100644 drivers/pwm/pwm-dwc-of.c
diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
index 7c54cdcb97a0..61f5d3f30fd7 100644
--- a/drivers/pwm/Kconfig
+++ b/drivers/pwm/Kconfig
@@ -205,6 +205,16 @@ config PWM_DWC
To compile this driver as a module, choose M here: the module
will be called pwm-dwc.
+config PWM_DWC_OF
+ tristate "DesignWare PWM Controller (OF bus)"
+ depends on HAS_IOMEM && (OF || COMPILE_TEST)
+ select PWM_DWC_CORE
+ help
+ PWM driver for Synopsys DWC PWM Controller on an OF bus.
+
+ To compile this driver as a module, choose M here: the module
+ will be called pwm-dwc-of.
+
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 de3ed77e8d7c..d27dfbb850b7 100644
--- a/drivers/pwm/Makefile
+++ b/drivers/pwm/Makefile
@@ -17,6 +17,7 @@ obj-$(CONFIG_PWM_CRC) += pwm-crc.o
obj-$(CONFIG_PWM_CROS_EC) += pwm-cros-ec.o
obj-$(CONFIG_PWM_DWC_CORE) += pwm-dwc-core.o
obj-$(CONFIG_PWM_DWC) += pwm-dwc.o
+obj-$(CONFIG_PWM_DWC_OF) += pwm-dwc-of.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-core.c b/drivers/pwm/pwm-dwc-core.c
index 0f07e26e6c30..ed102fc4b30a 100644
--- a/drivers/pwm/pwm-dwc-core.c
+++ b/drivers/pwm/pwm-dwc-core.c
@@ -16,6 +16,7 @@
#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/pci.h>
+#include <linux/clk.h>
#include <linux/pm_runtime.h>
#include <linux/pwm.h>
@@ -44,6 +45,9 @@ static int __dwc_pwm_configure_timer(struct dwc_pwm *dwc,
u32 high;
u32 low;
+ if (dwc->clk)
+ dwc->clk_rate = clk_get_rate(dwc->clk);
+
/*
* Calculate width of low and high period in terms of input clock
* periods and check are the result within HW limits between 1 and
@@ -128,6 +132,8 @@ static int dwc_pwm_get_state(struct pwm_chip *chip, struct pwm_device *pwm,
pm_runtime_get_sync(chip->dev);
+ if (dwc->clk)
+ dwc->clk_rate = clk_get_rate(dwc->clk);
clk_rate = dwc->clk_rate;
ctrl = dwc_pwm_readl(dwc, DWC_TIM_CTRL(pwm->hwpwm));
diff --git a/drivers/pwm/pwm-dwc-of.c b/drivers/pwm/pwm-dwc-of.c
new file mode 100644
index 000000000000..13a0b534b383
--- /dev/null
+++ b/drivers/pwm/pwm-dwc-of.c
@@ -0,0 +1,78 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * DesignWare PWM Controller driver OF
+ *
+ * Copyright (C) 2022 SiFive, Inc.
+ */
+
+#define DEFAULT_MODULE_NAMESACE dwc_pwm
+
+#include <linux/bitops.h>
+#include <linux/export.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/clk.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+#include <linux/pwm.h>
+#include <linux/io.h>
+
+#include "pwm-dwc.h"
+
+static int dwc_pwm_plat_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct dwc_pwm *dwc;
+ struct clk *bus;
+ u32 nr_pwm;
+
+ dwc = dwc_pwm_alloc(dev);
+ if (!dwc)
+ return -ENOMEM;
+
+ if (!device_property_read_u32(dev, "snps,pwm-number", &nr_pwm)) {
+ if (nr_pwm > DWC_TIMERS_TOTAL)
+ dev_err(dev, "too many PWMs (%d) specified, capping at %d\n",
+ nr_pwm, dwc->chip.npwm);
+ else
+ dwc->chip.npwm = nr_pwm;
+ }
+
+ dwc->base = devm_platform_ioremap_resource(pdev, 0);
+ if (IS_ERR(dwc->base))
+ return PTR_ERR(dwc->base);
+
+ bus = devm_clk_get_enabled(dev, NULL);
+ if (IS_ERR(bus))
+ return dev_err_probe(dev, PTR_ERR(bus),
+ "failed to get clock\n");
+
+ dwc->clk = devm_clk_get_enabled(dev, "timer");
+ if (IS_ERR(dwc->clk))
+ return dev_err_probe(dev, PTR_ERR(dwc->clk),
+ "failed to get timer clock\n");
+
+ dwc->clk_rate = clk_get_rate(dwc->clk);
+ return devm_pwmchip_add(dev, &dwc->chip);
+}
+
+static const struct of_device_id dwc_pwm_dt_ids[] = {
+ { .compatible = "snps,dw-apb-timers-pwm2" },
+ { },
+};
+MODULE_DEVICE_TABLE(of, dwc_pwm_dt_ids);
+
+static struct platform_driver dwc_pwm_plat_driver = {
+ .driver = {
+ .name = "dwc-pwm",
+ .of_match_table = dwc_pwm_dt_ids,
+ },
+ .probe = dwc_pwm_plat_probe,
+};
+
+module_platform_driver(dwc_pwm_plat_driver);
+
+MODULE_ALIAS("platform:dwc-pwm-of");
+MODULE_AUTHOR("Ben Dooks <ben.dooks@sifive.com>");
+MODULE_DESCRIPTION("DesignWare PWM Controller");
+MODULE_LICENSE("GPL");
diff --git a/drivers/pwm/pwm-dwc.c b/drivers/pwm/pwm-dwc.c
index bd9cadb497d7..7c32bd06ed33 100644
--- a/drivers/pwm/pwm-dwc.c
+++ b/drivers/pwm/pwm-dwc.c
@@ -20,6 +20,7 @@
#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/pci.h>
+#include <linux/clk.h>
#include <linux/pm_runtime.h>
#include <linux/pwm.h>
diff --git a/drivers/pwm/pwm-dwc.h b/drivers/pwm/pwm-dwc.h
index e0a940fd6e87..18e98c2c07d7 100644
--- a/drivers/pwm/pwm-dwc.h
+++ b/drivers/pwm/pwm-dwc.h
@@ -42,6 +42,7 @@ struct dwc_pwm_ctx {
struct dwc_pwm {
struct pwm_chip chip;
void __iomem *base;
+ struct clk *clk;
unsigned long clk_rate;
struct dwc_pwm_ctx ctx[DWC_TIMERS_TOTAL];
};
--
2.39.2
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH v8 5/5] pwm: dwc: add of/platform support
2023-06-14 17:14 ` [PATCH v8 5/5] pwm: dwc: add of/platform support Ben Dooks
@ 2023-07-15 19:51 ` Uwe Kleine-König
0 siblings, 0 replies; 14+ messages in thread
From: Uwe Kleine-König @ 2023-07-15 19:51 UTC (permalink / raw)
To: Ben Dooks
Cc: linux-pwm, devicetree, linux-kernel, ben.dooks, Thierry Reding,
Krzysztof Kozlowski, Greentime Hu, jarkko.nikula, William Salmon,
Jude Onyenegecha
[-- Attachment #1: Type: text/plain, Size: 6255 bytes --]
On Wed, Jun 14, 2023 at 06:14:57PM +0100, Ben Dooks wrote:
> The dwc pwm controller can be used in non-PCI systems, so allow
> either platform or OF based probing.
A document describing the binding is needed here.
>
> Signed-off-by: Ben Dooks <ben.dooks@sifive.com>
> ---
> v8:
> - add compile test for of-case
> - add module namespace
> - move later in the series
> v7:
> - fixup kconfig from previous pcie changes
> v5:
> - fix missing " in kconfig
> - remove .remove method, devm already sorts this.
> - merge pwm-number code
> - split the of code out of the core
> - get bus clock
> v4:
> - moved the compile test code earlier
> - fixed review comments
> - used NS_PER_SEC
> - use devm_clk_get_enabled
> - ensure we get the bus clock
> v3:
> - changed compatible name
> ---
> drivers/pwm/Kconfig | 10 +++++
> drivers/pwm/Makefile | 1 +
> drivers/pwm/pwm-dwc-core.c | 6 +++
> drivers/pwm/pwm-dwc-of.c | 78 ++++++++++++++++++++++++++++++++++++++
> drivers/pwm/pwm-dwc.c | 1 +
> drivers/pwm/pwm-dwc.h | 1 +
> 6 files changed, 97 insertions(+)
> create mode 100644 drivers/pwm/pwm-dwc-of.c
>
> diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
> index 7c54cdcb97a0..61f5d3f30fd7 100644
> --- a/drivers/pwm/Kconfig
> +++ b/drivers/pwm/Kconfig
> @@ -205,6 +205,16 @@ config PWM_DWC
> To compile this driver as a module, choose M here: the module
> will be called pwm-dwc.
>
> +config PWM_DWC_OF
> + tristate "DesignWare PWM Controller (OF bus)"
> + depends on HAS_IOMEM && (OF || COMPILE_TEST)
> + select PWM_DWC_CORE
> + help
> + PWM driver for Synopsys DWC PWM Controller on an OF bus.
> +
> + To compile this driver as a module, choose M here: the module
> + will be called pwm-dwc-of.
> +
> 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 de3ed77e8d7c..d27dfbb850b7 100644
> --- a/drivers/pwm/Makefile
> +++ b/drivers/pwm/Makefile
> @@ -17,6 +17,7 @@ obj-$(CONFIG_PWM_CRC) += pwm-crc.o
> obj-$(CONFIG_PWM_CROS_EC) += pwm-cros-ec.o
> obj-$(CONFIG_PWM_DWC_CORE) += pwm-dwc-core.o
> obj-$(CONFIG_PWM_DWC) += pwm-dwc.o
> +obj-$(CONFIG_PWM_DWC_OF) += pwm-dwc-of.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-core.c b/drivers/pwm/pwm-dwc-core.c
> index 0f07e26e6c30..ed102fc4b30a 100644
> --- a/drivers/pwm/pwm-dwc-core.c
> +++ b/drivers/pwm/pwm-dwc-core.c
> @@ -16,6 +16,7 @@
> #include <linux/kernel.h>
> #include <linux/module.h>
> #include <linux/pci.h>
> +#include <linux/clk.h>
> #include <linux/pm_runtime.h>
> #include <linux/pwm.h>
>
> @@ -44,6 +45,9 @@ static int __dwc_pwm_configure_timer(struct dwc_pwm *dwc,
> u32 high;
> u32 low;
>
> + if (dwc->clk)
> + dwc->clk_rate = clk_get_rate(dwc->clk);
> +
> /*
> * Calculate width of low and high period in terms of input clock
> * periods and check are the result within HW limits between 1 and
> @@ -128,6 +132,8 @@ static int dwc_pwm_get_state(struct pwm_chip *chip, struct pwm_device *pwm,
>
> pm_runtime_get_sync(chip->dev);
>
> + if (dwc->clk)
> + dwc->clk_rate = clk_get_rate(dwc->clk);
> clk_rate = dwc->clk_rate;
>
> ctrl = dwc_pwm_readl(dwc, DWC_TIM_CTRL(pwm->hwpwm));
> diff --git a/drivers/pwm/pwm-dwc-of.c b/drivers/pwm/pwm-dwc-of.c
> new file mode 100644
> index 000000000000..13a0b534b383
> --- /dev/null
> +++ b/drivers/pwm/pwm-dwc-of.c
> @@ -0,0 +1,78 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * DesignWare PWM Controller driver OF
> + *
> + * Copyright (C) 2022 SiFive, Inc.
> + */
> +
> +#define DEFAULT_MODULE_NAMESACE dwc_pwm
missing P? I'd have put this into drivers/pwm/pwm-dwc.h.
> +
> +#include <linux/bitops.h>
> +#include <linux/export.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/clk.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/pwm.h>
> +#include <linux/io.h>
> +
> +#include "pwm-dwc.h"
> +
> +static int dwc_pwm_plat_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct dwc_pwm *dwc;
> + struct clk *bus;
> + u32 nr_pwm;
> +
> + dwc = dwc_pwm_alloc(dev);
> + if (!dwc)
> + return -ENOMEM;
> +
> + if (!device_property_read_u32(dev, "snps,pwm-number", &nr_pwm)) {
> + if (nr_pwm > DWC_TIMERS_TOTAL)
> + dev_err(dev, "too many PWMs (%d) specified, capping at %d\n",
> + nr_pwm, dwc->chip.npwm);
> + else
> + dwc->chip.npwm = nr_pwm;
> + }
> +
> + dwc->base = devm_platform_ioremap_resource(pdev, 0);
> + if (IS_ERR(dwc->base))
> + return PTR_ERR(dwc->base);
> +
> + bus = devm_clk_get_enabled(dev, NULL);
> + if (IS_ERR(bus))
> + return dev_err_probe(dev, PTR_ERR(bus),
> + "failed to get clock\n");
> +
> + dwc->clk = devm_clk_get_enabled(dev, "timer");
> + if (IS_ERR(dwc->clk))
> + return dev_err_probe(dev, PTR_ERR(dwc->clk),
> + "failed to get timer clock\n");
> +
> + dwc->clk_rate = clk_get_rate(dwc->clk);
Do you need this here? Isn't clk_rate assigned each time it's used when
clk != NULL?
> + return devm_pwmchip_add(dev, &dwc->chip);
> +}
> +
> +static const struct of_device_id dwc_pwm_dt_ids[] = {
> + { .compatible = "snps,dw-apb-timers-pwm2" },
> + { },
> +};
> +MODULE_DEVICE_TABLE(of, dwc_pwm_dt_ids);
> +
> +static struct platform_driver dwc_pwm_plat_driver = {
> + .driver = {
> + .name = "dwc-pwm",
> + .of_match_table = dwc_pwm_dt_ids,
> + },
> + .probe = dwc_pwm_plat_probe,
> +};
> +
> +module_platform_driver(dwc_pwm_plat_driver);
> +
> +MODULE_ALIAS("platform:dwc-pwm-of");
> +MODULE_AUTHOR("Ben Dooks <ben.dooks@sifive.com>");
Given that this email address is (or soon will be) unavailable, maybe
better put your codethink address here?
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | https://www.pengutronix.de/ |
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v8 0/5] DesignWare PWM driver updates
2023-06-14 17:14 [PATCH v8 0/5] DesignWare PWM driver updates Ben Dooks
` (4 preceding siblings ...)
2023-06-14 17:14 ` [PATCH v8 5/5] pwm: dwc: add of/platform support Ben Dooks
@ 2023-06-20 12:59 ` Jarkko Nikula
2023-06-23 9:46 ` Ben Dooks
5 siblings, 1 reply; 14+ messages in thread
From: Jarkko Nikula @ 2023-06-20 12:59 UTC (permalink / raw)
To: Ben Dooks, linux-pwm
Cc: devicetree, linux-kernel, ben.dooks, u.kleine-koenig,
Thierry Reding, Krzysztof Kozlowski, Greentime Hu, William Salmon,
Jude Onyenegecha
On 6/14/23 20:14, Ben Dooks wrote:
> This series is an update for the DesignWare PWM driver to add support for
> OF (and split the PCI bits out if aynone else wants them). This is mostly
> the same as the v7 series, but with code moved around and module-namespace
> added, plus review comments processed.
>
> Since we no longer have the hardware, the clock code hasn't been changed to
> either lock the rate whilst the PWM is running, or to deal with any sort
> of change callback. This is left to future work (and I would rather get
> something in that does currently work) (second note, the hardware we did
> use had a fixed clock tree anyway)
>
> This account is probably going away soon, I have cc'd my main work
> email to catch any responses.
>
> Thank you all for the reviews.
>
I tested the patchset on Intel Elkhart Lake and didn't see issues.
Tested-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v8 0/5] DesignWare PWM driver updates
2023-06-20 12:59 ` [PATCH v8 0/5] DesignWare PWM driver updates Jarkko Nikula
@ 2023-06-23 9:46 ` Ben Dooks
0 siblings, 0 replies; 14+ messages in thread
From: Ben Dooks @ 2023-06-23 9:46 UTC (permalink / raw)
To: Jarkko Nikula
Cc: Ben Dooks, linux-pwm, devicetree, linux-kernel, u.kleine-koenig,
Thierry Reding, Krzysztof Kozlowski, Greentime Hu, William Salmon,
Jude Onyenegecha
On 2023-06-20 13:59, Jarkko Nikula wrote:
> On 6/14/23 20:14, Ben Dooks wrote:
>> This series is an update for the DesignWare PWM driver to add support
>> for
>> OF (and split the PCI bits out if aynone else wants them). This is
>> mostly
>> the same as the v7 series, but with code moved around and
>> module-namespace
>> added, plus review comments processed.
>>
>> Since we no longer have the hardware, the clock code hasn't been
>> changed to
>> either lock the rate whilst the PWM is running, or to deal with any
>> sort
>> of change callback. This is left to future work (and I would rather
>> get
>> something in that does currently work) (second note, the hardware we
>> did
>> use had a fixed clock tree anyway)
>>
>> This account is probably going away soon, I have cc'd my main work
>> email to catch any responses.
>>
>> Thank you all for the reviews.
>>
> I tested the patchset on Intel Elkhart Lake and didn't see issues.
>
> Tested-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
Great, thank you.
Is this series likely to get into the next kernel release?
--
ben
^ permalink raw reply [flat|nested] 14+ messages in thread