Re: [PATCH v1 01/11] drivers: usb: otg: add a new driver for omap usb2 phy
Hi,
On Tue, Jul 10, 2012 at 11:16 AM, Rajendra Nayak rna...@ti.com wrote:
diff --git a/Documentation/devicetree/bindings/usb/omap-usb.txt
b/Documentation/devicetree/bindings/usb/omap-usb.txt
new file mode 100644
index 000..80a28c9
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/omap-usb.txt
@@ -0,0 +1,16 @@
+OMAP USB PHY
+
+OMAP USB2 PHY
+
+Required properties:
+ - compatible: Should be ti,omap-usb2
+ - reg : Address and length of the register set for the device. Also
+add the address of control module dev conf register until a driver for
+control module is added
+
+This is usually a subnode of ocp2scp to which it is connected.
+
+usb2phy@0x4a0ad080 {
+ compatible = ti,omap-usb2;
+ reg =0x4a0ad080 0x58;
Don;t you need a 'ti,hwmods' entry for this one?
I don't think it needs one as it has nothing more than this one
address space. (it doesn't have sysconfig, doesn't have any prcm
register..).
--- /dev/null
+++ b/drivers/usb/otg/omap-usb2.c
@@ -0,0 +1,273 @@
+/*
+ * omap-usb2.c - USB PHY, talking to musb controller in OMAP.
+ *
+ * Copyright (C) 2011 Texas Instruments Incorporated - http://www.ti.com
Copyright (C) 2012? Same for the couple of headers below.
Will fix it.
+ * 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: Kishon Vijay Abraham Ikis...@ti.com
+ *
+ * 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.
+ *
+ */
+
+
+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.
+ 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?
+
+ 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 =
Re: [PATCH v1 01/11] drivers: usb: otg: add a new driver for omap usb2 phy
Hi,
On Tue, Jul 10, 2012 at 11:33 AM, Venu Byravarasu
vbyravar...@nvidia.com wrote:
+
+#ifdef CONFIG_PM
Should it not be CONFIG_PM_SLEEP instead of just CONFIG_PM?
Why? I think we should have CONFIG_PM_SLEEP only when we have
*suspend*, *resume* hooks. But this driver has only *runtime_suspend*
and *runtime_resume* hooks.
+
+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);
+
+static int __init usb2_omap_init(void)
+{
+ return platform_driver_register(omap_usb2_driver);
+}
+arch_initcall(usb2_omap_init);
+
+static void __exit usb2_omap_exit(void)
+{
+ platform_driver_unregister(omap_usb2_driver);
+}
+module_exit(usb2_omap_exit);
+
Do you really need arch_initcall here?
If not, then you can replace above two function calls with
module_platform_driver().
I indeed want it to be arch_initcall. When the module is built-in, I
want this module to loaded before twl6030-usb.c
Thanks
Kishon
--
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
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-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v1 01/11] drivers: usb: otg: add a new driver for omap usb2 phy
On Tuesday 10 July 2012 12:18 PM, ABRAHAM, KISHON VIJAY wrote: Hi, On Tue, Jul 10, 2012 at 11:33 AM, Venu Byravarasu vbyravar...@nvidia.com wrote: + +#ifdef CONFIG_PM Should it not be CONFIG_PM_SLEEP instead of just CONFIG_PM? Why? I think we should have CONFIG_PM_SLEEP only when we have *suspend*, *resume* hooks. But this driver has only *runtime_suspend* and *runtime_resume* hooks. CONFIG_PM_RUNTIME maybe then? -- 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
Re: [PATCH v1 01/11] drivers: usb: otg: add a new driver for omap usb2 phy
diff --git a/Documentation/devicetree/bindings/usb/omap-usb.txt
b/Documentation/devicetree/bindings/usb/omap-usb.txt
new file mode 100644
index 000..80a28c9
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/omap-usb.txt
@@ -0,0 +1,16 @@
+OMAP USB PHY
+
+OMAP USB2 PHY
+
+Required properties:
+ - compatible: Should be ti,omap-usb2
+ - reg : Address and length of the register set for the device. Also
+add the address of control module dev conf register until a driver for
+control module is added
+
+This is usually a subnode of ocp2scp to which it is connected.
+
+usb2phy@0x4a0ad080 {
+ compatible = ti,omap-usb2;
+ reg =0x4a0ad080 0x58;
Don;t you need a 'ti,hwmods' entry for this one?
--- /dev/null
+++ b/drivers/usb/otg/omap-usb2.c
@@ -0,0 +1,273 @@
+/*
+ * omap-usb2.c - USB PHY, talking to musb controller in OMAP.
+ *
+ * Copyright (C) 2011 Texas Instruments Incorporated - http://www.ti.com
Copyright (C) 2012? Same for the couple of headers below.
+ * 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: Kishon Vijay Abraham Ikis...@ti.com
+ *
+ * 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.
+ *
+ */
+
+
+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?
+ 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?
+
+ 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
