Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1640114

Re: [PATCH] i2c-designware: add i2c gpio recovery option

Show all headers | View raw

On 11/05/2017 21:53, Andy Shevchenko wrote:
>>>> +static int i2c_dw_init_recovery_info(struct dw_i2c_dev *dev,
>>>> +                                    struct i2c_adapter *adap)
>>>> +{
>>>> +       struct i2c_bus_recovery_info *rinfo = &dev->rinfo;
>>>> +
>>>> +       dev->gpio_scl = devm_gpiod_get_optional(dev->dev,
>>>> +                                               "scl",
>>>> +                                               GPIOD_OUT_HIGH);
>>>> +       if (IS_ERR_OR_NULL(dev->gpio_scl))
>>> This is wrong. You should not use this macro in most cases. And
>>> especially it breaks the logic behind _optional().
>> My logic here was that if the gpio is optional return null we return
>> 0.
> Why?!
> 
> _optional()*implies*  that all rest calls will go fine and do nothing.
> 
>> which is an okay status.
>> But this breaks if !CONFIG_GPIOLIB, which I keep forgetting. I've
>> never
>> quite wrapped my head around why that's the case.
>>
>> But the probe function only bails out if this returns EPROBE_DEFER.
>> Not sure that's the best approach
> You need something like
> 
> desc = devm_gpiod_get_optional(...);
> if (IS_ERR(desc))
>   return PTR_ERR(desc);
> 
I found that continuing without the check on null results in a kernel panic for a dereferenced null pointer.
So something in the gpiolib doesn't treat a null desc as optional.

It was probably this code:
int desc_to_gpio(const struct gpio_desc *desc)
{
	return desc->gdev->base + (desc - &desc->gdev->descs[0]);
}

So perhaps this should return an invalid gpio number when desc == null

I don't know what the intents are, so don't know if its a "bug" or  by design.

-- 
Regards
Phil Reid

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

RFC: i2c designware gpio recovery Tim Sander <tim@krieglstein.org> - 2017-04-28 17:50 +0200
  Re: RFC: i2c designware gpio recovery Tim Sander <tim@krieglstein.org> - 2017-04-28 18:20 +0200
    Re: RFC: i2c designware gpio recovery Phil Reid <preid@electromag.com.au> - 2017-05-01 04:10 +0200
      Re: RFC: i2c designware gpio recovery Tim Sander <tim@krieglstein.org> - 2017-05-01 15:40 +0200
        Re: RFC: i2c designware gpio recovery Phil Reid <preid@electromag.com.au> - 2017-05-03 03:40 +0200
          Re: RFC: i2c designware gpio recovery Tim Sander <tim@krieglstein.org> - 2017-05-03 21:10 +0200
            Re: RFC: i2c designware gpio recovery Phil Reid <preid@electromag.com.au> - 2017-05-10 09:20 +0200
              [PATCH] i2c-designware: add i2c gpio recovery option Tim Sander <tim@krieglstein.org> - 2017-05-10 14:00 +0200
                Re: [PATCH] i2c-designware: add i2c gpio recovery option Andy Shevchenko <andriy.shevchenko@linux.intel.com> - 2017-05-10 15:20 +0200
                Re: [PATCH] i2c-designware: add i2c gpio recovery option Phil Reid <preid@electromag.com.au> - 2017-05-11 03:30 +0200
                Re: [PATCH] i2c-designware: add i2c gpio recovery option Andy Shevchenko <andriy.shevchenko@linux.intel.com> - 2017-05-11 16:00 +0200
                Re: [PATCH] i2c-designware: add i2c gpio recovery option Andy Shevchenko <andriy.shevchenko@linux.intel.com> - 2017-05-11 16:10 +0200
                Re: [PATCH] i2c-designware: add i2c gpio recovery option Phil Reid <preid@electromag.com.au> - 2017-05-12 04:00 +0200
                Re: [PATCH] i2c-designware: add i2c gpio recovery option Andy Shevchenko <andriy.shevchenko@linux.intel.com> - 2017-05-12 12:20 +0200
  Re: RFC: i2c designware gpio recovery Phil Reid <preid@electromag.com.au> - 2017-05-01 04:20 +0200

csiph-web