From: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
To: timur@tabi.org
Cc: Linus Walleij <linus.walleij@linaro.org>,
swboyd@chromium.org, linux-gpio@vger.kernel.org,
LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] gpiolib: Show correct direction from the beginning
Date: Thu, 20 Sep 2018 16:14:00 +0200 [thread overview]
Message-ID: <CAPybu_2ovB3T1fOD2gP9rM46z37WVuOuzu5FyKamfyMWoVg=yw@mail.gmail.com> (raw)
In-Reply-To: <015715f7-64bc-aca6-77fe-68ddb6c938a8@kernel.org>
Hi
On Thu, Sep 20, 2018 at 2:20 PM Timur Tabi <timur@tabi.org> wrote:
>
>
>
> On 09/19/2018 10:27 AM, Ricardo Ribalda Delgado wrote:
> > Let me explain my current setup
> >
> > I have a board with input and output gpios, the direction is defined
> > via pdata. When I run gpioinfo all the gpios are shown as input,
> > regardless if they are input or outputs: Eg:
> >
> > root@qt5022:/tmp# ./gpioinfo
> >
> > gpiochip0 - 16 lines:
> > line 0: "PROG_B" unused input active-high
> > line 1: "M0" unused input active-high
> > line 2: "M1" unused input active-high
> > line 3: "M2" unused input active-high
> > line 4: "DIN" unused input active-high
> > line 5: "CCLK" unused input active-high
> > line 6: unnamed unused input active-high
> > line 7: unnamed unused input active-high
> > line 8: "DONE" unused input active-high
> > line 9: "INIT_B" unused input active-high
> > line 10: unnamed unused input active-high
> > line 11: unnamed unused input active-high
> > line 12: unnamed unused input active-high
> > line 13: unnamed unused input active-high
> > line 14: unnamed unused input active-high
> > line 15: unnamed unused input active-high
>
> Yes, this is a known problem that should be fixed.
>
> > That is wrong and very confusing to the user, it can also lead to a
> > mayor fuckup if the user decides to connect two output gpio pins
> > because he expects that both are input. (This is the programming port,
> > but I also have 24 V -high current GPIOs)
>
> Users are expected to program the direction for every GPIO they want to
> use, regardless of whatever it's set to before they open it.
I do not agree that the user should program the direction of a GPIO
which direction cannot be used.
Also I am not talking about programming a gpio, I am talking about an
technician connecting portA to portB and burning something because
the system provided erroneous information
>
> > There is a function in the API to tell libgpio if a gpio is out our
> > in. Why not use it?
>
> Because calling that API before properly claiming the GPIO is a
> programming error.
Is there a place where this API is defined?. Which functions require
to be defined.? What is the correct order.?
>
> > - If the configuration is hardcoded, the driver will return a fixed value
> > - If it is cheap to query the hardware, the driver will query the hardware,
> > - If it is expensive to query the hardware the driver can either
> > return a cached value or a fake value (current situation)
>
> The reason why the Qualcomm driver is impacted the most is because on
> ACPI platforms, the GPIO map is "sparse". That is, not every GPIO
> between 0 and n-1 actually exists. So reading a GPIO that doesn't exist
> is invalid.
Why are we adding GPIOs that are invalid?
If you can figure out that a GPIO is invalid when the user claims a
gpio, you can also figure it out when the user asks the direction.
>
> The way to protect against that is to claim the GPIO first. If the
> claim is rejected, then you know that you can't access that GPIO.
>
> The bug is that the original code that I deleted (and that you're trying
> to put back) doesn't claim the GPIO first.
>
> >>From my point of view: "The get_direction callback normally triggers
> > a read/write to hardware, but we shouldn't be touching the hardware
> > for an individual GPIO until after it's been properly claimed." is
> > an statement specific for your platform and should be fixed in your
> > driver.
> >
> > Either that, or I have completely missunderstund the purpouse of gpiod
> > :), and that could easily be the case.
>
> It's not a platform-specific statement. It applies to all drivers. In
> some drivers, the get_direction function had side-effects (like
> programming muxes, IIRC) that no one really cared about but was
> technically wrong.
A get operation should not set any functionality..., it should return
a cached value or query safely the hardware.
>
> I'm not sure how to properly fix this, but I wonder if we need some kind
> of late-stage initialization where gpiolib scans all the GPIOs by
> claiming them first, reading the directions, and then releasing them.
That sounds like a good compromise. Or returning
-unconfigured / unknown
is also an option.
--
Ricardo Ribalda
next prev parent reply other threads:[~2018-09-20 14:14 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-09-14 7:08 [RFC] gpiolib: Fix gpio_direction_* for single direction GPIOs Ricardo Ribalda Delgado
2018-09-14 7:08 ` [PATCH] gpiolib: Show correct direction from the beginning Ricardo Ribalda Delgado
2018-09-18 22:40 ` Linus Walleij
2018-09-19 4:04 ` Ricardo Ribalda Delgado
2018-09-19 11:50 ` Timur Tabi
2018-09-19 15:27 ` Ricardo Ribalda Delgado
2018-09-20 12:20 ` Timur Tabi
2018-09-20 14:14 ` Ricardo Ribalda Delgado [this message]
2018-09-20 22:43 ` Linus Walleij
2018-09-20 12:25 ` Timur Tabi
2018-09-20 5:23 ` Linus Walleij
2018-09-20 12:35 ` Timur Tabi
2018-09-20 22:36 ` Linus Walleij
2018-09-21 2:05 ` Timur Tabi
2018-09-21 16:07 ` Linus Walleij
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='CAPybu_2ovB3T1fOD2gP9rM46z37WVuOuzu5FyKamfyMWoVg=yw@mail.gmail.com' \
--to=ricardo.ribalda@gmail.com \
--cc=linus.walleij@linaro.org \
--cc=linux-gpio@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=swboyd@chromium.org \
--cc=timur@tabi.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
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;
as well as URLs for NNTP newsgroup(s).