| Message ID | 1396967803-28868-2-git-send-email-gautam.vivek@samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Vivek, Please see my comments inline. On 08.04.2014 16:36, Vivek Gautam wrote: > Add a new driver for the USB 3.0 PHY on Exynos5 series of SoCs. > The new driver uses the generic PHY framework and will interact > with DWC3 controller present on Exynos5 series of SoCs. > Thereby, removing old phy-samsung-usb3 driver and related code > used untill now which was based on usb/phy framework. > > Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com> > --- > .../devicetree/bindings/phy/samsung-phy.txt | 42 ++ > drivers/phy/Kconfig | 11 + > drivers/phy/Makefile | 1 + > drivers/phy/phy-exynos5-usbdrd.c | 668 ++++++++++++++++++++ > 4 files changed, 722 insertions(+) > create mode 100644 drivers/phy/phy-exynos5-usbdrd.c [snip] > + Additional clock required for Exynos5420: > + - usb30_sclk_100m: Additional special clock used for PHY operation > + depicted as 'sclk_usbphy30' in CMU of Exynos5420. Are you sure this isn't simply a gate for the ref clock, as it can be found on another SoC that is not upstream yet? I don't have documentation for Exynos 5420 so I can't tell, but I'd like to ask you to recheck this. > +- samsung,syscon-phandle: phandle for syscon interface, which is used to > + control pmu registers for power isolation. > +- samsung,pmu-offset: phy power control register offset to pmu-system-controller > + base. > +- #phy-cells : from the generic PHY bindings, must be 1; > + > +For "samsung,exynos5250-usbdrd-phy" and "samsung,exynos5420-usbdrd-phy" > +compatible PHYs, the second cell in the PHY specifier identifies the > +PHY id, which is interpreted as follows: > + 0 - UTMI+ type phy, > + 1 - PIPE3 type phy, > + > +Example: > + usb3_phy: usbphy@12100000 { > + compatible = "samsung,exynos5250-usbdrd-phy"; > + reg = <0x12100000 0x100>; > + clocks = <&clock 286>, <&clock 1>; > + clock-names = "phy", "usb3phy_refclk"; Binding description above doesn't mention "usb3phy_refclk" entry. > + samsung,syscon-phandle = <&pmu_syscon>; > + samsung,pmu-offset = <0x704>; > + #phy-cells = <1>; > + }; [snip] > diff --git a/drivers/phy/phy-exynos5-usbdrd.c b/drivers/phy/phy-exynos5-usbdrd.c > new file mode 100644 > index 0000000..ff54a7c > --- /dev/null > +++ b/drivers/phy/phy-exynos5-usbdrd.c [snip] > +static int exynos5_usbdrd_phy_probe(struct platform_device *pdev) > +{ > + struct device *dev = &pdev->dev; > + struct device_node *node = dev->of_node; > + struct exynos5_usbdrd_phy *phy_drd; > + struct phy_provider *phy_provider; > + struct resource *res; > + const struct of_device_id *match; > + const struct exynos5_usbdrd_phy_drvdata *drv_data; > + struct regmap *reg_pmu; > + u32 pmu_offset; > + int i; > + > + /* > + * Exynos systems are completely DT enabled, > + * so lets not have any platform data support for this driver. > + */ > + if (!node) { > + dev_err(dev, "no device node found\n"); This error message is not very meaningful. I'd rather use something like "This driver can be only instantiated using Device Tree". > + return -ENODEV; > + } > + > + match = of_match_node(exynos5_usbdrd_phy_of_match, pdev->dev.of_node); > + if (!match) { > + dev_err(dev, "of_match_node() failed\n"); > + return -EINVAL; > + } > + drv_data = match->data; > + > + phy_drd = devm_kzalloc(dev, sizeof(*phy_drd), GFP_KERNEL); > + if (!phy_drd) > + return -ENOMEM; > + > + dev_set_drvdata(dev, phy_drd); > + phy_drd->dev = dev; > + > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > + phy_drd->reg_phy = devm_ioremap_resource(dev, res); > + if (IS_ERR(phy_drd->reg_phy)) { > + dev_err(dev, "Failed to map register memory (phy)\n"); devm_ioremap_resource() already prints an error message. Best regards, Tomasz
Hi, On Wed, Apr 9, 2014 at 4:36 PM, Tomasz Figa <t.figa@samsung.com> wrote: > Hi Vivek, > > Please see my comments inline. > > > On 08.04.2014 16:36, Vivek Gautam wrote: >> >> Add a new driver for the USB 3.0 PHY on Exynos5 series of SoCs. >> The new driver uses the generic PHY framework and will interact >> with DWC3 controller present on Exynos5 series of SoCs. >> Thereby, removing old phy-samsung-usb3 driver and related code >> used untill now which was based on usb/phy framework. >> >> Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com> >> --- >> .../devicetree/bindings/phy/samsung-phy.txt | 42 ++ >> drivers/phy/Kconfig | 11 + >> drivers/phy/Makefile | 1 + >> drivers/phy/phy-exynos5-usbdrd.c | 668 >> ++++++++++++++++++++ >> 4 files changed, 722 insertions(+) >> create mode 100644 drivers/phy/phy-exynos5-usbdrd.c > > > [snip] > > >> + Additional clock required for Exynos5420: >> + - usb30_sclk_100m: Additional special clock used for PHY operation >> + depicted as 'sclk_usbphy30' in CMU of >> Exynos5420. > > > Are you sure this isn't simply a gate for the ref clock, as it can be found > on another SoC that is not upstream yet? I don't have documentation for > Exynos 5420 so I can't tell, but I'd like to ask you to recheck this. From what i can see in the manual : sclk_usbphy30 is derived from OSCCLK. It is coming from a MUX (default input line to this is OSCCLK) and then through a DIV there's this gate. {OSCCLK + other sources} --->[MUX] ---> [DIV] --> [GATE for sclk_usbphy30] the {rate of sclk_usbphy30} == OSCCLK However the 'ref' clock that we have been using is the actual oscillator clock. And on SoC Exynos5250, we don't have any such gate (sclk_usbphy30). So should this mean that ref clock and sclk_usbphy30 are still be controlled by two different gates ? > > >> +- samsung,syscon-phandle: phandle for syscon interface, which is used to >> + control pmu registers for power isolation. >> +- samsung,pmu-offset: phy power control register offset to >> pmu-system-controller >> + base. >> +- #phy-cells : from the generic PHY bindings, must be 1; >> + >> +For "samsung,exynos5250-usbdrd-phy" and "samsung,exynos5420-usbdrd-phy" >> +compatible PHYs, the second cell in the PHY specifier identifies the >> +PHY id, which is interpreted as follows: >> + 0 - UTMI+ type phy, >> + 1 - PIPE3 type phy, >> + >> +Example: >> + usb3_phy: usbphy@12100000 { >> + compatible = "samsung,exynos5250-usbdrd-phy"; >> + reg = <0x12100000 0x100>; >> + clocks = <&clock 286>, <&clock 1>; >> + clock-names = "phy", "usb3phy_refclk"; > > > Binding description above doesn't mention "usb3phy_refclk" entry. my bad !! will correct this. > > >> + samsung,syscon-phandle = <&pmu_syscon>; >> + samsung,pmu-offset = <0x704>; >> + #phy-cells = <1>; >> + }; > > > [snip] > > >> diff --git a/drivers/phy/phy-exynos5-usbdrd.c >> b/drivers/phy/phy-exynos5-usbdrd.c >> new file mode 100644 >> index 0000000..ff54a7c >> --- /dev/null >> +++ b/drivers/phy/phy-exynos5-usbdrd.c > > > [snip] > > >> +static int exynos5_usbdrd_phy_probe(struct platform_device *pdev) >> +{ >> + struct device *dev = &pdev->dev; >> + struct device_node *node = dev->of_node; >> + struct exynos5_usbdrd_phy *phy_drd; >> + struct phy_provider *phy_provider; >> + struct resource *res; >> + const struct of_device_id *match; >> + const struct exynos5_usbdrd_phy_drvdata *drv_data; >> + struct regmap *reg_pmu; >> + u32 pmu_offset; >> + int i; >> + >> + /* >> + * Exynos systems are completely DT enabled, >> + * so lets not have any platform data support for this driver. >> + */ >> + if (!node) { >> + dev_err(dev, "no device node found\n"); > > > This error message is not very meaningful. I'd rather use something like > "This driver can be only instantiated using Device Tree". Sure, will amend this. > > >> + return -ENODEV; >> + } >> + >> + match = of_match_node(exynos5_usbdrd_phy_of_match, >> pdev->dev.of_node); >> + if (!match) { >> + dev_err(dev, "of_match_node() failed\n"); >> + return -EINVAL; >> + } >> + drv_data = match->data; >> + >> + phy_drd = devm_kzalloc(dev, sizeof(*phy_drd), GFP_KERNEL); >> + if (!phy_drd) >> + return -ENOMEM; >> + >> + dev_set_drvdata(dev, phy_drd); >> + phy_drd->dev = dev; >> + >> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); >> + phy_drd->reg_phy = devm_ioremap_resource(dev, res); >> + if (IS_ERR(phy_drd->reg_phy)) { >> + dev_err(dev, "Failed to map register memory (phy)\n"); > > > devm_ioremap_resource() already prints an error message. Ok, will remove this message. > > Best regards, > Tomasz > -- > To unsubscribe from this list: send the line "unsubscribe linux-usb" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html
On 09.04.2014 13:49, Vivek Gautam wrote: > Hi, > > > On Wed, Apr 9, 2014 at 4:36 PM, Tomasz Figa <t.figa@samsung.com> wrote: >> Hi Vivek, >> >> Please see my comments inline. >> >> >> On 08.04.2014 16:36, Vivek Gautam wrote: >>> >>> Add a new driver for the USB 3.0 PHY on Exynos5 series of SoCs. >>> The new driver uses the generic PHY framework and will interact >>> with DWC3 controller present on Exynos5 series of SoCs. >>> Thereby, removing old phy-samsung-usb3 driver and related code >>> used untill now which was based on usb/phy framework. >>> >>> Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com> >>> --- >>> .../devicetree/bindings/phy/samsung-phy.txt | 42 ++ >>> drivers/phy/Kconfig | 11 + >>> drivers/phy/Makefile | 1 + >>> drivers/phy/phy-exynos5-usbdrd.c | 668 >>> ++++++++++++++++++++ >>> 4 files changed, 722 insertions(+) >>> create mode 100644 drivers/phy/phy-exynos5-usbdrd.c >> >> >> [snip] >> >> >>> + Additional clock required for Exynos5420: >>> + - usb30_sclk_100m: Additional special clock used for PHY operation >>> + depicted as 'sclk_usbphy30' in CMU of >>> Exynos5420. >> >> >> Are you sure this isn't simply a gate for the ref clock, as it can be found >> on another SoC that is not upstream yet? I don't have documentation for >> Exynos 5420 so I can't tell, but I'd like to ask you to recheck this. > >>From what i can see in the manual : > sclk_usbphy30 is derived from OSCCLK. > It is coming from a MUX (default input line to this is OSCCLK) and > then through a DIV > there's this gate. > > {OSCCLK + other sources} --->[MUX] ---> [DIV] --> [GATE for > sclk_usbphy30] > > the {rate of sclk_usbphy30} == OSCCLK > > However the 'ref' clock that we have been using is the actual oscillator clock. > And on SoC Exynos5250, we don't have any such gate (sclk_usbphy30). > So should this mean that ref clock and sclk_usbphy30 are still be controlled by > two different gates ? > Is there maybe a diagram of PHY input clocks in the datasheet, like for USB 2.0 PHY in Exynos4210/4412/5250 datasheets in the chapter about USB2.0 Device? Something like: ____________________________________ | | | ___________| XusbXTI | Phy_fsel[2:0] | _______ | _______[X]_______| | __________|_|___|\__|_| | | _v___ | _____ ^ | |/ | | _____ | | | | | | | | ___ | | ___ | | | | | | | | | |_|_| |___| | | X 0 |____|_| PLL |__|_|_|CLK|_|_| _____ | | | | | | |DIV|_|_| |_______[X] | |_____| 12 |_____|480 | |___| | | | MHz MHz |Digital| | XusbXTO | USB PHY |_______| | |____________________________________| Best regards, Tomasz
On Wed, Apr 9, 2014 at 7:03 PM, Tomasz Figa <t.figa@samsung.com> wrote: > On 09.04.2014 13:49, Vivek Gautam wrote: >> >> Hi, >> >> >> On Wed, Apr 9, 2014 at 4:36 PM, Tomasz Figa <t.figa@samsung.com> wrote: >>> >>> Hi Vivek, >>> >>> Please see my comments inline. >>> >>> >>> On 08.04.2014 16:36, Vivek Gautam wrote: >>>> >>>> >>>> Add a new driver for the USB 3.0 PHY on Exynos5 series of SoCs. >>>> The new driver uses the generic PHY framework and will interact >>>> with DWC3 controller present on Exynos5 series of SoCs. >>>> Thereby, removing old phy-samsung-usb3 driver and related code >>>> used untill now which was based on usb/phy framework. >>>> >>>> Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com> >>>> --- >>>> .../devicetree/bindings/phy/samsung-phy.txt | 42 ++ >>>> drivers/phy/Kconfig | 11 + >>>> drivers/phy/Makefile | 1 + >>>> drivers/phy/phy-exynos5-usbdrd.c | 668 >>>> ++++++++++++++++++++ >>>> 4 files changed, 722 insertions(+) >>>> create mode 100644 drivers/phy/phy-exynos5-usbdrd.c >>> >>> >>> >>> [snip] >>> >>> >>>> + Additional clock required for Exynos5420: >>>> + - usb30_sclk_100m: Additional special clock used for PHY >>>> operation >>>> + depicted as 'sclk_usbphy30' in CMU of >>>> Exynos5420. >>> >>> >>> >>> Are you sure this isn't simply a gate for the ref clock, as it can be >>> found >>> on another SoC that is not upstream yet? I don't have documentation for >>> Exynos 5420 so I can't tell, but I'd like to ask you to recheck this. >> >> >>> From what i can see in the manual : >> >> sclk_usbphy30 is derived from OSCCLK. >> It is coming from a MUX (default input line to this is OSCCLK) and >> then through a DIV >> there's this gate. >> >> {OSCCLK + other sources} --->[MUX] ---> [DIV] --> [GATE for >> sclk_usbphy30] >> >> the {rate of sclk_usbphy30} == OSCCLK >> >> However the 'ref' clock that we have been using is the actual oscillator >> clock. >> And on SoC Exynos5250, we don't have any such gate (sclk_usbphy30). >> So should this mean that ref clock and sclk_usbphy30 are still be >> controlled by >> two different gates ? >> > > Is there maybe a diagram of PHY input clocks in the datasheet, like for USB > 2.0 PHY in Exynos4210/4412/5250 datasheets in the chapter about USB2.0 > Device? Something like: > > ____________________________________ > | | > | ___________| > XusbXTI | Phy_fsel[2:0] | _______ | > _______[X]_______| | __________|_|___|\__|_| > | | _v___ | _____ ^ | |/ | | > _____ | | | | | | | | ___ | | > ___ | | | | | | | | | |_|_| > |___| | | X 0 |____|_| PLL |__|_|_|CLK|_|_| > _____ | | | | | | |DIV|_|_| > |_______[X] | |_____| 12 |_____|480 | |___| | | > | MHz MHz |Digital| | > XusbXTO | USB PHY |_______| | > |____________________________________| > > Below is the block diagram given for DRD controller.
Hi, On Wednesday 09 April 2014 04:36 PM, Tomasz Figa wrote: > Hi Vivek, > > Please see my comments inline. > > On 08.04.2014 16:36, Vivek Gautam wrote: >> Add a new driver for the USB 3.0 PHY on Exynos5 series of SoCs. >> The new driver uses the generic PHY framework and will interact >> with DWC3 controller present on Exynos5 series of SoCs. >> Thereby, removing old phy-samsung-usb3 driver and related code >> used untill now which was based on usb/phy framework. >> >> Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com> >> --- >> .../devicetree/bindings/phy/samsung-phy.txt | 42 ++ >> drivers/phy/Kconfig | 11 + >> drivers/phy/Makefile | 1 + >> drivers/phy/phy-exynos5-usbdrd.c | 668 ++++++++++++++++++++ >> 4 files changed, 722 insertions(+) >> create mode 100644 drivers/phy/phy-exynos5-usbdrd.c > > [snip] > >> + Additional clock required for Exynos5420: >> + - usb30_sclk_100m: Additional special clock used for PHY operation >> + depicted as 'sclk_usbphy30' in CMU of Exynos5420. > > Are you sure this isn't simply a gate for the ref clock, as it can be found on > another SoC that is not upstream yet? I don't have documentation for Exynos > 5420 so I can't tell, but I'd like to ask you to recheck this. > >> +- samsung,syscon-phandle: phandle for syscon interface, which is used to >> + control pmu registers for power isolation. >> +- samsung,pmu-offset: phy power control register offset to >> pmu-system-controller >> + base. >> +- #phy-cells : from the generic PHY bindings, must be 1; >> + >> +For "samsung,exynos5250-usbdrd-phy" and "samsung,exynos5420-usbdrd-phy" >> +compatible PHYs, the second cell in the PHY specifier identifies the >> +PHY id, which is interpreted as follows: >> + 0 - UTMI+ type phy, >> + 1 - PIPE3 type phy, >> + >> +Example: >> + usb3_phy: usbphy@12100000 { >> + compatible = "samsung,exynos5250-usbdrd-phy"; >> + reg = <0x12100000 0x100>; >> + clocks = <&clock 286>, <&clock 1>; >> + clock-names = "phy", "usb3phy_refclk"; > > Binding description above doesn't mention "usb3phy_refclk" entry. > >> + samsung,syscon-phandle = <&pmu_syscon>; >> + samsung,pmu-offset = <0x704>; >> + #phy-cells = <1>; >> + }; > > [snip] > >> diff --git a/drivers/phy/phy-exynos5-usbdrd.c b/drivers/phy/phy-exynos5-usbdrd.c >> new file mode 100644 >> index 0000000..ff54a7c >> --- /dev/null >> +++ b/drivers/phy/phy-exynos5-usbdrd.c > > [snip] > >> +static int exynos5_usbdrd_phy_probe(struct platform_device *pdev) >> +{ >> + struct device *dev = &pdev->dev; >> + struct device_node *node = dev->of_node; >> + struct exynos5_usbdrd_phy *phy_drd; >> + struct phy_provider *phy_provider; >> + struct resource *res; >> + const struct of_device_id *match; >> + const struct exynos5_usbdrd_phy_drvdata *drv_data; >> + struct regmap *reg_pmu; >> + u32 pmu_offset; >> + int i; >> + >> + /* >> + * Exynos systems are completely DT enabled, >> + * so lets not have any platform data support for this driver. >> + */ >> + if (!node) { >> + dev_err(dev, "no device node found\n"); > > This error message is not very meaningful. I'd rather use something like "This > driver can be only instantiated using Device Tree". how about just adding depend_on OF in Kconfig? Thanks Kishon
Hi Kishon, On Mon, Apr 14, 2014 at 5:24 PM, Kishon Vijay Abraham I <kishon@ti.com> wrote: > Hi, > > On Wednesday 09 April 2014 04:36 PM, Tomasz Figa wrote: >> Hi Vivek, >> >> Please see my comments inline. >> >> On 08.04.2014 16:36, Vivek Gautam wrote: >>> Add a new driver for the USB 3.0 PHY on Exynos5 series of SoCs. >>> The new driver uses the generic PHY framework and will interact >>> with DWC3 controller present on Exynos5 series of SoCs. >>> Thereby, removing old phy-samsung-usb3 driver and related code >>> used untill now which was based on usb/phy framework. >>> >>> Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com> >>> --- >>> .../devicetree/bindings/phy/samsung-phy.txt | 42 ++ >>> drivers/phy/Kconfig | 11 + >>> drivers/phy/Makefile | 1 + >>> drivers/phy/phy-exynos5-usbdrd.c | 668 ++++++++++++++++++++ >>> 4 files changed, 722 insertions(+) >>> create mode 100644 drivers/phy/phy-exynos5-usbdrd.c >> >> [snip] >> >>> + Additional clock required for Exynos5420: >>> + - usb30_sclk_100m: Additional special clock used for PHY operation >>> + depicted as 'sclk_usbphy30' in CMU of Exynos5420. >> >> Are you sure this isn't simply a gate for the ref clock, as it can be found on >> another SoC that is not upstream yet? I don't have documentation for Exynos >> 5420 so I can't tell, but I'd like to ask you to recheck this. >> >>> +- samsung,syscon-phandle: phandle for syscon interface, which is used to >>> + control pmu registers for power isolation. >>> +- samsung,pmu-offset: phy power control register offset to >>> pmu-system-controller >>> + base. >>> +- #phy-cells : from the generic PHY bindings, must be 1; >>> + >>> +For "samsung,exynos5250-usbdrd-phy" and "samsung,exynos5420-usbdrd-phy" >>> +compatible PHYs, the second cell in the PHY specifier identifies the >>> +PHY id, which is interpreted as follows: >>> + 0 - UTMI+ type phy, >>> + 1 - PIPE3 type phy, >>> + >>> +Example: >>> + usb3_phy: usbphy@12100000 { >>> + compatible = "samsung,exynos5250-usbdrd-phy"; >>> + reg = <0x12100000 0x100>; >>> + clocks = <&clock 286>, <&clock 1>; >>> + clock-names = "phy", "usb3phy_refclk"; >> >> Binding description above doesn't mention "usb3phy_refclk" entry. >> >>> + samsung,syscon-phandle = <&pmu_syscon>; >>> + samsung,pmu-offset = <0x704>; >>> + #phy-cells = <1>; >>> + }; >> >> [snip] >> >>> diff --git a/drivers/phy/phy-exynos5-usbdrd.c b/drivers/phy/phy-exynos5-usbdrd.c >>> new file mode 100644 >>> index 0000000..ff54a7c >>> --- /dev/null >>> +++ b/drivers/phy/phy-exynos5-usbdrd.c >> >> [snip] >> >>> +static int exynos5_usbdrd_phy_probe(struct platform_device *pdev) >>> +{ >>> + struct device *dev = &pdev->dev; >>> + struct device_node *node = dev->of_node; >>> + struct exynos5_usbdrd_phy *phy_drd; >>> + struct phy_provider *phy_provider; >>> + struct resource *res; >>> + const struct of_device_id *match; >>> + const struct exynos5_usbdrd_phy_drvdata *drv_data; >>> + struct regmap *reg_pmu; >>> + u32 pmu_offset; >>> + int i; >>> + >>> + /* >>> + * Exynos systems are completely DT enabled, >>> + * so lets not have any platform data support for this driver. >>> + */ >>> + if (!node) { >>> + dev_err(dev, "no device node found\n"); >> >> This error message is not very meaningful. I'd rather use something like "This >> driver can be only instantiated using Device Tree". > > how about just adding depend_on OF in Kconfig? Already added a depend on 'OF'. Copying below the part of Kconfig in this patch. config PHY_EXYNOS5_USBDRD tristate "Exynos5 SoC series USB DRD PHY driver" depends on ARCH_EXYNOS5 && OF depends on HAS_IOMEM
Hi, On Tuesday 08 April 2014 08:06 PM, Vivek Gautam wrote: > Add a new driver for the USB 3.0 PHY on Exynos5 series of SoCs. > The new driver uses the generic PHY framework and will interact > with DWC3 controller present on Exynos5 series of SoCs. > Thereby, removing old phy-samsung-usb3 driver and related code > used untill now which was based on usb/phy framework. > > Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com> > --- > .../devicetree/bindings/phy/samsung-phy.txt | 42 ++ > drivers/phy/Kconfig | 11 + > drivers/phy/Makefile | 1 + > drivers/phy/phy-exynos5-usbdrd.c | 668 ++++++++++++++++++++ > 4 files changed, 722 insertions(+) > create mode 100644 drivers/phy/phy-exynos5-usbdrd.c > > diff --git a/Documentation/devicetree/bindings/phy/samsung-phy.txt b/Documentation/devicetree/bindings/phy/samsung-phy.txt > index 28f9edb..6d99ba9 100644 > --- a/Documentation/devicetree/bindings/phy/samsung-phy.txt > +++ b/Documentation/devicetree/bindings/phy/samsung-phy.txt > @@ -74,3 +74,45 @@ phy-consumer@12340000 { > > Refer to DT bindings documentation of particular PHY consumer devices for more > information about required PHYs and the way of specification. > + > +Samsung Exynos5 SoC series USB DRD PHY controller > +-------------------------------------------------- > + > +Required properties: > +- compatible : Should be set to one of the following supported values: > + - "samsung,exynos5250-usbdrd-phy" - for exynos5250 SoC, > + - "samsung,exynos5420-usbdrd-phy" - for exynos5420 SoC. > +- reg : Register offset and length of USB DRD PHY register set; > +- clocks: Clock IDs array as required by the controller > +- clock-names: names of clocks correseponding to IDs in the clock property; > + Required clocks: > + - phy: main PHY clock (same as USB DRD controller i.e. DWC3 IP clock), > + used for register access. > + - ref: PHY's reference clock (usually crystal clock), associated by > + phy name, used to determine bit values for clock settings > + register. > + Additional clock required for Exynos5420: > + - usb30_sclk_100m: Additional special clock used for PHY operation > + depicted as 'sclk_usbphy30' in CMU of Exynos5420. > +- samsung,syscon-phandle: phandle for syscon interface, which is used to > + control pmu registers for power isolation. > +- samsung,pmu-offset: phy power control register offset to pmu-system-controller > + base. > +- #phy-cells : from the generic PHY bindings, must be 1; > + > +For "samsung,exynos5250-usbdrd-phy" and "samsung,exynos5420-usbdrd-phy" > +compatible PHYs, the second cell in the PHY specifier identifies the > +PHY id, which is interpreted as follows: > + 0 - UTMI+ type phy, > + 1 - PIPE3 type phy, > + > +Example: > + usb3_phy: usbphy@12100000 { > + compatible = "samsung,exynos5250-usbdrd-phy"; > + reg = <0x12100000 0x100>; > + clocks = <&clock 286>, <&clock 1>; > + clock-names = "phy", "usb3phy_refclk"; > + samsung,syscon-phandle = <&pmu_syscon>; > + samsung,pmu-offset = <0x704>; > + #phy-cells = <1>; > + }; > diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig > index 8d3c49c..d955a05 100644 > --- a/drivers/phy/Kconfig > +++ b/drivers/phy/Kconfig > @@ -166,4 +166,15 @@ config PHY_XGENE > help > This option enables support for APM X-Gene SoC multi-purpose PHY. > > +config PHY_EXYNOS5_USBDRD > + tristate "Exynos5 SoC series USB DRD PHY driver" > + depends on ARCH_EXYNOS5 && OF > + depends on HAS_IOMEM > + select GENERIC_PHY > + select MFD_SYSCON Lets try to avoid select in Kconfig. We've got enough problems with that. > + help > + Enable USB DRD PHY support for Exynos 5 SoC series. > + This driver provides PHY interface for USB 3.0 DRD controller > + present on Exynos5 SoC series. > + > endmenu > diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile > index 2faf78e..31baa0c 100644 > --- a/drivers/phy/Makefile > +++ b/drivers/phy/Makefile > @@ -18,3 +18,4 @@ obj-$(CONFIG_PHY_EXYNOS4210_USB2) += phy-exynos4210-usb2.o > obj-$(CONFIG_PHY_EXYNOS4X12_USB2) += phy-exynos4x12-usb2.o > obj-$(CONFIG_PHY_EXYNOS5250_USB2) += phy-exynos5250-usb2.o > obj-$(CONFIG_PHY_XGENE) += phy-xgene.o > +obj-$(CONFIG_PHY_EXYNOS5_USBDRD) += phy-exynos5-usbdrd.o > diff --git a/drivers/phy/phy-exynos5-usbdrd.c b/drivers/phy/phy-exynos5-usbdrd.c > new file mode 100644 > index 0000000..ff54a7c > --- /dev/null > +++ b/drivers/phy/phy-exynos5-usbdrd.c > @@ -0,0 +1,668 @@ > +/* > + * Samsung EXYNOS5 SoC series USB DRD PHY driver > + * > + * Phy provider for USB 3.0 DRD controller on Exynos5 SoC series > + * > + * Copyright (C) 2013 Samsung Electronics Co., Ltd. 2014 already ;-) > + * Author: Vivek Gautam <gautam.vivek@samsung.com> > + * > + * 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 > + * published by the Free Software Foundation. > + */ > + . . <sniip> . . > + > +/* > + * Sets the pipe3 phy's clk as EXTREFCLK (XXTI) which is internal clock > + * from clock core. Further sets multiplier values and spread spectrum > + * clock settings for SuperSpeed operations. > + */ > +static unsigned int > +exynos5_usbdrd_pipe3_set_refclk(struct phy_usb_instance *inst) > +{ > + static u32 reg; > + struct exynos5_usbdrd_phy *phy_drd = to_usbdrd_phy(inst); > + > + /* restore any previous reference clock settings */ > + reg = phy_drd->refclk_reg; Why don't we just read back from the register instead? > + > + /* Use EXTREFCLK as ref clock */ > + reg &= ~PHYCLKRST_REFCLKSEL_MASK; > + reg |= PHYCLKRST_REFCLKSEL_EXT_REFCLK; > + > + /* FSEL settings corresponding to reference clock */ > + reg &= ~PHYCLKRST_FSEL_PIPE_MASK | > + PHYCLKRST_MPLL_MULTIPLIER_MASK | > + PHYCLKRST_SSC_REFCLKSEL_MASK; > + switch (phy_drd->extrefclk) { > + case EXYNOS5_FSEL_50MHZ: > + reg |= (PHYCLKRST_MPLL_MULTIPLIER_50M_REF | > + PHYCLKRST_SSC_REFCLKSEL(0x00)); > + break; > + case EXYNOS5_FSEL_24MHZ: > + reg |= (PHYCLKRST_MPLL_MULTIPLIER_24MHZ_REF | > + PHYCLKRST_SSC_REFCLKSEL(0x88)); > + break; > + case EXYNOS5_FSEL_20MHZ: > + reg |= (PHYCLKRST_MPLL_MULTIPLIER_20MHZ_REF | > + PHYCLKRST_SSC_REFCLKSEL(0x00)); > + break; > + case EXYNOS5_FSEL_19MHZ2: > + reg |= (PHYCLKRST_MPLL_MULTIPLIER_19200KHZ_REF | > + PHYCLKRST_SSC_REFCLKSEL(0x88)); > + break; > + default: > + dev_dbg(phy_drd->dev, "unsupported ref clk\n"); > + break; > + } > + > + /* save refclk settings for multiple phy inits */ > + phy_drd->refclk_reg = reg; > + > + return reg; > +} > + > +/* > + * Sets the utmi phy's clk as EXTREFCLK (XXTI) which is internal clock > + * from clock core. Further sets the FSEL values for HighSpeed operations. > + */ > +static unsigned int > +exynos5_usbdrd_utmi_set_refclk(struct phy_usb_instance *inst) > +{ > + static u32 reg; > + struct exynos5_usbdrd_phy *phy_drd = to_usbdrd_phy(inst); > + > + reg = phy_drd->refclk_reg; same here.. Thanks Kishon
Hi, On Mon, Apr 14, 2014 at 5:57 PM, Kishon Vijay Abraham I <kishon@ti.com> wrote: > Hi, > > On Tuesday 08 April 2014 08:06 PM, Vivek Gautam wrote: >> Add a new driver for the USB 3.0 PHY on Exynos5 series of SoCs. >> The new driver uses the generic PHY framework and will interact >> with DWC3 controller present on Exynos5 series of SoCs. >> Thereby, removing old phy-samsung-usb3 driver and related code >> used untill now which was based on usb/phy framework. >> >> Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com> >> --- >> .../devicetree/bindings/phy/samsung-phy.txt | 42 ++ >> drivers/phy/Kconfig | 11 + >> drivers/phy/Makefile | 1 + >> drivers/phy/phy-exynos5-usbdrd.c | 668 ++++++++++++++++++++ >> 4 files changed, 722 insertions(+) >> create mode 100644 drivers/phy/phy-exynos5-usbdrd.c >> >> diff --git a/Documentation/devicetree/bindings/phy/samsung-phy.txt b/Documentation/devicetree/bindings/phy/samsung-phy.txt >> index 28f9edb..6d99ba9 100644 >> --- a/Documentation/devicetree/bindings/phy/samsung-phy.txt >> +++ b/Documentation/devicetree/bindings/phy/samsung-phy.txt >> @@ -74,3 +74,45 @@ phy-consumer@12340000 { >> >> Refer to DT bindings documentation of particular PHY consumer devices for more >> information about required PHYs and the way of specification. >> + >> +Samsung Exynos5 SoC series USB DRD PHY controller >> +-------------------------------------------------- >> + >> +Required properties: >> +- compatible : Should be set to one of the following supported values: >> + - "samsung,exynos5250-usbdrd-phy" - for exynos5250 SoC, >> + - "samsung,exynos5420-usbdrd-phy" - for exynos5420 SoC. >> +- reg : Register offset and length of USB DRD PHY register set; >> +- clocks: Clock IDs array as required by the controller >> +- clock-names: names of clocks correseponding to IDs in the clock property; >> + Required clocks: >> + - phy: main PHY clock (same as USB DRD controller i.e. DWC3 IP clock), >> + used for register access. >> + - ref: PHY's reference clock (usually crystal clock), associated by >> + phy name, used to determine bit values for clock settings >> + register. >> + Additional clock required for Exynos5420: >> + - usb30_sclk_100m: Additional special clock used for PHY operation >> + depicted as 'sclk_usbphy30' in CMU of Exynos5420. >> +- samsung,syscon-phandle: phandle for syscon interface, which is used to >> + control pmu registers for power isolation. >> +- samsung,pmu-offset: phy power control register offset to pmu-system-controller >> + base. >> +- #phy-cells : from the generic PHY bindings, must be 1; >> + >> +For "samsung,exynos5250-usbdrd-phy" and "samsung,exynos5420-usbdrd-phy" >> +compatible PHYs, the second cell in the PHY specifier identifies the >> +PHY id, which is interpreted as follows: >> + 0 - UTMI+ type phy, >> + 1 - PIPE3 type phy, >> + >> +Example: >> + usb3_phy: usbphy@12100000 { >> + compatible = "samsung,exynos5250-usbdrd-phy"; >> + reg = <0x12100000 0x100>; >> + clocks = <&clock 286>, <&clock 1>; >> + clock-names = "phy", "usb3phy_refclk"; >> + samsung,syscon-phandle = <&pmu_syscon>; >> + samsung,pmu-offset = <0x704>; >> + #phy-cells = <1>; >> + }; >> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig >> index 8d3c49c..d955a05 100644 >> --- a/drivers/phy/Kconfig >> +++ b/drivers/phy/Kconfig >> @@ -166,4 +166,15 @@ config PHY_XGENE >> help >> This option enables support for APM X-Gene SoC multi-purpose PHY. >> >> +config PHY_EXYNOS5_USBDRD >> + tristate "Exynos5 SoC series USB DRD PHY driver" >> + depends on ARCH_EXYNOS5 && OF >> + depends on HAS_IOMEM >> + select GENERIC_PHY >> + select MFD_SYSCON > > Lets try to avoid select in Kconfig. We've got enough problems with that. I hope you meant with "select MFD_SYSCON". We are referencing the syscon for accessing pmu reg, for which we need this config to be selected. Other Exynos phy drivers also need this config and for that they have selected this. Do you want me to do it any other way ? >> + help >> + Enable USB DRD PHY support for Exynos 5 SoC series. >> + This driver provides PHY interface for USB 3.0 DRD controller >> + present on Exynos5 SoC series. >> + >> endmenu >> diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile >> index 2faf78e..31baa0c 100644 >> --- a/drivers/phy/Makefile >> +++ b/drivers/phy/Makefile >> @@ -18,3 +18,4 @@ obj-$(CONFIG_PHY_EXYNOS4210_USB2) += phy-exynos4210-usb2.o >> obj-$(CONFIG_PHY_EXYNOS4X12_USB2) += phy-exynos4x12-usb2.o >> obj-$(CONFIG_PHY_EXYNOS5250_USB2) += phy-exynos5250-usb2.o >> obj-$(CONFIG_PHY_XGENE) += phy-xgene.o >> +obj-$(CONFIG_PHY_EXYNOS5_USBDRD) += phy-exynos5-usbdrd.o >> diff --git a/drivers/phy/phy-exynos5-usbdrd.c b/drivers/phy/phy-exynos5-usbdrd.c >> new file mode 100644 >> index 0000000..ff54a7c >> --- /dev/null >> +++ b/drivers/phy/phy-exynos5-usbdrd.c >> @@ -0,0 +1,668 @@ >> +/* >> + * Samsung EXYNOS5 SoC series USB DRD PHY driver >> + * >> + * Phy provider for USB 3.0 DRD controller on Exynos5 SoC series >> + * >> + * Copyright (C) 2013 Samsung Electronics Co., Ltd. > > 2014 already ;-) Yea. :-) will correct. >> + * Author: Vivek Gautam <gautam.vivek@samsung.com> >> + * >> + * 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 >> + * published by the Free Software Foundation. >> + */ >> + > . > . > <sniip> > . > . > >> + >> +/* >> + * Sets the pipe3 phy's clk as EXTREFCLK (XXTI) which is internal clock >> + * from clock core. Further sets multiplier values and spread spectrum >> + * clock settings for SuperSpeed operations. >> + */ >> +static unsigned int >> +exynos5_usbdrd_pipe3_set_refclk(struct phy_usb_instance *inst) >> +{ >> + static u32 reg; >> + struct exynos5_usbdrd_phy *phy_drd = to_usbdrd_phy(inst); >> + >> + /* restore any previous reference clock settings */ >> + reg = phy_drd->refclk_reg; > > Why don't we just read back from the register instead? Yes, we can do that too. Will amend it. >> + >> + /* Use EXTREFCLK as ref clock */ >> + reg &= ~PHYCLKRST_REFCLKSEL_MASK; >> + reg |= PHYCLKRST_REFCLKSEL_EXT_REFCLK; >> + >> + /* FSEL settings corresponding to reference clock */ >> + reg &= ~PHYCLKRST_FSEL_PIPE_MASK | >> + PHYCLKRST_MPLL_MULTIPLIER_MASK | >> + PHYCLKRST_SSC_REFCLKSEL_MASK; >> + switch (phy_drd->extrefclk) { >> + case EXYNOS5_FSEL_50MHZ: >> + reg |= (PHYCLKRST_MPLL_MULTIPLIER_50M_REF | >> + PHYCLKRST_SSC_REFCLKSEL(0x00)); >> + break; >> + case EXYNOS5_FSEL_24MHZ: >> + reg |= (PHYCLKRST_MPLL_MULTIPLIER_24MHZ_REF | >> + PHYCLKRST_SSC_REFCLKSEL(0x88)); >> + break; >> + case EXYNOS5_FSEL_20MHZ: >> + reg |= (PHYCLKRST_MPLL_MULTIPLIER_20MHZ_REF | >> + PHYCLKRST_SSC_REFCLKSEL(0x00)); >> + break; >> + case EXYNOS5_FSEL_19MHZ2: >> + reg |= (PHYCLKRST_MPLL_MULTIPLIER_19200KHZ_REF | >> + PHYCLKRST_SSC_REFCLKSEL(0x88)); >> + break; >> + default: >> + dev_dbg(phy_drd->dev, "unsupported ref clk\n"); >> + break; >> + } >> + >> + /* save refclk settings for multiple phy inits */ >> + phy_drd->refclk_reg = reg; >> + >> + return reg; >> +} >> + >> +/* >> + * Sets the utmi phy's clk as EXTREFCLK (XXTI) which is internal clock >> + * from clock core. Further sets the FSEL values for HighSpeed operations. >> + */ >> +static unsigned int >> +exynos5_usbdrd_utmi_set_refclk(struct phy_usb_instance *inst) >> +{ >> + static u32 reg; >> + struct exynos5_usbdrd_phy *phy_drd = to_usbdrd_phy(inst); >> + >> + reg = phy_drd->refclk_reg; > > same here.. sure > > Thanks > Kishon > -- > To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html
On Monday 14 April 2014 06:12 PM, Vivek Gautam wrote: > Hi, > > > On Mon, Apr 14, 2014 at 5:57 PM, Kishon Vijay Abraham I <kishon@ti.com> wrote: >> Hi, >> >> On Tuesday 08 April 2014 08:06 PM, Vivek Gautam wrote: >>> Add a new driver for the USB 3.0 PHY on Exynos5 series of SoCs. >>> The new driver uses the generic PHY framework and will interact >>> with DWC3 controller present on Exynos5 series of SoCs. >>> Thereby, removing old phy-samsung-usb3 driver and related code >>> used untill now which was based on usb/phy framework. >>> >>> Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com> >>> --- >>> .../devicetree/bindings/phy/samsung-phy.txt | 42 ++ >>> drivers/phy/Kconfig | 11 + >>> drivers/phy/Makefile | 1 + >>> drivers/phy/phy-exynos5-usbdrd.c | 668 ++++++++++++++++++++ >>> 4 files changed, 722 insertions(+) >>> create mode 100644 drivers/phy/phy-exynos5-usbdrd.c >>> >>> diff --git a/Documentation/devicetree/bindings/phy/samsung-phy.txt b/Documentation/devicetree/bindings/phy/samsung-phy.txt >>> index 28f9edb..6d99ba9 100644 >>> --- a/Documentation/devicetree/bindings/phy/samsung-phy.txt >>> +++ b/Documentation/devicetree/bindings/phy/samsung-phy.txt >>> @@ -74,3 +74,45 @@ phy-consumer@12340000 { >>> >>> Refer to DT bindings documentation of particular PHY consumer devices for more >>> information about required PHYs and the way of specification. >>> + >>> +Samsung Exynos5 SoC series USB DRD PHY controller >>> +-------------------------------------------------- >>> + >>> +Required properties: >>> +- compatible : Should be set to one of the following supported values: >>> + - "samsung,exynos5250-usbdrd-phy" - for exynos5250 SoC, >>> + - "samsung,exynos5420-usbdrd-phy" - for exynos5420 SoC. >>> +- reg : Register offset and length of USB DRD PHY register set; >>> +- clocks: Clock IDs array as required by the controller >>> +- clock-names: names of clocks correseponding to IDs in the clock property; >>> + Required clocks: >>> + - phy: main PHY clock (same as USB DRD controller i.e. DWC3 IP clock), >>> + used for register access. >>> + - ref: PHY's reference clock (usually crystal clock), associated by >>> + phy name, used to determine bit values for clock settings >>> + register. >>> + Additional clock required for Exynos5420: >>> + - usb30_sclk_100m: Additional special clock used for PHY operation >>> + depicted as 'sclk_usbphy30' in CMU of Exynos5420. >>> +- samsung,syscon-phandle: phandle for syscon interface, which is used to >>> + control pmu registers for power isolation. >>> +- samsung,pmu-offset: phy power control register offset to pmu-system-controller >>> + base. >>> +- #phy-cells : from the generic PHY bindings, must be 1; >>> + >>> +For "samsung,exynos5250-usbdrd-phy" and "samsung,exynos5420-usbdrd-phy" >>> +compatible PHYs, the second cell in the PHY specifier identifies the >>> +PHY id, which is interpreted as follows: >>> + 0 - UTMI+ type phy, >>> + 1 - PIPE3 type phy, >>> + >>> +Example: >>> + usb3_phy: usbphy@12100000 { >>> + compatible = "samsung,exynos5250-usbdrd-phy"; >>> + reg = <0x12100000 0x100>; >>> + clocks = <&clock 286>, <&clock 1>; >>> + clock-names = "phy", "usb3phy_refclk"; >>> + samsung,syscon-phandle = <&pmu_syscon>; >>> + samsung,pmu-offset = <0x704>; >>> + #phy-cells = <1>; >>> + }; >>> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig >>> index 8d3c49c..d955a05 100644 >>> --- a/drivers/phy/Kconfig >>> +++ b/drivers/phy/Kconfig >>> @@ -166,4 +166,15 @@ config PHY_XGENE >>> help >>> This option enables support for APM X-Gene SoC multi-purpose PHY. >>> >>> +config PHY_EXYNOS5_USBDRD >>> + tristate "Exynos5 SoC series USB DRD PHY driver" >>> + depends on ARCH_EXYNOS5 && OF >>> + depends on HAS_IOMEM >>> + select GENERIC_PHY >>> + select MFD_SYSCON >> >> Lets try to avoid select in Kconfig. We've got enough problems with that. > > I hope you meant with "select MFD_SYSCON". > We are referencing the syscon for accessing pmu reg, for which we need > this config to be selected. > Other Exynos phy drivers also need this config and for that they have > selected this. > > Do you want me to do it any other way ? depends on is one option. Thanks Kishon
On Monday 14 April 2014 05:35 PM, Vivek Gautam wrote: > Hi Kishon, > > > On Mon, Apr 14, 2014 at 5:24 PM, Kishon Vijay Abraham I <kishon@ti.com> wrote: >> Hi, >> >> On Wednesday 09 April 2014 04:36 PM, Tomasz Figa wrote: >>> Hi Vivek, >>> >>> Please see my comments inline. >>> >>> On 08.04.2014 16:36, Vivek Gautam wrote: >>>> Add a new driver for the USB 3.0 PHY on Exynos5 series of SoCs. >>>> The new driver uses the generic PHY framework and will interact >>>> with DWC3 controller present on Exynos5 series of SoCs. >>>> Thereby, removing old phy-samsung-usb3 driver and related code >>>> used untill now which was based on usb/phy framework. >>>> >>>> Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com> >>>> --- >>>> .../devicetree/bindings/phy/samsung-phy.txt | 42 ++ >>>> drivers/phy/Kconfig | 11 + >>>> drivers/phy/Makefile | 1 + >>>> drivers/phy/phy-exynos5-usbdrd.c | 668 ++++++++++++++++++++ >>>> 4 files changed, 722 insertions(+) >>>> create mode 100644 drivers/phy/phy-exynos5-usbdrd.c >>> >>> [snip] >>> >>>> + Additional clock required for Exynos5420: >>>> + - usb30_sclk_100m: Additional special clock used for PHY operation >>>> + depicted as 'sclk_usbphy30' in CMU of Exynos5420. >>> >>> Are you sure this isn't simply a gate for the ref clock, as it can be found on >>> another SoC that is not upstream yet? I don't have documentation for Exynos >>> 5420 so I can't tell, but I'd like to ask you to recheck this. >>> >>>> +- samsung,syscon-phandle: phandle for syscon interface, which is used to >>>> + control pmu registers for power isolation. >>>> +- samsung,pmu-offset: phy power control register offset to >>>> pmu-system-controller >>>> + base. >>>> +- #phy-cells : from the generic PHY bindings, must be 1; >>>> + >>>> +For "samsung,exynos5250-usbdrd-phy" and "samsung,exynos5420-usbdrd-phy" >>>> +compatible PHYs, the second cell in the PHY specifier identifies the >>>> +PHY id, which is interpreted as follows: >>>> + 0 - UTMI+ type phy, >>>> + 1 - PIPE3 type phy, >>>> + >>>> +Example: >>>> + usb3_phy: usbphy@12100000 { >>>> + compatible = "samsung,exynos5250-usbdrd-phy"; >>>> + reg = <0x12100000 0x100>; >>>> + clocks = <&clock 286>, <&clock 1>; >>>> + clock-names = "phy", "usb3phy_refclk"; >>> >>> Binding description above doesn't mention "usb3phy_refclk" entry. >>> >>>> + samsung,syscon-phandle = <&pmu_syscon>; >>>> + samsung,pmu-offset = <0x704>; >>>> + #phy-cells = <1>; >>>> + }; >>> >>> [snip] >>> >>>> diff --git a/drivers/phy/phy-exynos5-usbdrd.c b/drivers/phy/phy-exynos5-usbdrd.c >>>> new file mode 100644 >>>> index 0000000..ff54a7c >>>> --- /dev/null >>>> +++ b/drivers/phy/phy-exynos5-usbdrd.c >>> >>> [snip] >>> >>>> +static int exynos5_usbdrd_phy_probe(struct platform_device *pdev) >>>> +{ >>>> + struct device *dev = &pdev->dev; >>>> + struct device_node *node = dev->of_node; >>>> + struct exynos5_usbdrd_phy *phy_drd; >>>> + struct phy_provider *phy_provider; >>>> + struct resource *res; >>>> + const struct of_device_id *match; >>>> + const struct exynos5_usbdrd_phy_drvdata *drv_data; >>>> + struct regmap *reg_pmu; >>>> + u32 pmu_offset; >>>> + int i; >>>> + >>>> + /* >>>> + * Exynos systems are completely DT enabled, >>>> + * so lets not have any platform data support for this driver. >>>> + */ >>>> + if (!node) { >>>> + dev_err(dev, "no device node found\n"); >>> >>> This error message is not very meaningful. I'd rather use something like "This >>> driver can be only instantiated using Device Tree". >> >> how about just adding depend_on OF in Kconfig? > > Already added a depend on 'OF'. Copying below the part of Kconfig in this patch. Alright.. Do we need the check then? If config_OF is enabled devices will be created using device tree no? Thanks Kishon
On Mon, Apr 14, 2014 at 6:29 PM, Kishon Vijay Abraham I <kishon@ti.com> wrote: > > > On Monday 14 April 2014 06:12 PM, Vivek Gautam wrote: >> Hi, >> >> >> On Mon, Apr 14, 2014 at 5:57 PM, Kishon Vijay Abraham I <kishon@ti.com> wrote: >>> Hi, >>> >>> On Tuesday 08 April 2014 08:06 PM, Vivek Gautam wrote: >>>> Add a new driver for the USB 3.0 PHY on Exynos5 series of SoCs. >>>> The new driver uses the generic PHY framework and will interact >>>> with DWC3 controller present on Exynos5 series of SoCs. >>>> Thereby, removing old phy-samsung-usb3 driver and related code >>>> used untill now which was based on usb/phy framework. >>>> >>>> Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com> >>>> --- >>>> .../devicetree/bindings/phy/samsung-phy.txt | 42 ++ >>>> drivers/phy/Kconfig | 11 + >>>> drivers/phy/Makefile | 1 + >>>> drivers/phy/phy-exynos5-usbdrd.c | 668 ++++++++++++++++++++ >>>> 4 files changed, 722 insertions(+) >>>> create mode 100644 drivers/phy/phy-exynos5-usbdrd.c >>>> >>>> diff --git a/Documentation/devicetree/bindings/phy/samsung-phy.txt b/Documentation/devicetree/bindings/phy/samsung-phy.txt >>>> index 28f9edb..6d99ba9 100644 >>>> --- a/Documentation/devicetree/bindings/phy/samsung-phy.txt >>>> +++ b/Documentation/devicetree/bindings/phy/samsung-phy.txt >>>> @@ -74,3 +74,45 @@ phy-consumer@12340000 { >>>> >>>> Refer to DT bindings documentation of particular PHY consumer devices for more >>>> information about required PHYs and the way of specification. >>>> + >>>> +Samsung Exynos5 SoC series USB DRD PHY controller >>>> +-------------------------------------------------- >>>> + >>>> +Required properties: >>>> +- compatible : Should be set to one of the following supported values: >>>> + - "samsung,exynos5250-usbdrd-phy" - for exynos5250 SoC, >>>> + - "samsung,exynos5420-usbdrd-phy" - for exynos5420 SoC. >>>> +- reg : Register offset and length of USB DRD PHY register set; >>>> +- clocks: Clock IDs array as required by the controller >>>> +- clock-names: names of clocks correseponding to IDs in the clock property; >>>> + Required clocks: >>>> + - phy: main PHY clock (same as USB DRD controller i.e. DWC3 IP clock), >>>> + used for register access. >>>> + - ref: PHY's reference clock (usually crystal clock), associated by >>>> + phy name, used to determine bit values for clock settings >>>> + register. >>>> + Additional clock required for Exynos5420: >>>> + - usb30_sclk_100m: Additional special clock used for PHY operation >>>> + depicted as 'sclk_usbphy30' in CMU of Exynos5420. >>>> +- samsung,syscon-phandle: phandle for syscon interface, which is used to >>>> + control pmu registers for power isolation. >>>> +- samsung,pmu-offset: phy power control register offset to pmu-system-controller >>>> + base. >>>> +- #phy-cells : from the generic PHY bindings, must be 1; >>>> + >>>> +For "samsung,exynos5250-usbdrd-phy" and "samsung,exynos5420-usbdrd-phy" >>>> +compatible PHYs, the second cell in the PHY specifier identifies the >>>> +PHY id, which is interpreted as follows: >>>> + 0 - UTMI+ type phy, >>>> + 1 - PIPE3 type phy, >>>> + >>>> +Example: >>>> + usb3_phy: usbphy@12100000 { >>>> + compatible = "samsung,exynos5250-usbdrd-phy"; >>>> + reg = <0x12100000 0x100>; >>>> + clocks = <&clock 286>, <&clock 1>; >>>> + clock-names = "phy", "usb3phy_refclk"; >>>> + samsung,syscon-phandle = <&pmu_syscon>; >>>> + samsung,pmu-offset = <0x704>; >>>> + #phy-cells = <1>; >>>> + }; >>>> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig >>>> index 8d3c49c..d955a05 100644 >>>> --- a/drivers/phy/Kconfig >>>> +++ b/drivers/phy/Kconfig >>>> @@ -166,4 +166,15 @@ config PHY_XGENE >>>> help >>>> This option enables support for APM X-Gene SoC multi-purpose PHY. >>>> >>>> +config PHY_EXYNOS5_USBDRD >>>> + tristate "Exynos5 SoC series USB DRD PHY driver" >>>> + depends on ARCH_EXYNOS5 && OF >>>> + depends on HAS_IOMEM >>>> + select GENERIC_PHY >>>> + select MFD_SYSCON >>> >>> Lets try to avoid select in Kconfig. We've got enough problems with that. >> >> I hope you meant with "select MFD_SYSCON". >> We are referencing the syscon for accessing pmu reg, for which we need >> this config to be selected. >> Other Exynos phy drivers also need this config and for that they have >> selected this. >> >> Do you want me to do it any other way ? > > depends on is one option. Ok, i can see there are places where depends_on MFD_SYSCON is used. "drivers/gpu/drm/exynos/Kconfig:60" so, do you want me to fix at other places too ? But i also have a question here. MFD_SYSCON is a subsystem that's facilitating us in getting our work done here by giving an access to pmu_system_controller. So unless MFD_SYSCON is exposed, the phy driver will not be exposed to us. So is that valid enough really ? > > Thanks > Kishon > -- > To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html
On Monday 14 April 2014 06:50 PM, Vivek Gautam wrote: > On Mon, Apr 14, 2014 at 6:29 PM, Kishon Vijay Abraham I <kishon@ti.com> wrote: >> >> >> On Monday 14 April 2014 06:12 PM, Vivek Gautam wrote: >>> Hi, >>> >>> >>> On Mon, Apr 14, 2014 at 5:57 PM, Kishon Vijay Abraham I <kishon@ti.com> wrote: >>>> Hi, >>>> >>>> On Tuesday 08 April 2014 08:06 PM, Vivek Gautam wrote: >>>>> Add a new driver for the USB 3.0 PHY on Exynos5 series of SoCs. >>>>> The new driver uses the generic PHY framework and will interact >>>>> with DWC3 controller present on Exynos5 series of SoCs. >>>>> Thereby, removing old phy-samsung-usb3 driver and related code >>>>> used untill now which was based on usb/phy framework. >>>>> >>>>> Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com> >>>>> --- >>>>> .../devicetree/bindings/phy/samsung-phy.txt | 42 ++ >>>>> drivers/phy/Kconfig | 11 + >>>>> drivers/phy/Makefile | 1 + >>>>> drivers/phy/phy-exynos5-usbdrd.c | 668 ++++++++++++++++++++ >>>>> 4 files changed, 722 insertions(+) >>>>> create mode 100644 drivers/phy/phy-exynos5-usbdrd.c >>>>> >>>>> diff --git a/Documentation/devicetree/bindings/phy/samsung-phy.txt b/Documentation/devicetree/bindings/phy/samsung-phy.txt >>>>> index 28f9edb..6d99ba9 100644 >>>>> --- a/Documentation/devicetree/bindings/phy/samsung-phy.txt >>>>> +++ b/Documentation/devicetree/bindings/phy/samsung-phy.txt >>>>> @@ -74,3 +74,45 @@ phy-consumer@12340000 { >>>>> >>>>> Refer to DT bindings documentation of particular PHY consumer devices for more >>>>> information about required PHYs and the way of specification. >>>>> + >>>>> +Samsung Exynos5 SoC series USB DRD PHY controller >>>>> +-------------------------------------------------- >>>>> + >>>>> +Required properties: >>>>> +- compatible : Should be set to one of the following supported values: >>>>> + - "samsung,exynos5250-usbdrd-phy" - for exynos5250 SoC, >>>>> + - "samsung,exynos5420-usbdrd-phy" - for exynos5420 SoC. >>>>> +- reg : Register offset and length of USB DRD PHY register set; >>>>> +- clocks: Clock IDs array as required by the controller >>>>> +- clock-names: names of clocks correseponding to IDs in the clock property; >>>>> + Required clocks: >>>>> + - phy: main PHY clock (same as USB DRD controller i.e. DWC3 IP clock), >>>>> + used for register access. >>>>> + - ref: PHY's reference clock (usually crystal clock), associated by >>>>> + phy name, used to determine bit values for clock settings >>>>> + register. >>>>> + Additional clock required for Exynos5420: >>>>> + - usb30_sclk_100m: Additional special clock used for PHY operation >>>>> + depicted as 'sclk_usbphy30' in CMU of Exynos5420. >>>>> +- samsung,syscon-phandle: phandle for syscon interface, which is used to >>>>> + control pmu registers for power isolation. >>>>> +- samsung,pmu-offset: phy power control register offset to pmu-system-controller >>>>> + base. >>>>> +- #phy-cells : from the generic PHY bindings, must be 1; >>>>> + >>>>> +For "samsung,exynos5250-usbdrd-phy" and "samsung,exynos5420-usbdrd-phy" >>>>> +compatible PHYs, the second cell in the PHY specifier identifies the >>>>> +PHY id, which is interpreted as follows: >>>>> + 0 - UTMI+ type phy, >>>>> + 1 - PIPE3 type phy, >>>>> + >>>>> +Example: >>>>> + usb3_phy: usbphy@12100000 { >>>>> + compatible = "samsung,exynos5250-usbdrd-phy"; >>>>> + reg = <0x12100000 0x100>; >>>>> + clocks = <&clock 286>, <&clock 1>; >>>>> + clock-names = "phy", "usb3phy_refclk"; >>>>> + samsung,syscon-phandle = <&pmu_syscon>; >>>>> + samsung,pmu-offset = <0x704>; >>>>> + #phy-cells = <1>; >>>>> + }; >>>>> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig >>>>> index 8d3c49c..d955a05 100644 >>>>> --- a/drivers/phy/Kconfig >>>>> +++ b/drivers/phy/Kconfig >>>>> @@ -166,4 +166,15 @@ config PHY_XGENE >>>>> help >>>>> This option enables support for APM X-Gene SoC multi-purpose PHY. >>>>> >>>>> +config PHY_EXYNOS5_USBDRD >>>>> + tristate "Exynos5 SoC series USB DRD PHY driver" >>>>> + depends on ARCH_EXYNOS5 && OF >>>>> + depends on HAS_IOMEM >>>>> + select GENERIC_PHY >>>>> + select MFD_SYSCON >>>> >>>> Lets try to avoid select in Kconfig. We've got enough problems with that. >>> >>> I hope you meant with "select MFD_SYSCON". >>> We are referencing the syscon for accessing pmu reg, for which we need >>> this config to be selected. >>> Other Exynos phy drivers also need this config and for that they have >>> selected this. >>> >>> Do you want me to do it any other way ? >> >> depends on is one option. > > Ok, i can see there are places where depends_on MFD_SYSCON is used. > "drivers/gpu/drm/exynos/Kconfig:60" > > so, do you want me to fix at other places too ? > > But i also have a question here. > MFD_SYSCON is a subsystem that's facilitating us in getting our work > done here by giving an access to pmu_system_controller. > So unless MFD_SYSCON is exposed, the phy driver will not be exposed to us. > So is that valid enough really ? maybe in the Kconfig for MFD_SYSCON, we can select it if PHY_EXYNOS5_USBDRD is enabled? config MFD_SYSCON default y if PHY_EXYNOS5_USBDRD Thanks Kishon
On Mon, Apr 14, 2014 at 6:56 PM, Kishon Vijay Abraham I <kishon@ti.com> wrote: > > > On Monday 14 April 2014 06:50 PM, Vivek Gautam wrote: >> On Mon, Apr 14, 2014 at 6:29 PM, Kishon Vijay Abraham I <kishon@ti.com> wrote: >>> >>> >>> On Monday 14 April 2014 06:12 PM, Vivek Gautam wrote: >>>> Hi, >>>> >>>> >>>> On Mon, Apr 14, 2014 at 5:57 PM, Kishon Vijay Abraham I <kishon@ti.com> wrote: >>>>> Hi, >>>>> >>>>> On Tuesday 08 April 2014 08:06 PM, Vivek Gautam wrote: >>>>>> Add a new driver for the USB 3.0 PHY on Exynos5 series of SoCs. >>>>>> The new driver uses the generic PHY framework and will interact >>>>>> with DWC3 controller present on Exynos5 series of SoCs. >>>>>> Thereby, removing old phy-samsung-usb3 driver and related code >>>>>> used untill now which was based on usb/phy framework. >>>>>> >>>>>> Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com> >>>>>> --- >>>>>> .../devicetree/bindings/phy/samsung-phy.txt | 42 ++ >>>>>> drivers/phy/Kconfig | 11 + >>>>>> drivers/phy/Makefile | 1 + >>>>>> drivers/phy/phy-exynos5-usbdrd.c | 668 ++++++++++++++++++++ >>>>>> 4 files changed, 722 insertions(+) >>>>>> create mode 100644 drivers/phy/phy-exynos5-usbdrd.c >>>>>> >>>>>> diff --git a/Documentation/devicetree/bindings/phy/samsung-phy.txt b/Documentation/devicetree/bindings/phy/samsung-phy.txt >>>>>> index 28f9edb..6d99ba9 100644 >>>>>> --- a/Documentation/devicetree/bindings/phy/samsung-phy.txt >>>>>> +++ b/Documentation/devicetree/bindings/phy/samsung-phy.txt >>>>>> @@ -74,3 +74,45 @@ phy-consumer@12340000 { >>>>>> >>>>>> Refer to DT bindings documentation of particular PHY consumer devices for more >>>>>> information about required PHYs and the way of specification. >>>>>> + >>>>>> +Samsung Exynos5 SoC series USB DRD PHY controller >>>>>> +-------------------------------------------------- >>>>>> + >>>>>> +Required properties: >>>>>> +- compatible : Should be set to one of the following supported values: >>>>>> + - "samsung,exynos5250-usbdrd-phy" - for exynos5250 SoC, >>>>>> + - "samsung,exynos5420-usbdrd-phy" - for exynos5420 SoC. >>>>>> +- reg : Register offset and length of USB DRD PHY register set; >>>>>> +- clocks: Clock IDs array as required by the controller >>>>>> +- clock-names: names of clocks correseponding to IDs in the clock property; >>>>>> + Required clocks: >>>>>> + - phy: main PHY clock (same as USB DRD controller i.e. DWC3 IP clock), >>>>>> + used for register access. >>>>>> + - ref: PHY's reference clock (usually crystal clock), associated by >>>>>> + phy name, used to determine bit values for clock settings >>>>>> + register. >>>>>> + Additional clock required for Exynos5420: >>>>>> + - usb30_sclk_100m: Additional special clock used for PHY operation >>>>>> + depicted as 'sclk_usbphy30' in CMU of Exynos5420. >>>>>> +- samsung,syscon-phandle: phandle for syscon interface, which is used to >>>>>> + control pmu registers for power isolation. >>>>>> +- samsung,pmu-offset: phy power control register offset to pmu-system-controller >>>>>> + base. >>>>>> +- #phy-cells : from the generic PHY bindings, must be 1; >>>>>> + >>>>>> +For "samsung,exynos5250-usbdrd-phy" and "samsung,exynos5420-usbdrd-phy" >>>>>> +compatible PHYs, the second cell in the PHY specifier identifies the >>>>>> +PHY id, which is interpreted as follows: >>>>>> + 0 - UTMI+ type phy, >>>>>> + 1 - PIPE3 type phy, >>>>>> + >>>>>> +Example: >>>>>> + usb3_phy: usbphy@12100000 { >>>>>> + compatible = "samsung,exynos5250-usbdrd-phy"; >>>>>> + reg = <0x12100000 0x100>; >>>>>> + clocks = <&clock 286>, <&clock 1>; >>>>>> + clock-names = "phy", "usb3phy_refclk"; >>>>>> + samsung,syscon-phandle = <&pmu_syscon>; >>>>>> + samsung,pmu-offset = <0x704>; >>>>>> + #phy-cells = <1>; >>>>>> + }; >>>>>> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig >>>>>> index 8d3c49c..d955a05 100644 >>>>>> --- a/drivers/phy/Kconfig >>>>>> +++ b/drivers/phy/Kconfig >>>>>> @@ -166,4 +166,15 @@ config PHY_XGENE >>>>>> help >>>>>> This option enables support for APM X-Gene SoC multi-purpose PHY. >>>>>> >>>>>> +config PHY_EXYNOS5_USBDRD >>>>>> + tristate "Exynos5 SoC series USB DRD PHY driver" >>>>>> + depends on ARCH_EXYNOS5 && OF >>>>>> + depends on HAS_IOMEM >>>>>> + select GENERIC_PHY >>>>>> + select MFD_SYSCON >>>>> >>>>> Lets try to avoid select in Kconfig. We've got enough problems with that. >>>> >>>> I hope you meant with "select MFD_SYSCON". >>>> We are referencing the syscon for accessing pmu reg, for which we need >>>> this config to be selected. >>>> Other Exynos phy drivers also need this config and for that they have >>>> selected this. >>>> >>>> Do you want me to do it any other way ? >>> >>> depends on is one option. >> >> Ok, i can see there are places where depends_on MFD_SYSCON is used. >> "drivers/gpu/drm/exynos/Kconfig:60" >> >> so, do you want me to fix at other places too ? >> >> But i also have a question here. >> MFD_SYSCON is a subsystem that's facilitating us in getting our work >> done here by giving an access to pmu_system_controller. >> So unless MFD_SYSCON is exposed, the phy driver will not be exposed to us. >> So is that valid enough really ? > > maybe in the Kconfig for MFD_SYSCON, we can select it if PHY_EXYNOS5_USBDRD is > enabled? > > config MFD_SYSCON > default y if PHY_EXYNOS5_USBDRD Yes, that seems to be one option. But we will end up adding this dependency for all the drivers which want to use syscon. Adding few people who worked on MFS_SYSCON, just to gather ideas. Re-quoting our problem here: "Inside our phy driver we are accessing power mgt unit registers, which we register using syscon in ut device tree. Now we don't want to select MFD_SYSCON from our phy driver, instead want to go 'depend_on' way." So one solution seems to be enabling MFD_SYSCON by default, if any of driver using syscon is enabled. Can we use some other way here ? > > Thanks > Kishon
On 14.04.2014 15:05, Kishon Vijay Abraham I wrote: > > > On Monday 14 April 2014 05:35 PM, Vivek Gautam wrote: >> Hi Kishon, >> >> >> On Mon, Apr 14, 2014 at 5:24 PM, Kishon Vijay Abraham I <kishon@ti.com> wrote: >>> Hi, >>> >>> On Wednesday 09 April 2014 04:36 PM, Tomasz Figa wrote: >>>> Hi Vivek, >>>> >>>> Please see my comments inline. >>>> >>>> On 08.04.2014 16:36, Vivek Gautam wrote: >>>>> Add a new driver for the USB 3.0 PHY on Exynos5 series of SoCs. >>>>> The new driver uses the generic PHY framework and will interact >>>>> with DWC3 controller present on Exynos5 series of SoCs. >>>>> Thereby, removing old phy-samsung-usb3 driver and related code >>>>> used untill now which was based on usb/phy framework. >>>>> >>>>> Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com> >>>>> --- >>>>> .../devicetree/bindings/phy/samsung-phy.txt | 42 ++ >>>>> drivers/phy/Kconfig | 11 + >>>>> drivers/phy/Makefile | 1 + >>>>> drivers/phy/phy-exynos5-usbdrd.c | 668 ++++++++++++++++++++ >>>>> 4 files changed, 722 insertions(+) >>>>> create mode 100644 drivers/phy/phy-exynos5-usbdrd.c >>>> >>>> [snip] >>>> >>>>> + Additional clock required for Exynos5420: >>>>> + - usb30_sclk_100m: Additional special clock used for PHY operation >>>>> + depicted as 'sclk_usbphy30' in CMU of Exynos5420. >>>> >>>> Are you sure this isn't simply a gate for the ref clock, as it can be found on >>>> another SoC that is not upstream yet? I don't have documentation for Exynos >>>> 5420 so I can't tell, but I'd like to ask you to recheck this. >>>> >>>>> +- samsung,syscon-phandle: phandle for syscon interface, which is used to >>>>> + control pmu registers for power isolation. >>>>> +- samsung,pmu-offset: phy power control register offset to >>>>> pmu-system-controller >>>>> + base. >>>>> +- #phy-cells : from the generic PHY bindings, must be 1; >>>>> + >>>>> +For "samsung,exynos5250-usbdrd-phy" and "samsung,exynos5420-usbdrd-phy" >>>>> +compatible PHYs, the second cell in the PHY specifier identifies the >>>>> +PHY id, which is interpreted as follows: >>>>> + 0 - UTMI+ type phy, >>>>> + 1 - PIPE3 type phy, >>>>> + >>>>> +Example: >>>>> + usb3_phy: usbphy@12100000 { >>>>> + compatible = "samsung,exynos5250-usbdrd-phy"; >>>>> + reg = <0x12100000 0x100>; >>>>> + clocks = <&clock 286>, <&clock 1>; >>>>> + clock-names = "phy", "usb3phy_refclk"; >>>> >>>> Binding description above doesn't mention "usb3phy_refclk" entry. >>>> >>>>> + samsung,syscon-phandle = <&pmu_syscon>; >>>>> + samsung,pmu-offset = <0x704>; >>>>> + #phy-cells = <1>; >>>>> + }; >>>> >>>> [snip] >>>> >>>>> diff --git a/drivers/phy/phy-exynos5-usbdrd.c b/drivers/phy/phy-exynos5-usbdrd.c >>>>> new file mode 100644 >>>>> index 0000000..ff54a7c >>>>> --- /dev/null >>>>> +++ b/drivers/phy/phy-exynos5-usbdrd.c >>>> >>>> [snip] >>>> >>>>> +static int exynos5_usbdrd_phy_probe(struct platform_device *pdev) >>>>> +{ >>>>> + struct device *dev = &pdev->dev; >>>>> + struct device_node *node = dev->of_node; >>>>> + struct exynos5_usbdrd_phy *phy_drd; >>>>> + struct phy_provider *phy_provider; >>>>> + struct resource *res; >>>>> + const struct of_device_id *match; >>>>> + const struct exynos5_usbdrd_phy_drvdata *drv_data; >>>>> + struct regmap *reg_pmu; >>>>> + u32 pmu_offset; >>>>> + int i; >>>>> + >>>>> + /* >>>>> + * Exynos systems are completely DT enabled, >>>>> + * so lets not have any platform data support for this driver. >>>>> + */ >>>>> + if (!node) { >>>>> + dev_err(dev, "no device node found\n"); >>>> >>>> This error message is not very meaningful. I'd rather use something like "This >>>> driver can be only instantiated using Device Tree". >>> >>> how about just adding depend_on OF in Kconfig? >> >> Already added a depend on 'OF'. Copying below the part of Kconfig in this patch. > > Alright.. Do we need the check then? If config_OF is enabled devices will be > created using device tree no? Not necessarily. Enabling support for OF doesn't mean that it is the only boot method that can be used. Legacy board files may be still available. I'm not sure why someone would try to instantiate this driver from them, though. Best regards, Tomasz
On Mon, Apr 14, 2014 at 7:10 PM, Vivek Gautam <gautam.vivek@samsung.com> wrote: Just correcting mail-ids for Mark and Dong with the latest ones (earlier ones got bounced back) > On Mon, Apr 14, 2014 at 6:56 PM, Kishon Vijay Abraham I <kishon@ti.com> wrote: >> >> >> On Monday 14 April 2014 06:50 PM, Vivek Gautam wrote: >>> On Mon, Apr 14, 2014 at 6:29 PM, Kishon Vijay Abraham I <kishon@ti.com> wrote: >>>> >>>> >>>> On Monday 14 April 2014 06:12 PM, Vivek Gautam wrote: >>>>> Hi, >>>>> >>>>> >>>>> On Mon, Apr 14, 2014 at 5:57 PM, Kishon Vijay Abraham I <kishon@ti.com> wrote: >>>>>> Hi, >>>>>> >>>>>> On Tuesday 08 April 2014 08:06 PM, Vivek Gautam wrote: >>>>>>> Add a new driver for the USB 3.0 PHY on Exynos5 series of SoCs. >>>>>>> The new driver uses the generic PHY framework and will interact >>>>>>> with DWC3 controller present on Exynos5 series of SoCs. >>>>>>> Thereby, removing old phy-samsung-usb3 driver and related code >>>>>>> used untill now which was based on usb/phy framework. >>>>>>> >>>>>>> Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com> >>>>>>> --- >>>>>>> .../devicetree/bindings/phy/samsung-phy.txt | 42 ++ >>>>>>> drivers/phy/Kconfig | 11 + >>>>>>> drivers/phy/Makefile | 1 + >>>>>>> drivers/phy/phy-exynos5-usbdrd.c | 668 ++++++++++++++++++++ >>>>>>> 4 files changed, 722 insertions(+) >>>>>>> create mode 100644 drivers/phy/phy-exynos5-usbdrd.c >>>>>>> >>>>>>> diff --git a/Documentation/devicetree/bindings/phy/samsung-phy.txt b/Documentation/devicetree/bindings/phy/samsung-phy.txt >>>>>>> index 28f9edb..6d99ba9 100644 >>>>>>> --- a/Documentation/devicetree/bindings/phy/samsung-phy.txt >>>>>>> +++ b/Documentation/devicetree/bindings/phy/samsung-phy.txt >>>>>>> @@ -74,3 +74,45 @@ phy-consumer@12340000 { >>>>>>> >>>>>>> Refer to DT bindings documentation of particular PHY consumer devices for more >>>>>>> information about required PHYs and the way of specification. >>>>>>> + >>>>>>> +Samsung Exynos5 SoC series USB DRD PHY controller >>>>>>> +-------------------------------------------------- >>>>>>> + >>>>>>> +Required properties: >>>>>>> +- compatible : Should be set to one of the following supported values: >>>>>>> + - "samsung,exynos5250-usbdrd-phy" - for exynos5250 SoC, >>>>>>> + - "samsung,exynos5420-usbdrd-phy" - for exynos5420 SoC. >>>>>>> +- reg : Register offset and length of USB DRD PHY register set; >>>>>>> +- clocks: Clock IDs array as required by the controller >>>>>>> +- clock-names: names of clocks correseponding to IDs in the clock property; >>>>>>> + Required clocks: >>>>>>> + - phy: main PHY clock (same as USB DRD controller i.e. DWC3 IP clock), >>>>>>> + used for register access. >>>>>>> + - ref: PHY's reference clock (usually crystal clock), associated by >>>>>>> + phy name, used to determine bit values for clock settings >>>>>>> + register. >>>>>>> + Additional clock required for Exynos5420: >>>>>>> + - usb30_sclk_100m: Additional special clock used for PHY operation >>>>>>> + depicted as 'sclk_usbphy30' in CMU of Exynos5420. >>>>>>> +- samsung,syscon-phandle: phandle for syscon interface, which is used to >>>>>>> + control pmu registers for power isolation. >>>>>>> +- samsung,pmu-offset: phy power control register offset to pmu-system-controller >>>>>>> + base. >>>>>>> +- #phy-cells : from the generic PHY bindings, must be 1; >>>>>>> + >>>>>>> +For "samsung,exynos5250-usbdrd-phy" and "samsung,exynos5420-usbdrd-phy" >>>>>>> +compatible PHYs, the second cell in the PHY specifier identifies the >>>>>>> +PHY id, which is interpreted as follows: >>>>>>> + 0 - UTMI+ type phy, >>>>>>> + 1 - PIPE3 type phy, >>>>>>> + >>>>>>> +Example: >>>>>>> + usb3_phy: usbphy@12100000 { >>>>>>> + compatible = "samsung,exynos5250-usbdrd-phy"; >>>>>>> + reg = <0x12100000 0x100>; >>>>>>> + clocks = <&clock 286>, <&clock 1>; >>>>>>> + clock-names = "phy", "usb3phy_refclk"; >>>>>>> + samsung,syscon-phandle = <&pmu_syscon>; >>>>>>> + samsung,pmu-offset = <0x704>; >>>>>>> + #phy-cells = <1>; >>>>>>> + }; >>>>>>> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig >>>>>>> index 8d3c49c..d955a05 100644 >>>>>>> --- a/drivers/phy/Kconfig >>>>>>> +++ b/drivers/phy/Kconfig >>>>>>> @@ -166,4 +166,15 @@ config PHY_XGENE >>>>>>> help >>>>>>> This option enables support for APM X-Gene SoC multi-purpose PHY. >>>>>>> >>>>>>> +config PHY_EXYNOS5_USBDRD >>>>>>> + tristate "Exynos5 SoC series USB DRD PHY driver" >>>>>>> + depends on ARCH_EXYNOS5 && OF >>>>>>> + depends on HAS_IOMEM >>>>>>> + select GENERIC_PHY >>>>>>> + select MFD_SYSCON >>>>>> >>>>>> Lets try to avoid select in Kconfig. We've got enough problems with that. >>>>> >>>>> I hope you meant with "select MFD_SYSCON". >>>>> We are referencing the syscon for accessing pmu reg, for which we need >>>>> this config to be selected. >>>>> Other Exynos phy drivers also need this config and for that they have >>>>> selected this. >>>>> >>>>> Do you want me to do it any other way ? >>>> >>>> depends on is one option. >>> >>> Ok, i can see there are places where depends_on MFD_SYSCON is used. >>> "drivers/gpu/drm/exynos/Kconfig:60" >>> >>> so, do you want me to fix at other places too ? >>> >>> But i also have a question here. >>> MFD_SYSCON is a subsystem that's facilitating us in getting our work >>> done here by giving an access to pmu_system_controller. >>> So unless MFD_SYSCON is exposed, the phy driver will not be exposed to us. >>> So is that valid enough really ? >> >> maybe in the Kconfig for MFD_SYSCON, we can select it if PHY_EXYNOS5_USBDRD is >> enabled? >> >> config MFD_SYSCON >> default y if PHY_EXYNOS5_USBDRD > > Yes, that seems to be one option. But we will end up adding this > dependency for all the drivers which want to use syscon. > > Adding few people who worked on MFS_SYSCON, just to gather ideas. > > Re-quoting our problem here: > "Inside our phy driver we are accessing power mgt unit registers, > which we register using syscon in ut device tree. > Now we don't want to select MFD_SYSCON from our phy driver, instead > want to go 'depend_on' way." > > So one solution seems to be enabling MFD_SYSCON by default, if any of > driver using syscon is enabled. > Can we use some other way here ? > > > >> >> Thanks >> Kishon
Hi, On Mon, Apr 14, 2014 at 7:14 PM, Tomasz Figa <tomasz.figa@gmail.com> wrote: > On 14.04.2014 15:05, Kishon Vijay Abraham I wrote: >> >> >> >> On Monday 14 April 2014 05:35 PM, Vivek Gautam wrote: >>> >>> Hi Kishon, >>> >>> >>> On Mon, Apr 14, 2014 at 5:24 PM, Kishon Vijay Abraham I <kishon@ti.com> >>> wrote: >>>> >>>> Hi, >>>> >>>> On Wednesday 09 April 2014 04:36 PM, Tomasz Figa wrote: >>>>> >>>>> Hi Vivek, >>>>> >>>>> Please see my comments inline. >>>>> >>>>> On 08.04.2014 16:36, Vivek Gautam wrote: >>>>>> >>>>>> Add a new driver for the USB 3.0 PHY on Exynos5 series of SoCs. >>>>>> The new driver uses the generic PHY framework and will interact >>>>>> with DWC3 controller present on Exynos5 series of SoCs. >>>>>> Thereby, removing old phy-samsung-usb3 driver and related code >>>>>> used untill now which was based on usb/phy framework. >>>>>> >>>>>> Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com> >>>>>> --- >>>>>> .../devicetree/bindings/phy/samsung-phy.txt | 42 ++ >>>>>> drivers/phy/Kconfig | 11 + >>>>>> drivers/phy/Makefile | 1 + >>>>>> drivers/phy/phy-exynos5-usbdrd.c | 668 >>>>>> ++++++++++++++++++++ >>>>>> 4 files changed, 722 insertions(+) >>>>>> create mode 100644 drivers/phy/phy-exynos5-usbdrd.c >>>>> >>>>> >>>>> [snip] >>>>> >>>>>> + Additional clock required for Exynos5420: >>>>>> + - usb30_sclk_100m: Additional special clock used for PHY >>>>>> operation >>>>>> + depicted as 'sclk_usbphy30' in CMU of Exynos5420. >>>>> >>>>> >>>>> Are you sure this isn't simply a gate for the ref clock, as it can be >>>>> found on >>>>> another SoC that is not upstream yet? I don't have documentation for >>>>> Exynos >>>>> 5420 so I can't tell, but I'd like to ask you to recheck this. >>>>> >>>>>> +- samsung,syscon-phandle: phandle for syscon interface, which is used >>>>>> to >>>>>> + control pmu registers for power isolation. >>>>>> +- samsung,pmu-offset: phy power control register offset to >>>>>> pmu-system-controller >>>>>> + base. >>>>>> +- #phy-cells : from the generic PHY bindings, must be 1; >>>>>> + >>>>>> +For "samsung,exynos5250-usbdrd-phy" and >>>>>> "samsung,exynos5420-usbdrd-phy" >>>>>> +compatible PHYs, the second cell in the PHY specifier identifies the >>>>>> +PHY id, which is interpreted as follows: >>>>>> + 0 - UTMI+ type phy, >>>>>> + 1 - PIPE3 type phy, >>>>>> + >>>>>> +Example: >>>>>> + usb3_phy: usbphy@12100000 { >>>>>> + compatible = "samsung,exynos5250-usbdrd-phy"; >>>>>> + reg = <0x12100000 0x100>; >>>>>> + clocks = <&clock 286>, <&clock 1>; >>>>>> + clock-names = "phy", "usb3phy_refclk"; >>>>> >>>>> >>>>> Binding description above doesn't mention "usb3phy_refclk" entry. >>>>> >>>>>> + samsung,syscon-phandle = <&pmu_syscon>; >>>>>> + samsung,pmu-offset = <0x704>; >>>>>> + #phy-cells = <1>; >>>>>> + }; >>>>> >>>>> >>>>> [snip] >>>>> >>>>>> diff --git a/drivers/phy/phy-exynos5-usbdrd.c >>>>>> b/drivers/phy/phy-exynos5-usbdrd.c >>>>>> new file mode 100644 >>>>>> index 0000000..ff54a7c >>>>>> --- /dev/null >>>>>> +++ b/drivers/phy/phy-exynos5-usbdrd.c >>>>> >>>>> >>>>> [snip] >>>>> >>>>>> +static int exynos5_usbdrd_phy_probe(struct platform_device *pdev) >>>>>> +{ >>>>>> + struct device *dev = &pdev->dev; >>>>>> + struct device_node *node = dev->of_node; >>>>>> + struct exynos5_usbdrd_phy *phy_drd; >>>>>> + struct phy_provider *phy_provider; >>>>>> + struct resource *res; >>>>>> + const struct of_device_id *match; >>>>>> + const struct exynos5_usbdrd_phy_drvdata *drv_data; >>>>>> + struct regmap *reg_pmu; >>>>>> + u32 pmu_offset; >>>>>> + int i; >>>>>> + >>>>>> + /* >>>>>> + * Exynos systems are completely DT enabled, >>>>>> + * so lets not have any platform data support for this driver. >>>>>> + */ >>>>>> + if (!node) { >>>>>> + dev_err(dev, "no device node found\n"); >>>>> >>>>> >>>>> This error message is not very meaningful. I'd rather use something >>>>> like "This >>>>> driver can be only instantiated using Device Tree". >>>> >>>> >>>> how about just adding depend_on OF in Kconfig? >>> >>> >>> Already added a depend on 'OF'. Copying below the part of Kconfig in this >>> patch. >> >> >> Alright.. Do we need the check then? If config_OF is enabled devices will >> be >> created using device tree no? > > > Not necessarily. Enabling support for OF doesn't mean that it is the only > boot method that can be used. Legacy board files may be still available. I'm > not sure why someone would try to instantiate this driver from them, though. True, we don't have a scope of instantiating this driver using old platform device and old legacy board files. So we don't need this check then, right ?
On 14.04.2014 15:40, Vivek Gautam wrote: > On Mon, Apr 14, 2014 at 6:56 PM, Kishon Vijay Abraham I <kishon@ti.com> wrote: >> >> >> On Monday 14 April 2014 06:50 PM, Vivek Gautam wrote: >>> On Mon, Apr 14, 2014 at 6:29 PM, Kishon Vijay Abraham I <kishon@ti.com> wrote: >>>> >>>> >>>> On Monday 14 April 2014 06:12 PM, Vivek Gautam wrote: >>>>> Hi, >>>>> >>>>> >>>>> On Mon, Apr 14, 2014 at 5:57 PM, Kishon Vijay Abraham I <kishon@ti.com> wrote: >>>>>> Hi, >>>>>> >>>>>> On Tuesday 08 April 2014 08:06 PM, Vivek Gautam wrote: >>>>>>> Add a new driver for the USB 3.0 PHY on Exynos5 series of SoCs. >>>>>>> The new driver uses the generic PHY framework and will interact >>>>>>> with DWC3 controller present on Exynos5 series of SoCs. >>>>>>> Thereby, removing old phy-samsung-usb3 driver and related code >>>>>>> used untill now which was based on usb/phy framework. >>>>>>> >>>>>>> Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com> >>>>>>> --- >>>>>>> .../devicetree/bindings/phy/samsung-phy.txt | 42 ++ >>>>>>> drivers/phy/Kconfig | 11 + >>>>>>> drivers/phy/Makefile | 1 + >>>>>>> drivers/phy/phy-exynos5-usbdrd.c | 668 ++++++++++++++++++++ >>>>>>> 4 files changed, 722 insertions(+) >>>>>>> create mode 100644 drivers/phy/phy-exynos5-usbdrd.c >>>>>>> >>>>>>> diff --git a/Documentation/devicetree/bindings/phy/samsung-phy.txt b/Documentation/devicetree/bindings/phy/samsung-phy.txt >>>>>>> index 28f9edb..6d99ba9 100644 >>>>>>> --- a/Documentation/devicetree/bindings/phy/samsung-phy.txt >>>>>>> +++ b/Documentation/devicetree/bindings/phy/samsung-phy.txt >>>>>>> @@ -74,3 +74,45 @@ phy-consumer@12340000 { >>>>>>> >>>>>>> Refer to DT bindings documentation of particular PHY consumer devices for more >>>>>>> information about required PHYs and the way of specification. >>>>>>> + >>>>>>> +Samsung Exynos5 SoC series USB DRD PHY controller >>>>>>> +-------------------------------------------------- >>>>>>> + >>>>>>> +Required properties: >>>>>>> +- compatible : Should be set to one of the following supported values: >>>>>>> + - "samsung,exynos5250-usbdrd-phy" - for exynos5250 SoC, >>>>>>> + - "samsung,exynos5420-usbdrd-phy" - for exynos5420 SoC. >>>>>>> +- reg : Register offset and length of USB DRD PHY register set; >>>>>>> +- clocks: Clock IDs array as required by the controller >>>>>>> +- clock-names: names of clocks correseponding to IDs in the clock property; >>>>>>> + Required clocks: >>>>>>> + - phy: main PHY clock (same as USB DRD controller i.e. DWC3 IP clock), >>>>>>> + used for register access. >>>>>>> + - ref: PHY's reference clock (usually crystal clock), associated by >>>>>>> + phy name, used to determine bit values for clock settings >>>>>>> + register. >>>>>>> + Additional clock required for Exynos5420: >>>>>>> + - usb30_sclk_100m: Additional special clock used for PHY operation >>>>>>> + depicted as 'sclk_usbphy30' in CMU of Exynos5420. >>>>>>> +- samsung,syscon-phandle: phandle for syscon interface, which is used to >>>>>>> + control pmu registers for power isolation. >>>>>>> +- samsung,pmu-offset: phy power control register offset to pmu-system-controller >>>>>>> + base. >>>>>>> +- #phy-cells : from the generic PHY bindings, must be 1; >>>>>>> + >>>>>>> +For "samsung,exynos5250-usbdrd-phy" and "samsung,exynos5420-usbdrd-phy" >>>>>>> +compatible PHYs, the second cell in the PHY specifier identifies the >>>>>>> +PHY id, which is interpreted as follows: >>>>>>> + 0 - UTMI+ type phy, >>>>>>> + 1 - PIPE3 type phy, >>>>>>> + >>>>>>> +Example: >>>>>>> + usb3_phy: usbphy@12100000 { >>>>>>> + compatible = "samsung,exynos5250-usbdrd-phy"; >>>>>>> + reg = <0x12100000 0x100>; >>>>>>> + clocks = <&clock 286>, <&clock 1>; >>>>>>> + clock-names = "phy", "usb3phy_refclk"; >>>>>>> + samsung,syscon-phandle = <&pmu_syscon>; >>>>>>> + samsung,pmu-offset = <0x704>; >>>>>>> + #phy-cells = <1>; >>>>>>> + }; >>>>>>> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig >>>>>>> index 8d3c49c..d955a05 100644 >>>>>>> --- a/drivers/phy/Kconfig >>>>>>> +++ b/drivers/phy/Kconfig >>>>>>> @@ -166,4 +166,15 @@ config PHY_XGENE >>>>>>> help >>>>>>> This option enables support for APM X-Gene SoC multi-purpose PHY. >>>>>>> >>>>>>> +config PHY_EXYNOS5_USBDRD >>>>>>> + tristate "Exynos5 SoC series USB DRD PHY driver" >>>>>>> + depends on ARCH_EXYNOS5 && OF >>>>>>> + depends on HAS_IOMEM >>>>>>> + select GENERIC_PHY >>>>>>> + select MFD_SYSCON >>>>>> >>>>>> Lets try to avoid select in Kconfig. We've got enough problems with that. >>>>> >>>>> I hope you meant with "select MFD_SYSCON". >>>>> We are referencing the syscon for accessing pmu reg, for which we need >>>>> this config to be selected. >>>>> Other Exynos phy drivers also need this config and for that they have >>>>> selected this. >>>>> >>>>> Do you want me to do it any other way ? >>>> >>>> depends on is one option. >>> >>> Ok, i can see there are places where depends_on MFD_SYSCON is used. >>> "drivers/gpu/drm/exynos/Kconfig:60" >>> >>> so, do you want me to fix at other places too ? >>> >>> But i also have a question here. >>> MFD_SYSCON is a subsystem that's facilitating us in getting our work >>> done here by giving an access to pmu_system_controller. >>> So unless MFD_SYSCON is exposed, the phy driver will not be exposed to us. >>> So is that valid enough really ? >> >> maybe in the Kconfig for MFD_SYSCON, we can select it if PHY_EXYNOS5_USBDRD is >> enabled? >> >> config MFD_SYSCON >> default y if PHY_EXYNOS5_USBDRD > > Yes, that seems to be one option. But we will end up adding this > dependency for all the drivers which want to use syscon. > > Adding few people who worked on MFS_SYSCON, just to gather ideas. > > Re-quoting our problem here: > "Inside our phy driver we are accessing power mgt unit registers, > which we register using syscon in ut device tree. > Now we don't want to select MFD_SYSCON from our phy driver, instead > want to go 'depend_on' way." > > So one solution seems to be enabling MFD_SYSCON by default, if any of > driver using syscon is enabled. > Can we use some other way here ? This is an awful idea from maintainability perspective... This means that every driver depending on MFD_SYSCON would have to add new "default y if" to MFD_SYSCON Kconfig entry. Even with respect to readability, it is more logical to have all dependencies of a driver listed in its Kconfig entry. Anyway, PHY_EXYNOS5_USBDRD can't work without MFD_SYSCON and somehow this needs to be stated. I believe select is the right thing here and I'm not sure why it could be a problem. The biggest reason for going this way is that users wouldn't have to care about internal dependencies, instead they would simply enable drivers for their hardware. Best regards, Tomasz
On 14/04/14 15:49, Vivek Gautam wrote: > True, we don't have a scope of instantiating this driver using old > platform device and > old legacy board files. > So we don't need this check then, right ? I think it can be dropped. At least IMHO there is no point to increase size of the module with an error log that has no chance to be ever called in practice. -- Thanks, Sylwester
On 08/04/14 16:36, Vivek Gautam wrote: > diff --git a/Documentation/devicetree/bindings/phy/samsung-phy.txt b/Documentation/devicetree/bindings/phy/samsung-phy.txt > index 28f9edb..6d99ba9 100644 > --- a/Documentation/devicetree/bindings/phy/samsung-phy.txt > +++ b/Documentation/devicetree/bindings/phy/samsung-phy.txt > @@ -74,3 +74,45 @@ phy-consumer@12340000 { > > Refer to DT bindings documentation of particular PHY consumer devices for more > information about required PHYs and the way of specification. > + > +Samsung Exynos5 SoC series USB DRD PHY controller > +-------------------------------------------------- > + > +Required properties: > +- compatible : Should be set to one of the following supported values: > + - "samsung,exynos5250-usbdrd-phy" - for exynos5250 SoC, > + - "samsung,exynos5420-usbdrd-phy" - for exynos5420 SoC. > +- reg : Register offset and length of USB DRD PHY register set; > +- clocks: Clock IDs array as required by the controller > +- clock-names: names of clocks correseponding to IDs in the clock property; > + Required clocks: > + - phy: main PHY clock (same as USB DRD controller i.e. DWC3 IP clock), > + used for register access. > + - ref: PHY's reference clock (usually crystal clock), associated by > + phy name, used to determine bit values for clock settings > + register. > + Additional clock required for Exynos5420: > + - usb30_sclk_100m: Additional special clock used for PHY operation > + depicted as 'sclk_usbphy30' in CMU of Exynos5420. > +- samsung,syscon-phandle: phandle for syscon interface, which is used to > + control pmu registers for power isolation. Why to append "-phandle" to the property's name ? If this is for PMU perhaps make it more explicit and name it: samsung,pmu-syscon or samsung,pmureg ? > +- samsung,pmu-offset: phy power control register offset to pmu-system-controller > + base. > +- #phy-cells : from the generic PHY bindings, must be 1; > + > +For "samsung,exynos5250-usbdrd-phy" and "samsung,exynos5420-usbdrd-phy" > +compatible PHYs, the second cell in the PHY specifier identifies the > +PHY id, which is interpreted as follows: > + 0 - UTMI+ type phy, > + 1 - PIPE3 type phy,
Hi Sylwester, On Mon, Apr 14, 2014 at 7:51 PM, Sylwester Nawrocki <s.nawrocki@samsung.com> wrote: > On 14/04/14 15:49, Vivek Gautam wrote: >> True, we don't have a scope of instantiating this driver using old >> platform device and >> old legacy board files. >> So we don't need this check then, right ? > > I think it can be dropped. > At least IMHO there is no point to increase size of the module with > an error log that has no chance to be ever called in practice. Ok, i will remove this error message and in fact this check.
Hi, On Mon, Apr 14, 2014 at 8:07 PM, Sylwester Nawrocki <s.nawrocki@samsung.com> wrote: > On 08/04/14 16:36, Vivek Gautam wrote: >> diff --git a/Documentation/devicetree/bindings/phy/samsung-phy.txt b/Documentation/devicetree/bindings/phy/samsung-phy.txt >> index 28f9edb..6d99ba9 100644 >> --- a/Documentation/devicetree/bindings/phy/samsung-phy.txt >> +++ b/Documentation/devicetree/bindings/phy/samsung-phy.txt >> @@ -74,3 +74,45 @@ phy-consumer@12340000 { >> >> Refer to DT bindings documentation of particular PHY consumer devices for more >> information about required PHYs and the way of specification. >> + >> +Samsung Exynos5 SoC series USB DRD PHY controller >> +-------------------------------------------------- >> + >> +Required properties: >> +- compatible : Should be set to one of the following supported values: >> + - "samsung,exynos5250-usbdrd-phy" - for exynos5250 SoC, >> + - "samsung,exynos5420-usbdrd-phy" - for exynos5420 SoC. >> +- reg : Register offset and length of USB DRD PHY register set; >> +- clocks: Clock IDs array as required by the controller >> +- clock-names: names of clocks correseponding to IDs in the clock property; >> + Required clocks: >> + - phy: main PHY clock (same as USB DRD controller i.e. DWC3 IP clock), >> + used for register access. >> + - ref: PHY's reference clock (usually crystal clock), associated by >> + phy name, used to determine bit values for clock settings >> + register. >> + Additional clock required for Exynos5420: >> + - usb30_sclk_100m: Additional special clock used for PHY operation >> + depicted as 'sclk_usbphy30' in CMU of Exynos5420. >> +- samsung,syscon-phandle: phandle for syscon interface, which is used to >> + control pmu registers for power isolation. > > Why to append "-phandle" to the property's name ? If this is for PMU > perhaps make it more explicit and name it: samsung,pmu-syscon or > samsung,pmureg ? Right, thanks for pointing out this. Will rename it to samsung,pmu-syscon. That will be inline with the phandle it points to.
Hi Tomasz, On Thu, Apr 10, 2014 at 5:09 PM, Vivek Gautam <gautamvivek1987@gmail.com> wrote: > On Wed, Apr 9, 2014 at 7:03 PM, Tomasz Figa <t.figa@samsung.com> wrote: >> On 09.04.2014 13:49, Vivek Gautam wrote: >>> >>> Hi, >>> >>> >>> On Wed, Apr 9, 2014 at 4:36 PM, Tomasz Figa <t.figa@samsung.com> wrote: >>>> >>>> Hi Vivek, >>>> >>>> Please see my comments inline. >>>> >>>> >>>> On 08.04.2014 16:36, Vivek Gautam wrote: >>>>> >>>>> >>>>> Add a new driver for the USB 3.0 PHY on Exynos5 series of SoCs. >>>>> The new driver uses the generic PHY framework and will interact >>>>> with DWC3 controller present on Exynos5 series of SoCs. >>>>> Thereby, removing old phy-samsung-usb3 driver and related code >>>>> used untill now which was based on usb/phy framework. >>>>> >>>>> Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com> >>>>> --- >>>>> .../devicetree/bindings/phy/samsung-phy.txt | 42 ++ >>>>> drivers/phy/Kconfig | 11 + >>>>> drivers/phy/Makefile | 1 + >>>>> drivers/phy/phy-exynos5-usbdrd.c | 668 >>>>> ++++++++++++++++++++ >>>>> 4 files changed, 722 insertions(+) >>>>> create mode 100644 drivers/phy/phy-exynos5-usbdrd.c >>>> >>>> >>>> >>>> [snip] >>>> >>>> >>>>> + Additional clock required for Exynos5420: >>>>> + - usb30_sclk_100m: Additional special clock used for PHY >>>>> operation >>>>> + depicted as 'sclk_usbphy30' in CMU of >>>>> Exynos5420. >>>> >>>> >>>> >>>> Are you sure this isn't simply a gate for the ref clock, as it can be >>>> found >>>> on another SoC that is not upstream yet? I don't have documentation for >>>> Exynos 5420 so I can't tell, but I'd like to ask you to recheck this. >>> >>> >>>> From what i can see in the manual : >>> >>> sclk_usbphy30 is derived from OSCCLK. >>> It is coming from a MUX (default input line to this is OSCCLK) and >>> then through a DIV >>> there's this gate. >>> >>> {OSCCLK + other sources} --->[MUX] ---> [DIV] --> [GATE for >>> sclk_usbphy30] >>> >>> the {rate of sclk_usbphy30} == OSCCLK >>> >>> However the 'ref' clock that we have been using is the actual oscillator >>> clock. >>> And on SoC Exynos5250, we don't have any such gate (sclk_usbphy30). >>> So should this mean that ref clock and sclk_usbphy30 are still be >>> controlled by >>> two different gates ? >>> >> >> Is there maybe a diagram of PHY input clocks in the datasheet, like for USB >> 2.0 PHY in Exynos4210/4412/5250 datasheets in the chapter about USB2.0 >> Device? Something like: >> >> ____________________________________ >> | | >> | ___________| >> XusbXTI | Phy_fsel[2:0] | _______ | >> _______[X]_______| | __________|_|___|\__|_| >> | | _v___ | _____ ^ | |/ | | >> _____ | | | | | | | | ___ | | >> ___ | | | | | | | | | |_|_| >> |___| | | X 0 |____|_| PLL |__|_|_|CLK|_|_| >> _____ | | | | | | |DIV|_|_| >> |_______[X] | |_____| 12 |_____|480 | |___| | | >> | MHz MHz |Digital| | >> XusbXTO | USB PHY |_______| | >> |____________________________________| >> >> > > Below is the block diagram given for DRD controller. > > ___________________ > | | > | ____________ | > | | PHY | | > | | controller |-----|----------------------------------------------- > | |__________ | | | > | | > | > | USB 3.0 | V > | DRD | > ----------------------- > | Controller | | > | > | | USB30_SCLK_100M | USB 3.0 DRD | > | ---------------- | -----------------------> > | PHY | > | | Link cont. | | | > | > | ------------- | > | | > |___________________| |_____________| > > Does this help ? > > So, USB30_SCLK_100M is the SCLK that we are talking in the driver. I > don't see any reference to XXTI in the USB 3.0 DRD controller chapter > (in both Exynos5250 and 5420) > In addition to this there's one more point to be noticed here. > On Exynos5420 system, the sclk_usbphy300 (which is the sclk_usbphy30 > for USB DRD channel 0), is also the PICO phy clock, i.e. USB 2.0 phy > clock. > So we should add a similar clk_get() for this clock in the > phy-exynos5250-usb2 driver too, to support Exynos5420. Is something clear from the above block diagram ? (although the diagram looks weird - space and tabs problem :-( ) Basically there's the clock USB30_SCLK_100M which is going into the USB 3.0 DRD PHY controller. And this is the only sclk mentioned in the block diagram for USB 3.0 DRD controller in Exynos5420. Same is not there in the block diagram in Exynos5250 UM.
Hi, On Monday 14 April 2014 07:19 PM, Tomasz Figa wrote: > On 14.04.2014 15:40, Vivek Gautam wrote: >> On Mon, Apr 14, 2014 at 6:56 PM, Kishon Vijay Abraham I <kishon@ti.com> wrote: >>> >>> >>> On Monday 14 April 2014 06:50 PM, Vivek Gautam wrote: >>>> On Mon, Apr 14, 2014 at 6:29 PM, Kishon Vijay Abraham I <kishon@ti.com> wrote: >>>>> >>>>> >>>>> On Monday 14 April 2014 06:12 PM, Vivek Gautam wrote: >>>>>> Hi, >>>>>> >>>>>> >>>>>> On Mon, Apr 14, 2014 at 5:57 PM, Kishon Vijay Abraham I <kishon@ti.com> >>>>>> wrote: >>>>>>> Hi, >>>>>>> >>>>>>> On Tuesday 08 April 2014 08:06 PM, Vivek Gautam wrote: >>>>>>>> Add a new driver for the USB 3.0 PHY on Exynos5 series of SoCs. >>>>>>>> The new driver uses the generic PHY framework and will interact >>>>>>>> with DWC3 controller present on Exynos5 series of SoCs. >>>>>>>> Thereby, removing old phy-samsung-usb3 driver and related code >>>>>>>> used untill now which was based on usb/phy framework. >>>>>>>> >>>>>>>> Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com> >>>>>>>> --- >>>>>>>> .../devicetree/bindings/phy/samsung-phy.txt | 42 ++ >>>>>>>> drivers/phy/Kconfig | 11 + >>>>>>>> drivers/phy/Makefile | 1 + >>>>>>>> drivers/phy/phy-exynos5-usbdrd.c | 668 >>>>>>>> ++++++++++++++++++++ >>>>>>>> 4 files changed, 722 insertions(+) >>>>>>>> create mode 100644 drivers/phy/phy-exynos5-usbdrd.c >>>>>>>> >>>>>>>> diff --git a/Documentation/devicetree/bindings/phy/samsung-phy.txt >>>>>>>> b/Documentation/devicetree/bindings/phy/samsung-phy.txt >>>>>>>> index 28f9edb..6d99ba9 100644 >>>>>>>> --- a/Documentation/devicetree/bindings/phy/samsung-phy.txt >>>>>>>> +++ b/Documentation/devicetree/bindings/phy/samsung-phy.txt >>>>>>>> @@ -74,3 +74,45 @@ phy-consumer@12340000 { >>>>>>>> >>>>>>>> Refer to DT bindings documentation of particular PHY consumer devices >>>>>>>> for more >>>>>>>> information about required PHYs and the way of specification. >>>>>>>> + >>>>>>>> +Samsung Exynos5 SoC series USB DRD PHY controller >>>>>>>> +-------------------------------------------------- >>>>>>>> + >>>>>>>> +Required properties: >>>>>>>> +- compatible : Should be set to one of the following supported values: >>>>>>>> + - "samsung,exynos5250-usbdrd-phy" - for exynos5250 SoC, >>>>>>>> + - "samsung,exynos5420-usbdrd-phy" - for exynos5420 SoC. >>>>>>>> +- reg : Register offset and length of USB DRD PHY register set; >>>>>>>> +- clocks: Clock IDs array as required by the controller >>>>>>>> +- clock-names: names of clocks correseponding to IDs in the clock >>>>>>>> property; >>>>>>>> + Required clocks: >>>>>>>> + - phy: main PHY clock (same as USB DRD controller i.e. DWC3 IP >>>>>>>> clock), >>>>>>>> + used for register access. >>>>>>>> + - ref: PHY's reference clock (usually crystal clock), associated by >>>>>>>> + phy name, used to determine bit values for clock settings >>>>>>>> + register. >>>>>>>> + Additional clock required for Exynos5420: >>>>>>>> + - usb30_sclk_100m: Additional special clock used for PHY operation >>>>>>>> + depicted as 'sclk_usbphy30' in CMU of Exynos5420. >>>>>>>> +- samsung,syscon-phandle: phandle for syscon interface, which is used to >>>>>>>> + control pmu registers for power isolation. >>>>>>>> +- samsung,pmu-offset: phy power control register offset to >>>>>>>> pmu-system-controller >>>>>>>> + base. >>>>>>>> +- #phy-cells : from the generic PHY bindings, must be 1; >>>>>>>> + >>>>>>>> +For "samsung,exynos5250-usbdrd-phy" and "samsung,exynos5420-usbdrd-phy" >>>>>>>> +compatible PHYs, the second cell in the PHY specifier identifies the >>>>>>>> +PHY id, which is interpreted as follows: >>>>>>>> + 0 - UTMI+ type phy, >>>>>>>> + 1 - PIPE3 type phy, >>>>>>>> + >>>>>>>> +Example: >>>>>>>> + usb3_phy: usbphy@12100000 { >>>>>>>> + compatible = "samsung,exynos5250-usbdrd-phy"; >>>>>>>> + reg = <0x12100000 0x100>; >>>>>>>> + clocks = <&clock 286>, <&clock 1>; >>>>>>>> + clock-names = "phy", "usb3phy_refclk"; >>>>>>>> + samsung,syscon-phandle = <&pmu_syscon>; >>>>>>>> + samsung,pmu-offset = <0x704>; >>>>>>>> + #phy-cells = <1>; >>>>>>>> + }; >>>>>>>> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig >>>>>>>> index 8d3c49c..d955a05 100644 >>>>>>>> --- a/drivers/phy/Kconfig >>>>>>>> +++ b/drivers/phy/Kconfig >>>>>>>> @@ -166,4 +166,15 @@ config PHY_XGENE >>>>>>>> help >>>>>>>> This option enables support for APM X-Gene SoC multi-purpose PHY. >>>>>>>> >>>>>>>> +config PHY_EXYNOS5_USBDRD >>>>>>>> + tristate "Exynos5 SoC series USB DRD PHY driver" >>>>>>>> + depends on ARCH_EXYNOS5 && OF >>>>>>>> + depends on HAS_IOMEM >>>>>>>> + select GENERIC_PHY >>>>>>>> + select MFD_SYSCON >>>>>>> >>>>>>> Lets try to avoid select in Kconfig. We've got enough problems with that. >>>>>> >>>>>> I hope you meant with "select MFD_SYSCON". >>>>>> We are referencing the syscon for accessing pmu reg, for which we need >>>>>> this config to be selected. >>>>>> Other Exynos phy drivers also need this config and for that they have >>>>>> selected this. >>>>>> >>>>>> Do you want me to do it any other way ? >>>>> >>>>> depends on is one option. >>>> >>>> Ok, i can see there are places where depends_on MFD_SYSCON is used. >>>> "drivers/gpu/drm/exynos/Kconfig:60" >>>> >>>> so, do you want me to fix at other places too ? >>>> >>>> But i also have a question here. >>>> MFD_SYSCON is a subsystem that's facilitating us in getting our work >>>> done here by giving an access to pmu_system_controller. >>>> So unless MFD_SYSCON is exposed, the phy driver will not be exposed to us. >>>> So is that valid enough really ? >>> >>> maybe in the Kconfig for MFD_SYSCON, we can select it if PHY_EXYNOS5_USBDRD is >>> enabled? >>> >>> config MFD_SYSCON >>> default y if PHY_EXYNOS5_USBDRD >> >> Yes, that seems to be one option. But we will end up adding this >> dependency for all the drivers which want to use syscon. >> >> Adding few people who worked on MFS_SYSCON, just to gather ideas. >> >> Re-quoting our problem here: >> "Inside our phy driver we are accessing power mgt unit registers, >> which we register using syscon in ut device tree. >> Now we don't want to select MFD_SYSCON from our phy driver, instead >> want to go 'depend_on' way." >> >> So one solution seems to be enabling MFD_SYSCON by default, if any of >> driver using syscon is enabled. >> Can we use some other way here ? > > This is an awful idea from maintainability perspective... This means that every > driver depending on MFD_SYSCON would have to add new "default y if" to > MFD_SYSCON Kconfig entry. Even with respect to readability, it is more logical > to have all dependencies of a driver listed in its Kconfig entry. > > Anyway, PHY_EXYNOS5_USBDRD can't work without MFD_SYSCON and somehow this needs > to be stated. I believe select is the right thing here and I'm not sure why it > could be a problem. The biggest reason for going this way is that users 'select' selects a module without caring for the dependencies and introduce compilation warnings. It should always be used with caution. Thanks Kishon
Hi Vivek, On 15.04.2014 08:09, Vivek Gautam wrote: > Hi Tomasz, > > > On Thu, Apr 10, 2014 at 5:09 PM, Vivek Gautam <gautamvivek1987@gmail.com> wrote: >> On Wed, Apr 9, 2014 at 7:03 PM, Tomasz Figa <t.figa@samsung.com> wrote: >>> On 09.04.2014 13:49, Vivek Gautam wrote: >>>> >>>> Hi, >>>> >>>> >>>> On Wed, Apr 9, 2014 at 4:36 PM, Tomasz Figa <t.figa@samsung.com> wrote: >>>>> >>>>> Hi Vivek, >>>>> >>>>> Please see my comments inline. >>>>> >>>>> >>>>> On 08.04.2014 16:36, Vivek Gautam wrote: >>>>>> >>>>>> >>>>>> Add a new driver for the USB 3.0 PHY on Exynos5 series of SoCs. >>>>>> The new driver uses the generic PHY framework and will interact >>>>>> with DWC3 controller present on Exynos5 series of SoCs. >>>>>> Thereby, removing old phy-samsung-usb3 driver and related code >>>>>> used untill now which was based on usb/phy framework. >>>>>> >>>>>> Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com> >>>>>> --- >>>>>> .../devicetree/bindings/phy/samsung-phy.txt | 42 ++ >>>>>> drivers/phy/Kconfig | 11 + >>>>>> drivers/phy/Makefile | 1 + >>>>>> drivers/phy/phy-exynos5-usbdrd.c | 668 >>>>>> ++++++++++++++++++++ >>>>>> 4 files changed, 722 insertions(+) >>>>>> create mode 100644 drivers/phy/phy-exynos5-usbdrd.c >>>>> >>>>> >>>>> >>>>> [snip] >>>>> >>>>> >>>>>> + Additional clock required for Exynos5420: >>>>>> + - usb30_sclk_100m: Additional special clock used for PHY >>>>>> operation >>>>>> + depicted as 'sclk_usbphy30' in CMU of >>>>>> Exynos5420. >>>>> >>>>> >>>>> >>>>> Are you sure this isn't simply a gate for the ref clock, as it can be >>>>> found >>>>> on another SoC that is not upstream yet? I don't have documentation for >>>>> Exynos 5420 so I can't tell, but I'd like to ask you to recheck this. >>>> >>>> >>>>> From what i can see in the manual : >>>> >>>> sclk_usbphy30 is derived from OSCCLK. >>>> It is coming from a MUX (default input line to this is OSCCLK) and >>>> then through a DIV >>>> there's this gate. >>>> >>>> {OSCCLK + other sources} --->[MUX] ---> [DIV] --> [GATE for >>>> sclk_usbphy30] >>>> >>>> the {rate of sclk_usbphy30} == OSCCLK >>>> >>>> However the 'ref' clock that we have been using is the actual oscillator >>>> clock. >>>> And on SoC Exynos5250, we don't have any such gate (sclk_usbphy30). >>>> So should this mean that ref clock and sclk_usbphy30 are still be >>>> controlled by >>>> two different gates ? >>>> >>> >>> Is there maybe a diagram of PHY input clocks in the datasheet, like for USB >>> 2.0 PHY in Exynos4210/4412/5250 datasheets in the chapter about USB2.0 >>> Device? Something like: >>> >>> ____________________________________ >>> | | >>> | ___________| >>> XusbXTI | Phy_fsel[2:0] | _______ | >>> _______[X]_______| | __________|_|___|\__|_| >>> | | _v___ | _____ ^ | |/ | | >>> _____ | | | | | | | | ___ | | >>> ___ | | | | | | | | | |_|_| >>> |___| | | X 0 |____|_| PLL |__|_|_|CLK|_|_| >>> _____ | | | | | | |DIV|_|_| >>> |_______[X] | |_____| 12 |_____|480 | |___| | | >>> | MHz MHz |Digital| | >>> XusbXTO | USB PHY |_______| | >>> |____________________________________| >>> >>> >> >> Below is the block diagram given for DRD controller. >> >> ___________________ >> | | >> | ____________ | >> | | PHY | | >> | | controller |-----|----------------------------------------------- >> | |__________ | | | >> | | >> | >> | USB 3.0 | V >> | DRD | >> ----------------------- >> | Controller | | >> | >> | | USB30_SCLK_100M | USB 3.0 DRD | >> | ---------------- | -----------------------> >> | PHY | >> | | Link cont. | | | >> | >> | ------------- | >> | | >> |___________________| |_____________| >> >> Does this help ? >> >> So, USB30_SCLK_100M is the SCLK that we are talking in the driver. I >> don't see any reference to XXTI in the USB 3.0 DRD controller chapter >> (in both Exynos5250 and 5420) >> In addition to this there's one more point to be noticed here. >> On Exynos5420 system, the sclk_usbphy300 (which is the sclk_usbphy30 >> for USB DRD channel 0), is also the PICO phy clock, i.e. USB 2.0 phy >> clock. >> So we should add a similar clk_get() for this clock in the >> phy-exynos5250-usb2 driver too, to support Exynos5420. > > Is something clear from the above block diagram ? (although the > diagram looks weird - space and tabs problem :-( ) > Basically there's the clock USB30_SCLK_100M which is going into the > USB 3.0 DRD PHY controller. > And this is the only sclk mentioned in the block diagram for USB 3.0 > DRD controller in Exynos5420. > Same is not there in the block diagram in Exynos5250 UM. From what I can see in the documentation, there are 4 USB 3.0 related clocks generated in CMU: - sclk_usbphy300, - sclk_usbphy301, - sclk_usbdrd300, - sclk_usbdrd301, They are all rated to max. 24 MHz and the recommended operating frequency is 24 MHz, so it looks exactly like USB PHY reference, which is usually a 24 MHz clock. To me, this looks like on Exynos5420 a separate special clock path is used instead of xusbxti as reference of USB 3.0 PHY and so the sclk should be simply passed as the "ref" clock. Best regards, Tomasz
Hi, On Wed, Apr 16, 2014 at 7:14 PM, Tomasz Figa <t.figa@samsung.com> wrote: > Hi Vivek, > > > On 15.04.2014 08:09, Vivek Gautam wrote: >> >> Hi Tomasz, >> >> >> On Thu, Apr 10, 2014 at 5:09 PM, Vivek Gautam <gautamvivek1987@gmail.com> >> wrote: >>> >>> On Wed, Apr 9, 2014 at 7:03 PM, Tomasz Figa <t.figa@samsung.com> wrote: >>>> >>>> On 09.04.2014 13:49, Vivek Gautam wrote: >>>>> >>>>> >>>>> Hi, >>>>> >>>>> >>>>> On Wed, Apr 9, 2014 at 4:36 PM, Tomasz Figa <t.figa@samsung.com> wrote: >>>>>> >>>>>> >>>>>> Hi Vivek, >>>>>> >>>>>> Please see my comments inline. >>>>>> >>>>>> >>>>>> On 08.04.2014 16:36, Vivek Gautam wrote: >>>>>>> >>>>>>> >>>>>>> >>>>>>> Add a new driver for the USB 3.0 PHY on Exynos5 series of SoCs. >>>>>>> The new driver uses the generic PHY framework and will interact >>>>>>> with DWC3 controller present on Exynos5 series of SoCs. >>>>>>> Thereby, removing old phy-samsung-usb3 driver and related code >>>>>>> used untill now which was based on usb/phy framework. >>>>>>> >>>>>>> Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com> >>>>>>> --- >>>>>>> .../devicetree/bindings/phy/samsung-phy.txt | 42 ++ >>>>>>> drivers/phy/Kconfig | 11 + >>>>>>> drivers/phy/Makefile | 1 + >>>>>>> drivers/phy/phy-exynos5-usbdrd.c | 668 >>>>>>> ++++++++++++++++++++ >>>>>>> 4 files changed, 722 insertions(+) >>>>>>> create mode 100644 drivers/phy/phy-exynos5-usbdrd.c >>>>>> >>>>>> >>>>>> >>>>>> >>>>>> [snip] >>>>>> >>>>>> >>>>>>> + Additional clock required for Exynos5420: >>>>>>> + - usb30_sclk_100m: Additional special clock used for PHY >>>>>>> operation >>>>>>> + depicted as 'sclk_usbphy30' in CMU of >>>>>>> Exynos5420. >>>>>> >>>>>> >>>>>> >>>>>> >>>>>> Are you sure this isn't simply a gate for the ref clock, as it can be >>>>>> found >>>>>> on another SoC that is not upstream yet? I don't have documentation >>>>>> for >>>>>> Exynos 5420 so I can't tell, but I'd like to ask you to recheck this. >>>>> >>>>> >>>>> >>>>>> From what i can see in the manual : >>>>> >>>>> >>>>> sclk_usbphy30 is derived from OSCCLK. >>>>> It is coming from a MUX (default input line to this is OSCCLK) and >>>>> then through a DIV >>>>> there's this gate. >>>>> >>>>> {OSCCLK + other sources} --->[MUX] ---> [DIV] --> [GATE for >>>>> sclk_usbphy30] >>>>> >>>>> the {rate of sclk_usbphy30} == OSCCLK >>>>> >>>>> However the 'ref' clock that we have been using is the actual >>>>> oscillator >>>>> clock. >>>>> And on SoC Exynos5250, we don't have any such gate (sclk_usbphy30). >>>>> So should this mean that ref clock and sclk_usbphy30 are still be >>>>> controlled by >>>>> two different gates ? >>>>> >>>> >>>> Is there maybe a diagram of PHY input clocks in the datasheet, like for >>>> USB >>>> 2.0 PHY in Exynos4210/4412/5250 datasheets in the chapter about USB2.0 >>>> Device? Something like: >>>> >>>> ____________________________________ >>>> | | >>>> | ___________| >>>> XusbXTI | Phy_fsel[2:0] | _______ | >>>> _______[X]_______| | __________|_|___|\__|_| >>>> | | _v___ | _____ ^ | |/ | | >>>> _____ | | | | | | | | ___ | | >>>> ___ | | | | | | | | | |_|_| >>>> |___| | | X 0 |____|_| PLL |__|_|_|CLK|_|_| >>>> _____ | | | | | | |DIV|_|_| >>>> |_______[X] | |_____| 12 |_____|480 | |___| | | >>>> | MHz MHz |Digital| | >>>> XusbXTO | USB PHY |_______| | >>>> |____________________________________| >>>> >>>> >>> >>> Below is the block diagram given for DRD controller. >>> >>> ___________________ >>> | | >>> | ____________ | >>> | | PHY | | >>> | | controller >>> |-----|----------------------------------------------- >>> | |__________ | | >>> | >>> | | >>> | >>> | USB 3.0 | >>> V >>> | DRD | >>> ----------------------- >>> | Controller | | >>> | >>> | | USB30_SCLK_100M | USB 3.0 DRD | >>> | ---------------- | -----------------------> >>> | PHY | >>> | | Link cont. | | | >>> | >>> | ------------- | >>> | | >>> |___________________| |_____________| >>> >>> Does this help ? >>> >>> So, USB30_SCLK_100M is the SCLK that we are talking in the driver. I >>> don't see any reference to XXTI in the USB 3.0 DRD controller chapter >>> (in both Exynos5250 and 5420) >>> In addition to this there's one more point to be noticed here. >>> On Exynos5420 system, the sclk_usbphy300 (which is the sclk_usbphy30 >>> for USB DRD channel 0), is also the PICO phy clock, i.e. USB 2.0 phy >>> clock. >>> So we should add a similar clk_get() for this clock in the >>> phy-exynos5250-usb2 driver too, to support Exynos5420. >> >> >> Is something clear from the above block diagram ? (although the >> diagram looks weird - space and tabs problem :-( ) >> Basically there's the clock USB30_SCLK_100M which is going into the >> USB 3.0 DRD PHY controller. >> And this is the only sclk mentioned in the block diagram for USB 3.0 >> DRD controller in Exynos5420. >> Same is not there in the block diagram in Exynos5250 UM. > > > From what I can see in the documentation, there are 4 USB 3.0 related clocks > generated in CMU: > > - sclk_usbphy300, > - sclk_usbphy301, > - sclk_usbdrd300, > - sclk_usbdrd301, > > They are all rated to max. 24 MHz and the recommended operating frequency is > 24 MHz, so it looks exactly like USB PHY reference, which is usually a 24 > MHz clock. > > To me, this looks like on Exynos5420 a separate special clock path is used > instead of xusbxti as reference of USB 3.0 PHY and so the sclk should be > simply passed as the "ref" clock. Ok, i will clear on this with the hardware engineer also once. May be Jingoo can help me with this. Jingoo, Can you please enquire about the clock path of usbphy30 reference clocks on Exynos5420. As mentioned by Tomasz above, we have sclk_usbphy300 and sclk_usbphy301 as the reference clocks for USB3.0 DRD phy. *Also* sclk_usbphy300 is used for Pico phy (which is the USb 2.0 phy used by ehci/ohci controller on Exynos5420). It will be of great help. Thanks Vivek
On Wednesday, April 16, 2014 11:49 PM, Vivek Gautam wrote: > On Wed, Apr 16, 2014 at 7:14 PM, Tomasz Figa <t.figa@samsung.com> wrote: > > On 15.04.2014 08:09, Vivek Gautam wrote: > >> On Thu, Apr 10, 2014 at 5:09 PM, Vivek Gautam wrote: > >>> On Wed, Apr 9, 2014 at 7:03 PM, Tomasz Figa <t.figa@samsung.com> wrote: > >>>> On 09.04.2014 13:49, Vivek Gautam wrote: > >>> > >>> So, USB30_SCLK_100M is the SCLK that we are talking in the driver. I > >>> don't see any reference to XXTI in the USB 3.0 DRD controller chapter > >>> (in both Exynos5250 and 5420) > >>> In addition to this there's one more point to be noticed here. > >>> On Exynos5420 system, the sclk_usbphy300 (which is the sclk_usbphy30 > >>> for USB DRD channel 0), is also the PICO phy clock, i.e. USB 2.0 phy > >>> clock. > >>> So we should add a similar clk_get() for this clock in the > >>> phy-exynos5250-usb2 driver too, to support Exynos5420. > >> > >> > >> Is something clear from the above block diagram ? (although the > >> diagram looks weird - space and tabs problem :-( ) > >> Basically there's the clock USB30_SCLK_100M which is going into the > >> USB 3.0 DRD PHY controller. > >> And this is the only sclk mentioned in the block diagram for USB 3.0 > >> DRD controller in Exynos5420. > >> Same is not there in the block diagram in Exynos5250 UM. > > > > > > From what I can see in the documentation, there are 4 USB 3.0 related clocks > > generated in CMU: > > > > - sclk_usbphy300, > > - sclk_usbphy301, > > - sclk_usbdrd300, > > - sclk_usbdrd301, > > > > They are all rated to max. 24 MHz and the recommended operating frequency is > > 24 MHz, so it looks exactly like USB PHY reference, which is usually a 24 > > MHz clock. > > > > To me, this looks like on Exynos5420 a separate special clock path is used > > instead of xusbxti as reference of USB 3.0 PHY and so the sclk should be > > simply passed as the "ref" clock. > > Ok, i will clear on this with the hardware engineer also once. > May be Jingoo can help me with this. > > Jingoo, > Can you please enquire about the clock path of usbphy30 reference > clocks on Exynos5420. > As mentioned by Tomasz above, we have sclk_usbphy300 and > sclk_usbphy301 as the reference clocks for USB3.0 DRD phy. *Also* > sclk_usbphy300 is used for Pico phy (which is the USb 2.0 phy used by > ehci/ohci controller on Exynos5420). > It will be of great help. Hi Vevek, Tomasz Long time no see. I asked USB S/W engineer and USB H/W engineer. There are two USB3.0 on Exynos5420; thus there are two sclks such as 'sclk_usbphy300 and sclk_usbphy301'. As Tomasz mentioned, 'sclk_usbphy300 and sclk_usbphy301' can be used instead of 'xusbxti' as reference of USB 3.0 PHY. However, on Exynos5420, "ONLY" 'sclk_usbphy300' can be used for USB2.0 pico phy. (so, '301' CANNOT support USB2.0 pico phy.) Best regards, Jingoo Han
Hi Jingoo, On Tue, Apr 22, 2014 at 7:48 AM, Jingoo Han <jg1.han@samsung.com> wrote: > On Wednesday, April 16, 2014 11:49 PM, Vivek Gautam wrote: >> On Wed, Apr 16, 2014 at 7:14 PM, Tomasz Figa <t.figa@samsung.com> wrote: >> > On 15.04.2014 08:09, Vivek Gautam wrote: >> >> On Thu, Apr 10, 2014 at 5:09 PM, Vivek Gautam wrote: >> >>> On Wed, Apr 9, 2014 at 7:03 PM, Tomasz Figa <t.figa@samsung.com> wrote: >> >>>> On 09.04.2014 13:49, Vivek Gautam wrote: >> >>> >> >>> So, USB30_SCLK_100M is the SCLK that we are talking in the driver. I >> >>> don't see any reference to XXTI in the USB 3.0 DRD controller chapter >> >>> (in both Exynos5250 and 5420) >> >>> In addition to this there's one more point to be noticed here. >> >>> On Exynos5420 system, the sclk_usbphy300 (which is the sclk_usbphy30 >> >>> for USB DRD channel 0), is also the PICO phy clock, i.e. USB 2.0 phy >> >>> clock. >> >>> So we should add a similar clk_get() for this clock in the >> >>> phy-exynos5250-usb2 driver too, to support Exynos5420. >> >> >> >> >> >> Is something clear from the above block diagram ? (although the >> >> diagram looks weird - space and tabs problem :-( ) >> >> Basically there's the clock USB30_SCLK_100M which is going into the >> >> USB 3.0 DRD PHY controller. >> >> And this is the only sclk mentioned in the block diagram for USB 3.0 >> >> DRD controller in Exynos5420. >> >> Same is not there in the block diagram in Exynos5250 UM. >> > >> > >> > From what I can see in the documentation, there are 4 USB 3.0 related clocks >> > generated in CMU: >> > >> > - sclk_usbphy300, >> > - sclk_usbphy301, >> > - sclk_usbdrd300, >> > - sclk_usbdrd301, >> > >> > They are all rated to max. 24 MHz and the recommended operating frequency is >> > 24 MHz, so it looks exactly like USB PHY reference, which is usually a 24 >> > MHz clock. >> > >> > To me, this looks like on Exynos5420 a separate special clock path is used >> > instead of xusbxti as reference of USB 3.0 PHY and so the sclk should be >> > simply passed as the "ref" clock. >> >> Ok, i will clear on this with the hardware engineer also once. >> May be Jingoo can help me with this. >> >> Jingoo, >> Can you please enquire about the clock path of usbphy30 reference >> clocks on Exynos5420. >> As mentioned by Tomasz above, we have sclk_usbphy300 and >> sclk_usbphy301 as the reference clocks for USB3.0 DRD phy. *Also* >> sclk_usbphy300 is used for Pico phy (which is the USb 2.0 phy used by >> ehci/ohci controller on Exynos5420). >> It will be of great help. > > Hi Vevek, Tomasz > > Long time no see. > > I asked USB S/W engineer and USB H/W engineer. > > There are two USB3.0 on Exynos5420; thus there are two sclks > such as 'sclk_usbphy300 and sclk_usbphy301'. > > As Tomasz mentioned, 'sclk_usbphy300 and sclk_usbphy301' can > be used instead of 'xusbxti' as reference of USB 3.0 PHY. Thank you so much for getting this information. I can re-spin the patch. :-) > > However, on Exynos5420, "ONLY" 'sclk_usbphy300' can be used > for USB2.0 pico phy. (so, '301' CANNOT support USB2.0 pico phy.) True, for USB 2.0 pico phy, only sclk_usbphy300 can be used.
On 04/14/2014 08:07 PM, Sylwester Nawrocki wrote: > On 08/04/14 16:36, Vivek Gautam wrote: >> diff --git a/Documentation/devicetree/bindings/phy/samsung-phy.txt b/Documentation/devicetree/bindings/phy/samsung-phy.txt >> index 28f9edb..6d99ba9 100644 >> --- a/Documentation/devicetree/bindings/phy/samsung-phy.txt >> +++ b/Documentation/devicetree/bindings/phy/samsung-phy.txt >> @@ -74,3 +74,45 @@ phy-consumer@12340000 { >> >> Refer to DT bindings documentation of particular PHY consumer devices for more >> information about required PHYs and the way of specification. >> + >> +Samsung Exynos5 SoC series USB DRD PHY controller >> +-------------------------------------------------- >> + >> +Required properties: >> +- compatible : Should be set to one of the following supported values: >> + - "samsung,exynos5250-usbdrd-phy" - for exynos5250 SoC, >> + - "samsung,exynos5420-usbdrd-phy" - for exynos5420 SoC. >> +- reg : Register offset and length of USB DRD PHY register set; >> +- clocks: Clock IDs array as required by the controller >> +- clock-names: names of clocks correseponding to IDs in the clock property; >> + Required clocks: >> + - phy: main PHY clock (same as USB DRD controller i.e. DWC3 IP clock), >> + used for register access. >> + - ref: PHY's reference clock (usually crystal clock), associated by >> + phy name, used to determine bit values for clock settings >> + register. >> + Additional clock required for Exynos5420: >> + - usb30_sclk_100m: Additional special clock used for PHY operation >> + depicted as 'sclk_usbphy30' in CMU of Exynos5420. >> +- samsung,syscon-phandle: phandle for syscon interface, which is used to >> + control pmu registers for power isolation. > > Why to append "-phandle" to the property's name ? If this is for PMU > perhaps make it more explicit and name it: samsung,pmu-syscon or > samsung,pmureg ? > There are already a couple of nodes (watchdog and sata) using samsung,syscon-phandle. IMHO, we should keep only property string for syscon node. Either we keep syscon-phandle here or change sata/watchdog driver to use the modified property name.
Hi, On Fri, Apr 25, 2014 at 1:27 PM, Tushar Behera <tushar.behera@linaro.org> wrote: > On 04/14/2014 08:07 PM, Sylwester Nawrocki wrote: >> On 08/04/14 16:36, Vivek Gautam wrote: >>> diff --git a/Documentation/devicetree/bindings/phy/samsung-phy.txt b/Documentation/devicetree/bindings/phy/samsung-phy.txt >>> index 28f9edb..6d99ba9 100644 >>> --- a/Documentation/devicetree/bindings/phy/samsung-phy.txt >>> +++ b/Documentation/devicetree/bindings/phy/samsung-phy.txt >>> @@ -74,3 +74,45 @@ phy-consumer@12340000 { >>> >>> Refer to DT bindings documentation of particular PHY consumer devices for more >>> information about required PHYs and the way of specification. >>> + >>> +Samsung Exynos5 SoC series USB DRD PHY controller >>> +-------------------------------------------------- >>> + >>> +Required properties: >>> +- compatible : Should be set to one of the following supported values: >>> + - "samsung,exynos5250-usbdrd-phy" - for exynos5250 SoC, >>> + - "samsung,exynos5420-usbdrd-phy" - for exynos5420 SoC. >>> +- reg : Register offset and length of USB DRD PHY register set; >>> +- clocks: Clock IDs array as required by the controller >>> +- clock-names: names of clocks correseponding to IDs in the clock property; >>> + Required clocks: >>> + - phy: main PHY clock (same as USB DRD controller i.e. DWC3 IP clock), >>> + used for register access. >>> + - ref: PHY's reference clock (usually crystal clock), associated by >>> + phy name, used to determine bit values for clock settings >>> + register. >>> + Additional clock required for Exynos5420: >>> + - usb30_sclk_100m: Additional special clock used for PHY operation >>> + depicted as 'sclk_usbphy30' in CMU of Exynos5420. >>> +- samsung,syscon-phandle: phandle for syscon interface, which is used to >>> + control pmu registers for power isolation. >> >> Why to append "-phandle" to the property's name ? If this is for PMU >> perhaps make it more explicit and name it: samsung,pmu-syscon or >> samsung,pmureg ? >> > > There are already a couple of nodes (watchdog and sata) using > samsung,syscon-phandle. IMHO, we should keep only property string for > syscon node. Either we keep syscon-phandle here or change sata/watchdog > driver to use the modified property name. IMHO samsung,pmu-syscon make more sense rather than appending a '-phandle' to the property name. This is a 'phandle' and that is in fact understood, isn't it ? We can change in the watchdog, sata drivers to use use the modified name. I can send a patch for the same if we are OK with this, so that we maintain the consistency in the device tree.
diff --git a/Documentation/devicetree/bindings/phy/samsung-phy.txt b/Documentation/devicetree/bindings/phy/samsung-phy.txt index 28f9edb..6d99ba9 100644 --- a/Documentation/devicetree/bindings/phy/samsung-phy.txt +++ b/Documentation/devicetree/bindings/phy/samsung-phy.txt @@ -74,3 +74,45 @@ phy-consumer@12340000 { Refer to DT bindings documentation of particular PHY consumer devices for more information about required PHYs and the way of specification. + +Samsung Exynos5 SoC series USB DRD PHY controller +-------------------------------------------------- + +Required properties: +- compatible : Should be set to one of the following supported values: + - "samsung,exynos5250-usbdrd-phy" - for exynos5250 SoC, + - "samsung,exynos5420-usbdrd-phy" - for exynos5420 SoC. +- reg : Register offset and length of USB DRD PHY register set; +- clocks: Clock IDs array as required by the controller +- clock-names: names of clocks correseponding to IDs in the clock property; + Required clocks: + - phy: main PHY clock (same as USB DRD controller i.e. DWC3 IP clock), + used for register access. + - ref: PHY's reference clock (usually crystal clock), associated by + phy name, used to determine bit values for clock settings + register. + Additional clock required for Exynos5420: + - usb30_sclk_100m: Additional special clock used for PHY operation + depicted as 'sclk_usbphy30' in CMU of Exynos5420. +- samsung,syscon-phandle: phandle for syscon interface, which is used to + control pmu registers for power isolation. +- samsung,pmu-offset: phy power control register offset to pmu-system-controller + base. +- #phy-cells : from the generic PHY bindings, must be 1; + +For "samsung,exynos5250-usbdrd-phy" and "samsung,exynos5420-usbdrd-phy" +compatible PHYs, the second cell in the PHY specifier identifies the +PHY id, which is interpreted as follows: + 0 - UTMI+ type phy, + 1 - PIPE3 type phy, + +Example: + usb3_phy: usbphy@12100000 { + compatible = "samsung,exynos5250-usbdrd-phy"; + reg = <0x12100000 0x100>; + clocks = <&clock 286>, <&clock 1>; + clock-names = "phy", "usb3phy_refclk"; + samsung,syscon-phandle = <&pmu_syscon>; + samsung,pmu-offset = <0x704>; + #phy-cells = <1>; + }; diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig index 8d3c49c..d955a05 100644 --- a/drivers/phy/Kconfig +++ b/drivers/phy/Kconfig @@ -166,4 +166,15 @@ config PHY_XGENE help This option enables support for APM X-Gene SoC multi-purpose PHY. +config PHY_EXYNOS5_USBDRD + tristate "Exynos5 SoC series USB DRD PHY driver" + depends on ARCH_EXYNOS5 && OF + depends on HAS_IOMEM + select GENERIC_PHY + select MFD_SYSCON + help + Enable USB DRD PHY support for Exynos 5 SoC series. + This driver provides PHY interface for USB 3.0 DRD controller + present on Exynos5 SoC series. + endmenu diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile index 2faf78e..31baa0c 100644 --- a/drivers/phy/Makefile +++ b/drivers/phy/Makefile @@ -18,3 +18,4 @@ obj-$(CONFIG_PHY_EXYNOS4210_USB2) += phy-exynos4210-usb2.o obj-$(CONFIG_PHY_EXYNOS4X12_USB2) += phy-exynos4x12-usb2.o obj-$(CONFIG_PHY_EXYNOS5250_USB2) += phy-exynos5250-usb2.o obj-$(CONFIG_PHY_XGENE) += phy-xgene.o +obj-$(CONFIG_PHY_EXYNOS5_USBDRD) += phy-exynos5-usbdrd.o diff --git a/drivers/phy/phy-exynos5-usbdrd.c b/drivers/phy/phy-exynos5-usbdrd.c new file mode 100644 index 0000000..ff54a7c --- /dev/null +++ b/drivers/phy/phy-exynos5-usbdrd.c @@ -0,0 +1,668 @@ +/* + * Samsung EXYNOS5 SoC series USB DRD PHY driver + * + * Phy provider for USB 3.0 DRD controller on Exynos5 SoC series + * + * Copyright (C) 2013 Samsung Electronics Co., Ltd. + * Author: Vivek Gautam <gautam.vivek@samsung.com> + * + * 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 + * published by the Free Software Foundation. + */ + +#include <linux/clk.h> +#include <linux/delay.h> +#include <linux/io.h> +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/of.h> +#include <linux/of_address.h> +#include <linux/phy/phy.h> +#include <linux/platform_device.h> +#include <linux/mutex.h> +#include <linux/mfd/syscon.h> +#include <linux/regmap.h> + +/* Exynos USB PHY registers */ +#define EXYNOS5_FSEL_9MHZ6 0x0 +#define EXYNOS5_FSEL_10MHZ 0x1 +#define EXYNOS5_FSEL_12MHZ 0x2 +#define EXYNOS5_FSEL_19MHZ2 0x3 +#define EXYNOS5_FSEL_20MHZ 0x4 +#define EXYNOS5_FSEL_24MHZ 0x5 +#define EXYNOS5_FSEL_50MHZ 0x7 + +/* EXYNOS5: USB 3.0 DRD PHY registers */ +#define EXYNOS5_DRD_LINKSYSTEM 0x04 + +#define LINKSYSTEM_FLADJ_MASK (0x3f << 1) +#define LINKSYSTEM_FLADJ(_x) ((_x) << 1) +#define LINKSYSTEM_XHCI_VERSION_CONTROL BIT(27) + +#define EXYNOS5_DRD_PHYUTMI 0x08 + +#define PHYUTMI_OTGDISABLE BIT(6) +#define PHYUTMI_FORCESUSPEND BIT(1) +#define PHYUTMI_FORCESLEEP BIT(0) + +#define EXYNOS5_DRD_PHYPIPE 0x0c + +#define EXYNOS5_DRD_PHYCLKRST 0x10 + +#define PHYCLKRST_EN_UTMISUSPEND BIT(31) + +#define PHYCLKRST_SSC_REFCLKSEL_MASK (0xff << 23) +#define PHYCLKRST_SSC_REFCLKSEL(_x) ((_x) << 23) + +#define PHYCLKRST_SSC_RANGE_MASK (0x03 << 21) +#define PHYCLKRST_SSC_RANGE(_x) ((_x) << 21) + +#define PHYCLKRST_SSC_EN BIT(20) +#define PHYCLKRST_REF_SSP_EN BIT(19) +#define PHYCLKRST_REF_CLKDIV2 BIT(18) + +#define PHYCLKRST_MPLL_MULTIPLIER_MASK (0x7f << 11) +#define PHYCLKRST_MPLL_MULTIPLIER_100MHZ_REF (0x19 << 11) +#define PHYCLKRST_MPLL_MULTIPLIER_50M_REF (0x32 << 11) +#define PHYCLKRST_MPLL_MULTIPLIER_24MHZ_REF (0x68 << 11) +#define PHYCLKRST_MPLL_MULTIPLIER_20MHZ_REF (0x7d << 11) +#define PHYCLKRST_MPLL_MULTIPLIER_19200KHZ_REF (0x02 << 11) + +#define PHYCLKRST_FSEL_UTMI_MASK (0x7 << 5) +#define PHYCLKRST_FSEL_PIPE_MASK (0x7 << 8) +#define PHYCLKRST_FSEL(_x) ((_x) << 5) +#define PHYCLKRST_FSEL_PAD_100MHZ (0x27 << 5) +#define PHYCLKRST_FSEL_PAD_24MHZ (0x2a << 5) +#define PHYCLKRST_FSEL_PAD_20MHZ (0x31 << 5) +#define PHYCLKRST_FSEL_PAD_19_2MHZ (0x38 << 5) + +#define PHYCLKRST_RETENABLEN BIT(4) + +#define PHYCLKRST_REFCLKSEL_MASK (0x03 << 2) +#define PHYCLKRST_REFCLKSEL_PAD_REFCLK (0x2 << 2) +#define PHYCLKRST_REFCLKSEL_EXT_REFCLK (0x3 << 2) + +#define PHYCLKRST_PORTRESET BIT(1) +#define PHYCLKRST_COMMONONN BIT(0) + +#define EXYNOS5_DRD_PHYREG0 0x14 +#define EXYNOS5_DRD_PHYREG1 0x18 + +#define EXYNOS5_DRD_PHYPARAM0 0x1c + +#define PHYPARAM0_REF_USE_PAD BIT(31) +#define PHYPARAM0_REF_LOSLEVEL_MASK (0x1f << 26) +#define PHYPARAM0_REF_LOSLEVEL (0x9 << 26) + +#define EXYNOS5_DRD_PHYPARAM1 0x20 + +#define PHYPARAM1_PCS_TXDEEMPH_MASK (0x1f << 0) +#define PHYPARAM1_PCS_TXDEEMPH (0x1c) + +#define EXYNOS5_DRD_PHYTERM 0x24 + +#define EXYNOS5_DRD_PHYTEST 0x28 + +#define PHYTEST_POWERDOWN_SSP BIT(3) +#define PHYTEST_POWERDOWN_HSP BIT(2) + +#define EXYNOS5_DRD_PHYADP 0x2c + +#define EXYNOS5_DRD_PHYUTMICLKSEL 0x30 + +#define PHYUTMICLKSEL_UTMI_CLKSEL BIT(2) + +#define EXYNOS5_DRD_PHYRESUME 0x34 +#define EXYNOS5_DRD_LINKPORT 0x44 + +/* Power isolation defined in power management unit */ +#define EXYNOS5_USBDRD_PMU_ISOL BIT(0) + +#define KHZ 1000 +#define MHZ (KHZ * KHZ) + +enum exynos5_usbdrd_phy_id { + EXYNOS5_DRDPHY_UTMI, + EXYNOS5_DRDPHY_PIPE3, + EXYNOS5_DRDPHYS_NUM, +}; + +struct phy_usb_instance; +struct exynos5_usbdrd_phy; + +struct usbdrd_phy_config { + u32 id; + void (*phy_isol)(struct phy_usb_instance *inst, u32 on); + void (*phy_init)(struct exynos5_usbdrd_phy *phy_drd); + unsigned int (*set_refclk)(struct phy_usb_instance *inst); +}; + +struct exynos5_usbdrd_phy_drvdata { + bool has_usb30_sclk; + const struct usbdrd_phy_config *phy_cfg; +}; + +/** + * struct exynos5_usbdrd_phy - driver data for USB 3.0 PHY + * @dev: pointer to device instance of this platform device + * @reg_phy: usb phy controller register memory base + * @clk: phy clock for register access + * @usb30_sclk: additional special clock for phy operations + * @drv_data: pointer to SoC level driver data structure + * @phys[]: array for 'EXYNOS5_DRDPHYS_NUM' number of PHY + * instances each with its 'phy' and 'phy_cfg'. + * @extrefclk: frequency select settings when using 'separate + * reference clocks' for SS and HS operations + * @ref_clk: reference clock to PHY block from which PHY's + * operational clocks are derived + * @ref_rate: rate of above reference clock + * @refclk_reg: holds PHY's referece clock settings + */ +struct exynos5_usbdrd_phy { + struct device *dev; + void __iomem *reg_phy; + struct clk *clk; + struct clk *usb30_sclk; + const struct exynos5_usbdrd_phy_drvdata *drv_data; + struct phy_usb_instance { + struct phy *phy; + u32 index; + struct regmap *reg_pmu; + u32 pmu_offset; + const struct usbdrd_phy_config *phy_cfg; + } phys[EXYNOS5_DRDPHYS_NUM]; + unsigned int extrefclk; + struct clk *ref_clk; + unsigned long ref_rate; + unsigned int refclk_reg; +}; + +#define to_usbdrd_phy(inst) \ + container_of((inst), struct exynos5_usbdrd_phy, \ + phys[(inst)->index]); + +/* + * exynos5_rate_to_clk() converts the supplied clock rate to the value that + * can be written to the phy register. + */ +static unsigned int exynos5_rate_to_clk(unsigned long rate) +{ + unsigned int clksel; + + /* EXYNOS5_FSEL_MASK */ + + switch (rate) { + case 9600 * KHZ: + clksel = EXYNOS5_FSEL_9MHZ6; + break; + case 10 * MHZ: + clksel = EXYNOS5_FSEL_10MHZ; + break; + case 12 * MHZ: + clksel = EXYNOS5_FSEL_12MHZ; + break; + case 19200 * KHZ: + clksel = EXYNOS5_FSEL_19MHZ2; + break; + case 20 * MHZ: + clksel = EXYNOS5_FSEL_20MHZ; + break; + case 24 * MHZ: + clksel = EXYNOS5_FSEL_24MHZ; + break; + case 50 * MHZ: + clksel = EXYNOS5_FSEL_50MHZ; + break; + default: + clksel = -EINVAL; + } + + return clksel; +} + +static void exynos5_usbdrd_phy_isol(struct phy_usb_instance *inst, + unsigned int on) +{ + if (!inst->reg_pmu) + return; + + regmap_update_bits(inst->reg_pmu, inst->pmu_offset, + EXYNOS5_USBDRD_PMU_ISOL, ~on); +} + +/* + * Sets the pipe3 phy's clk as EXTREFCLK (XXTI) which is internal clock + * from clock core. Further sets multiplier values and spread spectrum + * clock settings for SuperSpeed operations. + */ +static unsigned int +exynos5_usbdrd_pipe3_set_refclk(struct phy_usb_instance *inst) +{ + static u32 reg; + struct exynos5_usbdrd_phy *phy_drd = to_usbdrd_phy(inst); + + /* restore any previous reference clock settings */ + reg = phy_drd->refclk_reg; + + /* Use EXTREFCLK as ref clock */ + reg &= ~PHYCLKRST_REFCLKSEL_MASK; + reg |= PHYCLKRST_REFCLKSEL_EXT_REFCLK; + + /* FSEL settings corresponding to reference clock */ + reg &= ~PHYCLKRST_FSEL_PIPE_MASK | + PHYCLKRST_MPLL_MULTIPLIER_MASK | + PHYCLKRST_SSC_REFCLKSEL_MASK; + switch (phy_drd->extrefclk) { + case EXYNOS5_FSEL_50MHZ: + reg |= (PHYCLKRST_MPLL_MULTIPLIER_50M_REF | + PHYCLKRST_SSC_REFCLKSEL(0x00)); + break; + case EXYNOS5_FSEL_24MHZ: + reg |= (PHYCLKRST_MPLL_MULTIPLIER_24MHZ_REF | + PHYCLKRST_SSC_REFCLKSEL(0x88)); + break; + case EXYNOS5_FSEL_20MHZ: + reg |= (PHYCLKRST_MPLL_MULTIPLIER_20MHZ_REF | + PHYCLKRST_SSC_REFCLKSEL(0x00)); + break; + case EXYNOS5_FSEL_19MHZ2: + reg |= (PHYCLKRST_MPLL_MULTIPLIER_19200KHZ_REF | + PHYCLKRST_SSC_REFCLKSEL(0x88)); + break; + default: + dev_dbg(phy_drd->dev, "unsupported ref clk\n"); + break; + } + + /* save refclk settings for multiple phy inits */ + phy_drd->refclk_reg = reg; + + return reg; +} + +/* + * Sets the utmi phy's clk as EXTREFCLK (XXTI) which is internal clock + * from clock core. Further sets the FSEL values for HighSpeed operations. + */ +static unsigned int +exynos5_usbdrd_utmi_set_refclk(struct phy_usb_instance *inst) +{ + static u32 reg; + struct exynos5_usbdrd_phy *phy_drd = to_usbdrd_phy(inst); + + reg = phy_drd->refclk_reg; + + reg &= ~PHYCLKRST_REFCLKSEL_MASK; + reg |= PHYCLKRST_REFCLKSEL_EXT_REFCLK; + + reg &= ~PHYCLKRST_FSEL_UTMI_MASK | + PHYCLKRST_MPLL_MULTIPLIER_MASK | + PHYCLKRST_SSC_REFCLKSEL_MASK; + reg |= PHYCLKRST_FSEL(phy_drd->extrefclk); + + phy_drd->refclk_reg = reg; + + return reg; +} + +static void exynos5_usbdrd_pipe3_init(struct exynos5_usbdrd_phy *phy_drd) +{ + u32 reg; + + reg = readl(phy_drd->reg_phy + EXYNOS5_DRD_PHYPARAM1); + /* Set Tx De-Emphasis level */ + reg &= ~PHYPARAM1_PCS_TXDEEMPH_MASK; + reg |= PHYPARAM1_PCS_TXDEEMPH; + writel(reg, phy_drd->reg_phy + EXYNOS5_DRD_PHYPARAM1); + + reg = readl(phy_drd->reg_phy + EXYNOS5_DRD_PHYTEST); + reg &= ~PHYTEST_POWERDOWN_SSP; + writel(reg, phy_drd->reg_phy + EXYNOS5_DRD_PHYTEST); +} + +static void exynos5_usbdrd_utmi_init(struct exynos5_usbdrd_phy *phy_drd) +{ + u32 reg; + + reg = readl(phy_drd->reg_phy + EXYNOS5_DRD_PHYPARAM0); + /* Set Loss-of-Signal Detector sensitivity */ + reg &= ~PHYPARAM0_REF_LOSLEVEL_MASK; + reg |= PHYPARAM0_REF_LOSLEVEL; + writel(reg, phy_drd->reg_phy + EXYNOS5_DRD_PHYPARAM0); + + reg = readl(phy_drd->reg_phy + EXYNOS5_DRD_PHYPARAM1); + /* Set Tx De-Emphasis level */ + reg &= ~PHYPARAM1_PCS_TXDEEMPH_MASK; + reg |= PHYPARAM1_PCS_TXDEEMPH; + writel(reg, phy_drd->reg_phy + EXYNOS5_DRD_PHYPARAM1); + + /* UTMI Power Control */ + writel(PHYUTMI_OTGDISABLE, phy_drd->reg_phy + EXYNOS5_DRD_PHYUTMI); + + reg = readl(phy_drd->reg_phy + EXYNOS5_DRD_PHYTEST); + reg &= ~PHYTEST_POWERDOWN_HSP; + writel(reg, phy_drd->reg_phy + EXYNOS5_DRD_PHYTEST); +} + +static int exynos5_usbdrd_phy_init(struct phy *phy) +{ + int ret; + u32 reg; + struct phy_usb_instance *inst = phy_get_drvdata(phy); + struct exynos5_usbdrd_phy *phy_drd = to_usbdrd_phy(inst); + + ret = clk_prepare_enable(phy_drd->clk); + if (ret) + return ret; + + phy_drd->extrefclk = exynos5_rate_to_clk(phy_drd->ref_rate); + if (phy_drd->extrefclk == -EINVAL) { + dev_err(phy_drd->dev, "Clock rate (%ld) not supported\n", + phy_drd->ref_rate); + return -EINVAL; + } + + /* Reset USB 3.0 PHY */ + writel(0x0, phy_drd->reg_phy + EXYNOS5_DRD_PHYREG0); + writel(0x0, phy_drd->reg_phy + EXYNOS5_DRD_PHYRESUME); + + /* + * Setting the Frame length Adj value[6:1] to default 0x20 + * See xHCI 1.0 spec, 5.2.4 + */ + reg = LINKSYSTEM_XHCI_VERSION_CONTROL | + LINKSYSTEM_FLADJ(0x20); + writel(reg, phy_drd->reg_phy + EXYNOS5_DRD_LINKSYSTEM); + + reg = readl(phy_drd->reg_phy + EXYNOS5_DRD_PHYPARAM0); + /* Select PHY CLK source */ + reg &= ~PHYPARAM0_REF_USE_PAD; + writel(reg, phy_drd->reg_phy + EXYNOS5_DRD_PHYPARAM0); + + /* This bit must be set for both HS and SS operations */ + reg = readl(phy_drd->reg_phy + EXYNOS5_DRD_PHYUTMICLKSEL); + reg |= PHYUTMICLKSEL_UTMI_CLKSEL; + writel(reg, phy_drd->reg_phy + EXYNOS5_DRD_PHYUTMICLKSEL); + + /* UTMI or PIPE3 specific init */ + inst->phy_cfg->phy_init(phy_drd); + + /* reference clock settings */ + reg = inst->phy_cfg->set_refclk(inst); + + /* Digital power supply in normal operating mode */ + reg |= PHYCLKRST_RETENABLEN | + /* Enable ref clock for SS function */ + PHYCLKRST_REF_SSP_EN | + /* Enable spread spectrum */ + PHYCLKRST_SSC_EN | + /* Power down HS Bias and PLL blocks in suspend mode */ + PHYCLKRST_COMMONONN | + /* Reset the port */ + PHYCLKRST_PORTRESET; + + writel(reg, phy_drd->reg_phy + EXYNOS5_DRD_PHYCLKRST); + + udelay(10); + + reg &= ~PHYCLKRST_PORTRESET; + writel(reg, phy_drd->reg_phy + EXYNOS5_DRD_PHYCLKRST); + + clk_disable_unprepare(phy_drd->clk); + + return 0; +} + +static int exynos5_usbdrd_phy_exit(struct phy *phy) +{ + int ret; + u32 reg; + struct phy_usb_instance *inst = phy_get_drvdata(phy); + struct exynos5_usbdrd_phy *phy_drd = to_usbdrd_phy(inst); + + ret = clk_prepare_enable(phy_drd->clk); + if (ret) + return ret; + + reg = PHYUTMI_OTGDISABLE | + PHYUTMI_FORCESUSPEND | + PHYUTMI_FORCESLEEP; + writel(reg, phy_drd->reg_phy + EXYNOS5_DRD_PHYUTMI); + + /* Resetting the PHYCLKRST enable bits to reduce leakage current */ + reg = readl(phy_drd->reg_phy + EXYNOS5_DRD_PHYCLKRST); + reg &= ~(PHYCLKRST_REF_SSP_EN | + PHYCLKRST_SSC_EN | + PHYCLKRST_COMMONONN); + writel(reg, phy_drd->reg_phy + EXYNOS5_DRD_PHYCLKRST); + + /* Control PHYTEST to remove leakage current */ + reg = readl(phy_drd->reg_phy + EXYNOS5_DRD_PHYTEST); + reg |= PHYTEST_POWERDOWN_SSP | + PHYTEST_POWERDOWN_HSP; + writel(reg, phy_drd->reg_phy + EXYNOS5_DRD_PHYTEST); + + clk_disable_unprepare(phy_drd->clk); + + return 0; +} + +static int exynos5_usbdrd_phy_power_on(struct phy *phy) +{ + struct phy_usb_instance *inst = phy_get_drvdata(phy); + struct exynos5_usbdrd_phy *phy_drd = to_usbdrd_phy(inst); + + dev_dbg(phy_drd->dev, "Request to power_on usbdrd_phy phy\n"); + + clk_prepare_enable(phy_drd->ref_clk); + + if (!IS_ERR(phy_drd->usb30_sclk)) + clk_prepare_enable(phy_drd->usb30_sclk); + + /* Power-on PHY*/ + inst->phy_cfg->phy_isol(inst, 0); + + return 0; +} + +static int exynos5_usbdrd_phy_power_off(struct phy *phy) +{ + struct phy_usb_instance *inst = phy_get_drvdata(phy); + struct exynos5_usbdrd_phy *phy_drd = to_usbdrd_phy(inst); + + dev_dbg(phy_drd->dev, "Request to power_off usbdrd_phy phy\n"); + + /* Power-off the PHY */ + inst->phy_cfg->phy_isol(inst, 1); + + if (!IS_ERR(phy_drd->usb30_sclk)) + clk_disable_unprepare(phy_drd->usb30_sclk); + + clk_disable_unprepare(phy_drd->ref_clk); + + return 0; +} + +static struct phy *exynos5_usbdrd_phy_xlate(struct device *dev, + struct of_phandle_args *args) +{ + struct exynos5_usbdrd_phy *phy_drd = dev_get_drvdata(dev); + + if (WARN_ON(args->args[0] > EXYNOS5_DRDPHYS_NUM)) + return ERR_PTR(-ENODEV); + + return phy_drd->phys[args->args[0]].phy; +} + +static struct phy_ops exynos5_usbdrd_phy_ops = { + .init = exynos5_usbdrd_phy_init, + .exit = exynos5_usbdrd_phy_exit, + .power_on = exynos5_usbdrd_phy_power_on, + .power_off = exynos5_usbdrd_phy_power_off, + .owner = THIS_MODULE, +}; + +const struct usbdrd_phy_config exynos5_usbdrd_phy_cfg[] = { + { + .id = EXYNOS5_DRDPHY_UTMI, + .phy_isol = exynos5_usbdrd_phy_isol, + .phy_init = exynos5_usbdrd_utmi_init, + .set_refclk = exynos5_usbdrd_utmi_set_refclk, + }, + { + .id = EXYNOS5_DRDPHY_PIPE3, + .phy_isol = exynos5_usbdrd_phy_isol, + .phy_init = exynos5_usbdrd_pipe3_init, + .set_refclk = exynos5_usbdrd_pipe3_set_refclk, + }, + {}, +}; + +const struct exynos5_usbdrd_phy_drvdata exynos5420_usbdrd_phy = { + .has_usb30_sclk = true, + .phy_cfg = exynos5_usbdrd_phy_cfg, +}; + +const struct exynos5_usbdrd_phy_drvdata exynos5250_usbdrd_phy = { + .has_usb30_sclk = false, + .phy_cfg = exynos5_usbdrd_phy_cfg, +}; + +static const struct of_device_id exynos5_usbdrd_phy_of_match[] = { + { + .compatible = "samsung,exynos5250-usbdrd-phy", + .data = &exynos5250_usbdrd_phy + }, { + .compatible = "samsung,exynos5420-usbdrd-phy", + .data = &exynos5420_usbdrd_phy + }, + { }, +}; + +static int exynos5_usbdrd_phy_probe(struct platform_device *pdev) +{ + struct device *dev = &pdev->dev; + struct device_node *node = dev->of_node; + struct exynos5_usbdrd_phy *phy_drd; + struct phy_provider *phy_provider; + struct resource *res; + const struct of_device_id *match; + const struct exynos5_usbdrd_phy_drvdata *drv_data; + struct regmap *reg_pmu; + u32 pmu_offset; + int i; + + /* + * Exynos systems are completely DT enabled, + * so lets not have any platform data support for this driver. + */ + if (!node) { + dev_err(dev, "no device node found\n"); + return -ENODEV; + } + + match = of_match_node(exynos5_usbdrd_phy_of_match, pdev->dev.of_node); + if (!match) { + dev_err(dev, "of_match_node() failed\n"); + return -EINVAL; + } + drv_data = match->data; + + phy_drd = devm_kzalloc(dev, sizeof(*phy_drd), GFP_KERNEL); + if (!phy_drd) + return -ENOMEM; + + dev_set_drvdata(dev, phy_drd); + phy_drd->dev = dev; + + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); + phy_drd->reg_phy = devm_ioremap_resource(dev, res); + if (IS_ERR(phy_drd->reg_phy)) { + dev_err(dev, "Failed to map register memory (phy)\n"); + return PTR_ERR(phy_drd->reg_phy); + } + + phy_drd->drv_data = drv_data; + + if (of_property_read_u32(node, "samsung,pmu-offset", &pmu_offset)) { + dev_err(dev, "Missing pmu-offset for phy isolation\n"); + return -EINVAL; + } + + phy_drd->clk = devm_clk_get(dev, "phy"); + if (IS_ERR(phy_drd->clk)) { + dev_err(dev, "Failed to get clock of phy controller\n"); + return PTR_ERR(phy_drd->clk); + } + + /* + * Exysno5420 SoC has an additional special clock, used for + * for USB 3.0 PHY operation, this clock goes to the PHY block + * as a reference clock to clock generation block of the controller, + * named as 'USB30_SCLK_100M'. + */ + if (drv_data->has_usb30_sclk) { + phy_drd->usb30_sclk = devm_clk_get(dev, "usb30_sclk_100m"); + if (IS_ERR(phy_drd->usb30_sclk)) { + dev_err(dev, "Failed to get phy reference clock\n"); + return PTR_ERR(phy_drd->usb30_sclk); + } + } + + phy_drd->ref_clk = devm_clk_get(dev, "ref"); + if (IS_ERR(phy_drd->ref_clk)) { + dev_err(dev, "Failed to get reference clock of usbdrd_phy phy\n"); + return PTR_ERR(phy_drd->ref_clk); + } + phy_drd->ref_rate = clk_get_rate(phy_drd->ref_clk); + + dev_vdbg(dev, "Creating usbdrd_phy phy\n"); + + reg_pmu = syscon_regmap_lookup_by_phandle(dev->of_node, + "samsung,syscon-phandle"); + if (IS_ERR(reg_pmu)) { + dev_err(dev, "Failed to map PMU register (via syscon)\n"); + return PTR_ERR(reg_pmu); + } + + for (i = 0; i < EXYNOS5_DRDPHYS_NUM; i++) { + struct phy *phy = devm_phy_create(dev, &exynos5_usbdrd_phy_ops, + NULL); + if (IS_ERR(phy)) { + dev_err(dev, "Failed to create usbdrd_phy phy\n"); + return PTR_ERR(phy); + } + + phy_drd->phys[i].phy = phy; + phy_drd->phys[i].index = i; + phy_drd->phys[i].reg_pmu = reg_pmu; + phy_drd->phys[i].pmu_offset = pmu_offset; + phy_drd->phys[i].phy_cfg = &drv_data->phy_cfg[i]; + phy_set_drvdata(phy, &phy_drd->phys[i]); + } + + phy_provider = devm_of_phy_provider_register(dev, + exynos5_usbdrd_phy_xlate); + if (IS_ERR(phy_provider)) { + dev_err(phy_drd->dev, "Failed to register phy provider\n"); + return PTR_ERR(phy_provider); + } + + return 0; +} + +static struct platform_driver exynos5_usb3drd_phy = { + .probe = exynos5_usbdrd_phy_probe, + .driver = { + .of_match_table = exynos5_usbdrd_phy_of_match, + .name = "exynos5_usb3drd_phy", + .owner = THIS_MODULE, + } +}; + +module_platform_driver(exynos5_usb3drd_phy); +MODULE_DESCRIPTION("Samsung EXYNOS5 SoCs USB 3.0 DRD controller PHY driver"); +MODULE_AUTHOR("Vivek Gautam <gautam.vivek@samsung.com>"); +MODULE_LICENSE("GPL v2"); +MODULE_ALIAS("platform:exynos5_usb3drd_phy");
Add a new driver for the USB 3.0 PHY on Exynos5 series of SoCs. The new driver uses the generic PHY framework and will interact with DWC3 controller present on Exynos5 series of SoCs. Thereby, removing old phy-samsung-usb3 driver and related code used untill now which was based on usb/phy framework. Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com> --- .../devicetree/bindings/phy/samsung-phy.txt | 42 ++ drivers/phy/Kconfig | 11 + drivers/phy/Makefile | 1 + drivers/phy/phy-exynos5-usbdrd.c | 668 ++++++++++++++++++++ 4 files changed, 722 insertions(+) create mode 100644 drivers/phy/phy-exynos5-usbdrd.c