On Fri, Nov 08, 2019 at 07:37:26AM +0900, Keith Busch wrote:
> On Fri, Oct 11, 2019 at 12:02:30PM +0300, Andy Shevchenko wrote:
> > No functional changes implied.
>
> [snip]
>
> > - while (true) {
> > + do {
> > pcie_capability_read_word(pdev, PCI_EXP_SLTSTA, &slot_status);
> > if (slot_status == (u16) ~0) {
> > ctrl_info(ctrl, "%s: no response from device\n",
> > @@ -81,11 +81,9 @@ static int pcie_poll_cmd(struct controller *ctrl, int timeout)
> > PCI_EXP_SLTSTA_CC);
> > return 1;
> > }
> > - if (timeout < 0)
> > - break;
> > msleep(10);
> > timeout -= 10;
> > - }
> > + } while (timeout > 0);
> > return 0; /* timeout */
> > }
>
> If you really want to ensure no funcitonal change, I think the end of
> the loop needs to be 'while (timeout >= 0);'
With this suggested change, you can add:
Reviewed-by: Andrew Murray <andrew.murray@xxxxxxx>
I can't get too excited by coding styles, however I find this more
readable now, due to the fact that the loop is clearly bounded.
