On 2018-02-23 00:13, Christoph Hellwig wrote:


+/**
+ * pci__wait_for_link - Wait for link till its active/inactive

typo - just wants a single underscore.

+       pcie_capability_read_word(pdev, PCI_EXP_LNKSTA, &lnk_status);
+       ret = !!(lnk_status & PCI_EXP_LNKSTA_DLLLA);

no need for the !! when assigning to a boolean.

bool is 8 bit, while
#define  PCI_EXP_LNKSTA_DLLLA   0x2000  /* Data Link Layer Link Active */
so I think we need it. otherwise it will treat lnk_status & PCI_EXP_LNKSTA_DLLLA as 0 even if 13th bit is set.


+
+       while ((ret != active) && (timeout > 0)) {

No need for either pair of inner braces.

+               msleep(10);
+               timeout -= 10;
+               pcie_capability_read_word(pdev, PCI_EXP_LNKSTA, &lnk_status);
+               ret = !!(lnk_status & PCI_EXP_LNKSTA_DLLLA);

Same as above.

+       }
+
+       if (ret == active)
+               return true;

Seems like the structure is a bit odd.  Why not:

        for (;;) {
                pcie_capability_read_word(pdev, PCI_EXP_LNKSTA, &lnk_status);
                if ((lnk_status & PCI_EXP_LNKSTA_DLLLA) == active)
                        return true;
                if (timeout <= 0)
                        break;
                timeout -= 10;
        }

Thanks for suggestion, will do this way. thanks.
Regards,
Oza.


Reply via email to