Re: [RFC 2/3] ACPI: move ACPI functionality out of r8152 driver

[Greg KH <gregkh@linuxfoundation.org>]

On Fri, Aug 23, 2019 at 10:28:24PM +0000, Charles.Hyde@dellteam.com wrote:
> --- /dev/null
> +++ b/lib/acpi_mac_passthru.c
> @@ -0,0 +1,61 @@
> +/*
> + *  Copyright (c) 2019 Dell Technology. All rights reserved.
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * version 2 as published by the Free Software Foundation.
> + *
> + */

You didn't run your patch through checkpatch.pl :(

Anyway, drop the license boilerplate please and use a SPDX line, like
checkpatch asks you to.

> +
> +#include <acpi/acpi_mac_passthru.h>
> +#include <linux/etherdevice.h>
> +
> +int get_acpi_mac_passthru(struct device *dev, struct sockaddr *sa)
> +{
> +#ifdef CONFIG_ACPI
> +	acpi_status status;
> +	struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
> +	union acpi_object *obj;
> +	int ret = -EINVAL;
> +	unsigned char buf[6];
> +
> +	/* returns _AUXMAC_#AABBCCDDEEFF# */
> +	status = acpi_evaluate_object(NULL, "\\_SB.AMAC", NULL, &buffer);
> +	obj = (union acpi_object *)buffer.pointer;
> +	if (!ACPI_SUCCESS(status))
> +		return -ENODEV;
> +	if (obj->type != ACPI_TYPE_BUFFER || obj->string.length != 0x17) {
> +		dev_warn(dev,
> +			 "Invalid buffer for pass-thru MAC addr: (%d, %d)\n",
> +			 obj->type, obj->string.length);
> +		goto amacout;
> +	}
> +	if (strncmp(obj->string.pointer, "_AUXMAC_#", 9) != 0 ||
> +	    strncmp(obj->string.pointer + 0x15, "#", 1) != 0) {
> +		dev_warn(dev,
> +			 "Invalid header when reading pass-thru MAC addr\n");
> +		goto amacout;
> +	}
> +	ret = hex2bin(buf, obj->string.pointer + 9, 6);
> +	if (!(ret == 0 && is_valid_ether_addr(buf))) {
> +		dev_warn(dev,
> +			 "Invalid MAC for pass-thru MAC addr: %d, %pM\n",
> +			 ret, buf);
> +		ret = -EINVAL;
> +		goto amacout;
> +	}
> +	memcpy(sa->sa_data, buf, 6);
> +	dev_info(dev, "Pass-thru MAC addr %pM\n", sa->sa_data);
> +
> +amacout:
> +	kfree(obj);
> +	return ret;
> +
> +#else	/* !CONFIG_ACPI */
> +	(void)dev;
> +	(void)sa;
> +
> +	return -ENODEV;

No #ifdef in .c files, especially for something as trivial as this.  The
#ifdef needs to be in the .h file, and don't build this unless acpi is
enabled.  And then, just move this to the acpi core, not in lib/

thanks,

greg k-h