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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 6 Dec 2018 10:02:41 +0000
From:   Pawel Laszczak <pawell@...ence.com>
To:     Peter Chen <hzpeterchen@...il.com>, "rogerq@...com" <rogerq@...com>
CC:     "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        "linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>,
        lkml <linux-kernel@...r.kernel.org>,
        Alan Douglas <adouglas@...ence.com>,
        "jbergsagel@...com" <jbergsagel@...com>,
        "nsekhar@...com" <nsekhar@...com>, "nm@...com" <nm@...com>,
        Suresh Punnoose <sureshp@...ence.com>,
        "peter.chen@....com" <peter.chen@....com>,
        Pawel Jez <pjez@...ence.com>,
        Rahul Kumar <kurahul@...ence.com>,
        Tomasz Klimek <tklimek@...ence.com>
Subject: RE: [RFC PATCH v2 04/15] usb:cdns3: Driver initialization code.

Hi,

>>> > +
>>> > +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.
>
>Hi, Tomek do you have any comment for this.
>We have in RTL whole OTG machine and we can read all states.

It's not the IP issue, but with PHY. I told with Tomek and he confirmed this issue.
In my testing platform I use different phy version and I don't have such issue. 

CDNS3_ROLE_END stay in driver for compatibility with Peter PHY version. 

>From otg specification we have in otg_state such bits:
>5:3	host_otg_state	"Current state of the OTG Host FSM.
>3'b000 : H_IDLE
>3'b001 : H_VBUS_ON
>3'b010 : H_VBUS_FAILED
>3'b011 : H_OTG_HOST_MODE
>3'b100 : H_HOST_MODE
>3'b101 : H_SWITCH_TO_DEVICE
>3'b110 : H_A_SUSPEND
>3'b111 : H_WAIT_VBUS_FALL"	RO	0x0
>2:0	dev_otg_state	"Current state of the OTG Device FSM.
>3'b000 : DEV_IDLE
>3'b001 : DEV_MODE
>3'b010 : DEV_SRP
>3'b011 : DEV_WAIT_VBUS_FALL
>3'b100 : DEV_SWITCH_TO_HOST
>3'b101 : DEV_WAIT_FOR_CONN"
>
>>
>>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;

Cheers 
Pawel