[U-Boot] [PATCH] MX: Set a common gpio.h for all i.MX

[stefano babic]

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
=====================================================================