Date: Mon, 20 Jul 2015 09:45:01 +0800
From: Peter Chen <peter.chen@freescale.com>
To: Roger Quadros <rogerq@ti.com>
CC: Li Jun <b47624@freescale.com>, <stern@rowland.harvard.edu>, <balbi@ti.com>,
        <gregkh@linuxfoundation.org>, <dan.j.williams@intel.com>,
        <jun.li@freescale.com>, <mathias.nyman@linux.intel.com>,
        <tony@atomide.com>, <Joao.Pinto@synopsys.com>,
        <linux-usb@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
        <linux-omap@vger.kernel.org>
Subject: Re: [PATCH v3 07/11] usb: otg: add OTG core
Message-ID: <20150720014500.GC4584@shlinux2>
References: <1436350777-28056-1-git-send-email-rogerq@ti.com>
 <1436350777-28056-8-git-send-email-rogerq@ti.com>
 <20150717074832.GA4515@shlinux2>
 <55A8EFBA.1000504@ti.com>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <55A8EFBA.1000504@ti.com>
User-Agent: Mutt/1.5.21 (2010-09-15)
BN3PR0301MB1219: X-MS-Exchange-Organization-RulesExecuted
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 20 Jul 2015 02:54:27.8299
 (UTC)
X-MS-Exchange-CrossTenant-Id: 710a03f5-10f6-4d38-9ff4-a80b81da590d
X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=710a03f5-10f6-4d38-9ff4-a80b81da590d;Ip=[192.88.168.50];Helo=[tx30smr01.am.freescale.net]
X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN3PR0301MB1219
X-Microsoft-Exchange-Diagnostics: 1;BN3PR0301MB1220;2:0kYVDSNKkO3yZ8BimBp1IcMkImZf+FUGKQyowV7GAvYN08q6mcN02n36cNTHrZke;3:E+T69f42oMVtXKBtMfWUF8/6p7Ani5lUtY3GH9VUa7yWXagomQQP+Y+HBtIk/zj959MLR9eSSKUs6hyRabpTNDcYaEmYjTPUqzRmNFW8FVt0UdhKH28bkVbcXiJXV97b3WqQXBg9aScH6QZym1zZuJLcj5sIMnqqmFOW/B6M6NNVJdiuVoyfbHgd8KGqmcvDOvigNUvarD7xh1cemkvS/A1pZM3P1mKuyIPWujGONJk=;25:XM/OfbZ79B1nacNHIKx6vvJuNpqvJLZf+WW+Rw47kAEtdCRblKM8zbW2K3lLmbR/ceRNW86083lO7+p6vNUdc5oiRCqnJFCNcprUUollTTXJoleBriDgodBuYtquvmStvy4Yj2qBUzFsIEgWiaaNn64LbZSuystNDI6zEnYX4VB9rv0S6bwbPa1dCsLa9v1BrcC01wq97D/EDQA+RzuGeFjeA3cONMyZYrSfT2UL4Rouwe1gbsLZXsJY+XmWCVicRVk+C0XMCrEFXY8R+GVKsA==;23:rLZ92wMF1gl35lEA3wyE3+PoIQunhwGLBjf2sIpFZFRE5ufrBdeIVg3UWV5EWhWd2n1mvQMWqSLtp1aE+Nnxe7f7cv7IMiWMfQQ2ss4ZVrkhmf1lhXnxgATIgrrGis/c/NM3J4a7rOBau2b7XoNuGkDu9k6Twl0Za3i8vZiQ7OJIHpxG/8R4nVq2pZ8/RBOXkRUt+Va3isSePWKAj2UIfsWG+bVmHu+BpQFvmrFSk849Mf+PWHYScWW51lBXGXKE
X-OriginatorOrg: freescale.com
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
Content-Length: 2424
Lines: 73

On Fri, Jul 17, 2015 at 03:06:18PM +0300, Roger Quadros wrote:
> >> +
> >> +/**
> >> + * OTG FSM ops function to start/stop host
> >> + */
> >> +static int usb_otg_start_host(struct otg_fsm *fsm, int on)
> >> +{
> >> +	struct otg_data *otgd = container_of(fsm, struct otg_data, fsm);
> >> +	struct otg_hcd_ops *hcd_ops;
> >> +
> >> +	dev_dbg(otgd->dev, "otg: %s %d\n", __func__, on);
> >> +	if (!fsm->otg->host) {
> >> +		WARN_ONCE(1, "otg: fsm running without host\n");
> >> +		return 0;
> >> +	}
> >> +
> >> +	if (on) {
> >> +		/* OTG device operations */
> >> +		if (otgd->start_host)
> >> +			otgd->start_host(fsm, on);
> >> +
> >> +		/* start host */
> >> +		hcd_ops = otgd->primary_hcd.ops;
> >> +		hcd_ops->add(otgd->primary_hcd.hcd, otgd->primary_hcd.irqnum,
> >> +			     otgd->primary_hcd.irqflags);
> >> +		if (otgd->shared_hcd.hcd) {
> >> +			hcd_ops = otgd->shared_hcd.ops;
> >> +			hcd_ops->add(otgd->shared_hcd.hcd,
> >> +				     otgd->shared_hcd.irqnum,
> >> +				     otgd->shared_hcd.irqflags);
> >> +		}
> >> +	} else {
> >> +		/* stop host */
> >> +		if (otgd->shared_hcd.hcd) {
> >> +			hcd_ops = otgd->shared_hcd.ops;
> >> +			hcd_ops->remove(otgd->shared_hcd.hcd);
> >> +		}
> >> +		hcd_ops = otgd->primary_hcd.ops;
> >> +		hcd_ops->remove(otgd->primary_hcd.hcd);
> >> +
> >> +		/* OTG device operations */
> >> +		if (otgd->start_host)
> >> +			otgd->start_host(fsm, on);
> >> +	}
> >> +
> >> +	return 0;
> >> +}
> > 
> > I do not see much benefit by this override function, usb_add/remove_hcd
> > can be simply included by controller's start_host function, also there
> > maybe some additional operations after usb_add_hcd, but this override
> > function limit usb_add_hcd() is the last step.
> 
> I had tried host start/stop way before but Alan's suggestion was to use
> bind/unbind the host controller completely as that is much simpler
> 
> [1] http://article.gmane.org/gmane.linux.usb.general/123842
> 

Jun may want we have some ways to override otg_fsm->ops by platform
drivers, I think we all agree to call usb_add_hcd/usb_remove_hcd
to start/stop host roles, just some platforms may need more than
just call them.

-- 

Best Regards,
Peter Chen
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/