All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Munegowda, Keshava" <keshava_mgowda@ti.com>
To: linux-omap@vger.kernel.org, Russ Dill <Russ.Dill@ti.com>
Cc: linux-arm-kernel@lists.infradead.org, linux-usb@vger.kernel.org,
	Felipe Balbi <balbi@ti.com>, Partha Basak <parthab@india.ti.com>,
	Igor Grinberg <grinberg@compulab.co.il>,
	Samuel Ortiz <sameo@linux.intel.com>,
	mantesh@ti.com, s-sapna1@ti.com, Russ Dill <Russ.Dill@gmail.com>
Subject: Re: [PATCH v3] ARM: OMAP3: USB: Fix the EHCI ULPI PHY reset fix issues.
Date: Mon, 2 Jul 2012 12:06:08 +0530	[thread overview]
Message-ID: <CAP05o4+NhGgmZkUymmKEGY9szrot6ND_3oDedbMHpc_+sW0HHw@mail.gmail.com> (raw)
In-Reply-To: <Pine.LNX.4.44L0.1206291104270.1638-100000@iolanthe.rowland.org>

On Fri, Jun 29, 2012 at 9:03 PM, Alan Stern <stern@rowland.harvard.edu> wrote:
> On Wed, 27 Jun 2012, Russ Dill wrote:
>
>> From: Russ Dill <Russ.Dill@gmail.com>
>>
>> 'ARM: OMAP3: USB: Fix the EHCI ULPI PHY reset issue' (1fcb57d0) fixes
>> an issue where the ULPI PHYs were not held in reset while initializing
>> the EHCI controller. However, it also changes behavior in
>> omap-usb-host.c omap_usbhs_init by releasing reset while the
>> configuration in that function was done.
>>
>> This change caused a regression on BB-xM where USB would not function
>> if 'usb start' had been run from u-boot before booting. A change was
>> made to release reset a little bit earlier which fixed the issue on
>> BB-xM and did not cause any regressions on 3430 sdp, the board for
>> which the fix was originally made.
>>
>> This new fix, 'USB: EHCI: OMAP: Finish ehci omap phy reset cycle
>> before adding hcd.', (3aa2ae74) caused a regression on OMAP5.
>>
>> The original fix to hold the EHCI controller in reset during
>> initialization was correct, however it appears that changing
>> omap_usbhs_init to not hold the PHYs in reset during it's
>> configuration was incorrect. This patch first reverts both fixes, and
>> then changes ehci_hcd_omap_probe in ehci-omap.c to hold the PHYs in
>> reset as the original patch had done. It also is sure to incorporate
>> the _cansleep change that has been made in the meantime.
>>
>> Tested on Beagleboard xM.
>
> Looking at the result of this patch, there seem to be a few mistakes
> remaining in the probe routine.
>
> If the regulator_get() call fails, the failure is logged but ignored.
> Is that really the right thing to do?
>
> The pm_runtime_get_sync() call occurs before the probe is finished.
> This means that ehci-hcd's resume routine will try to power-up the
> device before its data structures have been initialized.  That can't be
> right.
>
> The "get clocks" section occurs after the call to usb_add_hcd().  This
> means the controller will start running before the clock references are
> acquired -- so the clocks might still be turned off.  That can't be
> right either.
>
> If the clk_get(dev, "utmi_p1_gfclk") call fails, the error path never
> calls usb_remove_hcd().
>
> The "err_add_hcd" pathway never calls usb_put_hcd().
>
> I'm going to resubmit my most recent patch based the current
> ehci-omap.c, but you or someone else will still have to fix these
> problems.
>
> Alan Stern
>

Hi Rus Dill,
    once Alan post his changes on ehci-omap.c, please re-base this
patch on top of it.
so that, I will rebase UHH-TLL split series on top your patch.

regards
keshava

WARNING: multiple messages have this Message-ID (diff)
From: keshava_mgowda@ti.com (Munegowda, Keshava)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v3] ARM: OMAP3: USB: Fix the EHCI ULPI PHY reset fix issues.
Date: Mon, 2 Jul 2012 12:06:08 +0530	[thread overview]
Message-ID: <CAP05o4+NhGgmZkUymmKEGY9szrot6ND_3oDedbMHpc_+sW0HHw@mail.gmail.com> (raw)
In-Reply-To: <Pine.LNX.4.44L0.1206291104270.1638-100000@iolanthe.rowland.org>

On Fri, Jun 29, 2012 at 9:03 PM, Alan Stern <stern@rowland.harvard.edu> wrote:
> On Wed, 27 Jun 2012, Russ Dill wrote:
>
>> From: Russ Dill <Russ.Dill@gmail.com>
>>
>> 'ARM: OMAP3: USB: Fix the EHCI ULPI PHY reset issue' (1fcb57d0) fixes
>> an issue where the ULPI PHYs were not held in reset while initializing
>> the EHCI controller. However, it also changes behavior in
>> omap-usb-host.c omap_usbhs_init by releasing reset while the
>> configuration in that function was done.
>>
>> This change caused a regression on BB-xM where USB would not function
>> if 'usb start' had been run from u-boot before booting. A change was
>> made to release reset a little bit earlier which fixed the issue on
>> BB-xM and did not cause any regressions on 3430 sdp, the board for
>> which the fix was originally made.
>>
>> This new fix, 'USB: EHCI: OMAP: Finish ehci omap phy reset cycle
>> before adding hcd.', (3aa2ae74) caused a regression on OMAP5.
>>
>> The original fix to hold the EHCI controller in reset during
>> initialization was correct, however it appears that changing
>> omap_usbhs_init to not hold the PHYs in reset during it's
>> configuration was incorrect. This patch first reverts both fixes, and
>> then changes ehci_hcd_omap_probe in ehci-omap.c to hold the PHYs in
>> reset as the original patch had done. It also is sure to incorporate
>> the _cansleep change that has been made in the meantime.
>>
>> Tested on Beagleboard xM.
>
> Looking at the result of this patch, there seem to be a few mistakes
> remaining in the probe routine.
>
> If the regulator_get() call fails, the failure is logged but ignored.
> Is that really the right thing to do?
>
> The pm_runtime_get_sync() call occurs before the probe is finished.
> This means that ehci-hcd's resume routine will try to power-up the
> device before its data structures have been initialized.  That can't be
> right.
>
> The "get clocks" section occurs after the call to usb_add_hcd().  This
> means the controller will start running before the clock references are
> acquired -- so the clocks might still be turned off.  That can't be
> right either.
>
> If the clk_get(dev, "utmi_p1_gfclk") call fails, the error path never
> calls usb_remove_hcd().
>
> The "err_add_hcd" pathway never calls usb_put_hcd().
>
> I'm going to resubmit my most recent patch based the current
> ehci-omap.c, but you or someone else will still have to fix these
> problems.
>
> Alan Stern
>

Hi Rus Dill,
    once Alan post his changes on ehci-omap.c, please re-base this
patch on top of it.
so that, I will rebase UHH-TLL split series on top your patch.

regards
keshava

  reply	other threads:[~2012-07-02  6:36 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-06-14 16:21 [PATCH v2] ARM: OMAP3: USB: Fix the EHCI ULPI PHY reset fix issues Russ Dill
2012-06-14 16:21 ` Russ Dill
2012-06-28  1:59 ` [PATCH v3] " Russ Dill
2012-06-28  1:59   ` Russ Dill
     [not found]   ` <1340848783-9480-1-git-send-email-Russ.Dill-l0cyMroinI0@public.gmane.org>
2012-06-28 23:12     ` Sarashetti, Mantesh
2012-06-28 23:12       ` Sarashetti, Mantesh
2012-06-29 15:33   ` Alan Stern
2012-06-29 15:33     ` Alan Stern
2012-07-02  6:36     ` Munegowda, Keshava [this message]
2012-07-02  6:36       ` Munegowda, Keshava
2012-06-14 16:24 Russ Dill
2012-06-14 16:24 ` Russ Dill
2012-06-14 19:06 ` Sarashetti, Mantesh
2012-06-14 19:06   ` Sarashetti, Mantesh
     [not found]   ` <393884FF03A1CE4A86A02FC97D0954E73E91B6EC-Er742YJ7I/eIQmiDNMet8wC/G2K4zDHf@public.gmane.org>
2012-06-18  8:29     ` Munegowda, Keshava
2012-06-18  8:29       ` Munegowda, Keshava
2012-06-18 13:00       ` Sunil Matange
2012-06-18 13:00         ` Sunil Matange
2012-07-02 11:22 ` Samuel Ortiz
2012-07-02 11:22   ` Samuel Ortiz
2012-07-02 11:21   ` Munegowda, Keshava
2012-07-02 11:21     ` Munegowda, Keshava
2012-07-02 14:05     ` Alan Stern
2012-07-02 14:05       ` Alan Stern
2012-07-02 14:24       ` Munegowda, Keshava
2012-07-02 14:24         ` Munegowda, Keshava

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAP05o4+NhGgmZkUymmKEGY9szrot6ND_3oDedbMHpc_+sW0HHw@mail.gmail.com \
    --to=keshava_mgowda@ti.com \
    --cc=Russ.Dill@gmail.com \
    --cc=Russ.Dill@ti.com \
    --cc=balbi@ti.com \
    --cc=grinberg@compulab.co.il \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-omap@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=mantesh@ti.com \
    --cc=parthab@india.ti.com \
    --cc=s-sapna1@ti.com \
    --cc=sameo@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.