linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC 00/10] Add USB support for Hikey 970
@ 2020-09-04 10:23 Mauro Carvalho Chehab
  2020-09-04 10:23 ` [RFC 01/10] phy: hisilicon: add USB physical layer for Kirin 3670 Mauro Carvalho Chehab
                   ` (10 more replies)
  0 siblings, 11 replies; 22+ messages in thread
From: Mauro Carvalho Chehab @ 2020-09-04 10:23 UTC (permalink / raw)
  Cc: linuxarm, mauro.chehab, Mauro Carvalho Chehab, John Stultz,
	Manivannan Sadhasivam, Robin Murphy, Krzysztof Kozlowski,
	linux-kernel, Mark Brown, Greg Kroah-Hartman, Derek Kiernan,
	Arnd Bergmann, Vinod Koul, Kishon Vijay Abraham I,
	David S. Miller, Rob Herring, Yu Chen, Rob Herring,
	Dragan Cvetic, linux-arm-kernel, devicetree, Liam Girdwood,
	Wei Xu

This RFC adds what seems to be needed for USB to work with Hikey 970.
While this driver works fine on Kernel 4.9 and 4.19, there's a hack there,
in the form of some special binding logic under dwg3 driver,  that seems to
 be just adding some delay,  and doing some different initializations at
PM (basically, disabling autosuspend).

With upstream Kernel, however, I'm getting a hard to track
Kernel panic:

[    1.837458] SError Interrupt on CPU0, code 0xbf000002 -- SError
[    1.837462] CPU: 0 PID: 74 Comm: kworker/0:1 Not tainted 5.8.0+ #205
[    1.837463] Hardware name: HiKey970 (DT)
[    1.837465] Workqueue: events deferred_probe_work_func
[    1.837467] pstate: 20000005 (nzCv daif -PAN -UAO BTYPE=--)
[    1.837468] pc : _raw_spin_unlock_irqrestore+0x18/0x50
[    1.837469] lr : regmap_unlock_spinlock+0x14/0x20
[    1.837470] sp : ffff8000124dba60
[    1.837471] x29: ffff8000124dba60 x28: 0000000000000000 
[    1.837474] x27: ffff0001b7e854c8 x26: ffff80001204ea18 
[    1.837476] x25: 0000000000000005 x24: ffff800011f918f8 
[    1.837479] x23: ffff800011fbb588 x22: ffff0001b7e40e00 
[    1.837481] x21: 0000000000000100 x20: 0000000000000000 
[    1.837483] x19: ffff0001b767ec00 x18: 00000000ff10c000 
[    1.837485] x17: 0000000000000002 x16: 0000b0740fdb9950 
[    1.837488] x15: ffff8000116c1198 x14: ffffffffffffffff 
[    1.837490] x13: 0000000000000030 x12: 0101010101010101 
[    1.837493] x11: 0000000000000020 x10: ffff0001bf17d130 
[    1.837495] x9 : 0000000000000000 x8 : ffff0001b6938080 
[    1.837497] x7 : 0000000000000000 x6 : 000000000000003f 
[    1.837500] x5 : 0000000000000000 x4 : 0000000000000000 
[    1.837502] x3 : ffff80001096a880 x2 : 0000000000000000 
[    1.837505] x1 : ffff0001b7e40e00 x0 : 0000000100000001 
[    1.837507] Kernel panic - not syncing: Asynchronous SError Interrupt
[    1.837509] CPU: 0 PID: 74 Comm: kworker/0:1 Not tainted 5.8.0+ #205
[    1.837510] Hardware name: HiKey970 (DT)
[    1.837511] Workqueue: events deferred_probe_work_func
[    1.837513] Call trace:
[    1.837514]  dump_backtrace+0x0/0x1e0
[    1.837515]  show_stack+0x18/0x24
[    1.837516]  dump_stack+0xc0/0x11c
[    1.837517]  panic+0x15c/0x324
[    1.837518]  nmi_panic+0x8c/0x90
[    1.837519]  arm64_serror_panic+0x78/0x84
[    1.837520]  do_serror+0x158/0x15c
[    1.837521]  el1_error+0x84/0x100
[    1.837522]  _raw_spin_unlock_irqrestore+0x18/0x50
[    1.837523]  regmap_write+0x58/0x80
[    1.837524]  hi3660_reset_deassert+0x28/0x34
[    1.837526]  reset_control_deassert+0x50/0x260
[    1.837527]  reset_control_deassert+0xf4/0x260
[    1.837528]  dwc3_probe+0x5dc/0xe6c
[    1.837529]  platform_drv_probe+0x54/0xb0
[    1.837530]  really_probe+0xe0/0x490
[    1.837531]  driver_probe_device+0xf4/0x160
[    1.837532]  __device_attach_driver+0x8c/0x114
[    1.837533]  bus_for_each_drv+0x78/0xcc
[    1.837534]  __device_attach+0x108/0x1a0
[    1.837535]  device_initial_probe+0x14/0x20
[    1.837537]  bus_probe_device+0x98/0xa0
[    1.837538]  deferred_probe_work_func+0x88/0xe0
[    1.837539]  process_one_work+0x1cc/0x350
[    1.837540]  worker_thread+0x2c0/0x470
[    1.837541]  kthread+0x154/0x160
[    1.837542]  ret_from_fork+0x10/0x30
[    1.837569] SMP: stopping secondary CPUs
[    1.837570] Kernel Offset: 0x1d0000 from 0xffff800010000000
[    1.837571] PHYS_OFFSET: 0x0
[    1.837572] CPU features: 0x240002,20882004
[    1.837573] Memory Limit: none

I suspect that the driver works downstream because of the extra
delay of probing an additional driver, as porting such driver
to upstream sometimes makes this driver to work. So, it could 
be due to some race condition.

The problem could also be due to something wrong at DT,
as, with the additional driver, the DT bindings are different.

As I'm not familiar about the Serror error mechanism on ARM, 
I'm wondering if someone could give me some hint about how
can I identify what's happening here.  Due to the panic(), I
can't check what wrong happened there. Not sure if it would
be safe to just hack the error handling part of Serror, in order
to let the boot to finish.

PS.: the driver/misc hisi_hikey_usb driver on this series came 
from the John Stultz Hikey 960 tree.

Thanks!
Mauro

Mauro Carvalho Chehab (7):
  phy: hisilicon: phy-hi3670-usb3: use a consistent namespace
  phy: hisilicon: phy-hi3670-usb3: fix coding style
  phy: hisilicon: phy-hi3670-usb3: change some DT properties
  dt-bindings: phy: convert phy-kirin970-usb3.txt to yaml
  MAINTAINERS: add myself as maintainer for Kirin 970 USB PHY
  misc: hisi_hikey_usb: add support for Hikey 970
  dts: hisilicon: add support for USB3 on Hikey 970

Yu Chen (3):
  phy: hisilicon: add USB physical layer for Kirin 3670
  phy: hisilicon: phy-hi3670-usb3: fix some issues at the init code
  misc: hisi_hikey_usb: Driver to support onboard USB gpio hub on
    Hikey960

 .../bindings/phy/hisilicon,hi3670-usb3.yaml   |  72 ++
 MAINTAINERS                                   |  16 +-
 .../boot/dts/hisilicon/hi3670-hikey970.dts    | 103 +++
 arch/arm64/boot/dts/hisilicon/hi3670.dtsi     |  49 ++
 drivers/misc/Kconfig                          |   9 +
 drivers/misc/Makefile                         |   1 +
 drivers/misc/hisi_hikey_usb.c                 | 272 +++++++
 drivers/phy/hisilicon/Kconfig                 |  10 +
 drivers/phy/hisilicon/Makefile                |   1 +
 drivers/phy/hisilicon/phy-hi3670-usb3.c       | 671 ++++++++++++++++++
 10 files changed, 1203 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/phy/hisilicon,hi3670-usb3.yaml
 create mode 100644 drivers/misc/hisi_hikey_usb.c
 create mode 100644 drivers/phy/hisilicon/phy-hi3670-usb3.c

-- 
2.26.2



^ permalink raw reply	[flat|nested] 22+ messages in thread

* [RFC 01/10] phy: hisilicon: add USB physical layer for Kirin 3670
  2020-09-04 10:23 [RFC 00/10] Add USB support for Hikey 970 Mauro Carvalho Chehab
@ 2020-09-04 10:23 ` Mauro Carvalho Chehab
  2020-09-04 10:23 ` [RFC 02/10] phy: hisilicon: phy-hi3670-usb3: fix some issues at the init code Mauro Carvalho Chehab
                   ` (9 subsequent siblings)
  10 siblings, 0 replies; 22+ messages in thread
From: Mauro Carvalho Chehab @ 2020-09-04 10:23 UTC (permalink / raw)
  Cc: linuxarm, mauro.chehab, Yu Chen, Mauro Carvalho Chehab,
	John Stultz, Manivannan Sadhasivam, Robin Murphy,
	Kishon Vijay Abraham I, Vinod Koul, Rob Herring,
	Krzysztof Kozlowski, linux-kernel, devicetree

From: Yu Chen <chenyu56@huawei.com>

Add the Hisilicon Kirin 3670 USB phy driver just after the
hi3660, using the same namespace.

This driver was imported from Linaro's official Hikey 970
tree, from the original patch, removing the addition of
the dwg3-specific parts.

Signed-off-by: Yu Chen <chenyu56@huawei.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
 .../bindings/phy/phy-hi3670-usb3.txt          |  25 +
 drivers/phy/hisilicon/Kconfig                 |  10 +
 drivers/phy/hisilicon/Makefile                |   1 +
 drivers/phy/hisilicon/phy-hi3670-usb3.c       | 682 ++++++++++++++++++
 4 files changed, 718 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/phy/phy-hi3670-usb3.txt
 create mode 100644 drivers/phy/hisilicon/phy-hi3670-usb3.c

diff --git a/Documentation/devicetree/bindings/phy/phy-hi3670-usb3.txt b/Documentation/devicetree/bindings/phy/phy-hi3670-usb3.txt
new file mode 100644
index 000000000000..4cb02612ff23
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/phy-hi3670-usb3.txt
@@ -0,0 +1,25 @@
+Hisilicon Kirin970 usb PHY
+-----------------------
+
+Required properties:
+- compatible: should be "hisilicon,kirin970-usb-phy"
+- #phy-cells: must be 0
+- hisilicon,pericrg-syscon: phandle of syscon used to control phy.
+- hisilicon,pctrl-syscon: phandle of syscon used to control phy.
+- hisilicon,sctrl-syscon: phandle of syscon used to control phy.
+- hisilicon,usb31-misc-syscon: phandle of syscon used to control phy.
+- eye-diagram-param: parameter set for phy
+- usb3-phy-tx-vboost-lvl: parameter set for phy
+Refer to phy/phy-bindings.txt for the generic PHY binding properties
+
+Example:
+	usb_phy: usbphy {
+		compatible = "hisilicon,kirin970-usb-phy";
+		#phy-cells = <0>;
+		hisilicon,pericrg-syscon = <&crg_ctrl>;
+		hisilicon,pctrl-syscon = <&pctrl>;
+		hisilicon,sctrl-syscon = <&sctrl>;
+		hisilicon,usb31-misc-syscon = <&usb31_misc>;
+		eye-diagram-param = <0xFDFEE4>;
+		usb3-phy-tx-vboost-lvl = <0x5>;
+	};
diff --git a/drivers/phy/hisilicon/Kconfig b/drivers/phy/hisilicon/Kconfig
index 1c73053bcc98..4d008cfc279c 100644
--- a/drivers/phy/hisilicon/Kconfig
+++ b/drivers/phy/hisilicon/Kconfig
@@ -23,6 +23,16 @@ config PHY_HI3660_USB
 
 	  To compile this driver as a module, choose M here.
 
+config PHY_HI3670_USB
+	tristate "hi3670 USB PHY support"
+	depends on (ARCH_HISI && ARM64) || COMPILE_TEST
+	select GENERIC_PHY
+	select MFD_SYSCON
+	help
+	  Enable this to support the HISILICON HI3670 USB PHY.
+
+	  To compile this driver as a module, choose M here.
+
 config PHY_HISTB_COMBPHY
 	tristate "HiSilicon STB SoCs COMBPHY support"
 	depends on (ARCH_HISI && ARM64) || COMPILE_TEST
diff --git a/drivers/phy/hisilicon/Makefile b/drivers/phy/hisilicon/Makefile
index 92e874ae9c74..51729868145b 100644
--- a/drivers/phy/hisilicon/Makefile
+++ b/drivers/phy/hisilicon/Makefile
@@ -1,6 +1,7 @@
 # SPDX-License-Identifier: GPL-2.0-only
 obj-$(CONFIG_PHY_HI6220_USB)		+= phy-hi6220-usb.o
 obj-$(CONFIG_PHY_HI3660_USB)		+= phy-hi3660-usb3.o
+obj-$(CONFIG_PHY_HI3670_USB)		+= phy-hi3670-usb3.o
 obj-$(CONFIG_PHY_HISTB_COMBPHY)		+= phy-histb-combphy.o
 obj-$(CONFIG_PHY_HISI_INNO_USB2)	+= phy-hisi-inno-usb2.o
 obj-$(CONFIG_PHY_HIX5HD2_SATA)		+= phy-hix5hd2-sata.o
diff --git a/drivers/phy/hisilicon/phy-hi3670-usb3.c b/drivers/phy/hisilicon/phy-hi3670-usb3.c
new file mode 100644
index 000000000000..4e04ac97728d
--- /dev/null
+++ b/drivers/phy/hisilicon/phy-hi3670-usb3.c
@@ -0,0 +1,682 @@
+/*
+ * Phy provider for USB 3.1 controller on HiSilicon Kirin970 platform
+ *
+ * Copyright (C) 2017-2018 Hilisicon Electronics Co., Ltd.
+ *		http://www.huawei.com
+ *
+ * Authors: Yu Chen <chenyu56@huawei.com>
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2  of
+ * the License 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/kernel.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/clk.h>
+
+#define SCTRL_SCDEEPSLEEPED		(0x0)
+#define USB_CLK_SELECTED		BIT(20)
+
+#define PERI_CRG_PEREN0			(0x00)
+#define PERI_CRG_PERDIS0		(0x04)
+#define PERI_CRG_PEREN4			(0x40)
+#define PERI_CRG_PERDIS4		(0x44)
+#define PERI_CRG_PERRSTEN4		(0x90)
+#define PERI_CRG_PERRSTDIS4		(0x94)
+#define PERI_CRG_ISODIS			(0x148)
+#define PERI_CRG_PEREN6			(0x410)
+#define PERI_CRG_PERDIS6		(0x414)
+
+#define USB_REFCLK_ISO_EN		BIT(25)
+
+#define GT_CLK_USB2PHY_REF		BIT(19)
+
+#define PCTRL_PERI_CTRL3		(0x10)
+#define PCTRL_PERI_CTRL3_MSK_START	(16)
+#define USB_TCXO_EN			BIT(1)
+
+#define PCTRL_PERI_CTRL24		(0x64)
+#define SC_CLK_USB3PHY_3MUX1_SEL	BIT(25)
+
+#define USB3OTG_CTRL0			(0x00)
+#define USB3OTG_CTRL3			(0x0C)
+#define USB3OTG_CTRL4			(0x10)
+#define USB3OTG_CTRL5			(0x14)
+#define USB3OTG_CTRL7			(0x1C)
+#define USB_MISC_CFG50			(0x50)
+#define USB_MISC_CFG54			(0x54)
+#define USB_MISC_CFG58			(0x58)
+#define USB_MISC_CFG5C			(0x5C)
+#define USB_MISC_CFGA0			(0xA0)
+#define TCA_CLK_RST			(0x200)
+#define TCA_INTR_EN			(0x204)
+#define TCA_INTR_STS			(0x208)
+#define TCA_GCFG			(0x210)
+#define TCA_TCPC			(0x214)
+#define TCA_VBUS_CTRL			(0x240)
+
+#define CTRL0_USB3_VBUSVLD		BIT(7)
+#define CTRL0_USB3_VBUSVLD_SEL		BIT(6)
+
+#define CTRL3_USB2_VBUSVLDEXT0		BIT(6)
+#define CTRL3_USB2_VBUSVLDEXTSEL0	BIT(5)
+
+#define CTRL5_USB2_SIDDQ		BIT(0)
+
+#define CTRL7_USB2_REFCLKSEL_MASK	(3 << 3)
+#define CTRL7_USB2_REFCLKSEL_ABB	(3 << 3)
+#define CTRL7_USB2_REFCLKSEL_PAD	(2 << 3)
+
+#define CFG50_USB3_PHY_TEST_POWERDOWN	BIT(23)
+
+#define CFG54_USB31PHY_CR_ADDR_MASK	(0xFFFF)
+#define CFG54_USB31PHY_CR_ADDR_SHIFT	(16)
+#define CFG54_USB3PHY_REF_USE_PAD	BIT(12)
+#define CFG54_PHY0_PMA_PWR_STABLE	BIT(11)
+#define CFG54_PHY0_PCS_PWR_STABLE	BIT(9)
+#define CFG54_USB31PHY_CR_ACK		BIT(7)
+#define CFG54_USB31PHY_CR_WR_EN		BIT(5)
+#define CFG54_USB31PHY_CR_SEL		BIT(4)
+#define CFG54_USB31PHY_CR_RD_EN		BIT(3)
+#define CFG54_USB31PHY_CR_CLK		BIT(2)
+#define CFG54_USB3_PHY0_ANA_PWR_EN	BIT(1)
+
+#define CFG58_USB31PHY_CR_DATA_MASK     (0xFFFF)
+#define CFG58_USB31PHY_CR_DATA_RD_START (16)
+
+#define CFG5C_USB3_PHY0_SS_MPLLA_SSC_EN	BIT(1)
+
+#define CFGA0_VAUX_RESET		BIT(9)
+#define CFGA0_USB31C_RESET		BIT(8)
+#define CFGA0_USB2PHY_REFCLK_SELECT	BIT(4)
+#define CFGA0_USB3PHY_RESET		BIT(1)
+#define CFGA0_USB2PHY_POR		BIT(0)
+
+#define INTR_EN_XA_TIMEOUT_EVT_EN	BIT(1)
+#define INTR_EN_XA_ACK_EVT_EN		BIT(0)
+
+#define CLK_RST_TCA_REF_CLK_EN		BIT(1)
+#define CLK_RST_SUSPEND_CLK_EN		BIT(0)
+
+#define GCFG_ROLE_HSTDEV		BIT(4)
+
+#define TCPC_VALID			BIT(4)
+#define TCPC_LOW_POWER_EN		BIT(3)
+#define TCPC_MUX_CONTROL_MASK		(3 << 0)
+#define TCPC_MUX_CONTROL_USB31		(1 << 0)
+
+#define VBUS_CTRL_POWERPRESENT_OVERRD	(3 << 2)
+#define VBUS_CTRL_VBUSVALID_OVERRD	(3 << 0)
+
+#define KIRIN970_USB_DEFAULT_PHY_PARAM	(0xFDFEE4)
+#define KIRIN970_USB_DEFAULT_PHY_VBOOST	(0x5)
+
+#define TX_VBOOST_LVL_REG		(0xf)
+#define TX_VBOOST_LVL_START		(6)
+#define TX_VBOOST_LVL_ENABLE		BIT(9)
+
+struct kirin970_priv {
+	struct device *dev;
+	struct regmap *peri_crg;
+	struct regmap *pctrl;
+	struct regmap *sctrl;
+	struct regmap *usb31misc;
+
+	u32 eye_diagram_param;
+	u32 tx_vboost_lvl;
+
+	u32 peri_crg_offset;
+	u32 pctrl_offset;
+	u32 usb31misc_offset;
+};
+
+static int kirin970_phy_cr_clk(struct regmap *usb31misc)
+{
+	int ret;
+
+	/* Clock up */
+	ret = regmap_update_bits(usb31misc, USB_MISC_CFG54,
+			CFG54_USB31PHY_CR_CLK, CFG54_USB31PHY_CR_CLK);
+	if (ret)
+		return ret;
+
+	/* Clock down */
+	ret = regmap_update_bits(usb31misc, USB_MISC_CFG54,
+			CFG54_USB31PHY_CR_CLK, 0);
+
+	return ret;
+}
+
+static int kirin970_phy_cr_set_sel(struct regmap *usb31misc)
+{
+	return regmap_update_bits(usb31misc, USB_MISC_CFG54,
+			CFG54_USB31PHY_CR_SEL, CFG54_USB31PHY_CR_SEL);
+}
+
+static int kirin970_phy_cr_start(struct regmap *usb31misc, int direction)
+{
+	int ret;
+
+	if (direction)
+		ret = regmap_update_bits(usb31misc, USB_MISC_CFG54,
+			CFG54_USB31PHY_CR_WR_EN, CFG54_USB31PHY_CR_WR_EN);
+	else
+		ret = regmap_update_bits(usb31misc, USB_MISC_CFG54,
+			CFG54_USB31PHY_CR_RD_EN, CFG54_USB31PHY_CR_RD_EN);
+
+	if (ret)
+		return ret;
+
+	ret = kirin970_phy_cr_clk(usb31misc);
+	if (ret)
+		return ret;
+
+	ret = regmap_update_bits(usb31misc, USB_MISC_CFG54,
+			CFG54_USB31PHY_CR_RD_EN | CFG54_USB31PHY_CR_WR_EN, 0);
+
+	return ret;
+}
+
+static int kirin970_phy_cr_wait_ack(struct regmap *usb31misc)
+{
+	u32 reg;
+	int retry = 100000;
+	int ret;
+
+	while (retry-- > 0) {
+		ret = regmap_read(usb31misc, USB_MISC_CFG54, &reg);
+		if (ret)
+			return ret;
+		if ((reg & CFG54_USB31PHY_CR_ACK) == CFG54_USB31PHY_CR_ACK)
+			return 0;
+
+		ret = kirin970_phy_cr_clk(usb31misc);
+		if (ret)
+			return ret;
+	}
+
+	return -ETIMEDOUT;
+}
+
+static int kirin970_phy_cr_set_addr(struct regmap *usb31misc, u32 addr)
+{
+	u32 reg;
+	int ret;
+
+	ret = regmap_read(usb31misc, USB_MISC_CFG54, &reg);
+	if (ret)
+		return ret;
+
+	reg &= ~(CFG54_USB31PHY_CR_ADDR_MASK << CFG54_USB31PHY_CR_ADDR_SHIFT);
+	reg |= ((addr & CFG54_USB31PHY_CR_ADDR_MASK) <<
+			CFG54_USB31PHY_CR_ADDR_SHIFT);
+	ret = regmap_write(usb31misc, USB_MISC_CFG54, reg);
+
+	return ret;
+}
+
+static int kirin970_phy_cr_read(struct regmap *usb31misc, u32 addr, u32 *val)
+{
+	int reg;
+	int i;
+	int ret;
+
+	for (i = 0; i < 100; i++) {
+		ret = kirin970_phy_cr_clk(usb31misc);
+		if (ret)
+			return ret;
+	}
+
+	ret = kirin970_phy_cr_set_sel(usb31misc);
+	if (ret)
+		return ret;
+
+	ret = kirin970_phy_cr_set_addr(usb31misc, addr);
+	if (ret)
+		return ret;
+
+	ret = kirin970_phy_cr_start(usb31misc, 0);
+	if (ret)
+		return ret;
+
+	ret = kirin970_phy_cr_wait_ack(usb31misc);
+	if (ret)
+		return ret;
+
+	ret = regmap_read(usb31misc, USB_MISC_CFG58, &reg);
+	if (ret)
+		return ret;
+
+	*val = (reg >> CFG58_USB31PHY_CR_DATA_RD_START) &
+		CFG58_USB31PHY_CR_DATA_MASK;
+
+	return 0;
+}
+
+static int kirin970_phy_cr_write(struct regmap *usb31misc, u32 addr, u32 val)
+{
+	int i;
+	int ret;
+
+	for (i = 0; i < 100; i++) {
+		ret = kirin970_phy_cr_clk(usb31misc);
+		if (ret)
+			return ret;
+	}
+
+	ret = kirin970_phy_cr_set_sel(usb31misc);
+	if (ret)
+		return ret;
+
+	ret = kirin970_phy_cr_set_addr(usb31misc, addr);
+	if (ret)
+		return ret;
+
+	ret = regmap_write(usb31misc, USB_MISC_CFG58,
+			val & CFG58_USB31PHY_CR_DATA_MASK);
+	if (ret)
+		return ret;
+
+	ret = kirin970_phy_cr_start(usb31misc, 1);
+	if (ret)
+		return ret;
+
+	ret = kirin970_phy_cr_wait_ack(usb31misc);
+
+	return ret;
+}
+
+static int kirin970_phy_set_params(struct kirin970_priv *priv)
+{
+	u32 reg;
+	int ret;
+	int retry = 3;
+
+	ret = regmap_write(priv->usb31misc, USB3OTG_CTRL4,
+			priv->eye_diagram_param);
+	if (ret) {
+		dev_err(priv->dev, "set USB3OTG_CTRL4 failed\n");
+		return ret;
+	}
+
+	while (retry-- > 0) {
+		ret = kirin970_phy_cr_read(priv->usb31misc,
+				TX_VBOOST_LVL_REG, &reg);
+		if (!ret)
+			break;
+		else if (ret != -ETIMEDOUT) {
+			dev_err(priv->dev, "read TX_VBOOST_LVL_REG failed\n");
+			return ret;
+		}
+	}
+	if (ret)
+		return ret;
+
+	reg |= (TX_VBOOST_LVL_ENABLE |
+			(priv->tx_vboost_lvl << TX_VBOOST_LVL_START));
+	ret = kirin970_phy_cr_write(priv->usb31misc, TX_VBOOST_LVL_REG, reg);
+	if (ret)
+		dev_err(priv->dev, "write TX_VBOOST_LVL_REG failed\n");
+
+	return ret;
+}
+
+static int kirin970_is_abbclk_seleted(struct kirin970_priv *priv)
+{
+	u32 reg;
+
+	if (!priv->sctrl) {
+		dev_err(priv->dev, "priv->sctrl is null!\n");
+		return 1;
+	}
+
+	if (regmap_read(priv->sctrl, SCTRL_SCDEEPSLEEPED, &reg)) {
+		dev_err(priv->dev, "SCTRL_SCDEEPSLEEPED read failed!\n");
+		return 1;
+	}
+
+	if ((reg & USB_CLK_SELECTED) == 0)
+		return 1;
+
+	return 0;
+}
+
+static int kirin970_config_phy_clock(struct kirin970_priv *priv)
+{
+	u32 val, mask;
+	int ret;
+
+	if (kirin970_is_abbclk_seleted(priv)) {
+		/* usb refclk iso disable */
+		ret = regmap_write(priv->peri_crg, PERI_CRG_ISODIS,
+				USB_REFCLK_ISO_EN);
+		if (ret)
+			goto out;
+
+		/* select usbphy clk from abb */
+		mask = SC_CLK_USB3PHY_3MUX1_SEL;
+		ret = regmap_update_bits(priv->pctrl,
+				PCTRL_PERI_CTRL24, mask, 0);
+		if (ret)
+			goto out;
+
+		ret = regmap_update_bits(priv->usb31misc, USB_MISC_CFGA0,
+				CFGA0_USB2PHY_REFCLK_SELECT, 0);
+		if (ret)
+			goto out;
+
+		ret = regmap_read(priv->usb31misc, USB3OTG_CTRL7, &val);
+		if (ret)
+			goto out;
+		val &= ~CTRL7_USB2_REFCLKSEL_MASK;
+		val |= CTRL7_USB2_REFCLKSEL_ABB;
+		ret = regmap_write(priv->usb31misc, USB3OTG_CTRL7, val);
+		if (ret)
+			goto out;
+	} else {
+		ret = regmap_update_bits(priv->usb31misc, USB_MISC_CFG54,
+				CFG54_USB3PHY_REF_USE_PAD,
+				CFG54_USB3PHY_REF_USE_PAD);
+		if (ret)
+			goto out;
+
+		ret = regmap_update_bits(priv->usb31misc, USB_MISC_CFGA0,
+				CFGA0_USB2PHY_REFCLK_SELECT,
+				CFGA0_USB2PHY_REFCLK_SELECT);
+		if (ret)
+			goto out;
+
+		ret = regmap_read(priv->usb31misc, USB3OTG_CTRL7, &val);
+		if (ret)
+			goto out;
+		val &= ~CTRL7_USB2_REFCLKSEL_MASK;
+		val |= CTRL7_USB2_REFCLKSEL_PAD;
+		ret = regmap_write(priv->usb31misc, USB3OTG_CTRL7, val);
+		if (ret)
+			goto out;
+
+		ret = regmap_write(priv->peri_crg,
+				PERI_CRG_PEREN6, GT_CLK_USB2PHY_REF);
+		if (ret)
+			goto out;
+	}
+
+	return 0;
+out:
+	dev_err(priv->dev, "failed to config phy clock ret: %d\n", ret);
+	return ret;
+}
+
+static int kirin970_config_tca(struct kirin970_priv *priv)
+{
+	u32 val, mask;
+	int ret;
+
+	ret = regmap_write(priv->usb31misc, TCA_INTR_STS, 0xffff);
+	if (ret)
+		goto out;
+
+	ret = regmap_write(priv->usb31misc, TCA_INTR_EN,
+			INTR_EN_XA_TIMEOUT_EVT_EN | INTR_EN_XA_ACK_EVT_EN);
+	if (ret)
+		goto out;
+
+	mask = CLK_RST_TCA_REF_CLK_EN | CLK_RST_SUSPEND_CLK_EN;
+	ret = regmap_update_bits(priv->usb31misc, TCA_CLK_RST, mask, 0);
+	if (ret)
+		goto out;
+
+	ret = regmap_update_bits(priv->usb31misc, TCA_GCFG,
+			GCFG_ROLE_HSTDEV, GCFG_ROLE_HSTDEV);
+	if (ret)
+		goto out;
+
+	ret = regmap_read(priv->usb31misc, TCA_TCPC, &val);
+	if (ret)
+		goto out;
+	val &= ~(TCPC_VALID | TCPC_LOW_POWER_EN | TCPC_MUX_CONTROL_MASK);
+	val |= (TCPC_VALID | TCPC_MUX_CONTROL_USB31);
+	ret = regmap_write(priv->usb31misc, TCA_TCPC, val);
+	if (ret)
+		goto out;
+
+	ret = regmap_write(priv->usb31misc, TCA_VBUS_CTRL,
+		VBUS_CTRL_POWERPRESENT_OVERRD | VBUS_CTRL_VBUSVALID_OVERRD);
+	if (ret)
+		goto out;
+
+	return 0;
+out:
+	dev_err(priv->dev, "failed to config phy clock ret: %d\n", ret);
+	return ret;
+}
+
+static int kirin970_phy_exit(struct phy *phy);
+
+static int kirin970_phy_init(struct phy *phy)
+{
+	struct kirin970_priv *priv = phy_get_drvdata(phy);
+	u32 val;
+	int ret;
+
+	kirin970_phy_exit(phy);
+	dev_info(priv->dev, "%s in\n", __func__);
+	/* assert controller */
+	val = CFGA0_VAUX_RESET | CFGA0_USB31C_RESET;
+	ret = regmap_update_bits(priv->usb31misc, USB_MISC_CFGA0, val, 0);
+	if (ret)
+		goto out;
+
+	ret = kirin970_config_phy_clock(priv);
+	if (ret)
+		goto out;
+
+	/* Exit from IDDQ mode */
+	ret = regmap_update_bits(priv->usb31misc, USB3OTG_CTRL5,
+			CTRL5_USB2_SIDDQ, 0);
+	if (ret)
+		goto out;
+
+	/* Release USB31 PHY out of TestPowerDown mode */
+	ret = regmap_update_bits(priv->usb31misc, USB_MISC_CFG50,
+			CFG50_USB3_PHY_TEST_POWERDOWN, 0);
+	if (ret)
+		goto out;
+
+	/* Tell the PHY power is stable */
+	val = CFG54_USB3_PHY0_ANA_PWR_EN | CFG54_PHY0_PCS_PWR_STABLE |
+		CFG54_PHY0_PMA_PWR_STABLE;
+	ret = regmap_update_bits(priv->usb31misc, USB_MISC_CFG54,
+			val, val);
+	if (ret)
+		goto out;
+
+	ret = kirin970_config_tca(priv);
+	if (ret)
+		goto out;
+
+	/* Enable SSC */
+	ret = regmap_update_bits(priv->usb31misc, USB_MISC_CFG5C,
+			CFG5C_USB3_PHY0_SS_MPLLA_SSC_EN,
+			CFG5C_USB3_PHY0_SS_MPLLA_SSC_EN);
+	if (ret)
+		goto out;
+
+	/* Deassert phy */
+	val = CFGA0_USB3PHY_RESET | CFGA0_USB2PHY_POR;
+	ret = regmap_update_bits(priv->usb31misc, USB_MISC_CFGA0, val, val);
+	if (ret)
+		goto out;
+
+	udelay(100);
+
+	/* Deassert controller */
+	val = CFGA0_VAUX_RESET | CFGA0_USB31C_RESET;
+	ret = regmap_update_bits(priv->usb31misc, USB_MISC_CFGA0, val, val);
+	if (ret)
+		goto out;
+
+	udelay(100);
+
+	/* Set fake vbus valid signal */
+	val = CTRL0_USB3_VBUSVLD | CTRL0_USB3_VBUSVLD_SEL;
+	ret = regmap_update_bits(priv->usb31misc, USB3OTG_CTRL0, val, val);
+	if (ret)
+		goto out;
+
+	val = CTRL3_USB2_VBUSVLDEXT0 | CTRL3_USB2_VBUSVLDEXTSEL0;
+	ret = regmap_update_bits(priv->usb31misc, USB3OTG_CTRL3, val, val);
+	if (ret)
+		goto out;
+
+	udelay(100);
+
+	ret = kirin970_phy_set_params(priv);
+	if (ret)
+		goto out;
+
+	{
+		ret = regmap_read(priv->peri_crg, 0x4c,
+				&val);
+		if (!ret)
+			dev_info(priv->dev, "peri_crg 0x4c %x\n", val);
+		ret = regmap_read(priv->peri_crg, 0x404,
+				&val);
+		if (!ret)
+			dev_info(priv->dev, "peri_crg 0x404 %x\n", val);
+		ret = regmap_read(priv->peri_crg, 0xc,
+				&val);
+		if (!ret)
+			dev_info(priv->dev, "peri_crg 0xc %x\n", val);
+		ret = regmap_read(priv->peri_crg, 0xac,
+				&val);
+		if (!ret)
+			dev_info(priv->dev, "peri_crg 0xac %x\n", val);
+		ret = regmap_read(priv->pctrl, 0x10,
+				&val);
+		if (!ret)
+			dev_info(priv->dev, "pctrl 0x10 %x\n", val);
+	}
+
+	return 0;
+out:
+	dev_err(priv->dev, "failed to init phy ret: %d\n", ret);
+	return ret;
+}
+
+static int kirin970_phy_exit(struct phy *phy)
+{
+	struct kirin970_priv *priv = phy_get_drvdata(phy);
+	u32 mask;
+	int ret;
+
+	/* Assert phy */
+	mask = CFGA0_USB3PHY_RESET | CFGA0_USB2PHY_POR;
+	ret = regmap_update_bits(priv->usb31misc, USB_MISC_CFGA0, mask, 0);
+	if (ret)
+		goto out;
+
+	if (!kirin970_is_abbclk_seleted(priv)) {
+		ret = regmap_write(priv->peri_crg, PERI_CRG_PERDIS6,
+				GT_CLK_USB2PHY_REF);
+		if (ret)
+			goto out;
+	}
+
+	return 0;
+out:
+	dev_err(priv->dev, "failed to exit phy ret: %d\n", ret);
+	return ret;
+}
+
+static struct phy_ops kirin970_phy_ops = {
+	.init		= kirin970_phy_init,
+	.exit		= kirin970_phy_exit,
+	.owner		= THIS_MODULE,
+};
+
+static int kirin970_phy_probe(struct platform_device *pdev)
+{
+	struct phy_provider *phy_provider;
+	struct device *dev = &pdev->dev;
+	struct phy *phy;
+	struct kirin970_priv *priv;
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv->dev = dev;
+	priv->peri_crg = syscon_regmap_lookup_by_phandle(dev->of_node,
+					"hisilicon,pericrg-syscon");
+	if (IS_ERR(priv->peri_crg)) {
+		dev_err(dev, "no hisilicon,pericrg-syscon\n");
+		return PTR_ERR(priv->peri_crg);
+	}
+
+	priv->pctrl = syscon_regmap_lookup_by_phandle(dev->of_node,
+					"hisilicon,pctrl-syscon");
+	if (IS_ERR(priv->pctrl)) {
+		dev_err(dev, "no hisilicon,pctrl-syscon\n");
+		return PTR_ERR(priv->pctrl);
+	}
+
+	priv->sctrl = syscon_regmap_lookup_by_phandle(dev->of_node,
+					"hisilicon,sctrl-syscon");
+	if (IS_ERR(priv->sctrl)) {
+		dev_err(dev, "no hisilicon,sctrl-syscon\n");
+		return PTR_ERR(priv->sctrl);
+	}
+
+	priv->usb31misc = syscon_regmap_lookup_by_phandle(dev->of_node,
+					"hisilicon,usb31-misc-syscon");
+	if (IS_ERR(priv->usb31misc)) {
+		dev_err(dev, "no hisilicon,usb31-misc-syscon\n");
+		return PTR_ERR(priv->usb31misc);
+	}
+
+	if (of_property_read_u32(dev->of_node, "eye-diagram-param",
+				&(priv->eye_diagram_param)))
+		priv->eye_diagram_param = KIRIN970_USB_DEFAULT_PHY_PARAM;
+
+	if (of_property_read_u32(dev->of_node, "usb3-phy-tx-vboost-lvl",
+				&(priv->tx_vboost_lvl)))
+		priv->eye_diagram_param = KIRIN970_USB_DEFAULT_PHY_VBOOST;
+
+	phy = devm_phy_create(dev, NULL, &kirin970_phy_ops);
+	if (IS_ERR(phy))
+		return PTR_ERR(phy);
+
+	phy_set_drvdata(phy, priv);
+	phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
+	return PTR_ERR_OR_ZERO(phy_provider);
+}
+
+static const struct of_device_id kirin970_phy_of_match[] = {
+	{.compatible = "hisilicon,kirin970-usb-phy",},
+	{ },
+};
+MODULE_DEVICE_TABLE(of, kirin970_phy_of_match);
+
+static struct platform_driver kirin970_phy_driver = {
+	.probe	= kirin970_phy_probe,
+	.driver = {
+		.name	= "kirin970-usb-phy",
+		.of_match_table	= kirin970_phy_of_match,
+	}
+};
+module_platform_driver(kirin970_phy_driver);
+
+MODULE_AUTHOR("Yu Chen <chenyu56@huawei.com>");
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("Hilisicon Kirin970 USB31 PHY Driver");
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [RFC 02/10] phy: hisilicon: phy-hi3670-usb3: fix some issues at the init code
  2020-09-04 10:23 [RFC 00/10] Add USB support for Hikey 970 Mauro Carvalho Chehab
  2020-09-04 10:23 ` [RFC 01/10] phy: hisilicon: add USB physical layer for Kirin 3670 Mauro Carvalho Chehab
@ 2020-09-04 10:23 ` Mauro Carvalho Chehab
  2020-09-04 10:23 ` [RFC 03/10] phy: hisilicon: phy-hi3670-usb3: use a consistent namespace Mauro Carvalho Chehab
                   ` (8 subsequent siblings)
  10 siblings, 0 replies; 22+ messages in thread
From: Mauro Carvalho Chehab @ 2020-09-04 10:23 UTC (permalink / raw)
  Cc: linuxarm, mauro.chehab, Yu Chen, Mauro Carvalho Chehab,
	John Stultz, Manivannan Sadhasivam, Robin Murphy,
	Kishon Vijay Abraham I, Vinod Koul, linux-kernel

From: Yu Chen <chenyu56@huawei.com>

There are some problems at the initialization part of this phy.
Solve them.

Signed-off-by: Yu Chen <chenyu56@huawei.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
 drivers/phy/hisilicon/phy-hi3670-usb3.c | 70 +++++++++++--------------
 1 file changed, 32 insertions(+), 38 deletions(-)

diff --git a/drivers/phy/hisilicon/phy-hi3670-usb3.c b/drivers/phy/hisilicon/phy-hi3670-usb3.c
index 4e04ac97728d..1d4caf7a2aaf 100644
--- a/drivers/phy/hisilicon/phy-hi3670-usb3.c
+++ b/drivers/phy/hisilicon/phy-hi3670-usb3.c
@@ -63,6 +63,7 @@
 #define TCA_INTR_STS			(0x208)
 #define TCA_GCFG			(0x210)
 #define TCA_TCPC			(0x214)
+#define TCA_SYSMODE_CFG			(0x218)
 #define TCA_VBUS_CTRL			(0x240)
 
 #define CTRL0_USB3_VBUSVLD		BIT(7)
@@ -109,12 +110,16 @@
 #define CLK_RST_SUSPEND_CLK_EN		BIT(0)
 
 #define GCFG_ROLE_HSTDEV		BIT(4)
+#define GCFG_OP_MODE			(3 << 0)
+#define GCFG_OP_MODE_CTRL_SYNC_MODE	BIT(0)
 
 #define TCPC_VALID			BIT(4)
 #define TCPC_LOW_POWER_EN		BIT(3)
 #define TCPC_MUX_CONTROL_MASK		(3 << 0)
 #define TCPC_MUX_CONTROL_USB31		(1 << 0)
 
+#define SYSMODE_CFG_TYPEC_DISABLE	BIT(3)
+
 #define VBUS_CTRL_POWERPRESENT_OVERRD	(3 << 2)
 #define VBUS_CTRL_VBUSVALID_OVERRD	(3 << 0)
 
@@ -363,6 +368,11 @@ static int kirin970_config_phy_clock(struct kirin970_priv *priv)
 		if (ret)
 			goto out;
 
+		/* enable usb_tcxo_en */
+		ret = regmap_write(priv->pctrl, PCTRL_PERI_CTRL3,
+				USB_TCXO_EN |
+				(USB_TCXO_EN << PCTRL_PERI_CTRL3_MSK_START));
+
 		/* select usbphy clk from abb */
 		mask = SC_CLK_USB3PHY_3MUX1_SEL;
 		ret = regmap_update_bits(priv->pctrl,
@@ -437,7 +447,13 @@ static int kirin970_config_tca(struct kirin970_priv *priv)
 		goto out;
 
 	ret = regmap_update_bits(priv->usb31misc, TCA_GCFG,
-			GCFG_ROLE_HSTDEV, GCFG_ROLE_HSTDEV);
+			GCFG_ROLE_HSTDEV | GCFG_OP_MODE,
+			GCFG_ROLE_HSTDEV | GCFG_OP_MODE_CTRL_SYNC_MODE);
+	if (ret)
+		goto out;
+
+	ret = regmap_update_bits(priv->usb31misc, TCA_SYSMODE_CFG,
+			SYSMODE_CFG_TYPEC_DISABLE, 0);
 	if (ret)
 		goto out;
 
@@ -461,18 +477,15 @@ static int kirin970_config_tca(struct kirin970_priv *priv)
 	return ret;
 }
 
-static int kirin970_phy_exit(struct phy *phy);
-
 static int kirin970_phy_init(struct phy *phy)
 {
 	struct kirin970_priv *priv = phy_get_drvdata(phy);
 	u32 val;
 	int ret;
 
-	kirin970_phy_exit(phy);
-	dev_info(priv->dev, "%s in\n", __func__);
 	/* assert controller */
-	val = CFGA0_VAUX_RESET | CFGA0_USB31C_RESET;
+	val = CFGA0_VAUX_RESET | CFGA0_USB31C_RESET |
+		CFGA0_USB3PHY_RESET | CFGA0_USB2PHY_POR;
 	ret = regmap_update_bits(priv->usb31misc, USB_MISC_CFGA0, val, 0);
 	if (ret)
 		goto out;
@@ -493,6 +506,14 @@ static int kirin970_phy_init(struct phy *phy)
 	if (ret)
 		goto out;
 
+	/* Deassert phy */
+	val = CFGA0_USB3PHY_RESET | CFGA0_USB2PHY_POR;
+	ret = regmap_update_bits(priv->usb31misc, USB_MISC_CFGA0, val, val);
+	if (ret)
+		goto out;
+
+	udelay(100);
+
 	/* Tell the PHY power is stable */
 	val = CFG54_USB3_PHY0_ANA_PWR_EN | CFG54_PHY0_PCS_PWR_STABLE |
 		CFG54_PHY0_PMA_PWR_STABLE;
@@ -512,14 +533,6 @@ static int kirin970_phy_init(struct phy *phy)
 	if (ret)
 		goto out;
 
-	/* Deassert phy */
-	val = CFGA0_USB3PHY_RESET | CFGA0_USB2PHY_POR;
-	ret = regmap_update_bits(priv->usb31misc, USB_MISC_CFGA0, val, val);
-	if (ret)
-		goto out;
-
-	udelay(100);
-
 	/* Deassert controller */
 	val = CFGA0_VAUX_RESET | CFGA0_USB31C_RESET;
 	ret = regmap_update_bits(priv->usb31misc, USB_MISC_CFGA0, val, val);
@@ -545,29 +558,6 @@ static int kirin970_phy_init(struct phy *phy)
 	if (ret)
 		goto out;
 
-	{
-		ret = regmap_read(priv->peri_crg, 0x4c,
-				&val);
-		if (!ret)
-			dev_info(priv->dev, "peri_crg 0x4c %x\n", val);
-		ret = regmap_read(priv->peri_crg, 0x404,
-				&val);
-		if (!ret)
-			dev_info(priv->dev, "peri_crg 0x404 %x\n", val);
-		ret = regmap_read(priv->peri_crg, 0xc,
-				&val);
-		if (!ret)
-			dev_info(priv->dev, "peri_crg 0xc %x\n", val);
-		ret = regmap_read(priv->peri_crg, 0xac,
-				&val);
-		if (!ret)
-			dev_info(priv->dev, "peri_crg 0xac %x\n", val);
-		ret = regmap_read(priv->pctrl, 0x10,
-				&val);
-		if (!ret)
-			dev_info(priv->dev, "pctrl 0x10 %x\n", val);
-	}
-
 	return 0;
 out:
 	dev_err(priv->dev, "failed to init phy ret: %d\n", ret);
@@ -586,7 +576,11 @@ static int kirin970_phy_exit(struct phy *phy)
 	if (ret)
 		goto out;
 
-	if (!kirin970_is_abbclk_seleted(priv)) {
+	if (kirin970_is_abbclk_seleted(priv)) {
+		/* disable usb_tcxo_en */
+		ret = regmap_write(priv->pctrl, PCTRL_PERI_CTRL3,
+				USB_TCXO_EN << PCTRL_PERI_CTRL3_MSK_START);
+	} else {
 		ret = regmap_write(priv->peri_crg, PERI_CRG_PERDIS6,
 				GT_CLK_USB2PHY_REF);
 		if (ret)
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [RFC 03/10] phy: hisilicon: phy-hi3670-usb3: use a consistent namespace
  2020-09-04 10:23 [RFC 00/10] Add USB support for Hikey 970 Mauro Carvalho Chehab
  2020-09-04 10:23 ` [RFC 01/10] phy: hisilicon: add USB physical layer for Kirin 3670 Mauro Carvalho Chehab
  2020-09-04 10:23 ` [RFC 02/10] phy: hisilicon: phy-hi3670-usb3: fix some issues at the init code Mauro Carvalho Chehab
@ 2020-09-04 10:23 ` Mauro Carvalho Chehab
  2020-09-09 20:15   ` Rob Herring
  2020-09-04 10:23 ` [RFC 04/10] phy: hisilicon: phy-hi3670-usb3: fix coding style Mauro Carvalho Chehab
                   ` (7 subsequent siblings)
  10 siblings, 1 reply; 22+ messages in thread
From: Mauro Carvalho Chehab @ 2020-09-04 10:23 UTC (permalink / raw)
  Cc: linuxarm, mauro.chehab, Mauro Carvalho Chehab, John Stultz,
	Manivannan Sadhasivam, Robin Murphy, Kishon Vijay Abraham I,
	Vinod Koul, Rob Herring, Yu Chen, linux-kernel, devicetree

Rename hikey970 to hi3670, in order to use a namespace
similar to hi3660 driver.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
 .../bindings/phy/phy-hi3670-usb3.txt          |  4 +-
 drivers/phy/hisilicon/phy-hi3670-usb3.c       | 98 +++++++++----------
 2 files changed, 51 insertions(+), 51 deletions(-)

diff --git a/Documentation/devicetree/bindings/phy/phy-hi3670-usb3.txt b/Documentation/devicetree/bindings/phy/phy-hi3670-usb3.txt
index 4cb02612ff23..2fb27cb8beaf 100644
--- a/Documentation/devicetree/bindings/phy/phy-hi3670-usb3.txt
+++ b/Documentation/devicetree/bindings/phy/phy-hi3670-usb3.txt
@@ -2,7 +2,7 @@ Hisilicon Kirin970 usb PHY
 -----------------------
 
 Required properties:
-- compatible: should be "hisilicon,kirin970-usb-phy"
+- compatible: should be "hisilicon,hi3670-usb-phy"
 - #phy-cells: must be 0
 - hisilicon,pericrg-syscon: phandle of syscon used to control phy.
 - hisilicon,pctrl-syscon: phandle of syscon used to control phy.
@@ -14,7 +14,7 @@ Refer to phy/phy-bindings.txt for the generic PHY binding properties
 
 Example:
 	usb_phy: usbphy {
-		compatible = "hisilicon,kirin970-usb-phy";
+		compatible = "hisilicon,hi3670-usb-phy";
 		#phy-cells = <0>;
 		hisilicon,pericrg-syscon = <&crg_ctrl>;
 		hisilicon,pctrl-syscon = <&pctrl>;
diff --git a/drivers/phy/hisilicon/phy-hi3670-usb3.c b/drivers/phy/hisilicon/phy-hi3670-usb3.c
index 1d4caf7a2aaf..fd679b39185a 100644
--- a/drivers/phy/hisilicon/phy-hi3670-usb3.c
+++ b/drivers/phy/hisilicon/phy-hi3670-usb3.c
@@ -130,7 +130,7 @@
 #define TX_VBOOST_LVL_START		(6)
 #define TX_VBOOST_LVL_ENABLE		BIT(9)
 
-struct kirin970_priv {
+struct hi3670_priv {
 	struct device *dev;
 	struct regmap *peri_crg;
 	struct regmap *pctrl;
@@ -145,7 +145,7 @@ struct kirin970_priv {
 	u32 usb31misc_offset;
 };
 
-static int kirin970_phy_cr_clk(struct regmap *usb31misc)
+static int hi3670_phy_cr_clk(struct regmap *usb31misc)
 {
 	int ret;
 
@@ -162,13 +162,13 @@ static int kirin970_phy_cr_clk(struct regmap *usb31misc)
 	return ret;
 }
 
-static int kirin970_phy_cr_set_sel(struct regmap *usb31misc)
+static int hi3670_phy_cr_set_sel(struct regmap *usb31misc)
 {
 	return regmap_update_bits(usb31misc, USB_MISC_CFG54,
 			CFG54_USB31PHY_CR_SEL, CFG54_USB31PHY_CR_SEL);
 }
 
-static int kirin970_phy_cr_start(struct regmap *usb31misc, int direction)
+static int hi3670_phy_cr_start(struct regmap *usb31misc, int direction)
 {
 	int ret;
 
@@ -182,7 +182,7 @@ static int kirin970_phy_cr_start(struct regmap *usb31misc, int direction)
 	if (ret)
 		return ret;
 
-	ret = kirin970_phy_cr_clk(usb31misc);
+	ret = hi3670_phy_cr_clk(usb31misc);
 	if (ret)
 		return ret;
 
@@ -192,7 +192,7 @@ static int kirin970_phy_cr_start(struct regmap *usb31misc, int direction)
 	return ret;
 }
 
-static int kirin970_phy_cr_wait_ack(struct regmap *usb31misc)
+static int hi3670_phy_cr_wait_ack(struct regmap *usb31misc)
 {
 	u32 reg;
 	int retry = 100000;
@@ -205,7 +205,7 @@ static int kirin970_phy_cr_wait_ack(struct regmap *usb31misc)
 		if ((reg & CFG54_USB31PHY_CR_ACK) == CFG54_USB31PHY_CR_ACK)
 			return 0;
 
-		ret = kirin970_phy_cr_clk(usb31misc);
+		ret = hi3670_phy_cr_clk(usb31misc);
 		if (ret)
 			return ret;
 	}
@@ -213,7 +213,7 @@ static int kirin970_phy_cr_wait_ack(struct regmap *usb31misc)
 	return -ETIMEDOUT;
 }
 
-static int kirin970_phy_cr_set_addr(struct regmap *usb31misc, u32 addr)
+static int hi3670_phy_cr_set_addr(struct regmap *usb31misc, u32 addr)
 {
 	u32 reg;
 	int ret;
@@ -230,31 +230,31 @@ static int kirin970_phy_cr_set_addr(struct regmap *usb31misc, u32 addr)
 	return ret;
 }
 
-static int kirin970_phy_cr_read(struct regmap *usb31misc, u32 addr, u32 *val)
+static int hi3670_phy_cr_read(struct regmap *usb31misc, u32 addr, u32 *val)
 {
 	int reg;
 	int i;
 	int ret;
 
 	for (i = 0; i < 100; i++) {
-		ret = kirin970_phy_cr_clk(usb31misc);
+		ret = hi3670_phy_cr_clk(usb31misc);
 		if (ret)
 			return ret;
 	}
 
-	ret = kirin970_phy_cr_set_sel(usb31misc);
+	ret = hi3670_phy_cr_set_sel(usb31misc);
 	if (ret)
 		return ret;
 
-	ret = kirin970_phy_cr_set_addr(usb31misc, addr);
+	ret = hi3670_phy_cr_set_addr(usb31misc, addr);
 	if (ret)
 		return ret;
 
-	ret = kirin970_phy_cr_start(usb31misc, 0);
+	ret = hi3670_phy_cr_start(usb31misc, 0);
 	if (ret)
 		return ret;
 
-	ret = kirin970_phy_cr_wait_ack(usb31misc);
+	ret = hi3670_phy_cr_wait_ack(usb31misc);
 	if (ret)
 		return ret;
 
@@ -268,22 +268,22 @@ static int kirin970_phy_cr_read(struct regmap *usb31misc, u32 addr, u32 *val)
 	return 0;
 }
 
-static int kirin970_phy_cr_write(struct regmap *usb31misc, u32 addr, u32 val)
+static int hi3670_phy_cr_write(struct regmap *usb31misc, u32 addr, u32 val)
 {
 	int i;
 	int ret;
 
 	for (i = 0; i < 100; i++) {
-		ret = kirin970_phy_cr_clk(usb31misc);
+		ret = hi3670_phy_cr_clk(usb31misc);
 		if (ret)
 			return ret;
 	}
 
-	ret = kirin970_phy_cr_set_sel(usb31misc);
+	ret = hi3670_phy_cr_set_sel(usb31misc);
 	if (ret)
 		return ret;
 
-	ret = kirin970_phy_cr_set_addr(usb31misc, addr);
+	ret = hi3670_phy_cr_set_addr(usb31misc, addr);
 	if (ret)
 		return ret;
 
@@ -292,16 +292,16 @@ static int kirin970_phy_cr_write(struct regmap *usb31misc, u32 addr, u32 val)
 	if (ret)
 		return ret;
 
-	ret = kirin970_phy_cr_start(usb31misc, 1);
+	ret = hi3670_phy_cr_start(usb31misc, 1);
 	if (ret)
 		return ret;
 
-	ret = kirin970_phy_cr_wait_ack(usb31misc);
+	ret = hi3670_phy_cr_wait_ack(usb31misc);
 
 	return ret;
 }
 
-static int kirin970_phy_set_params(struct kirin970_priv *priv)
+static int hi3670_phy_set_params(struct hi3670_priv *priv)
 {
 	u32 reg;
 	int ret;
@@ -315,7 +315,7 @@ static int kirin970_phy_set_params(struct kirin970_priv *priv)
 	}
 
 	while (retry-- > 0) {
-		ret = kirin970_phy_cr_read(priv->usb31misc,
+		ret = hi3670_phy_cr_read(priv->usb31misc,
 				TX_VBOOST_LVL_REG, &reg);
 		if (!ret)
 			break;
@@ -329,14 +329,14 @@ static int kirin970_phy_set_params(struct kirin970_priv *priv)
 
 	reg |= (TX_VBOOST_LVL_ENABLE |
 			(priv->tx_vboost_lvl << TX_VBOOST_LVL_START));
-	ret = kirin970_phy_cr_write(priv->usb31misc, TX_VBOOST_LVL_REG, reg);
+	ret = hi3670_phy_cr_write(priv->usb31misc, TX_VBOOST_LVL_REG, reg);
 	if (ret)
 		dev_err(priv->dev, "write TX_VBOOST_LVL_REG failed\n");
 
 	return ret;
 }
 
-static int kirin970_is_abbclk_seleted(struct kirin970_priv *priv)
+static int hi3670_is_abbclk_seleted(struct hi3670_priv *priv)
 {
 	u32 reg;
 
@@ -356,12 +356,12 @@ static int kirin970_is_abbclk_seleted(struct kirin970_priv *priv)
 	return 0;
 }
 
-static int kirin970_config_phy_clock(struct kirin970_priv *priv)
+static int hi3670_config_phy_clock(struct hi3670_priv *priv)
 {
 	u32 val, mask;
 	int ret;
 
-	if (kirin970_is_abbclk_seleted(priv)) {
+	if (hi3670_is_abbclk_seleted(priv)) {
 		/* usb refclk iso disable */
 		ret = regmap_write(priv->peri_crg, PERI_CRG_ISODIS,
 				USB_REFCLK_ISO_EN);
@@ -427,7 +427,7 @@ static int kirin970_config_phy_clock(struct kirin970_priv *priv)
 	return ret;
 }
 
-static int kirin970_config_tca(struct kirin970_priv *priv)
+static int hi3670_config_tca(struct hi3670_priv *priv)
 {
 	u32 val, mask;
 	int ret;
@@ -477,9 +477,9 @@ static int kirin970_config_tca(struct kirin970_priv *priv)
 	return ret;
 }
 
-static int kirin970_phy_init(struct phy *phy)
+static int hi3670_phy_init(struct phy *phy)
 {
-	struct kirin970_priv *priv = phy_get_drvdata(phy);
+	struct hi3670_priv *priv = phy_get_drvdata(phy);
 	u32 val;
 	int ret;
 
@@ -490,7 +490,7 @@ static int kirin970_phy_init(struct phy *phy)
 	if (ret)
 		goto out;
 
-	ret = kirin970_config_phy_clock(priv);
+	ret = hi3670_config_phy_clock(priv);
 	if (ret)
 		goto out;
 
@@ -522,7 +522,7 @@ static int kirin970_phy_init(struct phy *phy)
 	if (ret)
 		goto out;
 
-	ret = kirin970_config_tca(priv);
+	ret = hi3670_config_tca(priv);
 	if (ret)
 		goto out;
 
@@ -554,7 +554,7 @@ static int kirin970_phy_init(struct phy *phy)
 
 	udelay(100);
 
-	ret = kirin970_phy_set_params(priv);
+	ret = hi3670_phy_set_params(priv);
 	if (ret)
 		goto out;
 
@@ -564,9 +564,9 @@ static int kirin970_phy_init(struct phy *phy)
 	return ret;
 }
 
-static int kirin970_phy_exit(struct phy *phy)
+static int hi3670_phy_exit(struct phy *phy)
 {
-	struct kirin970_priv *priv = phy_get_drvdata(phy);
+	struct hi3670_priv *priv = phy_get_drvdata(phy);
 	u32 mask;
 	int ret;
 
@@ -576,7 +576,7 @@ static int kirin970_phy_exit(struct phy *phy)
 	if (ret)
 		goto out;
 
-	if (kirin970_is_abbclk_seleted(priv)) {
+	if (hi3670_is_abbclk_seleted(priv)) {
 		/* disable usb_tcxo_en */
 		ret = regmap_write(priv->pctrl, PCTRL_PERI_CTRL3,
 				USB_TCXO_EN << PCTRL_PERI_CTRL3_MSK_START);
@@ -593,18 +593,18 @@ static int kirin970_phy_exit(struct phy *phy)
 	return ret;
 }
 
-static struct phy_ops kirin970_phy_ops = {
-	.init		= kirin970_phy_init,
-	.exit		= kirin970_phy_exit,
+static struct phy_ops hi3670_phy_ops = {
+	.init		= hi3670_phy_init,
+	.exit		= hi3670_phy_exit,
 	.owner		= THIS_MODULE,
 };
 
-static int kirin970_phy_probe(struct platform_device *pdev)
+static int hi3670_phy_probe(struct platform_device *pdev)
 {
 	struct phy_provider *phy_provider;
 	struct device *dev = &pdev->dev;
 	struct phy *phy;
-	struct kirin970_priv *priv;
+	struct hi3670_priv *priv;
 
 	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
 	if (!priv)
@@ -647,7 +647,7 @@ static int kirin970_phy_probe(struct platform_device *pdev)
 				&(priv->tx_vboost_lvl)))
 		priv->eye_diagram_param = KIRIN970_USB_DEFAULT_PHY_VBOOST;
 
-	phy = devm_phy_create(dev, NULL, &kirin970_phy_ops);
+	phy = devm_phy_create(dev, NULL, &hi3670_phy_ops);
 	if (IS_ERR(phy))
 		return PTR_ERR(phy);
 
@@ -656,20 +656,20 @@ static int kirin970_phy_probe(struct platform_device *pdev)
 	return PTR_ERR_OR_ZERO(phy_provider);
 }
 
-static const struct of_device_id kirin970_phy_of_match[] = {
-	{.compatible = "hisilicon,kirin970-usb-phy",},
+static const struct of_device_id hi3670_phy_of_match[] = {
+	{.compatible = "hisilicon,hi3670-usb-phy",},
 	{ },
 };
-MODULE_DEVICE_TABLE(of, kirin970_phy_of_match);
+MODULE_DEVICE_TABLE(of, hi3670_phy_of_match);
 
-static struct platform_driver kirin970_phy_driver = {
-	.probe	= kirin970_phy_probe,
+static struct platform_driver hi3670_phy_driver = {
+	.probe	= hi3670_phy_probe,
 	.driver = {
-		.name	= "kirin970-usb-phy",
-		.of_match_table	= kirin970_phy_of_match,
+		.name	= "hi3670-usb-phy",
+		.of_match_table	= hi3670_phy_of_match,
 	}
 };
-module_platform_driver(kirin970_phy_driver);
+module_platform_driver(hi3670_phy_driver);
 
 MODULE_AUTHOR("Yu Chen <chenyu56@huawei.com>");
 MODULE_LICENSE("GPL v2");
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [RFC 04/10] phy: hisilicon: phy-hi3670-usb3: fix coding style
  2020-09-04 10:23 [RFC 00/10] Add USB support for Hikey 970 Mauro Carvalho Chehab
                   ` (2 preceding siblings ...)
  2020-09-04 10:23 ` [RFC 03/10] phy: hisilicon: phy-hi3670-usb3: use a consistent namespace Mauro Carvalho Chehab
@ 2020-09-04 10:23 ` Mauro Carvalho Chehab
  2020-09-04 10:23 ` [RFC 05/10] phy: hisilicon: phy-hi3670-usb3: change some DT properties Mauro Carvalho Chehab
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 22+ messages in thread
From: Mauro Carvalho Chehab @ 2020-09-04 10:23 UTC (permalink / raw)
  Cc: linuxarm, mauro.chehab, Mauro Carvalho Chehab, John Stultz,
	Manivannan Sadhasivam, Robin Murphy, Kishon Vijay Abraham I,
	Vinod Koul, Yu Chen, linux-kernel

Address the issues reported by checkpatch --strict,
and add a SPDX tag.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
 drivers/phy/hisilicon/phy-hi3670-usb3.c | 157 ++++++++++++------------
 1 file changed, 76 insertions(+), 81 deletions(-)

diff --git a/drivers/phy/hisilicon/phy-hi3670-usb3.c b/drivers/phy/hisilicon/phy-hi3670-usb3.c
index fd679b39185a..cb0bfcbbfbfa 100644
--- a/drivers/phy/hisilicon/phy-hi3670-usb3.c
+++ b/drivers/phy/hisilicon/phy-hi3670-usb3.c
@@ -1,28 +1,20 @@
+// SPDX-License-Identifier: GPL-2.0-only
 /*
  * Phy provider for USB 3.1 controller on HiSilicon Kirin970 platform
  *
- * Copyright (C) 2017-2018 Hilisicon Electronics Co., Ltd.
+ * Copyright (C) 2017-2020 Hilisicon Electronics Co., Ltd.
  *		http://www.huawei.com
  *
  * Authors: Yu Chen <chenyu56@huawei.com>
- *
- * This program is free software: you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2  of
- * the License 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/clk.h>
 #include <linux/kernel.h>
 #include <linux/mfd/syscon.h>
 #include <linux/module.h>
-#include <linux/platform_device.h>
 #include <linux/phy/phy.h>
+#include <linux/platform_device.h>
 #include <linux/regmap.h>
-#include <linux/clk.h>
 
 #define SCTRL_SCDEEPSLEEPED		(0x0)
 #define USB_CLK_SELECTED		BIT(20)
@@ -116,7 +108,7 @@
 #define TCPC_VALID			BIT(4)
 #define TCPC_LOW_POWER_EN		BIT(3)
 #define TCPC_MUX_CONTROL_MASK		(3 << 0)
-#define TCPC_MUX_CONTROL_USB31		(1 << 0)
+#define TCPC_MUX_CONTROL_USB31		BIT(0)
 
 #define SYSMODE_CFG_TYPEC_DISABLE	BIT(3)
 
@@ -151,13 +143,13 @@ static int hi3670_phy_cr_clk(struct regmap *usb31misc)
 
 	/* Clock up */
 	ret = regmap_update_bits(usb31misc, USB_MISC_CFG54,
-			CFG54_USB31PHY_CR_CLK, CFG54_USB31PHY_CR_CLK);
+				 CFG54_USB31PHY_CR_CLK, CFG54_USB31PHY_CR_CLK);
 	if (ret)
 		return ret;
 
 	/* Clock down */
 	ret = regmap_update_bits(usb31misc, USB_MISC_CFG54,
-			CFG54_USB31PHY_CR_CLK, 0);
+				 CFG54_USB31PHY_CR_CLK, 0);
 
 	return ret;
 }
@@ -165,7 +157,7 @@ static int hi3670_phy_cr_clk(struct regmap *usb31misc)
 static int hi3670_phy_cr_set_sel(struct regmap *usb31misc)
 {
 	return regmap_update_bits(usb31misc, USB_MISC_CFG54,
-			CFG54_USB31PHY_CR_SEL, CFG54_USB31PHY_CR_SEL);
+				  CFG54_USB31PHY_CR_SEL, CFG54_USB31PHY_CR_SEL);
 }
 
 static int hi3670_phy_cr_start(struct regmap *usb31misc, int direction)
@@ -174,10 +166,12 @@ static int hi3670_phy_cr_start(struct regmap *usb31misc, int direction)
 
 	if (direction)
 		ret = regmap_update_bits(usb31misc, USB_MISC_CFG54,
-			CFG54_USB31PHY_CR_WR_EN, CFG54_USB31PHY_CR_WR_EN);
+					 CFG54_USB31PHY_CR_WR_EN,
+					 CFG54_USB31PHY_CR_WR_EN);
 	else
 		ret = regmap_update_bits(usb31misc, USB_MISC_CFG54,
-			CFG54_USB31PHY_CR_RD_EN, CFG54_USB31PHY_CR_RD_EN);
+					 CFG54_USB31PHY_CR_RD_EN,
+					 CFG54_USB31PHY_CR_RD_EN);
 
 	if (ret)
 		return ret;
@@ -187,7 +181,7 @@ static int hi3670_phy_cr_start(struct regmap *usb31misc, int direction)
 		return ret;
 
 	ret = regmap_update_bits(usb31misc, USB_MISC_CFG54,
-			CFG54_USB31PHY_CR_RD_EN | CFG54_USB31PHY_CR_WR_EN, 0);
+				 CFG54_USB31PHY_CR_RD_EN | CFG54_USB31PHY_CR_WR_EN, 0);
 
 	return ret;
 }
@@ -223,8 +217,7 @@ static int hi3670_phy_cr_set_addr(struct regmap *usb31misc, u32 addr)
 		return ret;
 
 	reg &= ~(CFG54_USB31PHY_CR_ADDR_MASK << CFG54_USB31PHY_CR_ADDR_SHIFT);
-	reg |= ((addr & CFG54_USB31PHY_CR_ADDR_MASK) <<
-			CFG54_USB31PHY_CR_ADDR_SHIFT);
+	reg |= ((addr & CFG54_USB31PHY_CR_ADDR_MASK) << CFG54_USB31PHY_CR_ADDR_SHIFT);
 	ret = regmap_write(usb31misc, USB_MISC_CFG54, reg);
 
 	return ret;
@@ -288,7 +281,7 @@ static int hi3670_phy_cr_write(struct regmap *usb31misc, u32 addr, u32 val)
 		return ret;
 
 	ret = regmap_write(usb31misc, USB_MISC_CFG58,
-			val & CFG58_USB31PHY_CR_DATA_MASK);
+			   val & CFG58_USB31PHY_CR_DATA_MASK);
 	if (ret)
 		return ret;
 
@@ -308,7 +301,7 @@ static int hi3670_phy_set_params(struct hi3670_priv *priv)
 	int retry = 3;
 
 	ret = regmap_write(priv->usb31misc, USB3OTG_CTRL4,
-			priv->eye_diagram_param);
+			   priv->eye_diagram_param);
 	if (ret) {
 		dev_err(priv->dev, "set USB3OTG_CTRL4 failed\n");
 		return ret;
@@ -316,10 +309,11 @@ static int hi3670_phy_set_params(struct hi3670_priv *priv)
 
 	while (retry-- > 0) {
 		ret = hi3670_phy_cr_read(priv->usb31misc,
-				TX_VBOOST_LVL_REG, &reg);
+					 TX_VBOOST_LVL_REG, &reg);
 		if (!ret)
 			break;
-		else if (ret != -ETIMEDOUT) {
+
+		if (ret != -ETIMEDOUT) {
 			dev_err(priv->dev, "read TX_VBOOST_LVL_REG failed\n");
 			return ret;
 		}
@@ -327,8 +321,7 @@ static int hi3670_phy_set_params(struct hi3670_priv *priv)
 	if (ret)
 		return ret;
 
-	reg |= (TX_VBOOST_LVL_ENABLE |
-			(priv->tx_vboost_lvl << TX_VBOOST_LVL_START));
+	reg |= (TX_VBOOST_LVL_ENABLE | (priv->tx_vboost_lvl << TX_VBOOST_LVL_START));
 	ret = hi3670_phy_cr_write(priv->usb31misc, TX_VBOOST_LVL_REG, reg);
 	if (ret)
 		dev_err(priv->dev, "write TX_VBOOST_LVL_REG failed\n");
@@ -364,24 +357,24 @@ static int hi3670_config_phy_clock(struct hi3670_priv *priv)
 	if (hi3670_is_abbclk_seleted(priv)) {
 		/* usb refclk iso disable */
 		ret = regmap_write(priv->peri_crg, PERI_CRG_ISODIS,
-				USB_REFCLK_ISO_EN);
+				   USB_REFCLK_ISO_EN);
 		if (ret)
 			goto out;
 
 		/* enable usb_tcxo_en */
 		ret = regmap_write(priv->pctrl, PCTRL_PERI_CTRL3,
-				USB_TCXO_EN |
-				(USB_TCXO_EN << PCTRL_PERI_CTRL3_MSK_START));
+				   USB_TCXO_EN |
+				   (USB_TCXO_EN << PCTRL_PERI_CTRL3_MSK_START));
 
 		/* select usbphy clk from abb */
 		mask = SC_CLK_USB3PHY_3MUX1_SEL;
 		ret = regmap_update_bits(priv->pctrl,
-				PCTRL_PERI_CTRL24, mask, 0);
+					 PCTRL_PERI_CTRL24, mask, 0);
 		if (ret)
 			goto out;
 
 		ret = regmap_update_bits(priv->usb31misc, USB_MISC_CFGA0,
-				CFGA0_USB2PHY_REFCLK_SELECT, 0);
+					 CFGA0_USB2PHY_REFCLK_SELECT, 0);
 		if (ret)
 			goto out;
 
@@ -393,34 +386,36 @@ static int hi3670_config_phy_clock(struct hi3670_priv *priv)
 		ret = regmap_write(priv->usb31misc, USB3OTG_CTRL7, val);
 		if (ret)
 			goto out;
-	} else {
-		ret = regmap_update_bits(priv->usb31misc, USB_MISC_CFG54,
-				CFG54_USB3PHY_REF_USE_PAD,
-				CFG54_USB3PHY_REF_USE_PAD);
-		if (ret)
-			goto out;
 
-		ret = regmap_update_bits(priv->usb31misc, USB_MISC_CFGA0,
-				CFGA0_USB2PHY_REFCLK_SELECT,
-				CFGA0_USB2PHY_REFCLK_SELECT);
-		if (ret)
-			goto out;
-
-		ret = regmap_read(priv->usb31misc, USB3OTG_CTRL7, &val);
-		if (ret)
-			goto out;
-		val &= ~CTRL7_USB2_REFCLKSEL_MASK;
-		val |= CTRL7_USB2_REFCLKSEL_PAD;
-		ret = regmap_write(priv->usb31misc, USB3OTG_CTRL7, val);
-		if (ret)
-			goto out;
-
-		ret = regmap_write(priv->peri_crg,
-				PERI_CRG_PEREN6, GT_CLK_USB2PHY_REF);
-		if (ret)
-			goto out;
+		return 0;
 	}
 
+	ret = regmap_update_bits(priv->usb31misc, USB_MISC_CFG54,
+				 CFG54_USB3PHY_REF_USE_PAD,
+				 CFG54_USB3PHY_REF_USE_PAD);
+	if (ret)
+		goto out;
+
+	ret = regmap_update_bits(priv->usb31misc, USB_MISC_CFGA0,
+				 CFGA0_USB2PHY_REFCLK_SELECT,
+				 CFGA0_USB2PHY_REFCLK_SELECT);
+	if (ret)
+		goto out;
+
+	ret = regmap_read(priv->usb31misc, USB3OTG_CTRL7, &val);
+	if (ret)
+		goto out;
+	val &= ~CTRL7_USB2_REFCLKSEL_MASK;
+	val |= CTRL7_USB2_REFCLKSEL_PAD;
+	ret = regmap_write(priv->usb31misc, USB3OTG_CTRL7, val);
+	if (ret)
+		goto out;
+
+	ret = regmap_write(priv->peri_crg,
+			   PERI_CRG_PEREN6, GT_CLK_USB2PHY_REF);
+	if (ret)
+		goto out;
+
 	return 0;
 out:
 	dev_err(priv->dev, "failed to config phy clock ret: %d\n", ret);
@@ -437,7 +432,7 @@ static int hi3670_config_tca(struct hi3670_priv *priv)
 		goto out;
 
 	ret = regmap_write(priv->usb31misc, TCA_INTR_EN,
-			INTR_EN_XA_TIMEOUT_EVT_EN | INTR_EN_XA_ACK_EVT_EN);
+			   INTR_EN_XA_TIMEOUT_EVT_EN | INTR_EN_XA_ACK_EVT_EN);
 	if (ret)
 		goto out;
 
@@ -447,13 +442,13 @@ static int hi3670_config_tca(struct hi3670_priv *priv)
 		goto out;
 
 	ret = regmap_update_bits(priv->usb31misc, TCA_GCFG,
-			GCFG_ROLE_HSTDEV | GCFG_OP_MODE,
-			GCFG_ROLE_HSTDEV | GCFG_OP_MODE_CTRL_SYNC_MODE);
+				 GCFG_ROLE_HSTDEV | GCFG_OP_MODE,
+				 GCFG_ROLE_HSTDEV | GCFG_OP_MODE_CTRL_SYNC_MODE);
 	if (ret)
 		goto out;
 
 	ret = regmap_update_bits(priv->usb31misc, TCA_SYSMODE_CFG,
-			SYSMODE_CFG_TYPEC_DISABLE, 0);
+				 SYSMODE_CFG_TYPEC_DISABLE, 0);
 	if (ret)
 		goto out;
 
@@ -467,7 +462,7 @@ static int hi3670_config_tca(struct hi3670_priv *priv)
 		goto out;
 
 	ret = regmap_write(priv->usb31misc, TCA_VBUS_CTRL,
-		VBUS_CTRL_POWERPRESENT_OVERRD | VBUS_CTRL_VBUSVALID_OVERRD);
+			   VBUS_CTRL_POWERPRESENT_OVERRD | VBUS_CTRL_VBUSVALID_OVERRD);
 	if (ret)
 		goto out;
 
@@ -485,7 +480,7 @@ static int hi3670_phy_init(struct phy *phy)
 
 	/* assert controller */
 	val = CFGA0_VAUX_RESET | CFGA0_USB31C_RESET |
-		CFGA0_USB3PHY_RESET | CFGA0_USB2PHY_POR;
+	      CFGA0_USB3PHY_RESET | CFGA0_USB2PHY_POR;
 	ret = regmap_update_bits(priv->usb31misc, USB_MISC_CFGA0, val, 0);
 	if (ret)
 		goto out;
@@ -496,13 +491,13 @@ static int hi3670_phy_init(struct phy *phy)
 
 	/* Exit from IDDQ mode */
 	ret = regmap_update_bits(priv->usb31misc, USB3OTG_CTRL5,
-			CTRL5_USB2_SIDDQ, 0);
+				 CTRL5_USB2_SIDDQ, 0);
 	if (ret)
 		goto out;
 
 	/* Release USB31 PHY out of TestPowerDown mode */
 	ret = regmap_update_bits(priv->usb31misc, USB_MISC_CFG50,
-			CFG50_USB3_PHY_TEST_POWERDOWN, 0);
+				 CFG50_USB3_PHY_TEST_POWERDOWN, 0);
 	if (ret)
 		goto out;
 
@@ -512,13 +507,13 @@ static int hi3670_phy_init(struct phy *phy)
 	if (ret)
 		goto out;
 
-	udelay(100);
+	usleep_range(100, 120);
 
 	/* Tell the PHY power is stable */
 	val = CFG54_USB3_PHY0_ANA_PWR_EN | CFG54_PHY0_PCS_PWR_STABLE |
-		CFG54_PHY0_PMA_PWR_STABLE;
+	      CFG54_PHY0_PMA_PWR_STABLE;
 	ret = regmap_update_bits(priv->usb31misc, USB_MISC_CFG54,
-			val, val);
+				 val, val);
 	if (ret)
 		goto out;
 
@@ -528,8 +523,8 @@ static int hi3670_phy_init(struct phy *phy)
 
 	/* Enable SSC */
 	ret = regmap_update_bits(priv->usb31misc, USB_MISC_CFG5C,
-			CFG5C_USB3_PHY0_SS_MPLLA_SSC_EN,
-			CFG5C_USB3_PHY0_SS_MPLLA_SSC_EN);
+				 CFG5C_USB3_PHY0_SS_MPLLA_SSC_EN,
+				 CFG5C_USB3_PHY0_SS_MPLLA_SSC_EN);
 	if (ret)
 		goto out;
 
@@ -539,7 +534,7 @@ static int hi3670_phy_init(struct phy *phy)
 	if (ret)
 		goto out;
 
-	udelay(100);
+	usleep_range(100, 120);
 
 	/* Set fake vbus valid signal */
 	val = CTRL0_USB3_VBUSVLD | CTRL0_USB3_VBUSVLD_SEL;
@@ -552,7 +547,7 @@ static int hi3670_phy_init(struct phy *phy)
 	if (ret)
 		goto out;
 
-	udelay(100);
+	usleep_range(100, 120);
 
 	ret = hi3670_phy_set_params(priv);
 	if (ret)
@@ -579,10 +574,10 @@ static int hi3670_phy_exit(struct phy *phy)
 	if (hi3670_is_abbclk_seleted(priv)) {
 		/* disable usb_tcxo_en */
 		ret = regmap_write(priv->pctrl, PCTRL_PERI_CTRL3,
-				USB_TCXO_EN << PCTRL_PERI_CTRL3_MSK_START);
+				   USB_TCXO_EN << PCTRL_PERI_CTRL3_MSK_START);
 	} else {
 		ret = regmap_write(priv->peri_crg, PERI_CRG_PERDIS6,
-				GT_CLK_USB2PHY_REF);
+				   GT_CLK_USB2PHY_REF);
 		if (ret)
 			goto out;
 	}
@@ -612,39 +607,39 @@ static int hi3670_phy_probe(struct platform_device *pdev)
 
 	priv->dev = dev;
 	priv->peri_crg = syscon_regmap_lookup_by_phandle(dev->of_node,
-					"hisilicon,pericrg-syscon");
+							 "hisilicon,pericrg-syscon");
 	if (IS_ERR(priv->peri_crg)) {
 		dev_err(dev, "no hisilicon,pericrg-syscon\n");
 		return PTR_ERR(priv->peri_crg);
 	}
 
 	priv->pctrl = syscon_regmap_lookup_by_phandle(dev->of_node,
-					"hisilicon,pctrl-syscon");
+						      "hisilicon,pctrl-syscon");
 	if (IS_ERR(priv->pctrl)) {
 		dev_err(dev, "no hisilicon,pctrl-syscon\n");
 		return PTR_ERR(priv->pctrl);
 	}
 
 	priv->sctrl = syscon_regmap_lookup_by_phandle(dev->of_node,
-					"hisilicon,sctrl-syscon");
+						      "hisilicon,sctrl-syscon");
 	if (IS_ERR(priv->sctrl)) {
 		dev_err(dev, "no hisilicon,sctrl-syscon\n");
 		return PTR_ERR(priv->sctrl);
 	}
 
 	priv->usb31misc = syscon_regmap_lookup_by_phandle(dev->of_node,
-					"hisilicon,usb31-misc-syscon");
+							  "hisilicon,usb31-misc-syscon");
 	if (IS_ERR(priv->usb31misc)) {
 		dev_err(dev, "no hisilicon,usb31-misc-syscon\n");
 		return PTR_ERR(priv->usb31misc);
 	}
 
 	if (of_property_read_u32(dev->of_node, "eye-diagram-param",
-				&(priv->eye_diagram_param)))
+				 &priv->eye_diagram_param))
 		priv->eye_diagram_param = KIRIN970_USB_DEFAULT_PHY_PARAM;
 
 	if (of_property_read_u32(dev->of_node, "usb3-phy-tx-vboost-lvl",
-				&(priv->tx_vboost_lvl)))
+				 &priv->tx_vboost_lvl))
 		priv->eye_diagram_param = KIRIN970_USB_DEFAULT_PHY_VBOOST;
 
 	phy = devm_phy_create(dev, NULL, &hi3670_phy_ops);
@@ -657,7 +652,7 @@ static int hi3670_phy_probe(struct platform_device *pdev)
 }
 
 static const struct of_device_id hi3670_phy_of_match[] = {
-	{.compatible = "hisilicon,hi3670-usb-phy",},
+	{ .compatible = "hisilicon,hi3670-usb-phy" },
 	{ },
 };
 MODULE_DEVICE_TABLE(of, hi3670_phy_of_match);
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [RFC 05/10] phy: hisilicon: phy-hi3670-usb3: change some DT properties
  2020-09-04 10:23 [RFC 00/10] Add USB support for Hikey 970 Mauro Carvalho Chehab
                   ` (3 preceding siblings ...)
  2020-09-04 10:23 ` [RFC 04/10] phy: hisilicon: phy-hi3670-usb3: fix coding style Mauro Carvalho Chehab
@ 2020-09-04 10:23 ` Mauro Carvalho Chehab
  2020-09-04 10:23 ` [RFC 06/10] dt-bindings: phy: convert phy-kirin970-usb3.txt to yaml Mauro Carvalho Chehab
                   ` (5 subsequent siblings)
  10 siblings, 0 replies; 22+ messages in thread
From: Mauro Carvalho Chehab @ 2020-09-04 10:23 UTC (permalink / raw)
  Cc: linuxarm, mauro.chehab, Mauro Carvalho Chehab, John Stultz,
	Manivannan Sadhasivam, Robin Murphy, Kishon Vijay Abraham I,
	Vinod Koul, Yu Chen, linux-kernel

Do some changes at the DT properties in order to make it
follow the phy-hi3660-usb3 example and to simplify
usb3-phy-tx-vboost-lvl name.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
 drivers/phy/hisilicon/phy-hi3670-usb3.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/phy/hisilicon/phy-hi3670-usb3.c b/drivers/phy/hisilicon/phy-hi3670-usb3.c
index cb0bfcbbfbfa..42dbc20a0b9a 100644
--- a/drivers/phy/hisilicon/phy-hi3670-usb3.c
+++ b/drivers/phy/hisilicon/phy-hi3670-usb3.c
@@ -627,18 +627,18 @@ static int hi3670_phy_probe(struct platform_device *pdev)
 		return PTR_ERR(priv->sctrl);
 	}
 
-	priv->usb31misc = syscon_regmap_lookup_by_phandle(dev->of_node,
-							  "hisilicon,usb31-misc-syscon");
+	/* node of hi3670 phy is a sub-node of usb3_otg_bc */
+	priv->usb31misc = syscon_node_to_regmap(dev->parent->of_node);
 	if (IS_ERR(priv->usb31misc)) {
-		dev_err(dev, "no hisilicon,usb31-misc-syscon\n");
+		dev_err(dev, "no hisilicon,usb3-otg-bc-syscon\n");
 		return PTR_ERR(priv->usb31misc);
 	}
 
-	if (of_property_read_u32(dev->of_node, "eye-diagram-param",
+	if (of_property_read_u32(dev->of_node, "hisilicon,eye-diagram-param",
 				 &priv->eye_diagram_param))
 		priv->eye_diagram_param = KIRIN970_USB_DEFAULT_PHY_PARAM;
 
-	if (of_property_read_u32(dev->of_node, "usb3-phy-tx-vboost-lvl",
+	if (of_property_read_u32(dev->of_node, "hisilicon,tx-vboost-lvl",
 				 &priv->tx_vboost_lvl))
 		priv->eye_diagram_param = KIRIN970_USB_DEFAULT_PHY_VBOOST;
 
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [RFC 06/10] dt-bindings: phy: convert phy-kirin970-usb3.txt to yaml
  2020-09-04 10:23 [RFC 00/10] Add USB support for Hikey 970 Mauro Carvalho Chehab
                   ` (4 preceding siblings ...)
  2020-09-04 10:23 ` [RFC 05/10] phy: hisilicon: phy-hi3670-usb3: change some DT properties Mauro Carvalho Chehab
@ 2020-09-04 10:23 ` Mauro Carvalho Chehab
  2020-09-04 10:23 ` [RFC 07/10] MAINTAINERS: add myself as maintainer for Kirin 970 USB PHY Mauro Carvalho Chehab
                   ` (4 subsequent siblings)
  10 siblings, 0 replies; 22+ messages in thread
From: Mauro Carvalho Chehab @ 2020-09-04 10:23 UTC (permalink / raw)
  Cc: linuxarm, mauro.chehab, Mauro Carvalho Chehab, John Stultz,
	Manivannan Sadhasivam, Robin Murphy, Kishon Vijay Abraham I,
	Vinod Koul, Rob Herring, linux-kernel, devicetree

Use the new YAML for this physical layer.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
 .../bindings/phy/hisilicon,hi3670-usb3.yaml   | 72 +++++++++++++++++++
 .../bindings/phy/phy-hi3670-usb3.txt          | 25 -------
 2 files changed, 72 insertions(+), 25 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/phy/hisilicon,hi3670-usb3.yaml
 delete mode 100644 Documentation/devicetree/bindings/phy/phy-hi3670-usb3.txt

diff --git a/Documentation/devicetree/bindings/phy/hisilicon,hi3670-usb3.yaml b/Documentation/devicetree/bindings/phy/hisilicon,hi3670-usb3.yaml
new file mode 100644
index 000000000000..125a5d6546ae
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/hisilicon,hi3670-usb3.yaml
@@ -0,0 +1,72 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/phy/hisilicon,hi3670-usb3.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Hisilicon Kirin970 USB PHY
+
+maintainers:
+  - Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
+description: |+
+  Bindings for USB3 PHY on HiSilicon Kirin 970.
+
+properties:
+  compatible:
+    const: hisilicon,hi3670-usb-phy
+
+  "#phy-cells":
+    const: 0
+
+  hisilicon,pericrg-syscon:
+    $ref: '/schemas/types.yaml#/definitions/phandle'
+    description: phandle of syscon used to control iso refclk.
+
+  hisilicon,pctrl-syscon:
+    $ref: '/schemas/types.yaml#/definitions/phandle'
+    description: phandle of syscon used to control usb tcxo.
+
+  hisilicon,sctrl-syscon:
+    $ref: '/schemas/types.yaml#/definitions/phandle'
+    description: phandle of syscon used to control phy deep sleep.
+
+  hisilicon,eye-diagram-param:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: Eye diagram for phy.
+
+  hisilicon,tx-vboost-lvl:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: TX level vboost for phy.
+
+required:
+  - compatible
+  - hisilicon,pericrg-syscon
+  - hisilicon,pctrl-syscon
+  - hisilicon,sctrl-syscon
+  - hisilicon,eye-diagram-param
+  - hisilicon,tx-vboost-lvl
+  - "#phy-cells"
+
+additionalProperties: false
+
+examples:
+  - |
+    bus {
+      #address-cells = <2>;
+      #size-cells = <2>;
+
+      usb3_otg_bc: usb3_otg_bc@ff200000 {
+        compatible = "syscon", "simple-mfd";
+        reg = <0x0 0xff200000 0x0 0x1000>;
+
+        usb_phy {
+          compatible = "hisilicon,hi3670-usb-phy";
+          #phy-cells = <0>;
+          hisilicon,pericrg-syscon = <&crg_ctrl>;
+          hisilicon,pctrl-syscon = <&pctrl>;
+          hisilicon,sctrl-syscon = <&sctrl>;
+          hisilicon,eye-diagram-param = <0xfdfee4>;
+          hisilicon,tx-vboost-lvl = <0x5>;
+        };
+      };
+    };
diff --git a/Documentation/devicetree/bindings/phy/phy-hi3670-usb3.txt b/Documentation/devicetree/bindings/phy/phy-hi3670-usb3.txt
deleted file mode 100644
index 2fb27cb8beaf..000000000000
--- a/Documentation/devicetree/bindings/phy/phy-hi3670-usb3.txt
+++ /dev/null
@@ -1,25 +0,0 @@
-Hisilicon Kirin970 usb PHY
------------------------
-
-Required properties:
-- compatible: should be "hisilicon,hi3670-usb-phy"
-- #phy-cells: must be 0
-- hisilicon,pericrg-syscon: phandle of syscon used to control phy.
-- hisilicon,pctrl-syscon: phandle of syscon used to control phy.
-- hisilicon,sctrl-syscon: phandle of syscon used to control phy.
-- hisilicon,usb31-misc-syscon: phandle of syscon used to control phy.
-- eye-diagram-param: parameter set for phy
-- usb3-phy-tx-vboost-lvl: parameter set for phy
-Refer to phy/phy-bindings.txt for the generic PHY binding properties
-
-Example:
-	usb_phy: usbphy {
-		compatible = "hisilicon,hi3670-usb-phy";
-		#phy-cells = <0>;
-		hisilicon,pericrg-syscon = <&crg_ctrl>;
-		hisilicon,pctrl-syscon = <&pctrl>;
-		hisilicon,sctrl-syscon = <&sctrl>;
-		hisilicon,usb31-misc-syscon = <&usb31_misc>;
-		eye-diagram-param = <0xFDFEE4>;
-		usb3-phy-tx-vboost-lvl = <0x5>;
-	};
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [RFC 07/10] MAINTAINERS: add myself as maintainer for Kirin 970 USB PHY
  2020-09-04 10:23 [RFC 00/10] Add USB support for Hikey 970 Mauro Carvalho Chehab
                   ` (5 preceding siblings ...)
  2020-09-04 10:23 ` [RFC 06/10] dt-bindings: phy: convert phy-kirin970-usb3.txt to yaml Mauro Carvalho Chehab
@ 2020-09-04 10:23 ` Mauro Carvalho Chehab
  2020-09-04 10:23 ` [RFC 08/10] misc: hisi_hikey_usb: Driver to support onboard USB gpio hub on Hikey960 Mauro Carvalho Chehab
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 22+ messages in thread
From: Mauro Carvalho Chehab @ 2020-09-04 10:23 UTC (permalink / raw)
  Cc: linuxarm, mauro.chehab, Mauro Carvalho Chehab, John Stultz,
	Manivannan Sadhasivam, Robin Murphy, David S. Miller,
	Rob Herring, linux-kernel

Now that this driver was added upsream, it needs a maintainer.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
 MAINTAINERS | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 2f497c7d659c..267ba0b7a52e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -17711,7 +17711,7 @@ L:	linux-usb@vger.kernel.org
 S:	Maintained
 F:	drivers/usb/roles/intel-xhci-usb-role-switch.c
 
-USB IP DRIVER FOR HISILICON KIRIN
+USB IP DRIVER FOR HISILICON KIRIN 960
 M:	Yu Chen <chenyu56@huawei.com>
 M:	Binghui Wang <wangbinghui@hisilicon.com>
 L:	linux-usb@vger.kernel.org
@@ -17719,6 +17719,13 @@ S:	Maintained
 F:	Documentation/devicetree/bindings/phy/phy-hi3660-usb3.txt
 F:	drivers/phy/hisilicon/phy-hi3660-usb3.c
 
+USB IP DRIVER FOR HISILICON KIRIN 970
+M:	Mauro Carvalho Chehab <mchehab@kernel.org>
+L:	linux-usb@vger.kernel.org
+S:	Maintained
+F:	Documentation/devicetree/bindings/phy/hisilicon,kirin970-usb3.yaml
+F:	drivers/phy/hisilicon/phy-kirin970-usb3.c
+
 USB ISP116X DRIVER
 M:	Olav Kongas <ok@artecdesign.ee>
 L:	linux-usb@vger.kernel.org
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [RFC 08/10] misc: hisi_hikey_usb: Driver to support onboard USB gpio hub on Hikey960
  2020-09-04 10:23 [RFC 00/10] Add USB support for Hikey 970 Mauro Carvalho Chehab
                   ` (6 preceding siblings ...)
  2020-09-04 10:23 ` [RFC 07/10] MAINTAINERS: add myself as maintainer for Kirin 970 USB PHY Mauro Carvalho Chehab
@ 2020-09-04 10:23 ` Mauro Carvalho Chehab
  2020-09-04 12:53   ` Randy Dunlap
  2020-09-04 10:23 ` [RFC 09/10] misc: hisi_hikey_usb: add support for Hikey 970 Mauro Carvalho Chehab
                   ` (2 subsequent siblings)
  10 siblings, 1 reply; 22+ messages in thread
From: Mauro Carvalho Chehab @ 2020-09-04 10:23 UTC (permalink / raw)
  Cc: linuxarm, mauro.chehab, Yu Chen, Mauro Carvalho Chehab,
	John Stultz, Manivannan Sadhasivam, Robin Murphy, Derek Kiernan,
	Dragan Cvetic, Arnd Bergmann, Greg Kroah-Hartman,
	David S. Miller, Rob Herring, linux-kernel

From: Yu Chen <chenyu56@huawei.com>

The HiKey960 has a fairly complex USB configuration due to it
needing to support a USB-C port for host/device mode and multiple
USB-A ports in host mode, all using a single USB controller.

See schematics here:
  https://github.com/96boards/documentation/raw/master/consumer/hikey/hikey960/hardware-docs/HiKey960_Schematics.pdf

This driver acts as a usb-role-switch intermediary, intercepting
the role switch notifications from the tcpm code, and passing
them on to the dwc3 core.

In doing so, it also controls the onboard hub and power gpios in
order to properly route the data lines between the USB-C port
and the onboard hub to the USB-A ports.

Signed-off-by: Yu Chen <chenyu56@huawei.com>
[jstultz: Major rework to make the driver a usb-role-switch
          intermediary]
Signed-off-by: John Stultz <john.stultz@linaro.org>
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
 MAINTAINERS                   |   7 ++
 drivers/misc/Kconfig          |   9 ++
 drivers/misc/Makefile         |   1 +
 drivers/misc/hisi_hikey_usb.c | 205 ++++++++++++++++++++++++++++++++++
 4 files changed, 222 insertions(+)
 create mode 100644 drivers/misc/hisi_hikey_usb.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 267ba0b7a52e..5ea805543b86 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7781,6 +7781,13 @@ W:	http://www.hisilicon.com
 F:	Documentation/devicetree/bindings/net/hisilicon*.txt
 F:	drivers/net/ethernet/hisilicon/
 
+HIKEY960 ONBOARD USB GPIO HUB DRIVER
+M:	John Stultz <john.stultz@linaro.org>
+L:	linux-kernel@vger.kernel.org
+S:	Maintained
+F:	drivers/misc/hisi_hikey_usb.c
+F:	Documentation/devicetree/bindings/misc/hisilicon-hikey-usb.yaml
+
 HISILICON PMU DRIVER
 M:	Shaokun Zhang <zhangshaokun@hisilicon.com>
 S:	Supported
diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index e1b1ba5e2b92..8ceba6153ecc 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -456,6 +456,15 @@ config PVPANIC
 	  a paravirtualized device provided by QEMU; it lets a virtual machine
 	  (guest) communicate panic events to the host.
 
+config HISI_HIKEY_USB
+	tristate "USB GPIO Hub on HiSilicon Hikey960 Platform"
+	depends on OF && GPIOLIB
+	help
+	  If you say yes here this adds support for the on-board USB gpio hub
+	  found on the HiKey960, which is necssary to support switching between
+	  the dual-role USB-C port and the USB-A host ports using only one USB
+	  controller.
+
 source "drivers/misc/c2port/Kconfig"
 source "drivers/misc/eeprom/Kconfig"
 source "drivers/misc/cb710/Kconfig"
diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
index c7bd01ac6291..2521359e8ef7 100644
--- a/drivers/misc/Makefile
+++ b/drivers/misc/Makefile
@@ -57,3 +57,4 @@ obj-$(CONFIG_PVPANIC)   	+= pvpanic.o
 obj-$(CONFIG_HABANA_AI)		+= habanalabs/
 obj-$(CONFIG_UACCE)		+= uacce/
 obj-$(CONFIG_XILINX_SDFEC)	+= xilinx_sdfec.o
+obj-$(CONFIG_HISI_HIKEY_USB)	+= hisi_hikey_usb.o
diff --git a/drivers/misc/hisi_hikey_usb.c b/drivers/misc/hisi_hikey_usb.c
new file mode 100644
index 000000000000..3a98a890757c
--- /dev/null
+++ b/drivers/misc/hisi_hikey_usb.c
@@ -0,0 +1,205 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Support for usb functionality of Hikey series boards
+ * based on Hisilicon Kirin Soc.
+ *
+ * Copyright (C) 2017-2018 Hilisicon Electronics Co., Ltd.
+ *		http://www.huawei.com
+ *
+ * Authors: Yu Chen <chenyu56@huawei.com>
+ */
+
+#include <linux/gpio/consumer.h>
+#include <linux/kernel.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/notifier.h>
+#include <linux/platform_device.h>
+#include <linux/property.h>
+#include <linux/slab.h>
+#include <linux/usb/role.h>
+
+#define DEVICE_DRIVER_NAME "hisi_hikey_usb"
+
+#define HUB_VBUS_POWER_ON 1
+#define HUB_VBUS_POWER_OFF 0
+#define USB_SWITCH_TO_HUB 1
+#define USB_SWITCH_TO_TYPEC 0
+#define TYPEC_VBUS_POWER_ON 1
+#define TYPEC_VBUS_POWER_OFF 0
+
+struct hisi_hikey_usb {
+	struct gpio_desc *otg_switch;
+	struct gpio_desc *typec_vbus;
+	struct gpio_desc *hub_vbus;
+
+	struct usb_role_switch *hub_role_sw;
+
+	struct usb_role_switch *dev_role_sw;
+	enum usb_role role;
+
+	struct mutex lock;
+	struct work_struct work;
+
+	struct notifier_block nb;
+};
+
+static void hub_power_ctrl(struct hisi_hikey_usb *hisi_hikey_usb, int value)
+{
+	gpiod_set_value_cansleep(hisi_hikey_usb->hub_vbus, value);
+}
+
+static void usb_switch_ctrl(struct hisi_hikey_usb *hisi_hikey_usb,
+			    int switch_to)
+{
+	gpiod_set_value_cansleep(hisi_hikey_usb->otg_switch, switch_to);
+}
+
+static void usb_typec_power_ctrl(struct hisi_hikey_usb *hisi_hikey_usb,
+				 int value)
+{
+	gpiod_set_value_cansleep(hisi_hikey_usb->typec_vbus, value);
+}
+
+
+
+static void relay_set_role_switch(struct work_struct *work)
+{
+	struct hisi_hikey_usb *hisi_hikey_usb = container_of(work,
+							struct hisi_hikey_usb,
+							work);
+	struct usb_role_switch *sw;
+	enum usb_role role;
+
+	if (!hisi_hikey_usb || !hisi_hikey_usb->dev_role_sw)
+		return;
+
+	mutex_lock(&hisi_hikey_usb->lock);
+	switch (hisi_hikey_usb->role) {
+	case USB_ROLE_NONE:
+		usb_typec_power_ctrl(hisi_hikey_usb, TYPEC_VBUS_POWER_OFF);
+		usb_switch_ctrl(hisi_hikey_usb, USB_SWITCH_TO_HUB);
+		hub_power_ctrl(hisi_hikey_usb, HUB_VBUS_POWER_ON);
+		break;
+	case USB_ROLE_HOST:
+		hub_power_ctrl(hisi_hikey_usb, HUB_VBUS_POWER_OFF);
+		usb_switch_ctrl(hisi_hikey_usb, USB_SWITCH_TO_TYPEC);
+		usb_typec_power_ctrl(hisi_hikey_usb, TYPEC_VBUS_POWER_ON);
+		break;
+	case USB_ROLE_DEVICE:
+		hub_power_ctrl(hisi_hikey_usb, HUB_VBUS_POWER_OFF);
+		usb_typec_power_ctrl(hisi_hikey_usb, TYPEC_VBUS_POWER_OFF);
+		usb_switch_ctrl(hisi_hikey_usb, USB_SWITCH_TO_TYPEC);
+		break;
+	default:
+		break;
+	}
+	sw = hisi_hikey_usb->dev_role_sw;
+	role = hisi_hikey_usb->role;
+	mutex_unlock(&hisi_hikey_usb->lock);
+
+	usb_role_switch_set_role(sw, role);
+}
+
+static int hub_usb_role_switch_set(struct usb_role_switch *sw, enum usb_role role)
+{
+	struct hisi_hikey_usb *hisi_hikey_usb = usb_role_switch_get_drvdata(sw);
+
+	if (!hisi_hikey_usb || !hisi_hikey_usb->dev_role_sw)
+		return -EINVAL;
+
+	mutex_lock(&hisi_hikey_usb->lock);
+	hisi_hikey_usb->role = role;
+	mutex_unlock(&hisi_hikey_usb->lock);
+
+	schedule_work(&hisi_hikey_usb->work);
+
+	return 0;
+}
+
+static int hisi_hikey_usb_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct hisi_hikey_usb *hisi_hikey_usb;
+	struct usb_role_switch_desc hub_role_switch = {NULL};
+
+	hisi_hikey_usb = devm_kzalloc(dev, sizeof(*hisi_hikey_usb), GFP_KERNEL);
+	if (!hisi_hikey_usb)
+		return -ENOMEM;
+
+	hisi_hikey_usb->typec_vbus = devm_gpiod_get(dev, "typec-vbus",
+						    GPIOD_OUT_LOW);
+	if (IS_ERR(hisi_hikey_usb->typec_vbus))
+		return PTR_ERR(hisi_hikey_usb->typec_vbus);
+
+	hisi_hikey_usb->otg_switch = devm_gpiod_get(dev, "otg-switch",
+						    GPIOD_OUT_HIGH);
+	if (IS_ERR(hisi_hikey_usb->otg_switch))
+		return PTR_ERR(hisi_hikey_usb->otg_switch);
+
+	/* hub-vdd33-en is optional */
+	hisi_hikey_usb->hub_vbus = devm_gpiod_get_optional(dev, "hub-vdd33-en",
+							   GPIOD_OUT_HIGH);
+	if (IS_ERR(hisi_hikey_usb->hub_vbus))
+		return PTR_ERR(hisi_hikey_usb->hub_vbus);
+
+	hisi_hikey_usb->dev_role_sw = usb_role_switch_get(dev);
+	if (!hisi_hikey_usb->dev_role_sw)
+		return -EPROBE_DEFER;
+	if (IS_ERR(hisi_hikey_usb->dev_role_sw))
+		return PTR_ERR(hisi_hikey_usb->dev_role_sw);
+
+
+	INIT_WORK(&hisi_hikey_usb->work, relay_set_role_switch);
+	mutex_init(&hisi_hikey_usb->lock);
+
+	hub_role_switch.fwnode = dev_fwnode(dev);
+	hub_role_switch.set = hub_usb_role_switch_set;
+	hub_role_switch.driver_data = hisi_hikey_usb;
+
+	hisi_hikey_usb->hub_role_sw = usb_role_switch_register(dev,
+							&hub_role_switch);
+
+	if (IS_ERR(hisi_hikey_usb->hub_role_sw)) {
+		usb_role_switch_put(hisi_hikey_usb->dev_role_sw);
+		return PTR_ERR(hisi_hikey_usb->hub_role_sw);
+	}
+
+	platform_set_drvdata(pdev, hisi_hikey_usb);
+
+	return 0;
+}
+
+static int  hisi_hikey_usb_remove(struct platform_device *pdev)
+{
+	struct hisi_hikey_usb *hisi_hikey_usb = platform_get_drvdata(pdev);
+
+	if (hisi_hikey_usb->hub_role_sw)
+		usb_role_switch_unregister(hisi_hikey_usb->hub_role_sw);
+
+	if (hisi_hikey_usb->dev_role_sw)
+		usb_role_switch_put(hisi_hikey_usb->dev_role_sw);
+
+	return 0;
+}
+
+static const struct of_device_id id_table_hisi_hikey_usb[] = {
+	{.compatible = "hisilicon,gpio_hubv1"},
+	{}
+};
+MODULE_DEVICE_TABLE(of, id_table_hisi_hikey_usb);
+
+static struct platform_driver hisi_hikey_usb_driver = {
+	.probe = hisi_hikey_usb_probe,
+	.remove = hisi_hikey_usb_remove,
+	.driver = {
+		.name = DEVICE_DRIVER_NAME,
+		.of_match_table = id_table_hisi_hikey_usb,
+	},
+};
+
+module_platform_driver(hisi_hikey_usb_driver);
+
+MODULE_AUTHOR("Yu Chen <chenyu56@huawei.com>");
+MODULE_DESCRIPTION("Driver Support for USB functionality of Hikey");
+MODULE_LICENSE("GPL v2");
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [RFC 09/10] misc: hisi_hikey_usb: add support for Hikey 970
  2020-09-04 10:23 [RFC 00/10] Add USB support for Hikey 970 Mauro Carvalho Chehab
                   ` (7 preceding siblings ...)
  2020-09-04 10:23 ` [RFC 08/10] misc: hisi_hikey_usb: Driver to support onboard USB gpio hub on Hikey960 Mauro Carvalho Chehab
@ 2020-09-04 10:23 ` Mauro Carvalho Chehab
  2020-09-04 12:23   ` Mark Brown
  2020-09-04 10:23 ` [RFC 10/10] dts: hisilicon: add support for USB3 on " Mauro Carvalho Chehab
  2020-09-04 10:53 ` [RFC 00/10] Add USB support for " Robin Murphy
  10 siblings, 1 reply; 22+ messages in thread
From: Mauro Carvalho Chehab @ 2020-09-04 10:23 UTC (permalink / raw)
  Cc: linuxarm, mauro.chehab, Mauro Carvalho Chehab, John Stultz,
	Manivannan Sadhasivam, Robin Murphy, Arnd Bergmann,
	Greg Kroah-Hartman, Liam Girdwood, Mark Brown, linux-kernel

The HiKey 970 board uses a voltage regulator and GPIO reset pin.

Add support for them.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
 drivers/misc/hisi_hikey_usb.c | 97 +++++++++++++++++++++++++++++------
 1 file changed, 82 insertions(+), 15 deletions(-)

diff --git a/drivers/misc/hisi_hikey_usb.c b/drivers/misc/hisi_hikey_usb.c
index 3a98a890757c..3b08ca880151 100644
--- a/drivers/misc/hisi_hikey_usb.c
+++ b/drivers/misc/hisi_hikey_usb.c
@@ -14,8 +14,10 @@
 #include <linux/mod_devicetable.h>
 #include <linux/module.h>
 #include <linux/notifier.h>
+#include <linux/of_gpio.h>
 #include <linux/platform_device.h>
 #include <linux/property.h>
+#include <linux/regulator/consumer.h>
 #include <linux/slab.h>
 #include <linux/usb/role.h>
 
@@ -46,23 +48,30 @@ struct hisi_hikey_usb {
 
 static void hub_power_ctrl(struct hisi_hikey_usb *hisi_hikey_usb, int value)
 {
+	if (!hisi_hikey_usb->hub_vbus)
+		return;
+
 	gpiod_set_value_cansleep(hisi_hikey_usb->hub_vbus, value);
 }
 
 static void usb_switch_ctrl(struct hisi_hikey_usb *hisi_hikey_usb,
 			    int switch_to)
 {
+	if (!hisi_hikey_usb->otg_switch)
+		return;
+
 	gpiod_set_value_cansleep(hisi_hikey_usb->otg_switch, switch_to);
 }
 
 static void usb_typec_power_ctrl(struct hisi_hikey_usb *hisi_hikey_usb,
 				 int value)
 {
+	if (!hisi_hikey_usb->typec_vbus)
+		return;
+
 	gpiod_set_value_cansleep(hisi_hikey_usb->typec_vbus, value);
 }
 
-
-
 static void relay_set_role_switch(struct work_struct *work)
 {
 	struct hisi_hikey_usb *hisi_hikey_usb = container_of(work,
@@ -117,31 +126,89 @@ static int hub_usb_role_switch_set(struct usb_role_switch *sw, enum usb_role rol
 	return 0;
 }
 
+static int hisi_hikey_usb_parse_kirin970(struct platform_device *pdev)
+{
+	struct regulator *regulator;
+	int hub_reset_en_gpio;
+	int ret;
+
+	regulator = devm_regulator_get_optional(&pdev->dev, "hub-vdd");
+	if (IS_ERR(regulator)) {
+		if (PTR_ERR(regulator) == -EPROBE_DEFER) {
+			dev_info(&pdev->dev,
+				 "waiting for hub-vdd-supply to be probed\n");
+			return PTR_ERR(regulator);
+		}
+
+		/* let it fall back to regulator dummy */
+		regulator = devm_regulator_get(&pdev->dev, "hub-vdd");
+		if (IS_ERR(regulator)) {
+			dev_err(&pdev->dev,
+				"get hub-vdd-supply failed with error %ld\n",
+				PTR_ERR(regulator));
+			return PTR_ERR(regulator);
+		}
+	}
+
+	ret = regulator_set_voltage(regulator, 3300000, 3300000);
+	if (ret)
+		dev_err(&pdev->dev, "set hub-vdd-supply voltage failed\n");
+
+	hub_reset_en_gpio = of_get_named_gpio(pdev->dev.of_node,
+					      "hub_reset_en_gpio", 0);
+	if (!gpio_is_valid(hub_reset_en_gpio)) {
+		dev_err(&pdev->dev, "Failed to get a valid reset gpio\n");
+		return -ENODEV;
+	}
+
+	ret = devm_gpio_request(&pdev->dev, hub_reset_en_gpio,
+				"hub_reset_en_gpio");
+	if (ret) {
+		dev_err(&pdev->dev, "Failed to request the reset gpio\n");
+		return ret;
+	}
+	ret = gpio_direction_output(hub_reset_en_gpio, 1);
+	if (ret)
+		dev_err(&pdev->dev,
+			"Failed to set the direction of the reset gpio\n");
+
+	return ret;
+}
+
 static int hisi_hikey_usb_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	struct hisi_hikey_usb *hisi_hikey_usb;
 	struct usb_role_switch_desc hub_role_switch = {NULL};
+	int ret;
 
 	hisi_hikey_usb = devm_kzalloc(dev, sizeof(*hisi_hikey_usb), GFP_KERNEL);
 	if (!hisi_hikey_usb)
 		return -ENOMEM;
 
-	hisi_hikey_usb->typec_vbus = devm_gpiod_get(dev, "typec-vbus",
-						    GPIOD_OUT_LOW);
-	if (IS_ERR(hisi_hikey_usb->typec_vbus))
-		return PTR_ERR(hisi_hikey_usb->typec_vbus);
-
 	hisi_hikey_usb->otg_switch = devm_gpiod_get(dev, "otg-switch",
 						    GPIOD_OUT_HIGH);
 	if (IS_ERR(hisi_hikey_usb->otg_switch))
 		return PTR_ERR(hisi_hikey_usb->otg_switch);
 
-	/* hub-vdd33-en is optional */
-	hisi_hikey_usb->hub_vbus = devm_gpiod_get_optional(dev, "hub-vdd33-en",
-							   GPIOD_OUT_HIGH);
-	if (IS_ERR(hisi_hikey_usb->hub_vbus))
-		return PTR_ERR(hisi_hikey_usb->hub_vbus);
+	hisi_hikey_usb->typec_vbus = devm_gpiod_get(dev, "typec-vbus",
+						    GPIOD_OUT_LOW);
+	if (IS_ERR(hisi_hikey_usb->typec_vbus))
+		return PTR_ERR(hisi_hikey_usb->typec_vbus);
+
+	/* Parse Kirin 970-specific OF data */
+	if (of_device_is_compatible(pdev->dev.of_node,
+				    "hisilicon,kirin970_hikey_usbhub")) {
+		ret = hisi_hikey_usb_parse_kirin970(pdev);
+		if (ret)
+			return ret;
+	} else {
+		/* hub-vdd33-en is optional */
+		hisi_hikey_usb->hub_vbus = devm_gpiod_get_optional(dev, "hub-vdd33-en",
+								   GPIOD_OUT_HIGH);
+		if (IS_ERR(hisi_hikey_usb->hub_vbus))
+			return PTR_ERR(hisi_hikey_usb->hub_vbus);
+	}
 
 	hisi_hikey_usb->dev_role_sw = usb_role_switch_get(dev);
 	if (!hisi_hikey_usb->dev_role_sw)
@@ -149,7 +216,6 @@ static int hisi_hikey_usb_probe(struct platform_device *pdev)
 	if (IS_ERR(hisi_hikey_usb->dev_role_sw))
 		return PTR_ERR(hisi_hikey_usb->dev_role_sw);
 
-
 	INIT_WORK(&hisi_hikey_usb->work, relay_set_role_switch);
 	mutex_init(&hisi_hikey_usb->lock);
 
@@ -158,7 +224,7 @@ static int hisi_hikey_usb_probe(struct platform_device *pdev)
 	hub_role_switch.driver_data = hisi_hikey_usb;
 
 	hisi_hikey_usb->hub_role_sw = usb_role_switch_register(dev,
-							&hub_role_switch);
+							       &hub_role_switch);
 
 	if (IS_ERR(hisi_hikey_usb->hub_role_sw)) {
 		usb_role_switch_put(hisi_hikey_usb->dev_role_sw);
@@ -184,7 +250,8 @@ static int  hisi_hikey_usb_remove(struct platform_device *pdev)
 }
 
 static const struct of_device_id id_table_hisi_hikey_usb[] = {
-	{.compatible = "hisilicon,gpio_hubv1"},
+	{ .compatible = "hisilicon,gpio_hubv1" },
+	{ .compatible = "hisilicon,kirin970_hikey_usbhub" },
 	{}
 };
 MODULE_DEVICE_TABLE(of, id_table_hisi_hikey_usb);
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [RFC 10/10] dts: hisilicon: add support for USB3 on Hikey 970
  2020-09-04 10:23 [RFC 00/10] Add USB support for Hikey 970 Mauro Carvalho Chehab
                   ` (8 preceding siblings ...)
  2020-09-04 10:23 ` [RFC 09/10] misc: hisi_hikey_usb: add support for Hikey 970 Mauro Carvalho Chehab
@ 2020-09-04 10:23 ` Mauro Carvalho Chehab
  2020-09-04 10:53 ` [RFC 00/10] Add USB support for " Robin Murphy
  10 siblings, 0 replies; 22+ messages in thread
From: Mauro Carvalho Chehab @ 2020-09-04 10:23 UTC (permalink / raw)
  Cc: linuxarm, mauro.chehab, Mauro Carvalho Chehab, John Stultz,
	Manivannan Sadhasivam, Robin Murphy, Wei Xu, Rob Herring,
	linux-arm-kernel, devicetree, linux-kernel

Add the USB3 bindings for Kirin 970 phy and Hikey 970 board.

Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
 .../boot/dts/hisilicon/hi3670-hikey970.dts    | 103 ++++++++++++++++++
 arch/arm64/boot/dts/hisilicon/hi3670.dtsi     |  49 +++++++++
 2 files changed, 152 insertions(+)

diff --git a/arch/arm64/boot/dts/hisilicon/hi3670-hikey970.dts b/arch/arm64/boot/dts/hisilicon/hi3670-hikey970.dts
index f218acceec0b..f1773e5ecaef 100644
--- a/arch/arm64/boot/dts/hisilicon/hi3670-hikey970.dts
+++ b/arch/arm64/boot/dts/hisilicon/hi3670-hikey970.dts
@@ -62,6 +62,29 @@ wlan_en: wlan-en-1-8v {
 		startup-delay-us = <70000>;
 		enable-active-high;
 	};
+	hikey_usbhub: hikey_usbhub {
+		compatible = "hisilicon,kirin970_hikey_usbhub";
+
+		typec-vbus-gpios = <&gpio26 1 0>;
+		otg-switch-gpios = <&gpio4 2 0>;
+		hub_reset_en_gpio = <&gpio0 3 0>;
+		hub-vdd-supply = <&ldo17>;
+		usb-role-switch;
+
+		port {
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			hikey_usb_ep0: endpoint@0 {
+				reg = <0>;
+				remote-endpoint = <&dwc3_role_switch>;
+			};
+			hikey_usb_ep1: endpoint@1 {
+				reg = <1>;
+				remote-endpoint = <&rt1711h_ep>;
+			};
+		};
+	};
 };
 
 /*
@@ -440,6 +463,57 @@ &uart6 {
 	status = "okay";
 };
 
+&i2c1 {
+	status = "okay";
+
+	rt1711h: rt1711h@4e {
+		compatible = "richtek,rt1711h";
+		reg = <0x4e>;
+		status = "ok";
+		interrupt-parent = <&gpio27>;
+		interrupts = <5 IRQ_TYPE_LEVEL_LOW>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&usb_cfg_func>;
+
+		usb_con: connector {
+			compatible = "usb-c-connector";
+			label = "USB-C";
+			data-role = "dual";
+			power-role = "dual";
+			try-power-role = "sink";
+			source-pdos = <PDO_FIXED(5000, 500, PDO_FIXED_USB_COMM)>;
+			sink-pdos = <PDO_FIXED(5000, 500, PDO_FIXED_USB_COMM)
+				PDO_VAR(5000, 5000, 1000)>;
+			op-sink-microwatt = <10000000>;
+
+			ports {
+				#address-cells = <1>;
+				#size-cells = <0>;
+				port@1 {
+					reg = <1>;
+					usb_con_ss: endpoint {
+						remote-endpoint = <&dwc3_ss>;
+					};
+				};
+			};
+		};
+		port {
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			rt1711h_ep: endpoint@0 {
+				reg = <0>;
+				remote-endpoint = <&hikey_usb_ep1>;
+			};
+		};
+	};
+};
+
+&i2c2 {
+	/* USB HUB is on this bus at address 0x44 */
+	status = "okay";
+};
+
 &i2c4 {
 	status = "okay";
 
@@ -469,3 +543,32 @@ adv7533_in: endpoint {
 		};
 	};
 };
+
+&dwc3 { /* USB */
+	dr_mode = "otg";
+	maximum-speed = "super-speed";
+	phy_type = "utmi";
+	snps,dis-del-phy-power-chg-quirk;
+	snps,lfps_filter_quirk;
+	snps,dis_u2_susphy_quirk;
+	snps,dis_u3_susphy_quirk;
+	snps,tx_de_emphasis_quirk;
+	snps,tx_de_emphasis = <1>;
+	snps,dis_enblslpm_quirk;
+	snps,gctl-reset-quirk;
+	usb-role-switch;
+	role-switch-default-mode = "host";
+	port {
+		#address-cells = <1>;
+		#size-cells = <0>;
+		dwc3_role_switch: endpoint@0 {
+			reg = <0>;
+			remote-endpoint = <&hikey_usb_ep0>;
+		};
+
+		dwc3_ss: endpoint@1 {
+			reg = <1>;
+			remote-endpoint = <&usb_con_ss>;
+		};
+	};
+};
diff --git a/arch/arm64/boot/dts/hisilicon/hi3670.dtsi b/arch/arm64/boot/dts/hisilicon/hi3670.dtsi
index e2b2e21295a7..bde3f5ece80c 100644
--- a/arch/arm64/boot/dts/hisilicon/hi3670.dtsi
+++ b/arch/arm64/boot/dts/hisilicon/hi3670.dtsi
@@ -8,6 +8,7 @@
 
 #include <dt-bindings/interrupt-controller/arm-gic.h>
 #include <dt-bindings/clock/hi3670-clock.h>
+#include <dt-bindings/usb/pd.h>
 
 / {
 	compatible = "hisilicon,hi3670";
@@ -800,5 +801,53 @@ i2c4: i2c@fdf0d000 {
 			pinctrl-0 = <&i2c4_pmx_func &i2c4_cfg_func>;
 			status = "disabled";
 		};
+
+		usb3_otg_bc: usb3_otg_bc@ff200000 {
+			compatible = "syscon", "simple-mfd";
+			reg = <0x0 0xff200000 0x0 0x1000>;
+
+			usb_phy: usbphy {
+				compatible = "hisilicon,hi3670-usb-phy";
+				#phy-cells = <0>;
+				hisilicon,pericrg-syscon = <&crg_ctrl>;
+				hisilicon,pctrl-syscon = <&pctrl>;
+				hisilicon,sctrl-syscon = <&sctrl>;
+				hisilicon,eye-diagram-param = <0xFDFEE4>;
+				hisilicon,tx-vboost-lvl = <0x5>;
+			};
+		};
+
+		usb31_misc_rst: usb31_misc_rst_controller {
+			compatible = "hisilicon,hi3660-reset";
+			#reset-cells = <2>;
+			hisi,rst-syscon = <&usb3_otg_bc>;
+		};
+
+		dwc3: dwc3@ff100000 {
+			compatible = "snps,dwc3";
+			reg = <0x0 0xff100000 0x0 0x100000>;
+
+			clocks = <&crg_ctrl HI3670_CLK_GATE_ABB_USB>,
+				 <&crg_ctrl HI3670_HCLK_GATE_USB3OTG>,
+				 <&crg_ctrl HI3670_CLK_GATE_USB3OTG_REF>,
+				 <&crg_ctrl HI3670_ACLK_GATE_USB3DVFS>;
+			clock-names = "clk_gate_abb_usb",
+				      "hclk_gate_usb3otg",
+				      "clk_gate_usb3otg_ref",
+				      "aclk_gate_usb3dvfs";
+
+			assigned-clocks = <&crg_ctrl HI3670_ACLK_GATE_USB3DVFS>;
+			assigned-clock-rates = <238000000>;
+			resets = <&crg_rst 0x90 6>,
+				 <&crg_rst 0x90 7>,
+				 <&usb31_misc_rst 0xA0 8>,
+				 <&usb31_misc_rst 0xA0 9>;
+
+			interrupts = <0 159 IRQ_TYPE_LEVEL_HIGH>,
+				     <0 161 IRQ_TYPE_LEVEL_HIGH>;
+
+			phys = <&usb_phy>;
+			phy-names = "usb3-phy";
+		};
 	};
 };
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [RFC 00/10] Add USB support for Hikey 970
  2020-09-04 10:23 [RFC 00/10] Add USB support for Hikey 970 Mauro Carvalho Chehab
                   ` (9 preceding siblings ...)
  2020-09-04 10:23 ` [RFC 10/10] dts: hisilicon: add support for USB3 on " Mauro Carvalho Chehab
@ 2020-09-04 10:53 ` Robin Murphy
  2020-09-04 12:28   ` Mauro Carvalho Chehab
  10 siblings, 1 reply; 22+ messages in thread
From: Robin Murphy @ 2020-09-04 10:53 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: linuxarm, mauro.chehab, John Stultz, Manivannan Sadhasivam,
	Krzysztof Kozlowski, linux-kernel, Mark Brown,
	Greg Kroah-Hartman, Derek Kiernan, Arnd Bergmann, Vinod Koul,
	Kishon Vijay Abraham I, David S. Miller, Rob Herring, Yu Chen,
	Rob Herring, Dragan Cvetic, linux-arm-kernel, devicetree,
	Liam Girdwood, Wei Xu

On 2020-09-04 11:23, Mauro Carvalho Chehab wrote:
> This RFC adds what seems to be needed for USB to work with Hikey 970.
> While this driver works fine on Kernel 4.9 and 4.19, there's a hack there,
> in the form of some special binding logic under dwg3 driver,  that seems to
>   be just adding some delay,  and doing some different initializations at
> PM (basically, disabling autosuspend).
> 
> With upstream Kernel, however, I'm getting a hard to track
> Kernel panic:
> 
> [    1.837458] SError Interrupt on CPU0, code 0xbf000002 -- SError
> [    1.837462] CPU: 0 PID: 74 Comm: kworker/0:1 Not tainted 5.8.0+ #205
> [    1.837463] Hardware name: HiKey970 (DT)
> [    1.837465] Workqueue: events deferred_probe_work_func
> [    1.837467] pstate: 20000005 (nzCv daif -PAN -UAO BTYPE=--)
> [    1.837468] pc : _raw_spin_unlock_irqrestore+0x18/0x50
> [    1.837469] lr : regmap_unlock_spinlock+0x14/0x20
> [    1.837470] sp : ffff8000124dba60
> [    1.837471] x29: ffff8000124dba60 x28: 0000000000000000
> [    1.837474] x27: ffff0001b7e854c8 x26: ffff80001204ea18
> [    1.837476] x25: 0000000000000005 x24: ffff800011f918f8
> [    1.837479] x23: ffff800011fbb588 x22: ffff0001b7e40e00
> [    1.837481] x21: 0000000000000100 x20: 0000000000000000
> [    1.837483] x19: ffff0001b767ec00 x18: 00000000ff10c000
> [    1.837485] x17: 0000000000000002 x16: 0000b0740fdb9950
> [    1.837488] x15: ffff8000116c1198 x14: ffffffffffffffff
> [    1.837490] x13: 0000000000000030 x12: 0101010101010101
> [    1.837493] x11: 0000000000000020 x10: ffff0001bf17d130
> [    1.837495] x9 : 0000000000000000 x8 : ffff0001b6938080
> [    1.837497] x7 : 0000000000000000 x6 : 000000000000003f
> [    1.837500] x5 : 0000000000000000 x4 : 0000000000000000
> [    1.837502] x3 : ffff80001096a880 x2 : 0000000000000000
> [    1.837505] x1 : ffff0001b7e40e00 x0 : 0000000100000001
> [    1.837507] Kernel panic - not syncing: Asynchronous SError Interrupt
> [    1.837509] CPU: 0 PID: 74 Comm: kworker/0:1 Not tainted 5.8.0+ #205
> [    1.837510] Hardware name: HiKey970 (DT)
> [    1.837511] Workqueue: events deferred_probe_work_func
> [    1.837513] Call trace:
> [    1.837514]  dump_backtrace+0x0/0x1e0
> [    1.837515]  show_stack+0x18/0x24
> [    1.837516]  dump_stack+0xc0/0x11c
> [    1.837517]  panic+0x15c/0x324
> [    1.837518]  nmi_panic+0x8c/0x90
> [    1.837519]  arm64_serror_panic+0x78/0x84
> [    1.837520]  do_serror+0x158/0x15c
> [    1.837521]  el1_error+0x84/0x100
> [    1.837522]  _raw_spin_unlock_irqrestore+0x18/0x50
> [    1.837523]  regmap_write+0x58/0x80
> [    1.837524]  hi3660_reset_deassert+0x28/0x34
> [    1.837526]  reset_control_deassert+0x50/0x260
> [    1.837527]  reset_control_deassert+0xf4/0x260
> [    1.837528]  dwc3_probe+0x5dc/0xe6c
> [    1.837529]  platform_drv_probe+0x54/0xb0
> [    1.837530]  really_probe+0xe0/0x490
> [    1.837531]  driver_probe_device+0xf4/0x160
> [    1.837532]  __device_attach_driver+0x8c/0x114
> [    1.837533]  bus_for_each_drv+0x78/0xcc
> [    1.837534]  __device_attach+0x108/0x1a0
> [    1.837535]  device_initial_probe+0x14/0x20
> [    1.837537]  bus_probe_device+0x98/0xa0
> [    1.837538]  deferred_probe_work_func+0x88/0xe0
> [    1.837539]  process_one_work+0x1cc/0x350
> [    1.837540]  worker_thread+0x2c0/0x470
> [    1.837541]  kthread+0x154/0x160
> [    1.837542]  ret_from_fork+0x10/0x30
> [    1.837569] SMP: stopping secondary CPUs
> [    1.837570] Kernel Offset: 0x1d0000 from 0xffff800010000000
> [    1.837571] PHYS_OFFSET: 0x0
> [    1.837572] CPU features: 0x240002,20882004
> [    1.837573] Memory Limit: none
> 
> I suspect that the driver works downstream because of the extra
> delay of probing an additional driver, as porting such driver
> to upstream sometimes makes this driver to work. So, it could
> be due to some race condition.
> 
> The problem could also be due to something wrong at DT,
> as, with the additional driver, the DT bindings are different.
> 
> As I'm not familiar about the Serror error mechanism on ARM,
> I'm wondering if someone could give me some hint about how
> can I identify what's happening here.  Due to the panic(), I
> can't check what wrong happened there. Not sure if it would
> be safe to just hack the error handling part of Serror, in order
> to let the boot to finish.

As one of my colleagues so wonderfully puts it, SError is effectively 
"part of the SoC has fallen off" - it's an asynchronous notification of 
some serious error condition external to the CPU. In this case, it seems 
quite likely from trying to access a hardware block before it's powered 
up or has finished its reset cycle, and so some CPU access is raising an 
error in the interconnect instead of completing as expected.

Robin.

> PS.: the driver/misc hisi_hikey_usb driver on this series came
> from the John Stultz Hikey 960 tree.
> 
> Thanks!
> Mauro
> 
> Mauro Carvalho Chehab (7):
>    phy: hisilicon: phy-hi3670-usb3: use a consistent namespace
>    phy: hisilicon: phy-hi3670-usb3: fix coding style
>    phy: hisilicon: phy-hi3670-usb3: change some DT properties
>    dt-bindings: phy: convert phy-kirin970-usb3.txt to yaml
>    MAINTAINERS: add myself as maintainer for Kirin 970 USB PHY
>    misc: hisi_hikey_usb: add support for Hikey 970
>    dts: hisilicon: add support for USB3 on Hikey 970
> 
> Yu Chen (3):
>    phy: hisilicon: add USB physical layer for Kirin 3670
>    phy: hisilicon: phy-hi3670-usb3: fix some issues at the init code
>    misc: hisi_hikey_usb: Driver to support onboard USB gpio hub on
>      Hikey960
> 
>   .../bindings/phy/hisilicon,hi3670-usb3.yaml   |  72 ++
>   MAINTAINERS                                   |  16 +-
>   .../boot/dts/hisilicon/hi3670-hikey970.dts    | 103 +++
>   arch/arm64/boot/dts/hisilicon/hi3670.dtsi     |  49 ++
>   drivers/misc/Kconfig                          |   9 +
>   drivers/misc/Makefile                         |   1 +
>   drivers/misc/hisi_hikey_usb.c                 | 272 +++++++
>   drivers/phy/hisilicon/Kconfig                 |  10 +
>   drivers/phy/hisilicon/Makefile                |   1 +
>   drivers/phy/hisilicon/phy-hi3670-usb3.c       | 671 ++++++++++++++++++
>   10 files changed, 1203 insertions(+), 1 deletion(-)
>   create mode 100644 Documentation/devicetree/bindings/phy/hisilicon,hi3670-usb3.yaml
>   create mode 100644 drivers/misc/hisi_hikey_usb.c
>   create mode 100644 drivers/phy/hisilicon/phy-hi3670-usb3.c
> 

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [RFC 09/10] misc: hisi_hikey_usb: add support for Hikey 970
  2020-09-04 10:23 ` [RFC 09/10] misc: hisi_hikey_usb: add support for Hikey 970 Mauro Carvalho Chehab
@ 2020-09-04 12:23   ` Mark Brown
  2020-09-04 12:38     ` Mauro Carvalho Chehab
  0 siblings, 1 reply; 22+ messages in thread
From: Mark Brown @ 2020-09-04 12:23 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: linuxarm, mauro.chehab, John Stultz, Manivannan Sadhasivam,
	Robin Murphy, Arnd Bergmann, Greg Kroah-Hartman, Liam Girdwood,
	linux-kernel

[-- Attachment #1: Type: text/plain, Size: 1440 bytes --]

On Fri, Sep 04, 2020 at 12:23:31PM +0200, Mauro Carvalho Chehab wrote:

> +	regulator = devm_regulator_get_optional(&pdev->dev, "hub-vdd");
> +	if (IS_ERR(regulator)) {
> +		if (PTR_ERR(regulator) == -EPROBE_DEFER) {
> +			dev_info(&pdev->dev,
> +				 "waiting for hub-vdd-supply to be probed\n");
> +			return PTR_ERR(regulator);
> +		}
> +
> +		/* let it fall back to regulator dummy */
> +		regulator = devm_regulator_get(&pdev->dev, "hub-vdd");
> +		if (IS_ERR(regulator)) {
> +			dev_err(&pdev->dev,
> +				"get hub-vdd-supply failed with error %ld\n",
> +				PTR_ERR(regulator));
> +			return PTR_ERR(regulator);
> +		}
> +	}

This seems weird - if the supply is non-optional why is the code trying
with devm_regulator_get_optional()?  Just use normal get directly.

> +	ret = regulator_set_voltage(regulator, 3300000, 3300000);
> +	if (ret)
> +		dev_err(&pdev->dev, "set hub-vdd-supply voltage failed\n");

Unless the device is actively managing the voltage at runtime it should
just let the voltage be set by machine constraints, most of the time
this will do nothing.  This only sets the voltage in this one place.

> +	hub_reset_en_gpio = of_get_named_gpio(pdev->dev.of_node,
> +					      "hub_reset_en_gpio", 0);
> +	if (!gpio_is_valid(hub_reset_en_gpio)) {
> +		dev_err(&pdev->dev, "Failed to get a valid reset gpio\n");
> +		return -ENODEV;
> +	}

Why not just use devm_gpio_request() which already asks for the GPIO by
name?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [RFC 00/10] Add USB support for Hikey 970
  2020-09-04 10:53 ` [RFC 00/10] Add USB support for " Robin Murphy
@ 2020-09-04 12:28   ` Mauro Carvalho Chehab
  2020-09-04 12:34     ` Mark Brown
  0 siblings, 1 reply; 22+ messages in thread
From: Mauro Carvalho Chehab @ 2020-09-04 12:28 UTC (permalink / raw)
  To: Robin Murphy
  Cc: linuxarm, mauro.chehab, John Stultz, Manivannan Sadhasivam,
	Krzysztof Kozlowski, linux-kernel, Mark Brown,
	Greg Kroah-Hartman, Derek Kiernan, Arnd Bergmann, Vinod Koul,
	Kishon Vijay Abraham I, David S. Miller, Rob Herring, Yu Chen,
	Rob Herring, Dragan Cvetic, linux-arm-kernel, devicetree,
	Liam Girdwood, Wei Xu

Em Fri, 4 Sep 2020 11:53:10 +0100
Robin Murphy <robin.murphy@arm.com> escreveu:

> On 2020-09-04 11:23, Mauro Carvalho Chehab wrote:
> > This RFC adds what seems to be needed for USB to work with Hikey 970.
> > While this driver works fine on Kernel 4.9 and 4.19, there's a hack there,
> > in the form of some special binding logic under dwg3 driver,  that seems to
> >   be just adding some delay,  and doing some different initializations at
> > PM (basically, disabling autosuspend).
> > 
> > With upstream Kernel, however, I'm getting a hard to track
> > Kernel panic:
> > 
> > [    1.837458] SError Interrupt on CPU0, code 0xbf000002 -- SError
> > [    1.837462] CPU: 0 PID: 74 Comm: kworker/0:1 Not tainted 5.8.0+ #205
> > [    1.837463] Hardware name: HiKey970 (DT)
> > [    1.837465] Workqueue: events deferred_probe_work_func
> > [    1.837467] pstate: 20000005 (nzCv daif -PAN -UAO BTYPE=--)
> > [    1.837468] pc : _raw_spin_unlock_irqrestore+0x18/0x50
> > [    1.837469] lr : regmap_unlock_spinlock+0x14/0x20
> > [    1.837470] sp : ffff8000124dba60
> > [    1.837471] x29: ffff8000124dba60 x28: 0000000000000000
> > [    1.837474] x27: ffff0001b7e854c8 x26: ffff80001204ea18
> > [    1.837476] x25: 0000000000000005 x24: ffff800011f918f8
> > [    1.837479] x23: ffff800011fbb588 x22: ffff0001b7e40e00
> > [    1.837481] x21: 0000000000000100 x20: 0000000000000000
> > [    1.837483] x19: ffff0001b767ec00 x18: 00000000ff10c000
> > [    1.837485] x17: 0000000000000002 x16: 0000b0740fdb9950
> > [    1.837488] x15: ffff8000116c1198 x14: ffffffffffffffff
> > [    1.837490] x13: 0000000000000030 x12: 0101010101010101
> > [    1.837493] x11: 0000000000000020 x10: ffff0001bf17d130
> > [    1.837495] x9 : 0000000000000000 x8 : ffff0001b6938080
> > [    1.837497] x7 : 0000000000000000 x6 : 000000000000003f
> > [    1.837500] x5 : 0000000000000000 x4 : 0000000000000000
> > [    1.837502] x3 : ffff80001096a880 x2 : 0000000000000000
> > [    1.837505] x1 : ffff0001b7e40e00 x0 : 0000000100000001
> > [    1.837507] Kernel panic - not syncing: Asynchronous SError Interrupt
> > [    1.837509] CPU: 0 PID: 74 Comm: kworker/0:1 Not tainted 5.8.0+ #205
> > [    1.837510] Hardware name: HiKey970 (DT)
> > [    1.837511] Workqueue: events deferred_probe_work_func
> > [    1.837513] Call trace:
> > [    1.837514]  dump_backtrace+0x0/0x1e0
> > [    1.837515]  show_stack+0x18/0x24
> > [    1.837516]  dump_stack+0xc0/0x11c
> > [    1.837517]  panic+0x15c/0x324
> > [    1.837518]  nmi_panic+0x8c/0x90
> > [    1.837519]  arm64_serror_panic+0x78/0x84
> > [    1.837520]  do_serror+0x158/0x15c
> > [    1.837521]  el1_error+0x84/0x100
> > [    1.837522]  _raw_spin_unlock_irqrestore+0x18/0x50
> > [    1.837523]  regmap_write+0x58/0x80
> > [    1.837524]  hi3660_reset_deassert+0x28/0x34
> > [    1.837526]  reset_control_deassert+0x50/0x260
> > [    1.837527]  reset_control_deassert+0xf4/0x260
> > [    1.837528]  dwc3_probe+0x5dc/0xe6c
> > [    1.837529]  platform_drv_probe+0x54/0xb0
> > [    1.837530]  really_probe+0xe0/0x490
> > [    1.837531]  driver_probe_device+0xf4/0x160
> > [    1.837532]  __device_attach_driver+0x8c/0x114
> > [    1.837533]  bus_for_each_drv+0x78/0xcc
> > [    1.837534]  __device_attach+0x108/0x1a0
> > [    1.837535]  device_initial_probe+0x14/0x20
> > [    1.837537]  bus_probe_device+0x98/0xa0
> > [    1.837538]  deferred_probe_work_func+0x88/0xe0
> > [    1.837539]  process_one_work+0x1cc/0x350
> > [    1.837540]  worker_thread+0x2c0/0x470
> > [    1.837541]  kthread+0x154/0x160
> > [    1.837542]  ret_from_fork+0x10/0x30
> > [    1.837569] SMP: stopping secondary CPUs
> > [    1.837570] Kernel Offset: 0x1d0000 from 0xffff800010000000
> > [    1.837571] PHYS_OFFSET: 0x0
> > [    1.837572] CPU features: 0x240002,20882004
> > [    1.837573] Memory Limit: none
> > 
> > I suspect that the driver works downstream because of the extra
> > delay of probing an additional driver, as porting such driver
> > to upstream sometimes makes this driver to work. So, it could
> > be due to some race condition.
> > 
> > The problem could also be due to something wrong at DT,
> > as, with the additional driver, the DT bindings are different.
> > 
> > As I'm not familiar about the Serror error mechanism on ARM,
> > I'm wondering if someone could give me some hint about how
> > can I identify what's happening here.  Due to the panic(), I
> > can't check what wrong happened there. Not sure if it would
> > be safe to just hack the error handling part of Serror, in order
> > to let the boot to finish.  
> 
> As one of my colleagues so wonderfully puts it, SError is effectively 
> "part of the SoC has fallen off" - it's an asynchronous notification of 
> some serious error condition external to the CPU. In this case, it seems 
> quite likely from trying to access a hardware block before it's powered 
> up or has finished its reset cycle, and so some CPU access is raising an 
> error in the interconnect instead of completing as expected.

Looking at the datasheet:

	https://www.96boards.org/documentation/consumer/hikey/hikey970/hardware-docs/files/hikey970-schematics.pdf

It sounds that the only power supply related to USB is for the USB
hub (ldo17), with sounds unlikely to affect dwc3. Yet, just in case,
I added a dirty hack at dwc3_probe:

<snip>
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 25c686a752b0..e42be91fe6c2 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -1491,6 +1491,11 @@ static int dwc3_probe(struct platform_device *pdev)
 
 	}
 
+struct regulator *reg;
+reg = devm_regulator_get(dev, "ldo17");
+ret = PTR_ERR_OR_ZERO(reg);
+if (ret) return ret;
+
 	ret = reset_control_deassert(dwc->reset);
 	if (ret)
 		return ret;
</snip>

Nothing changed.

The enclosed patch is enough for it to stop panic'ing and for the driver
to show signs of life:

	$ lsusb
	Bus 002 Device 001: ID 1d6b:0003 Linux Foundation 3.0 root hub
	Bus 001 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub

The probe code there is pretty much the same as the dwc3 one, but
it doesn't contain calls to pm_* stuff. So, as far as I understood, 
this way, dwc3 won't touch the clocks. 

Thanks,
Mauro




diff --git a/arch/arm64/boot/dts/hisilicon/hi3670.dtsi b/arch/arm64/boot/dts/hisilicon/hi3670.dtsi
index bde3f5ece80c..dcc436de9b41 100644
--- a/arch/arm64/boot/dts/hisilicon/hi3670.dtsi
+++ b/arch/arm64/boot/dts/hisilicon/hi3670.dtsi
@@ -823,14 +823,16 @@ usb31_misc_rst: usb31_misc_rst_controller {
 			hisi,rst-syscon = <&usb3_otg_bc>;
 		};
 
-		dwc3: dwc3@ff100000 {
-			compatible = "snps,dwc3";
-			reg = <0x0 0xff100000 0x0 0x100000>;
+		usb3: hisi_dwc3 {
+			compatible = "hisilicon,kirin970-dwc3";
+			#address-cells = <2>;
+			#size-cells = <2>;
+			ranges;
 
 			clocks = <&crg_ctrl HI3670_CLK_GATE_ABB_USB>,
-				 <&crg_ctrl HI3670_HCLK_GATE_USB3OTG>,
-				 <&crg_ctrl HI3670_CLK_GATE_USB3OTG_REF>,
-				 <&crg_ctrl HI3670_ACLK_GATE_USB3DVFS>;
+				  <&crg_ctrl HI3670_HCLK_GATE_USB3OTG>,
+				  <&crg_ctrl HI3670_CLK_GATE_USB3OTG_REF>,
+				  <&crg_ctrl HI3670_ACLK_GATE_USB3DVFS>;
 			clock-names = "clk_gate_abb_usb",
 				      "hclk_gate_usb3otg",
 				      "clk_gate_usb3otg_ref",
@@ -843,11 +845,16 @@ dwc3: dwc3@ff100000 {
 				 <&usb31_misc_rst 0xA0 8>,
 				 <&usb31_misc_rst 0xA0 9>;
 
-			interrupts = <0 159 IRQ_TYPE_LEVEL_HIGH>,
-				     <0 161 IRQ_TYPE_LEVEL_HIGH>;
+			dwc3: dwc3@ff100000 {
+				compatible = "snps,dwc3";
+				reg = <0x0 0xff100000 0x0 0x100000>;
 
-			phys = <&usb_phy>;
-			phy-names = "usb3-phy";
+				interrupts = <0 159 IRQ_TYPE_LEVEL_HIGH>,
+					    <0 161 IRQ_TYPE_LEVEL_HIGH>;
+
+				phys = <&usb_phy>;
+				phy-names = "usb3-phy";
+			};
 		};
 	};
 };
diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
index 7a2304565a73..be6985775046 100644
--- a/drivers/usb/dwc3/Kconfig
+++ b/drivers/usb/dwc3/Kconfig
@@ -139,4 +139,12 @@ config USB_DWC3_QCOM
 	  for peripheral mode support.
 	  Say 'Y' or 'M' if you have one such device.
 
+config USB_DWC3_HISI
+	tristate "HiSilicon Kirin Platforms"
+	depends on ((ARCH_HISI && ARM64) || COMPILE_TEST) && OF
+	default USB_DWC3
+	help
+	  Support of USB2/3 functionality in HiSilicon Kirin platforms.
+	  Say 'Y' or 'M' if you have one such device.
+
 endif
diff --git a/drivers/usb/dwc3/Makefile b/drivers/usb/dwc3/Makefile
index ae86da0dc5bd..8f4d8440c309 100644
--- a/drivers/usb/dwc3/Makefile
+++ b/drivers/usb/dwc3/Makefile
@@ -51,3 +51,4 @@ obj-$(CONFIG_USB_DWC3_MESON_G12A)	+= dwc3-meson-g12a.o
 obj-$(CONFIG_USB_DWC3_OF_SIMPLE)	+= dwc3-of-simple.o
 obj-$(CONFIG_USB_DWC3_ST)		+= dwc3-st.o
 obj-$(CONFIG_USB_DWC3_QCOM)		+= dwc3-qcom.o
+obj-$(CONFIG_USB_DWC3_HISI)		+= dwc3-hisi.o
diff --git a/drivers/usb/dwc3/dwc3-hisi.c b/drivers/usb/dwc3/dwc3-hisi.c
new file mode 100644
index 000000000000..9b40b5ec6ead
--- /dev/null
+++ b/drivers/usb/dwc3/dwc3-hisi.c
@@ -0,0 +1,329 @@
+/**
+ * dwc3-hisi.c - Support for dwc3 platform devices on HiSilicon platforms
+ *
+ * Copyright (C) 2017-2018 Hilisicon Electronics Co., Ltd.
+ *		http://www.huawei.com
+ *
+ * Authors: Yu Chen <chenyu56@huawei.com>
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2  of
+ * the License 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/kernel.h>
+#include <linux/slab.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+#include <linux/pm_runtime.h>
+#include <linux/clk.h>
+#include <linux/clk-provider.h>
+#include <linux/regmap.h>
+#include <linux/reset.h>
+#include <linux/extcon.h>
+#include <linux/regulator/consumer.h>
+#include <linux/pinctrl/consumer.h>
+
+struct dwc3_hisi {
+	struct device		*dev;
+	struct regulator	*usb_regu;
+	struct clk		**clks;
+	int			num_clocks;
+	struct reset_control	**rstcs;
+	int			num_rstcs;
+};
+
+struct dwc3_hisi *g_dwc3_hisi;
+
+static int dwc3_hisi_init_clks(struct dwc3_hisi *dwc3_hisi,
+		struct device_node *np)
+{
+	struct device *dev = dwc3_hisi->dev;
+	int i;
+
+	dwc3_hisi->num_clocks = of_clk_get_parent_count(np);
+	if (!dwc3_hisi->num_clocks)
+		return -ENOENT;
+
+	dwc3_hisi->clks = devm_kcalloc(dev, dwc3_hisi->num_clocks,
+			sizeof(struct clk *), GFP_KERNEL);
+	if (!dwc3_hisi->clks)
+		return -ENOMEM;
+
+	for (i = 0; i < dwc3_hisi->num_clocks; i++) {
+		struct clk	*clk;
+
+		clk = of_clk_get(np, i);
+		if (IS_ERR(clk)) {
+			while (--i >= 0)
+				clk_put(dwc3_hisi->clks[i]);
+
+			return PTR_ERR(clk);
+		}
+
+		dwc3_hisi->clks[i] = clk;
+	}
+
+	return 0;
+}
+
+static int dwc3_hisi_enable_clks(struct dwc3_hisi *dwc3_hisi)
+{
+	int i;
+	int ret;
+
+	for (i = 0; i < dwc3_hisi->num_clocks; i++) {
+		ret = clk_prepare_enable(dwc3_hisi->clks[i]);
+		if (ret < 0) {
+			while (--i >= 0)
+				clk_disable_unprepare(dwc3_hisi->clks[i]);
+
+			return ret;
+		}
+	}
+
+	return 0;
+}
+
+static void dwc3_hisi_disable_clks(struct dwc3_hisi *dwc3_hisi)
+{
+	int i;
+
+	for (i = 0; i < dwc3_hisi->num_clocks; i++)
+		clk_disable_unprepare(dwc3_hisi->clks[i]);
+}
+
+static int dwc3_hisi_get_rstcs(struct dwc3_hisi *dwc3_hisi,
+		struct device_node *np)
+{
+	struct device *dev = dwc3_hisi->dev;
+	int i;
+
+	dwc3_hisi->num_rstcs = of_count_phandle_with_args(np,
+			"resets", "#reset-cells");
+	if (!dwc3_hisi->num_rstcs)
+		return -ENOENT;
+
+	dwc3_hisi->rstcs = devm_kcalloc(dev, dwc3_hisi->num_rstcs,
+			sizeof(struct reset_control *), GFP_KERNEL);
+	if (!dwc3_hisi->rstcs)
+		return -ENOMEM;
+
+	for (i = 0; i < dwc3_hisi->num_rstcs; i++) {
+		struct reset_control *rstc;
+
+		rstc = of_reset_control_get_shared_by_index(np, i);
+		if (IS_ERR(rstc)) {
+			while (--i >= 0)
+				reset_control_put(dwc3_hisi->rstcs[i]);
+			return PTR_ERR(rstc);
+		}
+
+		dwc3_hisi->rstcs[i] = rstc;
+	}
+
+	return 0;
+}
+
+static int dwc3_hisi_reset_control_assert(struct dwc3_hisi *dwc3_hisi)
+{
+	int i, ret;
+
+	for (i = dwc3_hisi->num_rstcs - 1; i >= 0 ; i--) {
+		ret = reset_control_assert(dwc3_hisi->rstcs[i]);
+		if (ret) {
+			while (--i >= 0)
+				reset_control_deassert(dwc3_hisi->rstcs[i]);
+			return ret;
+		}
+		udelay(10);
+	}
+
+	return 0;
+}
+
+static int dwc3_hisi_reset_control_deassert(struct dwc3_hisi *dwc3_hisi)
+{
+	int i, ret;
+
+	for (i = 0; i < dwc3_hisi->num_rstcs; i++) {
+		ret = reset_control_deassert(dwc3_hisi->rstcs[i]);
+		if (ret) {
+			while (--i >= 0)
+				reset_control_assert(dwc3_hisi->rstcs[i]);
+			return ret;
+		}
+		udelay(10);
+	}
+
+	return 0;
+}
+
+static int dwc3_hisi_probe(struct platform_device *pdev)
+{
+	struct dwc3_hisi	*dwc3_hisi;
+	struct device		*dev = &pdev->dev;
+	struct device_node	*np = dev->of_node;
+
+	int			ret;
+	int			i;
+
+	dwc3_hisi = devm_kzalloc(dev, sizeof(*dwc3_hisi), GFP_KERNEL);
+	if (!dwc3_hisi)
+		return -ENOMEM;
+
+	platform_set_drvdata(pdev, dwc3_hisi);
+	dwc3_hisi->dev = dev;
+
+	ret = dwc3_hisi_init_clks(dwc3_hisi, np);
+	if (ret) {
+		dev_err(dev, "could not get clocks\n");
+		return ret;
+	}
+
+	ret = dwc3_hisi_enable_clks(dwc3_hisi);
+	if (ret) {
+		dev_err(dev, "could not enable clocks\n");
+		goto err_put_clks;
+	}
+
+	ret = dwc3_hisi_get_rstcs(dwc3_hisi, np);
+	if (ret) {
+		dev_err(dev, "could not get reset controllers\n");
+		goto err_disable_clks;
+	}
+	ret = dwc3_hisi_reset_control_deassert(dwc3_hisi);
+	if (ret) {
+		dev_err(dev, "reset control deassert failed\n");
+		goto err_put_rstcs;
+	}
+
+	pm_runtime_set_active(dev);
+	pm_runtime_enable(dev);
+	pm_runtime_get_sync(dev);
+
+	ret = of_platform_populate(np, NULL, NULL, dev);
+	if (ret) {
+		dev_err(dev, "failed to add dwc3 core\n");
+		goto err_reset_assert;
+	}
+
+	g_dwc3_hisi = dwc3_hisi;
+	return 0;
+
+err_reset_assert:
+	ret = dwc3_hisi_reset_control_assert(dwc3_hisi);
+	if (ret)
+		dev_err(dev, "reset control assert failed\n");
+err_put_rstcs:
+	for (i = 0; i < dwc3_hisi->num_rstcs; i++)
+		reset_control_put(dwc3_hisi->rstcs[i]);
+err_disable_clks:
+	dwc3_hisi_disable_clks(dwc3_hisi);
+err_put_clks:
+	for (i = 0; i < dwc3_hisi->num_clocks; i++)
+		clk_put(dwc3_hisi->clks[i]);
+
+	return ret;
+}
+
+static int dwc3_hisi_remove(struct platform_device *pdev)
+{
+	struct dwc3_hisi	*dwc3_hisi = platform_get_drvdata(pdev);
+	struct device		*dev = &pdev->dev;
+	int			i, ret;
+
+	of_platform_depopulate(dev);
+
+	ret = dwc3_hisi_reset_control_assert(dwc3_hisi);
+	if (ret) {
+		dev_err(dev, "reset control assert failed\n");
+		return ret;
+	}
+
+	for (i = 0; i < dwc3_hisi->num_clocks; i++) {
+		clk_disable_unprepare(dwc3_hisi->clks[i]);
+		clk_put(dwc3_hisi->clks[i]);
+	}
+
+	pm_runtime_put_sync(dev);
+	pm_runtime_disable(dev);
+
+	return 0;
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int dwc3_hisi_suspend(struct device *dev)
+{
+	struct dwc3_hisi *dwc3_hisi = dev_get_drvdata(dev);
+	int ret;
+
+	dev_info(dev, "%s\n", __func__);
+
+	ret = dwc3_hisi_reset_control_assert(dwc3_hisi);
+	if (ret) {
+		dev_err(dev, "reset control assert failed\n");
+		return ret;
+	}
+
+	dwc3_hisi_disable_clks(dwc3_hisi);
+	pinctrl_pm_select_default_state(dev);
+	return 0;
+}
+
+static int dwc3_hisi_resume(struct device *dev)
+{
+	struct dwc3_hisi *dwc3_hisi = dev_get_drvdata(dev);
+	int ret;
+
+	dev_info(dev, "%s\n", __func__);
+	pinctrl_pm_select_default_state(dev);
+
+	ret = dwc3_hisi_enable_clks(dwc3_hisi);
+	if (ret) {
+		dev_err(dev, "could not enable clocks\n");
+		return ret;
+	}
+
+	ret = dwc3_hisi_reset_control_deassert(dwc3_hisi);
+	if (ret) {
+		dev_err(dev, "reset control deassert failed\n");
+		return ret;
+	}
+
+	return 0;
+}
+#endif /* CONFIG_PM_SLEEP */
+
+static SIMPLE_DEV_PM_OPS(dwc3_hisi_dev_pm_ops,
+		dwc3_hisi_suspend, dwc3_hisi_resume);
+
+static const struct of_device_id dwc3_hisi_match[] = {
+	{ .compatible = "hisilicon,hi3660-dwc3" },
+	{ .compatible = "hisilicon,kirin970-dwc3" },
+	{ /* sentinel */ },
+};
+
+MODULE_DEVICE_TABLE(of, dwc3_hisi_match);
+
+static struct platform_driver dwc3_hisi_driver = {
+	.probe = dwc3_hisi_probe,
+	.remove = dwc3_hisi_remove,
+	.driver = {
+		.name = "usb-hisi-dwc3",
+		.of_match_table = dwc3_hisi_match,
+		.pm = &dwc3_hisi_dev_pm_ops,
+	},
+};
+
+module_platform_driver(dwc3_hisi_driver);
+
+MODULE_AUTHOR("Yu Chen <chenyu56@huawei.com>");
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("DesignWare USB3 HiSilicon Glue Layer");



^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [RFC 00/10] Add USB support for Hikey 970
  2020-09-04 12:28   ` Mauro Carvalho Chehab
@ 2020-09-04 12:34     ` Mark Brown
  0 siblings, 0 replies; 22+ messages in thread
From: Mark Brown @ 2020-09-04 12:34 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: Robin Murphy, linuxarm, mauro.chehab, John Stultz,
	Manivannan Sadhasivam, Krzysztof Kozlowski, linux-kernel,
	Greg Kroah-Hartman, Derek Kiernan, Arnd Bergmann, Vinod Koul,
	Kishon Vijay Abraham I, David S. Miller, Rob Herring, Yu Chen,
	Rob Herring, Dragan Cvetic, linux-arm-kernel, devicetree,
	Liam Girdwood, Wei Xu

[-- Attachment #1: Type: text/plain, Size: 813 bytes --]

On Fri, Sep 04, 2020 at 02:28:49PM +0200, Mauro Carvalho Chehab wrote:

> It sounds that the only power supply related to USB is for the USB
> hub (ldo17), with sounds unlikely to affect dwc3. Yet, just in case,
> I added a dirty hack at dwc3_probe:

> <snip>
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index 25c686a752b0..e42be91fe6c2 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -1491,6 +1491,11 @@ static int dwc3_probe(struct platform_device *pdev)
>  
>  	}

> +struct regulator *reg;
> +reg = devm_regulator_get(dev, "ldo17");
> +ret = PTR_ERR_OR_ZERO(reg);
> +if (ret) return ret;

A regulator_get() is going to do absolutely nothing to the state of the
regulator, if you're trying to enable it you need to do regulator_enable()

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [RFC 09/10] misc: hisi_hikey_usb: add support for Hikey 970
  2020-09-04 12:23   ` Mark Brown
@ 2020-09-04 12:38     ` Mauro Carvalho Chehab
  2020-09-04 12:45       ` Mark Brown
  0 siblings, 1 reply; 22+ messages in thread
From: Mauro Carvalho Chehab @ 2020-09-04 12:38 UTC (permalink / raw)
  To: Mark Brown
  Cc: linuxarm, mauro.chehab, John Stultz, Manivannan Sadhasivam,
	Robin Murphy, Arnd Bergmann, Greg Kroah-Hartman, Liam Girdwood,
	linux-kernel

Em Fri, 4 Sep 2020 13:23:03 +0100
Mark Brown <broonie@kernel.org> escreveu:

> On Fri, Sep 04, 2020 at 12:23:31PM +0200, Mauro Carvalho Chehab wrote:
> 
> > +	regulator = devm_regulator_get_optional(&pdev->dev, "hub-vdd");
> > +	if (IS_ERR(regulator)) {
> > +		if (PTR_ERR(regulator) == -EPROBE_DEFER) {
> > +			dev_info(&pdev->dev,
> > +				 "waiting for hub-vdd-supply to be probed\n");
> > +			return PTR_ERR(regulator);
> > +		}
> > +
> > +		/* let it fall back to regulator dummy */
> > +		regulator = devm_regulator_get(&pdev->dev, "hub-vdd");
> > +		if (IS_ERR(regulator)) {
> > +			dev_err(&pdev->dev,
> > +				"get hub-vdd-supply failed with error %ld\n",
> > +				PTR_ERR(regulator));
> > +			return PTR_ERR(regulator);
> > +		}
> > +	}  
> 
> This seems weird - if the supply is non-optional why is the code trying
> with devm_regulator_get_optional()?  Just use normal get directly.

That's meant to avoid problems with EPROBE_DEFER.

See, Hikey 970 need to initialize 4 drivers for the regulators:
SPMI core, SPMI bus controller, MFD and regulator. This can take
some time. So, a first call to *regulator_get() may return
EPROBE_DEFER, specially if both regulator drivers and USB HUB
are builtin.

I ended doing the same as some other DRM drivers do (like adv7535).


> > +	ret = regulator_set_voltage(regulator, 3300000, 3300000);
> > +	if (ret)
> > +		dev_err(&pdev->dev, "set hub-vdd-supply voltage failed\n");  
> 
> Unless the device is actively managing the voltage at runtime it should
> just let the voltage be set by machine constraints, most of the time
> this will do nothing.  This only sets the voltage in this one place.

Ok, I'll drop this.

> > +	hub_reset_en_gpio = of_get_named_gpio(pdev->dev.of_node,
> > +					      "hub_reset_en_gpio", 0);
> > +	if (!gpio_is_valid(hub_reset_en_gpio)) {
> > +		dev_err(&pdev->dev, "Failed to get a valid reset gpio\n");
> > +		return -ENODEV;
> > +	}  
> 
> Why not just use devm_gpio_request() which already asks for the GPIO by
> name?

Makes sense.

Thanks,
Mauro

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [RFC 09/10] misc: hisi_hikey_usb: add support for Hikey 970
  2020-09-04 12:38     ` Mauro Carvalho Chehab
@ 2020-09-04 12:45       ` Mark Brown
  2020-09-04 13:47         ` Mauro Carvalho Chehab
  0 siblings, 1 reply; 22+ messages in thread
From: Mark Brown @ 2020-09-04 12:45 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: linuxarm, mauro.chehab, John Stultz, Manivannan Sadhasivam,
	Robin Murphy, Arnd Bergmann, Greg Kroah-Hartman, Liam Girdwood,
	linux-kernel

[-- Attachment #1: Type: text/plain, Size: 1676 bytes --]

On Fri, Sep 04, 2020 at 02:38:48PM +0200, Mauro Carvalho Chehab wrote:
> Em Fri, 4 Sep 2020 13:23:03 +0100
> Mark Brown <broonie@kernel.org> escreveu:
> 
> > On Fri, Sep 04, 2020 at 12:23:31PM +0200, Mauro Carvalho Chehab wrote:
> > 
> > > +	regulator = devm_regulator_get_optional(&pdev->dev, "hub-vdd");
> > > +	if (IS_ERR(regulator)) {
> > > +		if (PTR_ERR(regulator) == -EPROBE_DEFER) {
> > > +			dev_info(&pdev->dev,
> > > +				 "waiting for hub-vdd-supply to be probed\n");
> > > +			return PTR_ERR(regulator);
> > > +		}
> > > +
> > > +		/* let it fall back to regulator dummy */
> > > +		regulator = devm_regulator_get(&pdev->dev, "hub-vdd");
> > > +		if (IS_ERR(regulator)) {
> > > +			dev_err(&pdev->dev,
> > > +				"get hub-vdd-supply failed with error %ld\n",
> > > +				PTR_ERR(regulator));
> > > +			return PTR_ERR(regulator);
> > > +		}
> > > +	}  

> > This seems weird - if the supply is non-optional why is the code trying
> > with devm_regulator_get_optional()?  Just use normal get directly.

> That's meant to avoid problems with EPROBE_DEFER.

Which problems and in what way does it avoid them?

> See, Hikey 970 need to initialize 4 drivers for the regulators:
> SPMI core, SPMI bus controller, MFD and regulator. This can take
> some time. So, a first call to *regulator_get() may return
> EPROBE_DEFER, specially if both regulator drivers and USB HUB
> are builtin.

This is totally normal and works fine with normal regulator_get().

> I ended doing the same as some other DRM drivers do (like adv7535).

I can't find any references to regulator_get_optional()
drivers/gpu/drm/bridge/adv7511/adv7511_drv.c?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [RFC 08/10] misc: hisi_hikey_usb: Driver to support onboard USB gpio hub on Hikey960
  2020-09-04 10:23 ` [RFC 08/10] misc: hisi_hikey_usb: Driver to support onboard USB gpio hub on Hikey960 Mauro Carvalho Chehab
@ 2020-09-04 12:53   ` Randy Dunlap
  0 siblings, 0 replies; 22+ messages in thread
From: Randy Dunlap @ 2020-09-04 12:53 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: linuxarm, mauro.chehab, Yu Chen, John Stultz,
	Manivannan Sadhasivam, Robin Murphy, Derek Kiernan,
	Dragan Cvetic, Arnd Bergmann, Greg Kroah-Hartman,
	David S. Miller, Rob Herring, linux-kernel

On 9/4/20 3:23 AM, Mauro Carvalho Chehab wrote:
> diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
> index e1b1ba5e2b92..8ceba6153ecc 100644
> --- a/drivers/misc/Kconfig
> +++ b/drivers/misc/Kconfig
> @@ -456,6 +456,15 @@ config PVPANIC
>  	  a paravirtualized device provided by QEMU; it lets a virtual machine
>  	  (guest) communicate panic events to the host.
>  
> +config HISI_HIKEY_USB
> +	tristate "USB GPIO Hub on HiSilicon Hikey960 Platform"
> +	depends on OF && GPIOLIB

	depends on (OF && GPIOLIB) || COMPILE_TEST
?
Both of those have stubs if they are not enabled IIRC.

> +	help
> +	  If you say yes here this adds support for the on-board USB gpio hub

	                                                             GPIO

> +	  found on the HiKey960, which is necssary to support switching between

	                                  necessary

> +	  the dual-role USB-C port and the USB-A host ports using only one USB
> +	  controller.
> +
>  source "drivers/misc/c2port/Kconfig"
>  source "drivers/misc/eeprom/Kconfig"
>  source "drivers/misc/cb710/Kconfig"


thanks.
-- 
~Randy


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [RFC 09/10] misc: hisi_hikey_usb: add support for Hikey 970
  2020-09-04 12:45       ` Mark Brown
@ 2020-09-04 13:47         ` Mauro Carvalho Chehab
  0 siblings, 0 replies; 22+ messages in thread
From: Mauro Carvalho Chehab @ 2020-09-04 13:47 UTC (permalink / raw)
  To: Mark Brown
  Cc: linuxarm, mauro.chehab, John Stultz, Manivannan Sadhasivam,
	Robin Murphy, Arnd Bergmann, Greg Kroah-Hartman, Liam Girdwood,
	linux-kernel

Em Fri, 4 Sep 2020 13:45:23 +0100
Mark Brown <broonie@kernel.org> escreveu:

> On Fri, Sep 04, 2020 at 02:38:48PM +0200, Mauro Carvalho Chehab wrote:
> > Em Fri, 4 Sep 2020 13:23:03 +0100
> > Mark Brown <broonie@kernel.org> escreveu:
> >   
> > > On Fri, Sep 04, 2020 at 12:23:31PM +0200, Mauro Carvalho Chehab wrote:
> > >   
> > > > +	regulator = devm_regulator_get_optional(&pdev->dev, "hub-vdd");
> > > > +	if (IS_ERR(regulator)) {
> > > > +		if (PTR_ERR(regulator) == -EPROBE_DEFER) {
> > > > +			dev_info(&pdev->dev,
> > > > +				 "waiting for hub-vdd-supply to be probed\n");
> > > > +			return PTR_ERR(regulator);
> > > > +		}
> > > > +
> > > > +		/* let it fall back to regulator dummy */
> > > > +		regulator = devm_regulator_get(&pdev->dev, "hub-vdd");
> > > > +		if (IS_ERR(regulator)) {
> > > > +			dev_err(&pdev->dev,
> > > > +				"get hub-vdd-supply failed with error %ld\n",
> > > > +				PTR_ERR(regulator));
> > > > +			return PTR_ERR(regulator);
> > > > +		}
> > > > +	}    
> 
> > > This seems weird - if the supply is non-optional why is the code trying
> > > with devm_regulator_get_optional()?  Just use normal get directly.  
> 
> > That's meant to avoid problems with EPROBE_DEFER.  
> 
> Which problems and in what way does it avoid them?
> 
> > See, Hikey 970 need to initialize 4 drivers for the regulators:
> > SPMI core, SPMI bus controller, MFD and regulator. This can take
> > some time. So, a first call to *regulator_get() may return
> > EPROBE_DEFER, specially if both regulator drivers and USB HUB
> > are builtin.  
> 
> This is totally normal and works fine with normal regulator_get().

When I was porting the drm driver (first from OOT Kernel 4.9 to 4.19.5,
and then from 4.19.5 to 5.7), I'm pretty sure I got several of those
messages:

	"supply %s not found, using dummy regulator\n"

due to EPROBE_DEFER. I remember I ended checked the implementation
of the regulator code on that time. Yet, looking at the current 
implementation of _regulator_get():

        rdev = regulator_dev_lookup(dev, id);
        if (IS_ERR(rdev)) {
                ret = PTR_ERR(rdev);

I see that devm_regulator_get() will do the right thing with
EPROBE_DEFER.

So, I'll replace it at the driver. Thanks for the review!

Thanks,
Mauro

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [RFC 03/10] phy: hisilicon: phy-hi3670-usb3: use a consistent namespace
  2020-09-04 10:23 ` [RFC 03/10] phy: hisilicon: phy-hi3670-usb3: use a consistent namespace Mauro Carvalho Chehab
@ 2020-09-09 20:15   ` Rob Herring
  2020-09-10  5:37     ` Mauro Carvalho Chehab
  0 siblings, 1 reply; 22+ messages in thread
From: Rob Herring @ 2020-09-09 20:15 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: Linuxarm, mauro.chehab, John Stultz, Manivannan Sadhasivam,
	Robin Murphy, Kishon Vijay Abraham I, Vinod Koul, Yu Chen,
	linux-kernel, devicetree

On Fri, Sep 4, 2020 at 4:23 AM Mauro Carvalho Chehab
<mchehab+huawei@kernel.org> wrote:
>
> Rename hikey970 to hi3670, in order to use a namespace
> similar to hi3660 driver.
>
> Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
> ---
>  .../bindings/phy/phy-hi3670-usb3.txt          |  4 +-

Bindings should be a separate patch.

>  drivers/phy/hisilicon/phy-hi3670-usb3.c       | 98 +++++++++----------
>  2 files changed, 51 insertions(+), 51 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/phy/phy-hi3670-usb3.txt b/Documentation/devicetree/bindings/phy/phy-hi3670-usb3.txt
> index 4cb02612ff23..2fb27cb8beaf 100644
> --- a/Documentation/devicetree/bindings/phy/phy-hi3670-usb3.txt
> +++ b/Documentation/devicetree/bindings/phy/phy-hi3670-usb3.txt
> @@ -2,7 +2,7 @@ Hisilicon Kirin970 usb PHY
>  -----------------------
>
>  Required properties:
> -- compatible: should be "hisilicon,kirin970-usb-phy"
> +- compatible: should be "hisilicon,hi3670-usb-phy"

Unless this is unused, we can't just change it. It's an ABI.

>  - #phy-cells: must be 0
>  - hisilicon,pericrg-syscon: phandle of syscon used to control phy.
>  - hisilicon,pctrl-syscon: phandle of syscon used to control phy.
> @@ -14,7 +14,7 @@ Refer to phy/phy-bindings.txt for the generic PHY binding properties
>
>  Example:
>         usb_phy: usbphy {
> -               compatible = "hisilicon,kirin970-usb-phy";
> +               compatible = "hisilicon,hi3670-usb-phy";
>                 #phy-cells = <0>;
>                 hisilicon,pericrg-syscon = <&crg_ctrl>;
>                 hisilicon,pctrl-syscon = <&pctrl>;

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [RFC 03/10] phy: hisilicon: phy-hi3670-usb3: use a consistent namespace
  2020-09-09 20:15   ` Rob Herring
@ 2020-09-10  5:37     ` Mauro Carvalho Chehab
  2020-09-15 16:51       ` Rob Herring
  0 siblings, 1 reply; 22+ messages in thread
From: Mauro Carvalho Chehab @ 2020-09-10  5:37 UTC (permalink / raw)
  To: Rob Herring
  Cc: Linuxarm, mauro.chehab, John Stultz, Manivannan Sadhasivam,
	Robin Murphy, Kishon Vijay Abraham I, Vinod Koul, Yu Chen,
	linux-kernel, devicetree

Em Wed, 9 Sep 2020 14:15:59 -0600
Rob Herring <robh+dt@kernel.org> escreveu:

> On Fri, Sep 4, 2020 at 4:23 AM Mauro Carvalho Chehab
> <mchehab+huawei@kernel.org> wrote:
> >
> > Rename hikey970 to hi3670, in order to use a namespace
> > similar to hi3660 driver.
> >
> > Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
> > ---
> >  .../bindings/phy/phy-hi3670-usb3.txt          |  4 +-  
> 
> Bindings should be a separate patch.

Ok. I'll split it.

> 
> >  drivers/phy/hisilicon/phy-hi3670-usb3.c       | 98 +++++++++----------
> >  2 files changed, 51 insertions(+), 51 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/phy/phy-hi3670-usb3.txt b/Documentation/devicetree/bindings/phy/phy-hi3670-usb3.txt
> > index 4cb02612ff23..2fb27cb8beaf 100644
> > --- a/Documentation/devicetree/bindings/phy/phy-hi3670-usb3.txt
> > +++ b/Documentation/devicetree/bindings/phy/phy-hi3670-usb3.txt
> > @@ -2,7 +2,7 @@ Hisilicon Kirin970 usb PHY
> >  -----------------------
> >
> >  Required properties:
> > -- compatible: should be "hisilicon,kirin970-usb-phy"
> > +- compatible: should be "hisilicon,hi3670-usb-phy"  
> 
> Unless this is unused, we can't just change it. It's an ABI.

From upstream PoV, this binding is for a new driver that will be added
via this patchset. 
> 
> >  - #phy-cells: must be 0
> >  - hisilicon,pericrg-syscon: phandle of syscon used to control phy.
> >  - hisilicon,pctrl-syscon: phandle of syscon used to control phy.
> > @@ -14,7 +14,7 @@ Refer to phy/phy-bindings.txt for the generic PHY binding properties
> >
> >  Example:
> >         usb_phy: usbphy {
> > -               compatible = "hisilicon,kirin970-usb-phy";
> > +               compatible = "hisilicon,hi3670-usb-phy";
> >                 #phy-cells = <0>;
> >                 hisilicon,pericrg-syscon = <&crg_ctrl>;
> >                 hisilicon,pctrl-syscon = <&pctrl>;  



Thanks,
Mauro

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [RFC 03/10] phy: hisilicon: phy-hi3670-usb3: use a consistent namespace
  2020-09-10  5:37     ` Mauro Carvalho Chehab
@ 2020-09-15 16:51       ` Rob Herring
  0 siblings, 0 replies; 22+ messages in thread
From: Rob Herring @ 2020-09-15 16:51 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: Linuxarm, mauro.chehab, John Stultz, Manivannan Sadhasivam,
	Robin Murphy, Kishon Vijay Abraham I, Vinod Koul, Yu Chen,
	linux-kernel, devicetree

On Wed, Sep 9, 2020 at 11:37 PM Mauro Carvalho Chehab
<mchehab+huawei@kernel.org> wrote:
>
> Em Wed, 9 Sep 2020 14:15:59 -0600
> Rob Herring <robh+dt@kernel.org> escreveu:
>
> > On Fri, Sep 4, 2020 at 4:23 AM Mauro Carvalho Chehab
> > <mchehab+huawei@kernel.org> wrote:
> > >
> > > Rename hikey970 to hi3670, in order to use a namespace
> > > similar to hi3660 driver.
> > >
> > > Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
> > > ---
> > >  .../bindings/phy/phy-hi3670-usb3.txt          |  4 +-
> >
> > Bindings should be a separate patch.
>
> Ok. I'll split it.
>
> >
> > >  drivers/phy/hisilicon/phy-hi3670-usb3.c       | 98 +++++++++----------
> > >  2 files changed, 51 insertions(+), 51 deletions(-)
> > >
> > > diff --git a/Documentation/devicetree/bindings/phy/phy-hi3670-usb3.txt b/Documentation/devicetree/bindings/phy/phy-hi3670-usb3.txt
> > > index 4cb02612ff23..2fb27cb8beaf 100644
> > > --- a/Documentation/devicetree/bindings/phy/phy-hi3670-usb3.txt
> > > +++ b/Documentation/devicetree/bindings/phy/phy-hi3670-usb3.txt
> > > @@ -2,7 +2,7 @@ Hisilicon Kirin970 usb PHY
> > >  -----------------------
> > >
> > >  Required properties:
> > > -- compatible: should be "hisilicon,kirin970-usb-phy"
> > > +- compatible: should be "hisilicon,hi3670-usb-phy"
> >
> > Unless this is unused, we can't just change it. It's an ABI.
>
> From upstream PoV, this binding is for a new driver that will be added
> via this patchset.

Please squash this change then.

Rob

^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2020-09-15 17:40 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-04 10:23 [RFC 00/10] Add USB support for Hikey 970 Mauro Carvalho Chehab
2020-09-04 10:23 ` [RFC 01/10] phy: hisilicon: add USB physical layer for Kirin 3670 Mauro Carvalho Chehab
2020-09-04 10:23 ` [RFC 02/10] phy: hisilicon: phy-hi3670-usb3: fix some issues at the init code Mauro Carvalho Chehab
2020-09-04 10:23 ` [RFC 03/10] phy: hisilicon: phy-hi3670-usb3: use a consistent namespace Mauro Carvalho Chehab
2020-09-09 20:15   ` Rob Herring
2020-09-10  5:37     ` Mauro Carvalho Chehab
2020-09-15 16:51       ` Rob Herring
2020-09-04 10:23 ` [RFC 04/10] phy: hisilicon: phy-hi3670-usb3: fix coding style Mauro Carvalho Chehab
2020-09-04 10:23 ` [RFC 05/10] phy: hisilicon: phy-hi3670-usb3: change some DT properties Mauro Carvalho Chehab
2020-09-04 10:23 ` [RFC 06/10] dt-bindings: phy: convert phy-kirin970-usb3.txt to yaml Mauro Carvalho Chehab
2020-09-04 10:23 ` [RFC 07/10] MAINTAINERS: add myself as maintainer for Kirin 970 USB PHY Mauro Carvalho Chehab
2020-09-04 10:23 ` [RFC 08/10] misc: hisi_hikey_usb: Driver to support onboard USB gpio hub on Hikey960 Mauro Carvalho Chehab
2020-09-04 12:53   ` Randy Dunlap
2020-09-04 10:23 ` [RFC 09/10] misc: hisi_hikey_usb: add support for Hikey 970 Mauro Carvalho Chehab
2020-09-04 12:23   ` Mark Brown
2020-09-04 12:38     ` Mauro Carvalho Chehab
2020-09-04 12:45       ` Mark Brown
2020-09-04 13:47         ` Mauro Carvalho Chehab
2020-09-04 10:23 ` [RFC 10/10] dts: hisilicon: add support for USB3 on " Mauro Carvalho Chehab
2020-09-04 10:53 ` [RFC 00/10] Add USB support for " Robin Murphy
2020-09-04 12:28   ` Mauro Carvalho Chehab
2020-09-04 12:34     ` Mark Brown

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).