Re: [PATCH] usb: phy: am335x: Prevent GPIO reset line request

Re: [PATCH] usb: phy: am335x: Prevent GPIO reset line request

[Ezequiel Garcia]

(Forgot to Cc OMAP people)

On Wed, Nov 20, 2013 at 06:58:34PM -0300, Ezequiel Garcia wrote:
> The 'gpio_reset' field was implicitly set to zero, which is a valid
> GPIO line and results in the NOP PHY layer trying to request it.
> 
> Instead, the AM335x SoC need special USB PHY reset handling so let's
> set 'gpio_reset' to EINVAL and prevent the NOP PHY from messing with it.
> 
> Signed-off-by: Ezequiel Garcia <ezequiel.garcia-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
> ---
> Commit bd27fa44e13830d2baa278d5702e766380659cb3 (usb: phy: generic:
> Don't use regulator framework for RESET line) introduced this.
> 
>  drivers/usb/phy/phy-am335x.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/usb/phy/phy-am335x.c b/drivers/usb/phy/phy-am335x.c
> index 6370e50..7fff9f3 100644
> --- a/drivers/usb/phy/phy-am335x.c
> +++ b/drivers/usb/phy/phy-am335x.c
> @@ -43,6 +43,9 @@ static int am335x_phy_probe(struct platform_device *pdev)
>  	if (!am_phy)
>  		return -ENOMEM;
>  
> +	/* Prevent GPIO Reset from the Generic PHY */
> +	am_phy->usb_phy_gen.gpio_reset = -EINVAL;
> +
>  	am_phy->phy_ctrl = am335x_get_phy_control(dev);
>  	if (!am_phy->phy_ctrl)
>  		return -EPROBE_DEFER;
> -- 
> 1.8.1.5
> 

-- 
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] usb: phy: am335x: Prevent GPIO reset line request

[Sebastian Andrzej Siewior]

On 11/20/2013 11:02 PM, Ezequiel Garcia wrote:
> (Forgot to Cc OMAP people)

Felipe posted a redo of this area to fix this problem a while back. See
  http://www.spinics.net/lists/linux-usb/msg97286.html

Sebastian

Re: [PATCH] usb: phy: am335x: Prevent GPIO reset line request

[Ezequiel Garcia]

On Thu, Nov 21, 2013 at 09:16:40AM +0100, Sebastian Andrzej Siewior wrote:
> On 11/20/2013 11:02 PM, Ezequiel Garcia wrote:
> > (Forgot to Cc OMAP people)
> 
> Felipe posted a redo of this area to fix this problem a while back. See
>   http://www.spinics.net/lists/linux-usb/msg97286.html
> 

Ah, good to know. That patch should be picked ASAP, without it the USB
in AM335x is broken. Is it already too late to -rc1?

Who is supposed to merge that? Greg?
-- 
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] usb: phy: am335x: Prevent GPIO reset line request

[Sebastian Andrzej Siewior]

On 11/21/2013 12:30 PM, Ezequiel Garcia wrote:
> Ah, good to know. That patch should be picked ASAP, without it the USB
> in AM335x is broken. Is it already too late to -rc1?
Yes.

> Who is supposed to merge that? Greg?

Felipe will start collecting fixes once -rc1 is out [0] so it should be
part of -rc2.

[0] This was his work flow for the last couple cycles, I assume he will
    continue.

Sebastian

Re: [PATCH] usb: phy: am335x: Prevent GPIO reset line request

[Ezequiel Garcia]

On Thu, Nov 21, 2013 at 12:44:51PM +0100, Sebastian Andrzej Siewior wrote:
> On 11/21/2013 12:30 PM, Ezequiel Garcia wrote:
> > Ah, good to know. That patch should be picked ASAP, without it the USB
> > in AM335x is broken. Is it already too late to -rc1?
> Yes.
> 
> > Who is supposed to merge that? Greg?
> 
> Felipe will start collecting fixes once -rc1 is out [0] so it should be
> part of -rc2.
> 

OK, fine by me. Please note that Felipe's commit is a much bigger (and
nicer) rework of the code, and that it doesn't clearly state it fixes
the current code.

Felipe: Maybe you should add some message stating it fixes a regression?

Just a suggestion though...
-- 
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] usb: phy: am335x: Prevent GPIO reset line request

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 863 bytes --]

On Thu, Nov 21, 2013 at 08:55:20AM -0300, Ezequiel Garcia wrote:
> On Thu, Nov 21, 2013 at 12:44:51PM +0100, Sebastian Andrzej Siewior wrote:
> > On 11/21/2013 12:30 PM, Ezequiel Garcia wrote:
> > > Ah, good to know. That patch should be picked ASAP, without it the USB
> > > in AM335x is broken. Is it already too late to -rc1?
> > Yes.
> > 
> > > Who is supposed to merge that? Greg?
> > 
> > Felipe will start collecting fixes once -rc1 is out [0] so it should be
> > part of -rc2.
> > 
> 
> OK, fine by me. Please note that Felipe's commit is a much bigger (and
> nicer) rework of the code, and that it doesn't clearly state it fixes
> the current code.
> 
> Felipe: Maybe you should add some message stating it fixes a regression?

sure, makes sense. Sebastian had already mentioned it sounded too nice
of a commit log ;-)

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] usb: phy: am335x: Prevent GPIO reset line request

[Ezequiel Garcia]

On Thu, Nov 21, 2013 at 07:01:55AM -0600, Felipe Balbi wrote:
> On Thu, Nov 21, 2013 at 08:55:20AM -0300, Ezequiel Garcia wrote:
> > On Thu, Nov 21, 2013 at 12:44:51PM +0100, Sebastian Andrzej Siewior wrote:
> > > On 11/21/2013 12:30 PM, Ezequiel Garcia wrote:
> > > > Ah, good to know. That patch should be picked ASAP, without it the USB
> > > > in AM335x is broken. Is it already too late to -rc1?
> > > Yes.
> > > 
> > > > Who is supposed to merge that? Greg?
> > > 
> > > Felipe will start collecting fixes once -rc1 is out [0] so it should be
> > > part of -rc2.
> > > 
> > 
> > OK, fine by me. Please note that Felipe's commit is a much bigger (and
> > nicer) rework of the code, and that it doesn't clearly state it fixes
> > the current code.
> > 
> > Felipe: Maybe you should add some message stating it fixes a regression?
> 
> sure, makes sense. Sebastian had already mentioned it sounded too nice
> of a commit log ;-)
> 

I'm looking at -rc2 and it seems the fix was never pushed, so my board
is probably still broken :-(

Can anybody _please_please_ fix the issue by merging some fix so we can
use USB with mainline?

(FWIW, the fix from Felipe (pointed out by Sebastian) is sitting at
linux-next).

Thanks a lot!
-- 
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] usb: phy: am335x: Prevent GPIO reset line request

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 1322 bytes --]

On Sat, Nov 30, 2013 at 07:45:05PM -0300, Ezequiel Garcia wrote:
> On Thu, Nov 21, 2013 at 07:01:55AM -0600, Felipe Balbi wrote:
> > On Thu, Nov 21, 2013 at 08:55:20AM -0300, Ezequiel Garcia wrote:
> > > On Thu, Nov 21, 2013 at 12:44:51PM +0100, Sebastian Andrzej Siewior wrote:
> > > > On 11/21/2013 12:30 PM, Ezequiel Garcia wrote:
> > > > > Ah, good to know. That patch should be picked ASAP, without it the USB
> > > > > in AM335x is broken. Is it already too late to -rc1?
> > > > Yes.
> > > > 
> > > > > Who is supposed to merge that? Greg?
> > > > 
> > > > Felipe will start collecting fixes once -rc1 is out [0] so it should be
> > > > part of -rc2.
> > > > 
> > > 
> > > OK, fine by me. Please note that Felipe's commit is a much bigger (and
> > > nicer) rework of the code, and that it doesn't clearly state it fixes
> > > the current code.
> > > 
> > > Felipe: Maybe you should add some message stating it fixes a regression?
> > 
> > sure, makes sense. Sebastian had already mentioned it sounded too nice
> > of a commit log ;-)
> > 
> 
> I'm looking at -rc2 and it seems the fix was never pushed, so my board
> is probably still broken :-(
> 
> Can anybody _please_please_ fix the issue by merging some fix so we can
> use USB with mainline?

it's in Greg's tree

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] usb: phy: am335x: Prevent GPIO reset line request

[Ezequiel Garcia]

On Fri, Dec 06, 2013 at 02:16:23PM -0600, Felipe Balbi wrote:
> On Sat, Nov 30, 2013 at 07:45:05PM -0300, Ezequiel Garcia wrote:
> > On Thu, Nov 21, 2013 at 07:01:55AM -0600, Felipe Balbi wrote:
> > > On Thu, Nov 21, 2013 at 08:55:20AM -0300, Ezequiel Garcia wrote:
> > > > On Thu, Nov 21, 2013 at 12:44:51PM +0100, Sebastian Andrzej Siewior wrote:
> > > > > On 11/21/2013 12:30 PM, Ezequiel Garcia wrote:
> > > > > > Ah, good to know. That patch should be picked ASAP, without it the USB
> > > > > > in AM335x is broken. Is it already too late to -rc1?
> > > > > Yes.
> > > > > 
> > > > > > Who is supposed to merge that? Greg?
> > > > > 
> > > > > Felipe will start collecting fixes once -rc1 is out [0] so it should be
> > > > > part of -rc2.
> > > > > 
> > > > 
> > > > OK, fine by me. Please note that Felipe's commit is a much bigger (and
> > > > nicer) rework of the code, and that it doesn't clearly state it fixes
> > > > the current code.
> > > > 
> > > > Felipe: Maybe you should add some message stating it fixes a regression?
> > > 
> > > sure, makes sense. Sebastian had already mentioned it sounded too nice
> > > of a commit log ;-)
> > > 
> > 
> > I'm looking at -rc2 and it seems the fix was never pushed, so my board
> > is probably still broken :-(
> > 
> > Can anybody _please_please_ fix the issue by merging some fix so we can
> > use USB with mainline?
> 
> it's in Greg's tree
> 

Yeah, I just saw the pull. Thanks for the notice! I'll re-test in -rc4
and let you know if I find any more problems.

FWIW, I'm not entirely happy with the solution. Probably being a bit
paranoid, but it seemed to me the fix could be smaller (and fix only
the problem) and then pospone your refactoring until v3.14.

Just my two cents and thanks again for the notice.
-- 
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html