gpio: rcar: select General Output Register to set output states
diff mbox series

Message ID 20181210142135.23960-1-vladimir_zapolskiy@mentor.com
State New
Headers show
Series
  • gpio: rcar: select General Output Register to set output states
Related show

Commit Message

Vladimir Zapolskiy Dec. 10, 2018, 2:21 p.m. UTC
R-Car GPIO controller provides two interfaces to set GPIO line output
signal state, and for a particular GPIO line the selected interface is
determined by OUTDTSEL bit value.

At the moment the driver supports only one of two interfaces, namely
OUTDT General Output Register is used to control the output signal.

While this selection is the default one on reset, it is not explicitly
configured on probe, thus it might be possible that kernel and userspace
consumers of a GPIO won't be able to set the wanted GPIO output signal.

Below is a simple test case to reproduce the described problem and
verify this fix in the kernel on H3 ULCB by setting non-default OUTDTSEL
configuration from a bootloader:

  u-boot    > mw.l 0xe6055440 0x3000 1
  ...
  userspace > echo -n default-on > /sys/devices/platform/leds/leds/led5/trigger
  userspace > echo -n default-on > /sys/devices/platform/leds/leds/led6/trigger

Fixes: 119f5e448d32c ("gpio: Renesas R-Car GPIO driver V3")
Signed-off-by: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>
---

The proposed change could be seen as an invitation for a more interesting
discussion about a necessity to add a pretty trivial support of the second
interface, for instance by selecting between OUTDT and OUTDTH/OUTDTL on
basis of read-only value of OUTDTSEL register, or, simply by switching
the driver to use the second interface only, because it does not require
an additional gpio_rcar_read() call, theoretically it should give noticeably
faster rate of bitbanging.

For reference the problem with the original interface comes from an inability
to set GPIO output signals from an RTOS, which runs in parallel to Linux and
wants to control some GPIOs on its own, usage of OUTDTH/OUTDTL excludes
a race in concurrent read/write register operations.

As a note in my opinion the selection of OUTDT vs. OUTDTH/OUTDTL should
NOT be done in DTS, extension to 3-cell values for GPIO consumers seems
unreasonable.

Below is the list of helpful tips for change reviewers, comments are welcome:
* I didn't manage to find H1 or M1A User's Manuals to confirm that
  OUTDTSEL register and the second OUTDTH/OUTDTL interface is present
  on the GPIO controllers found on R-Car Gen1 SoCs,
* Fixes tag here is pretty weak, nevertheless I suppose it is a fix in fact,
* gpio_rcar_suspend()/gpio_rcar_resume() don't respect OUTDTSEL/OUTDTH/OUTDTL
  values, if there is a reason to dump/restore registers, it might be good
  to include them to the list also,
* alternatively it might be possible to replace the original interface with
  OUTDTH/OUTDTL one, it will be a nice valid fix also.

 drivers/gpio/gpio-rcar.c | 5 +++++
 1 file changed, 5 insertions(+)

Comments

Linus Walleij Dec. 16, 2018, 12:19 a.m. UTC | #1
On Mon, Dec 10, 2018 at 3:22 PM Vladimir Zapolskiy
<vladimir_zapolskiy@mentor.com> wrote:

> R-Car GPIO controller provides two interfaces to set GPIO line output
> signal state, and for a particular GPIO line the selected interface is
> determined by OUTDTSEL bit value.
>
> At the moment the driver supports only one of two interfaces, namely
> OUTDT General Output Register is used to control the output signal.
>
> While this selection is the default one on reset, it is not explicitly
> configured on probe, thus it might be possible that kernel and userspace
> consumers of a GPIO won't be able to set the wanted GPIO output signal.
>
> Below is a simple test case to reproduce the described problem and
> verify this fix in the kernel on H3 ULCB by setting non-default OUTDTSEL
> configuration from a bootloader:
>
>   u-boot    > mw.l 0xe6055440 0x3000 1
>   ...
>   userspace > echo -n default-on > /sys/devices/platform/leds/leds/led5/trigger
>   userspace > echo -n default-on > /sys/devices/platform/leds/leds/led6/trigger
>
> Fixes: 119f5e448d32c ("gpio: Renesas R-Car GPIO driver V3")
> Signed-off-by: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>

Patch applied for devel as "non-urgent fix" as I can't see any
immediate regressions.

Thanks for your thorough investigation of the issue!

Yours,
Linus Walleij
Geert Uytterhoeven Dec. 16, 2018, 10:03 a.m. UTC | #2
Hi Vladimir,

(increasing review priority due to Linus applying the patch)

On Mon, Dec 10, 2018 at 3:22 PM Vladimir Zapolskiy
<vladimir_zapolskiy@mentor.com> wrote:
> R-Car GPIO controller provides two interfaces to set GPIO line output
> signal state, and for a particular GPIO line the selected interface is
> determined by OUTDTSEL bit value.
>
> At the moment the driver supports only one of two interfaces, namely
> OUTDT General Output Register is used to control the output signal.
>
> While this selection is the default one on reset, it is not explicitly
> configured on probe, thus it might be possible that kernel and userspace
> consumers of a GPIO won't be able to set the wanted GPIO output signal.
>
> Below is a simple test case to reproduce the described problem and
> verify this fix in the kernel on H3 ULCB by setting non-default OUTDTSEL
> configuration from a bootloader:
>
>   u-boot    > mw.l 0xe6055440 0x3000 1
>   ...
>   userspace > echo -n default-on > /sys/devices/platform/leds/leds/led5/trigger
>   userspace > echo -n default-on > /sys/devices/platform/leds/leds/led6/trigger
>
> Fixes: 119f5e448d32c ("gpio: Renesas R-Car GPIO driver V3")
> Signed-off-by: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>

Thanks for your patch!

> The proposed change could be seen as an invitation for a more interesting
> discussion about a necessity to add a pretty trivial support of the second
> interface, for instance by selecting between OUTDT and OUTDTH/OUTDTL on
> basis of read-only value of OUTDTSEL register, or, simply by switching
> the driver to use the second interface only, because it does not require
> an additional gpio_rcar_read() call, theoretically it should give noticeably
> faster rate of bitbanging.
>
> For reference the problem with the original interface comes from an inability
> to set GPIO output signals from an RTOS, which runs in parallel to Linux and
> wants to control some GPIOs on its own, usage of OUTDTH/OUTDTL excludes
> a race in concurrent read/write register operations.
>
> As a note in my opinion the selection of OUTDT vs. OUTDTH/OUTDTL should
> NOT be done in DTS, extension to 3-cell values for GPIO consumers seems
> unreasonable.

Indeed, this is pure software configuration, not hardware description, so it
does not belong in DT.

> Below is the list of helpful tips for change reviewers, comments are welcome:
> * I didn't manage to find H1 or M1A User's Manuals to confirm that
>   OUTDTSEL register and the second OUTDTH/OUTDTL interface is present
>   on the GPIO controllers found on R-Car Gen1 SoCs,

Unfortunately R-Car M1A and H1 do not have the OUTDTSEL nor OUTDTH/OUTDL
registers. So your patch may break them.

> * Fixes tag here is pretty weak, nevertheless I suppose it is a fix in fact,

IMHO the SHA1 is not appropriate, as commit 119f5e448d32c ("gpio: Renesas
R-Car GPIO driver V3") added support for R-Car Gen1 only, while the OUTDT*
registers appeared in R-Car Gen2.

> * gpio_rcar_suspend()/gpio_rcar_resume() don't respect OUTDTSEL/OUTDTH/OUTDTL
>   values, if there is a reason to dump/restore registers, it might be good
>   to include them to the list also,

Given resume calls gpio_rcar_direction_{in,out}put(), at least OUTDTSEL
will be restored for output. Is that sufficient, or should it be restored for
input, too?

> * alternatively it might be possible to replace the original interface with
>   OUTDTH/OUTDTL one, it will be a nice valid fix also.

Unfortunately that is not supported by all SoCs supported by the driver.

Gr{oetje,eeting}s,

                        Geert
Geert Uytterhoeven Dec. 16, 2018, 10:04 a.m. UTC | #3
Hi Linus,

On Sun, Dec 16, 2018 at 1:19 AM Linus Walleij <linus.walleij@linaro.org> wrote:
> On Mon, Dec 10, 2018 at 3:22 PM Vladimir Zapolskiy
> <vladimir_zapolskiy@mentor.com> wrote:
> > R-Car GPIO controller provides two interfaces to set GPIO line output
> > signal state, and for a particular GPIO line the selected interface is
> > determined by OUTDTSEL bit value.
> >
> > At the moment the driver supports only one of two interfaces, namely
> > OUTDT General Output Register is used to control the output signal.
> >
> > While this selection is the default one on reset, it is not explicitly
> > configured on probe, thus it might be possible that kernel and userspace
> > consumers of a GPIO won't be able to set the wanted GPIO output signal.
> >
> > Below is a simple test case to reproduce the described problem and
> > verify this fix in the kernel on H3 ULCB by setting non-default OUTDTSEL
> > configuration from a bootloader:
> >
> >   u-boot    > mw.l 0xe6055440 0x3000 1
> >   ...
> >   userspace > echo -n default-on > /sys/devices/platform/leds/leds/led5/trigger
> >   userspace > echo -n default-on > /sys/devices/platform/leds/leds/led6/trigger
> >
> > Fixes: 119f5e448d32c ("gpio: Renesas R-Car GPIO driver V3")
> > Signed-off-by: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>
>
> Patch applied for devel as "non-urgent fix" as I can't see any
> immediate regressions.

Unfortunately the driver now writes unconditionally to a register that does
not exist on R-Car Gen1. Hence please revert the patch.

Thanks!

Gr{oetje,eeting}s,

                        Geert
Vladimir Zapolskiy Dec. 16, 2018, 7:06 p.m. UTC | #4
Hi Linus, Geert,

On 12/16/2018 12:04 PM, Geert Uytterhoeven wrote:
> Hi Linus,
> 
> On Sun, Dec 16, 2018 at 1:19 AM Linus Walleij <linus.walleij@linaro.org> wrote:
>> On Mon, Dec 10, 2018 at 3:22 PM Vladimir Zapolskiy
>> <vladimir_zapolskiy@mentor.com> wrote:
>>> R-Car GPIO controller provides two interfaces to set GPIO line output
>>> signal state, and for a particular GPIO line the selected interface is
>>> determined by OUTDTSEL bit value.
>>>
>>> At the moment the driver supports only one of two interfaces, namely
>>> OUTDT General Output Register is used to control the output signal.
>>>
>>> While this selection is the default one on reset, it is not explicitly
>>> configured on probe, thus it might be possible that kernel and userspace
>>> consumers of a GPIO won't be able to set the wanted GPIO output signal.
>>>
>>> Below is a simple test case to reproduce the described problem and
>>> verify this fix in the kernel on H3 ULCB by setting non-default OUTDTSEL
>>> configuration from a bootloader:
>>>
>>>   u-boot    > mw.l 0xe6055440 0x3000 1
>>>   ...
>>>   userspace > echo -n default-on > /sys/devices/platform/leds/leds/led5/trigger
>>>   userspace > echo -n default-on > /sys/devices/platform/leds/leds/led6/trigger
>>>
>>> Fixes: 119f5e448d32c ("gpio: Renesas R-Car GPIO driver V3")
>>> Signed-off-by: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>
>>
>> Patch applied for devel as "non-urgent fix" as I can't see any
>> immediate regressions.
> 
> Unfortunately the driver now writes unconditionally to a register that does
> not exist on R-Car Gen1. Hence please revert the patch.

yes, for sure revert the change then.

I'll prepare v2 which does not touch R-Car Gen1 platforms.

--
Best wishes,
Vladimir
Linus Walleij Dec. 17, 2018, 2:27 p.m. UTC | #5
On Sun, Dec 16, 2018 at 11:04 AM Geert Uytterhoeven
<geert@linux-m68k.org> wrote:

> > > Fixes: 119f5e448d32c ("gpio: Renesas R-Car GPIO driver V3")
> > > Signed-off-by: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>
> >
> > Patch applied for devel as "non-urgent fix" as I can't see any
> > immediate regressions.
>
> Unfortunately the driver now writes unconditionally to a register that does
> not exist on R-Car Gen1. Hence please revert the patch.

Ooopsidaisy, I pulled it out!

Yours,
Linus Walleij
Vladimir Zapolskiy Dec. 18, 2018, 12:58 p.m. UTC | #6
Hi Geert,

On 12/16/2018 12:03 PM, Geert Uytterhoeven wrote:
> Hi Vladimir,
> 
> (increasing review priority due to Linus applying the patch)
> 
> On Mon, Dec 10, 2018 at 3:22 PM Vladimir Zapolskiy
> <vladimir_zapolskiy@mentor.com> wrote:
>> R-Car GPIO controller provides two interfaces to set GPIO line output
>> signal state, and for a particular GPIO line the selected interface is
>> determined by OUTDTSEL bit value.
>>
>> At the moment the driver supports only one of two interfaces, namely
>> OUTDT General Output Register is used to control the output signal.
>>
>> While this selection is the default one on reset, it is not explicitly
>> configured on probe, thus it might be possible that kernel and userspace
>> consumers of a GPIO won't be able to set the wanted GPIO output signal.
>>
>> Below is a simple test case to reproduce the described problem and
>> verify this fix in the kernel on H3 ULCB by setting non-default OUTDTSEL
>> configuration from a bootloader:
>>
>>   u-boot    > mw.l 0xe6055440 0x3000 1
>>   ...
>>   userspace > echo -n default-on > /sys/devices/platform/leds/leds/led5/trigger
>>   userspace > echo -n default-on > /sys/devices/platform/leds/leds/led6/trigger
>>
>> Fixes: 119f5e448d32c ("gpio: Renesas R-Car GPIO driver V3")
>> Signed-off-by: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>
> 
> Thanks for your patch!
> 
>> The proposed change could be seen as an invitation for a more interesting
>> discussion about a necessity to add a pretty trivial support of the second
>> interface, for instance by selecting between OUTDT and OUTDTH/OUTDTL on
>> basis of read-only value of OUTDTSEL register, or, simply by switching
>> the driver to use the second interface only, because it does not require
>> an additional gpio_rcar_read() call, theoretically it should give noticeably
>> faster rate of bitbanging.
>>
>> For reference the problem with the original interface comes from an inability
>> to set GPIO output signals from an RTOS, which runs in parallel to Linux and
>> wants to control some GPIOs on its own, usage of OUTDTH/OUTDTL excludes
>> a race in concurrent read/write register operations.
>>
>> As a note in my opinion the selection of OUTDT vs. OUTDTH/OUTDTL should
>> NOT be done in DTS, extension to 3-cell values for GPIO consumers seems
>> unreasonable.
> 
> Indeed, this is pure software configuration, not hardware description, so it
> does not belong in DT.
> 
>> Below is the list of helpful tips for change reviewers, comments are welcome:
>> * I didn't manage to find H1 or M1A User's Manuals to confirm that
>>   OUTDTSEL register and the second OUTDTH/OUTDTL interface is present
>>   on the GPIO controllers found on R-Car Gen1 SoCs,
> 
> Unfortunately R-Car M1A and H1 do not have the OUTDTSEL nor OUTDTH/OUTDL
> registers. So your patch may break them.

FWIW I've managed to find only R01UH0573EJ0100 Rev.1.00 R-Car D1 User’s
Manual (see Merlot evaluation board), and it describes OUTDTSEL/OUTDTH/OUTDTL
and BOTHEDGE registers, thus a GPIO controller on this R-Car Gen1 SoC looks
similar to GPIO controllers on R-Car Gen2/Gen3 SoCs.

Thank you for clarification about R-Car M1A and H1 GPIO controllers, my v2
is in progress.

>> * Fixes tag here is pretty weak, nevertheless I suppose it is a fix in fact,
> 
> IMHO the SHA1 is not appropriate, as commit 119f5e448d32c ("gpio: Renesas
> R-Car GPIO driver V3") added support for R-Car Gen1 only, while the OUTDT*
> registers appeared in R-Car Gen2.
> 
>> * gpio_rcar_suspend()/gpio_rcar_resume() don't respect OUTDTSEL/OUTDTH/OUTDTL
>>   values, if there is a reason to dump/restore registers, it might be good
>>   to include them to the list also,
> 
> Given resume calls gpio_rcar_direction_{in,out}put(), at least OUTDTSEL
> will be restored for output. Is that sufficient, or should it be restored for
> input, too?

Hmm, I was reflecting on necessity to save/restore OUTDTSEL value as a whole
independently of per line gpiochip_line_is_valid() value, but let's omit it.

I'm still influenced by a use-case of competing access to a GPIO controller
from two OSes, there might be an overlapping with Linux PM routines in
the driver.

As a side note I'm not convinced that gpiochip_line_is_valid() and
gpiochip->valid_mask usage in the driver is justified, unless it is agreed
that 'gpio-reserved-ranges' property is really supposed to describe "holes"
in GPIO controllers. The property found in r8a77470.dtsi (RZ/G1C) looks like
a kludge instead of making a proper assignment of 'gpio-ranges' property:

-                       gpio-ranges = <&pfc 0 96 30>;
-                       gpio-reserved-ranges = <17 10>;
+                       gpio-ranges = <&pfc 0 96 17>, <&pfc 27 123 3>;

The change above is untested and I have no access to RZ/G1C manual, it is
shared just to demonstrate an alternative idea of describing holes.

>> * alternatively it might be possible to replace the original interface with
>>   OUTDTH/OUTDTL one, it will be a nice valid fix also.
> 
> Unfortunately that is not supported by all SoCs supported by the driver.

Would it be seen as beneficial to add support of a likely better interface
for modern SoCs? The associated complexity in the driver won't be drastic.

--
Best wishes,
Vladimir
Geert Uytterhoeven Dec. 18, 2018, 2:57 p.m. UTC | #7
Hi Vladimir,

On Tue, Dec 18, 2018 at 1:58 PM Vladimir Zapolskiy
<vladimir_zapolskiy@mentor.com> wrote:
> On 12/16/2018 12:03 PM, Geert Uytterhoeven wrote:
> > On Mon, Dec 10, 2018 at 3:22 PM Vladimir Zapolskiy
> > <vladimir_zapolskiy@mentor.com> wrote:
> >> R-Car GPIO controller provides two interfaces to set GPIO line output
> >> signal state, and for a particular GPIO line the selected interface is
> >> determined by OUTDTSEL bit value.
> >>
> >> At the moment the driver supports only one of two interfaces, namely
> >> OUTDT General Output Register is used to control the output signal.
> >>
> >> While this selection is the default one on reset, it is not explicitly
> >> configured on probe, thus it might be possible that kernel and userspace
> >> consumers of a GPIO won't be able to set the wanted GPIO output signal.
> >>
> >> Below is a simple test case to reproduce the described problem and
> >> verify this fix in the kernel on H3 ULCB by setting non-default OUTDTSEL
> >> configuration from a bootloader:
> >>
> >>   u-boot    > mw.l 0xe6055440 0x3000 1
> >>   ...
> >>   userspace > echo -n default-on > /sys/devices/platform/leds/leds/led5/trigger
> >>   userspace > echo -n default-on > /sys/devices/platform/leds/leds/led6/trigger
> >>
> >> Fixes: 119f5e448d32c ("gpio: Renesas R-Car GPIO driver V3")
> >> Signed-off-by: Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>
> >
> > Thanks for your patch!
> >
> >> The proposed change could be seen as an invitation for a more interesting
> >> discussion about a necessity to add a pretty trivial support of the second
> >> interface, for instance by selecting between OUTDT and OUTDTH/OUTDTL on
> >> basis of read-only value of OUTDTSEL register, or, simply by switching
> >> the driver to use the second interface only, because it does not require
> >> an additional gpio_rcar_read() call, theoretically it should give noticeably
> >> faster rate of bitbanging.
> >>
> >> For reference the problem with the original interface comes from an inability
> >> to set GPIO output signals from an RTOS, which runs in parallel to Linux and
> >> wants to control some GPIOs on its own, usage of OUTDTH/OUTDTL excludes
> >> a race in concurrent read/write register operations.
> >>
> >> As a note in my opinion the selection of OUTDT vs. OUTDTH/OUTDTL should
> >> NOT be done in DTS, extension to 3-cell values for GPIO consumers seems
> >> unreasonable.
> >
> > Indeed, this is pure software configuration, not hardware description, so it
> > does not belong in DT.
> >
> >> Below is the list of helpful tips for change reviewers, comments are welcome:
> >> * I didn't manage to find H1 or M1A User's Manuals to confirm that
> >>   OUTDTSEL register and the second OUTDTH/OUTDTL interface is present
> >>   on the GPIO controllers found on R-Car Gen1 SoCs,
> >
> > Unfortunately R-Car M1A and H1 do not have the OUTDTSEL nor OUTDTH/OUTDL
> > registers. So your patch may break them.
>
> FWIW I've managed to find only R01UH0573EJ0100 Rev.1.00 R-Car D1 User’s

I don't have that one...

> Manual (see Merlot evaluation board), and it describes OUTDTSEL/OUTDTH/OUTDTL

Never heard of the Merlot board...

> and BOTHEDGE registers, thus a GPIO controller on this R-Car Gen1 SoC looks
> similar to GPIO controllers on R-Car Gen2/Gen3 SoCs.

I'm not that surprised R-Car D1 contains R-Car Gen2 DNA: it's newer than R-Car
H1/M1, and usually listed together with R-Car Gen2 SoCs.

> >> * Fixes tag here is pretty weak, nevertheless I suppose it is a fix in fact,
> >
> > IMHO the SHA1 is not appropriate, as commit 119f5e448d32c ("gpio: Renesas
> > R-Car GPIO driver V3") added support for R-Car Gen1 only, while the OUTDT*
> > registers appeared in R-Car Gen2.
> >
> >> * gpio_rcar_suspend()/gpio_rcar_resume() don't respect OUTDTSEL/OUTDTH/OUTDTL
> >>   values, if there is a reason to dump/restore registers, it might be good
> >>   to include them to the list also,
> >
> > Given resume calls gpio_rcar_direction_{in,out}put(), at least OUTDTSEL
> > will be restored for output. Is that sufficient, or should it be restored for
> > input, too?
>
> Hmm, I was reflecting on necessity to save/restore OUTDTSEL value as a whole
> independently of per line gpiochip_line_is_valid() value, but let's omit it.
>
> I'm still influenced by a use-case of competing access to a GPIO controller
> from two OSes, there might be an overlapping with Linux PM routines in
> the driver.

PM is the module clock? If the second OS runs on the RT core, and uses the
_R_MSTPCRn registers, everything should work fine.

> As a side note I'm not convinced that gpiochip_line_is_valid() and
> gpiochip->valid_mask usage in the driver is justified, unless it is agreed
> that 'gpio-reserved-ranges' property is really supposed to describe "holes"
> in GPIO controllers. The property found in r8a77470.dtsi (RZ/G1C) looks like
> a kludge instead of making a proper assignment of 'gpio-ranges' property:
>
> -                       gpio-ranges = <&pfc 0 96 30>;
> -                       gpio-reserved-ranges = <17 10>;
> +                       gpio-ranges = <&pfc 0 96 17>, <&pfc 27 123 3>;
>
> The change above is untested and I have no access to RZ/G1C manual, it is
> shared just to demonstrate an alternative idea of describing holes.

FWIW, the RZ/G1C "User's Manual: Hardware" can be download from
www.renesas.com.

> >> * alternatively it might be possible to replace the original interface with
> >>   OUTDTH/OUTDTL one, it will be a nice valid fix also.
> >
> > Unfortunately that is not supported by all SoCs supported by the driver.
>
> Would it be seen as beneficial to add support of a likely better interface
> for modern SoCs? The associated complexity in the driver won't be drastic.

Sure.


Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Patch
diff mbox series

diff --git a/drivers/gpio/gpio-rcar.c b/drivers/gpio/gpio-rcar.c
index 8802b59bfec7..d6ada162b167 100644
--- a/drivers/gpio/gpio-rcar.c
+++ b/drivers/gpio/gpio-rcar.c
@@ -63,6 +63,7 @@  struct gpio_rcar_priv {
 #define POSNEG 0x20	/* Positive/Negative Logic Select Register */
 #define EDGLEVEL 0x24	/* Edge/level Select Register */
 #define FILONOFF 0x28	/* Chattering Prevention On/Off Register */
+#define OUTDTSEL 0x40	/* Output Data Select Register */
 #define BOTHEDGE 0x4c	/* One Edge/Both Edge Select Register */
 
 #define RCAR_MAX_GPIO_PER_BANK		32
@@ -243,6 +244,10 @@  static void gpio_rcar_config_general_input_output_mode(struct gpio_chip *chip,
 	/* Select Input Mode or Output Mode in INOUTSEL */
 	gpio_rcar_modify_bit(p, INOUTSEL, gpio, output);
 
+	/* Select General Output Register to output data in OUTDTSEL */
+	if (output)
+		gpio_rcar_modify_bit(p, OUTDTSEL, gpio, false);
+
 	spin_unlock_irqrestore(&p->lock, flags);
 }