Re: 2.6.18-rc5-mm1: drivers/infiniband/hw/amso1100/c2.c compile error

[Tom Tucker <tom@opengridcomputing.com>]

So to make sure I understand all this...

The purpose of these services is to provide a platform independent API
for reading and writing 16, 32 and 64b values to MMIO devices. The
rationale for needing these services is that there is currently no
platform independent API for efficiently reading and writing these
values to BE devices on MMIO PCI devices. Examples are the mthca and
amso1100 devices.

Two classes of service are needed, atomic services that are interrupt
safe and services that either don't require atomicity or are called with
a suitable lock already held.

Does the API look something like this?

void mmio_wr_be16(__be16 val, void __iomem *addr);
void mmio_wr_be32(__be32 val, void __iomem *addr);
void mmio_wr_be64(__be64 val, void __iomem *addr);

void mmio_atomic_wr_be16(__be16 val, void __iomem *addr);
void mmio_atomic_wr_be32(__be32 val, void __iomem *addr);
void mmio_atomic_wr_be64(__be64 val, void __iomem *addr);

__be16 mmio_rd_be16(void __iomem *addr);
__be32 mmio_rd_be32(void __iomem *addr);
__be64 mmio_rd_be64(void __iomem *addr);

__be16 mmio_atomic_wr_be16(void __iomem *addr);
__be32 mmio_atomic_wr_be32(void __iomem *addr);
__be64 mmio_atomic_wr_be64(void __iomem *addr);


On Fri, 2006-09-01 at 13:04 -0700, Andrew Morton wrote:
> On Fri, 01 Sep 2006 12:53:47 -0700
> Roland Dreier <rdreier@cisco.com> wrote:
> 
> >     Roland> My understanding is that __raw_writeq() is like writeq()
> >     Roland> except not strongly ordered and without the byte-swap on
> >     Roland> big-endian architectures.  The __raw_writeX() variants are
> >     Roland> convenient to avoid having to write inefficient code like
> >     Roland> writel(swab32(foo), ...) when talking to a PCI device that
> >     Roland> wants big-endian data.  Without the raw variant, you end
> >     Roland> up with a double swap on big-endian architectures.
> > 
> > Oh, I left one other thing out: writeq() and __raw_writeq() shold be
> > atomic in the sense that no other transactions should be able to get
> > onto the IO bus in the middle -- so implementing writeq() as two
> > writel()s in a row is not allowed
> > 
> >     Andrew> OK.  Can we please stop hacking around this in drivers and
> > 
> >     Andrew> a) work out what it's supposed to do
> > 
> >     Andrew> b) document that (Documentation/DocBook/deviceiobook.tmpl
> >     Andrew> or code comment or whatever)
> > 
> >     Andrew> c) tell arch maintainers?
> > 
> > Yes, I agree that's a good plan, especially the documentation part.
> > However I would argue that what's in drivers/infiniband/hw/mthca/mthca_doorbell.h 
> > is legitimate: the driver uses __raw_writeq() when it exists and uses
> > two __raw_writel()s properly serialized with a device-specific lock to
> > get exactly the atomicity it needs on 32-bit archs.
> 
> No, driver-specific workarounds are not legitimate, sorry.
> 
> The driver should simply fail to compile on architectures which do not
> implement __raw_writeq().
> 
> We can speed up the process by sending helpful emails to architecture
> maintainers, but they'll notice either way.
> 
> Let's fix it once, and in the correct place.
> 
> > It's an open question what drivers that don't actually need atomicity
> > but just want a convenient way to write 64 bits at time should do.
> 
> Well yeah.  We should sort out the design issues before implementing
> things ;)
> 


-- 
VGER BF report: H 0