Messages in this thread | | | Date | Fri, 4 Aug 2023 09:09:16 +0300 | From | Andy Shevchenko <> | Subject | Re: [RFC PATCH v1 11/21] swnode: Add support to create early during boot |
| |
On Thu, Aug 03, 2023 at 11:29:06PM +0530, Sunil V L wrote: > From: Anup Patel <apatel@ventanamicro.com> > > swnode framework can be used to create fwnode for interrupt > controllers.
Why? What is this for? Can you elaborate? This commit message is poorly written...
And why firmware node is not enough for ACPI case? I assume the fwnode in DT case is already provided by OF.
> This helps in keeping the drivers same for both > DT and ACPI. To enable this, enhance the swnode framework so > that it can be created early during boot without dependency > on sysfs.
...
> - swnode->kobj.kset = swnode_kset; > + swnode->kobj.kset = (!early) ? swnode_kset : NULL;
Too many unneeded characters. Why parentheses? Why negative check?
...
> + if (early) { > + ret = 0; > + kobject_init(&swnode->kobj, &software_node_type_early); > + swnode->kobj.parent = parent ? &parent->kobj : NULL; > + if (node->name) > + ret = kobject_set_name(&swnode->kobj, > + "%s", node->name); > + else > + ret = kobject_set_name(&swnode->kobj, > + "node%d", swnode->id); > + if (!ret) { > + spin_lock(&swnode_early_lock); > + list_add_tail(&swnode->early, &swnode_early_list); > + spin_unlock(&swnode_early_lock); > + } > + } else { > + if (node->name) > + ret = kobject_init_and_add(&swnode->kobj, &software_node_type, > + parent ? &parent->kobj : NULL,
This looks like have a duplication.
> + "%s", node->name); > + else > + ret = kobject_init_and_add(&swnode->kobj, &software_node_type, > + parent ? &parent->kobj : NULL, > + "node%d", swnode->id); > + }
Maybe it's possible to refactor this piece to be more compact?
...
> - return PTR_ERR_OR_ZERO(swnode_register(node, parent, 0)); > + return PTR_ERR_OR_ZERO(swnode_register(node, parent, 0, 0));
In one case you use boolean, here is unsigned int for early flag, why is the inconsistency added?
...
> -struct fwnode_handle * > -fwnode_create_software_node(const struct property_entry *properties, > - const struct fwnode_handle *parent) > +static struct fwnode_handle * > +fwnode_create_software_node_common(const struct property_entry *properties, > + const struct fwnode_handle *parent, > + bool early)
Why would you need this API in early stages?
-- With Best Regards, Andy Shevchenko
| |