Hi Doug, Sorry!! for the delayed response. On Thu, Dec 20, 2012 at 4:31 AM, Doug Anderson <dianders@xxxxxxxxxxxx> wrote: > Vivek, > > I don't really have a good 10000 foot view about how all of the USB > bits fit together, but a few detail-oriented comments below. > > > On Tue, Dec 18, 2012 at 6:43 AM, Vivek Gautam <gautam.vivek@xxxxxxxxxxx> wrote: >> This patch adds host phy support to samsung-usbphy.c and >> further adds support for samsung's exynos5250 usb-phy. >> >> Signed-off-by: Praveen Paneri <p.paneri@xxxxxxxxxxx> >> Signed-off-by: Vivek Gautam <gautam.vivek@xxxxxxxxxxx> >> --- >> .../devicetree/bindings/usb/samsung-usbphy.txt | 25 +- >> drivers/usb/phy/Kconfig | 2 +- >> drivers/usb/phy/samsung-usbphy.c | 465 ++++++++++++++++++-- >> include/linux/usb/samsung_usb_phy.h | 13 + >> 4 files changed, 454 insertions(+), 51 deletions(-) >> >> diff --git a/Documentation/devicetree/bindings/usb/samsung-usbphy.txt b/Documentation/devicetree/bindings/usb/samsung-usbphy.txt >> index a7b28b2..2ec5400 100644 >> --- a/Documentation/devicetree/bindings/usb/samsung-usbphy.txt >> +++ b/Documentation/devicetree/bindings/usb/samsung-usbphy.txt >> @@ -1,23 +1,38 @@ >> * Samsung's usb phy transceiver >> >> -The Samsung's phy transceiver is used for controlling usb otg phy for >> -s3c-hsotg usb device controller. >> +The Samsung's phy transceiver is used for controlling usb phy for >> +s3c-hsotg as well as ehci-s5p and ohci-exynos usb controllers >> +across Samsung SOCs. >> TODO: Adding the PHY binding with controller(s) according to the under >> developement generic PHY driver. >> >> Required properties: >> + >> +Exynos4210: >> - compatible : should be "samsung,exynos4210-usbphy" >> - reg : base physical address of the phy registers and length of memory mapped >> region. >> >> +Exynos5250: >> +- compatible : should be "samsung,exynos5250-usbphy" >> +- reg : base physical address of the phy registers and length of memory mapped >> + region. >> + >> Optional properties: >> - samsung,usb-phyhandle : should point to usb-phyhandle sub-node which provides >> binding data to enable/disable device PHY handled by >> - PMU register. >> + PMU register; or to configure usb2.0 phy handled by >> + SYSREG. >> >> Required properties: >> - compatible : should be "samsung,usbdev-phyctrl" for >> - DEVICE type phy. >> + DEVICE type phy; or >> + should be "samsung,usbhost-phyctrl" for >> + HOST type phy; or >> + should be "samsung,usb-phycfg" for >> + USB2.0 PHY_CFG. >> - samsung,phyhandle-reg: base physical address of >> - PHY_CONTROL register in PMU. >> + PHY_CONTROL register in PMU; >> + or USB2.0 PHY_CFG register >> + in SYSREG. >> - samsung,enable-mask : should be '1' >> diff --git a/drivers/usb/phy/Kconfig b/drivers/usb/phy/Kconfig >> index 17ad743..13c0eaf 100644 >> --- a/drivers/usb/phy/Kconfig >> +++ b/drivers/usb/phy/Kconfig >> @@ -47,7 +47,7 @@ config USB_RCAR_PHY >> >> config SAMSUNG_USBPHY >> bool "Samsung USB PHY controller Driver" >> - depends on USB_S3C_HSOTG >> + depends on USB_S3C_HSOTG || USB_EHCI_S5P || USB_OHCI_EXYNOS >> select USB_OTG_UTILS >> help >> Enable this to support Samsung USB phy controller for samsung >> diff --git a/drivers/usb/phy/samsung-usbphy.c b/drivers/usb/phy/samsung-usbphy.c >> index 4ceabe3..621348a 100644 >> --- a/drivers/usb/phy/samsung-usbphy.c >> +++ b/drivers/usb/phy/samsung-usbphy.c >> @@ -5,7 +5,8 @@ >> * >> * Author: Praveen Paneri <p.paneri@xxxxxxxxxxx> >> * >> - * Samsung USB2.0 High-speed OTG transceiver, talks to S3C HS OTG controller >> + * Samsung USB-PHY transceiver; talks to S3C HS OTG controller, EHCI-S5P and >> + * OHCI-EXYNOS controllers. >> * >> * This program is free software; you can redistribute it and/or modify >> * it under the terms of the GNU General Public License version 2 as >> @@ -24,7 +25,7 @@ >> #include <linux/err.h> >> #include <linux/io.h> >> #include <linux/of.h> >> -#include <linux/usb/otg.h> >> +#include <linux/usb/samsung_usb_phy.h> >> #include <linux/platform_data/samsung-usbphy.h> >> >> /* Register definitions */ >> @@ -56,6 +57,103 @@ >> #define RSTCON_HLINK_SWRST (0x1 << 1) >> #define RSTCON_SWRST (0x1 << 0) >> >> +/* EXYNOS5 */ >> +#define EXYNOS5_PHY_HOST_CTRL0 (0x00) >> + >> +#define HOST_CTRL0_PHYSWRSTALL (0x1 << 31) >> + >> +#define HOST_CTRL0_REFCLKSEL_MASK (0x3) >> +#define HOST_CTRL0_REFCLKSEL_XTAL (0x0 << 19) >> +#define HOST_CTRL0_REFCLKSEL_EXTL (0x1 << 19) >> +#define HOST_CTRL0_REFCLKSEL_CLKCORE (0x2 << 19) >> + >> +#define HOST_CTRL0_FSEL_MASK (0x7 << 16) >> +#define HOST_CTRL0_FSEL(_x) ((_x) << 16) >> +#define HOST_CTRL0_FSEL_CLKSEL_50M (0x7) >> +#define HOST_CTRL0_FSEL_CLKSEL_24M (0x5) >> +#define HOST_CTRL0_FSEL_CLKSEL_20M (0x4) >> +#define HOST_CTRL0_FSEL_CLKSEL_19200K (0x3) >> +#define HOST_CTRL0_FSEL_CLKSEL_12M (0x2) >> +#define HOST_CTRL0_FSEL_CLKSEL_10M (0x1) >> +#define HOST_CTRL0_FSEL_CLKSEL_9600K (0x0) > > Add the shifts to the #defines and remove HOST_CTRL0_FSEL(_x). That > makes it match the older phy more closely. > Wouldn't it hamper the readability when shifts are used ?? I mean if we have something like this - phyhost |= HOST_CTRL0_FSEL(phyclk) so, if we are using the shifts then phyhost |= (HOST_CTRL0_FSEL_CLKSEL_24M << HOST_CTRL0_FSEL_SHIFT) If you say i will add the shifts here. >> + >> +#define HOST_CTRL0_TESTBURNIN (0x1 << 11) >> +#define HOST_CTRL0_RETENABLE (0x1 << 10) >> +#define HOST_CTRL0_COMMONON_N (0x1 << 9) >> +#define HOST_CTRL0_SIDDQ (0x1 << 6) >> +#define HOST_CTRL0_FORCESLEEP (0x1 << 5) >> +#define HOST_CTRL0_FORCESUSPEND (0x1 << 4) >> +#define HOST_CTRL0_WORDINTERFACE (0x1 << 3) >> +#define HOST_CTRL0_UTMISWRST (0x1 << 2) >> +#define HOST_CTRL0_LINKSWRST (0x1 << 1) >> +#define HOST_CTRL0_PHYSWRST (0x1 << 0) >> + >> +#define EXYNOS5_PHY_HOST_TUNE0 (0x04) >> + >> +#define EXYNOS5_PHY_HSIC_CTRL1 (0x10) >> + >> +#define EXYNOS5_PHY_HSIC_TUNE1 (0x14) >> + >> +#define EXYNOS5_PHY_HSIC_CTRL2 (0x20) >> + >> +#define EXYNOS5_PHY_HSIC_TUNE2 (0x24) >> + >> +#define HSIC_CTRL_REFCLKSEL_MASK (0x3) >> +#define HSIC_CTRL_REFCLKSEL (0x2 << 23) >> + >> +#define HSIC_CTRL_REFCLKDIV_MASK (0x7f) >> +#define HSIC_CTRL_REFCLKDIV(_x) ((_x) << 16) >> +#define HSIC_CTRL_REFCLKDIV_12 (0x24 << 16) >> +#define HSIC_CTRL_REFCLKDIV_15 (0x1c << 16) >> +#define HSIC_CTRL_REFCLKDIV_16 (0x1a << 16) >> +#define HSIC_CTRL_REFCLKDIV_19_2 (0x15 << 16) >> +#define HSIC_CTRL_REFCLKDIV_20 (0x14) >> + >> +#define HSIC_CTRL_SIDDQ (0x1 << 6) >> +#define HSIC_CTRL_FORCESLEEP (0x1 << 5) >> +#define HSIC_CTRL_FORCESUSPEND (0x1 << 4) >> +#define HSIC_CTRL_WORDINTERFACE (0x1 << 3) >> +#define HSIC_CTRL_UTMISWRST (0x1 << 2) >> +#define HSIC_CTRL_PHYSWRST (0x1 << 0) >> + >> +#define EXYNOS5_PHY_HOST_EHCICTRL (0x30) >> + >> +#define HOST_EHCICTRL_ENAINCRXALIGN (0x1 << 29) >> +#define HOST_EHCICTRL_ENAINCR4 (0x1 << 28) >> +#define HOST_EHCICTRL_ENAINCR8 (0x1 << 27) >> +#define HOST_EHCICTRL_ENAINCR16 (0x1 << 26) >> + >> +#define EXYNOS5_PHY_HOST_OHCICTRL (0x34) >> + >> +#define HOST_OHCICTRL_SUSPLGCY (0x1 << 3) >> +#define HOST_OHCICTRL_APPSTARTCLK (0x1 << 2) >> +#define HOST_OHCICTRL_CNTSEL (0x1 << 1) >> +#define HOST_OHCICTRL_CLKCKTRST (0x1 << 0) >> + >> +#define EXYNOS5_PHY_OTG_SYS (0x38) >> + >> +#define OTG_SYS_PHYLINK_SWRESET (0x1 << 14) >> +#define OTG_SYS_LINKSWRST_UOTG (0x1 << 13) >> +#define OTG_SYS_PHY0_SWRST (0x1 << 12) >> + >> +#define OTG_SYS_REFCLKSEL_MASK (0x3 << 9) >> +#define OTG_SYS_REFCLKSEL_XTAL (0x0 << 9) >> +#define OTG_SYS_REFCLKSEL_EXTL (0x1 << 9) >> +#define OTG_SYS_REFCLKSEL_CLKCORE (0x2 << 9) >> + >> +#define OTG_SYS_IDPULLUP_UOTG (0x1 << 8) >> +#define OTG_SYS_COMMON_ON (0x1 << 7) >> + >> +#define OTG_SYS_FSEL_MASK (0x7 << 4) >> +#define OTG_SYS_FSEL(_x) ((_x) << 4) >> + >> +#define OTG_SYS_FORCESLEEP (0x1 << 3) >> +#define OTG_SYS_OTGDISABLE (0x1 << 2) >> +#define OTG_SYS_SIDDQ_UOTG (0x1 << 1) >> +#define OTG_SYS_FORCESUSPEND (0x1 << 0) >> + >> +#define EXYNOS5_PHY_OTG_TUNE (0x40) >> + >> #ifndef MHZ >> #define MHZ (1000*1000) >> #endif >> @@ -63,6 +161,7 @@ >> enum samsung_cpu_type { >> TYPE_S3C64XX, >> TYPE_EXYNOS4210, >> + TYPE_EXYNOS5250, >> }; >> >> /* >> @@ -73,9 +172,13 @@ enum samsung_cpu_type { >> * @clk: usb phy clock >> * @regs: usb phy register memory base >> * @devctrl_reg: usb device phy-control pmu register memory base >> + * @hostctrl_reg: usb host phy-control pmu register memory base >> + * @phycfg_reg: usb2.0 phy-cfg system register memory base >> * @en_mask: enable mask >> * @ref_clk_freq: reference clock frequency selection >> * @cpu_type: machine identifier >> + * @phy_type: It keeps track of the PHY type. >> + * @host_usage: host_phy usage count. >> */ >> struct samsung_usbphy { >> struct usb_phy phy; >> @@ -84,9 +187,13 @@ struct samsung_usbphy { >> struct clk *clk; >> void __iomem *regs; >> void __iomem *devctrl_reg; >> + void __iomem *hostctrl_reg; >> + void __iomem *phycfg_reg; >> u32 en_mask; >> int ref_clk_freq; >> int cpu_type; >> + enum samsung_usb_phy_type phy_type; >> + atomic_t host_usage; >> }; >> >> #define phy_to_sphy(x) container_of((x), struct samsung_usbphy, phy) >> @@ -96,26 +203,49 @@ static int samsung_usbphy_parse_dt_param(struct samsung_usbphy *sphy) >> struct device_node *usb_phyctrl; >> u32 reg; >> int lenp; >> + int count = 0; >> >> if (!sphy->dev->of_node) { >> sphy->devctrl_reg = NULL; > > Remove init to NULL here (and in other error cases). > ...I was going to suggest instead to add setting of hostctrl_reg and > phycfg_reg to NULL, but then realized that we should just rely on the > fact that the devm_kzalloc() will zero things. > this will change in accordance with v6 patch for pmu_isolation. >> return -ENODEV; >> } >> >> - if (of_get_property(sphy->dev->of_node, "samsung,usb-phyhandle", &lenp)) { >> - usb_phyctrl = of_parse_phandle(sphy->dev->of_node, >> - "samsung,usb-phyhandle", 0); >> - if (!usb_phyctrl) { >> - dev_warn(sphy->dev, "Can't get usb-phy handle\n"); >> - sphy->devctrl_reg = NULL; >> - } >> - >> - of_property_read_u32(usb_phyctrl, "samsung,phyhandle-reg", ®); >> + if (of_get_property(sphy->dev->of_node, >> + "samsung,usb-phyhandle", &lenp)) { >> + do { > > Indentation gets excessive here. Can you break this into its own function? > ditto. >> + usb_phyctrl = of_parse_phandle(sphy->dev->of_node, >> + "samsung,usb-phyhandle", >> + count++); >> + if (!usb_phyctrl) { >> + dev_warn(sphy->dev, >> + "Can't get usb-phy handle\n"); >> + sphy->devctrl_reg = NULL; > > This is a confusing way to handle the error. Essentially you want to > print an error if you get to the end of the function and devctrl_reg, > hostctrl_reg, and phycfg_reg are all NULL, right? Just check for > that. > sure, will amend this. >> + } >> + >> + of_property_read_u32(usb_phyctrl, >> + "samsung,phyhandle-reg", ®); >> + >> + /* >> + * Considering here the maximum number of configurable >> + * register settings: DEVICE_PHY_CONTROL >> + * HOST_PHY_CONTROL >> + * PHY20_CFG >> + */ >> + if (of_device_is_compatible(usb_phyctrl, >> + "samsung,usbdev-phyctrl")) >> + sphy->devctrl_reg = ioremap(reg, SZ_4); >> + else if (of_device_is_compatible(usb_phyctrl, >> + "samsung,usbhost-phyctrl")) >> + sphy->hostctrl_reg = ioremap(reg, SZ_4); >> + else if (of_device_is_compatible(usb_phyctrl, >> + "samsung,usb-phycfg")) >> + sphy->phycfg_reg = ioremap(reg, SZ_4); >> + >> + of_property_read_u32(sphy->dev->of_node, >> + "samsung,enable-mask", &sphy->en_mask); >> + } while (of_parse_phandle(sphy->dev->of_node, >> + "samsung,usb-phyhandle", count)); > > Can't you use (*lenp) to figure out how many times to loop? Seems > like that would be simpler. > this will change in accordance with v6 patch for pmu_isolation. >> >> - sphy->devctrl_reg = ioremap(reg, SZ_4); >> - >> - of_property_read_u32(sphy->dev->of_node, "samsung,enable-mask", >> - &sphy->en_mask); >> of_node_put(usb_phyctrl); >> } else { >> dev_warn(sphy->dev, "Can't get usb-phy handle\n"); >> @@ -129,13 +259,18 @@ static int samsung_usbphy_parse_dt_param(struct samsung_usbphy *sphy) >> * Set isolation here for phy. >> * SOCs control this by controlling corresponding PMU registers >> */ >> -static void samsung_usbphy_set_isolation(struct samsung_usbphy *sphy, int on) >> +static void samsung_usbphy_set_isolation(struct samsung_usbphy *sphy, >> + int on, int type) > > Why add type to this function. It's always sphy->type, isn't it? > Valid, :-) will change this. >> { >> - void __iomem *usb_phyctrl_reg; >> + void __iomem *usb_phyctrl_reg = NULL; >> u32 en_mask = sphy->en_mask; >> u32 reg; >> >> - usb_phyctrl_reg = sphy->devctrl_reg; >> + if (type == USB_PHY_TYPE_DEVICE) { > > nit: no braces for single line "if". Please run checkpatch. > this will change in accordance with v6 patch for pmu_isolation. >> + usb_phyctrl_reg = sphy->devctrl_reg; >> + } else if (type == USB_PHY_TYPE_HOST) { >> + usb_phyctrl_reg = sphy->hostctrl_reg; >> + } >> >> if (!usb_phyctrl_reg) { >> dev_warn(sphy->dev, "Can't set pmu isolation\n"); >> @@ -151,6 +286,47 @@ static void samsung_usbphy_set_isolation(struct samsung_usbphy *sphy, int on) >> } >> >> /* >> + * Configure the mode of working og usb-phy here: HOST/DEVICE. > > s/og/otg? > s/og/of here >> + * SOCs control this by controlling corresponding system register >> + */ >> +static void samsung_usbphy_cfg_sel(struct samsung_usbphy *sphy, int type) > > Why add type to this function. It's always sphy->type, isn't it? > Sure, no need of type argument here. will change this. >> +{ >> + void __iomem *usb_phycfg_reg; >> + u32 en_mask = sphy->en_mask; >> + u32 reg; >> + >> + usb_phycfg_reg = sphy->phycfg_reg; >> + >> + if (!usb_phycfg_reg) { >> + dev_warn(sphy->dev, "Can't select specified phy mode\n"); >> + return; >> + } >> + >> + reg = readl(usb_phycfg_reg); >> + >> + if (type == USB_PHY_TYPE_DEVICE) > > This is really confusing. You disable for device and enable for host? Eh? > This is written badly :-( Actually resetting the bit puts it to device mode and setting the bit to Host. Should have used a separate mask for phycfg. Will amend this. >> + writel(reg & ~en_mask, usb_phycfg_reg); >> + else if (type == USB_PHY_TYPE_HOST) >> + writel(reg | en_mask, usb_phycfg_reg); >> +} >> + >> +/* >> + * PHYs are different for USB Device and USB Host. Controllers can make >> + * sure that the correct PHY type is selected by calling this function >> + * before any PHY operation. >> + */ >> +int samsung_usbphy_set_type(struct usb_phy *phy, >> + enum samsung_usb_phy_type phy_type) >> +{ >> + struct samsung_usbphy *sphy = phy_to_sphy(phy); >> + >> + if (sphy->phy_type != phy_type) >> + sphy->phy_type = phy_type; > > Kill useless "if" > Ok >> + >> + return 0; >> +} >> + >> +/* >> * Returns reference clock frequency selection value >> */ >> static int samsung_usbphy_get_refclk_freq(struct samsung_usbphy *sphy) >> @@ -158,34 +334,178 @@ static int samsung_usbphy_get_refclk_freq(struct samsung_usbphy *sphy) >> struct clk *ref_clk; >> int refclk_freq = 0; >> >> - ref_clk = clk_get(sphy->dev, "xusbxti"); >> + if (sphy->cpu_type == TYPE_EXYNOS5250) >> + ref_clk = clk_get(sphy->dev, "ext_xtal"); >> + else >> + ref_clk = clk_get(sphy->dev, "xusbxti"); > > Seems strange that this clock has a special different name on 5250. > Maybe that should be fixed rather than hacking it here? > clk_add_alias()? > Actually, SoC untill exynos4 were using clock "xusbxti" for USB-PHY which has a fixed rate of 24MHz, now exynos5250 uses "ext_xtal" clock for USB-PHY whose rate is determined at the time of clk_init (no doubt it's also 24MHz for exynos5250). >> if (IS_ERR(ref_clk)) { >> dev_err(sphy->dev, "Failed to get reference clock\n"); >> return PTR_ERR(ref_clk); >> } >> >> - switch (clk_get_rate(ref_clk)) { >> - case 12 * MHZ: >> - refclk_freq = PHYCLK_CLKSEL_12M; >> - break; >> - case 24 * MHZ: >> - refclk_freq = PHYCLK_CLKSEL_24M; >> - break; >> - case 48 * MHZ: >> - refclk_freq = PHYCLK_CLKSEL_48M; >> - break; >> - default: >> - if (sphy->cpu_type == TYPE_S3C64XX) >> - refclk_freq = PHYCLK_CLKSEL_48M; >> - else >> + if (sphy->cpu_type == TYPE_EXYNOS5250) { >> + /* set clock frequency for PLL */ >> + switch (clk_get_rate(ref_clk)) { >> + case 96 * 100000: > > nit: change to 9600 * KHZ to match; below too. > sure. >> + refclk_freq |= HOST_CTRL0_FSEL_CLKSEL_9600K; > > Why |= with 0? > kept this just to keep things look similar :-). will remove this line, >> + break; >> + case 10 * MHZ: >> + refclk_freq |= HOST_CTRL0_FSEL_CLKSEL_10M; >> + break; >> + case 12 * MHZ: >> + refclk_freq |= HOST_CTRL0_FSEL_CLKSEL_12M; >> + break; >> + case 192 * 100000: >> + refclk_freq |= HOST_CTRL0_FSEL_CLKSEL_19200K; >> + break; >> + case 20 * MHZ: >> + refclk_freq |= HOST_CTRL0_FSEL_CLKSEL_20M; >> + break; >> + case 50 * MHZ: >> + refclk_freq |= HOST_CTRL0_FSEL_CLKSEL_50M; >> + break; >> + case 24 * MHZ: >> + default: >> + /* default reference clock */ >> + refclk_freq |= HOST_CTRL0_FSEL_CLKSEL_24M; >> + break; >> + } >> + } else { >> + switch (clk_get_rate(ref_clk)) { >> + case 12 * MHZ: >> + refclk_freq = PHYCLK_CLKSEL_12M; >> + break; >> + case 24 * MHZ: >> refclk_freq = PHYCLK_CLKSEL_24M; >> - break; >> + break; >> + case 48 * MHZ: >> + refclk_freq = PHYCLK_CLKSEL_48M; >> + break; >> + default: >> + if (sphy->cpu_type == TYPE_S3C64XX) >> + refclk_freq = PHYCLK_CLKSEL_48M; >> + else >> + refclk_freq = PHYCLK_CLKSEL_24M; >> + break; >> + } >> } >> clk_put(ref_clk); >> >> return refclk_freq; >> } >> >> +static int exynos5_phyhost_is_on(void *regs) > > return bool? > Yeah, sure. >> +{ >> + u32 reg; >> + >> + reg = readl(regs + EXYNOS5_PHY_HOST_CTRL0); >> + >> + return !(reg & HOST_CTRL0_SIDDQ); >> +} >> + >> +static void samsung_exynos5_usbphy_enable(struct samsung_usbphy *sphy) >> +{ >> + void __iomem *regs = sphy->regs; >> + u32 phyclk = sphy->ref_clk_freq; >> + u32 phyhost; >> + u32 phyotg; >> + u32 phyhsic; >> + u32 ehcictrl; >> + u32 ohcictrl; >> + >> + atomic_inc(&sphy->host_usage); > > Can you explain what situations you expect host_usage to be > 1? Why > is that special for exynos5? > this host_usage keeps a track of initialization and de-initialization of PHY. since both ehci and ohci are customers for this phy, host_usage keeps either of them from disabling the phy when other is still using it. > If you really expect multiple users of this, it seems like you need > better locking. Without better locking one process could be in the > middle of disabling at the same time another process is enabling, > couldn't it? > Yes, sure we need to add better locking. Will check on this more. >> + >> + if (exynos5_phyhost_is_on(regs)) { >> + dev_info(sphy->dev, "Already power on PHY\n"); >> + return; >> + } >> + >> + /* Selecting Host/OTG mode; After reset USB2.0PHY_CFG: HOST */ >> + samsung_usbphy_cfg_sel(sphy, USB_PHY_TYPE_HOST); >> + >> + /* Host configuration */ >> + phyhost = readl(regs + EXYNOS5_PHY_HOST_CTRL0); >> + >> + /* phy reference clock configuration */ >> + phyhost &= ~HOST_CTRL0_FSEL_MASK; >> + phyhost |= HOST_CTRL0_FSEL(phyclk); >> + >> + /* host phy reset */ >> + phyhost &= ~(HOST_CTRL0_PHYSWRST | >> + HOST_CTRL0_PHYSWRSTALL | >> + HOST_CTRL0_SIDDQ | >> + /* Enable normal mode of operation */ >> + HOST_CTRL0_FORCESUSPEND | >> + HOST_CTRL0_FORCESLEEP); >> + >> + /* Link reset */ >> + phyhost |= (HOST_CTRL0_LINKSWRST | >> + HOST_CTRL0_UTMISWRST | >> + /* COMMON Block configuration during suspend */ >> + HOST_CTRL0_COMMONON_N); >> + writel(phyhost, regs + EXYNOS5_PHY_HOST_CTRL0); >> + udelay(10); >> + phyhost &= ~(HOST_CTRL0_LINKSWRST | >> + HOST_CTRL0_UTMISWRST); >> + writel(phyhost, regs + EXYNOS5_PHY_HOST_CTRL0); >> + >> + /* OTG configuration */ >> + phyotg = readl(regs + EXYNOS5_PHY_OTG_SYS); >> + >> + /* phy reference clock configuration */ >> + phyotg &= ~OTG_SYS_FSEL_MASK; >> + phyotg |= OTG_SYS_FSEL(phyclk); >> + >> + /* Enable normal mode of operation */ >> + phyotg &= ~(OTG_SYS_FORCESUSPEND | >> + OTG_SYS_SIDDQ_UOTG | >> + OTG_SYS_FORCESLEEP | >> + OTG_SYS_REFCLKSEL_MASK | >> + /* COMMON Block configuration during suspend */ >> + OTG_SYS_COMMON_ON); >> + >> + /* OTG phy & link reset */ >> + phyotg |= (OTG_SYS_PHY0_SWRST | >> + OTG_SYS_LINKSWRST_UOTG | >> + OTG_SYS_PHYLINK_SWRESET | >> + OTG_SYS_OTGDISABLE | >> + /* Set phy refclk */ >> + OTG_SYS_REFCLKSEL_CLKCORE); >> + >> + writel(phyotg, regs + EXYNOS5_PHY_OTG_SYS); >> + udelay(10); >> + phyotg &= ~(OTG_SYS_PHY0_SWRST | >> + OTG_SYS_LINKSWRST_UOTG | >> + OTG_SYS_PHYLINK_SWRESET); >> + writel(phyotg, regs + EXYNOS5_PHY_OTG_SYS); >> + >> + /* HSIC phy configuration */ >> + phyhsic = (HSIC_CTRL_REFCLKDIV_12 | >> + HSIC_CTRL_REFCLKSEL | >> + HSIC_CTRL_PHYSWRST); >> + writel(phyhsic, regs + EXYNOS5_PHY_HSIC_CTRL1); >> + writel(phyhsic, regs + EXYNOS5_PHY_HSIC_CTRL2); >> + udelay(10); >> + phyhsic &= ~HSIC_CTRL_PHYSWRST; >> + writel(phyhsic, regs + EXYNOS5_PHY_HSIC_CTRL1); >> + writel(phyhsic, regs + EXYNOS5_PHY_HSIC_CTRL2); >> + >> + udelay(80); >> + >> + /* enable EHCI DMA burst */ >> + ehcictrl = readl(regs + EXYNOS5_PHY_HOST_EHCICTRL); >> + ehcictrl |= (HOST_EHCICTRL_ENAINCRXALIGN | >> + HOST_EHCICTRL_ENAINCR4 | >> + HOST_EHCICTRL_ENAINCR8 | >> + HOST_EHCICTRL_ENAINCR16); >> + writel(ehcictrl, regs + EXYNOS5_PHY_HOST_EHCICTRL); >> + >> + /* set ohci_suspend_on_n */ >> + ohcictrl = readl(regs + EXYNOS5_PHY_HOST_OHCICTRL); >> + ohcictrl |= HOST_OHCICTRL_SUSPLGCY; >> + writel(ohcictrl, regs + EXYNOS5_PHY_HOST_OHCICTRL); >> +} >> + >> static void samsung_usbphy_enable(struct samsung_usbphy *sphy) >> { >> void __iomem *regs = sphy->regs; >> @@ -221,6 +541,41 @@ static void samsung_usbphy_enable(struct samsung_usbphy *sphy) >> writel(rstcon, regs + SAMSUNG_RSTCON); >> } >> >> +static void samsung_exynos5_usbphy_disable(struct samsung_usbphy *sphy) >> +{ >> + void __iomem *regs = sphy->regs; >> + u32 phyhost; >> + u32 phyotg; >> + u32 phyhsic; >> + >> + if (atomic_dec_return(&sphy->host_usage) > 0) { >> + dev_info(sphy->dev, "still being used\n"); >> + return; >> + } >> + >> + phyhsic = (HSIC_CTRL_REFCLKDIV_12 | >> + HSIC_CTRL_REFCLKSEL | >> + HSIC_CTRL_SIDDQ | >> + HSIC_CTRL_FORCESLEEP | >> + HSIC_CTRL_FORCESUSPEND); >> + writel(phyhsic, regs + EXYNOS5_PHY_HSIC_CTRL1); >> + writel(phyhsic, regs + EXYNOS5_PHY_HSIC_CTRL2); >> + >> + phyhost = readl(regs + EXYNOS5_PHY_HOST_CTRL0); >> + phyhost |= (HOST_CTRL0_SIDDQ | >> + HOST_CTRL0_FORCESUSPEND | >> + HOST_CTRL0_FORCESLEEP | >> + HOST_CTRL0_PHYSWRST | >> + HOST_CTRL0_PHYSWRSTALL); >> + writel(phyhost, regs + EXYNOS5_PHY_HOST_CTRL0); >> + >> + phyotg = readl(regs + EXYNOS5_PHY_OTG_SYS); >> + phyotg |= (OTG_SYS_FORCESUSPEND | >> + OTG_SYS_SIDDQ_UOTG | >> + OTG_SYS_FORCESLEEP); >> + writel(phyotg, regs + EXYNOS5_PHY_OTG_SYS); >> +} >> + >> static void samsung_usbphy_disable(struct samsung_usbphy *sphy) >> { >> void __iomem *regs = sphy->regs; >> @@ -263,10 +618,13 @@ static int samsung_usbphy_init(struct usb_phy *phy) >> if (sphy->plat && sphy->plat->pmu_isolation) >> sphy->plat->pmu_isolation(false); >> else >> - samsung_usbphy_set_isolation(sphy, false); >> + samsung_usbphy_set_isolation(sphy, false, sphy->phy_type); >> >> /* Initialize usb phy registers */ >> - samsung_usbphy_enable(sphy); >> + if (sphy->cpu_type == TYPE_EXYNOS5250) >> + samsung_exynos5_usbphy_enable(sphy); >> + else >> + samsung_usbphy_enable(sphy); >> >> /* Disable the phy clock */ >> clk_disable_unprepare(sphy->clk); >> @@ -288,13 +646,16 @@ static void samsung_usbphy_shutdown(struct usb_phy *phy) >> } >> >> /* De-initialize usb phy registers */ >> - samsung_usbphy_disable(sphy); >> + if (sphy->cpu_type == TYPE_EXYNOS5250) >> + samsung_exynos5_usbphy_disable(sphy); >> + else >> + samsung_usbphy_disable(sphy); >> >> /* Enable phy isolation */ >> if (sphy->plat && sphy->plat->pmu_isolation) >> sphy->plat->pmu_isolation(true); >> else >> - samsung_usbphy_set_isolation(sphy, true); >> + samsung_usbphy_set_isolation(sphy, true, sphy->phy_type); >> >> clk_disable_unprepare(sphy->clk); >> } >> @@ -339,12 +700,6 @@ static int __devinit samsung_usbphy_probe(struct platform_device *pdev) >> if (!sphy) >> return -ENOMEM; >> >> - clk = devm_clk_get(dev, "otg"); >> - if (IS_ERR(clk)) { >> - dev_err(dev, "Failed to get otg clock\n"); >> - return PTR_ERR(clk); >> - } >> - >> sphy->dev = &pdev->dev; >> >> ret = samsung_usbphy_parse_dt_param(sphy); >> @@ -361,7 +716,6 @@ static int __devinit samsung_usbphy_probe(struct platform_device *pdev) >> >> sphy->plat = pdata; >> sphy->regs = phy_base; >> - sphy->clk = clk; >> sphy->phy.dev = sphy->dev; >> sphy->phy.label = "samsung-usbphy"; >> sphy->phy.init = samsung_usbphy_init; >> @@ -371,6 +725,17 @@ static int __devinit samsung_usbphy_probe(struct platform_device *pdev) >> >> platform_set_drvdata(pdev, sphy); >> >> + if (sphy->cpu_type == TYPE_EXYNOS5250) >> + clk = devm_clk_get(dev, "usbhost"); > > Why is 5250 special in that it is always host? Something doesn't seem > right here. > exynos5250 onward usb-phy block gets its clock from 'usb host' unlike earlier SoCs where it was getting the clock from 'otg' So this change was required. > ...also, are you sure you want to move this down to after > platform_set_drvdata()? I don't think it will hurt anything, but > seems better to leave it where it was and just make call > samsung_usbphy_get_driver_data(pdev) earlier in the function. > Ok, will amend this as suggested. -- Thanks & Regards Vivek -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html