Re: [PATCH 00/24] ARM: OMAP2+: Adapt to ehci-omap changes for 3.10

Tony Lindgren
2013-03-14T16:54:46Z

* Roger Quadros <rogerq@ti.com> [130314 08:45]:
>
> OK. Let me know how the below patch looks. After that, the board code
> will look like.
>
> static struct usbhs_phy_data phy_data[] = {
> {
> .reset_gpio = 147,
> .vcc_gpio = 148
> .vcc_polarity = 1,
> .phy_id = "nop_usb_xceiv.2",
> },
> {}, /* Terminator */
> };
>
> usbhs_init_phys(phy_data);

Great, looks good to me.

> Patch to implement usbhs_init_phys();
>
> diff --git a/arch/arm/mach-omap2/usb-host.c b/arch/arm/mach-omap2/usb-host.c
> index 5706bdc..b9d6bff 100644
> --- a/arch/arm/mach-omap2/usb-host.c
> +++ b/arch/arm/mach-omap2/usb-host.c
> @@ -22,8 +22,12 @@
> #include <linux/platform_device.h>
> #include <linux/slab.h>
> #include <linux/dma-mapping.h>
> +#include <linux/regulator/machine.h>
> +#include <linux/regulator/fixed.h>
> +#include <linux/string.h>
>
> #include <asm/io.h>
> +#include <asm/gpio.h>

Please change these both to linux/io.h and linux/gpio.h.

> #include "soc.h"
> #include "omap_device.h"
> @@ -472,6 +476,141 @@ void __init setup_4430ohci_io_mux(const enum usbhs_omap_port_mode *port_mode)
> }
> }
>
> +static const char *reset_supply = "reset";
> +static const char *vcc_supply = "vcc";
> +
> +/* Template for PHY regulators */
> +static struct regulator_consumer_supply hsusb_reg_supplies[] = {
> + { /* .supply & .dev_name filled later */ },
> +};
> +
> +static struct regulator_init_data hsusb_reg_data = {
> + .constraints = {
> + .valid_ops_mask = REGULATOR_CHANGE_STATUS,
> + },
> + .consumer_supplies = hsusb_reg_supplies,
> + .num_consumer_supplies = ARRAY_SIZE(hsusb_reg_supplies),
> +};
> +
> +static struct fixed_voltage_config hsusb_reg_config = {
> + /* .supply_name filled later */
> + .microvolts = 3300000,
> + .gpio = -1, /* updated later */
> + .startup_delay = 70000, /* 70msec */
> + .enable_high = 1, /* updated later */
> + .enabled_at_boot = 0, /* keep in RESET */
> + /* .init_data filled later */
> +};
> +
> +static struct platform_device_info hsusb_reg_pdev_info = {
> + .name = "reg-fixed-voltage",
> + .id = PLATFORM_DEVID_AUTO,
> +};
> +
> +int __init usbhs_init_phys(struct usbhs_phy_data *phy)
> +{
> + struct regulator_consumer_supply *supplies;
> + struct regulator_init_data *reg_data;
> + struct fixed_voltage_config *config;
> + char *supply_name;
> + int i;
> +
> +
> + for (i = 1; i <= OMAP3_HS_USB_PORTS; i++) {

Maybe pass the number of ports to initialize too to the
function? Might be more future proof, although it will only
be needed until we have converted to DT.

> +
> + if (!phy->phy_id) /* Terminator ? */
> + break;
> +
> + if (!gpio_is_valid(phy->reset_gpio))
> + goto check_vcc;
> +
> + supplies = kmemdup(hsusb_reg_supplies,
> + ARRAY_SIZE(hsusb_reg_supplies) *
> + sizeof(struct regulator_consumer_supply),
> + GFP_KERNEL);
> + if (!supplies)
> + return -ENOMEM;
> +
> + supplies->supply = reset_supply;
> + supplies->dev_name = phy->phy_id;
> +
> + reg_data = kmemdup(&hsusb_reg_data, sizeof(hsusb_reg_data),
> + GFP_KERNEL);
> + if (!reg_data)
> + return -ENOMEM;
> +
> + reg_data->consumer_supplies = supplies;
> +
> + config = kmemdup(&hsusb_reg_config, sizeof(hsusb_reg_config),
> + GFP_KERNEL);
> + if (!config)
> + return -ENOMEM;
> +
> + supply_name = kmalloc(14, GFP_KERNEL);
> + if (!supply_name)
> + return -ENOMEM;
> +
> + scnprintf(supply_name, 13, "hsusb%d_reset", i);
> + config->supply_name = supply_name;
> + config->gpio = phy->reset_gpio;
> + config->init_data = reg_data;
> +
> + hsusb_reg_pdev_info.data = config;
> + hsusb_reg_pdev_info.size_data = sizeof(hsusb_reg_config);
> + platform_device_register_full(&hsusb_reg_pdev_info);
> +
> +check_vcc:
> + if (!gpio_is_valid(phy->vcc_gpio))
> + goto next;
> +
> + supplies = kmemdup(hsusb_reg_supplies,
> + ARRAY_SIZE(hsusb_reg_supplies) *
> + sizeof(struct regulator_consumer_supply),
> + GFP_KERNEL);
> + if (!supplies)
> + return -ENOMEM;
> +
> + supplies->supply = vcc_supply;
> + supplies->dev_name = phy->phy_id;
> +
> + reg_data = kmemdup(&hsusb_reg_data, sizeof(hsusb_reg_data),
> + GFP_KERNEL);
> + if (!reg_data)
> + return -ENOMEM;
> +
> + reg_data->consumer_supplies = supplies;
> +
> + config = kmemdup(&hsusb_reg_config, sizeof(hsusb_reg_config),
> + GFP_KERNEL);
> + if (!config)
> + return -ENOMEM;
> +
> + supply_name = kmalloc(14, GFP_KERNEL);
> + if (!supply_name)
> + return -ENOMEM;
> +
> + scnprintf(supply_name, 13, "hsusb%d_vcc", i);
> + config->supply_name = supply_name;
> + config->gpio = phy->vcc_gpio;
> + config->enable_high = phy->vcc_polarity;
> + config->init_data = reg_data;
> +
> + hsusb_reg_pdev_info.data = config;
> + hsusb_reg_pdev_info.size_data = sizeof(hsusb_reg_config);
> + platform_device_register_full(&hsusb_reg_pdev_info);
> +
> +next:
> + phy++;
> + }
> +
> + return 0;
> +}
> +
> void __init usbhs_init(struct usbhs_omap_platform_data *pdata)
> {
> struct omap_hwmod *uhh_hwm, *tll_hwm;
> diff --git a/arch/arm/mach-omap2/usb.h b/arch/arm/mach-omap2/usb.h
> index 3319f5c..70a8c63 100644
> --- a/arch/arm/mach-omap2/usb.h
> +++ b/arch/arm/mach-omap2/usb.h
> @@ -53,8 +53,16 @@
> #define USBPHY_OTGSESSEND_EN (1 << 20)
> #define USBPHY_DATA_POLARITY (1 << 23)
>
> +struct usbhs_phy_data {
> + int reset_gpio;
> + int vcc_gpio;
> + bool vcc_polarity; /* 1 active high, 0 active low */
> + char *phy_id;
> +};
> +
> extern void usb_musb_init(struct omap_musb_board_data *board_data);
> extern void usbhs_init(struct usbhs_omap_platform_data *pdata);
> +extern int usbhs_init_phys(struct usbhs_phy_data *phy);

Maybe need a static inline version when no EHCI is selected?

Otherwise looks good to me, thanks for updating it.

Regards,

Tony