diff mbox

[v4,4/6] gpio: davinci: add OF support

Message ID 1383406775-14902-5-git-send-email-prabhakar.csenng@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Prabhakar Nov. 2, 2013, 3:39 p.m. UTC
From: KV Sujith <sujithkv@ti.com>

This patch adds OF parser support for davinci gpio
driver and also appropriate documentation in gpio-davinci.txt
located at Documentation/devicetree/bindings/gpio/.

Signed-off-by: KV Sujith <sujithkv@ti.com>
Signed-off-by: Philip Avinash <avinashphilip@ti.com>
[prabhakar.csengg@gmail.com: simplified the OF code, removed
		unnecessary DT property and also simplified
		the commit message]
Signed-off-by: Lad, Prabhakar <prabhakar.csengg@gmail.com>
---
 .../devicetree/bindings/gpio/gpio-davinci.txt      |   32 ++++++++++++
 drivers/gpio/gpio-davinci.c                        |   54 ++++++++++++++++++--
 2 files changed, 83 insertions(+), 3 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/gpio/gpio-davinci.txt

Comments

Grygorii Strashko Nov. 4, 2013, 6:28 p.m. UTC | #1
Hi Prabhakar Lad,

On 11/02/2013 05:39 PM, Lad, Prabhakar wrote:
> From: KV Sujith <sujithkv@ti.com>
>
> This patch adds OF parser support for davinci gpio
> driver and also appropriate documentation in gpio-davinci.txt
> located at Documentation/devicetree/bindings/gpio/.

I worry, do we need to have gpio_chip.of_xlate() callback implemented?
- From one side, Davinci GPIO controller in DT described by one entry
which defines number of supported GPIOs as "ti,ngpio = <144>;"

- From other side, on Linux level more than one gpio_chip objects are 
instantiated (one per each 32 GPIO).

How the standard GPIO biding will work in this case? .. And will they?

Linus, I'd very appreciate if you will be able to clarify this point.

>
> Signed-off-by: KV Sujith <sujithkv@ti.com>
> Signed-off-by: Philip Avinash <avinashphilip@ti.com>
> [prabhakar.csengg@gmail.com: simplified the OF code, removed
> 		unnecessary DT property and also simplified
> 		the commit message]
> Signed-off-by: Lad, Prabhakar <prabhakar.csengg@gmail.com>
> ---
>   .../devicetree/bindings/gpio/gpio-davinci.txt      |   32 ++++++++++++
>   drivers/gpio/gpio-davinci.c                        |   54 ++++++++++++++++++--
>   2 files changed, 83 insertions(+), 3 deletions(-)
>   create mode 100644 Documentation/devicetree/bindings/gpio/gpio-davinci.txt
>
> diff --git a/Documentation/devicetree/bindings/gpio/gpio-davinci.txt b/Documentation/devicetree/bindings/gpio/gpio-davinci.txt
> new file mode 100644
> index 0000000..55aae1c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/gpio/gpio-davinci.txt
> @@ -0,0 +1,32 @@
> +Davinci GPIO controller bindings
> +
> +Required Properties:
> +- compatible: should be "ti,dm6441-gpio"
> +
> +- reg: Physical base address of the controller and the size of memory mapped
> +       registers.
> +
> +- gpio-controller : Marks the device node as a gpio controller.
> +
> +- interrupts: Array of GPIO interrupt number.

May be meaning of <interrupts> property need to be extended, because,
as of now, only banked or unbanked IRQs are supported - and not both.

> +
> +- ti,ngpio: The number of GPIO pins supported.
> +
> +- ti,davinci-gpio-unbanked: The number of GPIOs that have an individual interrupt
> +		             line to processor.

Should interrupt-controller; specifier be added here?

> +
> +Example:
> +
> +gpio: gpio@1e26000 {
> +	compatible = "ti,dm6441-gpio";
> +	gpio-controller;
> +	reg = <0x226000 0x1000>;
> +	interrupts = <42 IRQ_TYPE_EDGE_BOTH 43 IRQ_TYPE_EDGE_BOTH
> +		44 IRQ_TYPE_EDGE_BOTH 45 IRQ_TYPE_EDGE_BOTH
> +		46 IRQ_TYPE_EDGE_BOTH 47 IRQ_TYPE_EDGE_BOTH
> +		48 IRQ_TYPE_EDGE_BOTH 49 IRQ_TYPE_EDGE_BOTH
> +		50 IRQ_TYPE_EDGE_BOTH>;
> +	ti,ngpio = <144>;
> +	ti,davinci-gpio-irq-base = <101>;

         ^^ Is it still needed?

> +	ti,davinci-gpio-unbanked = <0>;
> +};
> diff --git a/drivers/gpio/gpio-davinci.c b/drivers/gpio/gpio-davinci.c
> index bcb6d8d..bb20a39 100644
> --- a/drivers/gpio/gpio-davinci.c
> +++ b/drivers/gpio/gpio-davinci.c
> @@ -17,6 +17,9 @@
>   #include <linux/io.h>
>   #include <linux/irq.h>
>   #include <linux/irqdomain.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
>   #include <linux/platform_device.h>
>   #include <linux/platform_data/gpio-davinci.h>
>
> @@ -134,6 +137,40 @@ davinci_gpio_set(struct gpio_chip *chip, unsigned offset, int value)
>   	__raw_writel((1 << offset), value ? &g->set_data : &g->clr_data);
>   }
>
> +static struct davinci_gpio_platform_data *
> +davinci_gpio_get_pdata(struct platform_device *pdev)
> +{
> +	struct device_node *dn = pdev->dev.of_node;
> +	struct davinci_gpio_platform_data *pdata;
> +	int ret;
> +	u32 val;
> +
> +	if (!IS_ENABLED(CONFIG_OF) || !pdev->dev.of_node)
> +		return pdev->dev.platform_data;
> +
> +	pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
> +	if (!pdata)
> +		return NULL;
> +
> +	ret = of_property_read_u32(dn, "ti,ngpio", &val);
> +	if (ret)
> +		goto of_err;
> +
> +	pdata->ngpio = val;
> +
> +	ret = of_property_read_u32(dn, "ti,davinci-gpio-unbanked", &val);
> +	if (ret)
> +		goto of_err;
> +
> +	pdata->gpio_unbanked = val;
> +
> +	return pdata;
> +
> +of_err:
> +	dev_err(&pdev->dev, "Populating pdata from DT failed: err %d\n", ret);
> +	return NULL;
> +}
> +
>   static int davinci_gpio_probe(struct platform_device *pdev)
>   {
>   	int i, base;
> @@ -144,12 +181,14 @@ static int davinci_gpio_probe(struct platform_device *pdev)
>   	struct device *dev = &pdev->dev;
>   	struct resource *res;
>
> -	pdata = dev->platform_data;
> +	pdata = davinci_gpio_get_pdata(pdev);
>   	if (!pdata) {
>   		dev_err(dev, "No platform data found\n");
>   		return -EINVAL;
>   	}
>
> +	dev->platform_data = pdata;
> +
>   	/*
>   	 * The gpio banks conceptually expose a segmented bitmap,
>   	 * and "ngpio" is one more than the largest zero-based
> @@ -501,11 +540,20 @@ done:
>   	return 0;
>   }
>
> +#if IS_ENABLED(CONFIG_OF)
> +static const struct of_device_id davinci_gpio_ids[] = {
> +	{ .compatible = "ti,dm6441-gpio", },
> +	{ /* sentinel */ },
> +};
> +MODULE_DEVICE_TABLE(of, davinci_gpio_ids);
> +#endif
> +
>   static struct platform_driver davinci_gpio_driver = {
>   	.probe		= davinci_gpio_probe,
>   	.driver		= {
> -		.name	= "davinci_gpio",
> -		.owner	= THIS_MODULE,
> +		.name		= "davinci_gpio",
> +		.owner		= THIS_MODULE,
> +		.of_match_table	= of_match_ptr(davinci_gpio_ids),
>   	},
>   };
>
>
Regards,
- Grygorii
Prabhakar Nov. 5, 2013, 8:53 a.m. UTC | #2
Hi Grygorii,

Thanks for the review.

On Mon, Nov 4, 2013 at 11:58 PM, Grygorii Strashko
<grygorii.strashko@ti.com> wrote:
> Hi Prabhakar Lad,
>
>
> On 11/02/2013 05:39 PM, Lad, Prabhakar wrote:
>>
>> From: KV Sujith <sujithkv@ti.com>
>>
>> This patch adds OF parser support for davinci gpio
>> driver and also appropriate documentation in gpio-davinci.txt
>> located at Documentation/devicetree/bindings/gpio/.
>
>
> I worry, do we need to have gpio_chip.of_xlate() callback implemented?

I looked for the other OF GPIO implementations with same "ngpio"
property (marvel, msm) but I don’t see of_xlate() callback implemented.

> - From one side, Davinci GPIO controller in DT described by one entry
> which defines number of supported GPIOs as "ti,ngpio = <144>;"
>
> - From other side, on Linux level more than one gpio_chip objects are
> instantiated (one per each 32 GPIO).
>
> How the standard GPIO biding will work in this case? .. And will they?
>
> Linus, I'd very appreciate if you will be able to clarify this point.
>
>
>>
>> Signed-off-by: KV Sujith <sujithkv@ti.com>
>> Signed-off-by: Philip Avinash <avinashphilip@ti.com>
>> [prabhakar.csengg@gmail.com: simplified the OF code, removed
>>                 unnecessary DT property and also simplified
>>                 the commit message]
>> Signed-off-by: Lad, Prabhakar <prabhakar.csengg@gmail.com>
>> ---
>>   .../devicetree/bindings/gpio/gpio-davinci.txt      |   32 ++++++++++++
>>   drivers/gpio/gpio-davinci.c                        |   54
>> ++++++++++++++++++--
>>   2 files changed, 83 insertions(+), 3 deletions(-)
>>   create mode 100644
>> Documentation/devicetree/bindings/gpio/gpio-davinci.txt
>>
>> diff --git a/Documentation/devicetree/bindings/gpio/gpio-davinci.txt
>> b/Documentation/devicetree/bindings/gpio/gpio-davinci.txt
>> new file mode 100644
>> index 0000000..55aae1c
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/gpio/gpio-davinci.txt
>> @@ -0,0 +1,32 @@
>> +Davinci GPIO controller bindings
>> +
>> +Required Properties:
>> +- compatible: should be "ti,dm6441-gpio"
>> +
>> +- reg: Physical base address of the controller and the size of memory
>> mapped
>> +       registers.
>> +
>> +- gpio-controller : Marks the device node as a gpio controller.
>> +
>> +- interrupts: Array of GPIO interrupt number.
>
>
> May be meaning of <interrupts> property need to be extended, because,
> as of now, only banked or unbanked IRQs are supported - and not both.
>
>
OK

>> +
>> +- ti,ngpio: The number of GPIO pins supported.
>> +
>> +- ti,davinci-gpio-unbanked: The number of GPIOs that have an individual
>> interrupt
>> +                            line to processor.
>
>
> Should interrupt-controller; specifier be added here?
>
No

>
>> +
>> +Example:
>> +
>> +gpio: gpio@1e26000 {
>> +       compatible = "ti,dm6441-gpio";
>> +       gpio-controller;
>> +       reg = <0x226000 0x1000>;
>> +       interrupts = <42 IRQ_TYPE_EDGE_BOTH 43 IRQ_TYPE_EDGE_BOTH
>> +               44 IRQ_TYPE_EDGE_BOTH 45 IRQ_TYPE_EDGE_BOTH
>> +               46 IRQ_TYPE_EDGE_BOTH 47 IRQ_TYPE_EDGE_BOTH
>> +               48 IRQ_TYPE_EDGE_BOTH 49 IRQ_TYPE_EDGE_BOTH
>> +               50 IRQ_TYPE_EDGE_BOTH>;
>> +       ti,ngpio = <144>;
>> +       ti,davinci-gpio-irq-base = <101>;
>
>
>         ^^ Is it still needed?
>
OOps missed to remove that.

Regards,
--Prabhakar Lad
Grygorii Strashko Nov. 5, 2013, 4:59 p.m. UTC | #3
On 11/05/2013 10:53 AM, Prabhakar Lad wrote:> Hi Grygorii,
 >
 > Thanks for the review.
 >
 > On Mon, Nov 4, 2013 at 11:58 PM, Grygorii Strashko
 > <grygorii.strashko@ti.com> wrote:
 >> Hi Prabhakar Lad,
 >>
 >>
 >> On 11/02/2013 05:39 PM, Lad, Prabhakar wrote:
 >>>
 >>> From: KV Sujith <sujithkv@ti.com>
 >>>
 >>> This patch adds OF parser support for davinci gpio
 >>> driver and also appropriate documentation in gpio-davinci.txt
 >>> located at Documentation/devicetree/bindings/gpio/.
 >>
 >>
 >> I worry, do we need to have gpio_chip.of_xlate() callback implemented?
 >
 > I looked for the other OF GPIO implementations with same "ngpio"
 > property (marvel, msm) but I don’t see of_xlate() callback implemented.

The question: will below definitions in DT work or not after this series?
  Will of_get_gpio()/of_get_named_gpio() work?

Example1 - leds:
	leds {
		compatible = "gpio-leds";
		debug0 {
			label = "green:debug0";
			gpios = <&gpio 29 GPIO_ACTIVE_HIGH>;
		};
	};

Example2 - any dev:
devA {
	compatible = "devA";
	gpios = <&gpio 120 GPIO_ACTIVE_LOW>;
}


 >
 >> - From one side, Davinci GPIO controller in DT described by one entry
 >> which defines number of supported GPIOs as "ti,ngpio = <144>;"
 >>
 >> - From other side, on Linux level more than one gpio_chip objects are
 >> instantiated (one per each 32 GPIO).
 >>
 >> How the standard GPIO biding will work in this case? .. And will they?
 >>
 >> Linus, I'd very appreciate if you will be able to clarify this point.
 >>
 >>
 >>>
 >>> Signed-off-by: KV Sujith <sujithkv@ti.com>
 >>> Signed-off-by: Philip Avinash <avinashphilip@ti.com>
 >>> [prabhakar.csengg@gmail.com: simplified the OF code, removed
 >>>                  unnecessary DT property and also simplified
 >>>                  the commit message]
 >>> Signed-off-by: Lad, Prabhakar <prabhakar.csengg@gmail.com>
 >>> ---
 >>>    .../devicetree/bindings/gpio/gpio-davinci.txt      |   32 
++++++++++++
 >>>    drivers/gpio/gpio-davinci.c                        |   54
 >>> ++++++++++++++++++--
 >>>    2 files changed, 83 insertions(+), 3 deletions(-)
 >>>    create mode 100644
 >>> Documentation/devicetree/bindings/gpio/gpio-davinci.txt
 >>>
 >>> diff --git a/Documentation/devicetree/bindings/gpio/gpio-davinci.txt
 >>> b/Documentation/devicetree/bindings/gpio/gpio-davinci.txt
 >>> new file mode 100644
 >>> index 0000000..55aae1c
 >>> --- /dev/null
 >>> +++ b/Documentation/devicetree/bindings/gpio/gpio-davinci.txt
 >>> @@ -0,0 +1,32 @@
 >>> +Davinci GPIO controller bindings29
 >>> +
 >>> +Required Properties:
 >>> +- compatible: should be "ti,dm6441-gpio"
 >>> +
 >>> +- reg: Physical base address of the controller and the size of memory
 >>> mapped
 >>> +       registers.
 >>> +
 >>> +- gpio-controller : Marks the device node as a gpio controller.
 >>> +
 >>> +- interrupts: Array of GPIO interrupt number.
 >>
 >>
 >> May be meaning of <interrupts> property need to be extended, because,
 >> as of now, only banked or unbanked IRQs are supported - and not both.
 >>
 >>
 > OK
 >
 >>> +
 >>> +- ti,ngpio: The number of GPIO pins supported.
 >>> +
 >>> +- ti,davinci-gpio-unbanked: The number of GPIOs that have an 
individual
 >>> interrupt
 >>> +                            line to processor.
 >>
 >>
 >> Should interrupt-controller; specifier be added here?
 >>
 > No

So, it would be impossible to map GPIO IRQ to device through DT. Right?
Like:
	devX@0 {
		compatible = "devX";
		interrupt-parent = <&gpio>;
		interrupts = <50 IRQ_TYPE_EDGE_FALLING>; /* gpio line 50 */

	};


 >
 >>
 >>> +
 >>> +Example:
 >>> +
 >>> +gpio: gpio@1e26000 {
 >>> +       compatible = "ti,dm6441-gpio";
 >>> +       gpio-controller;
 >>> +       reg = <0x226000 0x1000>;
 >>> +       interrupts = <42 IRQ_TYPE_EDGE_BOTH 43 IRQ_TYPE_EDGE_BOTH
 >>> +               44 IRQ_TYPE_EDGE_BOTH 45 IRQ_TYPE_EDGE_BOTH
 >>> +               46 IRQ_TYPE_EDGE_BOTH 47 IRQ_TYPE_EDGE_BOTH
 >>> +               48 IRQ_TYPE_EDGE_BOTH 49 IRQ_TYPE_EDGE_BOTH
 >>> +               50 IRQ_TYPE_EDGE_BOTH>;
 >>> +       ti,ngpio = <144>;
 >>> +       ti,davinci-gpio-irq-base = <101>;
 >>
 >>
 >>          ^^ Is it still needed?
 >>
 > OOps missed to remove that.
 >
Regards,
-grygorii
Prabhakar Nov. 6, 2013, 10:08 a.m. UTC | #4
Hi Grygorii,

On Tue, Nov 5, 2013 at 10:29 PM, Grygorii Strashko
<grygorii.strashko@ti.com> wrote:
> On 11/05/2013 10:53 AM, Prabhakar Lad wrote:> Hi Grygorii,
>
>>
>> Thanks for the review.
>>
>> On Mon, Nov 4, 2013 at 11:58 PM, Grygorii Strashko
>> <grygorii.strashko@ti.com> wrote:
>>> Hi Prabhakar Lad,
>>>
>>>
>>> On 11/02/2013 05:39 PM, Lad, Prabhakar wrote:
>>>>
>>>> From: KV Sujith <sujithkv@ti.com>
>>>>
>>>> This patch adds OF parser support for davinci gpio
>>>> driver and also appropriate documentation in gpio-davinci.txt
>>>> located at Documentation/devicetree/bindings/gpio/.
>>>
>>>
>>> I worry, do we need to have gpio_chip.of_xlate() callback implemented?
>>
>> I looked for the other OF GPIO implementations with same "ngpio"
>> property (marvel, msm) but I don’t see of_xlate() callback implemented.
>
> The question: will below definitions in DT work or not after this series?
>  Will of_get_gpio()/of_get_named_gpio() work?
>
> Example1 - leds:
>         leds {
>                 compatible = "gpio-leds";
>                 debug0 {
>                         label = "green:debug0";
>                         gpios = <&gpio 29 GPIO_ACTIVE_HIGH>;
>                 };
>         };
>
> Example2 - any dev:
> devA {
>         compatible = "devA";
>         gpios = <&gpio 120 GPIO_ACTIVE_LOW>;
>
> }
>
>
Agreed of_get_gpio()/of_get_named_gpio() wont work without
xlate callback implemented, but I think this can be added as a
incremental patch later.

>>
>>> - From one side, Davinci GPIO controller in DT described by one entry
>>> which defines number of supported GPIOs as "ti,ngpio = <144>;"
>>>
>>> - From other side, on Linux level more than one gpio_chip objects are
>>> instantiated (one per each 32 GPIO).
>>>
>>> How the standard GPIO biding will work in this case? .. And will they?
>>>
>>> Linus, I'd very appreciate if you will be able to clarify this point.
>>>
>>>
>>>>
>>>> Signed-off-by: KV Sujith <sujithkv@ti.com>
>>>> Signed-off-by: Philip Avinash <avinashphilip@ti.com>
>>>> [prabhakar.csengg@gmail.com: simplified the OF code, removed
>>>>                  unnecessary DT property and also simplified
>>>>                  the commit message]
>>>> Signed-off-by: Lad, Prabhakar <prabhakar.csengg@gmail.com>
>>>> ---
>>>>    .../devicetree/bindings/gpio/gpio-davinci.txt      |   32
>>>> ++++++++++++
>>>>    drivers/gpio/gpio-davinci.c                        |   54
>>>> ++++++++++++++++++--
>>>>    2 files changed, 83 insertions(+), 3 deletions(-)
>>>>    create mode 100644
>>>> Documentation/devicetree/bindings/gpio/gpio-davinci.txt
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/gpio/gpio-davinci.txt
>>>> b/Documentation/devicetree/bindings/gpio/gpio-davinci.txt
>>>> new file mode 100644
>>>> index 0000000..55aae1c
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/gpio/gpio-davinci.txt
>>>> @@ -0,0 +1,32 @@
>>>> +Davinci GPIO controller bindings29
>
>>>> +
>>>> +Required Properties:
>>>> +- compatible: should be "ti,dm6441-gpio"
>>>> +
>>>> +- reg: Physical base address of the controller and the size of memory
>>>> mapped
>>>> +       registers.
>>>> +
>>>> +- gpio-controller : Marks the device node as a gpio controller.
>>>> +
>>>> +- interrupts: Array of GPIO interrupt number.
>>>
>>>
>>> May be meaning of <interrupts> property need to be extended, because,
>>> as of now, only banked or unbanked IRQs are supported - and not both.
>>>
>>>
>> OK
>>
>>>> +
>>>> +- ti,ngpio: The number of GPIO pins supported.
>>>> +
>>>> +- ti,davinci-gpio-unbanked: The number of GPIOs that have an individual
>>>> interrupt
>>>> +                            line to processor.
>>>
>>>
>>> Should interrupt-controller; specifier be added here?
>>>
>> No
>
> So, it would be impossible to map GPIO IRQ to device through DT. Right?
> Like:
>         devX@0 {
>                 compatible = "devX";
>                 interrupt-parent = <&gpio>;
>                 interrupts = <50 IRQ_TYPE_EDGE_FALLING>; /* gpio line 50 */
>
>
>         };
>
>
may be I took you wrong here, the interrupt-controller is inherited
property taken from its parent, so didn’t mention that in the documentation

Regards,
--Prabhakar Lad
Grygorii Strashko Nov. 6, 2013, 11:55 a.m. UTC | #5
On 11/06/2013 12:08 PM, Prabhakar Lad wrote:
> Hi Grygorii,
>
> On Tue, Nov 5, 2013 at 10:29 PM, Grygorii Strashko
> <grygorii.strashko@ti.com> wrote:
>> On 11/05/2013 10:53 AM, Prabhakar Lad wrote:> Hi Grygorii,
>>
>>>
>>> Thanks for the review.
>>>
>>> On Mon, Nov 4, 2013 at 11:58 PM, Grygorii Strashko
>>> <grygorii.strashko@ti.com> wrote:
>>>> Hi Prabhakar Lad,
>>>>
>>>>
>>>> On 11/02/2013 05:39 PM, Lad, Prabhakar wrote:
>>>>>
>>>>> From: KV Sujith <sujithkv@ti.com>
>>>>>
>>>>> This patch adds OF parser support for davinci gpio
>>>>> driver and also appropriate documentation in gpio-davinci.txt
>>>>> located at Documentation/devicetree/bindings/gpio/.
>>>>
>>>>
>>>> I worry, do we need to have gpio_chip.of_xlate() callback implemented?
>>>
>>> I looked for the other OF GPIO implementations with same "ngpio"
>>> property (marvel, msm) but I don’t see of_xlate() callback implemented.
>>
>> The question: will below definitions in DT work or not after this series?
>>   Will of_get_gpio()/of_get_named_gpio() work?
>>
>> Example1 - leds:
>>          leds {
>>                  compatible = "gpio-leds";
>>                  debug0 {
>>                          label = "green:debug0";
>>                          gpios = <&gpio 29 GPIO_ACTIVE_HIGH>;
>>                  };
>>          };
>>
>> Example2 - any dev:
>> devA {
>>          compatible = "devA";
>>          gpios = <&gpio 120 GPIO_ACTIVE_LOW>;
>>
>> }
>>
>>
> Agreed of_get_gpio()/of_get_named_gpio() wont work without
> xlate callback implemented, but I think this can be added as a
> incremental patch later.
>
>>>
>>>> - From one side, Davinci GPIO controller in DT described by one entry
>>>> which defines number of supported GPIOs as "ti,ngpio = <144>;"
>>>>
>>>> - From other side, on Linux level more than one gpio_chip objects are
>>>> instantiated (one per each 32 GPIO).
>>>>
>>>> How the standard GPIO biding will work in this case? .. And will they?
>>>>
>>>> Linus, I'd very appreciate if you will be able to clarify this point.
>>>>
>>>>
>>>>>
>>>>> Signed-off-by: KV Sujith <sujithkv@ti.com>
>>>>> Signed-off-by: Philip Avinash <avinashphilip@ti.com>
>>>>> [prabhakar.csengg@gmail.com: simplified the OF code, removed
>>>>>                   unnecessary DT property and also simplified
>>>>>                   the commit message]
>>>>> Signed-off-by: Lad, Prabhakar <prabhakar.csengg@gmail.com>
>>>>> ---
>>>>>     .../devicetree/bindings/gpio/gpio-davinci.txt      |   32
>>>>> ++++++++++++
>>>>>     drivers/gpio/gpio-davinci.c                        |   54
>>>>> ++++++++++++++++++--
>>>>>     2 files changed, 83 insertions(+), 3 deletions(-)
>>>>>     create mode 100644
>>>>> Documentation/devicetree/bindings/gpio/gpio-davinci.txt
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/gpio/gpio-davinci.txt
>>>>> b/Documentation/devicetree/bindings/gpio/gpio-davinci.txt
>>>>> new file mode 100644
>>>>> index 0000000..55aae1c
>>>>> --- /dev/null
>>>>> +++ b/Documentation/devicetree/bindings/gpio/gpio-davinci.txt
>>>>> @@ -0,0 +1,32 @@
>>>>> +Davinci GPIO controller bindings29
>>
>>>>> +
>>>>> +Required Properties:
>>>>> +- compatible: should be "ti,dm6441-gpio"
>>>>> +
>>>>> +- reg: Physical base address of the controller and the size of memory
>>>>> mapped
>>>>> +       registers.
>>>>> +
>>>>> +- gpio-controller : Marks the device node as a gpio controller.
>>>>> +
>>>>> +- interrupts: Array of GPIO interrupt number.
>>>>
>>>>
>>>> May be meaning of <interrupts> property need to be extended, because,
>>>> as of now, only banked or unbanked IRQs are supported - and not both.
>>>>
>>>>
>>> OK
>>>
>>>>> +
>>>>> +- ti,ngpio: The number of GPIO pins supported.
>>>>> +
>>>>> +- ti,davinci-gpio-unbanked: The number of GPIOs that have an individual
>>>>> interrupt
>>>>> +                            line to processor.
>>>>
>>>>
>>>> Should interrupt-controller; specifier be added here?
>>>>
>>> No
>>
>> So, it would be impossible to map GPIO IRQ to device through DT. Right?
>> Like:
>>          devX@0 {
>>                  compatible = "devX";
>>                  interrupt-parent = <&gpio>;
>>                  interrupts = <50 IRQ_TYPE_EDGE_FALLING>; /* gpio line 50 */
>>
>>
>>          };
>>
>>
> may be I took you wrong here, the interrupt-controller is inherited
> property taken from its parent, so didn’t mention that in the documentation

The GPIO controller uses interrupts form parent controller INTC/GIC
from one side, but from other side it can provide interrupts to its
users.
And as result can be interrupt-controller.

INTC/GIC -> GPIO -> user

It could work for banked IRQs only now :)


>
> Regards,
> --Prabhakar Lad
>
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/gpio/gpio-davinci.txt b/Documentation/devicetree/bindings/gpio/gpio-davinci.txt
new file mode 100644
index 0000000..55aae1c
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/gpio-davinci.txt
@@ -0,0 +1,32 @@ 
+Davinci GPIO controller bindings
+
+Required Properties:
+- compatible: should be "ti,dm6441-gpio"
+
+- reg: Physical base address of the controller and the size of memory mapped
+       registers.
+
+- gpio-controller : Marks the device node as a gpio controller.
+
+- interrupts: Array of GPIO interrupt number.
+
+- ti,ngpio: The number of GPIO pins supported.
+
+- ti,davinci-gpio-unbanked: The number of GPIOs that have an individual interrupt
+		             line to processor.
+
+Example:
+
+gpio: gpio@1e26000 {
+	compatible = "ti,dm6441-gpio";
+	gpio-controller;
+	reg = <0x226000 0x1000>;
+	interrupts = <42 IRQ_TYPE_EDGE_BOTH 43 IRQ_TYPE_EDGE_BOTH
+		44 IRQ_TYPE_EDGE_BOTH 45 IRQ_TYPE_EDGE_BOTH
+		46 IRQ_TYPE_EDGE_BOTH 47 IRQ_TYPE_EDGE_BOTH
+		48 IRQ_TYPE_EDGE_BOTH 49 IRQ_TYPE_EDGE_BOTH
+		50 IRQ_TYPE_EDGE_BOTH>;
+	ti,ngpio = <144>;
+	ti,davinci-gpio-irq-base = <101>;
+	ti,davinci-gpio-unbanked = <0>;
+};
diff --git a/drivers/gpio/gpio-davinci.c b/drivers/gpio/gpio-davinci.c
index bcb6d8d..bb20a39 100644
--- a/drivers/gpio/gpio-davinci.c
+++ b/drivers/gpio/gpio-davinci.c
@@ -17,6 +17,9 @@ 
 #include <linux/io.h>
 #include <linux/irq.h>
 #include <linux/irqdomain.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
 #include <linux/platform_device.h>
 #include <linux/platform_data/gpio-davinci.h>
 
@@ -134,6 +137,40 @@  davinci_gpio_set(struct gpio_chip *chip, unsigned offset, int value)
 	__raw_writel((1 << offset), value ? &g->set_data : &g->clr_data);
 }
 
+static struct davinci_gpio_platform_data *
+davinci_gpio_get_pdata(struct platform_device *pdev)
+{
+	struct device_node *dn = pdev->dev.of_node;
+	struct davinci_gpio_platform_data *pdata;
+	int ret;
+	u32 val;
+
+	if (!IS_ENABLED(CONFIG_OF) || !pdev->dev.of_node)
+		return pdev->dev.platform_data;
+
+	pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
+	if (!pdata)
+		return NULL;
+
+	ret = of_property_read_u32(dn, "ti,ngpio", &val);
+	if (ret)
+		goto of_err;
+
+	pdata->ngpio = val;
+
+	ret = of_property_read_u32(dn, "ti,davinci-gpio-unbanked", &val);
+	if (ret)
+		goto of_err;
+
+	pdata->gpio_unbanked = val;
+
+	return pdata;
+
+of_err:
+	dev_err(&pdev->dev, "Populating pdata from DT failed: err %d\n", ret);
+	return NULL;
+}
+
 static int davinci_gpio_probe(struct platform_device *pdev)
 {
 	int i, base;
@@ -144,12 +181,14 @@  static int davinci_gpio_probe(struct platform_device *pdev)
 	struct device *dev = &pdev->dev;
 	struct resource *res;
 
-	pdata = dev->platform_data;
+	pdata = davinci_gpio_get_pdata(pdev);
 	if (!pdata) {
 		dev_err(dev, "No platform data found\n");
 		return -EINVAL;
 	}
 
+	dev->platform_data = pdata;
+
 	/*
 	 * The gpio banks conceptually expose a segmented bitmap,
 	 * and "ngpio" is one more than the largest zero-based
@@ -501,11 +540,20 @@  done:
 	return 0;
 }
 
+#if IS_ENABLED(CONFIG_OF)
+static const struct of_device_id davinci_gpio_ids[] = {
+	{ .compatible = "ti,dm6441-gpio", },
+	{ /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, davinci_gpio_ids);
+#endif
+
 static struct platform_driver davinci_gpio_driver = {
 	.probe		= davinci_gpio_probe,
 	.driver		= {
-		.name	= "davinci_gpio",
-		.owner	= THIS_MODULE,
+		.name		= "davinci_gpio",
+		.owner		= THIS_MODULE,
+		.of_match_table	= of_match_ptr(davinci_gpio_ids),
 	},
 };