Hi, Kishon > -----Original Message----- > From: Kishon Vijay Abraham I [mailto:kis...@ti.com] > Sent: Monday, July 04, 2016 6:59 PM > To: Lipengcheng; linux-kernel@vger.kernel.org; Lidongpo > Subject: Re: [PATCH v2] phy: add phy-hisi-inno-usb2 > > Hi, > > On Sunday 03 July 2016 12:20 PM, l00229106 wrote: > > Add support for inno usb2 phy integrated on some hisilicon SOCs. > > > > Signed-off-by: Pengcheng Li <lpc...@hisilicon.com> > > --- > > .../devicetree/bindings/phy/phy-hisi-inno-usb2.txt | 48 ++++ > > drivers/phy/Kconfig | 10 + > > drivers/phy/Makefile | 1 + > > drivers/phy/phy-hisi-inno-usb2.c | 298 > > +++++++++++++++++++++ > > 4 files changed, 357 insertions(+) > > create mode 100644 > > Documentation/devicetree/bindings/phy/phy-hisi-inno-usb2.txt > > create mode 100644 drivers/phy/phy-hisi-inno-usb2.c > > > > diff --git > > a/Documentation/devicetree/bindings/phy/phy-hisi-inno-usb2.txt > > b/Documentation/devicetree/bindings/phy/phy-hisi-inno-usb2.txt > > new file mode 100644 > > index 0000000..59eaf73 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/phy/phy-hisi-inno-usb2.txt > > @@ -0,0 +1,48 @@ > > +HiSilicon INNO USB2 PHY > > +----------------------- > > +Required properties: > > +- compatible: Should be "hisilicon,inno_usb2_phy" > > +- #phy-cells: Must be 0 > > +- hisilicon,peripheral-syscon: Phandle of syscon used to control phy. > > +- hisilicon,reg-num: Number of phy registers which should be > > +configured at phy intialization stage > > +- hisilicon,reg-seq: Sequence of triplets of (address, value, delay-us). > > +The number of triplets is equal to "hisilicon,reg-num". Each triplet > > +is used to write one phy register. The delay-us cell represents the > > +delay time in microseconds to be applied after each write. > > +- clocks: Phandle and clock specifier pair for reference clock utmi_refclk. > > +- resets: List of phandle and reset specifier pairs for each reset > > +signal in reset-names. > > +- reset-names: Should be "por_rst" and "test_rst". The test_rst only > > +exists in some of SOCs, so it is optional. > > + > > +Phy node can includes up to four subnodes. Each subnode represents one > > port. > > +The required properties of port node are as follows: > > +- clocks: Phandle and clock specifier pair for utmi_clock. > > +- resets: List of phandle and reset specifier pairs for port reset and > > utmi reset. > > +- reset-names: List of reset signal names. Should be "port_rst" and > > "utmi_rst" > > + > > +Refer to phy/phy-bindings.txt for the generic PHY binding properties > > + > > +Example: > > +usb_phy: phy { > > + compatible = "hisilicon,inno_usb2_phy"; > > + #phy-cells = <0>; > > + hisilicon,peripheral-syscon = <&peri_ctrl>; > > + hisilicon,reg-num = <7>; > > + hisilicon,reg-seq = <0x80 0x800000 20>, > > + <0x80 0xa0060c 200>, > > + <0x80 0x80001c 20>, > > + <0x80 0xa0001c 20>, > > + <0x80 0x80060f 20>, > > + <0x80 0xa0060f 20>, > > + <0x80 0x800a4b 20>; > > + clocks = <&crg USB2_REF_CLK>; > > + resets = <&crg 0xb4 2>; > > + reset-names = "por_rst"; > > + port0 { > > + clocks = <&crg USB2_UTMI0_CLK>; > > + resets = <&crg 0xb4 5>, <&crg 0xb4 1>; > > + reset-names = "port_rst", "utmi_rst"; > > + }; > > + }; > > diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig index > > 26566db..8f043ca 100644 > > --- a/drivers/phy/Kconfig > > +++ b/drivers/phy/Kconfig > > @@ -205,6 +205,16 @@ config PHY_EXYNOS5250_SATA > > SATA 3.0 Gb/s, SATA 6.0 Gb/s speeds. It supports one SATA host > > port to accept one SATA device. > > > > +config PHY_HISI_INNO_USB2 > > + tristate "Hisilicon Inno USB2 PHY support" > > + depends on (ARCH_HISI) || COMPILE_TEST > > + select GENERIC_PHY > > + select MFD_SYSCON > > + help > > + Support for INNO PHY on Hisilicon Socs. This Phy supports > > + USB 1.5Mb/s, USB 12Mb/s, USB 480Mb/s speeds. It suppots one > > + USB host port to accept one USB device. > > + > > config PHY_HIX5HD2_SATA > > tristate "HIX5HD2 SATA PHY Driver" > > depends on ARCH_HIX5HD2 && OF && HAS_IOMEM diff --git > > a/drivers/phy/Makefile b/drivers/phy/Makefile index 24596a9..ef6a24b > > 100644 > > --- a/drivers/phy/Makefile > > +++ b/drivers/phy/Makefile > > @@ -24,6 +24,7 @@ obj-$(CONFIG_TI_PIPE3) += > > phy-ti-pipe3.o > > obj-$(CONFIG_TWL4030_USB) += phy-twl4030-usb.o > > obj-$(CONFIG_PHY_EXYNOS5250_SATA) += phy-exynos5250-sata.o > > obj-$(CONFIG_PHY_HIX5HD2_SATA) += phy-hix5hd2-sata.o > > +obj-$(CONFIG_PHY_HISI_INNO_USB2) += phy-hisi-inno-usb2.o > > obj-$(CONFIG_PHY_HI6220_USB) += phy-hi6220-usb.o > > obj-$(CONFIG_PHY_MT65XX_USB3) += phy-mt65xx-usb3.o > > obj-$(CONFIG_PHY_SUN4I_USB) += phy-sun4i-usb.o > > diff --git a/drivers/phy/phy-hisi-inno-usb2.c > > b/drivers/phy/phy-hisi-inno-usb2.c > > new file mode 100644 > > index 0000000..6930bb0 > > --- /dev/null > > +++ b/drivers/phy/phy-hisi-inno-usb2.c > > @@ -0,0 +1,298 @@ > > + /* > > + * HiSilicon INNO USB2 PHY Driver. > > + * > > + * Copyright (c) 2016 HiSilicon Technologies Co., Ltd. > > + * > > + * 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. > > + * > > + * You should have received a copy of the GNU General Public License > > + * along with this program. If not, see <http://www.gnu.org/licenses/>. > > + */ > > + > > +#include <linux/clk.h> > > +#include <linux/delay.h> > > +#include <linux/mfd/syscon.h> > > +#include <linux/module.h> > > +#include <linux/platform_device.h> > > +#include <linux/phy/phy.h> > > +#include <linux/regmap.h> > > +#include <linux/reset.h> > > + > > +#define MAX_PORTS 4 > > + > > +struct hisi_inno_phy_port { > > + struct clk *utmi_clk; > > + struct reset_control *port_rst; > > + struct reset_control *utmi_rst; > > +}; > > + > > +struct hisi_inno_phy_priv { > > + struct regmap *reg_peri; > > + struct clk *ref_clk; > > + struct reset_control *test_rst; > > + struct reset_control *por_rst; > > + struct reg_sequence *reg_seq; > > + u32 reg_num; > > + struct hisi_inno_phy_port *ports; > > + u8 port_num; > > +}; > > + > > +static int hisi_inno_phy_setup(struct hisi_inno_phy_priv *priv) { > > + return regmap_multi_reg_write_bypassed(priv->reg_peri, > > + priv->reg_seq, priv->reg_num); > > +} > > + > > +static int hisi_inno_port_init(struct hisi_inno_phy_port *port) { > > + int ret = 0; > > + > > + reset_control_deassert(port->port_rst); > > + mdelay(2); > > + > > + ret = clk_prepare_enable(port->utmi_clk); > > + if (ret) > > + return ret; > > + udelay(200); > > + > > + reset_control_deassert(port->utmi_rst); > > + udelay(200); > > + > > + return 0; > > +} > > + > > +static int hisi_inno_phy_init(struct phy *phy) { > > + struct hisi_inno_phy_priv *priv = phy_get_drvdata(phy); > > + int ret, port; > > + > > + ret = clk_prepare_enable(priv->ref_clk); > > + if (ret) > > + return ret; > > + udelay(100); > > + > > + if (priv->test_rst) { > > + reset_control_deassert(priv->test_rst); > > + udelay(100); > > + } > > + > > + reset_control_deassert(priv->por_rst); > > + udelay(300); > > + > > + /* config phy clk and phy eye diagram */ > > + ret = hisi_inno_phy_setup(priv); > > + if (ret) > > + goto err_disable_ref_clk; > > + > > + for (port = 0; port < priv->port_num; port++) { > > + ret = hisi_inno_port_init(&priv->ports[port]); > > + if (ret) > > + goto err_disable_clks; > > + } > > + > > + return 0; > > + > > +err_disable_clks: > > + while (--port >= 0) > > + clk_disable_unprepare(priv->ports[port].utmi_clk); > > +err_disable_ref_clk: > > + clk_disable_unprepare(priv->ref_clk); > > + > > + return ret; > > +} > > + > > +static void hisi_inno_phy_disable(struct phy *phy) { > > + struct hisi_inno_phy_priv *priv = phy_get_drvdata(phy); > > + int i; > > + > > + for (i = 0; i < priv->port_num; i++) > > + clk_disable_unprepare(priv->ports[i].utmi_clk); > > + > > + clk_disable_unprepare(priv->ref_clk); > > +} > > + > > +static int hisi_inno_phy_of_get_ports(struct device *dev, > > + struct hisi_inno_phy_priv *priv) { > > + struct device_node *node = dev->of_node, *child; > > + int port = 0, ret = 0; > > + > > + priv->port_num = of_get_child_count(node); > > + if (priv->port_num > MAX_PORTS) { > > + dev_err(dev, "too many ports : %d (max = %d)\n", > > + priv->port_num, MAX_PORTS); > > + return -EINVAL; > > + } > > + > > + priv->ports = devm_kcalloc(dev, priv->port_num, > > + sizeof(struct hisi_inno_phy_port), GFP_KERNEL); > > + if (!priv->ports) > > + return -ENOMEM; > > + > > + for_each_child_of_node(node, child) { > > + struct hisi_inno_phy_port *phy_port = &priv->ports[port]; > > + > > + phy_port->utmi_clk = of_clk_get(child, 0); > > + if (IS_ERR(phy_port->utmi_clk)) { > > + ret = PTR_ERR(phy_port->utmi_clk); > > + goto fail; > > + } > > + > > + phy_port->port_rst = of_reset_control_get(child, "port_rst"); > > + if (IS_ERR(phy_port->port_rst)) { > > + ret = PTR_ERR(phy_port->port_rst); > > + clk_put(phy_port->utmi_clk); > > + goto fail; > > + } > > + > > + phy_port->utmi_rst = of_reset_control_get(child, "utmi_rst"); > > + if (IS_ERR(phy_port->utmi_rst)) { > > + ret = PTR_ERR(phy_port->utmi_rst); > > + reset_control_put(phy_port->port_rst); > > + clk_put(phy_port->utmi_clk); > > + goto fail; > > + } > > + port++; > > + } > > + > > + return ret; > > + > > +fail: > > + while (--port >= 0) { > > + struct hisi_inno_phy_port *phy_port = &priv->ports[port]; > > + > > + reset_control_put(phy_port->utmi_rst); > > + reset_control_put(phy_port->port_rst); > > + clk_put(phy_port->utmi_clk); > > + } > > + of_node_put(child); > > + > > + return ret; > > +} > > + > > +static struct phy_ops hisi_inno_phy_ops = { > > + .owner = THIS_MODULE, > > +}; > > There is no phy ops here ^^^. > > + > > +static int hisi_inno_phy_probe(struct platform_device *pdev) { > > + struct phy_provider *phy_provider; > > + struct device *dev = &pdev->dev; > > + struct phy *phy; > > + struct hisi_inno_phy_priv *priv; > > + struct device_node *node = dev->of_node; > > + int ret = 0; > > + > > + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL); > > + if (!priv) > > + return -ENOMEM; > > + > > + ret = of_property_read_u32(node, "hisilicon,reg-num", &priv->reg_num); > > + if (ret) { > > + dev_err(dev, "can't read the phy registers number!\n"); > > + return ret; > > + } > > + > > + priv->reg_seq = devm_kcalloc(dev, priv->reg_num, > > + sizeof(struct reg_sequence), GFP_KERNEL); > > + if (!priv->reg_seq) > > + return -ENOMEM; > > + > > + ret = of_property_read_u32_array(node, "hisilicon,reg-seq", > > + (u32 *)priv->reg_seq, 3 * priv->reg_num); > > + if (ret) { > > + dev_err(dev, "can't read the phy offset and value!\n"); > > + return ret; > > + } > > + > > + priv->reg_peri = syscon_regmap_lookup_by_phandle(node, > > + "hisilicon,peripheral-syscon"); > > + if (IS_ERR(priv->reg_peri)) { > > + dev_err(dev, "no hisilicon,peripheral-syscon\n"); > > + return PTR_ERR(priv->reg_peri); > > + } > > + > > + priv->ref_clk = devm_clk_get(dev, NULL); > > + if (IS_ERR(priv->ref_clk)) > > + return PTR_ERR(priv->ref_clk); > > + > > + priv->por_rst = devm_reset_control_get(dev, "por_rst"); > > + if (IS_ERR(priv->por_rst)) > > + return PTR_ERR(priv->por_rst); > > + > > + priv->test_rst = devm_reset_control_get(dev, "test_rst"); > > + if (IS_ERR(priv->test_rst)) > > + priv->test_rst = NULL; > > + > > + ret = hisi_inno_phy_of_get_ports(dev, priv); > > + if (ret) > > + return ret; > > + > > + phy = devm_phy_create(dev, NULL, &hisi_inno_phy_ops); > > + if (IS_ERR(phy)) > > + return PTR_ERR(phy); > > But you register it with the phy framework. Do you really intend to use the > PHY framework for some reason. > I do not use the PHY framework. I will delete the code about the PHY framework in next patch.
> Thanks > Kishon Thanks Pengcheng Li
