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
> <[email protected]> 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 --
> [email protected]
>
> 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
