Sergei, Thanks for the review. I am sorry, I posted the patches that were aligned with ARAGO project, will post the one that I have aligned against Davinci GIT.
I am not sure why you see spaces instead of tabs here. Will re-work on the same. Regards, Khasim > -----Original Message----- > From: Sergei Shtylyov [mailto:[email protected]] > Sent: Monday, November 23, 2009 10:15 PM > To: Syed Mohammed, Khasim > Cc: [email protected] > Subject: Re: [RFC 2/4] Adds support for OMAPL138 based hawkboard > > Hello. > > Syed Mohammed, Khasim wrote: > > > This patch adds support for OMAPL138 based hawkboard > > > http://www.hawkboard.org > > > Signed-off-by: Syed Mohammed Khasim <[email protected]> > > Your patch has spaces instead of tabs everywhere... > > diff --git a/arch/arm/mach-davinci/Makefile b/arch/arm/mach-davinci/Makefile > > index 5021408..db50ba6 100644 > > --- a/arch/arm/mach-davinci/Makefile > > +++ b/arch/arm/mach-davinci/Makefile > > @@ -30,7 +30,7 @@ obj-$(CONFIG_MACH_DAVINCI_DM6467_EVM) += board-dm646x- > evm.o > > obj-$(CONFIG_MACH_DAVINCI_DM365_EVM) += board-dm365-evm.o > > obj-$(CONFIG_MACH_DAVINCI_DA830_EVM) += board-da830-evm.o > > obj-$(CONFIG_MACH_DAVINCI_DA850_EVM) += board-da850-evm.o > > - > > +obj-$(CONFIG_MACH_OMAPL138_HAWKBOARD) += board-omapl138-hawk.o > > # Power Management > > obj-$(CONFIG_CPU_FREQ) += cpufreq.o > > obj-$(CONFIG_CPU_IDLE) += cpuidle.o > > > > diff --git a/arch/arm/mach-davinci/board-omapl138-hawk.c b/arch/arm/mach- > davinci/board-omapl138-hawk.c > > new file mode 100755 > > index 0000000..c9a7f95 > > --- /dev/null > > +++ b/arch/arm/mach-davinci/board-omapl138-hawk.c > > @@ -0,0 +1,574 @@ > > +/* > > + * Hawkboar.org based on TI's OMAP-L138 Platform > > hawkboard.org? > > > > +#include <linux/i2c/at24.h> > > +#include <linux/i2c/pca953x.h> > > I don't see you really using these... > > > +#include <linux/gpio.h> > > +#include <linux/platform_device.h> > > +#include <linux/mtd/mtd.h> > > +#include <linux/mtd/nand.h> > > +#include <linux/mtd/partitions.h> > > +#include <linux/mtd/physmap.h> > > I don't see you using a "physmap" device -- why #include the header? > > > +#include <linux/regulator/machine.h> > > Do you really need this file? > > [...] > > > +static struct da8xx_ohci_root_hub omapl138_hawk_usb11_pdata = { > > + .set_power = omapl138_hawk_usb_set_power, > > + .get_power = omapl138_hawk_usb_get_power, > > + .get_oci = omapl138_hawk_usb_get_oci, > > + .ocic_notify = omapl138_hawk_usb_ocic_notify, > > + > > + /* TPS2065 switch @ 5V */ > > + .potpgt = (3 + 1) / 2, /* 3 ms max */ > > Are you sure you have that same power switching chip as DA830 EVM? > > [...] > > > +static struct musb_hdrc_platform_data usb_evm_data[] = { > > + { > > +#ifdef CONFIG_USB_MUSB_OTG > > + .mode = MUSB_OTG, > > +#elif defined(CONFIG_USB_MUSB_DUAL_ROLE) > > + .mode = MUSB_DUAL_ROLE, > > +#elif defined(CONFIG_USB_MUSB_PERIPHERAL) > > + .mode = MUSB_PERIPHERAL, > > +#elif defined(CONFIG_USB_MUSB_HOST) > > + .mode = MUSB_HOST, > > +#endif > > + .power = 255, > > + .potpgt = 8, > > + .set_vbus = NULL, /* VBUs is directly controlled by the IP > */ > > + } > > +}; > > The MUSB platfrom data is currently in arch/arm/mach-davinci/usb.c. You > call davinci_setup_usb() to pass 'power' and 'potpgt' and register the device. > > > +static __init void omapl138_hawk_usb_init(void) > > +{ > > + int ret; > > + u32 cfgchip2; > > + > > + /* > > + * Setup the Ref. clock frequency for the EVM at 24 MHz. > > + */ > > + cfgchip2 = __raw_readl(DA8XX_SYSCFG_VIRT(DA8XX_CFGCHIP2_REG)); > > + cfgchip2 &= ~CFGCHIP2_REFFREQ; > > + cfgchip2 |= CFGCHIP2_REFFREQ_24MHZ; > > + __raw_writel(cfgchip2, DA8XX_SYSCFG_VIRT(DA8XX_CFGCHIP2_REG)); > > + > > + da8xx_usb20_configure(usb_evm_data, ARRAY_SIZE(usb_evm_data)); > > + > > + ret = da8xx_pinmux_setup(da850_evm_usb11_pins); > > Hm... I don't see 'da850_evm_usb11_pins' defined anywhere, and you can't > use data from board-da850-evm anyway. > > > + if (ret) { > > + pr_warning("%s: USB 1.1 PinMux setup failed: %d\n", > > + __func__, ret); > > + return; > > + } > > + > > + ret = gpio_request(DA850_USB1_VBUS_PIN, "USB1 VBUS\n"); > > + if (ret) { > > + printk(KERN_ERR "%s: failed to request GPIO for USB 1.1 port > " > > Use pr_err() or pr_warning() for consistenly. > > > + "power control: %d\n", __func__, ret); > > + return; > > + } > > + gpio_direction_output(DA850_USB1_VBUS_PIN, 0); > > + > > + ret = gpio_request(DA850_USB1_OC_PIN, "USB1 OC"); > > + if (ret) { > > + printk(KERN_ERR "%s: failed to request GPIO for USB 1.1 port > " > > pr_err() or pr_warning(). > > > +static __init void omapl138_hawk_init(void) > > +{ > [...] > > + if (HAS_MMC) { > > + ret = da8xx_pinmux_setup(da850_mmcsd0_pins); > > You cannot just use da850_mmcsd0_pins here as it doesn't include your 2 > GPIO pins used as MMC CD/WP signals. > > [...] > > > +static __init void omapl138_hawk_irq_init(void) > > +{ > > + struct davinci_soc_info *soc_info = &davinci_soc_info; > > Pointless variable, you can use davinci_soc_info directly here... > > > + > > + cp_intc_init((void __iomem *)DA8XX_CP_INTC_VIRT, > DA850_N_CP_INTC_IRQ, > > + soc_info->intc_irq_prios); > > +} > > + > > +static void __init omapl138_hawk_map_io(void) > > +{ > > + da850_init(); > > +} > > + > > +MACHINE_START(OMAPL138_HAWKBOARD, "OMAPL 138 Hawkboard.org") > > + .phys_io = IO_PHYS, > > + .io_pg_offst = (__IO_ADDRESS(IO_PHYS) >> 18) & 0xfffc, > > + .boot_params = (DA8XX_DDR_BASE + 0x100), > > + .map_io = omapl138_hawk_map_io, > > + .init_irq = omapl138_hawk_irq_init, > > + .timer = &davinci_timer, > > + .init_machine = omapl138_hawk_init, > > +MACHINE_END > > > > Won't empty line here break the patch? > > > diff --git a/arch/arm/mach-davinci/include/mach/debug-macro.S > b/arch/arm/mach-davinci/include/mach/debug-macro.S > > index 17ab523..f04c481 100644 > > --- a/arch/arm/mach-davinci/include/mach/debug-macro.S > > +++ b/arch/arm/mach-davinci/include/mach/debug-macro.S > > @@ -27,7 +27,8 @@ > > #if defined(CONFIG_ARCH_DAVINCI_DA8XX) && defined(CONFIG_ARCH_DAVINCI_DMx) > > #error Cannot enable DaVinci and DA8XX platforms concurrently > > #elif defined(CONFIG_MACH_DAVINCI_DA830_EVM) || \ > > - defined(CONFIG_MACH_DAVINCI_DA850_EVM) > > + defined(CONFIG_MACH_DAVINCI_DA850_EVM) || \ > > + defined(CONFIG_MACH_OMAPL138_HAWKBOARD) > > Please keep the alignment... > > WBR, Sergei _______________________________________________ Davinci-linux-open-source mailing list [email protected] http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
