On 10/04/2020 08:38, Kieran Bingham wrote:
> Hi Jacopo,
>>>
>>> priv->gpiod_pwdn = devm_gpiod_get_optional(&client->dev, "enable",
>>> GPIOD_OUT_HIGH);
>>> - if (IS_ERR(priv->gpiod_pwdn))
>>> - return PTR_ERR(priv->gpiod_pwdn);
>>> + if (IS_ERR(priv->gpiod_pwdn)) {
>>> + ret = PTR_ERR(priv->gpiod_pwdn);
>>> + goto err_cleanup_dt;
>>> + }
>>>
>>> gpiod_set_consumer_name(priv->gpiod_pwdn, "max9286-pwdn");
>>> gpiod_set_value_cansleep(priv->gpiod_pwdn, 1);
>>
>> As we get_optional(), shouldn't this be protected by an
>> if (priv->gpiod_pwdn)
>>
>> ?
>>
>
> Err - yes! That's odd - I was sure I had tested this without a gpio
> specifier, and I thought those functions were null-safe, and were a
> no-op if there was no desc. Clearly some wire got crossed in my head -
> because I see no such null-checks now!
Ahem, ok - so as highlighted by Geert, - these *are* NULL safe...
>
>
> I've added a new squash patch on top to correct for this (including
> checking priv->gpiod_pwdn on all uses).
now dropped :-)
