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.
--
balbi
Attachment:
signature.asc
Description: Digital signature
