* [PATCH v9 1/6] pwm: dwc: split pci out of core driver
2023-09-07 16:12 [PATCH v9 0/6] designware pwm driver updates Ben Dooks
@ 2023-09-07 16:12 ` Ben Dooks
2023-09-07 21:15 ` Uwe Kleine-König
2023-09-07 16:12 ` [PATCH v9 2/6] pwm: dwc: make timer clock configurable Ben Dooks
` (4 subsequent siblings)
5 siblings, 1 reply; 14+ messages in thread
From: Ben Dooks @ 2023-09-07 16:12 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
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@codethink.co.uk>
---
v9:
- change back to codethink email from sifive
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 8ebcddf91f7b..507c8b8547a5 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 c822389c2a24..c5ec9e168ee7 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.40.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH v9 1/6] pwm: dwc: split pci out of core driver
2023-09-07 16:12 ` [PATCH v9 1/6] pwm: dwc: split pci out of core driver Ben Dooks
@ 2023-09-07 21:15 ` Uwe Kleine-König
0 siblings, 0 replies; 14+ messages in thread
From: Uwe Kleine-König @ 2023-09-07 21:15 UTC (permalink / raw)
To: Ben Dooks
Cc: linux-pwm, devicetree, linux-kernel, Thierry Reding,
Krzysztof Kozlowski, Greentime Hu, jarkko.nikula, William Salmon,
Jude Onyenegecha
[-- Attachment #1: Type: text/plain, Size: 1795 bytes --]
Hello,
On Thu, Sep 07, 2023 at 05:12:37PM +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@codethink.co.uk>
Great you continue here even though your commercial interest already
ended!
This series conflicts with my lifetime series[1], but I guess we can
sort that one when at least one of these two series is applied.
[1] https://lore.kernel.org/linux-pwm/20230808171931.944154-1-u.kleine-koenig@pengutronix.de/
> [...]
> 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
> +
There is no exported symbol in this driver, so this #define is unused.
> #include <linux/bitops.h>
> #include <linux/export.h>
> #include <linux/kernel.h>
Otherwise looks fine:
Acked-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
(with or without dropping DEFAULT_MOUDLE_NAMESPACE from pwm-dwc.c)
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 v9 2/6] pwm: dwc: make timer clock configurable
2023-09-07 16:12 [PATCH v9 0/6] designware pwm driver updates Ben Dooks
2023-09-07 16:12 ` [PATCH v9 1/6] pwm: dwc: split pci out of core driver Ben Dooks
@ 2023-09-07 16:12 ` Ben Dooks
2023-09-07 16:12 ` [PATCH v9 3/6] 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-09-07 16:12 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
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@codethink.co.uk>
Reviewed-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
v9:
- moved email to codethink from sifive
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.40.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH v9 3/6] pwm: dwc: add PWM bit unset in get_state call
2023-09-07 16:12 [PATCH v9 0/6] designware pwm driver updates Ben Dooks
2023-09-07 16:12 ` [PATCH v9 1/6] pwm: dwc: split pci out of core driver Ben Dooks
2023-09-07 16:12 ` [PATCH v9 2/6] pwm: dwc: make timer clock configurable Ben Dooks
@ 2023-09-07 16:12 ` Ben Dooks
2023-09-22 17:35 ` Uwe Kleine-König
2023-09-07 16:12 ` [PATCH v9 4/6] 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-09-07 16:12 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
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@codethink.co.uk>
---
v9:
- fixed multi-line comment
- put authour back to codethink email from sifive
v8:
- fixed rename issues
v4:
- fixed review comment on mulit-line calculations
---
drivers/pwm/pwm-dwc-core.c | 30 +++++++++++++++++++-----------
1 file changed, 19 insertions(+), 11 deletions(-)
diff --git a/drivers/pwm/pwm-dwc-core.c b/drivers/pwm/pwm-dwc-core.c
index 4b4b7b9e1d82..3fc281a78c9a 100644
--- a/drivers/pwm/pwm-dwc-core.c
+++ b/drivers/pwm/pwm-dwc-core.c
@@ -122,24 +122,32 @@ 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.40.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH v9 3/6] pwm: dwc: add PWM bit unset in get_state call
2023-09-07 16:12 ` [PATCH v9 3/6] pwm: dwc: add PWM bit unset in get_state call Ben Dooks
@ 2023-09-22 17:35 ` Uwe Kleine-König
2023-09-25 7:02 ` Ben Dooks
0 siblings, 1 reply; 14+ messages in thread
From: Uwe Kleine-König @ 2023-09-22 17:35 UTC (permalink / raw)
To: Ben Dooks
Cc: linux-pwm, devicetree, linux-kernel, Thierry Reding,
Krzysztof Kozlowski, Greentime Hu, jarkko.nikula
[-- Attachment #1: Type: text/plain, Size: 3071 bytes --]
Hello,
[dropping William Salmon and Jude Onyenegecha from Cc: as in the other
mails before]
I'd change the Subject to:
pwm: dwc: Support DWC_TIM_CTRL_PWM unset in .get_state()
On Thu, Sep 07, 2023 at 05:12:39PM +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@codethink.co.uk>
> ---
> v9:
> - fixed multi-line comment
> - put authour back to codethink email from sifive
> v8:
> - fixed rename issues
> v4:
> - fixed review comment on mulit-line calculations
> ---
> drivers/pwm/pwm-dwc-core.c | 30 +++++++++++++++++++-----------
> 1 file changed, 19 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/pwm/pwm-dwc-core.c b/drivers/pwm/pwm-dwc-core.c
> index 4b4b7b9e1d82..3fc281a78c9a 100644
> --- a/drivers/pwm/pwm-dwc-core.c
> +++ b/drivers/pwm/pwm-dwc-core.c
> @@ -122,24 +122,32 @@ 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);
The change looks right,
Reviewed-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Do you intend to address the review feedback for the other patches in
this series? It would be sad if you efforts didn't result in these
improvements getting in.
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 v9 3/6] pwm: dwc: add PWM bit unset in get_state call
2023-09-22 17:35 ` Uwe Kleine-König
@ 2023-09-25 7:02 ` Ben Dooks
0 siblings, 0 replies; 14+ messages in thread
From: Ben Dooks @ 2023-09-25 7:02 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: linux-pwm, devicetree, linux-kernel, Thierry Reding,
Krzysztof Kozlowski, Greentime Hu, jarkko.nikula
On 22/09/2023 18:35, Uwe Kleine-König wrote:
> Hello,
>
> [dropping William Salmon and Jude Onyenegecha from Cc: as in the other
> mails before]
>
> I'd change the Subject to:
>
> pwm: dwc: Support DWC_TIM_CTRL_PWM unset in .get_state()
>
> On Thu, Sep 07, 2023 at 05:12:39PM +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@codethink.co.uk>
>> ---
>> v9:
>> - fixed multi-line comment
>> - put authour back to codethink email from sifive
>> v8:
>> - fixed rename issues
>> v4:
>> - fixed review comment on mulit-line calculations
>> ---
>> drivers/pwm/pwm-dwc-core.c | 30 +++++++++++++++++++-----------
>> 1 file changed, 19 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/pwm/pwm-dwc-core.c b/drivers/pwm/pwm-dwc-core.c
>> index 4b4b7b9e1d82..3fc281a78c9a 100644
>> --- a/drivers/pwm/pwm-dwc-core.c
>> +++ b/drivers/pwm/pwm-dwc-core.c
>> @@ -122,24 +122,32 @@ 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);
>
> The change looks right,
>
> Reviewed-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
>
> Do you intend to address the review feedback for the other patches in
> this series? It would be sad if you efforts didn't result in these
> improvements getting in.
I'm going to try and get through the review comments this week,
I've been ill and then on leave so not had any time to look at
this.
--
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
* [PATCH v9 4/6] pwm: dwc: use clock rate in hz to avoid rounding issues
2023-09-07 16:12 [PATCH v9 0/6] designware pwm driver updates Ben Dooks
` (2 preceding siblings ...)
2023-09-07 16:12 ` [PATCH v9 3/6] pwm: dwc: add PWM bit unset in get_state call Ben Dooks
@ 2023-09-07 16:12 ` Ben Dooks
2023-09-07 21:34 ` Uwe Kleine-König
2023-09-07 16:12 ` [PATCH v9 5/6] pwm: dwc: round rate divisions up Ben Dooks
2023-09-07 16:12 ` [PATCH v9 6/6] pwm: dwc: add of/platform support Ben Dooks
5 siblings, 1 reply; 14+ messages in thread
From: Ben Dooks @ 2023-09-07 16:12 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
As noted, the clock-rate when not a nice multiple of ns is probably
going to end up with inacurate calculations, 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@codethink.co.uk>
Reported-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
v9:
- fixed commit spelling
- changed to use codethink email instead of sifive
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 3fc281a78c9a..3b856685029d 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));
@@ -137,17 +141,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);
@@ -168,7 +174,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.40.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH v9 4/6] pwm: dwc: use clock rate in hz to avoid rounding issues
2023-09-07 16:12 ` [PATCH v9 4/6] pwm: dwc: use clock rate in hz to avoid rounding issues Ben Dooks
@ 2023-09-07 21:34 ` Uwe Kleine-König
2023-09-11 7:33 ` Ben Dooks
0 siblings, 1 reply; 14+ messages in thread
From: Uwe Kleine-König @ 2023-09-07 21:34 UTC (permalink / raw)
To: Ben Dooks
Cc: linux-pwm, devicetree, linux-kernel, Thierry Reding,
Krzysztof Kozlowski, Greentime Hu, jarkko.nikula
[-- Attachment #1: Type: text/plain, Size: 4989 bytes --]
Hello,
[Dropped William Salmon and Jude Onyenegecha from the list of recipents,
their email addresses don't seem to work any more.]
On Thu, Sep 07, 2023 at 05:12:40PM +0100, Ben Dooks wrote:
> As noted, the clock-rate when not a nice multiple of ns is probably
> going to end up with inacurate calculations, 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@codethink.co.uk>
> Reported-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
> v9:
> - fixed commit spelling
> - changed to use codethink email instead of sifive
> 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 3fc281a78c9a..3b856685029d 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;
This might overflow. You can prevent this by asserting that clk_rate is
<= NSEC_PER_SEC and using mul_u64_u64_div_u64.
> + 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));
> @@ -137,17 +141,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;
A comment that/why this cannot overflow would be nice. (I didn't check,
maybe it can?)
> + state->period = DIV_ROUND_CLOSEST_ULL(period, clk_rate);
> + state->duty_cycle = DIV_ROUND_CLOSEST_ULL(duty, clk_rate);
Without having thought deeply about this, I think you need to round up
here. Hmm, but given that .apply() uses round-closest, too, this needs
to be addressed separately.
(The ugly thing about round-closest is that .apply(mypwm,
.get_state(mypwm)) isn't idempotent in general. Consider a PWM that can
implement period = 41.7ns and period = 42.4 ns. If it's configured with
42.4, .get_state will return period = 42. Reapplying this will configure
for 41.7ns. This won't happen with the PCI clkrate, but it might in the
of case. Another reason to use rounding-down in .apply is that
mul_u64_u64_div_u64 doesn't have a round-nearest variant.)
> state->polarity = PWM_POLARITY_INVERSED;
> - state->period = period;
> - state->duty_cycle = duty;
>
> pm_runtime_put_sync(chip->dev);
>
> @@ -168,7 +174,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;
Given that clk_ns was only introduced in patch #2 I think it would be
cleaner to squash these two patches together.
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 v9 4/6] pwm: dwc: use clock rate in hz to avoid rounding issues
2023-09-07 21:34 ` Uwe Kleine-König
@ 2023-09-11 7:33 ` Ben Dooks
2023-09-11 19:47 ` Uwe Kleine-König
0 siblings, 1 reply; 14+ messages in thread
From: Ben Dooks @ 2023-09-11 7:33 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: linux-pwm, devicetree, linux-kernel, Thierry Reding,
Krzysztof Kozlowski, Greentime Hu, jarkko.nikula
On 07/09/2023 22:34, Uwe Kleine-König wrote:
> Hello,
>
> [Dropped William Salmon and Jude Onyenegecha from the list of recipents,
> their email addresses don't seem to work any more.]
>
> On Thu, Sep 07, 2023 at 05:12:40PM +0100, Ben Dooks wrote:
>> As noted, the clock-rate when not a nice multiple of ns is probably
>> going to end up with inacurate calculations, 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@codethink.co.uk>
>> Reported-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
>> ---
>> v9:
>> - fixed commit spelling
>> - changed to use codethink email instead of sifive
>> 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 3fc281a78c9a..3b856685029d 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;
>
> This might overflow. You can prevent this by asserting that clk_rate is
> <= NSEC_PER_SEC and using mul_u64_u64_div_u64.
>
>> + 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));
>> @@ -137,17 +141,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;
>
> A comment that/why this cannot overflow would be nice. (I didn't check,
> maybe it can?)
I /think/ that as long as NSEC_PER_SEC 2^32 then this shouldn't
overflow.
>
>> + state->period = DIV_ROUND_CLOSEST_ULL(period, clk_rate);
>> + state->duty_cycle = DIV_ROUND_CLOSEST_ULL(duty, clk_rate);
>
> Without having thought deeply about this, I think you need to round up
> here. Hmm, but given that .apply() uses round-closest, too, this needs
> to be addressed separately.
>
> (The ugly thing about round-closest is that .apply(mypwm,
> .get_state(mypwm)) isn't idempotent in general. Consider a PWM that can
> implement period = 41.7ns and period = 42.4 ns. If it's configured with
> 42.4, .get_state will return period = 42. Reapplying this will configure
> for 41.7ns. This won't happen with the PCI clkrate, but it might in the
> of case. Another reason to use rounding-down in .apply is that
> mul_u64_u64_div_u64 doesn't have a round-nearest variant.)
>
>> state->polarity = PWM_POLARITY_INVERSED;
>> - state->period = period;
>> - state->duty_cycle = duty;
>>
>> pm_runtime_put_sync(chip->dev);
>>
>> @@ -168,7 +174,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;
>
> Given that clk_ns was only introduced in patch #2 I think it would be
> cleaner to squash these two patches together.
I'll have a look at how much work re-ordering the patches would
be.
--
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 v9 4/6] pwm: dwc: use clock rate in hz to avoid rounding issues
2023-09-11 7:33 ` Ben Dooks
@ 2023-09-11 19:47 ` Uwe Kleine-König
0 siblings, 0 replies; 14+ messages in thread
From: Uwe Kleine-König @ 2023-09-11 19:47 UTC (permalink / raw)
To: Ben Dooks
Cc: linux-pwm, devicetree, linux-kernel, Thierry Reding,
Krzysztof Kozlowski, Greentime Hu, jarkko.nikula
[-- Attachment #1: Type: text/plain, Size: 697 bytes --]
Hello Ben,
On Mon, Sep 11, 2023 at 08:33:02AM +0100, Ben Dooks wrote:
> On 07/09/2023 22:34, Uwe Kleine-König wrote:
> > > + duty *= NSEC_PER_SEC;
> > > + period *= NSEC_PER_SEC;
> >
> > A comment that/why this cannot overflow would be nice. (I didn't check,
> > maybe it can?)
>
> I /think/ that as long as NSEC_PER_SEC 2^32 then this shouldn't
> overflow.
I guess NSEC_PER_SEC won't change in the near future :-)
double checking and writing the result down in a comment would be
appreciated.
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: 484 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v9 5/6] pwm: dwc: round rate divisions up
2023-09-07 16:12 [PATCH v9 0/6] designware pwm driver updates Ben Dooks
` (3 preceding siblings ...)
2023-09-07 16:12 ` [PATCH v9 4/6] pwm: dwc: use clock rate in hz to avoid rounding issues Ben Dooks
@ 2023-09-07 16:12 ` Ben Dooks
2023-09-07 21:36 ` Uwe Kleine-König
2023-09-07 16:12 ` [PATCH v9 6/6] pwm: dwc: add of/platform support Ben Dooks
5 siblings, 1 reply; 14+ messages in thread
From: Ben Dooks @ 2023-09-07 16:12 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
As suggested, round up the counter variables to ensure we
always produce a longer period calculation.
Reported-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
---
drivers/pwm/pwm-dwc-core.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/pwm/pwm-dwc-core.c b/drivers/pwm/pwm-dwc-core.c
index 3b856685029d..6358e3345210 100644
--- a/drivers/pwm/pwm-dwc-core.c
+++ b/drivers/pwm/pwm-dwc-core.c
@@ -50,13 +50,13 @@ static int __dwc_pwm_configure_timer(struct dwc_pwm *dwc,
* 2^32 periods.
*/
tmp = state->duty_cycle * dwc->clk_rate;
- tmp = DIV_ROUND_CLOSEST_ULL(tmp, NSEC_PER_SEC);
+ tmp = DIV_ROUND_UP_ULL(tmp, NSEC_PER_SEC);
if (tmp < 1 || tmp > (1ULL << 32))
return -ERANGE;
low = tmp - 1;
tmp = (state->period - state->duty_cycle) * dwc->clk_rate;
- tmp = DIV_ROUND_CLOSEST_ULL(tmp, NSEC_PER_SEC);
+ tmp = DIV_ROUND_UP_ULL(tmp, NSEC_PER_SEC);
if (tmp < 1 || tmp > (1ULL << 32))
return -ERANGE;
high = tmp - 1;
--
2.40.1
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH v9 5/6] pwm: dwc: round rate divisions up
2023-09-07 16:12 ` [PATCH v9 5/6] pwm: dwc: round rate divisions up Ben Dooks
@ 2023-09-07 21:36 ` Uwe Kleine-König
0 siblings, 0 replies; 14+ messages in thread
From: Uwe Kleine-König @ 2023-09-07 21:36 UTC (permalink / raw)
To: Ben Dooks
Cc: linux-pwm, devicetree, linux-kernel, Thierry Reding,
Krzysztof Kozlowski, Greentime Hu, jarkko.nikula
[-- Attachment #1: Type: text/plain, Size: 1609 bytes --]
Hello,
[again dropped William Salmon and Jude Onyenegecha from Cc:]
On Thu, Sep 07, 2023 at 05:12:41PM +0100, Ben Dooks wrote:
> As suggested, round up the counter variables to ensure we
> always produce a longer period calculation.
>
> Reported-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
> ---
> drivers/pwm/pwm-dwc-core.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pwm/pwm-dwc-core.c b/drivers/pwm/pwm-dwc-core.c
> index 3b856685029d..6358e3345210 100644
> --- a/drivers/pwm/pwm-dwc-core.c
> +++ b/drivers/pwm/pwm-dwc-core.c
> @@ -50,13 +50,13 @@ static int __dwc_pwm_configure_timer(struct dwc_pwm *dwc,
> * 2^32 periods.
> */
> tmp = state->duty_cycle * dwc->clk_rate;
> - tmp = DIV_ROUND_CLOSEST_ULL(tmp, NSEC_PER_SEC);
> + tmp = DIV_ROUND_UP_ULL(tmp, NSEC_PER_SEC);
> if (tmp < 1 || tmp > (1ULL << 32))
> return -ERANGE;
> low = tmp - 1;
>
> tmp = (state->period - state->duty_cycle) * dwc->clk_rate;
> - tmp = DIV_ROUND_CLOSEST_ULL(tmp, NSEC_PER_SEC);
> + tmp = DIV_ROUND_UP_ULL(tmp, NSEC_PER_SEC);
> if (tmp < 1 || tmp > (1ULL << 32))
> return -ERANGE;
> high = tmp - 1;
Ah, I asked for that in the reply I just sent out to patch #4. Maybe move
this before the change from #4?! I think .get_state needs to be adapted
accoringly (to round up).
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 v9 6/6] pwm: dwc: add of/platform support
2023-09-07 16:12 [PATCH v9 0/6] designware pwm driver updates Ben Dooks
` (4 preceding siblings ...)
2023-09-07 16:12 ` [PATCH v9 5/6] pwm: dwc: round rate divisions up Ben Dooks
@ 2023-09-07 16:12 ` Ben Dooks
5 siblings, 0 replies; 14+ messages in thread
From: Ben Dooks @ 2023-09-07 16:12 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
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@codethink.co.uk>
---
v9:
- add clk_rate_exclusive_get() to lock clock rate for now
- fix email address from sifive to codethink
- fix module namespace
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 | 93 ++++++++++++++++++++++++++++++++++++++
drivers/pwm/pwm-dwc.c | 1 +
drivers/pwm/pwm-dwc.h | 1 +
6 files changed, 112 insertions(+)
create mode 100644 drivers/pwm/pwm-dwc-of.c
diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
index 507c8b8547a5..22d58cb334e7 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 c5ec9e168ee7..e3c867cb13d1 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 6358e3345210..8b4d96d655c7 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..30b860484895
--- /dev/null
+++ b/drivers/pwm/pwm-dwc-of.c
@@ -0,0 +1,93 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * DesignWare PWM Controller driver OF
+ *
+ * Copyright (C) 2022 SiFive, Inc.
+ */
+
+#define DEFAULT_MODULE_NAMESPACE 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;
+ int ret;
+
+ 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");
+
+ ret = clk_rate_exclusive_get(dwc->clk);
+ if (ret)
+ return dev_err_probe(dev, ret,
+ "clk_rate_exclusive_get() failed\n");
+
+ dwc->clk_rate = clk_get_rate(dwc->clk);
+ return devm_pwmchip_add(dev, &dwc->chip);
+}
+
+static int dwc_pwm_plat_remove(struct platform_device *pdev)
+{
+ struct dwc_pwm *dwc = dev_get_drvdata(&pdev->dev);
+
+ clk_rate_exclusive_put(dwc->clk);
+ return 0;
+}
+
+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,
+ .remove = dwc_pwm_plat_remove,
+};
+
+module_platform_driver(dwc_pwm_plat_driver);
+
+MODULE_ALIAS("platform:dwc-pwm-of");
+MODULE_AUTHOR("Ben Dooks <ben.dooks@codethink.co.uk>");
+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.40.1
^ permalink raw reply related [flat|nested] 14+ messages in thread