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

[Marek Vasut]

On 11/23/19 12:31 AM, Vladimir Olovyannikov wrote:
[...]

> +#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) << 0)

Are the parenthesis correct here ?
Might make sense to rewrite it as ((AWCACHE << 4) | AWPROT)

> +#define USBAXIWR_SA_MASK	(0xff)

Here the parenthesis are not needed.

> +#define USBAXIWR_UA_VAL		((USBAXI_AWCACHE << 4 | USBAXI_AWPROT) << 16)
> +#define USBAXIWR_UA_MASK	((0xff) << 16)
> +#define USBAXIRD_SA_VAL		((USBAXI_ARCACHE << 4 | USBAXI_ARPROT) << 0)
> +#define USBAXIRD_SA_MASK	(0xff)
> +#define USBAXIRD_UA_VAL		((USBAXI_ARCACHE << 4 | USBAXI_ARPROT) << 16)
> +#define USBAXIRD_UA_MASK	((0xff) << 16)
> +
> +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);
> +	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;
> +	}
> +
> +	len = HC_LENGTH(xhci_readl(&(hcd)->cr_capbase));
> +	hcor = (struct xhci_hcor *)((uintptr_t)hcd + len);

Please clean up the extraneous parenthesis ^

> +	/* Save the default values of AXI read and write attributes */
> +	plat->awcache = readl((uintptr_t)hcd + DRD2U3H_XHC_REGS_AXIWRA);
> +	plat->arcache = readl((uintptr_t)hcd + DRD2U3H_XHC_REGS_AXIRDA);
> +
> +	/* Enable AXI read and write attributes. */
> +	clrsetbits_le32(((uintptr_t)hcd + DRD2U3H_XHC_REGS_AXIWRA),
> +			(USBAXIWR_UA_MASK | USBAXIWR_SA_MASK),
> +			(USBAXIWR_UA_VAL | USBAXIWR_SA_VAL));
> +	clrsetbits_le32(((uintptr_t)hcd + DRD2U3H_XHC_REGS_AXIRDA),
> +			(USBAXIRD_UA_MASK | USBAXIRD_SA_MASK),
> +			(USBAXIRD_UA_VAL | USBAXIRD_SA_VAL));
> +
> +	ret = xhci_register(dev, hcd, hcor);
> +	if (ret)
> +		dev_err(dev, "Failed to register xHCI\n");
> +
> +	return ret;
> +}
> +
> +static int xhci_brcm_deregister(struct udevice *dev)
> +{
> +	struct xhci_hccr *hcd;
> +	struct brcm_xhci_platdata *plat = dev_get_platdata(dev);
> +
> +	hcd = (struct xhci_hccr *)((uintptr_t)dev_read_addr(dev));
> +
> +	/* Restore the default values for AXI read and write attributes */
> +	writel(plat->awcache, ((uintptr_t)hcd + DRD2U3H_XHC_REGS_AXIWRA));
> +	writel(plat->arcache, ((uintptr_t)hcd + DRD2U3H_XHC_REGS_AXIRDA));

Here too.

Looks good otherwise, thanks.