Hi, > From: Vivek Gautam [mailto:gautamvivek1987@xxxxxxxxx] > Sent: Thursday, December 26, 2013 11:32 AM > > Hi Kamil, > > > On Fri, Dec 20, 2013 at 6:54 PM, Kamil Debski <k.debski@xxxxxxxxxxx> > wrote: > > Add support to PHY of USB2 of the Exynos 5250 SoC. > > > > Signed-off-by: Kamil Debski <k.debski@xxxxxxxxxxx> > > --- > > arch/arm/boot/dts/exynos5250.dtsi | 33 ++++++++++++------- > > drivers/phy/phy-exynos5250-usb2.c | 64 > +++++++++++++++++++++++++++++++++---- > > 2 files changed, 78 insertions(+), 19 deletions(-) > > > > diff --git a/arch/arm/boot/dts/exynos5250.dtsi > > b/arch/arm/boot/dts/exynos5250.dtsi > > index 2f264ad..922e0ed 100644 > > --- a/arch/arm/boot/dts/exynos5250.dtsi > > +++ b/arch/arm/boot/dts/exynos5250.dtsi > > @@ -163,6 +163,11 @@ > > interrupts = <0 47 0>; > > }; > > > > + sys_syscon: syscon@10040000 { > > + compatible = "samsung,exynos5250-sys", "syscon"; > > + reg = <0x10050000 0x5000>; > > + }; > > + > > pmu_syscon: syscon@10040000 { > > compatible = "samsung,exynos5250-pmu", "syscon"; > > reg = <0x10040000 0x5000>; @@ -505,6 +510,14 @@ > > > > clocks = <&clock 285>; > > clock-names = "usbhost"; > > + #address-cells = <1>; > > + #size-cells = <0>; > > + port@0 { > > + reg = <0>; > > + phys = <&usb2_phy 1>; > > + phy-names = "host"; > > + status = "ok"; > > + }; > > }; > > > > usb@12120000 { > > @@ -516,19 +529,15 @@ > > clock-names = "usbhost"; > > }; > > > > - usb2_phy: usbphy@12130000 { > > - compatible = "samsung,exynos5250-usb2phy"; > > + usb2_phy: phy@12130000 { > > + compatible = "samsung,exynos5250-usb2-phy"; > > reg = <0x12130000 0x100>; > > - clocks = <&clock 1>, <&clock 285>; > > - clock-names = "ext_xtal", "usbhost"; > > - #address-cells = <1>; > > - #size-cells = <1>; > > - ranges; > > - > > - usbphy-sys { > > - reg = <0x10040704 0x8>, > > - <0x10050230 0x4>; > > - }; > > + clocks = <&clock 285>, <&clock 1>, <&clock 1>, > <&clock 1>, > > + > <&clock 1>; > > + clock-names = "phy", "device", "host", "hsic0", > "hsic1"; > > + #phy-cells = <1>; > > + samsung,sysreg-phandle = <&sys_syscon>; > > + samsung,pmureg-phandle = <&pmu_syscon>; > > }; > > > > amba { > > diff --git a/drivers/phy/phy-exynos5250-usb2.c > > b/drivers/phy/phy-exynos5250-usb2.c > > index b9b3b98..337bf82 100644 > > --- a/drivers/phy/phy-exynos5250-usb2.c > > +++ b/drivers/phy/phy-exynos5250-usb2.c > > Separate patches for dt and driver ? > I think you wanted to move these changes to : > [PATCH v5 7/9] phy: Add Exynos 5250 support to the Exynos USB 2.0 PHY > driver Good point. I am planning to reorganise this patchset to prevent breaking git bisect. I wanted to wait for more comments to this version, so I could address any issues that may be reported. > > > @@ -58,7 +58,13 @@ > > #define EXYNOS_5250_HOSTPHYCTRL2 0x20 > > Shouldn't we leave the naming as EXYNOS_5250_HSICPHYCTRL2 instead of > EXYNOS_5250_HOSTPHYCTRL2 ? That will go in sync with the user-manual > too. > and similar for EXYNOS_5250_HOSTPHYCTRL1 and below bit definitions too > ? Thank you for pointing this out. > > > > #define EXYNOS_5250_HOSTPHYCTRLX_REFCLKSEL_MASK (0x3 > << 23) > > +#define EXYNOS_5250_HOSTPHYCTRLX_REFCLKSEL_DEFAULT (0x2 << 23) > > #define EXYNOS_5250_HOSTPHYCTRLX_REFCLKDIV_MASK (0x7f > << 16) > > +#define EXYNOS_5250_HOSTPHYCTRLX_REFCLKDIV_12 (0x24 << 16) > > +#define EXYNOS_5250_HOSTPHYCTRLX_REFCLKDIV_15 (0x1c << 16) > > +#define EXYNOS_5250_HOSTPHYCTRLX_REFCLKDIV_16 (0x1a << 16) > > +#define EXYNOS_5250_HOSTPHYCTRLX_REFCLKDIV_19_2 (0x15 > << 16) > > +#define EXYNOS_5250_HOSTPHYCTRLX_REFCLKDIV_20 (0x14 << 16) > > #define EXYNOS_5250_HOSTPHYCTRLX_SIDDQ BIT(6) > > #define EXYNOS_5250_HOSTPHYCTRLX_FORCESLEEP BIT(5) > > #define EXYNOS_5250_HOSTPHYCTRLX_FORCESUSPEND BIT(4) > > @@ -191,13 +197,14 @@ static void exynos5250_isol(struct > samsung_usb2_phy_instance *inst, bool on) > > regmap_update_bits(drv->reg_pmu, offset, mask, on ? 0 : > mask); > > } > > > > -static void exynos5250_phy_pwr(struct samsung_usb2_phy_instance > > *inst, bool on) > > +static int exynos5250_power_on(struct samsung_usb2_phy_instance > > +*inst) > > void ? we really don't have much to return in this function. Initially the idea was to enable the return of an error code. However, I see that for all currently supported SoCs the ops always returns 0. So I will consider switching to void. > > > { > > struct samsung_usb2_phy_driver *drv = inst->drv; > > u32 ctrl0; > > u32 otg; > > u32 ehci; > > u32 ohci; > > + u32 hsic; > > > > switch (inst->cfg->id) { > > case EXYNOS5250_DEVICE: > > @@ -234,6 +241,8 @@ static void exynos5250_phy_pwr(struct > > samsung_usb2_phy_instance *inst, bool on) > > > > break; > > case EXYNOS5250_HOST: > > + case EXYNOS5250_HSIC0: > > + case EXYNOS5250_HSIC1: > > /* Host registers configuration */ > > ctrl0 = readl(drv->reg_phy + > EXYNOS_5250_HOSTPHYCTRL0); > > /* The clock */ > > @@ -279,6 +288,18 @@ static void exynos5250_phy_pwr(struct > samsung_usb2_phy_instance *inst, bool on) > > EXYNOS_5250_USBOTGSYS_LINK_SW_RST_UOTG | > > EXYNOS_5250_USBOTGSYS_PHYLINK_SW_RESET); > > > > + /* HSIC phy configuration */ > > + hsic = (EXYNOS_5250_HOSTPHYCTRLX_REFCLKDIV_12 | > > + > EXYNOS_5250_HOSTPHYCTRLX_REFCLKSEL_DEFAULT | > > + EXYNOS_5250_HOSTPHYCTRLX_PHYSWRST); > > + writel(hsic, drv->reg_phy + > EXYNOS_5250_HOSTPHYCTRL1); > > + writel(hsic, drv->reg_phy + > EXYNOS_5250_HOSTPHYCTRL2); > > + udelay(10); > > + hsic &= ~EXYNOS_5250_HOSTPHYCTRLX_PHYSWRST; > > + writel(hsic, drv->reg_phy + > EXYNOS_5250_HOSTPHYCTRL1); > > + writel(hsic, drv->reg_phy + > EXYNOS_5250_HOSTPHYCTRL2); > > + udelay(80); > > + > > /* Enable EHCI DMA burst */ > > ehci = readl(drv->reg_phy + > EXYNOS_5250_HOSTEHCICTRL); > > ehci |= EXYNOS_5250_HOSTEHCICTRL_ENAINCRXALIGN | @@ > > -295,12 +316,7 @@ static void exynos5250_phy_pwr(struct > > samsung_usb2_phy_instance *inst, bool on) > > > > break; > > } > > -} > > - > > -static int exynos5250_power_on(struct samsung_usb2_phy_instance > > *inst) -{ > > inst->enabled = 1; > > - exynos5250_phy_pwr(inst, 1); > > exynos5250_isol(inst, 0); > > > > return 0; > > @@ -308,9 +324,43 @@ static int exynos5250_power_on(struct > > samsung_usb2_phy_instance *inst) > > > > static int exynos5250_power_off(struct samsung_usb2_phy_instance > > *inst) > > ditto > > > { > > + struct samsung_usb2_phy_driver *drv = inst->drv; > > + u32 ctrl0; > > + u32 otg; > > + u32 hsic; > > + > > inst->enabled = 0; > > exynos5250_isol(inst, 1); > > - exynos5250_phy_pwr(inst, 0); > > + > > + switch (inst->cfg->id) { > > + case EXYNOS5250_DEVICE: > > + otg = readl(drv->reg_phy + EXYNOS_5250_USBOTGSYS); > > + otg |= (EXYNOS_5250_USBOTGSYS_FORCE_SUSPEND | > > + EXYNOS_5250_USBOTGSYS_SIDDQ_UOTG | > > + EXYNOS_5250_USBOTGSYS_FORCE_SLEEP); > > + writel(otg, drv->reg_phy + EXYNOS_5250_USBOTGSYS); > > + break; > > + case EXYNOS5250_HOST: > > + ctrl0 = readl(drv->reg_phy + > EXYNOS_5250_HOSTPHYCTRL0); > > + ctrl0 |= (EXYNOS_5250_HOSTPHYCTRL0_SIDDQ | > > + EXYNOS_5250_HOSTPHYCTRL0_FORCESUSPEND > | > > + EXYNOS_5250_HOSTPHYCTRL0_FORCESLEEP | > > + EXYNOS_5250_HOSTPHYCTRL0_PHYSWRST | > > + > EXYNOS_5250_HOSTPHYCTRL0_PHYSWRSTALL); > > + writel(ctrl0, drv->reg_phy + > EXYNOS_5250_HOSTPHYCTRL0); > > + break; > > + case EXYNOS5250_HSIC0: > > + case EXYNOS5250_HSIC1: > > + hsic = (EXYNOS_5250_HOSTPHYCTRLX_REFCLKDIV_12 | > > + > EXYNOS_5250_HOSTPHYCTRLX_REFCLKSEL_DEFAULT | > > + EXYNOS_5250_HOSTPHYCTRLX_SIDDQ | > > + EXYNOS_5250_HOSTPHYCTRLX_FORCESLEEP | > > + EXYNOS_5250_HOSTPHYCTRLX_FORCESUSPEND > > + ); > > + writel(hsic, drv->reg_phy + > EXYNOS_5250_HOSTPHYCTRL1); > > + writel(hsic, drv->reg_phy + > EXYNOS_5250_HOSTPHYCTRL2); > > + break; > > + } > > > > return 0; > > } > > -- > > 1.7.9.5 > > > > -- > > To unsubscribe from this list: send the line "unsubscribe > > linux-samsung-soc" in the body of a message to > > majordomo@xxxxxxxxxxxxxxx More majordomo info at > > http://vger.kernel.org/majordomo-info.html > > > > -- > Best Regards > Vivek Gautam > Samsung R&D Institute, Bangalore > India Best wishes, -- Kamil Debski Samsung R&D Institute Poland -- 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
