From patchwork Thu May 14 10:38: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: 1290082 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=Kpc4tmm6; 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 49N7Ld6sGqz9sVF; Thu, 14 May 2020 20:38:49 +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 1jZBGG-0008EB-El; Thu, 14 May 2020 10:38:44 +0000 Received: from mail-pj1-f67.google.com ([209.85.216.67]) by huckleberry.canonical.com with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.86_2) (envelope-from ) id 1jZBGD-0008Dm-Nk for kernel-team@lists.ubuntu.com; Thu, 14 May 2020 10:38:41 +0000 Received: by mail-pj1-f67.google.com with SMTP id a5so12370875pjh.2 for ; Thu, 14 May 2020 03:38:41 -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=Z93Rv57JOmiks34IlzQLhIe/w56SUhHZSKnOrkINa4g=; b=Kpc4tmm6UlRnSZzPl4KHd2iN8Sa+fU1/nHN4XAWpAS9gA2M9cUg3UhH4/Rrd2lXgNx /IxTFxi5GU6i9wNn08AyrMD+lv+TcFZm/CsMLWAhZ59paOCs4nCMMa04Ptc7AyyigeIc K1mr66v+dXmfogb574gWAJ7uJqITzFDsXdXgfmbE2pOhZv8XbFGUvebuSOra/wMldmbh UBaOTOxHNbNG6K+Dcxhhk7nJDERu7QXgpCRa4ZauprPzeiQWCvHpNjmB4KUrTgP6RUXG oakKvznndCO6IJopfAfrIdXTRiZXqHyoRmFhG0SqgOVc8Rx7zTj7MCldMvCdLU+dUY5R uO/g== 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=Z93Rv57JOmiks34IlzQLhIe/w56SUhHZSKnOrkINa4g=; b=eWMYU/QzEjJcRDJQIEnaNLA+44ZF3EHVUt4/6bfcrPRdE2/v31UMnEKumUxbVQ633Z Y4pnax3l7bg809U5o5ZspgdR2EztAAgSVJwYcupVgKoELYa3Y4E6WAm64nAAYh3m8hrC 8/bzTzVPeq+sIQvlCGbfhr1Dj00F6OTCLEIkP2njJxlYKxjUmTulq6XdS9ebkD7++MF9 6ej2qG2kgagrgKoaG6S6lRO55r63v7Lef6ybhOHfzvsHSQgS8oHWcNl5ruXQWWQa4FqC 3Ni2an16eGnqQEBkyY/8+herGgOzSydR1D8vcIHjWuJ9yLJGxEjZvTDDObEsWJeUlhNs cXAg== X-Gm-Message-State: AGi0Pubm6Z5jzYko43Z7L5oK/tivOmtmtwqnjyXpLxQOSgwY2+xdynjJ yfh8vpBSdIYAkMtC6GODOBh5qMST X-Google-Smtp-Source: APiQypKwAxHlp/FwKfPGP6eoUauDJ2BRzDhh99BkiVkltBj389IfpBn4encQF5GBVFvmMVHXF8z6hA== X-Received: by 2002:a17:90b:438b:: with SMTP id in11mr39634416pjb.139.1589452719323; Thu, 14 May 2020 03:38:39 -0700 (PDT) Received: from localhost (61-220-137-37.HINET-IP.hinet.net. [61.220.137.37]) by smtp.gmail.com with ESMTPSA id p9sm1877956pgb.19.2020.05.14.03.38.38 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 14 May 2020 03:38:38 -0700 (PDT) From: AceLan Kao To: kernel-team@lists.ubuntu.com Subject: [PATCH v2 1/1][SRU][Bionic] UBUNTU: SAUCE: at24: add 16-bit width registers support Date: Thu, 14 May 2020 18:38:34 +0800 Message-Id: <20200514103834.932389-2-acelan.kao@canonical.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20200514103834.932389-1-acelan.kao@canonical.com> References: <20200514103834.932389-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 EEPROM with 16-bit width of registers via i2c SMBus block functions. This commit is derivated from below commit, and then modified and provided by customer https://patchwork.ozlabs.org/patch/545292/ Unfortunately, this commit didn't get merged into mainline in the end. From the discussion in the patch thread and another lengthy discussion[1], it looks like the maintainer is convinced to accept this patch[2]. But it turns out that there is a new idea to test[3] and the result is not good, and the origin patch has been forgotten and didn't get merged in the end. In the discussion, the implementation to support access 16-bit address data is not safe. The multi-command sequence of the read 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. 1. https://lkml.org/lkml/2015/2/4/436 2. https://lkml.org/lkml/2015/3/25/619 3. https://lkml.org/lkml/2015/3/27/99 Signed-off-by: AceLan Kao Signed-off-by: Pieri --- 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) */