From: Jonathan Cameron <jic23@cam.ac.uk>
To: Barry Song <21cnbao@gmail.com>
Cc: linux-iio@vger.kernel.org
Subject: Re: [PATCH 2/2] staging:iio: adis16220 extract bin_attribute structures from state
Date: Thu, 13 May 2010 11:18:19 +0100 [thread overview]
Message-ID: <4BEBD1EB.8050206@cam.ac.uk> (raw)
In-Reply-To: <AANLkTikFuJnf5AVZHQfEyXmSgE-AGLCD1gpTi4H9WrKc@mail.gmail.com>
On 05/13/10 04:06, Barry Song wrote:
> On Thu, May 13, 2010 at 4:53 AM, Jonathan Cameron <jic23@cam.ac.uk> wrote:
>> Signed-off-by: Jonathan Cameron <jic23@cam.ac.uk>
>> ---
>> drivers/staging/iio/accel/adis16220.h | 3 -
>> drivers/staging/iio/accel/adis16220_core.c | 106 +++++++++++++++-------------
>> 2 files changed, 56 insertions(+), 53 deletions(-)
>>
>> diff --git a/drivers/staging/iio/accel/adis16220.h b/drivers/staging/iio/accel/adis16220.h
>> index 6b49f70..2abf485 100644
>> --- a/drivers/staging/iio/accel/adis16220.h
>> +++ b/drivers/staging/iio/accel/adis16220.h
>> @@ -141,9 +141,6 @@ struct adis16220_state {
>> struct iio_dev *indio_dev;
>> u8 *tx;
>> u8 *rx;
>> - struct bin_attribute accel_bin;
>> - struct bin_attribute adc1_bin;
>> - struct bin_attribute adc2_bin;
>> struct mutex buf_lock;
>> };
>>
>> diff --git a/drivers/staging/iio/accel/adis16220_core.c b/drivers/staging/iio/accel/adis16220_core.c
>> index 8b845d9..e60de4a 100644
>> --- a/drivers/staging/iio/accel/adis16220_core.c
>> +++ b/drivers/staging/iio/accel/adis16220_core.c
>> @@ -27,8 +27,6 @@
>>
>> #define DRIVER_NAME "adis16220"
>>
>> -static int adis16220_check_status(struct device *dev);
>> -
>> /**
>> * adis16220_spi_write_reg_8() - write single byte to a register
>> * @dev: device associated with child of actual device (iio_dev or iio_trig)
>> @@ -133,8 +131,6 @@ static int adis16220_spi_read_reg_16(struct device *dev,
>> mutex_lock(&st->buf_lock);
>> st->tx[0] = ADIS16220_READ_REG(lower_reg_address);
>> st->tx[1] = 0;
>> - st->tx[2] = 0;
>> - st->tx[3] = 0;
>>
>> spi_message_init(&msg);
>> spi_message_add_tail(&xfers[0], &msg);
>> @@ -275,23 +271,6 @@ static ssize_t adis16220_write_capture(struct device *dev,
>> return -1;
>> }
>>
>> -static int adis16220_self_test(struct device *dev)
>> -{
>> - int ret;
>> - ret = adis16220_spi_write_reg_16(dev,
>> - ADIS16220_MSC_CTRL,
>> - ADIS16220_MSC_CTRL_SELF_TEST_EN);
>> - if (ret) {
>> - dev_err(dev, "problem starting self test");
>> - goto err_ret;
>> - }
>> -
>> - adis16220_check_status(dev);
>> -
>> -err_ret:
>> - return ret;
>> -}
>> -
>> static int adis16220_check_status(struct device *dev)
>> {
>> u16 status;
>> @@ -320,6 +299,23 @@ error_ret:
>> return ret;
>> }
>>
>> +static int adis16220_self_test(struct device *dev)
>> +{
>> + int ret;
>> + ret = adis16220_spi_write_reg_16(dev,
>> + ADIS16220_MSC_CTRL,
>> + ADIS16220_MSC_CTRL_SELF_TEST_EN);
>> + if (ret) {
>> + dev_err(dev, "problem starting self test");
>> + goto err_ret;
>> + }
>> +
>> + adis16220_check_status(dev);
>> +
>> +err_ret:
>> + return ret;
>> +}
>> +
>> static int adis16220_initial_setup(struct adis16220_state *st)
>> {
>> int ret;
>> @@ -433,6 +429,15 @@ static ssize_t adis16220_accel_bin_read(struct kobject *kobj,
>> ADIS16220_CAPT_BUFA);
>> }
>>
>> +static struct bin_attribute accel_bin = {
>> + .attr = {
>> + .name = "accel_bin",
>> + .mode = S_IRUSR,
> Here i can't understand why read permission is only given to
> user(probably root). Why not S_IRUGO which means
> (S_IRUSR|S_IRGRP|S_IROTH) ?
Good spot. That's what I get for lifting code from another
driver without thinking about it.
>
>> + },
>> + .read = adis16220_accel_bin_read,
>> + .size = ADIS16220_CAPTURE_SIZE,
>> +};
>> +
>> static ssize_t adis16220_adc1_bin_read(struct kobject *kobj,
>> struct bin_attribute *attr,
>> char *buf, loff_t off,
>> @@ -447,6 +452,15 @@ static ssize_t adis16220_adc1_bin_read(struct kobject *kobj,
>> ADIS16220_CAPT_BUF1);
>> }
>>
>> +static struct bin_attribute adc1_bin = {
>> + .attr = {
>> + .name = "in0_bin",
>> + .mode = S_IRUSR,
>> + },
>> + .read = adis16220_adc1_bin_read,
>> + .size = ADIS16220_CAPTURE_SIZE,
>> +};
>> +
>> static ssize_t adis16220_adc2_bin_read(struct kobject *kobj,
>> struct bin_attribute *attr,
>> char *buf, loff_t off,
>> @@ -461,6 +475,16 @@ static ssize_t adis16220_adc2_bin_read(struct kobject *kobj,
>> ADIS16220_CAPT_BUF2);
>> }
>>
>> +
>> +static struct bin_attribute adc2_bin = {
>> + .attr = {
>> + .name = "in1_bin",
>> + .mode = S_IRUSR,
>> + },
>> + .read = adis16220_adc2_bin_read,
>> + .size = ADIS16220_CAPTURE_SIZE,
>> +};
>> +
>> static IIO_DEV_ATTR_IN_NAMED_RAW(supply, adis16220_read_12bit_unsigned,
>> ADIS16220_CAPT_SUPPLY);
>> static IIO_CONST_ATTR(in_supply_scale, "0.0012207");
>> @@ -481,12 +505,12 @@ static IIO_DEV_ATTR_IN_RAW(1, adis16220_read_16bit, ADIS16220_CAPT_BUF2);
>> static IIO_DEVICE_ATTR(reset, S_IWUSR, NULL,
>> adis16220_write_reset, 0);
>>
>> -#define IIO_DEV_ATTR_CAPTURE(_store) \
>> +#define IIO_DEV_ATTR_CAPTURE(_store) \
>> IIO_DEVICE_ATTR(capture, S_IWUGO, NULL, _store, 0)
>>
>> static IIO_DEV_ATTR_CAPTURE(adis16220_write_capture);
>>
>> -#define IIO_DEV_ATTR_CAPTURE_COUNT(_mode, _show, _store, _addr) \
>> +#define IIO_DEV_ATTR_CAPTURE_COUNT(_mode, _show, _store, _addr) \
>> IIO_DEVICE_ATTR(capture_count, _mode, _show, _store, _addr)
>>
>> static IIO_DEV_ATTR_CAPTURE_COUNT(S_IWUSR | S_IRUGO,
>> @@ -563,33 +587,15 @@ static int __devinit adis16220_probe(struct spi_device *spi)
>> goto error_free_dev;
>> regdone = 1;
>>
>> - st->accel_bin.attr.name = "accel_bin";
>> - st->accel_bin.attr.mode = S_IRUGO;
>> - st->accel_bin.attr.owner = THIS_MODULE;
>> - st->accel_bin.read = adis16220_accel_bin_read;
>> - st->accel_bin.size = ADIS16220_CAPTURE_SIZE;
>> -
>> - ret = sysfs_create_bin_file(&st->indio_dev->dev.kobj, &st->accel_bin);
>> + ret = sysfs_create_bin_file(&st->indio_dev->dev.kobj, &accel_bin);
>> if (ret)
>> goto error_free_dev;
>>
>> - st->adc1_bin.attr.name = "adc1_bin";
>> - st->adc1_bin.attr.mode = S_IRUGO;
>> - st->adc1_bin.attr.owner = THIS_MODULE;
>> - st->adc1_bin.read = adis16220_adc1_bin_read;
>> - st->adc1_bin.size = ADIS16220_CAPTURE_SIZE;
>> -
>> - ret = sysfs_create_bin_file(&st->indio_dev->dev.kobj, &st->adc1_bin);
>> + ret = sysfs_create_bin_file(&st->indio_dev->dev.kobj, &adc1_bin);
>> if (ret)
>> goto error_rm_accel_bin;
>>
>> - st->adc2_bin.attr.name = "adc2_bin";
>> - st->adc2_bin.attr.mode = S_IRUGO;
>> - st->adc2_bin.attr.owner = THIS_MODULE;
>> - st->adc2_bin.read = adis16220_adc2_bin_read;
>> - st->adc2_bin.size = ADIS16220_CAPTURE_SIZE;
>> -
>> - ret = sysfs_create_bin_file(&st->indio_dev->dev.kobj, &st->adc2_bin);
>> + ret = sysfs_create_bin_file(&st->indio_dev->dev.kobj, &adc2_bin);
>> if (ret)
>> goto error_rm_adc1_bin;
>>
>> @@ -600,11 +606,11 @@ static int __devinit adis16220_probe(struct spi_device *spi)
>> return 0;
>>
>> error_rm_adc2_bin:
>> - sysfs_remove_bin_file(&st->indio_dev->dev.kobj, &st->adc2_bin);
>> + sysfs_remove_bin_file(&st->indio_dev->dev.kobj, &adc2_bin);
>> error_rm_adc1_bin:
>> - sysfs_remove_bin_file(&st->indio_dev->dev.kobj, &st->adc1_bin);
>> + sysfs_remove_bin_file(&st->indio_dev->dev.kobj, &adc1_bin);
>> error_rm_accel_bin:
>> - sysfs_remove_bin_file(&st->indio_dev->dev.kobj, &st->accel_bin);
>> + sysfs_remove_bin_file(&st->indio_dev->dev.kobj, &accel_bin);
>> error_free_dev:
>> if (regdone)
>> iio_device_unregister(st->indio_dev);
>> @@ -627,9 +633,9 @@ static int adis16220_remove(struct spi_device *spi)
>>
>> flush_scheduled_work();
>>
>> - sysfs_remove_bin_file(&st->indio_dev->dev.kobj, &st->adc2_bin);
>> - sysfs_remove_bin_file(&st->indio_dev->dev.kobj, &st->adc1_bin);
>> - sysfs_remove_bin_file(&st->indio_dev->dev.kobj, &st->accel_bin);
>> + sysfs_remove_bin_file(&st->indio_dev->dev.kobj, &adc2_bin);
>> + sysfs_remove_bin_file(&st->indio_dev->dev.kobj, &adc1_bin);
>> + sysfs_remove_bin_file(&st->indio_dev->dev.kobj, &accel_bin);
>> iio_device_unregister(indio_dev);
>> kfree(st->tx);
>> kfree(st->rx);
>> --
>> 1.7.0.4
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
>
prev parent reply other threads:[~2010-05-13 10:16 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-05-12 20:53 [PATCH 0/2] adis16220 driver Jonathan Cameron
2010-05-12 20:53 ` [PATCH 1/2] staging:iio: adis16220 vibration sensor driver Jonathan Cameron
2010-05-13 3:13 ` Barry Song
2010-05-12 20:53 ` [PATCH 2/2] staging:iio: adis16220 extract bin_attribute structures from state Jonathan Cameron
2010-05-13 3:06 ` Barry Song
2010-05-13 10:18 ` Jonathan Cameron [this message]
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=4BEBD1EB.8050206@cam.ac.uk \
--to=jic23@cam.ac.uk \
--cc=21cnbao@gmail.com \
--cc=linux-iio@vger.kernel.org \
/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.