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

[Doug Anderson]

Kever,

On Tue, Jul 29, 2014 at 6:35 PM, Kever Yang <kever.yang at 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 at 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?


> +       .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.


> +       .host_channels                  = 9,
> +       .phy_type                       = 1,    /* UTMI */
> +       .phy_utmi_width                 = 16,   /* 8 bits */

Either comment or value is wrong since 16 != 8 bits.


> +       .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?

-Doug