Skip to content

usb: device_next: allow to pass maximum possible MPS value to drivers #92831

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 4 additions & 10 deletions drivers/usb/udc/udc_ambiq.c
Original file line number Diff line number Diff line change
Expand Up @@ -640,14 +640,11 @@ static int udc_ambiq_init(const struct device *dev)
#endif

/* Enable Control Endpoints */
if (udc_ep_enable_internal(dev, USB_CONTROL_EP_OUT, USB_EP_TYPE_CONTROL, EP0_MPS, 0)) {
LOG_ERR("Failed to enable control endpoint");
return -EIO;
}
if (udc_ep_enable_internal(dev, USB_CONTROL_EP_IN, USB_EP_TYPE_CONTROL, EP0_MPS, 0)) {
if (udc_ep_enable_control(dev, EP0_MPS)) {
LOG_ERR("Failed to enable control endpoint");
return -EIO;
}

/* Connect and enable USB interrupt */
cfg->irq_enable_func(dev);

Expand All @@ -663,14 +660,11 @@ static int udc_ambiq_shutdown(const struct device *dev)
LOG_INF("shutdown");

/* Disable Control Endpoints */
if (udc_ep_disable_internal(dev, USB_CONTROL_EP_OUT)) {
LOG_ERR("Failed to disable control endpoint");
return -EIO;
}
if (udc_ep_disable_internal(dev, USB_CONTROL_EP_IN)) {
if (udc_ep_disable_control(dev)) {
LOG_ERR("Failed to disable control endpoint");
return -EIO;
}

/* Disable USB interrupt */
cfg->irq_disable_func(dev);
/* Assert USB PHY reset */
Expand Down
5 changes: 4 additions & 1 deletion drivers/usb/udc/udc_common.c
Original file line number Diff line number Diff line change
Expand Up @@ -330,6 +330,7 @@ int udc_ep_enable_internal(const struct device *dev,
const uint8_t ep,
const uint8_t attributes,
const uint16_t mps,
const uint16_t m_mps,
const uint8_t interval)
{
const struct udc_api *api = dev->api;
Expand All @@ -353,6 +354,7 @@ int udc_ep_enable_internal(const struct device *dev,

cfg->attributes = attributes;
cfg->mps = mps;
cfg->m_mps = m_mps;
cfg->interval = interval;

cfg->stat.odd = 0;
Expand All @@ -368,6 +370,7 @@ int udc_ep_enable(const struct device *dev,
const uint8_t ep,
const uint8_t attributes,
const uint16_t mps,
const uint16_t m_mps,
const uint8_t interval)
{
const struct udc_api *api = dev->api;
Expand All @@ -384,7 +387,7 @@ int udc_ep_enable(const struct device *dev,
goto ep_enable_error;
}

ret = udc_ep_enable_internal(dev, ep, attributes, mps, interval);
ret = udc_ep_enable_internal(dev, ep, attributes, mps, m_mps, interval);

ep_enable_error:
api->unlock(dev);
Expand Down
49 changes: 49 additions & 0 deletions drivers/usb/udc/udc_common.h
Original file line number Diff line number Diff line change
Expand Up @@ -198,8 +198,36 @@ int udc_ep_enable_internal(const struct device *dev,
const uint8_t ep,
const uint8_t attributes,
const uint16_t mps,
const uint16_t m_mps,
const uint8_t interval);

/**
* @brief Helper function to enable control endpoint.
*
* This function can be used by the driver to enable control endpoint.
*
* @param[in] dev Pointer to device struct of the driver instance
* @param[in] mps Maximum packet size (same as wMaxPacketSize)
*
* @return 0 on success, all other values should be treated as error.
* @retval -ENODEV endpoint is not assigned or no configuration found
* @retval -EALREADY endpoint is already enabled
*/
static inline int udc_ep_enable_control(const struct device *dev,
const uint16_t mps)
{
int err;

err = udc_ep_enable_internal(dev, USB_CONTROL_EP_OUT,
USB_EP_TYPE_CONTROL, mps, mps, 0);
if (err) {
return err;
}

return udc_ep_enable_internal(dev, USB_CONTROL_EP_IN,
USB_EP_TYPE_CONTROL, mps, mps, 0);
}

/**
* @brief Helper function to disable endpoint.
*
Expand All @@ -215,6 +243,27 @@ int udc_ep_enable_internal(const struct device *dev,
int udc_ep_disable_internal(const struct device *dev,
const uint8_t ep);

/**
* @brief Helper function to disable control endpoint.
*
* This function can be used by the driver to disable control endpoint.
*
* @param[in] dev Pointer to device struct of the driver instance
*
* @return 0 on success, all other values should be treated as error.
* @retval -ENODEV endpoint is not assigned or no configuration found
* @retval -EALREADY endpoint is already enabled
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like copy&paste error, if endpoint is "already enabled" then disable should succeed.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This -EALRADY description seems wrong. It is disable function.

*/
static inline int udc_ep_disable_control(const struct device *dev)
{
int err1, err2;

err1 = udc_ep_disable_internal(dev, USB_CONTROL_EP_IN);
err2 = udc_ep_disable_internal(dev, USB_CONTROL_EP_OUT);

return err1 ? err1 : err2;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This modifies the sequence that is performed on endpoint disable. Now the disable function will be called on both IN and OUT (before it was called first on OUT and then on IN) regardless of the returned error. Before first OUT endpoint was disabled and then IN endpoint was disabled only if OUT endpoint disable succeeded.

}

/**
* @brief Helper function to register endpoint configuration.
*
Expand Down
22 changes: 7 additions & 15 deletions drivers/usb/udc/udc_dwc2.c
Original file line number Diff line number Diff line change
Expand Up @@ -1315,7 +1315,11 @@ static int dwc2_set_dedicated_fifo(const struct device *dev,
/* Keep everything but FIFO number */
tmp = *diepctl & ~USB_DWC2_DEPCTL_TXFNUM_MASK;

reqdep = DIV_ROUND_UP(udc_mps_ep_size(cfg), 4U);
/* Use the maximum possible MPS value to ensure that the alternate
* setting does not result in too small memory window being allocated
* and locked because a higher FIFO is still in use.
*/
reqdep = DIV_ROUND_UP(USB_MPS_EP_SIZE(cfg->m_mps), 4U);
if (dwc2_in_buffer_dma_mode(dev)) {
/* In DMA mode, TxFIFO capable of holding 2 packets is enough */
reqdep *= MIN(2, (1 + addnl));
Expand Down Expand Up @@ -2136,14 +2140,7 @@ static int udc_dwc2_init_controller(const struct device *dev)
i, priv->max_txfifo_depth[i], dwc2_get_txfaddr(dev, i));
}

if (udc_ep_enable_internal(dev, USB_CONTROL_EP_OUT,
USB_EP_TYPE_CONTROL, 64, 0)) {
LOG_ERR("Failed to enable control endpoint");
return -EIO;
}

if (udc_ep_enable_internal(dev, USB_CONTROL_EP_IN,
USB_EP_TYPE_CONTROL, 64, 0)) {
if (udc_ep_enable_control(dev, 64)) {
LOG_ERR("Failed to enable control endpoint");
return -EIO;
}
Expand Down Expand Up @@ -2206,12 +2203,7 @@ static int udc_dwc2_disable(const struct device *dev)

LOG_DBG("Disable device %p", dev);

if (udc_ep_disable_internal(dev, USB_CONTROL_EP_OUT)) {
LOG_DBG("Failed to disable control endpoint");
return -EIO;
}

if (udc_ep_disable_internal(dev, USB_CONTROL_EP_IN)) {
if (udc_ep_disable_control(dev)) {
LOG_DBG("Failed to disable control endpoint");
return -EIO;
}
Expand Down
18 changes: 3 additions & 15 deletions drivers/usb/udc/udc_it82xx2.c
Original file line number Diff line number Diff line change
Expand Up @@ -1469,30 +1469,18 @@ static int it82xx2_init(const struct device *dev)

it82xx2_usb_dc_ip_init(dev);

ret = udc_ep_enable_internal(dev, USB_CONTROL_EP_OUT, USB_EP_TYPE_CONTROL,
config->ep_cfg_out[0].caps.mps, 0);
ret = udc_ep_enable_control(dev, config->ep_cfg_out[0].caps.mps);
if (ret) {
LOG_ERR("Failed to enable ep 0x%02x", USB_CONTROL_EP_OUT);
LOG_ERR("Failed to enable control endpoint");
return ret;
}

ret = udc_ep_enable_internal(dev, USB_CONTROL_EP_IN, USB_EP_TYPE_CONTROL,
config->ep_cfg_in[0].caps.mps, 0);
if (ret) {
LOG_ERR("Failed to enable ep 0x%02x", USB_CONTROL_EP_IN);
return ret;
}
return 0;
}

static int it82xx2_shutdown(const struct device *dev)
{
if (udc_ep_disable_internal(dev, USB_CONTROL_EP_OUT)) {
LOG_ERR("Failed to disable control endpoint");
return -EIO;
}

if (udc_ep_disable_internal(dev, USB_CONTROL_EP_IN)) {
if (udc_ep_disable_control(dev)) {
LOG_ERR("Failed to disable control endpoint");
return -EIO;
}
Expand Down
18 changes: 2 additions & 16 deletions drivers/usb/udc/udc_kinetis.c
Original file line number Diff line number Diff line change
Expand Up @@ -1020,16 +1020,7 @@ static int usbfsotg_init(const struct device *dev)
USB_INTEN_ERROREN_MASK |
USB_INTEN_USBRSTEN_MASK);

if (udc_ep_enable_internal(dev, USB_CONTROL_EP_OUT,
USB_EP_TYPE_CONTROL,
USBFSOTG_EP0_SIZE, 0)) {
LOG_ERR("Failed to enable control endpoint");
return -EIO;
}

if (udc_ep_enable_internal(dev, USB_CONTROL_EP_IN,
USB_EP_TYPE_CONTROL,
USBFSOTG_EP0_SIZE, 0)) {
if (udc_ep_enable_control(dev, USBFSOTG_EP0_SIZE)) {
LOG_ERR("Failed to enable control endpoint");
return -EIO;
}
Expand All @@ -1048,12 +1039,7 @@ static int usbfsotg_shutdown(const struct device *dev)

config->irq_disable_func(dev);

if (udc_ep_disable_internal(dev, USB_CONTROL_EP_OUT)) {
LOG_ERR("Failed to disable control endpoint");
return -EIO;
}

if (udc_ep_disable_internal(dev, USB_CONTROL_EP_IN)) {
if (udc_ep_disable_control(dev)) {
LOG_ERR("Failed to disable control endpoint");
return -EIO;
}
Expand Down
14 changes: 2 additions & 12 deletions drivers/usb/udc/udc_max32.c
Original file line number Diff line number Diff line change
Expand Up @@ -650,12 +650,7 @@ static int udc_max32_init(const struct device *dev)
return ret;
}

if (udc_ep_enable_internal(dev, USB_CONTROL_EP_OUT, USB_EP_TYPE_CONTROL, 64, 0)) {
LOG_ERR("Failed to enable control endpoint");
return -EIO;
}

if (udc_ep_enable_internal(dev, USB_CONTROL_EP_IN, USB_EP_TYPE_CONTROL, 64, 0)) {
if (udc_ep_enable_control(dev, 64)) {
LOG_ERR("Failed to enable control endpoint");
return -EIO;
}
Expand All @@ -670,12 +665,7 @@ static int udc_max32_shutdown(const struct device *dev)
MXC_USB_Shutdown();
irq_disable(DT_INST_IRQN(0));

if (udc_ep_disable_internal(dev, USB_CONTROL_EP_OUT)) {
LOG_ERR("Failed to disable control endpoint");
return -EIO;
}

if (udc_ep_disable_internal(dev, USB_CONTROL_EP_IN)) {
if (udc_ep_disable_control(dev)) {
LOG_ERR("Failed to disable control endpoint");
return -EIO;
}
Expand Down
20 changes: 2 additions & 18 deletions drivers/usb/udc/udc_mcux_ehci.c
Original file line number Diff line number Diff line change
Expand Up @@ -442,28 +442,12 @@ static void udc_mcux_work_handler(struct k_work *item)
mcux_msg = &ev->mcux_msg;

if (mcux_msg->code == kUSB_DeviceNotifyBusReset) {
struct udc_ep_config *cfg;

udc_mcux_control(ev->dev, kUSB_DeviceControlSetDefaultStatus, NULL);
cfg = udc_get_ep_cfg(ev->dev, USB_CONTROL_EP_OUT);
if (cfg->stat.enabled) {
udc_ep_disable_internal(ev->dev, USB_CONTROL_EP_OUT);
}
cfg = udc_get_ep_cfg(ev->dev, USB_CONTROL_EP_IN);
if (cfg->stat.enabled) {
udc_ep_disable_internal(ev->dev, USB_CONTROL_EP_IN);
}
if (udc_ep_enable_internal(ev->dev, USB_CONTROL_EP_OUT,
USB_EP_TYPE_CONTROL,
USB_MCUX_EP0_SIZE, 0)) {
(void)udc_ep_disable_control(ev->dev);
if (udc_ep_enable_control(ev->dev, USB_MCUX_EP0_SIZE)) {
LOG_ERR("Failed to enable control endpoint");
}

if (udc_ep_enable_internal(ev->dev, USB_CONTROL_EP_IN,
USB_EP_TYPE_CONTROL,
USB_MCUX_EP0_SIZE, 0)) {
LOG_ERR("Failed to enable control endpoint");
}
udc_submit_event(ev->dev, UDC_EVT_RESET, 0);
} else {
ep = mcux_msg->code;
Expand Down
20 changes: 2 additions & 18 deletions drivers/usb/udc/udc_mcux_ip3511.c
Original file line number Diff line number Diff line change
Expand Up @@ -442,28 +442,12 @@ static void udc_mcux_work_handler(struct k_work *item)
mcux_msg = &ev->mcux_msg;

if (mcux_msg->code == kUSB_DeviceNotifyBusReset) {
struct udc_ep_config *cfg;

udc_mcux_control(ev->dev, kUSB_DeviceControlSetDefaultStatus, NULL);
cfg = udc_get_ep_cfg(ev->dev, USB_CONTROL_EP_OUT);
if (cfg->stat.enabled) {
udc_ep_disable_internal(ev->dev, USB_CONTROL_EP_OUT);
}
cfg = udc_get_ep_cfg(ev->dev, USB_CONTROL_EP_IN);
if (cfg->stat.enabled) {
udc_ep_disable_internal(ev->dev, USB_CONTROL_EP_IN);
}
if (udc_ep_enable_internal(ev->dev, USB_CONTROL_EP_OUT,
USB_EP_TYPE_CONTROL,
USB_MCUX_EP0_SIZE, 0)) {
(void)udc_ep_disable_control(ev->dev);
if (udc_ep_enable_control(ev->dev, USB_MCUX_EP0_SIZE)) {
LOG_ERR("Failed to enable control endpoint");
}

if (udc_ep_enable_internal(ev->dev, USB_CONTROL_EP_IN,
USB_EP_TYPE_CONTROL,
USB_MCUX_EP0_SIZE, 0)) {
LOG_ERR("Failed to enable control endpoint");
}
udc_submit_event(ev->dev, UDC_EVT_RESET, 0);
} else {
ep = mcux_msg->code;
Expand Down
16 changes: 2 additions & 14 deletions drivers/usb/udc/udc_nrf.c
Original file line number Diff line number Diff line change
Expand Up @@ -1705,14 +1705,7 @@ static int udc_nrf_enable(const struct device *dev)
unsigned int key;
int ret;

if (udc_ep_enable_internal(dev, USB_CONTROL_EP_OUT,
USB_EP_TYPE_CONTROL, UDC_NRF_EP0_SIZE, 0)) {
LOG_ERR("Failed to enable control endpoint");
return -EIO;
}

if (udc_ep_enable_internal(dev, USB_CONTROL_EP_IN,
USB_EP_TYPE_CONTROL, UDC_NRF_EP0_SIZE, 0)) {
if (udc_ep_enable_control(dev, UDC_NRF_EP0_SIZE)) {
LOG_ERR("Failed to enable control endpoint");
return -EIO;
}
Expand All @@ -1738,12 +1731,7 @@ static int udc_nrf_disable(const struct device *dev)

nrf_usbd_legacy_disable();

if (udc_ep_disable_internal(dev, USB_CONTROL_EP_OUT)) {
LOG_ERR("Failed to disable control endpoint");
return -EIO;
}

if (udc_ep_disable_internal(dev, USB_CONTROL_EP_IN)) {
if (udc_ep_disable_control(dev)) {
LOG_ERR("Failed to disable control endpoint");
return -EIO;
}
Expand Down
14 changes: 2 additions & 12 deletions drivers/usb/udc/udc_numaker.c
Original file line number Diff line number Diff line change
Expand Up @@ -1608,12 +1608,7 @@ static int udc_numaker_init(const struct device *dev)
/* Initialize all EP H/W contexts */
numaker_usbd_ep_mgmt_init(dev);

if (udc_ep_enable_internal(dev, USB_CONTROL_EP_OUT, USB_EP_TYPE_CONTROL, 64, 0)) {
LOG_ERR("Failed to enable control endpoint");
return -EIO;
}

if (udc_ep_enable_internal(dev, USB_CONTROL_EP_IN, USB_EP_TYPE_CONTROL, 64, 0)) {
if (udc_ep_enable_control(dev, 64)) {
LOG_ERR("Failed to enable control endpoint");
return -EIO;
}
Expand All @@ -1625,12 +1620,7 @@ static int udc_numaker_shutdown(const struct device *dev)
{
struct udc_numaker_data *priv = udc_get_private(dev);

if (udc_ep_disable_internal(dev, USB_CONTROL_EP_OUT)) {
LOG_ERR("Failed to disable control endpoint");
return -EIO;
}

if (udc_ep_disable_internal(dev, USB_CONTROL_EP_IN)) {
if (udc_ep_disable_control(dev)) {
LOG_ERR("Failed to disable control endpoint");
return -EIO;
}
Expand Down
Loading