Subject: Re: [PATCH 7/16] powerpc: add support for ps3 platform

From: Geoff Levand <[email-masked]>
Date: Thu, 16 Nov 2006 18:44:11 -0800

Michael Ellerman wrote:
> On Thu, 2006-11-16 at 18:04 -0800, Geoff Levand wrote:
>> Michael Ellerman wrote:
>> > On Thu, 2006-11-16 at 17:33 -0800, Geoff Levand wrote:
>> >> Benjamin Herrenschmidt wrote:
>> >> >> > +
>> >> >> > +enum page_size {
>> >> >> > +	page_size_4k = 12U,
>> >> >> > +	page_size_64k = 16U,
>> >> >> > +	page_size_16m = 24U,
>> >> >> > +};
>> >> >> 
>> >> >> Please use ALL_CAPS for such constants.
>> >> > 
>> >> > And call them PAGE_SHIFT_* while at it :-)
>> >> 
>> >> The HV docs call this page size, so that is what I used for
>> >> the symbol.  I'll look at the usage again and reconsider.
>> > 
>> > No need to look at it, it's a shift not a size, the page size is not 12
>> > bytes. Keeping it consistent with the HV docs is a concern, but so is
>> > making it consistent with every other part of the kernel - which calls
>> > this a shift, see page.h f.e.
>> 
>> Well, I meant reconsider in more of a broad sense, in like, maybe I can just do
>> something to get rid of it...
> 
> Yeah ok, that's always a nice solution. My last post probably could have
> used a smiley in hindsight :)

Anyway, I hope everyone's happy...

+enum {
+	PAGE_SHIFT_4K = 12U,
+	PAGE_SHIFT_64K = 16U,
+	PAGE_SHIFT_16M = 24U,
+};