On Tue, Apr 10, 2018 at 3:03 PM, Yoshihiro Shimoda
<yoshihiro.shimoda.uh@xxxxxxxxxxx> wrote:
> This patch adds role switch support for R-Car SoCs. Some R-Car SoCs
> (e.g. R-Car H3) have USB 3.0 dual-role device controller which has
> the USB 3.0 xHCI host and Renesas USB 3.0 peripheral.
>
> Unfortunately, the mode change register contains the USB 3.0 peripheral
> controller side only. So, the USB 3.0 peripheral driver (renesas_usb3)
> manages this register. However, in peripheral mode, the host should
> stop. Also the host hardware needs to reinitialize its own registers
> when the mode changes from peripheral to host mode. Otherwise,
> the host cannot work correctly (e.g. detect a device as high-speed).
>
> To achieve this by a driver, this role switch driver manages
> the mode change register and attach/release the xhci-plat driver.
> The renesas_usb3 udc driver should call devm_of_platform_populate()
> to probe this driver.
> +#include <linux/err.h>
> +#include <linux/io.h>
> +#include <linux/module.h>
> +#include <linux/of_device.h>
Do you need this one?
> +#include <linux/platform_device.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/slab.h>
> +#include <linux/uaccess.h>
> +#include <linux/usb/role.h>
> +static const struct of_device_id rcar_usb3_role_switch_of_match[] = {
> + { .compatible = "renesas,rcar-usb3-role-switch" },
> + { },
Better to remove comma at terminator line.
> +};
> +MODULE_DEVICE_TABLE(of, rcar_usb3_role_switch_of_match);
> +static int rcar_usb3_role_switch_probe(struct platform_device *pdev)
> +{
> + /* Avoid devm_request_mem_region() calling by devm_ioremap_resource() */
Redundant comment.
> + host_node = of_parse_phandle(pdev->dev.of_node, "renesas,host", 0);
> + if (!host_node)
> + return -ENODEV;
> +
> + pdev_host = of_find_device_by_node(host_node);
> + if (!pdev_host)
> + return -ENODEV;
> +
> + of_node_put(host_node);
Isn't what Heikki tried to solve with graphs and stuff like that?
> + dev_info(&pdev->dev, "probed\n");
Noise. (Hint: initcall_debug is a good command line option)
> +}
> +#ifdef CONFIG_PM_SLEEP
Instead...
> +static int rcar_usb3_role_switch_suspend(struct device *dev)
...use __maybe_unused annotation.
> +static int rcar_usb3_role_switch_resume(struct device *dev)
Ditto.
> +#endif
> +static struct platform_driver rcar_usb3_role_switch_driver = {
> + .probe = rcar_usb3_role_switch_probe,
> + .remove = rcar_usb3_role_switch_remove,
> + .driver = {
> + .name = "rcar_usb3_role_switch",
> + .pm = &rcar_usb3_role_switch_pm_ops,
> + .of_match_table = of_match_ptr(rcar_usb3_role_switch_of_match),
Is it possible to have this driver w/o OF? Does it make sense in that case?
Otherwise remove of_match_ptr() call and below modalias.
> + },
> +};
> +MODULE_ALIAS("platform:rcar_usb3_role_switch");
--
With Best Regards,
Andy Shevchenko
