Re: [PATCH 2/2] ARM: mvebu: Enable ARCH_WANT_LIBATA_LEDS in Armada 38x

2018-09-26 Thread Aditya Prayoga
On Thu, Sep 20, 2018 at 2:26 PM Pavel Machek  wrote:
>
> On Wed 2018-09-19 11:45:30, Aditya Prayoga wrote:
> > Enable hidden symbol ARCH_WANT_LIBATA_LEDS so CONFIG_ATA_LEDS can be
> > used in kernel configuration.
>
> Should that be hidden symbol and should that be architecture specific?
>

The intention was to minimize the risk of the ledtrig code being
included by accident. The
code itself does not have architecture specific instruction

> For a notebook, I may want scrolllock LED to indicate ATA activity
> (because I'm using USB keyboard and can't see the disk led).
>
> Hmm. And while at it... it would be nice to have option to watch all
> ATA ports combined, as well.

but there is disk-activity trigger for that purpose.

Aditya

> Pavel
> --
> (english) http://www.livejournal.com/~pavelmachek
> (cesky, pictures) 
> http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


Re: [PATCH 2/2] ARM: mvebu: Enable ARCH_WANT_LIBATA_LEDS in Armada 38x

2018-09-26 Thread Aditya Prayoga
On Thu, Sep 20, 2018 at 2:26 PM Pavel Machek  wrote:
>
> On Wed 2018-09-19 11:45:30, Aditya Prayoga wrote:
> > Enable hidden symbol ARCH_WANT_LIBATA_LEDS so CONFIG_ATA_LEDS can be
> > used in kernel configuration.
>
> Should that be hidden symbol and should that be architecture specific?
>

The intention was to minimize the risk of the ledtrig code being
included by accident. The
code itself does not have architecture specific instruction

> For a notebook, I may want scrolllock LED to indicate ATA activity
> (because I'm using USB keyboard and can't see the disk led).
>
> Hmm. And while at it... it would be nice to have option to watch all
> ATA ports combined, as well.

but there is disk-activity trigger for that purpose.

Aditya

> Pavel
> --
> (english) http://www.livejournal.com/~pavelmachek
> (cesky, pictures) 
> http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


Re: [PATCH 1/2] libata: add ledtrig support

2018-09-20 Thread Aditya Prayoga
Hi Andrew,

thank you for your feedback. It seem i also need to resolve the issue
reported by kbuild test robot.

Aditya


Re: [PATCH 1/2] libata: add ledtrig support

2018-09-20 Thread Aditya Prayoga
Hi Andrew,

thank you for your feedback. It seem i also need to resolve the issue
reported by kbuild test robot.

Aditya


[PATCH 2/2] ARM: mvebu: Enable ARCH_WANT_LIBATA_LEDS in Armada 38x

2018-09-18 Thread Aditya Prayoga
Enable hidden symbol ARCH_WANT_LIBATA_LEDS so CONFIG_ATA_LEDS can be
used in kernel configuration.

URL: https://lists.openwrt.org/pipermail/openwrt-
devel/2017-March/006582.html

Signed-off-by: Aditya Prayoga 
---
 arch/arm/mach-mvebu/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/mach-mvebu/Kconfig b/arch/arm/mach-mvebu/Kconfig
index 2c20599..51f3256 100644
--- a/arch/arm/mach-mvebu/Kconfig
+++ b/arch/arm/mach-mvebu/Kconfig
@@ -68,6 +68,7 @@ config MACH_ARMADA_38X
select HAVE_SMP
select MACH_MVEBU_V7
select PINCTRL_ARMADA_38X
+   select ARCH_WANT_LIBATA_LEDS
help
  Say 'Y' here if you want your kernel to support boards based
  on the Marvell Armada 380/385 SoC with device tree.
-- 
2.7.4



[PATCH 2/2] ARM: mvebu: Enable ARCH_WANT_LIBATA_LEDS in Armada 38x

2018-09-18 Thread Aditya Prayoga
Enable hidden symbol ARCH_WANT_LIBATA_LEDS so CONFIG_ATA_LEDS can be
used in kernel configuration.

URL: https://lists.openwrt.org/pipermail/openwrt-
devel/2017-March/006582.html

Signed-off-by: Aditya Prayoga 
---
 arch/arm/mach-mvebu/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/mach-mvebu/Kconfig b/arch/arm/mach-mvebu/Kconfig
index 2c20599..51f3256 100644
--- a/arch/arm/mach-mvebu/Kconfig
+++ b/arch/arm/mach-mvebu/Kconfig
@@ -68,6 +68,7 @@ config MACH_ARMADA_38X
select HAVE_SMP
select MACH_MVEBU_V7
select PINCTRL_ARMADA_38X
+   select ARCH_WANT_LIBATA_LEDS
help
  Say 'Y' here if you want your kernel to support boards based
  on the Marvell Armada 380/385 SoC with device tree.
-- 
2.7.4



[PATCH 1/2] libata: add ledtrig support

2018-09-18 Thread Aditya Prayoga
From: Daniel Golle 

This adds a LED trigger for each ATA port indicating disk activity.

As this is needed only on specific platforms (NAS SoCs and such),
these platforms should define ARCH_WANTS_LIBATA_LEDS if there
are boards with LED(s) intended to indicate ATA disk activity and
need the OS to take care of that.
In that way, if not selected, LED trigger support not will be
included in libata-core and both, codepaths and structures remain
untouched.

I'm currently deploying this for the oxnas target in OpenWrt
https://dev.openwrt.org/changeset/43675/

v2: rebased to kernel/git/tj/libata.git
plus small corrections and comments added

Signed-off-by: Daniel Golle 
URL: https://patchwork.ozlabs.org/patch/420733/
[Aditya Prayoga:
* Port forward
* Change ATA_LEDS default to no
* Reduce performance impact by moving ata_led_act() call from
ata_qc_new() to ata_qc_complete()]
Signed-off-by: Aditya Prayoga 

---

If there is anything fundamentally wrong with that approach, I'd be
glad for some advise on how to do it better.
I conciously choose an #ifdev approach to make sure performance will
not be affected for non-users of that code.

Thanks

Daniel

---
---
 drivers/ata/Kconfig   | 16 ++
 drivers/ata/libata-core.c | 56 +++
 include/linux/libata.h|  7 ++
 3 files changed, 79 insertions(+)

diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig
index 39b181d..a2c64ce 100644
--- a/drivers/ata/Kconfig
+++ b/drivers/ata/Kconfig
@@ -46,6 +46,22 @@ config ATA_VERBOSE_ERROR
 
  If unsure, say Y.
 
+config ARCH_WANT_LIBATA_LEDS
+   bool
+
+config ATA_LEDS
+   bool "support ATA port LED triggers"
+   depends on ARCH_WANT_LIBATA_LEDS
+   select NEW_LEDS
+   select LEDS_CLASS
+   select LEDS_TRIGGERS
+   default n
+   help
+ This option adds a LED trigger for each registered ATA port.
+ It is used to drive disk activity leds connected via GPIO.
+
+ If unsure, say N.
+
 config ATA_ACPI
bool "ATA ACPI Support"
depends on ACPI
diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 599e01b..65228f5 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -5105,6 +5105,30 @@ void swap_buf_le16(u16 *buf, unsigned int buf_words)
 }
 
 /**
+ * ata_led_act - Trigger port activity LED
+ * @ap: indicating port
+ *
+ * Blinks any LEDs registered to the trigger.
+ * Commonly used with leds-gpio on NAS systems with disk activity
+ * indicator LEDs.
+ *
+ * LOCKING:
+ * None.
+ */
+static inline void ata_led_act(struct ata_port *ap)
+{
+#ifdef CONFIG_ATA_LEDS
+#define LIBATA_BLINK_DELAY 20 /* ms */
+   unsigned long led_delay = LIBATA_BLINK_DELAY;
+
+   if (unlikely(!ap->ledtrig))
+   return;
+
+   led_trigger_blink_oneshot(ap->ledtrig, _delay, _delay, 0);
+#endif /* CONFIG_ATA_LEDS */
+}
+
+/**
  * ata_qc_new_init - Request an available ATA command, and initialize it
  * @dev: Device from whom we request an available command structure
  * @tag: tag
@@ -5249,6 +5273,10 @@ void ata_qc_complete(struct ata_queued_cmd *qc)
/* Trigger the LED (if available) */
ledtrig_disk_activity(!!(qc->tf.flags & ATA_TFLAG_WRITE));
 
+#ifdef CONFIG_ATA_LEDS
+   ata_led_act(ap);
+#endif
+
/* XXX: New EH and old EH use different mechanisms to
 * synchronize EH with regular execution path.
 *
@@ -6028,6 +6056,9 @@ struct ata_port *ata_port_alloc(struct ata_host *host)
ap->stats.unhandled_irq = 1;
ap->stats.idle_irq = 1;
 #endif
+#ifdef CONFIG_ATA_LEDS
+   ap->ledtrig = kzalloc(sizeof(struct led_trigger), GFP_KERNEL);
+#endif
ata_sff_port_init(ap);
 
return ap;
@@ -6063,6 +6094,12 @@ static void ata_host_release(struct kref *kref)
 
kfree(ap->pmp_link);
kfree(ap->slave_link);
+#ifdef CONFIG_ATA_LEDS
+   if (ap->ledtrig) {
+   led_trigger_unregister(ap->ledtrig);
+   kfree(ap->ledtrig);
+   };
+#endif
kfree(ap);
host->ports[i] = NULL;
}
@@ -6527,6 +6564,25 @@ int ata_host_register(struct ata_host *host, struct 
scsi_host_template *sht)
host->ports[i]->local_port_no = i + 1;
}
 
+#ifdef CONFIG_ATA_LEDS
+   /* register LED triggers for all ports */
+   for (i = 0; i < host->n_ports; i++) {
+   if (unlikely(!host->ports[i]->ledtrig))
+   continue;
+
+   snprintf(host->ports[i]->ledtrig_name,
+   sizeof(host->ports[i]->ledtrig_name), "ata%u",
+   host->ports[i]->print_id);
+
+   host->ports[i]->ledtrig->name = host->ports[i]->ledtrig_name;
+
+

[PATCH 1/2] libata: add ledtrig support

2018-09-18 Thread Aditya Prayoga
From: Daniel Golle 

This adds a LED trigger for each ATA port indicating disk activity.

As this is needed only on specific platforms (NAS SoCs and such),
these platforms should define ARCH_WANTS_LIBATA_LEDS if there
are boards with LED(s) intended to indicate ATA disk activity and
need the OS to take care of that.
In that way, if not selected, LED trigger support not will be
included in libata-core and both, codepaths and structures remain
untouched.

I'm currently deploying this for the oxnas target in OpenWrt
https://dev.openwrt.org/changeset/43675/

v2: rebased to kernel/git/tj/libata.git
plus small corrections and comments added

Signed-off-by: Daniel Golle 
URL: https://patchwork.ozlabs.org/patch/420733/
[Aditya Prayoga:
* Port forward
* Change ATA_LEDS default to no
* Reduce performance impact by moving ata_led_act() call from
ata_qc_new() to ata_qc_complete()]
Signed-off-by: Aditya Prayoga 

---

If there is anything fundamentally wrong with that approach, I'd be
glad for some advise on how to do it better.
I conciously choose an #ifdev approach to make sure performance will
not be affected for non-users of that code.

Thanks

Daniel

---
---
 drivers/ata/Kconfig   | 16 ++
 drivers/ata/libata-core.c | 56 +++
 include/linux/libata.h|  7 ++
 3 files changed, 79 insertions(+)

diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig
index 39b181d..a2c64ce 100644
--- a/drivers/ata/Kconfig
+++ b/drivers/ata/Kconfig
@@ -46,6 +46,22 @@ config ATA_VERBOSE_ERROR
 
  If unsure, say Y.
 
+config ARCH_WANT_LIBATA_LEDS
+   bool
+
+config ATA_LEDS
+   bool "support ATA port LED triggers"
+   depends on ARCH_WANT_LIBATA_LEDS
+   select NEW_LEDS
+   select LEDS_CLASS
+   select LEDS_TRIGGERS
+   default n
+   help
+ This option adds a LED trigger for each registered ATA port.
+ It is used to drive disk activity leds connected via GPIO.
+
+ If unsure, say N.
+
 config ATA_ACPI
bool "ATA ACPI Support"
depends on ACPI
diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 599e01b..65228f5 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -5105,6 +5105,30 @@ void swap_buf_le16(u16 *buf, unsigned int buf_words)
 }
 
 /**
+ * ata_led_act - Trigger port activity LED
+ * @ap: indicating port
+ *
+ * Blinks any LEDs registered to the trigger.
+ * Commonly used with leds-gpio on NAS systems with disk activity
+ * indicator LEDs.
+ *
+ * LOCKING:
+ * None.
+ */
+static inline void ata_led_act(struct ata_port *ap)
+{
+#ifdef CONFIG_ATA_LEDS
+#define LIBATA_BLINK_DELAY 20 /* ms */
+   unsigned long led_delay = LIBATA_BLINK_DELAY;
+
+   if (unlikely(!ap->ledtrig))
+   return;
+
+   led_trigger_blink_oneshot(ap->ledtrig, _delay, _delay, 0);
+#endif /* CONFIG_ATA_LEDS */
+}
+
+/**
  * ata_qc_new_init - Request an available ATA command, and initialize it
  * @dev: Device from whom we request an available command structure
  * @tag: tag
@@ -5249,6 +5273,10 @@ void ata_qc_complete(struct ata_queued_cmd *qc)
/* Trigger the LED (if available) */
ledtrig_disk_activity(!!(qc->tf.flags & ATA_TFLAG_WRITE));
 
+#ifdef CONFIG_ATA_LEDS
+   ata_led_act(ap);
+#endif
+
/* XXX: New EH and old EH use different mechanisms to
 * synchronize EH with regular execution path.
 *
@@ -6028,6 +6056,9 @@ struct ata_port *ata_port_alloc(struct ata_host *host)
ap->stats.unhandled_irq = 1;
ap->stats.idle_irq = 1;
 #endif
+#ifdef CONFIG_ATA_LEDS
+   ap->ledtrig = kzalloc(sizeof(struct led_trigger), GFP_KERNEL);
+#endif
ata_sff_port_init(ap);
 
return ap;
@@ -6063,6 +6094,12 @@ static void ata_host_release(struct kref *kref)
 
kfree(ap->pmp_link);
kfree(ap->slave_link);
+#ifdef CONFIG_ATA_LEDS
+   if (ap->ledtrig) {
+   led_trigger_unregister(ap->ledtrig);
+   kfree(ap->ledtrig);
+   };
+#endif
kfree(ap);
host->ports[i] = NULL;
}
@@ -6527,6 +6564,25 @@ int ata_host_register(struct ata_host *host, struct 
scsi_host_template *sht)
host->ports[i]->local_port_no = i + 1;
}
 
+#ifdef CONFIG_ATA_LEDS
+   /* register LED triggers for all ports */
+   for (i = 0; i < host->n_ports; i++) {
+   if (unlikely(!host->ports[i]->ledtrig))
+   continue;
+
+   snprintf(host->ports[i]->ledtrig_name,
+   sizeof(host->ports[i]->ledtrig_name), "ata%u",
+   host->ports[i]->print_id);
+
+   host->ports[i]->ledtrig->name = host->ports[i]->ledtrig_name;
+
+

[PATCH 0/2] Add support per ATA port ledtrigger on armada 38x

2018-09-18 Thread Aditya Prayoga
Hi everyone,

This series adds support LED trigger for each ATA port indicating disk activity
in Armada 38x. I pick up the work done by Daniel Golle which can be found at [1]

Helios4 which is based on Armada 388, has four LEDs designed to indicate disk
activity on each SATA ports. As the final switch (CONFIG_ATA_LEDS) to enable
the codepath by default is no, it should not affect the rest of Armada 38x 
based boards.

[1] https://patchwork.ozlabs.org/patch/420733/ 
---
Notes
  checkpatch.pl complains about "WARNING: please write a paragraph that
describes the config symbol fully" but I think the description is clear
enough to ignore the warning.

  Some performance number tested on Western Digital Red 2TB WD20EFRX
using fio randrw
* CONFIG_ATA_LEDS disabled and selected trigger is none
  read : iops=326
  write : iops=109
* CONFIG_ATA_LEDS disabled and selected trigger is disk-activity
  read : iops=325
  write : iops=108
* CONFIG_ATA_LEDS enabled and selected trigger is ata1
  read : iops=325
  write : iops=108

---
Aditya Prayoga (1):
  ARM: mvebu: Enable ARCH_WANT_LIBATA_LEDS in Armada 38x

Daniel Golle (1):
  libata: add ledtrig support

 arch/arm/mach-mvebu/Kconfig |  1 +
 drivers/ata/Kconfig | 16 +
 drivers/ata/libata-core.c   | 56 +
 include/linux/libata.h  |  7 ++
 4 files changed, 80 insertions(+)

-- 
2.7.4



[PATCH 0/2] Add support per ATA port ledtrigger on armada 38x

2018-09-18 Thread Aditya Prayoga
Hi everyone,

This series adds support LED trigger for each ATA port indicating disk activity
in Armada 38x. I pick up the work done by Daniel Golle which can be found at [1]

Helios4 which is based on Armada 388, has four LEDs designed to indicate disk
activity on each SATA ports. As the final switch (CONFIG_ATA_LEDS) to enable
the codepath by default is no, it should not affect the rest of Armada 38x 
based boards.

[1] https://patchwork.ozlabs.org/patch/420733/ 
---
Notes
  checkpatch.pl complains about "WARNING: please write a paragraph that
describes the config symbol fully" but I think the description is clear
enough to ignore the warning.

  Some performance number tested on Western Digital Red 2TB WD20EFRX
using fio randrw
* CONFIG_ATA_LEDS disabled and selected trigger is none
  read : iops=326
  write : iops=109
* CONFIG_ATA_LEDS disabled and selected trigger is disk-activity
  read : iops=325
  write : iops=108
* CONFIG_ATA_LEDS enabled and selected trigger is ata1
  read : iops=325
  write : iops=108

---
Aditya Prayoga (1):
  ARM: mvebu: Enable ARCH_WANT_LIBATA_LEDS in Armada 38x

Daniel Golle (1):
  libata: add ledtrig support

 arch/arm/mach-mvebu/Kconfig |  1 +
 drivers/ata/Kconfig | 16 +
 drivers/ata/libata-core.c   | 56 +
 include/linux/libata.h  |  7 ++
 4 files changed, 80 insertions(+)

-- 
2.7.4



Re: [PATCH v2 1/1] gpio: mvebu: Add support for multiple PWM lines per GPIO chip

2018-09-13 Thread Aditya Prayoga
On Wed, Sep 12, 2018 at 8:01 PM Andrew Lunn  wrote:
>
> >  static int mvebu_pwm_request(struct pwm_chip *chip, struct pwm_device *pwm)
> >  {
> >   struct mvebu_pwm *mvpwm = to_mvebu_pwm(chip);
> >   struct mvebu_gpio_chip *mvchip = mvpwm->mvchip;
> >   struct gpio_desc *desc;
> > + struct mvebu_pwm *counter;
> >   unsigned long flags;
> >   int ret = 0;
> >
> >   spin_lock_irqsave(>lock, flags);
> >
> > - if (mvpwm->gpiod) {
> > - ret = -EBUSY;
> > - } else {
> > - desc = gpiochip_request_own_desc(>chip,
> > -  pwm->hwpwm, "mvebu-pwm");
> > - if (IS_ERR(desc)) {
> > - ret = PTR_ERR(desc);
> > + counter = mvpwm;
> > + if (counter->gpiod) {
> > + counter = mvebu_pwm_get_avail_counter();
> > + if (!counter) {
> > + ret = -EBUSY;
>
> I don't understand this bit of code. Please could you explain what is
> going on.
Check whether bank's default counter is already used. If it's used then
try to find and check other counter. If it also being used that mean both,
counter A and counter B, already assigned to some PWM device so
return EBUSY.

>
> >   goto out;
> >   }
> >
> > - ret = gpiod_direction_output(desc, 0);
> > - if (ret) {
> > - gpiochip_free_own_desc(desc);
> > - goto out;
> > - }
> > + pwm->chip_data = counter;
> > + }
> >
> > - mvpwm->gpiod = desc;
> > + desc = gpiochip_request_own_desc(>chip,
> > +  pwm->hwpwm, "mvebu-pwm");
> > + if (IS_ERR(desc)) {
> > + ret = PTR_ERR(desc);
> > + goto out;
> >   }
> > +
> > + ret = gpiod_direction_output(desc, 0);
> > + if (ret) {
> > + gpiochip_free_own_desc(desc);
> > + goto out;
> > + }
> > +
> > + regmap_update_bits(mvchip->regs, GPIO_BLINK_CNT_SELECT_OFF +
> > +mvchip->offset, BIT(pwm->hwpwm),
> > +counter->id ? BIT(pwm->hwpwm) : 0);
> > + regmap_read(mvchip->regs, GPIO_BLINK_CNT_SELECT_OFF +
> > + mvchip->offset, >blink_select);
> > +
> > + counter->gpiod = desc;
> >  out:
> >   spin_unlock_irqrestore(>lock, flags);
> >   return ret;
> > @@ -632,6 +666,11 @@ static void mvebu_pwm_free(struct pwm_chip *chip, 
> > struct pwm_device *pwm)
> >   struct mvebu_pwm *mvpwm = to_mvebu_pwm(chip);
> >   unsigned long flags;
> >
> > + if (pwm->chip_data) {
> > + mvpwm = (struct mvebu_pwm *) pwm->chip_data;
> > + pwm->chip_data = NULL;
> > + }
> > +
> >   spin_lock_irqsave(>lock, flags);
> >   gpiochip_free_own_desc(mvpwm->gpiod);
> >   mvpwm->gpiod = NULL;
> > @@ -648,6 +687,9 @@ static void mvebu_pwm_get_state(struct pwm_chip *chip,
> >   unsigned long flags;
> >   u32 u;
> >
> > + if (pwm->chip_data)
> > + mvpwm = (struct mvebu_pwm *) pwm->chip_data;
> > +
>
> You should not need a cast here, if chip_data is a void *.
>
> What is pwm->chip_data is a NULL? Don't you then use an uninitialized
> mvpwm?

mvpwm is declared and initialized as:
struct mvebu_pwm *mvpwm = to_mvebu_pwm(chip);
so it's not an uninitialized variable.
pwm->chip_data is set when the pwm use counter other then default.
pwm->chip_data take precedence over to_mvebu_pwm(chip).

After looked at other PWM driver, i think i should use pwm_set_chip_data()
and pwm_get_chip_data() instead of directly access pwm->chip_data.

Now i think it would be better if i use
struct mvebu_pwm *mvpwm = pwm_get_chip_data(pwm);
and pwm_set_chip_data() would be called during mvebu_pwm_probe() to
set the data to bank's default counter.

Aditya

> Andrew


Re: [PATCH v2 1/1] gpio: mvebu: Add support for multiple PWM lines per GPIO chip

2018-09-13 Thread Aditya Prayoga
On Wed, Sep 12, 2018 at 8:01 PM Andrew Lunn  wrote:
>
> >  static int mvebu_pwm_request(struct pwm_chip *chip, struct pwm_device *pwm)
> >  {
> >   struct mvebu_pwm *mvpwm = to_mvebu_pwm(chip);
> >   struct mvebu_gpio_chip *mvchip = mvpwm->mvchip;
> >   struct gpio_desc *desc;
> > + struct mvebu_pwm *counter;
> >   unsigned long flags;
> >   int ret = 0;
> >
> >   spin_lock_irqsave(>lock, flags);
> >
> > - if (mvpwm->gpiod) {
> > - ret = -EBUSY;
> > - } else {
> > - desc = gpiochip_request_own_desc(>chip,
> > -  pwm->hwpwm, "mvebu-pwm");
> > - if (IS_ERR(desc)) {
> > - ret = PTR_ERR(desc);
> > + counter = mvpwm;
> > + if (counter->gpiod) {
> > + counter = mvebu_pwm_get_avail_counter();
> > + if (!counter) {
> > + ret = -EBUSY;
>
> I don't understand this bit of code. Please could you explain what is
> going on.
Check whether bank's default counter is already used. If it's used then
try to find and check other counter. If it also being used that mean both,
counter A and counter B, already assigned to some PWM device so
return EBUSY.

>
> >   goto out;
> >   }
> >
> > - ret = gpiod_direction_output(desc, 0);
> > - if (ret) {
> > - gpiochip_free_own_desc(desc);
> > - goto out;
> > - }
> > + pwm->chip_data = counter;
> > + }
> >
> > - mvpwm->gpiod = desc;
> > + desc = gpiochip_request_own_desc(>chip,
> > +  pwm->hwpwm, "mvebu-pwm");
> > + if (IS_ERR(desc)) {
> > + ret = PTR_ERR(desc);
> > + goto out;
> >   }
> > +
> > + ret = gpiod_direction_output(desc, 0);
> > + if (ret) {
> > + gpiochip_free_own_desc(desc);
> > + goto out;
> > + }
> > +
> > + regmap_update_bits(mvchip->regs, GPIO_BLINK_CNT_SELECT_OFF +
> > +mvchip->offset, BIT(pwm->hwpwm),
> > +counter->id ? BIT(pwm->hwpwm) : 0);
> > + regmap_read(mvchip->regs, GPIO_BLINK_CNT_SELECT_OFF +
> > + mvchip->offset, >blink_select);
> > +
> > + counter->gpiod = desc;
> >  out:
> >   spin_unlock_irqrestore(>lock, flags);
> >   return ret;
> > @@ -632,6 +666,11 @@ static void mvebu_pwm_free(struct pwm_chip *chip, 
> > struct pwm_device *pwm)
> >   struct mvebu_pwm *mvpwm = to_mvebu_pwm(chip);
> >   unsigned long flags;
> >
> > + if (pwm->chip_data) {
> > + mvpwm = (struct mvebu_pwm *) pwm->chip_data;
> > + pwm->chip_data = NULL;
> > + }
> > +
> >   spin_lock_irqsave(>lock, flags);
> >   gpiochip_free_own_desc(mvpwm->gpiod);
> >   mvpwm->gpiod = NULL;
> > @@ -648,6 +687,9 @@ static void mvebu_pwm_get_state(struct pwm_chip *chip,
> >   unsigned long flags;
> >   u32 u;
> >
> > + if (pwm->chip_data)
> > + mvpwm = (struct mvebu_pwm *) pwm->chip_data;
> > +
>
> You should not need a cast here, if chip_data is a void *.
>
> What is pwm->chip_data is a NULL? Don't you then use an uninitialized
> mvpwm?

mvpwm is declared and initialized as:
struct mvebu_pwm *mvpwm = to_mvebu_pwm(chip);
so it's not an uninitialized variable.
pwm->chip_data is set when the pwm use counter other then default.
pwm->chip_data take precedence over to_mvebu_pwm(chip).

After looked at other PWM driver, i think i should use pwm_set_chip_data()
and pwm_get_chip_data() instead of directly access pwm->chip_data.

Now i think it would be better if i use
struct mvebu_pwm *mvpwm = pwm_get_chip_data(pwm);
and pwm_set_chip_data() would be called during mvebu_pwm_probe() to
set the data to bank's default counter.

Aditya

> Andrew


[PATCH v2 0/1] gpio: mvebu: Add support for multiple PWM lines

2018-09-11 Thread Aditya Prayoga


Hi everyone,

Helios4, an Armada 388 based NAS SBC, provides 2 (4-pins) fan connectors.
The PWM pins on both connector are connected to GPIO on bank 1. Current
gpio-mvebu does not allow more than one PWM on the same bank.

Aditya

---

Changes v1->v2:
  * Merge/squash "[Patch 2/2] gpio: mvebu: Allow to use non-default PWM counter"
  * Allow only two PWMs as suggested by Andrew Lunn and Richard Genoud

---

Aditya Prayoga (1):
  gpio: mvebu: Add support for multiple PWM lines per GPIO chip

 drivers/gpio/gpio-mvebu.c | 73 ++-
 1 file changed, 60 insertions(+), 13 deletions(-)

-- 
2.7.4



[PATCH v2 1/1] gpio: mvebu: Add support for multiple PWM lines per GPIO chip

2018-09-11 Thread Aditya Prayoga
Allow more than 1 PWM request (eg. PWM fan) on the same GPIO chip. If
the other PWM counter is unused, allocate it to next PWM request.
The priority would be:
1. Default counter assigned to the bank
2. Unused counter that is assigned to other bank

Since there are only two PWM counters, only two PWMs supported.

Signed-off-by: Aditya Prayoga 
---
 drivers/gpio/gpio-mvebu.c | 73 ++-
 1 file changed, 60 insertions(+), 13 deletions(-)

diff --git a/drivers/gpio/gpio-mvebu.c b/drivers/gpio/gpio-mvebu.c
index 6e02148..2d46b87 100644
--- a/drivers/gpio/gpio-mvebu.c
+++ b/drivers/gpio/gpio-mvebu.c
@@ -92,9 +92,16 @@
 
 #define MVEBU_MAX_GPIO_PER_BANK32
 
+enum mvebu_pwm_counter {
+   MVEBU_PWM_CTRL_SET_A = 0,
+   MVEBU_PWM_CTRL_SET_B,
+   MVEBU_PWM_CTRL_MAX
+};
+
 struct mvebu_pwm {
void __iomem*membase;
unsigned longclk_rate;
+   enum mvebu_pwm_counter   id;
struct gpio_desc*gpiod;
struct pwm_chip  chip;
spinlock_t   lock;
@@ -128,6 +135,8 @@ struct mvebu_gpio_chip {
u32level_mask_regs[4];
 };
 
+static struct mvebu_pwm*mvebu_pwm_list[MVEBU_PWM_CTRL_MAX];
+
 /*
  * Functions returning addresses of individual registers for a given
  * GPIO controller.
@@ -594,34 +603,59 @@ static struct mvebu_pwm *to_mvebu_pwm(struct pwm_chip 
*chip)
return container_of(chip, struct mvebu_pwm, chip);
 }
 
+static struct mvebu_pwm *mvebu_pwm_get_avail_counter(void)
+{
+   enum mvebu_pwm_counter i;
+
+   for (i = MVEBU_PWM_CTRL_SET_A; i < MVEBU_PWM_CTRL_MAX; i++) {
+   if (!mvebu_pwm_list[i]->gpiod)
+   return mvebu_pwm_list[i];
+   }
+   return NULL;
+}
+
 static int mvebu_pwm_request(struct pwm_chip *chip, struct pwm_device *pwm)
 {
struct mvebu_pwm *mvpwm = to_mvebu_pwm(chip);
struct mvebu_gpio_chip *mvchip = mvpwm->mvchip;
struct gpio_desc *desc;
+   struct mvebu_pwm *counter;
unsigned long flags;
int ret = 0;
 
spin_lock_irqsave(>lock, flags);
 
-   if (mvpwm->gpiod) {
-   ret = -EBUSY;
-   } else {
-   desc = gpiochip_request_own_desc(>chip,
-pwm->hwpwm, "mvebu-pwm");
-   if (IS_ERR(desc)) {
-   ret = PTR_ERR(desc);
+   counter = mvpwm;
+   if (counter->gpiod) {
+   counter = mvebu_pwm_get_avail_counter();
+   if (!counter) {
+   ret = -EBUSY;
goto out;
}
 
-   ret = gpiod_direction_output(desc, 0);
-   if (ret) {
-   gpiochip_free_own_desc(desc);
-   goto out;
-   }
+   pwm->chip_data = counter;
+   }
 
-   mvpwm->gpiod = desc;
+   desc = gpiochip_request_own_desc(>chip,
+pwm->hwpwm, "mvebu-pwm");
+   if (IS_ERR(desc)) {
+   ret = PTR_ERR(desc);
+   goto out;
}
+
+   ret = gpiod_direction_output(desc, 0);
+   if (ret) {
+   gpiochip_free_own_desc(desc);
+   goto out;
+   }
+
+   regmap_update_bits(mvchip->regs, GPIO_BLINK_CNT_SELECT_OFF +
+  mvchip->offset, BIT(pwm->hwpwm),
+  counter->id ? BIT(pwm->hwpwm) : 0);
+   regmap_read(mvchip->regs, GPIO_BLINK_CNT_SELECT_OFF +
+   mvchip->offset, >blink_select);
+
+   counter->gpiod = desc;
 out:
spin_unlock_irqrestore(>lock, flags);
return ret;
@@ -632,6 +666,11 @@ static void mvebu_pwm_free(struct pwm_chip *chip, struct 
pwm_device *pwm)
struct mvebu_pwm *mvpwm = to_mvebu_pwm(chip);
unsigned long flags;
 
+   if (pwm->chip_data) {
+   mvpwm = (struct mvebu_pwm *) pwm->chip_data;
+   pwm->chip_data = NULL;
+   }
+
spin_lock_irqsave(>lock, flags);
gpiochip_free_own_desc(mvpwm->gpiod);
mvpwm->gpiod = NULL;
@@ -648,6 +687,9 @@ static void mvebu_pwm_get_state(struct pwm_chip *chip,
unsigned long flags;
u32 u;
 
+   if (pwm->chip_data)
+   mvpwm = (struct mvebu_pwm *) pwm->chip_data;
+
spin_lock_irqsave(>lock, flags);
 
val = (unsigned long long)
@@ -695,6 +737,9 @@ static int mvebu_pwm_apply(struct pwm_chip *chip, struct 
pwm_device *pwm,
unsigned long flags;
unsigned int on, off;
 
+   if (pwm->chip_data)
+   mvpwm = (struct mvebu_pwm *) pwm->chip_data;
+
val = (unsigned long long) mvpwm->clk_rate * state->duty_cycle;
do_div(val, NSEC_PER_SEC);
if (val > UINT_MAX)
@@ -804,6 

[PATCH v2 0/1] gpio: mvebu: Add support for multiple PWM lines

2018-09-11 Thread Aditya Prayoga


Hi everyone,

Helios4, an Armada 388 based NAS SBC, provides 2 (4-pins) fan connectors.
The PWM pins on both connector are connected to GPIO on bank 1. Current
gpio-mvebu does not allow more than one PWM on the same bank.

Aditya

---

Changes v1->v2:
  * Merge/squash "[Patch 2/2] gpio: mvebu: Allow to use non-default PWM counter"
  * Allow only two PWMs as suggested by Andrew Lunn and Richard Genoud

---

Aditya Prayoga (1):
  gpio: mvebu: Add support for multiple PWM lines per GPIO chip

 drivers/gpio/gpio-mvebu.c | 73 ++-
 1 file changed, 60 insertions(+), 13 deletions(-)

-- 
2.7.4



[PATCH v2 1/1] gpio: mvebu: Add support for multiple PWM lines per GPIO chip

2018-09-11 Thread Aditya Prayoga
Allow more than 1 PWM request (eg. PWM fan) on the same GPIO chip. If
the other PWM counter is unused, allocate it to next PWM request.
The priority would be:
1. Default counter assigned to the bank
2. Unused counter that is assigned to other bank

Since there are only two PWM counters, only two PWMs supported.

Signed-off-by: Aditya Prayoga 
---
 drivers/gpio/gpio-mvebu.c | 73 ++-
 1 file changed, 60 insertions(+), 13 deletions(-)

diff --git a/drivers/gpio/gpio-mvebu.c b/drivers/gpio/gpio-mvebu.c
index 6e02148..2d46b87 100644
--- a/drivers/gpio/gpio-mvebu.c
+++ b/drivers/gpio/gpio-mvebu.c
@@ -92,9 +92,16 @@
 
 #define MVEBU_MAX_GPIO_PER_BANK32
 
+enum mvebu_pwm_counter {
+   MVEBU_PWM_CTRL_SET_A = 0,
+   MVEBU_PWM_CTRL_SET_B,
+   MVEBU_PWM_CTRL_MAX
+};
+
 struct mvebu_pwm {
void __iomem*membase;
unsigned longclk_rate;
+   enum mvebu_pwm_counter   id;
struct gpio_desc*gpiod;
struct pwm_chip  chip;
spinlock_t   lock;
@@ -128,6 +135,8 @@ struct mvebu_gpio_chip {
u32level_mask_regs[4];
 };
 
+static struct mvebu_pwm*mvebu_pwm_list[MVEBU_PWM_CTRL_MAX];
+
 /*
  * Functions returning addresses of individual registers for a given
  * GPIO controller.
@@ -594,34 +603,59 @@ static struct mvebu_pwm *to_mvebu_pwm(struct pwm_chip 
*chip)
return container_of(chip, struct mvebu_pwm, chip);
 }
 
+static struct mvebu_pwm *mvebu_pwm_get_avail_counter(void)
+{
+   enum mvebu_pwm_counter i;
+
+   for (i = MVEBU_PWM_CTRL_SET_A; i < MVEBU_PWM_CTRL_MAX; i++) {
+   if (!mvebu_pwm_list[i]->gpiod)
+   return mvebu_pwm_list[i];
+   }
+   return NULL;
+}
+
 static int mvebu_pwm_request(struct pwm_chip *chip, struct pwm_device *pwm)
 {
struct mvebu_pwm *mvpwm = to_mvebu_pwm(chip);
struct mvebu_gpio_chip *mvchip = mvpwm->mvchip;
struct gpio_desc *desc;
+   struct mvebu_pwm *counter;
unsigned long flags;
int ret = 0;
 
spin_lock_irqsave(>lock, flags);
 
-   if (mvpwm->gpiod) {
-   ret = -EBUSY;
-   } else {
-   desc = gpiochip_request_own_desc(>chip,
-pwm->hwpwm, "mvebu-pwm");
-   if (IS_ERR(desc)) {
-   ret = PTR_ERR(desc);
+   counter = mvpwm;
+   if (counter->gpiod) {
+   counter = mvebu_pwm_get_avail_counter();
+   if (!counter) {
+   ret = -EBUSY;
goto out;
}
 
-   ret = gpiod_direction_output(desc, 0);
-   if (ret) {
-   gpiochip_free_own_desc(desc);
-   goto out;
-   }
+   pwm->chip_data = counter;
+   }
 
-   mvpwm->gpiod = desc;
+   desc = gpiochip_request_own_desc(>chip,
+pwm->hwpwm, "mvebu-pwm");
+   if (IS_ERR(desc)) {
+   ret = PTR_ERR(desc);
+   goto out;
}
+
+   ret = gpiod_direction_output(desc, 0);
+   if (ret) {
+   gpiochip_free_own_desc(desc);
+   goto out;
+   }
+
+   regmap_update_bits(mvchip->regs, GPIO_BLINK_CNT_SELECT_OFF +
+  mvchip->offset, BIT(pwm->hwpwm),
+  counter->id ? BIT(pwm->hwpwm) : 0);
+   regmap_read(mvchip->regs, GPIO_BLINK_CNT_SELECT_OFF +
+   mvchip->offset, >blink_select);
+
+   counter->gpiod = desc;
 out:
spin_unlock_irqrestore(>lock, flags);
return ret;
@@ -632,6 +666,11 @@ static void mvebu_pwm_free(struct pwm_chip *chip, struct 
pwm_device *pwm)
struct mvebu_pwm *mvpwm = to_mvebu_pwm(chip);
unsigned long flags;
 
+   if (pwm->chip_data) {
+   mvpwm = (struct mvebu_pwm *) pwm->chip_data;
+   pwm->chip_data = NULL;
+   }
+
spin_lock_irqsave(>lock, flags);
gpiochip_free_own_desc(mvpwm->gpiod);
mvpwm->gpiod = NULL;
@@ -648,6 +687,9 @@ static void mvebu_pwm_get_state(struct pwm_chip *chip,
unsigned long flags;
u32 u;
 
+   if (pwm->chip_data)
+   mvpwm = (struct mvebu_pwm *) pwm->chip_data;
+
spin_lock_irqsave(>lock, flags);
 
val = (unsigned long long)
@@ -695,6 +737,9 @@ static int mvebu_pwm_apply(struct pwm_chip *chip, struct 
pwm_device *pwm,
unsigned long flags;
unsigned int on, off;
 
+   if (pwm->chip_data)
+   mvpwm = (struct mvebu_pwm *) pwm->chip_data;
+
val = (unsigned long long) mvpwm->clk_rate * state->duty_cycle;
do_div(val, NSEC_PER_SEC);
if (val > UINT_MAX)
@@ -804,6 

Re: [PATCH RESEND 2/2] gpio: mvebu: Allow to use non-default PWM counter

2018-08-08 Thread Aditya Prayoga
On Mon, Aug 6, 2018 at 8:53 PM Andrew Lunn  wrote:
>
> On Mon, Aug 06, 2018 at 10:29:16AM +0800, Aditya Prayoga wrote:
> > On multiple PWM lines, if the other PWM counter is unused, allocate it
> > to next PWM request. The priority would be:
> > 1. Default counter assigned to the bank
> > 2. Unused counter that is assigned to other bank
> > 3. Fallback to default counter
> >
> > For example on second bank there are three PWM request, first one would
> > use default counter (counter B), second one would try to use counter A,
> > and the third one would use counter B.
>
> Hi Aditya
>
> There are only two PWM counters for all the GPIO lines. So you cannot
> support 3 PWM requests. You have to enforce a maximum of two PWMs.
>
> When i implemented this PWM code, i only needed one PWM. So it took
> the easy option. GPIO bank 0 uses counter A, GPIO bank1 uses counter
> B. For the hardware you have, this is not sufficient, so you need to
> generalise this. Any PWM can use any counter, whatever is available
> when the PWM is requested.

Hi Andrew

Understood. I will change it in next version.

> Rather than have a linked list of PWM, i think it would be better to
> have a static array of two mvebu_pwm structures. Index 0 uses counter
> A, index 1 uses counter B. You can then keep with the concept of
> pwm->pgiod != NULL means the counter is in use. The request() call can
> then find an unused PWM, set pwm->gpiod, and point mvchip->mvpwm to
> one of the two static instances.

That was my initial idea to use static array but then I thought maybe
I could generalise it for future device by using linked list.

Regards,

Aditya
>
> Andrew


Re: [PATCH RESEND 2/2] gpio: mvebu: Allow to use non-default PWM counter

2018-08-08 Thread Aditya Prayoga
On Mon, Aug 6, 2018 at 8:53 PM Andrew Lunn  wrote:
>
> On Mon, Aug 06, 2018 at 10:29:16AM +0800, Aditya Prayoga wrote:
> > On multiple PWM lines, if the other PWM counter is unused, allocate it
> > to next PWM request. The priority would be:
> > 1. Default counter assigned to the bank
> > 2. Unused counter that is assigned to other bank
> > 3. Fallback to default counter
> >
> > For example on second bank there are three PWM request, first one would
> > use default counter (counter B), second one would try to use counter A,
> > and the third one would use counter B.
>
> Hi Aditya
>
> There are only two PWM counters for all the GPIO lines. So you cannot
> support 3 PWM requests. You have to enforce a maximum of two PWMs.
>
> When i implemented this PWM code, i only needed one PWM. So it took
> the easy option. GPIO bank 0 uses counter A, GPIO bank1 uses counter
> B. For the hardware you have, this is not sufficient, so you need to
> generalise this. Any PWM can use any counter, whatever is available
> when the PWM is requested.

Hi Andrew

Understood. I will change it in next version.

> Rather than have a linked list of PWM, i think it would be better to
> have a static array of two mvebu_pwm structures. Index 0 uses counter
> A, index 1 uses counter B. You can then keep with the concept of
> pwm->pgiod != NULL means the counter is in use. The request() call can
> then find an unused PWM, set pwm->gpiod, and point mvchip->mvpwm to
> one of the two static instances.

That was my initial idea to use static array but then I thought maybe
I could generalise it for future device by using linked list.

Regards,

Aditya
>
> Andrew


Re: [PATCH RESEND 1/2] gpio: mvebu: Add support for multiple PWM lines per GPIO chip

2018-08-08 Thread Aditya Prayoga
On Mon, Aug 6, 2018 at 10:38 AM Andrew Lunn  wrote:
>
> On Mon, Aug 06, 2018 at 10:29:15AM +0800, Aditya Prayoga wrote:
>
> Hi Aditya
>
> > + item = kzalloc(sizeof(*item), GFP_KERNEL);
> > + if (!item)
> > + return -ENODEV;
>
> ENOMEM would be better, since it is a memory allocation which is
> failing.
>
> >
> > - ret = gpiod_direction_output(desc, 0);
> > - if (ret) {
> > - gpiochip_free_own_desc(desc);
> > - goto out;
> > - }
> > + spin_lock_irqsave(>lock, flags);
> > + desc = gpiochip_request_own_desc(>chip,
> > +  pwm->hwpwm, "mvebu-pwm");
> > + if (IS_ERR(desc)) {
> > + ret = PTR_ERR(desc);
> > + goto out;
> > + }
> >
> > - mvpwm->gpiod = desc;
> > + ret = gpiod_direction_output(desc, 0);
> > + if (ret) {
> > + gpiochip_free_own_desc(desc);
> > + goto out;
> >   }
> > + item->gpiod = desc;
> > + item->device = pwm;
> > + INIT_LIST_HEAD(>node);
> > + list_add_tail(>node, >pwms);
> >  out:
> >   spin_unlock_irqrestore(>lock, flags);
> >   return ret;
>
> You don't cleanup item on the error path.
>
> > @@ -630,12 +639,20 @@ static int mvebu_pwm_request(struct pwm_chip *chip, 
> > struct pwm_device *pwm)
> >  static void mvebu_pwm_free(struct pwm_chip *chip, struct pwm_device *pwm)
> >  {
> >   struct mvebu_pwm *mvpwm = to_mvebu_pwm(chip);
> > + struct mvebu_pwm_item *item, *tmp;
> >   unsigned long flags;
> >
> > - spin_lock_irqsave(>lock, flags);
> > - gpiochip_free_own_desc(mvpwm->gpiod);
> > - mvpwm->gpiod = NULL;
> > - spin_unlock_irqrestore(>lock, flags);
> > + list_for_each_entry_safe(item, tmp, >pwms, node) {
> > + if (item->device == pwm) {
> > + spin_lock_irqsave(>lock, flags);
> > + gpiochip_free_own_desc(item->gpiod);
> > + item->gpiod = NULL;
> > + item->device = NULL;
>
> Since you are about to free item, these two lines are pointless.
>
> > + list_del(>node);
> > + spin_unlock_irqrestore(>lock, flags);
> > + kfree(item);
> > + }
> > + }
> >  }
>
>Andrew

Hi Andrew,

Thanks, I will fix it on next version.

Regards,

Aditya


Re: [PATCH RESEND 1/2] gpio: mvebu: Add support for multiple PWM lines per GPIO chip

2018-08-08 Thread Aditya Prayoga
On Mon, Aug 6, 2018 at 10:38 AM Andrew Lunn  wrote:
>
> On Mon, Aug 06, 2018 at 10:29:15AM +0800, Aditya Prayoga wrote:
>
> Hi Aditya
>
> > + item = kzalloc(sizeof(*item), GFP_KERNEL);
> > + if (!item)
> > + return -ENODEV;
>
> ENOMEM would be better, since it is a memory allocation which is
> failing.
>
> >
> > - ret = gpiod_direction_output(desc, 0);
> > - if (ret) {
> > - gpiochip_free_own_desc(desc);
> > - goto out;
> > - }
> > + spin_lock_irqsave(>lock, flags);
> > + desc = gpiochip_request_own_desc(>chip,
> > +  pwm->hwpwm, "mvebu-pwm");
> > + if (IS_ERR(desc)) {
> > + ret = PTR_ERR(desc);
> > + goto out;
> > + }
> >
> > - mvpwm->gpiod = desc;
> > + ret = gpiod_direction_output(desc, 0);
> > + if (ret) {
> > + gpiochip_free_own_desc(desc);
> > + goto out;
> >   }
> > + item->gpiod = desc;
> > + item->device = pwm;
> > + INIT_LIST_HEAD(>node);
> > + list_add_tail(>node, >pwms);
> >  out:
> >   spin_unlock_irqrestore(>lock, flags);
> >   return ret;
>
> You don't cleanup item on the error path.
>
> > @@ -630,12 +639,20 @@ static int mvebu_pwm_request(struct pwm_chip *chip, 
> > struct pwm_device *pwm)
> >  static void mvebu_pwm_free(struct pwm_chip *chip, struct pwm_device *pwm)
> >  {
> >   struct mvebu_pwm *mvpwm = to_mvebu_pwm(chip);
> > + struct mvebu_pwm_item *item, *tmp;
> >   unsigned long flags;
> >
> > - spin_lock_irqsave(>lock, flags);
> > - gpiochip_free_own_desc(mvpwm->gpiod);
> > - mvpwm->gpiod = NULL;
> > - spin_unlock_irqrestore(>lock, flags);
> > + list_for_each_entry_safe(item, tmp, >pwms, node) {
> > + if (item->device == pwm) {
> > + spin_lock_irqsave(>lock, flags);
> > + gpiochip_free_own_desc(item->gpiod);
> > + item->gpiod = NULL;
> > + item->device = NULL;
>
> Since you are about to free item, these two lines are pointless.
>
> > + list_del(>node);
> > + spin_unlock_irqrestore(>lock, flags);
> > + kfree(item);
> > + }
> > + }
> >  }
>
>Andrew

Hi Andrew,

Thanks, I will fix it on next version.

Regards,

Aditya


[PATCH RESEND 1/2] gpio: mvebu: Add support for multiple PWM lines per GPIO chip

2018-08-05 Thread Aditya Prayoga
Allow more than 1 PWM request (eg. PWM fan) on the same GPIO chip.

based on initial work on LK4.4 by Alban Browaeys.
URL: https://github.com/helios-4/linux-marvell/commit/743ae97
[Aditya Prayoga: forward port, cleanup]
Signed-off-by: Aditya Prayoga 
---
 drivers/gpio/gpio-mvebu.c | 63 ++-
 1 file changed, 41 insertions(+), 22 deletions(-)

diff --git a/drivers/gpio/gpio-mvebu.c b/drivers/gpio/gpio-mvebu.c
index 6e02148..0617e66 100644
--- a/drivers/gpio/gpio-mvebu.c
+++ b/drivers/gpio/gpio-mvebu.c
@@ -92,10 +92,17 @@
 
 #define MVEBU_MAX_GPIO_PER_BANK32
 
+struct mvebu_pwm_item {
+   struct gpio_desc*gpiod;
+   struct pwm_device   *device;
+   struct list_head node;
+};
+
 struct mvebu_pwm {
void __iomem*membase;
unsigned longclk_rate;
-   struct gpio_desc*gpiod;
+   int  id;
+   struct list_head pwms;
struct pwm_chip  chip;
spinlock_t   lock;
struct mvebu_gpio_chip  *mvchip;
@@ -599,29 +606,31 @@ static int mvebu_pwm_request(struct pwm_chip *chip, 
struct pwm_device *pwm)
struct mvebu_pwm *mvpwm = to_mvebu_pwm(chip);
struct mvebu_gpio_chip *mvchip = mvpwm->mvchip;
struct gpio_desc *desc;
+   struct mvebu_pwm_item *item;
unsigned long flags;
int ret = 0;
 
-   spin_lock_irqsave(>lock, flags);
-
-   if (mvpwm->gpiod) {
-   ret = -EBUSY;
-   } else {
-   desc = gpiochip_request_own_desc(>chip,
-pwm->hwpwm, "mvebu-pwm");
-   if (IS_ERR(desc)) {
-   ret = PTR_ERR(desc);
-   goto out;
-   }
+   item = kzalloc(sizeof(*item), GFP_KERNEL);
+   if (!item)
+   return -ENODEV;
 
-   ret = gpiod_direction_output(desc, 0);
-   if (ret) {
-   gpiochip_free_own_desc(desc);
-   goto out;
-   }
+   spin_lock_irqsave(>lock, flags);
+   desc = gpiochip_request_own_desc(>chip,
+pwm->hwpwm, "mvebu-pwm");
+   if (IS_ERR(desc)) {
+   ret = PTR_ERR(desc);
+   goto out;
+   }
 
-   mvpwm->gpiod = desc;
+   ret = gpiod_direction_output(desc, 0);
+   if (ret) {
+   gpiochip_free_own_desc(desc);
+   goto out;
}
+   item->gpiod = desc;
+   item->device = pwm;
+   INIT_LIST_HEAD(>node);
+   list_add_tail(>node, >pwms);
 out:
spin_unlock_irqrestore(>lock, flags);
return ret;
@@ -630,12 +639,20 @@ static int mvebu_pwm_request(struct pwm_chip *chip, 
struct pwm_device *pwm)
 static void mvebu_pwm_free(struct pwm_chip *chip, struct pwm_device *pwm)
 {
struct mvebu_pwm *mvpwm = to_mvebu_pwm(chip);
+   struct mvebu_pwm_item *item, *tmp;
unsigned long flags;
 
-   spin_lock_irqsave(>lock, flags);
-   gpiochip_free_own_desc(mvpwm->gpiod);
-   mvpwm->gpiod = NULL;
-   spin_unlock_irqrestore(>lock, flags);
+   list_for_each_entry_safe(item, tmp, >pwms, node) {
+   if (item->device == pwm) {
+   spin_lock_irqsave(>lock, flags);
+   gpiochip_free_own_desc(item->gpiod);
+   item->gpiod = NULL;
+   item->device = NULL;
+   list_del(>node);
+   spin_unlock_irqrestore(>lock, flags);
+   kfree(item);
+   }
+   }
 }
 
 static void mvebu_pwm_get_state(struct pwm_chip *chip,
@@ -804,6 +821,8 @@ static int mvebu_pwm_probe(struct platform_device *pdev,
return -ENOMEM;
mvchip->mvpwm = mvpwm;
mvpwm->mvchip = mvchip;
+   mvpwm->id = id;
+   INIT_LIST_HEAD(>pwms);
 
mvpwm->membase = devm_ioremap_resource(dev, res);
if (IS_ERR(mvpwm->membase))
-- 
2.7.4



[PATCH RESEND 0/2] gpio: mvebu: Add support for multiple PWM lines

2018-08-05 Thread Aditya Prayoga
Hi everyone,

Helios4, an Armada 388 based NAS SBC, provides 2 (4-pins) fan connectors.
The PWM pins on both connector are connected to GPIO on bank 1. Current gpio-
mvebu does not allow more than one PWM on the same bank.

Resend the patch to add more reviewer.

Aditya

---

Aditya Prayoga (2):
  gpio: mvebu: Add support for multiple PWM lines per GPIO chip
  gpio: mvebu: Allow to use non-default PWM counter

 drivers/gpio/gpio-mvebu.c | 111 ++
 1 file changed, 92 insertions(+), 19 deletions(-)

-- 
2.7.4



[PATCH RESEND 1/2] gpio: mvebu: Add support for multiple PWM lines per GPIO chip

2018-08-05 Thread Aditya Prayoga
Allow more than 1 PWM request (eg. PWM fan) on the same GPIO chip.

based on initial work on LK4.4 by Alban Browaeys.
URL: https://github.com/helios-4/linux-marvell/commit/743ae97
[Aditya Prayoga: forward port, cleanup]
Signed-off-by: Aditya Prayoga 
---
 drivers/gpio/gpio-mvebu.c | 63 ++-
 1 file changed, 41 insertions(+), 22 deletions(-)

diff --git a/drivers/gpio/gpio-mvebu.c b/drivers/gpio/gpio-mvebu.c
index 6e02148..0617e66 100644
--- a/drivers/gpio/gpio-mvebu.c
+++ b/drivers/gpio/gpio-mvebu.c
@@ -92,10 +92,17 @@
 
 #define MVEBU_MAX_GPIO_PER_BANK32
 
+struct mvebu_pwm_item {
+   struct gpio_desc*gpiod;
+   struct pwm_device   *device;
+   struct list_head node;
+};
+
 struct mvebu_pwm {
void __iomem*membase;
unsigned longclk_rate;
-   struct gpio_desc*gpiod;
+   int  id;
+   struct list_head pwms;
struct pwm_chip  chip;
spinlock_t   lock;
struct mvebu_gpio_chip  *mvchip;
@@ -599,29 +606,31 @@ static int mvebu_pwm_request(struct pwm_chip *chip, 
struct pwm_device *pwm)
struct mvebu_pwm *mvpwm = to_mvebu_pwm(chip);
struct mvebu_gpio_chip *mvchip = mvpwm->mvchip;
struct gpio_desc *desc;
+   struct mvebu_pwm_item *item;
unsigned long flags;
int ret = 0;
 
-   spin_lock_irqsave(>lock, flags);
-
-   if (mvpwm->gpiod) {
-   ret = -EBUSY;
-   } else {
-   desc = gpiochip_request_own_desc(>chip,
-pwm->hwpwm, "mvebu-pwm");
-   if (IS_ERR(desc)) {
-   ret = PTR_ERR(desc);
-   goto out;
-   }
+   item = kzalloc(sizeof(*item), GFP_KERNEL);
+   if (!item)
+   return -ENODEV;
 
-   ret = gpiod_direction_output(desc, 0);
-   if (ret) {
-   gpiochip_free_own_desc(desc);
-   goto out;
-   }
+   spin_lock_irqsave(>lock, flags);
+   desc = gpiochip_request_own_desc(>chip,
+pwm->hwpwm, "mvebu-pwm");
+   if (IS_ERR(desc)) {
+   ret = PTR_ERR(desc);
+   goto out;
+   }
 
-   mvpwm->gpiod = desc;
+   ret = gpiod_direction_output(desc, 0);
+   if (ret) {
+   gpiochip_free_own_desc(desc);
+   goto out;
}
+   item->gpiod = desc;
+   item->device = pwm;
+   INIT_LIST_HEAD(>node);
+   list_add_tail(>node, >pwms);
 out:
spin_unlock_irqrestore(>lock, flags);
return ret;
@@ -630,12 +639,20 @@ static int mvebu_pwm_request(struct pwm_chip *chip, 
struct pwm_device *pwm)
 static void mvebu_pwm_free(struct pwm_chip *chip, struct pwm_device *pwm)
 {
struct mvebu_pwm *mvpwm = to_mvebu_pwm(chip);
+   struct mvebu_pwm_item *item, *tmp;
unsigned long flags;
 
-   spin_lock_irqsave(>lock, flags);
-   gpiochip_free_own_desc(mvpwm->gpiod);
-   mvpwm->gpiod = NULL;
-   spin_unlock_irqrestore(>lock, flags);
+   list_for_each_entry_safe(item, tmp, >pwms, node) {
+   if (item->device == pwm) {
+   spin_lock_irqsave(>lock, flags);
+   gpiochip_free_own_desc(item->gpiod);
+   item->gpiod = NULL;
+   item->device = NULL;
+   list_del(>node);
+   spin_unlock_irqrestore(>lock, flags);
+   kfree(item);
+   }
+   }
 }
 
 static void mvebu_pwm_get_state(struct pwm_chip *chip,
@@ -804,6 +821,8 @@ static int mvebu_pwm_probe(struct platform_device *pdev,
return -ENOMEM;
mvchip->mvpwm = mvpwm;
mvpwm->mvchip = mvchip;
+   mvpwm->id = id;
+   INIT_LIST_HEAD(>pwms);
 
mvpwm->membase = devm_ioremap_resource(dev, res);
if (IS_ERR(mvpwm->membase))
-- 
2.7.4



[PATCH RESEND 0/2] gpio: mvebu: Add support for multiple PWM lines

2018-08-05 Thread Aditya Prayoga
Hi everyone,

Helios4, an Armada 388 based NAS SBC, provides 2 (4-pins) fan connectors.
The PWM pins on both connector are connected to GPIO on bank 1. Current gpio-
mvebu does not allow more than one PWM on the same bank.

Resend the patch to add more reviewer.

Aditya

---

Aditya Prayoga (2):
  gpio: mvebu: Add support for multiple PWM lines per GPIO chip
  gpio: mvebu: Allow to use non-default PWM counter

 drivers/gpio/gpio-mvebu.c | 111 ++
 1 file changed, 92 insertions(+), 19 deletions(-)

-- 
2.7.4



[PATCH RESEND 2/2] gpio: mvebu: Allow to use non-default PWM counter

2018-08-05 Thread Aditya Prayoga
On multiple PWM lines, if the other PWM counter is unused, allocate it
to next PWM request. The priority would be:
1. Default counter assigned to the bank
2. Unused counter that is assigned to other bank
3. Fallback to default counter

For example on second bank there are three PWM request, first one would
use default counter (counter B), second one would try to use counter A,
and the third one would use counter B.

Signed-off-by: Aditya Prayoga 
---
 drivers/gpio/gpio-mvebu.c | 58 +--
 1 file changed, 56 insertions(+), 2 deletions(-)

diff --git a/drivers/gpio/gpio-mvebu.c b/drivers/gpio/gpio-mvebu.c
index 0617e66..5a39478 100644
--- a/drivers/gpio/gpio-mvebu.c
+++ b/drivers/gpio/gpio-mvebu.c
@@ -92,6 +92,11 @@
 
 #define MVEBU_MAX_GPIO_PER_BANK32
 
+enum mvebu_pwm_counter {
+   MVEBU_PWM_COUNTER_A = 0,
+   MVEBU_PWM_COUNTER_B,
+};
+
 struct mvebu_pwm_item {
struct gpio_desc*gpiod;
struct pwm_device   *device;
@@ -101,7 +106,8 @@ struct mvebu_pwm_item {
 struct mvebu_pwm {
void __iomem*membase;
unsigned longclk_rate;
-   int  id;
+   enum mvebu_pwm_counter   id;
+   struct list_head node;
struct list_head pwms;
struct pwm_chip  chip;
spinlock_t   lock;
@@ -113,6 +119,8 @@ struct mvebu_pwm {
u32  blink_off_duration;
 };
 
+static LIST_HEAD(mvebu_pwm_list);
+
 struct mvebu_gpio_chip {
struct gpio_chip   chip;
struct regmap *regs;
@@ -601,12 +609,24 @@ static struct mvebu_pwm *to_mvebu_pwm(struct pwm_chip 
*chip)
return container_of(chip, struct mvebu_pwm, chip);
 }
 
+static struct mvebu_pwm *mvebu_pwm_get_avail_counter(void)
+{
+   struct mvebu_pwm *counter;
+
+   list_for_each_entry(counter, _pwm_list, node) {
+   if (list_empty(>pwms))
+   return counter;
+   }
+   return NULL;
+}
+
 static int mvebu_pwm_request(struct pwm_chip *chip, struct pwm_device *pwm)
 {
struct mvebu_pwm *mvpwm = to_mvebu_pwm(chip);
struct mvebu_gpio_chip *mvchip = mvpwm->mvchip;
struct gpio_desc *desc;
struct mvebu_pwm_item *item;
+   struct mvebu_pwm *counter;
unsigned long flags;
int ret = 0;
 
@@ -615,6 +635,14 @@ static int mvebu_pwm_request(struct pwm_chip *chip, struct 
pwm_device *pwm)
return -ENODEV;
 
spin_lock_irqsave(>lock, flags);
+   regmap_read(mvchip->regs, GPIO_BLINK_EN_OFF + mvchip->offset,
+   >blink_en_reg);
+
+   if (mvchip->blink_en_reg & BIT(pwm->hwpwm)) {
+   ret = -EBUSY;
+   goto out;
+   }
+
desc = gpiochip_request_own_desc(>chip,
 pwm->hwpwm, "mvebu-pwm");
if (IS_ERR(desc)) {
@@ -627,10 +655,25 @@ static int mvebu_pwm_request(struct pwm_chip *chip, 
struct pwm_device *pwm)
gpiochip_free_own_desc(desc);
goto out;
}
+
+   counter = mvpwm;
+   if (!list_empty(>pwms)) {
+   counter = mvebu_pwm_get_avail_counter();
+   if (counter)
+   pwm->chip_data = counter;
+   else
+   counter = mvpwm;
+   }
+
+   regmap_update_bits(mvchip->regs, GPIO_BLINK_CNT_SELECT_OFF +
+   mvchip->offset, BIT(pwm->hwpwm),
+   counter->id ? BIT(pwm->hwpwm) : 0);
+   regmap_read(mvchip->regs, GPIO_BLINK_CNT_SELECT_OFF +
+   mvchip->offset, >blink_select);
item->gpiod = desc;
item->device = pwm;
INIT_LIST_HEAD(>node);
-   list_add_tail(>node, >pwms);
+   list_add_tail(>node, >pwms);
 out:
spin_unlock_irqrestore(>lock, flags);
return ret;
@@ -642,6 +685,9 @@ static void mvebu_pwm_free(struct pwm_chip *chip, struct 
pwm_device *pwm)
struct mvebu_pwm_item *item, *tmp;
unsigned long flags;
 
+   if (pwm->chip_data)
+   mvpwm = (struct mvebu_pwm *) pwm->chip_data;
+
list_for_each_entry_safe(item, tmp, >pwms, node) {
if (item->device == pwm) {
spin_lock_irqsave(>lock, flags);
@@ -665,6 +711,9 @@ static void mvebu_pwm_get_state(struct pwm_chip *chip,
unsigned long flags;
u32 u;
 
+   if (pwm->chip_data)
+   mvpwm = (struct mvebu_pwm *) pwm->chip_data;
+
spin_lock_irqsave(>lock, flags);
 
val = (unsigned long long)
@@ -712,6 +761,9 @@ static int mvebu_pwm_apply(struct pwm_chip *chip, struct 
pwm_device *pwm,
unsigned long flags;
unsigned int on, off;
 
+   if (pwm->chip_data)
+   mvpwm = (struct mvebu_pwm 

[PATCH RESEND 2/2] gpio: mvebu: Allow to use non-default PWM counter

2018-08-05 Thread Aditya Prayoga
On multiple PWM lines, if the other PWM counter is unused, allocate it
to next PWM request. The priority would be:
1. Default counter assigned to the bank
2. Unused counter that is assigned to other bank
3. Fallback to default counter

For example on second bank there are three PWM request, first one would
use default counter (counter B), second one would try to use counter A,
and the third one would use counter B.

Signed-off-by: Aditya Prayoga 
---
 drivers/gpio/gpio-mvebu.c | 58 +--
 1 file changed, 56 insertions(+), 2 deletions(-)

diff --git a/drivers/gpio/gpio-mvebu.c b/drivers/gpio/gpio-mvebu.c
index 0617e66..5a39478 100644
--- a/drivers/gpio/gpio-mvebu.c
+++ b/drivers/gpio/gpio-mvebu.c
@@ -92,6 +92,11 @@
 
 #define MVEBU_MAX_GPIO_PER_BANK32
 
+enum mvebu_pwm_counter {
+   MVEBU_PWM_COUNTER_A = 0,
+   MVEBU_PWM_COUNTER_B,
+};
+
 struct mvebu_pwm_item {
struct gpio_desc*gpiod;
struct pwm_device   *device;
@@ -101,7 +106,8 @@ struct mvebu_pwm_item {
 struct mvebu_pwm {
void __iomem*membase;
unsigned longclk_rate;
-   int  id;
+   enum mvebu_pwm_counter   id;
+   struct list_head node;
struct list_head pwms;
struct pwm_chip  chip;
spinlock_t   lock;
@@ -113,6 +119,8 @@ struct mvebu_pwm {
u32  blink_off_duration;
 };
 
+static LIST_HEAD(mvebu_pwm_list);
+
 struct mvebu_gpio_chip {
struct gpio_chip   chip;
struct regmap *regs;
@@ -601,12 +609,24 @@ static struct mvebu_pwm *to_mvebu_pwm(struct pwm_chip 
*chip)
return container_of(chip, struct mvebu_pwm, chip);
 }
 
+static struct mvebu_pwm *mvebu_pwm_get_avail_counter(void)
+{
+   struct mvebu_pwm *counter;
+
+   list_for_each_entry(counter, _pwm_list, node) {
+   if (list_empty(>pwms))
+   return counter;
+   }
+   return NULL;
+}
+
 static int mvebu_pwm_request(struct pwm_chip *chip, struct pwm_device *pwm)
 {
struct mvebu_pwm *mvpwm = to_mvebu_pwm(chip);
struct mvebu_gpio_chip *mvchip = mvpwm->mvchip;
struct gpio_desc *desc;
struct mvebu_pwm_item *item;
+   struct mvebu_pwm *counter;
unsigned long flags;
int ret = 0;
 
@@ -615,6 +635,14 @@ static int mvebu_pwm_request(struct pwm_chip *chip, struct 
pwm_device *pwm)
return -ENODEV;
 
spin_lock_irqsave(>lock, flags);
+   regmap_read(mvchip->regs, GPIO_BLINK_EN_OFF + mvchip->offset,
+   >blink_en_reg);
+
+   if (mvchip->blink_en_reg & BIT(pwm->hwpwm)) {
+   ret = -EBUSY;
+   goto out;
+   }
+
desc = gpiochip_request_own_desc(>chip,
 pwm->hwpwm, "mvebu-pwm");
if (IS_ERR(desc)) {
@@ -627,10 +655,25 @@ static int mvebu_pwm_request(struct pwm_chip *chip, 
struct pwm_device *pwm)
gpiochip_free_own_desc(desc);
goto out;
}
+
+   counter = mvpwm;
+   if (!list_empty(>pwms)) {
+   counter = mvebu_pwm_get_avail_counter();
+   if (counter)
+   pwm->chip_data = counter;
+   else
+   counter = mvpwm;
+   }
+
+   regmap_update_bits(mvchip->regs, GPIO_BLINK_CNT_SELECT_OFF +
+   mvchip->offset, BIT(pwm->hwpwm),
+   counter->id ? BIT(pwm->hwpwm) : 0);
+   regmap_read(mvchip->regs, GPIO_BLINK_CNT_SELECT_OFF +
+   mvchip->offset, >blink_select);
item->gpiod = desc;
item->device = pwm;
INIT_LIST_HEAD(>node);
-   list_add_tail(>node, >pwms);
+   list_add_tail(>node, >pwms);
 out:
spin_unlock_irqrestore(>lock, flags);
return ret;
@@ -642,6 +685,9 @@ static void mvebu_pwm_free(struct pwm_chip *chip, struct 
pwm_device *pwm)
struct mvebu_pwm_item *item, *tmp;
unsigned long flags;
 
+   if (pwm->chip_data)
+   mvpwm = (struct mvebu_pwm *) pwm->chip_data;
+
list_for_each_entry_safe(item, tmp, >pwms, node) {
if (item->device == pwm) {
spin_lock_irqsave(>lock, flags);
@@ -665,6 +711,9 @@ static void mvebu_pwm_get_state(struct pwm_chip *chip,
unsigned long flags;
u32 u;
 
+   if (pwm->chip_data)
+   mvpwm = (struct mvebu_pwm *) pwm->chip_data;
+
spin_lock_irqsave(>lock, flags);
 
val = (unsigned long long)
@@ -712,6 +761,9 @@ static int mvebu_pwm_apply(struct pwm_chip *chip, struct 
pwm_device *pwm,
unsigned long flags;
unsigned int on, off;
 
+   if (pwm->chip_data)
+   mvpwm = (struct mvebu_pwm 

[PATCH 2/2] gpio: mvebu: Allow to use non-default PWM counter

2018-07-20 Thread Aditya Prayoga
On multiple PWM lines, if the other PWM counter is unused, allocate it
to next PWM request. The priority would be:
1. Default counter assigned to the bank
2. Unused counter that is assigned to other bank
3. Fallback to default counter

For example on second bank there are three PWM request, first one would
use default counter (counter B), second one would try to use counter A,
and the third one would use counter B.

Signed-off-by: Aditya Prayoga 
---
 drivers/gpio/gpio-mvebu.c | 58 +--
 1 file changed, 56 insertions(+), 2 deletions(-)

diff --git a/drivers/gpio/gpio-mvebu.c b/drivers/gpio/gpio-mvebu.c
index 0617e66..5a39478 100644
--- a/drivers/gpio/gpio-mvebu.c
+++ b/drivers/gpio/gpio-mvebu.c
@@ -92,6 +92,11 @@
 
 #define MVEBU_MAX_GPIO_PER_BANK32
 
+enum mvebu_pwm_counter {
+   MVEBU_PWM_COUNTER_A = 0,
+   MVEBU_PWM_COUNTER_B,
+};
+
 struct mvebu_pwm_item {
struct gpio_desc*gpiod;
struct pwm_device   *device;
@@ -101,7 +106,8 @@ struct mvebu_pwm_item {
 struct mvebu_pwm {
void __iomem*membase;
unsigned longclk_rate;
-   int  id;
+   enum mvebu_pwm_counter   id;
+   struct list_head node;
struct list_head pwms;
struct pwm_chip  chip;
spinlock_t   lock;
@@ -113,6 +119,8 @@ struct mvebu_pwm {
u32  blink_off_duration;
 };
 
+static LIST_HEAD(mvebu_pwm_list);
+
 struct mvebu_gpio_chip {
struct gpio_chip   chip;
struct regmap *regs;
@@ -601,12 +609,24 @@ static struct mvebu_pwm *to_mvebu_pwm(struct pwm_chip 
*chip)
return container_of(chip, struct mvebu_pwm, chip);
 }
 
+static struct mvebu_pwm *mvebu_pwm_get_avail_counter(void)
+{
+   struct mvebu_pwm *counter;
+
+   list_for_each_entry(counter, _pwm_list, node) {
+   if (list_empty(>pwms))
+   return counter;
+   }
+   return NULL;
+}
+
 static int mvebu_pwm_request(struct pwm_chip *chip, struct pwm_device *pwm)
 {
struct mvebu_pwm *mvpwm = to_mvebu_pwm(chip);
struct mvebu_gpio_chip *mvchip = mvpwm->mvchip;
struct gpio_desc *desc;
struct mvebu_pwm_item *item;
+   struct mvebu_pwm *counter;
unsigned long flags;
int ret = 0;
 
@@ -615,6 +635,14 @@ static int mvebu_pwm_request(struct pwm_chip *chip, struct 
pwm_device *pwm)
return -ENODEV;
 
spin_lock_irqsave(>lock, flags);
+   regmap_read(mvchip->regs, GPIO_BLINK_EN_OFF + mvchip->offset,
+   >blink_en_reg);
+
+   if (mvchip->blink_en_reg & BIT(pwm->hwpwm)) {
+   ret = -EBUSY;
+   goto out;
+   }
+
desc = gpiochip_request_own_desc(>chip,
 pwm->hwpwm, "mvebu-pwm");
if (IS_ERR(desc)) {
@@ -627,10 +655,25 @@ static int mvebu_pwm_request(struct pwm_chip *chip, 
struct pwm_device *pwm)
gpiochip_free_own_desc(desc);
goto out;
}
+
+   counter = mvpwm;
+   if (!list_empty(>pwms)) {
+   counter = mvebu_pwm_get_avail_counter();
+   if (counter)
+   pwm->chip_data = counter;
+   else
+   counter = mvpwm;
+   }
+
+   regmap_update_bits(mvchip->regs, GPIO_BLINK_CNT_SELECT_OFF +
+   mvchip->offset, BIT(pwm->hwpwm),
+   counter->id ? BIT(pwm->hwpwm) : 0);
+   regmap_read(mvchip->regs, GPIO_BLINK_CNT_SELECT_OFF +
+   mvchip->offset, >blink_select);
item->gpiod = desc;
item->device = pwm;
INIT_LIST_HEAD(>node);
-   list_add_tail(>node, >pwms);
+   list_add_tail(>node, >pwms);
 out:
spin_unlock_irqrestore(>lock, flags);
return ret;
@@ -642,6 +685,9 @@ static void mvebu_pwm_free(struct pwm_chip *chip, struct 
pwm_device *pwm)
struct mvebu_pwm_item *item, *tmp;
unsigned long flags;
 
+   if (pwm->chip_data)
+   mvpwm = (struct mvebu_pwm *) pwm->chip_data;
+
list_for_each_entry_safe(item, tmp, >pwms, node) {
if (item->device == pwm) {
spin_lock_irqsave(>lock, flags);
@@ -665,6 +711,9 @@ static void mvebu_pwm_get_state(struct pwm_chip *chip,
unsigned long flags;
u32 u;
 
+   if (pwm->chip_data)
+   mvpwm = (struct mvebu_pwm *) pwm->chip_data;
+
spin_lock_irqsave(>lock, flags);
 
val = (unsigned long long)
@@ -712,6 +761,9 @@ static int mvebu_pwm_apply(struct pwm_chip *chip, struct 
pwm_device *pwm,
unsigned long flags;
unsigned int on, off;
 
+   if (pwm->chip_data)
+   mvpwm = (struct mvebu_pwm 

[PATCH 1/2] gpio: mvebu: Add support for multiple PWM lines per GPIO chip

2018-07-20 Thread Aditya Prayoga
Allow more than 1 PWM request (eg. PWM fan) on the same GPIO chip.

based on initial work on LK4.4 by Alban Browaeys.
URL: https://github.com/helios-4/linux-marvell/commit/743ae97
[Aditya Prayoga: forward port, cleanup]
Signed-off-by: Aditya Prayoga 
---
 drivers/gpio/gpio-mvebu.c | 63 ++-
 1 file changed, 41 insertions(+), 22 deletions(-)

diff --git a/drivers/gpio/gpio-mvebu.c b/drivers/gpio/gpio-mvebu.c
index 6e02148..0617e66 100644
--- a/drivers/gpio/gpio-mvebu.c
+++ b/drivers/gpio/gpio-mvebu.c
@@ -92,10 +92,17 @@
 
 #define MVEBU_MAX_GPIO_PER_BANK32
 
+struct mvebu_pwm_item {
+   struct gpio_desc*gpiod;
+   struct pwm_device   *device;
+   struct list_head node;
+};
+
 struct mvebu_pwm {
void __iomem*membase;
unsigned longclk_rate;
-   struct gpio_desc*gpiod;
+   int  id;
+   struct list_head pwms;
struct pwm_chip  chip;
spinlock_t   lock;
struct mvebu_gpio_chip  *mvchip;
@@ -599,29 +606,31 @@ static int mvebu_pwm_request(struct pwm_chip *chip, 
struct pwm_device *pwm)
struct mvebu_pwm *mvpwm = to_mvebu_pwm(chip);
struct mvebu_gpio_chip *mvchip = mvpwm->mvchip;
struct gpio_desc *desc;
+   struct mvebu_pwm_item *item;
unsigned long flags;
int ret = 0;
 
-   spin_lock_irqsave(>lock, flags);
-
-   if (mvpwm->gpiod) {
-   ret = -EBUSY;
-   } else {
-   desc = gpiochip_request_own_desc(>chip,
-pwm->hwpwm, "mvebu-pwm");
-   if (IS_ERR(desc)) {
-   ret = PTR_ERR(desc);
-   goto out;
-   }
+   item = kzalloc(sizeof(*item), GFP_KERNEL);
+   if (!item)
+   return -ENODEV;
 
-   ret = gpiod_direction_output(desc, 0);
-   if (ret) {
-   gpiochip_free_own_desc(desc);
-   goto out;
-   }
+   spin_lock_irqsave(>lock, flags);
+   desc = gpiochip_request_own_desc(>chip,
+pwm->hwpwm, "mvebu-pwm");
+   if (IS_ERR(desc)) {
+   ret = PTR_ERR(desc);
+   goto out;
+   }
 
-   mvpwm->gpiod = desc;
+   ret = gpiod_direction_output(desc, 0);
+   if (ret) {
+   gpiochip_free_own_desc(desc);
+   goto out;
}
+   item->gpiod = desc;
+   item->device = pwm;
+   INIT_LIST_HEAD(>node);
+   list_add_tail(>node, >pwms);
 out:
spin_unlock_irqrestore(>lock, flags);
return ret;
@@ -630,12 +639,20 @@ static int mvebu_pwm_request(struct pwm_chip *chip, 
struct pwm_device *pwm)
 static void mvebu_pwm_free(struct pwm_chip *chip, struct pwm_device *pwm)
 {
struct mvebu_pwm *mvpwm = to_mvebu_pwm(chip);
+   struct mvebu_pwm_item *item, *tmp;
unsigned long flags;
 
-   spin_lock_irqsave(>lock, flags);
-   gpiochip_free_own_desc(mvpwm->gpiod);
-   mvpwm->gpiod = NULL;
-   spin_unlock_irqrestore(>lock, flags);
+   list_for_each_entry_safe(item, tmp, >pwms, node) {
+   if (item->device == pwm) {
+   spin_lock_irqsave(>lock, flags);
+   gpiochip_free_own_desc(item->gpiod);
+   item->gpiod = NULL;
+   item->device = NULL;
+   list_del(>node);
+   spin_unlock_irqrestore(>lock, flags);
+   kfree(item);
+   }
+   }
 }
 
 static void mvebu_pwm_get_state(struct pwm_chip *chip,
@@ -804,6 +821,8 @@ static int mvebu_pwm_probe(struct platform_device *pdev,
return -ENOMEM;
mvchip->mvpwm = mvpwm;
mvpwm->mvchip = mvchip;
+   mvpwm->id = id;
+   INIT_LIST_HEAD(>pwms);
 
mvpwm->membase = devm_ioremap_resource(dev, res);
if (IS_ERR(mvpwm->membase))
-- 
2.7.4



[PATCH 2/2] gpio: mvebu: Allow to use non-default PWM counter

2018-07-20 Thread Aditya Prayoga
On multiple PWM lines, if the other PWM counter is unused, allocate it
to next PWM request. The priority would be:
1. Default counter assigned to the bank
2. Unused counter that is assigned to other bank
3. Fallback to default counter

For example on second bank there are three PWM request, first one would
use default counter (counter B), second one would try to use counter A,
and the third one would use counter B.

Signed-off-by: Aditya Prayoga 
---
 drivers/gpio/gpio-mvebu.c | 58 +--
 1 file changed, 56 insertions(+), 2 deletions(-)

diff --git a/drivers/gpio/gpio-mvebu.c b/drivers/gpio/gpio-mvebu.c
index 0617e66..5a39478 100644
--- a/drivers/gpio/gpio-mvebu.c
+++ b/drivers/gpio/gpio-mvebu.c
@@ -92,6 +92,11 @@
 
 #define MVEBU_MAX_GPIO_PER_BANK32
 
+enum mvebu_pwm_counter {
+   MVEBU_PWM_COUNTER_A = 0,
+   MVEBU_PWM_COUNTER_B,
+};
+
 struct mvebu_pwm_item {
struct gpio_desc*gpiod;
struct pwm_device   *device;
@@ -101,7 +106,8 @@ struct mvebu_pwm_item {
 struct mvebu_pwm {
void __iomem*membase;
unsigned longclk_rate;
-   int  id;
+   enum mvebu_pwm_counter   id;
+   struct list_head node;
struct list_head pwms;
struct pwm_chip  chip;
spinlock_t   lock;
@@ -113,6 +119,8 @@ struct mvebu_pwm {
u32  blink_off_duration;
 };
 
+static LIST_HEAD(mvebu_pwm_list);
+
 struct mvebu_gpio_chip {
struct gpio_chip   chip;
struct regmap *regs;
@@ -601,12 +609,24 @@ static struct mvebu_pwm *to_mvebu_pwm(struct pwm_chip 
*chip)
return container_of(chip, struct mvebu_pwm, chip);
 }
 
+static struct mvebu_pwm *mvebu_pwm_get_avail_counter(void)
+{
+   struct mvebu_pwm *counter;
+
+   list_for_each_entry(counter, _pwm_list, node) {
+   if (list_empty(>pwms))
+   return counter;
+   }
+   return NULL;
+}
+
 static int mvebu_pwm_request(struct pwm_chip *chip, struct pwm_device *pwm)
 {
struct mvebu_pwm *mvpwm = to_mvebu_pwm(chip);
struct mvebu_gpio_chip *mvchip = mvpwm->mvchip;
struct gpio_desc *desc;
struct mvebu_pwm_item *item;
+   struct mvebu_pwm *counter;
unsigned long flags;
int ret = 0;
 
@@ -615,6 +635,14 @@ static int mvebu_pwm_request(struct pwm_chip *chip, struct 
pwm_device *pwm)
return -ENODEV;
 
spin_lock_irqsave(>lock, flags);
+   regmap_read(mvchip->regs, GPIO_BLINK_EN_OFF + mvchip->offset,
+   >blink_en_reg);
+
+   if (mvchip->blink_en_reg & BIT(pwm->hwpwm)) {
+   ret = -EBUSY;
+   goto out;
+   }
+
desc = gpiochip_request_own_desc(>chip,
 pwm->hwpwm, "mvebu-pwm");
if (IS_ERR(desc)) {
@@ -627,10 +655,25 @@ static int mvebu_pwm_request(struct pwm_chip *chip, 
struct pwm_device *pwm)
gpiochip_free_own_desc(desc);
goto out;
}
+
+   counter = mvpwm;
+   if (!list_empty(>pwms)) {
+   counter = mvebu_pwm_get_avail_counter();
+   if (counter)
+   pwm->chip_data = counter;
+   else
+   counter = mvpwm;
+   }
+
+   regmap_update_bits(mvchip->regs, GPIO_BLINK_CNT_SELECT_OFF +
+   mvchip->offset, BIT(pwm->hwpwm),
+   counter->id ? BIT(pwm->hwpwm) : 0);
+   regmap_read(mvchip->regs, GPIO_BLINK_CNT_SELECT_OFF +
+   mvchip->offset, >blink_select);
item->gpiod = desc;
item->device = pwm;
INIT_LIST_HEAD(>node);
-   list_add_tail(>node, >pwms);
+   list_add_tail(>node, >pwms);
 out:
spin_unlock_irqrestore(>lock, flags);
return ret;
@@ -642,6 +685,9 @@ static void mvebu_pwm_free(struct pwm_chip *chip, struct 
pwm_device *pwm)
struct mvebu_pwm_item *item, *tmp;
unsigned long flags;
 
+   if (pwm->chip_data)
+   mvpwm = (struct mvebu_pwm *) pwm->chip_data;
+
list_for_each_entry_safe(item, tmp, >pwms, node) {
if (item->device == pwm) {
spin_lock_irqsave(>lock, flags);
@@ -665,6 +711,9 @@ static void mvebu_pwm_get_state(struct pwm_chip *chip,
unsigned long flags;
u32 u;
 
+   if (pwm->chip_data)
+   mvpwm = (struct mvebu_pwm *) pwm->chip_data;
+
spin_lock_irqsave(>lock, flags);
 
val = (unsigned long long)
@@ -712,6 +761,9 @@ static int mvebu_pwm_apply(struct pwm_chip *chip, struct 
pwm_device *pwm,
unsigned long flags;
unsigned int on, off;
 
+   if (pwm->chip_data)
+   mvpwm = (struct mvebu_pwm 

[PATCH 1/2] gpio: mvebu: Add support for multiple PWM lines per GPIO chip

2018-07-20 Thread Aditya Prayoga
Allow more than 1 PWM request (eg. PWM fan) on the same GPIO chip.

based on initial work on LK4.4 by Alban Browaeys.
URL: https://github.com/helios-4/linux-marvell/commit/743ae97
[Aditya Prayoga: forward port, cleanup]
Signed-off-by: Aditya Prayoga 
---
 drivers/gpio/gpio-mvebu.c | 63 ++-
 1 file changed, 41 insertions(+), 22 deletions(-)

diff --git a/drivers/gpio/gpio-mvebu.c b/drivers/gpio/gpio-mvebu.c
index 6e02148..0617e66 100644
--- a/drivers/gpio/gpio-mvebu.c
+++ b/drivers/gpio/gpio-mvebu.c
@@ -92,10 +92,17 @@
 
 #define MVEBU_MAX_GPIO_PER_BANK32
 
+struct mvebu_pwm_item {
+   struct gpio_desc*gpiod;
+   struct pwm_device   *device;
+   struct list_head node;
+};
+
 struct mvebu_pwm {
void __iomem*membase;
unsigned longclk_rate;
-   struct gpio_desc*gpiod;
+   int  id;
+   struct list_head pwms;
struct pwm_chip  chip;
spinlock_t   lock;
struct mvebu_gpio_chip  *mvchip;
@@ -599,29 +606,31 @@ static int mvebu_pwm_request(struct pwm_chip *chip, 
struct pwm_device *pwm)
struct mvebu_pwm *mvpwm = to_mvebu_pwm(chip);
struct mvebu_gpio_chip *mvchip = mvpwm->mvchip;
struct gpio_desc *desc;
+   struct mvebu_pwm_item *item;
unsigned long flags;
int ret = 0;
 
-   spin_lock_irqsave(>lock, flags);
-
-   if (mvpwm->gpiod) {
-   ret = -EBUSY;
-   } else {
-   desc = gpiochip_request_own_desc(>chip,
-pwm->hwpwm, "mvebu-pwm");
-   if (IS_ERR(desc)) {
-   ret = PTR_ERR(desc);
-   goto out;
-   }
+   item = kzalloc(sizeof(*item), GFP_KERNEL);
+   if (!item)
+   return -ENODEV;
 
-   ret = gpiod_direction_output(desc, 0);
-   if (ret) {
-   gpiochip_free_own_desc(desc);
-   goto out;
-   }
+   spin_lock_irqsave(>lock, flags);
+   desc = gpiochip_request_own_desc(>chip,
+pwm->hwpwm, "mvebu-pwm");
+   if (IS_ERR(desc)) {
+   ret = PTR_ERR(desc);
+   goto out;
+   }
 
-   mvpwm->gpiod = desc;
+   ret = gpiod_direction_output(desc, 0);
+   if (ret) {
+   gpiochip_free_own_desc(desc);
+   goto out;
}
+   item->gpiod = desc;
+   item->device = pwm;
+   INIT_LIST_HEAD(>node);
+   list_add_tail(>node, >pwms);
 out:
spin_unlock_irqrestore(>lock, flags);
return ret;
@@ -630,12 +639,20 @@ static int mvebu_pwm_request(struct pwm_chip *chip, 
struct pwm_device *pwm)
 static void mvebu_pwm_free(struct pwm_chip *chip, struct pwm_device *pwm)
 {
struct mvebu_pwm *mvpwm = to_mvebu_pwm(chip);
+   struct mvebu_pwm_item *item, *tmp;
unsigned long flags;
 
-   spin_lock_irqsave(>lock, flags);
-   gpiochip_free_own_desc(mvpwm->gpiod);
-   mvpwm->gpiod = NULL;
-   spin_unlock_irqrestore(>lock, flags);
+   list_for_each_entry_safe(item, tmp, >pwms, node) {
+   if (item->device == pwm) {
+   spin_lock_irqsave(>lock, flags);
+   gpiochip_free_own_desc(item->gpiod);
+   item->gpiod = NULL;
+   item->device = NULL;
+   list_del(>node);
+   spin_unlock_irqrestore(>lock, flags);
+   kfree(item);
+   }
+   }
 }
 
 static void mvebu_pwm_get_state(struct pwm_chip *chip,
@@ -804,6 +821,8 @@ static int mvebu_pwm_probe(struct platform_device *pdev,
return -ENOMEM;
mvchip->mvpwm = mvpwm;
mvpwm->mvchip = mvchip;
+   mvpwm->id = id;
+   INIT_LIST_HEAD(>pwms);
 
mvpwm->membase = devm_ioremap_resource(dev, res);
if (IS_ERR(mvpwm->membase))
-- 
2.7.4



[PATCH 0/2] gpio: mvebu: Add support for multiple PWM lines

2018-07-20 Thread Aditya Prayoga
Hi everyone,

Helios4, an Armada 388 based NAS SBC, provides 2 (4-pins) fan connectors.
The PWM pins on both connector are connected to GPIO on bank 1. Current gpio-
mvebu does not allow more than one PWM on the same bank.

Aditya

---

Aditya Prayoga (2):
  gpio: mvebu: Add support for multiple PWM lines per GPIO chip
  gpio: mvebu: Allow to use non-default PWM counter

 drivers/gpio/gpio-mvebu.c | 111 ++
 1 file changed, 92 insertions(+), 19 deletions(-)

-- 
2.7.4



[PATCH 0/2] gpio: mvebu: Add support for multiple PWM lines

2018-07-20 Thread Aditya Prayoga
Hi everyone,

Helios4, an Armada 388 based NAS SBC, provides 2 (4-pins) fan connectors.
The PWM pins on both connector are connected to GPIO on bank 1. Current gpio-
mvebu does not allow more than one PWM on the same bank.

Aditya

---

Aditya Prayoga (2):
  gpio: mvebu: Add support for multiple PWM lines per GPIO chip
  gpio: mvebu: Allow to use non-default PWM counter

 drivers/gpio/gpio-mvebu.c | 111 ++
 1 file changed, 92 insertions(+), 19 deletions(-)

-- 
2.7.4