Re: [PATCH v5 1/1] Fixes: 3145229f9191 ("iio:pressure:ms5611: power regulator support")

[Martin Kepplinger <martink@posteo.de>]

Am 2016-03-09 um 11:02 schrieb Gregor Boirie:
> Hi Daniel,
> 
> On 03/08/2016 10:16 PM, Daniel Baluta wrote:
>> On Tue, Mar 8, 2016 at 8:00 PM, Gregor Boirie
>> <gregor.boirie@parrot.com> wrote:
>>> When not compiled-in, regulator layer will return a NULL pointer when
>>> trying to get a reference to any regulator using
>>> devm_regulator_get().  As
>>> IS_ERR() does not consider this an error, the ms5611 probing
>>> operation will
>>> try to enable a NULL regulator, which will invariably cause a kernel
>>> crash.
>>> This patch fixes this situation by using devm_regulator_get_optional()
>>> instead of devm_regulator_get().
>>>
>>> Signed-off-by: Gregor Boirie <gregor.boirie@parrot.com>
>> $Subject should be here. And the commit subject should be something
>> more appropriate like:
>>
>> iio: pressure: ms5611: Fix crash when ...
> I'm a bit lost here since when Jonathan asked for "a fixes tag" inclusion
> (http://permalink.gmane.org/gmane.linux.kernel.iio/22425), I supposed he
> meant a tag such as one described in the Documentation/SubmittingPatches
> file.
> My initial attempt was sent with the subject:
> "iio:pressure:ms5611: fix oops when probing regulator".
> 
> Would you please be kind enough to give me some sort of rules as to when
> and how
> to write subject lines related to fixes patch ?
> 

I guess you can just put in a "Fixes:" line, along with your
"Signed-off-by:" lines.

> Regards,
> Grégor.
> 
>>
>>
>>> ---
>>>   drivers/iio/pressure/ms5611_core.c | 8 ++++++--
>>>   1 file changed, 6 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/iio/pressure/ms5611_core.c
>>> b/drivers/iio/pressure/ms5611_core.c
>>> index 37dbc04..a2a871b 100644
>>> --- a/drivers/iio/pressure/ms5611_core.c
>>> +++ b/drivers/iio/pressure/ms5611_core.c
>>> @@ -387,8 +387,8 @@ static const struct iio_info ms5611_info = {
>>>   static int ms5611_init(struct iio_dev *indio_dev)
>>>   {
>>>          int ret;
>>> -       struct regulator *vdd =
>>> devm_regulator_get(indio_dev->dev.parent,
>>> -                                                  "vdd");
>>> +       struct regulator *vdd =
>>> +               devm_regulator_get_optional(indio_dev->dev.parent,
>>> "vdd");
>>>
>>>          /* Enable attached regulator if any. */
>>>          if (!IS_ERR(vdd)) {
>>> @@ -398,6 +398,10 @@ static int ms5611_init(struct iio_dev *indio_dev)
>>>                                  "failed to enable Vdd supply: %d\n",
>>> ret);
>>>                          return ret;
>>>                  }
>>> +       } else {
>>> +               ret = PTR_ERR(vdd);
>>> +               if (ret != -ENODEV)
>>> +                       return ret;
>>>          }
>>>
>>>          ret = ms5611_reset(indio_dev);
>>> -- 
>>> 2.1.4
>>>
>>> -- 
>>> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> -- 
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html