Re: [U-Boot] [PATCH 3/3] imx: mx6sllevk: add usb support

2016-12-22 Thread Peng Fan 

Hi Marek,
> -Original Message-
> From: Marek Vasut [mailto:ma...@denx.de]
> Sent: Thursday, December 22, 2016 4:58 PM
> To: Peng Fan ; sba...@denx.de
> Cc: u-boot@lists.denx.de; van.free...@gmail.com
> Subject: Re: [PATCH 3/3] imx: mx6sllevk: add usb support
> 
> On 12/22/2016 09:55 AM, Peng Fan wrote:
> >
> >
> >>> +#define USB_OTHERREGS_OFFSET   0x800
> >>> +#define UCTRL_PWR_POL  (1 << 9)
> >>> +
> >>> +int board_ehci_hcd_init(int port) {
> >>> + u32 *usbnc_usb_ctrl;
> >>> +
> >>> + if (port > 1)
> >>> + return -EINVAL;
> >>> +
> >>> + usbnc_usb_ctrl = (u32 *)(USB_BASE_ADDR +
> >> USB_OTHERREGS_OFFSET
> >> +
> >>> +  port * 4);
> >>> +
> >>> + /* Set Power polarity */
> >>> + setbits_le32(usbnc_usb_ctrl, UCTRL_PWR_POL);
> >>> + return 0;
> >>> +}
> >>
> >> Is this function similar to what usb_oc_config() does ?
> >
> > No, this bit is not for overcurrent. According to RM, this is
> > OTG Power Polarity This bit should be set according to power
> > switch's enable
>  polarity.
> > 1 Power switch has an active-high enable input
> > 0 Power switch has an active-low enable input
> >
> > This is board specific.
> 
>  Great, except it should also be part of the driver , the same
>  way as polarity is configured, yes ?
> >>>
> >>> I just found that there is code for mx7,
> >>> http://lists.denx.de/pipermail/u-boot/2016-July/260321.html
> >>>
> >>> Then do you agree I add such piece code in usb_power_config for
> mx6?
> >>> "
> >>> #ifdef CONFIG_MXC_USB_OTG_HACTIVE
> >>> setbits_le32(ctrl, UCTRL_PWR_POL); #else
> >>> clrbits_le32(ctrl, UCTRL_PWR_POL); #endif "
> >>
> >> Didn't you add DT support in a patch sent like ... yesterday ? :)
> >> Just use DT property instead of ifdef.
> >
> > There is no property for otg power polatiry in upstream Linux. I
> > would not like to introduce one in U-Boot. We can drop the ifdef
> > when there is an
>  property in upstream. What do you think?
> 
>  So uh, how can the USB work in mainline Linux on that board ?
> >>>
> >>> Confirmed with IC, if using gpio to control vbus power, no need to
> >>> configure
> >> this bit.
> >>> Since in dts, we are using gpio regulator to control vbus, this
> >>> piece code in patch 3/3 can be discarded. I'll drop it in V2.
> >>
> >> Hum, OK. Let's revisit this when we need this then ?
> >
> > Take i.MX6SLL as an example,
> > The pin key_col4 can be configured with the two functions.
> > [1] MX6_PAD_KEY_COL4__USB_OTG1_PWR
> > [2] MX6_PAD_KEY_COL4__GPIO4_IO00
> >
> > In fsl i.mx uboot code, the common way is to use [1] and configure the
> polarity to enable the vbus power.
> > But in kernel, it is configured as [2] and use gpio regulator to enable the 
> > vbus
> power.
> >
> > This is board specific. There is no upstream property for this bit.
> > When there is an upstream property for this, We could add it in probe
> function for those that use [1] in dts pinmux.
> >
> > Now I am using the same dts from kernel which this pinmux configured as [2],
> so this piece code is not needed for now.
> 
> Well, can you submit a proposal with the new prop for devicetree@ ?
> It can then be improved in linux too ...

I'll talk with our internal Linux usb maintainer to see whether he has plan to 
do this.

Thanks,
Peng.
> 
> --
> Best regards,
> Marek Vasut
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH 3/3] imx: mx6sllevk: add usb support

2016-12-22 Thread Peng Fan 


> > +#define USB_OTHERREGS_OFFSET   0x800
> > +#define UCTRL_PWR_POL  (1 << 9)
> > +
> > +int board_ehci_hcd_init(int port) {
> > +   u32 *usbnc_usb_ctrl;
> > +
> > +   if (port > 1)
> > +   return -EINVAL;
> > +
> > +   usbnc_usb_ctrl = (u32 *)(USB_BASE_ADDR +
>  USB_OTHERREGS_OFFSET
>  +
> > +port * 4);
> > +
> > +   /* Set Power polarity */
> > +   setbits_le32(usbnc_usb_ctrl, UCTRL_PWR_POL);
> > +   return 0;
> > +}
> 
>  Is this function similar to what usb_oc_config() does ?
> >>>
> >>> No, this bit is not for overcurrent. According to RM, this is
> >>> OTG Power Polarity This bit should be set according to power
> >>> switch's enable
> >> polarity.
> >>> 1 Power switch has an active-high enable input
> >>> 0 Power switch has an active-low enable input
> >>>
> >>> This is board specific.
> >>
> >> Great, except it should also be part of the driver , the same way
> >> as polarity is configured, yes ?
> >
> > I just found that there is code for mx7,
> > http://lists.denx.de/pipermail/u-boot/2016-July/260321.html
> >
> > Then do you agree I add such piece code in usb_power_config for mx6?
> > "
> > #ifdef CONFIG_MXC_USB_OTG_HACTIVE
> > setbits_le32(ctrl, UCTRL_PWR_POL); #else
> > clrbits_le32(ctrl, UCTRL_PWR_POL); #endif "
> 
>  Didn't you add DT support in a patch sent like ... yesterday ? :)
>  Just use DT property instead of ifdef.
> >>>
> >>> There is no property for otg power polatiry in upstream Linux. I
> >>> would not like to introduce one in U-Boot. We can drop the ifdef
> >>> when there is an
> >> property in upstream. What do you think?
> >>
> >> So uh, how can the USB work in mainline Linux on that board ?
> >
> > Confirmed with IC, if using gpio to control vbus power, no need to configure
> this bit.
> > Since in dts, we are using gpio regulator to control vbus, this piece
> > code in patch 3/3 can be discarded. I'll drop it in V2.
> 
> Hum, OK. Let's revisit this when we need this then ?

Take i.MX6SLL as an example,
The pin key_col4 can be configured with the two functions.
[1] MX6_PAD_KEY_COL4__USB_OTG1_PWR 
[2] MX6_PAD_KEY_COL4__GPIO4_IO00

In fsl i.mx uboot code, the common way is to use [1] and configure the polarity 
to enable the vbus power.
But in kernel, it is configured as [2] and use gpio regulator to enable the 
vbus power.

This is board specific. There is no upstream property for this bit. When there 
is an upstream property for this,
We could add it in probe function for those that use [1] in dts pinmux.

Now I am using the same dts from kernel which this pinmux configured as [2], so 
this piece code is not needed for now.

Thanks,
Peng.
> 
> --
> Best regards,
> Marek Vasut
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH 3/3] imx: mx6sllevk: add usb support

2016-12-22 Thread Peng Fan 


> -Original Message-
> From: Marek Vasut [mailto:ma...@denx.de]
> Sent: Thursday, December 22, 2016 3:12 PM
> To: Peng Fan ; sba...@denx.de
> Cc: u-boot@lists.denx.de; van.free...@gmail.com
> Subject: Re: [PATCH 3/3] imx: mx6sllevk: add usb support
> 
> On 12/22/2016 07:38 AM, Peng Fan wrote:
> >
> >
> >> -Original Message-
> >> From: Marek Vasut [mailto:ma...@denx.de]
> >> Sent: Thursday, December 22, 2016 2:12 PM
> >> To: Peng Fan ; sba...@denx.de
> >> Cc: u-boot@lists.denx.de; van.free...@gmail.com
> >> Subject: Re: [PATCH 3/3] imx: mx6sllevk: add usb support
> >>
> >> On 12/22/2016 07:03 AM, Peng Fan wrote:
> >>>
> >>> Hi Marek,
> >>>
>  -Original Message-
>  From: Marek Vasut [mailto:ma...@denx.de]
>  Sent: Thursday, December 22, 2016 12:40 PM
>  To: Peng Fan ; sba...@denx.de
>  Cc: u-boot@lists.denx.de; van.free...@gmail.com
>  Subject: Re: [PATCH 3/3] imx: mx6sllevk: add usb support
> 
>  On 12/22/2016 02:13 AM, Peng Fan wrote:
> >
> >
> >> -Original Message-
> >> From: Marek Vasut [mailto:ma...@denx.de]
> >> Sent: Wednesday, December 21, 2016 10:10 PM
> >> To: Peng Fan ; sba...@denx.de
> >> Cc: u-boot@lists.denx.de; van.free...@gmail.com
> >> Subject: Re: [PATCH 3/3] imx: mx6sllevk: add usb support
> >>
> >> On 12/21/2016 09:14 AM, Peng Fan wrote:
> >>> Add usb support for mx6sllevk board.
> >>>
> >>> Signed-off-by: Peng Fan 
> >>> Cc: Stefano Babic 
> >>> ---
> > [..]
> >
> >>> +
> >>> +#define USB_OTHERREGS_OFFSET   0x800
> >>> +#define UCTRL_PWR_POL  (1 << 9)
> >>> +
> >>> +int board_ehci_hcd_init(int port) {
> >>> + u32 *usbnc_usb_ctrl;
> >>> +
> >>> + if (port > 1)
> >>> + return -EINVAL;
> >>> +
> >>> + usbnc_usb_ctrl = (u32 *)(USB_BASE_ADDR +
> >> USB_OTHERREGS_OFFSET
> >> +
> >>> +  port * 4);
> >>> +
> >>> + /* Set Power polarity */
> >>> + setbits_le32(usbnc_usb_ctrl, UCTRL_PWR_POL);
> >>> + return 0;
> >>> +}
> >>
> >> Is this function similar to what usb_oc_config() does ?
> >
> > No, this bit is not for overcurrent. According to RM, this is OTG
> > Power Polarity This bit should be set according to power switch's
> > enable
>  polarity.
> > 1 Power switch has an active-high enable input
> > 0 Power switch has an active-low enable input
> >
> > This is board specific.
> 
>  Great, except it should also be part of the driver , the same way
>  as polarity is configured, yes ?
> >>>
> >>> I just found that there is code for mx7,
> >>> http://lists.denx.de/pipermail/u-boot/2016-July/260321.html
> >>>
> >>> Then do you agree I add such piece code in usb_power_config for mx6?
> >>> "
> >>> #ifdef CONFIG_MXC_USB_OTG_HACTIVE
> >>> setbits_le32(ctrl, UCTRL_PWR_POL); #else
> >>> clrbits_le32(ctrl, UCTRL_PWR_POL); #endif "
> >>
> >> Didn't you add DT support in a patch sent like ... yesterday ? :)
> >> Just use DT property instead of ifdef.
> >
> > There is no property for otg power polatiry in upstream Linux. I would
> > not like to introduce one in U-Boot. We can drop the ifdef when there is an
> property in upstream. What do you think?
> 
> So uh, how can the USB work in mainline Linux on that board ?

Confirmed with IC, if using gpio to control vbus power, no need to configure 
this bit.
Since in dts, we are using gpio regulator to control vbus, this piece code in 
patch 3/3
can be discarded. I'll drop it in V2.

Thanks,
Peng.

> 
> --
> Best regards,
> Marek Vasut
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH 3/3] imx: mx6sllevk: add usb support

2016-12-22 Thread Marek Vasut 
On 12/22/2016 09:55 AM, Peng Fan wrote:
> 
> 
>>> +#define USB_OTHERREGS_OFFSET   0x800
>>> +#define UCTRL_PWR_POL  (1 << 9)
>>> +
>>> +int board_ehci_hcd_init(int port) {
>>> +   u32 *usbnc_usb_ctrl;
>>> +
>>> +   if (port > 1)
>>> +   return -EINVAL;
>>> +
>>> +   usbnc_usb_ctrl = (u32 *)(USB_BASE_ADDR +
>> USB_OTHERREGS_OFFSET
>> +
>>> +port * 4);
>>> +
>>> +   /* Set Power polarity */
>>> +   setbits_le32(usbnc_usb_ctrl, UCTRL_PWR_POL);
>>> +   return 0;
>>> +}
>>
>> Is this function similar to what usb_oc_config() does ?
>
> No, this bit is not for overcurrent. According to RM, this is
> OTG Power Polarity This bit should be set according to power
> switch's enable
 polarity.
> 1 Power switch has an active-high enable input
> 0 Power switch has an active-low enable input
>
> This is board specific.

 Great, except it should also be part of the driver , the same way
 as polarity is configured, yes ?
>>>
>>> I just found that there is code for mx7,
>>> http://lists.denx.de/pipermail/u-boot/2016-July/260321.html
>>>
>>> Then do you agree I add such piece code in usb_power_config for mx6?
>>> "
>>> #ifdef CONFIG_MXC_USB_OTG_HACTIVE
>>> setbits_le32(ctrl, UCTRL_PWR_POL); #else
>>> clrbits_le32(ctrl, UCTRL_PWR_POL); #endif "
>>
>> Didn't you add DT support in a patch sent like ... yesterday ? :)
>> Just use DT property instead of ifdef.
>
> There is no property for otg power polatiry in upstream Linux. I
> would not like to introduce one in U-Boot. We can drop the ifdef
> when there is an
 property in upstream. What do you think?

 So uh, how can the USB work in mainline Linux on that board ?
>>>
>>> Confirmed with IC, if using gpio to control vbus power, no need to configure
>> this bit.
>>> Since in dts, we are using gpio regulator to control vbus, this piece
>>> code in patch 3/3 can be discarded. I'll drop it in V2.
>>
>> Hum, OK. Let's revisit this when we need this then ?
> 
> Take i.MX6SLL as an example,
> The pin key_col4 can be configured with the two functions.
> [1] MX6_PAD_KEY_COL4__USB_OTG1_PWR 
> [2] MX6_PAD_KEY_COL4__GPIO4_IO00
> 
> In fsl i.mx uboot code, the common way is to use [1] and configure the 
> polarity to enable the vbus power.
> But in kernel, it is configured as [2] and use gpio regulator to enable the 
> vbus power.
> 
> This is board specific. There is no upstream property for this bit. When 
> there is an upstream property for this,
> We could add it in probe function for those that use [1] in dts pinmux.
> 
> Now I am using the same dts from kernel which this pinmux configured as [2], 
> so this piece code is not needed for now.

Well, can you submit a proposal with the new prop for devicetree@ ?
It can then be improved in linux too ...

-- 
Best regards,
Marek Vasut
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH 3/3] imx: mx6sllevk: add usb support

2016-12-22 Thread Marek Vasut 
On 12/22/2016 09:38 AM, Peng Fan wrote:
> 
> 
>> -Original Message-
>> From: Marek Vasut [mailto:ma...@denx.de]
>> Sent: Thursday, December 22, 2016 3:12 PM
>> To: Peng Fan ; sba...@denx.de
>> Cc: u-boot@lists.denx.de; van.free...@gmail.com
>> Subject: Re: [PATCH 3/3] imx: mx6sllevk: add usb support
>>
>> On 12/22/2016 07:38 AM, Peng Fan wrote:
>>>
>>>
 -Original Message-
 From: Marek Vasut [mailto:ma...@denx.de]
 Sent: Thursday, December 22, 2016 2:12 PM
 To: Peng Fan ; sba...@denx.de
 Cc: u-boot@lists.denx.de; van.free...@gmail.com
 Subject: Re: [PATCH 3/3] imx: mx6sllevk: add usb support

 On 12/22/2016 07:03 AM, Peng Fan wrote:
>
> Hi Marek,
>
>> -Original Message-
>> From: Marek Vasut [mailto:ma...@denx.de]
>> Sent: Thursday, December 22, 2016 12:40 PM
>> To: Peng Fan ; sba...@denx.de
>> Cc: u-boot@lists.denx.de; van.free...@gmail.com
>> Subject: Re: [PATCH 3/3] imx: mx6sllevk: add usb support
>>
>> On 12/22/2016 02:13 AM, Peng Fan wrote:
>>>
>>>
 -Original Message-
 From: Marek Vasut [mailto:ma...@denx.de]
 Sent: Wednesday, December 21, 2016 10:10 PM
 To: Peng Fan ; sba...@denx.de
 Cc: u-boot@lists.denx.de; van.free...@gmail.com
 Subject: Re: [PATCH 3/3] imx: mx6sllevk: add usb support

 On 12/21/2016 09:14 AM, Peng Fan wrote:
> Add usb support for mx6sllevk board.
>
> Signed-off-by: Peng Fan 
> Cc: Stefano Babic 
> ---
>>> [..]
>>>
> +
> +#define USB_OTHERREGS_OFFSET   0x800
> +#define UCTRL_PWR_POL  (1 << 9)
> +
> +int board_ehci_hcd_init(int port) {
> + u32 *usbnc_usb_ctrl;
> +
> + if (port > 1)
> + return -EINVAL;
> +
> + usbnc_usb_ctrl = (u32 *)(USB_BASE_ADDR +
 USB_OTHERREGS_OFFSET
 +
> +  port * 4);
> +
> + /* Set Power polarity */
> + setbits_le32(usbnc_usb_ctrl, UCTRL_PWR_POL);
> + return 0;
> +}

 Is this function similar to what usb_oc_config() does ?
>>>
>>> No, this bit is not for overcurrent. According to RM, this is OTG
>>> Power Polarity This bit should be set according to power switch's
>>> enable
>> polarity.
>>> 1 Power switch has an active-high enable input
>>> 0 Power switch has an active-low enable input
>>>
>>> This is board specific.
>>
>> Great, except it should also be part of the driver , the same way
>> as polarity is configured, yes ?
>
> I just found that there is code for mx7,
> http://lists.denx.de/pipermail/u-boot/2016-July/260321.html
>
> Then do you agree I add such piece code in usb_power_config for mx6?
> "
> #ifdef CONFIG_MXC_USB_OTG_HACTIVE
> setbits_le32(ctrl, UCTRL_PWR_POL); #else
> clrbits_le32(ctrl, UCTRL_PWR_POL); #endif "

 Didn't you add DT support in a patch sent like ... yesterday ? :)
 Just use DT property instead of ifdef.
>>>
>>> There is no property for otg power polatiry in upstream Linux. I would
>>> not like to introduce one in U-Boot. We can drop the ifdef when there is an
>> property in upstream. What do you think?
>>
>> So uh, how can the USB work in mainline Linux on that board ?
> 
> Confirmed with IC, if using gpio to control vbus power, no need to configure 
> this bit.
> Since in dts, we are using gpio regulator to control vbus, this piece code in 
> patch 3/3
> can be discarded. I'll drop it in V2.

Hum, OK. Let's revisit this when we need this then ?

-- 
Best regards,
Marek Vasut
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH 3/3] imx: mx6sllevk: add usb support

2016-12-21 Thread Marek Vasut 
On 12/22/2016 07:38 AM, Peng Fan wrote:
> 
> 
>> -Original Message-
>> From: Marek Vasut [mailto:ma...@denx.de]
>> Sent: Thursday, December 22, 2016 2:12 PM
>> To: Peng Fan ; sba...@denx.de
>> Cc: u-boot@lists.denx.de; van.free...@gmail.com
>> Subject: Re: [PATCH 3/3] imx: mx6sllevk: add usb support
>>
>> On 12/22/2016 07:03 AM, Peng Fan wrote:
>>>
>>> Hi Marek,
>>>
 -Original Message-
 From: Marek Vasut [mailto:ma...@denx.de]
 Sent: Thursday, December 22, 2016 12:40 PM
 To: Peng Fan ; sba...@denx.de
 Cc: u-boot@lists.denx.de; van.free...@gmail.com
 Subject: Re: [PATCH 3/3] imx: mx6sllevk: add usb support

 On 12/22/2016 02:13 AM, Peng Fan wrote:
>
>
>> -Original Message-
>> From: Marek Vasut [mailto:ma...@denx.de]
>> Sent: Wednesday, December 21, 2016 10:10 PM
>> To: Peng Fan ; sba...@denx.de
>> Cc: u-boot@lists.denx.de; van.free...@gmail.com
>> Subject: Re: [PATCH 3/3] imx: mx6sllevk: add usb support
>>
>> On 12/21/2016 09:14 AM, Peng Fan wrote:
>>> Add usb support for mx6sllevk board.
>>>
>>> Signed-off-by: Peng Fan 
>>> Cc: Stefano Babic 
>>> ---
> [..]
>
>>> +
>>> +#define USB_OTHERREGS_OFFSET   0x800
>>> +#define UCTRL_PWR_POL  (1 << 9)
>>> +
>>> +int board_ehci_hcd_init(int port) {
>>> +   u32 *usbnc_usb_ctrl;
>>> +
>>> +   if (port > 1)
>>> +   return -EINVAL;
>>> +
>>> +   usbnc_usb_ctrl = (u32 *)(USB_BASE_ADDR +
>> USB_OTHERREGS_OFFSET
>> +
>>> +port * 4);
>>> +
>>> +   /* Set Power polarity */
>>> +   setbits_le32(usbnc_usb_ctrl, UCTRL_PWR_POL);
>>> +   return 0;
>>> +}
>>
>> Is this function similar to what usb_oc_config() does ?
>
> No, this bit is not for overcurrent. According to RM, this is OTG
> Power Polarity This bit should be set according to power switch's
> enable
 polarity.
> 1 Power switch has an active-high enable input
> 0 Power switch has an active-low enable input
>
> This is board specific.

 Great, except it should also be part of the driver , the same way as
 polarity is configured, yes ?
>>>
>>> I just found that there is code for mx7,
>>> http://lists.denx.de/pipermail/u-boot/2016-July/260321.html
>>>
>>> Then do you agree I add such piece code in usb_power_config for mx6?
>>> "
>>> #ifdef CONFIG_MXC_USB_OTG_HACTIVE
>>> setbits_le32(ctrl, UCTRL_PWR_POL); #else
>>> clrbits_le32(ctrl, UCTRL_PWR_POL);
>>> #endif
>>> "
>>
>> Didn't you add DT support in a patch sent like ... yesterday ? :) Just use DT
>> property instead of ifdef.
> 
> There is no property for otg power polatiry in upstream Linux. I would not 
> like to introduce one
> in U-Boot. We can drop the ifdef when there is an property in upstream. What 
> do you think?

So uh, how can the USB work in mainline Linux on that board ?

-- 
Best regards,
Marek Vasut
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH 3/3] imx: mx6sllevk: add usb support

2016-12-21 Thread Peng Fan 


> -Original Message-
> From: Marek Vasut [mailto:ma...@denx.de]
> Sent: Thursday, December 22, 2016 2:12 PM
> To: Peng Fan ; sba...@denx.de
> Cc: u-boot@lists.denx.de; van.free...@gmail.com
> Subject: Re: [PATCH 3/3] imx: mx6sllevk: add usb support
> 
> On 12/22/2016 07:03 AM, Peng Fan wrote:
> >
> > Hi Marek,
> >
> >> -Original Message-
> >> From: Marek Vasut [mailto:ma...@denx.de]
> >> Sent: Thursday, December 22, 2016 12:40 PM
> >> To: Peng Fan ; sba...@denx.de
> >> Cc: u-boot@lists.denx.de; van.free...@gmail.com
> >> Subject: Re: [PATCH 3/3] imx: mx6sllevk: add usb support
> >>
> >> On 12/22/2016 02:13 AM, Peng Fan wrote:
> >>>
> >>>
>  -Original Message-
>  From: Marek Vasut [mailto:ma...@denx.de]
>  Sent: Wednesday, December 21, 2016 10:10 PM
>  To: Peng Fan ; sba...@denx.de
>  Cc: u-boot@lists.denx.de; van.free...@gmail.com
>  Subject: Re: [PATCH 3/3] imx: mx6sllevk: add usb support
> 
>  On 12/21/2016 09:14 AM, Peng Fan wrote:
> > Add usb support for mx6sllevk board.
> >
> > Signed-off-by: Peng Fan 
> > Cc: Stefano Babic 
> > ---
> >>> [..]
> >>>
> > +
> > +#define USB_OTHERREGS_OFFSET   0x800
> > +#define UCTRL_PWR_POL  (1 << 9)
> > +
> > +int board_ehci_hcd_init(int port) {
> > +   u32 *usbnc_usb_ctrl;
> > +
> > +   if (port > 1)
> > +   return -EINVAL;
> > +
> > +   usbnc_usb_ctrl = (u32 *)(USB_BASE_ADDR +
> USB_OTHERREGS_OFFSET
>  +
> > +port * 4);
> > +
> > +   /* Set Power polarity */
> > +   setbits_le32(usbnc_usb_ctrl, UCTRL_PWR_POL);
> > +   return 0;
> > +}
> 
>  Is this function similar to what usb_oc_config() does ?
> >>>
> >>> No, this bit is not for overcurrent. According to RM, this is OTG
> >>> Power Polarity This bit should be set according to power switch's
> >>> enable
> >> polarity.
> >>> 1 Power switch has an active-high enable input
> >>> 0 Power switch has an active-low enable input
> >>>
> >>> This is board specific.
> >>
> >> Great, except it should also be part of the driver , the same way as
> >> polarity is configured, yes ?
> >
> > I just found that there is code for mx7,
> > http://lists.denx.de/pipermail/u-boot/2016-July/260321.html
> >
> > Then do you agree I add such piece code in usb_power_config for mx6?
> > "
> > #ifdef CONFIG_MXC_USB_OTG_HACTIVE
> > setbits_le32(ctrl, UCTRL_PWR_POL); #else
> > clrbits_le32(ctrl, UCTRL_PWR_POL);
> > #endif
> > "
> 
> Didn't you add DT support in a patch sent like ... yesterday ? :) Just use DT
> property instead of ifdef.

There is no property for otg power polatiry in upstream Linux. I would not like 
to introduce one
in U-Boot. We can drop the ifdef when there is an property in upstream. What do 
you think?

Regards,
Peng.

> 
> --
> Best regards,
> Marek Vasut
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH 3/3] imx: mx6sllevk: add usb support

2016-12-21 Thread Marek Vasut 
On 12/22/2016 07:03 AM, Peng Fan wrote:
> 
> Hi Marek,
> 
>> -Original Message-
>> From: Marek Vasut [mailto:ma...@denx.de]
>> Sent: Thursday, December 22, 2016 12:40 PM
>> To: Peng Fan ; sba...@denx.de
>> Cc: u-boot@lists.denx.de; van.free...@gmail.com
>> Subject: Re: [PATCH 3/3] imx: mx6sllevk: add usb support
>>
>> On 12/22/2016 02:13 AM, Peng Fan wrote:
>>>
>>>
 -Original Message-
 From: Marek Vasut [mailto:ma...@denx.de]
 Sent: Wednesday, December 21, 2016 10:10 PM
 To: Peng Fan ; sba...@denx.de
 Cc: u-boot@lists.denx.de; van.free...@gmail.com
 Subject: Re: [PATCH 3/3] imx: mx6sllevk: add usb support

 On 12/21/2016 09:14 AM, Peng Fan wrote:
> Add usb support for mx6sllevk board.
>
> Signed-off-by: Peng Fan 
> Cc: Stefano Babic 
> ---
>>> [..]
>>>
> +
> +#define USB_OTHERREGS_OFFSET   0x800
> +#define UCTRL_PWR_POL  (1 << 9)
> +
> +int board_ehci_hcd_init(int port)
> +{
> + u32 *usbnc_usb_ctrl;
> +
> + if (port > 1)
> + return -EINVAL;
> +
> + usbnc_usb_ctrl = (u32 *)(USB_BASE_ADDR + USB_OTHERREGS_OFFSET
 +
> +  port * 4);
> +
> + /* Set Power polarity */
> + setbits_le32(usbnc_usb_ctrl, UCTRL_PWR_POL);
> + return 0;
> +}

 Is this function similar to what usb_oc_config() does ?
>>>
>>> No, this bit is not for overcurrent. According to RM, this is OTG
>>> Power Polarity This bit should be set according to power switch's enable
>> polarity.
>>> 1 Power switch has an active-high enable input
>>> 0 Power switch has an active-low enable input
>>>
>>> This is board specific.
>>
>> Great, except it should also be part of the driver , the same way as 
>> polarity is
>> configured, yes ?
> 
> I just found that there is code for mx7, 
> http://lists.denx.de/pipermail/u-boot/2016-July/260321.html
> 
> Then do you agree I add such piece code in usb_power_config for mx6?
> "
> #ifdef CONFIG_MXC_USB_OTG_HACTIVE
> setbits_le32(ctrl, UCTRL_PWR_POL); 
> #else 
> clrbits_le32(ctrl, UCTRL_PWR_POL);
> #endif
> "

Didn't you add DT support in a patch sent like ... yesterday ? :) Just
use DT property instead of ifdef.

-- 
Best regards,
Marek Vasut
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH 3/3] imx: mx6sllevk: add usb support

2016-12-21 Thread Peng Fan 

Hi Marek,

> -Original Message-
> From: Marek Vasut [mailto:ma...@denx.de]
> Sent: Thursday, December 22, 2016 12:40 PM
> To: Peng Fan ; sba...@denx.de
> Cc: u-boot@lists.denx.de; van.free...@gmail.com
> Subject: Re: [PATCH 3/3] imx: mx6sllevk: add usb support
> 
> On 12/22/2016 02:13 AM, Peng Fan wrote:
> >
> >
> >> -Original Message-
> >> From: Marek Vasut [mailto:ma...@denx.de]
> >> Sent: Wednesday, December 21, 2016 10:10 PM
> >> To: Peng Fan ; sba...@denx.de
> >> Cc: u-boot@lists.denx.de; van.free...@gmail.com
> >> Subject: Re: [PATCH 3/3] imx: mx6sllevk: add usb support
> >>
> >> On 12/21/2016 09:14 AM, Peng Fan wrote:
> >>> Add usb support for mx6sllevk board.
> >>>
> >>> Signed-off-by: Peng Fan 
> >>> Cc: Stefano Babic 
> >>> ---
> > [..]
> >
> >>> +
> >>> +#define USB_OTHERREGS_OFFSET   0x800
> >>> +#define UCTRL_PWR_POL  (1 << 9)
> >>> +
> >>> +int board_ehci_hcd_init(int port)
> >>> +{
> >>> + u32 *usbnc_usb_ctrl;
> >>> +
> >>> + if (port > 1)
> >>> + return -EINVAL;
> >>> +
> >>> + usbnc_usb_ctrl = (u32 *)(USB_BASE_ADDR + USB_OTHERREGS_OFFSET
> >> +
> >>> +  port * 4);
> >>> +
> >>> + /* Set Power polarity */
> >>> + setbits_le32(usbnc_usb_ctrl, UCTRL_PWR_POL);
> >>> + return 0;
> >>> +}
> >>
> >> Is this function similar to what usb_oc_config() does ?
> >
> > No, this bit is not for overcurrent. According to RM, this is OTG
> > Power Polarity This bit should be set according to power switch's enable
> polarity.
> > 1 Power switch has an active-high enable input
> > 0 Power switch has an active-low enable input
> >
> > This is board specific.
> 
> Great, except it should also be part of the driver , the same way as polarity 
> is
> configured, yes ?

I just found that there is code for mx7, 
http://lists.denx.de/pipermail/u-boot/2016-July/260321.html

Then do you agree I add such piece code in usb_power_config for mx6?
"
#ifdef CONFIG_MXC_USB_OTG_HACTIVE
setbits_le32(ctrl, UCTRL_PWR_POL); 
#else 
clrbits_le32(ctrl, UCTRL_PWR_POL);
#endif
"

Thanks,
Peng
> 
> 
> --
> Best regards,
> Marek Vasut
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH 3/3] imx: mx6sllevk: add usb support

2016-12-21 Thread Marek Vasut 
On 12/22/2016 02:13 AM, Peng Fan wrote:
> 
> 
>> -Original Message-
>> From: Marek Vasut [mailto:ma...@denx.de]
>> Sent: Wednesday, December 21, 2016 10:10 PM
>> To: Peng Fan ; sba...@denx.de
>> Cc: u-boot@lists.denx.de; van.free...@gmail.com
>> Subject: Re: [PATCH 3/3] imx: mx6sllevk: add usb support
>>
>> On 12/21/2016 09:14 AM, Peng Fan wrote:
>>> Add usb support for mx6sllevk board.
>>>
>>> Signed-off-by: Peng Fan 
>>> Cc: Stefano Babic 
>>> ---
> [..]
> 
>>> +
>>> +#define USB_OTHERREGS_OFFSET   0x800
>>> +#define UCTRL_PWR_POL  (1 << 9)
>>> +
>>> +int board_ehci_hcd_init(int port)
>>> +{
>>> +   u32 *usbnc_usb_ctrl;
>>> +
>>> +   if (port > 1)
>>> +   return -EINVAL;
>>> +
>>> +   usbnc_usb_ctrl = (u32 *)(USB_BASE_ADDR + USB_OTHERREGS_OFFSET
>> +
>>> +port * 4);
>>> +
>>> +   /* Set Power polarity */
>>> +   setbits_le32(usbnc_usb_ctrl, UCTRL_PWR_POL);
>>> +   return 0;
>>> +}
>>
>> Is this function similar to what usb_oc_config() does ?
> 
> No, this bit is not for overcurrent. According to RM, this is OTG Power 
> Polarity
> This bit should be set according to power switch's enable polarity.
> 1 Power switch has an active-high enable input
> 0 Power switch has an active-low enable input
> 
> This is board specific.

Great, except it should also be part of the driver , the same way as
polarity is configured, yes ?


-- 
Best regards,
Marek Vasut
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH 3/3] imx: mx6sllevk: add usb support

2016-12-21 Thread Peng Fan 


> -Original Message-
> From: Jagan Teki [mailto:ja...@openedev.com]
> Sent: Wednesday, December 21, 2016 10:02 PM
> To: Peng Fan <peng@nxp.com>
> Cc: Stefano Babic <sba...@denx.de>; Marek Vasut <ma...@denx.de>; u-
> b...@lists.denx.de
> Subject: Re: [U-Boot] [PATCH 3/3] imx: mx6sllevk: add usb support
> 
> On Wed, Dec 21, 2016 at 9:14 AM, Peng Fan <peng@nxp.com> wrote:
> > Add usb support for mx6sllevk board.
> >
[.]

> > +#define UCTRL_PWR_POL  (1 << 9)
> > +
> > +int board_ehci_hcd_init(int port)
> 
> Can't we do this from driver itself, .probe or somewhere?

This is to handle OTG power polarity, which is board specific.
There is no dts property now, so I add this in board file just like other fsl 
boards.
When there is an dts property upstreamed in Linux side, we could use that and 
move this code into driver part.

> 
> > +{
> > +   u32 *usbnc_usb_ctrl;
> > +
> > +   if (port > 1)
> > +   return -EINVAL;
> > +
> > +   usbnc_usb_ctrl = (u32 *)(USB_BASE_ADDR + USB_OTHERREGS_OFFSET
> +
> > +port * 4);
> > +
> > +   /* Set Power polarity */
> > +   setbits_le32(usbnc_usb_ctrl, UCTRL_PWR_POL);
> > +   return 0;
> > +}
> > diff --git a/configs/mx6sllevk_defconfig b/configs/mx6sllevk_defconfig
> > index 8ae049e..267cdc6 100644
> > --- a/configs/mx6sllevk_defconfig
> > +++ b/configs/mx6sllevk_defconfig
> > @@ -10,6 +10,7 @@ CONFIG_CMD_BOOTZ=y
> >  CONFIG_CMD_MEMTEST=y
> >  CONFIG_CMD_MMC=y
> >  CONFIG_CMD_I2C=y
> > +CONFIG_CMD_USB=y
> >  CONFIG_CMD_GPIO=y
> >  CONFIG_CMD_DHCP=y
> >  CONFIG_CMD_PING=y
> > @@ -34,3 +35,7 @@ CONFIG_DM_REGULATOR=y
> > CONFIG_DM_REGULATOR_PFUZE100=y  CONFIG_DM_REGULATOR_FIXED=y
> > CONFIG_DM_REGULATOR_GPIO=y
> > +CONFIG_USB=y
> > +CONFIG_DM_USB=y
> > +CONFIG_USB_EHCI_HCD=y
> > +CONFIG_USB_STORAGE=y
> > diff --git a/configs/mx6sllevk_plugin_defconfig
> > b/configs/mx6sllevk_plugin_defconfig
> > index e6be979..63d5bbc 100644
> > --- a/configs/mx6sllevk_plugin_defconfig
> > +++ b/configs/mx6sllevk_plugin_defconfig
> > @@ -11,6 +11,7 @@ CONFIG_CMD_BOOTZ=y
> >  CONFIG_CMD_MEMTEST=y
> >  CONFIG_CMD_MMC=y
> >  CONFIG_CMD_I2C=y
> > +CONFIG_CMD_USB=y
> >  CONFIG_CMD_GPIO=y
> >  CONFIG_CMD_DHCP=y
> >  CONFIG_CMD_PING=y
> > @@ -35,3 +36,7 @@ CONFIG_DM_REGULATOR=y
> > CONFIG_DM_REGULATOR_PFUZE100=y  CONFIG_DM_REGULATOR_FIXED=y
> > CONFIG_DM_REGULATOR_GPIO=y
> > +CONFIG_USB=y
> > +CONFIG_DM_USB=y
> > +CONFIG_USB_EHCI_HCD=y
> > +CONFIG_USB_STORAGE=y
> > diff --git a/include/configs/mx6sllevk.h b/include/configs/mx6sllevk.h
> > index b9f25cf..3f665a3 100644
> > --- a/include/configs/mx6sllevk.h
> > +++ b/include/configs/mx6sllevk.h
> > @@ -149,4 +149,13 @@
> >
> >  #define CONFIG_IOMUX_LPSR
> >
> > +/* USB Configs */
> > +#ifdef CONFIG_CMD_USB
> > +#define CONFIG_USB_HOST_ETHER
> > +#define CONFIG_USB_ETHER_ASIX
> > +#define CONFIG_USB_ETHER_RTL8152
> > +#define CONFIG_MXC_USB_PORTSC  (PORT_PTS_UTMI |
> PORT_PTS_PTW)
> > +#define CONFIG_USB_MAX_CONTROLLER_COUNT2
> 
> I think this is even should managed from driver isn't it?

Thanks for pointing this out. This is not needed by DM USB, I'll remove it in 
V2.

Thanks,
Peng.

> 
> thanks!
> --
> Jagan Teki
> Free Software Engineer | www.openedev.com U-Boot, Linux | Upstream
> Maintainer Hyderabad, India.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH 3/3] imx: mx6sllevk: add usb support

2016-12-21 Thread Peng Fan 


> -Original Message-
> From: Marek Vasut [mailto:ma...@denx.de]
> Sent: Wednesday, December 21, 2016 10:10 PM
> To: Peng Fan ; sba...@denx.de
> Cc: u-boot@lists.denx.de; van.free...@gmail.com
> Subject: Re: [PATCH 3/3] imx: mx6sllevk: add usb support
> 
> On 12/21/2016 09:14 AM, Peng Fan wrote:
> > Add usb support for mx6sllevk board.
> >
> > Signed-off-by: Peng Fan 
> > Cc: Stefano Babic 
> > ---
[..]

> > +
> > +#define USB_OTHERREGS_OFFSET   0x800
> > +#define UCTRL_PWR_POL  (1 << 9)
> > +
> > +int board_ehci_hcd_init(int port)
> > +{
> > +   u32 *usbnc_usb_ctrl;
> > +
> > +   if (port > 1)
> > +   return -EINVAL;
> > +
> > +   usbnc_usb_ctrl = (u32 *)(USB_BASE_ADDR + USB_OTHERREGS_OFFSET
> +
> > +port * 4);
> > +
> > +   /* Set Power polarity */
> > +   setbits_le32(usbnc_usb_ctrl, UCTRL_PWR_POL);
> > +   return 0;
> > +}
> 
> Is this function similar to what usb_oc_config() does ?

No, this bit is not for overcurrent. According to RM, this is OTG Power Polarity
This bit should be set according to power switch's enable polarity.
1 Power switch has an active-high enable input
0 Power switch has an active-low enable input

This is board specific.

Regards,
Peng.

> 
> --
> Best regards,
> Marek Vasut
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH 3/3] imx: mx6sllevk: add usb support

2016-12-21 Thread Marek Vasut 
On 12/21/2016 09:14 AM, Peng Fan wrote:
> Add usb support for mx6sllevk board.
> 
> Signed-off-by: Peng Fan 
> Cc: Stefano Babic 
> ---
>  board/freescale/mx6sllevk/mx6sllevk.c | 18 ++
>  configs/mx6sllevk_defconfig   |  5 +
>  configs/mx6sllevk_plugin_defconfig|  5 +
>  include/configs/mx6sllevk.h   |  9 +
>  4 files changed, 37 insertions(+)
> 
> diff --git a/board/freescale/mx6sllevk/mx6sllevk.c 
> b/board/freescale/mx6sllevk/mx6sllevk.c
> index 74a27a3..e6679fd 100644
> --- a/board/freescale/mx6sllevk/mx6sllevk.c
> +++ b/board/freescale/mx6sllevk/mx6sllevk.c
> @@ -129,3 +129,21 @@ int mmc_map_to_kernel_blk(int devno)
>  {
>   return devno;
>  }
> +
> +#define USB_OTHERREGS_OFFSET   0x800
> +#define UCTRL_PWR_POL  (1 << 9)
> +
> +int board_ehci_hcd_init(int port)
> +{
> + u32 *usbnc_usb_ctrl;
> +
> + if (port > 1)
> + return -EINVAL;
> +
> + usbnc_usb_ctrl = (u32 *)(USB_BASE_ADDR + USB_OTHERREGS_OFFSET +
> +  port * 4);
> +
> + /* Set Power polarity */
> + setbits_le32(usbnc_usb_ctrl, UCTRL_PWR_POL);
> + return 0;
> +}

Is this function similar to what usb_oc_config() does ?

-- 
Best regards,
Marek Vasut
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH 3/3] imx: mx6sllevk: add usb support

2016-12-21 Thread Jagan Teki 
On Wed, Dec 21, 2016 at 9:14 AM, Peng Fan  wrote:
> Add usb support for mx6sllevk board.
>
> Signed-off-by: Peng Fan 
> Cc: Stefano Babic 
> ---
>  board/freescale/mx6sllevk/mx6sllevk.c | 18 ++
>  configs/mx6sllevk_defconfig   |  5 +
>  configs/mx6sllevk_plugin_defconfig|  5 +
>  include/configs/mx6sllevk.h   |  9 +
>  4 files changed, 37 insertions(+)
>
> diff --git a/board/freescale/mx6sllevk/mx6sllevk.c 
> b/board/freescale/mx6sllevk/mx6sllevk.c
> index 74a27a3..e6679fd 100644
> --- a/board/freescale/mx6sllevk/mx6sllevk.c
> +++ b/board/freescale/mx6sllevk/mx6sllevk.c
> @@ -129,3 +129,21 @@ int mmc_map_to_kernel_blk(int devno)
>  {
> return devno;
>  }
> +
> +#define USB_OTHERREGS_OFFSET   0x800
> +#define UCTRL_PWR_POL  (1 << 9)
> +
> +int board_ehci_hcd_init(int port)

Can't we do this from driver itself, .probe or somewhere?

> +{
> +   u32 *usbnc_usb_ctrl;
> +
> +   if (port > 1)
> +   return -EINVAL;
> +
> +   usbnc_usb_ctrl = (u32 *)(USB_BASE_ADDR + USB_OTHERREGS_OFFSET +
> +port * 4);
> +
> +   /* Set Power polarity */
> +   setbits_le32(usbnc_usb_ctrl, UCTRL_PWR_POL);
> +   return 0;
> +}
> diff --git a/configs/mx6sllevk_defconfig b/configs/mx6sllevk_defconfig
> index 8ae049e..267cdc6 100644
> --- a/configs/mx6sllevk_defconfig
> +++ b/configs/mx6sllevk_defconfig
> @@ -10,6 +10,7 @@ CONFIG_CMD_BOOTZ=y
>  CONFIG_CMD_MEMTEST=y
>  CONFIG_CMD_MMC=y
>  CONFIG_CMD_I2C=y
> +CONFIG_CMD_USB=y
>  CONFIG_CMD_GPIO=y
>  CONFIG_CMD_DHCP=y
>  CONFIG_CMD_PING=y
> @@ -34,3 +35,7 @@ CONFIG_DM_REGULATOR=y
>  CONFIG_DM_REGULATOR_PFUZE100=y
>  CONFIG_DM_REGULATOR_FIXED=y
>  CONFIG_DM_REGULATOR_GPIO=y
> +CONFIG_USB=y
> +CONFIG_DM_USB=y
> +CONFIG_USB_EHCI_HCD=y
> +CONFIG_USB_STORAGE=y
> diff --git a/configs/mx6sllevk_plugin_defconfig 
> b/configs/mx6sllevk_plugin_defconfig
> index e6be979..63d5bbc 100644
> --- a/configs/mx6sllevk_plugin_defconfig
> +++ b/configs/mx6sllevk_plugin_defconfig
> @@ -11,6 +11,7 @@ CONFIG_CMD_BOOTZ=y
>  CONFIG_CMD_MEMTEST=y
>  CONFIG_CMD_MMC=y
>  CONFIG_CMD_I2C=y
> +CONFIG_CMD_USB=y
>  CONFIG_CMD_GPIO=y
>  CONFIG_CMD_DHCP=y
>  CONFIG_CMD_PING=y
> @@ -35,3 +36,7 @@ CONFIG_DM_REGULATOR=y
>  CONFIG_DM_REGULATOR_PFUZE100=y
>  CONFIG_DM_REGULATOR_FIXED=y
>  CONFIG_DM_REGULATOR_GPIO=y
> +CONFIG_USB=y
> +CONFIG_DM_USB=y
> +CONFIG_USB_EHCI_HCD=y
> +CONFIG_USB_STORAGE=y
> diff --git a/include/configs/mx6sllevk.h b/include/configs/mx6sllevk.h
> index b9f25cf..3f665a3 100644
> --- a/include/configs/mx6sllevk.h
> +++ b/include/configs/mx6sllevk.h
> @@ -149,4 +149,13 @@
>
>  #define CONFIG_IOMUX_LPSR
>
> +/* USB Configs */
> +#ifdef CONFIG_CMD_USB
> +#define CONFIG_USB_HOST_ETHER
> +#define CONFIG_USB_ETHER_ASIX
> +#define CONFIG_USB_ETHER_RTL8152
> +#define CONFIG_MXC_USB_PORTSC  (PORT_PTS_UTMI | PORT_PTS_PTW)
> +#define CONFIG_USB_MAX_CONTROLLER_COUNT2

I think this is even should managed from driver isn't it?

thanks!
-- 
Jagan Teki
Free Software Engineer | www.openedev.com
U-Boot, Linux | Upstream Maintainer
Hyderabad, India.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot