Hi Phil-san, > From: Phil Edworthy, Sent: Thursday, July 21, 2022 11:21 PM > > Hi Yoshihiro, > > On 21 July 2022 13:51 Yoshihiro Shimoda wrote: > > > From: Geert Uytterhoeven, Sent: Thursday, July 21, 2022 9:43 PM > > > On Thu, Jul 21, 2022 at 2:25 PM Phil Edworthy wrote: > > > > On 21 July 2022 13:10 Geert Uytterhoeven wrote: > > > > > On Mon, Jul 18, 2022 at 3:45 PM Phil Edworthy wrote: > > > > > > RZ/V2M (r9a09g011) has a few differences: > > > > > > - The USB3_DRD_CON register has moved, its called > > USB_PERI_DRD_CON in > > > > > > the RZ/V2M hardware manual. > > > > > > It has additional bits for host and peripheral reset that need > > to > > > > > > cleared to use usb host and peripheral respectively. > > > > > > - The USB3_OTG_STA, USB3_OTG_INT_STA and USB3_OTG_INT_ENA > > registers > > > > > > have been moved and renamed to USB_PERI_DRD_STA, > > USB_PERI_DRD_INT_STA > > > > > > and USB_PERI_DRD_INT_E. > > > > > > - The IDMON bit used in the above regs for role detection have > > moved > > > > > > from bit 4 to bit 0. > > > > > > - RZ/V2M has an separate interrupt for DRD, i.e. for changes to > > IDMON. > > > > > > - There are reset lines for DRD and USBP > > > > > > - There is another clock, managed by runtime PM. > > > > > > > > > > > > Signed-off-by: Phil Edworthy <phil.edworthy@xxxxxxxxxxx> > > > > > > Reviewed-by: Biju Das <biju.das.jz@xxxxxxxxxxxxxx> > > > > > > > > > > Thanks for your patch! > > > > > > > > > > > --- a/drivers/usb/gadget/udc/renesas_usb3.c > > > > > > +++ b/drivers/usb/gadget/udc/renesas_usb3.c > > > > > > > > > > > @@ -363,6 +368,7 @@ struct renesas_usb3 { > > > > > > bool forced_b_device; > > > > > > bool start_to_connect; > > > > > > bool role_sw_by_connector; > > > > > > + bool r9a09g011; > > > > > > > > > > Any better name for this feature flag? > > > > Nothing springs to mind. We could use separate flags for has_resets, > > > > has_drd_irq, max_nr_pipes but I am struggling to come up with names > > > > for the offset registers and moved bits. Any suggestions? > > > > > > OK, so "is_rzv2m"? > > > > The flag name looks good to me. However, I don't like the following usage > > in macros. > > --- > > -#define USB3_DRD_CON 0x218 > > +#define USB3_DRD_CON (usb3->r9a09g011 ? 0x400 : 0x218) > > ... > > -#define USB_OTG_IDMON BIT(4) > > +#define USB_OTG_IDMON (usb3->r9a09g011 ? BIT(0) : BIT(4)) > > ---- > > > > About registers' offset/bit, I think having specific members into > > a new struct is better like below. But, what do you think? > > > > struct renesas_usb3_reg { > > u16 drd_con_offset; > > ... > > u32 otg_idmon_bit; > > ... > > }; > > > > struct renesas_usb3 { > > ... > > struct renesas_usb3_reg regs; > > ... > > }; > > I think that might be a bit overly complex for the problem. > How about: > #define USB3_DRD_CON(p) ((p)->is_rzv2m ? 0x400 : 0x218) Thank you for your suggestion. It looks good to me. Best regards, Yoshihiro Shimoda