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.
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
