Re: [U-Boot] [PATCH v3 1/2] imx: add macro to detect whether USB PHY is active

[Stefano Babic]

On 13/09/2017 23:29, Stefan Agner wrote:
> From: Stefan Agner 
> 
> This macro allows to detect whether the USB PHY is active. This
> is helpful to detect if the boot ROM has previously started the
> USB serial downloader.
> 
> The idea is taken from the mfgtool support in the NXP U-Boot:
> http://git.freescale.com/git/cgit.cgi/imx/uboot-imx.git/commit/?h=imx_v2016.03_4.1.15_2.0.0_ga=a352ed3c5184b95c4c9f7468f5fbb5f43de5e412
> 
> Signed-off-by: Stefan Agner 
> Acked-by: Marcel Ziswiler 
> Tested-by: Fabio Estevam 
> ---

Applied to u-boot-imx, thanks !

Best regards,
Stefano Babic


-- 
=
DENX Software Engineering GmbH,  Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sba...@denx.de
=
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH v3 1/2] imx: add macro to detect whether USB PHY is active

[Stefan Agner]

On 2017-09-13 22:23, Eric Nelson wrote:
> On 09/13/2017 02:29 PM, Stefan Agner wrote:
>> From: Stefan Agner 
>>
>> This macro allows to detect whether the USB PHY is active. This
>> is helpful to detect if the boot ROM has previously started the
>> USB serial downloader.
>>
>> The idea is taken from the mfgtool support in the NXP U-Boot:
>> http://git.freescale.com/git/cgit.cgi/imx/uboot-imx.git/commit/?h=imx_v2016.03_4.1.15_2.0.0_ga=a352ed3c5184b95c4c9f7468f5fbb5f43de5e412
>>
>> Signed-off-by: Stefan Agner 
>> Acked-by: Marcel Ziswiler 
>> Tested-by: Fabio Estevam 
>> ---
>>
>> Changes in v4:
>> - Rename macro to is_usbotg_phy_active()
>>
>> Changes in v3: None
>> Changes in v2:
>> - Move macro to sys_proto.h
>> - Renamed from is_boot_from_usb() to is_usbphy_active()
>> - Use defines for register offset and field
>> - Remove tab after define
>> - Remove comment since the actual "magic" is happening and
>>documented at usage side
>>
>>   arch/arm/include/asm/arch-mx6/sys_proto.h | 7 +++
>>   1 file changed, 7 insertions(+)
>>
>> diff --git a/arch/arm/include/asm/arch-mx6/sys_proto.h 
>> b/arch/arm/include/asm/arch-mx6/sys_proto.h
>> index 14f5d948c9..ba73943260 100644
>> --- a/arch/arm/include/asm/arch-mx6/sys_proto.h
>> +++ b/arch/arm/include/asm/arch-mx6/sys_proto.h
>> @@ -6,3 +6,10 @@
>>*/
>> #include 
> 
> I'd be more worried about these macro names (asking that they also
> include "OTG") if they weren't defined in such close proximity to
> their only use.
> 

The registers for the host and otg phy are the same, one might reuse
them for host...

>> +
>> +#define USBPHY_PWD  0x
>> +
>> +#define USBPHY_PWD_RXPWDRX  (1 << 20) /* receiver block power down */
>> +
>> +#define is_usbotg_phy_active(void) (!(readl(USB_PHY0_BASE_ADDR + 
>> USBPHY_PWD) & \
>> +   USBPHY_PWD_RXPWDRX))
>>

In contrast, this function define is really OTG phy specific since we
use PHY0.

> 
> Otherwise,
> 
> Reviewed-by: Eric Nelson 

Thx!

--
Stefan
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH v3 1/2] imx: add macro to detect whether USB PHY is active

[Eric Nelson]

On 09/13/2017 02:29 PM, Stefan Agner wrote:

From: Stefan Agner 

This macro allows to detect whether the USB PHY is active. This
is helpful to detect if the boot ROM has previously started the
USB serial downloader.

The idea is taken from the mfgtool support in the NXP U-Boot:
http://git.freescale.com/git/cgit.cgi/imx/uboot-imx.git/commit/?h=imx_v2016.03_4.1.15_2.0.0_ga=a352ed3c5184b95c4c9f7468f5fbb5f43de5e412

Signed-off-by: Stefan Agner 
Acked-by: Marcel Ziswiler 
Tested-by: Fabio Estevam 
---

Changes in v4:
- Rename macro to is_usbotg_phy_active()

Changes in v3: None
Changes in v2:
- Move macro to sys_proto.h
- Renamed from is_boot_from_usb() to is_usbphy_active()
- Use defines for register offset and field
- Remove tab after define
- Remove comment since the actual "magic" is happening and
   documented at usage side

  arch/arm/include/asm/arch-mx6/sys_proto.h | 7 +++
  1 file changed, 7 insertions(+)

diff --git a/arch/arm/include/asm/arch-mx6/sys_proto.h 
b/arch/arm/include/asm/arch-mx6/sys_proto.h
index 14f5d948c9..ba73943260 100644
--- a/arch/arm/include/asm/arch-mx6/sys_proto.h
+++ b/arch/arm/include/asm/arch-mx6/sys_proto.h
@@ -6,3 +6,10 @@
   */
  
  #include 


I'd be more worried about these macro names (asking that they also
include "OTG") if they weren't defined in such close proximity to
their only use.


+
+#define USBPHY_PWD 0x
+
+#define USBPHY_PWD_RXPWDRX (1 << 20) /* receiver block power down */
+
+#define is_usbotg_phy_active(void) (!(readl(USB_PHY0_BASE_ADDR + USBPHY_PWD) & 
\
+  USBPHY_PWD_RXPWDRX))



Otherwise,

Reviewed-by: Eric Nelson 
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH v3 1/2] imx: add macro to detect whether USB PHY is active

[Stefan Agner]

Sorry, the subject should have been v4.

--
Stefan

On 2017-09-13 14:29, Stefan Agner wrote:
> From: Stefan Agner 
> 
> This macro allows to detect whether the USB PHY is active. This
> is helpful to detect if the boot ROM has previously started the
> USB serial downloader.
> 
> The idea is taken from the mfgtool support in the NXP U-Boot:
> http://git.freescale.com/git/cgit.cgi/imx/uboot-imx.git/commit/?h=imx_v2016.03_4.1.15_2.0.0_ga=a352ed3c5184b95c4c9f7468f5fbb5f43de5e412
> 
> Signed-off-by: Stefan Agner 
> Acked-by: Marcel Ziswiler 
> Tested-by: Fabio Estevam 
> ---
> 
> Changes in v4:
> - Rename macro to is_usbotg_phy_active()
> 
> Changes in v3: None
> Changes in v2:
> - Move macro to sys_proto.h
> - Renamed from is_boot_from_usb() to is_usbphy_active()
> - Use defines for register offset and field
> - Remove tab after define
> - Remove comment since the actual "magic" is happening and
>   documented at usage side
> 
>  arch/arm/include/asm/arch-mx6/sys_proto.h | 7 +++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/arch/arm/include/asm/arch-mx6/sys_proto.h
> b/arch/arm/include/asm/arch-mx6/sys_proto.h
> index 14f5d948c9..ba73943260 100644
> --- a/arch/arm/include/asm/arch-mx6/sys_proto.h
> +++ b/arch/arm/include/asm/arch-mx6/sys_proto.h
> @@ -6,3 +6,10 @@
>   */
>  
>  #include 
> +
> +#define USBPHY_PWD   0x
> +
> +#define USBPHY_PWD_RXPWDRX   (1 << 20) /* receiver block power down */
> +
> +#define is_usbotg_phy_active(void) (!(readl(USB_PHY0_BASE_ADDR +
> USBPHY_PWD) & \
> +USBPHY_PWD_RXPWDRX))


On 2017-09-13 14:29, Stefan Agner wrote:
> From: Stefan Agner 
> 
> This macro allows to detect whether the USB PHY is active. This
> is helpful to detect if the boot ROM has previously started the
> USB serial downloader.
> 
> The idea is taken from the mfgtool support in the NXP U-Boot:
> http://git.freescale.com/git/cgit.cgi/imx/uboot-imx.git/commit/?h=imx_v2016.03_4.1.15_2.0.0_ga=a352ed3c5184b95c4c9f7468f5fbb5f43de5e412
> 
> Signed-off-by: Stefan Agner 
> Acked-by: Marcel Ziswiler 
> Tested-by: Fabio Estevam 
> ---
> 
> Changes in v4:
> - Rename macro to is_usbotg_phy_active()
> 
> Changes in v3: None
> Changes in v2:
> - Move macro to sys_proto.h
> - Renamed from is_boot_from_usb() to is_usbphy_active()
> - Use defines for register offset and field
> - Remove tab after define
> - Remove comment since the actual "magic" is happening and
>   documented at usage side
> 
>  arch/arm/include/asm/arch-mx6/sys_proto.h | 7 +++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/arch/arm/include/asm/arch-mx6/sys_proto.h
> b/arch/arm/include/asm/arch-mx6/sys_proto.h
> index 14f5d948c9..ba73943260 100644
> --- a/arch/arm/include/asm/arch-mx6/sys_proto.h
> +++ b/arch/arm/include/asm/arch-mx6/sys_proto.h
> @@ -6,3 +6,10 @@
>   */
>  
>  #include 
> +
> +#define USBPHY_PWD   0x
> +
> +#define USBPHY_PWD_RXPWDRX   (1 << 20) /* receiver block power down */
> +
> +#define is_usbotg_phy_active(void) (!(readl(USB_PHY0_BASE_ADDR +
> USBPHY_PWD) & \
> +USBPHY_PWD_RXPWDRX))
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

[U-Boot] [PATCH v3 1/2] imx: add macro to detect whether USB PHY is active

[Stefan Agner]

From: Stefan Agner 

This macro allows to detect whether the USB PHY is active. This
is helpful to detect if the boot ROM has previously started the
USB serial downloader.

The idea is taken from the mfgtool support in the NXP U-Boot:
http://git.freescale.com/git/cgit.cgi/imx/uboot-imx.git/commit/?h=imx_v2016.03_4.1.15_2.0.0_ga=a352ed3c5184b95c4c9f7468f5fbb5f43de5e412

Signed-off-by: Stefan Agner 
Acked-by: Marcel Ziswiler 
Tested-by: Fabio Estevam 
---

Changes in v4:
- Rename macro to is_usbotg_phy_active()

Changes in v3: None
Changes in v2:
- Move macro to sys_proto.h
- Renamed from is_boot_from_usb() to is_usbphy_active()
- Use defines for register offset and field
- Remove tab after define
- Remove comment since the actual "magic" is happening and
  documented at usage side

 arch/arm/include/asm/arch-mx6/sys_proto.h | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/arch/arm/include/asm/arch-mx6/sys_proto.h 
b/arch/arm/include/asm/arch-mx6/sys_proto.h
index 14f5d948c9..ba73943260 100644
--- a/arch/arm/include/asm/arch-mx6/sys_proto.h
+++ b/arch/arm/include/asm/arch-mx6/sys_proto.h
@@ -6,3 +6,10 @@
  */
 
 #include 
+
+#define USBPHY_PWD 0x
+
+#define USBPHY_PWD_RXPWDRX (1 << 20) /* receiver block power down */
+
+#define is_usbotg_phy_active(void) (!(readl(USB_PHY0_BASE_ADDR + USBPHY_PWD) & 
\
+  USBPHY_PWD_RXPWDRX))
-- 
2.14.1

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

Re: [U-Boot] [PATCH v3 1/2] imx: add macro to detect whether USB PHY is active

[Eric Nelson]

Hi Stefan,

On 09/13/2017 12:47 PM, Stefan Agner wrote:

On 2017-09-13 02:19, Stefano Babic wrote:

Hi Eric, Stefan,

On 13/09/2017 02:30, Eric Nelson wrote:

Hi Stefan,

I hate to be fussy about this, but I don't think I saw a reply
to my earlier comment about the term "USB PHY".

https://lists.denx.de/pipermail/u-boot/2017-September/305123.html

Since i.MX6 SoCs have USB **Host** Phy's as well as the USB OTG Phy,
this patch is a bit misleading.

There's no reference to OTG anywhere in this or patch 2.



Hm, sorry I missed that.


No worries. We're here to nag...



To be consistent with the names, Eric is right. We have PHY0 for OTG and
PHY1 for Host. If you still want to use as name is_usbphy_active, then
this should take as parameter the phy number, or you switch to a "otg"
name to ensure that it is only for the OTG interface.


For the registers itself I followed the notation used in chapter 66 and
arch/arm/include/asm/arch-mx6/imx-regs.h.

Renaming the function macro makes sense I agree.

The USB serial downloader only works with the OTG USB PHY anyway, so I
will rename it. I like to have USB still in there, how about:

is_usbotg_phy_active()



Perfect.
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH v3 1/2] imx: add macro to detect whether USB PHY is active

[Fabio Estevam]

On Wed, Sep 13, 2017 at 4:47 PM, Stefan Agner  wrote:

> The USB serial downloader only works with the OTG USB PHY anyway, so I
> will rename it. I like to have USB still in there, how about:
>
> is_usbotg_phy_active()

Looks fine. Please send a v4 with it and we should be good :-)
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH v3 1/2] imx: add macro to detect whether USB PHY is active

[Stefano Babic]

Hi Eric, Stefan,

On 13/09/2017 02:30, Eric Nelson wrote:
> Hi Stefan,
> 
> I hate to be fussy about this, but I don't think I saw a reply
> to my earlier comment about the term "USB PHY".
> 
> https://lists.denx.de/pipermail/u-boot/2017-September/305123.html
> 
> Since i.MX6 SoCs have USB **Host** Phy's as well as the USB OTG Phy,
> this patch is a bit misleading.
> 
> There's no reference to OTG anywhere in this or patch 2.
> 

To be consistent with the names, Eric is right. We have PHY0 for OTG and
PHY1 for Host. If you still want to use as name is_usbphy_active, then
this should take as parameter the phy number, or you switch to a "otg"
name to ensure that it is only for the OTG interface.

Best regards,
Stefano

> On 09/12/2017 04:54 PM, Stefan Agner wrote:
>> From: Stefan Agner 
>>
>> This macro allows to detect whether the USB PHY is active. This
>> is helpful to detect if the boot ROM has previously started the
>> USB serial downloader.
>>
>> The idea is taken from the mfgtool support in the NXP U-Boot:
>> http://git.freescale.com/git/cgit.cgi/imx/uboot-imx.git/commit/?h=imx_v2016.03_4.1.15_2.0.0_ga=a352ed3c5184b95c4c9f7468f5fbb5f43de5e412
>>
>>
>> Signed-off-by: Stefan Agner 
>> Acked-by: Marcel Ziswiler 
>> Tested-by: Fabio Estevam 
>> ---
>>
>> Changes in v3: None
>> Changes in v2:
>> - Move macro to sys_proto.h
>> - Renamed from is_boot_from_usb() to is_usbphy_active()
>> - Use defines for register offset and field
>> - Remove tab after define
>> - Remove comment since the actual "magic" is happening and
>>documented at usage side
>>
>>   arch/arm/include/asm/arch-mx6/sys_proto.h | 7 +++
>>   1 file changed, 7 insertions(+)
>>
>> diff --git a/arch/arm/include/asm/arch-mx6/sys_proto.h
>> b/arch/arm/include/asm/arch-mx6/sys_proto.h
>> index 14f5d948c9..9d4b1d6768 100644
>> --- a/arch/arm/include/asm/arch-mx6/sys_proto.h
>> +++ b/arch/arm/include/asm/arch-mx6/sys_proto.h
>> @@ -6,3 +6,10 @@
>>*/
>> #include 
>> +
>> +#define USBPHY_PWD0x
>> +
>> +#define USBPHY_PWD_RXPWDRX(1 << 20) /* receiver block power down */
>> +
>> +#define is_usbphy_active(void)(!(readl(USB_PHY0_BASE_ADDR +
>> USBPHY_PWD) & \
>> +   USBPHY_PWD_RXPWDRX))
>>
> 
> Regards,
> 
> 
> Eric


-- 
=
DENX Software Engineering GmbH,  Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sba...@denx.de
=
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH v3 1/2] imx: add macro to detect whether USB PHY is active

[Eric Nelson]

Hi Stefan,

I hate to be fussy about this, but I don't think I saw a reply
to my earlier comment about the term "USB PHY".

https://lists.denx.de/pipermail/u-boot/2017-September/305123.html

Since i.MX6 SoCs have USB **Host** Phy's as well as the USB OTG Phy,
this patch is a bit misleading.

There's no reference to OTG anywhere in this or patch 2.

On 09/12/2017 04:54 PM, Stefan Agner wrote:

From: Stefan Agner 

This macro allows to detect whether the USB PHY is active. This
is helpful to detect if the boot ROM has previously started the
USB serial downloader.

The idea is taken from the mfgtool support in the NXP U-Boot:
http://git.freescale.com/git/cgit.cgi/imx/uboot-imx.git/commit/?h=imx_v2016.03_4.1.15_2.0.0_ga=a352ed3c5184b95c4c9f7468f5fbb5f43de5e412

Signed-off-by: Stefan Agner 
Acked-by: Marcel Ziswiler 
Tested-by: Fabio Estevam 
---

Changes in v3: None
Changes in v2:
- Move macro to sys_proto.h
- Renamed from is_boot_from_usb() to is_usbphy_active()
- Use defines for register offset and field
- Remove tab after define
- Remove comment since the actual "magic" is happening and
   documented at usage side

  arch/arm/include/asm/arch-mx6/sys_proto.h | 7 +++
  1 file changed, 7 insertions(+)

diff --git a/arch/arm/include/asm/arch-mx6/sys_proto.h 
b/arch/arm/include/asm/arch-mx6/sys_proto.h
index 14f5d948c9..9d4b1d6768 100644
--- a/arch/arm/include/asm/arch-mx6/sys_proto.h
+++ b/arch/arm/include/asm/arch-mx6/sys_proto.h
@@ -6,3 +6,10 @@
   */
  
  #include 

+
+#define USBPHY_PWD 0x
+
+#define USBPHY_PWD_RXPWDRX (1 << 20) /* receiver block power down */
+
+#define is_usbphy_active(void) (!(readl(USB_PHY0_BASE_ADDR + USBPHY_PWD) & \
+  USBPHY_PWD_RXPWDRX))



Regards,


Eric
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

[U-Boot] [PATCH v3 1/2] imx: add macro to detect whether USB PHY is active

[Stefan Agner]

From: Stefan Agner 

This macro allows to detect whether the USB PHY is active. This
is helpful to detect if the boot ROM has previously started the
USB serial downloader.

The idea is taken from the mfgtool support in the NXP U-Boot:
http://git.freescale.com/git/cgit.cgi/imx/uboot-imx.git/commit/?h=imx_v2016.03_4.1.15_2.0.0_ga=a352ed3c5184b95c4c9f7468f5fbb5f43de5e412

Signed-off-by: Stefan Agner 
Acked-by: Marcel Ziswiler 
Tested-by: Fabio Estevam 
---

Changes in v3: None
Changes in v2:
- Move macro to sys_proto.h
- Renamed from is_boot_from_usb() to is_usbphy_active()
- Use defines for register offset and field
- Remove tab after define
- Remove comment since the actual "magic" is happening and
  documented at usage side

 arch/arm/include/asm/arch-mx6/sys_proto.h | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/arch/arm/include/asm/arch-mx6/sys_proto.h 
b/arch/arm/include/asm/arch-mx6/sys_proto.h
index 14f5d948c9..9d4b1d6768 100644
--- a/arch/arm/include/asm/arch-mx6/sys_proto.h
+++ b/arch/arm/include/asm/arch-mx6/sys_proto.h
@@ -6,3 +6,10 @@
  */
 
 #include 
+
+#define USBPHY_PWD 0x
+
+#define USBPHY_PWD_RXPWDRX (1 << 20) /* receiver block power down */
+
+#define is_usbphy_active(void) (!(readl(USB_PHY0_BASE_ADDR + USBPHY_PWD) & \
+  USBPHY_PWD_RXPWDRX))
-- 
2.14.1

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