On Thu, Mar 09, 2023 at 04:34:36PM +0800, Kangzhen Lou wrote:
> Make cdc_ncm to support ACPI MAC address pass through functionality that
> also exists in the Realtek r8152 driver.
>
> RTL8153DD will load cdc_ncm driver by default with mainline 6.2 kernel.
> This is to support Realtek RTL8153DD Ethernet Interface Controller's MAC
> pass through function which used in dock, dongle or monitor.
>
> Although there is patch “ec51fbd1b8a2bca2948dede99c14ec63dc57ff6b” will
> make RTL8153DD load r8152-cfgselector instead cdc_ncm driver, would also
> submit this patch in case anyone need this feature in cdc_ncm in the
> future.
Please reference commits in the correct way, as documented:
ec51fbd1b8a2 ("r8152: add USB device driver for config selection")
But because of that commit, why is your change needed at all?
>
> Signed-off-by: Kangzhen Lou <kangzhen.lou@xxxxxxxx>
> ---
> drivers/net/usb/cdc_ncm.c | 199 +++++++++++++++++++++++++++++++++++++-
> 1 file changed, 197 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
> index 6ce8f4f0c70e..8179516b819c 100644
> --- a/drivers/net/usb/cdc_ncm.c
> +++ b/drivers/net/usb/cdc_ncm.c
> @@ -53,6 +53,7 @@
> #include <linux/usb/usbnet.h>
> #include <linux/usb/cdc.h>
> #include <linux/usb/cdc_ncm.h>
> +#include <linux/acpi.h>
What about non-acpi systems?
>
> #if IS_ENABLED(CONFIG_USB_NET_CDC_MBIM)
> static bool prefer_mbim = true;
> @@ -814,6 +815,177 @@ static const struct net_device_ops cdc_ncm_netdev_ops = {
> .ndo_validate_addr = eth_validate_addr,
> };
>
> +int acpi_mac_passthru_invalid(void)
> +{
> + acpi_status status;
> + struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
> + union acpi_object *obj;
> + int ret = -EINVAL;
> +
> + /* 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) {
> + acpi_info("Invalid buffer for pass-thru MAC addr: (%d, %d)\n",
> + obj->type, obj->string.length);
Why is that an "info" message?
> + goto amacout;
> + }
> + if (strncmp(obj->string.pointer, "_AUXMAC_#", 9) != 0 ||
> + strncmp(obj->string.pointer + 0x15, "#", 1) != 0) {
> + acpi_info("Invalid header when reading pass-thru MAC addr\n");
Again, info? Shouldn't this be an error? Or just debug as userspace
can't do anything about it.
> + goto amacout;
> + }
> + /* return success, otherwise return non-zero if invalid buffer read or
> + * MAC pass through is disabled in BIOS
> + */
> + ret = 0;
> +
> +amacout:
> + kfree(obj);
> + return ret;
> +}
> +
> +int get_acpi_mac_passthru(char *MACAddress)
> +{
> + 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;
> +
> + ret = hex2bin(buf, obj->string.pointer + 9, 6);
> + if (!(ret == 0 && is_valid_ether_addr(buf))) {
> + acpi_info("Invalid MAC for pass-thru MAC addr: %d, %pM\n",
> + ret, buf);
> + ret = -EINVAL;
> + goto amacout;
> + }
> + memcpy(MACAddress, buf, 6);
> + acpi_info("Pass-thru MAC addr %pM\n", MACAddress);
> +
> +amacout:
> + kfree(obj);
> + return ret;
> +}
> +
> +/* Provide method to get MAC address from the USB device's ethernet controller.
> + * If the device supports CDC_GET_ADDRESS, we should receive just six bytes.
> + * Otherwise, use the prior method by asking for the descriptor.
> + */
> +static int cdc_ncm_get_ethernet_address(struct usbnet *dev,
> + struct cdc_ncm_ctx *ctx)
> +{
> + int ret;
> + u8 iface_no = ctx->control->cur_altsetting->desc.bInterfaceNumber;
> +
> + ret = usbnet_read_cmd(dev, USB_CDC_GET_NET_ADDRESS,
> + USB_DIR_IN | USB_TYPE_CLASS
> + | USB_RECIP_INTERFACE, 0,
> + iface_no, dev->net->dev_addr, ETH_ALEN);
> +
> + if (ret == ETH_ALEN) {
> + ret = 0; /* success */
> + } else {
> + ret = usbnet_get_ethernet_addr(dev,
> + ctx->ether_desc->iMACAddress);
> + }
> +
> + return ret;
> +}
> +
> +/* Provide method to push MAC address to the USB device's ethernet controller.
> + * If the device does not support CDC_SET_ADDRESS, there is no harm and we
> + * proceed as before.
> + */
> +static int cdc_ncm_set_ethernet_address(struct usbnet *dev,
> + struct sockaddr *addr)
> +{
> + int ret;
> + struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev->data[0];
> + u8 iface_no_data = ctx->data->cur_altsetting->desc.bInterfaceNumber;
> + u8 iface_no_control = ctx->control->cur_altsetting->desc.bInterfaceNumber;
> + int temp;
> +
> + /* The host shall only send SET_NET_ADDRESS command while
> + * the NCM Data Interface is in alternate setting 0.
> + */
> + temp = usb_set_interface(dev->udev, iface_no_data, 0);
> + if (temp) {
> + dev_dbg(&dev->udev->dev, "set interface failed\n");
> + return -ENODEV;
> + }
Always use checkpatch.pl before submitting patches so you don't get
grumpy reviewers telling you to use checkpatch.pl.
thanks,
greg k-h
