Message ID | 1362034848-6371-2-git-send-email-voice.shen@atmel.com |
---|---|
State | Superseded, archived |
Delegated to: | Andreas Bießmann |
Headers | show |
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
On 3/7/2013 17:12, Andreas Bießmann wrote: > 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. OK. I will consider this for a future patch, not in this one. Best Regards, Bo Shen > The rest looks sane to me. > > Best regards > > Andreas Bießmann >
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) /* 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) + 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)
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(-)