Re: [PATCH v6 08/12] gpio: Add GPIO driver for the RK805 PMIC

[Heiko Stuebner]

Am Freitag, 21. Juli 2017, 10:00:29 CEST schrieb Jianhong Chen:
> 
> 在 2017/6/29 18:29, Heiko Stübner 写道:
> 
> > Hi,
> >
> > Am Mittwoch, 14. Juni 2017, 20:11:06 CEST schrieb Jianhong Chen:
> >> 在 2017/6/9 20:17, Heiko Stuebner 写道:
> >>> Am Freitag, 9. Juni 2017, 13:37:26 CEST schrieb Linus Walleij:
>  Heiko, can you please look at this patch.
> 
>  On Thu, Jun 8, 2017 at 9:30 AM, Jianhong Chen 
> > wrote:
> > From: chenjh 
>  Full name please.
> >>> git config --global user.name "John Doe"
> >>>
> >>> might do the and make this permanent for all your commits :-)
> >>>
> > RK805 has two configurable GPIOs that can be used for several
> > purposes. These are output only.
> >
> > This driver is generic for other Rockchip PMICs to be added.
> >
> > Signed-off-by: chenjh 
>  Dito.
> 
>  Your commit message says they are output-only, yet you implement
>  .direction_input(). So what is is going to be?
> >>> So far, I've only seen the rk808 and rk818. Both do not have any
> >>> configurable pins.
> >>>
> >>> The rk805 which is a sort of variant of the above, does have the two
> >>> pins defined below, but in the manual I could also only find them as
> >>> output-only and having no other function than being output-pins.
> >>>
> >>> So I don't really know if all the input- or "gpio-mode"- handling is only
> >>> an oversight (copy'n'paste) or if there are yet other rk808 variants
> >>> around
> >>> that can actually be configured as inputs or even non-gpio modes?
> >>>
> >>> I hope Jianhong will be able to answer that.
> >>>
> >>>
> >>> Heiko
> >> This driver is not only for rk805, but also intend for rk816 and furtrue
> >> PMICs.
> >> The rk816 has one multi function pin(TS/GPIO), when setting as gpio, it
> >> can be configured as output or input.
> >> Here is simple description from manual: "Thermistor input. Connect a
> >> thermistor from this pin to ground. The thermistor is usually inside the
> >> battery pack. (multi-function for GPIO) ".
> > As Linus suggested, this sounds like you want a pinctrl driver that
> > also handles the gpios.
> >
> > Ideally you might also directly provide support for this rk816 in the
> > same patch series, so reviewers can see the full extend of what is
> > supported.
> >
> >
> > Heiko
> >
> 
> Hi, Heiko:
> 
>   I have moved gpio-rk805.c to drivers/pinctrl/pinctrl-rk805.c, this 
> driver is also designed for rk816 or furture PMICs to extend.
>   RK816 is not in our team's plan at this porting, it will be added 
> at next time, so I don't directly provide rk816 in this driver. I will 
> add more descriptions in commit message to express my design purpose.
> 
>   Are you agreed ?  If so, I will send [PATCH v7] today.

yep, sounds good. But we will of course see more, once you have sent
the patches :-) .


Heiko

Re: [PATCH v6 08/12] gpio: Add GPIO driver for the RK805 PMIC

[Heiko Stuebner]

Am Freitag, 21. Juli 2017, 10:00:29 CEST schrieb Jianhong Chen:
> 
> 在 2017/6/29 18:29, Heiko Stübner 写道:
> 
> > Hi,
> >
> > Am Mittwoch, 14. Juni 2017, 20:11:06 CEST schrieb Jianhong Chen:
> >> 在 2017/6/9 20:17, Heiko Stuebner 写道:
> >>> Am Freitag, 9. Juni 2017, 13:37:26 CEST schrieb Linus Walleij:
>  Heiko, can you please look at this patch.
> 
>  On Thu, Jun 8, 2017 at 9:30 AM, Jianhong Chen 
> > wrote:
> > From: chenjh 
>  Full name please.
> >>> git config --global user.name "John Doe"
> >>>
> >>> might do the and make this permanent for all your commits :-)
> >>>
> > RK805 has two configurable GPIOs that can be used for several
> > purposes. These are output only.
> >
> > This driver is generic for other Rockchip PMICs to be added.
> >
> > Signed-off-by: chenjh 
>  Dito.
> 
>  Your commit message says they are output-only, yet you implement
>  .direction_input(). So what is is going to be?
> >>> So far, I've only seen the rk808 and rk818. Both do not have any
> >>> configurable pins.
> >>>
> >>> The rk805 which is a sort of variant of the above, does have the two
> >>> pins defined below, but in the manual I could also only find them as
> >>> output-only and having no other function than being output-pins.
> >>>
> >>> So I don't really know if all the input- or "gpio-mode"- handling is only
> >>> an oversight (copy'n'paste) or if there are yet other rk808 variants
> >>> around
> >>> that can actually be configured as inputs or even non-gpio modes?
> >>>
> >>> I hope Jianhong will be able to answer that.
> >>>
> >>>
> >>> Heiko
> >> This driver is not only for rk805, but also intend for rk816 and furtrue
> >> PMICs.
> >> The rk816 has one multi function pin(TS/GPIO), when setting as gpio, it
> >> can be configured as output or input.
> >> Here is simple description from manual: "Thermistor input. Connect a
> >> thermistor from this pin to ground. The thermistor is usually inside the
> >> battery pack. (multi-function for GPIO) ".
> > As Linus suggested, this sounds like you want a pinctrl driver that
> > also handles the gpios.
> >
> > Ideally you might also directly provide support for this rk816 in the
> > same patch series, so reviewers can see the full extend of what is
> > supported.
> >
> >
> > Heiko
> >
> 
> Hi, Heiko:
> 
>   I have moved gpio-rk805.c to drivers/pinctrl/pinctrl-rk805.c, this 
> driver is also designed for rk816 or furture PMICs to extend.
>   RK816 is not in our team's plan at this porting, it will be added 
> at next time, so I don't directly provide rk816 in this driver. I will 
> add more descriptions in commit message to express my design purpose.
> 
>   Are you agreed ?  If so, I will send [PATCH v7] today.

yep, sounds good. But we will of course see more, once you have sent
the patches :-) .


Heiko

Re: [PATCH v6 08/12] gpio: Add GPIO driver for the RK805 PMIC

[Jianhong Chen]

在 2017/6/29 18:29, Heiko Stübner 写道:


Hi,

Am Mittwoch, 14. Juni 2017, 20:11:06 CEST schrieb Jianhong Chen:

在 2017/6/9 20:17, Heiko Stuebner 写道:

Am Freitag, 9. Juni 2017, 13:37:26 CEST schrieb Linus Walleij:

Heiko, can you please look at this patch.

On Thu, Jun 8, 2017 at 9:30 AM, Jianhong Chen 

wrote:

From: chenjh 

Full name please.

git config --global user.name "John Doe"

might do the and make this permanent for all your commits :-)


RK805 has two configurable GPIOs that can be used for several
purposes. These are output only.

This driver is generic for other Rockchip PMICs to be added.

Signed-off-by: chenjh 

Dito.

Your commit message says they are output-only, yet you implement
.direction_input(). So what is is going to be?

So far, I've only seen the rk808 and rk818. Both do not have any
configurable pins.

The rk805 which is a sort of variant of the above, does have the two
pins defined below, but in the manual I could also only find them as
output-only and having no other function than being output-pins.

So I don't really know if all the input- or "gpio-mode"- handling is only
an oversight (copy'n'paste) or if there are yet other rk808 variants
around
that can actually be configured as inputs or even non-gpio modes?

I hope Jianhong will be able to answer that.


Heiko

This driver is not only for rk805, but also intend for rk816 and furtrue
PMICs.
The rk816 has one multi function pin(TS/GPIO), when setting as gpio, it
can be configured as output or input.
Here is simple description from manual: "Thermistor input. Connect a
thermistor from this pin to ground. The thermistor is usually inside the
battery pack. (multi-function for GPIO) ".

As Linus suggested, this sounds like you want a pinctrl driver that
also handles the gpios.

Ideally you might also directly provide support for this rk816 in the
same patch series, so reviewers can see the full extend of what is
supported.


Heiko



Hi, Heiko:

 I have moved gpio-rk805.c to drivers/pinctrl/pinctrl-rk805.c, this 
driver is also designed for rk816 or furture PMICs to extend.
 RK816 is not in our team's plan at this porting, it will be added 
at next time, so I don't directly provide rk816 in this driver. I will 
add more descriptions in commit message to express my design purpose.


 Are you agreed ?  If so, I will send [PATCH v7] today.


--
Best Regards

陈健洪 (Joseph Chen)
E-mail：che...@rock-chips.com
福州瑞芯微电子股份有限公司
Fuzhou Rockchip Electronics Co.Ltd
福建省福州市铜盘路软件大道89号软件园A区21号楼 (350003)
No. 21 Building, A District, No.89,software Boulevard Fuzhou,Fujian,PRC
TEL：0591-83991906/07-8573

Re: [PATCH v6 08/12] gpio: Add GPIO driver for the RK805 PMIC

[Jianhong Chen]

在 2017/6/29 18:29, Heiko Stübner 写道:


Hi,

Am Mittwoch, 14. Juni 2017, 20:11:06 CEST schrieb Jianhong Chen:

在 2017/6/9 20:17, Heiko Stuebner 写道:

Am Freitag, 9. Juni 2017, 13:37:26 CEST schrieb Linus Walleij:

Heiko, can you please look at this patch.

On Thu, Jun 8, 2017 at 9:30 AM, Jianhong Chen 

wrote:

From: chenjh 

Full name please.

git config --global user.name "John Doe"

might do the and make this permanent for all your commits :-)


RK805 has two configurable GPIOs that can be used for several
purposes. These are output only.

This driver is generic for other Rockchip PMICs to be added.

Signed-off-by: chenjh 

Dito.

Your commit message says they are output-only, yet you implement
.direction_input(). So what is is going to be?

So far, I've only seen the rk808 and rk818. Both do not have any
configurable pins.

The rk805 which is a sort of variant of the above, does have the two
pins defined below, but in the manual I could also only find them as
output-only and having no other function than being output-pins.

So I don't really know if all the input- or "gpio-mode"- handling is only
an oversight (copy'n'paste) or if there are yet other rk808 variants
around
that can actually be configured as inputs or even non-gpio modes?

I hope Jianhong will be able to answer that.


Heiko

This driver is not only for rk805, but also intend for rk816 and furtrue
PMICs.
The rk816 has one multi function pin(TS/GPIO), when setting as gpio, it
can be configured as output or input.
Here is simple description from manual: "Thermistor input. Connect a
thermistor from this pin to ground. The thermistor is usually inside the
battery pack. (multi-function for GPIO) ".

As Linus suggested, this sounds like you want a pinctrl driver that
also handles the gpios.

Ideally you might also directly provide support for this rk816 in the
same patch series, so reviewers can see the full extend of what is
supported.


Heiko



Hi, Heiko:

 I have moved gpio-rk805.c to drivers/pinctrl/pinctrl-rk805.c, this 
driver is also designed for rk816 or furture PMICs to extend.
 RK816 is not in our team's plan at this porting, it will be added 
at next time, so I don't directly provide rk816 in this driver. I will 
add more descriptions in commit message to express my design purpose.


 Are you agreed ?  If so, I will send [PATCH v7] today.


--
Best Regards

陈健洪 (Joseph Chen)
E-mail：che...@rock-chips.com
福州瑞芯微电子股份有限公司
Fuzhou Rockchip Electronics Co.Ltd
福建省福州市铜盘路软件大道89号软件园A区21号楼 (350003)
No. 21 Building, A District, No.89,software Boulevard Fuzhou,Fujian,PRC
TEL：0591-83991906/07-8573

Re: [PATCH v6 08/12] gpio: Add GPIO driver for the RK805 PMIC

[Heiko Stübner]

Hi,

Am Mittwoch, 14. Juni 2017, 20:11:06 CEST schrieb Jianhong Chen:
> 在 2017/6/9 20:17, Heiko Stuebner 写道:
> > Am Freitag, 9. Juni 2017, 13:37:26 CEST schrieb Linus Walleij:
> >> Heiko, can you please look at this patch.
> >> 
> >> On Thu, Jun 8, 2017 at 9:30 AM, Jianhong Chen  
wrote:
> >>> From: chenjh 
> >> 
> >> Full name please.
> > 
> > git config --global user.name "John Doe"
> > 
> > might do the and make this permanent for all your commits :-)
> > 
> >>> RK805 has two configurable GPIOs that can be used for several
> >>> purposes. These are output only.
> >>> 
> >>> This driver is generic for other Rockchip PMICs to be added.
> >>> 
> >>> Signed-off-by: chenjh 
> >> 
> >> Dito.
> >> 
> >> Your commit message says they are output-only, yet you implement
> >> .direction_input(). So what is is going to be?
> > 
> > So far, I've only seen the rk808 and rk818. Both do not have any
> > configurable pins.
> > 
> > The rk805 which is a sort of variant of the above, does have the two
> > pins defined below, but in the manual I could also only find them as
> > output-only and having no other function than being output-pins.
> > 
> > So I don't really know if all the input- or "gpio-mode"- handling is only
> > an oversight (copy'n'paste) or if there are yet other rk808 variants
> > around
> > that can actually be configured as inputs or even non-gpio modes?
> > 
> > I hope Jianhong will be able to answer that.
> > 
> > 
> > Heiko
> 
> This driver is not only for rk805, but also intend for rk816 and furtrue
> PMICs.
> The rk816 has one multi function pin(TS/GPIO), when setting as gpio, it
> can be configured as output or input.
> Here is simple description from manual: "Thermistor input. Connect a
> thermistor from this pin to ground. The thermistor is usually inside the
> battery pack. (multi-function for GPIO) ".

As Linus suggested, this sounds like you want a pinctrl driver that 
also handles the gpios.

Ideally you might also directly provide support for this rk816 in the
same patch series, so reviewers can see the full extend of what is
supported.


Heiko

Re: [PATCH v6 08/12] gpio: Add GPIO driver for the RK805 PMIC

[Heiko Stübner]

Hi,

Am Mittwoch, 14. Juni 2017, 20:11:06 CEST schrieb Jianhong Chen:
> 在 2017/6/9 20:17, Heiko Stuebner 写道:
> > Am Freitag, 9. Juni 2017, 13:37:26 CEST schrieb Linus Walleij:
> >> Heiko, can you please look at this patch.
> >> 
> >> On Thu, Jun 8, 2017 at 9:30 AM, Jianhong Chen  
wrote:
> >>> From: chenjh 
> >> 
> >> Full name please.
> > 
> > git config --global user.name "John Doe"
> > 
> > might do the and make this permanent for all your commits :-)
> > 
> >>> RK805 has two configurable GPIOs that can be used for several
> >>> purposes. These are output only.
> >>> 
> >>> This driver is generic for other Rockchip PMICs to be added.
> >>> 
> >>> Signed-off-by: chenjh 
> >> 
> >> Dito.
> >> 
> >> Your commit message says they are output-only, yet you implement
> >> .direction_input(). So what is is going to be?
> > 
> > So far, I've only seen the rk808 and rk818. Both do not have any
> > configurable pins.
> > 
> > The rk805 which is a sort of variant of the above, does have the two
> > pins defined below, but in the manual I could also only find them as
> > output-only and having no other function than being output-pins.
> > 
> > So I don't really know if all the input- or "gpio-mode"- handling is only
> > an oversight (copy'n'paste) or if there are yet other rk808 variants
> > around
> > that can actually be configured as inputs or even non-gpio modes?
> > 
> > I hope Jianhong will be able to answer that.
> > 
> > 
> > Heiko
> 
> This driver is not only for rk805, but also intend for rk816 and furtrue
> PMICs.
> The rk816 has one multi function pin(TS/GPIO), when setting as gpio, it
> can be configured as output or input.
> Here is simple description from manual: "Thermistor input. Connect a
> thermistor from this pin to ground. The thermistor is usually inside the
> battery pack. (multi-function for GPIO) ".

As Linus suggested, this sounds like you want a pinctrl driver that 
also handles the gpios.

Ideally you might also directly provide support for this rk816 in the
same patch series, so reviewers can see the full extend of what is
supported.


Heiko

Re: [PATCH v6 08/12] gpio: Add GPIO driver for the RK805 PMIC

[Jianhong Chen]

在 2017/6/9 20:17, Heiko Stuebner 写道:

Hi,

Am Freitag, 9. Juni 2017, 13:37:26 CEST schrieb Linus Walleij:

Heiko, can you please look at this patch.

On Thu, Jun 8, 2017 at 9:30 AM, Jianhong Chen  wrote:


From: chenjh 

Full name please.

git config --global user.name "John Doe"

might do the and make this permanent for all your commits :-)


RK805 has two configurable GPIOs that can be used for several
purposes. These are output only.

This driver is generic for other Rockchip PMICs to be added.

Signed-off-by: chenjh 

Dito.

Your commit message says they are output-only, yet you implement
.direction_input(). So what is is going to be?

So far, I've only seen the rk808 and rk818. Both do not have any
configurable pins.

The rk805 which is a sort of variant of the above, does have the two
pins defined below, but in the manual I could also only find them as
output-only and having no other function than being output-pins.

So I don't really know if all the input- or "gpio-mode"- handling is only
an oversight (copy'n'paste) or if there are yet other rk808 variants around
that can actually be configured as inputs or even non-gpio modes?

I hope Jianhong will be able to answer that.


Heiko
This driver is not only for rk805, but also intend for rk816 and furtrue 
PMICs.
The rk816 has one multi function pin(TS/GPIO), when setting as gpio, it 
can be configured as output or input.
Here is simple description from manual: "Thermistor input. Connect a 
thermistor from this pin to ground. The thermistor is usually inside the 
battery pack. (multi-function for GPIO) ".


At beginning, I want to name it "gpio-rk8xx.c", but I found rk808 and 
rk818 don't have gpio function. So I abandon this name and use 
"gpio-rk805.c", but still implement

it as a generic driver for furture PMICs to be added.

+#include 
+#include 

Only use:
#include 


+/*
+ * @mode: supported modes for this gpio, i.e. OUTPUT_MODE, OUTPUT_MODE...

Are you saying this should be an enum or a set of flags?
Yes, as explain above,  these "OUTPUT_MODE, INPUTOUT" flags are prepared 
for RK816 or furture PMICs.  Maybe these should be enum are better.

+static int rk805_gpio_get(struct gpio_chip *chip, unsigned offset)
+{
+   int ret, val;
+   struct rk805_gpio *gpio = gpiochip_get_data(chip);
+
+   ret = regmap_read(gpio->rk808->regmap, gpio->pins[offset].reg, );
+   if (ret) {
+   dev_err(gpio->dev, "gpio%d not support output mode\n", offset);
+   return ret;
+   }
+
+   return (val & gpio->pins[offset].val_msk) ? 1 : 0;

Do this:

return !!(val & gpio->pins[offset].val_msk)


+static int rk805_gpio_request(struct gpio_chip *chip, unsigned offset)
+{
+   int ret;
+   struct rk805_gpio *gpio = gpiochip_get_data(chip);
+
+   /* switch to gpio mode */
+   if (gpio->pins[offset].func_mask) {
+   ret = regmap_update_bits(gpio->rk808->regmap,
+gpio->pins[offset].reg,
+gpio->pins[offset].func_mask,
+gpio->pins[offset].func_mask);
+   if (ret) {
+   dev_err(gpio->dev, "set gpio%d func failed\n", offset);
+   return ret;
+   }
+   }
+
+   return 0;
+}

This is pin control. Why don't you implement a proper pin control
driver for this chip?

If you don't, this will just come back and haunt you.

Why not merge the driver into drivers/pinctrl/* and name it
pinctrl-rk805.c to begin with?


Becuase I refered to other PMICs, I see most of their gpio driver is 
merged into drivers/gpio/*,  Only a few are added in drivers/pinctrl/*.

So, it is better to merge the driver into drivers/pinctrl/* ?


+static const struct gpio_chip rk805_chip = {
+   .label  = "rk805-gpio",
+   .owner  = THIS_MODULE,
+   .direction_input= rk805_gpio_direction_input,
+   .direction_output   = rk805_gpio_direction_output,

Please implement .get_direction()


+   .get= rk805_gpio_get,
+   .set= rk805_gpio_set,
+   .request= rk805_gpio_request,
+   .base   = -1,
+   .ngpio  = 2,
+   .can_sleep  = true,

Consider assigning the .names[] array some pin names.

Yours,
Linus Walleij

___
Linux-rockchip mailing list
linux-rockc...@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip







--
陈健洪
E-mail：che...@rock-chips.com
福州瑞芯微电子股份有限公司
Fuzhou Rockchip Electronics Co.Ltd
福建省福州市铜盘路软件大道89号软件园A区21号楼 (350003)
No. 21 Building, A District, No.89,software Boulevard Fuzhou,Fujian,PRC
TEL：0591-83991906/07-8573

Re: [PATCH v6 08/12] gpio: Add GPIO driver for the RK805 PMIC

[Jianhong Chen]

在 2017/6/9 20:17, Heiko Stuebner 写道:

Hi,

Am Freitag, 9. Juni 2017, 13:37:26 CEST schrieb Linus Walleij:

Heiko, can you please look at this patch.

On Thu, Jun 8, 2017 at 9:30 AM, Jianhong Chen  wrote:


From: chenjh 

Full name please.

git config --global user.name "John Doe"

might do the and make this permanent for all your commits :-)


RK805 has two configurable GPIOs that can be used for several
purposes. These are output only.

This driver is generic for other Rockchip PMICs to be added.

Signed-off-by: chenjh 

Dito.

Your commit message says they are output-only, yet you implement
.direction_input(). So what is is going to be?

So far, I've only seen the rk808 and rk818. Both do not have any
configurable pins.

The rk805 which is a sort of variant of the above, does have the two
pins defined below, but in the manual I could also only find them as
output-only and having no other function than being output-pins.

So I don't really know if all the input- or "gpio-mode"- handling is only
an oversight (copy'n'paste) or if there are yet other rk808 variants around
that can actually be configured as inputs or even non-gpio modes?

I hope Jianhong will be able to answer that.


Heiko
This driver is not only for rk805, but also intend for rk816 and furtrue 
PMICs.
The rk816 has one multi function pin(TS/GPIO), when setting as gpio, it 
can be configured as output or input.
Here is simple description from manual: "Thermistor input. Connect a 
thermistor from this pin to ground. The thermistor is usually inside the 
battery pack. (multi-function for GPIO) ".


At beginning, I want to name it "gpio-rk8xx.c", but I found rk808 and 
rk818 don't have gpio function. So I abandon this name and use 
"gpio-rk805.c", but still implement

it as a generic driver for furture PMICs to be added.

+#include 
+#include 

Only use:
#include 


+/*
+ * @mode: supported modes for this gpio, i.e. OUTPUT_MODE, OUTPUT_MODE...

Are you saying this should be an enum or a set of flags?
Yes, as explain above,  these "OUTPUT_MODE, INPUTOUT" flags are prepared 
for RK816 or furture PMICs.  Maybe these should be enum are better.

+static int rk805_gpio_get(struct gpio_chip *chip, unsigned offset)
+{
+   int ret, val;
+   struct rk805_gpio *gpio = gpiochip_get_data(chip);
+
+   ret = regmap_read(gpio->rk808->regmap, gpio->pins[offset].reg, );
+   if (ret) {
+   dev_err(gpio->dev, "gpio%d not support output mode\n", offset);
+   return ret;
+   }
+
+   return (val & gpio->pins[offset].val_msk) ? 1 : 0;

Do this:

return !!(val & gpio->pins[offset].val_msk)


+static int rk805_gpio_request(struct gpio_chip *chip, unsigned offset)
+{
+   int ret;
+   struct rk805_gpio *gpio = gpiochip_get_data(chip);
+
+   /* switch to gpio mode */
+   if (gpio->pins[offset].func_mask) {
+   ret = regmap_update_bits(gpio->rk808->regmap,
+gpio->pins[offset].reg,
+gpio->pins[offset].func_mask,
+gpio->pins[offset].func_mask);
+   if (ret) {
+   dev_err(gpio->dev, "set gpio%d func failed\n", offset);
+   return ret;
+   }
+   }
+
+   return 0;
+}

This is pin control. Why don't you implement a proper pin control
driver for this chip?

If you don't, this will just come back and haunt you.

Why not merge the driver into drivers/pinctrl/* and name it
pinctrl-rk805.c to begin with?


Becuase I refered to other PMICs, I see most of their gpio driver is 
merged into drivers/gpio/*,  Only a few are added in drivers/pinctrl/*.

So, it is better to merge the driver into drivers/pinctrl/* ?


+static const struct gpio_chip rk805_chip = {
+   .label  = "rk805-gpio",
+   .owner  = THIS_MODULE,
+   .direction_input= rk805_gpio_direction_input,
+   .direction_output   = rk805_gpio_direction_output,

Please implement .get_direction()


+   .get= rk805_gpio_get,
+   .set= rk805_gpio_set,
+   .request= rk805_gpio_request,
+   .base   = -1,
+   .ngpio  = 2,
+   .can_sleep  = true,

Consider assigning the .names[] array some pin names.

Yours,
Linus Walleij

___
Linux-rockchip mailing list
linux-rockc...@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip







--
陈健洪
E-mail：che...@rock-chips.com
福州瑞芯微电子股份有限公司
Fuzhou Rockchip Electronics Co.Ltd
福建省福州市铜盘路软件大道89号软件园A区21号楼 (350003)
No. 21 Building, A District, No.89,software Boulevard Fuzhou,Fujian,PRC
TEL：0591-83991906/07-8573

Re: [PATCH v6 08/12] gpio: Add GPIO driver for the RK805 PMIC

[Heiko Stuebner]

Hi,

Am Freitag, 9. Juni 2017, 13:37:26 CEST schrieb Linus Walleij:
> Heiko, can you please look at this patch.
> 
> On Thu, Jun 8, 2017 at 9:30 AM, Jianhong Chen  wrote:
> 
> > From: chenjh 
> 
> Full name please.

git config --global user.name "John Doe"

might do the trick and make this permanent for all your commits :-)


> > RK805 has two configurable GPIOs that can be used for several
> > purposes. These are output only.
> >
> > This driver is generic for other Rockchip PMICs to be added.
> >
> > Signed-off-by: chenjh 
> 
> Dito.
> 
> Your commit message says they are output-only, yet you implement
> .direction_input(). So what is is going to be?

So far, I've only seen the rk808 and rk818. Both do not have any
configurable pins.

The rk805 which is a sort of variant of the above, does have the two
pins defined below, but in the manual I could also only find them as
output-only and having no other function than being output-pins.

So I don't really know if all the input- or "gpio-mode"- handling is only
an oversight (copy'n'paste) or if there are yet other rk808 variants around
that can actually be configured as inputs or even non-gpio modes?

I hope Jianhong will be able to answer that.


Heiko

> 
> > +#include 
> > +#include 
> 
> Only use:
> #include 
> 
> > +/*
> > + * @mode: supported modes for this gpio, i.e. OUTPUT_MODE, OUTPUT_MODE...
> 
> Are you saying this should be an enum or a set of flags?
> 
> > +static int rk805_gpio_get(struct gpio_chip *chip, unsigned offset)
> > +{
> > +   int ret, val;
> > +   struct rk805_gpio *gpio = gpiochip_get_data(chip);
> > +
> > +   ret = regmap_read(gpio->rk808->regmap, gpio->pins[offset].reg, 
> > );
> > +   if (ret) {
> > +   dev_err(gpio->dev, "gpio%d not support output mode\n", 
> > offset);
> > +   return ret;
> > +   }
> > +
> > +   return (val & gpio->pins[offset].val_msk) ? 1 : 0;
> 
> Do this:
> 
> return !!(val & gpio->pins[offset].val_msk)
> 
> > +static int rk805_gpio_request(struct gpio_chip *chip, unsigned offset)
> > +{
> > +   int ret;
> > +   struct rk805_gpio *gpio = gpiochip_get_data(chip);
> > +
> > +   /* switch to gpio mode */
> > +   if (gpio->pins[offset].func_mask) {
> > +   ret = regmap_update_bits(gpio->rk808->regmap,
> > +gpio->pins[offset].reg,
> > +gpio->pins[offset].func_mask,
> > +gpio->pins[offset].func_mask);
> > +   if (ret) {
> > +   dev_err(gpio->dev, "set gpio%d func failed\n", 
> > offset);
> > +   return ret;
> > +   }
> > +   }
> > +
> > +   return 0;
> > +}
> 
> This is pin control. Why don't you implement a proper pin control
> driver for this chip?
> 
> If you don't, this will just come back and haunt you.
> 
> Why not merge the driver into drivers/pinctrl/* and name it
> pinctrl-rk805.c to begin with?
> 
> > +static const struct gpio_chip rk805_chip = {
> > +   .label  = "rk805-gpio",
> > +   .owner  = THIS_MODULE,
> > +   .direction_input= rk805_gpio_direction_input,
> > +   .direction_output   = rk805_gpio_direction_output,
> 
> Please implement .get_direction()
> 
> > +   .get= rk805_gpio_get,
> > +   .set= rk805_gpio_set,
> > +   .request= rk805_gpio_request,
> > +   .base   = -1,
> > +   .ngpio  = 2,
> > +   .can_sleep  = true,
> 
> Consider assigning the .names[] array some pin names.
> 
> Yours,
> Linus Walleij
> 
> ___
> Linux-rockchip mailing list
> linux-rockc...@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip
> 
> 

Re: [PATCH v6 08/12] gpio: Add GPIO driver for the RK805 PMIC

[Heiko Stuebner]

Hi,

Am Freitag, 9. Juni 2017, 13:37:26 CEST schrieb Linus Walleij:
> Heiko, can you please look at this patch.
> 
> On Thu, Jun 8, 2017 at 9:30 AM, Jianhong Chen  wrote:
> 
> > From: chenjh 
> 
> Full name please.

git config --global user.name "John Doe"

might do the trick and make this permanent for all your commits :-)


> > RK805 has two configurable GPIOs that can be used for several
> > purposes. These are output only.
> >
> > This driver is generic for other Rockchip PMICs to be added.
> >
> > Signed-off-by: chenjh 
> 
> Dito.
> 
> Your commit message says they are output-only, yet you implement
> .direction_input(). So what is is going to be?

So far, I've only seen the rk808 and rk818. Both do not have any
configurable pins.

The rk805 which is a sort of variant of the above, does have the two
pins defined below, but in the manual I could also only find them as
output-only and having no other function than being output-pins.

So I don't really know if all the input- or "gpio-mode"- handling is only
an oversight (copy'n'paste) or if there are yet other rk808 variants around
that can actually be configured as inputs or even non-gpio modes?

I hope Jianhong will be able to answer that.


Heiko

> 
> > +#include 
> > +#include 
> 
> Only use:
> #include 
> 
> > +/*
> > + * @mode: supported modes for this gpio, i.e. OUTPUT_MODE, OUTPUT_MODE...
> 
> Are you saying this should be an enum or a set of flags?
> 
> > +static int rk805_gpio_get(struct gpio_chip *chip, unsigned offset)
> > +{
> > +   int ret, val;
> > +   struct rk805_gpio *gpio = gpiochip_get_data(chip);
> > +
> > +   ret = regmap_read(gpio->rk808->regmap, gpio->pins[offset].reg, 
> > );
> > +   if (ret) {
> > +   dev_err(gpio->dev, "gpio%d not support output mode\n", 
> > offset);
> > +   return ret;
> > +   }
> > +
> > +   return (val & gpio->pins[offset].val_msk) ? 1 : 0;
> 
> Do this:
> 
> return !!(val & gpio->pins[offset].val_msk)
> 
> > +static int rk805_gpio_request(struct gpio_chip *chip, unsigned offset)
> > +{
> > +   int ret;
> > +   struct rk805_gpio *gpio = gpiochip_get_data(chip);
> > +
> > +   /* switch to gpio mode */
> > +   if (gpio->pins[offset].func_mask) {
> > +   ret = regmap_update_bits(gpio->rk808->regmap,
> > +gpio->pins[offset].reg,
> > +gpio->pins[offset].func_mask,
> > +gpio->pins[offset].func_mask);
> > +   if (ret) {
> > +   dev_err(gpio->dev, "set gpio%d func failed\n", 
> > offset);
> > +   return ret;
> > +   }
> > +   }
> > +
> > +   return 0;
> > +}
> 
> This is pin control. Why don't you implement a proper pin control
> driver for this chip?
> 
> If you don't, this will just come back and haunt you.
> 
> Why not merge the driver into drivers/pinctrl/* and name it
> pinctrl-rk805.c to begin with?
> 
> > +static const struct gpio_chip rk805_chip = {
> > +   .label  = "rk805-gpio",
> > +   .owner  = THIS_MODULE,
> > +   .direction_input= rk805_gpio_direction_input,
> > +   .direction_output   = rk805_gpio_direction_output,
> 
> Please implement .get_direction()
> 
> > +   .get= rk805_gpio_get,
> > +   .set= rk805_gpio_set,
> > +   .request= rk805_gpio_request,
> > +   .base   = -1,
> > +   .ngpio  = 2,
> > +   .can_sleep  = true,
> 
> Consider assigning the .names[] array some pin names.
> 
> Yours,
> Linus Walleij
> 
> ___
> Linux-rockchip mailing list
> linux-rockc...@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip
> 
> 

Re: [PATCH v6 08/12] gpio: Add GPIO driver for the RK805 PMIC

[Linus Walleij]

Heiko, can you please look at this patch.

On Thu, Jun 8, 2017 at 9:30 AM, Jianhong Chen  wrote:

> From: chenjh 

Full name please.

> RK805 has two configurable GPIOs that can be used for several
> purposes. These are output only.
>
> This driver is generic for other Rockchip PMICs to be added.
>
> Signed-off-by: chenjh 

Dito.

Your commit message says they are output-only, yet you implement
.direction_input(). So what is is going to be?

> +#include 
> +#include 

Only use:
#include 

> +/*
> + * @mode: supported modes for this gpio, i.e. OUTPUT_MODE, OUTPUT_MODE...

Are you saying this should be an enum or a set of flags?

> +static int rk805_gpio_get(struct gpio_chip *chip, unsigned offset)
> +{
> +   int ret, val;
> +   struct rk805_gpio *gpio = gpiochip_get_data(chip);
> +
> +   ret = regmap_read(gpio->rk808->regmap, gpio->pins[offset].reg, );
> +   if (ret) {
> +   dev_err(gpio->dev, "gpio%d not support output mode\n", 
> offset);
> +   return ret;
> +   }
> +
> +   return (val & gpio->pins[offset].val_msk) ? 1 : 0;

Do this:

return !!(val & gpio->pins[offset].val_msk)

> +static int rk805_gpio_request(struct gpio_chip *chip, unsigned offset)
> +{
> +   int ret;
> +   struct rk805_gpio *gpio = gpiochip_get_data(chip);
> +
> +   /* switch to gpio mode */
> +   if (gpio->pins[offset].func_mask) {
> +   ret = regmap_update_bits(gpio->rk808->regmap,
> +gpio->pins[offset].reg,
> +gpio->pins[offset].func_mask,
> +gpio->pins[offset].func_mask);
> +   if (ret) {
> +   dev_err(gpio->dev, "set gpio%d func failed\n", 
> offset);
> +   return ret;
> +   }
> +   }
> +
> +   return 0;
> +}

This is pin control. Why don't you implement a proper pin control
driver for this chip?

If you don't, this will just come back and haunt you.

Why not merge the driver into drivers/pinctrl/* and name it
pinctrl-rk805.c to begin with?

> +static const struct gpio_chip rk805_chip = {
> +   .label  = "rk805-gpio",
> +   .owner  = THIS_MODULE,
> +   .direction_input= rk805_gpio_direction_input,
> +   .direction_output   = rk805_gpio_direction_output,

Please implement .get_direction()

> +   .get= rk805_gpio_get,
> +   .set= rk805_gpio_set,
> +   .request= rk805_gpio_request,
> +   .base   = -1,
> +   .ngpio  = 2,
> +   .can_sleep  = true,

Consider assigning the .names[] array some pin names.

Yours,
Linus Walleij

Re: [PATCH v6 08/12] gpio: Add GPIO driver for the RK805 PMIC

[Linus Walleij]

Heiko, can you please look at this patch.

On Thu, Jun 8, 2017 at 9:30 AM, Jianhong Chen  wrote:

> From: chenjh 

Full name please.

> RK805 has two configurable GPIOs that can be used for several
> purposes. These are output only.
>
> This driver is generic for other Rockchip PMICs to be added.
>
> Signed-off-by: chenjh 

Dito.

Your commit message says they are output-only, yet you implement
.direction_input(). So what is is going to be?

> +#include 
> +#include 

Only use:
#include 

> +/*
> + * @mode: supported modes for this gpio, i.e. OUTPUT_MODE, OUTPUT_MODE...

Are you saying this should be an enum or a set of flags?

> +static int rk805_gpio_get(struct gpio_chip *chip, unsigned offset)
> +{
> +   int ret, val;
> +   struct rk805_gpio *gpio = gpiochip_get_data(chip);
> +
> +   ret = regmap_read(gpio->rk808->regmap, gpio->pins[offset].reg, );
> +   if (ret) {
> +   dev_err(gpio->dev, "gpio%d not support output mode\n", 
> offset);
> +   return ret;
> +   }
> +
> +   return (val & gpio->pins[offset].val_msk) ? 1 : 0;

Do this:

return !!(val & gpio->pins[offset].val_msk)

> +static int rk805_gpio_request(struct gpio_chip *chip, unsigned offset)
> +{
> +   int ret;
> +   struct rk805_gpio *gpio = gpiochip_get_data(chip);
> +
> +   /* switch to gpio mode */
> +   if (gpio->pins[offset].func_mask) {
> +   ret = regmap_update_bits(gpio->rk808->regmap,
> +gpio->pins[offset].reg,
> +gpio->pins[offset].func_mask,
> +gpio->pins[offset].func_mask);
> +   if (ret) {
> +   dev_err(gpio->dev, "set gpio%d func failed\n", 
> offset);
> +   return ret;
> +   }
> +   }
> +
> +   return 0;
> +}

This is pin control. Why don't you implement a proper pin control
driver for this chip?

If you don't, this will just come back and haunt you.

Why not merge the driver into drivers/pinctrl/* and name it
pinctrl-rk805.c to begin with?

> +static const struct gpio_chip rk805_chip = {
> +   .label  = "rk805-gpio",
> +   .owner  = THIS_MODULE,
> +   .direction_input= rk805_gpio_direction_input,
> +   .direction_output   = rk805_gpio_direction_output,

Please implement .get_direction()

> +   .get= rk805_gpio_get,
> +   .set= rk805_gpio_set,
> +   .request= rk805_gpio_request,
> +   .base   = -1,
> +   .ngpio  = 2,
> +   .can_sleep  = true,

Consider assigning the .names[] array some pin names.

Yours,
Linus Walleij

[PATCH v6 08/12] gpio: Add GPIO driver for the RK805 PMIC

[Jianhong Chen]

From: chenjh 

RK805 has two configurable GPIOs that can be used for several
purposes. These are output only.

This driver is generic for other Rockchip PMICs to be added.

Signed-off-by: chenjh 
---
 drivers/gpio/Kconfig  |   6 ++
 drivers/gpio/Makefile |   1 +
 drivers/gpio/gpio-rk805.c | 234 ++
 3 files changed, 241 insertions(+)
 create mode 100644 drivers/gpio/gpio-rk805.c

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 0504307..c8cca89 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -974,6 +974,12 @@ config GPIO_RC5T583
  This driver provides the support for driving/reading the gpio pins
  of RC5T583 device through standard gpio library.
 
+config GPIO_RK805
+   bool "Rockchip RK805 GPIO"
+   depends on MFD_RK808
+   help
+ Select this option to enable GPIO driver for the RK805 PMIC.
+
 config GPIO_STMPE
bool "STMPE GPIOs"
depends on MFD_STMPE
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index becb96c..55ba941 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -99,6 +99,7 @@ obj-$(CONFIG_GPIO_PXA)+= gpio-pxa.o
 obj-$(CONFIG_GPIO_RC5T583) += gpio-rc5t583.o
 obj-$(CONFIG_GPIO_RDC321X) += gpio-rdc321x.o
 obj-$(CONFIG_GPIO_RCAR)+= gpio-rcar.o
+obj-$(CONFIG_GPIO_RK805)   += gpio-rk805.o
 obj-$(CONFIG_ARCH_SA1100)  += gpio-sa1100.o
 obj-$(CONFIG_GPIO_SCH) += gpio-sch.o
 obj-$(CONFIG_GPIO_SCH311X) += gpio-sch311x.o
diff --git a/drivers/gpio/gpio-rk805.c b/drivers/gpio/gpio-rk805.c
new file mode 100644
index 000..bc17c92
--- /dev/null
+++ b/drivers/gpio/gpio-rk805.c
@@ -0,0 +1,234 @@
+/*
+ * GPIO driver for Rockchip RK805 PMIC
+ *
+ * Copyright (c) 2017, Fuzhou Rockchip Electronics Co., Ltd
+ *
+ * Author: Chen Jianhong 
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under  the terms of the GNU General  Public License as published by the
+ * Free Software Foundation;  either version 2 of the License, or (at your
+ * option) any later version.
+ *
+ * Based on the TPS65218 driver
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/* rk805 */
+#define RK805_OUT_REG  0x52
+#define RK805_OUT1_VAL_MSK BIT(0)
+#define RK805_OUT2_VAL_MSK BIT(1)
+
+#define OUTPUT_MODEBIT(0)
+#define INPUT_MODE BIT(1)
+
+/*
+ * @mode: supported modes for this gpio, i.e. OUTPUT_MODE, OUTPUT_MODE...
+ * @reg: gpio status setting register
+ * @func_mask: functions select mask value
+ * @dir_mask: input or output mask value
+ * @val_mask: gpio set value
+ */
+struct gpio_pin {
+   u8 mode;
+   u8 reg;
+   u8 func_mask;
+   u8 dir_msk;
+   u8 val_msk;
+};
+
+struct rk805_gpio {
+   struct device *dev;
+   struct gpio_chip chip;
+   struct gpio_pin *pins;
+   struct rk808 *rk808;
+};
+
+static int rk805_gpio_direction_input(struct gpio_chip *chip,
+ unsigned offset)
+{
+   int ret;
+   struct rk805_gpio *gpio = gpiochip_get_data(chip);
+
+   if (!(gpio->pins[offset].mode & INPUT_MODE)) {
+   dev_err(gpio->dev, "gpio%d not support input mode\n", offset);
+   return -EINVAL;
+   }
+
+   if (gpio->pins[offset].dir_msk) {
+   ret = regmap_update_bits(gpio->rk808->regmap,
+gpio->pins[offset].reg,
+gpio->pins[offset].dir_msk, 0);
+   if (ret) {
+   dev_err(gpio->dev, "set gpio%d input failed\n", offset);
+   return ret;
+   }
+   }
+
+   return 0;
+}
+
+static int rk805_gpio_direction_output(struct gpio_chip *chip,
+  unsigned offset, int value)
+{
+   int ret;
+   struct rk805_gpio *gpio = gpiochip_get_data(chip);
+
+   if (!(gpio->pins[offset].mode & OUTPUT_MODE)) {
+   dev_err(gpio->dev, "gpio%d not support output mode\n", offset);
+   return -EINVAL;
+   }
+
+   if (gpio->pins[offset].dir_msk) {
+   ret = regmap_update_bits(gpio->rk808->regmap,
+gpio->pins[offset].reg,
+gpio->pins[offset].dir_msk,
+gpio->pins[offset].dir_msk);
+   if (ret) {
+   dev_err(gpio->dev, "set gpio%d out failed\n", offset);
+   return ret;
+   }
+   }
+
+   ret = regmap_update_bits(gpio->rk808->regmap,
+gpio->pins[offset].reg,
+gpio->pins[offset].val_msk,
+value ? gpio->pins[offset].val_msk : 0);
+   if (ret) {
+  

[PATCH v6 08/12] gpio: Add GPIO driver for the RK805 PMIC

[Jianhong Chen]

From: chenjh 

RK805 has two configurable GPIOs that can be used for several
purposes. These are output only.

This driver is generic for other Rockchip PMICs to be added.

Signed-off-by: chenjh 
---
 drivers/gpio/Kconfig  |   6 ++
 drivers/gpio/Makefile |   1 +
 drivers/gpio/gpio-rk805.c | 234 ++
 3 files changed, 241 insertions(+)
 create mode 100644 drivers/gpio/gpio-rk805.c

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 0504307..c8cca89 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -974,6 +974,12 @@ config GPIO_RC5T583
  This driver provides the support for driving/reading the gpio pins
  of RC5T583 device through standard gpio library.
 
+config GPIO_RK805
+   bool "Rockchip RK805 GPIO"
+   depends on MFD_RK808
+   help
+ Select this option to enable GPIO driver for the RK805 PMIC.
+
 config GPIO_STMPE
bool "STMPE GPIOs"
depends on MFD_STMPE
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index becb96c..55ba941 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -99,6 +99,7 @@ obj-$(CONFIG_GPIO_PXA)+= gpio-pxa.o
 obj-$(CONFIG_GPIO_RC5T583) += gpio-rc5t583.o
 obj-$(CONFIG_GPIO_RDC321X) += gpio-rdc321x.o
 obj-$(CONFIG_GPIO_RCAR)+= gpio-rcar.o
+obj-$(CONFIG_GPIO_RK805)   += gpio-rk805.o
 obj-$(CONFIG_ARCH_SA1100)  += gpio-sa1100.o
 obj-$(CONFIG_GPIO_SCH) += gpio-sch.o
 obj-$(CONFIG_GPIO_SCH311X) += gpio-sch311x.o
diff --git a/drivers/gpio/gpio-rk805.c b/drivers/gpio/gpio-rk805.c
new file mode 100644
index 000..bc17c92
--- /dev/null
+++ b/drivers/gpio/gpio-rk805.c
@@ -0,0 +1,234 @@
+/*
+ * GPIO driver for Rockchip RK805 PMIC
+ *
+ * Copyright (c) 2017, Fuzhou Rockchip Electronics Co., Ltd
+ *
+ * Author: Chen Jianhong 
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under  the terms of the GNU General  Public License as published by the
+ * Free Software Foundation;  either version 2 of the License, or (at your
+ * option) any later version.
+ *
+ * Based on the TPS65218 driver
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/* rk805 */
+#define RK805_OUT_REG  0x52
+#define RK805_OUT1_VAL_MSK BIT(0)
+#define RK805_OUT2_VAL_MSK BIT(1)
+
+#define OUTPUT_MODEBIT(0)
+#define INPUT_MODE BIT(1)
+
+/*
+ * @mode: supported modes for this gpio, i.e. OUTPUT_MODE, OUTPUT_MODE...
+ * @reg: gpio status setting register
+ * @func_mask: functions select mask value
+ * @dir_mask: input or output mask value
+ * @val_mask: gpio set value
+ */
+struct gpio_pin {
+   u8 mode;
+   u8 reg;
+   u8 func_mask;
+   u8 dir_msk;
+   u8 val_msk;
+};
+
+struct rk805_gpio {
+   struct device *dev;
+   struct gpio_chip chip;
+   struct gpio_pin *pins;
+   struct rk808 *rk808;
+};
+
+static int rk805_gpio_direction_input(struct gpio_chip *chip,
+ unsigned offset)
+{
+   int ret;
+   struct rk805_gpio *gpio = gpiochip_get_data(chip);
+
+   if (!(gpio->pins[offset].mode & INPUT_MODE)) {
+   dev_err(gpio->dev, "gpio%d not support input mode\n", offset);
+   return -EINVAL;
+   }
+
+   if (gpio->pins[offset].dir_msk) {
+   ret = regmap_update_bits(gpio->rk808->regmap,
+gpio->pins[offset].reg,
+gpio->pins[offset].dir_msk, 0);
+   if (ret) {
+   dev_err(gpio->dev, "set gpio%d input failed\n", offset);
+   return ret;
+   }
+   }
+
+   return 0;
+}
+
+static int rk805_gpio_direction_output(struct gpio_chip *chip,
+  unsigned offset, int value)
+{
+   int ret;
+   struct rk805_gpio *gpio = gpiochip_get_data(chip);
+
+   if (!(gpio->pins[offset].mode & OUTPUT_MODE)) {
+   dev_err(gpio->dev, "gpio%d not support output mode\n", offset);
+   return -EINVAL;
+   }
+
+   if (gpio->pins[offset].dir_msk) {
+   ret = regmap_update_bits(gpio->rk808->regmap,
+gpio->pins[offset].reg,
+gpio->pins[offset].dir_msk,
+gpio->pins[offset].dir_msk);
+   if (ret) {
+   dev_err(gpio->dev, "set gpio%d out failed\n", offset);
+   return ret;
+   }
+   }
+
+   ret = regmap_update_bits(gpio->rk808->regmap,
+gpio->pins[offset].reg,
+gpio->pins[offset].val_msk,
+value ? gpio->pins[offset].val_msk : 0);
+   if (ret) {
+   dev_err(gpio->dev, "set gpio%d value failed\n", offset);
+