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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <7d47cbfdbc31436b89d3d92bf980c8fa@realtek.com>
Date:   Thu, 3 Aug 2023 06:55:58 +0000
From:   Stanley Chang[昌育德] 
        <stanley_chang@...ltek.com>
To:     Thinh Nguyen <Thinh.Nguyen@...opsys.com>
CC:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Conor Dooley <conor+dt@...nel.org>,
        "linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH v2 1/2] usb: dwc3: add Realtek DHC RTD SoC dwc3 glue layer driver

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