Dear Bo Shen,
On 28.02.13 08:00, Bo Shen wrote:
Add OHCI support for sama5d3x devices
Signed-off-by: Bo Shen voice.shen@atmel.com
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