Actually this is generic 'I have no GPIO' support for the AK8975, instead
we have to go bus polling.
Huang Liang produced an initial patch which worked by removing the support
for GPIO pins. This patch instead makes GPIO support optional and Huang then
fixed some bugs in it.
Signed-off-by: Alan Cox <[email protected]>
---
drivers/staging/iio/magnetometer/ak8975.c | 120 ++++++++++++++++++++---------
1 files changed, 83 insertions(+), 37 deletions(-)
diff --git a/drivers/staging/iio/magnetometer/ak8975.c b/drivers/staging/iio/magnetometer/ak8975.c
index 420f206..d71904a 100644
--- a/drivers/staging/iio/magnetometer/ak8975.c
+++ b/drivers/staging/iio/magnetometer/ak8975.c
@@ -206,7 +206,7 @@ static int ak8975_setup(struct i2c_client *client)
}
/* Precalculate scale factor for each axis and
- store in the device data. */
+ store in the device data. */
data->raw_to_gauss[0] = ((data->asa[0] + 128) * 30) >> 8;
data->raw_to_gauss[1] = ((data->asa[1] + 128) * 30) >> 8;
data->raw_to_gauss[2] = ((data->asa[2] + 128) * 30) >> 8;
@@ -316,6 +316,59 @@ static ssize_t show_scale(struct device *dev, struct device_attribute *devattr,
return sprintf(buf, "%ld\n", data->raw_to_gauss[this_attr->address]);
}
+static int wait_conversion_complete_gpio(struct ak8975_data *data)
+{
+ struct i2c_client *client = data->client;
+ u8 read_status;
+ u32 timeout_ms = AK8975_MAX_CONVERSION_TIMEOUT;
+ int ret;
+
+ /* Wait for the conversion to complete. */
+ while (timeout_ms) {
+ msleep(AK8975_CONVERSION_DONE_POLL_TIME);
+ if (gpio_get_value(data->eoc_gpio))
+ break;
+ timeout_ms -= AK8975_CONVERSION_DONE_POLL_TIME;
+ }
+ if (!timeout_ms) {
+ dev_err(&client->dev, "Conversion timeout happened\n");
+ return -EINVAL;
+ }
+
+ ret = ak8975_read_data(client, AK8975_REG_ST1, 1, &read_status);
+ if (ret < 0) {
+ dev_err(&client->dev, "Error in reading ST1\n");
+ return ret;
+ }
+ return read_status;
+}
+
+static int wait_conversion_complete_polled(struct ak8975_data *data)
+{
+ struct i2c_client *client = data->client;
+ u8 read_status;
+ u32 timeout_ms = AK8975_MAX_CONVERSION_TIMEOUT;
+ int ret;
+
+ /* Wait for the conversion to complete. */
+ while (timeout_ms) {
+ msleep(AK8975_CONVERSION_DONE_POLL_TIME);
+ ret = ak8975_read_data(client, AK8975_REG_ST1, 1, &read_status);
+ if (ret < 0) {
+ dev_err(&client->dev, "Error in reading ST1\n");
+ return ret;
+ }
+ if (read_status)
+ break;
+ timeout_ms -= AK8975_CONVERSION_DONE_POLL_TIME;
+ }
+ if (!timeout_ms) {
+ dev_err(&client->dev, "Conversion timeout happened\n");
+ return -EINVAL;
+ }
+ return read_status;
+}
+
/*
* Emits the raw flux value for the x, y, or z axis.
*/
@@ -326,7 +379,6 @@ static ssize_t show_raw(struct device *dev, struct device_attribute *devattr,
struct ak8975_data *data = indio_dev->dev_data;
struct i2c_client *client = data->client;
struct iio_dev_attr *this_attr = to_iio_dev_attr(devattr);
- u32 timeout_ms = AK8975_MAX_CONVERSION_TIMEOUT;
u16 meas_reg;
s16 raw;
u8 read_status;
@@ -352,23 +404,14 @@ static ssize_t show_raw(struct device *dev, struct device_attribute *devattr,
}
/* Wait for the conversion to complete. */
- while (timeout_ms) {
- msleep(AK8975_CONVERSION_DONE_POLL_TIME);
- if (gpio_get_value(data->eoc_gpio))
- break;
- timeout_ms -= AK8975_CONVERSION_DONE_POLL_TIME;
- }
- if (!timeout_ms) {
- dev_err(&client->dev, "Conversion timeout happened\n");
- ret = -EINVAL;
+ if (data->eoc_gpio)
+ ret = wait_conversion_complete_gpio(data);
+ else
+ ret = wait_conversion_complete_polled(data);
+ if (ret < 0)
goto exit;
- }
- ret = ak8975_read_data(client, AK8975_REG_ST1, 1, &read_status);
- if (ret < 0) {
- dev_err(&client->dev, "Error in reading ST1\n");
- goto exit;
- }
+ read_status = ret;
if (read_status & AK8975_REG_ST1_DRDY_MASK) {
ret = ak8975_read_data(client, AK8975_REG_ST2, 1, &read_status);
@@ -454,25 +497,26 @@ static int ak8975_probe(struct i2c_client *client,
data->eoc_irq = client->irq;
data->eoc_gpio = irq_to_gpio(client->irq);
- if (!data->eoc_gpio) {
- dev_err(&client->dev, "failed, no valid GPIO\n");
- err = -EINVAL;
- goto exit_free;
- }
-
- err = gpio_request(data->eoc_gpio, "ak_8975");
- if (err < 0) {
- dev_err(&client->dev, "failed to request GPIO %d, error %d\n",
- data->eoc_gpio, err);
- goto exit_free;
- }
+ /* We may not have a GPIO based IRQ to scan, that is fine, we will
+ poll if so */
+ if (data->eoc_gpio > 0) {
+ err = gpio_request(data->eoc_gpio, "ak_8975");
+ if (err < 0) {
+ dev_err(&client->dev,
+ "failed to request GPIO %d, error %d\n",
+ data->eoc_gpio, err);
+ goto exit_free;
+ }
- err = gpio_direction_input(data->eoc_gpio);
- if (err < 0) {
- dev_err(&client->dev, "Failed to configure input direction for"
- " GPIO %d, error %d\n", data->eoc_gpio, err);
- goto exit_gpio;
- }
+ err = gpio_direction_input(data->eoc_gpio);
+ if (err < 0) {
+ dev_err(&client->dev,
+ "Failed to configure input direction for GPIO %d, error %d\n",
+ data->eoc_gpio, err);
+ goto exit_gpio;
+ }
+ } else
+ data->eoc_gpio = 0; /* No GPIO available */
/* Perform some basic start-of-day setup of the device. */
err = ak8975_setup(client);
@@ -503,7 +547,8 @@ static int ak8975_probe(struct i2c_client *client,
exit_free_iio:
iio_free_device(data->indio_dev);
exit_gpio:
- gpio_free(data->eoc_gpio);
+ if (data->eoc_gpio)
+ gpio_free(data->eoc_gpio);
exit_free:
kfree(data);
exit:
@@ -517,7 +562,8 @@ static int ak8975_remove(struct i2c_client *client)
iio_device_unregister(data->indio_dev);
iio_free_device(data->indio_dev);
- gpio_free(data->eoc_gpio);
+ if (data->eoc_gpio)
+ gpio_free(data->eoc_gpio);
kfree(data);
On 04/06/11 13:31, Alan Cox wrote:
> Actually this is generic 'I have no GPIO' support for the AK8975, instead
> we have to go bus polling.
>
> Huang Liang produced an initial patch which worked by removing the support
> for GPIO pins. This patch instead makes GPIO support optional and Huang then
> fixed some bugs in it.
cc'd linux-iio
> Signed-off-by: Alan Cox <[email protected]>
> ---
>
> drivers/staging/iio/magnetometer/ak8975.c | 120 ++++++++++++++++++++---------
> 1 files changed, 83 insertions(+), 37 deletions(-)
>
> diff --git a/drivers/staging/iio/magnetometer/ak8975.c b/drivers/staging/iio/magnetometer/ak8975.c
> index 420f206..d71904a 100644
> --- a/drivers/staging/iio/magnetometer/ak8975.c
> +++ b/drivers/staging/iio/magnetometer/ak8975.c
> @@ -206,7 +206,7 @@ static int ak8975_setup(struct i2c_client *client)
> }
>
> /* Precalculate scale factor for each axis and
> - store in the device data. */
> + store in the device data. */
> data->raw_to_gauss[0] = ((data->asa[0] + 128) * 30) >> 8;
> data->raw_to_gauss[1] = ((data->asa[1] + 128) * 30) >> 8;
> data->raw_to_gauss[2] = ((data->asa[2] + 128) * 30) >> 8;
> @@ -316,6 +316,59 @@ static ssize_t show_scale(struct device *dev, struct device_attribute *devattr,
> return sprintf(buf, "%ld\n", data->raw_to_gauss[this_attr->address]);
> }
>
> +static int wait_conversion_complete_gpio(struct ak8975_data *data)
> +{
> + struct i2c_client *client = data->client;
> + u8 read_status;
> + u32 timeout_ms = AK8975_MAX_CONVERSION_TIMEOUT;
> + int ret;
> +
> + /* Wait for the conversion to complete. */
> + while (timeout_ms) {
> + msleep(AK8975_CONVERSION_DONE_POLL_TIME);
> + if (gpio_get_value(data->eoc_gpio))
> + break;
> + timeout_ms -= AK8975_CONVERSION_DONE_POLL_TIME;
> + }
> + if (!timeout_ms) {
> + dev_err(&client->dev, "Conversion timeout happened\n");
> + return -EINVAL;
> + }
> +
> + ret = ak8975_read_data(client, AK8975_REG_ST1, 1, &read_status);
> + if (ret < 0) {
> + dev_err(&client->dev, "Error in reading ST1\n");
> + return ret;
> + }
> + return read_status;
> +}
> +
> +static int wait_conversion_complete_polled(struct ak8975_data *data)
> +{
> + struct i2c_client *client = data->client;
> + u8 read_status;
> + u32 timeout_ms = AK8975_MAX_CONVERSION_TIMEOUT;
> + int ret;
> +
> + /* Wait for the conversion to complete. */
> + while (timeout_ms) {
> + msleep(AK8975_CONVERSION_DONE_POLL_TIME);
> + ret = ak8975_read_data(client, AK8975_REG_ST1, 1, &read_status);
> + if (ret < 0) {
> + dev_err(&client->dev, "Error in reading ST1\n");
> + return ret;
> + }
> + if (read_status)
> + break;
> + timeout_ms -= AK8975_CONVERSION_DONE_POLL_TIME;
> + }
> + if (!timeout_ms) {
> + dev_err(&client->dev, "Conversion timeout happened\n");
> + return -EINVAL;
> + }
> + return read_status;
> +}
> +
> /*
> * Emits the raw flux value for the x, y, or z axis.
> */
> @@ -326,7 +379,6 @@ static ssize_t show_raw(struct device *dev, struct device_attribute *devattr,
> struct ak8975_data *data = indio_dev->dev_data;
> struct i2c_client *client = data->client;
> struct iio_dev_attr *this_attr = to_iio_dev_attr(devattr);
> - u32 timeout_ms = AK8975_MAX_CONVERSION_TIMEOUT;
> u16 meas_reg;
> s16 raw;
> u8 read_status;
> @@ -352,23 +404,14 @@ static ssize_t show_raw(struct device *dev, struct device_attribute *devattr,
> }
>
> /* Wait for the conversion to complete. */
> - while (timeout_ms) {
> - msleep(AK8975_CONVERSION_DONE_POLL_TIME);
> - if (gpio_get_value(data->eoc_gpio))
> - break;
> - timeout_ms -= AK8975_CONVERSION_DONE_POLL_TIME;
> - }
> - if (!timeout_ms) {
> - dev_err(&client->dev, "Conversion timeout happened\n");
> - ret = -EINVAL;
> + if (data->eoc_gpio)
> + ret = wait_conversion_complete_gpio(data);
> + else
> + ret = wait_conversion_complete_polled(data);
> + if (ret < 0)
> goto exit;
> - }
>
> - ret = ak8975_read_data(client, AK8975_REG_ST1, 1, &read_status);
> - if (ret < 0) {
> - dev_err(&client->dev, "Error in reading ST1\n");
> - goto exit;
> - }
> + read_status = ret;
>
> if (read_status & AK8975_REG_ST1_DRDY_MASK) {
> ret = ak8975_read_data(client, AK8975_REG_ST2, 1, &read_status);
> @@ -454,25 +497,26 @@ static int ak8975_probe(struct i2c_client *client,
> data->eoc_irq = client->irq;
> data->eoc_gpio = irq_to_gpio(client->irq);
>
> - if (!data->eoc_gpio) {
> - dev_err(&client->dev, "failed, no valid GPIO\n");
> - err = -EINVAL;
> - goto exit_free;
> - }
> -
> - err = gpio_request(data->eoc_gpio, "ak_8975");
> - if (err < 0) {
> - dev_err(&client->dev, "failed to request GPIO %d, error %d\n",
> - data->eoc_gpio, err);
> - goto exit_free;
> - }
> + /* We may not have a GPIO based IRQ to scan, that is fine, we will
> + poll if so */
> + if (data->eoc_gpio > 0) {
> + err = gpio_request(data->eoc_gpio, "ak_8975");
> + if (err < 0) {
> + dev_err(&client->dev,
> + "failed to request GPIO %d, error %d\n",
> + data->eoc_gpio, err);
> + goto exit_free;
> + }
>
> - err = gpio_direction_input(data->eoc_gpio);
> - if (err < 0) {
> - dev_err(&client->dev, "Failed to configure input direction for"
> - " GPIO %d, error %d\n", data->eoc_gpio, err);
> - goto exit_gpio;
> - }
> + err = gpio_direction_input(data->eoc_gpio);
> + if (err < 0) {
> + dev_err(&client->dev,
> + "Failed to configure input direction for GPIO %d, error %d\n",
> + data->eoc_gpio, err);
> + goto exit_gpio;
> + }
> + } else
> + data->eoc_gpio = 0; /* No GPIO available */
>
> /* Perform some basic start-of-day setup of the device. */
> err = ak8975_setup(client);
> @@ -503,7 +547,8 @@ static int ak8975_probe(struct i2c_client *client,
> exit_free_iio:
> iio_free_device(data->indio_dev);
> exit_gpio:
> - gpio_free(data->eoc_gpio);
> + if (data->eoc_gpio)
> + gpio_free(data->eoc_gpio);
> exit_free:
> kfree(data);
> exit:
> @@ -517,7 +562,8 @@ static int ak8975_remove(struct i2c_client *client)
> iio_device_unregister(data->indio_dev);
> iio_free_device(data->indio_dev);
>
> - gpio_free(data->eoc_gpio);
> + if (data->eoc_gpio)
> + gpio_free(data->eoc_gpio);
>
> kfree(data);
>
>
On 04/06/11 14:39, Jonathan Cameron wrote:
> On 04/06/11 13:31, Alan Cox wrote:
>> Actually this is generic 'I have no GPIO' support for the AK8975, instead
>> we have to go bus polling.
Can we rename the patch to reflect that?
>>
>> Huang Liang produced an initial patch which worked by removing the support
>> for GPIO pins. This patch instead makes GPIO support optional and Huang then
>> fixed some bugs in it.
Patch is fine, but could perhaps be a little shorter and tidier as suggested
below.
>
> cc'd linux-iio
>> Signed-off-by: Alan Cox <[email protected]>
>> ---
>>
>> drivers/staging/iio/magnetometer/ak8975.c | 120 ++++++++++++++++++++---------
>> 1 files changed, 83 insertions(+), 37 deletions(-)
>>
>> diff --git a/drivers/staging/iio/magnetometer/ak8975.c b/drivers/staging/iio/magnetometer/ak8975.c
>> index 420f206..d71904a 100644
>> --- a/drivers/staging/iio/magnetometer/ak8975.c
>> +++ b/drivers/staging/iio/magnetometer/ak8975.c
>> @@ -206,7 +206,7 @@ static int ak8975_setup(struct i2c_client *client)
>> }
>>
>> /* Precalculate scale factor for each axis and
>> - store in the device data. */
>> + store in the device data. */
>> data->raw_to_gauss[0] = ((data->asa[0] + 128) * 30) >> 8;
>> data->raw_to_gauss[1] = ((data->asa[1] + 128) * 30) >> 8;
>> data->raw_to_gauss[2] = ((data->asa[2] + 128) * 30) >> 8;
>> @@ -316,6 +316,59 @@ static ssize_t show_scale(struct device *dev, struct device_attribute *devattr,
>> return sprintf(buf, "%ld\n", data->raw_to_gauss[this_attr->address]);
>> }
>>
>> +static int wait_conversion_complete_gpio(struct ak8975_data *data)
>> +{
>> + struct i2c_client *client = data->client;
>> + u8 read_status;
>> + u32 timeout_ms = AK8975_MAX_CONVERSION_TIMEOUT;
>> + int ret;
>> +
>> + /* Wait for the conversion to complete. */
>> + while (timeout_ms) {
>> + msleep(AK8975_CONVERSION_DONE_POLL_TIME);
>> + if (gpio_get_value(data->eoc_gpio))
>> + break;
>> + timeout_ms -= AK8975_CONVERSION_DONE_POLL_TIME;
>> + }
>> + if (!timeout_ms) {
>> + dev_err(&client->dev, "Conversion timeout happened\n");
>> + return -EINVAL;
>> + }
>> +
>> + ret = ak8975_read_data(client, AK8975_REG_ST1, 1, &read_status);
>> + if (ret < 0) {
>> + dev_err(&client->dev, "Error in reading ST1\n");
>> + return ret;
>> + }
>> + return read_status;
>> +}
>> +
Its not immediately obvious to me why we need these two separate functions.
They only differ by a couple of lines. Perhaps push querying if the gpio
down into a single function?
>> +static int wait_conversion_complete_polled(struct ak8975_data *data)
>> +{
>> + struct i2c_client *client = data->client;
>> + u8 read_status;
>> + u32 timeout_ms = AK8975_MAX_CONVERSION_TIMEOUT;
>> + int ret;
>> +
>> + /* Wait for the conversion to complete. */
>> + while (timeout_ms) {
>> + msleep(AK8975_CONVERSION_DONE_POLL_TIME);
>> + ret = ak8975_read_data(client, AK8975_REG_ST1, 1, &read_status);
>> + if (ret < 0) {
>> + dev_err(&client->dev, "Error in reading ST1\n");
>> + return ret;
>> + }
>> + if (read_status)
>> + break;
>> + timeout_ms -= AK8975_CONVERSION_DONE_POLL_TIME;
>> + }
>> + if (!timeout_ms) {
>> + dev_err(&client->dev, "Conversion timeout happened\n");
>> + return -EINVAL;
>> + }
>> + return read_status;
>> +}
>> +
>> /*
>> * Emits the raw flux value for the x, y, or z axis.
>> */
>> @@ -326,7 +379,6 @@ static ssize_t show_raw(struct device *dev, struct device_attribute *devattr,
>> struct ak8975_data *data = indio_dev->dev_data;
>> struct i2c_client *client = data->client;
>> struct iio_dev_attr *this_attr = to_iio_dev_attr(devattr);
>> - u32 timeout_ms = AK8975_MAX_CONVERSION_TIMEOUT;
>> u16 meas_reg;
>> s16 raw;
>> u8 read_status;
>> @@ -352,23 +404,14 @@ static ssize_t show_raw(struct device *dev, struct device_attribute *devattr,
>> }
>>
>> /* Wait for the conversion to complete. */
>> - while (timeout_ms) {
>> - msleep(AK8975_CONVERSION_DONE_POLL_TIME);
>> - if (gpio_get_value(data->eoc_gpio))
>> - break;
>> - timeout_ms -= AK8975_CONVERSION_DONE_POLL_TIME;
>> - }
>> - if (!timeout_ms) {
>> - dev_err(&client->dev, "Conversion timeout happened\n");
>> - ret = -EINVAL;
>> + if (data->eoc_gpio)
>> + ret = wait_conversion_complete_gpio(data);
>> + else
>> + ret = wait_conversion_complete_polled(data);
>> + if (ret < 0)
>> goto exit;
>> - }
>>
>> - ret = ak8975_read_data(client, AK8975_REG_ST1, 1, &read_status);
>> - if (ret < 0) {
>> - dev_err(&client->dev, "Error in reading ST1\n");
>> - goto exit;
>> - }
>> + read_status = ret;
>>
>> if (read_status & AK8975_REG_ST1_DRDY_MASK) {
>> ret = ak8975_read_data(client, AK8975_REG_ST2, 1, &read_status);
>> @@ -454,25 +497,26 @@ static int ak8975_probe(struct i2c_client *client,
>> data->eoc_irq = client->irq;
>> data->eoc_gpio = irq_to_gpio(client->irq);
>>
>> - if (!data->eoc_gpio) {
>> - dev_err(&client->dev, "failed, no valid GPIO\n");
>> - err = -EINVAL;
>> - goto exit_free;
>> - }
>> -
>> - err = gpio_request(data->eoc_gpio, "ak_8975");
>> - if (err < 0) {
>> - dev_err(&client->dev, "failed to request GPIO %d, error %d\n",
>> - data->eoc_gpio, err);
>> - goto exit_free;
>> - }
>> + /* We may not have a GPIO based IRQ to scan, that is fine, we will
>> + poll if so */
>> + if (data->eoc_gpio > 0) {
>> + err = gpio_request(data->eoc_gpio, "ak_8975");
>> + if (err < 0) {
>> + dev_err(&client->dev,
>> + "failed to request GPIO %d, error %d\n",
>> + data->eoc_gpio, err);
>> + goto exit_free;
>> + }
>>
>> - err = gpio_direction_input(data->eoc_gpio);
>> - if (err < 0) {
>> - dev_err(&client->dev, "Failed to configure input direction for"
>> - " GPIO %d, error %d\n", data->eoc_gpio, err);
>> - goto exit_gpio;
>> - }
>> + err = gpio_direction_input(data->eoc_gpio);
>> + if (err < 0) {
>> + dev_err(&client->dev,
>> + "Failed to configure input direction for GPIO %d, error %d\n",
>> + data->eoc_gpio, err);
>> + goto exit_gpio;
Not really relevant to this patch, but this handling could be cleaned up using gpio_request_one.
>> + }
>> + } else
>> + data->eoc_gpio = 0; /* No GPIO available */
>>
>> /* Perform some basic start-of-day setup of the device. */
>> err = ak8975_setup(client);
>> @@ -503,7 +547,8 @@ static int ak8975_probe(struct i2c_client *client,
>> exit_free_iio:
>> iio_free_device(data->indio_dev);
>> exit_gpio:
>> - gpio_free(data->eoc_gpio);
>> + if (data->eoc_gpio)
>> + gpio_free(data->eoc_gpio);
>> exit_free:
>> kfree(data);
>> exit:
>> @@ -517,7 +562,8 @@ static int ak8975_remove(struct i2c_client *client)
>> iio_device_unregister(data->indio_dev);
>> iio_free_device(data->indio_dev);
>>
>> - gpio_free(data->eoc_gpio);
>> + if (data->eoc_gpio)
>> + gpio_free(data->eoc_gpio);
>>
>> kfree(data);
>>
>>
>
> --
> 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
>
> >> +static int wait_conversion_complete_gpio(struct ak8975_data *data)
> >> +{
> >> + struct i2c_client *client = data->client;
> >> + u8 read_status;
> >> + u32 timeout_ms = AK8975_MAX_CONVERSION_TIMEOUT;
> >> + int ret;
> >> +
> >> + /* Wait for the conversion to complete. */
> >> + while (timeout_ms) {
> >> + msleep(AK8975_CONVERSION_DONE_POLL_TIME);
> >> + if (gpio_get_value(data->eoc_gpio))
> >> + break;
> >> + timeout_ms -= AK8975_CONVERSION_DONE_POLL_TIME;
> >> + }
> >> + if (!timeout_ms) {
> >> + dev_err(&client->dev, "Conversion timeout happened\n");
> >> + return -EINVAL;
> >> + }
> >> +
> >> + ret = ak8975_read_data(client, AK8975_REG_ST1, 1, &read_status);
> >> + if (ret < 0) {
> >> + dev_err(&client->dev, "Error in reading ST1\n");
> >> + return ret;
> >> + }
> >> + return read_status;
> >> +}
> >> +
> Its not immediately obvious to me why we need these two separate functions.
> They only differ by a couple of lines. Perhaps push querying if the gpio
> down into a single function?
That was actually my starting point but they are quite different - one
does the gpio check each loop then a read, the other does a read each
loop and then nothing. Mixed together you get
while() {
if
if
else
if
}
if
else
and it looks like spagetti ..
> >> +static int wait_conversion_complete_polled(struct ak8975_data *data)
> >> +{
> >> + struct i2c_client *client = data->client;
> >> + u8 read_status;
> >> + u32 timeout_ms = AK8975_MAX_CONVERSION_TIMEOUT;
> >> + int ret;
> >> +
> >> + /* Wait for the conversion to complete. */
> >> + while (timeout_ms) {
> >> + msleep(AK8975_CONVERSION_DONE_POLL_TIME);
> >> + ret = ak8975_read_data(client, AK8975_REG_ST1, 1, &read_status);
> >> + if (ret < 0) {
> >> + dev_err(&client->dev, "Error in reading ST1\n");
> >> + return ret;
> >> + }
> >> + if (read_status)
> >> + break;
> >> + timeout_ms -= AK8975_CONVERSION_DONE_POLL_TIME;
> >> + }
> >> + if (!timeout_ms) {
> >> + dev_err(&client->dev, "Conversion timeout happened\n");
> >> + return -EINVAL;
> >> + }
> >> + return read_status;
> >> +}
> >> +
> >> /*
> >> * Emits the raw flux value for the x, y, or z axis.
> >> */
> >> @@ -326,7 +379,6 @@ static ssize_t show_raw(struct device *dev, struct device_attribute *devattr,
> >> struct ak8975_data *data = indio_dev->dev_data;
> >> struct i2c_client *client = data->client;
> >> struct iio_dev_attr *this_attr = to_iio_dev_attr(devattr);
> >> - u32 timeout_ms = AK8975_MAX_CONVERSION_TIMEOUT;
> >> u16 meas_reg;
> >> s16 raw;
> >> u8 read_status;
> >> @@ -352,23 +404,14 @@ static ssize_t show_raw(struct device *dev, struct device_attribute *devattr,
> >> }
> >>
> >> /* Wait for the conversion to complete. */
> >> - while (timeout_ms) {
> >> - msleep(AK8975_CONVERSION_DONE_POLL_TIME);
> >> - if (gpio_get_value(data->eoc_gpio))
> >> - break;
> >> - timeout_ms -= AK8975_CONVERSION_DONE_POLL_TIME;
> >> - }
> >> - if (!timeout_ms) {
> >> - dev_err(&client->dev, "Conversion timeout happened\n");
> >> - ret = -EINVAL;
> >> + if (data->eoc_gpio)
> >> + ret = wait_conversion_complete_gpio(data);
> >> + else
> >> + ret = wait_conversion_complete_polled(data);
> >> + if (ret < 0)
> >> goto exit;
> >> - }
> >>
> >> - ret = ak8975_read_data(client, AK8975_REG_ST1, 1, &read_status);
> >> - if (ret < 0) {
> >> - dev_err(&client->dev, "Error in reading ST1\n");
> >> - goto exit;
> >> - }
> >> + read_status = ret;
> >>
> >> if (read_status & AK8975_REG_ST1_DRDY_MASK) {
> >> ret = ak8975_read_data(client, AK8975_REG_ST2, 1, &read_status);
> >> @@ -454,25 +497,26 @@ static int ak8975_probe(struct i2c_client *client,
> >> data->eoc_irq = client->irq;
> >> data->eoc_gpio = irq_to_gpio(client->irq);
> >>
> >> - if (!data->eoc_gpio) {
> >> - dev_err(&client->dev, "failed, no valid GPIO\n");
> >> - err = -EINVAL;
> >> - goto exit_free;
> >> - }
> >> -
> >> - err = gpio_request(data->eoc_gpio, "ak_8975");
> >> - if (err < 0) {
> >> - dev_err(&client->dev, "failed to request GPIO %d, error %d\n",
> >> - data->eoc_gpio, err);
> >> - goto exit_free;
> >> - }
> >> + /* We may not have a GPIO based IRQ to scan, that is fine, we will
> >> + poll if so */
> >> + if (data->eoc_gpio > 0) {
> >> + err = gpio_request(data->eoc_gpio, "ak_8975");
> >> + if (err < 0) {
> >> + dev_err(&client->dev,
> >> + "failed to request GPIO %d, error %d\n",
> >> + data->eoc_gpio, err);
> >> + goto exit_free;
> >> + }
> >>
> >> - err = gpio_direction_input(data->eoc_gpio);
> >> - if (err < 0) {
> >> - dev_err(&client->dev, "Failed to configure input direction for"
> >> - " GPIO %d, error %d\n", data->eoc_gpio, err);
> >> - goto exit_gpio;
> >> - }
> >> + err = gpio_direction_input(data->eoc_gpio);
> >> + if (err < 0) {
> >> + dev_err(&client->dev,
> >> + "Failed to configure input direction for GPIO %d, error %d\n",
> >> + data->eoc_gpio, err);
> >> + goto exit_gpio;
> Not really relevant to this patch, but this handling could be cleaned up using gpio_request_one.
> >> + }
> >> + } else
> >> + data->eoc_gpio = 0; /* No GPIO available */
> >>
> >> /* Perform some basic start-of-day setup of the device. */
> >> err = ak8975_setup(client);
> >> @@ -503,7 +547,8 @@ static int ak8975_probe(struct i2c_client *client,
> >> exit_free_iio:
> >> iio_free_device(data->indio_dev);
> >> exit_gpio:
> >> - gpio_free(data->eoc_gpio);
> >> + if (data->eoc_gpio)
> >> + gpio_free(data->eoc_gpio);
> >> exit_free:
> >> kfree(data);
> >> exit:
> >> @@ -517,7 +562,8 @@ static int ak8975_remove(struct i2c_client *client)
> >> iio_device_unregister(data->indio_dev);
> >> iio_free_device(data->indio_dev);
> >>
> >> - gpio_free(data->eoc_gpio);
> >> + if (data->eoc_gpio)
> >> + gpio_free(data->eoc_gpio);
> >>
> >> kfree(data);
> >>
> >>
> >
> > --
> > 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
> >
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
--
--
"Alan, I'm getting a bit worried about you."
-- Linus Torvalds
On 04/06/11 15:02, Alan Cox wrote:
>>>> +static int wait_conversion_complete_gpio(struct ak8975_data *data)
>>>> +{
>>>> + struct i2c_client *client = data->client;
>>>> + u8 read_status;
>>>> + u32 timeout_ms = AK8975_MAX_CONVERSION_TIMEOUT;
>>>> + int ret;
>>>> +
>>>> + /* Wait for the conversion to complete. */
>>>> + while (timeout_ms) {
>>>> + msleep(AK8975_CONVERSION_DONE_POLL_TIME);
>>>> + if (gpio_get_value(data->eoc_gpio))
>>>> + break;
>>>> + timeout_ms -= AK8975_CONVERSION_DONE_POLL_TIME;
>>>> + }
>>>> + if (!timeout_ms) {
>>>> + dev_err(&client->dev, "Conversion timeout happened\n");
>>>> + return -EINVAL;
>>>> + }
>>>> +
>>>> + ret = ak8975_read_data(client, AK8975_REG_ST1, 1, &read_status);
>>>> + if (ret < 0) {
>>>> + dev_err(&client->dev, "Error in reading ST1\n");
>>>> + return ret;
>>>> + }
>>>> + return read_status;
>>>> +}
>>>> +
>> Its not immediately obvious to me why we need these two separate functions.
>> They only differ by a couple of lines. Perhaps push querying if the gpio
>> down into a single function?
>
> That was actually my starting point but they are quite different - one
> does the gpio check each loop then a read, the other does a read each
> loop and then nothing. Mixed together you get
>
> while() {
> if
> if
> else
> if
>
> }
> if
>
> else
>
> and it looks like spagetti ..
Good point. I clearly didn't look closely enough.
Acked-by: Jonathan Cameron <[email protected]>
>
>
>
>
>>>> +static int wait_conversion_complete_polled(struct ak8975_data *data)
>>>> +{
>>>> + struct i2c_client *client = data->client;
>>>> + u8 read_status;
>>>> + u32 timeout_ms = AK8975_MAX_CONVERSION_TIMEOUT;
>>>> + int ret;
>>>> +
>>>> + /* Wait for the conversion to complete. */
>>>> + while (timeout_ms) {
>>>> + msleep(AK8975_CONVERSION_DONE_POLL_TIME);
>>>> + ret = ak8975_read_data(client, AK8975_REG_ST1, 1, &read_status);
>>>> + if (ret < 0) {
>>>> + dev_err(&client->dev, "Error in reading ST1\n");
>>>> + return ret;
>>>> + }
>>>> + if (read_status)
>>>> + break;
>>>> + timeout_ms -= AK8975_CONVERSION_DONE_POLL_TIME;
>>>> + }
>>>> + if (!timeout_ms) {
>>>> + dev_err(&client->dev, "Conversion timeout happened\n");
>>>> + return -EINVAL;
>>>> + }
>>>> + return read_status;
>>>> +}
>>>> +
>>>> /*
>>>> * Emits the raw flux value for the x, y, or z axis.
>>>> */
>>>> @@ -326,7 +379,6 @@ static ssize_t show_raw(struct device *dev, struct device_attribute *devattr,
>>>> struct ak8975_data *data = indio_dev->dev_data;
>>>> struct i2c_client *client = data->client;
>>>> struct iio_dev_attr *this_attr = to_iio_dev_attr(devattr);
>>>> - u32 timeout_ms = AK8975_MAX_CONVERSION_TIMEOUT;
>>>> u16 meas_reg;
>>>> s16 raw;
>>>> u8 read_status;
>>>> @@ -352,23 +404,14 @@ static ssize_t show_raw(struct device *dev, struct device_attribute *devattr,
>>>> }
>>>>
>>>> /* Wait for the conversion to complete. */
>>>> - while (timeout_ms) {
>>>> - msleep(AK8975_CONVERSION_DONE_POLL_TIME);
>>>> - if (gpio_get_value(data->eoc_gpio))
>>>> - break;
>>>> - timeout_ms -= AK8975_CONVERSION_DONE_POLL_TIME;
>>>> - }
>>>> - if (!timeout_ms) {
>>>> - dev_err(&client->dev, "Conversion timeout happened\n");
>>>> - ret = -EINVAL;
>>>> + if (data->eoc_gpio)
>>>> + ret = wait_conversion_complete_gpio(data);
>>>> + else
>>>> + ret = wait_conversion_complete_polled(data);
>>>> + if (ret < 0)
>>>> goto exit;
>>>> - }
>>>>
>>>> - ret = ak8975_read_data(client, AK8975_REG_ST1, 1, &read_status);
>>>> - if (ret < 0) {
>>>> - dev_err(&client->dev, "Error in reading ST1\n");
>>>> - goto exit;
>>>> - }
>>>> + read_status = ret;
>>>>
>>>> if (read_status & AK8975_REG_ST1_DRDY_MASK) {
>>>> ret = ak8975_read_data(client, AK8975_REG_ST2, 1, &read_status);
>>>> @@ -454,25 +497,26 @@ static int ak8975_probe(struct i2c_client *client,
>>>> data->eoc_irq = client->irq;
>>>> data->eoc_gpio = irq_to_gpio(client->irq);
>>>>
>>>> - if (!data->eoc_gpio) {
>>>> - dev_err(&client->dev, "failed, no valid GPIO\n");
>>>> - err = -EINVAL;
>>>> - goto exit_free;
>>>> - }
>>>> -
>>>> - err = gpio_request(data->eoc_gpio, "ak_8975");
>>>> - if (err < 0) {
>>>> - dev_err(&client->dev, "failed to request GPIO %d, error %d\n",
>>>> - data->eoc_gpio, err);
>>>> - goto exit_free;
>>>> - }
>>>> + /* We may not have a GPIO based IRQ to scan, that is fine, we will
>>>> + poll if so */
>>>> + if (data->eoc_gpio > 0) {
>>>> + err = gpio_request(data->eoc_gpio, "ak_8975");
>>>> + if (err < 0) {
>>>> + dev_err(&client->dev,
>>>> + "failed to request GPIO %d, error %d\n",
>>>> + data->eoc_gpio, err);
>>>> + goto exit_free;
>>>> + }
>>>>
>>>> - err = gpio_direction_input(data->eoc_gpio);
>>>> - if (err < 0) {
>>>> - dev_err(&client->dev, "Failed to configure input direction for"
>>>> - " GPIO %d, error %d\n", data->eoc_gpio, err);
>>>> - goto exit_gpio;
>>>> - }
>>>> + err = gpio_direction_input(data->eoc_gpio);
>>>> + if (err < 0) {
>>>> + dev_err(&client->dev,
>>>> + "Failed to configure input direction for GPIO %d, error %d\n",
>>>> + data->eoc_gpio, err);
>>>> + goto exit_gpio;
>> Not really relevant to this patch, but this handling could be cleaned up using gpio_request_one.
>>>> + }
>>>> + } else
>>>> + data->eoc_gpio = 0; /* No GPIO available */
>>>>
>>>> /* Perform some basic start-of-day setup of the device. */
>>>> err = ak8975_setup(client);
>>>> @@ -503,7 +547,8 @@ static int ak8975_probe(struct i2c_client *client,
>>>> exit_free_iio:
>>>> iio_free_device(data->indio_dev);
>>>> exit_gpio:
>>>> - gpio_free(data->eoc_gpio);
>>>> + if (data->eoc_gpio)
>>>> + gpio_free(data->eoc_gpio);
>>>> exit_free:
>>>> kfree(data);
>>>> exit:
>>>> @@ -517,7 +562,8 @@ static int ak8975_remove(struct i2c_client *client)
>>>> iio_device_unregister(data->indio_dev);
>>>> iio_free_device(data->indio_dev);
>>>>
>>>> - gpio_free(data->eoc_gpio);
>>>> + if (data->eoc_gpio)
>>>> + gpio_free(data->eoc_gpio);
>>>>
>>>> kfree(data);
>>>>
>>>>
>>>
>>> --
>>> 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
>>>
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>> the body of a message to [email protected]
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>> Please read the FAQ at http://www.tux.org/lkml/
>
>
On 04/06/11 13:31, Alan Cox wrote:
> Actually this is generic 'I have no GPIO' support for the AK8975, instead
> we have to go bus polling.
>
> Huang Liang produced an initial patch which worked by removing the support
> for GPIO pins. This patch instead makes GPIO support optional and Huang then
> fixed some bugs in it.
I think this is just a resend in which case I'll add my ack again (it was
buried deep in the thread last time.)
>
> Signed-off-by: Alan Cox <[email protected]>
Acked-by: Jonathan Cameron <[email protected]>
> >
> ---
>
> drivers/staging/iio/magnetometer/ak8975.c | 120 ++++++++++++++++++++---------
> 1 files changed, 83 insertions(+), 37 deletions(-)
>
> diff --git a/drivers/staging/iio/magnetometer/ak8975.c b/drivers/staging/iio/magnetometer/ak8975.c
> index 420f206..d71904a 100644
> --- a/drivers/staging/iio/magnetometer/ak8975.c
> +++ b/drivers/staging/iio/magnetometer/ak8975.c
> @@ -206,7 +206,7 @@ static int ak8975_setup(struct i2c_client *client)
> }
>
> /* Precalculate scale factor for each axis and
> - store in the device data. */
> + store in the device data. */
> data->raw_to_gauss[0] = ((data->asa[0] + 128) * 30) >> 8;
> data->raw_to_gauss[1] = ((data->asa[1] + 128) * 30) >> 8;
> data->raw_to_gauss[2] = ((data->asa[2] + 128) * 30) >> 8;
> @@ -316,6 +316,59 @@ static ssize_t show_scale(struct device *dev, struct device_attribute *devattr,
> return sprintf(buf, "%ld\n", data->raw_to_gauss[this_attr->address]);
> }
>
> +static int wait_conversion_complete_gpio(struct ak8975_data *data)
> +{
> + struct i2c_client *client = data->client;
> + u8 read_status;
> + u32 timeout_ms = AK8975_MAX_CONVERSION_TIMEOUT;
> + int ret;
> +
> + /* Wait for the conversion to complete. */
> + while (timeout_ms) {
> + msleep(AK8975_CONVERSION_DONE_POLL_TIME);
> + if (gpio_get_value(data->eoc_gpio))
> + break;
> + timeout_ms -= AK8975_CONVERSION_DONE_POLL_TIME;
> + }
> + if (!timeout_ms) {
> + dev_err(&client->dev, "Conversion timeout happened\n");
> + return -EINVAL;
> + }
> +
> + ret = ak8975_read_data(client, AK8975_REG_ST1, 1, &read_status);
> + if (ret < 0) {
> + dev_err(&client->dev, "Error in reading ST1\n");
> + return ret;
> + }
> + return read_status;
> +}
> +
> +static int wait_conversion_complete_polled(struct ak8975_data *data)
> +{
> + struct i2c_client *client = data->client;
> + u8 read_status;
> + u32 timeout_ms = AK8975_MAX_CONVERSION_TIMEOUT;
> + int ret;
> +
> + /* Wait for the conversion to complete. */
> + while (timeout_ms) {
> + msleep(AK8975_CONVERSION_DONE_POLL_TIME);
> + ret = ak8975_read_data(client, AK8975_REG_ST1, 1, &read_status);
> + if (ret < 0) {
> + dev_err(&client->dev, "Error in reading ST1\n");
> + return ret;
> + }
> + if (read_status)
> + break;
> + timeout_ms -= AK8975_CONVERSION_DONE_POLL_TIME;
> + }
> + if (!timeout_ms) {
> + dev_err(&client->dev, "Conversion timeout happened\n");
> + return -EINVAL;
> + }
> + return read_status;
> +}
> +
> /*
> * Emits the raw flux value for the x, y, or z axis.
> */
> @@ -326,7 +379,6 @@ static ssize_t show_raw(struct device *dev, struct device_attribute *devattr,
> struct ak8975_data *data = indio_dev->dev_data;
> struct i2c_client *client = data->client;
> struct iio_dev_attr *this_attr = to_iio_dev_attr(devattr);
> - u32 timeout_ms = AK8975_MAX_CONVERSION_TIMEOUT;
> u16 meas_reg;
> s16 raw;
> u8 read_status;
> @@ -352,23 +404,14 @@ static ssize_t show_raw(struct device *dev, struct device_attribute *devattr,
> }
>
> /* Wait for the conversion to complete. */
> - while (timeout_ms) {
> - msleep(AK8975_CONVERSION_DONE_POLL_TIME);
> - if (gpio_get_value(data->eoc_gpio))
> - break;
> - timeout_ms -= AK8975_CONVERSION_DONE_POLL_TIME;
> - }
> - if (!timeout_ms) {
> - dev_err(&client->dev, "Conversion timeout happened\n");
> - ret = -EINVAL;
> + if (data->eoc_gpio)
> + ret = wait_conversion_complete_gpio(data);
> + else
> + ret = wait_conversion_complete_polled(data);
> + if (ret < 0)
> goto exit;
> - }
>
> - ret = ak8975_read_data(client, AK8975_REG_ST1, 1, &read_status);
> - if (ret < 0) {
> - dev_err(&client->dev, "Error in reading ST1\n");
> - goto exit;
> - }
> + read_status = ret;
>
> if (read_status & AK8975_REG_ST1_DRDY_MASK) {
> ret = ak8975_read_data(client, AK8975_REG_ST2, 1, &read_status);
> @@ -454,25 +497,26 @@ static int ak8975_probe(struct i2c_client *client,
> data->eoc_irq = client->irq;
> data->eoc_gpio = irq_to_gpio(client->irq);
>
> - if (!data->eoc_gpio) {
> - dev_err(&client->dev, "failed, no valid GPIO\n");
> - err = -EINVAL;
> - goto exit_free;
> - }
> -
> - err = gpio_request(data->eoc_gpio, "ak_8975");
> - if (err < 0) {
> - dev_err(&client->dev, "failed to request GPIO %d, error %d\n",
> - data->eoc_gpio, err);
> - goto exit_free;
> - }
> + /* We may not have a GPIO based IRQ to scan, that is fine, we will
> + poll if so */
> + if (data->eoc_gpio > 0) {
> + err = gpio_request(data->eoc_gpio, "ak_8975");
> + if (err < 0) {
> + dev_err(&client->dev,
> + "failed to request GPIO %d, error %d\n",
> + data->eoc_gpio, err);
> + goto exit_free;
> + }
>
> - err = gpio_direction_input(data->eoc_gpio);
> - if (err < 0) {
> - dev_err(&client->dev, "Failed to configure input direction for"
> - " GPIO %d, error %d\n", data->eoc_gpio, err);
> - goto exit_gpio;
> - }
> + err = gpio_direction_input(data->eoc_gpio);
> + if (err < 0) {
> + dev_err(&client->dev,
> + "Failed to configure input direction for GPIO %d, error %d\n",
> + data->eoc_gpio, err);
> + goto exit_gpio;
> + }
> + } else
> + data->eoc_gpio = 0; /* No GPIO available */
>
> /* Perform some basic start-of-day setup of the device. */
> err = ak8975_setup(client);
> @@ -503,7 +547,8 @@ static int ak8975_probe(struct i2c_client *client,
> exit_free_iio:
> iio_free_device(data->indio_dev);
> exit_gpio:
> - gpio_free(data->eoc_gpio);
> + if (data->eoc_gpio)
> + gpio_free(data->eoc_gpio);
> exit_free:
> kfree(data);
> exit:
> @@ -517,7 +562,8 @@ static int ak8975_remove(struct i2c_client *client)
> iio_device_unregister(data->indio_dev);
> iio_free_device(data->indio_dev);
>
> - gpio_free(data->eoc_gpio);
> + if (data->eoc_gpio)
> + gpio_free(data->eoc_gpio);
>
> kfree(data);
>
>
>