| Message ID | 20200519124402.26076-5-jonathan.albrieux@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | iio: magnetometer: ak8975: Add gpio reset support | expand |
On Tue, May 19, 2020 at 02:43:54PM +0200, Jonathan Albrieux wrote: > According to AK09911 datasheet, if reset gpio is provided then > deassert reset on ak8975_power_on() and assert reset on ak8975_power_off(). > > Without reset's deassertion during ak8975_power_on(), driver's probe fails > on ak8975_who_i_am while() checking for device identity for AK09911 chip. Wrong position of (), but hold on, this is so minor, no need to send a new version because of this. > AK09911 has an active low reset gpio to handle register's reset. > AK09911 datasheed says that, if not used, reset pin should be connected > to VID. This patch emulates this situation. Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > Signed-off-by: Jonathan Albrieux <jonathan.albrieux@gmail.com> > --- > drivers/iio/magnetometer/ak8975.c | 18 ++++++++++++++++++ > 1 file changed, 18 insertions(+) > > diff --git a/drivers/iio/magnetometer/ak8975.c b/drivers/iio/magnetometer/ak8975.c > index fd368455cd7b..a23422aad97d 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,6 +385,9 @@ 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 is 200us > * and the minimum wait time before mode setting is 100us, in > @@ -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,16 @@ static int ak8975_probe(struct i2c_client *client, > if (eoc_gpiod) > gpiod_set_consumer_name(eoc_gpiod, "ak_8975"); > > + /* > + * According to AK09911 datasheet, if reset GPIO is provided then > + * deassert reset on ak8975_power_on() and assert reset on > + * ak8975_power_off(). > + */ > + 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 +883,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 >
On Tue, May 19, 2020 at 03:57:13PM +0300, Andy Shevchenko wrote: > On Tue, May 19, 2020 at 02:43:54PM +0200, Jonathan Albrieux wrote: > > According to AK09911 datasheet, if reset gpio is provided then > > deassert reset on ak8975_power_on() and assert reset on ak8975_power_off(). > > > > Without reset's deassertion during ak8975_power_on(), driver's probe fails > > on ak8975_who_i_am while() checking for device identity for AK09911 chip. > > Wrong position of (), but hold on, this is so minor, no need to send a new > version because of this. > Ops, if this represents a problem I can fix it, there's absolutely no problem! > > AK09911 has an active low reset gpio to handle register's reset. > > AK09911 datasheed says that, if not used, reset pin should be connected > > to VID. This patch emulates this situation. > > Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > > > Signed-off-by: Jonathan Albrieux <jonathan.albrieux@gmail.com> > > --- > > drivers/iio/magnetometer/ak8975.c | 18 ++++++++++++++++++ > > 1 file changed, 18 insertions(+) > > > > diff --git a/drivers/iio/magnetometer/ak8975.c b/drivers/iio/magnetometer/ak8975.c > > index fd368455cd7b..a23422aad97d 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,6 +385,9 @@ 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 is 200us > > * and the minimum wait time before mode setting is 100us, in > > @@ -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,16 @@ static int ak8975_probe(struct i2c_client *client, > > if (eoc_gpiod) > > gpiod_set_consumer_name(eoc_gpiod, "ak_8975"); > > > > + /* > > + * According to AK09911 datasheet, if reset GPIO is provided then > > + * deassert reset on ak8975_power_on() and assert reset on > > + * ak8975_power_off(). > > + */ > > + 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 +883,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 > > > > -- > With Best Regards, > Andy Shevchenko > > Best regards, Jonathan Albrieux
On Tue, May 19, 2020 at 02:43:54PM +0200, Jonathan Albrieux wrote: > According to AK09911 datasheet, if reset gpio is provided then > deassert reset on ak8975_power_on() and assert reset on ak8975_power_off(). > > Without reset's deassertion during ak8975_power_on(), driver's probe fails > on ak8975_who_i_am while() checking for device identity for AK09911 chip. > > AK09911 has an active low reset gpio to handle register's reset. > AK09911 datasheed says that, if not used, reset pin should be connected Another minor typo: datasheed -> datasheet In any case, FWIW: Reviewed-by: Stephan Gerhold <stephan@gerhold.net> > to VID. This patch emulates this situation. > > Signed-off-by: Jonathan Albrieux <jonathan.albrieux@gmail.com> > --- > drivers/iio/magnetometer/ak8975.c | 18 ++++++++++++++++++ > 1 file changed, 18 insertions(+) > > diff --git a/drivers/iio/magnetometer/ak8975.c b/drivers/iio/magnetometer/ak8975.c > index fd368455cd7b..a23422aad97d 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,6 +385,9 @@ 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 is 200us > * and the minimum wait time before mode setting is 100us, in > @@ -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,16 @@ static int ak8975_probe(struct i2c_client *client, > if (eoc_gpiod) > gpiod_set_consumer_name(eoc_gpiod, "ak_8975"); > > + /* > + * According to AK09911 datasheet, if reset GPIO is provided then > + * deassert reset on ak8975_power_on() and assert reset on > + * ak8975_power_off(). > + */ > + 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 +883,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
On Tue, May 19, 2020 at 03:25:12PM +0200, Stephan Gerhold wrote: > On Tue, May 19, 2020 at 02:43:54PM +0200, Jonathan Albrieux wrote: > > According to AK09911 datasheet, if reset gpio is provided then > > deassert reset on ak8975_power_on() and assert reset on ak8975_power_off(). > > > > Without reset's deassertion during ak8975_power_on(), driver's probe fails > > on ak8975_who_i_am while() checking for device identity for AK09911 chip. > > > > AK09911 has an active low reset gpio to handle register's reset. > > AK09911 datasheed says that, if not used, reset pin should be connected > > Another minor typo: datasheed -> datasheet > > In any case, FWIW: > Reviewed-by: Stephan Gerhold <stephan@gerhold.net> > > > to VID. This patch emulates this situation. > > > > Signed-off-by: Jonathan Albrieux <jonathan.albrieux@gmail.com> > > --- > > drivers/iio/magnetometer/ak8975.c | 18 ++++++++++++++++++ > > 1 file changed, 18 insertions(+) > > > > diff --git a/drivers/iio/magnetometer/ak8975.c b/drivers/iio/magnetometer/ak8975.c > > index fd368455cd7b..a23422aad97d 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,6 +385,9 @@ 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 is 200us > > * and the minimum wait time before mode setting is 100us, in > > @@ -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,16 @@ static int ak8975_probe(struct i2c_client *client, > > if (eoc_gpiod) > > gpiod_set_consumer_name(eoc_gpiod, "ak_8975"); > > > > + /* > > + * According to AK09911 datasheet, if reset GPIO is provided then > > + * deassert reset on ak8975_power_on() and assert reset on > > + * ak8975_power_off(). > > + */ > > + 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 +883,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 Thank you, will fix that typo too, Best regards, Jonathan Albrieux
diff --git a/drivers/iio/magnetometer/ak8975.c b/drivers/iio/magnetometer/ak8975.c index fd368455cd7b..a23422aad97d 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,6 +385,9 @@ 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 is 200us * and the minimum wait time before mode setting is 100us, in @@ -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,16 @@ static int ak8975_probe(struct i2c_client *client, if (eoc_gpiod) gpiod_set_consumer_name(eoc_gpiod, "ak_8975"); + /* + * According to AK09911 datasheet, if reset GPIO is provided then + * deassert reset on ak8975_power_on() and assert reset on + * ak8975_power_off(). + */ + 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 +883,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);
According to AK09911 datasheet, if reset gpio is provided then deassert reset on ak8975_power_on() and assert reset on ak8975_power_off(). Without reset's deassertion during ak8975_power_on(), driver's probe fails on ak8975_who_i_am while() checking for device identity for AK09911 chip. AK09911 has an active low reset gpio to handle register's reset. AK09911 datasheed says that, if not used, reset pin should be connected to VID. This patch emulates this situation. Signed-off-by: Jonathan Albrieux <jonathan.albrieux@gmail.com> --- drivers/iio/magnetometer/ak8975.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+)