Re: [PATCH RESEND] usb: otg: OMAP4: Fix the omap4430_phy_set_clk function

[Felipe Balbi <balbi@ti.com>]

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

Hi,

On Thu, Jul 05, 2012 at 08:16:54PM +0300, Ruslan Bilovol wrote:
> Hi,
> 
> On Tue, Jul 3, 2012 at 7:13 PM, Felipe Balbi <balbi@ti.com> wrote:
> >
> > Hi,
> >
> > On Mon, Jul 02, 2012 at 08:10:49PM +0300, Ruslan Bilovol wrote:
> > > Hi,
> > >
> > >
> > > On Mon, Jul 2, 2012 at 11:13 AM, Felipe Balbi <balbi@ti.com> wrote:
> > > >
> > > > Hi,
> > > >
> > > > On Tue, Jun 12, 2012 at 08:36:21PM +0300, Ruslan Bilovol wrote:
> > > > > If the clocks are enabled and we want to enable them again
> > > > > omap4430_phy_set_clk disables them.
> > > > >
> > > > > Fix this - so now if we try to enable already enabled clocks
> > > > > it works correctly.
> > > > >
> > > > > Signed-off-by: Ruslan Bilovol <ruslan.bilovol@ti.com>
> > > > > ---
> > > > >  arch/arm/mach-omap2/omap_phy_internal.c |    2 +-
> > > > >  1 files changed, 1 insertions(+), 1 deletions(-)
> > > > >
> > > > > diff --git a/arch/arm/mach-omap2/omap_phy_internal.c
> > > > > b/arch/arm/mach-omap2/omap_phy_internal.c
> > > > > index 4c90477..0196683 100644
> > > > > --- a/arch/arm/mach-omap2/omap_phy_internal.c
> > > > > +++ b/arch/arm/mach-omap2/omap_phy_internal.c
> > > > > @@ -97,7 +97,7 @@ int omap4430_phy_set_clk(struct device *dev, int
> > > > > on)
> > > > >               clk_enable(clk48m);
> > > > >               clk_enable(clk32k);
> > > > >               state = 1;
> > > > > -     } else if (state) {
> > > > > +     } else if (!on && state) {
> > > >
> > > > why don't you let clocks be enabled twice then ? That would cut down
> > > > the
> > > > churn.
> > >
> > > Currently we have unbalanced call of this function.
> > > I meet first during musb initialization - it tries to disable the phy
> > > that leads to disabling already disabled clocks.
> > > Next goal is to use internal clocks counter and to throw static
> > > variable 'state'.
> >
> > don't even go that way... what you need is to fix the unbalanced calls
> > instead of hacking around some generic API.
> 
> Okay Felipe, I understand your position and agree with you. However,
> right now the 'hack' that I'm fixing works incorrectly.
> So while we do not have replacement of the 'hack', it will be good to
> have at least fixed version of this 'hack'.

the problem is that once it "works" nobody ever looks into this again.
So, sorry but I can't accept anything other than a real fix

-- 
balbi

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