Re: [PATCH] at24: support eeproms that do not roll over page reads.

[kbuild test robot <lkp@intel.com>]

[-- Attachment #1: Type: text/plain, Size: 10534 bytes --]

Hi Sven,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on linus/master]
[also build test WARNING on v4.14-rc7]
[cannot apply to next-20171018]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Sven-Van-Asbroeck/at24-support-eeproms-that-do-not-roll-over-page-reads/20171101-114231
config: sparc64-allyesconfig (attached as .config)
compiler: sparc64-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=sparc64 

All warnings (new ones prefixed by >>):

   In file included from drivers/misc/eeprom/at24.c:12:0:
   drivers/misc/eeprom/at24.c: In function 'at24_translate_offset':
   include/linux/kernel.h:790:16: warning: comparison of distinct pointer types lacks a cast
     (void) (&min1 == &min2);   \
                   ^
   include/linux/kernel.h:799:2: note: in expansion of macro '__min'
     __min(typeof(x), typeof(y),   \
     ^~~~~
>> drivers/misc/eeprom/at24.c:210:12: note: in expansion of macro 'min'
      *count = min(*count, remainder);
               ^~~

vim +/min +210 drivers/misc/eeprom/at24.c

  > 12	#include <linux/kernel.h>
    13	#include <linux/init.h>
    14	#include <linux/module.h>
    15	#include <linux/slab.h>
    16	#include <linux/delay.h>
    17	#include <linux/mutex.h>
    18	#include <linux/mod_devicetable.h>
    19	#include <linux/log2.h>
    20	#include <linux/bitops.h>
    21	#include <linux/jiffies.h>
    22	#include <linux/property.h>
    23	#include <linux/acpi.h>
    24	#include <linux/i2c.h>
    25	#include <linux/nvmem-provider.h>
    26	#include <linux/platform_data/at24.h>
    27	
    28	/*
    29	 * I2C EEPROMs from most vendors are inexpensive and mostly interchangeable.
    30	 * Differences between different vendor product lines (like Atmel AT24C or
    31	 * MicroChip 24LC, etc) won't much matter for typical read/write access.
    32	 * There are also I2C RAM chips, likewise interchangeable. One example
    33	 * would be the PCF8570, which acts like a 24c02 EEPROM (256 bytes).
    34	 *
    35	 * However, misconfiguration can lose data. "Set 16-bit memory address"
    36	 * to a part with 8-bit addressing will overwrite data. Writing with too
    37	 * big a page size also loses data. And it's not safe to assume that the
    38	 * conventional addresses 0x50..0x57 only hold eeproms; a PCF8563 RTC
    39	 * uses 0x51, for just one example.
    40	 *
    41	 * Accordingly, explicit board-specific configuration data should be used
    42	 * in almost all cases. (One partial exception is an SMBus used to access
    43	 * "SPD" data for DRAM sticks. Those only use 24c02 EEPROMs.)
    44	 *
    45	 * So this driver uses "new style" I2C driver binding, expecting to be
    46	 * told what devices exist. That may be in arch/X/mach-Y/board-Z.c or
    47	 * similar kernel-resident tables; or, configuration data coming from
    48	 * a bootloader.
    49	 *
    50	 * Other than binding model, current differences from "eeprom" driver are
    51	 * that this one handles write access and isn't restricted to 24c02 devices.
    52	 * It also handles larger devices (32 kbit and up) with two-byte addresses,
    53	 * which won't work on pure SMBus systems.
    54	 */
    55	
    56	struct at24_data {
    57		struct at24_platform_data chip;
    58		int use_smbus;
    59		int use_smbus_write;
    60	
    61		ssize_t (*read_func)(struct at24_data *, char *, unsigned int, size_t);
    62		ssize_t (*write_func)(struct at24_data *,
    63				      const char *, unsigned int, size_t);
    64	
    65		/*
    66		 * Lock protects against activities from other Linux tasks,
    67		 * but not from changes by other I2C masters.
    68		 */
    69		struct mutex lock;
    70	
    71		u8 *writebuf;
    72		unsigned write_max;
    73		unsigned num_addresses;
    74	
    75		struct nvmem_config nvmem_config;
    76		struct nvmem_device *nvmem;
    77	
    78		/*
    79		 * Some chips tie up multiple I2C addresses; dummy devices reserve
    80		 * them for us, and we'll use them with SMBus calls.
    81		 */
    82		struct i2c_client *client[];
    83	};
    84	
    85	/*
    86	 * This parameter is to help this driver avoid blocking other drivers out
    87	 * of I2C for potentially troublesome amounts of time. With a 100 kHz I2C
    88	 * clock, one 256 byte read takes about 1/43 second which is excessive;
    89	 * but the 1/170 second it takes at 400 kHz may be quite reasonable; and
    90	 * at 1 MHz (Fm+) a 1/430 second delay could easily be invisible.
    91	 *
    92	 * This value is forced to be a power of two so that writes align on pages.
    93	 */
    94	static unsigned io_limit = 128;
    95	module_param(io_limit, uint, 0);
    96	MODULE_PARM_DESC(io_limit, "Maximum bytes per I/O (default 128)");
    97	
    98	/*
    99	 * Specs often allow 5 msec for a page write, sometimes 20 msec;
   100	 * it's important to recover from write timeouts.
   101	 */
   102	static unsigned write_timeout = 25;
   103	module_param(write_timeout, uint, 0);
   104	MODULE_PARM_DESC(write_timeout, "Time (in ms) to try writes (default 25)");
   105	
   106	#define AT24_SIZE_BYTELEN 5
   107	#define AT24_SIZE_FLAGS 8
   108	
   109	#define AT24_BITMASK(x) (BIT(x) - 1)
   110	
   111	/* create non-zero magic value for given eeprom parameters */
   112	#define AT24_DEVICE_MAGIC(_len, _flags) 		\
   113		((1 << AT24_SIZE_FLAGS | (_flags)) 		\
   114		    << AT24_SIZE_BYTELEN | ilog2(_len))
   115	
   116	/*
   117	 * Both reads and writes fail if the previous write didn't complete yet. This
   118	 * macro loops a few times waiting at least long enough for one entire page
   119	 * write to work while making sure that at least one iteration is run before
   120	 * checking the break condition.
   121	 *
   122	 * It takes two parameters: a variable in which the future timeout in jiffies
   123	 * will be stored and a temporary variable holding the time of the last
   124	 * iteration of processing the request. Both should be unsigned integers
   125	 * holding at least 32 bits.
   126	 */
   127	#define loop_until_timeout(tout, op_time)				\
   128		for (tout = jiffies + msecs_to_jiffies(write_timeout), op_time = 0; \
   129		     op_time ? time_before(op_time, tout) : true;		\
   130		     usleep_range(1000, 1500), op_time = jiffies)
   131	
   132	static const struct i2c_device_id at24_ids[] = {
   133		/* needs 8 addresses as A0-A2 are ignored */
   134		{ "24c00",	AT24_DEVICE_MAGIC(128 / 8,	AT24_FLAG_TAKE8ADDR) },
   135		/* old variants can't be handled with this generic entry! */
   136		{ "24c01",	AT24_DEVICE_MAGIC(1024 / 8,	0) },
   137		{ "24cs01",	AT24_DEVICE_MAGIC(16,
   138					AT24_FLAG_SERIAL | AT24_FLAG_READONLY) },
   139		{ "24c02",	AT24_DEVICE_MAGIC(2048 / 8,	0) },
   140		{ "24cs02",	AT24_DEVICE_MAGIC(16,
   141					AT24_FLAG_SERIAL | AT24_FLAG_READONLY) },
   142		{ "24mac402",	AT24_DEVICE_MAGIC(48 / 8,
   143					AT24_FLAG_MAC | AT24_FLAG_READONLY) },
   144		{ "24mac602",	AT24_DEVICE_MAGIC(64 / 8,
   145					AT24_FLAG_MAC | AT24_FLAG_READONLY) },
   146		/* spd is a 24c02 in memory DIMMs */
   147		{ "spd",	AT24_DEVICE_MAGIC(2048 / 8,
   148					AT24_FLAG_READONLY | AT24_FLAG_IRUGO) },
   149		{ "24c04",	AT24_DEVICE_MAGIC(4096 / 8,	0) },
   150		{ "24cs04",	AT24_DEVICE_MAGIC(16,
   151					AT24_FLAG_SERIAL | AT24_FLAG_READONLY) },
   152		/* 24rf08 quirk is handled at i2c-core */
   153		{ "24c08",	AT24_DEVICE_MAGIC(8192 / 8,	0) },
   154		{ "24cs08",	AT24_DEVICE_MAGIC(16,
   155					AT24_FLAG_SERIAL | AT24_FLAG_READONLY) },
   156		{ "24c16",	AT24_DEVICE_MAGIC(16384 / 8,	0) },
   157		{ "24cs16",	AT24_DEVICE_MAGIC(16,
   158					AT24_FLAG_SERIAL | AT24_FLAG_READONLY) },
   159		{ "24c32",	AT24_DEVICE_MAGIC(32768 / 8,	AT24_FLAG_ADDR16) },
   160		{ "24cs32",	AT24_DEVICE_MAGIC(16,
   161					AT24_FLAG_ADDR16 |
   162					AT24_FLAG_SERIAL |
   163					AT24_FLAG_READONLY) },
   164		{ "24c64",	AT24_DEVICE_MAGIC(65536 / 8,	AT24_FLAG_ADDR16) },
   165		{ "24cs64",	AT24_DEVICE_MAGIC(16,
   166					AT24_FLAG_ADDR16 |
   167					AT24_FLAG_SERIAL |
   168					AT24_FLAG_READONLY) },
   169		{ "24c128",	AT24_DEVICE_MAGIC(131072 / 8,	AT24_FLAG_ADDR16) },
   170		{ "24c256",	AT24_DEVICE_MAGIC(262144 / 8,	AT24_FLAG_ADDR16) },
   171		{ "24c512",	AT24_DEVICE_MAGIC(524288 / 8,	AT24_FLAG_ADDR16) },
   172		{ "24c1024",	AT24_DEVICE_MAGIC(1048576 / 8,	AT24_FLAG_ADDR16) },
   173		{ "at24", 0 },
   174		{ /* END OF LIST */ }
   175	};
   176	MODULE_DEVICE_TABLE(i2c, at24_ids);
   177	
   178	static const struct acpi_device_id at24_acpi_ids[] = {
   179		{ "INT3499", AT24_DEVICE_MAGIC(8192 / 8, 0) },
   180		{ }
   181	};
   182	MODULE_DEVICE_TABLE(acpi, at24_acpi_ids);
   183	
   184	/*-------------------------------------------------------------------------*/
   185	
   186	/*
   187	 * This routine supports chips which consume multiple I2C addresses. It
   188	 * computes the addressing information to be used for a given r/w request.
   189	 * Assumes that sanity checks for offset happened at sysfs-layer.
   190	 *
   191	 * Slave address and byte offset derive from the offset. Always
   192	 * set the byte address; on a multi-master board, another master
   193	 * may have changed the chip's "current" address pointer.
   194	 *
   195	 * In case of chips that don't rollover page reads, truncate the count
   196	 * to the nearest page boundary. This might result in the
   197	 * at24_eeprom_read_XXX functions reading fewer bytes than requested,
   198	 * but this is compensated for in at24_read().
   199	 */
   200	static struct i2c_client *at24_translate_offset(struct at24_data *at24,
   201			unsigned int *offset, size_t *count)
   202	{
   203		unsigned int i, bits, remainder;
   204	
   205		bits = (at24->chip.flags & AT24_FLAG_ADDR16) ? 16 : 8;
   206		i = *offset >> bits;
   207		*offset &= AT24_BITMASK(bits);
   208		if ((at24->chip.flags & AT24_FLAG_NO_RDROL) && count) {
   209			remainder = BIT(bits) - *offset;
 > 210			*count = min(*count, remainder);
   211		}
   212	
   213		return at24->client[i];
   214	}
   215	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 52060 bytes --]