On Thu, Jul 12, 2012 at 03:01:52PM +0800, Richard Zhao wrote:

i.MX usb controllers shares non-core registers, which may include
SoC specific controls. We take it as a usbmisc device and usbmisc
driver export functions needed by ci13xxx_imx driver.

So this is SoC and not chipidea specific and should not be sorted into
this subdir.

For example, Sabrelite board has bad over-current design, we can
usbmisc to disable over-current detect.

This driver layout is also reflected in:

arch/arm/mach-imx/ehci-imx*.c

You should use these existing mx*_initialize_usb_hw functions to avoid
code duplication and add your mx6_initialize_usb_hw routine for mx6 there.

This devicetree based glue code in this file can be reused to
call the right mx*_initialize_usb_hw by the compatible.

We already have common flags available like i.e. MXC_EHCI_POWER_PINS_ENABLED
which is currently used to disable overcurrent detection.

Global variable?

+
+static int imx6q_usbmisc_init(struct usbmisc *usbmisc, int id)

What is initialized here? How about "preconfigure"?
Even better is to reuse mx*_initialize_usb_hw here.

+{
+	u32 reg;
+
+	if (usbmisc->dis_oc) {
+		reg = readl_relaxed(usbmisc->base + id * 4);
+		writel_relaxed(reg | (1 << 7), usbmisc->base + id * 4);

#define IMX6_OTG_CTRL_USBMISC        1 << 7

+	}
+
+	return 0;
+}
+
+static struct soc_data imx6q_data = {
+	.init = imx6q_usbmisc_init,
+};
+
+
+static const struct of_device_id imx_usbmisc_dt_ids[] = {
+	{ .compatible = "fsl,imx6q-usbmisc", .data = &imx6q_data },

        something like this:
	{ .compatible = "fsl,imx6q-usbmisc", .data = &mx6_initialize_usb_hw },

+	{ /* sentinel */ }
+};
+
+static int __devinit imx_usbmisc_probe(struct platform_device *pdev)
+{
+	struct resource	*res;
+	struct usbmisc *data;
+	const struct of_device_id *of_id =
+			of_match_device(imx_usbmisc_dt_ids, &pdev->dev);
+
+	int ret;
+
+	if (usbmisc)
+		return -EBUSY;
+
+	data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	data->base = devm_request_and_ioremap(&pdev->dev, res);
+	if (!data->base)
+		return -EADDRNOTAVAIL;
+
+	data->clk = devm_clk_get(&pdev->dev, NULL);
+	if (IS_ERR(data->clk)) {
+		dev_err(&pdev->dev,
+			"failed to get clock, err=%ld\n", PTR_ERR(data->clk));
+		return PTR_ERR(data->clk);
+	}
+
+	ret = clk_prepare_enable(data->clk);
+	if (!ret) {

Better:
        if (ret)
                return ret;

+		if (of_find_property(pdev->dev.of_node,
+			"fsl,disable-over-current", NULL))
+			data->dis_oc = 1;
+		data->soc_data = of_id->data;
+		usbmisc = data;
+	}
+

        if (of_find_property(pdev->dev.of_node,
        ...

+	return ret;
+}
+
+static int __devexit imx_usbmisc_remove(struct platform_device *pdev)
+{
+	clk_disable_unprepare(usbmisc->clk);
+	return 0;
+}
+
+static struct platform_driver imx_usbmisc_driver = {
+	.probe = imx_usbmisc_probe,
+	.remove = __devexit_p(imx_usbmisc_remove),
+	.driver = {
+		.name = "imx_usbmisc",
+		.owner = THIS_MODULE,
+		.of_match_table = imx_usbmisc_dt_ids,
+	 },
+};
+
+int imx_usbmisc_init(struct device *usbdev)
+{
+	struct device_node *np = usbdev->of_node;
+	int ret;
+
+	if (!usbmisc)
+		return 0;
+
+	ret = of_alias_get_id(np, "usb");
+	if (ret < 0) {
+		dev_err(usbdev, "failed to get alias id, errno %d\n", ret);
+		return ret;
+	}
+
+	return usbmisc->soc_data->init(usbmisc, ret);
+}
+EXPORT_SYMBOL_GPL(imx_usbmisc_init);

EXPORT_SYMBOL is only needed because it is
compliled as an extra kernel module.

+
+static int __init imx_usbmisc_drv_init(void)
+{
+	return platform_driver_register(&imx_usbmisc_driver);
+}
+subsys_initcall(imx_usbmisc_drv_init);
+
+static void __exit imx_usbmisc_drv_exit(void)
+{
+	platform_driver_unregister(&imx_usbmisc_driver);
+}
+module_exit(imx_usbmisc_drv_exit);
+
+MODULE_LICENSE("GPL v2");
-- 
1.7.9.5

Thanks,
Michael

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |