> > > > +int ethtool_cmis_wait_for_cond(struct net_device *dev, u8 flags, u8 flag,
> > > > + u16 max_duration, u32 offset,
> > > > + bool (*cond_success)(u8), bool (*cond_fail)(u8),
> > > > + u8 *state)
> > > > +{
> > > > + const struct ethtool_ops *ops = dev->ethtool_ops;
> > > > + struct ethtool_module_eeprom page_data = {0};
> > > > + struct cmis_wait_for_cond_rpl rpl = {};
> > > > + struct netlink_ext_ack extack = {};
> > > > + unsigned long end;
> > > > + int err;
> > > > +
> > > > + if (!(flags & flag))
> > > > + return 0;
> > > > +
> > > > + if (max_duration == 0)
> > > > + max_duration = U16_MAX;
> > > > +
> > > > + end = jiffies + msecs_to_jiffies(max_duration);
> > > > + do {
> > > > + ethtool_cmis_page_init(&page_data, 0, offset, sizeof(rpl));
> > > > + page_data.data = (u8 *)&rpl;
> > > > +
> > > > + err = ops->get_module_eeprom_by_page(dev, &page_data,
> > > &extack);
> > > > + if (err < 0) {
> > > > + if (extack._msg)
> > > > + netdev_err(dev, "%s\n", extack._msg);
> > > > + continue;
> > >
> > > continue here is interested. Say you get -EIO because the module has
> > > been ejected. I would say that is fatal. Won't this spam the logs, as
> > > fast as the I2C bus can fail, without the 20ms sleep, for 65535 jiffies?
> >
> > If the module is ejected from some reason, it might span the logs I guess.
Please could you test it.
65535 jiffies is i think 655 seconds? That is probably too long to
loop when the module has been ejected. Maybe replace it with HZ?
Maybe netdev_err() should become netdev_dbg()? And please add a 20ms
delay before the continue.
> > > > + }
> > > > +
> > > > + if ((*cond_success)(rpl.state))
> > > > + return 0;
> > > > +
> > > > + if (*cond_fail && (*cond_fail)(rpl.state))
> > > > + break;
> > > > +
> > > > + msleep(20);
> > > > + } while (time_before(jiffies, end));
> > >
> > > Please could you implement this using iopoll.h. This appears to have
> > > the usual problem. Say msleep(20) actually sleeps a lot longer,
> > > because the system is busy doing other things. time_before(jiffies,
> > > end)) is false, because of the long delay, but in fact the operation
> > > has completed without error. Yet you return EBUSY. iopoll.h gets this
> > > correct, it does one more evaluation of the condition after exiting
> > > the loop to handle this issue.
> >
> > OK.
>
> Hi Andrew,
>
> Therefore, unfortunately in this case I'd rather stay with the origin code.
O.K. Please evaluate the condition again after the while() just so
ETIMEDOUT is not returned in error.
Andrew
