> > >Hi Pawel, > >On 16/10/2019 07:32, Pawel Laszczak wrote: >> Hi Roger >> >>> >>> At startup we should trigger the HW state machine >>> only if it is OTG mode. Otherwise we should just >>> start the respective role. >>> >>> Initialize idle role by default. If we don't do this then >>> cdns3_idle_role_stop() is not called when switching to >>> host/device role and so lane switch mechanism >>> doesn't work. This results to super-speed device not working >>> in one orientation if it was plugged before driver probe. >>> >>> Signed-off-by: Roger Quadros <[email protected]> >>> Signed-off-by: Sekhar Nori <[email protected]> >>> --- >>> drivers/usb/cdns3/core.c | 20 +++++++++++++++++++- >>> 1 file changed, 19 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/usb/cdns3/core.c b/drivers/usb/cdns3/core.c >>> index 06f1e105be4e..1109dc5a4c39 100644 >>> --- a/drivers/usb/cdns3/core.c >>> +++ b/drivers/usb/cdns3/core.c >>> @@ -160,10 +160,28 @@ static int cdns3_core_init_role(struct cdns3 *cdns) >>> if (ret) >>> goto err; >>> >>> - if (cdns->dr_mode != USB_DR_MODE_OTG) { >>> + /* Initialize idle role to start with */ >>> + ret = cdns3_role_start(cdns, USB_ROLE_NONE); >>> + if (ret) >>> + goto err; >>> + >>> + switch (cdns->dr_mode) { >>> + case USB_DR_MODE_UNKNOWN: >> >> One note in this place. USB_DR_MODE_UNKNOWN is not possible in this place. >> If cdns->dr_mode will be USB_DR_MODE_UNKNOWN then driver returns -EINVAL > >At which place? I could not find.
In this patch we can't see this line: https://elixir.bootlin.com/linux/v5.4-rc2/source/drivers/usb/cdns3/core.c#L159 There is called cdns3_drd_update_mode(cdns); int cdns3_drd_update_mode(struct cdns3 *cdns) { int ret = 0; switch (cdns->dr_mode) { case USB_DR_MODE_PERIPHERAL: ret = cdns3_set_mode(cdns, USB_DR_MODE_PERIPHERAL); break; case USB_DR_MODE_HOST: ret = cdns3_set_mode(cdns, USB_DR_MODE_HOST); break; case USB_DR_MODE_OTG: ret = cdns3_init_otg_mode(cdns); break; default: dev_err(cdns->dev, "Unsupported mode of operation %d\n", cdns->dr_mode); return -EINVAL; } return ret; } After calling cdns3_drd_update_mode we have 2 first lines from this patch if (ret) goto err; Pawel > >> some line before after returning form cdns3_drd_update_mode and in >> consequence >> it jump to err label. >> >> Maybe for better readability it this condition should be treated here also >> as error. >> >>> + case USB_DR_MODE_OTG: >>> ret = cdns3_hw_role_switch(cdns); >>> if (ret) >>> goto err; >>> + break; >>> + case USB_DR_MODE_PERIPHERAL: >>> + ret = cdns3_role_start(cdns, USB_ROLE_DEVICE); >>> + if (ret) >>> + goto err; >>> + break; >>> + case USB_DR_MODE_HOST: >>> + ret = cdns3_role_start(cdns, USB_ROLE_HOST); >>> + if (ret) >>> + goto err; >>> + break; >>> } >>> >>> return ret; >> >> Reviewed-by: Pawel Laszczak <[email protected]> >> Tested-by: Pawel Laszczak <[email protected]> >> >> -- >> Regards, >> Pawel >> > >-- >cheers, >-roger >Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. >Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
