Subject: Re: [RFC/PATCH 0/7] Powerpc MSI Implementation

From: Michael Ellerman <[email-masked]>
Date: Thu, 09 Nov 2006 10:33:56 +1100

On Wed, 2006-11-08 at 03:26 -0700, Eric W. Biederman wrote:
> Michael Ellerman <[email-masked]> writes:
> 
> >> Oh, and I think your first two patches can be applied now to the tree
> >> (header file changes).  Any objection to me doing this?
> >
> > There's a trivial bug in 1/7 (HT #defines), so hold back on that one
> > until I resend. The movement of MSI-X #defines is good to go. I also
> > sent you the patch to add HT_SUBCAP_OFFSET yesterday that I think is
> > ready to merge.
> 
> Actually now that I am thinking about it I'm not at all certain about
> the HT_SUBCAP_OFFSET patch.  The basic issue is that it suggests that
> the field that specifies which type of hypertransport capability has a
> fixed number of bits.  While in reality the encoding is variable
> length.

Yeah OK, I did notice that was a bit weird, I had a quick look and
assumed that because all the HT_CAPTYPE's were #defined as byte values
it was OK.

Looking closer most of them are 5 bits, the high 5 bits, and happen to
sit next to reserved fields (which must be zero), so reading the byte
should work in practice. But for a few of them you'll get cruft in the
low bits.

I don't know what they were thinking when they decided to have 3 and 5
bit capability fields, and then specify some of them as being a byte
wide as well. Perhaps the spec committee had a big night out ;)

I was going to write a generic version of pci_find_ht_capability() (as
suggested by Segher), so along with that I'll clean up the #defines to
just be the 3 or 5 bit capability codes, and then have a shift for
getting the capability out of the byte.

Users will still need to know if they're looking for a 3 or 5 bit
capability, but we can encapsulate that in pci_find_ht_capability() and
hopefully most people won't have to see the difference.

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