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
