Linux-USB Archive on lore.kernel.org
 help / color / Atom feed
From: Greg KH <gregkh@linuxfoundation.org>
To: Charles.Hyde@dellteam.com
Cc: linux-acpi@vger.kernel.org, Mario.Limonciello@dell.com,
	oliver@neukum.org, nic_swsd@realtek.com,
	linux-usb@vger.kernel.org
Subject: Re: [RFC 2/3] ACPI: move ACPI functionality out of r8152 driver
Date: Fri, 23 Aug 2019 19:41:15 -0700
Message-ID: <20190824024115.GA11208@kroah.com> (raw)
In-Reply-To: <b84b32cb144f4ba8918ee2406e69275a@AUSX13MPS303.AMER.DELL.COM>

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

  reply index

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-08-23 22:28 Charles.Hyde
2019-08-24  2:41 ` Greg KH [this message]
2019-08-24 10:25 ` Bjørn Mork
2019-08-24 13:24   ` Mario.Limonciello
2019-08-27 22:08     ` Charles.Hyde
2019-08-27 22:19       ` Mario.Limonciello
2019-08-28 16:41         ` Charles.Hyde

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190824024115.GA11208@kroah.com \
    --to=gregkh@linuxfoundation.org \
    --cc=Charles.Hyde@dellteam.com \
    --cc=Mario.Limonciello@dell.com \
    --cc=linux-acpi@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=nic_swsd@realtek.com \
    --cc=oliver@neukum.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-USB Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-usb/0 linux-usb/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-usb linux-usb/ https://lore.kernel.org/linux-usb \
		linux-usb@vger.kernel.org
	public-inbox-index linux-usb

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-usb


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git