[PATCH 1/3] tty: Address checkpatch warnings in goldfish.c

[PATCH 1/3] tty: Address checkpatch warnings in goldfish.c

[rkir]

From: Roman Kiryanov <rkir@google.com>

To make further maintenance easier.

Signed-off-by: Roman Kiryanov <rkir@google.com>
---
 drivers/tty/goldfish.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/goldfish.c b/drivers/tty/goldfish.c
index 37caba7c3aff..a92fcb2b0002 100644
--- a/drivers/tty/goldfish.c
+++ b/drivers/tty/goldfish.c
@@ -172,6 +172,7 @@ static void goldfish_tty_shutdown(struct tty_port *port)
 static int goldfish_tty_open(struct tty_struct *tty, struct file *filp)
 {
 	struct goldfish_tty *qtty = &goldfish_ttys[tty->index];
+
 	return tty_port_open(&qtty->port, tty, filp);
 }
 
@@ -201,11 +202,12 @@ static int goldfish_tty_chars_in_buffer(struct tty_struct *tty)
 {
 	struct goldfish_tty *qtty = &goldfish_ttys[tty->index];
 	void __iomem *base = qtty->base;
+
 	return readl(base + GOLDFISH_TTY_REG_BYTES_READY);
 }
 
 static void goldfish_tty_console_write(struct console *co, const char *b,
-								unsigned count)
+				       unsigned int count)
 {
 	goldfish_tty_do_write(co->index, b, count);
 }
@@ -219,7 +221,7 @@ static struct tty_driver *goldfish_tty_console_device(struct console *c,
 
 static int goldfish_tty_console_setup(struct console *co, char *options)
 {
-	if ((unsigned)co->index >= goldfish_tty_line_count)
+	if ((unsigned int)co->index >= goldfish_tty_line_count)
 		return -ENODEV;
 	if (!goldfish_ttys[co->index].base)
 		return -ENODEV;
-- 
2.18.0.203.gfac676dfb9-goog

[PATCH 2/3] tty: Make constants to be enums instead of #define in goldfish.c

[rkir]

From: Roman Kiryanov <rkir@google.com>

enums produce better compilation errors than defines.

Signed-off-by: Roman Kiryanov <rkir@google.com>
---
 drivers/tty/goldfish.c | 24 ++++++++++++++----------
 1 file changed, 14 insertions(+), 10 deletions(-)

diff --git a/drivers/tty/goldfish.c b/drivers/tty/goldfish.c
index a92fcb2b0002..a9c8ab8a4750 100644
--- a/drivers/tty/goldfish.c
+++ b/drivers/tty/goldfish.c
@@ -19,18 +19,22 @@
 #include <linux/serial_core.h>
 
 /* Goldfish tty register's offsets */
-#define	GOLDFISH_TTY_REG_BYTES_READY	0x04
-#define	GOLDFISH_TTY_REG_CMD		0x08
-#define	GOLDFISH_TTY_REG_DATA_PTR	0x10
-#define	GOLDFISH_TTY_REG_DATA_LEN	0x14
-#define	GOLDFISH_TTY_REG_DATA_PTR_HIGH	0x18
-#define	GOLDFISH_TTY_REG_VERSION	0x20
+enum {
+	GOLDFISH_TTY_REG_BYTES_READY	= 0x04,
+	GOLDFISH_TTY_REG_CMD		= 0x08,
+	GOLDFISH_TTY_REG_DATA_PTR	= 0x10,
+	GOLDFISH_TTY_REG_DATA_LEN	= 0x14,
+	GOLDFISH_TTY_REG_DATA_PTR_HIGH	= 0x18,
+	GOLDFISH_TTY_REG_VERSION	= 0x20,
+};
 
 /* Goldfish tty commands */
-#define	GOLDFISH_TTY_CMD_INT_DISABLE	0
-#define	GOLDFISH_TTY_CMD_INT_ENABLE	1
-#define	GOLDFISH_TTY_CMD_WRITE_BUFFER	2
-#define	GOLDFISH_TTY_CMD_READ_BUFFER	3
+enum {
+	GOLDFISH_TTY_CMD_INT_DISABLE	= 0,
+	GOLDFISH_TTY_CMD_INT_ENABLE	= 1,
+	GOLDFISH_TTY_CMD_WRITE_BUFFER	= 2,
+	GOLDFISH_TTY_CMD_READ_BUFFER	= 3,
+};
 
 struct goldfish_tty {
 	struct tty_port port;
-- 
2.18.0.203.gfac676dfb9-goog

Re: [PATCH 2/3] tty: Make constants to be enums instead of #define in goldfish.c

[Greg KH]

On Tue, Jul 17, 2018 at 05:14:54PM -0700, rkir@google.com wrote:
> From: Roman Kiryanov <rkir@google.com>
> 
> enums produce better compilation errors than defines.

Yes, if you name the enum.  But you didn't do that here, so this patch
really does not do anything to cause any "protection" at all :(

Please fix this up and resend.

thanks,

greg k-h

[PATCH 3/3] tty: Mark goldfish_tty_line_count as const

[rkir]

From: Roman Kiryanov <rkir@google.com>

The driver never mutates this variable - no benefits of
keeping it mutable.

Signed-off-by: Roman Kiryanov <rkir@google.com>
---
 drivers/tty/goldfish.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/tty/goldfish.c b/drivers/tty/goldfish.c
index a9c8ab8a4750..32b77bf54918 100644
--- a/drivers/tty/goldfish.c
+++ b/drivers/tty/goldfish.c
@@ -49,7 +49,7 @@ struct goldfish_tty {
 
 static DEFINE_MUTEX(goldfish_tty_lock);
 static struct tty_driver *goldfish_tty_driver;
-static u32 goldfish_tty_line_count = 8;
+static const u32 goldfish_tty_line_count = 8;
 static u32 goldfish_tty_current_line_count;
 static struct goldfish_tty *goldfish_ttys;
 
-- 
2.18.0.203.gfac676dfb9-goog

Re: [PATCH 3/3] tty: Mark goldfish_tty_line_count as const

[Greg KH]

On Tue, Jul 17, 2018 at 05:14:55PM -0700, rkir@google.com wrote:
> From: Roman Kiryanov <rkir@google.com>
> 
> The driver never mutates this variable - no benefits of
> keeping it mutable.

Then why not just make it a #define?  No need to waste the memory of a
variable, right?

thanks,

greg k-h

Re: [PATCH 3/3] tty: Mark goldfish_tty_line_count as const

[Roman Kiryanov]

> Then why not just make it a #define?

With "const" the diff is smaller.

> No need to waste the memory of a variable, right?

I believe the compiler will produce the same binary for const and for
#define if optimization is enabled.

Re: [PATCH 1/3] tty: Address checkpatch warnings in goldfish.c

[Greg KH]

On Tue, Jul 17, 2018 at 05:14:53PM -0700, rkir@google.com wrote:
> From: Roman Kiryanov <rkir@google.com>
> 
> To make further maintenance easier.

That sentance makes no sense :(

Please be specific as to what exactly you are doing here.

thanks,

greg k-h