On Tue, Jan 12, 2021 at 7:02 PM Andy Shevchenko
[off-list ref] wrote:

On Tue, Jan 12, 2021 at 09:30:31AM -0800, Saravana Kannan wrote:

quoted

On Tue, Jan 12, 2021 at 5:42 AM Calvin Johnson
[off-list ref] wrote:

quoted

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

...

quoted

quoted

+/**
+ * fwnode_get_id - Get the id of a fwnode.
+ * @fwnode: firmware node
+ * @id: id of the fwnode
+ *
+ * This function provides the id of a fwnode which can be either
+ * DT or ACPI node. For ACPI, "reg" property value, if present will
+ * be provided or else _ADR value will be provided.
+ * Returns 0 on success or a negative errno.
+ */
+int fwnode_get_id(struct fwnode_handle *fwnode, u32 *id)
+{
+#ifdef CONFIG_ACPI
+       unsigned long long adr;
+       acpi_status status;
+#endif
+       int ret;
+
+       ret = fwnode_property_read_u32(fwnode, "reg", id);
+       if (!(ret && is_acpi_node(fwnode)))
+               return ret;
+
+#ifdef CONFIG_ACPI
+       status = acpi_evaluate_integer(ACPI_HANDLE_FWNODE(fwnode),
+                                      METHOD_NAME__ADR, NULL, &adr);
+       if (ACPI_FAILURE(status))
+               return -EINVAL;
+       *id = (u32)adr;
+#endif
+       return 0;
+}
+EXPORT_SYMBOL_GPL(fwnode_get_id);

quoted

Please don't do it this way. The whole point of fwnode_operations is
to avoid conditional stuff at the fwnode level.

Not fully true. We have non-POD getters that are conditional. Moreover,
we have additional layer of Primary / Secondary fwnodes on top of that.

The caller of fwnode API is indeed agnostic, but under the hood it differs by
the definition (obviously due to natural differences between ACPI and DT and
whatever else might come in the future.

quoted

Also ACPI and DT
aren't mutually exclusive if I'm not mistaken.

That's why we try 'reg' property for both cases first.

is_acpi_fwnode() conditional is that what I don't like though.

I'm not sure what you mean here, care to elaborate?