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

Message ID 20170913212945.5659-1-stefan@agner.ch
State Awaiting Upstream
Delegated to: Stefano Babic
Headers show
Series
  • [U-Boot,v3,1/2] imx: add macro to detect whether USB PHY is active
Related show

Commit Message

Stefan Agner Sept. 13, 2017, 9:29 p.m.
From: Stefan Agner <stefan.agner@toradex.com>

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&id=a352ed3c5184b95c4c9f7468f5fbb5f43de5e412

Signed-off-by: Stefan Agner <stefan.agner@toradex.com>
Acked-by: Marcel Ziswiler <marcel.ziswiler@toradex.com>
Tested-by: Fabio Estevam <fabio.estevam@nxp.com>
---

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(+)

Comments

Stefan Agner Sept. 13, 2017, 9:28 p.m. | #1
Sorry, the subject should have been v4.

--
Stefan

On 2017-09-13 14:29, Stefan Agner wrote:
> From: Stefan Agner <stefan.agner@toradex.com>
> 
> 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&id=a352ed3c5184b95c4c9f7468f5fbb5f43de5e412
> 
> Signed-off-by: Stefan Agner <stefan.agner@toradex.com>
> Acked-by: Marcel Ziswiler <marcel.ziswiler@toradex.com>
> Tested-by: Fabio Estevam <fabio.estevam@nxp.com>
> ---
> 
> 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 <asm/mach-imx/sys_proto.h>
> +
> +#define USBPHY_PWD		0x00000000
> +
> +#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 <stefan.agner@toradex.com>
> 
> 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&id=a352ed3c5184b95c4c9f7468f5fbb5f43de5e412
> 
> Signed-off-by: Stefan Agner <stefan.agner@toradex.com>
> Acked-by: Marcel Ziswiler <marcel.ziswiler@toradex.com>
> Tested-by: Fabio Estevam <fabio.estevam@nxp.com>
> ---
> 
> 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 <asm/mach-imx/sys_proto.h>
> +
> +#define USBPHY_PWD		0x00000000
> +
> +#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))
Eric Nelson Sept. 14, 2017, 5:23 a.m. | #2
On 09/13/2017 02:29 PM, Stefan Agner wrote:
> From: Stefan Agner <stefan.agner@toradex.com>
> 
> 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&id=a352ed3c5184b95c4c9f7468f5fbb5f43de5e412
> 
> Signed-off-by: Stefan Agner <stefan.agner@toradex.com>
> Acked-by: Marcel Ziswiler <marcel.ziswiler@toradex.com>
> Tested-by: Fabio Estevam <fabio.estevam@nxp.com>
> ---
> 
> 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 <asm/mach-imx/sys_proto.h>

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		0x00000000
> +
> +#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 <eric@nelint.com>
Stefan Agner Sept. 14, 2017, 6:13 a.m. | #3
On 2017-09-13 22:23, Eric Nelson wrote:
> On 09/13/2017 02:29 PM, Stefan Agner wrote:
>> From: Stefan Agner <stefan.agner@toradex.com>
>>
>> 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&id=a352ed3c5184b95c4c9f7468f5fbb5f43de5e412
>>
>> Signed-off-by: Stefan Agner <stefan.agner@toradex.com>
>> Acked-by: Marcel Ziswiler <marcel.ziswiler@toradex.com>
>> Tested-by: Fabio Estevam <fabio.estevam@nxp.com>
>> ---
>>
>> 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 <asm/mach-imx/sys_proto.h>
> 
> 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		0x00000000
>> +
>> +#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 <eric@nelint.com>

Thx!

--
Stefan
Stefano Babic Sept. 20, 2017, 1:36 p.m. | #4
On 13/09/2017 23:29, Stefan Agner wrote:
> From: Stefan Agner <stefan.agner@toradex.com>
> 
> 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&id=a352ed3c5184b95c4c9f7468f5fbb5f43de5e412
> 
> Signed-off-by: Stefan Agner <stefan.agner@toradex.com>
> Acked-by: Marcel Ziswiler <marcel.ziswiler@toradex.com>
> Tested-by: Fabio Estevam <fabio.estevam@nxp.com>
> ---

Applied to u-boot-imx, thanks !

Best regards,
Stefano Babic

Patch

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 <asm/mach-imx/sys_proto.h>
+
+#define USBPHY_PWD		0x00000000
+
+#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))