From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752446AbbCEWeD (ORCPT ); Thu, 5 Mar 2015 17:34:03 -0500 Received: from mail-ig0-f175.google.com ([209.85.213.175]:39273 "EHLO mail-ig0-f175.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750872AbbCEWd7 (ORCPT ); Thu, 5 Mar 2015 17:33:59 -0500 Date: Thu, 5 Mar 2015 14:33:55 -0800 From: Dmitry Torokhov To: Arun Ramamurthy Cc: Kishon Vijay Abraham I , Rob Herring , Pawel Moll , Mark Rutland , Ian Campbell , Kumar Gala , devicetree@vger.kernel.org, Jonathan Richardson , Scott Branden , Ray Jui , linux-arm-kernel@lists.infradead.org, bcm-kernel-feedback-list@broadcom.com, linux-kernel@vger.kernel.org, Anatol Pomazau Subject: Re: [PATCH 2/2] phy: usbphy: Add Broadcom Cygnus USB PHY driver Message-ID: <20150305223355.GA12062@dtor-ws> References: <1424200821-16368-1-git-send-email-arun.ramamurthy@broadcom.com> <1424200821-16368-3-git-send-email-arun.ramamurthy@broadcom.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1424200821-16368-3-git-send-email-arun.ramamurthy@broadcom.com> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Arun, On Tue, Feb 17, 2015 at 11:20:21AM -0800, Arun Ramamurthy wrote: > This driver adds support for USB 2.0 host and device phy > for Broadcom's Cygnus chipset Mostly nitpicks... > > Reviewed-by: Ray Jui > Reviewed-by: Scott Branden > Signed-off-by: Arun Ramamurthy > --- > drivers/phy/Kconfig | 12 + > drivers/phy/Makefile | 2 + > drivers/phy/phy-bcm-cygnus-usb.c | 491 +++++++++++++++++++++++++++++++++++++++ > 3 files changed, 505 insertions(+) > create mode 100644 drivers/phy/phy-bcm-cygnus-usb.c > > diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig > index ccad880..c934090 100644 > --- a/drivers/phy/Kconfig > +++ b/drivers/phy/Kconfig > @@ -277,4 +277,16 @@ config PHY_STIH41X_USB > Enable this to support the USB transceiver that is part of > STMicroelectronics STiH41x SoC series. > > +config PHY_BCM_CYGNUS_USB > + tristate "Broadcom Cygnus USB PHY support" > + depends on OF > + depends on ARCH_BCM_CYGNUS > + select GENERIC_PHY > + default ARCH_BCM_CYGNUS > + help > + Enable this to support the USB PHY in Broadcom's Cygnus chip. > + The phys are capable of supporting host mode for all ports > + and device mode for port 2. The host or device configuration is > + read from the device tree. > + > endmenu > diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile > index aa74f96..b235677 100644 > --- a/drivers/phy/Makefile > +++ b/drivers/phy/Makefile > @@ -7,6 +7,7 @@ obj-$(CONFIG_PHY_BERLIN_USB) += phy-berlin-usb.o > obj-$(CONFIG_PHY_BERLIN_SATA) += phy-berlin-sata.o > obj-$(CONFIG_ARMADA375_USBCLUSTER_PHY) += phy-armada375-usb2.o > obj-$(CONFIG_BCM_KONA_USB2_PHY) += phy-bcm-kona-usb2.o > +obj-$(CONFIG_PHY_BCM_CYGNUS_USB) += phy-bcm-cygnus-usb.o > obj-$(CONFIG_PHY_EXYNOS_DP_VIDEO) += phy-exynos-dp-video.o > obj-$(CONFIG_PHY_EXYNOS_MIPI_VIDEO) += phy-exynos-mipi-video.o > obj-$(CONFIG_PHY_MVEBU_SATA) += phy-mvebu-sata.o > @@ -34,3 +35,4 @@ obj-$(CONFIG_PHY_ST_SPEAR1340_MIPHY) += phy-spear1340-miphy.o > obj-$(CONFIG_PHY_XGENE) += phy-xgene.o > obj-$(CONFIG_PHY_STIH407_USB) += phy-stih407-usb.o > obj-$(CONFIG_PHY_STIH41X_USB) += phy-stih41x-usb.o > + > diff --git a/drivers/phy/phy-bcm-cygnus-usb.c b/drivers/phy/phy-bcm-cygnus-usb.c > new file mode 100644 > index 0000000..e5d3e50 > --- /dev/null > +++ b/drivers/phy/phy-bcm-cygnus-usb.c > @@ -0,0 +1,491 @@ > +/* > + * Copyright (C) 2014 Broadcom Corporation > + * > + * 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 > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#define CDRU_USBPHY_CLK_RST_SEL_OFFSET 0x11b4 > +#define CDRU_USBPHY2_HOST_DEV_SEL_OFFSET 0x11b8 > +#define CDRU_SPARE_REG_0_OFFSET 0x1238 > +#define CRMU_USB_PHY_AON_CTRL_OFFSET 0x00028 > +#define CDRU_USB_DEV_SUSPEND_RESUME_CTRL_OFFSET 0x1210 > +#define CDRU_USBPHY_P2_STATUS_OFFSET 0x1200 > + > +#define CDRU_USB_DEV_SUSPEND_RESUME_CTRL_DISABLE 0 > +#define PHY2_DEV_HOST_CTRL_SEL_DEVICE 0 > +#define PHY2_DEV_HOST_CTRL_SEL_HOST 1 Do we need these 2? Seems like a boolean for host mode to me. > +#define CDRU_USBPHY_P2_STATUS__USBPHY_ILDO_ON_FLAG 1 > +#define CDRU_USBPHY_P2_STATUS__USBPHY_PLL_LOCK 0 > +#define CRMU_USBPHY_P0_AFE_CORERDY_VDDC 1 > +#define CRMU_USBPHY_P0_RESETB 2 > +#define CRMU_USBPHY_P1_AFE_CORERDY_VDDC 9 > +#define CRMU_USBPHY_P1_RESETB 10 > +#define CRMU_USBPHY_P2_AFE_CORERDY_VDDC 17 > +#define CRMU_USBPHY_P2_RESETB 18 > + > +#define USB2_IDM_IDM_IO_CONTROL_DIRECT_OFFSET 0x0408 > +#define USB2_IDM_IDM_IO_CONTROL_DIRECT__clk_enable 0 > +#define USB2_IDM_IDM_RESET_CONTROL_OFFSET 0x0800 > +#define USB2_IDM_IDM_RESET_CONTROL__RESET 0 > + > +#define PLL_LOCK_RETRY_COUNT 1000 > +#define MAX_REGULATOR_NAME_LEN 25 > + > +struct bcm_phy_instance; > + > +struct bcm_phy_driver { > + void __iomem *usbphy_regs; > + void __iomem *usb2h_idm_regs; > + void __iomem *usb2d_idm_regs; > + spinlock_t lock; > + int num_phys, idm_host_enabled; Make idm_host_enabled a boolean? > + struct bcm_phy_instance *instances; > +}; > + > +struct bcm_phy_instance { > + struct bcm_phy_driver *driver; > + struct phy *generic_phy; > + int port; > + int host_mode; /* 1 - Host , 0 - device */ bool? > + int power; /* 1 -powered_on 0 -powered off */ bool? > + struct regulator *vbus_supply; > +}; > + > +static inline int cdru_usbphy_p2_status_wait(int reg_bit, > + struct bcm_phy_driver *phy_driver) > +{ > + /* Wait for the PLL lock status */ > + int retry = PLL_LOCK_RETRY_COUNT; > + u32 reg_val; > + > + do { > + udelay(1); > + reg_val = readl(phy_driver->usbphy_regs + > + CDRU_USBPHY_P2_STATUS_OFFSET); > + } while (--retry > 0 && (reg_val & (1 << reg_bit)) == 0); > + > + if (retry == 0) > + return -EBUSY; > + else > + return 0; Hmm, on the last iteration you'll return -EBUSY even if operation succeeds. How about: do { udelay(1); reg_val = readl(phy_driver->usbphy_regs + CDRU_USBPHY_P2_STATUS_OFFSET); if (reg_val & (1 << reg_bit)) return 0; } while (--retry > 0); return -EBUSY; > +} > + > +static struct phy *bcm_usb_phy_xlate(struct device *dev, > + struct of_phandle_args *args) > +{ > + struct bcm_phy_driver *phy_driver = dev_get_drvdata(dev); > + > + if (!phy_driver) > + return ERR_PTR(-EINVAL); > + > + if (WARN_ON(args->args[0] >= phy_driver->num_phys)) > + return ERR_PTR(-ENODEV); > + > + if (WARN_ON(args->args[1] < 0 || args->args[1] > 1)) > + return ERR_PTR(-EINVAL); > + > + if (WARN_ON(args->args_count < 2)) > + return ERR_PTR(-EINVAL); > + > + phy_driver->instances[args->args[0]].port = args->args[0]; > + phy_driver->instances[args->args[0]].host_mode = args->args[1]; > + > + return phy_driver->instances[args->args[0]].generic_phy; > +} > + > +static int bcm_phy_init(struct phy *generic_phy) > +{ > + u32 reg_val; > + unsigned long flags; > + struct bcm_phy_instance *instance_ptr = phy_get_drvdata(generic_phy); > + struct bcm_phy_driver *phy_driver = instance_ptr->driver; > + > + spin_lock_irqsave(&phy_driver->lock, flags); > + > + /* Only PORT 2 is capabale of being device and host > + * Default setting is device, check if it is set to host */ > + if (instance_ptr->port == 2) { > + if (instance_ptr->host_mode == PHY2_DEV_HOST_CTRL_SEL_HOST) > + writel(PHY2_DEV_HOST_CTRL_SEL_HOST, > + phy_driver->usbphy_regs + > + CDRU_USBPHY2_HOST_DEV_SEL_OFFSET); > + else { > + /* Disable suspend/resume signals to device controller > + when a port is in device mode */ > + reg_val = readl(phy_driver->usbphy_regs + > + CDRU_USB_DEV_SUSPEND_RESUME_CTRL_OFFSET); > + reg_val |= > + (1 << CDRU_USB_DEV_SUSPEND_RESUME_CTRL_DISABLE); > + writel(reg_val, phy_driver->usbphy_regs + > + CDRU_USB_DEV_SUSPEND_RESUME_CTRL_OFFSET); > + } > + } > + > + spin_unlock_irqrestore(&phy_driver->lock, flags); > + return 0; > +} > + > +static int bcm_phy_shutdown(struct phy *generic_phy) > +{ > + > + u32 reg_val, powered_on_phy; > + int i, power_off_flag = 1; > + unsigned long flags; > + struct bcm_phy_instance *instance_ptr = phy_get_drvdata(generic_phy); > + struct bcm_phy_driver *phy_driver = instance_ptr->driver; > + > + spin_lock_irqsave(&phy_driver->lock, flags); > + > + /* power down the phy */ > + reg_val = readl(phy_driver->usbphy_regs + > + CRMU_USB_PHY_AON_CTRL_OFFSET); > + if (instance_ptr->port == 0) { > + reg_val &= ~(1 << CRMU_USBPHY_P0_AFE_CORERDY_VDDC); > + reg_val &= ~(1 << CRMU_USBPHY_P0_RESETB); > + } else if (instance_ptr->port == 1) { > + reg_val &= ~(1 << CRMU_USBPHY_P1_AFE_CORERDY_VDDC); > + reg_val &= ~(1 << CRMU_USBPHY_P1_RESETB); > + } else if (instance_ptr->port == 2) { > + reg_val &= ~(1 << CRMU_USBPHY_P2_AFE_CORERDY_VDDC); > + reg_val &= ~(1 << CRMU_USBPHY_P2_RESETB); > + } > + writel(reg_val, phy_driver->usbphy_regs + > + CRMU_USB_PHY_AON_CTRL_OFFSET); > + > + instance_ptr->power = 0; > + > + /* if a port is configured to device and it is being shutdown, > + * turn off the clocks to the usb device controller */ > + if (instance_ptr->port == 2 && > + instance_ptr->host_mode == PHY2_DEV_HOST_CTRL_SEL_DEVICE) { if (instance_ptr->port == 2 && !instance_ptr->host_mode) { > + reg_val = readl(phy_driver->usb2d_idm_regs + > + USB2_IDM_IDM_IO_CONTROL_DIRECT_OFFSET); > + reg_val &= ~(1 << USB2_IDM_IDM_IO_CONTROL_DIRECT__clk_enable); > + writel(reg_val, phy_driver->usb2d_idm_regs + > + USB2_IDM_IDM_IO_CONTROL_DIRECT_OFFSET); > + > + reg_val = readl(phy_driver->usb2d_idm_regs + > + USB2_IDM_IDM_RESET_CONTROL_OFFSET); > + reg_val |= (1 << USB2_IDM_IDM_RESET_CONTROL__RESET); > + writel(reg_val, phy_driver->usb2d_idm_regs + > + USB2_IDM_IDM_RESET_CONTROL_OFFSET); > + } else { > + > + /* If the phy being shutdown provides clock and reset to > + * the host controller, change it do a different powered on phy > + * If all phys are powered off, shut of the host controller > + */ Here and elsewhere: preferred multi-line comment formatting is: /* * Multi * line * comment */ > + reg_val = readl(phy_driver->usbphy_regs + > + CDRU_USBPHY_CLK_RST_SEL_OFFSET); > + powered_on_phy = reg_val; > + if (reg_val == instance_ptr->port) { > + for (i = 0; i < phy_driver->num_phys; i++) { > + if (phy_driver->instances[i].power == 1 && > + phy_driver->instances[i].host_mode == > + PHY2_DEV_HOST_CTRL_SEL_HOST) { > + power_off_flag = 0; > + powered_on_phy = i; > + } > + } > + } > + > + if (power_off_flag) { > + /* Put the host controller into reset > + state and disable clock */ > + reg_val = readl(phy_driver->usb2h_idm_regs + > + USB2_IDM_IDM_IO_CONTROL_DIRECT_OFFSET); > + reg_val &= > + ~(1 << USB2_IDM_IDM_IO_CONTROL_DIRECT__clk_enable); > + writel(reg_val, phy_driver->usb2h_idm_regs + > + USB2_IDM_IDM_IO_CONTROL_DIRECT_OFFSET); > + > + reg_val = readl(phy_driver->usb2h_idm_regs + > + USB2_IDM_IDM_RESET_CONTROL_OFFSET); > + reg_val |= (1 << USB2_IDM_IDM_RESET_CONTROL__RESET); > + writel(reg_val, phy_driver->usb2h_idm_regs + > + USB2_IDM_IDM_RESET_CONTROL_OFFSET); > + phy_driver->idm_host_enabled = 0; > + } else > + writel(powered_on_phy, phy_driver->usbphy_regs + > + CDRU_USBPHY_CLK_RST_SEL_OFFSET); if one branch has curly braces all of them shoudl have them: } else { writel(...); } > + } > + > + spin_unlock_irqrestore(&phy_driver->lock, flags); > + > + if (instance_ptr->vbus_supply) > + regulator_disable(instance_ptr->vbus_supply); > + > + return 0; > +} > + > +static int bcm_phy_poweron(struct phy *generic_phy) > +{ > + int ret, clock_reset_flag = 1; bool clock_reset_flag = true; > + unsigned long flags; > + u32 reg_val; > + struct bcm_phy_instance *instance_ptr = phy_get_drvdata(generic_phy); > + struct bcm_phy_driver *phy_driver = instance_ptr->driver; > + > + if (instance_ptr->vbus_supply) { > + ret = regulator_enable(instance_ptr->vbus_supply); > + if (ret) { > + dev_err(&generic_phy->dev, > + "failed to enable regulator\n"); > + return ret; > + } > + } > + > + spin_lock_irqsave(&phy_driver->lock, flags); > + > + /* Bring the AFE block out of reset to start powering up the PHY */ > + reg_val = readl(phy_driver->usbphy_regs + CRMU_USB_PHY_AON_CTRL_OFFSET); > + > + if (instance_ptr->port == 0) > + reg_val |= (1 << CRMU_USBPHY_P0_AFE_CORERDY_VDDC); > + else if (instance_ptr->port == 1) > + reg_val |= (1 << CRMU_USBPHY_P1_AFE_CORERDY_VDDC); > + else if (instance_ptr->port == 2) > + reg_val |= (1 << CRMU_USBPHY_P2_AFE_CORERDY_VDDC); switch (instance_ptr->port) { case 0: ... } or maybe have a table for instance and corresponding control bits? > + writel(reg_val, phy_driver->usbphy_regs + CRMU_USB_PHY_AON_CTRL_OFFSET); > + > + instance_ptr->power = 1; instance_ptr->power = true; > + > + /* Check if the port 2 is configured for device */ > + if (instance_ptr->port == 2 && > + instance_ptr->host_mode == PHY2_DEV_HOST_CTRL_SEL_DEVICE) { if (instance_ptr->port == 2 && !instance_ptr->host_mode) { > + > + ret = cdru_usbphy_p2_status_wait > + (CDRU_USBPHY_P2_STATUS__USBPHY_ILDO_ON_FLAG, phy_driver); > + if (ret < 0) { > + dev_err(&generic_phy->dev, "Timed out waiting for USBPHY_ILDO_ON_FLAG on CDRU_USBPHY_P2_STATUS"); > + goto err_shutdown; > + } > + > + ret = cdru_usbphy_p2_status_wait > + (CDRU_USBPHY_P2_STATUS__USBPHY_PLL_LOCK, phy_driver); > + if (ret < 0) { > + dev_err(&generic_phy->dev, "Timed out waiting for USBPHY_PLL_LOCK on CDRU_USBPHY_P2_STATUS"); > + goto err_shutdown; > + } > + > + Extra blank line. > + /* Enable clock to USB device and get the USB device out of reset */ Indentation. > + reg_val = readl(phy_driver->usb2d_idm_regs + > + USB2_IDM_IDM_IO_CONTROL_DIRECT_OFFSET); > + reg_val |= (1 << USB2_IDM_IDM_IO_CONTROL_DIRECT__clk_enable); > + writel(reg_val, phy_driver->usb2d_idm_regs + > + USB2_IDM_IDM_IO_CONTROL_DIRECT_OFFSET); > + > + reg_val = readl(phy_driver->usb2d_idm_regs + > + USB2_IDM_IDM_RESET_CONTROL_OFFSET); > + reg_val &= ~(1 << USB2_IDM_IDM_RESET_CONTROL__RESET); > + writel(reg_val, phy_driver->usb2d_idm_regs + > + USB2_IDM_IDM_RESET_CONTROL_OFFSET); > + > + } else { > + reg_val = readl(phy_driver->usbphy_regs + > + CDRU_USBPHY_CLK_RST_SEL_OFFSET); > + > + /* Check if the phy that is configured > + * to provide clock and reset is powered on*/ > + if (reg_val >= 0 && reg_val < phy_driver->num_phys) { > + if (phy_driver->instances[reg_val].power == 1) > + clock_reset_flag = 0; > + } > + > + /* if not set the current phy */ > + if (clock_reset_flag) { > + reg_val = instance_ptr->port; > + writel(reg_val, phy_driver->usbphy_regs + > + CDRU_USBPHY_CLK_RST_SEL_OFFSET); > + } > + } > + > + if (phy_driver->idm_host_enabled != 1) { if (!phy_driver->idm_host_enabled) { > + /* Enable clock to USB and get the USB out of reset */ > + reg_val = readl(phy_driver->usb2h_idm_regs + > + USB2_IDM_IDM_IO_CONTROL_DIRECT_OFFSET); > + reg_val |= (1 << USB2_IDM_IDM_IO_CONTROL_DIRECT__clk_enable); > + writel(reg_val, phy_driver->usb2h_idm_regs + > + USB2_IDM_IDM_IO_CONTROL_DIRECT_OFFSET); > + > + reg_val = readl(phy_driver->usb2h_idm_regs + > + USB2_IDM_IDM_RESET_CONTROL_OFFSET); > + reg_val &= ~(1 << USB2_IDM_IDM_RESET_CONTROL__RESET); > + writel(reg_val, phy_driver->usb2h_idm_regs + > + USB2_IDM_IDM_RESET_CONTROL_OFFSET); > + phy_driver->idm_host_enabled = 1; = true; > + } > + > + spin_unlock_irqrestore(&phy_driver->lock, flags); > + return 0; > + > +err_shutdown: > + spin_unlock_irqrestore(&phy_driver->lock, flags); > + bcm_phy_shutdown(generic_phy); > + return ret; > +} > + > +static struct phy_ops ops = { > + .init = bcm_phy_init, > + .power_on = bcm_phy_poweron, > + .power_off = bcm_phy_shutdown, > +}; > + > +static const struct of_device_id bcm_phy_dt_ids[] = { > + { .compatible = "brcm,cygnus-usb-phy", }, > + { } > +}; > + > +static int bcm_phy_probe(struct platform_device *pdev) > +{ > + struct device *dev = &pdev->dev; > + struct resource res; > + struct bcm_phy_driver *phy_driver; > + struct phy_provider *phy_provider; > + int ret, num_phys, i; > + u32 reg_val; > + > + /* get number of phy cells */ > + ret = of_property_read_u32(dev->of_node, "num-phys", &num_phys); > + if (ret) { > + dev_err(dev, "Failed to obtain device tree resource num-phys\n"); > + return ret; > + } > + > + /* allocate memory for each phy instance */ > + phy_driver = devm_kzalloc(dev, sizeof(struct bcm_phy_driver), > + GFP_KERNEL); > + if (!phy_driver) > + return -ENOMEM; > + > + phy_driver->instances = devm_kcalloc(dev, num_phys, > + sizeof(struct bcm_phy_instance), > + GFP_KERNEL); > + phy_driver->num_phys = num_phys; > + spin_lock_init(&phy_driver->lock); > + > + ret = of_address_to_resource(dev->of_node, 0, &res); > + if (ret) { > + dev_err(dev, "Failed to obtain device tree resource usbphy_regs\n"); > + return ret; > + } > + phy_driver->usbphy_regs = devm_ioremap_nocache(dev, res.start, > + resource_size(&res)); > + if (!phy_driver->usbphy_regs) { > + dev_err(dev, "Failed to remap usbphy_regs\n"); > + return -ENOMEM; > + } > + > + ret = of_address_to_resource(dev->of_node, 1, &res); > + if (ret) { > + dev_err(dev, "Failed to obtain device tree resource usb2h_idm_regs\n"); > + return ret; > + } > + phy_driver->usb2h_idm_regs = devm_ioremap_nocache(dev, res.start, > + resource_size(&res)); > + if (!phy_driver->usb2h_idm_regs) { > + dev_err(dev, "Failed to remap usb2h_idm_regs\n"); > + return -ENOMEM; > + } > + > + ret = of_address_to_resource(dev->of_node, 2, &res); > + if (ret) { > + dev_err(dev, "Failed to obtain device tree resource usb2d_idm_regs\n"); > + return ret; > + } > + phy_driver->usb2d_idm_regs = devm_ioremap_nocache(dev, res.start, > + resource_size(&res)); > + if (!phy_driver->usb2d_idm_regs) { > + dev_err(dev, "Failed to remap usb2d_idm_regs\n"); > + return -ENOMEM; > + } > + dev_set_drvdata(dev, phy_driver); > + phy_driver->idm_host_enabled = 0; = false; > + > + /* Shutdown all ports. They can be powered up as > + * required */ > + reg_val = readl(phy_driver->usbphy_regs + > + CRMU_USB_PHY_AON_CTRL_OFFSET); > + reg_val &= ~(1 << CRMU_USBPHY_P0_AFE_CORERDY_VDDC); > + reg_val &= ~(1 << CRMU_USBPHY_P0_RESETB); > + reg_val &= ~(1 << CRMU_USBPHY_P1_AFE_CORERDY_VDDC); > + reg_val &= ~(1 << CRMU_USBPHY_P1_RESETB); > + reg_val &= ~(1 << CRMU_USBPHY_P2_AFE_CORERDY_VDDC); > + reg_val &= ~(1 << CRMU_USBPHY_P2_RESETB); > + writel(reg_val, phy_driver->usbphy_regs + > + CRMU_USB_PHY_AON_CTRL_OFFSET); > + > + for (i = 0; i < phy_driver->num_phys; i++) { > + char *vbus_name; > + struct bcm_phy_instance *instance_ptr = > + &phy_driver->instances[i]; > + > + vbus_name = devm_kzalloc(dev, MAX_REGULATOR_NAME_LEN, > + GFP_KERNEL); Why is this devm if it is freed immediately? MAX_REGULATOR_NAME_LEN is only 25 bytes, just have on-stack array for this. > + if (!vbus_name) > + return -ENOMEM; > + > + /* regulator use is optional */ > + sprintf(vbus_name, "vbus-p%d", i); > + instance_ptr->vbus_supply = devm_regulator_get(dev, vbus_name); > + if (IS_ERR(instance_ptr->vbus_supply)) > + instance_ptr->vbus_supply = NULL; > + devm_kfree(dev, vbus_name); > + > + instance_ptr->generic_phy = > + devm_phy_create(dev, NULL, &ops); > + > + if (IS_ERR(instance_ptr->generic_phy)) { > + dev_err(dev, "Failed to create usb phy %d", i); > + return PTR_ERR(instance_ptr->generic_phy); > + } > + instance_ptr->driver = phy_driver; > + phy_set_drvdata(instance_ptr->generic_phy, instance_ptr); > + } > + > + phy_provider = devm_of_phy_provider_register(dev, > + bcm_usb_phy_xlate); > + > + if (IS_ERR(phy_provider)) { > + dev_err(dev, "Failed to register as phy provider\n"); > + return PTR_ERR(phy_provider); > + } > + > + platform_set_drvdata(pdev, phy_driver); > + > + return 0; > + > +} > + > +MODULE_DEVICE_TABLE(of, bcm_phy_dt_ids); > + > +static struct platform_driver bcm_phy_driver = { > + .probe = bcm_phy_probe, > + .driver = { > + .name = "bcm-cygnus-usbphy", > + .of_match_table = of_match_ptr(bcm_phy_dt_ids), > + }, > +}; > +module_platform_driver(bcm_phy_driver); > + > +MODULE_ALIAS("platform:bcm-cygnus-usbphy"); > +MODULE_AUTHOR("Broadcom"); > +MODULE_DESCRIPTION("Broadcom Cygnus USB PHY driver"); > +MODULE_LICENSE("GPL V2"); > -- > 2.3.0 > Thanks. -- Dmitry From mboxrd@z Thu Jan 1 00:00:00 1970 From: dtor@google.com (Dmitry Torokhov) Date: Thu, 5 Mar 2015 14:33:55 -0800 Subject: [PATCH 2/2] phy: usbphy: Add Broadcom Cygnus USB PHY driver In-Reply-To: <1424200821-16368-3-git-send-email-arun.ramamurthy@broadcom.com> References: <1424200821-16368-1-git-send-email-arun.ramamurthy@broadcom.com> <1424200821-16368-3-git-send-email-arun.ramamurthy@broadcom.com> Message-ID: <20150305223355.GA12062@dtor-ws> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org Hi Arun, On Tue, Feb 17, 2015 at 11:20:21AM -0800, Arun Ramamurthy wrote: > This driver adds support for USB 2.0 host and device phy > for Broadcom's Cygnus chipset Mostly nitpicks... > > Reviewed-by: Ray Jui > Reviewed-by: Scott Branden > Signed-off-by: Arun Ramamurthy > --- > drivers/phy/Kconfig | 12 + > drivers/phy/Makefile | 2 + > drivers/phy/phy-bcm-cygnus-usb.c | 491 +++++++++++++++++++++++++++++++++++++++ > 3 files changed, 505 insertions(+) > create mode 100644 drivers/phy/phy-bcm-cygnus-usb.c > > diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig > index ccad880..c934090 100644 > --- a/drivers/phy/Kconfig > +++ b/drivers/phy/Kconfig > @@ -277,4 +277,16 @@ config PHY_STIH41X_USB > Enable this to support the USB transceiver that is part of > STMicroelectronics STiH41x SoC series. > > +config PHY_BCM_CYGNUS_USB > + tristate "Broadcom Cygnus USB PHY support" > + depends on OF > + depends on ARCH_BCM_CYGNUS > + select GENERIC_PHY > + default ARCH_BCM_CYGNUS > + help > + Enable this to support the USB PHY in Broadcom's Cygnus chip. > + The phys are capable of supporting host mode for all ports > + and device mode for port 2. The host or device configuration is > + read from the device tree. > + > endmenu > diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile > index aa74f96..b235677 100644 > --- a/drivers/phy/Makefile > +++ b/drivers/phy/Makefile > @@ -7,6 +7,7 @@ obj-$(CONFIG_PHY_BERLIN_USB) += phy-berlin-usb.o > obj-$(CONFIG_PHY_BERLIN_SATA) += phy-berlin-sata.o > obj-$(CONFIG_ARMADA375_USBCLUSTER_PHY) += phy-armada375-usb2.o > obj-$(CONFIG_BCM_KONA_USB2_PHY) += phy-bcm-kona-usb2.o > +obj-$(CONFIG_PHY_BCM_CYGNUS_USB) += phy-bcm-cygnus-usb.o > obj-$(CONFIG_PHY_EXYNOS_DP_VIDEO) += phy-exynos-dp-video.o > obj-$(CONFIG_PHY_EXYNOS_MIPI_VIDEO) += phy-exynos-mipi-video.o > obj-$(CONFIG_PHY_MVEBU_SATA) += phy-mvebu-sata.o > @@ -34,3 +35,4 @@ obj-$(CONFIG_PHY_ST_SPEAR1340_MIPHY) += phy-spear1340-miphy.o > obj-$(CONFIG_PHY_XGENE) += phy-xgene.o > obj-$(CONFIG_PHY_STIH407_USB) += phy-stih407-usb.o > obj-$(CONFIG_PHY_STIH41X_USB) += phy-stih41x-usb.o > + > diff --git a/drivers/phy/phy-bcm-cygnus-usb.c b/drivers/phy/phy-bcm-cygnus-usb.c > new file mode 100644 > index 0000000..e5d3e50 > --- /dev/null > +++ b/drivers/phy/phy-bcm-cygnus-usb.c > @@ -0,0 +1,491 @@ > +/* > + * Copyright (C) 2014 Broadcom Corporation > + * > + * 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 > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#define CDRU_USBPHY_CLK_RST_SEL_OFFSET 0x11b4 > +#define CDRU_USBPHY2_HOST_DEV_SEL_OFFSET 0x11b8 > +#define CDRU_SPARE_REG_0_OFFSET 0x1238 > +#define CRMU_USB_PHY_AON_CTRL_OFFSET 0x00028 > +#define CDRU_USB_DEV_SUSPEND_RESUME_CTRL_OFFSET 0x1210 > +#define CDRU_USBPHY_P2_STATUS_OFFSET 0x1200 > + > +#define CDRU_USB_DEV_SUSPEND_RESUME_CTRL_DISABLE 0 > +#define PHY2_DEV_HOST_CTRL_SEL_DEVICE 0 > +#define PHY2_DEV_HOST_CTRL_SEL_HOST 1 Do we need these 2? Seems like a boolean for host mode to me. > +#define CDRU_USBPHY_P2_STATUS__USBPHY_ILDO_ON_FLAG 1 > +#define CDRU_USBPHY_P2_STATUS__USBPHY_PLL_LOCK 0 > +#define CRMU_USBPHY_P0_AFE_CORERDY_VDDC 1 > +#define CRMU_USBPHY_P0_RESETB 2 > +#define CRMU_USBPHY_P1_AFE_CORERDY_VDDC 9 > +#define CRMU_USBPHY_P1_RESETB 10 > +#define CRMU_USBPHY_P2_AFE_CORERDY_VDDC 17 > +#define CRMU_USBPHY_P2_RESETB 18 > + > +#define USB2_IDM_IDM_IO_CONTROL_DIRECT_OFFSET 0x0408 > +#define USB2_IDM_IDM_IO_CONTROL_DIRECT__clk_enable 0 > +#define USB2_IDM_IDM_RESET_CONTROL_OFFSET 0x0800 > +#define USB2_IDM_IDM_RESET_CONTROL__RESET 0 > + > +#define PLL_LOCK_RETRY_COUNT 1000 > +#define MAX_REGULATOR_NAME_LEN 25 > + > +struct bcm_phy_instance; > + > +struct bcm_phy_driver { > + void __iomem *usbphy_regs; > + void __iomem *usb2h_idm_regs; > + void __iomem *usb2d_idm_regs; > + spinlock_t lock; > + int num_phys, idm_host_enabled; Make idm_host_enabled a boolean? > + struct bcm_phy_instance *instances; > +}; > + > +struct bcm_phy_instance { > + struct bcm_phy_driver *driver; > + struct phy *generic_phy; > + int port; > + int host_mode; /* 1 - Host , 0 - device */ bool? > + int power; /* 1 -powered_on 0 -powered off */ bool? > + struct regulator *vbus_supply; > +}; > + > +static inline int cdru_usbphy_p2_status_wait(int reg_bit, > + struct bcm_phy_driver *phy_driver) > +{ > + /* Wait for the PLL lock status */ > + int retry = PLL_LOCK_RETRY_COUNT; > + u32 reg_val; > + > + do { > + udelay(1); > + reg_val = readl(phy_driver->usbphy_regs + > + CDRU_USBPHY_P2_STATUS_OFFSET); > + } while (--retry > 0 && (reg_val & (1 << reg_bit)) == 0); > + > + if (retry == 0) > + return -EBUSY; > + else > + return 0; Hmm, on the last iteration you'll return -EBUSY even if operation succeeds. How about: do { udelay(1); reg_val = readl(phy_driver->usbphy_regs + CDRU_USBPHY_P2_STATUS_OFFSET); if (reg_val & (1 << reg_bit)) return 0; } while (--retry > 0); return -EBUSY; > +} > + > +static struct phy *bcm_usb_phy_xlate(struct device *dev, > + struct of_phandle_args *args) > +{ > + struct bcm_phy_driver *phy_driver = dev_get_drvdata(dev); > + > + if (!phy_driver) > + return ERR_PTR(-EINVAL); > + > + if (WARN_ON(args->args[0] >= phy_driver->num_phys)) > + return ERR_PTR(-ENODEV); > + > + if (WARN_ON(args->args[1] < 0 || args->args[1] > 1)) > + return ERR_PTR(-EINVAL); > + > + if (WARN_ON(args->args_count < 2)) > + return ERR_PTR(-EINVAL); > + > + phy_driver->instances[args->args[0]].port = args->args[0]; > + phy_driver->instances[args->args[0]].host_mode = args->args[1]; > + > + return phy_driver->instances[args->args[0]].generic_phy; > +} > + > +static int bcm_phy_init(struct phy *generic_phy) > +{ > + u32 reg_val; > + unsigned long flags; > + struct bcm_phy_instance *instance_ptr = phy_get_drvdata(generic_phy); > + struct bcm_phy_driver *phy_driver = instance_ptr->driver; > + > + spin_lock_irqsave(&phy_driver->lock, flags); > + > + /* Only PORT 2 is capabale of being device and host > + * Default setting is device, check if it is set to host */ > + if (instance_ptr->port == 2) { > + if (instance_ptr->host_mode == PHY2_DEV_HOST_CTRL_SEL_HOST) > + writel(PHY2_DEV_HOST_CTRL_SEL_HOST, > + phy_driver->usbphy_regs + > + CDRU_USBPHY2_HOST_DEV_SEL_OFFSET); > + else { > + /* Disable suspend/resume signals to device controller > + when a port is in device mode */ > + reg_val = readl(phy_driver->usbphy_regs + > + CDRU_USB_DEV_SUSPEND_RESUME_CTRL_OFFSET); > + reg_val |= > + (1 << CDRU_USB_DEV_SUSPEND_RESUME_CTRL_DISABLE); > + writel(reg_val, phy_driver->usbphy_regs + > + CDRU_USB_DEV_SUSPEND_RESUME_CTRL_OFFSET); > + } > + } > + > + spin_unlock_irqrestore(&phy_driver->lock, flags); > + return 0; > +} > + > +static int bcm_phy_shutdown(struct phy *generic_phy) > +{ > + > + u32 reg_val, powered_on_phy; > + int i, power_off_flag = 1; > + unsigned long flags; > + struct bcm_phy_instance *instance_ptr = phy_get_drvdata(generic_phy); > + struct bcm_phy_driver *phy_driver = instance_ptr->driver; > + > + spin_lock_irqsave(&phy_driver->lock, flags); > + > + /* power down the phy */ > + reg_val = readl(phy_driver->usbphy_regs + > + CRMU_USB_PHY_AON_CTRL_OFFSET); > + if (instance_ptr->port == 0) { > + reg_val &= ~(1 << CRMU_USBPHY_P0_AFE_CORERDY_VDDC); > + reg_val &= ~(1 << CRMU_USBPHY_P0_RESETB); > + } else if (instance_ptr->port == 1) { > + reg_val &= ~(1 << CRMU_USBPHY_P1_AFE_CORERDY_VDDC); > + reg_val &= ~(1 << CRMU_USBPHY_P1_RESETB); > + } else if (instance_ptr->port == 2) { > + reg_val &= ~(1 << CRMU_USBPHY_P2_AFE_CORERDY_VDDC); > + reg_val &= ~(1 << CRMU_USBPHY_P2_RESETB); > + } > + writel(reg_val, phy_driver->usbphy_regs + > + CRMU_USB_PHY_AON_CTRL_OFFSET); > + > + instance_ptr->power = 0; > + > + /* if a port is configured to device and it is being shutdown, > + * turn off the clocks to the usb device controller */ > + if (instance_ptr->port == 2 && > + instance_ptr->host_mode == PHY2_DEV_HOST_CTRL_SEL_DEVICE) { if (instance_ptr->port == 2 && !instance_ptr->host_mode) { > + reg_val = readl(phy_driver->usb2d_idm_regs + > + USB2_IDM_IDM_IO_CONTROL_DIRECT_OFFSET); > + reg_val &= ~(1 << USB2_IDM_IDM_IO_CONTROL_DIRECT__clk_enable); > + writel(reg_val, phy_driver->usb2d_idm_regs + > + USB2_IDM_IDM_IO_CONTROL_DIRECT_OFFSET); > + > + reg_val = readl(phy_driver->usb2d_idm_regs + > + USB2_IDM_IDM_RESET_CONTROL_OFFSET); > + reg_val |= (1 << USB2_IDM_IDM_RESET_CONTROL__RESET); > + writel(reg_val, phy_driver->usb2d_idm_regs + > + USB2_IDM_IDM_RESET_CONTROL_OFFSET); > + } else { > + > + /* If the phy being shutdown provides clock and reset to > + * the host controller, change it do a different powered on phy > + * If all phys are powered off, shut of the host controller > + */ Here and elsewhere: preferred multi-line comment formatting is: /* * Multi * line * comment */ > + reg_val = readl(phy_driver->usbphy_regs + > + CDRU_USBPHY_CLK_RST_SEL_OFFSET); > + powered_on_phy = reg_val; > + if (reg_val == instance_ptr->port) { > + for (i = 0; i < phy_driver->num_phys; i++) { > + if (phy_driver->instances[i].power == 1 && > + phy_driver->instances[i].host_mode == > + PHY2_DEV_HOST_CTRL_SEL_HOST) { > + power_off_flag = 0; > + powered_on_phy = i; > + } > + } > + } > + > + if (power_off_flag) { > + /* Put the host controller into reset > + state and disable clock */ > + reg_val = readl(phy_driver->usb2h_idm_regs + > + USB2_IDM_IDM_IO_CONTROL_DIRECT_OFFSET); > + reg_val &= > + ~(1 << USB2_IDM_IDM_IO_CONTROL_DIRECT__clk_enable); > + writel(reg_val, phy_driver->usb2h_idm_regs + > + USB2_IDM_IDM_IO_CONTROL_DIRECT_OFFSET); > + > + reg_val = readl(phy_driver->usb2h_idm_regs + > + USB2_IDM_IDM_RESET_CONTROL_OFFSET); > + reg_val |= (1 << USB2_IDM_IDM_RESET_CONTROL__RESET); > + writel(reg_val, phy_driver->usb2h_idm_regs + > + USB2_IDM_IDM_RESET_CONTROL_OFFSET); > + phy_driver->idm_host_enabled = 0; > + } else > + writel(powered_on_phy, phy_driver->usbphy_regs + > + CDRU_USBPHY_CLK_RST_SEL_OFFSET); if one branch has curly braces all of them shoudl have them: } else { writel(...); } > + } > + > + spin_unlock_irqrestore(&phy_driver->lock, flags); > + > + if (instance_ptr->vbus_supply) > + regulator_disable(instance_ptr->vbus_supply); > + > + return 0; > +} > + > +static int bcm_phy_poweron(struct phy *generic_phy) > +{ > + int ret, clock_reset_flag = 1; bool clock_reset_flag = true; > + unsigned long flags; > + u32 reg_val; > + struct bcm_phy_instance *instance_ptr = phy_get_drvdata(generic_phy); > + struct bcm_phy_driver *phy_driver = instance_ptr->driver; > + > + if (instance_ptr->vbus_supply) { > + ret = regulator_enable(instance_ptr->vbus_supply); > + if (ret) { > + dev_err(&generic_phy->dev, > + "failed to enable regulator\n"); > + return ret; > + } > + } > + > + spin_lock_irqsave(&phy_driver->lock, flags); > + > + /* Bring the AFE block out of reset to start powering up the PHY */ > + reg_val = readl(phy_driver->usbphy_regs + CRMU_USB_PHY_AON_CTRL_OFFSET); > + > + if (instance_ptr->port == 0) > + reg_val |= (1 << CRMU_USBPHY_P0_AFE_CORERDY_VDDC); > + else if (instance_ptr->port == 1) > + reg_val |= (1 << CRMU_USBPHY_P1_AFE_CORERDY_VDDC); > + else if (instance_ptr->port == 2) > + reg_val |= (1 << CRMU_USBPHY_P2_AFE_CORERDY_VDDC); switch (instance_ptr->port) { case 0: ... } or maybe have a table for instance and corresponding control bits? > + writel(reg_val, phy_driver->usbphy_regs + CRMU_USB_PHY_AON_CTRL_OFFSET); > + > + instance_ptr->power = 1; instance_ptr->power = true; > + > + /* Check if the port 2 is configured for device */ > + if (instance_ptr->port == 2 && > + instance_ptr->host_mode == PHY2_DEV_HOST_CTRL_SEL_DEVICE) { if (instance_ptr->port == 2 && !instance_ptr->host_mode) { > + > + ret = cdru_usbphy_p2_status_wait > + (CDRU_USBPHY_P2_STATUS__USBPHY_ILDO_ON_FLAG, phy_driver); > + if (ret < 0) { > + dev_err(&generic_phy->dev, "Timed out waiting for USBPHY_ILDO_ON_FLAG on CDRU_USBPHY_P2_STATUS"); > + goto err_shutdown; > + } > + > + ret = cdru_usbphy_p2_status_wait > + (CDRU_USBPHY_P2_STATUS__USBPHY_PLL_LOCK, phy_driver); > + if (ret < 0) { > + dev_err(&generic_phy->dev, "Timed out waiting for USBPHY_PLL_LOCK on CDRU_USBPHY_P2_STATUS"); > + goto err_shutdown; > + } > + > + Extra blank line. > + /* Enable clock to USB device and get the USB device out of reset */ Indentation. > + reg_val = readl(phy_driver->usb2d_idm_regs + > + USB2_IDM_IDM_IO_CONTROL_DIRECT_OFFSET); > + reg_val |= (1 << USB2_IDM_IDM_IO_CONTROL_DIRECT__clk_enable); > + writel(reg_val, phy_driver->usb2d_idm_regs + > + USB2_IDM_IDM_IO_CONTROL_DIRECT_OFFSET); > + > + reg_val = readl(phy_driver->usb2d_idm_regs + > + USB2_IDM_IDM_RESET_CONTROL_OFFSET); > + reg_val &= ~(1 << USB2_IDM_IDM_RESET_CONTROL__RESET); > + writel(reg_val, phy_driver->usb2d_idm_regs + > + USB2_IDM_IDM_RESET_CONTROL_OFFSET); > + > + } else { > + reg_val = readl(phy_driver->usbphy_regs + > + CDRU_USBPHY_CLK_RST_SEL_OFFSET); > + > + /* Check if the phy that is configured > + * to provide clock and reset is powered on*/ > + if (reg_val >= 0 && reg_val < phy_driver->num_phys) { > + if (phy_driver->instances[reg_val].power == 1) > + clock_reset_flag = 0; > + } > + > + /* if not set the current phy */ > + if (clock_reset_flag) { > + reg_val = instance_ptr->port; > + writel(reg_val, phy_driver->usbphy_regs + > + CDRU_USBPHY_CLK_RST_SEL_OFFSET); > + } > + } > + > + if (phy_driver->idm_host_enabled != 1) { if (!phy_driver->idm_host_enabled) { > + /* Enable clock to USB and get the USB out of reset */ > + reg_val = readl(phy_driver->usb2h_idm_regs + > + USB2_IDM_IDM_IO_CONTROL_DIRECT_OFFSET); > + reg_val |= (1 << USB2_IDM_IDM_IO_CONTROL_DIRECT__clk_enable); > + writel(reg_val, phy_driver->usb2h_idm_regs + > + USB2_IDM_IDM_IO_CONTROL_DIRECT_OFFSET); > + > + reg_val = readl(phy_driver->usb2h_idm_regs + > + USB2_IDM_IDM_RESET_CONTROL_OFFSET); > + reg_val &= ~(1 << USB2_IDM_IDM_RESET_CONTROL__RESET); > + writel(reg_val, phy_driver->usb2h_idm_regs + > + USB2_IDM_IDM_RESET_CONTROL_OFFSET); > + phy_driver->idm_host_enabled = 1; = true; > + } > + > + spin_unlock_irqrestore(&phy_driver->lock, flags); > + return 0; > + > +err_shutdown: > + spin_unlock_irqrestore(&phy_driver->lock, flags); > + bcm_phy_shutdown(generic_phy); > + return ret; > +} > + > +static struct phy_ops ops = { > + .init = bcm_phy_init, > + .power_on = bcm_phy_poweron, > + .power_off = bcm_phy_shutdown, > +}; > + > +static const struct of_device_id bcm_phy_dt_ids[] = { > + { .compatible = "brcm,cygnus-usb-phy", }, > + { } > +}; > + > +static int bcm_phy_probe(struct platform_device *pdev) > +{ > + struct device *dev = &pdev->dev; > + struct resource res; > + struct bcm_phy_driver *phy_driver; > + struct phy_provider *phy_provider; > + int ret, num_phys, i; > + u32 reg_val; > + > + /* get number of phy cells */ > + ret = of_property_read_u32(dev->of_node, "num-phys", &num_phys); > + if (ret) { > + dev_err(dev, "Failed to obtain device tree resource num-phys\n"); > + return ret; > + } > + > + /* allocate memory for each phy instance */ > + phy_driver = devm_kzalloc(dev, sizeof(struct bcm_phy_driver), > + GFP_KERNEL); > + if (!phy_driver) > + return -ENOMEM; > + > + phy_driver->instances = devm_kcalloc(dev, num_phys, > + sizeof(struct bcm_phy_instance), > + GFP_KERNEL); > + phy_driver->num_phys = num_phys; > + spin_lock_init(&phy_driver->lock); > + > + ret = of_address_to_resource(dev->of_node, 0, &res); > + if (ret) { > + dev_err(dev, "Failed to obtain device tree resource usbphy_regs\n"); > + return ret; > + } > + phy_driver->usbphy_regs = devm_ioremap_nocache(dev, res.start, > + resource_size(&res)); > + if (!phy_driver->usbphy_regs) { > + dev_err(dev, "Failed to remap usbphy_regs\n"); > + return -ENOMEM; > + } > + > + ret = of_address_to_resource(dev->of_node, 1, &res); > + if (ret) { > + dev_err(dev, "Failed to obtain device tree resource usb2h_idm_regs\n"); > + return ret; > + } > + phy_driver->usb2h_idm_regs = devm_ioremap_nocache(dev, res.start, > + resource_size(&res)); > + if (!phy_driver->usb2h_idm_regs) { > + dev_err(dev, "Failed to remap usb2h_idm_regs\n"); > + return -ENOMEM; > + } > + > + ret = of_address_to_resource(dev->of_node, 2, &res); > + if (ret) { > + dev_err(dev, "Failed to obtain device tree resource usb2d_idm_regs\n"); > + return ret; > + } > + phy_driver->usb2d_idm_regs = devm_ioremap_nocache(dev, res.start, > + resource_size(&res)); > + if (!phy_driver->usb2d_idm_regs) { > + dev_err(dev, "Failed to remap usb2d_idm_regs\n"); > + return -ENOMEM; > + } > + dev_set_drvdata(dev, phy_driver); > + phy_driver->idm_host_enabled = 0; = false; > + > + /* Shutdown all ports. They can be powered up as > + * required */ > + reg_val = readl(phy_driver->usbphy_regs + > + CRMU_USB_PHY_AON_CTRL_OFFSET); > + reg_val &= ~(1 << CRMU_USBPHY_P0_AFE_CORERDY_VDDC); > + reg_val &= ~(1 << CRMU_USBPHY_P0_RESETB); > + reg_val &= ~(1 << CRMU_USBPHY_P1_AFE_CORERDY_VDDC); > + reg_val &= ~(1 << CRMU_USBPHY_P1_RESETB); > + reg_val &= ~(1 << CRMU_USBPHY_P2_AFE_CORERDY_VDDC); > + reg_val &= ~(1 << CRMU_USBPHY_P2_RESETB); > + writel(reg_val, phy_driver->usbphy_regs + > + CRMU_USB_PHY_AON_CTRL_OFFSET); > + > + for (i = 0; i < phy_driver->num_phys; i++) { > + char *vbus_name; > + struct bcm_phy_instance *instance_ptr = > + &phy_driver->instances[i]; > + > + vbus_name = devm_kzalloc(dev, MAX_REGULATOR_NAME_LEN, > + GFP_KERNEL); Why is this devm if it is freed immediately? MAX_REGULATOR_NAME_LEN is only 25 bytes, just have on-stack array for this. > + if (!vbus_name) > + return -ENOMEM; > + > + /* regulator use is optional */ > + sprintf(vbus_name, "vbus-p%d", i); > + instance_ptr->vbus_supply = devm_regulator_get(dev, vbus_name); > + if (IS_ERR(instance_ptr->vbus_supply)) > + instance_ptr->vbus_supply = NULL; > + devm_kfree(dev, vbus_name); > + > + instance_ptr->generic_phy = > + devm_phy_create(dev, NULL, &ops); > + > + if (IS_ERR(instance_ptr->generic_phy)) { > + dev_err(dev, "Failed to create usb phy %d", i); > + return PTR_ERR(instance_ptr->generic_phy); > + } > + instance_ptr->driver = phy_driver; > + phy_set_drvdata(instance_ptr->generic_phy, instance_ptr); > + } > + > + phy_provider = devm_of_phy_provider_register(dev, > + bcm_usb_phy_xlate); > + > + if (IS_ERR(phy_provider)) { > + dev_err(dev, "Failed to register as phy provider\n"); > + return PTR_ERR(phy_provider); > + } > + > + platform_set_drvdata(pdev, phy_driver); > + > + return 0; > + > +} > + > +MODULE_DEVICE_TABLE(of, bcm_phy_dt_ids); > + > +static struct platform_driver bcm_phy_driver = { > + .probe = bcm_phy_probe, > + .driver = { > + .name = "bcm-cygnus-usbphy", > + .of_match_table = of_match_ptr(bcm_phy_dt_ids), > + }, > +}; > +module_platform_driver(bcm_phy_driver); > + > +MODULE_ALIAS("platform:bcm-cygnus-usbphy"); > +MODULE_AUTHOR("Broadcom"); > +MODULE_DESCRIPTION("Broadcom Cygnus USB PHY driver"); > +MODULE_LICENSE("GPL V2"); > -- > 2.3.0 > Thanks. -- Dmitry