lkml.org 
[lkml]   [2013]   [Oct]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 3/7] drivers: phy: usb3/pipe3: Adapt pipe3 driver to Generic PHY Framework
    On 09/16/2013 10:37 AM, Roger Quadros wrote:
    > On 09/16/2013 06:01 AM, Kishon Vijay Abraham I wrote:
    >> 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@ti.com>
    >>>> ---
    >>>> 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?
    >
    > OK. Up to you.
    >
    >>>
    >>>> 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_USB
    >>> how 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.
    >
    > You can use select, provided the item you are selecting doesn't depend on anything else.
    > As OMAP_CONTROL_USB depends on ARCH_OMAP2PLUS, your configuration will fail if a user enables
    > OMAP_PIPE3 on non OMAP configuration.
    >
    > Further, in this case since it is OMAP related driver, there is no point in showing/building it
    > if OMAP platform is not selected, so you at least need [depends on "ARCH_OMAP2PLUS"] to fix
    > both issue I mentioned.
    >
    >>>
    >>> 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. This
    >>>
    >>> worth 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?
    >
    > OK. up to you.
    >>>
    >>>> 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.
    >
    > We can always add specialized options later when needed. AFAIK just the
    > DPLL data is different among the different PHYs.
    >>>
    >>>> + { .compatible = "ti,omap-sata" },
    >>>> + { .compatible = "ti,omap-pcie" },
    >>>> { .compatible = "ti,omap-usb3" },
    >
    > I think compatible strings should be improved to indicate that it is a PHY.
    >
    > e.g. "ti,omap-phy-sata" or just "ti,pipe3-phy-sata"
    >

    Please remove "ti,omap-pcie" for now, you can add it later whenever you add
    dpll settings for pcie.

    Also, please change the newly added compatible strings to

    "ti,phy-pipe3-usb3" and "ti,phy-pipe3-sata"

    Also you need to rearrange the patches so that the DT binding document is first
    moded from usb/ to phy/ and then the change is done for pipe3.

    cheers,
    -roger


    \
     
     \ /
      Last update: 2013-10-11 17:41    [W:3.658 / U:0.148 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site