Re: [PATCH 1/2] USB: gadget: f_hid: Add Get-Feature report

[Greg Kroah-Hartman <gregkh@linuxfoundation.org>]

On Mon, Jul 25, 2022 at 05:58:25PM -0700, Vicki Pfau wrote:
> --- /dev/null
> +++ b/include/uapi/linux/usb/g_hid.h
> @@ -0,0 +1,38 @@
> +/* SPDX-License-Identifier: GPL-2.0+ WITH Linux-syscall-note */
> +/*
> + * g_hid.h -- Header file for USB HID gadget driver
> + *
> + * Copyright (C) 2022 Valve Software
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA

This whole license "boilerplate" is not needed, and should be removed
(especially things like addresses, that's crazy).

Only thing that is needed is the SPDX line.

> + */
> +
> +#ifndef __UAPI_LINUX_USB_G_HID_H
> +#define __UAPI_LINUX_USB_G_HID_H
> +
> +#include <linux/types.h>
> +
> +struct usb_hidg_report {
> +	__u16 length;
> +	__u8 data[512];

Why 512?

> +};
> +
> +/* The 'g' code is also used by gadgetfs and hid gadget ioctl requests.
> + * Don't add any colliding codes to either driver, and keep
> + * them in unique ranges (size 0x20 for now).
> + */
> +#define GADGET_HID_WRITE_GET_REPORT	_IOW('g', 0x42, struct usb_hidg_report)

This should be in the same .h file so that we don't get confused and
accidentally use the same ioctl.

> +
> +#endif /* __UAPI_LINUX_USB_G_HID_H */
> diff --git a/include/uapi/linux/usb/gadgetfs.h b/include/uapi/linux/usb/gadgetfs.h
> index 835473910a49..9754822b2a40 100644
> --- a/include/uapi/linux/usb/gadgetfs.h
> +++ b/include/uapi/linux/usb/gadgetfs.h
> @@ -62,7 +62,7 @@ struct usb_gadgetfs_event {
>  };
>  
>  
> -/* The 'g' code is also used by printer gadget ioctl requests.
> +/* The 'g' code is also used by printer and hid gadget ioctl requests.

Yeah, put the definition here.

thanks,

greg k-h