[U-Boot] [PATCH] ppc44x: config GPIOs for USB on canyonlands board

[U-Boot] [PATCH] ppc44x: config GPIOs for USB on canyonlands board

[Rupjyoti Sarmah]

The GPIO 16 and 19 reconfiguration should be done once USB is initialized.
So moved the reconfiguration to the USB init  function 

Signed-off-by: Rupjyoti Sarmah <rsarmah@apm.com>
---
 board/amcc/canyonlands/canyonlands.c |   19 +++++++++----------
 1 files changed, 9 insertions(+), 10 deletions(-)

diff --git a/board/amcc/canyonlands/canyonlands.c b/board/amcc/canyonlands/canyonlands.c
index 158f7bb..1e547f7 100644
--- a/board/amcc/canyonlands/canyonlands.c
+++ b/board/amcc/canyonlands/canyonlands.c
@@ -195,16 +195,6 @@ int board_early_init_f(void)
 	mtdcr(AHB_TOP, 0x8000004B);
 	mtdcr(AHB_BOT, 0x8000004B);
 
-	if (pvr_460ex()) {
-		/*
-		 * Configure USB-STP pins as alternate and not GPIO
-		 * It seems to be neccessary to configure the STP pins as GPIO
-		 * input at powerup (perhaps while USB reset is asserted). So
-		 * we configure those pins to their "real" function now.
-		 */
-		gpio_config(16, GPIO_OUT, GPIO_ALT1, GPIO_OUT_1);
-		gpio_config(19, GPIO_OUT, GPIO_ALT1, GPIO_OUT_1);
-	}
 #endif
 
 	return 0;
@@ -222,6 +212,15 @@ int usb_board_init(void)
 	val &= ~(BCSR_USBCTRL_OTG_RST | BCSR_USBCTRL_HOST_RST);
 	out_8(&bcsr_data->usb_ctrl, val);
 
+	/*
+	 * Configure USB-STP pins as alternate and not GPIO
+	 * It seems to be neccessary to configure the STP pins as GPIO
+	 * input at powerup (perhaps while USB reset is asserted). So
+	 * we configure those pins to their "real" function now.
+	 */
+	gpio_config(16, GPIO_OUT, GPIO_ALT1, GPIO_OUT_1);
+	gpio_config(19, GPIO_OUT, GPIO_ALT1, GPIO_OUT_1);
+
 	return 0;
 }
 
-- 
1.5.6.3

[U-Boot] [PATCH] ppc44x: config GPIOs for USB on canyonlands board

[Wolfgang Denk]

Dear Rupjyoti Sarmah,

In message <201009231421.o8NELOE9022250@amcc.com> you wrote:
> 
> The GPIO 16 and 19 reconfiguration should be done once USB is initialized.
> So moved the reconfiguration to the USB init  function 

Does the Linux kernel perform the same initiaalization of the GPIO
pins?  If not, then your change will most likely cause the USB is not
working in Linux unless you used USB in U-Boot.


> --- a/board/amcc/canyonlands/canyonlands.c
> +++ b/board/amcc/canyonlands/canyonlands.c
> @@ -195,16 +195,6 @@ int board_early_init_f(void)
>  	mtdcr(AHB_TOP, 0x8000004B);
>  	mtdcr(AHB_BOT, 0x8000004B);
>  
> -	if (pvr_460ex()) {
> -		/*
> -		 * Configure USB-STP pins as alternate and not GPIO
> -		 * It seems to be neccessary to configure the STP pins as GPIO
> -		 * input at powerup (perhaps while USB reset is asserted). So
> -		 * we configure those pins to their "real" function now.
> -		 */
> -		gpio_config(16, GPIO_OUT, GPIO_ALT1, GPIO_OUT_1);
> -		gpio_config(19, GPIO_OUT, GPIO_ALT1, GPIO_OUT_1);
> -	}
>  #endif
>  
>  	return 0;
> @@ -222,6 +212,15 @@ int usb_board_init(void)
>  	val &= ~(BCSR_USBCTRL_OTG_RST | BCSR_USBCTRL_HOST_RST);
>  	out_8(&bcsr_data->usb_ctrl, val);
>  
> +	/*
> +	 * Configure USB-STP pins as alternate and not GPIO
> +	 * It seems to be neccessary to configure the STP pins as GPIO
> +	 * input at powerup (perhaps while USB reset is asserted). So
> +	 * we configure those pins to their "real" function now.
> +	 */
> +	gpio_config(16, GPIO_OUT, GPIO_ALT1, GPIO_OUT_1);
> +	gpio_config(19, GPIO_OUT, GPIO_ALT1, GPIO_OUT_1);
> +

You drop the "if (pvr_460ex()) {" part here. Is this intentional?


Also, when adding this code to usb_board_init(), would it not be
logical to undo this initialization in usb_board_stop()?

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
In theory, there is no difference between  theory  and  practice.  In
practice, however, there is.

[U-Boot] [PATCH] ppc44x: config GPIOs for USB on canyonlands board

[Rupjyoti Sarmah]

Hello Wolfgang,


> So moved the reconfiguration to the USB init  function

Does the Linux kernel perform the same initiaalization of the GPIO
pins?  If not, then your change will most likely cause the USB is not
working in Linux unless you used USB in U-Boot.

RUP >> Yes, it does.

> --- a/board/amcc/canyonlands/canyonlands.c
> +

You drop the "if (pvr_460ex()) {" part here. Is this intentional?
RUP>> Yes

Also, when adding this code to usb_board_init(), would it not be
logical to undo this initialization in usb_board_stop()?

RUP >> Ok, I will include your suggestion and resubmit


Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
In theory, there is no difference between  theory  and  practice.  In
practice, however, there is.

[U-Boot] [PATCH] ppc44x: config GPIOs for USB on canyonlands board

[Feng Kan]

>
> Also, when adding this code to usb_board_init(), would it not be
> logical to undo this initialization in usb_board_stop()?
>
> RUP >> Ok, I will include your suggestion and resubmit
>

FKAN: Dear Wolfgang, is the symmetry needed here? If a user
plan to use the usb, he will trigger the function. Otherwise, on
a stop what value would we put it back to.

>
> Best regards,
>
> Wolfgang Denk
>
> --
> DENX Software Engineering GmbH, ? ? MD: Wolfgang Denk & Detlev Zundel
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
> In theory, there is no difference between ?theory ?and ?practice. ?In
> practice, however, there is.
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
>

[U-Boot] [PATCH] ppc44x: config GPIOs for USB on canyonlands board

[Wolfgang Denk]

Dear Feng Kan,

In message <AANLkTin-rbM0NDqZmZKCYp45-m9S+3H_Fb7DwzzEHkUw@mail.gmail.com> you wrote:
>
> FKAN: Dear Wolfgang, is the symmetry needed here? If a user
> plan to use the usb, he will trigger the function. Otherwise, on
> a stop what value would we put it back to.

Design rules say:

	Shall initialize only such peripherals used by U-Boot itself,
	and must deinitialize them after use. Note that especially the
	deinitialization is mandatory! 

Isn't this GPI reset to the initial values part of the
deinitialization sequence?

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
 The software required `Windows 95 or better', so I installed Linux.

[U-Boot] [PATCH] ppc44x: config GPIOs for USB on canyonlands board

[Feng Kan]

On Fri, Sep 24, 2010 at 10:43 AM, Wolfgang Denk <wd@denx.de> wrote:
> Dear Feng Kan,
>
> In message <AANLkTin-rbM0NDqZmZKCYp45-m9S+3H_Fb7DwzzEHkUw@mail.gmail.com> you wrote:
>>
>> FKAN: Dear Wolfgang, is the symmetry needed here? If a user
>> plan to use the usb, he will trigger the function. Otherwise, on
>> a stop what value would we put it back to.
>
> Design rules say:
>
> ? ? ? ?Shall initialize only such peripherals used by U-Boot itself,
> ? ? ? ?and must deinitialize them after use. Note that especially the
> ? ? ? ?deinitialization is mandatory!
>
FKAN: I agree, thanks. However, this conflict with one of your other
rule "Don't make U-Boot slow". If another software entity wish to use
the GPIO after, the code would deinit to gpio state and the
other init would init gpio to the same state. Essentially doubling the
code doing the same thing.

> Isn't this GPI reset to the initial values part of the
> deinitialization sequence?

FKAN: In this case, gpio is double muxed in functionality. Do we need
to remember state if the gpio is tri-muxed?

>
> Best regards,
>
> Wolfgang Denk
>
> --
> DENX Software Engineering GmbH, ? ? MD: Wolfgang Denk & Detlev Zundel
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
> ?The software required `Windows 95 or better', so I installed Linux.
>

[U-Boot] [PATCH] ppc44x: config GPIOs for USB on canyonlands board

[Detlev Zundel]

Hi Feng,

[general remark - isn't it possible that you quote mails "the regular
way"[1]?  I'm having a hard time to distinguish the new text from the
original text].

> On Fri, Sep 24, 2010 at 10:43 AM, Wolfgang Denk <wd@denx.de> wrote:
>> Dear Feng Kan,
>>
>> In message <AANLkTin-rbM0NDqZmZKCYp45-m9S+3H_Fb7DwzzEHkUw@mail.gmail.com> you wrote:
>>>
>>> FKAN: Dear Wolfgang, is the symmetry needed here? If a user
>>> plan to use the usb, he will trigger the function. Otherwise, on
>>> a stop what value would we put it back to.
>>
>> Design rules say:
>>
>> ? ? ? ?Shall initialize only such peripherals used by U-Boot itself,
>> ? ? ? ?and must deinitialize them after use. Note that especially the
>> ? ? ? ?deinitialization is mandatory!
>>
> FKAN: I agree, thanks. However, this conflict with one of your other
> rule "Don't make U-Boot slow". If another software entity wish to use
> the GPIO after, the code would deinit to gpio state and the
> other init would init gpio to the same state. Essentially doubling the
> code doing the same thing.

The time it takes to initialize a GPIO pin is really negligible in
comparision to the gain of robustness and correctness of the code.  When
a driver needs the pin in a certain state, it should initialize this and
_not_ depend on any other piece of software.  This is the well known
rule of modularity[2].

>> Isn't this GPI reset to the initial values part of the
>> deinitialization sequence?
>
> FKAN: In this case, gpio is double muxed in functionality. Do we need
> to remember state if the gpio is tri-muxed?

I don't understand this question.  The de-initialization is meant to
stop functional blocks and put pins into a "conservative state",
i.e. configure GPIOs as inputs so that they don't drive possible shared
lines.  Code using the pins will surely know which mode they need and
put the pins into this mode upon initialization.

Cheers
  Detlev

[1] http://www.netmeister.org/news/learn2quote.html
[2] http://www.catb.org/~esr/writings/taoup/html/ch01s06.html#id2877537

-- 
A statistician can have his head in an oven and his feet in ice, and
he will say that on the average he feels fine.
--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: dzu at denx.de