[PATCH v3 0/5] ARM: berlin: PWM support

[PATCH v3 0/5] ARM: berlin: PWM support

[Antoine Tenart]

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

[PATCH v3 1/5] pwm: add the Berlin pwm controller driver

[Antoine Tenart]

Add a PWM controller driver for the Marvell Berlin SoCs. This PWM
controller has 4 channels.

Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
---
 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 <antoine.tenart@free-electrons.com>
+ *
+ * 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 <antoine.tenart@free-electrons.com>");
+MODULE_DESCRIPTION("Marvell Berlin PWM driver");
+MODULE_LICENSE("GPL v2");
-- 
2.5.0

[PATCH v3 1/5] pwm: add the Berlin pwm controller driver

[Sebastian Hesselbarth]

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 <antoine.tenart@free-electrons.com>

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 <sebastian.hesselbarth@gmail.com>

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 <antoine.tenart@free-electrons.com>");
> +MODULE_DESCRIPTION("Marvell Berlin PWM driver");
> +MODULE_LICENSE("GPL v2");
>

[PATCH v3 1/5] pwm: add the Berlin pwm controller driver

[Antoine Tenart]

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 <antoine.tenart@free-electrons.com>
> 
> 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 <sebastian.hesselbarth@gmail.com>

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 <antoine.tenart@free-electrons.com>");
> >+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

[PATCH v3 1/5] pwm: add the Berlin pwm controller driver

[Jisheng Zhang]

Dear Antoine,

On Wed, 12 Aug 2015 15:51:33 +0200
Antoine Tenart <antoine.tenart@free-electrons.com> wrote:

> Add a PWM controller driver for the Marvell Berlin SoCs. This PWM
> controller has 4 channels.
> 
> Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
> ---
>  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 <antoine.tenart@free-electrons.com>
> + *
> + * 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 <antoine.tenart@free-electrons.com>");
> +MODULE_DESCRIPTION("Marvell Berlin PWM driver");
> +MODULE_LICENSE("GPL v2");

[PATCH v3 2/5] Documentation: bindings: document the Berlin PWM driver

[Antoine Tenart]

Following the addition of a Berlin PWM driver, this patch adds the
corresponding documentation.

Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
Acked-by: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
---
 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 at f7f20000 {
+	compatible = "marvell,berlin-pwm";
+	reg = <0xf7f20000 0x40>;
+	clocks = <&chip_clk CLKID_CFG>;
+	#pwm-cells = <3>;
+}
-- 
2.5.0

[PATCH v3 3/5] ARM: berlin: add a PWM node on the BG2Q

[Antoine Tenart]

This patch adds a PWM node in the Berlin BG2Q device tree, using the
newly added Berlin PWM driver.

Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
---
 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 at f20000 {
+			compatible = "marvell,berlin-pwm";
+			reg = <0xf20000 0x40>;
+			clocks = <&chip_clk CLKID_CFG>;
+			#pwm-cells = <3>;
+		};
+
 		apb at fc0000 {
 			compatible = "simple-bus";
 			#address-cells = <1>;
-- 
2.5.0

[PATCH v3 4/5] ARM: berlin: add a PWM node on the BG2

[Antoine Tenart]

This patch adds a PWM node in the Berlin BG2 device tree, using the
newly added Berlin PWM driver.

Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
---
 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 at f20000 {
+			compatible = "marvell,berlin-pwm";
+			reg = <0xf20000 0x40>;
+			clocks = <&chip_clk CLKID_CFG>;
+			#pwm-cells = <3>;
+		};
+
 		apb at fc0000 {
 			compatible = "simple-bus";
 			#address-cells = <1>;
-- 
2.5.0

[PATCH v3 5/5] ARM: berlin: add a PWM node on the BG2CD

[Antoine Tenart]

This patch adds a PWM node in the Berlin BG2CD device tree, using the
newly added Berlin PWM driver.

Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
---
 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 at f20000 {
+			compatible = "marvell,berlin-pwm";
+			reg = <0xf20000 0x40>;
+			clocks = <&chip_clk CLKID_CFG>;
+			#pwm-cells = <3>;
+		};
+
 		apb at fc0000 {
 			compatible = "simple-bus";
 			#address-cells = <1>;
-- 
2.5.0