Patchwork [U-Boot,2/3] tx25: Use generic gpio_* calls

login
register
mail settings
Submitter Vikram Narayanan
Date June 10, 2012, 1:03 p.m.
Message ID <4FD49B2F.8090706@gmail.com>
Download mbox | patch
Permalink /patch/163995/
State Superseded
Headers show

Comments

Vikram Narayanan - June 10, 2012, 1:03 p.m.
Instead of manipulating gpio registers directly, use the calls
from the gpio library.

Signed-off-by: Vikram Narayanan <vikram186@gmail.com>
Cc: John Rigby <jcrigby@gmail.com>
Cc: Stefano Babic <sbabic@denx.de>
---
 board/karo/tx25/tx25.c |   28 ++++++++++++----------------
 1 files changed, 12 insertions(+), 16 deletions(-)
Fabio Estevam - June 10, 2012, 2:38 p.m.
On Sun, Jun 10, 2012 at 10:03 AM, Vikram Narayanan <vikram186@gmail.com> wrote:

>        /* drop PHY power and assert reset (low) */
> -       val = readl(&gpio4->gpio_dr) & ~((1 << 7) | (1 << 9));
> -       writel(val, &gpio4->gpio_dr);
> -       val = readl(&gpio4->gpio_dir) | (1 << 7) | (1 << 9);
> -       writel(val, &gpio4->gpio_dir);
> +       gpio_direction_output(GPIO_FEC_RESET_B, 0);
> +       gpio_direction_output(GPIO_FEC_ENABLE_B, 0);
> +
> +       gpio_direction_output(GPIO_FEC_RESET_B, 1);
> +       gpio_direction_output(GPIO_FEC_ENABLE_B, 1);

gpio_direction_output should be set only once for each GPIO.

After you set the direction (input or output) you should use
'gpio_set_value' for setting the GPIO to 0 or 1.

Also, please remove the:

> +       gpio_direction_output(GPIO_FEC_RESET_B, 1);
> +       gpio_direction_output(GPIO_FEC_ENABLE_B, 1);

lines, as the setting to 1 should be after the delay (via gpio_set_value).
Vikram Narayanan - June 11, 2012, 2:58 p.m.
Hi Fabio,

On 6/10/2012 8:08 PM, Fabio Estevam wrote:
> On Sun, Jun 10, 2012 at 10:03 AM, Vikram Narayanan<vikram186@gmail.com>  wrote:
>
>>         /* drop PHY power and assert reset (low) */
>> -       val = readl(&gpio4->gpio_dr)&  ~((1<<  7) | (1<<  9));
>> -       writel(val,&gpio4->gpio_dr);
>> -       val = readl(&gpio4->gpio_dir) | (1<<  7) | (1<<  9);
>> -       writel(val,&gpio4->gpio_dir);
>> +       gpio_direction_output(GPIO_FEC_RESET_B, 0);
>> +       gpio_direction_output(GPIO_FEC_ENABLE_B, 0);
>> +
>> +       gpio_direction_output(GPIO_FEC_RESET_B, 1);
>> +       gpio_direction_output(GPIO_FEC_ENABLE_B, 1);
>
> gpio_direction_output should be set only once for each GPIO.
>
> After you set the direction (input or output) you should use
> 'gpio_set_value' for setting the GPIO to 0 or 1.

That was my understanding too. But some board file used it in the above 
way. Hope I got a wrong example. I'll fix this.

> Also, please remove the:
>
>> +       gpio_direction_output(GPIO_FEC_RESET_B, 1);
>> +       gpio_direction_output(GPIO_FEC_ENABLE_B, 1);
>
> lines, as the setting to 1 should be after the delay (via gpio_set_value).

Any idea about the delay value?

Thanks,
Vikram
Fabio Estevam - June 11, 2012, 3:01 p.m.
On Mon, Jun 11, 2012 at 11:58 AM, Vikram Narayanan <vikram186@gmail.com> wrote:
> Hi Fabio,
>
>
> On 6/10/2012 8:08 PM, Fabio Estevam wrote:
>>
>> On Sun, Jun 10, 2012 at 10:03 AM, Vikram Narayanan<vikram186@gmail.com>
>>  wrote:
>>
>>>        /* drop PHY power and assert reset (low) */
>>> -       val = readl(&gpio4->gpio_dr)&  ~((1<<  7) | (1<<  9));
>>> -       writel(val,&gpio4->gpio_dr);
>>>
>>> -       val = readl(&gpio4->gpio_dir) | (1<<  7) | (1<<  9);
>>> -       writel(val,&gpio4->gpio_dir);
>>>
>>> +       gpio_direction_output(GPIO_FEC_RESET_B, 0);
>>> +       gpio_direction_output(GPIO_FEC_ENABLE_B, 0);
>>> +
>>> +       gpio_direction_output(GPIO_FEC_RESET_B, 1);
>>> +       gpio_direction_output(GPIO_FEC_ENABLE_B, 1);
>>
>>
>> gpio_direction_output should be set only once for each GPIO.
>>
>> After you set the direction (input or output) you should use
>> 'gpio_set_value' for setting the GPIO to 0 or 1.
>
>
> That was my understanding too. But some board file used it in the above way.
> Hope I got a wrong example. I'll fix this.
>
>
>> Also, please remove the:
>>
>>> +       gpio_direction_output(GPIO_FEC_RESET_B, 1);
>>> +       gpio_direction_output(GPIO_FEC_ENABLE_B, 1);
>>
>>
>> lines, as the setting to 1 should be after the delay (via gpio_set_value).
>
>
> Any idea about the delay value?

You should not change the delay value, just use the same as in current code.

My comment was only for you to remove the

gpio_direction_output(GPIO_FEC_RESET_B, 1);
gpio_direction_output(GPIO_FEC_ENABLE_B, 1);

,immediately after setting to 0.

Regards,

Fabio Estevam

Patch

diff --git a/board/karo/tx25/tx25.c b/board/karo/tx25/tx25.c
index 2a29943..6d03130 100644
--- a/board/karo/tx25/tx25.c
+++ b/board/karo/tx25/tx25.c
@@ -34,14 +34,13 @@ 
 DECLARE_GLOBAL_DATA_PTR;
 
 #ifdef CONFIG_FEC_MXC
+#define GPIO_FEC_RESET_B	MXC_GPIO_PORT_TO_NUM(4, 7)
+#define GPIO_FEC_ENABLE_B	MXC_GPIO_PORT_TO_NUM(4, 9)
 void tx25_fec_init(void)
 {
 	struct iomuxc_mux_ctl *muxctl;
 	struct iomuxc_pad_ctl *padctl;
-	u32 val;
 	u32 gpio_mux_mode = MX25_PIN_MUX_MODE(5);
-	struct gpio_regs *gpio4 = (struct gpio_regs *)IMX_GPIO4_BASE;
-	struct gpio_regs *gpio3 = (struct gpio_regs *)IMX_GPIO3_BASE;
 	u32 saved_rdata0_mode, saved_rdata1_mode, saved_rx_dv_mode;
 
 	debug("tx25_fec_init\n");
@@ -66,18 +65,18 @@  void tx25_fec_init(void)
 	writel(0x0, &padctl->pad_d11);
 
 	/* drop PHY power and assert reset (low) */
-	val = readl(&gpio4->gpio_dr) & ~((1 << 7) | (1 << 9));
-	writel(val, &gpio4->gpio_dr);
-	val = readl(&gpio4->gpio_dir) | (1 << 7) | (1 << 9);
-	writel(val, &gpio4->gpio_dir);
+	gpio_direction_output(GPIO_FEC_RESET_B, 0);
+	gpio_direction_output(GPIO_FEC_ENABLE_B, 0);
+
+	gpio_direction_output(GPIO_FEC_RESET_B, 1);
+	gpio_direction_output(GPIO_FEC_ENABLE_B, 1);
 
 	mdelay(5);
 
 	debug("resetting phy\n");
 
 	/* turn on PHY power leaving reset asserted */
-	val = readl(&gpio4->gpio_dr) | 1 << 9;
-	writel(val, &gpio4->gpio_dr);
+	gpio_direction_output(GPIO_FEC_ENABLE_B, 1);
 
 	mdelay(10);
 
@@ -107,19 +106,16 @@  void tx25_fec_init(void)
 	/*
 	 * set each to 1 and make each an output
 	 */
-	val = readl(&gpio3->gpio_dr) | (1 << 10) | (1 << 11) | (1 << 12);
-	writel(val, &gpio3->gpio_dr);
-	val = readl(&gpio3->gpio_dir) | (1 << 10) | (1 << 11) | (1 << 12);
-	writel(val, &gpio3->gpio_dir);
+	gpio_direction_output(MXC_GPIO_PORT_TO_NUM(3, 10), 1);
+	gpio_direction_output(MXC_GPIO_PORT_TO_NUM(3, 11), 1);
+	gpio_direction_output(MXC_GPIO_PORT_TO_NUM(3, 12), 1);
 
 	mdelay(22);		/* this value came from RedBoot */
 
 	/*
 	 * deassert PHY reset
 	 */
-	val = readl(&gpio4->gpio_dr) | 1 << 7;
-	writel(val, &gpio4->gpio_dr);
-	writel(val, &gpio4->gpio_dr);
+	gpio_direction_output(GPIO_FEC_RESET_B, 1);
 
 	mdelay(5);