> +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 ((*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.
> +static u8 cmis_cdb_calc_checksum(const void *data, size_t size)
> +{
> + const u8 *bytes = (const u8 *)data;
> + u8 checksum = 0;
> +
> + for (size_t i = 0; i < size; i++)
> + checksum += bytes[i];
> +
> + return ~checksum;
> +}
I expect there is already a helper do that somewhere.
Andrew
---
pw-bot: cr
