Hi, >>> + >>> +/* 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"? So can't we just write: #define VGIC_ADDR_IRQ_MASK(bpi) (((bpi) * 1024 / 8) - 1) 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 ;-) Cheers, Andre. >> */ >> >>> +#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' ? > >> * <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. > -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
