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 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html