| Message ID | 1388655719-8851-2-git-send-email-inderpal.singh@linaro.org |
|---|---|
| State | Accepted |
| Delegated to: | Marek Vasut |
| Headers | show |
On Thursday, January 02, 2014 at 10:41:58 AM, Inderpal Singh wrote: > From: Inderpal Singh <chander.kashyap@linaro.org> > > The controller has 3 ports. The port0 is for USB 2.0 Phy, port1 and port2 > are for HSIC phys. The usb 2.0 phy is already being setup. This patch > sets up the hsic phys. > > Signed-off-by: Inderpal Singh <inderpal.singh@linaro.org> > --- > arch/arm/include/asm/arch-exynos/ehci.h | 14 +++++++++++ > drivers/usb/host/ehci-exynos.c | 39 > +++++++++++++++++++++++++++++++ 2 files changed, 53 insertions(+) > Is it OK to set all the ports up unconditionally ? I am not sure about exynos of course, but is it possible there are some machines which don't use the HSIC ports and this would have some kind of adverse effects on those? Best regards, Marek Vasut
Hi Marek, Thanks for the review. On 3 January 2014 06:24, Marek Vasut <marex@denx.de> wrote: > On Thursday, January 02, 2014 at 10:41:58 AM, Inderpal Singh wrote: > > From: Inderpal Singh <chander.kashyap@linaro.org> > > > > The controller has 3 ports. The port0 is for USB 2.0 Phy, port1 and port2 > > are for HSIC phys. The usb 2.0 phy is already being setup. This patch > > sets up the hsic phys. > > > > Signed-off-by: Inderpal Singh <inderpal.singh@linaro.org> > > --- > > arch/arm/include/asm/arch-exynos/ehci.h | 14 +++++++++++ > > drivers/usb/host/ehci-exynos.c | 39 > > +++++++++++++++++++++++++++++++ 2 files changed, 53 insertions(+) > > > > Is it OK to set all the ports up unconditionally ? I am not sure about > exynos of > course, but is it possible there are some machines which don't use the HSIC > ports and this would have some kind of adverse effects on those? > > I feel it should not cause any side effect as it wont interfere with the normal USB 2.0 phy port. Also, its being done along the same lines as kernel driver at drivers/usb/phy/phy-samsung-usb2.c, which also sets up all ports unconditionally. Regards, Inder > Best regards, > Marek Vasut >
On Friday, January 03, 2014 at 06:03:47 AM, Inderpal Singh wrote: > Hi Marek, > > Thanks for the review. > > On 3 January 2014 06:24, Marek Vasut <marex@denx.de> wrote: > > On Thursday, January 02, 2014 at 10:41:58 AM, Inderpal Singh wrote: > > > From: Inderpal Singh <chander.kashyap@linaro.org> > > > > > > The controller has 3 ports. The port0 is for USB 2.0 Phy, port1 and > > > port2 are for HSIC phys. The usb 2.0 phy is already being setup. This > > > patch sets up the hsic phys. > > > > > > Signed-off-by: Inderpal Singh <inderpal.singh@linaro.org> > > > --- > > > > > > arch/arm/include/asm/arch-exynos/ehci.h | 14 +++++++++++ > > > drivers/usb/host/ehci-exynos.c | 39 > > > > > > +++++++++++++++++++++++++++++++ 2 files changed, 53 insertions(+) > > > > Is it OK to set all the ports up unconditionally ? I am not sure about > > exynos of > > course, but is it possible there are some machines which don't use the > > HSIC ports and this would have some kind of adverse effects on those? > > I feel it should not cause any side effect as it wont interfere with the > normal USB 2.0 phy port. Also, its being done along the same lines as > kernel driver at drivers/usb/phy/phy-samsung-usb2.c, which also sets up all > ports unconditionally. OK, I won't fight this. I would be much more inclined to being able to conditionally select which ports get configured. Especially, since you do know that information from DT, dont you? Best regards, Marek Vasut
On 4 January 2014 12:46, Marek Vasut <marex@denx.de> wrote: > On Friday, January 03, 2014 at 06:03:47 AM, Inderpal Singh wrote: > > Hi Marek, > > > > Thanks for the review. > > > > On 3 January 2014 06:24, Marek Vasut <marex@denx.de> wrote: > > > On Thursday, January 02, 2014 at 10:41:58 AM, Inderpal Singh wrote: > > > > From: Inderpal Singh <chander.kashyap@linaro.org> > > > > > > > > The controller has 3 ports. The port0 is for USB 2.0 Phy, port1 and > > > > port2 are for HSIC phys. The usb 2.0 phy is already being setup. This > > > > patch sets up the hsic phys. > > > > > > > > Signed-off-by: Inderpal Singh <inderpal.singh@linaro.org> > > > > --- > > > > > > > > arch/arm/include/asm/arch-exynos/ehci.h | 14 +++++++++++ > > > > drivers/usb/host/ehci-exynos.c | 39 > > > > > > > > +++++++++++++++++++++++++++++++ 2 files changed, 53 insertions(+) > > > > > > Is it OK to set all the ports up unconditionally ? I am not sure about > > > exynos of > > > course, but is it possible there are some machines which don't use the > > > HSIC ports and this would have some kind of adverse effects on those? > > > > I feel it should not cause any side effect as it wont interfere with the > > normal USB 2.0 phy port. Also, its being done along the same lines as > > kernel driver at drivers/usb/phy/phy-samsung-usb2.c, which also sets up > all > > ports unconditionally. > > OK, I won't fight this. I would be much more inclined to being able to > conditionally select which ports get configured. Especially, since you do > know > that information from DT, dont you? > Ok, Thanks Marek. As of now DT is not providing port information. Regards, Inder > Best regards, > Marek Vasut >
On Monday, January 06, 2014 at 07:20:20 AM, Inderpal Singh wrote: > On 4 January 2014 12:46, Marek Vasut <marex@denx.de> wrote: > > On Friday, January 03, 2014 at 06:03:47 AM, Inderpal Singh wrote: > > > Hi Marek, > > > > > > Thanks for the review. > > > > > > On 3 January 2014 06:24, Marek Vasut <marex@denx.de> wrote: > > > > On Thursday, January 02, 2014 at 10:41:58 AM, Inderpal Singh wrote: > > > > > From: Inderpal Singh <chander.kashyap@linaro.org> > > > > > > > > > > The controller has 3 ports. The port0 is for USB 2.0 Phy, port1 and > > > > > port2 are for HSIC phys. The usb 2.0 phy is already being setup. > > > > > This patch sets up the hsic phys. > > > > > > > > > > Signed-off-by: Inderpal Singh <inderpal.singh@linaro.org> > > > > > --- > > > > > > > > > > arch/arm/include/asm/arch-exynos/ehci.h | 14 +++++++++++ > > > > > drivers/usb/host/ehci-exynos.c | 39 > > > > > > > > > > +++++++++++++++++++++++++++++++ 2 files changed, 53 insertions(+) > > > > > > > > Is it OK to set all the ports up unconditionally ? I am not sure > > > > about exynos of > > > > course, but is it possible there are some machines which don't use > > > > the HSIC ports and this would have some kind of adverse effects on > > > > those? > > > > > > I feel it should not cause any side effect as it wont interfere with > > > the normal USB 2.0 phy port. Also, its being done along the same lines > > > as kernel driver at drivers/usb/phy/phy-samsung-usb2.c, which also > > > sets up > > > > all > > > > > ports unconditionally. > > > > OK, I won't fight this. I would be much more inclined to being able to > > conditionally select which ports get configured. Especially, since you do > > know > > that information from DT, dont you? > > Ok, Thanks Marek. > As of now DT is not providing port information. Bah, I'd expect -- especially in case of exynos, which is targetting the mobile segment -- to focus on power consumption very much. Anyway, like I said, I won't fight this. Minkyu, what's your take on this one please? Best regards, Marek Vasut
On 07/01/14 00:45, Marek Vasut wrote: > On Monday, January 06, 2014 at 07:20:20 AM, Inderpal Singh wrote: >> On 4 January 2014 12:46, Marek Vasut <marex@denx.de> wrote: >>> On Friday, January 03, 2014 at 06:03:47 AM, Inderpal Singh wrote: >>>> Hi Marek, >>>> >>>> Thanks for the review. >>>> >>>> On 3 January 2014 06:24, Marek Vasut <marex@denx.de> wrote: >>>>> On Thursday, January 02, 2014 at 10:41:58 AM, Inderpal Singh wrote: >>>>>> From: Inderpal Singh <chander.kashyap@linaro.org> >>>>>> >>>>>> The controller has 3 ports. The port0 is for USB 2.0 Phy, port1 and >>>>>> port2 are for HSIC phys. The usb 2.0 phy is already being setup. >>>>>> This patch sets up the hsic phys. >>>>>> >>>>>> Signed-off-by: Inderpal Singh <inderpal.singh@linaro.org> >>>>>> --- >>>>>> >>>>>> arch/arm/include/asm/arch-exynos/ehci.h | 14 +++++++++++ >>>>>> drivers/usb/host/ehci-exynos.c | 39 >>>>>> >>>>>> +++++++++++++++++++++++++++++++ 2 files changed, 53 insertions(+) >>>>> >>>>> Is it OK to set all the ports up unconditionally ? I am not sure >>>>> about exynos of >>>>> course, but is it possible there are some machines which don't use >>>>> the HSIC ports and this would have some kind of adverse effects on >>>>> those? >>>> >>>> I feel it should not cause any side effect as it wont interfere with >>>> the normal USB 2.0 phy port. Also, its being done along the same lines >>>> as kernel driver at drivers/usb/phy/phy-samsung-usb2.c, which also >>>> sets up >>> >>> all >>> >>>> ports unconditionally. >>> >>> OK, I won't fight this. I would be much more inclined to being able to >>> conditionally select which ports get configured. Especially, since you do >>> know >>> that information from DT, dont you? >> >> Ok, Thanks Marek. >> As of now DT is not providing port information. > > Bah, I'd expect -- especially in case of exynos, which is targetting the mobile > segment -- to focus on power consumption very much. Anyway, like I said, I won't > fight this. Minkyu, what's your take on this one please? I agree with Marek. Thanks, Minkyu Kang.
diff --git a/arch/arm/include/asm/arch-exynos/ehci.h b/arch/arm/include/asm/arch-exynos/ehci.h index d79f25c..d2d70bd 100644 --- a/arch/arm/include/asm/arch-exynos/ehci.h +++ b/arch/arm/include/asm/arch-exynos/ehci.h @@ -29,6 +29,20 @@ #define EHCICTRL_ENAINCR8 (1 << 27) #define EHCICTRL_ENAINCR16 (1 << 26) +#define HSIC_CTRL_REFCLKSEL (0x2) +#define HSIC_CTRL_REFCLKSEL_MASK (0x3) +#define HSIC_CTRL_REFCLKSEL_SHIFT (23) + +#define HSIC_CTRL_REFCLKDIV_12 (0x24) +#define HSIC_CTRL_REFCLKDIV_MASK (0x7f) +#define HSIC_CTRL_REFCLKDIV_SHIFT (16) + +#define HSIC_CTRL_SIDDQ (0x1 << 6) +#define HSIC_CTRL_FORCESLEEP (0x1 << 5) +#define HSIC_CTRL_FORCESUSPEND (0x1 << 4) +#define HSIC_CTRL_UTMISWRST (0x1 << 2) +#define HSIC_CTRL_PHYSWRST (0x1 << 0) + /* Register map for PHY control */ struct exynos_usb_phy { unsigned int usbphyctrl0; diff --git a/drivers/usb/host/ehci-exynos.c b/drivers/usb/host/ehci-exynos.c index 66b4de0..88e6466 100644 --- a/drivers/usb/host/ehci-exynos.c +++ b/drivers/usb/host/ehci-exynos.c @@ -88,6 +88,8 @@ static int exynos_usb_parse_dt(const void *blob, struct exynos_ehci *exynos) /* Setup the EHCI host controller. */ static void setup_usb_phy(struct exynos_usb_phy *usb) { + u32 hsic_ctrl; + set_usbhost_mode(USB20_PHY_CFG_HOST_LINK_EN); set_usbhost_phy_ctrl(POWER_USB_HOST_PHY_CTRL_EN); @@ -112,6 +114,32 @@ static void setup_usb_phy(struct exynos_usb_phy *usb) clrbits_le32(&usb->usbphyctrl0, HOST_CTRL0_LINKSWRST | HOST_CTRL0_UTMISWRST); + + /* HSIC Phy Setting */ + hsic_ctrl = (HSIC_CTRL_FORCESUSPEND | + HSIC_CTRL_FORCESLEEP | + HSIC_CTRL_SIDDQ); + + clrbits_le32(&usb->hsicphyctrl1, hsic_ctrl); + clrbits_le32(&usb->hsicphyctrl2, hsic_ctrl); + + hsic_ctrl = (((HSIC_CTRL_REFCLKDIV_12 & HSIC_CTRL_REFCLKDIV_MASK) + << HSIC_CTRL_REFCLKDIV_SHIFT) + | ((HSIC_CTRL_REFCLKSEL & HSIC_CTRL_REFCLKSEL_MASK) + << HSIC_CTRL_REFCLKSEL_SHIFT) + | HSIC_CTRL_UTMISWRST); + + setbits_le32(&usb->hsicphyctrl1, hsic_ctrl); + setbits_le32(&usb->hsicphyctrl2, hsic_ctrl); + + udelay(10); + + clrbits_le32(&usb->hsicphyctrl1, HSIC_CTRL_PHYSWRST | + HSIC_CTRL_UTMISWRST); + + clrbits_le32(&usb->hsicphyctrl2, HSIC_CTRL_PHYSWRST | + HSIC_CTRL_UTMISWRST); + udelay(20); /* EHCI Ctrl setting */ @@ -125,6 +153,8 @@ static void setup_usb_phy(struct exynos_usb_phy *usb) /* Reset the EHCI host controller. */ static void reset_usb_phy(struct exynos_usb_phy *usb) { + u32 hsic_ctrl; + /* HOST_PHY reset */ setbits_le32(&usb->usbphyctrl0, HOST_CTRL0_PHYSWRST | @@ -133,6 +163,15 @@ static void reset_usb_phy(struct exynos_usb_phy *usb) HOST_CTRL0_FORCESUSPEND | HOST_CTRL0_FORCESLEEP); + /* HSIC Phy reset */ + hsic_ctrl = (HSIC_CTRL_FORCESUSPEND | + HSIC_CTRL_FORCESLEEP | + HSIC_CTRL_SIDDQ | + HSIC_CTRL_PHYSWRST); + + setbits_le32(&usb->hsicphyctrl1, hsic_ctrl); + setbits_le32(&usb->hsicphyctrl2, hsic_ctrl); + set_usbhost_phy_ctrl(POWER_USB_HOST_PHY_CTRL_DISABLE); }