Date: Thu, 31 Jan 2008 00:15:21 +0100
From: Pavel Machek <pavel@ucw.cz>
To: "Stephen Blackheath [to Foxconn]" <stephen@symmetric.co.nz>
Cc: David Sterba <dsterba@suse.cz>, torvalds@linux-foundation.org,
       linux-kernel@vger.kernel.org, jkosina@suse.cz, benm@symmetric.co.nz
Subject: Re: [PATCH] ipwireless: driver for 3G PC Card
Message-ID: <20080130231521.GA5937@elf.ucw.cz>
References: <20080128171929.GA3906@ds.suse.cz> <20080130134046.GB5139@elf.ucw.cz> <47A0EC3A.1040502@symmetric.co.nz>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <47A0EC3A.1040502@symmetric.co.nz>
User-Agent: Mutt/1.5.17 (2007-11-01)
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 1764
Lines: 47

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 majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/