Hi Dimitris,

> 1) You declare registers HcFmRemaining and HcFmNumber as RW, but at
> least for the isp1160, these are read-only.
> 
For ISP1362 they are read/write.

> 2) I now know the meaning of Fld, FExtrt etc. but these are defined in
> header file "bitfield.h", which is only available for certain arm-based
> archs. You use them to create a proper PTD structure. This was one of my
> problems (creating a proper PTD), and I really like your way, but I
> cannot use it on an i386 arch. Now apart from that, this is a very
> platform-specific solution, so we should really come up with a more
> general way of doing it. Any ideas? (or opinions as to why my
> assumptions are wrong. I really am new to all of this, so I might be
> missing something...)
> 
These are generic macros that could be used on any platform. Why they
are located in include/asm-arm/arch-pxa/ I don't know. But we could
achieve the same without using those macros.

> 3) Even if we set aside the problem (2), inside a PTD, there exist some
> 10-bit fields. I noticed that you use just one Fld() for _PTD_COUNT but
> two Fld() for all the other 10-bit fields (for example, you have a
> _PTD_MPS for bits 0-7 of MaxPacketSize and _PTD_MPS_H for bits 8-9). Why
> is that? The thing is that 10-bit fields cross the boundaries of 1
> byte-alignment and enter the next byte, but I don't understand the two
> approaches between _PTD_COUNT and all the others...
> 
The current version of the code available from
http://www.karo-electronics.de/132.0.html has both variants (all 10
bits in one field and a definition for the most significant bits only)
for all such fields.

> 4) What about REG_ACCESS_[R,W,RW,M,MASK]? Why do you need all these? And
> although I've not checked all your code with detail, I can see that you
> only use them when debugging. I'm particularly intrigued with M and
> MASK...
> 
I introduced them to trap illegal register accesses like writing a
read-only register or accessing a 16bit register with a 32bit access.
Furthermore there are some registers in the chip that implement only
part of the bits from the OHCI spec. The remaining bits are stored in
a shadow copy of the register and maintained by the driver. The 'M'
bit tells the register functions that a merge of the value from the HW
register with the shadow copy is required.

> 5) I noticed that you don't use any spinlocks. There is no need to do
> it? How can this happen? (this is a really generic question but I'm not
> yet in the position to propose where exactly you should use one...)
> 
On my platform it didn't make sense to use spinlocks since it isn't
SMP capable. But if the driver is going to be used on SMP capable
platforms too, it doesn't hurt to use spinlocks generally.



Lothar Wassmann


-------------------------------------------------------
This SF.net email is sponsored by: IT Product Guide on ITManagersJournal
Use IT products in your business? Tell us what you think of them. Give us
Your Opinions, Get Free ThinkGeek Gift Certificates! Click to find out more
http://productguide.itmanagersjournal.com/guidepromo.tmpl
_______________________________________________
[EMAIL PROTECTED]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel

Reply via email to