On Wed, May 18, 2016 at 05:46:55PM +0100, Andre Przywara wrote:

Hi,

quoted

quoted

quoted

+
+/* generate a mask that covers 1024 interrupts with <b> bits per IRQ */

Hmmm. I'd appreciate some additional comments, specially when it comes
to the various restrictions. May I'd suggest something like:

/*
 * Generate a mask that covers the number of bytes required to address
 * up to 1024 interrupts, each represented by <b> bits. This assumes
 * that <b> is a power of two.
 *
 * ilog2(b) + ilog2(1024) is the number of bits required to bit-address
 * 1024 interrupts, each represented by b bits. Minus ilog2(8) converts
 * this to a byte address.

So I'm guessting this is a rewrite of ilog2( (b * 1024) / 8), but I'm
stupid enough to not understand our use of logarithms here.  Can someone
remind me whatever I forgot at CS 101?

I guess it was more me not seeing the wood for the trees here:
Indeed doing the multiplication first and then calling ilog2 seems to
make more sense. Also I was thinking: Isn't
"GENMASK_ULL(ilog2(n) - 1, 0)" the same as "n - 1"?

if there's no integer rounding taking place with ilog2 (iow. n is a
power of 2) then yes, I believe it is.

So can't we just write:

#define VGIC_ADDR_IRQ_MASK(bpi) (((bpi) * 1024 / 8) - 1)

that certianly all of the sudden feels intuitive.

Proven by enumeration - over the values we use ;-)

I'd keep the first paragraph of Marc's comment above then, but we can
avoid mentioning Advanced Maths textbooks about binary logarithmic ;-)

Haha, you saved my day with that comment.  I feel slightly less idiotic
now, yes, let's call it advanced quantum math or something instead of CS
101 :)

-Christoffer

quoted

quoted

 */

quoted

+#define VGIC_ADDR_IRQ_MASK(b) GENMASK_ULL(ilog2(b) + ilog2(1024) - \
+					  ilog2(BITS_PER_BYTE) - 1, 0)

/*
 * Convert a base address into a base interrupt (each interrupt
 * represented by <bits> bits. This assumes that <bits> is a power
 * of two, that <addr> both part of a memory region aligned on a

did you mean '<addr> *is* both part of' ?

quoted

 * <b> bits boundary, and itself aligned on that same boundary
 * (for regions that describe an interrupt with more than a single
 * byte of data).
 */

In any case, thanks for the commentary, I was faily lost here.