gpio: davinci: fix gpio selection for OF

PATCH Superseded Alexander Holler 4 participants 23 messages Mar 4, 2014 – Mar 21, 2014 1 message not in the archive

Subsystems (2)

GPIO TI DaVinci series GPIO driver

lore

Share & tools

lore

A message that is not in the archive (sent off-list or not yet received): [email protected]

Alexander Holler

author

2014-03-04 22:06 UTC #1 lore raw

PATCH 1/1 gpio: davinci: fix gpio selection for OF

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 <[email protected]>

---
 drivers/gpio/gpio-davinci.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

drivers/gpio/gpio-davinci.c +27 -0

index 7629b4f..79f45c4 100644
@@ -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;
 }

Signature · 1 line

1.8.3.1

Linus Walleij

2014-03-05 02:27 UTC #2 lore raw

Quoted 5 lines · 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.

Quoted 7 lines

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 <[email protected]>

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

Yours,
Linus Walleij

Grygorii Strashko

2014-03-05 09:50 UTC #3 lore raw

In reply to Alexander Holler

Quoted 37 lines · 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 <[email protected]>
---
  drivers/gpio/gpio-davinci.c | 27 +++++++++++++++++++++++++++
  1 file changed, 27 insertions(+)
drivers/gpio/gpio-davinci.c +6 -0
index 7629b4f..79f45c4 100644
@@ -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,

drivers/gpio/gpio-davinci.c of_err: · line 190

Quoted 10 lines

drivers/gpio/gpio-davinci.c +10 -0

+			     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.

drivers/gpio/gpio-davinci.c static int davinci_gpio_probe(struct platform_device *pdev) · line 258

Quoted 11 lines

drivers/gpio/gpio-davinci.c +5 -0

+
+	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;

Quoted 7 lines

drivers/gpio/gpio-davinci.c +2 -0

  #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);

drivers/gpio/gpio-davinci.c static int davinci_gpio_probe(struct platform_device *pdev) · line 282

Quoted 9 lines

drivers/gpio/gpio-davinci.c +4 -0

@@ -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

Alexander Holler

author

2014-03-05 11:21 UTC #4 lore raw

PATCH v2 1/1 gpio: davinci: fix gpio selection for OF

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.

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

Signed-off-by Alexander Holler <[email protected]>

Signed-off-by Grygorii Strashko <[email protected]>

---
 drivers/gpio/gpio-davinci.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

Changes in v2: replaced static variables by indirections.

drivers/gpio/gpio-davinci.c +24 -0

index 7629b4f..92574a0 100644
@@ -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);

Signature · 1 line

1.8.3.1

Grygorii Strashko

2014-03-05 12:19 UTC #5 lore raw

Hi Alexander,

Quoted 5 lines · 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.

Quoted 8 lines

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 <[email protected]>
Signed-off-by Grygorii Strashko <[email protected]>

Looks good to me now. Thanks.

Quoted 49 lines

---
  drivers/gpio/gpio-davinci.c | 24 ++++++++++++++++++++++++
  1 file changed, 24 insertions(+)

Changes in v2: replaced static variables by indirections.

drivers/gpio/gpio-davinci.c +24 -0

index 7629b4f..92574a0 100644
@@ -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);

Linus Walleij

2014-03-11 10:15 UTC #6 lore raw

In reply to Alexander Holler

Quoted 5 lines · 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.

Quoted 8 lines

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 <[email protected]>
Signed-off-by Grygorii Strashko <[email protected]>

This v2 version applied, thanks!

Yours,
Linus Walleij

Alexander Holler

author

2014-03-14 10:08 UTC #7 lore raw

Am 11.03.2014 11:15, schrieb Linus Walleij:

Quoted 11 lines · Alexander Holler

Quoted 10 lines
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.

Quoted 6 lines

Quoted 4 lines
Thanks to Grygorii Strashko for the hint to the existing code in gpio-pxa.
Signed-off-by Alexander Holler <[email protected]>
Signed-off-by Grygorii Strashko <[email protected]>
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

Linus Walleij

2014-03-14 11:02 UTC #8 lore raw

Quoted 17 lines · Alexander Holler

Am 11.03.2014 11:15, schrieb Linus Walleij:
Quoted 16 lines · Alexander Holler
Quoted 14 lines
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 <[email protected]>
Signed-off-by Grygorii Strashko <[email protected]>

Quoted 6 lines

Quoted 1 line
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

Alexander Holler

author

2014-03-14 12:38 UTC #9 lore raw

Am 14.03.2014 12:02, schrieb Linus Walleij:

Quoted 20 lines · Alexander Holler

Quoted 19 lines
Am 11.03.2014 11:15, schrieb Linus Walleij:
Quoted 17 lines · Alexander Holler
Quoted 6 lines
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.
Quoted 8 lines
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 <[email protected]>
Signed-off-by Grygorii Strashko <[email protected]>
This v2 version applied, thanks!

Quoted 8 lines

Quoted 4 lines
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.

Quoted 1 line

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

Linus Walleij

2014-03-14 13:54 UTC #10 lore raw

Quoted 6 lines · Alexander Holler

Quoted 2 lines
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

Alexander Holler

author

2014-03-14 18:33 UTC #11 lore raw

Am 14.03.2014 14:54, schrieb Linus Walleij:

Quoted 5 lines · Alexander Holler

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

Quoted 7 lines

Quoted 3 lines
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.

Quoted 4 lines

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

Linus Walleij

2014-03-14 19:52 UTC #12 lore raw

Quoted 21 lines · Alexander Holler

Am 14.03.2014 14:54, schrieb Linus Walleij:
Quoted 12 lines
Quoted 7 lines · Alexander Holler
Quoted 2 lines
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. ;)
Quoted 5 lines
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).

Quoted 2 lines

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

Alexander Holler

author

2014-03-15 07:37 UTC #13 lore raw

Am 14.03.2014 20:52, schrieb Linus Walleij:

Quoted 3 lines

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

Sekhar Nori

2014-03-17 13:29 UTC #14 lore raw

In reply to Linus Walleij

Quoted 25 lines · Linus Walleij

Quoted 17 lines · Alexander Holler
Am 11.03.2014 11:15, schrieb Linus Walleij:
Quoted 16 lines · Alexander Holler
Quoted 14 lines
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 <[email protected]>
Signed-off-by Grygorii Strashko <[email protected]>
Quoted 6 lines
Quoted 1 line
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.

Quoted 5 lines

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

Linus Walleij

2014-03-17 14:05 UTC #15 lore raw

Quoted 6 lines · 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.
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

Sekhar Nori

Tested-by

2014-03-18 08:37 UTC #16 lore raw

Quoted 10 lines · Linus Walleij

Quoted 6 lines · 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.
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 <[email protected]>

Quoted 4 lines

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

Alexander Holler

author

2014-03-18 09:45 UTC #17 lore raw

Am 18.03.2014 09:37, schrieb Sekhar Nori:

Quoted 3 lines

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

Sekhar Nori

2014-03-18 10:54 UTC #18 lore raw

Hi Alexander,

Quoted 6 lines · Alexander Holler

Am 18.03.2014 09:37, schrieb Sekhar Nori:
Quoted 3 lines
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.

Quoted 6 lines

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

Linus Walleij

2014-03-21 08:28 UTC #19 lore raw

In reply to Alexander Holler

Quoted 2 lines · Alexander Holler

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

Alexander Holler

author

2014-03-21 08:59 UTC #20 lore raw

Am 21.03.2014 09:28, schrieb Linus Walleij:

Quoted 9 lines · Alexander Holler

Quoted 3 lines
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.

Quoted 5 lines

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.

Quoted 1 line

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

author

2014-03-17 14:11 UTC #21 lore raw

In reply to Sekhar Nori

Am 17.03.2014 14:29, schrieb Sekhar Nori:

Quoted 3 lines

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?

Quoted 2 lines

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 <[email protected]>

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

Regards,

Alexander Holler

Linus Walleij

2014-03-17 14:13 UTC #22 lore raw

Quoted 3 lines · Alexander Holler

Thanks, maybe someone could add a
Cc <[email protected]>

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

Quoted 1 line

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

It's already in linux-next.

Yours,
Linus Walleij

Grygorii Strashko

2014-03-17 14:12 UTC #23 lore raw

In reply to Sekhar Nori

Quoted 30 lines · Sekhar Nori

Quoted 29 lines · Linus Walleij
Quoted 17 lines · Alexander Holler
Am 11.03.2014 11:15, schrieb Linus Walleij:
Quoted 16 lines · Alexander Holler
Quoted 14 lines
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 <[email protected]>
Signed-off-by Grygorii Strashko <[email protected]>
Quoted 6 lines
Quoted 1 line
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?

Quoted 8 lines

Quoted 1 line
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