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;
From:   Asmaa Mnebhi <Asmaa@mellanox.com>
To:     Vijay Khemka <vijaykhemka@fb.com>, Corey Minyard <minyard@acm.org>,
        Arnd Bergmann <arnd@arndb.de>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        "openipmi-developer@lists.sourceforge.net" 
        <openipmi-developer@lists.sourceforge.net>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
CC:     "cminyard@mvista.com" <cminyard@mvista.com>,
        "joel@jms.id.au" <joel@jms.id.au>,
        "linux-aspeed@lists.ozlabs.org" <linux-aspeed@lists.ozlabs.org>,
        "sdasari@fb.com" <sdasari@fb.com>
Subject: RE: [PATCH v3] drivers: ipmi: Support raw i2c packet in IPMB
Thread-Topic: [PATCH v3] drivers: ipmi: Support raw i2c packet in IPMB
Thread-Index: AQHVmlgl1O96SHtmeUyoyP3vT4129aeJh9MQ
Date:   Wed, 13 Nov 2019 20:32:57 +0000
Message-ID: <DB6PR0501MB2712FAF45EE8CB2D513465A9DA760@DB6PR0501MB2712.eurprd05.prod.outlook.com>
References: <20191113192325.2821207-1-vijaykhemka@fb.com>
In-Reply-To: <20191113192325.2821207-1-vijaykhemka@fb.com>
Accept-Language: en-US
Content-Language: en-US
received-spf: None (protection.outlook.com: mellanox.com does not designate
 permitted sender hosts)
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: bef2e555-82d9-4217-d319-08d76878ab88
X-MS-Exchange-CrossTenant-originalarrivaltime: 13 Nov 2019 20:32:57.8909
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: a652971c-7d2e-4d9b-a6a4-d149256f461b
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: 7VQ2MyFr/CGf4+jzOxAxtWZfkQa3z9n1a/O5QoO4/iLRKAC7qEd2t062xVJ2GlbV1HT88QRFoXdYtYhMP9vyUg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DB6PR0501MB2616
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Inline response:

-----Original Message-----
From: Vijay Khemka <vijaykhemka@fb.com>=20
Sent: Wednesday, November 13, 2019 2:23 PM
To: Corey Minyard <minyard@acm.org>; Arnd Bergmann <arnd@arndb.de>; Greg Kr=
oah-Hartman <gregkh@linuxfoundation.org>; openipmi-developer@lists.sourcefo=
rge.net; linux-kernel@vger.kernel.org
Cc: vijaykhemka@fb.com; cminyard@mvista.com; Asmaa Mnebhi <Asmaa@mellanox.c=
om>; joel@jms.id.au; linux-aspeed@lists.ozlabs.org; sdasari@fb.com
Subject: [PATCH v3] drivers: ipmi: Support raw i2c packet in IPMB

Many IPMB devices doesn't support smbus protocol and current driver support=
 only smbus devices. Added support for raw i2c packets.

User can define use-i2c-block in device tree to use i2c raw transfer.

Asmaa>> Fix the description: "The ipmb_dev_int driver only supports the smb=
us protocol at the moment. Add support for the i2c protocol as well. There =
will be a variable passed by though the device tree or ACPI table which set=
s the configures the protocol as either i2c or smbus."

Signed-off-by: Vijay Khemka <vijaykhemka@fb.com>
---
 drivers/char/ipmi/ipmb_dev_int.c | 48 ++++++++++++++++++++++++++++++++
 1 file changed, 48 insertions(+)

diff --git a/drivers/char/ipmi/ipmb_dev_int.c b/drivers/char/ipmi/ipmb_dev_=
int.c
index ae3bfba27526..16d5d4b636a9 100644
--- a/drivers/char/ipmi/ipmb_dev_int.c
+++ b/drivers/char/ipmi/ipmb_dev_int.c
@@ -63,6 +63,7 @@ struct ipmb_dev {
 	spinlock_t lock;
 	wait_queue_head_t wait_queue;
 	struct mutex file_mutex;
+	bool use_i2c;
 };
=20
Asmaa>> rename this variable : is_i2c_protocol

 static inline struct ipmb_dev *to_ipmb_dev(struct file *file) @@ -112,6 +1=
13,39 @@ static ssize_t ipmb_read(struct file *file, char __user *buf, size=
_t count,
 	return ret < 0 ? ret : count;
 }
=20
+static int ipmb_i2c_write(struct i2c_client *client, u8 *msg) {
+	unsigned char *i2c_buf;
+	struct i2c_msg i2c_msg;
+	ssize_t ret;
+	u8 msg_len;
+
+	/*
+	 * subtract 1 byte (rq_sa) from the length of the msg passed to
+	 * raw i2c_transfer
+	 */
+	msg_len =3D msg[IPMB_MSG_LEN_IDX] - 1;
+
+	i2c_buf =3D kzalloc(msg_len, GFP_KERNEL);

Asmaa >> We do not want to use kzalloc every time you execute this write fu=
nction. It would create so much fragmentation.
You don't really need to use kzalloc anyways.=20

Also, this code chunk is short, so you can call it directly from the write =
function. I don't think you need a separate function for it.

+	if (!i2c_buf)
+		return -EFAULT;
+
+	/* Copy message to buffer except first 2 bytes (length and address) */
+	memcpy(i2c_buf, msg+2, msg_len);
+
+	i2c_msg.addr =3D GET_7BIT_ADDR(msg[RQ_SA_8BIT_IDX]);
+	i2c_msg.flags =3D client->flags &
+			(I2C_M_TEN | I2C_CLIENT_PEC | I2C_CLIENT_SCCB);
Asmaa>> I don't think ipmb supports 10 bit addresses. The max number of bit=
s in the IPMB address field is 8.

+	i2c_msg.len =3D msg_len;
+	i2c_msg.buf =3D i2c_buf;
+
+	ret =3D i2c_transfer(client->adapter, &i2c_msg, 1);
+	kfree(i2c_buf);
+
+	return ret;
+
+}
+
 static ssize_t ipmb_write(struct file *file, const char __user *buf,
 			size_t count, loff_t *ppos)
 {
@@ -133,6 +167,12 @@ static ssize_t ipmb_write(struct file *file, const cha=
r __user *buf,
 	rq_sa =3D GET_7BIT_ADDR(msg[RQ_SA_8BIT_IDX]);
 	netf_rq_lun =3D msg[NETFN_LUN_IDX];
=20
+	/* Check i2c block transfer vs smbus */
+	if (ipmb_dev->use_i2c) {
+		ret =3D ipmb_i2c_write(ipmb_dev->client, msg);
+		return (ret =3D=3D 1) ? count : ret;
+	}
+
 	/*
 	 * subtract rq_sa and netf_rq_lun from the length of the msg passed to
 	 * i2c_smbus_xfer
@@ -277,6 +317,7 @@ static int ipmb_probe(struct i2c_client *client,
 			const struct i2c_device_id *id)
 {
 	struct ipmb_dev *ipmb_dev;
+	struct device_node *np;
 	int ret;
=20
 	ipmb_dev =3D devm_kzalloc(&client->dev, sizeof(*ipmb_dev), @@ -302,6 +343=
,13 @@ static int ipmb_probe(struct i2c_client *client,
 	if (ret)
 		return ret;
=20
+	/* Check if i2c block xmit needs to use instead of smbus */
+	np =3D client->dev.of_node;
+	if (np && of_get_property(np, "use-i2c-block", NULL))
Asmaa>> Rename this variable i2c-protocol. And also, apply this to ACPI as =
well.
+		ipmb_dev->use_i2c =3D true;
+	else
+		ipmb_dev->use_i2c =3D false;
+
 	ipmb_dev->client =3D client;
 	i2c_set_clientdata(client, ipmb_dev);
 	ret =3D i2c_slave_register(client, ipmb_slave_cb);
--
2.17.1