Re: [PATCH v2 3/5] drivers: usb: host: efi: add efi io driver

Hi,

On 8/17/26 10:47 AM, Fabian Pflug wrote:
> +config USB_EFI_IO_PROTOCOL
> +	bool "EFI USB I/O Protocol"
> +	depends on EFI_PAYLOAD
> +	depends on USB_HOST

This depends is redundant, because the whole file is only included when
USB_HOST is selected.

> +static int efi_usb_error_check(efi_status_t efiret, int status)
> +{
> +	if (efiret == EFI_DEVICE_ERROR) {
> +		if (status & EFI_USB_ERR_TIMEOUT)
> +			return -ETIMEDOUT;
> +		if (status & EFI_USB_ERR_STALL)
> +			return -ETIMEDOUT;

return -EPIPE


> +
> +	interface->no_of_ep = interface->desc.bNumEndpoints;
> +
> +	if (interface->no_of_ep > USB_MAXENDPOINTS) {
> +		err = -EPROTO;
> +		goto out_err;
> +	}
> +
> +	for (int i = 0; i < interface->no_of_ep; i++) {
> +		efiret = priv->protocol->get_endpoint_descriptor(
> +			priv->protocol, i, &interface->ep_desc[i]);
> +		if (EFI_ERROR(efiret))
> +			return -efi_errno(efiret);

Should be goto.

> +		usb_set_maxpacket_ep(dev, &interface->ep_desc[i]);
> +	}
> +
> +	efiret = priv->protocol->get_supported_languages(priv->protocol,
> +							 &lang_ids, &table_size);
> +	if (EFI_ERROR(efiret)) {
> +		err = -efi_errno(efiret);
> +		goto out_err;
> +	}
> +
> +	/* table size is given in bytes, not entries */
> +	if (table_size < sizeof(*lang_ids)) {
> +		err = -EPROTO;
> +		goto out_err;
> +	}
> +
> +	dev->string_langid = lang_ids[0];
> +	dev->have_langid = true;

Set this to false when table_size == 0 instead of an error?

> +
> +	dev_info(&dev->dev, "new device: Mfr=%d, Product=%d, SerialNumber=%d\n",
> +		 dev->descriptor->iManufacturer, dev->descriptor->iProduct,
> +		 dev->descriptor->iSerialNumber);
> +
> +	err = efi_get_usb_string(priv->protocol, dev->string_langid,
> +				 dev->descriptor->iManufacturer, dev->mf,
> +				 sizeof(dev->mf));
> +	if (err)
> +		goto out_err;
> +	err = efi_get_usb_string(priv->protocol, dev->string_langid,
> +				 dev->descriptor->iProduct, dev->prod,
> +				 sizeof(dev->prod));
> +	if (err)
> +		goto out_err;
> +	err = efi_get_usb_string(priv->protocol, dev->string_langid,
> +				 dev->descriptor->iSerialNumber, dev->serial,
> +				 sizeof(dev->serial));
> +	if (err)
> +		goto out_err;
> +
> +	dev_info(&dev->dev, "Bus %03d Device %03d: ID %04x:%04x %s\n",
> +		 dev->host->busnum, dev->devnum, dev->descriptor->idVendor,
> +		 dev->descriptor->idProduct, dev->prod);
> +
> +	err = register_device(&dev->dev);
> +	if (err) {
> +		dev_err(&dev->dev, "Failed to register device: %pe\n",
> +			ERR_PTR(err));
> +		goto out_err;
> +	}
> +
> +	// register as root device for host
> +	host->root_dev = dev;
> +
> +	return 0;
> +
> +out_err:
> +	dev_err(&dev->dev, "Failed to create UEFI-USB-IO device: %pe\n",
> +		ERR_PTR(err));
> +	usb_free_device(dev);
> +	return err;
> +}
> +
> +static int efi_usb_io_probe(struct efi_device *efidev)
> +{
> +	struct device *dev = &efidev->dev;
> +	struct efi_usb_io_priv *priv;
> +	struct usb_host *host;
> +
> +	priv = xzalloc(sizeof(*priv));
> +
> +	BS->handle_protocol(efidev->handle, &efi_usb_io_protocol_guid,
> +			    (void **)&priv->protocol);
> +	if (!priv->protocol)
> +		return -ENODEV;
> +
> +	dev->priv = priv;
> +	priv->dev = dev;
> +
> +	// EFI has one device per probe, which now needs its own host controller,
> +	// since there is no shared host controller resource.
> +
> +	host = &priv->host;
> +	host->submit_int_msg = efi_usb_int_msg;
> +	host->submit_control_msg = efi_usb_control_msg;
> +	host->submit_bulk_msg = efi_usb_bulk_msg;

host->hw_dev must be set.

Cheers,
Ahmad

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