This is a series to allow the Synopsys PWM driver to be built for
device-tree/platform usage.
An open question is whether there should be some form of standard
property for the number of PWMs that each device supports.
Note, the patches 5 and 6 could be grouped together as patch
6 undoes some of the bits in 5 for configuration of clock rate.
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]>
---
.../devicetree/bindings/pwm/pwm-synposys.yaml | 40 ++++++++++++++
drivers/pwm/Kconfig | 5 +-
drivers/pwm/pwm-dwc.c | 53 +++++++++++++++++++
3 files changed, 96 insertions(+), 2 deletions(-)
create mode 100644 Documentation/devicetree/bindings/pwm/pwm-synposys.yaml
diff --git a/Documentation/devicetree/bindings/pwm/pwm-synposys.yaml b/Documentation/devicetree/bindings/pwm/pwm-synposys.yaml
new file mode 100644
index 000000000000..38ac0da75272
--- /dev/null
+++ b/Documentation/devicetree/bindings/pwm/pwm-synposys.yaml
@@ -0,0 +1,40 @@
+# 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/pwm-synposys.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Synopsys PWM controller
+
+maintainers:
+ - Ben Dooks <[email protected]>
+
+properties:
+ "#pwm-cells":
+ description: |
+ See pwm.yaml in this directory for a description of the cells format.
+
+ clocks:
+ items:
+ - description: Interface bus clock
+ - description: PWM reference clock
+
+ clock-names:
+ items:
+ - const: bus
+ - const: timer
+
+ compatible:
+ oneOf:
+ - items:
+ - const: snps,pwm
+
+required:
+ - "#pwm-cells"
+ - compatible
+ - reg
+ - clocks
+ - clock-names
+
+additionalProperties: false
diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
index 904de8d61828..e1aa645c1084 100644
--- a/drivers/pwm/Kconfig
+++ b/drivers/pwm/Kconfig
@@ -166,9 +166,10 @@ config PWM_CROS_EC
config PWM_DWC
tristate "DesignWare PWM Controller"
- depends on PCI
+ depends on PCI || OF
help
- PWM driver for Synopsys DWC PWM Controller attached to a PCI bus.
+ PWM driver for Synopsys DWC PWM Controller attached to either a
+ PCI or platform bus.
To compile this driver as a module, choose M here: the module
will be called pwm-dwc.
diff --git a/drivers/pwm/pwm-dwc.c b/drivers/pwm/pwm-dwc.c
index 61f11e0a9319..235cb730c888 100644
--- a/drivers/pwm/pwm-dwc.c
+++ b/drivers/pwm/pwm-dwc.c
@@ -18,6 +18,7 @@
#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/pci.h>
+#include <linux/platform_device.h>
#include <linux/pm_runtime.h>
#include <linux/pwm.h>
@@ -319,6 +320,58 @@ static struct pci_driver dwc_pwm_driver = {
module_pci_driver(dwc_pwm_driver);
+#ifdef CONFIG_OF
+static int dwc_pwm_plat_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct dwc_pwm *dwc;
+ int ret;
+
+ dwc = dwc_pwm_alloc(dev);
+ if (!dwc)
+ return -ENOMEM;
+
+ dwc->base = devm_platform_ioremap_resource(pdev, 0);
+ if (IS_ERR(dwc->base))
+ return dev_err_probe(dev, PTR_ERR(dwc->base),
+ "failed to map IO\n");
+
+ ret = pwmchip_add(&dwc->chip);
+ if (ret)
+ return ret;
+
+ return 0;
+}
+
+static int dwc_pwm_plat_remove(struct platform_device *pdev)
+{
+ struct dwc_pwm *dwc = platform_get_drvdata(pdev);
+
+ pwmchip_remove(&dwc->chip);
+ return 0;
+}
+
+static const struct of_device_id dwc_pwm_dt_ids[] = {
+ { .compatible = "snps,pwm" },
+ { },
+};
+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");
+#endif /* CONFIG_OF */
+
+
MODULE_AUTHOR("Felipe Balbi (Intel)");
MODULE_AUTHOR("Jarkko Nikula <[email protected]>");
MODULE_AUTHOR("Raymond Tan <[email protected]>");
--
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]>
---
drivers/pwm/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
index e1aa645c1084..e654655c4b3f 100644
--- a/drivers/pwm/Kconfig
+++ b/drivers/pwm/Kconfig
@@ -166,7 +166,7 @@ config PWM_CROS_EC
config PWM_DWC
tristate "DesignWare PWM Controller"
- depends on PCI || OF
+ depends on PCI || OF || COMPILE_TEST
help
PWM driver for Synopsys DWC PWM Controller attached to either a
PCI or platform bus.
--
2.35.1
Add snps,pwm-number property to indicate if the block does not have
all 8 of the PWM blocks.
Not sure if this should be a general PWM property consider optional
for all PWM types, so have added a specific one here (there is only
one other controller with a property for PWM count at the moment)
Signed-off-by: Ben Dooks <[email protected]>
---
Documentation/devicetree/bindings/pwm/pwm-synposys.yaml | 5 +++++
drivers/pwm/pwm-dwc.c | 8 ++++++++
2 files changed, 13 insertions(+)
diff --git a/Documentation/devicetree/bindings/pwm/pwm-synposys.yaml b/Documentation/devicetree/bindings/pwm/pwm-synposys.yaml
index 38ac0da75272..15bdf764b46a 100644
--- a/Documentation/devicetree/bindings/pwm/pwm-synposys.yaml
+++ b/Documentation/devicetree/bindings/pwm/pwm-synposys.yaml
@@ -30,11 +30,16 @@ properties:
- items:
- const: snps,pwm
+ snps,pwm-number:
+ $ref: '/schemas/types.yaml#/definitions/uint32'
+ description: u32 value representing the number of PWM devices
+
required:
- "#pwm-cells"
- compatible
- reg
- clocks
- clock-names
+ - snps,pwm-number
additionalProperties: false
diff --git a/drivers/pwm/pwm-dwc.c b/drivers/pwm/pwm-dwc.c
index 6a4364a5d137..abdde83452ad 100644
--- a/drivers/pwm/pwm-dwc.c
+++ b/drivers/pwm/pwm-dwc.c
@@ -328,12 +328,20 @@ static int dwc_pwm_plat_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
struct dwc_pwm *dwc;
+ 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 specified (%d)\n", nr_pwm);
+ else
+ dwc->chip.npwm = nr_pwm;
+ }
+
dwc->base = devm_platform_ioremap_resource(pdev, 0);
if (IS_ERR(dwc->base))
return dev_err_probe(dev, PTR_ERR(dwc->base),
--
2.35.1
We should probably change from the #ifdef added earlier in
49a0f4692a8752c7b03cb26d54282bee5c8c71bb ("wm: dwc: add timer clock")
and just have it always in the dwc data so if we have a system with
both PCI and OF probing it should work
-- consider merging with original patch
---
drivers/pwm/pwm-dwc.c | 15 +++++----------
1 file changed, 5 insertions(+), 10 deletions(-)
diff --git a/drivers/pwm/pwm-dwc.c b/drivers/pwm/pwm-dwc.c
index aa0486b89bdd..6a4364a5d137 100644
--- a/drivers/pwm/pwm-dwc.c
+++ b/drivers/pwm/pwm-dwc.c
@@ -37,12 +37,6 @@
#define DWC_TIMERS_TOTAL 8
-#ifndef CONFIG_OF
-#define DWC_CLK_PERIOD_NS 10
-#else
-#define DWC_CLK_PERIOD_NS dwc->clk_ns
-#endif
-
/* Timer Control Register */
#define DWC_TIM_CTRL_EN BIT(0)
#define DWC_TIM_CTRL_MODE BIT(1)
@@ -104,13 +98,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;
@@ -185,12 +179,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;
@@ -213,6 +207,7 @@ static 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;
--
2.35.1
On 12/07/2022 12:01, Ben Dooks wrote:
> 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]>
> ---
> .../devicetree/bindings/pwm/pwm-synposys.yaml | 40 ++++++++++++++
Bindings must be a separate patch. Preferably first in the series. Use
proper subject prefix matching the subsystem.
Best regards,
Krzysztof
On 12/07/2022 12:01, Ben Dooks wrote:
> We should probably change from the #ifdef added earlier in
> 49a0f4692a8752c7b03cb26d54282bee5c8c71bb ("wm: dwc: add timer clock")
> and just have it always in the dwc data so if we have a system with
> both PCI and OF probing it should work
>
> -- consider merging with original patch
Missing SoB. Please run checkpatch.
Best regards,
Krzysztof
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]>
---
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
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
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]>
---
drivers/pwm/pwm-dwc.c | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/drivers/pwm/pwm-dwc.c b/drivers/pwm/pwm-dwc.c
index 235cb730c888..aa0486b89bdd 100644
--- a/drivers/pwm/pwm-dwc.c
+++ b/drivers/pwm/pwm-dwc.c
@@ -18,6 +18,7 @@
#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/pci.h>
+#include <linux/clk.h>
#include <linux/platform_device.h>
#include <linux/pm_runtime.h>
#include <linux/pwm.h>
@@ -35,7 +36,12 @@
#define DWC_TIMERS_COMP_VERSION 0xac
#define DWC_TIMERS_TOTAL 8
+
+#ifndef CONFIG_OF
#define DWC_CLK_PERIOD_NS 10
+#else
+#define DWC_CLK_PERIOD_NS dwc->clk_ns
+#endif
/* Timer Control Register */
#define DWC_TIM_CTRL_EN BIT(0)
@@ -54,6 +60,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))
@@ -336,6 +344,14 @@ static int dwc_pwm_plat_probe(struct platform_device *pdev)
return dev_err_probe(dev, PTR_ERR(dwc->base),
"failed to map IO\n");
+ dwc->clk = devm_clk_get(dev, "timer");
+ if (IS_ERR(dwc->clk))
+ return dev_err_probe(dev, PTR_ERR(dwc->clk),
+ "failed to get timer clock\n");
+
+ clk_prepare_enable(dwc->clk);
+ dwc->clk_ns = 1000000000 /clk_get_rate(dwc->clk);
+
ret = pwmchip_add(&dwc->chip);
if (ret)
return ret;
@@ -347,6 +363,7 @@ static int dwc_pwm_plat_remove(struct platform_device *pdev)
{
struct dwc_pwm *dwc = platform_get_drvdata(pdev);
+ clk_disable_unprepare(dwc->clk);
pwmchip_remove(&dwc->chip);
return 0;
}
--
2.35.1
On 12/07/2022 12:01, Ben Dooks wrote:
> Add snps,pwm-number property to indicate if the block does not have
> all 8 of the PWM blocks.
>
> Not sure if this should be a general PWM property consider optional
> for all PWM types, so have added a specific one here (there is only
> one other controller with a property for PWM count at the moment)
>
> Signed-off-by: Ben Dooks <[email protected]>
> ---
> Documentation/devicetree/bindings/pwm/pwm-synposys.yaml | 5 +++++
No. Please do not hide bindings patches in the code. They must be
separate and checkpatch complains about it, so please be sure you run
checkpatch.
Best regards,
Krzysztof
On 12/07/2022 11:09, Krzysztof Kozlowski wrote:
> On 12/07/2022 12:01, Ben Dooks wrote:
>> We should probably change from the #ifdef added earlier in
>> 49a0f4692a8752c7b03cb26d54282bee5c8c71bb ("wm: dwc: add timer clock")
>> and just have it always in the dwc data so if we have a system with
>> both PCI and OF probing it should work
>>
>> -- consider merging with original patch
>
> Missing SoB. Please run checkpatch.
This was meant to be an RFC about whether it should be a single patch
or merged back into the previous one.
On Tue, 12 Jul 2022 11:01:09 +0100, Ben Dooks wrote:
> 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]>
> ---
> .../devicetree/bindings/pwm/pwm-synposys.yaml | 40 ++++++++++++++
> drivers/pwm/Kconfig | 5 +-
> drivers/pwm/pwm-dwc.c | 53 +++++++++++++++++++
> 3 files changed, 96 insertions(+), 2 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/pwm/pwm-synposys.yaml
>
My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):
yamllint warnings/errors:
./Documentation/devicetree/bindings/pwm/pwm-synposys.yaml:11:4: [warning] wrong indentation: expected 2 but found 3 (indentation)
./Documentation/devicetree/bindings/pwm/pwm-synposys.yaml:31:9: [warning] wrong indentation: expected 10 but found 8 (indentation)
dtschema/dtc warnings/errors:
doc reference errors (make refcheckdocs):
See https://patchwork.ozlabs.org/patch/
This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.
If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:
pip3 install dtschema --upgrade
Please check and re-submit.
On Tue, Jul 12, 2022 at 11:01:09AM +0100, Ben Dooks wrote:
> 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]>
> ---
> .../devicetree/bindings/pwm/pwm-synposys.yaml | 40 ++++++++++++++
Use compatible string for filename.
> drivers/pwm/Kconfig | 5 +-
> drivers/pwm/pwm-dwc.c | 53 +++++++++++++++++++
> 3 files changed, 96 insertions(+), 2 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/pwm/pwm-synposys.yaml
>
> diff --git a/Documentation/devicetree/bindings/pwm/pwm-synposys.yaml b/Documentation/devicetree/bindings/pwm/pwm-synposys.yaml
> new file mode 100644
> index 000000000000..38ac0da75272
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pwm/pwm-synposys.yaml
> @@ -0,0 +1,40 @@
> +# 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/pwm-synposys.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Synopsys PWM controller
> +
> +maintainers:
> + - Ben Dooks <[email protected]>
> +
> +properties:
> + "#pwm-cells":
> + description: |
> + See pwm.yaml in this directory for a description of the cells format.
pwm.yaml doesn't define how many cells. You need to. And you don't need
generic descriptions.
> +
> + clocks:
> + items:
> + - description: Interface bus clock
> + - description: PWM reference clock
> +
> + clock-names:
> + items:
> + - const: bus
> + - const: timer
> +
> + compatible:
Convention is compatible comes first in the list.
> + oneOf:
> + - items:
> + - const: snps,pwm
Don't need oneOf or items. Just 'const: snps,pwm'.
That's pretty generic for a compatible. There's only 1 version of the IP
or is the version discoverable?
> +
> +required:
> + - "#pwm-cells"
> + - compatible
> + - reg
> + - clocks
> + - clock-names
> +
> +additionalProperties: false
On Tue, Jul 12, 2022 at 11:01:11AM +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]>
> ---
> drivers/pwm/pwm-dwc.c | 17 +++++++++++++++++
> 1 file changed, 17 insertions(+)
>
> diff --git a/drivers/pwm/pwm-dwc.c b/drivers/pwm/pwm-dwc.c
> index 235cb730c888..aa0486b89bdd 100644
> --- a/drivers/pwm/pwm-dwc.c
> +++ b/drivers/pwm/pwm-dwc.c
> @@ -18,6 +18,7 @@
> #include <linux/kernel.h>
> #include <linux/module.h>
> #include <linux/pci.h>
> +#include <linux/clk.h>
> #include <linux/platform_device.h>
> #include <linux/pm_runtime.h>
> #include <linux/pwm.h>
> @@ -35,7 +36,12 @@
> #define DWC_TIMERS_COMP_VERSION 0xac
>
> #define DWC_TIMERS_TOTAL 8
> +
> +#ifndef CONFIG_OF
> #define DWC_CLK_PERIOD_NS 10
> +#else
> +#define DWC_CLK_PERIOD_NS dwc->clk_ns
> +#endif
Hmm, that looks wrong. If you have CONFIG_OF but use the pci device ...
IMHO it would help readability if you used ifdef. When there is an #else
branch anyhow, there is no reason to use the negative variant.
> /* Timer Control Register */
> #define DWC_TIM_CTRL_EN BIT(0)
> @@ -54,6 +60,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))
> @@ -336,6 +344,14 @@ static int dwc_pwm_plat_probe(struct platform_device *pdev)
> return dev_err_probe(dev, PTR_ERR(dwc->base),
> "failed to map IO\n");
>
> + dwc->clk = devm_clk_get(dev, "timer");
> + if (IS_ERR(dwc->clk))
> + return dev_err_probe(dev, PTR_ERR(dwc->clk),
> + "failed to get timer clock\n");
> +
> + clk_prepare_enable(dwc->clk);
> + dwc->clk_ns = 1000000000 /clk_get_rate(dwc->clk);
> +
> ret = pwmchip_add(&dwc->chip);
> if (ret)
> return ret;
Here you're missing clk_disable_unprepare(). (Alternatively use
devm_clk_get_enabled().)
> @@ -347,6 +363,7 @@ static int dwc_pwm_plat_remove(struct platform_device *pdev)
> {
> struct dwc_pwm *dwc = platform_get_drvdata(pdev);
>
> + clk_disable_unprepare(dwc->clk);
> pwmchip_remove(&dwc->chip);
The order is wrong here. You must only stop the clk when the pwmchip is
removed. Until that the PWM is supposed to stay functional.
> return 0;
> }
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | https://www.pengutronix.de/ |
On Tue, Jul 12, 2022 at 11:20:23AM +0100, Ben Dooks wrote:
> On 12/07/2022 11:09, Krzysztof Kozlowski wrote:
> > On 12/07/2022 12:01, Ben Dooks wrote:
> > > We should probably change from the #ifdef added earlier in
> > > 49a0f4692a8752c7b03cb26d54282bee5c8c71bb ("wm: dwc: add timer clock")
> > > and just have it always in the dwc data so if we have a system with
> > > both PCI and OF probing it should work
> > >
> > > -- consider merging with original patch
> >
> > Missing SoB. Please run checkpatch.
>
> This was meant to be an RFC about whether it should be a single patch
> or merged back into the previous one.
+1 for merging these
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | https://www.pengutronix.de/ |
On Tue, Jul 12, 2022 at 11:01:07AM +0100, Ben Dooks wrote:
> 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.
Looks reasonable.
Acked-by: Uwe Kleine-K?nig <[email protected]>
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | https://www.pengutronix.de/ |
On 12/07/2022 11:08, Krzysztof Kozlowski wrote:
> On 12/07/2022 12:01, Ben Dooks wrote:
>> 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]>
>> ---
>> .../devicetree/bindings/pwm/pwm-synposys.yaml | 40 ++++++++++++++
>
> Bindings must be a separate patch. Preferably first in the series. Use
> proper subject prefix matching the subsystem.
>
> Best regards,
> Krzysztof
Thanks, seems counter-intuitive that one change is split up like that.
--
Ben
On 13/07/2022 11:21, Ben Dooks wrote:
> On 12/07/2022 11:08, Krzysztof Kozlowski wrote:
>> On 12/07/2022 12:01, Ben Dooks wrote:
>>> 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]>
>>> ---
>>> .../devicetree/bindings/pwm/pwm-synposys.yaml | 40 ++++++++++++++
>>
>> Bindings must be a separate patch. Preferably first in the series. Use
>> proper subject prefix matching the subsystem.
>>
>> Best regards,
>> Krzysztof
>
> Thanks, seems counter-intuitive that one change is split up like that.
Why? Bindings are not really related to driver implementation, they are
shared with other projects. Binding reviewers usually care less about
implementation, so combining it with drivers forces me to read carefully
each one of 300 emails I receive...
Anyway, that's the policy since years, so nothing new here.
Best regards,
Krzysztof
On 12/07/2022 11:09, Krzysztof Kozlowski wrote:
> On 12/07/2022 12:01, Ben Dooks wrote:
>> We should probably change from the #ifdef added earlier in
>> 49a0f4692a8752c7b03cb26d54282bee5c8c71bb ("wm: dwc: add timer clock")
>> and just have it always in the dwc data so if we have a system with
>> both PCI and OF probing it should work
>>
>> -- consider merging with original patch
>
> Missing SoB. Please run checkpatch.
>
> Best regards,
> Krzysztof
Will be merging this with the previous patch now, thanks.
On 13/07/2022 09:16, Uwe Kleine-König wrote:
> On Tue, Jul 12, 2022 at 11:01:07AM +0100, Ben Dooks wrote:
>> 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.
>
> Looks reasonable.
>
> Acked-by: Uwe Kleine-König <[email protected]>
Thanks for the reviews, I'll get a new series out tomorrow.
I might avoid the last patch about number of pwms for now
and just get the basic driver updates sorted.
--
Ben
On 12/07/2022 23:17, Rob Herring wrote:
> On Tue, Jul 12, 2022 at 11:01:09AM +0100, Ben Dooks wrote:
>> 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]>
>> ---
>> .../devicetree/bindings/pwm/pwm-synposys.yaml | 40 ++++++++++++++
>
> Use compatible string for filename.
ok, will fix.
>> drivers/pwm/Kconfig | 5 +-
>> drivers/pwm/pwm-dwc.c | 53 +++++++++++++++++++
>> 3 files changed, 96 insertions(+), 2 deletions(-)
>> create mode 100644 Documentation/devicetree/bindings/pwm/pwm-synposys.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/pwm/pwm-synposys.yaml b/Documentation/devicetree/bindings/pwm/pwm-synposys.yaml
>> new file mode 100644
>> index 000000000000..38ac0da75272
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/pwm/pwm-synposys.yaml
>> @@ -0,0 +1,40 @@
>> +# 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/pwm-synposys.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Synopsys PWM controller
>> +
>> +maintainers:
>> + - Ben Dooks <[email protected]>
>> +
>> +properties:
>> + "#pwm-cells":
>> + description: |
>> + See pwm.yaml in this directory for a description of the cells format.
>
> pwm.yaml doesn't define how many cells. You need to. And you don't need
> generic descriptions.
"#pwm-cells":
const: 1
should be sufficient then?
>> +
>> + clocks:
>> + items:
>> + - description: Interface bus clock
>> + - description: PWM reference clock
>> +
>> + clock-names:
>> + items:
>> + - const: bus
>> + - const: timer
>> +
>> + compatible:
>
> Convention is compatible comes first in the list.
ok, fixed.
>> + oneOf:
>> + - items:
>> + - const: snps,pwm
>
> Don't need oneOf or items. Just 'const: snps,pwm'.
ok, fixed.
> That's pretty generic for a compatible. There's only 1 version of the IP
> or is the version discoverable?
This IP block doesn't seem to have evolved much since being included so
I don't think so at this point, most of the revision list is for the
internal options and cleanup.
I will go through and check if all the compile-time properties are deal
with, but that'll be adding more properiteis if they are needed.
>> +
>> +required:
>> + - "#pwm-cells"
>> + - compatible
>> + - reg
>> + - clocks
>> + - clock-names
>> +
>> +additionalProperties: false
On 12/07/2022 15:26, Rob Herring wrote:
> On Tue, 12 Jul 2022 11:01:09 +0100, Ben Dooks wrote:
>> 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]>
>> ---
>> .../devicetree/bindings/pwm/pwm-synposys.yaml | 40 ++++++++++++++
>> drivers/pwm/Kconfig | 5 +-
>> drivers/pwm/pwm-dwc.c | 53 +++++++++++++++++++
>> 3 files changed, 96 insertions(+), 2 deletions(-)
>> create mode 100644 Documentation/devicetree/bindings/pwm/pwm-synposys.yaml
>>
>
> My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
> on your patch (DT_CHECKER_FLAGS is new in v5.13):
Is the following equivalent:
> make dt_binding_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/pwm/snps,pwm.yaml
> yamllint warnings/errors:
> ./Documentation/devicetree/bindings/pwm/pwm-synposys.yaml:11:4: [warning] wrong indentation: expected 2 but found 3 (indentation)
> ./Documentation/devicetree/bindings/pwm/pwm-synposys.yaml:31:9: [warning] wrong indentation: expected 10 but found 8 (indentation)
>
> dtschema/dtc warnings/errors:
>
> doc reference errors (make refcheckdocs):
>
> See https://patchwork.ozlabs.org/patch/
>
> This check can fail if there are any dependencies. The base for a patch
> series is generally the most recent rc1.
>
> If you already ran 'make dt_binding_check' and didn't see the above
> error(s), then make sure 'yamllint' is installed and dt-schema is up to
> date:
Whoops, turns out I hadn't installed yamllint.
> pip3 install dtschema --upgrade
>
> Please check and re-submit.
>
On Wed, Jul 13, 2022 at 12:56:55PM +0100, Ben Dooks wrote:
> On 12/07/2022 23:17, Rob Herring wrote:
> > On Tue, Jul 12, 2022 at 11:01:09AM +0100, Ben Dooks wrote:
> > > 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]>
> > > ---
> > > .../devicetree/bindings/pwm/pwm-synposys.yaml | 40 ++++++++++++++
> >
> > Use compatible string for filename.
>
> ok, will fix.
>
> > > drivers/pwm/Kconfig | 5 +-
> > > drivers/pwm/pwm-dwc.c | 53 +++++++++++++++++++
> > > 3 files changed, 96 insertions(+), 2 deletions(-)
> > > create mode 100644 Documentation/devicetree/bindings/pwm/pwm-synposys.yaml
> > >
> > > diff --git a/Documentation/devicetree/bindings/pwm/pwm-synposys.yaml b/Documentation/devicetree/bindings/pwm/pwm-synposys.yaml
> > > new file mode 100644
> > > index 000000000000..38ac0da75272
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/pwm/pwm-synposys.yaml
> > > @@ -0,0 +1,40 @@
> > > +# 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/pwm-synposys.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: Synopsys PWM controller
> > > +
> > > +maintainers:
> > > + - Ben Dooks <[email protected]>
> > > +
> > > +properties:
> > > + "#pwm-cells":
> > > + description: |
> > > + See pwm.yaml in this directory for a description of the cells format.
> >
> > pwm.yaml doesn't define how many cells. You need to. And you don't need
> > generic descriptions.
>
> "#pwm-cells":
> const: 1
>
> should be sufficient then?
I would expect a value of (at least) 2 or (better) 3.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | https://www.pengutronix.de/ |
On 13/07/2022 14:52, Uwe Kleine-König wrote:
> On Wed, Jul 13, 2022 at 12:56:55PM +0100, Ben Dooks wrote:
>> On 12/07/2022 23:17, Rob Herring wrote:
>>> On Tue, Jul 12, 2022 at 11:01:09AM +0100, Ben Dooks wrote:
>>>> 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]>
[snip]
>>>> +properties:
>>>> + "#pwm-cells":
>>>> + description: |
>>>> + See pwm.yaml in this directory for a description of the cells format.
>>>
>>> pwm.yaml doesn't define how many cells. You need to. And you don't need
>>> generic descriptions.
>>
>> "#pwm-cells":
>> const: 1
>>
>> should be sufficient then?
>
> I would expect a value of (at least) 2 or (better) 3.
OOPS, forgot the phandle.
I will have to check if we have any support yet for dealing
with any of the pwm flags yet.
> Best regards
> Uwe
>
On Wed, Jul 13, 2022 at 03:30:07PM +0100, Ben Dooks wrote:
> On 13/07/2022 14:52, Uwe Kleine-K?nig wrote:
> > On Wed, Jul 13, 2022 at 12:56:55PM +0100, Ben Dooks wrote:
> > > On 12/07/2022 23:17, Rob Herring wrote:
> > > > On Tue, Jul 12, 2022 at 11:01:09AM +0100, Ben Dooks wrote:
> > > > > 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]>
>
> [snip]
>
> > > > > +properties:
> > > > > + "#pwm-cells":
> > > > > + description: |
> > > > > + See pwm.yaml in this directory for a description of the cells format.
> > > >
> > > > pwm.yaml doesn't define how many cells. You need to. And you don't need
> > > > generic descriptions.
> > >
> > > "#pwm-cells":
> > > const: 1
> > >
> > > should be sufficient then?
> >
> > I would expect a value of (at least) 2 or (better) 3.
>
> OOPS, forgot the phandle.
>
> I will have to check if we have any support yet for dealing
> with any of the pwm flags yet.
I didn't double check, but given that the driver only supports inversed
polarity it might not even work without passing the flag for inversed
polarity. Having said that, I expect you have to only add "#pwm-cells =
<3>;" to your dts and then everything should work just fine.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | https://www.pengutronix.de/ |
On 13/07/2022 16:07, Uwe Kleine-König wrote:
> On Wed, Jul 13, 2022 at 03:30:07PM +0100, Ben Dooks wrote:
>> On 13/07/2022 14:52, Uwe Kleine-König wrote:
>>> On Wed, Jul 13, 2022 at 12:56:55PM +0100, Ben Dooks wrote:
>>>> On 12/07/2022 23:17, Rob Herring wrote:
>>>>> On Tue, Jul 12, 2022 at 11:01:09AM +0100, Ben Dooks wrote:
>>>>>> 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]>
>>
>> [snip]
>>
>>>>>> +properties:
>>>>>> + "#pwm-cells":
>>>>>> + description: |
>>>>>> + See pwm.yaml in this directory for a description of the cells format.
>>>>>
>>>>> pwm.yaml doesn't define how many cells. You need to. And you don't need
>>>>> generic descriptions.
>>>>
>>>> "#pwm-cells":
>>>> const: 1
>>>>
>>>> should be sufficient then?
>>>
>>> I would expect a value of (at least) 2 or (better) 3.
>>
>> OOPS, forgot the phandle.
>>
>> I will have to check if we have any support yet for dealing
>> with any of the pwm flags yet.
>
> I didn't double check, but given that the driver only supports inversed
> polarity it might not even work without passing the flag for inversed
> polarity. Having said that, I expect you have to only add "#pwm-cells =
> <3>;" to your dts and then everything should work just fine.
I hadn't noticed, but we've been testing with the sysfs interface
as we're using qemu.
On 13/07/2022 07:11, Uwe Kleine-König wrote:
> On Tue, Jul 12, 2022 at 11:20:23AM +0100, Ben Dooks wrote:
>> On 12/07/2022 11:09, Krzysztof Kozlowski wrote:
>>> On 12/07/2022 12:01, Ben Dooks wrote:
>>>> We should probably change from the #ifdef added earlier in
>>>> 49a0f4692a8752c7b03cb26d54282bee5c8c71bb ("wm: dwc: add timer clock")
>>>> and just have it always in the dwc data so if we have a system with
>>>> both PCI and OF probing it should work
>>>>
>>>> -- consider merging with original patch
>>>
>>> Missing SoB. Please run checkpatch.
>>
>> This was meant to be an RFC about whether it should be a single patch
>> or merged back into the previous one.
>
> +1 for merging these
Yes, done for v2.
On 13/07/2022 09:16, Uwe Kleine-König wrote:
> On Tue, Jul 12, 2022 at 11:01:07AM +0100, Ben Dooks wrote:
>> 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.
>
> Looks reasonable.
>
> Acked-by: Uwe Kleine-König <[email protected]>
ack for 1/7 or the series?
I'll repost v2 this week once we've dealt with polarity.
>
On 13/07/2022 16:07, Uwe Kleine-König wrote:
> On Wed, Jul 13, 2022 at 03:30:07PM +0100, Ben Dooks wrote:
>> On 13/07/2022 14:52, Uwe Kleine-König wrote:
>>> On Wed, Jul 13, 2022 at 12:56:55PM +0100, Ben Dooks wrote:
>>>> On 12/07/2022 23:17, Rob Herring wrote:
>>>>> On Tue, Jul 12, 2022 at 11:01:09AM +0100, Ben Dooks wrote:
>>>>>> 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]>
>>
>> [snip]
>>
>>>>>> +properties:
>>>>>> + "#pwm-cells":
>>>>>> + description: |
>>>>>> + See pwm.yaml in this directory for a description of the cells format.
>>>>>
>>>>> pwm.yaml doesn't define how many cells. You need to. And you don't need
>>>>> generic descriptions.
>>>>
>>>> "#pwm-cells":
>>>> const: 1
>>>>
>>>> should be sufficient then?
>>>
>>> I would expect a value of (at least) 2 or (better) 3.
>>
>> OOPS, forgot the phandle.
>>
>> I will have to check if we have any support yet for dealing
>> with any of the pwm flags yet.
>
> I didn't double check, but given that the driver only supports inversed
> polarity it might not even work without passing the flag for inversed
> polarity. Having said that, I expect you have to only add "#pwm-cells =
> <3>;" to your dts and then everything should work just fine.
I've gone back over the documentation we have for the block, and it
should have a count for high and a count for low in the PWM mode the
driver puts it into. I have no idea /why/ the driver is reporting it
as inversed, unless the PCI version has this automatically set....
I will go back and talk with the engineer who did the testing of the
PWM to get the test-bench re-set and check this, however my expectation
is we could easily do both and for the of/plat case we should just
report normal polarity (and we could deal with the inversed by simply
swapping the low and high values).
I also noted the v2 block supports 0 and 100% by setting a bit in the
control and the timers to a given value, so that can also be added to
the series (although this requires an IP generation option to be
set) which we can also add.
Thnak you for pointing this out, hopefully we can have this sorted
today and if so we will need to change this to a range of 2..3 for
the PWM cells.
> Best regards
> Uwe
>
On Mon, Jul 18, 2022 at 08:19:16AM +0100, Ben Dooks wrote:
> On 13/07/2022 09:16, Uwe Kleine-K?nig wrote:
> > On Tue, Jul 12, 2022 at 11:01:07AM +0100, Ben Dooks wrote:
> > > 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.
> >
> > Looks reasonable.
> >
> > Acked-by: Uwe Kleine-K?nig <[email protected]>
>
> ack for 1/7 or the series?
The former. For the other patches I assume they will change in v2.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | https://www.pengutronix.de/ |
Hello,
On Mon, Jul 18, 2022 at 09:49:54AM +0200, Uwe Kleine-K?nig wrote:
> On Mon, Jul 18, 2022 at 08:19:16AM +0100, Ben Dooks wrote:
> > On 13/07/2022 09:16, Uwe Kleine-K?nig wrote:
> > > On Tue, Jul 12, 2022 at 11:01:07AM +0100, Ben Dooks wrote:
> > > > 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.
> > >
> > > Looks reasonable.
> > >
> > > Acked-by: Uwe Kleine-K?nig <[email protected]>
> >
> > ack for 1/7 or the series?
>
> The former. For the other patches I assume they will change in v2.
Oh well, and 2/7 is so obviously a preparation patch that I'll wait to
see v2 for that one, too.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | https://www.pengutronix.de/ |
On Tue, Jul 12, 2022 at 11:01:13AM +0100, Ben Dooks wrote:
> Add snps,pwm-number property to indicate if the block does not have
> all 8 of the PWM blocks.
>
> Not sure if this should be a general PWM property consider optional
> for all PWM types, so have added a specific one here (there is only
> one other controller with a property for PWM count at the moment)
>
> Signed-off-by: Ben Dooks <[email protected]>
> ---
> Documentation/devicetree/bindings/pwm/pwm-synposys.yaml | 5 +++++
> drivers/pwm/pwm-dwc.c | 8 ++++++++
> 2 files changed, 13 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/pwm/pwm-synposys.yaml b/Documentation/devicetree/bindings/pwm/pwm-synposys.yaml
> index 38ac0da75272..15bdf764b46a 100644
> --- a/Documentation/devicetree/bindings/pwm/pwm-synposys.yaml
> +++ b/Documentation/devicetree/bindings/pwm/pwm-synposys.yaml
> @@ -30,11 +30,16 @@ properties:
> - items:
> - const: snps,pwm
>
> + snps,pwm-number:
> + $ref: '/schemas/types.yaml#/definitions/uint32'
> + description: u32 value representing the number of PWM devices
Why do we need to know this? Are you going to have a consumer to a
non-existent PWM? If you do need to know how many, it should be implied
by the compatible string.
Rob
On 18/07/2022 21:08, Rob Herring wrote:
> On Tue, Jul 12, 2022 at 11:01:13AM +0100, Ben Dooks wrote:
>> Add snps,pwm-number property to indicate if the block does not have
>> all 8 of the PWM blocks.
>>
>> Not sure if this should be a general PWM property consider optional
>> for all PWM types, so have added a specific one here (there is only
>> one other controller with a property for PWM count at the moment)
>>
>> Signed-off-by: Ben Dooks <[email protected]>
>> ---
>> Documentation/devicetree/bindings/pwm/pwm-synposys.yaml | 5 +++++
>> drivers/pwm/pwm-dwc.c | 8 ++++++++
>> 2 files changed, 13 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/pwm/pwm-synposys.yaml b/Documentation/devicetree/bindings/pwm/pwm-synposys.yaml
>> index 38ac0da75272..15bdf764b46a 100644
>> --- a/Documentation/devicetree/bindings/pwm/pwm-synposys.yaml
>> +++ b/Documentation/devicetree/bindings/pwm/pwm-synposys.yaml
>> @@ -30,11 +30,16 @@ properties:
>> - items:
>> - const: snps,pwm
>>
>> + snps,pwm-number:
>> + $ref: '/schemas/types.yaml#/definitions/uint32'
>> + description: u32 value representing the number of PWM devices
>
> Why do we need to know this? Are you going to have a consumer to a
> non-existent PWM? If you do need to know how many, it should be implied
> by the compatible string.
For this IP block it is a build time option for 1..8 timers
so I thought it best we don't register non-existant timers
The system we are working on only has 1 PWM timer per block.
--
Ben