public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: stefano babic <sbabic@denx.de>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH] MX: Set a common gpio.h for all i.MX
Date: Sun, 19 Aug 2012 00:13:37 +0200	[thread overview]
Message-ID: <50301391.7010307@denx.de> (raw)
In-Reply-To: <1281266427.2541001.1345317958784.JavaMail.root@advansee.com>

Am 18/08/2012 21:25, schrieb Beno?t Th?baudeau:

>>  #define MXC_GPIO_PORT_TO_NUM(port, bit) (((port - 1) << 5) + (bit &
>>  0x1f))
> 
> Keeping this is also useless. GPIO_NUMBER() from the new <asm/imx-common/gpio.h>
> can be used instead everywhere needed.

That is right - I drop it.

>>  
>> -/* GPIO registers */
>> -struct gpio_regs {
>> -	u32	gpio_dr;
>> -	u32	gpio_dir;
>> -	u32	gpio_psr;
>> -};
>> +#include <asm/imx-common/gpio.h>
>>  
>>  #endif	/* __ASM_ARCH_MX6_GPIO_H */
> 
> Why do you keep all these old <asm/gpio.h>? The new <asm/imx-common/gpio.h> can
> be included instead everywhere needed.

No. The GPIO is common for all SOCs in u-boot, not only i.MX. The common
interface requires that a asm/gpio.h exists. See common/cmd_gpio.c.

>> +#ifndef __ASM_ARCH_IMX_GPIO_H
>> +#define __ASM_ARCH_IMX_GPIO_H
>> +
>> +#if !(defined(__KERNEL_STRICT_NAMES) || defined(__ASSEMBLY__))
>> +/* GPIO registers */
>> +struct gpio_regs {
>> +	u32 gpio_dr;	/* data */
>> +	u32 gpio_dir;	/* direction */
>> +	u32 psr;	/* pad satus */
> 
> Why don't you rename this to gpio_psr to be more consistent?

I can do it, or drop it: psr is not used by the driver.

> 
> This made me wonder how mxc_gpio.c could build successfully with this naming.

psr is not used

> It's because gpio_get_value() uses DR instead of PSR.

Exactly

> This is an issue. Linux
> uses PSR, and U-Boot should do so since DR does not always reflect the pin
> state, while PSR does. This makes a big difference if you want to detect a short
> circuit on a GPIO pin configured as output, or if you want to read the level of
> a pin driven by a non-GPIO function. This is another patch to make.

Right, this is a patch for the gpio driver itself.

> 
> The rest sounds good.
> 
> This is a bit off topic,

Never mind.

> but shouldn't the iomux-v3 stuff be moved to a common
> location for all these i.MXs too? As to the header file, it's already done, but
> the C file is under arch/arm/cpu/armv7/imx-common/ while it is absolutely not
> armv7-specific. Unlike Linux's, U-Boot's SoC files are organized per CPU instead
> of per platform, which makes the organization of such platform common code a bit
> complicated.

Right. I miss in u-boot a plat-mxc for common code.

> This also encourages the duplication of platform code. Perhaps it
> could be moved to a new arch/arm/cpu/imx-common/ folder (this would require an
> addition to the main Makefile)?

Up now it was not yet done, because it is an exception in u-boot. But
surely, a common place for code across cpu boundaries helps for i.MX.

Best regards,
Stefano

-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================

  parent reply	other threads:[~2012-08-18 22:13 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-08-18 15:26 [U-Boot] [PATCH] MX: Set a common gpio.h for all i.MX Stefano Babic
2012-08-18 19:25 ` Benoît Thébaudeau
2012-08-18 20:55   ` Matt Sealey
2012-08-18 22:01     ` Marek Vasut
2012-08-18 22:13   ` stefano babic [this message]
2012-08-19  0:25     ` Benoît Thébaudeau
2012-08-19  8:35       ` Stefano Babic
2012-08-18 22:59 ` Fabio Estevam
2012-08-18 23:01   ` Marek Vasut
2012-08-19  7:46   ` stefano babic
2012-08-19  8:40 ` [U-Boot] [PATCH v2] " Stefano Babic
2012-08-19  9:11 ` [U-Boot] [PATCH v3] " Stefano Babic
2012-08-19 13:30   ` Benoît Thébaudeau
2012-08-19 16:41     ` stefano babic
2012-08-20  7:33 ` [U-Boot] [PATCH v4] " Stefano Babic
2012-08-20 20:33   ` Benoît Thébaudeau
2012-08-20 21:20   ` Matt Sealey
2012-08-21  6:12     ` Stefano Babic

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=50301391.7010307@denx.de \
    --to=sbabic@denx.de \
    --cc=u-boot@lists.denx.de \
    /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