| Message ID | 20200630045708.14166-5-alexandru.ardelean@analog.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | iio: core: wrap IIO device into an iio_dev_opaque object | expand |
On Tue, 2020-06-30 at 07:57 +0300, Alexandru Ardelean wrote: > This change moves all iio_dev debugfs fields to the iio_dev_priv object. > It's not the biggest advantage yet (to the whole thing of > abstractization) > but it's a start. > > The iio_get_debugfs_dentry() function (which is moved in > industrialio-core.c) needs to also be guarded against the CONFIG_DEBUG_FS > symbol, when it isn't defined. We do want to keep the inline definition > in > the iio.h header, so that the compiler can better infer when to compile > out > debugfs code that is related to the IIO debugfs directory. > Well, pretty much only this patch changed since V3. I thought about maybe re-doing just this patch, then I thought maybe I'd get a minor complaint that I should re-send the series. Either way, I prefer a complaint on this V4 series-re-send than if I were to have re-sent just this patch. > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com> > --- > drivers/iio/industrialio-core.c | 46 +++++++++++++++++++++++---------- > include/linux/iio/iio-opaque.h | 10 +++++++ > include/linux/iio/iio.h | 13 +--------- > 3 files changed, 44 insertions(+), 25 deletions(-) > > diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio- > core.c > index 27005ba4d09c..64174052641a 100644 > --- a/drivers/iio/industrialio-core.c > +++ b/drivers/iio/industrialio-core.c > @@ -165,6 +165,19 @@ static const char * const iio_chan_info_postfix[] = > { > [IIO_CHAN_INFO_THERMOCOUPLE_TYPE] = "thermocouple_type", > }; > > +#if !defined(CONFIG_DEBUG_FS) > +/** > + * There's also a CONFIG_DEBUG_FS guard in include/linux/iio/iio.h for > + * iio_get_debugfs_dentry() to make it inline if CONFIG_DEBUG_FS is > undefined > + */ > +struct dentry *iio_get_debugfs_dentry(struct iio_dev *indio_dev) > +{ > + struct iio_dev_opaque *iio_dev_opaque = > to_iio_dev_opaque(indio_dev); > + return iio_dev_opaque->debugfs_dentry; > +} > +EXPORT_SYMBOL_GPL(iio_get_debugfs_dentry); > +#endif > + > /** > * iio_find_channel_from_si() - get channel from its scan index > * @indio_dev: device > @@ -308,35 +321,37 @@ static ssize_t iio_debugfs_read_reg(struct file > *file, char __user *userbuf, > size_t count, loff_t *ppos) > { > struct iio_dev *indio_dev = file->private_data; > + struct iio_dev_opaque *iio_dev_opaque = > to_iio_dev_opaque(indio_dev); > unsigned val = 0; > int ret; > > if (*ppos > 0) > return simple_read_from_buffer(userbuf, count, ppos, > - indio_dev->read_buf, > - indio_dev->read_buf_len); > + iio_dev_opaque->read_buf, > + iio_dev_opaque- > >read_buf_len); > > ret = indio_dev->info->debugfs_reg_access(indio_dev, > - indio_dev- > >cached_reg_addr, > + iio_dev_opaque- > >cached_reg_addr, > 0, &val); > if (ret) { > dev_err(indio_dev->dev.parent, "%s: read failed\n", > __func__); > return ret; > } > > - indio_dev->read_buf_len = snprintf(indio_dev->read_buf, > - sizeof(indio_dev->read_buf), > - "0x%X\n", val); > + iio_dev_opaque->read_buf_len = snprintf(iio_dev_opaque->read_buf, > + sizeof(iio_dev_opaque- > >read_buf), > + "0x%X\n", val); > > return simple_read_from_buffer(userbuf, count, ppos, > - indio_dev->read_buf, > - indio_dev->read_buf_len); > + iio_dev_opaque->read_buf, > + iio_dev_opaque->read_buf_len); > } > > static ssize_t iio_debugfs_write_reg(struct file *file, > const char __user *userbuf, size_t count, loff_t > *ppos) > { > struct iio_dev *indio_dev = file->private_data; > + struct iio_dev_opaque *iio_dev_opaque = > to_iio_dev_opaque(indio_dev); > unsigned reg, val; > char buf[80]; > int ret; > @@ -351,10 +366,10 @@ static ssize_t iio_debugfs_write_reg(struct file > *file, > > switch (ret) { > case 1: > - indio_dev->cached_reg_addr = reg; > + iio_dev_opaque->cached_reg_addr = reg; > break; > case 2: > - indio_dev->cached_reg_addr = reg; > + iio_dev_opaque->cached_reg_addr = reg; > ret = indio_dev->info->debugfs_reg_access(indio_dev, reg, > val, NULL); > if (ret) { > @@ -378,23 +393,28 @@ static const struct file_operations > iio_debugfs_reg_fops = { > > static void iio_device_unregister_debugfs(struct iio_dev *indio_dev) > { > - debugfs_remove_recursive(indio_dev->debugfs_dentry); > + struct iio_dev_opaque *iio_dev_opaque = > to_iio_dev_opaque(indio_dev); > + debugfs_remove_recursive(iio_dev_opaque->debugfs_dentry); > } > > static void iio_device_register_debugfs(struct iio_dev *indio_dev) > { > + struct iio_dev_opaque *iio_dev_opaque; > + > if (indio_dev->info->debugfs_reg_access == NULL) > return; > > if (!iio_debugfs_dentry) > return; > > - indio_dev->debugfs_dentry = > + iio_dev_opaque = to_iio_dev_opaque(indio_dev); > + > + iio_dev_opaque->debugfs_dentry = > debugfs_create_dir(dev_name(&indio_dev->dev), > iio_debugfs_dentry); > > debugfs_create_file("direct_reg_access", 0644, > - indio_dev->debugfs_dentry, indio_dev, > + iio_dev_opaque->debugfs_dentry, indio_dev, > &iio_debugfs_reg_fops); > } > #else > diff --git a/include/linux/iio/iio-opaque.h b/include/linux/iio/iio- > opaque.h > index 1375674f14cd..b3f234b4c1e9 100644 > --- a/include/linux/iio/iio-opaque.h > +++ b/include/linux/iio/iio-opaque.h > @@ -6,9 +6,19 @@ > /** > * struct iio_dev_opaque - industrial I/O device opaque information > * @indio_dev: public industrial I/O device > information > + * @debugfs_dentry: device specific debugfs dentry > + * @cached_reg_addr: cached register address for debugfs reads > + * @read_buf: read buffer to be used for the > initial reg read > + * @read_buf_len: data length in @read_buf > */ > struct iio_dev_opaque { > struct iio_dev indio_dev; > +#if defined(CONFIG_DEBUG_FS) > + struct dentry *debugfs_dentry; > + unsigned cached_reg_addr; > + char read_buf[20]; > + unsigned int read_buf_len; > +#endif > }; > > #define to_iio_dev_opaque(indio_dev) \ > diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h > index 86112e35ae5f..bb0aae11a111 100644 > --- a/include/linux/iio/iio.h > +++ b/include/linux/iio/iio.h > @@ -520,8 +520,6 @@ struct iio_buffer_setup_ops { > * @groups: [INTERN] attribute groups > * @groupcounter: [INTERN] index of next attribute group > * @flags: [INTERN] file ops related flags including busy > flag. > - * @debugfs_dentry: [INTERN] device specific debugfs dentry. > - * @cached_reg_addr: [INTERN] cached register address for debugfs reads. > * @priv: [DRIVER] reference to driver's private information > * **MUST** be accessed **ONLY** via iio_priv() helper > */ > @@ -567,12 +565,6 @@ struct iio_dev { > int groupcounter; > > unsigned long flags; > -#if defined(CONFIG_DEBUG_FS) > - struct dentry *debugfs_dentry; > - unsigned cached_reg_addr; > - char read_buf[20]; > - unsigned int read_buf_len; > -#endif > void *priv; > }; > > @@ -727,10 +719,7 @@ static inline bool iio_buffer_enabled(struct iio_dev > *indio_dev) > * @indio_dev: IIO device structure for device > **/ > #if defined(CONFIG_DEBUG_FS) > -static inline struct dentry *iio_get_debugfs_dentry(struct iio_dev > *indio_dev) > -{ > - return indio_dev->debugfs_dentry; > -} > +struct dentry *iio_get_debugfs_dentry(struct iio_dev *indio_dev); > #else > static inline struct dentry *iio_get_debugfs_dentry(struct iio_dev > *indio_dev) > {
On Tue, 30 Jun 2020 04:58:06 +0000 "Ardelean, Alexandru" <alexandru.Ardelean@analog.com> wrote: > On Tue, 2020-06-30 at 07:57 +0300, Alexandru Ardelean wrote: > > This change moves all iio_dev debugfs fields to the iio_dev_priv object. > > It's not the biggest advantage yet (to the whole thing of > > abstractization) > > but it's a start. > > > > The iio_get_debugfs_dentry() function (which is moved in > > industrialio-core.c) needs to also be guarded against the CONFIG_DEBUG_FS > > symbol, when it isn't defined. We do want to keep the inline definition > > in > > the iio.h header, so that the compiler can better infer when to compile > > out > > debugfs code that is related to the IIO debugfs directory. > > > > Well, pretty much only this patch changed since V3. > I thought about maybe re-doing just this patch, then I thought maybe I'd > get a minor complaint that I should re-send the series. > > Either way, I prefer a complaint on this V4 series-re-send than if I were > to have re-sent just this patch. Either way worked. However this doesn't pass my basic build test. Config condition is reversed. Fixed up and pushed out as testing. Jonathan > > > > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com> > > --- > > drivers/iio/industrialio-core.c | 46 +++++++++++++++++++++++---------- > > include/linux/iio/iio-opaque.h | 10 +++++++ > > include/linux/iio/iio.h | 13 +--------- > > 3 files changed, 44 insertions(+), 25 deletions(-) > > > > diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio- > > core.c > > index 27005ba4d09c..64174052641a 100644 > > --- a/drivers/iio/industrialio-core.c > > +++ b/drivers/iio/industrialio-core.c > > @@ -165,6 +165,19 @@ static const char * const iio_chan_info_postfix[] = > > { > > [IIO_CHAN_INFO_THERMOCOUPLE_TYPE] = "thermocouple_type", > > }; > > > > +#if !defined(CONFIG_DEBUG_FS) Don't we want this if it is defined. > > +/** > > + * There's also a CONFIG_DEBUG_FS guard in include/linux/iio/iio.h for > > + * iio_get_debugfs_dentry() to make it inline if CONFIG_DEBUG_FS is > > undefined > > + */ > > +struct dentry *iio_get_debugfs_dentry(struct iio_dev *indio_dev) > > +{ > > + struct iio_dev_opaque *iio_dev_opaque = > > to_iio_dev_opaque(indio_dev); > > + return iio_dev_opaque->debugfs_dentry; > > +} > > +EXPORT_SYMBOL_GPL(iio_get_debugfs_dentry); > > +#endif > > + > > /** > > * iio_find_channel_from_si() - get channel from its scan index > > * @indio_dev: device > > @@ -308,35 +321,37 @@ static ssize_t iio_debugfs_read_reg(struct file > > *file, char __user *userbuf, > > size_t count, loff_t *ppos) > > { > > struct iio_dev *indio_dev = file->private_data; > > + struct iio_dev_opaque *iio_dev_opaque = > > to_iio_dev_opaque(indio_dev); > > unsigned val = 0; > > int ret; > > > > if (*ppos > 0) > > return simple_read_from_buffer(userbuf, count, ppos, > > - indio_dev->read_buf, > > - indio_dev->read_buf_len); > > + iio_dev_opaque->read_buf, > > + iio_dev_opaque- > > >read_buf_len); > > > > ret = indio_dev->info->debugfs_reg_access(indio_dev, > > - indio_dev- > > >cached_reg_addr, > > + iio_dev_opaque- > > >cached_reg_addr, > > 0, &val); > > if (ret) { > > dev_err(indio_dev->dev.parent, "%s: read failed\n", > > __func__); > > return ret; > > } > > > > - indio_dev->read_buf_len = snprintf(indio_dev->read_buf, > > - sizeof(indio_dev->read_buf), > > - "0x%X\n", val); > > + iio_dev_opaque->read_buf_len = snprintf(iio_dev_opaque->read_buf, > > + sizeof(iio_dev_opaque- > > >read_buf), > > + "0x%X\n", val); > > > > return simple_read_from_buffer(userbuf, count, ppos, > > - indio_dev->read_buf, > > - indio_dev->read_buf_len); > > + iio_dev_opaque->read_buf, > > + iio_dev_opaque->read_buf_len); > > } > > > > static ssize_t iio_debugfs_write_reg(struct file *file, > > const char __user *userbuf, size_t count, loff_t > > *ppos) > > { > > struct iio_dev *indio_dev = file->private_data; > > + struct iio_dev_opaque *iio_dev_opaque = > > to_iio_dev_opaque(indio_dev); > > unsigned reg, val; > > char buf[80]; > > int ret; > > @@ -351,10 +366,10 @@ static ssize_t iio_debugfs_write_reg(struct file > > *file, > > > > switch (ret) { > > case 1: > > - indio_dev->cached_reg_addr = reg; > > + iio_dev_opaque->cached_reg_addr = reg; > > break; > > case 2: > > - indio_dev->cached_reg_addr = reg; > > + iio_dev_opaque->cached_reg_addr = reg; > > ret = indio_dev->info->debugfs_reg_access(indio_dev, reg, > > val, NULL); > > if (ret) { > > @@ -378,23 +393,28 @@ static const struct file_operations > > iio_debugfs_reg_fops = { > > > > static void iio_device_unregister_debugfs(struct iio_dev *indio_dev) > > { > > - debugfs_remove_recursive(indio_dev->debugfs_dentry); > > + struct iio_dev_opaque *iio_dev_opaque = > > to_iio_dev_opaque(indio_dev); > > + debugfs_remove_recursive(iio_dev_opaque->debugfs_dentry); > > } > > > > static void iio_device_register_debugfs(struct iio_dev *indio_dev) > > { > > + struct iio_dev_opaque *iio_dev_opaque; > > + > > if (indio_dev->info->debugfs_reg_access == NULL) > > return; > > > > if (!iio_debugfs_dentry) > > return; > > > > - indio_dev->debugfs_dentry = > > + iio_dev_opaque = to_iio_dev_opaque(indio_dev); > > + > > + iio_dev_opaque->debugfs_dentry = > > debugfs_create_dir(dev_name(&indio_dev->dev), > > iio_debugfs_dentry); > > > > debugfs_create_file("direct_reg_access", 0644, > > - indio_dev->debugfs_dentry, indio_dev, > > + iio_dev_opaque->debugfs_dentry, indio_dev, > > &iio_debugfs_reg_fops); > > } > > #else > > diff --git a/include/linux/iio/iio-opaque.h b/include/linux/iio/iio- > > opaque.h > > index 1375674f14cd..b3f234b4c1e9 100644 > > --- a/include/linux/iio/iio-opaque.h > > +++ b/include/linux/iio/iio-opaque.h > > @@ -6,9 +6,19 @@ > > /** > > * struct iio_dev_opaque - industrial I/O device opaque information > > * @indio_dev: public industrial I/O device > > information > > + * @debugfs_dentry: device specific debugfs dentry > > + * @cached_reg_addr: cached register address for debugfs reads > > + * @read_buf: read buffer to be used for the > > initial reg read > > + * @read_buf_len: data length in @read_buf > > */ > > struct iio_dev_opaque { > > struct iio_dev indio_dev; > > +#if defined(CONFIG_DEBUG_FS) > > + struct dentry *debugfs_dentry; > > + unsigned cached_reg_addr; > > + char read_buf[20]; > > + unsigned int read_buf_len; > > +#endif > > }; > > > > #define to_iio_dev_opaque(indio_dev) \ > > diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h > > index 86112e35ae5f..bb0aae11a111 100644 > > --- a/include/linux/iio/iio.h > > +++ b/include/linux/iio/iio.h > > @@ -520,8 +520,6 @@ struct iio_buffer_setup_ops { > > * @groups: [INTERN] attribute groups > > * @groupcounter: [INTERN] index of next attribute group > > * @flags: [INTERN] file ops related flags including busy > > flag. > > - * @debugfs_dentry: [INTERN] device specific debugfs dentry. > > - * @cached_reg_addr: [INTERN] cached register address for debugfs reads. > > * @priv: [DRIVER] reference to driver's private information > > * **MUST** be accessed **ONLY** via iio_priv() helper > > */ > > @@ -567,12 +565,6 @@ struct iio_dev { > > int groupcounter; > > > > unsigned long flags; > > -#if defined(CONFIG_DEBUG_FS) > > - struct dentry *debugfs_dentry; > > - unsigned cached_reg_addr; > > - char read_buf[20]; > > - unsigned int read_buf_len; > > -#endif > > void *priv; > > }; > > > > @@ -727,10 +719,7 @@ static inline bool iio_buffer_enabled(struct iio_dev > > *indio_dev) > > * @indio_dev: IIO device structure for device > > **/ > > #if defined(CONFIG_DEBUG_FS) > > -static inline struct dentry *iio_get_debugfs_dentry(struct iio_dev > > *indio_dev) > > -{ > > - return indio_dev->debugfs_dentry; > > -} > > +struct dentry *iio_get_debugfs_dentry(struct iio_dev *indio_dev); > > #else > > static inline struct dentry *iio_get_debugfs_dentry(struct iio_dev > > *indio_dev) > > {
On Wed, 2020-07-01 at 19:42 +0100, Jonathan Cameron wrote: > [External] > > On Tue, 30 Jun 2020 04:58:06 +0000 > "Ardelean, Alexandru" <alexandru.Ardelean@analog.com> wrote: > > > On Tue, 2020-06-30 at 07:57 +0300, Alexandru Ardelean wrote: > > > This change moves all iio_dev debugfs fields to the iio_dev_priv > > > object. > > > It's not the biggest advantage yet (to the whole thing of > > > abstractization) > > > but it's a start. > > > > > > The iio_get_debugfs_dentry() function (which is moved in > > > industrialio-core.c) needs to also be guarded against the > > > CONFIG_DEBUG_FS > > > symbol, when it isn't defined. We do want to keep the inline > > > definition > > > in > > > the iio.h header, so that the compiler can better infer when to > > > compile > > > out > > > debugfs code that is related to the IIO debugfs directory. > > > > > > > Well, pretty much only this patch changed since V3. > > I thought about maybe re-doing just this patch, then I thought maybe > > I'd > > get a minor complaint that I should re-send the series. > > > > Either way, I prefer a complaint on this V4 series-re-send than if I > > were > > to have re-sent just this patch. > > Either way worked. > > However this doesn't pass my basic build test. Config condition > is reversed. > > Fixed up and pushed out as testing. Sorry for the goof. I tested with make allmodconfig. Maybe I didn't pay attention somewhere. > > > Jonathan > > > > > > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com> > > > --- > > > drivers/iio/industrialio-core.c | 46 +++++++++++++++++++++++------ > > > ---- > > > include/linux/iio/iio-opaque.h | 10 +++++++ > > > include/linux/iio/iio.h | 13 +--------- > > > 3 files changed, 44 insertions(+), 25 deletions(-) > > > > > > diff --git a/drivers/iio/industrialio-core.c > > > b/drivers/iio/industrialio- > > > core.c > > > index 27005ba4d09c..64174052641a 100644 > > > --- a/drivers/iio/industrialio-core.c > > > +++ b/drivers/iio/industrialio-core.c > > > @@ -165,6 +165,19 @@ static const char * const > > > iio_chan_info_postfix[] = > > > { > > > [IIO_CHAN_INFO_THERMOCOUPLE_TYPE] = "thermocouple_type", > > > }; > > > > > > +#if !defined(CONFIG_DEBUG_FS) > > Don't we want this if it is defined. > > > > +/** > > > + * There's also a CONFIG_DEBUG_FS guard in include/linux/iio/iio.h > > > for > > > + * iio_get_debugfs_dentry() to make it inline if CONFIG_DEBUG_FS is > > > undefined > > > + */ > > > +struct dentry *iio_get_debugfs_dentry(struct iio_dev *indio_dev) > > > +{ > > > + struct iio_dev_opaque *iio_dev_opaque = > > > to_iio_dev_opaque(indio_dev); > > > + return iio_dev_opaque->debugfs_dentry; > > > +} > > > +EXPORT_SYMBOL_GPL(iio_get_debugfs_dentry); > > > +#endif > > > + > > > /** > > > * iio_find_channel_from_si() - get channel from its scan index > > > * @indio_dev: device > > > @@ -308,35 +321,37 @@ static ssize_t iio_debugfs_read_reg(struct file > > > *file, char __user *userbuf, > > > size_t count, loff_t *ppos) > > > { > > > struct iio_dev *indio_dev = file->private_data; > > > + struct iio_dev_opaque *iio_dev_opaque = > > > to_iio_dev_opaque(indio_dev); > > > unsigned val = 0; > > > int ret; > > > > > > if (*ppos > 0) > > > return simple_read_from_buffer(userbuf, count, ppos, > > > - indio_dev->read_buf, > > > - indio_dev->read_buf_len); > > > + iio_dev_opaque->read_buf, > > > + iio_dev_opaque- > > > > read_buf_len); > > > > > > ret = indio_dev->info->debugfs_reg_access(indio_dev, > > > - indio_dev- > > > > cached_reg_addr, > > > + iio_dev_opaque- > > > > cached_reg_addr, > > > 0, &val); > > > if (ret) { > > > dev_err(indio_dev->dev.parent, "%s: read failed\n", > > > __func__); > > > return ret; > > > } > > > > > > - indio_dev->read_buf_len = snprintf(indio_dev->read_buf, > > > - sizeof(indio_dev->read_buf), > > > - "0x%X\n", val); > > > + iio_dev_opaque->read_buf_len = snprintf(iio_dev_opaque->read_buf, > > > + sizeof(iio_dev_opaque- > > > > read_buf), > > > + "0x%X\n", val); > > > > > > return simple_read_from_buffer(userbuf, count, ppos, > > > - indio_dev->read_buf, > > > - indio_dev->read_buf_len); > > > + iio_dev_opaque->read_buf, > > > + iio_dev_opaque->read_buf_len); > > > } > > > > > > static ssize_t iio_debugfs_write_reg(struct file *file, > > > const char __user *userbuf, size_t count, loff_t > > > *ppos) > > > { > > > struct iio_dev *indio_dev = file->private_data; > > > + struct iio_dev_opaque *iio_dev_opaque = > > > to_iio_dev_opaque(indio_dev); > > > unsigned reg, val; > > > char buf[80]; > > > int ret; > > > @@ -351,10 +366,10 @@ static ssize_t iio_debugfs_write_reg(struct > > > file > > > *file, > > > > > > switch (ret) { > > > case 1: > > > - indio_dev->cached_reg_addr = reg; > > > + iio_dev_opaque->cached_reg_addr = reg; > > > break; > > > case 2: > > > - indio_dev->cached_reg_addr = reg; > > > + iio_dev_opaque->cached_reg_addr = reg; > > > ret = indio_dev->info->debugfs_reg_access(indio_dev, reg, > > > val, NULL); > > > if (ret) { > > > @@ -378,23 +393,28 @@ static const struct file_operations > > > iio_debugfs_reg_fops = { > > > > > > static void iio_device_unregister_debugfs(struct iio_dev *indio_dev) > > > { > > > - debugfs_remove_recursive(indio_dev->debugfs_dentry); > > > + struct iio_dev_opaque *iio_dev_opaque = > > > to_iio_dev_opaque(indio_dev); > > > + debugfs_remove_recursive(iio_dev_opaque->debugfs_dentry); > > > } > > > > > > static void iio_device_register_debugfs(struct iio_dev *indio_dev) > > > { > > > + struct iio_dev_opaque *iio_dev_opaque; > > > + > > > if (indio_dev->info->debugfs_reg_access == NULL) > > > return; > > > > > > if (!iio_debugfs_dentry) > > > return; > > > > > > - indio_dev->debugfs_dentry = > > > + iio_dev_opaque = to_iio_dev_opaque(indio_dev); > > > + > > > + iio_dev_opaque->debugfs_dentry = > > > debugfs_create_dir(dev_name(&indio_dev->dev), > > > iio_debugfs_dentry); > > > > > > debugfs_create_file("direct_reg_access", 0644, > > > - indio_dev->debugfs_dentry, indio_dev, > > > + iio_dev_opaque->debugfs_dentry, indio_dev, > > > &iio_debugfs_reg_fops); > > > } > > > #else > > > diff --git a/include/linux/iio/iio-opaque.h b/include/linux/iio/iio- > > > opaque.h > > > index 1375674f14cd..b3f234b4c1e9 100644 > > > --- a/include/linux/iio/iio-opaque.h > > > +++ b/include/linux/iio/iio-opaque.h > > > @@ -6,9 +6,19 @@ > > > /** > > > * struct iio_dev_opaque - industrial I/O device opaque information > > > * @indio_dev: public industrial I/O device > > > information > > > + * @debugfs_dentry: device specific debugfs dentry > > > + * @cached_reg_addr: cached register address for debugfs > > > reads > > > + * @read_buf: read buffer to be used for the > > > initial reg read > > > + * @read_buf_len: data length in @read_buf > > > */ > > > struct iio_dev_opaque { > > > struct iio_dev indio_dev; > > > +#if defined(CONFIG_DEBUG_FS) > > > + struct dentry *debugfs_dentry; > > > + unsigned cached_reg_addr; > > > + char read_buf[20]; > > > + unsigned int read_buf_len; > > > +#endif > > > }; > > > > > > #define to_iio_dev_opaque(indio_dev) \ > > > diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h > > > index 86112e35ae5f..bb0aae11a111 100644 > > > --- a/include/linux/iio/iio.h > > > +++ b/include/linux/iio/iio.h > > > @@ -520,8 +520,6 @@ struct iio_buffer_setup_ops { > > > * @groups: [INTERN] attribute groups > > > * @groupcounter: [INTERN] index of next attribute group > > > * @flags: [INTERN] file ops related flags including > > > busy > > > flag. > > > - * @debugfs_dentry: [INTERN] device specific debugfs dentry. > > > - * @cached_reg_addr: [INTERN] cached register address for > > > debugfs reads. > > > * @priv: [DRIVER] reference to driver's private > > > information > > > * **MUST** be accessed **ONLY** via > > > iio_priv() helper > > > */ > > > @@ -567,12 +565,6 @@ struct iio_dev { > > > int groupcounter; > > > > > > unsigned long flags; > > > -#if defined(CONFIG_DEBUG_FS) > > > - struct dentry *debugfs_dentry; > > > - unsigned cached_reg_addr; > > > - char read_buf[20]; > > > - unsigned int read_buf_len; > > > -#endif > > > void *priv; > > > }; > > > > > > @@ -727,10 +719,7 @@ static inline bool iio_buffer_enabled(struct > > > iio_dev > > > *indio_dev) > > > * @indio_dev: IIO device structure for device > > > **/ > > > #if defined(CONFIG_DEBUG_FS) > > > -static inline struct dentry *iio_get_debugfs_dentry(struct iio_dev > > > *indio_dev) > > > -{ > > > - return indio_dev->debugfs_dentry; > > > -} > > > +struct dentry *iio_get_debugfs_dentry(struct iio_dev *indio_dev); > > > #else > > > static inline struct dentry *iio_get_debugfs_dentry(struct iio_dev > > > *indio_dev) > > > {
diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c index 27005ba4d09c..64174052641a 100644 --- a/drivers/iio/industrialio-core.c +++ b/drivers/iio/industrialio-core.c @@ -165,6 +165,19 @@ static const char * const iio_chan_info_postfix[] = { [IIO_CHAN_INFO_THERMOCOUPLE_TYPE] = "thermocouple_type", }; +#if !defined(CONFIG_DEBUG_FS) +/** + * There's also a CONFIG_DEBUG_FS guard in include/linux/iio/iio.h for + * iio_get_debugfs_dentry() to make it inline if CONFIG_DEBUG_FS is undefined + */ +struct dentry *iio_get_debugfs_dentry(struct iio_dev *indio_dev) +{ + struct iio_dev_opaque *iio_dev_opaque = to_iio_dev_opaque(indio_dev); + return iio_dev_opaque->debugfs_dentry; +} +EXPORT_SYMBOL_GPL(iio_get_debugfs_dentry); +#endif + /** * iio_find_channel_from_si() - get channel from its scan index * @indio_dev: device @@ -308,35 +321,37 @@ static ssize_t iio_debugfs_read_reg(struct file *file, char __user *userbuf, size_t count, loff_t *ppos) { struct iio_dev *indio_dev = file->private_data; + struct iio_dev_opaque *iio_dev_opaque = to_iio_dev_opaque(indio_dev); unsigned val = 0; int ret; if (*ppos > 0) return simple_read_from_buffer(userbuf, count, ppos, - indio_dev->read_buf, - indio_dev->read_buf_len); + iio_dev_opaque->read_buf, + iio_dev_opaque->read_buf_len); ret = indio_dev->info->debugfs_reg_access(indio_dev, - indio_dev->cached_reg_addr, + iio_dev_opaque->cached_reg_addr, 0, &val); if (ret) { dev_err(indio_dev->dev.parent, "%s: read failed\n", __func__); return ret; } - indio_dev->read_buf_len = snprintf(indio_dev->read_buf, - sizeof(indio_dev->read_buf), - "0x%X\n", val); + iio_dev_opaque->read_buf_len = snprintf(iio_dev_opaque->read_buf, + sizeof(iio_dev_opaque->read_buf), + "0x%X\n", val); return simple_read_from_buffer(userbuf, count, ppos, - indio_dev->read_buf, - indio_dev->read_buf_len); + iio_dev_opaque->read_buf, + iio_dev_opaque->read_buf_len); } static ssize_t iio_debugfs_write_reg(struct file *file, const char __user *userbuf, size_t count, loff_t *ppos) { struct iio_dev *indio_dev = file->private_data; + struct iio_dev_opaque *iio_dev_opaque = to_iio_dev_opaque(indio_dev); unsigned reg, val; char buf[80]; int ret; @@ -351,10 +366,10 @@ static ssize_t iio_debugfs_write_reg(struct file *file, switch (ret) { case 1: - indio_dev->cached_reg_addr = reg; + iio_dev_opaque->cached_reg_addr = reg; break; case 2: - indio_dev->cached_reg_addr = reg; + iio_dev_opaque->cached_reg_addr = reg; ret = indio_dev->info->debugfs_reg_access(indio_dev, reg, val, NULL); if (ret) { @@ -378,23 +393,28 @@ static const struct file_operations iio_debugfs_reg_fops = { static void iio_device_unregister_debugfs(struct iio_dev *indio_dev) { - debugfs_remove_recursive(indio_dev->debugfs_dentry); + struct iio_dev_opaque *iio_dev_opaque = to_iio_dev_opaque(indio_dev); + debugfs_remove_recursive(iio_dev_opaque->debugfs_dentry); } static void iio_device_register_debugfs(struct iio_dev *indio_dev) { + struct iio_dev_opaque *iio_dev_opaque; + if (indio_dev->info->debugfs_reg_access == NULL) return; if (!iio_debugfs_dentry) return; - indio_dev->debugfs_dentry = + iio_dev_opaque = to_iio_dev_opaque(indio_dev); + + iio_dev_opaque->debugfs_dentry = debugfs_create_dir(dev_name(&indio_dev->dev), iio_debugfs_dentry); debugfs_create_file("direct_reg_access", 0644, - indio_dev->debugfs_dentry, indio_dev, + iio_dev_opaque->debugfs_dentry, indio_dev, &iio_debugfs_reg_fops); } #else diff --git a/include/linux/iio/iio-opaque.h b/include/linux/iio/iio-opaque.h index 1375674f14cd..b3f234b4c1e9 100644 --- a/include/linux/iio/iio-opaque.h +++ b/include/linux/iio/iio-opaque.h @@ -6,9 +6,19 @@ /** * struct iio_dev_opaque - industrial I/O device opaque information * @indio_dev: public industrial I/O device information + * @debugfs_dentry: device specific debugfs dentry + * @cached_reg_addr: cached register address for debugfs reads + * @read_buf: read buffer to be used for the initial reg read + * @read_buf_len: data length in @read_buf */ struct iio_dev_opaque { struct iio_dev indio_dev; +#if defined(CONFIG_DEBUG_FS) + struct dentry *debugfs_dentry; + unsigned cached_reg_addr; + char read_buf[20]; + unsigned int read_buf_len; +#endif }; #define to_iio_dev_opaque(indio_dev) \ diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h index 86112e35ae5f..bb0aae11a111 100644 --- a/include/linux/iio/iio.h +++ b/include/linux/iio/iio.h @@ -520,8 +520,6 @@ struct iio_buffer_setup_ops { * @groups: [INTERN] attribute groups * @groupcounter: [INTERN] index of next attribute group * @flags: [INTERN] file ops related flags including busy flag. - * @debugfs_dentry: [INTERN] device specific debugfs dentry. - * @cached_reg_addr: [INTERN] cached register address for debugfs reads. * @priv: [DRIVER] reference to driver's private information * **MUST** be accessed **ONLY** via iio_priv() helper */ @@ -567,12 +565,6 @@ struct iio_dev { int groupcounter; unsigned long flags; -#if defined(CONFIG_DEBUG_FS) - struct dentry *debugfs_dentry; - unsigned cached_reg_addr; - char read_buf[20]; - unsigned int read_buf_len; -#endif void *priv; }; @@ -727,10 +719,7 @@ static inline bool iio_buffer_enabled(struct iio_dev *indio_dev) * @indio_dev: IIO device structure for device **/ #if defined(CONFIG_DEBUG_FS) -static inline struct dentry *iio_get_debugfs_dentry(struct iio_dev *indio_dev) -{ - return indio_dev->debugfs_dentry; -} +struct dentry *iio_get_debugfs_dentry(struct iio_dev *indio_dev); #else static inline struct dentry *iio_get_debugfs_dentry(struct iio_dev *indio_dev) {
This change moves all iio_dev debugfs fields to the iio_dev_priv object. It's not the biggest advantage yet (to the whole thing of abstractization) but it's a start. The iio_get_debugfs_dentry() function (which is moved in industrialio-core.c) needs to also be guarded against the CONFIG_DEBUG_FS symbol, when it isn't defined. We do want to keep the inline definition in the iio.h header, so that the compiler can better infer when to compile out debugfs code that is related to the IIO debugfs directory. Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com> --- drivers/iio/industrialio-core.c | 46 +++++++++++++++++++++++---------- include/linux/iio/iio-opaque.h | 10 +++++++ include/linux/iio/iio.h | 13 +--------- 3 files changed, 44 insertions(+), 25 deletions(-)