From: Milton Miller <miltonm@bga•com>
To: michael@ellerman•id.au
Cc: linuxppc-dev@ozlabs•org, linux-pci <linux-pci@atrey•karlin.mff.cuni.cz>
Subject: Re: [PATCH 6/7] MPIC MSI allocator
Date: Mon, 23 Apr 2007 01:20:11 -0500 [thread overview]
Message-ID: <63dd3d59fb8f043738b30026139feff6@bga.com> (raw)
In-Reply-To: <1177301062.3889.27.camel@concordia.ozlabs.ibm.com>
On Apr 22, 2007, at 11:04 PM, Michael Ellerman wrote:
> On Sat, 2007-04-21 at 18:17 -0500, Milton Miller wrote:
>> On Apr 19, 2007, Michael Ellerman wrote:
>>> To support MSI on MPIC we need a way to reserve and allocate hardware
>>> irq
>>> numbers, this patch implements an allocator for that.
>>> + /* Reserve source numbers we know are reserved in the HW */
>>> + for (i = 0; i < 8; i++) __mpic_msi_reserve_hwirq(mpic, i);
>>> + for (i = 42; i < 46; i++) __mpic_msi_reserve_hwirq(mpic, i);
>>> + for (i = 100; i < 105; i++) __mpic_msi_reserve_hwirq(mpic, i);
>>
>> More lines please.
>>
>>> +#else
>>> +static int mpic_msi_reserve_u3_hwirqs(struct mpic *mpic) { return
>>> -1;
>>> }
>>
>> and here.
>
> Blank lines aren't free you know!
I didn't ask for any blank ones, so you can keep the bill. :-)
>>
>>> + if (len % 8 != 0) {
Here you use 8 as the size of the element to process.
>>> + printk(KERN_WARNING "mpic: Malformed msi-available-ranges "
>>> + "property on %s\n", mpic->of_node->full_name);
>>> + return -EINVAL;
>>> + }
>>> +
>>> + bitmap_allocate_region(mpic->hwirq_bitmap, 0,
>>> + fls(mpic->irq_count) - 1);
>>> +
>>> + /* Format is: (<u32 start> <u32 count>)+ */
>>> + len /= sizeof(u32);
Here you hide a divide by 4 and follow with divide by 2.
>>> + for (i = 0; i < len / 2; i++, p += 2)
>>
>> how about just dividing by the calculated 8 above? or use
>> count = len / 8.
>
> I'm not sure I follow. If you can think of a clearer way I'm all ears,
> or are you just trying to save a divide :)
I'm saying that it doesn't make sense to do both of the above.
I'd move the comment on being pairs of cells to before the
if () EINVAL then follow immediately with the divide. If you
think that hides length being adjusted then I was optionally
suggesting making a new variable count to use in the for loop.
Actually, since i isn't used in the loop you could just
increment i by 8 while < len, and save both divides.
milton
next prev parent reply other threads:[~2007-04-23 6:20 UTC|newest]
Thread overview: 34+ messages / expand[flat|nested] mbox.gz Atom feed top
2007-04-19 7:35 [PATCH 1/7] Rip out the existing powerpc msi stubs Michael Ellerman
2007-04-19 7:35 ` [PATCH 3/7] RTAS MSI implementation Michael Ellerman
2007-04-19 7:35 ` [PATCH 2/7] Powerpc MSI infrastructure Michael Ellerman
2007-04-21 23:15 ` Milton Miller
2007-04-23 4:28 ` Michael Ellerman
2007-04-19 7:35 ` [PATCH 5/7] Enable MSI mappings for MPIC Michael Ellerman
2007-04-21 23:16 ` Milton Miller
2007-04-22 0:06 ` Benjamin Herrenschmidt
2007-04-22 4:53 ` Milton Miller
2007-04-23 4:10 ` Michael Ellerman
2007-04-19 7:35 ` [PATCH 4/7] Tell Phyp we support MSI Michael Ellerman
2007-04-19 7:35 ` [PATCH 6/7] MPIC MSI allocator Michael Ellerman
2007-04-21 23:17 ` Milton Miller
2007-04-23 4:04 ` Michael Ellerman
2007-04-23 6:20 ` Milton Miller [this message]
2007-04-23 3:50 ` Olof Johansson
2007-04-23 3:53 ` Michael Ellerman
2007-04-24 1:29 ` Benjamin Herrenschmidt
2007-04-24 9:26 ` Segher Boessenkool
2007-04-24 9:39 ` Benjamin Herrenschmidt
2007-04-24 9:44 ` Segher Boessenkool
2007-04-24 9:51 ` Benjamin Herrenschmidt
2007-04-19 7:35 ` [PATCH 7/7] MPIC MSI backend Michael Ellerman
2007-04-20 8:40 ` Segher Boessenkool
2007-04-20 14:39 ` Michael Ellerman
2007-04-21 23:17 ` Milton Miller
2007-04-22 7:06 ` Segher Boessenkool
2007-04-22 19:30 ` Olof Johansson
2007-04-23 4:24 ` Michael Ellerman
2007-04-23 8:24 ` Segher Boessenkool
2007-04-23 8:31 ` Michael Ellerman
2007-04-23 17:33 ` Segher Boessenkool
2007-04-23 3:45 ` Michael Ellerman
-- strict thread matches above, loose matches on Subject: below --
2007-05-08 2:58 [PATCH 1/7] Rip out the existing powerpc msi stubs Michael Ellerman
2007-05-08 2:58 ` [PATCH 6/7] MPIC MSI allocator Michael Ellerman
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=63dd3d59fb8f043738b30026139feff6@bga.com \
--to=miltonm@bga$(echo .)com \
--cc=linux-pci@atrey$(echo .)karlin.mff.cuni.cz \
--cc=linuxppc-dev@ozlabs$(echo .)org \
--cc=michael@ellerman$(echo .)id.au \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox