Re: [PATCH 1/2] usb: cdns3: fix cdns3_core_init_role()

[Roger Quadros]

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 <rogerq@xxxxxx>
> > Signed-off-by: Sekhar Nori <nsekhar@xxxxxx>
> > ---
> > 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.

> 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 <pawell@xxxxxxxxxxx>
> Tested-by: Pawel Laszczak <pawell@xxxxxxxxxxx>
>
> --
> Regards,
> Pawel

--
cheers,
-roger
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki