Hi Thinh,
> > +struct dwc3_rtk {
> > + struct device *dev;
> > + void __iomem *regs;
> > + size_t regs_size;
> > +
> > + struct dwc3 *dwc;
> > +
> > + int cur_dr_mode; /* current dr mode */
> > + bool support_drd_mode; /* if support Host/device switch */
>
> I think we may not need this and do away the boolean support_drd_mode.
Yes, the initial flow should be simplified as
@@ -346,12 +342,7 @@ static int dwc3_rtk_probe_dwc3_core(struct dwc3_rtk *rtk)
rtk->cur_dr_mode = dr_mode;
- if (device_property_read_bool(dwc3_dev, "usb-role-switch"))
- rtk->support_drd_mode = true;
- else
- rtk->support_drd_mode = false;
-
- if (rtk->support_drd_mode) {
+ if (device_property_read_bool(dwc3_dev, "usb-role-switch")) {
dwc3_rtk_setup_role_switch(rtk);
rtk->cur_dr_mode = dwc3_rtk_get_dr_mode(rtk);
}
> > +static int dwc3_rtk_set_dr_mode(struct dwc3_rtk *rtk, int dr_mode)
>
> Why return the mode rather than status if the setting? You're not checking the
> return of this function in the caller anyway.
You are right, this return value is unnecessary.
I will remove it.
> > +{
> > + if (!rtk->support_drd_mode)
> > + return rtk->cur_dr_mode;
> > +
> > + rtk->cur_dr_mode = dr_mode;
> > +
> > + switch_dwc3_dr_mode(rtk, dr_mode);
> > + mdelay(10);
> > + switch_usb2_dr_mode(rtk, dr_mode);
> > +
> > + return rtk->cur_dr_mode;
> > +}
> > +
> > +static int dwc3_rtk_setup_role_switch(struct dwc3_rtk *rtk)
>
> Any reason why we're doing the role switch here and not what's implemented
> from the core?
>
Because we have to set the usb 2.0 phy mode through switch_usb2_dr_mode in the function dwc3_rtk_set_dr_mode.
In fact, switch_dwc3_dr_mode will use the role switching implemented by core.
> > +
> > +module_platform_driver(dwc3_rtk_driver);
> > +
> > +MODULE_AUTHOR("Stanley Chang <stanley_chang@xxxxxxxxxxx>");
> > +MODULE_DESCRIPTION("DesignWare USB3 Realtek Glue Layer");
> > +MODULE_ALIAS("platform:rtk-dwc3");
> > +MODULE_LICENSE("GPL");
>
> I'm not familiar with licensing much, but can the SPDX header indicates
> different version than the module license?
>
Thanks Greg for your comment.
Either GPL or GPL v2 are suitable for our source code.
Thanks,
Stanley
