All of lore.kernel.org
 help / color / mirror / Atom feed
From: Stefan Roese <sr@denx.de>
To: "Steven A. Falco" <sfalco@harris.com>
Cc: "linuxppc-dev@ozlabs.org" <linuxppc-dev@ozlabs.org>
Subject: Re: [PATCH v6] PPC440EPx gpio driver
Date: Fri, 10 Oct 2008 08:44:54 +0200	[thread overview]
Message-ID: <200810100844.54672.sr@denx.de> (raw)
In-Reply-To: <48EE6408.3020006@harris.com>

On Thursday 09 October 2008, Steven A. Falco wrote:
> This patch adds support for the GPIO functions of PPC40x and PPC44x
> SOCs.

Thanks. Tested successfully on 405EP board.

I only have one comment below left.

> Signed-off-by: Steve Falco <sfalco at harris.com>
> ---
>
> I looked more closely at the datasheet.  Stefan is correct that the
> shadow registers are not needed for these processors, because they have
> separate registers for input and output.
>
> I've incorporated the other changes, with one exception.  I want
> ppc4xx_gpio_get() to return 0 or 1 (rather than Anton's comment that any
> non-zero value is ok), because when you use the new "export feature" in
> sysfs, you see the raw value returned from ppc4xx_gpio_get().  So, without
> the !!  in the return statement, you would see a strange value, like 32768
> instead of 1:
>
> 	# cd gpio208
> 	# cat value
> 	32768
>
> So, I think it is worth sanitizing the return value here.

<snip>

> +static int __init ppc4xx_add_gpiochips(void)
> +{
> +	struct device_node *np;
> +
> +	for_each_compatible_node(np, NULL, "amcc,ppc4xx-gpio") {

Since the GPIO IP-core implementation is from IBM and not AMCC I suggest to 
use a different compatible property:

	for_each_compatible_node(np, NULL, "ibm,ppc4xx-gpio") {

Other than this:

Acked-by: Stefan Roese <sr@denx.de>

Best regards,
Stefan

      parent reply	other threads:[~2008-10-10  6:47 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-10-09 20:05 [PATCH v6] PPC440EPx gpio driver Steven A. Falco
2008-10-09 21:18 ` Anton Vorontsov
2008-10-10  0:12   ` David Brownell
2008-10-10  6:44 ` Stefan Roese [this message]

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=200810100844.54672.sr@denx.de \
    --to=sr@denx.de \
    --cc=linuxppc-dev@ozlabs.org \
    --cc=sfalco@harris.com \
    /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.