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

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

 



On 2019-02-19 14:13, Wolfram Sang wrote:
> 
>>>>> +       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;
>>
>> That may also overwrite the error code, of course. Isn't it
>> usually best to return the first error? I have no clue if that
>> guideline does not apply here, though...
> 
> I am neither entirely sure here. My take was that the above was the more
> severe error. Because if setting to output fails, the GPIO I2C bus will
> be broken. If the former stuff fails, well, the injection didn't work or
> was interrupted.
> 
> However, the GPIO was set to output before the injector. So, if setting
> it back fails, then the system likely has more severe problems anyhow?

One way to end up with that is if there is an irq attached to the gpio
pin. If you disable the irq, you are (sometimes) allowed to change the
gpio to output, but not if the irq is active. So, if some other part of
the driver "steals" the gpio by activating an irq while the injector is
doing its thing, it is possible to end up with this.

But that seems like a gigantic corner case.

> I am open to any better solution. However, let's not forget, this is
> debug code aimed to be used by devs.
> 

You are right, please ignore me.

Cheers,
Peter




[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