This is an updated version of the Designware PWM driver updates
for OF support, which now splits the driver into PCI and OF parts
as well as tries to sort out the review comments.
Hopefully this can now be queued for the next kernel version.
v6:
- fix removal ordering of DWC_PERIOD_NS
v5:
- fixed kconfig string error
- merged pwm-nr into main of code
- split of code from pci code
- updated pwm-nr capping
- fix duplicate error reporting in of-code
- fix return in of-probe
- remove unecessary remove function as devm_ functions sort this
- fixed ordering of properties
- added missing reg item
- fixed missing split of the two clock sources.
- get bus clock in of code
v4:
- split pci and of into new modules
- fixup review comments
- fix typos in dt-bindings
v3:
- change the compatible name
- squash down pwm count patch
- fixup patch naming
v2:
- fix #pwm-cells count to be 3
- fix indetation
- merge the two clock patches
- add HAS_IOMEM as a config dependency
Ben Dooks (10):
dt-bindings: pwm: Document Synopsys DesignWare
snps,pwm-dw-apb-timers-pwm2
pwm: dwc: allow driver to be built with COMPILE_TEST
pwm: dwc: change &pci->dev to dev in probe
pwm: dwc: move memory alloc to own function
pwm: dwc: use devm_pwmchip_add
pwm: dwc: split pci out of core driver
pwm: dwc: make timer clock configurable
pwm: dwc: add of/platform support
pwm: dwc: add PWM bit unset in get_state call
pwm: dwc: use clock rate in hz to avoid rounding issues
.../bindings/pwm/snps,dw-apb-timers-pwm2.yaml | 68 ++++++
drivers/pwm/Kconfig | 24 +-
drivers/pwm/Makefile | 2 +
drivers/pwm/pwm-dwc-of.c | 76 +++++++
drivers/pwm/pwm-dwc-pci.c | 134 +++++++++++
drivers/pwm/pwm-dwc.c | 210 ++++--------------
drivers/pwm/pwm-dwc.h | 59 +++++
7 files changed, 402 insertions(+), 171 deletions(-)
create mode 100644 Documentation/devicetree/bindings/pwm/snps,dw-apb-timers-pwm2.yaml
create mode 100644 drivers/pwm/pwm-dwc-of.c
create mode 100644 drivers/pwm/pwm-dwc-pci.c
create mode 100644 drivers/pwm/pwm-dwc.h
--
2.35.1
Use devm_pwmchip_add() to add the pwm chip to avoid having to manually
remove it (useful for the next patch which adds the platform-device
support).
Signed-off-by: Ben Dooks <[email protected]>
---
drivers/pwm/pwm-dwc.c | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/drivers/pwm/pwm-dwc.c b/drivers/pwm/pwm-dwc.c
index 61f11e0a9319..56cde9da2c0e 100644
--- a/drivers/pwm/pwm-dwc.c
+++ b/drivers/pwm/pwm-dwc.c
@@ -242,7 +242,7 @@ static int dwc_pwm_probe(struct pci_dev *pci, const struct pci_device_id *id)
return -ENOMEM;
}
- ret = pwmchip_add(&dwc->chip);
+ ret = devm_pwmchip_add(dev, &dwc->chip);
if (ret)
return ret;
@@ -254,12 +254,8 @@ static int dwc_pwm_probe(struct pci_dev *pci, const struct pci_device_id *id)
static void dwc_pwm_remove(struct pci_dev *pci)
{
- struct dwc_pwm *dwc = pci_get_drvdata(pci);
-
pm_runtime_forbid(&pci->dev);
pm_runtime_get_noresume(&pci->dev);
-
- pwmchip_remove(&dwc->chip);
}
#ifdef CONFIG_PM_SLEEP
--
2.35.1
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 <[email protected]>
Reported-by: Uwe Kleine-König <[email protected]>
---
drivers/pwm/pwm-dwc-of.c | 2 +-
drivers/pwm/pwm-dwc.c | 29 ++++++++++++++++++++---------
drivers/pwm/pwm-dwc.h | 2 +-
3 files changed, 22 insertions(+), 11 deletions(-)
diff --git a/drivers/pwm/pwm-dwc-of.c b/drivers/pwm/pwm-dwc-of.c
index c5b4351cc7b0..5f7f066859d4 100644
--- a/drivers/pwm/pwm-dwc-of.c
+++ b/drivers/pwm/pwm-dwc-of.c
@@ -50,7 +50,7 @@ static int dwc_pwm_plat_probe(struct platform_device *pdev)
return dev_err_probe(dev, PTR_ERR(dwc->clk),
"failed to get timer clock\n");
- dwc->clk_ns = NSEC_PER_SEC / clk_get_rate(dwc->clk);
+ dwc->clk_rate = clk_get_rate(dwc->clk);
return devm_pwmchip_add(dev, &dwc->chip);
}
diff --git a/drivers/pwm/pwm-dwc.c b/drivers/pwm/pwm-dwc.c
index 5ef0fe7ea3e9..f48a6245a3b5 100644
--- a/drivers/pwm/pwm-dwc.c
+++ b/drivers/pwm/pwm-dwc.c
@@ -43,18 +43,22 @@ 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
* 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;
@@ -120,6 +124,7 @@ static void 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;
@@ -129,22 +134,28 @@ static void dwc_pwm_get_state(struct pwm_chip *chip, struct pwm_device *pwm,
ld = dwc_pwm_readl(dwc, DWC_TIM_LD_CNT(pwm->hwpwm));
ld2 = dwc_pwm_readl(dwc, DWC_TIM_LD_CNT2(pwm->hwpwm));
+ if (dwc->clk)
+ dwc->clk_rate = clk_get_rate(dwc->clk);
+
+ clk_rate = dwc->clk_rate;
state->enabled = !!(ctrl & DWC_TIM_CTRL_EN);
/* 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;
+ duty = ld + 1;
+ period = ld2 + 1;
period += duty;
} else {
- duty = (ld + 1) * dwc->clk_ns;
+ duty = ld + 1;
period = duty * 2;
}
- state->period = period;
- state->duty_cycle = duty;
+ 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;
pm_runtime_put_sync(chip->dev);
@@ -164,7 +175,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 dc451cb2eff5..19bdc2224690 100644
--- a/drivers/pwm/pwm-dwc.h
+++ b/drivers/pwm/pwm-dwc.h
@@ -41,7 +41,7 @@ struct dwc_pwm {
struct pwm_chip chip;
void __iomem *base;
struct clk *clk;
- 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.35.1
Add documentation for the bindings for Synopsys' DesignWare PWM block
as we will be adding DT/platform support to the Linux driver soon.
Signed-off-by: Ben Dooks <[email protected]>
---
v5:
- fixed order of properties
- corrected clock to two items
v4:
- fixed typos, added reg
v3:
- add description and example
- merge the snps,pwm-number into this patch
- rename snps,pwm to snps,dw-apb-timers-pwm2
v2:
- fix #pwm-cells to be 3
- fix indentation and ordering issues
---
.../bindings/pwm/snps,dw-apb-timers-pwm2.yaml | 68 +++++++++++++++++++
1 file changed, 68 insertions(+)
create mode 100644 Documentation/devicetree/bindings/pwm/snps,dw-apb-timers-pwm2.yaml
diff --git a/Documentation/devicetree/bindings/pwm/snps,dw-apb-timers-pwm2.yaml b/Documentation/devicetree/bindings/pwm/snps,dw-apb-timers-pwm2.yaml
new file mode 100644
index 000000000000..9aabdb373afa
--- /dev/null
+++ b/Documentation/devicetree/bindings/pwm/snps,dw-apb-timers-pwm2.yaml
@@ -0,0 +1,68 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+# Copyright (C) 2022 SiFive, Inc.
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pwm/snps,dw-apb-timers-pwm2.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Synopsys DW-APB timers PWM controller
+
+maintainers:
+ - Ben Dooks <[email protected]>
+
+description:
+ This describes the DesignWare APB timers module when used in the PWM
+ mode. The IP core can be generated with various options which can
+ control the functionality, the number of PWMs available and other
+ internal controls the designer requires.
+
+ The IP block has a version register so this can be used for detection
+ instead of having to encode the IP version number in the device tree
+ comaptible.
+
+allOf:
+ - $ref: pwm.yaml#
+
+properties:
+ compatible:
+ const: snps,dw-apb-timers-pwm2
+
+ reg:
+ maxItems: 1
+
+ "#pwm-cells":
+ const: 3
+
+ clocks:
+ items:
+ - description: Interface bus clock
+ - description: PWM reference clock
+
+ clock-names:
+ items:
+ - const: bus
+ - const: timer
+
+ snps,pwm-number:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description: The number of PWM channels configured for this instance
+ enum: [1, 2, 3, 4, 5, 6, 7, 8]
+
+required:
+ - compatible
+ - reg
+ - "#pwm-cells"
+ - clocks
+ - clock-names
+
+additionalProperties: false
+
+examples:
+ - |
+ pwm: pwm@180000 {
+ compatible = "snps,dw-apb-timers-pwm2";
+ reg = <0x180000 0x200>;
+ #pwm-cells = <3>;
+ clocks = <&bus>, <&timer>;
+ clock-names = "bus", "timer";
+ };
--
2.35.1
Allow dwc driver to be built with COMPILE_TEST should allow
better coverage when build testing.
Signed-off-by: Ben Dooks <[email protected]>
---
v4:
- moved to earlier in the series
v3:
- add HAS_IOMEM depdency for compile testing
---
drivers/pwm/Kconfig | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
index 60d13a949bc5..3f3c53af4a56 100644
--- a/drivers/pwm/Kconfig
+++ b/drivers/pwm/Kconfig
@@ -176,7 +176,8 @@ config PWM_CROS_EC
config PWM_DWC
tristate "DesignWare PWM Controller"
- depends on PCI
+ depends on PCI || COMPILE_TEST
+ depends on HAS_IOMEM
help
PWM driver for Synopsys DWC PWM Controller attached to a PCI bus.
--
2.35.1
In preparation for adding other bus support, move the allocation
of the pwm struct out of the main driver code.
Signed-off-by: Ben Dooks <[email protected]>
---
drivers/pwm/pwm-dwc.c | 24 +++++++++++++++++-------
1 file changed, 17 insertions(+), 7 deletions(-)
diff --git a/drivers/pwm/pwm-dwc.c b/drivers/pwm/pwm-dwc.c
index c706ef9a7ba1..61f11e0a9319 100644
--- a/drivers/pwm/pwm-dwc.c
+++ b/drivers/pwm/pwm-dwc.c
@@ -196,13 +196,29 @@ static const struct pwm_ops dwc_pwm_ops = {
.owner = THIS_MODULE,
};
+static struct dwc_pwm *dwc_pwm_alloc(struct device *dev)
+{
+ struct dwc_pwm *dwc;
+
+ 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;
+}
+
static int dwc_pwm_probe(struct pci_dev *pci, const struct pci_device_id *id)
{
struct device *dev = &pci->dev;
struct dwc_pwm *dwc;
int ret;
- dwc = devm_kzalloc(dev, sizeof(*dwc), GFP_KERNEL);
+ dwc = dwc_pwm_alloc(dev);
if (!dwc)
return -ENOMEM;
@@ -226,12 +242,6 @@ static int dwc_pwm_probe(struct pci_dev *pci, const struct pci_device_id *id)
return -ENOMEM;
}
- pci_set_drvdata(pci, dwc);
-
- dwc->chip.dev = dev;
- dwc->chip.ops = &dwc_pwm_ops;
- dwc->chip.npwm = DWC_TIMERS_TOTAL;
-
ret = pwmchip_add(&dwc->chip);
if (ret)
return ret;
--
2.35.1
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 <[email protected]>
---
v4:
- fixed review comment on mulit-line calculations
---
drivers/pwm/pwm-dwc.c | 29 ++++++++++++++++++-----------
1 file changed, 18 insertions(+), 11 deletions(-)
diff --git a/drivers/pwm/pwm-dwc.c b/drivers/pwm/pwm-dwc.c
index 1251620ab771..5ef0fe7ea3e9 100644
--- a/drivers/pwm/pwm-dwc.c
+++ b/drivers/pwm/pwm-dwc.c
@@ -121,23 +121,30 @@ static void 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->period = period;
+ state->duty_cycle = duty;
state->polarity = PWM_POLARITY_INVERSED;
pm_runtime_put_sync(chip->dev);
--
2.35.1
On 20/10/2022 11:16, Ben Dooks wrote:
> Add documentation for the bindings for Synopsys' DesignWare PWM block
> as we will be adding DT/platform support to the Linux driver soon.
>
> Signed-off-by: Ben Dooks <[email protected]>
> ---
> v5:
> - fixed order of properties
> - corrected clock to two items
This is a friendly reminder during the review process.
It looks like you received a tag and forgot to add it.
If you do not know the process, here is a short explanation:
Please add Acked-by/Reviewed-by/Tested-by tags when posting new
versions. However,
there's no need to repost patches *only* to add the tags. The upstream
maintainer will do that for acks received on the version they apply.
https://elixir.bootlin.com/linux/v5.17/source/Documentation/process/submitting-patches.rst#L540
If a tag was not added on purpose, please state why and what changed.
Reviewed-by: Krzysztof Kozlowski <[email protected]>
Best regards,
Krzysztof
On Thu, Oct 20, 2022 at 04:16:05PM +0100, Ben Dooks wrote:
> Use devm_pwmchip_add() to add the pwm chip to avoid having to manually
> remove it (useful for the next patch which adds the platform-device
> support).
>
> Signed-off-by: Ben Dooks <[email protected]>
Reviewed-by: Uwe Kleine-K?nig <[email protected]>
Thanks
Uwe
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | https://www.pengutronix.de/ |
The dwc_pwm_probe() assignes dev to be &pci->dev but then uses
&pci->dev throughout the function. Change these all to the be
'dev' variable to make lines shorter.
Signed-off-by: Ben Dooks <[email protected]>
Acked-by: Uwe Kleine-König <[email protected]>
---
drivers/pwm/pwm-dwc.c | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/drivers/pwm/pwm-dwc.c b/drivers/pwm/pwm-dwc.c
index 7568300bb11e..c706ef9a7ba1 100644
--- a/drivers/pwm/pwm-dwc.c
+++ b/drivers/pwm/pwm-dwc.c
@@ -202,14 +202,13 @@ static int dwc_pwm_probe(struct pci_dev *pci, const struct pci_device_id *id)
struct dwc_pwm *dwc;
int ret;
- dwc = devm_kzalloc(&pci->dev, sizeof(*dwc), GFP_KERNEL);
+ dwc = devm_kzalloc(dev, sizeof(*dwc), GFP_KERNEL);
if (!dwc)
return -ENOMEM;
ret = pcim_enable_device(pci);
if (ret) {
- dev_err(&pci->dev,
- "Failed to enable device (%pe)\n", ERR_PTR(ret));
+ dev_err(dev, "Failed to enable device (%pe)\n", ERR_PTR(ret));
return ret;
}
@@ -217,14 +216,13 @@ static int dwc_pwm_probe(struct pci_dev *pci, const struct pci_device_id *id)
ret = pcim_iomap_regions(pci, BIT(0), pci_name(pci));
if (ret) {
- dev_err(&pci->dev,
- "Failed to iomap PCI BAR (%pe)\n", ERR_PTR(ret));
+ dev_err(dev, "Failed to iomap PCI BAR (%pe)\n", ERR_PTR(ret));
return ret;
}
dwc->base = pcim_iomap_table(pci)[0];
if (!dwc->base) {
- dev_err(&pci->dev, "Base address missing\n");
+ dev_err(dev, "Base address missing\n");
return -ENOMEM;
}
--
2.35.1
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 <[email protected]>
---
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-pci.c | 1 +
drivers/pwm/pwm-dwc.c | 10 ++++++----
drivers/pwm/pwm-dwc.h | 3 ++-
3 files changed, 9 insertions(+), 5 deletions(-)
diff --git a/drivers/pwm/pwm-dwc-pci.c b/drivers/pwm/pwm-dwc-pci.c
index 2213d0e7f3c8..949423e368f9 100644
--- a/drivers/pwm/pwm-dwc-pci.c
+++ b/drivers/pwm/pwm-dwc-pci.c
@@ -20,6 +20,7 @@
#include <linux/pci.h>
#include <linux/pm_runtime.h>
#include <linux/pwm.h>
+#include <linux/clk.h>
#include "pwm-dwc.h"
diff --git a/drivers/pwm/pwm-dwc.c b/drivers/pwm/pwm-dwc.c
index 90a8ae1252a1..1251620ab771 100644
--- a/drivers/pwm/pwm-dwc.c
+++ b/drivers/pwm/pwm-dwc.c
@@ -14,6 +14,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>
@@ -47,13 +48,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;
@@ -128,12 +129,12 @@ static void 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;
@@ -156,6 +157,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 68f98eb76152..dc451cb2eff5 100644
--- a/drivers/pwm/pwm-dwc.h
+++ b/drivers/pwm/pwm-dwc.h
@@ -22,7 +22,6 @@
#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)
@@ -41,6 +40,8 @@ struct dwc_pwm_ctx {
struct dwc_pwm {
struct pwm_chip chip;
void __iomem *base;
+ struct clk *clk;
+ 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.35.1
On Thu, Oct 20, 2022 at 04:16:02PM +0100, Ben Dooks wrote:
> Allow dwc driver to be built with COMPILE_TEST should allow
> better coverage when build testing.
>
> Signed-off-by: Ben Dooks <[email protected]>
Acked-by: Uwe Kleine-K?nig <[email protected]>
(forwarded from v4)
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | https://www.pengutronix.de/ |
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 <[email protected]>
---
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-pci.c | 133 ++++++++++++++++++++++++++++++++
drivers/pwm/pwm-dwc.c | 158 +-------------------------------------
drivers/pwm/pwm-dwc.h | 58 ++++++++++++++
5 files changed, 207 insertions(+), 157 deletions(-)
create mode 100644 drivers/pwm/pwm-dwc-pci.c
create mode 100644 drivers/pwm/pwm-dwc.h
diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
index 3f3c53af4a56..a9f1c554db2b 100644
--- a/drivers/pwm/Kconfig
+++ b/drivers/pwm/Kconfig
@@ -175,15 +175,23 @@ config PWM_CROS_EC
Controller.
config PWM_DWC
- tristate "DesignWare PWM Controller"
- depends on PCI || COMPILE_TEST
+ tristate "DesignWare PWM Controller core"
depends on HAS_IOMEM
help
- PWM driver for Synopsys DWC PWM Controller attached to a PCI bus.
+ PWM driver for Synopsys DWC PWM Controller.
To compile this driver as a module, choose M here: the module
will be called pwm-dwc.
+config PWM_DWC_PCI
+ tristate "DesignWare PWM Controller core"
+ depends on PWM_DWC && HAS_IOMEM && PCI
+ help
+ PWM driver for Synopsys DWC PWM Controller attached to a PCI bus.
+
+ To compile this driver as a module, choose M here: the module
+ will be called pwm-dwc-pci.
+
config PWM_EP93XX
tristate "Cirrus Logic EP93xx PWM support"
depends on ARCH_EP93XX || COMPILE_TEST
diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile
index 7bf1a29f02b8..a70d36623129 100644
--- a/drivers/pwm/Makefile
+++ b/drivers/pwm/Makefile
@@ -15,6 +15,7 @@ obj-$(CONFIG_PWM_CLPS711X) += pwm-clps711x.o
obj-$(CONFIG_PWM_CRC) += pwm-crc.o
obj-$(CONFIG_PWM_CROS_EC) += pwm-cros-ec.o
obj-$(CONFIG_PWM_DWC) += pwm-dwc.o
+obj-$(CONFIG_PWM_DWC_PCI) += pwm-dwc-pci.o
obj-$(CONFIG_PWM_EP93XX) += pwm-ep93xx.o
obj-$(CONFIG_PWM_FSL_FTM) += pwm-fsl-ftm.o
obj-$(CONFIG_PWM_HIBVT) += pwm-hibvt.o
diff --git a/drivers/pwm/pwm-dwc-pci.c b/drivers/pwm/pwm-dwc-pci.c
new file mode 100644
index 000000000000..2213d0e7f3c8
--- /dev/null
+++ b/drivers/pwm/pwm-dwc-pci.c
@@ -0,0 +1,133 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * DesignWare PWM Controller driver (PCI part)
+ *
+ * Copyright (C) 2018-2020 Intel Corporation
+ *
+ * Author: Felipe Balbi (Intel)
+ * Author: Jarkko Nikula <[email protected]>
+ * Author: Raymond Tan <[email protected]>
+ *
+ * Limitations:
+ * - The hardware cannot generate a 0 % or 100 % duty cycle. Both high and low
+ * periods are one or more input clock periods long.
+ */
+
+#include <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 int dwc_pwm_probe(struct pci_dev *pci, const struct pci_device_id *id)
+{
+ struct device *dev = &pci->dev;
+ struct dwc_pwm *dwc;
+ int ret;
+
+ dwc = dwc_pwm_alloc(dev);
+ if (!dwc)
+ return -ENOMEM;
+
+ ret = pcim_enable_device(pci);
+ if (ret) {
+ dev_err(dev, "Failed to enable device (%pe)\n", ERR_PTR(ret));
+ return ret;
+ }
+
+ pci_set_master(pci);
+
+ ret = pcim_iomap_regions(pci, BIT(0), pci_name(pci));
+ if (ret) {
+ dev_err(dev, "Failed to iomap PCI BAR (%pe)\n", ERR_PTR(ret));
+ return ret;
+ }
+
+ dwc->base = pcim_iomap_table(pci)[0];
+ if (!dwc->base) {
+ dev_err(dev, "Base address missing\n");
+ return -ENOMEM;
+ }
+
+ ret = devm_pwmchip_add(dev, &dwc->chip);
+ if (ret)
+ return ret;
+
+ pm_runtime_put(dev);
+ pm_runtime_allow(dev);
+
+ return 0;
+}
+
+static void dwc_pwm_remove(struct pci_dev *pci)
+{
+ pm_runtime_forbid(&pci->dev);
+ pm_runtime_get_noresume(&pci->dev);
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int dwc_pwm_suspend(struct device *dev)
+{
+ struct pci_dev *pdev = container_of(dev, struct pci_dev, dev);
+ struct dwc_pwm *dwc = pci_get_drvdata(pdev);
+ int i;
+
+ for (i = 0; i < DWC_TIMERS_TOTAL; i++) {
+ if (dwc->chip.pwms[i].state.enabled) {
+ dev_err(dev, "PWM %u in use by consumer (%s)\n",
+ i, dwc->chip.pwms[i].label);
+ return -EBUSY;
+ }
+ dwc->ctx[i].cnt = dwc_pwm_readl(dwc, DWC_TIM_LD_CNT(i));
+ dwc->ctx[i].cnt2 = dwc_pwm_readl(dwc, DWC_TIM_LD_CNT2(i));
+ dwc->ctx[i].ctrl = dwc_pwm_readl(dwc, DWC_TIM_CTRL(i));
+ }
+
+ return 0;
+}
+
+static int dwc_pwm_resume(struct device *dev)
+{
+ struct pci_dev *pdev = container_of(dev, struct pci_dev, dev);
+ struct dwc_pwm *dwc = pci_get_drvdata(pdev);
+ int i;
+
+ for (i = 0; i < DWC_TIMERS_TOTAL; i++) {
+ dwc_pwm_writel(dwc, dwc->ctx[i].cnt, DWC_TIM_LD_CNT(i));
+ dwc_pwm_writel(dwc, dwc->ctx[i].cnt2, DWC_TIM_LD_CNT2(i));
+ dwc_pwm_writel(dwc, dwc->ctx[i].ctrl, DWC_TIM_CTRL(i));
+ }
+
+ return 0;
+}
+#endif
+
+static SIMPLE_DEV_PM_OPS(dwc_pwm_pm_ops, dwc_pwm_suspend, dwc_pwm_resume);
+
+static const struct pci_device_id dwc_pwm_id_table[] = {
+ { PCI_VDEVICE(INTEL, 0x4bb7) }, /* Elkhart Lake */
+ { } /* Terminating Entry */
+};
+MODULE_DEVICE_TABLE(pci, dwc_pwm_id_table);
+
+static struct pci_driver dwc_pwm_driver = {
+ .name = "pwm-dwc",
+ .probe = dwc_pwm_probe,
+ .remove = dwc_pwm_remove,
+ .id_table = dwc_pwm_id_table,
+ .driver = {
+ .pm = &dwc_pwm_pm_ops,
+ },
+};
+
+module_pci_driver(dwc_pwm_driver);
+
+MODULE_AUTHOR("Felipe Balbi (Intel)");
+MODULE_AUTHOR("Jarkko Nikula <[email protected]>");
+MODULE_AUTHOR("Raymond Tan <[email protected]>");
+MODULE_DESCRIPTION("DesignWare PWM Controller");
+MODULE_LICENSE("GPL");
diff --git a/drivers/pwm/pwm-dwc.c b/drivers/pwm/pwm-dwc.c
index 56cde9da2c0e..90a8ae1252a1 100644
--- a/drivers/pwm/pwm-dwc.c
+++ b/drivers/pwm/pwm-dwc.c
@@ -1,16 +1,12 @@
// SPDX-License-Identifier: GPL-2.0
/*
- * DesignWare PWM Controller driver
+ * DesignWare PWM Controller driver core
*
* Copyright (C) 2018-2020 Intel Corporation
*
* Author: Felipe Balbi (Intel)
* Author: Jarkko Nikula <[email protected]>
* Author: Raymond Tan <[email protected]>
- *
- * Limitations:
- * - The hardware cannot generate a 0 % or 100 % duty cycle. Both high and low
- * periods are one or more input clock periods long.
*/
#include <linux/bitops.h>
@@ -21,51 +17,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);
-}
+#include "pwm-dwc.h"
static void __dwc_pwm_set_enable(struct dwc_pwm *dwc, int pwm, int enabled)
{
@@ -196,7 +148,7 @@ static const struct pwm_ops dwc_pwm_ops = {
.owner = THIS_MODULE,
};
-static struct dwc_pwm *dwc_pwm_alloc(struct device *dev)
+struct dwc_pwm *dwc_pwm_alloc(struct device *dev)
{
struct dwc_pwm *dwc;
@@ -211,109 +163,7 @@ static struct dwc_pwm *dwc_pwm_alloc(struct device *dev)
dev_set_drvdata(dev, dwc);
return dwc;
}
-
-static int dwc_pwm_probe(struct pci_dev *pci, const struct pci_device_id *id)
-{
- struct device *dev = &pci->dev;
- struct dwc_pwm *dwc;
- int ret;
-
- dwc = dwc_pwm_alloc(dev);
- if (!dwc)
- return -ENOMEM;
-
- ret = pcim_enable_device(pci);
- if (ret) {
- dev_err(dev, "Failed to enable device (%pe)\n", ERR_PTR(ret));
- return ret;
- }
-
- pci_set_master(pci);
-
- ret = pcim_iomap_regions(pci, BIT(0), pci_name(pci));
- if (ret) {
- dev_err(dev, "Failed to iomap PCI BAR (%pe)\n", ERR_PTR(ret));
- return ret;
- }
-
- dwc->base = pcim_iomap_table(pci)[0];
- if (!dwc->base) {
- dev_err(dev, "Base address missing\n");
- return -ENOMEM;
- }
-
- ret = devm_pwmchip_add(dev, &dwc->chip);
- if (ret)
- return ret;
-
- pm_runtime_put(dev);
- pm_runtime_allow(dev);
-
- return 0;
-}
-
-static void dwc_pwm_remove(struct pci_dev *pci)
-{
- pm_runtime_forbid(&pci->dev);
- pm_runtime_get_noresume(&pci->dev);
-}
-
-#ifdef CONFIG_PM_SLEEP
-static int dwc_pwm_suspend(struct device *dev)
-{
- struct pci_dev *pdev = container_of(dev, struct pci_dev, dev);
- struct dwc_pwm *dwc = pci_get_drvdata(pdev);
- int i;
-
- for (i = 0; i < DWC_TIMERS_TOTAL; i++) {
- if (dwc->chip.pwms[i].state.enabled) {
- dev_err(dev, "PWM %u in use by consumer (%s)\n",
- i, dwc->chip.pwms[i].label);
- return -EBUSY;
- }
- dwc->ctx[i].cnt = dwc_pwm_readl(dwc, DWC_TIM_LD_CNT(i));
- dwc->ctx[i].cnt2 = dwc_pwm_readl(dwc, DWC_TIM_LD_CNT2(i));
- dwc->ctx[i].ctrl = dwc_pwm_readl(dwc, DWC_TIM_CTRL(i));
- }
-
- return 0;
-}
-
-static int dwc_pwm_resume(struct device *dev)
-{
- struct pci_dev *pdev = container_of(dev, struct pci_dev, dev);
- struct dwc_pwm *dwc = pci_get_drvdata(pdev);
- int i;
-
- for (i = 0; i < DWC_TIMERS_TOTAL; i++) {
- dwc_pwm_writel(dwc, dwc->ctx[i].cnt, DWC_TIM_LD_CNT(i));
- dwc_pwm_writel(dwc, dwc->ctx[i].cnt2, DWC_TIM_LD_CNT2(i));
- dwc_pwm_writel(dwc, dwc->ctx[i].ctrl, DWC_TIM_CTRL(i));
- }
-
- return 0;
-}
-#endif
-
-static SIMPLE_DEV_PM_OPS(dwc_pwm_pm_ops, dwc_pwm_suspend, dwc_pwm_resume);
-
-static const struct pci_device_id dwc_pwm_id_table[] = {
- { PCI_VDEVICE(INTEL, 0x4bb7) }, /* Elkhart Lake */
- { } /* Terminating Entry */
-};
-MODULE_DEVICE_TABLE(pci, dwc_pwm_id_table);
-
-static struct pci_driver dwc_pwm_driver = {
- .name = "pwm-dwc",
- .probe = dwc_pwm_probe,
- .remove = dwc_pwm_remove,
- .id_table = dwc_pwm_id_table,
- .driver = {
- .pm = &dwc_pwm_pm_ops,
- },
-};
-
-module_pci_driver(dwc_pwm_driver);
+EXPORT_SYMBOL_GPL(dwc_pwm_alloc);
MODULE_AUTHOR("Felipe Balbi (Intel)");
MODULE_AUTHOR("Jarkko Nikula <[email protected]>");
diff --git a/drivers/pwm/pwm-dwc.h b/drivers/pwm/pwm-dwc.h
new file mode 100644
index 000000000000..68f98eb76152
--- /dev/null
+++ b/drivers/pwm/pwm-dwc.h
@@ -0,0 +1,58 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * DesignWare PWM Controller driver
+ *
+ * Copyright (C) 2018-2020 Intel Corporation
+ *
+ * Author: Felipe Balbi (Intel)
+ * Author: Jarkko Nikula <[email protected]>
+ * Author: Raymond Tan <[email protected]>
+ */
+
+#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.35.1
The dwc pwm controller can be used in non-PCI systems, so allow
either platform or OF based probing.
Signed-off-by: Ben Dooks <[email protected]>
---
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
fixup add pwm/Kconfig
---
drivers/pwm/Kconfig | 9 +++++
drivers/pwm/Makefile | 1 +
drivers/pwm/pwm-dwc-of.c | 76 ++++++++++++++++++++++++++++++++++++++++
3 files changed, 86 insertions(+)
create mode 100644 drivers/pwm/pwm-dwc-of.c
diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
index a9f1c554db2b..c734f58a8bfc 100644
--- a/drivers/pwm/Kconfig
+++ b/drivers/pwm/Kconfig
@@ -192,6 +192,15 @@ config PWM_DWC_PCI
To compile this driver as a module, choose M here: the module
will be called pwm-dwc-pci.
+config PWM_DWC_OF
+ tristate "DesignWare PWM Controller (OF bus)"
+ depends on PWM_DWC && OF
+ 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 a70d36623129..d1fd1641f077 100644
--- a/drivers/pwm/Makefile
+++ b/drivers/pwm/Makefile
@@ -15,6 +15,7 @@ obj-$(CONFIG_PWM_CLPS711X) += pwm-clps711x.o
obj-$(CONFIG_PWM_CRC) += pwm-crc.o
obj-$(CONFIG_PWM_CROS_EC) += pwm-cros-ec.o
obj-$(CONFIG_PWM_DWC) += pwm-dwc.o
+obj-$(CONFIG_PWM_DWC_OF) += pwm-dwc-of.o
obj-$(CONFIG_PWM_DWC_PCI) += pwm-dwc-pci.o
obj-$(CONFIG_PWM_EP93XX) += pwm-ep93xx.o
obj-$(CONFIG_PWM_FSL_FTM) += pwm-fsl-ftm.o
diff --git a/drivers/pwm/pwm-dwc-of.c b/drivers/pwm/pwm-dwc-of.c
new file mode 100644
index 000000000000..c5b4351cc7b0
--- /dev/null
+++ b/drivers/pwm/pwm-dwc-of.c
@@ -0,0 +1,76 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * DesignWare PWM Controller driver OF
+ *
+ * Copyright (C) 2022 SiFive, Inc.
+ */
+
+#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_ns = NSEC_PER_SEC / 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 <[email protected]>");
+MODULE_DESCRIPTION("DesignWare PWM Controller");
+MODULE_LICENSE("GPL");
--
2.35.1
Hello Ben,
On Thu, Oct 20, 2022 at 04:16:01PM +0100, Ben Dooks wrote:
> Add documentation for the bindings for Synopsys' DesignWare PWM block
> as we will be adding DT/platform support to the Linux driver soon.
>
> Signed-off-by: Ben Dooks <[email protected]>
you would make life a bit easier if you added the tags you got for the
previous rounds (iff the patch didn't change). Here that would be:
Reviewed-by: Krzysztof Kozlowski <[email protected]>
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | https://www.pengutronix.de/ |
Hi
On 10/20/22 18:16, Ben Dooks wrote:
> This is an updated version of the Designware PWM driver updates
> for OF support, which now splits the driver into PCI and OF parts
> as well as tries to sort out the review comments.
>
> Hopefully this can now be queued for the next kernel version.
>
> v6:
> - fix removal ordering of DWC_PERIOD_NS
I did a quick test on our HW and PWM was counting as before.
Tested-by: Jarkko Nikula <[email protected]>
On 24/10/2022 09:39, Jarkko Nikula wrote:
> Hi
>
> On 10/20/22 18:16, Ben Dooks wrote:
>> This is an updated version of the Designware PWM driver updates
>> for OF support, which now splits the driver into PCI and OF parts
>> as well as tries to sort out the review comments.
>>
>> Hopefully this can now be queued for the next kernel version.
>>
>> v6:
>> - fix removal ordering of DWC_PERIOD_NS
>
> I did a quick test on our HW and PWM was counting as before.
>
> Tested-by: Jarkko Nikula <[email protected]>
Ok, great to hear, thank you.
I guess I should sort out putting a v7 out this week with all
the updated comments.
On 20/10/2022 16:36, Krzysztof Kozlowski wrote:
> On 20/10/2022 11:16, Ben Dooks wrote:
>> Add documentation for the bindings for Synopsys' DesignWare PWM block
>> as we will be adding DT/platform support to the Linux driver soon.
>>
>> Signed-off-by: Ben Dooks <[email protected]>
>> ---
>> v5:
>> - fixed order of properties
>> - corrected clock to two items
>
> This is a friendly reminder during the review process.
>
> It looks like you received a tag and forgot to add it.
>
> If you do not know the process, here is a short explanation:
> Please add Acked-by/Reviewed-by/Tested-by tags when posting new
> versions. However,
> there's no need to repost patches *only* to add the tags. The upstream
> maintainer will do that for acks received on the version they apply.
>
> https://elixir.bootlin.com/linux/v5.17/source/Documentation/process/submitting-patches.rst#L540
>
> If a tag was not added on purpose, please state why and what changed.
>
> Reviewed-by: Krzysztof Kozlowski <[email protected]>
Given the number of updates I should probably post a v7 of this series
with all the tags updated.
--
Ben
On 24/10/2022 09:39, Jarkko Nikula wrote:
> Hi
>
> On 10/20/22 18:16, Ben Dooks wrote:
>> This is an updated version of the Designware PWM driver updates
>> for OF support, which now splits the driver into PCI and OF parts
>> as well as tries to sort out the review comments.
>>
>> Hopefully this can now be queued for the next kernel version.
>>
>> v6:
>> - fix removal ordering of DWC_PERIOD_NS
>
> I did a quick test on our HW and PWM was counting as before.
>
> Tested-by: Jarkko Nikula <[email protected]>
Just to follow up on this, should I post a v7 of this (given
I think it is all just updates for review/tested) ?
--
Ben
On Tue, Nov 08, 2022 at 11:19:44AM +0000, Ben Dooks wrote:
> On 24/10/2022 09:39, Jarkko Nikula wrote:
> > Hi
> >
> > On 10/20/22 18:16, Ben Dooks wrote:
> > > This is an updated version of the Designware PWM driver updates
> > > for OF support, which now splits the driver into PCI and OF parts
> > > as well as tries to sort out the review comments.
> > >
> > > Hopefully this can now be queued for the next kernel version.
> > >
> > > v6:
> > > ? - fix removal ordering of DWC_PERIOD_NS
> >
> > I did a quick test on our HW and PWM was counting as before.
> >
> > Tested-by: Jarkko Nikula <[email protected]>
>
> Just to follow up on this, should I post a v7 of this (given
> I think it is all just updates for review/tested) ?
Just to add the tags doesn't justify a resend.
Reviewing this is in my todo list, I hope to come to it later this week.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | https://www.pengutronix.de/ |
Hello Ben,
On Thu, Oct 20, 2022 at 04:16:10PM +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
> 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 <[email protected]>
> Reported-by: Uwe Kleine-K?nig <[email protected]>
> ---
> drivers/pwm/pwm-dwc-of.c | 2 +-
> drivers/pwm/pwm-dwc.c | 29 ++++++++++++++++++++---------
> drivers/pwm/pwm-dwc.h | 2 +-
> 3 files changed, 22 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/pwm/pwm-dwc-of.c b/drivers/pwm/pwm-dwc-of.c
> index c5b4351cc7b0..5f7f066859d4 100644
> --- a/drivers/pwm/pwm-dwc-of.c
> +++ b/drivers/pwm/pwm-dwc-of.c
> @@ -50,7 +50,7 @@ static int dwc_pwm_plat_probe(struct platform_device *pdev)
> return dev_err_probe(dev, PTR_ERR(dwc->clk),
> "failed to get timer clock\n");
>
> - dwc->clk_ns = NSEC_PER_SEC / clk_get_rate(dwc->clk);
> + dwc->clk_rate = clk_get_rate(dwc->clk);
Given that clk_ns is introduced only in this series, I suggest to make
it right from the start.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | https://www.pengutronix.de/ |
On Thu, Oct 20, 2022 at 04:16:06PM +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 <[email protected]>
> ---
> 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-pci.c | 133 ++++++++++++++++++++++++++++++++
> drivers/pwm/pwm-dwc.c | 158 +-------------------------------------
> drivers/pwm/pwm-dwc.h | 58 ++++++++++++++
> 5 files changed, 207 insertions(+), 157 deletions(-)
> create mode 100644 drivers/pwm/pwm-dwc-pci.c
> create mode 100644 drivers/pwm/pwm-dwc.h
>
> diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
> index 3f3c53af4a56..a9f1c554db2b 100644
> --- a/drivers/pwm/Kconfig
> +++ b/drivers/pwm/Kconfig
> @@ -175,15 +175,23 @@ config PWM_CROS_EC
> Controller.
>
> config PWM_DWC
> - tristate "DesignWare PWM Controller"
> - depends on PCI || COMPILE_TEST
> + tristate "DesignWare PWM Controller core"
> depends on HAS_IOMEM
> help
> - PWM driver for Synopsys DWC PWM Controller attached to a PCI bus.
> + PWM driver for Synopsys DWC PWM Controller.
Currently pwm-dwc doesn't contain a driver but is only a library used by
pwm-dwc-pci (and later the of driver). As such it doesn't make sense to
be user-selectible, does it?
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | https://www.pengutronix.de/ |
On 10/11/2022 15:29, Uwe Kleine-König wrote:
> On Thu, Oct 20, 2022 at 04:16:06PM +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 <[email protected]>
>> ---
>> 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-pci.c | 133 ++++++++++++++++++++++++++++++++
>> drivers/pwm/pwm-dwc.c | 158 +-------------------------------------
>> drivers/pwm/pwm-dwc.h | 58 ++++++++++++++
>> 5 files changed, 207 insertions(+), 157 deletions(-)
>> create mode 100644 drivers/pwm/pwm-dwc-pci.c
>> create mode 100644 drivers/pwm/pwm-dwc.h
>>
>> diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
>> index 3f3c53af4a56..a9f1c554db2b 100644
>> --- a/drivers/pwm/Kconfig
>> +++ b/drivers/pwm/Kconfig
>> @@ -175,15 +175,23 @@ config PWM_CROS_EC
>> Controller.
>>
>> config PWM_DWC
>> - tristate "DesignWare PWM Controller"
>> - depends on PCI || COMPILE_TEST
>> + tristate "DesignWare PWM Controller core"
>> depends on HAS_IOMEM
>> help
>> - PWM driver for Synopsys DWC PWM Controller attached to a PCI bus.
>> + PWM driver for Synopsys DWC PWM Controller.
>
> Currently pwm-dwc doesn't contain a driver but is only a library used by
> pwm-dwc-pci (and later the of driver). As such it doesn't make sense to
> be user-selectible, does it?
I suppose the PWM_DWC should select the original driver an a new
PWM_DW_CORE should build the actual core driver.
--
Ben
Hello,
On Thu, Oct 20, 2022 at 04:16:07PM +0100, Ben Dooks wrote:
> 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 <[email protected]>
> ---
> 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-pci.c | 1 +
> drivers/pwm/pwm-dwc.c | 10 ++++++----
> drivers/pwm/pwm-dwc.h | 3 ++-
> 3 files changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/pwm/pwm-dwc-pci.c b/drivers/pwm/pwm-dwc-pci.c
> index 2213d0e7f3c8..949423e368f9 100644
> --- a/drivers/pwm/pwm-dwc-pci.c
> +++ b/drivers/pwm/pwm-dwc-pci.c
> @@ -20,6 +20,7 @@
> #include <linux/pci.h>
> #include <linux/pm_runtime.h>
> #include <linux/pwm.h>
> +#include <linux/clk.h>
>
> #include "pwm-dwc.h"
>
I assume this is only needed because pwm-dwc.h now uses struct clk.
Maybe just add
struct clk;
to the pwm-dwc.h instead of this hunk?
> [...]
> diff --git a/drivers/pwm/pwm-dwc.h b/drivers/pwm/pwm-dwc.h
> index 68f98eb76152..dc451cb2eff5 100644
> --- a/drivers/pwm/pwm-dwc.h
> +++ b/drivers/pwm/pwm-dwc.h
> @@ -22,7 +22,6 @@
> #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)
> @@ -41,6 +40,8 @@ struct dwc_pwm_ctx {
> struct dwc_pwm {
> struct pwm_chip chip;
> void __iomem *base;
> + struct clk *clk;
This is unsed, please add it in the patch that makes use of it.
> + unsigned int clk_ns;
> 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/ |
Hello,
On Thu, Oct 20, 2022 at 04:16:04PM +0100, Ben Dooks wrote:
> In preparation for adding other bus support, move the allocation
> of the pwm struct out of the main driver code.
>
> Signed-off-by: Ben Dooks <[email protected]>
> ---
> drivers/pwm/pwm-dwc.c | 24 +++++++++++++++++-------
> 1 file changed, 17 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/pwm/pwm-dwc.c b/drivers/pwm/pwm-dwc.c
> index c706ef9a7ba1..61f11e0a9319 100644
> --- a/drivers/pwm/pwm-dwc.c
> +++ b/drivers/pwm/pwm-dwc.c
> @@ -196,13 +196,29 @@ static const struct pwm_ops dwc_pwm_ops = {
> .owner = THIS_MODULE,
> };
>
> +static struct dwc_pwm *dwc_pwm_alloc(struct device *dev)
> +{
> + struct dwc_pwm *dwc;
> +
> + 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);
This was a pci_set_drvdata before. While it's common knowledge (and
implicitly used in several code parts) that pci_set_drvdata(pdev, data)
is just the same as dev_set_drvdata(&pdev->dev, data), it would be nice
if the driver only used either dev_[sg]et_drvdata or pci_[sg]et_drvdata.
Using the former in dwc_pwm_resume and dwc_pwm_suspend is even a tad
shorter.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | https://www.pengutronix.de/ |
On 10/11/2022 15:42, Uwe Kleine-König wrote:
> Hello Ben,
>
> On Thu, Oct 20, 2022 at 04:16:10PM +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
>> 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 <[email protected]>
>> Reported-by: Uwe Kleine-König <[email protected]>
>> ---
>> drivers/pwm/pwm-dwc-of.c | 2 +-
>> drivers/pwm/pwm-dwc.c | 29 ++++++++++++++++++++---------
>> drivers/pwm/pwm-dwc.h | 2 +-
>> 3 files changed, 22 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/pwm/pwm-dwc-of.c b/drivers/pwm/pwm-dwc-of.c
>> index c5b4351cc7b0..5f7f066859d4 100644
>> --- a/drivers/pwm/pwm-dwc-of.c
>> +++ b/drivers/pwm/pwm-dwc-of.c
>> @@ -50,7 +50,7 @@ static int dwc_pwm_plat_probe(struct platform_device *pdev)
>> return dev_err_probe(dev, PTR_ERR(dwc->clk),
>> "failed to get timer clock\n");
>>
>> - dwc->clk_ns = NSEC_PER_SEC / clk_get_rate(dwc->clk);
>> + dwc->clk_rate = clk_get_rate(dwc->clk);
>
> Given that clk_ns is introduced only in this series, I suggest to make
> it right from the start.
I was trying to keep the splitting of the driver and the clock changes
separate to make any possible bisection easier.
--
Ben
Hello,
On Thu, Nov 10, 2022 at 04:35:56PM +0000, Ben Dooks wrote:
> On 10/11/2022 15:29, Uwe Kleine-K?nig wrote:
> > On Thu, Oct 20, 2022 at 04:16:06PM +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 <[email protected]>
> > > ---
> > > 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-pci.c | 133 ++++++++++++++++++++++++++++++++
> > > drivers/pwm/pwm-dwc.c | 158 +-------------------------------------
> > > drivers/pwm/pwm-dwc.h | 58 ++++++++++++++
> > > 5 files changed, 207 insertions(+), 157 deletions(-)
> > > create mode 100644 drivers/pwm/pwm-dwc-pci.c
> > > create mode 100644 drivers/pwm/pwm-dwc.h
> > >
> > > diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
> > > index 3f3c53af4a56..a9f1c554db2b 100644
> > > --- a/drivers/pwm/Kconfig
> > > +++ b/drivers/pwm/Kconfig
> > > @@ -175,15 +175,23 @@ config PWM_CROS_EC
> > > Controller.
> > > config PWM_DWC
> > > - tristate "DesignWare PWM Controller"
> > > - depends on PCI || COMPILE_TEST
> > > + tristate "DesignWare PWM Controller core"
> > > depends on HAS_IOMEM
> > > help
> > > - PWM driver for Synopsys DWC PWM Controller attached to a PCI bus.
> > > + PWM driver for Synopsys DWC PWM Controller.
> >
> > Currently pwm-dwc doesn't contain a driver but is only a library used by
> > pwm-dwc-pci (and later the of driver). As such it doesn't make sense to
> > be user-selectible, does it?
>
> I suppose the PWM_DWC should select the original driver an a new
> PWM_DW_CORE should build the actual core driver.
Sounds about right.
I will discard the complete v6 series from the PWM patchwork in the
expectation that you will send a new round with all patches.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | https://www.pengutronix.de/ |
On Thu, Oct 20, 2022 at 04:16:01PM +0100, Ben Dooks wrote:
> Add documentation for the bindings for Synopsys' DesignWare PWM block
> as we will be adding DT/platform support to the Linux driver soon.
>
> Signed-off-by: Ben Dooks <[email protected]>
Acked-by: Uwe Kleine-K?nig <[email protected]>
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | https://www.pengutronix.de/ |