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
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
