Dear Bo Shen, On 28.02.13 08:00, Bo Shen wrote: > Add OHCI support for sama5d3x devices > > Signed-off-by: Bo Shen <[email protected]> > --- > drivers/usb/host/ohci-at91.c | 14 ++++++++++++-- > 1 file changed, 12 insertions(+), 2 deletions(-) > > diff --git a/drivers/usb/host/ohci-at91.c b/drivers/usb/host/ohci-at91.c > index efd711d..35a282e 100644 > --- a/drivers/usb/host/ohci-at91.c > +++ b/drivers/usb/host/ohci-at91.c > @@ -42,7 +42,7 @@ int usb_cpu_init(void) > while ((readl(&pmc->sr) & AT91_PMC_LOCKB) != AT91_PMC_LOCKB) > ; > #elif defined(CONFIG_AT91SAM9G45) || defined(CONFIG_AT91SAM9M10G45) || \ > - defined(CONFIG_AT91SAM9X5) > + defined(CONFIG_AT91SAM9X5) || defined(CONFIG_SAMA5D3)
I think these ifdeffery increases alarmingly here and there. Can we find a better solution like #if defined(ATMEL_OHCI_NEEDS_UPLL) or whatever we can call it. I mean to classify this and enable it in the respective SoC headers. Maybe here we can distinguish upon the IP version? > /* Enable UPLL */ > writel(readl(&pmc->uckr) | AT91_PMC_UPLLEN | AT91_PMC_BIASEN, > &pmc->uckr); > @@ -54,7 +54,12 @@ int usb_cpu_init(void) > #endif > > /* Enable USB host clock. */ > +#if defined(CONFIG_SAMA5D3) I think this ifdef is Ok instead. > + writel(1 << (ATMEL_ID_UHP - 32), &pmc->pcer1); > +#else > writel(1 << ATMEL_ID_UHP, &pmc->pcer); > +#endif > + > #ifdef CONFIG_AT91SAM9261 > writel(ATMEL_PMC_UHP | AT91_PMC_HCK0, &pmc->scer); > #else > @@ -69,7 +74,12 @@ int usb_cpu_stop(void) > at91_pmc_t *pmc = (at91_pmc_t *)ATMEL_BASE_PMC; > > /* Disable USB host clock. */ > +#if defined(CONFIG_SAMA5D3) > + writel(1 << (ATMEL_ID_UHP - 32), &pmc->pcdr1); > +#else > writel(1 << ATMEL_ID_UHP, &pmc->pcdr); > +#endif > + > #ifdef CONFIG_AT91SAM9261 > writel(ATMEL_PMC_UHP | AT91_PMC_HCK0, &pmc->scdr); > #else > @@ -83,7 +93,7 @@ int usb_cpu_stop(void) > while ((readl(&pmc->sr) & AT91_PMC_LOCKB) != 0) > ; > #elif defined(CONFIG_AT91SAM9G45) || defined(CONFIG_AT91SAM9M10G45) || \ > - defined(CONFIG_AT91SAM9X5) > + defined(CONFIG_AT91SAM9X5) || defined(CONFIG_SAMA5D3) > /* Disable UPLL */ > writel(readl(&pmc->uckr) & (~AT91_PMC_UPLLEN), &pmc->uckr); > while ((readl(&pmc->sr) & AT91_PMC_LOCKU) == AT91_PMC_LOCKU) > I think the ifdef comment above is no show stopper for this patch but should be considered at least for a future patch. The rest looks sane to me. Best regards Andreas Bießmann _______________________________________________ U-Boot mailing list [email protected] http://lists.denx.de/mailman/listinfo/u-boot
