This patch series documents the Xilinx Versal DWC3 controller. This also
adds a new Xilinx specific driver for adding new features in the future.
Changes in v2:
- Addressed review comments from v1
- merged normal and runtime suspend resume functions as they are
same
- Improved description of some register operations to avoid
confusion
- Updated commit log for patch 2/2 for better clarity.
Changes in v3:
- Removed snps,enable-hibernation property from the devicetree
binding.
Changes in v4:
- Error checking added for devm_phy_get
- Documented resets in dt-bindings
Manish Narani (2):
dt-bindings: usb: dwc3-xilinx: Add documentation for Versal DWC3
Controller
usb: dwc3: Add driver for Xilinx platforms
.../devicetree/bindings/usb/dwc3-xilinx.txt | 28 +-
drivers/usb/dwc3/Kconfig | 9 +
drivers/usb/dwc3/Makefile | 1 +
drivers/usb/dwc3/dwc3-of-simple.c | 1 -
drivers/usb/dwc3/dwc3-xilinx.c | 339 +++++++++++++++++++++
5 files changed, 375 insertions(+), 3 deletions(-)
create mode 100644 drivers/usb/dwc3/dwc3-xilinx.c
--
2.1.1
Add a new driver for supporting Xilinx platforms. This driver is used
for some sequence of operations required for Xilinx USB controllers.
This driver is also used to choose between PIPE clock coming from SerDes
and the Suspend Clock. Before the controller is out of reset, the clock
selection should be changed to PIPE clock in order to make the USB
controller work. There is a register added in Xilinx USB controller
register space for the same.
Signed-off-by: Manish Narani <[email protected]>
---
drivers/usb/dwc3/Kconfig | 9 +
drivers/usb/dwc3/Makefile | 1 +
drivers/usb/dwc3/dwc3-of-simple.c | 1 -
drivers/usb/dwc3/dwc3-xilinx.c | 339 ++++++++++++++++++++++++++++++++++++++
4 files changed, 349 insertions(+), 1 deletion(-)
create mode 100644 drivers/usb/dwc3/dwc3-xilinx.c
diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
index 2133acf..66b1454 100644
--- a/drivers/usb/dwc3/Kconfig
+++ b/drivers/usb/dwc3/Kconfig
@@ -149,4 +149,13 @@ config USB_DWC3_IMX8MP
functionality.
Say 'Y' or 'M' if you have one such device.
+config USB_DWC3_XILINX
+ tristate "Xilinx Platforms"
+ depends on (ARCH_ZYNQMP || ARCH_VERSAL) && OF
+ default USB_DWC3
+ help
+ Support Xilinx SoCs with DesignWare Core USB3 IP.
+ This driver handles both ZynqMP and Versal SoC operations.
+ Say 'Y' or 'M' if you have one such device.
+
endif
diff --git a/drivers/usb/dwc3/Makefile b/drivers/usb/dwc3/Makefile
index 2259f88..2d499de 100644
--- a/drivers/usb/dwc3/Makefile
+++ b/drivers/usb/dwc3/Makefile
@@ -52,3 +52,4 @@ 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_IMX8MP) += dwc3-imx8mp.o
+obj-$(CONFIG_USB_DWC3_XILINX) += dwc3-xilinx.o
diff --git a/drivers/usb/dwc3/dwc3-of-simple.c b/drivers/usb/dwc3/dwc3-of-simple.c
index e62ecd2..71fd620 100644
--- a/drivers/usb/dwc3/dwc3-of-simple.c
+++ b/drivers/usb/dwc3/dwc3-of-simple.c
@@ -172,7 +172,6 @@ static const struct dev_pm_ops dwc3_of_simple_dev_pm_ops = {
static const struct of_device_id of_dwc3_simple_match[] = {
{ .compatible = "rockchip,rk3399-dwc3" },
- { .compatible = "xlnx,zynqmp-dwc3" },
{ .compatible = "cavium,octeon-7130-usb-uctl" },
{ .compatible = "sprd,sc9860-dwc3" },
{ .compatible = "allwinner,sun50i-h6-dwc3" },
diff --git a/drivers/usb/dwc3/dwc3-xilinx.c b/drivers/usb/dwc3/dwc3-xilinx.c
new file mode 100644
index 0000000..a59e149
--- /dev/null
+++ b/drivers/usb/dwc3/dwc3-xilinx.c
@@ -0,0 +1,339 @@
+// SPDX-License-Identifier: GPL-2.0
+/**
+ * dwc3-xilinx.c - Xilinx DWC3 controller specific glue driver
+ *
+ * Authors: Manish Narani <[email protected]>
+ * Anurag Kumar Vulisha <[email protected]>
+ */
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/slab.h>
+#include <linux/clk.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/dma-mapping.h>
+#include <linux/of_platform.h>
+#include <linux/pm_runtime.h>
+#include <linux/reset.h>
+#include <linux/of_address.h>
+#include <linux/delay.h>
+#include <linux/firmware/xlnx-zynqmp.h>
+#include <linux/io.h>
+
+#include <linux/phy/phy.h>
+
+/* USB phy reset mask register */
+#define XLNX_USB_PHY_RST_EN 0x001C
+#define XLNX_PHY_RST_MASK 0x1
+
+/* Xilinx USB 3.0 IP Register */
+#define XLNX_USB_TRAFFIC_ROUTE_CONFIG 0x005C
+#define XLNX_USB_TRAFFIC_ROUTE_FPD 0x1
+
+/* Versal USB Reset ID */
+#define VERSAL_USB_RESET_ID 0xC104036
+
+#define XLNX_USB_FPD_PIPE_CLK 0x7c
+#define PIPE_CLK_DESELECT 1
+#define PIPE_CLK_SELECT 0
+#define XLNX_USB_FPD_POWER_PRSNT 0x80
+#define FPD_POWER_PRSNT_OPTION BIT(0)
+
+struct dwc3_xlnx {
+ int num_clocks;
+ struct clk_bulk_data *clks;
+ struct device *dev;
+ void __iomem *regs;
+ int (*pltfm_init)(struct dwc3_xlnx *data);
+};
+
+static void dwc3_xlnx_mask_phy_rst(struct dwc3_xlnx *priv_data, bool mask)
+{
+ u32 reg;
+
+ /*
+ * Enable or disable ULPI PHY reset from USB Controller.
+ * This does not actually reset the phy, but just controls
+ * whether USB controller can or cannot reset ULPI PHY.
+ */
+ reg = readl(priv_data->regs + XLNX_USB_PHY_RST_EN);
+
+ if (mask)
+ reg &= ~XLNX_PHY_RST_MASK;
+ else
+ reg |= XLNX_PHY_RST_MASK;
+
+ writel(reg, priv_data->regs + XLNX_USB_PHY_RST_EN);
+}
+
+static int dwc3_xlnx_init_versal(struct dwc3_xlnx *priv_data)
+{
+ struct device *dev = priv_data->dev;
+ int ret;
+
+ dwc3_xlnx_mask_phy_rst(priv_data, false);
+
+ /* Assert and De-assert reset */
+ ret = zynqmp_pm_reset_assert(VERSAL_USB_RESET_ID,
+ PM_RESET_ACTION_ASSERT);
+ if (ret < 0) {
+ dev_err_probe(dev, ret, "failed to assert Reset\n");
+ return ret;
+ }
+
+ ret = zynqmp_pm_reset_assert(VERSAL_USB_RESET_ID,
+ PM_RESET_ACTION_RELEASE);
+ if (ret < 0) {
+ dev_err_probe(dev, ret, "failed to De-assert Reset\n");
+ return ret;
+ }
+
+ dwc3_xlnx_mask_phy_rst(priv_data, true);
+
+ return 0;
+}
+
+static int dwc3_xlnx_init_zynqmp(struct dwc3_xlnx *priv_data)
+{
+ struct device *dev = priv_data->dev;
+ struct reset_control *crst, *hibrst, *apbrst;
+ struct phy *usb3_phy;
+ int ret;
+ u32 reg;
+
+ usb3_phy = devm_phy_get(dev, "usb3-phy");
+ if (PTR_ERR(usb3_phy) == -EPROBE_DEFER) {
+ ret = -EPROBE_DEFER;
+ goto err;
+ } else if (IS_ERR(usb3_phy)) {
+ usb3_phy = NULL;
+ }
+
+ crst = devm_reset_control_get_exclusive(dev, "usb_crst");
+ if (IS_ERR(crst)) {
+ ret = PTR_ERR(crst);
+ dev_err_probe(dev, ret,
+ "failed to get core reset signal\n");
+ goto err;
+ }
+
+ hibrst = devm_reset_control_get_exclusive(dev, "usb_hibrst");
+ if (IS_ERR(hibrst)) {
+ ret = PTR_ERR(hibrst);
+ dev_err_probe(dev, ret,
+ "failed to get hibernation reset signal\n");
+ goto err;
+ }
+
+ apbrst = devm_reset_control_get_exclusive(dev, "usb_apbrst");
+ if (IS_ERR(apbrst)) {
+ ret = PTR_ERR(apbrst);
+ dev_err_probe(dev, ret,
+ "failed to get APB reset signal\n");
+ goto err;
+ }
+
+ ret = reset_control_assert(crst);
+ if (ret < 0) {
+ dev_err(dev, "Failed to assert core reset\n");
+ goto err;
+ }
+
+ ret = reset_control_assert(hibrst);
+ if (ret < 0) {
+ dev_err(dev, "Failed to assert hibernation reset\n");
+ goto err;
+ }
+
+ ret = reset_control_assert(apbrst);
+ if (ret < 0) {
+ dev_err(dev, "Failed to assert APB reset\n");
+ goto err;
+ }
+
+ ret = phy_init(usb3_phy);
+ if (ret < 0) {
+ phy_exit(usb3_phy);
+ goto err;
+ }
+
+ ret = reset_control_deassert(apbrst);
+ if (ret < 0) {
+ dev_err(dev, "Failed to release APB reset\n");
+ goto err;
+ }
+
+ /* Set PIPE Power Present signal in FPD Power Present Register*/
+ writel(FPD_POWER_PRSNT_OPTION, priv_data->regs + XLNX_USB_FPD_POWER_PRSNT);
+
+ /* Set the PIPE Clock Select bit in FPD PIPE Clock register */
+ writel(PIPE_CLK_SELECT, priv_data->regs + XLNX_USB_FPD_PIPE_CLK);
+
+ ret = reset_control_deassert(crst);
+ if (ret < 0) {
+ dev_err(dev, "Failed to release core reset\n");
+ goto err;
+ }
+
+ ret = reset_control_deassert(hibrst);
+ if (ret < 0) {
+ dev_err(dev, "Failed to release hibernation reset\n");
+ goto err;
+ }
+
+ ret = phy_power_on(usb3_phy);
+ if (ret < 0) {
+ phy_exit(usb3_phy);
+ goto err;
+ }
+
+ /*
+ * This routes the USB DMA traffic to go through FPD path instead
+ * of reaching DDR directly. This traffic routing is needed to
+ * make SMMU and CCI work with USB DMA.
+ */
+ if (of_dma_is_coherent(dev->of_node) || device_iommu_mapped(dev)) {
+ reg = readl(priv_data->regs + XLNX_USB_TRAFFIC_ROUTE_CONFIG);
+ reg |= XLNX_USB_TRAFFIC_ROUTE_FPD;
+ writel(reg, priv_data->regs + XLNX_USB_TRAFFIC_ROUTE_CONFIG);
+ }
+
+err:
+ return ret;
+}
+
+static const struct of_device_id dwc3_xlnx_of_match[] = {
+ {
+ .compatible = "xlnx,zynqmp-dwc3",
+ .data = &dwc3_xlnx_init_zynqmp,
+ },
+ {
+ .compatible = "xlnx,versal-dwc3",
+ .data = &dwc3_xlnx_init_versal,
+ },
+ { /* Sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, dwc3_xlnx_of_match);
+
+static int dwc3_xlnx_probe(struct platform_device *pdev)
+{
+ struct dwc3_xlnx *priv_data;
+ struct device *dev = &pdev->dev;
+ struct device_node *np = dev->of_node;
+ const struct of_device_id *match;
+ void __iomem *regs;
+ int ret;
+
+ priv_data = devm_kzalloc(dev, sizeof(*priv_data), GFP_KERNEL);
+ if (!priv_data)
+ return -ENOMEM;
+
+ regs = devm_platform_ioremap_resource(pdev, 0);
+ if (IS_ERR(regs)) {
+ ret = PTR_ERR(regs);
+ dev_err_probe(dev, ret, "failed to map registers\n");
+ return ret;
+ }
+
+ match = of_match_node(dwc3_xlnx_of_match, pdev->dev.of_node);
+
+ priv_data->pltfm_init = match->data;
+ priv_data->regs = regs;
+ priv_data->dev = dev;
+
+ platform_set_drvdata(pdev, priv_data);
+
+ ret = devm_clk_bulk_get_all(priv_data->dev, &priv_data->clks);
+ if (ret < 0)
+ return ret;
+
+ priv_data->num_clocks = ret;
+
+ ret = clk_bulk_prepare_enable(priv_data->num_clocks, priv_data->clks);
+ if (ret)
+ return ret;
+
+ ret = priv_data->pltfm_init(priv_data);
+ if (ret)
+ goto err_clk_put;
+
+ ret = of_platform_populate(np, NULL, NULL, dev);
+ if (ret)
+ goto err_clk_put;
+
+ pm_runtime_set_active(dev);
+ pm_runtime_enable(dev);
+ pm_suspend_ignore_children(dev, false);
+ pm_runtime_get_sync(dev);
+
+ return 0;
+
+err_clk_put:
+ clk_bulk_disable_unprepare(priv_data->num_clocks, priv_data->clks);
+ clk_bulk_put_all(priv_data->num_clocks, priv_data->clks);
+
+ return ret;
+}
+
+static int dwc3_xlnx_remove(struct platform_device *pdev)
+{
+ struct dwc3_xlnx *priv_data = platform_get_drvdata(pdev);
+ struct device *dev = &pdev->dev;
+
+ of_platform_depopulate(dev);
+
+ clk_bulk_disable_unprepare(priv_data->num_clocks, priv_data->clks);
+ clk_bulk_put_all(priv_data->num_clocks, priv_data->clks);
+ priv_data->num_clocks = 0;
+
+ pm_runtime_disable(dev);
+ pm_runtime_put_noidle(dev);
+ pm_runtime_set_suspended(dev);
+
+ return 0;
+}
+
+static int __maybe_unused dwc3_xlnx_suspend_common(struct device *dev)
+{
+ struct dwc3_xlnx *priv_data = dev_get_drvdata(dev);
+
+ clk_bulk_disable(priv_data->num_clocks, priv_data->clks);
+
+ return 0;
+}
+
+static int __maybe_unused dwc3_xlnx_resume_common(struct device *dev)
+{
+ struct dwc3_xlnx *priv_data = dev_get_drvdata(dev);
+
+ return clk_bulk_enable(priv_data->num_clocks, priv_data->clks);
+}
+
+static int __maybe_unused dwc3_xlnx_runtime_idle(struct device *dev)
+{
+ pm_runtime_mark_last_busy(dev);
+ pm_runtime_autosuspend(dev);
+
+ return 0;
+}
+
+static UNIVERSAL_DEV_PM_OPS(dwc3_xlnx_dev_pm_ops, dwc3_xlnx_suspend_common,
+ dwc3_xlnx_resume_common, dwc3_xlnx_runtime_idle);
+
+static struct platform_driver dwc3_xlnx_driver = {
+ .probe = dwc3_xlnx_probe,
+ .remove = dwc3_xlnx_remove,
+ .driver = {
+ .name = "dwc3-xilinx",
+ .of_match_table = dwc3_xlnx_of_match,
+ .pm = &dwc3_xlnx_dev_pm_ops,
+ },
+};
+
+module_platform_driver(dwc3_xlnx_driver);
+
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("Xilinx DWC3 controller specific glue driver");
+MODULE_AUTHOR("Manish Narani <[email protected]>");
+MODULE_AUTHOR("Anurag Kumar Vulisha <[email protected]>");
--
2.1.1
Add documentation for Versal DWC3 controller. Add required property
'reg' for the same. Also add optional properties for snps,dwc3.
Signed-off-by: Manish Narani <[email protected]>
Reviewed-by: Rob Herring <[email protected]>
---
.../devicetree/bindings/usb/dwc3-xilinx.txt | 28 ++++++++++++++++++++--
1 file changed, 26 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/usb/dwc3-xilinx.txt b/Documentation/devicetree/bindings/usb/dwc3-xilinx.txt
index a668f43..04813a4 100644
--- a/Documentation/devicetree/bindings/usb/dwc3-xilinx.txt
+++ b/Documentation/devicetree/bindings/usb/dwc3-xilinx.txt
@@ -1,32 +1,56 @@
Xilinx SuperSpeed DWC3 USB SoC controller
Required properties:
-- compatible: Should contain "xlnx,zynqmp-dwc3"
+- compatible: May contain "xlnx,zynqmp-dwc3" or "xlnx,versal-dwc3"
+- reg: Base address and length of the register control block
- clocks: A list of phandles for the clocks listed in clock-names
- clock-names: Should contain the following:
"bus_clk" Master/Core clock, have to be >= 125 MHz for SS
operation and >= 60MHz for HS operation
"ref_clk" Clock source to core during PHY power down
+- resets: A list of phandles for resets listed in reset-names
+- reset-names:
+ "usb_crst" USB core reset
+ "usb_hibrst" USB hibernation reset
+ "usb_apbrst" USB APB reset
Required child node:
A child node must exist to represent the core DWC3 IP block. The name of
the node is not important. The content of the node is defined in dwc3.txt.
+Optional properties for snps,dwc3:
+- dma-coherent: Enable this flag if CCI is enabled in design. Adding this
+ flag configures Global SoC bus Configuration Register and
+ Xilinx USB 3.0 IP - USB coherency register to enable CCI.
+- interrupt-names: Should contain the following:
+ "dwc_usb3" USB gadget mode interrupts
+ "otg" USB OTG mode interrupts
+ "hiber" USB hibernation interrupts
+
Example device node:
usb@0 {
#address-cells = <0x2>;
#size-cells = <0x1>;
compatible = "xlnx,zynqmp-dwc3";
+ reg = <0x0 0xff9d0000 0x0 0x100>;
clock-names = "bus_clk", "ref_clk";
clocks = <&clk125>, <&clk125>;
+ resets = <&zynqmp_reset ZYNQMP_RESET_USB1_CORERESET>,
+ <&zynqmp_reset ZYNQMP_RESET_USB1_HIBERRESET>,
+ <&zynqmp_reset ZYNQMP_RESET_USB1_APB>;
+ reset-names = "usb_crst", "usb_hibrst", "usb_apbrst";
ranges;
dwc3@fe200000 {
compatible = "snps,dwc3";
reg = <0x0 0xfe200000 0x40000>;
- interrupts = <0x0 0x41 0x4>;
+ interrupt-names = "dwc_usb3", "otg", "hiber";
+ interrupts = <0 65 4>, <0 69 4>, <0 75 4>;
+ phys = <&psgtr 2 PHY_TYPE_USB3 0 2>;
+ phy-names = "usb3-phy";
dr_mode = "host";
+ dma-coherent;
};
};
--
2.1.1
Hi Manish,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on usb/usb-testing]
[also build test WARNING on robh/for-next v5.12-rc3 next-20210316]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Manish-Narani/Add-a-separate-DWC3-OF-driver-for-Xilinx-platforms/20210317-145425
base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
config: arm64-allyesconfig (attached as .config)
compiler: aarch64-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/def409fdf931cd77f4a88812570ea6f38f4053d8
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Manish-Narani/Add-a-separate-DWC3-OF-driver-for-Xilinx-platforms/20210317-145425
git checkout def409fdf931cd77f4a88812570ea6f38f4053d8
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=arm64
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>
All warnings (new ones prefixed by >>):
>> drivers/usb/dwc3/dwc3-xilinx.c:27: warning: expecting prototype for dwc3(). Prototype was for XLNX_USB_PHY_RST_EN() instead
vim +27 drivers/usb/dwc3/dwc3-xilinx.c
25
26 /* USB phy reset mask register */
> 27 #define XLNX_USB_PHY_RST_EN 0x001C
28 #define XLNX_PHY_RST_MASK 0x1
29
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]
On Wed, Mar 17, 2021 at 05:50:22PM +0800, kernel test robot wrote:
> Hi Manish,
>
> Thank you for the patch! Perhaps something to improve:
>
> [auto build test WARNING on usb/usb-testing]
> [also build test WARNING on robh/for-next v5.12-rc3 next-20210316]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch]
>
> url: https://github.com/0day-ci/linux/commits/Manish-Narani/Add-a-separate-DWC3-OF-driver-for-Xilinx-platforms/20210317-145425
> base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
> config: arm64-allyesconfig (attached as .config)
> compiler: aarch64-linux-gcc (GCC) 9.3.0
> reproduce (this is a W=1 build):
> wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> chmod +x ~/bin/make.cross
> # https://github.com/0day-ci/linux/commit/def409fdf931cd77f4a88812570ea6f38f4053d8
> git remote add linux-review https://github.com/0day-ci/linux
> git fetch --no-tags linux-review Manish-Narani/Add-a-separate-DWC3-OF-driver-for-Xilinx-platforms/20210317-145425
> git checkout def409fdf931cd77f4a88812570ea6f38f4053d8
> # save the attached .config to linux build tree
> COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=arm64
>
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <[email protected]>
>
> All warnings (new ones prefixed by >>):
>
> >> drivers/usb/dwc3/dwc3-xilinx.c:27: warning: expecting prototype for dwc3(). Prototype was for XLNX_USB_PHY_RST_EN() instead
>
>
> vim +27 drivers/usb/dwc3/dwc3-xilinx.c
>
> 25
> 26 /* USB phy reset mask register */
> > 27 #define XLNX_USB_PHY_RST_EN 0x001C
> 28 #define XLNX_PHY_RST_MASK 0x1
> 29
I do not understand this warning message. What is it trying to say?
confused,
greg k-h
On 3/23/21 7:47 PM, Greg KH wrote:
> On Wed, Mar 17, 2021 at 05:50:22PM +0800, kernel test robot wrote:
>> Hi Manish,
>>
>> Thank you for the patch! Perhaps something to improve:
>>
>> [auto build test WARNING on usb/usb-testing]
>> [also build test WARNING on robh/for-next v5.12-rc3 next-20210316]
>> [If your patch is applied to the wrong git tree, kindly drop us a note.
>> And when submitting patch, we suggest to use '--base' as documented in
>> https://git-scm.com/docs/git-format-patch]
>>
>> url: https://github.com/0day-ci/linux/commits/Manish-Narani/Add-a-separate-DWC3-OF-driver-for-Xilinx-platforms/20210317-145425
>> base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
>> config: arm64-allyesconfig (attached as .config)
>> compiler: aarch64-linux-gcc (GCC) 9.3.0
>> reproduce (this is a W=1 build):
>> wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
>> chmod +x ~/bin/make.cross
>> # https://github.com/0day-ci/linux/commit/def409fdf931cd77f4a88812570ea6f38f4053d8
>> git remote add linux-review https://github.com/0day-ci/linux
>> git fetch --no-tags linux-review Manish-Narani/Add-a-separate-DWC3-OF-driver-for-Xilinx-platforms/20210317-145425
>> git checkout def409fdf931cd77f4a88812570ea6f38f4053d8
>> # save the attached .config to linux build tree
>> COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=arm64
>>
>> If you fix the issue, kindly add following tag as appropriate
>> Reported-by: kernel test robot <[email protected]>
>>
>> All warnings (new ones prefixed by >>):
>>
>>>> drivers/usb/dwc3/dwc3-xilinx.c:27: warning: expecting prototype for dwc3(). Prototype was for XLNX_USB_PHY_RST_EN() instead
>>
>> vim +27 drivers/usb/dwc3/dwc3-xilinx.c
>>
>> 25
>> 26 /* USB phy reset mask register */
>> > 27 #define XLNX_USB_PHY_RST_EN 0x001C
>> 28 #define XLNX_PHY_RST_MASK 0x1
>> 29
> I do not understand this warning message. What is it trying to say?
Hi Greg,
It's a kernel-doc warning:
$ ./scripts/kernel-doc -none drivers/usb/dwc3/dwc3-xilinx.c
drivers/usb/dwc3/dwc3-xilinx.c:27: warning: expecting prototype for
dwc3(). Prototype was for XLNX_USB_PHY_RST_EN() instead
the root cause is that there's a redundant symbol ( * ) at the beginning:
diff --git a/drivers/usb/dwc3/dwc3-xilinx.c b/drivers/usb/dwc3/dwc3-xilinx.c
index a59e1494b1a0..f42f4cbffab0 100644
--- a/drivers/usb/dwc3/dwc3-xilinx.c
+++ b/drivers/usb/dwc3/dwc3-xilinx.c
@@ -1,5 +1,5 @@
// SPDX-License-Identifier: GPL-2.0
-/**
+/*
* dwc3-xilinx.c - Xilinx DWC3 controller specific glue driver
*
* Authors: Manish Narani <[email protected]>
Best Regards,
Rong Chen
>
> confused,
>
> greg k-h
>
On Wed, Mar 17, 2021 at 12:22:29PM +0530, Manish Narani wrote:
> Add a new driver for supporting Xilinx platforms. This driver is used
> for some sequence of operations required for Xilinx USB controllers.
> This driver is also used to choose between PIPE clock coming from SerDes
> and the Suspend Clock. Before the controller is out of reset, the clock
> selection should be changed to PIPE clock in order to make the USB
> controller work. There is a register added in Xilinx USB controller
> register space for the same.
>
> Signed-off-by: Manish Narani <[email protected]>
Trying this driver with qemu (v6.0.0-rc2) results in:
[ 15.184242] dwc3-xilinx ff9d0000.usb: error -ENODEV: failed to assert Reset
[ 15.185754] Unable to handle kernel paging request at virtual address 006b6b6b6b6b6b9b
[ 15.185994] Mem abort info:
[ 15.186065] ESR = 0x96000004
[ 15.186317] EC = 0x25: DABT (current EL), IL = 32 bits
[ 15.186414] SET = 0, FnV = 0
[ 15.186498] EA = 0, S1PTW = 0
[ 15.186579] Data abort info:
[ 15.186666] ISV = 0, ISS = 0x00000004
[ 15.186756] CM = 0, WnR = 0
[ 15.186887] [006b6b6b6b6b6b9b] address between user and kernel address ranges
[ 15.187436] Internal error: Oops: 96000004 [#1] PREEMPT SMP
[ 15.187777] Modules linked in:
[ 15.188060] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.12.0-rc6-next-20210406-00006-g05407f068fc9-dirty #1
[ 15.188265] Hardware name: Xilinx Versal Virtual development board (DT)
[ 15.188495] pstate: 80000005 (Nzcv daif -PAN -UAO -TCO BTYPE=--)
[ 15.188614] pc : __clk_put+0x24/0x138
[ 15.188716] lr : __clk_put+0x24/0x138
[ 15.188791] sp : ffff80001326bac0
[ 15.188853] x29: ffff80001326bac0 x28: ffff00000644ed00
[ 15.188982] x27: ffff00000421ecd0 x26: ffff00000421e810
[ 15.189076] x25: ffff00000644f100 x24: 0000000000000000
[ 15.189170] x23: ffff8000126a2570 x22: 0000000000000005
[ 15.189271] x21: ffff00000644ed00 x20: ffff000006449970
[ 15.189367] x19: 6b6b6b6b6b6b6b6b x18: 0000000000000010
[ 15.189456] x17: 0000000000000001 x16: 0000000000000000
[ 15.189546] x15: ffff000003af0490 x14: 00000000000001b7
[ 15.189642] x13: ffff000003af0490 x12: 00000000ffffffea
[ 15.189729] x11: ffff8000123b6460 x10: 0000000000000080
[ 15.189815] x9 : 00000000676993c6 x8 : 00000000676993c6
[ 15.189941] x7 : 000000007d152ab3 x6 : ffff800012768480
[ 15.190047] x5 : 0000000000000000 x4 : 000000007f97631e
[ 15.190139] x3 : 00000000d5bdf2c2 x2 : 000000000000000b
[ 15.190233] x1 : ffff000003af0040 x0 : 0000000000000001
[ 15.190432] Call trace:
[ 15.190506] __clk_put+0x24/0x138
[ 15.190588] clk_put+0x10/0x20
[ 15.190653] clk_bulk_put+0x3c/0x60
[ 15.190724] devm_clk_bulk_release+0x1c/0x28
[ 15.190806] release_nodes+0x1c0/0x2b0
[ 15.190887] devres_release_all+0x38/0x60
[ 15.190963] really_probe+0x1e4/0x3a8
[ 15.191042] driver_probe_device+0x64/0xc8
...
because of ...
> +
> + ret = devm_clk_bulk_get_all(priv_data->dev, &priv_data->clks);
> + if (ret < 0)
> + return ret;
> +
...
> +
> +err_clk_put:
> + clk_bulk_disable_unprepare(priv_data->num_clocks, priv_data->clks);
> + clk_bulk_put_all(priv_data->num_clocks, priv_data->clks);
clk_bulk_put_all() is not necessary because of devm_clk_bulk_get_all(),
and results in a double free.
> +static int dwc3_xlnx_remove(struct platform_device *pdev)
> +{
> + struct dwc3_xlnx *priv_data = platform_get_drvdata(pdev);
> + struct device *dev = &pdev->dev;
> +
> + of_platform_depopulate(dev);
> +
> + clk_bulk_disable_unprepare(priv_data->num_clocks, priv_data->clks);
> + clk_bulk_put_all(priv_data->num_clocks, priv_data->clks);
Same here. This will likely crash the driver on unload.
Guenter
Hi Guenter,
On 4/7/21 11:48 PM, Guenter Roeck wrote:
> On Wed, Mar 17, 2021 at 12:22:29PM +0530, Manish Narani wrote:
>> Add a new driver for supporting Xilinx platforms. This driver is used
>> for some sequence of operations required for Xilinx USB controllers.
>> This driver is also used to choose between PIPE clock coming from SerDes
>> and the Suspend Clock. Before the controller is out of reset, the clock
>> selection should be changed to PIPE clock in order to make the USB
>> controller work. There is a register added in Xilinx USB controller
>> register space for the same.
>>
>> Signed-off-by: Manish Narani <[email protected]>
>
> Trying this driver with qemu (v6.0.0-rc2) results in:
>
> [ 15.184242] dwc3-xilinx ff9d0000.usb: error -ENODEV: failed to assert Reset
> [ 15.185754] Unable to handle kernel paging request at virtual address 006b6b6b6b6b6b9b
> [ 15.185994] Mem abort info:
> [ 15.186065] ESR = 0x96000004
> [ 15.186317] EC = 0x25: DABT (current EL), IL = 32 bits
> [ 15.186414] SET = 0, FnV = 0
> [ 15.186498] EA = 0, S1PTW = 0
> [ 15.186579] Data abort info:
> [ 15.186666] ISV = 0, ISS = 0x00000004
> [ 15.186756] CM = 0, WnR = 0
> [ 15.186887] [006b6b6b6b6b6b9b] address between user and kernel address ranges
> [ 15.187436] Internal error: Oops: 96000004 [#1] PREEMPT SMP
> [ 15.187777] Modules linked in:
> [ 15.188060] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.12.0-rc6-next-20210406-00006-g05407f068fc9-dirty #1
> [ 15.188265] Hardware name: Xilinx Versal Virtual development board (DT)
> [ 15.188495] pstate: 80000005 (Nzcv daif -PAN -UAO -TCO BTYPE=--)
> [ 15.188614] pc : __clk_put+0x24/0x138
> [ 15.188716] lr : __clk_put+0x24/0x138
> [ 15.188791] sp : ffff80001326bac0
> [ 15.188853] x29: ffff80001326bac0 x28: ffff00000644ed00
> [ 15.188982] x27: ffff00000421ecd0 x26: ffff00000421e810
> [ 15.189076] x25: ffff00000644f100 x24: 0000000000000000
> [ 15.189170] x23: ffff8000126a2570 x22: 0000000000000005
> [ 15.189271] x21: ffff00000644ed00 x20: ffff000006449970
> [ 15.189367] x19: 6b6b6b6b6b6b6b6b x18: 0000000000000010
> [ 15.189456] x17: 0000000000000001 x16: 0000000000000000
> [ 15.189546] x15: ffff000003af0490 x14: 00000000000001b7
> [ 15.189642] x13: ffff000003af0490 x12: 00000000ffffffea
> [ 15.189729] x11: ffff8000123b6460 x10: 0000000000000080
> [ 15.189815] x9 : 00000000676993c6 x8 : 00000000676993c6
> [ 15.189941] x7 : 000000007d152ab3 x6 : ffff800012768480
> [ 15.190047] x5 : 0000000000000000 x4 : 000000007f97631e
> [ 15.190139] x3 : 00000000d5bdf2c2 x2 : 000000000000000b
> [ 15.190233] x1 : ffff000003af0040 x0 : 0000000000000001
> [ 15.190432] Call trace:
> [ 15.190506] __clk_put+0x24/0x138
> [ 15.190588] clk_put+0x10/0x20
> [ 15.190653] clk_bulk_put+0x3c/0x60
> [ 15.190724] devm_clk_bulk_release+0x1c/0x28
> [ 15.190806] release_nodes+0x1c0/0x2b0
> [ 15.190887] devres_release_all+0x38/0x60
> [ 15.190963] really_probe+0x1e4/0x3a8
> [ 15.191042] driver_probe_device+0x64/0xc8
> ...
>
> because of ...
>
>> +
>> + ret = devm_clk_bulk_get_all(priv_data->dev, &priv_data->clks);
>> + if (ret < 0)
>> + return ret;
>> +
> ...
>> +
>> +err_clk_put:
>> + clk_bulk_disable_unprepare(priv_data->num_clocks, priv_data->clks);
>> + clk_bulk_put_all(priv_data->num_clocks, priv_data->clks);
>
> clk_bulk_put_all() is not necessary because of devm_clk_bulk_get_all(),
> and results in a double free.
>
>> +static int dwc3_xlnx_remove(struct platform_device *pdev)
>> +{
>> + struct dwc3_xlnx *priv_data = platform_get_drvdata(pdev);
>> + struct device *dev = &pdev->dev;
>> +
>> + of_platform_depopulate(dev);
>> +
>> + clk_bulk_disable_unprepare(priv_data->num_clocks, priv_data->clks);
>> + clk_bulk_put_all(priv_data->num_clocks, priv_data->clks);
>
> Same here. This will likely crash the driver on unload.
It looks like that you directly created the patch. Isn't it better to
send it yourself? Or do you want Manish to create it based on guidance
above?
Manish: Can you please take a look at this?
Thanks,
Michal
Michal,
On 4/7/21 11:08 PM, Michal Simek wrote:
...
> It looks like that you directly created the patch. Isn't it better to
> send it yourself? Or do you want Manish to create it based on guidance
> above?
>
-next is substantially broken all over the place. I already spend way too much
time bisecting and analyzing the failures, and making sure that the problems
are not caused by qemu (which is why I tracked down this problem in such detail).
I don't really have time to write patches and guide them through the process,
sorry.
Guenter
Hi Guenter,
On 4/11/21 4:02 PM, Guenter Roeck wrote:
> Michal,
>
> On 4/7/21 11:08 PM, Michal Simek wrote:
> ...
>> It looks like that you directly created the patch. Isn't it better to
>> send it yourself? Or do you want Manish to create it based on guidance
>> above?
>>
> -next is substantially broken all over the place. I already spend way too much
> time bisecting and analyzing the failures, and making sure that the problems
> are not caused by qemu (which is why I tracked down this problem in such detail).
> I don't really have time to write patches and guide them through the process,
> sorry.
I definitely appreciate your help on this.
Manish has sent a patch for it already.
http://lore.kernel.org/r/[email protected]
Not sure why he hasn't added you to CC but at least Reported-by: tag
with your name should be added.
Thanks,
Michal