Re: [hid:for-5.13/upstream-fixes 3/17] undefined reference to `usb_hid_driver'

[Hans de Goede <hdegoede@redhat.com>]

Hi,

On 5/26/21 5:51 PM, kernel test robot wrote:
> tree:   https://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git for-5.13/upstream-fixes
> head:   624d8a18c8cd62e7613a9b2b6850205e0b6b9c7d
> commit: 2d21c6e884482b01b3878be220c51368a442bcc6 [3/17] HID: core: Add a hid_is_usb_device() helper function
> config: nios2-randconfig-c003-20210526 (attached as .config)
> compiler: nios2-linux-gcc (GCC) 9.3.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://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git/commit/?id=2d21c6e884482b01b3878be220c51368a442bcc6
>         git remote add hid https://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git
>         git fetch --no-tags hid for-5.13/upstream-fixes
>         git checkout 2d21c6e884482b01b3878be220c51368a442bcc6
>         # save the attached .config to linux build tree
>         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=nios2 
> 
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>

Ugh, this is caused by:

CONFIG_HID=y
CONFIG_USB_HID=m

I expected this to never happen because CONFIG_USB_HID does "select CONFIG_HID"
but CONFIG_HID is also selected by CONFIG_I2C_HID_CORE which is:

CONFIG_I2C_HID_CORE=y

in the .config triggering this error; and I see that:

drivers/hid/intel-ish-hid/Kconfig
drivers/hid/surface-hid/Kconfig

both also do a "select HID", where as drivers/hid/amd-sfh-hid/Kconfig
interestingly enough uses "depends on HID", so that can only be build of
one of the other HID ll-driver providers are enabled.

So I see 3 possible fixes:

1. Use IS_REACHABLE instead of IS_ENABLED inside hid_is_usb_device,
which means that hid_is_usb_device will always return false when
CONFIG_HID=y && CONFIG_USB_HID=m which is wrong.

2. Add a name field to struct hid_ll_driver, or maybe an enum
   and use strcmp (or == in case of an enum)

3. Talking about using an enum another fix would be to check that
   hid_dev->bus == BUS_USB

Actually I now see that there are already various drivers doing 3,
including hid-core.c (to decided to remove HID_CONNECT_HIDDEV from the
connect_mask), instead of the hid_is_using_ll_driver() check,
so 3 is probably best.

One issue with 3 though is that we also have the special hid-logitech-dj
code which acts as a hid_ll_driver for devices behind logitech wireless
receivers and which uses BUS_USB for those devices behind the
receiver too.

So a complete check using the bus check would look something like this:

bool hid_is_usb_device(struct hid_device *hid)
{
	/*
	 * The logi_dj_ll_driver also creates hid_devices with bus == BUS_USB
	 * but it does not have an output_report function so also check for that.
	 */
	return hdev->bus == BUS_USB && hdev->ll_driver->output_report;
}

This is esp. important for the usage in hid-multitouch.c because some
Logitech wireless devices are multi-touch touchpads which I believe are
using hid-multitouch.c and we do not want the parent device of those to
be treated as a usb-device...

So writing that makes me realize that mt_parent_may_wake() helper will
do the wrong thing for those hid_devices, which in turn makes me believe
that the whole approach taken in mt_parent_may_wake() is wrong.

Thinking about this more, this really is info which should be provided
by the hid_ll_driver to a new call-back.

So I believe that it is best to just drop:

[PATCH v2 2/6] HID: core: Add a hid_is_usb_device() helper function
[PATCH v2 4/6] HID: multitouch: Disable event reporting on suspend when our parent is not a wakeup-source
[PATCH v2 6/6] HID: asus: Switch to the new hid_is_usb_device() helper

For now and then I'll rework things when I have some time, sorry about this.

Regards,

Hans










> 
> All errors (new ones prefixed by >>):
> 
>    nios2-linux-ld: drivers/hid/hid-core.o: in function `hid_is_usb_device':
>>> (.text+0x298): undefined reference to `usb_hid_driver'
>>> nios2-linux-ld: (.text+0x29c): undefined reference to `usb_hid_driver'
> 
> ---
> 0-DAY CI Kernel Test Service, Intel Corporation
> https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
>