Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20200630045708.14166-1-alexandru.ardelean@analog.com>
 <20200630045708.14166-3-alexandru.ardelean@analog.com> <20200721094643.GA1598380@eriador.lumag.spb.ru>
In-Reply-To: <20200721094643.GA1598380@eriador.lumag.spb.ru>
From:   Alexandru Ardelean <ardeleanalex@gmail.com>
Date:   Tue, 21 Jul 2020 13:00:11 +0300
Message-ID: <CA+U=DsqLAnXG0BZvXw5=_rUS_N-1qSs2Xxu7-x2qA2HH7pPyVA@mail.gmail.com>
Subject: Re: [PATCH v4 2/7] iio: core: wrap IIO device into an iio_dev_opaque object
To:     Dmitry Baryshkov <dbaryshkov@gmail.com>
Cc:     Alexandru Ardelean <alexandru.ardelean@analog.com>,
        linux-iio@vger.kernel.org, LKML <linux-kernel@vger.kernel.org>,
        Jonathan Cameron <jic23@kernel.org>,
        Lars-Peter Clausen <lars@metafoo.de>,
        Peter Meerwald-Stadler <pmeerw@pmeerw.net>,
        Hartmut Knaack <knaack.h@gmx.de>,
        Jonathan Cameron <Jonathan.Cameron@huawei.com>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Tue, Jul 21, 2020 at 12:47 PM Dmitry Baryshkov <dbaryshkov@gmail.com> 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@analog.com>
> > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > ---
> >  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