On 08/12/2016 04:15 PM, Russell King wrote: > 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 <rmk+kernel at armlinux.org.uk> > --- > 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> > +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. > + else > + addresses = priv->addresses |= BIT(addr); > + > + /* TDA9950 doesn't want address 15 set */ > + addr &= 0x7fff; > + buf[0] = addresses >> 8; > + buf[1] = addresses; > + > + return tda9950_write_range(priv->client, REG_ACKH, buf, 2); > +} > + <snip> > +static int tda9950_cec_adap_enable(struct cec_adapter *adap, bool enable) > +{ > + struct tda9950_priv *priv = adap->priv; > + > + if (!enable) { > + tda9950_release(priv); > + return 0; > + } else { Nitpick: no need for 'else' here since the 'if' always returns. > + return tda9950_open(priv); > + } > +} Regards, Hans