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

[Peng Fan]

Hi Marek,
> -----Original Message-----
> From: Marek Vasut [mailto:marex at denx.de]
> Sent: Thursday, December 22, 2016 4:58 PM
> To: Peng Fan <peng.fan at nxp.com>; sbabic at denx.de
> Cc: u-boot at lists.denx.de; van.freenix at 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