From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
	id S1753436AbbIGJwD (ORCPT <rfc822;w@1wt.eu>);
	Mon, 7 Sep 2015 05:52:03 -0400
Received: from devils.ext.ti.com ([198.47.26.153]:56876 "EHLO
	devils.ext.ti.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1751018AbbIGJv7 (ORCPT
	<rfc822;linux-kernel@vger.kernel.org>);
	Mon, 7 Sep 2015 05:51:59 -0400
Subject: Re: [PATCH v4 13/13] usb: otg: Add dual-role device (DRD) support
To: Li Jun <b47624@freescale.com>
References: <1440422484-4737-1-git-send-email-rogerq@ti.com>
 <1440422484-4737-14-git-send-email-rogerq@ti.com>
 <20150907075347.GB30578@shlinux2>
CC: <stern@rowland.harvard.edu>, <balbi@ti.com>, <gregkh@linuxfoundation.org>,
        <peter.chen@freescale.com>, <dan.j.williams@intel.com>,
        <jun.li@freescale.com>, <mathias.nyman@linux.intel.com>,
        <tony@atomide.com>, <Joao.Pinto@synopsys.com>, <abrestic@chromium.org>,
        <linux-usb@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
        <linux-omap@vger.kernel.org>
From: Roger Quadros <rogerq@ti.com>
Message-ID: <55ED5E32.80203@ti.com>
Date: Mon, 7 Sep 2015 12:51:46 +0300
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101
 Thunderbird/38.2.0
MIME-Version: 1.0
In-Reply-To: <20150907075347.GB30578@shlinux2>
Content-Type: text/plain; charset="windows-1252"
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 07/09/15 10:53, Li Jun wrote:
> On Mon, Aug 24, 2015 at 04:21:24PM +0300, Roger Quadros wrote:
>> DRD mode is a reduced functionality OTG mode. In this mode
>> we don't support SRP, HNP and dynamic role-swap.
>>
>> In DRD operation, the controller mode (Host or Peripheral)
>> is decided based on the ID pin status. Once a cable plug (Type-A
>> or Type-B) is attached the controller selects the state
>> and doesn't change till the cable in unplugged and a different
>> cable type is inserted.
>>
>> As we don't need most of the complex OTG states and OTG timers
>> we implement a lean DRD state machine in usb-otg.c.
>> The DRD state machine is only interested in 2 hardware inputs
>> 'id' and 'b_sess_vld'.
>>
>> Signed-off-by: Roger Quadros <rogerq@ti.com>
>> ---
>>  drivers/usb/common/usb-otg.c | 178 +++++++++++++++++++++++++++++++++++++++++--
>>  include/linux/usb/otg-fsm.h  |   5 ++
>>  include/linux/usb/otg.h      |   2 +
>>  3 files changed, 177 insertions(+), 8 deletions(-)
>>
> 
> ... ...
> 
>> +/* Called when entering a DRD state */
>> +static void drd_set_state(struct otg_fsm *fsm, enum usb_otg_state new_state)
>> +{
>> +	struct usb_otg *otgd = container_of(fsm, struct usb_otg, fsm);
>> +
>> +	if (fsm->otg->state == new_state)
>> +		return;
>> +
>> +	fsm->state_changed = 1;
>> +	dev_dbg(otgd->dev, "otg: set state: %s\n",
>> +		usb_otg_state_string(new_state));
>> +	switch (new_state) {
>> +	case OTG_STATE_B_IDLE:
>> +		drd_set_protocol(fsm, PROTO_UNDEF);
> 
> You didn't address this comment for your previous version.
> 
> otg_drv_vbus(fsm, 0);
> 
>> +		break;
>> +	case OTG_STATE_B_PERIPHERAL:
>> +		drd_set_protocol(fsm, PROTO_GADGET);
> 
> otg_drv_vbus(fsm, 0);
> 
>> +		break;
>> +	case OTG_STATE_A_HOST:
> 
> otg_drv_vbus(fsm, 1);
> 

Sorry, I missed it. Will add in next version.

--
cheers,
-roger

>> +		drd_set_protocol(fsm, PROTO_HOST);
>> +		break;
>> +	case OTG_STATE_UNDEFINED:
>> +	case OTG_STATE_B_SRP_INIT:
>> +	case OTG_STATE_B_WAIT_ACON:
>> +	case OTG_STATE_B_HOST:
>> +	case OTG_STATE_A_IDLE:
>> +	case OTG_STATE_A_WAIT_VRISE:
>> +	case OTG_STATE_A_WAIT_BCON:
>> +	case OTG_STATE_A_SUSPEND:
>> +	case OTG_STATE_A_PERIPHERAL:
>> +	case OTG_STATE_A_WAIT_VFALL:
>> +	case OTG_STATE_A_VBUS_ERR:
>> +	default:
>> +		dev_warn(otgd->dev, "%s: otg: invalid state: %s\n",
>> +			 __func__, usb_otg_state_string(new_state));
>> +		break;
>> +	}
>> +
>> +	fsm->otg->state = new_state;
>> +}
>> +
> ... ...
>