Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1690101 > unrolled thread
| Started by | Keerthy <j-keerthy@ti.com> |
|---|---|
| First post | 2017-07-18 13:00 +0200 |
| Last post | 2017-07-25 05:30 +0200 |
| Articles | 7 on this page of 27 — 6 participants |
Back to article view | Back to linux.kernel
[PATCH 0/4] gpio: davinci: Add keystone-k2g support and few clean ups Keerthy <j-keerthy@ti.com> - 2017-07-18 13:00 +0200
[PATCH 4/4] ARM: dts: keystone-k2g-evm: Add gpio nodes Keerthy <j-keerthy@ti.com> - 2017-07-18 13:00 +0200
Re: [PATCH 4/4] ARM: dts: keystone-k2g-evm: Add gpio nodes Suman Anna <s-anna@ti.com> - 2017-07-18 21:20 +0200
Re: [PATCH 4/4] ARM: dts: keystone-k2g-evm: Add gpio nodes Keerthy <j-keerthy@ti.com> - 2017-07-19 06:10 +0200
[PATCH 1/4] gpio: davinci: Use devm_gpiochip_add_data in place of gpiochip_add_data Keerthy <j-keerthy@ti.com> - 2017-07-18 13:00 +0200
Re: [PATCH 1/4] gpio: davinci: Use devm_gpiochip_add_data in place of gpiochip_add_data Suman Anna <s-anna@ti.com> - 2017-07-18 19:00 +0200
Re: [PATCH 1/4] gpio: davinci: Use devm_gpiochip_add_data in place of gpiochip_add_data Keerthy <j-keerthy@ti.com> - 2017-07-18 19:50 +0200
Re: [PATCH 1/4] gpio: davinci: Use devm_gpiochip_add_data in place of gpiochip_add_data Johan Hovold <johan@kernel.org> - 2017-07-19 11:40 +0200
Re: [PATCH 1/4] gpio: davinci: Use devm_gpiochip_add_data in place of gpiochip_add_data Keerthy <j-keerthy@ti.com> - 2017-07-19 12:00 +0200
[PATCH 2/4] gpio: davinci: Handle the return value of davinci_gpio_irq_setup function Keerthy <j-keerthy@ti.com> - 2017-07-18 13:00 +0200
Re: [PATCH 2/4] gpio: davinci: Handle the return value of davinci_gpio_irq_setup function Suman Anna <s-anna@ti.com> - 2017-07-18 19:00 +0200
Re: [PATCH 2/4] gpio: davinci: Handle the return value of davinci_gpio_irq_setup function Keerthy <j-keerthy@ti.com> - 2017-07-18 19:50 +0200
Re: [PATCH 2/4] gpio: davinci: Handle the return value of davinci_gpio_irq_setup function Johan Hovold <johan@kernel.org> - 2017-07-19 13:20 +0200
Re: [PATCH 2/4] gpio: davinci: Handle the return value of davinci_gpio_irq_setup function Keerthy <j-keerthy@ti.com> - 2017-07-20 08:50 +0200
Re: [PATCH 2/4] gpio: davinci: Handle the return value of davinci_gpio_irq_setup function Keerthy <j-keerthy@ti.com> - 2017-07-20 11:20 +0200
Re: [PATCH 2/4] gpio: davinci: Handle the return value of davinci_gpio_irq_setup function Johan Hovold <johan@kernel.org> - 2017-07-20 12:00 +0200
Re: [PATCH 2/4] gpio: davinci: Handle the return value of davinci_gpio_irq_setup function Keerthy <j-keerthy@ti.com> - 2017-07-20 12:10 +0200
Re: [PATCH 2/4] gpio: davinci: Handle the return value of davinci_gpio_irq_setup function Johan Hovold <johan@kernel.org> - 2017-07-20 12:10 +0200
Re: [PATCH 2/4] gpio: davinci: Handle the return value of davinci_gpio_irq_setup function Keerthy <j-keerthy@ti.com> - 2017-07-20 12:20 +0200
Re: [PATCH 2/4] gpio: davinci: Handle the return value of davinci_gpio_irq_setup function Grygorii Strashko <grygorii.strashko@ti.com> - 2017-07-20 23:40 +0200
Re: [PATCH 2/4] gpio: davinci: Handle the return value of davinci_gpio_irq_setup function Keerthy <j-keerthy@ti.com> - 2017-07-21 06:00 +0200
Re: [PATCH 2/4] gpio: davinci: Handle the return value of davinci_gpio_irq_setup function Johan Hovold <johan@kernel.org> - 2017-07-21 09:50 +0200
[PATCH 3/4] gpio: davinci: Add a separate compatible for keystone-k2g soc Keerthy <j-keerthy@ti.com> - 2017-07-18 13:10 +0200
Re: [PATCH 3/4] gpio: davinci: Add a separate compatible for keystone-k2g soc Sekhar Nori <nsekhar@ti.com> - 2017-07-18 13:40 +0200
Re: [PATCH 3/4] gpio: davinci: Add a separate compatible for keystone-k2g soc Keerthy <j-keerthy@ti.com> - 2017-07-18 19:50 +0200
Re: [PATCH 3/4] gpio: davinci: Add a separate compatible for keystone-k2g soc Rob Herring <robh@kernel.org> - 2017-07-24 20:30 +0200
Re: [PATCH 3/4] gpio: davinci: Add a separate compatible for keystone-k2g soc Keerthy <j-keerthy@ti.com> - 2017-07-25 05:30 +0200
Page 2 of 2 — ← Prev page 1 [2]
| From | Keerthy <j-keerthy@ti.com> |
|---|---|
| Date | 2017-07-21 06:00 +0200 |
| Subject | Re: [PATCH 2/4] gpio: davinci: Handle the return value of davinci_gpio_irq_setup function |
| Message-ID | <u5xgC-83X-7@gated-at.bofh.it> |
| In reply to | #1693244 |
On Friday 21 July 2017 03:04 AM, Grygorii Strashko wrote: > > > On 07/20/2017 05:05 AM, Johan Hovold wrote: >> On Thu, Jul 20, 2017 at 03:32:27PM +0530, Keerthy wrote: >>> On Thursday 20 July 2017 03:20 PM, Johan Hovold wrote: >>>> On Thu, Jul 20, 2017 at 02:40:37PM +0530, Keerthy wrote: >>>>> On Thursday 20 July 2017 12:14 PM, Keerthy wrote: >>>>>> On Wednesday 19 July 2017 04:40 PM, Johan Hovold wrote: >> >>>>>>> There's a separate but related bug here too as the clk_prepare_enable() >>>>>>> in davinci_gpio_irq_setup() is never balanced on driver unbind. >>>>>> >>>>>> Yes Johan. I will send that as a separate patch. >>>>> >>>>> This is already fixed in the latest kernel: >>>>> >>>>> commit 6dc0048cff988858254fcc26becfc1e9753efa79 >>>>> Author: Arvind Yadav <arvind.yadav.cs@gmail.com> >>>>> Date: Tue May 23 14:48:57 2017 +0530 >>>> >>>> That change only handles errors in davinci_gpio_irq_setup() (i.e. during >>>> probe) and not the imbalance at driver unbind that I was referring to. >>> >>> Okay got it. One more clk_unprepare_disable() call needs to be there in >>> probe err path. >> >> No, you need to balance it on driver unbind, that is, in a new remove() >> callback. >> > > Sry, but manual driver unbind for this driver is really smth unexpected ;( > So, I'm not sure if it need to be implemented and even yes - it should not be > a part of this patch. Probably, smth like "convert driver to be a module". > The GPIO_DAVINCI config is bool. Thanks for checking on that Grygorii. > By the way, I've tried to unbind gpio-omap, result - failure (expected), > as unbind does not take into account module refcnt state. Okay. > >
[toc] | [prev] | [next] | [standalone]
| From | Johan Hovold <johan@kernel.org> |
|---|---|
| Date | 2017-07-21 09:50 +0200 |
| Subject | Re: [PATCH 2/4] gpio: davinci: Handle the return value of davinci_gpio_irq_setup function |
| Message-ID | <u5ARb-1Up-11@gated-at.bofh.it> |
| In reply to | #1693244 |
On Thu, Jul 20, 2017 at 04:34:42PM -0500, Grygorii Strashko wrote: > > > On 07/20/2017 05:05 AM, Johan Hovold wrote: > > On Thu, Jul 20, 2017 at 03:32:27PM +0530, Keerthy wrote: > >> On Thursday 20 July 2017 03:20 PM, Johan Hovold wrote: > >>> On Thu, Jul 20, 2017 at 02:40:37PM +0530, Keerthy wrote: > >>>> On Thursday 20 July 2017 12:14 PM, Keerthy wrote: > >>>>> On Wednesday 19 July 2017 04:40 PM, Johan Hovold wrote: > > > >>>>>> There's a separate but related bug here too as the clk_prepare_enable() > >>>>>> in davinci_gpio_irq_setup() is never balanced on driver unbind. > >>>>> > >>>>> Yes Johan. I will send that as a separate patch. > >>>> > >>>> This is already fixed in the latest kernel: > >>>> > >>>> commit 6dc0048cff988858254fcc26becfc1e9753efa79 > >>>> Author: Arvind Yadav <arvind.yadav.cs@gmail.com> > >>>> Date: Tue May 23 14:48:57 2017 +0530 > >>> > >>> That change only handles errors in davinci_gpio_irq_setup() (i.e. during > >>> probe) and not the imbalance at driver unbind that I was referring to. > >> > >> Okay got it. One more clk_unprepare_disable() call needs to be there in > >> probe err path. > > > > No, you need to balance it on driver unbind, that is, in a new remove() > > callback. > > > > Sry, but manual driver unbind for this driver is really smth unexpected ;( It certainly wouldn't be something often used (e.g. besides during development) but that doesn't mean it should not be implemented. > So, I'm not sure if it need to be implemented and even yes - it should not be > a part of this patch. That's why I said "separate, but related" above. > Probably, smth like "convert driver to be a module". > > By the way, I've tried to unbind gpio-omap, result - failure (expected), > as unbind does not take into account module refcnt state. Indeed. We also have CONFIG_DEBUG_TEST_DRIVER_REMOVE which would try to unbind this driver during probe if enabled. Getting into the habit of properly cleaning up allocated and enabled resources is only a good thing; it shows that the author has thought this through and serves as documentation of what needs to be released in both probe error paths and driver unbind callbacks. Assumptions also change over time (e.g. deferred probe and CONFIG_DEBUG_TEST_DRIVER_REMOVE), and by not taking such shortcuts we are also preventing incomplete code from being copied and reproduced in other drivers (e.g. on hotpluggable buses). So, just add the remove callback (in a separate patch) and everything is good. Thanks, Johan
[toc] | [prev] | [next] | [standalone]
| From | Keerthy <j-keerthy@ti.com> |
|---|---|
| Date | 2017-07-18 13:10 +0200 |
| Subject | [PATCH 3/4] gpio: davinci: Add a separate compatible for keystone-k2g soc |
| Message-ID | <u4yy8-1Ez-99@gated-at.bofh.it> |
| In reply to | #1690101 |
Add a separate compatible for keystone-k2g soc
Signed-off-by: Keerthy <j-keerthy@ti.com>
---
Documentation/devicetree/bindings/gpio/gpio-davinci.txt | 3 ++-
drivers/gpio/gpio-davinci.c | 1 +
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/gpio/gpio-davinci.txt b/Documentation/devicetree/bindings/gpio/gpio-davinci.txt
index 5079ba7..1a5c1a2 100644
--- a/Documentation/devicetree/bindings/gpio/gpio-davinci.txt
+++ b/Documentation/devicetree/bindings/gpio/gpio-davinci.txt
@@ -1,7 +1,8 @@
Davinci/Keystone GPIO controller bindings
Required Properties:
-- compatible: should be "ti,dm6441-gpio", "ti,keystone-gpio"
+- compatible: should be "ti,dm6441-gpio", "ti,keystone-gpio",
+ "ti,keystone-k2g-gpio"
- reg: Physical base address of the controller and the size of memory mapped
registers.
diff --git a/drivers/gpio/gpio-davinci.c b/drivers/gpio/gpio-davinci.c
index 932f270..a8d8dd9 100644
--- a/drivers/gpio/gpio-davinci.c
+++ b/drivers/gpio/gpio-davinci.c
@@ -610,6 +610,7 @@ static int davinci_gpio_irq_setup(struct platform_device *pdev)
static const struct of_device_id davinci_gpio_ids[] = {
{ .compatible = "ti,keystone-gpio", keystone_gpio_get_irq_chip},
{ .compatible = "ti,dm6441-gpio", davinci_gpio_get_irq_chip},
+ { .compatible = "ti,keystone-k2g-gpio", keystone_gpio_get_irq_chip},
{ /* sentinel */ },
};
MODULE_DEVICE_TABLE(of, davinci_gpio_ids);
--
1.9.1
[toc] | [prev] | [next] | [standalone]
| From | Sekhar Nori <nsekhar@ti.com> |
|---|---|
| Date | 2017-07-18 13:40 +0200 |
| Subject | Re: [PATCH 3/4] gpio: davinci: Add a separate compatible for keystone-k2g soc |
| Message-ID | <u4z18-1Qn-23@gated-at.bofh.it> |
| In reply to | #1690134 |
Hi Keerthy,
On Tuesday 18 July 2017 04:27 PM, Keerthy wrote:
> Add a separate compatible for keystone-k2g soc
>
> Signed-off-by: Keerthy <j-keerthy@ti.com>
> ---
> Documentation/devicetree/bindings/gpio/gpio-davinci.txt | 3 ++-
> drivers/gpio/gpio-davinci.c | 1 +
> 2 files changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/gpio/gpio-davinci.txt b/Documentation/devicetree/bindings/gpio/gpio-davinci.txt
> index 5079ba7..1a5c1a2 100644
> --- a/Documentation/devicetree/bindings/gpio/gpio-davinci.txt
> +++ b/Documentation/devicetree/bindings/gpio/gpio-davinci.txt
> @@ -1,7 +1,8 @@
> Davinci/Keystone GPIO controller bindings
>
> Required Properties:
> -- compatible: should be "ti,dm6441-gpio", "ti,keystone-gpio"
> +- compatible: should be "ti,dm6441-gpio", "ti,keystone-gpio",
> + "ti,keystone-k2g-gpio"
>
> - reg: Physical base address of the controller and the size of memory mapped
> registers.
> diff --git a/drivers/gpio/gpio-davinci.c b/drivers/gpio/gpio-davinci.c
> index 932f270..a8d8dd9 100644
> --- a/drivers/gpio/gpio-davinci.c
> +++ b/drivers/gpio/gpio-davinci.c
> @@ -610,6 +610,7 @@ static int davinci_gpio_irq_setup(struct platform_device *pdev)
> static const struct of_device_id davinci_gpio_ids[] = {
> { .compatible = "ti,keystone-gpio", keystone_gpio_get_irq_chip},
> { .compatible = "ti,dm6441-gpio", davinci_gpio_get_irq_chip},
> + { .compatible = "ti,keystone-k2g-gpio", keystone_gpio_get_irq_chip},
You don't have to add compatible matching to the driver if you don't
need any special handling for K2G ATM. Your dts should have:
compatible = "ti,keystone-k2g-gpio", "ti,keystone-gpio";
this way, the driver continues to probe using "ti,keystone-gpio", and
when you really discover a need to do some special handling for K2G, a
kernel update will do without the need for a DT update.
Thanks,
Sekhar
[toc] | [prev] | [next] | [standalone]
| From | Keerthy <j-keerthy@ti.com> |
|---|---|
| Date | 2017-07-18 19:50 +0200 |
| Subject | Re: [PATCH 3/4] gpio: davinci: Add a separate compatible for keystone-k2g soc |
| Message-ID | <u4ENc-5q1-25@gated-at.bofh.it> |
| In reply to | #1690181 |
On Tuesday 18 July 2017 05:01 PM, Sekhar Nori wrote:
> Hi Keerthy,
>
> On Tuesday 18 July 2017 04:27 PM, Keerthy wrote:
>> Add a separate compatible for keystone-k2g soc
>>
>> Signed-off-by: Keerthy <j-keerthy@ti.com>
>> ---
>> Documentation/devicetree/bindings/gpio/gpio-davinci.txt | 3 ++-
>> drivers/gpio/gpio-davinci.c | 1 +
>> 2 files changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/devicetree/bindings/gpio/gpio-davinci.txt b/Documentation/devicetree/bindings/gpio/gpio-davinci.txt
>> index 5079ba7..1a5c1a2 100644
>> --- a/Documentation/devicetree/bindings/gpio/gpio-davinci.txt
>> +++ b/Documentation/devicetree/bindings/gpio/gpio-davinci.txt
>> @@ -1,7 +1,8 @@
>> Davinci/Keystone GPIO controller bindings
>>
>> Required Properties:
>> -- compatible: should be "ti,dm6441-gpio", "ti,keystone-gpio"
>> +- compatible: should be "ti,dm6441-gpio", "ti,keystone-gpio",
>> + "ti,keystone-k2g-gpio"
>>
>> - reg: Physical base address of the controller and the size of memory mapped
>> registers.
>> diff --git a/drivers/gpio/gpio-davinci.c b/drivers/gpio/gpio-davinci.c
>> index 932f270..a8d8dd9 100644
>> --- a/drivers/gpio/gpio-davinci.c
>> +++ b/drivers/gpio/gpio-davinci.c
>> @@ -610,6 +610,7 @@ static int davinci_gpio_irq_setup(struct platform_device *pdev)
>> static const struct of_device_id davinci_gpio_ids[] = {
>> { .compatible = "ti,keystone-gpio", keystone_gpio_get_irq_chip},
>> { .compatible = "ti,dm6441-gpio", davinci_gpio_get_irq_chip},
>> + { .compatible = "ti,keystone-k2g-gpio", keystone_gpio_get_irq_chip},
>
> You don't have to add compatible matching to the driver if you don't
> need any special handling for K2G ATM. Your dts should have:
>
> compatible = "ti,keystone-k2g-gpio", "ti,keystone-gpio";
>
> this way, the driver continues to probe using "ti,keystone-gpio", and
> when you really discover a need to do some special handling for K2G, a
> kernel update will do without the need for a DT update.
Sure that is better to do as currently i do not see any differences
between k2g and other k2 devices.
>
> Thanks,
> Sekhar
>
[toc] | [prev] | [next] | [standalone]
| From | Rob Herring <robh@kernel.org> |
|---|---|
| Date | 2017-07-24 20:30 +0200 |
| Subject | Re: [PATCH 3/4] gpio: davinci: Add a separate compatible for keystone-k2g soc |
| Message-ID | <u6Qhd-ac-19@gated-at.bofh.it> |
| In reply to | #1690134 |
On Tue, Jul 18, 2017 at 04:27:15PM +0530, Keerthy wrote:
> Add a separate compatible for keystone-k2g soc
>
> Signed-off-by: Keerthy <j-keerthy@ti.com>
> ---
> Documentation/devicetree/bindings/gpio/gpio-davinci.txt | 3 ++-
> drivers/gpio/gpio-davinci.c | 1 +
> 2 files changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/gpio/gpio-davinci.txt b/Documentation/devicetree/bindings/gpio/gpio-davinci.txt
> index 5079ba7..1a5c1a2 100644
> --- a/Documentation/devicetree/bindings/gpio/gpio-davinci.txt
> +++ b/Documentation/devicetree/bindings/gpio/gpio-davinci.txt
> @@ -1,7 +1,8 @@
> Davinci/Keystone GPIO controller bindings
>
> Required Properties:
> -- compatible: should be "ti,dm6441-gpio", "ti,keystone-gpio"
> +- compatible: should be "ti,dm6441-gpio", "ti,keystone-gpio",
> + "ti,keystone-k2g-gpio"
Reformat to one valid combination per line.
>
> - reg: Physical base address of the controller and the size of memory mapped
> registers.
> diff --git a/drivers/gpio/gpio-davinci.c b/drivers/gpio/gpio-davinci.c
> index 932f270..a8d8dd9 100644
> --- a/drivers/gpio/gpio-davinci.c
> +++ b/drivers/gpio/gpio-davinci.c
> @@ -610,6 +610,7 @@ static int davinci_gpio_irq_setup(struct platform_device *pdev)
> static const struct of_device_id davinci_gpio_ids[] = {
> { .compatible = "ti,keystone-gpio", keystone_gpio_get_irq_chip},
> { .compatible = "ti,dm6441-gpio", davinci_gpio_get_irq_chip},
> + { .compatible = "ti,keystone-k2g-gpio", keystone_gpio_get_irq_chip},
> { /* sentinel */ },
> };
> MODULE_DEVICE_TABLE(of, davinci_gpio_ids);
> --
> 1.9.1
>
[toc] | [prev] | [next] | [standalone]
| From | Keerthy <j-keerthy@ti.com> |
|---|---|
| Date | 2017-07-25 05:30 +0200 |
| Subject | Re: [PATCH 3/4] gpio: davinci: Add a separate compatible for keystone-k2g soc |
| Message-ID | <u6YHM-5XO-7@gated-at.bofh.it> |
| In reply to | #1694983 |
On Monday 24 July 2017 11:54 PM, Rob Herring wrote:
> On Tue, Jul 18, 2017 at 04:27:15PM +0530, Keerthy wrote:
>> Add a separate compatible for keystone-k2g soc
>>
>> Signed-off-by: Keerthy <j-keerthy@ti.com>
>> ---
>> Documentation/devicetree/bindings/gpio/gpio-davinci.txt | 3 ++-
>> drivers/gpio/gpio-davinci.c | 1 +
>> 2 files changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/devicetree/bindings/gpio/gpio-davinci.txt b/Documentation/devicetree/bindings/gpio/gpio-davinci.txt
>> index 5079ba7..1a5c1a2 100644
>> --- a/Documentation/devicetree/bindings/gpio/gpio-davinci.txt
>> +++ b/Documentation/devicetree/bindings/gpio/gpio-davinci.txt
>> @@ -1,7 +1,8 @@
>> Davinci/Keystone GPIO controller bindings
>>
>> Required Properties:
>> -- compatible: should be "ti,dm6441-gpio", "ti,keystone-gpio"
>> +- compatible: should be "ti,dm6441-gpio", "ti,keystone-gpio",
>> + "ti,keystone-k2g-gpio"
>
> Reformat to one valid combination per line.
Sure Rob. Thanks for reviewing.
>
>>
>> - reg: Physical base address of the controller and the size of memory mapped
>> registers.
>> diff --git a/drivers/gpio/gpio-davinci.c b/drivers/gpio/gpio-davinci.c
>> index 932f270..a8d8dd9 100644
>> --- a/drivers/gpio/gpio-davinci.c
>> +++ b/drivers/gpio/gpio-davinci.c
>> @@ -610,6 +610,7 @@ static int davinci_gpio_irq_setup(struct platform_device *pdev)
>> static const struct of_device_id davinci_gpio_ids[] = {
>> { .compatible = "ti,keystone-gpio", keystone_gpio_get_irq_chip},
>> { .compatible = "ti,dm6441-gpio", davinci_gpio_get_irq_chip},
>> + { .compatible = "ti,keystone-k2g-gpio", keystone_gpio_get_irq_chip},
>> { /* sentinel */ },
>> };
>> MODULE_DEVICE_TABLE(of, davinci_gpio_ids);
>> --
>> 1.9.1
>>
[toc] | [prev] | [standalone]
Page 2 of 2 — ← Prev page 1 [2]
Back to top | Article view | linux.kernel
csiph-web