Re: [PATCH 2/2] add w1_ds28e17 driver for the DS28E17 Onewire to I2C master bridge

[Evgeniy Polyakov <zbr@ioremap.net>]

Hi Jan

16.07.2016, 23:15, "Jan Kandziora" <jjj@gmx.de>:
> This subpatch adds a driver for the DS28E17 Onewire to I2C master bridge.
>
> Signed-off-by: Jan Kandziora <jjj@gmx.de>

Both patches look good to me, I ack and recommend them for inclusion.
There is a tiny typo and a bit general question below.

Acked-by: Evgeniy Polyakov <zbr@ioremap.net>

> +/* Contants for calculating the busy sleep. */

It should be 'constants' I suppose

> +/* Wait a while until the busy flag clears. */
> +static int w1_f19_i2c_busy_wait(struct w1_slave *sl, size_t count)
> +{
> + const unsigned long timebases[3] = W1_F19_BUSY_TIMEBASES;
> + struct w1_f19_data *data = sl->family_data;
> + unsigned int checks;
> +
> + /* Check the busy flag first in any case.*/
> + if (w1_touch_bit(sl->master, 1) == 0)
> + return 0;
> +
> + /*
> + * Do a generously long sleep in the beginning,
> + * as we have to wait at least this time for all
> + * the I2C bytes at the given speed to be transferred.
> + */
> + usleep_range(timebases[data->speed] * (data->stretch) * count,
> + timebases[data->speed] * (data->stretch) * count
> + + W1_F19_BUSY_GRATUITY);
> +
> + /* Now continusly check the busy flag sent by the DS28E17. */
> + checks = W1_F19_BUSY_CHECKS;
> + while ((checks--) > 0) {

This will burn CPU for 1000 cycles of timebase[data->speed] useconds.
Is that a hardware limitation that there is no interrupt or other completion mechanism which would handle this case?

> + /* Return success if the busy flag is cleared. */
> + if (w1_touch_bit(sl->master, 1) == 0)
> + return 0;
> +
> + /* Wait one non-streched byte timeslot. */
> + udelay(timebases[data->speed]);
> + }
> +
> + /* Timeout. */
> + dev_warn(&sl->dev, "busy timeout\n");
> + return -EIO;
> +}