Drop the driver version of the line-coding request and use the protocol
definition directly as was originally intended instead.
This specifically avoids having the two versions of what is supposed to
be the same struct ever getting out of sync.
Note that this has in fact already happened once when the protocol
definition had its implicit padding removed while the driver struct
wasn't updated. The fact that we used the size of the then larger driver
struct when memcpying its content to the stack didn't exactly make
things better. A later addition of a flow-control field incidentally
made the structures match again.
Signed-off-by: Johan Hovold <johan(a)kernel.org>
---
drivers/staging/greybus/uart.c | 19 ++++---------------
1 file changed, 4 insertions(+), 15 deletions(-)
diff --git a/drivers/staging/greybus/uart.c b/drivers/staging/greybus/uart.c
index 55c51143bb09..84de56800a21 100644
--- a/drivers/staging/greybus/uart.c
+++ b/drivers/staging/greybus/uart.c
@@ -40,14 +40,6 @@
#define GB_UART_FIRMWARE_CREDITS 4096
#define GB_UART_CREDIT_WAIT_TIMEOUT_MSEC 10000
-struct gb_tty_line_coding {
- __le32 rate;
- __u8 format;
- __u8 parity;
- __u8 data_bits;
- __u8 flow_control;
-};
-
struct gb_tty {
struct gbphy_device *gbphy_dev;
struct tty_port port;
@@ -66,7 +58,7 @@ struct gb_tty {
struct mutex mutex;
u8 ctrlin; /* input control lines */
u8 ctrlout; /* output control lines */
- struct gb_tty_line_coding line_coding;
+ struct gb_uart_set_line_coding_request line_coding;
struct work_struct tx_work;
struct kfifo write_fifo;
bool close_pending;
@@ -288,12 +280,9 @@ static void gb_uart_tx_write_work(struct work_struct *work)
static int send_line_coding(struct gb_tty *tty)
{
- struct gb_uart_set_line_coding_request request;
-
- memcpy(&request, &tty->line_coding,
- sizeof(tty->line_coding));
return gb_operation_sync(tty->connection, GB_UART_TYPE_SET_LINE_CODING,
- &request, sizeof(request), NULL, 0);
+ &tty->line_coding, sizeof(tty->line_coding),
+ NULL, 0);
}
static int send_control(struct gb_tty *gb_tty, u8 control)
@@ -493,9 +482,9 @@ static int gb_tty_break_ctl(struct tty_struct *tty, int state)
static void gb_tty_set_termios(struct tty_struct *tty,
struct ktermios *termios_old)
{
+ struct gb_uart_set_line_coding_request newline;
struct gb_tty *gb_tty = tty->driver_data;
struct ktermios *termios = &tty->termios;
- struct gb_tty_line_coding newline;
u8 newctrl = gb_tty->ctrlout;
newline.rate = cpu_to_le32(tty_get_baud_rate(tty));
--
2.26.2
In the "gb_tty_set_termios" function the "newline" variable is declared
but not initialized. So the "flow_control" member is not initialized and
the OR / AND operations with itself results in an undefined value in
this member.
The purpose of the code is to set the flow control type, so remove the
OR / AND self operator and set the value directly.
Addresses-Coverity-ID: 1374016 ("Uninitialized scalar variable")
Fixes: e55c25206d5c9 ("greybus: uart: Handle CRTSCTS flag in termios")
Signed-off-by: Oscar Carter <oscar.carter(a)gmx.com>
---
drivers/staging/greybus/uart.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/greybus/uart.c b/drivers/staging/greybus/uart.c
index 55c51143bb09..4ffb334cd5cd 100644
--- a/drivers/staging/greybus/uart.c
+++ b/drivers/staging/greybus/uart.c
@@ -537,9 +537,9 @@ static void gb_tty_set_termios(struct tty_struct *tty,
}
if (C_CRTSCTS(tty) && C_BAUD(tty) != B0)
- newline.flow_control |= GB_SERIAL_AUTO_RTSCTS_EN;
+ newline.flow_control = GB_SERIAL_AUTO_RTSCTS_EN;
else
- newline.flow_control &= ~GB_SERIAL_AUTO_RTSCTS_EN;
+ newline.flow_control = 0;
if (memcmp(&gb_tty->line_coding, &newline, sizeof(newline))) {
memcpy(&gb_tty->line_coding, &newline, sizeof(newline));
--
2.20.1
On Wed, Apr 08, 2020 at 10:28:10AM +0200, zenyu(a)tuta.io wrote:
> From: Zenyu Sy <zenyu(a)tuta.io>
>
> Fix the article typo "an" -> "a" in Kconfig
>
> Signed-off-by: Zenyu Sy <zenyu(a)tuta.io>
> ---
> base-commit: f5e94d10e4c468357019e5c28d48499f677b284f
> drivers/greybus/Kconfig | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/greybus/Kconfig b/drivers/greybus/Kconfig
> index b84fcaf8b..56cbd87fc 100644
> --- a/drivers/greybus/Kconfig
> +++ b/drivers/greybus/Kconfig
> @@ -3,7 +3,7 @@ menuconfig GREYBUS
> tristate "Greybus support"
> depends on SYSFS
> ---help---
> - This option enables the Greybus driver core. Greybus is an
> + This option enables the Greybus driver core. Greybus is a
> hardware protocol that was designed to provide Unipro with a
> sane application layer. It was originally designed for the
> ARA project, a module phone system, but has shown up in other
> --
> 2.26.0
You can't send patches in html format, please use git send-email to do
this properly.
thanks,
greg k-h
From: Zenyu Sy <zenyu(a)tuta.io>
Fix typos ("an" -> "a", "chose" -> "choose") in Kconfig
Signed-off-by: Zenyu Sy <zenyu(a)tuta.io>
---
base-commit: f5e94d10e4c468357019e5c28d48499f677b284f
Sorry, I just submitted a wrong patch several minutes ago,
so please ignore that one.
drivers/greybus/Kconfig | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/greybus/Kconfig b/drivers/greybus/Kconfig
index b84fcaf8b..aeea082f1 100644
--- a/drivers/greybus/Kconfig
+++ b/drivers/greybus/Kconfig
@@ -3,7 +3,7 @@ menuconfig GREYBUS
tristate "Greybus support"
depends on SYSFS
---help---
- This option enables the Greybus driver core. Greybus is an
+ This option enables the Greybus driver core. Greybus is a
hardware protocol that was designed to provide Unipro with a
sane application layer. It was originally designed for the
ARA project, a module phone system, but has shown up in other
@@ -12,7 +12,7 @@ menuconfig GREYBUS
Say Y here to enable support for these types of drivers.
- To compile this code as a module, chose M here: the module
+ To compile this code as a module, choose M here: the module
will be called greybus.ko
if GREYBUS
@@ -25,7 +25,7 @@ config GREYBUS_ES2
acts as a Greybus "host controller". This device is a bridge
from a USB device to a Unipro network.
- To compile this code as a module, chose M here: the module
+ To compile this code as a module, choose M here: the module
will be called gb-es2.ko
endif # GREYBUS
--
2.26.0
Using a fixed 1s timeout for all commands is a bit problematic.
For some commands it means waiting longer than needed for the timeout to
expire, which may not a big issue, but still. For other commands, like for
an erase (CMD38) that uses a R1B response, may require longer timeouts than
1s. In these cases, we may end up treating the command as it failed, while
it just needed some more time to complete successfully.
Fix the problem by respecting the cmd->busy_timeout, which is provided by
the mmc core.
Cc: Rui Miguel Silva <rmfrfs(a)gmail.com>
Cc: Johan Hovold <johan(a)kernel.org>
Cc: Alex Elder <elder(a)kernel.org>
Cc: Greg Kroah-Hartman <gregkh(a)linuxfoundation.org>
Cc: greybus-dev(a)lists.linaro.org
Signed-off-by: Ulf Hansson <ulf.hansson(a)linaro.org>
---
drivers/staging/greybus/sdio.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/greybus/sdio.c b/drivers/staging/greybus/sdio.c
index 68c5718be827..c4b16bb5c1a4 100644
--- a/drivers/staging/greybus/sdio.c
+++ b/drivers/staging/greybus/sdio.c
@@ -411,6 +411,7 @@ static int gb_sdio_command(struct gb_sdio_host *host, struct mmc_command *cmd)
struct gb_sdio_command_request request = {0};
struct gb_sdio_command_response response;
struct mmc_data *data = host->mrq->data;
+ unsigned int timeout_ms;
u8 cmd_flags;
u8 cmd_type;
int i;
@@ -469,9 +470,12 @@ static int gb_sdio_command(struct gb_sdio_host *host, struct mmc_command *cmd)
request.data_blksz = cpu_to_le16(data->blksz);
}
- ret = gb_operation_sync(host->connection, GB_SDIO_TYPE_COMMAND,
- &request, sizeof(request), &response,
- sizeof(response));
+ timeout_ms = cmd->busy_timeout ? cmd->busy_timeout :
+ GB_OPERATION_TIMEOUT_DEFAULT;
+
+ ret = gb_operation_sync_timeout(host->connection, GB_SDIO_TYPE_COMMAND,
+ &request, sizeof(request), &response,
+ sizeof(response), timeout_ms);
if (ret < 0)
goto out;
--
2.20.1
This patch fixes the checkpatch.pl warning:
WARNING: braces {} are not necessary for single statement blocks
Per Alex Elder's <elder(a)ieee.org> suggestion, noting that this is the only
instance of the problem noted by checkpatch.pl in staging: greybus.
Signed-off-by: Dan Jessie <dtjessie(a)gmail.com>
---
drivers/staging/greybus/hid.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/staging/greybus/hid.c b/drivers/staging/greybus/hid.c
index 04bfd9110502..ed706f39e87a 100644
--- a/drivers/staging/greybus/hid.c
+++ b/drivers/staging/greybus/hid.c
@@ -290,9 +290,8 @@ static int gb_hid_parse(struct hid_device *hid)
}
rdesc = kzalloc(rsize, GFP_KERNEL);
- if (!rdesc) {
+ if (!rdesc)
return -ENOMEM;
- }
ret = gb_hid_get_report_desc(ghid, rdesc);
if (ret) {
--
2.25.1