[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <4FFBD060.5010903@ti.com>
Date: Tue, 10 Jul 2012 12:19:04 +0530
From: Rajendra Nayak <rnayak@...com>
To: "ABRAHAM, KISHON VIJAY" <kishon@...com>
CC: grant.likely@...retlab.ca, rob.herring@...xeda.com,
rob@...dley.net, linux@....linux.org.uk,
gregkh@...uxfoundation.org, b-cousson@...com, tony@...mide.com,
devicetree-discuss@...ts.ozlabs.org, linux-doc@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
balbi@...com, linux-omap@...r.kernel.org,
linux-usb@...r.kernel.org, hvaibhav@...com
Subject: Re: [PATCH v1 01/11] drivers: usb: otg: add a new driver for omap
usb2 phy
>>
>>> +
>>> +static int __devinit omap_usb2_probe(struct platform_device *pdev)
>>> +{
>>> + struct omap_usb *phy;
>>> + struct usb_otg *otg;
>>> + struct resource *res;
>>> +
>>> + phy = devm_kzalloc(&pdev->dev, sizeof(*phy), GFP_KERNEL);
>>> + if (!phy) {
>>> + dev_err(&pdev->dev, "unable to allocate memory for USB2
>>> PHY\n");
>>> + return -ENOMEM;
>>> + }
>>> +
>>> + otg = devm_kzalloc(&pdev->dev, sizeof(*otg), GFP_KERNEL);
>>> + if (!otg) {
>>> + dev_err(&pdev->dev, "unable to allocate memory for USB
>>> OTG\n");
>>> + return -ENOMEM;
>>> + }
>>> +
>>> + phy->dev =&pdev->dev;
>>>
>>> +
>>> + phy->phy.dev = phy->dev;
>>> + phy->phy.label = "omap-usb2";
>>> + phy->phy.set_suspend = omap_usb2_suspend;
>>> + phy->phy.otg = otg;
>>> +
>>> + res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
>>> +
>>> + phy->control_dev_conf = devm_request_and_ioremap(&pdev->dev, res);
>>> + if (phy->control_dev_conf == NULL) {
>>> + dev_err(&pdev->dev, "Failed to obtain io memory\n");
>>> + return -ENXIO;
>>> + }
>>> +
>>> + phy->is_suspended = 1;
>>> + omap_usb_phy_power(phy, 0);
>>> +
>>> + otg->set_host = omap_usb_set_host;
>>> + otg->set_peripheral = omap_usb_set_peripheral;
>>> + otg->set_vbus = omap_usb_set_vbus;
>>> + otg->start_srp = omap_usb_start_srp;
>>> + otg->phy =&phy->phy;
>>>
>>> +
>>> + phy->wkupclk = devm_clk_get(phy->dev, "usb_phy_cm_clk32k");
>>
>>
>> Why not just use clk_get()? What does devm_clk_get() do?
> It just associates the clk with the device. So whenever the the driver
> gets detached, the devres will take care to do a clk_put() of the
> clock.
ok, makes sense.
>>
>>
>>> + if (IS_ERR(phy->wkupclk)) {
>>> + dev_err(&pdev->dev, "unable to get usb_phy_cm_clk32k\n");
>>> + return PTR_ERR(phy->wkupclk);
>>> + }
>>> + clk_prepare(phy->wkupclk);
>>
>>
>> Ideally clk_prepare() is an extension of clk_enable() and is expected
>> to be used that way. Not to be clubbed with clk_get(). Same with
>> clk_unprepare(). Do you do a clk_enable()/_disable() in interrupt/
>> atomic context?
>
> Currently it is called from a work queue. But Felipe wanted to remove
> those work_queue from omap2430 glue. Then this would be called from
> atomic context.
> A query for you here. If pm_runtime_get_sync() is called in interrupt
> context, will runtime resume of that device will also be called in the
> same context?
Yes, it would. You also need to then tell the runtime pm framework about
it by calling a pm_runtime_irq_safe() api I guess.
regards,
Rajendra
>
>>
>>
>>> +
>>> + usb_add_phy(&phy->phy, USB_PHY_TYPE_USB2);
>>> +
>>> + platform_set_drvdata(pdev, phy);
>>> +
>>> + pm_runtime_enable(phy->dev);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int __devexit omap_usb2_remove(struct platform_device *pdev)
>>> +{
>>> + struct omap_usb *phy = platform_get_drvdata(pdev);
>>> +
>>> + clk_unprepare(phy->wkupclk);
>>> + usb_remove_phy(&phy->phy);
>>> + platform_set_drvdata(pdev, NULL);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +#ifdef CONFIG_PM
>>> +
>>> +static int omap_usb2_runtime_suspend(struct device *dev)
>>> +{
>>> + struct platform_device *pdev = to_platform_device(dev);
>>> + struct omap_usb *phy = platform_get_drvdata(pdev);
>>> +
>>> + clk_disable(phy->wkupclk);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int omap_usb2_runtime_resume(struct device *dev)
>>> +{
>>> + struct platform_device *pdev = to_platform_device(dev);
>>> + struct omap_usb *phy = platform_get_drvdata(pdev);
>>> +
>>> + clk_enable(phy->wkupclk);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static const struct dev_pm_ops omap_usb2_pm_ops = {
>>> + SET_RUNTIME_PM_OPS(omap_usb2_runtime_suspend,
>>> omap_usb2_runtime_resume,
>>> + NULL)
>>> +};
>>> +
>>> +#define DEV_PM_OPS (&omap_usb2_pm_ops)
>>> +#else
>>> +#define DEV_PM_OPS NULL
>>> +#endif
>>> +
>>> +#ifdef CONFIG_OF
>>> +static const struct of_device_id omap_usb2_id_table[] = {
>>> + { .compatible = "ti,omap-usb2" },
>>> + {}
>>> +};
>>> +MODULE_DEVICE_TABLE(of, omap_usb2_id_table);
>>> +#else
>>> +#define omap_usb2_id_table NULL;
>>> +#endif
>>> +
>>> +static struct platform_driver omap_usb2_driver = {
>>> + .probe = omap_usb2_probe,
>>> + .remove = __devexit_p(omap_usb2_remove),
>>> + .driver = {
>>> + .name = "omap-usb2",
>>> + .owner = THIS_MODULE,
>>> + .pm = DEV_PM_OPS,
>>> + .of_match_table = omap_usb2_id_table,
>>
>>
>> Use of_match_ptr() instead.
>
> Ok. And I'll remove #define omap_usb2_id_table NULL;.
>
> Thanks
> Kishon
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists