From patchwork Mon May 4 11:19:34 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: AceLan Kao X-Patchwork-Id: 1282436 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=none (no SPF record) smtp.mailfrom=lists.ubuntu.com (client-ip=91.189.94.19; helo=huckleberry.canonical.com; envelope-from=kernel-team-bounces@lists.ubuntu.com; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=canonical.com Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.a=rsa-sha256 header.s=20161025 header.b=AM9wJ8hY; dkim-atps=neutral Received: from huckleberry.canonical.com (huckleberry.canonical.com [91.189.94.19]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 49G0kV5ZT1z9sSd; Mon, 4 May 2020 21:19:46 +1000 (AEST) Received: from localhost ([127.0.0.1] helo=huckleberry.canonical.com) by huckleberry.canonical.com with esmtp (Exim 4.86_2) (envelope-from ) id 1jVZ8Q-0002jd-Fn; Mon, 04 May 2020 11:19:42 +0000 Received: from mail-pg1-f196.google.com ([209.85.215.196]) by huckleberry.canonical.com with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.86_2) (envelope-from ) id 1jVZ8O-0002j0-6h for kernel-team@lists.ubuntu.com; Mon, 04 May 2020 11:19:40 +0000 Received: by mail-pg1-f196.google.com with SMTP id l12so5457773pgr.10 for ; Mon, 04 May 2020 04:19:40 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=sender:from:to:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=zp5T0was/GgxgLRY9cBcsr59CCktYGs1nKAfSe8qD5I=; b=AM9wJ8hYFlmmj6JgPZqc6TIbPThPHigUVuIgk2JtIOxsBE7lQgcc6hsuJyUhO6ZziE Yhm4tfj5I6AXXCp8vUc6zNSGduPySUmZL+NOd5EidyCEQnlDyYllT4wA4DQxywrqWq7d 2ZYDYdfE9rCYBKgu8cwDh43s1AkN/Huozg34tKyP5YatoZ1xgB5FCroy4dubuSQ0sdal qy3EwWHZfHJ+bqwqPekfxpkNIzxmM106X6GTJ7wC1emU1M6T8MMjSUzOsXKetC6UlQLY LXzKiaUIVN+qR5To6CG5Sqw1yA9qekMPZwUaxHWZ+Q/XQ8JCH6otbDC6IQt+n7LF2jaJ e4eg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:sender:from:to:subject:date:message-id :in-reply-to:references:mime-version:content-transfer-encoding; bh=zp5T0was/GgxgLRY9cBcsr59CCktYGs1nKAfSe8qD5I=; b=IS81Som7UgBKRFiu3x3jGACZMFdSr3iaCcACtsOiW2xkfQnYaWbUf9jt0ITxuVL9Jw p39pfd0IbQNDP70kLkCoKPxUlrWEXCkqjk/hdal+tK+8lzQQ64RWbITGCe8vTlxXKS50 OT0M9qMdBuiWz7WfzuOtVOL9LR+H554YRk1zaZtZf5rAPzg/PruSIv75DDHTNGwCfPbg JPmtEV9vPITZ30AX724RYf0T/osqe/0JycSzNzbMTlMNiG/udKcMiy44lBqD0yD0l+oz i9k4+YisbwzCpDA7o4eOaNxqbRxgGrnynfchQ+kcJ9DFuBeyCfONAnuUlueYlW+XBCsl ISOQ== X-Gm-Message-State: AGi0PuZRT52rJtfYeF0s2veNiBRj7ClzsbXbyVzh0LRKDoajfwy7FW82 Qtp+965MHc5dVcci+TRTxJc7vh8f X-Google-Smtp-Source: APiQypK3tM0Ax9o1M6Pp2YxIarbEEN6Fp/TtKxOhwVaAm5Y0ckogL5o6DCl6slCxeoxnHOXm3hs3PA== X-Received: by 2002:a63:7f1a:: with SMTP id a26mr16783496pgd.252.1588591178251; Mon, 04 May 2020 04:19:38 -0700 (PDT) Received: from localhost (220-135-95-34.HINET-IP.hinet.net. [220.135.95.34]) by smtp.gmail.com with ESMTPSA id q11sm8602191pfl.97.2020.05.04.04.19.37 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 04 May 2020 04:19:37 -0700 (PDT) From: AceLan Kao To: kernel-team@lists.ubuntu.com Subject: [PATCH 1/1][SRU][Bionic] UBUNTU: SAUCE: at24: add 16-bit width registers support Date: Mon, 4 May 2020 19:19:34 +0800 Message-Id: <20200504111934.457277-2-acelan.kao@canonical.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20200504111934.457277-1-acelan.kao@canonical.com> References: <20200504111934.457277-1-acelan.kao@canonical.com> MIME-Version: 1.0 X-BeenThere: kernel-team@lists.ubuntu.com X-Mailman-Version: 2.1.20 Precedence: list List-Id: Kernel team discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: kernel-team-bounces@lists.ubuntu.com Sender: "kernel-team" From: Pieri BugLink: https://bugs.launchpad.net/bugs/1876699 This allows to access data with 16-bit width of registers via i2c SMBus block functions. The multi-command sequence of the reading function is not safe and may read the wrong data from other address if other commands are sent in-between the SMBus commands in the read function. Signed-off-by: AceLan Kao --- drivers/misc/eeprom/Kconfig | 5 +- drivers/misc/eeprom/at24.c | 139 +++++++++++++++++++++++++++++++----- 2 files changed, 127 insertions(+), 17 deletions(-) diff --git a/drivers/misc/eeprom/Kconfig b/drivers/misc/eeprom/Kconfig index 3f93e4564cab9..27b0e8ee04926 100644 --- a/drivers/misc/eeprom/Kconfig +++ b/drivers/misc/eeprom/Kconfig @@ -23,7 +23,10 @@ config EEPROM_AT24 If you use this with an SMBus adapter instead of an I2C adapter, full functionality is not available. Only smaller devices are - supported (24c16 and below, max 4 kByte). + supported via block reads (24c16 and below, max 4 kByte). + Larger devices that use 16-bit addresses will only work with + individual byte reads, which is very slow in general and is unsafe + in multi-master SMBus topologies. This driver can also be built as a module. If so, the module will be called at24. diff --git a/drivers/misc/eeprom/at24.c b/drivers/misc/eeprom/at24.c index 7554b001b502f..614b5b2c5bbd9 100644 --- a/drivers/misc/eeprom/at24.c +++ b/drivers/misc/eeprom/at24.c @@ -3,6 +3,7 @@ * * Copyright (C) 2005-2007 David Brownell * Copyright (C) 2008 Wolfram Sang, Pengutronix + * Copyright (C) 2015 Extreme Engineering Solutions, Inc. * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -52,7 +53,7 @@ * Other than binding model, current differences from "eeprom" driver are * that this one handles write access and isn't restricted to 24c02 devices. * It also handles larger devices (32 kbit and up) with two-byte addresses, - * which won't work on pure SMBus systems. + * which don't work without risks on pure SMBus systems. */ struct at24_data { @@ -246,6 +247,88 @@ MODULE_DEVICE_TABLE(acpi, at24_acpi_ids); * one "eeprom" file not four, but larger reads would fail when * they crossed certain pages. */ + +/* + * Write a byte to an AT24 device using SMBus cycles. + */ +static inline s32 at24_smbus_write_byte_data(struct at24_data *at24, + struct i2c_client *client, u16 offset, u8 value) +{ + if (!(at24->chip.flags & AT24_FLAG_ADDR16)) + return i2c_smbus_write_byte_data(client, offset, value); + + /* + * Emulate I2C multi-byte write by using SMBus "write word" + * cycle. We split up the 16-bit offset among the "command" + * byte and the first data byte. + */ + return i2c_smbus_write_word_data(client, offset >> 8, + (value << 8) | (offset & 0xff)); +} + +/* + * Write block data to an AT24 device using SMBus cycles. + */ +static inline s32 at24_smbus_write_i2c_block_data(struct at24_data *at24, + const struct i2c_client *client, u16 off, u8 len, const u8 *vals) +{ + s32 res; + + if (!(at24->chip.flags & AT24_FLAG_ADDR16)) + return i2c_smbus_write_i2c_block_data(client, off, len, vals); + + /* Insert extra address byte into data stream */ + at24->writebuf[0] = off & 0xff; + memcpy(&at24->writebuf[1], vals, len); + + res = i2c_smbus_write_i2c_block_data(client, off >> 8, len + 1, + at24->writebuf); + + return res; +} + +/* + * Read block data from an AT24 device using SMBus cycles. + */ +static inline s32 at24_smbus_read_block_data(struct at24_data *at24, + const struct i2c_client *client, u16 off, u8 len, u8 *vals) +{ + int count; + s32 res; + + if (!(at24->chip.flags & AT24_FLAG_ADDR16)) + return i2c_smbus_read_i2c_block_data_or_emulated(client, off, + len, vals); + + /* + * Emulate I2C multi-byte read by using SMBus "write byte" and + * "receive byte". This is slightly unsafe since there is an + * additional STOP involved, which exposes the SMBus and (this + * device!) to takeover by another bus master. However, it's the + * only way to work on SMBus-only controllers when talking to + * EEPROMs with multi-byte addresses. + */ + + /* Address "dummy" write */ + res = i2c_smbus_write_byte_data(client, off >> 8, off & 0xff); + if (res < 0) + return res; + + count = 0; + do { + /* Current Address Read */ + res = i2c_smbus_read_byte(client); + if (res < 0) + break; + + *(vals++) = res; + count++; + len--; + } while (len > 0); + + return count; +} + static struct i2c_client *at24_translate_offset(struct at24_data *at24, unsigned int *offset) { @@ -286,10 +369,8 @@ static ssize_t at24_eeprom_read_smbus(struct at24_data *at24, char *buf, */ read_time = jiffies; - status = i2c_smbus_read_i2c_block_data_or_emulated(client, - offset, - count, buf); - + status = at24_smbus_read_block_data(at24, client, + offset, count, buf); dev_dbg(&client->dev, "read %zu@%d --> %d (%ld)\n", count, offset, status, jiffies); @@ -508,8 +589,8 @@ static ssize_t at24_eeprom_write_smbus_block(struct at24_data *at24, */ write_time = jiffies; - status = i2c_smbus_write_i2c_block_data(client, - offset, count, buf); + status = at24_smbus_write_i2c_block_data(at24, + client, offset, count, buf); if (status == 0) status = count; @@ -543,7 +624,8 @@ static ssize_t at24_eeprom_write_smbus_byte(struct at24_data *at24, */ write_time = jiffies; - status = i2c_smbus_write_byte_data(client, offset, buf[0]); + status = at24_smbus_write_byte_data(at24, client, offset, + buf[0]); if (status == 0) status = count; @@ -810,13 +892,32 @@ static int at24_probe(struct i2c_client *client, const struct i2c_device_id *id) /* Use I2C operations unless we're stuck with SMBus extensions. */ if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) { - if (chip.flags & AT24_FLAG_ADDR16) - return -EPFNOSUPPORT; - - if (i2c_check_functionality(client->adapter, + if ((chip.flags & AT24_FLAG_ADDR16) && + i2c_check_functionality(client->adapter, + I2C_FUNC_SMBUS_READ_BYTE | + I2C_FUNC_SMBUS_WRITE_BYTE_DATA)) { + /* + * We need SMBUS_WRITE_BYTE_DATA and SMBUS_READ_BYTE to + * implement byte reads for 16-bit address devices. + * This will be slow, but better than nothing (e.g. + * read @ 3.6 KiB/s). It is also unsafe in a multi- + * master topology. + */ + use_smbus = I2C_SMBUS_BYTE_DATA; + } else if (i2c_check_functionality(client->adapter, I2C_FUNC_SMBUS_READ_I2C_BLOCK)) { use_smbus = I2C_SMBUS_I2C_BLOCK_DATA; - } else if (i2c_check_functionality(client->adapter, + } else if ((chip.flags & AT24_FLAG_ADDR16) && + i2c_check_functionality(client->adapter, + I2C_FUNC_SMBUS_WRITE_WORD_DATA)) { + /* + * We need SMBUS_WRITE_WORD_DATA to implement + * byte writes for 16-bit address devices. + */ + use_smbus_write = I2C_SMBUS_BYTE_DATA; + chip.page_size = 1; + } else if (!(chip.flags & AT24_FLAG_ADDR16) && + i2c_check_functionality(client->adapter, I2C_FUNC_SMBUS_READ_WORD_DATA)) { use_smbus = I2C_SMBUS_WORD_DATA; } else if (i2c_check_functionality(client->adapter, @@ -836,6 +937,9 @@ static int at24_probe(struct i2c_client *client, const struct i2c_device_id *id) } } + if (strcmp(client->name, "24c256") == 0) + chip.page_size = 64; + if (chip.flags & AT24_FLAG_TAKE8ADDR) num_addresses = 8; else @@ -881,12 +985,15 @@ static int at24_probe(struct i2c_client *client, const struct i2c_device_id *id) if (writable) { if (!use_smbus || use_smbus_write) { - unsigned write_max = chip.page_size; + unsigned int write_max = chip.page_size; + unsigned int smbus_max = (chip.flags & AT24_FLAG_ADDR16) ? + I2C_SMBUS_BLOCK_MAX - 1 : + I2C_SMBUS_BLOCK_MAX; if (write_max > io_limit) write_max = io_limit; - if (use_smbus && write_max > I2C_SMBUS_BLOCK_MAX) - write_max = I2C_SMBUS_BLOCK_MAX; + if (use_smbus && write_max > smbus_max) + write_max = smbus_max; at24->write_max = write_max; /* buffer (data + address at the beginning) */