Re: [PATCH RFC 5/5] drm/i2c: add tda998x/tda9950 CEC driver

On Fri, Aug 12, 2016 at 04:38:04PM +0200, Hans Verkuil wrote:

On 08/12/2016 04:15 PM, Russell King wrote:

quoted

Add a CEC driver for the TDA9950, which is a stand-alone I2C CEC device.
The TDA9950 contains a command processor which handles retransmissions
and the low level bus protocol.  The driver just has to read and write
the messages, and handle error conditions.

Signed-off-by: Russell King <redacted>
---
drivers/gpu/drm/i2c/Kconfig           |   5 +
drivers/gpu/drm/i2c/Makefile          |   1 +
drivers/gpu/drm/i2c/tda9950.c         | 514 ++++++++++++++++++++++++++++++++++
include/linux/platform_data/tda9950.h |  15 +
4 files changed, 535 insertions(+)
create mode 100644 drivers/gpu/drm/i2c/tda9950.c
create mode 100644 include/linux/platform_data/tda9950.h

<snip>

quoted

+static int tda9950_cec_adap_log_addr(struct cec_adapter *adap, u8 addr)
+{
+	struct tda9950_priv *priv = adap->priv;
+	u16 addresses;
+	u8 buf[2];
+
+	if (addr = CEC_LOG_ADDR_INVALID)
+		addresses = priv->addresses = BIT(15);

I saw this in patch 4/5 as well: why set bit 15? I would expect that this
is just set to 0. And priv->addresses doesn't seem to be used anywhere.

It's needed to be set so that the hardware receives messages sent
to the broadcast (15) address.

quoted

+	else
+		addresses = priv->addresses |= BIT(addr);

For the second point, read this line, paying close attention to the |=.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.