On Mon Jan 19, 2026 at 12:36 PM CET, Danilo Krummrich wrote:
> On Mon Jan 19, 2026 at 11:40 AM CET, Bartosz Golaszewski wrote:
>> -static void soc_device_get_machine(struct soc_device_attribute
>> *soc_dev_attr)
>> +int soc_device_get_machine(struct soc_device_attribute *soc_dev_attr)
>> {
>> - struct device_node *np;
>> -
>> if (soc_dev_attr->machine)
>> - return;
>> + return -EBUSY;
>> +
>> + struct device_node *np __free(device_node) = of_find_node_by_path("/");
>> + if (!np)
>> + return -ENOENT;
>
> This should never fail at this point, no? Also, can't we just use of_root?
Regarding of_root, please disregard my earlier comment. I mistakenly assumed
that it would also be guarded by CONFIG_OF.
But I still think we do not need the NULL check.
>> - np = of_find_node_by_path("/");
>> - of_property_read_string(np, "model", &soc_dev_attr->machine);
>> - of_node_put(np);
>> + return of_property_read_string(np, "model", &soc_dev_attr->machine);
>> }
>> +EXPORT_SYMBOL_GPL(soc_device_get_machine);
>
> If we want to export this, we shouldn't reuse the existing name, which is
> misleading.
>
> soc_device_get_machine() reads as if we return a reference count of something.
> Additionally, it operates on struct soc_device_attribute instead of struct
> soc_device, where the name suggests the latter.
>
> Instead this should be soc_device_attribute_read_machine() or if we want a
> shorter name, just soc_attr_read_machine().
