2023-08-14 06:19:58

by Reka Norman

[permalink] [raw]
Subject: [PATCH 5/9] media: cros-ec-cec: Support multiple ports in MKBP cec_events

Use the top four bits of the cec_events MKBP event to store the port
number.

Signed-off-by: Reka Norman <[email protected]>
---

drivers/media/cec/platform/cros-ec/cros-ec-cec.c | 13 +++++++++++--
include/linux/platform_data/cros_ec_commands.h | 10 ++++++++++
2 files changed, 21 insertions(+), 2 deletions(-)

diff --git a/drivers/media/cec/platform/cros-ec/cros-ec-cec.c b/drivers/media/cec/platform/cros-ec/cros-ec-cec.c
index d674a432dfdd..eb4b778f83e9 100644
--- a/drivers/media/cec/platform/cros-ec/cros-ec-cec.c
+++ b/drivers/media/cec/platform/cros-ec/cros-ec-cec.c
@@ -77,8 +77,17 @@ static void handle_cec_message(struct cros_ec_cec *cros_ec_cec)
static void handle_cec_event(struct cros_ec_cec *cros_ec_cec)
{
struct cros_ec_device *cros_ec = cros_ec_cec->cros_ec;
- uint32_t events = cros_ec->event_data.data.cec_events;
- struct cros_ec_cec_port *port = cros_ec_cec->ports[CEC_PORT];
+ uint32_t cec_events = cros_ec->event_data.data.cec_events;
+ int port_num = EC_MKBP_EVENT_CEC_GET_PORT(cec_events);
+ uint32_t events = EC_MKBP_EVENT_CEC_GET_EVENTS(cec_events);
+ struct cros_ec_cec_port *port;
+
+ if (port_num < 0 || port_num >= cros_ec_cec->num_ports) {
+ dev_err(cros_ec->dev,
+ "received CEC event for invalid port %d\n", port_num);
+ return;
+ }
+ port = cros_ec_cec->ports[port_num];

if (events & EC_MKBP_CEC_SEND_OK)
cec_transmit_attempt_done(port->adap, CEC_TX_STATUS_OK);
diff --git a/include/linux/platform_data/cros_ec_commands.h b/include/linux/platform_data/cros_ec_commands.h
index 9a0c6e28f370..b7e8573a8a49 100644
--- a/include/linux/platform_data/cros_ec_commands.h
+++ b/include/linux/platform_data/cros_ec_commands.h
@@ -4440,6 +4440,16 @@ struct ec_response_i2c_passthru_protect {

#define MAX_CEC_MSG_LEN 16

+/*
+ * Helper macros for packing/unpacking cec_events.
+ * bits[27:0] : bitmask of events from enum mkbp_cec_event
+ * bits[31:28]: port number
+ */
+#define EC_MKBP_EVENT_CEC_PACK(events, port) \
+ (((events) & GENMASK(27, 0)) | (((port) & 0xf) << 28))
+#define EC_MKBP_EVENT_CEC_GET_EVENTS(event) ((event) & GENMASK(27, 0))
+#define EC_MKBP_EVENT_CEC_GET_PORT(event) (((event) >> 28) & 0xf)
+
/* CEC message from the AP to be written on the CEC bus */
#define EC_CMD_CEC_WRITE_MSG 0x00B8

--
2.41.0.640.ga95def55d0-goog



2023-08-21 13:52:29

by Hans Verkuil

[permalink] [raw]
Subject: Re: [PATCH 5/9] media: cros-ec-cec: Support multiple ports in MKBP cec_events

Hi Reka,

On 14/08/2023 06:29, Reka Norman wrote:
> Use the top four bits of the cec_events MKBP event to store the port
> number.
>
> Signed-off-by: Reka Norman <[email protected]>
> ---
>
> drivers/media/cec/platform/cros-ec/cros-ec-cec.c | 13 +++++++++++--
> include/linux/platform_data/cros_ec_commands.h | 10 ++++++++++
> 2 files changed, 21 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/media/cec/platform/cros-ec/cros-ec-cec.c b/drivers/media/cec/platform/cros-ec/cros-ec-cec.c
> index d674a432dfdd..eb4b778f83e9 100644
> --- a/drivers/media/cec/platform/cros-ec/cros-ec-cec.c
> +++ b/drivers/media/cec/platform/cros-ec/cros-ec-cec.c
> @@ -77,8 +77,17 @@ static void handle_cec_message(struct cros_ec_cec *cros_ec_cec)
> static void handle_cec_event(struct cros_ec_cec *cros_ec_cec)
> {
> struct cros_ec_device *cros_ec = cros_ec_cec->cros_ec;
> - uint32_t events = cros_ec->event_data.data.cec_events;
> - struct cros_ec_cec_port *port = cros_ec_cec->ports[CEC_PORT];
> + uint32_t cec_events = cros_ec->event_data.data.cec_events;
> + int port_num = EC_MKBP_EVENT_CEC_GET_PORT(cec_events);
> + uint32_t events = EC_MKBP_EVENT_CEC_GET_EVENTS(cec_events);
> + struct cros_ec_cec_port *port;
> +
> + if (port_num < 0 || port_num >= cros_ec_cec->num_ports) {

Since cec_events is unsigned, then I would also keep port_num unsigned.
Mixing signed and unsigned for bit shifting is a bit dangerous.

That will also mean that you can skip the 'port_num < 0' check.

Regards,

Hans

> + dev_err(cros_ec->dev,
> + "received CEC event for invalid port %d\n", port_num);
> + return;
> + }
> + port = cros_ec_cec->ports[port_num];
>
> if (events & EC_MKBP_CEC_SEND_OK)
> cec_transmit_attempt_done(port->adap, CEC_TX_STATUS_OK);
> diff --git a/include/linux/platform_data/cros_ec_commands.h b/include/linux/platform_data/cros_ec_commands.h
> index 9a0c6e28f370..b7e8573a8a49 100644
> --- a/include/linux/platform_data/cros_ec_commands.h
> +++ b/include/linux/platform_data/cros_ec_commands.h
> @@ -4440,6 +4440,16 @@ struct ec_response_i2c_passthru_protect {
>
> #define MAX_CEC_MSG_LEN 16
>
> +/*
> + * Helper macros for packing/unpacking cec_events.
> + * bits[27:0] : bitmask of events from enum mkbp_cec_event
> + * bits[31:28]: port number
> + */
> +#define EC_MKBP_EVENT_CEC_PACK(events, port) \
> + (((events) & GENMASK(27, 0)) | (((port) & 0xf) << 28))
> +#define EC_MKBP_EVENT_CEC_GET_EVENTS(event) ((event) & GENMASK(27, 0))
> +#define EC_MKBP_EVENT_CEC_GET_PORT(event) (((event) >> 28) & 0xf)
> +
> /* CEC message from the AP to be written on the CEC bus */
> #define EC_CMD_CEC_WRITE_MSG 0x00B8
>