Received: by 2002:ac0:a582:0:0:0:0:0 with SMTP id m2-v6csp3540699imm; Mon, 8 Oct 2018 05:50:27 -0700 (PDT) X-Google-Smtp-Source: ACcGV62ewB+PfMkRMLJLJkATjK458vgq3U6s8mfuFqB/aKNSJviW6RP145NYmtxNthmxFEfYuGkH X-Received: by 2002:a62:9850:: with SMTP id q77-v6mr24615414pfd.249.1539003027761; Mon, 08 Oct 2018 05:50:27 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1539003027; cv=none; d=google.com; s=arc-20160816; b=T18bu2xQsUgunIBwM7qy+Qc86KiApUg5JyxtNpxXWENfTDV27jPbub29QXcwuQf1v8 MXMeZIkOouC1DTsDen3NG/Auop+TuRWW/f0WBAfFD4BV7wHSOFs26XtuuQa7DoZCmJIN z5CzxmRzcF8KKv8EJBOSI/QC4HKpO5rszjq1otjiK/BXHnUkT4oFnvl0qcsAV39xZQ1i lwvDAV5V+gW32AH1I3eT1mUZ3ln8Ggbh0vemwkitclve7Hz7y+tvJBEm4DlchAD3X5qI 8c6G0yM75sbUS8K9kdkFhHlsY4UcVBd37S3dCzSD6D4M5LsyyvHzH3yKtsCkNNXY/kOl qS9g== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:user-agent:in-reply-to :content-disposition:mime-version:references:message-id:subject:cc :to:from:date:dkim-signature; bh=K/DSV1sAZ0e5EikcdUzHm7P3ynJU9FK0q0eixfdl4Z0=; b=hIMc91umItkhqHJXzxnFg1bDGAp3evkzmobDEd/KHB0jGjB7f/NtoHJvK+ImrkwZJf uJyHM6ND5Dcvims9zcnZ4pi7bbUXyq1fuApyeve91wKJ3ytXkc/lm6wjcl/p1hJi1uEh 21GNZKtO0v/TEGPzjD5aR9aa33vUqsQCtKQNltz0kxFh7afX24jKir1fU/35NzuJCHTt V/jdyj99dr9Vd/OX/VKJs3wfIEeTTvj6E8BIcnoThM825DhYG5M1AdVvy9/h/mt7gOX6 6Up00YRbUwNhlkZZ6mw5Wdav50u1JY9QZIM5NAGbeSgrqBcZ4aqB8PC5MTSGHTM2aQMX OHLQ== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@gmail.com header.s=20161025 header.b=pIcVmskC; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id 187-v6si20410950pfe.182.2018.10.08.05.50.12; Mon, 08 Oct 2018 05:50:27 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; dkim=pass header.i=@gmail.com header.s=20161025 header.b=pIcVmskC; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726142AbeJHUBj (ORCPT + 99 others); Mon, 8 Oct 2018 16:01:39 -0400 Received: from mail-wr1-f68.google.com ([209.85.221.68]:34518 "EHLO mail-wr1-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725914AbeJHUBi (ORCPT ); Mon, 8 Oct 2018 16:01:38 -0400 Received: by mail-wr1-f68.google.com with SMTP id z4-v6so20699680wrb.1; Mon, 08 Oct 2018 05:50:02 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=K/DSV1sAZ0e5EikcdUzHm7P3ynJU9FK0q0eixfdl4Z0=; b=pIcVmskCE6lEtELGuxPJLSfCam4AHz3oE9nzveGgkBoZ+VSgqdPUhbXABjMZ6omcoH DRLwN/zWZPM8NxQRAD52uOhGZUPXkvkzLagvtVNLzrU2LeyKm7rzeQc3SIYUd9wI5ZDR 1eoyZqgnF1oecA33QvZLiVmtk6cM8I5fiHinkK9s3cZ6inZ3DGxtUPjyKWr823UH6ifS JGxl+0YtyZeUS3j8y+ucxDHlkTIuzMj9kVQzsaa96+HxNfd4UW2C1pYm+IwdqwNh6f6Q 2HIYAKa5WNwQP5TW60CCuhC530mZuf1b2sk4rPhsSJFSvEC5z5hO5ci4iuvQvcTX2Bm3 z6dg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=K/DSV1sAZ0e5EikcdUzHm7P3ynJU9FK0q0eixfdl4Z0=; b=BDDMSNJvjPch7/dnhLtaQnvSJIrfDvzXB1/2rne7ViAWzn+mKMGADizJQWcg/Q38g3 xykImE+pkDSQUTo6hagmJSgdY+SyT/59Ln+Gp/ythcZkFAMj4+a3/Z0sy0Uy3zYbG61S sAe9pf+paMRMiOvOyp0c9wA/WpuLc3IcoMJKODVeJIUI+SpRj28ceeXpsEcR60niItlf KuM1vodU7xX+OCfeF9DJs0zZzQGL4qdYx1FYFMWJSNNmg3YW/CpXIUdy76ywSeDeXLr0 tGi24uFFNTcHH2Zslkfyfll9+vJv8+zWlLwczn2dR8N5CXKwgONAv2ECPchpI0wJPePN 34xw== X-Gm-Message-State: ABuFfoj629CIUijbpwJpvk/NNqwBrK4YET9cpUDUNjh1YDRbdHzOXZd0 v1wjuwUkD88whBjbaNraEVY= X-Received: by 2002:adf:b188:: with SMTP id q8-v6mr16485539wra.95.1539003001409; Mon, 08 Oct 2018 05:50:01 -0700 (PDT) Received: from Ettosoft-T55 (generic-nat1.unisi.it. [193.205.5.2]) by smtp.gmail.com with ESMTPSA id l195-v6sm11655315wmd.37.2018.10.08.05.49.59 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Mon, 08 Oct 2018 05:50:00 -0700 (PDT) Date: Mon, 8 Oct 2018 14:49:53 +0200 From: ektor5 To: jacopo mondi Cc: hverkuil@xs4all.nl, luca.pisani@udoo.org, jose.abreu@synopsys.com, sean@mess.org, sakari.ailus@linux.intel.com, Mauro Carvalho Chehab , Greg Kroah-Hartman , "David S. Miller" , Andrew Morton , Arnd Bergmann , Hans Verkuil , Laurent Pinchart , Geert Uytterhoeven , Jacob Chen , Benoit Parrot , Aaron Wu , Todor Tomov , Neil Armstrong , linux-kernel@vger.kernel.org, linux-media@vger.kernel.org Subject: Re: [PATCH v2 1/2] media: add SECO cec driver Message-ID: <20181008124951.gzdfoeuaecjpqm5b@Ettosoft-T55> References: <6ed0212a54febd725a44078e911b0878a272befb.1538760098.git.ek5.chimenti@gmail.com> <20181006134918.GU31281@w540> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20181006134918.GU31281@w540> User-Agent: NeoMutt/20180716-421-ff8684 Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Sat, Oct 06, 2018 at 03:49:18PM +0200, jacopo mondi wrote: > Hi Ettore, > some more comments below. > > On Fri, Oct 05, 2018 at 07:33:58PM +0200, ektor5 wrote: > > From: Ettore Chimenti > > > > This patch adds support to the CEC device implemented with a STM32 > > microcontroller in X86 SECO Boards, including UDOO X86. > > > > The communication is achieved via Braswell integrated SMBus > > (i2c-i801). The driver use direct access to the PCI addresses, due to > > the limitations of the specific driver in presence of ACPI calls. > > > > The basic functionalities are tested with success with cec-ctl and > > cec-compliance. > > > > Inspired by cros-ec-cec implementation, attaches to i915 driver > > cec-notifier. > > > > Signed-off-by: Ettore Chimenti > > Iterations of the same patchset are not usually sent 'in-reply-to' > mode as far as I've seen. Yes, sorry. Thought it was useul. :) > > > --- > > MAINTAINERS | 6 + > > drivers/media/platform/Kconfig | 12 + > > drivers/media/platform/Makefile | 2 + > > drivers/media/platform/seco-cec/Makefile | 1 + > > drivers/media/platform/seco-cec/seco-cec.c | 706 +++++++++++++++++++++ > > drivers/media/platform/seco-cec/seco-cec.h | 130 ++++ > > 6 files changed, 857 insertions(+) > > create mode 100644 drivers/media/platform/seco-cec/Makefile > > create mode 100644 drivers/media/platform/seco-cec/seco-cec.c > > create mode 100644 drivers/media/platform/seco-cec/seco-cec.h > > > > diff --git a/MAINTAINERS b/MAINTAINERS > > index 4ece30f15777..1062912a5ff4 100644 > > --- a/MAINTAINERS > > +++ b/MAINTAINERS > > @@ -12972,6 +12972,12 @@ L: sdricohcs-devel@lists.sourceforge.net (subscribers-only) > > S: Maintained > > F: drivers/mmc/host/sdricoh_cs.c > > > > +SECO BOARDS CEC DRIVER > > +M: Ettore Chimenti > > +S: Maintained > > +F: drivers/media/platform/seco-cec/seco-cec.c > > +F: drivers/media/platform/seco-cec/seco-cec.h > > + > > SECURE COMPUTING > > M: Kees Cook > > R: Andy Lutomirski > > diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig > > index 94c1fe0e9787..51cd1fd005e3 100644 > > --- a/drivers/media/platform/Kconfig > > +++ b/drivers/media/platform/Kconfig > > @@ -613,6 +613,18 @@ config VIDEO_TEGRA_HDMI_CEC > > The CEC bus is present in the HDMI connector and enables communication > > between compatible devices. > > > > +config VIDEO_SECO_CEC > > + tristate "SECO Boards HDMI CEC driver" > > + depends on (X86 || IA64) || COMPILE_TEST > > + depends on PCI && DMI > > + select CEC_CORE > > + select CEC_NOTIFIER > > + help > > + This is a driver for SECO Boards integrated CEC interface. > > + Selecting it will enable support for this device. > > + CEC bus is present in the HDMI connector and enables communication > > + between compatible devices. > > + > > endif #CEC_PLATFORM_DRIVERS > > > > menuconfig SDR_PLATFORM_DRIVERS > > diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile > > index 41322ab65802..5d2b06c4c68a 100644 > > --- a/drivers/media/platform/Makefile > > +++ b/drivers/media/platform/Makefile > > @@ -53,6 +53,8 @@ obj-$(CONFIG_VIDEO_TEGRA_HDMI_CEC) += tegra-cec/ > > > > obj-y += stm32/ > > > > +obj-$(CONFIG_VIDEO_SECO_CEC) += seco-cec/ > > + > > obj-y += davinci/ > > > > obj-$(CONFIG_VIDEO_SH_VOU) += sh_vou.o > > diff --git a/drivers/media/platform/seco-cec/Makefile b/drivers/media/platform/seco-cec/Makefile > > new file mode 100644 > > index 000000000000..09900b087d02 > > --- /dev/null > > +++ b/drivers/media/platform/seco-cec/Makefile > > @@ -0,0 +1 @@ > > +obj-y += seco-cec.o > > diff --git a/drivers/media/platform/seco-cec/seco-cec.c b/drivers/media/platform/seco-cec/seco-cec.c > > new file mode 100644 > > index 000000000000..990e88f979a2 > > --- /dev/null > > +++ b/drivers/media/platform/seco-cec/seco-cec.c > > @@ -0,0 +1,706 @@ > > +// SPDX-License-Identifier: GPL-2.0 OR BSD-3-Clause > > +/* > > + * CEC driver for SECO X86 Boards > > + * > > + * Author: Ettore Chimenti > > + * Copyright (C) 2018, SECO Srl. > > + * Copyright (C) 2018, Aidilab Srl. > > + */ > > + > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > + > > Please keep headers alphabetically sorted, it's easier to identify > duplications or unnecessary inclusions Yes, seems ok. > > > +/* CEC Framework */ > > +#include > > + > > +#include "seco-cec.h" > > + > > +struct secocec_data { > > + struct device *dev; > > + struct platform_device *pdev; > > + struct cec_adapter *cec_adap; > > + struct cec_notifier *notifier; > > + int irq; > > +}; > > + > > +#define smb_wr16(cmd, data) smb_word_op(CMD_WORD_DATA, SECOCEC_MICRO_ADDRESS, \ > > + cmd, data, SMBUS_WRITE, NULL) > > +#define smb_rd16(cmd, res) smb_word_op(CMD_WORD_DATA, SECOCEC_MICRO_ADDRESS, \ > > + cmd, 0, SMBUS_READ, res) > > + > > +static int smb_word_op(short data_format, u16 slave_addr, u8 cmd, u16 data, > > + u8 operation, u16 *result) > > +{ > > + unsigned int count; > > + short _data_format; > > + int status = 0; > > + > > + switch (data_format) { > > + case CMD_BYTE_DATA: > > + _data_format = BRA_SMB_CMD_BYTE_DATA; > > + break; > > + case CMD_WORD_DATA: > > + _data_format = BRA_SMB_CMD_WORD_DATA; > > + break; > > + default: > > + return -EINVAL; > > + } > > + > > + /* Active wait until ready */ > > + for (count = 0; count <= SMBTIMEOUT; ++count) { > > + if (!(inb(HSTS) & BRA_INUSE_STS)) > > + break; > > + udelay(SMB_POLL_UDELAY); > > + } > > + > > + if (count > SMBTIMEOUT) { > > + /* Reset the lock instead of failing */ > > + outb(0xff, HSTS); > > + pr_warn("%s: SMBTIMEOUT\n", __func__); > > Still a pr_ debug function here. No big deal, but you could easily > pass a reference to your device here. Up to you (and others to no > agree this is fine) I've left it by mistake, it doesn't provide any additional info. > > > + } > > + > > + outb(0x00, HCNT); > > + outb((u8)(slave_addr & 0xfe) | operation, XMIT_SLVA); > > + outb(cmd, HCMD); > > + inb(HCNT); > > + > > + if (operation == SMBUS_WRITE) { > > + outb((u8)data, HDAT0); > > + outb((u8)(data >> 8), HDAT1); > > + } > > + > > + outb(BRA_START + _data_format, HCNT); > > + > > + for (count = 0; count <= SMBTIMEOUT; count++) { > > + if (!(inb(HSTS) & BRA_HOST_BUSY)) > > + break; > > + udelay(SMB_POLL_UDELAY); > > + } > > + > > + if (count > SMBTIMEOUT) { > > + pr_debug("%s: SMBTIMEOUT_1\n", __func__); > > As you return an error this seems more like a pr_err (or dev_err if > possible) Same here. The error code speaks for itself. > > > + status = -EBUSY; > > + goto err; > > + } > > + > > + if (inb(HSTS) & BRA_HSTS_ERR_MASK) { > > + status = -EIO; > > + goto err; > > + } > > + > > + if (operation == SMBUS_READ) > > + *result = ((inb(HDAT0) & 0xff) + ((inb(HDAT1) & 0xff) << 8)); > > + > > +err: > > + outb(0xff, HSTS); > > + return status; > > +} > > + > > +static int secocec_adap_enable(struct cec_adapter *adap, bool enable) > > +{ > > + struct secocec_data *cec = cec_get_drvdata(adap); > > + struct device *dev = cec->dev; > > + u16 val = 0; > > + int status; > > + > > + if (enable) { > > + /* Clear the status register */ > > + status = smb_rd16(SECOCEC_STATUS_REG_1, &val); > > + if (status) > > + goto err; > > + > > + status = smb_wr16(SECOCEC_STATUS_REG_1, val); > > + if (status) > > + goto err; > > + > > + /* Enable the interrupts */ > > + status = smb_rd16(SECOCEC_ENABLE_REG_1, &val); > > + if (status) > > + goto err; > > + > > + status = smb_wr16(SECOCEC_ENABLE_REG_1, > > + val | SECOCEC_ENABLE_REG_1_CEC); > > + if (status) > > + goto err; > > + > > + dev_dbg(dev, "Device enabled"); > > + } else { > > + /* Clear the status register */ > > + status = smb_rd16(SECOCEC_STATUS_REG_1, &val); > > + if (status) > > + goto err; > > + > > + status = smb_wr16(SECOCEC_STATUS_REG_1, val); > > + if (status) > > + goto err; > > + > > + /* Disable the interrupts */ > > + status = smb_rd16(SECOCEC_ENABLE_REG_1, &val); > > + if (status) > > + goto err; > > + > > + status = smb_wr16(SECOCEC_ENABLE_REG_1, val & > > + ~SECOCEC_ENABLE_REG_1_CEC & > > + ~SECOCEC_ENABLE_REG_1_IR); > > + if (status) > > + goto err; > > + > > + dev_dbg(dev, "Device disabled"); > > + } > > + > > + return 0; > > +err: > > + dev_err(dev, "Adapter setup failed (%d)", status); > > + return status; > > +} > > + > > +static int secocec_adap_log_addr(struct cec_adapter *adap, u8 logical_addr) > > +{ > > + u16 enable_val = 0; > > + int status; > > + > > + /* Disable device */ > > + status = smb_rd16(SECOCEC_ENABLE_REG_1, &enable_val); > > + if (status) > > + return status; > > + > > + status = smb_wr16(SECOCEC_ENABLE_REG_1, > > + enable_val & ~SECOCEC_ENABLE_REG_1_CEC); > > + if (status) > > + return status; > > + > > + /* Write logical address */ > > + status = smb_wr16(SECOCEC_DEVICE_LA, logical_addr); > > + if (status) > > + return status; > > + > > + /* Re-enable device */ > > + status = smb_wr16(SECOCEC_ENABLE_REG_1, > > + enable_val | SECOCEC_ENABLE_REG_1_CEC); > > + if (status) > > + return status; > > + > > + return 0; > > +} > > + > > +static int secocec_adap_transmit(struct cec_adapter *adap, u8 attempts, > > + u32 signal_free_time, struct cec_msg *msg) > > +{ > > + struct secocec_data *cec = cec_get_drvdata(adap); > > + struct device *dev = cec->dev; > > + u16 payload_len, payload_id_len, destination, val = 0; > > + u8 *payload_msg; > > + int status; > > + u8 i; > > + > > + /* Device msg len already accounts for header */ > > + payload_id_len = msg->len - 1; > > Does it make sense to continue if payload_id_len == 0 ? > You're going to send the length only (0) and that's it. Is your device > happy with this? Please consider returning here if it is not. I agree with Hans, len == 0 means it is a PING message. > > > + > > + /* Send data length */ > > + status = smb_wr16(SECOCEC_WRITE_DATA_LENGTH, payload_id_len); > > + if (status) > > + goto err; > > + > > + /* Send Operation ID if present */ > > + if (payload_id_len > 0) { > > + status = smb_wr16(SECOCEC_WRITE_OPERATION_ID, msg->msg[1]); > > + if (status) > > + goto err; > > + } > > + /* Send data if present */ > > + if (payload_id_len > 1) { > > + /* Only data; */ > > + payload_len = msg->len - 2; > > + payload_msg = &msg->msg[2]; > > + > > + /* Copy message into registers */ > > + for (i = 0; i < payload_len / 2 + payload_len % 2; i++) { > > Am I wrong or with an odd message length you're going to access memory > after msg->msg[] ends? > > If len is, say, 5, you're going to cycle from 0 to 2 (as 5 / 2 + (5 % > 2) == 3) > > > + /* hi byte */ > > + val = payload_msg[(i << 1) + 1] << 8; > > And here during the last iteration you'll access payload_msg[2 << 1 + 1] > which is after payload_msg end. Please make sure you catch this and > skip this assignment. > > index: 0 1 2 3 4 > payload_msg[5] = [x, y, w, v, z] > i = 0 > val = y << 8 | x; > i = 1 > val = v << 8 | w; > i = 2 > val = ?? < 8 | z; > > I'm not sure how msg->msg is allocated or initialized, nor what is > its actual length, but this feels buggy to me. > > What do you think? Yes, the msg->msg is actually a fixed lenght array of 16 bytes, but the device have only 8 registers of 2 bytes, so it is impossible to go out-of-bound. Also, there is no need of caring of the last odd byte, because msg->len already accounts for the legit bytes in msg->msg. I will fix as suggested by Hans, I found it fine to implement. > > Thanks > j Thanks again, Ettore > > > + > > + /* lo byte */ > > + val |= payload_msg[(i << 1)]; > > + > > + status = smb_wr16(SECOCEC_WRITE_DATA_00 + i, val); > > + if (status) > > + goto err; > > + } > > + } > > + /* Send msg source/destination and fire msg */ > > + destination = msg->msg[0]; > > + status = smb_wr16(SECOCEC_WRITE_BYTE0, destination); > > + if (status) > > + goto err; > > + > > + return 0; > > + > > +err: > > + dev_err(dev, "Transmit failed (%d)", status); > > + return status; > > +} > > + > > +static int secocec_tx_done(struct cec_adapter *adap, u16 status_val) > > +{ > > + int status = 0; > > + > > + if (status_val & SECOCEC_STATUS_TX_ERROR_MASK) { > > + if (status_val & SECOCEC_STATUS_TX_NACK_ERROR) { > > + cec_transmit_attempt_done(adap, CEC_TX_STATUS_NACK); > > + status = -EAGAIN; > > + } else { > > + cec_transmit_attempt_done(adap, CEC_TX_STATUS_ERROR); > > + status = -EIO; > > + } > > + } else { > > + cec_transmit_attempt_done(adap, CEC_TX_STATUS_OK); > > + } > > + > > + /* Reset status reg */ > > + status_val = SECOCEC_STATUS_TX_ERROR_MASK | > > + SECOCEC_STATUS_MSG_SENT_MASK | > > + SECOCEC_STATUS_TX_NACK_ERROR; > > + smb_wr16(SECOCEC_STATUS, status_val); > > + > > + return status; > > +} > > + > > +static int secocec_rx_done(struct cec_adapter *adap, u16 status_val) > > +{ > > + struct secocec_data *cec = cec_get_drvdata(adap); > > + struct device *dev = cec->dev; > > + struct cec_msg msg = { }; > > + bool flag_overflow = false; > > + u8 payload_len, i = 0; > > + u8 *payload_msg; > > + u16 val = 0; > > + int status; > > + > > + if (status_val & SECOCEC_STATUS_RX_OVERFLOW_MASK) { > > + dev_warn(dev, "Received more than 16 bytes. Discarding"); > > + flag_overflow = true; > > + } > > + > > + if (status_val & SECOCEC_STATUS_RX_ERROR_MASK) { > > + dev_warn(dev, "Message received with errors. Discarding"); > > + status = -EIO; > > + goto rxerr; > > + } > > + > > + /* Read message length */ > > + status = smb_rd16(SECOCEC_READ_DATA_LENGTH, &val); > > + if (status) > > + goto err; > > + > > + dev_dbg(dev, "Incoming message (payload len %d):", val); > > + > > + /* Device msg len already accounts for the header */ > > + msg.len = min(val + 1, CEC_MAX_MSG_SIZE); > > + > > + /* Read logical address */ > > + status = smb_rd16(SECOCEC_READ_BYTE0, &val); > > + if (status) > > + goto err; > > + > > + /* device stores source LA and destination */ > > + msg.msg[0] = val; > > + > > + /* Read operation ID */ > > + status = smb_rd16(SECOCEC_READ_OPERATION_ID, &val); > > + if (status) > > + goto err; > > + > > + msg.msg[1] = val; > > + > > + /* Read data if present */ > > + if (msg.len > 1) { > > + payload_len = msg.len - 2; > > + payload_msg = &msg.msg[2]; > > + > > + /* device stores 2 bytes in every 16-bit val */ > > + for (i = 0; i < payload_len / 2 + payload_len % 2; i++) { > > + status = smb_rd16(SECOCEC_READ_DATA_00 + i, &val); > > + if (status) > > + goto err; > > + > > + /* low byte, skipping header */ > > + payload_msg[(i << 1)] = val & 0x00ff; > > + > > + /* hi byte */ > > + payload_msg[(i << 1) + 1] = (val & 0xff00) >> 8; > > + } > > + } > > + > > + cec_received_msg(cec->cec_adap, &msg); > > + > > + /* Reset status reg */ > > + status_val = SECOCEC_STATUS_MSG_RECEIVED_MASK; > > + if (flag_overflow) > > + status_val |= SECOCEC_STATUS_RX_OVERFLOW_MASK; > > + > > + status = smb_wr16(SECOCEC_STATUS, status_val); > > + if (status) > > + goto err; > > + > > + dev_dbg(dev, "Message received successfully"); > > + > > + return 0; > > + > > +rxerr: > > + /* Reset error reg */ > > + status_val = SECOCEC_STATUS_MSG_RECEIVED_MASK | > > + SECOCEC_STATUS_RX_ERROR_MASK; > > + if (flag_overflow) > > + status_val |= SECOCEC_STATUS_RX_OVERFLOW_MASK; > > + smb_wr16(SECOCEC_STATUS, status_val); > > + > > +err: > > + dev_err(dev, "Receive message failed (%d)", status); > > + return status; > > +} > > + > > +struct cec_adap_ops secocec_cec_adap_ops = { > > + /* Low-level callbacks */ > > + .adap_enable = secocec_adap_enable, > > + .adap_log_addr = secocec_adap_log_addr, > > + .adap_transmit = secocec_adap_transmit, > > +}; > > + > > +static irqreturn_t secocec_irq_handler(int irq, void *priv) > > +{ > > + struct secocec_data *cec = priv; > > + struct device *dev = cec->dev; > > + u16 status_val, cec_val, val = 0; > > + int status; > > + > > + /* Read status register */ > > + status = smb_rd16(SECOCEC_STATUS_REG_1, &status_val); > > + if (status) > > + goto err; > > + > > + if (status_val & SECOCEC_STATUS_REG_1_CEC) { > > + /* Read CEC status register */ > > + status = smb_rd16(SECOCEC_STATUS, &cec_val); > > + if (status) > > + goto err; > > + > > + if (cec_val & SECOCEC_STATUS_MSG_RECEIVED_MASK) > > + secocec_rx_done(cec->cec_adap, cec_val); > > + > > + if (cec_val & SECOCEC_STATUS_MSG_SENT_MASK) > > + secocec_tx_done(cec->cec_adap, cec_val); > > + > > + if ((~cec_val & SECOCEC_STATUS_MSG_SENT_MASK) && > > + (~cec_val & SECOCEC_STATUS_MSG_RECEIVED_MASK)) > > + dev_warn(dev, > > + "Message not received or sent, but interrupt fired"); > > + > > + val = SECOCEC_STATUS_REG_1_CEC; > > + } > > + > > + if (status_val & SECOCEC_STATUS_REG_1_IR) { > > + dev_dbg(dev, "IR RC5 Interrupt Caught"); > > + val |= SECOCEC_STATUS_REG_1_IR; > > + /* TODO IRDA RX */ > > + } > > + > > + /* Reset status register */ > > + status = smb_wr16(SECOCEC_STATUS_REG_1, val); > > + if (status) > > + goto err; > > + > > + return IRQ_HANDLED; > > + > > +err: > > + dev_err(dev, "IRQ: Read/Write SMBus operation failed (%d)", status); > > + > > + /* Reset status register */ > > + val = SECOCEC_STATUS_REG_1_CEC | SECOCEC_STATUS_REG_1_IR; > > + smb_wr16(SECOCEC_STATUS_REG_1, val); > > + > > + return IRQ_HANDLED; > > +} > > + > > +struct cec_dmi_match { > > + char *sys_vendor; > > + char *product_name; > > + char *devname; > > + char *conn; > > +}; > > + > > +static const struct cec_dmi_match secocec_dmi_match_table[] = { > > + /* UDOO X86 */ > > + { "SECO", "UDOO x86", "0000:00:02.0", "Port B" }, > > +}; > > + > > +static int secocec_cec_get_notifier(struct cec_notifier **notify) > > +{ > > + int i; > > + > > + for (i = 0 ; i < ARRAY_SIZE(secocec_dmi_match_table) ; ++i) { > > + const struct cec_dmi_match *m = &secocec_dmi_match_table[i]; > > + > > + if (dmi_match(DMI_SYS_VENDOR, m->sys_vendor) && > > + dmi_match(DMI_PRODUCT_NAME, m->product_name)) { > > + struct device *d; > > + > > + /* Find the device, bail out if not yet registered */ > > + d = bus_find_device_by_name(&pci_bus_type, NULL, > > + m->devname); > > + if (!d) > > + return -EPROBE_DEFER; > > + > > + *notify = cec_notifier_get_conn(d, m->conn); > > + > > + return 0; > > + } > > + } > > + > > + return -EINVAL; > > +} > > + > > +static int secocec_acpi_probe(struct secocec_data *sdev) > > +{ > > + struct device *dev = sdev->dev; > > + struct gpio_desc *gpio; > > + int irq = 0; > > + > > + gpio = devm_gpiod_get(dev, NULL, GPIOF_IN); > > + if (IS_ERR(gpio)) { > > + dev_err(dev, "Cannot request interrupt gpio"); > > + return PTR_ERR(gpio); > > + } > > + > > + irq = gpiod_to_irq(gpio); > > + if (irq < 0) { > > + dev_err(dev, "Cannot find valid irq"); > > + return -ENODEV; > > + } > > + dev_dbg(dev, "irq-gpio is bound to IRQ %d", irq); > > + > > + sdev->irq = irq; > > + > > + return 0; > > +} > > + > > +static int secocec_probe(struct platform_device *pdev) > > +{ > > + struct secocec_data *secocec; > > + struct device *dev = &pdev->dev; > > + int ret; > > + u16 val; > > + > > + secocec = devm_kzalloc(dev, sizeof(*secocec), GFP_KERNEL); > > + if (!secocec) > > + return -ENOMEM; > > + > > + dev_set_drvdata(dev, secocec); > > + > > + /* Request SMBus regions */ > > + if (!request_muxed_region(BRA_SMB_BASE_ADDR, 7, "CEC00001")) { > > + dev_err(dev, "Request memory region failed"); > > + return -ENXIO; > > + } > > + > > + secocec->pdev = pdev; > > + secocec->dev = dev; > > + > > + if (!has_acpi_companion(dev)) { > > + dev_dbg(dev, "Cannot find any ACPI companion"); > > + ret = -ENODEV; > > + goto err; > > + } > > + > > + ret = secocec_acpi_probe(secocec); > > + if (ret) { > > + dev_err(dev, "Cannot assign gpio to IRQ"); > > + ret = -ENODEV; > > + goto err; > > + } > > + > > + dev_dbg(dev, "IRQ detected at %d", secocec->irq); > > + > > + /* Firmware version check */ > > + ret = smb_rd16(SECOCEC_VERSION, &val); > > + if (ret) { > > + dev_err(dev, "Cannot check fw version"); > > + goto err; > > + } > > + if (val < SECOCEC_LATEST_FW) { > > + dev_err(dev, "CEC Firmware not supported (v.%04x). Use ver > v.%04x", > > + val, SECOCEC_LATEST_FW); > > + ret = -EINVAL; > > + goto err; > > + } > > + > > + ret = secocec_cec_get_notifier(&secocec->notifier); > > + if (ret) { > > + dev_err(dev, "no CEC notifier available\n"); > > + goto err; > > + } > > + > > + ret = devm_request_threaded_irq(dev, > > + secocec->irq, > > + NULL, > > + secocec_irq_handler, > > + IRQF_TRIGGER_RISING | IRQF_ONESHOT, > > + dev_name(&pdev->dev), secocec); > > + > > + if (ret) { > > + dev_err(dev, "Cannot request IRQ %d", secocec->irq); > > + ret = -EIO; > > + goto err; > > + } > > + > > + /* Allocate CEC adapter */ > > + secocec->cec_adap = cec_allocate_adapter(&secocec_cec_adap_ops, > > + secocec, > > + dev_name(dev), > > + CEC_CAP_DEFAULTS, > > + SECOCEC_MAX_ADDRS); > > + > > + if (IS_ERR(secocec->cec_adap)) { > > + ret = PTR_ERR(secocec->cec_adap); > > + goto err; > > + } > > + > > + ret = cec_register_adapter(secocec->cec_adap, dev); > > + if (ret) > > + goto err_delete_adapter; > > + > > + if (secocec->notifier) > > + cec_register_cec_notifier(secocec->cec_adap, secocec->notifier); > > + > > + platform_set_drvdata(pdev, secocec); > > + > > + dev_dbg(dev, "Device registered"); > > + > > + return ret; > > + > > +err_delete_adapter: > > + cec_delete_adapter(secocec->cec_adap); > > +err: > > + dev_err(dev, "%s device probe failed\n", dev_name(dev)); > > + > > + return ret; > > +} > > + > > +static int secocec_remove(struct platform_device *pdev) > > +{ > > + struct secocec_data *secocec = platform_get_drvdata(pdev); > > + > > + cec_unregister_adapter(secocec->cec_adap); > > + > > + if (secocec->notifier) > > + cec_notifier_put(secocec->notifier); > > + > > + release_region(BRA_SMB_BASE_ADDR, 7); > > + > > + dev_dbg(&pdev->dev, "CEC device removed"); > > + > > + return 0; > > +} > > + > > +#ifdef CONFIG_PM_SLEEP > > +static int secocec_suspend(struct device *dev) > > +{ > > + int status; > > + u16 val; > > + > > + dev_dbg(dev, "Device going to suspend, disabling"); > > + > > + /* Clear the status register */ > > + status = smb_rd16(SECOCEC_STATUS_REG_1, &val); > > + if (status) > > + goto err; > > + > > + status = smb_wr16(SECOCEC_STATUS_REG_1, val); > > + if (status) > > + goto err; > > + > > + /* Disable the interrupts */ > > + status = smb_rd16(SECOCEC_ENABLE_REG_1, &val); > > + if (status) > > + goto err; > > + > > + status = smb_wr16(SECOCEC_ENABLE_REG_1, val & > > + ~SECOCEC_ENABLE_REG_1_CEC & ~SECOCEC_ENABLE_REG_1_IR); > > + if (status) > > + goto err; > > + > > + return 0; > > + > > +err: > > + dev_err(dev, "Suspend failed (err: %d)", status); > > + return status; > > +} > > + > > +static int secocec_resume(struct device *dev) > > +{ > > + int status; > > + u16 val; > > + > > + dev_dbg(dev, "Resuming device from suspend"); > > + > > + /* Clear the status register */ > > + status = smb_rd16(SECOCEC_STATUS_REG_1, &val); > > + if (status) > > + goto err; > > + > > + status = smb_wr16(SECOCEC_STATUS_REG_1, val); > > + if (status) > > + goto err; > > + > > + /* Enable the interrupts */ > > + status = smb_rd16(SECOCEC_ENABLE_REG_1, &val); > > + if (status) > > + goto err; > > + > > + status = smb_wr16(SECOCEC_ENABLE_REG_1, val | SECOCEC_ENABLE_REG_1_CEC); > > + if (status) > > + goto err; > > + > > + dev_dbg(dev, "Device resumed from suspend"); > > + > > + return 0; > > + > > +err: > > + dev_err(dev, "Resume failed (err: %d)", status); > > + return status; > > +} > > + > > +static SIMPLE_DEV_PM_OPS(secocec_pm_ops, secocec_suspend, secocec_resume); > > +#define SECOCEC_PM_OPS (&secocec_pm_ops) > > +#else > > +#define SECOCEC_PM_OPS NULL > > +#endif > > + > > +#ifdef CONFIG_ACPI > > +static const struct acpi_device_id secocec_acpi_match[] = { > > + {"CEC00001", 0}, > > + {}, > > +}; > > + > > +MODULE_DEVICE_TABLE(acpi, secocec_acpi_match); > > +#endif > > + > > +static struct platform_driver secocec_driver = { > > + .driver = { > > + .name = SECOCEC_DEV_NAME, > > + .acpi_match_table = ACPI_PTR(secocec_acpi_match), > > + .pm = SECOCEC_PM_OPS, > > + }, > > + .probe = secocec_probe, > > + .remove = secocec_remove, > > +}; > > + > > +module_platform_driver(secocec_driver); > > + > > +MODULE_DESCRIPTION("SECO CEC X86 Driver"); > > +MODULE_AUTHOR("Ettore Chimenti "); > > +MODULE_LICENSE("Dual BSD/GPL"); > > diff --git a/drivers/media/platform/seco-cec/seco-cec.h b/drivers/media/platform/seco-cec/seco-cec.h > > new file mode 100644 > > index 000000000000..93020900935e > > --- /dev/null > > +++ b/drivers/media/platform/seco-cec/seco-cec.h > > @@ -0,0 +1,130 @@ > > +/* SPDX-License-Identifier: GPL-2.0 OR BSD-3-Clause */ > > +/* > > + * SECO X86 Boards CEC register defines > > + * > > + * Author: Ettore Chimenti > > + * Copyright (C) 2018, SECO Srl. > > + * Copyright (C) 2018, Aidilab Srl. > > + */ > > + > > +#ifndef __SECO_CEC_H__ > > +#define __SECO_CEC_H__ > > + > > +#define SECOCEC_MAX_ADDRS 1 > > +#define SECOCEC_DEV_NAME "secocec" > > +#define SECOCEC_LATEST_FW 0x0f0b > > + > > +#define SMBTIMEOUT 0xffff > > +#define SMB_POLL_UDELAY 10 > > + > > +#define SMBUS_WRITE 0 > > +#define SMBUS_READ 1 > > + > > +#define CMD_BYTE_DATA 0 > > +#define CMD_WORD_DATA 1 > > + > > +/* > > + * SMBus definitons for Braswell > > + */ > > + > > +#define BRA_DONE_STATUS BIT(7) > > +#define BRA_INUSE_STS BIT(6) > > +#define BRA_FAILED_OP BIT(4) > > +#define BRA_BUS_ERR BIT(3) > > +#define BRA_DEV_ERR BIT(2) > > +#define BRA_INTR BIT(1) > > +#define BRA_HOST_BUSY BIT(0) > > +#define BRA_HSTS_ERR_MASK (BRA_FAILED_OP | BRA_BUS_ERR | BRA_DEV_ERR) > > + > > +#define BRA_PEC_EN BIT(7) > > +#define BRA_START BIT(6) > > +#define BRA_LAST__BYTE BIT(5) > > +#define BRA_INTREN BIT(0) > > +#define BRA_SMB_CMD (7 << 2) > > +#define BRA_SMB_CMD_QUICK (0 << 2) > > +#define BRA_SMB_CMD_BYTE (1 << 2) > > +#define BRA_SMB_CMD_BYTE_DATA (2 << 2) > > +#define BRA_SMB_CMD_WORD_DATA (3 << 2) > > +#define BRA_SMB_CMD_PROCESS_CALL (4 << 2) > > +#define BRA_SMB_CMD_BLOCK (5 << 2) > > +#define BRA_SMB_CMD_I2CREAD (6 << 2) > > +#define BRA_SMB_CMD_BLOCK_PROCESS (7 << 2) > > + > > +#define BRA_SMB_BASE_ADDR 0x2040 > > +#define HSTS (BRA_SMB_BASE_ADDR + 0) > > +#define HCNT (BRA_SMB_BASE_ADDR + 2) > > +#define HCMD (BRA_SMB_BASE_ADDR + 3) > > +#define XMIT_SLVA (BRA_SMB_BASE_ADDR + 4) > > +#define HDAT0 (BRA_SMB_BASE_ADDR + 5) > > +#define HDAT1 (BRA_SMB_BASE_ADDR + 6) > > + > > +/* > > + * Microcontroller Address > > + */ > > + > > +#define SECOCEC_MICRO_ADDRESS 0x40 > > + > > +/* > > + * STM32 SMBus Registers > > + */ > > + > > +#define SECOCEC_VERSION 0x00 > > +#define SECOCEC_ENABLE_REG_1 0x01 > > +#define SECOCEC_ENABLE_REG_2 0x02 > > +#define SECOCEC_STATUS_REG_1 0x03 > > +#define SECOCEC_STATUS_REG_2 0x04 > > + > > +#define SECOCEC_STATUS 0x28 > > +#define SECOCEC_DEVICE_LA 0x29 > > +#define SECOCEC_READ_OPERATION_ID 0x2a > > +#define SECOCEC_READ_DATA_LENGTH 0x2b > > +#define SECOCEC_READ_DATA_00 0x2c > > +#define SECOCEC_READ_DATA_02 0x2d > > +#define SECOCEC_READ_DATA_04 0x2e > > +#define SECOCEC_READ_DATA_06 0x2f > > +#define SECOCEC_READ_DATA_08 0x30 > > +#define SECOCEC_READ_DATA_10 0x31 > > +#define SECOCEC_READ_DATA_12 0x32 > > +#define SECOCEC_READ_BYTE0 0x33 > > +#define SECOCEC_WRITE_OPERATION_ID 0x34 > > +#define SECOCEC_WRITE_DATA_LENGTH 0x35 > > +#define SECOCEC_WRITE_DATA_00 0x36 > > +#define SECOCEC_WRITE_DATA_02 0x37 > > +#define SECOCEC_WRITE_DATA_04 0x38 > > +#define SECOCEC_WRITE_DATA_06 0x39 > > +#define SECOCEC_WRITE_DATA_08 0x3a > > +#define SECOCEC_WRITE_DATA_10 0x3b > > +#define SECOCEC_WRITE_DATA_12 0x3c > > +#define SECOCEC_WRITE_BYTE0 0x3d > > + > > +#define SECOCEC_IR_READ_DATA 0x3e > > + > > +/* > > + * Enabling register > > + */ > > + > > +#define SECOCEC_ENABLE_REG_1_CEC 0x1000 > > +#define SECOCEC_ENABLE_REG_1_IR 0x2000 > > +#define SECOCEC_ENABLE_REG_1_IR_PASSTHROUGH 0x4000 > > + > > +/* > > + * Status register > > + */ > > + > > +#define SECOCEC_STATUS_REG_1_CEC SECOCEC_ENABLE_REG_1_CEC > > +#define SECOCEC_STATUS_REG_1_IR SECOCEC_ENABLE_REG_1_IR > > +#define SECOCEC_STATUS_REG_1_IR_PASSTHR SECOCEC_ENABLE_REG_1_IR_PASSTHR > > + > > +/* > > + * Status data > > + */ > > + > > +#define SECOCEC_STATUS_MSG_RECEIVED_MASK BIT(0) > > +#define SECOCEC_STATUS_RX_ERROR_MASK BIT(1) > > +#define SECOCEC_STATUS_MSG_SENT_MASK BIT(2) > > +#define SECOCEC_STATUS_TX_ERROR_MASK BIT(3) > > + > > +#define SECOCEC_STATUS_TX_NACK_ERROR BIT(4) > > +#define SECOCEC_STATUS_RX_OVERFLOW_MASK BIT(5) > > + > > +#endif /* __SECO_CEC_H__ */ > > -- > > 2.18.0 > >