diff mbox

[v1] gpio: keystone: add dsp gpio controller driver

Message ID 1405507426-18992-1-git-send-email-grygorii.strashko@ti.com
State New
Headers show

Commit Message

Grygorii Strashko July 16, 2014, 10:43 a.m. UTC
From: Murali Karicheri <m-karicheri2@ti.com>

On Keystone SOCs, ARM host can send interrupts to DSP cores using the
DSP GPIO controller IP. Each DSP GPIO controller provides 28 IRQ signals for
each DSP core. This is one of the component used by the IPC mechanism used
on Keystone SOCs.

Keystone 2 DSP GPIO controller has specific features:
- each GPIO can be configured only as output pin;
- setting GPIO value to 1 causes IRQ generation on target DSP core;
- reading pin value returns 0 - if IRQ was handled or 1 - IRQ is still
  pending.

Signed-off-by: Murali Karicheri <m-karicheri2@ti.com>
Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
---
 .../devicetree/bindings/gpio/gpio-keystone.txt     |   43 ++++++
 drivers/gpio/Kconfig                               |    8 ++
 drivers/gpio/Makefile                              |    1 +
 drivers/gpio/gpio-keystone.c                       |  138 ++++++++++++++++++++
 4 files changed, 190 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/gpio/gpio-keystone.txt
 create mode 100644 drivers/gpio/gpio-keystone.c

Comments

Varka Bhadram July 16, 2014, 10:05 a.m. UTC | #1
On 07/16/2014 04:13 PM, Grygorii Strashko wrote:
> From: Murali Karicheri <m-karicheri2@ti.com>
>
> On Keystone SOCs, ARM host can send interrupts to DSP cores using the
> DSP GPIO controller IP. Each DSP GPIO controller provides 28 IRQ signals for
> each DSP core. This is one of the component used by the IPC mechanism used
> on Keystone SOCs.
>
> Keystone 2 DSP GPIO controller has specific features:
> - each GPIO can be configured only as output pin;
> - setting GPIO value to 1 causes IRQ generation on target DSP core;
> - reading pin value returns 0 - if IRQ was handled or 1 - IRQ is still
>    pending.
>
> Signed-off-by: Murali Karicheri <m-karicheri2@ti.com>
> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
> ---
>   .../devicetree/bindings/gpio/gpio-keystone.txt     |   43 ++++++
>   drivers/gpio/Kconfig                               |    8 ++
>   drivers/gpio/Makefile                              |    1 +
>   drivers/gpio/gpio-keystone.c                       |  138 ++++++++++++++++++++
>   4 files changed, 190 insertions(+)
>   create mode 100644 Documentation/devicetree/bindings/gpio/gpio-keystone.txt
>   create mode 100644 drivers/gpio/gpio-keystone.c
>
> diff --git a/Documentation/devicetree/bindings/gpio/gpio-keystone.txt b/Documentation/devicetree/bindings/gpio/gpio-keystone.txt
> new file mode 100644
> index 0000000..4f92af4
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/gpio/gpio-keystone.txt
> @@ -0,0 +1,43 @@
> +Keystone 2 DSP GPIO controller bindings
> +
> +HOST OS userland running on ARM can send interrupts to DSP cores using
> +the DSP GPIO controller IP. It provides 28 IRQ signals per each DSP core.
> +This is one of the component used by the IPC mechanism used on Keystone SOCs.
> +
> +For example TCI6638K2K SoC has 8 DSP GPIO controllers:
> + - 8 for C66x CorePacx CPUs 0-7
> +
> +Keystone 2 DSP GPIO controller has specific features:
> +- each GPIO can be configured only as output pin;
> +- setting GPIO value to 1 causes IRQ generation on target DSP core;
> +- reading pin value returns 0 - if IRQ was handled or 1 - IRQ is still
> +  pending.
> +
> +Required Properties:
> +- compatible: should be "ti,keystone-dsp-gpio"
> +
> +- ti,syscon-dev:	phandle/offset pair. The phandle to syscon used to
> +			access device state control registers and the offset
> +			in order to use block of device's specific registers.
> +
> +- gpio-controller : Marks the device node as a gpio controller.
> +
> +- #gpio-cells : Should be one.
> +				See gpio.txt in this directory for a of the cells format

All the properties not properly aligned. It would be more readable if

Required Properties:
- compatible		: should be "ti,keystone-dsp-gpio"
- ti,syscon-dev		: phandle/offset pair. The phandle to syscon used to
			  access device state control registers and the offset
			  in order to use block of device's specific registers.
- gpio-controller	: Marks the device node as a gpio controller.
- #gpio-cells		: Should be one.
			  See gpio.txt in this directory for a of the cells format

> +
> +Please refer to gpio.txt in this directory for details of the common GPIO
> +bindings used by client devices.
> +

(...)

> +static struct platform_driver keystone_gpio_driver = {
> +	.probe		= keystone_gpio_probe,
> +	.remove		= keystone_gpio_remove,
> +	.driver		= {
> +		.name	= "keystone-dsp-gpio",
> +		.owner	= THIS_MODULE,

We can drop owner field...  :-)  .It will update by module_platform_driver
Grygorii Strashko July 21, 2014, 2:20 p.m. UTC | #2
Hi Varka Bhadram,

On 07/16/2014 01:05 PM, Varka Bhadram wrote:
> On 07/16/2014 04:13 PM, Grygorii Strashko wrote:
>> From: Murali Karicheri <m-karicheri2@ti.com>
>>
>> On Keystone SOCs, ARM host can send interrupts to DSP cores using the
>> DSP GPIO controller IP. Each DSP GPIO controller provides 28 IRQ
>> signals for
>> each DSP core. This is one of the component used by the IPC mechanism
>> used
>> on Keystone SOCs.
>>
>> Keystone 2 DSP GPIO controller has specific features:
>> - each GPIO can be configured only as output pin;
>> - setting GPIO value to 1 causes IRQ generation on target DSP core;
>> - reading pin value returns 0 - if IRQ was handled or 1 - IRQ is still
>>    pending.
>>
>> Signed-off-by: Murali Karicheri <m-karicheri2@ti.com>
>> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
>> ---
>>   .../devicetree/bindings/gpio/gpio-keystone.txt     |   43 ++++++
>>   drivers/gpio/Kconfig                               |    8 ++
>>   drivers/gpio/Makefile                              |    1 +
>>   drivers/gpio/gpio-keystone.c                       |  138
>> ++++++++++++++++++++
>>   4 files changed, 190 insertions(+)
>>   create mode 100644
>> Documentation/devicetree/bindings/gpio/gpio-keystone.txt
>>   create mode 100644 drivers/gpio/gpio-keystone.c
>>
>> diff --git a/Documentation/devicetree/bindings/gpio/gpio-keystone.txt
>> b/Documentation/devicetree/bindings/gpio/gpio-keystone.txt
>> new file mode 100644
>> index 0000000..4f92af4
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/gpio/gpio-keystone.txt
>> @@ -0,0 +1,43 @@
>> +Keystone 2 DSP GPIO controller bindings
>> +
>> +HOST OS userland running on ARM can send interrupts to DSP cores using
>> +the DSP GPIO controller IP. It provides 28 IRQ signals per each DSP
>> core.
>> +This is one of the component used by the IPC mechanism used on
>> Keystone SOCs.
>> +
>> +For example TCI6638K2K SoC has 8 DSP GPIO controllers:
>> + - 8 for C66x CorePacx CPUs 0-7
>> +
>> +Keystone 2 DSP GPIO controller has specific features:
>> +- each GPIO can be configured only as output pin;
>> +- setting GPIO value to 1 causes IRQ generation on target DSP core;
>> +- reading pin value returns 0 - if IRQ was handled or 1 - IRQ is still
>> +  pending.
>> +
>> +Required Properties:
>> +- compatible: should be "ti,keystone-dsp-gpio"
>> +
>> +- ti,syscon-dev:    phandle/offset pair. The phandle to syscon used to
>> +            access device state control registers and the offset
>> +            in order to use block of device's specific registers.
>> +
>> +- gpio-controller : Marks the device node as a gpio controller.
>> +
>> +- #gpio-cells : Should be one.
>> +                See gpio.txt in this directory for a of the cells format
>
> All the properties not properly aligned. It would be more readable if
>
> Required Properties:
> - compatible        : should be "ti,keystone-dsp-gpio"
> - ti,syscon-dev        : phandle/offset pair. The phandle to syscon used to
>                access device state control registers and the offset
>                in order to use block of device's specific registers.
> - gpio-controller    : Marks the device node as a gpio controller.
> - #gpio-cells        : Should be one.
>                See gpio.txt in this directory for a of the cells format

Unfortunately, the format of DT bindings description is not 
standardized, but i will try to beautify it :)

>
>> +
>> +Please refer to gpio.txt in this directory for details of the common
>> GPIO
>> +bindings used by client devices.
>> +
>
> (...)
>
>> +static struct platform_driver keystone_gpio_driver = {
>> +    .probe        = keystone_gpio_probe,
>> +    .remove        = keystone_gpio_remove,
>> +    .driver        = {
>> +        .name    = "keystone-dsp-gpio",
>> +        .owner    = THIS_MODULE,
>
> We can drop owner field...  :-)  .It will update by module_platform_driver
>
>

Thanks for your comments - I'll wait a bit and re-send.

Regards,
- grygorii

--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Linus Walleij July 23, 2014, 3:10 p.m. UTC | #3
On Wed, Jul 16, 2014 at 12:43 PM, Grygorii Strashko
<grygorii.strashko@ti.com> wrote:

> From: Murali Karicheri <m-karicheri2@ti.com>
>
> On Keystone SOCs, ARM host can send interrupts to DSP cores using the
> DSP GPIO controller IP. Each DSP GPIO controller provides 28 IRQ signals for
> each DSP core. This is one of the component used by the IPC mechanism used
> on Keystone SOCs.
>
> Keystone 2 DSP GPIO controller has specific features:
> - each GPIO can be configured only as output pin;
> - setting GPIO value to 1 causes IRQ generation on target DSP core;
> - reading pin value returns 0 - if IRQ was handled or 1 - IRQ is still
>   pending.
>
> Signed-off-by: Murali Karicheri <m-karicheri2@ti.com>
> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>

Pardon me. How is this GENERAL PURPOSE Input/Output?

It seems very very much SPECIAL PURPOSE to me, it's like
you're just shoehorning some IPC mechanism into the GPIO
subsystem, and this may be because the datasheet calls it
GPIO when it's not.

What other stuff than DSP is connected to these lines, and is it
really even external lines? Aren't these just polysilicon rails
pretty much hammered to be used by the DSP and nothing else.

What is the difference between this and a mailbox IRQ line
and the kind of stuff handled by drivers/mailbox?

I'd like Suman and Jassi to have a look at this to see if it's
actually a mailbox before we proceed.

And if you proceed with this, please integrate it with
drivers/gpio/gpio-syscon.c, I don't need more special
syscons GPIO handlers.

> +#include <linux/mfd/syscon.h>

Kconfig needs depends on MFD_SYSCON, right?

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
Alexander Shiyan July 23, 2014, 3:19 p.m. UTC | #4
Wed, 23 Jul 2014 17:10:26 +0200 от Linus Walleij <linus.walleij@linaro.org>:
> On Wed, Jul 16, 2014 at 12:43 PM, Grygorii Strashko
> <grygorii.strashko@ti.com> wrote:
> 
> > From: Murali Karicheri <m-karicheri2@ti.com>
> >
> > On Keystone SOCs, ARM host can send interrupts to DSP cores using the
> > DSP GPIO controller IP. Each DSP GPIO controller provides 28 IRQ signals for
> > each DSP core. This is one of the component used by the IPC mechanism used
> > on Keystone SOCs.
> >
> > Keystone 2 DSP GPIO controller has specific features:
> > - each GPIO can be configured only as output pin;
> > - setting GPIO value to 1 causes IRQ generation on target DSP core;
> > - reading pin value returns 0 - if IRQ was handled or 1 - IRQ is still
> >   pending.
> >
> > Signed-off-by: Murali Karicheri <m-karicheri2@ti.com>
> > Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
...
> And if you proceed with this, please integrate it with
> drivers/gpio/gpio-syscon.c, I don't need more special
> syscons GPIO handlers.
> 
> > +#include <linux/mfd/syscon.h>
> 
> Kconfig needs depends on MFD_SYSCON, right?

It should be selected by the platform. For compile test this symbol
is not needed.

---
Santosh Shilimkar July 23, 2014, 3:25 p.m. UTC | #5
On Wednesday 23 July 2014 11:10 AM, Linus Walleij wrote:
> On Wed, Jul 16, 2014 at 12:43 PM, Grygorii Strashko
> <grygorii.strashko@ti.com> wrote:
> 
>> From: Murali Karicheri <m-karicheri2@ti.com>
>>
>> On Keystone SOCs, ARM host can send interrupts to DSP cores using the
>> DSP GPIO controller IP. Each DSP GPIO controller provides 28 IRQ signals for
>> each DSP core. This is one of the component used by the IPC mechanism used
>> on Keystone SOCs.
>>
>> Keystone 2 DSP GPIO controller has specific features:
>> - each GPIO can be configured only as output pin;
>> - setting GPIO value to 1 causes IRQ generation on target DSP core;
>> - reading pin value returns 0 - if IRQ was handled or 1 - IRQ is still
>>   pending.
>>
>> Signed-off-by: Murali Karicheri <m-karicheri2@ti.com>
>> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
> 
> Pardon me. How is this GENERAL PURPOSE Input/Output?
> 
> It seems very very much SPECIAL PURPOSE to me, it's like
> you're just shoehorning some IPC mechanism into the GPIO
> subsystem, and this may be because the datasheet calls it
> GPIO when it's not.
>
> What other stuff than DSP is connected to these lines, and is it
> really even external lines? Aren't these just polysilicon rails
> pretty much hammered to be used by the DSP and nothing else.
>
> What is the difference between this and a mailbox IRQ line
> and the kind of stuff handled by drivers/mailbox?
>
I will try to answer this. This IP is indeed a GPIO block
but the IO's are used just OUTPUT lines from Linux
HOST perspective. These IOs are connected to the DSPs
as input/IRQ lines. The DSP-ARM host IPC mechanism used on
Keystone is Linux user-space based and it does as one of the
component.

Its not mailbox since there is no payload etc attached to it.
GPIO lib does expose the IO's to userspace and thats what is
being the case here. I think it is a legitimate usecase.

Sometimes the sending signals to another co-processor might
appear like shoehorning or odd but it just uses standard
GPIO library and its capabilities as is.

Regards,
Santosh






--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Linus Walleij July 24, 2014, 2:12 p.m. UTC | #6
On Wed, Jul 23, 2014 at 5:25 PM, Santosh Shilimkar
<santosh.shilimkar@ti.com> wrote:

> I will try to answer this. This IP is indeed a GPIO block
> but the IO's are used just OUTPUT lines from Linux
> HOST perspective. These IOs are connected to the DSPs
> as input/IRQ lines.

So the DSP is another discrete IC, and could be something
different, so this is board-level information?

I'm really worrying whether this is general purpose or not :-/

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Santosh Shilimkar July 24, 2014, 2:21 p.m. UTC | #7
On Thursday 24 July 2014 10:12 AM, Linus Walleij wrote:
> On Wed, Jul 23, 2014 at 5:25 PM, Santosh Shilimkar
> <santosh.shilimkar@ti.com> wrote:
> 
>> I will try to answer this. This IP is indeed a GPIO block
>> but the IO's are used just OUTPUT lines from Linux
>> HOST perspective. These IOs are connected to the DSPs
>> as input/IRQ lines.
> 
> So the DSP is another discrete IC, and could be something
> different, so this is board-level information?
> 
> I'm really worrying whether this is general purpose or not :-/
> 
Am not sure I follow you. This IP is completely controlled
by Linux OS to generate output signals. How does it matter
whether its connected to a peripheral or a discrete IC. For
example instead of DSP if I connected these lines to an
external PMIC, which considers these as input lines to
perform some actions. Isn't that GPIO usecase as per you ?
Given that this IP only output functionality is used but
that shouldn't matter. We have seen SOCs where GPIOs
are just used as input to form a Matrix Keyboard.

May be I am missing your point.

Regards,
Santosh



--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Linus Walleij July 24, 2014, 3:23 p.m. UTC | #8
On Thu, Jul 24, 2014 at 4:21 PM, Santosh Shilimkar
<santosh.shilimkar@ti.com> wrote:
> On Thursday 24 July 2014 10:12 AM, Linus Walleij wrote:
>> On Wed, Jul 23, 2014 at 5:25 PM, Santosh Shilimkar
>> <santosh.shilimkar@ti.com> wrote:
>>
>>> I will try to answer this. This IP is indeed a GPIO block
>>> but the IO's are used just OUTPUT lines from Linux
>>> HOST perspective. These IOs are connected to the DSPs
>>> as input/IRQ lines.
>>
>> So the DSP is another discrete IC, and could be something
>> different, so this is board-level information?
>>
>> I'm really worrying whether this is general purpose or not :-/
>>
> Am not sure I follow you. This IP is completely controlled
> by Linux OS to generate output signals. How does it matter
> whether its connected to a peripheral or a discrete IC.

What matters to me is whether it is general purpose or
not, and what the use case is.

The Kconfig symbol is called GPIO_KEYSTONE_DSP
not GPIO_KEYSTONE. That does not sound very general
purpose at all. Why is "DSP" at the end of that config
option if it is general purpose?

And we know the Keystone
already has another GPIO block, selected from the
Kconfig symbol GPIO_DAVINCI. Probably that is the
only real GPIO on this system.

And the use case doesn't seem to be exactly for
things like driving leds, reading keys, bit-banging SPI
or MMC card detect or other such common cases.
It seems to be to trigger IRQs on another processor and
nothing else. Not general purpose.

If writing bit such and such in some register has the
effect of pulling a bit high or low in some other IP
is not GPIO. It should be part of the driver for that
other IP block.

Further you wrote:

> The DSP-ARM host IPC mechanism used on
> Keystone is Linux user-space based and it does as
> one of the component.

And given that it's already hinted that this is actually
only there to aid some userspace driver I'm even *less*
interested in having it in the GPIO subsystem.

Shoehorning this into the GPIO subsystem just seems
like some convenient way to keep that DSP driver code
in userspace instead of writing a proper kernel driver
for the DSP.

Like someone wants to avoid things like this:
drivers/staging/tidspbridge
drivers/remoteproc/omap_remoteproc.c
drivers/remoteproc/da8xx_remoteproc.c

As a community maintainer, naturally doing such real
kernel drivers is way better than trying to sneak something
in under the radar, and now I'm worried that this is what
is actually attempted by this driver, so I'm concerned.

> Given that this IP only output functionality is used but
> that shouldn't matter. We have seen SOCs where GPIOs
> are just used as input to form a Matrix Keyboard.

Yes, that is common. And that is for example done with
GPIO_DAVINCI which has lines out to open space
and general purposes.

This is not GPIO, this is DSPIO IMO.

Yours,
Linus Walleij
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Santosh Shilimkar July 24, 2014, 3:40 p.m. UTC | #9
On Thursday 24 July 2014 11:23 AM, Linus Walleij wrote:
> On Thu, Jul 24, 2014 at 4:21 PM, Santosh Shilimkar
> <santosh.shilimkar@ti.com> wrote:
>> On Thursday 24 July 2014 10:12 AM, Linus Walleij wrote:
>>> On Wed, Jul 23, 2014 at 5:25 PM, Santosh Shilimkar
>>> <santosh.shilimkar@ti.com> wrote:
>>>
>>>> I will try to answer this. This IP is indeed a GPIO block
>>>> but the IO's are used just OUTPUT lines from Linux
>>>> HOST perspective. These IOs are connected to the DSPs
>>>> as input/IRQ lines.
>>>
>>> So the DSP is another discrete IC, and could be something
>>> different, so this is board-level information?
>>>
>>> I'm really worrying whether this is general purpose or not :-/
>>>
>> Am not sure I follow you. This IP is completely controlled
>> by Linux OS to generate output signals. How does it matter
>> whether its connected to a peripheral or a discrete IC.
> 
> What matters to me is whether it is general purpose or
> not, and what the use case is.
> 
> The Kconfig symbol is called GPIO_KEYSTONE_DSP
> not GPIO_KEYSTONE. That does not sound very general
> purpose at all. Why is "DSP" at the end of that config
> option if it is general purpose?
> 
That DSP is to just give different name since there
is another GPIO IP on keystone. We can get rid of that
DSP name but I think thats not your concern.

> And we know the Keystone
> already has another GPIO block, selected from the
> Kconfig symbol GPIO_DAVINCI. Probably that is the
> only real GPIO on this system.
>
Am sorry to say but real, unreal is very debatable.
A SOC can have more than one IP instance for different
purposes.

> And the use case doesn't seem to be exactly for
> things like driving leds, reading keys, bit-banging SPI
> or MMC card detect or other such common cases.
> It seems to be to trigger IRQs on another processor and
> nothing else. Not general purpose.
>
> If writing bit such and such in some register has the
> effect of pulling a bit high or low in some other IP
> is not GPIO. It should be part of the driver for that
> other IP block.
>
Using GPIO as an interrupt line is a legitimate
usecase already supported GPIO lib.
 
> Further you wrote:
> 
>> The DSP-ARM host IPC mechanism used on
>> Keystone is Linux user-space based and it does as
>> one of the component.
> 
> And given that it's already hinted that this is actually
> only there to aid some userspace driver I'm even *less*
> interested in having it in the GPIO subsystem.
> 
> Shoehorning this into the GPIO subsystem just seems
> like some convenient way to keep that DSP driver code
> in userspace instead of writing a proper kernel driver
> for the DSP.
> 
> Like someone wants to avoid things like this:
> drivers/staging/tidspbridge
> drivers/remoteproc/omap_remoteproc.c
> drivers/remoteproc/da8xx_remoteproc.c
>
Not at all. The usecase is different. remoteproc's are more
for firmware download, powerup, powerdown, boot an external
co-processor.
 
> As a community maintainer, naturally doing such real
> kernel drivers is way better than trying to sneak something
> in under the radar, and now I'm worried that this is what
> is actually attempted by this driver, so I'm concerned.
>
I respect your view but in this particular case, I just
thought we are denying a legitimate plumbing. Because if
it doesn't fit here, fitting it in other subsystems will
be shoehorning in my view.
 
>> Given that this IP only output functionality is used but
>> that shouldn't matter. We have seen SOCs where GPIOs
>> are just used as input to form a Matrix Keyboard.
> 
> Yes, that is common. And that is for example done with
> GPIO_DAVINCI which has lines out to open space
> and general purposes.
> 
> This is not GPIO, this is DSPIO IMO.
> 
I just think you are too much reading into that DSP name
which was just there to differentiate it from the other
GPIO IP.

Regards,
Santosh
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jassi Brar July 24, 2014, 5:19 p.m. UTC | #10
On 23 July 2014 20:40, Linus Walleij <linus.walleij@linaro.org> wrote:
> On Wed, Jul 16, 2014 at 12:43 PM, Grygorii Strashko
> <grygorii.strashko@ti.com> wrote:
>
>> From: Murali Karicheri <m-karicheri2@ti.com>
>>
>> On Keystone SOCs, ARM host can send interrupts to DSP cores using the
>> DSP GPIO controller IP. Each DSP GPIO controller provides 28 IRQ signals for
>> each DSP core. This is one of the component used by the IPC mechanism used
>> on Keystone SOCs.
>>
>> Keystone 2 DSP GPIO controller has specific features:
>> - each GPIO can be configured only as output pin;
>> - setting GPIO value to 1 causes IRQ generation on target DSP core;
>> - reading pin value returns 0 - if IRQ was handled or 1 - IRQ is still
>>   pending.
>>
>> Signed-off-by: Murali Karicheri <m-karicheri2@ti.com>
>> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
>
> Pardon me. How is this GENERAL PURPOSE Input/Output?
>
> It seems very very much SPECIAL PURPOSE to me, it's like
> you're just shoehorning some IPC mechanism into the GPIO
> subsystem, and this may be because the datasheet calls it
> GPIO when it's not.
>
> What other stuff than DSP is connected to these lines, and is it
> really even external lines? Aren't these just polysilicon rails
> pretty much hammered to be used by the DSP and nothing else.
>
> What is the difference between this and a mailbox IRQ line
> and the kind of stuff handled by drivers/mailbox?
>
> I'd like Suman and Jassi to have a look at this to see if it's
> actually a mailbox before we proceed.
>
The controller seems like most others, only incapable of reading
signals (output only).
 The userspace driving those signals to communicate with a DSP isn't
enough to call it a mailbox usecase, because on a different board the
userspace may drive those signals to control LEDs :)

Cheers,
Jassi
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Santosh Shilimkar July 24, 2014, 5:22 p.m. UTC | #11
On Thursday 24 July 2014 01:19 PM, Jassi Brar wrote:
> On 23 July 2014 20:40, Linus Walleij <linus.walleij@linaro.org> wrote:
>> On Wed, Jul 16, 2014 at 12:43 PM, Grygorii Strashko
>> <grygorii.strashko@ti.com> wrote:
>>
>>> From: Murali Karicheri <m-karicheri2@ti.com>
>>>
>>> On Keystone SOCs, ARM host can send interrupts to DSP cores using the
>>> DSP GPIO controller IP. Each DSP GPIO controller provides 28 IRQ signals for
>>> each DSP core. This is one of the component used by the IPC mechanism used
>>> on Keystone SOCs.
>>>
>>> Keystone 2 DSP GPIO controller has specific features:
>>> - each GPIO can be configured only as output pin;
>>> - setting GPIO value to 1 causes IRQ generation on target DSP core;
>>> - reading pin value returns 0 - if IRQ was handled or 1 - IRQ is still
>>>   pending.
>>>
>>> Signed-off-by: Murali Karicheri <m-karicheri2@ti.com>
>>> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
>>
>> Pardon me. How is this GENERAL PURPOSE Input/Output?
>>
>> It seems very very much SPECIAL PURPOSE to me, it's like
>> you're just shoehorning some IPC mechanism into the GPIO
>> subsystem, and this may be because the datasheet calls it
>> GPIO when it's not.
>>
>> What other stuff than DSP is connected to these lines, and is it
>> really even external lines? Aren't these just polysilicon rails
>> pretty much hammered to be used by the DSP and nothing else.
>>
>> What is the difference between this and a mailbox IRQ line
>> and the kind of stuff handled by drivers/mailbox?
>>
>> I'd like Suman and Jassi to have a look at this to see if it's
>> actually a mailbox before we proceed.
>>
> The controller seems like most others, only incapable of reading
> signals (output only).
>  The userspace driving those signals to communicate with a DSP isn't
> enough to call it a mailbox usecase, because on a different board the
> userspace may drive those signals to control LEDs :)
> 
Exactly !!
And that was my point. Thanks for echo.

Regards,
Santosh



--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jassi Brar July 24, 2014, 6:12 p.m. UTC | #12
On 24 July 2014 22:52, Santosh Shilimkar <santosh.shilimkar@ti.com> wrote:
> On Thursday 24 July 2014 01:19 PM, Jassi Brar wrote:
>> On 23 July 2014 20:40, Linus Walleij <linus.walleij@linaro.org> wrote:
>>> On Wed, Jul 16, 2014 at 12:43 PM, Grygorii Strashko
>>> <grygorii.strashko@ti.com> wrote:
>>>
>>>> From: Murali Karicheri <m-karicheri2@ti.com>
>>>>
>>>> On Keystone SOCs, ARM host can send interrupts to DSP cores using the
>>>> DSP GPIO controller IP. Each DSP GPIO controller provides 28 IRQ signals for
>>>> each DSP core. This is one of the component used by the IPC mechanism used
>>>> on Keystone SOCs.
>>>>
>>>> Keystone 2 DSP GPIO controller has specific features:
>>>> - each GPIO can be configured only as output pin;
>>>> - setting GPIO value to 1 causes IRQ generation on target DSP core;
>>>> - reading pin value returns 0 - if IRQ was handled or 1 - IRQ is still
>>>>   pending.
>>>>
>>>> Signed-off-by: Murali Karicheri <m-karicheri2@ti.com>
>>>> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
>>>
>>> Pardon me. How is this GENERAL PURPOSE Input/Output?
>>>
>>> It seems very very much SPECIAL PURPOSE to me, it's like
>>> you're just shoehorning some IPC mechanism into the GPIO
>>> subsystem, and this may be because the datasheet calls it
>>> GPIO when it's not.
>>>
>>> What other stuff than DSP is connected to these lines, and is it
>>> really even external lines? Aren't these just polysilicon rails
>>> pretty much hammered to be used by the DSP and nothing else.
>>>
>>> What is the difference between this and a mailbox IRQ line
>>> and the kind of stuff handled by drivers/mailbox?
>>>
>>> I'd like Suman and Jassi to have a look at this to see if it's
>>> actually a mailbox before we proceed.
>>>
>> The controller seems like most others, only incapable of reading
>> signals (output only).
>>  The userspace driving those signals to communicate with a DSP isn't
>> enough to call it a mailbox usecase, because on a different board the
>> userspace may drive those signals to control LEDs :)
>>
> Exactly !!
> And that was my point. Thanks for echo.
>
Yeah but if the AP and DSP are within the same package (i.e, the
'pins' can't be used for any other purpose on any board), one might
sell it as a mailbox. However, since the mailbox protocol driver would
be in userspace, I think it is justified to expose that as GPIO
otherwise we'll have to add another interface for userspace to control
the DSP.

Cheers,
Jassi
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Suman Anna July 24, 2014, 6:52 p.m. UTC | #13
Hi,

On 07/24/2014 01:12 PM, Jassi Brar wrote:
> On 24 July 2014 22:52, Santosh Shilimkar <santosh.shilimkar@ti.com> wrote:
>> On Thursday 24 July 2014 01:19 PM, Jassi Brar wrote:
>>> On 23 July 2014 20:40, Linus Walleij <linus.walleij@linaro.org> wrote:
>>>> On Wed, Jul 16, 2014 at 12:43 PM, Grygorii Strashko
>>>> <grygorii.strashko@ti.com> wrote:
>>>>
>>>>> From: Murali Karicheri <m-karicheri2@ti.com>
>>>>>
>>>>> On Keystone SOCs, ARM host can send interrupts to DSP cores using the
>>>>> DSP GPIO controller IP. Each DSP GPIO controller provides 28 IRQ signals for
>>>>> each DSP core. This is one of the component used by the IPC mechanism used
>>>>> on Keystone SOCs.
>>>>>
>>>>> Keystone 2 DSP GPIO controller has specific features:
>>>>> - each GPIO can be configured only as output pin;
>>>>> - setting GPIO value to 1 causes IRQ generation on target DSP core;
>>>>> - reading pin value returns 0 - if IRQ was handled or 1 - IRQ is still
>>>>>   pending.
>>>>>
>>>>> Signed-off-by: Murali Karicheri <m-karicheri2@ti.com>
>>>>> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
>>>>
>>>> Pardon me. How is this GENERAL PURPOSE Input/Output?
>>>>
>>>> It seems very very much SPECIAL PURPOSE to me, it's like
>>>> you're just shoehorning some IPC mechanism into the GPIO
>>>> subsystem, and this may be because the datasheet calls it
>>>> GPIO when it's not.
>>>>
>>>> What other stuff than DSP is connected to these lines, and is it
>>>> really even external lines? Aren't these just polysilicon rails
>>>> pretty much hammered to be used by the DSP and nothing else.
>>>>
>>>> What is the difference between this and a mailbox IRQ line
>>>> and the kind of stuff handled by drivers/mailbox?
>>>>
>>>> I'd like Suman and Jassi to have a look at this to see if it's
>>>> actually a mailbox before we proceed.
>>>>
>>> The controller seems like most others, only incapable of reading
>>> signals (output only).
>>>  The userspace driving those signals to communicate with a DSP isn't
>>> enough to call it a mailbox usecase, because on a different board the
>>> userspace may drive those signals to control LEDs :)
>>>
>> Exactly !!
>> And that was my point. Thanks for echo.
>>
> Yeah but if the AP and DSP are within the same package (i.e, the
> 'pins' can't be used for any other purpose on any board), one might
> sell it as a mailbox. However, since the mailbox protocol driver would
> be in userspace, I think it is justified to expose that as GPIO
> otherwise we'll have to add another interface for userspace to control
> the DSP.

If all these pins or a group of pins are collectively being used to
denote a message to the DSP, then I can see the argument for this being
a mailbox platform driver. It then probably would need to be
supplemented by some other protocol driver to expose the necessary
functionality to userspace. Otherwise, I agree with Jassi in general.

That said, having two different drivers for different GPIO IP instances
within the same SoC doesn't make sense. There should be a single driver
for all the GPIO IPs in keystone, output/input only are properties of
the instance and the driver should handle that IMHO.

regards
Suman
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Santosh Shilimkar July 24, 2014, 11:41 p.m. UTC | #14
On Thursday 24 July 2014 02:52 PM, Suman Anna wrote:
> Hi,
> 
> On 07/24/2014 01:12 PM, Jassi Brar wrote:
>> On 24 July 2014 22:52, Santosh Shilimkar <santosh.shilimkar@ti.com> wrote:
>>> On Thursday 24 July 2014 01:19 PM, Jassi Brar wrote:
>>>> On 23 July 2014 20:40, Linus Walleij <linus.walleij@linaro.org> wrote:
>>>>> On Wed, Jul 16, 2014 at 12:43 PM, Grygorii Strashko
>>>>> <grygorii.strashko@ti.com> wrote:
>>>>>
>>>>>> From: Murali Karicheri <m-karicheri2@ti.com>
>>>>>>
>>>>>> On Keystone SOCs, ARM host can send interrupts to DSP cores using the
>>>>>> DSP GPIO controller IP. Each DSP GPIO controller provides 28 IRQ signals for
>>>>>> each DSP core. This is one of the component used by the IPC mechanism used
>>>>>> on Keystone SOCs.
>>>>>>
>>>>>> Keystone 2 DSP GPIO controller has specific features:
>>>>>> - each GPIO can be configured only as output pin;
>>>>>> - setting GPIO value to 1 causes IRQ generation on target DSP core;
>>>>>> - reading pin value returns 0 - if IRQ was handled or 1 - IRQ is still
>>>>>>   pending.
>>>>>>
>>>>>> Signed-off-by: Murali Karicheri <m-karicheri2@ti.com>
>>>>>> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
>>>>>
>>>>> Pardon me. How is this GENERAL PURPOSE Input/Output?
>>>>>
>>>>> It seems very very much SPECIAL PURPOSE to me, it's like
>>>>> you're just shoehorning some IPC mechanism into the GPIO
>>>>> subsystem, and this may be because the datasheet calls it
>>>>> GPIO when it's not.
>>>>>
>>>>> What other stuff than DSP is connected to these lines, and is it
>>>>> really even external lines? Aren't these just polysilicon rails
>>>>> pretty much hammered to be used by the DSP and nothing else.
>>>>>
>>>>> What is the difference between this and a mailbox IRQ line
>>>>> and the kind of stuff handled by drivers/mailbox?
>>>>>
>>>>> I'd like Suman and Jassi to have a look at this to see if it's
>>>>> actually a mailbox before we proceed.
>>>>>
>>>> The controller seems like most others, only incapable of reading
>>>> signals (output only).
>>>>  The userspace driving those signals to communicate with a DSP isn't
>>>> enough to call it a mailbox usecase, because on a different board the
>>>> userspace may drive those signals to control LEDs :)
>>>>
>>> Exactly !!
>>> And that was my point. Thanks for echo.
>>>
>> Yeah but if the AP and DSP are within the same package (i.e, the
>> 'pins' can't be used for any other purpose on any board), one might
>> sell it as a mailbox. However, since the mailbox protocol driver would
>> be in userspace, I think it is justified to expose that as GPIO
>> otherwise we'll have to add another interface for userspace to control
>> the DSP.
> 
> If all these pins or a group of pins are collectively being used to
> denote a message to the DSP, then I can see the argument for this being
> a mailbox platform driver. It then probably would need to be
> supplemented by some other protocol driver to expose the necessary
> functionality to userspace. Otherwise, I agree with Jassi in general.
>
Just 1 line is used as I mentioned to you off-list already.
 
> That said, having two different drivers for different GPIO IP instances
> within the same SoC doesn't make sense. There should be a single driver
> for all the GPIO IPs in keystone, output/input only are properties of
> the instance and the driver should handle that IMHO.
> 
I wish the hardware integration was the way you said. The one GPIO
controller supports only 32 IO's and thats used for IO's. The IP
block was carried over from Legacy SOCs. The $subject IP block 
has sligtly different programming model, address space, clock domains
etc so there is no question of handling that in 1 driver where the
two hardware IPs are different. Hope its clear why they are in
two separate drivers.

Regards,
Santosh

--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Alexander Shiyan Aug. 11, 2014, 3:26 p.m. UTC | #15
TW9uLCAxMSBBdWcgMjAxNCAxOTowNjo1NCArMDMwMCDQvtGCIEdyeWdvcmlpIFN0cmFzaGtvIDxn
cnlnb3JpaS5zdHJhc2hrb0B0aS5jb20+Ogo+IEhpIEFsZXhhbmRlciwgTGludXMsCj4gCj4gT24g
MDcvMjMvMjAxNCAwNjoxOSBQTSwgQWxleGFuZGVyIFNoaXlhbiB3cm90ZToKPiA+IFdlZCwgMjMg
SnVsIDIwMTQgMTc6MTA6MjYgKzAyMDAg0L7RgiBMaW51cyBXYWxsZWlqIDxsaW51cy53YWxsZWlq
QGxpbmFyby5vcmc+Ogo+ID4+IE9uIFdlZCwgSnVsIDE2LCAyMDE0IGF0IDEyOjQzIFBNLCBHcnln
b3JpaSBTdHJhc2hrbwo+ID4+IDxncnlnb3JpaS5zdHJhc2hrb0B0aS5jb20+IHdyb3RlOgo+ID4+
Cj4gPj4+IEZyb206IE11cmFsaSBLYXJpY2hlcmkgPG0ta2FyaWNoZXJpMkB0aS5jb20+Cj4gPj4+
Cj4gPj4+IE9uIEtleXN0b25lIFNPQ3MsIEFSTSBob3N0IGNhbiBzZW5kIGludGVycnVwdHMgdG8g
RFNQIGNvcmVzIHVzaW5nIHRoZQo+ID4+PiBEU1AgR1BJTyBjb250cm9sbGVyIElQLiBFYWNoIERT
UCBHUElPIGNvbnRyb2xsZXIgcHJvdmlkZXMgMjggSVJRIHNpZ25hbHMgZm9yCj4gPj4+IGVhY2gg
RFNQIGNvcmUuIFRoaXMgaXMgb25lIG9mIHRoZSBjb21wb25lbnQgdXNlZCBieSB0aGUgSVBDIG1l
Y2hhbmlzbSB1c2VkCj4gPj4+IG9uIEtleXN0b25lIFNPQ3MuCj4gPj4+Cj4gPj4+IEtleXN0b25l
IDIgRFNQIEdQSU8gY29udHJvbGxlciBoYXMgc3BlY2lmaWMgZmVhdHVyZXM6Cj4gPj4+IC0gZWFj
aCBHUElPIGNhbiBiZSBjb25maWd1cmVkIG9ubHkgYXMgb3V0cHV0IHBpbjsKPiA+Pj4gLSBzZXR0
aW5nIEdQSU8gdmFsdWUgdG8gMSBjYXVzZXMgSVJRIGdlbmVyYXRpb24gb24gdGFyZ2V0IERTUCBj
b3JlOwo+ID4+PiAtIHJlYWRpbmcgcGluIHZhbHVlIHJldHVybnMgMCAtIGlmIElSUSB3YXMgaGFu
ZGxlZCBvciAxIC0gSVJRIGlzIHN0aWxsCj4gPj4+ICAgIHBlbmRpbmcuCj4gPj4+Cj4gPj4+IFNp
Z25lZC1vZmYtYnk6IE11cmFsaSBLYXJpY2hlcmkgPG0ta2FyaWNoZXJpMkB0aS5jb20+Cj4gPj4+
IFNpZ25lZC1vZmYtYnk6IEdyeWdvcmlpIFN0cmFzaGtvIDxncnlnb3JpaS5zdHJhc2hrb0B0aS5j
b20+Cj4gPiAuLi4KPiA+PiBBbmQgaWYgeW91IHByb2NlZWQgd2l0aCB0aGlzLCBwbGVhc2UgaW50
ZWdyYXRlIGl0IHdpdGgKPiA+PiBkcml2ZXJzL2dwaW8vZ3Bpby1zeXNjb24uYywgSSBkb24ndCBu
ZWVkIG1vcmUgc3BlY2lhbAo+ID4+IHN5c2NvbnMgR1BJTyBoYW5kbGVycy4KPiAKPiBUaGFua3Mg
Zm9yIHlvdXIgY29tbWVudHMuCj4gCj4gSSdtIGdvaW5nIHRvIHVwZGF0ZSBpdCBmb3IgdXNpbmcg
ImdwaW8tc3lzY29uIi4KPiBCdXQgImdwaW8tc3lzY29uIiBkcml2ZXIgaXRzZWxmIHdpbGwgbmVl
ZCB0byBiZSBtb2RpZmllZCB0byBzdXBwb3J0IG91cgo+IGRldmljZSBzcGVjaWZpYyBjYWxsYmFj
a3M6Cj4gLSBncGlvY2hpcC5zZXQoKTogaXQgc2hvdWxkIHNldCBiaXQgMCB0byAxIGFsd2F5cyB0
byBwaHlzaWNhbGx5IGFwcGx5IEdQSU8gdmFsdWVzCj4gLSBncGlvY2hpcC5kaXJlY3Rpb25fb3V0
cHV0KCk6IGl0IHNob3VsZCBkbyBub3RoaW5nIGluIG91ciBjYXNlLCBqdXN0IHJldHVybiAwCj4g
Cj4gU28sIEknbGwgZXh0ZW5kIHN0cnVjdCBzeXNjb25fZ3Bpb19kYXRhIHRvIHN1cHBvcnQgY3Vz
dG9tIGltcGxlbWVudGF0aW9uIG9mCj4gc2V0L2RpcmVjdGlvbl9vdXRwdXQgY2FsbGJhY2tzLiBX
aWxsIGl0IGJlIG9rIGZvciB5b3U/CgpMZXQncyBtYWtlIGEgcGF0Y2ggYW5kIEkgd2lsbCB0ZXN0
IGl0IGZvciBwb3NzaWJsZSByZWdyZXNzaW9ucy4KCi0tLQoK
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Grygorii Strashko Aug. 11, 2014, 4:06 p.m. UTC | #16
Hi Alexander, Linus,

On 07/23/2014 06:19 PM, Alexander Shiyan wrote:
> Wed, 23 Jul 2014 17:10:26 +0200 от Linus Walleij <linus.walleij@linaro.org>:
>> On Wed, Jul 16, 2014 at 12:43 PM, Grygorii Strashko
>> <grygorii.strashko@ti.com> wrote:
>>
>>> From: Murali Karicheri <m-karicheri2@ti.com>
>>>
>>> On Keystone SOCs, ARM host can send interrupts to DSP cores using the
>>> DSP GPIO controller IP. Each DSP GPIO controller provides 28 IRQ signals for
>>> each DSP core. This is one of the component used by the IPC mechanism used
>>> on Keystone SOCs.
>>>
>>> Keystone 2 DSP GPIO controller has specific features:
>>> - each GPIO can be configured only as output pin;
>>> - setting GPIO value to 1 causes IRQ generation on target DSP core;
>>> - reading pin value returns 0 - if IRQ was handled or 1 - IRQ is still
>>>    pending.
>>>
>>> Signed-off-by: Murali Karicheri <m-karicheri2@ti.com>
>>> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
> ...
>> And if you proceed with this, please integrate it with
>> drivers/gpio/gpio-syscon.c, I don't need more special
>> syscons GPIO handlers.

Thanks for your comments.

I'm going to update it for using "gpio-syscon".
But "gpio-syscon" driver itself will need to be modified to support our
device specific callbacks:
- gpiochip.set(): it should set bit 0 to 1 always to physically apply GPIO values
- gpiochip.direction_output(): it should do nothing in our case, just return 0

So, I'll extend struct syscon_gpio_data to support custom implementation of
set/direction_output callbacks. Will it be ok for you?

Best regards,
- grygorii
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/gpio/gpio-keystone.txt b/Documentation/devicetree/bindings/gpio/gpio-keystone.txt
new file mode 100644
index 0000000..4f92af4
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/gpio-keystone.txt
@@ -0,0 +1,43 @@ 
+Keystone 2 DSP GPIO controller bindings
+
+HOST OS userland running on ARM can send interrupts to DSP cores using
+the DSP GPIO controller IP. It provides 28 IRQ signals per each DSP core.
+This is one of the component used by the IPC mechanism used on Keystone SOCs.
+
+For example TCI6638K2K SoC has 8 DSP GPIO controllers:
+ - 8 for C66x CorePacx CPUs 0-7
+
+Keystone 2 DSP GPIO controller has specific features:
+- each GPIO can be configured only as output pin;
+- setting GPIO value to 1 causes IRQ generation on target DSP core;
+- reading pin value returns 0 - if IRQ was handled or 1 - IRQ is still
+  pending.
+
+Required Properties:
+- compatible: should be "ti,keystone-dsp-gpio"
+
+- ti,syscon-dev:	phandle/offset pair. The phandle to syscon used to
+			access device state control registers and the offset
+			in order to use block of device's specific registers.
+
+- gpio-controller : Marks the device node as a gpio controller.
+
+- #gpio-cells : Should be one.
+				See gpio.txt in this directory for a of the cells format
+
+Please refer to gpio.txt in this directory for details of the common GPIO
+bindings used by client devices.
+
+Example:
+	dspgpio0: keystone_dsp_gpio@02620240 {
+		compatible = "ti,keystone-dsp-gpio";
+		ti,syscon-dev = <&devctrl 0x240>;
+		gpio-controller;
+		#gpio-cells = <2>;
+	};
+
+	dsp0: dsp0 {
+		compatible = "linux,rproc-user";
+		...
+		kick-gpio = <&dspgpio0 27>;
+	};
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 4a1b511..990871f 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -158,6 +158,14 @@  config GPIO_EP93XX
 	depends on ARCH_EP93XX
 	select GPIO_GENERIC
 
+config GPIO_KEYSTONE_DSP
+	tristate "Keystone DSP GPIO support"
+	depends on ARCH_KEYSTONE
+	help
+	  Say yes here to support the DSP GPIO driver for Keystone 2. This defines
+	  up to 28 GPIOs per each Remote (DSP) core. This is used to send
+	  signals from ARM to the Remote (DSP) core.
+
 config GPIO_ZEVIO
 	bool "LSI ZEVIO SoC memory mapped GPIOs"
 	depends on ARM && OF_GPIO
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index d10f6a9..15c3389 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -34,6 +34,7 @@  obj-$(CONFIG_GPIO_IOP)		+= gpio-iop.o
 obj-$(CONFIG_GPIO_IT8761E)	+= gpio-it8761e.o
 obj-$(CONFIG_GPIO_JANZ_TTL)	+= gpio-janz-ttl.o
 obj-$(CONFIG_GPIO_KEMPLD)	+= gpio-kempld.o
+obj-$(CONFIG_GPIO_KEYSTONE_DSP)	+= gpio-keystone.o
 obj-$(CONFIG_ARCH_KS8695)	+= gpio-ks8695.o
 obj-$(CONFIG_GPIO_INTEL_MID)	+= gpio-intel-mid.o
 obj-$(CONFIG_GPIO_LP3943)	+= gpio-lp3943.o
diff --git a/drivers/gpio/gpio-keystone.c b/drivers/gpio/gpio-keystone.c
new file mode 100644
index 0000000..988ac0b
--- /dev/null
+++ b/drivers/gpio/gpio-keystone.c
@@ -0,0 +1,138 @@ 
+/*
+ * Keystone 2 DSP GPIO support.
+ *
+ * Copyright (C) 2014 Texas Instruments, Inc.
+ * Author: Murali Karicheri <m-karicheri2@ti.com>
+ *	   Grygorii Strashko <grygorii.strashko@ti.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+#include <linux/module.h>
+#include <linux/of_platform.h>
+#include <linux/gpio.h>
+#include <linux/mfd/syscon.h>
+#include <linux/regmap.h>
+
+/* 28 bits in IPCGRx are treated as GPIO pins to generate interrupt */
+#define GPIOS_PER_BANK		28
+#define GPIO_OFFSET		4
+
+struct keystone_gpio_bank {
+	struct gpio_chip	 chip;
+	struct device		*dev;
+	struct regmap		*devctrl_regs;
+	u32			devctrl_offset;
+};
+#define chip_to_bank(c) \
+	container_of(c, struct keystone_gpio_bank, chip)
+
+static int keystone_gpio_direction_out(struct gpio_chip *c, unsigned ofs, int val)
+{
+	return 0;
+}
+
+static int keystone_gpio_get(struct gpio_chip *c, unsigned ofs)
+{
+	struct keystone_gpio_bank *bank = chip_to_bank(c);
+	int bit = ofs + GPIO_OFFSET;
+	int ret;
+	u32 val = 0;
+
+	ret = regmap_read(bank->devctrl_regs, bank->devctrl_offset, &val);
+	if (ret < 0)
+		dev_dbg(bank->dev, "gpio read failed ret(%d)\n", ret);
+
+	return (val >> bit) & 1;
+}
+
+static void keystone_gpio_set(struct gpio_chip *c, unsigned ofs, int val)
+{
+	struct keystone_gpio_bank *bank = chip_to_bank(c);
+	int bit = ofs + GPIO_OFFSET;
+	int ret;
+
+	if (!val)
+		return;
+
+	ret = regmap_write(bank->devctrl_regs, bank->devctrl_offset,
+			   BIT(bit) | 1);
+	if (ret < 0)
+		dev_dbg(bank->dev, "gpio write failed ret(%d)\n", ret);
+}
+
+static int keystone_gpio_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct device_node *np = dev->of_node;
+	struct keystone_gpio_bank *bank;
+	int ret = 0;
+
+	bank = devm_kzalloc(&pdev->dev, sizeof(*bank), GFP_KERNEL);
+	if (!bank)
+		return -ENOMEM;
+
+	bank->devctrl_regs =
+		syscon_regmap_lookup_by_phandle(np, "ti,syscon-dev");
+	if (IS_ERR(bank->devctrl_regs))
+		return PTR_ERR(bank->devctrl_regs);
+
+	ret = of_property_read_u32_index(np, "ti,syscon-dev", 1,
+					 &bank->devctrl_offset);
+	if (ret) {
+		dev_err(dev, "couldn't read the devctrl_offset offset!\n");
+		return ret;
+	}
+
+	bank->dev			= dev;
+#ifdef CONFIG_OF_GPIO
+	bank->chip.of_node		= of_node_get(np);
+#endif
+	bank->chip.label		= dev_name(dev);
+	bank->chip.get			= keystone_gpio_get;
+	bank->chip.set			= keystone_gpio_set;
+	bank->chip.direction_output	= keystone_gpio_direction_out;
+	bank->chip.base			= -1;
+	bank->chip.ngpio		= GPIOS_PER_BANK;
+
+	platform_set_drvdata(pdev, bank);
+
+	ret = gpiochip_add(&bank->chip);
+	if (ret) {
+		dev_err(dev, "gpio chip registration failed ret(%d)\n", ret);
+		return ret;
+	}
+
+	dev_info(bank->dev, "registered %d gpios\n", bank->chip.ngpio);
+	return ret;
+}
+
+static int keystone_gpio_remove(struct platform_device *pdev)
+{
+	struct keystone_gpio_bank *bank = platform_get_drvdata(pdev);
+
+	return gpiochip_remove(&bank->chip);
+}
+
+static const struct of_device_id keystone_gpio_dt_ids[] = {
+	{ .compatible = "ti,keystone-dsp-gpio", },
+	{ },
+};
+
+static struct platform_driver keystone_gpio_driver = {
+	.probe		= keystone_gpio_probe,
+	.remove		= keystone_gpio_remove,
+	.driver		= {
+		.name	= "keystone-dsp-gpio",
+		.owner	= THIS_MODULE,
+		.of_match_table = keystone_gpio_dt_ids,
+	},
+};
+
+module_platform_driver(keystone_gpio_driver);
+
+MODULE_AUTHOR("Texas Instruments");
+MODULE_AUTHOR("Murali Karicheri <m-karicheri2@ti.com>");
+MODULE_DESCRIPTION("Texas Instruments Keystone 2 DSP GPIO");
+MODULE_LICENSE("GPL v2");