RE: [PATCH v10 5/6] usb:cdns3 Add Cadence USB3 DRD Driver

2019-08-26 Thread Pawel Laszczak
Hi Peter

>
>On 19-07-21 19:32:18, Pawel Laszczak wrote:
>> This patch introduce new Cadence USBSS DRD driver to Linux kernel.
>>
>> The Cadence USBSS DRD Controller is a highly configurable IP Core which
>> can be instantiated as Dual-Role Device (DRD), Peripheral Only and
>> Host Only (XHCI)configurations.
>>
>> The current driver has been validated with FPGA platform. We have
>> support for PCIe bus, which is used on FPGA prototyping.
>>
>> The host side of USBSS-DRD controller is compliant with XHCI
>> specification, so it works with standard XHCI Linux driver.
>>
>> Signed-off-by: Pawel Laszczak 
>> ---
>>  drivers/usb/Kconfig|2 +
>>  drivers/usb/Makefile   |2 +
>>  drivers/usb/cdns3/Kconfig  |   46 +
>>  drivers/usb/cdns3/Makefile |   17 +
>>  drivers/usb/cdns3/cdns3-pci-wrap.c |  203 +++
>>  drivers/usb/cdns3/core.c   |  554 +++
>>  drivers/usb/cdns3/core.h   |  109 ++
>>  drivers/usb/cdns3/debug.h  |  171 ++
>>  drivers/usb/cdns3/debugfs.c|   87 ++
>>  drivers/usb/cdns3/drd.c|  390 +
>>  drivers/usb/cdns3/drd.h|  166 ++
>>  drivers/usb/cdns3/ep0.c|  914 +++
>>  drivers/usb/cdns3/gadget-export.h  |   28 +
>>  drivers/usb/cdns3/gadget.c | 2338 
>>  drivers/usb/cdns3/gadget.h | 1321 
>>  drivers/usb/cdns3/host-export.h|   28 +
>>  drivers/usb/cdns3/host.c   |   71 +
>>  drivers/usb/cdns3/trace.c  |   11 +
>>  drivers/usb/cdns3/trace.h  |  493 ++
>>  19 files changed, 6951 insertions(+)
>>  create mode 100644 drivers/usb/cdns3/Kconfig
>>  create mode 100644 drivers/usb/cdns3/Makefile
>>  create mode 100644 drivers/usb/cdns3/cdns3-pci-wrap.c
>>  create mode 100644 drivers/usb/cdns3/core.c
>>  create mode 100644 drivers/usb/cdns3/core.h
>>  create mode 100644 drivers/usb/cdns3/debug.h
>>  create mode 100644 drivers/usb/cdns3/debugfs.c
>>  create mode 100644 drivers/usb/cdns3/drd.c
>>  create mode 100644 drivers/usb/cdns3/drd.h
>>  create mode 100644 drivers/usb/cdns3/ep0.c
>>  create mode 100644 drivers/usb/cdns3/gadget-export.h
>>  create mode 100644 drivers/usb/cdns3/gadget.c
>>  create mode 100644 drivers/usb/cdns3/gadget.h
>>  create mode 100644 drivers/usb/cdns3/host-export.h
>>  create mode 100644 drivers/usb/cdns3/host.c
>>  create mode 100644 drivers/usb/cdns3/trace.c
>>  create mode 100644 drivers/usb/cdns3/trace.h
>>
>
>Pawel, one question duirng my debug, what's purpose
>for below code:
>function: cdns3_gadget_ep_dequeue
>
>   /* Update ring */
>   request = cdns3_next_request(&priv_ep->deferred_req_list);

If you have on mind this line, then yes it should be removed from here. 

Driver only should allow controller to jump to next TRB in TR. 
If it's last TRB in HW ring and there is next request on deferred_req_list then
new transfer will be started in TRBERR event.

I Will remove it, 
Thanks Peter 
 
>   if (request) {
>   priv_req = to_cdns3_request(request);
>
>   link_trb->buffer = TRB_BUFFER(priv_ep->trb_pool_dma +
> (priv_req->start_trb * TRB_SIZE));
>   link_trb->control = (link_trb->control & TRB_CYCLE) |
>   TRB_TYPE(TRB_LINK) | TRB_CHAIN | TRB_TOGGLE;
>   } else {
>   priv_ep->flags |= EP_UPDATE_EP_TRBADDR;
>   }
>
>Besides, would you please cc me when you send next version?
>Thanks,
>

I was always adding you.  


>> +
>> +#include 
>> --
>> 2.17.1
>>

Cheers,
Pawell



RE: [PATCH v10 5/6] usb:cdns3 Add Cadence USB3 DRD Driver

2019-08-23 Thread Pawel Laszczak
Hi,

>Hi,
>
>On 22/07/19 12:02 AM, Pawel Laszczak wrote:
>> +
>> +/**
>> + * 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 */
>> +if (priv_dev->is_selfpowered)
>> +usb_status = BIT(USB_DEVICE_SELF_POWERED);
>> +
>> +if (priv_dev->wake_up_flag)
>> +usb_status |= BIT(USB_DEVICE_REMOTE_WAKEUP);
>> +
>> +if (priv_dev->gadget.speed != USB_SPEED_SUPER)
>> +break;
>> +
>> +reg = readl(&priv_dev->regs->usb_sts);
>
>I see usb_sts is read, but never used in this function?

It's true, it's not used. 

Thanks.

>
>> +
>> +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);
>> +
>> +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_buf;
>> +*response_pkt = cpu_to_le16(usb_status);
>> +
>> +cdns3_ep0_run_transfer(priv_dev, priv_dev->setup_dma,
>> +   sizeof(*response_pkt), 1, 0);
>> +return 0;
>> +}
>> +
>
--
Cheers
Pawell



Re: [PATCH v10 5/6] usb:cdns3 Add Cadence USB3 DRD Driver

2019-08-23 Thread Vignesh Raghavendra
Hi,

On 22/07/19 12:02 AM, Pawel Laszczak wrote:
> +
> +/**
> + * 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 */
> + if (priv_dev->is_selfpowered)
> + usb_status = BIT(USB_DEVICE_SELF_POWERED);
> +
> + if (priv_dev->wake_up_flag)
> + usb_status |= BIT(USB_DEVICE_REMOTE_WAKEUP);
> +
> + if (priv_dev->gadget.speed != USB_SPEED_SUPER)
> + break;
> +
> + reg = readl(&priv_dev->regs->usb_sts);

I see usb_sts is read, but never used in this function?

> +
> + 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);
> +
> + 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_buf;
> + *response_pkt = cpu_to_le16(usb_status);
> +
> + cdns3_ep0_run_transfer(priv_dev, priv_dev->setup_dma,
> +sizeof(*response_pkt), 1, 0);
> + return 0;
> +}
> +

-- 
Regards
Vignesh


Re: [PATCH v10 5/6] usb:cdns3 Add Cadence USB3 DRD Driver

2019-08-19 Thread Roger Quadros
Pawel,

On 19/08/2019 13:30, Pawel Laszczak wrote:
> Hi,
> 
>>
>> On 21/07/2019 21:32, Pawel Laszczak wrote:
>>> This patch introduce new Cadence USBSS DRD driver to Linux kernel.
>>>
>>> The Cadence USBSS DRD Controller is a highly configurable IP Core which
>>> can be instantiated as Dual-Role Device (DRD), Peripheral Only and
>>> Host Only (XHCI)configurations.
>>>
>>> The current driver has been validated with FPGA platform. We have
>>> support for PCIe bus, which is used on FPGA prototyping.
>>>
>>> The host side of USBSS-DRD controller is compliant with XHCI
>>> specification, so it works with standard XHCI Linux driver.
>>>
>>> Signed-off-by: Pawel Laszczak 
>>> ---
>>>  drivers/usb/Kconfig|2 +
>>>  drivers/usb/Makefile   |2 +
>>>  drivers/usb/cdns3/Kconfig  |   46 +
>>>  drivers/usb/cdns3/Makefile |   17 +
>>>  drivers/usb/cdns3/cdns3-pci-wrap.c |  203 +++
>>>  drivers/usb/cdns3/core.c   |  554 +++
>>>  drivers/usb/cdns3/core.h   |  109 ++
>>>  drivers/usb/cdns3/debug.h  |  171 ++
>>>  drivers/usb/cdns3/debugfs.c|   87 ++
>>>  drivers/usb/cdns3/drd.c|  390 +
>>>  drivers/usb/cdns3/drd.h|  166 ++
>>>  drivers/usb/cdns3/ep0.c|  914 +++
>>>  drivers/usb/cdns3/gadget-export.h  |   28 +
>>>  drivers/usb/cdns3/gadget.c | 2338 
>>>  drivers/usb/cdns3/gadget.h | 1321 
>>>  drivers/usb/cdns3/host-export.h|   28 +
>>>  drivers/usb/cdns3/host.c   |   71 +
>>>  drivers/usb/cdns3/trace.c  |   11 +
>>>  drivers/usb/cdns3/trace.h  |  493 ++
>>>  19 files changed, 6951 insertions(+)
>>>  create mode 100644 drivers/usb/cdns3/Kconfig
>>>  create mode 100644 drivers/usb/cdns3/Makefile
>>>  create mode 100644 drivers/usb/cdns3/cdns3-pci-wrap.c
>>>  create mode 100644 drivers/usb/cdns3/core.c
>>>  create mode 100644 drivers/usb/cdns3/core.h
>>>  create mode 100644 drivers/usb/cdns3/debug.h
>>>  create mode 100644 drivers/usb/cdns3/debugfs.c
>>>  create mode 100644 drivers/usb/cdns3/drd.c
>>>  create mode 100644 drivers/usb/cdns3/drd.h
>>>  create mode 100644 drivers/usb/cdns3/ep0.c
>>>  create mode 100644 drivers/usb/cdns3/gadget-export.h
>>>  create mode 100644 drivers/usb/cdns3/gadget.c
>>>  create mode 100644 drivers/usb/cdns3/gadget.h
>>>  create mode 100644 drivers/usb/cdns3/host-export.h
>>>  create mode 100644 drivers/usb/cdns3/host.c
>>>  create mode 100644 drivers/usb/cdns3/trace.c
>>>  create mode 100644 drivers/usb/cdns3/trace.h
>>>
>>
>> 
>>
>>> diff --git a/drivers/usb/cdns3/core.c b/drivers/usb/cdns3/core.c
>>> new file mode 100644
>>> index ..900b2ce08162
>>> --- /dev/null
>>> +++ b/drivers/usb/cdns3/core.c
>>> @@ -0,0 +1,554 @@
>>> +// SPDX-License-Identifier: GPL-2.0
>>> +/*
>>> + * Cadence USBSS DRD Driver.
>>> + *
>>> + * Copyright (C) 2018-2019 Cadence.
>>> + * Copyright (C) 2017-2018 NXP
>>> + * Copyright (C) 2019 Texas Instruments
>>> + *
>>> + * Author: Peter Chen 
>>> + * Pawel Laszczak 
>>> + * Roger Quadros 
>>> + */
>>> +
>>> +#include 
>>> +#include 
>>> +#include 
>>> +#include 
>>> +#include 
>>> +#include 
>>> +#include 
>>> +
>>> +#include "gadget.h"
>>> +#include "core.h"
>>> +#include "host-export.h"
>>> +#include "gadget-export.h"
>>> +#include "drd.h"
>>> +#include "debug.h"
>>> +
>>> +static inline
>>> +struct cdns3_role_driver *cdns3_get_current_role_driver(struct cdns3 *cdns)
>>> +{
>>> +   WARN_ON(!cdns->roles[cdns->role]);
>>> +   return cdns->roles[cdns->role];
>>> +}
>>> +
>>> +static int cdns3_role_start(struct cdns3 *cdns, enum usb_role role)
>>> +{
>>> +   int ret;
>>> +
>>> +   if (WARN_ON(role > USB_ROLE_DEVICE))
>>> +   return 0;
>>> +
>>> +   mutex_lock(&cdns->mutex);
>>> +   cdns->role = role;
>>> +   mutex_unlock(&cdns->mutex);
>>> +
>>> +   if (role == USB_ROLE_NONE)
>>> +   return 0;
>>
>> We will need to have a role driver for NONE case so we can deal with
>> type-C cable swap case. I will post a patch at the end about this.
>> You can squash the patches and use them in the next revision.
> 
>>
>>> +
>>> +   if (!cdns->roles[role])
>>> +   return -ENXIO;
>>> +
>>> +   if (cdns->roles[role]->state == CDNS3_ROLE_STATE_ACTIVE)
>>> +   return 0;
>>> +
>>> +   mutex_lock(&cdns->mutex);
>>> +   if (role == USB_ROLE_HOST)
>>> +   cdns3_drd_switch_host(cdns, 1);
>>> +   else
>>> +   cdns3_drd_switch_gadget(cdns, 1);
>>
>> Please don't do this here. You can call them from the respective role
>> drivers at the start. i.e. __cdns3_host_init(), __cdns3_gadget_init()
> 
> Why it can't do this here?
> 
Because it looks so wrong. You are dealing with state specific functionality
in a generic role start/stop function.

When you add idle state then it doesn't scale well.

> Do we really want to mix  drd code with gadget/host ?

I don't see it like that. You are only doing host/gadget

RE: [PATCH v10 5/6] usb:cdns3 Add Cadence USB3 DRD Driver

2019-08-19 Thread Pawel Laszczak
Hi,

>
>On 21/07/2019 21:32, Pawel Laszczak wrote:
>> This patch introduce new Cadence USBSS DRD driver to Linux kernel.
>>
>> The Cadence USBSS DRD Controller is a highly configurable IP Core which
>> can be instantiated as Dual-Role Device (DRD), Peripheral Only and
>> Host Only (XHCI)configurations.
>>
>> The current driver has been validated with FPGA platform. We have
>> support for PCIe bus, which is used on FPGA prototyping.
>>
>> The host side of USBSS-DRD controller is compliant with XHCI
>> specification, so it works with standard XHCI Linux driver.
>>
>> Signed-off-by: Pawel Laszczak 
>> ---
>>  drivers/usb/Kconfig|2 +
>>  drivers/usb/Makefile   |2 +
>>  drivers/usb/cdns3/Kconfig  |   46 +
>>  drivers/usb/cdns3/Makefile |   17 +
>>  drivers/usb/cdns3/cdns3-pci-wrap.c |  203 +++
>>  drivers/usb/cdns3/core.c   |  554 +++
>>  drivers/usb/cdns3/core.h   |  109 ++
>>  drivers/usb/cdns3/debug.h  |  171 ++
>>  drivers/usb/cdns3/debugfs.c|   87 ++
>>  drivers/usb/cdns3/drd.c|  390 +
>>  drivers/usb/cdns3/drd.h|  166 ++
>>  drivers/usb/cdns3/ep0.c|  914 +++
>>  drivers/usb/cdns3/gadget-export.h  |   28 +
>>  drivers/usb/cdns3/gadget.c | 2338 
>>  drivers/usb/cdns3/gadget.h | 1321 
>>  drivers/usb/cdns3/host-export.h|   28 +
>>  drivers/usb/cdns3/host.c   |   71 +
>>  drivers/usb/cdns3/trace.c  |   11 +
>>  drivers/usb/cdns3/trace.h  |  493 ++
>>  19 files changed, 6951 insertions(+)
>>  create mode 100644 drivers/usb/cdns3/Kconfig
>>  create mode 100644 drivers/usb/cdns3/Makefile
>>  create mode 100644 drivers/usb/cdns3/cdns3-pci-wrap.c
>>  create mode 100644 drivers/usb/cdns3/core.c
>>  create mode 100644 drivers/usb/cdns3/core.h
>>  create mode 100644 drivers/usb/cdns3/debug.h
>>  create mode 100644 drivers/usb/cdns3/debugfs.c
>>  create mode 100644 drivers/usb/cdns3/drd.c
>>  create mode 100644 drivers/usb/cdns3/drd.h
>>  create mode 100644 drivers/usb/cdns3/ep0.c
>>  create mode 100644 drivers/usb/cdns3/gadget-export.h
>>  create mode 100644 drivers/usb/cdns3/gadget.c
>>  create mode 100644 drivers/usb/cdns3/gadget.h
>>  create mode 100644 drivers/usb/cdns3/host-export.h
>>  create mode 100644 drivers/usb/cdns3/host.c
>>  create mode 100644 drivers/usb/cdns3/trace.c
>>  create mode 100644 drivers/usb/cdns3/trace.h
>>
>
>
>
>> diff --git a/drivers/usb/cdns3/core.c b/drivers/usb/cdns3/core.c
>> new file mode 100644
>> index ..900b2ce08162
>> --- /dev/null
>> +++ b/drivers/usb/cdns3/core.c
>> @@ -0,0 +1,554 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Cadence USBSS DRD Driver.
>> + *
>> + * Copyright (C) 2018-2019 Cadence.
>> + * Copyright (C) 2017-2018 NXP
>> + * Copyright (C) 2019 Texas Instruments
>> + *
>> + * Author: Peter Chen 
>> + * Pawel Laszczak 
>> + * Roger Quadros 
>> + */
>> +
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +
>> +#include "gadget.h"
>> +#include "core.h"
>> +#include "host-export.h"
>> +#include "gadget-export.h"
>> +#include "drd.h"
>> +#include "debug.h"
>> +
>> +static inline
>> +struct cdns3_role_driver *cdns3_get_current_role_driver(struct cdns3 *cdns)
>> +{
>> +WARN_ON(!cdns->roles[cdns->role]);
>> +return cdns->roles[cdns->role];
>> +}
>> +
>> +static int cdns3_role_start(struct cdns3 *cdns, enum usb_role role)
>> +{
>> +int ret;
>> +
>> +if (WARN_ON(role > USB_ROLE_DEVICE))
>> +return 0;
>> +
>> +mutex_lock(&cdns->mutex);
>> +cdns->role = role;
>> +mutex_unlock(&cdns->mutex);
>> +
>> +if (role == USB_ROLE_NONE)
>> +return 0;
>
>We will need to have a role driver for NONE case so we can deal with
>type-C cable swap case. I will post a patch at the end about this.
>You can squash the patches and use them in the next revision.

>
>> +
>> +if (!cdns->roles[role])
>> +return -ENXIO;
>> +
>> +if (cdns->roles[role]->state == CDNS3_ROLE_STATE_ACTIVE)
>> +return 0;
>> +
>> +mutex_lock(&cdns->mutex);
>> +if (role == USB_ROLE_HOST)
>> +cdns3_drd_switch_host(cdns, 1);
>> +else
>> +cdns3_drd_switch_gadget(cdns, 1);
>
>Please don't do this here. You can call them from the respective role
>drivers at the start. i.e. __cdns3_host_init(), __cdns3_gadget_init()

Why it can't do this here?

Do we really want to mix  drd code with gadget/host ?

>> +
>> +ret = cdns->roles[role]->start(cdns);
>> +if (!ret)
>> +cdns->roles[role]->state = CDNS3_ROLE_STATE_ACTIVE;
>> +mutex_unlock(&cdns->mutex);
>> +
>> +return ret;
>> +}
>> +
>> +static void cdns3_role_stop(struct cdns3 *cdns)
>> +{
>> +enum usb_role role = cdns->role;
>> +
>> +if (WARN_ON(role > USB_ROLE_DEVICE))
>> +return;
>> +
>> +if (role == USB

RE: [PATCH v10 5/6] usb:cdns3 Add Cadence USB3 DRD Driver

2019-08-19 Thread Pawel Laszczak
Hi Roger,

>
>On 21/07/2019 21:32, Pawel Laszczak wrote:
>> This patch introduce new Cadence USBSS DRD driver to Linux kernel.
>>
>> The Cadence USBSS DRD Controller is a highly configurable IP Core which
>> can be instantiated as Dual-Role Device (DRD), Peripheral Only and
>> Host Only (XHCI)configurations.
>>
>> The current driver has been validated with FPGA platform. We have
>> support for PCIe bus, which is used on FPGA prototyping.
>>
>> The host side of USBSS-DRD controller is compliant with XHCI
>> specification, so it works with standard XHCI Linux driver.
>>
>> Signed-off-by: Pawel Laszczak 
>> ---
>>  drivers/usb/Kconfig|2 +
>>  drivers/usb/Makefile   |2 +
>>  drivers/usb/cdns3/Kconfig  |   46 +
>>  drivers/usb/cdns3/Makefile |   17 +
>>  drivers/usb/cdns3/cdns3-pci-wrap.c |  203 +++
>>  drivers/usb/cdns3/core.c   |  554 +++
>>  drivers/usb/cdns3/core.h   |  109 ++
>>  drivers/usb/cdns3/debug.h  |  171 ++
>>  drivers/usb/cdns3/debugfs.c|   87 ++
>>  drivers/usb/cdns3/drd.c|  390 +
>>  drivers/usb/cdns3/drd.h|  166 ++
>>  drivers/usb/cdns3/ep0.c|  914 +++
>>  drivers/usb/cdns3/gadget-export.h  |   28 +
>>  drivers/usb/cdns3/gadget.c | 2338 
>>  drivers/usb/cdns3/gadget.h | 1321 
>>  drivers/usb/cdns3/host-export.h|   28 +
>>  drivers/usb/cdns3/host.c   |   71 +
>>  drivers/usb/cdns3/trace.c  |   11 +
>>  drivers/usb/cdns3/trace.h  |  493 ++
>>  19 files changed, 6951 insertions(+)
>>  create mode 100644 drivers/usb/cdns3/Kconfig
>>  create mode 100644 drivers/usb/cdns3/Makefile
>>  create mode 100644 drivers/usb/cdns3/cdns3-pci-wrap.c
>>  create mode 100644 drivers/usb/cdns3/core.c
>>  create mode 100644 drivers/usb/cdns3/core.h
>>  create mode 100644 drivers/usb/cdns3/debug.h
>>  create mode 100644 drivers/usb/cdns3/debugfs.c
>>  create mode 100644 drivers/usb/cdns3/drd.c
>>  create mode 100644 drivers/usb/cdns3/drd.h
>>  create mode 100644 drivers/usb/cdns3/ep0.c
>>  create mode 100644 drivers/usb/cdns3/gadget-export.h
>>  create mode 100644 drivers/usb/cdns3/gadget.c
>>  create mode 100644 drivers/usb/cdns3/gadget.h
>>  create mode 100644 drivers/usb/cdns3/host-export.h
>>  create mode 100644 drivers/usb/cdns3/host.c
>>  create mode 100644 drivers/usb/cdns3/trace.c
>>  create mode 100644 drivers/usb/cdns3/trace.h
>>
>
>
>
>It is getting really hard to review this patch as it is so large.
>I would still suggest to split host/gadget/drd if possible.
>Felipe, any objections?
>
>> diff --git a/drivers/usb/cdns3/gadget.c b/drivers/usb/cdns3/gadget.c
>> new file mode 100644
>> index ..291f08be56fe
>> --- /dev/null
>> +++ b/drivers/usb/cdns3/gadget.c
>> @@ -0,0 +1,2338 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Cadence USBSS DRD Driver - gadget side.
>> + *
>> + * Copyright (C) 2018-2019 Cadence Design Systems.
>> + * Copyright (C) 2017-2018 NXP
>> + *
>> + * Authors: Pawel Jez ,
>> + *  Pawel Laszczak 
>> + *  Peter Chen 
>> + */
>> +
>
>
>
>> diff --git a/drivers/usb/cdns3/gadget.h b/drivers/usb/cdns3/gadget.h
>> new file mode 100644
>> index ..2a97c16aea4b
>> --- /dev/null
>> +++ b/drivers/usb/cdns3/gadget.h
>> @@ -0,0 +1,1321 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +/*
>> + * USBSS device controller driver header file
>> + *
>> + * Copyright (C) 2018-2019 Cadence.
>> + * Copyright (C) 2017-2018 NXP
>> + *
>> + * Author: Pawel Laszczak 
>> + * Pawel Jez 
>> + * Peter Chen 
>> + */
>> +#ifndef __LINUX_CDNS3_GADGET
>> +#define __LINUX_CDNS3_GADGET
>> +#include 
>> +
>> +/*
>> + * USBSS-DEV register interface.
>> + * This corresponds to the USBSS Device Controller Interface
>> + */
>> +
>> +/**
>> + * struct cdns3_usb_regs - device controller registers.
>> + * @usb_conf:  Global Configuration Register.
>> + * @usb_sts:   Global Status Register.
>> + * @usb_cmd:   Global Command Register.
>> + * @usb_itpn:  ITP/SOF number Register.
>> + * @usb_lpm:   Global Command Register.
>> + * @usb_ien:   USB Interrupt Enable Register.
>> + * @usb_ists:  USB Interrupt Status Register.
>> + * @ep_sel:Endpoint Select Register.
>> + * @ep_traddr: Endpoint Transfer Ring Address Register.
>> + * @ep_cfg:Endpoint Configuration Register.
>> + * @ep_cmd:Endpoint Command Register.
>> + * @ep_sts:Endpoint Status Register.
>> + * @ep_sts_sid:Endpoint Status Register.
>> + * @ep_sts_en: Endpoint Status Register Enable.
>> + * @drbl:  Doorbell Register.
>> + * @ep_ien:EP Interrupt Enable Register.
>> + * @ep_ists:   EP Interrupt Status Register.
>> + * @usb_pwr:   Global Power Configuration Register.
>> + * @usb_conf2: Global Configuration Register 2.
>> + * @usb_cap1:  Capability Register 1.
>> + * @usb_cap2:  Capabil

Re: [PATCH v10 5/6] usb:cdns3 Add Cadence USB3 DRD Driver

2019-08-15 Thread Roger Quadros
Pawel,

On 21/07/2019 21:32, Pawel Laszczak wrote:
> This patch introduce new Cadence USBSS DRD driver to Linux kernel.
> 
> The Cadence USBSS DRD Controller is a highly configurable IP Core which
> can be instantiated as Dual-Role Device (DRD), Peripheral Only and
> Host Only (XHCI)configurations.
> 
> The current driver has been validated with FPGA platform. We have
> support for PCIe bus, which is used on FPGA prototyping.
> 
> The host side of USBSS-DRD controller is compliant with XHCI
> specification, so it works with standard XHCI Linux driver.
> 
> Signed-off-by: Pawel Laszczak 
> ---
>  drivers/usb/Kconfig|2 +
>  drivers/usb/Makefile   |2 +
>  drivers/usb/cdns3/Kconfig  |   46 +
>  drivers/usb/cdns3/Makefile |   17 +
>  drivers/usb/cdns3/cdns3-pci-wrap.c |  203 +++
>  drivers/usb/cdns3/core.c   |  554 +++
>  drivers/usb/cdns3/core.h   |  109 ++
>  drivers/usb/cdns3/debug.h  |  171 ++
>  drivers/usb/cdns3/debugfs.c|   87 ++
>  drivers/usb/cdns3/drd.c|  390 +
>  drivers/usb/cdns3/drd.h|  166 ++
>  drivers/usb/cdns3/ep0.c|  914 +++
>  drivers/usb/cdns3/gadget-export.h  |   28 +
>  drivers/usb/cdns3/gadget.c | 2338 
>  drivers/usb/cdns3/gadget.h | 1321 
>  drivers/usb/cdns3/host-export.h|   28 +
>  drivers/usb/cdns3/host.c   |   71 +
>  drivers/usb/cdns3/trace.c  |   11 +
>  drivers/usb/cdns3/trace.h  |  493 ++
>  19 files changed, 6951 insertions(+)
>  create mode 100644 drivers/usb/cdns3/Kconfig
>  create mode 100644 drivers/usb/cdns3/Makefile
>  create mode 100644 drivers/usb/cdns3/cdns3-pci-wrap.c
>  create mode 100644 drivers/usb/cdns3/core.c
>  create mode 100644 drivers/usb/cdns3/core.h
>  create mode 100644 drivers/usb/cdns3/debug.h
>  create mode 100644 drivers/usb/cdns3/debugfs.c
>  create mode 100644 drivers/usb/cdns3/drd.c
>  create mode 100644 drivers/usb/cdns3/drd.h
>  create mode 100644 drivers/usb/cdns3/ep0.c
>  create mode 100644 drivers/usb/cdns3/gadget-export.h
>  create mode 100644 drivers/usb/cdns3/gadget.c
>  create mode 100644 drivers/usb/cdns3/gadget.h
>  create mode 100644 drivers/usb/cdns3/host-export.h
>  create mode 100644 drivers/usb/cdns3/host.c
>  create mode 100644 drivers/usb/cdns3/trace.c
>  create mode 100644 drivers/usb/cdns3/trace.h
> 



> diff --git a/drivers/usb/cdns3/core.c b/drivers/usb/cdns3/core.c
> new file mode 100644
> index ..900b2ce08162
> --- /dev/null
> +++ b/drivers/usb/cdns3/core.c
> @@ -0,0 +1,554 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Cadence USBSS DRD Driver.
> + *
> + * Copyright (C) 2018-2019 Cadence.
> + * Copyright (C) 2017-2018 NXP
> + * Copyright (C) 2019 Texas Instruments
> + *
> + * Author: Peter Chen 
> + * Pawel Laszczak 
> + * Roger Quadros 
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include "gadget.h"
> +#include "core.h"
> +#include "host-export.h"
> +#include "gadget-export.h"
> +#include "drd.h"
> +#include "debug.h"
> +
> +static inline
> +struct cdns3_role_driver *cdns3_get_current_role_driver(struct cdns3 *cdns)
> +{
> + WARN_ON(!cdns->roles[cdns->role]);
> + return cdns->roles[cdns->role];
> +}
> +
> +static int cdns3_role_start(struct cdns3 *cdns, enum usb_role role)
> +{
> + int ret;
> +
> + if (WARN_ON(role > USB_ROLE_DEVICE))
> + return 0;
> +
> + mutex_lock(&cdns->mutex);
> + cdns->role = role;
> + mutex_unlock(&cdns->mutex);
> +
> + if (role == USB_ROLE_NONE)
> + return 0;

We will need to have a role driver for NONE case so we can deal with
type-C cable swap case. I will post a patch at the end about this.
You can squash the patches and use them in the next revision.

> +
> + if (!cdns->roles[role])
> + return -ENXIO;
> +
> + if (cdns->roles[role]->state == CDNS3_ROLE_STATE_ACTIVE)
> + return 0;
> +
> + mutex_lock(&cdns->mutex);
> + if (role == USB_ROLE_HOST)
> + cdns3_drd_switch_host(cdns, 1);
> + else
> + cdns3_drd_switch_gadget(cdns, 1);

Please don't do this here. You can call them from the respective role
drivers at the start. i.e. __cdns3_host_init(), __cdns3_gadget_init()
> +
> + ret = cdns->roles[role]->start(cdns);
> + if (!ret)
> + cdns->roles[role]->state = CDNS3_ROLE_STATE_ACTIVE;
> + mutex_unlock(&cdns->mutex);
> +
> + return ret;
> +}
> +
> +static void cdns3_role_stop(struct cdns3 *cdns)
> +{
> + enum usb_role role = cdns->role;
> +
> + if (WARN_ON(role > USB_ROLE_DEVICE))
> + return;
> +
> + if (role == USB_ROLE_NONE)
> + return;
> +
> + if (cdns->roles[role]->state == CDNS3_ROLE_STATE_INACTIVE)
> + return;
> +
> + mutex_lock(&cdns->mutex);
> + cdns->roles[role]->st

Re: [PATCH v10 5/6] usb:cdns3 Add Cadence USB3 DRD Driver

2019-08-14 Thread Felipe Balbi


Hi,

Roger Quadros  writes:

> On 21/07/2019 21:32, Pawel Laszczak wrote:
>> This patch introduce new Cadence USBSS DRD driver to Linux kernel.
>> 
>> The Cadence USBSS DRD Controller is a highly configurable IP Core which
>> can be instantiated as Dual-Role Device (DRD), Peripheral Only and
>> Host Only (XHCI)configurations.
>> 
>> The current driver has been validated with FPGA platform. We have
>> support for PCIe bus, which is used on FPGA prototyping.
>> 
>> The host side of USBSS-DRD controller is compliant with XHCI
>> specification, so it works with standard XHCI Linux driver.
>> 
>> Signed-off-by: Pawel Laszczak 
>> ---
>>  drivers/usb/Kconfig|2 +
>>  drivers/usb/Makefile   |2 +
>>  drivers/usb/cdns3/Kconfig  |   46 +
>>  drivers/usb/cdns3/Makefile |   17 +
>>  drivers/usb/cdns3/cdns3-pci-wrap.c |  203 +++
>>  drivers/usb/cdns3/core.c   |  554 +++
>>  drivers/usb/cdns3/core.h   |  109 ++
>>  drivers/usb/cdns3/debug.h  |  171 ++
>>  drivers/usb/cdns3/debugfs.c|   87 ++
>>  drivers/usb/cdns3/drd.c|  390 +
>>  drivers/usb/cdns3/drd.h|  166 ++
>>  drivers/usb/cdns3/ep0.c|  914 +++
>>  drivers/usb/cdns3/gadget-export.h  |   28 +
>>  drivers/usb/cdns3/gadget.c | 2338 
>>  drivers/usb/cdns3/gadget.h | 1321 
>>  drivers/usb/cdns3/host-export.h|   28 +
>>  drivers/usb/cdns3/host.c   |   71 +
>>  drivers/usb/cdns3/trace.c  |   11 +
>>  drivers/usb/cdns3/trace.h  |  493 ++
>>  19 files changed, 6951 insertions(+)
>>  create mode 100644 drivers/usb/cdns3/Kconfig
>>  create mode 100644 drivers/usb/cdns3/Makefile
>>  create mode 100644 drivers/usb/cdns3/cdns3-pci-wrap.c
>>  create mode 100644 drivers/usb/cdns3/core.c
>>  create mode 100644 drivers/usb/cdns3/core.h
>>  create mode 100644 drivers/usb/cdns3/debug.h
>>  create mode 100644 drivers/usb/cdns3/debugfs.c
>>  create mode 100644 drivers/usb/cdns3/drd.c
>>  create mode 100644 drivers/usb/cdns3/drd.h
>>  create mode 100644 drivers/usb/cdns3/ep0.c
>>  create mode 100644 drivers/usb/cdns3/gadget-export.h
>>  create mode 100644 drivers/usb/cdns3/gadget.c
>>  create mode 100644 drivers/usb/cdns3/gadget.h
>>  create mode 100644 drivers/usb/cdns3/host-export.h
>>  create mode 100644 drivers/usb/cdns3/host.c
>>  create mode 100644 drivers/usb/cdns3/trace.c
>>  create mode 100644 drivers/usb/cdns3/trace.h
>> 
>
> 
>
> It is getting really hard to review this patch as it is so large.
> I would still suggest to split host/gadget/drd if possible.
> Felipe, any objections?

Maybe you guys can do a few rounds off-list then?

-- 
balbi


RE: [PATCH v10 5/6] usb:cdns3 Add Cadence USB3 DRD Driver

2019-08-14 Thread Pawel Laszczak
Hi Roger,

>
>On 21/07/2019 21:32, Pawel Laszczak wrote:
>> This patch introduce new Cadence USBSS DRD driver to Linux kernel.
>>
>> The Cadence USBSS DRD Controller is a highly configurable IP Core which
>> can be instantiated as Dual-Role Device (DRD), Peripheral Only and
>> Host Only (XHCI)configurations.
>>
>> The current driver has been validated with FPGA platform. We have
>> support for PCIe bus, which is used on FPGA prototyping.
>>
>> The host side of USBSS-DRD controller is compliant with XHCI
>> specification, so it works with standard XHCI Linux driver.
>>
>> Signed-off-by: Pawel Laszczak 
>> ---
>>  drivers/usb/Kconfig|2 +
>>  drivers/usb/Makefile   |2 +
>>  drivers/usb/cdns3/Kconfig  |   46 +
>>  drivers/usb/cdns3/Makefile |   17 +
>>  drivers/usb/cdns3/cdns3-pci-wrap.c |  203 +++
>>  drivers/usb/cdns3/core.c   |  554 +++
>>  drivers/usb/cdns3/core.h   |  109 ++
>>  drivers/usb/cdns3/debug.h  |  171 ++
>>  drivers/usb/cdns3/debugfs.c|   87 ++
>>  drivers/usb/cdns3/drd.c|  390 +
>>  drivers/usb/cdns3/drd.h|  166 ++
>>  drivers/usb/cdns3/ep0.c|  914 +++
>>  drivers/usb/cdns3/gadget-export.h  |   28 +
>>  drivers/usb/cdns3/gadget.c | 2338 
>>  drivers/usb/cdns3/gadget.h | 1321 
>>  drivers/usb/cdns3/host-export.h|   28 +
>>  drivers/usb/cdns3/host.c   |   71 +
>>  drivers/usb/cdns3/trace.c  |   11 +
>>  drivers/usb/cdns3/trace.h  |  493 ++
>>  19 files changed, 6951 insertions(+)
>>  create mode 100644 drivers/usb/cdns3/Kconfig
>>  create mode 100644 drivers/usb/cdns3/Makefile
>>  create mode 100644 drivers/usb/cdns3/cdns3-pci-wrap.c
>>  create mode 100644 drivers/usb/cdns3/core.c
>>  create mode 100644 drivers/usb/cdns3/core.h
>>  create mode 100644 drivers/usb/cdns3/debug.h
>>  create mode 100644 drivers/usb/cdns3/debugfs.c
>>  create mode 100644 drivers/usb/cdns3/drd.c
>>  create mode 100644 drivers/usb/cdns3/drd.h
>>  create mode 100644 drivers/usb/cdns3/ep0.c
>>  create mode 100644 drivers/usb/cdns3/gadget-export.h
>>  create mode 100644 drivers/usb/cdns3/gadget.c
>>  create mode 100644 drivers/usb/cdns3/gadget.h
>>  create mode 100644 drivers/usb/cdns3/host-export.h
>>  create mode 100644 drivers/usb/cdns3/host.c
>>  create mode 100644 drivers/usb/cdns3/trace.c
>>  create mode 100644 drivers/usb/cdns3/trace.h
>>
>
>
>
>> diff --git a/drivers/usb/cdns3/gadget.c b/drivers/usb/cdns3/gadget.c
>> new file mode 100644
>> index ..291f08be56fe
>> --- /dev/null
>> +++ b/drivers/usb/cdns3/gadget.c
>> @@ -0,0 +1,2338 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Cadence USBSS DRD Driver - gadget side.
>> + *
>> + * Copyright (C) 2018-2019 Cadence Design Systems.
>> + * Copyright (C) 2017-2018 NXP
>> + *
>> + * Authors: Pawel Jez ,
>> + *  Pawel Laszczak 
>> + *  Peter Chen 
>> + */
>> +
>
>
>
>> +
>> +static void cdns3_gadget_config(struct cdns3_device *priv_dev)
>> +{
>> +struct cdns3_usb_regs __iomem *regs = priv_dev->regs;
>> +u32 reg;
>> +
>> +cdns3_ep0_config(priv_dev);
>> +
>> +/* enable interrupts for endpoint 0 (in and out) */
>> +writel(EP_IEN_EP_OUT0 | EP_IEN_EP_IN0, ®s->ep_ien);
>> +
>> +/*
>> + * Driver needs to modify LFPS minimal U1 Exit time for DEV_VER_TI_V1
>> + * revision of controller.
>> + */
>> +if (priv_dev->dev_ver == DEV_VER_TI_V1) {
>> +reg = readl(®s->dbg_link1);
>> +
>> +reg &= ~DBG_LINK1_LFPS_MIN_GEN_U1_EXIT_MASK;
>> +reg |= DBG_LINK1_LFPS_MIN_GEN_U1_EXIT(0x55) |
>> +   DBG_LINK1_LFPS_MIN_GEN_U1_EXIT_SET;
>> +writel(reg, ®s->dbg_link1);
>> +}
>> +
>> +/*
>> + * By default some platforms has set protected access to memory.
>> + * This cause problem with cache, so driver restore non-secure
>> + * access to memory.
>> + */
>> +reg = readl(®s->dma_axi_ctrl);
>
>Why read the reg at all if you are just overwriting it below?
>
>> +reg = DMA_AXI_CTRL_MARPROT(DMA_AXI_CTRL_NON_SECURE) |
>> +  DMA_AXI_CTRL_MAWPROT(DMA_AXI_CTRL_NON_SECURE);
>
>
>Otherwise you need to read modify only necessary bits and then write.

Yes, we also has found this bug. 
It's will be corrected in next version.

>i.e.
>   #define DMA_AXI_CTRL_MAPROT_MASK 0x3
>   reg &= ~(DMA_AXI_CTRL_MARPROT(DMA_AXI_CTRL_MAPROT_MASK) |
>DMA_AXI_CTRL_MARPROT(DMA_AXI_CTRL_MAPROT_MASK))
>   reg |= DMA_AXI_CTRL_MARPROT(DMA_AXI_CTRL_NON_SECURE) |
>  DMA_AXI_CTRL_MAWPROT(DMA_AXI_CTRL_NON_SECURE);
>
>> +writel(reg, ®s->dma_axi_ctrl);
>> +
>> +/* enable generic interrupt*/
>> +writel(USB_IEN_INIT, ®s->usb_ien);
>> +writel(USB_CONF_CLK2OFFDS | USB_CONF_L1DS, ®s->usb_conf);
>> +
>> +cdns3_configure_dmult(priv_dev, NULL);
>> +
>> +cdns3_gadget_pullup(&priv_dev->gadget, 

Re: [PATCH v10 5/6] usb:cdns3 Add Cadence USB3 DRD Driver

2019-08-14 Thread Roger Quadros



On 21/07/2019 21:32, Pawel Laszczak wrote:
> This patch introduce new Cadence USBSS DRD driver to Linux kernel.
> 
> The Cadence USBSS DRD Controller is a highly configurable IP Core which
> can be instantiated as Dual-Role Device (DRD), Peripheral Only and
> Host Only (XHCI)configurations.
> 
> The current driver has been validated with FPGA platform. We have
> support for PCIe bus, which is used on FPGA prototyping.
> 
> The host side of USBSS-DRD controller is compliant with XHCI
> specification, so it works with standard XHCI Linux driver.
> 
> Signed-off-by: Pawel Laszczak 
> ---
>  drivers/usb/Kconfig|2 +
>  drivers/usb/Makefile   |2 +
>  drivers/usb/cdns3/Kconfig  |   46 +
>  drivers/usb/cdns3/Makefile |   17 +
>  drivers/usb/cdns3/cdns3-pci-wrap.c |  203 +++
>  drivers/usb/cdns3/core.c   |  554 +++
>  drivers/usb/cdns3/core.h   |  109 ++
>  drivers/usb/cdns3/debug.h  |  171 ++
>  drivers/usb/cdns3/debugfs.c|   87 ++
>  drivers/usb/cdns3/drd.c|  390 +
>  drivers/usb/cdns3/drd.h|  166 ++
>  drivers/usb/cdns3/ep0.c|  914 +++
>  drivers/usb/cdns3/gadget-export.h  |   28 +
>  drivers/usb/cdns3/gadget.c | 2338 
>  drivers/usb/cdns3/gadget.h | 1321 
>  drivers/usb/cdns3/host-export.h|   28 +
>  drivers/usb/cdns3/host.c   |   71 +
>  drivers/usb/cdns3/trace.c  |   11 +
>  drivers/usb/cdns3/trace.h  |  493 ++
>  19 files changed, 6951 insertions(+)
>  create mode 100644 drivers/usb/cdns3/Kconfig
>  create mode 100644 drivers/usb/cdns3/Makefile
>  create mode 100644 drivers/usb/cdns3/cdns3-pci-wrap.c
>  create mode 100644 drivers/usb/cdns3/core.c
>  create mode 100644 drivers/usb/cdns3/core.h
>  create mode 100644 drivers/usb/cdns3/debug.h
>  create mode 100644 drivers/usb/cdns3/debugfs.c
>  create mode 100644 drivers/usb/cdns3/drd.c
>  create mode 100644 drivers/usb/cdns3/drd.h
>  create mode 100644 drivers/usb/cdns3/ep0.c
>  create mode 100644 drivers/usb/cdns3/gadget-export.h
>  create mode 100644 drivers/usb/cdns3/gadget.c
>  create mode 100644 drivers/usb/cdns3/gadget.h
>  create mode 100644 drivers/usb/cdns3/host-export.h
>  create mode 100644 drivers/usb/cdns3/host.c
>  create mode 100644 drivers/usb/cdns3/trace.c
>  create mode 100644 drivers/usb/cdns3/trace.h
> 



> diff --git a/drivers/usb/cdns3/gadget.c b/drivers/usb/cdns3/gadget.c
> new file mode 100644
> index ..291f08be56fe
> --- /dev/null
> +++ b/drivers/usb/cdns3/gadget.c
> @@ -0,0 +1,2338 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Cadence USBSS DRD Driver - gadget side.
> + *
> + * Copyright (C) 2018-2019 Cadence Design Systems.
> + * Copyright (C) 2017-2018 NXP
> + *
> + * Authors: Pawel Jez ,
> + *  Pawel Laszczak 
> + *  Peter Chen 
> + */
> +



> +
> +static void cdns3_gadget_config(struct cdns3_device *priv_dev)
> +{
> + struct cdns3_usb_regs __iomem *regs = priv_dev->regs;
> + u32 reg;
> +
> + cdns3_ep0_config(priv_dev);
> +
> + /* enable interrupts for endpoint 0 (in and out) */
> + writel(EP_IEN_EP_OUT0 | EP_IEN_EP_IN0, ®s->ep_ien);
> +
> + /*
> +  * Driver needs to modify LFPS minimal U1 Exit time for DEV_VER_TI_V1
> +  * revision of controller.
> +  */
> + if (priv_dev->dev_ver == DEV_VER_TI_V1) {
> + reg = readl(®s->dbg_link1);
> +
> + reg &= ~DBG_LINK1_LFPS_MIN_GEN_U1_EXIT_MASK;
> + reg |= DBG_LINK1_LFPS_MIN_GEN_U1_EXIT(0x55) |
> +DBG_LINK1_LFPS_MIN_GEN_U1_EXIT_SET;
> + writel(reg, ®s->dbg_link1);
> + }
> +
> + /*
> +  * By default some platforms has set protected access to memory.
> +  * This cause problem with cache, so driver restore non-secure
> +  * access to memory.
> +  */
> + reg = readl(®s->dma_axi_ctrl);

Why read the reg at all if you are just overwriting it below?

> + reg = DMA_AXI_CTRL_MARPROT(DMA_AXI_CTRL_NON_SECURE) |
> +   DMA_AXI_CTRL_MAWPROT(DMA_AXI_CTRL_NON_SECURE);


Otherwise you need to read modify only necessary bits and then write.
i.e.
#define DMA_AXI_CTRL_MAPROT_MASK 0x3
reg &= ~(DMA_AXI_CTRL_MARPROT(DMA_AXI_CTRL_MAPROT_MASK) |
 DMA_AXI_CTRL_MARPROT(DMA_AXI_CTRL_MAPROT_MASK))
reg |= DMA_AXI_CTRL_MARPROT(DMA_AXI_CTRL_NON_SECURE) |
   DMA_AXI_CTRL_MAWPROT(DMA_AXI_CTRL_NON_SECURE);

> + writel(reg, ®s->dma_axi_ctrl);
> +
> + /* enable generic interrupt*/
> + writel(USB_IEN_INIT, ®s->usb_ien);
> + writel(USB_CONF_CLK2OFFDS | USB_CONF_L1DS, ®s->usb_conf);
> +
> + cdns3_configure_dmult(priv_dev, NULL);
> +
> + cdns3_gadget_pullup(&priv_dev->gadget, 1);
> +}
> +



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


Re: [PATCH v10 5/6] usb:cdns3 Add Cadence USB3 DRD Driver

2019-08-14 Thread Roger Quadros



On 21/07/2019 21:32, Pawel Laszczak wrote:
> This patch introduce new Cadence USBSS DRD driver to Linux kernel.
> 
> The Cadence USBSS DRD Controller is a highly configurable IP Core which
> can be instantiated as Dual-Role Device (DRD), Peripheral Only and
> Host Only (XHCI)configurations.
> 
> The current driver has been validated with FPGA platform. We have
> support for PCIe bus, which is used on FPGA prototyping.
> 
> The host side of USBSS-DRD controller is compliant with XHCI
> specification, so it works with standard XHCI Linux driver.
> 
> Signed-off-by: Pawel Laszczak 
> ---
>  drivers/usb/Kconfig|2 +
>  drivers/usb/Makefile   |2 +
>  drivers/usb/cdns3/Kconfig  |   46 +
>  drivers/usb/cdns3/Makefile |   17 +
>  drivers/usb/cdns3/cdns3-pci-wrap.c |  203 +++
>  drivers/usb/cdns3/core.c   |  554 +++
>  drivers/usb/cdns3/core.h   |  109 ++
>  drivers/usb/cdns3/debug.h  |  171 ++
>  drivers/usb/cdns3/debugfs.c|   87 ++
>  drivers/usb/cdns3/drd.c|  390 +
>  drivers/usb/cdns3/drd.h|  166 ++
>  drivers/usb/cdns3/ep0.c|  914 +++
>  drivers/usb/cdns3/gadget-export.h  |   28 +
>  drivers/usb/cdns3/gadget.c | 2338 
>  drivers/usb/cdns3/gadget.h | 1321 
>  drivers/usb/cdns3/host-export.h|   28 +
>  drivers/usb/cdns3/host.c   |   71 +
>  drivers/usb/cdns3/trace.c  |   11 +
>  drivers/usb/cdns3/trace.h  |  493 ++
>  19 files changed, 6951 insertions(+)
>  create mode 100644 drivers/usb/cdns3/Kconfig
>  create mode 100644 drivers/usb/cdns3/Makefile
>  create mode 100644 drivers/usb/cdns3/cdns3-pci-wrap.c
>  create mode 100644 drivers/usb/cdns3/core.c
>  create mode 100644 drivers/usb/cdns3/core.h
>  create mode 100644 drivers/usb/cdns3/debug.h
>  create mode 100644 drivers/usb/cdns3/debugfs.c
>  create mode 100644 drivers/usb/cdns3/drd.c
>  create mode 100644 drivers/usb/cdns3/drd.h
>  create mode 100644 drivers/usb/cdns3/ep0.c
>  create mode 100644 drivers/usb/cdns3/gadget-export.h
>  create mode 100644 drivers/usb/cdns3/gadget.c
>  create mode 100644 drivers/usb/cdns3/gadget.h
>  create mode 100644 drivers/usb/cdns3/host-export.h
>  create mode 100644 drivers/usb/cdns3/host.c
>  create mode 100644 drivers/usb/cdns3/trace.c
>  create mode 100644 drivers/usb/cdns3/trace.h
> 



It is getting really hard to review this patch as it is so large.
I would still suggest to split host/gadget/drd if possible.
Felipe, any objections?

> diff --git a/drivers/usb/cdns3/gadget.c b/drivers/usb/cdns3/gadget.c
> new file mode 100644
> index ..291f08be56fe
> --- /dev/null
> +++ b/drivers/usb/cdns3/gadget.c
> @@ -0,0 +1,2338 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Cadence USBSS DRD Driver - gadget side.
> + *
> + * Copyright (C) 2018-2019 Cadence Design Systems.
> + * Copyright (C) 2017-2018 NXP
> + *
> + * Authors: Pawel Jez ,
> + *  Pawel Laszczak 
> + *  Peter Chen 
> + */
> +



> diff --git a/drivers/usb/cdns3/gadget.h b/drivers/usb/cdns3/gadget.h
> new file mode 100644
> index ..2a97c16aea4b
> --- /dev/null
> +++ b/drivers/usb/cdns3/gadget.h
> @@ -0,0 +1,1321 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * USBSS device controller driver header file
> + *
> + * Copyright (C) 2018-2019 Cadence.
> + * Copyright (C) 2017-2018 NXP
> + *
> + * Author: Pawel Laszczak 
> + * Pawel Jez 
> + * Peter Chen 
> + */
> +#ifndef __LINUX_CDNS3_GADGET
> +#define __LINUX_CDNS3_GADGET
> +#include 
> +
> +/*
> + * USBSS-DEV register interface.
> + * This corresponds to the USBSS Device Controller Interface
> + */
> +
> +/**
> + * struct cdns3_usb_regs - device controller registers.
> + * @usb_conf:  Global Configuration Register.
> + * @usb_sts:   Global Status Register.
> + * @usb_cmd:   Global Command Register.
> + * @usb_itpn:  ITP/SOF number Register.
> + * @usb_lpm:   Global Command Register.
> + * @usb_ien:   USB Interrupt Enable Register.
> + * @usb_ists:  USB Interrupt Status Register.
> + * @ep_sel:Endpoint Select Register.
> + * @ep_traddr: Endpoint Transfer Ring Address Register.
> + * @ep_cfg:Endpoint Configuration Register.
> + * @ep_cmd:Endpoint Command Register.
> + * @ep_sts:Endpoint Status Register.
> + * @ep_sts_sid:Endpoint Status Register.
> + * @ep_sts_en: Endpoint Status Register Enable.
> + * @drbl:  Doorbell Register.
> + * @ep_ien:EP Interrupt Enable Register.
> + * @ep_ists:   EP Interrupt Status Register.
> + * @usb_pwr:   Global Power Configuration Register.
> + * @usb_conf2: Global Configuration Register 2.
> + * @usb_cap1:  Capability Register 1.
> + * @usb_cap2:  Capability Register 2.
> + * @usb_cap3:  Capability Register 3.
> + * @usb_cap4:  Capability Register 4.
> + * @usb_cap5:  Capabil

Re: [PATCH v10 5/6] usb:cdns3 Add Cadence USB3 DRD Driver

2019-08-13 Thread Chunfeng Yun
On Tue, 2019-08-13 at 10:48 +0300, Roger Quadros wrote:
> 
> On 13/08/2019 10:30, Chunfeng Yun wrote:
> > On Mon, 2019-08-12 at 16:04 +0300, Roger Quadros wrote:
> >>
> >> On 12/08/2019 15:46, Felipe Balbi wrote:
> >>>
> >>> Hi,
> >>>
> >>> Roger Quadros  writes:
> > The sysfs file we expose from the class for the role switches is
> > primarily meant for supporting proprietary protocols that require us
> > to basically override the connector USB data role. The default role
> > should always be selected in the drivers.
> 
>  OK. Let's take this example
>  - Port is dual-role port micro AB.
>  - microAB to type-A adapter is connected which pulls ID low. port 
>  transitions
>  to "host" role by the controller driver.
>  - proprietary protocol want to switch role to device role so writes 
>  "device" to
>  mode switch sysfs. port transitions to "device" role.
> 
>  Now, how does controller driver know to fall back to HW based role 
>  switching?
> >>>
> >>> Use a 'disconnect' or 'suspend' event to go reset it? But that should,
> >>> probably, be done at kernel space, no?
> >>>
> >>
> >> Yes that could be one option.
> >> So after a disconnect, sysfs role should reflect actual hardware role. 
> >> correct?
> > 
> > Maybe it's difficult to support both HW based role switch and SW based
> > role switch by sysfs at the same if the HW's FSM rely on, such as, the
> > state of Vbus pin or ID pin. Likes the upper example, when user writes
> > "device" to mode switch sysfs, the driver should skip the HW state of ID
> > pin, due to it's state is Low, or force it as High.
> > 
> 
> We do need a clear way of indicating that SW wants to override so HW
> state is ignored.
> 
> > Another option way is that introduces a property in DTS to indicate the
> > way the driver want to use (HW based or SW based, usb_role_switch
> > doesn't provide this information for the controller driver), but is not
> > flexible enough.
> 
> That is not good enough for us. We need both HW and SW based role switching.
> 
> Can we introduce a new state (e.g. "auto") in usb_role_switch. This would
> explicitly indicate the driver to do HW based switching.
But "auto" is not a role?

How about introducing a new attribute in usb_role_switch?

> 
> This way we don't need to depend on connect/disconnect events and can
> do role switch tests even without cable/device connected.
> 




Re: [PATCH v10 5/6] usb:cdns3 Add Cadence USB3 DRD Driver

2019-08-13 Thread Roger Quadros



On 13/08/2019 10:30, Chunfeng Yun wrote:
> On Mon, 2019-08-12 at 16:04 +0300, Roger Quadros wrote:
>>
>> On 12/08/2019 15:46, Felipe Balbi wrote:
>>>
>>> Hi,
>>>
>>> Roger Quadros  writes:
> The sysfs file we expose from the class for the role switches is
> primarily meant for supporting proprietary protocols that require us
> to basically override the connector USB data role. The default role
> should always be selected in the drivers.

 OK. Let's take this example
 - Port is dual-role port micro AB.
 - microAB to type-A adapter is connected which pulls ID low. port 
 transitions
 to "host" role by the controller driver.
 - proprietary protocol want to switch role to device role so writes 
 "device" to
 mode switch sysfs. port transitions to "device" role.

 Now, how does controller driver know to fall back to HW based role 
 switching?
>>>
>>> Use a 'disconnect' or 'suspend' event to go reset it? But that should,
>>> probably, be done at kernel space, no?
>>>
>>
>> Yes that could be one option.
>> So after a disconnect, sysfs role should reflect actual hardware role. 
>> correct?
> 
> Maybe it's difficult to support both HW based role switch and SW based
> role switch by sysfs at the same if the HW's FSM rely on, such as, the
> state of Vbus pin or ID pin. Likes the upper example, when user writes
> "device" to mode switch sysfs, the driver should skip the HW state of ID
> pin, due to it's state is Low, or force it as High.
> 

We do need a clear way of indicating that SW wants to override so HW
state is ignored.

> Another option way is that introduces a property in DTS to indicate the
> way the driver want to use (HW based or SW based, usb_role_switch
> doesn't provide this information for the controller driver), but is not
> flexible enough.

That is not good enough for us. We need both HW and SW based role switching.

Can we introduce a new state (e.g. "auto") in usb_role_switch. This would
explicitly indicate the driver to do HW based switching.

This way we don't need to depend on connect/disconnect events and can
do role switch tests even without cable/device connected.

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


Re: [PATCH v10 5/6] usb:cdns3 Add Cadence USB3 DRD Driver

2019-08-13 Thread Chunfeng Yun
On Mon, 2019-08-12 at 16:04 +0300, Roger Quadros wrote:
> 
> On 12/08/2019 15:46, Felipe Balbi wrote:
> > 
> > Hi,
> > 
> > Roger Quadros  writes:
> >>> The sysfs file we expose from the class for the role switches is
> >>> primarily meant for supporting proprietary protocols that require us
> >>> to basically override the connector USB data role. The default role
> >>> should always be selected in the drivers.
> >>
> >> OK. Let's take this example
> >> - Port is dual-role port micro AB.
> >> - microAB to type-A adapter is connected which pulls ID low. port 
> >> transitions
> >> to "host" role by the controller driver.
> >> - proprietary protocol want to switch role to device role so writes 
> >> "device" to
> >> mode switch sysfs. port transitions to "device" role.
> >>
> >> Now, how does controller driver know to fall back to HW based role 
> >> switching?
> > 
> > Use a 'disconnect' or 'suspend' event to go reset it? But that should,
> > probably, be done at kernel space, no?
> > 
> 
> Yes that could be one option.
> So after a disconnect, sysfs role should reflect actual hardware role. 
> correct?

Maybe it's difficult to support both HW based role switch and SW based
role switch by sysfs at the same if the HW's FSM rely on, such as, the
state of Vbus pin or ID pin. Likes the upper example, when user writes
"device" to mode switch sysfs, the driver should skip the HW state of ID
pin, due to it's state is Low, or force it as High.

Another option way is that introduces a property in DTS to indicate the
way the driver want to use (HW based or SW based, usb_role_switch
doesn't provide this information for the controller driver), but is not
flexible enough.
 
> 
> cheers,
> -roger




Re: [PATCH v10 5/6] usb:cdns3 Add Cadence USB3 DRD Driver

2019-08-12 Thread Felipe Balbi


Hi,

Roger Quadros  writes:
>> Roger Quadros  writes:
 The sysfs file we expose from the class for the role switches is
 primarily meant for supporting proprietary protocols that require us
 to basically override the connector USB data role. The default role
 should always be selected in the drivers.
>>>
>>> OK. Let's take this example
>>> - Port is dual-role port micro AB.
>>> - microAB to type-A adapter is connected which pulls ID low. port 
>>> transitions
>>> to "host" role by the controller driver.
>>> - proprietary protocol want to switch role to device role so writes 
>>> "device" to
>>> mode switch sysfs. port transitions to "device" role.
>>>
>>> Now, how does controller driver know to fall back to HW based role 
>>> switching?
>> 
>> Use a 'disconnect' or 'suspend' event to go reset it? But that should,
>> probably, be done at kernel space, no?
>> 
>
> Yes that could be one option.
> So after a disconnect, sysfs role should reflect actual hardware role. 
> correct?

that would be my expectation

-- 
balbi


Re: [PATCH v10 5/6] usb:cdns3 Add Cadence USB3 DRD Driver

2019-08-12 Thread Roger Quadros



On 12/08/2019 15:46, Felipe Balbi wrote:
> 
> Hi,
> 
> Roger Quadros  writes:
>>> The sysfs file we expose from the class for the role switches is
>>> primarily meant for supporting proprietary protocols that require us
>>> to basically override the connector USB data role. The default role
>>> should always be selected in the drivers.
>>
>> OK. Let's take this example
>> - Port is dual-role port micro AB.
>> - microAB to type-A adapter is connected which pulls ID low. port transitions
>> to "host" role by the controller driver.
>> - proprietary protocol want to switch role to device role so writes "device" 
>> to
>> mode switch sysfs. port transitions to "device" role.
>>
>> Now, how does controller driver know to fall back to HW based role switching?
> 
> Use a 'disconnect' or 'suspend' event to go reset it? But that should,
> probably, be done at kernel space, no?
> 

Yes that could be one option.
So after a disconnect, sysfs role should reflect actual hardware role. correct?

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


Re: [PATCH v10 5/6] usb:cdns3 Add Cadence USB3 DRD Driver

2019-08-12 Thread Felipe Balbi

Hi,

Roger Quadros  writes:
>> The sysfs file we expose from the class for the role switches is
>> primarily meant for supporting proprietary protocols that require us
>> to basically override the connector USB data role. The default role
>> should always be selected in the drivers.
>
> OK. Let's take this example
> - Port is dual-role port micro AB.
> - microAB to type-A adapter is connected which pulls ID low. port transitions
> to "host" role by the controller driver.
> - proprietary protocol want to switch role to device role so writes "device" 
> to
> mode switch sysfs. port transitions to "device" role.
>
> Now, how does controller driver know to fall back to HW based role switching?

Use a 'disconnect' or 'suspend' event to go reset it? But that should,
probably, be done at kernel space, no?

-- 
balbi


signature.asc
Description: PGP signature


RE: [PATCH v10 5/6] usb:cdns3 Add Cadence USB3 DRD Driver

2019-08-12 Thread Pawel Laszczak
>
>On 11/08/2019 14:59, Pawel Laszczak wrote:
>> Hi,
>>
>>>
>>> On 21/07/2019 21:32, Pawel Laszczak wrote:
 This patch introduce new Cadence USBSS DRD driver to Linux kernel.

 The Cadence USBSS DRD Controller is a highly configurable IP Core which
 can be instantiated as Dual-Role Device (DRD), Peripheral Only and
 Host Only (XHCI)configurations.

 The current driver has been validated with FPGA platform. We have
 support for PCIe bus, which is used on FPGA prototyping.

 The host side of USBSS-DRD controller is compliant with XHCI
 specification, so it works with standard XHCI Linux driver.

 Signed-off-by: Pawel Laszczak 
 ---
  drivers/usb/Kconfig|2 +
  drivers/usb/Makefile   |2 +
  drivers/usb/cdns3/Kconfig  |   46 +
  drivers/usb/cdns3/Makefile |   17 +
  drivers/usb/cdns3/cdns3-pci-wrap.c |  203 +++
  drivers/usb/cdns3/core.c   |  554 +++
  drivers/usb/cdns3/core.h   |  109 ++
  drivers/usb/cdns3/debug.h  |  171 ++
  drivers/usb/cdns3/debugfs.c|   87 ++
  drivers/usb/cdns3/drd.c|  390 +
  drivers/usb/cdns3/drd.h|  166 ++
  drivers/usb/cdns3/ep0.c|  914 +++
  drivers/usb/cdns3/gadget-export.h  |   28 +
  drivers/usb/cdns3/gadget.c | 2338 
  drivers/usb/cdns3/gadget.h | 1321 
  drivers/usb/cdns3/host-export.h|   28 +
  drivers/usb/cdns3/host.c   |   71 +
  drivers/usb/cdns3/trace.c  |   11 +
  drivers/usb/cdns3/trace.h  |  493 ++
  19 files changed, 6951 insertions(+)
  create mode 100644 drivers/usb/cdns3/Kconfig
  create mode 100644 drivers/usb/cdns3/Makefile
  create mode 100644 drivers/usb/cdns3/cdns3-pci-wrap.c
  create mode 100644 drivers/usb/cdns3/core.c
  create mode 100644 drivers/usb/cdns3/core.h
  create mode 100644 drivers/usb/cdns3/debug.h
  create mode 100644 drivers/usb/cdns3/debugfs.c
  create mode 100644 drivers/usb/cdns3/drd.c
  create mode 100644 drivers/usb/cdns3/drd.h
  create mode 100644 drivers/usb/cdns3/ep0.c
  create mode 100644 drivers/usb/cdns3/gadget-export.h
  create mode 100644 drivers/usb/cdns3/gadget.c
  create mode 100644 drivers/usb/cdns3/gadget.h
  create mode 100644 drivers/usb/cdns3/host-export.h
  create mode 100644 drivers/usb/cdns3/host.c
  create mode 100644 drivers/usb/cdns3/trace.c
  create mode 100644 drivers/usb/cdns3/trace.h

>
>
>
>>>
 diff --git a/drivers/usb/cdns3/gadget.c b/drivers/usb/cdns3/gadget.c
 new file mode 100644
 index ..291f08be56fe
 --- /dev/null
 +++ b/drivers/usb/cdns3/gadget.c
 @@ -0,0 +1,2338 @@
 +// SPDX-License-Identifier: GPL-2.0
 +/*
 + * Cadence USBSS DRD Driver - gadget side.
 + *
 + * Copyright (C) 2018-2019 Cadence Design Systems.
 + * Copyright (C) 2017-2018 NXP
 + *
 + * Authors: Pawel Jez ,
 + *  Pawel Laszczak 
 + *  Peter Chen 
 + */
 +
>>>
>>> 
>>>
 +/**
 + * cdns3_device_irq_handler- interrupt handler for device part of 
 controller
 + *
 + * @irq: irq number for cdns3 core device
 + * @data: structure of cdns3
 + *
 + * Returns IRQ_HANDLED or IRQ_NONE
 + */
 +static irqreturn_t cdns3_device_irq_handler(int irq, void *data)
 +{
 +  struct cdns3_device *priv_dev;
 +  struct cdns3 *cdns = data;
 +  irqreturn_t ret = IRQ_NONE;
 +  unsigned long flags;
 +  u32 reg;
 +
 +  priv_dev = cdns->gadget_dev;
 +  spin_lock_irqsave(&priv_dev->lock, flags);
 +
 +  /* check USB device interrupt */
 +  reg = readl(&priv_dev->regs->usb_ists);
 +
 +  if (reg) {
 +  writel(reg, &priv_dev->regs->usb_ists);
>>>
>>> Do we need to mask device interrupts till thread handler has done processing
>>> current set of events?
>>
>> Yes, we need do this to avoid raising the next the same interrupts.
>> If we return back from hard irq handler without clearing reported interrupts,
>> then system report them again. The solution for this is to use IRQF_ONESHOT
>> flag during registering interrupt, but I can't use this flag
>> because I have shared interrupt line with other component.
>>
>> In this version these (usb_ists) interrupts are handled in hard irq, but in 
>> next version
>> will be moved to thread handler.
>>
 +  cdns3_check_usb_interrupt_proceed(priv_dev, reg);
 +  ret = IRQ_HANDLED;
 +  }
 +
 +  /* check endpoint interrupt */
 +  reg = readl(&priv_dev->regs->ep_ists);
 +
 +  if (reg) {
 +  priv_dev->shadow_ep_en |= reg;
 +  reg = ~reg & readl(&priv_dev->regs->ep_ien);
 +  /* mask deferred interrupt. */
 +  writel(reg, &

Re: [PATCH v10 5/6] usb:cdns3 Add Cadence USB3 DRD Driver

2019-08-12 Thread Roger Quadros
On 12/08/2019 13:31, Heikki Krogerus wrote:
> Hi,
> 
>> +real_role = cdsn3_real_role_switch_get(cdns->dev);
>> +
>> +current_role = role;
>> +dev_dbg(cdns->dev, "Switching role");
>> +
>> +ret = cdns3_role_start(cdns, real_role);
>> +if (ret) {
>> +/* Back to current role */
>> +dev_err(cdns->dev, "set %d has failed, back to %d\n",
>> +role, current_role);
>> +ret = cdns3_role_start(cdns, current_role);
>> +if (ret)
>> +dev_err(cdns->dev, "back to %d failed too\n",
>> +current_role);
>> +}
>> +exit:
>> +pm_runtime_put_sync(cdns->dev);
>> +return ret;
>> +}
>> +
>> +static const struct usb_role_switch_desc cdns3_switch_desc = {
>> +.set = cdns3_role_switch_set,
>> +.get = cdsn3_real_role_switch_get,
>> +.allow_userspace_control = true,
>
> how does user initiated cdns3_role_switch_set() via sysfs co-exist with 
> role
> changes done by hardware events. e.g. ID/VBUS?
>

 Do you expect any issues whit this,  have you seen any problem with this
 on your  platform ?

 I assume that it should work in this way:
 1. user change role by sysfs
 2. Driver change the role according with user request.
 3. If we receive correct ID/VBUS then role should not be changed
 because new role is the same as current set in point 2.

>>>
>>> I have not tested this series yet.
>>> My understanding is that if user sets role to "host" or "device" then it 
>>> should
>>> remain in that role irrespective of ID/VBUS. Once user sets it to "none" 
>>> then
>>> port should set role based on ID/VBUS.
>>
>> According with your understanding it works the same way as by debugfs. 
>> Now I have no doubts to remove debugfs.c file :)
> 
> Hold on! The role "none" means that the connector should not be
> connected to either the host nor device.

OK.
> 
> The sysfs file we expose from the class for the role switches is
> primarily meant for supporting proprietary protocols that require us
> to basically override the connector USB data role. The default role
> should always be selected in the drivers.

OK. Let's take this example
- Port is dual-role port micro AB.
- microAB to type-A adapter is connected which pulls ID low. port transitions
to "host" role by the controller driver.
- proprietary protocol want to switch role to device role so writes "device" to
mode switch sysfs. port transitions to "device" role.

Now, how does controller driver know to fall back to HW based role switching?

> 
> With USB Type-C connectors and alternate modes, the "none" role is
> used for example when the connector is put into "USB Safe State". In
> case you guys are not familiar with USB Safe State, then it is a state
> (defined in USB PD specifications) for the connector where the data
> lines on the connector should not be physically connected to anything.
> The connector needs to be put into safe state always when entering
> or exiting an alternate mode, before the final mode (USB or alternate)
> is actually being set for the connector.
> 
> 
> thanks,
> 

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


Re: [PATCH v10 5/6] usb:cdns3 Add Cadence USB3 DRD Driver

2019-08-12 Thread Heikki Krogerus
Hi,

>  +real_role = cdsn3_real_role_switch_get(cdns->dev);
>  +
>  +current_role = role;
>  +dev_dbg(cdns->dev, "Switching role");
>  +
>  +ret = cdns3_role_start(cdns, real_role);
>  +if (ret) {
>  +/* Back to current role */
>  +dev_err(cdns->dev, "set %d has failed, back to %d\n",
>  +role, current_role);
>  +ret = cdns3_role_start(cdns, current_role);
>  +if (ret)
>  +dev_err(cdns->dev, "back to %d failed too\n",
>  +current_role);
>  +}
>  +exit:
>  +pm_runtime_put_sync(cdns->dev);
>  +return ret;
>  +}
>  +
>  +static const struct usb_role_switch_desc cdns3_switch_desc = {
>  +.set = cdns3_role_switch_set,
>  +.get = cdsn3_real_role_switch_get,
>  +.allow_userspace_control = true,
> >>>
> >>> how does user initiated cdns3_role_switch_set() via sysfs co-exist with 
> >>> role
> >>> changes done by hardware events. e.g. ID/VBUS?
> >>>
> >>
> >> Do you expect any issues whit this,  have you seen any problem with this
> >> on your  platform ?
> >>
> >> I assume that it should work in this way:
> >> 1. user change role by sysfs
> >> 2. Driver change the role according with user request.
> >> 3. If we receive correct ID/VBUS then role should not be changed
> >> because new role is the same as current set in point 2.
> >>
> >
> >I have not tested this series yet.
> >My understanding is that if user sets role to "host" or "device" then it 
> >should
> >remain in that role irrespective of ID/VBUS. Once user sets it to "none" then
> >port should set role based on ID/VBUS.
> 
> According with your understanding it works the same way as by debugfs. 
> Now I have no doubts to remove debugfs.c file :)

Hold on! The role "none" means that the connector should not be
connected to either the host nor device.

The sysfs file we expose from the class for the role switches is
primarily meant for supporting proprietary protocols that require us
to basically override the connector USB data role. The default role
should always be selected in the drivers.

With USB Type-C connectors and alternate modes, the "none" role is
used for example when the connector is put into "USB Safe State". In
case you guys are not familiar with USB Safe State, then it is a state
(defined in USB PD specifications) for the connector where the data
lines on the connector should not be physically connected to anything.
The connector needs to be put into safe state always when entering
or exiting an alternate mode, before the final mode (USB or alternate)
is actually being set for the connector.


thanks,

-- 
heikki


Re: [PATCH v10 5/6] usb:cdns3 Add Cadence USB3 DRD Driver

2019-08-12 Thread Roger Quadros



On 11/08/2019 14:59, Pawel Laszczak wrote:
> Hi,
> 
>>
>> On 21/07/2019 21:32, Pawel Laszczak wrote:
>>> This patch introduce new Cadence USBSS DRD driver to Linux kernel.
>>>
>>> The Cadence USBSS DRD Controller is a highly configurable IP Core which
>>> can be instantiated as Dual-Role Device (DRD), Peripheral Only and
>>> Host Only (XHCI)configurations.
>>>
>>> The current driver has been validated with FPGA platform. We have
>>> support for PCIe bus, which is used on FPGA prototyping.
>>>
>>> The host side of USBSS-DRD controller is compliant with XHCI
>>> specification, so it works with standard XHCI Linux driver.
>>>
>>> Signed-off-by: Pawel Laszczak 
>>> ---
>>>  drivers/usb/Kconfig|2 +
>>>  drivers/usb/Makefile   |2 +
>>>  drivers/usb/cdns3/Kconfig  |   46 +
>>>  drivers/usb/cdns3/Makefile |   17 +
>>>  drivers/usb/cdns3/cdns3-pci-wrap.c |  203 +++
>>>  drivers/usb/cdns3/core.c   |  554 +++
>>>  drivers/usb/cdns3/core.h   |  109 ++
>>>  drivers/usb/cdns3/debug.h  |  171 ++
>>>  drivers/usb/cdns3/debugfs.c|   87 ++
>>>  drivers/usb/cdns3/drd.c|  390 +
>>>  drivers/usb/cdns3/drd.h|  166 ++
>>>  drivers/usb/cdns3/ep0.c|  914 +++
>>>  drivers/usb/cdns3/gadget-export.h  |   28 +
>>>  drivers/usb/cdns3/gadget.c | 2338 
>>>  drivers/usb/cdns3/gadget.h | 1321 
>>>  drivers/usb/cdns3/host-export.h|   28 +
>>>  drivers/usb/cdns3/host.c   |   71 +
>>>  drivers/usb/cdns3/trace.c  |   11 +
>>>  drivers/usb/cdns3/trace.h  |  493 ++
>>>  19 files changed, 6951 insertions(+)
>>>  create mode 100644 drivers/usb/cdns3/Kconfig
>>>  create mode 100644 drivers/usb/cdns3/Makefile
>>>  create mode 100644 drivers/usb/cdns3/cdns3-pci-wrap.c
>>>  create mode 100644 drivers/usb/cdns3/core.c
>>>  create mode 100644 drivers/usb/cdns3/core.h
>>>  create mode 100644 drivers/usb/cdns3/debug.h
>>>  create mode 100644 drivers/usb/cdns3/debugfs.c
>>>  create mode 100644 drivers/usb/cdns3/drd.c
>>>  create mode 100644 drivers/usb/cdns3/drd.h
>>>  create mode 100644 drivers/usb/cdns3/ep0.c
>>>  create mode 100644 drivers/usb/cdns3/gadget-export.h
>>>  create mode 100644 drivers/usb/cdns3/gadget.c
>>>  create mode 100644 drivers/usb/cdns3/gadget.h
>>>  create mode 100644 drivers/usb/cdns3/host-export.h
>>>  create mode 100644 drivers/usb/cdns3/host.c
>>>  create mode 100644 drivers/usb/cdns3/trace.c
>>>  create mode 100644 drivers/usb/cdns3/trace.h
>>>



>>
>>> diff --git a/drivers/usb/cdns3/gadget.c b/drivers/usb/cdns3/gadget.c
>>> new file mode 100644
>>> index ..291f08be56fe
>>> --- /dev/null
>>> +++ b/drivers/usb/cdns3/gadget.c
>>> @@ -0,0 +1,2338 @@
>>> +// SPDX-License-Identifier: GPL-2.0
>>> +/*
>>> + * Cadence USBSS DRD Driver - gadget side.
>>> + *
>>> + * Copyright (C) 2018-2019 Cadence Design Systems.
>>> + * Copyright (C) 2017-2018 NXP
>>> + *
>>> + * Authors: Pawel Jez ,
>>> + *  Pawel Laszczak 
>>> + *  Peter Chen 
>>> + */
>>> +
>>
>> 
>>
>>> +/**
>>> + * cdns3_device_irq_handler- interrupt handler for device part of 
>>> controller
>>> + *
>>> + * @irq: irq number for cdns3 core device
>>> + * @data: structure of cdns3
>>> + *
>>> + * Returns IRQ_HANDLED or IRQ_NONE
>>> + */
>>> +static irqreturn_t cdns3_device_irq_handler(int irq, void *data)
>>> +{
>>> +   struct cdns3_device *priv_dev;
>>> +   struct cdns3 *cdns = data;
>>> +   irqreturn_t ret = IRQ_NONE;
>>> +   unsigned long flags;
>>> +   u32 reg;
>>> +
>>> +   priv_dev = cdns->gadget_dev;
>>> +   spin_lock_irqsave(&priv_dev->lock, flags);
>>> +
>>> +   /* check USB device interrupt */
>>> +   reg = readl(&priv_dev->regs->usb_ists);
>>> +
>>> +   if (reg) {
>>> +   writel(reg, &priv_dev->regs->usb_ists);
>>
>> Do we need to mask device interrupts till thread handler has done processing
>> current set of events?
> 
> Yes, we need do this to avoid raising the next the same interrupts. 
> If we return back from hard irq handler without clearing reported interrupts,
> then system report them again. The solution for this is to use IRQF_ONESHOT
> flag during registering interrupt, but I can't use this flag 
> because I have shared interrupt line with other component. 
> 
> In this version these (usb_ists) interrupts are handled in hard irq, but in 
> next version
> will be moved to thread handler. 
> 
>>> +   cdns3_check_usb_interrupt_proceed(priv_dev, reg);
>>> +   ret = IRQ_HANDLED;
>>> +   }
>>> +
>>> +   /* check endpoint interrupt */
>>> +   reg = readl(&priv_dev->regs->ep_ists);
>>> +
>>> +   if (reg) {
>>> +   priv_dev->shadow_ep_en |= reg;
>>> +   reg = ~reg & readl(&priv_dev->regs->ep_ien);
>>> +   /* mask deferred interrupt. */
>>> +   writel(reg, &priv_dev->regs->ep_ien);
>>> +   ret = IRQ_WAKE_THREAD;
>>> +   }
>>> +
>>> +   spin_unlock_irq

RE: [PATCH v10 5/6] usb:cdns3 Add Cadence USB3 DRD Driver

2019-08-12 Thread Pawel Laszczak
Hi,

>
>On 11/08/2019 14:59, Pawel Laszczak wrote:
>> Hi,
>>
>>>
>>> On 21/07/2019 21:32, Pawel Laszczak wrote:
 This patch introduce new Cadence USBSS DRD driver to Linux kernel.

 The Cadence USBSS DRD Controller is a highly configurable IP Core which
 can be instantiated as Dual-Role Device (DRD), Peripheral Only and
 Host Only (XHCI)configurations.

 The current driver has been validated with FPGA platform. We have
 support for PCIe bus, which is used on FPGA prototyping.

 The host side of USBSS-DRD controller is compliant with XHCI
 specification, so it works with standard XHCI Linux driver.

 Signed-off-by: Pawel Laszczak 
 ---
  drivers/usb/Kconfig|2 +
  drivers/usb/Makefile   |2 +
  drivers/usb/cdns3/Kconfig  |   46 +
  drivers/usb/cdns3/Makefile |   17 +
  drivers/usb/cdns3/cdns3-pci-wrap.c |  203 +++
  drivers/usb/cdns3/core.c   |  554 +++
  drivers/usb/cdns3/core.h   |  109 ++
  drivers/usb/cdns3/debug.h  |  171 ++
  drivers/usb/cdns3/debugfs.c|   87 ++
  drivers/usb/cdns3/drd.c|  390 +
  drivers/usb/cdns3/drd.h|  166 ++
  drivers/usb/cdns3/ep0.c|  914 +++
  drivers/usb/cdns3/gadget-export.h  |   28 +
  drivers/usb/cdns3/gadget.c | 2338 
  drivers/usb/cdns3/gadget.h | 1321 
  drivers/usb/cdns3/host-export.h|   28 +
  drivers/usb/cdns3/host.c   |   71 +
  drivers/usb/cdns3/trace.c  |   11 +
  drivers/usb/cdns3/trace.h  |  493 ++
  19 files changed, 6951 insertions(+)
  create mode 100644 drivers/usb/cdns3/Kconfig
  create mode 100644 drivers/usb/cdns3/Makefile
  create mode 100644 drivers/usb/cdns3/cdns3-pci-wrap.c
  create mode 100644 drivers/usb/cdns3/core.c
  create mode 100644 drivers/usb/cdns3/core.h
  create mode 100644 drivers/usb/cdns3/debug.h
  create mode 100644 drivers/usb/cdns3/debugfs.c
  create mode 100644 drivers/usb/cdns3/drd.c
  create mode 100644 drivers/usb/cdns3/drd.h
  create mode 100644 drivers/usb/cdns3/ep0.c
  create mode 100644 drivers/usb/cdns3/gadget-export.h
  create mode 100644 drivers/usb/cdns3/gadget.c
  create mode 100644 drivers/usb/cdns3/gadget.h
  create mode 100644 drivers/usb/cdns3/host-export.h
  create mode 100644 drivers/usb/cdns3/host.c
  create mode 100644 drivers/usb/cdns3/trace.c
  create mode 100644 drivers/usb/cdns3/trace.h

 diff --git a/drivers/usb/Kconfig b/drivers/usb/Kconfig
 index e4b27413f528..c2e78882f8c2 100644
 --- a/drivers/usb/Kconfig
 +++ b/drivers/usb/Kconfig
 @@ -113,6 +113,8 @@ source "drivers/usb/usbip/Kconfig"

  endif




 +  real_role = cdsn3_real_role_switch_get(cdns->dev);
 +
 +  current_role = role;
 +  dev_dbg(cdns->dev, "Switching role");
 +
 +  ret = cdns3_role_start(cdns, real_role);
 +  if (ret) {
 +  /* Back to current role */
 +  dev_err(cdns->dev, "set %d has failed, back to %d\n",
 +  role, current_role);
 +  ret = cdns3_role_start(cdns, current_role);
 +  if (ret)
 +  dev_err(cdns->dev, "back to %d failed too\n",
 +  current_role);
 +  }
 +exit:
 +  pm_runtime_put_sync(cdns->dev);
 +  return ret;
 +}
 +
 +static const struct usb_role_switch_desc cdns3_switch_desc = {
 +  .set = cdns3_role_switch_set,
 +  .get = cdsn3_real_role_switch_get,
 +  .allow_userspace_control = true,
>>>
>>> how does user initiated cdns3_role_switch_set() via sysfs co-exist with role
>>> changes done by hardware events. e.g. ID/VBUS?
>>>
>>
>> Do you expect any issues whit this,  have you seen any problem with this
>> on your  platform ?
>>
>> I assume that it should work in this way:
>> 1. user change role by sysfs
>> 2. Driver change the role according with user request.
>> 3. If we receive correct ID/VBUS then role should not be changed
>> because new role is the same as current set in point 2.
>>
>
>I have not tested this series yet.
>My understanding is that if user sets role to "host" or "device" then it should
>remain in that role irrespective of ID/VBUS. Once user sets it to "none" then
>port should set role based on ID/VBUS.

According with your understanding it works the same way as by debugfs. 
Now I have no doubts to remove debugfs.c file :)

>
>>
 +};
 +
 +/**
 + * cdns3_probe - probe for cdns3 core device
 + * @pdev: Pointer to cdns3 core platform device
 + *
 + * Returns 0 on success otherwise negative errno
 + */
 +static int cdns3_probe(struct platform_device *pdev)



 + * Returns 0 on success otherwise negative errno
 + */
 +int cdns3

Re: [PATCH v10 5/6] usb:cdns3 Add Cadence USB3 DRD Driver

2019-08-12 Thread Roger Quadros
Hi,

On 11/08/2019 14:59, Pawel Laszczak wrote:
> Hi,
> 
>>
>> On 21/07/2019 21:32, Pawel Laszczak wrote:
>>> This patch introduce new Cadence USBSS DRD driver to Linux kernel.
>>>
>>> The Cadence USBSS DRD Controller is a highly configurable IP Core which
>>> can be instantiated as Dual-Role Device (DRD), Peripheral Only and
>>> Host Only (XHCI)configurations.
>>>
>>> The current driver has been validated with FPGA platform. We have
>>> support for PCIe bus, which is used on FPGA prototyping.
>>>
>>> The host side of USBSS-DRD controller is compliant with XHCI
>>> specification, so it works with standard XHCI Linux driver.
>>>
>>> Signed-off-by: Pawel Laszczak 
>>> ---
>>>  drivers/usb/Kconfig|2 +
>>>  drivers/usb/Makefile   |2 +
>>>  drivers/usb/cdns3/Kconfig  |   46 +
>>>  drivers/usb/cdns3/Makefile |   17 +
>>>  drivers/usb/cdns3/cdns3-pci-wrap.c |  203 +++
>>>  drivers/usb/cdns3/core.c   |  554 +++
>>>  drivers/usb/cdns3/core.h   |  109 ++
>>>  drivers/usb/cdns3/debug.h  |  171 ++
>>>  drivers/usb/cdns3/debugfs.c|   87 ++
>>>  drivers/usb/cdns3/drd.c|  390 +
>>>  drivers/usb/cdns3/drd.h|  166 ++
>>>  drivers/usb/cdns3/ep0.c|  914 +++
>>>  drivers/usb/cdns3/gadget-export.h  |   28 +
>>>  drivers/usb/cdns3/gadget.c | 2338 
>>>  drivers/usb/cdns3/gadget.h | 1321 
>>>  drivers/usb/cdns3/host-export.h|   28 +
>>>  drivers/usb/cdns3/host.c   |   71 +
>>>  drivers/usb/cdns3/trace.c  |   11 +
>>>  drivers/usb/cdns3/trace.h  |  493 ++
>>>  19 files changed, 6951 insertions(+)
>>>  create mode 100644 drivers/usb/cdns3/Kconfig
>>>  create mode 100644 drivers/usb/cdns3/Makefile
>>>  create mode 100644 drivers/usb/cdns3/cdns3-pci-wrap.c
>>>  create mode 100644 drivers/usb/cdns3/core.c
>>>  create mode 100644 drivers/usb/cdns3/core.h
>>>  create mode 100644 drivers/usb/cdns3/debug.h
>>>  create mode 100644 drivers/usb/cdns3/debugfs.c
>>>  create mode 100644 drivers/usb/cdns3/drd.c
>>>  create mode 100644 drivers/usb/cdns3/drd.h
>>>  create mode 100644 drivers/usb/cdns3/ep0.c
>>>  create mode 100644 drivers/usb/cdns3/gadget-export.h
>>>  create mode 100644 drivers/usb/cdns3/gadget.c
>>>  create mode 100644 drivers/usb/cdns3/gadget.h
>>>  create mode 100644 drivers/usb/cdns3/host-export.h
>>>  create mode 100644 drivers/usb/cdns3/host.c
>>>  create mode 100644 drivers/usb/cdns3/trace.c
>>>  create mode 100644 drivers/usb/cdns3/trace.h
>>>
>>> diff --git a/drivers/usb/Kconfig b/drivers/usb/Kconfig
>>> index e4b27413f528..c2e78882f8c2 100644
>>> --- a/drivers/usb/Kconfig
>>> +++ b/drivers/usb/Kconfig
>>> @@ -113,6 +113,8 @@ source "drivers/usb/usbip/Kconfig"
>>>
>>>  endif
>>>
>>> +source "drivers/usb/cdns3/Kconfig"
>>> +
>>>  source "drivers/usb/mtu3/Kconfig"
>>>
>>>  source "drivers/usb/musb/Kconfig"
>>> diff --git a/drivers/usb/Makefile b/drivers/usb/Makefile
>>> index 7d1b8c82b208..ab125b966cac 100644
>>> --- a/drivers/usb/Makefile
>>> +++ b/drivers/usb/Makefile
>>> @@ -12,6 +12,8 @@ obj-$(CONFIG_USB_DWC3)+= dwc3/
>>>  obj-$(CONFIG_USB_DWC2) += dwc2/
>>>  obj-$(CONFIG_USB_ISP1760)  += isp1760/
>>>
>>> +obj-$(CONFIG_USB_CDNS3)+= cdns3/
>>> +
>>>  obj-$(CONFIG_USB_MON)  += mon/
>>>  obj-$(CONFIG_USB_MTU3) += mtu3/
>>>
>>> diff --git a/drivers/usb/cdns3/Kconfig b/drivers/usb/cdns3/Kconfig
>>> new file mode 100644
>>> index ..d0331613a355
>>> --- /dev/null
>>> +++ b/drivers/usb/cdns3/Kconfig
>>> @@ -0,0 +1,46 @@
>>> +config USB_CDNS3
>>> +   tristate "Cadence USB3 Dual-Role Controller"
>>> +   depends on USB_SUPPORT && (USB || USB_GADGET) && HAS_DMA
>>> +   select USB_XHCI_PLATFORM if USB_XHCI_HCD
>>> +   select USB_ROLE_SWITCH
>>> +   help
>>> + Say Y here if your system has a Cadence USB3 dual-role controller.
>>> + It supports: dual-role switch, Host-only, and Peripheral-only.
>>> +
>>> + If you choose to build this driver is a dynamically linked
>>> + as module, the module will be called cdns3.ko.
>>> +
>>> +if USB_CDNS3
>>> +
>>> +config USB_CDNS3_GADGET
>>> +   bool "Cadence USB3 device controller"
>>> +   depends on USB_GADGET=y || USB_GADGET=USB_CDNS3
>>> +   help
>>> + Say Y here to enable device controller functionality of the
>>> + Cadence USBSS-DEV driver.
>>> +
>>> + This controller supports FF, HS and SS mode. It doesn't support
>>
>> s/FF/FS
>>
>>> + LS and SSP mode.
>>> +
>>> +config USB_CDNS3_HOST
>>> +   bool "Cadence USB3 host controller"
>>> +   depends on USB=y || USB=USB_CDNS3
>>> +   help
>>> + Say Y here to enable host controller functionality of the
>>> + Cadence driver.
>>> +
>>> + Host controller is compliant with XHCI so it will use
>>> + standard XHCI driver.
>>> +
>>> +config USB_CDNS3_PCI_WRAP
>>> +   tristate "Cadence USB3 support on PCIe-based p

RE: [PATCH v10 5/6] usb:cdns3 Add Cadence USB3 DRD Driver

2019-08-11 Thread Pawel Laszczak
Hi,

>
>On 21/07/2019 21:32, Pawel Laszczak wrote:
>> This patch introduce new Cadence USBSS DRD driver to Linux kernel.
>>
>> The Cadence USBSS DRD Controller is a highly configurable IP Core which
>> can be instantiated as Dual-Role Device (DRD), Peripheral Only and
>> Host Only (XHCI)configurations.
>>
>> The current driver has been validated with FPGA platform. We have
>> support for PCIe bus, which is used on FPGA prototyping.
>>
>> The host side of USBSS-DRD controller is compliant with XHCI
>> specification, so it works with standard XHCI Linux driver.
>>
>> Signed-off-by: Pawel Laszczak 
>> ---
>>  drivers/usb/Kconfig|2 +
>>  drivers/usb/Makefile   |2 +
>>  drivers/usb/cdns3/Kconfig  |   46 +
>>  drivers/usb/cdns3/Makefile |   17 +
>>  drivers/usb/cdns3/cdns3-pci-wrap.c |  203 +++
>>  drivers/usb/cdns3/core.c   |  554 +++
>>  drivers/usb/cdns3/core.h   |  109 ++
>>  drivers/usb/cdns3/debug.h  |  171 ++
>>  drivers/usb/cdns3/debugfs.c|   87 ++
>>  drivers/usb/cdns3/drd.c|  390 +
>>  drivers/usb/cdns3/drd.h|  166 ++
>>  drivers/usb/cdns3/ep0.c|  914 +++
>>  drivers/usb/cdns3/gadget-export.h  |   28 +
>>  drivers/usb/cdns3/gadget.c | 2338 
>>  drivers/usb/cdns3/gadget.h | 1321 
>>  drivers/usb/cdns3/host-export.h|   28 +
>>  drivers/usb/cdns3/host.c   |   71 +
>>  drivers/usb/cdns3/trace.c  |   11 +
>>  drivers/usb/cdns3/trace.h  |  493 ++
>>  19 files changed, 6951 insertions(+)
>>  create mode 100644 drivers/usb/cdns3/Kconfig
>>  create mode 100644 drivers/usb/cdns3/Makefile
>>  create mode 100644 drivers/usb/cdns3/cdns3-pci-wrap.c
>>  create mode 100644 drivers/usb/cdns3/core.c
>>  create mode 100644 drivers/usb/cdns3/core.h
>>  create mode 100644 drivers/usb/cdns3/debug.h
>>  create mode 100644 drivers/usb/cdns3/debugfs.c
>>  create mode 100644 drivers/usb/cdns3/drd.c
>>  create mode 100644 drivers/usb/cdns3/drd.h
>>  create mode 100644 drivers/usb/cdns3/ep0.c
>>  create mode 100644 drivers/usb/cdns3/gadget-export.h
>>  create mode 100644 drivers/usb/cdns3/gadget.c
>>  create mode 100644 drivers/usb/cdns3/gadget.h
>>  create mode 100644 drivers/usb/cdns3/host-export.h
>>  create mode 100644 drivers/usb/cdns3/host.c
>>  create mode 100644 drivers/usb/cdns3/trace.c
>>  create mode 100644 drivers/usb/cdns3/trace.h
>>
>> diff --git a/drivers/usb/Kconfig b/drivers/usb/Kconfig
>> index e4b27413f528..c2e78882f8c2 100644
>> --- a/drivers/usb/Kconfig
>> +++ b/drivers/usb/Kconfig
>> @@ -113,6 +113,8 @@ source "drivers/usb/usbip/Kconfig"
>>
>>  endif
>>
>> +source "drivers/usb/cdns3/Kconfig"
>> +
>>  source "drivers/usb/mtu3/Kconfig"
>>
>>  source "drivers/usb/musb/Kconfig"
>> diff --git a/drivers/usb/Makefile b/drivers/usb/Makefile
>> index 7d1b8c82b208..ab125b966cac 100644
>> --- a/drivers/usb/Makefile
>> +++ b/drivers/usb/Makefile
>> @@ -12,6 +12,8 @@ obj-$(CONFIG_USB_DWC3) += dwc3/
>>  obj-$(CONFIG_USB_DWC2)  += dwc2/
>>  obj-$(CONFIG_USB_ISP1760)   += isp1760/
>>
>> +obj-$(CONFIG_USB_CDNS3) += cdns3/
>> +
>>  obj-$(CONFIG_USB_MON)   += mon/
>>  obj-$(CONFIG_USB_MTU3)  += mtu3/
>>
>> diff --git a/drivers/usb/cdns3/Kconfig b/drivers/usb/cdns3/Kconfig
>> new file mode 100644
>> index ..d0331613a355
>> --- /dev/null
>> +++ b/drivers/usb/cdns3/Kconfig
>> @@ -0,0 +1,46 @@
>> +config USB_CDNS3
>> +tristate "Cadence USB3 Dual-Role Controller"
>> +depends on USB_SUPPORT && (USB || USB_GADGET) && HAS_DMA
>> +select USB_XHCI_PLATFORM if USB_XHCI_HCD
>> +select USB_ROLE_SWITCH
>> +help
>> +  Say Y here if your system has a Cadence USB3 dual-role controller.
>> +  It supports: dual-role switch, Host-only, and Peripheral-only.
>> +
>> +  If you choose to build this driver is a dynamically linked
>> +  as module, the module will be called cdns3.ko.
>> +
>> +if USB_CDNS3
>> +
>> +config USB_CDNS3_GADGET
>> +bool "Cadence USB3 device controller"
>> +depends on USB_GADGET=y || USB_GADGET=USB_CDNS3
>> +help
>> +  Say Y here to enable device controller functionality of the
>> +  Cadence USBSS-DEV driver.
>> +
>> +  This controller supports FF, HS and SS mode. It doesn't support
>
>s/FF/FS
>
>> +  LS and SSP mode.
>> +
>> +config USB_CDNS3_HOST
>> +bool "Cadence USB3 host controller"
>> +depends on USB=y || USB=USB_CDNS3
>> +help
>> +  Say Y here to enable host controller functionality of the
>> +  Cadence driver.
>> +
>> +  Host controller is compliant with XHCI so it will use
>> +  standard XHCI driver.
>> +
>> +config USB_CDNS3_PCI_WRAP
>> +tristate "Cadence USB3 support on PCIe-based platforms"
>> +depends on USB_PCI && ACPI
>> +default USB_CDNS3
>> +help
>> +  If you're using the USBSS Core IP with a PCIe, please s

Re: [PATCH v10 5/6] usb:cdns3 Add Cadence USB3 DRD Driver

2019-08-07 Thread Roger Quadros
Pawel,

On 21/07/2019 21:32, Pawel Laszczak wrote:
> This patch introduce new Cadence USBSS DRD driver to Linux kernel.
> 
> The Cadence USBSS DRD Controller is a highly configurable IP Core which
> can be instantiated as Dual-Role Device (DRD), Peripheral Only and
> Host Only (XHCI)configurations.
> 
> The current driver has been validated with FPGA platform. We have
> support for PCIe bus, which is used on FPGA prototyping.
> 
> The host side of USBSS-DRD controller is compliant with XHCI
> specification, so it works with standard XHCI Linux driver.
> 
> Signed-off-by: Pawel Laszczak 
> ---
>  drivers/usb/Kconfig|2 +
>  drivers/usb/Makefile   |2 +
>  drivers/usb/cdns3/Kconfig  |   46 +
>  drivers/usb/cdns3/Makefile |   17 +
>  drivers/usb/cdns3/cdns3-pci-wrap.c |  203 +++
>  drivers/usb/cdns3/core.c   |  554 +++
>  drivers/usb/cdns3/core.h   |  109 ++
>  drivers/usb/cdns3/debug.h  |  171 ++
>  drivers/usb/cdns3/debugfs.c|   87 ++
>  drivers/usb/cdns3/drd.c|  390 +
>  drivers/usb/cdns3/drd.h|  166 ++
>  drivers/usb/cdns3/ep0.c|  914 +++
>  drivers/usb/cdns3/gadget-export.h  |   28 +
>  drivers/usb/cdns3/gadget.c | 2338 
>  drivers/usb/cdns3/gadget.h | 1321 
>  drivers/usb/cdns3/host-export.h|   28 +
>  drivers/usb/cdns3/host.c   |   71 +
>  drivers/usb/cdns3/trace.c  |   11 +
>  drivers/usb/cdns3/trace.h  |  493 ++
>  19 files changed, 6951 insertions(+)
>  create mode 100644 drivers/usb/cdns3/Kconfig
>  create mode 100644 drivers/usb/cdns3/Makefile
>  create mode 100644 drivers/usb/cdns3/cdns3-pci-wrap.c
>  create mode 100644 drivers/usb/cdns3/core.c
>  create mode 100644 drivers/usb/cdns3/core.h
>  create mode 100644 drivers/usb/cdns3/debug.h
>  create mode 100644 drivers/usb/cdns3/debugfs.c
>  create mode 100644 drivers/usb/cdns3/drd.c
>  create mode 100644 drivers/usb/cdns3/drd.h
>  create mode 100644 drivers/usb/cdns3/ep0.c
>  create mode 100644 drivers/usb/cdns3/gadget-export.h
>  create mode 100644 drivers/usb/cdns3/gadget.c
>  create mode 100644 drivers/usb/cdns3/gadget.h
>  create mode 100644 drivers/usb/cdns3/host-export.h
>  create mode 100644 drivers/usb/cdns3/host.c
>  create mode 100644 drivers/usb/cdns3/trace.c
>  create mode 100644 drivers/usb/cdns3/trace.h
> 
> diff --git a/drivers/usb/Kconfig b/drivers/usb/Kconfig
> index e4b27413f528..c2e78882f8c2 100644
> --- a/drivers/usb/Kconfig
> +++ b/drivers/usb/Kconfig
> @@ -113,6 +113,8 @@ source "drivers/usb/usbip/Kconfig"
>  
>  endif
>  
> +source "drivers/usb/cdns3/Kconfig"
> +
>  source "drivers/usb/mtu3/Kconfig"
>  
>  source "drivers/usb/musb/Kconfig"
> diff --git a/drivers/usb/Makefile b/drivers/usb/Makefile
> index 7d1b8c82b208..ab125b966cac 100644
> --- a/drivers/usb/Makefile
> +++ b/drivers/usb/Makefile
> @@ -12,6 +12,8 @@ obj-$(CONFIG_USB_DWC3)  += dwc3/
>  obj-$(CONFIG_USB_DWC2)   += dwc2/
>  obj-$(CONFIG_USB_ISP1760)+= isp1760/
>  
> +obj-$(CONFIG_USB_CDNS3)  += cdns3/
> +
>  obj-$(CONFIG_USB_MON)+= mon/
>  obj-$(CONFIG_USB_MTU3)   += mtu3/
>  
> diff --git a/drivers/usb/cdns3/Kconfig b/drivers/usb/cdns3/Kconfig
> new file mode 100644
> index ..d0331613a355
> --- /dev/null
> +++ b/drivers/usb/cdns3/Kconfig
> @@ -0,0 +1,46 @@
> +config USB_CDNS3
> + tristate "Cadence USB3 Dual-Role Controller"
> + depends on USB_SUPPORT && (USB || USB_GADGET) && HAS_DMA
> + select USB_XHCI_PLATFORM if USB_XHCI_HCD
> + select USB_ROLE_SWITCH
> + help
> +   Say Y here if your system has a Cadence USB3 dual-role controller.
> +   It supports: dual-role switch, Host-only, and Peripheral-only.
> +
> +   If you choose to build this driver is a dynamically linked
> +   as module, the module will be called cdns3.ko.
> +
> +if USB_CDNS3
> +
> +config USB_CDNS3_GADGET
> + bool "Cadence USB3 device controller"
> + depends on USB_GADGET=y || USB_GADGET=USB_CDNS3
> + help
> +   Say Y here to enable device controller functionality of the
> +   Cadence USBSS-DEV driver.
> +
> +   This controller supports FF, HS and SS mode. It doesn't support

s/FF/FS

> +   LS and SSP mode.
> +
> +config USB_CDNS3_HOST
> + bool "Cadence USB3 host controller"
> + depends on USB=y || USB=USB_CDNS3
> + help
> +   Say Y here to enable host controller functionality of the
> +   Cadence driver.
> +
> +   Host controller is compliant with XHCI so it will use
> +   standard XHCI driver.
> +
> +config USB_CDNS3_PCI_WRAP
> + tristate "Cadence USB3 support on PCIe-based platforms"
> + depends on USB_PCI && ACPI
> + default USB_CDNS3
> + help
> +   If you're using the USBSS Core IP with a PCIe, please say
> +   'Y' or 'M' here.
> +
> +   If you choose to build this driver