On 18/11/18 12:09, Pawel Laszczak wrote:
> Patch implements callback functions for non-default endpoints
> defined in usb_ep_ops object.
> 
> Signed-off-by: Pawel Laszczak <paw...@cadence.com>
> ---
>  drivers/usb/cdns3/ep0.c    |  18 ++
>  drivers/usb/cdns3/gadget.c | 442 ++++++++++++++++++++++++++++++++++++-
>  drivers/usb/cdns3/gadget.h |   3 +
>  3 files changed, 461 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/cdns3/ep0.c b/drivers/usb/cdns3/ep0.c
> index c08d02665f9d..ca1795467155 100644
> --- a/drivers/usb/cdns3/ep0.c
> +++ b/drivers/usb/cdns3/ep0.c
> @@ -23,6 +23,24 @@ static void cdns3_prepare_setup_packet(struct cdns3_device 
> *priv_dev)
>       //TODO: Implements this function
>  }
>  
> +/**
> + * cdns3_gadget_ep_set_wedge Set wedge on selected endpoint
> + * @ep: endpoint object
> + *
> + * Returns 0
> + */
> +int cdns3_gadget_ep_set_wedge(struct usb_ep *ep)
> +{
> +     struct cdns3_endpoint *priv_ep = ep_to_cdns3_ep(ep);
> +     struct cdns3_device *priv_dev = priv_ep->cdns3_dev;
> +
> +     dev_dbg(&priv_dev->dev, "Wedge for %s\n", ep->name);
> +     cdns3_gadget_ep_set_halt(ep, 1);
> +     priv_ep->flags |= EP_WEDGE;
> +
> +     return 0;
> +}
> +
>  /**
>   * cdns3_ep0_config - Configures default endpoint
>   * @priv_dev: extended gadget object
> diff --git a/drivers/usb/cdns3/gadget.c b/drivers/usb/cdns3/gadget.c
> index 702a05faa664..1f2a434486dc 100644
> --- a/drivers/usb/cdns3/gadget.c
> +++ b/drivers/usb/cdns3/gadget.c
> @@ -58,6 +58,19 @@ void cdns3_set_register_bit(void __iomem *ptr, u32 mask)
>       writel(mask, ptr);
>  }
>  
> +/**
> + * cdns3_next_request - returns next request from list
> + * @list: list containing requests
> + *
> + * Returns request or NULL if no requests in list
> + */
> +struct usb_request *cdns3_next_request(struct list_head *list)
> +{
> +     if (list_empty(list))
> +             return NULL;
> +     return list_first_entry(list, struct usb_request, list);
> +}
> +
>  /**
>   * select_ep - selects endpoint
>   * @priv_dev:  extended gadget object
> @@ -73,6 +86,53 @@ void cdns3_select_ep(struct cdns3_device *priv_dev, u32 ep)
>       writel(ep, &priv_dev->regs->ep_sel);
>  }
>  
> +/**
> + * cdns3_allocate_trb_pool - Allocates TRB's pool for selected endpoint
> + * @priv_ep:  endpoint object
> + *
> + * Function will return 0 on success or -ENOMEM on allocation error
> + */
> +static int cdns3_allocate_trb_pool(struct cdns3_endpoint *priv_ep)
> +{
> +     struct cdns3_device *priv_dev = priv_ep->cdns3_dev;
> +     struct cdns3_trb *link_trb;
> +
> +     if (!priv_ep->trb_pool) {
> +             priv_ep->trb_pool = dma_zalloc_coherent(priv_dev->sysdev,
> +                                                     TRB_RIGN_SIZE,

TRB_RING_SIZE

> +                                                     &priv_ep->trb_pool_dma,
> +                                                     GFP_DMA);
> +             if (!priv_ep->trb_pool)
> +                     return -ENOMEM;
> +     } else {
> +             memset(priv_ep->trb_pool, 0, TRB_RIGN_SIZE);

here too.

> +     }
> +
> +     if (!priv_ep->aligned_buff) {
> +             priv_ep->aligned_buff = dma_alloc_coherent(priv_dev->sysdev,
> +                                                        
> CDNS3_UNALIGNED_BUF_SIZE,

CDNS3_ALIGNED_BUF_SIZE

> +                                                        
> &priv_ep->aligned_dma_addr,
> +                                                        GFP_DMA);
> +             if (!priv_ep->aligned_buff) {
> +                     dma_free_coherent(priv_dev->sysdev,
> +                                       TRB_RIGN_SIZE,
> +                                       priv_ep->trb_pool,
> +                                       priv_ep->trb_pool_dma);
> +                     priv_ep->trb_pool = NULL;
> +
> +                     return -ENOMEM;
> +             }
> +     }
> +
> +     /* Initialize the last TRB as Link TRB */
> +     link_trb = (priv_ep->trb_pool + TRBS_PER_SEGMENT - 1);
> +     link_trb->buffer = TRB_BUFFER(priv_ep->trb_pool_dma);
> +     link_trb->control = TRB_CYCLE | TRB_TYPE(TRB_LINK) |
> +                         TRB_CHAIN | TRB_TOGGLE;
> +
> +     return 0;
> +}
> +
>  static void cdns3_free_trb_pool(struct cdns3_endpoint *priv_ep)
>  {
>       struct cdns3_device *priv_dev = priv_ep->cdns3_dev;
> @@ -92,6 +152,73 @@ static void cdns3_free_trb_pool(struct cdns3_endpoint 
> *priv_ep)
>       }
>  }
>  
> +/**
> + * cdns3_data_flush - flush data at onchip buffer
> + * @priv_ep: endpoint object
> + *
> + * Endpoint must be selected before call to this function
> + *
> + * Returns zero on success or negative value on failure
> + */
> +static int cdns3_data_flush(struct cdns3_endpoint *priv_ep)
> +{
> +     struct cdns3_device *priv_dev = priv_ep->cdns3_dev;
> +
> +     writel(EP_CMD_DFLUSH, &priv_dev->regs->ep_cmd);
> +
> +     /* wait for DFLUSH cleared */
> +     return cdns3_handshake(&priv_dev->regs->ep_cmd, EP_CMD_DFLUSH, 0, 100);
> +}
> +
> +/**
> + * cdns3_ep_stall_flush - Stalls and flushes selected endpoint
> + * @priv_ep: endpoint object
> + *
> + * Endpoint must be selected before call to this function
> + */
> +static void cdns3_ep_stall_flush(struct cdns3_endpoint *priv_ep)
> +{
> +     struct cdns3_device *priv_dev = priv_ep->cdns3_dev;
> +
> +     writel(EP_CMD_DFLUSH | EP_CMD_ERDY | EP_CMD_SSTALL,
> +            &priv_dev->regs->ep_cmd);
> +
> +     /* wait for DFLUSH cleared */
> +     cdns3_handshake(&priv_dev->regs->ep_cmd, EP_CMD_DFLUSH, 0, 100);
> +     priv_ep->flags |= EP_STALL;
> +}
> +
> +/**
> + * cdns3_gadget_giveback - call struct usb_request's ->complete callback
> + * @priv_ep: The endpoint to whom the request belongs to
> + * @priv_req: The request we're giving back
> + * @status: completion code for the request
> + *
> + * Must be called with controller's lock held and interrupts disabled. This
> + * function will unmap @req and call its ->complete() callback to notify 
> upper
> + * layers that it has completed.
> + */
> +void cdns3_gadget_giveback(struct cdns3_endpoint *priv_ep,
> +                        struct cdns3_request *priv_req,
> +                        int status)
> +{
> +     //TODO: Implements this function.
> +}
> +
> +/**
> + * cdns3_ep_run_transfer - start transfer on no-default endpoint hardware
> + * @priv_ep: endpoint object
> + *
> + * Returns zero on success or negative value on failure
> + */
> +int cdns3_ep_run_transfer(struct cdns3_endpoint *priv_ep,
> +                       struct usb_request *request)
> +{
> +     //TODO: Implements this function.
> +
> +     return 0;
> +}
> +
>  /**
>   * cdns3_irq_handler - irq line interrupt handler
>   * @cdns: cdns3 instance
> @@ -170,6 +297,318 @@ static struct usb_ep *cdns3_gadget_match_ep(struct 
> usb_gadget *gadget,
>       return &priv_ep->endpoint;
>  }
>  
> +/**
> + * cdns3_gadget_ep_enable Enable endpoint
> + * @ep: endpoint object
> + * @desc: endpoint descriptor
> + *
> + * Returns 0 on success, error code elsewhere
> + */
> +static int cdns3_gadget_ep_enable(struct usb_ep *ep,
> +                               const struct usb_endpoint_descriptor *desc)
> +{
> +     struct cdns3_endpoint *priv_ep;
> +     struct cdns3_device *priv_dev;
> +     unsigned long flags;
> +     int ret;
> +     u32 reg;
> +
> +     priv_ep = ep_to_cdns3_ep(ep);
> +     priv_dev = priv_ep->cdns3_dev;
> +
> +     if (!ep || !desc || desc->bDescriptorType != USB_DT_ENDPOINT) {
> +             dev_err(&priv_dev->dev, "usbss: invalid parameters\n");

dev_dbg()?

Gadget driver will be more verbose.

> +             return -EINVAL;
> +     }
> +
> +     if (!desc->wMaxPacketSize) {
> +             dev_err(&priv_dev->dev, "usbss: missing wMaxPacketSize\n");
> +             return -EINVAL;
> +     }
> +
> +     if (dev_WARN_ONCE(&priv_dev->dev, priv_ep->flags & EP_ENABLED,
> +                       "%s is already enabled\n", priv_ep->name))
> +             return 0;
> +
> +     ret = cdns3_allocate_trb_pool(priv_ep);
> +     if (ret)
> +             return ret;

Why not allocate the TRB pool once for all endpoints at gadget init?

you don't seem to be calling cdns3_allocate_trb_pool() in 
cdns3_gadget_ep_disable().

> +
> +     dev_dbg(&priv_dev->dev, "Enabling endpoint: %s\n", ep->name);
> +     spin_lock_irqsave(&priv_dev->lock, flags);
> +     cdns3_select_ep(priv_dev, desc->bEndpointAddress);
> +     writel(EP_CMD_EPRST, &priv_dev->regs->ep_cmd);
> +
> +     ret = cdns3_handshake(&priv_dev->regs->ep_cmd,
> +                           EP_CMD_CSTALL | EP_CMD_EPRST, 0, 100);
> +
> +     cdns3_set_register_bit(&priv_dev->regs->ep_cfg, EP_CFG_ENABLE);
> +
> +     ep->desc = desc;
> +     priv_ep->flags &= ~(EP_PENDING_REQUEST | EP_STALL);
> +     priv_ep->flags |= EP_ENABLED | EP_UPDATE_EP_TRBADDR;
> +     priv_ep->enqueue = 0;
> +     priv_ep->dequeue = 0;
> +     reg = readl(&priv_dev->regs->ep_sts);
> +     priv_ep->pcs = !!EP_STS_CCS(reg);
> +     priv_ep->ccs = !!EP_STS_CCS(reg);
> +     /* one TRB is reserved for link TRB used in DMULT mode*/
> +     priv_ep->free_trbs = TRBS_PER_SEGMENT - 1;
> +
> +     spin_unlock_irqrestore(&priv_dev->lock, flags);
> +     return 0;
> +}
> +
> +/**
> + * cdns3_gadget_ep_disable Disable endpoint
> + * @ep: endpoint object
> + *
> + * Returns 0 on success, error code elsewhere
> + */
> +static int cdns3_gadget_ep_disable(struct usb_ep *ep)
> +{
> +     struct cdns3_endpoint *priv_ep;
> +     struct cdns3_device *priv_dev;
> +     unsigned long flags;
> +     int ret = 0;
> +     struct usb_request *request;
> +     u32 ep_cfg;
> +
> +     if (!ep) {
> +             pr_debug("usbss: invalid parameters\n");
> +             return -EINVAL;
> +     }
> +
> +     priv_ep = ep_to_cdns3_ep(ep);
> +     priv_dev = priv_ep->cdns3_dev;
> +
> +     if (dev_WARN_ONCE(&priv_dev->dev, !(priv_ep->flags & EP_ENABLED),
> +                       "%s is already disabled\n", priv_ep->name))
> +             return 0;
> +
> +     spin_lock_irqsave(&priv_dev->lock, flags);
> +     if (!priv_dev->start_gadget) {
> +             dev_dbg(&priv_dev->dev,
> +                     "Disabling endpoint at disconnection: %s\n", ep->name);

This flag is looking very tricky.
What do you mean by "disabling at disconnection"?

> +             spin_unlock_irqrestore(&priv_dev->lock, flags);
> +             return 0;

EP is not yet disabled and we're returning 0. This will cause an unbalance.
I'd avoid that flag altogether.

> +     }
> +
> +     dev_dbg(&priv_dev->dev, "Disabling endpoint: %s\n", ep->name);
> +
> +     cdns3_select_ep(priv_dev, ep->desc->bEndpointAddress);
> +     ret = cdns3_data_flush(priv_ep);
> +     while (!list_empty(&priv_ep->request_list)) {
> +             request = cdns3_next_request(&priv_ep->request_list);
> +
> +             cdns3_gadget_giveback(priv_ep, to_cdns3_request(request),
> +                                   -ESHUTDOWN);
> +     }
> +
> +     ep_cfg = readl(&priv_dev->regs->ep_cfg);
> +     ep_cfg &= ~EP_CFG_ENABLE;
> +     writel(ep_cfg, &priv_dev->regs->ep_cfg);
> +     ep->desc = NULL;
> +     priv_ep->flags &= ~EP_ENABLED;
> +
> +     spin_unlock_irqrestore(&priv_dev->lock, flags);
> +
> +     return ret;
> +}
> +
> +/**
> + * cdns3_gadget_ep_alloc_request Allocates request
> + * @ep: endpoint object associated with request
> + * @gfp_flags: gfp flags
> + *
> + * Returns allocated request address, NULL on allocation error
> + */
> +struct usb_request *cdns3_gadget_ep_alloc_request(struct usb_ep *ep,
> +                                               gfp_t gfp_flags)
> +{
> +     struct cdns3_endpoint *priv_ep = ep_to_cdns3_ep(ep);
> +     struct cdns3_request *priv_req;
> +
> +     priv_req = kzalloc(sizeof(*priv_req), gfp_flags);
> +     if (!priv_req)
> +             return NULL;
> +
> +     priv_req->priv_ep = priv_ep;
> +
> +     return &priv_req->request;
> +}
> +
> +/**
> + * cdns3_gadget_ep_free_request Free memory occupied by request
> + * @ep: endpoint object associated with request
> + * @request: request to free memory
> + */
> +void cdns3_gadget_ep_free_request(struct usb_ep *ep,
> +                               struct usb_request *request)
> +{
> +     struct cdns3_request *priv_req = to_cdns3_request(request);
> +
> +     kfree(priv_req);
> +}
> +
> +/**
> + * cdns3_gadget_ep_queue Transfer data on endpoint
> + * @ep: endpoint object
> + * @request: request object
> + * @gfp_flags: gfp flags
> + *
> + * Returns 0 on success, error code elsewhere
> + */
> +static int __cdns3_gadget_ep_queue(struct usb_ep *ep,
> +                                struct usb_request *request,
> +                                gfp_t gfp_flags)
> +{
> +     struct cdns3_endpoint *priv_ep = ep_to_cdns3_ep(ep);
> +     struct cdns3_device *priv_dev = priv_ep->cdns3_dev;
> +     int ret = 0;
> +
> +     request->actual = 0;
> +     request->status = -EINPROGRESS;
> +
> +     dev_dbg(&priv_dev->dev, "Queuing to endpoint: %s\n", priv_ep->name);
> +
> +     ret = usb_gadget_map_request_by_dev(priv_dev->sysdev, request,
> +                                         usb_endpoint_dir_in(ep->desc));
> +
> +     if (ret)
> +             return ret;
> +
> +     if (!cdns3_ep_run_transfer(priv_ep, request))
> +             list_add_tail(&request->list, &priv_ep->request_list);

how about catching the return value if cdns3_ep_run_transfer() fails?

> +
> +     return ret;
> +}
> +
> +static int cdns3_gadget_ep_queue(struct usb_ep *ep, struct usb_request 
> *request,
> +                              gfp_t gfp_flags)
> +{
> +     struct cdns3_endpoint *priv_ep = ep_to_cdns3_ep(ep);
> +     struct cdns3_device *priv_dev = priv_ep->cdns3_dev;
> +     struct usb_request *zlp_request;
> +     unsigned long flags;
> +     int ret;
> +
> +     spin_lock_irqsave(&priv_dev->lock, flags);
> +     ret = __cdns3_gadget_ep_queue(ep, request, gfp_flags);
> +
> +     if (ret == 0 && request->zero && request->length &&
> +         (request->length % ep->maxpacket == 0)) {
> +             zlp_request = cdns3_gadget_ep_alloc_request(ep, GFP_ATOMIC);
> +             zlp_request->buf = priv_dev->zlp_buf;
> +             zlp_request->length = 0;
> +
> +             dev_dbg(&priv_dev->dev, "Queuing ZLP for endpoint: %s\n",
> +                     priv_ep->name);
> +             ret = __cdns3_gadget_ep_queue(ep, zlp_request, gfp_flags);

Who is going to free this zlp_request?

> +     }
> +
> +     spin_unlock_irqrestore(&priv_dev->lock, flags);
> +     return ret;
> +}
> +
> +/**
> + * cdns3_gadget_ep_dequeue Remove request from transfer queue
> + * @ep: endpoint object associated with request
> + * @request: request object
> + *
> + * Returns 0 on success, error code elsewhere
> + */
> +int cdns3_gadget_ep_dequeue(struct usb_ep *ep,
> +                         struct usb_request *request)
> +{
> +     struct cdns3_endpoint *priv_ep = ep_to_cdns3_ep(ep);
> +     struct cdns3_device *priv_dev = priv_ep->cdns3_dev;
> +     struct usb_request *req, *req_temp;
> +     unsigned long flags;
> +     int ret = 0;
> +
> +     if (!ep || !request || !ep->desc)
> +             return -EINVAL;
> +
> +     spin_lock_irqsave(&priv_dev->lock, flags);
> +     dev_dbg(&priv_dev->dev, "Dequeue from %s\n", ep->name);
> +
> +     cdns3_select_ep(priv_dev, ep->desc->bEndpointAddress);
> +     if (priv_dev->start_gadget)
> +             ret = cdns3_data_flush(priv_ep);
> +
> +     list_for_each_entry_safe(req, req_temp, &priv_ep->request_list, list) {
> +             if (request == req) {
> +                     cdns3_gadget_giveback(priv_ep,
> +                                           to_cdns3_request(request),
> +                                           -ECONNRESET);
> +                     break;
> +             }
> +     }
> +
> +     spin_unlock_irqrestore(&priv_dev->lock, flags);
> +     return ret;
> +}
> +
> +/**
> + * cdns3_gadget_ep_set_halt Sets/clears stall on selected endpoint
> + * @ep: endpoint object to set/clear stall on
> + * @value: 1 for set stall, 0 for clear stall
> + *
> + * Returns 0 on success, error code elsewhere
> + */
> +int cdns3_gadget_ep_set_halt(struct usb_ep *ep, int value)
> +{
> +     struct cdns3_endpoint *priv_ep = ep_to_cdns3_ep(ep);
> +     struct cdns3_device *priv_dev = priv_ep->cdns3_dev;
> +     unsigned long flags;
> +     int ret = 0;
> +
> +     if (!(priv_ep->flags & EP_ENABLED))
> +             return -EPERM;
> +
> +     /* if actual transfer is pending defer setting stall on this endpoint */
> +     if ((priv_ep->flags & EP_PENDING_REQUEST) && value) {
> +             priv_ep->flags |= EP_STALL;
> +             return 0;
> +     }
> +
> +     dev_dbg(&priv_dev->dev, "Halt endpoint %s\n", priv_ep->name);
> +
> +     spin_lock_irqsave(&priv_dev->lock, flags);

How about grabbing the lock before checking for priv->ep->flags?

> +
> +     cdns3_select_ep(priv_dev, ep->desc->bEndpointAddress);
> +     if (value) {
> +             cdns3_ep_stall_flush(priv_ep);
> +     } else {
> +             priv_ep->flags &= ~EP_WEDGE;
> +             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 there was an error we shouldn't be clearing the EP_STALL right
and leave the pending flag?

> +             priv_ep->flags &= ~EP_STALL;
> +     }
> +
> +     priv_ep->flags &= ~EP_PENDING_REQUEST;
> +     spin_unlock_irqrestore(&priv_dev->lock, flags);
> +
> +     return ret;
> +}
> +
> +extern const struct usb_ep_ops cdns3_gadget_ep0_ops;
> +
> +static const struct usb_ep_ops cdns3_gadget_ep_ops = {
> +     .enable = cdns3_gadget_ep_enable,
> +     .disable = cdns3_gadget_ep_disable,
> +     .alloc_request = cdns3_gadget_ep_alloc_request,
> +     .free_request = cdns3_gadget_ep_free_request,
> +     .queue = cdns3_gadget_ep_queue,
> +     .dequeue = cdns3_gadget_ep_dequeue,
> +     .set_halt = cdns3_gadget_ep_set_halt,
> +     .set_wedge = cdns3_gadget_ep_set_wedge,
> +};
> +
>  /**
>   * cdns3_gadget_get_frame Returns number of actual ITP frame
>   * @gadget: gadget object
> @@ -365,8 +804,7 @@ static int cdns3_init_ep(struct cdns3_device *priv_dev)
>               usb_ep_set_maxpacket_limit(&priv_ep->endpoint,
>                                          ENDPOINT_MAX_PACKET_LIMIT);
>               priv_ep->endpoint.max_streams = ENDPOINT_MAX_STREAMS;
> -             //TODO: Add implementation of cdns3_gadget_ep_ops
> -             //priv_ep->endpoint.ops = &cdns3_gadget_ep_ops;
> +             priv_ep->endpoint.ops = &cdns3_gadget_ep_ops;
>               if (ep_dir)
>                       priv_ep->endpoint.caps.dir_in = 1;
>               else
> diff --git a/drivers/usb/cdns3/gadget.h b/drivers/usb/cdns3/gadget.h
> index 3b0d4d2e4831..a4be288b34cb 100644
> --- a/drivers/usb/cdns3/gadget.h
> +++ b/drivers/usb/cdns3/gadget.h
> @@ -1072,4 +1072,7 @@ void cdns3_set_register_bit(void __iomem *ptr, u32 
> mask);
>  int cdns3_init_ep0(struct cdns3_device *priv_dev);
>  void cdns3_ep0_config(struct cdns3_device *priv_dev);
>  void cdns3_select_ep(struct cdns3_device *priv_dev, u32 ep);
> +int cdns3_gadget_ep_set_wedge(struct usb_ep *ep);
> +int cdns3_gadget_ep_set_halt(struct usb_ep *ep, int value);
> +
>  #endif /* __LINUX_CDNS3_GADGET */
> 

cheers,
-roger
-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

Reply via email to