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
Date: Wed, 16 Oct 2013 14:02:21 -0300 [thread overview]
Message-ID: <20131016140221.6f2299d8@samsung.com> (raw)
In-Reply-To: <1381935366-11731-4-git-send-email-gong.chen@linux.intel.com>
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);
--
Cheers,
Mauro
next prev parent reply other threads:[~2013-10-16 17:02 UTC|newest]
Thread overview: 47+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-10-16 14:55 [PATCH v2 0/9] Extended H/W error log driver Chen, Gong
2013-10-16 14:55 ` [PATCH v2 1/9] ACPI, APEI, CPER: Fix status check during error printing Chen, Gong
2013-10-16 16:53 ` Mauro Carvalho Chehab
2013-10-16 14:55 ` [PATCH v2 2/9] ACPI, CPER: Update cper info Chen, Gong
2013-10-16 16:28 ` Borislav Petkov
2013-10-16 16:52 ` Mauro Carvalho Chehab
2013-10-16 14:56 ` [PATCH v2 3/9] bitops: Introduce a more generic BITMASK macro Chen, Gong
2013-10-16 16:41 ` Borislav Petkov
2013-10-16 17:02 ` Mauro Carvalho Chehab [this message]
2013-10-17 2:31 ` Chen Gong
2013-10-17 2:59 ` Joe Perches
2013-10-17 6:30 ` Chen Gong
2013-10-17 6:58 ` Joe Perches
2013-10-17 7:38 ` Chen Gong
2013-10-17 8:32 ` Joe Perches
2013-10-17 8:40 ` Borislav Petkov
2013-10-17 8:55 ` Joe Perches
2013-10-17 16:10 ` Tony Luck
2013-10-17 18:13 ` Joe Perches
2013-10-16 14:56 ` [PATCH v2 4/9] ACPI, x86: Extended error log driver for x86 platform Chen, Gong
2013-10-16 17:02 ` Borislav Petkov
2013-10-16 14:56 ` [PATCH v2 5/9] DMI: Parse memory device (type 17) in SMBIOS Chen, Gong
2013-10-16 17:05 ` Borislav Petkov
2013-10-17 10:14 ` Mauro Carvalho Chehab
2013-10-16 14:56 ` [PATCH v2 6/9] ACPI, APEI, CPER: Add UEFI 2.4 support for memory error Chen, Gong
2013-10-16 16:43 ` Mauro Carvalho Chehab
2013-10-17 10:23 ` Mauro Carvalho Chehab
2013-10-17 12:16 ` Chen Gong
2013-10-17 12:23 ` Naveen N. Rao
2013-10-16 14:56 ` [PATCH v2 7/9] ACPI, APEI, CPER: Enhance memory reporting capability Chen, Gong
2013-10-16 17:11 ` Borislav Petkov
2013-10-17 10:24 ` Mauro Carvalho Chehab
2013-10-16 14:56 ` [PATCH v2 8/9] ACPI, APEI, CPER: Cleanup CPER memory error output format Chen, Gong
2013-10-16 17:24 ` Borislav Petkov
2013-10-17 10:27 ` Mauro Carvalho Chehab
2013-10-16 14:56 ` [PATCH v2 9/9] ACPI / trace: Add trace interface for eMCA driver Chen, Gong
2013-10-16 15:50 ` Mauro Carvalho Chehab
2013-10-16 17:29 ` Borislav Petkov
2013-10-16 15:06 ` [PATCH v2 0/9] Extended H/W error log driver Chen Gong
2013-10-16 16:05 ` Borislav Petkov
2013-10-16 16:49 ` Joe Perches
2013-10-16 16:56 ` Steven Rostedt
2013-10-16 18:00 ` Borislav Petkov
2013-10-16 18:11 ` Borislav Petkov
2013-10-17 14:33 ` Chen Gong
2013-10-17 15:25 ` Steven Rostedt
2013-10-17 15:35 ` Borislav Petkov
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=20131016140221.6f2299d8@samsung.com \
--to=m.chehab@samsung.com \
--cc=arozansk@redhat.com \
--cc=bp@alien8.de \
--cc=gong.chen@linux.intel.com \
--cc=joe@perches.com \
--cc=linux-acpi@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=naveen.n.rao@linux.vnet.ibm.com \
--cc=plagnioj@jcrosoft.com \
--cc=thomas@winischhofer.net \
--cc=tomi.valkeinen@ti.com \
--cc=tony.luck@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
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