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

From: kbuild test robot
Date: Wed Nov 01 2017 - 00:16:55 EST

Next message: Cong Wang: "Re: linux-next: manual merge of the net-next tree with the net tree"
Previous message: gengdongjiu: "Re: [RFC/RFT PATCH 3/6] ACPI / APEI: Replace ioremap_page_range() with fixmap"
Next in thread: kbuild test robot: "Re: [PATCH] at24: support eeproms that do not roll over page reads."
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

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: .config.gz
Description: application/gzip

Next message: Cong Wang: "Re: linux-next: manual merge of the net-next tree with the net tree"
Previous message: gengdongjiu: "Re: [RFC/RFT PATCH 3/6] ACPI / APEI: Replace ioremap_page_range() with fixmap"
Next in thread: kbuild test robot: "Re: [PATCH] at24: support eeproms that do not roll over page reads."
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]