On Sat, Aug 30, 2008 at 08:16:00PM +0300, Felipe Balbi wrote:
> From: Felipe Balbi <felipe.balbi@xxxxxxxxx>
>
> The following drivers are going upstream for integration.
> They have been sitting on linux-omap for quite a while just
> increasing the diff against mainline and probability of
> merge conflicts.
Just one comment to this. I had to left bluetooth driver out of the
series because it's using omap2_block/allow_sleep in the driver code.
That should be fixed and the set_clock function should come via
platform_data to the driver.
53 static void hci_h4p_set_clk(struct hci_h4p_info *info, int *clock, int enable)
54 {
55 unsigned long flags;
56
57 spin_lock_irqsave(&info->clocks_lock, flags);
58 if (enable && !*clock) {
59 NBT_DBG_POWER("Enabling %p\n", clock);
60 clk_enable(info->uart_fclk);
61 #ifdef CONFIG_ARCH_OMAP2
62 if (cpu_is_omap24xx()) {
63 clk_enable(info->uart_iclk);
64 omap2_block_sleep();
65 }
66 #endif
67 }
68 if (!enable && *clock) {
69 NBT_DBG_POWER("Disabling %p\n", clock);
70 clk_disable(info->uart_fclk);
71 #ifdef CONFIG_ARCH_OMAP2
72 if (cpu_is_omap24xx()) {
73 clk_disable(info->uart_iclk);
74 omap2_allow_sleep();
75 }
76 #endif
77 }
78
79 *clock = enable;
80 spin_unlock_irqrestore(&info->clocks_lock, flags);
81 }
That driver is full of arch specific code and should be cleaned up ASAP.
A few things I could get by briefly looking at the driver (actualy only
drivers/bluetooth/hci_h4p/core.c):
* set_clock should come via platform_data
* gpio handling should use gpiolib instead of omap-specific ones
* irq should come via platform_data to avoid 'if (cpu_is_xxx())'
* unnecessary casts should be removed
* one line ifs do not need {}
* there's a memory leak on probe if no platform_data is found
* use of io_p2v() should be avoided on drivers, refer to Russel King's
patch
* OMAP_GPIO_IRQ() should not be used on the driver.
--
balbi
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
