Hi David,

On Fri, Aug 7, 2015 at 8:14 PM, David Daney [off-list ref] wrote:

On 08/07/2015 07:54 AM, Graeme Gregory wrote:

quoted

On Thu, Aug 06, 2015 at 05:33:10PM -0700, David Daney wrote:

quoted

From: David Daney <redacted>

Find out which PHYs belong to which BGX instance in the ACPI way.

Set the MAC address of the device as provided by ACPI tables. This is
similar to the implementation for devicetree in
of_get_mac_address(). The table is searched for the device property
entries "mac-address", "local-mac-address" and "address" in that
order. The address is provided in a u64 variable and must contain a
valid 6 bytes-len mac addr.

Based on code from: Narinder Dhillon [off-list ref]
                     Tomasz Nowicki [off-list ref]
                     Robert Richter [off-list ref]

Signed-off-by: Tomasz Nowicki <redacted>
Signed-off-by: Robert Richter <redacted>
Signed-off-by: David Daney <redacted>
---
  drivers/net/ethernet/cavium/thunder/thunder_bgx.c | 137
+++++++++++++++++++++-
  1 file changed, 135 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/cavium/thunder/thunder_bgx.c

b/drivers/net/ethernet/cavium/thunder/thunder_bgx.c
index 615b2af..2056583 100644

--- a/drivers/net/ethernet/cavium/thunder/thunder_bgx.c
+++ b/drivers/net/ethernet/cavium/thunder/thunder_bgx.c

[...]

quoted

quoted

+
+static int acpi_get_mac_address(struct acpi_device *adev, u8 *dst)
+{
+       const union acpi_object *prop;
+       u64 mac_val;
+       u8 mac[ETH_ALEN];
+       int i, j;
+       int ret;
+
+       for (i = 0; i < ARRAY_SIZE(addr_propnames); i++) {
+               ret = acpi_dev_get_property(adev, addr_propnames[i],
+                                           ACPI_TYPE_INTEGER, &prop);

Shouldn't this be trying to use device_property_read_* API and making
the DT/ACPI path the same where possible?

Ideally, something like you suggest would be possible.  However, there are a
couple of problems trying to do it in the kernel as it exists today:

1) There is no 'struct device *' here, so device_property_read_* is not
applicable.

2) There is no standard ACPI binding for MAC addresses, so it is impossible
to create a hypothetical fw_get_mac_address(), which would be analogous to
of_get_mac_address().

Other e-mail threads have suggested that the path to an elegant solution is
to inter-mix a bunch of calls to acpi_dev_get_property*() and
fwnode_property_read*() as to use these more generic fwnode_property_read*()
functions whereever possible.  I rejected this approach as it seems cleaner
to me to consistently use a single set of APIs.

Actually, that wasn't my intention.

I wanted to say that once you'd got an ACPI device pointer (struct
acpi_device), you could easly convert it to a struct fwnode_handle
pointer and operate that going forward when accessing properties.
That at least would help with the properties that do not differ
between DT and ACPI.

Thanks,
Rafael