All of lore.kernel.org
 help / color / mirror / Atom feed
From: Roger Quadros <rogerq@ti.com>
To: Pawel Laszczak <pawell@cadence.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	"linux-usb@vger.kernel.org" <linux-usb@vger.kernel.org>,
	Felipe Balbi <balbi@kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Lukasz Tyrala <ltyrala@cadence.com>,
	Alan Douglas <adouglas@cadence.com>
Subject: Re: [PATCH 06/31] usb: usbssp: added template functions used by upper layer.
Date: Mon, 6 Aug 2018 11:57:01 +0300	[thread overview]
Message-ID: <a44cf29a-3097-eb11-1dd4-de810eda0406@ti.com> (raw)
In-Reply-To: <BYAPR07MB47093370E0143D8E0E42E3EEDD220@BYAPR07MB4709.namprd07.prod.outlook.com>



On 04/08/18 09:37, Pawel Laszczak wrote:
>>> Patch adds some functionality for initialization process.
>>> It adds to driver usbssp_reset and usbssp_start function.
>>>
>>> Next elements added are objects usbssp_gadget_ep0_ops,
>>> usbssp_gadget_ep_ops and usbssp_gadget_ops. These objects
>>> constitute the interface used by gadget subsystem.
>>> At this moment functions related to these objects are empty
>>> and do nothing.
>>>
>>> This patch also implements usbssp_gadget_init_endpoint and
>>> usbssp_gadget_free_endpoint used during initialization.
>>>
>>> Signed-off-by: Pawel Laszczak <pawell@cadence.com>
>>> ---
>>>  drivers/usb/usbssp/gadget-ext-caps.h |   3 +
>>>  drivers/usb/usbssp/gadget-if.c       | 269 ++++++++++++++++++++++++++-
>>>  drivers/usb/usbssp/gadget.c          |  84 ++++++++-
>>>  3 files changed, 350 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/drivers/usb/usbssp/gadget-ext-caps.h b/drivers/usb/usbssp/gadget-ext-caps.h
>>> index 2bf327046376..86c0ce331037 100644
>>> --- a/drivers/usb/usbssp/gadget-ext-caps.h
>>> +++ b/drivers/usb/usbssp/gadget-ext-caps.h
>>> @@ -51,3 +51,6 @@
>>>  #define USBSSP_CMD_EWE			BIT(10)
>>>
>>>  #define USBSSP_IRQS	(USBSSP_CMD_EIE | USBSSP_CMD_HSEIE | USBSSP_CMD_EWE)
>>> +
>>> +/* true: Controller Not Ready to accept doorbell or op reg writes after reset */
>>> +#define USBSSP_STS_CNR			BIT(11)
>>> diff --git a/drivers/usb/usbssp/gadget-if.c b/drivers/usb/usbssp/gadget-if.c
>>> index d53e0fb65299..70def978b085 100644
>>> --- a/drivers/usb/usbssp/gadget-if.c
>>> +++ b/drivers/usb/usbssp/gadget-if.c
>>> @@ -12,13 +12,278 @@
>>>  #include <linux/usb/composite.h>
>>>  #include "gadget.h"
>>>
>>> +static int usbssp_gadget_ep_enable(struct usb_ep *ep,
>>> +				   const struct usb_endpoint_descriptor *desc)
>>> +{
>>> +	struct usbssp_ep *ep_priv = to_usbssp_ep(ep);
>>> +	int ret = 0;
>>> +
>>> +	if (!ep_priv)
>>> +		return -EINVAL;
>>> +
>>> +	/*TODO: implements this function*/
>>> +	return ret;
>>> +}
>>> +
>>> +int usbssp_gadget_ep_disable(struct usb_ep *ep)
>>> +{
>>> +	struct usbssp_ep *ep_priv = to_usbssp_ep(ep);
>>> +	int ret = 0;
>>> +
>>> +	if (!ep_priv)
>>> +		return -EINVAL;
>>> +
>>> +	/*TODO: implements this function*/
>>> +	return ret;
>>> +}
>>> +
>>> +static struct usb_request *usbssp_gadget_ep_alloc_request(struct usb_ep *ep,
>>> +							  gfp_t gfp_flags)
>>> +{
>>> +	struct usbssp_ep *ep_priv = to_usbssp_ep(ep);
>>> +
>>> +	if (!ep_priv)
>>> +		return NULL;
>>> +
>>> +	/*TODO: implements this function*/
>>> +	return NULL;
>>> +}
>>> +
>>> +static void usbssp_gadget_ep_free_request(struct usb_ep *ep,
>>> +					  struct usb_request *request)
>>> +{
>>> +	struct usbssp_ep *ep_priv = to_usbssp_ep(ep);
>>> +
>>> +	if (!ep_priv)
>>> +		return;
>>> +
>>> +	/*TODO: implements this function*/
>>> +}
>>> +
>>> +static int usbssp_gadget_ep_queue(struct usb_ep *ep,
>>> +				  struct usb_request *request,
>>> +				  gfp_t gfp_flags)
>>> +{
>>> +	struct usbssp_ep *ep_priv = to_usbssp_ep(ep);
>>> +	int ret = 0;
>>> +
>>> +	if (!ep_priv)
>>> +		return -EINVAL;
>>> +
>>> +	/*TODO: implements this function*/
>>> +	return ret;
>>> +}
>>> +
>>> +static int usbssp_gadget_ep_dequeue(struct usb_ep *ep,
>>> +				    struct usb_request *request)
>>> +{
>>> +	struct usbssp_ep *ep_priv = to_usbssp_ep(ep);
>>> +	int ret = 0;
>>> +
>>> +	if (!ep_priv)
>>> +		return -EINVAL;
>>> +
>>> +	/*TODO: implements this function*/
>>> +	return ret;
>>> +}
>>> +
>>> +static int usbssp_gadget_ep_set_halt(struct usb_ep *ep, int value)
>>> +{
>>> +	struct usbssp_ep *ep_priv = to_usbssp_ep(ep);
>>> +	int ret = 0;
>>> +
>>> +	if (!ep_priv)
>>> +		return -EINVAL;
>>> +
>>> +	/*TODO: implements this function*/
>>> +	return ret;
>>> +}
>>> +
>>> +static int usbssp_gadget_ep_set_wedge(struct usb_ep *ep)
>>> +{
>>> +	struct usbssp_ep *ep_priv = to_usbssp_ep(ep);
>>> +	int ret = 0;
>>> +
>>> +	if (!ep_priv)
>>> +		return -EINVAL;
>>> +
>>> +	/*TODO: implements this function*/
>>> +	return ret;
>>> +}
>>> +
>>> +static const struct usb_ep_ops usbssp_gadget_ep0_ops = {
>>> +	.enable		= usbssp_gadget_ep_enable,
>>> +	.disable	= usbssp_gadget_ep_disable,
>>> +	.alloc_request	= usbssp_gadget_ep_alloc_request,
>>> +	.free_request	= usbssp_gadget_ep_free_request,
>>> +	.queue		= usbssp_gadget_ep_queue,
>>> +	.dequeue	= usbssp_gadget_ep_dequeue,
>>> +	.set_halt	= usbssp_gadget_ep_set_halt,
>>> +	.set_wedge	= usbssp_gadget_ep_set_wedge,
>>> +};
>>> +
>>> +static const struct usb_ep_ops usbssp_gadget_ep_ops = {
>>> +	.enable		= usbssp_gadget_ep_enable,
>>> +	.disable	= usbssp_gadget_ep_disable,
>>> +	.alloc_request	= usbssp_gadget_ep_alloc_request,
>>> +	.free_request	= usbssp_gadget_ep_free_request,
>>> +	.queue		= usbssp_gadget_ep_queue,
>>> +	.dequeue	= usbssp_gadget_ep_dequeue,
>>> +	.set_halt	= usbssp_gadget_ep_set_halt,
>>> +	.set_wedge	= usbssp_gadget_ep_set_wedge,
>>> +};
>>> +
>>> +static struct usb_endpoint_descriptor usbssp_gadget_ep0_desc = {
>>> +	.bLength =		USB_DT_ENDPOINT_SIZE,
>>> +	.bDescriptorType =	USB_DT_ENDPOINT,
>>> +	.bmAttributes =		USB_ENDPOINT_XFER_CONTROL,
>>> +};
>>> +
>>> +static int usbssp_gadget_start(struct usb_gadget *g,
>>> +			       struct usb_gadget_driver *driver)
>>> +{
>>> +	struct usbssp_udc *usbssp_data = gadget_to_usbssp(g);
>>> +	int ret = 0;
>>> +
>>> +	if (usbssp_data->gadget_driver) {
>>> +		dev_err(usbssp_data->dev, "%s is already bound to %s\n",
>>> +				usbssp_data->gadget.name,
>>> +				usbssp_data->gadget_driver->driver.name);
>>> +		ret = -EBUSY;
>>> +	}
>>> +
>>> +	/*TODO: add implementation*/
>>> +	return ret;
>>> +}
>>> +
>>> +static int usbssp_gadget_stop(struct usb_gadget *g)
>>> +{
>>> +	struct usbssp_udc *usbssp_data = gadget_to_usbssp(g);
>>> +
>>> +	if (!usbssp_data)
>>> +		return -EINVAL;
>>> +	/*TODO: add implementation*/
>>> +	return 0;
>>> +}
>>> +
>>> +static int usbssp_gadget_get_frame(struct usb_gadget *g)
>>> +{
>>> +	struct usbssp_udc *usbssp_data = gadget_to_usbssp(g);
>>> +
>>> +	if (!usbssp_data)
>>> +		return -EINVAL;
>>> +
>>> +	/*TODO: add implementation*/
>>> +	return 0;
>>> +}
>>> +
>>> +static int usbssp_gadget_wakeup(struct usb_gadget *g)
>>> +{
>>> +	struct usbssp_udc *usbssp_data = gadget_to_usbssp(g);
>>> +
>>> +	if (!usbssp_data)
>>> +		return -EINVAL;
>>> +
>>> +	/*TODO: add implementation*/
>>> +	return 0;
>>> +}
>>> +
>>> +static int usbssp_gadget_set_selfpowered(struct usb_gadget *g,
>>> +					 int is_selfpowered)
>>> +{
>>> +	struct usbssp_udc *usbssp_data = gadget_to_usbssp(g);
>>> +
>>> +	if (!usbssp_data)
>>> +		return -EINVAL;
>>> +
>>> +	/*TODO: add implementation*/
>>> +	return 0;
>>> +}
>>> +
>>> +static const struct usb_gadget_ops usbssp_gadget_ops = {
>>> +	.get_frame		= usbssp_gadget_get_frame,
>>> +	.wakeup			= usbssp_gadget_wakeup,
>>> +	.set_selfpowered	= usbssp_gadget_set_selfpowered,
>>> +	.udc_start		= usbssp_gadget_start,
>>> +	.udc_stop		= usbssp_gadget_stop,
>>> +};
>>> +
>>>  int usbssp_gadget_init_endpoint(struct usbssp_udc *usbssp_data)
>>
>> Since we're initializing all endpoints this function should be named usbssp_gadget_init_endpoints().
>>
>>>  {
>>> -	/*TODO: it has to be implemented*/
>>> +	int i = 0;
>>> +	struct usbssp_ep *ep_priv;
>>> +
>>> +	usbssp_data->num_endpoints = USBSSP_ENDPOINTS_NUM;
>>> +	INIT_LIST_HEAD(&usbssp_data->gadget.ep_list);
>>> +
>>> +	for (i = 1; i < usbssp_data->num_endpoints; i++) {
>>> +		bool direction = i & 1; /*start from OUT endpoint*/
>>> +		u8 epnum = (i >> 1);
>>> +
>>> +		ep_priv = &usbssp_data->devs.eps[i-1];
>>> +		ep_priv->usbssp_data = usbssp_data;
>>> +		ep_priv->number = epnum;
>>> +		ep_priv->direction = direction; /*0 for OUT, 1 for IN*/
>>> +
>>> +		snprintf(ep_priv->name, sizeof(ep_priv->name), "ep%d%s", epnum,
>>> +				(ep_priv->direction) ? "in" : "out");
>>> +
>>> +		ep_priv->endpoint.name = ep_priv->name;
>>> +
>>> +		if (ep_priv->number < 2) {
>>> +			ep_priv->endpoint.desc = &usbssp_gadget_ep0_desc;
>>> +			ep_priv->endpoint.comp_desc = NULL;
>>> +		}
>>> +
>>> +		if (epnum == 0) {
>>> +			/*EP0 is bidirectional endpoint*/
>>> +			usb_ep_set_maxpacket_limit(&ep_priv->endpoint, 512);
>>> +			dev_dbg(usbssp_data->dev,
>>> +				"Initializing %s, MaxPack: %04x Type: Ctrl\n",
>>> +				ep_priv->name, 512);
>>> +			ep_priv->endpoint.maxburst = 1;
>>> +			ep_priv->endpoint.ops = &usbssp_gadget_ep0_ops;
>>> +			ep_priv->endpoint.caps.type_control = true;
>>> +
>>> +			usbssp_data->usb_req_ep0_in.epnum = ep_priv->number;
>>> +			usbssp_data->usb_req_ep0_in.dep = ep_priv;
>>> +
>>> +			if (!epnum)
>>> +				usbssp_data->gadget.ep0 = &ep_priv->endpoint;
>>> +		} else {
>>> +			usb_ep_set_maxpacket_limit(&ep_priv->endpoint, 1024);
>>> +			ep_priv->endpoint.maxburst = 15;
>>> +			ep_priv->endpoint.ops = &usbssp_gadget_ep_ops;
>>> +			list_add_tail(&ep_priv->endpoint.ep_list,
>>> +					&usbssp_data->gadget.ep_list);
>>> +			ep_priv->endpoint.caps.type_iso = true;
>>> +			ep_priv->endpoint.caps.type_bulk = true;
>>> +			ep_priv->endpoint.caps.type_int = true;
>>> +
>>> +		}
>>> +
>>> +		ep_priv->endpoint.caps.dir_in = direction;
>>> +		ep_priv->endpoint.caps.dir_out = !direction;
>>
>> Since you start from 1, ep0 will only be initialized as dir_in.
>> Is it better to represent EP0in and EP0out separately?
>> If so you can start i from 0 and use
>> 	ep_priv = &usbssp_data->devs.eps[i];
>>
>> This should fix the direction. eps[0] will be ep0out and eps[1] will be ep0in.
> 
> I wanted to be consistent with Device Context Data Structure from 
> XHCI specification (EP Context 0 BiDir, EP Context 1 OUT, EP Context 1 IN ...).
> 
> I little change this function, and now it look like this:
> int usbssp_gadget_init_endpoints(struct usbssp_udc *usbssp_data)
> {
> 	int i = 0;
> 	struct usbssp_ep *ep_priv;
> 
> 	usbssp_data->num_endpoints = USBSSP_ENDPOINTS_NUM;
> 	INIT_LIST_HEAD(&usbssp_data->gadget.ep_list);
> 
> 	for (i = 0; i < usbssp_data->num_endpoints; i++) {
> 		bool direction = i & 1; /*start from OUT endpoint*/
> 		u8 epnum = ((i + 1) >> 1);
> 
> 		ep_priv = &usbssp_data->devs.eps[i];
> 		ep_priv->usbssp_data = usbssp_data;
> 		ep_priv->number = epnum;
> 		ep_priv->direction = direction; /*0 for OUT, 1 for IN*/
> 
> 		/*
> 		 *Ep0 is bidirectional so Ep0In and Ep0Out is represented by
> 		 * usbssp_data->devs.eps[0]
> 		 */
> 		if (epnum == 0) {
> 			snprintf(ep_priv->name, sizeof(ep_priv->name), "ep%d%s",
> 				epnum, "BiDir");
> 
> 			usb_ep_set_maxpacket_limit(&ep_priv->endpoint, 512);
> 			ep_priv->endpoint.maxburst = 1;
> 			ep_priv->endpoint.ops = &usbssp_gadget_ep0_ops;
> 			ep_priv->endpoint.desc = &usbssp_gadget_ep0_desc;
> 
> 			ep_priv->endpoint.comp_desc = NULL;
> 			ep_priv->endpoint.caps.type_control = true;
> 			ep_priv->endpoint.caps.dir_in = true;
> 			ep_priv->endpoint.caps.dir_out = true;
> 
> 			usbssp_data->usb_req_ep0_in.epnum = ep_priv->number;
> 			usbssp_data->usb_req_ep0_in.dep = ep_priv;
> 			usbssp_data->gadget.ep0 = &ep_priv->endpoint;
> 
> 		} else {
> 			snprintf(ep_priv->name, sizeof(ep_priv->name), "ep%d%s",
> 				epnum, (ep_priv->direction) ? "in"  : "out");
> 
> 			usb_ep_set_maxpacket_limit(&ep_priv->endpoint, 1024);
> 			ep_priv->endpoint.maxburst = 15;
> 			ep_priv->endpoint.ops = &usbssp_gadget_ep_ops;
> 			list_add_tail(&ep_priv->endpoint.ep_list,
> 					&usbssp_data->gadget.ep_list);
> 
> 			ep_priv->endpoint.caps.type_iso = true;
> 			ep_priv->endpoint.caps.type_bulk = true;
> 			ep_priv->endpoint.caps.type_int = true;
> 
> 			ep_priv->endpoint.caps.dir_in = direction;
> 			ep_priv->endpoint.caps.dir_out = !direction;
> 		}
> 
> 		ep_priv->endpoint.name = ep_priv->name;

I think this is much better now.

> 
> 		dev_dbg(usbssp_data->dev, "Init %s, MaxPack: %04x SupType: "
> 			"CTRL: %s, INT: %s, BULK: %s, ISOC %s, "
> 			"SupDir IN: %s, OUT: %s\n",
> 			ep_priv->name, 1024,
> 			(ep_priv->endpoint.caps.type_control) ? "yes" : "no",
> 			(ep_priv->endpoint.caps.type_int) ? "yes" : "no",
> 			(ep_priv->endpoint.caps.type_bulk) ? "yes" : "no",
> 			(ep_priv->endpoint.caps.type_iso) ? "yes" : "no",
> 			(ep_priv->endpoint.caps.dir_in) ? "yes" : "no",
> 			(ep_priv->endpoint.caps.dir_out) ? "yes" : "no");
> 

In some places you use usbssp_dbg_trace() and at others you use dev_dbg().
Is it better to stick with one throughout?

> 		INIT_LIST_HEAD(&ep_priv->pending_list);
> 	}
> 
> 	return 0;
> }
> 
>>> +
>>> +		dev_dbg(usbssp_data->dev, "Init %s, MaxPack: %04x SupType:"
>>> +				" INT/BULK/ISOC , SupDir %s\n",
>>> +				ep_priv->name, 1024,
>>> +				(ep_priv->endpoint.caps.dir_in) ? "IN" : "OUT");
>>> +
>>> +		INIT_LIST_HEAD(&ep_priv->pending_list);
>>> +	}
>>>  	return 0;
>>>  }
>>>
>>>  void usbssp_gadget_free_endpoint(struct usbssp_udc *usbssp_data)
>>>  {
>>> -	/*TODO: it has to be implemented*/
>>> +	int i;
>>> +	struct usbssp_ep *ep_priv;
>>> +
>>> +	for (i = 0; i < usbssp_data->num_endpoints; i++) {
>>> +		ep_priv = &usbssp_data->devs.eps[i];
>>> +
>>
>> if you start i from 1 then you can skip the if().
>>
>>> +		if (ep_priv->number != 0)
>>> +			list_del(&ep_priv->endpoint.ep_list);
>>> +	}
>>>  }
>>> diff --git a/drivers/usb/usbssp/gadget.c b/drivers/usb/usbssp/gadget.c
>>> index 338ec2ec18b1..195f5777cf8a 100644
>>> --- a/drivers/usb/usbssp/gadget.c
>>> +++ b/drivers/usb/usbssp/gadget.c
>>> @@ -103,6 +103,83 @@ int usbssp_halt(struct usbssp_udc *usbssp_data)
>>>  	return ret;
>>>  }
>>>
>>> +/*
>>> + * Set the run bit and wait for the device to be running.
>>> + */
>>> +int usbssp_start(struct usbssp_udc *usbssp_data)
>>> +{
>>> +	u32 temp;
>>> +	int ret;
>>> +
>>> +	temp = readl(&usbssp_data->op_regs->command);
>>> +	temp |= (CMD_RUN | CMD_DEVEN);
>>> +	usbssp_dbg_trace(usbssp_data, trace_usbssp_dbg_init,
>>> +			"// Turn on USBSSP, cmd = 0x%x.", temp);
>>> +	writel(temp, &usbssp_data->op_regs->command);
>>> +
>>> +	/*
>>> +	 * Wait for the HCHalted Staus bit to be 0 to indicate the device is
>>> +	 * running.
>>> +	 */
>>> +	ret = usbssp_handshake(&usbssp_data->op_regs->status,
>>> +			STS_HALT, 0, USBSSP_MAX_HALT_USEC);
>>> +
>>> +	if (ret == -ETIMEDOUT)
>>> +		dev_err(usbssp_data->dev, "Device took too long to start, waited %u microseconds.\n",
>>> +			USBSSP_MAX_HALT_USEC);
>>> +	if (!ret)
>>> +		/* clear state flags. Including dying, halted or removing */
>>> +		usbssp_data->usbssp_state = 0;
>>> +
>>> +	return ret;
>>> +}
>>> +
>>> +/*
>>> + * Reset a halted DC.
>>> + *
>>> + * This resets pipelines, timers, counters, state machines, etc.
>>> + * Transactions will be terminated immediately, and operational registers
>>> + * will be set to their defaults.
>>> + */
>>> +int usbssp_reset(struct usbssp_udc *usbssp_data)
>>> +{
>>> +	u32 command;
>>> +	u32 state;
>>> +	int ret;
>>> +
>>> +	state = readl(&usbssp_data->op_regs->status);
>>> +
>>> +	if (state == ~(u32)0) {
>>> +		dev_warn(usbssp_data->dev, "Device not accessible, reset failed.\n");
>>> +		return -ENODEV;
>>> +	}
>>> +
>>> +	if ((state & STS_HALT) == 0) {
>>> +		dev_warn(usbssp_data->dev, "DC not halted, aborting reset.\n");
>>
>> DC is not a familiar abbreviation. Mabe just use Controller? or Device controller.
> 
> In xhci driver is often used HC for host controller. In device driver I change this abbreviation 
> to  DC. I will review the patches and change this to Controller.  
> 
>>
>>> +		return 0;
>>> +	}
>>> +
>>> +	usbssp_dbg_trace(usbssp_data, trace_usbssp_dbg_init, "// Reset the DC");
>>> +	command = readl(&usbssp_data->op_regs->command);
>>> +	command |= CMD_RESET;
>>> +	writel(command, &usbssp_data->op_regs->command);
>>> +
>>> +	ret = usbssp_handshake(&usbssp_data->op_regs->command,
>>> +			CMD_RESET, 0, 10 * 1000 * 1000);
>>> +
>>> +	if (ret)
>>> +		return ret;
>>> +	usbssp_dbg_trace(usbssp_data, trace_usbssp_dbg_init,
>>> +		"Wait for controller to be ready for doorbell rings");
>>> +	/*
>>> +	 * USBSSP cannot write to any doorbells or operational registers other
>>> +	 * than status until the "Controller Not Ready" flag is cleared.
>>> +	 */
>>> +	ret = usbssp_handshake(&usbssp_data->op_regs->status,
>>> +			STS_CNR, 0, 10 * 1000 * 1000);
>>> +
>>> +	return ret;
>>> +}
>>>
>>>  /*
>>>   * Initialize memory for gadget driver and USBSSP (one-time init).
>>> @@ -179,8 +256,7 @@ int usbssp_gen_setup(struct usbssp_udc *usbssp_data)
>>>
>>>  	dev_dbg(usbssp_data->dev, "Resetting Device Controller\n");
>>>  	/* Reset the internal DC memory state and registers. */
>>> -	/*TODO: add implementation of usbssp_reset function*/
>>> -	//retval = usbssp_reset(usbssp_data);
>>> +	retval = usbssp_reset(usbssp_data);
>>>  	if (retval)
>>>  		return retval;
>>>  	dev_dbg(usbssp_data->dev, "Reset complete\n");
>>> @@ -244,8 +320,7 @@ int usbssp_gadget_init(struct usbssp_udc *usbssp_data)
>>>  	BUILD_BUG_ON(sizeof(struct usbssp_run_regs) != (8+8*128)*32/8);
>>>
>>>  	/* fill gadget fields */
>>> -	/*TODO: implements usbssp_gadget_ops object*/
>>> -	//usbssp_data->gadget.ops = &usbssp_gadget_ops;
>>> +	usbssp_data->gadget.ops = &usbssp_gadget_ops;
>>>  	usbssp_data->gadget.name = "usbssp-gadget";
>>>  	usbssp_data->gadget.max_speed = USB_SPEED_SUPER_PLUS;
>>>  	usbssp_data->gadget.speed = USB_SPEED_UNKNOWN;
>>> @@ -288,6 +363,7 @@ int usbssp_gadget_init(struct usbssp_udc *usbssp_data)
>>>  	usbssp_halt(usbssp_data);
>>>  	/*TODO add implementation of usbssp_reset function*/
>>>  	//usbssp_reset(usbssp_data);
>>> +	usbssp_reset(usbssp_data);
>>>  	/*TODO add implementation of freeing memory*/
>>>  	//usbssp_mem_cleanup(usbssp_data);
>>>  err3:
>>>
>>
>> --
>> cheers,
>> -roger
>>
>> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
>> Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
> 
> Cheers
> Pawel
> 

-- 
cheers,
-roger

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

WARNING: multiple messages have this Message-ID (diff)
From: Roger Quadros <rogerq@ti.com>
To: Pawel Laszczak <pawell@cadence.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	"linux-usb@vger.kernel.org" <linux-usb@vger.kernel.org>,
	Felipe Balbi <balbi@kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Lukasz Tyrala <ltyrala@cadence.com>,
	Alan Douglas <adouglas@cadence.com>
Subject: [06/31] usb: usbssp: added template functions used by upper layer.
Date: Mon, 6 Aug 2018 11:57:01 +0300	[thread overview]
Message-ID: <a44cf29a-3097-eb11-1dd4-de810eda0406@ti.com> (raw)

On 04/08/18 09:37, Pawel Laszczak wrote:
>>> Patch adds some functionality for initialization process.
>>> It adds to driver usbssp_reset and usbssp_start function.
>>>
>>> Next elements added are objects usbssp_gadget_ep0_ops,
>>> usbssp_gadget_ep_ops and usbssp_gadget_ops. These objects
>>> constitute the interface used by gadget subsystem.
>>> At this moment functions related to these objects are empty
>>> and do nothing.
>>>
>>> This patch also implements usbssp_gadget_init_endpoint and
>>> usbssp_gadget_free_endpoint used during initialization.
>>>
>>> Signed-off-by: Pawel Laszczak <pawell@cadence.com>
>>> ---
>>>  drivers/usb/usbssp/gadget-ext-caps.h |   3 +
>>>  drivers/usb/usbssp/gadget-if.c       | 269 ++++++++++++++++++++++++++-
>>>  drivers/usb/usbssp/gadget.c          |  84 ++++++++-
>>>  3 files changed, 350 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/drivers/usb/usbssp/gadget-ext-caps.h b/drivers/usb/usbssp/gadget-ext-caps.h
>>> index 2bf327046376..86c0ce331037 100644
>>> --- a/drivers/usb/usbssp/gadget-ext-caps.h
>>> +++ b/drivers/usb/usbssp/gadget-ext-caps.h
>>> @@ -51,3 +51,6 @@
>>>  #define USBSSP_CMD_EWE			BIT(10)
>>>
>>>  #define USBSSP_IRQS	(USBSSP_CMD_EIE | USBSSP_CMD_HSEIE | USBSSP_CMD_EWE)
>>> +
>>> +/* true: Controller Not Ready to accept doorbell or op reg writes after reset */
>>> +#define USBSSP_STS_CNR			BIT(11)
>>> diff --git a/drivers/usb/usbssp/gadget-if.c b/drivers/usb/usbssp/gadget-if.c
>>> index d53e0fb65299..70def978b085 100644
>>> --- a/drivers/usb/usbssp/gadget-if.c
>>> +++ b/drivers/usb/usbssp/gadget-if.c
>>> @@ -12,13 +12,278 @@
>>>  #include <linux/usb/composite.h>
>>>  #include "gadget.h"
>>>
>>> +static int usbssp_gadget_ep_enable(struct usb_ep *ep,
>>> +				   const struct usb_endpoint_descriptor *desc)
>>> +{
>>> +	struct usbssp_ep *ep_priv = to_usbssp_ep(ep);
>>> +	int ret = 0;
>>> +
>>> +	if (!ep_priv)
>>> +		return -EINVAL;
>>> +
>>> +	/*TODO: implements this function*/
>>> +	return ret;
>>> +}
>>> +
>>> +int usbssp_gadget_ep_disable(struct usb_ep *ep)
>>> +{
>>> +	struct usbssp_ep *ep_priv = to_usbssp_ep(ep);
>>> +	int ret = 0;
>>> +
>>> +	if (!ep_priv)
>>> +		return -EINVAL;
>>> +
>>> +	/*TODO: implements this function*/
>>> +	return ret;
>>> +}
>>> +
>>> +static struct usb_request *usbssp_gadget_ep_alloc_request(struct usb_ep *ep,
>>> +							  gfp_t gfp_flags)
>>> +{
>>> +	struct usbssp_ep *ep_priv = to_usbssp_ep(ep);
>>> +
>>> +	if (!ep_priv)
>>> +		return NULL;
>>> +
>>> +	/*TODO: implements this function*/
>>> +	return NULL;
>>> +}
>>> +
>>> +static void usbssp_gadget_ep_free_request(struct usb_ep *ep,
>>> +					  struct usb_request *request)
>>> +{
>>> +	struct usbssp_ep *ep_priv = to_usbssp_ep(ep);
>>> +
>>> +	if (!ep_priv)
>>> +		return;
>>> +
>>> +	/*TODO: implements this function*/
>>> +}
>>> +
>>> +static int usbssp_gadget_ep_queue(struct usb_ep *ep,
>>> +				  struct usb_request *request,
>>> +				  gfp_t gfp_flags)
>>> +{
>>> +	struct usbssp_ep *ep_priv = to_usbssp_ep(ep);
>>> +	int ret = 0;
>>> +
>>> +	if (!ep_priv)
>>> +		return -EINVAL;
>>> +
>>> +	/*TODO: implements this function*/
>>> +	return ret;
>>> +}
>>> +
>>> +static int usbssp_gadget_ep_dequeue(struct usb_ep *ep,
>>> +				    struct usb_request *request)
>>> +{
>>> +	struct usbssp_ep *ep_priv = to_usbssp_ep(ep);
>>> +	int ret = 0;
>>> +
>>> +	if (!ep_priv)
>>> +		return -EINVAL;
>>> +
>>> +	/*TODO: implements this function*/
>>> +	return ret;
>>> +}
>>> +
>>> +static int usbssp_gadget_ep_set_halt(struct usb_ep *ep, int value)
>>> +{
>>> +	struct usbssp_ep *ep_priv = to_usbssp_ep(ep);
>>> +	int ret = 0;
>>> +
>>> +	if (!ep_priv)
>>> +		return -EINVAL;
>>> +
>>> +	/*TODO: implements this function*/
>>> +	return ret;
>>> +}
>>> +
>>> +static int usbssp_gadget_ep_set_wedge(struct usb_ep *ep)
>>> +{
>>> +	struct usbssp_ep *ep_priv = to_usbssp_ep(ep);
>>> +	int ret = 0;
>>> +
>>> +	if (!ep_priv)
>>> +		return -EINVAL;
>>> +
>>> +	/*TODO: implements this function*/
>>> +	return ret;
>>> +}
>>> +
>>> +static const struct usb_ep_ops usbssp_gadget_ep0_ops = {
>>> +	.enable		= usbssp_gadget_ep_enable,
>>> +	.disable	= usbssp_gadget_ep_disable,
>>> +	.alloc_request	= usbssp_gadget_ep_alloc_request,
>>> +	.free_request	= usbssp_gadget_ep_free_request,
>>> +	.queue		= usbssp_gadget_ep_queue,
>>> +	.dequeue	= usbssp_gadget_ep_dequeue,
>>> +	.set_halt	= usbssp_gadget_ep_set_halt,
>>> +	.set_wedge	= usbssp_gadget_ep_set_wedge,
>>> +};
>>> +
>>> +static const struct usb_ep_ops usbssp_gadget_ep_ops = {
>>> +	.enable		= usbssp_gadget_ep_enable,
>>> +	.disable	= usbssp_gadget_ep_disable,
>>> +	.alloc_request	= usbssp_gadget_ep_alloc_request,
>>> +	.free_request	= usbssp_gadget_ep_free_request,
>>> +	.queue		= usbssp_gadget_ep_queue,
>>> +	.dequeue	= usbssp_gadget_ep_dequeue,
>>> +	.set_halt	= usbssp_gadget_ep_set_halt,
>>> +	.set_wedge	= usbssp_gadget_ep_set_wedge,
>>> +};
>>> +
>>> +static struct usb_endpoint_descriptor usbssp_gadget_ep0_desc = {
>>> +	.bLength =		USB_DT_ENDPOINT_SIZE,
>>> +	.bDescriptorType =	USB_DT_ENDPOINT,
>>> +	.bmAttributes =		USB_ENDPOINT_XFER_CONTROL,
>>> +};
>>> +
>>> +static int usbssp_gadget_start(struct usb_gadget *g,
>>> +			       struct usb_gadget_driver *driver)
>>> +{
>>> +	struct usbssp_udc *usbssp_data = gadget_to_usbssp(g);
>>> +	int ret = 0;
>>> +
>>> +	if (usbssp_data->gadget_driver) {
>>> +		dev_err(usbssp_data->dev, "%s is already bound to %s\n",
>>> +				usbssp_data->gadget.name,
>>> +				usbssp_data->gadget_driver->driver.name);
>>> +		ret = -EBUSY;
>>> +	}
>>> +
>>> +	/*TODO: add implementation*/
>>> +	return ret;
>>> +}
>>> +
>>> +static int usbssp_gadget_stop(struct usb_gadget *g)
>>> +{
>>> +	struct usbssp_udc *usbssp_data = gadget_to_usbssp(g);
>>> +
>>> +	if (!usbssp_data)
>>> +		return -EINVAL;
>>> +	/*TODO: add implementation*/
>>> +	return 0;
>>> +}
>>> +
>>> +static int usbssp_gadget_get_frame(struct usb_gadget *g)
>>> +{
>>> +	struct usbssp_udc *usbssp_data = gadget_to_usbssp(g);
>>> +
>>> +	if (!usbssp_data)
>>> +		return -EINVAL;
>>> +
>>> +	/*TODO: add implementation*/
>>> +	return 0;
>>> +}
>>> +
>>> +static int usbssp_gadget_wakeup(struct usb_gadget *g)
>>> +{
>>> +	struct usbssp_udc *usbssp_data = gadget_to_usbssp(g);
>>> +
>>> +	if (!usbssp_data)
>>> +		return -EINVAL;
>>> +
>>> +	/*TODO: add implementation*/
>>> +	return 0;
>>> +}
>>> +
>>> +static int usbssp_gadget_set_selfpowered(struct usb_gadget *g,
>>> +					 int is_selfpowered)
>>> +{
>>> +	struct usbssp_udc *usbssp_data = gadget_to_usbssp(g);
>>> +
>>> +	if (!usbssp_data)
>>> +		return -EINVAL;
>>> +
>>> +	/*TODO: add implementation*/
>>> +	return 0;
>>> +}
>>> +
>>> +static const struct usb_gadget_ops usbssp_gadget_ops = {
>>> +	.get_frame		= usbssp_gadget_get_frame,
>>> +	.wakeup			= usbssp_gadget_wakeup,
>>> +	.set_selfpowered	= usbssp_gadget_set_selfpowered,
>>> +	.udc_start		= usbssp_gadget_start,
>>> +	.udc_stop		= usbssp_gadget_stop,
>>> +};
>>> +
>>>  int usbssp_gadget_init_endpoint(struct usbssp_udc *usbssp_data)
>>
>> Since we're initializing all endpoints this function should be named usbssp_gadget_init_endpoints().
>>
>>>  {
>>> -	/*TODO: it has to be implemented*/
>>> +	int i = 0;
>>> +	struct usbssp_ep *ep_priv;
>>> +
>>> +	usbssp_data->num_endpoints = USBSSP_ENDPOINTS_NUM;
>>> +	INIT_LIST_HEAD(&usbssp_data->gadget.ep_list);
>>> +
>>> +	for (i = 1; i < usbssp_data->num_endpoints; i++) {
>>> +		bool direction = i & 1; /*start from OUT endpoint*/
>>> +		u8 epnum = (i >> 1);
>>> +
>>> +		ep_priv = &usbssp_data->devs.eps[i-1];
>>> +		ep_priv->usbssp_data = usbssp_data;
>>> +		ep_priv->number = epnum;
>>> +		ep_priv->direction = direction; /*0 for OUT, 1 for IN*/
>>> +
>>> +		snprintf(ep_priv->name, sizeof(ep_priv->name), "ep%d%s", epnum,
>>> +				(ep_priv->direction) ? "in" : "out");
>>> +
>>> +		ep_priv->endpoint.name = ep_priv->name;
>>> +
>>> +		if (ep_priv->number < 2) {
>>> +			ep_priv->endpoint.desc = &usbssp_gadget_ep0_desc;
>>> +			ep_priv->endpoint.comp_desc = NULL;
>>> +		}
>>> +
>>> +		if (epnum == 0) {
>>> +			/*EP0 is bidirectional endpoint*/
>>> +			usb_ep_set_maxpacket_limit(&ep_priv->endpoint, 512);
>>> +			dev_dbg(usbssp_data->dev,
>>> +				"Initializing %s, MaxPack: %04x Type: Ctrl\n",
>>> +				ep_priv->name, 512);
>>> +			ep_priv->endpoint.maxburst = 1;
>>> +			ep_priv->endpoint.ops = &usbssp_gadget_ep0_ops;
>>> +			ep_priv->endpoint.caps.type_control = true;
>>> +
>>> +			usbssp_data->usb_req_ep0_in.epnum = ep_priv->number;
>>> +			usbssp_data->usb_req_ep0_in.dep = ep_priv;
>>> +
>>> +			if (!epnum)
>>> +				usbssp_data->gadget.ep0 = &ep_priv->endpoint;
>>> +		} else {
>>> +			usb_ep_set_maxpacket_limit(&ep_priv->endpoint, 1024);
>>> +			ep_priv->endpoint.maxburst = 15;
>>> +			ep_priv->endpoint.ops = &usbssp_gadget_ep_ops;
>>> +			list_add_tail(&ep_priv->endpoint.ep_list,
>>> +					&usbssp_data->gadget.ep_list);
>>> +			ep_priv->endpoint.caps.type_iso = true;
>>> +			ep_priv->endpoint.caps.type_bulk = true;
>>> +			ep_priv->endpoint.caps.type_int = true;
>>> +
>>> +		}
>>> +
>>> +		ep_priv->endpoint.caps.dir_in = direction;
>>> +		ep_priv->endpoint.caps.dir_out = !direction;
>>
>> Since you start from 1, ep0 will only be initialized as dir_in.
>> Is it better to represent EP0in and EP0out separately?
>> If so you can start i from 0 and use
>> 	ep_priv = &usbssp_data->devs.eps[i];
>>
>> This should fix the direction. eps[0] will be ep0out and eps[1] will be ep0in.
> 
> I wanted to be consistent with Device Context Data Structure from 
> XHCI specification (EP Context 0 BiDir, EP Context 1 OUT, EP Context 1 IN ...).
> 
> I little change this function, and now it look like this:
> int usbssp_gadget_init_endpoints(struct usbssp_udc *usbssp_data)
> {
> 	int i = 0;
> 	struct usbssp_ep *ep_priv;
> 
> 	usbssp_data->num_endpoints = USBSSP_ENDPOINTS_NUM;
> 	INIT_LIST_HEAD(&usbssp_data->gadget.ep_list);
> 
> 	for (i = 0; i < usbssp_data->num_endpoints; i++) {
> 		bool direction = i & 1; /*start from OUT endpoint*/
> 		u8 epnum = ((i + 1) >> 1);
> 
> 		ep_priv = &usbssp_data->devs.eps[i];
> 		ep_priv->usbssp_data = usbssp_data;
> 		ep_priv->number = epnum;
> 		ep_priv->direction = direction; /*0 for OUT, 1 for IN*/
> 
> 		/*
> 		 *Ep0 is bidirectional so Ep0In and Ep0Out is represented by
> 		 * usbssp_data->devs.eps[0]
> 		 */
> 		if (epnum == 0) {
> 			snprintf(ep_priv->name, sizeof(ep_priv->name), "ep%d%s",
> 				epnum, "BiDir");
> 
> 			usb_ep_set_maxpacket_limit(&ep_priv->endpoint, 512);
> 			ep_priv->endpoint.maxburst = 1;
> 			ep_priv->endpoint.ops = &usbssp_gadget_ep0_ops;
> 			ep_priv->endpoint.desc = &usbssp_gadget_ep0_desc;
> 
> 			ep_priv->endpoint.comp_desc = NULL;
> 			ep_priv->endpoint.caps.type_control = true;
> 			ep_priv->endpoint.caps.dir_in = true;
> 			ep_priv->endpoint.caps.dir_out = true;
> 
> 			usbssp_data->usb_req_ep0_in.epnum = ep_priv->number;
> 			usbssp_data->usb_req_ep0_in.dep = ep_priv;
> 			usbssp_data->gadget.ep0 = &ep_priv->endpoint;
> 
> 		} else {
> 			snprintf(ep_priv->name, sizeof(ep_priv->name), "ep%d%s",
> 				epnum, (ep_priv->direction) ? "in"  : "out");
> 
> 			usb_ep_set_maxpacket_limit(&ep_priv->endpoint, 1024);
> 			ep_priv->endpoint.maxburst = 15;
> 			ep_priv->endpoint.ops = &usbssp_gadget_ep_ops;
> 			list_add_tail(&ep_priv->endpoint.ep_list,
> 					&usbssp_data->gadget.ep_list);
> 
> 			ep_priv->endpoint.caps.type_iso = true;
> 			ep_priv->endpoint.caps.type_bulk = true;
> 			ep_priv->endpoint.caps.type_int = true;
> 
> 			ep_priv->endpoint.caps.dir_in = direction;
> 			ep_priv->endpoint.caps.dir_out = !direction;
> 		}
> 
> 		ep_priv->endpoint.name = ep_priv->name;

I think this is much better now.

> 
> 		dev_dbg(usbssp_data->dev, "Init %s, MaxPack: %04x SupType: "
> 			"CTRL: %s, INT: %s, BULK: %s, ISOC %s, "
> 			"SupDir IN: %s, OUT: %s\n",
> 			ep_priv->name, 1024,
> 			(ep_priv->endpoint.caps.type_control) ? "yes" : "no",
> 			(ep_priv->endpoint.caps.type_int) ? "yes" : "no",
> 			(ep_priv->endpoint.caps.type_bulk) ? "yes" : "no",
> 			(ep_priv->endpoint.caps.type_iso) ? "yes" : "no",
> 			(ep_priv->endpoint.caps.dir_in) ? "yes" : "no",
> 			(ep_priv->endpoint.caps.dir_out) ? "yes" : "no");
> 

In some places you use usbssp_dbg_trace() and at others you use dev_dbg().
Is it better to stick with one throughout?

> 		INIT_LIST_HEAD(&ep_priv->pending_list);
> 	}
> 
> 	return 0;
> }
> 
>>> +
>>> +		dev_dbg(usbssp_data->dev, "Init %s, MaxPack: %04x SupType:"
>>> +				" INT/BULK/ISOC , SupDir %s\n",
>>> +				ep_priv->name, 1024,
>>> +				(ep_priv->endpoint.caps.dir_in) ? "IN" : "OUT");
>>> +
>>> +		INIT_LIST_HEAD(&ep_priv->pending_list);
>>> +	}
>>>  	return 0;
>>>  }
>>>
>>>  void usbssp_gadget_free_endpoint(struct usbssp_udc *usbssp_data)
>>>  {
>>> -	/*TODO: it has to be implemented*/
>>> +	int i;
>>> +	struct usbssp_ep *ep_priv;
>>> +
>>> +	for (i = 0; i < usbssp_data->num_endpoints; i++) {
>>> +		ep_priv = &usbssp_data->devs.eps[i];
>>> +
>>
>> if you start i from 1 then you can skip the if().
>>
>>> +		if (ep_priv->number != 0)
>>> +			list_del(&ep_priv->endpoint.ep_list);
>>> +	}
>>>  }
>>> diff --git a/drivers/usb/usbssp/gadget.c b/drivers/usb/usbssp/gadget.c
>>> index 338ec2ec18b1..195f5777cf8a 100644
>>> --- a/drivers/usb/usbssp/gadget.c
>>> +++ b/drivers/usb/usbssp/gadget.c
>>> @@ -103,6 +103,83 @@ int usbssp_halt(struct usbssp_udc *usbssp_data)
>>>  	return ret;
>>>  }
>>>
>>> +/*
>>> + * Set the run bit and wait for the device to be running.
>>> + */
>>> +int usbssp_start(struct usbssp_udc *usbssp_data)
>>> +{
>>> +	u32 temp;
>>> +	int ret;
>>> +
>>> +	temp = readl(&usbssp_data->op_regs->command);
>>> +	temp |= (CMD_RUN | CMD_DEVEN);
>>> +	usbssp_dbg_trace(usbssp_data, trace_usbssp_dbg_init,
>>> +			"// Turn on USBSSP, cmd = 0x%x.", temp);
>>> +	writel(temp, &usbssp_data->op_regs->command);
>>> +
>>> +	/*
>>> +	 * Wait for the HCHalted Staus bit to be 0 to indicate the device is
>>> +	 * running.
>>> +	 */
>>> +	ret = usbssp_handshake(&usbssp_data->op_regs->status,
>>> +			STS_HALT, 0, USBSSP_MAX_HALT_USEC);
>>> +
>>> +	if (ret == -ETIMEDOUT)
>>> +		dev_err(usbssp_data->dev, "Device took too long to start, waited %u microseconds.\n",
>>> +			USBSSP_MAX_HALT_USEC);
>>> +	if (!ret)
>>> +		/* clear state flags. Including dying, halted or removing */
>>> +		usbssp_data->usbssp_state = 0;
>>> +
>>> +	return ret;
>>> +}
>>> +
>>> +/*
>>> + * Reset a halted DC.
>>> + *
>>> + * This resets pipelines, timers, counters, state machines, etc.
>>> + * Transactions will be terminated immediately, and operational registers
>>> + * will be set to their defaults.
>>> + */
>>> +int usbssp_reset(struct usbssp_udc *usbssp_data)
>>> +{
>>> +	u32 command;
>>> +	u32 state;
>>> +	int ret;
>>> +
>>> +	state = readl(&usbssp_data->op_regs->status);
>>> +
>>> +	if (state == ~(u32)0) {
>>> +		dev_warn(usbssp_data->dev, "Device not accessible, reset failed.\n");
>>> +		return -ENODEV;
>>> +	}
>>> +
>>> +	if ((state & STS_HALT) == 0) {
>>> +		dev_warn(usbssp_data->dev, "DC not halted, aborting reset.\n");
>>
>> DC is not a familiar abbreviation. Mabe just use Controller? or Device controller.
> 
> In xhci driver is often used HC for host controller. In device driver I change this abbreviation 
> to  DC. I will review the patches and change this to Controller.  
> 
>>
>>> +		return 0;
>>> +	}
>>> +
>>> +	usbssp_dbg_trace(usbssp_data, trace_usbssp_dbg_init, "// Reset the DC");
>>> +	command = readl(&usbssp_data->op_regs->command);
>>> +	command |= CMD_RESET;
>>> +	writel(command, &usbssp_data->op_regs->command);
>>> +
>>> +	ret = usbssp_handshake(&usbssp_data->op_regs->command,
>>> +			CMD_RESET, 0, 10 * 1000 * 1000);
>>> +
>>> +	if (ret)
>>> +		return ret;
>>> +	usbssp_dbg_trace(usbssp_data, trace_usbssp_dbg_init,
>>> +		"Wait for controller to be ready for doorbell rings");
>>> +	/*
>>> +	 * USBSSP cannot write to any doorbells or operational registers other
>>> +	 * than status until the "Controller Not Ready" flag is cleared.
>>> +	 */
>>> +	ret = usbssp_handshake(&usbssp_data->op_regs->status,
>>> +			STS_CNR, 0, 10 * 1000 * 1000);
>>> +
>>> +	return ret;
>>> +}
>>>
>>>  /*
>>>   * Initialize memory for gadget driver and USBSSP (one-time init).
>>> @@ -179,8 +256,7 @@ int usbssp_gen_setup(struct usbssp_udc *usbssp_data)
>>>
>>>  	dev_dbg(usbssp_data->dev, "Resetting Device Controller\n");
>>>  	/* Reset the internal DC memory state and registers. */
>>> -	/*TODO: add implementation of usbssp_reset function*/
>>> -	//retval = usbssp_reset(usbssp_data);
>>> +	retval = usbssp_reset(usbssp_data);
>>>  	if (retval)
>>>  		return retval;
>>>  	dev_dbg(usbssp_data->dev, "Reset complete\n");
>>> @@ -244,8 +320,7 @@ int usbssp_gadget_init(struct usbssp_udc *usbssp_data)
>>>  	BUILD_BUG_ON(sizeof(struct usbssp_run_regs) != (8+8*128)*32/8);
>>>
>>>  	/* fill gadget fields */
>>> -	/*TODO: implements usbssp_gadget_ops object*/
>>> -	//usbssp_data->gadget.ops = &usbssp_gadget_ops;
>>> +	usbssp_data->gadget.ops = &usbssp_gadget_ops;
>>>  	usbssp_data->gadget.name = "usbssp-gadget";
>>>  	usbssp_data->gadget.max_speed = USB_SPEED_SUPER_PLUS;
>>>  	usbssp_data->gadget.speed = USB_SPEED_UNKNOWN;
>>> @@ -288,6 +363,7 @@ int usbssp_gadget_init(struct usbssp_udc *usbssp_data)
>>>  	usbssp_halt(usbssp_data);
>>>  	/*TODO add implementation of usbssp_reset function*/
>>>  	//usbssp_reset(usbssp_data);
>>> +	usbssp_reset(usbssp_data);
>>>  	/*TODO add implementation of freeing memory*/
>>>  	//usbssp_mem_cleanup(usbssp_data);
>>>  err3:
>>>
>>
>> --
>> cheers,
>> -roger
>>
>> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
>> Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
> 
> Cheers
> Pawel
>

  reply	other threads:[~2018-08-06  8:57 UTC|newest]

Thread overview: 102+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-07-19 17:57 [PATCH 00/31] Introduced new Cadence USBSSP DRD Driver Pawel Laszczak
2018-07-19 17:57 ` [PATCH 01/31] usb: usbssp: Defined register maps and other useful structures Pawel Laszczak
2018-07-19 17:57   ` [01/31] " Pawel Laszczak
2018-07-19 17:57 ` [PATCH 02/31] usb: usbssp: Added some decoding functions Pawel Laszczak
2018-07-19 17:57   ` [02/31] " Pawel Laszczak
2018-09-10 18:18   ` [PATCH 02/31] " Greg Kroah-Hartman
2018-09-10 18:18     ` [02/31] " Greg Kroah-Hartman
2018-09-11  5:48     ` [PATCH 02/31] " Felipe Balbi
2018-09-11  5:48       ` [02/31] " Felipe Balbi
2018-09-11  8:12       ` [PATCH 02/31] " Greg Kroah-Hartman
2018-09-11  8:12         ` [02/31] " Greg Kroah-Hartman
2018-09-11  9:01         ` [PATCH 02/31] " Pawel Laszczak
2018-09-11  9:01           ` [02/31] " Pawel Laszczak
2018-07-19 17:57 ` [PATCH 03/31] usb: usbssp: Add trace events used in driver Pawel Laszczak
2018-07-19 17:57   ` [03/31] " Pawel Laszczak
2018-07-19 17:57 ` [PATCH 04/31] usb: usbssp: Added USBSSP platform driver Pawel Laszczak
2018-07-19 17:57   ` [04/31] " Pawel Laszczak
2018-08-01 12:24   ` [PATCH 04/31] " Roger Quadros
2018-08-01 12:24     ` [04/31] " Roger Quadros
2018-08-02  6:25     ` [PATCH 04/31] " Pawel Laszczak
2018-08-02  6:25       ` [04/31] " Pawel Laszczak
2018-08-02 13:28       ` [PATCH 04/31] " Roger Quadros
2018-08-02 13:28         ` [04/31] " Roger Quadros
2018-07-19 17:57 ` [PATCH 05/31] usb: usbssp: Added first part of initialization sequence Pawel Laszczak
2018-07-19 17:57   ` [05/31] " Pawel Laszczak
2018-08-03 10:17   ` [PATCH 05/31] " Roger Quadros
2018-08-03 10:17     ` [05/31] " Roger Quadros
2018-08-06  8:57     ` [PATCH 05/31] " Pawel Laszczak
2018-08-06  8:57       ` [05/31] " Pawel Laszczak
2018-08-06 10:33       ` [PATCH 05/31] " Roger Quadros
2018-08-06 10:33         ` [05/31] " Roger Quadros
2018-08-06 12:03         ` [PATCH 05/31] " Pawel Laszczak
2018-08-06 12:03           ` [05/31] " Pawel Laszczak
2018-07-19 17:57 ` [PATCH 06/31] usb: usbssp: added template functions used by upper layer Pawel Laszczak
2018-07-19 17:57   ` [06/31] " Pawel Laszczak
2018-08-03 10:42   ` [PATCH 06/31] " Roger Quadros
2018-08-03 10:42     ` [06/31] " Roger Quadros
2018-08-04  6:37     ` [PATCH 06/31] " Pawel Laszczak
2018-08-04  6:37       ` [06/31] " Pawel Laszczak
2018-08-06  8:57       ` Roger Quadros [this message]
2018-08-06  8:57         ` Roger Quadros
2018-08-06 11:40         ` [PATCH 06/31] " Pawel Laszczak
2018-08-06 11:40           ` [06/31] " Pawel Laszczak
2018-07-19 17:57 ` [PATCH 07/31] usb: usbssp: Initialization - added usbssp_mem_init Pawel Laszczak
2018-07-19 17:57   ` [07/31] " Pawel Laszczak
2018-07-19 17:57 ` [PATCH 08/31] usb: usbssp: Added ring and segment handling functions Pawel Laszczak
2018-07-19 17:57   ` [08/31] " Pawel Laszczak
2018-07-19 17:57 ` [PATCH 09/31] usb: usbssp: add implementation of usbssp_mem_cleanup Pawel Laszczak
2018-07-19 17:57   ` [09/31] " Pawel Laszczak
2018-07-19 17:57 ` [PATCH 10/31] usb: usbssp: added usbssp_trb_in_td function Pawel Laszczak
2018-07-19 17:57   ` [10/31] " Pawel Laszczak
2018-07-19 17:57 ` [PATCH 11/31] usb: usbssp: added function for stopping driver Pawel Laszczak
2018-07-19 17:57   ` [11/31] " Pawel Laszczak
2018-07-19 17:57 ` [PATCH 12/31] usb: usbssp: added functions for queuing commands Pawel Laszczak
2018-07-19 17:57   ` [12/31] " Pawel Laszczak
2018-07-19 17:57 ` [PATCH 13/31] usb: usbssp: addec procedure for handlin Port Status Change events Pawel Laszczak
2018-07-19 17:57   ` [13/31] " Pawel Laszczak
2018-07-19 17:57 ` [PATCH 14/31] usb: usbssp: added procedure handling command completion events Pawel Laszczak
2018-07-19 17:57   ` [14/31] " Pawel Laszczak
2018-07-19 17:57 ` [PATCH 15/31] usb: usbssp: added device controller error, transfer and SETUP completion event Pawel Laszczak
2018-07-19 17:57   ` [15/31] " Pawel Laszczak
2018-07-19 17:57 ` [PATCH 16/31] usb: usbssp: added connect/disconnect procedures Pawel Laszczak
2018-07-19 17:57   ` [16/31] " Pawel Laszczak
2018-07-19 17:57 ` [PATCH 17/31] usb: usbssp: added implementation of usbssp_halt_endpoint function Pawel Laszczak
2018-07-19 17:57   ` [17/31] " Pawel Laszczak
2018-07-19 17:57 ` [PATCH 18/31] usb: usbssp: added handling of Port Reset event Pawel Laszczak
2018-07-19 17:57   ` [18/31] " Pawel Laszczak
2018-07-19 17:57 ` [PATCH 19/31] usb: usbssp: added support for USB enumeration process Pawel Laszczak
2018-07-19 17:57   ` [19/31] " Pawel Laszczak
2018-07-19 17:57 ` [PATCH 20/31] usb: usbssp: added queuing procedure for control transfer Pawel Laszczak
2018-07-19 17:57   ` [20/31] " Pawel Laszczak
2018-07-19 17:57 ` [PATCH 21/31] usb: usbssp: added queuing procedure for BULK and INT transfer Pawel Laszczak
2018-07-19 17:57   ` [21/31] " Pawel Laszczak
2018-07-19 17:57 ` [PATCH 22/31] usb: usbssp: added procedure removing request from transfer ring Pawel Laszczak
2018-07-19 17:57   ` [22/31] " Pawel Laszczak
2018-07-19 17:57 ` [PATCH 23/31] usb: usbssp: added implementation of transfer events Pawel Laszczak
2018-07-19 17:57   ` [23/31] " Pawel Laszczak
2018-07-19 17:57 ` [PATCH 24/31] usb: usbssp: added detecting command timeout Pawel Laszczak
2018-07-19 17:57   ` [24/31] " Pawel Laszczak
2018-07-19 17:57 ` [PATCH 25/31] usb: usbssp: added implementation of usbssp interface Pawel Laszczak
2018-07-19 17:57   ` [25/31] " Pawel Laszczak
2018-07-19 17:57 ` [PATCH 26/31] usb: usbssp: added endpoint configuration functionality Pawel Laszczak
2018-07-19 17:57   ` [26/31] " Pawel Laszczak
2018-07-19 17:58 ` [PATCH 27/31] usb: usbssp: implements usbssp_gadget_ep_enable function Pawel Laszczak
2018-07-19 17:58   ` [27/31] " Pawel Laszczak
2018-07-19 17:58 ` [PATCH 28/31] usb: usbssp: implemented usbssp_gadget_ep_disable function Pawel Laszczak
2018-07-19 17:58   ` [28/31] " Pawel Laszczak
2018-07-19 17:58 ` [PATCH 29/31] usb: usbssp: added support for LPM Pawel Laszczak
2018-07-19 17:58   ` [29/31] " Pawel Laszczak
2018-07-19 17:58 ` [PATCH 30/31] usb: usbssp: added support for TEST_MODE Pawel Laszczak
2018-07-19 17:58   ` [30/31] " Pawel Laszczak
2018-07-19 17:58 ` [PATCH 31/31] usb: usbssp: add pci to platform driver wrapper Pawel Laszczak
2018-07-19 17:58   ` [31/31] " Pawel Laszczak
2018-08-01 11:27 ` [PATCH 00/31] Introduced new Cadence USBSSP DRD Driver Roger Quadros
2018-08-02  4:26   ` Pawel Laszczak
2018-08-02 13:24     ` Roger Quadros
2018-09-10 18:21     ` Greg Kroah-Hartman
2018-08-17 21:05 ` Bin Liu
2018-08-21 14:50   ` Pawel Laszczak
2018-09-10 18:20     ` Greg Kroah-Hartman
2018-09-10 18:16 ` Greg Kroah-Hartman
  -- strict thread matches above, loose matches on Subject: below --
2018-07-12  5:46 Pawel Laszczak
2018-07-12  5:47 ` [PATCH 06/31] usb: usbssp: added template functions used by upper layer Pawel Laszczak

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=a44cf29a-3097-eb11-1dd4-de810eda0406@ti.com \
    --to=rogerq@ti.com \
    --cc=adouglas@cadence.com \
    --cc=balbi@kernel.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=ltyrala@cadence.com \
    --cc=pawell@cadence.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.