[PATCH 2/2] net, thunder, bgx: Add support for ACPI binding.

[Rafael J. Wysocki]

Hi David,

On Fri, Aug 7, 2015 at 8:14 PM, David Daney <ddaney at caviumnetworks.com> wrote:
> On 08/07/2015 07:54 AM, Graeme Gregory wrote:
>>
>> On Thu, Aug 06, 2015 at 05:33:10PM -0700, David Daney wrote:
>>>
>>> From: David Daney <david.daney at cavium.com>
>>>
>>> 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 <ndhillon at cavium.com>
>>>                      Tomasz Nowicki <tomasz.nowicki at linaro.org>
>>>                      Robert Richter <rrichter at cavium.com>
>>>
>>> Signed-off-by: Tomasz Nowicki <tomasz.nowicki at linaro.org>
>>> Signed-off-by: Robert Richter <rrichter at cavium.com>
>>> Signed-off-by: David Daney <david.daney at cavium.com>
>>> ---
>>>   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
>
> [...]
>>>
>>> +
>>> +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