On Fri, May 25, 2018 at 10:12 AM, Yoshihiro Shimoda
<yoshihiro.shimoda.uh@xxxxxxxxxxx> wrote:
> -static void usb3_set_mode(struct renesas_usb3 *usb3, bool host)
Wouldn't be better to choose another name for a new function?
> + struct renesas_usb3 *usb3 = container_of(work, struct renesas_usb3,
> + role_work);
Matter of style, though I would rather put entire container_of() on
the next line (see for the existing style in the module and use it).
> + /* This device_attach() might sleep */
> + if (device_attach(host) < 0)
> + dev_err(dev, "device_attach(usb3_port) failed\n");
can't be "usb3_port" part derived from the host variable somehow and
to some extend?
> + usb3->role_sw = usb_role_switch_register(&pdev->dev,
> + &renesas_usb3_role_switch_desc);
> + if (!IS_ERR(usb3->role_sw)) {
> + usb3->host_dev = usb_of_get_companion_dev(&pdev->dev);
Hmm... Can it possible return -EPROBE_DEFER? If so, would it be better
to use other approach to handle it?
> + if (IS_ERR_OR_NULL(usb3->host_dev)) {
> + /* If not found, this driver will not use a role sw */
> + usb_role_switch_unregister(usb3->role_sw);
> + usb3->role_sw = NULL;
> + }
> + } else {
> + usb3->role_sw = NULL;
> + }
--
With Best Regards,
Andy Shevchenko
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
