Received: by 2002:a25:ab43:0:0:0:0:0 with SMTP id u61csp293003ybi; Wed, 29 May 2019 21:35:18 -0700 (PDT) X-Google-Smtp-Source: APXvYqzaBsm/17SKq14373laz1plJ8k42iUC74j1OzbF9nTJdmygmS/7jPswKDMLf8/xINs/u/vE X-Received: by 2002:a63:1919:: with SMTP id z25mr1988124pgl.440.1559190918544; Wed, 29 May 2019 21:35:18 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1559190918; cv=none; d=google.com; s=arc-20160816; b=HbGJrsk3FzTyBe6zEle0S20Mwf6Nd4RNuFXdKsKY/rwEBAncRy2bL7AXP7R3cKE4VR 7Xbdx4acWonHQM0FddJ3pKyRBhmTMZl10q9ndjtTdALxYY/hNZohLa34IRntWSpDV88X +qIRLFNFjCMERUMsPIBpgU95idNUmWu/CLJIbAJViCiR2jIXFMJrtzXIeU3K78EW/50L E7F0JLw7nGypz0VuemhZfPkoNTKZpZY6cbh/IsQ4wAhj5OBmKIIflqODWWUn3SHDabPm EPS+b4ka865KSVXKxDlItaC2174YwpkvcclKP7rVB0aQhyIBxRqMdip7AtseF6CJ61o4 pWhw== 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=BQVuOlTLIt7kOletp8svIa6+869Kfh+6ig9xKiK7qRU=; b=tgh4O8Sc//GU6QB9as2TiB63fGdIszmSlqVf5BkHO5MiOz+MKIv1UAwjmIX2wTI0DJ DVrnFEEDn5K+uwtwpQYJuycSvjS6oPRKWDv/2P7P8nXqhMjHEqF7lwPOnxdxiINtTA80 xKr2takTQHzlleVVYDy+zXlr0xf+QFUS1kw6COVeM2RTQxT1cPbmn415tLkQLB/5N17l Eb81Sgo9QuxemqG7johkVnrEe99/6Xvdjguoyl5C/TG4HgQ1zqEAX7X0iGG64o9GUHP2 Gjwo+YujlAPLspDwU/Cyanw1jfMGTXivCW+Sm6+2dPyp/t5iMtuKkf5FXXmfIda3Y9W2 ai2g== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@amazon.com header.s=amazon201209 header.b=chmgD6Ad; 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=QUARANTINE sp=QUARANTINE dis=NONE) header.from=amazon.com Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id f7si2062077pgv.105.2019.05.29.21.35.03; Wed, 29 May 2019 21:35:18 -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=@amazon.com header.s=amazon201209 header.b=chmgD6Ad; 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=QUARANTINE sp=QUARANTINE dis=NONE) header.from=amazon.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1729653AbfE3Ecj (ORCPT + 99 others); Thu, 30 May 2019 00:32:39 -0400 Received: from smtp-fw-9102.amazon.com ([207.171.184.29]:51115 "EHLO smtp-fw-9102.amazon.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1729509AbfE3DNe (ORCPT ); Wed, 29 May 2019 23:13:34 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=amazon.com; i=@amazon.com; q=dns/txt; s=amazon201209; t=1559186013; x=1590722013; h=date:from:to:cc:subject:message-id:references: mime-version:in-reply-to; bh=BQVuOlTLIt7kOletp8svIa6+869Kfh+6ig9xKiK7qRU=; b=chmgD6Ad6p3gY+6ev17PM38i0xMW8xmddLjKZQJQbCAlRTrNRu1jokp4 PVWSjCf6SfONJ5mbP2ysiNFE/rfD15K1TLSnr2RqSBPoMRdXuhLZJ4ZSG qvUHFEvLuDFJCUr/hjQi0aQf+oykBQB3QhM0eUgDV+DCrKLDUkVvLj/H9 c=; X-IronPort-AV: E=Sophos;i="5.60,529,1549929600"; d="scan'208";a="677127364" Received: from sea3-co-svc-lb6-vlan3.sea.amazon.com (HELO email-inbound-relay-2a-69849ee2.us-west-2.amazon.com) ([10.47.22.38]) by smtp-border-fw-out-9102.sea19.amazon.com with ESMTP; 30 May 2019 03:13:31 +0000 Received: from EX13MTAUEA001.ant.amazon.com (pdx1-ws-svc-p6-lb9-vlan3.pdx.amazon.com [10.236.137.198]) by email-inbound-relay-2a-69849ee2.us-west-2.amazon.com (Postfix) with ESMTPS id 3E6DEA21C0; Thu, 30 May 2019 03:13:31 +0000 (UTC) Received: from EX13D04UEA002.ant.amazon.com (10.43.61.61) by EX13MTAUEA001.ant.amazon.com (10.43.61.243) with Microsoft SMTP Server (TLS) id 15.0.1367.3; Thu, 30 May 2019 03:13:30 +0000 Received: from EX13MTAUEA001.ant.amazon.com (10.43.61.82) by EX13D04UEA002.ant.amazon.com (10.43.61.61) with Microsoft SMTP Server (TLS) id 15.0.1367.3; Thu, 30 May 2019 03:13:30 +0000 Received: from localhost (10.94.220.85) by mail-relay.amazon.com (10.43.61.243) with Microsoft SMTP Server id 15.0.1367.3 via Frontend Transport; Thu, 30 May 2019 03:13:30 +0000 Date: Wed, 29 May 2019 20:13:30 -0700 From: Eduardo Valentin To: Haiyue Wang CC: , , , , , , , Subject: Re: [PATCH i2c/slave-mqueue v5] i2c: slave-mqueue: add a slave backend to receive and queue messages Message-ID: <20190530031330.GA5890@u40b0340c692b58f6553c.ant.amazon.com> References: <1524503192-4176-1-git-send-email-haiyue.wang@linux.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Disposition: inline In-Reply-To: <1524503192-4176-1-git-send-email-haiyue.wang@linux.intel.com> User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Apr 24, 2018 at 01:06:32AM +0800, Haiyue Wang wrote: > Some protocols over I2C are designed for bi-directional transferring > messages by using I2C Master Write protocol. Like the MCTP (Management > Component Transport Protocol) and IPMB (Intelligent Platform Management > Bus), they both require that the userspace can receive messages from > I2C dirvers under slave mode. > > This new slave mqueue backend is used to receive and queue messages, it > will exposes these messages to userspace by sysfs bin file. > > Signed-off-by: Haiyue Wang > --- > v4 -> v5: > - Typo: bellowing -> the below > > v3 -> v4: > - Drop the small message after receiving I2C STOP. > > v2 -> v3: > - Just remove the ';' after the end '}' of i2c_slave_mqueue_probe(). > > v1 -> v2: > - Change MQ_MSGBUF_SIZE and MQ_QUEUE_SIZE to be configurable by Kconfig. > --- > Documentation/i2c/slave-mqueue-backend.rst | 125 ++++++++++++++++++ > drivers/i2c/Kconfig | 25 ++++ > drivers/i2c/Makefile | 1 + > drivers/i2c/i2c-slave-mqueue.c | 203 +++++++++++++++++++++++++++++ > 4 files changed, 354 insertions(+) > create mode 100644 Documentation/i2c/slave-mqueue-backend.rst > create mode 100644 drivers/i2c/i2c-slave-mqueue.c > > diff --git a/Documentation/i2c/slave-mqueue-backend.rst b/Documentation/i2c/slave-mqueue-backend.rst > new file mode 100644 > index 0000000..3966cf0 > --- /dev/null > +++ b/Documentation/i2c/slave-mqueue-backend.rst > @@ -0,0 +1,125 @@ > +.. SPDX-License-Identifier: GPL-2.0 > + > +===================================== > +Linux I2C slave message queue backend > +===================================== > + > +:Author: Haiyue Wang > + > +Some protocols over I2C/SMBus are designed for bi-directional transferring > +messages by using I2C Master Write protocol. This requires that both sides > +of the communication have slave addresses. > + > +Like MCTP (Management Component Transport Protocol) and IPMB (Intelligent > +Platform Management Bus), they both require that the userspace can receive > +messages from i2c dirvers under slave mode. > + > +This I2C slave mqueue (message queue) backend is used to receive and queue > +messages from the remote i2c intelligent device; and it will add the target > +slave address (with R/W# bit is always 0) into the message at the first byte, > +so that userspace can use this byte to dispatch the messages into different > +handling modules. Also, like IPMB, the address byte is in its message format, > +it needs it to do checksum. > + > +For messages are time related, so this backend will flush the oldest message > +to queue the newest one. > + > +Link > +---- > +`Intelligent Platform Management Bus > +Communications Protocol Specification > +`_ > + > +`Management Component Transport Protocol (MCTP) > +SMBus/I2C Transport Binding Specification > +`_ > + > +How to use > +---------- > +For example, the I2C5 bus has slave address 0x10, the below command will create > +the related message queue interface: > + > + echo slave-mqueue 0x1010 > /sys/bus/i2c/devices/i2c-5/new_device > + > +Then you can dump the messages like this: > + > + hexdump -C /sys/bus/i2c/devices/5-1010/slave-mqueue > + > +Code Example > +------------ > +*Note: call 'lseek' before 'read', this is a requirement from kernfs' design.* > + > +:: > + > + #include > + #include > + #include > + #include > + #include > + #include > + #include > + > + int main(int argc, char *argv[]) > + { > + int i, r; > + struct pollfd pfd; > + struct timespec ts; > + unsigned char data[256]; > + > + pfd.fd = open(argv[1], O_RDONLY | O_NONBLOCK); > + if (pfd.fd < 0) > + return -1; > + > + pfd.events = POLLPRI; > + > + while (1) { > + r = poll(&pfd, 1, 5000); > + > + if (r < 0) > + break; > + > + if (r == 0 || !(pfd.revents & POLLPRI)) > + continue; > + > + lseek(pfd.fd, 0, SEEK_SET); > + r = read(pfd.fd, data, sizeof(data)); > + if (r <= 0) > + continue; > + > + clock_gettime(CLOCK_MONOTONIC, &ts); > + printf("[%ld.%.9ld] :", ts.tv_sec, ts.tv_nsec); > + for (i = 0; i < r; i++) > + printf(" %02x", data[i]); > + printf("\n"); > + } > + > + close(pfd.fd); > + > + return 0; > + } > + > +Result > +------ > +*./a.out "/sys/bus/i2c/devices/5-1010/slave-mqueue"* > + > +:: > + > + [10183.232500449] : 20 18 c8 2c 78 01 5b > + [10183.479358348] : 20 18 c8 2c 78 01 5b > + [10183.726556812] : 20 18 c8 2c 78 01 5b > + [10183.972605863] : 20 18 c8 2c 78 01 5b > + [10184.220124772] : 20 18 c8 2c 78 01 5b > + [10184.467764166] : 20 18 c8 2c 78 01 5b > + [10193.233421784] : 20 18 c8 2c 7c 01 57 > + [10193.480273460] : 20 18 c8 2c 7c 01 57 > + [10193.726788733] : 20 18 c8 2c 7c 01 57 > + [10193.972781945] : 20 18 c8 2c 7c 01 57 > + [10194.220487360] : 20 18 c8 2c 7c 01 57 > + [10194.468089259] : 20 18 c8 2c 7c 01 57 > + [10203.233433099] : 20 18 c8 2c 80 01 53 > + [10203.481058715] : 20 18 c8 2c 80 01 53 > + [10203.727610472] : 20 18 c8 2c 80 01 53 > + [10203.974044856] : 20 18 c8 2c 80 01 53 > + [10204.220734634] : 20 18 c8 2c 80 01 53 > + [10204.468461664] : 20 18 c8 2c 80 01 53 > + > diff --git a/drivers/i2c/Kconfig b/drivers/i2c/Kconfig > index efc3354..31e57d2 100644 > --- a/drivers/i2c/Kconfig > +++ b/drivers/i2c/Kconfig > @@ -118,6 +118,31 @@ if I2C_SLAVE > config I2C_SLAVE_EEPROM > tristate "I2C eeprom slave driver" > > +config I2C_SLAVE_MQUEUE_MESSAGE_SIZE > + int "The message size of I2C mqueue slave" > + default 120 > + > +config I2C_SLAVE_MQUEUE_QUEUE_SIZE > + int "The queue size of I2C mqueue slave" > + default 32 > + help > + This number MUST be power of 2. > + > +config I2C_SLAVE_MQUEUE > + tristate "I2C mqueue (message queue) slave driver" > + select I2C_SLAVE_MQUEUE_MESSAGE_SIZE > + select I2C_SLAVE_MQUEUE_QUEUE_SIZE > + help > + Some protocols over I2C are designed for bi-directional transferring > + messages by using I2C Master Write protocol. This driver is used to > + receive and queue messages from the remote I2C device. > + > + Userspace can get the messages by reading sysfs file that this driver > + exposes. > + > + This support is also available as a module. If so, the module will be > + called i2c-slave-mqueue. > + > endif > > config I2C_DEBUG_CORE > diff --git a/drivers/i2c/Makefile b/drivers/i2c/Makefile > index 72c94c6..7ec287b 100644 > --- a/drivers/i2c/Makefile > +++ b/drivers/i2c/Makefile > @@ -16,6 +16,7 @@ obj-$(CONFIG_I2C_MUX) += i2c-mux.o > obj-y += algos/ busses/ muxes/ > obj-$(CONFIG_I2C_STUB) += i2c-stub.o > obj-$(CONFIG_I2C_SLAVE_EEPROM) += i2c-slave-eeprom.o > +obj-$(CONFIG_I2C_SLAVE_MQUEUE) += i2c-slave-mqueue.o > > ccflags-$(CONFIG_I2C_DEBUG_CORE) := -DDEBUG > CFLAGS_i2c-core-base.o := -Wno-deprecated-declarations > diff --git a/drivers/i2c/i2c-slave-mqueue.c b/drivers/i2c/i2c-slave-mqueue.c > new file mode 100644 > index 0000000..424f435 > --- /dev/null > +++ b/drivers/i2c/i2c-slave-mqueue.c > @@ -0,0 +1,203 @@ > +// SPDX-License-Identifier: GPL-2.0 > +// Copyright (c) 2017 - 2018, Intel Corporation. > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#define MQ_MSGBUF_SIZE CONFIG_I2C_SLAVE_MQUEUE_MESSAGE_SIZE > +#define MQ_QUEUE_SIZE CONFIG_I2C_SLAVE_MQUEUE_QUEUE_SIZE > +#define MQ_QUEUE_NEXT(x) (((x) + 1) & (MQ_QUEUE_SIZE - 1)) > + > +struct mq_msg { > + int len; > + u8 *buf; > +}; > + > +struct mq_queue { > + struct bin_attribute bin; > + struct kernfs_node *kn; > + > + spinlock_t lock; /* spinlock for queue index handling */ > + int in; > + int out; > + > + struct mq_msg *curr; > + int truncated; /* drop current if truncated */ > + struct mq_msg queue[MQ_QUEUE_SIZE]; > +}; > + > +static int i2c_slave_mqueue_callback(struct i2c_client *client, > + enum i2c_slave_event event, u8 *val) > +{ > + struct mq_queue *mq = i2c_get_clientdata(client); > + struct mq_msg *msg = mq->curr; > + int ret = 0; > + > + switch (event) { > + case I2C_SLAVE_WRITE_REQUESTED: > + mq->truncated = 0; > + > + msg->len = 1; > + msg->buf[0] = client->addr << 1; > + break; > + > + case I2C_SLAVE_WRITE_RECEIVED: > + if (msg->len < MQ_MSGBUF_SIZE) { > + msg->buf[msg->len++] = *val; > + } else { > + dev_err(&client->dev, "message is truncated!\n"); > + mq->truncated = 1; > + ret = -EINVAL; > + } > + break; > + > + case I2C_SLAVE_STOP: > + if (unlikely(mq->truncated || msg->len < 2)) > + break; > + > + spin_lock(&mq->lock); > + mq->in = MQ_QUEUE_NEXT(mq->in); > + mq->curr = &mq->queue[mq->in]; > + mq->curr->len = 0; > + > + /* Flush the oldest message */ > + if (mq->out == mq->in) > + mq->out = MQ_QUEUE_NEXT(mq->out); > + spin_unlock(&mq->lock); > + > + kernfs_notify(mq->kn); > + break; > + > + default: > + *val = 0xFF; > + break; > + } > + > + return ret; > +} > + > +static ssize_t i2c_slave_mqueue_bin_read(struct file *filp, > + struct kobject *kobj, > + struct bin_attribute *attr, > + char *buf, loff_t pos, size_t count) > +{ > + struct mq_queue *mq; > + struct mq_msg *msg; > + unsigned long flags; > + bool more = false; > + ssize_t ret = 0; > + > + mq = dev_get_drvdata(container_of(kobj, struct device, kobj)); > + > + spin_lock_irqsave(&mq->lock, flags); > + if (mq->out != mq->in) { > + msg = &mq->queue[mq->out]; > + > + if (msg->len <= count) { > + ret = msg->len; > + memcpy(buf, msg->buf, ret); > + } else { > + ret = -EOVERFLOW; /* Drop this HUGE one. */ > + } > + > + mq->out = MQ_QUEUE_NEXT(mq->out); > + if (mq->out != mq->in) > + more = true; > + } > + spin_unlock_irqrestore(&mq->lock, flags); > + > + if (more) > + kernfs_notify(mq->kn); > + > + return ret; > +} > + > +static int i2c_slave_mqueue_probe(struct i2c_client *client, > + const struct i2c_device_id *id) > +{ > + struct device *dev = &client->dev; > + struct mq_queue *mq; > + int ret, i; > + void *buf; > + > + mq = devm_kzalloc(dev, sizeof(*mq), GFP_KERNEL); > + if (!mq) > + return -ENOMEM; > + > + BUILD_BUG_ON(!is_power_of_2(MQ_QUEUE_SIZE)); > + > + buf = devm_kmalloc_array(dev, MQ_QUEUE_SIZE, MQ_MSGBUF_SIZE, > + GFP_KERNEL); > + if (!buf) > + return -ENOMEM; > + > + for (i = 0; i < MQ_QUEUE_SIZE; i++) > + mq->queue[i].buf = buf + i * MQ_MSGBUF_SIZE; > + > + i2c_set_clientdata(client, mq); > + > + spin_lock_init(&mq->lock); > + mq->curr = &mq->queue[0]; > + > + sysfs_bin_attr_init(&mq->bin); > + mq->bin.attr.name = "slave-mqueue"; > + mq->bin.attr.mode = 0400; > + mq->bin.read = i2c_slave_mqueue_bin_read; > + mq->bin.size = MQ_MSGBUF_SIZE * MQ_QUEUE_SIZE; > + > + ret = sysfs_create_bin_file(&dev->kobj, &mq->bin); > + if (ret) > + return ret; > + > + mq->kn = kernfs_find_and_get(dev->kobj.sd, mq->bin.attr.name); > + if (!mq->kn) { > + sysfs_remove_bin_file(&dev->kobj, &mq->bin); > + return -EFAULT; > + } > + > + ret = i2c_slave_register(client, i2c_slave_mqueue_callback); > + if (ret) { > + kernfs_put(mq->kn); > + sysfs_remove_bin_file(&dev->kobj, &mq->bin); > + return ret; > + } > + > + return 0; > +} > + > +static int i2c_slave_mqueue_remove(struct i2c_client *client) > +{ > + struct mq_queue *mq = i2c_get_clientdata(client); > + > + i2c_slave_unregister(client); > + > + kernfs_put(mq->kn); > + sysfs_remove_bin_file(&client->dev.kobj, &mq->bin); > + > + return 0; > +} > + > +static const struct i2c_device_id i2c_slave_mqueue_id[] = { > + { "slave-mqueue", 0 }, > + { } > +}; > +MODULE_DEVICE_TABLE(i2c, i2c_slave_mqueue_id); > + Can we have this driver supporting device tree based probing? > +static struct i2c_driver i2c_slave_mqueue_driver = { > + .driver = { > + .name = "i2c-slave-mqueue", > + }, > + .probe = i2c_slave_mqueue_probe, > + .remove = i2c_slave_mqueue_remove, > + .id_table = i2c_slave_mqueue_id, > +}; > +module_i2c_driver(i2c_slave_mqueue_driver); > + > +MODULE_LICENSE("GPL v2"); > +MODULE_AUTHOR("Haiyue Wang "); > +MODULE_DESCRIPTION("I2C slave mode for receiving and queuing messages"); > -- > 2.7.4 > -- All the best, Eduardo Valentin