hi, On Friday 07 July 2017 06:37 PM, Srinath Mannam wrote: > USB phy driver for stingray USB DRDU3 and DRDU2/USB3H > phy modules > > Signed-off-by: Srinath Mannam <srinath.mannam@xxxxxxxxxxxx> > Reviewed-by: Ray Jui <ray.jui@xxxxxxxxxxxx> > Reviewed-by: Scott Branden <scott.branden@xxxxxxxxxxxx> > --- > drivers/phy/broadcom/Kconfig | 13 + > drivers/phy/broadcom/Makefile | 1 + > drivers/phy/broadcom/phy-bcm-stingray-usb.c | 722 ++++++++++++++++++++++++++++ > 3 files changed, 736 insertions(+) > create mode 100644 drivers/phy/broadcom/phy-bcm-stingray-usb.c > > diff --git a/drivers/phy/broadcom/Kconfig b/drivers/phy/broadcom/Kconfig > index 37371b8..185a84b 100644 > --- a/drivers/phy/broadcom/Kconfig > +++ b/drivers/phy/broadcom/Kconfig > @@ -58,6 +58,19 @@ config PHY_NS2_USB_DRD > > If unsure, say N. > > +config PHY_STINGRAY_USB > + tristate "Broadcom Stingray USB2 DRD PHY driver" > + depends on OF && (ARCH_BCM_IPROC || COMPILE_TEST) > + select GENERIC_PHY > + default ARCH_BCM_IPROC > + help > + Enable this to support the Broadcom Stingray USB PHY for DRDU3, > + DRDU2 and USB3H. > + This driver initializes the PHY which can be used in both > + Host and Device modes. > + > + If unsure, say N. > + > config PHY_BRCM_SATA > tristate "Broadcom SATA PHY driver" > depends on ARCH_BRCMSTB || ARCH_BCM_IPROC || BMIPS_GENERIC || COMPILE_TEST > diff --git a/drivers/phy/broadcom/Makefile b/drivers/phy/broadcom/Makefile > index 4eb82ec..78c1033 100644 > --- a/drivers/phy/broadcom/Makefile > +++ b/drivers/phy/broadcom/Makefile > @@ -4,4 +4,5 @@ obj-$(CONFIG_PHY_BCM_NS_USB2) += phy-bcm-ns-usb2.o > obj-$(CONFIG_PHY_BCM_NS_USB3) += phy-bcm-ns-usb3.o > obj-$(CONFIG_PHY_NS2_PCIE) += phy-bcm-ns2-pcie.o > obj-$(CONFIG_PHY_NS2_USB_DRD) += phy-bcm-ns2-usbdrd.o > +obj-$(CONFIG_PHY_STINGRAY_USB) += phy-bcm-stingray-usb.o > obj-$(CONFIG_PHY_BRCM_SATA) += phy-brcm-sata.o > diff --git a/drivers/phy/broadcom/phy-bcm-stingray-usb.c b/drivers/phy/broadcom/phy-bcm-stingray-usb.c > new file mode 100644 > index 0000000..03603fd > --- /dev/null > +++ b/drivers/phy/broadcom/phy-bcm-stingray-usb.c > @@ -0,0 +1,722 @@ > +/* > + * Copyright (C) 2017 Broadcom > + * > + * This program is free software; you can redistribute it and/or > + * modify it under the terms of the GNU General Public License as > + * published by the Free Software Foundation version 2. > + * > + * This program is distributed "as is" WITHOUT ANY WARRANTY of any > + * kind, whether express or implied; without even the implied warranty > + * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + */ > + > +#include <linux/delay.h> > +#include <linux/io.h> > +#include <linux/module.h> > +#include <linux/of.h> > +#include <linux/phy/phy.h> > +#include <linux/platform_device.h> > + > +#define DRDU2_U2PLL_NDIV_FRAC 0x0 > + > +#define DRDU2_U2PLL_NDIV_INT 0x4 > + > +#define DRDU2_U2PLL_CTRL 0x8 > +#define DRDU2_U2PLL_LOCK BIT(6) > +#define DRDU2_U2PLL_RESETB BIT(5) > +#define DRDU2_U2PLL_PDIV_MASK 0xF > +#define DRDU2_U2PLL_PDIV_OFFSET 1 > +#define DRDU2_U2PLL_SUSPEND_EN BIT(0) > + > +#define DRDU2_PHY_CTRL 0x0C > +#define DRDU2_U2IDDQ BIT(30) > +#define DRDU2_U2SOFT_RST_N BIT(29) > +#define DRDU2_U2PHY_ON_FLAG BIT(22) > +#define DRDU2_U2PHY_PCTL_MASK 0xFFFF > +#define DRDU2_U2PHY_PCTL_OFFSET 6 > +#define DRDU2_U2PHY_RESETB BIT(5) > +#define DRDU2_U2PHY_ISO BIT(4) > +#define DRDU2_U2AFE_BG_PWRDWNB BIT(3) > +#define DRDU2_U2AFE_PLL_PWRDWNB BIT(2) > +#define DRDU2_U2AFE_LDO_PWRDWNB BIT(1) > +#define DRDU2_U2CTRL_CORERDY BIT(0) > + > +#define DRDU2_STRAP_CTRL 0x18 > +#define DRDU2_FORCE_HOST_MODE BIT(5) > +#define DRDU2_FORCE_DEVICE_MODE BIT(4) > +#define BDC_USB_STP_SPD_MASK 0x7 > +#define BDC_USB_STP_SPD_OFFSET 0 > + > +#define DRDU2_PWR_CTRL 0x1C > +#define DRDU2_U2PHY_DFE_SWITCH_PWROKIN_I BIT(2) > +#define DRDU2_U2PHY_DFE_SWITCH_PWRONIN_I BIT(1) > + > +#define DRDU2_SOFT_RESET_CTRL 0x20 > +#define DRDU2_BDC_AXI_SOFT_RST_N BIT(0) > + > +#define USB3H_U2PLL_NDIV_FRAC 0x4 > + > +#define USB3H_U2PLL_NDIV_INT 0x8 > + > +#define USB3H_U2PLL_CTRL 0xC > +#define USB3H_U2PLL_LOCK BIT(6) > +#define USB3H_U2PLL_RESETB BIT(5) > +#define USB3H_U2PLL_PDIV_MASK 0xF > +#define USB3H_U2PLL_PDIV_OFFSET 1 > + > +#define USB3H_U2PHY_CTRL 0x10 > +#define USB3H_U2PHY_ON_FLAG 22 > +#define USB3H_U2PHY_PCTL_MASK 0xFFFF > +#define USB3H_U2PHY_PCTL_OFFSET 6 > +#define USB3H_U2PHY_RESETB BIT(5) > +#define USB3H_U2PHY_ISO BIT(4) > +#define USB3H_U2AFE_BG_PWRDWNB BIT(3) > +#define USB3H_U2AFE_PLL_PWRDWNB BIT(2) > +#define USB3H_U2AFE_LDO_PWRDWNB BIT(1) > +#define USB3H_U2CTRL_CORERDY BIT(0) > + > +#define USB3H_U3PHY_CTRL 0x14 > +#define USB3H_U3SOFT_RST_N BIT(30) > +#define USB3H_U3MDIO_RESETB_I BIT(29) > +#define USB3H_U3POR_RESET_I BIT(28) > +#define USB3H_U3PHY_PCTL_MASK 0xFFFF > +#define USB3H_U3PHY_PCTL_OFFSET 2 > +#define USB3H_U3PHY_RESETB BIT(1) > + > +#define USB3H_U3PHY_PLL_CTRL 0x18 > +#define USB3H_U3PLL_REFCLK_MASK 0x7 > +#define USB3H_U3PLL_REFCLK_OFFSET 4 > +#define USB3H_U3PLL_SS_LOCK BIT(3) > +#define USB3H_U3PLL_SEQ_START BIT(2) > +#define USB3H_U3SSPLL_SUSPEND_EN BIT(1) > +#define USB3H_U3PLL_RESETB BIT(0) > + > +#define USB3H_PWR_CTRL 0x28 > +#define USB3H_PWR_CTRL_OVERRIDE_I_R 4 > + > +#define USB3H_SOFT_RESET_CTRL 0x2C > +#define USB3H_XHC_AXI_SOFT_RST_N BIT(1) > + > +#define USB3H_PHY_PWR_CTRL 0x38 > +#define USB3H_DISABLE_USB30_P0 BIT(2) > +#define USB3H_DISABLE_EUSB_P1 BIT(1) > +#define USB3H_DISABLE_EUSB_P0 BIT(0) > + > + > +#define DRDU3_U2PLL_NDIV_FRAC 0x4 > + > +#define DRDU3_U2PLL_NDIV_INT 0x8 > + > +#define DRDU3_U2PLL_CTRL 0xC > +#define DRDU3_U2PLL_LOCK BIT(6) > +#define DRDU3_U2PLL_RESETB BIT(5) > +#define DRDU3_U2PLL_PDIV_MASK 0xF > +#define DRDU3_U2PLL_PDIV_OFFSET 1 > + > +#define DRDU3_U2PHY_CTRL 0x10 > +#define DRDU3_U2PHY_ON_FLAG BIT(22) > +#define DRDU3_U2PHY_PCTL_MASK 0xFFFF > +#define DRDU3_U2PHY_PCTL_OFFSET 6 > +#define DRDU3_U2PHY_RESETB BIT(5) > +#define DRDU3_U2PHY_ISO BIT(4) > +#define DRDU3_U2AFE_BG_PWRDWNB BIT(3) > +#define DRDU3_U2AFE_PLL_PWRDWNB BIT(2) > +#define DRDU3_U2AFE_LDO_PWRDWNB BIT(1) > +#define DRDU3_U2CTRL_CORERDY BIT(0) > + > +#define DRDU3_U3PHY_CTRL 0x14 > +#define DRDU3_U3XHC_SOFT_RST_N BIT(31) > +#define DRDU3_U3BDC_SOFT_RST_N BIT(30) > +#define DRDU3_U3MDIO_RESETB_I BIT(29) > +#define DRDU3_U3POR_RESET_I BIT(28) > +#define DRDU3_U3PHY_PCTL_MASK 0xFFFF > +#define DRDU3_U3PHY_PCTL_OFFSET 2 > +#define DRDU3_U3PHY_RESETB BIT(1) > + > +#define DRDU3_U3PHY_PLL_CTRL 0x18 > +#define DRDU3_U3PLL_REFCLK_MASK 0x7 > +#define DRDU3_U3PLL_REFCLK_OFFSET 4 > +#define DRDU3_U3PLL_SS_LOCK BIT(3) > +#define DRDU3_U3PLL_SEQ_START BIT(2) > +#define DRDU3_U3SSPLL_SUSPEND_EN BIT(1) > +#define DRDU3_U3PLL_RESETB BIT(0) > + > +#define DRDU3_STRAP_CTRL 0x28 > +#define BDC_USB_STP_SPD_MASK 0x7 > +#define BDC_USB_STP_SPD_OFFSET 0 > +#define BDC_USB_STP_SPD_SS 0x0 > +#define BDC_USB_STP_SPD_HS 0x2 > + > +#define DRDU3_PWR_CTRL 0x2c > +#define DRDU3_PWR_CTRL_OVERRIDE_I_R 4 > + > +#define DRDU3_SOFT_RESET_CTRL 0x30 > +#define DRDU3_XHC_AXI_SOFT_RST_N BIT(1) > +#define DRDU3_BDC_AXI_SOFT_RST_N BIT(0) > + > +#define DRDU3_PHY_PWR_CTRL 0x3c > +#define DRDU3_DISABLE_USB30_P0 BIT(2) > +#define DRDU3_DISABLE_EUSB_P1 BIT(1) > +#define DRDU3_DISABLE_EUSB_P0 BIT(0) > + > +#define PLL_REFCLK_PAD 0x0 > +#define PLL_REFCLK_25MHZ 0x1 > +#define PLL_REFCLK_96MHZ 0x2 > +#define PLL_REFCLK_INTERNAL 0x3 > +#define PLL_LOCK_RETRY_COUNT 1000 > + > + > +#define U2PLL_NDIV_INT_VAL 0x13 > +#define U2PLL_NDIV_FRAC_VAL 0x1005 > +#define U2PLL_PDIV_VAL 0x1 > +/* > + * 0x0806 of PCTL_VAL has below bits set > + * BIT-8 : refclk divider 1 > + * BIT-3:2: device mode; mode is not effect > + * BIT-1: soft reset active low > + */ > +#define U2PHY_PCTL_VAL 0x0806 > +#define U3PHY_PCTL_VAL 0x0006 > + > +#define MAX_NR_PORTS 3 > + > +#define USB3H_DRDU2_PHY 1 > +#define DRDU3_PHY 2 > + > +#define USB_HOST_MODE 1 > +#define USB_DEV_MODE 2 > + > +#define USB3SS_PORT 0 > +#define DRDU2_PORT 1 > +#define USB3HS_PORT 2 > + > +#define DRD3SS_PORT 0 > +#define DRD3HS_PORT 1 > + > +struct sr_usb_phy { > + void __iomem *drdu2reg; > + void __iomem *usb3hreg; > + void __iomem *drdu3reg; > + uint32_t phy_id; > + struct sr_usb_phy_port *phy_port; > +}; > + > +struct sr_usb_phy_port { > + uint32_t port_id; > + uint32_t mode; > + struct phy *gphy; > + struct sr_usb_phy *p; > +}; > + > +static inline void reg32_clrbits(void __iomem *addr, uint32_t clear) > +{ > + writel(readl(addr) & ~clear, addr); > +} > + > +static inline void reg32_setbits(void __iomem *addr, uint32_t set) > +{ > + writel(readl(addr) | set, addr); > +} > + > +static int pll_lock_check(void __iomem *addr, u32 bit) > +{ > + int retry; > + u32 rd_data; > + > + retry = PLL_LOCK_RETRY_COUNT; > + do { > + rd_data = readl(addr); > + if (rd_data & bit) > + return 0; > + udelay(1); > + } while (--retry > 0); > + > + pr_err("%s: FAIL\n", __func__); > + return -ETIMEDOUT; > +} It would be nice if you can prefix all functions with bcm_usb. > + > +static struct phy *bcm_usb_phy_xlate(struct device *dev, > + struct of_phandle_args *args) > +{ > + struct sr_usb_phy_port *phy_port = dev_get_drvdata(dev); > + struct sr_usb_phy *p; > + > + if (!phy_port) > + return ERR_PTR(-EINVAL); > + > + p = phy_port->p; > + > + if (WARN_ON(args->args[0] < 1 || args->args[0] > 2)) Please use constant enums. I think here it means PHY_MODE_USB_HOST/PHY_MODE_USB_DEVICE? > + return ERR_PTR(-EINVAL); > + > + if (WARN_ON(args->args_count < 1)) > + return ERR_PTR(-EINVAL); > + > + if ((p->phy_id == USB3H_DRDU2_PHY) && (phy_port->port_id == 0) && > + (args->args[0] == USB_DEV_MODE)) > + return ERR_PTR(-EINVAL); port 0 cannot be used for host mode? How about other ports? > + > + phy_port->mode |= args->args[0]; When is this mode used? If mode is never used, then this custom xlate is not required at all. > + > + return phy_port->gphy; > +} > + > +static int usb3h_u2_phy_power_on(void __iomem *regs) > +{ > + u32 rd_data; > + int ret = 0; > + > + /* Set Core Ready high */ > + reg32_setbits(regs + USB3H_U2PHY_CTRL, USB3H_U2CTRL_CORERDY); > + msleep(100); That looks like a huge delay? Is it really necessary > + > + rd_data = readl(regs + USB3H_U2PHY_CTRL); > + rd_data &= ~(USB3H_U2PHY_PCTL_MASK << USB3H_U2PHY_PCTL_OFFSET); > + rd_data |= (U2PHY_PCTL_VAL << USB3H_U2PHY_PCTL_OFFSET); > + writel(rd_data, regs + USB3H_U2PHY_CTRL); > + > + msleep(300); one more here. > + reg32_setbits(regs + USB3H_U2PLL_CTRL, > + USB3H_U2PLL_RESETB); > + > + reg32_setbits(regs + USB3H_U2PHY_CTRL, > + USB3H_U2PHY_RESETB); > + > + ret = pll_lock_check(regs + USB3H_U2PLL_CTRL, USB3H_U2PLL_LOCK); > + > + return ret; > +} > + > + > +static int usb3h_u3_phy_power_on(void __iomem *regs) > +{ > + u32 rd_data; > + int ret = 0; > + > + /* Set pctl with mode and soft reset */ > + rd_data = readl(regs + USB3H_U3PHY_CTRL); > + rd_data &= ~(USB3H_U3PHY_PCTL_MASK << USB3H_U3PHY_PCTL_OFFSET); > + rd_data |= (U3PHY_PCTL_VAL << USB3H_U3PHY_PCTL_OFFSET); > + writel(rd_data, regs + USB3H_U3PHY_CTRL); > + > + reg32_clrbits(regs + USB3H_U3PHY_PLL_CTRL, > + USB3H_U3SSPLL_SUSPEND_EN); > + reg32_setbits(regs + USB3H_U3PHY_PLL_CTRL, > + USB3H_U3PLL_SEQ_START); > + reg32_setbits(regs + USB3H_U3PHY_PLL_CTRL, > + USB3H_U3PLL_RESETB); > + > + msleep(30); > + > + ret = pll_lock_check(regs + USB3H_U3PHY_PLL_CTRL, USB3H_U3PLL_SS_LOCK); > + > + return ret; > +} > + > + > +static int drdu3_u2_phy_power_on(void __iomem *regs) > +{ > + u32 rd_data; > + int ret = 0; > + > + /* Set Core Ready high */ > + reg32_setbits(regs + DRDU3_U2PHY_CTRL, DRDU3_U2CTRL_CORERDY); > + > + msleep(100); > + > + rd_data = readl(regs + DRDU3_STRAP_CTRL); > + rd_data &= (~(BDC_USB_STP_SPD_MASK << BDC_USB_STP_SPD_OFFSET)); > + rd_data |= (BDC_USB_STP_SPD_SS << BDC_USB_STP_SPD_OFFSET); > + writel(rd_data, regs + DRDU3_STRAP_CTRL); > + > + rd_data = readl(regs + DRDU3_U2PHY_CTRL); > + rd_data &= ~(DRDU3_U2PHY_PCTL_MASK << DRDU3_U2PHY_PCTL_OFFSET); > + rd_data |= (U2PHY_PCTL_VAL << DRDU3_U2PHY_PCTL_OFFSET); > + writel(rd_data, regs + DRDU3_U2PHY_CTRL); > + > + msleep(300); > + > + reg32_setbits(regs + DRDU3_U2PLL_CTRL, > + DRDU3_U2PLL_RESETB); > + > + reg32_setbits(regs + DRDU3_U2PHY_CTRL, > + DRDU3_U2PHY_RESETB); > + > + ret = pll_lock_check(regs + DRDU3_U2PLL_CTRL, DRDU3_U2PLL_LOCK); > + > + return ret; > +} > + > +static int drdu3_u3_phy_power_on(void __iomem *regs) > +{ > + u32 rd_data; > + int ret = 0; > + > + /* Set pctl with mode and soft reset */ > + rd_data = readl(regs + DRDU3_U3PHY_CTRL); > + rd_data &= ~(DRDU3_U3PHY_PCTL_MASK << DRDU3_U3PHY_PCTL_OFFSET); > + rd_data |= (U3PHY_PCTL_VAL << DRDU3_U3PHY_PCTL_OFFSET); > + writel(rd_data, regs + DRDU3_U3PHY_CTRL); > + > + reg32_clrbits(regs + DRDU3_U3PHY_PLL_CTRL, > + DRDU3_U3SSPLL_SUSPEND_EN); > + reg32_setbits(regs + DRDU3_U3PHY_PLL_CTRL, > + DRDU3_U3PLL_SEQ_START); > + reg32_setbits(regs + DRDU3_U3PHY_PLL_CTRL, > + DRDU3_U3PLL_RESETB); > + > + msleep(30); > + > + ret = pll_lock_check(regs + DRDU3_U3PHY_PLL_CTRL, DRDU3_U3PLL_SS_LOCK); > + > + return ret; > +} > + > + > +static int drdu2_u2_phy_power_on(void __iomem *regs) > +{ > + u32 rd_data; > + int ret = 0; > + > + writel(U2PLL_NDIV_INT_VAL, regs + DRDU2_U2PLL_NDIV_INT); > + writel(U2PLL_NDIV_FRAC_VAL, regs + DRDU2_U2PLL_NDIV_FRAC); > + > + rd_data = readl(regs + DRDU2_U2PLL_CTRL); > + rd_data &= ~(DRDU2_U2PLL_PDIV_MASK << DRDU2_U2PLL_PDIV_OFFSET); > + rd_data |= (U2PLL_PDIV_VAL << DRDU2_U2PLL_PDIV_OFFSET); > + writel(rd_data, regs + DRDU2_U2PLL_CTRL); > + > + reg32_setbits(regs + DRDU2_PHY_CTRL, DRDU2_U2CTRL_CORERDY); > + > + msleep(100); > + reg32_setbits(regs + DRDU2_U2PLL_CTRL, > + DRDU2_U2PLL_RESETB); > + > + reg32_setbits(regs + DRDU2_PHY_CTRL, > + DRDU2_U2PHY_RESETB); > + > + rd_data = readl(regs + DRDU2_PHY_CTRL); > + rd_data &= ~(DRDU2_U2PHY_PCTL_MASK << DRDU2_U2PHY_PCTL_OFFSET); > + rd_data |= (U2PHY_PCTL_VAL << DRDU2_U2PHY_PCTL_OFFSET); > + writel(rd_data, regs + DRDU2_PHY_CTRL); > + > + ret = pll_lock_check(regs + DRDU2_U2PLL_CTRL, DRDU2_U2PLL_LOCK); > + > + return ret; > +} > +static int sr_u3h_u2drd_phy_reset(struct phy *gphy) > +{ > + struct sr_usb_phy_port *phy_port = phy_get_drvdata(gphy); > + struct sr_usb_phy *p = phy_port->p; > + > + switch (phy_port->port_id) { > + case USB3HS_PORT: > + reg32_clrbits(p->usb3hreg + USB3H_U2PHY_CTRL, > + USB3H_U2CTRL_CORERDY); > + reg32_setbits(p->usb3hreg + USB3H_U2PHY_CTRL, > + USB3H_U2CTRL_CORERDY); > + break; > + case DRDU2_PORT: > + reg32_clrbits(p->drdu2reg + DRDU2_PHY_CTRL, > + DRDU2_U2CTRL_CORERDY); > + reg32_setbits(p->drdu2reg + DRDU2_PHY_CTRL, > + DRDU2_U2CTRL_CORERDY); > + break; > + } > + return 0; > +} > + > +static int sr_u3drd_phy_reset(struct phy *gphy) > +{ > + struct sr_usb_phy_port *phy_port = phy_get_drvdata(gphy); > + struct sr_usb_phy *p = phy_port->p; > + > + switch (phy_port->port_id) { > + case DRD3HS_PORT: > + reg32_clrbits(p->drdu3reg + DRDU3_U2PHY_CTRL, > + DRDU3_U2CTRL_CORERDY); > + reg32_setbits(p->drdu3reg + DRDU3_U2PHY_CTRL, > + DRDU3_U2CTRL_CORERDY); > + break; > + } > + return 0; > +} > + > +static int sr_u3h_u2drd_phy_power_on(struct phy *gphy) > +{ > + struct sr_usb_phy_port *phy_port = phy_get_drvdata(gphy); > + struct sr_usb_phy *p = phy_port->p; > + int ret = 0; > + > + switch (phy_port->port_id) { > + case USB3SS_PORT: > + reg32_clrbits(p->usb3hreg + USB3H_PHY_PWR_CTRL, > + USB3H_DISABLE_USB30_P0); > + ret = usb3h_u3_phy_power_on(p->usb3hreg); > + if (ret) > + goto err_usb3h_phy_on; > + break; > + case USB3HS_PORT: > + reg32_clrbits(p->usb3hreg + USB3H_PHY_PWR_CTRL, > + USB3H_DISABLE_EUSB_P1); > + ret = usb3h_u2_phy_power_on(p->usb3hreg); > + if (ret) > + goto err_usb3h_phy_on; > + > + break; > + case DRDU2_PORT: > + reg32_clrbits(p->usb3hreg + USB3H_PHY_PWR_CTRL, > + USB3H_DISABLE_EUSB_P0); > + > + ret = drdu2_u2_phy_power_on(p->drdu2reg); > + if (ret) { > + reg32_setbits(p->usb3hreg + USB3H_PHY_PWR_CTRL, > + USB3H_DISABLE_EUSB_P0); > + goto err_drdu2_phy_on; > + } > + break; > + } > + > + /* Device Mode */ > + if (phy_port->port_id == DRDU2_PORT) { > + writel(DRDU2_BDC_AXI_SOFT_RST_N, > + p->drdu2reg + DRDU2_SOFT_RESET_CTRL); > + reg32_setbits(p->drdu2reg + DRDU2_PHY_CTRL, > + DRDU2_U2SOFT_RST_N); > + } > + /* Host Mode */ > + writel(USB3H_XHC_AXI_SOFT_RST_N, > + p->usb3hreg + USB3H_SOFT_RESET_CTRL); > + reg32_setbits(p->usb3hreg + USB3H_U3PHY_CTRL, > + USB3H_U3SOFT_RST_N); > + > + return 0; > +err_usb3h_phy_on: > + reg32_setbits(p->usb3hreg + USB3H_PHY_PWR_CTRL, > + (USB3H_DISABLE_EUSB_P1 | > + USB3H_DISABLE_USB30_P0)); > +err_drdu2_phy_on: > + > + return ret; > +} > + > +static int sr_u3drd_phy_power_on(struct phy *gphy) > +{ > + struct sr_usb_phy_port *phy_port = phy_get_drvdata(gphy); > + struct sr_usb_phy *p = phy_port->p; > + int ret = 0; > + > + switch (phy_port->port_id) { > + case DRD3SS_PORT: > + reg32_clrbits(p->drdu3reg + USB3H_PHY_PWR_CTRL, > + DRDU3_DISABLE_USB30_P0); > + > + ret = drdu3_u3_phy_power_on(p->drdu3reg); > + if (ret) > + goto err_drdu3_phy_on; > + break; > + case DRD3HS_PORT: > + reg32_clrbits(p->drdu3reg + USB3H_PHY_PWR_CTRL, > + DRDU3_DISABLE_EUSB_P0); > + ret = drdu3_u2_phy_power_on(p->drdu3reg); > + if (ret) > + goto err_drdu3_phy_on; > + break; > + } > + /* Host Mode */ > + reg32_setbits(p->drdu3reg + DRDU3_SOFT_RESET_CTRL, > + DRDU3_XHC_AXI_SOFT_RST_N); > + reg32_setbits(p->drdu3reg + DRDU3_U3PHY_CTRL, > + DRDU3_U3XHC_SOFT_RST_N); > + /* Device Mode */ > + reg32_setbits(p->drdu3reg + DRDU3_SOFT_RESET_CTRL, > + DRDU3_BDC_AXI_SOFT_RST_N); > + reg32_setbits(p->drdu3reg + DRDU3_U3PHY_CTRL, > + DRDU3_U3BDC_SOFT_RST_N); > + > + return 0; > +err_drdu3_phy_on: > + reg32_setbits(p->drdu3reg + DRDU3_PHY_PWR_CTRL, > + (DRDU3_DISABLE_EUSB_P0 | > + DRDU3_DISABLE_USB30_P0)); > + > + return ret; > +} > + > +static int sr_u3h_u2drd_phy_power_off(struct phy *gphy) > +{ > + struct sr_usb_phy_port *phy_port = phy_get_drvdata(gphy); > + struct sr_usb_phy *p = phy_port->p; > + > + switch (phy_port->port_id) { > + case USB3SS_PORT: > + reg32_setbits(p->usb3hreg + USB3H_PHY_PWR_CTRL, > + USB3H_DISABLE_USB30_P0); > + break; > + case USB3HS_PORT: > + reg32_setbits(p->usb3hreg + USB3H_PHY_PWR_CTRL, > + USB3H_DISABLE_EUSB_P1); > + break; > + case DRDU2_PORT: > + reg32_setbits(p->usb3hreg + USB3H_PHY_PWR_CTRL, > + USB3H_DISABLE_EUSB_P0); > + break; > + } > + > + return 0; > +} > + > +static int sr_u3drd_phy_power_off(struct phy *gphy) > +{ > + struct sr_usb_phy_port *phy_port = phy_get_drvdata(gphy); > + struct sr_usb_phy *p = phy_port->p; > + > + switch (phy_port->port_id) { > + case DRD3SS_PORT: > + reg32_setbits(p->drdu3reg + DRDU3_PHY_PWR_CTRL, > + DRDU3_DISABLE_USB30_P0); > + break; > + case DRD3HS_PORT: > + reg32_setbits(p->drdu3reg + DRDU3_PHY_PWR_CTRL, > + DRDU3_DISABLE_EUSB_P0); > + break; > + } > + return 0; > +} > + > +static struct phy_ops sr_u3h_u2drd_phy_ops = { > + .power_on = sr_u3h_u2drd_phy_power_on, > + .power_off = sr_u3h_u2drd_phy_power_off, > + .reset = sr_u3h_u2drd_phy_reset, > + .owner = THIS_MODULE, > +}; > + > +static struct phy_ops sr_u3drd_phy_ops = { > + .power_on = sr_u3drd_phy_power_on, > + .power_off = sr_u3drd_phy_power_off, > + .reset = sr_u3drd_phy_reset, > + .owner = THIS_MODULE, > +}; Can you split this patch into 2 so that it's easier to review. 1st patch add u2drd_phy and then add u3drd_phy. > + > +static const struct of_device_id sr_usb_phy_of_match[] = { > + { > + .compatible = "brcm,sr-u3h-u2drd-phy", > + .data = &sr_u3h_u2drd_phy_ops, > + }, > + { > + .compatible = "brcm,sr-u3drd-phy", > + .data = &sr_u3drd_phy_ops, > + }, > + { /* sentinel */ }, > +}; > +MODULE_DEVICE_TABLE(of, sr_usb_phy_of_match); > + > +static int sr_usb_phy_probe(struct platform_device *pdev) > +{ > + struct device *dev = &pdev->dev; > + struct sr_usb_phy *p; > + struct device_node *dn = dev->of_node, *child; > + struct resource *res; > + struct phy *gphy; > + const struct phy_ops *ops; > + struct phy_provider *phy_provider; > + const struct of_device_id *match; > + struct sr_usb_phy_port *phy_port; > + uint32_t child_cnt, cnt = 0; > + > + match = of_match_node(sr_usb_phy_of_match, dn); > + if (!match) { > + dev_err(dev, "of_match_node() failed\n"); > + return -EINVAL; > + } > + ops = match->data; > + > + child_cnt = of_get_child_count(dn); > + if (!child_cnt || child_cnt > MAX_NR_PORTS) > + return -ENODEV; > + > + p = devm_kzalloc(dev, sizeof(*p), GFP_KERNEL); > + if (!p) > + return -ENOMEM; > + > + p->phy_port = devm_kcalloc(dev, child_cnt, > + sizeof(struct sr_usb_phy_port), > + GFP_KERNEL); > + if (!p->phy_port) > + return -ENOMEM; > + > + if (of_device_is_compatible(dn, "brcm,sr-u3h-u2drd-phy")) { > + void __iomem *drdu2reg, *usb3hreg; > + > + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, > + "drdu2"); > + drdu2reg = devm_ioremap_resource(&pdev->dev, res); > + if (IS_ERR(drdu2reg)) > + return PTR_ERR(drdu2reg); > + > + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, > + "usb3h"); > + usb3hreg = devm_ioremap_resource(&pdev->dev, res); > + if (IS_ERR(usb3hreg)) > + return PTR_ERR(usb3hreg); > + > + p->drdu2reg = drdu2reg; > + p->usb3hreg = usb3hreg; > + p->phy_id = USB3H_DRDU2_PHY; > + } > + if (of_device_is_compatible(dn, "brcm,sr-u3drd-phy")) { > + void __iomem *drdu3reg; > + > + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, > + "drdu3"); > + drdu3reg = devm_ioremap_resource(&pdev->dev, res); > + if (IS_ERR(drdu3reg)) > + return PTR_ERR(drdu3reg); > + > + p->drdu3reg = drdu3reg; > + p->phy_id = DRDU3_PHY; > + } > + > + for_each_available_child_of_node(dn, child) { > + phy_port = &p->phy_port[cnt]; > + if (of_property_read_u32(child, "reg", &phy_port->port_id)) { > + dev_err(dev, "missing reg property in node %s\n", > + child->name); > + return -EINVAL; > + } > + if (phy_port->port_id >= MAX_NR_PORTS) > + return -EINVAL; > + > + gphy = devm_phy_create(dev, child, ops); > + if (IS_ERR(gphy)) > + return PTR_ERR(gphy); > + > + phy_set_drvdata(gphy, phy_port); > + dev_set_drvdata(&gphy->dev, phy_port); why do you need dev_set_drvdata when phy_set_drvdata already saves private data for you? Thanks Kishon -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html