Hello,

On 15-04-09 18:09:46, Marek Vasut wrote:
> On Tuesday, April 07, 2015 at 09:03:45 AM, maitysancha...@gmail.com wrote:
> > Hello,
> > 
> > On 15-04-01 21:15:21, Marek Vasut wrote:
> > > On Wednesday, April 01, 2015 at 11:54:22 AM, Sanchayan Maity wrote:
> > > 
> > > The commit message is missing, please fix in v2.
> > > 
> > > > Signed-off-by: Sanchayan Maity <maitysancha...@gmail.com>
> > > 
> > > [...]
> > > 
> > > > +#define USB_NC_REG_OFFSET                              0x00000800
> > > > +#define USBCx_CTRL_OFFSET                              0x00000000
> > > > +#define USBCx_PHY_CTRL_OFFSET                  0x00000018
> > > 
> > > Please define the register offsets using the regular struct {} method,
> > > see for example struct mxs_usbphy_regs and it's usage in ehci-mxs.c .
> > 
> > I had a query here, just to be sure and avoid rework. The vybrid defines
> > would be similar to mxs. I assume I can add them to the regs-common.h
> > file along with a note that the VF610 also has the same _set, _clr,
> > _tog register? Or perhaps it would be more appropriate to have the file
> > have generic names which mxs, vf and imx can all leverage? Though for
> > now this would require reworking all the three drivers.
> > 
> > The USB phy definitions part is ok, as they would go in the arch
> > specific folder.
> 
> If these are really IMX/MXS/VF specific, then the defines should go into
> arch/arm/include/asm/imx-common/ . Otherwise, you can make chipidea specific 
> file in include/usb/ .

Not really much VF specific. I was not sure about using the mxs_ prefix 
accessors for VF as well. In the end I found usage in one iMX6 file and 
decided that it makes better sense to actually use the mxs_ existing 
defines. They can be used for Vybrid as well. Except for the non core 
regsiters and a few others no other difference.

I send a v2 version of the patchset based taking your suggestion into 
account. The driver looks cleaner in comparison to the previous :) 
IMHO. Thanks.

https://www.mail-archive.com/u-boot@lists.denx.de/msg168727.html


- Sanchayan.

> 
> > > > +#define USBPHY_CTRL                                            
> 0x00000030
> > > > +#define USBPHY_CTRL_SET                                        
> > > > 0x00000034
> > > > +#define USBPHY_CTRL_CLR                                        
> > > > 0x00000038
> > > > +#define USBPHY_CTRL_TOG                                        
> > > > 0x0000003c
> > > > +
> > > > +#define USBPHY_PWD                                             
> 0x00000000
> > > > +#define USBPHY_TX                                              
> 0x00000010
> > > > +#define USBPHY_RX                                              
> 0x00000020
> > > > +#define USBPHY_DEBUG                                   0x00000050
> > > > +#define USBPHY_CTRL_SFTRST                             0x80000000
> > > > +#define USBPHY_CTRL_CLKGATE                            0x40000000
> > > > +#define USBPHY_CTRL_ENUTMILEVEL3               0x00008000
> > > > +#define USBPHY_CTRL_ENUTMILEVEL2               0x00004000
> > > > +#define USBPHY_CTRL_OTG_ID                             0x08000000
> > > > +
> > > > +#define ANADIG_PLL_CTRL_BYPASS                 0x00010000
> > > > +#define ANADIG_PLL_CTRL_ENABLE                 0x00002000
> > > > +#define ANADIG_PLL_CTRL_POWER                  0x00001000
> > > > +#define ANADIG_PLL_CTRL_EN_USB_CLKS            0x00000040
> > > > +
> > > > +#define UCTRL_OVER_CUR_POL     (1 << 8) /* OTG Polarity of Overcurrent 
> */
> > > > +#define UCTRL_OVER_CUR_DIS     (1 << 7) /* Disable OTG Overcurrent
> > > > Detection */ +
> > > > +/* USBCMD */
> > > > +#define UCMD_RUN_STOP          (1 << 0) /* controller run/stop */
> > > > +#define UCMD_RESET                     (1 << 1) /* controller reset */
> > > 
> > > This looks very much like the USB PHY used on MX28 , can you double-check
> > > this please ?
> > 
> > MX28 IP also seems similar to the Vybrid USB IP except for a few
> > registers and the non core registers. Perhaps to be expected as they all
> > have a common chipidea IP core, though having a different version
> > thereof.
> 
> Yep, I agree :)
> 
> Thanks!
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to