[PATCH v3 07/10] drivers: phy: usb3/pipe3: Adapt pipe3 driver to Generic PHY Framework

Roger Quadros
2013-12-06T14:35:19Z

Hi Kishon,

On 11/25/2013 12:01 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-ti-pipe3 since this same driver will be used for SATA PHY and
> PCIE PHY.
>
> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
> ---
> drivers/phy/Kconfig | 11 +
> drivers/phy/Makefile | 1 +
> .../phy/phy-omap-usb3.c => phy/phy-ti-pipe3.c} | 232 ++++++++++++--------
> drivers/usb/phy/Kconfig | 11 -
> drivers/usb/phy/Makefile | 1 -
> 5 files changed, 149 insertions(+), 107 deletions(-)
> rename drivers/{usb/phy/phy-omap-usb3.c => phy/phy-ti-pipe3.c} (55%)
>
> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
> index a344f3d..1abbfcc 100644
> --- a/drivers/phy/Kconfig
> +++ b/drivers/phy/Kconfig
> @@ -33,6 +33,17 @@ config OMAP_USB2
> The USB OTG controller communicates with the comparator using this
> driver.
>
> +config TI_PIPE3
> + tristate "TI PIPE3 PHY Driver"
> + depends on ARCH_OMAP2PLUS || COMPILE_TEST
> + select GENERIC_PHY
> + select OMAP_CONTROL_USB
> + help
> + Enable this to support the PIPE3 PHY that is part of TI SOCs. This
> + 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 d0caae9..94a1a79 100644
> --- a/drivers/phy/Makefile
> +++ b/drivers/phy/Makefile
> @@ -6,4 +6,5 @@ obj-$(CONFIG_GENERIC_PHY) += phy-core.o
> obj-$(CONFIG_PHY_EXYNOS_DP_VIDEO) += phy-exynos-dp-video.o
> obj-$(CONFIG_PHY_EXYNOS_MIPI_VIDEO) += phy-exynos-mipi-video.o
> obj-$(CONFIG_OMAP_USB2) += phy-omap-usb2.o
> +obj-$(CONFIG_TI_PIPE3) += phy-ti-pipe3.o
> obj-$(CONFIG_TWL4030_USB) += phy-twl4030-usb.o
> diff --git a/drivers/usb/phy/phy-omap-usb3.c b/drivers/phy/phy-ti-pipe3.c
> similarity index 55%
> rename from drivers/usb/phy/phy-omap-usb3.c
> rename to drivers/phy/phy-ti-pipe3.c
> index 0c6ba29..410b286 100644
> --- a/drivers/usb/phy/phy-omap-usb3.c
> +++ b/drivers/phy/phy-ti-pipe3.c
> @@ -1,5 +1,5 @@
> /*
> - * omap-usb3 - USB PHY, talking to dwc3 controller in OMAP.
> + * phy-ti-pipe3 - PIPE3 PHY driver.
> *
> * Copyright (C) 2013 Texas Instruments Incorporated - http://www.ti.com
> * This program is free software; you can redistribute it and/or modify
> @@ -19,10 +19,11 @@
> #include <linux/module.h>
> #include <linux/platform_device.h>
> #include <linux/slab.h>
> -#include <linux/usb/omap_usb.h>
> +#include <linux/phy/phy.h>
> #include <linux/of.h>
> #include <linux/clk.h>
> #include <linux/err.h>
> +#include <linux/io.h>
> #include <linux/pm_runtime.h>
> #include <linux/delay.h>
> #include <linux/usb/omap_control_usb.h>
> @@ -52,17 +53,34 @@
>
> /*
> * 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_params {
> + u16 m;
> + u8 n;
> + u8 freq:3;
> + u8 sd;
> + u32 mf;
> +};
> +
> +struct ti_pipe3 {
> + void __iomem *pll_ctrl_base;
> + struct device *dev;
> + struct device *control_dev;
> + struct clk *wkupclk;
> + struct clk *sys_clk;
> + struct clk *optclk;
> +};
> +
> +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 +89,18 @@ 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 inline u32 ti_pipe3_readl(void __iomem *addr, unsigned offset)
> +{
> + return __raw_readl(addr + offset);
> +}
> +
> +static inline void ti_pipe3_writel(void __iomem *addr, unsigned offset,
> + u32 data)
> +{
> + __raw_writel(data, addr + offset);
> +}
> +
> +static struct pipe3_dpll_params *ti_pipe3_get_dpll_params(unsigned long rate)
> {
> int i;
>
> @@ -83,110 +112,113 @@ static struct usb_dpll_params *omap_usb3_get_dpll_params(unsigned long rate)
> return NULL;
> }
>
> -static int omap_usb3_suspend(struct usb_phy *x, int suspend)
> +static int ti_pipe3_power_off(struct phy *x)
> {
> - struct omap_usb *phy = phy_to_omapusb(x);
> - int val;
> + struct ti_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 = ti_pipe3_readl(phy->pll_ctrl_base, PLL_CONFIGURATION2);
> + val |= PLL_IDLE;
> + ti_pipe3_writel(phy->pll_ctrl_base, PLL_CONFIGURATION2, val);
> +
> + do {
> + val = ti_pipe3_readl(phy->pll_ctrl_base, PLL_STATUS);
> + if (val & PLL_TICOPWDN)
> + break;
> + usleep_range(1, 5);

I had suggested to use sleep instead of udelay here but usleep for < 10 us might not be not optimal.
see Documentation/timers/timers-howto.txt

Why can't we just use msleep(1)?

Do we know approximately how much time it takes for the block to power down?

> + } while (--timeout);
> +

what if there was a timeout? you need to exit with return code and preferably print an error message.

> + omap_control_usb_phy_power(phy->control_dev, 0);
> +
> + return 0;
> +}
> +
> +static int ti_pipe3_power_on(struct phy *x)
> +{
> + struct ti_pipe3 *phy = phy_get_drvdata(x);
> + int val;
> + int timeout = PLL_IDLE_TIME;
> +
> + val = ti_pipe3_readl(phy->pll_ctrl_base, PLL_CONFIGURATION2);
> + val &= ~PLL_IDLE;
> + ti_pipe3_writel(phy->pll_ctrl_base, PLL_CONFIGURATION2, val);
> +
> + do {
> + val = ti_pipe3_readl(phy->pll_ctrl_base, PLL_STATUS);
> + if (!(val & PLL_TICOPWDN))
> + break;
> + usleep_range(1, 5);
> + } while (--timeout);

here as well.
>
> return 0;
> }
>
> -static void omap_usb_dpll_relock(struct omap_usb *phy)
> +static void ti_pipe3_dpll_relock(struct ti_pipe3 *phy)
> {
> u32 val;
> unsigned long timeout;
>
> - omap_usb_writel(phy->pll_ctrl_base, PLL_GO, SET_PLL_GO);
> + ti_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 = ti_pipe3_readl(phy->pll_ctrl_base, PLL_STATUS);
> if (val & PLL_LOCK)
> break;
> } while (!WARN_ON(time_after(jiffies, timeout)));
> }

cheers,
-roger