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

[Vladimir Olovyannikov]

HI Marek,

> 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.
Thanks for review Marek,
Issues will be resolved with the next patch set.