2014-04-08 14:37:04

by Vivek Gautam

[permalink] [raw]
Subject: [PATCH V4 0/5] Add Exynos5 USB 3.0 phy driver based on generic PHY framework

Based on 'next' branch of Kishon's phy tree (linux-phy).
Tested on 'usb-next' of Greg's usb tree.

Changes from V3:
1) Separated out the phy init sequences for utmi and pipe3 phys.
2) Changed the nomenclature across the phy to 'usbdrd-phy' to
indicate USB 3.0 DRD PHY controller; and thereby changed the names
of functions correspondingly, including specific functions for
utmi and pipe3 phys.
3) Modified the DT nodes for the updated nomenclature.
4) Using BIT macro for single bit definitions.
5) Keeping track of reference clock after getting till the removal of
phy, and getting the ref clock using devm_clk_get() api.
6) Removed aliases for mutiple channel PHYs, and instead using
'samsung,pmu-offset' property for PHY power control register offset.
7) Keeping the phy_init() and phy_power_on() separately in order to
align with phy handling in the consumer (DWC3).

Changes from v2:
1) Added support for multiple PHYs (UTMI+ and PIPE3) and
related changes in the driver structuring.
2) Added a xlate function to get the required phy out of
number of PHYs in mutiple PHY scenerio.
3) Changed the names of few structures and variables to
have a clearer meaning.
4) Added 'usb3phy_config' structure to take care of mutiple
phys for a SoC having 'exynos5_usb3phy_drv_data' driver data.
5) Not deleting support for old driver 'phy-samsung-usb3' until
required support for generic phy is added to DWC3.

Changes from RFC patch-set:
1) fixes in documentation file
- added provision for syscon interface for using PMU register.
- added clock names and description
- modified description style for 'compatible property'
- made usb30_sclk as additional clock rather then making it optional, since
it is actually an additional clock for Exynos5420 Soc.
2) fixes in phy-exynos5-usb3 driver file
- removed unnecessary #ifndef around KHZ and MHZ definitions
- removed 'samsung_cpu_type', 'usb3phy_state' enums; and merged necessary
necessary from 'usb3phy_instance' structure to 'usb3phy_driver'.
- changed name 'sclk_usbphy30' to 'usb30_sclk_100m' since this is the name
indicated as input to the PHY block; and also added (!IS_ERR()) check for
using usb30_sclk.
- removed unnecessary 'state' check code.
- moved 'of_device_id' structure definitions before 'probe()' to avoid
unnecessary declaration.
- added (pdev->dev.of_node == NULL) check at the starting of probe()
- moved 'devm_of_phy_provider_register()' call to end of the probe().
- removed 'label' for usb3drd phy.
- corrected macros definition 'PHYCLKRST_MPLL_MULTIPLIER_50M_REF' from
0x02 to 0x32 after confirming same from PHY's data sheet.
- replaced pmu register handling, used for power-isolation, with syscon
interface api's.
- added '.init' and '.exit' callbacks and using them for one time
PHY-initialization and deinitialization.
- Filtering out the PHY 'power-on' and 'power-off' sequence to '.power_on"
and ".power_off" callbacks.
- Removed drivers/usb/phy/phy-samsung-usb3.c driver and related code.
3) fixes in dt files
- added reference for 'samsung,syscon-phandle' to used for PMU register.
- removed second register field which was earlier used for PMU.

Vivek Gautam (5):
phy: Add new Exynos5 USB 3.0 PHY driver
dt: exynos5420: Enable support for USB 3.0 PHY controller
dt: exynos5420: Enable support for DWC3 controller
dt: exynos5250: Enable support for generic USB DRD phy
usb-phy: samsung-usb3: Remove older phy-samsung-usb3 driver

.../devicetree/bindings/phy/samsung-phy.txt | 42 ++
arch/arm/boot/dts/exynos5250.dtsi | 21 +-
arch/arm/boot/dts/exynos5420.dtsi | 54 ++
drivers/phy/Kconfig | 11 +
drivers/phy/Makefile | 1 +
drivers/phy/phy-exynos5-usbdrd.c | 668 ++++++++++++++++++++
drivers/usb/phy/phy-samsung-usb.h | 83 ---
drivers/usb/phy/phy-samsung-usb3.c | 350 ----------
8 files changed, 785 insertions(+), 445 deletions(-)
create mode 100644 drivers/phy/phy-exynos5-usbdrd.c
delete mode 100644 drivers/usb/phy/phy-samsung-usb3.c

--
1.7.10.4


2014-04-08 14:37:35

by Vivek Gautam

[permalink] [raw]
Subject: [PATCH V4 4/5] dt: exynos5250: Enable support for generic USB DRD phy

Add device tree node for new usbdrd-phy driver, which
is based on generic phy framework.

Signed-off-by: Vivek Gautam <[email protected]>
---
arch/arm/boot/dts/exynos5250.dtsi | 10 ++++++++++
1 file changed, 10 insertions(+)

diff --git a/arch/arm/boot/dts/exynos5250.dtsi b/arch/arm/boot/dts/exynos5250.dtsi
index b7dec41..92c6fcd 100644
--- a/arch/arm/boot/dts/exynos5250.dtsi
+++ b/arch/arm/boot/dts/exynos5250.dtsi
@@ -530,6 +530,16 @@
};
};

+ usbdrd_phy: phy@12100000 {
+ compatible = "samsung,exynos5250-usbdrd-phy";
+ reg = <0x12100000 0x100>;
+ clocks = <&clock 286>, <&clock 1>;
+ clock-names = "phy", "ref";
+ samsung,syscon-phandle = <&pmu_system_controller>;
+ samsung,pmu-offset = <0x704>;
+ #phy-cells = <1>;
+ };
+
usb@12110000 {
compatible = "samsung,exynos4210-ehci";
reg = <0x12110000 0x100>;
--
1.7.10.4

2014-04-08 14:38:09

by Vivek Gautam

[permalink] [raw]
Subject: [PATCH V4 5/5] usb-phy: samsung-usb3: Remove older phy-samsung-usb3 driver

Removing this older USB 3.0 DRD controller PHY driver, since
a new driver based on generic phy framework is now available.

Also removing the dt node for older driver from Exynos5250
device tree and updating the dt node for DWC3 controller.

Signed-off-by: Vivek Gautam <[email protected]>
---

NOTE: This patch should be merged only when the new USB 3.0
DRD phy controller driver is available in the tree from the
patches:
phy: Add new Exynos5 USB 3.0 PHY driver; and
dt: exynos5250: Enable support for generic USB DRD phy

arch/arm/boot/dts/exynos5250.dtsi | 17 +-
drivers/usb/phy/phy-samsung-usb.h | 83 ---------
drivers/usb/phy/phy-samsung-usb3.c | 350 ------------------------------------
3 files changed, 2 insertions(+), 448 deletions(-)
delete mode 100644 drivers/usb/phy/phy-samsung-usb3.c

diff --git a/arch/arm/boot/dts/exynos5250.dtsi b/arch/arm/boot/dts/exynos5250.dtsi
index 92c6fcd..1cb1e91 100644
--- a/arch/arm/boot/dts/exynos5250.dtsi
+++ b/arch/arm/boot/dts/exynos5250.dtsi
@@ -512,21 +512,8 @@
compatible = "synopsys,dwc3";
reg = <0x12000000 0x10000>;
interrupts = <0 72 0>;
- usb-phy = <&usb2_phy &usb3_phy>;
- };
- };
-
- usb3_phy: usbphy@12100000 {
- compatible = "samsung,exynos5250-usb3phy";
- reg = <0x12100000 0x100>;
- clocks = <&clock 1>, <&clock 286>;
- clock-names = "ext_xtal", "usbdrd30";
- #address-cells = <1>;
- #size-cells = <1>;
- ranges;
-
- usbphy-sys {
- reg = <0x10040704 0x8>;
+ phys = <&usbdrd_phy 0>, <&usbdrd_phy 1>;
+ phy-names = "usb2-phy", "usb3-phy";
};
};

diff --git a/drivers/usb/phy/phy-samsung-usb.h b/drivers/usb/phy/phy-samsung-usb.h
index 68771bf..ced8b65 100644
--- a/drivers/usb/phy/phy-samsung-usb.h
+++ b/drivers/usb/phy/phy-samsung-usb.h
@@ -155,86 +155,6 @@

#define EXYNOS5_PHY_OTG_TUNE (0x40)

-/* EXYNOS5: USB 3.0 DRD */
-#define EXYNOS5_DRD_LINKSYSTEM (0x04)
-
-#define LINKSYSTEM_FLADJ_MASK (0x3f << 1)
-#define LINKSYSTEM_FLADJ(_x) ((_x) << 1)
-#define LINKSYSTEM_XHCI_VERSION_CONTROL (0x1 << 27)
-
-#define EXYNOS5_DRD_PHYUTMI (0x08)
-
-#define PHYUTMI_OTGDISABLE (0x1 << 6)
-#define PHYUTMI_FORCESUSPEND (0x1 << 1)
-#define PHYUTMI_FORCESLEEP (0x1 << 0)
-
-#define EXYNOS5_DRD_PHYPIPE (0x0c)
-
-#define EXYNOS5_DRD_PHYCLKRST (0x10)
-
-#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 (0x1 << 20)
-#define PHYCLKRST_REF_SSP_EN (0x1 << 19)
-#define PHYCLKRST_REF_CLKDIV2 (0x1 << 18)
-
-#define PHYCLKRST_MPLL_MULTIPLIER_MASK (0x7f << 11)
-#define PHYCLKRST_MPLL_MULTIPLIER_100MHZ_REF (0x19 << 11)
-#define PHYCLKRST_MPLL_MULTIPLIER_50M_REF (0x02 << 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_MASK (0x3f << 5)
-#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 (0x1 << 4)
-
-#define PHYCLKRST_REFCLKSEL_MASK (0x03 << 2)
-#define PHYCLKRST_REFCLKSEL_PAD_REFCLK (0x2 << 2)
-#define PHYCLKRST_REFCLKSEL_EXT_REFCLK (0x3 << 2)
-
-#define PHYCLKRST_PORTRESET (0x1 << 1)
-#define PHYCLKRST_COMMONONN (0x1 << 0)
-
-#define EXYNOS5_DRD_PHYREG0 (0x14)
-#define EXYNOS5_DRD_PHYREG1 (0x18)
-
-#define EXYNOS5_DRD_PHYPARAM0 (0x1c)
-
-#define PHYPARAM0_REF_USE_PAD (0x1 << 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 (0x1 << 3)
-#define PHYTEST_POWERDOWN_HSP (0x1 << 2)
-
-#define EXYNOS5_DRD_PHYADP (0x2c)
-
-#define EXYNOS5_DRD_PHYBATCHG (0x30)
-
-#define PHYBATCHG_UTMI_CLKSEL (0x1 << 2)
-
-#define EXYNOS5_DRD_PHYRESUME (0x34)
-#define EXYNOS5_DRD_LINKPORT (0x44)
-
#ifndef MHZ
#define MHZ (1000*1000)
#endif
@@ -262,8 +182,6 @@ struct samsung_usbphy;
* @cpu_type: machine identifier
* @devphy_en_mask: device phy enable mask for PHY CONTROL register
* @hostphy_en_mask: host phy enable mask for PHY CONTROL register
- * @devphy_reg_offset: offset to DEVICE PHY CONTROL register from
- * mapped address of system controller.
* @hostphy_reg_offset: offset to HOST PHY CONTROL register from
* mapped address of system controller.
*
@@ -279,7 +197,6 @@ struct samsung_usbphy_drvdata {
int cpu_type;
int devphy_en_mask;
int hostphy_en_mask;
- u32 devphy_reg_offset;
u32 hostphy_reg_offset;
int (*rate_to_clksel)(struct samsung_usbphy *, unsigned long);
void (*set_isolation)(struct samsung_usbphy *, bool);
diff --git a/drivers/usb/phy/phy-samsung-usb3.c b/drivers/usb/phy/phy-samsung-usb3.c
deleted file mode 100644
index cc08192..0000000
--- a/drivers/usb/phy/phy-samsung-usb3.c
+++ /dev/null
@@ -1,350 +0,0 @@
-/* linux/drivers/usb/phy/phy-samsung-usb3.c
- *
- * Copyright (c) 2013 Samsung Electronics Co., Ltd.
- * http://www.samsung.com
- *
- * Author: Vivek Gautam <[email protected]>
- *
- * Samsung USB 3.0 PHY transceiver; talks to DWC3 controller.
- *
- * 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.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- * GNU General Public License for more details.
- */
-
-#include <linux/module.h>
-#include <linux/platform_device.h>
-#include <linux/clk.h>
-#include <linux/delay.h>
-#include <linux/err.h>
-#include <linux/io.h>
-#include <linux/of.h>
-#include <linux/usb/samsung_usb_phy.h>
-#include <linux/platform_data/samsung-usbphy.h>
-
-#include "phy-samsung-usb.h"
-
-/*
- * Sets the phy clk as EXTREFCLK (XXTI) which is internal clock from clock core.
- */
-static u32 samsung_usb3phy_set_refclk(struct samsung_usbphy *sphy)
-{
- u32 reg;
- u32 refclk;
-
- refclk = sphy->ref_clk_freq;
-
- reg = PHYCLKRST_REFCLKSEL_EXT_REFCLK |
- PHYCLKRST_FSEL(refclk);
-
- switch (refclk) {
- case FSEL_CLKSEL_50M:
- reg |= (PHYCLKRST_MPLL_MULTIPLIER_50M_REF |
- PHYCLKRST_SSC_REFCLKSEL(0x00));
- break;
- case FSEL_CLKSEL_20M:
- reg |= (PHYCLKRST_MPLL_MULTIPLIER_20MHZ_REF |
- PHYCLKRST_SSC_REFCLKSEL(0x00));
- break;
- case FSEL_CLKSEL_19200K:
- reg |= (PHYCLKRST_MPLL_MULTIPLIER_19200KHZ_REF |
- PHYCLKRST_SSC_REFCLKSEL(0x88));
- break;
- case FSEL_CLKSEL_24M:
- default:
- reg |= (PHYCLKRST_MPLL_MULTIPLIER_24MHZ_REF |
- PHYCLKRST_SSC_REFCLKSEL(0x88));
- break;
- }
-
- return reg;
-}
-
-static void samsung_exynos5_usb3phy_enable(struct samsung_usbphy *sphy)
-{
- void __iomem *regs = sphy->regs;
- u32 phyparam0;
- u32 phyparam1;
- u32 linksystem;
- u32 phybatchg;
- u32 phytest;
- u32 phyclkrst;
-
- /* Reset USB 3.0 PHY */
- writel(0x0, regs + EXYNOS5_DRD_PHYREG0);
-
- phyparam0 = readl(regs + EXYNOS5_DRD_PHYPARAM0);
- /* Select PHY CLK source */
- phyparam0 &= ~PHYPARAM0_REF_USE_PAD;
- /* Set Loss-of-Signal Detector sensitivity */
- phyparam0 &= ~PHYPARAM0_REF_LOSLEVEL_MASK;
- phyparam0 |= PHYPARAM0_REF_LOSLEVEL;
- writel(phyparam0, regs + EXYNOS5_DRD_PHYPARAM0);
-
- writel(0x0, regs + EXYNOS5_DRD_PHYRESUME);
-
- /*
- * Setting the Frame length Adj value[6:1] to default 0x20
- * See xHCI 1.0 spec, 5.2.4
- */
- linksystem = LINKSYSTEM_XHCI_VERSION_CONTROL |
- LINKSYSTEM_FLADJ(0x20);
- writel(linksystem, regs + EXYNOS5_DRD_LINKSYSTEM);
-
- phyparam1 = readl(regs + EXYNOS5_DRD_PHYPARAM1);
- /* Set Tx De-Emphasis level */
- phyparam1 &= ~PHYPARAM1_PCS_TXDEEMPH_MASK;
- phyparam1 |= PHYPARAM1_PCS_TXDEEMPH;
- writel(phyparam1, regs + EXYNOS5_DRD_PHYPARAM1);
-
- phybatchg = readl(regs + EXYNOS5_DRD_PHYBATCHG);
- phybatchg |= PHYBATCHG_UTMI_CLKSEL;
- writel(phybatchg, regs + EXYNOS5_DRD_PHYBATCHG);
-
- /* PHYTEST POWERDOWN Control */
- phytest = readl(regs + EXYNOS5_DRD_PHYTEST);
- phytest &= ~(PHYTEST_POWERDOWN_SSP |
- PHYTEST_POWERDOWN_HSP);
- writel(phytest, regs + EXYNOS5_DRD_PHYTEST);
-
- /* UTMI Power Control */
- writel(PHYUTMI_OTGDISABLE, regs + EXYNOS5_DRD_PHYUTMI);
-
- phyclkrst = samsung_usb3phy_set_refclk(sphy);
-
- phyclkrst |= PHYCLKRST_PORTRESET |
- /* Digital power supply in normal operating mode */
- 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;
-
- writel(phyclkrst, regs + EXYNOS5_DRD_PHYCLKRST);
-
- udelay(10);
-
- phyclkrst &= ~(PHYCLKRST_PORTRESET);
- writel(phyclkrst, regs + EXYNOS5_DRD_PHYCLKRST);
-}
-
-static void samsung_exynos5_usb3phy_disable(struct samsung_usbphy *sphy)
-{
- u32 phyutmi;
- u32 phyclkrst;
- u32 phytest;
- void __iomem *regs = sphy->regs;
-
- phyutmi = PHYUTMI_OTGDISABLE |
- PHYUTMI_FORCESUSPEND |
- PHYUTMI_FORCESLEEP;
- writel(phyutmi, regs + EXYNOS5_DRD_PHYUTMI);
-
- /* Resetting the PHYCLKRST enable bits to reduce leakage current */
- phyclkrst = readl(regs + EXYNOS5_DRD_PHYCLKRST);
- phyclkrst &= ~(PHYCLKRST_REF_SSP_EN |
- PHYCLKRST_SSC_EN |
- PHYCLKRST_COMMONONN);
- writel(phyclkrst, regs + EXYNOS5_DRD_PHYCLKRST);
-
- /* Control PHYTEST to remove leakage current */
- phytest = readl(regs + EXYNOS5_DRD_PHYTEST);
- phytest |= (PHYTEST_POWERDOWN_SSP |
- PHYTEST_POWERDOWN_HSP);
- writel(phytest, regs + EXYNOS5_DRD_PHYTEST);
-}
-
-static int samsung_usb3phy_init(struct usb_phy *phy)
-{
- struct samsung_usbphy *sphy;
- unsigned long flags;
- int ret = 0;
-
- sphy = phy_to_sphy(phy);
-
- /* Enable the phy clock */
- ret = clk_prepare_enable(sphy->clk);
- if (ret) {
- dev_err(sphy->dev, "%s: clk_prepare_enable failed\n", __func__);
- return ret;
- }
-
- spin_lock_irqsave(&sphy->lock, flags);
-
- /* setting default phy-type for USB 3.0 */
- samsung_usbphy_set_type(&sphy->phy, USB_PHY_TYPE_DEVICE);
-
- /* Disable phy isolation */
- if (sphy->drv_data->set_isolation)
- sphy->drv_data->set_isolation(sphy, false);
-
- /* Initialize usb phy registers */
- sphy->drv_data->phy_enable(sphy);
-
- spin_unlock_irqrestore(&sphy->lock, flags);
-
- /* Disable the phy clock */
- clk_disable_unprepare(sphy->clk);
-
- return ret;
-}
-
-/*
- * The function passed to the usb driver for phy shutdown
- */
-static void samsung_usb3phy_shutdown(struct usb_phy *phy)
-{
- struct samsung_usbphy *sphy;
- unsigned long flags;
-
- sphy = phy_to_sphy(phy);
-
- if (clk_prepare_enable(sphy->clk)) {
- dev_err(sphy->dev, "%s: clk_prepare_enable failed\n", __func__);
- return;
- }
-
- spin_lock_irqsave(&sphy->lock, flags);
-
- /* setting default phy-type for USB 3.0 */
- samsung_usbphy_set_type(&sphy->phy, USB_PHY_TYPE_DEVICE);
-
- /* De-initialize usb phy registers */
- sphy->drv_data->phy_disable(sphy);
-
- /* Enable phy isolation */
- if (sphy->drv_data->set_isolation)
- sphy->drv_data->set_isolation(sphy, true);
-
- spin_unlock_irqrestore(&sphy->lock, flags);
-
- clk_disable_unprepare(sphy->clk);
-}
-
-static int samsung_usb3phy_probe(struct platform_device *pdev)
-{
- struct samsung_usbphy *sphy;
- struct samsung_usbphy_data *pdata = dev_get_platdata(&pdev->dev);
- struct device *dev = &pdev->dev;
- struct resource *phy_mem;
- void __iomem *phy_base;
- struct clk *clk;
- int ret;
-
- phy_mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- phy_base = devm_ioremap_resource(dev, phy_mem);
- if (IS_ERR(phy_base))
- return PTR_ERR(phy_base);
-
- sphy = devm_kzalloc(dev, sizeof(*sphy), GFP_KERNEL);
- if (!sphy)
- return -ENOMEM;
-
- clk = devm_clk_get(dev, "usbdrd30");
- if (IS_ERR(clk)) {
- dev_err(dev, "Failed to get device clock\n");
- return PTR_ERR(clk);
- }
-
- sphy->dev = dev;
-
- if (dev->of_node) {
- ret = samsung_usbphy_parse_dt(sphy);
- if (ret < 0)
- return ret;
- } else {
- if (!pdata) {
- dev_err(dev, "no platform data specified\n");
- return -EINVAL;
- }
- }
-
- sphy->plat = pdata;
- sphy->regs = phy_base;
- sphy->clk = clk;
- sphy->phy.dev = sphy->dev;
- sphy->phy.label = "samsung-usb3phy";
- sphy->phy.type = USB_PHY_TYPE_USB3;
- sphy->phy.init = samsung_usb3phy_init;
- sphy->phy.shutdown = samsung_usb3phy_shutdown;
- sphy->drv_data = samsung_usbphy_get_driver_data(pdev);
-
- sphy->ref_clk_freq = samsung_usbphy_get_refclk_freq(sphy);
- if (sphy->ref_clk_freq < 0)
- return -EINVAL;
-
- spin_lock_init(&sphy->lock);
-
- platform_set_drvdata(pdev, sphy);
-
- return usb_add_phy_dev(&sphy->phy);
-}
-
-static int samsung_usb3phy_remove(struct platform_device *pdev)
-{
- struct samsung_usbphy *sphy = platform_get_drvdata(pdev);
-
- usb_remove_phy(&sphy->phy);
-
- if (sphy->pmuregs)
- iounmap(sphy->pmuregs);
- if (sphy->sysreg)
- iounmap(sphy->sysreg);
-
- return 0;
-}
-
-static struct samsung_usbphy_drvdata usb3phy_exynos5 = {
- .cpu_type = TYPE_EXYNOS5250,
- .devphy_en_mask = EXYNOS_USBPHY_ENABLE,
- .rate_to_clksel = samsung_usbphy_rate_to_clksel_4x12,
- .set_isolation = samsung_usbphy_set_isolation_4210,
- .phy_enable = samsung_exynos5_usb3phy_enable,
- .phy_disable = samsung_exynos5_usb3phy_disable,
-};
-
-#ifdef CONFIG_OF
-static const struct of_device_id samsung_usbphy_dt_match[] = {
- {
- .compatible = "samsung,exynos5250-usb3phy",
- .data = &usb3phy_exynos5
- },
- {},
-};
-MODULE_DEVICE_TABLE(of, samsung_usbphy_dt_match);
-#endif
-
-static struct platform_device_id samsung_usbphy_driver_ids[] = {
- {
- .name = "exynos5250-usb3phy",
- .driver_data = (unsigned long)&usb3phy_exynos5,
- },
- {},
-};
-
-MODULE_DEVICE_TABLE(platform, samsung_usbphy_driver_ids);
-
-static struct platform_driver samsung_usb3phy_driver = {
- .probe = samsung_usb3phy_probe,
- .remove = samsung_usb3phy_remove,
- .id_table = samsung_usbphy_driver_ids,
- .driver = {
- .name = "samsung-usb3phy",
- .owner = THIS_MODULE,
- .of_match_table = of_match_ptr(samsung_usbphy_dt_match),
- },
-};
-
-module_platform_driver(samsung_usb3phy_driver);
-
-MODULE_DESCRIPTION("Samsung USB 3.0 phy controller");
-MODULE_AUTHOR("Vivek Gautam <[email protected]>");
-MODULE_LICENSE("GPL");
-MODULE_ALIAS("platform:samsung-usb3phy");
--
1.7.10.4

2014-04-08 14:37:20

by Vivek Gautam

[permalink] [raw]
Subject: [PATCH v4 2/5] dt: exynos5420: Enable support for USB 3.0 PHY controller

Add device tree nodes for USB 3.0 PHY present alongwith
USB 3.0 controller Exynos 5420 SoC. This phy driver is
based on generic phy framework.

Signed-off-by: Vivek Gautam <[email protected]>
---
arch/arm/boot/dts/exynos5420.dtsi | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)

diff --git a/arch/arm/boot/dts/exynos5420.dtsi b/arch/arm/boot/dts/exynos5420.dtsi
index 8db792b..a6efb52 100644
--- a/arch/arm/boot/dts/exynos5420.dtsi
+++ b/arch/arm/boot/dts/exynos5420.dtsi
@@ -652,4 +652,24 @@
clocks = <&clock 319>, <&clock 318>;
clock-names = "tmu_apbif", "tmu_triminfo_apbif";
};
+
+ usbdrd_phy0: phy@12100000 {
+ compatible = "samsung,exynos5420-usbdrd-phy";
+ reg = <0x12100000 0x100>;
+ clocks = <&clock 366>, <&clock 1>, <&clock 152>;
+ clock-names = "phy", "ref", "usb30_sclk_100m";
+ samsung,syscon-phandle = <&pmu_system_controller>;
+ samsung,pmu-offset = <0x704>;
+ #phy-cells = <1>;
+ };
+
+ usbdrd_phy1: phy@12500000 {
+ compatible = "samsung,exynos5420-usbdrd-phy";
+ reg = <0x12500000 0x100>;
+ clocks = <&clock 367>, <&clock 1>, <&clock 153>;
+ clock-names = "phy", "ref", "usb30_sclk_100m";
+ samsung,syscon-phandle = <&pmu_system_controller>;
+ samsung,pmu-offset = <0x708>;
+ #phy-cells = <1>;
+ };
};
--
1.7.10.4

2014-04-08 14:41:55

by Vivek Gautam

[permalink] [raw]
Subject: [PATCH V4 3/5] dt: exynos5420: Enable support for DWC3 controller

Add device tree nodes for DWC3 controller present on
Exynos 5420 SoC, to enable support for USB 3.0.

Signed-off-by: Vivek Gautam <[email protected]>
---
arch/arm/boot/dts/exynos5420.dtsi | 34 ++++++++++++++++++++++++++++++++++
1 file changed, 34 insertions(+)

diff --git a/arch/arm/boot/dts/exynos5420.dtsi b/arch/arm/boot/dts/exynos5420.dtsi
index a6efb52..20c2d0b 100644
--- a/arch/arm/boot/dts/exynos5420.dtsi
+++ b/arch/arm/boot/dts/exynos5420.dtsi
@@ -653,6 +653,23 @@
clock-names = "tmu_apbif", "tmu_triminfo_apbif";
};

+ usb@12000000 {
+ compatible = "samsung,exynos5250-dwusb3";
+ clocks = <&clock 366>;
+ clock-names = "usbdrd30";
+ #address-cells = <1>;
+ #size-cells = <1>;
+ ranges;
+
+ dwc3 {
+ compatible = "synopsys,dwc3";
+ reg = <0x12000000 0x10000>;
+ interrupts = <0 72 0>;
+ phys = <&usbdrd_phy0 0>, <&usbdrd_phy0 1>;
+ phy-names = "usb2-phy", "usb3-phy";
+ };
+ };
+
usbdrd_phy0: phy@12100000 {
compatible = "samsung,exynos5420-usbdrd-phy";
reg = <0x12100000 0x100>;
@@ -663,6 +680,23 @@
#phy-cells = <1>;
};

+ usb@12400000 {
+ compatible = "samsung,exynos5250-dwusb3";
+ clocks = <&clock 367>;
+ clock-names = "usbdrd30";
+ #address-cells = <1>;
+ #size-cells = <1>;
+ ranges;
+
+ dwc3 {
+ compatible = "synopsys,dwc3";
+ reg = <0x12400000 0x10000>;
+ interrupts = <0 73 0>;
+ phys = <&usbdrd_phy1 0>, <&usbdrd_phy1 1>;
+ phy-names = "usb2-phy", "usb3-phy";
+ };
+ };
+
usbdrd_phy1: phy@12500000 {
compatible = "samsung,exynos5420-usbdrd-phy";
reg = <0x12500000 0x100>;
--
1.7.10.4

2014-04-08 14:37:12

by Vivek Gautam

[permalink] [raw]
Subject: [PATCH V4 1/5] phy: Add new Exynos5 USB 3.0 PHY driver

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 <[email protected]>
---
.../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
+ 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 <[email protected]>
+ *
+ * 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 <[email protected]>");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:exynos5_usb3drd_phy");
--
1.7.10.4

2014-04-09 11:06:35

by Tomasz Figa

[permalink] [raw]
Subject: Re: [PATCH V4 1/5] phy: Add new Exynos5 USB 3.0 PHY driver

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 <[email protected]>
> ---
> .../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

2014-04-09 11:10:43

by Tomasz Figa

[permalink] [raw]
Subject: Re: [PATCH v4 2/5] dt: exynos5420: Enable support for USB 3.0 PHY controller

On 08.04.2014 16:36, Vivek Gautam wrote:
> Add device tree nodes for USB 3.0 PHY present alongwith
> USB 3.0 controller Exynos 5420 SoC. This phy driver is
> based on generic phy framework.
>
> Signed-off-by: Vivek Gautam <[email protected]>
> ---
> arch/arm/boot/dts/exynos5420.dtsi | 20 ++++++++++++++++++++
> 1 file changed, 20 insertions(+)
>
> diff --git a/arch/arm/boot/dts/exynos5420.dtsi b/arch/arm/boot/dts/exynos5420.dtsi
> index 8db792b..a6efb52 100644
> --- a/arch/arm/boot/dts/exynos5420.dtsi
> +++ b/arch/arm/boot/dts/exynos5420.dtsi
> @@ -652,4 +652,24 @@
> clocks = <&clock 319>, <&clock 318>;
> clock-names = "tmu_apbif", "tmu_triminfo_apbif";
> };
> +
> + usbdrd_phy0: phy@12100000 {
> + compatible = "samsung,exynos5420-usbdrd-phy";
> + reg = <0x12100000 0x100>;
> + clocks = <&clock 366>, <&clock 1>, <&clock 152>;
> + clock-names = "phy", "ref", "usb30_sclk_100m";

As I mentioned in another reply, please make sure that "usb30_sclk_100m"
isn't simply a gate clock for "ref" clock.

Otherwise,

Reviewed-by: Tomasz Figa <[email protected]>

--
Best regards,
Tomasz

2014-04-09 11:11:16

by Tomasz Figa

[permalink] [raw]
Subject: Re: [PATCH V4 3/5] dt: exynos5420: Enable support for DWC3 controller

On 08.04.2014 16:36, Vivek Gautam wrote:
> Add device tree nodes for DWC3 controller present on
> Exynos 5420 SoC, to enable support for USB 3.0.
>
> Signed-off-by: Vivek Gautam <[email protected]>
> ---
> arch/arm/boot/dts/exynos5420.dtsi | 34 ++++++++++++++++++++++++++++++++++
> 1 file changed, 34 insertions(+)
>
> diff --git a/arch/arm/boot/dts/exynos5420.dtsi b/arch/arm/boot/dts/exynos5420.dtsi
> index a6efb52..20c2d0b 100644
> --- a/arch/arm/boot/dts/exynos5420.dtsi
> +++ b/arch/arm/boot/dts/exynos5420.dtsi
> @@ -653,6 +653,23 @@
> clock-names = "tmu_apbif", "tmu_triminfo_apbif";
> };
>
> + usb@12000000 {
> + compatible = "samsung,exynos5250-dwusb3";
> + clocks = <&clock 366>;
> + clock-names = "usbdrd30";
> + #address-cells = <1>;
> + #size-cells = <1>;
> + ranges;
> +
> + dwc3 {
> + compatible = "synopsys,dwc3";
> + reg = <0x12000000 0x10000>;
> + interrupts = <0 72 0>;
> + phys = <&usbdrd_phy0 0>, <&usbdrd_phy0 1>;
> + phy-names = "usb2-phy", "usb3-phy";
> + };
> + };
> +
> usbdrd_phy0: phy@12100000 {
> compatible = "samsung,exynos5420-usbdrd-phy";
> reg = <0x12100000 0x100>;
> @@ -663,6 +680,23 @@
> #phy-cells = <1>;
> };
>
> + usb@12400000 {
> + compatible = "samsung,exynos5250-dwusb3";
> + clocks = <&clock 367>;
> + clock-names = "usbdrd30";
> + #address-cells = <1>;
> + #size-cells = <1>;
> + ranges;
> +
> + dwc3 {
> + compatible = "synopsys,dwc3";
> + reg = <0x12400000 0x10000>;
> + interrupts = <0 73 0>;
> + phys = <&usbdrd_phy1 0>, <&usbdrd_phy1 1>;
> + phy-names = "usb2-phy", "usb3-phy";
> + };
> + };
> +
> usbdrd_phy1: phy@12500000 {
> compatible = "samsung,exynos5420-usbdrd-phy";
> reg = <0x12500000 0x100>;
>

Reviewed-by: Tomasz Figa <[email protected]>

--
Best regards,
Tomasz

2014-04-09 11:11:40

by Tomasz Figa

[permalink] [raw]
Subject: Re: [PATCH V4 4/5] dt: exynos5250: Enable support for generic USB DRD phy

On 08.04.2014 16:36, Vivek Gautam wrote:
> Add device tree node for new usbdrd-phy driver, which
> is based on generic phy framework.
>
> Signed-off-by: Vivek Gautam <[email protected]>
> ---
> arch/arm/boot/dts/exynos5250.dtsi | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/arch/arm/boot/dts/exynos5250.dtsi b/arch/arm/boot/dts/exynos5250.dtsi
> index b7dec41..92c6fcd 100644
> --- a/arch/arm/boot/dts/exynos5250.dtsi
> +++ b/arch/arm/boot/dts/exynos5250.dtsi
> @@ -530,6 +530,16 @@
> };
> };
>
> + usbdrd_phy: phy@12100000 {
> + compatible = "samsung,exynos5250-usbdrd-phy";
> + reg = <0x12100000 0x100>;
> + clocks = <&clock 286>, <&clock 1>;
> + clock-names = "phy", "ref";
> + samsung,syscon-phandle = <&pmu_system_controller>;
> + samsung,pmu-offset = <0x704>;
> + #phy-cells = <1>;
> + };
> +
> usb@12110000 {
> compatible = "samsung,exynos4210-ehci";
> reg = <0x12110000 0x100>;
>

Reviewed-by: Tomasz Figa <[email protected]>

--
Best regards,
Tomasz

2014-04-09 11:13:39

by Tomasz Figa

[permalink] [raw]
Subject: Re: [PATCH V4 5/5] usb-phy: samsung-usb3: Remove older phy-samsung-usb3 driver

Hi Vivek,

On 08.04.2014 16:36, Vivek Gautam wrote:
> Removing this older USB 3.0 DRD controller PHY driver, since
> a new driver based on generic phy framework is now available.
>
> Also removing the dt node for older driver from Exynos5250
> device tree and updating the dt node for DWC3 controller.
>
> Signed-off-by: Vivek Gautam <[email protected]>
> ---
>
> NOTE: This patch should be merged only when the new USB 3.0
> DRD phy controller driver is available in the tree from the
> patches:
> phy: Add new Exynos5 USB 3.0 PHY driver; and
> dt: exynos5250: Enable support for generic USB DRD phy
>
> arch/arm/boot/dts/exynos5250.dtsi | 17 +-
> drivers/usb/phy/phy-samsung-usb.h | 83 ---------
> drivers/usb/phy/phy-samsung-usb3.c | 350 ------------------------------------
> 3 files changed, 2 insertions(+), 448 deletions(-)
> delete mode 100644 drivers/usb/phy/phy-samsung-usb3.c
>
> diff --git a/arch/arm/boot/dts/exynos5250.dtsi b/arch/arm/boot/dts/exynos5250.dtsi
> index 92c6fcd..1cb1e91 100644
> --- a/arch/arm/boot/dts/exynos5250.dtsi
> +++ b/arch/arm/boot/dts/exynos5250.dtsi

IMHO driver and dts changes should be separated into two patches, first
updating device tree to use the new driver and second removing the driver.

After fixing this issue,

Reviewed-by: Tomasz Figa <[email protected]>

--
Best regards,
Tomasz

2014-04-09 11:41:49

by Vivek Gautam

[permalink] [raw]
Subject: Re: [PATCH V4 5/5] usb-phy: samsung-usb3: Remove older phy-samsung-usb3 driver

Hi Tomasz,
'

On Wed, Apr 9, 2014 at 4:43 PM, Tomasz Figa <[email protected]> wrote:
> Hi Vivek,
>
Thanks for reviewing the patch series.

>
> On 08.04.2014 16:36, Vivek Gautam wrote:
>>
>> Removing this older USB 3.0 DRD controller PHY driver, since
>> a new driver based on generic phy framework is now available.
>>
>> Also removing the dt node for older driver from Exynos5250
>> device tree and updating the dt node for DWC3 controller.
>>
>> Signed-off-by: Vivek Gautam <[email protected]>
>> ---
>>
>> NOTE: This patch should be merged only when the new USB 3.0
>> DRD phy controller driver is available in the tree from the
>> patches:
>> phy: Add new Exynos5 USB 3.0 PHY driver; and
>> dt: exynos5250: Enable support for generic USB DRD phy
>>
>> arch/arm/boot/dts/exynos5250.dtsi | 17 +-
>> drivers/usb/phy/phy-samsung-usb.h | 83 ---------
>> drivers/usb/phy/phy-samsung-usb3.c | 350
>> ------------------------------------
>> 3 files changed, 2 insertions(+), 448 deletions(-)
>> delete mode 100644 drivers/usb/phy/phy-samsung-usb3.c
>>
>> diff --git a/arch/arm/boot/dts/exynos5250.dtsi
>> b/arch/arm/boot/dts/exynos5250.dtsi
>> index 92c6fcd..1cb1e91 100644
>> --- a/arch/arm/boot/dts/exynos5250.dtsi
>> +++ b/arch/arm/boot/dts/exynos5250.dtsi
>
>
> IMHO driver and dts changes should be separated into two patches, first
> updating device tree to use the new driver and second removing the driver.

Sure will separate the patch into driver and dts change.

>
> After fixing this issue,
>
> Reviewed-by: Tomasz Figa <[email protected]>
>
> --
> Best regards,
> Tomasz
> --
> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

2014-04-09 11:49:57

by Vivek Gautam

[permalink] [raw]
Subject: Re: [PATCH V4 1/5] phy: Add new Exynos5 USB 3.0 PHY driver

Hi,


On Wed, Apr 9, 2014 at 4:36 PM, Tomasz Figa <[email protected]> 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 <[email protected]>
>> ---
>> .../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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

2014-04-09 13:33:17

by Tomasz Figa

[permalink] [raw]
Subject: Re: [PATCH V4 1/5] phy: Add new Exynos5 USB 3.0 PHY driver

On 09.04.2014 13:49, Vivek Gautam wrote:
> Hi,
>
>
> On Wed, Apr 9, 2014 at 4:36 PM, Tomasz Figa <[email protected]> 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 <[email protected]>
>>> ---
>>> .../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

2014-04-10 11:40:02

by Vivek Gautam

[permalink] [raw]
Subject: Re: [PATCH V4 1/5] phy: Add new Exynos5 USB 3.0 PHY driver

On Wed, Apr 9, 2014 at 7:03 PM, Tomasz Figa <[email protected]> wrote:
> On 09.04.2014 13:49, Vivek Gautam wrote:
>>
>> Hi,
>>
>>
>> On Wed, Apr 9, 2014 at 4:36 PM, Tomasz Figa <[email protected]> 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 <[email protected]>
>>>> ---
>>>> .../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.


>
> Best regards,
> Tomasz
> --
> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html



--
Best Regards
Vivek Gautam
Samsung R&D Institute, Bangalore
India

2014-04-14 11:55:40

by Kishon Vijay Abraham I

[permalink] [raw]
Subject: Re: [PATCH V4 1/5] phy: Add new Exynos5 USB 3.0 PHY driver

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 <[email protected]>
>> ---
>> .../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

2014-04-14 12:05:47

by Vivek Gautam

[permalink] [raw]
Subject: Re: [PATCH V4 1/5] phy: Add new Exynos5 USB 3.0 PHY driver

Hi Kishon,


On Mon, Apr 14, 2014 at 5:24 PM, Kishon Vijay Abraham I <[email protected]> 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 <[email protected]>
>>> ---
>>> .../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

2014-04-14 12:28:26

by Kishon Vijay Abraham I

[permalink] [raw]
Subject: Re: [PATCH V4 1/5] phy: Add new Exynos5 USB 3.0 PHY driver

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 <[email protected]>
> ---
> .../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 <[email protected]>
> + *
> + * 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

2014-04-14 12:42:37

by Vivek Gautam

[permalink] [raw]
Subject: Re: [PATCH V4 1/5] phy: Add new Exynos5 USB 3.0 PHY driver

Hi,


On Mon, Apr 14, 2014 at 5:57 PM, Kishon Vijay Abraham I <[email protected]> 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 <[email protected]>
>> ---
>> .../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 <[email protected]>
>> + *
>> + * 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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

2014-04-14 13:00:18

by Kishon Vijay Abraham I

[permalink] [raw]
Subject: Re: [PATCH V4 1/5] phy: Add new Exynos5 USB 3.0 PHY driver



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 <[email protected]> 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 <[email protected]>
>>> ---
>>> .../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

2014-04-14 13:06:06

by Kishon Vijay Abraham I

[permalink] [raw]
Subject: Re: [PATCH V4 1/5] phy: Add new Exynos5 USB 3.0 PHY driver



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 <[email protected]> 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 <[email protected]>
>>>> ---
>>>> .../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

2014-04-14 13:20:54

by Vivek Gautam

[permalink] [raw]
Subject: Re: [PATCH V4 1/5] phy: Add new Exynos5 USB 3.0 PHY driver

On Mon, Apr 14, 2014 at 6:29 PM, Kishon Vijay Abraham I <[email protected]> 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 <[email protected]> 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 <[email protected]>
>>>> ---
>>>> .../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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html



--
Best Regards
Vivek Gautam
Samsung R&D Institute, Bangalore
India

2014-04-14 13:27:27

by Kishon Vijay Abraham I

[permalink] [raw]
Subject: Re: [PATCH V4 1/5] phy: Add new Exynos5 USB 3.0 PHY driver



On Monday 14 April 2014 06:50 PM, Vivek Gautam wrote:
> On Mon, Apr 14, 2014 at 6:29 PM, Kishon Vijay Abraham I <[email protected]> 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 <[email protected]> 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 <[email protected]>
>>>>> ---
>>>>> .../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

2014-04-14 13:40:15

by Vivek Gautam

[permalink] [raw]
Subject: Re: [PATCH V4 1/5] phy: Add new Exynos5 USB 3.0 PHY driver

On Mon, Apr 14, 2014 at 6:56 PM, Kishon Vijay Abraham I <[email protected]> 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 <[email protected]> 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 <[email protected]> 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 <[email protected]>
>>>>>> ---
>>>>>> .../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

2014-04-14 13:45:06

by Tomasz Figa

[permalink] [raw]
Subject: Re: [PATCH V4 1/5] phy: Add new Exynos5 USB 3.0 PHY driver

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 <[email protected]> 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 <[email protected]>
>>>>> ---
>>>>> .../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

2014-04-14 13:46:20

by Vivek Gautam

[permalink] [raw]
Subject: Re: [PATCH V4 1/5] phy: Add new Exynos5 USB 3.0 PHY driver

On Mon, Apr 14, 2014 at 7:10 PM, Vivek Gautam <[email protected]> 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 <[email protected]> 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 <[email protected]> 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 <[email protected]> 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 <[email protected]>
>>>>>>> ---
>>>>>>> .../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



--
Best Regards
Vivek Gautam
Samsung R&D Institute, Bangalore
India

2014-04-14 13:49:25

by Vivek Gautam

[permalink] [raw]
Subject: Re: [PATCH V4 1/5] phy: Add new Exynos5 USB 3.0 PHY driver

Hi,


On Mon, Apr 14, 2014 at 7:14 PM, Tomasz Figa <[email protected]> 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 <[email protected]>
>>> 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 <[email protected]>
>>>>>> ---
>>>>>> .../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 ?


--
Best Regards
Vivek Gautam
Samsung R&D Institute, Bangalore
India

2014-04-14 13:50:03

by Tomasz Figa

[permalink] [raw]
Subject: Re: [PATCH V4 1/5] phy: Add new Exynos5 USB 3.0 PHY driver

On 14.04.2014 15:40, Vivek Gautam wrote:
> On Mon, Apr 14, 2014 at 6:56 PM, Kishon Vijay Abraham I <[email protected]> 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 <[email protected]> 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 <[email protected]> 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 <[email protected]>
>>>>>>> ---
>>>>>>> .../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

2014-04-14 14:21:44

by Sylwester Nawrocki

[permalink] [raw]
Subject: Re: [PATCH V4 1/5] phy: Add new Exynos5 USB 3.0 PHY driver

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


2014-04-14 14:37:11

by Sylwester Nawrocki

[permalink] [raw]
Subject: Re: [PATCH V4 1/5] phy: Add new Exynos5 USB 3.0 PHY driver

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,

--
Thanks,
Sylwester

2014-04-15 05:07:30

by Vivek Gautam

[permalink] [raw]
Subject: Re: [PATCH V4 1/5] phy: Add new Exynos5 USB 3.0 PHY driver

Hi Sylwester,


On Mon, Apr 14, 2014 at 7:51 PM, Sylwester Nawrocki
<[email protected]> 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.



--
Best Regards
Vivek Gautam
Samsung R&D Institute, Bangalore
India

2014-04-15 05:09:27

by Vivek Gautam

[permalink] [raw]
Subject: Re: [PATCH V4 1/5] phy: Add new Exynos5 USB 3.0 PHY driver

Hi,


On Mon, Apr 14, 2014 at 8:07 PM, Sylwester Nawrocki
<[email protected]> 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.

2014-04-15 06:09:29

by Vivek Gautam

[permalink] [raw]
Subject: Re: [PATCH V4 1/5] phy: Add new Exynos5 USB 3.0 PHY driver

Hi Tomasz,


On Thu, Apr 10, 2014 at 5:09 PM, Vivek Gautam <[email protected]> wrote:
> On Wed, Apr 9, 2014 at 7:03 PM, Tomasz Figa <[email protected]> wrote:
>> On 09.04.2014 13:49, Vivek Gautam wrote:
>>>
>>> Hi,
>>>
>>>
>>> On Wed, Apr 9, 2014 at 4:36 PM, Tomasz Figa <[email protected]> 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 <[email protected]>
>>>>> ---
>>>>> .../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.

2014-04-15 14:00:35

by Kishon Vijay Abraham I

[permalink] [raw]
Subject: Re: [PATCH V4 1/5] phy: Add new Exynos5 USB 3.0 PHY driver


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 <[email protected]> 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 <[email protected]> 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 <[email protected]>
>>>>>> 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 <[email protected]>
>>>>>>>> ---
>>>>>>>> .../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

2014-04-16 13:33:29

by Richard Genoud

[permalink] [raw]
Subject: Re: [PATCH V4 5/5] usb-phy: samsung-usb3: Remove older phy-samsung-usb3 driver

Hi Vivek,

2014-04-09 13:34 GMT+02:00 Vivek Gautam <[email protected]>:
> Hi Tomasz,
> '
>
> On Wed, Apr 9, 2014 at 4:43 PM, Tomasz Figa <[email protected]> wrote:
>> Hi Vivek,
>>
> Thanks for reviewing the patch series.
>
>>
>> On 08.04.2014 16:36, Vivek Gautam wrote:
>>>
>>> Removing this older USB 3.0 DRD controller PHY driver, since
>>> a new driver based on generic phy framework is now available.
>>>
>>> Also removing the dt node for older driver from Exynos5250
>>> device tree and updating the dt node for DWC3 controller.
>>>
>>> Signed-off-by: Vivek Gautam <[email protected]>
>>> ---
>>>
>>> NOTE: This patch should be merged only when the new USB 3.0
>>> DRD phy controller driver is available in the tree from the
>>> patches:
>>> phy: Add new Exynos5 USB 3.0 PHY driver; and
>>> dt: exynos5250: Enable support for generic USB DRD phy
>>>
>>> arch/arm/boot/dts/exynos5250.dtsi | 17 +-
>>> drivers/usb/phy/phy-samsung-usb.h | 83 ---------
>>> drivers/usb/phy/phy-samsung-usb3.c | 350
>>> ------------------------------------
>>> 3 files changed, 2 insertions(+), 448 deletions(-)
>>> delete mode 100644 drivers/usb/phy/phy-samsung-usb3.c
>>>
>>> diff --git a/arch/arm/boot/dts/exynos5250.dtsi
>>> b/arch/arm/boot/dts/exynos5250.dtsi
>>> index 92c6fcd..1cb1e91 100644
>>> --- a/arch/arm/boot/dts/exynos5250.dtsi
>>> +++ b/arch/arm/boot/dts/exynos5250.dtsi
>>
>>
>> IMHO driver and dts changes should be separated into two patches, first
>> updating device tree to use the new driver and second removing the driver.
>
> Sure will separate the patch into driver and dts change.
>
>>
>> After fixing this issue,
>>
>> Reviewed-by: Tomasz Figa <[email protected]>
>>
I guess you should also remove phy-samsung-usb3.c references in the
Makefile, Kconfig and exynos_defconfig

Richard.

2014-04-16 13:44:20

by Tomasz Figa

[permalink] [raw]
Subject: Re: [PATCH V4 1/5] phy: Add new Exynos5 USB 3.0 PHY driver

Hi Vivek,

On 15.04.2014 08:09, Vivek Gautam wrote:
> Hi Tomasz,
>
>
> On Thu, Apr 10, 2014 at 5:09 PM, Vivek Gautam <[email protected]> wrote:
>> On Wed, Apr 9, 2014 at 7:03 PM, Tomasz Figa <[email protected]> wrote:
>>> On 09.04.2014 13:49, Vivek Gautam wrote:
>>>>
>>>> Hi,
>>>>
>>>>
>>>> On Wed, Apr 9, 2014 at 4:36 PM, Tomasz Figa <[email protected]> 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 <[email protected]>
>>>>>> ---
>>>>>> .../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

2014-04-16 14:43:04

by Vivek Gautam

[permalink] [raw]
Subject: Re: [PATCH V4 5/5] usb-phy: samsung-usb3: Remove older phy-samsung-usb3 driver

Hi Richard,


On Wed, Apr 16, 2014 at 7:03 PM, Richard Genoud
<[email protected]> wrote:
> Hi Vivek,
>
> 2014-04-09 13:34 GMT+02:00 Vivek Gautam <[email protected]>:
>> Hi Tomasz,
>> '
>>
>> On Wed, Apr 9, 2014 at 4:43 PM, Tomasz Figa <[email protected]> wrote:
>>> Hi Vivek,
>>>
>> Thanks for reviewing the patch series.
>>
>>>
>>> On 08.04.2014 16:36, Vivek Gautam wrote:
>>>>
>>>> Removing this older USB 3.0 DRD controller PHY driver, since
>>>> a new driver based on generic phy framework is now available.
>>>>
>>>> Also removing the dt node for older driver from Exynos5250
>>>> device tree and updating the dt node for DWC3 controller.
>>>>
>>>> Signed-off-by: Vivek Gautam <[email protected]>
>>>> ---
>>>>
>>>> NOTE: This patch should be merged only when the new USB 3.0
>>>> DRD phy controller driver is available in the tree from the
>>>> patches:
>>>> phy: Add new Exynos5 USB 3.0 PHY driver; and
>>>> dt: exynos5250: Enable support for generic USB DRD phy
>>>>
>>>> arch/arm/boot/dts/exynos5250.dtsi | 17 +-
>>>> drivers/usb/phy/phy-samsung-usb.h | 83 ---------
>>>> drivers/usb/phy/phy-samsung-usb3.c | 350
>>>> ------------------------------------
>>>> 3 files changed, 2 insertions(+), 448 deletions(-)
>>>> delete mode 100644 drivers/usb/phy/phy-samsung-usb3.c
>>>>
>>>> diff --git a/arch/arm/boot/dts/exynos5250.dtsi
>>>> b/arch/arm/boot/dts/exynos5250.dtsi
>>>> index 92c6fcd..1cb1e91 100644
>>>> --- a/arch/arm/boot/dts/exynos5250.dtsi
>>>> +++ b/arch/arm/boot/dts/exynos5250.dtsi
>>>
>>>
>>> IMHO driver and dts changes should be separated into two patches, first
>>> updating device tree to use the new driver and second removing the driver.
>>
>> Sure will separate the patch into driver and dts change.
>>
>>>
>>> After fixing this issue,
>>>
>>> Reviewed-by: Tomasz Figa <[email protected]>
>>>
> I guess you should also remove phy-samsung-usb3.c references in the
> Makefile, Kconfig and exynos_defconfig

You are correct. In fact i identified this issue once i submitted the patch :-(

I have amended the things in my next series which i will be posting soon.



Thanks
Vivek

2014-04-16 14:49:22

by Vivek Gautam

[permalink] [raw]
Subject: Re: [PATCH V4 1/5] phy: Add new Exynos5 USB 3.0 PHY driver

Hi,


On Wed, Apr 16, 2014 at 7:14 PM, Tomasz Figa <[email protected]> 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 <[email protected]>
>> wrote:
>>>
>>> On Wed, Apr 9, 2014 at 7:03 PM, Tomasz Figa <[email protected]> wrote:
>>>>
>>>> On 09.04.2014 13:49, Vivek Gautam wrote:
>>>>>
>>>>>
>>>>> Hi,
>>>>>
>>>>>
>>>>> On Wed, Apr 9, 2014 at 4:36 PM, Tomasz Figa <[email protected]> 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 <[email protected]>
>>>>>>> ---
>>>>>>> .../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

2014-04-22 02:18:48

by Jingoo Han

[permalink] [raw]
Subject: Re: [PATCH V4 1/5] phy: Add new Exynos5 USB 3.0 PHY driver

On Wednesday, April 16, 2014 11:49 PM, Vivek Gautam wrote:
> On Wed, Apr 16, 2014 at 7:14 PM, Tomasz Figa <[email protected]> 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 <[email protected]> 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

2014-04-22 03:35:36

by Vivek Gautam

[permalink] [raw]
Subject: Re: [PATCH V4 1/5] phy: Add new Exynos5 USB 3.0 PHY driver

Hi Jingoo,


On Tue, Apr 22, 2014 at 7:48 AM, Jingoo Han <[email protected]> wrote:
> On Wednesday, April 16, 2014 11:49 PM, Vivek Gautam wrote:
>> On Wed, Apr 16, 2014 at 7:14 PM, Tomasz Figa <[email protected]> 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 <[email protected]> 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.

2014-04-25 07:57:52

by Tushar Behera

[permalink] [raw]
Subject: Re: [PATCH V4 1/5] phy: Add new Exynos5 USB 3.0 PHY driver

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.

--
Tushar Behera

2014-04-25 08:08:54

by Vivek Gautam

[permalink] [raw]
Subject: Re: [PATCH V4 1/5] phy: Add new Exynos5 USB 3.0 PHY driver

Hi,


On Fri, Apr 25, 2014 at 1:27 PM, Tushar Behera <[email protected]> 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.


--
Best Regards
Vivek Gautam
Samsung R&D Institute, Bangalore
India