Messages in this thread | | | Date | Fri, 23 Feb 2018 12:23:30 +0530 | From | poza@codeauro ... | Subject | Re: [PATCH v10 6/7] PCI: Unify wait for link active into generic pci |
| |
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.
| |