On Tue, Dec 15, 2020 at 7:00 PM Laurent Pinchart
[off-list ref] wrote:

On Tue, Dec 15, 2020 at 10:13:11PM +0530, Calvin Johnson wrote:

quoted

Using fwnode_get_id(), get the reg property value for DT node
and get the _ADR object value for ACPI node.

...

quoted

+/**
+ * fwnode_get_id - Get the id of a fwnode.
+ * @fwnode: firmware node
+ * @id: id of the fwnode

Is the concept of fwnode ID documented clearly somewhere ? I think this
function should otherwise have more documentation, at least to explain
what the ID is.

I'm afraid that OF has no clear concept of this either. It's usually
used as a unique ID for the children of some device (like MFD) and can
represent a lot of things. But I agree it should be documented.

Rob, any ideas about this?

quoted

+ * Returns 0 on success or a negative errno.
+ */
+int fwnode_get_id(struct fwnode_handle *fwnode, u32 *id)
+{
+     unsigned long long adr;
+     acpi_status status;
+
+     if (is_of_node(fwnode)) {
+             return of_property_read_u32(to_of_node(fwnode), "reg", id);
+     } else if (is_acpi_node(fwnode)) {
+             status = acpi_evaluate_integer(ACPI_HANDLE_FWNODE(fwnode),
+                                            METHOD_NAME__ADR, NULL, &adr);
+             if (ACPI_FAILURE(status))
+                     return -ENODATA;

Would it make sense to standardize error codes ? of_property_read_u32()
can return -EINVAL, -ENODATA or -EOVERFLOW. I don't think the caller of
this function would be very interested to tell those three cases apart.
Maybe we should return -EINVAL in all error cases ? Or maybe different
error codes to mean "the backend doesn't support the concept of IDs",
and "the device doesn't have an ID" ?

We may actually get mapping to all three if first we check for the
method/name existence followed by value check.
But I don't think we need to bloat this simple one.

quoted

+             *id = (u32)adr;
+             return 0;
+     }
+     return -EINVAL;
+}

-- 
With Best Regards,
Andy Shevchenko