Re: [U-Boot] [RESEND PATCH 5/6] usb: host: Add ehci-vf USB driver for ARM Vybrid SoC's

2015-04-09 Thread maitysanchayan
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  0x0800
+#define USBCx_CTRL_OFFSET  0x
+#define USBCx_PHY_CTRL_OFFSET  0x0018
   
   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
 0x0030
+#define USBPHY_CTRL_SET
0x0034
+#define USBPHY_CTRL_CLR
0x0038
+#define USBPHY_CTRL_TOG
0x003c
+
+#define USBPHY_PWD 
 0x
+#define USBPHY_TX  
 0x0010
+#define USBPHY_RX  
 0x0020
+#define USBPHY_DEBUG   0x0050
+#define USBPHY_CTRL_SFTRST 0x8000
+#define USBPHY_CTRL_CLKGATE0x4000
+#define USBPHY_CTRL_ENUTMILEVEL3   0x8000
+#define USBPHY_CTRL_ENUTMILEVEL2   0x4000
+#define USBPHY_CTRL_OTG_ID 0x0800
+
+#define ANADIG_PLL_CTRL_BYPASS 0x0001
+#define ANADIG_PLL_CTRL_ENABLE 0x2000
+#define ANADIG_PLL_CTRL_POWER  0x1000
+#define ANADIG_PLL_CTRL_EN_USB_CLKS0x0040
+
+#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


Re: [U-Boot] [RESEND PATCH 5/6] usb: host: Add ehci-vf USB driver for ARM Vybrid SoC's

2015-04-09 Thread Marek Vasut
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_OFFSET0x0800
   +#define USBCx_CTRL_OFFSET0x
   +#define USBCx_PHY_CTRL_OFFSET0x0018
  
  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/ .

   +#define USBPHY_CTRL  
0x0030
   +#define USBPHY_CTRL_SET  0x0034
   +#define USBPHY_CTRL_CLR  0x0038
   +#define USBPHY_CTRL_TOG  0x003c
   +
   +#define USBPHY_PWD   
0x
   +#define USBPHY_TX
0x0010
   +#define USBPHY_RX
0x0020
   +#define USBPHY_DEBUG 0x0050
   +#define USBPHY_CTRL_SFTRST   0x8000
   +#define USBPHY_CTRL_CLKGATE  0x4000
   +#define USBPHY_CTRL_ENUTMILEVEL3 0x8000
   +#define USBPHY_CTRL_ENUTMILEVEL2 0x4000
   +#define USBPHY_CTRL_OTG_ID   0x0800
   +
   +#define ANADIG_PLL_CTRL_BYPASS   0x0001
   +#define ANADIG_PLL_CTRL_ENABLE   0x2000
   +#define ANADIG_PLL_CTRL_POWER0x1000
   +#define ANADIG_PLL_CTRL_EN_USB_CLKS  0x0040
   +
   +#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


Re: [U-Boot] [RESEND PATCH 5/6] usb: host: Add ehci-vf USB driver for ARM Vybrid SoC's

2015-04-09 Thread Fabio Estevam
Hi Marek,

On Wed, Apr 1, 2015 at 4:15 PM, Marek Vasut ma...@denx.de wrote:

 +#define USB_NC_REG_OFFSET0x0800
 +#define USBCx_CTRL_OFFSET0x
 +#define USBCx_PHY_CTRL_OFFSET0x0018

 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 thought we were trying to get rid of the 'no register access via
offset' rule in U-boot.
https://www.marc.info/?l=u-bootm=142609602127309w=2

Please see:

Regards,

Fabio Estevam
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [RESEND PATCH 5/6] usb: host: Add ehci-vf USB driver for ARM Vybrid SoC's

2015-04-07 Thread maitysanchayan
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  0x0800
  +#define USBCx_CTRL_OFFSET  0x
  +#define USBCx_PHY_CTRL_OFFSET  0x0018
 
 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.

Thoughts?

 
  +#define USBPHY_CTRL
  0x0030
  +#define USBPHY_CTRL_SET0x0034
  +#define USBPHY_CTRL_CLR0x0038
  +#define USBPHY_CTRL_TOG0x003c
  +
  +#define USBPHY_PWD 0x
  +#define USBPHY_TX  0x0010
  +#define USBPHY_RX  0x0020
  +#define USBPHY_DEBUG   0x0050
  +#define USBPHY_CTRL_SFTRST 0x8000
  +#define USBPHY_CTRL_CLKGATE0x4000
  +#define USBPHY_CTRL_ENUTMILEVEL3   0x8000
  +#define USBPHY_CTRL_ENUTMILEVEL2   0x4000
  +#define USBPHY_CTRL_OTG_ID 0x0800
  +
  +#define ANADIG_PLL_CTRL_BYPASS 0x0001
  +#define ANADIG_PLL_CTRL_ENABLE 0x2000
  +#define ANADIG_PLL_CTRL_POWER  0x1000
  +#define ANADIG_PLL_CTRL_EN_USB_CLKS0x0040
  +
  +#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.

 
 Best regards,
 Marek Vasut

- Sanchayan.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [RESEND PATCH 5/6] usb: host: Add ehci-vf USB driver for ARM Vybrid SoC's

2015-04-02 Thread maitysanchayan
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.

Ok. Will add the commit message in v2.

 
  Signed-off-by: Sanchayan Maity maitysancha...@gmail.com
 
 [...]
 
  +#define USB_NC_REG_OFFSET  0x0800
  +#define USBCx_CTRL_OFFSET  0x
  +#define USBCx_PHY_CTRL_OFFSET  0x0018
 
 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 .

Ok. Will fix in v2.

 
  +#define USBPHY_CTRL
  0x0030
  +#define USBPHY_CTRL_SET0x0034
  +#define USBPHY_CTRL_CLR0x0038
  +#define USBPHY_CTRL_TOG0x003c
  +
  +#define USBPHY_PWD 0x
  +#define USBPHY_TX  0x0010
  +#define USBPHY_RX  0x0020
  +#define USBPHY_DEBUG   0x0050
  +#define USBPHY_CTRL_SFTRST 0x8000
  +#define USBPHY_CTRL_CLKGATE0x4000
  +#define USBPHY_CTRL_ENUTMILEVEL3   0x8000
  +#define USBPHY_CTRL_ENUTMILEVEL2   0x4000
  +#define USBPHY_CTRL_OTG_ID 0x0800
  +
  +#define ANADIG_PLL_CTRL_BYPASS 0x0001
  +#define ANADIG_PLL_CTRL_ENABLE 0x2000
  +#define ANADIG_PLL_CTRL_POWER  0x1000
  +#define ANADIG_PLL_CTRL_EN_USB_CLKS0x0040
  +
  +#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 ?

I have not yet looked at MX28, but, the Vybrid USB IP is in fact very 
similar to i.MX6, however, the non core registers are spread in two 
different register areas. Will have a look at MX28 as well.

This ehci-vf driver is very closely based on ehci-mx6 driver already in 
uboot. 

 
 Best regards,
 Marek Vasut

- Sanchayan.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [RESEND PATCH 5/6] usb: host: Add ehci-vf USB driver for ARM Vybrid SoC's

2015-04-02 Thread Marek Vasut
On Thursday, April 02, 2015 at 12:55:57 PM, maitysancha...@gmail.com wrote:
 Hello,

Hi!
[...]
   +#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 ?
 
 I have not yet looked at MX28, but, the Vybrid USB IP is in fact very
 similar to i.MX6, however, the non core registers are spread in two
 different register areas. Will have a look at MX28 as well.
 
 This ehci-vf driver is very closely based on ehci-mx6 driver already in
 uboot.

Understood, thanks!

Best regards,
Marek Vasut
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [RESEND PATCH 5/6] usb: host: Add ehci-vf USB driver for ARM Vybrid SoC's

2015-04-01 Thread Marek Vasut
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_OFFSET0x0800
 +#define USBCx_CTRL_OFFSET0x
 +#define USBCx_PHY_CTRL_OFFSET0x0018

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 .

 +#define USBPHY_CTRL  0x0030
 +#define USBPHY_CTRL_SET  0x0034
 +#define USBPHY_CTRL_CLR  0x0038
 +#define USBPHY_CTRL_TOG  0x003c
 +
 +#define USBPHY_PWD   0x
 +#define USBPHY_TX0x0010
 +#define USBPHY_RX0x0020
 +#define USBPHY_DEBUG 0x0050
 +#define USBPHY_CTRL_SFTRST   0x8000
 +#define USBPHY_CTRL_CLKGATE  0x4000
 +#define USBPHY_CTRL_ENUTMILEVEL3 0x8000
 +#define USBPHY_CTRL_ENUTMILEVEL2 0x4000
 +#define USBPHY_CTRL_OTG_ID   0x0800
 +
 +#define ANADIG_PLL_CTRL_BYPASS   0x0001
 +#define ANADIG_PLL_CTRL_ENABLE   0x2000
 +#define ANADIG_PLL_CTRL_POWER0x1000
 +#define ANADIG_PLL_CTRL_EN_USB_CLKS  0x0040
 +
 +#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 ?

Best regards,
Marek Vasut
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot