[Patch v7 1/3] usb: dwc3: Add Qualcomm DWC3 glue layer driver

On Tuesday, July 01, 2014 1:04 AM, Felipe Balbi wrote:

How about adding the machine dependency? Ex. ARCH_QCOM?
Currently, other dwc3 glue layers are using ARCH_* dependency.

+       select USB_QCOM_DWC3_PHY
+       help
+         Recent Qualcomm SoCs ship with one DesignWare Core USB3 IP inside,
+         say 'Y' or 'M' if you have one such device.
+
+

Please remove an unnecessary line. One line is enough.

Please remove an unnecessary line. One line is enough.

+struct dwc3_qcom {
+	struct device		*dev;
+
+	struct clk		*core_clk;
+	struct clk		*iface_clk;
+	struct clk		*sleep_clk;
+
+	struct regulator	*gdsc;
+};
+
+static int dwc3_qcom_probe(struct platform_device *pdev)
+{
+	struct device_node *node = pdev->dev.of_node;
+	struct dwc3_qcom *qdwc;
+	int ret = 0;
+
+	qdwc = devm_kzalloc(&pdev->dev, sizeof(*qdwc), GFP_KERNEL);
+	if (!qdwc)
+		return -ENOMEM;
+
+	platform_set_drvdata(pdev, qdwc);
+
+	qdwc->dev = &pdev->dev;
+
+	qdwc->gdsc = devm_regulator_get(qdwc->dev, "gdsc");
+
+	qdwc->core_clk = devm_clk_get(qdwc->dev, "core");
+	if (IS_ERR(qdwc->core_clk)) {
+		dev_dbg(qdwc->dev, "failed to get core clock\n");
+		return PTR_ERR(qdwc->core_clk);
+	}
+
+	qdwc->iface_clk = devm_clk_get(qdwc->dev, "iface");
+	if (IS_ERR(qdwc->iface_clk)) {
+		dev_dbg(qdwc->dev, "failed to get iface clock, skipping\n");
+		qdwc->iface_clk = NULL;
+	}
+
+	qdwc->sleep_clk = devm_clk_get(qdwc->dev, "sleep");
+	if (IS_ERR(qdwc->sleep_clk)) {
+		dev_dbg(qdwc->dev, "failed to get sleep clock, skipping\n");
+		qdwc->sleep_clk = NULL;
+	}
+
+	if (!IS_ERR(qdwc->gdsc)) {
+		ret = regulator_enable(qdwc->gdsc);
+		if (ret)
+			dev_err(qdwc->dev, "cannot enable gdsc\n");
+	}
+
+	clk_prepare_enable(qdwc->core_clk);
+
+	if (qdwc->iface_clk)
+		clk_prepare_enable(qdwc->iface_clk);
+
+	if (qdwc->sleep_clk)
+		clk_prepare_enable(qdwc->sleep_clk);
+
+	ret = of_platform_populate(node, NULL, NULL, qdwc->dev);
+	if (ret) {
+		dev_err(qdwc->dev, "failed to register core - %d\n", ret);
+		dev_dbg(qdwc->dev, "failed to add create dwc3 core\n");

Is there any reason to use two similar messages?
For me, it looks duplicated.

+		goto dis_clks;
+	}
+
+	return 0;
+
+dis_clks:
+	if (qdwc->sleep_clk)
+		clk_disable_unprepare(qdwc->sleep_clk);
+
+	if (qdwc->iface_clk)
+		clk_disable_unprepare(qdwc->iface_clk);
+
+	clk_disable_unprepare(qdwc->core_clk);
+
+	if (!IS_ERR(qdwc->gdsc)) {
+		ret = regulator_disable(qdwc->gdsc);
+		if (ret)
+			dev_dbg(qdwc->dev, "cannot disable gdsc\n");
+	}
+
+	return ret;
+}
+
+static int dwc3_qcom_remove(struct platform_device *pdev)
+{
+	int ret = 0;
+
+	struct dwc3_qcom *qdwc = platform_get_drvdata(pdev);
+
+	if (qdwc->sleep_clk)
+		clk_disable_unprepare(qdwc->sleep_clk);
+
+	if (qdwc->iface_clk)
+		clk_disable_unprepare(qdwc->iface_clk);
+
+	clk_disable_unprepare(qdwc->core_clk);
+
+	if (!IS_ERR(qdwc->gdsc)) {
+		ret = regulator_disable(qdwc->gdsc);
+		if (ret)
+			dev_dbg(qdwc->dev, "cannot disable gdsc\n");
+	}
+	return ret;
+}
+
+static const struct of_device_id of_dwc3_match[] = {
+	{ .compatible = "qcom,dwc3" },
+	{ /* Sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, of_dwc3_match);
+
+static struct platform_driver dwc3_qcom_driver = {
+	.probe		= dwc3_qcom_probe,
+	.remove		= dwc3_qcom_remove,
+	.driver		= {
+		.name	= "qcom-dwc3",
+		.owner	= THIS_MODULE,
+		.of_match_table	= of_dwc3_match,
+	},
+};
+
+module_platform_driver(dwc3_qcom_driver);
+
+MODULE_ALIAS("platform:qcom-dwc3");

How about adding the module author as Ivan T. Ivanov and you?

MODULE_AUTHOR("Ivan T. Ivanov [off-list ref]");
MODULE_AUTHOR("Andy Gross [off-list ref]");

Best regards,
Jingoo Han

+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("DesignWare USB3 QCOM Glue Layer");
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation