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

[Pawel Laszczak]

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