From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S936527AbcKDSvt (ORCPT ); Fri, 4 Nov 2016 14:51:49 -0400 Received: from vern.gendns.com ([206.190.152.46]:48495 "EHLO vern.gendns.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S935292AbcKDSvr (ORCPT ); Fri, 4 Nov 2016 14:51:47 -0400 Subject: Re: [PATCH v4 3/4] usb: musb: Add a quirk flag to skip the phy set mode To: Alexandre Bailon , b-liu@ti.com, balbi@kernel.org References: <1478277818-5091-1-git-send-email-abailon@baylibre.com> <1478277818-5091-4-git-send-email-abailon@baylibre.com> Cc: kishon@ti.com, khilman@baylibre.com, linux-kernel@vger.kernel.org, linux-usb@vger.kernel.org, nsekhar@ti.com From: David Lechner Message-ID: <5c5c15c3-6b51-e12c-79ad-f8e5a00c3b49@lechnology.com> Date: Fri, 4 Nov 2016 13:51:44 -0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.3.0 MIME-Version: 1.0 In-Reply-To: <1478277818-5091-4-git-send-email-abailon@baylibre.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit X-AntiAbuse: This header was added to track abuse, please include it with any abuse report X-AntiAbuse: Primary Hostname - vern.gendns.com X-AntiAbuse: Original Domain - vger.kernel.org X-AntiAbuse: Originator/Caller UID/GID - [47 12] / [47 12] X-AntiAbuse: Sender Address Domain - lechnology.com X-Get-Message-Sender-Via: vern.gendns.com: authenticated_id: davidmain+lechnology.com/only user confirmed/virtual account not confirmed X-Authenticated-Sender: vern.gendns.com: davidmain@lechnology.com X-Source: X-Source-Args: X-Source-Dir: Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 11/04/2016 11:43 AM, Alexandre Bailon wrote: > During the init, the driver will use the mode to configure > the controller mode and the phy mode. > The PHY of DA8xx has some issues when the phy is forced in host or device. > Add way to skip the set mode and let the da8xx glue manage the phy mode. > I think you are on the right track here. But I think a cleaner way to do this would be to add a bool parameter to musb_platform_set_mode() that is passed musb->ops->set_mode() callback. You could call this parameter init or first_call or something like that. This way, the set_mode() callback can have a different behavior depending on when it is called. In musb_init_controller(), for example, you would have... status = musb_platform_set_mode(musb, MUSB_HOST, true); and in musb_mode_store(), you would have... status = musb_platform_set_mode(musb, MUSB_HOST, false); > Signed-off-by: Alexandre Bailon > --- > drivers/usb/musb/musb_core.c | 15 ++++++++++----- > drivers/usb/musb/musb_core.h | 1 + > 2 files changed, 11 insertions(+), 5 deletions(-) > > diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c > index 27dadc0..6f5f039 100644 > --- a/drivers/usb/musb/musb_core.c > +++ b/drivers/usb/musb/musb_core.c > @@ -2046,6 +2046,7 @@ musb_init_controller(struct device *dev, int nIrq, void __iomem *ctrl) > { > int status; > struct musb *musb; > + enum musb_mode mode = MUSB_UNDEFINED; > struct musb_hdrc_platform_data *plat = dev_get_platdata(dev); > > /* The driver might handle more features than the board; OK. > @@ -2261,13 +2262,13 @@ musb_init_controller(struct device *dev, int nIrq, void __iomem *ctrl) > status = musb_host_setup(musb, plat->power); > if (status < 0) > goto fail3; > - status = musb_platform_set_mode(musb, MUSB_HOST); > + mode = MUSB_HOST; > break; > case MUSB_PORT_MODE_GADGET: > status = musb_gadget_setup(musb); > if (status < 0) > goto fail3; > - status = musb_platform_set_mode(musb, MUSB_PERIPHERAL); > + mode = MUSB_PERIPHERAL; > break; > case MUSB_PORT_MODE_DUAL_ROLE: > status = musb_host_setup(musb, plat->power); > @@ -2278,15 +2279,19 @@ musb_init_controller(struct device *dev, int nIrq, void __iomem *ctrl) > musb_host_cleanup(musb); > goto fail3; > } > - status = musb_platform_set_mode(musb, MUSB_OTG); > + mode = MUSB_OTG; > break; > default: > dev_err(dev, "unsupported port mode %d\n", musb->port_mode); > break; > } > > - if (status < 0) > - goto fail3; > + if (mode != MUSB_UNDEFINED && > + !(musb->io.quirks & MUSB_SKIP_SET_MODE)) { > + status = musb_platform_set_mode(musb, mode); > + if (status < 0) > + goto fail3; > + } > > status = musb_init_debugfs(musb); > if (status < 0) > diff --git a/drivers/usb/musb/musb_core.h b/drivers/usb/musb/musb_core.h > index 2cb88a49..9bd8b6b 100644 > --- a/drivers/usb/musb/musb_core.h > +++ b/drivers/usb/musb/musb_core.h > @@ -172,6 +172,7 @@ struct musb_io; > */ > struct musb_platform_ops { > > +#define MUSB_SKIP_SET_MODE BIT(7) This would be better described as MUSB_SKIP_SET_MODE_IN_INIT > #define MUSB_DMA_UX500 BIT(6) > #define MUSB_DMA_CPPI41 BIT(5) > #define MUSB_DMA_CPPI BIT(4) >