Hi folks, looking into drivers/of/base.c, I notices a couple of oddities in of_update_property(). Both were interoduced with commit 75b57ecf (of: Make device nodes kobjects so they show up in sysfs). In both cases, there is an unnecessary value check. Sending a patch to remove the checks would be simple, but I want to make sure that the checks are not supposed to address some other condition. 1) Unnecessary check if rc != 0 Here are all the uses of the 'rc' variable in of_update_property(): rc = of_property_notify(OF_RECONFIG_UPDATE_PROPERTY, np, newprop); if (rc) return rc; ... while (*next) { ... } raw_spin_unlock_irqrestore(&devtree_lock, flags); if (rc) return rc; What is the purpose of the second check ? Can it be removed, or is there some other condition which should have been checked instead ? Should the check possibly be "if (!found)" ? 2) Unnecessary check if oldprop != NULL oldprop = of_find_property(np, newprop->name, NULL); if (!oldprop) return of_add_property(np, newprop); ... /* Update the sysfs attribute */ if (oldprop) sysfs_remove_bin_file(&np->kobj, &oldprop->attr); oldprop is not modified between the two checks. Can the check be removed, or is there some other condition which should have been checked instead ? 3) At the very end of the function, there is a check if (!found) return -ENODEV; This is _after_ the sysfs entry for the old property was removed, and the sysfs entry for the new property was created. Yet, '!found' suggests that there is some inconsistency in the property, so I wonder if the check should happen earlier, before sysfs entries are touched. Thanks, Guenter -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html