On 1/27/20 7:40 AM, Heiko Schocher wrote:
> Hello Marek,
>
> Am 24.01.2020 um 19:12 schrieb Marek Vasut:
>> Add custom deblock dequence for the I2C bus, needed on some devices.
>> This sequence is issued once, when probing the driver, and is controlled
>> by DT property, "i2c-gpio,deblock".
>>
>> Signed-off-by: Marek Vasut <ma...@denx.de>
>> ---
>> drivers/i2c/i2c-gpio.c | 67 ++++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 67 insertions(+)
>>
>> diff --git a/drivers/i2c/i2c-gpio.c b/drivers/i2c/i2c-gpio.c
>> index 4e8fa21473..46975ef5c8 100644
>> --- a/drivers/i2c/i2c-gpio.c
>> +++ b/drivers/i2c/i2c-gpio.c
>> @@ -39,6 +39,11 @@ static int i2c_gpio_sda_get(struct gpio_desc *sda)
>> return dm_gpio_get_value(sda);
>> }
>> +static int i2c_gpio_scl_get(struct gpio_desc *scl)
>> +{
>> + return dm_gpio_get_value(scl);
>> +}
>> +
>> static void i2c_gpio_sda_set(struct gpio_desc *sda, int bit)
>> {
>> if (bit)
>> @@ -305,6 +310,67 @@ static int i2c_gpio_set_bus_speed(struct udevice
>> *dev, unsigned int speed_hz)
>> return 0;
>> }
>> +/*
>> + * I2C is a synchronous protocol and resets of the processor in the
>> middle
>> + * of an access can block the I2C Bus until a powerdown of the full
>> unit is
>> + * done. This function toggles the SCL until the SCL and SCA line are
>> + * released, but max. 16 times, after this a I2C start-sequence is sent.
>> + * This I2C Deblocking mechanism was developed by Keymile in association
>> + * with Anatech and Atmel in 1998.
>> + */
>> +static void i2c_gpio_make_abort(struct udevice *dev)
>> +{
>> + struct i2c_gpio_bus *bus = dev_get_priv(dev);
>> + struct gpio_desc *scl = &bus->gpios[PIN_SCL];
>> + struct gpio_desc *sda = &bus->gpios[PIN_SDA];
>> + int scl_state = 0;
>> + int sda_state = 0;
>> + int i = 0;
>> + int ret = 0;
>> +
>> +#define DELAY_ABORT_SEQ 62 /* @200kHz 9 clocks = 44us, 62us is
>> ok */
>> +
>> + i2c_gpio_scl_set(scl, 1);
>> +
>> + if (!i2c_gpio_sda_get(sda)) {
>> + ret = -1;
>> + for (i = 0; i < 16; i++) {
>> + i2c_gpio_scl_set(scl, 0);
>> + udelay(DELAY_ABORT_SEQ);
>> + i2c_gpio_scl_set(scl, 1);
>> + udelay(DELAY_ABORT_SEQ);
>> + scl_state = i2c_gpio_scl_get(scl);
>> + sda_state = i2c_gpio_sda_get(sda);
>> + if (scl_state && sda_state) {
>> + ret = 0;
>> + break;
>> + }
>> + }
>> + }
>> +
>> + if (!ret) {
>> + for (i = 0; i < 5; i++) {
>> + i2c_gpio_send_start(scl, sda, 2 * bus->udelay);
>> + i2c_gpio_scl_set(scl, 0);
>> + }
>> + }
>> +
>> + /* respect stop setup time */
>> + udelay(DELAY_ABORT_SEQ);
>> + i2c_gpio_scl_set(scl, 1);
>> + udelay(DELAY_ABORT_SEQ);
>> + i2c_gpio_sda_set(sda, 1);
>> + i2c_gpio_sda_get(sda);
>> +}
>> +
>> +static int i2c_gpio_drv_probe(struct udevice *dev)
>> +{
>> + if (dev_read_bool(dev, "i2c-gpio,deblock"))
>> + i2c_gpio_make_abort(dev);
>> +
>> + return 0;
>> +}
>> +
>> static int i2c_gpio_ofdata_to_platdata(struct udevice *dev)
>> {
>> struct i2c_gpio_bus *bus = dev_get_priv(dev);
>> @@ -341,6 +407,7 @@ U_BOOT_DRIVER(i2c_gpio) = {
>> .name = "i2c-gpio",
>> .id = UCLASS_I2C,
>> .of_match = i2c_gpio_ids,
>> + .probe = i2c_gpio_drv_probe,
>> .ofdata_to_platdata = i2c_gpio_ofdata_to_platdata,
>> .priv_auto_alloc_size = sizeof(struct i2c_gpio_bus),
>> .ops = &i2c_gpio_ops,
>>
>
> Hmm.. why do you add this into the probe function and do not add
> it to:
>
> https://gitlab.denx.de/u-boot/u-boot/blob/master/include/i2c.h#L382
Because I wasn't aware of this callback.
> which is called from:
> https://gitlab.denx.de/u-boot/u-boot/blob/master/drivers/i2c/i2c-uclass.c#L586
>
>
>
> Ok, i2c_deblock() currently is only used from do_i2c_reset(), but may it
> is worth to adapt drivers/i2c-core/i2c-uclass.c to call the deblock
> sequence also from i2c_pre_probe()?
>
> May dependend on your proposed DT property "i2c-gpio,deblock" ?
>
> But please update also the documentation in
>
> u-boot:/doc/device-tree-bindings/i2c/i2c-gpio.txt
>
> or if we go the more generic way:
>
> u-boot:/doc/device-tree-bindings/i2c/i2c.txt
>
> Thanks!
>
> BTW: May it makes sense to adapt:
> https://gitlab.denx.de/u-boot/u-boot/blob/master/drivers/i2c/i2c-uclass.c#L504
I need to take a look at this one.
--
Best regards,
Marek Vasut
