| Message ID | 20160703193354.25900-3-stefan@agner.ch |
|---|---|
| State | Changes Requested |
| Delegated to: | Stefano Babic |
| Headers | show |
On 07/03/2016 09:33 PM, Stefan Agner wrote: > From: Stefan Agner <stefan.agner@toradex.com> > > USBNC_n_CTRL1 bit 9 actually controls the power pin polarity. > Rename UCTRL_PM to align reference manual and set the bit in > the appropriate callback usb_power_config. > > Signed-off-by: Stefan Agner <stefan.agner@toradex.com> Just for the extra safety, can you please check all the MX6 datasheets and also MX7 to be dead sure this bit is always OTG power polarity ? Also, you always set the bit, but I'd expect the bit to be cleared by default, so wouldn't this break existing configurations? > --- > > drivers/usb/host/ehci-mx6.c | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/drivers/usb/host/ehci-mx6.c b/drivers/usb/host/ehci-mx6.c > index bb48d0d..0dbabb2 100644 > --- a/drivers/usb/host/ehci-mx6.c > +++ b/drivers/usb/host/ehci-mx6.c > @@ -49,7 +49,7 @@ > #define USBNC_OFFSET 0x200 > #define USBNC_PHYSTATUS_ID_DIG (1 << 4) /* otg_id status */ > #define USBNC_PHYCFG2_ACAENB (1 << 4) /* otg_id detection enable */ > -#define UCTRL_PM (1 << 9) /* OTG Power Mask */ > +#define UCTRL_PWR_POL (1 << 9) /* OTG Polarity of Power Pin */ > #define UCTRL_OVER_CUR_POL (1 << 8) /* OTG Polarity of Overcurrent */ > #define UCTRL_OVER_CUR_DIS (1 << 7) /* Disable OTG Overcurrent Detection */ > > @@ -206,9 +206,13 @@ static void usb_power_config(int index) > struct usbnc_regs *usbnc = (struct usbnc_regs *)(USB_BASE_ADDR + > (0x10000 * index) + USBNC_OFFSET); > void __iomem *phy_cfg2 = (void __iomem *)(&usbnc->phy_cfg2); > + void __iomem *ctrl = (void __iomem *)(&usbnc->ctrl1); > > /* Enable usb_otg_id detection */ > setbits_le32(phy_cfg2, USBNC_PHYCFG2_ACAENB); > + > + /* Set power polarity to high active */ > + setbits_le32(ctrl, UCTRL_PWR_POL); > } > > int usb_phy_mode(int port) > @@ -246,11 +250,7 @@ static void usb_oc_config(int index) > setbits_le32(ctrl, UCTRL_OVER_CUR_POL); > #endif > > -#if defined(CONFIG_MX6) > setbits_le32(ctrl, UCTRL_OVER_CUR_DIS); > -#elif defined(CONFIG_MX7) > - setbits_le32(ctrl, UCTRL_OVER_CUR_DIS | UCTRL_PM); > -#endif > } > > /** >
On 2016-07-03 15:32, Marek Vasut wrote: > On 07/03/2016 09:33 PM, Stefan Agner wrote: >> From: Stefan Agner <stefan.agner@toradex.com> >> >> USBNC_n_CTRL1 bit 9 actually controls the power pin polarity. >> Rename UCTRL_PM to align reference manual and set the bit in >> the appropriate callback usb_power_config. >> >> Signed-off-by: Stefan Agner <stefan.agner@toradex.com> > > Just for the extra safety, can you please check all the MX6 datasheets > and also MX7 to be dead sure this bit is always OTG power polarity ? Checked i.MX 6Solo/DualLite, Dual/Quad, SoloLite and SoloX data sheet, all call that bit PWR_POL and its described the same way. > > Also, you always set the bit, but I'd expect the bit to be cleared by > default, so wouldn't this break existing configurations? > The bit is cleared on reset yes. I don't understand why it should break existing configuration, the code does exactly the same as before: Sets the bit on i.MX 7 (note that this usb_power_config function is in a if defined(CONFIG_MX7) section. -- Stefan >> --- >> >> drivers/usb/host/ehci-mx6.c | 10 +++++----- >> 1 file changed, 5 insertions(+), 5 deletions(-) >> >> diff --git a/drivers/usb/host/ehci-mx6.c b/drivers/usb/host/ehci-mx6.c >> index bb48d0d..0dbabb2 100644 >> --- a/drivers/usb/host/ehci-mx6.c >> +++ b/drivers/usb/host/ehci-mx6.c >> @@ -49,7 +49,7 @@ >> #define USBNC_OFFSET 0x200 >> #define USBNC_PHYSTATUS_ID_DIG (1 << 4) /* otg_id status */ >> #define USBNC_PHYCFG2_ACAENB (1 << 4) /* otg_id detection enable */ >> -#define UCTRL_PM (1 << 9) /* OTG Power Mask */ >> +#define UCTRL_PWR_POL (1 << 9) /* OTG Polarity of Power Pin */ >> #define UCTRL_OVER_CUR_POL (1 << 8) /* OTG Polarity of Overcurrent */ >> #define UCTRL_OVER_CUR_DIS (1 << 7) /* Disable OTG Overcurrent Detection */ >> >> @@ -206,9 +206,13 @@ static void usb_power_config(int index) >> struct usbnc_regs *usbnc = (struct usbnc_regs *)(USB_BASE_ADDR + >> (0x10000 * index) + USBNC_OFFSET); >> void __iomem *phy_cfg2 = (void __iomem *)(&usbnc->phy_cfg2); >> + void __iomem *ctrl = (void __iomem *)(&usbnc->ctrl1); >> >> /* Enable usb_otg_id detection */ >> setbits_le32(phy_cfg2, USBNC_PHYCFG2_ACAENB); >> + >> + /* Set power polarity to high active */ >> + setbits_le32(ctrl, UCTRL_PWR_POL); >> } >> >> int usb_phy_mode(int port) >> @@ -246,11 +250,7 @@ static void usb_oc_config(int index) >> setbits_le32(ctrl, UCTRL_OVER_CUR_POL); >> #endif >> >> -#if defined(CONFIG_MX6) >> setbits_le32(ctrl, UCTRL_OVER_CUR_DIS); >> -#elif defined(CONFIG_MX7) >> - setbits_le32(ctrl, UCTRL_OVER_CUR_DIS | UCTRL_PM); >> -#endif >> } >> >> /** >>
On 07/05/2016 10:28 PM, Stefan Agner wrote: > On 2016-07-03 15:32, Marek Vasut wrote: >> On 07/03/2016 09:33 PM, Stefan Agner wrote: >>> From: Stefan Agner <stefan.agner@toradex.com> >>> >>> USBNC_n_CTRL1 bit 9 actually controls the power pin polarity. >>> Rename UCTRL_PM to align reference manual and set the bit in >>> the appropriate callback usb_power_config. >>> >>> Signed-off-by: Stefan Agner <stefan.agner@toradex.com> >> >> Just for the extra safety, can you please check all the MX6 datasheets >> and also MX7 to be dead sure this bit is always OTG power polarity ? > > Checked i.MX 6Solo/DualLite, Dual/Quad, SoloLite and SoloX data sheet, > all call that bit PWR_POL and its described the same way. > >> >> Also, you always set the bit, but I'd expect the bit to be cleared by >> default, so wouldn't this break existing configurations? >> > > The bit is cleared on reset yes. > > I don't understand why it should break existing configuration, the code > does exactly the same as before: Sets the bit on i.MX 7 (note that this > usb_power_config function is in a if defined(CONFIG_MX7) section. Ah, sorry, taking a second look, you're right it doesn't change the behavior. > -- > Stefan > > >>> --- >>> >>> drivers/usb/host/ehci-mx6.c | 10 +++++----- >>> 1 file changed, 5 insertions(+), 5 deletions(-) >>> >>> diff --git a/drivers/usb/host/ehci-mx6.c b/drivers/usb/host/ehci-mx6.c >>> index bb48d0d..0dbabb2 100644 >>> --- a/drivers/usb/host/ehci-mx6.c >>> +++ b/drivers/usb/host/ehci-mx6.c >>> @@ -49,7 +49,7 @@ >>> #define USBNC_OFFSET 0x200 >>> #define USBNC_PHYSTATUS_ID_DIG (1 << 4) /* otg_id status */ >>> #define USBNC_PHYCFG2_ACAENB (1 << 4) /* otg_id detection enable */ >>> -#define UCTRL_PM (1 << 9) /* OTG Power Mask */ >>> +#define UCTRL_PWR_POL (1 << 9) /* OTG Polarity of Power Pin */ >>> #define UCTRL_OVER_CUR_POL (1 << 8) /* OTG Polarity of Overcurrent */ >>> #define UCTRL_OVER_CUR_DIS (1 << 7) /* Disable OTG Overcurrent Detection */ >>> >>> @@ -206,9 +206,13 @@ static void usb_power_config(int index) >>> struct usbnc_regs *usbnc = (struct usbnc_regs *)(USB_BASE_ADDR + >>> (0x10000 * index) + USBNC_OFFSET); >>> void __iomem *phy_cfg2 = (void __iomem *)(&usbnc->phy_cfg2); >>> + void __iomem *ctrl = (void __iomem *)(&usbnc->ctrl1); >>> >>> /* Enable usb_otg_id detection */ >>> setbits_le32(phy_cfg2, USBNC_PHYCFG2_ACAENB); >>> + >>> + /* Set power polarity to high active */ >>> + setbits_le32(ctrl, UCTRL_PWR_POL); >>> } >>> >>> int usb_phy_mode(int port) >>> @@ -246,11 +250,7 @@ static void usb_oc_config(int index) >>> setbits_le32(ctrl, UCTRL_OVER_CUR_POL); >>> #endif >>> >>> -#if defined(CONFIG_MX6) >>> setbits_le32(ctrl, UCTRL_OVER_CUR_DIS); >>> -#elif defined(CONFIG_MX7) >>> - setbits_le32(ctrl, UCTRL_OVER_CUR_DIS | UCTRL_PM); >>> -#endif >>> } >>> >>> /** >>>
diff --git a/drivers/usb/host/ehci-mx6.c b/drivers/usb/host/ehci-mx6.c index bb48d0d..0dbabb2 100644 --- a/drivers/usb/host/ehci-mx6.c +++ b/drivers/usb/host/ehci-mx6.c @@ -49,7 +49,7 @@ #define USBNC_OFFSET 0x200 #define USBNC_PHYSTATUS_ID_DIG (1 << 4) /* otg_id status */ #define USBNC_PHYCFG2_ACAENB (1 << 4) /* otg_id detection enable */ -#define UCTRL_PM (1 << 9) /* OTG Power Mask */ +#define UCTRL_PWR_POL (1 << 9) /* OTG Polarity of Power Pin */ #define UCTRL_OVER_CUR_POL (1 << 8) /* OTG Polarity of Overcurrent */ #define UCTRL_OVER_CUR_DIS (1 << 7) /* Disable OTG Overcurrent Detection */ @@ -206,9 +206,13 @@ static void usb_power_config(int index) struct usbnc_regs *usbnc = (struct usbnc_regs *)(USB_BASE_ADDR + (0x10000 * index) + USBNC_OFFSET); void __iomem *phy_cfg2 = (void __iomem *)(&usbnc->phy_cfg2); + void __iomem *ctrl = (void __iomem *)(&usbnc->ctrl1); /* Enable usb_otg_id detection */ setbits_le32(phy_cfg2, USBNC_PHYCFG2_ACAENB); + + /* Set power polarity to high active */ + setbits_le32(ctrl, UCTRL_PWR_POL); } int usb_phy_mode(int port) @@ -246,11 +250,7 @@ static void usb_oc_config(int index) setbits_le32(ctrl, UCTRL_OVER_CUR_POL); #endif -#if defined(CONFIG_MX6) setbits_le32(ctrl, UCTRL_OVER_CUR_DIS); -#elif defined(CONFIG_MX7) - setbits_le32(ctrl, UCTRL_OVER_CUR_DIS | UCTRL_PM); -#endif } /**