| Message ID | 1363360604-21368-7-git-send-email-rogerq@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi, I think you can get rid of quite a bit more of the repeated data for most boards, see below. * Roger Quadros <rogerq@ti.com> [130315 08:21]: > Add 2 platform devices for 'nop-usb-xceiv'. These will be used > as PHYs for HS USB ports 1 and 2 so provide binding information > for them. > > Use usbhs_init_phys() to register the PHY's RESET regulators. > > Signed-off-by: Roger Quadros <rogerq@ti.com> > --- > arch/arm/mach-omap2/board-3430sdp.c | 47 ++++++++++++++++++++++++++++++---- > 1 files changed, 41 insertions(+), 6 deletions(-) > > diff --git a/arch/arm/mach-omap2/board-3430sdp.c b/arch/arm/mach-omap2/board-3430sdp.c > index ce812de..5584682 100644 > --- a/arch/arm/mach-omap2/board-3430sdp.c > +++ b/arch/arm/mach-omap2/board-3430sdp.c > @@ -21,6 +21,7 @@ > #include <linux/spi/spi.h> > #include <linux/i2c/twl.h> > #include <linux/regulator/machine.h> > +#include <linux/usb/phy.h> > #include <linux/io.h> > #include <linux/gpio.h> > #include <linux/mmc/host.h> > @@ -445,16 +446,37 @@ static void enable_board_wakeup_source(void) > OMAP_WAKEUP_EN | OMAP_PIN_INPUT_PULLUP); > } > > +/* PHY device on HS USB Port 1 i.e. nop_usb_xceiv.1 */ > +static struct platform_device hsusb1_phy_device = { > + .name = "nop_usb_xceiv", > + .id = 1, > +}; > + > +/* PHY device on HS USB Port 2 i.e. nop_usb_xceiv.2 */ > +static struct platform_device hsusb2_phy_device = { > + .name = "nop_usb_xceiv", > + .id = 2, > +}; Please allocate these in the common ehci platform init code so you don't have to repeat the same nop_usb_xceiv for each board-*.c file. If you need to override these, then you can pass something instead of NULL to the init function. > +static struct usbhs_phy_data phy_data[] __initdata = { > + { > + .port = 1, > + .reset_gpio = 57, > + .vcc_gpio = -EINVAL, > + .phy_id = "nop_usb_xceiv.1", > + }, > + { > + .port = 2, > + .reset_gpio = 61, > + .vcc_gpio = -EINVAL, > + .phy_id = "nop_usb_xceiv.2", > + }, > +}; This is OK as it's the board specific data needed. If it's the same for many boards, then you might want to add some init function for the default case. > +static struct platform_device *sdp3430_devices[] __initdata = { > + &hsusb1_phy_device, > + &hsusb2_phy_device, > +}; It then seems that this can be in the common ehci platform init code for most cases. > static void __init omap_3430sdp_init(void) > { > int gpio_pendown; > @@ -606,6 +633,14 @@ static void __init omap_3430sdp_init(void) > board_flash_init(sdp_flash_partitions, chip_sel_3430, 0); > sdp3430_display_init(); > enable_board_wakeup_source(); > + > + platform_add_devices(sdp3430_devices, ARRAY_SIZE(sdp3430_devices)); > + /* PHY on HSUSB Port 1 i.e. index 0 */ > + usb_bind_phy("ehci-omap.0", 0, "nop_usb_xceiv.1"); > + /* PHY on HSUSB Port 2 i.e. index 1 */ > + usb_bind_phy("ehci-omap.0", 1, "nop_usb_xceiv.2"); And this part too in most cases. > + usbhs_init_phys(phy_data, ARRAY_SIZE(phy_data)); > usbhs_init(&usbhs_bdata); > } Regards, Tony -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 03/15/2013 06:12 PM, Tony Lindgren wrote: > Hi, > > I think you can get rid of quite a bit more of the repeated data for > most boards, see below. > > * Roger Quadros <rogerq@ti.com> [130315 08:21]: >> Add 2 platform devices for 'nop-usb-xceiv'. These will be used >> as PHYs for HS USB ports 1 and 2 so provide binding information >> for them. >> >> Use usbhs_init_phys() to register the PHY's RESET regulators. >> >> Signed-off-by: Roger Quadros <rogerq@ti.com> >> --- >> arch/arm/mach-omap2/board-3430sdp.c | 47 ++++++++++++++++++++++++++++++---- >> 1 files changed, 41 insertions(+), 6 deletions(-) >> >> diff --git a/arch/arm/mach-omap2/board-3430sdp.c b/arch/arm/mach-omap2/board-3430sdp.c >> index ce812de..5584682 100644 >> --- a/arch/arm/mach-omap2/board-3430sdp.c >> +++ b/arch/arm/mach-omap2/board-3430sdp.c >> @@ -21,6 +21,7 @@ >> #include <linux/spi/spi.h> >> #include <linux/i2c/twl.h> >> #include <linux/regulator/machine.h> >> +#include <linux/usb/phy.h> >> #include <linux/io.h> >> #include <linux/gpio.h> >> #include <linux/mmc/host.h> >> @@ -445,16 +446,37 @@ static void enable_board_wakeup_source(void) >> OMAP_WAKEUP_EN | OMAP_PIN_INPUT_PULLUP); >> } >> >> +/* PHY device on HS USB Port 1 i.e. nop_usb_xceiv.1 */ >> +static struct platform_device hsusb1_phy_device = { >> + .name = "nop_usb_xceiv", >> + .id = 1, >> +}; >> + >> +/* PHY device on HS USB Port 2 i.e. nop_usb_xceiv.2 */ >> +static struct platform_device hsusb2_phy_device = { >> + .name = "nop_usb_xceiv", >> + .id = 2, >> +}; > > Please allocate these in the common ehci platform init code > so you don't have to repeat the same nop_usb_xceiv for each > board-*.c file. If you need to override these, then you can > pass something instead of NULL to the init function. OK. I think there is nothing to be overridden except the platform_data. But that I can pass into via phy_data structure below. > >> +static struct usbhs_phy_data phy_data[] __initdata = { >> + { >> + .port = 1, >> + .reset_gpio = 57, >> + .vcc_gpio = -EINVAL, >> + .phy_id = "nop_usb_xceiv.1", >> + }, >> + { >> + .port = 2, >> + .reset_gpio = 61, >> + .vcc_gpio = -EINVAL, >> + .phy_id = "nop_usb_xceiv.2", >> + }, >> +}; > > This is OK as it's the board specific data needed. If it's the > same for many boards, then you might want to add some init function > for the default case. To keep it simple I'll leave this the way it is. There is nothing much that can be kept common. If reset_gpio and vcc_gpio is not used then the data structure is not required at all. > >> +static struct platform_device *sdp3430_devices[] __initdata = { >> + &hsusb1_phy_device, >> + &hsusb2_phy_device, >> +}; > > It then seems that this can be in the common ehci platform init > code for most cases. OK. > >> static void __init omap_3430sdp_init(void) >> { >> int gpio_pendown; >> @@ -606,6 +633,14 @@ static void __init omap_3430sdp_init(void) >> board_flash_init(sdp_flash_partitions, chip_sel_3430, 0); >> sdp3430_display_init(); >> enable_board_wakeup_source(); >> + >> + platform_add_devices(sdp3430_devices, ARRAY_SIZE(sdp3430_devices)); >> + /* PHY on HSUSB Port 1 i.e. index 0 */ >> + usb_bind_phy("ehci-omap.0", 0, "nop_usb_xceiv.1"); >> + /* PHY on HSUSB Port 2 i.e. index 1 */ >> + usb_bind_phy("ehci-omap.0", 1, "nop_usb_xceiv.2"); > > And this part too in most cases. Yes. > >> + usbhs_init_phys(phy_data, ARRAY_SIZE(phy_data)); >> usbhs_init(&usbhs_bdata); >> } > cheers, -roger -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/arm/mach-omap2/board-3430sdp.c b/arch/arm/mach-omap2/board-3430sdp.c index ce812de..5584682 100644 --- a/arch/arm/mach-omap2/board-3430sdp.c +++ b/arch/arm/mach-omap2/board-3430sdp.c @@ -21,6 +21,7 @@ #include <linux/spi/spi.h> #include <linux/i2c/twl.h> #include <linux/regulator/machine.h> +#include <linux/usb/phy.h> #include <linux/io.h> #include <linux/gpio.h> #include <linux/mmc/host.h> @@ -445,16 +446,37 @@ static void enable_board_wakeup_source(void) OMAP_WAKEUP_EN | OMAP_PIN_INPUT_PULLUP); } +/* PHY device on HS USB Port 1 i.e. nop_usb_xceiv.1 */ +static struct platform_device hsusb1_phy_device = { + .name = "nop_usb_xceiv", + .id = 1, +}; + +/* PHY device on HS USB Port 2 i.e. nop_usb_xceiv.2 */ +static struct platform_device hsusb2_phy_device = { + .name = "nop_usb_xceiv", + .id = 2, +}; + +static struct usbhs_phy_data phy_data[] __initdata = { + { + .port = 1, + .reset_gpio = 57, + .vcc_gpio = -EINVAL, + .phy_id = "nop_usb_xceiv.1", + }, + { + .port = 2, + .reset_gpio = 61, + .vcc_gpio = -EINVAL, + .phy_id = "nop_usb_xceiv.2", + }, +}; + static struct usbhs_omap_platform_data usbhs_bdata __initdata = { .port_mode[0] = OMAP_EHCI_PORT_MODE_PHY, .port_mode[1] = OMAP_EHCI_PORT_MODE_PHY, - .port_mode[2] = OMAP_USBHS_PORT_MODE_UNUSED, - - .phy_reset = true, - .reset_gpio_port[0] = 57, - .reset_gpio_port[1] = 61, - .reset_gpio_port[2] = -EINVAL }; #ifdef CONFIG_OMAP_MUX @@ -585,6 +607,11 @@ static struct flash_partitions sdp_flash_partitions[] = { }, }; +static struct platform_device *sdp3430_devices[] __initdata = { + &hsusb1_phy_device, + &hsusb2_phy_device, +}; + static void __init omap_3430sdp_init(void) { int gpio_pendown; @@ -606,6 +633,14 @@ static void __init omap_3430sdp_init(void) board_flash_init(sdp_flash_partitions, chip_sel_3430, 0); sdp3430_display_init(); enable_board_wakeup_source(); + + platform_add_devices(sdp3430_devices, ARRAY_SIZE(sdp3430_devices)); + /* PHY on HSUSB Port 1 i.e. index 0 */ + usb_bind_phy("ehci-omap.0", 0, "nop_usb_xceiv.1"); + /* PHY on HSUSB Port 2 i.e. index 1 */ + usb_bind_phy("ehci-omap.0", 1, "nop_usb_xceiv.2"); + + usbhs_init_phys(phy_data, ARRAY_SIZE(phy_data)); usbhs_init(&usbhs_bdata); }
Add 2 platform devices for 'nop-usb-xceiv'. These will be used as PHYs for HS USB ports 1 and 2 so provide binding information for them. Use usbhs_init_phys() to register the PHY's RESET regulators. Signed-off-by: Roger Quadros <rogerq@ti.com> --- arch/arm/mach-omap2/board-3430sdp.c | 47 ++++++++++++++++++++++++++++++---- 1 files changed, 41 insertions(+), 6 deletions(-)