[PATCH 1/3] iio: core: wrap IIO device into a iio_dev_priv object

From: Alexandru Ardelean
Date: Fri May 08 2020 - 10:13:12 EST


There are plenty of bad designs we want to discourage or not have to review
manually usually about accessing private (marked as [INTERN]) fields of
'struct iio_dev'.

This is difficult, as a lot of users copy drivers, and not always the best
examples.

A better idea is to hide those fields into the framework.
For 'struct iio_dev' this is a 'struct iio_dev_priv' which wraps a public
'struct iio_dev' object.

In the next patches, some fields will be moved to this new struct, each
with it's own rework.

This rework will not be complete[-able] for a while, as many fields need
some drivers to be reworked in order to finalize them
(e.g. 'indio_dev->mlock').

But some fields can already be moved, and in time, all of them may get
there (in the 'struct iio_dev_priv' object).

We also need to hide the implementations for 'iio_priv()' &
'iio_priv_to_dev()', as the pointer arithmetic will not match once things
are moved.

Signed-off-by: Alexandru Ardelean <alexandru.ardelean@xxxxxxxxxx>
---

Just as a note here, I've been running this patchset without a problem
for 2 weeks now in a work branch.
But it's only been a setup, so no idea if some other thing may cause
bigger issues.

This small patchset is meant to kickstart this, for GSoC people or for
people wanting to start contributing to IIO.

drivers/iio/iio_core.h | 11 +++++++++++
drivers/iio/industrialio-core.c | 32 +++++++++++++++++++++++++++-----
include/linux/iio/iio.h | 12 ++----------
3 files changed, 40 insertions(+), 15 deletions(-)

diff --git a/drivers/iio/iio_core.h b/drivers/iio/iio_core.h
index fd9a5f1d5e51..84f3b4590c05 100644
--- a/drivers/iio/iio_core.h
+++ b/drivers/iio/iio_core.h
@@ -17,6 +17,17 @@ struct iio_dev;

extern struct device_type iio_device_type;

+/**
+ * struct iio_dev_priv - industrial I/O device private information
+ * @indio_dev: public IIO device object
+ */
+struct iio_dev_priv {
+ struct iio_dev indio_dev;
+};
+
+#define to_iio_dev_priv(indio_dev) \
+ container_of(indio_dev, struct iio_dev_priv, indio_dev)
+
int __iio_add_chan_devattr(const char *postfix,
struct iio_chan_spec const *chan,
ssize_t (*func)(struct device *dev,
diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
index 462d3e810013..f0888dd84d3d 100644
--- a/drivers/iio/industrialio-core.c
+++ b/drivers/iio/industrialio-core.c
@@ -164,6 +164,23 @@ static const char * const iio_chan_info_postfix[] = {
[IIO_CHAN_INFO_THERMOCOUPLE_TYPE] = "thermocouple_type",
};

+
+void *iio_priv(const struct iio_dev *indio_dev)
+{
+ struct iio_dev_priv *iio_dev_priv = to_iio_dev_priv(indio_dev);
+ return (char *)iio_dev_priv + ALIGN(sizeof(struct iio_dev_priv), IIO_ALIGN);
+}
+EXPORT_SYMBOL_GPL(iio_priv);
+
+struct iio_dev *iio_priv_to_dev(void *priv)
+{
+ struct iio_dev_priv *iio_dev_priv =
+ (struct iio_dev_priv *)((char *)priv -
+ ALIGN(sizeof(struct iio_dev_priv), IIO_ALIGN));
+ return &iio_dev_priv->indio_dev;
+}
+EXPORT_SYMBOL_GPL(iio_priv_to_dev);
+
/**
* iio_find_channel_from_si() - get channel from its scan index
* @indio_dev: device
@@ -1476,6 +1493,8 @@ static void iio_device_unregister_sysfs(struct iio_dev *indio_dev)
static void iio_dev_release(struct device *device)
{
struct iio_dev *indio_dev = dev_to_iio_dev(device);
+ struct iio_dev_priv *iio_dev_priv = to_iio_dev_priv(indio_dev);
+
if (indio_dev->modes & INDIO_ALL_TRIGGERED_MODES)
iio_device_unregister_trigger_consumer(indio_dev);
iio_device_unregister_eventset(indio_dev);
@@ -1484,7 +1503,7 @@ static void iio_dev_release(struct device *device)
iio_buffer_put(indio_dev->buffer);

ida_simple_remove(&iio_ida, indio_dev->id);
- kfree(indio_dev);
+ kfree(iio_dev_priv);
}

struct device_type iio_device_type = {
@@ -1498,10 +1517,11 @@ struct device_type iio_device_type = {
**/
struct iio_dev *iio_device_alloc(int sizeof_priv)
{
+ struct iio_dev_priv *iio_dev_priv;
struct iio_dev *dev;
size_t alloc_size;

- alloc_size = sizeof(struct iio_dev);
+ alloc_size = sizeof(struct iio_dev_priv);
if (sizeof_priv) {
alloc_size = ALIGN(alloc_size, IIO_ALIGN);
alloc_size += sizeof_priv;
@@ -1509,10 +1529,12 @@ struct iio_dev *iio_device_alloc(int sizeof_priv)
/* ensure 32-byte alignment of whole construct ? */
alloc_size += IIO_ALIGN - 1;

- dev = kzalloc(alloc_size, GFP_KERNEL);
- if (!dev)
+ iio_dev_priv = kzalloc(alloc_size, GFP_KERNEL);
+ if (!iio_dev_priv)
return NULL;

+ dev = &iio_dev_priv->indio_dev;
+
dev->dev.groups = dev->groups;
dev->dev.type = &iio_device_type;
dev->dev.bus = &iio_bus_type;
@@ -1526,7 +1548,7 @@ struct iio_dev *iio_device_alloc(int sizeof_priv)
if (dev->id < 0) {
/* cannot use a dev_err as the name isn't available */
pr_err("failed to get device id\n");
- kfree(dev);
+ kfree(iio_dev_priv);
return NULL;
}
dev_set_name(&dev->dev, "iio:device%d", dev->id);
diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h
index 5f9f439a4f01..38c4ea505394 100644
--- a/include/linux/iio/iio.h
+++ b/include/linux/iio/iio.h
@@ -678,16 +678,8 @@ static inline void *iio_device_get_drvdata(struct iio_dev *indio_dev)
#define IIO_ALIGN L1_CACHE_BYTES
struct iio_dev *iio_device_alloc(int sizeof_priv);

-static inline void *iio_priv(const struct iio_dev *indio_dev)
-{
- return (char *)indio_dev + ALIGN(sizeof(struct iio_dev), IIO_ALIGN);
-}
-
-static inline struct iio_dev *iio_priv_to_dev(void *priv)
-{
- return (struct iio_dev *)((char *)priv -
- ALIGN(sizeof(struct iio_dev), IIO_ALIGN));
-}
+void *iio_priv(const struct iio_dev *indio_dev);
+struct iio_dev *iio_priv_to_dev(void *priv);

void iio_device_free(struct iio_dev *indio_dev);
struct iio_dev *devm_iio_device_alloc(struct device *dev, int sizeof_priv);
--
2.17.1