Hi
> On Wed, May 02, 2012 at 10:03:10AM +0000, Gupta, Ajay Kumar wrote:
> > Hi,
> >
> > [..]
> > > > > > + static u8 ti816x, ti814x, ti81xx, am33xx, once;
> > > > > > +
> > > > > > + if (!once) {
> > > > > > + ti816x = cpu_is_ti816x();
> > > > > > + ti814x = cpu_is_ti814x();
> > > > > > + ti81xx = cpu_is_ti81xx();
> > > > > > + am33xx = cpu_is_am33xx();
> > > > > > + once = 1;
> > > > > > + }
> > > > >
> > > > > wow!!! This is so wrong... This whole omap_phy_internal needs to
> > > become
> > > > > a driver. I'm not taking this patch, sorry. Only the next one,
> if it
> > > > > compiles properly.
> > > >
> > > > I understand you have been asking for a driver for
> omap_phy_internal
> > > but
> > > > the same time this patch and above change was based on feedback
> from
> >
> > [...]
> >
> > > Most likely you have a revision register around to test against it
> and,
> >
> > Yes, correct, revision register can be used and so cpu_is_xxx check
> can be dropped.
> >
> > > even if you decide not to use the revision resgister (why would you
> even decided
> > > that ?) you don't need 5 u8 variables for this. You could have
> something
> > > like:
> > >
> > > u8 features = 0;
> > >
> > > if (cpu_is_ti816x())
> > > features |= OMAP_PHY_HAS_PWRDN_BITS;
> > >
> > > then, when you need to check for your usbphycfg register, you could:
> > >
> > > if (features & OMAP_PHY_HAS_PWRDN_BITS) {
> > > usbphycfg &= ~(USBPHY_CM_PWRDN | USBPHY_OTG_PWRDN
> > > | USBPHY_DPINPUT | USBPHY_DMINPUT);
> > > usbphycfg |= (USBPHY_OTGVDET_EN | USBPHY_OTGSESSEND_EN
> > > | USBPHY_DPOPBUFCTL | USBPHY_DMOPBUFCTL);
> > > } else {
> > > usbphycfg |= TI816X_USBPHY0_NORMAL_MODE;
> > > usbphycfg &= ~TI816X_USBPHY_REFCLK_OSC;
> > > }
> > >
> > > Ideally, though, this would be done by matching against a revision
> > > register and completely drop the CPU tests.
> >
> > Agree. But we need to first kill omap_phy_internal and add a separate
> > driver for the same and then add phy control support for ti81xx/am33xx
> so
> > I think it will have to wait longer.
>
> why ? You don't need a "driver" to be able to read a revision register.
> You already read/write to many registers without this file being using
> any of the driver model infrastructure. Still, it can wait a little
> longer, no problems.
If this file is not following any driver model then why to allow to even
grow it further. We should not have allowed the omap4 phy control at first
place itself.
ajay
>
> --
> balbi
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
