On Tue, Nov 03, 2020 at 04:22:23PM -0600, Bjorn Helgaas wrote:
> On Tue, Oct 27, 2020 at 02:00:11PM +0800, Wan Ahmad Zainie wrote:
> > +static int keembay_pcie_link_up(struct dw_pcie *pci)
> > +{
> > + struct keembay_pcie *pcie = dev_get_drvdata(pci->dev);
> > + u32 val, mask;
> > +
> > + val = keembay_pcie_readl(pcie, PCIE_REGS_PCIE_SII_PM_STATE);
> > + mask = SMLH_LINK_UP | RDLH_LINK_UP;
> > +
> > + return !!((val & mask) == mask);
>
> I think the "!!" is redundant since you're applying it to a value
> that's a boolean already. So you should be able to do:
>
> return (val & mask) == mask;
>
> But it seems like "mask" just obfuscates a little bit, too.
> Personally I think it'd be easier to add something like:
>
> #define PCIE_REGS_PCIE_SII_LINK_UP (SMLH_LINK_UP | RDLH_LINK_UP)
>
> and then:
> if ((val & PCIE_REGS_PCIE_SII_LINK_UP) == PCIE_REGS_PCIE_SII_LINK_UP)
> return 1;
> return 0;
The !! is usual way to guarantee that *int* type out of boolean will be only
0 or 1. That said, the above proposal suits better.
> or even:
>
> return (val & PCIE_REGS_PCIE_SII_LINK_UP) == PCIE_REGS_PCIE_SII_LINK_UP;
--
With Best Regards,
Andy Shevchenko
