Hi Wolfram,
On Sun, Feb 17, 2019 at 1:41 PM Wolfram Sang
<wsa+renesas@xxxxxxxxxxxxxxxxxxxx> wrote:
> Add a fault injector simulating 'arbitration lost' from multi-master
> setups. Read the docs for its usage.
>
> A helper function for future fault injectors using SCL interrupts is
> created to achieve this.
>
> Signed-off-by: Wolfram Sang <wsa+renesas@xxxxxxxxxxxxxxxxxxxx>
> ---
>
> Change since RFC:
>
> * user supplied value is now duration of interference instead of number of
> interferences
> * refactored code to suppy a resusable helper function to install SCL interrupt
> handlers
> * added error checks to interrupt number
> * slightly renamed the SCL interrupt when registering
Thanks for the update!
> --- a/Documentation/i2c/gpio-fault-injection
> +++ b/Documentation/i2c/gpio-fault-injection
> @@ -83,3 +83,28 @@ This is why bus recovery (up to 9 clock pulses) must either check SDA or send
> additional STOP conditions to ensure the bus has been released. Otherwise
> random data will be written to a device!
>
> +Lost arbitration
> +================
> +
> +Here, we want to simulate the condition where the master under tests loses the
test
> +bus arbitration against another master in a multi-master setup.
> +
> +"lose_arbitration"
> +------------------
> +
> +This file is write only and you need to write the duration of the arbitration
> +interference (in us). The calling process will then sleep and wait for the
µs
We do UTF-8 in documentation, don't we?
> --- a/drivers/i2c/busses/i2c-gpio.c
> +++ b/drivers/i2c/busses/i2c-gpio.c
> @@ -162,6 +167,67 @@ static int fops_incomplete_write_byte_set(void *data, u64 addr)
> }
> DEFINE_DEBUGFS_ATTRIBUTE(fops_incomplete_write_byte, NULL, fops_incomplete_write_byte_set, "%llu\n");
>
> +static int i2c_gpio_fi_act_on_scl_irq(struct i2c_gpio_private_data *priv,
> + irqreturn_t handler(int, void*))
> +{
> + int ret, irq = gpiod_to_irq(priv->scl);
> +
> + if (irq < 0)
> + return irq;
> +
> + i2c_lock_bus(&priv->adap, I2C_LOCK_ROOT_ADAPTER);
> +
> + ret = gpiod_direction_input(priv->scl);
> + if (ret)
> + goto unlock;
> +
> + reinit_completion(&priv->scl_irq_completion);
> +
> + ret = request_irq(irq, handler, IRQF_TRIGGER_FALLING,
> + "i2c_gpio_fault_injector_scl_irq", priv);
> + if (ret)
> + goto output;
> +
> + wait_for_completion_interruptible(&priv->scl_irq_completion);
Error checking/propagation (-ERESTARTSYS)?
> +
> + free_irq(irq, priv);
> + output:
> + ret = gpiod_direction_output(priv->scl, 1);
This may overwrite the error code returned by request_irq().
> + unlock:
> + i2c_unlock_bus(&priv->adap, I2C_LOCK_ROOT_ADAPTER);
> +
> + return ret;
> +}
> +
> +static irqreturn_t lose_arbitration_irq(int irq, void *dev_id)
> +{
> + struct i2c_gpio_private_data *priv = dev_id;
> +
> + setsda(&priv->bit_data, 0);
> + udelay(priv->scl_irq_data);
On 32-bit, u64 scl_irq_data is truncated...
> + setsda(&priv->bit_data, 1);
> +
> + complete(&priv->scl_irq_completion);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static int fops_lose_arbitration_set(void *data, u64 duration)
> +{
> + struct i2c_gpio_private_data *priv = data;
> +
> + priv->scl_irq_data = duration;
... since calling udelay() with large numbers can be dangerous, perhaps you
want to limit it to say 100 ms max anyway?
> + /*
> + * Interrupt on falling SCL. This ensures that the master under test has
> + * really started the transfer. Interrupt on falling SDA did only
> + * exercise 'bus busy' detection on some HW but not 'arbitration lost'.
> + * Note that the interrupt latency may cause the first bits to be
> + * transmitted correctly.
> + */
> + return i2c_gpio_fi_act_on_scl_irq(priv, lose_arbitration_irq);
> +}
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
