From: Sergei Shtylyov <sshtylyov-Igf4POYTYCDQT0dZR+AlfA@public.gmane.org>
To: Constantine Shulyupin <const-XSqIthwXjGBDPfheJLI6IQ@public.gmane.org>
Cc: davinci-linux-open-source-VycZQUHpC/PFrsHnngEfi1aTQe2KTcn/@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-embedded-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Subject: Re: [PATCH] Enable USB on TI DM365
Date: Mon, 27 Jun 2011 13:12:05 +0400 [thread overview]
Message-ID: <4E084965.4020409@mvista.com> (raw)
In-Reply-To: <BANLkTims=UsUMkDQ8uB+J6YTVRSBD__otw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
Hello.
On 24-06-2011 22:51, Constantine Shulyupin wrote:
> Enable USB on TI DM365
On DM365 EVM board, you should have said.
Do not duplicate the summary in the description. Also, more details here
wouldn't hurt...
> Signed-off-by: Constantine Shulyupin <const-GkuDRZ1haSFDPfheJLI6IQ@public.gmane.org>
[...]
> diff --git a/arch/arm/mach-davinci/Makefile b/arch/arm/mach-davinci/Makefile
> index 0b87a1c..52961a8 100644
> --- a/arch/arm/mach-davinci/Makefile
> +++ b/arch/arm/mach-davinci/Makefile
> @@ -5,7 +5,7 @@
>
> # Common objects
> obj-y := time.o clock.o serial.o io.o psc.o \
> - gpio.o dma.o usb.o common.o sram.o aemif.o
> + gpio.o dma.o common.o sram.o aemif.o
>
> obj-$(CONFIG_DAVINCI_MUX) += mux.o
>
> @@ -40,3 +40,4 @@ obj-$(CONFIG_MACH_OMAPL138_HAWKBOARD) += board-omapl138-hawk.o
> obj-$(CONFIG_CPU_FREQ) += cpufreq.o
> obj-$(CONFIG_CPU_IDLE) += cpuidle.o
> obj-$(CONFIG_SUSPEND) += pm.o sleep.o
> +obj-$(CONFIG_USB_MUSB_DAVINCI) += usb.o
And kernel linking will happily fail if CONFIG_USB_MUSB_DAVINCI=n. :-/
Also, I don't think making usb.c a module is a good idea...
> diff --git a/arch/arm/mach-davinci/usb.c b/arch/arm/mach-davinci/usb.c
> index 23d2b6d..8b12206 100644
> --- a/arch/arm/mach-davinci/usb.c
> +++ b/arch/arm/mach-davinci/usb.c
> @@ -4,19 +4,22 @@
> #include<linux/init.h>
> #include<linux/platform_device.h>
> #include<linux/dma-mapping.h>
> -
> #include<linux/usb/musb.h>
>
> #include<mach/common.h>
> #include<mach/irqs.h>
> #include<mach/cputype.h>
> #include<mach/usb.h>
> +#include<mach/mux.h>
> +#include<linux/gpio.h>
>
> #define DAVINCI_USB_OTG_BASE 0x01c64000
>
> #define DA8XX_USB0_BASE 0x01e00000
> #define DA8XX_USB1_BASE 0x01e25000
>
> +static int retval;
Why in the world it's 'static'? :-O
> @@ -87,7 +90,7 @@ static struct platform_device usb_dev = {
> .num_resources = ARRAY_SIZE(usb_resources),
> };
>
> -void __init davinci_setup_usb(unsigned mA, unsigned potpgt_ms)
> +int __init davinci_setup_usb(unsigned mA, unsigned potpgt_ms)
> {
> usb_data.power = mA> 510 ? 255 : mA / 2;
> usb_data.potpgt = (potpgt_ms + 1) / 2;
> @@ -99,7 +102,8 @@ void __init davinci_setup_usb(unsigned mA, unsigned
> potpgt_ms)
The patch is line-wrapped...
> } else /* other devices don't have dedicated CPPI IRQ */
> usb_dev.num_resources = 2;
>
> - platform_device_register(&usb_dev);
> + retval = platform_device_register(&usb_dev);
> + return retval;
Why not just:
return platform_device_register(&usb_dev);
> }
>
> #ifdef CONFIG_ARCH_DAVINCI_DA8XX
> @@ -132,7 +136,7 @@ int __init da8xx_register_usb20(unsigned mA,
> unsigned potpgt)
>
> #else
>
> -void __init davinci_setup_usb(unsigned mA, unsigned potpgt_ms)
> +int __init davinci_setup_usb(unsigned mA, unsigned potpgt_ms)
> {
> }
>
> @@ -178,3 +182,23 @@ int __init da8xx_register_usb11(struct
> da8xx_ohci_root_hub *pdata)
> return platform_device_register(&da8xx_usb11_device);
> }
> #endif /* CONFIG_DAVINCI_DA8XX */
> +
> +#ifdef CONFIG_MACH_DAVINCI_DM365_EVM
How do you think why do we have arch/arm/mach-davinci/board-dm365-evm.c?
> +int __init dm365evm_usb_configure(void)
> +{
> + davinci_cfg_reg(DM365_GPIO33);
> + gpio_request(33, "usb");
> + gpio_direction_output(33, 1);
> + retval = davinci_setup_usb(500, 8);
> + return retval;
Why not just:
return davinci_setup_usb(500, 8);
> diff --git a/drivers/usb/musb/davinci.c b/drivers/usb/musb/davinci.c
> index 2a2adf6..0147f5c 100644
> --- a/drivers/usb/musb/davinci.c
> +++ b/drivers/usb/musb/davinci.c
> @@ -72,6 +72,16 @@ static inline void phy_on(void)
> /* power everything up; start the on-chip PHY and its PLL */
> phy_ctrl&= ~(USBPHY_OSCPDWN | USBPHY_OTGPDWN | USBPHY_PHYPDWN);
> phy_ctrl |= USBPHY_SESNDEN | USBPHY_VBDTCTEN | USBPHY_PHYPLLON;
> +
> + if (cpu_is_davinci_dm365()) {
> + /*
> + * DM365 PHYCLKFREQ field [15:12] is set to 2
> + * to get clock from 24MHz crystal
> + */
> + phy_ctrl |= USBPHY_CLKFREQ_24MHZ;
I do think this should be set in the board specific code instead, like we
do it on DA830.
> + /*phy_ctrl&= ~USBPHY_PHYPDWN;*/
Don't include commented out code.
> + }
> +
> __raw_writel(phy_ctrl, USB_PHY_CTRL);
>
> /* wait for PLL to lock before proceeding */
> @@ -193,6 +203,9 @@ static void davinci_musb_source_power(struct musb
> *musb, int is_on, int immediat
> else
> schedule_work(&evm_vbus_work);
> }
> +
> + if (cpu_is_davinci_dm365())
> + gpio_set_value(33, is_on);
This is board specific code, not CPU specific.
> diff --git a/drivers/usb/musb/davinci.h b/drivers/usb/musb/davinci.h
> index 046c844..1bf50e6 100644
> --- a/drivers/usb/musb/davinci.h
> +++ b/drivers/usb/musb/davinci.h
> @@ -17,6 +17,7 @@
> /* Integrated highspeed/otg PHY */
> #define USBPHY_CTL_PADDR (DAVINCI_SYSTEM_MODULE_BASE + 0x34)
> #define USBPHY_DATAPOL BIT(11) /* (dm355) switch D+/D- */
> +#define USBPHY_CLKFREQ_24MHZ BIT(13)
> #define USBPHY_PHYCLKGD BIT(8)
> #define USBPHY_SESNDEN BIT(7) /* v(sess_end) comparator */
> #define USBPHY_VBDTCTEN BIT(6) /* v(bus) comparator */
prev parent reply other threads:[~2011-06-27 9:12 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-06-24 18:51 [PATCH] Enable USB on TI DM365 Constantine Shulyupin
2011-06-26 12:11 ` Sebastian Heß
[not found] ` <BANLkTims=UsUMkDQ8uB+J6YTVRSBD__otw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2011-06-27 9:12 ` Sergei Shtylyov [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4E084965.4020409@mvista.com \
--to=sshtylyov-igf4poytycdqt0dzr+alfa@public.gmane.org \
--cc=const-XSqIthwXjGBDPfheJLI6IQ@public.gmane.org \
--cc=davinci-linux-open-source-VycZQUHpC/PFrsHnngEfi1aTQe2KTcn/@public.gmane.org \
--cc=linux-embedded-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
--cc=linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).