2018-07-18 00:16:36

by Roman Kiryanov

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

From: Roman Kiryanov <[email protected]>

To make further maintenance easier.

Signed-off-by: Roman Kiryanov <[email protected]>
---
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



2018-07-18 00:16:45

by Roman Kiryanov

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

From: Roman Kiryanov <[email protected]>

enums produce better compilation errors than defines.

Signed-off-by: Roman Kiryanov <[email protected]>
---
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


2018-07-18 00:16:51

by Roman Kiryanov

[permalink] [raw]
Subject: [PATCH 3/3] tty: Mark goldfish_tty_line_count as const

From: Roman Kiryanov <[email protected]>

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

Signed-off-by: Roman Kiryanov <[email protected]>
---
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


2018-07-21 07:45:40

by Greg Kroah-Hartman

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

On Tue, Jul 17, 2018 at 05:14:53PM -0700, [email protected] wrote:
> From: Roman Kiryanov <[email protected]>
>
> 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

2018-07-21 07:46:34

by Greg Kroah-Hartman

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

On Tue, Jul 17, 2018 at 05:14:54PM -0700, [email protected] wrote:
> From: Roman Kiryanov <[email protected]>
>
> 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

2018-07-21 07:48:21

by Greg Kroah-Hartman

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

On Tue, Jul 17, 2018 at 05:14:55PM -0700, [email protected] wrote:
> From: Roman Kiryanov <[email protected]>
>
> 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

2018-07-24 20:01:35

by Roman Kiryanov

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

> 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.