| Message ID | 1310467956-21739-1-git-send-email-const@MakeLinux.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
Hello.
On 12-07-2011 14:52, const@MakeLinux.com wrote:
There's no use resending -- the patch won't be accepted as is.
> # 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
>
It seems the beginning of the patch got eaten by something.
> @@ -132,7 +133,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)
> {
You need the *return* statement here now...
> }
>
WBR, Sergei
Hello, Please Cc linux-arm-kernel@lists.infradead.org for all arch/arm/mach-davinci/* patch submissions. Apart from what Sergei said, please take care of the following: On Tue, Jul 12, 2011 at 16:22:36, const@MakeLinux.com wrote: > # 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 These makefile changes should be separate patch by itself since it affects all platforms not just DM365. > diff --git a/arch/arm/mach-davinci/include/mach/usb.h b/arch/arm/mach-davinci/include/mach/usb.h > index e0bc4ab..863c27f 100644 > --- a/arch/arm/mach-davinci/include/mach/usb.h > +++ b/arch/arm/mach-davinci/include/mach/usb.h > @@ -54,6 +54,6 @@ struct da8xx_ohci_root_hub { > u8 potpgt; > }; > > -void davinci_setup_usb(unsigned mA, unsigned potpgt_ms); > +int davinci_setup_usb(unsigned mA, unsigned potpgt_ms); > > #endif /* ifndef __ASM_ARCH_USB_H */ > diff --git a/arch/arm/mach-davinci/usb.c b/arch/arm/mach-davinci/usb.c > index 23d2b6d..daf5398 100644 > --- a/arch/arm/mach-davinci/usb.c > +++ b/arch/arm/mach-davinci/usb.c > @@ -4,13 +4,14 @@ > #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 > > @@ -87,7 +88,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 +100,7 @@ void __init davinci_setup_usb(unsigned mA, unsigned potpgt_ms) > } else /* other devices don't have dedicated CPPI IRQ */ > usb_dev.num_resources = 2; > > - platform_device_register(&usb_dev); > + return platform_device_register(&usb_dev); > } > > #ifdef CONFIG_ARCH_DAVINCI_DA8XX > @@ -132,7 +133,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) > { > } This looks like a bugfix/enhancement which is not related to adding DM365 support. Please keep it separate. > > @@ -178,3 +179,23 @@ int __init da8xx_register_usb11(struct da8xx_ohci_root_hub *pdata) > return platform_device_register(&da8xx_usb11_device); > } > #endif /* CONFIG_DAVINCI_DA8XX */ > + > +#ifdef ARCH_DAVINCI_DM365 > +int __init dm365_usb_configure(void) > +{ > + /* GPIO33 is multiplexed with USB DRVVBUS */ > + davinci_cfg_reg(DM365_GPIO33); > + gpio_request(33, "usb"); This can fail. Also use gpio_request_one(). > + gpio_direction_output(33, 1); > + return davinci_setup_usb(500, 8); > +} > +subsys_initcall(dm365_usb_configure); > + > +static void __exit dm365evm_usb_exit(void) > +{ > + platform_device_unregister(&usb_dev); > +} > +module_exit(dm365evm_usb_exit); This is board code which should go into board-dm365-evm.c Also, please don't use initcalls for this kind of code, simply invoke the API in the board init sequence. > +#endif > + > +MODULE_LICENSE("GPL"); > diff --git a/drivers/usb/musb/davinci.c b/drivers/usb/musb/davinci.c > index 2a2adf6..e6721e1 100644 > --- a/drivers/usb/musb/davinci.c > +++ b/drivers/usb/musb/davinci.c > @@ -72,6 +72,15 @@ 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; > + } Please separate out the DM365 SoC specific changes into a separate patch and CC the USB maintainers Felipe/Greg K-H on it. Also, CC the linux-usb list. scripts/get_maintainer.pl is there to help you find the right maintainers for the code your are submitting. > + > __raw_writel(phy_ctrl, USB_PHY_CTRL); > > /* wait for PLL to lock before proceeding */ > @@ -193,6 +202,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 again looks like an EVM specific change so should be confined to board-dm365-evm.c. > if (immediate) > vbus_state = is_on; > #endif > 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) Please use tabs for indentation. Thanks, Sekhar
Hello. On 21-07-2011 14:19, Nori, Sekhar wrote: > Apart from what Sergei said, please take care of the following: I've alresdy reviewed the first posting in detail, and we've exchanged some porivate emails with Constantin too... >> diff --git a/arch/arm/mach-davinci/usb.c b/arch/arm/mach-davinci/usb.c >> index 23d2b6d..daf5398 100644 >> --- a/arch/arm/mach-davinci/usb.c >> +++ b/arch/arm/mach-davinci/usb.c [...] >> @@ -178,3 +179,23 @@ int __init da8xx_register_usb11(struct da8xx_ohci_root_hub *pdata) >> return platform_device_register(&da8xx_usb11_device); >> } >> #endif /* CONFIG_DAVINCI_DA8XX */ >> + >> +#ifdef ARCH_DAVINCI_DM365 >> +int __init dm365_usb_configure(void) >> +{ >> + /* GPIO33 is multiplexed with USB DRVVBUS */ >> + davinci_cfg_reg(DM365_GPIO33); >> + gpio_request(33, "usb"); > This can fail. Also use gpio_request_one(). The main question as it turned out is why GPIO API was used at all. DM365 has real VBUS signal and it is *multiplexed* to that GPIO. I keep wondering why VBUS signal was not directly controlled... >> + gpio_direction_output(33, 1); >> + return davinci_setup_usb(500, 8); >> +} >> +subsys_initcall(dm365_usb_configure); >> + >> +static void __exit dm365evm_usb_exit(void) >> +{ >> + platform_device_unregister(&usb_dev); >> +} >> +module_exit(dm365evm_usb_exit); > This is board code which should go into board-dm365-evm.c That I've noted when reviewing the first posting of this patch. >> diff --git a/drivers/usb/musb/davinci.c b/drivers/usb/musb/davinci.c >> index 2a2adf6..e6721e1 100644 >> --- a/drivers/usb/musb/davinci.c >> +++ b/drivers/usb/musb/davinci.c >> @@ -72,6 +72,15 @@ 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; >> + } > Please separate out the DM365 SoC specific changes into > a separate patch and CC the USB maintainers Felipe/Greg K-H > on it. Also, CC the linux-usb list. scripts/get_maintainer.pl > is there to help you find the right maintainers for the > code your are submitting. I think this code is actually board specific, and should be in board-dm365-evm.c instead... >> + >> __raw_writel(phy_ctrl, USB_PHY_CTRL); >> >> /* wait for PLL to lock before proceeding */ >> @@ -193,6 +202,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 again looks like an EVM specific change so > should be confined to board-dm365-evm.c. This time it's DM365 specific as that GPIO is *multiplexed* to VBUS. WBR, Sergei
diff --git a/arch/arm/mach-davinci/include/mach/usb.h b/arch/arm/mach-davinci/include/mach/usb.h index e0bc4ab..863c27f 100644 --- a/arch/arm/mach-davinci/include/mach/usb.h +++ b/arch/arm/mach-davinci/include/mach/usb.h @@ -54,6 +54,6 @@ struct da8xx_ohci_root_hub { u8 potpgt; }; -void davinci_setup_usb(unsigned mA, unsigned potpgt_ms); +int davinci_setup_usb(unsigned mA, unsigned potpgt_ms); #endif /* ifndef __ASM_ARCH_USB_H */ diff --git a/arch/arm/mach-davinci/usb.c b/arch/arm/mach-davinci/usb.c index 23d2b6d..daf5398 100644 --- a/arch/arm/mach-davinci/usb.c +++ b/arch/arm/mach-davinci/usb.c @@ -4,13 +4,14 @@ #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 @@ -87,7 +88,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 +100,7 @@ void __init davinci_setup_usb(unsigned mA, unsigned potpgt_ms) } else /* other devices don't have dedicated CPPI IRQ */ usb_dev.num_resources = 2; - platform_device_register(&usb_dev); + return platform_device_register(&usb_dev); } #ifdef CONFIG_ARCH_DAVINCI_DA8XX @@ -132,7 +133,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 +179,23 @@ int __init da8xx_register_usb11(struct da8xx_ohci_root_hub *pdata) return platform_device_register(&da8xx_usb11_device); } #endif /* CONFIG_DAVINCI_DA8XX */ + +#ifdef ARCH_DAVINCI_DM365 +int __init dm365_usb_configure(void) +{ + /* GPIO33 is multiplexed with USB DRVVBUS */ + davinci_cfg_reg(DM365_GPIO33); + gpio_request(33, "usb"); + gpio_direction_output(33, 1); + return davinci_setup_usb(500, 8); +} +subsys_initcall(dm365_usb_configure); + +static void __exit dm365evm_usb_exit(void) +{ + platform_device_unregister(&usb_dev); +} +module_exit(dm365evm_usb_exit); +#endif + +MODULE_LICENSE("GPL"); diff --git a/drivers/usb/musb/davinci.c b/drivers/usb/musb/davinci.c index 2a2adf6..e6721e1 100644 --- a/drivers/usb/musb/davinci.c +++ b/drivers/usb/musb/davinci.c @@ -72,6 +72,15 @@ 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; + } + __raw_writel(phy_ctrl, USB_PHY_CTRL); /* wait for PLL to lock before proceeding */ @@ -193,6 +202,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); if (immediate) vbus_state = is_on; #endif 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 */