Re: [PATCH 1/2] asm-generic: io: remove {read,write} string functions

[Mike Frysinger]

On Friday 27 April 2012 12:53:06 Will Deacon wrote:
> On Fri, Apr 27, 2012 at 05:17:47PM +0100, Mike Frysinger wrote:
> > On Friday 27 April 2012 06:42:55 Will Deacon wrote:
> > > The {read,write}s{b,w,l} functions are not defined across all
> > > architectures and therefore shouldn't be used by portable drivers. We
> > > should encourage driver writers to use the io{read,write}{8,16,32}_rep
> > > functions instead.
> > 
> > well, that isn't true today as a grep of the drivers/ tree shows. 
> > perhaps we should fix that first ?  quite a number of architectures do
> > implement these.
> 
> Sure, and architectures can continue to implement these if they're using
> drivers that require them (I'll reintroduce them for blackfin). They
> shouldn't be used for new architectures though, so any drivers that are
> required by folks using asm-generic/io.h will need converting.

will they though ?  or without documentation, will they simply copy & paste 
the interfaces from another arch to build the driver ?  most people doing arch 
ports (especially new ones) don't have the experience or confidence to push 
back and update the driver rather than squirreling away 6 #define's into their 
arch-specific asm/io.h.

> > > This patch removes the {read,write} string functions for the generic IO
> > > header as they have no place in a new architecture port.
> > 
> > i don't see any file anywhere that describes what the baseline API is
> > supposed to be, and what each set of funcs are for.  there is just the
> > random ugliness in each arch's asm/io.h cobbled together until things
> > work.  i think that also needs to be addressed before we go
> > extending/contracting the API provides by asm-generic/io.h.
> 
> I agree that it's a bit of a mess, but if we can keep the asm-generic
> interfaces clean then it can help driver writers see what we have as a
> common base. If a driver is not portable (maybe for some
> architecture-specific device) then they could use extra accessors if they
> really need to.
> 
> The point of removing these functions is to make it clear that they're not
> part of the baseline API and instead encourage people to use other
> accessors instead if they want to write portable code.

maybe i'm pessimistic, but without addressing the underlying problem (writing 
clear documentation on what should exist, what they're for, and specifically 
what functions *shouldn't* exist), we're just playing a losing game of whack-
a-mole.  you remove an interface that appears to be mostly common, then 
someone else posts a patch to add some other set of interfaces and people 
don't notice, and we just keep flip-flopping.
-mike

Attachment: signature.asc
Description: This is a digitally signed message part.