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

[Ahmad Fatoum <a.fatoum@pengutronix.de>]

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 <f.pflug@pengutronix.de>

Reviewed-by: Ahmad Fatoum <a.fatoum@pengutronix.de>

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

> +
> +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?

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