Skip to contentSkip to conversation

iio:pressure:ms5611: fix and enhancements

PATCH Superseded Gregor Boirie 5 participants 7 messages Mar 8, 2016 – Mar 9, 2016 2 messages not in the archive IIO

v5 of 6 Superseded Latest is v6 →

lore
Share & tools
lore
#0 lore raw

v5 cover iio:pressure:ms5611: fix and enhancements

Changes since v1:
* temperature compensation fix:
* reword commit message
* remove useless parenthesis
* power regulator: reword commit message
* add oversampling rate support

Changes since v2:
* remove non printable characters from commit messages

Changes since v3:
* drop first 3 patches since applied onto testing
* fix oops at regulator probing time if regulator support not compiled-in
* complete device-tree support and documentation
* remove continuous sampling support (see Jonathan's "kthread tight loop trigger" proposal)
* checkpatch'ed

Changes since v4:
* drop first 2 patches since applied
* amend commit to include "Fixes" tag

Gregor Boirie (1):

Fixes 3145229f9191 ("iio:pressure:ms5611: power regulator support")
 drivers/iio/pressure/ms5611_core.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)
Signature · 1 line
2.1.4

A message that is not in the archive (sent off-list or not yet received): cover.1457459537.git.gregor.boirie-ITF29qwbsa/[email protected]

#2 lore raw

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

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().

---
 drivers/iio/pressure/ms5611_core.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)
drivers/iio/pressure/ms5611_core.c +6 -2
index 37dbc04..a2a871b 100644
@@ -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);
Signature · 1 line
2.1.4

A message that is not in the archive (sent off-list or not yet received): 26a4b9ab8ae9b40f9c96c51f12c977fdad36532f.1457459537.git.gregor.boirie-ITF29qwbsa/[email protected]

#4 lore raw

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

Quoted 9 lines · Gregor Boirie

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().

$Subject should be here. And the commit subject should be something more appropriate like:

iio: pressure: ms5611: Fix crash when ...

Quoted 37 lines
---
 drivers/iio/pressure/ms5611_core.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)
drivers/iio/pressure/ms5611_core.c +6 -3
index 37dbc04..a2a871b 100644
@@ -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 [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

#5 lore raw

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

Hi Daniel,

Quoted 9 lines · Daniel Baluta
Quoted 8 lines · Gregor Boirie

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().

Quoted 5 lines
Quoted 1 line

$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 ?

Regards,
Grégor.

Quoted 39 lines
Quoted 39 lines

---
  drivers/iio/pressure/ms5611_core.c | 8 ++++++--
  1 file changed, 6 insertions(+), 2 deletions(-)
drivers/iio/pressure/ms5611_core.c +6 -3
index 37dbc04..a2a871b 100644
@@ -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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

#6 lore raw

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

Am 2016-03-09 um 11:02 schrieb Gregor Boirie:

Quoted 27 lines

Hi Daniel,

Quoted 12 lines · Daniel Baluta
Quoted 10 lines · Gregor Boirie

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().

Quoted 5 lines
Quoted 1 line

$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".

Quoted 4 lines

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.

Quoted 51 lines

Regards,
Grégor.

Quoted 43 lines
Quoted 43 lines

---
  drivers/iio/pressure/ms5611_core.c | 8 ++++++--
  1 file changed, 6 insertions(+), 2 deletions(-)
drivers/iio/pressure/ms5611_core.c +2 -2
index 37dbc04..a2a871b 100644
@@ -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)) {
drivers/iio/pressure/ms5611_core.c +4 -0
@@ -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);
Signature · 6 lines
2.1.4

-- 
To unsubscribe from this list: send the line "unsubscribe linux-iio" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Signature · 3 lines
To unsubscribe from this list: send the line "unsubscribe linux-iio" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
#7 lore raw

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

Quoted 28 lines · Martin Kepplinger

Am 2016-03-09 um 11:02 schrieb Gregor Boirie:

Quoted 27 lines

Hi Daniel,

Quoted 12 lines · Daniel Baluta
Quoted 10 lines · Gregor Boirie

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().

Quoted 5 lines
Quoted 1 line

$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".

Quoted 6 lines
Quoted 4 lines

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.

Oh... <light bulb enlightenment/> ! I definitely need some decent manners lessons :) Thanks.

Quoted 50 lines
Quoted 50 lines

Regards,
Grégor.

Quoted 42 lines
Quoted 42 lines
---
   drivers/iio/pressure/ms5611_core.c | 8 ++++++--
   1 file changed, 6 insertions(+), 2 deletions(-)
drivers/iio/pressure/ms5611_core.c +2 -2
index 37dbc04..a2a871b 100644
@@ -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)) {
drivers/iio/pressure/ms5611_core.c +4 -0
@@ -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);
Signature · 6 lines
2.1.4

-- 
To unsubscribe from this list: send the line "unsubscribe linux-iio" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Signature · 3 lines
To unsubscribe from this list: send the line "unsubscribe linux-iio" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
#8 lore raw

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

Quoted 28 lines · Gregor Boirie
Quoted 25 lines · Martin Kepplinger

Am 2016-03-09 um 11:02 schrieb Gregor Boirie:

Quoted 23 lines

Hi Daniel,

Quoted 19 lines · Daniel Baluta
Quoted 11 lines · Gregor Boirie

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().

Quoted 1 line

$Subject should be here. And the commit subject should be something more appropriate like:

Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 1 line

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".

The initial subject was OK.

Quoted 11 lines
Quoted 7 lines
Quoted 5 lines

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.

Oh... <light bulb enlightenment/> ! I definitely need some decent manners lessons :)
Thanks.

What Martin said :). For example, you can look at this patch:

https://lkml.org/lkml/2016/2/16/505

Daniel.