| Message ID | 20170912235441.19238-1-stefan@agner.ch |
|---|---|
| State | Superseded |
| Delegated to: | Stefano Babic |
| Headers | show |
| Series | [U-Boot,v3,1/2] imx: add macro to detect whether USB PHY is active | expand |
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 <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 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 <asm/mach-imx/sys_proto.h> > + > +#define USBPHY_PWD 0x00000000 > + > +#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
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 <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 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 <asm/mach-imx/sys_proto.h> >> + >> +#define USBPHY_PWD 0x00000000 >> + >> +#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
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. > > 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() -- Stefan > > Best regards, > Stefano > >> On 09/12/2017 04:54 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 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 <asm/mach-imx/sys_proto.h> >>> + >>> +#define USBPHY_PWD 0x00000000 >>> + >>> +#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
On Wed, Sep 13, 2017 at 4:47 PM, Stefan Agner <stefan@agner.ch> 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 :-)
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.
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 <asm/mach-imx/sys_proto.h> + +#define USBPHY_PWD 0x00000000 + +#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))