[U-Boot] [PATCH v2 05/11] x86: quark: Add USB PHY initialization support

[Bin Meng]

Hi Simon,

On Wed, Sep 2, 2015 at 10:05 PM, Simon Glass <sjg at chromium.org> wrote:
> Hi Bin,
>
> On 2 September 2015 at 03:17, Bin Meng <bmeng.cn at gmail.com> wrote:
>> USB PHY needs to be properly initialized per Quark firmware writer
>> guide, otherwise the EHCI controller on Quark SoC won't work.
>>
>> Signed-off-by: Bin Meng <bmeng.cn at gmail.com>
>>
>> ---
>>
>> Changes in v2:
>> - New patch to add USB PHY initialization support
>>
>>  arch/x86/cpu/quark/quark.c              | 41 +++++++++++++++++++++++++++++++++
>>  arch/x86/include/asm/arch-quark/quark.h |  8 +++++++
>>  2 files changed, 49 insertions(+)
>
> Acked-by: Simon Glass <sjg at chromium.org>
>
> But please see below.
>
>>
>> diff --git a/arch/x86/cpu/quark/quark.c b/arch/x86/cpu/quark/quark.c
>> index dda3c7c..637c370 100644
>> --- a/arch/x86/cpu/quark/quark.c
>> +++ b/arch/x86/cpu/quark/quark.c
>> @@ -125,6 +125,44 @@ static void quark_pcie_early_init(void)
>>         msg_port_io_write(MSG_PORT_PCIE_AFE, PCIE_RXPICTRL0_L1, pcie_cfg);
>>  }
>>
>> +static void quark_usb_early_init(void)
>> +{
>> +       u32 usb;
>> +
>> +       /* The sequence below comes from Quark firmware writer guide */
>> +
>> +       usb = msg_port_alt_read(MSG_PORT_USB_AFE, USB2_GLOBAL_PORT);
>> +       usb &= ~(1 << 1);
>> +       usb |= ((1 << 6) | (1 << 7));
>> +       msg_port_alt_write(MSG_PORT_USB_AFE, USB2_GLOBAL_PORT, usb);
>
> How about adding new a function so you can do something like:
>
> msg_port_alt_clrsetbits(MSG_PORT_USB_AFE, USB2_GLOBAL_PORT, 1 << 1, (1
> << 6) | (1 << 7))
>
> We did this with pmic also. It seems like you have enough code doing
> this read/write dance that it would be worthwhile, perhaps as a
> follow-on patch.

Yep, I can do in a follow-on patch.

>
> Also these shifts should have enums/#defines.

Sorry I tried my best to give these magic numbers some names. These
registers/bits are undocumented in the Quark datasheet. They are just
mentioned in the firmware writer guide, but still magic numbers, no
detailed description at all. I referred to Intel's UEFI BIOS for
Quark, still BIT0, BIT1 macros, like the one we dealt with on the MRC
codes. So I have to use the (1 << n) format here (not the BITn macro)

>
>> +
>> +       usb = msg_port_alt_read(MSG_PORT_USB_AFE, USB2_COMPBG);
>> +       usb &= ~((1 << 8) | (1 << 9));
>> +       usb |= ((1 << 7) | (1 << 10));
>> +       msg_port_alt_write(MSG_PORT_USB_AFE, USB2_COMPBG, usb);
>> +
>> +       usb = msg_port_alt_read(MSG_PORT_USB_AFE, USB2_PLL2);
>> +       usb |= (1 << 29);
>> +       msg_port_alt_write(MSG_PORT_USB_AFE, USB2_PLL2, usb);
>> +
>> +       usb = msg_port_alt_read(MSG_PORT_USB_AFE, USB2_PLL1);
>> +       usb |= (1 << 1);
>> +       msg_port_alt_write(MSG_PORT_USB_AFE, USB2_PLL1, usb);
>> +
>> +       usb = msg_port_alt_read(MSG_PORT_USB_AFE, USB2_PLL1);
>> +       usb &= ~((1 << 3) | (1 << 4) | (1 << 5));
>> +       usb |= (1 << 6);
>> +       msg_port_alt_write(MSG_PORT_USB_AFE, USB2_PLL1, usb);
>> +
>> +       usb = msg_port_alt_read(MSG_PORT_USB_AFE, USB2_PLL2);
>> +       usb &= ~(1 << 29);
>> +       msg_port_alt_write(MSG_PORT_USB_AFE, USB2_PLL2, usb);
>> +
>> +       usb = msg_port_alt_read(MSG_PORT_USB_AFE, USB2_PLL2);
>> +       usb |= (1 << 24);
>> +       msg_port_alt_write(MSG_PORT_USB_AFE, USB2_PLL2, usb);
>> +}
>> +

[snip]

Regards,
Bin