[U-Boot] [PATCH v3 2/2] drivers: usb: host: Add BRCM xHCI driver

[Marek Vasut]

On 11/26/19 7:41 PM, Vladimir Olovyannikov wrote:
Hi,

[...]

> +#define USBAXI_AWCACHE		0xF
> +#define USBAXI_ARCACHE		0xF
> +#define USBAXI_AWPROT		0x8
> +#define USBAXI_ARPROT		0x8
> +#define USBAXIWR_SA_VAL		((USBAXI_AWCACHE << 4) | USBAXI_AWPROT)
> +#define USBAXIWR_SA_MASK	0xFF
> +#define USBAXIWR_UA_VAL		(((USBAXI_AWCACHE << 4) | USBAXI_AWPROT) << 16)

Maybe you can do "#define USBAXIWR_UA_VAL (USBAXIWR_SA_VAL << 16)" here?

> +#define USBAXIWR_UA_MASK	(0xFF << 16)
> +#define USBAXIRD_SA_VAL		((USBAXI_ARCACHE << 4) | USBAXI_ARPROT)
> +#define USBAXIRD_SA_MASK	0xFF
> +#define USBAXIRD_UA_VAL		(((USBAXI_ARCACHE << 4) | USBAXI_ARPROT) << 16)

And here too ?

> +#define USBAXIRD_UA_MASK	(0xFF << 16)

The masks are the same for RD and WR, maybe it makes sense to unify them
into one set of masks ?

> +struct brcm_xhci_platdata {
> +	unsigned int arcache;
> +	unsigned int awcache;
> +};
> +
> +static int xhci_brcm_probe(struct udevice *dev)
> +{
> +	struct xhci_hccr *hcd;
> +	struct xhci_hcor *hcor;
> +	struct brcm_xhci_platdata *plat = dev_get_platdata(dev);
> +	uintptr_t hc_base;
> +	int len, ret = 0;
> +
> +	if (!plat) {
> +		dev_err(dev, "Can't get xHCI Plat data\n");
> +		return -ENOMEM;
> +	}
> +
> +	hcd = dev_read_addr_ptr(dev);
> +	if (!hcd) {
> +		dev_err(dev, "Can't get the xHCI register base address\n");
> +		return -ENXIO;
> +	}
> +
> +	hc_base = (uintptr_t)hcd;
> +	len = HC_LENGTH(xhci_readl(&hcd->cr_capbase));
> +	hcor = (struct xhci_hcor *)(hc_base + len);
> +
> +	/* Save the default values of AXI read and write attributes */
> +	plat->awcache = readl(hc_base + DRD2U3H_XHC_REGS_AXIWRA);
> +	plat->arcache = readl(hc_base + DRD2U3H_XHC_REGS_AXIRDA);
> +
> +	/* Enable AXI read and write attributes. */
> +	clrsetbits_le32((hc_base + DRD2U3H_XHC_REGS_AXIWRA),                        ^
            This one is unnecessary too. Please fix them globally.


> +			(USBAXIWR_UA_MASK | USBAXIWR_SA_MASK),

Here

> +			(USBAXIWR_UA_VAL | USBAXIWR_SA_VAL));

Here

> +	clrsetbits_le32((hc_base + DRD2U3H_XHC_REGS_AXIRDA),

Here

> +			(USBAXIRD_UA_MASK | USBAXIRD_SA_MASK),

Here

> +			(USBAXIRD_UA_VAL | USBAXIRD_SA_VAL));

And here

[...]