On Thursday 12 September 2013 04:49 PM, Roger Quadros wrote:
Hi Kishon, On 09/02/2013 06:43 PM, Kishon Vijay Abraham I wrote:Adapted omap-usb3 PHY driver to Generic PHY Framework and moved phy-omap-usb3 driver in drivers/usb/phy to drivers/phy and also renamed the file to phy-omap-pipe3 since this same driver will be used for SATA PHY and PCIE PHY.I would suggest to split the renaming and PHY adaptation into 2 separate patches.
Alright.
Signed-off-by: Kishon Vijay Abraham I <kishon@xxxxxx> --- Documentation/devicetree/bindings/usb/usb-phy.txt | 5 +- drivers/phy/Kconfig | 10 + drivers/phy/Makefile | 1 + .../phy/phy-omap-usb3.c => phy/phy-omap-pipe3.c} | 206 +++++++++++---------how about naming it to phy-ti-pipe3.c as it is used on OMAP as well as non-OMAP e.g. DRA7.
hmm.. I thought it would be consistent with other PHY drivers (phy-omap-usb2). Moreover DRA7 is OMAP based platform ;-) Maybe we should reserve that for later?
drivers/usb/phy/Kconfig | 11 -- drivers/usb/phy/Makefile | 1 - include/linux/phy/omap_pipe3.h | 52 +++++ 7 files changed, 177 insertions(+), 109 deletions(-) rename drivers/{usb/phy/phy-omap-usb3.c => phy/phy-omap-pipe3.c} (57%) create mode 100644 include/linux/phy/omap_pipe3.h diff --git a/Documentation/devicetree/bindings/usb/usb-phy.txt b/Documentation/devicetree/bindings/usb/usb-phy.txt index c0245c8..36bdb17 100644 --- a/Documentation/devicetree/bindings/usb/usb-phy.txt +++ b/Documentation/devicetree/bindings/usb/usb-phy.txt @@ -21,10 +21,11 @@ usb2phy@4a0ad080 { #phy-cells = <0>; }; -OMAP USB3 PHY +OMAP PIPE3 PHY Required properties: - - compatible: Should be "ti,omap-usb3" + - compatible: Should be "ti,omap-usb3", "ti,omap-pipe3", "ti,omap-sata" + or "ti,omap-pcie" - reg : Address and length of the register set for the device. - reg-names: The names of the register addresses corresponding to the registers filled in "reg". diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig index ac239ac..5c2e7a0 100644 --- a/drivers/phy/Kconfig +++ b/drivers/phy/Kconfig @@ -27,6 +27,16 @@ config OMAP_USB2 The USB OTG controller communicates with the comparator using this driver. +config OMAP_PIPE3 + tristate "OMAP PIPE3 PHY Driver" + select GENERIC_PHY + select OMAP_CONTROL_USBhow about depends on OMAP_CONTROL_USB
From whatever I could make out from comments of Greg in my Generic PHY Framework, it's better to do a select of dependent modules instead of depends on.
Also, if this is TI/OMAP it might as well depend on ARCH_OMAP.+ help + Enable this to support the PIPE3 PHY that is part of SOC. Thisworth mentioning TI OMAP/DRA SoCs.
right.
+ driver takes care of all the PHY functionality apart from comparator. + This driver interacts with the "OMAP Control PHY Driver" to power + on/off the PHY. + config TWL4030_USB tristate "TWL4030 USB Transceiver Driver" depends on TWL4030_CORE && REGULATOR_TWL4030 && USB_MUSB_OMAP2PLUS diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile index 0dd8a98..48bf9f2 100644 --- a/drivers/phy/Makefile +++ b/drivers/phy/Makefile @@ -4,4 +4,5 @@ obj-$(CONFIG_GENERIC_PHY) += phy-core.o obj-$(CONFIG_OMAP_USB2) += phy-omap-usb2.o +obj-$(CONFIG_OMAP_PIPE3) += phy-omap-pipe3.o obj-$(CONFIG_TWL4030_USB) += phy-twl4030-usb.o diff --git a/drivers/usb/phy/phy-omap-usb3.c b/drivers/phy/phy-omap-pipe3.c similarity index 57% rename from drivers/usb/phy/phy-omap-usb3.c rename to drivers/phy/phy-omap-pipe3.c index 4004f82..ee9a901 100644 --- a/drivers/usb/phy/phy-omap-usb3.c +++ b/drivers/phy/phy-omap-pipe3.c @@ -1,5 +1,5 @@ /* - * omap-usb3 - USB PHY, talking to dwc3 controller in OMAP. + * omap-pipe3 - PHY driver for SATA, USB and PCIE in OMAP platforms * * Copyright (C) 2013 Texas Instruments Incorporated - http://www.ti.com * This program is free software; you can redistribute it and/or modify @@ -19,7 +19,8 @@ #include <linux/module.h> #include <linux/platform_device.h> #include <linux/slab.h> -#include <linux/usb/omap_usb.h> +#include <linux/phy/omap_pipe3.h> +#include <linux/phy/phy.h> #include <linux/of.h> #include <linux/clk.h> #include <linux/err.h> @@ -52,17 +53,17 @@ /* * This is an Empirical value that works, need to confirm the actual - * value required for the USB3PHY_PLL_CONFIGURATION2.PLL_IDLE status - * to be correctly reflected in the USB3PHY_PLL_STATUS register. + * value required for the PIPE3PHY_PLL_CONFIGURATION2.PLL_IDLE status + * to be correctly reflected in the PIPE3PHY_PLL_STATUS register. */ # define PLL_IDLE_TIME 100; -struct usb_dpll_map { +struct pipe3_dpll_map { unsigned long rate; - struct usb_dpll_params params; + struct pipe3_dpll_params params; }; -static struct usb_dpll_map dpll_map[] = { +static struct pipe3_dpll_map dpll_map[] = { {12000000, {1250, 5, 4, 20, 0} }, /* 12 MHz */ {16800000, {3125, 20, 4, 20, 0} }, /* 16.8 MHz */ {19200000, {1172, 8, 4, 20, 65537} }, /* 19.2 MHz */ @@ -71,7 +72,7 @@ static struct usb_dpll_map dpll_map[] = { {38400000, {3125, 47, 4, 20, 92843} }, /* 38.4 MHz */ }; -static struct usb_dpll_params *omap_usb3_get_dpll_params(unsigned long rate) +static struct pipe3_dpll_params *omap_pipe3_get_dpll_params(unsigned long rate) { int i; @@ -83,110 +84,113 @@ static struct usb_dpll_params *omap_usb3_get_dpll_params(unsigned long rate) return 0; } -static int omap_usb3_suspend(struct usb_phy *x, int suspend) +static int omap_pipe3_power_off(struct phy *x) { - struct omap_usb *phy = phy_to_omapusb(x); - int val; + struct omap_pipe3 *phy = phy_get_drvdata(x); + int val; int timeout = PLL_IDLE_TIME; - if (suspend && !phy->is_suspended) { - val = omap_usb_readl(phy->pll_ctrl_base, PLL_CONFIGURATION2); - val |= PLL_IDLE; - omap_usb_writel(phy->pll_ctrl_base, PLL_CONFIGURATION2, val); - - do { - val = omap_usb_readl(phy->pll_ctrl_base, PLL_STATUS); - if (val & PLL_TICOPWDN) - break; - udelay(1); - } while (--timeout); - - omap_control_usb_phy_power(phy->control_dev, 0); - - phy->is_suspended = 1; - } else if (!suspend && phy->is_suspended) { - phy->is_suspended = 0; - - val = omap_usb_readl(phy->pll_ctrl_base, PLL_CONFIGURATION2); - val &= ~PLL_IDLE; - omap_usb_writel(phy->pll_ctrl_base, PLL_CONFIGURATION2, val); - - do { - val = omap_usb_readl(phy->pll_ctrl_base, PLL_STATUS); - if (!(val & PLL_TICOPWDN)) - break; - udelay(1); - } while (--timeout); - } + val = omap_pipe3_readl(phy->pll_ctrl_base, PLL_CONFIGURATION2); + val |= PLL_IDLE; + omap_pipe3_writel(phy->pll_ctrl_base, PLL_CONFIGURATION2, val); + + do { + val = omap_pipe3_readl(phy->pll_ctrl_base, PLL_STATUS); + if (val & PLL_TICOPWDN) + break; + udelay(1);Is it better to sleep instead of udelay?
hmm.. yeah, I guess this function wont be called from interrupt context.
+ } while (--timeout); + + omap_control_usb_phy_power(phy->control_dev, 0); + + return 0; +} + +static int omap_pipe3_power_on(struct phy *x) +{ + struct omap_pipe3 *phy = phy_get_drvdata(x); + int val; + int timeout = PLL_IDLE_TIME; + + val = omap_pipe3_readl(phy->pll_ctrl_base, PLL_CONFIGURATION2); + val &= ~PLL_IDLE; + omap_pipe3_writel(phy->pll_ctrl_base, PLL_CONFIGURATION2, val); + + do { + val = omap_pipe3_readl(phy->pll_ctrl_base, PLL_STATUS); + if (!(val & PLL_TICOPWDN)) + break; + udelay(1);here too.
Ok.
+ } while (--timeout); return 0; } -static void omap_usb_dpll_relock(struct omap_usb *phy) +static void omap_pipe3_dpll_relock(struct omap_pipe3 *phy) { u32 val; unsigned long timeout; - omap_usb_writel(phy->pll_ctrl_base, PLL_GO, SET_PLL_GO); + omap_pipe3_writel(phy->pll_ctrl_base, PLL_GO, SET_PLL_GO); timeout = jiffies + msecs_to_jiffies(20); do { - val = omap_usb_readl(phy->pll_ctrl_base, PLL_STATUS); + val = omap_pipe3_readl(phy->pll_ctrl_base, PLL_STATUS); if (val & PLL_LOCK) break; } while (!WARN_ON(time_after(jiffies, timeout))); } -static int omap_usb_dpll_lock(struct omap_usb *phy) +static int omap_pipe3_dpll_lock(struct omap_pipe3 *phy) { u32 val; unsigned long rate; - struct usb_dpll_params *dpll_params; + struct pipe3_dpll_params *dpll_params; rate = clk_get_rate(phy->sys_clk); - dpll_params = omap_usb3_get_dpll_params(rate); + dpll_params = omap_pipe3_get_dpll_params(rate); if (!dpll_params) { dev_err(phy->dev, "No DPLL configuration for %lu Hz SYS CLK\n", rate); return -EINVAL; } - val = omap_usb_readl(phy->pll_ctrl_base, PLL_CONFIGURATION1); + val = omap_pipe3_readl(phy->pll_ctrl_base, PLL_CONFIGURATION1); val &= ~PLL_REGN_MASK; val |= dpll_params->n << PLL_REGN_SHIFT; - omap_usb_writel(phy->pll_ctrl_base, PLL_CONFIGURATION1, val); + omap_pipe3_writel(phy->pll_ctrl_base, PLL_CONFIGURATION1, val); - val = omap_usb_readl(phy->pll_ctrl_base, PLL_CONFIGURATION2); + val = omap_pipe3_readl(phy->pll_ctrl_base, PLL_CONFIGURATION2); val &= ~PLL_SELFREQDCO_MASK; val |= dpll_params->freq << PLL_SELFREQDCO_SHIFT; - omap_usb_writel(phy->pll_ctrl_base, PLL_CONFIGURATION2, val); + omap_pipe3_writel(phy->pll_ctrl_base, PLL_CONFIGURATION2, val); - val = omap_usb_readl(phy->pll_ctrl_base, PLL_CONFIGURATION1); + val = omap_pipe3_readl(phy->pll_ctrl_base, PLL_CONFIGURATION1); val &= ~PLL_REGM_MASK; val |= dpll_params->m << PLL_REGM_SHIFT; - omap_usb_writel(phy->pll_ctrl_base, PLL_CONFIGURATION1, val); + omap_pipe3_writel(phy->pll_ctrl_base, PLL_CONFIGURATION1, val); - val = omap_usb_readl(phy->pll_ctrl_base, PLL_CONFIGURATION4); + val = omap_pipe3_readl(phy->pll_ctrl_base, PLL_CONFIGURATION4); val &= ~PLL_REGM_F_MASK; val |= dpll_params->mf << PLL_REGM_F_SHIFT; - omap_usb_writel(phy->pll_ctrl_base, PLL_CONFIGURATION4, val); + omap_pipe3_writel(phy->pll_ctrl_base, PLL_CONFIGURATION4, val); - val = omap_usb_readl(phy->pll_ctrl_base, PLL_CONFIGURATION3); + val = omap_pipe3_readl(phy->pll_ctrl_base, PLL_CONFIGURATION3); val &= ~PLL_SD_MASK; val |= dpll_params->sd << PLL_SD_SHIFT; - omap_usb_writel(phy->pll_ctrl_base, PLL_CONFIGURATION3, val); + omap_pipe3_writel(phy->pll_ctrl_base, PLL_CONFIGURATION3, val); - omap_usb_dpll_relock(phy); + omap_pipe3_dpll_relock(phy); return 0; } -static int omap_usb3_init(struct usb_phy *x) +static int omap_pipe3_init(struct phy *x) { - struct omap_usb *phy = phy_to_omapusb(x); + struct omap_pipe3 *phy = phy_get_drvdata(x); int ret; - ret = omap_usb_dpll_lock(phy); + ret = omap_pipe3_dpll_lock(phy); if (ret) return ret; @@ -195,9 +199,18 @@ static int omap_usb3_init(struct usb_phy *x) return 0; } -static int omap_usb3_probe(struct platform_device *pdev) +static struct phy_ops ops = { + .init = omap_pipe3_init, + .power_on = omap_pipe3_power_on, + .power_off = omap_pipe3_power_off, + .owner = THIS_MODULE, +}; + +static int omap_pipe3_probe(struct platform_device *pdev) { - struct omap_usb *phy; + struct omap_pipe3 *phy; + struct phy *generic_phy; + struct phy_provider *phy_provider; struct resource *res; struct device_node *node = pdev->dev.of_node; struct device_node *control_node; @@ -208,7 +221,7 @@ static int omap_usb3_probe(struct platform_device *pdev) phy = devm_kzalloc(&pdev->dev, sizeof(*phy), GFP_KERNEL); if (!phy) { - dev_err(&pdev->dev, "unable to alloc mem for OMAP USB3 PHY\n"); + dev_err(&pdev->dev, "unable to alloc mem for OMAP PIPE3 PHY\n"); return -ENOMEM; } @@ -219,13 +232,6 @@ static int omap_usb3_probe(struct platform_device *pdev) phy->dev = &pdev->dev; - phy->phy.dev = phy->dev; - phy->phy.label = "omap-usb3"; - phy->phy.init = omap_usb3_init; - phy->phy.set_suspend = omap_usb3_suspend; - phy->phy.type = USB_PHY_TYPE_USB3; - - phy->is_suspended = 1; phy->wkupclk = devm_clk_get(phy->dev, "usb_phy_cm_clk32k");As this is no longer USB specific, we need to make the clock binding generic as well.
right. Its also needed when we have the same driver work for sata and pcie. Maybe do that in a separate patch?
if (IS_ERR(phy->wkupclk)) { dev_err(&pdev->dev, "unable to get usb_phy_cm_clk32k\n"); @@ -251,6 +257,11 @@ static int omap_usb3_probe(struct platform_device *pdev) dev_err(&pdev->dev, "Failed to get control device phandle\n"); return -EINVAL; } + phy_provider = devm_of_phy_provider_register(phy->dev, + of_phy_simple_xlate); + if (IS_ERR(phy_provider)) + return PTR_ERR(phy_provider); + control_pdev = of_find_device_by_node(control_node); if (!control_pdev) { dev_err(&pdev->dev, "Failed to get control device\n"); @@ -260,23 +271,27 @@ static int omap_usb3_probe(struct platform_device *pdev) phy->control_dev = &control_pdev->dev; omap_control_usb_phy_power(phy->control_dev, 0); - usb_add_phy_dev(&phy->phy); platform_set_drvdata(pdev, phy); - pm_runtime_enable(phy->dev); + + generic_phy = devm_phy_create(phy->dev, &ops, NULL); + if (IS_ERR(generic_phy)) + return PTR_ERR(generic_phy); + + phy_set_drvdata(generic_phy, phy); + pm_runtime_get(&pdev->dev); return 0; } -static int omap_usb3_remove(struct platform_device *pdev) +static int omap_pipe3_remove(struct platform_device *pdev) { - struct omap_usb *phy = platform_get_drvdata(pdev); + struct omap_pipe3 *phy = platform_get_drvdata(pdev); clk_unprepare(phy->wkupclk); clk_unprepare(phy->optclk); - usb_remove_phy(&phy->phy); if (!pm_runtime_suspended(&pdev->dev)) pm_runtime_put(&pdev->dev); pm_runtime_disable(&pdev->dev); @@ -286,10 +301,9 @@ static int omap_usb3_remove(struct platform_device *pdev) #ifdef CONFIG_PM_RUNTIME -static int omap_usb3_runtime_suspend(struct device *dev) +static int omap_pipe3_runtime_suspend(struct device *dev) { - struct platform_device *pdev = to_platform_device(dev); - struct omap_usb *phy = platform_get_drvdata(pdev); + struct omap_pipe3 *phy = dev_get_drvdata(dev); clk_disable(phy->wkupclk); clk_disable(phy->optclk); @@ -297,11 +311,10 @@ static int omap_usb3_runtime_suspend(struct device *dev) return 0; } -static int omap_usb3_runtime_resume(struct device *dev) +static int omap_pipe3_runtime_resume(struct device *dev) { u32 ret = 0; - struct platform_device *pdev = to_platform_device(dev); - struct omap_usb *phy = platform_get_drvdata(pdev); + struct omap_pipe3 *phy = dev_get_drvdata(dev); ret = clk_enable(phy->optclk); if (ret) { @@ -324,38 +337,41 @@ err1: return ret; } -static const struct dev_pm_ops omap_usb3_pm_ops = { - SET_RUNTIME_PM_OPS(omap_usb3_runtime_suspend, omap_usb3_runtime_resume, - NULL) +static const struct dev_pm_ops omap_pipe3_pm_ops = { + SET_RUNTIME_PM_OPS(omap_pipe3_runtime_suspend, + omap_pipe3_runtime_resume, NULL) }; -#define DEV_PM_OPS (&omap_usb3_pm_ops) +#define DEV_PM_OPS (&omap_pipe3_pm_ops) #else #define DEV_PM_OPS NULL #endif #ifdef CONFIG_OF -static const struct of_device_id omap_usb3_id_table[] = { +static const struct of_device_id omap_pipe3_id_table[] = { + { .compatible = "ti,omap-pipe3" },why do you need "omap-pipe3", isn't sata, pcie and usb3 sufficient?
I thought it would be better if everyone uses omap-pipe3 and added pcie, sata if there are any specific settings (for pcie or sata) that should be done.
+ { .compatible = "ti,omap-sata" }, + { .compatible = "ti,omap-pcie" }, { .compatible = "ti,omap-usb3" }, {} }; -MODULE_DEVICE_TABLE(of, omap_usb3_id_table); +MODULE_DEVICE_TABLE(of, omap_pipe3_id_table); #endif -static struct platform_driver omap_usb3_driver = { - .probe = omap_usb3_probe, - .remove = omap_usb3_remove, +static struct platform_driver omap_pipe3_driver = { + .probe = omap_pipe3_probe, + .remove = omap_pipe3_remove, .driver = { - .name = "omap-usb3", + .name = "omap-pipe3", .owner = THIS_MODULE, .pm = DEV_PM_OPS, - .of_match_table = of_match_ptr(omap_usb3_id_table), + .of_match_table = of_match_ptr(omap_pipe3_id_table), }, }; -module_platform_driver(omap_usb3_driver); +module_platform_driver(omap_pipe3_driver); -MODULE_ALIAS("platform: omap_usb3"); +MODULE_ALIAS("platform: omap_pipe3"); MODULE_AUTHOR("Texas Instruments Inc."); -MODULE_DESCRIPTION("OMAP USB3 phy driver"); +MODULE_DESCRIPTION("OMAP PIPE3 phy driver"); MODULE_LICENSE("GPL v2"); diff --git a/drivers/usb/phy/Kconfig b/drivers/usb/phy/Kconfig index d69cdad..0210e03 100644 --- a/drivers/usb/phy/Kconfig +++ b/drivers/usb/phy/Kconfig @@ -66,17 +66,6 @@ config OMAP_CONTROL_USB power on the USB2 PHY is present in OMAP4 and OMAP5. OMAP5 has an additional register to power on USB3 PHY. -config OMAP_USB3 - tristate "OMAP USB3 PHY Driver" - depends on ARCH_OMAP2PLUS || COMPILE_TEST - select OMAP_CONTROL_USB - select USB_PHY - help - Enable this to support the USB3 PHY that is part of SOC. This - driver takes care of all the PHY functionality apart from comparator. - This driver interacts with the "OMAP Control USB Driver" to power - on/off the PHY. - config AM335X_CONTROL_USB tristate diff --git a/drivers/usb/phy/Makefile b/drivers/usb/phy/Makefile index 51968d7..d1c9683 100644 --- a/drivers/usb/phy/Makefile +++ b/drivers/usb/phy/Makefile @@ -13,7 +13,6 @@ obj-$(CONFIG_ISP1301_OMAP) += phy-isp1301-omap.o obj-$(CONFIG_MV_U3D_PHY) += phy-mv-u3d-usb.o obj-$(CONFIG_NOP_USB_XCEIV) += phy-generic.o obj-$(CONFIG_OMAP_CONTROL_USB) += phy-omap-control.o -obj-$(CONFIG_OMAP_USB3) += phy-omap-usb3.o obj-$(CONFIG_SAMSUNG_USBPHY) += phy-samsung-usb.o obj-$(CONFIG_SAMSUNG_USB2PHY) += phy-samsung-usb2.o obj-$(CONFIG_SAMSUNG_USB3PHY) += phy-samsung-usb3.o diff --git a/include/linux/phy/omap_pipe3.h b/include/linux/phy/omap_pipe3.h new file mode 100644 index 0000000..7329056 --- /dev/null +++ b/include/linux/phy/omap_pipe3.hti_pipe3.h ?@@ -0,0 +1,52 @@ +/* + * omap_pipe3.h -- omap pipe3 phy header file + * + * Copyright (C) 2013 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: Kishon Vijay Abraham I <kishon@xxxxxx> + * + * 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. + * + */ + +#ifndef __DRIVERS_OMAP_PIPE3_H +#define __DRIVERS_OMAP_PIPE3_H + +#include <linux/io.h> + +struct pipe3_dpll_params { + u16 m; + u8 n; + u8 freq:3; + u8 sd; + u32 mf; +}; + +struct omap_pipe3 { + void __iomem *pll_ctrl_base; + struct device *dev; + struct device *control_dev; + struct clk *wkupclk; + struct clk *sys_clk;sysclk. To be consistent with others?
Ok. Thanks Kishon -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
