Hi Jan 16.07.2016, 23:15, "Jan Kandziora" <jjj@xxxxxx>: > This subpatch adds a driver for the DS28E17 Onewire to I2C master bridge. > > Signed-off-by: Jan Kandziora <jjj@xxxxxx> 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@xxxxxxxxxxx> > +/* 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; > +} -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html