From: Roman Kiryanov <[email protected]>
checkpatch.pl complained about missing blank lines
(between variable definitions and executable code)
and using just "unsigned" instead of "unsigned int".
Signed-off-by: Roman Kiryanov <[email protected]>
---
Changes in v2:
- Updated the commit message.
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.233.g985f88cf7e-goog
From: Roman Kiryanov <[email protected]>
enums produce better compilation errors than defines.
Signed-off-by: Roman Kiryanov <[email protected]>
---
Changes in v2:
- Added the enum types (goldfish_tty_reg and goldfish_tty_cmd).
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..173383f2a4c1 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 {
+ 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 {
+ 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.233.g985f88cf7e-goog
From: Roman Kiryanov <[email protected]>
The driver never mutates this variable - no benefits of
keeping it mutable.
Signed-off-by: Roman Kiryanov <[email protected]>
---
Changes in v2:
- Replaced "const u32" with "#define".
drivers/tty/goldfish.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/tty/goldfish.c b/drivers/tty/goldfish.c
index 173383f2a4c1..eb88db4f243c 100644
--- a/drivers/tty/goldfish.c
+++ b/drivers/tty/goldfish.c
@@ -49,10 +49,11 @@ struct goldfish_tty {
static DEFINE_MUTEX(goldfish_tty_lock);
static struct tty_driver *goldfish_tty_driver;
-static u32 goldfish_tty_line_count = 8;
static u32 goldfish_tty_current_line_count;
static struct goldfish_tty *goldfish_ttys;
+#define GOLDFISH_TTY_LINE_COUNT 8
+
static void do_rw_io(struct goldfish_tty *qtty,
unsigned long address,
unsigned int count,
@@ -225,7 +226,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 int)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;
@@ -251,14 +252,14 @@ static int goldfish_tty_create_driver(void)
int ret;
struct tty_driver *tty;
- goldfish_ttys = kcalloc(goldfish_tty_line_count,
+ goldfish_ttys = kcalloc(GOLDFISH_TTY_LINE_COUNT,
sizeof(*goldfish_ttys),
GFP_KERNEL);
if (goldfish_ttys == NULL) {
ret = -ENOMEM;
goto err_alloc_goldfish_ttys_failed;
}
- tty = alloc_tty_driver(goldfish_tty_line_count);
+ tty = alloc_tty_driver(GOLDFISH_TTY_LINE_COUNT);
if (tty == NULL) {
ret = -ENOMEM;
goto err_alloc_tty_driver_failed;
@@ -333,7 +334,7 @@ static int goldfish_tty_probe(struct platform_device *pdev)
else
line = pdev->id;
- if (line >= goldfish_tty_line_count) {
+ if (line >= GOLDFISH_TTY_LINE_COUNT) {
pr_err("goldfish_tty: Reached maximum tty number of %d.\n",
goldfish_tty_current_line_count);
ret = -ENOMEM;
--
2.18.0.233.g985f88cf7e-goog
On Tue, Jul 24, 2018 at 05:51:32PM -0700, [email protected] wrote:
> From: Roman Kiryanov <[email protected]>
>
> enums produce better compilation errors than defines.
Yes, they can, if you use them correctly. Which you are not doing here,
so this patch really does nothing. Why should we take it?
confused,
greg k-h
On Tue, Jul 24, 2018 at 05:51:31PM -0700, [email protected] wrote:
> From: Roman Kiryanov <[email protected]>
>
> checkpatch.pl complained about missing blank lines
> (between variable definitions and executable code)
> and using just "unsigned" instead of "unsigned int".
That is two different things, please make two different patches.
thanks,
greg k-h
On Tue, Jul 24, 2018 at 05:51:33PM -0700, [email protected] wrote:
> From: Roman Kiryanov <[email protected]>
>
> The driver never mutates this variable - no benefits of
> keeping it mutable.
>
> Signed-off-by: Roman Kiryanov <[email protected]>
> ---
> Changes in v2:
> - Replaced "const u32" with "#define".
>
> drivers/tty/goldfish.c | 11 ++++++-----
> 1 file changed, 6 insertions(+), 5 deletions(-)
Does not apply to my tree :(
On Tue, 24 Jul 2018 17:51:33 -0700
[email protected] wrote:
> From: Roman Kiryanov <[email protected]>
>
> The driver never mutates this variable - no benefits of
> keeping it mutable.
>
> Signed-off-by: Roman Kiryanov <[email protected]>
> ---
> Changes in v2:
> - Replaced "const u32" with "#define".
v1 was IMHO definitely better. And you are correct that gcc will not
allocate memory for a static const object unless you do things like take
its address. It will (unlike a define) however properly typecheck
everything.
Alan