On Fri, Nov 08, 2019 at 10:18:16AM +0000, Andrew Murray wrote:
> 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.
Thank you, Keith and Andrew, I'll submit v2 soon.
--
With Best Regards,
Andy Shevchenko
