RE: [PATCH v2 1/2] usb: dwc3: add Realtek DHC RTD SoC dwc3 glue layer driver

From: Stanley Chang[昌育德]
Date: Thu Aug 03 2023 - 02:56:36 EST


Hi Thinh,

> > > > +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.
> >
>
> I don't think this is a good way to go about it. Do you (the glue
> driver) create a role switch structure and register to the same role switch
> device that the dwc3 core would register later? I did not expect that to work at
> all.
>
In our application, this role switch in the glue driver works fine.

> How about we create a couple ops callbacks to dwc3 structure. One to
> initialize gadget and the other for host. Should any platform specific device
> need some intialization before initializing the specific role, we can invoke
> these callbacks.
>
> e.g.:
>
> struct dwc3_glue_ops {
> int (*init_gadget)(struct dwc3 *dwc);
> int (*init_host)(struct dwc3 *dwc); }
>
> struct dwc3 {
> ..
> const struct dwc3_glue_ops *ops;
> }
>
>
> static int dwc3_glue_init_gadget(struct dwc3 *dwc) {
> if (!dwc->ops->init_gadget)
> return 0;
>
> return dwc->ops->init_gadget(dwc); }
>
> static int dwc3_glue_init_host(struct dwc3 *dwc) {
> if (!dwc->ops->init_host)
> return 0;
>
> return dwc->ops->init_host(dwc); }
>
>
> Your glue driver would implement these callbacks. In dwc3_gadget_init() and
> dwc3_host_init(), we can call these callbacks at start. I admit that this may not
> be the greatest solution since it would require the glue driver to alloc and
> register a dwc3 platform device. But I think should be ok. If anyone else can
> chime in for more idea, it'd be great.


Thanks for your advice.
Here is a solution. As you're concerned, it must register callbacks with the core driver. It looks a bit complicated.
I noticed the phy_set_mode api.
Maybe I can try to move switch_usb2_dr_mode to phy driver.

phy_set_mode(dwc->usb2_generic_phy, PHY_MODE_USB_HOST);
phy_set_mode(dwc->usb2_generic_phy, PHY_MODE_USB_DEVICE);

Thanks,
Stanley