diff mbox

[v3,3/3] gpio: add support for Cypress CYUSBS234 USB-GPIO adapter

Message ID 1412606822-3406-1-git-send-email-muth@cypress.com
State Changes Requested
Headers show

Commit Message

Muthu Mani Oct. 6, 2014, 2:47 p.m. UTC
Adds support for USB-GPIO interface of Cypress Semiconductor
CYUSBS234 USB-Serial Bridge controller.

The GPIO get/set can be done through vendor command on control endpoint
for the configured gpios.

Details about the device can be found at:
http://www.cypress.com/?rID=84126

Signed-off-by: Muthu Mani <muth@cypress.com>
Signed-off-by: Rajaram Regupathy <rera@cypress.com>
---
Changes since v2:
* added helper macros
* removed lock
* given gpio chip device for dev_xxx
* cleaned up the code

Changes since v1:
* allocated memory on heap for usb transfer data
* changed gpio label as platform device name to identify multiple devices

 drivers/gpio/Kconfig          |  13 ++++
 drivers/gpio/Makefile         |   1 +
 drivers/gpio/gpio-cyusbs23x.c | 174 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 188 insertions(+)
 create mode 100644 drivers/gpio/gpio-cyusbs23x.c

Comments

Alexandre Courbot Oct. 7, 2014, 10:03 a.m. UTC | #1
On Mon, Oct 6, 2014 at 11:47 PM, Muthu Mani <muth@cypress.com> wrote:
> +
> +static int cy_gpio_direction_input(struct gpio_chip *chip,
> +                                       unsigned offset)
> +{
> +       return 0;
> +}
> +
> +static int cy_gpio_direction_output(struct gpio_chip *chip,
> +                                       unsigned offset, int value)
> +{
> +       return 0;
> +}

If that chip is capable of both output and input, shouldn't these
functions be implemented? I think this has already been pointed out in
a previous version but you did not reply.

> +
> +static int cyusbs23x_gpio_probe(struct platform_device *pdev)
> +{
> +       struct cyusbs23x *cyusbs;
> +       struct cyusbs_gpio *cy_gpio;
> +       int ret = 0;
> +
> +       dev_dbg(&pdev->dev, "%s\n", __func__);

> +
> +       cyusbs = dev_get_drvdata(pdev->dev.parent);
> +
> +       cy_gpio = devm_kzalloc(&pdev->dev, sizeof(*cy_gpio), GFP_KERNEL);
> +       if (cy_gpio == NULL)
> +               return -ENOMEM;
> +
> +       cy_gpio->cyusbs = cyusbs;
> +       /* registering gpio */
> +       cy_gpio->gpio.label = dev_name(&pdev->dev);
> +       cy_gpio->gpio.dev = &pdev->dev;
> +       cy_gpio->gpio.owner = THIS_MODULE;
> +       cy_gpio->gpio.base = -1;
> +       cy_gpio->gpio.ngpio = 12; /* total GPIOs */

Isn't there a way to get the number of GPIOs from a reliable source?
If not, please at least use a macro here.
--
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
Muthu Mani Oct. 8, 2014, 7:09 a.m. UTC | #2
PiAtLS0tLU9yaWdpbmFsIE1lc3NhZ2UtLS0tLQ0KPiBGcm9tOiBBbGV4YW5kcmUgQ291cmJvdCBb
bWFpbHRvOmdudXJvdUBnbWFpbC5jb21dDQo+IFNlbnQ6IFR1ZXNkYXksIE9jdG9iZXIgMDcsIDIw
MTQgMzozNCBQTQ0KPiBUbzogTXV0aHUgTWFuaQ0KPiBDYzogU2FtdWVsIE9ydGl6OyBMZWUgSm9u
ZXM7IFdvbGZyYW0gU2FuZzsgTGludXMgV2FsbGVpajsgR3JlZyBLcm9haC0NCj4gSGFydG1hbjsg
bGludXgtaTJjQHZnZXIua2VybmVsLm9yZzsgbGludXgtZ3Bpb0B2Z2VyLmtlcm5lbC5vcmc7IGxp
bnV4LQ0KPiB1c2JAdmdlci5rZXJuZWwub3JnOyBMaW51eCBLZXJuZWwgTWFpbGluZyBMaXN0OyBS
YWphcmFtIFJlZ3VwYXRoeTsgSm9oYW4NCj4gSG92b2xkDQo+IFN1YmplY3Q6IFJlOiBbUEFUQ0gg
djMgMy8zXSBncGlvOiBhZGQgc3VwcG9ydCBmb3IgQ3lwcmVzcyBDWVVTQlMyMzQgVVNCLQ0KPiBH
UElPIGFkYXB0ZXINCj4gDQo+IE9uIE1vbiwgT2N0IDYsIDIwMTQgYXQgMTE6NDcgUE0sIE11dGh1
IE1hbmkgPG11dGhAY3lwcmVzcy5jb20+IHdyb3RlOg0KPiA+ICsNCj4gPiArc3RhdGljIGludCBj
eV9ncGlvX2RpcmVjdGlvbl9pbnB1dChzdHJ1Y3QgZ3Bpb19jaGlwICpjaGlwLA0KPiA+ICsgICAg
ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICB1bnNpZ25lZCBvZmZzZXQpIHsNCj4g
PiArICAgICAgIHJldHVybiAwOw0KPiA+ICt9DQo+ID4gKw0KPiA+ICtzdGF0aWMgaW50IGN5X2dw
aW9fZGlyZWN0aW9uX291dHB1dChzdHJ1Y3QgZ3Bpb19jaGlwICpjaGlwLA0KPiA+ICsgICAgICAg
ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICB1bnNpZ25lZCBvZmZzZXQsIGludCB2YWx1
ZSkgew0KPiA+ICsgICAgICAgcmV0dXJuIDA7DQo+ID4gK30NCj4gDQo+IElmIHRoYXQgY2hpcCBp
cyBjYXBhYmxlIG9mIGJvdGggb3V0cHV0IGFuZCBpbnB1dCwgc2hvdWxkbid0IHRoZXNlIGZ1bmN0
aW9ucyBiZQ0KPiBpbXBsZW1lbnRlZD8gSSB0aGluayB0aGlzIGhhcyBhbHJlYWR5IGJlZW4gcG9p
bnRlZCBvdXQgaW4gYSBwcmV2aW91cyB2ZXJzaW9uDQo+IGJ1dCB5b3UgZGlkIG5vdCByZXBseS4N
Cg0KVGhhbmtzIGZvciB5b3VyIGlucHV0cy4NCg0KT25seSB0aGUgR1BJT3Mgd2hpY2ggYXJlIGNv
bmZpZ3VyZWQgdG8gYmUgb3V0cHV0IEdQSU8gY2FuIGJlIHNldC4NClRoZSBzZXQgb3BlcmF0aW9u
IHdvdWxkIGZhaWwgdHJ5aW5nIHRvIHNldCB0aGUgaW5wdXQgb3IgdW5jb25maWd1cmVkIEdQSU9z
Lg0KSW4gdGhpcyB2ZXJzaW9uIG9mIGRyaXZlciwgdGhpcyBzdXBwb3J0IGlzIG5vdCBhZGRlZCwg
aXQgY2FuIGJlIGludHJvZHVjZWQgaW4gZnV0dXJlIHZlcnNpb25zLg0KSSB3aWxsIGFkZCBhIFRP
RE8gbm90ZSBpbiB0aGUgY29kZS4NCg0KPiANCj4gPiArDQo+ID4gK3N0YXRpYyBpbnQgY3l1c2Jz
MjN4X2dwaW9fcHJvYmUoc3RydWN0IHBsYXRmb3JtX2RldmljZSAqcGRldikgew0KPiA+ICsgICAg
ICAgc3RydWN0IGN5dXNiczIzeCAqY3l1c2JzOw0KPiA+ICsgICAgICAgc3RydWN0IGN5dXNic19n
cGlvICpjeV9ncGlvOw0KPiA+ICsgICAgICAgaW50IHJldCA9IDA7DQo+ID4gKw0KPiA+ICsgICAg
ICAgZGV2X2RiZygmcGRldi0+ZGV2LCAiJXNcbiIsIF9fZnVuY19fKTsNCj4gDQo+ID4gKw0KPiA+
ICsgICAgICAgY3l1c2JzID0gZGV2X2dldF9kcnZkYXRhKHBkZXYtPmRldi5wYXJlbnQpOw0KPiA+
ICsNCj4gPiArICAgICAgIGN5X2dwaW8gPSBkZXZtX2t6YWxsb2MoJnBkZXYtPmRldiwgc2l6ZW9m
KCpjeV9ncGlvKSwgR0ZQX0tFUk5FTCk7DQo+ID4gKyAgICAgICBpZiAoY3lfZ3BpbyA9PSBOVUxM
KQ0KPiA+ICsgICAgICAgICAgICAgICByZXR1cm4gLUVOT01FTTsNCj4gPiArDQo+ID4gKyAgICAg
ICBjeV9ncGlvLT5jeXVzYnMgPSBjeXVzYnM7DQo+ID4gKyAgICAgICAvKiByZWdpc3RlcmluZyBn
cGlvICovDQo+ID4gKyAgICAgICBjeV9ncGlvLT5ncGlvLmxhYmVsID0gZGV2X25hbWUoJnBkZXYt
PmRldik7DQo+ID4gKyAgICAgICBjeV9ncGlvLT5ncGlvLmRldiA9ICZwZGV2LT5kZXY7DQo+ID4g
KyAgICAgICBjeV9ncGlvLT5ncGlvLm93bmVyID0gVEhJU19NT0RVTEU7DQo+ID4gKyAgICAgICBj
eV9ncGlvLT5ncGlvLmJhc2UgPSAtMTsNCj4gPiArICAgICAgIGN5X2dwaW8tPmdwaW8ubmdwaW8g
PSAxMjsgLyogdG90YWwgR1BJT3MgKi8NCj4gDQo+IElzbid0IHRoZXJlIGEgd2F5IHRvIGdldCB0
aGUgbnVtYmVyIG9mIEdQSU9zIGZyb20gYSByZWxpYWJsZSBzb3VyY2U/DQo+IElmIG5vdCwgcGxl
YXNlIGF0IGxlYXN0IHVzZSBhIG1hY3JvIGhlcmUuDQoNCkNZVVNCUzIzNCBoYXMgMTIgR1BJT3Mg
aW4gdG90YWwgYXMgcGVyIHRoZSBkYXRhc2hlZXQuIEkgd2lsbCBhZGQgYSBtYWNybyBhbmQgdXNl
IGl0Lg0K
--
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
Alexandre Courbot Oct. 8, 2014, 7:18 a.m. UTC | #3
On Wed, Oct 8, 2014 at 4:09 PM, Muthu Mani <muth@cypress.com> wrote:
>> -----Original Message-----
>> From: Alexandre Courbot [mailto:gnurou@gmail.com]
>> Sent: Tuesday, October 07, 2014 3:34 PM
>> To: Muthu Mani
>> Cc: Samuel Ortiz; Lee Jones; Wolfram Sang; Linus Walleij; Greg Kroah-
>> Hartman; linux-i2c@vger.kernel.org; linux-gpio@vger.kernel.org; linux-
>> usb@vger.kernel.org; Linux Kernel Mailing List; Rajaram Regupathy; Johan
>> Hovold
>> Subject: Re: [PATCH v3 3/3] gpio: add support for Cypress CYUSBS234 USB-
>> GPIO adapter
>>
>> On Mon, Oct 6, 2014 at 11:47 PM, Muthu Mani <muth@cypress.com> wrote:
>> > +
>> > +static int cy_gpio_direction_input(struct gpio_chip *chip,
>> > +                                       unsigned offset) {
>> > +       return 0;
>> > +}
>> > +
>> > +static int cy_gpio_direction_output(struct gpio_chip *chip,
>> > +                                       unsigned offset, int value) {
>> > +       return 0;
>> > +}
>>
>> If that chip is capable of both output and input, shouldn't these functions be
>> implemented? I think this has already been pointed out in a previous version
>> but you did not reply.
>
> Thanks for your inputs.
>
> Only the GPIOs which are configured to be output GPIO can be set.

In that case cy_gpio_set() should return an error for GPIOs which are
not configured as outputs. Is that guaranteed by the current
implementation?

> The set operation would fail trying to set the input or unconfigured GPIOs.
> In this version of driver, this support is not added, it can be introduced in future versions.
> I will add a TODO note in the code.

Argh, no TODO please. Actual code that will turn this code into a
solid driver that can be merged.

Can all GPIOs be set as input or output? If so, please implement
cy_gpio_direction_*() and make sure that cy_gpio_set() behaves
properly if a GPIO is not an output. If the input/output GPIOs are
fixed, please make cy_gpio_direction_*() return an error if the GPIO
capabilities does not correspond to what is asked, and again ensure
that cy_gpio_set() works as expected.
--
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
RR Oct. 8, 2014, 11:46 p.m. UTC | #4
On Wed, Oct 8, 2014 at 12:18 AM, Alexandre Courbot <gnurou@gmail.com> wrote:
> On Wed, Oct 8, 2014 at 4:09 PM, Muthu Mani <muth@cypress.com> wrote:
>>> -----Original Message-----
>>> From: Alexandre Courbot [mailto:gnurou@gmail.com]
>>> Sent: Tuesday, October 07, 2014 3:34 PM
>>> To: Muthu Mani
>>> Cc: Samuel Ortiz; Lee Jones; Wolfram Sang; Linus Walleij; Greg Kroah-
>>> Hartman; linux-i2c@vger.kernel.org; linux-gpio@vger.kernel.org; linux-
>>> usb@vger.kernel.org; Linux Kernel Mailing List; Rajaram Regupathy; Johan
>>> Hovold
>>> Subject: Re: [PATCH v3 3/3] gpio: add support for Cypress CYUSBS234 USB-
>>> GPIO adapter
>>>
>>> On Mon, Oct 6, 2014 at 11:47 PM, Muthu Mani <muth@cypress.com> wrote:
>>> > +
>>> > +static int cy_gpio_direction_input(struct gpio_chip *chip,
>>> > +                                       unsigned offset) {
>>> > +       return 0;
>>> > +}
>>> > +
>>> > +static int cy_gpio_direction_output(struct gpio_chip *chip,
>>> > +                                       unsigned offset, int value) {
>>> > +       return 0;
>>> > +}
>>>
>>> If that chip is capable of both output and input, shouldn't these functions be
>>> implemented? I think this has already been pointed out in a previous version
>>> but you did not reply.
>>
>> Thanks for your inputs.
>>
>> Only the GPIOs which are configured to be output GPIO can be set.
>
> In that case cy_gpio_set() should return an error for GPIOs which are
> not configured as outputs. Is that guaranteed by the current
> implementation?
>
>> The set operation would fail trying to set the input or unconfigured GPIOs.
>> In this version of driver, this support is not added, it can be introduced in future versions.
>> I will add a TODO note in the code.
>
> Argh, no TODO please. Actual code that will turn this code into a
> solid driver that can be merged.

Does a driver targeted for a custom device has to implement every
functionality in the 1st version ? My understanding is that Linux
follows incremental model and allows incremental merge.

(On a side note the driver is functionally verified with the necessary hardware)

Please correct me if I am missing something

>
> Can all GPIOs be set as input or output? If so, please implement
> cy_gpio_direction_*() and make sure that cy_gpio_set() behaves
> properly if a GPIO is not an output. If the input/output GPIOs are
> fixed, please make cy_gpio_direction_*() return an error if the GPIO
> capabilities does not correspond to what is asked, and again ensure
> that cy_gpio_set() works as expected.
> --
> 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/
--
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
Alexandre Courbot Oct. 9, 2014, 7:59 a.m. UTC | #5
On Thu, Oct 9, 2014 at 8:46 AM, RR <rajaram.officemails@gmail.com> wrote:
> On Wed, Oct 8, 2014 at 12:18 AM, Alexandre Courbot <gnurou@gmail.com> wrote:
>> On Wed, Oct 8, 2014 at 4:09 PM, Muthu Mani <muth@cypress.com> wrote:
>>>> -----Original Message-----
>>>> From: Alexandre Courbot [mailto:gnurou@gmail.com]
>>>> Sent: Tuesday, October 07, 2014 3:34 PM
>>>> To: Muthu Mani
>>>> Cc: Samuel Ortiz; Lee Jones; Wolfram Sang; Linus Walleij; Greg Kroah-
>>>> Hartman; linux-i2c@vger.kernel.org; linux-gpio@vger.kernel.org; linux-
>>>> usb@vger.kernel.org; Linux Kernel Mailing List; Rajaram Regupathy; Johan
>>>> Hovold
>>>> Subject: Re: [PATCH v3 3/3] gpio: add support for Cypress CYUSBS234 USB-
>>>> GPIO adapter
>>>>
>>>> On Mon, Oct 6, 2014 at 11:47 PM, Muthu Mani <muth@cypress.com> wrote:
>>>> > +
>>>> > +static int cy_gpio_direction_input(struct gpio_chip *chip,
>>>> > +                                       unsigned offset) {
>>>> > +       return 0;
>>>> > +}
>>>> > +
>>>> > +static int cy_gpio_direction_output(struct gpio_chip *chip,
>>>> > +                                       unsigned offset, int value) {
>>>> > +       return 0;
>>>> > +}
>>>>
>>>> If that chip is capable of both output and input, shouldn't these functions be
>>>> implemented? I think this has already been pointed out in a previous version
>>>> but you did not reply.
>>>
>>> Thanks for your inputs.
>>>
>>> Only the GPIOs which are configured to be output GPIO can be set.
>>
>> In that case cy_gpio_set() should return an error for GPIOs which are
>> not configured as outputs. Is that guaranteed by the current
>> implementation?
>>
>>> The set operation would fail trying to set the input or unconfigured GPIOs.
>>> In this version of driver, this support is not added, it can be introduced in future versions.
>>> I will add a TODO note in the code.
>>
>> Argh, no TODO please. Actual code that will turn this code into a
>> solid driver that can be merged.
>
> Does a driver targeted for a custom device has to implement every
> functionality in the 1st version ? My understanding is that Linux
> follows incremental model and allows incremental merge.

Certainly, but we are talking about very basic functionality here.

Right now this driver will incorrectly returns success when trying to
set the direction of a GPIO, even though it did not do anything. There
is also no hint that _set will return an error if called on an
input/unconfigured GPIO.

So while we are not asking for a driver to be perfect on the first
iteration, setting the direction of a GPIO is a very basic
functionality that is essential for a GPIO driver to just work. Custom
device or not, this is mainline. The rest of the driver seems to be
ok, so please take the last few steps that will allow us to merge a
featured driver.
--
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 Oct. 27, 2014, 3:30 p.m. UTC | #6
On Thu, Oct 9, 2014 at 1:46 AM, RR <rajaram.officemails@gmail.com> wrote:
> On Wed, Oct 8, 2014 at 12:18 AM, Alexandre Courbot <gnurou@gmail.com> wrote:
>> On Wed, Oct 8, 2014 at 4:09 PM, Muthu Mani <muth@cypress.com> wrote:

>>>> > +static int cy_gpio_direction_output(struct gpio_chip *chip,
>>>> > +                                       unsigned offset, int value) {
>>>> > +       return 0;
>>>> > +}
>>>>
>>>> If that chip is capable of both output and input, shouldn't these functions be
>>>> implemented? I think this has already been pointed out in a previous version
>>>> but you did not reply.
>>>
>>> Thanks for your inputs.
>>>
>>> Only the GPIOs which are configured to be output GPIO can be set.
>>
>> In that case cy_gpio_set() should return an error for GPIOs which are
>> not configured as outputs. Is that guaranteed by the current
>> implementation?
>>
>>> The set operation would fail trying to set the input or unconfigured GPIOs.
>>> In this version of driver, this support is not added, it can be introduced in future versions.
>>> I will add a TODO note in the code.
>>
>> Argh, no TODO please. Actual code that will turn this code into a
>> solid driver that can be merged.
>
> Does a driver targeted for a custom device has to implement every
> functionality in the 1st version ?

When you post a driver to the GPIO maintainers it is *NOT* tageted
at a consumer device, it is targeted at the kernel community and
upstream maintainers.

Of course you can deliver add-on patches out-of-tree to your
customers, it's generally a bad idea for the long term and maintenance
of your driver, but it's your pick.

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
RR Oct. 29, 2014, 2:21 a.m. UTC | #7
On Mon, Oct 27, 2014 at 9:00 PM, Linus Walleij <linus.walleij@linaro.org> wrote:
> On Thu, Oct 9, 2014 at 1:46 AM, RR <rajaram.officemails@gmail.com> wrote:
>> On Wed, Oct 8, 2014 at 12:18 AM, Alexandre Courbot <gnurou@gmail.com> wrote:
>>> On Wed, Oct 8, 2014 at 4:09 PM, Muthu Mani <muth@cypress.com> wrote:
>
>>>>> > +static int cy_gpio_direction_output(struct gpio_chip *chip,
>>>>> > +                                       unsigned offset, int value) {
>>>>> > +       return 0;
>>>>> > +}
>>>>>
>>>>> If that chip is capable of both output and input, shouldn't these functions be
>>>>> implemented? I think this has already been pointed out in a previous version
>>>>> but you did not reply.
>>>>
>>>> Thanks for your inputs.
>>>>
>>>> Only the GPIOs which are configured to be output GPIO can be set.
>>>
>>> In that case cy_gpio_set() should return an error for GPIOs which are
>>> not configured as outputs. Is that guaranteed by the current
>>> implementation?
>>>
>>>> The set operation would fail trying to set the input or unconfigured GPIOs.
>>>> In this version of driver, this support is not added, it can be introduced in future versions.
>>>> I will add a TODO note in the code.
>>>
>>> Argh, no TODO please. Actual code that will turn this code into a
>>> solid driver that can be merged.
>>
>> Does a driver targeted for a custom device has to implement every
>> functionality in the 1st version ?
>
> When you post a driver to the GPIO maintainers it is *NOT* tageted
> at a consumer device, it is targeted at the kernel community and
> upstream maintainers.

Totally agree. What I was conveying the patch has not modified
any "core" kernel function and is specific to a device thus will not
affect system.

>
> Of course you can deliver add-on patches out-of-tree to your
> customers, it's generally a bad idea for the long term and maintenance
> of your driver, but it's your pick.

AFAIR In the recent past xHCI or gadget core or musb or dw3
patches were added in increments. May be my analogy is incorrect and
I am ignorant of some philosophy here.

Sincerely I somehow was not convinced basic functionality is missing
as referred in the review comment.We have tested the driver for most of
the functionality of our DVK and is working perfectly.

Moreover currently we do not expect an user to set gpio direction as
it involves vendor specific usb control commands.

Having said that we have taken the feedback and working to close this.

>
> 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
diff mbox

Patch

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 9de1515..932e07c 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -886,6 +886,19 @@  config GPIO_BCM_KONA
 
 comment "USB GPIO expanders:"
 
+config GPIO_CYUSBS23X
+	tristate "CYUSBS23x GPIO support"
+	depends on MFD_CYUSBS23X && USB
+	help
+	  Say yes here to access the GPIO signals of Cypress
+	  Semiconductor CYUSBS23x USB Serial Bridge Controller.
+
+	  This driver enables the GPIO interface of CYUSBS23x USB Serial
+	  Bridge controller.
+
+	  This driver can also be built as a module. If so, the module will be
+	  called gpio-cyusbs23x.
+
 config GPIO_VIPERBOARD
 	tristate "Viperboard GPIO a & b support"
 	depends on MFD_VIPERBOARD && USB
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index 5d024e3..3ad89f1 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -23,6 +23,7 @@  obj-$(CONFIG_GPIO_BT8XX)	+= gpio-bt8xx.o
 obj-$(CONFIG_GPIO_CLPS711X)	+= gpio-clps711x.o
 obj-$(CONFIG_GPIO_CS5535)	+= gpio-cs5535.o
 obj-$(CONFIG_GPIO_CRYSTAL_COVE)	+= gpio-crystalcove.o
+obj-$(CONFIG_GPIO_CYUSBS23X)	+= gpio-cyusbs23x.o
 obj-$(CONFIG_GPIO_DA9052)	+= gpio-da9052.o
 obj-$(CONFIG_GPIO_DA9055)	+= gpio-da9055.o
 obj-$(CONFIG_GPIO_DAVINCI)	+= gpio-davinci.o
diff --git a/drivers/gpio/gpio-cyusbs23x.c b/drivers/gpio/gpio-cyusbs23x.c
new file mode 100644
index 0000000..f2acbb8
--- /dev/null
+++ b/drivers/gpio/gpio-cyusbs23x.c
@@ -0,0 +1,174 @@ 
+/*
+ * GPIO subdriver for Cypress CYUSBS234 USB-Serial Bridge controller.
+ * Details about the device can be found at:
+ *    http://www.cypress.com/?rID=84126
+ *
+ * Copyright (c) 2014 Cypress Semiconductor Corporation.
+ *
+ * Author:
+ *   Muthu Mani <muth@cypress.com>
+ *
+ * Additional contributors include:
+ *   Rajaram Regupathy <rera@cypress.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.
+ */
+
+/*
+ * All GPIOs are exposed for get operation. Only the GPIOs which are configured
+ * by the user using the Configuration Utility can be set. Attempting to set
+ * value of unconfigured GPIOs would fail
+ */
+
+#include <linux/kernel.h>
+#include <linux/errno.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/types.h>
+#include <linux/mutex.h>
+#include <linux/platform_device.h>
+
+#include <linux/usb.h>
+#include <linux/gpio.h>
+
+#include <linux/mfd/cyusbs23x.h>
+
+#define CY_GPIO_GET_LEN		2
+
+struct cyusbs_gpio {
+	struct gpio_chip gpio;
+	struct cyusbs23x *cyusbs;
+};
+
+#define to_cyusbs_gpio(chip) container_of(chip, struct cyusbs_gpio, gpio)
+
+static int cy_gpio_get(struct gpio_chip *chip,
+			unsigned offset)
+{
+	int ret;
+	char *buf;
+	u16 wIndex, wValue;
+	struct cyusbs_gpio *gpio = to_cyusbs_gpio(chip);
+	struct cyusbs23x *cyusbs = gpio->cyusbs;
+
+	wValue = offset;
+	wIndex = 0;
+	buf = kmalloc(CY_GPIO_GET_LEN, GFP_KERNEL);
+	if (buf == NULL)
+		return -ENOMEM;
+
+	ret = usb_control_msg(cyusbs->usb_dev,
+			usb_rcvctrlpipe(cyusbs->usb_dev, 0),
+			CY_GPIO_GET_VALUE_CMD,
+			USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_IN,
+			wValue, wIndex, buf, CY_GPIO_GET_LEN,
+			CY_USBS_CTRL_XFER_TIMEOUT);
+	if (ret == CY_GPIO_GET_LEN) {
+		dev_dbg(chip->dev, "%s: %02X %02X\n", __func__, buf[0], buf[1]);
+		if (buf[0] == 0)
+			ret = buf[1];
+		else
+			ret = -EIO;
+	} else {
+		dev_err(chip->dev, "%s: %d\n", __func__, ret);
+		ret = -EIO;
+	}
+
+	kfree(buf);
+	return ret;
+}
+
+static void cy_gpio_set(struct gpio_chip *chip,
+			unsigned offset, int value)
+{
+	int ret;
+	u16 wIndex, wValue;
+	struct cyusbs_gpio *gpio = to_cyusbs_gpio(chip);
+	struct cyusbs23x *cyusbs = gpio->cyusbs;
+
+	wValue = offset;
+	wIndex = value;
+
+	ret = usb_control_msg(cyusbs->usb_dev,
+			usb_sndctrlpipe(cyusbs->usb_dev, 0),
+			CY_GPIO_SET_VALUE_CMD,
+			USB_TYPE_VENDOR | USB_RECIP_DEVICE | USB_DIR_IN,
+			wValue, wIndex, NULL, 0, CY_USBS_CTRL_XFER_TIMEOUT);
+	if (ret < 0)
+		dev_err(chip->dev, "error setting gpio: %d\n", ret);
+}
+
+static int cy_gpio_direction_input(struct gpio_chip *chip,
+					unsigned offset)
+{
+	return 0;
+}
+
+static int cy_gpio_direction_output(struct gpio_chip *chip,
+					unsigned offset, int value)
+{
+	return 0;
+}
+
+static int cyusbs23x_gpio_probe(struct platform_device *pdev)
+{
+	struct cyusbs23x *cyusbs;
+	struct cyusbs_gpio *cy_gpio;
+	int ret = 0;
+
+	dev_dbg(&pdev->dev, "%s\n", __func__);
+
+	cyusbs = dev_get_drvdata(pdev->dev.parent);
+
+	cy_gpio = devm_kzalloc(&pdev->dev, sizeof(*cy_gpio), GFP_KERNEL);
+	if (cy_gpio == NULL)
+		return -ENOMEM;
+
+	cy_gpio->cyusbs = cyusbs;
+	/* registering gpio */
+	cy_gpio->gpio.label = dev_name(&pdev->dev);
+	cy_gpio->gpio.dev = &pdev->dev;
+	cy_gpio->gpio.owner = THIS_MODULE;
+	cy_gpio->gpio.base = -1;
+	cy_gpio->gpio.ngpio = 12; /* total GPIOs */
+	cy_gpio->gpio.can_sleep = true;
+	cy_gpio->gpio.set = cy_gpio_set;
+	cy_gpio->gpio.get = cy_gpio_get;
+	cy_gpio->gpio.direction_input = cy_gpio_direction_input;
+	cy_gpio->gpio.direction_output = cy_gpio_direction_output;
+	ret = gpiochip_add(&cy_gpio->gpio);
+	if (ret < 0) {
+		dev_err(cy_gpio->gpio.dev, "could not add gpio\n");
+		return ret;
+	}
+
+	platform_set_drvdata(pdev, cy_gpio);
+
+	dev_dbg(&pdev->dev, "added GPIO\n");
+	return ret;
+}
+
+static int cyusbs23x_gpio_remove(struct platform_device *pdev)
+{
+	struct cyusbs_gpio *cy_gpio = platform_get_drvdata(pdev);
+
+	dev_dbg(&pdev->dev, "%s\n", __func__);
+	gpiochip_remove(&cy_gpio->gpio);
+	return 0;
+}
+
+static struct platform_driver cyusbs23x_gpio_driver = {
+	.driver.name    = "cyusbs23x-gpio",
+	.probe          = cyusbs23x_gpio_probe,
+	.remove         = cyusbs23x_gpio_remove,
+};
+
+module_platform_driver(cyusbs23x_gpio_driver);
+
+MODULE_AUTHOR("Rajaram Regupathy <rera@cypress.com>");
+MODULE_AUTHOR("Muthu Mani <muth@cypress.com>");
+MODULE_DESCRIPTION("GPIO driver for CYUSBS23x");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:cyusbs23x-gpio");