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

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

[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 1/2] gpio: mvebu: Add support for multiple PWM lines per GPIO chip

[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 <aditya@kobol.io>
---
 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_BANK		32
 
+struct mvebu_pwm_item {
+	struct gpio_desc	*gpiod;
+	struct pwm_device	*device;
+	struct list_head	 node;
+};
+
 struct mvebu_pwm {
 	void __iomem		*membase;
 	unsigned long		 clk_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(&mvpwm->lock, flags);
-
-	if (mvpwm->gpiod) {
-		ret = -EBUSY;
-	} else {
-		desc = gpiochip_request_own_desc(&mvchip->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(&mvpwm->lock, flags);
+	desc = gpiochip_request_own_desc(&mvchip->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(&item->node);
+	list_add_tail(&item->node, &mvpwm->pwms);
 out:
 	spin_unlock_irqrestore(&mvpwm->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(&mvpwm->lock, flags);
-	gpiochip_free_own_desc(mvpwm->gpiod);
-	mvpwm->gpiod = NULL;
-	spin_unlock_irqrestore(&mvpwm->lock, flags);
+	list_for_each_entry_safe(item, tmp, &mvpwm->pwms, node) {
+		if (item->device == pwm) {
+			spin_lock_irqsave(&mvpwm->lock, flags);
+			gpiochip_free_own_desc(item->gpiod);
+			item->gpiod = NULL;
+			item->device = NULL;
+			list_del(&item->node);
+			spin_unlock_irqrestore(&mvpwm->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(&mvpwm->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

[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 <aditya@kobol.io>
---
 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_BANK		32
 
+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 long		 clk_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, &mvebu_pwm_list, node) {
+		if (list_empty(&counter->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(&mvpwm->lock, flags);
+	regmap_read(mvchip->regs, GPIO_BLINK_EN_OFF + mvchip->offset,
+		    &mvchip->blink_en_reg);
+
+	if (mvchip->blink_en_reg & BIT(pwm->hwpwm)) {
+		ret = -EBUSY;
+		goto out;
+	}
+
 	desc = gpiochip_request_own_desc(&mvchip->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(&mvpwm->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, &mvpwm->blink_select);
 	item->gpiod = desc;
 	item->device = pwm;
 	INIT_LIST_HEAD(&item->node);
-	list_add_tail(&item->node, &mvpwm->pwms);
+	list_add_tail(&item->node, &counter->pwms);
 out:
 	spin_unlock_irqrestore(&mvpwm->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, &mvpwm->pwms, node) {
 		if (item->device == pwm) {
 			spin_lock_irqsave(&mvpwm->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(&mvpwm->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 *) pwm->chip_data;
+
 	val = (unsigned long long) mvpwm->clk_rate * state->duty_cycle;
 	do_div(val, NSEC_PER_SEC);
 	if (val > UINT_MAX)
@@ -823,6 +875,7 @@ static int mvebu_pwm_probe(struct platform_device *pdev,
 	mvpwm->mvchip = mvchip;
 	mvpwm->id     = id;
 	INIT_LIST_HEAD(&mvpwm->pwms);
+	INIT_LIST_HEAD(&mvpwm->node);
 
 	mvpwm->membase = devm_ioremap_resource(dev, res);
 	if (IS_ERR(mvpwm->membase))
@@ -846,6 +899,7 @@ static int mvebu_pwm_probe(struct platform_device *pdev,
 	mvpwm->chip.base = -1;
 
 	spin_lock_init(&mvpwm->lock);
+	list_add_tail(&mvpwm->node, &mvebu_pwm_list);
 
 	return pwmchip_add(&mvpwm->chip);
 }
-- 
2.7.4

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

[Linus Walleij]

Hoping for some review from Gergory, Ralph or Richard who all seem
to use this driver!

Yours,
Linus Walleij

On Fri, Jul 20, 2018 at 9:24 PM Aditya Prayoga <aditya@kobol.io> wrote:
>
> 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
>

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

[Gregory CLEMENT]

Hi Linus and Adita
 
 On dim., juil. 29 2018, Linus Walleij <linus.walleij@linaro.org> wrote:

> Hoping for some review from Gergory, Ralph or Richard who all seem
> to use this driver!

Would it be possible to resend the series adding me in CC?  I would
like to comment the patches, but unfortunately it seems that I was in
none of the mailing list where the series had been sent.

I found the patches on patchwork, and started to have a look on it for
example, in the patch "gpio: mvebu: Add support for multiple PWM lines
per GPIO chip", I wonder why the id is stored as it was not used at all.

Having the patch inlined in an email would male the review easier.

Thanks,

Gregory

>
> Yours,
> Linus Walleij
>
> On Fri, Jul 20, 2018 at 9:24 PM Aditya Prayoga <aditya@kobol.io> wrote:
>>
>> 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
>>

-- 
Gregory Clement, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
http://bootlin.com

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

[Richard Genoud]

On 03/08/2018 11:36, Gregory CLEMENT wrote:
> Hi Linus and Adita
>  
>  On dim., juil. 29 2018, Linus Walleij <linus.walleij@linaro.org> wrote:
> 
>> Hoping for some review from Gergory, Ralph or Richard who all seem
>> to use this driver!
> 
> Would it be possible to resend the series adding me in CC?  I would
> like to comment the patches, but unfortunately it seems that I was in
> none of the mailing list where the series had been sent.
> 
> I found the patches on patchwork, and started to have a look on it for
> example, in the patch "gpio: mvebu: Add support for multiple PWM lines
> per GPIO chip", I wonder why the id is stored as it was not used at all.
> 
> Having the patch inlined in an email would male the review easier.
Same for me :)

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

[Richard Genoud]

On 03/08/2018 11:42, Richard Genoud wrote:
> On 03/08/2018 11:36, Gregory CLEMENT wrote:
>> Hi Linus and Adita
>>  
>>  On dim., juil. 29 2018, Linus Walleij <linus.walleij@linaro.org> wrote:
>>
>>> Hoping for some review from Gergory, Ralph or Richard who all seem
>>> to use this driver!
>>
>> Would it be possible to resend the series adding me in CC?  I would
>> like to comment the patches, but unfortunately it seems that I was in
>> none of the mailing list where the series had been sent.
>>
>> I found the patches on patchwork, and started to have a look on it for
>> example, in the patch "gpio: mvebu: Add support for multiple PWM lines
>> per GPIO chip", I wonder why the id is stored as it was not used at all.
>>
>> Having the patch inlined in an email would male the review easier.
> Same for me :)
> 

Sorry, for the noise, but please use my gmail address.

Thanks !

Richard