linux-kernel - [PATCH v5 07/15] drm/bridge: tc358767: Simplify AUX data write

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20190612083252.15321-8-andrew.smirnov@gmail.com>
Date:   Wed, 12 Jun 2019 01:32:44 -0700
From:   Andrey Smirnov <andrew.smirnov@...il.com>
To:     dri-devel@...ts.freedesktop.org
Cc:     Andrey Smirnov <andrew.smirnov@...il.com>,
        Andrzej Hajda <a.hajda@...sung.com>,
        Laurent Pinchart <Laurent.pinchart@...asonboard.com>,
        Tomi Valkeinen <tomi.valkeinen@...com>,
        Andrey Gusakov <andrey.gusakov@...entembedded.com>,
        Philipp Zabel <p.zabel@...gutronix.de>,
        Cory Tusar <cory.tusar@....aero>,
        Chris Healy <cphealy@...il.com>,
        Lucas Stach <l.stach@...gutronix.de>,
        linux-kernel@...r.kernel.org
Subject: [PATCH v5 07/15] drm/bridge: tc358767: Simplify AUX data write

Simplify AUX data write by dropping index arithmetic and shifting and
replacing it with a call to a helper function that does two things:

    1. Copies user-provided data into a write buffer
    2. Transfers contents of the write buffer to up to 4 32-bit
       registers on the chip

Note that separate data endianness fix:

    tmp = (tmp << 8) | buf[i];

that was reserved for DP_AUX_I2C_WRITE looks really strange, since it
will place data differently depending on the passed user-data
size. E.g. for a write of 1 byte, data transferred to the chip would
look like:

[byte0] [dummy1] [dummy2] [dummy3]

whereas for a write of 4 bytes we'd get:

[byte3] [byte2] [byte1] [byte0]

Since there's no indication in the datasheet that I2C write buffer
should be treated differently than AUX write buffer and no comment in
the original code explaining why it was done this way, that special
I2C write buffer transformation was dropped in this patch.

Signed-off-by: Andrey Smirnov <andrew.smirnov@...il.com>
Reviewed-by: Andrzej Hajda <a.hajda@...sung.com>
Cc: Andrzej Hajda <a.hajda@...sung.com>
Cc: Laurent Pinchart <Laurent.pinchart@...asonboard.com>
Cc: Tomi Valkeinen <tomi.valkeinen@...com>
Cc: Andrey Gusakov <andrey.gusakov@...entembedded.com>
Cc: Philipp Zabel <p.zabel@...gutronix.de>
Cc: Cory Tusar <cory.tusar@....aero>
Cc: Chris Healy <cphealy@...il.com>
Cc: Lucas Stach <l.stach@...gutronix.de>
Cc: dri-devel@...ts.freedesktop.org
Cc: linux-kernel@...r.kernel.org
---
 drivers/gpu/drm/bridge/tc358767.c | 48 +++++++++++++++++--------------
 1 file changed, 26 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/bridge/tc358767.c b/drivers/gpu/drm/bridge/tc358767.c
index 7152b44db8a3..e60692b8cd69 100644
--- a/drivers/gpu/drm/bridge/tc358767.c
+++ b/drivers/gpu/drm/bridge/tc358767.c
@@ -321,6 +321,21 @@ static int tc_aux_get_status(struct tc_data *tc, u8 *reply)
 	return 0;
 }
 
+static int tc_aux_write_data(struct tc_data *tc, const void *data,
+			     size_t size)
+{
+	u32 auxwdata[DP_AUX_MAX_PAYLOAD_BYTES / sizeof(u32)] = { 0 };
+	int ret, count = ALIGN(size, sizeof(u32));
+
+	memcpy(auxwdata, data, size);
+
+	ret = regmap_raw_write(tc->regmap, DP0_AUXWDATA(0), auxwdata, count);
+	if (ret)
+		return ret;
+
+	return size;
+}
+
 static int tc_aux_read_data(struct tc_data *tc, void *data, size_t size)
 {
 	u32 auxrdata[DP_AUX_MAX_PAYLOAD_BYTES / sizeof(u32)];
@@ -341,9 +356,6 @@ static ssize_t tc_aux_transfer(struct drm_dp_aux *aux,
 	struct tc_data *tc = aux_to_tc(aux);
 	size_t size = min_t(size_t, 8, msg->size);
 	u8 request = msg->request & ~DP_AUX_I2C_MOT;
-	u8 *buf = msg->buffer;
-	u32 tmp = 0;
-	int i = 0;
 	int ret;
 
 	if (size == 0)
@@ -353,25 +365,17 @@ static ssize_t tc_aux_transfer(struct drm_dp_aux *aux,
 	if (ret)
 		return ret;
 
-	if (request == DP_AUX_I2C_WRITE || request == DP_AUX_NATIVE_WRITE) {
-		/* Store data */
-		while (i < size) {
-			if (request == DP_AUX_NATIVE_WRITE)
-				tmp = tmp | (buf[i] << (8 * (i & 0x3)));
-			else
-				tmp = (tmp << 8) | buf[i];
-			i++;
-			if (((i % 4) == 0) || (i == size)) {
-				ret = regmap_write(tc->regmap,
-						   DP0_AUXWDATA((i - 1) >> 2),
-						   tmp);
-				if (ret)
-					return ret;
-				tmp = 0;
-			}
-		}
-	} else if (request != DP_AUX_I2C_READ &&
-		   request != DP_AUX_NATIVE_READ) {
+	switch (request) {
+	case DP_AUX_NATIVE_READ:
+	case DP_AUX_I2C_READ:
+		break;
+	case DP_AUX_NATIVE_WRITE:
+	case DP_AUX_I2C_WRITE:
+		ret = tc_aux_write_data(tc, msg->buffer, size);
+		if (ret < 0)
+			return ret;
+		break;
+	default:
 		return -EINVAL;
 	}
 
-- 
2.21.0