Hi all,
This series adds a driver for the Marvell Berlin PWM controller, which
has 4 channels.
This has been tested on a BG2Q DMP.
Thanks,
Antoine
Changes since v2:
- Reworked the prescaler/tcnt calculation
- Added an input clock
- Updated the documentation
- Reordered the pwm node
- Added some definition for BIT()
Changes since v1:
- Added a sentinel in berlin_pwm_match[]
Antoine Tenart (5):
pwm: add the Berlin pwm controller driver
Documentation: bindings: document the Berlin PWM driver
ARM: berlin: add a PWM node on the BG2Q
ARM: berlin: add a PWM node on the BG2
ARM: berlin: add a PWM node on the BG2CD
.../devicetree/bindings/pwm/pwm-berlin.txt | 19 ++
arch/arm/boot/dts/berlin2.dtsi | 7 +
arch/arm/boot/dts/berlin2cd.dtsi | 7 +
arch/arm/boot/dts/berlin2q.dtsi | 7 +
drivers/pwm/Kconfig | 9 +
drivers/pwm/Makefile | 1 +
drivers/pwm/pwm-berlin.c | 227 +++++++++++++++++++++
7 files changed, 277 insertions(+)
create mode 100644 Documentation/devicetree/bindings/pwm/pwm-berlin.txt
create mode 100644 drivers/pwm/pwm-berlin.c
--
2.5.0
Add a PWM controller driver for the Marvell Berlin SoCs. This PWM
controller has 4 channels.
Signed-off-by: Antoine Tenart <[email protected]>
---
drivers/pwm/Kconfig | 9 ++
drivers/pwm/Makefile | 1 +
drivers/pwm/pwm-berlin.c | 227 +++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 237 insertions(+)
create mode 100644 drivers/pwm/pwm-berlin.c
diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
index b1541f40fd8d..1773da8145b8 100644
--- a/drivers/pwm/Kconfig
+++ b/drivers/pwm/Kconfig
@@ -92,6 +92,15 @@ config PWM_BCM2835
To compile this driver as a module, choose M here: the module
will be called pwm-bcm2835.
+config PWM_BERLIN
+ tristate "Berlin PWM support"
+ depends on ARCH_BERLIN
+ help
+ PWM framework driver for Berlin.
+
+ To compile this driver as a module, choose M here: the module
+ will be called pwm-berlin.
+
config PWM_BFIN
tristate "Blackfin PWM support"
depends on BFIN_GPTIMERS
diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile
index ec50eb5b5a8f..670c5fce8bbb 100644
--- a/drivers/pwm/Makefile
+++ b/drivers/pwm/Makefile
@@ -6,6 +6,7 @@ obj-$(CONFIG_PWM_ATMEL_HLCDC_PWM) += pwm-atmel-hlcdc.o
obj-$(CONFIG_PWM_ATMEL_TCB) += pwm-atmel-tcb.o
obj-$(CONFIG_PWM_BCM_KONA) += pwm-bcm-kona.o
obj-$(CONFIG_PWM_BCM2835) += pwm-bcm2835.o
+obj-$(CONFIG_PWM_BERLIN) += pwm-berlin.o
obj-$(CONFIG_PWM_BFIN) += pwm-bfin.o
obj-$(CONFIG_PWM_CLPS711X) += pwm-clps711x.o
obj-$(CONFIG_PWM_EP93XX) += pwm-ep93xx.o
diff --git a/drivers/pwm/pwm-berlin.c b/drivers/pwm/pwm-berlin.c
new file mode 100644
index 000000000000..f89e25ea5c6d
--- /dev/null
+++ b/drivers/pwm/pwm-berlin.c
@@ -0,0 +1,227 @@
+/*
+ * Marvell Berlin PWM driver
+ *
+ * Copyright (C) 2015 Marvell Technology Group Ltd.
+ *
+ * Antoine Tenart <[email protected]>
+ *
+ * This file is licensed under the terms of the GNU General Public
+ * License version 2. This program is licensed "as is" without any
+ * warranty of any kind, whether express or implied.
+ */
+
+#include <linux/clk.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/pwm.h>
+#include <linux/spinlock.h>
+
+#define BERLIN_PWM_EN 0x0
+#define BERLIN_PWM_CONTROL 0x4
+#define BERLIN_PWM_DUTY 0x8
+#define BERLIN_PWM_TCNT 0xc
+
+#define BERLIN_PWM_ENABLE BIT(0)
+#define BERLIN_PWM_DISABLE 0x0
+#define BERLIN_PWM_INVERT_POLARITY BIT(3)
+#define BERLIN_PWM_PRESCALE_MASK 0x7
+#define BERLIN_PWM_PRESCALE_MAX 4096
+#define BERLIN_PWM_MAX_TCNT 65535
+
+struct berlin_pwm_chip {
+ struct pwm_chip chip;
+ struct clk *clk;
+ void __iomem *base;
+ spinlock_t lock;
+};
+
+#define to_berlin_pwm_chip(chip) \
+ container_of((chip), struct berlin_pwm_chip, chip)
+
+#define berlin_pwm_readl(chip, channel, offset) \
+ readl((chip)->base + (channel) * 0x10 + offset)
+#define berlin_pwm_writel(val, chip, channel, offset) \
+ writel(val, (chip)->base + (channel) * 0x10 + offset)
+
+/* prescaler table: {1, 4, 8, 16, 64, 256, 1024, 4096} */
+static const u32 prescaler_diff_table[] = {
+ 1, 4, 2, 2, 4, 4, 4, 4,
+};
+
+static int berlin_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
+ int duty_ns, int period_ns)
+{
+ struct berlin_pwm_chip *berlin_chip = to_berlin_pwm_chip(chip);
+ int ret, prescale = 0;
+ u32 val, duty, period;
+ u64 cycles;
+
+ cycles = clk_get_rate(berlin_chip->clk);
+ cycles *= period_ns;
+ do_div(cycles, NSEC_PER_SEC);
+
+ while (cycles > BERLIN_PWM_MAX_TCNT)
+ do_div(cycles, prescaler_diff_table[++prescale]);
+
+ if (cycles > BERLIN_PWM_MAX_TCNT)
+ return -EINVAL;
+
+ period = cycles;
+ cycles *= duty_ns;
+ do_div(cycles, period_ns);
+ duty = cycles;
+
+ ret = clk_prepare_enable(berlin_chip->clk);
+ if (ret)
+ return ret;
+
+ spin_lock(&berlin_chip->lock);
+
+ val = berlin_pwm_readl(berlin_chip, pwm->hwpwm, BERLIN_PWM_CONTROL);
+ val &= ~BERLIN_PWM_PRESCALE_MASK;
+ val |= prescale;
+ berlin_pwm_writel(val, berlin_chip, pwm->hwpwm, BERLIN_PWM_CONTROL);
+
+ berlin_pwm_writel(duty, berlin_chip, pwm->hwpwm, BERLIN_PWM_DUTY);
+ berlin_pwm_writel(period, berlin_chip, pwm->hwpwm, BERLIN_PWM_TCNT);
+
+ spin_unlock(&berlin_chip->lock);
+
+ clk_disable_unprepare(berlin_chip->clk);
+
+ return 0;
+}
+
+static int berlin_pwm_set_polarity(struct pwm_chip *chip,
+ struct pwm_device *pwm,
+ enum pwm_polarity polarity)
+{
+ struct berlin_pwm_chip *berlin_chip = to_berlin_pwm_chip(chip);
+ int ret;
+ u32 val;
+
+ ret = clk_prepare_enable(berlin_chip->clk);
+ if (ret)
+ return ret;
+
+ spin_lock(&berlin_chip->lock);
+
+ val = berlin_pwm_readl(berlin_chip, pwm->hwpwm, BERLIN_PWM_CONTROL);
+
+ if (polarity == PWM_POLARITY_NORMAL)
+ val &= ~BERLIN_PWM_INVERT_POLARITY;
+ else
+ val |= BERLIN_PWM_INVERT_POLARITY;
+
+ berlin_pwm_writel(val, berlin_chip, pwm->hwpwm, BERLIN_PWM_CONTROL);
+
+ spin_unlock(&berlin_chip->lock);
+
+ clk_disable_unprepare(berlin_chip->clk);
+
+ return 0;
+}
+
+static int berlin_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm)
+{
+ struct berlin_pwm_chip *berlin_chip = to_berlin_pwm_chip(chip);
+ int ret;
+
+ ret = clk_prepare_enable(berlin_chip->clk);
+ if (ret)
+ return ret;
+
+ spin_lock(&berlin_chip->lock);
+ berlin_pwm_writel(BERLIN_PWM_ENABLE, berlin_chip, pwm->hwpwm, BERLIN_PWM_EN);
+ spin_unlock(&berlin_chip->lock);
+
+ return 0;
+}
+
+static void berlin_pwm_disable(struct pwm_chip *chip, struct pwm_device *pwm)
+{
+ struct berlin_pwm_chip *berlin_chip = to_berlin_pwm_chip(chip);
+
+ spin_lock(&berlin_chip->lock);
+ berlin_pwm_writel(BERLIN_PWM_DISABLE, berlin_chip, pwm->hwpwm, BERLIN_PWM_EN);
+ spin_unlock(&berlin_chip->lock);
+
+ clk_disable_unprepare(berlin_chip->clk);
+}
+
+static const struct pwm_ops berlin_pwm_ops = {
+ .config = berlin_pwm_config,
+ .set_polarity = berlin_pwm_set_polarity,
+ .enable = berlin_pwm_enable,
+ .disable = berlin_pwm_disable,
+ .owner = THIS_MODULE,
+};
+
+static const struct of_device_id berlin_pwm_match[] = {
+ { .compatible = "marvell,berlin-pwm" },
+ { },
+};
+MODULE_DEVICE_TABLE(of, berlin_pwm_match);
+
+static int berlin_pwm_probe(struct platform_device *pdev)
+{
+ struct berlin_pwm_chip *pwm;
+ struct resource *res;
+ int ret;
+
+ pwm = devm_kzalloc(&pdev->dev, sizeof(*pwm), GFP_KERNEL);
+ if (!pwm)
+ return -ENOMEM;
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ pwm->base = devm_ioremap_resource(&pdev->dev, res);
+ if (IS_ERR(pwm->base))
+ return PTR_ERR(pwm->base);
+
+ pwm->clk = devm_clk_get(&pdev->dev, NULL);
+ if (IS_ERR(pwm->clk))
+ return PTR_ERR(pwm->clk);
+
+ pwm->chip.dev = &pdev->dev;
+ pwm->chip.ops = &berlin_pwm_ops;
+ pwm->chip.base = -1;
+ pwm->chip.npwm = 4;
+ pwm->chip.can_sleep = true;
+ pwm->chip.of_xlate = of_pwm_xlate_with_flags;
+ pwm->chip.of_pwm_n_cells = 3;
+
+ spin_lock_init(&pwm->lock);
+
+ ret = pwmchip_add(&pwm->chip);
+ if (ret < 0) {
+ dev_err(&pdev->dev, "Failed to add PWM chip: %d\n", ret);
+ return ret;
+ }
+
+ platform_set_drvdata(pdev, pwm);
+
+ return 0;
+}
+
+static int berlin_pwm_remove(struct platform_device *pdev)
+{
+ struct berlin_pwm_chip *pwm = platform_get_drvdata(pdev);
+
+ return pwmchip_remove(&pwm->chip);
+}
+
+static struct platform_driver berlin_pwm_driver = {
+ .probe = berlin_pwm_probe,
+ .remove = berlin_pwm_remove,
+ .driver = {
+ .name = "berlin-pwm",
+ .of_match_table = berlin_pwm_match,
+ },
+};
+module_platform_driver(berlin_pwm_driver);
+
+MODULE_AUTHOR("Antoine Tenart <[email protected]>");
+MODULE_DESCRIPTION("Marvell Berlin PWM driver");
+MODULE_LICENSE("GPL v2");
--
2.5.0
Following the addition of a Berlin PWM driver, this patch adds the
corresponding documentation.
Signed-off-by: Antoine Tenart <[email protected]>
Acked-by: Sebastian Hesselbarth <[email protected]>
---
Documentation/devicetree/bindings/pwm/pwm-berlin.txt | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
create mode 100644 Documentation/devicetree/bindings/pwm/pwm-berlin.txt
diff --git a/Documentation/devicetree/bindings/pwm/pwm-berlin.txt b/Documentation/devicetree/bindings/pwm/pwm-berlin.txt
new file mode 100644
index 000000000000..8f9bc11f8c4c
--- /dev/null
+++ b/Documentation/devicetree/bindings/pwm/pwm-berlin.txt
@@ -0,0 +1,19 @@
+Berlin PWM controller
+
+PWM IP found in Marvell Berlin SoCs.
+
+Required properties:
+- compatible: should be "marvell,berlin-pwm"
+- reg: physical base address and length of the controller's registers
+- clocks: phandle to the input clock
+- #pwm-cells: should be 3. See pwm.txt in this directory for a description of
+ the cells format.
+
+Example:
+
+pwm: pwm@f7f20000 {
+ compatible = "marvell,berlin-pwm";
+ reg = <0xf7f20000 0x40>;
+ clocks = <&chip_clk CLKID_CFG>;
+ #pwm-cells = <3>;
+}
--
2.5.0
This patch adds a PWM node in the Berlin BG2Q device tree, using the
newly added Berlin PWM driver.
Signed-off-by: Antoine Tenart <[email protected]>
---
arch/arm/boot/dts/berlin2q.dtsi | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/arch/arm/boot/dts/berlin2q.dtsi b/arch/arm/boot/dts/berlin2q.dtsi
index 63a48490e2f9..5bfa71a87617 100644
--- a/arch/arm/boot/dts/berlin2q.dtsi
+++ b/arch/arm/boot/dts/berlin2q.dtsi
@@ -477,6 +477,13 @@
status = "disabled";
};
+ pwm: pwm@f20000 {
+ compatible = "marvell,berlin-pwm";
+ reg = <0xf20000 0x40>;
+ clocks = <&chip_clk CLKID_CFG>;
+ #pwm-cells = <3>;
+ };
+
apb@fc0000 {
compatible = "simple-bus";
#address-cells = <1>;
--
2.5.0
This patch adds a PWM node in the Berlin BG2 device tree, using the
newly added Berlin PWM driver.
Signed-off-by: Antoine Tenart <[email protected]>
---
arch/arm/boot/dts/berlin2.dtsi | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/arch/arm/boot/dts/berlin2.dtsi b/arch/arm/boot/dts/berlin2.dtsi
index ef811de09908..f094b06d16aa 100644
--- a/arch/arm/boot/dts/berlin2.dtsi
+++ b/arch/arm/boot/dts/berlin2.dtsi
@@ -404,6 +404,13 @@
};
};
+ pwm: pwm@f20000 {
+ compatible = "marvell,berlin-pwm";
+ reg = <0xf20000 0x40>;
+ clocks = <&chip_clk CLKID_CFG>;
+ #pwm-cells = <3>;
+ };
+
apb@fc0000 {
compatible = "simple-bus";
#address-cells = <1>;
--
2.5.0
This patch adds a PWM node in the Berlin BG2CD device tree, using the
newly added Berlin PWM driver.
Signed-off-by: Antoine Tenart <[email protected]>
---
arch/arm/boot/dts/berlin2cd.dtsi | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/arch/arm/boot/dts/berlin2cd.dtsi b/arch/arm/boot/dts/berlin2cd.dtsi
index 900213d78a32..5d984f58dde7 100644
--- a/arch/arm/boot/dts/berlin2cd.dtsi
+++ b/arch/arm/boot/dts/berlin2cd.dtsi
@@ -368,6 +368,13 @@
status = "disabled";
};
+ pwm: pwm@f20000 {
+ compatible = "marvell,berlin-pwm";
+ reg = <0xf20000 0x40>;
+ clocks = <&chip_clk CLKID_CFG>;
+ #pwm-cells = <3>;
+ };
+
apb@fc0000 {
compatible = "simple-bus";
#address-cells = <1>;
--
2.5.0
On 08/12/2015 03:51 PM, Antoine Tenart wrote:
> Add a PWM controller driver for the Marvell Berlin SoCs. This PWM
> controller has 4 channels.
>
> Signed-off-by: Antoine Tenart <[email protected]>
Antoine,
nice rework, but...
[...]
> diff --git a/drivers/pwm/pwm-berlin.c b/drivers/pwm/pwm-berlin.c
> new file mode 100644
> index 000000000000..f89e25ea5c6d
> --- /dev/null
> +++ b/drivers/pwm/pwm-berlin.c
> @@ -0,0 +1,227 @@
[...]
> +#define BERLIN_PWM_ENABLE BIT(0)
> +#define BERLIN_PWM_DISABLE 0x0
I'd drop BERLIN_PWM_DISABLE and use reg & ~BERLIN_PWM_ENABLE below.
Even if there are no more writable bits in that register, IMHO it
is good practice to affect as little bits as possible.
[...]
> +/* prescaler table: {1, 4, 8, 16, 64, 256, 1024, 4096} */
> +static const u32 prescaler_diff_table[] = {
> + 1, 4, 2, 2, 4, 4, 4, 4,
> +};
> +
> +static int berlin_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
> + int duty_ns, int period_ns)
> +{
> + struct berlin_pwm_chip *berlin_chip = to_berlin_pwm_chip(chip);
> + int ret, prescale = 0;
> + u32 val, duty, period;
> + u64 cycles;
> +
> + cycles = clk_get_rate(berlin_chip->clk);
> + cycles *= period_ns;
> + do_div(cycles, NSEC_PER_SEC);
> +
> + while (cycles > BERLIN_PWM_MAX_TCNT)
> + do_div(cycles, prescaler_diff_table[++prescale]);
> +
> + if (cycles > BERLIN_PWM_MAX_TCNT)
> + return -EINVAL;
Does my idea actually work? Did you test it with some different
pwm frequencies?
> + period = cycles;
> + cycles *= duty_ns;
> + do_div(cycles, period_ns);
> + duty = cycles;
> +
> + ret = clk_prepare_enable(berlin_chip->clk);
> + if (ret)
> + return ret;
Hmm. I understand that this may be required as the pwm framework
calls .pwm_config before .pwm_enable, but...
> +
> + spin_lock(&berlin_chip->lock);
> +
> + val = berlin_pwm_readl(berlin_chip, pwm->hwpwm, BERLIN_PWM_CONTROL);
> + val &= ~BERLIN_PWM_PRESCALE_MASK;
> + val |= prescale;
> + berlin_pwm_writel(val, berlin_chip, pwm->hwpwm, BERLIN_PWM_CONTROL);
> +
> + berlin_pwm_writel(duty, berlin_chip, pwm->hwpwm, BERLIN_PWM_DUTY);
> + berlin_pwm_writel(period, berlin_chip, pwm->hwpwm, BERLIN_PWM_TCNT);
> +
> + spin_unlock(&berlin_chip->lock);
> +
> + clk_disable_unprepare(berlin_chip->clk);
Are you sure that register contents are retained after disabling the
clock? I understand that cfg_clk is the _ip_ input clock and pwm gets
derived from it.
Actually, since cfg_clk seems to be an important, internal SoC clock
I'd say to clk_prepare_enable() in _probe() before reading any
registers and clk_disable_unprepare() on _remove() (see below).
Internal low-frequency clocks don't consume that much power that it
is worth the pain ;)
> + return 0;
> +}
> +
> +static int berlin_pwm_set_polarity(struct pwm_chip *chip,
> + struct pwm_device *pwm,
> + enum pwm_polarity polarity)
> +{
> + struct berlin_pwm_chip *berlin_chip = to_berlin_pwm_chip(chip);
> + int ret;
> + u32 val;
> +
> + ret = clk_prepare_enable(berlin_chip->clk);
> + if (ret)
> + return ret;
These would become unnecessary then.
> + spin_lock(&berlin_chip->lock);
> +
> + val = berlin_pwm_readl(berlin_chip, pwm->hwpwm, BERLIN_PWM_CONTROL);
> +
> + if (polarity == PWM_POLARITY_NORMAL)
> + val &= ~BERLIN_PWM_INVERT_POLARITY;
> + else
> + val |= BERLIN_PWM_INVERT_POLARITY;
> +
> + berlin_pwm_writel(val, berlin_chip, pwm->hwpwm, BERLIN_PWM_CONTROL);
> +
> + spin_unlock(&berlin_chip->lock);
> +
> + clk_disable_unprepare(berlin_chip->clk);
ditto.
> + return 0;
> +}
> +
> +static int berlin_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm)
> +{
> + struct berlin_pwm_chip *berlin_chip = to_berlin_pwm_chip(chip);
> + int ret;
> +
> + ret = clk_prepare_enable(berlin_chip->clk);
> + if (ret)
> + return ret;
ditto.
> + spin_lock(&berlin_chip->lock);
> + berlin_pwm_writel(BERLIN_PWM_ENABLE, berlin_chip, pwm->hwpwm, BERLIN_PWM_EN);
> + spin_unlock(&berlin_chip->lock);
> +
> + return 0;
> +}
> +
> +static void berlin_pwm_disable(struct pwm_chip *chip, struct pwm_device *pwm)
> +{
> + struct berlin_pwm_chip *berlin_chip = to_berlin_pwm_chip(chip);
> +
> + spin_lock(&berlin_chip->lock);
> + berlin_pwm_writel(BERLIN_PWM_DISABLE, berlin_chip, pwm->hwpwm, BERLIN_PWM_EN);
> + spin_unlock(&berlin_chip->lock);
> +
> + clk_disable_unprepare(berlin_chip->clk);
ditto.
> +}
> +
> +static const struct pwm_ops berlin_pwm_ops = {
> + .config = berlin_pwm_config,
> + .set_polarity = berlin_pwm_set_polarity,
> + .enable = berlin_pwm_enable,
> + .disable = berlin_pwm_disable,
> + .owner = THIS_MODULE,
> +};
> +
> +static const struct of_device_id berlin_pwm_match[] = {
> + { .compatible = "marvell,berlin-pwm" },
> + { },
> +};
> +MODULE_DEVICE_TABLE(of, berlin_pwm_match);
> +
> +static int berlin_pwm_probe(struct platform_device *pdev)
> +{
> + struct berlin_pwm_chip *pwm;
> + struct resource *res;
> + int ret;
> +
> + pwm = devm_kzalloc(&pdev->dev, sizeof(*pwm), GFP_KERNEL);
> + if (!pwm)
> + return -ENOMEM;
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + pwm->base = devm_ioremap_resource(&pdev->dev, res);
> + if (IS_ERR(pwm->base))
> + return PTR_ERR(pwm->base);
> +
> + pwm->clk = devm_clk_get(&pdev->dev, NULL);
> + if (IS_ERR(pwm->clk))
> + return PTR_ERR(pwm->clk);
> +
> + pwm->chip.dev = &pdev->dev;
> + pwm->chip.ops = &berlin_pwm_ops;
> + pwm->chip.base = -1;
> + pwm->chip.npwm = 4;
> + pwm->chip.can_sleep = true;
> + pwm->chip.of_xlate = of_pwm_xlate_with_flags;
> + pwm->chip.of_pwm_n_cells = 3;
> +
> + spin_lock_init(&pwm->lock);
add clk_prepare_enable() before adding the pwmchip...
> + ret = pwmchip_add(&pwm->chip);
> + if (ret < 0) {
> + dev_err(&pdev->dev, "Failed to add PWM chip: %d\n", ret);
> + return ret;
> + }
> +
> + platform_set_drvdata(pdev, pwm);
> +
> + return 0;
> +}
> +
> +static int berlin_pwm_remove(struct platform_device *pdev)
> +{
> + struct berlin_pwm_chip *pwm = platform_get_drvdata(pdev);
> +
> + return pwmchip_remove(&pwm->chip);
... and clk_disable_unprepare after removing it.
Besides the comments, for Berlin you get my
Acked-by: Sebastian Hesselbarth <[email protected]>
Thanks!
> +}
> +
> +static struct platform_driver berlin_pwm_driver = {
> + .probe = berlin_pwm_probe,
> + .remove = berlin_pwm_remove,
> + .driver = {
> + .name = "berlin-pwm",
> + .of_match_table = berlin_pwm_match,
> + },
> +};
> +module_platform_driver(berlin_pwm_driver);
> +
> +MODULE_AUTHOR("Antoine Tenart <[email protected]>");
> +MODULE_DESCRIPTION("Marvell Berlin PWM driver");
> +MODULE_LICENSE("GPL v2");
>
Dear Antoine,
On Wed, 12 Aug 2015 15:51:33 +0200
Antoine Tenart <[email protected]> wrote:
> Add a PWM controller driver for the Marvell Berlin SoCs. This PWM
> controller has 4 channels.
>
> Signed-off-by: Antoine Tenart <[email protected]>
> ---
> drivers/pwm/Kconfig | 9 ++
> drivers/pwm/Makefile | 1 +
> drivers/pwm/pwm-berlin.c | 227 +++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 237 insertions(+)
> create mode 100644 drivers/pwm/pwm-berlin.c
>
> diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
> index b1541f40fd8d..1773da8145b8 100644
> --- a/drivers/pwm/Kconfig
> +++ b/drivers/pwm/Kconfig
> @@ -92,6 +92,15 @@ config PWM_BCM2835
> To compile this driver as a module, choose M here: the module
> will be called pwm-bcm2835.
>
> +config PWM_BERLIN
> + tristate "Berlin PWM support"
> + depends on ARCH_BERLIN
> + help
> + PWM framework driver for Berlin.
> +
> + To compile this driver as a module, choose M here: the module
> + will be called pwm-berlin.
> +
> config PWM_BFIN
> tristate "Blackfin PWM support"
> depends on BFIN_GPTIMERS
> diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile
> index ec50eb5b5a8f..670c5fce8bbb 100644
> --- a/drivers/pwm/Makefile
> +++ b/drivers/pwm/Makefile
> @@ -6,6 +6,7 @@ obj-$(CONFIG_PWM_ATMEL_HLCDC_PWM) += pwm-atmel-hlcdc.o
> obj-$(CONFIG_PWM_ATMEL_TCB) += pwm-atmel-tcb.o
> obj-$(CONFIG_PWM_BCM_KONA) += pwm-bcm-kona.o
> obj-$(CONFIG_PWM_BCM2835) += pwm-bcm2835.o
> +obj-$(CONFIG_PWM_BERLIN) += pwm-berlin.o
> obj-$(CONFIG_PWM_BFIN) += pwm-bfin.o
> obj-$(CONFIG_PWM_CLPS711X) += pwm-clps711x.o
> obj-$(CONFIG_PWM_EP93XX) += pwm-ep93xx.o
> diff --git a/drivers/pwm/pwm-berlin.c b/drivers/pwm/pwm-berlin.c
> new file mode 100644
> index 000000000000..f89e25ea5c6d
> --- /dev/null
> +++ b/drivers/pwm/pwm-berlin.c
> @@ -0,0 +1,227 @@
> +/*
> + * Marvell Berlin PWM driver
> + *
> + * Copyright (C) 2015 Marvell Technology Group Ltd.
> + *
> + * Antoine Tenart <[email protected]>
> + *
> + * This file is licensed under the terms of the GNU General Public
> + * License version 2. This program is licensed "as is" without any
> + * warranty of any kind, whether express or implied.
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/pwm.h>
> +#include <linux/spinlock.h>
> +
> +#define BERLIN_PWM_EN 0x0
> +#define BERLIN_PWM_CONTROL 0x4
> +#define BERLIN_PWM_DUTY 0x8
> +#define BERLIN_PWM_TCNT 0xc
> +
> +#define BERLIN_PWM_ENABLE BIT(0)
> +#define BERLIN_PWM_DISABLE 0x0
> +#define BERLIN_PWM_INVERT_POLARITY BIT(3)
> +#define BERLIN_PWM_PRESCALE_MASK 0x7
> +#define BERLIN_PWM_PRESCALE_MAX 4096
> +#define BERLIN_PWM_MAX_TCNT 65535
> +
> +struct berlin_pwm_chip {
> + struct pwm_chip chip;
> + struct clk *clk;
> + void __iomem *base;
> + spinlock_t lock;
> +};
> +
> +#define to_berlin_pwm_chip(chip) \
> + container_of((chip), struct berlin_pwm_chip, chip)
> +
> +#define berlin_pwm_readl(chip, channel, offset) \
> + readl((chip)->base + (channel) * 0x10 + offset)
> +#define berlin_pwm_writel(val, chip, channel, offset) \
> + writel(val, (chip)->base + (channel) * 0x10 + offset)
It's better to use relaxed version. In some platforms with PL310 such as
BG2Q, the writel/readl will cost much time on spinning the lock if there's
heavy L2 cache maintenance ongoing, this time cost is not necessary.
And IIRC, in patch V2 review, Sebastian doesn't object to relaxed version.
Thanks,
Jisheng
> +
> +/* prescaler table: {1, 4, 8, 16, 64, 256, 1024, 4096} */
> +static const u32 prescaler_diff_table[] = {
> + 1, 4, 2, 2, 4, 4, 4, 4,
> +};
> +
> +static int berlin_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
> + int duty_ns, int period_ns)
> +{
> + struct berlin_pwm_chip *berlin_chip = to_berlin_pwm_chip(chip);
> + int ret, prescale = 0;
> + u32 val, duty, period;
> + u64 cycles;
> +
> + cycles = clk_get_rate(berlin_chip->clk);
> + cycles *= period_ns;
> + do_div(cycles, NSEC_PER_SEC);
> +
> + while (cycles > BERLIN_PWM_MAX_TCNT)
> + do_div(cycles, prescaler_diff_table[++prescale]);
> +
> + if (cycles > BERLIN_PWM_MAX_TCNT)
> + return -EINVAL;
> +
> + period = cycles;
> + cycles *= duty_ns;
> + do_div(cycles, period_ns);
> + duty = cycles;
> +
> + ret = clk_prepare_enable(berlin_chip->clk);
> + if (ret)
> + return ret;
> +
> + spin_lock(&berlin_chip->lock);
> +
> + val = berlin_pwm_readl(berlin_chip, pwm->hwpwm, BERLIN_PWM_CONTROL);
> + val &= ~BERLIN_PWM_PRESCALE_MASK;
> + val |= prescale;
> + berlin_pwm_writel(val, berlin_chip, pwm->hwpwm, BERLIN_PWM_CONTROL);
> +
> + berlin_pwm_writel(duty, berlin_chip, pwm->hwpwm, BERLIN_PWM_DUTY);
> + berlin_pwm_writel(period, berlin_chip, pwm->hwpwm, BERLIN_PWM_TCNT);
> +
> + spin_unlock(&berlin_chip->lock);
> +
> + clk_disable_unprepare(berlin_chip->clk);
> +
> + return 0;
> +}
> +
> +static int berlin_pwm_set_polarity(struct pwm_chip *chip,
> + struct pwm_device *pwm,
> + enum pwm_polarity polarity)
> +{
> + struct berlin_pwm_chip *berlin_chip = to_berlin_pwm_chip(chip);
> + int ret;
> + u32 val;
> +
> + ret = clk_prepare_enable(berlin_chip->clk);
> + if (ret)
> + return ret;
> +
> + spin_lock(&berlin_chip->lock);
> +
> + val = berlin_pwm_readl(berlin_chip, pwm->hwpwm, BERLIN_PWM_CONTROL);
> +
> + if (polarity == PWM_POLARITY_NORMAL)
> + val &= ~BERLIN_PWM_INVERT_POLARITY;
> + else
> + val |= BERLIN_PWM_INVERT_POLARITY;
> +
> + berlin_pwm_writel(val, berlin_chip, pwm->hwpwm, BERLIN_PWM_CONTROL);
> +
> + spin_unlock(&berlin_chip->lock);
> +
> + clk_disable_unprepare(berlin_chip->clk);
> +
> + return 0;
> +}
> +
> +static int berlin_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm)
> +{
> + struct berlin_pwm_chip *berlin_chip = to_berlin_pwm_chip(chip);
> + int ret;
> +
> + ret = clk_prepare_enable(berlin_chip->clk);
> + if (ret)
> + return ret;
> +
> + spin_lock(&berlin_chip->lock);
> + berlin_pwm_writel(BERLIN_PWM_ENABLE, berlin_chip, pwm->hwpwm, BERLIN_PWM_EN);
> + spin_unlock(&berlin_chip->lock);
> +
> + return 0;
> +}
> +
> +static void berlin_pwm_disable(struct pwm_chip *chip, struct pwm_device *pwm)
> +{
> + struct berlin_pwm_chip *berlin_chip = to_berlin_pwm_chip(chip);
> +
> + spin_lock(&berlin_chip->lock);
> + berlin_pwm_writel(BERLIN_PWM_DISABLE, berlin_chip, pwm->hwpwm, BERLIN_PWM_EN);
> + spin_unlock(&berlin_chip->lock);
> +
> + clk_disable_unprepare(berlin_chip->clk);
> +}
> +
> +static const struct pwm_ops berlin_pwm_ops = {
> + .config = berlin_pwm_config,
> + .set_polarity = berlin_pwm_set_polarity,
> + .enable = berlin_pwm_enable,
> + .disable = berlin_pwm_disable,
> + .owner = THIS_MODULE,
> +};
> +
> +static const struct of_device_id berlin_pwm_match[] = {
> + { .compatible = "marvell,berlin-pwm" },
> + { },
> +};
> +MODULE_DEVICE_TABLE(of, berlin_pwm_match);
> +
> +static int berlin_pwm_probe(struct platform_device *pdev)
> +{
> + struct berlin_pwm_chip *pwm;
> + struct resource *res;
> + int ret;
> +
> + pwm = devm_kzalloc(&pdev->dev, sizeof(*pwm), GFP_KERNEL);
> + if (!pwm)
> + return -ENOMEM;
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + pwm->base = devm_ioremap_resource(&pdev->dev, res);
> + if (IS_ERR(pwm->base))
> + return PTR_ERR(pwm->base);
> +
> + pwm->clk = devm_clk_get(&pdev->dev, NULL);
> + if (IS_ERR(pwm->clk))
> + return PTR_ERR(pwm->clk);
> +
> + pwm->chip.dev = &pdev->dev;
> + pwm->chip.ops = &berlin_pwm_ops;
> + pwm->chip.base = -1;
> + pwm->chip.npwm = 4;
> + pwm->chip.can_sleep = true;
> + pwm->chip.of_xlate = of_pwm_xlate_with_flags;
> + pwm->chip.of_pwm_n_cells = 3;
> +
> + spin_lock_init(&pwm->lock);
> +
> + ret = pwmchip_add(&pwm->chip);
> + if (ret < 0) {
> + dev_err(&pdev->dev, "Failed to add PWM chip: %d\n", ret);
> + return ret;
> + }
> +
> + platform_set_drvdata(pdev, pwm);
> +
> + return 0;
> +}
> +
> +static int berlin_pwm_remove(struct platform_device *pdev)
> +{
> + struct berlin_pwm_chip *pwm = platform_get_drvdata(pdev);
> +
> + return pwmchip_remove(&pwm->chip);
> +}
> +
> +static struct platform_driver berlin_pwm_driver = {
> + .probe = berlin_pwm_probe,
> + .remove = berlin_pwm_remove,
> + .driver = {
> + .name = "berlin-pwm",
> + .of_match_table = berlin_pwm_match,
> + },
> +};
> +module_platform_driver(berlin_pwm_driver);
> +
> +MODULE_AUTHOR("Antoine Tenart <[email protected]>");
> +MODULE_DESCRIPTION("Marvell Berlin PWM driver");
> +MODULE_LICENSE("GPL v2");
Sebastian,
On Wed, Aug 12, 2015 at 04:21:16PM +0200, Sebastian Hesselbarth wrote:
> On 08/12/2015 03:51 PM, Antoine Tenart wrote:
> >Add a PWM controller driver for the Marvell Berlin SoCs. This PWM
> >controller has 4 channels.
> >
> >Signed-off-by: Antoine Tenart <[email protected]>
>
> nice rework, but...
>
> [...]
> >diff --git a/drivers/pwm/pwm-berlin.c b/drivers/pwm/pwm-berlin.c
> >new file mode 100644
> >index 000000000000..f89e25ea5c6d
> >--- /dev/null
> >+++ b/drivers/pwm/pwm-berlin.c
> >@@ -0,0 +1,227 @@
> [...]
> >+#define BERLIN_PWM_ENABLE BIT(0)
> >+#define BERLIN_PWM_DISABLE 0x0
>
> I'd drop BERLIN_PWM_DISABLE and use reg & ~BERLIN_PWM_ENABLE below.
> Even if there are no more writable bits in that register, IMHO it
> is good practice to affect as little bits as possible.
Sure.
> [...]
> >+/* prescaler table: {1, 4, 8, 16, 64, 256, 1024, 4096} */
> >+static const u32 prescaler_diff_table[] = {
> >+ 1, 4, 2, 2, 4, 4, 4, 4,
> >+};
> >+
> >+static int berlin_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
> >+ int duty_ns, int period_ns)
> >+{
> >+ struct berlin_pwm_chip *berlin_chip = to_berlin_pwm_chip(chip);
> >+ int ret, prescale = 0;
> >+ u32 val, duty, period;
> >+ u64 cycles;
> >+
> >+ cycles = clk_get_rate(berlin_chip->clk);
> >+ cycles *= period_ns;
> >+ do_div(cycles, NSEC_PER_SEC);
> >+
> >+ while (cycles > BERLIN_PWM_MAX_TCNT)
> >+ do_div(cycles, prescaler_diff_table[++prescale]);
> >+
> >+ if (cycles > BERLIN_PWM_MAX_TCNT)
> >+ return -EINVAL;
>
> Does my idea actually work? Did you test it with some different
> pwm frequencies?
I tested, and it worked well with different pwm frequencies!
> >+ period = cycles;
> >+ cycles *= duty_ns;
> >+ do_div(cycles, period_ns);
> >+ duty = cycles;
> >+
> >+ ret = clk_prepare_enable(berlin_chip->clk);
> >+ if (ret)
> >+ return ret;
>
> Hmm. I understand that this may be required as the pwm framework
> calls .pwm_config before .pwm_enable, but...
>
> >+
> >+ spin_lock(&berlin_chip->lock);
> >+
> >+ val = berlin_pwm_readl(berlin_chip, pwm->hwpwm, BERLIN_PWM_CONTROL);
> >+ val &= ~BERLIN_PWM_PRESCALE_MASK;
> >+ val |= prescale;
> >+ berlin_pwm_writel(val, berlin_chip, pwm->hwpwm, BERLIN_PWM_CONTROL);
> >+
> >+ berlin_pwm_writel(duty, berlin_chip, pwm->hwpwm, BERLIN_PWM_DUTY);
> >+ berlin_pwm_writel(period, berlin_chip, pwm->hwpwm, BERLIN_PWM_TCNT);
> >+
> >+ spin_unlock(&berlin_chip->lock);
> >+
> >+ clk_disable_unprepare(berlin_chip->clk);
>
> Are you sure that register contents are retained after disabling the
> clock? I understand that cfg_clk is the _ip_ input clock and pwm gets
> derived from it.
>
> Actually, since cfg_clk seems to be an important, internal SoC clock
> I'd say to clk_prepare_enable() in _probe() before reading any
> registers and clk_disable_unprepare() on _remove() (see below).
I'll update.
>
> Internal low-frequency clocks don't consume that much power that it
> is worth the pain ;)
>
> >+ return 0;
> >+}
> >+
> >+static int berlin_pwm_set_polarity(struct pwm_chip *chip,
> >+ struct pwm_device *pwm,
> >+ enum pwm_polarity polarity)
> >+{
> >+ struct berlin_pwm_chip *berlin_chip = to_berlin_pwm_chip(chip);
> >+ int ret;
> >+ u32 val;
> >+
> >+ ret = clk_prepare_enable(berlin_chip->clk);
> >+ if (ret)
> >+ return ret;
>
> These would become unnecessary then.
>
> >+ spin_lock(&berlin_chip->lock);
> >+
> >+ val = berlin_pwm_readl(berlin_chip, pwm->hwpwm, BERLIN_PWM_CONTROL);
> >+
> >+ if (polarity == PWM_POLARITY_NORMAL)
> >+ val &= ~BERLIN_PWM_INVERT_POLARITY;
> >+ else
> >+ val |= BERLIN_PWM_INVERT_POLARITY;
> >+
> >+ berlin_pwm_writel(val, berlin_chip, pwm->hwpwm, BERLIN_PWM_CONTROL);
> >+
> >+ spin_unlock(&berlin_chip->lock);
> >+
> >+ clk_disable_unprepare(berlin_chip->clk);
>
> ditto.
>
> >+ return 0;
> >+}
> >+
> >+static int berlin_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm)
> >+{
> >+ struct berlin_pwm_chip *berlin_chip = to_berlin_pwm_chip(chip);
> >+ int ret;
> >+
> >+ ret = clk_prepare_enable(berlin_chip->clk);
> >+ if (ret)
> >+ return ret;
>
> ditto.
>
> >+ spin_lock(&berlin_chip->lock);
> >+ berlin_pwm_writel(BERLIN_PWM_ENABLE, berlin_chip, pwm->hwpwm, BERLIN_PWM_EN);
> >+ spin_unlock(&berlin_chip->lock);
> >+
> >+ return 0;
> >+}
> >+
> >+static void berlin_pwm_disable(struct pwm_chip *chip, struct pwm_device *pwm)
> >+{
> >+ struct berlin_pwm_chip *berlin_chip = to_berlin_pwm_chip(chip);
> >+
> >+ spin_lock(&berlin_chip->lock);
> >+ berlin_pwm_writel(BERLIN_PWM_DISABLE, berlin_chip, pwm->hwpwm, BERLIN_PWM_EN);
> >+ spin_unlock(&berlin_chip->lock);
> >+
> >+ clk_disable_unprepare(berlin_chip->clk);
>
> ditto.
>
> >+}
> >+
> >+static const struct pwm_ops berlin_pwm_ops = {
> >+ .config = berlin_pwm_config,
> >+ .set_polarity = berlin_pwm_set_polarity,
> >+ .enable = berlin_pwm_enable,
> >+ .disable = berlin_pwm_disable,
> >+ .owner = THIS_MODULE,
> >+};
> >+
> >+static const struct of_device_id berlin_pwm_match[] = {
> >+ { .compatible = "marvell,berlin-pwm" },
> >+ { },
> >+};
> >+MODULE_DEVICE_TABLE(of, berlin_pwm_match);
> >+
> >+static int berlin_pwm_probe(struct platform_device *pdev)
> >+{
> >+ struct berlin_pwm_chip *pwm;
> >+ struct resource *res;
> >+ int ret;
> >+
> >+ pwm = devm_kzalloc(&pdev->dev, sizeof(*pwm), GFP_KERNEL);
> >+ if (!pwm)
> >+ return -ENOMEM;
> >+
> >+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> >+ pwm->base = devm_ioremap_resource(&pdev->dev, res);
> >+ if (IS_ERR(pwm->base))
> >+ return PTR_ERR(pwm->base);
> >+
> >+ pwm->clk = devm_clk_get(&pdev->dev, NULL);
> >+ if (IS_ERR(pwm->clk))
> >+ return PTR_ERR(pwm->clk);
> >+
> >+ pwm->chip.dev = &pdev->dev;
> >+ pwm->chip.ops = &berlin_pwm_ops;
> >+ pwm->chip.base = -1;
> >+ pwm->chip.npwm = 4;
> >+ pwm->chip.can_sleep = true;
> >+ pwm->chip.of_xlate = of_pwm_xlate_with_flags;
> >+ pwm->chip.of_pwm_n_cells = 3;
> >+
> >+ spin_lock_init(&pwm->lock);
>
> add clk_prepare_enable() before adding the pwmchip...
>
> >+ ret = pwmchip_add(&pwm->chip);
> >+ if (ret < 0) {
> >+ dev_err(&pdev->dev, "Failed to add PWM chip: %d\n", ret);
> >+ return ret;
> >+ }
> >+
> >+ platform_set_drvdata(pdev, pwm);
> >+
> >+ return 0;
> >+}
> >+
> >+static int berlin_pwm_remove(struct platform_device *pdev)
> >+{
> >+ struct berlin_pwm_chip *pwm = platform_get_drvdata(pdev);
> >+
> >+ return pwmchip_remove(&pwm->chip);
>
> ... and clk_disable_unprepare after removing it.
>
> Besides the comments, for Berlin you get my
>
> Acked-by: Sebastian Hesselbarth <[email protected]>
Thanks!
Antoine
>
> >+}
> >+
> >+static struct platform_driver berlin_pwm_driver = {
> >+ .probe = berlin_pwm_probe,
> >+ .remove = berlin_pwm_remove,
> >+ .driver = {
> >+ .name = "berlin-pwm",
> >+ .of_match_table = berlin_pwm_match,
> >+ },
> >+};
> >+module_platform_driver(berlin_pwm_driver);
> >+
> >+MODULE_AUTHOR("Antoine Tenart <[email protected]>");
> >+MODULE_DESCRIPTION("Marvell Berlin PWM driver");
> >+MODULE_LICENSE("GPL v2");
> >
--
Antoine T?nart, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com