On Wed, May 25, 2016 at 10:44 PM, Jun Li wrote: > Hi Roger > > > >> > > >> Here, we should be checking if user needs to disable any OTG > > >> features. So, > > >> > > >> if (dev->of_node) > > >> of_usb_update_otg_caps(dev->of_node, &otg->caps); > > >> > > >> Do you agree? > > >> This means we need to change otg->caps from 'struct usb_otg_caps > > *caps;' > > >> to 'struct usb_otg_caps caps;' so that we can modify the local copy > > >> instead of the one passed by the OTG controller. > > > > > > Why can't modify the one from OTG controller directly? > > > > > > > There are 2 things. > > 1) OTG features supported by hardware. This is the controller's config- > > >otg_caps > > 2) OTG features needed by system designer. This can be a subset of (1). > > Let's make things simple, we only need this subnet, which can be set > by controller driver in config->otg_caps before pass (its address) > to OTG core. > > So controller driver should get the capability of HW(+SW) and user config > by whatever approach, then set its config->otg_caps. > > Li Jun > We may not use controller's otg capabilities after initialization, so struct usb_otg_caps in controller device's structure can standard for board's otg capabilities. My suggestion is passing controller's cap to usb_otg_register, and update in it using of_usb_update_otg_caps, in that case, each controller driver doesn't need to do it by each other. -- BR, Peter Chen