[PATCH] gpio: davinci: fix gpio selection for OF

[PATCH] gpio: davinci: fix gpio selection for OF

[Alexander Holler]

The driver missed an of_xlate function to translate gpio numbers
as found in the DT to the correct chip and number.

While there I've set #gpio_cells to a fixed value of 2.

I've used gpio-pxa.c as template for those changes and tested my changes
successfully on a da850 board using entries for gpio-leds in a DT. So I didn't
reinvent the wheel but just copied and tested stuff.

Thanks to Grygorii Strashko for the hint of the existing code in gpio-pxa.

Signed-off-by: Alexander Holler <holler@ahsoftware.de>
---
 drivers/gpio/gpio-davinci.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/drivers/gpio/gpio-davinci.c b/drivers/gpio/gpio-davinci.c
index 7629b4f..79f45c4 100644
--- a/drivers/gpio/gpio-davinci.c
+++ b/drivers/gpio/gpio-davinci.c
@@ -44,6 +44,9 @@ struct davinci_gpio_regs {
 
 static void __iomem *gpio_base;
 
+static struct davinci_gpio_controller *davinci_gpio_chips;
+static int davinci_last_gpio;
+
 static struct davinci_gpio_regs __iomem *gpio2regs(unsigned gpio)
 {
 	void __iomem *ptr;
@@ -172,6 +175,24 @@ of_err:
 	return NULL;
 }
 
+#ifdef CONFIG_OF_GPIO
+static int davinci_gpio_of_xlate(struct gpio_chip *gc,
+			     const struct of_phandle_args *gpiospec,
+			     u32 *flags)
+{
+	if (gpiospec->args[0] > davinci_last_gpio)
+		return -EINVAL;
+
+	if (gc != &davinci_gpio_chips[gpiospec->args[0] / 32].chip)
+		return -EINVAL;
+
+	if (flags)
+		*flags = gpiospec->args[1];
+
+	return gpiospec->args[0] % 32;
+}
+#endif
+
 static int davinci_gpio_probe(struct platform_device *pdev)
 {
 	int i, base;
@@ -236,6 +257,8 @@ static int davinci_gpio_probe(struct platform_device *pdev)
 			chips[i].chip.ngpio = 32;
 
 #ifdef CONFIG_OF_GPIO
+		chips[i].chip.of_gpio_n_cells = 2;
+		chips[i].chip.of_xlate = davinci_gpio_of_xlate;
 		chips[i].chip.of_node = dev->of_node;
 #endif
 		spin_lock_init(&chips[i].lock);
@@ -251,6 +274,10 @@ static int davinci_gpio_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, chips);
 	davinci_gpio_irq_setup(pdev);
+
+	davinci_gpio_chips = chips;
+	davinci_last_gpio = ngpio;
+
 	return 0;
 }
 
-- 
1.8.3.1

[PATCH] gpio: davinci: fix gpio selection for OF

[Linus Walleij]

On Wed, Mar 5, 2014 at 6:06 AM, Alexander Holler <holler@ahsoftware.de> wrote:

> The driver missed an of_xlate function to translate gpio numbers
> as found in the DT to the correct chip and number.
>
> While there I've set #gpio_cells to a fixed value of 2.
>
> I've used gpio-pxa.c as template for those changes and tested my changes
> successfully on a da850 board using entries for gpio-leds in a DT. So I didn't
> reinvent the wheel but just copied and tested stuff.
>
> Thanks to Grygorii Strashko for the hint of the existing code in gpio-pxa.
>
> Signed-off-by: Alexander Holler <holler@ahsoftware.de>

Grygorii, can I have your review tag on this patch so I can apply it?

Yours,
Linus Walleij

[PATCH] gpio: davinci: fix gpio selection for OF

[Grygorii Strashko]

On 03/05/2014 12:06 AM, Alexander Holler wrote:
> The driver missed an of_xlate function to translate gpio numbers
> as found in the DT to the correct chip and number.
>
> While there I've set #gpio_cells to a fixed value of 2.
>
> I've used gpio-pxa.c as template for those changes and tested my changes
> successfully on a da850 board using entries for gpio-leds in a DT. So I didn't
> reinvent the wheel but just copied and tested stuff.
>
> Thanks to Grygorii Strashko for the hint of the existing code in gpio-pxa.
>
> Signed-off-by: Alexander Holler <holler@ahsoftware.de>
> ---
>   drivers/gpio/gpio-davinci.c | 27 +++++++++++++++++++++++++++
>   1 file changed, 27 insertions(+)
>
> diff --git a/drivers/gpio/gpio-davinci.c b/drivers/gpio/gpio-davinci.c
> index 7629b4f..79f45c4 100644
> --- a/drivers/gpio/gpio-davinci.c
> +++ b/drivers/gpio/gpio-davinci.c
> @@ -44,6 +44,9 @@ struct davinci_gpio_regs {
>
>   static void __iomem *gpio_base;
>
> +static struct davinci_gpio_controller *davinci_gpio_chips;
> +static int davinci_last_gpio;
> +
>   static struct davinci_gpio_regs __iomem *gpio2regs(unsigned gpio)
>   {
>   	void __iomem *ptr;
> @@ -172,6 +175,24 @@ of_err:
>   	return NULL;
>   }
>
> +#ifdef CONFIG_OF_GPIO
> +static int davinci_gpio_of_xlate(struct gpio_chip *gc,
> +			     const struct of_phandle_args *gpiospec,
> +			     u32 *flags)
> +{
> +	if (gpiospec->args[0] > davinci_last_gpio)
> +		return -EINVAL;
> +
> +	if (gc != &davinci_gpio_chips[gpiospec->args[0] / 32].chip)
> +		return -EINVAL;
> +
> +	if (flags)
> +		*flags = gpiospec->args[1];

Just one question here - Could we use gpio_chip-dev and hence, drop
static variables (davinci_gpio_chips, davinci_last_gpio)?

The Davinci device holds davinci_gpio_platform_data
in dev->platform_data and pointer on davinci_gpio_controller array
in dev->p->driver_data.

And looks like, dev_get_platdata(gc->dev) and
dev_get_drvdata(gc->dev) can be used here.

> +
> +	return gpiospec->args[0] % 32;
> +}
> +#endif
> +
>   static int davinci_gpio_probe(struct platform_device *pdev)
>   {
>   	int i, base;
> @@ -236,6 +257,8 @@ static int davinci_gpio_probe(struct platform_device *pdev)
>   			chips[i].chip.ngpio = 32;
>

add here	chips[i].chip.dev = dev;

>   #ifdef CONFIG_OF_GPIO
> +		chips[i].chip.of_gpio_n_cells = 2;
> +		chips[i].chip.of_xlate = davinci_gpio_of_xlate;
>   		chips[i].chip.of_node = dev->of_node;
>   #endif
>   		spin_lock_init(&chips[i].lock);
> @@ -251,6 +274,10 @@ static int davinci_gpio_probe(struct platform_device *pdev)
>
>   	platform_set_drvdata(pdev, chips);
>   	davinci_gpio_irq_setup(pdev);
> +
> +	davinci_gpio_chips = chips;
> +	davinci_last_gpio = ngpio;
> +
>   	return 0;
>   }

regards,
-grygorii

[PATCH v2] gpio: davinci: fix gpio selection for OF

[Alexander Holler]

The driver missed an of_xlate function to translate gpio numbers
as found in the DT to the correct chip and number.

While there I've set #gpio_cells to a fixed value of 2.

I've used gpio-pxa.c as template for those changes and tested my changes
successfully on a da850 board using entries for gpio-leds in a DT. So I didn't
reinvent the wheel but just copied and tested stuff.

Thanks to Grygorii Strashko for the hint to the existing code in gpio-pxa.

Signed-off-by: Alexander Holler <holler@ahsoftware.de>
Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
 drivers/gpio/gpio-davinci.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

 Changes in v2: replaced static variables by indirections.

diff --git a/drivers/gpio/gpio-davinci.c b/drivers/gpio/gpio-davinci.c
index 7629b4f..92574a0 100644
--- a/drivers/gpio/gpio-davinci.c
+++ b/drivers/gpio/gpio-davinci.c
@@ -172,6 +172,27 @@ of_err:
 	return NULL;
 }
 
+#ifdef CONFIG_OF_GPIO
+static int davinci_gpio_of_xlate(struct gpio_chip *gc,
+			     const struct of_phandle_args *gpiospec,
+			     u32 *flags)
+{
+	struct davinci_gpio_controller *chips = dev_get_drvdata(gc->dev);
+	struct davinci_gpio_platform_data *pdata = dev_get_platdata(gc->dev);
+
+	if (gpiospec->args[0] > pdata->ngpio)
+		return -EINVAL;
+
+	if (gc != &chips[gpiospec->args[0] / 32].chip)
+		return -EINVAL;
+
+	if (flags)
+		*flags = gpiospec->args[1];
+
+	return gpiospec->args[0] % 32;
+}
+#endif
+
 static int davinci_gpio_probe(struct platform_device *pdev)
 {
 	int i, base;
@@ -236,6 +257,9 @@ static int davinci_gpio_probe(struct platform_device *pdev)
 			chips[i].chip.ngpio = 32;
 
 #ifdef CONFIG_OF_GPIO
+		chips[i].chip.of_gpio_n_cells = 2;
+		chips[i].chip.of_xlate = davinci_gpio_of_xlate;
+		chips[i].chip.dev = dev;
 		chips[i].chip.of_node = dev->of_node;
 #endif
 		spin_lock_init(&chips[i].lock);
-- 
1.8.3.1

[PATCH v2] gpio: davinci: fix gpio selection for OF

[Grygorii Strashko]

Hi Alexander,

On 03/05/2014 01:21 PM, Alexander Holler wrote:
> The driver missed an of_xlate function to translate gpio numbers
> as found in the DT to the correct chip and number.
>
> While there I've set #gpio_cells to a fixed value of 2.
>
> I've used gpio-pxa.c as template for those changes and tested my changes
> successfully on a da850 board using entries for gpio-leds in a DT. So I didn't
> reinvent the wheel but just copied and tested stuff.
>
> Thanks to Grygorii Strashko for the hint to the existing code in gpio-pxa.
>
> Signed-off-by: Alexander Holler <holler@ahsoftware.de>
> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>

Looks good to me now. Thanks.

> ---
>   drivers/gpio/gpio-davinci.c | 24 ++++++++++++++++++++++++
>   1 file changed, 24 insertions(+)
>
>   Changes in v2: replaced static variables by indirections.
>
> diff --git a/drivers/gpio/gpio-davinci.c b/drivers/gpio/gpio-davinci.c
> index 7629b4f..92574a0 100644
> --- a/drivers/gpio/gpio-davinci.c
> +++ b/drivers/gpio/gpio-davinci.c
> @@ -172,6 +172,27 @@ of_err:
>   	return NULL;
>   }
>
> +#ifdef CONFIG_OF_GPIO
> +static int davinci_gpio_of_xlate(struct gpio_chip *gc,
> +			     const struct of_phandle_args *gpiospec,
> +			     u32 *flags)
> +{
> +	struct davinci_gpio_controller *chips = dev_get_drvdata(gc->dev);
> +	struct davinci_gpio_platform_data *pdata = dev_get_platdata(gc->dev);
> +
> +	if (gpiospec->args[0] > pdata->ngpio)
> +		return -EINVAL;
> +
> +	if (gc != &chips[gpiospec->args[0] / 32].chip)
> +		return -EINVAL;
> +
> +	if (flags)
> +		*flags = gpiospec->args[1];
> +
> +	return gpiospec->args[0] % 32;
> +}
> +#endif
> +
>   static int davinci_gpio_probe(struct platform_device *pdev)
>   {
>   	int i, base;
> @@ -236,6 +257,9 @@ static int davinci_gpio_probe(struct platform_device *pdev)
>   			chips[i].chip.ngpio = 32;
>
>   #ifdef CONFIG_OF_GPIO
> +		chips[i].chip.of_gpio_n_cells = 2;
> +		chips[i].chip.of_xlate = davinci_gpio_of_xlate;
> +		chips[i].chip.dev = dev;
>   		chips[i].chip.of_node = dev->of_node;
>   #endif
>   		spin_lock_init(&chips[i].lock);
>

[PATCH v2] gpio: davinci: fix gpio selection for OF

[Linus Walleij]

On Wed, Mar 5, 2014 at 12:21 PM, Alexander Holler <holler@ahsoftware.de> wrote:

> The driver missed an of_xlate function to translate gpio numbers
> as found in the DT to the correct chip and number.
>
> While there I've set #gpio_cells to a fixed value of 2.
>
> I've used gpio-pxa.c as template for those changes and tested my changes
> successfully on a da850 board using entries for gpio-leds in a DT. So I didn't
> reinvent the wheel but just copied and tested stuff.
>
> Thanks to Grygorii Strashko for the hint to the existing code in gpio-pxa.
>
> Signed-off-by: Alexander Holler <holler@ahsoftware.de>
> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>

This v2 version applied, thanks!

Yours,
Linus Walleij

[PATCH v2] gpio: davinci: fix gpio selection for OF

[Alexander Holler]

Am 11.03.2014 11:15, schrieb Linus Walleij:
> On Wed, Mar 5, 2014 at 12:21 PM, Alexander Holler <holler@ahsoftware.de> wrote:
> 
>> The driver missed an of_xlate function to translate gpio numbers
>> as found in the DT to the correct chip and number.
>>
>> While there I've set #gpio_cells to a fixed value of 2.
>>
>> I've used gpio-pxa.c as template for those changes and tested my changes
>> successfully on a da850 board using entries for gpio-leds in a DT. So I didn't
>> reinvent the wheel but just copied and tested stuff.
>>
>> Thanks to Grygorii Strashko for the hint to the existing code in gpio-pxa.
>>
>> Signed-off-by: Alexander Holler <holler@ahsoftware.de>
>> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
> 
> This v2 version applied, thanks!

Thanks, but actually that should have been a fix for 3.14 with which the
OF functionality for davinci gpio gets introduced. I assum with the
patch in for-next, 3.14 will appear with that functionality broken and
it will become a candidate for -stable.

Regards,

Alexander Holler

[PATCH v2] gpio: davinci: fix gpio selection for OF

[Linus Walleij]

On Fri, Mar 14, 2014 at 11:08 AM, Alexander Holler <holler@ahsoftware.de> wrote:
> Am 11.03.2014 11:15, schrieb Linus Walleij:
>> On Wed, Mar 5, 2014 at 12:21 PM, Alexander Holler <holler@ahsoftware.de> wrote:
>>
>>> The driver missed an of_xlate function to translate gpio numbers
>>> as found in the DT to the correct chip and number.
>>>
>>> While there I've set #gpio_cells to a fixed value of 2.
>>>
>>> I've used gpio-pxa.c as template for those changes and tested my changes
>>> successfully on a da850 board using entries for gpio-leds in a DT. So I didn't
>>> reinvent the wheel but just copied and tested stuff.
>>>
>>> Thanks to Grygorii Strashko for the hint to the existing code in gpio-pxa.
>>>
>>> Signed-off-by: Alexander Holler <holler@ahsoftware.de>
>>> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
>>
>> This v2 version applied, thanks!
>
> Thanks, but actually that should have been a fix for 3.14 with which the
> OF functionality for davinci gpio gets introduced. I assum with the
> patch in for-next, 3.14 will appear with that functionality broken and
> it will become a candidate for -stable.

I just get the impression that DT support for DaVinci in v3.14 is so risky
and unstable that noone except those implementing it (i.e. you) is really
using it, is that correct?

In that case it is hardly a fix that we need to rush out to the entire world.

But if you have bug reports from DaVinci developers doing advanced DT
stuff and scratching their heads, then we can push this to fixes.

Yours,
Linus Walleij

[PATCH v2] gpio: davinci: fix gpio selection for OF

[Alexander Holler]

Am 14.03.2014 12:02, schrieb Linus Walleij:
> On Fri, Mar 14, 2014 at 11:08 AM, Alexander Holler <holler@ahsoftware.de> wrote:
>> Am 11.03.2014 11:15, schrieb Linus Walleij:
>>> On Wed, Mar 5, 2014 at 12:21 PM, Alexander Holler <holler@ahsoftware.de> wrote:
>>>
>>>> The driver missed an of_xlate function to translate gpio numbers
>>>> as found in the DT to the correct chip and number.
>>>>
>>>> While there I've set #gpio_cells to a fixed value of 2.
>>>>
>>>> I've used gpio-pxa.c as template for those changes and tested my changes
>>>> successfully on a da850 board using entries for gpio-leds in a DT. So I didn't
>>>> reinvent the wheel but just copied and tested stuff.
>>>>
>>>> Thanks to Grygorii Strashko for the hint to the existing code in gpio-pxa.
>>>>
>>>> Signed-off-by: Alexander Holler <holler@ahsoftware.de>
>>>> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
>>>
>>> This v2 version applied, thanks!
>>
>> Thanks, but actually that should have been a fix for 3.14 with which the
>> OF functionality for davinci gpio gets introduced. I assum with the
>> patch in for-next, 3.14 will appear with that functionality broken and
>> it will become a candidate for -stable.
>
> I just get the impression that DT support for DaVinci in v3.14 is so risky
> and unstable that noone except those implementing it (i.e. you) is really
> using it, is that correct?

Yes. DT support for DaVinci is still incomplete and I don't think there 
are much people out there which already try to use it. Nevertheless is 
using DT a much easier way (and more future-proof) than patching 
board-files, if a board isn't one of those few supported dev-boards and 
needs different configurations.

> In that case it is hardly a fix that we need to rush out to the entire world.

Hmm, I think it's better to send something working and tested to world, 
than something untested and non-working.

And especially things like pinctrl and gpio are fundamental to even try 
to use DT. E.g. in my case I didn't even try to use DT for DaVinci 
because it doesn't make sense without support for DT in gpio. And now 
people will get promised but broken support for gpio in 3.14, which 
might cost a lot of time, if someone tries to use it.

And I thought the reason for -rc is actually to fix bugs. But I never 
understood the magical ways and timings patches make their way into 
mainline. ;)

Regards,

Alexander Holler

[PATCH v2] gpio: davinci: fix gpio selection for OF

[Linus Walleij]

On Fri, Mar 14, 2014 at 1:38 PM, Alexander Holler <holler@ahsoftware.de> wrote:

>> In that case it is hardly a fix that we need to rush out to the entire
>> world.
>
> And I thought the reason for -rc is actually to fix bugs. But I never
> understood the magical ways and timings patches make their way into
> mainline. ;)

OK so it works like this: early in the -rc cycle we fix any bugs, documentation
or whatever. At this point it's *regressions* so the fix need to fix something
that broke in the merge window (or an earlier merge window).

If it is a new feature that never worked in the first place I would
not call that
a regression. There are no existing users out there that can experience
regressions from a previously working system.

So this is why I'm a bit conservative.

Yours,
Linus Walleij

[PATCH v2] gpio: davinci: fix gpio selection for OF

[Alexander Holler]

Am 14.03.2014 14:54, schrieb Linus Walleij:
> On Fri, Mar 14, 2014 at 1:38 PM, Alexander Holler <holler@ahsoftware.de> wrote:
>
>>> In that case it is hardly a fix that we need to rush out to the entire
>>> world.
>>
>> And I thought the reason for -rc is actually to fix bugs. But I never
>> understood the magical ways and timings patches make their way into
>> mainline. ;)
>
> OK so it works like this: early in the -rc cycle we fix any bugs, documentation
> or whatever. At this point it's *regressions* so the fix need to fix something
> that broke in the merge window (or an earlier merge window).

Sorry, but I don't believe that. It's always time to fix regressions and 
bugs.

> If it is a new feature that never worked in the first place I would
> not call that
> a regression. There are no existing users out there that can experience
> regressions from a previously working system.

I believe that no (stable) kernel should introduce new known bugs (if 
fixes are available) and I wouldn't make a difference if it's a new 
feature or not. Usually people don't care if something is new or not if 
it has bugs

Anyway, I'm just curious and try to understand, I have a fix. ;)

Regards,

Alexander Holler

[PATCH v2] gpio: davinci: fix gpio selection for OF

[Linus Walleij]

On Fri, Mar 14, 2014 at 7:33 PM, Alexander Holler <holler@ahsoftware.de> wrote:
> Am 14.03.2014 14:54, schrieb Linus Walleij:
>
>> On Fri, Mar 14, 2014 at 1:38 PM, Alexander Holler <holler@ahsoftware.de>
>> wrote:
>>
>>>> In that case it is hardly a fix that we need to rush out to the entire
>>>> world.
>>>
>>>
>>> And I thought the reason for -rc is actually to fix bugs. But I never
>>> understood the magical ways and timings patches make their way into
>>> mainline. ;)
>>
>>
>> OK so it works like this: early in the -rc cycle we fix any bugs,
>> documentation
>> or whatever. At this point it's *regressions* so the fix need to fix
>> something
>> that broke in the merge window (or an earlier merge window).
>
>
> Sorry, but I don't believe that. It's always time to fix regressions and
> bugs.

This thing is not binary. Please consult
Documentation/development-process/2.Process

In this case we are very late in the release cycle, the patch is not
a simple oneliner and I need some more backing proof to know it
is a reasonable fix to push at this time and will not cause new
problems.

So a few Tested-by's from the people using this driver would for
example convince me that it is solving a real problem for them
and it needs to go into fixes.

Yours,
Linus Walleij

[PATCH v2] gpio: davinci: fix gpio selection for OF

[Alexander Holler]

Am 14.03.2014 20:52, schrieb Linus Walleij:

> So a few Tested-by's from the people using this driver would for
> example convince me that it is solving a real problem for them
> and it needs to go into fixes.

2001: a space odyssey is fast action movie compared with the movie 
kernel bug fixing. And 2001 is a masterpiece of slowness.

Regards,

Alexander Holler

[PATCH v2] gpio: davinci: fix gpio selection for OF

[Sekhar Nori]

On Friday 14 March 2014 04:32 PM, Linus Walleij wrote:
> On Fri, Mar 14, 2014 at 11:08 AM, Alexander Holler <holler@ahsoftware.de> wrote:
>> Am 11.03.2014 11:15, schrieb Linus Walleij:
>>> On Wed, Mar 5, 2014 at 12:21 PM, Alexander Holler <holler@ahsoftware.de> wrote:
>>>
>>>> The driver missed an of_xlate function to translate gpio numbers
>>>> as found in the DT to the correct chip and number.
>>>>
>>>> While there I've set #gpio_cells to a fixed value of 2.
>>>>
>>>> I've used gpio-pxa.c as template for those changes and tested my changes
>>>> successfully on a da850 board using entries for gpio-leds in a DT. So I didn't
>>>> reinvent the wheel but just copied and tested stuff.
>>>>
>>>> Thanks to Grygorii Strashko for the hint to the existing code in gpio-pxa.
>>>>
>>>> Signed-off-by: Alexander Holler <holler@ahsoftware.de>
>>>> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
>>>
>>> This v2 version applied, thanks!
>>
>> Thanks, but actually that should have been a fix for 3.14 with which the
>> OF functionality for davinci gpio gets introduced. I assum with the
>> patch in for-next, 3.14 will appear with that functionality broken and
>> it will become a candidate for -stable.
> 
> I just get the impression that DT support for DaVinci in v3.14 is so risky
> and unstable that noone except those implementing it (i.e. you) is really
> using it, is that correct?
> 
> In that case it is hardly a fix that we need to rush out to the entire world.

One thing to note is that this driver is used by keystone too and all
its users are DT-only. Although I do not see any in-kernel DT GPIO users
even there.

I can confirm the patch does not break my gpiolib based test module
(test with and without DT), but then it did not have an issue even before.

Thanks,
Sekhar

[PATCH v2] gpio: davinci: fix gpio selection for OF

[Linus Walleij]

On Mon, Mar 17, 2014 at 2:29 PM, Sekhar Nori <nsekhar@ti.com> wrote:

> One thing to note is that this driver is used by keystone too and all
> its users are DT-only. Although I do not see any in-kernel DT GPIO users
> even there.
>
> I can confirm the patch does not break my gpiolib based test module
> (test with and without DT), but then it did not have an issue even before.

Is that a Tested-by tag? :-)

If you're also convinced that fix is safe I'll push it as a fix to v3.14-rcN
if for nothing else so for getting Mr. Holler to stop poking me in the
chest.

Yours,
Linus Walleij

[PATCH v2] gpio: davinci: fix gpio selection for OF

[Sekhar Nori]

On Monday 17 March 2014 07:35 PM, Linus Walleij wrote:
> On Mon, Mar 17, 2014 at 2:29 PM, Sekhar Nori <nsekhar@ti.com> wrote:
> 
>> One thing to note is that this driver is used by keystone too and all
>> its users are DT-only. Although I do not see any in-kernel DT GPIO users
>> even there.
>>
>> I can confirm the patch does not break my gpiolib based test module
>> (test with and without DT), but then it did not have an issue even before.
> 
> Is that a Tested-by tag? :-)

Yes.

Tested-by: Sekhar Nori <nsekhar@ti.com>

> 
> If you're also convinced that fix is safe I'll push it as a fix to v3.14-rcN
> if for nothing else so for getting Mr. Holler to stop poking me in the
> chest.

It is safe - at the least it does not break anything that is already
working. I guess the decision to put it into -rc depends on whether you
consider out of tree dtbs to be a valid usecase for the kernel.

Thanks,
Sekhar

[PATCH v2] gpio: davinci: fix gpio selection for OF

[Alexander Holler]

Am 18.03.2014 09:37, schrieb Sekhar Nori:

> It is safe - at the least it does not break anything that is already
> working. I guess the decision to put it into -rc depends on whether you
> consider out of tree dtbs to be a valid usecase for the kernel.

That's all DT is about, getting rid of the necessity for in-tree 
hw-descriptions. ;)

But I don't need any rush here, I'm just unable to understand why the 
-rc phase isn't used for bug fixing as I believe that's what this phase 
is for.

Most people are unable to track the various -next trees, therefor many 
failures only come up when stuff reaches mainline where it might be 
tested by some more people. And if all bugs found in the -rc phase are 
fixed in -next only, the -rc phase would be useless.

And it just happened to me too often, that even silly bugfixes (like 
oneliners) needed between 5 and 9 months to reach a stable kernel and 
thus users.

Regards,

Alexander Holler

[PATCH v2] gpio: davinci: fix gpio selection for OF

[Sekhar Nori]

Hi Alexander,

On Tuesday 18 March 2014 03:15 PM, Alexander Holler wrote:
> Am 18.03.2014 09:37, schrieb Sekhar Nori:
> 
>> It is safe - at the least it does not break anything that is already
>> working. I guess the decision to put it into -rc depends on whether you
>> consider out of tree dtbs to be a valid usecase for the kernel.
> 
> That's all DT is about, getting rid of the necessity for in-tree
> hw-descriptions. ;)
> 
> But I don't need any rush here, I'm just unable to understand why the
> -rc phase isn't used for bug fixing as I believe that's what this phase
> is for.

The push back you are seeing is because this is pretty late in -rc
cycle. If this push back was not there the bug fix cycle would probably
never close.

In all probability, if this was -rc2 or even -rc3 there would not be so
much discussion.

Thanks,
Sekhar

[PATCH v2] gpio: davinci: fix gpio selection for OF

[Linus Walleij]

On Tue, Mar 18, 2014 at 10:45 AM, Alexander Holler <holler@ahsoftware.de> wrote:

> But I don't need any rush here, I'm just unable to understand why the -rc
> phase isn't used for bug fixing as I believe that's what this phase is for.

Right now it is mostly a practical issue to me, as I applied the patch to
the devel (for-next) branch, then committed new development on top of
it.

If I send it for fixes now the same patch will come in two ways as I
really do not like to rebase my tree at this point.

So I'd prefer to keep this for next and then have it tagged for stable as
v3.14 is released, if that is OK?

It's as simple as sending a mail to Greg once it's upstream.

Yours,
Linus Walleij

[PATCH v2] gpio: davinci: fix gpio selection for OF

[Alexander Holler]

Am 21.03.2014 09:28, schrieb Linus Walleij:
> On Tue, Mar 18, 2014 at 10:45 AM, Alexander Holler <holler@ahsoftware.de> wrote:
>
>> But I don't need any rush here, I'm just unable to understand why the -rc
>> phase isn't used for bug fixing as I believe that's what this phase is for.
>
> Right now it is mostly a practical issue to me, as I applied the patch to
> the devel (for-next) branch, then committed new development on top of
> it.
>
> If I send it for fixes now the same patch will come in two ways as I
> really do not like to rebase my tree at this point.
>
> So I'd prefer to keep this for next and then have it tagged for stable as
> v3.14 is released, if that is OK?

Sure.

> It's as simple as sending a mail to Greg once it's upstream.

I already though about how to tag every patch I send as Cc: stable, 
because almost any fix I send either goes through some -next or even 
-next-next or already is for some stable kernel.
(I usually don't even look at -rc kernels, just for some special things 
I'm waiting for, like this of-gpio for davinci.)

Regards,

Alexander Holler

[PATCH v2] gpio: davinci: fix gpio selection for OF

[Alexander Holler]

Am 17.03.2014 14:29, schrieb Sekhar Nori:

> One thing to note is that this driver is used by keystone too and all
> its users are DT-only. Although I do not see any in-kernel DT GPIO users
> even there.

Common gpio users are, as always, gpio-keys and gpio-leds. Doesn't have 
one of the keystone boards at least LEDs?

> I can confirm the patch does not break my gpiolib based test module
> (test with and without DT), but then it did not have an issue even before.

Thanks, maybe someone could add a

Cc: <stable@vger.kernel.org>

to get the fix back into 3.14 if it has go through -next.

Regards,

Alexander Holler

[PATCH v2] gpio: davinci: fix gpio selection for OF

[Linus Walleij]

On Mon, Mar 17, 2014 at 3:11 PM, Alexander Holler <holler@ahsoftware.de> wrote:

> Thanks, maybe someone could add a
>
> Cc: <stable@vger.kernel.org>

OK if it goes in as fix I'll add this.

> to get the fix back into 3.14 if it has go through -next.

It's already in linux-next.

Yours,
Linus Walleij

[PATCH v2] gpio: davinci: fix gpio selection for OF

[Grygorii Strashko]

On 03/17/2014 03:29 PM, Sekhar Nori wrote:
> On Friday 14 March 2014 04:32 PM, Linus Walleij wrote:
>> On Fri, Mar 14, 2014 at 11:08 AM, Alexander Holler <holler@ahsoftware.de> wrote:
>>> Am 11.03.2014 11:15, schrieb Linus Walleij:
>>>> On Wed, Mar 5, 2014 at 12:21 PM, Alexander Holler <holler@ahsoftware.de> wrote:
>>>>
>>>>> The driver missed an of_xlate function to translate gpio numbers
>>>>> as found in the DT to the correct chip and number.
>>>>>
>>>>> While there I've set #gpio_cells to a fixed value of 2.
>>>>>
>>>>> I've used gpio-pxa.c as template for those changes and tested my changes
>>>>> successfully on a da850 board using entries for gpio-leds in a DT. So I didn't
>>>>> reinvent the wheel but just copied and tested stuff.
>>>>>
>>>>> Thanks to Grygorii Strashko for the hint to the existing code in gpio-pxa.
>>>>>
>>>>> Signed-off-by: Alexander Holler <holler@ahsoftware.de>
>>>>> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
>>>>
>>>> This v2 version applied, thanks!
>>>
>>> Thanks, but actually that should have been a fix for 3.14 with which the
>>> OF functionality for davinci gpio gets introduced. I assum with the
>>> patch in for-next, 3.14 will appear with that functionality broken and
>>> it will become a candidate for -stable.
>>
>> I just get the impression that DT support for DaVinci in v3.14 is so risky
>> and unstable that noone except those implementing it (i.e. you) is really
>> using it, is that correct?
>>
>> In that case it is hardly a fix that we need to rush out to the entire world.
> 
> One thing to note is that this driver is used by keystone too and all
> its users are DT-only. Although I do not see any in-kernel DT GPIO users
> even there.
> 
> I can confirm the patch does not break my gpiolib based test module
> (test with and without DT), but then it did not have an issue even before.

The issues isn't observed on Keystone2 as it has <32 gpios as of now, also GPIO
 support for keystone is going to be added in 3.15 (including DT changes).

Regards,
-grygorii