Re: [U-Boot] [PATCH v2 1/4] ehci-omap: Clean up added ehci-omap.c

2012-02-03 Thread Govindraj
Hi Igor,

Sorry for late reply, Was really busy with some other tasks.


On Sun, Jan 29, 2012 at 3:12 PM, Igor Grinberg grinb...@compulab.co.il wrote:
 Hi Govindraj,

 Put Remy on Cc.


Yes Sure,

 Note: my new mail client for some reason, messes with white spaces...
 (Thunderbird (9) just gets better and better...)
 So, please ignore those.
 BTW, is there anyone who has those issues fixed and can save me a wasted
 hour?


[...]



 Ok, I think you should split this patch to:
 1) modify the ulpi framework... (this one extends the ulpi functionality
   and fixes current users)
 2) add omap ulpi support (this one makes use of the ulpi framework
   on omap platforms and IMO, should amended to you original
   1/4 patch: ehci-omap: Clean up added ehci-omap.c)

Yes Correct, I have done that will be posting the complete
series in short while.




 Tested on beagle-xm and Panda.
 Compile tested on efikamx platform.

 This patch is based on earlier posted patch series[1].


 Shouldn't this be [2]?

yes correct my mistake.




 Prior to this patch ulpi reset was done directly now
 it is done using ulpi_reset.

 Thanks to Igor Grinberggrinb...@compulab.co.il  for reviewing the
 omap-ehci patches and suggesting this approach.

 This patch along with the patch series [1] rebased on latest u-boot


 Probably same here, or am I mistaken?

yes you are right, it should be [2]



 is available here.
        git://gitorious.org/denx_u-boot/denx_uboot_omap.git v2_ehci_omap4

 [1]: http://www.mail-archive.com/u-boot@lists.denx.de/msg76076.html
 [2]: [PATCH v2 0/4] Clean up ehci-omap and extend support for omap3/4
 socs

 Signed-off-by: Govindraj.Rgovindraj.r...@ti.com
 ---
  board/efikamx/efikamx-usb.c           |   21 ---
  drivers/usb/host/ehci-omap.c          |   21 ++-
  drivers/usb/ulpi/Makefile             |    1 +
  drivers/usb/ulpi/omap-ulpi-viewport.c |  110
 +
  drivers/usb/ulpi/ulpi-viewport.c      |   30 +-
  drivers/usb/ulpi/ulpi.c               |   54 
  include/configs/omap3_beagle.h        |    3 +
  include/configs/omap4_panda.h         |    2 +
  include/usb/ulpi.h                    |   37 


 You should also, update the README file with the new
 CONFIG_USB_ULPI_VIEWPORT_OMAP option.
 (Also may be some OMAP specific doc files..)


yes fine, I am adding a new doc
README.omap-ulpi-viewport
and adding some minimal info on the omap-ulpi-viewport
usage and usecase.



  9 files changed, 202 insertions(+), 77 deletions(-)
  create mode 100644 drivers/usb/ulpi/omap-ulpi-viewport.c

 diff --git a/board/efikamx/efikamx-usb.c b/board/efikamx/efikamx-usb.c
 index 840bd9a..bb1398b 100644
 --- a/board/efikamx/efikamx-usb.c
 +++ b/board/efikamx/efikamx-usb.c
 @@ -120,6 +120,7 @@ static void efika_ehci_init(struct usb_ehci *ehci,
 uint32_t stp_gpio,
  {
        int ret;
        struct ulpi_regs *ulpi = (struct ulpi_regs *)0;
 +       struct ulpi_viewport ulpi_vp;

        mxc_request_iomux(stp_gpio, alt0);
        mxc_iomux_set_pad(stp_gpio, PAD_CTL_DRV_HIGH |
 @@ -133,23 +134,25 @@ static void efika_ehci_init(struct usb_ehci
 *ehci, uint32_t stp_gpio,
        mxc_iomux_set_pad(stp_gpio, USB_PAD_CONFIG);
        udelay(1);

 -       ret = ulpi_init((u32)ehci-ulpi_viewpoint);
 +       ulpi_vp.viewport_addr = (u32)ehci-ulpi_viewpoint;


 here, you should also:

 +       ulpi_vp.port_num = 0;

okay.


 because the ulpi_vp structure is not initialized and can have garbage in it.

 +
 +       ret = ulpi_init(ulpi_vp);
        if (ret) {
                printf(Efika USB ULPI initialization failed\n);
                return;
        }

        /* ULPI set flags */
 -       ulpi_write((u32)ehci-ulpi_viewpoint,ulpi-otg_ctrl,
 +       ulpi_write(ulpi_vp,ulpi-otg_ctrl,

                        ULPI_OTG_DP_PULLDOWN | ULPI_OTG_DM_PULLDOWN |
                        ULPI_OTG_EXTVBUSIND);
 -       ulpi_write((u32)ehci-ulpi_viewpoint,ulpi-function_ctrl,
 +       ulpi_write(ulpi_vp,ulpi-function_ctrl,

                        ULPI_FC_FULL_SPEED | ULPI_FC_OPMODE_NORMAL |
                        ULPI_FC_SUSPENDM);
 -       ulpi_write((u32)ehci-ulpi_viewpoint,ulpi-iface_ctrl, 0);
 +       ulpi_write(ulpi_vp,ulpi-iface_ctrl, 0);


        /* Set VBus */
 -       ulpi_write((u32)ehci-ulpi_viewpoint,ulpi-otg_ctrl_set,
 +       ulpi_write(ulpi_vp,ulpi-otg_ctrl_set,

                        ULPI_OTG_DRVVBUS | ULPI_OTG_DRVVBUS_EXT);

        /*
 @@ -158,7 +161,7 @@ static void efika_ehci_init(struct usb_ehci *ehci,
 uint32_t stp_gpio,
         * NOTE: This violates USB specification, but otherwise, USB on
 Efika
         * doesn't work.
         */
 -       ulpi_write((u32)ehci-ulpi_viewpoint,ulpi-otg_ctrl_set,
 +       ulpi_write(ulpi_vp,ulpi-otg_ctrl_set,
                        ULPI_OTG_CHRGVBUS);


 This fits 80 characters, right? Can this now be on the same line?

sure.


  }

 @@ -177,9 +180,11 @@ void ehci_powerup_fixup(uint32_t *status_reg,
 uint32_t *reg)
      

Re: [U-Boot] [PATCH v2 1/4] ehci-omap: Clean up added ehci-omap.c

2012-02-03 Thread Govindraj
On Sun, Jan 29, 2012 at 5:12 PM, Stefano Babic sba...@denx.de wrote:
 On 29/01/2012 10:42, Igor Grinberg wrote:
 Hi Govindraj,


 Hi all,

 Put Remy on Cc.

 Remy resigned as USB maintainer - but Tom is aware about these patches...


 Note: my new mail client for some reason, messes with white spaces...
 (Thunderbird (9) just gets better and better...)

 I had the opposite problem - this patch from patchwork is messy.
 Patchworks mixed some answer with the patch and it is not appliable. But
 using Thunderbird 9 I had no problem...

 So, please ignore those.
 BTW, is there anyone who has those issues fixed and can save me a wasted
 hour?

 I do not know exactly your problem, but I pushed my tree for TAM3517 on

        git.denx.de/u-boot-imx, spl_linux_twister branch

 Maybe this can help you

 diff --git a/include/configs/omap4_panda.h
 b/include/configs/omap4_panda.h
 index 23c0230..0228a66 100644
 --- a/include/configs/omap4_panda.h
 +++ b/include/configs/omap4_panda.h
 @@ -38,6 +38,8 @@
   #define CONFIG_USB_HOST
   #define CONFIG_USB_EHCI
   #define CONFIG_USB_EHCI_OMAP
 +#define CONFIG_USB_ULPI
 +#define CONFIG_USB_OMAP_ULPI_VIEWPORT
   #define CONFIG_USB_STORAGE
   #define CONFIG_SYS_USB_EHCI_MAX_ROOT_PORTS 3

 This also should apply for TAM3517 board.
 Stefano, can you test this on TAM3517?

 Tested on twister / TAM3517, I can read a USB pen.

 Tested-by: Stefano Babic sba...@denx.de

Thanks for testing I have added your tested by after splitting this
into two patches
as per Igor Grinberg grinb...@compulab.co.il comments.

--
Thanks,
Govindraj.R
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH v2 1/4] ehci-omap: Clean up added ehci-omap.c

2012-02-03 Thread Igor Grinberg
On 02/03/12 14:50, Govindraj wrote:

[...]


  COBJS := $(COBJS-y)
  SRCS  := $(COBJS:.o=.c)
 diff --git a/drivers/usb/ulpi/omap-ulpi-viewport.c
 b/drivers/usb/ulpi/omap-ulpi-viewport.c
 new file mode 100644
 index 000..1718788
 --- /dev/null
 +++ b/drivers/usb/ulpi/omap-ulpi-viewport.c
 @@ -0,0 +1,110 @@


 [...]

 +/* ULPI viewport control bits */
 +#define OMAP_ULPI_PORT_SHIFT   24
 +
 +#define OMAP_ULPI_WR_OPSEL (3  21)
 +#define OMAP_ULPI_ACCESS   (1  31)
 +
 +/*
 + * Wait for the ULPI Access to complete
 + */
 +static int ulpi_wait(struct ulpi_viewport *ulpi_vp, u32 mask)
 +{
 +   int timeout = CONFIG_USB_ULPI_TIMEOUT;
 +
 +   while (--timeout) {
 +   if ((readl(ulpi_vp-viewport_addr)  mask))

 +   return 0;
 +
 +   udelay(1);
 +   }
 +
 +   return ULPI_ERROR;
 +}
 +
 +/*
 + * Wake the ULPI PHY up for communication
 + *
 + * returns 0 on success.
 + */
 +static int ulpi_wakeup(struct ulpi_viewport *ulpi_vp)
 +{
 +   int err;
 +
 +   if (readl(ulpi_vp-viewport_addr)  OMAP_ULPI_ACCESS)

 +   return 0; /* already awake */
 +
 +   writel(OMAP_ULPI_ACCESS, ulpi_vp-viewport_addr);
 +
 +   err = ulpi_wait(ulpi_vp, OMAP_ULPI_ACCESS);
 +   if (err)
 +   debug(ULPI wakeup timed out\n);
 +
 +   return err;
 +}


 This function looks really weird...
 You write the OMAP_ULPI_ACCESS bit, but you don't specify the port...
 Now, if you will specify the port, what access type (read/write) should it
 be?
 So, I don't really know what whould be the consequence of that access...

 TRM does not talk about waking the ULPI PHY at all or have I missed it...
 I think this function is unneeded on OMAP - it looks like OMAP wakes
 the ULPI PHY automatically as soon as you access it for read/write,
 but the TRM does not talk too much on that...

 Have you tried without this function?
 Just calling ulpi_wait() to verify that the previous access has completed,
 should do the job, no? Can you, please, check that?

 
 Actually we have to start the ulpi access first, then we can write
 port number and mode to be used and after writing we can check for access
 done bit. (checking read/write op was successful can be polled for
 access done bit)
 
 without ulpi_wake access done was not happening and timing out.

This is really weird... because we actually don't know which port (PHY)
is being wakened (may be all of them...).
But if you say it does not work without it, then I'm fine with it.

Thanks Govindraj.

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


Re: [U-Boot] [PATCH v2 1/4] ehci-omap: Clean up added ehci-omap.c

2012-02-01 Thread Igor Grinberg
Hi Stefano,

On 01/29/12 13:42, Stefano Babic wrote:
 On 29/01/2012 10:42, Igor Grinberg wrote:
 Hi Govindraj,

 
 Hi all,
 
 Put Remy on Cc.
 
 Remy resigned as USB maintainer - but Tom is aware about these patches...

Right. This patch touches the USB subsystem, therefore I put Remy in Cc.

 

 Note: my new mail client for some reason, messes with white spaces...
 (Thunderbird (9) just gets better and better...)
 
 I had the opposite problem - this patch from patchwork is messy.
 Patchworks mixed some answer with the patch and it is not appliable. But
 using Thunderbird 9 I had no problem...
 
 So, please ignore those.
 BTW, is there anyone who has those issues fixed and can save me a wasted
 hour?
 
 I do not know exactly your problem, but I pushed my tree for TAM3517 on
 
   git.denx.de/u-boot-imx, spl_linux_twister branch
 
 Maybe this can help you

Thanks, actually it turned out to be some kind of dynamic libraries problem
that Thunderbird uses (probably spelling libraries...), so reinstalling
Thunderbird along with all libraries it depends on, did the trick.

 
 diff --git a/include/configs/omap4_panda.h
 b/include/configs/omap4_panda.h
 index 23c0230..0228a66 100644
 --- a/include/configs/omap4_panda.h
 +++ b/include/configs/omap4_panda.h
 @@ -38,6 +38,8 @@
   #define CONFIG_USB_HOST
   #define CONFIG_USB_EHCI
   #define CONFIG_USB_EHCI_OMAP
 +#define CONFIG_USB_ULPI
 +#define CONFIG_USB_OMAP_ULPI_VIEWPORT
   #define CONFIG_USB_STORAGE
   #define CONFIG_SYS_USB_EHCI_MAX_ROOT_PORTS 3

 This also should apply for TAM3517 board.
 Stefano, can you test this on TAM3517?
 
 Tested on twister / TAM3517, I can read a USB pen.
 
 Tested-by: Stefano Babic sba...@denx.de

Thanks for testing!

Govidraj, can the enabling the ULPI for boards reside
in a separate patch and also include the boards that were
tested with it (e.g. TAM3517)?

Thanks.


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


Re: [U-Boot] [PATCH v2 1/4] ehci-omap: Clean up added ehci-omap.c

2012-02-01 Thread Stefano Babic
On 01/02/2012 12:01, Igor Grinberg wrote:

 Thanks for testing!
 
 Govidraj, can the enabling the ULPI for boards reside
 in a separate patch and also include the boards that were
 tested with it (e.g. TAM3517)?

Well, this is maybe too much ;-)

... and Govidraj has no possibility to test it. I will send a patch for
TAM3517 as soon as Govidraj's patches will be merged, so you do not need
to worry about it !

Stefano

-- 
=
DENX Software Engineering GmbH, MD: Wolfgang Denk  Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: off...@denx.de
=
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH v2 1/4] ehci-omap: Clean up added ehci-omap.c

2012-02-01 Thread Igor Grinberg
On 02/01/12 13:26, Stefano Babic wrote:
 On 01/02/2012 12:01, Igor Grinberg wrote:
 
 Thanks for testing!

 Govidraj, can the enabling the ULPI for boards reside
 in a separate patch and also include the boards that were
 tested with it (e.g. TAM3517)?
 
 Well, this is maybe too much ;-)
 
 ... and Govidraj has no possibility to test it. I will send a patch for
 TAM3517 as soon as Govidraj's patches will be merged, so you do not need
 to worry about it !

Fare enough, let it be so :)


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


Re: [U-Boot] [PATCH v2 1/4] ehci-omap: Clean up added ehci-omap.c

2012-01-29 Thread Igor Grinberg

Hi Govindraj,

Put Remy on Cc.

Note: my new mail client for some reason, messes with white spaces...
(Thunderbird (9) just gets better and better...)
So, please ignore those.
BTW, is there anyone who has those issues fixed and can save me a wasted hour?

On 01/27/12 15:29, Govindraj wrote:

On Wed, Jan 25, 2012 at 7:10 PM, Igor Grinberggrinb...@compulab.co.il  wrote:


[...]



My suggestion for the change is:
1) introduce some kind of
struct ulpi_viewport {
u32 viewport_addr;
uint portnum;
}

2) use the above struct _instead_ of the u32 ulpi_viewport parameter

Another way, would be instead of uint portnum, use void *private_data,
but I think it will just complicate things too much and there will be no
real benefit (and also will add, otherwise needless, castings).
If the above structure will not be enough for some platform,
it can be extended easily and without changing the API anymore.



Thanks for the suggestions.

Here [1] is the first attempt to implement the same.

sorry for late reply, as I was preempted with some other work.


Thanks for the work you've done!
Overall it looks very good, though several tiny comments below.



--
Thanks,
Govindraj.R

[1]:


From 2bc311c26572aff3ebaac035106434b444692f68 Mon Sep 17 00:00:00 2001

From: Govindraj.Rgovindraj.r...@ti.com
Date: Fri, 27 Jan 2012 18:34:11 +0530
Subject: [PATCH] ulpi: Modify the ulpi framework to accept port number

Based on discussion in the thread [1] this patch
attempts to modify the ulpi framework to accept the
port number along with view port address, adding the omap-ulpi
view port that helps us in using the generic ulpi
framework to configure ulpi phy using the INSNREG05_ULPI viewport
reg available on omap platform.


Ok, I think you should split this patch to:
1) modify the ulpi framework... (this one extends the ulpi functionality
   and fixes current users)
2) add omap ulpi support (this one makes use of the ulpi framework
   on omap platforms and IMO, should amended to you original
   1/4 patch: ehci-omap: Clean up added ehci-omap.c)



Tested on beagle-xm and Panda.
Compile tested on efikamx platform.

This patch is based on earlier posted patch series[1].


Shouldn't this be [2]?



Prior to this patch ulpi reset was done directly now
it is done using ulpi_reset.

Thanks to Igor Grinberggrinb...@compulab.co.il  for reviewing the
omap-ehci patches and suggesting this approach.

This patch along with the patch series [1] rebased on latest u-boot


Probably same here, or am I mistaken?


is available here.
git://gitorious.org/denx_u-boot/denx_uboot_omap.git v2_ehci_omap4

[1]: http://www.mail-archive.com/u-boot@lists.denx.de/msg76076.html
[2]: [PATCH v2 0/4] Clean up ehci-omap and extend support for omap3/4 socs

Signed-off-by: Govindraj.Rgovindraj.r...@ti.com
---
  board/efikamx/efikamx-usb.c   |   21 ---
  drivers/usb/host/ehci-omap.c  |   21 ++-
  drivers/usb/ulpi/Makefile |1 +
  drivers/usb/ulpi/omap-ulpi-viewport.c |  110 +
  drivers/usb/ulpi/ulpi-viewport.c  |   30 +-
  drivers/usb/ulpi/ulpi.c   |   54 
  include/configs/omap3_beagle.h|3 +
  include/configs/omap4_panda.h |2 +
  include/usb/ulpi.h|   37 


You should also, update the README file with the new
CONFIG_USB_ULPI_VIEWPORT_OMAP option.
(Also may be some OMAP specific doc files..)



  9 files changed, 202 insertions(+), 77 deletions(-)
  create mode 100644 drivers/usb/ulpi/omap-ulpi-viewport.c

diff --git a/board/efikamx/efikamx-usb.c b/board/efikamx/efikamx-usb.c
index 840bd9a..bb1398b 100644
--- a/board/efikamx/efikamx-usb.c
+++ b/board/efikamx/efikamx-usb.c
@@ -120,6 +120,7 @@ static void efika_ehci_init(struct usb_ehci *ehci,
uint32_t stp_gpio,
  {
int ret;
struct ulpi_regs *ulpi = (struct ulpi_regs *)0;
+   struct ulpi_viewport ulpi_vp;

mxc_request_iomux(stp_gpio, alt0);
mxc_iomux_set_pad(stp_gpio, PAD_CTL_DRV_HIGH |
@@ -133,23 +134,25 @@ static void efika_ehci_init(struct usb_ehci
*ehci, uint32_t stp_gpio,
mxc_iomux_set_pad(stp_gpio, USB_PAD_CONFIG);
udelay(1);

-   ret = ulpi_init((u32)ehci-ulpi_viewpoint);
+   ulpi_vp.viewport_addr = (u32)ehci-ulpi_viewpoint;


here, you should also:

+   ulpi_vp.port_num = 0;

because the ulpi_vp structure is not initialized and can have garbage in it.


+
+   ret = ulpi_init(ulpi_vp);
if (ret) {
printf(Efika USB ULPI initialization failed\n);
return;
}

/* ULPI set flags */
-   ulpi_write((u32)ehci-ulpi_viewpoint,ulpi-otg_ctrl,
+   ulpi_write(ulpi_vp,ulpi-otg_ctrl,
ULPI_OTG_DP_PULLDOWN | ULPI_OTG_DM_PULLDOWN |
ULPI_OTG_EXTVBUSIND);
-   ulpi_write((u32)ehci-ulpi_viewpoint,ulpi-function_ctrl,
+   ulpi_write(ulpi_vp,ulpi-function_ctrl,
   

Re: [U-Boot] [PATCH v2 1/4] ehci-omap: Clean up added ehci-omap.c

2012-01-29 Thread Stefano Babic
On 29/01/2012 10:42, Igor Grinberg wrote:
 Hi Govindraj,
 

Hi all,

 Put Remy on Cc.

Remy resigned as USB maintainer - but Tom is aware about these patches...

 
 Note: my new mail client for some reason, messes with white spaces...
 (Thunderbird (9) just gets better and better...)

I had the opposite problem - this patch from patchwork is messy.
Patchworks mixed some answer with the patch and it is not appliable. But
using Thunderbird 9 I had no problem...

 So, please ignore those.
 BTW, is there anyone who has those issues fixed and can save me a wasted
 hour?

I do not know exactly your problem, but I pushed my tree for TAM3517 on

git.denx.de/u-boot-imx, spl_linux_twister branch

Maybe this can help you

 diff --git a/include/configs/omap4_panda.h
 b/include/configs/omap4_panda.h
 index 23c0230..0228a66 100644
 --- a/include/configs/omap4_panda.h
 +++ b/include/configs/omap4_panda.h
 @@ -38,6 +38,8 @@
   #define CONFIG_USB_HOST
   #define CONFIG_USB_EHCI
   #define CONFIG_USB_EHCI_OMAP
 +#define CONFIG_USB_ULPI
 +#define CONFIG_USB_OMAP_ULPI_VIEWPORT
   #define CONFIG_USB_STORAGE
   #define CONFIG_SYS_USB_EHCI_MAX_ROOT_PORTS 3
 
 This also should apply for TAM3517 board.
 Stefano, can you test this on TAM3517?

Tested on twister / TAM3517, I can read a USB pen.

Tested-by: Stefano Babic sba...@denx.de

Best regards,
Stefano Babic

-- 
=
DENX Software Engineering GmbH, MD: Wolfgang Denk  Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: off...@denx.de
=
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH v2 1/4] ehci-omap: Clean up added ehci-omap.c

2012-01-27 Thread Govindraj
On Wed, Jan 25, 2012 at 7:10 PM, Igor Grinberg grinb...@compulab.co.il wrote:
 Hi Govindraj,

 On 01/25/12 11:04, Govindraj wrote:
 Hi Igor,

 On Sun, Jan 22, 2012 at 5:50 PM, Igor Grinberg grinb...@compulab.co.il 
 wrote:
 On 01/19/12 10:15, Govindraj wrote:
 On Wed, Jan 18, 2012 at 11:21 PM, Igor Grinberg grinb...@compulab.co.il 
 wrote:
 Hi Govindraj,

 On 01/17/12 08:10, Govindraj wrote:


 [...]

 and add an ability to pass some kind of private data to the viewport, 
 which
 in case of OMAP will be the port number.


 I started on adding omap-ulpi-viewport.c which will work with ulpi.c
 if omap_ehci.c is used.

 Good! Thanks for working on that.


 for port id can we just set a global data field that will inform the
 omap_view port
 on the port id, or we have to modify most api's syntax in
 drivers/usb/ulpi/ulpi.c

 IMO, we should modify the API, because it does not make sense
 to use the generic layer, but workaround some missing feature...

 My suggestion for the change is:
 1) introduce some kind of
 struct ulpi_viewport {
        u32 viewport_addr;
        uint portnum;
 }

 2) use the above struct _instead_ of the u32 ulpi_viewport parameter

 Another way, would be instead of uint portnum, use void *private_data,
 but I think it will just complicate things too much and there will be no
 real benefit (and also will add, otherwise needless, castings).
 If the above structure will not be enough for some platform,
 it can be extended easily and without changing the API anymore.


Thanks for the suggestions.

Here [1] is the first attempt to implement the same.

sorry for late reply, as I was preempted with some other work.

--
Thanks,
Govindraj.R

[1]:

From 2bc311c26572aff3ebaac035106434b444692f68 Mon Sep 17 00:00:00 2001
From: Govindraj.R govindraj.r...@ti.com
Date: Fri, 27 Jan 2012 18:34:11 +0530
Subject: [PATCH] ulpi: Modify the ulpi framework to accept port number

Based on discussion in the thread [1] this patch
attempts to modify the ulpi framework to accept the
port number along with view port address, adding the omap-ulpi
view port that helps us in using the generic ulpi
framework to configure ulpi phy using the INSNREG05_ULPI viewport
reg available on omap platform.

Tested on beagle-xm and Panda.
Compile tested on efikamx platform.

This patch is based on earlier posted patch series[1].

Prior to this patch ulpi reset was done directly now
it is done using ulpi_reset.

Thanks to Igor Grinberg grinb...@compulab.co.il for reviewing the
omap-ehci patches and suggesting this approach.

This patch along with the patch series [1] rebased on latest u-boot
is available here.
git://gitorious.org/denx_u-boot/denx_uboot_omap.git v2_ehci_omap4

[1]: http://www.mail-archive.com/u-boot@lists.denx.de/msg76076.html
[2]: [PATCH v2 0/4] Clean up ehci-omap and extend support for omap3/4 socs

Signed-off-by: Govindraj.R govindraj.r...@ti.com
---
 board/efikamx/efikamx-usb.c   |   21 ---
 drivers/usb/host/ehci-omap.c  |   21 ++-
 drivers/usb/ulpi/Makefile |1 +
 drivers/usb/ulpi/omap-ulpi-viewport.c |  110 +
 drivers/usb/ulpi/ulpi-viewport.c  |   30 +-
 drivers/usb/ulpi/ulpi.c   |   54 
 include/configs/omap3_beagle.h|3 +
 include/configs/omap4_panda.h |2 +
 include/usb/ulpi.h|   37 
 9 files changed, 202 insertions(+), 77 deletions(-)
 create mode 100644 drivers/usb/ulpi/omap-ulpi-viewport.c

diff --git a/board/efikamx/efikamx-usb.c b/board/efikamx/efikamx-usb.c
index 840bd9a..bb1398b 100644
--- a/board/efikamx/efikamx-usb.c
+++ b/board/efikamx/efikamx-usb.c
@@ -120,6 +120,7 @@ static void efika_ehci_init(struct usb_ehci *ehci,
uint32_t stp_gpio,
 {
int ret;
struct ulpi_regs *ulpi = (struct ulpi_regs *)0;
+   struct ulpi_viewport ulpi_vp;

mxc_request_iomux(stp_gpio, alt0);
mxc_iomux_set_pad(stp_gpio, PAD_CTL_DRV_HIGH |
@@ -133,23 +134,25 @@ static void efika_ehci_init(struct usb_ehci
*ehci, uint32_t stp_gpio,
mxc_iomux_set_pad(stp_gpio, USB_PAD_CONFIG);
udelay(1);

-   ret = ulpi_init((u32)ehci-ulpi_viewpoint);
+   ulpi_vp.viewport_addr = (u32)ehci-ulpi_viewpoint;
+
+   ret = ulpi_init(ulpi_vp);
if (ret) {
printf(Efika USB ULPI initialization failed\n);
return;
}

/* ULPI set flags */
-   ulpi_write((u32)ehci-ulpi_viewpoint, ulpi-otg_ctrl,
+   ulpi_write(ulpi_vp, ulpi-otg_ctrl,
ULPI_OTG_DP_PULLDOWN | ULPI_OTG_DM_PULLDOWN |
ULPI_OTG_EXTVBUSIND);
-   ulpi_write((u32)ehci-ulpi_viewpoint, ulpi-function_ctrl,
+   ulpi_write(ulpi_vp, ulpi-function_ctrl,
ULPI_FC_FULL_SPEED | ULPI_FC_OPMODE_NORMAL |
ULPI_FC_SUSPENDM);
-   ulpi_write((u32)ehci-ulpi_viewpoint, ulpi-iface_ctrl, 0);
+   ulpi_write(ulpi_vp, 

Re: [U-Boot] [PATCH v2 1/4] ehci-omap: Clean up added ehci-omap.c

2012-01-25 Thread Govindraj
Hi Igor,

On Sun, Jan 22, 2012 at 5:50 PM, Igor Grinberg grinb...@compulab.co.il wrote:
 On 01/19/12 10:15, Govindraj wrote:
 On Wed, Jan 18, 2012 at 11:21 PM, Igor Grinberg grinb...@compulab.co.il 
 wrote:
 Hi Govindraj,

 On 01/17/12 08:10, Govindraj wrote:


[...]

 +             if (get_timer(init)  CONFIG_SYS_HZ) {
 +                     debug(OMAP EHCI error: timeout resetting phy\n);
 +                     break;
 +             }
 +}

 Ok, this function is kind of duplication of the ULPI code.
 ulpi_reset() function in drivers/usb/ulpi/ulpi.c provides an implementation
 of the ULPI spec. and should be used by the drivers.
 What it lacks currently, is a way to pass a port number to the viewport
 implementation and of course the omap-ulpi-viewport(.c) implementation 
 itself...
 So, IMO, the right way would be to implement ULPI accessors 
 (omap-ulpi-viewport.c)

 so you mean add omap-ulpi-viewport.c which will do ulpi read writes
 for ulpi implementation
 within tll module of omap host controller.

 No. What I meant is that omap-ulpi-viewport.c will do the ULPI access
 to the PHY (which is not TLL), but after the above question, I think
 it can do both: TLL and non-TLL.


 we just need func reset to be done for ulpi which is done using ehci register
 INSNREG05_ULPI. IMHO I don't see any use case or true requirement of
 omap-ulpi-viewport.c framework.

 The fact that the reset is done by writing the ULPI_FUNC_CTRL_RESET bit
 of the ULPI_FUNC_CTRL register - is the requirement...
 For example tomorrow, you will find out that besides reset, you also need
 to set some other bit in a register inside the ULPI PHY (e.g. VBUS), so
 you will implement another ehci function that will do a write to ULPI
 and thus duplicate another portion of code...


 and add an ability to pass some kind of private data to the viewport, which
 in case of OMAP will be the port number.


I started on adding omap-ulpi-viewport.c which will work with ulpi.c
if omap_ehci.c is used.

for port id can we just set a global data field that will inform the
omap_view port
on the port id, or we have to modify most api's syntax in
drivers/usb/ulpi/ulpi.c

Is it okay to have the port id from ehci-omap.c set and used in
drivers/usb/ulpi/omap-ulpi-viewport.c ?

--
Thanks,
Govindraj.R
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH v2 1/4] ehci-omap: Clean up added ehci-omap.c

2012-01-25 Thread Igor Grinberg
Hi Govindraj,

On 01/25/12 11:04, Govindraj wrote:
 Hi Igor,
 
 On Sun, Jan 22, 2012 at 5:50 PM, Igor Grinberg grinb...@compulab.co.il 
 wrote:
 On 01/19/12 10:15, Govindraj wrote:
 On Wed, Jan 18, 2012 at 11:21 PM, Igor Grinberg grinb...@compulab.co.il 
 wrote:
 Hi Govindraj,

 On 01/17/12 08:10, Govindraj wrote:


[...]

 and add an ability to pass some kind of private data to the viewport, which
 in case of OMAP will be the port number.
 
 
 I started on adding omap-ulpi-viewport.c which will work with ulpi.c
 if omap_ehci.c is used.

Good! Thanks for working on that.

 
 for port id can we just set a global data field that will inform the
 omap_view port
 on the port id, or we have to modify most api's syntax in
 drivers/usb/ulpi/ulpi.c

IMO, we should modify the API, because it does not make sense
to use the generic layer, but workaround some missing feature...

My suggestion for the change is:
1) introduce some kind of
struct ulpi_viewport {
u32 viewport_addr;
uint portnum;
}

2) use the above struct _instead_ of the u32 ulpi_viewport parameter

Another way, would be instead of uint portnum, use void *private_data,
but I think it will just complicate things too much and there will be no
real benefit (and also will add, otherwise needless, castings).
If the above structure will not be enough for some platform,
it can be extended easily and without changing the API anymore.

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


Re: [U-Boot] [PATCH v2 1/4] ehci-omap: Clean up added ehci-omap.c

2012-01-22 Thread Igor Grinberg
On 01/19/12 10:15, Govindraj wrote:
 On Wed, Jan 18, 2012 at 11:21 PM, Igor Grinberg grinb...@compulab.co.il 
 wrote:
 Hi Govindraj,

 On 01/17/12 08:10, Govindraj wrote:

 And just to clarify further there is no code duplication for ulpi read 
 writes
 in ehci-omap.c done with this patch.

 This is not just about code duplication,
 this is also about using the ULPI framework instead of direct writes to the
 ULPI PHYs.

 Patch intends only in configuring ehci to right modes as specified by
 board file.  Modes possible are hsic_mode, tll_mode, ulpi_phy mode.

 This patch is derived with reference to linux kernel and even there
 we can see that no ulpi_reg map registers are used and only ehci
 is configured is respective mode as passed by board data.

 This is not correct. They are used, see:
 drivers/usb/host/ehci-omap.c file, omap_ehci_soft_phy_reset() function.
 It does exactly the same, but yes it does not use the ULPI framework,
 because in Linux currently, there is no generic enough ULPI framework,
 so drivers can use it.
 
 linux-2.6/drivers/usb/otg/ulpi.c ?

No, this is a kind of attempt to implement the ULPI framework,
but not really a good one... and that is the reason why no driver inside
drivers/usb/otg/ is using it...

 
 There were discussions about it and decided that until there is such
 a framework, things can be done the way they are.


 Here [1] is the patch fixing Marek Vasut marek.va...@gmail.com
 comments.

 This corrected patch along with dependent patch from
 Ilya Yanok ya...@emcraft.com
 [PATCH V4 1/2] ehci-omap: driver for EHCI host on OMAP3

 Is available here:
 git://gitorious.org/denx_u-boot/denx_uboot_omap.git v2_ehci_omap

 --
 Thanks,
 Govindraj.R

 [1]:

 From 56b1b94128495ed4bf83e2f20f3884833e2aa082 Mon Sep 17 00:00:00 2001
 From: Govindraj.R govindraj.r...@ti.com
 Date: Tue, 27 Dec 2011 14:53:12 +0530
 Subject: [PATCH 2/6] ehci-omap: Clean up added ehci-omap.c

 Clean up added ehci-omap.c and make it generic for re-use across
 soc having same ehci ip block. Also pass the modes to be configured
 and configure the ports accordingly. All usb layers are not cache
 aligned till then keep cache off for usb ops as ehci will use
 internally dma for all usb ops.

 * Add a generic common header ehci-omap.h having common ip block
   data and reg shifts.
 * Rename and modify ehci-omap3 to ehci.h retain only conflicting
   sysc reg shifts remove others and move to common header file.

 Signed-off-by: Govindraj.R govindraj.r...@ti.com
 ---
  arch/arm/include/asm/arch-omap3/ehci.h   |   55 ++
  arch/arm/include/asm/arch-omap3/ehci_omap3.h |   58 ---
  arch/arm/include/asm/arch-omap4/ehci.h   |   49 ++
  arch/arm/include/asm/ehci-omap.h |  147 +
  drivers/usb/host/ehci-omap.c |  228 
 +++---
  5 files changed, 423 insertions(+), 114 deletions(-)
  create mode 100644 arch/arm/include/asm/arch-omap3/ehci.h
  delete mode 100644 arch/arm/include/asm/arch-omap3/ehci_omap3.h
  create mode 100644 arch/arm/include/asm/arch-omap4/ehci.h
  create mode 100644 arch/arm/include/asm/ehci-omap.h

 diff --git a/arch/arm/include/asm/arch-omap3/ehci.h
 b/arch/arm/include/asm/arch-omap3/ehci.h
 new file mode 100644
 index 000..d622363
 --- /dev/null
 +++ b/arch/arm/include/asm/arch-omap3/ehci.h
 @@ -0,0 +1,55 @@
 +/*
 + * (C) Copyright 2011
 + * Alexander Holler hol...@ahsoftware.de
 + *
 + * Based on drivers/usb/host/ehci-omap.c from Linux 2.6.37
 + *
 + * See there for additional Copyrights.
 + *
 + * See file CREDITS for list of people who contributed to this
 + * project.
 + *
 + * This program is free software; you can redistribute it and/or
 + * modify it under the terms of the GNU General Public License as
 + * published by the Free Software Foundation; either version 2 of
 + * the License, or (at your option) any later version.
 + *
 + * This program is distributed in the hope that it will be useful,
 + * but WITHOUT ANY WARRANTY; without even the implied warranty of
 + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
 + * GNU General Public License for more details.
 + *
 + * You should have received a copy of the GNU General Public License
 + * along with this program; if not, write to the Free Software
 + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston,
 + * MA 02110-1301 USA
 + */
 +#ifndef _EHCI_H_
 +#define _EHCI_H_

 hmmm... isn't it too generic?
 I think you should have included a part of the path to the file,
 so no namespace collision will occure.
 something like _OMAP3_EHCI_H_?

 
 since these include's are part of arch folder
 will not collide with other ones.
 
 But still its better to have _OMAP3_EHCI_H_
 
 will correct this.
 
 
 +
 +/* USB/EHCI registers */
 +#define OMAP_USBTLL_BASE 0x48062000UL
 +#define OMAP_UHH_BASE0x48064000UL
 +#define OMAP_EHCI_BASE   

Re: [U-Boot] [PATCH v2 1/4] ehci-omap: Clean up added ehci-omap.c

2012-01-19 Thread Govindraj
On Wed, Jan 18, 2012 at 11:21 PM, Igor Grinberg grinb...@compulab.co.il wrote:
 Hi Govindraj,

 On 01/17/12 08:10, Govindraj wrote:

 And just to clarify further there is no code duplication for ulpi read writes
 in ehci-omap.c done with this patch.

 This is not just about code duplication,
 this is also about using the ULPI framework instead of direct writes to the
 ULPI PHYs.

 Patch intends only in configuring ehci to right modes as specified by
 board file.  Modes possible are hsic_mode, tll_mode, ulpi_phy mode.

 This patch is derived with reference to linux kernel and even there
 we can see that no ulpi_reg map registers are used and only ehci
 is configured is respective mode as passed by board data.

 This is not correct. They are used, see:
 drivers/usb/host/ehci-omap.c file, omap_ehci_soft_phy_reset() function.
 It does exactly the same, but yes it does not use the ULPI framework,
 because in Linux currently, there is no generic enough ULPI framework,
 so drivers can use it.

linux-2.6/drivers/usb/otg/ulpi.c ?

 There were discussions about it and decided that until there is such
 a framework, things can be done the way they are.


 Here [1] is the patch fixing Marek Vasut marek.va...@gmail.com
 comments.

 This corrected patch along with dependent patch from
 Ilya Yanok ya...@emcraft.com
 [PATCH V4 1/2] ehci-omap: driver for EHCI host on OMAP3

 Is available here:
 git://gitorious.org/denx_u-boot/denx_uboot_omap.git v2_ehci_omap

 --
 Thanks,
 Govindraj.R

 [1]:

From 56b1b94128495ed4bf83e2f20f3884833e2aa082 Mon Sep 17 00:00:00 2001
 From: Govindraj.R govindraj.r...@ti.com
 Date: Tue, 27 Dec 2011 14:53:12 +0530
 Subject: [PATCH 2/6] ehci-omap: Clean up added ehci-omap.c

 Clean up added ehci-omap.c and make it generic for re-use across
 soc having same ehci ip block. Also pass the modes to be configured
 and configure the ports accordingly. All usb layers are not cache
 aligned till then keep cache off for usb ops as ehci will use
 internally dma for all usb ops.

 * Add a generic common header ehci-omap.h having common ip block
   data and reg shifts.
 * Rename and modify ehci-omap3 to ehci.h retain only conflicting
   sysc reg shifts remove others and move to common header file.

 Signed-off-by: Govindraj.R govindraj.r...@ti.com
 ---
  arch/arm/include/asm/arch-omap3/ehci.h       |   55 ++
  arch/arm/include/asm/arch-omap3/ehci_omap3.h |   58 ---
  arch/arm/include/asm/arch-omap4/ehci.h       |   49 ++
  arch/arm/include/asm/ehci-omap.h             |  147 +
  drivers/usb/host/ehci-omap.c                 |  228 
 +++---
  5 files changed, 423 insertions(+), 114 deletions(-)
  create mode 100644 arch/arm/include/asm/arch-omap3/ehci.h
  delete mode 100644 arch/arm/include/asm/arch-omap3/ehci_omap3.h
  create mode 100644 arch/arm/include/asm/arch-omap4/ehci.h
  create mode 100644 arch/arm/include/asm/ehci-omap.h

 diff --git a/arch/arm/include/asm/arch-omap3/ehci.h
 b/arch/arm/include/asm/arch-omap3/ehci.h
 new file mode 100644
 index 000..d622363
 --- /dev/null
 +++ b/arch/arm/include/asm/arch-omap3/ehci.h
 @@ -0,0 +1,55 @@
 +/*
 + * (C) Copyright 2011
 + * Alexander Holler hol...@ahsoftware.de
 + *
 + * Based on drivers/usb/host/ehci-omap.c from Linux 2.6.37
 + *
 + * See there for additional Copyrights.
 + *
 + * See file CREDITS for list of people who contributed to this
 + * project.
 + *
 + * This program is free software; you can redistribute it and/or
 + * modify it under the terms of the GNU General Public License as
 + * published by the Free Software Foundation; either version 2 of
 + * the License, or (at your option) any later version.
 + *
 + * This program is distributed in the hope that it will be useful,
 + * but WITHOUT ANY WARRANTY; without even the implied warranty of
 + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
 + * GNU General Public License for more details.
 + *
 + * You should have received a copy of the GNU General Public License
 + * along with this program; if not, write to the Free Software
 + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston,
 + * MA 02110-1301 USA
 + */
 +#ifndef _EHCI_H_
 +#define _EHCI_H_

 hmmm... isn't it too generic?
 I think you should have included a part of the path to the file,
 so no namespace collision will occure.
 something like _OMAP3_EHCI_H_?


since these include's are part of arch folder
will not collide with other ones.

But still its better to have _OMAP3_EHCI_H_

will correct this.


 +
 +/* USB/EHCI registers */
 +#define OMAP_USBTLL_BASE                             0x48062000UL
 +#define OMAP_UHH_BASE                                        0x48064000UL
 +#define OMAP_EHCI_BASE                                       0x48064800UL
 +
 +/* TLL Register Set */
 +#define OMAP_USBTLL_SYSCONFIG_SOFTRESET                      (1  1)
 +#define OMAP_USBTLL_SYSCONFIG_ENAWAKEUP                      (1  2)
 +#define OMAP_USBTLL_SYSCONFIG_SIDLEMODE              

Re: [U-Boot] [PATCH v2 1/4] ehci-omap: Clean up added ehci-omap.c

2012-01-18 Thread Igor Grinberg
Hi Govindraj,

On 01/17/12 08:10, Govindraj wrote:
 
 And just to clarify further there is no code duplication for ulpi read writes
 in ehci-omap.c done with this patch.

This is not just about code duplication,
this is also about using the ULPI framework instead of direct writes to the
ULPI PHYs.

 Patch intends only in configuring ehci to right modes as specified by
 board file.  Modes possible are hsic_mode, tll_mode, ulpi_phy mode.
 
 This patch is derived with reference to linux kernel and even there
 we can see that no ulpi_reg map registers are used and only ehci
 is configured is respective mode as passed by board data.

This is not correct. They are used, see:
drivers/usb/host/ehci-omap.c file, omap_ehci_soft_phy_reset() function.
It does exactly the same, but yes it does not use the ULPI framework,
because in Linux currently, there is no generic enough ULPI framework,
so drivers can use it.
There were discussions about it and decided that until there is such
a framework, things can be done the way they are.

 
 Here [1] is the patch fixing Marek Vasut marek.va...@gmail.com
 comments.
 
 This corrected patch along with dependent patch from
 Ilya Yanok ya...@emcraft.com
 [PATCH V4 1/2] ehci-omap: driver for EHCI host on OMAP3
 
 Is available here:
 git://gitorious.org/denx_u-boot/denx_uboot_omap.git v2_ehci_omap
 
 --
 Thanks,
 Govindraj.R
 
 [1]:
 
From 56b1b94128495ed4bf83e2f20f3884833e2aa082 Mon Sep 17 00:00:00 2001
 From: Govindraj.R govindraj.r...@ti.com
 Date: Tue, 27 Dec 2011 14:53:12 +0530
 Subject: [PATCH 2/6] ehci-omap: Clean up added ehci-omap.c
 
 Clean up added ehci-omap.c and make it generic for re-use across
 soc having same ehci ip block. Also pass the modes to be configured
 and configure the ports accordingly. All usb layers are not cache
 aligned till then keep cache off for usb ops as ehci will use
 internally dma for all usb ops.
 
 * Add a generic common header ehci-omap.h having common ip block
   data and reg shifts.
 * Rename and modify ehci-omap3 to ehci.h retain only conflicting
   sysc reg shifts remove others and move to common header file.
 
 Signed-off-by: Govindraj.R govindraj.r...@ti.com
 ---
  arch/arm/include/asm/arch-omap3/ehci.h   |   55 ++
  arch/arm/include/asm/arch-omap3/ehci_omap3.h |   58 ---
  arch/arm/include/asm/arch-omap4/ehci.h   |   49 ++
  arch/arm/include/asm/ehci-omap.h |  147 +
  drivers/usb/host/ehci-omap.c |  228 
 +++---
  5 files changed, 423 insertions(+), 114 deletions(-)
  create mode 100644 arch/arm/include/asm/arch-omap3/ehci.h
  delete mode 100644 arch/arm/include/asm/arch-omap3/ehci_omap3.h
  create mode 100644 arch/arm/include/asm/arch-omap4/ehci.h
  create mode 100644 arch/arm/include/asm/ehci-omap.h
 
 diff --git a/arch/arm/include/asm/arch-omap3/ehci.h
 b/arch/arm/include/asm/arch-omap3/ehci.h
 new file mode 100644
 index 000..d622363
 --- /dev/null
 +++ b/arch/arm/include/asm/arch-omap3/ehci.h
 @@ -0,0 +1,55 @@
 +/*
 + * (C) Copyright 2011
 + * Alexander Holler hol...@ahsoftware.de
 + *
 + * Based on drivers/usb/host/ehci-omap.c from Linux 2.6.37
 + *
 + * See there for additional Copyrights.
 + *
 + * See file CREDITS for list of people who contributed to this
 + * project.
 + *
 + * This program is free software; you can redistribute it and/or
 + * modify it under the terms of the GNU General Public License as
 + * published by the Free Software Foundation; either version 2 of
 + * the License, or (at your option) any later version.
 + *
 + * This program is distributed in the hope that it will be useful,
 + * but WITHOUT ANY WARRANTY; without even the implied warranty of
 + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
 + * GNU General Public License for more details.
 + *
 + * You should have received a copy of the GNU General Public License
 + * along with this program; if not, write to the Free Software
 + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston,
 + * MA 02110-1301 USA
 + */
 +#ifndef _EHCI_H_
 +#define _EHCI_H_

hmmm... isn't it too generic?
I think you should have included a part of the path to the file,
so no namespace collision will occure.
something like _OMAP3_EHCI_H_?

 +
 +/* USB/EHCI registers */
 +#define OMAP_USBTLL_BASE 0x48062000UL
 +#define OMAP_UHH_BASE0x48064000UL
 +#define OMAP_EHCI_BASE   0x48064800UL
 +
 +/* TLL Register Set */
 +#define OMAP_USBTLL_SYSCONFIG_SOFTRESET  (1  1)
 +#define OMAP_USBTLL_SYSCONFIG_ENAWAKEUP  (1  2)
 +#define OMAP_USBTLL_SYSCONFIG_SIDLEMODE  (1  3)
 +#define OMAP_USBTLL_SYSCONFIG_CACTIVITY  (1  8)
 +#define OMAP_USBTLL_SYSSTATUS_RESETDONE  1
 +
 +/* UHH Register Set */
 +#define OMAP_UHH_SYSCONFIG_SOFTRESET (1  1)
 +#define 

Re: [U-Boot] [PATCH v2 1/4] ehci-omap: Clean up added ehci-omap.c

2012-01-17 Thread Igor Grinberg
Hi Govindraj,

I'm sorry it took me so much time, I just was very busy these days...
I've looked in the TRMs of both OMAP3 and OMAP4, please see below.

On 01/12/12 12:52, Govindraj wrote:
 On Thu, Jan 12, 2012 at 2:53 PM, Igor Grinberg grinb...@compulab.co.il 
 wrote:
 Hi Govindraj,

 On 01/12/12 07:45, Govindraj wrote:
 Hi Igor,

 On Wed, Jan 11, 2012 at 8:33 PM, Igor Grinberg grinb...@compulab.co.il 
 wrote:
 Hi Guys,

 On 01/11/12 16:34, Marek Vasut wrote:
 On Wed, Jan 11, 2012 at 6:58 PM, Marek Vasut marek.va...@gmail.com 
 wrote:
 On Wed, Jan 11, 2012 at 6:16 PM, Marek Vasut marek.va...@gmail.com 
 wrote:
 On Wed, Jan 11, 2012 at 4:22 PM, Marek Vasut marek.va...@gmail.com
 wrote:
 Hi Marek,

 Thanks for you review.

 On Tue, Jan 10, 2012 at 9:37 PM, Marek Vasut
 marek.va...@gmail.com

 wrote:
 From: Govindraj.R govindraj.r...@ti.com

 Clean up added ehci-omap.c and make it generic for re-use
 across soc having same ehci ip block. Also pass the modes to
 be configured and configure the ports accordingly. All usb
 layers are not cache aligned till then keep cache off for usb
 ops as ehci will use internally dma for all usb ops.

 * Add a generic common header ehci-omap.h having common ip
 block data and reg shifts.
 * Rename and modify ehci-omap3 to ehci.h retain only
 conflicting sysc reg shifts remove others and move to common
 header file.

 Don't reimplement the ulpi stuff ... there's already some ulpi
 stuff in uboot that needs fixing, so fix it and use it.

Why do you keep saying that it needs fixing?
Can you point to a specific problem?


 I am not implementing any ulpi stuff I am just configuring OMAP on
 soc usb host controller (ehci). All the configuration stuff
 is OMAP specific things which are done in ehci-omap.c file

Yes, it is OMAP specific, but only the INSNREG05_ULPI register part...


 stuffs done are like soft-reset, port mode to be used and putting
 port in no -idle mode(omap specific pm implementation) etc.

Well, the soft-reset, does not fit here...
The soft reset is defined by the ULPI spec and there is already an
implementation for this (as defined by the ULPI spec.)


 This stuff:

 +/* ULPI */
 +#define ULPI_SET(a)(a + 1)
 +#define ULPI_CLR(a)(a + 2)
 +#define ULPI_FUNC_CTRL 0x04
 +#define ULPI_FUNC_CTRL_RESET   (1  5)

 is just accidentally conforming to ULPI spec?

 These are for configuring INSNREG05_ULPI reg in EHCI reg map
 of omap while configuring in ulpi-phy mode.

By writing to the INSNREG05_ULPI register, you do not configure it,
but rather initiate the ULPI transaction to the ULPI PHY.


 looking into struct ulpi_regs {..}
 then it doesn't map this configuration.

ulpi_regs {...} provides a kind of virtual mapping (to conform to U-Boot
way of accessing the registers) for the ULPI registers inside the ULPI PHY,
which are accessible in a platform specific manner which is sometimes called
a ULPI viewport.
In case of OMAP, the ULPI viewport is the INSNREG05_ULPI register.


 Can you point me to some documentation about this please? It's not
 that I don't trust you, I'd rather prefer to avoid unnecessary
 duplication.

 Yes that would be fine.

 You can download the omap4460 public trm from here:

 http://www.ti.com/pdfs/wtbu/OMAP4460_ES.1x_PUBLIC_TRM_vM.zip

 Go to chapter 23.11.6.6.1 EHCI Register Summary
 (page number 5171 and 5186/87)

 Sure, but the macro above looks more like 23.11.6.3, doesn't it ? And 
 for
 that purpose, the struct ulpi_regs is fitting ok.

No, Marek, there is another thing inside OMAP, which is called TLL.
It provides a kind of virtual ULPI interface and can be used instead of ULPI
PHY for the on-board connected devices.
TLL is not related to this discussion.


 Actually ... can you check the ulpi_read and ulpi_write stuff that's
 already in u-boot and explain why they can not be used with this port?

Marek, OMAP has a different ULPI viewport register structure, so
to reuse the ULPI layer, the omap-ulpi-viewport.c should be implemented
with its own ULPI accessors (e.g. ulpi_{read|write}()).


 echi-omap.c is no where writing to those registers
 and the macro was used only to configure INSNREG05_ULPI reg in EHCI reg 
 map

This is not exactly true...
Indeed, the INSNREG05_ULPI register is in EHCI registers address space.
In fact it does not meter where that register is located.

After those macros was written to the INSNREG05_ULPI register,
the ULPI transaction is issued to write the value inside the ULPI register.


 reg map in 23.11.6.3 used only for a external ulpi-phy communication.
 and debug purpose(to view vid, pid etc) and to hack external phy
 configuration through ulpi commands

Almost... TLL is exactly the opposite... It is a PHY-less configuration.
It is the TLL ULPI pseudo registers and it is not related to the discussion.

 from omap - usb host controller point of view only needs
 INSNREG05_ULPI reg in 

Re: [U-Boot] [PATCH v2 1/4] ehci-omap: Clean up added ehci-omap.c

2012-01-16 Thread Govindraj
On Thu, Jan 12, 2012 at 4:22 PM, Govindraj govindraj...@gmail.com wrote:
 On Thu, Jan 12, 2012 at 2:53 PM, Igor Grinberg grinb...@compulab.co.il 
 wrote:
 Hi Govindraj,

 On 01/12/12 07:45, Govindraj wrote:
 Hi Igor,

 On Wed, Jan 11, 2012 at 8:33 PM, Igor Grinberg grinb...@compulab.co.il 
 wrote:
 Hi Guys,

 On 01/11/12 16:34, Marek Vasut wrote:
 On Wed, Jan 11, 2012 at 6:58 PM, Marek Vasut marek.va...@gmail.com 
 wrote:
 On Wed, Jan 11, 2012 at 6:16 PM, Marek Vasut marek.va...@gmail.com 
 wrote:
 On Wed, Jan 11, 2012 at 4:22 PM, Marek Vasut marek.va...@gmail.com
 wrote:
 Hi Marek,

 Thanks for you review.

 On Tue, Jan 10, 2012 at 9:37 PM, Marek Vasut
 marek.va...@gmail.com

 wrote:
 From: Govindraj.R govindraj.r...@ti.com

 Clean up added ehci-omap.c and make it generic for re-use
 across soc having same ehci ip block. Also pass the modes to
 be configured and configure the ports accordingly. All usb
 layers are not cache aligned till then keep cache off for usb
 ops as ehci will use internally dma for all usb ops.

 * Add a generic common header ehci-omap.h having common ip
 block data and reg shifts.
 * Rename and modify ehci-omap3 to ehci.h retain only
 conflicting sysc reg shifts remove others and move to common
 header file.

 Don't reimplement the ulpi stuff ... there's already some ulpi
 stuff in uboot that needs fixing, so fix it and use it.

 I am not implementing any ulpi stuff I am just configuring OMAP on
 soc usb host controller (ehci). All the configuration stuff
 is OMAP specific things which are done in ehci-omap.c file

 stuffs done are like soft-reset, port mode to be used and putting
 port in no -idle mode(omap specific pm implementation) etc.

 This stuff:

 +/* ULPI */
 +#define ULPI_SET(a)                                    (a + 1)
 +#define ULPI_CLR(a)                                    (a + 2)
 +#define ULPI_FUNC_CTRL                                 0x04
 +#define ULPI_FUNC_CTRL_RESET                           (1  5)

 is just accidentally conforming to ULPI spec?

 These are for configuring INSNREG05_ULPI reg in EHCI reg map
 of omap while configuring in ulpi-phy mode.

 looking into struct ulpi_regs {..}
 then it doesn't map this configuration.

 Can you point me to some documentation about this please? It's not
 that I don't trust you, I'd rather prefer to avoid unnecessary
 duplication.

 Yes that would be fine.

 You can download the omap4460 public trm from here:

 http://www.ti.com/pdfs/wtbu/OMAP4460_ES.1x_PUBLIC_TRM_vM.zip

 Go to chapter 23.11.6.6.1 EHCI Register Summary
 (page number 5171 and 5186/87)

 Sure, but the macro above looks more like 23.11.6.3, doesn't it ? And 
 for
 that purpose, the struct ulpi_regs is fitting ok.

 Actually ... can you check the ulpi_read and ulpi_write stuff that's
 already in u-boot and explain why they can not be used with this port?

 echi-omap.c is no where writing to those registers
 and the macro was used only to configure INSNREG05_ULPI reg in EHCI reg 
 map

 reg map in 23.11.6.3 used only for a external ulpi-phy communication.
 and debug purpose(to view vid, pid etc) and to hack external phy
 configuration through ulpi commands
 from omap - usb host controller point of view only needs
 INSNREG05_ULPI reg in EHCI reg configuration
 rest on soc host controller takes care of it.

 Can someone else comment on this? I think I don't understand well (as I'm 
 not
 OMAP guy).

 Well, it is on my list, actually,
 but I will be able to get to it only in a couple of days.
 (I'm really busy right now).


 Could you please let me know what exactly that you will be
 updating?

 So that I can accordingly post my v3 of this patch fixing comments
 from Marek Vasut marek.va...@gmail.com

 Well, I did not say, I'm going to update anything.
 What I meant is that I'm going to look into TI's documentation
 regarding EHCI and the ULPI to understand the dependencies and see
 how your code meets those and if the generic ULPI layer can be used
 for that.


 okay, Thanks,

 So to lower the work load from from you, I'd suggest you to wait
 till Monday (if you can of course) to let me look into this.


Gentle Ping.

And just to clarify further there is no code duplication for ulpi read writes
in ehci-omap.c done with this patch.
Patch intends only in configuring ehci to right modes as specified by
board file.  Modes possible are hsic_mode, tll_mode, ulpi_phy mode.

This patch is derived with reference to linux kernel and even there
we can see that no ulpi_reg map registers are used and only ehci
is configured is respective mode as passed by board data.

Here [1] is the patch fixing Marek Vasut marek.va...@gmail.com
comments.

This corrected patch along with dependent patch from
Ilya Yanok ya...@emcraft.com
[PATCH V4 1/2] ehci-omap: driver for EHCI host on OMAP3

Is available here:
git://gitorious.org/denx_u-boot/denx_uboot_omap.git v2_ehci_omap

--
Thanks,
Govindraj.R

[1]:

From 56b1b94128495ed4bf83e2f20f3884833e2aa082 Mon Sep 17 00:00:00 2001
From: 

Re: [U-Boot] [PATCH v2 1/4] ehci-omap: Clean up added ehci-omap.c

2012-01-12 Thread Igor Grinberg
Hi Govindraj,

On 01/12/12 07:45, Govindraj wrote:
 Hi Igor,
 
 On Wed, Jan 11, 2012 at 8:33 PM, Igor Grinberg grinb...@compulab.co.il 
 wrote:
 Hi Guys,

 On 01/11/12 16:34, Marek Vasut wrote:
 On Wed, Jan 11, 2012 at 6:58 PM, Marek Vasut marek.va...@gmail.com wrote:
 On Wed, Jan 11, 2012 at 6:16 PM, Marek Vasut marek.va...@gmail.com 
 wrote:
 On Wed, Jan 11, 2012 at 4:22 PM, Marek Vasut marek.va...@gmail.com
 wrote:
 Hi Marek,

 Thanks for you review.

 On Tue, Jan 10, 2012 at 9:37 PM, Marek Vasut
 marek.va...@gmail.com

 wrote:
 From: Govindraj.R govindraj.r...@ti.com

 Clean up added ehci-omap.c and make it generic for re-use
 across soc having same ehci ip block. Also pass the modes to
 be configured and configure the ports accordingly. All usb
 layers are not cache aligned till then keep cache off for usb
 ops as ehci will use internally dma for all usb ops.

 * Add a generic common header ehci-omap.h having common ip
 block data and reg shifts.
 * Rename and modify ehci-omap3 to ehci.h retain only
 conflicting sysc reg shifts remove others and move to common
 header file.

 Don't reimplement the ulpi stuff ... there's already some ulpi
 stuff in uboot that needs fixing, so fix it and use it.

 I am not implementing any ulpi stuff I am just configuring OMAP on
 soc usb host controller (ehci). All the configuration stuff
 is OMAP specific things which are done in ehci-omap.c file

 stuffs done are like soft-reset, port mode to be used and putting
 port in no -idle mode(omap specific pm implementation) etc.

 This stuff:

 +/* ULPI */
 +#define ULPI_SET(a)(a + 1)
 +#define ULPI_CLR(a)(a + 2)
 +#define ULPI_FUNC_CTRL 0x04
 +#define ULPI_FUNC_CTRL_RESET   (1  5)

 is just accidentally conforming to ULPI spec?

 These are for configuring INSNREG05_ULPI reg in EHCI reg map
 of omap while configuring in ulpi-phy mode.

 looking into struct ulpi_regs {..}
 then it doesn't map this configuration.

 Can you point me to some documentation about this please? It's not
 that I don't trust you, I'd rather prefer to avoid unnecessary
 duplication.

 Yes that would be fine.

 You can download the omap4460 public trm from here:

 http://www.ti.com/pdfs/wtbu/OMAP4460_ES.1x_PUBLIC_TRM_vM.zip

 Go to chapter 23.11.6.6.1 EHCI Register Summary
 (page number 5171 and 5186/87)

 Sure, but the macro above looks more like 23.11.6.3, doesn't it ? And for
 that purpose, the struct ulpi_regs is fitting ok.

 Actually ... can you check the ulpi_read and ulpi_write stuff that's
 already in u-boot and explain why they can not be used with this port?

 echi-omap.c is no where writing to those registers
 and the macro was used only to configure INSNREG05_ULPI reg in EHCI reg map

 reg map in 23.11.6.3 used only for a external ulpi-phy communication.
 and debug purpose(to view vid, pid etc) and to hack external phy
 configuration through ulpi commands
 from omap - usb host controller point of view only needs
 INSNREG05_ULPI reg in EHCI reg configuration
 rest on soc host controller takes care of it.

 Can someone else comment on this? I think I don't understand well (as I'm 
 not
 OMAP guy).

 Well, it is on my list, actually,
 but I will be able to get to it only in a couple of days.
 (I'm really busy right now).

 
 Could you please let me know what exactly that you will be
 updating?
 
 So that I can accordingly post my v3 of this patch fixing comments
 from Marek Vasut marek.va...@gmail.com

Well, I did not say, I'm going to update anything.
What I meant is that I'm going to look into TI's documentation
regarding EHCI and the ULPI to understand the dependencies and see
how your code meets those and if the generic ULPI layer can be used
for that.

So to lower the work load from from you, I'd suggest you to wait
till Monday (if you can of course) to let me look into this.
It is also possible that I will be able to check the above today
or tomorrow, but I can't promise, as I'm really busy now.

Thanks.


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


Re: [U-Boot] [PATCH v2 1/4] ehci-omap: Clean up added ehci-omap.c

2012-01-11 Thread Marek Vasut
 Hi Marek,
 
 Thanks for you review.
 
 On Tue, Jan 10, 2012 at 9:37 PM, Marek Vasut marek.va...@gmail.com wrote:
  From: Govindraj.R govindraj.r...@ti.com
  
  Clean up added ehci-omap.c and make it generic for re-use across
  soc having same ehci ip block. Also pass the modes to be configured
  and configure the ports accordingly. All usb layers are not cache
  aligned till then keep cache off for usb ops as ehci will use
  internally dma for all usb ops.
  
  * Add a generic common header ehci-omap.h having common ip block
data and reg shifts.
  * Rename and modify ehci-omap3 to ehci.h retain only conflicting
sysc reg shifts remove others and move to common header file.
  
  Don't reimplement the ulpi stuff ... there's already some ulpi stuff in
  uboot that needs fixing, so fix it and use it.
 
 I am not implementing any ulpi stuff I am just configuring OMAP on
 soc usb host controller (ehci). All the configuration stuff
 is OMAP specific things which are done in ehci-omap.c file
 
 stuffs done are like soft-reset, port mode to be used and putting
 port in no -idle mode(omap specific pm implementation) etc.
 

This stuff:

+/* ULPI */
+#define ULPI_SET(a)(a + 1)
+#define ULPI_CLR(a)(a + 2)
+#define ULPI_FUNC_CTRL 0x04
+#define ULPI_FUNC_CTRL_RESET   (1  5)

is just accidentally conforming to ULPI spec?

M

btw. somewhere in the patch is one more asterisk at the end of line:
+ * Copyright (C) 2011 Texas Instruments Incorporated - http://www.ti.com*

[...]

  + /* perform TLL soft reset, and wait until reset is complete */
  + writel(OMAP_USBTLL_SYSCONFIG_SOFTRESET, usbtll-sysc);
  +
  + /* Wait for TLL reset to complete */
  + while (!(readl(usbtll-syss)  OMAP_USBTLL_SYSSTATUS_RESETDONE))
  
  Add timeout, fix globally
 
 Sorry I didn't get you here.
 
 The function uses a timeout value init and then same init
 value to used to poll for CONFIG_SYS_HZ time for reset to be
 done else prints timeout failure.

Ah sorry, I didn't notice. OK.
 
  + if (get_timer(init)  CONFIG_SYS_HZ) {
  + debug(OMAP EHCI error: timeout resetting TLL\n);
  + return -EL3RST;
  + }
  +
 
 [...]
 
  + /* setup ULPI bypass and burst configurations */
  + reg |= (OMAP_UHH_HOSTCONFIG_INCR4_BURST_EN |
  + OMAP_UHH_HOSTCONFIG_INCR8_BURST_EN |
  + OMAP_UHH_HOSTCONFIG_INCR16_BURST_EN);
  + reg = ~OMAP_UHH_HOSTCONFIG_INCRX_ALIGN_EN;
  
  clrsetbits_le32 ?
 
 yes can be used.

Use where applicable
 
 --
 Thanks,
 Govindraj.R
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH v2 1/4] ehci-omap: Clean up added ehci-omap.c

2012-01-11 Thread Govindraj
On Wed, Jan 11, 2012 at 4:22 PM, Marek Vasut marek.va...@gmail.com wrote:
 Hi Marek,

 Thanks for you review.

 On Tue, Jan 10, 2012 at 9:37 PM, Marek Vasut marek.va...@gmail.com wrote:
  From: Govindraj.R govindraj.r...@ti.com
 
  Clean up added ehci-omap.c and make it generic for re-use across
  soc having same ehci ip block. Also pass the modes to be configured
  and configure the ports accordingly. All usb layers are not cache
  aligned till then keep cache off for usb ops as ehci will use
  internally dma for all usb ops.
 
  * Add a generic common header ehci-omap.h having common ip block
    data and reg shifts.
  * Rename and modify ehci-omap3 to ehci.h retain only conflicting
    sysc reg shifts remove others and move to common header file.
 
  Don't reimplement the ulpi stuff ... there's already some ulpi stuff in
  uboot that needs fixing, so fix it and use it.

 I am not implementing any ulpi stuff I am just configuring OMAP on
 soc usb host controller (ehci). All the configuration stuff
 is OMAP specific things which are done in ehci-omap.c file

 stuffs done are like soft-reset, port mode to be used and putting
 port in no -idle mode(omap specific pm implementation) etc.


 This stuff:

 +/* ULPI */
 +#define ULPI_SET(a)                                    (a + 1)
 +#define ULPI_CLR(a)                                    (a + 2)
 +#define ULPI_FUNC_CTRL                                 0x04
 +#define ULPI_FUNC_CTRL_RESET                           (1  5)

 is just accidentally conforming to ULPI spec?


These are for configuring INSNREG05_ULPI reg in EHCI reg map
of omap while configuring in ulpi-phy mode.

looking into struct ulpi_regs {..}
then it doesn't map this configuration.

btw,
IIUC that ulpi_regs struct is for otg transceiver that uses a ulpi phy chip
for communication.

 M

 btw. somewhere in the patch is one more asterisk at the end of line:

Will check that

--
Thanks,
Govindraj.R
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH v2 1/4] ehci-omap: Clean up added ehci-omap.c

2012-01-11 Thread Marek Vasut
 On Wed, Jan 11, 2012 at 4:22 PM, Marek Vasut marek.va...@gmail.com wrote:
  Hi Marek,
  
  Thanks for you review.
  
  On Tue, Jan 10, 2012 at 9:37 PM, Marek Vasut marek.va...@gmail.com wrote:
   From: Govindraj.R govindraj.r...@ti.com
   
   Clean up added ehci-omap.c and make it generic for re-use across
   soc having same ehci ip block. Also pass the modes to be configured
   and configure the ports accordingly. All usb layers are not cache
   aligned till then keep cache off for usb ops as ehci will use
   internally dma for all usb ops.
   
   * Add a generic common header ehci-omap.h having common ip block
 data and reg shifts.
   * Rename and modify ehci-omap3 to ehci.h retain only conflicting
 sysc reg shifts remove others and move to common header file.
   
   Don't reimplement the ulpi stuff ... there's already some ulpi stuff
   in uboot that needs fixing, so fix it and use it.
  
  I am not implementing any ulpi stuff I am just configuring OMAP on
  soc usb host controller (ehci). All the configuration stuff
  is OMAP specific things which are done in ehci-omap.c file
  
  stuffs done are like soft-reset, port mode to be used and putting
  port in no -idle mode(omap specific pm implementation) etc.
  
  This stuff:
  
  +/* ULPI */
  +#define ULPI_SET(a)(a + 1)
  +#define ULPI_CLR(a)(a + 2)
  +#define ULPI_FUNC_CTRL 0x04
  +#define ULPI_FUNC_CTRL_RESET   (1  5)
  
  is just accidentally conforming to ULPI spec?
 
 These are for configuring INSNREG05_ULPI reg in EHCI reg map
 of omap while configuring in ulpi-phy mode.
 
 looking into struct ulpi_regs {..}
 then it doesn't map this configuration.

Can you point me to some documentation about this please? It's not that I don't 
trust you, I'd rather prefer to avoid unnecessary duplication.

 
 btw,
 IIUC that ulpi_regs struct is for otg transceiver that uses a ulpi phy chip
 for communication.
 
  M
 
  btw. somewhere in the patch is one more asterisk at the end of line:
 Will check that

It was on the line I pointed out 
 
 --
 Thanks,
 Govindraj.R
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH v2 1/4] ehci-omap: Clean up added ehci-omap.c

2012-01-11 Thread Govindraj
On Wed, Jan 11, 2012 at 6:16 PM, Marek Vasut marek.va...@gmail.com wrote:
 On Wed, Jan 11, 2012 at 4:22 PM, Marek Vasut marek.va...@gmail.com wrote:
  Hi Marek,
 
  Thanks for you review.
 
  On Tue, Jan 10, 2012 at 9:37 PM, Marek Vasut marek.va...@gmail.com 
  wrote:
   From: Govindraj.R govindraj.r...@ti.com
  
   Clean up added ehci-omap.c and make it generic for re-use across
   soc having same ehci ip block. Also pass the modes to be configured
   and configure the ports accordingly. All usb layers are not cache
   aligned till then keep cache off for usb ops as ehci will use
   internally dma for all usb ops.
  
   * Add a generic common header ehci-omap.h having common ip block
     data and reg shifts.
   * Rename and modify ehci-omap3 to ehci.h retain only conflicting
     sysc reg shifts remove others and move to common header file.
  
   Don't reimplement the ulpi stuff ... there's already some ulpi stuff
   in uboot that needs fixing, so fix it and use it.
 
  I am not implementing any ulpi stuff I am just configuring OMAP on
  soc usb host controller (ehci). All the configuration stuff
  is OMAP specific things which are done in ehci-omap.c file
 
  stuffs done are like soft-reset, port mode to be used and putting
  port in no -idle mode(omap specific pm implementation) etc.
 
  This stuff:
 
  +/* ULPI */
  +#define ULPI_SET(a)                                    (a + 1)
  +#define ULPI_CLR(a)                                    (a + 2)
  +#define ULPI_FUNC_CTRL                                 0x04
  +#define ULPI_FUNC_CTRL_RESET                           (1  5)
 
  is just accidentally conforming to ULPI spec?

 These are for configuring INSNREG05_ULPI reg in EHCI reg map
 of omap while configuring in ulpi-phy mode.

 looking into struct ulpi_regs {..}
 then it doesn't map this configuration.

 Can you point me to some documentation about this please? It's not that I 
 don't
 trust you, I'd rather prefer to avoid unnecessary duplication.


Yes that would be fine.

You can download the omap4460 public trm from here:

http://www.ti.com/pdfs/wtbu/OMAP4460_ES.1x_PUBLIC_TRM_vM.zip

Go to chapter 23.11.6.6.1 EHCI Register Summary
(page number 5171 and 5186/87)

click INSNREG05_ULPI

this for configuring in ulpi mode for external ulpi phy.
reference
chapter 23.11.4.1 refer to Figure 23-252. HS USB Host Controller Architecture
(page number 5096)


 btw,
 IIUC that ulpi_regs struct is for otg transceiver that uses a ulpi phy chip
 for communication.

  M

  btw. somewhere in the patch is one more asterisk at the end of line:
 Will check that

 It was on the line I pointed out

okay, got it.

--
Thanks,
Govindraj.R
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH v2 1/4] ehci-omap: Clean up added ehci-omap.c

2012-01-11 Thread Govindraj
On Wed, Jan 11, 2012 at 6:58 PM, Marek Vasut marek.va...@gmail.com wrote:
 On Wed, Jan 11, 2012 at 6:16 PM, Marek Vasut marek.va...@gmail.com wrote:
  On Wed, Jan 11, 2012 at 4:22 PM, Marek Vasut marek.va...@gmail.com 
  wrote:
   Hi Marek,
  
   Thanks for you review.
  
   On Tue, Jan 10, 2012 at 9:37 PM, Marek Vasut marek.va...@gmail.com
 wrote:
From: Govindraj.R govindraj.r...@ti.com
   
Clean up added ehci-omap.c and make it generic for re-use across
soc having same ehci ip block. Also pass the modes to be
configured and configure the ports accordingly. All usb layers
are not cache aligned till then keep cache off for usb ops as
ehci will use internally dma for all usb ops.
   
* Add a generic common header ehci-omap.h having common ip block
  data and reg shifts.
* Rename and modify ehci-omap3 to ehci.h retain only conflicting
  sysc reg shifts remove others and move to common header file.
   
Don't reimplement the ulpi stuff ... there's already some ulpi
stuff in uboot that needs fixing, so fix it and use it.
  
   I am not implementing any ulpi stuff I am just configuring OMAP on
   soc usb host controller (ehci). All the configuration stuff
   is OMAP specific things which are done in ehci-omap.c file
  
   stuffs done are like soft-reset, port mode to be used and putting
   port in no -idle mode(omap specific pm implementation) etc.
  
   This stuff:
  
   +/* ULPI */
   +#define ULPI_SET(a)                                    (a + 1)
   +#define ULPI_CLR(a)                                    (a + 2)
   +#define ULPI_FUNC_CTRL                                 0x04
   +#define ULPI_FUNC_CTRL_RESET                           (1  5)
  
   is just accidentally conforming to ULPI spec?
 
  These are for configuring INSNREG05_ULPI reg in EHCI reg map
  of omap while configuring in ulpi-phy mode.
 
  looking into struct ulpi_regs {..}
  then it doesn't map this configuration.
 
  Can you point me to some documentation about this please? It's not that I
  don't trust you, I'd rather prefer to avoid unnecessary duplication.

 Yes that would be fine.

 You can download the omap4460 public trm from here:

 http://www.ti.com/pdfs/wtbu/OMAP4460_ES.1x_PUBLIC_TRM_vM.zip

 Go to chapter 23.11.6.6.1 EHCI Register Summary
 (page number 5171 and 5186/87)

 Sure, but the macro above looks more like 23.11.6.3, doesn't it ? And for that
 purpose, the struct ulpi_regs is fitting ok.

 Actually ... can you check the ulpi_read and ulpi_write stuff that's already 
 in
 u-boot and explain why they can not be used with this port?


echi-omap.c is no where writing to those registers
and the macro was used only to configure INSNREG05_ULPI reg in EHCI reg map

reg map in 23.11.6.3 used only for a external ulpi-phy communication.
and debug purpose(to view vid, pid etc) and to hack external phy
configuration through ulpi commands
from omap - usb host controller point of view only needs
INSNREG05_ULPI reg in EHCI reg configuration
rest on soc host controller takes care of it.

--
Thanks,
Govindraj.R
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH v2 1/4] ehci-omap: Clean up added ehci-omap.c

2012-01-11 Thread Marek Vasut
 On Wed, Jan 11, 2012 at 6:58 PM, Marek Vasut marek.va...@gmail.com wrote:
  On Wed, Jan 11, 2012 at 6:16 PM, Marek Vasut marek.va...@gmail.com wrote:
   On Wed, Jan 11, 2012 at 4:22 PM, Marek Vasut marek.va...@gmail.com 
wrote:
Hi Marek,

Thanks for you review.

On Tue, Jan 10, 2012 at 9:37 PM, Marek Vasut
marek.va...@gmail.com
  
  wrote:
 From: Govindraj.R govindraj.r...@ti.com
 
 Clean up added ehci-omap.c and make it generic for re-use
 across soc having same ehci ip block. Also pass the modes to
 be configured and configure the ports accordingly. All usb
 layers are not cache aligned till then keep cache off for usb
 ops as ehci will use internally dma for all usb ops.
 
 * Add a generic common header ehci-omap.h having common ip
 block data and reg shifts.
 * Rename and modify ehci-omap3 to ehci.h retain only
 conflicting sysc reg shifts remove others and move to common
 header file.
 
 Don't reimplement the ulpi stuff ... there's already some ulpi
 stuff in uboot that needs fixing, so fix it and use it.

I am not implementing any ulpi stuff I am just configuring OMAP on
soc usb host controller (ehci). All the configuration stuff
is OMAP specific things which are done in ehci-omap.c file

stuffs done are like soft-reset, port mode to be used and putting
port in no -idle mode(omap specific pm implementation) etc.

This stuff:

+/* ULPI */
+#define ULPI_SET(a)(a + 1)
+#define ULPI_CLR(a)(a + 2)
+#define ULPI_FUNC_CTRL 0x04
+#define ULPI_FUNC_CTRL_RESET   (1  5)

is just accidentally conforming to ULPI spec?
   
   These are for configuring INSNREG05_ULPI reg in EHCI reg map
   of omap while configuring in ulpi-phy mode.
   
   looking into struct ulpi_regs {..}
   then it doesn't map this configuration.
   
   Can you point me to some documentation about this please? It's not
   that I don't trust you, I'd rather prefer to avoid unnecessary
   duplication.
  
  Yes that would be fine.
  
  You can download the omap4460 public trm from here:
  
  http://www.ti.com/pdfs/wtbu/OMAP4460_ES.1x_PUBLIC_TRM_vM.zip
  
  Go to chapter 23.11.6.6.1 EHCI Register Summary
  (page number 5171 and 5186/87)
  
  Sure, but the macro above looks more like 23.11.6.3, doesn't it ? And for
  that purpose, the struct ulpi_regs is fitting ok.
  
  Actually ... can you check the ulpi_read and ulpi_write stuff that's
  already in u-boot and explain why they can not be used with this port?
 
 echi-omap.c is no where writing to those registers
 and the macro was used only to configure INSNREG05_ULPI reg in EHCI reg map
 
 reg map in 23.11.6.3 used only for a external ulpi-phy communication.
 and debug purpose(to view vid, pid etc) and to hack external phy
 configuration through ulpi commands
 from omap - usb host controller point of view only needs
 INSNREG05_ULPI reg in EHCI reg configuration
 rest on soc host controller takes care of it.

Can someone else comment on this? I think I don't understand well (as I'm not 
OMAP guy).

M
 
 --
 Thanks,
 Govindraj.R
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH v2 1/4] ehci-omap: Clean up added ehci-omap.c

2012-01-11 Thread Igor Grinberg
Hi Guys,

On 01/11/12 16:34, Marek Vasut wrote:
 On Wed, Jan 11, 2012 at 6:58 PM, Marek Vasut marek.va...@gmail.com wrote:
 On Wed, Jan 11, 2012 at 6:16 PM, Marek Vasut marek.va...@gmail.com wrote:
 On Wed, Jan 11, 2012 at 4:22 PM, Marek Vasut marek.va...@gmail.com 
 wrote:
 Hi Marek,

 Thanks for you review.

 On Tue, Jan 10, 2012 at 9:37 PM, Marek Vasut
 marek.va...@gmail.com

 wrote:
 From: Govindraj.R govindraj.r...@ti.com

 Clean up added ehci-omap.c and make it generic for re-use
 across soc having same ehci ip block. Also pass the modes to
 be configured and configure the ports accordingly. All usb
 layers are not cache aligned till then keep cache off for usb
 ops as ehci will use internally dma for all usb ops.

 * Add a generic common header ehci-omap.h having common ip
 block data and reg shifts.
 * Rename and modify ehci-omap3 to ehci.h retain only
 conflicting sysc reg shifts remove others and move to common
 header file.

 Don't reimplement the ulpi stuff ... there's already some ulpi
 stuff in uboot that needs fixing, so fix it and use it.

 I am not implementing any ulpi stuff I am just configuring OMAP on
 soc usb host controller (ehci). All the configuration stuff
 is OMAP specific things which are done in ehci-omap.c file

 stuffs done are like soft-reset, port mode to be used and putting
 port in no -idle mode(omap specific pm implementation) etc.

 This stuff:

 +/* ULPI */
 +#define ULPI_SET(a)(a + 1)
 +#define ULPI_CLR(a)(a + 2)
 +#define ULPI_FUNC_CTRL 0x04
 +#define ULPI_FUNC_CTRL_RESET   (1  5)

 is just accidentally conforming to ULPI spec?

 These are for configuring INSNREG05_ULPI reg in EHCI reg map
 of omap while configuring in ulpi-phy mode.

 looking into struct ulpi_regs {..}
 then it doesn't map this configuration.

 Can you point me to some documentation about this please? It's not
 that I don't trust you, I'd rather prefer to avoid unnecessary
 duplication.

 Yes that would be fine.

 You can download the omap4460 public trm from here:

 http://www.ti.com/pdfs/wtbu/OMAP4460_ES.1x_PUBLIC_TRM_vM.zip

 Go to chapter 23.11.6.6.1 EHCI Register Summary
 (page number 5171 and 5186/87)

 Sure, but the macro above looks more like 23.11.6.3, doesn't it ? And for
 that purpose, the struct ulpi_regs is fitting ok.

 Actually ... can you check the ulpi_read and ulpi_write stuff that's
 already in u-boot and explain why they can not be used with this port?

 echi-omap.c is no where writing to those registers
 and the macro was used only to configure INSNREG05_ULPI reg in EHCI reg map

 reg map in 23.11.6.3 used only for a external ulpi-phy communication.
 and debug purpose(to view vid, pid etc) and to hack external phy
 configuration through ulpi commands
 from omap - usb host controller point of view only needs
 INSNREG05_ULPI reg in EHCI reg configuration
 rest on soc host controller takes care of it.
 
 Can someone else comment on this? I think I don't understand well (as I'm not 
 OMAP guy).

Well, it is on my list, actually,
but I will be able to get to it only in a couple of days.
(I'm really busy right now).


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


Re: [U-Boot] [PATCH v2 1/4] ehci-omap: Clean up added ehci-omap.c

2012-01-11 Thread Marek Vasut
 Hi Guys,
 
 On 01/11/12 16:34, Marek Vasut wrote:
  On Wed, Jan 11, 2012 at 6:58 PM, Marek Vasut marek.va...@gmail.com wrote:
  On Wed, Jan 11, 2012 at 6:16 PM, Marek Vasut marek.va...@gmail.com 
wrote:
  On Wed, Jan 11, 2012 at 4:22 PM, Marek Vasut marek.va...@gmail.com
  
  wrote:
  Hi Marek,
  
  Thanks for you review.
  
  On Tue, Jan 10, 2012 at 9:37 PM, Marek Vasut
  marek.va...@gmail.com
  
  wrote:
  From: Govindraj.R govindraj.r...@ti.com
  
  Clean up added ehci-omap.c and make it generic for re-use
  across soc having same ehci ip block. Also pass the modes to
  be configured and configure the ports accordingly. All usb
  layers are not cache aligned till then keep cache off for usb
  ops as ehci will use internally dma for all usb ops.
  
  * Add a generic common header ehci-omap.h having common ip
  block data and reg shifts.
  * Rename and modify ehci-omap3 to ehci.h retain only
  conflicting sysc reg shifts remove others and move to common
  header file.
  
  Don't reimplement the ulpi stuff ... there's already some ulpi
  stuff in uboot that needs fixing, so fix it and use it.
  
  I am not implementing any ulpi stuff I am just configuring OMAP on
  soc usb host controller (ehci). All the configuration stuff
  is OMAP specific things which are done in ehci-omap.c file
  
  stuffs done are like soft-reset, port mode to be used and putting
  port in no -idle mode(omap specific pm implementation) etc.
  
  This stuff:
  
  +/* ULPI */
  +#define ULPI_SET(a)(a + 1)
  +#define ULPI_CLR(a)(a + 2)
  +#define ULPI_FUNC_CTRL 0x04
  +#define ULPI_FUNC_CTRL_RESET   (1  5)
  
  is just accidentally conforming to ULPI spec?
  
  These are for configuring INSNREG05_ULPI reg in EHCI reg map
  of omap while configuring in ulpi-phy mode.
  
  looking into struct ulpi_regs {..}
  then it doesn't map this configuration.
  
  Can you point me to some documentation about this please? It's not
  that I don't trust you, I'd rather prefer to avoid unnecessary
  duplication.
  
  Yes that would be fine.
  
  You can download the omap4460 public trm from here:
  
  http://www.ti.com/pdfs/wtbu/OMAP4460_ES.1x_PUBLIC_TRM_vM.zip
  
  Go to chapter 23.11.6.6.1 EHCI Register Summary
  (page number 5171 and 5186/87)
  
  Sure, but the macro above looks more like 23.11.6.3, doesn't it ? And
  for that purpose, the struct ulpi_regs is fitting ok.
  
  Actually ... can you check the ulpi_read and ulpi_write stuff that's
  already in u-boot and explain why they can not be used with this port?
  
  echi-omap.c is no where writing to those registers
  and the macro was used only to configure INSNREG05_ULPI reg in EHCI reg
  map
  
  reg map in 23.11.6.3 used only for a external ulpi-phy communication.
  and debug purpose(to view vid, pid etc) and to hack external phy
  configuration through ulpi commands
  from omap - usb host controller point of view only needs
  INSNREG05_ULPI reg in EHCI reg configuration
  rest on soc host controller takes care of it.
  
  Can someone else comment on this? I think I don't understand well (as I'm
  not OMAP guy).
 
 Well, it is on my list, actually,
 but I will be able to get to it only in a couple of days.
 (I'm really busy right now).

Good, please keep me updated, Igor. Thank you
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH v2 1/4] ehci-omap: Clean up added ehci-omap.c

2012-01-11 Thread Govindraj
Hi Igor,

On Wed, Jan 11, 2012 at 8:33 PM, Igor Grinberg grinb...@compulab.co.il wrote:
 Hi Guys,

 On 01/11/12 16:34, Marek Vasut wrote:
 On Wed, Jan 11, 2012 at 6:58 PM, Marek Vasut marek.va...@gmail.com wrote:
 On Wed, Jan 11, 2012 at 6:16 PM, Marek Vasut marek.va...@gmail.com 
 wrote:
 On Wed, Jan 11, 2012 at 4:22 PM, Marek Vasut marek.va...@gmail.com
 wrote:
 Hi Marek,

 Thanks for you review.

 On Tue, Jan 10, 2012 at 9:37 PM, Marek Vasut
 marek.va...@gmail.com

 wrote:
 From: Govindraj.R govindraj.r...@ti.com

 Clean up added ehci-omap.c and make it generic for re-use
 across soc having same ehci ip block. Also pass the modes to
 be configured and configure the ports accordingly. All usb
 layers are not cache aligned till then keep cache off for usb
 ops as ehci will use internally dma for all usb ops.

 * Add a generic common header ehci-omap.h having common ip
 block data and reg shifts.
 * Rename and modify ehci-omap3 to ehci.h retain only
 conflicting sysc reg shifts remove others and move to common
 header file.

 Don't reimplement the ulpi stuff ... there's already some ulpi
 stuff in uboot that needs fixing, so fix it and use it.

 I am not implementing any ulpi stuff I am just configuring OMAP on
 soc usb host controller (ehci). All the configuration stuff
 is OMAP specific things which are done in ehci-omap.c file

 stuffs done are like soft-reset, port mode to be used and putting
 port in no -idle mode(omap specific pm implementation) etc.

 This stuff:

 +/* ULPI */
 +#define ULPI_SET(a)                                    (a + 1)
 +#define ULPI_CLR(a)                                    (a + 2)
 +#define ULPI_FUNC_CTRL                                 0x04
 +#define ULPI_FUNC_CTRL_RESET                           (1  5)

 is just accidentally conforming to ULPI spec?

 These are for configuring INSNREG05_ULPI reg in EHCI reg map
 of omap while configuring in ulpi-phy mode.

 looking into struct ulpi_regs {..}
 then it doesn't map this configuration.

 Can you point me to some documentation about this please? It's not
 that I don't trust you, I'd rather prefer to avoid unnecessary
 duplication.

 Yes that would be fine.

 You can download the omap4460 public trm from here:

 http://www.ti.com/pdfs/wtbu/OMAP4460_ES.1x_PUBLIC_TRM_vM.zip

 Go to chapter 23.11.6.6.1 EHCI Register Summary
 (page number 5171 and 5186/87)

 Sure, but the macro above looks more like 23.11.6.3, doesn't it ? And for
 that purpose, the struct ulpi_regs is fitting ok.

 Actually ... can you check the ulpi_read and ulpi_write stuff that's
 already in u-boot and explain why they can not be used with this port?

 echi-omap.c is no where writing to those registers
 and the macro was used only to configure INSNREG05_ULPI reg in EHCI reg map

 reg map in 23.11.6.3 used only for a external ulpi-phy communication.
 and debug purpose(to view vid, pid etc) and to hack external phy
 configuration through ulpi commands
 from omap - usb host controller point of view only needs
 INSNREG05_ULPI reg in EHCI reg configuration
 rest on soc host controller takes care of it.

 Can someone else comment on this? I think I don't understand well (as I'm not
 OMAP guy).

 Well, it is on my list, actually,
 but I will be able to get to it only in a couple of days.
 (I'm really busy right now).


Could you please let me know what exactly that you will be
updating?

So that I can accordingly post my v3 of this patch fixing comments
from Marek Vasut marek.va...@gmail.com

--
Thanks,
Govindraj.R
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH v2 1/4] ehci-omap: Clean up added ehci-omap.c

2012-01-10 Thread Marek Vasut
 From: Govindraj.R govindraj.r...@ti.com
 
 Clean up added ehci-omap.c and make it generic for re-use across
 soc having same ehci ip block. Also pass the modes to be configured
 and configure the ports accordingly. All usb layers are not cache
 aligned till then keep cache off for usb ops as ehci will use
 internally dma for all usb ops.
 
 * Add a generic common header ehci-omap.h having common ip block
   data and reg shifts.
 * Rename and modify ehci-omap3 to ehci.h retain only conflicting
   sysc reg shifts remove others and move to common header file.

Don't reimplement the ulpi stuff ... there's already some ulpi stuff in uboot 
that needs fixing, so fix it and use it.

 
 Signed-off-by: Govindraj.R govindraj.r...@ti.com
 ---
  arch/arm/include/asm/arch-omap3/ehci.h   |   55 ++
  arch/arm/include/asm/arch-omap3/ehci_omap3.h |   58 ---
  arch/arm/include/asm/arch-omap4/ehci.h   |   49 ++
  arch/arm/include/asm/ehci-omap.h |  147 +
  drivers/usb/host/ehci-omap.c |  228
 +++--- 5 files changed, 423 insertions(+), 114
 deletions(-)
  create mode 100644 arch/arm/include/asm/arch-omap3/ehci.h
  delete mode 100644 arch/arm/include/asm/arch-omap3/ehci_omap3.h
  create mode 100644 arch/arm/include/asm/arch-omap4/ehci.h
  create mode 100644 arch/arm/include/asm/ehci-omap.h
 
 diff --git a/arch/arm/include/asm/arch-omap3/ehci.h
 b/arch/arm/include/asm/arch-omap3/ehci.h new file mode 100644
 index 000..d622363
 --- /dev/null
 +++ b/arch/arm/include/asm/arch-omap3/ehci.h
 @@ -0,0 +1,55 @@
 +/*
 + * (C) Copyright 2011
 + * Alexander Holler hol...@ahsoftware.de
 + *
 + * Based on drivers/usb/host/ehci-omap.c from Linux 2.6.37
 + *
 + * See there for additional Copyrights.
 + *
 + * See file CREDITS for list of people who contributed to this
 + * project.
 + *
 + * This program is free software; you can redistribute it and/or
 + * modify it under the terms of the GNU General Public License as
 + * published by the Free Software Foundation; either version 2 of
 + * the License, or (at your option) any later version.
 + *
 + * This program is distributed in the hope that it will be useful,
 + * but WITHOUT ANY WARRANTY; without even the implied warranty of
 + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
 + * GNU General Public License for more details.
 + *
 + * You should have received a copy of the GNU General Public License
 + * along with this program; if not, write to the Free Software
 + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston,
 + * MA 02110-1301 USA
 + */
 +#ifndef _EHCI_H_
 +#define _EHCI_H_
 +
 +/* USB/EHCI registers */
 +#define OMAP_USBTLL_BASE 0x48062000UL
 +#define OMAP_UHH_BASE0x48064000UL
 +#define OMAP_EHCI_BASE   0x48064800UL
 +
 +/* TLL Register Set */
 +#define OMAP_USBTLL_SYSCONFIG_SOFTRESET  (1  1)
 +#define OMAP_USBTLL_SYSCONFIG_ENAWAKEUP  (1  2)
 +#define OMAP_USBTLL_SYSCONFIG_SIDLEMODE  (1  3)
 +#define OMAP_USBTLL_SYSCONFIG_CACTIVITY  (1  8)
 +#define OMAP_USBTLL_SYSSTATUS_RESETDONE  1
 +
 +/* UHH Register Set */
 +#define OMAP_UHH_SYSCONFIG_SOFTRESET (1  1)
 +#define OMAP_UHH_SYSCONFIG_CACTIVITY (1  8)
 +#define OMAP_UHH_SYSCONFIG_SIDLEMODE (1  3)
 +#define OMAP_UHH_SYSCONFIG_ENAWAKEUP (1  2)
 +#define OMAP_UHH_SYSCONFIG_MIDLEMODE (1  12)
 +#define OMAP_UHH_SYSSTATUS_EHCI_RESETDONE(1  2)
 +
 +#define OMAP_UHH_SYSCONFIG_VAL   (OMAP_UHH_SYSCONFIG_CACTIVITY | 
\
 + OMAP_UHH_SYSCONFIG_SIDLEMODE | \
 + OMAP_UHH_SYSCONFIG_ENAWAKEUP | \
 + OMAP_UHH_SYSCONFIG_MIDLEMODE)
 +
 +#endif /* _EHCI_H_ */
 diff --git a/arch/arm/include/asm/arch-omap3/ehci_omap3.h
 b/arch/arm/include/asm/arch-omap3/ehci_omap3.h deleted file mode 100644
 index cd01f50..000
 --- a/arch/arm/include/asm/arch-omap3/ehci_omap3.h
 +++ /dev/null
 @@ -1,58 +0,0 @@
 -/*
 - * (C) Copyright 2011
 - * Alexander Holler hol...@ahsoftware.de
 - *
 - * Based on drivers/usb/host/ehci-omap.c from Linux 2.6.37
 - *
 - * See there for additional Copyrights.
 - *
 - * See file CREDITS for list of people who contributed to this
 - * project.
 - *
 - * This program is free software; you can redistribute it and/or
 - * modify it under the terms of the GNU General Public License as
 - * published by the Free Software Foundation; either version 2 of
 - * the License, or (at your option) any later version.
 - *
 - * This program is distributed in the hope that it will be useful,
 - * but WITHOUT ANY WARRANTY; without even the implied warranty of
 - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
 - * 

Re: [U-Boot] [PATCH v2 1/4] ehci-omap: Clean up added ehci-omap.c

2012-01-10 Thread Govindraj
Hi Marek,

Thanks for you review.

On Tue, Jan 10, 2012 at 9:37 PM, Marek Vasut marek.va...@gmail.com wrote:
 From: Govindraj.R govindraj.r...@ti.com

 Clean up added ehci-omap.c and make it generic for re-use across
 soc having same ehci ip block. Also pass the modes to be configured
 and configure the ports accordingly. All usb layers are not cache
 aligned till then keep cache off for usb ops as ehci will use
 internally dma for all usb ops.

 * Add a generic common header ehci-omap.h having common ip block
   data and reg shifts.
 * Rename and modify ehci-omap3 to ehci.h retain only conflicting
   sysc reg shifts remove others and move to common header file.

 Don't reimplement the ulpi stuff ... there's already some ulpi stuff in uboot
 that needs fixing, so fix it and use it.


I am not implementing any ulpi stuff I am just configuring OMAP on
soc usb host controller (ehci). All the configuration stuff
is OMAP specific things which are done in ehci-omap.c file

stuffs done are like soft-reset, port mode to be used and putting
port in no -idle mode(omap specific pm implementation) etc.


 Signed-off-by: Govindraj.R govindraj.r...@ti.com
 ---
  arch/arm/include/asm/arch-omap3/ehci.h       |   55 ++
  arch/arm/include/asm/arch-omap3/ehci_omap3.h |   58 ---
  arch/arm/include/asm/arch-omap4/ehci.h       |   49 ++
  arch/arm/include/asm/ehci-omap.h             |  147 +
  drivers/usb/host/ehci-omap.c                 |  228

[...]

 +
 +#ifdef CONFIG_SYS_USB_EHCI_MAX_ROOT_PORTS
 +#define OMAP_HS_USB_PORTS    CONFIG_SYS_USB_EHCI_MAX_ROOT_PORTS
 +#else
 +#define OMAP_HS_USB_PORTS    3
 +#endif
 +
 +#define is_ehci_phy_mode(x)  (x == OMAP_EHCI_PORT_MODE_PHY)
 +#define is_ehci_tll_mode(x)  (x == OMAP_EHCI_PORT_MODE_TLL)
 +#define is_ehci_hsic_mode(x) (x == OMAP_EHCI_PORT_MODE_HSIC)

 This is unsafe ... use ((x) == ...)

Okay, will correct this.


 +
 +/* Values of UHH_REVISION - Note: these are not given in the TRM */
 +#define OMAP_USBHS_REV1                                      0x0010 /*
 OMAP3 */
 +#define OMAP_USBHS_REV2                                      0x50700100 /*
 OMAP4 */
 +

[...]

 +/* ULPI */
 +#define ULPI_SET(a)                                  (a + 1)
 +#define ULPI_CLR(a)                                  (a + 2)

 ULPI ... use generic stuff

Actually this for omap specific configuration done with omap
reg map.

EHCI register INSNREG05_ULPI needs to be configured if we
are in ulpi-phy mode same is done here.


 +#define ULPI_FUNC_CTRL                                       0x04
 +#define ULPI_FUNC_CTRL_RESET                         (1  5)
 +
 +struct omap_usbhs_board_data {
 +     enum usbhs_omap_port_mode port_mode[OMAP_HS_USB_PORTS];
 +};
 +
 +struct omap_usbtll {
 +     u32 rev;                /* 0x00 */
 +     u32 hwinfo;             /* 0x04 */
 +     u8 pad1[0x8];

 reserved1[] instead of pad1[], fix globally

yes fine, will correct this.

[..]


 +struct omap_uhh *const uhh = (struct omap_uhh *)OMAP_UHH_BASE;
 +struct omap_usbtll *const usbtll = (struct omap_usbtll *)OMAP_USBTLL_BASE;
 +struct omap_ehci *const ehci = (struct omap_ehci *)OMAP_EHCI_BASE;

 static


yes correct, will change this.

 +
 +static int omap_uhh_reset(void)
 +{

[...]

 +     /* perform TLL soft reset, and wait until reset is complete */
 +     writel(OMAP_USBTLL_SYSCONFIG_SOFTRESET, usbtll-sysc);
 +
 +     /* Wait for TLL reset to complete */
 +     while (!(readl(usbtll-syss)  OMAP_USBTLL_SYSSTATUS_RESETDONE))

 Add timeout, fix globally

Sorry I didn't get you here.

The function uses a timeout value init and then same init
value to used to poll for CONFIG_SYS_HZ time for reset to be
done else prints timeout failure.

 +             if (get_timer(init)  CONFIG_SYS_HZ) {
 +                     debug(OMAP EHCI error: timeout resetting TLL\n);
 +                     return -EL3RST;
 +     }
 +

[...]

 +     /* setup ULPI bypass and burst configurations */
 +     reg |= (OMAP_UHH_HOSTCONFIG_INCR4_BURST_EN |
 +             OMAP_UHH_HOSTCONFIG_INCR8_BURST_EN |
 +             OMAP_UHH_HOSTCONFIG_INCR16_BURST_EN);
 +     reg = ~OMAP_UHH_HOSTCONFIG_INCRX_ALIGN_EN;

 clrsetbits_le32 ?

yes can be used.

--
Thanks,
Govindraj.R
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot