From: Jonathan Cameron <jic23@kernel.org>
To: Peter Meerwald <pmeerw@pmeerw.net>
Cc: linux-iio@vger.kernel.org, Shubhrajyoti Datta <shubhrajyoti@ti.com>
Subject: Re: [PATCH 6/9] staging:iio:hmc5843: Trim sampling_frequencies to sampling_freq
Date: Sat, 27 Jul 2013 19:01:57 +0100 [thread overview]
Message-ID: <51F40B15.7080608@kernel.org> (raw)
In-Reply-To: <1374939112-18237-7-git-send-email-pmeerw@pmeerw.net>
On 07/27/13 16:31, Peter Meerwald wrote:
> Signed-off-by: Peter Meerwald <pmeerw@pmeerw.net>
> Cc: Shubhrajyoti Datta <shubhrajyoti@ti.com>
Glad to see this driver getting tidied up but I have a couple of questions about
the purpose of some of these attributes.
As you have observed elsewhere the device only supports the sysfs
interface at the moment, so do we actually want the various
modes? My quick reading of the datasheet suggests that we may
just want to do transitions from sleep to single sampling mode
on demand (i.e. when a reading is requested). Honestly I find
the differences between sleep and idle in the datasheet really
confusing.
Also is there a use (other than the self test) for the the generated
bias options under measurement_configuration?
> ---
> drivers/staging/iio/magnetometer/hmc5843.c | 38 ++++++++++++++----------------
> 1 file changed, 18 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/staging/iio/magnetometer/hmc5843.c b/drivers/staging/iio/magnetometer/hmc5843.c
> index 042467d..1ebb8f8 100644
> --- a/drivers/staging/iio/magnetometer/hmc5843.c
> +++ b/drivers/staging/iio/magnetometer/hmc5843.c
> @@ -371,15 +371,13 @@ exit:
> return count;
> }
>
> -static IIO_DEVICE_ATTR(meas_conf,
> - S_IWUSR | S_IRUGO,
> - hmc5843_show_measurement_configuration,
> - hmc5843_set_measurement_configuration,
> - 0);
> +static IIO_DEVICE_ATTR(measurement_configuration, S_IWUSR | S_IRUGO,
> + hmc5843_show_measurement_conf,
> + hmc5843_set_measurement_conf, 0);
>
> -static ssize_t hmc5843_show_sampling_frequencies_available(struct device *dev,
> - struct device_attribute *attr,
> - char *buf)
> +static ssize_t hmc5843_show_sampling_freq_available(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> {
> struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> struct hmc5843_data *data = iio_priv(indio_dev);
> @@ -387,7 +385,8 @@ static ssize_t hmc5843_show_sampling_frequencies_available(struct device *dev,
> int i;
>
> for (i = 0; i < HMC5843_RATE_NOT_USED; i++) {
> - ssize_t n = sprintf(buf, "%s ", data->variant->regval_to_sample_freq[i]);
> + ssize_t n = sprintf(buf, "%s ",
> + data->variant->regval_to_sample_freq[i]);
> buf += n;
> total_n += n;
> }
> @@ -397,7 +396,7 @@ static ssize_t hmc5843_show_sampling_frequencies_available(struct device *dev,
> return total_n;
> }
>
> -static IIO_DEV_ATTR_SAMP_FREQ_AVAIL(hmc5843_show_sampling_frequencies_available);
> +static IIO_DEV_ATTR_SAMP_FREQ_AVAIL(hmc5843_show_sampling_freq_available);
>
> static s32 hmc5843_set_rate(struct i2c_client *client,
> u8 rate)
> @@ -416,7 +415,7 @@ static s32 hmc5843_set_rate(struct i2c_client *client,
> return i2c_smbus_write_byte_data(client, HMC5843_CONFIG_REG_A, reg_val);
> }
>
> -static int hmc5843_check_sampling_frequency(struct hmc5843_data *data,
> +static int hmc5843_check_sampling_freq(struct hmc5843_data *data,
> const char *buf)
> {
> const char * const *samp_freq = data->variant->regval_to_sample_freq;
> @@ -430,7 +429,7 @@ static int hmc5843_check_sampling_frequency(struct hmc5843_data *data,
> return -EINVAL;
> }
>
> -static ssize_t hmc5843_set_sampling_frequency(struct device *dev,
> +static ssize_t hmc5843_set_sampling_freq(struct device *dev,
> struct device_attribute *attr,
> const char *buf, size_t count)
> {
> @@ -440,7 +439,7 @@ static ssize_t hmc5843_set_sampling_frequency(struct device *dev,
> struct hmc5843_data *data = iio_priv(indio_dev);
> int rate;
>
> - rate = hmc5843_check_sampling_frequency(data, buf);
> + rate = hmc5843_check_sampling_freq(data, buf);
> if (rate < 0) {
> dev_err(&client->dev,
> "sampling frequency is not supported\n");
> @@ -460,7 +459,7 @@ exit:
> return count;
> }
>
> -static ssize_t hmc5843_show_sampling_frequency(struct device *dev,
> +static ssize_t hmc5843_show_sampling_freq(struct device *dev,
> struct device_attribute *attr, char *buf)
> {
> struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> @@ -476,10 +475,9 @@ static ssize_t hmc5843_show_sampling_frequency(struct device *dev,
> return sprintf(buf, "%s\n", data->variant->regval_to_sample_freq[rate]);
> }
>
> -static IIO_DEVICE_ATTR(sampling_frequency,
> - S_IWUSR | S_IRUGO,
> - hmc5843_show_sampling_frequency,
> - hmc5843_set_sampling_frequency,
> +static IIO_DEVICE_ATTR(sampling_freq, S_IWUSR | S_IRUGO,
> + hmc5843_show_sampling_freq,
> + hmc5843_set_sampling_freq,
> HMC5843_CONFIG_REG_A);
>
> static ssize_t hmc5843_show_range_gain(struct device *dev,
> @@ -578,9 +576,9 @@ static const struct iio_chan_spec hmc5883_channels[] = {
> };
>
> static struct attribute *hmc5843_attributes[] = {
> - &iio_dev_attr_meas_conf.dev_attr.attr,
> + &iio_dev_attr_measurement_configuration.dev_attr.attr,
> &iio_dev_attr_operating_mode.dev_attr.attr,
> - &iio_dev_attr_sampling_frequency.dev_attr.attr,
> + &iio_dev_attr_sampling_freq.dev_attr.attr,
> &iio_dev_attr_in_magn_range.dev_attr.attr,
> &iio_dev_attr_sampling_frequency_available.dev_attr.attr,
> NULL
>
next prev parent reply other threads:[~2013-07-27 17:01 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-07-27 15:31 Peter Meerwald
2013-07-27 15:31 ` [PATCH 1/9] staging:iio:hmc5843: Drop I2C detection code Peter Meerwald
2013-08-04 10:01 ` Jonathan Cameron
2013-07-27 15:31 ` [PATCH 2/9] staging:iio:hmc5843: Remove id register #defines, not used anymore Peter Meerwald
2013-08-04 10:02 ` Jonathan Cameron
2013-07-27 15:31 ` [PATCH 3/9] staging:iio:hmc5843: Implement timeout in read function Peter Meerwald
2013-08-04 10:03 ` Jonathan Cameron
2013-07-27 15:31 ` [PATCH 4/9] staging:iio:hmc5843: 'add' is a poor abbreviation for address Peter Meerwald
2013-08-04 10:04 ` Jonathan Cameron
2013-07-27 15:31 ` [PATCH 5/9] staging:iio:hmc5843: Device has 3 channels, no need to store separately Peter Meerwald
2013-08-04 10:10 ` Jonathan Cameron
2013-07-27 15:31 ` [PATCH 6/9] staging:iio:hmc5843: Trim sampling_frequencies to sampling_freq Peter Meerwald
2013-07-27 18:01 ` Jonathan Cameron [this message]
2013-08-04 10:13 ` Jonathan Cameron
2013-07-27 15:31 ` [PATCH 7/9] staging:iio:hmc5843: Split and join lines to make checkpatch happy Peter Meerwald
2013-08-04 10:15 ` Jonathan Cameron
2013-07-27 15:31 ` [PATCH 8/9] staging:iio:hmc5843: Drop unneeded #includes Peter Meerwald
2013-08-04 10:22 ` Jonathan Cameron
2013-07-27 15:31 ` [PATCH 9/9] staging:iio:hmc5843: Use i2c_smbus_read_word_swapped() Peter Meerwald
2013-08-04 10:23 ` Jonathan Cameron
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=51F40B15.7080608@kernel.org \
--to=jic23@kernel.org \
--cc=linux-iio@vger.kernel.org \
--cc=pmeerw@pmeerw.net \
--cc=shubhrajyoti@ti.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.