Subject: Re: linux-next: manual merge of the sound tree with the arm tree

From: Takashi Iwai <[email-masked]>
Date: Wed, 11 Mar 2009 11:02:47 +0100

At Wed, 11 Mar 2009 09:41:27 +0000,
Russell King wrote:
> 
> On Wed, Mar 11, 2009 at 10:18:40AM +0100, Takashi Iwai wrote:
> > At Wed, 11 Mar 2009 08:52:54 +0000,
> > Russell King wrote:
> > > 
> > > On Tue, Mar 10, 2009 at 02:45:28PM +1100, Stephen Rothwell wrote:
> > > > Hi Takashi,
> > > > 
> > > > Today's linux-next merge of the sound tree got a conflict in
> > > > arch/arm/mach-shark/include/mach/io.h between commit
> > > > eab184c2362567f2b2e951b7bd0e0d353a7e5091 ("[ARM] 5363/1: Shark cleanup
> > > > and new defconfig") from the arm tree and commit
> > > > 8150bc886be5ce3cc301a2baca1fcf2cf7bd7f39 ("S3C24XX: Move and update IIS
> > > > headers") from the sound tree.
> > > > 
> > > > I fixed it up (see below) and can carry the fix as necessary.
> > > 
> > > NAK to this patch.
> > > 
> > > The greater concern is WTF are cleanup patches for the ARM architecture
> > > going via the sound tree.  Especially patches which I have concerns about.
> > > 
> > > > --- a/arch/arm/mach-shark/include/mach/io.h
> > > > +++ b/arch/arm/mach-shark/include/mach/io.h
> > > > @@@ -11,10 -11,10 +11,10 @@@
> > > >   #ifndef __ASM_ARM_ARCH_IO_H
> > > >   #define __ASM_ARM_ARCH_IO_H
> > > >   
> > > >  -#define PCIO_BASE	0xe0000000
> > > >  -#define IO_SPACE_LIMIT	0xffffffff
> > > >  +#define IO_SPACE_LIMIT 0xffffffff
> > > >   
> > > > - #define __io(a)                 ((void __iomem *)(0xe0000000 + (a)))
> > > >  -#define __io(a)		__typesafe_io(PCIO_BASE + (a))
> > > >  -#define __mem_pci(addr)	(addr)
> > > > ++#define __io(a)		__typesafe_io(0xe0000000 + (a))
> > > 
> > > Do not use __typesafe_io() with the addition here without first ensuring
> > > that you've investigated whether it causes the compiler to mis-optimise
> > > the code.
> > 
> > Hrm, is that particular change also in ARM tree, no?  Judging from the
> > conflict-diff above, it looks like the patch just follows that...
> > 
> > > Takashi - please remove all such patches from the sound tree.  They
> > > should not be in there.
> > 
> > Oh, I thought Mark already sent you Ben's patches.
> 
> What is in my tree is:
> 
> ------
> diff --git a/arch/arm/mach-shark/include/mach/io.h b/arch/arm/mach-shark/include/mach/io.h
> index c5cee82..9ccbcec 100644
> --- a/arch/arm/mach-shark/include/mach/io.h
> +++ b/arch/arm/mach-shark/include/mach/io.h
> @@ -11,10 +11,10 @@
>  #ifndef __ASM_ARM_ARCH_IO_H
>  #define __ASM_ARM_ARCH_IO_H
> 
> -#define PCIO_BASE      0xe0000000
> -#define IO_SPACE_LIMIT 0xffffffff
> +#define IO_SPACE_LIMIT 0xffffffff
> 
> -#define __io(a)                ((void __iomem *)(PCIO_BASE + (a)))
> -#define __mem_pci(addr)        (addr)
> +#define __io(a)                 ((void __iomem *)(0xe0000000 + (a)))
> +
> +#define __mem_pci(addr) (addr)
> 
>  #endif
> ------
> 
> which is basically the same thing from Alexander Schulz but without the
> addition of __typesafe_io(), and it's that which I'm objecting to.

OK, fair enough.

> > Sorry if it wasn't done yet.
> 
> It seems not.  Searching for all messages from Mark containing the
> string 'typesafe' results in no matches for messages since 1st January.

Hm, looks like there was some misunderstanding in communications.

> > Basically it's two patches to create a branch to move and update IIS
> > and audio.h headers.  Since some drivers require these changes, they
> > have to be in the sound tree, too.  Otherwise the tree gets
> > inconsistent and can't be built.
> 
> Maybe, but the point here is that's not the only thing it does.  It also
> makes other changes _as well_ that are not relevent to the changes you're
> talking about above.
> 
> > The relevant changes are in another branch (s3c-iis-header) of asoc
> > tree so that it can be easily pulled to another tree:
> >    git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound-2.6.git s3c-iis-header
> > 
> > So, if you can pull these changes above into your GIT tree simply via
> > "git pull", everything will settle down (as both trees share the same
> > GIT objects).
> 
> Why do I want to pull stuff that I consider broken into my tree?  Please
> get rid of the use of __typesafe_io in the shark io.h header in your tree
> or drop these patches entirely and ask the submitter to fix (whatever's
> easier for you.)

It's not too big issue to revert the changes, but it'd be better to
fix the spot as Mark suggested.  The continuous GIT tree history makes
the life of others easier a lot.

Thanks,

Takashi