All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] gpio: mvebu: fix blink counter register selection
@ 2017-05-30 12:28 ` Richard Genoud
  0 siblings, 0 replies; 21+ messages in thread
From: Richard Genoud @ 2017-05-30 12:28 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Alexandre Courbot, Andrew Lunn, Gregory Clement, Jason Cooper,
	linux-arm-kernel, linux-gpio, linux-kernel, linux-pwm,
	Mark Rutland, Ralph Sennhauser, Rob Herring, Russell King,
	Sebastian Hesselbarth, Thierry Reding, Richard Genoud

The blink counter A was always selected because 0 was forced in the
blink select counter register.
The variable 'set' was obviously there to be used as the register value,
selecting the B counter when id==1 and A counter when id==0.

Tested on clearfog-pro (Marvell 88F6828)

Signed-off-by: Richard Genoud <richard.genoud@gmail.com>
---
 drivers/gpio/gpio-mvebu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpio/gpio-mvebu.c b/drivers/gpio/gpio-mvebu.c
index 19a92efabbef..cdef2c78cb3b 100644
--- a/drivers/gpio/gpio-mvebu.c
+++ b/drivers/gpio/gpio-mvebu.c
@@ -747,7 +747,7 @@ static int mvebu_pwm_probe(struct platform_device *pdev,
 		set = U32_MAX;
 	else
 		return -EINVAL;
-	writel_relaxed(0, mvebu_gpioreg_blink_counter_select(mvchip));
+	writel_relaxed(set, mvebu_gpioreg_blink_counter_select(mvchip));
 
 	mvpwm = devm_kzalloc(dev, sizeof(struct mvebu_pwm), GFP_KERNEL);
 	if (!mvpwm)

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 1/2] gpio: mvebu: fix blink counter register selection
@ 2017-05-30 12:28 ` Richard Genoud
  0 siblings, 0 replies; 21+ messages in thread
From: Richard Genoud @ 2017-05-30 12:28 UTC (permalink / raw)
  To: linux-arm-kernel

The blink counter A was always selected because 0 was forced in the
blink select counter register.
The variable 'set' was obviously there to be used as the register value,
selecting the B counter when id==1 and A counter when id==0.

Tested on clearfog-pro (Marvell 88F6828)

Signed-off-by: Richard Genoud <richard.genoud@gmail.com>
---
 drivers/gpio/gpio-mvebu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpio/gpio-mvebu.c b/drivers/gpio/gpio-mvebu.c
index 19a92efabbef..cdef2c78cb3b 100644
--- a/drivers/gpio/gpio-mvebu.c
+++ b/drivers/gpio/gpio-mvebu.c
@@ -747,7 +747,7 @@ static int mvebu_pwm_probe(struct platform_device *pdev,
 		set = U32_MAX;
 	else
 		return -EINVAL;
-	writel_relaxed(0, mvebu_gpioreg_blink_counter_select(mvchip));
+	writel_relaxed(set, mvebu_gpioreg_blink_counter_select(mvchip));
 
 	mvpwm = devm_kzalloc(dev, sizeof(struct mvebu_pwm), GFP_KERNEL);
 	if (!mvpwm)

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 2/2] gpio: mvebu: fix gpio bank registration when pwm is used
  2017-05-30 12:28 ` Richard Genoud
@ 2017-05-30 12:28   ` Richard Genoud
  -1 siblings, 0 replies; 21+ messages in thread
From: Richard Genoud @ 2017-05-30 12:28 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Alexandre Courbot, Andrew Lunn, Gregory Clement, Jason Cooper,
	linux-arm-kernel, linux-gpio, linux-kernel, linux-pwm,
	Mark Rutland, Ralph Sennhauser, Rob Herring, Russell King,
	Sebastian Hesselbarth, Thierry Reding, Richard Genoud

If more than one gpio bank has the "pwm" property, only one will be
registered successfully, all the others will fail with:
mvebu-gpio: probe of f1018140.gpio failed with error -17

That's because in alloc_pwms(), the chip->base (aka "int pwm"), was not
set (thus, ==0) ; and 0 is a meaningful start value in alloc_pwm().
What was intended is chip->base = -1.
Like that, the numbering will be done auto-magically

Tested on clearfog-pro (Marvell 88F6828)

Signed-off-by: Richard Genoud <richard.genoud@gmail.com>
---
 drivers/gpio/gpio-mvebu.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpio/gpio-mvebu.c b/drivers/gpio/gpio-mvebu.c
index cdef2c78cb3b..4734923e11fd 100644
--- a/drivers/gpio/gpio-mvebu.c
+++ b/drivers/gpio/gpio-mvebu.c
@@ -768,6 +768,7 @@ static int mvebu_pwm_probe(struct platform_device *pdev,
 	mvpwm->chip.dev = dev;
 	mvpwm->chip.ops = &mvebu_pwm_ops;
 	mvpwm->chip.npwm = mvchip->chip.ngpio;
+	mvpwm->chip.base = -1;
 
 	spin_lock_init(&mvpwm->lock);
 

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* [PATCH 2/2] gpio: mvebu: fix gpio bank registration when pwm is used
@ 2017-05-30 12:28   ` Richard Genoud
  0 siblings, 0 replies; 21+ messages in thread
From: Richard Genoud @ 2017-05-30 12:28 UTC (permalink / raw)
  To: linux-arm-kernel

If more than one gpio bank has the "pwm" property, only one will be
registered successfully, all the others will fail with:
mvebu-gpio: probe of f1018140.gpio failed with error -17

That's because in alloc_pwms(), the chip->base (aka "int pwm"), was not
set (thus, ==0) ; and 0 is a meaningful start value in alloc_pwm().
What was intended is chip->base = -1.
Like that, the numbering will be done auto-magically

Tested on clearfog-pro (Marvell 88F6828)

Signed-off-by: Richard Genoud <richard.genoud@gmail.com>
---
 drivers/gpio/gpio-mvebu.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpio/gpio-mvebu.c b/drivers/gpio/gpio-mvebu.c
index cdef2c78cb3b..4734923e11fd 100644
--- a/drivers/gpio/gpio-mvebu.c
+++ b/drivers/gpio/gpio-mvebu.c
@@ -768,6 +768,7 @@ static int mvebu_pwm_probe(struct platform_device *pdev,
 	mvpwm->chip.dev = dev;
 	mvpwm->chip.ops = &mvebu_pwm_ops;
 	mvpwm->chip.npwm = mvchip->chip.ngpio;
+	mvpwm->chip.base = -1;
 
 	spin_lock_init(&mvpwm->lock);
 

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [PATCH 1/2] gpio: mvebu: fix blink counter register selection
  2017-05-30 12:28 ` Richard Genoud
@ 2017-05-30 13:01   ` Gregory CLEMENT
  -1 siblings, 0 replies; 21+ messages in thread
From: Gregory CLEMENT @ 2017-05-30 13:01 UTC (permalink / raw)
  To: Richard Genoud
  Cc: Linus Walleij, Alexandre Courbot, Andrew Lunn, Jason Cooper,
	linux-arm-kernel, linux-gpio, linux-kernel, linux-pwm,
	Mark Rutland, Ralph Sennhauser, Rob Herring, Russell King,
	Sebastian Hesselbarth, Thierry Reding

Hi Richard,
 
 On mar., mai 30 2017, Richard Genoud <richard.genoud@gmail.com> wrote:

> The blink counter A was always selected because 0 was forced in the
> blink select counter register.
> The variable 'set' was obviously there to be used as the register value,
> selecting the B counter when id==1 and A counter when id==0.
>
> Tested on clearfog-pro (Marvell 88F6828)
>
> Signed-off-by: Richard Genoud <richard.genoud@gmail.com>

Looks good for me:

Reviewed-by: Gregory CLEMENT <gregory.clement@free-electrons.com>

I thinks this one should go to v4.12-rc as it is a fix.

As I modified the same part of the code in my series, I think I will
have to rebase my series on top of this patch as soon as Linus Walleij
will have accepted it.

Thanks,

Gregory


> ---
>  drivers/gpio/gpio-mvebu.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpio/gpio-mvebu.c b/drivers/gpio/gpio-mvebu.c
> index 19a92efabbef..cdef2c78cb3b 100644
> --- a/drivers/gpio/gpio-mvebu.c
> +++ b/drivers/gpio/gpio-mvebu.c
> @@ -747,7 +747,7 @@ static int mvebu_pwm_probe(struct platform_device *pdev,
>  		set = U32_MAX;
>  	else
>  		return -EINVAL;
> -	writel_relaxed(0, mvebu_gpioreg_blink_counter_select(mvchip));
> +	writel_relaxed(set, mvebu_gpioreg_blink_counter_select(mvchip));
>  
>  	mvpwm = devm_kzalloc(dev, sizeof(struct mvebu_pwm), GFP_KERNEL);
>  	if (!mvpwm)

-- 
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] 21+ messages in thread

* [PATCH 1/2] gpio: mvebu: fix blink counter register selection
@ 2017-05-30 13:01   ` Gregory CLEMENT
  0 siblings, 0 replies; 21+ messages in thread
From: Gregory CLEMENT @ 2017-05-30 13:01 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Richard,
 
 On mar., mai 30 2017, Richard Genoud <richard.genoud@gmail.com> wrote:

> The blink counter A was always selected because 0 was forced in the
> blink select counter register.
> The variable 'set' was obviously there to be used as the register value,
> selecting the B counter when id==1 and A counter when id==0.
>
> Tested on clearfog-pro (Marvell 88F6828)
>
> Signed-off-by: Richard Genoud <richard.genoud@gmail.com>

Looks good for me:

Reviewed-by: Gregory CLEMENT <gregory.clement@free-electrons.com>

I thinks this one should go to v4.12-rc as it is a fix.

As I modified the same part of the code in my series, I think I will
have to rebase my series on top of this patch as soon as Linus Walleij
will have accepted it.

Thanks,

Gregory


> ---
>  drivers/gpio/gpio-mvebu.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpio/gpio-mvebu.c b/drivers/gpio/gpio-mvebu.c
> index 19a92efabbef..cdef2c78cb3b 100644
> --- a/drivers/gpio/gpio-mvebu.c
> +++ b/drivers/gpio/gpio-mvebu.c
> @@ -747,7 +747,7 @@ static int mvebu_pwm_probe(struct platform_device *pdev,
>  		set = U32_MAX;
>  	else
>  		return -EINVAL;
> -	writel_relaxed(0, mvebu_gpioreg_blink_counter_select(mvchip));
> +	writel_relaxed(set, mvebu_gpioreg_blink_counter_select(mvchip));
>  
>  	mvpwm = devm_kzalloc(dev, sizeof(struct mvebu_pwm), GFP_KERNEL);
>  	if (!mvpwm)

-- 
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] 21+ messages in thread

* Re: [PATCH 1/2] gpio: mvebu: fix blink counter register selection
  2017-05-30 13:01   ` Gregory CLEMENT
@ 2017-05-30 13:06     ` Gregory CLEMENT
  -1 siblings, 0 replies; 21+ messages in thread
From: Gregory CLEMENT @ 2017-05-30 13:06 UTC (permalink / raw)
  To: Richard Genoud
  Cc: Mark Rutland, Alexandre Courbot, Jason Cooper, Andrew Lunn,
	Linus Walleij, linux-kernel, linux-pwm, Russell King, linux-gpio,
	Rob Herring, Thierry Reding, Ralph Sennhauser, linux-arm-kernel,
	Sebastian Hesselbarth

Hi again,
 
 On mar., mai 30 2017, Gregory CLEMENT <gregory.clement@free-electrons.com> wrote:

> Hi Richard,
>  
>  On mar., mai 30 2017, Richard Genoud <richard.genoud@gmail.com> wrote:
>
>> The blink counter A was always selected because 0 was forced in the
>> blink select counter register.
>> The variable 'set' was obviously there to be used as the register value,
>> selecting the B counter when id==1 and A counter when id==0.
>>
>> Tested on clearfog-pro (Marvell 88F6828)
>>
>> Signed-off-by: Richard Genoud <richard.genoud@gmail.com>
>
> Looks good for me:
>
> Reviewed-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
>
> I thinks this one should go to v4.12-rc as it is a fix.
>
> As I modified the same part of the code in my series, I think I will
> have to rebase my series on top of this patch as soon as Linus Walleij
> will have accepted it.

I think we could also add:
Fixes: 757642f9a584 ("gpio: mvebu: Add limited PWM support")

Gregory

>
> Thanks,
>
> Gregory
>
>
>> ---
>>  drivers/gpio/gpio-mvebu.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpio/gpio-mvebu.c b/drivers/gpio/gpio-mvebu.c
>> index 19a92efabbef..cdef2c78cb3b 100644
>> --- a/drivers/gpio/gpio-mvebu.c
>> +++ b/drivers/gpio/gpio-mvebu.c
>> @@ -747,7 +747,7 @@ static int mvebu_pwm_probe(struct platform_device *pdev,
>>  		set = U32_MAX;
>>  	else
>>  		return -EINVAL;
>> -	writel_relaxed(0, mvebu_gpioreg_blink_counter_select(mvchip));
>> +	writel_relaxed(set, mvebu_gpioreg_blink_counter_select(mvchip));
>>  
>>  	mvpwm = devm_kzalloc(dev, sizeof(struct mvebu_pwm), GFP_KERNEL);
>>  	if (!mvpwm)
>
> -- 
> Gregory Clement, Free Electrons
> Kernel, drivers, real-time and embedded Linux
> development, consulting, training and support.
> http://free-electrons.com
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

-- 
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] 21+ messages in thread

* [PATCH 1/2] gpio: mvebu: fix blink counter register selection
@ 2017-05-30 13:06     ` Gregory CLEMENT
  0 siblings, 0 replies; 21+ messages in thread
From: Gregory CLEMENT @ 2017-05-30 13:06 UTC (permalink / raw)
  To: linux-arm-kernel

Hi again,
 
 On mar., mai 30 2017, Gregory CLEMENT <gregory.clement@free-electrons.com> wrote:

> Hi Richard,
>  
>  On mar., mai 30 2017, Richard Genoud <richard.genoud@gmail.com> wrote:
>
>> The blink counter A was always selected because 0 was forced in the
>> blink select counter register.
>> The variable 'set' was obviously there to be used as the register value,
>> selecting the B counter when id==1 and A counter when id==0.
>>
>> Tested on clearfog-pro (Marvell 88F6828)
>>
>> Signed-off-by: Richard Genoud <richard.genoud@gmail.com>
>
> Looks good for me:
>
> Reviewed-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
>
> I thinks this one should go to v4.12-rc as it is a fix.
>
> As I modified the same part of the code in my series, I think I will
> have to rebase my series on top of this patch as soon as Linus Walleij
> will have accepted it.

I think we could also add:
Fixes: 757642f9a584 ("gpio: mvebu: Add limited PWM support")

Gregory

>
> Thanks,
>
> Gregory
>
>
>> ---
>>  drivers/gpio/gpio-mvebu.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpio/gpio-mvebu.c b/drivers/gpio/gpio-mvebu.c
>> index 19a92efabbef..cdef2c78cb3b 100644
>> --- a/drivers/gpio/gpio-mvebu.c
>> +++ b/drivers/gpio/gpio-mvebu.c
>> @@ -747,7 +747,7 @@ static int mvebu_pwm_probe(struct platform_device *pdev,
>>  		set = U32_MAX;
>>  	else
>>  		return -EINVAL;
>> -	writel_relaxed(0, mvebu_gpioreg_blink_counter_select(mvchip));
>> +	writel_relaxed(set, mvebu_gpioreg_blink_counter_select(mvchip));
>>  
>>  	mvpwm = devm_kzalloc(dev, sizeof(struct mvebu_pwm), GFP_KERNEL);
>>  	if (!mvpwm)
>
> -- 
> Gregory Clement, Free Electrons
> Kernel, drivers, real-time and embedded Linux
> development, consulting, training and support.
> http://free-electrons.com
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

-- 
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] 21+ messages in thread

* Re: [PATCH 2/2] gpio: mvebu: fix gpio bank registration when pwm is used
  2017-05-30 12:28   ` Richard Genoud
@ 2017-05-30 13:16     ` Gregory CLEMENT
  -1 siblings, 0 replies; 21+ messages in thread
From: Gregory CLEMENT @ 2017-05-30 13:16 UTC (permalink / raw)
  To: Richard Genoud
  Cc: Linus Walleij, Alexandre Courbot, Andrew Lunn, Jason Cooper,
	linux-arm-kernel, linux-gpio, linux-kernel, linux-pwm,
	Mark Rutland, Ralph Sennhauser, Rob Herring, Russell King,
	Sebastian Hesselbarth, Thierry Reding

Hi Richard,
 
 On mar., mai 30 2017, Richard Genoud <richard.genoud@gmail.com> wrote:

> If more than one gpio bank has the "pwm" property, only one will be
> registered successfully, all the others will fail with:
> mvebu-gpio: probe of f1018140.gpio failed with error -17
>
> That's because in alloc_pwms(), the chip->base (aka "int pwm"), was not
> set (thus, ==0) ; and 0 is a meaningful start value in alloc_pwm().
> What was intended is chip->base = -1.
> Like that, the numbering will be done auto-magically
>
> Tested on clearfog-pro (Marvell 88F6828)
>
> Signed-off-by: Richard Genoud <richard.genoud@gmail.com>
> ---
>  drivers/gpio/gpio-mvebu.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/gpio/gpio-mvebu.c b/drivers/gpio/gpio-mvebu.c
> index cdef2c78cb3b..4734923e11fd 100644
> --- a/drivers/gpio/gpio-mvebu.c
> +++ b/drivers/gpio/gpio-mvebu.c
> @@ -768,6 +768,7 @@ static int mvebu_pwm_probe(struct platform_device *pdev,
>  	mvpwm->chip.dev = dev;
>  	mvpwm->chip.ops = &mvebu_pwm_ops;
>  	mvpwm->chip.npwm = mvchip->chip.ngpio;
> +	mvpwm->chip.base = -1;

Why not using
mvpwm->chip.base = id * MVEBU_MAX_GPIO_PER_BANK;
as it is done in the mvebu_gpio_probe() function?

I think that if you use base = -1, then the number start from (512 -
number of pin already use). So starting from a low number for one
compatible and a high number for an other compatible could be confusing.

Besides that I agree that mvpwm->chip.base must be initialized and here
again for adding mor context to this patch, we could add:

Fixes: 757642f9a584 ("gpio: mvebu: Add limited PWM support")

Gregory

>  
>  	spin_lock_init(&mvpwm->lock);
>  

-- 
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] 21+ messages in thread

* [PATCH 2/2] gpio: mvebu: fix gpio bank registration when pwm is used
@ 2017-05-30 13:16     ` Gregory CLEMENT
  0 siblings, 0 replies; 21+ messages in thread
From: Gregory CLEMENT @ 2017-05-30 13:16 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Richard,
 
 On mar., mai 30 2017, Richard Genoud <richard.genoud@gmail.com> wrote:

> If more than one gpio bank has the "pwm" property, only one will be
> registered successfully, all the others will fail with:
> mvebu-gpio: probe of f1018140.gpio failed with error -17
>
> That's because in alloc_pwms(), the chip->base (aka "int pwm"), was not
> set (thus, ==0) ; and 0 is a meaningful start value in alloc_pwm().
> What was intended is chip->base = -1.
> Like that, the numbering will be done auto-magically
>
> Tested on clearfog-pro (Marvell 88F6828)
>
> Signed-off-by: Richard Genoud <richard.genoud@gmail.com>
> ---
>  drivers/gpio/gpio-mvebu.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/gpio/gpio-mvebu.c b/drivers/gpio/gpio-mvebu.c
> index cdef2c78cb3b..4734923e11fd 100644
> --- a/drivers/gpio/gpio-mvebu.c
> +++ b/drivers/gpio/gpio-mvebu.c
> @@ -768,6 +768,7 @@ static int mvebu_pwm_probe(struct platform_device *pdev,
>  	mvpwm->chip.dev = dev;
>  	mvpwm->chip.ops = &mvebu_pwm_ops;
>  	mvpwm->chip.npwm = mvchip->chip.ngpio;
> +	mvpwm->chip.base = -1;

Why not using
mvpwm->chip.base = id * MVEBU_MAX_GPIO_PER_BANK;
as it is done in the mvebu_gpio_probe() function?

I think that if you use base = -1, then the number start from (512 -
number of pin already use). So starting from a low number for one
compatible and a high number for an other compatible could be confusing.

Besides that I agree that mvpwm->chip.base must be initialized and here
again for adding mor context to this patch, we could add:

Fixes: 757642f9a584 ("gpio: mvebu: Add limited PWM support")

Gregory

>  
>  	spin_lock_init(&mvpwm->lock);
>  

-- 
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] 21+ messages in thread

* Re: [PATCH 2/2] gpio: mvebu: fix gpio bank registration when pwm is used
  2017-05-30 13:16     ` Gregory CLEMENT
  (?)
@ 2017-05-30 14:45       ` Richard Genoud
  -1 siblings, 0 replies; 21+ messages in thread
From: Richard Genoud @ 2017-05-30 14:45 UTC (permalink / raw)
  To: Gregory CLEMENT
  Cc: Linus Walleij, Alexandre Courbot, Andrew Lunn, Jason Cooper,
	linux-arm-kernel, linux-gpio, linux-kernel, linux-pwm,
	Mark Rutland, Ralph Sennhauser, Rob Herring, Russell King,
	Sebastian Hesselbarth, Thierry Reding

2017-05-30 15:16 GMT+02:00 Gregory CLEMENT <gregory.clement@free-electrons.com>:
> Hi Richard,
Hi Greg !

>
>  On mar., mai 30 2017, Richard Genoud <richard.genoud@gmail.com> wrote:
>
>> If more than one gpio bank has the "pwm" property, only one will be
>> registered successfully, all the others will fail with:
>> mvebu-gpio: probe of f1018140.gpio failed with error -17
>>
>> That's because in alloc_pwms(), the chip->base (aka "int pwm"), was not
>> set (thus, ==0) ; and 0 is a meaningful start value in alloc_pwm().
>> What was intended is chip->base = -1.
>> Like that, the numbering will be done auto-magically
>>
>> Tested on clearfog-pro (Marvell 88F6828)
>>
>> Signed-off-by: Richard Genoud <richard.genoud@gmail.com>
>> ---
>>  drivers/gpio/gpio-mvebu.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/gpio/gpio-mvebu.c b/drivers/gpio/gpio-mvebu.c
>> index cdef2c78cb3b..4734923e11fd 100644
>> --- a/drivers/gpio/gpio-mvebu.c
>> +++ b/drivers/gpio/gpio-mvebu.c
>> @@ -768,6 +768,7 @@ static int mvebu_pwm_probe(struct platform_device *pdev,
>>       mvpwm->chip.dev = dev;
>>       mvpwm->chip.ops = &mvebu_pwm_ops;
>>       mvpwm->chip.npwm = mvchip->chip.ngpio;
>> +     mvpwm->chip.base = -1;
>
> Why not using
> mvpwm->chip.base = id * MVEBU_MAX_GPIO_PER_BANK;
> as it is done in the mvebu_gpio_probe() function?
Yes, that was my first move:
mvpwm->chip.base = mvchip->chip.base;

But after some reflexion, mvpwm->chip.base is not the GPIO base, it's
the PWM base,
(mvpwm->chip is a struct pwm_chip), so it would we weird to have
"holes" in the declared PWMs.
I'm not clear, so here's an example:
If, in the DTS, we have:
            gpio0: gpio@18100 {
                compatible = "marvell,armada-370-xp-gpio",
                         "marvell,orion-gpio";
                reg = <0x18100 0x40>, <0x181c0 0x08>;
                reg-names = "gpio";  /* "pwm" missing */
[...]
            gpio1: gpio@18140 {
                compatible = "marvell,armada-370-xp-gpio",
                         "marvell,orion-gpio";
                reg = <0x18140 0x40>, <0x181c8 0x08>;
                reg-names = "gpio", "pwm";
In this case, if gpio0 is not declared as PWM capable, the PWM
numbering will start at 32 if we have
mvpwm->chip.base = mvchip->chip.base;
but it will start at 0 if we have mvpwm->chip.base = -1;

The pros for having mvpwm->chip.base = mvchip->chip.base; is mainly
the stable numbering:
if we add the "pwm" feature to gpio0 afterwards, the pwm numbering in
sysfs will stay the same.
And if we have mvpwm->chip.base = -1; the pwm numbering will be shifted.

Looking back at the V5 of this patch
https://www.spinics.net/lists/kernel/msg2484889.html
There was the line:
mvpwm->chip.base = mvchip->chip.base;
I guess it got lost in the v6 rebase.

So I could change it back, but I'm not sure which one is better.

>
> I think that if you use base = -1, then the number start from (512 -
> number of pin already use). So starting from a low number for one
> compatible and a high number for an other compatible could be confusing.
>
> Besides that I agree that mvpwm->chip.base must be initialized and here
> again for adding mor context to this patch, we could add:
>
> Fixes: 757642f9a584 ("gpio: mvebu: Add limited PWM support")
yes, definitely !
should I resend the patch with it or the maintainer will add it ?

> Gregory
>
>>
>>       spin_lock_init(&mvpwm->lock);
>>
>
> --
> 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] 21+ messages in thread

* Re: [PATCH 2/2] gpio: mvebu: fix gpio bank registration when pwm is used
@ 2017-05-30 14:45       ` Richard Genoud
  0 siblings, 0 replies; 21+ messages in thread
From: Richard Genoud @ 2017-05-30 14:45 UTC (permalink / raw)
  To: Gregory CLEMENT
  Cc: Linus Walleij, Alexandre Courbot, Andrew Lunn, Jason Cooper,
	linux-arm-kernel, linux-gpio, linux-kernel, linux-pwm,
	Mark Rutland, Ralph Sennhauser, Rob Herring, Russell King,
	Sebastian Hesselbarth, Thierry Reding

2017-05-30 15:16 GMT+02:00 Gregory CLEMENT <gregory.clement@free-electrons.com>:
> Hi Richard,
Hi Greg !

>
>  On mar., mai 30 2017, Richard Genoud <richard.genoud@gmail.com> wrote:
>
>> If more than one gpio bank has the "pwm" property, only one will be
>> registered successfully, all the others will fail with:
>> mvebu-gpio: probe of f1018140.gpio failed with error -17
>>
>> That's because in alloc_pwms(), the chip->base (aka "int pwm"), was not
>> set (thus, ==0) ; and 0 is a meaningful start value in alloc_pwm().
>> What was intended is chip->base = -1.
>> Like that, the numbering will be done auto-magically
>>
>> Tested on clearfog-pro (Marvell 88F6828)
>>
>> Signed-off-by: Richard Genoud <richard.genoud@gmail.com>
>> ---
>>  drivers/gpio/gpio-mvebu.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/gpio/gpio-mvebu.c b/drivers/gpio/gpio-mvebu.c
>> index cdef2c78cb3b..4734923e11fd 100644
>> --- a/drivers/gpio/gpio-mvebu.c
>> +++ b/drivers/gpio/gpio-mvebu.c
>> @@ -768,6 +768,7 @@ static int mvebu_pwm_probe(struct platform_device *pdev,
>>       mvpwm->chip.dev = dev;
>>       mvpwm->chip.ops = &mvebu_pwm_ops;
>>       mvpwm->chip.npwm = mvchip->chip.ngpio;
>> +     mvpwm->chip.base = -1;
>
> Why not using
> mvpwm->chip.base = id * MVEBU_MAX_GPIO_PER_BANK;
> as it is done in the mvebu_gpio_probe() function?
Yes, that was my first move:
mvpwm->chip.base = mvchip->chip.base;

But after some reflexion, mvpwm->chip.base is not the GPIO base, it's
the PWM base,
(mvpwm->chip is a struct pwm_chip), so it would we weird to have
"holes" in the declared PWMs.
I'm not clear, so here's an example:
If, in the DTS, we have:
            gpio0: gpio@18100 {
                compatible = "marvell,armada-370-xp-gpio",
                         "marvell,orion-gpio";
                reg = <0x18100 0x40>, <0x181c0 0x08>;
                reg-names = "gpio";  /* "pwm" missing */
[...]
            gpio1: gpio@18140 {
                compatible = "marvell,armada-370-xp-gpio",
                         "marvell,orion-gpio";
                reg = <0x18140 0x40>, <0x181c8 0x08>;
                reg-names = "gpio", "pwm";
In this case, if gpio0 is not declared as PWM capable, the PWM
numbering will start at 32 if we have
mvpwm->chip.base = mvchip->chip.base;
but it will start at 0 if we have mvpwm->chip.base = -1;

The pros for having mvpwm->chip.base = mvchip->chip.base; is mainly
the stable numbering:
if we add the "pwm" feature to gpio0 afterwards, the pwm numbering in
sysfs will stay the same.
And if we have mvpwm->chip.base = -1; the pwm numbering will be shifted.

Looking back at the V5 of this patch
https://www.spinics.net/lists/kernel/msg2484889.html
There was the line:
mvpwm->chip.base = mvchip->chip.base;
I guess it got lost in the v6 rebase.

So I could change it back, but I'm not sure which one is better.

>
> I think that if you use base = -1, then the number start from (512 -
> number of pin already use). So starting from a low number for one
> compatible and a high number for an other compatible could be confusing.
>
> Besides that I agree that mvpwm->chip.base must be initialized and here
> again for adding mor context to this patch, we could add:
>
> Fixes: 757642f9a584 ("gpio: mvebu: Add limited PWM support")
yes, definitely !
should I resend the patch with it or the maintainer will add it ?

> Gregory
>
>>
>>       spin_lock_init(&mvpwm->lock);
>>
>
> --
> 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] 21+ messages in thread

* [PATCH 2/2] gpio: mvebu: fix gpio bank registration when pwm is used
@ 2017-05-30 14:45       ` Richard Genoud
  0 siblings, 0 replies; 21+ messages in thread
From: Richard Genoud @ 2017-05-30 14:45 UTC (permalink / raw)
  To: linux-arm-kernel

2017-05-30 15:16 GMT+02:00 Gregory CLEMENT <gregory.clement@free-electrons.com>:
> Hi Richard,
Hi Greg !

>
>  On mar., mai 30 2017, Richard Genoud <richard.genoud@gmail.com> wrote:
>
>> If more than one gpio bank has the "pwm" property, only one will be
>> registered successfully, all the others will fail with:
>> mvebu-gpio: probe of f1018140.gpio failed with error -17
>>
>> That's because in alloc_pwms(), the chip->base (aka "int pwm"), was not
>> set (thus, ==0) ; and 0 is a meaningful start value in alloc_pwm().
>> What was intended is chip->base = -1.
>> Like that, the numbering will be done auto-magically
>>
>> Tested on clearfog-pro (Marvell 88F6828)
>>
>> Signed-off-by: Richard Genoud <richard.genoud@gmail.com>
>> ---
>>  drivers/gpio/gpio-mvebu.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/gpio/gpio-mvebu.c b/drivers/gpio/gpio-mvebu.c
>> index cdef2c78cb3b..4734923e11fd 100644
>> --- a/drivers/gpio/gpio-mvebu.c
>> +++ b/drivers/gpio/gpio-mvebu.c
>> @@ -768,6 +768,7 @@ static int mvebu_pwm_probe(struct platform_device *pdev,
>>       mvpwm->chip.dev = dev;
>>       mvpwm->chip.ops = &mvebu_pwm_ops;
>>       mvpwm->chip.npwm = mvchip->chip.ngpio;
>> +     mvpwm->chip.base = -1;
>
> Why not using
> mvpwm->chip.base = id * MVEBU_MAX_GPIO_PER_BANK;
> as it is done in the mvebu_gpio_probe() function?
Yes, that was my first move:
mvpwm->chip.base = mvchip->chip.base;

But after some reflexion, mvpwm->chip.base is not the GPIO base, it's
the PWM base,
(mvpwm->chip is a struct pwm_chip), so it would we weird to have
"holes" in the declared PWMs.
I'm not clear, so here's an example:
If, in the DTS, we have:
            gpio0: gpio at 18100 {
                compatible = "marvell,armada-370-xp-gpio",
                         "marvell,orion-gpio";
                reg = <0x18100 0x40>, <0x181c0 0x08>;
                reg-names = "gpio";  /* "pwm" missing */
[...]
            gpio1: gpio at 18140 {
                compatible = "marvell,armada-370-xp-gpio",
                         "marvell,orion-gpio";
                reg = <0x18140 0x40>, <0x181c8 0x08>;
                reg-names = "gpio", "pwm";
In this case, if gpio0 is not declared as PWM capable, the PWM
numbering will start at 32 if we have
mvpwm->chip.base = mvchip->chip.base;
but it will start at 0 if we have mvpwm->chip.base = -1;

The pros for having mvpwm->chip.base = mvchip->chip.base; is mainly
the stable numbering:
if we add the "pwm" feature to gpio0 afterwards, the pwm numbering in
sysfs will stay the same.
And if we have mvpwm->chip.base = -1; the pwm numbering will be shifted.

Looking back at the V5 of this patch
https://www.spinics.net/lists/kernel/msg2484889.html
There was the line:
mvpwm->chip.base = mvchip->chip.base;
I guess it got lost in the v6 rebase.

So I could change it back, but I'm not sure which one is better.

>
> I think that if you use base = -1, then the number start from (512 -
> number of pin already use). So starting from a low number for one
> compatible and a high number for an other compatible could be confusing.
>
> Besides that I agree that mvpwm->chip.base must be initialized and here
> again for adding mor context to this patch, we could add:
>
> Fixes: 757642f9a584 ("gpio: mvebu: Add limited PWM support")
yes, definitely !
should I resend the patch with it or the maintainer will add it ?

> Gregory
>
>>
>>       spin_lock_init(&mvpwm->lock);
>>
>
> --
> 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] 21+ messages in thread

* Re: [PATCH 2/2] gpio: mvebu: fix gpio bank registration when pwm is used
  2017-05-30 14:45       ` Richard Genoud
  (?)
@ 2017-05-30 15:14         ` Ralph Sennhauser
  -1 siblings, 0 replies; 21+ messages in thread
From: Ralph Sennhauser @ 2017-05-30 15:14 UTC (permalink / raw)
  To: Richard Genoud
  Cc: Gregory CLEMENT, Linus Walleij, Alexandre Courbot, Andrew Lunn,
	Jason Cooper, linux-arm-kernel, linux-gpio, linux-kernel,
	linux-pwm, Mark Rutland, Rob Herring, Russell King,
	Sebastian Hesselbarth, Thierry Reding

Hi Richard

On Tue, 30 May 2017 16:45:24 +0200
Richard Genoud <richard.genoud@gmail.com> wrote:

> 2017-05-30 15:16 GMT+02:00 Gregory CLEMENT
> <gregory.clement@free-electrons.com>:
> > Hi Richard,  
> Hi Greg !
> 
> >
> >  On mar., mai 30 2017, Richard Genoud <richard.genoud@gmail.com>
> > wrote: 
> >> If more than one gpio bank has the "pwm" property, only one will be
> >> registered successfully, all the others will fail with:
> >> mvebu-gpio: probe of f1018140.gpio failed with error -17
> >>
> >> That's because in alloc_pwms(), the chip->base (aka "int pwm"),
> >> was not set (thus, ==0) ; and 0 is a meaningful start value in
> >> alloc_pwm(). What was intended is chip->base = -1.
> >> Like that, the numbering will be done auto-magically
> >>
> >> Tested on clearfog-pro (Marvell 88F6828)
> >>
> >> Signed-off-by: Richard Genoud <richard.genoud@gmail.com>
> >> ---
> >>  drivers/gpio/gpio-mvebu.c | 1 +
> >>  1 file changed, 1 insertion(+)
> >>
> >> diff --git a/drivers/gpio/gpio-mvebu.c b/drivers/gpio/gpio-mvebu.c
> >> index cdef2c78cb3b..4734923e11fd 100644
> >> --- a/drivers/gpio/gpio-mvebu.c
> >> +++ b/drivers/gpio/gpio-mvebu.c
> >> @@ -768,6 +768,7 @@ static int mvebu_pwm_probe(struct
> >> platform_device *pdev, mvpwm->chip.dev = dev;
> >>       mvpwm->chip.ops = &mvebu_pwm_ops;
> >>       mvpwm->chip.npwm = mvchip->chip.ngpio;
> >> +     mvpwm->chip.base = -1;  
> >
> > Why not using
> > mvpwm->chip.base = id * MVEBU_MAX_GPIO_PER_BANK;
> > as it is done in the mvebu_gpio_probe() function?  
> Yes, that was my first move:
> mvpwm->chip.base = mvchip->chip.base;
> 
> But after some reflexion, mvpwm->chip.base is not the GPIO base, it's
> the PWM base,
> (mvpwm->chip is a struct pwm_chip), so it would we weird to have
> "holes" in the declared PWMs.
> I'm not clear, so here's an example:
> If, in the DTS, we have:
>             gpio0: gpio@18100 {
>                 compatible = "marvell,armada-370-xp-gpio",
>                          "marvell,orion-gpio";
>                 reg = <0x18100 0x40>, <0x181c0 0x08>;
>                 reg-names = "gpio";  /* "pwm" missing */
> [...]
>             gpio1: gpio@18140 {
>                 compatible = "marvell,armada-370-xp-gpio",
>                          "marvell,orion-gpio";
>                 reg = <0x18140 0x40>, <0x181c8 0x08>;
>                 reg-names = "gpio", "pwm";
> In this case, if gpio0 is not declared as PWM capable, the PWM
> numbering will start at 32 if we have
> mvpwm->chip.base = mvchip->chip.base;
> but it will start at 0 if we have mvpwm->chip.base = -1;
> 
> The pros for having mvpwm->chip.base = mvchip->chip.base; is mainly
> the stable numbering:
> if we add the "pwm" feature to gpio0 afterwards, the pwm numbering in
> sysfs will stay the same.
> And if we have mvpwm->chip.base = -1; the pwm numbering will be
> shifted.
> 
> Looking back at the V5 of this patch
> https://www.spinics.net/lists/kernel/msg2484889.html
> There was the line:
> mvpwm->chip.base = mvchip->chip.base;
> I guess it got lost in the v6 rebase.
> 
> So I could change it back, but I'm not sure which one is better.

Thierry Redding pointed out that the region might already be occupied
by another PWM chip, unlikely but not impossible. That's why it got
changed for v6.

See https://www.spinics.net/lists/devicetree/msg173138.html

Ralph


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 2/2] gpio: mvebu: fix gpio bank registration when pwm is used
@ 2017-05-30 15:14         ` Ralph Sennhauser
  0 siblings, 0 replies; 21+ messages in thread
From: Ralph Sennhauser @ 2017-05-30 15:14 UTC (permalink / raw)
  To: Richard Genoud
  Cc: Gregory CLEMENT, Linus Walleij, Alexandre Courbot, Andrew Lunn,
	Jason Cooper, linux-arm-kernel, linux-gpio, linux-kernel,
	linux-pwm, Mark Rutland, Rob Herring, Russell King,
	Sebastian Hesselbarth, Thierry Reding

Hi Richard

On Tue, 30 May 2017 16:45:24 +0200
Richard Genoud <richard.genoud@gmail.com> wrote:

> 2017-05-30 15:16 GMT+02:00 Gregory CLEMENT
> <gregory.clement@free-electrons.com>:
> > Hi Richard,  
> Hi Greg !
> 
> >
> >  On mar., mai 30 2017, Richard Genoud <richard.genoud@gmail.com>
> > wrote: 
> >> If more than one gpio bank has the "pwm" property, only one will be
> >> registered successfully, all the others will fail with:
> >> mvebu-gpio: probe of f1018140.gpio failed with error -17
> >>
> >> That's because in alloc_pwms(), the chip->base (aka "int pwm"),
> >> was not set (thus, ==0) ; and 0 is a meaningful start value in
> >> alloc_pwm(). What was intended is chip->base = -1.
> >> Like that, the numbering will be done auto-magically
> >>
> >> Tested on clearfog-pro (Marvell 88F6828)
> >>
> >> Signed-off-by: Richard Genoud <richard.genoud@gmail.com>
> >> ---
> >>  drivers/gpio/gpio-mvebu.c | 1 +
> >>  1 file changed, 1 insertion(+)
> >>
> >> diff --git a/drivers/gpio/gpio-mvebu.c b/drivers/gpio/gpio-mvebu.c
> >> index cdef2c78cb3b..4734923e11fd 100644
> >> --- a/drivers/gpio/gpio-mvebu.c
> >> +++ b/drivers/gpio/gpio-mvebu.c
> >> @@ -768,6 +768,7 @@ static int mvebu_pwm_probe(struct
> >> platform_device *pdev, mvpwm->chip.dev = dev;
> >>       mvpwm->chip.ops = &mvebu_pwm_ops;
> >>       mvpwm->chip.npwm = mvchip->chip.ngpio;
> >> +     mvpwm->chip.base = -1;  
> >
> > Why not using
> > mvpwm->chip.base = id * MVEBU_MAX_GPIO_PER_BANK;
> > as it is done in the mvebu_gpio_probe() function?  
> Yes, that was my first move:
> mvpwm->chip.base = mvchip->chip.base;
> 
> But after some reflexion, mvpwm->chip.base is not the GPIO base, it's
> the PWM base,
> (mvpwm->chip is a struct pwm_chip), so it would we weird to have
> "holes" in the declared PWMs.
> I'm not clear, so here's an example:
> If, in the DTS, we have:
>             gpio0: gpio@18100 {
>                 compatible = "marvell,armada-370-xp-gpio",
>                          "marvell,orion-gpio";
>                 reg = <0x18100 0x40>, <0x181c0 0x08>;
>                 reg-names = "gpio";  /* "pwm" missing */
> [...]
>             gpio1: gpio@18140 {
>                 compatible = "marvell,armada-370-xp-gpio",
>                          "marvell,orion-gpio";
>                 reg = <0x18140 0x40>, <0x181c8 0x08>;
>                 reg-names = "gpio", "pwm";
> In this case, if gpio0 is not declared as PWM capable, the PWM
> numbering will start at 32 if we have
> mvpwm->chip.base = mvchip->chip.base;
> but it will start at 0 if we have mvpwm->chip.base = -1;
> 
> The pros for having mvpwm->chip.base = mvchip->chip.base; is mainly
> the stable numbering:
> if we add the "pwm" feature to gpio0 afterwards, the pwm numbering in
> sysfs will stay the same.
> And if we have mvpwm->chip.base = -1; the pwm numbering will be
> shifted.
> 
> Looking back at the V5 of this patch
> https://www.spinics.net/lists/kernel/msg2484889.html
> There was the line:
> mvpwm->chip.base = mvchip->chip.base;
> I guess it got lost in the v6 rebase.
> 
> So I could change it back, but I'm not sure which one is better.

Thierry Redding pointed out that the region might already be occupied
by another PWM chip, unlikely but not impossible. That's why it got
changed for v6.

See https://www.spinics.net/lists/devicetree/msg173138.html

Ralph

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH 2/2] gpio: mvebu: fix gpio bank registration when pwm is used
@ 2017-05-30 15:14         ` Ralph Sennhauser
  0 siblings, 0 replies; 21+ messages in thread
From: Ralph Sennhauser @ 2017-05-30 15:14 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Richard

On Tue, 30 May 2017 16:45:24 +0200
Richard Genoud <richard.genoud@gmail.com> wrote:

> 2017-05-30 15:16 GMT+02:00 Gregory CLEMENT
> <gregory.clement@free-electrons.com>:
> > Hi Richard,  
> Hi Greg !
> 
> >
> >  On mar., mai 30 2017, Richard Genoud <richard.genoud@gmail.com>
> > wrote: 
> >> If more than one gpio bank has the "pwm" property, only one will be
> >> registered successfully, all the others will fail with:
> >> mvebu-gpio: probe of f1018140.gpio failed with error -17
> >>
> >> That's because in alloc_pwms(), the chip->base (aka "int pwm"),
> >> was not set (thus, ==0) ; and 0 is a meaningful start value in
> >> alloc_pwm(). What was intended is chip->base = -1.
> >> Like that, the numbering will be done auto-magically
> >>
> >> Tested on clearfog-pro (Marvell 88F6828)
> >>
> >> Signed-off-by: Richard Genoud <richard.genoud@gmail.com>
> >> ---
> >>  drivers/gpio/gpio-mvebu.c | 1 +
> >>  1 file changed, 1 insertion(+)
> >>
> >> diff --git a/drivers/gpio/gpio-mvebu.c b/drivers/gpio/gpio-mvebu.c
> >> index cdef2c78cb3b..4734923e11fd 100644
> >> --- a/drivers/gpio/gpio-mvebu.c
> >> +++ b/drivers/gpio/gpio-mvebu.c
> >> @@ -768,6 +768,7 @@ static int mvebu_pwm_probe(struct
> >> platform_device *pdev, mvpwm->chip.dev = dev;
> >>       mvpwm->chip.ops = &mvebu_pwm_ops;
> >>       mvpwm->chip.npwm = mvchip->chip.ngpio;
> >> +     mvpwm->chip.base = -1;  
> >
> > Why not using
> > mvpwm->chip.base = id * MVEBU_MAX_GPIO_PER_BANK;
> > as it is done in the mvebu_gpio_probe() function?  
> Yes, that was my first move:
> mvpwm->chip.base = mvchip->chip.base;
> 
> But after some reflexion, mvpwm->chip.base is not the GPIO base, it's
> the PWM base,
> (mvpwm->chip is a struct pwm_chip), so it would we weird to have
> "holes" in the declared PWMs.
> I'm not clear, so here's an example:
> If, in the DTS, we have:
>             gpio0: gpio at 18100 {
>                 compatible = "marvell,armada-370-xp-gpio",
>                          "marvell,orion-gpio";
>                 reg = <0x18100 0x40>, <0x181c0 0x08>;
>                 reg-names = "gpio";  /* "pwm" missing */
> [...]
>             gpio1: gpio at 18140 {
>                 compatible = "marvell,armada-370-xp-gpio",
>                          "marvell,orion-gpio";
>                 reg = <0x18140 0x40>, <0x181c8 0x08>;
>                 reg-names = "gpio", "pwm";
> In this case, if gpio0 is not declared as PWM capable, the PWM
> numbering will start at 32 if we have
> mvpwm->chip.base = mvchip->chip.base;
> but it will start at 0 if we have mvpwm->chip.base = -1;
> 
> The pros for having mvpwm->chip.base = mvchip->chip.base; is mainly
> the stable numbering:
> if we add the "pwm" feature to gpio0 afterwards, the pwm numbering in
> sysfs will stay the same.
> And if we have mvpwm->chip.base = -1; the pwm numbering will be
> shifted.
> 
> Looking back at the V5 of this patch
> https://www.spinics.net/lists/kernel/msg2484889.html
> There was the line:
> mvpwm->chip.base = mvchip->chip.base;
> I guess it got lost in the v6 rebase.
> 
> So I could change it back, but I'm not sure which one is better.

Thierry Redding pointed out that the region might already be occupied
by another PWM chip, unlikely but not impossible. That's why it got
changed for v6.

See https://www.spinics.net/lists/devicetree/msg173138.html

Ralph

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 1/2] gpio: mvebu: fix blink counter register selection
  2017-05-30 12:28 ` Richard Genoud
@ 2017-05-30 15:18   ` Ralph Sennhauser
  -1 siblings, 0 replies; 21+ messages in thread
From: Ralph Sennhauser @ 2017-05-30 15:18 UTC (permalink / raw)
  To: Richard Genoud
  Cc: Linus Walleij, Alexandre Courbot, Andrew Lunn, Gregory Clement,
	Jason Cooper, linux-arm-kernel, linux-gpio, linux-kernel,
	linux-pwm, Mark Rutland, Rob Herring, Russell King,
	Sebastian Hesselbarth, Thierry Reding

Hi Richard

On Tue, 30 May 2017 14:28:47 +0200
Richard Genoud <richard.genoud@gmail.com> wrote:

> The blink counter A was always selected because 0 was forced in the
> blink select counter register.
> The variable 'set' was obviously there to be used as the register
> value, selecting the B counter when id==1 and A counter when id==0.
> 
> Tested on clearfog-pro (Marvell 88F6828)

As stated in the other thread, testing on clearfog-pro is likely bogus,
still the fix is correct.

Reviewed-by: Ralph Sennhauser <ralph.sennhauser@gmail.com>

Thanks
Ralph

> 
> Signed-off-by: Richard Genoud <richard.genoud@gmail.com>
> ---
>  drivers/gpio/gpio-mvebu.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpio/gpio-mvebu.c b/drivers/gpio/gpio-mvebu.c
> index 19a92efabbef..cdef2c78cb3b 100644
> --- a/drivers/gpio/gpio-mvebu.c
> +++ b/drivers/gpio/gpio-mvebu.c
> @@ -747,7 +747,7 @@ static int mvebu_pwm_probe(struct platform_device
> *pdev, set = U32_MAX;
>  	else
>  		return -EINVAL;
> -	writel_relaxed(0,
> mvebu_gpioreg_blink_counter_select(mvchip));
> +	writel_relaxed(set,
> mvebu_gpioreg_blink_counter_select(mvchip)); 
>  	mvpwm = devm_kzalloc(dev, sizeof(struct mvebu_pwm),
> GFP_KERNEL); if (!mvpwm)

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH 1/2] gpio: mvebu: fix blink counter register selection
@ 2017-05-30 15:18   ` Ralph Sennhauser
  0 siblings, 0 replies; 21+ messages in thread
From: Ralph Sennhauser @ 2017-05-30 15:18 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Richard

On Tue, 30 May 2017 14:28:47 +0200
Richard Genoud <richard.genoud@gmail.com> wrote:

> The blink counter A was always selected because 0 was forced in the
> blink select counter register.
> The variable 'set' was obviously there to be used as the register
> value, selecting the B counter when id==1 and A counter when id==0.
> 
> Tested on clearfog-pro (Marvell 88F6828)

As stated in the other thread, testing on clearfog-pro is likely bogus,
still the fix is correct.

Reviewed-by: Ralph Sennhauser <ralph.sennhauser@gmail.com>

Thanks
Ralph

> 
> Signed-off-by: Richard Genoud <richard.genoud@gmail.com>
> ---
>  drivers/gpio/gpio-mvebu.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpio/gpio-mvebu.c b/drivers/gpio/gpio-mvebu.c
> index 19a92efabbef..cdef2c78cb3b 100644
> --- a/drivers/gpio/gpio-mvebu.c
> +++ b/drivers/gpio/gpio-mvebu.c
> @@ -747,7 +747,7 @@ static int mvebu_pwm_probe(struct platform_device
> *pdev, set = U32_MAX;
>  	else
>  		return -EINVAL;
> -	writel_relaxed(0,
> mvebu_gpioreg_blink_counter_select(mvchip));
> +	writel_relaxed(set,
> mvebu_gpioreg_blink_counter_select(mvchip)); 
>  	mvpwm = devm_kzalloc(dev, sizeof(struct mvebu_pwm),
> GFP_KERNEL); if (!mvpwm)

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 2/2] gpio: mvebu: fix gpio bank registration when pwm is used
  2017-05-30 15:14         ` Ralph Sennhauser
  (?)
@ 2017-05-30 16:35           ` Richard Genoud
  -1 siblings, 0 replies; 21+ messages in thread
From: Richard Genoud @ 2017-05-30 16:35 UTC (permalink / raw)
  To: Ralph Sennhauser
  Cc: Gregory CLEMENT, Linus Walleij, Alexandre Courbot, Andrew Lunn,
	Jason Cooper, linux-arm-kernel, linux-gpio, linux-kernel,
	linux-pwm, Mark Rutland, Rob Herring, Russell King,
	Sebastian Hesselbarth, Thierry Reding

2017-05-30 17:14 GMT+02:00 Ralph Sennhauser <ralph.sennhauser@gmail.com>:
> Hi Richard
>
> On Tue, 30 May 2017 16:45:24 +0200
> Richard Genoud <richard.genoud@gmail.com> wrote:
>
>> 2017-05-30 15:16 GMT+02:00 Gregory CLEMENT
>> <gregory.clement@free-electrons.com>:
>> > Hi Richard,
>> Hi Greg !
>>
>> >
>> >  On mar., mai 30 2017, Richard Genoud <richard.genoud@gmail.com>
>> > wrote:
>> >> If more than one gpio bank has the "pwm" property, only one will be
>> >> registered successfully, all the others will fail with:
>> >> mvebu-gpio: probe of f1018140.gpio failed with error -17
>> >>
>> >> That's because in alloc_pwms(), the chip->base (aka "int pwm"),
>> >> was not set (thus, ==0) ; and 0 is a meaningful start value in
>> >> alloc_pwm(). What was intended is chip->base = -1.
>> >> Like that, the numbering will be done auto-magically
>> >>
>> >> Tested on clearfog-pro (Marvell 88F6828)
>> >>
>> >> Signed-off-by: Richard Genoud <richard.genoud@gmail.com>
>> >> ---
>> >>  drivers/gpio/gpio-mvebu.c | 1 +
>> >>  1 file changed, 1 insertion(+)
>> >>
>> >> diff --git a/drivers/gpio/gpio-mvebu.c b/drivers/gpio/gpio-mvebu.c
>> >> index cdef2c78cb3b..4734923e11fd 100644
>> >> --- a/drivers/gpio/gpio-mvebu.c
>> >> +++ b/drivers/gpio/gpio-mvebu.c
>> >> @@ -768,6 +768,7 @@ static int mvebu_pwm_probe(struct
>> >> platform_device *pdev, mvpwm->chip.dev = dev;
>> >>       mvpwm->chip.ops = &mvebu_pwm_ops;
>> >>       mvpwm->chip.npwm = mvchip->chip.ngpio;
>> >> +     mvpwm->chip.base = -1;
>> >
>> > Why not using
>> > mvpwm->chip.base = id * MVEBU_MAX_GPIO_PER_BANK;
>> > as it is done in the mvebu_gpio_probe() function?
>> Yes, that was my first move:
>> mvpwm->chip.base = mvchip->chip.base;
>>
>> But after some reflexion, mvpwm->chip.base is not the GPIO base, it's
>> the PWM base,
>> (mvpwm->chip is a struct pwm_chip), so it would we weird to have
>> "holes" in the declared PWMs.
>> I'm not clear, so here's an example:
>> If, in the DTS, we have:
>>             gpio0: gpio@18100 {
>>                 compatible = "marvell,armada-370-xp-gpio",
>>                          "marvell,orion-gpio";
>>                 reg = <0x18100 0x40>, <0x181c0 0x08>;
>>                 reg-names = "gpio";  /* "pwm" missing */
>> [...]
>>             gpio1: gpio@18140 {
>>                 compatible = "marvell,armada-370-xp-gpio",
>>                          "marvell,orion-gpio";
>>                 reg = <0x18140 0x40>, <0x181c8 0x08>;
>>                 reg-names = "gpio", "pwm";
>> In this case, if gpio0 is not declared as PWM capable, the PWM
>> numbering will start at 32 if we have
>> mvpwm->chip.base = mvchip->chip.base;
>> but it will start at 0 if we have mvpwm->chip.base = -1;
>>
>> The pros for having mvpwm->chip.base = mvchip->chip.base; is mainly
>> the stable numbering:
>> if we add the "pwm" feature to gpio0 afterwards, the pwm numbering in
>> sysfs will stay the same.
>> And if we have mvpwm->chip.base = -1; the pwm numbering will be
>> shifted.
>>
>> Looking back at the V5 of this patch
>> https://www.spinics.net/lists/kernel/msg2484889.html
>> There was the line:
>> mvpwm->chip.base = mvchip->chip.base;
>> I guess it got lost in the v6 rebase.
>>
>> So I could change it back, but I'm not sure which one is better.
>
> Thierry Redding pointed out that the region might already be occupied
> by another PWM chip, unlikely but not impossible. That's why it got
> changed for v6.
>
> See https://www.spinics.net/lists/devicetree/msg173138.html
Hum, I see.

But still, if mvpwm->chip.base is 0, we will have an error in alloc_pwms():

in pmvebu_pwm_probe(), mvpwm->chip.base is set to 0 (kzalloc), then
pwmchip_add(&mvpwm->chip) is called,
which will call alloc_pwms(chip->base/* == 0 */, chip->npwm);
The 1st time (1st gpio bank), it will be ok because:
start = bitmap_find_next_zero_area(allocated_pwms, MAX_PWMS, from, count, 0);
if (pwm >= 0 && start != pwm)
     return -EEXIST;
start is 0, pwm is 0, everything is good.
BUT, on the second time (for gpio1), we will have start = 32 and pwm=0
=> -EEXIST !

So there's something to fix.
And It seems that setting mvpwm->chip.base to -1 is a good choice
since it doesn't
force the numbering to something already taken.


Regards;
Richard.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 2/2] gpio: mvebu: fix gpio bank registration when pwm is used
@ 2017-05-30 16:35           ` Richard Genoud
  0 siblings, 0 replies; 21+ messages in thread
From: Richard Genoud @ 2017-05-30 16:35 UTC (permalink / raw)
  To: Ralph Sennhauser
  Cc: Gregory CLEMENT, Linus Walleij, Alexandre Courbot, Andrew Lunn,
	Jason Cooper, linux-arm-kernel, linux-gpio, linux-kernel,
	linux-pwm, Mark Rutland, Rob Herring, Russell King,
	Sebastian Hesselbarth, Thierry Reding

2017-05-30 17:14 GMT+02:00 Ralph Sennhauser <ralph.sennhauser@gmail.com>:
> Hi Richard
>
> On Tue, 30 May 2017 16:45:24 +0200
> Richard Genoud <richard.genoud@gmail.com> wrote:
>
>> 2017-05-30 15:16 GMT+02:00 Gregory CLEMENT
>> <gregory.clement@free-electrons.com>:
>> > Hi Richard,
>> Hi Greg !
>>
>> >
>> >  On mar., mai 30 2017, Richard Genoud <richard.genoud@gmail.com>
>> > wrote:
>> >> If more than one gpio bank has the "pwm" property, only one will be
>> >> registered successfully, all the others will fail with:
>> >> mvebu-gpio: probe of f1018140.gpio failed with error -17
>> >>
>> >> That's because in alloc_pwms(), the chip->base (aka "int pwm"),
>> >> was not set (thus, ==0) ; and 0 is a meaningful start value in
>> >> alloc_pwm(). What was intended is chip->base = -1.
>> >> Like that, the numbering will be done auto-magically
>> >>
>> >> Tested on clearfog-pro (Marvell 88F6828)
>> >>
>> >> Signed-off-by: Richard Genoud <richard.genoud@gmail.com>
>> >> ---
>> >>  drivers/gpio/gpio-mvebu.c | 1 +
>> >>  1 file changed, 1 insertion(+)
>> >>
>> >> diff --git a/drivers/gpio/gpio-mvebu.c b/drivers/gpio/gpio-mvebu.c
>> >> index cdef2c78cb3b..4734923e11fd 100644
>> >> --- a/drivers/gpio/gpio-mvebu.c
>> >> +++ b/drivers/gpio/gpio-mvebu.c
>> >> @@ -768,6 +768,7 @@ static int mvebu_pwm_probe(struct
>> >> platform_device *pdev, mvpwm->chip.dev = dev;
>> >>       mvpwm->chip.ops = &mvebu_pwm_ops;
>> >>       mvpwm->chip.npwm = mvchip->chip.ngpio;
>> >> +     mvpwm->chip.base = -1;
>> >
>> > Why not using
>> > mvpwm->chip.base = id * MVEBU_MAX_GPIO_PER_BANK;
>> > as it is done in the mvebu_gpio_probe() function?
>> Yes, that was my first move:
>> mvpwm->chip.base = mvchip->chip.base;
>>
>> But after some reflexion, mvpwm->chip.base is not the GPIO base, it's
>> the PWM base,
>> (mvpwm->chip is a struct pwm_chip), so it would we weird to have
>> "holes" in the declared PWMs.
>> I'm not clear, so here's an example:
>> If, in the DTS, we have:
>>             gpio0: gpio@18100 {
>>                 compatible = "marvell,armada-370-xp-gpio",
>>                          "marvell,orion-gpio";
>>                 reg = <0x18100 0x40>, <0x181c0 0x08>;
>>                 reg-names = "gpio";  /* "pwm" missing */
>> [...]
>>             gpio1: gpio@18140 {
>>                 compatible = "marvell,armada-370-xp-gpio",
>>                          "marvell,orion-gpio";
>>                 reg = <0x18140 0x40>, <0x181c8 0x08>;
>>                 reg-names = "gpio", "pwm";
>> In this case, if gpio0 is not declared as PWM capable, the PWM
>> numbering will start at 32 if we have
>> mvpwm->chip.base = mvchip->chip.base;
>> but it will start at 0 if we have mvpwm->chip.base = -1;
>>
>> The pros for having mvpwm->chip.base = mvchip->chip.base; is mainly
>> the stable numbering:
>> if we add the "pwm" feature to gpio0 afterwards, the pwm numbering in
>> sysfs will stay the same.
>> And if we have mvpwm->chip.base = -1; the pwm numbering will be
>> shifted.
>>
>> Looking back at the V5 of this patch
>> https://www.spinics.net/lists/kernel/msg2484889.html
>> There was the line:
>> mvpwm->chip.base = mvchip->chip.base;
>> I guess it got lost in the v6 rebase.
>>
>> So I could change it back, but I'm not sure which one is better.
>
> Thierry Redding pointed out that the region might already be occupied
> by another PWM chip, unlikely but not impossible. That's why it got
> changed for v6.
>
> See https://www.spinics.net/lists/devicetree/msg173138.html
Hum, I see.

But still, if mvpwm->chip.base is 0, we will have an error in alloc_pwms():

in pmvebu_pwm_probe(), mvpwm->chip.base is set to 0 (kzalloc), then
pwmchip_add(&mvpwm->chip) is called,
which will call alloc_pwms(chip->base/* == 0 */, chip->npwm);
The 1st time (1st gpio bank), it will be ok because:
start = bitmap_find_next_zero_area(allocated_pwms, MAX_PWMS, from, count, 0);
if (pwm >= 0 && start != pwm)
     return -EEXIST;
start is 0, pwm is 0, everything is good.
BUT, on the second time (for gpio1), we will have start = 32 and pwm=0
=> -EEXIST !

So there's something to fix.
And It seems that setting mvpwm->chip.base to -1 is a good choice
since it doesn't
force the numbering to something already taken.


Regards;
Richard.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* [PATCH 2/2] gpio: mvebu: fix gpio bank registration when pwm is used
@ 2017-05-30 16:35           ` Richard Genoud
  0 siblings, 0 replies; 21+ messages in thread
From: Richard Genoud @ 2017-05-30 16:35 UTC (permalink / raw)
  To: linux-arm-kernel

2017-05-30 17:14 GMT+02:00 Ralph Sennhauser <ralph.sennhauser@gmail.com>:
> Hi Richard
>
> On Tue, 30 May 2017 16:45:24 +0200
> Richard Genoud <richard.genoud@gmail.com> wrote:
>
>> 2017-05-30 15:16 GMT+02:00 Gregory CLEMENT
>> <gregory.clement@free-electrons.com>:
>> > Hi Richard,
>> Hi Greg !
>>
>> >
>> >  On mar., mai 30 2017, Richard Genoud <richard.genoud@gmail.com>
>> > wrote:
>> >> If more than one gpio bank has the "pwm" property, only one will be
>> >> registered successfully, all the others will fail with:
>> >> mvebu-gpio: probe of f1018140.gpio failed with error -17
>> >>
>> >> That's because in alloc_pwms(), the chip->base (aka "int pwm"),
>> >> was not set (thus, ==0) ; and 0 is a meaningful start value in
>> >> alloc_pwm(). What was intended is chip->base = -1.
>> >> Like that, the numbering will be done auto-magically
>> >>
>> >> Tested on clearfog-pro (Marvell 88F6828)
>> >>
>> >> Signed-off-by: Richard Genoud <richard.genoud@gmail.com>
>> >> ---
>> >>  drivers/gpio/gpio-mvebu.c | 1 +
>> >>  1 file changed, 1 insertion(+)
>> >>
>> >> diff --git a/drivers/gpio/gpio-mvebu.c b/drivers/gpio/gpio-mvebu.c
>> >> index cdef2c78cb3b..4734923e11fd 100644
>> >> --- a/drivers/gpio/gpio-mvebu.c
>> >> +++ b/drivers/gpio/gpio-mvebu.c
>> >> @@ -768,6 +768,7 @@ static int mvebu_pwm_probe(struct
>> >> platform_device *pdev, mvpwm->chip.dev = dev;
>> >>       mvpwm->chip.ops = &mvebu_pwm_ops;
>> >>       mvpwm->chip.npwm = mvchip->chip.ngpio;
>> >> +     mvpwm->chip.base = -1;
>> >
>> > Why not using
>> > mvpwm->chip.base = id * MVEBU_MAX_GPIO_PER_BANK;
>> > as it is done in the mvebu_gpio_probe() function?
>> Yes, that was my first move:
>> mvpwm->chip.base = mvchip->chip.base;
>>
>> But after some reflexion, mvpwm->chip.base is not the GPIO base, it's
>> the PWM base,
>> (mvpwm->chip is a struct pwm_chip), so it would we weird to have
>> "holes" in the declared PWMs.
>> I'm not clear, so here's an example:
>> If, in the DTS, we have:
>>             gpio0: gpio at 18100 {
>>                 compatible = "marvell,armada-370-xp-gpio",
>>                          "marvell,orion-gpio";
>>                 reg = <0x18100 0x40>, <0x181c0 0x08>;
>>                 reg-names = "gpio";  /* "pwm" missing */
>> [...]
>>             gpio1: gpio at 18140 {
>>                 compatible = "marvell,armada-370-xp-gpio",
>>                          "marvell,orion-gpio";
>>                 reg = <0x18140 0x40>, <0x181c8 0x08>;
>>                 reg-names = "gpio", "pwm";
>> In this case, if gpio0 is not declared as PWM capable, the PWM
>> numbering will start at 32 if we have
>> mvpwm->chip.base = mvchip->chip.base;
>> but it will start at 0 if we have mvpwm->chip.base = -1;
>>
>> The pros for having mvpwm->chip.base = mvchip->chip.base; is mainly
>> the stable numbering:
>> if we add the "pwm" feature to gpio0 afterwards, the pwm numbering in
>> sysfs will stay the same.
>> And if we have mvpwm->chip.base = -1; the pwm numbering will be
>> shifted.
>>
>> Looking back at the V5 of this patch
>> https://www.spinics.net/lists/kernel/msg2484889.html
>> There was the line:
>> mvpwm->chip.base = mvchip->chip.base;
>> I guess it got lost in the v6 rebase.
>>
>> So I could change it back, but I'm not sure which one is better.
>
> Thierry Redding pointed out that the region might already be occupied
> by another PWM chip, unlikely but not impossible. That's why it got
> changed for v6.
>
> See https://www.spinics.net/lists/devicetree/msg173138.html
Hum, I see.

But still, if mvpwm->chip.base is 0, we will have an error in alloc_pwms():

in pmvebu_pwm_probe(), mvpwm->chip.base is set to 0 (kzalloc), then
pwmchip_add(&mvpwm->chip) is called,
which will call alloc_pwms(chip->base/* == 0 */, chip->npwm);
The 1st time (1st gpio bank), it will be ok because:
start = bitmap_find_next_zero_area(allocated_pwms, MAX_PWMS, from, count, 0);
if (pwm >= 0 && start != pwm)
     return -EEXIST;
start is 0, pwm is 0, everything is good.
BUT, on the second time (for gpio1), we will have start = 32 and pwm=0
=> -EEXIST !

So there's something to fix.
And It seems that setting mvpwm->chip.base to -1 is a good choice
since it doesn't
force the numbering to something already taken.


Regards;
Richard.

^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2017-05-30 16:35 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-05-30 12:28 [PATCH 1/2] gpio: mvebu: fix blink counter register selection Richard Genoud
2017-05-30 12:28 ` Richard Genoud
2017-05-30 12:28 ` [PATCH 2/2] gpio: mvebu: fix gpio bank registration when pwm is used Richard Genoud
2017-05-30 12:28   ` Richard Genoud
2017-05-30 13:16   ` Gregory CLEMENT
2017-05-30 13:16     ` Gregory CLEMENT
2017-05-30 14:45     ` Richard Genoud
2017-05-30 14:45       ` Richard Genoud
2017-05-30 14:45       ` Richard Genoud
2017-05-30 15:14       ` Ralph Sennhauser
2017-05-30 15:14         ` Ralph Sennhauser
2017-05-30 15:14         ` Ralph Sennhauser
2017-05-30 16:35         ` Richard Genoud
2017-05-30 16:35           ` Richard Genoud
2017-05-30 16:35           ` Richard Genoud
2017-05-30 13:01 ` [PATCH 1/2] gpio: mvebu: fix blink counter register selection Gregory CLEMENT
2017-05-30 13:01   ` Gregory CLEMENT
2017-05-30 13:06   ` Gregory CLEMENT
2017-05-30 13:06     ` Gregory CLEMENT
2017-05-30 15:18 ` Ralph Sennhauser
2017-05-30 15:18   ` Ralph Sennhauser

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.