Em Fri, 4 Sep 2020 11:53:10 +0100 Robin Murphy <robin.murphy@xxxxxxx> 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@xxxxxxxxxx> + * + * 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@xxxxxxxxxx>"); +MODULE_LICENSE("GPL v2"); +MODULE_DESCRIPTION("DesignWare USB3 HiSilicon Glue Layer");
