RE: [RFC PATCH v2 04/15] usb:cdns3: Driver initialization code.

[Pawel Laszczak <pawell@cadence.com>]

Hi

+ Tomek

>> > + * Cadence USBSS DRD Driver.
>> > + *
>> > + * Copyright (C) 2018 Cadence.
>> > + *
>> > + * Author: Peter Chen <peter.chen@nxp.com>
>> > + *         Pawel Laszczak <pawell@cadence.com>
>> > + */
>> > +
>> > +#include <linux/module.h>
>> > +#include <linux/kernel.h>
>> > +#include <linux/platform_device.h>
>> > +#include <linux/interrupt.h>
>> > +#include <linux/io.h>
>> > +#include <linux/pm_runtime.h>
>> > +
>> > +#include "gadget.h"
>> > +#include "core.h"
>> > +
>> > +static inline struct cdns3_role_driver *cdns3_get_current_role_driver(struct cdns3 *cdns)
>> > +{
>> > +     WARN_ON(cdns->role >= CDNS3_ROLE_END || !cdns->roles[cdns->role]);
>> > +     return cdns->roles[cdns->role];
>> > +}
>> > +
>> > +static inline int cdns3_role_start(struct cdns3 *cdns, enum cdns3_roles role)
>> > +{
>> > +     int ret;
>> > +
>> > +     if (role >= CDNS3_ROLE_END)
>>
>> WARN_ON()?
>>
>> > +             return 0;
>> > +
>> > +     if (!cdns->roles[role])
>> > +             return -ENXIO;
>> > +
>> > +     mutex_lock(&cdns->mutex);
>> > +     cdns->role = role;
>> > +     ret = cdns->roles[role]->start(cdns);
>> > +     mutex_unlock(&cdns->mutex);
>> > +     return ret;
>> > +}
>> > +
>> > +static inline void cdns3_role_stop(struct cdns3 *cdns)
>> > +{
>> > +     enum cdns3_roles role = cdns->role;
>> > +
>> > +     if (role == CDNS3_ROLE_END)
>>
>> WARN_ON(role >= CNDS3_ROLE_END) ?
>>
>> > +             return;
>> > +
>> > +     mutex_lock(&cdns->mutex);
>> > +     cdns->roles[role]->stop(cdns);
>> > +     cdns->role = CDNS3_ROLE_END;
>>
>> Why change the role here? You are just stopping the role not changing it.
>> I think cdns->role should remain unchanged, so we can call cdns3_role_start()
>> if required without error.
>>
>
>The current version of this IP has some issues to detect vbus status correctly,
>we have to force vbus status accordingly, so we need a status to indicate
>vbus disconnection, and add some code to let controller know vbus
>removal, in that case, the controller's state machine can be correct.
>So, we increase one role 'CDNS3_ROLE_END' to for this purpose.
>
>CDNS3_ROLE_GADGET: gadget mode and VBUS on
>CDNS3_ROLE_HOST: host mode and VBUS on
>CDNS3_ROLE_END: VBUS off, eg either host or device cable on the port.

>
>So, we may start role from CDNS3_ROLE_END at probe when nothing is connected,
>and need to set role as CDNS3_ROLE_END at ->stop for further handling at
>role switch routine.
>
>> > +     mutex_unlock(&cdns->mutex);
>> > +}
>> > +
>> > +static enum cdns3_roles cdns3_get_role(struct cdns3 *cdns)
>> > +{
>> > +     if (cdns->roles[CDNS3_ROLE_HOST] && cdns->roles[CDNS3_ROLE_GADGET]) {
>> > +             //TODO: implements selecting device/host mode
>> > +             return CDNS3_ROLE_HOST;
>> > +     }
>> > +     return cdns->roles[CDNS3_ROLE_HOST]
>> > +             ? CDNS3_ROLE_HOST
>> > +             : CDNS3_ROLE_GADGET;
>>
>> Why not just
>>         return cdns->role;
>>
>> I'm wondering if we really need this function.
>
>cdns->role gets from cdns3_get_role, and this API tells role at the runtime.
>If both roles are supported, the role is decided by external
>conditions, eg, vbus/id
>or external connector. If only single role is supported, only one role structure
>is allocated, cdns->roles[CDNS3_ROLE_HOST] or cdns->roles[CDNS3_ROLE_GADGET]
>
>> > +}
>>
>> > +
>> > +/**
>> > + * cdns3_core_init_role - initialize role of operation
>> > + * @cdns: Pointer to cdns3 structure
>> > + *
>> > + * Returns 0 on success otherwise negative errno
>> > + */
>> > +static int cdns3_core_init_role(struct cdns3 *cdns)
>> > +{
>> > +     struct device *dev = cdns->dev;
>> > +     enum usb_dr_mode dr_mode;
>> > +
>> > +     dr_mode = usb_get_dr_mode(dev);
>> > +     cdns->role = CDNS3_ROLE_END;
>> > +
>> > +     /*
>> > +      * If driver can't read mode by means of usb_get_dr_mdoe function then
>> > +      * chooses mode according with Kernel configuration. This setting
>> > +      * can be restricted later depending on strap pin configuration.
>> > +      */
>> > +     if (dr_mode == USB_DR_MODE_UNKNOWN) {
>> > +             if (IS_ENABLED(CONFIG_USB_CDNS3_HOST) &&
>> > +                 IS_ENABLED(CONFIG_USB_CDNS3_GADGET))
>> > +                     dr_mode = USB_DR_MODE_OTG;
>> > +             else if (IS_ENABLED(CONFIG_USB_CDNS3_HOST))
>> > +                     dr_mode = USB_DR_MODE_HOST;
>> > +             else if (IS_ENABLED(CONFIG_USB_CDNS3_GADGET))
>> > +                     dr_mode = USB_DR_MODE_PERIPHERAL;
>> > +     }
>> > +
>> > +     if (dr_mode == USB_DR_MODE_OTG || dr_mode == USB_DR_MODE_HOST) {
>> > +             //TODO: implements host initialization
>>
>>                 /* TODO: Add host role */ ?
>>
>> > +     }
>> > +
>> > +     if (dr_mode == USB_DR_MODE_OTG || dr_mode == USB_DR_MODE_PERIPHERAL) {
>> > +             //TODO: implements device initialization
>>
>>                 /* TODO: Add device role */ ?
>>
>
>Yes, it needs to allocate cdns->roles[CDNS3_ROLE_HOST] and
>cdns->roles[CDNS3_ROLE_GADGET].
>
>> > +     }
>> > +
>> > +     if (!cdns->roles[CDNS3_ROLE_HOST] && !cdns->roles[CDNS3_ROLE_GADGET]) {
>> > +             dev_err(dev, "no supported roles\n");
>> > +             return -ENODEV;
>> > +     }
>> > +
>> > +     cdns->dr_mode = dr_mode;
>
>Pawel, why dr_mode needs to be introduced?

cdns->dr_mode - it's the mode depending on configuration. 
We have device tree from which we can read dr_mode. It can be Host, Device or OTG - the maximum is OTG.
Additionally we have  strap pin that allow to force selected mode. 
Additionally mode can be limited by kernel configuration. 

So, I assume that controller can support OTG (HOST+DEVICE) , but it can be restricted by kernel configuration, strap pin or DTS (dr_mode property). 

When this configuration is restricted by one of these then if dr_mode is :
Host - we simply load XHCI driver
Device  - we start Device part
OTG - we based on ID pin

Eg. 
Controller support OTG mode but STRAP  is set to Host only then  dr_mode is set to  host
Controller support OTG mode but STRAP  is set to Device only then  dr_mode is set  to device 
Controller support OTG mode and STRAP  is set to OTG then  dr_mode is set  to OTG and  it can support Device only, Host only or OTG mode.

Additionally I'm introduce dr_mode that can be changed from user space. It can use for testing purpose. 
So even dr_mode  = OTG then we can force switching role from user space. 

I'm using it for testing. 

So additionally in cdns3 object we have desired_dr_mode and current_dr_mode. 

>
>> > +     return 0;
>> > +}
>> > +
>> > +/**
>> > + * cdns3_irq - interrupt handler for cdns3 core device
>> > + *
>> > + * @irq: irq number for cdns3 core device
>> > + * @data: structure of cdns3
>> > + *
>> > + * Returns IRQ_HANDLED or IRQ_NONE
>> > + */
>> > +static irqreturn_t cdns3_irq(int irq, void *data)
>> > +{
>> > +     struct cdns3 *cdns = data;
>> > +     irqreturn_t ret = IRQ_NONE;
>> > +
>> > +     /* Handle device/host interrupt */
>> > +     if (cdns->role != CDNS3_ROLE_END)
>>
>> Is it because of this that you need to set role to END at role_stop?
>> I think it is better to add a state variable to struct cdns3_role_driver, so we can
>> check if it is active or stopped.
>>
>> e.g.
>>         if (cdns3_get_current_role_driver(cdns)->state == CDNS3_ROLE_STATE_ACTIVE)
>>
>> > +             ret = cdns3_get_current_role_driver(cdns)->irq(cdns);
>> > +
>> > +     return ret;
>> > +}
>> > +
>
> CDNS3_ROLE_END is introduced from above comments, we don't
>need another flag for it.
>If cdns->role == CDNS3_ROLE_END, it handles VBUS and ID interrupt.
>
>> > +static void cdns3_remove_roles(struct cdns3 *cdns)
>>
>> Should this be called cdns3_exit_roles() to be opposite of cdns3_init_roles()?
>>
>
>It is planed to called when at ->remove.
>> > +{
>> > +     //TODO: implements this function
>> > +}
>>
>> > +
>> > +static int cdns3_do_role_switch(struct cdns3 *cdns, enum cdns3_roles role)
>> > +{
>> > +     enum cdns3_roles current_role;
>> > +     int ret = 0;
>> > +
>> > +     current_role = cdns->role;
>> > +
>> > +     if (role == CDNS3_ROLE_END)
>> > +             return 0;
>>
>> role == END looks like error state. and it should never happen.
>> WARN here?
>>
>
>See my comments above.
>
>> > +
>> > +     dev_dbg(cdns->dev, "Switching role");
>> > +
>>
>> Don't you have to stop the previous role before starting the new role?
>>
>
>Yes, it is needed. Pawel may simply some flows to suit his platform.

>
>> > +     ret = cdns3_role_start(cdns, 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);
>> > +     }
>> > +
>> > +     return ret;
>> > +}
>> > +
>> > +/**
>> > + * cdns3_role_switch - work queue handler for role switch
>> > + *
>> > + * @work: work queue item structure
>> > + *
>> > + * Handles below events:
>> > + * - Role switch for dual-role devices
>> > + * - CDNS3_ROLE_GADGET <--> CDNS3_ROLE_END for peripheral-only devices
>> > + */
>> > +static void cdns3_role_switch(struct work_struct *work)
>> > +{
>> > +     enum cdns3_roles role = CDNS3_ROLE_END;
>> > +     struct cdns3 *cdns;
>> > +     bool device, host;
>> > +
>> > +     cdns = container_of(work, struct cdns3, role_switch_wq);
>> > +
>> > +     //TODO: implements this functions.
>> > +     //host = cdns3_is_host(cdns);
>> > +     //device = cdns3_is_device(cdns);
>> > +     host = 1;
>> > +     device = 0;
>> > +
>> > +     if (host)
>> > +             role = CDNS3_ROLE_HOST;
>> > +     else if (device)
>> > +             role = CDNS3_ROLE_GADGET;
>> > +
>> > +     if (cdns->desired_dr_mode == cdns->current_dr_mode &&
>> > +         cdns->role == role)
>> > +             return;
>> > +
>>
>> I think all the below code can be moved to cdns3_do_role_switch().
>>
>> > +     pm_runtime_get_sync(cdns->dev);
>> > +     cdns3_role_stop(cdns);
>> > +
>> > +     if (host) {
>> > +             if (cdns->roles[CDNS3_ROLE_HOST])
>> > +                     cdns3_do_role_switch(cdns, CDNS3_ROLE_HOST);
>> > +             pm_runtime_put_sync(cdns->dev);
>> > +             return;
>> > +     }
>> > +
>> > +     if (device)
>> > +             cdns3_do_role_switch(cdns, CDNS3_ROLE_GADGET);
>> > +     else
>> > +             cdns3_do_role_switch(cdns, CDNS3_ROLE_END);
>> > +
>> > +     pm_runtime_put_sync(cdns->dev);
>> > +}
>> > +
>> > +/**
>> > + * 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)
>> > +{
>> > +     struct device *dev = &pdev->dev;
>> > +     struct resource *res;
>> > +     struct cdns3 *cdns;
>> > +     void __iomem *regs;
>> > +     int ret;
>> > +
>> > +     cdns = devm_kzalloc(dev, sizeof(*cdns), GFP_KERNEL);
>> > +     if (!cdns)
>> > +             return -ENOMEM;
>> > +
>> > +     cdns->dev = dev;
>> > +
>> > +     platform_set_drvdata(pdev, cdns);
>> > +
>> > +     res = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
>> > +     if (!res) {
>> > +             dev_err(dev, "missing IRQ\n");
>> > +             return -ENODEV;
>> > +     }
>> > +     cdns->irq = res->start;
>> > +
>> > +     /*
>> > +      * Request memory region
>> > +      * region-0: xHCI
>> > +      * region-1: Peripheral
>> > +      * region-2: OTG registers
>> > +      */
>>
>> The memory region order is different from the dt-binding.
>> There it is OTG, host(xhci), device (peripheral).
>>
>> > +     res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> > +     regs = devm_ioremap_resource(dev, res);
>> > +
>> > +     if (IS_ERR(regs))
>> > +             return PTR_ERR(regs);
>> > +     cdns->xhci_regs = regs;
>> > +     cdns->xhci_res = res;
>> > +
>> > +     res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
>> > +     regs = devm_ioremap_resource(dev, res);
>> > +     if (IS_ERR(regs))
>> > +             return PTR_ERR(regs);
>> > +     cdns->dev_regs  = regs;
>> > +
>> > +     res = platform_get_resource(pdev, IORESOURCE_MEM, 2);
>> > +     regs = devm_ioremap_resource(dev, res);
>> > +     if (IS_ERR(regs))
>> > +             return PTR_ERR(regs);
>> > +     cdns->otg_regs = regs;
>> > +
>> > +     mutex_init(&cdns->mutex);
>> > +
>> > +     cdns->phy = devm_phy_get(dev, "cdns3,usbphy");
>>
>> "cdns3,usbphy" is not documented in dt-binding.
>>
>> > +     if (IS_ERR(cdns->phy)) {
>> > +             dev_info(dev, "no generic phy found\n");
>> > +             cdns->phy = NULL;
>> > +             /*
>> > +              * fall through here!
>> > +              * if no generic phy found, phy init
>> > +              * should be done under boot!
>> > +              */
>>
>> No you shouldn't fall through always if it is an error condition.
>> Something like this should work better.
>>
>>         if (IS_ERR(cnds->phy)) {
>>                 ret = PTR_ERR(cdns->phy);
>>                 if (ret == -ENOSYS || ret == -ENODEV) {
>>                         cdns->phy = NULL;
>>                 } else if (ret == -EPROBE_DEFER) {
>>                         return ret;
>>                 } else {
>>                         dev_err(dev, "no phy found\n");
>>                         goto err0;
>>                 }
>>         }
>>
>> So if PHY was provided in DT, and PHY support/drivers is present
>> and error condition means something is wrong and we have to error out.
>>
>> > +     } else {
>> > +             phy_init(cdns->phy);
>> > +     }
>>
>> You can do phy_init() outside the else.
>>
>> > +
>> > +     ret = cdns3_core_init_role(cdns);
>> > +     if (ret)
>> > +             goto err1;
>> > +
>> > +     INIT_WORK(&cdns->role_switch_wq, cdns3_role_switch);
>> > +     if (ret)
>> > +             goto err2;
>> > +
>> > +     if (ret)
>> > +             goto err2;
>> > +
>> > +     cdns->role = cdns3_get_role(cdns);
>>
>> I think this should move to cdns3_core_init_role().
>>
>
>I agree.
>
>> > +
>> > +     ret = devm_request_irq(dev, cdns->irq, cdns3_irq, IRQF_SHARED,
>> > +                            dev_name(dev), cdns);
>> > +
>> > +     if (ret)
>> > +             goto err2;
>>
>> How about moving request_irq to before cdsn3_core_init_role()?
>>
>> Then you can move cdns3_role_start() as well to core_init_role().
>>
>
>Usually, we request irq after hardware initialization has finished, if not,
>there may unexpected interrupt.

>
>Peter
>
>> > +
>> > +     ret = cdns3_role_start(cdns, cdns->role);
>> > +     if (ret) {
>> > +             dev_err(dev, "can't start %s role\n",
>> > +                     cdns3_get_current_role_driver(cdns)->name);
>> > +             goto err2;
>> > +     }
>> > +
>> > +     device_set_wakeup_capable(dev, true);
>> > +     pm_runtime_set_active(dev);
>> > +     pm_runtime_enable(dev);
>> > +
>> > +     /*
>> > +      * The controller needs less time between bus and controller suspend,
>> > +      * and we also needs a small delay to avoid frequently entering low
>> > +      * power mode.
>> > +      */
>> > +     pm_runtime_set_autosuspend_delay(dev, 20);
>> > +     pm_runtime_mark_last_busy(dev);
>> > +     pm_runtime_use_autosuspend(dev);
>> > +     dev_dbg(dev, "Cadence USB3 core: probe succeed\n");
>> > +
>> > +     return 0;
>> > +
>> > +err2:
>> > +     cdns3_remove_roles(cdns);
>> > +err1:
>>
>> phy_exit() ?
>>
>> > +     return ret;
>> > +}
>> > +
>> > +/**
>> > + * cdns3_remove - unbind drd driver and clean up
>> > + * @pdev: Pointer to Linux platform device
>> > + *
>> > + * Returns 0 on success otherwise negative errno
>> > + */
>> > +static int cdns3_remove(struct platform_device *pdev)
>> > +{
>> > +     struct cdns3 *cdns = platform_get_drvdata(pdev);
>> > +
>> > +     pm_runtime_get_sync(&pdev->dev);
>> > +     pm_runtime_disable(&pdev->dev);
>> > +     pm_runtime_put_noidle(&pdev->dev);
>> > +     cdns3_remove_roles(cdns);
>>
>> phy_exit() ?
>>
>> > +
>> > +     return 0;
>> > +}
>> > +
>> > +#ifdef CONFIG_OF
>> > +static const struct of_device_id of_cdns3_match[] = {
>> > +     { .compatible = "cdns,usb3" },
>> > +     { },
>> > +};
>> > +MODULE_DEVICE_TABLE(of, of_cdns3_match);
>> > +#endif
>> > +
>> > +#ifdef CONFIG_PM
>> > +
>> > +#ifdef CONFIG_PM_SLEEP
>> > +static int cdns3_suspend(struct device *dev)
>> > +{
>> > +     //TODO: implements this function
>> > +     return 0;
>> > +}
>> > +
>> > +static int cdns3_resume(struct device *dev)
>> > +{
>> > +     //TODO: implements this function
>> > +     return 0;
>> > +}
>> > +#endif /* CONFIG_PM_SLEEP */
>> > +static int cdns3_runtime_suspend(struct device *dev)
>> > +{    //TODO: implements this function
>> > +     return 0;
>> > +}
>> > +
>> > +static int cdns3_runtime_resume(struct device *dev)
>> > +{
>> > +     //TODO: implements this function
>> > +     return 0;
>> > +}
>> > +#endif /* CONFIG_PM */
>> > +
>> > +static const struct dev_pm_ops cdns3_pm_ops = {
>> > +     SET_SYSTEM_SLEEP_PM_OPS(cdns3_suspend, cdns3_resume)
>> > +     SET_RUNTIME_PM_OPS(cdns3_runtime_suspend, cdns3_runtime_resume, NULL)
>> > +};
>> > +
>> > +static struct platform_driver cdns3_driver = {
>> > +     .probe          = cdns3_probe,
>> > +     .remove         = cdns3_remove,
>> > +     .driver         = {
>> > +             .name   = "cdns-usb3",
>> > +             .of_match_table = of_match_ptr(of_cdns3_match),
>> > +             .pm     = &cdns3_pm_ops,
>> > +     },
>> > +};
>> > +
>> > +static int __init cdns3_driver_platform_register(void)
>> > +{
>> > +     return platform_driver_register(&cdns3_driver);
>> > +}
>> > +module_init(cdns3_driver_platform_register);
>> > +
>> > +static void __exit cdns3_driver_platform_unregister(void)
>> > +{
>> > +     platform_driver_unregister(&cdns3_driver);
>> > +}
>> > +module_exit(cdns3_driver_platform_unregister);
>> > +
>> > +MODULE_ALIAS("platform:cdns3");
>> > +MODULE_AUTHOR("Pawel Laszczak <pawell@cadence.com>");
>> > +MODULE_LICENSE("GPL v2");
>> > +MODULE_DESCRIPTION("Cadence USB3 DRD Controller Driver");
>> > diff --git a/drivers/usb/cdns3/core.h b/drivers/usb/cdns3/core.h
>> > new file mode 100644
>> > index 000000000000..7c8204fe4d3d
>> > --- /dev/null
>> > +++ b/drivers/usb/cdns3/core.h
>> > @@ -0,0 +1,100 @@
>> > +/* SPDX-License-Identifier: GPL-2.0 */
>> > +/*
>> > + * Cadence USBSS DRD Driver.
>> > + *
>> > + * Copyright (C) 2017 NXP
>> > + * Copyright (C) 2018 Cadence.
>> > + *
>> > + * Authors: Peter Chen <peter.chen@nxp.com>
>> > + *          Pawel Laszczak <pawell@cadence.com>
>> > + */
>> > +#include <linux/usb/otg.h>
>> > +
>> > +#ifndef __LINUX_CDNS3_CORE_H
>> > +#define __LINUX_CDNS3_CORE_H
>> > +
>> > +struct cdns3;
>> > +enum cdns3_roles {
>> > +     CDNS3_ROLE_HOST = 0,
>> > +     CDNS3_ROLE_GADGET,
>> > +     CDNS3_ROLE_END,
>> > +};
>> > +
>> > +/**
>> > + * struct cdns3_role_driver - host/gadget role driver
>> > + * @start: start this role
>> > + * @stop: stop this role
>> > + * @suspend: suspend callback for this role
>> > + * @resume: resume callback for this role
>> > + * @irq: irq handler for this role
>> > + * @name: role name string (host/gadget)
>> > + */
>> > +struct cdns3_role_driver {
>> > +     int (*start)(struct cdns3 *cdns);
>> > +     void (*stop)(struct cdns3 *cdns);
>> > +     int (*suspend)(struct cdns3 *cdns, bool do_wakeup);
>> > +     int (*resume)(struct cdns3 *cdns, bool hibernated);
>> > +     irqreturn_t (*irq)(struct cdns3 *cdns);
>> > +     const char *name;
>> > +};
>> > +
>> > +#define CDNS3_NUM_OF_CLKS    5
>> > +/**
>> > + * struct cdns3 - Representation of Cadence USB3 DRD controller.
>> > + * @dev: pointer to Cadence device struct
>> > + * @xhci_regs: pointer to base of xhci registers
>> > + * @xhci_res: the resource for xhci
>> > + * @dev_regs: pointer to base of dev registers
>> > + * @otg_regs: pointer to base of otg registers
>> > + * @irq: irq number for controller
>> > + * @roles: array of supported roles for this controller
>> > + * @role: current role
>> > + * @host_dev: the child host device pointer for cdns3 core
>> > + * @gadget_dev: the child gadget device pointer for cdns3 core
>> > + * @usb: phy for this controller
>> > + * @role_switch_wq: work queue item for role switch
>> > + * @in_lpm: the controller in low power mode
>> > + * @wakeup_int: the wakeup interrupt
>> > + * @mutex: the mutex for concurrent code at driver
>> > + * @dr_mode: supported mode of operation it can be only Host, only Device
>> > + *           or OTG mode that allow to switch between Device and Host mode.
>> > + *           This field based on hardware configuration and cant't be changed.
>>
>> But dr_mode can be forced in device-tree. So it isn't really only hardware configuration.
>>
>> > + * @current_dr_role: current mode of operation when in dual-role mode
>> > + * @desired_dr_role: desired mode of operation when in dual-role mode.
>> > + *           This value can be changed during runtime.
>> > + *           Available options depends on  dr_mode:
>> > + *           dr_mode                 |  desired_dr_role and current_dr_role
>> > + *           ----------------------------------------------------------------
>> > + *           USB_DR_MODE_HOST        | only USB_DR_MODE_HOST
>> > + *           USB_DR_MODE_PERIPHERAL  | only USB_DR_MODE_PERIPHERAL
>> > + *           USB_DR_MODE_OTG         | only USB_DR_MODE_HOST
>> > + *           USB_DR_MODE_OTG         | only USB_DR_MODE_PERIPHERAL
>> > + *           USB_DR_MODE_OTG         | USB_DR_MODE_OTG
>>
>> Do you need to update the right hand side to reflect ROLEs instead of MODE?
>>
>> > + *
>> > + *           Desired_dr_role can be changed by means of debugfs.
>> > + * @root: debugfs root folder pointer
>> > + */
>> > +struct cdns3 {
>> > +     struct device                   *dev;
>> > +     void __iomem                    *xhci_regs;
>> > +     struct resource                 *xhci_res;
>> > +     struct cdns3_usb_regs __iomem   *dev_regs;
>> > +     struct cdns3_otg_regs           *otg_regs;
>> > +     int irq;
>> > +     struct cdns3_role_driver        *roles[CDNS3_ROLE_END];
>> > +     enum cdns3_roles                role;
>> > +     struct device                   *host_dev;
>> > +     struct device                   *gadget_dev;
>> > +     struct phy                      *phy;
>> > +     struct work_struct              role_switch_wq;
>> > +     int                             in_lpm:1;
>> > +     int                             wakeup_int:1;
>> > +     /* mutext used in workqueue*/
>> > +     struct mutex                    mutex;
>> > +     enum usb_dr_mode                dr_mode;
>> > +     enum usb_dr_mode                current_dr_mode;
>> > +     enum usb_dr_mode                desired_dr_mode;
>> > +     struct dentry                   *root;
>> > +};
>> > +
>> > +#endif /* __LINUX_CDNS3_CORE_H */
>> >
>>
>> cheers,
>> -roger
>> --
>> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
>> Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki