Re: [PATCH 2/7] Powerpc MSI implementation

[Greg KH <greg@kroah.com>]

On Thu, Jan 11, 2007 at 10:25:19PM +1100, Michael Ellerman wrote:
> Powerpc MSI implementation, based on a collection of "ops" callbacks.
> We have to take the ops approach to accomodate RTAS, where firmware
> handles almost all details of MSI setup/teardown. Bare-metal MSI
> can be accomodated also.
> 
> See the comments in include/asm-powerpc/msi.h for more info.
> 
> Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
> ---
> 
>  arch/powerpc/kernel/msi.c     |  320 ++++++++++++++++++++++++++++++++++++++++++
>  include/asm-powerpc/machdep.h |    6 
>  include/asm-powerpc/msi.h     |  173 ++++++++++++++++++++++
>  include/linux/pci.h           |    7 
>  4 files changed, 506 insertions(+)
> 
> Index: msi/arch/powerpc/kernel/msi.c

Why isn't all of this in the drivers/pci/msi.c file instead?  You are
creating a new API here that is only availble to the PPC platform, which
is not acceptable.

Please work with the current MSI core code to work properly for PPC,
instead of reinventing the whole thing.

> +#if defined(CONFIG_PCI_MSI) && defined(CONFIG_PPC_MERGE)
> +struct msi_info;
> +#endif

CONFIG_PPC_MERGE?  What is that option for?

So, no, I don't agree with this implementation and don't want to see it
go through anyone's tree into mainline just yet.

thanks,

greg k-h