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 for Android: free password hash cracker in your pocket

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

Message-ID: <BYAPR07MB47095578C205570DD97EBFE0DDA80@BYAPR07MB4709.namprd07.prod.outlook.com>
Date:   Wed, 5 Dec 2018 19:42:01 +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.

+ Tomek Klimek 

>> > +
>> > +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. 

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,