Re: [PATCH v3 3/4] usb: dwc3: Add device property sgl-trb-cache-size-quirk

[Tejas Joglekar <Tejas.Joglekar@synopsys.com>]

Hi Felipe,
On 5/27/2020 4:11 PM, Tejas Joglekar wrote:
> This commit adds the sgl-trb-cache-size-quirk property to enable
> quirk for the XHCI driver with Synopsys xHC. This property is
> enabled as initial property for the dwc3-haps driver.
> 
> Signed-off-by: Tejas Joglekar <joglekar@synopsys.com>
> ---
>  drivers/usb/dwc3/core.c      | 2 ++
>  drivers/usb/dwc3/core.h      | 2 ++
>  drivers/usb/dwc3/dwc3-haps.c | 1 +
>  drivers/usb/dwc3/host.c      | 6 +++++-
>  4 files changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index 25c686a752b0..bc295477e1bc 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -1299,6 +1299,8 @@ static void dwc3_get_properties(struct dwc3 *dwc)
>  				"snps,usb3_lpm_capable");
>  	dwc->usb2_lpm_disable = device_property_read_bool(dev,
>  				"snps,usb2-lpm-disable");
> +	dwc->sgl_trb_cache_size_quirk = device_property_read_bool(dev,
> +				"snps,sgl-trb-cache-size-quirk");
>  	device_property_read_u8(dev, "snps,rx-thr-num-pkt-prd",
>  				&rx_thr_num_pkt_prd);
>  	device_property_read_u8(dev, "snps,rx-max-burst-prd",
> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
> index 013f42a2b5dc..0dca0dbf4309 100644
> --- a/drivers/usb/dwc3/core.h
> +++ b/drivers/usb/dwc3/core.h
> @@ -1021,6 +1021,7 @@ struct dwc3_scratchpad_array {
>   *			not needed for DWC_usb31 version 1.70a-ea06 and below
>   * @usb3_lpm_capable: set if hadrware supports Link Power Management
>   * @usb2_lpm_disable: set to disable usb2 lpm
> + * @sgl_trb_cache_size_quirk: set to enable the SG list consolidation
>   * @disable_scramble_quirk: set if we enable the disable scramble quirk
>   * @u2exit_lfps_quirk: set if we enable u2exit lfps quirk
>   * @u2ss_inp3_quirk: set if we enable P3 OK for U2/SS Inactive quirk
> @@ -1220,6 +1221,7 @@ struct dwc3 {
>  	unsigned		dis_start_transfer_quirk:1;
>  	unsigned		usb3_lpm_capable:1;
>  	unsigned		usb2_lpm_disable:1;
> +	unsigned		sgl_trb_cache_size_quirk:1;
>  
>  	unsigned		disable_scramble_quirk:1;
>  	unsigned		u2exit_lfps_quirk:1;
> diff --git a/drivers/usb/dwc3/dwc3-haps.c b/drivers/usb/dwc3/dwc3-haps.c
> index 3cecbf169452..9311cbe5f264 100644
> --- a/drivers/usb/dwc3/dwc3-haps.c
> +++ b/drivers/usb/dwc3/dwc3-haps.c
> @@ -29,6 +29,7 @@ static const struct property_entry initial_properties[] = {
>  	PROPERTY_ENTRY_BOOL("snps,usb3_lpm_capable"),
>  	PROPERTY_ENTRY_BOOL("snps,has-lpm-erratum"),
>  	PROPERTY_ENTRY_BOOL("snps,dis_enblslpm_quirk"),
> +	PROPERTY_ENTRY_BOOL("snps,sgl-trb-cache-size-quirk"),
>  	PROPERTY_ENTRY_BOOL("linux,sysdev_is_parent"),
>  	{ },
>  };
> diff --git a/drivers/usb/dwc3/host.c b/drivers/usb/dwc3/host.c
> index bef1c1ac2067..e0089c82728e 100644
> --- a/drivers/usb/dwc3/host.c
> +++ b/drivers/usb/dwc3/host.c
> @@ -44,7 +44,7 @@ static int dwc3_host_get_irq(struct dwc3 *dwc)
>  
>  int dwc3_host_init(struct dwc3 *dwc)
>  {
> -	struct property_entry	props[4];
> +	struct property_entry	props[5];
>  	struct platform_device	*xhci;
>  	int			ret, irq;
>  	struct resource		*res;
> @@ -95,6 +95,10 @@ int dwc3_host_init(struct dwc3 *dwc)
>  	if (dwc->usb2_lpm_disable)
>  		props[prop_idx++] = PROPERTY_ENTRY_BOOL("usb2-lpm-disable");
>  
> +	if (dwc->sgl_trb_cache_size_quirk)
> +		props[prop_idx++] =
> +			PROPERTY_ENTRY_BOOL("sgl-trb-cache-size-quirk");
> +
>  	/**
>  	 * WORKAROUND: dwc3 revisions <=3.00a have a limitation
>  	 * where Port Disable command doesn't work.
> 
Does this implementation looks good to you? Rob has some concerned over the DT entries,
you suggested using compatible string with this quirk addition.
Can you please brief about how you would like to have this quirk implemented?
I can send the updated patch. My patch series is pending for merge just because of the
DT and quirk issue. Can you please help?

Thanks & Regards,
 Tejas Joglekar