Re: [PATCH v7 1/3] gpio: exar: add gpio for exar cards

2017-01-09 Thread Sudip Mukherjee 

On Monday 09 January 2017 10:35 AM, Linus Walleij wrote:

On Sun, Jan 8, 2017 at 12:57 AM, Sudip Mukherjee
 wrote:


Exar XR17V352/354/358 chips have 16 multi-purpose inputs/outputs which
can be controlled using gpio interface.

Add the gpio specific code.

Signed-off-by: Sudip Mukherjee 


Will I be able to merge this independently to the GPIO trees
once we are done with review? (Looks like so...)


Yes, there should not be any dependency on the tty.




+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt


Is this really useful?


no, initially I used pr_*, but then that was converted to dev_*.




+#include 
+#include 


No use:
#include 
ONLY


+static LIST_HEAD(exar_list);
+static DEFINE_MUTEX(exar_list_mtx);
+DEFINE_IDA(ida_index);


What is this? A local list? I can understand the IDA index but in
general, follow the state container pattern instead:
Documentation/driver-model/design-patterns.txt


The local list is not doing anything now, after I have moved to using 
ida. But I will need the ida_index here to have the device number incase 
of multiple devices.


regards
sudip

Re: [PATCH v7 1/3] gpio: exar: add gpio for exar cards

2017-01-09 Thread Sudip Mukherjee 

On Monday 09 January 2017 10:35 AM, Linus Walleij wrote:

On Sun, Jan 8, 2017 at 12:57 AM, Sudip Mukherjee
 wrote:


Exar XR17V352/354/358 chips have 16 multi-purpose inputs/outputs which
can be controlled using gpio interface.

Add the gpio specific code.

Signed-off-by: Sudip Mukherjee 


Will I be able to merge this independently to the GPIO trees
once we are done with review? (Looks like so...)


Yes, there should not be any dependency on the tty.




+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt


Is this really useful?


no, initially I used pr_*, but then that was converted to dev_*.




+#include 
+#include 


No use:
#include 
ONLY


+static LIST_HEAD(exar_list);
+static DEFINE_MUTEX(exar_list_mtx);
+DEFINE_IDA(ida_index);


What is this? A local list? I can understand the IDA index but in
general, follow the state container pattern instead:
Documentation/driver-model/design-patterns.txt


The local list is not doing anything now, after I have moved to using 
ida. But I will need the ida_index here to have the device number incase 
of multiple devices.


regards
sudip

Re: [PATCH v7 1/3] gpio: exar: add gpio for exar cards

2017-01-09 Thread Linus Walleij 
On Sun, Jan 8, 2017 at 12:57 AM, Sudip Mukherjee
 wrote:

> Exar XR17V352/354/358 chips have 16 multi-purpose inputs/outputs which
> can be controlled using gpio interface.
>
> Add the gpio specific code.
>
> Signed-off-by: Sudip Mukherjee 

Will I be able to merge this independently to the GPIO trees
once we are done with review? (Looks like so...)

> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt

Is this really useful?

> +#include 
> +#include 

No use:
#include 
ONLY

> +static LIST_HEAD(exar_list);
> +static DEFINE_MUTEX(exar_list_mtx);
> +DEFINE_IDA(ida_index);

What is this? A local list? I can understand the IDA index but in
general, follow the state container pattern instead:
Documentation/driver-model/design-patterns.txt

> +#define to_exar_chip(n) container_of(n, struct exar_gpio_chip, gpio_chip)

Don't do this, use
gpiochip_get_data()

> +static inline unsigned int read_exar_reg(struct exar_gpio_chip *chip,
> +int offset)
> +{
> +   dev_dbg(chip->gpio_chip.parent, "regs=%p offset=%x\n",
> +   chip->regs, offset);
> +
> +   return readb(chip->regs + offset);
> +}
> +
> +static inline void write_exar_reg(struct exar_gpio_chip *chip, int offset,
> + int value)
> +{
> +   dev_dbg(chip->gpio_chip.parent,
> +   "regs=%p value=%x offset=%x\n", chip->regs, value,
> +   offset);
> +   writeb(value, chip->regs + offset);
> +}


I don't see why these need their own accessor functions, just inline
the readb()/writeb() calls.

> +static void exar_update(struct gpio_chip *chip, unsigned int reg, int val,
> +   unsigned int offset)
> +{
> +   struct exar_gpio_chip *exar_gpio = to_exar_chip(chip);

So just:

struct exar_gpio_chip *exar_gpio = gpiochip_get_data(chip);

> +   int temp;
> +
> +   mutex_lock(_gpio->lock);
> +   temp = read_exar_reg(exar_gpio, reg);
> +   temp &= ~(1 << offset);
> +   temp |= val << offset;

Use:
#include 

temp &= BIT(offset);
if (val)
  temp |= BIT(offset);

> +static int exar_get(struct gpio_chip *chip, unsigned int reg)
> +{
> +   struct exar_gpio_chip *exar_gpio = to_exar_chip(chip);
> +   int value;
> +
> +   mutex_lock(_gpio->lock);
> +   value = read_exar_reg(exar_gpio, reg);
> +   mutex_unlock(_gpio->lock);
> +
> +   return value;

Please use:

return !!value;

To clamp to bool.

> +static int exar_get_direction(struct gpio_chip *chip, unsigned int offset)
> +{
> +   int val;
> +
> +   if (offset < 8)
> +   val = exar_get(chip, EXAR_OFFSET_MPIOSEL_LO) >> offset;
> +   else
> +   val = exar_get(chip, EXAR_OFFSET_MPIOSEL_HI) >>
> +  (offset - 8);
> +
> +   return val & 0x01;

Just

return !!val;

or something.

> +static int exar_get_value(struct gpio_chip *chip, unsigned int offset)
> +{
> +   int val;
> +
> +   if (offset < 8)
> +   val = exar_get(chip, EXAR_OFFSET_MPIOLVL_LO) >> offset;
> +   else
> +   val = exar_get(chip, EXAR_OFFSET_MPIOLVL_HI) >>
> +  (offset - 8);
> +   return val & 0x01;
> +}

Dito.

> +static int gpio_exar_probe(struct platform_device *pdev)
> +{
> +   struct pci_dev *dev = platform_get_drvdata(pdev);
> +   struct exar_gpio_chip *exar_gpio;
> +   void __iomem *p;
> +   int index = 1;
> +   int ret;
> +
> +   if (dev->vendor != PCI_VENDOR_ID_EXAR)
> +   return -ENODEV;
> +
> +   p = pci_ioremap_bar(dev, 0);
> +   if (!p)
> +   return -ENOMEM;
> +
> +   exar_gpio = devm_kzalloc(>dev, sizeof(*exar_gpio), GFP_KERNEL);
> +   if (!exar_gpio)
> +   return -ENOMEM;
> +
> +   mutex_init(_gpio->lock);
> +   INIT_LIST_HEAD(_gpio->list);
> +
> +   index = ida_simple_get(_index, 0, 0, GFP_KERNEL);
> +   mutex_lock(_list_mtx);

This looks overkill.

> +   sprintf(exar_gpio->name, "exar_gpio%d", index);
> +   exar_gpio->gpio_chip.label = exar_gpio->name;
> +   exar_gpio->gpio_chip.parent = >dev;
> +   exar_gpio->gpio_chip.direction_output = exar_direction_output;
> +   exar_gpio->gpio_chip.direction_input = exar_direction_input;
> +   exar_gpio->gpio_chip.get_direction = exar_get_direction;
> +   exar_gpio->gpio_chip.get = exar_get_value;
> +   exar_gpio->gpio_chip.set = exar_set_value;
> +   exar_gpio->gpio_chip.base = -1;
> +   exar_gpio->gpio_chip.ngpio = 16;
> +   exar_gpio->regs = p;
> +   exar_gpio->index = index;
> +   exar_gpio->pcidev = dev;
> +
> +   ret = gpiochip_add(_gpio->gpio_chip);

Use devm_gpiochip_add_data(dev, _gpio->gpio_chip, exar_gpio)

So you can later use gpiochip_get_data()

> +static int gpio_exar_remove(struct platform_device *pdev)
> +{
> +   struct exar_gpio_chip *exar_gpio, *exar_temp;
> +   struct

Re: [PATCH v7 1/3] gpio: exar: add gpio for exar cards

2017-01-09 Thread Linus Walleij 
On Sun, Jan 8, 2017 at 12:57 AM, Sudip Mukherjee
 wrote:

> Exar XR17V352/354/358 chips have 16 multi-purpose inputs/outputs which
> can be controlled using gpio interface.
>
> Add the gpio specific code.
>
> Signed-off-by: Sudip Mukherjee 

Will I be able to merge this independently to the GPIO trees
once we are done with review? (Looks like so...)

> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt

Is this really useful?

> +#include 
> +#include 

No use:
#include 
ONLY

> +static LIST_HEAD(exar_list);
> +static DEFINE_MUTEX(exar_list_mtx);
> +DEFINE_IDA(ida_index);

What is this? A local list? I can understand the IDA index but in
general, follow the state container pattern instead:
Documentation/driver-model/design-patterns.txt

> +#define to_exar_chip(n) container_of(n, struct exar_gpio_chip, gpio_chip)

Don't do this, use
gpiochip_get_data()

> +static inline unsigned int read_exar_reg(struct exar_gpio_chip *chip,
> +int offset)
> +{
> +   dev_dbg(chip->gpio_chip.parent, "regs=%p offset=%x\n",
> +   chip->regs, offset);
> +
> +   return readb(chip->regs + offset);
> +}
> +
> +static inline void write_exar_reg(struct exar_gpio_chip *chip, int offset,
> + int value)
> +{
> +   dev_dbg(chip->gpio_chip.parent,
> +   "regs=%p value=%x offset=%x\n", chip->regs, value,
> +   offset);
> +   writeb(value, chip->regs + offset);
> +}


I don't see why these need their own accessor functions, just inline
the readb()/writeb() calls.

> +static void exar_update(struct gpio_chip *chip, unsigned int reg, int val,
> +   unsigned int offset)
> +{
> +   struct exar_gpio_chip *exar_gpio = to_exar_chip(chip);

So just:

struct exar_gpio_chip *exar_gpio = gpiochip_get_data(chip);

> +   int temp;
> +
> +   mutex_lock(_gpio->lock);
> +   temp = read_exar_reg(exar_gpio, reg);
> +   temp &= ~(1 << offset);
> +   temp |= val << offset;

Use:
#include 

temp &= BIT(offset);
if (val)
  temp |= BIT(offset);

> +static int exar_get(struct gpio_chip *chip, unsigned int reg)
> +{
> +   struct exar_gpio_chip *exar_gpio = to_exar_chip(chip);
> +   int value;
> +
> +   mutex_lock(_gpio->lock);
> +   value = read_exar_reg(exar_gpio, reg);
> +   mutex_unlock(_gpio->lock);
> +
> +   return value;

Please use:

return !!value;

To clamp to bool.

> +static int exar_get_direction(struct gpio_chip *chip, unsigned int offset)
> +{
> +   int val;
> +
> +   if (offset < 8)
> +   val = exar_get(chip, EXAR_OFFSET_MPIOSEL_LO) >> offset;
> +   else
> +   val = exar_get(chip, EXAR_OFFSET_MPIOSEL_HI) >>
> +  (offset - 8);
> +
> +   return val & 0x01;

Just

return !!val;

or something.

> +static int exar_get_value(struct gpio_chip *chip, unsigned int offset)
> +{
> +   int val;
> +
> +   if (offset < 8)
> +   val = exar_get(chip, EXAR_OFFSET_MPIOLVL_LO) >> offset;
> +   else
> +   val = exar_get(chip, EXAR_OFFSET_MPIOLVL_HI) >>
> +  (offset - 8);
> +   return val & 0x01;
> +}

Dito.

> +static int gpio_exar_probe(struct platform_device *pdev)
> +{
> +   struct pci_dev *dev = platform_get_drvdata(pdev);
> +   struct exar_gpio_chip *exar_gpio;
> +   void __iomem *p;
> +   int index = 1;
> +   int ret;
> +
> +   if (dev->vendor != PCI_VENDOR_ID_EXAR)
> +   return -ENODEV;
> +
> +   p = pci_ioremap_bar(dev, 0);
> +   if (!p)
> +   return -ENOMEM;
> +
> +   exar_gpio = devm_kzalloc(>dev, sizeof(*exar_gpio), GFP_KERNEL);
> +   if (!exar_gpio)
> +   return -ENOMEM;
> +
> +   mutex_init(_gpio->lock);
> +   INIT_LIST_HEAD(_gpio->list);
> +
> +   index = ida_simple_get(_index, 0, 0, GFP_KERNEL);
> +   mutex_lock(_list_mtx);

This looks overkill.

> +   sprintf(exar_gpio->name, "exar_gpio%d", index);
> +   exar_gpio->gpio_chip.label = exar_gpio->name;
> +   exar_gpio->gpio_chip.parent = >dev;
> +   exar_gpio->gpio_chip.direction_output = exar_direction_output;
> +   exar_gpio->gpio_chip.direction_input = exar_direction_input;
> +   exar_gpio->gpio_chip.get_direction = exar_get_direction;
> +   exar_gpio->gpio_chip.get = exar_get_value;
> +   exar_gpio->gpio_chip.set = exar_set_value;
> +   exar_gpio->gpio_chip.base = -1;
> +   exar_gpio->gpio_chip.ngpio = 16;
> +   exar_gpio->regs = p;
> +   exar_gpio->index = index;
> +   exar_gpio->pcidev = dev;
> +
> +   ret = gpiochip_add(_gpio->gpio_chip);

Use devm_gpiochip_add_data(dev, _gpio->gpio_chip, exar_gpio)

So you can later use gpiochip_get_data()

> +static int gpio_exar_remove(struct platform_device *pdev)
> +{
> +   struct exar_gpio_chip *exar_gpio, *exar_temp;
> +   struct pci_dev *pcidev;
> +   int index;
> +
> +   pcidev =

[PATCH v7 1/3] gpio: exar: add gpio for exar cards

2017-01-07 Thread Sudip Mukherjee 
Exar XR17V352/354/358 chips have 16 multi-purpose inputs/outputs which
can be controlled using gpio interface.

Add the gpio specific code.

Signed-off-by: Sudip Mukherjee 
---
 drivers/gpio/Kconfig |   7 ++
 drivers/gpio/Makefile|   1 +
 drivers/gpio/gpio-exar.c | 238 +++
 3 files changed, 246 insertions(+)
 create mode 100644 drivers/gpio/gpio-exar.c

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index ed37e59..89033d6 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -185,6 +185,13 @@ config GPIO_ETRAXFS
help
  Say yes here to support the GPIO controller on Axis ETRAX FS SoCs.
 
+config GPIO_EXAR
+   tristate "Support for GPIO pins on XR17V352/354/358"
+   depends on SERIAL_8250_EXAR
+   help
+ Selecting this option will enable handling of GPIO pins present
+ on Exar XR17V352/354/358 chips.
+
 config GPIO_GE_FPGA
bool "GE FPGA based GPIO"
depends on GE_FPGA
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index d074c22..d4d44f8 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -44,6 +44,7 @@ obj-$(CONFIG_GPIO_DWAPB)  += gpio-dwapb.o
 obj-$(CONFIG_GPIO_EM)  += gpio-em.o
 obj-$(CONFIG_GPIO_EP93XX)  += gpio-ep93xx.o
 obj-$(CONFIG_GPIO_ETRAXFS) += gpio-etraxfs.o
+obj-$(CONFIG_GPIO_EXAR)+= gpio-exar.o
 obj-$(CONFIG_GPIO_F7188X)  += gpio-f7188x.o
 obj-$(CONFIG_GPIO_GE_FPGA) += gpio-ge.o
 obj-$(CONFIG_GPIO_GPIO_MM) += gpio-gpio-mm.o
diff --git a/drivers/gpio/gpio-exar.c b/drivers/gpio/gpio-exar.c
new file mode 100644
index 000..b7d1363
--- /dev/null
+++ b/drivers/gpio/gpio-exar.c
@@ -0,0 +1,238 @@
+/*
+ * GPIO driver for Exar XR17V35X chip
+ *
+ * Copyright (C) 2015 Sudip Mukherjee 
+ *
+ * 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.
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define EXAR_OFFSET_MPIOLVL_LO 0x90
+#define EXAR_OFFSET_MPIOSEL_LO 0x93
+#define EXAR_OFFSET_MPIOLVL_HI 0x96
+#define EXAR_OFFSET_MPIOSEL_HI 0x99
+
+#define DRIVER_NAME "gpio_exar"
+
+static LIST_HEAD(exar_list);
+static DEFINE_MUTEX(exar_list_mtx);
+DEFINE_IDA(ida_index);
+
+struct exar_gpio_chip {
+   struct gpio_chip gpio_chip;
+   struct pci_dev *pcidev;
+   struct mutex lock;
+   struct list_head list;
+   int index;
+   void __iomem *regs;
+   char name[16];
+};
+
+#define to_exar_chip(n) container_of(n, struct exar_gpio_chip, gpio_chip)
+
+static inline unsigned int read_exar_reg(struct exar_gpio_chip *chip,
+int offset)
+{
+   dev_dbg(chip->gpio_chip.parent, "regs=%p offset=%x\n",
+   chip->regs, offset);
+
+   return readb(chip->regs + offset);
+}
+
+static inline void write_exar_reg(struct exar_gpio_chip *chip, int offset,
+ int value)
+{
+   dev_dbg(chip->gpio_chip.parent,
+   "regs=%p value=%x offset=%x\n", chip->regs, value,
+   offset);
+   writeb(value, chip->regs + offset);
+}
+
+static void exar_update(struct gpio_chip *chip, unsigned int reg, int val,
+   unsigned int offset)
+{
+   struct exar_gpio_chip *exar_gpio = to_exar_chip(chip);
+   int temp;
+
+   mutex_lock(_gpio->lock);
+   temp = read_exar_reg(exar_gpio, reg);
+   temp &= ~(1 << offset);
+   temp |= val << offset;
+   write_exar_reg(exar_gpio, reg, temp);
+   mutex_unlock(_gpio->lock);
+}
+
+static int exar_set_direction(struct gpio_chip *chip, int direction,
+ unsigned int offset)
+{
+   if (offset < 8)
+   exar_update(chip, EXAR_OFFSET_MPIOSEL_LO, direction,
+   offset);
+   else
+   exar_update(chip, EXAR_OFFSET_MPIOSEL_HI, direction,
+   offset - 8);
+   return 0;
+}
+
+static int exar_direction_output(struct gpio_chip *chip, unsigned int offset,
+int value)
+{
+   return exar_set_direction(chip, 0, offset);
+}
+
+static int exar_direction_input(struct gpio_chip *chip, unsigned int offset)
+{
+   return exar_set_direction(chip, 1, offset);
+}
+
+static int exar_get(struct gpio_chip *chip, unsigned int reg)
+{
+   struct exar_gpio_chip *exar_gpio = to_exar_chip(chip);
+   int value;
+
+   mutex_lock(_gpio->lock);
+   value = read_exar_reg(exar_gpio, reg);
+   mutex_unlock(_gpio->lock);
+
+   return value;
+}
+
+static int exar_get_direction(struct gpio_chip *chip, unsigned int offset)
+{
+   int val;
+
+   if (offset < 8)
+   val = exar_get(chip,

[PATCH v7 1/3] gpio: exar: add gpio for exar cards

2017-01-07 Thread Sudip Mukherjee 
Exar XR17V352/354/358 chips have 16 multi-purpose inputs/outputs which
can be controlled using gpio interface.

Add the gpio specific code.

Signed-off-by: Sudip Mukherjee 
---
 drivers/gpio/Kconfig |   7 ++
 drivers/gpio/Makefile|   1 +
 drivers/gpio/gpio-exar.c | 238 +++
 3 files changed, 246 insertions(+)
 create mode 100644 drivers/gpio/gpio-exar.c

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index ed37e59..89033d6 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -185,6 +185,13 @@ config GPIO_ETRAXFS
help
  Say yes here to support the GPIO controller on Axis ETRAX FS SoCs.
 
+config GPIO_EXAR
+   tristate "Support for GPIO pins on XR17V352/354/358"
+   depends on SERIAL_8250_EXAR
+   help
+ Selecting this option will enable handling of GPIO pins present
+ on Exar XR17V352/354/358 chips.
+
 config GPIO_GE_FPGA
bool "GE FPGA based GPIO"
depends on GE_FPGA
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index d074c22..d4d44f8 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -44,6 +44,7 @@ obj-$(CONFIG_GPIO_DWAPB)  += gpio-dwapb.o
 obj-$(CONFIG_GPIO_EM)  += gpio-em.o
 obj-$(CONFIG_GPIO_EP93XX)  += gpio-ep93xx.o
 obj-$(CONFIG_GPIO_ETRAXFS) += gpio-etraxfs.o
+obj-$(CONFIG_GPIO_EXAR)+= gpio-exar.o
 obj-$(CONFIG_GPIO_F7188X)  += gpio-f7188x.o
 obj-$(CONFIG_GPIO_GE_FPGA) += gpio-ge.o
 obj-$(CONFIG_GPIO_GPIO_MM) += gpio-gpio-mm.o
diff --git a/drivers/gpio/gpio-exar.c b/drivers/gpio/gpio-exar.c
new file mode 100644
index 000..b7d1363
--- /dev/null
+++ b/drivers/gpio/gpio-exar.c
@@ -0,0 +1,238 @@
+/*
+ * GPIO driver for Exar XR17V35X chip
+ *
+ * Copyright (C) 2015 Sudip Mukherjee 
+ *
+ * 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.
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define EXAR_OFFSET_MPIOLVL_LO 0x90
+#define EXAR_OFFSET_MPIOSEL_LO 0x93
+#define EXAR_OFFSET_MPIOLVL_HI 0x96
+#define EXAR_OFFSET_MPIOSEL_HI 0x99
+
+#define DRIVER_NAME "gpio_exar"
+
+static LIST_HEAD(exar_list);
+static DEFINE_MUTEX(exar_list_mtx);
+DEFINE_IDA(ida_index);
+
+struct exar_gpio_chip {
+   struct gpio_chip gpio_chip;
+   struct pci_dev *pcidev;
+   struct mutex lock;
+   struct list_head list;
+   int index;
+   void __iomem *regs;
+   char name[16];
+};
+
+#define to_exar_chip(n) container_of(n, struct exar_gpio_chip, gpio_chip)
+
+static inline unsigned int read_exar_reg(struct exar_gpio_chip *chip,
+int offset)
+{
+   dev_dbg(chip->gpio_chip.parent, "regs=%p offset=%x\n",
+   chip->regs, offset);
+
+   return readb(chip->regs + offset);
+}
+
+static inline void write_exar_reg(struct exar_gpio_chip *chip, int offset,
+ int value)
+{
+   dev_dbg(chip->gpio_chip.parent,
+   "regs=%p value=%x offset=%x\n", chip->regs, value,
+   offset);
+   writeb(value, chip->regs + offset);
+}
+
+static void exar_update(struct gpio_chip *chip, unsigned int reg, int val,
+   unsigned int offset)
+{
+   struct exar_gpio_chip *exar_gpio = to_exar_chip(chip);
+   int temp;
+
+   mutex_lock(_gpio->lock);
+   temp = read_exar_reg(exar_gpio, reg);
+   temp &= ~(1 << offset);
+   temp |= val << offset;
+   write_exar_reg(exar_gpio, reg, temp);
+   mutex_unlock(_gpio->lock);
+}
+
+static int exar_set_direction(struct gpio_chip *chip, int direction,
+ unsigned int offset)
+{
+   if (offset < 8)
+   exar_update(chip, EXAR_OFFSET_MPIOSEL_LO, direction,
+   offset);
+   else
+   exar_update(chip, EXAR_OFFSET_MPIOSEL_HI, direction,
+   offset - 8);
+   return 0;
+}
+
+static int exar_direction_output(struct gpio_chip *chip, unsigned int offset,
+int value)
+{
+   return exar_set_direction(chip, 0, offset);
+}
+
+static int exar_direction_input(struct gpio_chip *chip, unsigned int offset)
+{
+   return exar_set_direction(chip, 1, offset);
+}
+
+static int exar_get(struct gpio_chip *chip, unsigned int reg)
+{
+   struct exar_gpio_chip *exar_gpio = to_exar_chip(chip);
+   int value;
+
+   mutex_lock(_gpio->lock);
+   value = read_exar_reg(exar_gpio, reg);
+   mutex_unlock(_gpio->lock);
+
+   return value;
+}
+
+static int exar_get_direction(struct gpio_chip *chip, unsigned int offset)
+{
+   int val;
+
+   if (offset < 8)
+   val = exar_get(chip, EXAR_OFFSET_MPIOSEL_LO) >> offset;
+   else
+   val =