diff mbox

[v2,2/3] ARM: OMAP4: add _dev_attr_ to ocp2scp for representing usb_phy

Message ID 1349675983-23803-3-git-send-email-kishon@ti.com (mailing list archive)
State New, archived
Headers show

Commit Message

Kishon Vijay Abraham I Oct. 8, 2012, 5:59 a.m. UTC
In order to reflect devices(usb_phy) attached to ocp2scp bus, ocp2scp
is assigned a device attribute to represent the attached devices.

Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
Cc: Benoit Cousson <b-cousson@ti.com>
---
 arch/arm/mach-omap2/omap_hwmod_44xx_data.c |   28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

Comments

Tony Lindgren Oct. 11, 2012, 12:44 a.m. UTC | #1
Hi,

* Kishon Vijay Abraham I <kishon@ti.com> [121007 23:01]:
> In order to reflect devices(usb_phy) attached to ocp2scp bus, ocp2scp
> is assigned a device attribute to represent the attached devices.
...

> --- a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
> +++ b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
> @@ -21,6 +21,7 @@
>  #include <linux/io.h>
>  #include <linux/platform_data/gpio-omap.h>
>  #include <linux/power/smartreflex.h>
> +#include <linux/platform_data/omap_ocp2scp.h>
>  
>  #include <plat/omap_hwmod.h>
>  #include <plat/i2c.h>
> @@ -2681,6 +2682,32 @@ static struct omap_hwmod_class omap44xx_ocp2scp_hwmod_class = {
>  	.sysc	= &omap44xx_ocp2scp_sysc,
>  };
>  
> +/* ocp2scp dev_attr */
> +static struct resource omap44xx_usb_phy_and_pll_addrs[] = {
> +	{
> +		.name		= "usb_phy",
> +		.start		= 0x4a0ad080,
> +		.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,
> +	},
> +	{ }
> +};

Why don't you set the CONTROL_DEV_CONF as a fixed regulator?

That way you can define the fixed regulator in mach-omap2/control.c
and the driver can just pick it up by name.

The 4470 TRM says that this register "power down entire USB
PHY" and "controls USB2PHYCORE.PD pin".

That way you can also get rid of the mysterious mdelay(200)
in omap_usb_phy_power() in omap-usb2.c driver.

However one thing needs to be checked first.

If CONTROL_DEV_CONF is not a regulator but is a signal mux,
then you can map the register with mux framework using the
pinctrl-single. See the pinctrl-single,bits binding that
was recently merged to mainline.

In the mux case you can set up the named states "default" and
"disabled" that the PHY driver can then manipulate with
pinctrl_select_state().

Note that for the mux case we don't have a non-device tree
pinctrl framework solution available, so you're probably
better off setting it up as a fixed regulator with comments
in case it really is a mux.

BTW, the reason I'm thinking CONTROL_DEV_CONF may be a mux
is because omap3 has registers named CONTROL_DEVCONF0 and
CONTROL_DEVCONF1 that mux signals for various devices.
But maybe that naming is just a coincidence.

Regards,

Tony
Tony Lindgren Oct. 16, 2012, 4:49 p.m. UTC | #2
* Tony Lindgren <tony@atomide.com> [121010 17:46]:
> Hi,
> 
> * Kishon Vijay Abraham I <kishon@ti.com> [121007 23:01]:
> > In order to reflect devices(usb_phy) attached to ocp2scp bus, ocp2scp
> > is assigned a device attribute to represent the attached devices.
> ...
> 
> > --- a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
> > +++ b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
> > @@ -21,6 +21,7 @@
> >  #include <linux/io.h>
> >  #include <linux/platform_data/gpio-omap.h>
> >  #include <linux/power/smartreflex.h>
> > +#include <linux/platform_data/omap_ocp2scp.h>
> >  
> >  #include <plat/omap_hwmod.h>
> >  #include <plat/i2c.h>
> > @@ -2681,6 +2682,32 @@ static struct omap_hwmod_class omap44xx_ocp2scp_hwmod_class = {
> >  	.sysc	= &omap44xx_ocp2scp_sysc,
> >  };
> >  
> > +/* ocp2scp dev_attr */
> > +static struct resource omap44xx_usb_phy_and_pll_addrs[] = {
> > +	{
> > +		.name		= "usb_phy",
> > +		.start		= 0x4a0ad080,
> > +		.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,
> > +	},
> > +	{ }
> > +};
> 
> Why don't you set the CONTROL_DEV_CONF as a fixed regulator?

After some more investigating, we may be able to set it up as a
regulator or clock later on. But as a regression fix, this seems
OK for me.

Benoit, is this OK for you?

Regards,

Tony
diff mbox

Patch

diff --git a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
index 652d028..cf579b5 100644
--- a/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
+++ b/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
@@ -21,6 +21,7 @@ 
 #include <linux/io.h>
 #include <linux/platform_data/gpio-omap.h>
 #include <linux/power/smartreflex.h>
+#include <linux/platform_data/omap_ocp2scp.h>
 
 #include <plat/omap_hwmod.h>
 #include <plat/i2c.h>
@@ -2681,6 +2682,32 @@  static struct omap_hwmod_class omap44xx_ocp2scp_hwmod_class = {
 	.sysc	= &omap44xx_ocp2scp_sysc,
 };
 
+/* ocp2scp dev_attr */
+static struct resource omap44xx_usb_phy_and_pll_addrs[] = {
+	{
+		.name		= "usb_phy",
+		.start		= 0x4a0ad080,
+		.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,
+	},
+	{ }
+};
+
+static struct omap_ocp2scp_dev ocp2scp_dev_attr[] = {
+	{
+		.drv_name       = "omap-usb2",
+		.res		= omap44xx_usb_phy_and_pll_addrs,
+	},
+	{ }
+};
+
 /* ocp2scp_usb_phy */
 static struct omap_hwmod omap44xx_ocp2scp_usb_phy_hwmod = {
 	.name		= "ocp2scp_usb_phy",
@@ -2694,6 +2721,7 @@  static struct omap_hwmod omap44xx_ocp2scp_usb_phy_hwmod = {
 			.modulemode   = MODULEMODE_HWCTRL,
 		},
 	},
+	.dev_attr	= ocp2scp_dev_attr,
 };
 
 /*