Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
From:   Peter Chen <peter.chen@nxp.com>
To:     Ran Wang <ran.wang_1@nxp.com>
CC:     Rob Herring <robh+dt@kernel.org>, Felipe Balbi <balbi@kernel.org>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Mark Rutland <mark.rutland@arm.com>, Jun Li <jun.li@nxp.com>,
        Leo Li <leoyang.li@nxp.com>,
        "linux-usb@vger.kernel.org" <linux-usb@vger.kernel.org>,
        "devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v3 2/2] usb: dwc3: Add cache type configuration support
Thread-Topic: [PATCH v3 2/2] usb: dwc3: Add cache type configuration support
Thread-Index: AQHVoBVSAvvkYeBt2E6+VjH8nSE3k6eVYwIA
Date:   Thu, 21 Nov 2019 09:54:01 +0000
Message-ID: <20191121095356.GB7503@b29397-desktop>
References: <20191121024206.32933-1-ran.wang_1@nxp.com>
 <20191121024206.32933-2-ran.wang_1@nxp.com>
In-Reply-To: <20191121024206.32933-2-ran.wang_1@nxp.com>
Accept-Language: en-US
Content-Language: en-US
received-spf: None (protection.outlook.com: nxp.com does not designate
 permitted sender hosts)
Content-Type: text/plain; charset="us-ascii"
Content-ID: <BB4A3445FF491245890EE9FC582979D2@eurprd04.prod.outlook.com>
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: 0e44b0f3-a9b3-4b85-016e-08d76e68bce4
X-MS-Exchange-CrossTenant-originalarrivaltime: 21 Nov 2019 09:54:01.9998
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 686ea1d3-bc2b-4c6f-a92c-d99c5c301635
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: 4+ol4dN82ZcGgZ7dd7o63qpuXHUWwm5aRNPEk4A0iIAlsYyaSICQA8Z9eNTb36zIUdzAsPm1QMacePvrOOz47w==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: VI1PR04MB4061
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 19-11-21 10:42:06, Ran Wang wrote:
> This feature is telling how to configure cache type on 4 different
> transfer types: Data Read, Desc Read, Data Write and Desc write. For each
> treasfer type, controller has a 4-bit register field to enable different

%s/treasfer/transfer

> cache type. Quoted from DWC3 data book Table 6-5 Cache Type Bit Assignmen=
ts:
> ----------------------------------------------------------------
> MBUS_TYPE| bit[3]       |bit[2]       |bit[1]     |bit[0]
> ----------------------------------------------------------------
> AHB      |Cacheable     |Bufferable   |Privilegge |Data
> AXI3     |Write Allocate|Read Allocate|Cacheable  |Bufferable
> AXI4     |Allocate Other|Allocate     |Modifiable |Bufferable
> AXI4     |Other Allocate|Allocate     |Modifiable |Bufferable
> Native   |Same as AXI   |Same as AXI  |Same as AXI|Same as AXI
> ----------------------------------------------------------------
> Note: The AHB, AXI3, AXI4, and PCIe busses use different names for certai=
n
> signals, which have the same meaning:
>   Bufferable =3D Posted
>   Cacheable =3D Modifiable =3D Snoop (negation of No Snoop)
>=20
> In most cases, driver support is not required unless the default values o=
f
> registers are not correct *and* DWC3 node has enabled dma-coherent. So fa=
r we
> have observed USB device detect failure on some Layerscape platforms if t=
his
> programming was not applied.
>=20
> Related struct:
> struct dwc3_cache_type {
> 	u8 transfer_type_datard;
> 	u8 transfer_type_descrd;
> 	u8 transfer_type_datawr;
> 	u8 transfer_type_descwr;
> };
>=20
> Signed-off-by: Ran Wang <ran.wang_1@nxp.com>
> ---
> Change in v3:
> 	- Replace cache type sub-node parsing with chip-specifc data parsing.

If it is SoC implementation specific, you may move the implementation to
dwc3-of-simmple.c, and change your dts accordingly.

Feplie, what do you think?

Peter

>=20
> Change in v2:
> 	- Change most program logic to meet new DTS property define.
> 	- Rename related register address macros.
> 	- Rename function  dwc3_enable_snooping() to dwc3_set_cache_type().
>=20
>  drivers/usb/dwc3/core.c | 67 +++++++++++++++++++++++++++++++++++++++++++=
+-----
>  drivers/usb/dwc3/core.h | 15 +++++++++++
>  2 files changed, 76 insertions(+), 6 deletions(-)
>=20
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index 97d6ae3..0baa972 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -894,6 +894,53 @@ static void dwc3_set_incr_burst_type(struct dwc3 *dw=
c)
>  	dwc3_writel(dwc->regs, DWC3_GSBUSCFG0, cfg);
>  }
> =20
> +#ifdef CONFIG_OF
> +struct dwc3_cache_type {
> +	u8 transfer_type_datard;
> +	u8 transfer_type_descrd;
> +	u8 transfer_type_datawr;
> +	u8 transfer_type_descwr;
> +};
> +
> +static const struct dwc3_cache_type ls1088a_dwc3_cache_type =3D {
> +	.transfer_type_datard =3D 2,
> +	.transfer_type_descrd =3D 2,
> +	.transfer_type_datawr =3D 2,
> +	.transfer_type_descwr =3D 2,
> +};
> +
> +/**
> + * dwc3_set_cache_type - Configure cache type registers
> + * @dwc: Pointer to our controller context structure
> + */
> +static void dwc3_set_cache_type(struct dwc3 *dwc)
> +{
> +	u32 tmp, reg;
> +	const struct dwc3_cache_type *cache_type =3D
> +		device_get_match_data(dwc->dev);
> +
> +	if (cache_type) {
> +		reg =3D dwc3_readl(dwc->regs,  DWC3_GSBUSCFG0);
> +		tmp =3D reg;
> +
> +		reg &=3D ~DWC3_GSBUSCFG0_DATARD(~0);
> +		reg |=3D DWC3_GSBUSCFG0_DATARD(cache_type->transfer_type_datard);
> +
> +		reg &=3D ~DWC3_GSBUSCFG0_DESCRD(~0);
> +		reg |=3D DWC3_GSBUSCFG0_DESCRD(cache_type->transfer_type_descrd);
> +
> +		reg &=3D ~DWC3_GSBUSCFG0_DATAWR(~0);
> +		reg |=3D DWC3_GSBUSCFG0_DATAWR(cache_type->transfer_type_datawr);
> +
> +		reg &=3D ~DWC3_GSBUSCFG0_DESCWR(~0);
> +		reg |=3D DWC3_GSBUSCFG0_DESCWR(cache_type->transfer_type_descwr);
> +
> +		if (tmp !=3D reg)
> +			dwc3_writel(dwc->regs, DWC3_GSBUSCFG0, reg);
> +	}
> +}
> +#endif
> +
>  /**
>   * dwc3_core_init - Low-level initialization of DWC3 Core
>   * @dwc: Pointer to our controller context structure
> @@ -952,6 +999,10 @@ static int dwc3_core_init(struct dwc3 *dwc)
> =20
>  	dwc3_set_incr_burst_type(dwc);
> =20
> +#ifdef CONFIG_OF
> +	dwc3_set_cache_type(dwc);
> +#endif
> +
>  	usb_phy_set_suspend(dwc->usb2_phy, 0);
>  	usb_phy_set_suspend(dwc->usb3_phy, 0);
>  	ret =3D phy_power_on(dwc->usb2_generic_phy);
> @@ -1837,12 +1888,16 @@ static const struct dev_pm_ops dwc3_dev_pm_ops =
=3D {
> =20
>  #ifdef CONFIG_OF
>  static const struct of_device_id of_dwc3_match[] =3D {
> -	{
> -		.compatible =3D "snps,dwc3"
> -	},
> -	{
> -		.compatible =3D "synopsys,dwc3"
> -	},
> +	{ .compatible =3D "fsl,ls1012a-dwc3", .data =3D &ls1088a_dwc3_cache_typ=
e, },
> +	{ .compatible =3D "fsl,ls1021a-dwc3", .data =3D &ls1088a_dwc3_cache_typ=
e, },
> +	{ .compatible =3D "fsl,ls1028a-dwc3", .data =3D &ls1088a_dwc3_cache_typ=
e, },
> +	{ .compatible =3D "fsl,ls1043a-dwc3", .data =3D &ls1088a_dwc3_cache_typ=
e, },
> +	{ .compatible =3D "fsl,ls1046a-dwc3", .data =3D &ls1088a_dwc3_cache_typ=
e, },
> +	{ .compatible =3D "fsl,ls1088a-dwc3", .data =3D &ls1088a_dwc3_cache_typ=
e, },
> +	{ .compatible =3D "fsl,ls2088a-dwc3", .data =3D &ls1088a_dwc3_cache_typ=
e, },
> +	{ .compatible =3D "fsl,lx2160a-dwc3", .data =3D &ls1088a_dwc3_cache_typ=
e, },
> +	{ .compatible =3D "snps,dwc3" },
> +	{ .compatible =3D "synopsys,dwc3"	},
>  	{ },
>  };
>  MODULE_DEVICE_TABLE(of, of_dwc3_match);
> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
> index 1c8b3493..ac51dfe 100644
> --- a/drivers/usb/dwc3/core.h
> +++ b/drivers/usb/dwc3/core.h
> @@ -165,6 +165,21 @@
>  /* Bit fields */
> =20
>  /* Global SoC Bus Configuration INCRx Register 0 */
> +#ifdef CONFIG_OF
> +#define DWC3_GSBUSCFG0_DATARD_SHIFT	28
> +#define DWC3_GSBUSCFG0_DATARD(n)	(((n) & 0xf)		\
> +			<< DWC3_GSBUSCFG0_DATARD_SHIFT)
> +#define DWC3_GSBUSCFG0_DESCRD_SHIFT	24
> +#define DWC3_GSBUSCFG0_DESCRD(n)	(((n) & 0xf)		\
> +			<< DWC3_GSBUSCFG0_DESCRD_SHIFT)
> +#define DWC3_GSBUSCFG0_DATAWR_SHIFT	20
> +#define DWC3_GSBUSCFG0_DATAWR(n)	(((n) & 0xf)		\
> +			<< DWC3_GSBUSCFG0_DATAWR_SHIFT)
> +#define DWC3_GSBUSCFG0_DESCWR_SHIFT	16
> +#define DWC3_GSBUSCFG0_DESCWR(n)	(((n) & 0xf)		\
> +			<< DWC3_GSBUSCFG0_DESCWR_SHIFT)
> +#endif
> +
>  #define DWC3_GSBUSCFG0_INCR256BRSTENA	(1 << 7) /* INCR256 burst */
>  #define DWC3_GSBUSCFG0_INCR128BRSTENA	(1 << 6) /* INCR128 burst */
>  #define DWC3_GSBUSCFG0_INCR64BRSTENA	(1 << 5) /* INCR64 burst */
> --=20
> 2.7.4
>=20

--=20

Thanks,
Peter Chen=