Thread (2 messages) 2 messages, 2 authors, 2011-09-09
STALE5385d

[PATCH v4 04/11] OMAP2+: UART: Remove uart clock handling code from serial.c

From: Kevin Hilman <hidden>
Date: 2011-09-08 23:39:21
Also in: linux-omap, linux-serial

Possibly related (same subject, not in this thread)

"Govindraj.R" [off-list ref] writes:
Cleanup serial.c file in preparation to addition of runtime API's in omap-serial
file. Remove all clock handling mechanism as this will be taken care with
pm runtime API's in omap-serial.c file itself.

1.) Remove omap-device enable and disable. We can can use get_sync/put_sync API's
2.) Remove context save/restore can be done with runtime_resume callback for
    get_sync call. No need to save context as all reg details available in
    uart_port structure can be used for restore, so add missing regs in
    uart port struct.
3.) Add func to identify console uart.
I don't see that as part of this patch.
4.) Erratum handling informed as flag to driver and func to handle erratum
    can be moved to omap-serial driver itself.
OK, but I see two erratum flags removed, but only flag added back.
Also, the erratum handling is completely removed here and not added to
the driver.

In general, this way of moving things makes it very difficult to review.
You remove a bunch of things in this patch (and the previous one) and
imply that some of it is added back to the driver.  However, that's
really difficult to verify with patch review.

If code is being moved, it is customary to remove it and add it to the
new place in the same patch.
Acked-by: Alan Cox <redacted>
Signed-off-by: Govindraj.R <redacted>
---
 arch/arm/mach-omap2/serial.c                  |  742 ++-----------------------
 arch/arm/plat-omap/include/plat/omap-serial.h |   11 +-
 2 files changed, 53 insertions(+), 700 deletions(-)
[...]
-static DEVICE_ATTR(sleep_timeout, 0644, sleep_timeout_show,
-		sleep_timeout_store);
-#define DEV_CREATE_FILE(dev, attr) WARN_ON(device_create_file(dev, attr))
-#else
-static inline void omap_uart_idle_init(struct omap_uart_state *uart) {}
-static void omap_uart_block_sleep(struct omap_uart_state *uart)
+struct omap_hwmod *omap_uart_hwmod_lookup(int num)
function should be static

[...]
+	for (i = 0; i < OMAP_MAX_HSUART_PORTS; i++) {
+		oh = omap_uart_hwmod_lookup(i);
 		if (!oh)
-			break;
-
-		uart = kzalloc(sizeof(struct omap_uart_state), GFP_KERNEL);
-		if (WARN_ON(!uart))
-			return -ENODEV;
-
-		uart->oh = oh;
-		uart->num = i++;
-		list_add_tail(&uart->node, &uart_list);
-		num_uarts++;
-
-		/*
-		 * NOTE: omap_hwmod_setup*() has not yet been called,
-		 *       so no hwmod functions will work yet.
-		 */
-
-		/*
-		 * During UART early init, device need to be probed
-		 * to determine SoC specific init before omap_device
-		 * is ready.  Therefore, don't allow idle here
-		 */
-		uart->oh->flags |= HWMOD_INIT_NO_IDLE | HWMOD_INIT_NO_RESET;
-	} while (1);
+			continue;
 
+		oh->flags |= HWMOD_INIT_NO_IDLE | HWMOD_INIT_NO_RESET;
With proper runtime PM in the driver, I did not expect these to be
needed any longer by the end of the series, but I see they are still
there.  Are they really needed?

[...]
quoted hunk
@@ -864,15 +213,14 @@ void __init omap_serial_init_port(struct omap_board_data *bdata)
  */
 void __init omap_serial_init(void)
 {
-	struct omap_uart_state *uart;
 	struct omap_board_data bdata;
+	u8 i;
 
-	list_for_each_entry(uart, &uart_list, node) {
-		bdata.id = uart->num;
+	for (i = 0; i < OMAP_MAX_HSUART_PORTS; i++) {
This should probably use the hwmod class iterator.

Kevin
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help