Re: [PATCH v1 6/8] software node: Simplify swnode_register() a bit

[Andy Shevchenko]

On Sun, Mar 28, 2021 at 11:48 AM Greg Kroah-Hartman
 wrote:
>
> On Sun, Mar 28, 2021 at 12:20:10AM +0200, Andy Shevchenko wrote:
> > By introducing two temporary variables simplify swnode_register() a bit.
> > No functional change intended.
> >
> > Signed-off-by: Andy Shevchenko 
> > ---
> >  drivers/base/swnode.c | 11 +--
> >  1 file changed, 5 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/base/swnode.c b/drivers/base/swnode.c
> > index ae53c48f84b1..1e81aaf5f6a1 100644
> > --- a/drivers/base/swnode.c
> > +++ b/drivers/base/swnode.c
> > @@ -894,6 +894,8 @@ static struct fwnode_handle *
> >  swnode_register(const struct software_node *node, struct swnode *parent,
> >   unsigned int allocated)
> >  {
> > + struct ida *ids = parent ? &parent->child_ids : &swnode_root_ids;
> > + struct kobject *kobj_parent = parent ? &parent->kobj : NULL;
>
> ?: operations are horrid.  Please spell this out in real if statements
> so that we can properly understand and maintain them for the next 20+
> years.

Will do, thanks!

--
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 6/8] software node: Simplify swnode_register() a bit

[Greg Kroah-Hartman]

On Sun, Mar 28, 2021 at 12:20:10AM +0200, Andy Shevchenko wrote:
> By introducing two temporary variables simplify swnode_register() a bit.
> No functional change intended.
> 
> Signed-off-by: Andy Shevchenko 
> ---
>  drivers/base/swnode.c | 11 +--
>  1 file changed, 5 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/base/swnode.c b/drivers/base/swnode.c
> index ae53c48f84b1..1e81aaf5f6a1 100644
> --- a/drivers/base/swnode.c
> +++ b/drivers/base/swnode.c
> @@ -894,6 +894,8 @@ static struct fwnode_handle *
>  swnode_register(const struct software_node *node, struct swnode *parent,
>   unsigned int allocated)
>  {
> + struct ida *ids = parent ? &parent->child_ids : &swnode_root_ids;
> + struct kobject *kobj_parent = parent ? &parent->kobj : NULL;

?: operations are horrid.  Please spell this out in real if statements
so that we can properly understand and maintain them for the next 20+
years.

thanks,

greg k-h

[PATCH v1 6/8] software node: Simplify swnode_register() a bit

[Andy Shevchenko]

By introducing two temporary variables simplify swnode_register() a bit.
No functional change intended.

Signed-off-by: Andy Shevchenko 
---
 drivers/base/swnode.c | 11 +--
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/base/swnode.c b/drivers/base/swnode.c
index ae53c48f84b1..1e81aaf5f6a1 100644
--- a/drivers/base/swnode.c
+++ b/drivers/base/swnode.c
@@ -894,6 +894,8 @@ static struct fwnode_handle *
 swnode_register(const struct software_node *node, struct swnode *parent,
unsigned int allocated)
 {
+   struct ida *ids = parent ? &parent->child_ids : &swnode_root_ids;
+   struct kobject *kobj_parent = parent ? &parent->kobj : NULL;
struct swnode *swnode;
int ret;
 
@@ -901,8 +903,7 @@ swnode_register(const struct software_node *node, struct 
swnode *parent,
if (!swnode)
return ERR_PTR(-ENOMEM);
 
-   ret = ida_simple_get(parent ? &parent->child_ids : &swnode_root_ids,
-0, 0, GFP_KERNEL);
+   ret = ida_simple_get(ids, 0, 0, GFP_KERNEL);
if (ret < 0) {
kfree(swnode);
return ERR_PTR(ret);
@@ -920,12 +921,10 @@ swnode_register(const struct software_node *node, struct 
swnode *parent,
 
if (node->name)
ret = kobject_init_and_add(&swnode->kobj, &software_node_type,
-  parent ? &parent->kobj : NULL,
-  "%s", node->name);
+  kobj_parent, "%s", node->name);
else
ret = kobject_init_and_add(&swnode->kobj, &software_node_type,
-  parent ? &parent->kobj : NULL,
-  "node%d", swnode->id);
+  kobj_parent, "node%d", swnode->id);
if (ret) {
kobject_put(&swnode->kobj);
return ERR_PTR(ret);
-- 
2.30.2