Messages in this thread Patch in this message | | | Subject | Re: [PATCH v2 1/1] PCI: brcmstb: Use BIT() as __GENMASK() is for internal use only | From | Florian Fainelli <> | Date | Tue, 16 Nov 2021 12:38:39 -0800 |
| |
On 11/15/21 3:20 AM, Andy Shevchenko wrote: > Use BIT() as __GENMASK() is for internal use only. The rationale > of switching to BIT() is to provide better generated code. The > GENMASK() against non-constant numbers may produce an ugly assembler > code. On contrary the BIT() is simply converted to corresponding shift > operation.
The code is not necessarily any different on ARMv8 as far as I can tell, before:
static void brcm_msi_set_regs(struct brcm_msi *msi) { u32 val = __GENMASK(31, msi->legacy_shift); 84: b9406402 ldr w2, [x0,#100] 88: d2800021 mov x1, #0x1 // #1 8c: 9ac22021 lsl x1, x1, x2 90: 4b0103e1 neg w1, w1
after:
static void brcm_msi_set_regs(struct brcm_msi *msi) { u32 val = ~(BIT(msi->legacy_shift) - 1); 84: b9406402 ldr w2, [x0,#100] 88: d2800021 mov x1, #0x1 // #1 8c: 9ac22021 lsl x1, x1, x2 90: 4b0103e1 neg w1, w1
and the usage of BIT() does not make this any clearer.
How about this instead:
diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c index 1fc7bd49a7ad..f832c07337fa 100644 --- a/drivers/pci/controller/pcie-brcmstb.c +++ b/drivers/pci/controller/pcie-brcmstb.c @@ -144,6 +144,8 @@ #define BRCM_INT_PCI_MSI_NR 32 #define BRCM_INT_PCI_MSI_LEGACY_NR 8 #define BRCM_INT_PCI_MSI_SHIFT 0 +#define BRCM_INT_PCI_MSI_MASK GENMASK(BRCM_INT_PCI_MSI_NR - 1, 0) +#define BRCM_INT_PCI_MSI_LEGACY_MASK GENMASK(31, 32 - BRCM_INT_PCI_MSI_LEGACY_NR)
/* MSI target addresses */ #define BRCM_MSI_TARGET_ADDR_LT_4GB 0x0fffffffcULL @@ -619,7 +621,8 @@ static void brcm_msi_remove(struct brcm_pcie *pcie)
static void brcm_msi_set_regs(struct brcm_msi *msi) { - u32 val = __GENMASK(31, msi->legacy_shift); + u32 val = msi->legacy ? BRCM_INT_PCI_MSI_MASK : + BRCM_INT_PCI_MSI_LEGACY_MASK;
writel(val, msi->intr_base + MSI_INT_MASK_CLR); writel(val, msi->intr_base + MSI_INT_CLR);
> > Note, it's the only user of __GENMASK() in the kernel outside of its own realm. > > Fixes: 3baec684a531 ("PCI: brcmstb: Accommodate MSI for older chips") > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > --- > v2: switched to BIT() and elaborated why, hence not included tag > drivers/pci/controller/pcie-brcmstb.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c > index 1fc7bd49a7ad..0c49fc65792c 100644 > --- a/drivers/pci/controller/pcie-brcmstb.c > +++ b/drivers/pci/controller/pcie-brcmstb.c > @@ -619,7 +619,7 @@ static void brcm_msi_remove(struct brcm_pcie *pcie) > > static void brcm_msi_set_regs(struct brcm_msi *msi) > { > - u32 val = __GENMASK(31, msi->legacy_shift); > + u32 val = ~(BIT(msi->legacy_shift) - 1); > > writel(val, msi->intr_base + MSI_INT_MASK_CLR); > writel(val, msi->intr_base + MSI_INT_CLR); >
-- Florian
| |