RE: [PATCH 2/2] usb: gadget: udc: renesas_usb3: Add support for RZ/V2M

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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





[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux