| Message ID | 1415077654-17181-3-git-send-email-marex@denx.de |
|---|---|
| State | Accepted |
| Delegated to: | Marek Vasut |
| Headers | show |
On Tue 2014-11-04 06:07:32, Marek Vasut wrote: > Split the Samsung specific PHY control into a separate file > and compile this into the S3C OTG driver only if used on a > Samsung system. > > Signed-off-by: Marek Vasut <marex@denx.de> > Cc: Chin Liang See <clsee@altera.com> > Cc: Dinh Nguyen <dinguyen@opensource.altera.com> > Cc: Vince Bridgers <vbridger@altera.com> Acked-by: Pavel Machek <pavel@denx.de> I know you are just moving the code, but... > +void otg_phy_init(struct s3c_udc *dev) > +{ > + unsigned int usb_phy_ctrl = dev->pdata->usb_phy_ctrl; > + struct s3c_usbotg_phy *phy = > + (struct s3c_usbotg_phy *)dev->pdata->regs_phy; > + > + dev->pdata->phy_control(1); > + > + /*USB PHY0 Enable */ Wrong comment style. > + printf("USB PHY0 Enable\n"); > + > + /* Enable PHY */ > + writel(readl(usb_phy_ctrl) | USB_PHY_CTRL_EN0, usb_phy_ctrl); We have helpers for setting/clearing bits, right?
Hi Marek, > Split the Samsung specific PHY control into a separate file > and compile this into the S3C OTG driver only if used on a > Samsung system. > > Signed-off-by: Marek Vasut <marex@denx.de> > Cc: Chin Liang See <clsee@altera.com> > Cc: Dinh Nguyen <dinguyen@opensource.altera.com> > Cc: Vince Bridgers <vbridger@altera.com> > Cc: Pavel Machek <pavel@denx.de> > Cc: Stefan Roese <sr@denx.de> > Cc: Lukasz Majewski <l.majewski@samsung.com> > --- > drivers/usb/gadget/Makefile | 1 + > drivers/usb/gadget/s3c_udc_otg.c | 64 +---------------------- > drivers/usb/gadget/s3c_udc_otg_phy.c | 99 > ++++++++++++++++++++++++++++++++++++ > include/configs/exynos4-common.h | 1 + > include/configs/s5p_goni.h | 1 + > include/configs/s5pc210_universal.h | 1 + > include/configs/smdkv310.h | 1 + 7 files changed, 106 > insertions(+), 62 deletions(-) create mode 100644 > drivers/usb/gadget/s3c_udc_otg_phy.c > > diff --git a/drivers/usb/gadget/Makefile b/drivers/usb/gadget/Makefile > index 2efd5a4..70bb550 100644 > --- a/drivers/usb/gadget/Makefile > +++ b/drivers/usb/gadget/Makefile > @@ -12,6 +12,7 @@ obj-$(CONFIG_USB_ETHER) += epautoconf.o config.o > usbstring.o ifdef CONFIG_USB_GADGET > obj-$(CONFIG_USB_GADGET_ATMEL_USBA) += atmel_usba_udc.o > obj-$(CONFIG_USB_GADGET_S3C_UDC_OTG) += s3c_udc_otg.o > +obj-$(CONFIG_USB_GADGET_S3C_UDC_OTG_PHY) += s3c_udc_otg_phy.o > obj-$(CONFIG_USB_GADGET_FOTG210) += fotg210.o > obj-$(CONFIG_CI_UDC) += ci_udc.o > obj-$(CONFIG_THOR_FUNCTION) += f_thor.o > diff --git a/drivers/usb/gadget/s3c_udc_otg.c > b/drivers/usb/gadget/s3c_udc_otg.c index b808ddaf..b910053 100644 > --- a/drivers/usb/gadget/s3c_udc_otg.c > +++ b/drivers/usb/gadget/s3c_udc_otg.c > @@ -151,68 +151,8 @@ bool dfu_usb_get_reset(void) > return !!(readl(®->gintsts) & INT_RESET); > } > > -void otg_phy_init(struct s3c_udc *dev) > -{ > - unsigned int usb_phy_ctrl = dev->pdata->usb_phy_ctrl; > - struct s3c_usbotg_phy *phy = > - (struct s3c_usbotg_phy *)dev->pdata->regs_phy; > - > - dev->pdata->phy_control(1); > - > - /*USB PHY0 Enable */ > - printf("USB PHY0 Enable\n"); > - > - /* Enable PHY */ > - writel(readl(usb_phy_ctrl) | USB_PHY_CTRL_EN0, usb_phy_ctrl); > - > - if (dev->pdata->usb_flags == PHY0_SLEEP) /* C210 Universal */ > - writel((readl(&phy->phypwr) > - &~(PHY_0_SLEEP | OTG_DISABLE_0 | > ANALOG_PWRDOWN) > - &~FORCE_SUSPEND_0), &phy->phypwr); > - else /* C110 GONI */ > - writel((readl(&phy->phypwr) &~(OTG_DISABLE_0 | > ANALOG_PWRDOWN) > - &~FORCE_SUSPEND_0), &phy->phypwr); > - > - if (s5p_cpu_id == 0x4412) > - writel((readl(&phy->phyclk) & > ~(EXYNOS4X12_ID_PULLUP0 | > - EXYNOS4X12_COMMON_ON_N0)) | > EXYNOS4X12_CLK_SEL_24MHZ, > - &phy->phyclk); /* PLL 24Mhz */ > - else > - writel((readl(&phy->phyclk) & ~(ID_PULLUP0 | > COMMON_ON_N0)) | > - CLK_SEL_24MHZ, &phy->phyclk); /* PLL 24Mhz */ > - > - writel((readl(&phy->rstcon) &~(LINK_SW_RST | PHYLNK_SW_RST)) > - | PHY_SW_RST0, &phy->rstcon); > - udelay(10); > - writel(readl(&phy->rstcon) > - &~(PHY_SW_RST0 | LINK_SW_RST | PHYLNK_SW_RST), > &phy->rstcon); > - udelay(10); > -} > - > -void otg_phy_off(struct s3c_udc *dev) > -{ > - unsigned int usb_phy_ctrl = dev->pdata->usb_phy_ctrl; > - struct s3c_usbotg_phy *phy = > - (struct s3c_usbotg_phy *)dev->pdata->regs_phy; > - > - /* reset controller just in case */ > - writel(PHY_SW_RST0, &phy->rstcon); > - udelay(20); > - writel(readl(&phy->phypwr) &~PHY_SW_RST0, &phy->rstcon); > - udelay(20); > - > - writel(readl(&phy->phypwr) | OTG_DISABLE_0 | ANALOG_PWRDOWN > - | FORCE_SUSPEND_0, &phy->phypwr); > - > - writel(readl(usb_phy_ctrl) &~USB_PHY_CTRL_EN0, usb_phy_ctrl); > - > - writel((readl(&phy->phyclk) & ~(ID_PULLUP0 | COMMON_ON_N0)), > - &phy->phyclk); > - > - udelay(10000); > - > - dev->pdata->phy_control(0); > -} > +__weak void otg_phy_init(struct s3c_udc *dev) {} > +__weak void otg_phy_off(struct s3c_udc *dev) {} > > /***********************************************************/ > > diff --git a/drivers/usb/gadget/s3c_udc_otg_phy.c > b/drivers/usb/gadget/s3c_udc_otg_phy.c new file mode 100644 > index 0000000..055fe86 > --- /dev/null > +++ b/drivers/usb/gadget/s3c_udc_otg_phy.c > @@ -0,0 +1,99 @@ > +/* > + * drivers/usb/gadget/s3c_udc_otg.c > + * Samsung S3C on-chip full/high speed USB OTG 2.0 device controllers > + * > + * Copyright (C) 2008 for Samsung Electronics > + * > + * BSP Support for Samsung's UDC driver > + * available at: > + * > git://git.kernel.org/pub/scm/linux/kernel/git/kki_ap/linux-2.6-samsung.git > + * > + * State machine bugfixes: > + * Marek Szyprowski <m.szyprowski@samsung.com> > + * > + * Ported to u-boot: > + * Marek Szyprowski <m.szyprowski@samsung.com> > + * Lukasz Majewski <l.majewski@samsumg.com> > + * > + * SPDX-License-Identifier: GPL-2.0+ > + */ > + > +#include <common.h> > +#include <asm/errno.h> > +#include <linux/list.h> > +#include <malloc.h> > + > +#include <linux/usb/ch9.h> > +#include <linux/usb/gadget.h> > + > +#include <asm/byteorder.h> > +#include <asm/unaligned.h> > +#include <asm/io.h> > + > +#include <asm/mach-types.h> > + > +#include "regs-otg.h" > +#include <usb/lin_gadget_compat.h> > + > +void otg_phy_init(struct s3c_udc *dev) > +{ > + unsigned int usb_phy_ctrl = dev->pdata->usb_phy_ctrl; > + struct s3c_usbotg_phy *phy = > + (struct s3c_usbotg_phy *)dev->pdata->regs_phy; > + > + dev->pdata->phy_control(1); > + > + /*USB PHY0 Enable */ > + printf("USB PHY0 Enable\n"); > + > + /* Enable PHY */ > + writel(readl(usb_phy_ctrl) | USB_PHY_CTRL_EN0, usb_phy_ctrl); > + > + if (dev->pdata->usb_flags == PHY0_SLEEP) /* C210 Universal */ > + writel((readl(&phy->phypwr) > + &~(PHY_0_SLEEP | OTG_DISABLE_0 | > ANALOG_PWRDOWN) > + &~FORCE_SUSPEND_0), &phy->phypwr); > + else /* C110 GONI */ > + writel((readl(&phy->phypwr) &~(OTG_DISABLE_0 | > ANALOG_PWRDOWN) > + &~FORCE_SUSPEND_0), &phy->phypwr); > + > + if (s5p_cpu_id == 0x4412) > + writel((readl(&phy->phyclk) & > ~(EXYNOS4X12_ID_PULLUP0 | > + EXYNOS4X12_COMMON_ON_N0)) | > EXYNOS4X12_CLK_SEL_24MHZ, > + &phy->phyclk); /* PLL 24Mhz */ > + else > + writel((readl(&phy->phyclk) & ~(ID_PULLUP0 | > COMMON_ON_N0)) | > + CLK_SEL_24MHZ, &phy->phyclk); /* PLL 24Mhz */ > + > + writel((readl(&phy->rstcon) &~(LINK_SW_RST | PHYLNK_SW_RST)) > + | PHY_SW_RST0, &phy->rstcon); > + udelay(10); > + writel(readl(&phy->rstcon) > + &~(PHY_SW_RST0 | LINK_SW_RST | PHYLNK_SW_RST), > &phy->rstcon); > + udelay(10); > +} > + > +void otg_phy_off(struct s3c_udc *dev) > +{ > + unsigned int usb_phy_ctrl = dev->pdata->usb_phy_ctrl; > + struct s3c_usbotg_phy *phy = > + (struct s3c_usbotg_phy *)dev->pdata->regs_phy; > + > + /* reset controller just in case */ > + writel(PHY_SW_RST0, &phy->rstcon); > + udelay(20); > + writel(readl(&phy->phypwr) &~PHY_SW_RST0, &phy->rstcon); > + udelay(20); > + > + writel(readl(&phy->phypwr) | OTG_DISABLE_0 | ANALOG_PWRDOWN > + | FORCE_SUSPEND_0, &phy->phypwr); > + > + writel(readl(usb_phy_ctrl) &~USB_PHY_CTRL_EN0, usb_phy_ctrl); > + > + writel((readl(&phy->phyclk) & ~(ID_PULLUP0 | COMMON_ON_N0)), > + &phy->phyclk); > + > + udelay(10000); > + > + dev->pdata->phy_control(0); > +} > diff --git a/include/configs/exynos4-common.h > b/include/configs/exynos4-common.h index 89ba14e..41631c7 100644 > --- a/include/configs/exynos4-common.h > +++ b/include/configs/exynos4-common.h > @@ -59,6 +59,7 @@ > > #define CONFIG_USB_GADGET > #define CONFIG_USB_GADGET_S3C_UDC_OTG > +#define CONFIG_USB_GADGET_S3C_UDC_OTG_PHY > #define CONFIG_USB_GADGET_DUALSPEED > #define CONFIG_USB_GADGET_VBUS_DRAW 2 > > diff --git a/include/configs/s5p_goni.h b/include/configs/s5p_goni.h > index 3633a35..dfa2e07 100644 > --- a/include/configs/s5p_goni.h > +++ b/include/configs/s5p_goni.h > @@ -281,6 +281,7 @@ > #define CONFIG_SYS_MAX_I2C_BUS 7 > #define CONFIG_USB_GADGET > #define CONFIG_USB_GADGET_S3C_UDC_OTG > +#define CONFIG_USB_GADGET_S3C_UDC_OTG_PHY > #define CONFIG_USB_GADGET_DUALSPEED > #define CONFIG_USB_GADGET_VBUS_DRAW 2 > #define CONFIG_CMD_USB_MASS_STORAGE > diff --git a/include/configs/s5pc210_universal.h > b/include/configs/s5pc210_universal.h index 4b30d148..e7bace4 100644 > --- a/include/configs/s5pc210_universal.h > +++ b/include/configs/s5pc210_universal.h > @@ -181,6 +181,7 @@ > > #define CONFIG_USB_GADGET > #define CONFIG_USB_GADGET_S3C_UDC_OTG > +#define CONFIG_USB_GADGET_S3C_UDC_OTG_PHY > #define CONFIG_USB_GADGET_DUALSPEED > > /* > diff --git a/include/configs/smdkv310.h b/include/configs/smdkv310.h > index a2469eb..655025c 100644 > --- a/include/configs/smdkv310.h > +++ b/include/configs/smdkv310.h > @@ -14,6 +14,7 @@ > #undef CONFIG_BOARD_COMMON > #undef CONFIG_USB_GADGET > #undef CONFIG_USB_GADGET_S3C_UDC_OTG > +#undef CONFIG_USB_GADGET_S3C_UDC_OTG_PHY > #undef CONFIG_CMD_USB_MASS_STORAGE > #undef CONFIG_REVISION_TAG > #undef CONFIG_CMD_THOR_DOWNLOAD Unfortunately I've found following build errors: CC drivers/usb/gadget/s3c_udc_otg_phy.o drivers/usb/gadget/s3c_udc_otg_phy.c:38:26: warning: 'struct s3c_udc' declared inside parameter list [enabled by default] void otg_phy_init(struct s3c_udc *dev) ^ drivers/usb/gadget/s3c_udc_otg_phy.c:38:26: warning: its scope is only this definition or declaration, which is probably not what you want [enabled by default] drivers/usb/gadget/s3c_udc_otg_phy.c: In function 'otg_phy_init': drivers/usb/gadget/s3c_udc_otg_phy.c:40:33: error: dereferencing pointer to incomplete type unsigned int usb_phy_ctrl = dev->pdata->usb_phy_ctrl; ^ drivers/usb/gadget/s3c_udc_otg_phy.c:42:31: error: dereferencing pointer to incomplete type (struct s3c_usbotg_phy *)dev->pdata->regs_phy; ^ drivers/usb/gadget/s3c_udc_otg_phy.c:44:5: error: dereferencing pointer to incomplete type dev->pdata->phy_control(1); ^ drivers/usb/gadget/s3c_udc_otg_phy.c:52:9: error: dereferencing pointer to incomplete type if (dev->pdata->usb_flags == PHY0_SLEEP) /* C210 Universal */ ^ drivers/usb/gadget/s3c_udc_otg_phy.c:52:31: error: 'PHY0_SLEEP' undeclared (first use in this function) if (dev->pdata->usb_flags == PHY0_SLEEP) /* C210 Universal */ ^ drivers/usb/gadget/s3c_udc_otg_phy.c:52:31: note: each undeclared identifier is reported only once for each function it appears in drivers/usb/gadget/s3c_udc_otg_phy.c: At top level: drivers/usb/gadget/s3c_udc_otg_phy.c:76:25: warning: 'struct s3c_udc' declared inside parameter list [enabled by default] void otg_phy_off(struct s3c_udc *dev) ^ drivers/usb/gadget/s3c_udc_otg_phy.c: In function 'otg_phy_off': drivers/usb/gadget/s3c_udc_otg_phy.c:78:33: error: dereferencing pointer to incomplete type unsigned int usb_phy_ctrl = dev->pdata->usb_phy_ctrl; ^ drivers/usb/gadget/s3c_udc_otg_phy.c:80:31: error: dereferencing pointer to incomplete type (struct s3c_usbotg_phy *)dev->pdata->regs_phy; ^ drivers/usb/gadget/s3c_udc_otg_phy.c:98:5: error: dereferencing pointer to incomplete type dev->pdata->phy_control(0); ^ make[1]: *** [drivers/usb/gadget/s3c_udc_otg_phy.o] Error 1 make: *** [drivers/usb/gadget] Error 2 Build HW: trats2 (Exynos4412) CROSS_COMPILE=/home/lukma/work/ptxdist/toolchains/arm/OSELAS.Toolchain-2013.12.0/arm-v7a-linux-gnueabi/gcc-4.8.2-glibc-2.18-binutils-2.24-kernel-3.12-sanitized/bin/arm-v7a-linux-gnueabi- Patches applied on top of v2014.10.
On Tuesday, November 04, 2014 at 08:34:21 PM, Pavel Machek wrote: > On Tue 2014-11-04 06:07:32, Marek Vasut wrote: > > Split the Samsung specific PHY control into a separate file > > and compile this into the S3C OTG driver only if used on a > > Samsung system. > > > > Signed-off-by: Marek Vasut <marex@denx.de> > > Cc: Chin Liang See <clsee@altera.com> > > Cc: Dinh Nguyen <dinguyen@opensource.altera.com> > > Cc: Vince Bridgers <vbridger@altera.com> > > Acked-by: Pavel Machek <pavel@denx.de> > > I know you are just moving the code, but... > > > +void otg_phy_init(struct s3c_udc *dev) > > +{ > > + unsigned int usb_phy_ctrl = dev->pdata->usb_phy_ctrl; > > + struct s3c_usbotg_phy *phy = > > + (struct s3c_usbotg_phy *)dev->pdata->regs_phy; > > + > > + dev->pdata->phy_control(1); > > + > > + /*USB PHY0 Enable */ > > Wrong comment style. I'll fix this one. > > + printf("USB PHY0 Enable\n"); > > + > > + /* Enable PHY */ > > + writel(readl(usb_phy_ctrl) | USB_PHY_CTRL_EN0, usb_phy_ctrl); > > We have helpers for setting/clearing bits, right? Yes we do, Lukasz ... ? :) Best regards, Marek Vasut
Hi Marek, > On Tuesday, November 04, 2014 at 08:34:21 PM, Pavel Machek wrote: > > On Tue 2014-11-04 06:07:32, Marek Vasut wrote: > > > Split the Samsung specific PHY control into a separate file > > > and compile this into the S3C OTG driver only if used on a > > > Samsung system. > > > > > > Signed-off-by: Marek Vasut <marex@denx.de> > > > Cc: Chin Liang See <clsee@altera.com> > > > Cc: Dinh Nguyen <dinguyen@opensource.altera.com> > > > Cc: Vince Bridgers <vbridger@altera.com> > > > > Acked-by: Pavel Machek <pavel@denx.de> > > > > I know you are just moving the code, but... > > > > > +void otg_phy_init(struct s3c_udc *dev) > > > +{ > > > + unsigned int usb_phy_ctrl = dev->pdata->usb_phy_ctrl; > > > + struct s3c_usbotg_phy *phy = > > > + (struct s3c_usbotg_phy *)dev->pdata->regs_phy; > > > + > > > + dev->pdata->phy_control(1); > > > + > > > + /*USB PHY0 Enable */ > > > > Wrong comment style. > > I'll fix this one. > > > > + printf("USB PHY0 Enable\n"); > > > + > > > + /* Enable PHY */ > > > + writel(readl(usb_phy_ctrl) | USB_PHY_CTRL_EN0, > > > usb_phy_ctrl); > > > > We have helpers for setting/clearing bits, right? > > Yes we do, Lukasz ... ? :) If you think about set_bit()/ clear_bit() functions from ./arch/arm/include/asm/bitops.h, then I don't mind to replace current code with them. Feel free to use them for next version of the patch. > > Best regards, > Marek Vasut
On Friday, November 07, 2014 at 09:59:25 AM, Lukasz Majewski wrote: > Hi Marek, > > > On Tuesday, November 04, 2014 at 08:34:21 PM, Pavel Machek wrote: > > > On Tue 2014-11-04 06:07:32, Marek Vasut wrote: > > > > Split the Samsung specific PHY control into a separate file > > > > and compile this into the S3C OTG driver only if used on a > > > > Samsung system. > > > > > > > > Signed-off-by: Marek Vasut <marex@denx.de> > > > > Cc: Chin Liang See <clsee@altera.com> > > > > Cc: Dinh Nguyen <dinguyen@opensource.altera.com> > > > > Cc: Vince Bridgers <vbridger@altera.com> > > > > > > Acked-by: Pavel Machek <pavel@denx.de> > > > > > > I know you are just moving the code, but... > > > > > > > +void otg_phy_init(struct s3c_udc *dev) > > > > +{ > > > > + unsigned int usb_phy_ctrl = dev->pdata->usb_phy_ctrl; > > > > + struct s3c_usbotg_phy *phy = > > > > + (struct s3c_usbotg_phy *)dev->pdata->regs_phy; > > > > + > > > > + dev->pdata->phy_control(1); > > > > + > > > > + /*USB PHY0 Enable */ > > > > > > Wrong comment style. > > > > I'll fix this one. > > > > > > + printf("USB PHY0 Enable\n"); > > > > + > > > > + /* Enable PHY */ > > > > + writel(readl(usb_phy_ctrl) | USB_PHY_CTRL_EN0, > > > > usb_phy_ctrl); > > > > > > We have helpers for setting/clearing bits, right? > > > > Yes we do, Lukasz ... ? :) > > If you think about set_bit()/ clear_bit() functions > from ./arch/arm/include/asm/bitops.h, then I don't mind to replace > current code with them. > > Feel free to use them for next version of the patch. Pavel meant clrsetbits_le32() and friends . Personally, I would leave that to subsequent set :) Best regards, Marek Vasut
Hi Marek, > On Friday, November 07, 2014 at 09:59:25 AM, Lukasz Majewski wrote: > > Hi Marek, > > > > > On Tuesday, November 04, 2014 at 08:34:21 PM, Pavel Machek wrote: > > > > On Tue 2014-11-04 06:07:32, Marek Vasut wrote: > > > > > Split the Samsung specific PHY control into a separate file > > > > > and compile this into the S3C OTG driver only if used on a > > > > > Samsung system. > > > > > > > > > > Signed-off-by: Marek Vasut <marex@denx.de> > > > > > Cc: Chin Liang See <clsee@altera.com> > > > > > Cc: Dinh Nguyen <dinguyen@opensource.altera.com> > > > > > Cc: Vince Bridgers <vbridger@altera.com> > > > > > > > > Acked-by: Pavel Machek <pavel@denx.de> > > > > > > > > I know you are just moving the code, but... > > > > > > > > > +void otg_phy_init(struct s3c_udc *dev) > > > > > +{ > > > > > + unsigned int usb_phy_ctrl = dev->pdata->usb_phy_ctrl; > > > > > + struct s3c_usbotg_phy *phy = > > > > > + (struct s3c_usbotg_phy > > > > > *)dev->pdata->regs_phy; + > > > > > + dev->pdata->phy_control(1); > > > > > + > > > > > + /*USB PHY0 Enable */ > > > > > > > > Wrong comment style. > > > > > > I'll fix this one. > > > > > > > > + printf("USB PHY0 Enable\n"); > > > > > + > > > > > + /* Enable PHY */ > > > > > + writel(readl(usb_phy_ctrl) | USB_PHY_CTRL_EN0, > > > > > usb_phy_ctrl); > > > > > > > > We have helpers for setting/clearing bits, right? > > > > > > Yes we do, Lukasz ... ? :) > > > > If you think about set_bit()/ clear_bit() functions > > from ./arch/arm/include/asm/bitops.h, then I don't mind to replace > > current code with them. > > > > Feel free to use them for next version of the patch. > > Pavel meant clrsetbits_le32() and friends . Personally, I would leave > that to subsequent set :) This is up to you. I just would like to avoid changing too many things at once. > > Best regards, > Marek Vasut
On Friday, November 07, 2014 at 11:08:44 AM, Lukasz Majewski wrote: > Hi Marek, > > > On Friday, November 07, 2014 at 09:59:25 AM, Lukasz Majewski wrote: > > > Hi Marek, > > > > > > > On Tuesday, November 04, 2014 at 08:34:21 PM, Pavel Machek wrote: > > > > > On Tue 2014-11-04 06:07:32, Marek Vasut wrote: > > > > > > Split the Samsung specific PHY control into a separate file > > > > > > and compile this into the S3C OTG driver only if used on a > > > > > > Samsung system. > > > > > > > > > > > > Signed-off-by: Marek Vasut <marex@denx.de> > > > > > > Cc: Chin Liang See <clsee@altera.com> > > > > > > Cc: Dinh Nguyen <dinguyen@opensource.altera.com> > > > > > > Cc: Vince Bridgers <vbridger@altera.com> > > > > > > > > > > Acked-by: Pavel Machek <pavel@denx.de> > > > > > > > > > > I know you are just moving the code, but... > > > > > > > > > > > +void otg_phy_init(struct s3c_udc *dev) > > > > > > +{ > > > > > > + unsigned int usb_phy_ctrl = dev->pdata->usb_phy_ctrl; > > > > > > + struct s3c_usbotg_phy *phy = > > > > > > + (struct s3c_usbotg_phy > > > > > > *)dev->pdata->regs_phy; + > > > > > > + dev->pdata->phy_control(1); > > > > > > + > > > > > > + /*USB PHY0 Enable */ > > > > > > > > > > Wrong comment style. > > > > > > > > I'll fix this one. > > > > > > > > > > + printf("USB PHY0 Enable\n"); > > > > > > + > > > > > > + /* Enable PHY */ > > > > > > + writel(readl(usb_phy_ctrl) | USB_PHY_CTRL_EN0, > > > > > > usb_phy_ctrl); > > > > > > > > > > We have helpers for setting/clearing bits, right? > > > > > > > > Yes we do, Lukasz ... ? :) > > > > > > If you think about set_bit()/ clear_bit() functions > > > from ./arch/arm/include/asm/bitops.h, then I don't mind to replace > > > current code with them. > > > > > > Feel free to use them for next version of the patch. > > > > Pavel meant clrsetbits_le32() and friends . Personally, I would leave > > that to subsequent set :) > > This is up to you. I just would like to avoid changing too many things > at once. Definitelly. Did adding the missing include fix the issues with this patch ? Best regards, Marek Vasut
Hi Marek, > On Friday, November 07, 2014 at 11:08:44 AM, Lukasz Majewski wrote: > > Hi Marek, > > > > > On Friday, November 07, 2014 at 09:59:25 AM, Lukasz Majewski > > > wrote: > > > > Hi Marek, > > > > > > > > > On Tuesday, November 04, 2014 at 08:34:21 PM, Pavel Machek > > > > > wrote: > > > > > > On Tue 2014-11-04 06:07:32, Marek Vasut wrote: > > > > > > > Split the Samsung specific PHY control into a separate > > > > > > > file and compile this into the S3C OTG driver only if > > > > > > > used on a Samsung system. > > > > > > > > > > > > > > Signed-off-by: Marek Vasut <marex@denx.de> > > > > > > > Cc: Chin Liang See <clsee@altera.com> > > > > > > > Cc: Dinh Nguyen <dinguyen@opensource.altera.com> > > > > > > > Cc: Vince Bridgers <vbridger@altera.com> > > > > > > > > > > > > Acked-by: Pavel Machek <pavel@denx.de> > > > > > > > > > > > > I know you are just moving the code, but... > > > > > > > > > > > > > +void otg_phy_init(struct s3c_udc *dev) > > > > > > > +{ > > > > > > > + unsigned int usb_phy_ctrl = > > > > > > > dev->pdata->usb_phy_ctrl; > > > > > > > + struct s3c_usbotg_phy *phy = > > > > > > > + (struct s3c_usbotg_phy > > > > > > > *)dev->pdata->regs_phy; + > > > > > > > + dev->pdata->phy_control(1); > > > > > > > + > > > > > > > + /*USB PHY0 Enable */ > > > > > > > > > > > > Wrong comment style. > > > > > > > > > > I'll fix this one. > > > > > > > > > > > > + printf("USB PHY0 Enable\n"); > > > > > > > + > > > > > > > + /* Enable PHY */ > > > > > > > + writel(readl(usb_phy_ctrl) | USB_PHY_CTRL_EN0, > > > > > > > usb_phy_ctrl); > > > > > > > > > > > > We have helpers for setting/clearing bits, right? > > > > > > > > > > Yes we do, Lukasz ... ? :) > > > > > > > > If you think about set_bit()/ clear_bit() functions > > > > from ./arch/arm/include/asm/bitops.h, then I don't mind to > > > > replace current code with them. > > > > > > > > Feel free to use them for next version of the patch. > > > > > > Pavel meant clrsetbits_le32() and friends . Personally, I would > > > leave that to subsequent set :) > > > > This is up to you. I just would like to avoid changing too many > > things at once. > > Definitelly. Did adding the missing include fix the issues with this > patch ? > I've took the topic/s3c-otg from u-boot-usb repo. Reviewed-by: Lukasz Majewski <l.majewski@samsung.com> Tested-by: Lukasz Majewski <l.majewski@samsung.com> Test HW: Exynos4412 - Trats2. > Best regards, > Marek Vasut
On Friday, November 07, 2014 at 03:23:42 PM, Lukasz Majewski wrote: > Hi Marek, > > > On Friday, November 07, 2014 at 11:08:44 AM, Lukasz Majewski wrote: > > > Hi Marek, > > > > > > > On Friday, November 07, 2014 at 09:59:25 AM, Lukasz Majewski > > > > > > > > wrote: > > > > > Hi Marek, > > > > > > > > > > > On Tuesday, November 04, 2014 at 08:34:21 PM, Pavel Machek > > > > > > > > > > > > wrote: > > > > > > > On Tue 2014-11-04 06:07:32, Marek Vasut wrote: > > > > > > > > Split the Samsung specific PHY control into a separate > > > > > > > > file and compile this into the S3C OTG driver only if > > > > > > > > used on a Samsung system. > > > > > > > > > > > > > > > > Signed-off-by: Marek Vasut <marex@denx.de> > > > > > > > > Cc: Chin Liang See <clsee@altera.com> > > > > > > > > Cc: Dinh Nguyen <dinguyen@opensource.altera.com> > > > > > > > > Cc: Vince Bridgers <vbridger@altera.com> > > > > > > > > > > > > > > Acked-by: Pavel Machek <pavel@denx.de> > > > > > > > > > > > > > > I know you are just moving the code, but... > > > > > > > > > > > > > > > +void otg_phy_init(struct s3c_udc *dev) > > > > > > > > +{ > > > > > > > > + unsigned int usb_phy_ctrl = > > > > > > > > dev->pdata->usb_phy_ctrl; > > > > > > > > + struct s3c_usbotg_phy *phy = > > > > > > > > + (struct s3c_usbotg_phy > > > > > > > > *)dev->pdata->regs_phy; + > > > > > > > > + dev->pdata->phy_control(1); > > > > > > > > + > > > > > > > > + /*USB PHY0 Enable */ > > > > > > > > > > > > > > Wrong comment style. > > > > > > > > > > > > I'll fix this one. > > > > > > > > > > > > > > + printf("USB PHY0 Enable\n"); > > > > > > > > + > > > > > > > > + /* Enable PHY */ > > > > > > > > + writel(readl(usb_phy_ctrl) | USB_PHY_CTRL_EN0, > > > > > > > > usb_phy_ctrl); > > > > > > > > > > > > > > We have helpers for setting/clearing bits, right? > > > > > > > > > > > > Yes we do, Lukasz ... ? :) > > > > > > > > > > If you think about set_bit()/ clear_bit() functions > > > > > from ./arch/arm/include/asm/bitops.h, then I don't mind to > > > > > replace current code with them. > > > > > > > > > > Feel free to use them for next version of the patch. > > > > > > > > Pavel meant clrsetbits_le32() and friends . Personally, I would > > > > leave that to subsequent set :) > > > > > > This is up to you. I just would like to avoid changing too many > > > things at once. > > > > Definitelly. Did adding the missing include fix the issues with this > > patch ? > > I've took the topic/s3c-otg from u-boot-usb repo. > > Reviewed-by: Lukasz Majewski <l.majewski@samsung.com> > Tested-by: Lukasz Majewski <l.majewski@samsung.com> Thanks, I'll queue that up for master then.
diff --git a/drivers/usb/gadget/Makefile b/drivers/usb/gadget/Makefile index 2efd5a4..70bb550 100644 --- a/drivers/usb/gadget/Makefile +++ b/drivers/usb/gadget/Makefile @@ -12,6 +12,7 @@ obj-$(CONFIG_USB_ETHER) += epautoconf.o config.o usbstring.o ifdef CONFIG_USB_GADGET obj-$(CONFIG_USB_GADGET_ATMEL_USBA) += atmel_usba_udc.o obj-$(CONFIG_USB_GADGET_S3C_UDC_OTG) += s3c_udc_otg.o +obj-$(CONFIG_USB_GADGET_S3C_UDC_OTG_PHY) += s3c_udc_otg_phy.o obj-$(CONFIG_USB_GADGET_FOTG210) += fotg210.o obj-$(CONFIG_CI_UDC) += ci_udc.o obj-$(CONFIG_THOR_FUNCTION) += f_thor.o diff --git a/drivers/usb/gadget/s3c_udc_otg.c b/drivers/usb/gadget/s3c_udc_otg.c index b808ddaf..b910053 100644 --- a/drivers/usb/gadget/s3c_udc_otg.c +++ b/drivers/usb/gadget/s3c_udc_otg.c @@ -151,68 +151,8 @@ bool dfu_usb_get_reset(void) return !!(readl(®->gintsts) & INT_RESET); } -void otg_phy_init(struct s3c_udc *dev) -{ - unsigned int usb_phy_ctrl = dev->pdata->usb_phy_ctrl; - struct s3c_usbotg_phy *phy = - (struct s3c_usbotg_phy *)dev->pdata->regs_phy; - - dev->pdata->phy_control(1); - - /*USB PHY0 Enable */ - printf("USB PHY0 Enable\n"); - - /* Enable PHY */ - writel(readl(usb_phy_ctrl) | USB_PHY_CTRL_EN0, usb_phy_ctrl); - - if (dev->pdata->usb_flags == PHY0_SLEEP) /* C210 Universal */ - writel((readl(&phy->phypwr) - &~(PHY_0_SLEEP | OTG_DISABLE_0 | ANALOG_PWRDOWN) - &~FORCE_SUSPEND_0), &phy->phypwr); - else /* C110 GONI */ - writel((readl(&phy->phypwr) &~(OTG_DISABLE_0 | ANALOG_PWRDOWN) - &~FORCE_SUSPEND_0), &phy->phypwr); - - if (s5p_cpu_id == 0x4412) - writel((readl(&phy->phyclk) & ~(EXYNOS4X12_ID_PULLUP0 | - EXYNOS4X12_COMMON_ON_N0)) | EXYNOS4X12_CLK_SEL_24MHZ, - &phy->phyclk); /* PLL 24Mhz */ - else - writel((readl(&phy->phyclk) & ~(ID_PULLUP0 | COMMON_ON_N0)) | - CLK_SEL_24MHZ, &phy->phyclk); /* PLL 24Mhz */ - - writel((readl(&phy->rstcon) &~(LINK_SW_RST | PHYLNK_SW_RST)) - | PHY_SW_RST0, &phy->rstcon); - udelay(10); - writel(readl(&phy->rstcon) - &~(PHY_SW_RST0 | LINK_SW_RST | PHYLNK_SW_RST), &phy->rstcon); - udelay(10); -} - -void otg_phy_off(struct s3c_udc *dev) -{ - unsigned int usb_phy_ctrl = dev->pdata->usb_phy_ctrl; - struct s3c_usbotg_phy *phy = - (struct s3c_usbotg_phy *)dev->pdata->regs_phy; - - /* reset controller just in case */ - writel(PHY_SW_RST0, &phy->rstcon); - udelay(20); - writel(readl(&phy->phypwr) &~PHY_SW_RST0, &phy->rstcon); - udelay(20); - - writel(readl(&phy->phypwr) | OTG_DISABLE_0 | ANALOG_PWRDOWN - | FORCE_SUSPEND_0, &phy->phypwr); - - writel(readl(usb_phy_ctrl) &~USB_PHY_CTRL_EN0, usb_phy_ctrl); - - writel((readl(&phy->phyclk) & ~(ID_PULLUP0 | COMMON_ON_N0)), - &phy->phyclk); - - udelay(10000); - - dev->pdata->phy_control(0); -} +__weak void otg_phy_init(struct s3c_udc *dev) {} +__weak void otg_phy_off(struct s3c_udc *dev) {} /***********************************************************/ diff --git a/drivers/usb/gadget/s3c_udc_otg_phy.c b/drivers/usb/gadget/s3c_udc_otg_phy.c new file mode 100644 index 0000000..055fe86 --- /dev/null +++ b/drivers/usb/gadget/s3c_udc_otg_phy.c @@ -0,0 +1,99 @@ +/* + * drivers/usb/gadget/s3c_udc_otg.c + * Samsung S3C on-chip full/high speed USB OTG 2.0 device controllers + * + * Copyright (C) 2008 for Samsung Electronics + * + * BSP Support for Samsung's UDC driver + * available at: + * git://git.kernel.org/pub/scm/linux/kernel/git/kki_ap/linux-2.6-samsung.git + * + * State machine bugfixes: + * Marek Szyprowski <m.szyprowski@samsung.com> + * + * Ported to u-boot: + * Marek Szyprowski <m.szyprowski@samsung.com> + * Lukasz Majewski <l.majewski@samsumg.com> + * + * SPDX-License-Identifier: GPL-2.0+ + */ + +#include <common.h> +#include <asm/errno.h> +#include <linux/list.h> +#include <malloc.h> + +#include <linux/usb/ch9.h> +#include <linux/usb/gadget.h> + +#include <asm/byteorder.h> +#include <asm/unaligned.h> +#include <asm/io.h> + +#include <asm/mach-types.h> + +#include "regs-otg.h" +#include <usb/lin_gadget_compat.h> + +void otg_phy_init(struct s3c_udc *dev) +{ + unsigned int usb_phy_ctrl = dev->pdata->usb_phy_ctrl; + struct s3c_usbotg_phy *phy = + (struct s3c_usbotg_phy *)dev->pdata->regs_phy; + + dev->pdata->phy_control(1); + + /*USB PHY0 Enable */ + printf("USB PHY0 Enable\n"); + + /* Enable PHY */ + writel(readl(usb_phy_ctrl) | USB_PHY_CTRL_EN0, usb_phy_ctrl); + + if (dev->pdata->usb_flags == PHY0_SLEEP) /* C210 Universal */ + writel((readl(&phy->phypwr) + &~(PHY_0_SLEEP | OTG_DISABLE_0 | ANALOG_PWRDOWN) + &~FORCE_SUSPEND_0), &phy->phypwr); + else /* C110 GONI */ + writel((readl(&phy->phypwr) &~(OTG_DISABLE_0 | ANALOG_PWRDOWN) + &~FORCE_SUSPEND_0), &phy->phypwr); + + if (s5p_cpu_id == 0x4412) + writel((readl(&phy->phyclk) & ~(EXYNOS4X12_ID_PULLUP0 | + EXYNOS4X12_COMMON_ON_N0)) | EXYNOS4X12_CLK_SEL_24MHZ, + &phy->phyclk); /* PLL 24Mhz */ + else + writel((readl(&phy->phyclk) & ~(ID_PULLUP0 | COMMON_ON_N0)) | + CLK_SEL_24MHZ, &phy->phyclk); /* PLL 24Mhz */ + + writel((readl(&phy->rstcon) &~(LINK_SW_RST | PHYLNK_SW_RST)) + | PHY_SW_RST0, &phy->rstcon); + udelay(10); + writel(readl(&phy->rstcon) + &~(PHY_SW_RST0 | LINK_SW_RST | PHYLNK_SW_RST), &phy->rstcon); + udelay(10); +} + +void otg_phy_off(struct s3c_udc *dev) +{ + unsigned int usb_phy_ctrl = dev->pdata->usb_phy_ctrl; + struct s3c_usbotg_phy *phy = + (struct s3c_usbotg_phy *)dev->pdata->regs_phy; + + /* reset controller just in case */ + writel(PHY_SW_RST0, &phy->rstcon); + udelay(20); + writel(readl(&phy->phypwr) &~PHY_SW_RST0, &phy->rstcon); + udelay(20); + + writel(readl(&phy->phypwr) | OTG_DISABLE_0 | ANALOG_PWRDOWN + | FORCE_SUSPEND_0, &phy->phypwr); + + writel(readl(usb_phy_ctrl) &~USB_PHY_CTRL_EN0, usb_phy_ctrl); + + writel((readl(&phy->phyclk) & ~(ID_PULLUP0 | COMMON_ON_N0)), + &phy->phyclk); + + udelay(10000); + + dev->pdata->phy_control(0); +} diff --git a/include/configs/exynos4-common.h b/include/configs/exynos4-common.h index 89ba14e..41631c7 100644 --- a/include/configs/exynos4-common.h +++ b/include/configs/exynos4-common.h @@ -59,6 +59,7 @@ #define CONFIG_USB_GADGET #define CONFIG_USB_GADGET_S3C_UDC_OTG +#define CONFIG_USB_GADGET_S3C_UDC_OTG_PHY #define CONFIG_USB_GADGET_DUALSPEED #define CONFIG_USB_GADGET_VBUS_DRAW 2 diff --git a/include/configs/s5p_goni.h b/include/configs/s5p_goni.h index 3633a35..dfa2e07 100644 --- a/include/configs/s5p_goni.h +++ b/include/configs/s5p_goni.h @@ -281,6 +281,7 @@ #define CONFIG_SYS_MAX_I2C_BUS 7 #define CONFIG_USB_GADGET #define CONFIG_USB_GADGET_S3C_UDC_OTG +#define CONFIG_USB_GADGET_S3C_UDC_OTG_PHY #define CONFIG_USB_GADGET_DUALSPEED #define CONFIG_USB_GADGET_VBUS_DRAW 2 #define CONFIG_CMD_USB_MASS_STORAGE diff --git a/include/configs/s5pc210_universal.h b/include/configs/s5pc210_universal.h index 4b30d148..e7bace4 100644 --- a/include/configs/s5pc210_universal.h +++ b/include/configs/s5pc210_universal.h @@ -181,6 +181,7 @@ #define CONFIG_USB_GADGET #define CONFIG_USB_GADGET_S3C_UDC_OTG +#define CONFIG_USB_GADGET_S3C_UDC_OTG_PHY #define CONFIG_USB_GADGET_DUALSPEED /* diff --git a/include/configs/smdkv310.h b/include/configs/smdkv310.h index a2469eb..655025c 100644 --- a/include/configs/smdkv310.h +++ b/include/configs/smdkv310.h @@ -14,6 +14,7 @@ #undef CONFIG_BOARD_COMMON #undef CONFIG_USB_GADGET #undef CONFIG_USB_GADGET_S3C_UDC_OTG +#undef CONFIG_USB_GADGET_S3C_UDC_OTG_PHY #undef CONFIG_CMD_USB_MASS_STORAGE #undef CONFIG_REVISION_TAG #undef CONFIG_CMD_THOR_DOWNLOAD
Split the Samsung specific PHY control into a separate file and compile this into the S3C OTG driver only if used on a Samsung system. Signed-off-by: Marek Vasut <marex@denx.de> Cc: Chin Liang See <clsee@altera.com> Cc: Dinh Nguyen <dinguyen@opensource.altera.com> Cc: Vince Bridgers <vbridger@altera.com> Cc: Pavel Machek <pavel@denx.de> Cc: Stefan Roese <sr@denx.de> Cc: Lukasz Majewski <l.majewski@samsung.com> --- drivers/usb/gadget/Makefile | 1 + drivers/usb/gadget/s3c_udc_otg.c | 64 +---------------------- drivers/usb/gadget/s3c_udc_otg_phy.c | 99 ++++++++++++++++++++++++++++++++++++ include/configs/exynos4-common.h | 1 + include/configs/s5p_goni.h | 1 + include/configs/s5pc210_universal.h | 1 + include/configs/smdkv310.h | 1 + 7 files changed, 106 insertions(+), 62 deletions(-) create mode 100644 drivers/usb/gadget/s3c_udc_otg_phy.c