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

[Tony Lindgren <tony@atomide.com>]

* 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