On Mon, May 9, 2011 at 3:06 PM, Felipe Balbi <[email protected]> wrote:
> Hi,
>
> On Mon, May 09, 2011 at 02:40:03PM +0530, Keshava Munegowda wrote:
>> From: Keshava Munegowda <[email protected]>
>>
>> the disabling of clocks and freeing GPIO are changed
>> to fix the occurence of the crash of rmmod of ehci and ohci
>> drivers
>>
>> Signed-off-by: Keshava Munegowda <[email protected]>
>
> NAK
>
>> ---
>> drivers/mfd/omap-usb-host.c | 24 ++++++++++++++++--------
>> 1 files changed, 16 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/mfd/omap-usb-host.c b/drivers/mfd/omap-usb-host.c
>> index 3ab9ffa..cda0797 100644
>> --- a/drivers/mfd/omap-usb-host.c
>> +++ b/drivers/mfd/omap-usb-host.c
>> @@ -939,6 +939,7 @@ static void usbhs_disable(struct device *dev)
>> struct usbhs_hcd_omap *omap = dev_get_drvdata(dev);
>> struct usbhs_omap_platform_data *pdata = &omap->platdata;
>> unsigned long flags = 0;
>> + unsigned int halt = 0;
>
> you shouldn't need this.
>
>> @@ -994,24 +995,31 @@ static void usbhs_disable(struct device *dev)
>> dev_dbg(dev, "operation timed out\n");
>> }
>>
>> - if (pdata->ehci_data->phy_reset) {
>> - if (gpio_is_valid(pdata->ehci_data->reset_gpio_port[0]))
>> - gpio_free(pdata->ehci_data->reset_gpio_port[0]);
>> -
>> - if (gpio_is_valid(pdata->ehci_data->reset_gpio_port[1]))
>> - gpio_free(pdata->ehci_data->reset_gpio_port[1]);
>> + if (is_omap_usbhs_rev2(omap)) {
>> + if (is_ehci_tll_mode(pdata->port_mode[0]))
>> + clk_enable(omap->usbtll_p1_fck);
>> + if (is_ehci_tll_mode(pdata->port_mode[1]))
>> + clk_enable(omap->usbtll_p2_fck);
>> + clk_disable(omap->utmi_p2_fck);
>> + clk_disable(omap->utmi_p1_fck);
>> }
>>
>> - clk_disable(omap->utmi_p2_fck);
>> - clk_disable(omap->utmi_p1_fck);
>> clk_disable(omap->usbtll_ick);
>> clk_disable(omap->usbtll_fck);
>> clk_disable(omap->usbhost_fs_fck);
>> clk_disable(omap->usbhost_hs_fck);
>> clk_disable(omap->usbhost_ick);
>> + halt = 1;
>
> at least from this diff, you're always reaching that part of the code
> rendering this halt flag useless.
I you see only the patch ; its looks like variable halt is not needed;
If the code; it will be set only when the clocks are disabled;
Then after restoring irq, you will free the gpio based on this value.
The complete usbhs_disable code is follows:
static void usbhs_disable(struct device *dev)
{
struct usbhs_hcd_omap *omap = dev_get_drvdata(dev);
struct usbhs_omap_platform_data *pdata = &omap->platdata;
unsigned long flags = 0;
unsigned int halt = 0;
unsigned long timeout;
dev_dbg(dev, "stopping TI HSUSB Controller\n");
spin_lock_irqsave(&omap->lock, flags);
if (omap->count == 0)
goto end_disble;
omap->count--;
if (omap->count != 0)
goto end_disble;
/* Reset OMAP modules for insmod/rmmod to work */
usbhs_write(omap->uhh_base, OMAP_UHH_SYSCONFIG,
is_omap_usbhs_rev2(omap) ?
OMAP4_UHH_SYSCONFIG_SOFTRESET :
OMAP_UHH_SYSCONFIG_SOFTRESET);
timeout = jiffies + msecs_to_jiffies(100);
while (!(usbhs_read(omap->uhh_base, OMAP_UHH_SYSSTATUS)
& (1 << 0))) {
cpu_relax();
if (time_after(jiffies, timeout))
dev_dbg(dev, "operation timed out\n");
}
while (!(usbhs_read(omap->uhh_base, OMAP_UHH_SYSSTATUS)
& (1 << 1))) {
cpu_relax();
if (time_after(jiffies, timeout))
dev_dbg(dev, "operation timed out\n");
}
while (!(usbhs_read(omap->uhh_base, OMAP_UHH_SYSSTATUS)
& (1 << 2))) {
cpu_relax();
if (time_after(jiffies, timeout))
dev_dbg(dev, "operation timed out\n");
}
usbhs_write(omap->tll_base, OMAP_USBTLL_SYSCONFIG, (1 << 1));
while (!(usbhs_read(omap->tll_base, OMAP_USBTLL_SYSSTATUS)
& (1 << 0))) {
cpu_relax();
if (time_after(jiffies, timeout))
dev_dbg(dev, "operation timed out\n");
}
if (is_omap_usbhs_rev2(omap)) {
if (is_ehci_tll_mode(pdata->port_mode[0]))
clk_enable(omap->usbtll_p1_fck);
if (is_ehci_tll_mode(pdata->port_mode[1]))
clk_enable(omap->usbtll_p2_fck);
clk_disable(omap->utmi_p2_fck);
clk_disable(omap->utmi_p1_fck);
}
clk_disable(omap->usbtll_ick);
clk_disable(omap->usbtll_fck);
clk_disable(omap->usbhost_fs_fck);
clk_disable(omap->usbhost_hs_fck);
clk_disable(omap->usbhost_ick);
halt = 1;
end_disble:
spin_unlock_irqrestore(&omap->lock, flags);
if (halt && pdata->ehci_data->phy_reset) {
if (gpio_is_valid(pdata->ehci_data->reset_gpio_port[0]))
gpio_free(pdata->ehci_data->reset_gpio_port[0]);
if (gpio_is_valid(pdata->ehci_data->reset_gpio_port[1]))
gpio_free(pdata->ehci_data->reset_gpio_port[1]);
}
}
>
> --
> balbi
>
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
