Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
From:   Pawel Laszczak <pawell@cadence.com>
To:     Roger Quadros <rogerq@ti.com>,
        "felipe.balbi@linux.intel.com" <felipe.balbi@linux.intel.com>,
        "gregkh@linuxfoundation.org" <gregkh@linuxfoundation.org>
CC:     "peter.chen@nxp.com" <peter.chen@nxp.com>,
        "nsekhar@ti.com" <nsekhar@ti.com>,
        Rahul Kumar <kurahul@cadence.com>,
        "linux-usb@vger.kernel.org" <linux-usb@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: RE: [PATCH 1/2] usb: cdns3: fix cdns3_core_init_role()
Thread-Topic: [PATCH 1/2] usb: cdns3: fix cdns3_core_init_role()
Thread-Index: AQHVfQkEuRq610s5UEaj9CRQS2FO1adctV7A
Date:   Wed, 16 Oct 2019 04:32:29 +0000
Message-ID: <BYAPR07MB470974496C7C59FDE615E5F3DD920@BYAPR07MB4709.namprd07.prod.outlook.com>
References: <20191007121601.25996-1-rogerq@ti.com>
 <20191007121601.25996-2-rogerq@ti.com>
In-Reply-To: <20191007121601.25996-2-rogerq@ti.com>
Accept-Language: en-US
Content-Language: en-US
received-spf: None (protection.outlook.com: cadence.com does not designate
 permitted sender hosts)
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: cb9e288f-72d9-4991-fc10-08d751f1dadf
X-MS-Exchange-CrossTenant-originalarrivaltime: 16 Oct 2019 04:32:29.5405
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: d36035c5-6ce6-4662-a3dc-e762e61ae4c9
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: n4lGeaBBakni/hFevM6Q6bijcvSayzQJl9BkP6UC6Jg1lLBSKXSuza6vf1iHr+SMuHRdVoUjUVvocqCPtVX0JI5CHzE4Bpz8SVPfyF3Fcl0=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BYAPR07MB4565
X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.95,1.0.8
 definitions=2019-10-16_01:2019-10-15,2019-10-16 signatures=0
X-Proofpoint-Spam-Details: rule=outbound_check_notspam policy=outbound_check score=0 bulkscore=0
 adultscore=0 mlxlogscore=999 suspectscore=0 phishscore=0 spamscore=0
 mlxscore=0 impostorscore=0 lowpriorityscore=0 priorityscore=1501
 malwarescore=0 clxscore=1015 classifier=spam adjust=0 reason=mlx
 scancount=1 engine=8.12.0-1908290000 definitions=main-1910160041
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

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@ti.com>
>Signed-off-by: Sekhar Nori <nsekhar@ti.com>
>---
> 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 !=3D USB_DR_MODE_OTG) {
>+	/* Initialize idle role to start with */
>+	ret =3D 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
some line before after returning form cdns3_drd_update_mode and in conseque=
nce=20
it jump to err label.=20

Maybe for better readability it this condition should be treated here also =
as error.

>+	case USB_DR_MODE_OTG:
> 		ret =3D cdns3_hw_role_switch(cdns);
> 		if (ret)
> 			goto err;
>+		break;
>+	case USB_DR_MODE_PERIPHERAL:
>+		ret =3D cdns3_role_start(cdns, USB_ROLE_DEVICE);
>+		if (ret)
>+			goto err;
>+		break;
>+	case USB_DR_MODE_HOST:
>+		ret =3D cdns3_role_start(cdns, USB_ROLE_HOST);
>+		if (ret)
>+			goto err;
>+		break;
> 	}
>
> 	return ret;

Reviewed-by: Pawel Laszczak <pawell@cadence.com>
Tested-by: Pawel Laszczak <pawell@cadence.com>

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