Re: [PATCH 1/2] i2c: gpio: fault-injector: add 'lose_arbitration' injector

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Geert,

thanks for this review, too!

> >
> > +Lost arbitration
> > +================
> > +
> > +Here, we want to simulate the condition where the master under tests loses the
> 
> test

Ack.

> > +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?

Dunno, I can change. Only 4 occcurences of 'µs' in Documentation/ so far.

> > +       wait_for_completion_interruptible(&priv->scl_irq_completion);
> 
> Error checking/propagation (-ERESTARTSYS)?

Are you sure? ERESTARTSYS belongs to the "These should never be seen by
user programs." group.

> 
> > +
> > +       free_irq(irq, priv);
> > + output:
> > +       ret = gpiod_direction_output(priv->scl, 1);
> 
> This may overwrite the error code returned by request_irq().

Yeah. What do you think about this, is this too dense?

	ret = gpiod_direction_output(priv->scl, 1) ?: ret;

> > +       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?

Yeah, good idea. Will apply it for both injectors.

Happy hacking,

   Wolfram

Attachment: signature.asc
Description: PGP signature


[Index of Archives]     [Linux Samsung SOC]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]

  Powered by Linux