All of lore.kernel.org
 help / color / mirror / Atom feed
From: arnaud.patard@rtp-net.org (Arnaud Patard (Rtp))
To: linux-arm-kernel@lists.infradead.org
Subject: [patch 1/2] imx51: fix gpiolib support
Date: Wed, 01 Dec 2010 12:24:20 +0100	[thread overview]
Message-ID: <87d3plu463.fsf@lechat.rtp-net.org> (raw)
In-Reply-To: <201012011200.25430.alexander.stein@systec-electronic.com> (Alexander Stein's message of "Wed, 1 Dec 2010 11:59:59 +0100")

Alexander Stein <alexander.stein@systec-electronic.com> writes:

Hi,

> On Wednesday 01 December 2010, 11:09:32 Arnaud Patard wrote:
>> > Maybe only read GDIR on mx51?  Isn't the direction saved by gpiolib
>> > somewhere in RAM?  Is the same needed on other SoCs, too (see below for
>> > my grouping)?  Technically it's not needed at all I think.
>> > (Documentation/gpio.txt has: "However, note that not all platforms can
>> > read the value of output pins; those that can't should always return
>> > zero.".  I didn't check if the implementation conforms here though.)
>> 
>> yeah, I know that the gpiolib allows that and in this case, it's always
>> returning 0 but while I was debugging an other problem, I wanted to read
>> output value and it was not working. The manual was not explicitely
>> saying it was not possible so I thought it was a bug and propose a
>> fix. Hopefully, sending this patch allows me to get more feedback and
>> tell me if I got something wrong and there's a better fix.
>> 
>> > I wonder if cpu_is_mx51 is the right macro to test, I'd prefer something
>> > like imx_gpio_vX().  IMHO the gpio stuff needs a major overhaul getting
>> > rid of most runtime checks.
>> 
>> I used cpu_is_mx51() because I only had tested/seen that on imx515. I
>> wanted to avoid breaking other systems while trying to fix mine.
>> As regards getting rid of runtime checks, maybe registering different
>> gpiochip depending on the SoC would allow to check it a init time and no
>> more check later.
>
> I can conform, that reading from GPIO_DR on mx35 works too. Also checked with 
> the description is this register. It seem to show exactly what we want, 
> regarding GPIO_GDIR value. So even evaluating GPIO_DIR isn't needed at all.
> My fix was to simply replace GPIO_PSR with GPIO_DR.

Please look also at Lothar Wa?mann's comments. I plan to test that later
but if setting the SION bit allows things to work correctly, setting it
by default for the GPIOs may be a better fix than my patch.

Arnaud

  reply	other threads:[~2010-12-01 11:24 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-12-01  8:37 [patch 0/2] iMX51 small fixes Arnaud Patard (Rtp)
2010-12-01  8:37 ` [patch 1/2] imx51: fix gpiolib support Arnaud Patard (Rtp)
2010-12-01  9:22   ` Lothar Waßmann
2010-12-01 10:03     ` Arnaud Patard (Rtp)
2010-12-01  9:25   ` Uwe Kleine-König
2010-12-01 10:09     ` Arnaud Patard (Rtp)
2010-12-01 10:59       ` Alexander Stein
2010-12-01 11:24         ` Arnaud Patard (Rtp) [this message]
2010-12-01  8:37 ` [patch 2/2] Fix imx cpufreq driver as module Arnaud Patard (Rtp)

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=87d3plu463.fsf@lechat.rtp-net.org \
    --to=arnaud.patard@rtp-net.org \
    --cc=linux-arm-kernel@lists.infradead.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.