Patchwork [U-Boot,1/4] USB: ohci-at91: support sama5d3x devices

login
register
mail settings
Submitter Bo Shen
Date Feb. 28, 2013, 7 a.m.
Message ID <1362034848-6371-2-git-send-email-voice.shen@atmel.com>
Download mbox | patch
Permalink /patch/223781/
State Superseded
Delegated to: Andreas Bießmann
Headers show

Comments

Bo Shen - Feb. 28, 2013, 7 a.m.
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(-)
Andreas Bießmann - March 7, 2013, 9:12 a.m.
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
Bo Shen - March 8, 2013, 1:21 a.m.
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
>

Patch

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)