Re: [PATCH 2/2] PCI: keembay: Add support for Intel Keem Bay

[Andy Shevchenko <andriy.shevchenko@linux.intel.com>]

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