[U-Boot-Users] reg ISP 1561 integration with u-boot1.1.6

[U-Boot-Users] reg ISP 1561 integration with u-boot1.1.6

[mahendra varman]

Hi,

Iam Using Philips PCI based  ISP 1561 OHCI usb controller in a  PPC7410
processor  board.

For integrating the ISP 1561 into U-boot 1.1.6  wht config commands i have
to add... (I need to make cmd_usb.c in u boot 1.1.6 working for my device )
?

Please Guide me..

Thanks and regards
R.Mahendran
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.denx.de/pipermail/u-boot/attachments/20070104/ed7be7f2/attachment.htm 

[U-Boot-Users] reg ISP 1561 integration with u-boot1.1.6

[Matthias Fuchs]

Hi,

PCI OHCI controllers are currently not supported by U-Boot. Only some on-chip 
host controllers are currently supported - even though they are OHCI 
compatible PCI OHCI is not that easy because of the endianess.

I also need to get an OHCI controller working from U-Boot to access a mass 
storage device. It's also the ISP1561 :-) I started to work on a the 
testing-USB branch but it's not working yet.

Matthias

On Thursday 04 January 2007 12:58, mahendra varman wrote:
> Hi,
> 
> Iam Using Philips PCI based  ISP 1561 OHCI usb controller in a  PPC7410
> processor  board.
> 
> For integrating the ISP 1561 into U-boot 1.1.6  wht config commands i have
> to add... (I need to make cmd_usb.c in u boot 1.1.6 working for my device )
> ?
> 
> Please Guide me..
> 
> Thanks and regards
> R.Mahendran
> 

-- 
-----------------------------------------------------------------------
Dipl.-Ing. Matthias Fuchs             esd electronic system design gmbh
http://www.esd-electronics.com                    Vahrenwalder Str. 207
phone: +49-511-37298-0, fax: -68                30165 Hannover, Germany
-----------------------------------------------------------------------

[U-Boot-Users] reg ISP 1561 integration with u-boot1.1.6

[Matthias Fuchs]

Hi,

it's working. I modified the testing-USB branch to run against a PCI OHCI 
controller (Philips ISP1561) on the APC405 board (PowerPC PPC405GPr). 

Enumeration and USB storage is working. I do not dare to say 'working well'.

The main changes concern endianess fixes. The former code only supports OHCI 
controller with the same endianess as the CPU. The other fix allows a offset 
to access the system RAM by the busmastering OHCI controller. I called this 
constant CFG_PCIRAM_BASE. But the name is a little bit confusion, isn't it?

Here are my (first) questions: 
1) What do I have to do in usb_board_stop() and usb_board_init_fail()?
My current code can only initialize the OHCI controller once. A 2nd 'usb 
start' does not find any devices.

2) I had to disable the 'return -1' statement for an unfinished urb in 
sohci_submit_job(). Why isn't it finished?

Please do not blame me for the board dependent code still in the usb_ohci.c 
file. I will move it later.

The modified code is not tested on any other board (e.g. with on-chip OHCI 
controllers). I might break other boards.

Matthias

On Friday 05 January 2007 06:43, mahendra varman wrote:
> Hi,
> 
> Have u successfully enumerated the usb devices thru ISP1561 ?
> Iam facing problem even in enumeration ....
-- 
-----------------------------------------------------------------------
Dipl.-Ing. Matthias Fuchs             esd electronic system design gmbh
http://www.esd-electronics.com                    Vahrenwalder Str. 207
phone: +49-511-37298-0, fax: -68                30165 Hannover, Germany
-----------------------------------------------------------------------

-------------- next part --------------
A non-text attachment was scrubbed...
Name: u-boot_testing-USB_ISP1561-on-APC405-20070110.patch
Type: text/x-diff
Size: 11589 bytes
Desc: not available
Url : http://lists.denx.de/pipermail/u-boot/attachments/20070110/587bcb49/attachment.patch 

[U-Boot-Users] reg ISP 1561 integration with u-boot1.1.6

[Markus Klotzbücher]

Hi Matthias,

Matthias Fuchs <matthias.fuchs@esd-electronics.com> writes:

> it's working. I modified the testing-USB branch to run against a PCI OHCI 
> controller (Philips ISP1561) on the APC405 board (PowerPC PPC405GPr). 
>
> Enumeration and USB storage is working. I do not dare to say 'working well'.
>
> The main changes concern endianess fixes. The former code only supports OHCI 
> controller with the same endianess as the CPU. The other fix allows a offset 

IIRC same problem with the 440EP and MPC5200, thats what the extra
#ifdef in drivers/usb_ohci.c:103 is for.

> to access the system RAM by the busmastering OHCI controller. I called this 
> constant CFG_PCIRAM_BASE. But the name is a little bit confusion, isn't it?
>
> Here are my (first) questions: 
> 1) What do I have to do in usb_board_stop() and usb_board_init_fail()?
> My current code can only initialize the OHCI controller once. A 2nd 'usb 
> start' does not find any devices.

You might not have to do anything, but maybe you'll need to do some
board dependant cleanup for example. If not just leave them empty.

> 2) I had to disable the 'return -1' statement for an unfinished urb in 
> sohci_submit_job(). Why isn't it finished?

I really don't know, I guess only you can answer this!

> Please do not blame me for the board dependent code still in the usb_ohci.c 
> file. I will move it later.

Who else shall we blame then ;-) ?

Regards

Markus Klotzbuecher

[U-Boot-Users] reg ISP 1561 integration with u-boot1.1.6

[Matthias Fuchs]

On Wednesday 10 January 2007 23:01, Markus Klotzb?cher wrote:
> Hi Matthias,
> 
> Matthias Fuchs <matthias.fuchs@esd-electronics.com> writes:
> 
> > it's working. I modified the testing-USB branch to run against a PCI OHCI 
> > controller (Philips ISP1561) on the APC405 board (PowerPC PPC405GPr). 
> >
> > Enumeration and USB storage is working. I do not dare to say 'working well'.
> >
> > The main changes concern endianess fixes. The former code only supports OHCI 
> > controller with the same endianess as the CPU. The other fix allows a offset 
> 
> IIRC same problem with the 440EP and MPC5200, thats what the extra
> #ifdef in drivers/usb_ohci.c:103 is for.
These are different issues! usb_ohci.c uses readl and writel to access the 
controller's registers from the CPU (e.g. ohci.regs). The original code never
swaps here. But a PCI OHCI controller on a PowerPC needs it. The mXX_swap 
macros are used to swap data fields in structures that are passed to the host 
controller indirectly.
> 
> > to access the system RAM by the busmastering OHCI controller. I called this 
> > constant CFG_PCIRAM_BASE. But the name is a little bit confusion, isn't it?
> >
> > Here are my (first) questions: 
> > 1) What do I have to do in usb_board_stop() and usb_board_init_fail()?
> > My current code can only initialize the OHCI controller once. A 2nd 'usb 
> > start' does not find any devices.
> 
> You might not have to do anything, but maybe you'll need to do some
> board dependant cleanup for example. If not just leave them empty.
I thought that I have to do something here because stopping and restarting 
the usb stuff does not work at the moment on my system. Does it work on other
systems? I do not have any target with onchip USB at the moment 
(...but comming soon :-). Did anyone test the merged code on a 440EP so far?
> 
> > 2) I had to disable the 'return -1' statement for an unfinished urb in 
> > sohci_submit_job(). Why isn't it finished?
> 
> I really don't know, I guess only you can answer this!
This answer helps me a lot. Why is the urb_finished stuff needed? I go better
without :-)
> 
> > Please do not blame me for the board dependent code still in the usb_ohci.c 
> > file. I will move it later.
> 
> Who else shall we blame then ;-) ?
So please blame be. I just though about leaving the generic PCI OHCI code 
in the file. When it has been made fully generic, it is not CPU or even board 
dependant. PCI device identification constants (better classcode) could be 
defined in the board config header and that's it for CONFIG_USB_OHCI_PCI.

Matthias

[U-Boot-Users] reg ISP 1561 integration with u-boot1.1.6

[Markus Klotzbücher]

Hi Matthias,

Matthias Fuchs <matthias.fuchs@esd-electronics.com> writes:

> On Wednesday 10 January 2007 23:01, Markus Klotzb?cher wrote:
>> Matthias Fuchs <matthias.fuchs@esd-electronics.com> writes:

>> > The main changes concern endianess fixes. The former code only supports OHCI 
>> > controller with the same endianess as the CPU. The other fix allows a offset 
>> 
>> IIRC same problem with the 440EP and MPC5200, thats what the extra
>> #ifdef in drivers/usb_ohci.c:103 is for.
> These are different issues! usb_ohci.c uses readl and writel to access the 
> controller's registers from the CPU (e.g. ohci.regs). The original code never
> swaps here. But a PCI OHCI controller on a PowerPC needs it. The mXX_swap 
> macros are used to swap data fields in structures that are passed to the host 
> controller indirectly.

I get it. So in the end we have four cases: byte swapping register
access or not _and_ byte swapping data or not. Right? Doesn't sound too
complicated.

>> > Here are my (first) questions: 
>> > 1) What do I have to do in usb_board_stop() and usb_board_init_fail()?
>> > My current code can only initialize the OHCI controller once. A 2nd 'usb 
>> > start' does not find any devices.
>> 
>> You might not have to do anything, but maybe you'll need to do some
>> board dependant cleanup for example. If not just leave them empty.
> I thought that I have to do something here because stopping and restarting 
> the usb stuff does not work at the moment on my system. Does it work on other
> systems? I do not have any target with onchip USB at the moment 
> (...but comming soon :-). Did anyone test the merged code on a 440EP so far?

Yes, I did test it successfully.

>> > 2) I had to disable the 'return -1' statement for an unfinished urb in 
>> > sohci_submit_job(). Why isn't it finished?
>> 
>> I really don't know, I guess only you can answer this!
> This answer helps me a lot. Why is the urb_finished stuff needed? I go better
> without :-)

Did you see the comment in usb_ohci.c:1355? It's to make sure a
transaction has completed before a new one is started. I'm not insisting
to keep this if it's useless, but we need to prove that first. Obviously
it was put there for some reason.

>> > Please do not blame me for the board dependent code still in the usb_ohci.c 
>> > file. I will move it later.
>> 
>> Who else shall we blame then ;-) ?
> So please blame be. I just though about leaving the generic PCI OHCI code 
> in the file. When it has been made fully generic, it is not CPU or even board 
> dependant. PCI device identification constants (better classcode) could be 
> defined in the board config header and that's it for CONFIG_USB_OHCI_PCI.

That would be great!

Regards

Markus Klotzbuecher

[U-Boot-Users] reg ISP 1561 integration with u-boot1.1.6

[Matthias Fuchs]

Hi Markus,

On Thursday 11 January 2007 11:17, Markus Klotzb?cher wrote:
> Hi Matthias,
> 
> Matthias Fuchs <matthias.fuchs@esd-electronics.com> writes:
> 
> > On Wednesday 10 January 2007 23:01, Markus Klotzb?cher wrote:
> >> Matthias Fuchs <matthias.fuchs@esd-electronics.com> writes:
> 
> >> > The main changes concern endianess fixes. The former code only supports OHCI 
> >> > controller with the same endianess as the CPU. The other fix allows a offset 
> >> 
> >> IIRC same problem with the 440EP and MPC5200, thats what the extra
> >> #ifdef in drivers/usb_ohci.c:103 is for.
> > These are different issues! usb_ohci.c uses readl and writel to access the 
> > controller's registers from the CPU (e.g. ohci.regs). The original code never
> > swaps here. But a PCI OHCI controller on a PowerPC needs it. The mXX_swap 
> > macros are used to swap data fields in structures that are passed to the host 
> > controller indirectly.
> 
> I get it. So in the end we have four cases: byte swapping register
> access or not _and_ byte swapping data or not. Right? Doesn't sound too
> complicated.
What about taking some code / macros from the linux kernel (usb/host/ohci.h)?
It seems that all we need is the CONFIG_USB_OHCI_BIG_ENDIAN define.

> >> > 2) I had to disable the 'return -1' statement for an unfinished urb in 
> >> > sohci_submit_job(). Why isn't it finished?
> >> 
> >> I really don't know, I guess only you can answer this!
> > This answer helps me a lot. Why is the urb_finished stuff needed? I go better
> > without :-)
> 
> Did you see the comment in usb_ohci.c:1355? It's to make sure a
> transaction has completed before a new one is started. I'm not insisting
> to keep this if it's useless, but we need to prove that first. Obviously
> it was put there for some reason.
The code makes sense. But it does not work for me. I will investigate some more time 
on this and figure it out.
> 
> > So please blame be. I just though about leaving the generic PCI OHCI code 
> > in the file. When it has been made fully generic, it is not CPU or even board 
> > dependant. PCI device identification constants (better classcode) could be 
> > defined in the board config header and that's it for CONFIG_USB_OHCI_PCI.
> 
> That would be great!

Regards
Matthias

[U-Boot-Users] reg ISP 1561 integration with u-boot1.1.6

[Wolfgang Denk]

In message <873b6hna01.fsf@denx.de> you wrote:
> 
> I get it. So in the end we have four cases: byte swapping register
> access or not _and_ byte swapping data or not. Right? Doesn't sound too
> complicated.

...or all of this combined.

> Did you see the comment in usb_ohci.c:1355? It's to make sure a
> transaction has completed before a new one is started. I'm not insisting
> to keep this if it's useless, but we need to prove that first. Obviously
> it was put there for some reason.

Check the changelogs. IIRC it *is* there for a purpose.

Best regards,

Wolfgang Denk

-- 
Software Engineering:  Embedded and Realtime Systems,  Embedded Linux
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
If I have seen further it is by standing on the shoulders of  giants.
              - Isaac Newton, Letter to Robert Hooke, 5 February 1676

[U-Boot-Users] reg ISP 1561 integration with u-boot1.1.6

[Wolfgang Denk]

In message <200701110950.08710.matthias.fuchs@esd-electronics.com> you wrote:
>
> These are different issues! usb_ohci.c uses readl and writel to access the 
> controller's registers from the CPU (e.g. ohci.regs). The original code never
> swaps here. But a PCI OHCI controller on a PowerPC needs it. The mXX_swap
> macros are used to swap data fields in structures that are passed to the host
> controller indirectly.

Be careful! Keep in mind that you may need both cases in one system,
like using a PCI OHCI controller on a MPC5200 board where you have to
support USB both on the internal USB controllers and on the PCI card.

See our linuxppc_2_4_devel tree for one possible implementation.


Best regards,

Wolfgang Denk

-- 
Software Engineering:  Embedded and Realtime Systems,  Embedded Linux
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Women are more easily and more deeply terrified ...  generating  more
sheer horror than the male of the species.
	-- Spock, "Wolf in the Fold", stardate 3615.4

[U-Boot-Users] reg ISP 1561 integration with u-boot1.1.6

[Wolfgang Denk]

In message <200701111339.01196.matthias.fuchs@esd-electronics.com> you wrote:
> 
> What about taking some code / macros from the linux kernel (usb/host/ohci.h)?
> It seems that all we need is the CONFIG_USB_OHCI_BIG_ENDIAN define.

No, this is not sufficient. See my previous message.

Also be aware that the U-Boot USB code is much more  restrictive,  so
watch for incompatibilities when copying from Linux sources.


Best regards,

Wolfgang Denk

-- 
Software Engineering:  Embedded and Realtime Systems,  Embedded Linux
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
The evolution of the human race will not be accomplished in  the  ten
thousand  years  of  tame  animals,  but in the million years of wild
animals, because man is and will always be a wild animal.
                                              - Charles Galton Darwin

[U-Boot-Users] reg ISP 1561 integration with u-boot1.1.6

[Markus Klotzbücher]

Hi Matthias,

Matthias Fuchs <matthias.fuchs@esd-electronics.com> writes:

> On Thursday 11 January 2007 11:17, Markus Klotzb?cher wrote:
>> Hi Matthias,
>> 
>> Matthias Fuchs <matthias.fuchs@esd-electronics.com> writes:
>> 
>> > On Wednesday 10 January 2007 23:01, Markus Klotzb?cher wrote:
>> >> Matthias Fuchs <matthias.fuchs@esd-electronics.com> writes:
>> 
>> >> > The main changes concern endianess fixes. The former code only supports OHCI 
>> >> > controller with the same endianess as the CPU. The other fix allows a offset 
>> >> 
>> >> IIRC same problem with the 440EP and MPC5200, thats what the extra
>> >> #ifdef in drivers/usb_ohci.c:103 is for.
>> > These are different issues! usb_ohci.c uses readl and writel to access the 
>> > controller's registers from the CPU (e.g. ohci.regs). The original code never
>> > swaps here. But a PCI OHCI controller on a PowerPC needs it. The mXX_swap 
>> > macros are used to swap data fields in structures that are passed to the host 
>> > controller indirectly.
>> 
>> I get it. So in the end we have four cases: byte swapping register
>> access or not _and_ byte swapping data or not. Right? Doesn't sound too
>> complicated.
> What about taking some code / macros from the linux kernel (usb/host/ohci.h)?
> It seems that all we need is the CONFIG_USB_OHCI_BIG_ENDIAN define.

Seems reasonable. So we'll use these for register accesses, and the rest
stays as is doing CPU dependant byteswapping based on the LITTLEENDIAN
define (include/usb.h). Ok?

>> >> > 2) I had to disable the 'return -1' statement for an unfinished urb in 
>> >> > sohci_submit_job(). Why isn't it finished?
>> >> 
>> >> I really don't know, I guess only you can answer this!
>> > This answer helps me a lot. Why is the urb_finished stuff needed? I go better
>> > without :-)
>> 
>> Did you see the comment in usb_ohci.c:1355? It's to make sure a
>> transaction has completed before a new one is started. I'm not insisting
>> to keep this if it's useless, but we need to prove that first. Obviously
>> it was put there for some reason.
> The code makes sense. But it does not work for me. I will investigate some more time 
> on this and figure it out.

Thanks!

Regards

Markus Klotzbuecher

[U-Boot-Users] reg ISP 1561 integration with u-boot1.1.6

[Wolfgang Denk]

In message <877ivtu003.fsf@denx.de> you wrote:
> 
> Seems reasonable. So we'll use these for register accesses, and the rest
> stays as is doing CPU dependant byteswapping based on the LITTLEENDIAN
> define (include/usb.h). Ok?

Please don't.

Please keep in mind that mixed configurations exist, and it would be
nice to support these, too.

Best regards,

Wolfgang Denk

-- 
Software Engineering:  Embedded and Realtime Systems,  Embedded Linux
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
It is dangerous to be sincere unless you are also stupid.
                                                - George Bernard Shaw

[U-Boot-Users] reg ISP 1561 integration with u-boot1.1.6

[Matthias Fuchs]

Hi Wolfgang, 

On Thursday 11 January 2007 14:13, Wolfgang Denk wrote:
> > These are different issues! usb_ohci.c uses readl and writel to access the 
> > controller's registers from the CPU (e.g. ohci.regs). The original code 
never
> > swaps here. But a PCI OHCI controller on a PowerPC needs it. The mXX_swap
> > macros are used to swap data fields in structures that are passed to the 
host
> > controller indirectly.
> 
> Be careful! Keep in mind that you may need both cases in one system,
> like using a PCI OHCI controller on a MPC5200 board where you have to
> support USB both on the internal USB controllers and on the PCI card.
> 
> See our linuxppc_2_4_devel tree for one possible implementation.

Now it's getting ugly. But I will give it a glace.

Matthias

[U-Boot-Users] reg ISP 1561 integration with u-boot1.1.6

[Matthias Fuchs]

On Thursday 11 January 2007 14:15, Wolfgang Denk wrote:
> In message <200701111339.01196.matthias.fuchs@esd-electronics.com> you 
wrote:
> > 
> > What about taking some code / macros from the linux kernel 
(usb/host/ohci.h)?
> > It seems that all we need is the CONFIG_USB_OHCI_BIG_ENDIAN define.
> 
> No, this is not sufficient. See my previous message.
> 
> Also be aware that the U-Boot USB code is much more  restrictive,  so
> watch for incompatibilities when copying from Linux sources.

My idea is not to copy the ohci implementation, but the access macros and the 
naming convention in order to get a little closer to the kernel's 
implementation.

Matthias

[U-Boot-Users] reg ISP 1561 integration with u-boot1.1.6

[Matthias Fuchs]

Hi Markus,

On Thursday 11 January 2007 15:11, Markus Klotzb?cher wrote:
> >> >> IIRC same problem with the 440EP and MPC5200, thats what the extra
> >> >> #ifdef in drivers/usb_ohci.c:103 is for.
> >> > These are different issues! usb_ohci.c uses readl and writel to access the 
> >> > controller's registers from the CPU (e.g. ohci.regs). The original code never
> >> > swaps here. But a PCI OHCI controller on a PowerPC needs it. The mXX_swap 
> >> > macros are used to swap data fields in structures that are passed to the host 
> >> > controller indirectly.
> >> 
> >> I get it. So in the end we have four cases: byte swapping register
> >> access or not _and_ byte swapping data or not. Right? Doesn't sound too
> >> complicated.
> > What about taking some code / macros from the linux kernel (usb/host/ohci.h)?
> > It seems that all we need is the CONFIG_USB_OHCI_BIG_ENDIAN define.
> 
> Seems reasonable. So we'll use these for register accesses, and the rest
> stays as is doing CPU dependant byteswapping based on the LITTLEENDIAN
> define (include/usb.h). Ok?

Sounds reasonable to me. We still need a more pretty way to handle the +/- CFG_PCIRAM_BASE.
What about some cpu_to_bus/bus_to_cpu macros?

Matthias

[U-Boot-Users] reg ISP 1561 integration with u-boot1.1.6

[Markus Klotzbücher]

Hi Matthias,

Matthias Fuchs <matthias.fuchs@esd-electronics.com> writes:

> Sounds reasonable to me. We still need a more pretty way to handle the
> +/- CFG_PCIRAM_BASE.  What about some cpu_to_bus/bus_to_cpu macros?

Hmm, that sounds a bit confusing too if you're only accessing a on chip
controller. How about just renaming the define to something more generic
such as CFG_OHCI_RAM_OFFSET and adding a nice explanation in the README?
That way it wouldn't seem so strongly related to pci.

Regards

	Markus

[U-Boot-Users] reg ISP 1561 integration with u-boot1.1.6

[Matthias Fuchs]

Hi Markus,

On Friday 12 January 2007 12:07, Markus Klotzb?cher wrote:
> Hi Matthias,
> 
> Matthias Fuchs <matthias.fuchs@esd-electronics.com> writes:
> 
> > Sounds reasonable to me. We still need a more pretty way to handle the
> > +/- CFG_PCIRAM_BASE.  What about some cpu_to_bus/bus_to_cpu macros?
> 
> Hmm, that sounds a bit confusing too if you're only accessing a on chip
> controller. How about just renaming the define to something more generic
> such as CFG_OHCI_RAM_OFFSET and adding a nice explanation in the README?
> That way it wouldn't seem so strongly related to pci.
When dealing with a pci OHCI controller the offset if pci related. But you are 
right, when not running on a PCI platform your name ist better.

BTW, considering Wolfgang's desire for multi ohci controller support, the 
constant will change into a field in the ohci struct.

Matthias