| Message ID | 1381935366-11731-4-git-send-email-gong.chen@linux.intel.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
On Wed, Oct 16, 2013 at 10:56:00AM -0400, Chen, Gong wrote: > GENMASK is used to create a contiguous bitmask([hi:lo]). It is > implemented twice in current kernel. One is in EDAC driver, the other > is in SiS/XGI FB driver. Move it to a more generic place for other > usage. > > Signed-off-by: Chen, Gong <gong.chen@linux.intel.com> > Cc: Borislav Petkov <bp@alien8.de> > Cc: Thomas Winischhofer <thomas@winischhofer.net> > Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com> > Cc: Tomi Valkeinen <tomi.valkeinen@ti.com> Acked-by: Borislav Petkov <bp@suse.de>
Em Wed, 16 Oct 2013 10:56:00 -0400 "Chen, Gong" <gong.chen@linux.intel.com> escreveu: > GENMASK is used to create a contiguous bitmask([hi:lo]). It is > implemented twice in current kernel. One is in EDAC driver, the other > is in SiS/XGI FB driver. Move it to a more generic place for other > usage. > > Signed-off-by: Chen, Gong <gong.chen@linux.intel.com> > Cc: Borislav Petkov <bp@alien8.de> > Cc: Thomas Winischhofer <thomas@winischhofer.net> > Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com> > Cc: Tomi Valkeinen <tomi.valkeinen@ti.com> Acked-by: Mauro Carvalho Chehab <m.chehab@samsung.com> Btw, there's another incarnation of it at sb_edac.c (GET_BITFIELD). It could make sense to also replace it by the newly bitops.h macro. Regards, Mauro > --- > drivers/edac/amd64_edac.c | 46 ++++++++++++++++++++++++---------------------- > drivers/edac/amd64_edac.h | 8 -------- > drivers/video/sis/init.c | 5 ++--- > include/linux/bitops.h | 8 ++++++++ > 4 files changed, 34 insertions(+), 33 deletions(-) > > diff --git a/drivers/edac/amd64_edac.c b/drivers/edac/amd64_edac.c > index 3c9e4e9..b53d0de 100644 > --- a/drivers/edac/amd64_edac.c > +++ b/drivers/edac/amd64_edac.c > @@ -339,8 +339,8 @@ static void get_cs_base_and_mask(struct amd64_pvt *pvt, int csrow, u8 dct, > if (pvt->fam == 0xf && pvt->ext_model < K8_REV_F) { > csbase = pvt->csels[dct].csbases[csrow]; > csmask = pvt->csels[dct].csmasks[csrow]; > - base_bits = GENMASK(21, 31) | GENMASK(9, 15); > - mask_bits = GENMASK(21, 29) | GENMASK(9, 15); > + base_bits = GENMASK_ULL(31, 21) | GENMASK_ULL(15, 9); > + mask_bits = GENMASK_ULL(29, 21) | GENMASK_ULL(15, 9); > addr_shift = 4; > > /* > @@ -352,16 +352,16 @@ static void get_cs_base_and_mask(struct amd64_pvt *pvt, int csrow, u8 dct, > csbase = pvt->csels[dct].csbases[csrow]; > csmask = pvt->csels[dct].csmasks[csrow >> 1]; > > - *base = (csbase & GENMASK(5, 15)) << 6; > - *base |= (csbase & GENMASK(19, 30)) << 8; > + *base = (csbase & GENMASK_ULL(15, 5)) << 6; > + *base |= (csbase & GENMASK_ULL(30, 19)) << 8; > > *mask = ~0ULL; > /* poke holes for the csmask */ > - *mask &= ~((GENMASK(5, 15) << 6) | > - (GENMASK(19, 30) << 8)); > + *mask &= ~((GENMASK_ULL(15, 5) << 6) | > + (GENMASK_ULL(30, 19) << 8)); > > - *mask |= (csmask & GENMASK(5, 15)) << 6; > - *mask |= (csmask & GENMASK(19, 30)) << 8; > + *mask |= (csmask & GENMASK_ULL(15, 5)) << 6; > + *mask |= (csmask & GENMASK_ULL(30, 19)) << 8; > > return; > } else { > @@ -370,9 +370,11 @@ static void get_cs_base_and_mask(struct amd64_pvt *pvt, int csrow, u8 dct, > addr_shift = 8; > > if (pvt->fam == 0x15) > - base_bits = mask_bits = GENMASK(19,30) | GENMASK(5,13); > + base_bits = mask_bits = > + GENMASK_ULL(30,19) | GENMASK_ULL(13,5); > else > - base_bits = mask_bits = GENMASK(19,28) | GENMASK(5,13); > + base_bits = mask_bits = > + GENMASK_ULL(28,19) | GENMASK_ULL(13,5); > } > > *base = (csbase & base_bits) << addr_shift; > @@ -561,7 +563,7 @@ static u64 sys_addr_to_dram_addr(struct mem_ctl_info *mci, u64 sys_addr) > * section 3.4.2 of AMD publication 24592: AMD x86-64 Architecture > * Programmer's Manual Volume 1 Application Programming. > */ > - dram_addr = (sys_addr & GENMASK(0, 39)) - dram_base; > + dram_addr = (sys_addr & GENMASK_ULL(39, 0)) - dram_base; > > edac_dbg(2, "using DRAM Base register to translate SysAddr 0x%lx to DramAddr 0x%lx\n", > (unsigned long)sys_addr, (unsigned long)dram_addr); > @@ -597,7 +599,7 @@ static u64 dram_addr_to_input_addr(struct mem_ctl_info *mci, u64 dram_addr) > * concerning translating a DramAddr to an InputAddr. > */ > intlv_shift = num_node_interleave_bits(dram_intlv_en(pvt, 0)); > - input_addr = ((dram_addr >> intlv_shift) & GENMASK(12, 35)) + > + input_addr = ((dram_addr >> intlv_shift) & GENMASK_ULL(35, 12)) + > (dram_addr & 0xfff); > > edac_dbg(2, " Intlv Shift=%d DramAddr=0x%lx maps to InputAddr=0x%lx\n", > @@ -849,7 +851,7 @@ static u64 get_error_address(struct amd64_pvt *pvt, struct mce *m) > end_bit = 39; > } > > - addr = m->addr & GENMASK(start_bit, end_bit); > + addr = m->addr & GENMASK_ULL(end_bit, start_bit); > > /* > * Erratum 637 workaround > @@ -861,7 +863,7 @@ static u64 get_error_address(struct amd64_pvt *pvt, struct mce *m) > u16 mce_nid; > u8 intlv_en; > > - if ((addr & GENMASK(24, 47)) >> 24 != 0x00fdf7) > + if ((addr & GENMASK_ULL(47, 24)) >> 24 != 0x00fdf7) > return addr; > > mce_nid = amd_get_nb_id(m->extcpu); > @@ -871,7 +873,7 @@ static u64 get_error_address(struct amd64_pvt *pvt, struct mce *m) > intlv_en = tmp >> 21 & 0x7; > > /* add [47:27] + 3 trailing bits */ > - cc6_base = (tmp & GENMASK(0, 20)) << 3; > + cc6_base = (tmp & GENMASK_ULL(20, 0)) << 3; > > /* reverse and add DramIntlvEn */ > cc6_base |= intlv_en ^ 0x7; > @@ -880,18 +882,18 @@ static u64 get_error_address(struct amd64_pvt *pvt, struct mce *m) > cc6_base <<= 24; > > if (!intlv_en) > - return cc6_base | (addr & GENMASK(0, 23)); > + return cc6_base | (addr & GENMASK_ULL(23, 0)); > > amd64_read_pci_cfg(pvt->F1, DRAM_LOCAL_NODE_BASE, &tmp); > > /* faster log2 */ > - tmp_addr = (addr & GENMASK(12, 23)) << __fls(intlv_en + 1); > + tmp_addr = (addr & GENMASK_ULL(23, 12)) << __fls(intlv_en + 1); > > /* OR DramIntlvSel into bits [14:12] */ > - tmp_addr |= (tmp & GENMASK(21, 23)) >> 9; > + tmp_addr |= (tmp & GENMASK_ULL(23, 21)) >> 9; > > /* add remaining [11:0] bits from original MC4_ADDR */ > - tmp_addr |= addr & GENMASK(0, 11); > + tmp_addr |= addr & GENMASK_ULL(11, 0); > > return cc6_base | tmp_addr; > } > @@ -952,12 +954,12 @@ static void read_dram_base_limit_regs(struct amd64_pvt *pvt, unsigned range) > > amd64_read_pci_cfg(f1, DRAM_LOCAL_NODE_LIM, &llim); > > - pvt->ranges[range].lim.lo &= GENMASK(0, 15); > + pvt->ranges[range].lim.lo &= GENMASK_ULL(15, 0); > > /* {[39:27],111b} */ > pvt->ranges[range].lim.lo |= ((llim & 0x1fff) << 3 | 0x7) << 16; > > - pvt->ranges[range].lim.hi &= GENMASK(0, 7); > + pvt->ranges[range].lim.hi &= GENMASK_ULL(7, 0); > > /* [47:40] */ > pvt->ranges[range].lim.hi |= llim >> 13; > @@ -1330,7 +1332,7 @@ static u64 f1x_get_norm_dct_addr(struct amd64_pvt *pvt, u8 range, > chan_off = dram_base; > } > > - return (sys_addr & GENMASK(6,47)) - (chan_off & GENMASK(23,47)); > + return (sys_addr & GENMASK_ULL(47,6)) - (chan_off & GENMASK_ULL(47,23)); > } > > /* > diff --git a/drivers/edac/amd64_edac.h b/drivers/edac/amd64_edac.h > index d2443cf..6dc1fcc 100644 > --- a/drivers/edac/amd64_edac.h > +++ b/drivers/edac/amd64_edac.h > @@ -160,14 +160,6 @@ > #define OFF false > > /* > - * Create a contiguous bitmask starting at bit position @lo and ending at > - * position @hi. For example > - * > - * GENMASK(21, 39) gives us the 64bit vector 0x000000ffffe00000. > - */ > -#define GENMASK(lo, hi) (((1ULL << ((hi) - (lo) + 1)) - 1) << (lo)) > - > -/* > * PCI-defined configuration space registers > */ > #define PCI_DEVICE_ID_AMD_15H_M30H_NB_F1 0x141b > diff --git a/drivers/video/sis/init.c b/drivers/video/sis/init.c > index f082ae5..4f26bc2 100644 > --- a/drivers/video/sis/init.c > +++ b/drivers/video/sis/init.c > @@ -3320,9 +3320,8 @@ SiSSetMode(struct SiS_Private *SiS_Pr, unsigned short ModeNo) > } > > #ifndef GETBITSTR > -#define BITMASK(h,l) (((unsigned)(1U << ((h)-(l)+1))-1)<<(l)) > -#define GENMASK(mask) BITMASK(1?mask,0?mask) > -#define GETBITS(var,mask) (((var) & GENMASK(mask)) >> (0?mask)) > +#define GENBITSMASK(mask) GENMASK(1?mask,0?mask) > +#define GETBITS(var,mask) (((var) & GENBITSMASK(mask)) >> (0?mask)) > #define GETBITSTR(val,from,to) ((GETBITS(val,from)) << (0?to)) > #endif > > diff --git a/include/linux/bitops.h b/include/linux/bitops.h > index a3b6b82..bd0c459 100644 > --- a/include/linux/bitops.h > +++ b/include/linux/bitops.h > @@ -10,6 +10,14 @@ > #define BITS_TO_LONGS(nr) DIV_ROUND_UP(nr, BITS_PER_BYTE * sizeof(long)) > #endif > > +/* > + * Create a contiguous bitmask starting at bit position @l and ending at > + * position @h. For example > + * GENMASK_ULL(39, 21) gives us the 64bit vector 0x000000ffffe00000. > + */ > +#define GENMASK(h, l) (((U32_C(1) << ((h) - (l) + 1)) - 1) << (l)) > +#define GENMASK_ULL(h, l) (((U64_C(1) << ((h) - (l) + 1)) - 1) << (l)) > + > extern unsigned int __sw_hweight8(unsigned int w); > extern unsigned int __sw_hweight16(unsigned int w); > extern unsigned int __sw_hweight32(unsigned int w);
On Wed, Oct 16, 2013 at 02:02:21PM -0300, Mauro Carvalho Chehab wrote: > Date: Wed, 16 Oct 2013 14:02:21 -0300 > From: Mauro Carvalho Chehab <m.chehab@samsung.com> > To: "Chen, Gong" <gong.chen@linux.intel.com> > Cc: tony.luck@intel.com, bp@alien8.de, joe@perches.com, > naveen.n.rao@linux.vnet.ibm.com, arozansk@redhat.com, > linux-acpi@vger.kernel.org, linux-kernel@vger.kernel.org, Thomas > Winischhofer <thomas@winischhofer.net>, Jean-Christophe Plagniol-Villard > <plagnioj@jcrosoft.com>, Tomi Valkeinen <tomi.valkeinen@ti.com> > Subject: Re: [PATCH v2 3/9] bitops: Introduce a more generic BITMASK macro > X-Mailer: Claws Mail 3.9.2 (GTK+ 2.24.19; x86_64-redhat-linux-gnu) > > Em Wed, 16 Oct 2013 10:56:00 -0400 > "Chen, Gong" <gong.chen@linux.intel.com> escreveu: > > > GENMASK is used to create a contiguous bitmask([hi:lo]). It is > > implemented twice in current kernel. One is in EDAC driver, the other > > is in SiS/XGI FB driver. Move it to a more generic place for other > > usage. > > > > Signed-off-by: Chen, Gong <gong.chen@linux.intel.com> > > Cc: Borislav Petkov <bp@alien8.de> > > Cc: Thomas Winischhofer <thomas@winischhofer.net> > > Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com> > > Cc: Tomi Valkeinen <tomi.valkeinen@ti.com> > > Acked-by: Mauro Carvalho Chehab <m.chehab@samsung.com> > > Btw, there's another incarnation of it at sb_edac.c (GET_BITFIELD). > It could make sense to also replace it by the newly bitops.h macro. > > Regards, > Mauro Aha, I will update it in the next version.
On Wed, 2013-10-16 at 10:56 -0400, Chen, Gong wrote: > GENMASK is used to create a contiguous bitmask([hi:lo]). It is > implemented twice in current kernel. One is in EDAC driver, the other > is in SiS/XGI FB driver. Move it to a more generic place for other > usage. [] > diff --git a/include/linux/bitops.h b/include/linux/bitops.h [] > @@ -10,6 +10,14 @@ > #define BITS_TO_LONGS(nr) DIV_ROUND_UP(nr, BITS_PER_BYTE * sizeof(long)) > #endif > > +/* > + * Create a contiguous bitmask starting at bit position @l and ending at > + * position @h. For example > + * GENMASK_ULL(39, 21) gives us the 64bit vector 0x000000ffffe00000. ull > + */ > +#define GENMASK(h, l) (((U32_C(1) << ((h) - (l) + 1)) - 1) << (l)) > +#define GENMASK_ULL(h, l) (((U64_C(1) << ((h) - (l) + 1)) - 1) << (l)) Maybe add a BUILD_BUG_ON(__builtin_constant_p(l) && __builtin_constant_p(h) && \ (h) < (l)) -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Oct 16, 2013 at 07:59:09PM -0700, Joe Perches wrote: > Date: Wed, 16 Oct 2013 19:59:09 -0700 > From: Joe Perches <joe@perches.com> > To: "Chen, Gong" <gong.chen@linux.intel.com> > Cc: tony.luck@intel.com, bp@alien8.de, naveen.n.rao@linux.vnet.ibm.com, > m.chehab@samsung.com, arozansk@redhat.com, linux-acpi@vger.kernel.org, > linux-kernel@vger.kernel.org, Thomas Winischhofer > <thomas@winischhofer.net>, Jean-Christophe Plagniol-Villard > <plagnioj@jcrosoft.com>, Tomi Valkeinen <tomi.valkeinen@ti.com> > Subject: Re: [PATCH v2 3/9] bitops: Introduce a more generic BITMASK macro > X-Mailer: Evolution 3.6.4-0ubuntu1 > > On Wed, 2013-10-16 at 10:56 -0400, Chen, Gong wrote: > > GENMASK is used to create a contiguous bitmask([hi:lo]). It is > > implemented twice in current kernel. One is in EDAC driver, the other > > is in SiS/XGI FB driver. Move it to a more generic place for other > > usage. > [] > > diff --git a/include/linux/bitops.h b/include/linux/bitops.h > [] > > @@ -10,6 +10,14 @@ > > #define BITS_TO_LONGS(nr) DIV_ROUND_UP(nr, BITS_PER_BYTE * sizeof(long)) > > #endif > > > > +/* > > + * Create a contiguous bitmask starting at bit position @l and ending at > > + * position @h. For example > > + * GENMASK_ULL(39, 21) gives us the 64bit vector 0x000000ffffe00000. > > ull > > > + */ > > +#define GENMASK(h, l) (((U32_C(1) << ((h) - (l) + 1)) - 1) << (l)) > > +#define GENMASK_ULL(h, l) (((U64_C(1) << ((h) - (l) + 1)) - 1) << (l)) > > > Maybe add a > > BUILD_BUG_ON(__builtin_constant_p(l) && __builtin_constant_p(h) && \ > (h) < (l)) > No, if so, users can't use variables for this macro.
On Thu, 2013-10-17 at 02:30 -0400, Chen Gong wrote: > On Wed, Oct 16, 2013 at 07:59:09PM -0700, Joe Perches wrote: [] > > Maybe add a > > > > BUILD_BUG_ON(__builtin_constant_p(l) && __builtin_constant_p(h) && \ > > (h) < (l)) > > > No, if so, users can't use variables for this macro. __builtin_constant_p checks for constants Built-in Function: int __builtin_constant_p (exp) You can use the built-in function __builtin_constant_p to determine if a value is known to be constant at compile-time and hence that GCC can perform constant-folding on expressions involving that value. The argument of the function is the value to test. The function returns the integer 1 if the argument is known to be a compile-time constant and 0 if it is not known to be a compile-time constant. A return of 0 does not indicate that the value is not a constant, but merely that GCC cannot prove it is a constant with the specified value of the -O option. -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Oct 16, 2013 at 11:58:56PM -0700, Joe Perches wrote: > Date: Wed, 16 Oct 2013 23:58:56 -0700 > From: Joe Perches <joe@perches.com> > To: Chen Gong <gong.chen@linux.intel.com> > Cc: tony.luck@intel.com, bp@alien8.de, naveen.n.rao@linux.vnet.ibm.com, > m.chehab@samsung.com, arozansk@redhat.com, linux-acpi@vger.kernel.org, > linux-kernel@vger.kernel.org, Thomas Winischhofer > <thomas@winischhofer.net>, Jean-Christophe Plagniol-Villard > <plagnioj@jcrosoft.com>, Tomi Valkeinen <tomi.valkeinen@ti.com> > Subject: Re: [PATCH v2 3/9] bitops: Introduce a more generic BITMASK macro > X-Mailer: Evolution 3.6.4-0ubuntu1 > > On Thu, 2013-10-17 at 02:30 -0400, Chen Gong wrote: > > On Wed, Oct 16, 2013 at 07:59:09PM -0700, Joe Perches wrote: > [] > > > Maybe add a > > > > > > BUILD_BUG_ON(__builtin_constant_p(l) && __builtin_constant_p(h) && \ > > > (h) < (l)) > > > > > No, if so, users can't use variables for this macro. > > __builtin_constant_p checks for constants > > Built-in Function: int __builtin_constant_p (exp) > You can use the built-in function __builtin_constant_p to > determine if a value is known to be constant at compile-time and > hence that GCC can perform constant-folding on expressions > involving that value. The argument of the function is the value > to test. The function returns the integer 1 if the argument is > known to be a compile-time constant and 0 if it is not known to > be a compile-time constant. A return of 0 does not indicate that > the value is not a constant, but merely that GCC cannot prove it > is a constant with the specified value of the -O option. > Yes, even we have following codes __builtin_constant_p still can return 1, so long as the value of variable can be identified. int len = sizeof(int); if (__builtin_constant_p(len)) { do_1; } else { do_0; } but the point is we can use GENMASK like GENMASK(end_bit, start_bit) but we don't know the value of end_bit/start_bit at compile-time.
On Thu, 2013-10-17 at 03:38 -0400, Chen Gong wrote: > the point is we can use GENMASK like GENMASK(end_bit, start_bit) but > we don't know the value of end_bit/start_bit at compile-time. True. The BUILD_BUG_ON idea is just to avoid people using GENMASK(1, 2) instead of GENMASK(2, 1) #define GENMASK(h, l) \ ({ \ BUILD_BUG_ON(__builtin_constant_p(l) && \ __builtin_constant_p(h) && \ (l) > (h)); \ (((U32_C(1) << ((h) - (l) + 1)) - 1) << (l)); \ }) -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Oct 17, 2013 at 01:32:30AM -0700, Joe Perches wrote: > On Thu, 2013-10-17 at 03:38 -0400, Chen Gong wrote: > > the point is we can use GENMASK like GENMASK(end_bit, start_bit) but > > we don't know the value of end_bit/start_bit at compile-time. > > True. > > The BUILD_BUG_ON idea is just to avoid people using > GENMASK(1, 2) They'll notice the 0 pretty quickly when they test their code. Let's add those checks only when it is really necessary and people have actually made that mistake repeatedly.
On Thu, 2013-10-17 at 10:40 +0200, Borislav Petkov wrote: > On Thu, Oct 17, 2013 at 01:32:30AM -0700, Joe Perches wrote: > > On Thu, 2013-10-17 at 03:38 -0400, Chen Gong wrote: > > > the point is we can use GENMASK like GENMASK(end_bit, start_bit) but > > > we don't know the value of end_bit/start_bit at compile-time. > > > > True. > > > > The BUILD_BUG_ON idea is just to avoid people using > > GENMASK(1, 2) > > They'll notice the 0 pretty quickly when they test their code. > > Let's add those checks only when it is really necessary and people have > actually made that mistake repeatedly. It's cost free to add the BUILD_BUG_ON and perhaps you underestimate the runtime bug checking effort, -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Oct 17, 2013 at 1:55 AM, Joe Perches <joe@perches.com> wrote: > It's cost free to add the BUILD_BUG_ON > and perhaps you underestimate the runtime > bug checking effort, This looks OK to me. Gong: This doesn't stop people from using variables as arguments ... they just won't get a check for (h) < (l). -Tony -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, 2013-10-17 at 09:10 -0700, Tony Luck wrote: > On Thu, Oct 17, 2013 at 1:55 AM, Joe Perches <joe@perches.com> wrote: > > It's cost free to add the BUILD_BUG_ON > > and perhaps you underestimate the runtime > > bug checking effort, > > This looks OK to me. Gong: This doesn't stop people from using variables > as arguments ... they just won't get a check for (h) < (l). Another possibility is to swap high and low if necessary and maybe warn when either is negative or too large for the bit width. #define GENMASK(h, l) \ ({ \ size_t high = h; \ size_t low = l; \ BUILD_BUG_ON(__builtin_constant_p(l) && \ __builtin_constant_p(h) && \ (l) > (h)); \ BUILD_BUG_ON(__builtin_constant_p(l) && \ __builtin_constant_p(h) && \ ((l) >= BITS_PER_LONG || \ (h) >= BITS_PER_LONG)); \ WARN_ONCE((!__builtin_constant_p(l) && \ ((l) < 0 || (l) > BITS_PER_LONG)) || \ (!__builtin_constant_p(h) && \ ((h) < 0 || (h) > BITS_PER_LONG)) || \ (l) > (h), \ "GENMASK: invalid mask values: l: %u, h: %d\n", \ (l), (h)); \ if (low > high) \ swap(low, high); \ (((U32_C(1) << (high - low + 1)) - 1) << low); \ }) And maybe this should be renamed something like #define BIT_MASK_RANGE(h, l) or #define BIT_MASK_SHIFTED(h, l) -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/edac/amd64_edac.c b/drivers/edac/amd64_edac.c index 3c9e4e9..b53d0de 100644 --- a/drivers/edac/amd64_edac.c +++ b/drivers/edac/amd64_edac.c @@ -339,8 +339,8 @@ static void get_cs_base_and_mask(struct amd64_pvt *pvt, int csrow, u8 dct, if (pvt->fam == 0xf && pvt->ext_model < K8_REV_F) { csbase = pvt->csels[dct].csbases[csrow]; csmask = pvt->csels[dct].csmasks[csrow]; - base_bits = GENMASK(21, 31) | GENMASK(9, 15); - mask_bits = GENMASK(21, 29) | GENMASK(9, 15); + base_bits = GENMASK_ULL(31, 21) | GENMASK_ULL(15, 9); + mask_bits = GENMASK_ULL(29, 21) | GENMASK_ULL(15, 9); addr_shift = 4; /* @@ -352,16 +352,16 @@ static void get_cs_base_and_mask(struct amd64_pvt *pvt, int csrow, u8 dct, csbase = pvt->csels[dct].csbases[csrow]; csmask = pvt->csels[dct].csmasks[csrow >> 1]; - *base = (csbase & GENMASK(5, 15)) << 6; - *base |= (csbase & GENMASK(19, 30)) << 8; + *base = (csbase & GENMASK_ULL(15, 5)) << 6; + *base |= (csbase & GENMASK_ULL(30, 19)) << 8; *mask = ~0ULL; /* poke holes for the csmask */ - *mask &= ~((GENMASK(5, 15) << 6) | - (GENMASK(19, 30) << 8)); + *mask &= ~((GENMASK_ULL(15, 5) << 6) | + (GENMASK_ULL(30, 19) << 8)); - *mask |= (csmask & GENMASK(5, 15)) << 6; - *mask |= (csmask & GENMASK(19, 30)) << 8; + *mask |= (csmask & GENMASK_ULL(15, 5)) << 6; + *mask |= (csmask & GENMASK_ULL(30, 19)) << 8; return; } else { @@ -370,9 +370,11 @@ static void get_cs_base_and_mask(struct amd64_pvt *pvt, int csrow, u8 dct, addr_shift = 8; if (pvt->fam == 0x15) - base_bits = mask_bits = GENMASK(19,30) | GENMASK(5,13); + base_bits = mask_bits = + GENMASK_ULL(30,19) | GENMASK_ULL(13,5); else - base_bits = mask_bits = GENMASK(19,28) | GENMASK(5,13); + base_bits = mask_bits = + GENMASK_ULL(28,19) | GENMASK_ULL(13,5); } *base = (csbase & base_bits) << addr_shift; @@ -561,7 +563,7 @@ static u64 sys_addr_to_dram_addr(struct mem_ctl_info *mci, u64 sys_addr) * section 3.4.2 of AMD publication 24592: AMD x86-64 Architecture * Programmer's Manual Volume 1 Application Programming. */ - dram_addr = (sys_addr & GENMASK(0, 39)) - dram_base; + dram_addr = (sys_addr & GENMASK_ULL(39, 0)) - dram_base; edac_dbg(2, "using DRAM Base register to translate SysAddr 0x%lx to DramAddr 0x%lx\n", (unsigned long)sys_addr, (unsigned long)dram_addr); @@ -597,7 +599,7 @@ static u64 dram_addr_to_input_addr(struct mem_ctl_info *mci, u64 dram_addr) * concerning translating a DramAddr to an InputAddr. */ intlv_shift = num_node_interleave_bits(dram_intlv_en(pvt, 0)); - input_addr = ((dram_addr >> intlv_shift) & GENMASK(12, 35)) + + input_addr = ((dram_addr >> intlv_shift) & GENMASK_ULL(35, 12)) + (dram_addr & 0xfff); edac_dbg(2, " Intlv Shift=%d DramAddr=0x%lx maps to InputAddr=0x%lx\n", @@ -849,7 +851,7 @@ static u64 get_error_address(struct amd64_pvt *pvt, struct mce *m) end_bit = 39; } - addr = m->addr & GENMASK(start_bit, end_bit); + addr = m->addr & GENMASK_ULL(end_bit, start_bit); /* * Erratum 637 workaround @@ -861,7 +863,7 @@ static u64 get_error_address(struct amd64_pvt *pvt, struct mce *m) u16 mce_nid; u8 intlv_en; - if ((addr & GENMASK(24, 47)) >> 24 != 0x00fdf7) + if ((addr & GENMASK_ULL(47, 24)) >> 24 != 0x00fdf7) return addr; mce_nid = amd_get_nb_id(m->extcpu); @@ -871,7 +873,7 @@ static u64 get_error_address(struct amd64_pvt *pvt, struct mce *m) intlv_en = tmp >> 21 & 0x7; /* add [47:27] + 3 trailing bits */ - cc6_base = (tmp & GENMASK(0, 20)) << 3; + cc6_base = (tmp & GENMASK_ULL(20, 0)) << 3; /* reverse and add DramIntlvEn */ cc6_base |= intlv_en ^ 0x7; @@ -880,18 +882,18 @@ static u64 get_error_address(struct amd64_pvt *pvt, struct mce *m) cc6_base <<= 24; if (!intlv_en) - return cc6_base | (addr & GENMASK(0, 23)); + return cc6_base | (addr & GENMASK_ULL(23, 0)); amd64_read_pci_cfg(pvt->F1, DRAM_LOCAL_NODE_BASE, &tmp); /* faster log2 */ - tmp_addr = (addr & GENMASK(12, 23)) << __fls(intlv_en + 1); + tmp_addr = (addr & GENMASK_ULL(23, 12)) << __fls(intlv_en + 1); /* OR DramIntlvSel into bits [14:12] */ - tmp_addr |= (tmp & GENMASK(21, 23)) >> 9; + tmp_addr |= (tmp & GENMASK_ULL(23, 21)) >> 9; /* add remaining [11:0] bits from original MC4_ADDR */ - tmp_addr |= addr & GENMASK(0, 11); + tmp_addr |= addr & GENMASK_ULL(11, 0); return cc6_base | tmp_addr; } @@ -952,12 +954,12 @@ static void read_dram_base_limit_regs(struct amd64_pvt *pvt, unsigned range) amd64_read_pci_cfg(f1, DRAM_LOCAL_NODE_LIM, &llim); - pvt->ranges[range].lim.lo &= GENMASK(0, 15); + pvt->ranges[range].lim.lo &= GENMASK_ULL(15, 0); /* {[39:27],111b} */ pvt->ranges[range].lim.lo |= ((llim & 0x1fff) << 3 | 0x7) << 16; - pvt->ranges[range].lim.hi &= GENMASK(0, 7); + pvt->ranges[range].lim.hi &= GENMASK_ULL(7, 0); /* [47:40] */ pvt->ranges[range].lim.hi |= llim >> 13; @@ -1330,7 +1332,7 @@ static u64 f1x_get_norm_dct_addr(struct amd64_pvt *pvt, u8 range, chan_off = dram_base; } - return (sys_addr & GENMASK(6,47)) - (chan_off & GENMASK(23,47)); + return (sys_addr & GENMASK_ULL(47,6)) - (chan_off & GENMASK_ULL(47,23)); } /* diff --git a/drivers/edac/amd64_edac.h b/drivers/edac/amd64_edac.h index d2443cf..6dc1fcc 100644 --- a/drivers/edac/amd64_edac.h +++ b/drivers/edac/amd64_edac.h @@ -160,14 +160,6 @@ #define OFF false /* - * Create a contiguous bitmask starting at bit position @lo and ending at - * position @hi. For example - * - * GENMASK(21, 39) gives us the 64bit vector 0x000000ffffe00000. - */ -#define GENMASK(lo, hi) (((1ULL << ((hi) - (lo) + 1)) - 1) << (lo)) - -/* * PCI-defined configuration space registers */ #define PCI_DEVICE_ID_AMD_15H_M30H_NB_F1 0x141b diff --git a/drivers/video/sis/init.c b/drivers/video/sis/init.c index f082ae5..4f26bc2 100644 --- a/drivers/video/sis/init.c +++ b/drivers/video/sis/init.c @@ -3320,9 +3320,8 @@ SiSSetMode(struct SiS_Private *SiS_Pr, unsigned short ModeNo) } #ifndef GETBITSTR -#define BITMASK(h,l) (((unsigned)(1U << ((h)-(l)+1))-1)<<(l)) -#define GENMASK(mask) BITMASK(1?mask,0?mask) -#define GETBITS(var,mask) (((var) & GENMASK(mask)) >> (0?mask)) +#define GENBITSMASK(mask) GENMASK(1?mask,0?mask) +#define GETBITS(var,mask) (((var) & GENBITSMASK(mask)) >> (0?mask)) #define GETBITSTR(val,from,to) ((GETBITS(val,from)) << (0?to)) #endif diff --git a/include/linux/bitops.h b/include/linux/bitops.h index a3b6b82..bd0c459 100644 --- a/include/linux/bitops.h +++ b/include/linux/bitops.h @@ -10,6 +10,14 @@ #define BITS_TO_LONGS(nr) DIV_ROUND_UP(nr, BITS_PER_BYTE * sizeof(long)) #endif +/* + * Create a contiguous bitmask starting at bit position @l and ending at + * position @h. For example + * GENMASK_ULL(39, 21) gives us the 64bit vector 0x000000ffffe00000. + */ +#define GENMASK(h, l) (((U32_C(1) << ((h) - (l) + 1)) - 1) << (l)) +#define GENMASK_ULL(h, l) (((U64_C(1) << ((h) - (l) + 1)) - 1) << (l)) + extern unsigned int __sw_hweight8(unsigned int w); extern unsigned int __sw_hweight16(unsigned int w); extern unsigned int __sw_hweight32(unsigned int w);
GENMASK is used to create a contiguous bitmask([hi:lo]). It is implemented twice in current kernel. One is in EDAC driver, the other is in SiS/XGI FB driver. Move it to a more generic place for other usage. Signed-off-by: Chen, Gong <gong.chen@linux.intel.com> Cc: Borislav Petkov <bp@alien8.de> Cc: Thomas Winischhofer <thomas@winischhofer.net> Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com> --- drivers/edac/amd64_edac.c | 46 ++++++++++++++++++++++++---------------------- drivers/edac/amd64_edac.h | 8 -------- drivers/video/sis/init.c | 5 ++--- include/linux/bitops.h | 8 ++++++++ 4 files changed, 34 insertions(+), 33 deletions(-)