* [PATCH 1/2 v2] iio: event: use short-hand variable in iio_device_{un}register_eventset functions
@ 2020-09-21 10:31 Alexandru Ardelean
2020-09-21 10:31 ` [PATCH 2/2 v2] iio: event: NULL-ify IIO device's event_interface ref during unregister Alexandru Ardelean
2020-09-23 20:12 ` [PATCH 1/2 v2] iio: event: use short-hand variable in iio_device_{un}register_eventset functions Jonathan Cameron
0 siblings, 2 replies; 4+ messages in thread
From: Alexandru Ardelean @ 2020-09-21 10:31 UTC (permalink / raw)
To: linux-iio, linux-kernel; +Cc: jic23, Alexandru Ardelean
With the recent 'iio_dev_opaque' variable name, these two functions are
looking a bit ugly.
This change uses an 'ev_int' variable for the
iio_device_{un}register_eventset functions to make the code a little easier
to read.
Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---
Changelog v1 -> v2:
* move 'iio_dev_opaque->event_interface = ev_int;' assigment right after
allocation to avoid crash; 'iio_dev_opaque->event_interface' is accessed
after init
drivers/iio/industrialio-event.c | 50 +++++++++++++++-----------------
1 file changed, 24 insertions(+), 26 deletions(-)
diff --git a/drivers/iio/industrialio-event.c b/drivers/iio/industrialio-event.c
index 2ab4d4c44427..a85919eb7c4a 100644
--- a/drivers/iio/industrialio-event.c
+++ b/drivers/iio/industrialio-event.c
@@ -477,6 +477,7 @@ static const char *iio_event_group_name = "events";
int iio_device_register_eventset(struct iio_dev *indio_dev)
{
struct iio_dev_opaque *iio_dev_opaque = to_iio_dev_opaque(indio_dev);
+ struct iio_event_interface *ev_int;
struct iio_dev_attr *p;
int ret = 0, attrcount_orig = 0, attrcount, attrn;
struct attribute **attr;
@@ -485,14 +486,15 @@ int iio_device_register_eventset(struct iio_dev *indio_dev)
iio_check_for_dynamic_events(indio_dev)))
return 0;
- iio_dev_opaque->event_interface =
- kzalloc(sizeof(struct iio_event_interface), GFP_KERNEL);
- if (iio_dev_opaque->event_interface == NULL)
+ ev_int = kzalloc(sizeof(struct iio_event_interface), GFP_KERNEL);
+ if (ev_int == NULL)
return -ENOMEM;
- INIT_LIST_HEAD(&iio_dev_opaque->event_interface->dev_attr_list);
+ iio_dev_opaque->event_interface = ev_int;
+
+ INIT_LIST_HEAD(&ev_int->dev_attr_list);
- iio_setup_ev_int(iio_dev_opaque->event_interface);
+ iio_setup_ev_int(ev_int);
if (indio_dev->info->event_attrs != NULL) {
attr = indio_dev->info->event_attrs->attrs;
while (*attr++ != NULL)
@@ -506,34 +508,29 @@ int iio_device_register_eventset(struct iio_dev *indio_dev)
attrcount += ret;
}
- iio_dev_opaque->event_interface->group.name = iio_event_group_name;
- iio_dev_opaque->event_interface->group.attrs = kcalloc(attrcount + 1,
- sizeof(iio_dev_opaque->event_interface->group.attrs[0]),
- GFP_KERNEL);
- if (iio_dev_opaque->event_interface->group.attrs == NULL) {
+ ev_int->group.name = iio_event_group_name;
+ ev_int->group.attrs = kcalloc(attrcount + 1,
+ sizeof(ev_int->group.attrs[0]),
+ GFP_KERNEL);
+ if (ev_int->group.attrs == NULL) {
ret = -ENOMEM;
goto error_free_setup_event_lines;
}
if (indio_dev->info->event_attrs)
- memcpy(iio_dev_opaque->event_interface->group.attrs,
+ memcpy(ev_int->group.attrs,
indio_dev->info->event_attrs->attrs,
- sizeof(iio_dev_opaque->event_interface->group.attrs[0])
- *attrcount_orig);
+ sizeof(ev_int->group.attrs[0]) * attrcount_orig);
attrn = attrcount_orig;
/* Add all elements from the list. */
- list_for_each_entry(p,
- &iio_dev_opaque->event_interface->dev_attr_list,
- l)
- iio_dev_opaque->event_interface->group.attrs[attrn++] =
- &p->dev_attr.attr;
- indio_dev->groups[indio_dev->groupcounter++] =
- &iio_dev_opaque->event_interface->group;
+ list_for_each_entry(p, &ev_int->dev_attr_list, l)
+ ev_int->group.attrs[attrn++] = &p->dev_attr.attr;
+ indio_dev->groups[indio_dev->groupcounter++] = &ev_int->group;
return 0;
error_free_setup_event_lines:
- iio_free_chan_devattr_list(&iio_dev_opaque->event_interface->dev_attr_list);
- kfree(iio_dev_opaque->event_interface);
+ iio_free_chan_devattr_list(&ev_int->dev_attr_list);
+ kfree(ev_int);
iio_dev_opaque->event_interface = NULL;
return ret;
}
@@ -557,10 +554,11 @@ void iio_device_wakeup_eventset(struct iio_dev *indio_dev)
void iio_device_unregister_eventset(struct iio_dev *indio_dev)
{
struct iio_dev_opaque *iio_dev_opaque = to_iio_dev_opaque(indio_dev);
+ struct iio_event_interface *ev_int = iio_dev_opaque->event_interface;
- if (iio_dev_opaque->event_interface == NULL)
+ if (ev_int == NULL)
return;
- iio_free_chan_devattr_list(&iio_dev_opaque->event_interface->dev_attr_list);
- kfree(iio_dev_opaque->event_interface->group.attrs);
- kfree(iio_dev_opaque->event_interface);
+ iio_free_chan_devattr_list(&ev_int->dev_attr_list);
+ kfree(ev_int->group.attrs);
+ kfree(ev_int);
}
--
2.25.1
^ permalink raw reply related [flat|nested] 4+ messages in thread* [PATCH 2/2 v2] iio: event: NULL-ify IIO device's event_interface ref during unregister
2020-09-21 10:31 [PATCH 1/2 v2] iio: event: use short-hand variable in iio_device_{un}register_eventset functions Alexandru Ardelean
@ 2020-09-21 10:31 ` Alexandru Ardelean
2020-09-23 20:12 ` [PATCH 1/2 v2] iio: event: use short-hand variable in iio_device_{un}register_eventset functions Jonathan Cameron
1 sibling, 0 replies; 4+ messages in thread
From: Alexandru Ardelean @ 2020-09-21 10:31 UTC (permalink / raw)
To: linux-iio, linux-kernel; +Cc: jic23, Alexandru Ardelean
Though we know that the iio_device_unregister_eventset() call is followed
by the free-ing of the IIO device object, we should not make this
assumption in the iio_device_unregister_eventset() function. It should
allow for the clean unregistering of the event-set, allowing a re-register
should we decide to implement this at some point later.
Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---
drivers/iio/industrialio-event.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/iio/industrialio-event.c b/drivers/iio/industrialio-event.c
index a85919eb7c4a..99ba657b8568 100644
--- a/drivers/iio/industrialio-event.c
+++ b/drivers/iio/industrialio-event.c
@@ -561,4 +561,5 @@ void iio_device_unregister_eventset(struct iio_dev *indio_dev)
iio_free_chan_devattr_list(&ev_int->dev_attr_list);
kfree(ev_int->group.attrs);
kfree(ev_int);
+ iio_dev_opaque->event_interface = NULL;
}
--
2.25.1
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH 1/2 v2] iio: event: use short-hand variable in iio_device_{un}register_eventset functions
2020-09-21 10:31 [PATCH 1/2 v2] iio: event: use short-hand variable in iio_device_{un}register_eventset functions Alexandru Ardelean
2020-09-21 10:31 ` [PATCH 2/2 v2] iio: event: NULL-ify IIO device's event_interface ref during unregister Alexandru Ardelean
@ 2020-09-23 20:12 ` Jonathan Cameron
2020-09-24 6:53 ` Alexandru Ardelean
1 sibling, 1 reply; 4+ messages in thread
From: Jonathan Cameron @ 2020-09-23 20:12 UTC (permalink / raw)
To: Alexandru Ardelean; +Cc: linux-iio, linux-kernel
On Mon, 21 Sep 2020 13:31:55 +0300
Alexandru Ardelean <alexandru.ardelean@analog.com> wrote:
> With the recent 'iio_dev_opaque' variable name, these two functions are
> looking a bit ugly.
>
> This change uses an 'ev_int' variable for the
> iio_device_{un}register_eventset functions to make the code a little easier
> to read.
>
> Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
Seems sensible. Series applied to the togreg branch of iio.git and pushed out as
testing. Not sure if this will make it into a final pull request for this
cycle or not. Kind of depends what Linus says on Sunday about whether we are
going to see an rc8.
Thanks,
Jonathan
> ---
>
> Changelog v1 -> v2:
> * move 'iio_dev_opaque->event_interface = ev_int;' assigment right after
> allocation to avoid crash; 'iio_dev_opaque->event_interface' is accessed
> after init
>
> drivers/iio/industrialio-event.c | 50 +++++++++++++++-----------------
> 1 file changed, 24 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/iio/industrialio-event.c b/drivers/iio/industrialio-event.c
> index 2ab4d4c44427..a85919eb7c4a 100644
> --- a/drivers/iio/industrialio-event.c
> +++ b/drivers/iio/industrialio-event.c
> @@ -477,6 +477,7 @@ static const char *iio_event_group_name = "events";
> int iio_device_register_eventset(struct iio_dev *indio_dev)
> {
> struct iio_dev_opaque *iio_dev_opaque = to_iio_dev_opaque(indio_dev);
> + struct iio_event_interface *ev_int;
> struct iio_dev_attr *p;
> int ret = 0, attrcount_orig = 0, attrcount, attrn;
> struct attribute **attr;
> @@ -485,14 +486,15 @@ int iio_device_register_eventset(struct iio_dev *indio_dev)
> iio_check_for_dynamic_events(indio_dev)))
> return 0;
>
> - iio_dev_opaque->event_interface =
> - kzalloc(sizeof(struct iio_event_interface), GFP_KERNEL);
> - if (iio_dev_opaque->event_interface == NULL)
> + ev_int = kzalloc(sizeof(struct iio_event_interface), GFP_KERNEL);
> + if (ev_int == NULL)
> return -ENOMEM;
>
> - INIT_LIST_HEAD(&iio_dev_opaque->event_interface->dev_attr_list);
> + iio_dev_opaque->event_interface = ev_int;
> +
> + INIT_LIST_HEAD(&ev_int->dev_attr_list);
>
> - iio_setup_ev_int(iio_dev_opaque->event_interface);
> + iio_setup_ev_int(ev_int);
> if (indio_dev->info->event_attrs != NULL) {
> attr = indio_dev->info->event_attrs->attrs;
> while (*attr++ != NULL)
> @@ -506,34 +508,29 @@ int iio_device_register_eventset(struct iio_dev *indio_dev)
> attrcount += ret;
> }
>
> - iio_dev_opaque->event_interface->group.name = iio_event_group_name;
> - iio_dev_opaque->event_interface->group.attrs = kcalloc(attrcount + 1,
> - sizeof(iio_dev_opaque->event_interface->group.attrs[0]),
> - GFP_KERNEL);
> - if (iio_dev_opaque->event_interface->group.attrs == NULL) {
> + ev_int->group.name = iio_event_group_name;
> + ev_int->group.attrs = kcalloc(attrcount + 1,
> + sizeof(ev_int->group.attrs[0]),
> + GFP_KERNEL);
> + if (ev_int->group.attrs == NULL) {
> ret = -ENOMEM;
> goto error_free_setup_event_lines;
> }
> if (indio_dev->info->event_attrs)
> - memcpy(iio_dev_opaque->event_interface->group.attrs,
> + memcpy(ev_int->group.attrs,
> indio_dev->info->event_attrs->attrs,
> - sizeof(iio_dev_opaque->event_interface->group.attrs[0])
> - *attrcount_orig);
> + sizeof(ev_int->group.attrs[0]) * attrcount_orig);
> attrn = attrcount_orig;
> /* Add all elements from the list. */
> - list_for_each_entry(p,
> - &iio_dev_opaque->event_interface->dev_attr_list,
> - l)
> - iio_dev_opaque->event_interface->group.attrs[attrn++] =
> - &p->dev_attr.attr;
> - indio_dev->groups[indio_dev->groupcounter++] =
> - &iio_dev_opaque->event_interface->group;
> + list_for_each_entry(p, &ev_int->dev_attr_list, l)
> + ev_int->group.attrs[attrn++] = &p->dev_attr.attr;
> + indio_dev->groups[indio_dev->groupcounter++] = &ev_int->group;
>
> return 0;
>
> error_free_setup_event_lines:
> - iio_free_chan_devattr_list(&iio_dev_opaque->event_interface->dev_attr_list);
> - kfree(iio_dev_opaque->event_interface);
> + iio_free_chan_devattr_list(&ev_int->dev_attr_list);
> + kfree(ev_int);
> iio_dev_opaque->event_interface = NULL;
> return ret;
> }
> @@ -557,10 +554,11 @@ void iio_device_wakeup_eventset(struct iio_dev *indio_dev)
> void iio_device_unregister_eventset(struct iio_dev *indio_dev)
> {
> struct iio_dev_opaque *iio_dev_opaque = to_iio_dev_opaque(indio_dev);
> + struct iio_event_interface *ev_int = iio_dev_opaque->event_interface;
>
> - if (iio_dev_opaque->event_interface == NULL)
> + if (ev_int == NULL)
> return;
> - iio_free_chan_devattr_list(&iio_dev_opaque->event_interface->dev_attr_list);
> - kfree(iio_dev_opaque->event_interface->group.attrs);
> - kfree(iio_dev_opaque->event_interface);
> + iio_free_chan_devattr_list(&ev_int->dev_attr_list);
> + kfree(ev_int->group.attrs);
> + kfree(ev_int);
> }
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH 1/2 v2] iio: event: use short-hand variable in iio_device_{un}register_eventset functions
2020-09-23 20:12 ` [PATCH 1/2 v2] iio: event: use short-hand variable in iio_device_{un}register_eventset functions Jonathan Cameron
@ 2020-09-24 6:53 ` Alexandru Ardelean
0 siblings, 0 replies; 4+ messages in thread
From: Alexandru Ardelean @ 2020-09-24 6:53 UTC (permalink / raw)
To: Jonathan Cameron; +Cc: Alexandru Ardelean, linux-iio, LKML
On Wed, Sep 23, 2020 at 11:13 PM Jonathan Cameron <jic23@kernel.org> wrote:
>
> On Mon, 21 Sep 2020 13:31:55 +0300
> Alexandru Ardelean <alexandru.ardelean@analog.com> wrote:
>
> > With the recent 'iio_dev_opaque' variable name, these two functions are
> > looking a bit ugly.
> >
> > This change uses an 'ev_int' variable for the
> > iio_device_{un}register_eventset functions to make the code a little easier
> > to read.
> >
> > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
>
> Seems sensible. Series applied to the togreg branch of iio.git and pushed out as
> testing. Not sure if this will make it into a final pull request for this
> cycle or not. Kind of depends what Linus says on Sunday about whether we are
> going to see an rc8.
>
No hurry from my side when this goes in.
This is in a longer series of things that do with the whole
multiple-IIO-buffers-per-IIO-device.
I might need to take care [again] so that I don't block myself again
with too many small/parallel series.
> Thanks,
>
> Jonathan
>
> > ---
> >
> > Changelog v1 -> v2:
> > * move 'iio_dev_opaque->event_interface = ev_int;' assigment right after
> > allocation to avoid crash; 'iio_dev_opaque->event_interface' is accessed
> > after init
> >
> > drivers/iio/industrialio-event.c | 50 +++++++++++++++-----------------
> > 1 file changed, 24 insertions(+), 26 deletions(-)
> >
> > diff --git a/drivers/iio/industrialio-event.c b/drivers/iio/industrialio-event.c
> > index 2ab4d4c44427..a85919eb7c4a 100644
> > --- a/drivers/iio/industrialio-event.c
> > +++ b/drivers/iio/industrialio-event.c
> > @@ -477,6 +477,7 @@ static const char *iio_event_group_name = "events";
> > int iio_device_register_eventset(struct iio_dev *indio_dev)
> > {
> > struct iio_dev_opaque *iio_dev_opaque = to_iio_dev_opaque(indio_dev);
> > + struct iio_event_interface *ev_int;
> > struct iio_dev_attr *p;
> > int ret = 0, attrcount_orig = 0, attrcount, attrn;
> > struct attribute **attr;
> > @@ -485,14 +486,15 @@ int iio_device_register_eventset(struct iio_dev *indio_dev)
> > iio_check_for_dynamic_events(indio_dev)))
> > return 0;
> >
> > - iio_dev_opaque->event_interface =
> > - kzalloc(sizeof(struct iio_event_interface), GFP_KERNEL);
> > - if (iio_dev_opaque->event_interface == NULL)
> > + ev_int = kzalloc(sizeof(struct iio_event_interface), GFP_KERNEL);
> > + if (ev_int == NULL)
> > return -ENOMEM;
> >
> > - INIT_LIST_HEAD(&iio_dev_opaque->event_interface->dev_attr_list);
> > + iio_dev_opaque->event_interface = ev_int;
> > +
> > + INIT_LIST_HEAD(&ev_int->dev_attr_list);
> >
> > - iio_setup_ev_int(iio_dev_opaque->event_interface);
> > + iio_setup_ev_int(ev_int);
> > if (indio_dev->info->event_attrs != NULL) {
> > attr = indio_dev->info->event_attrs->attrs;
> > while (*attr++ != NULL)
> > @@ -506,34 +508,29 @@ int iio_device_register_eventset(struct iio_dev *indio_dev)
> > attrcount += ret;
> > }
> >
> > - iio_dev_opaque->event_interface->group.name = iio_event_group_name;
> > - iio_dev_opaque->event_interface->group.attrs = kcalloc(attrcount + 1,
> > - sizeof(iio_dev_opaque->event_interface->group.attrs[0]),
> > - GFP_KERNEL);
> > - if (iio_dev_opaque->event_interface->group.attrs == NULL) {
> > + ev_int->group.name = iio_event_group_name;
> > + ev_int->group.attrs = kcalloc(attrcount + 1,
> > + sizeof(ev_int->group.attrs[0]),
> > + GFP_KERNEL);
> > + if (ev_int->group.attrs == NULL) {
> > ret = -ENOMEM;
> > goto error_free_setup_event_lines;
> > }
> > if (indio_dev->info->event_attrs)
> > - memcpy(iio_dev_opaque->event_interface->group.attrs,
> > + memcpy(ev_int->group.attrs,
> > indio_dev->info->event_attrs->attrs,
> > - sizeof(iio_dev_opaque->event_interface->group.attrs[0])
> > - *attrcount_orig);
> > + sizeof(ev_int->group.attrs[0]) * attrcount_orig);
> > attrn = attrcount_orig;
> > /* Add all elements from the list. */
> > - list_for_each_entry(p,
> > - &iio_dev_opaque->event_interface->dev_attr_list,
> > - l)
> > - iio_dev_opaque->event_interface->group.attrs[attrn++] =
> > - &p->dev_attr.attr;
> > - indio_dev->groups[indio_dev->groupcounter++] =
> > - &iio_dev_opaque->event_interface->group;
> > + list_for_each_entry(p, &ev_int->dev_attr_list, l)
> > + ev_int->group.attrs[attrn++] = &p->dev_attr.attr;
> > + indio_dev->groups[indio_dev->groupcounter++] = &ev_int->group;
> >
> > return 0;
> >
> > error_free_setup_event_lines:
> > - iio_free_chan_devattr_list(&iio_dev_opaque->event_interface->dev_attr_list);
> > - kfree(iio_dev_opaque->event_interface);
> > + iio_free_chan_devattr_list(&ev_int->dev_attr_list);
> > + kfree(ev_int);
> > iio_dev_opaque->event_interface = NULL;
> > return ret;
> > }
> > @@ -557,10 +554,11 @@ void iio_device_wakeup_eventset(struct iio_dev *indio_dev)
> > void iio_device_unregister_eventset(struct iio_dev *indio_dev)
> > {
> > struct iio_dev_opaque *iio_dev_opaque = to_iio_dev_opaque(indio_dev);
> > + struct iio_event_interface *ev_int = iio_dev_opaque->event_interface;
> >
> > - if (iio_dev_opaque->event_interface == NULL)
> > + if (ev_int == NULL)
> > return;
> > - iio_free_chan_devattr_list(&iio_dev_opaque->event_interface->dev_attr_list);
> > - kfree(iio_dev_opaque->event_interface->group.attrs);
> > - kfree(iio_dev_opaque->event_interface);
> > + iio_free_chan_devattr_list(&ev_int->dev_attr_list);
> > + kfree(ev_int->group.attrs);
> > + kfree(ev_int);
> > }
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-09-24 6:53 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-09-21 10:31 [PATCH 1/2 v2] iio: event: use short-hand variable in iio_device_{un}register_eventset functions Alexandru Ardelean
2020-09-21 10:31 ` [PATCH 2/2 v2] iio: event: NULL-ify IIO device's event_interface ref during unregister Alexandru Ardelean
2020-09-23 20:12 ` [PATCH 1/2 v2] iio: event: use short-hand variable in iio_device_{un}register_eventset functions Jonathan Cameron
2020-09-24 6:53 ` Alexandru Ardelean
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox