On Thu 2008-01-31 10:29:30, Stephen Blackheath [to Foxconn] wrote:
> Pavel & all,
>
>
> Pavel Machek wrote:
>
> >> +/* I/O ports and bit definitions for version 2 of the hardware */
> >> +
> >> +struct MEMCCR {
> >> + unsigned short PCCOR; /* Configuration Option Register */
> >> + unsigned short PCCSR; /* Configuration and Status Register */
> >> + unsigned short PCPRR; /* Pin Replacemant Register */
> >> + unsigned short PCSCR; /* Socket and Copy Register */
> >> + unsigned short PCESR; /* Extendend Status Register */
> >> + unsigned short PCIOB; /* I/O Base Register */
> >> +};
> > Could we get better names? PCIOB is cryptic, pci_io_base is pretty
> > good.
> >
> >
> We should keep these names because they are part of the interface
> between host and card defined by the manufacturer.
No. Use sensible names, and put manufacturer-defined 5-letter crap in
the comments. Heck, notice that they just took first letter of each
word of good name....
> > Is some locking needed around *hw?
> >
> I don't think so, but I'm happy to be corrected.
You have a structure, and are accessing its fields from interrupts. I
assume you access the fields outside interrupt, too? As the fields are
not of atomic_t, I believe you need locking.
(Oh, and I should have said that earlier: Thanks for the driver and
congratulations for getting it this far).
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures)
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
