Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754233AbcDANSU (ORCPT ); Fri, 1 Apr 2016 09:18:20 -0400 Received: from arroyo.ext.ti.com ([192.94.94.40]:58146 "EHLO arroyo.ext.ti.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752803AbcDANSS (ORCPT ); Fri, 1 Apr 2016 09:18:18 -0400 Subject: Re: [PATCH v2 06/11] phy: da8xx-usb: new driver for DA8XX SoC USB PHY To: David Lechner References: <1458181615-27782-1-git-send-email-david@lechnology.com> <1458181615-27782-7-git-send-email-david@lechnology.com> CC: Petr Kulhavy , Rob Herring , Pawel Moll , Mark Rutland , Ian Campbell , Kumar Gala , Russell King , Sekhar Nori , Kevin Hilman , Alan Stern , Greg Kroah-Hartman , Bin Liu , =?UTF-8?Q?Andreas_F=c3=a4rber?= , Tony Lindgren , Robert Jarzmik , Sergei Shtylyov , , , , From: Kishon Vijay Abraham I Message-ID: <56FE74AC.6080303@ti.com> Date: Fri, 1 Apr 2016 18:46:28 +0530 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0 MIME-Version: 1.0 In-Reply-To: <1458181615-27782-7-git-send-email-david@lechnology.com> Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 11926 Lines: 429 Hi, On Thursday 17 March 2016 07:56 AM, David Lechner wrote: > This is a new phy driver for the SoC USB controllers on the TI DA8XX > family of microcontrollers. The USB 1.1 PHY is just a simple on/off. > The USB 2.0 PHY also allows overriding the VBUS and ID pins. > > Signed-off-by: David Lechner > --- > > v2 changes: This is new patch in v2. > > > > drivers/phy/Kconfig | 9 ++ > drivers/phy/Makefile | 1 + > drivers/phy/phy-da8xx-usb.c | 295 ++++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 305 insertions(+) > create mode 100644 drivers/phy/phy-da8xx-usb.c > > diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig > index 26566db..a6060ea 100644 > --- a/drivers/phy/Kconfig > +++ b/drivers/phy/Kconfig > @@ -35,6 +35,15 @@ config ARMADA375_USBCLUSTER_PHY > depends on OF && HAS_IOMEM > select GENERIC_PHY > > +config PHY_DA8XX_USB > + tristate "TI DA8XX USB PHY Driver" > + depends on ARCH_DAVINCI_DA8XX > + select GENERIC_PHY > + help > + Enable this to support the USB PHY on DA8XX SoCs. > + > + This driver controls both the USB 1.1 PHY and the USB 2.0 PHY. > + > config PHY_DM816X_USB > tristate "TI dm816x USB PHY driver" > depends on ARCH_OMAP2PLUS > diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile > index 24596a9..722e01c 100644 > --- a/drivers/phy/Makefile > +++ b/drivers/phy/Makefile > @@ -5,6 +5,7 @@ > obj-$(CONFIG_GENERIC_PHY) += phy-core.o > obj-$(CONFIG_PHY_BERLIN_USB) += phy-berlin-usb.o > obj-$(CONFIG_PHY_BERLIN_SATA) += phy-berlin-sata.o > +obj-$(CONFIG_PHY_DA8XX_USB) += phy-da8xx-usb.o > obj-$(CONFIG_PHY_DM816X_USB) += phy-dm816x-usb.o > obj-$(CONFIG_ARMADA375_USBCLUSTER_PHY) += phy-armada375-usb2.o > obj-$(CONFIG_BCM_KONA_USB2_PHY) += phy-bcm-kona-usb2.o > diff --git a/drivers/phy/phy-da8xx-usb.c b/drivers/phy/phy-da8xx-usb.c > new file mode 100644 > index 0000000..93a5f4d > --- /dev/null > +++ b/drivers/phy/phy-da8xx-usb.c > @@ -0,0 +1,295 @@ > +/* > + * phy-da8xx-usb - TI DaVinci DA8XX USB PHY driver > + * > + * Copyright (C) 2016 David Lechner > + * > + * 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; either version 2 of the License, or > + * (at your option) any later version. > + * > + * 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 > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +/* DA8xx CFGCHIP2 (USB PHY Control) register bits */ > +#define PHYCLKGD (1 << 17) > +#define VBUSSENSE (1 << 16) > +#define RESET (1 << 15) > +#define OTGMODE_MASK (3 << 13) > +#define NO_OVERRIDE (0 << 13) > +#define FORCE_HOST (1 << 13) > +#define FORCE_DEVICE (2 << 13) > +#define FORCE_HOST_VBUS_LOW (3 << 13) > +#define USB1PHYCLKMUX (1 << 12) > +#define USB2PHYCLKMUX (1 << 11) > +#define PHYPWRDN (1 << 10) > +#define OTGPWRDN (1 << 9) > +#define DATPOL (1 << 8) > +#define USB1SUSPENDM (1 << 7) > +#define PHY_PLLON (1 << 6) > +#define SESENDEN (1 << 5) > +#define VBDTCTEN (1 << 4) > +#define REFFREQ_MASK (0xf << 0) > +#define REFFREQ_12MHZ (1 << 0) > +#define REFFREQ_24MHZ (2 << 0) > +#define REFFREQ_48MHZ (3 << 0) > +#define REFFREQ_19_2MHZ (4 << 0) > +#define REFFREQ_38_4MHZ (5 << 0) > +#define REFFREQ_13MHZ (6 << 0) > +#define REFFREQ_26MHZ (7 << 0) > +#define REFFREQ_20MHZ (8 << 0) > +#define REFFREQ_40MHZ (9 << 0) > + > +struct da8xx_usbphy { > + struct phy_provider *phy_provider; > + struct phy *usb11_phy; > + struct phy *usb20_phy; > + struct clk *usb11_clk; > + struct clk *usb20_clk; > + void __iomem *phy_ctrl; > +}; > + > +static inline u32 da8xx_usbphy_readl(void __iomem *base) > +{ > + return readl(base); > +} > + > +static inline void da8xx_usbphy_writel(void __iomem *base, u32 value) > +{ > + writel(value, base); > +} > + > +static int da8xx_usb11_phy_init(struct phy *phy) > +{ > + struct da8xx_usbphy *d_phy = phy_get_drvdata(phy); > + int ret; > + u32 val; > + > + ret = clk_prepare_enable(d_phy->usb11_clk); > + if (ret) > + return ret; > + > + val = da8xx_usbphy_readl(d_phy->phy_ctrl); > + val |= USB1SUSPENDM; > + da8xx_usbphy_writel(d_phy->phy_ctrl, val); > + > + return 0; > +} > + > +static int da8xx_usb11_phy_shutdown(struct phy *phy) > +{ > + struct da8xx_usbphy *d_phy = phy_get_drvdata(phy); > + u32 val; > + > + val = da8xx_usbphy_readl(d_phy->phy_ctrl); > + val &= ~USB1SUSPENDM; > + da8xx_usbphy_writel(d_phy->phy_ctrl, val); > + > + clk_disable_unprepare(d_phy->usb11_clk); > + > + return 0; > +} > + > +static const struct phy_ops da8xx_usb11_phy_ops = { > + .power_on = da8xx_usb11_phy_init, > + .power_off = da8xx_usb11_phy_shutdown, > + .owner = THIS_MODULE, > +}; > + > +static int da8xx_usb20_phy_init(struct phy *phy) > +{ > + struct da8xx_usbphy *d_phy = phy_get_drvdata(phy); > + int ret; > + u32 val; > + > + ret = clk_prepare_enable(d_phy->usb20_clk); > + if (ret) > + return ret; > + > + val = da8xx_usbphy_readl(d_phy->phy_ctrl); > + val &= ~OTGPWRDN; > + da8xx_usbphy_writel(d_phy->phy_ctrl, val); > + > + return 0; > +} > + > +static int da8xx_usb20_phy_shutdown(struct phy *phy) > +{ > + struct da8xx_usbphy *d_phy = phy_get_drvdata(phy); > + u32 val; > + > + val = da8xx_usbphy_readl(d_phy->phy_ctrl); > + val |= OTGPWRDN; > + da8xx_usbphy_writel(d_phy->phy_ctrl, val); > + > + clk_disable_unprepare(d_phy->usb20_clk); > + > + return 0; > +} > + > +static const struct phy_ops da8xx_usb20_phy_ops = { > + .power_on = da8xx_usb20_phy_init, > + .power_off = da8xx_usb20_phy_shutdown, > + .owner = THIS_MODULE, > +}; > + > +int da8xx_usb20_phy_set_mode(struct phy *phy, enum musb_mode mode) > +{ > + struct da8xx_usbphy *d_phy = phy_get_drvdata(phy); > + u32 val; > + > + val = da8xx_usbphy_readl(d_phy->phy_ctrl); > + > + val &= ~OTGMODE_MASK; > + switch (mode) { > + case MUSB_HOST: /* Force VBUS valid, ID = 0 */ > + val |= FORCE_HOST; > + break; > + case MUSB_PERIPHERAL: /* Force VBUS valid, ID = 1 */ > + val |= FORCE_DEVICE; > + break; > + case MUSB_OTG: /* Don't override the VBUS/ID comparators */ > + val |= NO_OVERRIDE; > + break; > + default: > + return -EINVAL; > + } > + > + da8xx_usbphy_writel(d_phy->phy_ctrl, val); > + > + return 0; > +} > +EXPORT_SYMBOL_GPL(da8xx_usb20_phy_set_mode); Don't prefer export symbols from PHY driver. That'll create unnecessary dependencies between the controller and the PHY. I think it'll be better to create a new attribute and use it? 8<============ diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h index 8cf05e3..3d92b9b 100644 --- a/include/linux/phy/phy.h +++ b/include/linux/phy/phy.h @@ -22,6 +22,13 @@ struct phy; +enum phy_mode { + PHY_MODE_INVALID, + PHY_MODE_USB_HOST, + PHY_MODE_USB_DEVICE, + PHY_MODE_USB_OTG, +}; + /** * struct phy_ops - set of function pointers for performing phy operations * @init: operation to be performed for initializing phy @@ -44,6 +51,7 @@ struct phy_ops { */ struct phy_attrs { u32 bus_width; + enum phy_mode mode; }; /** @@ -127,6 +135,14 @@ static inline void phy_set_bus_width(struct phy *phy, int bus_width) { phy->attrs.bus_width = bus_width; } +static inline enum phy_mode phy_get_mode(struct phy *phy) +{ + return phy->attrs.mode; +} +static inline void phy_set_mode(struct phy *phy, enum phy_mode mode) +{ + phy->attrs.mode = mode; +} struct phy *phy_get(struct device *dev, const char *string); struct phy *phy_optional_get(struct device *dev, const char *string); struct phy *devm_phy_get(struct device *dev, const char *string); @@ -234,6 +250,16 @@ static inline void phy_set_bus_width(struct phy *phy, int bus_width) return; } +static inline enum phy_mode phy_get_mode(struct phy *phy) +{ + return PHY_MODE_INVALID; +} + +static inline void phy_set_mode(struct phy *phy, enum phy_mode mode) +{ + return; +} + static inline struct phy *phy_get(struct device *dev, const char *string) { return ERR_PTR(-ENOSYS); =====================>8 Thanks Kishon > + > +static struct phy *da8xx_usbphy_of_xlate(struct device *dev, > + struct of_phandle_args *args) > +{ > + struct da8xx_usbphy *d_phy = dev_get_drvdata(dev); > + > + if (!d_phy) > + return ERR_PTR(-ENODEV); > + > + switch (args->args[0]) { > + case 1: > + return d_phy->usb11_phy; > + case 2: > + return d_phy->usb20_phy; > + default: > + return ERR_PTR(-EINVAL); > + } > +} > + > +static int da8xx_usbphy_probe(struct platform_device *pdev) > +{ > + struct device *dev = &pdev->dev; > + struct device_node *node = dev->of_node; > + struct da8xx_usbphy *d_phy; > + struct resource *res; > + > + d_phy = devm_kzalloc(dev, sizeof(*d_phy), GFP_KERNEL); > + if (!d_phy) > + return -ENOMEM; > + > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > + d_phy->phy_ctrl = devm_ioremap_resource(dev, res); > + if (IS_ERR(d_phy->phy_ctrl)) { > + dev_err(dev, "Failed to map resource.\n"); > + return PTR_ERR(d_phy->phy_ctrl); > + } > + > + d_phy->usb11_clk = devm_clk_get(dev, "usb11_phy"); > + if (IS_ERR(d_phy->usb11_clk)) { > + dev_err(dev, "Failed to get usb11_phy clock.\n"); > + return PTR_ERR(d_phy->usb11_clk); > + } > + > + d_phy->usb20_clk = devm_clk_get(dev, "usb20_phy"); > + if (IS_ERR(d_phy->usb20_clk)) { > + dev_err(dev, "Failed to get usb20_phy clock.\n"); > + return PTR_ERR(d_phy->usb20_clk); > + } > + > + d_phy->usb11_phy = devm_phy_create(dev, node, &da8xx_usb11_phy_ops); > + if (IS_ERR(d_phy->usb11_phy)) { > + dev_err(dev, "Failed to create usb11 phy.\n"); > + return PTR_ERR(d_phy->usb11_phy); > + } > + > + d_phy->usb20_phy = devm_phy_create(dev, node, &da8xx_usb20_phy_ops); > + if (IS_ERR(d_phy->usb20_phy)) { > + dev_err(dev, "Failed to create usb20 phy.\n"); > + return PTR_ERR(d_phy->usb20_phy); > + } > + > + platform_set_drvdata(pdev, d_phy); > + phy_set_drvdata(d_phy->usb11_phy, d_phy); > + phy_set_drvdata(d_phy->usb20_phy, d_phy); > + > + if (node) { > + d_phy->phy_provider = devm_of_phy_provider_register(dev, > + da8xx_usbphy_of_xlate); > + if (IS_ERR(d_phy->phy_provider)) { > + dev_err(dev, "Failed to create phy provider.\n"); > + return PTR_ERR(d_phy->phy_provider); > + } > + } else { > + int ret; > + > + ret = phy_create_lookup(d_phy->usb11_phy, "usbphy", "ohci.0"); > + if (ret) > + dev_warn(dev, "Failed to create usb11 phy lookup .\n"); > + ret = phy_create_lookup(d_phy->usb20_phy, "usbphy", "musb-da8xx"); > + if (ret) > + dev_warn(dev, "Failed to create usb20 phy lookup .\n"); > + } > + > + return 0; > +} > + > +static int da8xx_usbphy_remove(struct platform_device *pdev) > +{ > + struct da8xx_usbphy *d_phy = platform_get_drvdata(pdev); > + > + if (!pdev->dev.of_node) { > + phy_remove_lookup(d_phy->usb20_phy, "usbphy", "musb-da8xx"); > + phy_remove_lookup(d_phy->usb11_phy, "usbphy", "ohci.0"); > + } > + > + return 0; > +} > + > +static const struct of_device_id da8xx_usbphy_ids[] = { > + { .compatible = "ti,da830-usbphy" }, > + { } > +}; > +MODULE_DEVICE_TABLE(of, da8xx_usbphy_ids); > + > +static struct platform_driver da8xx_usbphy_driver = { > + .probe = da8xx_usbphy_probe, > + .remove = da8xx_usbphy_remove, > + .driver = { > + .name = "da8xx-usbphy", > + .of_match_table = da8xx_usbphy_ids, > + }, > +}; > + > +module_platform_driver(da8xx_usbphy_driver); > + > +MODULE_ALIAS("platform:da8xx-usbphy"); > +MODULE_AUTHOR("David Lechner "); > +MODULE_DESCRIPTION("TI DA8XX USB PHY driver"); > +MODULE_LICENSE("GPL v2"); >