* Re: [PATCH] net: cdc_ncm: support ACPI MAC address pass through functionality
2023-03-09 8:34 [PATCH] net: cdc_ncm: support ACPI MAC address pass through functionality Kangzhen Lou
@ 2023-03-09 11:10 ` Greg KH
2023-03-28 3:54 ` [PATCH] net: cdc_ncm: support ACPI MAC address pass through functionality Message-ID: <ZAm+irMSf7FrcGK3@kroah.com> Kangzhen Lou
2023-03-09 12:15 ` [PATCH] net: cdc_ncm: support ACPI MAC address pass through functionality kernel test robot
` (2 subsequent siblings)
3 siblings, 1 reply; 6+ messages in thread
From: Greg KH @ 2023-03-09 11:10 UTC (permalink / raw)
To: Kangzhen Lou; +Cc: oliver, linux-usb
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@dell.com>
> ---
> 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
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] net: cdc_ncm: support ACPI MAC address pass through functionality Message-ID: <ZAm+irMSf7FrcGK3@kroah.com>
2023-03-09 11:10 ` Greg KH
@ 2023-03-28 3:54 ` Kangzhen Lou
0 siblings, 0 replies; 6+ messages in thread
From: Kangzhen Lou @ 2023-03-28 3:54 UTC (permalink / raw)
To: gregkh; +Cc: kangzhen.lou, linux-usb, oliver
> 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")
>
Thanks for reminder, my fault.
> But because of that commit, why is your change needed at all?
Because RTL8153DD supports both r8152 and cdc_ncm driver, and user can
select to load one of them by udev rule, in case someone want to load
cdc_ncm driver with MAC pass through function.
>
> >
> > Signed-off-by: Kangzhen Lou <kangzhen.lou@dell.com>
> > ---
> > 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?
About non-acpi system, it should return at first place:
if (!ACPI_SUCCESS(status))
return -ENODEV;
If I don't answer your question, may I know your thought?
>
> >
> > #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?
For the BIOS have different "\\_SB.AMAC" acpi object definition, may run
into here. I'm ok change to acpi_warning.
>
> > + 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.
If user disable MAC pass through in BIOS, it will also run into here.
So it's not an error. Depends on user selection in BIOS.
>
> > + 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.
I did run checkpatch.pl before submitting patches to make sure 0 error and
0 warning. Thanks for reminder again.
>
> thanks,
>
> greg k-h
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] net: cdc_ncm: support ACPI MAC address pass through functionality
2023-03-09 8:34 [PATCH] net: cdc_ncm: support ACPI MAC address pass through functionality Kangzhen Lou
2023-03-09 11:10 ` Greg KH
@ 2023-03-09 12:15 ` kernel test robot
2023-03-09 12:25 ` kernel test robot
2023-03-10 5:31 ` kernel test robot
3 siblings, 0 replies; 6+ messages in thread
From: kernel test robot @ 2023-03-09 12:15 UTC (permalink / raw)
To: Kangzhen Lou, oliver; +Cc: oe-kbuild-all, linux-usb, Kangzhen Lou
Hi Kangzhen,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on westeri-thunderbolt/next]
[also build test WARNING on linus/master v6.3-rc1 next-20230309]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Kangzhen-Lou/net-cdc_ncm-support-ACPI-MAC-address-pass-through-functionality/20230309-184736
base: https://git.kernel.org/pub/scm/linux/kernel/git/westeri/thunderbolt.git next
patch link: https://lore.kernel.org/r/20230309083436.6729-1-kangzhen.lou%40dell.com
patch subject: [PATCH] net: cdc_ncm: support ACPI MAC address pass through functionality
config: m68k-allyesconfig (https://download.01.org/0day-ci/archive/20230309/202303092024.QNv44vyX-lkp@intel.com/config)
compiler: m68k-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/97cd8ee9a774c36093af3d26255e415f6082b4a3
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Kangzhen-Lou/net-cdc_ncm-support-ACPI-MAC-address-pass-through-functionality/20230309-184736
git checkout 97cd8ee9a774c36093af3d26255e415f6082b4a3
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=m68k olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=m68k SHELL=/bin/bash drivers/net/
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202303092024.QNv44vyX-lkp@intel.com/
All warnings (new ones prefixed by >>):
>> drivers/net/usb/cdc_ncm.c:818:5: warning: no previous prototype for 'acpi_mac_passthru_invalid' [-Wmissing-prototypes]
818 | int acpi_mac_passthru_invalid(void)
| ^~~~~~~~~~~~~~~~~~~~~~~~~
>> drivers/net/usb/cdc_ncm.c:851:5: warning: no previous prototype for 'get_acpi_mac_passthru' [-Wmissing-prototypes]
851 | int get_acpi_mac_passthru(char *MACAddress)
| ^~~~~~~~~~~~~~~~~~~~~
drivers/net/usb/cdc_ncm.c: In function 'cdc_ncm_get_ethernet_address':
>> drivers/net/usb/cdc_ncm.c:894:49: warning: passing argument 6 of 'usbnet_read_cmd' discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
894 | iface_no, dev->net->dev_addr, ETH_ALEN);
| ~~~~~~~~^~~~~~~~~~
In file included from drivers/net/usb/cdc_ncm.c:53:
include/linux/usb/usbnet.h:181:49: note: expected 'void *' but argument is of type 'const unsigned char *'
181 | u16 value, u16 index, void *data, u16 size);
| ~~~~~~^~~~
In file included from include/linux/string.h:20,
from include/linux/bitmap.h:11,
from include/linux/cpumask.h:12,
from include/linux/mm_types_task.h:14,
from include/linux/mm_types.h:5,
from include/linux/buildid.h:5,
from include/linux/module.h:14,
from drivers/net/usb/cdc_ncm.c:41:
drivers/net/usb/cdc_ncm.c: In function 'cdc_ncm_determine_ethernet_addr':
>> drivers/net/usb/cdc_ncm.c:980:48: warning: passing argument 1 of '__builtin_memcpy' discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
980 | memcpy(dev->net->dev_addr, sa.sa_data, ETH_ALEN);
| ~~~~~~~~^~~~~~~~~~
arch/m68k/include/asm/string.h:52:42: note: in definition of macro 'memcpy'
52 | #define memcpy(d, s, n) __builtin_memcpy(d, s, n)
| ^
drivers/net/usb/cdc_ncm.c:980:48: note: expected 'void *' but argument is of type 'const unsigned char *'
980 | memcpy(dev->net->dev_addr, sa.sa_data, ETH_ALEN);
| ~~~~~~~~^~~~~~~~~~
arch/m68k/include/asm/string.h:52:42: note: in definition of macro 'memcpy'
52 | #define memcpy(d, s, n) __builtin_memcpy(d, s, n)
| ^
vim +/acpi_mac_passthru_invalid +818 drivers/net/usb/cdc_ncm.c
817
> 818 int acpi_mac_passthru_invalid(void)
819 {
820 acpi_status status;
821 struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
822 union acpi_object *obj;
823 int ret = -EINVAL;
824
825 /* returns _AUXMAC_#AABBCCDDEEFF# */
826 status = acpi_evaluate_object(NULL, "\\_SB.AMAC", NULL, &buffer);
827 obj = (union acpi_object *)buffer.pointer;
828
829 if (!ACPI_SUCCESS(status))
830 return -ENODEV;
831 if (obj->type != ACPI_TYPE_BUFFER || obj->string.length != 0x17) {
832 acpi_info("Invalid buffer for pass-thru MAC addr: (%d, %d)\n",
833 obj->type, obj->string.length);
834 goto amacout;
835 }
836 if (strncmp(obj->string.pointer, "_AUXMAC_#", 9) != 0 ||
837 strncmp(obj->string.pointer + 0x15, "#", 1) != 0) {
838 acpi_info("Invalid header when reading pass-thru MAC addr\n");
839 goto amacout;
840 }
841 /* return success, otherwise return non-zero if invalid buffer read or
842 * MAC pass through is disabled in BIOS
843 */
844 ret = 0;
845
846 amacout:
847 kfree(obj);
848 return ret;
849 }
850
> 851 int get_acpi_mac_passthru(char *MACAddress)
852 {
853 acpi_status status;
854 struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
855 union acpi_object *obj;
856 int ret = -EINVAL;
857 unsigned char buf[6];
858
859 /* returns _AUXMAC_#AABBCCDDEEFF# */
860 status = acpi_evaluate_object(NULL, "\\_SB.AMAC", NULL, &buffer);
861 obj = (union acpi_object *)buffer.pointer;
862
863 if (!ACPI_SUCCESS(status))
864 return -ENODEV;
865
866 ret = hex2bin(buf, obj->string.pointer + 9, 6);
867 if (!(ret == 0 && is_valid_ether_addr(buf))) {
868 acpi_info("Invalid MAC for pass-thru MAC addr: %d, %pM\n",
869 ret, buf);
870 ret = -EINVAL;
871 goto amacout;
872 }
873 memcpy(MACAddress, buf, 6);
874 acpi_info("Pass-thru MAC addr %pM\n", MACAddress);
875
876 amacout:
877 kfree(obj);
878 return ret;
879 }
880
881 /* Provide method to get MAC address from the USB device's ethernet controller.
882 * If the device supports CDC_GET_ADDRESS, we should receive just six bytes.
883 * Otherwise, use the prior method by asking for the descriptor.
884 */
885 static int cdc_ncm_get_ethernet_address(struct usbnet *dev,
886 struct cdc_ncm_ctx *ctx)
887 {
888 int ret;
889 u8 iface_no = ctx->control->cur_altsetting->desc.bInterfaceNumber;
890
891 ret = usbnet_read_cmd(dev, USB_CDC_GET_NET_ADDRESS,
892 USB_DIR_IN | USB_TYPE_CLASS
893 | USB_RECIP_INTERFACE, 0,
> 894 iface_no, dev->net->dev_addr, ETH_ALEN);
895
896 if (ret == ETH_ALEN) {
897 ret = 0; /* success */
898 } else {
899 ret = usbnet_get_ethernet_addr(dev,
900 ctx->ether_desc->iMACAddress);
901 }
902
903 return ret;
904 }
905
906 /* Provide method to push MAC address to the USB device's ethernet controller.
907 * If the device does not support CDC_SET_ADDRESS, there is no harm and we
908 * proceed as before.
909 */
910 static int cdc_ncm_set_ethernet_address(struct usbnet *dev,
911 struct sockaddr *addr)
912 {
913 int ret;
914 struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev->data[0];
915 u8 iface_no_data = ctx->data->cur_altsetting->desc.bInterfaceNumber;
916 u8 iface_no_control = ctx->control->cur_altsetting->desc.bInterfaceNumber;
917 int temp;
918
919 /* The host shall only send SET_NET_ADDRESS command while
920 * the NCM Data Interface is in alternate setting 0.
921 */
922 temp = usb_set_interface(dev->udev, iface_no_data, 0);
923 if (temp) {
924 dev_dbg(&dev->udev->dev, "set interface failed\n");
925 return -ENODEV;
926 }
927
928 ret = usbnet_write_cmd(dev, USB_CDC_SET_NET_ADDRESS,
929 USB_DIR_OUT | USB_TYPE_CLASS
930 | USB_RECIP_INTERFACE, 0,
931 iface_no_control, addr->sa_data, ETH_ALEN);
932
933 if (ret == ETH_ALEN)
934 ret = 0; // success
935 else if (ret < 0)
936 dev_dbg(&dev->udev->dev, "bad MAC address put, %d\n", ret);
937
938 /* restore alternate setting.
939 * The NCM data altsetting is fixed, so we hard-coded it.
940 */
941 temp = usb_set_interface(dev->udev, iface_no_data,
942 CDC_NCM_DATA_ALTSETTING_NCM);
943 if (temp) {
944 dev_dbg(&dev->udev->dev, "set interface failed\n");
945 return -ENODEV;
946 }
947
948 return ret;
949 }
950
951 static int cdc_ncm_determine_ethernet_addr(struct usb_interface *intf)
952 {
953 struct sockaddr sa;
954 struct usbnet *dev = usb_get_intfdata(intf);
955 struct cdc_ncm_ctx *ctx;
956 int ret = 0;
957
958 if (!dev)
959 return 0;
960
961 /* MAC pass through function only apply to Realtek RTL8153-DD chip */
962 if (!(dev->udev->descriptor.idVendor == 0x0bda
963 && dev->udev->descriptor.idProduct == 0x8153
964 && (dev->udev->descriptor.bcdDevice & 0xff00) == 0x3300))
965 return 0;
966
967 ctx = (struct cdc_ncm_ctx *)dev->data[0];
968 if (!ctx->ether_desc)
969 return 0;
970
971 ret = cdc_ncm_get_ethernet_address(dev, ctx);
972 if (ret) {
973 dev_dbg(&intf->dev, "failed to get mac address\n");
974 return ret;
975 }
976
977 if (!get_acpi_mac_passthru(sa.sa_data)) {
978 if (memcmp(dev->net->dev_addr, sa.sa_data, ETH_ALEN) != 0) {
979 if (!cdc_ncm_set_ethernet_address(dev, &sa))
> 980 memcpy(dev->net->dev_addr, sa.sa_data, ETH_ALEN);
981 }
982 }
983
984 dev_info(&intf->dev, "MAC-Address: %pM\n", dev->net->dev_addr);
985
986 return 0;
987 }
988
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] net: cdc_ncm: support ACPI MAC address pass through functionality
2023-03-09 8:34 [PATCH] net: cdc_ncm: support ACPI MAC address pass through functionality Kangzhen Lou
2023-03-09 11:10 ` Greg KH
2023-03-09 12:15 ` [PATCH] net: cdc_ncm: support ACPI MAC address pass through functionality kernel test robot
@ 2023-03-09 12:25 ` kernel test robot
2023-03-10 5:31 ` kernel test robot
3 siblings, 0 replies; 6+ messages in thread
From: kernel test robot @ 2023-03-09 12:25 UTC (permalink / raw)
To: Kangzhen Lou, oliver; +Cc: oe-kbuild-all, linux-usb, Kangzhen Lou
Hi Kangzhen,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on westeri-thunderbolt/next]
[also build test WARNING on linus/master v6.3-rc1 next-20230309]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Kangzhen-Lou/net-cdc_ncm-support-ACPI-MAC-address-pass-through-functionality/20230309-184736
base: https://git.kernel.org/pub/scm/linux/kernel/git/westeri/thunderbolt.git next
patch link: https://lore.kernel.org/r/20230309083436.6729-1-kangzhen.lou%40dell.com
patch subject: [PATCH] net: cdc_ncm: support ACPI MAC address pass through functionality
config: ia64-allyesconfig (https://download.01.org/0day-ci/archive/20230309/202303092027.01TO9rlB-lkp@intel.com/config)
compiler: ia64-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/97cd8ee9a774c36093af3d26255e415f6082b4a3
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Kangzhen-Lou/net-cdc_ncm-support-ACPI-MAC-address-pass-through-functionality/20230309-184736
git checkout 97cd8ee9a774c36093af3d26255e415f6082b4a3
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=ia64 olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=ia64 SHELL=/bin/bash drivers/net/
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202303092027.01TO9rlB-lkp@intel.com/
All warnings (new ones prefixed by >>):
drivers/net/usb/cdc_ncm.c:818:5: warning: no previous prototype for 'acpi_mac_passthru_invalid' [-Wmissing-prototypes]
818 | int acpi_mac_passthru_invalid(void)
| ^~~~~~~~~~~~~~~~~~~~~~~~~
drivers/net/usb/cdc_ncm.c:851:5: warning: no previous prototype for 'get_acpi_mac_passthru' [-Wmissing-prototypes]
851 | int get_acpi_mac_passthru(char *MACAddress)
| ^~~~~~~~~~~~~~~~~~~~~
drivers/net/usb/cdc_ncm.c: In function 'cdc_ncm_get_ethernet_address':
drivers/net/usb/cdc_ncm.c:894:49: warning: passing argument 6 of 'usbnet_read_cmd' discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
894 | iface_no, dev->net->dev_addr, ETH_ALEN);
| ~~~~~~~~^~~~~~~~~~
In file included from drivers/net/usb/cdc_ncm.c:53:
include/linux/usb/usbnet.h:181:49: note: expected 'void *' but argument is of type 'const unsigned char *'
181 | u16 value, u16 index, void *data, u16 size);
| ~~~~~~^~~~
drivers/net/usb/cdc_ncm.c: In function 'cdc_ncm_determine_ethernet_addr':
>> drivers/net/usb/cdc_ncm.c:980:48: warning: passing argument 1 of 'memcpy' discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
980 | memcpy(dev->net->dev_addr, sa.sa_data, ETH_ALEN);
| ~~~~~~~~^~~~~~~~~~
In file included from include/linux/string.h:20,
from include/linux/bitmap.h:11,
from include/linux/cpumask.h:12,
from include/linux/mm_types_task.h:14,
from include/linux/mm_types.h:5,
from include/linux/buildid.h:5,
from include/linux/module.h:14,
from drivers/net/usb/cdc_ncm.c:41:
arch/ia64/include/asm/string.h:19:22: note: expected 'void *' but argument is of type 'const unsigned char *'
19 | extern void *memcpy (void *, const void *, __kernel_size_t);
| ^~~~~~
vim +980 drivers/net/usb/cdc_ncm.c
950
951 static int cdc_ncm_determine_ethernet_addr(struct usb_interface *intf)
952 {
953 struct sockaddr sa;
954 struct usbnet *dev = usb_get_intfdata(intf);
955 struct cdc_ncm_ctx *ctx;
956 int ret = 0;
957
958 if (!dev)
959 return 0;
960
961 /* MAC pass through function only apply to Realtek RTL8153-DD chip */
962 if (!(dev->udev->descriptor.idVendor == 0x0bda
963 && dev->udev->descriptor.idProduct == 0x8153
964 && (dev->udev->descriptor.bcdDevice & 0xff00) == 0x3300))
965 return 0;
966
967 ctx = (struct cdc_ncm_ctx *)dev->data[0];
968 if (!ctx->ether_desc)
969 return 0;
970
971 ret = cdc_ncm_get_ethernet_address(dev, ctx);
972 if (ret) {
973 dev_dbg(&intf->dev, "failed to get mac address\n");
974 return ret;
975 }
976
977 if (!get_acpi_mac_passthru(sa.sa_data)) {
978 if (memcmp(dev->net->dev_addr, sa.sa_data, ETH_ALEN) != 0) {
979 if (!cdc_ncm_set_ethernet_address(dev, &sa))
> 980 memcpy(dev->net->dev_addr, sa.sa_data, ETH_ALEN);
981 }
982 }
983
984 dev_info(&intf->dev, "MAC-Address: %pM\n", dev->net->dev_addr);
985
986 return 0;
987 }
988
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] net: cdc_ncm: support ACPI MAC address pass through functionality
2023-03-09 8:34 [PATCH] net: cdc_ncm: support ACPI MAC address pass through functionality Kangzhen Lou
` (2 preceding siblings ...)
2023-03-09 12:25 ` kernel test robot
@ 2023-03-10 5:31 ` kernel test robot
3 siblings, 0 replies; 6+ messages in thread
From: kernel test robot @ 2023-03-10 5:31 UTC (permalink / raw)
To: Kangzhen Lou, oliver; +Cc: llvm, oe-kbuild-all, linux-usb, Kangzhen Lou
Hi Kangzhen,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on westeri-thunderbolt/next]
[also build test ERROR on linus/master v6.3-rc1 next-20230310]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Kangzhen-Lou/net-cdc_ncm-support-ACPI-MAC-address-pass-through-functionality/20230309-184736
base: https://git.kernel.org/pub/scm/linux/kernel/git/westeri/thunderbolt.git next
patch link: https://lore.kernel.org/r/20230309083436.6729-1-kangzhen.lou%40dell.com
patch subject: [PATCH] net: cdc_ncm: support ACPI MAC address pass through functionality
config: powerpc-g5_defconfig (https://download.01.org/0day-ci/archive/20230310/202303101331.rzCbgFQa-lkp@intel.com/config)
compiler: clang version 17.0.0 (https://github.com/llvm/llvm-project 67409911353323ca5edf2049ef0df54132fa1ca7)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install powerpc cross compiling tool for clang build
# apt-get install binutils-powerpc-linux-gnu
# https://github.com/intel-lab-lkp/linux/commit/97cd8ee9a774c36093af3d26255e415f6082b4a3
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Kangzhen-Lou/net-cdc_ncm-support-ACPI-MAC-address-pass-through-functionality/20230309-184736
git checkout 97cd8ee9a774c36093af3d26255e415f6082b4a3
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=powerpc olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=powerpc SHELL=/bin/bash arch/powerpc/kernel/ drivers/net/usb/
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202303101331.rzCbgFQa-lkp@intel.com/
All error/warnings (new ones prefixed by >>):
>> drivers/net/usb/cdc_ncm.c:818:5: warning: no previous prototype for function 'acpi_mac_passthru_invalid' [-Wmissing-prototypes]
int acpi_mac_passthru_invalid(void)
^
drivers/net/usb/cdc_ncm.c:818:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
int acpi_mac_passthru_invalid(void)
^
static
>> drivers/net/usb/cdc_ncm.c:851:5: warning: no previous prototype for function 'get_acpi_mac_passthru' [-Wmissing-prototypes]
int get_acpi_mac_passthru(char *MACAddress)
^
drivers/net/usb/cdc_ncm.c:851:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
int get_acpi_mac_passthru(char *MACAddress)
^
static
>> drivers/net/usb/cdc_ncm.c:894:20: error: passing 'const unsigned char *' to parameter of type 'void *' discards qualifiers [-Werror,-Wincompatible-pointer-types-discards-qualifiers]
iface_no, dev->net->dev_addr, ETH_ALEN);
^~~~~~~~~~~~~~~~~~
include/linux/usb/usbnet.h:181:35: note: passing argument to parameter 'data' here
u16 value, u16 index, void *data, u16 size);
^
drivers/net/usb/cdc_ncm.c:980:12: error: passing 'const unsigned char *' to parameter of type 'void *' discards qualifiers [-Werror,-Wincompatible-pointer-types-discards-qualifiers]
memcpy(dev->net->dev_addr, sa.sa_data, ETH_ALEN);
^~~~~~~~~~~~~~~~~~
arch/powerpc/include/asm/string.h:27:28: note: passing argument to parameter here
extern void * memcpy(void *,const void *,__kernel_size_t);
^
2 warnings and 2 errors generated.
vim +894 drivers/net/usb/cdc_ncm.c
817
> 818 int acpi_mac_passthru_invalid(void)
819 {
820 acpi_status status;
821 struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
822 union acpi_object *obj;
823 int ret = -EINVAL;
824
825 /* returns _AUXMAC_#AABBCCDDEEFF# */
826 status = acpi_evaluate_object(NULL, "\\_SB.AMAC", NULL, &buffer);
827 obj = (union acpi_object *)buffer.pointer;
828
829 if (!ACPI_SUCCESS(status))
830 return -ENODEV;
831 if (obj->type != ACPI_TYPE_BUFFER || obj->string.length != 0x17) {
832 acpi_info("Invalid buffer for pass-thru MAC addr: (%d, %d)\n",
833 obj->type, obj->string.length);
834 goto amacout;
835 }
836 if (strncmp(obj->string.pointer, "_AUXMAC_#", 9) != 0 ||
837 strncmp(obj->string.pointer + 0x15, "#", 1) != 0) {
838 acpi_info("Invalid header when reading pass-thru MAC addr\n");
839 goto amacout;
840 }
841 /* return success, otherwise return non-zero if invalid buffer read or
842 * MAC pass through is disabled in BIOS
843 */
844 ret = 0;
845
846 amacout:
847 kfree(obj);
848 return ret;
849 }
850
> 851 int get_acpi_mac_passthru(char *MACAddress)
852 {
853 acpi_status status;
854 struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
855 union acpi_object *obj;
856 int ret = -EINVAL;
857 unsigned char buf[6];
858
859 /* returns _AUXMAC_#AABBCCDDEEFF# */
860 status = acpi_evaluate_object(NULL, "\\_SB.AMAC", NULL, &buffer);
861 obj = (union acpi_object *)buffer.pointer;
862
863 if (!ACPI_SUCCESS(status))
864 return -ENODEV;
865
866 ret = hex2bin(buf, obj->string.pointer + 9, 6);
867 if (!(ret == 0 && is_valid_ether_addr(buf))) {
868 acpi_info("Invalid MAC for pass-thru MAC addr: %d, %pM\n",
869 ret, buf);
870 ret = -EINVAL;
871 goto amacout;
872 }
873 memcpy(MACAddress, buf, 6);
874 acpi_info("Pass-thru MAC addr %pM\n", MACAddress);
875
876 amacout:
877 kfree(obj);
878 return ret;
879 }
880
881 /* Provide method to get MAC address from the USB device's ethernet controller.
882 * If the device supports CDC_GET_ADDRESS, we should receive just six bytes.
883 * Otherwise, use the prior method by asking for the descriptor.
884 */
885 static int cdc_ncm_get_ethernet_address(struct usbnet *dev,
886 struct cdc_ncm_ctx *ctx)
887 {
888 int ret;
889 u8 iface_no = ctx->control->cur_altsetting->desc.bInterfaceNumber;
890
891 ret = usbnet_read_cmd(dev, USB_CDC_GET_NET_ADDRESS,
892 USB_DIR_IN | USB_TYPE_CLASS
893 | USB_RECIP_INTERFACE, 0,
> 894 iface_no, dev->net->dev_addr, ETH_ALEN);
895
896 if (ret == ETH_ALEN) {
897 ret = 0; /* success */
898 } else {
899 ret = usbnet_get_ethernet_addr(dev,
900 ctx->ether_desc->iMACAddress);
901 }
902
903 return ret;
904 }
905
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests
^ permalink raw reply [flat|nested] 6+ messages in thread