Re: [PATCH v2 1/5] efi: usb: add header for usb-efi

Hi Fabian,

On 8/19/26 3:32 PM, Fabian Pflug wrote:
> Hey,
> 
> On Wed, 2026-08-19 at 12:01 +0200, Ahmad Fatoum wrote:
>> Hello,
>>
>> On 8/17/26 10:47 AM, Fabian Pflug wrote:
>>> Convert the spec in [1] to a header, that is useable for barebox.
>>> The async headers are not defined, since it is currently not planned to
>>> support async in addition to sync.
>>>
>>> [1] https://uefi.org/specs/UEFI/2.11/17_Protocols_USB_Support.html
>>
>> Some nitpicks below. You don't need to resend for just those, but if you
>> are sending a v2, it would be good to address these as well.
>>
>>>
>>> Signed-off-by: Fabian Pflug <[email protected]>
>>
>> Reviewed-by: Ahmad Fatoum <[email protected]>
>>
>>>
>>> ---
>>> v2:
>>> - Fixed wrong type in header for get_string_descriptor
>>> ---
>>>  include/efi/protocol/usb.h | 66 ++++++++++++++++++++++++++++++++++++++++++++++
>>>  1 file changed, 66 insertions(+)
>>>
>>> diff --git a/include/efi/protocol/usb.h b/include/efi/protocol/usb.h
>>> new file mode 100644
>>> index 0000000000..78953a4fa7
>>> --- /dev/null
>>> +++ b/include/efi/protocol/usb.h
>>> @@ -0,0 +1,66 @@
>>> +/* SPDX-License-Identifier: GPL-2.0-only */
>>> +
>>> +#ifndef __EFI_PROTOCOL_USB_H_
>>> +#define __EFI_PROTOCOL_USB_H_
>>> +
>>> +#include <efi/types.h>
>>> +#include <linux/usb/usb.h>
>>
>> Nitpick: I would drop this and just predeclare struct devrequest.
> 
> It is also
> 
> struct usb_config_descriptor
> struct usb_device_descriptor
> struct usb_interface_descriptor
> struct usb_endpoint_descriptor

as all of these are used behind a pointer, just predeclare them as well
(don't define them).

> 
>>
>>> +
>>> +enum efi_usb_data_direction {
>>> +	EFI_USB_DATA_IN,
>>> +	EFI_USB_DATA_OUT,
>>> +	EFI_USB_NO_DATA
>>> +};
>>> +
>>> +//
>>> +// Error code for USB Transfer Results
>>> +//
>>> +#define EFI_USB_NOERROR 0x0000
>>> +#define EFI_USB_ERR_NOTEXECUTE 0x0001
>>> +#define EFI_USB_ERR_STALL 0x0002
>>> +#define EFI_USB_ERR_BUFFER 0x0004
>>> +#define EFI_USB_ERR_BABBLE 0x0008
>>> +#define EFI_USB_ERR_NAK 0x0010
>>> +#define EFI_USB_ERR_CRC 0x0020
>>> +#define EFI_USB_ERR_TIMEOUT 0x0040
>>> +#define EFI_USB_ERR_BITSTUFF 0x0080
>>> +#define EFI_USB_ERR_SYSTEM 0x0100
>>
>> Nitpick: Add some tabs between the macros and the values for indentation?
> 
> I could, but clang-format will remove the tabs. And I like to just call it to format the rest and not have the hassle to
> revert formatting here.

Does it also do that, when you sync against an up-to-date .clang-format
from the kernel?

Cheers,
Ahmad

> 
> /Fabian
> 
>>
>>> +struct efi_usb_io_protocol {
>>> +	efi_status_t(EFIAPI *control_transfer)(
>>> +		struct efi_usb_io_protocol *this, struct devrequest *request,
>>> +		enum efi_usb_data_direction direction, u32 timeout, void *data,
>>> +		efi_uintn_t length, u32 *status);
>>> +	efi_status_t(EFIAPI *bulk_transfer)(struct efi_usb_io_protocol *this,
>>> +					    u8 device_endpoint, void *data,
>>> +					    efi_uintn_t *length,
>>> +					    efi_uintn_t timeout, u32 *status);
>>> +	void *usb_io_async_interrupt_transfer;
>>> +	efi_status_t(EFIAPI *sync_interrupt_transfer)(
>>> +		struct efi_usb_io_protocol *this, u8 device_endpoint,
>>> +		void *data, efi_uintn_t *length, efi_uintn_t timeout,
>>> +		u32 *status);
>>> +	void *efi_usb_io_isochronous_transfer;
>>> +	void *efi_usb_io_async_isochronous_transfer;
>>> +	efi_status_t(EFIAPI *get_device_descriptor)(
>>> +		struct efi_usb_io_protocol *this,
>>> +		struct usb_device_descriptor *desc);
>>> +	efi_status_t(EFIAPI *get_config_descriptor)(
>>> +		struct efi_usb_io_protocol *this,
>>> +		struct usb_config_descriptor *desc);
>>> +	efi_status_t(EFIAPI *get_interface_descriptor)(
>>> +		struct efi_usb_io_protocol *this,
>>> +		struct usb_interface_descriptor *desc);
>>> +	efi_status_t(EFIAPI *get_endpoint_descriptor)(
>>> +		struct efi_usb_io_protocol *this, u8 index,
>>> +		struct usb_endpoint_descriptor *desc);
>>> +	efi_status_t(EFIAPI *get_string_descriptor)(
>>> +		struct efi_usb_io_protocol *this, u16 lang_id, u8 string_id,
>>> +		wchar_t **desc);
>>> +	efi_status_t(EFIAPI *get_supported_languages)(
>>> +		struct efi_usb_io_protocol *this, u16 **lang_id_table,
>>> +		u16 *table_size);
>>> +	efi_status_t(EFIAPI *port_reset)(struct efi_usb_io_protocol *this);
>>> +};
>>> +
>>> +#endif
>>>

-- 
Pengutronix e.K.                  |                             |
Steuerwalder Str. 21              | http://www.pengutronix.de/  |
31137 Hildesheim, Germany         | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686  | Fax:   +49-5121-206917-5555 |