Re: [PATCH 4/4] drivers: usb: start using the control module driver

[Felipe Balbi]

Hi,

On Fri, Jan 18, 2013 at 03:10:45PM +0530, Kishon Vijay Abraham I wrote:
> Start using the control module driver for powering on the PHY and for
> writing to the mailbox instead of writing to the control module
> registers on their own.
> 
> Signed-off-by: Kishon Vijay Abraham I <kishon@xxxxxx>
> ---
>  Documentation/devicetree/bindings/usb/omap-usb.txt |    4 ++
>  Documentation/devicetree/bindings/usb/usb-phy.txt  |    7 +--
>  arch/arm/mach-omap2/omap_hwmod_44xx_data.c         |   13 ----
>  drivers/usb/musb/Kconfig                           |    1 +
>  drivers/usb/musb/omap2430.c                        |   64 ++++++++------------
>  drivers/usb/musb/omap2430.h                        |    9 ---
>  drivers/usb/phy/Kconfig                            |    1 +
>  drivers/usb/phy/omap-usb2.c                        |   38 +++---------
>  include/linux/usb/omap_usb.h                       |    4 +-
>  9 files changed, 42 insertions(+), 99 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/usb/omap-usb.txt b/Documentation/devicetree/bindings/usb/omap-usb.txt
> index ead6ba9..8bedbba 100644
> --- a/Documentation/devicetree/bindings/usb/omap-usb.txt
> +++ b/Documentation/devicetree/bindings/usb/omap-usb.txt
> @@ -3,6 +3,9 @@ OMAP GLUE AND OTHER OMAP SPECIFIC COMPONENTS
>  OMAP MUSB GLUE
>   - compatible : Should be "ti,omap4-musb" or "ti,omap3-musb"
>   - ti,hwmods : must be "usb_otg_hs"
> + - ti,has_mailbox : to specify that omap uses an external mailbox
> +   (in control module) to communicate with the musb core during device connect
> +   and disconnect.
>   - multipoint : Should be "1" indicating the musb controller supports
>     multipoint. This is a MUSB configuration-specific setting.
>   - num_eps : Specifies the number of endpoints. This is also a
> @@ -24,6 +27,7 @@ SOC specific device node entry
>  usb_otg_hs: usb_otg_hs@4a0ab000 {
>  	compatible = "ti,omap4-musb";
>  	ti,hwmods = "usb_otg_hs";
> +	ti,has_mailbox;
>  	multipoint = <1>;
>  	num_eps = <16>;
>  	ram_bits = <12>;
> diff --git a/Documentation/devicetree/bindings/usb/usb-phy.txt b/Documentation/devicetree/bindings/usb/usb-phy.txt
> index 2466b6f..48761a2 100644
> --- a/Documentation/devicetree/bindings/usb/usb-phy.txt
> +++ b/Documentation/devicetree/bindings/usb/usb-phy.txt
> @@ -4,9 +4,7 @@ OMAP USB2 PHY
>  
>  Required properties:
>   - compatible: Should be "ti,omap-usb2"
> - - reg : Address and length of the register set for the device. Also
> -add the address of control module dev conf register until a driver for
> -control module is added
> + - reg : Address and length of the register set for the device.
>  
>  Optional properties:
>   - ctrl_module : phandle of the control module used by PHY driver to power on
> @@ -16,7 +14,6 @@ This is usually a subnode of ocp2scp to which it is connected.
>  
>  usb2phy@4a0ad080 {
>  	compatible = "ti,omap-usb2";
> -	reg = <0x4a0ad080 0x58>,
> -	      <0x4a002300 0x4>;
> +	reg = <0x4a0ad080 0x58>;
>  	ctrl_module = <&omap_control_usb>;
>  };
> diff --git a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
> index 129d508..103f4ba 100644
> --- a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
> +++ b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
> @@ -2698,13 +2698,6 @@ static struct resource omap44xx_usb_phy_and_pll_addrs[] = {
>  		.end		= 0x4a0ae000,
>  		.flags		= IORESOURCE_MEM,
>  	},
> -	{
> -		/* XXX: Remove this once control module driver is in place */
> -		.name		= "ctrl_dev",
> -		.start		= 0x4a002300,
> -		.end		= 0x4a002303,
> -		.flags		= IORESOURCE_MEM,
> -	},
>  	{ }
>  };
>  
> @@ -6152,12 +6145,6 @@ static struct omap_hwmod_addr_space omap44xx_usb_otg_hs_addrs[] = {
>  		.pa_end		= 0x4a0ab7ff,
>  		.flags		= ADDR_TYPE_RT
>  	},
> -	{
> -		/* XXX: Remove this once control module driver is in place */
> -		.pa_start	= 0x4a00233c,
> -		.pa_end		= 0x4a00233f,
> -		.flags		= ADDR_TYPE_RT
> -	},
>  	{ }
>  };
>  
> diff --git a/drivers/usb/musb/Kconfig b/drivers/usb/musb/Kconfig
> index 23a0b7f..de6e5ce 100644
> --- a/drivers/usb/musb/Kconfig
> +++ b/drivers/usb/musb/Kconfig
> @@ -11,6 +11,7 @@ config USB_MUSB_HDRC
>  	select NOP_USB_XCEIV if (SOC_TI81XX || SOC_AM33XX)
>  	select TWL4030_USB if MACH_OMAP_3430SDP
>  	select TWL6030_USB if MACH_OMAP_4430SDP || MACH_OMAP4_PANDA
> +	select OMAP_CONTROL_USB if MACH_OMAP_4430SDP || MACH_OMAP4_PANDA
>  	select USB_OTG_UTILS
>  	help
>  	  Say Y here if your system has a dual role high speed USB
> diff --git a/drivers/usb/musb/omap2430.c b/drivers/usb/musb/omap2430.c
> index da00af4..3e7ceef 100644
> --- a/drivers/usb/musb/omap2430.c
> +++ b/drivers/usb/musb/omap2430.c
> @@ -37,6 +37,7 @@
>  #include <linux/err.h>
>  #include <linux/delay.h>
>  #include <linux/usb/musb-omap.h>
> +#include <linux/usb/omap_control_usb.h>
>  
>  #include "musb_core.h"
>  #include "omap2430.h"
> @@ -46,7 +47,7 @@ struct omap2430_glue {
>  	struct platform_device	*musb;
>  	enum omap_musb_vbus_id_status status;
>  	struct work_struct	omap_musb_mailbox_work;
> -	u32 __iomem		*control_otghs;
> +	struct device		*control_otghs;
>  };
>  #define glue_to_musb(g)		platform_get_drvdata(g->musb)
>  
> @@ -54,26 +55,6 @@ struct omap2430_glue		*_glue;
>  
>  static struct timer_list musb_idle_timer;
>  
> -/**
> - * omap4_usb_phy_mailbox - write to usb otg mailbox
> - * @glue: struct omap2430_glue *
> - * @val: the value to be written to the mailbox
> - *
> - * On detection of a device (ID pin is grounded), this API should be called
> - * to set AVALID, VBUSVALID and ID pin is grounded.
> - *
> - * When OMAP is connected to a host (OMAP in device mode), this API
> - * is called to set AVALID, VBUSVALID and ID pin in high impedance.
> - *
> - * XXX: This function will be removed once we have a seperate driver for
> - * control module
> - */
> -static void omap4_usb_phy_mailbox(struct omap2430_glue *glue, u32 val)
> -{
> -	if (glue->control_otghs)
> -		writel(val, glue->control_otghs);
> -}
> -
>  static void musb_do_idle(unsigned long _musb)
>  {
>  	struct musb	*musb = (void *)_musb;
> @@ -269,7 +250,6 @@ EXPORT_SYMBOL_GPL(omap_musb_mailbox);
>  
>  static void omap_musb_set_mailbox(struct omap2430_glue *glue)
>  {
> -	u32 val;
>  	struct musb *musb = glue_to_musb(glue);
>  	struct device *dev = musb->controller;
>  	struct musb_hdrc_platform_data *pdata = dev->platform_data;
> @@ -285,8 +265,8 @@ static void omap_musb_set_mailbox(struct omap2430_glue *glue)
>  		musb->xceiv->last_event = USB_EVENT_ID;
>  		if (musb->gadget_driver) {
>  			pm_runtime_get_sync(dev);
> -			val = AVALID | VBUSVALID;
> -			omap4_usb_phy_mailbox(glue, val);
> +			if (!IS_ERR(glue->control_otghs))
> +				omap_control_usb_host_mode(glue->control_otghs);

you could introduce a helper to do the checks for you. something like:

static inline void omap_control_set_mode(*ctrl, int mode)
{
	if (IS_ERR(glue->control_otghs))
		return;

	switch (mode) {
	case HOST:
		omap_control_usb_host_mode(ctrl);
		break;
	case DEVICE:
		omap_control_usb_device(ctrl)
		break;
	default:
		meh();
	}
}

likewise for power() method

-- 
balbi

Attachment: signature.asc
Description: Digital signature