| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <BYAPR07MB470933EB8113360DB8FCF7F5DDAD0@BYAPR07MB4709.namprd07.prod.outlook.com>
Date: Sun, 2 Dec 2018 16:39:52 +0000
From: Pawel Laszczak <pawell@...ence.com>
To: Roger Quadros <rogerq@...com>,
"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>
CC: "gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
"linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
Alan Douglas <adouglas@...ence.com>,
"jbergsagel@...com" <jbergsagel@...com>,
"nsekhar@...com" <nsekhar@...com>, "nm@...com" <nm@...com>,
Suresh Punnoose <sureshp@...ence.com>,
"peter.chen@....com" <peter.chen@....com>,
Pawel Jez <pjez@...ence.com>, Rahul Kumar <kurahul@...ence.com>
Subject: RE: [RFC PATCH v2 12/15] usb:cdns3: Adds enumeration related
function.
>On 18/11/18 12:09, Pawel Laszczak wrote:
>> Patch implements a set of function related to enumeration process.
>> Some standard requests are handled on controller driver level and
>> other are delegated to gadget core driver.
>> All class requests are delegated to gadget core driver.
>>
>> Signed-off-by: Pawel Laszczak <pawell@...ence.com>
>> ---
>> drivers/usb/cdns3/ep0.c | 491 ++++++++++++++++++++++++++++++++++++-
>> drivers/usb/cdns3/gadget.c | 119 +++++++++
>> drivers/usb/cdns3/gadget.h | 4 +
>> 3 files changed, 610 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/usb/cdns3/ep0.c b/drivers/usb/cdns3/ep0.c
>> index eb92fd234bd7..6f33d98f7684 100644
>> --- a/drivers/usb/cdns3/ep0.c
>> +++ b/drivers/usb/cdns3/ep0.c
>> @@ -10,6 +10,7 @@
>> * Peter Chen <peter.chen@....com>
>> */
>>
>> +#include <linux/usb/composite.h>
>> #include "gadget.h"
>>
>> static struct usb_endpoint_descriptor cdns3_gadget_ep0_desc = {
>> @@ -52,9 +53,31 @@ static void cdns3_ep0_run_transfer(struct cdns3_device *priv_dev,
>> writel(EP_CMD_ERDY, &priv_dev->regs->ep_cmd);
>> }
>>
>> +/**
>> + * cdns3_ep0_delegate_req - Returns status of handling setup packet
>> + * Setup is handled by gadget driver
>> + * @priv_dev: extended gadget object
>> + * @ctrl_req: pointer to received setup packet
>> + *
>> + * Returns zero on success or negative value on failure
>> + */
>> +static int cdns3_ep0_delegate_req(struct cdns3_device *priv_dev,
>> + struct usb_ctrlrequest *ctrl_req)
>> +{
>> + int ret;
>> +
>> + spin_unlock(&priv_dev->lock);
>> + priv_dev->setup_pending = 1;
>> + ret = priv_dev->gadget_driver->setup(&priv_dev->gadget, ctrl_req);
>> + priv_dev->setup_pending = 0;
>
>Why is setup_pending flag being set and cleared?
This flag is checking during handling DESCMISS interrupt. If this flag is set
then driver not start DMA for SETUP packet waiting in on-chip buffer.
>
>> + spin_lock(&priv_dev->lock);
>> + return ret;
>> +}
>> +
>> static void cdns3_prepare_setup_packet(struct cdns3_device *priv_dev)
>> {
>> - //TODO: Implements this function
>> + priv_dev->ep0_data_dir = 0;
>> + cdns3_ep0_run_transfer(priv_dev, priv_dev->setup_dma, 8, 0);
>
>why hardcode to 8?
>Don't vendor specific requests have different lengths?
SETUP packet always has 8 bytes.
I will change this to sizeof(struct struct usb_ctrlrequest). It says more.
>
>> }
>>
>> static void cdns3_set_hw_configuration(struct cdns3_device *priv_dev)
>> @@ -90,9 +113,431 @@ static void cdns3_set_hw_configuration(struct cdns3_device *priv_dev)
>> }
>> }
>>
>> +/**
>> + * cdns3_req_ep0_set_configuration - Handling of SET_CONFIG standard USB request
>> + * @priv_dev: extended gadget object
>> + * @ctrl_req: pointer to received setup packet
>> + *
>> + * Returns 0 if success, 0x7FFF on deferred status stage, error code on error
>
>what is this magic number 0x7fff?
We will have USB_GADGET_DELAYED_STATUS instead 0x7FFF;
>
>> + */
>> +static int cdns3_req_ep0_set_configuration(struct cdns3_device *priv_dev,
>> + struct usb_ctrlrequest *ctrl_req)
>> +{
>> + enum usb_device_state device_state = priv_dev->gadget.state;
>> + struct cdns3_endpoint *priv_ep, *temp_ep;
>> + u32 config = le16_to_cpu(ctrl_req->wValue);
>> + int result = 0;
>> +
>> + switch (device_state) {
>> + case USB_STATE_ADDRESS:
>> + /* Configure non-control EPs */
>> + list_for_each_entry_safe(priv_ep, temp_ep,
>> + &priv_dev->ep_match_list,
>> + ep_match_pending_list)
>> + cdns3_ep_config(priv_ep);
>
>Why configure here? They should be configured at ep_enable. no?
>And you don't need to maintain a ep_match/pending_list.
It's a little tricky.
We need to configure all endpoint on SET_CONFIGURATION request.
After reset we can set only once CFGSET bit in usb_conf register.
We don't need to enable endpoint, but we need set all other parameters.
Not all endpoints are enabled before SET_CONFIGURATION, but
most of them, or even all are claimed during loading function by means
of usb_ep_autoconfig.
After setting CFGSET bit we can't change configuration of any endpoint.
We can only enable or disable them.
>> +
>> + result = cdns3_ep0_delegate_req(priv_dev, ctrl_req);
>> +
>> + if (result)
>> + return result;
>> +
>> + if (config) {
>
>What if result is USB_GADGET_DELAYED_STATUS?
It's handled be cdns3_ep0_setup_phase when this function return USB_GADGET_DELAYED_STATUS
>
>> + cdns3_set_hw_configuration(priv_dev);
>
>usb_gadget_set_state(USB_STATE_CONFIGURED) ?
It is set in set_config in composite driver.
I think that it is sufficient.
>
>> + } else {
>> + cdns3_gadget_unconfig(priv_dev);
>> + usb_gadget_set_state(&priv_dev->gadget,
>> + USB_STATE_ADDRESS);
>> + }
>> + break;
>> + case USB_STATE_CONFIGURED:
>> + result = cdns3_ep0_delegate_req(priv_dev, ctrl_req);
>> +
>> + if (!config && !result) {
>> + cdns3_gadget_unconfig(priv_dev);
>> + usb_gadget_set_state(&priv_dev->gadget,
>> + USB_STATE_ADDRESS);
>> + }
>> + break;
>> + default:
>> + result = -EINVAL;
>> + }
>> +
>> + return result;
>> +}
>> +
>> +/**
>> + * cdns3_req_ep0_set_address - Handling of SET_ADDRESS standard USB request
>> + * @priv_dev: extended gadget object
>> + * @ctrl_req: pointer to received setup packet
>> + *
>> + * Returns 0 if success, error code on error
>> + */
>> +static int cdns3_req_ep0_set_address(struct cdns3_device *priv_dev,
>> + struct usb_ctrlrequest *ctrl_req)
>> +{
>> + enum usb_device_state device_state = priv_dev->gadget.state;
>> + u32 reg;
>> + u32 addr;
>> +
>> + addr = le16_to_cpu(ctrl_req->wValue);
>> +
>> + if (addr > DEVICE_ADDRESS_MAX) {
>
>If DEVICE_ADDRESS_MAX comes from USB spec it must be in ch9.h.
>Maybe add something like
>
>#define USB_DEVICE_MAX_ADDRESS 127
>
Yes, it should, but I didn't see such macro definition in ch9.h.
I will add change this to USB_DEVICE_MAX_ADDRESS.
>> + dev_err(&priv_dev->dev,
>> + "Device address (%d) cannot be greater than %d\n",
>> + addr, DEVICE_ADDRESS_MAX);
>> + return -EINVAL;
>> + }
>> +
>> + if (device_state == USB_STATE_CONFIGURED) {
>> + dev_err(&priv_dev->dev, "USB device already configured\n");
>
>Message is misleading. How about "can't set_address from configured state"
>
Sounds better.
>> + return -EINVAL;
>> + }
>> +
>> + reg = readl(&priv_dev->regs->usb_cmd);
>> +
>> + writel(reg | USB_CMD_FADDR(addr) | USB_CMD_SET_ADDR,
>> + &priv_dev->regs->usb_cmd);
>> +
>> + usb_gadget_set_state(&priv_dev->gadget,
>> + (addr ? USB_STATE_ADDRESS : USB_STATE_DEFAULT));
>> +
>> + cdns3_prepare_setup_packet(priv_dev);
>
>why call this here? This should be done after the current ep0 request is complete.
It only arm DMA for next SETUP packet.
It's allow to eliminate one extra DESCMISS interrupt. This interrupt is
generated when packet is in on-chip buffer and DMA is not started.
Additionally, all OUT endpoints have common shared FIFO buffer. It's the reason why data from
This on-chip buffers should be copied to system memory ASAP. Each delay before
getting packet from this FIFO has impact on performance.
>> +
>> + writel(EP_CMD_ERDY | EP_CMD_REQ_CMPL, &priv_dev->regs->ep_cmd);
>> +
>> + return 0;
>> +}
>> +
>> +/**
>> + * cdns3_req_ep0_get_status - Handling of GET_STATUS standard USB request
>> + * @priv_dev: extended gadget object
>> + * @ctrl_req: pointer to received setup packet
>> + *
>> + * Returns 0 if success, error code on error
>> + */
>> +static int cdns3_req_ep0_get_status(struct cdns3_device *priv_dev,
>> + struct usb_ctrlrequest *ctrl)
>> +{
>> + __le16 *response_pkt;
>> + u16 usb_status = 0;
>> + u32 recip;
>> + u32 reg;
>> +
>> + recip = ctrl->bRequestType & USB_RECIP_MASK;
>> +
>> + switch (recip) {
>> + case USB_RECIP_DEVICE:
>> + /* self powered */
>> + usb_status |= priv_dev->gadget.is_selfpowered;
>
>if (prv_devgadget.is_selfpowered)
> usb_stats |= BIT(USB_DEVICE_SELF_POWERED);
>
Ok,
I don't understand why, but I see such solution in MTU3 driver
In musb it is used in such way:
result[0] = musb->g.is_selfpowered << USB_DEVICE_SELF_POWERED;
>> +
>> + if (priv_dev->gadget.speed != USB_SPEED_SUPER)
>
>You should check controller speed directly instead.
>
>> + break;
>> +
>> + reg = readl(&priv_dev->regs->usb_sts);
>> +
>> + if (priv_dev->u1_allowed)
>> + usb_status |= BIT(USB_DEV_STAT_U1_ENABLED);
>> +
>> + if (priv_dev->u2_allowed)
>> + usb_status |= BIT(USB_DEV_STAT_U2_ENABLED);
>> +
>> + if (priv_dev->wake_up_flag)
>> + usb_status |= BIT(USB_DEVICE_REMOTE_WAKEUP);
>
>Remote wakeup is not SS specific. So needs to go before the SS check.
Yes, sure.
>
>> + break;
>> + case USB_RECIP_INTERFACE:
>> + return cdns3_ep0_delegate_req(priv_dev, ctrl);
>> + case USB_RECIP_ENDPOINT:
>> + /* check if endpoint is stalled */
>> + cdns3_select_ep(priv_dev, ctrl->wIndex);
>> + if (EP_STS_STALL(readl(&priv_dev->regs->ep_sts)))
>> + usb_status = BIT(USB_ENDPOINT_HALT);
>> + break;
>> + default:
>> + return -EINVAL;
>> + }
>> +
>> + response_pkt = (__le16 *)priv_dev->setup;
>> + *response_pkt = cpu_to_le16(usb_status);
>> +
>> + cdns3_ep0_run_transfer(priv_dev, priv_dev->setup_dma,
>> + sizeof(*response_pkt), 1);
>> + return 0;
>> +}
>> +
>> +static int cdns3_ep0_feature_handle_device(struct cdns3_device *priv_dev,
>> + struct usb_ctrlrequest *ctrl,
>> + int set)
>> +{
>> + enum usb_device_state state;
>> + enum usb_device_speed speed;
>> + int ret = 0;
>> + u32 wValue;
>> + u32 wIndex;
>> + u16 tmode;
>> +
>> + wValue = le16_to_cpu(ctrl->wValue);
>> + wIndex = le16_to_cpu(ctrl->wIndex);
>> + state = priv_dev->gadget.state;
>> + speed = priv_dev->gadget.speed;
>> +
>> + switch (ctrl->wValue) {
>> + case USB_DEVICE_REMOTE_WAKEUP:
>> + priv_dev->wake_up_flag = !!set;
>> + break;
>> + case USB_DEVICE_U1_ENABLE:
>> + if (state != USB_STATE_CONFIGURED || speed != USB_SPEED_SUPER)
>> + return -EINVAL;
>> +
>> + priv_dev->u1_allowed = !!set;
>> + break;
>> + case USB_DEVICE_U2_ENABLE:
>> + if (state != USB_STATE_CONFIGURED || speed != USB_SPEED_SUPER)
>> + return -EINVAL;
>> +
>> + priv_dev->u2_allowed = !!set;
>> + break;
>> + case USB_DEVICE_LTM_ENABLE:
>> + ret = -EINVAL;
>> + break;
>> + case USB_DEVICE_TEST_MODE:
>> + if (state != USB_STATE_CONFIGURED || speed > USB_SPEED_HIGH)
>> + return -EINVAL;
>> +
>> + tmode = le16_to_cpu(ctrl->wIndex);
>> +
>> + if (!set || (tmode & 0xff) != 0)
>> + return -EINVAL;
>> +
>> + switch (tmode >> 8) {
>> + case TEST_J:
>> + case TEST_K:
>> + case TEST_SE0_NAK:
>> + case TEST_PACKET:
>> + cdns3_set_register_bit(&priv_dev->regs->usb_cmd,
>> + USB_CMD_STMODE |
>> + USB_STS_TMODE_SEL(tmode - 1));
>> + break;
>> + default:
>> + ret = -EINVAL;
>> + }
>> + break;
>> + default:
>> + ret = -EINVAL;
>> + }
>> +
>> + return ret;
>> +}
>> +
>> +static int cdns3_ep0_feature_handle_intf(struct cdns3_device *priv_dev,
>> + struct usb_ctrlrequest *ctrl,
>> + int set)
>> +{
>> + u32 wValue;
>> + int ret = 0;
>> +
>> + wValue = le16_to_cpu(ctrl->wValue);
>> +
>> + switch (wValue) {
>> + case USB_INTRF_FUNC_SUSPEND:
>> + break;
>> + default:
>> + ret = -EINVAL;
>> + }
>> +
>> + return ret;
>> +}
>> +
>> +static int cdns3_ep0_feature_handle_endpoint(struct cdns3_device *priv_dev,
>> + struct usb_ctrlrequest *ctrl,
>> + int set)
>> +{
>> + struct cdns3_endpoint *priv_ep;
>> + int ret = 0;
>> + u8 index;
>> +
>> + if (!(ctrl->wIndex & ~USB_DIR_IN))
>> + return 0;
>
>Why is this check?
Whether endpoint in wIndex is different then 0. I use this
value in next line and it must be greater than 0.
>
>> +
>> + index = cdns3_ep_addr_to_index(ctrl->wIndex);
>> + priv_ep = priv_dev->eps[index];
>> +
>> + cdns3_select_ep(priv_dev, ctrl->wIndex);
>> +
>> + if (le16_to_cpu(ctrl->wValue) != USB_ENDPOINT_HALT)
>> + return -EINVAL;
>
>This check should be done first before you try to decode wIndex.
I understand that you mean that it doesn't make sense doing anything when endpoint is halted.
>
>> +
>> + if (set) {
>> + writel(EP_CMD_SSTALL, &priv_dev->regs->ep_cmd);
>> + priv_ep->flags |= EP_STALL;
>> + } else {
>> + struct usb_request *request;
>> +
>> + if (priv_dev->eps[index]->flags & EP_WEDGE) {
>> + cdns3_select_ep(priv_dev, 0x00);
>> + return 0;
>> + }
>> +
>> + writel(EP_CMD_CSTALL | EP_CMD_EPRST, &priv_dev->regs->ep_cmd);
>> +
>> + /* wait for EPRST cleared */
>> + ret = cdns3_handshake(&priv_dev->regs->ep_cmd,
>> + EP_CMD_EPRST, 0, 100);
>> + if (ret)
>> + return -EINVAL;
>> +
>> + priv_ep->flags &= ~EP_STALL;
>> +
>> + request = cdns3_next_request(&priv_ep->request_list);
>> + if (request)
>> + cdns3_ep_run_transfer(priv_ep, request);
>> + }
>> + return ret;
>> +}
>> +
>> +/**
>> + * cdns3_req_ep0_handle_feature -
>> + * Handling of GET/SET_FEATURE standard USB request
>> + *
>> + * @priv_dev: extended gadget object
>> + * @ctrl_req: pointer to received setup packet
>> + * @set: must be set to 1 for SET_FEATURE request
>> + *
>> + * Returns 0 if success, error code on error
>> + */
>> +static int cdns3_req_ep0_handle_feature(struct cdns3_device *priv_dev,
>> + struct usb_ctrlrequest *ctrl,
>> + int set)
>> +{
>> + int ret = 0;
>> + u32 recip;
>> +
>> + recip = ctrl->bRequestType & USB_RECIP_MASK;
>> +
>> + switch (recip) {
>> + case USB_RECIP_DEVICE:
>> + ret = cdns3_ep0_feature_handle_device(priv_dev, ctrl, set);
>> + break;
>> + case USB_RECIP_INTERFACE:
>> + ret = cdns3_ep0_feature_handle_intf(priv_dev, ctrl, set);
>> + break;
>> + case USB_RECIP_ENDPOINT:
>> + ret = cdns3_ep0_feature_handle_endpoint(priv_dev, ctrl, set);
>> + break;
>> + default:
>> + return -EINVAL;
>> + }
>> +
>> + if (!ret)
>> + writel(EP_CMD_ERDY | EP_CMD_REQ_CMPL, &priv_dev->regs->ep_cmd);
>> +
>> + return ret;
>> +}
>> +
>> +/**
>> + * cdns3_req_ep0_set_sel - Handling of SET_SEL standard USB request
>> + * @priv_dev: extended gadget object
>> + * @ctrl_req: pointer to received setup packet
>> + *
>> + * Returns 0 if success, error code on error
>> + */
>> +static int cdns3_req_ep0_set_sel(struct cdns3_device *priv_dev,
>> + struct usb_ctrlrequest *ctrl_req)
>> +{
>> + if (priv_dev->gadget.state < USB_STATE_ADDRESS)
>> + return -EINVAL;
>> +
>> + if (ctrl_req->wLength != 6) {
>> + dev_err(&priv_dev->dev, "Set SEL should be 6 bytes, got %d\n",
>> + ctrl_req->wLength);
>> + return -EINVAL;
>> + }
>> +
>> + priv_dev->ep0_data_dir = 0;
>> + cdns3_ep0_run_transfer(priv_dev, priv_dev->setup_dma, 6, 1);
>> + return 0;
>> +}
>> +
>> +/**
>> + * cdns3_req_ep0_set_isoch_delay -
>> + * Handling of GET_ISOCH_DELAY standard USB request
>> + * @priv_dev: extended gadget object
>> + * @ctrl_req: pointer to received setup packet
>> + *
>> + * Returns 0 if success, error code on error
>> + */
>> +static int cdns3_req_ep0_set_isoch_delay(struct cdns3_device *priv_dev,
>> + struct usb_ctrlrequest *ctrl_req)
>> +{
>> + if (ctrl_req->wIndex || ctrl_req->wLength)
>> + return -EINVAL;
>> +
>> + priv_dev->isoch_delay = ctrl_req->wValue;
>> + writel(EP_CMD_ERDY | EP_CMD_REQ_CMPL, &priv_dev->regs->ep_cmd);
>> + return 0;
>> +}
>> +
>> +/**
>> + * cdns3_ep0_standard_request - Handling standard USB requests
>> + * @priv_dev: extended gadget object
>> + * @ctrl_req: pointer to received setup packet
>> + *
>> + * Returns 0 if success, error code on error
>> + */
>> +static int cdns3_ep0_standard_request(struct cdns3_device *priv_dev,
>> + struct usb_ctrlrequest *ctrl_req)
>> +{
>> + int ret;
>> +
>> + switch (ctrl_req->bRequest) {
>> + case USB_REQ_SET_ADDRESS:
>> + ret = cdns3_req_ep0_set_address(priv_dev, ctrl_req);
>> + break;
>> + case USB_REQ_SET_CONFIGURATION:
>> + ret = cdns3_req_ep0_set_configuration(priv_dev, ctrl_req);
>> + break;
>> + case USB_REQ_GET_STATUS:
>> + ret = cdns3_req_ep0_get_status(priv_dev, ctrl_req);
>> + break;
>> + case USB_REQ_CLEAR_FEATURE:
>> + ret = cdns3_req_ep0_handle_feature(priv_dev, ctrl_req, 0);
>> + break;
>> + case USB_REQ_SET_FEATURE:
>> + ret = cdns3_req_ep0_handle_feature(priv_dev, ctrl_req, 1);
>> + break;
>> + case USB_REQ_SET_SEL:
>> + ret = cdns3_req_ep0_set_sel(priv_dev, ctrl_req);
>> + break;
>> + case USB_REQ_SET_ISOCH_DELAY:
>> + ret = cdns3_req_ep0_set_isoch_delay(priv_dev, ctrl_req);
>> + break;
>> + default:
>> + ret = cdns3_ep0_delegate_req(priv_dev, ctrl_req);
>> + break;
>> + }
>> +
>> + return ret;
>> +}
>> +
>> static void __pending_setup_status_handler(struct cdns3_device *priv_dev)
>> {
>> - //TODO: Implements this function
>> + struct usb_request *request = priv_dev->pending_status_request;
>> +
>> + if (priv_dev->status_completion_no_call && request &&
>> + request->complete) {
>> + request->complete(priv_dev->gadget.ep0, request);
>> + priv_dev->status_completion_no_call = 0;
>> + }
>> +}
>> +
>> +void cdns3_pending_setup_status_handler(struct work_struct *work)
>> +{
>> + struct cdns3_device *priv_dev = container_of(work, struct cdns3_device,
>> + pending_status_wq);
>> + unsigned long flags;
>> +
>> + spin_lock_irqsave(&priv_dev->lock, flags);
>> + __pending_setup_status_handler(priv_dev);
>> + spin_unlock_irqrestore(&priv_dev->lock, flags);
>> }
>>
>> /**
>> @@ -101,12 +546,50 @@ static void __pending_setup_status_handler(struct cdns3_device *priv_dev)
>> */
>> static void cdns3_ep0_setup_phase(struct cdns3_device *priv_dev)
>> {
>> - //TODO: Implements this function.
>> + struct usb_ctrlrequest *ctrl = priv_dev->setup;
>> + int result;
>> +
>> + if ((ctrl->bRequestType & USB_TYPE_MASK) == USB_TYPE_STANDARD)
>> + result = cdns3_ep0_standard_request(priv_dev, ctrl);
>> + else
>> + result = cdns3_ep0_delegate_req(priv_dev, ctrl);
>> +
>> + if (result != 0 && result != USB_GADGET_DELAYED_STATUS) {
>> + dev_dbg(&priv_dev->dev, "STALL for ep0\n");
>> + /* set_stall on ep0 */
>> + cdns3_select_ep(priv_dev, 0x00);
>> + writel(EP_CMD_SSTALL, &priv_dev->regs->ep_cmd);
>> + writel(EP_CMD_ERDY | EP_CMD_REQ_CMPL, &priv_dev->regs->ep_cmd);
>> + }
>> }
>>
>> static void cdns3_transfer_completed(struct cdns3_device *priv_dev)
>> {
>> - //TODO: Implements this function
>> + if (priv_dev->ep0_request) {
>> + usb_gadget_unmap_request_by_dev(priv_dev->sysdev,
>> + priv_dev->ep0_request,
>> + priv_dev->ep0_data_dir);
>> +
>> + priv_dev->ep0_request->actual =
>> + TRB_LEN(le32_to_cpu(priv_dev->trb_ep0->length));
>> +
>> + dev_dbg(&priv_dev->dev, "Ep0 completion length %d\n",
>> + priv_dev->ep0_request->actual);
>> + list_del_init(&priv_dev->ep0_request->list);
>> + }
>> +
>> + if (priv_dev->ep0_request &&
>> + priv_dev->ep0_request->complete) {
>> + spin_unlock(&priv_dev->lock);
>> + priv_dev->ep0_request->complete(priv_dev->gadget.ep0,
>> + priv_dev->ep0_request);
>> +
>> + priv_dev->ep0_request = NULL;
>> + spin_lock(&priv_dev->lock);
>> + }
>> +
>> + cdns3_prepare_setup_packet(priv_dev);
>> + writel(EP_CMD_REQ_CMPL, &priv_dev->regs->ep_cmd);
>> }
>>
>> /**
>> diff --git a/drivers/usb/cdns3/gadget.c b/drivers/usb/cdns3/gadget.c
>> index 309202474e57..0202ff5f6c90 100644
>> --- a/drivers/usb/cdns3/gadget.c
>> +++ b/drivers/usb/cdns3/gadget.c
>> @@ -70,6 +70,30 @@ static u8 cdns3_ep_reg_pos_to_index(int i)
>> return ((i / 16) + (((i % 16) - 2) * 2));
>> }
>>
>> +/**
>> + * cdns3_ep_addr_to_index - Macro converts endpoint address to
>> + * index of endpoint object in cdns3_device.eps[] container
>> + * @ep_addr: endpoint address for which endpoint object is required
>> + *
>> + * Remember that endpoint container doesn't contain default endpoint
>> + */
>> +u8 cdns3_ep_addr_to_index(u8 ep_addr)
>> +{
>> + return (((ep_addr & 0x7F) - 1) + ((ep_addr & USB_DIR_IN) ? 1 : 0));
>> +}
>> +
>> +/**
>> + * cdns3_ep_addr_to_bit_pos - Macro converts endpoint address to
>> + * bit position in ep_ists register
>> + * @ep_addr: endpoint address for which bit position is required
>> + *
>> + * Remember that endpoint container doesn't contain default endpoint
>> + */
>> +static u32 cdns3_ep_addr_to_bit_pos(u8 ep_addr)
>> +{
>> + return (1 << (ep_addr & 0x7F)) << ((ep_addr & 0x80) ? 16 : 0);
>> +}
>> +
>> /**
>> * cdns3_next_request - returns next request from list
>> * @list: list containing requests
>> @@ -464,6 +488,99 @@ static irqreturn_t cdns3_irq_handler_thread(struct cdns3 *cdns)
>> return ret;
>> }
>>
>> +/**
>> + * cdns3_ep_onchip_buffer_alloc - Try to allocate onchip buf for EP
>> + *
>> + * The real allocation will occur during write to EP_CFG register,
>> + * this function is used to check if the 'size' allocation is allowed.
>> + *
>> + * @priv_dev: extended gadget object
>> + * @size: the size (KB) for EP would like to allocate
>> + * @is_in: the direction for EP
>> + *
>> + * Return 0 if the later allocation is allowed or negative value on failure
>> + */
>> +static int cdns3_ep_onchip_buffer_alloc(struct cdns3_device *priv_dev,
>> + int size, int is_in)
>> +{
>> + if (is_in) {
>> + priv_dev->onchip_mem_allocated_size += size;
>> + } else if (!priv_dev->out_mem_is_allocated) {
>> + /* ALL OUT EPs are shared the same chunk onchip memory */
>> + priv_dev->onchip_mem_allocated_size += size;
>> + priv_dev->out_mem_is_allocated = 1;
>> + }
>> +
>> + if (priv_dev->onchip_mem_allocated_size > CDNS3_ONCHIP_BUF_SIZE) {
>> + priv_dev->onchip_mem_allocated_size -= size;
>> + return -EPERM;
>> + } else {
>> + return 0;
>> + }
>> +}
>> +
>> +/**
>> + * cdns3_ep_config Configure hardware endpoint
>> + * @priv_ep: extended endpoint object
>> + */
>> +void cdns3_ep_config(struct cdns3_endpoint *priv_ep)
>> +{
>> + bool is_iso_ep = (priv_ep->type == USB_ENDPOINT_XFER_ISOC);
>> + struct cdns3_device *priv_dev = priv_ep->cdns3_dev;
>> + u32 bEndpointAddress = priv_ep->num | priv_ep->dir;
>> + u32 interrupt_mask = EP_STS_EN_TRBERREN;
>> + u32 max_packet_size = 0;
>> + u32 ep_cfg = 0;
>> + int ret;
>> +
>> + if (priv_ep->type == USB_ENDPOINT_XFER_INT) {
>> + ep_cfg = EP_CFG_EPTYPE(USB_ENDPOINT_XFER_INT);
>> + } else if (priv_ep->type == USB_ENDPOINT_XFER_BULK) {
>> + ep_cfg = EP_CFG_EPTYPE(USB_ENDPOINT_XFER_BULK);
>> + } else {
>> + ep_cfg = EP_CFG_EPTYPE(USB_ENDPOINT_XFER_ISOC);
>> + interrupt_mask = 0xFFFFFFFF;
>> + }
>> +
>> + switch (priv_dev->gadget.speed) {
>> + case USB_SPEED_FULL:
>> + max_packet_size = is_iso_ep ? 1023 : 64;
>> + break;
>> + case USB_SPEED_HIGH:
>> + max_packet_size = is_iso_ep ? 1024 : 512;
>> + break;
>> + case USB_SPEED_SUPER:
>> + max_packet_size = 1024;
>> + break;
>> + default:
>> + //all other speed are not supported
>> + return;
>> + }
>> +
>> + ret = cdns3_ep_onchip_buffer_alloc(priv_dev, CDNS3_EP_BUF_SIZE,
>> + priv_ep->dir);
>
>where do you free the buffer_alloc?
I don't free this buffer.
This function was added by Peter Chan, and it refer to on-chip buffer.
Peter in his platform has limited number of on-chip memory. If I remember
correctly It has only 16KB. It's kind of software protection against exceeding this buffer.
It's hard coded now in driver. I think that this value can be read from register,
I have to consult this with hardware team.
I think the cdns3_ep_onchip_buffer_reserve is a better name.
It is not associated with allocation of memory.
>
>> + if (ret) {
>> + dev_err(&priv_dev->dev, "onchip mem is full, ep is invalid\n");
>> + return;
>> + }
>> +
>> + ep_cfg |= EP_CFG_MAXPKTSIZE(max_packet_size) |
>> + EP_CFG_BUFFERING(CDNS3_EP_BUF_SIZE - 1) |
>> + EP_CFG_MAXBURST(priv_ep->endpoint.maxburst);
>> +
>> + cdns3_select_ep(priv_dev, bEndpointAddress);
>> +
>> + writel(ep_cfg, &priv_dev->regs->ep_cfg);
>> + writel(interrupt_mask, &priv_dev->regs->ep_sts_en);
>> +
>> + dev_dbg(&priv_dev->dev, "Configure %s: with val %08x\n",
>> + priv_ep->name, ep_cfg);
>> +
>> + /* enable interrupt for selected endpoint */
>> + cdns3_set_register_bit(&priv_dev->regs->ep_ien,
>> + cdns3_ep_addr_to_bit_pos(bEndpointAddress));
>> +}
>> +
>> /* Find correct direction for HW endpoint according to description */
>> static int cdns3_ep_dir_is_correct(struct usb_endpoint_descriptor *desc,
>> struct cdns3_endpoint *priv_ep)
>> @@ -1104,6 +1221,8 @@ static int __cdns3_gadget_init(struct cdns3 *cdns)
>> priv_dev->is_connected = 0;
>>
>> spin_lock_init(&priv_dev->lock);
>> + INIT_WORK(&priv_dev->pending_status_wq,
>> + cdns3_pending_setup_status_handler);
>>
>> priv_dev->in_standby_mode = 1;
>>
>> diff --git a/drivers/usb/cdns3/gadget.h b/drivers/usb/cdns3/gadget.h
>> index 8c2f363f9340..db8c6cb9f2a5 100644
>> --- a/drivers/usb/cdns3/gadget.h
>> +++ b/drivers/usb/cdns3/gadget.h
>> @@ -1070,14 +1070,18 @@ struct cdns3_device {
>>
>> int cdns3_handshake(void __iomem *ptr, u32 mask, u32 done, int usec);
>> void cdns3_set_register_bit(void __iomem *ptr, u32 mask);
>> +void cdns3_pending_setup_status_handler(struct work_struct *work);
>> int cdns3_init_ep0(struct cdns3_device *priv_dev);
>> void cdns3_ep0_config(struct cdns3_device *priv_dev);
>> +void cdns3_ep_config(struct cdns3_endpoint *priv_ep);
>> void cdns3_check_ep0_interrupt_proceed(struct cdns3_device *priv_dev, int dir);
>> void cdns3_select_ep(struct cdns3_device *priv_dev, u32 ep);
>> void cdns3_enable_l1(struct cdns3_device *priv_dev, int enable);
>> struct usb_request *cdns3_next_request(struct list_head *list);
>> +void cdns3_gadget_unconfig(struct cdns3_device *priv_dev);
>> int cdns3_ep_run_transfer(struct cdns3_endpoint *priv_ep,
>> struct usb_request *request);
>> +u8 cdns3_ep_addr_to_index(u8 ep_addr);
>> int cdns3_gadget_ep_set_wedge(struct usb_ep *ep);
>> int cdns3_gadget_ep_set_halt(struct usb_ep *ep, int value);
>> struct usb_request *cdns3_gadget_ep_alloc_request(struct usb_ep *ep,
>>
Cheers
Pawel
Powered by blists - more mailing lists