On Tue, Jul 21, 2020 at 12:47 PM Dmitry Baryshkov <dbaryshkov@xxxxxxxxx> wrote: > > Hello, > > On Tue, Jun 30, 2020 at 07:57:03AM +0300, Alexandru Ardelean wrote: > > 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'. > > > > Sometimes users copy drivers that are 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_opaque' which wraps a public > > 'struct iio_dev' object. > > > > In the next series, some fields will be moved to this new struct, each with > > it's own rework. > > [skipped] > > > > > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@xxxxxxxxxx> > > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@xxxxxxxxxx> > > --- > > drivers/iio/industrialio-core.c | 19 +++++++++++++------ > > include/linux/iio/iio-opaque.h | 17 +++++++++++++++++ > > include/linux/iio/iio.h | 6 +++++- > > 3 files changed, 35 insertions(+), 7 deletions(-) > > create mode 100644 include/linux/iio/iio-opaque.h > > > > diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c > > index 75661661aaba..33e2953cf021 100644 > > --- a/drivers/iio/industrialio-core.c > > +++ b/drivers/iio/industrialio-core.c > > [skipped] > > > @@ -1506,11 +1510,14 @@ struct iio_dev *iio_device_alloc(struct device *parent, 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_opaque = kzalloc(alloc_size, GFP_KERNEL); > > + if (!iio_dev_opaque) > > return NULL; > > > > - dev->dev.parent = parent; > > This chunk (together with 8cb631ccbb1952b6422917f2ed16f760d84a762e and > d3be83244c7dfe686d23f1c0bac75915587fc044) break devicetree bindings of > IIO clients. After these changes there are no links between devicetree > nodes and corresponding IIO channels. I'd kindly ask to restore this > dev->dev.parent assignment (which restores of node binding). > Ack. Will send a fix. Thanks for catching this. Apologies for the breakage. It seems that I was tripping quite a bit with too many patchsets. > > + dev = &iio_dev_opaque->indio_dev; > > + dev->priv = (char *)iio_dev_opaque + > > + ALIGN(sizeof(struct iio_dev_opaque), IIO_ALIGN); > > + > > dev->dev.groups = dev->groups; > > dev->dev.type = &iio_device_type; > > dev->dev.bus = &iio_bus_type; > > -- > With best wishes > Dmitry