Date:	Thu, 29 Mar 2007 14:13:01 +1000
From:	Michael Ellerman <michael@...erman.id.au>
To:	"Eric W. Biederman" <ebiederm@...ssion.com>
Cc:	linux-pci@...ey.karlin.mff.cuni.cz,
	Greg Kroah-Hartman <greg@...ah.com>,
	"David S. Miller" <davem@...emloft.net>,
	Benjamin Herrenschmidt <benh@...nel.crashing.org>,
	linux-kernel@...r.kernel.org, Andrew Morton <akpm@...l.org>,
	daniel.e.wolstenholme@...el.com
Subject: Re: [PATCH 10/21] MSI: Add an arch_msi_supported()

On Tue, 2007-03-27 at 23:54 -0600, Eric W. Biederman wrote:
> Michael Ellerman <michael@...erman.id.au> writes:
> 
> > Add an arch_msi_supported(), which gives archs a chance to check the input
> > to pci_enable_msi/x. For MSI-X this routine might need the entry array, so
> > pass it in. For plain MSI, NULL is passed, the arch routine needs to cope
> > with that. Propagate the error value returned from the arch routine out to
> > the caller.
> 
> Ugh.  I'm not very comfortable with passing struct msix_entry into
> the architectures right now.
> 
> There are a couple of reasons.
> - It's irq field is to small (so we need to change it at some point)
> - No a single driver that calls pci_enable_msix uses the scatter gather
>   feature (so the entry member is redundant).
> 
> So this struct msix_entry needs to change and we need to change the drivers
> along with it.  Having to change a couple of architectures as well sounds
> painful.  So we might as well fix that at the same time as we are
> adding the RTAS support so architectures don't have to deal with this
> nasty unused concept.
> 
> I'm thinking the same thing to do is to completely remove struct msix_entry
> and just let drivers walk the linked list you introduce a few patches
> later down.  All they need is to get their irq numbers anyway.

I agree with most of that. I thought of doing that change, but didn't
want to have the powerpc code stuck behind a huge pile of driver
changes.

My only other worry is that at some point we'll get a driver that does
want to choose the entries it's allocated, and at that point we'll have
to put back the msix_entry code (or something similar). I don't have any
idea of when/if that sort of hardware/driver requirement is likely to
surface though, if it's "not for a while" it might be worth ripping out
the complexity until we really need it.

> I was tempted to drop nvec as well since our irq numbers are virtual,
> we could always delay the failure into request_irq.  But there are
> a few embedded architectures like the arm where the number irqs
> numbers may stay limited for a long time and if the driver will never
> use all of the irqs we get to save some resources and some work.  So
> that makes sense.

I think nvec should stay.

> So can we please at least move this patch down to the end with the
> rest of the RTAS arch support?
> 
> Moving it towards the end will allow it to be reviewed in the context
> where it will be used and it will give us a chance to simplify
> pci_enable_msix before we get there.

I'm happy to move it to the end of the series. I'm also happy to stop
passing the msix_entry into the arch.

But I don't want to predicate the merge of our powerpc stuff on the
removal of msix_entry entirely, there's too much risk that we'll slip to
v23.

cheers

-- 
Michael Ellerman
OzLabs, IBM Australia Development Lab

wwweb: http://michael.ellerman.id.au
phone: +61 2 6212 1183 (tie line 70 21183)

We do not inherit the earth from our ancestors,
we borrow it from our children. - S.M.A.R.T Person

Download attachment "signature.asc" of type "application/pgp-signature" (190 bytes)

Powered by blists - more mailing lists