On 14/10/2021 11:17, Philipp Zabel wrote:
On Thu, 2021-10-14 at 11:34 +0200, Michael Tretter wrote:
On Wed, 13 Oct 2021 10:05:30 +0200, Michael Tretter wrote:
On Tue, 12 Oct 2021 15:27:11 +0200, Philipp Zabel wrote:
[...]
+static int isl7998x_wait_power_on(struct isl7998x *isl7998x)
+{
+ struct device *dev = isl7998x->subdev.dev;
+ unsigned int retry;
+ u32 chip_id;
+ int ret = -ETIMEDOUT;
+
+ for (retry = 10; ret && retry > 0; retry--) {
+ ret = regmap_read(isl7998x->regmap,
+ ISL7998x_REG_P0_PRODUCT_ID_CODE, &chip_id);
+ usleep_range(1000, 2000);
+ }
Consider using regmap_read_poll_timeout() here.
Ack. I forgot about this function.
regmap_read_poll_timeout() cannot be used here, because it returns if
regmap_read() returns an error. The driver uses the return value of
regmap_read() to detect, if the chip is available, and should continue polling
if regmap_read() failed. I can implement the necessary behavior with
read_poll_timeout(), but am not sure if it is really worth it.
Oh, right. I still think so, but your call.
regards
Philipp
This wait_power_on function seems odd to me. Wouldn't it be better to
just wait for the power-on delay specified in the datasheet and then
unconditionally go into isl7998x_init? If the device has failed to come
up, that init will fail in its regmap accesses. If you're trying to do
the init earlier than the datasheet specified time then being able to
read the product id code doesn't guarantee the rest of the chip is
ready. If there's no datasheet specification maybe just wait 10ms to 20ms?
Regards,
Ian
