lkml.org 
[lkml]   [2012]   [Jul]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [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



\
 
 \ /
  Last update: 2012-07-10 09:21    [W:3.526 / U:0.004 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site