From f47c1b023ee55737ab5f6b6f479cab06a5a03ba2 Mon Sep 17 00:00:00 2001 From: Viresh Kumar Date: Wed, 29 Jul 2015 11:42:53 +0530 Subject: [PATCH 1/7] greybus: operation: Drop alignment attribute from operation message header The buffers allocated for message header is already 64 bit aligned and we have explicit pad bytes in the header structure, to 64 bit align the operation specific data. And so there is no need to add the aligned attribute to the operation message header. Drop it. Suggested-by: Johan Hovold Signed-off-by: Viresh Kumar Reviewed-by: Alex Elder Reviewed-by: Johan Hovold Signed-off-by: Greg Kroah-Hartman --- drivers/staging/greybus/operation.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/staging/greybus/operation.h b/drivers/staging/greybus/operation.h index 00189e963a01..1b7cc6a8cc26 100644 --- a/drivers/staging/greybus/operation.h +++ b/drivers/staging/greybus/operation.h @@ -62,7 +62,7 @@ enum gb_operation_result { * * The wire format for all numeric fields in the header is little * endian. Any operation-specific data begins immediately after the - * header, and is 64-bit aligned. + * header. */ struct gb_operation_msg_hdr { __le16 size; /* Size in bytes of header + payload */ @@ -70,7 +70,7 @@ struct gb_operation_msg_hdr { __u8 type; /* E.g GB_I2C_TYPE_* or GB_GPIO_TYPE_* */ __u8 result; /* Result of request (in responses only) */ __u8 pad[2]; /* must be zero (ignore when read) */ -} __aligned(sizeof(u64)); +}; #define GB_OPERATION_MESSAGE_SIZE_MIN sizeof(struct gb_operation_msg_hdr) #define GB_OPERATION_MESSAGE_SIZE_MAX U16_MAX From e34fae58f6e004baf26ddac937ac13bd53b7c5f2 Mon Sep 17 00:00:00 2001 From: Viresh Kumar Date: Wed, 29 Jul 2015 11:42:54 +0530 Subject: [PATCH 2/7] greybus: operation: Move operation header to greybus_protocols.h This should be exposed to external users (like gbsim). Move it to greybus_protocols.h. Signed-off-by: Viresh Kumar Reviewed-by: Johan Hovold Reviewed-by: Alex Elder Signed-off-by: Greg Kroah-Hartman --- drivers/staging/greybus/greybus_protocols.h | 31 ++++++++++++++++++++- drivers/staging/greybus/operation.h | 29 ------------------- 2 files changed, 30 insertions(+), 30 deletions(-) diff --git a/drivers/staging/greybus/greybus_protocols.h b/drivers/staging/greybus/greybus_protocols.h index 61fe9dce6ec0..9192ee72c6b8 100644 --- a/drivers/staging/greybus/greybus_protocols.h +++ b/drivers/staging/greybus/greybus_protocols.h @@ -69,9 +69,38 @@ #define GB_CONTROL_CPORT_ID 2 -/* Control Protocol */ +/* + * All operation messages (both requests and responses) begin with + * a header that encodes the size of the message (header included). + * This header also contains a unique identifier, that associates a + * response message with its operation. The header contains an + * operation type field, whose interpretation is dependent on what + * type of protocol is used over the connection. The high bit + * (0x80) of the operation type field is used to indicate whether + * the message is a request (clear) or a response (set). + * + * Response messages include an additional result byte, which + * communicates the result of the corresponding request. A zero + * result value means the operation completed successfully. Any + * other value indicates an error; in this case, the payload of the + * response message (if any) is ignored. The result byte must be + * zero in the header for a request message. + * + * The wire format for all numeric fields in the header is little + * endian. Any operation-specific data begins immediately after the + * header. + */ +struct gb_operation_msg_hdr { + __le16 size; /* Size in bytes of header + payload */ + __le16 operation_id; /* Operation unique id */ + __u8 type; /* E.g GB_I2C_TYPE_* or GB_GPIO_TYPE_* */ + __u8 result; /* Result of request (in responses only) */ + __u8 pad[2]; /* must be zero (ignore when read) */ +}; +/* Control Protocol */ + /* version request has no payload */ struct gb_protocol_version_response { __u8 major; diff --git a/drivers/staging/greybus/operation.h b/drivers/staging/greybus/operation.h index 1b7cc6a8cc26..16488d016943 100644 --- a/drivers/staging/greybus/operation.h +++ b/drivers/staging/greybus/operation.h @@ -43,35 +43,6 @@ enum gb_operation_result { GB_OP_MALFUNCTION = 0xff, }; -/* - * All operation messages (both requests and responses) begin with - * a header that encodes the size of the message (header included). - * This header also contains a unique identifier, that associates a - * response message with its operation. The header contains an - * operation type field, whose interpretation is dependent on what - * type of protocol is used over the connection. The high bit - * (0x80) of the operation type field is used to indicate whether - * the message is a request (clear) or a response (set). - * - * Response messages include an additional result byte, which - * communicates the result of the corresponding request. A zero - * result value means the operation completed successfully. Any - * other value indicates an error; in this case, the payload of the - * response message (if any) is ignored. The result byte must be - * zero in the header for a request message. - * - * The wire format for all numeric fields in the header is little - * endian. Any operation-specific data begins immediately after the - * header. - */ -struct gb_operation_msg_hdr { - __le16 size; /* Size in bytes of header + payload */ - __le16 operation_id; /* Operation unique id */ - __u8 type; /* E.g GB_I2C_TYPE_* or GB_GPIO_TYPE_* */ - __u8 result; /* Result of request (in responses only) */ - __u8 pad[2]; /* must be zero (ignore when read) */ -}; - #define GB_OPERATION_MESSAGE_SIZE_MIN sizeof(struct gb_operation_msg_hdr) #define GB_OPERATION_MESSAGE_SIZE_MAX U16_MAX From 46d26c5d57daaa39a5f396b13544cdb4ec15d284 Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Sat, 1 Aug 2015 11:50:37 +0200 Subject: [PATCH 3/7] greybus: greybus_protocols: fix typo in comment Fix misspelled variable name in comment. Reviewed-by: Alex Elder Signed-off-by: Johan Hovold Signed-off-by: Greg Kroah-Hartman --- drivers/staging/greybus/greybus_protocols.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/greybus/greybus_protocols.h b/drivers/staging/greybus/greybus_protocols.h index 9192ee72c6b8..0441de5becd6 100644 --- a/drivers/staging/greybus/greybus_protocols.h +++ b/drivers/staging/greybus/greybus_protocols.h @@ -571,7 +571,7 @@ struct gb_spi_transfer_request { __u8 chip_select; /* of the spi device */ __u8 mode; /* of the spi device */ __le16 count; - struct gb_spi_transfer transfers[0]; /* trnasfer_count of these */ + struct gb_spi_transfer transfers[0]; /* count of these */ }; struct gb_spi_transfer_response { From 2130f09871a70ce425b8d328a641b7a1935ca831 Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Sat, 1 Aug 2015 11:50:38 +0200 Subject: [PATCH 4/7] greybus: svc: fix message packing Add missing packed attributes to prevent implicit structure padding. Signed-off-by: Johan Hovold Reviewed-by: Alex Elder Reviewed-by: Viresh Kumar Signed-off-by: Greg Kroah-Hartman --- drivers/staging/greybus/greybus_protocols.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/staging/greybus/greybus_protocols.h b/drivers/staging/greybus/greybus_protocols.h index 0441de5becd6..121300a84e1c 100644 --- a/drivers/staging/greybus/greybus_protocols.h +++ b/drivers/staging/greybus/greybus_protocols.h @@ -599,7 +599,7 @@ struct gb_spi_transfer_response { struct gb_svc_hello_request { __le16 endo_id; __u8 interface_id; -}; +} __packed; /* hello response has no payload */ struct gb_svc_intf_device_id_request { @@ -616,7 +616,7 @@ struct gb_svc_intf_hotplug_request { __le32 ara_vend_id; __le32 ara_prod_id; } data; -}; +} __packed; /* hotplug response has no payload */ struct gb_svc_intf_hot_unplug_request { @@ -634,7 +634,7 @@ struct gb_svc_conn_create_request { __u16 cport1_id; __u8 intf2_id; __u16 cport2_id; -}; +} __packed; /* connection create response has no payload */ struct gb_svc_conn_destroy_request { @@ -642,7 +642,7 @@ struct gb_svc_conn_destroy_request { __u16 cport1_id; __u8 intf2_id; __u16 cport2_id; -}; +} __packed; /* connection destroy response has no payload */ /* UART */ From cfc5f2c6deb5493d5854036f3d0f396ea4a3e99d Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Sat, 1 Aug 2015 11:50:39 +0200 Subject: [PATCH 5/7] greybus: uart: fix message packing Add missing packed attributes to prevent implicit structure padding. Signed-off-by: Johan Hovold Reviewed-by: Alex Elder Reviewed-by: Viresh Kumar Signed-off-by: Greg Kroah-Hartman --- drivers/staging/greybus/greybus_protocols.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/staging/greybus/greybus_protocols.h b/drivers/staging/greybus/greybus_protocols.h index 121300a84e1c..cdd5b54874bd 100644 --- a/drivers/staging/greybus/greybus_protocols.h +++ b/drivers/staging/greybus/greybus_protocols.h @@ -678,7 +678,7 @@ struct gb_uart_recv_data_request { __le16 size; __u8 flags; __u8 data[0]; -}; +} __packed; struct gb_uart_set_line_coding_request { __le32 rate; @@ -695,7 +695,7 @@ struct gb_uart_set_line_coding_request { #define GB_SERIAL_SPACE_PARITY 4 __u8 data_bits; -}; +} __packed; /* output control lines */ #define GB_UART_CTRL_DTR 0x01 From 8bbd9edcb71cee96fa7a835f3e445841b8c48fd7 Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Sat, 1 Aug 2015 11:50:40 +0200 Subject: [PATCH 6/7] greybus: sdio: fix message packing Add missing packed attributes to prevent implicit structure padding. Signed-off-by: Johan Hovold Reviewed-by: Alex Elder Reviewed-by: Viresh Kumar Signed-off-by: Greg Kroah-Hartman --- drivers/staging/greybus/greybus_protocols.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/staging/greybus/greybus_protocols.h b/drivers/staging/greybus/greybus_protocols.h index cdd5b54874bd..e49376d283f0 100644 --- a/drivers/staging/greybus/greybus_protocols.h +++ b/drivers/staging/greybus/greybus_protocols.h @@ -845,7 +845,7 @@ struct gb_sdio_set_ios_request { #define GB_SDIO_SET_DRIVER_TYPE_A 0x01 #define GB_SDIO_SET_DRIVER_TYPE_C 0x02 #define GB_SDIO_SET_DRIVER_TYPE_D 0x03 -}; +} __packed; /* command request */ struct gb_sdio_command_request { @@ -865,7 +865,7 @@ struct gb_sdio_command_request { #define GB_SDIO_CMD_BC 0x03 __le32 cmd_arg; -}; +} __packed; struct gb_sdio_command_response { __le32 resp[4]; @@ -881,7 +881,7 @@ struct gb_sdio_transfer_request { __le16 data_blocks; __le16 data_blksz; __u8 data[0]; -}; +} __packed; struct gb_sdio_transfer_response { __le16 data_blocks; From 5b559e64578783b944691f6f9e7c19812d2fced6 Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Sat, 1 Aug 2015 11:50:41 +0200 Subject: [PATCH 7/7] greybus: greybus_protocols: use only type attributes for message packing For consistency reasons, use only type attributes for message packing. Signed-off-by: Johan Hovold Reviewed-by: Alex Elder Reviewed-by: Viresh Kumar Signed-off-by: Greg Kroah-Hartman --- drivers/staging/greybus/greybus_protocols.h | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/staging/greybus/greybus_protocols.h b/drivers/staging/greybus/greybus_protocols.h index e49376d283f0..290b85f6ff68 100644 --- a/drivers/staging/greybus/greybus_protocols.h +++ b/drivers/staging/greybus/greybus_protocols.h @@ -277,8 +277,8 @@ struct gb_gpio_set_value_request { struct gb_gpio_set_debounce_request { __u8 which; - __le16 usec __packed; -}; + __le16 usec; +} __packed; /* debounce response has no payload */ struct gb_gpio_irq_type_request { @@ -336,9 +336,9 @@ struct gb_pwm_deactivate_request { struct gb_pwm_config_request { __u8 which; - __le32 duty __packed; - __le32 period __packed; -}; + __le32 duty; + __le32 period; +} __packed; struct gb_pwm_polarity_request { __u8 which;