Hi Geert-san, > From: Geert Uytterhoeven, Sent: Wednesday, March 29, 2017 9:23 PM > > Hi Shimoda-san, > > On Wed, Mar 29, 2017 at 1:42 PM, Yoshihiro Shimoda > <yoshihiro.shimoda.uh@xxxxxxxxxxx> wrote: > > --- a/drivers/usb/gadget/udc/renesas_usb3.c > > +++ b/drivers/usb/gadget/udc/renesas_usb3.c > > @@ -568,12 +573,29 @@ static void usb3_mode_a_host(struct renesas_usb3 *usb3) > > usb3_set_bit(usb3, DRD_CON_VBOUT, USB3_DRD_CON); > > } > > > > +static void usb3_mode_a_peri(struct renesas_usb3 *usb3) > > +{ > > + unsigned long flags; > > + > > + spin_lock_irqsave(&usb3->lock, flags); > > + usb3_set_bit(usb3, DRD_CON_PERI_CON, USB3_DRD_CON); > > + usb3_set_bit(usb3, DRD_CON_VBOUT, USB3_DRD_CON); > > + usb3_connect(usb3); > > + spin_unlock_irqrestore(&usb3->lock, flags); > > +} > > + > > static void usb3_mode_b_peri(struct renesas_usb3 *usb3) > > { > > usb3_set_bit(usb3, DRD_CON_PERI_CON, USB3_DRD_CON); > > usb3_clear_bit(usb3, DRD_CON_VBOUT, USB3_DRD_CON); > > } > > > > +static void usb3_mode_b_host(struct renesas_usb3 *usb3) > > +{ > > + usb3_clear_bit(usb3, DRD_CON_PERI_CON, USB3_DRD_CON); > > + usb3_clear_bit(usb3, DRD_CON_VBOUT, USB3_DRD_CON); > > +} > > + > > static bool usb3_is_a_device(struct renesas_usb3 *usb3) > > { > > return !(usb3_read(usb3, USB3_USB_OTG_STA) & USB_OTG_IDMON); > > @@ -1831,11 +1853,59 @@ static int renesas_usb3_set_selfpowered(struct usb_gadget *gadget, int is_self) > > .set_selfpowered = renesas_usb3_set_selfpowered, > > }; > > > > +static ssize_t role_store(struct device *dev, struct device_attribute *attr, > > + const char *buf, size_t count) > > +{ > > > + if (new_mode_is_host) { > > + if (usb3_is_a_device(usb3)) > > + usb3_mode_a_host(usb3); > > + else > > + usb3_mode_b_host(usb3); > > + } else { > > + if (usb3_is_a_device(usb3)) > > + usb3_mode_a_peri(usb3); > > + else > > + usb3_mode_b_peri(usb3); > > + } > > Given the similarity of the usb3_mode_X_{host,peri}() functions, I'm wondering > if the code would become easier to read and maintain by merging them pairwise > into usb3_mode_X_config(), and writing the above block like: > > if (usb3_is_a_device(usb3)) > usb3_mode_a_config(usb3, new_mode_is_host); > else > usb3_mode_b_config(usb3, new_mode_is_host); > > ? Thank you for the comment! I think so. So, I will modify the code and send v3 patch set. Best regards, Yoshihiro Shimoda > Gr{oetje,eeting}s, > > Geert > > -- > Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx > > In personal conversations with technical people, I call myself a hacker. But > when I'm talking to journalists I just say "programmer" or something like that. > -- Linus Torvalds ��.n��������+%������w��{.n�����{���)��jg��������ݢj����G�������j:+v���w�m������w�������h�����٥