Re: [PATCH 5/5 v6] usb: musb: Using runtime pm APIs for musb.
On Tue, Feb 15, 2011 at 6:12 AM, Kevin Hilman wrote: > Hema Kalliguddi writes: > > [...] > >>>On Thu, Feb 10, 2011 at 07:38:01PM +0530, Hema HK wrote: Calling runtime pm APIs pm_runtime_put_sync() and >>>pm_runtime_get_sync() for enabling/disabling the clocks, sysconfig settings. Enable clock, configure no-idle/standby when active and >>>configure force idle/standby and disable clock when idled. This is taken care by the >>>runtime framework when driver calls the pm_runtime_get_sync and pm_runtime_put_sync APIs. >>> >>>does it have to be the _sync() ?? >> Yes. Because immediately after this I access the registers. >> > > What about the put? Put can be async. I will send the patch with changes. Regards, Hema > > Kevin > > -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3] hwmon: twl4030: Driver for twl4030 madc module
Keerthy, Minor comments... Regards, Hema On Thu, Jan 6, 2011 at 9:47 AM, Keerthy wrote: > Introducing a driver for MADC on TWL4030 powerIC. MADC stands for monitoring > ADC. This driver monitors the real time conversion of analog signals like > battery temperature, battery type, battery level etc. User can also ask for > the conversion on a particular channel using the sysfs nodes. Tested the > conversion through sysfs nodes. Tested with DEBUG_LOCKDEP enabled. > > Signed-off-by: Keerthy > --- > > Several people have contributed to this driver on the linux-omap list. > V3: > > Return check added to all the functions reading and writing via i2c. > In probe function reverting all the steps succeded before a failure > and not reverting the step which failed. > Added the declaration of twl4030_get_madc_conversion function in the > header file. > Formatting the header file. > Comments added. > likely occurance removed since it is not in the hot path. > Renamed the_madc to twl4030_madc. > Added sleep command to avoid busy wait in conversion ready function. > > V2: > > Error path correction in probe function. > Return checks added. > the_madc pointer could not be removed. The external drivers will not be > knowing > device information of the madc. > Added another function which takes the channel number alone and returns > the channel reading if the caller wants TWL4030_MADC_SW2 method for ADC > conversion. > IOCTL function is removed. > Work struct is completely removed since request_threaded_irq is used. > > drivers/hwmon/Kconfig | 11 + > drivers/hwmon/Makefile | 1 + > drivers/hwmon/twl4030-madc.c | 794 > ++ > include/linux/i2c/twl4030-madc.h | 118 ++ > 4 files changed, 924 insertions(+), 0 deletions(-) > create mode 100644 drivers/hwmon/twl4030-madc.c > create mode 100644 include/linux/i2c/twl4030-madc.h > > V1: > > http://www.mail-archive.com/linux-omap@vger.kernel.org/msg34947.html > > diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig > index a56f6ad..eec1258 100644 > --- a/drivers/hwmon/Kconfig > +++ b/drivers/hwmon/Kconfig > @@ -920,6 +920,17 @@ config SENSORS_TMP421 > This driver can also be built as a module. If so, the module > will be called tmp421. > > +config SENSORS_TWL4030_MADC > + tristate "Texas Instrments TWL4030 MADC" > + depends on TWL4030_CORE > + help > + This driver provides support for triton TWL4030-MADC. The > + driver supports both RT and SW conversion methods. > + > + This driver can be built as part of kernel or can be built > + as a module. > + > + Only on new line is enough > config SENSORS_VIA_CPUTEMP > tristate "VIA CPU temperature sensor" > depends on X86 > diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile > index 2479b3d..a54af22 100644 > --- a/drivers/hwmon/Makefile > +++ b/drivers/hwmon/Makefile > @@ -100,6 +100,7 @@ obj-$(CONFIG_SENSORS_THMC50) += thmc50.o > obj-$(CONFIG_SENSORS_TMP102) += tmp102.o > obj-$(CONFIG_SENSORS_TMP401) += tmp401.o > obj-$(CONFIG_SENSORS_TMP421) += tmp421.o > +obj-$(CONFIG_SENSORS_TWL4030_MADC)+= twl4030-madc.o > obj-$(CONFIG_SENSORS_VIA_CPUTEMP)+= via-cputemp.o > obj-$(CONFIG_SENSORS_VIA686A) += via686a.o > obj-$(CONFIG_SENSORS_VT1211) += vt1211.o > diff --git a/drivers/hwmon/twl4030-madc.c b/drivers/hwmon/twl4030-madc.c > new file mode 100644 > index 000..523f19a > --- /dev/null > +++ b/drivers/hwmon/twl4030-madc.c > @@ -0,0 +1,794 @@ > +/* > + * > + * TWL4030 MADC module driver-This driver monitors the real time > + * conversion of analog signals like battery temperature, > + * battery type, battery level etc. User can also ask for the conversion on a > + * particular channel using the sysfs nodes. > + * > + * Copyright (C) 2010 Texas Instruments Incorporated - http://www.ti.com/ > + * J Keerthy > + * > + * Based on twl4030-madc.c > + * Copyright (C) 2008 Nokia Corporation > + * Mikko Ylinen > + * > + * Amit Kucheria > + * > + * This program is free software; you can redistribute it and/or > + * modify it under the terms of the GNU General Public License > + * version 2 as published by the Free Software Foundation. > + * > + * This program is distributed in the hope that it will be useful, but > + * WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + * General Public License for more details. > + * > + * You should have received a copy of the GNU General Public License > + * along with this program; if not, write to the Free Software > + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA > + * 02110-1301 USA > + * > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +/* > + * struct twl4030_madc_data - a container for madc info > + * @hwmon_dev - pointer to device structure for madc
Re: [PATCH 5/5 v5] usb: musb: Using runtime pm APIs for musb.
On Fri, Dec 10, 2010 at 8:08 PM, Felipe Balbi wrote: > hi, > > On Fri, Dec 10, 2010 at 08:05:39PM +0530, Hema HK wrote: >> >> Calling runtime pm APIs pm_runtime_put_sync() and pm_runtime_get_sync() >> for enabling/disabling the clocks, sysconfig settings. >> >> enable clock, configure no-idle/standby when active and configure force >> idle/standby >> and disable clock when idled. This is taken care by the runtime framework >> when >> driver calls the pm_runtime_get_sync and pm_runtime_put_sync APIs. >> Need to configure MUSB into force standby and force idle mode when usb not >> used >> >> Signed-off-by: Hema HK >> Cc: Felipe Balbi >> Cc: Tony Lindgren >> Cc: Kevin Hilman >> Cc: Cousson, Benoit >> Cc: Paul Walmsley >> --- >> >> drivers/usb/musb/musb_core.h | 2 - >> drivers/usb/musb/omap2430.c | 80 >> +++ >> 2 files changed, 23 insertions(+), 59 deletions(-) > > Now I see you removed the runtime_resume and runtime_suspend calls from > musb_core. Won't we loose context in that case ? > We need context save/restore when .suspend/.resume are called. That is done anyway in the musb_suspend/ musb_resume_noirq APIs. Moreover we are calling runtime pm with musb_omap2430 dev pointer, so musb runtime_suspend/resume call backs will not be called. Regards, Hema > -- > balbi > -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 5/5 v4] usb: musb: Using runtime pm APIs for musb.
On Fri, Dec 10, 2010 at 6:29 PM, Felipe Balbi wrote:
> Hi,
>
> On Fri, Dec 10, 2010 at 06:23:46PM +0530, Hema HK wrote:
>>
>> @@ -429,28 +392,29 @@ static int __init omap2430_probe(struct
>> pdev->num_resources);
>> if (ret) {
>> dev_err(&pdev->dev, "failed to add resources\n");
>> - goto err4;
>> + goto err2;
>> }
>>
>> ret = platform_device_add_data(musb, pdata, sizeof(*pdata));
>> if (ret) {
>> dev_err(&pdev->dev, "failed to add platform_data\n");
>> - goto err4;
>> + goto err2;
>> }
>>
>> ret = platform_device_add(musb);
>> if (ret) {
>> dev_err(&pdev->dev, "failed to register musb device\n");
>> - goto err4;
>> + goto err2;
>> }
>>
>> - return 0;
>> + pm_runtime_enable(&pdev->dev);
>> + if (pm_runtime_get_sync(dev)) {
>> + dev_err(dev, "pm_runtime_get_sync FAILED");
>> + goto err2;
>> + }
>
> don't you have to pm_runtime_disable() if get_sync fails ?
Yes. That is require. Otherwise next time driver load will through warning.
Thanks for pointing this out.
>
>> @@ -468,8 +432,7 @@ static int __exit omap2430_remove(struct
>>
>> platform_device_del(glue->musb);
>> platform_device_put(glue->musb);
>> - clk_disable(glue->clk);
>> - clk_put(glue->clk);
>> + pm_runtime_put_sync(&pdev->dev);
>
> same here, don't you need to pm_runtime_disable() ??
Yes.
Regards,
Hema
>
> --
> balbi
>
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/5 v4] OMAP2+: musb: HWMOD adaptation for musb.
On Fri, Dec 10, 2010 at 6:32 PM, Felipe Balbi wrote:
> Hi,
>
> On Fri, Dec 10, 2010 at 06:23:10PM +0530, Hema HK wrote:
>>
>> @@ -212,8 +228,28 @@ void __init usb_musb_init(struct omap_mu
>> musb_plat.mode = board_data->mode;
>> musb_plat.extvbus = board_data->extvbus;
>>
>> - if (platform_device_register(&musb_device) < 0)
>> - printk(KERN_ERR "Unable to register HS-USB (MUSB)
>> device\n");
>> + if (cpu_is_omap3517() || cpu_is_omap3505()) {
>> +
>> + if (platform_device_register(&musb_device) < 0)
>> + printk(KERN_ERR "Unable to register HS-USB \
>> + (MUSB) device\n");
>> + } else {
>
> you can amend these two branches to the previous one. move the
> platform_device_register() to the previous if (cpu_is_omap3517() ||
> cpu_is_omap3505()) check. similarly with the code below.
Yes... Can be done.
Regards,
Hema
>
>> + pdata = &musb_plat;
>> + od = omap_device_build(name, bus_id, oh, pdata,
>> + sizeof(*pdata), omap_musb_latency,
>> + ARRAY_SIZE(omap_musb_latency),
>> false);
>> + if (IS_ERR(od)) {
>> + pr_err("Could not build omap_device for %s %s\n",
>> + name, oh_name);
>> + return;
>> + }
>> + pdev = &od->pdev;
>> + dev = &pdev->dev;
>> + get_device(dev);
>> + dev->dma_mask = &musb_dmamask;
>> + dev->coherent_dma_mask = musb_dmamask;
>> + put_device(dev);
>> + }
>> }
>>
>> #else
>
> --
> balbi
>
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4] usb: musb: Adding names for IRQs in resource structure
On Fri, Dec 10, 2010 at 6:26 PM, Sergei Shtylyov wrote:
> Hello.
>
> On 10-12-2010 7:46, Kalliguddi, Hema wrote:
>
>>>>> The resource data is getting automatically populated from a set of data
>>>>> generated from TI's hardware database for the OMAP platform, While we
>>>>> could hack in some exceptions to that tool to generate resources
>>>>> in a specific order, it seems less fragile to use the resource name
>>>>> instead.That way, no matter what order the resources are generated, the
>>>>> driver still work.
>
>>>>> Modified the OMAP,Blackfin and Davinci architecture files to add the
>>>>> name
>>>>> of the IRQs
>>>>> in the resource structures and musb driver to use the get_irq_byname()
>>>>> api to
>>>>> get the device and dma irq numbers instead of using the index.
>
>>>>> Signed-off-by: Hema HK
>>>>> Cc: Felipe Balbi
>>>>> Cc: Tony Lindgren
>>>>> Cc: Kevin Hilman
>>>>> Cc: Cousson, Benoit
>>>>> Cc: Paul Walmsley
>>>>> Cc: Mike Frysinger
>>>>> ---
>
>>>> For the davinci changes:
>
>>>> Acked-by: Kevin Hilman
>
>>>> Kevin
>
>>> [...]
>
>>>>> Index: linux-omap-pm/arch/arm/mach-davinci/usb.c
>>>>> ===
>>>>> --- linux-omap-pm.orig/arch/arm/mach-davinci/usb.c
>>>>> +++ linux-omap-pm/arch/arm/mach-davinci/usb.c
>>>>> @@ -64,10 +64,12 @@ static struct resource usb_resources[] =
>>>>> {
>>>>> .start = IRQ_USBINT,
>>>>> .flags = IORESOURCE_IRQ,
>>>>> + .name = "mc"
>>>>> },
>>>>> {
>>>>> /* placeholder for the dedicated CPPI IRQ */
>>>>> .flags = IORESOURCE_IRQ,
>>>>> + .name = "dma"
>>>>> },
>>>>> };
>
>>> Argh! This failed to also modify da8xx_usb20_resources[]... :-(
>
>> I think when I posted these patch, da8xx support was not there in
>> mainline.
>
> No, it was added long ago, before the glue layer itself.
>
>> I will send patch to add this.
>
> I will care about this myself now.
Thanks.
Hema
>
>> Regards,
>> Hema
>
> WBR, Sergei
>
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/7 V2] usb: otg: Kconfig: Add Kconfig option for TWL6030 transceiver.
On Fri, Dec 10, 2010 at 5:13 PM, Felipe Balbi wrote: > On Fri, Dec 10, 2010 at 04:56:40PM +0530, Kalliguddi, Hema wrote: >> >> On Fri, Dec 10, 2010 at 2:11 PM, Felipe Balbi wrote: >>> >>> On Fri, Dec 10, 2010 at 12:34:35PM +0530, Hema HK wrote: >>>> >>>> Added the TWL6030-usb transceiver option in the Kconfig >>>> >>>> Signed-off-by: Hema HK >>>> Cc: Felipe Balbi >>>> Cc: David Brownell >>>> --- >>>> drivers/usb/otg/Kconfig | 12 >>>> 1 file changed, 12 insertions(+) >>>> >>>> Index: usb/drivers/usb/otg/Kconfig >>>> === >>>> --- usb.orig/drivers/usb/otg/Kconfig >>>> +++ usb/drivers/usb/otg/Kconfig >>>> @@ -59,6 +59,18 @@ config TWL4030_USB >>>> This transceiver supports high and full speed devices plus, >>>> in host mode, low speed. >>>> >>>> +config TWL6030_USB >>>> + tristate "TWL6030 USB Transceiver Driver" >>>> + depends on TWL4030_CORE >>>> + select USB_OTG_UTILS >>>> + help >>>> + Enable this to support the USB OTG transceiver on TWL6030 >>>> + family chips. This TWL6030 transceiver has the VBUS and ID GND >>>> + and OTG SRP events capabilities. For all other transceiver >>>> functionality >>>> + UTMI PHY is embedded in OMAP4430. The internal PHY >>>> configurations APIs >>>> + are hooked to this driver through platform_data structure. >>>> + The definition of internal PHY APIs are in the mach-omap2 >>>> layer. >>>> + >>> >>> When compiling with this patch: >>> >>> drivers/usb/otg/twl6030-usb.c: In function 'twl6030_set_phy_clk': >>> drivers/usb/otg/twl6030-usb.c:145: error: 'struct twl4030_usb_data' has >>> no member named 'phy_set_clock' >>> drivers/usb/otg/twl6030-usb.c: In function 'twl6030_phy_init': >>> drivers/usb/otg/twl6030-usb.c:166: error: 'struct twl4030_usb_data' has >>> no member named 'phy_power' >>> drivers/usb/otg/twl6030-usb.c:168: error: 'struct twl4030_usb_data' has >>> no member named 'phy_power' >>> drivers/usb/otg/twl6030-usb.c: In function 'twl6030_phy_shutdown': >>> drivers/usb/otg/twl6030-usb.c:182: error: 'struct twl4030_usb_data' has >>> no member named 'phy_power' >>> drivers/usb/otg/twl6030-usb.c: In function 'twl6030_usb_probe': >>> drivers/usb/otg/twl6030-usb.c:440: error: 'struct twl4030_usb_data' has >>> no member named 'phy_init' >>> drivers/usb/otg/twl6030-usb.c: In function 'twl6030_usb_remove': >>> drivers/usb/otg/twl6030-usb.c:462: error: 'struct twl4030_usb_data' has >>> no member named 'phy_exit' >>> make[2]: *** [drivers/usb/otg/twl6030-usb.o] Error 1 >>> make[1]: *** [drivers/usb/otg] Error 2 >>> make[1]: *** Waiting for unfinished jobs >>> make: *** [drivers] Error 2 >>> make: *** Waiting for unfinished jobs >>> >>> please fix ASAP or we will loose the merge window. >> >> With this patch twl6030_usb file still not added for build, I am >> wondering how are >> you getting How are you getting build error? > > This is *the* patch that adds it to Kbuild :-) the previous patch > already added Makefile change. > Agree. Got confused with omap_phy_internal.c file. I am fixing it. I will send you the patches. Regards, Hema > -- > balbi > -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/7 V2] mfd: TWL6030: OMAP4: Registering the TWL6030-usb device
On Fri, Dec 10, 2010 at 4:00 PM, Felipe Balbi wrote:
> Hi,
>
> On Fri, Dec 10, 2010 at 12:34:50PM +0530, Hema HK wrote:
>>
>> Registering the twl6030-usb transceiver device as a child to twl6030 core.
>> Removed the NOP transceiver init call from board file.
>>
>> Populated twl4030_usb_data platform data structure with the function
>> pointers for OMAP4430 internal PHY operation to be used by twl630-usb
>> driver.
>>
>>
>
> one blank line only.
OK. I will fix it
>
>> Index: usb/include/linux/i2c/twl.h
>> ===
>> --- usb.orig/include/linux/i2c/twl.h
>> +++ usb/include/linux/i2c/twl.h
>> @@ -593,6 +593,13 @@ enum twl4030_usb_mode {
>>
>> struct twl4030_usb_data {
>> enum twl4030_usb_mode usb_mode;
>> +
>> + int (*phy_init)(struct device *dev);
>> + int (*phy_exit)(struct device *dev);
>> + /* Power on/off the PHY */
>> + int (*phy_power)(struct device *dev, int iD, int on);
>> + /* enable/disable phy clocks */
>> + int (*phy_set_clock)(struct device *dev, int on);
>> };
>
> This hunk should come either before or together with patch:
>
Agree this patch should be before 4th patch.
> usb: otg: Adding twl6030-usb transceiver driver for
>
>> struct twl4030_ins {
>> Index: usb/arch/arm/mach-omap2/board-omap4panda.c
>> ===
>> --- usb.orig/arch/arm/mach-omap2/board-omap4panda.c
>> +++ usb/arch/arm/mach-omap2/board-omap4panda.c
>> @@ -137,6 +137,13 @@ static struct omap_musb_board_data musb_
>> .power = 100,
>> };
>>
>> +static struct twl4030_usb_data omap4_usbphy_data = {
>> + .phy_init = omap4430_phy_init,
>> + .phy_exit = omap4430_phy_exit,
>> + .phy_power = omap4430_phy_power,
>> + .phy_set_clock = omap4430_phy_set_clk,
>> +};
>> +
>> static struct omap2_hsmmc_info mmc[] = {
>> {
>> .mmc = 1,
>> @@ -345,6 +352,7 @@ static struct twl4030_platform_data omap
>> .vaux1 = &omap4_panda_vaux1,
>> .vaux2 = &omap4_panda_vaux2,
>> .vaux3 = &omap4_panda_vaux3,
>> + .usb = &omap4_usbphy_data,
>> };
>>
>> static struct i2c_board_info __initdata omap4_panda_i2c_boardinfo[] = {
>
> This hunk should come before adding the Kconfig entries for twl6030-usb.
Agree. I will correct it submit in sometime.
Regards,
Hema
>
> --
> balbi
>
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/7 V2] usb: otg: Kconfig: Add Kconfig option for TWL6030 transceiver.
On Fri, Dec 10, 2010 at 2:11 PM, Felipe Balbi wrote: > On Fri, Dec 10, 2010 at 12:34:35PM +0530, Hema HK wrote: >> >> Added the TWL6030-usb transceiver option in the Kconfig >> >> Signed-off-by: Hema HK >> Cc: Felipe Balbi >> Cc: David Brownell >> --- >> drivers/usb/otg/Kconfig | 12 >> 1 file changed, 12 insertions(+) >> >> Index: usb/drivers/usb/otg/Kconfig >> === >> --- usb.orig/drivers/usb/otg/Kconfig >> +++ usb/drivers/usb/otg/Kconfig >> @@ -59,6 +59,18 @@ config TWL4030_USB >> This transceiver supports high and full speed devices plus, >> in host mode, low speed. >> >> +config TWL6030_USB >> + tristate "TWL6030 USB Transceiver Driver" >> + depends on TWL4030_CORE >> + select USB_OTG_UTILS >> + help >> + Enable this to support the USB OTG transceiver on TWL6030 >> + family chips. This TWL6030 transceiver has the VBUS and ID GND >> + and OTG SRP events capabilities. For all other transceiver >> functionality >> + UTMI PHY is embedded in OMAP4430. The internal PHY >> configurations APIs >> + are hooked to this driver through platform_data structure. >> + The definition of internal PHY APIs are in the mach-omap2 layer. >> + > > When compiling with this patch: > > drivers/usb/otg/twl6030-usb.c: In function 'twl6030_set_phy_clk': > drivers/usb/otg/twl6030-usb.c:145: error: 'struct twl4030_usb_data' has > no member named 'phy_set_clock' > drivers/usb/otg/twl6030-usb.c: In function 'twl6030_phy_init': > drivers/usb/otg/twl6030-usb.c:166: error: 'struct twl4030_usb_data' has > no member named 'phy_power' > drivers/usb/otg/twl6030-usb.c:168: error: 'struct twl4030_usb_data' has > no member named 'phy_power' > drivers/usb/otg/twl6030-usb.c: In function 'twl6030_phy_shutdown': > drivers/usb/otg/twl6030-usb.c:182: error: 'struct twl4030_usb_data' has > no member named 'phy_power' > drivers/usb/otg/twl6030-usb.c: In function 'twl6030_usb_probe': > drivers/usb/otg/twl6030-usb.c:440: error: 'struct twl4030_usb_data' has > no member named 'phy_init' > drivers/usb/otg/twl6030-usb.c: In function 'twl6030_usb_remove': > drivers/usb/otg/twl6030-usb.c:462: error: 'struct twl4030_usb_data' has > no member named 'phy_exit' > make[2]: *** [drivers/usb/otg/twl6030-usb.o] Error 1 > make[1]: *** [drivers/usb/otg] Error 2 > make[1]: *** Waiting for unfinished jobs > make: *** [drivers] Error 2 > make: *** Waiting for unfinished jobs > > please fix ASAP or we will loose the merge window. With this patch twl6030_usb file still not added for build, I am wondering how are you getting How are you getting build error? Regards, Hema > > -- > balbi > -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4] usb: musb: Adding names for IRQs in resource structure
On Fri, Dec 10, 2010 at 1:04 AM, Sergei Shtylyov wrote:
> Hello.
>
> Kevin Hilman wrote:
>
>>> The resource data is getting automatically populated from a set of data
>>> generated from TI's hardware database for the OMAP platform, While we
>>> could hack in some exceptions to that tool to generate resources
>>> in a specific order, it seems less fragile to use the resource name
>>> instead.That way, no matter what order the resources are generated, the
>>> driver still work.
>
>>> Modified the OMAP,Blackfin and Davinci architecture files to add the name
>>> of the IRQs
>>> in the resource structures and musb driver to use the get_irq_byname()
>>> api to
>>> get the device and dma irq numbers instead of using the index.
>
>>> Signed-off-by: Hema HK
>>> Cc: Felipe Balbi
>>> Cc: Tony Lindgren
>>> Cc: Kevin Hilman
>>> Cc: Cousson, Benoit
>>> Cc: Paul Walmsley
>>> Cc: Mike Frysinger
>>> ---
>
>> For the davinci changes:
>
>> Acked-by: Kevin Hilman
>
>> Kevin
>
> [...]
>
>>> Index: linux-omap-pm/arch/arm/mach-davinci/usb.c
>>> ===
>>> --- linux-omap-pm.orig/arch/arm/mach-davinci/usb.c
>>> +++ linux-omap-pm/arch/arm/mach-davinci/usb.c
>>> @@ -64,10 +64,12 @@ static struct resource usb_resources[] =
>>> {
>>> .start = IRQ_USBINT,
>>> .flags = IORESOURCE_IRQ,
>>> + .name = "mc"
>>> },
>>> {
>>> /* placeholder for the dedicated CPPI IRQ */
>>> .flags = IORESOURCE_IRQ,
>>> + .name = "dma"
>>> },
>>> };
>
> Argh! This failed to also modify da8xx_usb20_resources[]... :-(
I think when I posted these patch, da8xx support was not there in mainline.
I will send patch to add this.
Regards,
Hema
>
> WBR, Sergei
>
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 5/7] usb: musb: TWL6030: Selecting TWL6030_USB transceiver
On Wed, Dec 8, 2010 at 9:33 PM, Sergei Shtylyov wrote: > Hello. > > Hema HK wrote: > >> Selecting the twl6030-usb for OMAP4430SDP and OMAP4 PANDA board and >> adding OMAP4 internal phy code for compilation > >> Signed-off-by: Hema HK >> Cc: Felipe Balbi >> Cc: Tony Lindgren > > [...] > >> Index: linux-2.6/drivers/usb/musb/Kconfig >> === >> --- linux-2.6.orig/drivers/usb/musb/Kconfig >> +++ linux-2.6/drivers/usb/musb/Kconfig >> @@ -12,6 +12,7 @@ config USB_MUSB_HDRC >> depends on (ARM || (BF54x && !BF544) || (BF52x && !BF522 && >> !BF523)) >> select NOP_USB_XCEIV if (ARCH_DAVINCI || MACH_OMAP3EVM || BLACKFIN) >> select TWL4030_USB if MACH_OMAP_3430SDP >> + select TWL6030_USB if (MACH_OMAP_3430SDP || MACH_OMAP4_PANDA) > > Parens are not necessary. Though the previous code uses them... > OK. Regards, Hema > WBR, Sergei > > -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/7] usb: otg: Kconfig: Add Kconfig option for TWL6030 transceiver.
Hi, On Wed, Dec 8, 2010 at 9:35 PM, Sergei Shtylyov wrote: > Hello. > > Hema HK wrote: > >> Added the TWL6030-usb transceiver option in the Kconfig > >> Signed-off-by: Hema HK >> Cc: Felipe Balbi >> Cc: David Brownell > > [...] > >> Index: linux-2.6/drivers/usb/otg/Kconfig >> === >> --- linux-2.6.orig/drivers/usb/otg/Kconfig >> +++ linux-2.6/drivers/usb/otg/Kconfig >> @@ -59,6 +59,18 @@ config TWL4030_USB >> This transceiver supports high and full speed devices plus, >> in host mode, low speed. >> +config TWL6030_USB >> + tristate "TWL6030 USB Transceiver Driver" >> + depends on TWL4030_CORE >> + select USB_OTG_UTILS >> + help >> + Enable this to support the USB OTG transceiver on TWL6030 >> + family chips. This TWL6030 transceiver has the VBUS and ID GND >> + and OTG SRP events capabilities. For all other transceiver >> functionality >> + UTMI PHY is embedded in OMAP4430.The internal PHY configurations >> APIs > > ^^ > Space missing after period. I will fix it. Regards, Hema > > WBR, Sergei > > -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/7] usb: otg: Adding twl6030-usb transceiver driver for
On Wed, Dec 8, 2010 at 3:53 PM, Kalliguddi, Hema wrote:
> On Wed, Dec 8, 2010 at 3:20 PM, Felipe Balbi wrote:
>> On Wed, Dec 08, 2010 at 09:31:15PM +0530, Hema HK wrote:
>>>
>>> Adding the twl6030-usb transceiver support for OMAP4 musb driver.
>>>
>>> OMAP4 supports 2 types of transceiver interface.
>>>
>>> 1. UTMI: The PHY is embedded within OMAP4. The transceiver functionality
>>> is split between the twl6030 PMIC chip and OMAP4430. The VBUS, ID pin
>>> sensing and OTG SRP generation part is integrated in TWL6030 and UTMI PHY
>>> functionality is embedded within the OMAP4430.
>>>
>>> There is no direct interactions between the MUSB controller and TWL6030
>>> chip to communicate the session-valid, session-end and ID-GND events.
>>> It has to be done through a software by setting/resetting bits in
>>> one of the control module register of OMAP4430 which in turn toggles
>>> the appropriate signals to MUSB controller.
>>>
>>> The internal transceiver has functional clocks and
>>> powerdown bits to powerdown the PHY for power saving.
>>>
>>> Since there is no option available for having 2 transceiver drivers
>>> for one USB controller, internal PHY specific APIs are passed through
>>> plaform_data function pointers to use in the twl6030-usb transceiver
>>> driver.
>>>
>>> 2. ULPI interface is provided for off-chip transceivers.
>>>
>>> Signed-off-by: Hema HK
>>> Cc: Felipe Balbi
>>> Cc: Tony Lindgren
>>> Cc: David Brownell
>>> ---
>>> arch/arm/mach-omap2/omap_phy_internal.c | 133
>>> arch/arm/plat-omap/include/plat/usb.h | 4
>>> drivers/usb/otg/Makefile | 1
>>> drivers/usb/otg/twl6030-usb.c | 514
>>>
>>> 4 files changed, 652 insertions(+)
>>>
>>> Index: linux-2.6/arch/arm/mach-omap2/omap_phy_internal.c
>>> ===
>>> --- /dev/null
>>> +++ linux-2.6/arch/arm/mach-omap2/omap_phy_internal.c
>>> @@ -0,0 +1,148 @@
>>> +/*
>>> + * This file configures the internal USB PHY in OMAP4430. Used
>>> + * with TWL6030 transceiver and MUSB on OMAP4430.
>>> + *
>>> + * Copyright (C) 2010 Texas Instruments Incorporated - http://www.ti.com
>>> + * This program is free software; you can redistribute it and/or modify
>>> + * it under the terms of the GNU General Public License as published by
>>> + * the Free Software Foundation; either version 2 of the License, or
>>> + * (at your option) any later version.
>>> + *
>>> + *Author: Hema HK
>>
>> ^^ you need a space after *
> OK.
>>
>>> + * This program is distributed in the hope that it will be useful,
>>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>>> + * GNU General Public License for more details.
>>> + *
>>> + * You should have received a copy of the GNU General Public License
>>> + * along with this program; if not, write to the Free Software
>>> + * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
>>> + *
>>> + */
>>> +
>>> +#include
>>> +#include
>>> +#include
>>> +#include
>>> +#include
>>> +#include
>>> +#include
>>> +
>>> +#include
>>> +
>>> +/* OMAP control module register for UTMI PHY */
>>> +#define CONTROL_DEV_CONF 0x300
>>> +#define PHY_PD 0x1
>>> +
>>> +#define USBOTGHS_CONTROL 0x33c
>>> +#define AVALID BIT(0)
>>> +#define BVALID BIT(1)
>>> +#define VBUSVALID BIT(2)
>>> +#define SESSEND BIT(3)
>>> +#define IDDIG BIT(4)
>>> +
>>> +static struct clk *phyclk, *clk48m, *clk32k;
>>> +static void __iomem *ctrl_base;
>>> +
>>> +int omap4430_phy_init(struct device *dev)
>>> +{
>>> + ctrl_base = ioremap(OMAP443X_SCM_BASE, SZ_1K);
>>> + if (!ctrl_base) {
>>> + dev_err(dev, "control module ioremap failed\n");
>>> + return -ENOMEM;
>>> + }
>>> +
Re: [PATCH 2/7] usb: otg: Adding twl6030-usb transceiver driver for
On Wed, Dec 8, 2010 at 3:20 PM, Felipe Balbi wrote:
> On Wed, Dec 08, 2010 at 09:31:15PM +0530, Hema HK wrote:
>>
>> Adding the twl6030-usb transceiver support for OMAP4 musb driver.
>>
>> OMAP4 supports 2 types of transceiver interface.
>>
>> 1. UTMI: The PHY is embedded within OMAP4. The transceiver functionality
>> is split between the twl6030 PMIC chip and OMAP4430. The VBUS, ID pin
>> sensing and OTG SRP generation part is integrated in TWL6030 and UTMI PHY
>> functionality is embedded within the OMAP4430.
>>
>> There is no direct interactions between the MUSB controller and TWL6030
>> chip to communicate the session-valid, session-end and ID-GND events.
>> It has to be done through a software by setting/resetting bits in
>> one of the control module register of OMAP4430 which in turn toggles
>> the appropriate signals to MUSB controller.
>>
>> The internal transceiver has functional clocks and
>> powerdown bits to powerdown the PHY for power saving.
>>
>> Since there is no option available for having 2 transceiver drivers
>> for one USB controller, internal PHY specific APIs are passed through
>> plaform_data function pointers to use in the twl6030-usb transceiver
>> driver.
>>
>> 2. ULPI interface is provided for off-chip transceivers.
>>
>> Signed-off-by: Hema HK
>> Cc: Felipe Balbi
>> Cc: Tony Lindgren
>> Cc: David Brownell
>> ---
>> arch/arm/mach-omap2/omap_phy_internal.c | 133
>> arch/arm/plat-omap/include/plat/usb.h | 4
>> drivers/usb/otg/Makefile | 1
>> drivers/usb/otg/twl6030-usb.c | 514
>>
>> 4 files changed, 652 insertions(+)
>>
>> Index: linux-2.6/arch/arm/mach-omap2/omap_phy_internal.c
>> ===
>> --- /dev/null
>> +++ linux-2.6/arch/arm/mach-omap2/omap_phy_internal.c
>> @@ -0,0 +1,148 @@
>> +/*
>> + * This file configures the internal USB PHY in OMAP4430. Used
>> + * with TWL6030 transceiver and MUSB on OMAP4430.
>> + *
>> + * Copyright (C) 2010 Texas Instruments Incorporated - http://www.ti.com
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published by
>> + * the Free Software Foundation; either version 2 of the License, or
>> + * (at your option) any later version.
>> + *
>> + *Author: Hema HK
>
> ^^ you need a space after *
OK.
>
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> + * GNU General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU General Public License
>> + * along with this program; if not, write to the Free Software
>> + * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
>> + *
>> + */
>> +
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +
>> +#include
>> +
>> +/* OMAP control module register for UTMI PHY */
>> +#define CONTROL_DEV_CONF 0x300
>> +#define PHY_PD 0x1
>> +
>> +#define USBOTGHS_CONTROL 0x33c
>> +#define AVALID BIT(0)
>> +#define BVALID BIT(1)
>> +#define VBUSVALID BIT(2)
>> +#define SESSEND BIT(3)
>> +#define IDDIG BIT(4)
>> +
>> +static struct clk *phyclk, *clk48m, *clk32k;
>> +static void __iomem *ctrl_base;
>> +
>> +int omap4430_phy_init(struct device *dev)
>> +{
>> + ctrl_base = ioremap(OMAP443X_SCM_BASE, SZ_1K);
>> + if (!ctrl_base) {
>> + dev_err(dev, "control module ioremap failed\n");
>> + return -ENOMEM;
>> + }
>> + /* Power down the phy */
>> + __raw_writel(PHY_PD, ctrl_base + CONTROL_DEV_CONF);
>> + phyclk = clk_get(dev, "ocp2scp_usb_phy_ick");
>> +
>> + if (IS_ERR(phyclk)) {
>> + dev_err(dev, "cannot clk_get ocp2scp_usb_phy_ick\n");
>> + iounmap(ctrl_base);
>> + return PTR_ERR(phyclk);
>> + }
>> +
>> + clk48m = clk_get(dev, "ocp2scp_usb_phy_phy_48m");
>> + if (IS_ERR(clk48m)) {
>> + dev_err(dev, "cannot clk_get ocp2scp_usb_phy_phy_48m\n");
>> + clk_put(phyclk);
>> + iounmap(ctrl_base);
>> + return PTR_ERR(clk48m);
>> + }
>> +
>> + clk32k = clk_get(dev, "usb_phy_cm_clk32k");
>> + if (IS_ERR(clk32k)) {
>> + dev_err(dev, "cannot clk_get usb_phy_cm_clk32k\n");
>> + clk_put(phyclk);
>> + clk_put(clk48m);
>> + iounmap(ctrl_base);
>> + return PTR_ERR(clk32k);
>> + }
>> + return 0;
>> +}
>> +
>> +int omap4430_phy_set_clk(struct device *dev, int on)
>>
Re: [PATCH 0/7] usb: otg: TWL6030: OMAP4430: musb & transceiver driver support for OMAP4
Please ignore this series of patches. I sent the older version the patches by mistake. I will send new versions again. Sorry for cluttering .. Regards, Hema On Wed, Dec 8, 2010 at 5:39 AM, Hema HK wrote: > This patch series has the support for TWL6030-usb > transceiver driver and changes in the musb driver to make it functional > with OMAP4430. > > OMAP4 musb support UTMI and ULPI transceiver interfaces. > > In UTMI mode, the transceiver functionality is split between > the TWL6030 PMIC chip and OMAP4 embedded PHY. > The TWL6030 transceiver driver code is under otg folder and > internal UTMI PHY specific code changes are defined under > mach-omap2 directory and functions are passed through platform_data > structure. > > This patch series is based on V2.6.37-rc4 + [1]+[2]+[3]+[4]. > > [1] http://www.listware.net/201011/linux-usb/ > 65625-patch-resend-v3-usb-musb-do-not-use-dma-for-control-transfers.html > > [2] http://www.spinics.net/lists/linux-usb/msg37105.html > > [3] http://permalink.gmane.org/gmane.linux.usb.general/37123 > > [4] Felipe's musb-reorg patch series. > http://gitorious.org/usb/usb/commits/musb-hw > > Tested musb device and host mode functionality with OMAP4430SDP > and OMAP3630 ZOOM3. > > Cc: Felipe Balbi > Cc: Tony Lindgren > Cc: David Brownell > Cc: Samuel Ortiz > --- > > Hema HK (7): > mfd: TWL6030: USBOTG VBUS event generation on charger VBUS events. > usb: otg: Adding twl6030-usb transceiver driver for OMAP4430 musb > usb: otg: Kconfig: Add Kconfig option for TWL6030 transceiver. > mfd: TWL6030: OMAP4: Registering the TWL6030-usb device > usb: musb: TWL6030: Selecting TWL6030_USB transceiver for OMAP4 > usb: musb: Adding musb support for OMAP4430 > arm: OMAP4430: musb: Enable musb device initialization for OMAP4430 > > arch/arm/mach-omap2/Makefile | 1 + > arch/arm/mach-omap2/board-4430sdp.c | 15 +- > arch/arm/mach-omap2/board-omap4panda.c | 9 +- > arch/arm/mach-omap2/omap_phy_internal.c | 123 +++ > arch/arm/plat-omap/include/plat/usb.h | 4 + > drivers/mfd/twl-core.c | 44 +++- > drivers/mfd/twl6030-irq.c | 9 +- > drivers/usb/musb/Kconfig | 1 + > drivers/usb/musb/musb_core.c | 5 + > drivers/usb/musb/musb_core.h | 1 + > drivers/usb/musb/musb_gadget.c | 1 + > drivers/usb/musb/omap2430.c | 85 +- > drivers/usb/otg/Kconfig | 11 + > drivers/usb/otg/Makefile | 1 + > drivers/usb/otg/twl6030-usb.c | 527 > +++ > include/linux/i2c/twl.h | 6 + > 16 files changed, 824 insertions(+), 19 deletions(-) > > > -- > To unsubscribe from this list: send the line "unsubscribe linux-omap" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 9/9 v3] usb : musb: Offmode fix for idle path
>-Original Message- >From: Kevin Hilman [mailto:khil...@deeprootsystems.com] >Sent: Tuesday, September 28, 2010 9:14 PM >To: Kalliguddi, Hema >Cc: linux-omap@vger.kernel.org; linux-...@vger.kernel.org; >Mankad, Maulik Ojas; Balbi, Felipe; Tony Lindgren; Cousson, >Benoit; Paul Walmsley >Subject: Re: [PATCH 9/9 v3] usb : musb: Offmode fix for idle path > >"Kalliguddi, Hema" writes: > >>>-Original Message- >>>From: Kevin Hilman [mailto:khil...@deeprootsystems.com] >>>Sent: Tuesday, September 28, 2010 12:27 AM >>>To: Kalliguddi, Hema >>>Cc: linux-omap@vger.kernel.org; linux-...@vger.kernel.org; >>>Mankad, Maulik Ojas; Balbi, Felipe; Tony Lindgren; Cousson, >>>Benoit; Paul Walmsley >>>Subject: Re: [PATCH 9/9 v3] usb : musb: Offmode fix for idle path >>> >>>"Kalliguddi, Hema" writes: >>> >>>>>-Original Message- >>>>>From: Kevin Hilman [mailto:khil...@deeprootsystems.com] >>>>>Sent: Saturday, September 25, 2010 1:12 AM >>>>>To: Kalliguddi, Hema >>>>>Cc: linux-omap@vger.kernel.org; linux-...@vger.kernel.org; >>>>>Mankad, Maulik Ojas; Balbi, Felipe; Tony Lindgren; Cousson, >>>>>Benoit; Paul Walmsley >>>>>Subject: Re: [PATCH 9/9 v3] usb : musb: Offmode fix for idle path >>>>> >>>>>Hema HK writes: >>>>> >>>>>> With OMAP core-off support musb was not functional as >>>>>context was getting >>>>>> lost after wakeup from core-off. >>>>> >>>>>This should be a separate patch. >>>>> >>>> >>>> Let me give the description of the musb offmode support in >>>the idle path. >>> >>>The detail you provided below is very good, and this is the level of >>>detail that needs to go into the changelogs. >>> >>>> With the current mainline code, offmode in the idle path is >>>not supported with usb. >>>> When the core hits off and wakes up the musb will not be >functional. >>>> This patch is to support the musb functionality with offmode >>>enabled in the idle path. >>> >>>OK, what about the PM branch. I was under the impression >that offmode >>>was working fine in the PM branch. >>> >> In current PM branch, the core hits retention and off mode >without usb drive loaded. >> Once after loading the musb driver it may not retention/off >also as the driver is >> configuring musb in smart idle/standby. Even if it hits >retention/off as it might >> work sometimes,MUSB is not functional.because there is no >context save/restore >> done in the driver. >> >>>And, looking at the PM branch, the only thing done in the >idle path is >>>the disable of autoidle upon wakeup. Everything else (context >>>save/restore etc.) is done in the driver. >>> >> No.There is no context save/restore done in the driver today. > >There is, just not in the idle path as you pointed out. > >The point of all this is that the changelog and documentation is not >clear (at least to me) about all these details save/restore in idle, or >in suspend/resume etc., and why the result is different (or needs to be >different) from the current code. > There is no difference in the settings or the result in the idle path or global suspend/resume path. GLOBAL SUSPEND/RESUME: - In the current PM line, musb is configured to force idle and forcestandby mode by default when the device is registered. When there is no driver loaded, theere is no issue, core hits retention/offmode. When the musb driver is loaded, it configures it into smart idle/standby mode. So when pm_ops .suspend/resume apis are called during global suspend/resume, context save/restore and clock disable/enable is happening, the observation is that sometimes, the core hits retention/offmode, but not consistently. The recomandation from the ip team to configure it to force idle/standby mode when device is not used. For the global suspend/resume, this is being achieved by setting the oh flags (HWMOD_SWSUP_SIDLE and HWMOD_SWSUP_MSTANDBY)in the hwmod data structure for OMAP2 OMAP3 and OMAP4.Description of why these flags are needed are mentioned in the respective patches. By calling runtime put_sync and get_sync apis in the driver pm_ops .suspend/resume will configure the musb in the required mode and save/restore the context is taken care in the runtime_suspend/resume hooks. This change is done in the [PATCH 8/9 v3] usb : musb: Using runtime pm apis for musb. IDLE PATH RRETENTION/
RE: [PATCH v4] usb: musb: Adding names for IRQs in resource structure
>-Original Message-
>From: Balbi, Felipe
>Sent: Wednesday, September 29, 2010 3:41 PM
>To: Kalliguddi, Hema
>Cc: Balbi, Felipe; linux-omap@vger.kernel.org;
>linux-...@vger.kernel.org; Tony Lindgren; Kevin Hilman;
>Cousson, Benoit; Paul Walmsley; Mike Frysinger
>Subject: Re: [PATCH v4] usb: musb: Adding names for IRQs in
>resource structure
>
>On Wed, Sep 29, 2010 at 03:54:45AM -0500, Kalliguddi, Hema wrote:
>> Hi,
>>
>>>-Original Message-
>>>From: Balbi, Felipe
>>>Sent: Wednesday, September 29, 2010 12:30 PM
>>>To: Kalliguddi, Hema
>>>Cc: linux-omap@vger.kernel.org; linux-...@vger.kernel.org;
>>>Balbi, Felipe; Tony Lindgren; Kevin Hilman; Cousson, Benoit;
>>>Paul Walmsley; Mike Frysinger
>>>Subject: Re: [PATCH v4] usb: musb: Adding names for IRQs in
>>>resource structure
>>>
>>>Hi,
>>>
>>>On Wed, Sep 29, 2010 at 11:26:39AM -0500, Kalliguddi, Hema wrote:
>>>>Index: linux-omap-pm/arch/arm/mach-davinci/usb.c
>>>>===
>>>>--- linux-omap-pm.orig/arch/arm/mach-davinci/usb.c
>>>>+++ linux-omap-pm/arch/arm/mach-davinci/usb.c
>>>>@@ -64,10 +64,12 @@ static struct resource usb_resources[] =
>>>>{
>>>>.start = IRQ_USBINT,
>>>>.flags = IORESOURCE_IRQ,
>>>>+ .name = "mc"
>>>>},
>>>>{
>>>>/* placeholder for the dedicated CPPI IRQ */
>>>>.flags = IORESOURCE_IRQ,
>>>>+ .name = "dma"
>>>>},
>>>> };
>>>
>>>you add names only to the IRQs, this will still require the
>memory base
>>>to be the first resource. Maybe adding a name like "regs" to
>the memory
>>>base and changing to platform_get_resource_byname(pdev, "regs") on
>>>musb-core.c
>>>
>>It does not mandate the base address to be the first
>resource, as the api
>>used for getting the base address takes the resource type as
>parameter.
>>"platform_get_resource(struct platform_device *dev,unsigned int type,
>>unsigned int num)". but it mandates the base addresses have to be in
>>certain order if there are multiple base address.Since for musb there
>>is one base address.
>
>true :-p Sorry about that. Just noise.
>
That's Ok...
>--
>balbi
>--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v4] usb: musb: Adding names for IRQs in resource structure
Hi,
>-Original Message-
>From: Balbi, Felipe
>Sent: Wednesday, September 29, 2010 12:30 PM
>To: Kalliguddi, Hema
>Cc: linux-omap@vger.kernel.org; linux-...@vger.kernel.org;
>Balbi, Felipe; Tony Lindgren; Kevin Hilman; Cousson, Benoit;
>Paul Walmsley; Mike Frysinger
>Subject: Re: [PATCH v4] usb: musb: Adding names for IRQs in
>resource structure
>
>Hi,
>
>On Wed, Sep 29, 2010 at 11:26:39AM -0500, Kalliguddi, Hema wrote:
>>Index: linux-omap-pm/arch/arm/mach-davinci/usb.c
>>===
>>--- linux-omap-pm.orig/arch/arm/mach-davinci/usb.c
>>+++ linux-omap-pm/arch/arm/mach-davinci/usb.c
>>@@ -64,10 +64,12 @@ static struct resource usb_resources[] =
>> {
>> .start = IRQ_USBINT,
>> .flags = IORESOURCE_IRQ,
>>+ .name = "mc"
>> },
>> {
>> /* placeholder for the dedicated CPPI IRQ */
>> .flags = IORESOURCE_IRQ,
>>+ .name = "dma"
>> },
>> };
>
>you add names only to the IRQs, this will still require the memory base
>to be the first resource. Maybe adding a name like "regs" to the memory
>base and changing to platform_get_resource_byname(pdev, "regs") on
>musb-core.c
>
It does not mandate the base address to be the first resource, as the api
used for getting the base address takes the resource type as parameter.
"platform_get_resource(struct platform_device *dev,unsigned int type,
unsigned int num)". but it mandates the base addresses have to be in
certain order if there are multiple base address.Since for musb there
is one base address.
>--
>balbi
>--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 2/9 v3] usb: musb: Remove board_data parameter from musb_platform_init()
Hi,
>-Original Message-
>From: Kalliguddi, Hema
>Sent: Thursday, September 23, 2010 5:58 AM
>To: linux-omap@vger.kernel.org; linux-...@vger.kernel.org
>Cc: Kalliguddi, Hema; Balbi, Felipe; Tony Lindgren; Kevin
>Hilman; Cousson, Benoit; Paul Walmsley
>Subject: [PATCH 2/9 v3] usb: musb: Remove board_data parameter
>from musb_platform_init()
>
>Removed the board_data parameter being passed to
>musb_platform_init function
>as board data can be extracted from device structure which is
>already member of
>musb structure.
>
>Signed-off-by: Hema HK
>Cc: Felipe Balbi
>Cc: Tony Lindgren
>Cc: Kevin Hilman
>Cc: Cousson, Benoit
>Cc: Paul Walmsley
>
>---
> drivers/usb/musb/blackfin.c |2 +-
> drivers/usb/musb/davinci.c |2 +-
> drivers/usb/musb/musb_core.c |2 +-
> drivers/usb/musb/musb_core.h |2 +-
> drivers/usb/musb/omap2430.c |6 --
> drivers/usb/musb/tusb6010.c |2 +-
> 6 files changed, 9 insertions(+), 7 deletions(-)
>
>Index: linux-omap-pm/drivers/usb/musb/blackfin.c
>===
>--- linux-omap-pm.orig/drivers/usb/musb/blackfin.c
>+++ linux-omap-pm/drivers/usb/musb/blackfin.c
>@@ -323,7 +323,7 @@ int musb_platform_set_mode(struct musb *
> return -EIO;
> }
>
>-int __init musb_platform_init(struct musb *musb, void *board_data)
>+int __init musb_platform_init(struct musb *musb)
> {
>
> /*
>Index: linux-omap-pm/drivers/usb/musb/davinci.c
>===
>--- linux-omap-pm.orig/drivers/usb/musb/davinci.c
>+++ linux-omap-pm/drivers/usb/musb/davinci.c
>@@ -376,7 +376,7 @@ int musb_platform_set_mode(struct musb *
> return -EIO;
> }
>
>-int __init musb_platform_init(struct musb *musb, void *board_data)
>+int __init musb_platform_init(struct musb *musb)
> {
> void __iomem*tibase = musb->ctrl_base;
> u32 revision;
>Index: linux-omap-pm/drivers/usb/musb/musb_core.c
>===
>--- linux-omap-pm.orig/drivers/usb/musb/musb_core.c
>+++ linux-omap-pm/drivers/usb/musb/musb_core.c
>@@ -2022,7 +2022,7 @@ bad_config:
>* isp1504, non-OTG, etc) mostly hooking up through ULPI.
>*/
> musb->isr = generic_interrupt;
>- status = musb_platform_init(musb, plat->board_data);
>+ status = musb_platform_init(musb);
> if (status < 0)
> goto fail2;
>
>Index: linux-omap-pm/drivers/usb/musb/musb_core.h
>===
>--- linux-omap-pm.orig/drivers/usb/musb/musb_core.h
>+++ linux-omap-pm/drivers/usb/musb/musb_core.h
>@@ -612,7 +612,7 @@ extern int musb_platform_get_vbus_status
> #define musb_platform_get_vbus_status(x) 0
> #endif
>
>-extern int __init musb_platform_init(struct musb *musb, void
>*board_data);
>+extern int __init musb_platform_init(struct musb *musb);
> extern int musb_platform_exit(struct musb *musb);
>
> #endif/* __MUSB_CORE_H__ */
>Index: linux-omap-pm/drivers/usb/musb/omap2430.c
>===
>--- linux-omap-pm.orig/drivers/usb/musb/omap2430.c
>+++ linux-omap-pm/drivers/usb/musb/omap2430.c
>@@ -187,10 +187,12 @@ int musb_platform_set_mode(struct musb *
> return 0;
> }
>
>-int __init musb_platform_init(struct musb *musb, void *board_data)
>+int __init musb_platform_init(struct musb *musb)
> {
> u32 l;
>- struct omap_musb_board_data *data = board_data;
>+ struct device *dev = musb->controller;
>+ struct musb_hdrc_platform_data *plat = dev->platform_data;
>+ struct omap_musb_board_data *data = plat->board_data;
>
> /* We require some kind of external transceiver, hooked
>* up through ULPI. TWL4030-family PMICs include one,
>Index: linux-omap-pm/drivers/usb/musb/tusb6010.c
>===
>--- linux-omap-pm.orig/drivers/usb/musb/tusb6010.c
>+++ linux-omap-pm/drivers/usb/musb/tusb6010.c
>@@ -1091,7 +1091,7 @@ err:
> return -ENODEV;
> }
>
>-int __init musb_platform_init(struct musb *musb, void *board_data)
>+int __init musb_platform_init(struct musb *musb)
> {
> struct platform_device *pdev;
> struct resource *mem;
>
Any comments on this patch?
Regards,
Hema
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 1/9 v3] usb: musb: Adding names for IRQs in resource structure
Felipe, >-Original Message- >From: Balbi, Felipe >Sent: Wednesday, September 29, 2010 12:01 PM >To: Kalliguddi, Hema >Cc: Mike Frysinger; Paul Walmsley; linux-omap@vger.kernel.org; >linux-...@vger.kernel.org; Balbi, Felipe; Tony Lindgren; Kevin >Hilman; Cousson, Benoit >Subject: Re: [PATCH 1/9 v3] usb: musb: Adding names for IRQs >in resource structure > >Hi, > >On Tue, Sep 28, 2010 at 10:32:07PM -0500, Kalliguddi, Hema wrote: >>I will repost the patches today for blackfin and omap changes >with the changelog >>updated accordingly. > >Have you re-sent the patches ? I just want to know if we're gonna merge >it through Greg or the patch will be split and merged through ARCH >trees. > >Each way if fine by me, but 2.6.37 is getting really close. > Then I will send a patch with changelog updated soon. >-- >balbi >-- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 1/9 v3] usb: musb: Adding names for IRQs in resource structure
Hi, >-Original Message- >From: vapier@gmail.com [mailto:vapier@gmail.com] On >Behalf Of Mike Frysinger >Sent: Wednesday, September 29, 2010 3:18 AM >To: Paul Walmsley >Cc: Kalliguddi, Hema; linux-omap@vger.kernel.org; >linux-...@vger.kernel.org; Balbi, Felipe; Tony Lindgren; Kevin >Hilman; Cousson, Benoit >Subject: Re: [PATCH 1/9 v3] usb: musb: Adding names for IRQs >in resource structure > >On Tue, Sep 28, 2010 at 17:44, Paul Walmsley wrote: >> On Tue, 28 Sep 2010, Mike Frysinger wrote: >>> On Tue, Sep 28, 2010 at 17:18, Paul Walmsley wrote: >>> > On Tue, 28 Sep 2010, Mike Frysinger wrote: >>> >> well, currently, your resource definition must always be >in the order >>> >> of dev int and then dma int. if it isnt, then i dont >think musb is >>> >> going to work. >>> >> >>> >> not sure why you wouldnt simply change your platform resources to >>> >> match the what the driver expects ... >>> > >>> > The resource data is getting automatically populated from >a set of data >>> > generated from TI's hardware database for the OMAP >platform, at least. >>> > While we could hack in some exceptions to that tool to >generate resources >>> > in a specific order, it seems less fragile to use the >resource name >>> > instead. That way, no matter what order the resources >are generated, the >>> > driver should still work. >>> >>> guessing you're not referring to a device tree setup, but something >>> even more convoluted ? >> >> No need to disparage it before you've seen it :-) > >ah, but that's when i do my best work > >>> i'd highly suggest that this patch be resent with the info >you've just >>> provided in its changelog so people can get a better >understanding of >>> the "why" ... i'm not against the patch, it just seemed to >be lacking >>> any background info >> >> Hema, would you care to update the patch changelog accordingly? > >i'll pre-ack that for the Blackfin changes ... or, if you split those >out, i can merge it separately for 2.6.37. I will repost the patches today for blackfin and omap changes with the changelog updated accordingly. >-mike >-- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 9/9 v3] usb : musb: Offmode fix for idle path
>-Original Message- >From: linux-usb-ow...@vger.kernel.org >[mailto:linux-usb-ow...@vger.kernel.org] On Behalf Of Kalliguddi, Hema >Sent: Tuesday, September 28, 2010 10:07 AM >To: Kevin Hilman >Cc: linux-omap@vger.kernel.org; linux-...@vger.kernel.org; >Mankad, Maulik Ojas; Balbi, Felipe; Tony Lindgren; Cousson, >Benoit; Paul Walmsley >Subject: RE: [PATCH 9/9 v3] usb : musb: Offmode fix for idle path > >Kevin, > >>-Original Message- >>From: Kevin Hilman [mailto:khil...@deeprootsystems.com] >>Sent: Tuesday, September 28, 2010 12:27 AM >>To: Kalliguddi, Hema >>Cc: linux-omap@vger.kernel.org; linux-...@vger.kernel.org; >>Mankad, Maulik Ojas; Balbi, Felipe; Tony Lindgren; Cousson, >>Benoit; Paul Walmsley >>Subject: Re: [PATCH 9/9 v3] usb : musb: Offmode fix for idle path >> >>"Kalliguddi, Hema" writes: >> >>>>-Original Message- >>>>From: Kevin Hilman [mailto:khil...@deeprootsystems.com] >>>>Sent: Saturday, September 25, 2010 1:12 AM >>>>To: Kalliguddi, Hema >>>>Cc: linux-omap@vger.kernel.org; linux-...@vger.kernel.org; >>>>Mankad, Maulik Ojas; Balbi, Felipe; Tony Lindgren; Cousson, >>>>Benoit; Paul Walmsley >>>>Subject: Re: [PATCH 9/9 v3] usb : musb: Offmode fix for idle path >>>> >>>>Hema HK writes: >>>> >>>>> With OMAP core-off support musb was not functional as >>>>context was getting >>>>> lost after wakeup from core-off. >>>> >>>>This should be a separate patch. >>>> >>> >>> Let me give the description of the musb offmode support in >>the idle path. >> >>The detail you provided below is very good, and this is the level of >>detail that needs to go into the changelogs. >> >>> With the current mainline code, offmode in the idle path is >>not supported with usb. >>> When the core hits off and wakes up the musb will not be functional. >>> This patch is to support the musb functionality with offmode >>enabled in the idle path. >> >>OK, what about the PM branch. I was under the impression that offmode >>was working fine in the PM branch. >> >In current PM branch, the core hits retention and off mode >without usb drive loaded. >Once after loading the musb driver it may not retention/off >also as the driver is >configuring musb in smart idle/standby. Even if it hits >retention/off as it might >work sometimes,MUSB is not functional.because there is no >context save/restore >done in the driver. > Just to clarify, I mean there is no context save/restore done in the sleep while idle path. It is done for global suspend path >>And, looking at the PM branch, the only thing done in the >idle path is >>the disable of autoidle upon wakeup. Everything else (context >>save/restore etc.) is done in the driver. >> >No.There is no context save/restore done in the driver today. > >>> Below are the requirements to support retention and offmode >>of OMAP in idle path with usb enabled >>> During idle and when there is no activity on the bus: >>> >>> 1.Since there is no hardware context save/restore supported >>in OMAP for musb, software >>> has to save the context. >>> 2.Configure the musb in force idle/standby mode during idle mode >> >>This needs more detailed description (TRM reference, etc.) >> >I have provided the link to the public TRM and I had refered >to the exact section >of the TRM sometime in the older version the patches, when >Benoit and you had asked >for the description. > >>> 3.May or may not disable the interface clock(as interface >>clock is autogated) >>> and the functional clock is from ULPI phy on triton chip. >>> >>> When OMAP awakes: >>> >>> 1.enable the clock if it is disabled.and >>> 2.Configure it back to no idle/standby or smart idle/standby >>after wakeup. >> >>In the PM branch, this part is done in idle. >> >I only see the disable autoidle bit function the idle path in >pm branch. >There is no code for setting the no idle/standby or smart >idle/standby bits >in the idle path. > >>> 3.Restore the context back. >> >>But this is done by the driver. >> >No. current driver is not doing any context save/restore. > >>> Idling of device can be done when there is no activity on >>the bus by using the pm_runtime_put_sync >>n> apis in when device disconnects or suspends, but resuming >>has to done as soon as the omap is
RE: [PATCH 9/9 v3] usb : musb: Offmode fix for idle path
Kevin, >-Original Message- >From: Kevin Hilman [mailto:khil...@deeprootsystems.com] >Sent: Tuesday, September 28, 2010 12:27 AM >To: Kalliguddi, Hema >Cc: linux-omap@vger.kernel.org; linux-...@vger.kernel.org; >Mankad, Maulik Ojas; Balbi, Felipe; Tony Lindgren; Cousson, >Benoit; Paul Walmsley >Subject: Re: [PATCH 9/9 v3] usb : musb: Offmode fix for idle path > >"Kalliguddi, Hema" writes: > >>>-Original Message- >>>From: Kevin Hilman [mailto:khil...@deeprootsystems.com] >>>Sent: Saturday, September 25, 2010 1:12 AM >>>To: Kalliguddi, Hema >>>Cc: linux-omap@vger.kernel.org; linux-...@vger.kernel.org; >>>Mankad, Maulik Ojas; Balbi, Felipe; Tony Lindgren; Cousson, >>>Benoit; Paul Walmsley >>>Subject: Re: [PATCH 9/9 v3] usb : musb: Offmode fix for idle path >>> >>>Hema HK writes: >>> >>>> With OMAP core-off support musb was not functional as >>>context was getting >>>> lost after wakeup from core-off. >>> >>>This should be a separate patch. >>> >> >> Let me give the description of the musb offmode support in >the idle path. > >The detail you provided below is very good, and this is the level of >detail that needs to go into the changelogs. > >> With the current mainline code, offmode in the idle path is >not supported with usb. >> When the core hits off and wakes up the musb will not be functional. >> This patch is to support the musb functionality with offmode >enabled in the idle path. > >OK, what about the PM branch. I was under the impression that offmode >was working fine in the PM branch. > In current PM branch, the core hits retention and off mode without usb drive loaded. Once after loading the musb driver it may not retention/off also as the driver is configuring musb in smart idle/standby. Even if it hits retention/off as it might work sometimes,MUSB is not functional.because there is no context save/restore done in the driver. >And, looking at the PM branch, the only thing done in the idle path is >the disable of autoidle upon wakeup. Everything else (context >save/restore etc.) is done in the driver. > No.There is no context save/restore done in the driver today. >> Below are the requirements to support retention and offmode >of OMAP in idle path with usb enabled >> During idle and when there is no activity on the bus: >> >> 1.Since there is no hardware context save/restore supported >in OMAP for musb, software >> has to save the context. >> 2.Configure the musb in force idle/standby mode during idle mode > >This needs more detailed description (TRM reference, etc.) > I have provided the link to the public TRM and I had refered to the exact section of the TRM sometime in the older version the patches, when Benoit and you had asked for the description. >> 3.May or may not disable the interface clock(as interface >clock is autogated) >> and the functional clock is from ULPI phy on triton chip. >> >> When OMAP awakes: >> >> 1.enable the clock if it is disabled.and >> 2.Configure it back to no idle/standby or smart idle/standby >after wakeup. > >In the PM branch, this part is done in idle. > I only see the disable autoidle bit function the idle path in pm branch. There is no code for setting the no idle/standby or smart idle/standby bits in the idle path. >> 3.Restore the context back. > >But this is done by the driver. > No. current driver is not doing any context save/restore. >> Idling of device can be done when there is no activity on >the bus by using the pm_runtime_put_sync >n> apis in when device disconnects or suspends, but resuming >has to done as soon as the omap is wokenup from >> retention or core off, as we have to put back the musb in >known state ie restore the conext atleast >> enabling D+/D- lines,enabling interrupts and configuring the >no idle/standby or smart idle/standby >> to even capture the irqs. Otherwise we will not be able to >capture the musb connect/reset or resume/remote >> wakeup events as D+/D- lines will disabled when the context >is lost duribg offmode. >> >> If I use the pm_runtime_put_sync in disconnect/suspend >handler when device suspends/disconnects >> and use pm_runtime_get_sync when OMAP wakesup, then there >will be mismatch in the usecount. >> >> We could have achieved the same by using the triton >connect/disconnect events to idle/resume musb, >> but some of the phys do not support the connect/disconnect events. >> >> So cameup with the design of idling musb device in idle loop >and resume once the OMAP wakes up. >>
RE: [PATCH 9/9 v3] usb : musb: Offmode fix for idle path
Hi, >-Original Message- >From: Kevin Hilman [mailto:khil...@deeprootsystems.com] >Sent: Saturday, September 25, 2010 1:12 AM >To: Kalliguddi, Hema >Cc: linux-omap@vger.kernel.org; linux-...@vger.kernel.org; >Mankad, Maulik Ojas; Balbi, Felipe; Tony Lindgren; Cousson, >Benoit; Paul Walmsley >Subject: Re: [PATCH 9/9 v3] usb : musb: Offmode fix for idle path > >Hema HK writes: > >> With OMAP core-off support musb was not functional as >context was getting >> lost after wakeup from core-off. > >This should be a separate patch. > Let me give the description of the musb offmode support in the idle path. With the current mainline code, offmode in the idle path is not supported with usb. When the core hits off and wakes up the musb will not be functional. This patch is to support the musb functionality with offmode enabled in the idle path. Below are the requirements to support retention and offmode of OMAP in idle path with usb enabled During idle and when there is no activity on the bus: 1.Since there is no hardware context save/restore supported in OMAP for musb, software has to save the context. 2.Configure the musb in force idle/standby mode during idle mode 3.May or may not disable the interface clock(as interface clock is autogated) and the functional clock is from ULPI phy on triton chip. When OMAP awakes: 1.enable the clock if it is disabled.and 2.Configure it back to no idle/standby or smart idle/standby after wakeup. 3.Restore the context back. Idling of device can be done when there is no activity on the bus by using the pm_runtime_put_sync apis in when device disconnects or suspends, but resuming has to done as soon as the omap is wokenup from retention or core off, as we have to put back the musb in known state ie restore the conext atleast enabling D+/D- lines,enabling interrupts and configuring the no idle/standby or smart idle/standby to even capture the irqs. Otherwise we will not be able to capture the musb connect/reset or resume/remote wakeup events as D+/D- lines will disabled when the context is lost duribg offmode. If I use the pm_runtime_put_sync in disconnect/suspend handler when device suspends/disconnects and use pm_runtime_get_sync when OMAP wakesup, then there will be mismatch in the usecount. We could have achieved the same by using the triton connect/disconnect events to idle/resume musb, but some of the phys do not support the connect/disconnect events. So cameup with the design of idling musb device in idle loop and resume once the OMAP wakes up. All this done onl when the musb is not active. Since the IDLE REQ/ACK protocol is broken, the recommendation from ip team is to configure the musb in force idle/standby during omap retention and offmode. Since we have to touch the sysconfig registers and context save/restore everytime, I am using the runtime pm apis. >> And also musb was blocking the core-off after loading the gadget >> driver even with no cable connected sometimes. > >this too > >> Added the idle and wakeup APIs in the platform layer which will >> be called in the idle and wakeup path. > >And this errata fix should be a separate patch, with reference to the >errata etc. > This is not an errata, this is requirement from the ip. it is mentioned in the functional spec that when device is not use put it in force idle/standby mode. >> Used the pm_runtime_put_sysc API to configure the >> musb to force idle/standby modes, saving the context and >disable the clk in >> while idling if there is no activity on the usb bus. > >Why? This should not be part of the idle path. > >If there is no activity on the bus, why hasn't the musb driver already >runtime suspended itself? > >If the driver want's to runtime_suspend itself based on inactivity, it >should use an inactivity timer, not hook into the idle loop. The >runtime PM API has a function for a deferred suspend > >int pm_schedule_suspend(struct device *dev, unsigned int delay) > >The only thing in the idle loop in the current code is the errata fix, >and even that I'm not sure I'm ready to merge. > >I'd really like to see even the errata fix out of the idle >path. I wonder if usb-musb.c could create a periodic, >deferrable timer >to fire that ensure that autoidle is disabled. > >> Used the pm_runtime_get_sync API to configure the musb to >> no idle/standby modes, enable the clock and restore the context >> after wakeup when there is no activity on the usb bus. > >Why would you wakeup MUSB in the idle path if there is no activity on >the bus? > This is because MUSB has to be in known good state to capture any musb interrupts. >I don't understand the motiviation or these last two changes, and they >are a departure from the behavior of
RE: [PATCH 8/9 v3] usb : musb: Using runtime pm apis for musb.
Hi, >-Original Message- >From: Balbi, Felipe >Sent: Friday, September 24, 2010 4:35 PM >To: Kalliguddi, Hema >Cc: Balbi, Felipe; Kevin Hilman; linux-omap@vger.kernel.org; >linux-...@vger.kernel.org; Basak, Partha; Tony Lindgren; >Cousson, Benoit; Paul Walmsley >Subject: Re: [PATCH 8/9 v3] usb : musb: Using runtime pm apis for musb. > >Hi, > >On Fri, Sep 24, 2010 at 06:00:26AM -0500, Kalliguddi, Hema wrote: >>then for now I can think of using set_clok platform function to call >>the runtime pm apis, Is there any alternative you are thinking of? > >how about not pushing a patch which isn't the best way to solve the >problem in the first place ? .38 isn't that far away. If we can get all >those cleanups by that time, converting to pm_runtime will be a lot >simpler and local change. > >I mean, if we keep as is for another major release cycle, it >won't cause >problems to anyone, right ? So it's better to get things right from the >beginning and avoid lots of re-work later. > Agreed that it is better to get the things done in the best way from the beginning. This may not be the best solution for converting usb to use runtime pm apis, but given the current mainline code, without the cleanup, do not have other alternatives. Do you think the other patches are in good shape to merge after fixing the comments I got from Kevin and Benoit and you? Kevin, Do you have any other comments on the hwmod patches other than runtime pm patch and offmode patch? Do you accept the hwmod patches without the runtime pm conversion? >-- >balbi >-- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 8/9 v3] usb : musb: Using runtime pm apis for musb.
Hi, >-Original Message- >From: Balbi, Felipe >Sent: Friday, September 24, 2010 4:27 PM >To: Kalliguddi, Hema >Cc: Balbi, Felipe; Kevin Hilman; linux-omap@vger.kernel.org; >linux-...@vger.kernel.org; Basak, Partha; Tony Lindgren; >Cousson, Benoit; Paul Walmsley >Subject: Re: [PATCH 8/9 v3] usb : musb: Using runtime pm apis for musb. > >Hi, > >On Fri, Sep 24, 2010 at 04:28:35AM -0500, Kalliguddi, Hema wrote: >>I just noticed one more think in the code. there are suspend/resume >>function pointers in platform_driver structure. Why can't we >use these >>for platform specific operations like enable/disable clocks, context >>save/restore? > >For doing that properly, we first need to split the HW glue layer to >another parent platform_device/driver. Otherwise, we will have >all sorts >of crazy stuff to change the function pointer depending on the ARCH. > Agreed, then for now I can think of using set_clok platform function to call the runtime pm apis, Is there any alternative you are thinking of? >-- >balbi >-- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] OMAP: hwmod: Set autoidle after smartidle during _sysc_enable
Paul,
>-Original Message-
>From: Kalliguddi, Hema
>Sent: Friday, September 24, 2010 9:17 AM
>To: linux-omap@vger.kernel.org
>Cc: Kalliguddi, Hema; Basak, Partha; Balbi, Felipe; Tony
>Lindgren; Kevin Hilman; Cousson, Benoit; Paul Walmsley
>Subject: [PATCH] OMAP: hwmod: Set autoidle after smartidle
>during _sysc_enable
>
>OMAP USBOTG module has a requirement to set the autoidle bit only after
>setting smartidle bit. Modified the _sys_enable api to set the
>smartidle
>first and then the autoidle bit. Setting this will not have
>any impact on the
>other modules.
>
Review comments from Benoit and Sergie are fixed in this patch.
Benoit acked the older version of the patch and you had asked me re-post with
Review comments incorporated.
http://www.mail-archive.com/linux-omap@vger.kernel.org/msg35464.html
>Signed-off-by: Hema HK
>Signed-off-by: Basak, Partha
>Cc: Felipe Balbi
>Cc: Tony Lindgren
>Cc: Kevin Hilman
>Cc: Cousson, Benoit
>Cc: Paul Walmsley
>---
> arch/arm/mach-omap2/omap_hwmod.c | 18 --
> 1 file changed, 12 insertions(+), 6 deletions(-)
>
>Index: linux-omap-pm/arch/arm/mach-omap2/omap_hwmod.c
>===
>--- linux-omap-pm.orig/arch/arm/mach-omap2/omap_hwmod.c
>+++ linux-omap-pm/arch/arm/mach-omap2/omap_hwmod.c
>@@ -654,12 +654,6 @@ static void _sysc_enable(struct omap_hwm
> _set_master_standbymode(oh, idlemode, &v);
> }
>
>- if (sf & SYSC_HAS_AUTOIDLE) {
>- idlemode = (oh->flags & HWMOD_NO_OCP_AUTOIDLE) ?
>- 0 : 1;
>- _set_module_autoidle(oh, idlemode, &v);
>- }
>-
> /* XXX OCP ENAWAKEUP bit? */
>
> /*
>@@ -672,6 +666,18 @@ static void _sysc_enable(struct omap_hwm
> _set_clockactivity(oh, oh->class->sysc->clockact, &v);
>
> _write_sysconfig(v, oh);
>+
>+ /*
>+ * Set the autoidle bit only after setting the smartidle bit
>+ * as this is requirement for some modules like USBOTG.
>+ * Setting this will not have any impact on the other modules.
>+ */
>+ if (sf & SYSC_HAS_AUTOIDLE) {
>+ idlemode = (oh->flags & HWMOD_NO_OCP_AUTOIDLE) ?
>+ 0 : 1;
>+ _set_module_autoidle(oh, idlemode, &v);
>+ _write_sysconfig(v, oh);
>+ }
> }
>
> /**
>--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 8/9 v3] usb : musb: Using runtime pm apis for musb.
>-Original Message- >From: linux-usb-ow...@vger.kernel.org >[mailto:linux-usb-ow...@vger.kernel.org] On Behalf Of Kalliguddi, Hema >Sent: Friday, September 24, 2010 2:23 PM >To: Balbi, Felipe >Cc: Kevin Hilman; linux-omap@vger.kernel.org; >linux-...@vger.kernel.org; Basak, Partha; Tony Lindgren; >Cousson, Benoit; Paul Walmsley >Subject: RE: [PATCH 8/9 v3] usb : musb: Using runtime pm apis for musb. > >Felipe, > > >>-Original Message- >>From: Balbi, Felipe >>Sent: Friday, September 24, 2010 2:14 PM >>To: Kalliguddi, Hema >>Cc: Kevin Hilman; Balbi, Felipe; linux-omap@vger.kernel.org; >>linux-...@vger.kernel.org; Basak, Partha; Tony Lindgren; >>Cousson, Benoit; Paul Walmsley >>Subject: Re: [PATCH 8/9 v3] usb : musb: Using runtime pm apis >for musb. >> >>Hi, >> >>On Fri, Sep 24, 2010 at 03:16:29AM -0500, Kalliguddi, Hema wrote: >>>>I guess that's Felipe's call, but I don't like that option. >>>> >>>>I think it's cleaner to have the ->set_clock hook be a noop on OMAP >>>>and the runtime hooks be noops on the other platforms. >>>> >>>I think the set_clock function was used for setting the platform >>>specific clocks for different architecture. Anyway we need >>this hook or >>>some other hook once we start handling the optional clocks >>and internal >>>phy clocks for musb. Since this is already exists I thought >of making >>>use of it instand of adding another hook. >>> >>>Felipe, What do you say? >> >>Ideally we will get rid of those. It's only a matter of >getting DaVinci >>to fully support clkdev. We should not have wrappers on top of clk >>framework neither pass down clock names as platform_data to driver, >>that's insane. >> >I think we will need to have some hooks for handling the clocks for >different platform as each platforms will have different clocks. >The way we are using set_clock is not the wrapper on top clock >frameowrk, >it is a platform hook to enabling the clocks which still uses >the clock framework. > >~Hema > I just noticed one more think in the code. there are suspend/resume function pointers in platform_driver structure. Why can't we use these for platform specific operations like enable/disable clocks, context save/restore? >>-- >>balbi >>-- >To unsubscribe from this list: send the line "unsubscribe linux-usb" in >the body of a message to majord...@vger.kernel.org >More majordomo info at http://vger.kernel.org/majordomo-info.html >-- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 8/9 v3] usb : musb: Using runtime pm apis for musb.
Felipe, >-Original Message- >From: Balbi, Felipe >Sent: Friday, September 24, 2010 2:14 PM >To: Kalliguddi, Hema >Cc: Kevin Hilman; Balbi, Felipe; linux-omap@vger.kernel.org; >linux-...@vger.kernel.org; Basak, Partha; Tony Lindgren; >Cousson, Benoit; Paul Walmsley >Subject: Re: [PATCH 8/9 v3] usb : musb: Using runtime pm apis for musb. > >Hi, > >On Fri, Sep 24, 2010 at 03:16:29AM -0500, Kalliguddi, Hema wrote: >>>I guess that's Felipe's call, but I don't like that option. >>> >>>I think it's cleaner to have the ->set_clock hook be a noop on OMAP >>>and the runtime hooks be noops on the other platforms. >>> >>I think the set_clock function was used for setting the platform >>specific clocks for different architecture. Anyway we need >this hook or >>some other hook once we start handling the optional clocks >and internal >>phy clocks for musb. Since this is already exists I thought of making >>use of it instand of adding another hook. >> >>Felipe, What do you say? > >Ideally we will get rid of those. It's only a matter of getting DaVinci >to fully support clkdev. We should not have wrappers on top of clk >framework neither pass down clock names as platform_data to driver, >that's insane. > I think we will need to have some hooks for handling the clocks for different platform as each platforms will have different clocks. The way we are using set_clock is not the wrapper on top clock frameowrk, it is a platform hook to enabling the clocks which still uses the clock framework. ~Hema >-- >balbi >-- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 8/9 v3] usb : musb: Using runtime pm apis for musb.
>-Original Message- >From: Kevin Hilman [mailto:khil...@deeprootsystems.com] >Sent: Thursday, September 23, 2010 11:04 PM >To: Kalliguddi, Hema >Cc: Balbi, Felipe; linux-omap@vger.kernel.org; >linux-...@vger.kernel.org; Basak, Partha; Tony Lindgren; >Cousson, Benoit; Paul Walmsley >Subject: Re: [PATCH 8/9 v3] usb : musb: Using runtime pm apis for musb. > >"Kalliguddi, Hema" writes: > >> >> >>>-Original Message- >>>From: Kevin Hilman [mailto:khil...@deeprootsystems.com] >>>Sent: Thursday, September 23, 2010 9:00 PM >>>To: Balbi, Felipe >>>Cc: Kalliguddi, Hema; linux-omap@vger.kernel.org; >>>linux-...@vger.kernel.org; Basak, Partha; Tony Lindgren; >>>Cousson, Benoit; Paul Walmsley >>>Subject: Re: [PATCH 8/9 v3] usb : musb: Using runtime pm >apis for musb. >>> >>>Felipe Balbi writes: >>> >>>> Hi, >>>> >>>> On Wed, Sep 22, 2010 at 07:30:30PM -0500, Kalliguddi, Hema wrote: >>>>>Calling runtime pm APIs pm_runtime_put_sync() and >>>pm_runtime_get_sync() >>>>>for enabling/disabling the clocks,sysconfig settings. >>>>> >>>>>Also need to put the USB in force standby and force idle >>>mode when usb not used >>>>>and set it back to no idle and no stndby after wakeup. >>>>>For OMAP3 auto idle bit has to be disabled because of the >>>errata.So using >>>>>HWMOD_NO_OCP_AUTOIDLE flag for OMAP3430. >>> >>>[...] >>> >>>>>@@ -2424,13 +2425,16 @@ static int musb_suspend(struct device *d >>>>>* they will even be wakeup-enabled. >>>>>*/ >>>>> } >>>>>+ pm_runtime_put_sync(dev); >>>>> >>>>>+#ifndef CONFIG_PM_RUNTIME >>>>> musb_save_context(musb); >>>>> >>>>> if (musb->set_clock) >>>>> musb->set_clock(musb->clock, 0); >>>>> else >>>>> clk_disable(musb->clock); >>>>>+#endif >>>> >>>> I would rather remove these, adding ifdefs is bad :-( Unless >>>other archs >>>> (blackfin, davinci) would have problems if we remove those. >>> >>>I didn't like these #ifdefs either, but davinci doesn't have >>>runtime PM, >>>and I don't think blackfin does either. >>> >>>But, rather than the ifdef here, this could be done with different >>>pointers in struct dev_pm_ops based on the arch. >>> >>>Also, this shouldn't be based on CONFIG_PM_RUNTIME, but rather on the >>>arch. We can still enable runtime PM on davinci for other subsystems >>>(PCI, USB core, etc.) but not have it supported for on-chip devices. >>> >> Is it a good idea to use the musb->set_clock function >pointer for OMAP architure and >> call the runtime pm apis from this function defined in the >usb platform driver(omap2430) > >I guess that's Felipe's call, but I don't like that option. > >I think it's cleaner to have the ->set_clock hook be a noop on OMAP and >the runtime hooks be noops on the other platforms. > I think the set_clock function was used for setting the platform specific clocks for different architecture. Anyway we need this hook or some other hook once we start handling the optional clocks and internal phy clocks for musb. Since this is already exists I thought of making use of it instand of adding another hook. Felipe, What do you say? ~Hema >Kevin > >-- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 8/9 v3] usb : musb: Using runtime pm apis for musb.
Hi,
>-Original Message-
>From: linux-usb-ow...@vger.kernel.org
>[mailto:linux-usb-ow...@vger.kernel.org] On Behalf Of Kalliguddi, Hema
>Sent: Thursday, September 23, 2010 9:10 PM
>To: Kevin Hilman; Balbi, Felipe
>Cc: linux-omap@vger.kernel.org; linux-...@vger.kernel.org;
>Basak, Partha; Tony Lindgren; Cousson, Benoit; Paul Walmsley
>Subject: RE: [PATCH 8/9 v3] usb : musb: Using runtime pm apis for musb.
>
>
>
>>-Original Message-
>>From: Kevin Hilman [mailto:khil...@deeprootsystems.com]
>>Sent: Thursday, September 23, 2010 9:00 PM
>>To: Balbi, Felipe
>>Cc: Kalliguddi, Hema; linux-omap@vger.kernel.org;
>>linux-...@vger.kernel.org; Basak, Partha; Tony Lindgren;
>>Cousson, Benoit; Paul Walmsley
>>Subject: Re: [PATCH 8/9 v3] usb : musb: Using runtime pm apis
>for musb.
>>
>>Felipe Balbi writes:
>>
>>> Hi,
>>>
>>> On Wed, Sep 22, 2010 at 07:30:30PM -0500, Kalliguddi, Hema wrote:
>>>>Calling runtime pm APIs pm_runtime_put_sync() and
>>pm_runtime_get_sync()
>>>>for enabling/disabling the clocks,sysconfig settings.
>>>>
>>>>Also need to put the USB in force standby and force idle
>>mode when usb not used
>>>>and set it back to no idle and no stndby after wakeup.
>>>>For OMAP3 auto idle bit has to be disabled because of the
>>errata.So using
>>>>HWMOD_NO_OCP_AUTOIDLE flag for OMAP3430.
>>
>>[...]
>>
>>>>@@ -2424,13 +2425,16 @@ static int musb_suspend(struct device *d
>>>> * they will even be wakeup-enabled.
>>>> */
>>>>}
>>>>+ pm_runtime_put_sync(dev);
>>>>
>>>>+#ifndef CONFIG_PM_RUNTIME
>>>>musb_save_context(musb);
>>>>
>>>>if (musb->set_clock)
>>>>musb->set_clock(musb->clock, 0);
>>>>else
>>>>clk_disable(musb->clock);
>>>>+#endif
>>>
>>> I would rather remove these, adding ifdefs is bad :-( Unless
>>other archs
>>> (blackfin, davinci) would have problems if we remove those.
>>
>>I didn't like these #ifdefs either, but davinci doesn't have
>>runtime PM,
>>and I don't think blackfin does either.
>>
>>But, rather than the ifdef here, this could be done with different
>>pointers in struct dev_pm_ops based on the arch.
>>
>>Also, this shouldn't be based on CONFIG_PM_RUNTIME, but rather on the
>>arch. We can still enable runtime PM on davinci for other subsystems
>>(PCI, USB core, etc.) but not have it supported for on-chip devices.
>>
>Is it a good idea to use the musb->set_clock function pointer
>for OMAP architure and
>call the runtime pm apis from this function defined in the usb
>platform driver(omap2430)
>
>>Kevin
Here is the patch which is making use of already existing platform set_clock
functions pointer.
With this we don't need to use #ifdefs.
If it looks good I will post it again along with series.
arch/arm/mach-omap2/usb-musb.c | 18 +
drivers/usb/musb/musb_core.c | 12 +++
drivers/usb/musb/omap2430.c| 43 ++---
3 files changed, 45 insertions(+), 28 deletions(-)
Index: linux-omap-pm/arch/arm/mach-omap2/usb-musb.c
===
--- linux-omap-pm.orig/arch/arm/mach-omap2/usb-musb.c
+++ linux-omap-pm/arch/arm/mach-omap2/usb-musb.c
@@ -25,6 +25,7 @@
#include
#include
+#include
#include
@@ -46,6 +47,7 @@ static struct platform_device dummy_pdev
static void __iomem *otg_base;
static struct clk *otg_clk;
+static struct omap_hwmod *oh_p;
static void __init usb_musb_pm_init(void)
{
@@ -129,6 +131,20 @@ static struct omap_device_pm_latency oma
},
};
+void omap_set_clock(struct clk *clock , int is_on)
+{
+
+ struct omap_hwmod *oh = oh_p;
+ struct omap_device *od = oh->od;
+ struct platform_device *pdev = &od->pdev;
+ struct device *dev = &pdev->dev;
+
+ if(is_on)
+ pm_runtime_get_sync(dev);
+ else
+ pm_runtime_put_sync(dev);
+}
+
void __init usb_musb_init(struct omap_musb_board_data *board_data)
{
struct omap_hwmod *oh;
@@ -154,8 +170,10 @@ void __init usb_musb_init(struct omap_mu
musb_plat.power = board_data->power >> 1;
musb_plat.mode = board_data->mode;
musb_plat.extvbus = board_data->extvbus;
+ musb_plat.set_clock = omap_set_clock;
pdata = &musb_plat;
+ oh_p = oh;
od = omap_device_build(name, bus_id, oh, pdata,
RE: [PATCH 8/9 v3] usb : musb: Using runtime pm apis for musb.
>-Original Message- >From: Kevin Hilman [mailto:khil...@deeprootsystems.com] >Sent: Thursday, September 23, 2010 9:00 PM >To: Balbi, Felipe >Cc: Kalliguddi, Hema; linux-omap@vger.kernel.org; >linux-...@vger.kernel.org; Basak, Partha; Tony Lindgren; >Cousson, Benoit; Paul Walmsley >Subject: Re: [PATCH 8/9 v3] usb : musb: Using runtime pm apis for musb. > >Felipe Balbi writes: > >> Hi, >> >> On Wed, Sep 22, 2010 at 07:30:30PM -0500, Kalliguddi, Hema wrote: >>>Calling runtime pm APIs pm_runtime_put_sync() and >pm_runtime_get_sync() >>>for enabling/disabling the clocks,sysconfig settings. >>> >>>Also need to put the USB in force standby and force idle >mode when usb not used >>>and set it back to no idle and no stndby after wakeup. >>>For OMAP3 auto idle bit has to be disabled because of the >errata.So using >>>HWMOD_NO_OCP_AUTOIDLE flag for OMAP3430. > >[...] > >>>@@ -2424,13 +2425,16 @@ static int musb_suspend(struct device *d >>> * they will even be wakeup-enabled. >>> */ >>> } >>>+pm_runtime_put_sync(dev); >>> >>>+#ifndef CONFIG_PM_RUNTIME >>> musb_save_context(musb); >>> >>> if (musb->set_clock) >>> musb->set_clock(musb->clock, 0); >>> else >>> clk_disable(musb->clock); >>>+#endif >> >> I would rather remove these, adding ifdefs is bad :-( Unless >other archs >> (blackfin, davinci) would have problems if we remove those. > >I didn't like these #ifdefs either, but davinci doesn't have >runtime PM, >and I don't think blackfin does either. > >But, rather than the ifdef here, this could be done with different >pointers in struct dev_pm_ops based on the arch. > >Also, this shouldn't be based on CONFIG_PM_RUNTIME, but rather on the >arch. We can still enable runtime PM on davinci for other subsystems >(PCI, USB core, etc.) but not have it supported for on-chip devices. > Is it a good idea to use the musb->set_clock function pointer for OMAP architure and call the runtime pm apis from this function defined in the usb platform driver(omap2430) >Kevin >-- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 9/9 v3] usb : musb: Offmode fix for idle path
Kevin,
>-Original Message-
>From: linux-usb-ow...@vger.kernel.org
>[mailto:linux-usb-ow...@vger.kernel.org] On Behalf Of Kalliguddi, Hema
>Sent: Thursday, September 23, 2010 1:29 PM
>To: Balbi, Felipe
>Cc: linux-omap@vger.kernel.org; linux-...@vger.kernel.org;
>Mankad, Maulik Ojas; Tony Lindgren; Kevin Hilman; Cousson,
>Benoit; Paul Walmsley
>Subject: RE: [PATCH 9/9 v3] usb : musb: Offmode fix for idle path
>
> Hi,
>
>>-Original Message-
>>From: Balbi, Felipe
>>Sent: Thursday, September 23, 2010 12:19 PM
>>To: Kalliguddi, Hema
>>Cc: linux-omap@vger.kernel.org; linux-...@vger.kernel.org;
>>Mankad, Maulik Ojas; Balbi, Felipe; Tony Lindgren; Kevin
>>Hilman; Cousson, Benoit; Paul Walmsley
>>Subject: Re: [PATCH 9/9 v3] usb : musb: Offmode fix for idle path
>>
>>Hi,
>>
>>On Wed, Sep 22, 2010 at 07:30:46PM -0500, Kalliguddi, Hema wrote:
>>>With OMAP core-off support musb was not functional as context
>>was getting
>>>lost after wakeup from core-off. And also musb was blocking
>>the core-off
>>>after loading the gadget driver even with no cable connected
>>sometimes.
>>>
>>>Added the idle and wakeup APIs in the platform layer which will
>>>be called in the idle and wakeup path.
>>>
>>>Used the pm_runtime_put_sysc API to configure the
>>
>>pm_runtime_put_sync(), typo.
>>
>>>musb to force idle/standby modes, saving the context and
>>disable the clk in
>>>while idling if there is no activity on the usb bus.
>>
>>this part is a bit fuzzy, care to re-phrase ?
>>
>Ok. I will re-phrase it.
>
>>>Used the pm_runtime_get_sync API to configure the musb to
>>>no idle/standby modes, enable the clock and restore the context
>>>after wakeup when there is no activity on the usb bus.
>>>
>>>Signed-off-by: Hema HK
>>>Signed-off-by: Maulik Mankad
>>>Cc: Felipe Balbi
>>>Cc: Tony Lindgren
>>>Cc: Kevin Hilman
>>>Cc: Cousson, Benoit
>>>Cc: Paul Walmsley
>>>
>>>---
>>> arch/arm/mach-omap2/cpuidle34xx.c |1
>>> arch/arm/mach-omap2/pm34xx.c |3
>>> arch/arm/mach-omap2/usb-musb.c| 107
>>++
>>> arch/arm/plat-omap/include/plat/usb.h |2
>>> drivers/usb/musb/musb_core.c | 15
>>> drivers/usb/musb/omap2430.c | 14
>>> include/linux/usb/musb.h |9 ++
>>> 7 files changed, 149 insertions(+), 2 deletions(-)
>>>
>>>Index: linux-omap-pm/arch/arm/mach-omap2/cpuidle34xx.c
>>>===
>>>--- linux-omap-pm.orig/arch/arm/mach-omap2/cpuidle34xx.c
>>>+++ linux-omap-pm/arch/arm/mach-omap2/cpuidle34xx.c
>>>@@ -31,6 +31,7 @@
>>> #include
>>> #include
>>> #include
>>>+#include
>>>
>>> #include "pm.h"
>>>
>>>Index: linux-omap-pm/arch/arm/mach-omap2/pm34xx.c
>>>===
>>>--- linux-omap-pm.orig/arch/arm/mach-omap2/pm34xx.c
>>>+++ linux-omap-pm/arch/arm/mach-omap2/pm34xx.c
>>>@@ -38,6 +38,7 @@
>>> #include
>>> #include
>>> #include
>>>+#include
>>>
>>> #include
>>>
>>>@@ -324,11 +325,13 @@ static void restore_table_entry(void)
>>> void omap3_device_idle(void)
>>> {
>>> omap2_gpio_prepare_for_idle();
>>>+musb_prepare_for_idle();
>>> }
>>>
>>> void omap3_device_resume(void)
>>> {
>>> omap2_gpio_resume_after_idle();
>>>+musb_wakeup_from_idle();
>>> }
>>>
>>> void omap_sram_idle(void)
>>>Index: linux-omap-pm/arch/arm/mach-omap2/usb-musb.c
>>>===
>>>--- linux-omap-pm.orig/arch/arm/mach-omap2/usb-musb.c
>>>+++ linux-omap-pm/arch/arm/mach-omap2/usb-musb.c
>>>@@ -25,16 +25,21 @@
>>> #include
>>>
>>> #include
>>>+#include
>>>
>>> #include
>>> #include
>>> #include
>>> #include
>>>+#include
>>>
>>> #ifdef CONFIG_USB_MUSB_SOC
>>> static const char name[] = "musb_hdrc";
>>> #define MAX_OMAP_MUSB_HWMOD_NAME_LEN16
>>>
>>>+struct omap_hwmod *oh_p;
>>>+static struct powerdomain *core_pwrdm;
>>&g
RE: [PATCH 8/9 v3] usb : musb: Using runtime pm apis for musb.
Hi,
>-Original Message-
>From: Balbi, Felipe
>Sent: Thursday, September 23, 2010 2:22 PM
>To: Kalliguddi, Hema
>Cc: Balbi, Felipe; linux-omap@vger.kernel.org;
>linux-...@vger.kernel.org; Basak, Partha; Tony Lindgren; Kevin
>Hilman; Cousson, Benoit; Paul Walmsley
>Subject: Re: [PATCH 8/9 v3] usb : musb: Using runtime pm apis for musb.
>
>Hi,
>
>On Thu, Sep 23, 2010 at 02:51:12AM -0500, Kalliguddi, Hema wrote:
>>>these two should a separate series, otherwise it's difficult for
>>>different maintainers to decide what they need to pick up :-)
>>>
>>I don't mind
>
>if you're re-sending already, please do split :-)
>
Ok. I will do that.
>>>I would rather remove these, adding ifdefs is bad :-( Unless
>>>other archs
>>>(blackfin, davinci) would have problems if we remove those.
>>>
>>If we remove this then how the clock will be enabled for the other
>>platforms where runtime pm is not used?
>
>yeah, let's see what other archs will say...
>
>>>>@@ -2457,9 +2465,26 @@ static int musb_resume_noirq(struct devi
>>>>return 0;
>>>> }
>>>>
>>>>+static int musb_runtime_suspend(struct device *dev)
>>>>+{
>>>>+ struct musb *musb = dev_to_musb(dev);
>>>
>>>missing lock ??
>>I am wondering whether we need the lock here? As these
>functions are supposed to be
>>Called by runtime pm framework only.
>
>is that done with IRQs disabled ?
At high level it was looking like the irqs are disabled by looking below
function.
int pm_runtime_resume(struct device *dev)
{
int retval;
spin_lock_irq(&dev->power.lock);
retval = __pm_runtime_resume(dev, false);
spin_unlock_irq(&dev->power.lock);
return retval;
}
But after going through further details, before calling the driver's
pm->runtime_suspend/resume functions the irqs are enabled back.
So we need to have lock.
>
>>>>+ musb_save_context(musb);
>>>
>>>shouldn't you disable the clock here ?
>>>
>>This hook is called when we call pm_runtime_put_sync api which takes
>>care of disabling clocks and configuring the sysconfig register.
>
>but those are methods you have provided, right ? I mean, in the end
>it'll call musb_runtime_suspend() and musb_runtime_resume() ?? Or am I
>missing something ?
The flow when you call pm_runtime_put_sync is as below.
1. Calls the driver's runtime_suspend hook
2. Configures the sysconfig
3. Disable the clocks.
The driver's runtime_suspend/resume hooks are provided to prepare the
Device for the idle incase if there is something to be done. For
Example you can use it for storing the context and restoring the context.
Since framework is taking of diabling the clock when runtime pm funtions are
called so there is no need of disabling the clock here.
>
>>>should you pm_runtime_put_sync(dev) here ??
>>>
>>Calling pm_runtime_put_sync leading to crash as
>>driver_detach calls __device_release_driver which intern calls
>>pm_runtime_put_sync function, with this the musb clocks are
>already disabled
>>And usecount is 0.
>
>looks weird though, I'll have to read that code more carefully when you
>send another version.
>
OK.
>>>>- l = musb_readl(musb->mregs, OTG_SYSCONFIG);
>>>>- l &= ~ENABLEWAKEUP; /* disable wakeup */
>>>>- musb_writel(musb->mregs, OTG_SYSCONFIG, l);
>>>>+ pm_runtime_enable(dev);
>>>>+ pm_runtime_get_sync(dev);
>>>
>>>seems like you're going to call pm_runtime_get_sync twice ? once here
>>>and another time on musb_resume_noirq(). why ?
>>
>>pm_runtime_get_sync is called in the musb_platform_resume function
>>during the initialization of the driver. Where we need to
>enable the clocks
>>and put the sysconfig registers to known configurations.
>>
>>pm_runtime_get_sync is called in musb_resume_noirq() to
>re-enable the cloks
>>and configure the sysconfig because the clocks are disabled
>in musb_suspend()
>>by calling pm_runtime_put_sync() during global suspend/resume.
>
>hmm, also weird logic. I'll wait for next version and read that code
>with more care.
>
musb_platform_resume functions gets executed when the driver is loaded.
Clock hs to be enabled before accessing the registers so calling the
pm_runtime_get_sync api to do the same.
When there is global suspend initiated, driver's pm_ops functions
Musb_suspend api is called and the clocks are disabled.
Now when the system resumes after wakeup,
Musb_resume_noirq is called, now need to re-enable the clocks so calling
pm_runtime_get_sync function.
Do you see wny problem with it?
>--
>balbi
>--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 8/9 v3] usb : musb: Using runtime pm apis for musb.
>-Original Message-
>From: linux-usb-ow...@vger.kernel.org
>[mailto:linux-usb-ow...@vger.kernel.org] On Behalf Of Kalliguddi, Hema
>Sent: Thursday, September 23, 2010 1:21 PM
>To: Balbi, Felipe
>Cc: linux-omap@vger.kernel.org; linux-...@vger.kernel.org;
>Basak, Partha; Tony Lindgren; Kevin Hilman; Cousson, Benoit;
>Paul Walmsley
>Subject: RE: [PATCH 8/9 v3] usb : musb: Using runtime pm apis for musb.
>
>
>
>>-Original Message-
>>From: Balbi, Felipe
>>Sent: Thursday, September 23, 2010 12:06 PM
>>To: Kalliguddi, Hema
>>Cc: linux-omap@vger.kernel.org; linux-...@vger.kernel.org;
>>Basak, Partha; Balbi, Felipe; Tony Lindgren; Kevin Hilman;
>>Cousson, Benoit; Paul Walmsley
>>Subject: Re: [PATCH 8/9 v3] usb : musb: Using runtime pm apis
>for musb.
>>
>>Hi,
>>
>>On Wed, Sep 22, 2010 at 07:30:30PM -0500, Kalliguddi, Hema wrote:
>>>Calling runtime pm APIs pm_runtime_put_sync() and
>>pm_runtime_get_sync()
>>>for enabling/disabling the clocks,sysconfig settings.
>>>
>>>Also need to put the USB in force standby and force idle mode
>>when usb not used
>>>and set it back to no idle and no stndby after wakeup.
>>>For OMAP3 auto idle bit has to be disabled because of the
>>errata.So using
>>>HWMOD_NO_OCP_AUTOIDLE flag for OMAP3430.
>>>
>>>Signed-off-by: Hema HK
>>>Signed-off-by: Basak, Partha
>>>Cc: Felipe Balbi
>>>Cc: Tony Lindgren
>>>Cc: Kevin Hilman
>>>Cc: Cousson, Benoit
>>>Cc: Paul Walmsley
>>
>>these two should a separate series, otherwise it's difficult for
>>different maintainers to decide what they need to pick up :-)
>>
>I don't mind
I mean I don't mind posting them as separate series. But this will have still
the driver
And architecture files changes.
>>Anyways, let me continue;
>>
>>>@@ -2424,13 +2425,16 @@ static int musb_suspend(struct device *d
>>> * they will even be wakeup-enabled.
>>> */
>>> }
>>>+pm_runtime_put_sync(dev);
>>>
>>>+#ifndef CONFIG_PM_RUNTIME
>>> musb_save_context(musb);
>>>
>>> if (musb->set_clock)
>>> musb->set_clock(musb->clock, 0);
>>> else
>>> clk_disable(musb->clock);
>>>+#endif
>>
>>I would rather remove these, adding ifdefs is bad :-( Unless
>>other archs
>>(blackfin, davinci) would have problems if we remove those.
>>
>If we remove this then how the clock will be enabled for the
>other platforms where runtime
>pm is not used?
>
>>>@@ -2457,9 +2465,26 @@ static int musb_resume_noirq(struct devi
>>> return 0;
>>> }
>>>
>>>+static int musb_runtime_suspend(struct device *dev)
>>>+{
>>>+struct musb *musb = dev_to_musb(dev);
>>
>>missing lock ??
>I am wondering whether we need the lock here? As these
>functions are supposed to be
>Called by runtime pm framework only.
>>
>>>+musb_save_context(musb);
>>
>>shouldn't you disable the clock here ?
>>
>This hook is called when we call pm_runtime_put_sync api which
>takes care of disabling
>clocks and configuring the sysconfig register.
>
>>>+return 0;
>>>+}
>>>+
>>>+static int musb_runtime_resume(struct device *dev)
>>>+{
>>>+struct musb *musb = dev_to_musb(dev);
>>
>>re-enable clock here and missing lock ??
>>
>
>Not required. This hook gets aclled when pm_runtime_get_sync
>is called by the driver
>Which will take care of enabling the clock.
>
>I am just wondering whether we need the lock here? As these
>functions are supposed to be
>Called by runtime pm framework only.
>
>>>@@ -253,15 +240,13 @@ int __init musb_platform_init(struct mus
>>> void musb_platform_save_context(struct musb *musb,
>>> struct musb_context_registers *musb_context)
>>> {
>>>-musb_context->otg_sysconfig = musb_readl(musb->mregs,
>>OTG_SYSCONFIG);
>>>-musb_context->otg_forcestandby =
>>musb_readl(musb->mregs, OTG_FORCESTDBY);
>>>+musb_writel(musb->mregs, OTG_FORCESTDBY, ENABLEFORCE);
>>
>>not really saving context anymore :-p but that's ok, we will
>>need change
>>all that mess anyway.
>>
>Yehh :-)
>
>>>@@ -277,37 +262,23 @@ static int musb_platform_suspend(struct
>>> l |= ENABLEFORCE; /* enable MSTANDBY */
>>>
RE: [PATCH 9/9 v3] usb : musb: Offmode fix for idle path
Hi,
>-Original Message-
>From: Balbi, Felipe
>Sent: Thursday, September 23, 2010 12:19 PM
>To: Kalliguddi, Hema
>Cc: linux-omap@vger.kernel.org; linux-...@vger.kernel.org;
>Mankad, Maulik Ojas; Balbi, Felipe; Tony Lindgren; Kevin
>Hilman; Cousson, Benoit; Paul Walmsley
>Subject: Re: [PATCH 9/9 v3] usb : musb: Offmode fix for idle path
>
>Hi,
>
>On Wed, Sep 22, 2010 at 07:30:46PM -0500, Kalliguddi, Hema wrote:
>>With OMAP core-off support musb was not functional as context
>was getting
>>lost after wakeup from core-off. And also musb was blocking
>the core-off
>>after loading the gadget driver even with no cable connected
>sometimes.
>>
>>Added the idle and wakeup APIs in the platform layer which will
>>be called in the idle and wakeup path.
>>
>>Used the pm_runtime_put_sysc API to configure the
>
>pm_runtime_put_sync(), typo.
>
>>musb to force idle/standby modes, saving the context and
>disable the clk in
>>while idling if there is no activity on the usb bus.
>
>this part is a bit fuzzy, care to re-phrase ?
>
Ok. I will re-phrase it.
>>Used the pm_runtime_get_sync API to configure the musb to
>>no idle/standby modes, enable the clock and restore the context
>>after wakeup when there is no activity on the usb bus.
>>
>>Signed-off-by: Hema HK
>>Signed-off-by: Maulik Mankad
>>Cc: Felipe Balbi
>>Cc: Tony Lindgren
>>Cc: Kevin Hilman
>>Cc: Cousson, Benoit
>>Cc: Paul Walmsley
>>
>>---
>> arch/arm/mach-omap2/cpuidle34xx.c |1
>> arch/arm/mach-omap2/pm34xx.c |3
>> arch/arm/mach-omap2/usb-musb.c| 107
>++
>> arch/arm/plat-omap/include/plat/usb.h |2
>> drivers/usb/musb/musb_core.c | 15
>> drivers/usb/musb/omap2430.c | 14
>> include/linux/usb/musb.h |9 ++
>> 7 files changed, 149 insertions(+), 2 deletions(-)
>>
>>Index: linux-omap-pm/arch/arm/mach-omap2/cpuidle34xx.c
>>===
>>--- linux-omap-pm.orig/arch/arm/mach-omap2/cpuidle34xx.c
>>+++ linux-omap-pm/arch/arm/mach-omap2/cpuidle34xx.c
>>@@ -31,6 +31,7 @@
>> #include
>> #include
>> #include
>>+#include
>>
>> #include "pm.h"
>>
>>Index: linux-omap-pm/arch/arm/mach-omap2/pm34xx.c
>>===
>>--- linux-omap-pm.orig/arch/arm/mach-omap2/pm34xx.c
>>+++ linux-omap-pm/arch/arm/mach-omap2/pm34xx.c
>>@@ -38,6 +38,7 @@
>> #include
>> #include
>> #include
>>+#include
>>
>> #include
>>
>>@@ -324,11 +325,13 @@ static void restore_table_entry(void)
>> void omap3_device_idle(void)
>> {
>> omap2_gpio_prepare_for_idle();
>>+ musb_prepare_for_idle();
>> }
>>
>> void omap3_device_resume(void)
>> {
>> omap2_gpio_resume_after_idle();
>>+ musb_wakeup_from_idle();
>> }
>>
>> void omap_sram_idle(void)
>>Index: linux-omap-pm/arch/arm/mach-omap2/usb-musb.c
>>===
>>--- linux-omap-pm.orig/arch/arm/mach-omap2/usb-musb.c
>>+++ linux-omap-pm/arch/arm/mach-omap2/usb-musb.c
>>@@ -25,16 +25,21 @@
>> #include
>>
>> #include
>>+#include
>>
>> #include
>> #include
>> #include
>> #include
>>+#include
>>
>> #ifdef CONFIG_USB_MUSB_SOC
>> static const char name[] = "musb_hdrc";
>> #define MAX_OMAP_MUSB_HWMOD_NAME_LEN 16
>>
>>+struct omap_hwmod *oh_p;
>>+static struct powerdomain *core_pwrdm;
>>+
>> static struct musb_hdrc_config musb_config = {
>> .multipoint = 1,
>> .dyn_fifo = 1,
>>@@ -58,6 +63,10 @@ static struct musb_hdrc_platform_data mu
>> * "mode", and should be passed to usb_musb_init().
>> */
>> .power = 50, /* up to 100 mA */
>>+
>>+ /* OMAP supports offmode */
>>+ .save_context = 1,
>>+ .restore_context= 1,
>> };
>>
>> static u64 musb_dmamask = DMA_BIT_MASK(32);
>>@@ -80,6 +89,7 @@ void __init usb_musb_init(struct omap_mu
>> const char *oh_name = "usb_otg_hs";
>> struct musb_hdrc_platform_data *pdata;
>>
>>+ core_pwrdm = pwrdm_lookup("per_pwrdm");
>
>per or core ???
>
Oh! It should be core. Now I understand why save/restore counts were not
matching with
C
RE: [PATCH 8/9 v3] usb : musb: Using runtime pm apis for musb.
>-Original Message-
>From: Balbi, Felipe
>Sent: Thursday, September 23, 2010 12:06 PM
>To: Kalliguddi, Hema
>Cc: linux-omap@vger.kernel.org; linux-...@vger.kernel.org;
>Basak, Partha; Balbi, Felipe; Tony Lindgren; Kevin Hilman;
>Cousson, Benoit; Paul Walmsley
>Subject: Re: [PATCH 8/9 v3] usb : musb: Using runtime pm apis for musb.
>
>Hi,
>
>On Wed, Sep 22, 2010 at 07:30:30PM -0500, Kalliguddi, Hema wrote:
>>Calling runtime pm APIs pm_runtime_put_sync() and
>pm_runtime_get_sync()
>>for enabling/disabling the clocks,sysconfig settings.
>>
>>Also need to put the USB in force standby and force idle mode
>when usb not used
>>and set it back to no idle and no stndby after wakeup.
>>For OMAP3 auto idle bit has to be disabled because of the
>errata.So using
>>HWMOD_NO_OCP_AUTOIDLE flag for OMAP3430.
>>
>>Signed-off-by: Hema HK
>>Signed-off-by: Basak, Partha
>>Cc: Felipe Balbi
>>Cc: Tony Lindgren
>>Cc: Kevin Hilman
>>Cc: Cousson, Benoit
>>Cc: Paul Walmsley
>
>these two should a separate series, otherwise it's difficult for
>different maintainers to decide what they need to pick up :-)
>
I don't mind
>Anyways, let me continue;
>
>>@@ -2424,13 +2425,16 @@ static int musb_suspend(struct device *d
>> * they will even be wakeup-enabled.
>> */
>> }
>>+ pm_runtime_put_sync(dev);
>>
>>+#ifndef CONFIG_PM_RUNTIME
>> musb_save_context(musb);
>>
>> if (musb->set_clock)
>> musb->set_clock(musb->clock, 0);
>> else
>> clk_disable(musb->clock);
>>+#endif
>
>I would rather remove these, adding ifdefs is bad :-( Unless
>other archs
>(blackfin, davinci) would have problems if we remove those.
>
If we remove this then how the clock will be enabled for the other platforms
where runtime
pm is not used?
>>@@ -2457,9 +2465,26 @@ static int musb_resume_noirq(struct devi
>> return 0;
>> }
>>
>>+static int musb_runtime_suspend(struct device *dev)
>>+{
>>+ struct musb *musb = dev_to_musb(dev);
>
>missing lock ??
I am wondering whether we need the lock here? As these functions are supposed
to be
Called by runtime pm framework only.
>
>>+ musb_save_context(musb);
>
>shouldn't you disable the clock here ?
>
This hook is called when we call pm_runtime_put_sync api which takes care of
disabling
clocks and configuring the sysconfig register.
>>+ return 0;
>>+}
>>+
>>+static int musb_runtime_resume(struct device *dev)
>>+{
>>+ struct musb *musb = dev_to_musb(dev);
>
>re-enable clock here and missing lock ??
>
Not required. This hook gets aclled when pm_runtime_get_sync is called by the
driver
Which will take care of enabling the clock.
I am just wondering whether we need the lock here? As these functions are
supposed to be
Called by runtime pm framework only.
>>@@ -253,15 +240,13 @@ int __init musb_platform_init(struct mus
>> void musb_platform_save_context(struct musb *musb,
>> struct musb_context_registers *musb_context)
>> {
>>- musb_context->otg_sysconfig = musb_readl(musb->mregs,
>OTG_SYSCONFIG);
>>- musb_context->otg_forcestandby =
>musb_readl(musb->mregs, OTG_FORCESTDBY);
>>+ musb_writel(musb->mregs, OTG_FORCESTDBY, ENABLEFORCE);
>
>not really saving context anymore :-p but that's ok, we will
>need change
>all that mess anyway.
>
Yehh :-)
>>@@ -277,37 +262,23 @@ static int musb_platform_suspend(struct
>> l |= ENABLEFORCE; /* enable MSTANDBY */
>> musb_writel(musb->mregs, OTG_FORCESTDBY, l);
>>
>>- l = musb_readl(musb->mregs, OTG_SYSCONFIG);
>>- l |= ENABLEWAKEUP; /* enable wakeup */
>>- musb_writel(musb->mregs, OTG_SYSCONFIG, l);
>>-
>> otg_set_suspend(musb->xceiv, 1);
>>
>>- if (musb->set_clock)
>>- musb->set_clock(musb->clock, 0);
>>- else
>>- clk_disable(musb->clock);
>>-
>
>should you pm_runtime_put_sync(dev) here ??
>
Calling pm_runtime_put_sync leading to crash as
driver_detach calls __device_release_driver which intern calls
pm_runtime_put_sync function, with this the musb clocks are already disabled
And usecount is 0.
>> return 0;
>> }
>>
>> static int musb_platform_resume(struct musb *musb)
>> {
>> u32 l;
>>+ struct device *dev = musb->controller;
>>
>> if (!musb->clock)
>> return 0;
>
>you're not
RE: [PATCH 7/9 v3] OMAP: Hwmod api changes
Hi, >-Original Message- >From: Balbi, Felipe >Sent: Thursday, September 23, 2010 11:53 AM >To: Cousson, Benoit >Cc: Kalliguddi, Hema; linux-omap@vger.kernel.org; >linux-...@vger.kernel.org; Basak, Partha; Balbi, Felipe; Tony >Lindgren; Kevin Hilman; Paul Walmsley >Subject: Re: [PATCH 7/9 v3] OMAP: Hwmod api changes > >On Wed, Sep 22, 2010 at 03:43:14PM -0500, Cousson, Benoit wrote: >>Hi Hema, >> >>On 9/23/2010 2:30 AM, Kalliguddi, Hema wrote: >>> OMAP USBOTG modules has a requirement to set the auto idle >bit only after >>> setting smart idle bit. Modified the _sys_enable api to set >the smart idle >>> first and then the autoidle bit. Setting this will not have >any impact on the >>> other modules. >> >>I think you should change the subject, because it does not reflect >>accurately the patch. >> >>Just a little bit of nitpicking... >>For me, an API change is a change in the signature of the API, not a >>change inside an API. Otherwise, since 99% of the code is inside a >>function, we can call most patches like that... >>Moreover I don't think _sysc_enable can be considered as an >API since it >>is a pure static helper function not exported to the outside. >> >>Something like that seems more accurate for my point of view: >> >>OMAP: hwmod: Set autoidle after smartidle during _sysc_enable > >+1 > I Agree. I will do the changes accordingly and post the patch. >-- >balbi >-- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 1/9 v3] usb: musb: Adding names for IRQs in resource structure
Hi, >-Original Message- >From: Balbi, Felipe >Sent: Thursday, September 23, 2010 12:21 PM >To: Kalliguddi, Hema >Cc: Balbi, Felipe; linux-omap@vger.kernel.org; >linux-...@vger.kernel.org; Tony Lindgren; Kevin Hilman; >Cousson, Benoit; Paul Walmsley >Subject: Re: [PATCH 1/9 v3] usb: musb: Adding names for IRQs >in resource structure > >Hi, > >On Thu, Sep 23, 2010 at 01:24:19AM -0500, Kalliguddi, Hema wrote: >>I understand that. I forgot to Cc Mike for blackfin arch files change. >>I will post it again today Cc ing him and try my luck :-) > >let's hope for the best, but while at that, would you take care of the >comments on the other patches ? I am trying my level best to incorporate all the comments today and post them. > >-- >balbi >-- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 6/9 v3] usb: musb: HWMOD database structures addition for OMAP2430
Hi, >-Original Message- >From: Balbi, Felipe >Sent: Thursday, September 23, 2010 11:52 AM >To: Kalliguddi, Hema >Cc: linux-omap@vger.kernel.org; linux-...@vger.kernel.org; >Balbi, Felipe; Tony Lindgren; Kevin Hilman; Cousson, Benoit; >Paul Walmsley >Subject: Re: [PATCH 6/9 v3] usb: musb: HWMOD database >structures addition for OMAP2430 > >On Wed, Sep 22, 2010 at 07:29:55PM -0500, Kalliguddi, Hema wrote: >>OMAP2430 hwmod data stuctures are populated with base >address, L3 and L4 >>interface clocks, IRQs,and sysconfig register details. >> >>Signed-off-by: Hema HK >>Cc: Felipe Balbi >>Cc: Tony Lindgren >>Cc: Kevin Hilman >>Cc: Cousson, Benoit >>Cc: Paul Walmsley > >looks like this should come before patch 5. Yes. Otherwise the OMAP2430 hwmod_lookup wil fail. I will change the order. ~ Hema > >-- >balbi >-- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 5/9 v3] usb: musb: Using omap_device_build for musb device registration
Hi,
>-Original Message-
>From: Balbi, Felipe
>Sent: Thursday, September 23, 2010 11:51 AM
>To: Kalliguddi, Hema
>Cc: linux-omap@vger.kernel.org; linux-...@vger.kernel.org;
>Balbi, Felipe; Tony Lindgren; Kevin Hilman; Cousson, Benoit;
>Paul Walmsley
>Subject: Re: [PATCH 5/9 v3] usb: musb: Using omap_device_build
>for musb device registration
>
>Hi,
>
>On Wed, Sep 22, 2010 at 07:29:10PM -0500, Kalliguddi, Hema wrote:
>>+#define MAX_OMAP_MUSB_HWMOD_NAME_LEN 16
>
>this isn't used anywhere.
>
>>@@ -75,31 +62,30 @@ static struct musb_hdrc_platform_data mu
>>
>> static u64 musb_dmamask = DMA_BIT_MASK(32);
>>
>>-static struct platform_device musb_device = {
>>- .name = "musb_hdrc",
>>- .id = -1,
>>- .dev = {
>>- .dma_mask = &musb_dmamask,
>>- .coherent_dma_mask = DMA_BIT_MASK(32),
>>- .platform_data = &musb_plat,
>>+static struct omap_device_pm_latency omap_musb_latency[] = {
>>+ {
>>+ .deactivate_func = omap_device_idle_hwmods,
>>+ .activate_func = omap_device_enable_hwmods,
>>+ .flags = OMAP_DEVICE_LATENCY_AUTO_ADJUST,
>> },
>>- .num_resources = ARRAY_SIZE(musb_resources),
>>- .resource = musb_resources,
>> };
>>
>> void __init usb_musb_init(struct omap_musb_board_data *board_data)
>> {
>>- if (cpu_is_omap243x()) {
>>- musb_resources[0].start = OMAP243X_HS_BASE;
>>- } else if (cpu_is_omap34xx()) {
>>- musb_resources[0].start = OMAP34XX_HSUSB_OTG_BASE;
>>- } else if (cpu_is_omap44xx()) {
>>- musb_resources[0].start = OMAP44XX_HSUSB_OTG_BASE;
>>- musb_resources[1].start = OMAP44XX_IRQ_HS_USB_MC_N;
>>- musb_resources[2].start = OMAP44XX_IRQ_HS_USB_DMA_N;
>>+ struct omap_hwmod *oh;
>>+ struct omap_device *od;
>>+ struct platform_device *pdev;
>>+ struct device *dev;
>>+ int bus_id = -1;
>>+ const char *oh_name = "usb_otg_hs";
>>+ struct musb_hdrc_platform_data *pdata;
>>+
>>+ oh = omap_hwmod_lookup(oh_name);
>>+
>>+ if (!oh) {
>>+ pr_err("Could not look up %s\n", oh_name);
>>+ return;
>> }
>
>Paul, Kevin, to me it looks like a duplication that all devices will
>have to:
>
>oh = omap_hwmod_lookup("my_hwmod_name");
>omap_device_build("my_device_name", bus_id, oh, pdata, sizeof(*pdata));
>
>could the omap_hwmod_lookup() part be moved to omap_device_build ? Or
>maybe create a omap_hwmod_lookup_and_build(oh_name, dev_name, bus_id,
>pdata, sizeof(*pdata)) ??
>>@@ -110,8 +96,23 @@ void __init usb_musb_init(struct omap_mu
>> musb_plat.mode = board_data->mode;
>> musb_plat.extvbus = board_data->extvbus;
>>
>>- if (platform_device_register(&musb_device) < 0)
>>- printk(KERN_ERR "Unable to register HS-USB
>(MUSB) device\n");
>>+ pdata = &musb_plat;
>>+
>>+ od = omap_device_build(name, bus_id, oh, pdata,
>>+sizeof(struct musb_hdrc_platform_data),
>
>use sizeof(*pdata), if we change the name of that structure (very
>unlikely, but still) it'll avoid unwanted compile breakage.
>
Ok. Make sense.
>>+ pdev = &od->pdev;
>>+ dev = &pdev->dev;
>>+ get_device(dev);
>>+ dev->dma_mask = &musb_dmamask;
>>+ dev->coherent_dma_mask = musb_dmamask;
>>+ put_device(dev);
>
>I think this is also a duplication, it's gonna on all hwmod device
>registration, no ?
Any driver which uses the device->dma_mask will have to it after the device
build.
~Hema
>
>--
>balbi
>--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 1/9 v3] usb: musb: Adding names for IRQs in resource structure
Hi Mike,
>-Original Message-
>From: Kalliguddi, Hema
>Sent: Thursday, September 23, 2010 5:57 AM
>To: linux-omap@vger.kernel.org; linux-...@vger.kernel.org
>Cc: Kalliguddi, Hema; Balbi, Felipe; Tony Lindgren; Kevin
>Hilman; Cousson, Benoit; Paul Walmsley
>Subject: [PATCH 1/9 v3] usb: musb: Adding names for IRQs in
>resource structure
>
>Modified the Omap,Blackfin and Davinci board files to add the
>name of the IRQs
>in the resource structures and musb driver to use the
>get_irq_byname() api to
>get the mc and dma irq numbers instead of using the index as
>the order of
>resource definition need not be always in order of device interrupt and
>then dma interrupt
>
>Signed-off-by: Hema HK
>Cc: Felipe Balbi
>Cc: Tony Lindgren
>Cc: Kevin Hilman
>Cc: Cousson, Benoit
>Cc: Paul Walmsley
>
>---
> arch/arm/mach-davinci/usb.c|2 ++
> arch/arm/mach-omap2/usb-musb.c |2 ++
> arch/blackfin/mach-bf527/boards/cm_bf527.c |2 ++
> arch/blackfin/mach-bf527/boards/ezbrd.c|2 ++
> arch/blackfin/mach-bf527/boards/ezkit.c|2 ++
> arch/blackfin/mach-bf548/boards/cm_bf548.c |2 ++
> arch/blackfin/mach-bf548/boards/ezkit.c|2 ++
> drivers/usb/musb/cppi_dma.c|2 +-
> drivers/usb/musb/musb_core.c |2 +-
> drivers/usb/musb/musbhsdma.c |2 +-
> 10 files changed, 17 insertions(+), 3 deletions(-)
>
>Index: linux-omap-pm/arch/arm/mach-davinci/usb.c
>===
>--- linux-omap-pm.orig/arch/arm/mach-davinci/usb.c
>+++ linux-omap-pm/arch/arm/mach-davinci/usb.c
>@@ -64,10 +64,12 @@ static struct resource usb_resources[] =
> {
> .start = IRQ_USBINT,
> .flags = IORESOURCE_IRQ,
>+ .name = "mc"
> },
> {
> /* placeholder for the dedicated CPPI IRQ */
> .flags = IORESOURCE_IRQ,
>+ .name = "dma"
> },
> };
>
>Index: linux-omap-pm/arch/arm/mach-omap2/usb-musb.c
>===
>--- linux-omap-pm.orig/arch/arm/mach-omap2/usb-musb.c
>+++ linux-omap-pm/arch/arm/mach-omap2/usb-musb.c
>@@ -39,10 +39,12 @@ static struct resource musb_resources[]
> [1] = { /* general IRQ */
> .start = INT_243X_HS_USB_MC,
> .flags = IORESOURCE_IRQ,
>+ .name = "mc",
> },
> [2] = { /* DMA IRQ */
> .start = INT_243X_HS_USB_DMA,
> .flags = IORESOURCE_IRQ,
>+ .name = "dma",
> },
> };
>
>Index: linux-omap-pm/arch/blackfin/mach-bf527/boards/cm_bf527.c
>===
>--- linux-omap-pm.orig/arch/blackfin/mach-bf527/boards/cm_bf527.c
>+++ linux-omap-pm/arch/blackfin/mach-bf527/boards/cm_bf527.c
>@@ -82,11 +82,13 @@ static struct resource musb_resources[]
> .start = IRQ_USB_INT0,
> .end= IRQ_USB_INT0,
> .flags = IORESOURCE_IRQ | IORESOURCE_IRQ_HIGHLEVEL,
>+ .name = "mc"
> },
> [2] = { /* DMA IRQ */
> .start = IRQ_USB_DMA,
> .end= IRQ_USB_DMA,
> .flags = IORESOURCE_IRQ | IORESOURCE_IRQ_HIGHLEVEL,
>+ .name = "dma"
> },
> };
>
>Index: linux-omap-pm/arch/blackfin/mach-bf527/boards/ezbrd.c
>===
>--- linux-omap-pm.orig/arch/blackfin/mach-bf527/boards/ezbrd.c
>+++ linux-omap-pm/arch/blackfin/mach-bf527/boards/ezbrd.c
>@@ -46,11 +46,13 @@ static struct resource musb_resources[]
> .start = IRQ_USB_INT0,
> .end= IRQ_USB_INT0,
> .flags = IORESOURCE_IRQ | IORESOURCE_IRQ_HIGHLEVEL,
>+ .name = "mc"
> },
> [2] = { /* DMA IRQ */
> .start = IRQ_USB_DMA,
> .end= IRQ_USB_DMA,
> .flags = IORESOURCE_IRQ | IORESOURCE_IRQ_HIGHLEVEL,
>+ .name = "dma"
> },
> };
>
>Index: linux-omap-pm/arch/blackfin/mach-bf527/boards/ezkit.c
>===
>--- linux-omap-pm.orig/arch/blackfin/mach-bf527/boards/ezkit.c
>+++ linux-omap-pm/arch/blackfin/mach-bf527/boards/ezkit.c
>@@ -86,11 +86,13 @@ static struct resource musb_resources[]
> .start = IRQ_USB_INT0,
> .e
RE: [PATCH 1/9 v3] usb: musb: Adding names for IRQs in resource structure
Hi, >-Original Message- >From: Balbi, Felipe >Sent: Thursday, September 23, 2010 11:40 AM >To: Kalliguddi, Hema >Cc: linux-omap@vger.kernel.org; linux-...@vger.kernel.org; >Balbi, Felipe; Tony Lindgren; Kevin Hilman; Cousson, Benoit; >Paul Walmsley >Subject: Re: [PATCH 1/9 v3] usb: musb: Adding names for IRQs >in resource structure > >On Wed, Sep 22, 2010 at 07:27:11PM -0500, Kalliguddi, Hema wrote: >>Modified the Omap,Blackfin and Davinci board files to add the >name of the IRQs >>in the resource structures and musb driver to use the >get_irq_byname() api to >>get the mc and dma irq numbers instead of using the index as >the order of >>resource definition need not be always in order of device >interrupt and >>then dma interrupt >> >>Signed-off-by: Hema HK >>Cc: Felipe Balbi >>Cc: Tony Lindgren >>Cc: Kevin Hilman >>Cc: Cousson, Benoit >>Cc: Paul Walmsley > >this is fine by me, but you need to Cc the other ARCH maintainers, Tony >can answer for OMAP and Kevin for DaVinci, but where's Blackfin's >maintainer ? Without all theirs ACKs for the arch-specific >part, I can't >queue this to .37 and it's already quite late. If we don't get all >needed Acks by tomorrow, I doubt Greg will accept these for .37 merge >window. > I understand that. I forgot to Cc Mike for blackfin arch files change. I will post it again today Cc ing him and try my luck :-) ~Hema >-- >balbi >-- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 2/2] OMAP2+: GPIO: move late PM out of interrupts-disabled idle path
>-Original Message-
>From: Kevin Hilman [mailto:khil...@deeprootsystems.com]
>Sent: Wednesday, September 22, 2010 7:55 PM
>To: Kalliguddi, Hema
>Cc: linux-omap@vger.kernel.org; Varadarajan, Charulatha;
>Basak, Partha; Tero Kristo
>Subject: Re: [PATCH 2/2] OMAP2+: GPIO: move late PM out of
>interrupts-disabled idle path
>
>"Kalliguddi, Hema" writes:
>
>>>-Original Message-
>>>From: linux-omap-ow...@vger.kernel.org
>>>[mailto:linux-omap-ow...@vger.kernel.org] On Behalf Of Kevin Hilman
>>>Sent: Tuesday, September 14, 2010 4:33 AM
>>>To: linux-omap@vger.kernel.org
>>>Cc: Varadarajan, Charulatha; Basak, Partha; Tero Kristo
>>>Subject: [PATCH 2/2] OMAP2+: GPIO: move late PM out of
>>>interrupts-disabled idle path
>>>
>>>From: Kevin Hilman
>>>
>>>Currently, we wait until late in the idle path where interrupts are
>>>disabled to do runtime-PM-like management for certain special-case
>>>devices like GPIO.
>>>
>>>As a prerequiste to moving GPIO to the new runtime PM framework, move
>>>this runtime-PM-like code out of the late idle path into new device
>>>idle and resume functions that can be called before interrupts are
>>>disabled by CPUidle and/or suspend.
>>>
>>>In addition, move all the GPIO-specific logic into the GPIO core
>>>instead of keeping GPIO-specific knowledge of power-states, context
>>>saving etc. in the PM core.
>>>
>>>Also, call the new device-idle and -resume methods from CPUidle and
>>>static suspend path.
>>>
>>>Signed-off-by: Kevin Hilman
>>>---
>>> arch/arm/mach-omap2/cpuidle34xx.c |4 ++
>>> arch/arm/mach-omap2/pm.h |2 +
>>> arch/arm/mach-omap2/pm24xx.c |2 +-
>>> arch/arm/mach-omap2/pm34xx.c | 38 +
>>> arch/arm/plat-omap/gpio.c | 57
>>>
>>> arch/arm/plat-omap/include/plat/gpio.h |4 +--
>>> 6 files changed, 67 insertions(+), 40 deletions(-)
>>>
>>>diff --git a/arch/arm/mach-omap2/cpuidle34xx.c
>>>b/arch/arm/mach-omap2/cpuidle34xx.c
>>>index 0923b82..681d823 100644
>>>--- a/arch/arm/mach-omap2/cpuidle34xx.c
>>>+++ b/arch/arm/mach-omap2/cpuidle34xx.c
>>>@@ -274,9 +274,13 @@ static int omap3_enter_idle_bm(struct
>>>cpuidle_device *dev,
>>> pwrdm_set_next_pwrst(per_pd, per_next_state);
>>>
>>> select_state:
>>>+omap3_device_idle();
>>>+
>>> dev->last_state = new_state;
>>> ret = omap3_enter_idle(dev, new_state);
>>>
>>>+omap3_device_resume();
>>>+
>>> /* Restore original PER state if it was modified */
>>> if (per_next_state != per_saved_state)
>>> pwrdm_set_next_pwrst(per_pd, per_saved_state);
>>>diff --git a/arch/arm/mach-omap2/pm.h b/arch/arm/mach-omap2/pm.h
>>>index 3de6ece..33cae4b 100644
>>>--- a/arch/arm/mach-omap2/pm.h
>>>+++ b/arch/arm/mach-omap2/pm.h
>>>@@ -22,6 +22,8 @@ extern void omap_sram_idle(void);
>>> extern int omap3_can_sleep(void);
>>> extern int set_pwrdm_state(struct powerdomain *pwrdm, u32 state);
>>> extern int omap3_idle_init(void);
>>>+extern void omap3_device_idle(void);
>>>+extern void omap3_device_resume(void);
>>>
>>> struct cpuidle_params {
>>> u8 valid;
>>>diff --git a/arch/arm/mach-omap2/pm24xx.c
>>>b/arch/arm/mach-omap2/pm24xx.c
>>>index 6aeedea..7bbf0db 100644
>>>--- a/arch/arm/mach-omap2/pm24xx.c
>>>+++ b/arch/arm/mach-omap2/pm24xx.c
>>>@@ -106,7 +106,7 @@ static void omap2_enter_full_retention(void)
>>> l = omap_ctrl_readl(OMAP2_CONTROL_DEVCONF0) |
>>>OMAP24XX_USBSTANDBYCTRL;
>>> omap_ctrl_writel(l, OMAP2_CONTROL_DEVCONF0);
>>>
>>>-omap2_gpio_prepare_for_idle(PWRDM_POWER_RET);
>>>+omap2_gpio_prepare_for_idle();
>>>
>>> if (omap2_pm_debug) {
>>> omap2_pm_dump(0, 0, 0);
>>>diff --git a/arch/arm/mach-omap2/pm34xx.c
>>>b/arch/arm/mach-omap2/pm34xx.c
>>>index bb2ba1e..9e590d9 100644
>>>--- a/arch/arm/mach-omap2/pm34xx.c
>>>+++ b/arch/arm/mach-omap2/pm34xx.c
>>>@@ -74,16 +74,6 @@ static struct powerdomain *mpu_pwrdm, *neon_pwrdm;
>>> static struct powerdomain *core_pwrdm, *per_pwrdm;
>>> static struct powerdomain *cam_pwrdm;
>>>
>>&g
RE: [PATCH 2/2] OMAP2+: GPIO: move late PM out of interrupts-disabled idle path
Hi,
>-Original Message-
>From: linux-omap-ow...@vger.kernel.org
>[mailto:linux-omap-ow...@vger.kernel.org] On Behalf Of Kevin Hilman
>Sent: Tuesday, September 14, 2010 4:33 AM
>To: linux-omap@vger.kernel.org
>Cc: Varadarajan, Charulatha; Basak, Partha; Tero Kristo
>Subject: [PATCH 2/2] OMAP2+: GPIO: move late PM out of
>interrupts-disabled idle path
>
>From: Kevin Hilman
>
>Currently, we wait until late in the idle path where interrupts are
>disabled to do runtime-PM-like management for certain special-case
>devices like GPIO.
>
>As a prerequiste to moving GPIO to the new runtime PM framework, move
>this runtime-PM-like code out of the late idle path into new device
>idle and resume functions that can be called before interrupts are
>disabled by CPUidle and/or suspend.
>
>In addition, move all the GPIO-specific logic into the GPIO core
>instead of keeping GPIO-specific knowledge of power-states, context
>saving etc. in the PM core.
>
>Also, call the new device-idle and -resume methods from CPUidle and
>static suspend path.
>
>Signed-off-by: Kevin Hilman
>---
> arch/arm/mach-omap2/cpuidle34xx.c |4 ++
> arch/arm/mach-omap2/pm.h |2 +
> arch/arm/mach-omap2/pm24xx.c |2 +-
> arch/arm/mach-omap2/pm34xx.c | 38 +
> arch/arm/plat-omap/gpio.c | 57
>
> arch/arm/plat-omap/include/plat/gpio.h |4 +--
> 6 files changed, 67 insertions(+), 40 deletions(-)
>
>diff --git a/arch/arm/mach-omap2/cpuidle34xx.c
>b/arch/arm/mach-omap2/cpuidle34xx.c
>index 0923b82..681d823 100644
>--- a/arch/arm/mach-omap2/cpuidle34xx.c
>+++ b/arch/arm/mach-omap2/cpuidle34xx.c
>@@ -274,9 +274,13 @@ static int omap3_enter_idle_bm(struct
>cpuidle_device *dev,
> pwrdm_set_next_pwrst(per_pd, per_next_state);
>
> select_state:
>+ omap3_device_idle();
>+
> dev->last_state = new_state;
> ret = omap3_enter_idle(dev, new_state);
>
>+ omap3_device_resume();
>+
> /* Restore original PER state if it was modified */
> if (per_next_state != per_saved_state)
> pwrdm_set_next_pwrst(per_pd, per_saved_state);
>diff --git a/arch/arm/mach-omap2/pm.h b/arch/arm/mach-omap2/pm.h
>index 3de6ece..33cae4b 100644
>--- a/arch/arm/mach-omap2/pm.h
>+++ b/arch/arm/mach-omap2/pm.h
>@@ -22,6 +22,8 @@ extern void omap_sram_idle(void);
> extern int omap3_can_sleep(void);
> extern int set_pwrdm_state(struct powerdomain *pwrdm, u32 state);
> extern int omap3_idle_init(void);
>+extern void omap3_device_idle(void);
>+extern void omap3_device_resume(void);
>
> struct cpuidle_params {
> u8 valid;
>diff --git a/arch/arm/mach-omap2/pm24xx.c
>b/arch/arm/mach-omap2/pm24xx.c
>index 6aeedea..7bbf0db 100644
>--- a/arch/arm/mach-omap2/pm24xx.c
>+++ b/arch/arm/mach-omap2/pm24xx.c
>@@ -106,7 +106,7 @@ static void omap2_enter_full_retention(void)
> l = omap_ctrl_readl(OMAP2_CONTROL_DEVCONF0) |
>OMAP24XX_USBSTANDBYCTRL;
> omap_ctrl_writel(l, OMAP2_CONTROL_DEVCONF0);
>
>- omap2_gpio_prepare_for_idle(PWRDM_POWER_RET);
>+ omap2_gpio_prepare_for_idle();
>
> if (omap2_pm_debug) {
> omap2_pm_dump(0, 0, 0);
>diff --git a/arch/arm/mach-omap2/pm34xx.c
>b/arch/arm/mach-omap2/pm34xx.c
>index bb2ba1e..9e590d9 100644
>--- a/arch/arm/mach-omap2/pm34xx.c
>+++ b/arch/arm/mach-omap2/pm34xx.c
>@@ -74,16 +74,6 @@ static struct powerdomain *mpu_pwrdm, *neon_pwrdm;
> static struct powerdomain *core_pwrdm, *per_pwrdm;
> static struct powerdomain *cam_pwrdm;
>
>-static inline void omap3_per_save_context(void)
>-{
>- omap_gpio_save_context();
>-}
>-
>-static inline void omap3_per_restore_context(void)
>-{
>- omap_gpio_restore_context();
>-}
>-
> static void omap3_enable_io_chain(void)
> {
> int timeout = 0;
>@@ -332,6 +322,16 @@ static void restore_table_entry(void)
> restore_control_register(control_reg_value);
> }
>
>+void omap3_device_idle(void)
>+{
>+ omap2_gpio_prepare_for_idle();
>+}
>+
Is not it is a good idea to pass the new_state as the parameter for the driver
calls?
It will reduce each individual drivers reading the PRCM register to findout the
next state.
This might save some time in the idle loop.
>+void omap3_device_resume(void)
>+{
>+ omap2_gpio_resume_after_idle();
>+}
>+
Here we will need to pass the next_state as well as previous_state as
parameters. If we agree to
pass the parameters.
~Hema
> void omap_sram_idle(void)
> {
> /* Variable to tell what needs to be saved and restored
>@@ -344,7 +344,7 @@ void omap_sram_idle(void)
> int mpu_next_state = PWRDM_POWER_ON;
> int per_next_state = PWRDM_POWER_ON;
> int core_next_state = PWRDM_POWER_ON;
>- int core_prev_state, per_prev_state;
>+ int core_prev_state;
> u32 sdrc_pwr = 0;
>
> if (!_omap_sram_idle)
>@@ -387,12 +387,9 @@ void omap_sram_idle(void)
> }
>
> /* PER */
>- if (per_next_state < PWRDM_PO
RE: [PATCH 8/8 v2] usb : musb: Using runtime pm apis for musb.
>-Original Message-
>From: Kevin Hilman [mailto:khil...@deeprootsystems.com]
>Sent: Tuesday, August 31, 2010 8:54 PM
>To: Kalliguddi, Hema
>Cc: linux-omap@vger.kernel.org; linux-...@vger.kernel.org;
>Basak, Partha; Felipe Balbi; Tony Lindgren; Cousson, Benoit;
>Paul Walmsley
>Subject: Re: [PATCH 8/8 v2] usb : musb: Using runtime pm apis for musb.
>
>"Kalliguddi, Hema" writes:
>
>>>> static int musb_platform_resume(struct musb *musb)
>>>> {
>>>> u32 l;
>>>> +struct device *dev = musb->controller;
>>>> +struct musb_hdrc_platform_data *pdata = dev->platform_data;
>>>> +struct platform_device *pdev = to_platform_device(dev);
>>>>
>>>> if (!musb->clock)
>>>> return 0;
>>>>
>>>> otg_set_suspend(musb->xceiv, 0);
>>>> -
>>>> -if (musb->set_clock)
>>>> -musb->set_clock(musb->clock, 1);
>>>> -else
>>>> -clk_enable(musb->clock);
>>>> -
>>>> -l = musb_readl(musb->mregs, OTG_SYSCONFIG);
>>>> -l &= ~ENABLEWAKEUP; /* disable wakeup */
>>>> -musb_writel(musb->mregs, OTG_SYSCONFIG, l);
>>>> -
>>>> +pm_runtime_enable(dev);
>>>> +pm_runtime_get_sync(dev);
>>>> +pdata->enable_wakeup(pdev);
>>>
>>>I think you mean ->disable_wakeup() here, right?
>>>
>>
>> No I meant enable_wakeup only here. When smart idle/standby
>is enabled,
>> wakeup bit has to be set to generate the s-wakeup when the
>devie is in idle
>> and system is in ret.
>
>OK, I'm confused.
>
>The code being removed just above disables wakeups and the new code
>enables wakeups.
>
The old code was not correct. The wakeup enable make sense only when the smart
idle/smart
Standby is enabled. And with get_sync musb is configured to be in smart
idle/standby mode.
>Also, you don't ever call ->disable_wakeup() elsewhere in the patch.
>You call pdata->enable_wakeup() both in suspend and resume.
Disable_wkaeup is not required now as there is no need of disabling this bit
because with force idle/standby
there is no use of wakeup enable/disable bit.
Anyway again, we need have these APIs if Rajendra is going to post the patch to
enable wkaeup in the
framework if smart idle/standby is enabled.
>
>Kevin
>--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 6/8 v2] usb: musb: Offmode fix for idle path
Hi,
>-Original Message-
>From: Kevin Hilman [mailto:khil...@deeprootsystems.com]
>Sent: Tuesday, August 31, 2010 8:49 PM
>To: Kalliguddi, Hema
>Cc: linux-omap@vger.kernel.org; linux-...@vger.kernel.org;
>Mankad, Maulik Ojas; Felipe Balbi; Tony Lindgren; Cousson,
>Benoit; Paul Walmsley
>Subject: Re: [PATCH 6/8 v2] usb: musb: Offmode fix for idle path
>
>"Kalliguddi, Hema" writes:
>
>>>-Original Message-
>>>From: Kevin Hilman [mailto:khil...@deeprootsystems.com]
>>>Sent: Thursday, August 26, 2010 5:40 AM
>>>To: Kalliguddi, Hema
>>>Cc: linux-omap@vger.kernel.org; linux-...@vger.kernel.org;
>>>Mankad, Maulik Ojas; Felipe Balbi; Tony Lindgren; Cousson,
>>>Benoit; Paul Walmsley
>>>Subject: Re: [PATCH 6/8 v2] usb: musb: Offmode fix for idle path
>>>
>>>Hema HK writes:
>>>
>>>> With OMAP core-off support musb was not functional as
>>>context was getting
>>>> lost after wakeup from core-off. And also musb was blocking
>>>the core-off
>>>> after loading the gadget driver even with no cable connected
>>>sometimes.
>>>>
>>>> Added the conext save/restore api in the platform layer which will
>>>> be called in the idle and wakeup path.
>>>>
>>>> Changed the usb sysconfig settings as per the usbotg
>functional spec.
>>>> When the device is not used, configure to force idle and
>>>force standby mode.
>>>> When it is being used, configure in smart standby and smart
>>>idle mode.
>>>> So while attempting to coreoff the usb is configured to
>>>force standby and
>>>> force idle mode, after wakeup configured in smart idle and
>>>smart standby.
>>>
>>>You don't describe the new .clk_autogated field added here.
>That part
>>>should be a separate patch as it's not directly related to
>>>$SUBJECT patch.
>>>
>>
>> In OMAP for USB there is no need to disable the interface
>clock. But for the other
>> platforms like davinci which uses mentor USB IP clock is not
>auto gated so
>> There is a need to disbale the clock when .suspend API
>defined in the driver is called.
>> So introduced a flag in the platform to enable/disable the clock
>> In .supend and .resume APIs appropriately in the driver code.
>
>Yes, I understand why it's there, and just suggested that it should be
>documented and done as a separate patch.
>
>> Yes. It may not be directly related to $SUBJECT patch, but I
>am reusing
>> .suspend and .resume APIs for context save and restore
>during idle path.
>> Because of that this fix as part of the patch.
>>
>>>> Signed-off-by: Hema HK
>>>> Signed-off-by: Maulik Mankad
>>>>
>>>> Cc: Felipe Balbi
>>>> Cc: Tony Lindgren
>>>> Cc: Kevin Hilman
>>>> Cc: Cousson, Benoit
>>>> Cc: Paul Walmsley
>>>> ---
>>>> arch/arm/mach-omap2/pm34xx.c |5 +++
>>>> arch/arm/mach-omap2/usb-musb.c| 42
>>>-
>>>> arch/arm/plat-omap/include/plat/usb.h |2 +
>>>> drivers/usb/musb/musb_core.c | 10 +++
>>>> drivers/usb/musb/musb_core.h |1 -
>>>> drivers/usb/musb/omap2430.c | 48
>>>++---
>>>> include/linux/usb/musb.h |6
>>>> 7 files changed, 102 insertions(+), 12 deletions(-)
>>>>
>>>> diff --git a/arch/arm/mach-omap2/pm34xx.c
>>>b/arch/arm/mach-omap2/pm34xx.c
>>>> index fb4994a..7b34201 100644
>>>> --- a/arch/arm/mach-omap2/pm34xx.c
>>>> +++ b/arch/arm/mach-omap2/pm34xx.c
>>>> @@ -39,6 +39,7 @@
>>>> #include
>>>> #include
>>>> #include
>>>> +#include
>>>>
>>>> #include
>>>>
>>>> @@ -416,6 +417,8 @@ void omap_sram_idle(void)
>>>>if (core_next_state == PWRDM_POWER_OFF) {
>>>>omap3_core_save_context();
>>>>omap3_prcm_save_context();
>>>> + /* Save MUSB context */
>>>> + musb_context_save_restore(1);
>>>>}
>>>>}
>>>>
>>>> @@ -458,6 +461,8 @@ void omap_sram_idle(void)
>>>>omap3_prcm_restore_context();
>>&g
RE: [PATCH 7/8 v2] OMAP: Hwmod api changes
Hi,
>-Original Message-
>From: Kevin Hilman [mailto:khil...@deeprootsystems.com]
>Sent: Thursday, August 26, 2010 5:12 AM
>To: Kalliguddi, Hema
>Cc: linux-omap@vger.kernel.org; linux-...@vger.kernel.org;
>Basak, Partha; Felipe Balbi; Tony Lindgren; Cousson, Benoit;
>Paul Walmsley
>Subject: Re: [PATCH 7/8 v2] OMAP: Hwmod api changes
>
>Hema HK writes:
>
>> OMAP USBOTG modules has a requirement to set the auto idle
>bit only after
>> setting smart idle bit. Modified the _sys_enable api to set
>the smart idle
>> first and then the autoidle bit. Setting this will not have
>any impact on the
>> other modules.
>>
>> Added 2 wrapper APIs in the omap device layer for wakeup
>enable/disable
>> and sidle/mstandby settings.
>
>This should be a separate patch, with an description of who
>the users of
>this API would be and why.
>
Ok. I can post it as separate patch also. But I think there was plan from
Rajendra to
Enable the wakeup as part of the sysc_enable() if smart idle/standby is
configured.
If that implementation is done then there is no need of this patch.
>> Signed-off-by: Hema HK
>> Signed-off-by: Basak, Partha
>>
>> Cc: Felipe Balbi
>> Cc: Tony Lindgren
>> Cc: Kevin Hilman
>> Cc: Cousson, Benoit
>> Cc: Paul Walmsley
>> ---
>> arch/arm/mach-omap2/omap_hwmod.c | 18 +++---
>> arch/arm/plat-omap/include/plat/omap_device.h |2 +
>> arch/arm/plat-omap/omap_device.c | 43
>+
>> 3 files changed, 57 insertions(+), 6 deletions(-)
>>
>> diff --git a/arch/arm/mach-omap2/omap_hwmod.c
>b/arch/arm/mach-omap2/omap_hwmod.c
>> index 9bd99ad..55507a6 100644
>> --- a/arch/arm/mach-omap2/omap_hwmod.c
>> +++ b/arch/arm/mach-omap2/omap_hwmod.c
>> @@ -654,12 +654,6 @@ static void _sysc_enable(struct omap_hwmod *oh)
>> _set_master_standbymode(oh, idlemode, &v);
>> }
>>
>> -if (sf & SYSC_HAS_AUTOIDLE) {
>> -idlemode = (oh->flags & HWMOD_NO_OCP_AUTOIDLE) ?
>> -0 : 1;
>> -_set_module_autoidle(oh, idlemode, &v);
>> -}
>> -
>> /* XXX OCP ENAWAKEUP bit? */
>>
>> /*
>> @@ -672,6 +666,18 @@ static void _sysc_enable(struct omap_hwmod *oh)
>> _set_clockactivity(oh, oh->class->sysc->clockact, &v);
>>
>> _write_sysconfig(v, oh);
>> +
>> +/* Set the auto idle bit only after setting the smartidle bit
>> + * as this is requirement for some modules like USBOTG
>> + * setting this will not have any impact on the other modues.
>> + */
>
>Please fix multi-line comment style. (search for multi-line in
>Documentation/CodingStyle)
>
Sure I will fix it.
>Kevin
>
>> +if (sf & SYSC_HAS_AUTOIDLE) {
>> +idlemode = (oh->flags & HWMOD_NO_OCP_AUTOIDLE) ?
>> +0 : 1;
>> +_set_module_autoidle(oh, idlemode, &v);
>> +}
>> +_write_sysconfig(v, oh);
>> }
>>
>> /**
>> diff --git a/arch/arm/plat-omap/include/plat/omap_device.h
>b/arch/arm/plat-omap/include/plat/omap_device.h
>> index 25cd9ac..c3eb07e 100644
>> --- a/arch/arm/plat-omap/include/plat/omap_device.h
>> +++ b/arch/arm/plat-omap/include/plat/omap_device.h
>> @@ -116,6 +116,8 @@ int omap_device_enable_hwmods(struct
>omap_device *od);
>> int omap_device_disable_clocks(struct omap_device *od);
>> int omap_device_enable_clocks(struct omap_device *od);
>>
>> +int omap_device_enable_wakeup(struct platform_device *pdev);
>> +int omap_device_disable_wakeup(struct platform_device *pdev);
>>
>> /*
>> * Entries should be kept in latency order ascending
>> diff --git a/arch/arm/plat-omap/omap_device.c
>b/arch/arm/plat-omap/omap_device.c
>> index d2b1609..10182b1 100644
>> --- a/arch/arm/plat-omap/omap_device.c
>> +++ b/arch/arm/plat-omap/omap_device.c
>> @@ -757,3 +757,46 @@ int omap_device_enable_clocks(struct
>omap_device *od)
>> /* XXX pass along return value here? */
>> return 0;
>> }
>> +
>> +/**
>> + * omap_device_enable_wakeup - Enable the wakeup bit
>> + * @od: struct omap_device *od
>> + *
>> + * Enable the wakup bit for omap_hwmods associated
>> + * with the omap_device. Returns 0.
>> + */
>> +int omap_device_enable_wakeup(struct platform_device *pdev)
>> +{
>> +struct omap_hwmod *oh;
>> +struct omap_device *od = _find_by_pdev(pdev);
>
RE: [PATCH 8/8 v2] usb : musb: Using runtime pm apis for musb.
Hi,
>-Original Message-
>From: Kevin Hilman [mailto:khil...@deeprootsystems.com]
>Sent: Thursday, August 26, 2010 5:44 AM
>To: Kalliguddi, Hema
>Cc: linux-omap@vger.kernel.org; linux-...@vger.kernel.org;
>Basak, Partha; Felipe Balbi; Tony Lindgren; Cousson, Benoit;
>Paul Walmsley
>Subject: Re: [PATCH 8/8 v2] usb : musb: Using runtime pm apis for musb.
>
>Hema HK writes:
>
>> Calling runtime pm APIs pm_runtime_put_sync() and
>pm_runtime_get_sync()
>> for enabling/disabling the clocks,sysconfig settings.
>>
>> used omap_hwmod_enable_wakeup & omap_hwmod_disable_wakeup
>apis to set/clear
>> the wakeup enable bit.
>> Also need to put the USB in force standby and force idle
>mode when usb not used
>> and set it back to smart idle and smart stndby after wakeup.
>> these cases are handled using the oh flags.
>> For omap3430 auto idle bit has to be disabled because of the
>errata.So using
>> HWMOD_NO_OCP_AUTOIDLE flag for OMAP3430.
>>
>> Signed-off-by: Hema HK
>> Signed-off-by: Basak, Partha
>>
>> Cc: Felipe Balbi
>> Cc: Tony Lindgren
>> Cc: Kevin Hilman
>> Cc: Cousson, Benoit
>> Cc: Paul Walmsley
>> ---
>> arch/arm/mach-omap2/pm34xx.c |8 ++-
>> arch/arm/mach-omap2/usb-musb.c| 86
>++--
>> arch/arm/plat-omap/include/plat/usb.h |9 +++-
>> drivers/usb/musb/musb_core.c | 12 +
>> drivers/usb/musb/omap2430.c | 65
>+
>> include/linux/usb/musb.h |8 +++
>> 6 files changed, 127 insertions(+), 61 deletions(-)
>>
>> diff --git a/arch/arm/mach-omap2/pm34xx.c
>b/arch/arm/mach-omap2/pm34xx.c
>> index 7b34201..0eb39b3 100644
>> --- a/arch/arm/mach-omap2/pm34xx.c
>> +++ b/arch/arm/mach-omap2/pm34xx.c
>> @@ -418,7 +418,9 @@ void omap_sram_idle(void)
>> omap3_core_save_context();
>> omap3_prcm_save_context();
>> /* Save MUSB context */
>> -musb_context_save_restore(1);
>> +musb_context_save_restore(save_context);
>> +} else {
>> +musb_context_save_restore(disable_clk);
>
>Presumably the 'disable_clk' is meant to mean "no need to save context,
>just disable clock", in which case the function name is not really
>accurate anymore.
>
>What is needed is just a general function that takes the next power
>state and let the function internals make the decision. The idle loop
>should not have IP-specific logic in it.
>
>I don't really want any IP specific logic in this part of the
>idle loop.
>What I really would like to see is all of this driver specific stuff
>moved out of the core idle path and done before interrupts are
>disabled.
>
>If we can do this before interrups are disabled (a bit earlier in the
>CPUidle path), then we can just use the normal runtime PM API and not
>have to handle the special cases of doing all this black magic inside
>the core idle path.
>
This can be done. I will have a generic usb idle and wakeup functions defined
and will be called with next/previous core state as parameter and call
Before/after the interrupts are disabled/enabled as you suggested, and handle
the required
cases in the musb module.
I will post the patch with changes after testing.
>> }
>> }
>>
>> @@ -462,7 +464,9 @@ void omap_sram_idle(void)
>> omap3_sram_restore_context();
>> omap2_sms_restore_context();
>> /* restore MUSB context */
>> -musb_context_save_restore(0);
>> +musb_context_save_restore(restore_context);
>> +} else {
>> +musb_context_save_restore(enable_clk);
>> }
>> omap_uart_resume_idle(0);
>> omap_uart_resume_idle(1);
>> diff --git a/arch/arm/mach-omap2/usb-musb.c
>b/arch/arm/mach-omap2/usb-musb.c
>> index 9d10440..b7736d2 100644
>> --- a/arch/arm/mach-omap2/usb-musb.c
>> +++ b/arch/arm/mach-omap2/usb-musb.c
>> @@ -23,6 +23,7 @@
>> #include
>> #include
>> #include
>> +#include
>>
>> #include
>>
>> @@ -64,13 +65,32 @@ static struct musb_hdrc_platform_data
>musb_plat = {
>> /* supports clock autogating */
>> .clk_autogated = 1,
>> };
>> +static int usb_idle_hwmod(struct omap_device *od)
>> +{
>> +struct omap_hwmod *oh = *od->hwmods;
>> +
RE: [PATCH 6/8 v2] usb: musb: Offmode fix for idle path
>-Original Message-
>From: Kevin Hilman [mailto:khil...@deeprootsystems.com]
>Sent: Thursday, August 26, 2010 5:40 AM
>To: Kalliguddi, Hema
>Cc: linux-omap@vger.kernel.org; linux-...@vger.kernel.org;
>Mankad, Maulik Ojas; Felipe Balbi; Tony Lindgren; Cousson,
>Benoit; Paul Walmsley
>Subject: Re: [PATCH 6/8 v2] usb: musb: Offmode fix for idle path
>
>Hema HK writes:
>
>> With OMAP core-off support musb was not functional as
>context was getting
>> lost after wakeup from core-off. And also musb was blocking
>the core-off
>> after loading the gadget driver even with no cable connected
>sometimes.
>>
>> Added the conext save/restore api in the platform layer which will
>> be called in the idle and wakeup path.
>>
>> Changed the usb sysconfig settings as per the usbotg functional spec.
>> When the device is not used, configure to force idle and
>force standby mode.
>> When it is being used, configure in smart standby and smart
>idle mode.
>> So while attempting to coreoff the usb is configured to
>force standby and
>> force idle mode, after wakeup configured in smart idle and
>smart standby.
>
>You don't describe the new .clk_autogated field added here. That part
>should be a separate patch as it's not directly related to
>$SUBJECT patch.
>
In OMAP for USB there is no need to disable the interface clock. But for the
other
platforms like davinci which uses mentor USB IP clock is not auto gated so
There is a need to disbale the clock when .suspend API defined in the driver is
called.
So introduced a flag in the platform to enable/disable the clock
In .supend and .resume APIs appropriately in the driver code.
Yes. It may not be directly related to $SUBJECT patch, but I am reusing
.suspend and .resume APIs for context save and restore during idle path.
Because of that this fix as part of the patch.
>> Signed-off-by: Hema HK
>> Signed-off-by: Maulik Mankad
>>
>> Cc: Felipe Balbi
>> Cc: Tony Lindgren
>> Cc: Kevin Hilman
>> Cc: Cousson, Benoit
>> Cc: Paul Walmsley
>> ---
>> arch/arm/mach-omap2/pm34xx.c |5 +++
>> arch/arm/mach-omap2/usb-musb.c| 42
>-
>> arch/arm/plat-omap/include/plat/usb.h |2 +
>> drivers/usb/musb/musb_core.c | 10 +++
>> drivers/usb/musb/musb_core.h |1 -
>> drivers/usb/musb/omap2430.c | 48
>++---
>> include/linux/usb/musb.h |6
>> 7 files changed, 102 insertions(+), 12 deletions(-)
>>
>> diff --git a/arch/arm/mach-omap2/pm34xx.c
>b/arch/arm/mach-omap2/pm34xx.c
>> index fb4994a..7b34201 100644
>> --- a/arch/arm/mach-omap2/pm34xx.c
>> +++ b/arch/arm/mach-omap2/pm34xx.c
>> @@ -39,6 +39,7 @@
>> #include
>> #include
>> #include
>> +#include
>>
>> #include
>>
>> @@ -416,6 +417,8 @@ void omap_sram_idle(void)
>> if (core_next_state == PWRDM_POWER_OFF) {
>> omap3_core_save_context();
>> omap3_prcm_save_context();
>> +/* Save MUSB context */
>> +musb_context_save_restore(1);
>> }
>> }
>>
>> @@ -458,6 +461,8 @@ void omap_sram_idle(void)
>> omap3_prcm_restore_context();
>> omap3_sram_restore_context();
>> omap2_sms_restore_context();
>> +/* restore MUSB context */
>> +musb_context_save_restore(0);
>> }
>> omap_uart_resume_idle(0);
>> omap_uart_resume_idle(1);
>> diff --git a/arch/arm/mach-omap2/usb-musb.c
>b/arch/arm/mach-omap2/usb-musb.c
>> index c228cc0..9d10440 100644
>> --- a/arch/arm/mach-omap2/usb-musb.c
>> +++ b/arch/arm/mach-omap2/usb-musb.c
>> @@ -35,6 +35,7 @@
>>
>> static const char name[] = "musb_hdrc";
>> #define MAX_OMAP_MUSB_HWMOD_NAME_LEN16
>> +struct omap_hwmod *oh_p =NULL;
>>
>> static struct musb_hdrc_config musb_config = {
>> .multipoint = 1,
>> @@ -59,6 +60,9 @@ static struct musb_hdrc_platform_data musb_plat = {
>> * "mode", and should be passed to usb_musb_init().
>> */
>> .power = 50, /* up to 100 mA */
>> +
>> +/* supports clock autogating */
>> +.clk_autogated = 1,
>
>This appears unrelated, and should be a separate patch.
>
>> };
>>
>> static u
RE: [PATCH 0/8 v2]usb: musb: hwmod and runtime pm support for musb
>-Original Message- >From: Kevin Hilman [mailto:khil...@deeprootsystems.com] >Sent: Thursday, August 26, 2010 5:29 AM >To: Kalliguddi, Hema >Cc: linux-omap@vger.kernel.org; linux-...@vger.kernel.org; >Felipe Balbi; Tony Lindgren; Cousson, Benoit; Paul Walmsley >Subject: Re: [PATCH 0/8 v2]usb: musb: hwmod and runtime pm >support for musb > >Hema HK writes: > >> Cc: Felipe Balbi >> Cc: Tony Lindgren >> Cc: Kevin Hilman >> Cc: Cousson, Benoit >> Cc: Paul Walmsley >> >> This patch series makes OMAP2PLUS musb Module implemented >> in HWMOD FW way. It also implements musb driver to >> use the runtime pm apis. >> >> PATCH[1/8 v2] and [PATCH 2/8 v2] are the pre-requisites for the hwmod >> support for the usb module. >> >> PATCH[6/8 v2] is core-offmode usb fix patch for the OMAP3 >> in idle path. >> >> As per the OMAP usbotg specification[1] musb sysconfig register >> has to be set to force idle and force standby when not used >> and set smart idle/standby during operation.otherwise core-off >> will be prevented by musb. >> >> [1]: http://focus.ti.com/pdfs/wtbu/SWPU223D_Final_EPDF_06_07_2010.pdf >> >> This patch series is created on "origin/pm-wip/hwmods-omap4". >> >> This patch series is tested on OMAP3630 zoom3 and OMAP4430 SDP. >> On OMAP3630 zoom3, off mode is tested on "origin/pm" branch using >> omap3_pm_defconfig. > >This IP is on OMAP2 as well, but I don't see any hwmods for OMAP2. > >Please also add hwmods for OMAP2 and test as well. > OK. I will do that and post a patch. >Thanks, > >Kevin > > >> >> Version History: >> --- >> Vesrion v2: >> >> Fixed review comments. >> Removed the omap_hwmod.h inclusion from musb.h file which was >> breaking the non-omap platform build. >> Using the runtime pm apis in the idle path(interrupts disabled). >> Added the omap4 hwmod data base. >> >> Version v1: >> initial version of the patch series. >> >> Some of the links for v1 >> >> >> http://www.spinics.net/lists/linux-usb/msg34570.html >> http://www.spinics.net/lists/linux-omap/msg34568.html >> http://www.spinics.net/lists/linux-usb/msg34544.html >> http://www.spinics.net/lists/linux-usb/msg34540.html >> http://www.spinics.net/lists/linux-usb/msg34589.html >> http://www.spinics.net/lists/linux-usb/msg34554.html >> http://www.mail-archive.com/linux-omap@vger.kernel.org/msg32973.html >> >> >> Cousson, Benoit (1): >> usb: musb: HWMOD database structures addition for OMAP4 >> >> Hema HK (7): >> usb: musb: Adding names for IRQs in resource structure >> usb: musb: Remove board_data parameter from musb_platform_init() >> usb: musb: HWMOD database structures addition for OMAP3 >> usb: musb: Using omap_device_build for musb device registration >> usb: musb: Offmode fix for idle path >> OMAP: Hwmod api changes >> usb : musb: Using runtime pm apis for musb. >> >> arch/arm/mach-davinci/usb.c |2 + >> arch/arm/mach-omap2/omap_hwmod.c | 18 ++- >> arch/arm/mach-omap2/omap_hwmod_3xxx_data.c| 100 + >> arch/arm/mach-omap2/omap_hwmod_44xx_data.c| 92 >> arch/arm/mach-omap2/pm34xx.c |9 ++ >> arch/arm/mach-omap2/usb-musb.c| 188 >- >> arch/arm/plat-omap/include/plat/omap_device.h |2 + >> arch/arm/plat-omap/include/plat/usb.h |9 ++ >> arch/arm/plat-omap/omap_device.c | 43 ++ >> arch/blackfin/mach-bf527/boards/cm_bf527.c|2 + >> arch/blackfin/mach-bf527/boards/ezbrd.c |2 + >> arch/blackfin/mach-bf527/boards/ezkit.c |2 + >> arch/blackfin/mach-bf548/boards/cm_bf548.c|2 + >> arch/blackfin/mach-bf548/boards/ezkit.c |2 + >> drivers/usb/musb/blackfin.c |2 +- >> drivers/usb/musb/cppi_dma.c |2 +- >> drivers/usb/musb/davinci.c|2 +- >> drivers/usb/musb/musb_core.c | 26 +++- >> drivers/usb/musb/musb_core.h |3 +- >> drivers/usb/musb/musbhsdma.c |2 +- >> drivers/usb/musb/omap2430.c | 87 ++-- >> drivers/usb/musb/tusb6010.c |2 +- >> include/linux/usb/musb.h | 20 +++ >> 23 files changed, 522 insertions(+), 97 deletions(-) >-- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 0/8 v2]usb: musb: hwmod and runtime pm support for musb
>-Original Message- >From: Kevin Hilman [mailto:khil...@deeprootsystems.com] >Sent: Thursday, August 26, 2010 5:29 AM >To: Kalliguddi, Hema >Cc: linux-omap@vger.kernel.org; linux-...@vger.kernel.org; >Felipe Balbi; Tony Lindgren; Cousson, Benoit; Paul Walmsley >Subject: Re: [PATCH 0/8 v2]usb: musb: hwmod and runtime pm >support for musb > >Hema HK writes: > >> Cc: Felipe Balbi >> Cc: Tony Lindgren >> Cc: Kevin Hilman >> Cc: Cousson, Benoit >> Cc: Paul Walmsley >> >> This patch series makes OMAP2PLUS musb Module implemented >> in HWMOD FW way. It also implements musb driver to >> use the runtime pm apis. >> >> PATCH[1/8 v2] and [PATCH 2/8 v2] are the pre-requisites for the hwmod >> support for the usb module. >> >> PATCH[6/8 v2] is core-offmode usb fix patch for the OMAP3 >> in idle path. >> >> As per the OMAP usbotg specification[1] musb sysconfig register >> has to be set to force idle and force standby when not used >> and set smart idle/standby during operation.otherwise core-off >> will be prevented by musb. >> >> [1]: http://focus.ti.com/pdfs/wtbu/SWPU223D_Final_EPDF_06_07_2010.pdf >> >> This patch series is created on "origin/pm-wip/hwmods-omap4". >> >> This patch series is tested on OMAP3630 zoom3 and OMAP4430 SDP. >> On OMAP3630 zoom3, off mode is tested on "origin/pm" branch using >> omap3_pm_defconfig. > >This IP is on OMAP2 as well, but I don't see any hwmods for OMAP2. > >Please also add hwmods for OMAP2 and test as well. > OK. I will do that and post a patch. >Thanks, > >Kevin > > >> >> Version History: >> --- >> Vesrion v2: >> >> Fixed review comments. >> Removed the omap_hwmod.h inclusion from musb.h file which was >> breaking the non-omap platform build. >> Using the runtime pm apis in the idle path(interrupts disabled). >> Added the omap4 hwmod data base. >> >> Version v1: >> initial version of the patch series. >> >> Some of the links for v1 >> >> >> http://www.spinics.net/lists/linux-usb/msg34570.html >> http://www.spinics.net/lists/linux-omap/msg34568.html >> http://www.spinics.net/lists/linux-usb/msg34544.html >> http://www.spinics.net/lists/linux-usb/msg34540.html >> http://www.spinics.net/lists/linux-usb/msg34589.html >> http://www.spinics.net/lists/linux-usb/msg34554.html >> http://www.mail-archive.com/linux-omap@vger.kernel.org/msg32973.html >> >> >> Cousson, Benoit (1): >> usb: musb: HWMOD database structures addition for OMAP4 >> >> Hema HK (7): >> usb: musb: Adding names for IRQs in resource structure >> usb: musb: Remove board_data parameter from musb_platform_init() >> usb: musb: HWMOD database structures addition for OMAP3 >> usb: musb: Using omap_device_build for musb device registration >> usb: musb: Offmode fix for idle path >> OMAP: Hwmod api changes >> usb : musb: Using runtime pm apis for musb. >> >> arch/arm/mach-davinci/usb.c |2 + >> arch/arm/mach-omap2/omap_hwmod.c | 18 ++- >> arch/arm/mach-omap2/omap_hwmod_3xxx_data.c| 100 + >> arch/arm/mach-omap2/omap_hwmod_44xx_data.c| 92 >> arch/arm/mach-omap2/pm34xx.c |9 ++ >> arch/arm/mach-omap2/usb-musb.c| 188 >- >> arch/arm/plat-omap/include/plat/omap_device.h |2 + >> arch/arm/plat-omap/include/plat/usb.h |9 ++ >> arch/arm/plat-omap/omap_device.c | 43 ++ >> arch/blackfin/mach-bf527/boards/cm_bf527.c|2 + >> arch/blackfin/mach-bf527/boards/ezbrd.c |2 + >> arch/blackfin/mach-bf527/boards/ezkit.c |2 + >> arch/blackfin/mach-bf548/boards/cm_bf548.c|2 + >> arch/blackfin/mach-bf548/boards/ezkit.c |2 + >> drivers/usb/musb/blackfin.c |2 +- >> drivers/usb/musb/cppi_dma.c |2 +- >> drivers/usb/musb/davinci.c|2 +- >> drivers/usb/musb/musb_core.c | 26 +++- >> drivers/usb/musb/musb_core.h |3 +- >> drivers/usb/musb/musbhsdma.c |2 +- >> drivers/usb/musb/omap2430.c | 87 ++-- >> drivers/usb/musb/tusb6010.c |2 +- >> include/linux/usb/musb.h | 20 +++ >> 23 files changed, 522 insertions(+), 97 deletions(-) >-- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 1/5] OMAP2420: McSPI: Add mcspi hwmod
Hi,
>-Original Message-
>From: linux-omap-ow...@vger.kernel.org
>[mailto:linux-omap-ow...@vger.kernel.org] On Behalf Of
>Varadarajan, Charulatha
>Sent: Friday, August 13, 2010 7:35 PM
>To: linux-omap@vger.kernel.org
>Cc: khil...@deeprootsystems.com; p...@pwsan.com; Cousson,
>Benoit; t...@atomide.com; grant.lik...@secretlab.ca;
>dbrown...@users.sourceforge.net;
>spi-devel-gene...@lists.sourceforge.net; Nayak, Rajendra;
>Basak, Partha; Varadarajan, Charulatha; Raja, Govindraj
>Subject: [PATCH 1/5] OMAP2420: McSPI: Add mcspi hwmod
>
>This patch updates the omap2420 hwmod data with the
>McSPI info.
>
>Signed-off-by: Charulatha V
>Signed-off-by: Partha Basak
>Signed-off-by: Govindraj.R
>---
> arch/arm/mach-omap2/omap_hwmod_2420_data.c | 138
>
> 1 files changed, 138 insertions(+), 0 deletions(-)
>
>diff --git a/arch/arm/mach-omap2/omap_hwmod_2420_data.c
>b/arch/arm/mach-omap2/omap_hwmod_2420_data.c
>index 3cc768e..7d1a0ff 100644
>--- a/arch/arm/mach-omap2/omap_hwmod_2420_data.c
>+++ b/arch/arm/mach-omap2/omap_hwmod_2420_data.c
>@@ -15,6 +15,7 @@
> #include
> #include
> #include
>+#include
>
> #include "omap_hwmod_common_data.h"
>
>@@ -33,6 +34,8 @@ static struct omap_hwmod omap2420_mpu_hwmod;
> static struct omap_hwmod omap2420_iva_hwmod;
> static struct omap_hwmod omap2420_l3_main_hwmod;
> static struct omap_hwmod omap2420_l4_core_hwmod;
>+static struct omap_hwmod omap2420_mcspi1_hwmod;
>+static struct omap_hwmod omap2420_mcspi2_hwmod;
>
> /* L3 -> L4_CORE interface */
> static struct omap_hwmod_ocp_if omap2420_l3_main__l4_core = {
>@@ -72,6 +75,42 @@ static struct omap_hwmod omap2420_l3_main_hwmod = {
>
> static struct omap_hwmod omap2420_l4_wkup_hwmod;
>
>+/* L4 CORE -> MCSPI1 interface */
>+static struct omap_hwmod_addr_space omap2420_mcspi1_addr_space[] = {
>+ {
>+ .pa_start = 0x48098000,
>+ .pa_end = 0x480980ff,
>+ .flags = ADDR_TYPE_RT,
>+ },
>+};
Align all of them to one tab..
>+
>+static struct omap_hwmod_ocp_if omap2420_l4_core__mcspi1 = {
>+ .master = &omap2420_l4_core_hwmod,
>+ .slave = &omap2420_mcspi1_hwmod,
>+ .clk= "mcspi1_ick",
>+ .addr = omap2420_mcspi1_addr_space,
>+ .addr_cnt = ARRAY_SIZE(omap2420_mcspi1_addr_space),
>+ .user = OCP_USER_MPU | OCP_USER_SDMA,
>+};
>+
>+/* L4 CORE -> MCSPI2 interface */
>+static struct omap_hwmod_addr_space omap2420_mcspi2_addr_space[] = {
>+ {
>+ .pa_start = 0x4809a000,
>+ .pa_end = 0x4809a0ff,
>+ .flags = ADDR_TYPE_RT,
>+ },
>+};
>+
Ditto align to one tab
>+static struct omap_hwmod_ocp_if omap2420_l4_core__mcspi2 = {
>+ .master = &omap2420_l4_core_hwmod,
>+ .slave = &omap2420_mcspi2_hwmod,
>+ .clk= "mcspi2_ick",
>+ .addr = omap2420_mcspi2_addr_space,
>+ .addr_cnt = ARRAY_SIZE(omap2420_mcspi2_addr_space),
>+ .user = OCP_USER_MPU | OCP_USER_SDMA,
>+};
>+
> /* L4_CORE -> L4_WKUP interface */
> static struct omap_hwmod_ocp_if omap2420_l4_core__l4_wkup = {
> .master = &omap2420_l4_core_hwmod,
>@@ -165,12 +204,111 @@ static struct omap_hwmod omap2420_iva_hwmod = {
> .omap_chip = OMAP_CHIP_INIT(CHIP_IS_OMAP2420)
> };
>
>+/* SPI common */
>+static struct omap_hwmod_class_sysconfig omap2420_mcspi_sysc = {
>+ .rev_offs = 0x,
>+ .sysc_offs = 0x0010,
>+ .syss_offs = 0x0014,
>+ .sysc_flags = (SYSC_HAS_CLOCKACTIVITY | SYSC_HAS_SIDLEMODE |
>+ SYSC_HAS_ENAWAKEUP |
>SYSC_HAS_SOFTRESET |
>+ SYSC_HAS_AUTOIDLE),
>+ .idlemodes = (SIDLE_FORCE | SIDLE_NO | SIDLE_SMART),
>+ .sysc_fields= &omap_hwmod_sysc_type1,
>+};
>+
>+static struct omap_hwmod_class omap2420_mcspi_class = {
>+ .name = "mcspi",
>+ .sysc = &omap2420_mcspi_sysc,
>+};
>+
>+/* SPI1 */
>+static struct omap_hwmod_irq_info omap2420_mcspi1_mpu_irqs[] = {
>+ { .irq = INT_24XX_SPI1_IRQ }, /* 65 */
>+};
>+
>+static struct omap_hwmod_dma_info omap2420_mcspi1_sdma_reqs[] = {
>+ { .name = "rx0", .dma_req = OMAP24XX_DMA_SPI1_RX0 }, /* 35 */
>+ { .name = "rx1", .dma_req = OMAP24XX_DMA_SPI1_RX1 }, /* 37 */
>+ { .name = "rx2", .dma_req = OMAP24XX_DMA_SPI1_RX2 }, /* 39 */
>+ { .name = "rx3", .dma_req = OMAP24XX_DMA_SPI1_RX3 }, /* 41 */
>+ { .name = "tx0", .dma_req = OMAP24XX_DMA_SPI1_TX0 }, /* 34 */
>+ { .name = "tx1", .dma_req = OMAP24XX_DMA_SPI1_TX1 }, /* 36 */
>+ { .name = "tx2", .dma_req = OMAP24XX_DMA_SPI1_TX2 }, /* 38 */
>+ { .name = "tx3", .dma_req = OMAP24XX_DMA_SPI1_TX3 }, /* 40 */
>+};
>+
>+static struct omap_hwmod_ocp_if *omap2420_mcspi1_slaves[] = {
>+ &omap2420_l4_core__mcspi1,
>+};
>+
>+static struct omap_hwmod omap2420_mcspi1_hwmod = {
>+ .name
RE: [PATCH] : Bug fix in generic runtime pm APIs
Hi,
>> -Original Message-
>> From: Kalliguddi, Hema
>> Sent: Friday, August 13, 2010 7:08 PM
>> To: Cousson, Benoit
>> Cc: linux-omap@vger.kernel.org; Tony Lindgren; Kevin Hilman;
>paul.walmsley[p...@pwsan.com]
>> Subject: RE: [PATCH] : Bug fix in generic runtime pm APIs
>>
>> Hi,
>>
>>> -Original Message-
>>> From: Cousson, Benoit
>>> Sent: Friday, August 13, 2010 6:29 PM
>>> To: Kalliguddi, Hema
>>> Cc: linux-omap@vger.kernel.org; Tony Lindgren; Kevin Hilman;
>>> paul.walmsley[p...@pwsan.com]
>>> Subject: Re: [PATCH] : Bug fix in generic runtime pm APIs
>>>
>>> Hi Hema,
>>>
>>> On 8/13/2010 2:31 PM, Kalliguddi, Hema wrote:
>>>> From: Hema HK
>>>>
>>>> pm_generic_runtime_resume/pm_generic_runtime_suspend api
>>> should return zero if
>>>> driver has not implemented runtime_suspend or runtime_resume apis.
>>>
>>> Why? Could you elaborate?
>>>
>> I don't have runtime_suspend and runtime_resume APIs
>implemented in the usb driver.
>> When runtime_pm_get_sync is called there is a -22 as return value.
>
>It is still not clear why it is related to the following patch?
>Can you explain in what context you observe that?
Sorry. It is not related o following commit. It is related to
http://git.kernel.org/?p=linux/kernel/git/khilman/linux-omap-pm.git;a=commit;h=1ebda92e47879750c739f6125ab11632a8f6f713
In this pm_generic_runtime_resume() API returns -EINVAL if there is no
runtime_resume and runtime_suspend APIs implemented by driver.
I observe this issue when I call runtime_pm_get_sync api in the driver and
check for the return value.
The return value is -22 because there is no runtime_resume API implemented in
the USB driver.
>
>> Is not it is valid if there is no runtime_suspend and
>runtime_resume implemented by drivers/device?
>> Why should this API returns -EINVAL if there is no
>runtime_resume and runtime_suspend APIs ?
>
>Good question, you should maybe ask the original author, who is Rafael.
>
Rafael,
Is it mandatory to implement the runtime_suspend and runtime_resume APIs in the
driver
even if there is nothing to done in the APIs?
>
>This patch is just adding platform_pm_suspend_noirq and
>platform_pm_resume_noirq custom implementation in order to use
>the same
>idle / enable as runtime PM.
>How it is related to your problem?
This is not related.
>
>>> This patch is changing some core driver code, so you should probably
>>> send it to Rafael / lkml / linux-pm.
>>>
>> I am not sure the above commit posted by Kevin already. This
>issue I am seeing on Kevin's pm-wip/hwmods-omap4 branch.
>
>That does not matter. The code you are trying to change is already in
>mainline kernel, so you just have to explain how you observe
>that issue.
Yes you are right. I will check with Rafael.
>
>Regards,
>Benoit
>
>>
>>> Regards,
>>> Benoit
>>>
>>>> Signed-off-by: Hema HK
>>>>
>>>> Cc: Tony Lindgren
>>>> Cc: Kevin Hilman
>>>> Cc: Cousson, Benoit
>>>> Cc: Paul Walmsley [p...@pwsan.com]
>>>> ---
>>>> Based of: Kevin's pm-wip/hwmods-omap4 branch
>>>>
>>>>drivers/base/power/generic_ops.c |8
>>>>1 file changed, 4 insertions(+), 4 deletions(-)
>>>>
>>>> Index: linux-omap-pm/drivers/base/power/generic_ops.c
>>>> ===
>>>> --- linux-omap-pm.orig/drivers/base/power/generic_ops.c
>>> 2010-08-13 08:05:54.705862674 -0400
>>>> +++ linux-omap-pm/drivers/base/power/generic_ops.c
>>> 2010-08-13 08:06:19.257924404 -0400
>>>> @@ -38,14 +38,14 @@
>>>> *
>>>> * If PM operations are defined for the @dev's driver and
>>> they include
>>>> * ->runtime_suspend(), execute it and return its error
>>> code. Otherwise,
>>>> - * return -EINVAL.
>>>> + * return Zero.
>>>> */
>>>>int pm_generic_runtime_suspend(struct device *dev)
>>>>{
>>>>const struct dev_pm_ops *pm = dev->driver ?
>>> dev->driver->pm : NULL;
>>>>int ret;
>>>>
>>>> - ret = pm&& pm->runtime_suspend ?
>>> pm->runtime_suspend(dev) : -EINVAL;
>>>> + ret = pm&& pm->runtime_suspend ? pm->runtime_suspend(dev) : 0;
>>>>
>>>>return ret;
>>>>}
>>>> @@ -57,14 +57,14 @@
>>>> *
>>>> * If PM operations are defined for the @dev's driver and
>>> they include
>>>> * ->runtime_resume(), execute it and return its error
>>> code. Otherwise,
>>>> - * return -EINVAL.
>>>> + * return Zero.
>>>> */
>>>>int pm_generic_runtime_resume(struct device *dev)
>>>>{
>>>>const struct dev_pm_ops *pm = dev->driver ?
>>> dev->driver->pm : NULL;
>>>>int ret;
>>>>
>>>> - ret = pm&& pm->runtime_resume ?
>>> pm->runtime_resume(dev) : -EINVAL;
>>>> + ret = pm&& pm->runtime_resume ? pm->runtime_resume(dev) : 0;
>>>>
>>>>return ret;
>>>>}
>>>
>>>
>
>--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 8/8 ]usb : musb:Using runtime pm apis for musb.
Hi, >-Original Message- >From: Sergei Shtylyov [mailto:sshtyl...@mvista.com] >Sent: Wednesday, August 11, 2010 5:05 PM >To: Kalliguddi, Hema >Cc: linux-...@vger.kernel.org; linux-omap@vger.kernel.org; >Basak, Partha; Felipe Balbi; Tony Lindgren; Kevin Hilman >Subject: Re: [PATCH 8/8 ]usb : musb:Using runtime pm apis for musb. > >Hello. > >I wrote: > >>> Calling runtime pm APIs pm_runtime_put_sync() and >>> pm_runtime_get_sync() for enabling/disabling the clocks,sysconfig >>> settings. > >>> used omap_hwmod_enable_wakeup & omap_hwmod_disable_wakeup apis to >>> set/clear >>> the wakeup enable bit. >>> Also need to put the USB in force standby and force idle >mode when usb >>> not used >>> and set it back to smart idle and smart stndby after wakeup. >>> these cases are handled using the oh flags. >>> For omap3430 auto idle bit has to be disabled because of >the errata.So >>> using HWMOD_NO_OCP_AUTOIDLE flag for OMAP3430. > >>> Signed-off-by: Hema HK >>> Signed-off-by: Basak, Partha > >>> Cc: Felipe Balbi >>> Cc: Tony Lindgren >>> Cc: Kevin Hilman > >[...] > >>> Index: linux-omap-pm/include/linux/usb/musb.h >>> === >>> --- linux-omap-pm.orig/include/linux/usb/musb.h2010-08-06 >>> 10:44:06.0 -0400 >>> +++ linux-omap-pm/include/linux/usb/musb.h2010-08-06 >>> 10:44:42.946115274 -0400 >>> @@ -10,6 +10,9 @@ >>> #ifndef __LINUX_USB_MUSB_H >>> #define __LINUX_USB_MUSB_H >>> >>> +#include > >>You could just use incomplete declaration of 'struct >platfrom_device' >> instead of this #include. > >>> +#include > >Did you think about the other platfroms where this file >doesn't exist? How >are they supposed to compile?! >I think this #include needs to be wrapped in #ifdef as well... Good point. I did not think of it. It need not be wrapped. Instead I am changing the Enable_wakeup/disable_wakeup parameter to platform_device instead of omap_device. So that I can avoid these includes. > >>> + >>> /* The USB role is defined by the connector used on the board, so >>> long as >>> * standards are being followed. (Developer boards >sometimes won't.) >>> */ >>> @@ -129,6 +132,21 @@ >>> >>> /* check usb device active state*/ >>> int(*is_usb_active)(struct device *dev); >>> + >>> +/* omap hwmod data structure */ >>> +structomap_hwmod *oh; > >>This should be wrapped into #ifdef, don't you think? > >>> +/* enable clocks and set sysconfig register*/ >>> +int(*device_enable)(struct platform_device *pdev); >>> + >>> +/* Disable clock and reset the sysconfig register settings*/ >>> +int(*device_idle)(struct platform_device *pdev); >>> + >>> +/* set the enable wakeup bit of sysconfig register */ >>> +int(*enable_wakeup)(struct omap_device *od); >>> + >>> +/* Clear the enable wakeup bit of sysconfig register */ >>> +int(*disable_wakeup)(struct omap_device *od); > >>This should be wrapped too, I think... It will be taken care. > >WBR, Sergei > >-- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 7/8] : Hwmod api changes
Hi,
>-Original Message-
>From: Cousson, Benoit
>Sent: Monday, August 09, 2010 5:42 PM
>To: Kalliguddi, Hema
>Cc: linux-...@vger.kernel.org; linux-omap@vger.kernel.org;
>Basak, Partha; Felipe Balbi; Tony Lindgren; Kevin Hilman
>Subject: Re: [PATCH 7/8] : Hwmod api changes
>
>On 8/6/2010 7:28 PM, Kalliguddi, Hema wrote:
>> From: Hema HK
>>
>> Omap USBOTG modules has a requirement to set the auto idle
>bit only after
>> setting smart idle bit.
>
>Is it a requirement or an errata? Could you provide more information
>(i.e. TRM page or errata number / description)?
>
This is a requirement as per the TRM section 24.1.5.4.4
http://focus.ti.com/pdfs/wtbu/SWPU223D_Final_EPDF_06_07_2010.pdf
There is note on this.
>> Modified the _sys_enable api to set the smart idle
>> first and then the autoidle bit. Setting this will not have
>any impact on the
>> other modules.
>>
>> Added 2 wrapper APIs in the omap device layer for wakeup
>enable/disable
>> and sidle/mstandby settings.
>>
>> Signed-off-by: Hema HK
>> Signed-off-by: Basak, Partha
>>
>> Cc: Felipe Balbi
>> Cc: Tony Lindgren
>> Cc: Kevin Hilman
>>
>> ---
>> arch/arm/mach-omap2/omap_hwmod.c | 18 +++
>> arch/arm/plat-omap/include/plat/omap_device.h |2 +
>> arch/arm/plat-omap/omap_device.c | 42
>++
>> 3 files changed, 56 insertions(+), 6 deletions(-)
>>
>> Index: linux-omap-pm/arch/arm/mach-omap2/omap_hwmod.c
>> ===
>> --- linux-omap-pm.orig/arch/arm/mach-omap2/omap_hwmod.c
>2010-08-06 08:59:03.641863815 -0400
>> +++ linux-omap-pm/arch/arm/mach-omap2/omap_hwmod.c
>2010-08-06 09:02:00.021864999 -0400
>> @@ -653,12 +653,6 @@
>> _set_master_standbymode(oh, idlemode,&v);
>> }
>>
>> -if (sf& SYSC_HAS_AUTOIDLE) {
>> -idlemode = (oh->flags& HWMOD_NO_OCP_AUTOIDLE) ?
>> -0 : 1;
>> -_set_module_autoidle(oh, idlemode,&v);
>> -}
>> -
>> /* XXX OCP ENAWAKEUP bit? */
>>
>> /*
>> @@ -671,6 +665,18 @@
>> _set_clockactivity(oh, oh->class->sysc->clockact,&v);
>>
>> _write_sysconfig(v, oh);
>> +
>> +/* Set the auto idle bit only after setting the smartidle bit
>> + * as this is requirement for some modules like USBOTG
>> + * setting this will not have any impact on the other modues.
>> + */
>
>Except that you are adding an extra access to a quite slow L4 slave
>interface. I'm not sure if write posted will help in that case.
>
I agree that it will add an extra access on L4 but this required for USBOTG.
I did not quite understood your Next Comment can elaborate please?
>> +
>> +if (sf& SYSC_HAS_AUTOIDLE) {
>> +idlemode = (oh->flags& HWMOD_NO_OCP_AUTOIDLE) ?
>> +0 : 1;
>> +_set_module_autoidle(oh, idlemode,&v);
>> +}
>> +_write_sysconfig(v, oh);
>> }
>>
>> /**
>> Index: linux-omap-pm/arch/arm/plat-omap/include/plat/omap_device.h
>> ===
>> ---
>linux-omap-pm.orig/arch/arm/plat-omap/include/plat/omap_d
>evice.h2010-08-06 08:59:03.661863725 -0400
>> +++
>linux-omap-pm/arch/arm/plat-omap/include/plat/omap_device.h
>2010-08-06 09:02:00.021864999 -0400
>> @@ -116,6 +116,8 @@
>> int omap_device_disable_clocks(struct omap_device *od);
>> int omap_device_enable_clocks(struct omap_device *od);
>>
>> +int omap_device_enable_wakeup(struct omap_device *od);
>> +int omap_device_disable_wakeup(struct omap_device *od);
>>
>> /*
>>* Entries should be kept in latency order ascending
>> Index: linux-omap-pm/arch/arm/plat-omap/omap_device.c
>> ===
>> --- linux-omap-pm.orig/arch/arm/plat-omap/omap_device.c
>2010-08-06 08:59:03.661863725 -0400
>> +++ linux-omap-pm/arch/arm/plat-omap/omap_device.c
>2010-08-06 09:02:00.021864999 -0400
>> @@ -757,3 +757,45 @@
>> /* XXX pass along return value here? */
>> return 0;
>> }
>> +
>> +/**
>> + * omap_device_enable_wakeup - Enable the wakeup bit
>> + * @od: struct omap_device *od
>> + *
>> + * Enable the wakup bit for omap_hwmods associated
>> + * with the omap_device. Returns 0.
>> + */
>> +
>> +int omap_device_enable_wakeup(struct omap_device *od)
>
>Why do you need that? Could you elaborate?
>
This required o set the wakeup enable bit in the sysconfig register for swakeup
generation
To enable the OCP clock When module is in smart-idle-mode and there is a
asyncronous events like resume signalling.
>Benoit
>
>--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH V2 6/8] usb: musb: Offmode fix for idle path
Hi, >-Original Message- >From: Sergei Shtylyov [mailto:sshtyl...@mvista.com] >Sent: Tuesday, August 10, 2010 4:50 PM >To: Kalliguddi, Hema >Cc: Sergei Shtylyov; linux-...@vger.kernel.org; >linux-omap@vger.kernel.org; Mankad, Maulik Ojas; Felipe Balbi; >Tony Lindgren; Kevin Hilman >Subject: Re: [PATCH V2 6/8] usb: musb: Offmode fix for idle path > >Hello. > >Kalliguddi, Hema wrote: > >>>> With OMAP core-off support musb was not functional as >context was getting >>>> lost after wakeup from core-off. And also musb was >blocking the core-off >>>> after loading the gadget driver even with no cable >connected sometimes. > >>>> Added the conext save/restore api in the platform layer which will >>>> be called in the idle and wakeup path. >>>> Changed the usb sysconfig settings as per the usbotg >functional spec. >>>> When the device is not active, configure to force idle and >force standby mode. >>>> When it is being used, configure in smart standby and >smart idle mode. >>>> So while attempting to coreoff the usb is configured to >force standby and >>>> force idle mode, after wakeup configured in smart idle and >smart standby. > >>>> Signed-off-by: Hema HK >>>> Signed-off-by: Maulik Mankad >>>> Cc: Felipe Balbi >>>> Cc: Tony Lindgren >>>> Cc: Kevin Hilman > >>> [...] > >>>> Index: linux-omap-pm/drivers/usb/musb/musb_core.c >>>> === >>>> --- linux-omap-pm.orig/drivers/usb/musb/musb_core.c >>> 2010-08-06 09:24:21.069863329 -0400 >>>> +++ linux-omap-pm/drivers/usb/musb/musb_core.c >>> 2010-08-06 10:44:06.369863527 -0400 >>>> @@ -2427,11 +2427,6 @@ >>>>} >>>> >>>>musb_save_context(musb); >>>> - >>>> - if (musb->set_clock) >>>> - musb->set_clock(musb->clock, 0); >>>> - else >>>> - clk_disable(musb->clock); >>>>spin_unlock_irqrestore(&musb->lock, flags); >>>>return 0; >>>> } >>>> @@ -2443,12 +2438,6 @@ >>>> >>>>if (!musb->clock) >>>>return 0; >>>> - >>>> - if (musb->set_clock) >>>> - musb->set_clock(musb->clock, 1); >>>> - else >>>> - clk_enable(musb->clock); >>>> - >>>>musb_restore_context(musb); > >>> The same question again: are you sure that clocks are >auto-gated on >>> non-OMAP platfroms? > >> Bon Lui confirmed that there is no clock for blackfin. > >What about DaVinci (and the coming DA8xx)? > I checked with Ajay, it is not auto gated for for Davinci so I can't just remove this. I am thinking of adding a member variable(clk_autogated) in them usb_hdrc_platform_data structure and check this in the musb_suspend() And musb_resume() APIs. Do you see any problem with it? Regards, Hema >WBR, Sergei > >-- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH V2 6/8] usb: musb: Offmode fix for idle path
>-Original Message-
>From: DebBarma, Tarun Kanti
>Sent: Monday, August 09, 2010 10:14 PM
>To: Kalliguddi, Hema; linux-...@vger.kernel.org;
>linux-omap@vger.kernel.org
>Cc: Kalliguddi, Hema; Mankad, Maulik Ojas; Felipe Balbi; Tony
>Lindgren; Kevin Hilman
>Subject: RE: [PATCH V2 6/8] usb: musb: Offmode fix for idle path
>
>Hema,
>
>> -Original Message-
>> From: linux-omap-ow...@vger.kernel.org [mailto:linux-omap-
>> ow...@vger.kernel.org] On Behalf Of Hema HK
>> Sent: Friday, August 06, 2010 10:57 PM
>> To: linux-...@vger.kernel.org; linux-omap@vger.kernel.org
>> Cc: Kalliguddi, Hema; Mankad, Maulik Ojas; Felipe Balbi;
>Tony Lindgren;
>> Kevin Hilman
>> Subject: [PATCH V2 6/8] usb: musb: Offmode fix for idle path
>>
>> From: Hema HK
>>
>> With OMAP core-off support musb was not functional as
>context was getting
>> lost after wakeup from core-off. And also musb was blocking
>the core-off
>> after loading the gadget driver even with no cable connected
>sometimes.
>>
>> Added the conext save/restore api in the platform layer which will
>> be called in the idle and wakeup path.
>>
>> Changed the usb sysconfig settings as per the usbotg functional spec.
>> When the device is not active, configure to force idle and
>force standby
>> mode.
>> When it is being used, configure in smart standby and smart
>idle mode.
>> So while attempting to coreoff the usb is configured to
>force standby and
>> force idle mode, after wakeup configured in smart idle and
>smart standby.
>>
>> Signed-off-by: Hema HK
>> Signed-off-by: Maulik Mankad
>>
>> Cc: Felipe Balbi
>> Cc: Tony Lindgren
>> Cc: Kevin Hilman
>> ---
>>
>> arch/arm/mach-omap2/pm34xx.c |4 ++
>> arch/arm/mach-omap2/usb-musb.c| 21 ++
>> arch/arm/plat-omap/include/plat/usb.h |2 +
>> drivers/usb/musb/musb_core.c | 11 ---
>> drivers/usb/musb/omap2430.c | 48
>> +++---
>> 5 files changed, 71 insertions(+), 15 deletions(-)
>>
>> Index: linux-omap-pm/arch/arm/mach-omap2/pm34xx.c
>> ===
>> --- linux-omap-pm.orig/arch/arm/mach-omap2/pm34xx.c 2010-08-06
>> 09:23:01.153862710 -0400
>> +++ linux-omap-pm/arch/arm/mach-omap2/pm34xx.c 2010-08-06
>> 10:44:06.393863125 -0400
>> @@ -39,6 +39,7 @@
>> #include
>> #include
>> #include
>> +#include
>>
>> #include
>>
>> @@ -416,6 +417,8 @@
>> if (core_next_state == PWRDM_POWER_OFF) {
>> omap3_core_save_context();
>> omap3_prcm_save_context();
>> +/* Save MUSB context */
>> +musb_context_save_restore(1);
>> }
>> }
>>
>> @@ -458,6 +461,8 @@
>> omap3_prcm_restore_context();
>> omap3_sram_restore_context();
>> omap2_sms_restore_context();
>> +/* restore MUSB context */
>> +musb_context_save_restore(0);
>> }
>> omap_uart_resume_idle(0);
>> omap_uart_resume_idle(1);
>> Index: linux-omap-pm/arch/arm/mach-omap2/usb-musb.c
>> ===
>> --- linux-omap-pm.orig/arch/arm/mach-omap2/usb-musb.c
>2010-08-06
>> 09:24:23.690112596 -0400
>> +++ linux-omap-pm/arch/arm/mach-omap2/usb-musb.c 2010-08-06
>> 10:44:06.385862697 -0400
>> @@ -120,6 +120,27 @@
>> }
>> }
>>
>> +void musb_context_save_restore(int save)
>> +{
>> +struct omap_hwmod *oh = omap_hwmod_lookup("usb_otg_hs");
>Might be good idea to check (oh) before proceeding?
>if (!oh) {
> /* error message */
> return;
>}
You are right. I will put a check.
>
>> +struct omap_device *od = oh->od;
>> +struct platform_device *pdev = &od->pdev;
>> +struct device *dev = &pdev->dev;
>> +struct device_driver *drv = dev->driver;
>> +
>> +if (drv) {
>> +struct musb_hdrc_platform_data *pdata =
>dev->platform_data;
>> +const struct dev_pm_ops *pm = drv->pm;
>> +if (!pdata->is_usb_active(dev)) {
>> +
>> +if (save)
>> +pm->suspend(dev);
>> +
RE: [PATCH V2 6/8] usb: musb: Offmode fix for idle path
Hi,
>-Original Message-
>From: Sergei Shtylyov [mailto:sshtyl...@mvista.com]
>Sent: Saturday, August 07, 2010 8:55 PM
>To: Kalliguddi, Hema
>Cc: linux-...@vger.kernel.org; linux-omap@vger.kernel.org;
>Mankad, Maulik Ojas; Felipe Balbi; Tony Lindgren; Kevin Hilman
>Subject: Re: [PATCH V2 6/8] usb: musb: Offmode fix for idle path
>
>Hello.
>
>Hema HK wrote:
>
>> With OMAP core-off support musb was not functional as
>context was getting
>> lost after wakeup from core-off. And also musb was blocking
>the core-off
>> after loading the gadget driver even with no cable connected
>sometimes.
>
>> Added the conext save/restore api in the platform layer which will
>> be called in the idle and wakeup path.
>
>> Changed the usb sysconfig settings as per the usbotg functional spec.
>> When the device is not active, configure to force idle and
>force standby mode.
>> When it is being used, configure in smart standby and smart
>idle mode.
>> So while attempting to coreoff the usb is configured to
>force standby and
>> force idle mode, after wakeup configured in smart idle and
>smart standby.
>
>> Signed-off-by: Hema HK
>> Signed-off-by: Maulik Mankad
>
>> Cc: Felipe Balbi
>> Cc: Tony Lindgren
>> Cc: Kevin Hilman
>
>[...]
>
>> Index: linux-omap-pm/arch/arm/mach-omap2/usb-musb.c
>> ===
>> --- linux-omap-pm.orig/arch/arm/mach-omap2/usb-musb.c
>2010-08-06 09:24:23.690112596 -0400
>> +++ linux-omap-pm/arch/arm/mach-omap2/usb-musb.c
>2010-08-06 10:44:06.385862697 -0400
>> @@ -120,6 +120,27 @@
>> }
>> }
>>
>> +void musb_context_save_restore(int save)
>> +{
>> +struct omap_hwmod *oh = omap_hwmod_lookup("usb_otg_hs");
>> +struct omap_device *od = oh->od;
>> +struct platform_device *pdev = &od->pdev;
>> +struct device *dev = &pdev->dev;
>> +struct device_driver *drv = dev->driver;
>> +
>> +if (drv) {
>> +struct musb_hdrc_platform_data *pdata =
>dev->platform_data;
>> +const struct dev_pm_ops *pm = drv->pm;
>
>Should be an empty line after the declaration block...
Ok.
>
>> +if (!pdata->is_usb_active(dev)) {
>> +
>> +if (save)
>> +pm->suspend(dev);
>> +else
>> +pm->resume_noirq(dev);
>> +}
>> +}
>> +}
>> +
>> #else
>> void __init usb_musb_init(struct omap_musb_board_data *board_data)
>> {
>> Index: linux-omap-pm/drivers/usb/musb/musb_core.c
>> ===
>> --- linux-omap-pm.orig/drivers/usb/musb/musb_core.c
>2010-08-06 09:24:21.069863329 -0400
>> +++ linux-omap-pm/drivers/usb/musb/musb_core.c
>2010-08-06 10:44:06.369863527 -0400
>> @@ -2427,11 +2427,6 @@
>> }
>>
>> musb_save_context(musb);
>> -
>> -if (musb->set_clock)
>> -musb->set_clock(musb->clock, 0);
>> -else
>> -clk_disable(musb->clock);
>> spin_unlock_irqrestore(&musb->lock, flags);
>> return 0;
>> }
>> @@ -2443,12 +2438,6 @@
>>
>> if (!musb->clock)
>> return 0;
>> -
>> -if (musb->set_clock)
>> -musb->set_clock(musb->clock, 1);
>> -else
>> -clk_enable(musb->clock);
>> -
>> musb_restore_context(musb);
>
> The same question again: are you sure that clocks are auto-gated on
>non-OMAP platfroms?
Bon Lui confirmed that there is no clock for blackfin.
>
>> Index: linux-omap-pm/drivers/usb/musb/omap2430.c
>> ===
>> --- linux-omap-pm.orig/drivers/usb/musb/omap2430.c
>2010-08-06 09:24:21.069863329 -0400
>> +++ linux-omap-pm/drivers/usb/musb/omap2430.c
>2010-08-06 10:44:30.093914217 -0400
>[...]
>> @@ -259,15 +273,42 @@
>> void musb_platform_save_context(struct musb *musb,
>> struct musb_context_registers *musb_context)
>> {
>> -musb_context->otg_sysconfig = musb_readl(musb->mregs,
>OTG_SYSCONFIG);
>> -musb_context->otg_forcestandby =
>musb_readl(musb->mregs, OTG_FORCESTDBY);
>
>If you're not saving the registers, you should remove the
>corresponding
>fields from 'struct musb_cont
RE: [PATCH V2 6/8] usb: musb: Offmode fix for idle path
>-Original Message-
>From: Bob Liu [mailto:lliu...@gmail.com]
>Sent: Saturday, August 07, 2010 7:54 AM
>To: Kalliguddi, Hema
>Cc: linux-...@vger.kernel.org; linux-omap@vger.kernel.org;
>Mankad, Maulik Ojas; Felipe Balbi; Tony Lindgren; Kevin Hilman
>Subject: Re: [PATCH V2 6/8] usb: musb: Offmode fix for idle path
>
>On Sat, Aug 7, 2010 at 1:27 AM, Hema HK wrote:
>> From: Hema HK
>>
>> With OMAP core-off support musb was not functional as
>context was getting
>> lost after wakeup from core-off. And also musb was blocking
>the core-off
>> after loading the gadget driver even with no cable connected
>sometimes.
>>
>> Added the conext save/restore api in the platform layer which will
>> be called in the idle and wakeup path.
>>
>> Changed the usb sysconfig settings as per the usbotg functional spec.
>> When the device is not active, configure to force idle and
>force standby mode.
>> When it is being used, configure in smart standby and smart
>idle mode.
>> So while attempting to coreoff the usb is configured to
>force standby and
>> force idle mode, after wakeup configured in smart idle and
>smart standby.
>>
>> Signed-off-by: Hema HK
>> Signed-off-by: Maulik Mankad
>>
>> Cc: Felipe Balbi
>> Cc: Tony Lindgren
>> Cc: Kevin Hilman
>> ---
>>
>> arch/arm/mach-omap2/pm34xx.c | 4 ++
>> arch/arm/mach-omap2/usb-musb.c | 21 ++
>> arch/arm/plat-omap/include/plat/usb.h | 2 +
>> drivers/usb/musb/musb_core.c | 11 ---
>> drivers/usb/musb/omap2430.c | 48
>+++---
>> 5 files changed, 71 insertions(+), 15 deletions(-)
>>
>> Index: linux-omap-pm/arch/arm/mach-omap2/pm34xx.c
>> ===
>> --- linux-omap-pm.orig/arch/arm/mach-omap2/pm34xx.c
>2010-08-06 09:23:01.153862710 -0400
>> +++ linux-omap-pm/arch/arm/mach-omap2/pm34xx.c 2010-08-06
>10:44:06.393863125 -0400
>> @@ -39,6 +39,7 @@
>> #include
>> #include
>> #include
>> +#include
>>
>> #include
>>
>> @@ -416,6 +417,8 @@
>> if (core_next_state == PWRDM_POWER_OFF) {
>> omap3_core_save_context();
>> omap3_prcm_save_context();
>> + /* Save MUSB context */
>> + musb_context_save_restore(1);
>> }
>> }
>>
>> @@ -458,6 +461,8 @@
>> omap3_prcm_restore_context();
>> omap3_sram_restore_context();
>> omap2_sms_restore_context();
>> + /* restore MUSB context */
>> + musb_context_save_restore(0);
>> }
>> omap_uart_resume_idle(0);
>> omap_uart_resume_idle(1);
>> Index: linux-omap-pm/arch/arm/mach-omap2/usb-musb.c
>> ===
>> --- linux-omap-pm.orig/arch/arm/mach-omap2/usb-musb.c
>2010-08-06 09:24:23.690112596 -0400
>> +++ linux-omap-pm/arch/arm/mach-omap2/usb-musb.c
>2010-08-06 10:44:06.385862697 -0400
>> @@ -120,6 +120,27 @@
>> }
>> }
>>
>> +void musb_context_save_restore(int save)
>> +{
>> + struct omap_hwmod *oh = omap_hwmod_lookup("usb_otg_hs");
>> + struct omap_device *od = oh->od;
>> + struct platform_device *pdev = &od->pdev;
>> + struct device *dev = &pdev->dev;
>> + struct device_driver *drv = dev->driver;
>> +
>> + if (drv) {
>> + struct musb_hdrc_platform_data *pdata =
>dev->platform_data;
>> + const struct dev_pm_ops *pm = drv->pm;
>> + if (!pdata->is_usb_active(dev)) {
>> +
>> + if (save)
>> + pm->suspend(dev);
>> + else
>> + pm->resume_noirq(dev);
>> + }
>> + }
>> +}
>> +
>> #else
>> void __init usb_musb_init(struct omap_musb_board_data *board_data)
>> {
>> Index: linux-omap-pm/arch/arm/plat-omap/include/plat/usb.h
>> ===
>> --- linux-omap-pm.orig/arch/arm/plat-omap/include/plat/usb.h
> 2010-08-06 09:23:01.137862514 -0400
>> +++ linux-omap-pm/arch/arm/plat-oma
RE: [PATCH 1/8] usb: musb: Adding names for IRQs in resource structure
Hi,
>-Original Message-
>From: Kevin Hilman [mailto:khil...@deeprootsystems.com]
>Sent: Tuesday, August 10, 2010 2:24 AM
>To: Kalliguddi, Hema
>Cc: linux-omap@vger.kernel.org; linux-...@vger.kernel.org;
>Felipe Balbi; Tony Lindgren
>Subject: Re: [PATCH 1/8] usb: musb: Adding names for IRQs in
>resource structure
>
>Hema HK writes:
>
>> From: Hema HK
>>
>> Modified the Omap,Blackfin and Davinci board files to add
>the name of the IRQs
>> in the resource structures and musb driver to use the
>get_irq_byname() api to
>> get the mc and dma irq numbers instead of using the index as
>the order of
>> resource definition need not be always in order of device
>interrupt and
>> then dma interrupt
>>
>> Signed-off-by: Hema HK
>> Cc: Felipe Balbi
>> Cc: Tony Lindgren
>> Cc: Kevin Hilman
>> ---
>>
>> Based off omap4-next branch.
>
>In patch 0/8 you say this is based on pm-wip/hwmods-omap4 branch. Here
>you say thisis omap4-next branch (presumably in Santosh's tree.)
>
>Otherwise, this change looks right.
It is a mistake. I will correct it.
>
>Kevin
>
>> arch/arm/mach-davinci/usb.c|2 ++
>> arch/arm/mach-omap2/usb-musb.c |2 ++
>> arch/blackfin/mach-bf527/boards/cm_bf527.c |2 ++
>> arch/blackfin/mach-bf527/boards/ezbrd.c|2 ++
>> arch/blackfin/mach-bf527/boards/ezkit.c|2 ++
>> arch/blackfin/mach-bf548/boards/cm_bf548.c |2 ++
>> arch/blackfin/mach-bf548/boards/ezkit.c|2 ++
>> drivers/usb/musb/cppi_dma.c|2 +-
>> drivers/usb/musb/musb_core.c |2 +-
>> drivers/usb/musb/musbhsdma.c |2 +-
>> 10 files changed, 17 insertions(+), 3 deletions(-)
>>
>> Index: linux-omap-pm/arch/arm/mach-davinci/usb.c
>> ===
>> --- linux-omap-pm.orig/arch/arm/mach-davinci/usb.c
>2010-08-06 09:01:23.605862579 -0400
>> +++ linux-omap-pm/arch/arm/mach-davinci/usb.c
>2010-08-06 09:01:25.526112352 -0400
>> @@ -64,10 +64,12 @@
>> {
>> .start = IRQ_USBINT,
>> .flags = IORESOURCE_IRQ,
>> +.name = "mc"
>> },
>> {
>> /* placeholder for the dedicated CPPI IRQ */
>> .flags = IORESOURCE_IRQ,
>> +.name = "dma"
>> },
>> };
>>
>> Index: linux-omap-pm/arch/arm/mach-omap2/usb-musb.c
>> ===
>> --- linux-omap-pm.orig/arch/arm/mach-omap2/usb-musb.c
>2010-08-06 09:01:23.613862415 -0400
>> +++ linux-omap-pm/arch/arm/mach-omap2/usb-musb.c
>2010-08-06 09:01:25.526112352 -0400
>> @@ -39,10 +39,12 @@
>> [1] = { /* general IRQ */
>> .start = INT_243X_HS_USB_MC,
>> .flags = IORESOURCE_IRQ,
>> +.name = "mc",
>> },
>> [2] = { /* DMA IRQ */
>> .start = INT_243X_HS_USB_DMA,
>> .flags = IORESOURCE_IRQ,
>> +.name = "dma",
>> },
>> };
>>
>> Index: linux-omap-pm/arch/blackfin/mach-bf527/boards/cm_bf527.c
>> ===
>> ---
>linux-omap-pm.orig/arch/blackfin/mach-bf527/boards/cm_bf527.c
>2010-08-06 09:01:23.645862783 -0400
>> +++ linux-omap-pm/arch/blackfin/mach-bf527/boards/cm_bf527.c
>2010-08-06 09:01:25.526112352 -0400
>> @@ -82,11 +82,13 @@
>> .start = IRQ_USB_INT0,
>> .end= IRQ_USB_INT0,
>> .flags = IORESOURCE_IRQ | IORESOURCE_IRQ_HIGHLEVEL,
>> +.name = "mc"
>> },
>> [2] = { /* DMA IRQ */
>> .start = IRQ_USB_DMA,
>> .end= IRQ_USB_DMA,
>> .flags = IORESOURCE_IRQ | IORESOURCE_IRQ_HIGHLEVEL,
>> +.name = "dma"
>> },
>> };
>>
>> Index: linux-omap-pm/arch/blackfin/mach-bf527/boards/ezbrd.c
>> ===
>> ---
>linux-omap-pm.orig/arch/blackfin/mach-bf527/boards/ezbrd.c
>2010-08-06 09:01:23.637862922 -0400
>> +++ linux-omap-pm/arch/blackfin/mach-bf527/boards/ezbrd.c
>2010-08-06 09:01:25.526112352 -0400
>> @@ -46,11 +46,13 @@
>> .start = IRQ_USB_INT0,
>> .end= IRQ_
RE: [PATCH 4/8]usb: musb: HWMOD database structures fixes OMAP4
Hi,
>-Original Message-
>From: Cousson, Benoit
>Sent: Monday, August 09, 2010 5:22 PM
>To: Kalliguddi, Hema
>Cc: linux-...@vger.kernel.org; linux-omap@vger.kernel.org;
>Felipe Balbi; Tony Lindgren; Kevin Hilman
>Subject: Re: [PATCH 4/8]usb: musb: HWMOD database structures
>fixes OMAP4
>
>Hi Hema,
>
>On 8/6/2010 5:57 PM, Kalliguddi, Hema wrote:
>> From: Hema HK
>>
>> Fixed the missing sysc settings for OMAP4 and enabled the OMAP4
>> hwmod data structure.
>>
>> Signed-off-by: Hema HK
>> Cc: Felipe Balbi
>> Cc: Tony Lindgren
>> Cc: Kevin Hilman
>
>It is a good practice, if not mandatory, to CC the authors of the file
>you are modifying with your patch.
>Neither Paul, nor myself are in CC of this patch. Could you please add
>us to this one and the other ones when applicable?
It is mistake of not CCing the owner. I will take care of it.
>
>> ---
>>
>> Index: linux-omap-pm/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
>> ===
>> ---
>linux-omap-pm.orig/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
>2010-08-06 08:31:45.885868560 -0400
>> +++ linux-omap-pm/arch/arm/mach-omap2/omap_hwmod_44xx_data.c
>2010-08-06 08:35:41.250112281 -0400
>> @@ -4516,8 +4516,15 @@
>>*/
>>
>> static struct omap_hwmod_class_sysconfig
>omap44xx_usb_otg_hs_sysc = {
>> -.sysc_flags = SYSS_MISSING,
>> -.idlemodes = (SIDLE_FORCE | SIDLE_NO | SIDLE_SMART),
>> +
>> +.rev_offs = 0x0400,
>> +.sysc_offs = 0x0404,
>> +.syss_offs = 0x0408,
>> +.sysc_flags = SYSC_HAS_SIDLEMODE | SYSC_HAS_MIDLEMODE|
>> + SYSC_HAS_ENAWAKEUP | SYSC_HAS_SOFTRESET |
>> + SYSC_HAS_AUTOIDLE,
>> +.idlemodes = SIDLE_FORCE | SIDLE_NO | SIDLE_SMART,
>> +.sysc_fields=&omap_hwmod_sysc_type1,
>> };
>
>This part if fine except the missing MIDLE_XXX modes. Here is the
>modified version using the same convention as other modules:
OK. I will add it.
>
> > static struct omap_hwmod_class_sysconfig
>omap44xx_usb_otg_hs_sysc = {
> > - .sysc_flags = SYSS_MISSING,
> > - .idlemodes = (SIDLE_FORCE | SIDLE_NO | SIDLE_SMART),
> > + .rev_offs = 0x0400,
> > + .sysc_offs = 0x0404,
> > + .syss_offs = 0x0408,
> > + .sysc_flags = (SYSC_HAS_AUTOIDLE | SYSC_HAS_ENAWAKEUP |
> > + SYSC_HAS_MIDLEMODE | SYSC_HAS_SIDLEMODE |
> > + SYSC_HAS_SOFTRESET),
> > + .idlemodes = (SIDLE_FORCE | SIDLE_NO | SIDLE_SMART |
> > + MSTANDBY_FORCE | MSTANDBY_NO |
>MSTANDBY_SMART),
> > + .sysc_fields= &omap_hwmod_sysc_type1,
> > };
>
>I don't have any preference for the parens, but in order to be
>consistent with the already existing hwmods, let's keep them.
There was comment from Sergie to remove the parens for omap3 database. So I
have removed.
>
>Regards,
>Benoit
>--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 8/8 ]usb : musb:Using runtime pm apis for musb.
Hi,
>-Original Message-
>From: Cousson, Benoit
>Sent: Monday, August 09, 2010 7:37 PM
>To: Kalliguddi, Hema
>Cc: linux-...@vger.kernel.org; linux-omap@vger.kernel.org;
>Basak, Partha; Felipe Balbi; Tony Lindgren; Kevin Hilman
>Subject: Re: [PATCH 8/8 ]usb : musb:Using runtime pm apis for musb.
>
>On 8/6/2010 7:29 PM, Hema HK wrote:
>> From: Hema HK
>>
>> Calling runtime pm APIs pm_runtime_put_sync() and
>pm_runtime_get_sync()
>> for enabling/disabling the clocks,sysconfig settings.
>>
>> used omap_hwmod_enable_wakeup& omap_hwmod_disable_wakeup
>apis to set/clear
>> the wakeup enable bit.
>> Also need to put the USB in force standby and force idle
>mode when usb not used
>> and set it back to smart idle and smart stndby after wakeup.
>> these cases are handled using the oh flags.
>> For omap3430 auto idle bit has to be disabled because of the
>errata.So using
>> HWMOD_NO_OCP_AUTOIDLE flag for OMAP3430.
>>
>> Signed-off-by: Hema HK
>> Signed-off-by: Basak, Partha
>>
>> Cc: Felipe Balbi
>> Cc: Tony Lindgren
>> Cc: Kevin Hilman
>> ---
>>
>> arch/arm/mach-omap2/pm34xx.c | 10 +++-
>> arch/arm/mach-omap2/usb-musb.c| 72
>++-
>> arch/arm/plat-omap/include/plat/usb.h |9 +++
>> drivers/usb/musb/musb_core.c | 12 +
>> drivers/usb/musb/omap2430.c | 77
>+-
>> include/linux/usb/musb.h | 18 +++
>> 6 files changed, 126 insertions(+), 72 deletions(-)
>>
>> Index: linux-omap-pm/arch/arm/mach-omap2/pm34xx.c
>> ===
>> --- linux-omap-pm.orig/arch/arm/mach-omap2/pm34xx.c
>2010-08-06 10:44:06.0 -0400
>> +++ linux-omap-pm/arch/arm/mach-omap2/pm34xx.c 2010-08-06
>10:44:42.942112875 -0400
>> @@ -418,7 +418,9 @@
>> omap3_core_save_context();
>> omap3_prcm_save_context();
>> /* Save MUSB context */
>> - musb_context_save_restore(1);
>> + musb_context_save_restore(save_context);
>> + } else {
>> + musb_context_save_restore(disable_clk);
>> }
>> }
>>
>> @@ -462,8 +464,10 @@
>> omap3_sram_restore_context();
>> omap2_sms_restore_context();
>> /* restore MUSB context */
>> - musb_context_save_restore(0);
>> - }
>> + musb_context_save_restore(restore_context);
>> + } else {
>> + musb_context_save_restore(enable_clk);
>> + }
>> omap_uart_resume_idle(0);
>> omap_uart_resume_idle(1);
>> if (core_next_state == PWRDM_POWER_OFF)
>> Index: linux-omap-pm/arch/arm/mach-omap2/usb-musb.c
>> ===
>> --- linux-omap-pm.orig/arch/arm/mach-omap2/usb-musb.c
>2010-08-06 10:44:06.0 -0400
>> +++ linux-omap-pm/arch/arm/mach-omap2/usb-musb.c
>2010-08-06 10:44:42.942112875 -0400
>> @@ -25,11 +25,13 @@
>> #include
>>
>> #include
>> +#include
>>
>> #include
>> #include
>> #include
>> #include
>> +#include
>>
>> #ifdef CONFIG_USB_MUSB_SOC
>>
>> @@ -99,6 +101,18 @@
>> musb_plat.board_data = board_data;
>> musb_plat.power = board_data->power>> 1;
>> musb_plat.mode = board_data->mode;
>> + musb_plat.device_enable = omap_device_enable;
>> + musb_plat.device_idle = omap_device_idle;
>> + musb_plat.enable_wakeup = omap_device_enable_wakeup;
>> + musb_plat.disable_wakeup =
>omap_device_disable_wakeup;
>> + /*
>> +* Errata 1.166 idle_req/ack is broken in omap3430
>> +* workaround is to disable the autodile bit
>for omap3430.
>> +*/
>
>As written in the errata document: Unique reference to be used for
>communication, Errata ID: i479. You should not use 1.166.
>Also the description is a little bit different:
>idle_req / idle_ack mechanism potentially broken when autoidle
>is enabled.
>OK, it looks like it can occur randomly during run time, meaning that
>"potentially"
RE: [PATCH V2 4/8]usb : musb:Using omap_device_build for musb device registration
Hi,
>-Original Message-
>From: Cousson, Benoit
>Sent: Monday, August 09, 2010 6:15 PM
>To: Kalliguddi, Hema
>Cc: linux-...@vger.kernel.org; linux-omap@vger.kernel.org;
>Felipe Balbi; Tony Lindgren; Kevin Hilman
>Subject: Re: [PATCH V2 4/8]usb : musb:Using omap_device_build
>for musb device registration
>
>On 8/6/2010 5:57 PM, Kalliguddi, Hema wrote:
>> From: Hema HK
>
>
>
>> void __init usb_musb_init(struct omap_musb_board_data *board_data)
>> {
>> -if (cpu_is_omap243x()) {
>> -musb_resources[0].start = OMAP243X_HS_BASE;
>> -} else if (cpu_is_omap34xx()) {
>> -musb_resources[0].start = OMAP34XX_HSUSB_OTG_BASE;
>> -} else if (cpu_is_omap44xx()) {
>> -musb_resources[0].start = OMAP44XX_HSUSB_OTG_BASE;
>> -musb_resources[1].start = OMAP44XX_IRQ_HS_USB_MC_N;
>> -musb_resources[2].start = OMAP44XX_IRQ_HS_USB_DMA_N;
>> +char oh_name[MAX_OMAP_MUSB_HWMOD_NAME_LEN];
>> +struct omap_hwmod *oh;
>> +struct omap_device *od;
>> +struct platform_device *pdev;
>> +struct device *dev;
>> +int l, bus_id = -1;
>> +struct musb_hdrc_platform_data *pdata;
>> +
>> +l = snprintf(oh_name, MAX_OMAP_MUSB_HWMOD_NAME_LEN,
>> +"usb_otg_hs");
>> +WARN(l>= MAX_OMAP_MUSB_HWMOD_NAME_LEN,
>> +"String buffer overflow in MUSB device
>setup\n");
>
>This is not needed in your case. Just use a const char*, and you will
>avoid the useless snprintf and test.
Ok.
>
>> +
>> +oh = omap_hwmod_lookup(oh_name);
>> +
>> +if (!oh) {
>> +pr_err("Could not look up %s\n", oh_name);
>> +} else {
>
>You can avoid that indentation be returning in case of failure.
Agreed.
>
>> +/*
>> + * REVISIT: This line can be removed once all
>the platforms
>> + * using musb_core.c have been converted to use
>use clkdev.
>> + */
>> +musb_plat.clock = "ick";
>> +musb_plat.board_data = board_data;
>> +musb_plat.power = board_data->power>> 1;
>> +musb_plat.mode = board_data->mode;
>> +pdata =&musb_plat;
>> +
>> +od = omap_device_build(name, bus_id, oh, pdata,
>> + sizeof(struct
>musb_hdrc_platform_data),
>> +
>omap_musb_latency,
>> +
>ARRAY_SIZE(omap_musb_latency), false);
>> +if (IS_ERR(od)) {
>> +pr_err("Could not build omap_device for
>%s %s\n",
>> +name, oh_name);
> > + } else {
>
>You can avoid that second level of indentation be returning in case of
>failure as well.
Agreed.
>
>Benoit
>--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 0/8]usb: musb:USB driver changes to use hwmod+ omap-device apis
Hi, >-Original Message- >From: Cousson, Benoit >Sent: Monday, August 09, 2010 6:02 PM >To: Kalliguddi, Hema >Cc: linux-...@vger.kernel.org; linux-omap@vger.kernel.org; >Felipe Balbi; Tony Lindgren; Kevin Hilman >Subject: Re: [PATCH 0/8]usb: musb:USB driver changes to use >hwmod+ omap-device apis > >On 8/6/2010 5:54 PM, HEMA HK wrote: >> From: Hema HK >> >> Series of patches to port musb driver to use hwmod and runtime pm >> APIs for OMAP4 and OMAP3.These patches are tested on OMAP4430SDP and >> OMAP3630-ZOOM3 boards. >> >> The patch 1 and 2 are are the prerequisites for the hwmod changes. >> Patch 6 is the OMAP3 offmode support patch. >> >> This patch series is generated on "origin/pm-wip/hwmods-omap4". >> >> Offmode is tested with origin/pm on omap3630 zoom3 board. > >The way you are submitting that series is quite confusing! > >You are mixing several numbering schemes and several series versions. >If this is a V2, you have to resubmit everything with a V2, >and you must >include an history. > >[8/8] usb : musb:Using runtime pm apis for musb. >[7/8] : Hwmod api changes >[V2,6/8] usb: musb: Offmode fix for idle path >[V2,4/8] usb : musb:Using omap_device_build for musb device >registration >[4/8] usb: musb: HWMOD database structures fixes OMAP4 >[V2,3/4] usb: musb: HWMOD database structures addition for OMAP3 >[2/8] usb: musb: Remove board_data parameter from musb_platform_init() >[1/8] usb: musb: Adding names for IRQs in resource structure > > >The cover letter does not contain the overall stat to summarize what >files you are modifying. > >Some spaces are missing after the ":" in the email subjects. I will take care of this in the next st of patches. Thanks, Hema > >Regards, >Benoit >-- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH V2 6/8] usb: musb: Offmode fix for idle path
Hi,
>-Original Message-
>From: Cousson, Benoit
>Sent: Monday, August 09, 2010 6:31 PM
>To: Kalliguddi, Hema
>Cc: linux-...@vger.kernel.org; linux-omap@vger.kernel.org;
>Mankad, Maulik Ojas; Felipe Balbi; Tony Lindgren; Kevin Hilman
>Subject: Re: [PATCH V2 6/8] usb: musb: Offmode fix for idle path
>
>On 8/6/2010 7:27 PM, Kalliguddi, Hema wrote:
>> From: Hema HK
>>
>> With OMAP core-off support musb was not functional as
>context was getting
>> lost after wakeup from core-off. And also musb was blocking
>the core-off
>> after loading the gadget driver even with no cable connected
>sometimes.
>>
>> Added the conext save/restore api in the platform layer which will
>> be called in the idle and wakeup path.
>>
>> Changed the usb sysconfig settings as per the usbotg functional spec.
>> When the device is not active, configure to force idle and
>force standby mode.
>> When it is being used, configure in smart standby and smart
>idle mode.
>> So while attempting to coreoff the usb is configured to
>force standby and
>> force idle mode, after wakeup configured in smart idle and
>smart standby.
>
>Is this valid for OMAP3 and OMAP4? I checked quickly the OMAP4
>TRM vB in
>the USB_OTG chapter related to PM (23.13.4.2 Power-Management), and I
>cannot find any note regarding that specific programming model.
>Could give us reference to the documentation that contains the details
>about that?
>
>Benoit
>
This is valid for both OMAP3 and OMAP4. You can find this in 24.1.5.4 section
With different modes. When not used with any application the mode has to be
programmed
is force idle and force standby. What I observed is that without this USB was
blocking the coreoff.
http://focus.ti.com/pdfs/wtbu/SWPU223D_Final_EPDF_06_07_2010.pdf
Regards,
Hema
>>
>> Signed-off-by: Hema HK
>> Signed-off-by: Maulik Mankad
>>
>> Cc: Felipe Balbi
>> Cc: Tony Lindgren
>> Cc: Kevin Hilman
>> ---
>>
>> arch/arm/mach-omap2/pm34xx.c |4 ++
>> arch/arm/mach-omap2/usb-musb.c| 21 ++
>> arch/arm/plat-omap/include/plat/usb.h |2 +
>> drivers/usb/musb/musb_core.c | 11 ---
>> drivers/usb/musb/omap2430.c | 48
>+++---
>> 5 files changed, 71 insertions(+), 15 deletions(-)
>>
>> Index: linux-omap-pm/arch/arm/mach-omap2/pm34xx.c
>> ===
>> --- linux-omap-pm.orig/arch/arm/mach-omap2/pm34xx.c
>2010-08-06 09:23:01.153862710 -0400
>> +++ linux-omap-pm/arch/arm/mach-omap2/pm34xx.c
>2010-08-06 10:44:06.393863125 -0400
>> @@ -39,6 +39,7 @@
>> #include
>> #include
>> #include
>> +#include
>>
>> #include
>>
>> @@ -416,6 +417,8 @@
>> if (core_next_state == PWRDM_POWER_OFF) {
>> omap3_core_save_context();
>> omap3_prcm_save_context();
>> +/* Save MUSB context */
>> +musb_context_save_restore(1);
>> }
>> }
>>
>> @@ -458,6 +461,8 @@
>> omap3_prcm_restore_context();
>> omap3_sram_restore_context();
>> omap2_sms_restore_context();
>> +/* restore MUSB context */
>> +musb_context_save_restore(0);
>> }
>> omap_uart_resume_idle(0);
>> omap_uart_resume_idle(1);
>> Index: linux-omap-pm/arch/arm/mach-omap2/usb-musb.c
>> ===
>> --- linux-omap-pm.orig/arch/arm/mach-omap2/usb-musb.c
>2010-08-06 09:24:23.690112596 -0400
>> +++ linux-omap-pm/arch/arm/mach-omap2/usb-musb.c
>2010-08-06 10:44:06.385862697 -0400
>> @@ -120,6 +120,27 @@
>> }
>> }
>>
>> +void musb_context_save_restore(int save)
>> +{
>> +struct omap_hwmod *oh = omap_hwmod_lookup("usb_otg_hs");
>> +struct omap_device *od = oh->od;
>> +struct platform_device *pdev =&od->pdev;
>> +struct device *dev =&pdev->dev;
>> +struct device_driver *drv = dev->driver;
>> +
>> +if (drv) {
>> +struct musb_hdrc_platform_data *pdata =
>dev->platform_data;
>> +const struct dev_pm_ops *pm = drv->pm;
>> +if (!pdata->is_usb_active(dev)) {
>> +
>> +if (save)
>> +pm
RE: [PATCH] usb: musb: Offmode fix for idle path
Hello,
>-Original Message-
>From: Sripathy, Vishwanath
>Sent: Thursday, July 08, 2010 7:14 PM
>To: Kalliguddi, Hema; linux-...@vger.kernel.org;
>linux-omap@vger.kernel.org
>Cc: Mankad, Maulik Ojas; Felipe Balbi; Tony Lindgren; Kevin Hilman
>Subject: RE: [PATCH] usb: musb: Offmode fix for idle path
>
>Hema,
>
>> -Original Message-
>> From: linux-omap-ow...@vger.kernel.org [mailto:linux-omap-
>> ow...@vger.kernel.org] On Behalf Of Kalliguddi, Hema
>> Sent: Thursday, July 08, 2010 3:59 PM
>> To: linux-...@vger.kernel.org; linux-omap@vger.kernel.org
>> Cc: Kalliguddi, Hema; Mankad, Maulik Ojas; Felipe Balbi;
>Tony Lindgren; Kevin Hilman
>> Subject: [PATCH] usb: musb: Offmode fix for idle path
>>
>> From: Hema HK
>>
>> With OMAP coreoff support usb was not functional as context
>was getting
>> lost after wakeup from coreoff. And also usb was blocking the coreoff
>> after loading the gadget driver even with no cable connected
>sometimes.
>>
>> Added the conext save/restore api in the platform layer which will
>> be called in the idle and wakeup path.
>>
>> Changed the usb sysconfig settings as per the usbotg functional spec.
>> When the device is not used, configure in force idle and
>force standby mode.
>> When it is being used, configure in smart standby and smart
>idle mode.
>> So while attempting to coreoff the usb is configured to
>force standby and
>> force idle mode, after wakeup configured in smart idle and
>smart standby.
>>
>> Since clock used for musb is auto gated, there is no need to
>specifically
>> enable/disable the clock. Removed enable/disable clock in
>suspend resume api.
>>
>> Signed-off-by: Hema HK
>> Signed-off-by: Maulik Mankad
>>
>> Cc: Felipe Balbi
>> Cc: Tony Lindgren
>> Cc: Kevin Hilman
>> ---
>> Based off : Kevin pm branch.
>>
>> arch/arm/mach-omap2/pm34xx.c |4
>> arch/arm/mach-omap2/usb-musb.c| 15 +++
>> arch/arm/plat-omap/include/plat/usb.h |2 ++
>> drivers/usb/musb/musb_core.c | 11 ---
>> drivers/usb/musb/omap2430.c | 32
>>
>> 5 files changed, 49 insertions(+), 15 deletions(-)
>>
>> Index: linux-omap-pm/arch/arm/mach-omap2/pm34xx.c
>> =
>> ==
>> --- linux-omap-pm.orig/arch/arm/mach-omap2/pm34xx.c
>> +++ linux-omap-pm/arch/arm/mach-omap2/pm34xx.c
>> @@ -431,6 +431,8 @@ void omap_sram_idle(void)
>> if (core_next_state == PWRDM_POWER_OFF) {
>> omap3_core_save_context();
>> omap3_prcm_save_context();
>> +/* Save MUSB context */
>> +musb_context_save_restore(1);
>Do you really need to save and restore USB context in every OS
>Idle? Can't it be done on a need basis like when USB is connected?
No. There are some transperent PHYs in which the connect/disconnect events
are not reported as interrupts.
In that case there is no way know when to save and restore.
We can do the optimization of saving the context only when needed.
>> }
>> }
>>
>> @@ -479,6 +481,8 @@ void omap_sram_idle(void)
>> omap3_prcm_restore_context();
>> omap3_sram_restore_context();
>> omap2_sms_restore_context();
>> +/* Restore MUSB context */
>> +musb_context_save_restore(0);
>> /*
>> * Errata 1.164 fix : OTG autoidle can prevent
>> * sleep
>> Index: linux-omap-pm/arch/arm/mach-omap2/usb-musb.c
>> =
>> ==
>> --- linux-omap-pm.orig/arch/arm/mach-omap2/usb-musb.c
>> +++ linux-omap-pm/arch/arm/mach-omap2/usb-musb.c
>> @@ -177,6 +177,21 @@ void __init usb_musb_init(struct omap_mu
>> usb_musb_pm_init();
>> }
>>
>> +void musb_context_save_restore(int save)
>> +{
>> +struct device *dev = &musb_device.dev;
>> +struct device_driver *drv = dev->driver;
>> +if (dev->driver) {
>> +
>> +const struct dev_pm_ops *pm = drv->pm;
>> +
>> +if (save)
>> +pm->suspend(dev);
>> +else
>> +pm->resume_noirq(dev);
>> +}
>> +}
>> +
>> #else
>> void __in
RE: [PATCH] usb: musb: Offmode fix for idle path
Hello,
>-Original Message-
>From: Sergei Shtylyov [mailto:sshtyl...@mvista.com]
>Sent: Thursday, July 08, 2010 6:58 PM
>To: Kalliguddi, Hema
>Cc: linux-...@vger.kernel.org; linux-omap@vger.kernel.org;
>Mankad, Maulik Ojas; Felipe Balbi; Tony Lindgren; Kevin Hilman
>Subject: Re: [PATCH] usb: musb: Offmode fix for idle path
>
>Hello.
>
>Hema HK wrote:
>
>> With OMAP coreoff support usb was not functional as context
>was getting
>> lost after wakeup from coreoff. And also usb was blocking
>the coreoff
>
>USB is an acronym.
>
Agree I will change it accordingly.
>> after loading the gadget driver even with no cable connected
>sometimes.
>
>> Added the conext save/restore api in the platform layer which will
>
>API is an acronym.
>
Agree.
>> be called in the idle and wakeup path.
>
>> Changed the usb sysconfig settings as per the usbotg functional spec.
>
>Do you mean the OTG supplement to USB 2.0 spec. or something else?
>
No it is TI OMAP USBOTG functional specifications.
>> When the device is not used, configure in force idle and
>force standby mode.
>> When it is being used, configure in smart standby and smart
>idle mode.
>> So while attempting to coreoff the usb is configured to
>force standby and
>> force idle mode, after wakeup configured in smart idle and
>smart standby.
>
>> Since clock used for musb is auto gated, there is no need to
>specifically
>> enable/disable the clock. Removed enable/disable clock in
>suspend resume api.
>
>I'm not sure it's auto-gated on all platforms...
Might be. need to check this.
Blackfin guys,
Please comment...
>
>> Signed-off-by: Hema HK
>> Signed-off-by: Maulik Mankad
>
>> Cc: Felipe Balbi
>> Cc: Tony Lindgren
>> Cc: Kevin Hilman
>
>> Index: linux-omap-pm/arch/arm/mach-omap2/usb-musb.c
>> ===
>> --- linux-omap-pm.orig/arch/arm/mach-omap2/usb-musb.c
>> +++ linux-omap-pm/arch/arm/mach-omap2/usb-musb.c
>> @@ -177,6 +177,21 @@ void __init usb_musb_init(struct omap_mu
>> usb_musb_pm_init();
>> }
>>
>> +void musb_context_save_restore(int save)
>> +{
>> +struct device *dev = &musb_device.dev;
>> +struct device_driver *drv = dev->driver;
>
>Need an empty line here.
>
OK.
>> +if (dev->driver) {
>
> You've just assigned that to 'drv' -- why not use it?
>
I will do that.
>> Index: linux-omap-pm/arch/arm/plat-omap/include/plat/usb.h
>> ===
>> --- linux-omap-pm.orig/arch/arm/plat-omap/include/plat/usb.h
>> +++ linux-omap-pm/arch/arm/plat-omap/include/plat/usb.h
>> @@ -82,6 +82,8 @@ extern void usb_ohci_init(const struct o
>> /* This is needed for OMAP3 errata 1.164: enabled autoidle
>can prevent sleep */
>> extern void usb_musb_disable_autoidle(void);
>>
>> +/* For saving and restoring the musb context during off/wakeup*/
>> +extern void musb_context_save_restore(int save);
>> #endif
>>
>> void omap_usb_init(struct omap_usb_config *pdata);
>> Index: linux-omap-pm/drivers/usb/musb/musb_core.c
>> ===
>> --- linux-omap-pm.orig/drivers/usb/musb/musb_core.c
>> +++ linux-omap-pm/drivers/usb/musb/musb_core.c
>> @@ -2430,11 +2430,6 @@ static int musb_suspend(struct device *d
>> }
>>
>> musb_save_context(musb);
>> -
>> -if (musb->set_clock)
>> -musb->set_clock(musb->clock, 0);
>> -else
>> -clk_disable(musb->clock);
>> spin_unlock_irqrestore(&musb->lock, flags);
>> return 0;
>> }
>> @@ -2446,12 +2441,6 @@ static int musb_resume_noirq(struct devi
>>
>> if (!musb->clock)
>> return 0;
>> -
>> -if (musb->set_clock)
>> -musb->set_clock(musb->clock, 1);
>> -else
>> -clk_enable(musb->clock);
>> -
>
>OK, maybe for OMAP the clock is auto-gated but what about
>the other platforms?
>
For OMAP it is autogated.
Blackfin guys comments please?
>> musb_restore_context(musb);
>>
>> /* for static cmos like DaVinci, register values were preserved
>> Index: linux-omap-pm/drivers/usb/musb/omap2430.c
>> ===
>> --- linux-omap-pm.orig/drivers/usb/musb/omap2430.c
>> +++ linu
RE: [PATCH 4/4]usb : musb:USB driver using omap_device_build for device registration.
Hello, >-Original Message- >From: Sergei Shtylyov [mailto:sshtyl...@mvista.com] >Sent: Tuesday, June 29, 2010 4:35 PM >To: Kalliguddi, Hema >Cc: linux-...@vger.kernel.org; linux-omap@vger.kernel.org; >Felipe Balbi; t...@atomide.com >Subject: Re: [PATCH 4/4]usb : musb:USB driver using >omap_device_build for device registration. > >Hello. > >Hema HK wrote: > >> From: Hema HK > >I think the patch description has clarity issues... > >> Changed the musb driver to use omap_build_device api instead of > >I don't see where are you changing the MUSB driver. There is function name mismatch in the description.it should be "omap_device_build" instead of omap_build_device. The change is in usb-musb.c file. I agree that it is not a driver file.I will change the description appropriately. > >> platform_device_registration. > >Why underscores here? > I intended to provide the API name here platform_device_register. I will correct it. >> The device specific resources defined in >> centralized database will be used. So removed the resource >definitions from >> the driver file. > >Do you mean usb-musb.c here? This is certainly not a driver. > Agreed not a driver. Regards, Hema >> Signed-off-by: Hema HK >> Cc: Felipe Balbi ,t...@atomide.com > >WBR, Sergei > >-- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: On the APIs for Enabling and Disabling Wakeup capability.
>-Original Message- >From: linux-omap-ow...@vger.kernel.org >[mailto:linux-omap-ow...@vger.kernel.org] On Behalf Of Kalliguddi, Hema >Sent: Thursday, June 24, 2010 8:25 PM >To: Kevin Hilman; Basak, Partha >Cc: Cousson, Benoit; p...@pwsan.com; Nayak, Rajendra; >linux-omap@vger.kernel.org; Gadiyar, Anand; Kamat, Nishant >Subject: RE: On the APIs for Enabling and Disabling Wakeup capability. > >Kevin, >Replying on top of latest chin. > >Thanks, >Hema > >>-Original Message- >>From: Kevin Hilman [mailto:khil...@deeprootsystems.com] >>Sent: Thursday, June 24, 2010 2:01 AM >>To: Basak, Partha >>Cc: Kalliguddi, Hema; Cousson, Benoit; p...@pwsan.com; Nayak, >>Rajendra; linux-omap@vger.kernel.org >>Subject: Re: On the APIs for Enabling and Disabling Wakeup capability. >> >>"Basak, Partha" writes: >> >>> Benoit/Kevin, >>> >>>> -----Original Message- >>>> From: Kevin Hilman [mailto:khil...@deeprootsystems.com] >>>> Sent: Friday, June 18, 2010 8:29 PM >>>> To: Kalliguddi, Hema >>>> Cc: Cousson, Benoit; Basak, Partha; p...@pwsan.com; Nayak, >Rajendra; >>>> linux-omap@vger.kernel.org >>>> Subject: Re: On the APIs for Enabling and Disabling Wakeup >>capability. >>>> >>>> "Kalliguddi, Hema" writes: >>>> >>>> > Hi Benoit, >>>> > >>>> > I have 2 cases in usb for the need of separate API for >>setting the auto >>>> idle bit. >>>> > >>>> > 1. Below the link for omap3430TRM. Please refer 24.1.5.4.2 and >>>> 24.1.5.4.3 there is note not to set smart >>>> > idle and autoidle bit simultaneously. Suggestion is to >>set the auto idle >>>> 0 before setting smart idle. Then set to 1. >>>> >>>> Maybe this sequence should be enforced by the hwmod code itself, >>>> rather than the knowledge living in the driver. >>>> >>>> However, based on the errata below, auto-idle will always >be zero so >>>> the there should be no conflict in setting smart-idle and auto-idle >>>> simultaneously today. >>>> >>>> > This applicable for omap4 as well. I don't have the >>omap4430 pblic TRM >>>> to share. >>>> > http://focus.ti.com/pdfs/wtbu/SWPU223D_Final_EPDF_06_07_2010.pdf >>>> > > >[HK] The errata is applicable to OMAP3430 only. >But this sequence applicable for omap4 as well. So for >submitting the changes we need >to have the HWMOD api to be changed to enaforce the Autodile >set only after the smart idle. > When can I expect the HWMOD API change which includes the auto idle setting only after Smart idle setting and wakeup enable/disable settings? Right now shall I proceed with setting Autoidle using omap_hwmod_set_slave_idlemode(), And omap_hwmod_enable_wakeup and omap_hwmod_disable_wakeup apis? >>>> > 2. There is a Errata in OMAP3 errata #1.59 that If auto >>idle is set then >>>> the USB can't wakeup the system by >>>> > usb external events. The suggested workaround is to >>disable the autoIdle >>>> for omap3. >>>> >>>> This one could be handled at init time in usb-musb.c by setting >>>> HWMOD_NO_OCP_AUTOIDLE for the hwmod with a comment summarizing this >>>> errata. >>>> >>>> Note also that Errata 1.166 is another one related to MUSB >auto-idle >>>> and we have a workaround in the PM branch to ensure that MUSB >>>> auto-idle is disabled in the idle path since it may be re-enabled >>>> after an off-mode transition. >>>> >>> Few questions: >>> 1. Are you mentioning about the following patch: >>> >>http://git.kernel.org/?p=linux/kernel/git/khilman/linux-omap-pm >>.git;a=commit;h=865f0e0b1dd25899fe30eee5c8f1dba420eea177? >> >>No, this is a totally unrelated problem related to a specific init >>sequene. >> >>> Though this patch allows clearing of AutoIdle bit(signified by >>> HWMOD_INIT_NO_IDLE) while remaining in Idle state, it does not allow >>> the reverse, i.e. setting back the AutoIdle bit, while still >>remaining >>> in Idle state. >> >>The use-case for setting the auto-idle bit while leaving the device in >>idle have not been described. >> >>> 2. Changing the hwmod flags (oh->flags) is acceptable in >>run-time. Correct? >> >>For errata workarounds, in
RE: [PATCH 4/4]usb : musb:USB driver using omap_device_build for device registration.
>-Original Message- >From: linux-usb-ow...@vger.kernel.org >[mailto:linux-usb-ow...@vger.kernel.org] On Behalf Of Kalliguddi, Hema >Sent: Monday, June 28, 2010 7:17 PM >To: Tony Lindgren >Cc: linux-...@vger.kernel.org; linux-omap@vger.kernel.org; Felipe Balbi >Subject: RE: [PATCH 4/4]usb : musb:USB driver using >omap_device_build for device registration. > > > >>-Original Message- >>From: Tony Lindgren [mailto:t...@atomide.com] >>Sent: Monday, June 28, 2010 7:12 PM >>To: Kalliguddi, Hema >>Cc: linux-...@vger.kernel.org; linux-omap@vger.kernel.org; >Felipe Balbi >>Subject: Re: [PATCH 4/4]usb : musb:USB driver using >>omap_device_build for device registration. >> >>* Kalliguddi, Hema [100628 16:26]: >>> >>> >>> >-Original Message- >>> >From: Tony Lindgren [mailto:t...@atomide.com] >>> >Sent: Monday, June 28, 2010 6:59 PM >>> >To: Kalliguddi, Hema >>> >Cc: linux-...@vger.kernel.org; linux-omap@vger.kernel.org; >>Felipe Balbi >>> >Subject: Re: [PATCH 4/4]usb : musb:USB driver using >>> >omap_device_build for device registration. >>> > >>> >* Hema HK [100628 15:12]: >>> >> - >>> >> -/* >>> >> - * REVISIT: This line can be removed once all the >>> >platforms using >>> >> - * musb_core.c have been converted to use use clkdev. >>> >> - */ >>> >> -musb_plat.clock = "ick"; >>> > >>> >This comment is still valid, please don't remove it. >>> This is still there. It is moved below under else. >> Yes. The comment is being removed in the patch I posted. I will keep it. Regards, Hema >>Tony >>-- >To unsubscribe from this list: send the line "unsubscribe linux-usb" in >the body of a message to majord...@vger.kernel.org >More majordomo info at http://vger.kernel.org/majordomo-info.html >-- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 4/4]usb : musb:USB driver using omap_device_build for device registration.
>-Original Message-
>From: Tony Lindgren [mailto:t...@atomide.com]
>Sent: Monday, June 28, 2010 7:12 PM
>To: Kalliguddi, Hema
>Cc: linux-...@vger.kernel.org; linux-omap@vger.kernel.org; Felipe Balbi
>Subject: Re: [PATCH 4/4]usb : musb:USB driver using
>omap_device_build for device registration.
>
>* Kalliguddi, Hema [100628 16:26]:
>>
>>
>> >-Original Message-
>> >From: Tony Lindgren [mailto:t...@atomide.com]
>> >Sent: Monday, June 28, 2010 6:59 PM
>> >To: Kalliguddi, Hema
>> >Cc: linux-...@vger.kernel.org; linux-omap@vger.kernel.org;
>Felipe Balbi
>> >Subject: Re: [PATCH 4/4]usb : musb:USB driver using
>> >omap_device_build for device registration.
>> >
>> >* Hema HK [100628 15:12]:
>> >> -
>> >> - /*
>> >> - * REVISIT: This line can be removed once all the
>> >platforms using
>> >> - * musb_core.c have been converted to use use clkdev.
>> >> - */
>> >> - musb_plat.clock = "ick";
>> >
>> >This comment is still valid, please don't remove it.
>> This is still there. It is moved below under else.
>
>Not in the patch you posted?
>
- musb_plat.clock = "ick";
- musb_plat.board_data = board_data;
- musb_plat.power = board_data->power >> 1;
- musb_plat.mode = board_data->mode;
- musb_plat.extvbus = board_data->extvbus;
-
- if (platform_device_register(&musb_device) < 0)
- printk(KERN_ERR "Unable to register HS-USB (MUSB) device\n");
+ char oh_name[MAX_OMAP_MUSB_HWMOD_NAME_LEN];
+ struct omap_hwmod *oh;
+ struct omap_device *od;
+ struct platform_device *pdev;
+ struct device *dev;
+ int l, bus_id = -1;
+ struct musb_hdrc_platform_data *pdata;
+
+ l = snprintf(oh_name, MAX_OMAP_MUSB_HWMOD_NAME_LEN,
+ "usb_otg_hs");
+ WARN(l >= MAX_OMAP_MUSB_HWMOD_NAME_LEN,
+ "String buffer overflow in MUSB device setup\n");
+ oh = omap_hwmod_lookup(oh_name);
+ if (!oh) {
+ pr_err("Could not look up %s\n", oh_name);
+ } else {
+ musb_plat.clock = "ick";
+ musb_plat.board_data = board_data;
+ musb_plat.power = board_data->power >> 1;
+ musb_plat.mode = board_data->mode;
+ pdata = &musb_plat;
It is there in the patch I posted.only the comment is being removed. You are
refering the comment also?
Regards,
Hema
>Tony
>--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 4/4]usb : musb:USB driver using omap_device_build for device registration.
>-Original Message- >From: Tony Lindgren [mailto:t...@atomide.com] >Sent: Monday, June 28, 2010 6:59 PM >To: Kalliguddi, Hema >Cc: linux-...@vger.kernel.org; linux-omap@vger.kernel.org; Felipe Balbi >Subject: Re: [PATCH 4/4]usb : musb:USB driver using >omap_device_build for device registration. > >* Hema HK [100628 15:12]: >> - >> -/* >> - * REVISIT: This line can be removed once all the >platforms using >> - * musb_core.c have been converted to use use clkdev. >> - */ >> -musb_plat.clock = "ick"; > >This comment is still valid, please don't remove it. This is still there. It is moved below under else. Regards, Hema > >Regards, > >Tony >-- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: On the APIs for Enabling and Disabling Wakeup capability.
Kevin, Replying on top of latest chin. Thanks, Hema >-Original Message- >From: Kevin Hilman [mailto:khil...@deeprootsystems.com] >Sent: Thursday, June 24, 2010 2:01 AM >To: Basak, Partha >Cc: Kalliguddi, Hema; Cousson, Benoit; p...@pwsan.com; Nayak, >Rajendra; linux-omap@vger.kernel.org >Subject: Re: On the APIs for Enabling and Disabling Wakeup capability. > >"Basak, Partha" writes: > >> Benoit/Kevin, >> >>> -Original Message- >>> From: Kevin Hilman [mailto:khil...@deeprootsystems.com] >>> Sent: Friday, June 18, 2010 8:29 PM >>> To: Kalliguddi, Hema >>> Cc: Cousson, Benoit; Basak, Partha; p...@pwsan.com; Nayak, Rajendra; >>> linux-omap@vger.kernel.org >>> Subject: Re: On the APIs for Enabling and Disabling Wakeup >capability. >>> >>> "Kalliguddi, Hema" writes: >>> >>> > Hi Benoit, >>> > >>> > I have 2 cases in usb for the need of separate API for >setting the auto >>> idle bit. >>> > >>> > 1. Below the link for omap3430TRM. Please refer 24.1.5.4.2 and >>> 24.1.5.4.3 there is note not to set smart >>> > idle and autoidle bit simultaneously. Suggestion is to >set the auto idle >>> 0 before setting smart idle. Then set to 1. >>> >>> Maybe this sequence should be enforced by the hwmod code itself, >>> rather than the knowledge living in the driver. >>> >>> However, based on the errata below, auto-idle will always be zero so >>> the there should be no conflict in setting smart-idle and auto-idle >>> simultaneously today. >>> >>> > This applicable for omap4 as well. I don't have the >omap4430 pblic TRM >>> to share. >>> > http://focus.ti.com/pdfs/wtbu/SWPU223D_Final_EPDF_06_07_2010.pdf >>> > [HK] The errata is applicable to OMAP3430 only. But this sequence applicable for omap4 as well. So for submitting the changes we need to have the HWMOD api to be changed to enaforce the Autodile set only after the smart idle. >>> > 2. There is a Errata in OMAP3 errata #1.59 that If auto >idle is set then >>> the USB can't wakeup the system by >>> > usb external events. The suggested workaround is to >disable the autoIdle >>> for omap3. >>> >>> This one could be handled at init time in usb-musb.c by setting >>> HWMOD_NO_OCP_AUTOIDLE for the hwmod with a comment summarizing this >>> errata. >>> >>> Note also that Errata 1.166 is another one related to MUSB auto-idle >>> and we have a workaround in the PM branch to ensure that MUSB >>> auto-idle is disabled in the idle path since it may be re-enabled >>> after an off-mode transition. >>> >> Few questions: >> 1. Are you mentioning about the following patch: >> >http://git.kernel.org/?p=linux/kernel/git/khilman/linux-omap-pm >.git;a=commit;h=865f0e0b1dd25899fe30eee5c8f1dba420eea177? > >No, this is a totally unrelated problem related to a specific init >sequene. > >> Though this patch allows clearing of AutoIdle bit(signified by >> HWMOD_INIT_NO_IDLE) while remaining in Idle state, it does not allow >> the reverse, i.e. setting back the AutoIdle bit, while still >remaining >> in Idle state. > >The use-case for setting the auto-idle bit while leaving the device in >idle have not been described. > >> 2. Changing the hwmod flags (oh->flags) is acceptable in >run-time. Correct? > >For errata workarounds, in device/SoC init code yes. In drivers, no. >Drivers should not have any knowledge of hwmod internals. > >> 3. I believe, SysConfig settings have been a tricky area because of >> different h/w-errata. Instead of looking into particular errata, as >> and when they come in time to time and explore how to fit them in the >> framework, would it not be more useful to provide a more generic >> framework to operate on the SysConfig settings? Of course, as you >> suggested, the preferred approach would be to absorb the changes in >> the omap_device/hw-mod layer as much as possible. But unfortunately, >> that may not be sufficient in all situations. > >For this kind of thing, I strongly prefer to better understand the >specific errata that require the special settings. > >History suggests that having an API to modify sysconfig means it would >get used not just for errata workarounds but also because "it doesn't >work unless I do this." We *really* need to better understand the >reasons behind these special cases. > [HK] I agree that t
RE: On the APIs for Enabling and Disabling Wakeup capability.
Kevin, >-Original Message- >From: Kevin Hilman [mailto:khil...@deeprootsystems.com] >Sent: Friday, June 18, 2010 8:29 PM >To: Kalliguddi, Hema >Cc: Cousson, Benoit; Basak, Partha; p...@pwsan.com; Nayak, >Rajendra; linux-omap@vger.kernel.org >Subject: Re: On the APIs for Enabling and Disabling Wakeup capability. > >"Kalliguddi, Hema" writes: > >> Hi Benoit, >> >> I have 2 cases in usb for the need of separate API for >setting the auto idle bit. >> >> 1. Below the link for omap3430TRM. Please refer 24.1.5.4.2 >and 24.1.5.4.3 there is note not to set smart >> idle and autoidle bit simultaneously. Suggestion is to set >the auto idle 0 before setting smart idle. Then set to 1. > >Maybe this sequence should be enforced by the hwmod code itself, >rather than the knowledge living in the driver. > >However, based on the errata below, auto-idle will always be zero so >the there should be no conflict in setting smart-idle and auto-idle >simultaneously today. The errata is applicable to OMAP3430 only. But this sequence applicable for omap4 as well. So for submitting the changes we need to have the HWMOD api to be changed to enaforce the Autodile set only after the smart idle. > >> This applicable for omap4 as well. I don't have the >omap4430 pblic TRM to share. >> http://focus.ti.com/pdfs/wtbu/SWPU223D_Final_EPDF_06_07_2010.pdf >> >> 2. There is a Errata in OMAP3 errata #1.59 that If auto idle >is set then the USB can't wakeup the system by >> usb external events. The suggested workaround is to disable >the autoIdle for omap3. > >This one could be handled at init time in usb-musb.c by setting >HWMOD_NO_OCP_AUTOIDLE for the hwmod with a comment summarizing this >errata. > >Note also that Errata 1.166 is another one related to MUSB auto-idle >and we have a workaround in the PM branch to ensure that MUSB >auto-idle is disabled in the idle path since it may be re-enabled >after an off-mode transition. > I agree that this can be hanled using hw->flags for omap3. Can you please point me the commit for the above errata fix? I am successful in getting the exact commit. There are few observation in the USBOTG with respect to offmode and sysconfig settings. Still the investigations are in progress. Current mainline code for 3630 have a patch for the offmode as below. Enable the force ilde and force standby before going to offmode. Reset the sysconfig with smart standby and smartidle in restore path. With the current API support I will not be in a position to implement the this workaround. I think there are some more errata which needs the dynamic configuration of the sysconfig register like Set the force idle when data transfer is complete and set back to smart idle/smart standby during data transfer. I see the errata already in UART #2.15. Can you please let me know how do we meet such requirement using the existing APIs? ~Hema >Kevin > >> >>>-Original Message- >>>From: Cousson, Benoit >>>Sent: Thursday, June 17, 2010 3:04 PM >>>To: Kalliguddi, Hema; Kevin Hilman; Basak, Partha >>>Cc: p...@pwsan.com; Nayak, Rajendra; linux-omap@vger.kernel.org >>>Subject: RE: On the APIs for Enabling and Disabling Wakeup >capability. >>> >>>Hi Hema, >>> >>>>From: linux-omap-ow...@vger.kernel.org >>>> >>>>Kevin, >>>> >>>>There is no errata in the USB which needs the Enable/Disable >>>>wakeup to be done >>>>Seperately. If it can be handled with omap_devie_enable/idle >>>>Apis it is sufficient. >>>> >>>>But there is a need of setting the Auto idle bit seperately as >>>>for the USBOTG there is a need to set the Autoidle only after >>>>configuring the smart idle. It is recommended in the TRM not >>>>set the auto idle and smart idle together. >>>>This I discussed with Partha he sent a mail to you. >>>> >>>>For setting autoidle there is an api at hwmod layer but not >>>>yet omap device layer. >>>>Is there a plan to add API at omap device layer for >>>>enabling/disabling the autoidle? >>> >>>The whole issue with exposing all the low level stuff at driver level >>>is that the hwmod abstraction become less and less useful. >>>Drivers will start hacking with that for no good reason. And >>>then we will start adding again some omap version specific >>>hacks in the driver. >>> >>>Could you provide the full errata description of that USB_OTG >>>issue? Or at least that TRM part you are talking about. >&g
RE: On the APIs for Enabling and Disabling Wakeup capability.
Hi Benoit, I have 2 cases in usb for the need of separate API for setting the auto idle bit. 1. Below the link for omap3430TRM. Please refer 24.1.5.4.2 and 24.1.5.4.3 there is note not to set smart idle and autoidle bit simultaneously. Suggestion is to set the auto idle 0 before setting smart idle. Then set to 1. This applicable for omap4 as well. I don't have the omap4430 pblic TRM to share. http://focus.ti.com/pdfs/wtbu/SWPU223D_Final_EPDF_06_07_2010.pdf 2. There is a Errata in OMAP3 errata #1.59 that If auto idle is set then the USB can't wakeup the system by usb external events. The suggested workaround is to disable the autoIdle for omap3. Regards, Hema >-Original Message- >From: Cousson, Benoit >Sent: Thursday, June 17, 2010 3:04 PM >To: Kalliguddi, Hema; Kevin Hilman; Basak, Partha >Cc: p...@pwsan.com; Nayak, Rajendra; linux-omap@vger.kernel.org >Subject: RE: On the APIs for Enabling and Disabling Wakeup capability. > >Hi Hema, > >>From: linux-omap-ow...@vger.kernel.org >> >>Kevin, >> >>There is no errata in the USB which needs the Enable/Disable >>wakeup to be done >>Seperately. If it can be handled with omap_devie_enable/idle >>Apis it is sufficient. >> >>But there is a need of setting the Auto idle bit seperately as >>for the USBOTG there is a need to set the Autoidle only after >>configuring the smart idle. It is recommended in the TRM not >>set the auto idle and smart idle together. >>This I discussed with Partha he sent a mail to you. >> >>For setting autoidle there is an api at hwmod layer but not >>yet omap device layer. >>Is there a plan to add API at omap device layer for >>enabling/disabling the autoidle? > >The whole issue with exposing all the low level stuff at driver level >is that the hwmod abstraction become less and less useful. >Drivers will start hacking with that for no good reason. And >then we will start adding again some omap version specific >hacks in the driver. > >Could you provide the full errata description of that USB_OTG >issue? Or at least that TRM part you are talking about. > >Thanks, >Benoit > >>Regards, >>Hema >> >> >>> >Texas Instruments France SA, 821 Avenue Jack Kilby, 06270 >Villeneuve Loubet. 036 420 040 R.C.S Antibes. Capital de EUR 753.920 > >-Original Message- > >>>From: Kevin Hilman [mailto:khil...@deeprootsystems.com] >>>Sent: Thursday, June 17, 2010 5:56 AM >>>To: Basak, Partha >>>Cc: p...@pwsan.com; Kalliguddi, Hema; Nayak, Rajendra; >>>linux-omap@vger.kernel.org >>>Subject: Re: On the APIs for Enabling and Disabling Wakeup >capability. >>> >>>"Basak, Partha" writes: >>> >>>> I wanted to close on the introduction of two new OMAP device APIs >>>> omap_device_enable_wakeup () & omap_device_disable_wakeup() in >>>> omap_device layer. >>>> >>>> These APIs are potentially needed by the USB driver (via function >>>> pointers) to work around some USB erratum. >>>> >>>> Alternatively, can we call omap_hwmod_enable_wakeup() via function >>>> pointer? Is it agreeable to call these from driver code (via >>>> function pointers)in some special cases such as to handle some >>>> errata? >>> >>>Hi Partha, >>> >>>First, we need to dig up the Errata details for that USB problem to >>>better understand the USB-specific issue. >>> >>>In addition, Paul and I discussed the option of >automatically managing >>>the wakeup during the hwmod enable/idle, since there isn't really a >>>need to have the wakeup enabled when the hwmod is active. >>> >>>Do you see any disadvantages to that? That would be much >cleaner than >>>manually managing the wakeup feature per-driver. >>> >>>Kevin >>>-- >>To unsubscribe from this list: send the line "unsubscribe >>linux-omap" in >>the body of a message to majord...@vger.kernel.org >>More majordomo info at http://vger.kernel.org/majordomo-info.html >> >-- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: On the APIs for Enabling and Disabling Wakeup capability.
Kevin, Kevin, There is no errata in the USB which needs the Enable/Disable wakeup to be done Seperately. If it can be handled with omap_devie_enable/idle Apis it is sufficient. But there is a need of setting the Auto idle bit seperately as for the USBOTG there is a need to set the Autoidle only after configuring the smart idle. It is recommended in the TRM not set the auto idle and smart idle together. This I discussed with Partha he sent a mail to you. For setting autoidle there is an api at hwmod layer but not yet omap device layer. Is there a plan to add API at omap device layer for enabling/disabling the autoidle? Regards, Hema >-Original Message- >From: Kevin Hilman [mailto:khil...@deeprootsystems.com] >Sent: Thursday, June 17, 2010 5:56 AM >To: Basak, Partha >Cc: p...@pwsan.com; Kalliguddi, Hema; Nayak, Rajendra; >linux-omap@vger.kernel.org >Subject: Re: On the APIs for Enabling and Disabling Wakeup capability. > >"Basak, Partha" writes: > >> I wanted to close on the introduction of two new OMAP device APIs >> omap_device_enable_wakeup () & omap_device_disable_wakeup() in >> omap_device layer. >> >> These APIs are potentially needed by the USB driver (via function >> pointers) to work around some USB erratum. >> >> Alternatively, can we call omap_hwmod_enable_wakeup() via function >> pointer? Is it agreeable to call these from driver code (via >> function pointers)in some special cases such as to handle some >> errata? > >Hi Partha, > >First, we need to dig up the Errata details for that USB problem to >better understand the USB-specific issue. > >In addition, Paul and I discussed the option of automatically managing >the wakeup during the hwmod enable/idle, since there isn't really a >need to have the wakeup enabled when the hwmod is active. > >Do you see any disadvantages to that? That would be much cleaner than >manually managing the wakeup feature per-driver. > >Kevin >-- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 2/3] musb: populate board_data within musb structure
>-Original Message-
>From: linux-usb-ow...@vger.kernel.org
>[mailto:linux-usb-ow...@vger.kernel.org] On Behalf Of Gupta, Ajay Kumar
>Sent: Thursday, May 27, 2010 12:35 PM
>To: linux-...@vger.kernel.org
>Cc: linux-omap@vger.kernel.org; felipe.ba...@nokia.com;
>amit.kuche...@verdurent.com; khil...@deeprootsystems.com;
>Gupta, Ajay Kumar
>Subject: [PATCH 2/3] musb: populate board_data within musb structure
>
>Added board_data within musb as it would be required in
>musb_platform_exit() also to unregister the nop transceiver.
>
>Also changed the signature of musb_platform_init() as now
>board_data can be taken from musb itself.
>
>Signed-off-by: Ajay Kumar Gupta
>---
> drivers/usb/musb/blackfin.c |2 +-
> drivers/usb/musb/davinci.c |2 +-
> drivers/usb/musb/musb_core.c |3 ++-
> drivers/usb/musb/musb_core.h |3 ++-
> drivers/usb/musb/omap2430.c |4 ++--
> drivers/usb/musb/tusb6010.c |2 +-
> 6 files changed, 9 insertions(+), 7 deletions(-)
>
>diff --git a/drivers/usb/musb/blackfin.c b/drivers/usb/musb/blackfin.c
>index b611420..0bef011 100644
>--- a/drivers/usb/musb/blackfin.c
>+++ b/drivers/usb/musb/blackfin.c
>@@ -323,7 +323,7 @@ int musb_platform_set_mode(struct musb
>*musb, u8 musb_mode)
> return -EIO;
> }
>
>-int __init musb_platform_init(struct musb *musb, void *board_data)
>+int __init musb_platform_init(struct musb *musb)
> {
>
> /*
>diff --git a/drivers/usb/musb/davinci.c b/drivers/usb/musb/davinci.c
>index 5762436..ce2e16f 100644
>--- a/drivers/usb/musb/davinci.c
>+++ b/drivers/usb/musb/davinci.c
>@@ -376,7 +376,7 @@ int musb_platform_set_mode(struct musb
>*musb, u8 mode)
> return -EIO;
> }
>
>-int __init musb_platform_init(struct musb *musb, void *board_data)
>+int __init musb_platform_init(struct musb *musb)
> {
> void __iomem*tibase = musb->ctrl_base;
> u32 revision;
>diff --git a/drivers/usb/musb/musb_core.c
>b/drivers/usb/musb/musb_core.c
>index d3911ab..1ccc4d7 100644
>--- a/drivers/usb/musb/musb_core.c
>+++ b/drivers/usb/musb/musb_core.c
>@@ -1961,6 +1961,7 @@ bad_config:
> }
>
> spin_lock_init(&musb->lock);
>+ musb->board_data = plat->board_data;
> musb->board_mode = plat->mode;
> musb->board_set_power = plat->set_power;
> musb->set_clock = plat->set_clock;
I think musb structure already has a pointer to device structure which intern
has the platform_data pointer.
I think all of these member variables are not required. Might need to cleanup
the musb structre.
>@@ -1995,7 +1996,7 @@ bad_config:
>* isp1504, non-OTG, etc) mostly hooking up through ULPI.
>*/
> musb->isr = generic_interrupt;
>- status = musb_platform_init(musb, plat->board_data);
>+ status = musb_platform_init(musb);
> if (status < 0)
> goto fail2;
>
>diff --git a/drivers/usb/musb/musb_core.h
>b/drivers/usb/musb/musb_core.h
>index 7cef2b7..9dddaa4 100644
>--- a/drivers/usb/musb/musb_core.h
>+++ b/drivers/usb/musb/musb_core.h
>@@ -393,6 +393,7 @@ struct musb {
> int (*board_set_power)(int state);
>
> int (*set_clock)(struct clk *clk,
>int is_active);
>+ void*board_data;/* board
>specific data */
>
> u8 min_power; /* vbus for
>periph, in mA/2 */
>
>@@ -604,7 +605,7 @@ extern int
>musb_platform_get_vbus_status(struct musb *musb);
> #define musb_platform_get_vbus_status(x) 0
> #endif
>
>-extern int __init musb_platform_init(struct musb *musb, void
>*board_data);
>+extern int __init musb_platform_init(struct musb *musb);
> extern int musb_platform_exit(struct musb *musb);
>
> #endif/* __MUSB_CORE_H__ */
>diff --git a/drivers/usb/musb/omap2430.c b/drivers/usb/musb/omap2430.c
>index e06d65e..50591e7 100644
>--- a/drivers/usb/musb/omap2430.c
>+++ b/drivers/usb/musb/omap2430.c
>@@ -189,10 +189,10 @@ int musb_platform_set_mode(struct musb
>*musb, u8 musb_mode)
> return 0;
> }
>
>-int __init musb_platform_init(struct musb *musb, void *board_data)
>+int __init musb_platform_init(struct musb *musb)
> {
> u32 l;
>- struct omap_musb_board_data *data = board_data;
>+ struct omap_musb_board_data *data = musb->board_data;
>
> #if defined(CONFIG_ARCH_OMAP2430)
> omap_cfg_reg(AE5_2430_USB0HS_STP);
>diff --git a/drivers/usb/musb/tusb6010.c b/drivers/usb/musb/tusb6010.c
>index 05c077f..60d3938 100644
>--- a/drivers/usb/musb/tusb6010.c
>+++ b/drivers/usb/musb/tusb6010.c
>@@ -1104,7 +1104,7 @@ err:
> return -ENODEV;
> }
>
>-int __init musb_platform_init(struct musb *musb, void *board_data)
>+int __init musb_platform_init(struct musb *musb)
> {
> struct platform_device *pdev;
> struct resource *mem;
>--
>1.6.2.4
>
>--
>To unsubscribe from this list: send the line "unsubscribe linux-usb" in
>the body of a message to majord...@vger.kernel.org
>More majordomo info at http://vger.kern
RE: [PATCH 2/5] musb: use system DMA to fix Inventra DMA issue on RTL-1.4
Hi,
>-Original Message-
>From: Gupta, Ajay Kumar
>Sent: Thursday, May 13, 2010 3:41 PM
>To: Kalliguddi, Hema; Gadiyar, Anand; m...@felipebalbi.com
>Cc: linux-...@vger.kernel.org; linux-omap@vger.kernel.org
>Subject: RE: [PATCH 2/5] musb: use system DMA to fix Inventra
>DMA issue on RTL-1.4
>
>Hi,
>> >> > > Another approach to use PIO mode in opposite direction would
>> >> > > increase
>> >> the
>> >> > > cpu loading and thus using system DMA is preferred workaround.
>> >> > >
>> >> > > Signed-off-by: Anand Gadiyar
>> >> > > Signed-off-by: Ajay Kumar Gupta
>> >> >
>> >> > I think falling back to pio is better than this patch.
>> >At the cost of cpu, which certainly is not preferred.
>> >
>> >>> It will most likely be only one transfer.
>> >
>> >How about host mode with multiple devices connected and doing
>> >transfers?
>> >Falling back to PIO would kill the cpu.
>> >
>> >>> Another approach is to allocate dma channels on a transfer basis.
>> >
>> >Can you elaborate this?
>>
>> It might be good idea to allocate the dma channels on
>tarnsfer basis as
>> Felipe
>> Suggested. The musb driver allocates dma channels for the
>first 8 enabled
>> endpoints and higher endpoints works in PIO mode.
>> For system dma if there are more nummber of Rx endpoints
>enabled but not
>> used for data
>> Transfer you might end up having many sdma channles
>allocated biut not
>> used which will
>> Impact in the system of not utilizing the sdma channels effectively.
>
>Felipe, Is this what you meant? If so then I have a patch
>(copied below)
>to fix this and I can post this one along with others.
>
>-- cut here -
>Currently DMA channels are allocated and they remain allocated
>even if there is no active data transfer. Added channel_release()
>whenever there is no pending request.
>
>Signed-off-by: Ajay Kumar Gupta
>---
> drivers/usb/musb/musb_gadget.c | 27 +--
> drivers/usb/musb/musb_host.c | 14 --
> 2 files changed, 33 insertions(+), 8 deletions(-)
>
>diff --git a/drivers/usb/musb/musb_gadget.c
>b/drivers/usb/musb/musb_gadget.c
>index fd842af..477a009 100644
>--- a/drivers/usb/musb/musb_gadget.c
>+++ b/drivers/usb/musb/musb_gadget.c
>@@ -297,9 +297,13 @@ static void txstate(struct musb *musb,
>struct musb_request *req)
> csr);
>
> #ifndef CONFIG_MUSB_PIO_ONLY
>- if (is_dma_capable() && musb_ep->dma) {
>+
>+ if (is_dma_capable() && musb->dma_controller) {
> struct dma_controller *c = musb->dma_controller;
>
>+ if (!musb_ep->dma)
>+ musb_ep->dma = c->channel_alloc(c,
>musb_ep->hw_ep, 1);
If the channel_alloc returns failure then there should be a way to fallback to
PIO mode.
I am posting a patch for dynamic dma channel allocation for mentor dma for
gadget driver.
~Hema
>+
> use_dma = (request->dma != DMA_ADDR_INVALID);
>
> /* MUSB_TXCSR_P_ISO is still set correctly */
>@@ -433,6 +437,7 @@ void musb_g_tx(struct musb *musb, u8 epnum)
> u8 __iomem *mbase = musb->mregs;
> struct musb_ep *musb_ep =
>&musb->endpoints[epnum].ep_in;
> void __iomem*epio = musb->endpoints[epnum].regs;
>+ struct dma_controller *c = musb->dma_controller;
> struct dma_channel *dma;
>
> musb_ep_select(mbase, epnum);
>@@ -535,6 +540,10 @@ void musb_g_tx(struct musb *musb, u8 epnum)
> if (!request) {
> DBG(4, "%s idle now\n",
> musb_ep->end_point.name);
>+ if (musb_ep->dma) {
>+
>c->channel_release(musb_ep->dma);
>+ musb_ep->dma = NULL;
>+ }
> return;
> }
> }
>@@ -585,6 +594,7 @@ static void rxstate(struct musb *musb,
>struct musb_request *req)
> struct usb_request *request = &req->request;
> struct musb_ep *musb_ep =
>&musb->endpoints[epnum].ep_out;
> void __iomem*epio = musb->endpoints[epnum].regs;
>+ struct dma_controller *c = musb->dma_controller;
> unsigned
RE: [PATCH 2/5] musb: use system DMA to fix Inventra DMA issue on RTL-1.4
Hi, >-Original Message- >From: linux-usb-ow...@vger.kernel.org >[mailto:linux-usb-ow...@vger.kernel.org] On Behalf Of Gupta, Ajay Kumar >Sent: Thursday, May 13, 2010 9:53 AM >To: Gadiyar, Anand; m...@felipebalbi.com >Cc: linux-...@vger.kernel.org; linux-omap@vger.kernel.org >Subject: RE: [PATCH 2/5] musb: use system DMA to fix Inventra >DMA issue on RTL-1.4 > >Hi, >>Subject: RE: [PATCH 2/5] musb: use system DMA to fix Inventra >DMA issue >>on >> RTL-1.4 > >Felipe, > >> > > Another approach to use PIO mode in opposite direction would >> > > increase >> the >> > > cpu loading and thus using system DMA is preferred workaround. >> > > >> > > Signed-off-by: Anand Gadiyar >> > > Signed-off-by: Ajay Kumar Gupta >> > >> > I think falling back to pio is better than this patch. >At the cost of cpu, which certainly is not preferred. > >>> It will most likely be only one transfer. > >How about host mode with multiple devices connected and doing >transfers? >Falling back to PIO would kill the cpu. > >>> Another approach is to allocate dma channels on a transfer basis. > >Can you elaborate this? It might be good idea to allocate the dma channels on tarnsfer basis as Felipe Suggested. The musb driver allocates dma channels for the first 8 enabled endpoints and higher endpoints works in PIO mode. For system dma if there are more nummber of Rx endpoints enabled but not used for data Transfer you might end up having many sdma channles allocated biut not used which will Impact in the system of not utilizing the sdma channels effectively. ~Hema >>> This is way too big of a problem. > > >If we think of the scenarios in host mode then certainly using >system DMA is best approach. > >-Ajay >> Which one is better depends on how many endpoints are doing, >transfers >> in parallel, I suppose. >> >> I did post the a patch for the other approach (to fall back to PIO). >> I haven't received a response to that yet. I'm okay with >either approach. >> >> >> - Anand >-- >To unsubscribe from this list: send the line "unsubscribe >linux-usb" in the body of a message to >majord...@vger.kernel.org More majordomo info at >http://vger.kernel.org/majordomo-info.html >-- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
