On Thu, Mar 7, 2019 at 10:46 AM Neil Armstrong <narmstrong@xxxxxxxxxxxx> wrote: [...] > >> + /* Setup role switcher */ > >> + priv->switch_desc.usb2_port = dwc3_meson_g12_find_child(dev, > >> + "snps,dwc3"); > >> + priv->switch_desc.udc = dwc3_meson_g12_find_child(dev, "snps,dwc2"); > >> + priv->switch_desc.allow_userspace_control = true; > >> + priv->switch_desc.set = dwc3_meson_g12a_role_set; > >> + priv->switch_desc.get = dwc3_meson_g12a_role_get; > > to me, use a local variable for switch_desc > > I'm never fan of changing a local global variable for a driver instance, > is it strictly required ? usb_role_switch_register (which is called a few lines below this) copies all data from "struct usb_role_switch_desc" to a newly allocated "struct usb_role_switch" in other words: you could make it a stack-variable personally I have no preference so I leave the decision up to you Regards Martin