public inbox for linux-arm-kernel@lists.infradead.org 
 help / color / mirror / Atom feed
From: j.uzycki@elproma•com.pl (Janusz Użycki)
To: linux-arm-kernel@lists•infradead.org
Subject: [PATCH] gpio: mxs: implement get_direction callback
Date: Sat, 15 Nov 2014 20:29:04 +0100	[thread overview]
Message-ID: <5467A980.5090204@elproma.com.pl> (raw)
In-Reply-To: <20141114232601.GW27002@pengutronix.de>

Hello Uwe,

W dniu 2014-11-15 o 00:26, Uwe Kleine-K?nig pisze:
> Hello,
>
> On Fri, Nov 14, 2014 at 11:27:06PM +0100, Janusz Uzycki wrote:
>> Function gpiod_get_direction() of gpiolib calls get_direction()
>> callback. If chip doesn't implement it EINVAL error is returned.
>> The function doesn't use for returned value shadowed FLAG_IS_OUT
>> bit of gpio_desc.flags field so the callback is required.
> This sounds like a bugfix but "required" is wrong as AFAIR this call is
> optional and hardly used. Or did that change? The only drawback is that
> the debug output for (by Linux) uninitialized gpios is wrong.

Yes, the callback is optional but gpiod_get_direction() doesn't work 
without it.
gpiod_get_direction() doesn't seem optional, 
Documentation/gpio/consumer.txt:
"A driver can also query the current direction of a GPIO:
          int gpiod_get_direction(const struct gpio_desc *desc)
This function will return either GPIOF_DIR_IN or GPIOF_DIR_OUT.
Be aware that there is no default direction for GPIOs. Therefore, 
**using a GPIO
without setting its direction first is illegal and will result in undefined
behavior!**"
There is nothing about EINVAL error. It happens despite direction was
set before. The reason is undefined get_direction callback.

The commit f9e42397d79b6e810437ba1130b0b4b594f5e56c
("serial: mxs-auart: add interrupts for modem control lines")
is based on Richard's commit ab5e4e4108ca5d8326cb6b4b3a21b096a002f68f
("tty/serial: at91: add interrupts for modem control lines").
Both patches use the condition:
"if (gpiod && (gpiod_get_direction(gpiod) == GPIOF_DIR_IN))"

at91 had defined get_direction() callback, mxs platform not.
The condition helps to find gpio-inputs to set them as interrupts.

Likely gpiod_get_direction() was used because 
drivers/tty/serial/serial_mctrl_gpio.c
has no function to read back "dir_out" from mctrl_gpios_desc table.

There were the ways to fix it:
a) add to serial_mctrl_gpio.c function to read the "dir_out" and use it 
instead of
     gpiod_get_direction()
if gpiod_get_direction() still used in the condition:
b) modify gpiod_get_direction() implementation in gpiolib.c:
     if get_direction() callback is not defined use shadow flag 
(FLAG_IS_OUT)
c) modify drivers/gpio/gpio-generic.c:
    bgpio_init() could assign to get_direction default callback
    if BGPIOF_UNREADABLE_REG_DIR is not set
d) implement get_direction callback in gpio-mxs.c

Solution d) was selected because it is safe for other drivers. Although 
a) and b)
are also nice the patch doesn't modify neither serial_mctrl_gpio.c and
atmel_serial.c nor gpiolib.c. It focuses on mxs platform as needed for
the commit "serial: mxs-auart: add interrupts for modem control lines".

Maybe Richard will be interested in a).

> Also the grammar of you sentence isn't German enough to sound correct.
Right, I was tired, sorry.
What about the following description for the patch?

gpiolib's function gpiod_get_direction() returns the EINVAL error
if get_direction() callback is not defined.
The patch implements the callback for mxs chip as commit
f9e42397d79b ("serial: mxs-auart: add interrupts for modem control lines")
uses the gpiod_get_direction() function.

best regards
Janusz

>
> Best regards
> Uwe
>

  reply	other threads:[~2014-11-15 19:29 UTC|newest]

Thread overview: 41+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-11-14 22:27 [PATCH] gpio: mxs: implement get_direction callback Janusz Uzycki
2014-11-14 23:26 ` Uwe Kleine-König
2014-11-15 19:29   ` Janusz Użycki [this message]
2014-11-16 21:42     ` Uwe Kleine-König
2014-11-16 21:48       ` Uwe Kleine-König
2014-11-16 23:59       ` Janusz Użycki
2014-11-17  1:58         ` Janusz Użycki
2014-11-17  8:28           ` Uwe Kleine-König
2014-11-17  8:38             ` Alexander Shiyan
2014-11-17  8:44               ` Uwe Kleine-König
2014-11-17  8:53                 ` Alexander Shiyan
2014-11-17  9:11             ` Janusz Użycki
2014-11-17  9:39               ` Uwe Kleine-König
2014-11-17  9:46               ` Richard Genoud
2014-11-17  9:59                 ` Uwe Kleine-König
2014-11-17 10:05                   ` Richard Genoud
2014-11-17 14:29                     ` Janusz Użycki
2014-11-17 16:14                       ` Richard Genoud
2014-11-17 15:53                     ` Uwe Kleine-König
2014-11-17 15:58                       ` Janusz Użycki
2014-11-17 16:02                         ` Uwe Kleine-König
2014-11-17 16:04                       ` Richard Genoud
2014-11-17 17:26                     ` Janusz Użycki
2014-11-17 10:05                   ` Alexander Shiyan
2014-11-17 10:09                     ` Russell King - ARM Linux
2014-11-17 10:10                     ` Richard Genoud
2014-11-17 10:17                       ` Russell King - ARM Linux
2014-11-17 12:40                 ` Janusz Użycki
2014-11-17  9:51               ` request an irq without enabling? [Was: Re: [PATCH] gpio: mxs: implement get_direction callback] Uwe Kleine-König
2014-11-17  9:57                 ` Richard Genoud
2014-11-17 17:00             ` [PATCH] gpio: mxs: implement get_direction callback Janusz Użycki
2014-11-17 17:07               ` Janusz Użycki
2014-11-17 18:42                 ` Uwe Kleine-König
2014-11-17 19:02                   ` Janusz Użycki
2014-11-17 22:21                     ` Uwe Kleine-König
2014-11-18  9:59                       ` Janusz Użycki
2014-11-17  9:26         ` Richard Genoud
2014-11-17 14:45           ` Janusz Użycki
2014-11-17 15:59             ` Uwe Kleine-König
2014-11-17  8:31       ` Richard Genoud
2014-11-17  8:39         ` Uwe Kleine-König

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5467A980.5090204@elproma.com.pl \
    --to=j.uzycki@elproma$(echo .)com.pl \
    --cc=linux-arm-kernel@lists$(echo .)infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox