[PATCH 0/3] iio: magnetometer: ak8975: Add gpio reset support

[PATCH 0/3] iio: magnetometer: ak8975: Add gpio reset support

[Jonathan Albrieux]

Convert documentation from txt format to yaml. Add documentation about
reset-gpio. 

Set reset gpio to high on ak8975_power_on, set reset gpio to low on
ak8975_power_off.

Without setting it to high on ak8975_power_on driver's probe fails
on ak8975_who_i_am while checking for device identity for AK09911 chip

AK09911 has a reset gpio to handle register's reset. If reset gpio is
set to low it will trigger the reset. AK09911 datasheed says that if not
used reset pin should be connected to VID and this patch emulates this
situation

Jonathan Albrieux (3):
  dt-bindings: iio: magnetometer: ak8975: convert txt format to yaml
  dt-bindings: iio: magnetometer: ak8975: add gpio reset support
  iio: magnetometer: ak8975: Add gpio reset support

 .../bindings/iio/magnetometer/ak8975.txt      | 30 --------
 .../bindings/iio/magnetometer/ak8975.yaml     | 70 +++++++++++++++++++
 drivers/iio/magnetometer/ak8975.c             | 21 +++++-
 3 files changed, 89 insertions(+), 32 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/iio/magnetometer/ak8975.txt
 create mode 100644 Documentation/devicetree/bindings/iio/magnetometer/ak8975.yaml

-- 
2.17.1

[PATCH 1/3] dt-bindings: iio: magnetometer: ak8975: convert txt format to yaml

[Jonathan Albrieux]

Converts documentation from txt format to yaml

Signed-off-by: Jonathan Albrieux <jonathan.albrieux@gmail.com>
---
 .../bindings/iio/magnetometer/ak8975.txt      | 30 ---------
 .../bindings/iio/magnetometer/ak8975.yaml     | 66 +++++++++++++++++++
 2 files changed, 66 insertions(+), 30 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/iio/magnetometer/ak8975.txt
 create mode 100644 Documentation/devicetree/bindings/iio/magnetometer/ak8975.yaml

diff --git a/Documentation/devicetree/bindings/iio/magnetometer/ak8975.txt b/Documentation/devicetree/bindings/iio/magnetometer/ak8975.txt
deleted file mode 100644
index aa67ceb0d4e0..000000000000
--- a/Documentation/devicetree/bindings/iio/magnetometer/ak8975.txt
+++ /dev/null
@@ -1,30 +0,0 @@
-* AsahiKASEI AK8975 magnetometer sensor
-
-Required properties:
-
-  - compatible : should be "asahi-kasei,ak8975"
-  - reg : the I2C address of the magnetometer
-
-Optional properties:
-
-  - gpios : should be device tree identifier of the magnetometer DRDY pin
-  - vdd-supply: an optional regulator that needs to be on to provide VDD
-  - mount-matrix: an optional 3x3 mounting rotation matrix
-
-Example:
-
-ak8975@c {
-        compatible = "asahi-kasei,ak8975";
-        reg = <0x0c>;
-        gpios = <&gpj0 7 0>;
-        vdd-supply = <&ldo_3v3_gnss>;
-        mount-matrix = "-0.984807753012208",  /* x0 */
-                       "0",                   /* y0 */
-                       "-0.173648177666930",  /* z0 */
-                       "0",                   /* x1 */
-                       "-1",                  /* y1 */
-                       "0",                   /* z1 */
-                       "-0.173648177666930",  /* x2 */
-                       "0",                   /* y2 */
-                       "0.984807753012208";   /* z2 */
-};
diff --git a/Documentation/devicetree/bindings/iio/magnetometer/ak8975.yaml b/Documentation/devicetree/bindings/iio/magnetometer/ak8975.yaml
new file mode 100644
index 000000000000..86e3efa693a8
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/magnetometer/ak8975.yaml
@@ -0,0 +1,66 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/magnetometer/ak8975.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: AsahiKASEI AK8975 magnetometer sensor
+
+maintainers:
+  - can't find a mantainer, author is Laxman Dewangan <ldewangan@nvidia.com>
+
+properties:
+  compatible:
+    enum:
+      - "asahi-kasei,ak8975"
+      - "ak8975"
+      - "asahi-kasei,ak8963"
+      - "ak8963"
+      - "asahi-kasei,ak09911"
+      - "ak09911"
+      - "asahi-kasei,ak09912"
+      - "ak09912"
+
+  reg:
+    maxItems: 1
+    description: the I2C address of the magnetometer
+
+  gpios:
+    description: should be device tree identifier of the magnetometer DRDY pin
+
+  vdd-supply:
+    maxItems: 1
+    description: |
+      an optional regulator that needs to be on to provide VDD power to
+      the sensor.
+
+  mount-matrix:
+    description: an optional 3x3 mounting rotation matrix
+
+required:
+  - compatible
+  - reg
+
+examples:
+  - |
+    i2c@78b7000 {
+        reg = <0x78b6000 0x600>;
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        ak8975@c {
+            compatible = "asahi-kasei,ak8975";
+            reg = <0x0c>;
+            gpios = <&gpj0 7 0>;
+            vdd-supply = <&ldo_3v3_gnss>;
+            mount-matrix = "-0.984807753012208",  /* x0 */
+                           "0",                   /* y0 */
+                           "-0.173648177666930",  /* z0 */
+                           "0",                   /* x1 */
+                           "-1",                  /* y1 */
+                           "0",                   /* z1 */
+                           "-0.173648177666930",  /* x2 */
+                           "0",                   /* y2 */
+                           "0.984807753012208";   /* z2 */
+        };
+    };
-- 
2.17.1

[PATCH 2/3] dt-bindings: iio: magnetometer: ak8975: add gpio reset support

[Jonathan Albrieux]

Add reset-gpio support.

AK09911 has a reset gpio to handle register's reset. If reset gpio is
set to low it will trigger the reset. AK09911 datasheed says that if not
used reset pin should be connected to VID and this patch emulates this
situation

Without setting it to high on ak8975_power_on driver's probe fails
on ak8975_who_i_am while checking for device identity for AK09911 chip

Signed-off-by: Jonathan Albrieux <jonathan.albrieux@gmail.com>
---
 .../devicetree/bindings/iio/magnetometer/ak8975.yaml          | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/iio/magnetometer/ak8975.yaml b/Documentation/devicetree/bindings/iio/magnetometer/ak8975.yaml
index 86e3efa693a8..a82c0ff5d098 100644
--- a/Documentation/devicetree/bindings/iio/magnetometer/ak8975.yaml
+++ b/Documentation/devicetree/bindings/iio/magnetometer/ak8975.yaml
@@ -37,6 +37,9 @@ properties:
   mount-matrix:
     description: an optional 3x3 mounting rotation matrix
 
+  reset-gpio:
+    description: an optional pin needed for AK09911 to set the reset state
+
 required:
   - compatible
   - reg
@@ -53,6 +56,7 @@ examples:
             reg = <0x0c>;
             gpios = <&gpj0 7 0>;
             vdd-supply = <&ldo_3v3_gnss>;
+            reset-gpio = <&msmgpio 111 1>;
             mount-matrix = "-0.984807753012208",  /* x0 */
                            "0",                   /* y0 */
                            "-0.173648177666930",  /* z0 */
-- 
2.17.1

Re: [PATCH 2/3] dt-bindings: iio: magnetometer: ak8975: add gpio reset support

[Linus Walleij]

On Mon, May 18, 2020 at 3:37 PM Jonathan Albrieux
<jonathan.albrieux@gmail.com> wrote:

> +  reset-gpio:
> +    description: an optional pin needed for AK09911 to set the reset state

This kind of properties should always be plural, so
reset-gpios please.

Yours,
Linus Walleij

Re: [PATCH 2/3] dt-bindings: iio: magnetometer: ak8975: add gpio reset support

[Jonathan Albrieux]

On Mon, May 25, 2020 at 10:43:35AM +0200, Linus Walleij wrote:
> On Mon, May 18, 2020 at 3:37 PM Jonathan Albrieux
> <jonathan.albrieux@gmail.com> wrote:
> 
> > +  reset-gpio:
> > +    description: an optional pin needed for AK09911 to set the reset state
> 
> This kind of properties should always be plural, so
> reset-gpios please.
> 
> Yours,
> Linus Walleij

Thank you, will include this change in current patch version I'm working on.

Best regards,
Jonathan Albrieux

[PATCH 3/3] iio: magnetometer: ak8975: Add gpio reset support

[Jonathan Albrieux]

Set reset gpio to high on ak8975_power_on, set reset gpio to low on
ak8975_power_off.

Without setting it to high on ak8975_power_on driver's probe fails
on ak8975_who_i_am while checking for device identity for AK09911 chip

AK09911 has a reset gpio to handle register's reset. If reset gpio is
set to low it will trigger the reset. AK09911 datasheed says that if not
used reset pin should be connected to VID and this patch emulates this
situation

Signed-off-by: Jonathan Albrieux <jonathan.albrieux@gmail.com>
---
 drivers/iio/magnetometer/ak8975.c | 21 +++++++++++++++++++--
 1 file changed, 19 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/magnetometer/ak8975.c b/drivers/iio/magnetometer/ak8975.c
index 3c881541ae72..c1ba5cd2cb6c 100644
--- a/drivers/iio/magnetometer/ak8975.c
+++ b/drivers/iio/magnetometer/ak8975.c
@@ -358,6 +358,7 @@ struct ak8975_data {
 	u8			asa[3];
 	long			raw_to_gauss[3];
 	struct gpio_desc	*eoc_gpiod;
+	struct gpio_desc	*reset_gpiod;
 	int			eoc_irq;
 	wait_queue_head_t	data_ready_queue;
 	unsigned long		flags;
@@ -384,10 +385,13 @@ static int ak8975_power_on(const struct ak8975_data *data)
 			 "Failed to enable specified Vid supply\n");
 		return ret;
 	}
+
+	gpiod_set_value_cansleep(data->reset_gpiod, 0);
+
 	/*
-	 * According to the datasheet the power supply rise time i 200us
+	 * According to the datasheet the power supply rise time is 200us
 	 * and the minimum wait time before mode setting is 100us, in
-	 * total 300 us. Add some margin and say minimum 500us here.
+	 * total 300us. Add some margin and say minimum 500us here.
 	 */
 	usleep_range(500, 1000);
 	return 0;
@@ -396,6 +400,8 @@ static int ak8975_power_on(const struct ak8975_data *data)
 /* Disable attached power regulator if any. */
 static void ak8975_power_off(const struct ak8975_data *data)
 {
+	gpiod_set_value_cansleep(data->reset_gpiod, 1);
+
 	regulator_disable(data->vid);
 	regulator_disable(data->vdd);
 }
@@ -839,6 +845,7 @@ static int ak8975_probe(struct i2c_client *client,
 	struct ak8975_data *data;
 	struct iio_dev *indio_dev;
 	struct gpio_desc *eoc_gpiod;
+	struct gpio_desc *reset_gpiod;
 	const void *match;
 	unsigned int i;
 	int err;
@@ -856,6 +863,15 @@ static int ak8975_probe(struct i2c_client *client,
 	if (eoc_gpiod)
 		gpiod_set_consumer_name(eoc_gpiod, "ak_8975");
 
+	/*
+	 * If reset pin is provided then will be set to high on power on
+	 * and to low on power off according to AK09911 datasheet
+	 */
+	reset_gpiod = devm_gpiod_get_optional(&client->dev,
+					      "reset", GPIOD_OUT_HIGH);
+	if (IS_ERR(reset_gpiod))
+		return PTR_ERR(reset_gpiod);
+
 	/* Register with IIO */
 	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
 	if (indio_dev == NULL)
@@ -866,6 +882,7 @@ static int ak8975_probe(struct i2c_client *client,
 
 	data->client = client;
 	data->eoc_gpiod = eoc_gpiod;
+	data->reset_gpiod = reset_gpiod;
 	data->eoc_irq = 0;
 
 	err = iio_read_mount_matrix(&client->dev, "mount-matrix", &data->orientation);
-- 
2.17.1

Re: [PATCH 3/3] iio: magnetometer: ak8975: Add gpio reset support

[Andy Shevchenko]

On Mon, May 18, 2020 at 4:38 PM Jonathan Albrieux
<jonathan.albrieux@gmail.com> wrote:

> +       gpiod_set_value_cansleep(data->reset_gpiod, 1);

(1)

...

> +       /*
> +        * If reset pin is provided then will be set to high on power on
> +        * and to low on power off according to AK09911 datasheet
> +        */

Wording is confusing, perhaps you have to use 'asserted / deasserted'.

Btw, in (1) it's also "high" (asserted). I barely understand how it's
supposed to work in all cases?

> +       reset_gpiod = devm_gpiod_get_optional(&client->dev,
> +                                             "reset", GPIOD_OUT_HIGH);
> +       if (IS_ERR(reset_gpiod))
> +               return PTR_ERR(reset_gpiod);


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 3/3] iio: magnetometer: ak8975: Add gpio reset support

[Jonathan Albrieux]

On Mon, May 18, 2020 at 05:55:51PM +0300, Andy Shevchenko wrote:
> On Mon, May 18, 2020 at 4:38 PM Jonathan Albrieux
> <jonathan.albrieux@gmail.com> wrote:
> 
> > +       gpiod_set_value_cansleep(data->reset_gpiod, 1);
> 
> (1)
> 
> ...
> 
> > +       /*
> > +        * If reset pin is provided then will be set to high on power on
> > +        * and to low on power off according to AK09911 datasheet
> > +        */
> 
> Wording is confusing, perhaps you have to use 'asserted / deasserted'.

Thank you for the suggestion, I'll be working on rewording as soon as
possible.

> Btw, in (1) it's also "high" (asserted). I barely understand how it's
> supposed to work in all cases?
> 
> > +       reset_gpiod = devm_gpiod_get_optional(&client->dev,
> > +                                             "reset", GPIOD_OUT_HIGH);
> > +       if (IS_ERR(reset_gpiod))
> > +               return PTR_ERR(reset_gpiod);
> 

I'm sorry but I'm not sure about what you mean by saying all cases.
Currently  I'm testing this driver on a msm8916 device having AK09911
magnetometer. At the current stage the driver is failing on probe 
because reset pin is not connected to VID (as datasheet requires in case
of pin not being used). In case of reset pin not asserted, register's
reset is triggered resulting in empty registers, leading to probe fail.
For this reason pin is asserted during power on in order to have 
informations in registers and deasserted before power off triggering
a reset.

A workaround that gets AK09911 working on device is by setting the
reset pin always high on device tree. This way registers gets reset by
a Power On Reset circuit autonomously and reset pin never triggers the
reset.

> -- 
> With Best Regards,
> Andy Shevchenko

Hoping to having answered to your question,
Best regards,
Jonathan Albrieux

Re: [PATCH 3/3] iio: magnetometer: ak8975: Add gpio reset support

[Andy Shevchenko]

On Mon, May 18, 2020 at 06:01:20PM +0200, Jonathan Albrieux wrote:
> On Mon, May 18, 2020 at 05:55:51PM +0300, Andy Shevchenko wrote:
> > On Mon, May 18, 2020 at 4:38 PM Jonathan Albrieux
> > <jonathan.albrieux@gmail.com> wrote:
> > 
> > > +       gpiod_set_value_cansleep(data->reset_gpiod, 1);
> > 
> > (1)
> > 
> > ...
> > 
> > > +       /*
> > > +        * If reset pin is provided then will be set to high on power on
> > > +        * and to low on power off according to AK09911 datasheet
> > > +        */
> > 
> > Wording is confusing, perhaps you have to use 'asserted / deasserted'.
> 
> Thank you for the suggestion, I'll be working on rewording as soon as
> possible.
> 
> > Btw, in (1) it's also "high" (asserted). I barely understand how it's
> > supposed to work in all cases?
> > 
> > > +       reset_gpiod = devm_gpiod_get_optional(&client->dev,
> > > +                                             "reset", GPIOD_OUT_HIGH);
> > > +       if (IS_ERR(reset_gpiod))
> > > +               return PTR_ERR(reset_gpiod);
> > 
> 
> I'm sorry but I'm not sure about what you mean by saying all cases.
> Currently  I'm testing this driver on a msm8916 device having AK09911
> magnetometer. At the current stage the driver is failing on probe 
> because reset pin is not connected to VID (as datasheet requires in case
> of pin not being used). In case of reset pin not asserted, register's
> reset is triggered resulting in empty registers, leading to probe fail.
> For this reason pin is asserted during power on in order to have 
> informations in registers and deasserted before power off triggering
> a reset.
> 
> A workaround that gets AK09911 working on device is by setting the
> reset pin always high on device tree. This way registers gets reset by
> a Power On Reset circuit autonomously and reset pin never triggers the
> reset.

You need to distinguish electrical level from logical (GPIO flag defines
logical). So, I'm talking about active-high vs. active-low case.

Now I re-read above, and see that here you assert the reset signal. But where
is desertion?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 3/3] iio: magnetometer: ak8975: Add gpio reset support

[Jonathan Albrieux]

On Mon, May 18, 2020 at 07:43:17PM +0300, Andy Shevchenko wrote:
> On Mon, May 18, 2020 at 06:01:20PM +0200, Jonathan Albrieux wrote:
> > On Mon, May 18, 2020 at 05:55:51PM +0300, Andy Shevchenko wrote:
> > > On Mon, May 18, 2020 at 4:38 PM Jonathan Albrieux
> > > <jonathan.albrieux@gmail.com> wrote:
> > > 
> > > > +       gpiod_set_value_cansleep(data->reset_gpiod, 1);
> > > 
> > > (1)
> > > 
> > > ...
> > > 
> > > > +       /*
> > > > +        * If reset pin is provided then will be set to high on power on
> > > > +        * and to low on power off according to AK09911 datasheet
> > > > +        */
> > > 
> > > Wording is confusing, perhaps you have to use 'asserted / deasserted'.
> > 
> > Thank you for the suggestion, I'll be working on rewording as soon as
> > possible.
> > 
> > > Btw, in (1) it's also "high" (asserted). I barely understand how it's
> > > supposed to work in all cases?
> > > 
> > > > +       reset_gpiod = devm_gpiod_get_optional(&client->dev,
> > > > +                                             "reset", GPIOD_OUT_HIGH);
> > > > +       if (IS_ERR(reset_gpiod))
> > > > +               return PTR_ERR(reset_gpiod);
> > > 
> > 
> > I'm sorry but I'm not sure about what you mean by saying all cases.
> > Currently  I'm testing this driver on a msm8916 device having AK09911
> > magnetometer. At the current stage the driver is failing on probe 
> > because reset pin is not connected to VID (as datasheet requires in case
> > of pin not being used). In case of reset pin not asserted, register's
> > reset is triggered resulting in empty registers, leading to probe fail.
> > For this reason pin is asserted during power on in order to have 
> > informations in registers and deasserted before power off triggering
> > a reset.
> > 
> > A workaround that gets AK09911 working on device is by setting the
> > reset pin always high on device tree. This way registers gets reset by
> > a Power On Reset circuit autonomously and reset pin never triggers the
> > reset.
> 
> You need to distinguish electrical level from logical (GPIO flag defines
> logical). So, I'm talking about active-high vs. active-low case.
> 
> Now I re-read above, and see that here you assert the reset signal. But where
> is desertion?

Oh I see, I'll try explaining by points the proposed approach:
- reset pin is active low
- during power on gpio is set to 0 so the reset pin is high, thus no reset
- during power off gpio is set to 1 so the reset pin becomes low, thus resetting

this is a possible solution but maybe there are other ways to achieve that, 
do you have suggestions on how to get a better approach for solving this issue?

> -- 
> With Best Regards,
> Andy Shevchenko
> 
> 

Best regards,
Jonathan Albrieux

Re: [PATCH 3/3] iio: magnetometer: ak8975: Add gpio reset support

[Andy Shevchenko]

On Mon, May 18, 2020 at 8:43 PM Jonathan Albrieux
<jonathan.albrieux@gmail.com> wrote:
>
> On Mon, May 18, 2020 at 07:43:17PM +0300, Andy Shevchenko wrote:
> > On Mon, May 18, 2020 at 06:01:20PM +0200, Jonathan Albrieux wrote:
> > > On Mon, May 18, 2020 at 05:55:51PM +0300, Andy Shevchenko wrote:
> > > > On Mon, May 18, 2020 at 4:38 PM Jonathan Albrieux
> > > > <jonathan.albrieux@gmail.com> wrote:
> > > >
> > > > > +       gpiod_set_value_cansleep(data->reset_gpiod, 1);
> > > >
> > > > (1)
> > > >
> > > > ...
> > > >
> > > > > +       /*
> > > > > +        * If reset pin is provided then will be set to high on power on
> > > > > +        * and to low on power off according to AK09911 datasheet
> > > > > +        */
> > > >
> > > > Wording is confusing, perhaps you have to use 'asserted / deasserted'.
> > >
> > > Thank you for the suggestion, I'll be working on rewording as soon as
> > > possible.
> > >
> > > > Btw, in (1) it's also "high" (asserted). I barely understand how it's
> > > > supposed to work in all cases?
> > > >
> > > > > +       reset_gpiod = devm_gpiod_get_optional(&client->dev,
> > > > > +                                             "reset", GPIOD_OUT_HIGH);
> > > > > +       if (IS_ERR(reset_gpiod))
> > > > > +               return PTR_ERR(reset_gpiod);
> > > >
> > >
> > > I'm sorry but I'm not sure about what you mean by saying all cases.
> > > Currently  I'm testing this driver on a msm8916 device having AK09911
> > > magnetometer. At the current stage the driver is failing on probe
> > > because reset pin is not connected to VID (as datasheet requires in case
> > > of pin not being used). In case of reset pin not asserted, register's
> > > reset is triggered resulting in empty registers, leading to probe fail.
> > > For this reason pin is asserted during power on in order to have
> > > informations in registers and deasserted before power off triggering
> > > a reset.
> > >
> > > A workaround that gets AK09911 working on device is by setting the
> > > reset pin always high on device tree. This way registers gets reset by
> > > a Power On Reset circuit autonomously and reset pin never triggers the
> > > reset.
> >
> > You need to distinguish electrical level from logical (GPIO flag defines
> > logical). So, I'm talking about active-high vs. active-low case.
> >
> > Now I re-read above, and see that here you assert the reset signal. But where
> > is desertion?
>
> Oh I see, I'll try explaining by points the proposed approach:
> - reset pin is active low
> - during power on gpio is set to 0 so the reset pin is high, thus no reset

deasserted

> - during power off gpio is set to 1 so the reset pin becomes low, thus resetting

asserted

> this is a possible solution but maybe there are other ways to achieve that,
> do you have suggestions on how to get a better approach for solving this issue?

I see now, that at requesting reset you wanted to chip be in reset
state (asserted) till driver calls power_on().

Seems everything you done is correct. Just correct terminology, please.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 3/3] iio: magnetometer: ak8975: Add gpio reset support

[Jonathan Albrieux]

On Mon, May 18, 2020 at 08:51:29PM +0300, Andy Shevchenko wrote:
> On Mon, May 18, 2020 at 8:43 PM Jonathan Albrieux
> <jonathan.albrieux@gmail.com> wrote:
> >
> > On Mon, May 18, 2020 at 07:43:17PM +0300, Andy Shevchenko wrote:
> > > On Mon, May 18, 2020 at 06:01:20PM +0200, Jonathan Albrieux wrote:
> > > > On Mon, May 18, 2020 at 05:55:51PM +0300, Andy Shevchenko wrote:
> > > > > On Mon, May 18, 2020 at 4:38 PM Jonathan Albrieux
> > > > > <jonathan.albrieux@gmail.com> wrote:
> > > > >
> > > > > > +       gpiod_set_value_cansleep(data->reset_gpiod, 1);
> > > > >
> > > > > (1)
> > > > >
> > > > > ...
> > > > >
> > > > > > +       /*
> > > > > > +        * If reset pin is provided then will be set to high on power on
> > > > > > +        * and to low on power off according to AK09911 datasheet
> > > > > > +        */
> > > > >
> > > > > Wording is confusing, perhaps you have to use 'asserted / deasserted'.
> > > >
> > > > Thank you for the suggestion, I'll be working on rewording as soon as
> > > > possible.
> > > >
> > > > > Btw, in (1) it's also "high" (asserted). I barely understand how it's
> > > > > supposed to work in all cases?
> > > > >
> > > > > > +       reset_gpiod = devm_gpiod_get_optional(&client->dev,
> > > > > > +                                             "reset", GPIOD_OUT_HIGH);
> > > > > > +       if (IS_ERR(reset_gpiod))
> > > > > > +               return PTR_ERR(reset_gpiod);
> > > > >
> > > >
> > > > I'm sorry but I'm not sure about what you mean by saying all cases.
> > > > Currently  I'm testing this driver on a msm8916 device having AK09911
> > > > magnetometer. At the current stage the driver is failing on probe
> > > > because reset pin is not connected to VID (as datasheet requires in case
> > > > of pin not being used). In case of reset pin not asserted, register's
> > > > reset is triggered resulting in empty registers, leading to probe fail.
> > > > For this reason pin is asserted during power on in order to have
> > > > informations in registers and deasserted before power off triggering
> > > > a reset.
> > > >
> > > > A workaround that gets AK09911 working on device is by setting the
> > > > reset pin always high on device tree. This way registers gets reset by
> > > > a Power On Reset circuit autonomously and reset pin never triggers the
> > > > reset.
> > >
> > > You need to distinguish electrical level from logical (GPIO flag defines
> > > logical). So, I'm talking about active-high vs. active-low case.
> > >
> > > Now I re-read above, and see that here you assert the reset signal. But where
> > > is desertion?
> >
> > Oh I see, I'll try explaining by points the proposed approach:
> > - reset pin is active low
> > - during power on gpio is set to 0 so the reset pin is high, thus no reset
> 
> deasserted
> 
> > - during power off gpio is set to 1 so the reset pin becomes low, thus resetting
> 
> asserted
> 
> > this is a possible solution but maybe there are other ways to achieve that,
> > do you have suggestions on how to get a better approach for solving this issue?
> 
> I see now, that at requesting reset you wanted to chip be in reset
> state (asserted) till driver calls power_on().
> 
> Seems everything you done is correct. Just correct terminology, please.

Will surely do, thank you!
 
> -- 
> With Best Regards,
> Andy Shevchenko

Best regards,
Jonathan Albrieux

Re: [PATCH 3/3] iio: magnetometer: ak8975: Add gpio reset support

[Pavel Machek]

Hi!

> AK09911 has a reset gpio to handle register's reset. If reset gpio is
> set to low it will trigger the reset. AK09911 datasheed says that if not
> used reset pin should be connected to VID and this patch emulates this
> situation
> 
> Signed-off-by: Jonathan Albrieux <jonathan.albrieux@gmail.com>
> ---
>  drivers/iio/magnetometer/ak8975.c | 21 +++++++++++++++++++--
>  1 file changed, 19 insertions(+), 2 deletions(-)
> 
>  	/*
> -	 * According to the datasheet the power supply rise time i 200us
> +	 * According to the datasheet the power supply rise time is 200us
>  	 * and the minimum wait time before mode setting is 100us, in
> -	 * total 300 us. Add some margin and say minimum 500us here.
> +	 * total 300us. Add some margin and say minimum 500us here.
>  	 */
>  	usleep_range(500, 1000);

I'd assume that datasheet added some safety margin too, and there's another one in usleep...
So I believe usleep..(300) should be okay..

Best regards,
										Pavel