Message ID | 1405507426-18992-1-git-send-email-grygorii.strashko@ti.com |
---|---|
State | New |
Headers | show |
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
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
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/
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. ---
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
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
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
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
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
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
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
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
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
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
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
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 --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");