By convention (according to doc) if function does not provide get_alt() callback composite framework should assume that it has only altsetting 0 and should respond with error if host tries to set other one. After commit dd4dff8b035f ("USB: composite: Fix bug: should test set_alt function pointer before use it") we started checking set_alt() callback instead of get_alt(). This check is useless as we check if set_alt() is set inside usb_add_function() and fail if it's NULL. Let's fix this check and move comment about why we check the get method instead of set a little bit closer to prevent future false fixes.
Unified · Split
| @@ -1694,9 +1694,7 @@ composite_setup(struct usb_gadget *gadget, const struct usb_ctrlrequest *ctrl) · 1693 unmodified lines | ||
| 1694 | 1694 | value = min(w_length, (u16) 1); |
| 1695 | 1695 | break; |
| 1696 | 1696 | |
| 1697 | − /* function drivers must handle get/set altsetting; if there's | |
| 1698 | − * no get() method, we know only altsetting zero works. | |
| 1699 | − */ | |
| 1697 | + /* function drivers must handle get/set altsetting */ | |
| 1700 | 1698 | case USB_REQ_SET_INTERFACE: |
| 1701 | 1699 | if (ctrl->bRequestType != USB_RECIP_INTERFACE) |
| 1702 | 1700 | goto unknown; |
| @@ -1705,7 +1703,13 @@ composite_setup(struct usb_gadget *gadget, const struct usb_ctrlrequest *ctrl) · 2 unmodified lines | ||
| 1705 | 1703 | f = cdev->config->interface[intf]; |
| 1706 | 1704 | if (!f) |
| 1707 | 1705 | break; |
| 1708 | − if (w_value && !f->set_alt) | |
| 1706 | + | |
| 1707 | + /* | |
| 1708 | + * If there's no get_alt() method, we know only altsetting zero | |
| 1709 | + * works. There is no need to check if set_alt() is not NULL | |
| 1710 | + * as we check this in usb_add_function(). | |
| 1711 | + */ | |
| 1712 | + if (w_value && !f->get_alt) | |
| 1709 | 1713 | break; |
| 1710 | 1714 | value = f->set_alt(f, w_index, w_value); |
| 1711 | 1715 | if (value == USB_GADGET_DELAYED_STATUS) { |