On Wed, Feb 25, 2009 at 12:35:05AM +0200, Grazvydas Ignotas wrote:
> On Mon, Feb 23, 2009 at 8:55 PM, Felipe Balbi <me@xxxxxxxxxxxxxxx> wrote:
> > Adding a platform_data to the driver allow us
> > to remove some of the ifdeferry in the code.
> >
>
> <snip>
>
> > diff --git a/arch/arm/mach-omap2/board-omap3pandora.c b/arch/arm/mach-omap2/board-omap3pandora.c
> > index b8a78c0..08215c0 100644
> > --- a/arch/arm/mach-omap2/board-omap3pandora.c
> > +++ b/arch/arm/mach-omap2/board-omap3pandora.c
> > @@ -297,7 +297,7 @@ static void __init omap3pandora_init(void)
> > spi_register_board_info(omap3pandora_spi_board_info,
> > ARRAY_SIZE(omap3pandora_spi_board_info));
> > usb_musb_init();
> > - usb_ehci_init();
> > + usb_ehci_init(EHCI_HCD_OMAP_MODE_PHY, true, true, 57, 61);
>
> Perhaps it would be better to have .chargepump and .phy_reset set to
> false by default, as pandora doesn't need that chargepump thing (I
> guess some other boards too), and phy reset GPIO is wrong not only for
> pandora I think. Board maintainers can then send patches as needed.
> There also should be a way to specify only one reset GPIO, most boards
> have only one EHCI port I think. Note that .phy_reset is required for
> EHCI to work on pandora (in case you have thoughts of removing that
> code later).
I made it true by default because those where always true in PHY mode as
of current driver. The correct way to do it, would be to move both to
board-file since those workarounds are board-specific.
We need a list of the boards that need the workarounds and a list of
those which don't need so we can move the workarounds to the
board-files.
>
> > diff --git a/arch/arm/mach-omap2/usb-ehci.c b/arch/arm/mach-omap2/usb-ehci.c
> > index 23fe857..30e1ad6 100644
> > --- a/arch/arm/mach-omap2/usb-ehci.c
> > +++ b/arch/arm/mach-omap2/usb-ehci.c
> > @@ -145,8 +145,20 @@ static void setup_ehci_io_mux(void)
> > return;
> > }
> >
> > -void __init usb_ehci_init(void)
> > +void __init usb_ehci_init(enum ehci_hcd_omap_mode phy_mode,
> > + int chargepump, int phy_reset, int reset_gpio_port1,
> > + int reset_gpio_port2)
> > {
> > + struct ehci_hcd_omap_platform_data pdata = {
> > + .phy_mode = phy_mode,
> > + .chargepump = chargepump,
> > + .phy_reset = phy_reset,
> > + .reset_gpio_port1 = reset_gpio_port1,
> > + .reset_gpio_port2 = reset_gpio_port2,
> > + };
> > +
> > + ehci_device.dev.platform_data = &pdata;
> > +
>
> hmm, doesn't pdata end up on stack here? platform_device_register()
> doesn't seem to make a copy of that structure.
hmm... should make that __initconst and use
platform_device_add_data();
Will fix, thanks a lot
--
balbi
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
