[PATCH 3/4] usb: dwc2: add compatible data for rockchip soc

[kever.yang@rock-chips.com (Kever Yang)]

Hi Doug:

On 07/29/2014 09:21 PM, Doug Anderson wrote:
> Kever,
>
> On Tue, Jul 29, 2014 at 6:35 PM, Kever Yang <kever.yang@rock-chips.com> wrote:
>> This patch add compatible data for dwc2 controller found on
>> rk3066, rk3188 and rk3288 processors from rockchip.
>>
>> Signed-off-by: Kever Yang <kever.yang@rock-chips.com>
>> ---
>>   drivers/usb/dwc2/platform.c |   29 +++++++++++++++++++++++++++++
>>   1 file changed, 29 insertions(+)
> I'm nowhere an expert here, but...
>
>
>> diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c
>> index a10e7a3..cc5983c 100644
>> --- a/drivers/usb/dwc2/platform.c
>> +++ b/drivers/usb/dwc2/platform.c
>> @@ -75,6 +75,34 @@ static const struct dwc2_core_params params_bcm2835 = {
>>          .uframe_sched                   = 0,
>>   };
>>
>> +static const struct dwc2_core_params params_rk3066 = {
>> +       .otg_cap                        = 2,    /* no HNP/SRP capable */
> Are you sure HNP/SRP is not available?  Do things break if you leave this at 0?
1. HNP/SRP is not need right now, I didn't see it used in mobile devices.
2. If HNP/SRP is enabled, the controller will monitor the otg_vbus about 5V,
     overcurrent change will happen if vbus not detect after switch to host
     for a period of time.
     If not, the controller will monitor b_valid signal about 3V instead 
of 5V vbus,
     which match the hardware reference design from Rockchip.
>
>
>> +       .otg_ver                        = 0,    /* 1.3 */
>> +       .dma_enable                     = 1,
>> +       .dma_desc_enable                = 0,
>> +       .speed                          = 0,    /* High Speed */
>> +       .enable_dynamic_fifo            = 1,
>> +       .en_multiple_tx_fifo            = 1,
>> +       .host_rx_fifo_size              = 520,  /* 520 DWORDs */
>> +       .host_nperio_tx_fifo_size       = 128,  /* 128 DWORDs */
>> +       .host_perio_tx_fifo_size        = 256,  /* 256 DWORDs */
>> +       .max_transfer_size              = 65536,
>> +       .max_packet_count               = 512,
> Header file says max_packet_count should be max of 511.
Yeap, you are right, I will fix this.
>
>
>> +       .host_channels                  = 9,
>> +       .phy_type                       = 1,    /* UTMI */
>> +       .phy_utmi_width                 = 16,   /* 8 bits */
> Either comment or value is wrong since 16 != 8 bits.
I will change this to auto detect.
>
>
>> +       .phy_ulpi_ddr                   = 0,    /* Single */
>> +       .phy_ulpi_ext_vbus              = 0,
>> +       .i2c_enable                     = 0,
>> +       .ulpi_fs_ls                     = 0,
>> +       .host_support_fs_ls_low_power   = 0,
>> +       .host_ls_low_power_phy_clk      = 0,    /* 48 MHz */
>> +       .ts_dline                       = 0,
>> +       .reload_ctl                     = 1,
>> +       .ahbcfg                         = 0x17, /* dma enable &  INCR16 */
>> +       .uframe_sched                   = 1,
>> +};
> Many of these values could just be -1 to autodetect / use default.
> Should we consider doing that?
Most of the parameter can be auto-detect right except some have more than
one choice, so I will leave the parameters to driver if it can do it right.
>
> -Doug
>
>
>