Hello, On Fri, Sep 28, 2012 at 09:11:34AM +0530, viresh kumar wrote: > >> + ndelay(delay); > >> + gpio_set_value(bri->scl_gpio, val); > >> + > >> + /* break if sda got high, check only when scl line is high */ > >> + if (!bri->skip_sda_polling && val) > >> + if (gpio_get_value(bri->sda_gpio)) > >> + break; > > I'm not sure it's worth to conditionally shortcut here. Either always or > > never shortcut. > > This was done, because few platforms may not have configuration bits to read > status of sda line.. For them skip_sda_polling was required. > > Whereas, others would need this to see if we can return early. What is the upside of returning early? I'd say, just don't do it. > >> + * @put_gpio: called after recover_bus() to get padmux configured for scl line > >> + * as scl. Only required if is_gpio_recovery == true. > >> + * @scl_gpio: gpio number of the scl line. Only required if is_gpio_recovery == > >> + * true. > >> + * @sda_gpio: gpio number of the sda line. Only required if is_gpio_recovery == > >> + * true and skip_sda_polling == false. > >> + * @scl_gpio_flags: flag for gpio_request_one of scl_gpio. 0 implies > >> + * GPIOF_OUT_INIT_LOW. > > IMHO you should not make this configurable but use > > > > GPIOF_OUT_INIT_HIGH | GPIOF_OPEN_DRAIN > > Discussed here: > http://www.spinics.net/lists/linux-i2c/msg07325.html Why do you default to GPIOF_OUT_INIT_LOW? The idle state of scl is high. Using low here introduces an additional clk pulse. Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-König | Industrial Linux Solutions | http://www.pengutronix.de/ | -- 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
