Re: [PATCH v2 1/1] x86: Add Isolated Memory Regions for Quark X1000

From: Bryan O'Donoghue
Date: Sat Jan 24 2015 - 16:57:02 EST


On 24/01/15 11:02, Andy Shevchenko wrote:
On Sat, Jan 24, 2015 at 3:48 AM, Ong, Boon Leong
<boon.leong.ong@xxxxxxxxx> wrote:

+static int imr_enabled(struct imr_regs *imr)
Do we want to make it inline perhaps since it is 1 liner?

Since it is declared static I would even suggest the new name is_imr_enabled().

I think imr_is_enabled() is a better name. Every other function is imr_ prefixed.

+
+ if ((imr_to_phys(imr.addr_lo) == base) &&
+ (imr_to_phys(imr.addr_hi) == max)) {
I think we need to take care of the size that has been fix-up here ...

+ found = 1;
+ reg = i;

According to your comment this line becomes redundant.


Ah but I still think though.

We eventually do an imr_write(reg, &imr); which I think reads better than imr_write(i, &imr);

--
BOD
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/