* [PATCH 0/7] Add PWM support to mvebu gpio driver
@ 2015-01-09 23:34 Andrew Lunn
2015-01-09 23:34 ` [PATCH 1/7] gpio: mvebu: checkpatch fixes Andrew Lunn
` (7 more replies)
0 siblings, 8 replies; 24+ messages in thread
From: Andrew Lunn @ 2015-01-09 23:34 UTC (permalink / raw)
To: linus.walleij
Cc: Thomas Petazzoni, kaloz, Gregory Clement, Sebastian Hesselbarth,
linux-gpio, linux-pwm, Andrew Lunn
This patchset starts out with some cleanups and fixes for the mvebu
gpio driver. Then new functionality is added. The Armada 370 and XP
SoC has the ability to "blink" the gpio lines at a configuration on
and off duration. So simple PWM support is added. However only a
single gpio line per gpio chip can be used as a PWM line due to
limitations of the hardware. The last patch shows one example of how
this could be used, converting a gpio-fan into a pwm-fan, so allowing
finer control over its rotation speed, and hence noise. This last
patch is not expected to be accepted, since the device is not yet in
mainline.
Andrew Lunn (7):
gpio: mvebu: checkpatch fixes
gpio: mvebu: Fix probe cleanup on error
gpio: mvebu: Add limited PWM support
DT: bindings: Extend mvebu gpio documentation with PWM
mvebu: xp: Add pwm properties to .dtsi files
arm: mvebu: Enable PWM in defconfig
mvebu: wrt1900ac: Use pwm-fan rather than gpio-fan
.../devicetree/bindings/gpio/gpio-mvebu.txt | 31 ++++
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 | 8 +-
arch/arm/boot/dts/armada-xp-mv78460.dtsi | 10 +-
arch/arm/boot/dts/armada-xp-wrt1900ac.dts | 8 +-
arch/arm/configs/mvebu_v7_defconfig | 1 +
drivers/gpio/Kconfig | 5 +
drivers/gpio/Makefile | 1 +
drivers/gpio/gpio-mvebu-pwm.c | 202 +++++++++++++++++++++
drivers/gpio/gpio-mvebu.c | 133 +++++++-------
drivers/gpio/gpio-mvebu.h | 79 ++++++++
12 files changed, 421 insertions(+), 77 deletions(-)
create mode 100644 drivers/gpio/gpio-mvebu-pwm.c
create mode 100644 drivers/gpio/gpio-mvebu.h
--
2.1.3
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 1/7] gpio: mvebu: checkpatch fixes
2015-01-09 23:34 [PATCH 0/7] Add PWM support to mvebu gpio driver Andrew Lunn
@ 2015-01-09 23:34 ` Andrew Lunn
2015-01-14 13:04 ` Linus Walleij
2015-01-09 23:34 ` [PATCH 2/7] gpio: mvebu: Fix probe cleanup on error Andrew Lunn
` (6 subsequent siblings)
7 siblings, 1 reply; 24+ messages in thread
From: Andrew Lunn @ 2015-01-09 23:34 UTC (permalink / raw)
To: linus.walleij
Cc: Thomas Petazzoni, kaloz, Gregory Clement, Sebastian Hesselbarth,
linux-gpio, linux-pwm, Andrew Lunn
Wrap some long lines.
Prefer seq_puts() over seq_printf().
space to tab conversions.
Spelling error fix.
Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
drivers/gpio/gpio-mvebu.c | 77 ++++++++++++++++++++++++++---------------------
1 file changed, 42 insertions(+), 35 deletions(-)
diff --git a/drivers/gpio/gpio-mvebu.c b/drivers/gpio/gpio-mvebu.c
index 7bc3e9b288f3..7533d446b820 100644
--- a/drivers/gpio/gpio-mvebu.c
+++ b/drivers/gpio/gpio-mvebu.c
@@ -59,7 +59,7 @@
#define GPIO_LEVEL_MASK_OFF 0x001c
/* The MV78200 has per-CPU registers for edge mask and level mask */
-#define GPIO_EDGE_MASK_MV78200_OFF(cpu) ((cpu) ? 0x30 : 0x18)
+#define GPIO_EDGE_MASK_MV78200_OFF(cpu) ((cpu) ? 0x30 : 0x18)
#define GPIO_LEVEL_MASK_MV78200_OFF(cpu) ((cpu) ? 0x34 : 0x1C)
/* The Armada XP has per-CPU registers for interrupt cause, interrupt
@@ -69,11 +69,11 @@
#define GPIO_EDGE_MASK_ARMADAXP_OFF(cpu) (0x10 + (cpu) * 0x4)
#define GPIO_LEVEL_MASK_ARMADAXP_OFF(cpu) (0x20 + (cpu) * 0x4)
-#define MVEBU_GPIO_SOC_VARIANT_ORION 0x1
-#define MVEBU_GPIO_SOC_VARIANT_MV78200 0x2
+#define MVEBU_GPIO_SOC_VARIANT_ORION 0x1
+#define MVEBU_GPIO_SOC_VARIANT_MV78200 0x2
#define MVEBU_GPIO_SOC_VARIANT_ARMADAXP 0x3
-#define MVEBU_MAX_GPIO_PER_BANK 32
+#define MVEBU_MAX_GPIO_PER_BANK 32
struct mvebu_gpio_chip {
struct gpio_chip chip;
@@ -82,9 +82,9 @@ struct mvebu_gpio_chip {
void __iomem *percpu_membase;
int irqbase;
struct irq_domain *domain;
- int soc_variant;
+ int soc_variant;
- /* Used to preserve GPIO registers accross suspend/resume */
+ /* Used to preserve GPIO registers across suspend/resume */
u32 out_reg;
u32 io_conf_reg;
u32 blink_en_reg;
@@ -107,7 +107,8 @@ static inline void __iomem *mvebu_gpioreg_blink(struct mvebu_gpio_chip *mvchip)
return mvchip->membase + GPIO_BLINK_EN_OFF;
}
-static inline void __iomem *mvebu_gpioreg_io_conf(struct mvebu_gpio_chip *mvchip)
+static inline void __iomem *
+mvebu_gpioreg_io_conf(struct mvebu_gpio_chip *mvchip)
{
return mvchip->membase + GPIO_IO_CONF_OFF;
}
@@ -117,12 +118,14 @@ static inline void __iomem *mvebu_gpioreg_in_pol(struct mvebu_gpio_chip *mvchip)
return mvchip->membase + GPIO_IN_POL_OFF;
}
-static inline void __iomem *mvebu_gpioreg_data_in(struct mvebu_gpio_chip *mvchip)
+static inline void __iomem *
+mvebu_gpioreg_data_in(struct mvebu_gpio_chip *mvchip)
{
return mvchip->membase + GPIO_DATA_IN_OFF;
}
-static inline void __iomem *mvebu_gpioreg_edge_cause(struct mvebu_gpio_chip *mvchip)
+static inline void __iomem *
+mvebu_gpioreg_edge_cause(struct mvebu_gpio_chip *mvchip)
{
int cpu;
@@ -132,13 +135,15 @@ static inline void __iomem *mvebu_gpioreg_edge_cause(struct mvebu_gpio_chip *mvc
return mvchip->membase + GPIO_EDGE_CAUSE_OFF;
case MVEBU_GPIO_SOC_VARIANT_ARMADAXP:
cpu = smp_processor_id();
- return mvchip->percpu_membase + GPIO_EDGE_CAUSE_ARMADAXP_OFF(cpu);
+ return mvchip->percpu_membase +
+ GPIO_EDGE_CAUSE_ARMADAXP_OFF(cpu);
default:
BUG();
}
}
-static inline void __iomem *mvebu_gpioreg_edge_mask(struct mvebu_gpio_chip *mvchip)
+static inline void __iomem *
+mvebu_gpioreg_edge_mask(struct mvebu_gpio_chip *mvchip)
{
int cpu;
@@ -150,7 +155,8 @@ static inline void __iomem *mvebu_gpioreg_edge_mask(struct mvebu_gpio_chip *mvch
return mvchip->membase + GPIO_EDGE_MASK_MV78200_OFF(cpu);
case MVEBU_GPIO_SOC_VARIANT_ARMADAXP:
cpu = smp_processor_id();
- return mvchip->percpu_membase + GPIO_EDGE_MASK_ARMADAXP_OFF(cpu);
+ return mvchip->percpu_membase +
+ GPIO_EDGE_MASK_ARMADAXP_OFF(cpu);
default:
BUG();
}
@@ -168,7 +174,8 @@ static void __iomem *mvebu_gpioreg_level_mask(struct mvebu_gpio_chip *mvchip)
return mvchip->membase + GPIO_LEVEL_MASK_MV78200_OFF(cpu);
case MVEBU_GPIO_SOC_VARIANT_ARMADAXP:
cpu = smp_processor_id();
- return mvchip->percpu_membase + GPIO_LEVEL_MASK_ARMADAXP_OFF(cpu);
+ return mvchip->percpu_membase +
+ GPIO_LEVEL_MASK_ARMADAXP_OFF(cpu);
default:
BUG();
}
@@ -364,22 +371,22 @@ static void mvebu_gpio_level_irq_unmask(struct irq_data *d)
* value of the line or the opposite value.
*
* Level IRQ handlers: DATA_IN is used directly as cause register.
- * Interrupt are masked by LEVEL_MASK registers.
+ * Interrupt are masked by LEVEL_MASK registers.
* Edge IRQ handlers: Change in DATA_IN are latched in EDGE_CAUSE.
- * Interrupt are masked by EDGE_MASK registers.
+ * Interrupt are masked by EDGE_MASK registers.
* Both-edge handlers: Similar to regular Edge handlers, but also swaps
- * the polarity to catch the next line transaction.
- * This is a race condition that might not perfectly
- * work on some use cases.
+ * the polarity to catch the next line transaction.
+ * This is a race condition that might not perfectly
+ * work on some use cases.
*
* Every eight GPIO lines are grouped (OR'ed) before going up to main
* cause register.
*
- * EDGE cause mask
- * data-in /--------| |-----| |----\
- * -----| |----- ---- to main cause reg
- * X \----------------| |----/
- * polarity LEVEL mask
+ * EDGE cause mask
+ * data-in /--------| |-----| |----\
+ * -----| |----- ---- to main cause reg
+ * X \----------------| |----/
+ * polarity LEVEL mask
*
****************************************************************************/
@@ -394,9 +401,8 @@ static int mvebu_gpio_irq_set_type(struct irq_data *d, unsigned int type)
pin = d->hwirq;
u = readl_relaxed(mvebu_gpioreg_io_conf(mvchip)) & (1 << pin);
- if (!u) {
+ if (!u)
return -EINVAL;
- }
type &= IRQ_TYPE_SENSE_MASK;
if (type == IRQ_TYPE_NONE)
@@ -529,13 +535,13 @@ static void mvebu_gpio_dbg_show(struct seq_file *s, struct gpio_chip *chip)
(data_in ^ in_pol) & msk ? "hi" : "lo",
in_pol & msk ? "lo" : "hi");
if (!((edg_msk | lvl_msk) & msk)) {
- seq_printf(s, " disabled\n");
+ seq_puts(s, " disabled\n");
continue;
}
if (edg_msk & msk)
- seq_printf(s, " edge ");
+ seq_puts(s, " edge ");
if (lvl_msk & msk)
- seq_printf(s, " level");
+ seq_puts(s, " level");
seq_printf(s, " (%s)\n", cause & msk ? "pending" : "clear ");
}
}
@@ -546,15 +552,15 @@ static void mvebu_gpio_dbg_show(struct seq_file *s, struct gpio_chip *chip)
static const struct of_device_id mvebu_gpio_of_match[] = {
{
.compatible = "marvell,orion-gpio",
- .data = (void *) MVEBU_GPIO_SOC_VARIANT_ORION,
+ .data = (void *) MVEBU_GPIO_SOC_VARIANT_ORION,
},
{
.compatible = "marvell,mv78200-gpio",
- .data = (void *) MVEBU_GPIO_SOC_VARIANT_MV78200,
+ .data = (void *) MVEBU_GPIO_SOC_VARIANT_MV78200,
},
{
.compatible = "marvell,armadaxp-gpio",
- .data = (void *) MVEBU_GPIO_SOC_VARIANT_ARMADAXP,
+ .data = (void *) MVEBU_GPIO_SOC_VARIANT_ARMADAXP,
},
{
/* sentinel */
@@ -668,7 +674,8 @@ static int mvebu_gpio_probe(struct platform_device *pdev)
else
soc_variant = MVEBU_GPIO_SOC_VARIANT_ORION;
- mvchip = devm_kzalloc(&pdev->dev, sizeof(struct mvebu_gpio_chip), GFP_KERNEL);
+ mvchip = devm_kzalloc(&pdev->dev, sizeof(struct mvebu_gpio_chip),
+ GFP_KERNEL);
if (!mvchip)
return -ENOMEM;
@@ -767,8 +774,8 @@ static int mvebu_gpio_probe(struct platform_device *pdev)
* interrupt handlers, with each handler dealing with 8 GPIO
* pins. */
for (i = 0; i < 4; i++) {
- int irq;
- irq = platform_get_irq(pdev, i);
+ int irq = platform_get_irq(pdev, i);
+
if (irq < 0)
continue;
irq_set_handler_data(irq, mvchip);
@@ -827,7 +834,7 @@ static int mvebu_gpio_probe(struct platform_device *pdev)
static struct platform_driver mvebu_gpio_driver = {
.driver = {
- .name = "mvebu-gpio",
+ .name = "mvebu-gpio",
.of_match_table = mvebu_gpio_of_match,
},
.probe = mvebu_gpio_probe,
--
2.1.3
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 2/7] gpio: mvebu: Fix probe cleanup on error
2015-01-09 23:34 [PATCH 0/7] Add PWM support to mvebu gpio driver Andrew Lunn
2015-01-09 23:34 ` [PATCH 1/7] gpio: mvebu: checkpatch fixes Andrew Lunn
@ 2015-01-09 23:34 ` Andrew Lunn
2015-01-14 13:05 ` Linus Walleij
2015-01-09 23:34 ` [PATCH 3/7] gpio: mvebu: Add limited PWM support Andrew Lunn
` (5 subsequent siblings)
7 siblings, 1 reply; 24+ messages in thread
From: Andrew Lunn @ 2015-01-09 23:34 UTC (permalink / raw)
To: linus.walleij
Cc: Thomas Petazzoni, kaloz, Gregory Clement, Sebastian Hesselbarth,
linux-gpio, linux-pwm, Andrew Lunn
Ensure that when there is an error during probe that the gpiochip is
removed and the generic irq chip is removed.
Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
drivers/gpio/gpio-mvebu.c | 23 +++++++++++++++++------
1 file changed, 17 insertions(+), 6 deletions(-)
diff --git a/drivers/gpio/gpio-mvebu.c b/drivers/gpio/gpio-mvebu.c
index 7533d446b820..d0bc123c7975 100644
--- a/drivers/gpio/gpio-mvebu.c
+++ b/drivers/gpio/gpio-mvebu.c
@@ -667,6 +667,7 @@ static int mvebu_gpio_probe(struct platform_device *pdev)
unsigned int ngpios;
int soc_variant;
int i, cpu, id;
+ int err;
match = of_match_device(mvebu_gpio_of_match, &pdev->dev);
if (match)
@@ -785,14 +786,16 @@ static int mvebu_gpio_probe(struct platform_device *pdev)
mvchip->irqbase = irq_alloc_descs(-1, 0, ngpios, -1);
if (mvchip->irqbase < 0) {
dev_err(&pdev->dev, "no irqs\n");
- return mvchip->irqbase;
+ err = mvchip->irqbase;
+ goto err_gpiochip_add;
}
gc = irq_alloc_generic_chip("mvebu_gpio_irq", 2, mvchip->irqbase,
mvchip->membase, handle_level_irq);
if (!gc) {
dev_err(&pdev->dev, "Cannot allocate generic irq_chip\n");
- return -ENOMEM;
+ err = -ENOMEM;
+ goto err_gpiochip_add;
}
gc->private = mvchip;
@@ -823,13 +826,21 @@ static int mvebu_gpio_probe(struct platform_device *pdev)
if (!mvchip->domain) {
dev_err(&pdev->dev, "couldn't allocate irq domain %s (DT).\n",
mvchip->chip.label);
- irq_remove_generic_chip(gc, IRQ_MSK(ngpios), IRQ_NOREQUEST,
- IRQ_LEVEL | IRQ_NOPROBE);
- kfree(gc);
- return -ENODEV;
+ err = -ENODEV;
+ goto err_generic_chip;
}
return 0;
+
+err_generic_chip:
+ irq_remove_generic_chip(gc, IRQ_MSK(ngpios), IRQ_NOREQUEST,
+ IRQ_LEVEL | IRQ_NOPROBE);
+ kfree(gc);
+
+err_gpiochip_add:
+ gpiochip_remove(&mvchip->chip);
+
+ return err;
}
static struct platform_driver mvebu_gpio_driver = {
--
2.1.3
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 3/7] gpio: mvebu: Add limited PWM support
2015-01-09 23:34 [PATCH 0/7] Add PWM support to mvebu gpio driver Andrew Lunn
2015-01-09 23:34 ` [PATCH 1/7] gpio: mvebu: checkpatch fixes Andrew Lunn
2015-01-09 23:34 ` [PATCH 2/7] gpio: mvebu: Fix probe cleanup on error Andrew Lunn
@ 2015-01-09 23:34 ` Andrew Lunn
2015-01-12 11:05 ` Imre Kaloz
` (2 more replies)
2015-01-09 23:34 ` [PATCH 4/7] DT: bindings: Extend mvebu gpio documentation with PWM Andrew Lunn
` (4 subsequent siblings)
7 siblings, 3 replies; 24+ messages in thread
From: Andrew Lunn @ 2015-01-09 23:34 UTC (permalink / raw)
To: linus.walleij
Cc: Thomas Petazzoni, kaloz, Gregory Clement, Sebastian Hesselbarth,
linux-gpio, linux-pwm, Andrew Lunn
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 <andrew@lunn.ch>
---
drivers/gpio/Kconfig | 5 ++
drivers/gpio/Makefile | 1 +
drivers/gpio/gpio-mvebu-pwm.c | 202 ++++++++++++++++++++++++++++++++++++++++++
drivers/gpio/gpio-mvebu.c | 37 +++-----
drivers/gpio/gpio-mvebu.h | 79 +++++++++++++++++
5 files changed, 299 insertions(+), 25 deletions(-)
create mode 100644 drivers/gpio/gpio-mvebu-pwm.c
create mode 100644 drivers/gpio/gpio-mvebu.h
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 633ec216e185..be98d531d735 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -239,6 +239,11 @@ config GPIO_MVEBU
select GPIO_GENERIC
select GENERIC_IRQ_CHIP
+config GPIO_MVEBU_PWM
+ def_bool y
+ depends on GPIO_MVEBU
+ depends on PWM
+
config GPIO_MXC
def_bool y
depends on ARCH_MXC
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index 81755f1305e6..be94f47c1ddb 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -60,6 +60,7 @@ obj-$(CONFIG_GPIO_MSIC) += gpio-msic.o
obj-$(CONFIG_GPIO_MSM_V1) += gpio-msm-v1.o
obj-$(CONFIG_GPIO_MSM_V2) += gpio-msm-v2.o
obj-$(CONFIG_GPIO_MVEBU) += gpio-mvebu.o
+obj-$(CONFIG_GPIO_MVEBU_PWM) += gpio-mvebu-pwm.o
obj-$(CONFIG_GPIO_MXC) += gpio-mxc.o
obj-$(CONFIG_GPIO_MXS) += gpio-mxs.o
obj-$(CONFIG_GPIO_OCTEON) += gpio-octeon.o
diff --git a/drivers/gpio/gpio-mvebu-pwm.c b/drivers/gpio/gpio-mvebu-pwm.c
new file mode 100644
index 000000000000..8a5af11aed67
--- /dev/null
+++ b/drivers/gpio/gpio-mvebu-pwm.c
@@ -0,0 +1,202 @@
+#include <linux/err.h>
+#include <linux/module.h>
+#include <linux/gpio.h>
+#include <linux/pwm.h>
+#include <linux/clk.h>
+#include <linux/platform_device.h>
+#include "gpio-mvebu.h"
+#include "gpiolib.h"
+
+static void __iomem *mvebu_gpioreg_blink_select(struct mvebu_gpio_chip *mvchip)
+{
+ return mvchip->membase + GPIO_BLINK_CNT_SELECT;
+}
+
+static inline struct mvebu_pwm *to_mvebu_pwm(struct pwm_chip *chip)
+{
+ return container_of(chip, struct mvebu_pwm, chip);
+}
+
+static inline struct mvebu_gpio_chip *to_mvchip(struct mvebu_pwm *pwm)
+{
+ return container_of(pwm, struct mvebu_gpio_chip, pwm);
+}
+
+static int mvebu_pwm_request(struct pwm_chip *chip, struct pwm_device *pwmd)
+{
+ struct mvebu_pwm *pwm = to_mvebu_pwm(chip);
+ struct mvebu_gpio_chip *mvchip = to_mvchip(pwm);
+ struct gpio_desc *desc = gpio_to_desc(pwmd->pwm);
+ unsigned long flags;
+ int ret = 0;
+
+ spin_lock_irqsave(&pwm->lock, flags);
+ if (pwm->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;
+ }
+
+ pwm->pin = pwmd->pwm - mvchip->chip.base;
+ pwm->used = true;
+ }
+
+out:
+ spin_unlock_irqrestore(&pwm->lock, flags);
+ return ret;
+}
+
+static void mvebu_pwm_free(struct pwm_chip *chip, struct pwm_device *pwmd)
+{
+ struct mvebu_pwm *pwm = to_mvebu_pwm(chip);
+ struct gpio_desc *desc = gpio_to_desc(pwmd->pwm);
+ unsigned long flags;
+
+ spin_lock_irqsave(&pwm->lock, flags);
+ gpiod_free(desc);
+ pwm->used = false;
+ spin_unlock_irqrestore(&pwm->lock, flags);
+}
+
+static int mvebu_pwm_config(struct pwm_chip *chip, struct pwm_device *pwmd,
+ int duty_ns, int period_ns)
+{
+ struct mvebu_pwm *pwm = to_mvebu_pwm(chip);
+ struct mvebu_gpio_chip *mvchip = to_mvchip(pwm);
+ unsigned int on, off;
+ unsigned long long val;
+ u32 u;
+
+ val = (unsigned long long) pwm->clk_rate * duty_ns;
+ do_div(val, NSEC_PER_SEC);
+ if (val > UINT_MAX)
+ return -EINVAL;
+ if (val)
+ on = val;
+ else
+ on = 1;
+
+ val = (unsigned long long) pwm->clk_rate * (period_ns - duty_ns);
+ do_div(val, NSEC_PER_SEC);
+ if (val > UINT_MAX)
+ return -EINVAL;
+ if (val)
+ off = val;
+ else
+ off = 1;
+
+ u = readl_relaxed(mvebu_gpioreg_blink_select(mvchip));
+ u &= ~(1 << pwm->pin);
+ u |= (pwm->id << pwm->pin);
+ writel_relaxed(u, mvebu_gpioreg_blink_select(mvchip));
+
+ writel_relaxed(on, pwm->membase + BLINK_ON_DURATION);
+ writel_relaxed(off, pwm->membase + BLINK_OFF_DURATION);
+
+ return 0;
+}
+
+static int mvebu_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwmd)
+{
+ struct mvebu_pwm *pwm = to_mvebu_pwm(chip);
+ struct mvebu_gpio_chip *mvchip = to_mvchip(pwm);
+
+ mvebu_gpio_blink(&mvchip->chip, pwm->pin, 1);
+
+ return 0;
+}
+
+static void mvebu_pwm_disable(struct pwm_chip *chip, struct pwm_device *pwmd)
+{
+ struct mvebu_pwm *pwm = to_mvebu_pwm(chip);
+ struct mvebu_gpio_chip *mvchip = to_mvchip(pwm);
+
+ mvebu_gpio_blink(&mvchip->chip, pwm->pin, 0);
+}
+
+static const struct pwm_ops mvebu_pwm_ops = {
+ .request = mvebu_pwm_request,
+ .free = mvebu_pwm_free,
+ .config = mvebu_pwm_config,
+ .enable = mvebu_pwm_enable,
+ .disable = mvebu_pwm_disable,
+ .owner = THIS_MODULE,
+};
+
+void mvebu_pwm_suspend(struct mvebu_gpio_chip *mvchip)
+{
+ struct mvebu_pwm *pwm = &mvchip->pwm;
+
+ pwm->blink_select = readl_relaxed(mvebu_gpioreg_blink_select(mvchip));
+ pwm->blink_on_duration =
+ readl_relaxed(pwm->membase + BLINK_ON_DURATION);
+ pwm->blink_off_duration =
+ readl_relaxed(pwm->membase + BLINK_OFF_DURATION);
+}
+
+void mvebu_pwm_resume(struct mvebu_gpio_chip *mvchip)
+{
+ struct mvebu_pwm *pwm = &mvchip->pwm;
+
+ writel_relaxed(pwm->blink_select, mvebu_gpioreg_blink_select(mvchip));
+ writel_relaxed(pwm->blink_on_duration,
+ pwm->membase + BLINK_ON_DURATION);
+ writel_relaxed(pwm->blink_off_duration,
+ pwm->membase + BLINK_OFF_DURATION);
+}
+
+/*
+ * 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.
+ */
+int mvebu_pwm_probe(struct platform_device *pdev,
+ struct mvebu_gpio_chip *mvchip,
+ int id)
+{
+ struct device *dev = &pdev->dev;
+ struct mvebu_pwm *pwm = &mvchip->pwm;
+ struct resource *res;
+
+ res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "pwm");
+ if (!res)
+ return 0;
+
+ mvchip->pwm.membase = devm_ioremap_resource(&pdev->dev, res);
+ if (IS_ERR(mvchip->pwm.membase))
+ return PTR_ERR(mvchip->percpu_membase);
+
+ if (id < 0 || id > 1)
+ return -EINVAL;
+ pwm->id = id;
+
+ if (IS_ERR(mvchip->clk))
+ return PTR_ERR(mvchip->clk);
+
+ pwm->clk_rate = clk_get_rate(mvchip->clk);
+ if (!pwm->clk_rate) {
+ dev_err(dev, "failed to get clock rate\n");
+ return -EINVAL;
+ }
+
+ pwm->chip.dev = dev;
+ pwm->chip.ops = &mvebu_pwm_ops;
+ pwm->chip.base = mvchip->chip.base;
+ pwm->chip.npwm = mvchip->chip.ngpio;
+ pwm->chip.can_sleep = false;
+
+ spin_lock_init(&pwm->lock);
+
+ return pwmchip_add(&pwm->chip);
+}
diff --git a/drivers/gpio/gpio-mvebu.c b/drivers/gpio/gpio-mvebu.c
index d0bc123c7975..147e76cb57e4 100644
--- a/drivers/gpio/gpio-mvebu.c
+++ b/drivers/gpio/gpio-mvebu.c
@@ -42,10 +42,11 @@
#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 "gpio-mvebu.h"
/*
* GPIO unit register offsets.
*/
@@ -75,24 +76,6 @@
#define MVEBU_MAX_GPIO_PER_BANK 32
-struct mvebu_gpio_chip {
- struct gpio_chip chip;
- spinlock_t lock;
- void __iomem *membase;
- void __iomem *percpu_membase;
- int irqbase;
- struct irq_domain *domain;
- int soc_variant;
-
- /* Used to preserve GPIO registers across suspend/resume */
- u32 out_reg;
- u32 io_conf_reg;
- u32 blink_en_reg;
- u32 in_pol_reg;
- u32 edge_mask_regs[4];
- u32 level_mask_regs[4];
-};
-
/*
* Functions returning addresses of individual registers for a given
* GPIO controller.
@@ -228,7 +211,7 @@ static int mvebu_gpio_get(struct gpio_chip *chip, unsigned pin)
return (u >> pin) & 1;
}
-static void mvebu_gpio_blink(struct gpio_chip *chip, unsigned pin, int value)
+void mvebu_gpio_blink(struct gpio_chip *chip, unsigned pin, int value)
{
struct mvebu_gpio_chip *mvchip =
container_of(chip, struct mvebu_gpio_chip, chip);
@@ -609,6 +592,8 @@ static int mvebu_gpio_suspend(struct platform_device *pdev, pm_message_t state)
BUG();
}
+ mvebu_pwm_suspend(mvchip);
+
return 0;
}
@@ -652,6 +637,8 @@ static int mvebu_gpio_resume(struct platform_device *pdev)
BUG();
}
+ mvebu_pwm_resume(mvchip);
+
return 0;
}
@@ -663,7 +650,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;
int soc_variant;
int i, cpu, id;
@@ -693,10 +679,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);
@@ -830,7 +816,8 @@ static int mvebu_gpio_probe(struct platform_device *pdev)
goto err_generic_chip;
}
- return 0;
+ /* Armada 370/XP has simple PWM support for gpio lines */
+ return mvebu_pwm_probe(pdev, mvchip, id);
err_generic_chip:
irq_remove_generic_chip(gc, IRQ_MSK(ngpios), IRQ_NOREQUEST,
diff --git a/drivers/gpio/gpio-mvebu.h b/drivers/gpio/gpio-mvebu.h
new file mode 100644
index 000000000000..7cd8c80c06da
--- /dev/null
+++ b/drivers/gpio/gpio-mvebu.h
@@ -0,0 +1,79 @@
+/*
+ * Interface between MVEBU GPIO driver and PWM driver for GPIO pins
+ *
+ * Copyright (C) 2015, Andrew Lunn <andrew@lunn.ch>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#ifndef MVEBU_GPIO_PWM_H
+#define MVEBU_GPIO_PWM_H
+
+#define BLINK_ON_DURATION 0x0
+#define BLINK_OFF_DURATION 0x4
+#define GPIO_BLINK_CNT_SELECT 0x0020
+
+struct mvebu_pwm {
+ void __iomem *membase;
+ unsigned long clk_rate;
+ bool used;
+ unsigned pin;
+ struct pwm_chip chip;
+ int id;
+ spinlock_t lock;
+
+ /* 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;
+ void __iomem *membase;
+ void __iomem *percpu_membase;
+ int irqbase;
+ struct irq_domain *domain;
+ int soc_variant;
+ struct clk *clk;
+#ifdef CONFIG_PWM
+ struct mvebu_pwm pwm;
+#endif
+ /* Used to preserve GPIO registers across suspend/resume */
+ u32 out_reg;
+ u32 io_conf_reg;
+ u32 blink_en_reg;
+ u32 in_pol_reg;
+ u32 edge_mask_regs[4];
+ u32 level_mask_regs[4];
+};
+
+void mvebu_gpio_blink(struct gpio_chip *chip, unsigned pin, int value);
+
+#ifdef CONFIG_PWM
+int mvebu_pwm_probe(struct platform_device *pdev,
+ struct mvebu_gpio_chip *mvchip,
+ int id);
+void mvebu_pwm_suspend(struct mvebu_gpio_chip *mvchip);
+void mvebu_pwm_resume(struct mvebu_gpio_chip *mvchip);
+#else
+int mvebu_pwm_probe(struct platform_device *pdev,
+ struct mvebu_gpio_chip *mvchip,
+ int id)
+{
+ return 0;
+}
+
+void mvebu_pwm_suspend(struct mvebu_gpio_chip *mvchip)
+{
+}
+
+void mvebu_pwm_resume(struct mvebu_gpio_chip *mvchip)
+{
+}
+#endif
+#endif
--
2.1.3
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 4/7] DT: bindings: Extend mvebu gpio documentation with PWM
2015-01-09 23:34 [PATCH 0/7] Add PWM support to mvebu gpio driver Andrew Lunn
` (2 preceding siblings ...)
2015-01-09 23:34 ` [PATCH 3/7] gpio: mvebu: Add limited PWM support Andrew Lunn
@ 2015-01-09 23:34 ` Andrew Lunn
2015-01-09 23:34 ` [PATCH 5/7] mvebu: xp: Add pwm properties to .dtsi files Andrew Lunn
` (3 subsequent siblings)
7 siblings, 0 replies; 24+ messages in thread
From: Andrew Lunn @ 2015-01-09 23:34 UTC (permalink / raw)
To: linus.walleij
Cc: Thomas Petazzoni, kaloz, Gregory Clement, Sebastian Hesselbarth,
linux-gpio, linux-pwm, Andrew Lunn
Document the optional parameters needed for PWM operation of gpio
lines.
Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
.../devicetree/bindings/gpio/gpio-mvebu.txt | 31 ++++++++++++++++++++++
1 file changed, 31 insertions(+)
diff --git a/Documentation/devicetree/bindings/gpio/gpio-mvebu.txt b/Documentation/devicetree/bindings/gpio/gpio-mvebu.txt
index a6f3bec1da7d..9984e2665568 100644
--- a/Documentation/devicetree/bindings/gpio/gpio-mvebu.txt
+++ b/Documentation/devicetree/bindings/gpio/gpio-mvebu.txt
@@ -38,6 +38,23 @@ 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 supports 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 two. The first cell is the pin number. The
+ second cell is reserved for flags, unused at the moment.
+
+- clocks: Must be a phandle to the clock for the gpio controller.
+
Example:
gpio0: gpio@d0018100 {
@@ -51,3 +68,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>;
+ };
--
2.1.3
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 5/7] mvebu: xp: Add pwm properties to .dtsi files
2015-01-09 23:34 [PATCH 0/7] Add PWM support to mvebu gpio driver Andrew Lunn
` (3 preceding siblings ...)
2015-01-09 23:34 ` [PATCH 4/7] DT: bindings: Extend mvebu gpio documentation with PWM Andrew Lunn
@ 2015-01-09 23:34 ` Andrew Lunn
2015-01-09 23:34 ` [PATCH 6/7] arm: mvebu: Enable PWM in defconfig Andrew Lunn
` (2 subsequent siblings)
7 siblings, 0 replies; 24+ messages in thread
From: Andrew Lunn @ 2015-01-09 23:34 UTC (permalink / raw)
To: linus.walleij
Cc: Thomas Petazzoni, kaloz, Gregory Clement, Sebastian Hesselbarth,
linux-gpio, linux-pwm, Andrew Lunn
Add properties to the gpio nodes to allow them to be also used
as pwm lines.
Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
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 | 8 ++++++--
arch/arm/boot/dts/armada-xp-mv78460.dtsi | 10 ++++++++--
4 files changed, 30 insertions(+), 8 deletions(-)
diff --git a/arch/arm/boot/dts/armada-370.dtsi b/arch/arm/boot/dts/armada-370.dtsi
index fdb3c12a6139..1a38d15adb88 100644
--- a/arch/arm/boot/dts/armada-370.dtsi
+++ b/arch/arm/boot/dts/armada-370.dtsi
@@ -123,24 +123,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 = <2>;
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 = <2>;
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 281ccd24295c..a37a73dd9252 100644
--- a/arch/arm/boot/dts/armada-xp-mv78230.dtsi
+++ b/arch/arm/boot/dts/armada-xp-mv78230.dtsi
@@ -169,24 +169,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 = <2>;
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 = <2>;
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 d7a8d0b0f385..a4b9fbd3a44d 100644
--- a/arch/arm/boot/dts/armada-xp-mv78260.dtsi
+++ b/arch/arm/boot/dts/armada-xp-mv78260.dtsi
@@ -253,24 +253,28 @@
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 = <2>;
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>;
ngpios = <32>;
gpio-controller;
#gpio-cells = <2>;
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 9c40c130d11a..77ae071134a0 100644
--- a/arch/arm/boot/dts/armada-xp-mv78460.dtsi
+++ b/arch/arm/boot/dts/armada-xp-mv78460.dtsi
@@ -291,24 +291,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 = <2>;
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 = <2>;
interrupt-controller;
#interrupt-cells = <2>;
interrupts = <87>, <88>, <89>, <90>;
+ clocks = <&coreclk 0>;
};
gpio2: gpio@18180 {
--
2.1.3
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 6/7] arm: mvebu: Enable PWM in defconfig
2015-01-09 23:34 [PATCH 0/7] Add PWM support to mvebu gpio driver Andrew Lunn
` (4 preceding siblings ...)
2015-01-09 23:34 ` [PATCH 5/7] mvebu: xp: Add pwm properties to .dtsi files Andrew Lunn
@ 2015-01-09 23:34 ` Andrew Lunn
2015-01-09 23:34 ` [PATCH 7/7] mvebu: wrt1900ac: Use pwm-fan rather than gpio-fan Andrew Lunn
2015-02-23 14:09 ` [PATCH 0/7] Add PWM support to mvebu gpio driver Gregory CLEMENT
7 siblings, 0 replies; 24+ messages in thread
From: Andrew Lunn @ 2015-01-09 23:34 UTC (permalink / raw)
To: linus.walleij
Cc: Thomas Petazzoni, kaloz, Gregory Clement, Sebastian Hesselbarth,
linux-gpio, linux-pwm, Andrew Lunn
Now that the gpio driver also supports PWM operation, enable
the PWM framework in mvebu_v7_defconfig.
Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
arch/arm/configs/mvebu_v7_defconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm/configs/mvebu_v7_defconfig b/arch/arm/configs/mvebu_v7_defconfig
index 627accea72fb..c80a5cbf6128 100644
--- a/arch/arm/configs/mvebu_v7_defconfig
+++ b/arch/arm/configs/mvebu_v7_defconfig
@@ -116,6 +116,7 @@ CONFIG_DMADEVICES=y
CONFIG_MV_XOR=y
# CONFIG_IOMMU_SUPPORT is not set
CONFIG_MEMORY=y
+CONFIG_PWM=y
CONFIG_EXT4_FS=y
CONFIG_ISO9660_FS=y
CONFIG_JOLIET=y
--
2.1.3
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 7/7] mvebu: wrt1900ac: Use pwm-fan rather than gpio-fan
2015-01-09 23:34 [PATCH 0/7] Add PWM support to mvebu gpio driver Andrew Lunn
` (5 preceding siblings ...)
2015-01-09 23:34 ` [PATCH 6/7] arm: mvebu: Enable PWM in defconfig Andrew Lunn
@ 2015-01-09 23:34 ` Andrew Lunn
2015-02-23 14:09 ` [PATCH 0/7] Add PWM support to mvebu gpio driver Gregory CLEMENT
7 siblings, 0 replies; 24+ messages in thread
From: Andrew Lunn @ 2015-01-09 23:34 UTC (permalink / raw)
To: linus.walleij
Cc: Thomas Petazzoni, kaloz, Gregory Clement, Sebastian Hesselbarth,
linux-gpio, linux-pwm, Andrew Lunn
The mvebu gpio driver can also perform PWM on some pins. Us the
pwm-fan driver to control the fan of the WRT1900AC, giving us fine
grain control over its speed and hence noise.
Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
arch/arm/boot/dts/armada-xp-wrt1900ac.dts | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/arch/arm/boot/dts/armada-xp-wrt1900ac.dts b/arch/arm/boot/dts/armada-xp-wrt1900ac.dts
index d53643ca2c0d..14a4d7cd4ff2 100644
--- a/arch/arm/boot/dts/armada-xp-wrt1900ac.dts
+++ b/arch/arm/boot/dts/armada-xp-wrt1900ac.dts
@@ -276,12 +276,10 @@
};
};
- 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@0 {
--
2.1.3
^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH 3/7] gpio: mvebu: Add limited PWM support
2015-01-09 23:34 ` [PATCH 3/7] gpio: mvebu: Add limited PWM support Andrew Lunn
@ 2015-01-12 11:05 ` Imre Kaloz
2015-01-12 13:12 ` Andrew Lunn
2015-01-12 14:06 ` Linus Walleij
2015-06-12 10:38 ` Thierry Reding
2 siblings, 1 reply; 24+ messages in thread
From: Imre Kaloz @ 2015-01-12 11:05 UTC (permalink / raw)
To: linus.walleij, Andrew Lunn
Cc: Thomas Petazzoni, Gregory Clement, Sebastian Hesselbarth,
linux-gpio, linux-pwm
On Sat, 10 Jan 2015 00:34:49 +0100, Andrew Lunn <andrew@lunn.ch> wrote:
<snip>
> diff --git a/drivers/gpio/gpio-mvebu-pwm.c
> b/drivers/gpio/gpio-mvebu-pwm.c
> new file mode 100644
> index 000000000000..8a5af11aed67
> --- /dev/null
> +++ b/drivers/gpio/gpio-mvebu-pwm.c
> @@ -0,0 +1,202 @@
> +#include <linux/err.h>
> +#include <linux/module.h>
> +#include <linux/gpio.h>
> +#include <linux/pwm.h>
> +#include <linux/clk.h>
> +#include <linux/platform_device.h>
> +#include "gpio-mvebu.h"
> +#include "gpiolib.h"
You also need to include <asm/io.h> for readl_relaxed, and a short
description/copyright on the top wouldn't hurt.
Imre
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 3/7] gpio: mvebu: Add limited PWM support
2015-01-12 11:05 ` Imre Kaloz
@ 2015-01-12 13:12 ` Andrew Lunn
0 siblings, 0 replies; 24+ messages in thread
From: Andrew Lunn @ 2015-01-12 13:12 UTC (permalink / raw)
To: Imre Kaloz
Cc: linus.walleij, Thomas Petazzoni, Gregory Clement,
Sebastian Hesselbarth, linux-gpio, linux-pwm
On Mon, Jan 12, 2015 at 12:05:50PM +0100, Imre Kaloz wrote:
> On Sat, 10 Jan 2015 00:34:49 +0100, Andrew Lunn <andrew@lunn.ch> wrote:
>
> <snip>
>
> >diff --git a/drivers/gpio/gpio-mvebu-pwm.c
> >b/drivers/gpio/gpio-mvebu-pwm.c
> >new file mode 100644
> >index 000000000000..8a5af11aed67
> >--- /dev/null
> >+++ b/drivers/gpio/gpio-mvebu-pwm.c
> >@@ -0,0 +1,202 @@
> >+#include <linux/err.h>
> >+#include <linux/module.h>
> >+#include <linux/gpio.h>
> >+#include <linux/pwm.h>
> >+#include <linux/clk.h>
> >+#include <linux/platform_device.h>
> >+#include "gpio-mvebu.h"
> >+#include "gpiolib.h"
>
> You also need to include <asm/io.h> for readl_relaxed, and a short
> description/copyright on the top wouldn't hurt.
Will do.
Thanks
Andrew
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 3/7] gpio: mvebu: Add limited PWM support
2015-01-09 23:34 ` [PATCH 3/7] gpio: mvebu: Add limited PWM support Andrew Lunn
2015-01-12 11:05 ` Imre Kaloz
@ 2015-01-12 14:06 ` Linus Walleij
2015-01-12 15:07 ` Andrew Lunn
2015-01-13 2:42 ` Andrew Lunn
2015-06-12 10:38 ` Thierry Reding
2 siblings, 2 replies; 24+ messages in thread
From: Linus Walleij @ 2015-01-12 14:06 UTC (permalink / raw)
To: Andrew Lunn, Thierry Reding, linux-pwm, Lee Jones, Samuel Ortiz
Cc: Thomas Petazzoni, Imre Kaloz, Gregory Clement,
Sebastian Hesselbarth, linux-gpio
On Sat, Jan 10, 2015 at 12:34 AM, Andrew Lunn <andrew@lunn.ch> wrote:
> 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 <andrew@lunn.ch>
> ---
> drivers/gpio/Kconfig | 5 ++
> drivers/gpio/Makefile | 1 +
> drivers/gpio/gpio-mvebu-pwm.c | 202 ++++++++++++++++++++++++++++++++++++++++++
(...)
> +++ b/drivers/gpio/gpio-mvebu-pwm.c
(...)
> +static const struct pwm_ops mvebu_pwm_ops = {
> + .request = mvebu_pwm_request,
> + .free = mvebu_pwm_free,
> + .config = mvebu_pwm_config,
> + .enable = mvebu_pwm_enable,
> + .disable = mvebu_pwm_disable,
> + .owner = THIS_MODULE,
> +};
So the first basic notes:
- PWM maintainer Thierry Reding has to review and ACK this, and the patch needs
to be posted to the linux-pwm mailing list too (check To: line)
- Should that driver portion really be in drivers/gpio or rather in drivers/pwm
- Why is this not modeled as an MFD spawning a GPIO and a PWM cell,
as is custom? (Bringing MFD maintainers into the picture.)
So I am aware that this takes the problem from "quick fix extension to the GPIO
driver" to "really nasty hairy re-engineering of the whole shebang" but there is
a lot of precedents in the kernel for splitting up this type of hardware in
separate drivers under an MFD hub.
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 3/7] gpio: mvebu: Add limited PWM support
2015-01-12 14:06 ` Linus Walleij
@ 2015-01-12 15:07 ` Andrew Lunn
2015-01-13 2:42 ` Andrew Lunn
1 sibling, 0 replies; 24+ messages in thread
From: Andrew Lunn @ 2015-01-12 15:07 UTC (permalink / raw)
To: Linus Walleij
Cc: Thierry Reding, linux-pwm, Lee Jones, Samuel Ortiz,
Thomas Petazzoni, Imre Kaloz, Gregory Clement,
Sebastian Hesselbarth, linux-gpio
On Mon, Jan 12, 2015 at 03:06:14PM +0100, Linus Walleij wrote:
> On Sat, Jan 10, 2015 at 12:34 AM, Andrew Lunn <andrew@lunn.ch> wrote:
>
> > 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 <andrew@lunn.ch>
> > ---
> > drivers/gpio/Kconfig | 5 ++
> > drivers/gpio/Makefile | 1 +
> > drivers/gpio/gpio-mvebu-pwm.c | 202 ++++++++++++++++++++++++++++++++++++++++++
> (...)
> > +++ b/drivers/gpio/gpio-mvebu-pwm.c
> (...)
> > +static const struct pwm_ops mvebu_pwm_ops = {
> > + .request = mvebu_pwm_request,
> > + .free = mvebu_pwm_free,
> > + .config = mvebu_pwm_config,
> > + .enable = mvebu_pwm_enable,
> > + .disable = mvebu_pwm_disable,
> > + .owner = THIS_MODULE,
> > +};
>
> So the first basic notes:
>
> - PWM maintainer Thierry Reding has to review and ACK this, and the patch needs
> to be posted to the linux-pwm mailing list too (check To: line)
Yes, Thierry was in To: and linux-pwm in Cc:
> - Should that driver portion really be in drivers/gpio or rather in drivers/pwm
I also had the same thoughts. What decided it for me is the use of
gpiolib.h. That file would have to move somewhere under
include/linux, or i find a different way of doing what i'm doing
without using it.
> - Why is this not modeled as an MFD spawning a GPIO and a PWM cell,
> as is custom? (Bringing MFD maintainers into the picture.)
>
> So I am aware that this takes the problem from "quick fix extension to the GPIO
> driver" to "really nasty hairy re-engineering of the whole shebang" but there is
> a lot of precedents in the kernel for splitting up this type of hardware in
> separate drivers under an MFD hub.
Yes, one example would be lp3943, an i2c GPIO and PWM driver.
The nasty hairy re-engineering is one thing that is putting me off.
Another is keeping DT compatibility. Here i'm just adding some
additional optional properties. I've not looked in detail, but i think
it is going to be really hard to add an MFD driver without breaking
the existing DT binding. That is not somewhere i want to go, i'd just
drop this altogether.
Do you know of any other driver with a DT binding which has been
refactored into an MFD? An example to look at would be useful.
Thanks
Andrew
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 3/7] gpio: mvebu: Add limited PWM support
2015-01-12 14:06 ` Linus Walleij
2015-01-12 15:07 ` Andrew Lunn
@ 2015-01-13 2:42 ` Andrew Lunn
2015-01-15 9:52 ` Linus Walleij
1 sibling, 1 reply; 24+ messages in thread
From: Andrew Lunn @ 2015-01-13 2:42 UTC (permalink / raw)
To: Linus Walleij
Cc: Thierry Reding, linux-pwm, Lee Jones, Samuel Ortiz,
Thomas Petazzoni, Imre Kaloz, Gregory Clement,
Sebastian Hesselbarth, linux-gpio
> - Why is this not modeled as an MFD spawning a GPIO and a PWM cell,
> as is custom? (Bringing MFD maintainers into the picture.)
>
> So I am aware that this takes the problem from "quick fix extension to the GPIO
> driver" to "really nasty hairy re-engineering of the whole shebang" but there is
> a lot of precedents in the kernel for splitting up this type of hardware in
> separate drivers under an MFD hub.
Hi Linus
So i thought about this some more. What would an MFD based solution
look like?
First issue is backwards compatibility. There are currently around 90
.dts files using this gpio driver. I could imagine a few of these
being changed to make use of an MFD based driver to make us of the new
features, but the rest expect backwards compatibility.
I think the only sensible way to achieve this is that the gpio driver
keeps its existing binding.
We then need to add a new MFD binding, with sub sections for the gpio
driver and the PWM driver. At a first stab, it would look something
like:
armada-gpio {
compatible = "marvell,armada-gpio";
reg = <0xd0018100 0x40>,
<0xd00181c0 0x08>;
gpio: gpio {
compatible = "marvell,armada-gpio-gpio";
ngpios = <32>;
gio-controller;
#gpio-cells = <2>;
interrupt-controller;
#interrupt-cells = <2>;
interrupts = <16>, <17>, <18>, <19>;
clocks = <&coreclk 0>;
};
pwm: pwm {
compatible = "marvell,armada-gpio-pwm";
#pwm-cells = <2>;
clocks = <&coreclk 0>;
};
};
This does not really describe the hardware. The hardware is more like:
gpio: gpio {
compatible = "marvell,orion-gpio";
reg = <0xd0018100 0x40>;
ngpios = <32>;
gio-controller;
#gpio-cells = <2>;
interrupt-controller;
#interrupt-cells = <2>;
interrupts = <16>, <17>, <18>, <19>;
clocks = <&coreclk 0>;
pwm: pwm {
compatible = "marvell,armada-pwm";
reg = <0xd00181c0 0x08>;
#pwm-cells = <2>;
clocks = <&coreclk 0>;
};
};
but i don't think MFD supports that sort of structure?
So assuming we go with the first binding above, this means the gpio
driver gains a second binding, probe function, etc for when it is
probed as part of an MFD.
Now it starts getting nasty hairy. The MFD core driver should offer a
set of functions to access the hardware, which are shared by the
gpio-gpio driver and the gpio-pwm driver. However when the gpio driver
is probed using its current binding, not the MFD binding, it also
needs access functions. So we need a library of access functions,
which can be used directly or via the MFD code. Where do we place
these? In the gpio driver would make sense, since gpio driver always
needs them, the MFD is optional. But they could be in the MFD
driver. The gpio driver is currently built in, meaning the MFD driver
would be built in, so even if the MFD driver is never probed, it can
still export functions.
Memory management then becomes fun. If the MFD is probed, it needs to
allocated the shared memory, used by the access functions etc. If the
standalone gpio driver is probed, it needs to allocated the memory
used by the access functions etc.
I'm also worried about race conditions during probe. The pwm driver is
not independent of the gpio driver. It is a child of the gpio
driver. It needs to know the gpiochip base and ngpio during its own
probe function. In effect, we need to ensure that the MFD gpio probe
has completed successfully before the MFD pwm probe is called.
Nothing completely unsolvable, it just seems ugly and complex.
Humm, that second binding just gave me an idea. The pwm driver is a
child of the gpio driver. That is the same relationship between an MFD
driver and its children. So maybe we should move mvebu-gpio.c into
drives/mfd and add some mfd functionality so it can mfd_add_device()
the pwm driver when it has completed its own probe? That nicely solves
the probe race issue, and the library of access functions etc. The
gpio binding is also backwards compatible since it is being extended
with an optional MFD child device.
Andrew
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 1/7] gpio: mvebu: checkpatch fixes
2015-01-09 23:34 ` [PATCH 1/7] gpio: mvebu: checkpatch fixes Andrew Lunn
@ 2015-01-14 13:04 ` Linus Walleij
0 siblings, 0 replies; 24+ messages in thread
From: Linus Walleij @ 2015-01-14 13:04 UTC (permalink / raw)
To: Andrew Lunn
Cc: Thomas Petazzoni, Imre Kaloz, Gregory Clement,
Sebastian Hesselbarth, linux-gpio, linux-pwm
On Sat, Jan 10, 2015 at 12:34 AM, Andrew Lunn <andrew@lunn.ch> wrote:
> Wrap some long lines.
> Prefer seq_puts() over seq_printf().
> space to tab conversions.
> Spelling error fix.
>
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>
Patch applied.
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 2/7] gpio: mvebu: Fix probe cleanup on error
2015-01-09 23:34 ` [PATCH 2/7] gpio: mvebu: Fix probe cleanup on error Andrew Lunn
@ 2015-01-14 13:05 ` Linus Walleij
0 siblings, 0 replies; 24+ messages in thread
From: Linus Walleij @ 2015-01-14 13:05 UTC (permalink / raw)
To: Andrew Lunn
Cc: Thomas Petazzoni, Imre Kaloz, Gregory Clement,
Sebastian Hesselbarth, linux-gpio, linux-pwm
On Sat, Jan 10, 2015 at 12:34 AM, Andrew Lunn <andrew@lunn.ch> wrote:
> Ensure that when there is an error during probe that the gpiochip is
> removed and the generic irq chip is removed.
>
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>
Patch applied.
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 3/7] gpio: mvebu: Add limited PWM support
2015-01-13 2:42 ` Andrew Lunn
@ 2015-01-15 9:52 ` Linus Walleij
2015-01-17 20:04 ` Andrew Lunn
` (2 more replies)
0 siblings, 3 replies; 24+ messages in thread
From: Linus Walleij @ 2015-01-15 9:52 UTC (permalink / raw)
To: Andrew Lunn
Cc: Thierry Reding, linux-pwm, Lee Jones, Samuel Ortiz,
Thomas Petazzoni, Imre Kaloz, Gregory Clement,
Sebastian Hesselbarth, linux-gpio
On Tue, Jan 13, 2015 at 3:42 AM, Andrew Lunn <andrew@lunn.ch> wrote:
> So i thought about this some more. What would an MFD based solution
> look like?
>
> First issue is backwards compatibility. There are currently around 90
> .dts files using this gpio driver. I could imagine a few of these
> being changed to make use of an MFD based driver to make us of the new
> features, but the rest expect backwards compatibility.
Good point.
> I think the only sensible way to achieve this is that the gpio driver
> keeps its existing binding.
Yup.
> This does not really describe the hardware. The hardware is more like:
>
> gpio: gpio {
> compatible = "marvell,orion-gpio";
> reg = <0xd0018100 0x40>;
> ngpios = <32>;
> gio-controller;
> #gpio-cells = <2>;
> interrupt-controller;
> #interrupt-cells = <2>;
> interrupts = <16>, <17>, <18>, <19>;
> clocks = <&coreclk 0>;
>
> pwm: pwm {
> compatible = "marvell,armada-pwm";
> reg = <0xd00181c0 0x08>;
> #pwm-cells = <2>;
> clocks = <&coreclk 0>;
> };
> };
>
> but i don't think MFD supports that sort of structure?
No it would have to be some custom DT code in the GPIO driver
spawning the PWM platform device.
I think it's better if we either go with the first solution of a combined
GPIO+PWM node (it's also elegant in a way, and perfectly
OK with device tree I think) but I want the PWM maintainer to
say if it's OK to have a PWM driver inside a GPIO driver.
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 3/7] gpio: mvebu: Add limited PWM support
2015-01-15 9:52 ` Linus Walleij
@ 2015-01-17 20:04 ` Andrew Lunn
2015-01-18 10:04 ` Lee Jones
2015-01-19 13:01 ` Thierry Reding
2 siblings, 0 replies; 24+ messages in thread
From: Andrew Lunn @ 2015-01-17 20:04 UTC (permalink / raw)
Cc: Thierry Reding, linux-pwm, Lee Jones, Samuel Ortiz,
Thomas Petazzoni, Imre Kaloz, Gregory Clement,
Sebastian Hesselbarth, linux-gpio, Linus Walleij
> I think it's better if we either go with the first solution of a combined
> GPIO+PWM node (it's also elegant in a way, and perfectly
> OK with device tree I think) but I want the PWM maintainer to
> say if it's OK to have a PWM driver inside a GPIO driver.
Hi Thierry
Please could you comment on this. I would like to get moving forward
with getting these patches accepted.
Thanks
Andrew
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 3/7] gpio: mvebu: Add limited PWM support
2015-01-15 9:52 ` Linus Walleij
2015-01-17 20:04 ` Andrew Lunn
@ 2015-01-18 10:04 ` Lee Jones
2015-01-19 12:59 ` Thierry Reding
2015-01-19 13:01 ` Thierry Reding
2 siblings, 1 reply; 24+ messages in thread
From: Lee Jones @ 2015-01-18 10:04 UTC (permalink / raw)
To: Linus Walleij
Cc: Andrew Lunn, Thierry Reding, linux-pwm, Samuel Ortiz,
Thomas Petazzoni, Imre Kaloz, Gregory Clement,
Sebastian Hesselbarth, linux-gpio
On Thu, 15 Jan 2015, Linus Walleij wrote:
> On Tue, Jan 13, 2015 at 3:42 AM, Andrew Lunn <andrew@lunn.ch> wrote:
>
> > So i thought about this some more. What would an MFD based solution
> > look like?
> >
> > First issue is backwards compatibility. There are currently around 90
> > .dts files using this gpio driver. I could imagine a few of these
> > being changed to make use of an MFD based driver to make us of the new
> > features, but the rest expect backwards compatibility.
>
> Good point.
>
> > I think the only sensible way to achieve this is that the gpio driver
> > keeps its existing binding.
>
> Yup.
>
> > This does not really describe the hardware. The hardware is more like:
> >
> > gpio: gpio {
> > compatible = "marvell,orion-gpio";
> > reg = <0xd0018100 0x40>;
> > ngpios = <32>;
> > gio-controller;
> > #gpio-cells = <2>;
> > interrupt-controller;
> > #interrupt-cells = <2>;
> > interrupts = <16>, <17>, <18>, <19>;
> > clocks = <&coreclk 0>;
> >
> > pwm: pwm {
> > compatible = "marvell,armada-pwm";
> > reg = <0xd00181c0 0x08>;
> > #pwm-cells = <2>;
> > clocks = <&coreclk 0>;
> > };
> > };
> >
> > but i don't think MFD supports that sort of structure?
>
> No it would have to be some custom DT code in the GPIO driver
> spawning the PWM platform device.
of_platform_populate()?
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 3/7] gpio: mvebu: Add limited PWM support
2015-01-18 10:04 ` Lee Jones
@ 2015-01-19 12:59 ` Thierry Reding
2015-01-20 10:52 ` Lee Jones
0 siblings, 1 reply; 24+ messages in thread
From: Thierry Reding @ 2015-01-19 12:59 UTC (permalink / raw)
To: Lee Jones
Cc: Linus Walleij, Andrew Lunn, linux-pwm, Samuel Ortiz,
Thomas Petazzoni, Imre Kaloz, Gregory Clement,
Sebastian Hesselbarth, linux-gpio
[-- Attachment #1: Type: text/plain, Size: 2093 bytes --]
On Sun, Jan 18, 2015 at 10:04:38AM +0000, Lee Jones wrote:
> On Thu, 15 Jan 2015, Linus Walleij wrote:
>
> > On Tue, Jan 13, 2015 at 3:42 AM, Andrew Lunn <andrew@lunn.ch> wrote:
> >
> > > So i thought about this some more. What would an MFD based solution
> > > look like?
> > >
> > > First issue is backwards compatibility. There are currently around 90
> > > .dts files using this gpio driver. I could imagine a few of these
> > > being changed to make use of an MFD based driver to make us of the new
> > > features, but the rest expect backwards compatibility.
> >
> > Good point.
> >
> > > I think the only sensible way to achieve this is that the gpio driver
> > > keeps its existing binding.
> >
> > Yup.
> >
> > > This does not really describe the hardware. The hardware is more like:
> > >
> > > gpio: gpio {
> > > compatible = "marvell,orion-gpio";
> > > reg = <0xd0018100 0x40>;
> > > ngpios = <32>;
> > > gio-controller;
> > > #gpio-cells = <2>;
> > > interrupt-controller;
> > > #interrupt-cells = <2>;
> > > interrupts = <16>, <17>, <18>, <19>;
> > > clocks = <&coreclk 0>;
> > >
> > > pwm: pwm {
> > > compatible = "marvell,armada-pwm";
> > > reg = <0xd00181c0 0x08>;
> > > #pwm-cells = <2>;
> > > clocks = <&coreclk 0>;
> > > };
> > > };
I think you technically need an (empty) ranges property in the gpio node
so that the address can be properly translated.
> > >
> > > but i don't think MFD supports that sort of structure?
> >
> > No it would have to be some custom DT code in the GPIO driver
> > spawning the PWM platform device.
>
> of_platform_populate()?
Huh? I was under the impression that mfd_add_devices() would already
deal with this situation? Isn't that what's parsed based on the cells
parameter?
Thierry
[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 3/7] gpio: mvebu: Add limited PWM support
2015-01-15 9:52 ` Linus Walleij
2015-01-17 20:04 ` Andrew Lunn
2015-01-18 10:04 ` Lee Jones
@ 2015-01-19 13:01 ` Thierry Reding
2 siblings, 0 replies; 24+ messages in thread
From: Thierry Reding @ 2015-01-19 13:01 UTC (permalink / raw)
To: Linus Walleij
Cc: Andrew Lunn, linux-pwm, Lee Jones, Samuel Ortiz,
Thomas Petazzoni, Imre Kaloz, Gregory Clement,
Sebastian Hesselbarth, linux-gpio
[-- Attachment #1: Type: text/plain, Size: 2160 bytes --]
On Thu, Jan 15, 2015 at 10:52:51AM +0100, Linus Walleij wrote:
> On Tue, Jan 13, 2015 at 3:42 AM, Andrew Lunn <andrew@lunn.ch> wrote:
>
> > So i thought about this some more. What would an MFD based solution
> > look like?
> >
> > First issue is backwards compatibility. There are currently around 90
> > .dts files using this gpio driver. I could imagine a few of these
> > being changed to make use of an MFD based driver to make us of the new
> > features, but the rest expect backwards compatibility.
>
> Good point.
>
> > I think the only sensible way to achieve this is that the gpio driver
> > keeps its existing binding.
>
> Yup.
>
> > This does not really describe the hardware. The hardware is more like:
> >
> > gpio: gpio {
> > compatible = "marvell,orion-gpio";
> > reg = <0xd0018100 0x40>;
> > ngpios = <32>;
> > gio-controller;
> > #gpio-cells = <2>;
> > interrupt-controller;
> > #interrupt-cells = <2>;
> > interrupts = <16>, <17>, <18>, <19>;
> > clocks = <&coreclk 0>;
> >
> > pwm: pwm {
> > compatible = "marvell,armada-pwm";
> > reg = <0xd00181c0 0x08>;
> > #pwm-cells = <2>;
> > clocks = <&coreclk 0>;
> > };
> > };
> >
> > but i don't think MFD supports that sort of structure?
>
> No it would have to be some custom DT code in the GPIO driver
> spawning the PWM platform device.
>
> I think it's better if we either go with the first solution of a combined
> GPIO+PWM node (it's also elegant in a way, and perfectly
> OK with device tree I think) but I want the PWM maintainer to
> say if it's OK to have a PWM driver inside a GPIO driver.
I'm fine with that, too. I'd request an update to MAINTAINERS so that at
least linux-pwm@vger.kernel.org gets included on patches against the
driver.
That said, the above DT description would lend itself nicely to MFD in
my opinion.
Thierry
[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 3/7] gpio: mvebu: Add limited PWM support
2015-01-19 12:59 ` Thierry Reding
@ 2015-01-20 10:52 ` Lee Jones
0 siblings, 0 replies; 24+ messages in thread
From: Lee Jones @ 2015-01-20 10:52 UTC (permalink / raw)
To: Thierry Reding
Cc: Linus Walleij, Andrew Lunn, linux-pwm, Samuel Ortiz,
Thomas Petazzoni, Imre Kaloz, Gregory Clement,
Sebastian Hesselbarth, linux-gpio
On Mon, 19 Jan 2015, Thierry Reding wrote:
> On Sun, Jan 18, 2015 at 10:04:38AM +0000, Lee Jones wrote:
> > On Thu, 15 Jan 2015, Linus Walleij wrote:
> >
> > > On Tue, Jan 13, 2015 at 3:42 AM, Andrew Lunn <andrew@lunn.ch> wrote:
> > >
> > > > So i thought about this some more. What would an MFD based solution
> > > > look like?
> > > >
> > > > First issue is backwards compatibility. There are currently around 90
> > > > .dts files using this gpio driver. I could imagine a few of these
> > > > being changed to make use of an MFD based driver to make us of the new
> > > > features, but the rest expect backwards compatibility.
> > >
> > > Good point.
> > >
> > > > I think the only sensible way to achieve this is that the gpio driver
> > > > keeps its existing binding.
> > >
> > > Yup.
> > >
> > > > This does not really describe the hardware. The hardware is more like:
> > > >
> > > > gpio: gpio {
> > > > compatible = "marvell,orion-gpio";
> > > > reg = <0xd0018100 0x40>;
> > > > ngpios = <32>;
> > > > gio-controller;
> > > > #gpio-cells = <2>;
> > > > interrupt-controller;
> > > > #interrupt-cells = <2>;
> > > > interrupts = <16>, <17>, <18>, <19>;
> > > > clocks = <&coreclk 0>;
> > > >
> > > > pwm: pwm {
> > > > compatible = "marvell,armada-pwm";
> > > > reg = <0xd00181c0 0x08>;
> > > > #pwm-cells = <2>;
> > > > clocks = <&coreclk 0>;
> > > > };
> > > > };
>
> I think you technically need an (empty) ranges property in the gpio node
> so that the address can be properly translated.
>
> > > >
> > > > but i don't think MFD supports that sort of structure?
> > >
> > > No it would have to be some custom DT code in the GPIO driver
> > > spawning the PWM platform device.
> >
> > of_platform_populate()?
>
> Huh? I was under the impression that mfd_add_devices() would already
> deal with this situation? Isn't that what's parsed based on the cells
> parameter?
There was talk of this _not_ being an MFD device. I was providing an
alternative way to get this device registered.
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 0/7] Add PWM support to mvebu gpio driver
2015-01-09 23:34 [PATCH 0/7] Add PWM support to mvebu gpio driver Andrew Lunn
` (6 preceding siblings ...)
2015-01-09 23:34 ` [PATCH 7/7] mvebu: wrt1900ac: Use pwm-fan rather than gpio-fan Andrew Lunn
@ 2015-02-23 14:09 ` Gregory CLEMENT
2015-02-23 15:48 ` Andrew Lunn
7 siblings, 1 reply; 24+ messages in thread
From: Gregory CLEMENT @ 2015-02-23 14:09 UTC (permalink / raw)
To: Andrew Lunn
Cc: linus.walleij, Thomas Petazzoni, kaloz, Sebastian Hesselbarth,
linux-gpio, linux-pwm
Hi Andrew,
On 10/01/2015 00:34, Andrew Lunn wrote:
> This patchset starts out with some cleanups and fixes for the mvebu
> gpio driver. Then new functionality is added. The Armada 370 and XP
> SoC has the ability to "blink" the gpio lines at a configuration on
> and off duration. So simple PWM support is added. However only a
> single gpio line per gpio chip can be used as a PWM line due to
> limitations of the hardware. The last patch shows one example of how
> this could be used, converting a gpio-fan into a pwm-fan, so allowing
> finer control over its rotation speed, and hence noise. This last
> patch is not expected to be accepted, since the device is not yet in
> mainline.
>
> Andrew Lunn (7):
> gpio: mvebu: checkpatch fixes
> gpio: mvebu: Fix probe cleanup on error
> gpio: mvebu: Add limited PWM support
> DT: bindings: Extend mvebu gpio documentation with PWM
> mvebu: xp: Add pwm properties to .dtsi files
> arm: mvebu: Enable PWM in defconfig
> mvebu: wrt1900ac: Use pwm-fan rather than gpio-fan
>
What is the status of this series?
I saw that the 2 first patches have been merged but what about the patch 3 and 4?
Do you plan to send a new series?
The patches 5 and 6 are good candidates to for to the mvebu tree but of course only
if the patch 3 is merged.
Thanks,
Gregory
> .../devicetree/bindings/gpio/gpio-mvebu.txt | 31 ++++
> 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 | 8 +-
> arch/arm/boot/dts/armada-xp-mv78460.dtsi | 10 +-
> arch/arm/boot/dts/armada-xp-wrt1900ac.dts | 8 +-
> arch/arm/configs/mvebu_v7_defconfig | 1 +
> drivers/gpio/Kconfig | 5 +
> drivers/gpio/Makefile | 1 +
> drivers/gpio/gpio-mvebu-pwm.c | 202 +++++++++++++++++++++
> drivers/gpio/gpio-mvebu.c | 133 +++++++-------
> drivers/gpio/gpio-mvebu.h | 79 ++++++++
> 12 files changed, 421 insertions(+), 77 deletions(-)
> create mode 100644 drivers/gpio/gpio-mvebu-pwm.c
> create mode 100644 drivers/gpio/gpio-mvebu.h
>
--
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 0/7] Add PWM support to mvebu gpio driver
2015-02-23 14:09 ` [PATCH 0/7] Add PWM support to mvebu gpio driver Gregory CLEMENT
@ 2015-02-23 15:48 ` Andrew Lunn
0 siblings, 0 replies; 24+ messages in thread
From: Andrew Lunn @ 2015-02-23 15:48 UTC (permalink / raw)
To: Gregory CLEMENT
Cc: linus.walleij, Thomas Petazzoni, kaloz, Sebastian Hesselbarth,
linux-gpio, linux-pwm
On Mon, Feb 23, 2015 at 03:09:32PM +0100, Gregory CLEMENT wrote:
> Hi Andrew,
>
> On 10/01/2015 00:34, Andrew Lunn wrote:
> > This patchset starts out with some cleanups and fixes for the mvebu
> > gpio driver. Then new functionality is added. The Armada 370 and XP
> > SoC has the ability to "blink" the gpio lines at a configuration on
> > and off duration. So simple PWM support is added. However only a
> > single gpio line per gpio chip can be used as a PWM line due to
> > limitations of the hardware. The last patch shows one example of how
> > this could be used, converting a gpio-fan into a pwm-fan, so allowing
> > finer control over its rotation speed, and hence noise. This last
> > patch is not expected to be accepted, since the device is not yet in
> > mainline.
> >
> > Andrew Lunn (7):
> > gpio: mvebu: checkpatch fixes
> > gpio: mvebu: Fix probe cleanup on error
> > gpio: mvebu: Add limited PWM support
> > DT: bindings: Extend mvebu gpio documentation with PWM
> > mvebu: xp: Add pwm properties to .dtsi files
> > arm: mvebu: Enable PWM in defconfig
> > mvebu: wrt1900ac: Use pwm-fan rather than gpio-fan
> >
>
> What is the status of this series?
> I saw that the 2 first patches have been merged but what about the patch 3 and 4?
>
> Do you plan to send a new series?
Yes, i plan to rebase onto -rc and resend, addressing the comments i
received.
Andrew
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 3/7] gpio: mvebu: Add limited PWM support
2015-01-09 23:34 ` [PATCH 3/7] gpio: mvebu: Add limited PWM support Andrew Lunn
2015-01-12 11:05 ` Imre Kaloz
2015-01-12 14:06 ` Linus Walleij
@ 2015-06-12 10:38 ` Thierry Reding
2 siblings, 0 replies; 24+ messages in thread
From: Thierry Reding @ 2015-06-12 10:38 UTC (permalink / raw)
To: Andrew Lunn
Cc: linus.walleij, Thomas Petazzoni, kaloz, Gregory Clement,
Sebastian Hesselbarth, linux-gpio, linux-pwm
[-- Attachment #1: Type: text/plain, Size: 1190 bytes --]
On Sat, Jan 10, 2015 at 12:34:49AM +0100, Andrew Lunn wrote:
> 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 <andrew@lunn.ch>
> ---
> drivers/gpio/Kconfig | 5 ++
> drivers/gpio/Makefile | 1 +
> drivers/gpio/gpio-mvebu-pwm.c | 202 ++++++++++++++++++++++++++++++++++++++++++
> drivers/gpio/gpio-mvebu.c | 37 +++-----
> drivers/gpio/gpio-mvebu.h | 79 +++++++++++++++++
> 5 files changed, 299 insertions(+), 25 deletions(-)
> create mode 100644 drivers/gpio/gpio-mvebu-pwm.c
> create mode 100644 drivers/gpio/gpio-mvebu.h
FWIW, I think this could easily just be all in the gpio-mvebu.c driver,
no need to split it up.
Thierry
[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 24+ messages in thread
end of thread, other threads:[~2015-06-12 10:38 UTC | newest]
Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-01-09 23:34 [PATCH 0/7] Add PWM support to mvebu gpio driver Andrew Lunn
2015-01-09 23:34 ` [PATCH 1/7] gpio: mvebu: checkpatch fixes Andrew Lunn
2015-01-14 13:04 ` Linus Walleij
2015-01-09 23:34 ` [PATCH 2/7] gpio: mvebu: Fix probe cleanup on error Andrew Lunn
2015-01-14 13:05 ` Linus Walleij
2015-01-09 23:34 ` [PATCH 3/7] gpio: mvebu: Add limited PWM support Andrew Lunn
2015-01-12 11:05 ` Imre Kaloz
2015-01-12 13:12 ` Andrew Lunn
2015-01-12 14:06 ` Linus Walleij
2015-01-12 15:07 ` Andrew Lunn
2015-01-13 2:42 ` Andrew Lunn
2015-01-15 9:52 ` Linus Walleij
2015-01-17 20:04 ` Andrew Lunn
2015-01-18 10:04 ` Lee Jones
2015-01-19 12:59 ` Thierry Reding
2015-01-20 10:52 ` Lee Jones
2015-01-19 13:01 ` Thierry Reding
2015-06-12 10:38 ` Thierry Reding
2015-01-09 23:34 ` [PATCH 4/7] DT: bindings: Extend mvebu gpio documentation with PWM Andrew Lunn
2015-01-09 23:34 ` [PATCH 5/7] mvebu: xp: Add pwm properties to .dtsi files Andrew Lunn
2015-01-09 23:34 ` [PATCH 6/7] arm: mvebu: Enable PWM in defconfig Andrew Lunn
2015-01-09 23:34 ` [PATCH 7/7] mvebu: wrt1900ac: Use pwm-fan rather than gpio-fan Andrew Lunn
2015-02-23 14:09 ` [PATCH 0/7] Add PWM support to mvebu gpio driver Gregory CLEMENT
2015-02-23 15:48 ` Andrew Lunn
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.