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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 7/28/22 01:08, Greg Kroah-Hartman wrote:
> 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.

I was just copying the header from g_printer.h and changing as needed. It hasn't been touched since 2017, so if that's no longer the convention you might want to update it too.

> 
>> + */
>> +
>> +#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?

I was reading the specs and one of them said the maximum report length for USB 3 (I believe) was 512 bytes (in contrast with USB 2's 64). I can try to find where it said this, or add a define for max report length.

> 
>> +};
>> +
>> +/* 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.

The same .h file as which? g_printer.h and gadgetfs.h are separate files.

> 
>> +
>> +#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.

Should I move the g_printer.h definitions here at the same time? Maybe stub out g_printer.h and make it include gadgetfs.h?

> 
> thanks,
> 
> greg k-h



[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux