2017-03-24 14:18:58

by Ralph Sennhauser

[permalink] [raw]
Subject: [PATCH v3 0/4] gpio: mvebu: Add PWM fan support

Hi everyone

This patch series was originally submitted by Andrew Lunn but got stalled.
I picked up the series and addressed what was discussed for the earlier
submission with some helpful input from Andrew. Hopefully this time support
for the PWM fan as found on Linksys WRT1900AC (Mamba) will make it in.

Implementing as an MFD was discarded due to backward compatibility. The
original discussion can be read at [1].

This series depends on two cleanup series. The first series [2] ("gpio:
mvebu: preparatory cleanup for pwm-fan support") is in linux next-20170320.
The second one [3] ("gpio: mvebu: use BIT macro instead of bit shifting")
got applied but didn't make it's way into linux-next yet.

See pending items below!

Ralph

[1] https://patchwork.ozlabs.org/patch/427287/
[2] https://lkml.org/lkml/2017/3/16/62
[3] https://lkml.org/lkml/2017/3/17/493

---

Changes v2->v3:
Patch 1/4 gpio: mvebu: Add limited PWM support:
* drop pin from mvebu_pwn, can be infered (suggested by Thierry Reding)
* rename pwm to mvpwm so pwm can be used for pwm_device as in the API,
avoids some mental gymnastic.
* drop id from struct mvebu_gpio_chip, select blink counter in
mvebu_pwm_probe for all lines instead. We do not care about the
unused ones. I think a clear improvement in readability.
Makes coming up with a good comment simple as well.
* Switch to new atomic PWM API (suggested by Thierry Reding)
* rename use mvebu_gpioreg_blink_select to
mvebu_gpioreg_blink_counter_select.
* mark *_suspend() / *_resume() as __maybe_unused (suggested by Linus
Walleij)
* document #pwm-cells = 1 (suggested by Thierry Reding)
Patch 2/4 mvebu: xp: Add PWM properties to .dtsi files
* add missing reg-names / #pwm-cell properties to
armada-xp-mv78260.dtsi gpio1 node
* set pwm-cells = 1 (suggested by Thierry Reding)
All:
* always uppercase GPIO/PWM in prose (suggested by Thierry Reding)

Changes v1 -> v2:
Patch 1/4 gpio: mvebu: Add limited PWM support:
* use BIT macro (suggested by Linus Walleij)
* move id from struct mvebu_pwm to struct mvebu_gpio_chip, implement
blink select as if else and comment on the chip id for code clarity
(to accommodate Linus Walleijs request for a code clarification /
comment. If you can word it better I'm all ears.)
* Move function comment mvebu_pwm_probe into the function itself.

---

Pending:
* npwm = 1? (suggested by Thierry Reding)
Like Andrew who commented on this item for v1 I'm not yet clear how
to achieve that. So while a lie a rather convenient one.
* Tested-by: Andrew Lunn for v1 needs renewing
* Needs ACK from Thierry Reding to be merged via linux-gpio tree by Linus
Walleij. (fine with the general approach, requested changes)

---

Andrew Lunn (4):
gpio: mvebu: Add limited PWM support
mvebu: xp: Add PWM properties to .dtsi files
ARM: mvebu: Enable SENSORS_PWM_FAN in defconfig
mvebu: wrt1900ac: Use pwm-fan rather than gpio-fan

.../devicetree/bindings/gpio/gpio-mvebu.txt | 29 ++
MAINTAINERS | 2 +
arch/arm/boot/dts/armada-370.dtsi | 10 +-
arch/arm/boot/dts/armada-xp-linksys-mamba.dts | 8 +-
arch/arm/boot/dts/armada-xp-mv78230.dtsi | 10 +-
arch/arm/boot/dts/armada-xp-mv78260.dtsi | 10 +-
arch/arm/boot/dts/armada-xp-mv78460.dtsi | 10 +-
arch/arm/configs/mvebu_v7_defconfig | 2 +
drivers/gpio/gpio-mvebu.c | 317 ++++++++++++++++++++-
9 files changed, 373 insertions(+), 25 deletions(-)

--
2.10.2


2017-03-24 14:18:53

by Ralph Sennhauser

[permalink] [raw]
Subject: [PATCH v3 1/4] gpio: mvebu: Add limited PWM support

From: Andrew Lunn <[email protected]>

Armada 370/XP devices can 'blink' GPIO lines with a configurable on
and off period. This can be modelled as a PWM.

However, there are only two sets of PWM configuration registers for
all the GPIO lines. This driver simply allows a single GPIO line per
GPIO chip of 32 lines to be used as a PWM. Attempts to use more return
EBUSY.

Due to the interleaving of registers it is not simple to separate the
PWM driver from the GPIO driver. Thus the GPIO driver has been
extended with a PWM driver.

Signed-off-by: Andrew Lunn <[email protected]>
URL: https://patchwork.ozlabs.org/patch/427287/
URL: https://patchwork.ozlabs.org/patch/427295/
[Ralph Sennhauser:
* Port forward
* Merge PWM portion into gpio-mvebu.c
* Switch to atomic PWM API
* Merge documentation patch
* Update MAINTAINERS]
Signed-off-by: Ralph Sennhauser <[email protected]>
---
.../devicetree/bindings/gpio/gpio-mvebu.txt | 29 ++
MAINTAINERS | 2 +
drivers/gpio/gpio-mvebu.c | 317 ++++++++++++++++++++-
3 files changed, 336 insertions(+), 12 deletions(-)

diff --git a/Documentation/devicetree/bindings/gpio/gpio-mvebu.txt b/Documentation/devicetree/bindings/gpio/gpio-mvebu.txt
index a6f3bec..748bbc7 100644
--- a/Documentation/devicetree/bindings/gpio/gpio-mvebu.txt
+++ b/Documentation/devicetree/bindings/gpio/gpio-mvebu.txt
@@ -38,6 +38,21 @@ Required properties:
- #gpio-cells: Should be two. The first cell is the pin number. The
second cell is reserved for flags, unused at the moment.

+Optional properties:
+
+In order to use the gpio lines in PWM mode, some additional optional
+properties are required. Only Armada 370 and XP support these properties.
+
+- reg: an additional register set is needed, for the GPIO Blink
+ Counter on/off registers.
+
+- reg-names: Must contain an entry "pwm" corresponding to the
+ additional register range needed for pwm operation.
+
+- #pwm-cells: Should be one. The first cell is the pin number.
+
+- clocks: Must be a phandle to the clock for the gpio controller.
+
Example:

gpio0: gpio@d0018100 {
@@ -51,3 +66,17 @@ Example:
#interrupt-cells = <2>;
interrupts = <16>, <17>, <18>, <19>;
};
+
+ gpio1: gpio@18140 {
+ compatible = "marvell,orion-gpio";
+ reg = <0x18140 0x40>, <0x181c8 0x08>;
+ reg-names = "gpio", "pwm";
+ ngpios = <17>;
+ gpio-controller;
+ #gpio-cells = <2>;
+ #pwm-cells = <2>;
+ interrupt-controller;
+ #interrupt-cells = <2>;
+ interrupts = <87>, <88>, <89>;
+ clocks = <&coreclk 0>;
+ };
diff --git a/MAINTAINERS b/MAINTAINERS
index c8f88d0..7fd2289 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -10291,6 +10291,8 @@ F: include/linux/pwm.h
F: drivers/pwm/
F: drivers/video/backlight/pwm_bl.c
F: include/linux/pwm_backlight.h
+F: drivers/gpio/gpio-mvebu.c
+F: Documentation/devicetree/bindings/gpio/gpio-mvebu.txt

PXA2xx/PXA3xx SUPPORT
M: Daniel Mack <[email protected]>
diff --git a/drivers/gpio/gpio-mvebu.c b/drivers/gpio/gpio-mvebu.c
index fae4db6..46b5151 100644
--- a/drivers/gpio/gpio-mvebu.c
+++ b/drivers/gpio/gpio-mvebu.c
@@ -42,22 +42,34 @@
#include <linux/io.h>
#include <linux/of_irq.h>
#include <linux/of_device.h>
+#include <linux/pwm.h>
#include <linux/clk.h>
#include <linux/pinctrl/consumer.h>
#include <linux/irqchip/chained_irq.h>
+#include <linux/platform_device.h>
#include <linux/bitops.h>

+#include "gpiolib.h"
+
/*
* GPIO unit register offsets.
*/
-#define GPIO_OUT_OFF 0x0000
-#define GPIO_IO_CONF_OFF 0x0004
-#define GPIO_BLINK_EN_OFF 0x0008
-#define GPIO_IN_POL_OFF 0x000c
-#define GPIO_DATA_IN_OFF 0x0010
-#define GPIO_EDGE_CAUSE_OFF 0x0014
-#define GPIO_EDGE_MASK_OFF 0x0018
-#define GPIO_LEVEL_MASK_OFF 0x001c
+#define GPIO_OUT_OFF 0x0000
+#define GPIO_IO_CONF_OFF 0x0004
+#define GPIO_BLINK_EN_OFF 0x0008
+#define GPIO_IN_POL_OFF 0x000c
+#define GPIO_DATA_IN_OFF 0x0010
+#define GPIO_EDGE_CAUSE_OFF 0x0014
+#define GPIO_EDGE_MASK_OFF 0x0018
+#define GPIO_LEVEL_MASK_OFF 0x001c
+#define GPIO_BLINK_CNT_SELECT_OFF 0x0020
+
+/*
+ * PWM register offsets.
+ */
+#define PWM_BLINK_ON_DURATION_OFF 0x0
+#define PWM_BLINK_OFF_DURATION_OFF 0x4
+

/* The MV78200 has per-CPU registers for edge mask and level mask */
#define GPIO_EDGE_MASK_MV78200_OFF(cpu) ((cpu) ? 0x30 : 0x18)
@@ -78,6 +90,20 @@

#define MVEBU_MAX_GPIO_PER_BANK 32

+struct mvebu_pwm {
+ void __iomem *membase;
+ unsigned long clk_rate;
+ bool used;
+ struct pwm_chip chip;
+ spinlock_t lock;
+ struct mvebu_gpio_chip *mvchip;
+
+ /* Used to preserve GPIO/PWM registers across suspend/resume */
+ u32 blink_select;
+ u32 blink_on_duration;
+ u32 blink_off_duration;
+};
+
struct mvebu_gpio_chip {
struct gpio_chip chip;
spinlock_t lock;
@@ -87,6 +113,10 @@ struct mvebu_gpio_chip {
struct irq_domain *domain;
int soc_variant;

+ /* Used for PWM support */
+ struct clk *clk;
+ struct mvebu_pwm *mvpwm;
+
/* Used to preserve GPIO registers across suspend/resume */
u32 out_reg;
u32 io_conf_reg;
@@ -110,6 +140,12 @@ static void __iomem *mvebu_gpioreg_blink(struct mvebu_gpio_chip *mvchip)
return mvchip->membase + GPIO_BLINK_EN_OFF;
}

+static void __iomem *mvebu_gpioreg_blink_counter_select(struct mvebu_gpio_chip
+ *mvchip)
+{
+ return mvchip->membase + GPIO_BLINK_CNT_SELECT_OFF;
+}
+
static void __iomem *mvebu_gpioreg_io_conf(struct mvebu_gpio_chip *mvchip)
{
return mvchip->membase + GPIO_IO_CONF_OFF;
@@ -181,6 +217,20 @@ static void __iomem *mvebu_gpioreg_level_mask(struct mvebu_gpio_chip *mvchip)
}

/*
+ * Functions returning addresses of individual registers for a given
+ * PWM controller.
+ */
+static void __iomem *mvebu_pwmreg_blink_on_duration(struct mvebu_pwm *pwm)
+{
+ return pwm->membase + PWM_BLINK_ON_DURATION_OFF;
+}
+
+static void __iomem *mvebu_pwmreg_blink_off_duration(struct mvebu_pwm *pwm)
+{
+ return pwm->membase + PWM_BLINK_OFF_DURATION_OFF;
+}
+
+/*
* Functions implementing the gpio_chip methods
*/
static void mvebu_gpio_set(struct gpio_chip *chip, unsigned int pin, int value)
@@ -484,6 +534,240 @@ static void mvebu_gpio_irq_handler(struct irq_desc *desc)
chained_irq_exit(chip, desc);
}

+/*
+ * Functions implementing the pwm_chip methods
+ */
+static struct mvebu_pwm *to_mvebu_pwm(struct pwm_chip *chip)
+{
+ return container_of(chip, struct mvebu_pwm, chip);
+}
+
+static int mvebu_pwm_request(struct pwm_chip *chip, struct pwm_device *pwm)
+{
+ struct mvebu_pwm *mvpwm = to_mvebu_pwm(chip);
+ struct gpio_desc *desc = gpio_to_desc(pwm->pwm);
+ unsigned long flags;
+ int ret = 0;
+
+ spin_lock_irqsave(&mvpwm->lock, flags);
+ if (mvpwm->used) {
+ ret = -EBUSY;
+ } else {
+ if (!desc) {
+ ret = -ENODEV;
+ goto out;
+ }
+ ret = gpiod_request(desc, "mvebu-pwm");
+ if (ret)
+ goto out;
+
+ ret = gpiod_direction_output(desc, 0);
+ if (ret) {
+ gpiod_free(desc);
+ goto out;
+ }
+
+ mvpwm->used = true;
+ }
+
+out:
+ spin_unlock_irqrestore(&mvpwm->lock, flags);
+ return ret;
+}
+
+static void mvebu_pwm_free(struct pwm_chip *chip, struct pwm_device *pwm)
+{
+ struct mvebu_pwm *mvpwm = to_mvebu_pwm(chip);
+ struct gpio_desc *desc = gpio_to_desc(pwm->pwm);
+ unsigned long flags;
+
+ spin_lock_irqsave(&mvpwm->lock, flags);
+ gpiod_free(desc);
+ mvpwm->used = false;
+ spin_unlock_irqrestore(&mvpwm->lock, flags);
+}
+
+static void mvebu_pwm_get_state(struct pwm_chip *chip,
+ struct pwm_device *pwm,
+ struct pwm_state *state) {
+
+ struct mvebu_pwm *mvpwm = to_mvebu_pwm(chip);
+ struct mvebu_gpio_chip *mvchip = mvpwm->mvchip;
+ unsigned long long val;
+ unsigned long flags;
+ u32 u;
+
+ spin_lock_irqsave(&mvpwm->lock, flags);
+
+ val = (unsigned long long)
+ readl_relaxed(mvebu_pwmreg_blink_on_duration);
+ val *= NSEC_PER_SEC;
+ do_div(val, mvpwm->clk_rate);
+ if (val > UINT_MAX)
+ state->duty_cycle = UINT_MAX;
+ else if (val)
+ state->duty_cycle = val;
+ else
+ state->duty_cycle = 1;
+
+ val = (unsigned long long)
+ readl_relaxed(mvebu_pwmreg_blink_on_duration);
+ val *= NSEC_PER_SEC;
+ do_div(val, mvpwm->clk_rate);
+ if (val < state->duty_cycle)
+ state->period = 1;
+ else {
+ val -= state->duty_cycle;
+ if (val > UINT_MAX)
+ state->period = UINT_MAX;
+ else if (val)
+ state->period = val;
+ else
+ state->period = 1;
+ }
+
+ u = readl_relaxed(mvebu_gpioreg_blink(mvchip));
+ if (u)
+ state->enabled = true;
+ else
+ state->enabled = false;
+
+ spin_unlock_irqrestore(&mvpwm->lock, flags);
+}
+
+static int mvebu_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
+ struct pwm_state *state)
+{
+ struct mvebu_pwm *mvpwm = to_mvebu_pwm(chip);
+ struct mvebu_gpio_chip *mvchip = mvpwm->mvchip;
+ unsigned long long val;
+ unsigned long flags;
+ unsigned int on, off;
+
+ val = (unsigned long long) mvpwm->clk_rate * state->duty_cycle;
+ do_div(val, NSEC_PER_SEC);
+ if (val > UINT_MAX)
+ return -EINVAL;
+ if (val)
+ on = val;
+ else
+ on = 1;
+
+ val = (unsigned long long) mvpwm->clk_rate *
+ (state->period - state->duty_cycle);
+ do_div(val, NSEC_PER_SEC);
+ if (val > UINT_MAX)
+ return -EINVAL;
+ if (val)
+ off = val;
+ else
+ off = 1;
+
+ spin_lock_irqsave(&mvpwm->lock, flags);
+
+ writel_relaxed(on, mvebu_pwmreg_blink_on_duration(mvpwm));
+ writel_relaxed(off, mvebu_pwmreg_blink_off_duration(mvpwm));
+ if (state->enabled)
+ mvebu_gpio_blink(&mvchip->chip, pwm->hwpwm, 1);
+ else
+ mvebu_gpio_blink(&mvchip->chip, pwm->hwpwm, 0);
+
+ spin_unlock_irqrestore(&mvpwm->lock, flags);
+
+ return 0;
+}
+
+static const struct pwm_ops mvebu_pwm_ops = {
+ .request = mvebu_pwm_request,
+ .free = mvebu_pwm_free,
+ .get_state = mvebu_pwm_get_state,
+ .apply = mvebu_pwm_apply,
+ .owner = THIS_MODULE,
+};
+
+static void __maybe_unused mvebu_pwm_suspend(struct mvebu_gpio_chip *mvchip)
+{
+ struct mvebu_pwm *mvpwm = mvchip->mvpwm;
+
+ mvpwm->blink_select =
+ readl_relaxed(mvebu_gpioreg_blink_counter_select(mvchip));
+ mvpwm->blink_on_duration =
+ readl_relaxed(mvebu_pwmreg_blink_on_duration(mvpwm));
+ mvpwm->blink_off_duration =
+ readl_relaxed(mvebu_pwmreg_blink_off_duration(mvpwm));
+}
+
+static void __maybe_unused mvebu_pwm_resume(struct mvebu_gpio_chip *mvchip)
+{
+ struct mvebu_pwm *mvpwm = mvchip->mvpwm;
+
+ writel_relaxed(mvpwm->blink_select,
+ mvebu_gpioreg_blink_counter_select(mvchip));
+ writel_relaxed(mvpwm->blink_on_duration,
+ mvebu_pwmreg_blink_on_duration(mvpwm));
+ writel_relaxed(mvpwm->blink_off_duration,
+ mvebu_pwmreg_blink_off_duration(mvpwm));
+}
+
+static int mvebu_pwm_probe(struct platform_device *pdev,
+ struct mvebu_gpio_chip *mvchip,
+ int id)
+{
+ struct device *dev = &pdev->dev;
+ struct mvebu_pwm *mvpwm;
+ struct resource *res;
+
+ /*
+ * Armada 370/XP has simple PWM support for GPIO lines. Other SoCs
+ * don't have this hardware. So if we don't have the necessary
+ * resource, it is not an error.
+ */
+ res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "pwm");
+ if (!res)
+ return 0;
+ /*
+ * There are only two sets of PWM configuration registers for all
+ * the GPIO lines on those SoCs. Use set A for all lines of GPIO
+ * chip with id 0, B for GPIO chip with id 1. Don't allow further
+ * GPIO chips to be used for PWM.
+ */
+ if (id == 0)
+ writel_relaxed(0, mvebu_gpioreg_blink_counter_select(mvchip));
+ else if (id == 1)
+ writel_relaxed(U32_MAX,
+ mvebu_gpioreg_blink_counter_select(mvchip));
+ else
+ return -EINVAL;
+
+ mvpwm = devm_kzalloc(dev, sizeof(struct mvebu_pwm), GFP_KERNEL);
+ if (!mvpwm)
+ return -ENOMEM;
+ mvchip->mvpwm = mvpwm;
+ mvpwm->mvchip = mvchip;
+
+ mvpwm->membase = devm_ioremap_resource(dev, res);
+ if (IS_ERR(mvpwm->membase))
+ return PTR_ERR(mvpwm->membase);
+
+ if (IS_ERR(mvchip->clk))
+ return PTR_ERR(mvchip->clk);
+
+ mvpwm->clk_rate = clk_get_rate(mvchip->clk);
+ if (!mvpwm->clk_rate) {
+ dev_err(dev, "failed to get clock rate\n");
+ return -EINVAL;
+ }
+
+ mvpwm->chip.dev = dev;
+ mvpwm->chip.ops = &mvebu_pwm_ops;
+ mvpwm->chip.base = mvchip->chip.base;
+ mvpwm->chip.npwm = mvchip->chip.ngpio;
+
+ spin_lock_init(&mvpwm->lock);
+
+ return pwmchip_add(&mvpwm->chip);
+}
+
#ifdef CONFIG_DEBUG_FS
#include <linux/seq_file.h>

@@ -600,6 +884,9 @@ static int mvebu_gpio_suspend(struct platform_device *pdev, pm_message_t state)
BUG();
}

+ if (IS_ENABLED(CONFIG_PWM))
+ mvebu_pwm_suspend(mvchip);
+
return 0;
}

@@ -643,6 +930,9 @@ static int mvebu_gpio_resume(struct platform_device *pdev)
BUG();
}

+ if (IS_ENABLED(CONFIG_PWM))
+ mvebu_pwm_resume(mvchip);
+
return 0;
}

@@ -654,7 +944,6 @@ static int mvebu_gpio_probe(struct platform_device *pdev)
struct resource *res;
struct irq_chip_generic *gc;
struct irq_chip_type *ct;
- struct clk *clk;
unsigned int ngpios;
bool have_irqs;
int soc_variant;
@@ -688,10 +977,10 @@ static int mvebu_gpio_probe(struct platform_device *pdev)
return id;
}

- clk = devm_clk_get(&pdev->dev, NULL);
+ mvchip->clk = devm_clk_get(&pdev->dev, NULL);
/* Not all SoCs require a clock.*/
- if (!IS_ERR(clk))
- clk_prepare_enable(clk);
+ if (!IS_ERR(mvchip->clk))
+ clk_prepare_enable(mvchip->clk);

mvchip->soc_variant = soc_variant;
mvchip->chip.label = dev_name(&pdev->dev);
@@ -822,6 +1111,10 @@ static int mvebu_gpio_probe(struct platform_device *pdev)
mvchip);
}

+ /* Armada 370/XP has simple PWM support for GPIO lines */
+ if (IS_ENABLED(CONFIG_PWM))
+ return mvebu_pwm_probe(pdev, mvchip, id);
+
return 0;

err_domain:
--
2.10.2

2017-03-24 14:19:25

by Ralph Sennhauser

[permalink] [raw]
Subject: [PATCH v3 4/4] mvebu: wrt1900ac: Use pwm-fan rather than gpio-fan

From: Andrew Lunn <[email protected]>

The mvebu GPIO driver can also perform PWM on some pins. Use the pwm-fan
driver to control the fan of the WRT1900AC, giving us finer grained control
over its speed and hence noise.

Signed-off-by: Andrew Lunn <[email protected]>
URL: https://patchwork.ozlabs.org/patch/427291/
Signed-off-by: Ralph Sennhauser <[email protected]>
---
arch/arm/boot/dts/armada-xp-linksys-mamba.dts | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/arch/arm/boot/dts/armada-xp-linksys-mamba.dts b/arch/arm/boot/dts/armada-xp-linksys-mamba.dts
index 3744ba3..836b275e 100644
--- a/arch/arm/boot/dts/armada-xp-linksys-mamba.dts
+++ b/arch/arm/boot/dts/armada-xp-linksys-mamba.dts
@@ -293,13 +293,11 @@
};
};

- gpio_fan {
+ pwm_fan {
/* SUNON HA4010V4-0000-C99 */
- compatible = "gpio-fan";
- gpios = <&gpio0 24 0>;

- gpio-fan,speed-map = <0 0
- 4500 1>;
+ compatible = "pwm-fan";
+ pwms = <&gpio0 24 4000 0>;
};

dsa {
--
2.10.2

2017-03-24 14:19:41

by Ralph Sennhauser

[permalink] [raw]
Subject: [PATCH v3 2/4] mvebu: xp: Add PWM properties to .dtsi files

From: Andrew Lunn <[email protected]>

Add properties to the GPIO nodes to allow them to be also used as PWM
lines.

Signed-off-by: Andrew Lunn <[email protected]>
URL: https://patchwork.ozlabs.org/patch/427294/
[Ralph Sennhauser:
* Use #pwm-cells = <1>
* Add missing reg-names / #pwm-cell properties to armada-xp-mv78260.dtsi
'gpio1' node]
Signed-off-by: Ralph Sennhauser <[email protected]>
---
arch/arm/boot/dts/armada-370.dtsi | 10 ++++++++--
arch/arm/boot/dts/armada-xp-mv78230.dtsi | 10 ++++++++--
arch/arm/boot/dts/armada-xp-mv78260.dtsi | 10 ++++++++--
arch/arm/boot/dts/armada-xp-mv78460.dtsi | 10 ++++++++--
4 files changed, 32 insertions(+), 8 deletions(-)

diff --git a/arch/arm/boot/dts/armada-370.dtsi b/arch/arm/boot/dts/armada-370.dtsi
index cc011c8..de7295c 100644
--- a/arch/arm/boot/dts/armada-370.dtsi
+++ b/arch/arm/boot/dts/armada-370.dtsi
@@ -138,24 +138,30 @@

gpio0: gpio@18100 {
compatible = "marvell,orion-gpio";
- reg = <0x18100 0x40>;
+ reg = <0x18100 0x40>, <0x181c0 0x08>;
+ reg-names = "gpio", "pwm";
ngpios = <32>;
gpio-controller;
#gpio-cells = <2>;
+ #pwm-cells = <1>;
interrupt-controller;
#interrupt-cells = <2>;
interrupts = <82>, <83>, <84>, <85>;
+ clocks = <&coreclk 0>;
};

gpio1: gpio@18140 {
compatible = "marvell,orion-gpio";
- reg = <0x18140 0x40>;
+ reg = <0x18140 0x40>, <0x181c8 0x08>;
+ reg-names = "gpio", "pwm";
ngpios = <32>;
gpio-controller;
#gpio-cells = <2>;
+ #pwm-cells = <1>;
interrupt-controller;
#interrupt-cells = <2>;
interrupts = <87>, <88>, <89>, <90>;
+ clocks = <&coreclk 0>;
};

gpio2: gpio@18180 {
diff --git a/arch/arm/boot/dts/armada-xp-mv78230.dtsi b/arch/arm/boot/dts/armada-xp-mv78230.dtsi
index 07c5090..9ae856b 100644
--- a/arch/arm/boot/dts/armada-xp-mv78230.dtsi
+++ b/arch/arm/boot/dts/armada-xp-mv78230.dtsi
@@ -203,24 +203,30 @@
internal-regs {
gpio0: gpio@18100 {
compatible = "marvell,orion-gpio";
- reg = <0x18100 0x40>;
+ reg = <0x18100 0x40>, <0x181c0 0x08>;
+ reg-names = "gpio", "pwm";
ngpios = <32>;
gpio-controller;
#gpio-cells = <2>;
+ #pwm-cells = <1>;
interrupt-controller;
#interrupt-cells = <2>;
interrupts = <82>, <83>, <84>, <85>;
+ clocks = <&coreclk 0>;
};

gpio1: gpio@18140 {
compatible = "marvell,orion-gpio";
- reg = <0x18140 0x40>;
+ reg = <0x18140 0x40>, <0x181c8 0x08>;
+ reg-names = "gpio", "pwm";
ngpios = <17>;
gpio-controller;
#gpio-cells = <2>;
+ #pwm-cells = <1>;
interrupt-controller;
#interrupt-cells = <2>;
interrupts = <87>, <88>, <89>;
+ clocks = <&coreclk 0>;
};
};
};
diff --git a/arch/arm/boot/dts/armada-xp-mv78260.dtsi b/arch/arm/boot/dts/armada-xp-mv78260.dtsi
index 64e936a..11eb6ff 100644
--- a/arch/arm/boot/dts/armada-xp-mv78260.dtsi
+++ b/arch/arm/boot/dts/armada-xp-mv78260.dtsi
@@ -286,24 +286,30 @@
internal-regs {
gpio0: gpio@18100 {
compatible = "marvell,orion-gpio";
- reg = <0x18100 0x40>;
+ reg = <0x18100 0x40>, <0x181c0 0x08>;
+ reg-names = "gpio", "pwm";
ngpios = <32>;
gpio-controller;
#gpio-cells = <2>;
+ #pwm-cells = <1>;
interrupt-controller;
#interrupt-cells = <2>;
interrupts = <82>, <83>, <84>, <85>;
+ clocks = <&coreclk 0>;
};

gpio1: gpio@18140 {
compatible = "marvell,orion-gpio";
- reg = <0x18140 0x40>;
+ reg = <0x18140 0x40>, <0x181c8 0x08>;
+ reg-names = "gpio", "pwm";
ngpios = <32>;
gpio-controller;
#gpio-cells = <2>;
+ #pwm-cells = <1>;
interrupt-controller;
#interrupt-cells = <2>;
interrupts = <87>, <88>, <89>, <90>;
+ clocks = <&coreclk 0>;
};

gpio2: gpio@18180 {
diff --git a/arch/arm/boot/dts/armada-xp-mv78460.dtsi b/arch/arm/boot/dts/armada-xp-mv78460.dtsi
index d1383dd..597b0d6 100644
--- a/arch/arm/boot/dts/armada-xp-mv78460.dtsi
+++ b/arch/arm/boot/dts/armada-xp-mv78460.dtsi
@@ -324,24 +324,30 @@
internal-regs {
gpio0: gpio@18100 {
compatible = "marvell,orion-gpio";
- reg = <0x18100 0x40>;
+ reg = <0x18100 0x40>, <0x181c0 0x08>;
+ reg-names = "gpio", "pwm";
ngpios = <32>;
gpio-controller;
#gpio-cells = <2>;
+ #pwm-cells = <1>;
interrupt-controller;
#interrupt-cells = <2>;
interrupts = <82>, <83>, <84>, <85>;
+ clocks = <&coreclk 0>;
};

gpio1: gpio@18140 {
compatible = "marvell,orion-gpio";
- reg = <0x18140 0x40>;
+ reg = <0x18140 0x40>, <0x181c8 0x08>;
+ reg-names = "gpio", "pwm";
ngpios = <32>;
gpio-controller;
#gpio-cells = <2>;
+ #pwm-cells = <1>;
interrupt-controller;
#interrupt-cells = <2>;
interrupts = <87>, <88>, <89>, <90>;
+ clocks = <&coreclk 0>;
};

gpio2: gpio@18180 {
--
2.10.2

2017-03-24 14:19:51

by Ralph Sennhauser

[permalink] [raw]
Subject: [PATCH v3 3/4] ARM: mvebu: Enable SENSORS_PWM_FAN in defconfig

From: Andrew Lunn <[email protected]>

Now that the GPIO driver also supports PWM operation, enable the PWM
framework and fan driver in mvebu_v7_defconfig.

Signed-off-by: Andrew Lunn <[email protected]>
URL: https://patchwork.ozlabs.org/patch/427297/
[Ralph Sennhauser: add fan driver to defconfig]
Signed-off-by: Ralph Sennhauser <[email protected]>
---
arch/arm/configs/mvebu_v7_defconfig | 2 ++
1 file changed, 2 insertions(+)

diff --git a/arch/arm/configs/mvebu_v7_defconfig b/arch/arm/configs/mvebu_v7_defconfig
index f1a0e25..6955370 100644
--- a/arch/arm/configs/mvebu_v7_defconfig
+++ b/arch/arm/configs/mvebu_v7_defconfig
@@ -135,6 +135,8 @@ CONFIG_DMADEVICES=y
CONFIG_MV_XOR=y
# CONFIG_IOMMU_SUPPORT is not set
CONFIG_MEMORY=y
+CONFIG_PWM=y
+CONFIG_SENSORS_PWM_FAN=y
CONFIG_EXT4_FS=y
CONFIG_ISO9660_FS=y
CONFIG_JOLIET=y
--
2.10.2

2017-03-24 14:23:56

by Andrew Lunn

[permalink] [raw]
Subject: Re: [PATCH v3 1/4] gpio: mvebu: Add limited PWM support

Hi Ralph

> +- #pwm-cells: Should be one. The first cell is the pin number.

> + #pwm-cells = <2>;

You missed updating the example.

2017-03-24 14:35:20

by Andrew Lunn

[permalink] [raw]
Subject: Re: [PATCH v3 4/4] mvebu: wrt1900ac: Use pwm-fan rather than gpio-fan

> + pwm_fan {
> /* SUNON HA4010V4-0000-C99 */
> - compatible = "gpio-fan";
> - gpios = <&gpio0 24 0>;
>
> - gpio-fan,speed-map = <0 0
> - 4500 1>;
> + compatible = "pwm-fan";
> + pwms = <&gpio0 24 4000 0>;

Hi Ralph

I believe this last 0 is the flags parameter. Now that we have
#pwm-cells = 1, i think this last 0 should be dropped.

Andrew

2017-03-24 15:10:02

by Andrew Lunn

[permalink] [raw]
Subject: Re: [PATCH v3 1/4] gpio: mvebu: Add limited PWM support

> + if (val < state->duty_cycle)
> + state->period = 1;
> + else {
> + val -= state->duty_cycle;
> + if (val > UINT_MAX)
> + state->period = UINT_MAX;
> + else if (val)
> + state->period = val;
> + else
> + state->period = 1;
> + }

Hi Ralph

The coding style guide says that if one side of an if else clause has
{}, the other side must as well, even if it is a single statement.

Andrew

2017-03-24 22:23:58

by Ralph Sennhauser

[permalink] [raw]
Subject: Re: [PATCH v3 4/4] mvebu: wrt1900ac: Use pwm-fan rather than gpio-fan

On Fri, 24 Mar 2017 15:35:05 +0100
Andrew Lunn <[email protected]> wrote:

> > + pwm_fan {
> > /* SUNON HA4010V4-0000-C99 */
> > - compatible = "gpio-fan";
> > - gpios = <&gpio0 24 0>;
> >
> > - gpio-fan,speed-map = <0 0
> > - 4500 1>;
> > + compatible = "pwm-fan";
> > + pwms = <&gpio0 24 4000 0>;
>
> Hi Ralph
>
> I believe this last 0 is the flags parameter. Now that we have
> #pwm-cells = 1, i think this last 0 should be dropped.
>
> Andrew

Missed it. Duly noted like the other comments.

Thanks
Ralph

2017-03-26 21:06:23

by Ralph Sennhauser

[permalink] [raw]
Subject: Re: [PATCH v3 4/4] mvebu: wrt1900ac: Use pwm-fan rather than gpio-fan

On Fri, 24 Mar 2017 15:35:05 +0100
Andrew Lunn <[email protected]> wrote:

> > + pwm_fan {
> > /* SUNON HA4010V4-0000-C99 */
> > - compatible = "gpio-fan";
> > - gpios = <&gpio0 24 0>;
> >
> > - gpio-fan,speed-map = <0 0
> > - 4500 1>;
> > + compatible = "pwm-fan";
> > + pwms = <&gpio0 24 4000 0>;
>
> Hi Ralph
>
> I believe this last 0 is the flags parameter. Now that we have
> #pwm-cells = 1, i think this last 0 should be dropped.
>
> Andrew

Hi Andrew,

isn't the 4000 (period?) pwm-cell #2 and 0 (flags?) pwm-cell #3? I
actually expect "pwms = <&gpio0 24>;" here or "#pwm-cells = <2>;"

Ralph

2017-03-27 13:17:52

by Andrew Lunn

[permalink] [raw]
Subject: Re: [PATCH v3 4/4] mvebu: wrt1900ac: Use pwm-fan rather than gpio-fan

On Sun, Mar 26, 2017 at 11:05:58PM +0200, Ralph Sennhauser wrote:
> On Fri, 24 Mar 2017 15:35:05 +0100
> Andrew Lunn <[email protected]> wrote:
>
> > > + pwm_fan {
> > > /* SUNON HA4010V4-0000-C99 */
> > > - compatible = "gpio-fan";
> > > - gpios = <&gpio0 24 0>;
> > >
> > > - gpio-fan,speed-map = <0 0
> > > - 4500 1>;
> > > + compatible = "pwm-fan";
> > > + pwms = <&gpio0 24 4000 0>;
> >
> > Hi Ralph
> >
> > I believe this last 0 is the flags parameter. Now that we have
> > #pwm-cells = 1, i think this last 0 should be dropped.
> >
> > Andrew
>
> Hi Andrew,
>
> isn't the 4000 (period?) pwm-cell #2 and 0 (flags?) pwm-cell #3? I
> actually expect "pwms = <&gpio0 24>;" here or "#pwm-cells = <2>;"

Hi Ralph

>From Documentation/devicetree/bindings/pwm/pwm.txt:

pwm-list ::= <single-pwm> [pwm-list]
single-pwm ::= <pwm-phandle> <pwm-specifier>
pwm-phandle : phandle to PWM controller node
pwm-specifier : array of #pwm-cells specifying the given PWM
(controller specific)

Our pwm-list has a single single-pwm.
phandle is &gpio0.
The remaining parts are the specifier, or which there should be #pwm-calls.

>From Documentation/devicetree/bindings/pwm/pwm.txt again:

pwm-specifier typically encodes the chip-relative PWM number and the PWM
period in nanoseconds.

Optionally, the pwm-specifier can encode a number of flags (defined in
<dt-bindings/pwm/pwm.h>) in a third cell:
- PWM_POLARITY_INVERTED: invert the PWM signal polarity

So we are using the 24th PWM and 4000 nanosecod period. We don't want
any flags.

So

pwms = <&gpio0 24 4000>;

has a phandle, and then 2 cells.

Andrew